From dd801d86b1a2b0fea553d721aebc4c5ab75a326b Mon Sep 17 00:00:00 2001 From: Nafis Faysal <7595238+nafisfaysal@users.noreply.github.com> Date: Fri, 21 Dec 2018 03:15:03 +0600 Subject: [PATCH] Refactoring code to improve HTTP status code (#1005) * refactoring code to improve HTTP status code * refactoring code to improve HTTP status code * refactoring code to improve HTTP status code * refactoring code to improve HTTP status code --- cmd/proxy/actions/basicauth.go | 2 +- cmd/proxy/actions/health.go | 3 ++- cmd/proxy/actions/home.go | 3 ++- cmd/proxy/actions/readiness.go | 5 +++-- cmd/proxy/actions/version.go | 3 ++- pkg/download/latest.go | 2 +- pkg/download/list.go | 2 +- pkg/download/version_module.go | 3 ++- pkg/download/version_zip.go | 3 ++- pkg/log/buffalo_formatter.go | 5 +++-- pkg/middleware/cache_control_test.go | 2 +- pkg/middleware/middleware_test.go | 10 +++++----- 12 files changed, 25 insertions(+), 18 deletions(-) diff --git a/cmd/proxy/actions/basicauth.go b/cmd/proxy/actions/basicauth.go index 7310770f..904e87ee 100644 --- a/cmd/proxy/actions/basicauth.go +++ b/cmd/proxy/actions/basicauth.go @@ -12,7 +12,7 @@ func basicAuth(user, pass string) buffalo.MiddlewareFunc { return func(c buffalo.Context) error { if !checkAuth(c.Request(), user, pass) { c.Response().Header().Set("WWW-Authenticate", `Basic realm="basic auth required"`) - c.Render(401, nil) + c.Render(http.StatusUnauthorized, nil) return nil } diff --git a/cmd/proxy/actions/health.go b/cmd/proxy/actions/health.go index 8d24bfce..d16b9133 100644 --- a/cmd/proxy/actions/health.go +++ b/cmd/proxy/actions/health.go @@ -2,8 +2,9 @@ package actions import ( "github.com/gobuffalo/buffalo" + "net/http" ) func healthHandler(c buffalo.Context) error { - return c.Render(200, nil) + return c.Render(http.StatusOK, nil) } diff --git a/cmd/proxy/actions/home.go b/cmd/proxy/actions/home.go index ca42bcf4..403cb338 100644 --- a/cmd/proxy/actions/home.go +++ b/cmd/proxy/actions/home.go @@ -2,8 +2,9 @@ package actions import ( "github.com/gobuffalo/buffalo" + "net/http" ) func proxyHomeHandler(c buffalo.Context) error { - return c.Render(200, proxy.JSON("Welcome to The Athens Proxy")) + return c.Render(http.StatusOK, proxy.JSON("Welcome to The Athens Proxy")) } diff --git a/cmd/proxy/actions/readiness.go b/cmd/proxy/actions/readiness.go index 52d8fcf5..3a4d2691 100644 --- a/cmd/proxy/actions/readiness.go +++ b/cmd/proxy/actions/readiness.go @@ -3,14 +3,15 @@ package actions import ( "github.com/gobuffalo/buffalo" "github.com/gomods/athens/pkg/storage" + "net/http" ) func getReadinessHandler(s storage.Backend) buffalo.Handler { return func(c buffalo.Context) error { if _, err := s.List(c, "github.com/gomods/athens"); err != nil { - return c.Render(500, nil) + return c.Render(http.StatusInternalServerError, nil) } - return c.Render(200, nil) + return c.Render(http.StatusOK, nil) } } diff --git a/cmd/proxy/actions/version.go b/cmd/proxy/actions/version.go index e530059d..0d707105 100644 --- a/cmd/proxy/actions/version.go +++ b/cmd/proxy/actions/version.go @@ -3,8 +3,9 @@ package actions import ( "github.com/gobuffalo/buffalo" "github.com/gomods/athens/pkg/build" + "net/http" ) func versionHandler(c buffalo.Context) error { - return c.Render(200, proxy.JSON(build.Data())) + return c.Render(http.StatusOK, proxy.JSON(build.Data())) } diff --git a/pkg/download/latest.go b/pkg/download/latest.go index 0f4ba6b2..eb00de2b 100644 --- a/pkg/download/latest.go +++ b/pkg/download/latest.go @@ -20,7 +20,7 @@ func LatestHandler(dp Protocol, lggr log.Entry, eng *render.Engine) buffalo.Hand mod, err := paths.GetModule(c) if err != nil { lggr.SystemErr(errors.E(op, err)) - return c.Render(500, nil) + return c.Render(http.StatusInternalServerError, nil) } info, err := dp.Latest(c, mod) diff --git a/pkg/download/list.go b/pkg/download/list.go index ebb5d9aa..dfeb067b 100644 --- a/pkg/download/list.go +++ b/pkg/download/list.go @@ -21,7 +21,7 @@ func ListHandler(dp Protocol, lggr log.Entry, eng *render.Engine) buffalo.Handle mod, err := paths.GetModule(c) if err != nil { lggr.SystemErr(errors.E(op, err)) - return c.Render(500, nil) + return c.Render(http.StatusInternalServerError, nil) } versions, err := dp.List(c, mod) diff --git a/pkg/download/version_module.go b/pkg/download/version_module.go index 663ff02f..a89097dd 100644 --- a/pkg/download/version_module.go +++ b/pkg/download/version_module.go @@ -5,6 +5,7 @@ import ( "github.com/gobuffalo/buffalo/render" "github.com/gomods/athens/pkg/errors" "github.com/gomods/athens/pkg/log" + "net/http" ) // PathVersionModule URL. @@ -28,6 +29,6 @@ func VersionModuleHandler(dp Protocol, lggr log.Entry, eng *render.Engine) buffa // Calling c.Response().Write will write the header directly // and we would get a 0 status in the buffalo logs. - return c.Render(200, eng.String(string(modBts))) + return c.Render(http.StatusOK, eng.String(string(modBts))) } } diff --git a/pkg/download/version_zip.go b/pkg/download/version_zip.go index 4323504e..cdd8a058 100644 --- a/pkg/download/version_zip.go +++ b/pkg/download/version_zip.go @@ -2,6 +2,7 @@ package download import ( "io" + "net/http" "github.com/gobuffalo/buffalo" "github.com/gobuffalo/buffalo/render" @@ -31,7 +32,7 @@ func VersionZipHandler(dp Protocol, lggr log.Entry, eng *render.Engine) buffalo. // Calling c.Response().Write will write the header directly // and we would get a 0 status in the buffalo logs. - c.Render(200, nil) + c.Render(http.StatusOK, nil) _, err = io.Copy(c.Response(), zip) if err != nil { lggr.SystemErr(errors.E(op, errors.M(mod), errors.V(ver), err)) diff --git a/pkg/log/buffalo_formatter.go b/pkg/log/buffalo_formatter.go index fa7091f6..d2a563e7 100644 --- a/pkg/log/buffalo_formatter.go +++ b/pkg/log/buffalo_formatter.go @@ -2,6 +2,7 @@ package log import ( "fmt" + "net/http" "github.com/fatih/color" "github.com/sirupsen/logrus" @@ -19,9 +20,9 @@ func (buffaloFormatter) Format(entry *logrus.Entry) ([]byte, error) { statusCode, _ := entry.Data["status"].(int) status := fmt.Sprint(statusCode) switch { - case statusCode < 400: + case statusCode < http.StatusBadRequest: status = color.GreenString("%v", status) - case statusCode >= 400 && statusCode < 500: + case statusCode >= http.StatusBadRequest && statusCode < http.StatusInternalServerError: status = color.HiYellowString("%v", status) default: status = color.HiRedString("%v", status) diff --git a/pkg/middleware/cache_control_test.go b/pkg/middleware/cache_control_test.go index 35bc1966..2883d969 100644 --- a/pkg/middleware/cache_control_test.go +++ b/pkg/middleware/cache_control_test.go @@ -9,7 +9,7 @@ import ( ) func TestCacheControl(t *testing.T) { - h := func(c buffalo.Context) error { return c.Render(200, nil) } + h := func(c buffalo.Context) error { return c.Render(http.StatusOK, nil) } a := buffalo.New(buffalo.Options{}) a.GET("/test", h) diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index faa2786b..e83b721a 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -32,7 +32,7 @@ func testConfigFile(t *testing.T) (testConfigFile string) { func middlewareFilterApp(filterFile, registryEndpoint string) (*buffalo.App, error) { h := func(c buffalo.Context) error { - return c.Render(200, nil) + return c.Render(http.StatusOK, nil) } a := buffalo.New(buffalo.Options{}) @@ -85,22 +85,22 @@ func Test_FilterMiddleware(t *testing.T) { paths := []string{"/github.com/gomods/athens/@v/list/", "/github.com/gomods/athens/@v/list"} for _, path := range paths { res := w.JSON(path).Get() - r.Equal(303, res.Code) + r.Equal(http.StatusSeeOther, res.Code) r.Equal(conf.GlobalEndpoint+"/github.com/gomods/athens/@v/list/", res.HeaderMap.Get("Location")) } // Excluded, expects a 403 res := w.JSON("/github.com/athens-artifacts/no-tags/@v/list").Get() - r.Equal(403, res.Code) + r.Equal(http.StatusForbidden, res.Code) // Private, the proxy is working and returns a 200 res = w.JSON("/github.com/athens-artifacts/happy-path/@v/list").Get() - r.Equal(200, res.Code) + r.Equal(http.StatusOK, res.Code) } func hookFilterApp(hook string) *buffalo.App { h := func(c buffalo.Context) error { - return c.Render(200, nil) + return c.Render(http.StatusOK, nil) } a := buffalo.New(buffalo.Options{})