From ec227c2ba3351890974ad2a92bba6bf53988c7a0 Mon Sep 17 00:00:00 2001 From: pancho horrillo Date: Mon, 11 Jan 2021 17:54:54 +0100 Subject: [PATCH] fix: don't flush status immediately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes: #148 Co-authored-by: Roberto Abdelkader Martínez Pérez --- internal/server/data/resource.go | 7 ++++- internal/server/data/resource_test.go | 2 +- internal/server/model/handler.go | 3 +++ internal/server/user/mux/handlerbuilder.go | 6 +++++ .../server/user/mux/handlerbuilder_test.go | 27 ++++++++++++------- .../features/data/response/success.feature | 1 - 6 files changed, 34 insertions(+), 12 deletions(-) diff --git a/internal/server/data/resource.go b/internal/server/data/resource.go index 3c9c098..4e8026a 100644 --- a/internal/server/data/resource.go +++ b/internal/server/data/resource.go @@ -194,7 +194,6 @@ func setResponseStatus(w http.ResponseWriter, r *http.Request, h *model.Handler) httperror.ErrorJSON(w, InvalidStatusCode, http.StatusBadRequest) } else { h.Status = si - h.Writer.WriteHeader(si) } } @@ -223,6 +222,12 @@ func setResponseCookies(w http.ResponseWriter, r *http.Request, h *model.Handler } func setResponseBody(w http.ResponseWriter, r *http.Request, h *model.Handler) { + + if !h.BodyOut { + h.Writer.WriteHeader(h.Status) + h.BodyOut = true + } + n, err := io.Copy(h.Writer, r.Body) if err != nil { if n > 0 { diff --git a/internal/server/data/resource_test.go b/internal/server/data/resource_test.go index e1db6c1..93d00ad 100644 --- a/internal/server/data/resource_test.go +++ b/internal/server/data/resource_test.go @@ -1319,7 +1319,7 @@ func TestSetResponseStatusSetsGivenStatus(t *testing.T) { setResponseStatus(w, r, &h) res := hw.Result() - if res.StatusCode != http.StatusTeapot { + if h.Status != http.StatusTeapot { t.Errorf("Status code mismatch. Expected: 418, Got: %d", res.StatusCode) } } diff --git a/internal/server/model/handler.go b/internal/server/model/handler.go index 851c459..ff9a76e 100644 --- a/internal/server/model/handler.go +++ b/internal/server/model/handler.go @@ -47,4 +47,7 @@ type Handler struct { // SentBytes is the number of sent bytes SentBytes int64 + + // The transfer of the body has started + BodyOut bool } diff --git a/internal/server/user/mux/handlerbuilder.go b/internal/server/user/mux/handlerbuilder.go index c0ec1c7..6a6eece 100644 --- a/internal/server/user/mux/handlerbuilder.go +++ b/internal/server/user/mux/handlerbuilder.go @@ -75,6 +75,12 @@ func handlerBuilder(route model.Route) http.Handler { err = spawner(h, nil, nil) } + // In case of the user not setting /request/body + if !h.BodyOut { + h.Writer.WriteHeader(h.Status) + h.BodyOut = true + } + if err != nil { logger.L.Println(err) } diff --git a/internal/server/user/mux/handlerbuilder_test.go b/internal/server/user/mux/handlerbuilder_test.go index aa53e2a..9ae16a7 100644 --- a/internal/server/user/mux/handlerbuilder_test.go +++ b/internal/server/user/mux/handlerbuilder_test.go @@ -46,8 +46,9 @@ func TestHandlerBuilderCallsSpawner(t *testing.T) { called = true return nil } + w := httptest.NewRecorder() - handlerBuilder(route).ServeHTTP(nil, nil) + handlerBuilder(route).ServeHTTP(w, nil) if !called { t.Error("Didn't call spawner") @@ -64,8 +65,9 @@ func TestHandlerBuilderStoresHandlerInDataHandlers(t *testing.T) { } h := handlerBuilder(route) data.Handlers = data.New() + w := httptest.NewRecorder() - h.ServeHTTP(nil, nil) + h.ServeHTTP(w, nil) if !added { t.Error("handler not added to data.Handlers") @@ -85,8 +87,9 @@ func TestHandlerBuilderStoresTheProperRoute(t *testing.T) { return nil } + w := httptest.NewRecorder() - handlerBuilder(route).ServeHTTP(nil, nil) + handlerBuilder(route).ServeHTTP(w, nil) if !reflect.DeepEqual(got, route) { t.Error("Route not stored properly in the handler") @@ -105,8 +108,9 @@ func TestHandlerBuilderStoresTheProperRequest(t *testing.T) { return nil } r := &http.Request{} + w := httptest.NewRecorder() - handlerBuilder(route).ServeHTTP(nil, r) + handlerBuilder(route).ServeHTTP(w, r) if got != r { t.Error("Request not stored properly in the handler") @@ -145,8 +149,9 @@ func TestHandlerBuilderGeneratesAProperID(t *testing.T) { return nil } + w := httptest.NewRecorder() - handlerBuilder(route).ServeHTTP(nil, nil) + handlerBuilder(route).ServeHTTP(w, nil) if _, err := uuid.Parse(got); err != nil { t.Error("ID not generated properly") @@ -165,8 +170,9 @@ func TestHandlerBuilderCallsSpawnerWithTheStoredHandler(t *testing.T) { return nil } + w := httptest.NewRecorder() - handlerBuilder(route).ServeHTTP(nil, nil) + handlerBuilder(route).ServeHTTP(w, nil) if gotStored != gotPassed { t.Error("Proper handler not passed to spawner()") @@ -196,8 +202,9 @@ func TestHandlerBuilderRemovesHandlerWhenDone(t *testing.T) { spawner = spawn.Spawn idGenerator = uuid.NewUUID route := model.Route{} + w := httptest.NewRecorder() - handlerBuilder(route).ServeHTTP(nil, nil) + handlerBuilder(route).ServeHTTP(w, nil) if len(data.Handlers.ListIDs()) != 0 { t.Error("Handler not removed upon completion") @@ -218,8 +225,9 @@ func TestHandlerBuilderLogToLogHandlerWhenDebugIsEnabled(t *testing.T) { return nil } + w := httptest.NewRecorder() - handlerBuilder(route).ServeHTTP(nil, nil) + handlerBuilder(route).ServeHTTP(w, nil) // NOTE: logStream will write stdout and stderr contents eventually. // We do not have any control the goroutines running logStream, thus we @@ -252,8 +260,9 @@ func TestHandlerBuilderDoesNotLogToLogHandlerWhenDebugIsDisabled(t *testing.T) { return nil } + w := httptest.NewRecorder() - handlerBuilder(route).ServeHTTP(nil, nil) + handlerBuilder(route).ServeHTTP(w, nil) // NOTE: logStream will write stdout and stderr contents eventually. // We do not have any control the goroutines running logStream, thus we diff --git a/spec/test/features/data/response/success.feature b/spec/test/features/data/response/success.feature index 05cff7e..bc51836 100644 --- a/spec/test/features/data/response/success.feature +++ b/spec/test/features/data/response/success.feature @@ -51,7 +51,6 @@ Feature: Setting values for handler response resources in Kapow! server. | /response/body | bodyValue1 | body | - | | /response/stream | bodyValue2 | body | - | - @skip Scenario: Overwrite a resource for the current response. Write twice a on a resource, such as a gzip middleware would require: kapow get /response/body | gzip -c | kapow set /response/body