Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

Commit

Permalink
Magic Numbers => HTTP Status Codes (#1802)
Browse files Browse the repository at this point in the history
  • Loading branch information
laynemcnish authored and markbates committed Oct 17, 2019
1 parent 8bb3200 commit 0855886
Show file tree
Hide file tree
Showing 48 changed files with 319 additions and 251 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ First, thank you so much for wanting to contribute! It means so much that you ca
**Here are the core rules to respect**:
* If you have any question, please consider using the [Slack channel](https://gobuffalo.io/docs/slack) (*#buffalo*, *#buffalo_fr* or *#buffalo-dev* for contribution related questions) or [Stack Overflow](https://stackoverflow.com/questions/tagged/buffalo). We use GitHub issues for **bug reports and feature requests only**.
* All contributors of this project are working on their free time: be patient and kind. :)
* Consider opening an issue **BEFORE** creating a Pull request (PR): you won't loose your time on fixing non-existing bugs, or fixing the wrong bug. Also we can help you to produce the best PR!
* Consider opening an issue **BEFORE** creating a Pull request (PR): you won't lose your time on fixing non-existing bugs, or fixing the wrong bug. Also we can help you to produce the best PR!
* All PRs **MUST** be opened against the *development* branch. If you want to write an hot-fix, we'll first fix the *development* branch before moving the patch to *master* branch.

**WE WILL CLOSE ANY ISSUE OR PR NOT FOLLOWING THESE CORE RULES**.
Expand Down
8 changes: 4 additions & 4 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ func New(opts Options) *App {
a := &App{
Options: opts,
ErrorHandlers: ErrorHandlers{
404: defaultErrorHandler,
500: defaultErrorHandler,
http.StatusNotFound: defaultErrorHandler,
http.StatusInternalServerError: defaultErrorHandler,
},
router: mux.NewRouter(),
moot: &sync.RWMutex{},
Expand All @@ -62,8 +62,8 @@ func New(opts Options) *App {
}
}

a.router.NotFoundHandler = notFoundHandler("path not found: %s %s", 404)
a.router.MethodNotAllowedHandler = notFoundHandler("method not found: %s %s", 405)
a.router.NotFoundHandler = notFoundHandler("path not found: %s %s", http.StatusNotFound)
a.router.MethodNotAllowedHandler = notFoundHandler("method not found: %s %s", http.StatusMethodNotAllowed)

if a.MethodOverride == nil {
a.MethodOverride = MethodOverride
Expand Down
3 changes: 2 additions & 1 deletion binding/binding_custom_decoder_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package binding_test

import (
"net/http"
"testing"

"github.com/gobuffalo/buffalo"
Expand Down Expand Up @@ -33,7 +34,7 @@ func Test_RegisterCustomDecoder(t *testing.T) {
res := w.HTML("/").Post(&U{
Xt: Xt{[]string{"foo"}},
})
r.Equal(200, res.Code)
r.Equal(http.StatusOK, res.Code)

r.Equal([]string{"X"}, ux.Xt.Vals)
}
12 changes: 6 additions & 6 deletions binding/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,21 @@ func App() *buffalo.App {
if err := c.Bind(wf); err != nil {
return err
}
return c.Render(201, render.String(wf.MyFile.Filename))
return c.Render(http.StatusCreated, render.String(wf.MyFile.Filename))
})
a.POST("/named-file", func(c buffalo.Context) error {
wf := &NamedFile{}
if err := c.Bind(wf); err != nil {
return err
}
return c.Render(201, render.String(wf.MyFile.Filename))
return c.Render(http.StatusCreated, render.String(wf.MyFile.Filename))
})
a.POST("/on-context", func(c buffalo.Context) error {
f, err := c.File("MyFile")
if err != nil {
return err
}
return c.Render(201, render.String(f.Filename))
return c.Render(http.StatusCreated, render.String(f.Filename))
})

return a
Expand All @@ -60,7 +60,7 @@ func Test_File_Upload_On_Struct(t *testing.T) {

App().ServeHTTP(res, req)

r.Equal(201, res.Code)
r.Equal(http.StatusCreated, res.Code)
r.Equal("file_test.go", res.Body.String())
}

Expand All @@ -73,7 +73,7 @@ func Test_File_Upload_On_Struct_WithTag(t *testing.T) {

App().ServeHTTP(res, req)

r.Equal(201, res.Code)
r.Equal(http.StatusCreated, res.Code)
r.Equal("file_test.go", res.Body.String())
}

Expand All @@ -86,7 +86,7 @@ func Test_File_Upload_On_Context(t *testing.T) {

App().ServeHTTP(res, req)

r.Equal(201, res.Code)
r.Equal(http.StatusCreated, res.Code)
r.Equal("file_test.go", res.Body.String())
}

Expand Down
4 changes: 2 additions & 2 deletions buffalo/cmd/filetests/generate_underscore.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[{
"path": "actions/person_events.go",
"contains": [
"c.Render(200, r.Auto(c, personEvents))",
"c.Render(200, r.Auto(c, personEvent))"
"c.Render(http.StatusOK, r.Auto(c, personEvents))",
"c.Render(http.StatusOK, r.Auto(c, personEvent))"
]
}]
4 changes: 2 additions & 2 deletions default_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (d *DefaultContext) Render(status int, rr render.Renderer) error {
if er, ok := errx.Unwrap(err).(render.ErrRedirect); ok {
return d.Redirect(er.Status, er.URL)
}
return HTTPError{Status: 500, Cause: err}
return HTTPError{Status: http.StatusInternalServerError, Cause: err}
}

if d.Session() != nil {
Expand All @@ -142,7 +142,7 @@ func (d *DefaultContext) Render(status int, rr render.Renderer) error {
d.Response().WriteHeader(status)
_, err = io.Copy(d.Response(), bb)
if err != nil {
return HTTPError{Status: 500, Cause: err}
return HTTPError{Status: http.StatusInternalServerError, Cause: err}
}

return nil
Expand Down
44 changes: 22 additions & 22 deletions default_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func Test_DefaultContext_Redirect(t *testing.T) {
a := New(Options{})
u := "/foo?bar=http%3A%2F%2Flocalhost%3A3000%2Flogin%2Fcallback%2Ffacebook"
a.GET("/", func(c Context) error {
return c.Redirect(302, u)
return c.Redirect(http.StatusFound, u)
})

w := httptest.New(a)
Expand All @@ -47,23 +47,23 @@ func Test_DefaultContext_Redirect_Helper(t *testing.T) {
{
E: "/foo/baz/",
I: map[string]interface{}{"bar": "baz"},
S: 302,
S: http.StatusPermanentRedirect,
},
{
S: 500,
S: http.StatusInternalServerError,
},
}

for _, tt := range table {
a := New(Options{})
a.GET("/foo/{bar}", func(c Context) error {
return c.Render(200, render.String(c.Param("bar")))
return c.Render(http.StatusOK, render.String(c.Param("bar")))
})
a.GET("/", func(c Context) error {
return c.Redirect(302, "fooPath()", tt.I)
return c.Redirect(http.StatusPermanentRedirect, "fooPath()", tt.I)
})
a.GET("/nomap", func(c Context) error {
return c.Redirect(302, "rootPath()")
return c.Redirect(http.StatusPermanentRedirect, "rootPath()")
})

w := httptest.New(a)
Expand All @@ -72,7 +72,7 @@ func Test_DefaultContext_Redirect_Helper(t *testing.T) {
r.Equal(tt.E, res.Location())

res = w.HTML("/nomap").Get()
r.Equal(302, res.Code)
r.Equal(http.StatusPermanentRedirect, res.Code)
r.Equal("/", res.Location())
}
}
Expand Down Expand Up @@ -103,7 +103,7 @@ func Test_DefaultContext_Param_form(t *testing.T) {
"name": "Mark",
})

r.Equal(200, res.Code)
r.Equal(http.StatusOK, res.Code)
r.Equal("Mark", name)
}

Expand Down Expand Up @@ -137,10 +137,10 @@ func Test_DefaultContext_Render(t *testing.T) {
c.params = url.Values{"name": []string{"Mark"}}
c.Set("greet", "Hello")

err := c.Render(123, render.String(`<%= greet %> <%= params["name"] %>!`))
err := c.Render(http.StatusTeapot, render.String(`<%= greet %> <%= params["name"] %>!`))
r.NoError(err)

r.Equal(123, res.Code)
r.Equal(http.StatusTeapot, res.Code)
r.Equal("Hello Mark!", res.Body.String())
}

Expand All @@ -157,13 +157,13 @@ func Test_DefaultContext_Bind_Default(t *testing.T) {
if err != nil {
return err
}
return c.Render(201, nil)
return c.Render(http.StatusCreated, nil)
})

w := httptest.New(a)
uv := url.Values{"first_name": []string{"Mark"}}
res := w.HTML("/").Post(uv)
r.Equal(201, res.Code)
r.Equal(http.StatusCreated, res.Code)

r.Equal("Mark", user.FirstName)
}
Expand All @@ -181,9 +181,9 @@ func Test_DefaultContext_Bind_No_ContentType(t *testing.T) {
a.POST("/", func(c Context) error {
err := c.Bind(&user)
if err != nil {
return c.Error(422, err)
return c.Error(http.StatusUnprocessableEntity, err)
}
return c.Render(201, nil)
return c.Render(http.StatusCreated, nil)
})

bb := &bytes.Buffer{}
Expand All @@ -192,7 +192,7 @@ func Test_DefaultContext_Bind_No_ContentType(t *testing.T) {
req.Header.Del("Content-Type")
res := httptest.NewRecorder()
a.ServeHTTP(res, req)
r.Equal(422, res.Code)
r.Equal(http.StatusUnprocessableEntity, res.Code)
r.Contains(res.Body.String(), "blank content type")
}

Expand All @@ -209,9 +209,9 @@ func Test_DefaultContext_Bind_Empty_ContentType(t *testing.T) {
a.POST("/", func(c Context) error {
err := c.Bind(&user)
if err != nil {
return c.Error(422, err)
return c.Error(http.StatusUnprocessableEntity, err)
}
return c.Render(201, nil)
return c.Render(http.StatusCreated, nil)
})

bb := &bytes.Buffer{}
Expand All @@ -221,7 +221,7 @@ func Test_DefaultContext_Bind_Empty_ContentType(t *testing.T) {
req.Header.Set("Content-Type", "")
res := httptest.NewRecorder()
a.ServeHTTP(res, req)
r.Equal(422, res.Code)
r.Equal(http.StatusUnprocessableEntity, res.Code)
r.Contains(res.Body.String(), "blank content type")
}

Expand All @@ -240,13 +240,13 @@ func Test_DefaultContext_Bind_Default_BlankFields(t *testing.T) {
if err != nil {
return err
}
return c.Render(201, nil)
return c.Render(http.StatusCreated, nil)
})

w := httptest.New(a)
uv := url.Values{"first_name": []string{""}}
res := w.HTML("/").Post(uv)
r.Equal(201, res.Code)
r.Equal(http.StatusCreated, res.Code)

r.Equal("", user.FirstName)
}
Expand All @@ -264,14 +264,14 @@ func Test_DefaultContext_Bind_JSON(t *testing.T) {
if err != nil {
return err
}
return c.Render(201, nil)
return c.Render(http.StatusCreated, nil)
})

w := httptest.New(a)
res := w.JSON("/").Post(map[string]string{
"first_name": "Mark",
})
r.Equal(201, res.Code)
r.Equal(http.StatusCreated, res.Code)

r.Equal("Mark", user.FirstName)
}
12 changes: 6 additions & 6 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type ErrorHandler func(int, error, Context) error
// ErrorHandlers is used to hold a list of ErrorHandler
// types that can be used to handle specific status codes.
/*
a.ErrorHandlers[500] = func(status int, err error, c buffalo.Context) error {
a.ErrorHandlers[http.StatusInternalServerError] = func(status int, err error, c buffalo.Context) error {
res := c.Response()
res.WriteHeader(status)
res.Write([]byte(err.Error()))
Expand Down Expand Up @@ -88,8 +88,8 @@ func (a *App) PanicHandler(next Handler) Handler {
"app": a,
},
)
eh := a.ErrorHandlers.Get(500)
eh(500, err, c)
eh := a.ErrorHandlers.Get(http.StatusInternalServerError)
eh(http.StatusInternalServerError, err, c)
}
}()
return next(c)
Expand All @@ -102,12 +102,12 @@ func (a *App) defaultErrorMiddleware(next Handler) Handler {
if err == nil {
return nil
}
status := 500
status := http.StatusInternalServerError
// unpack root cause and check for HTTPError
cause := errx.Unwrap(err)
switch cause {
case sql.ErrNoRows:
status = 404
status = http.StatusNotFound
default:
if h, ok := cause.(HTTPError); ok {
status = h.Status
Expand All @@ -130,7 +130,7 @@ func (a *App) defaultErrorMiddleware(next Handler) Handler {
})
// things have really hit the fan if we're here!!
a.Logger.Error(err)
c.Response().WriteHeader(500)
c.Response().WriteHeader(http.StatusInternalServerError)
c.Response().Write([]byte(err.Error()))
}
return nil
Expand Down
Loading

0 comments on commit 0855886

Please sign in to comment.