From c59a2df17960eb6b9451ecb0a1642e915148fe77 Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Date: Wed, 10 Apr 2024 17:09:10 +0530 Subject: [PATCH] Fix status code for response of Post and Delete requests (#452) --- examples/http-server-using-redis/main_test.go | 2 +- examples/using-add-rest-handlers/main_test.go | 4 ++-- examples/using-file-bind/main_test.go | 2 +- examples/using-migrations/main_test.go | 2 +- examples/using-publisher/main_test.go | 4 ++-- pkg/gofr/crud_handlers_test.go | 2 +- pkg/gofr/handler.go | 2 +- pkg/gofr/handler_test.go | 17 +++++++++++++---- pkg/gofr/http/responder.go | 16 ++++++++++++---- pkg/gofr/http/responder_test.go | 4 ++-- 10 files changed, 36 insertions(+), 19 deletions(-) diff --git a/examples/http-server-using-redis/main_test.go b/examples/http-server-using-redis/main_test.go index 331b92ea6..dd2a52b00 100644 --- a/examples/http-server-using-redis/main_test.go +++ b/examples/http-server-using-redis/main_test.go @@ -32,7 +32,7 @@ func TestHTTPServerUsingRedis(t *testing.T) { statusCode int }{ {"post handler", http.MethodPost, []byte(`{"key1":"GoFr"}`), "/redis", - http.StatusOK}, + http.StatusCreated}, {"post invalid body", http.MethodPost, []byte(`{key:abc}`), "/redis", http.StatusInternalServerError}, {"get handler", http.MethodGet, nil, "/redis/key1", http.StatusOK}, diff --git a/examples/using-add-rest-handlers/main_test.go b/examples/using-add-rest-handlers/main_test.go index 8c478e751..04bd4cefb 100644 --- a/examples/using-add-rest-handlers/main_test.go +++ b/examples/using-add-rest-handlers/main_test.go @@ -23,12 +23,12 @@ func TestIntegration_AddRESTHandlers(t *testing.T) { }{ {"empty path", http.MethodGet, "/", nil, 404}, {"success Create", http.MethodPost, "/user", - []byte(`{"id":10, "name":"john doe", "age":99, "isEmployed": true}`), 200}, + []byte(`{"id":10, "name":"john doe", "age":99, "isEmployed": true}`), 201}, {"success GetAll", http.MethodGet, "/user", nil, 200}, {"success Get", http.MethodGet, "/user/10", nil, 200}, {"success Update", http.MethodPut, "/user/10", []byte(`{"name":"john doe", "age":99, "isEmployed": false}`), 200}, - {"success Delete", http.MethodDelete, "/user/10", nil, 200}, + {"success Delete", http.MethodDelete, "/user/10", nil, 204}, } for i, tc := range tests { diff --git a/examples/using-file-bind/main_test.go b/examples/using-file-bind/main_test.go index 96b8f535e..d0c2532d7 100644 --- a/examples/using-file-bind/main_test.go +++ b/examples/using-file-bind/main_test.go @@ -30,7 +30,7 @@ func TestMain_BindError(t *testing.T) { req.Header.Set("content-type", contentType) resp, err = c.Do(req) - assert.Equal(t, 200, resp.StatusCode) + assert.Equal(t, 201, resp.StatusCode) } func generateMultiPartBody(t *testing.T) (*bytes.Buffer, string) { diff --git a/examples/using-migrations/main_test.go b/examples/using-migrations/main_test.go index fc6a743d3..f199ee52b 100644 --- a/examples/using-migrations/main_test.go +++ b/examples/using-migrations/main_test.go @@ -22,7 +22,7 @@ func TestExampleMigration(t *testing.T) { statusCode int }{ {"post new employee with valid data", http.MethodPost, "/employee", - []byte(`{"id":2,"name":"John","gender":"Male","contact_number":1234567890,"dob":"2000-01-01"}`), 200}, + []byte(`{"id":2,"name":"John","gender":"Male","contact_number":1234567890,"dob":"2000-01-01"}`), 201}, {"get employee with valid name", http.MethodGet, "/employee?name=John", nil, 200}, {"get employee does not exist", http.MethodGet, "/employee?name=Invalid", nil, 500}, {"get employee with empty name", http.MethodGet, "/employee", nil, http.StatusInternalServerError}, diff --git a/examples/using-publisher/main_test.go b/examples/using-publisher/main_test.go index c2f132698..8e5233c7c 100644 --- a/examples/using-publisher/main_test.go +++ b/examples/using-publisher/main_test.go @@ -25,7 +25,7 @@ func TestExamplePublisher(t *testing.T) { desc: "valid order", path: "/publish-order", body: []byte(`{"data":{"orderId":"123","status":"pending"}}`), - expectedStatusCode: http.StatusOK, + expectedStatusCode: http.StatusCreated, }, { desc: "invalid order", @@ -37,7 +37,7 @@ func TestExamplePublisher(t *testing.T) { desc: "valid product", path: "/publish-product", body: []byte(`{"data":{"productId":"123","price":"599"}}`), - expectedStatusCode: http.StatusOK, + expectedStatusCode: http.StatusCreated, }, { desc: "invalid product", diff --git a/pkg/gofr/crud_handlers_test.go b/pkg/gofr/crud_handlers_test.go index be19981dc..3ddac1182 100644 --- a/pkg/gofr/crud_handlers_test.go +++ b/pkg/gofr/crud_handlers_test.go @@ -32,7 +32,7 @@ func createTestContext(method, path, id string, body []byte, cont *container.Con testReq.Header.Set("Content-Type", "application/json") gofrReq := gofrHTTP.NewRequest(testReq) - return newContext(gofrHTTP.NewResponder(httptest.NewRecorder()), gofrReq, cont) + return newContext(gofrHTTP.NewResponder(httptest.NewRecorder(), method), gofrReq, cont) } func Test_scanEntity(t *testing.T) { diff --git a/pkg/gofr/handler.go b/pkg/gofr/handler.go index c983e99c5..79c71b119 100644 --- a/pkg/gofr/handler.go +++ b/pkg/gofr/handler.go @@ -30,7 +30,7 @@ type handler struct { } func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - c := newContext(gofrHTTP.NewResponder(w), gofrHTTP.NewRequest(r), h.container) + c := newContext(gofrHTTP.NewResponder(w, r.Method), gofrHTTP.NewRequest(r), h.container) defer c.Trace("gofr-handler").End() c.responder.Respond(h.function(c)) } diff --git a/pkg/gofr/handler_test.go b/pkg/gofr/handler_test.go index ceb21a7a5..86351732f 100644 --- a/pkg/gofr/handler_test.go +++ b/pkg/gofr/handler_test.go @@ -24,17 +24,25 @@ var ( func TestHandler_ServeHTTP(t *testing.T) { testCases := []struct { desc string + method string data interface{} err error statusCode int + body string }{ - {"data is nil and error is nil", nil, nil, http.StatusOK}, - {"data is mil, error is not nil", nil, errTest, http.StatusInternalServerError}, + {"method is get, data is nil and error is nil", http.MethodGet, nil, nil, http.StatusOK, + `{}`}, + {"method is get, data is mil, error is not nil", http.MethodGet, nil, errTest, http.StatusInternalServerError, + `{"error":{"message":"some error"}}`}, + {"method is post, data is nil and error is nil", http.MethodPost, "Created", nil, http.StatusCreated, + `{"data":"Created"}`}, + {"method is delete, data is nil and error is nil", http.MethodDelete, nil, nil, http.StatusNoContent, + `{}`}, } for i, tc := range testCases { w := httptest.NewRecorder() - r := httptest.NewRequest(http.MethodGet, "/", http.NoBody) + r := httptest.NewRequest(tc.method, "/", http.NoBody) c := &container.Container{ Logger: logging.NewLogger(logging.FATAL), } @@ -46,7 +54,8 @@ func TestHandler_ServeHTTP(t *testing.T) { container: c, }.ServeHTTP(w, r) - assert.Equal(t, w.Code, tc.statusCode, "TEST[%d], Failed.\n%s", i, tc.desc) + assert.Containsf(t, w.Body.String(), tc.body, "TEST[%d], Failed.\n%s", i, tc.desc) + assert.Equal(t, tc.statusCode, w.Code, "TEST[%d], Failed.\n%s", i, tc.desc) } } diff --git a/pkg/gofr/http/responder.go b/pkg/gofr/http/responder.go index fccb31570..92c62790e 100644 --- a/pkg/gofr/http/responder.go +++ b/pkg/gofr/http/responder.go @@ -9,13 +9,14 @@ import ( ) // NewResponder creates a new Responder instance from the given http.ResponseWriter.. -func NewResponder(w http.ResponseWriter) *Responder { - return &Responder{w: w} +func NewResponder(w http.ResponseWriter, method string) *Responder { + return &Responder{w: w, method: method} } // Responder encapsulates an http.ResponseWriter and is responsible for crafting structured responses. type Responder struct { - w http.ResponseWriter + w http.ResponseWriter + method string } // Respond sends a response with the given data and handles potential errors, setting appropriate @@ -51,7 +52,14 @@ func (r Responder) Respond(data interface{}, err error) { // HTTPStatusFromError maps errors to HTTP status codes. func (r Responder) HTTPStatusFromError(err error) (status int, errObj interface{}) { if err == nil { - return http.StatusOK, nil + switch r.method { + case http.MethodPost: + return http.StatusCreated, nil + case http.MethodDelete: + return http.StatusNoContent, nil + default: + return http.StatusOK, nil + } } if errors.Is(err, http.ErrMissingFile) { diff --git a/pkg/gofr/http/responder_test.go b/pkg/gofr/http/responder_test.go index 606cd92b2..13e6907c6 100644 --- a/pkg/gofr/http/responder_test.go +++ b/pkg/gofr/http/responder_test.go @@ -11,7 +11,7 @@ import ( ) func TestResponder_Respond(t *testing.T) { - r := NewResponder(httptest.NewRecorder()) + r := NewResponder(httptest.NewRecorder(), http.MethodGet) tests := []struct { desc string @@ -32,7 +32,7 @@ func TestResponder_Respond(t *testing.T) { } func TestResponder_HTTPStatusFromError(t *testing.T) { - r := NewResponder(httptest.NewRecorder()) + r := NewResponder(httptest.NewRecorder(), http.MethodGet) tests := []struct { desc string