From 835f66fdc9ce24a3074055b3b801a7f8e101993f Mon Sep 17 00:00:00 2001 From: Manu Mtz-Almeida Date: Sat, 30 May 2015 14:45:13 +0200 Subject: [PATCH] 404 not found performance improvements benchmark old ns/op new ns/op delta Benchmark404 737 249 -66.21% Benchmark404Many 2330 454 -80.52% benchmark old allocs new allocs delta Benchmark404 3 0 -100.00% Benchmark404Many 10 0 -100.00% benchmark old bytes new bytes delta Benchmark404 115 68 -40.87% Benchmark404Many 235 57 -75.74% --- context.go | 2 +- errors.go | 2 +- gin.go | 18 ++++++++++-------- gin_test.go | 3 --- middleware_test.go | 40 +++++++++++++++++++++++++++++++++++++++- path.go | 2 +- path_test.go | 6 +++--- render/data.go | 2 +- render/text.go | 3 ++- response_writer.go | 11 +++++++++++ routes_test.go | 21 +++++++++++++++++++-- 11 files changed, 88 insertions(+), 22 deletions(-) diff --git a/context.go b/context.go index 7473d3d2..68383525 100644 --- a/context.go +++ b/context.go @@ -283,7 +283,7 @@ func (c *Context) Header(key, value string) { } func (c *Context) Render(code int, r render.Render) { - c.Writer.WriteHeader(code) + c.writermem.WriteHeader(code) if err := r.Write(c.Writer); err != nil { debugPrintError(err) c.AbortWithError(500, err).SetType(ErrorTypeRender) diff --git a/errors.go b/errors.go index 607a583b..b8987882 100644 --- a/errors.go +++ b/errors.go @@ -111,7 +111,7 @@ func (a errorMsgs) Last() *Error { // `` func (a errorMsgs) Errors() []string { if len(a) == 0 { - return []string{} + return nil } errorStrings := make([]string, len(a)) for i, err := range a { diff --git a/gin.go b/gin.go index adb734b3..d263f524 100644 --- a/gin.go +++ b/gin.go @@ -11,7 +11,6 @@ import ( "os" "sync" - "github.com/gin-gonic/gin/binding" "github.com/gin-gonic/gin/render" ) @@ -73,8 +72,8 @@ func New() *Engine { BasePath: "/", }, RedirectTrailingSlash: true, - RedirectFixedPath: true, - HandleMethodNotAllowed: true, + RedirectFixedPath: false, + HandleMethodNotAllowed: false, trees: make(methodTrees, 0, 6), } engine.RouterGroup.engine = engine @@ -285,7 +284,7 @@ func (engine *Engine) serveAutoRedirect(c *Context, root *node, tsr bool) bool { // Try to fix the request path if engine.RedirectFixedPath { fixedPath, found := root.findCaseInsensitivePath( - CleanPath(path), + cleanPath(path), engine.RedirectTrailingSlash, ) if found { @@ -299,14 +298,17 @@ func (engine *Engine) serveAutoRedirect(c *Context, root *node, tsr bool) bool { return false } +var mimePlain = []string{MIMEPlain} + func serveError(c *Context, code int, defaultMessage []byte) { c.writermem.status = code c.Next() - if !c.Writer.Written() { - if c.Writer.Status() == code { - c.Data(-1, binding.MIMEPlain, defaultMessage) + if !c.writermem.Written() { + if c.writermem.Status() == code { + c.writermem.Header()["Content-Type"] = mimePlain + c.Writer.Write(defaultMessage) } else { - c.Writer.WriteHeaderNow() + c.writermem.WriteHeaderNow() } } } diff --git a/gin_test.go b/gin_test.go index 03107dd4..24a192ee 100644 --- a/gin_test.go +++ b/gin_test.go @@ -25,9 +25,6 @@ func TestCreateEngine(t *testing.T) { assert.Equal(t, "/", router.BasePath) assert.Equal(t, router.engine, router) assert.Empty(t, router.Handlers) - assert.True(t, router.RedirectTrailingSlash) - assert.True(t, router.RedirectFixedPath) - assert.True(t, router.HandleMethodNotAllowed) assert.Panics(t, func() { router.addRoute("", "/", HandlersChain{func(_ *Context) {}}) }) assert.Panics(t, func() { router.addRoute("GET", "a", HandlersChain{func(_ *Context) {}}) }) diff --git a/middleware_test.go b/middleware_test.go index 10031cf9..7876c7ef 100644 --- a/middleware_test.go +++ b/middleware_test.go @@ -77,9 +77,10 @@ func TestMiddlewareNoRoute(t *testing.T) { assert.Equal(t, signature, "ACEGHFDB") } -func TestMiddlewareNoMethod(t *testing.T) { +func TestMiddlewareNoMethodEnabled(t *testing.T) { signature := "" router := New() + router.HandleMethodNotAllowed = true router.Use(func(c *Context) { signature += "A" c.Next() @@ -113,6 +114,43 @@ func TestMiddlewareNoMethod(t *testing.T) { assert.Equal(t, signature, "ACEGHFDB") } +func TestMiddlewareNoMethodDisabled(t *testing.T) { + signature := "" + router := New() + router.HandleMethodNotAllowed = false + router.Use(func(c *Context) { + signature += "A" + c.Next() + signature += "B" + }) + router.Use(func(c *Context) { + signature += "C" + c.Next() + signature += "D" + }) + router.NoMethod(func(c *Context) { + signature += "E" + c.Next() + signature += "F" + }, func(c *Context) { + signature += "G" + c.Next() + signature += "H" + }) + router.NoRoute(func(c *Context) { + signature += " X " + }) + router.POST("/", func(c *Context) { + signature += " XX " + }) + // RUN + w := performRequest(router, "GET", "/") + + // TEST + assert.Equal(t, w.Code, 404) + assert.Equal(t, signature, "AC X DB") +} + func TestMiddlewareAbort(t *testing.T) { signature := "" router := New() diff --git a/path.go b/path.go index 40b63bdb..43cdd047 100644 --- a/path.go +++ b/path.go @@ -18,7 +18,7 @@ package gin // that is, replace "/.." by "/" at the beginning of a path. // // If the result of this process is an empty string, "/" is returned -func CleanPath(p string) string { +func cleanPath(p string) string { // Turn empty string into "/" if p == "" { return "/" diff --git a/path_test.go b/path_test.go index 9bd1d933..3bf2799b 100644 --- a/path_test.go +++ b/path_test.go @@ -67,8 +67,8 @@ var cleanTests = []struct { func TestPathClean(t *testing.T) { for _, test := range cleanTests { - assert.Equal(t, CleanPath(test.path), test.result) - assert.Equal(t, CleanPath(test.result), test.result) + assert.Equal(t, cleanPath(test.path), test.result) + assert.Equal(t, cleanPath(test.result), test.result) } } @@ -82,7 +82,7 @@ func TestPathCleanMallocs(t *testing.T) { } for _, test := range cleanTests { - allocs := testing.AllocsPerRun(100, func() { CleanPath(test.result) }) + allocs := testing.AllocsPerRun(100, func() { cleanPath(test.result) }) assert.Equal(t, allocs, 0) } } diff --git a/render/data.go b/render/data.go index 9d06f24e..1e86a43f 100644 --- a/render/data.go +++ b/render/data.go @@ -13,7 +13,7 @@ type Data struct { func (r Data) Write(w http.ResponseWriter) error { if len(r.ContentType) > 0 { - w.Header().Set("Content-Type", r.ContentType) + w.Header()["Content-Type"] = []string{r.ContentType} } w.Write(r.Data) return nil diff --git a/render/text.go b/render/text.go index d1e901fc..af76007b 100644 --- a/render/text.go +++ b/render/text.go @@ -6,6 +6,7 @@ package render import ( "fmt" + "io" "net/http" ) @@ -24,7 +25,7 @@ func (r String) Write(w http.ResponseWriter) error { if len(r.Data) > 0 { fmt.Fprintf(w, r.Format, r.Data...) } else { - w.Write([]byte(r.Format)) + io.WriteString(w, r.Format) } return nil } diff --git a/response_writer.go b/response_writer.go index 9b1077ed..5a75335f 100644 --- a/response_writer.go +++ b/response_writer.go @@ -6,6 +6,7 @@ package gin import ( "bufio" + "io" "net" "net/http" ) @@ -24,6 +25,7 @@ type ( Status() int Size() int + WriteString(string) (int, error) Written() bool WriteHeaderNow() } @@ -35,6 +37,8 @@ type ( } ) +var _ ResponseWriter = &responseWriter{} + func (w *responseWriter) reset(writer http.ResponseWriter) { w.ResponseWriter = writer w.size = noWritten @@ -64,6 +68,13 @@ func (w *responseWriter) Write(data []byte) (n int, err error) { return } +func (w *responseWriter) WriteString(s string) (n int, err error) { + w.WriteHeaderNow() + n, err = io.WriteString(w.ResponseWriter, s) + w.size += n + return +} + func (w *responseWriter) Status() int { return w.status } diff --git a/routes_test.go b/routes_test.go index 6dee24cf..4db2aa84 100644 --- a/routes_test.go +++ b/routes_test.go @@ -61,6 +61,7 @@ func testRouteNotOK(method string, t *testing.T) { func testRouteNotOK2(method string, t *testing.T) { passed := false router := New() + router.HandleMethodNotAllowed = true var methodRoute string if method == "POST" { methodRoute = "GET" @@ -224,9 +225,9 @@ func TestRouterMiddlewareAndStatic(t *testing.T) { assert.Equal(t, w.HeaderMap.Get("x-GIN"), "Gin Framework") } -func TestRouteNotAllowed(t *testing.T) { +func TestRouteNotAllowedEnabled(t *testing.T) { router := New() - + router.HandleMethodNotAllowed = true router.POST("/path", func(c *Context) {}) w := performRequest(router, "GET", "/path") assert.Equal(t, w.Code, http.StatusMethodNotAllowed) @@ -239,8 +240,24 @@ func TestRouteNotAllowed(t *testing.T) { assert.Equal(t, w.Code, http.StatusTeapot) } +func TestRouteNotAllowedDisabled(t *testing.T) { + router := New() + router.HandleMethodNotAllowed = false + router.POST("/path", func(c *Context) {}) + w := performRequest(router, "GET", "/path") + assert.Equal(t, w.Code, 404) + + router.NoMethod(func(c *Context) { + c.String(http.StatusTeapot, "responseText") + }) + w = performRequest(router, "GET", "/path") + assert.Equal(t, w.Body.String(), "404 page not found") + assert.Equal(t, w.Code, 404) +} + func TestRouterNotFound(t *testing.T) { router := New() + router.RedirectFixedPath = true router.GET("/path", func(c *Context) {}) router.GET("/dir/", func(c *Context) {}) router.GET("/", func(c *Context) {})