Skip to content

Commit

Permalink
Set http header entries before writing header
Browse files Browse the repository at this point in the history
Also, update unit tests to use the httptest.ResponseRecorder.Result()
to get the actual result of the http operation.

FAB-18007

Signed-off-by: Will Lahti <[email protected]>
  • Loading branch information
wlahti authored and ale-linux committed Jun 19, 2020
1 parent 9d2258c commit 4a193ed
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 18 deletions.
2 changes: 1 addition & 1 deletion common/flogging/httpadmin/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ func (h *SpecHandler) sendResponse(resp http.ResponseWriter, code int, payload i
payload = &ErrorResponse{Error: err.Error()}
}

resp.Header().Set("Content-Type", "application/json")
resp.WriteHeader(code)

resp.Header().Set("Content-Type", "application/json")
if err := encoder.Encode(payload); err != nil {
h.Logger.Errorw("failed to encode payload", "error", err)
}
Expand Down
12 changes: 6 additions & 6 deletions common/flogging/httpadmin/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,17 @@ var _ = Describe("SpecHandler", func() {
handler.ServeHTTP(resp, req)

Expect(fakeLogging.SpecCallCount()).To(Equal(1))
Expect(resp.Code).To(Equal(http.StatusOK))
Expect(resp.Result().StatusCode).To(Equal(http.StatusOK))
Expect(resp.Body).To(MatchJSON(`{"spec": "the-returned-specification"}`))
Expect(resp.Header().Get("Content-Type")).To(Equal("application/json"))
Expect(resp.Result().Header.Get("Content-Type")).To(Equal("application/json"))
})

It("sets the current logging spec", func() {
req := httptest.NewRequest("PUT", "/ignored", strings.NewReader(`{"spec": "updated-spec"}`))
resp := httptest.NewRecorder()
handler.ServeHTTP(resp, req)

Expect(resp.Code).To(Equal(http.StatusNoContent))
Expect(resp.Result().StatusCode).To(Equal(http.StatusNoContent))
Expect(fakeLogging.ActivateSpecCallCount()).To(Equal(1))
Expect(fakeLogging.ActivateSpecArgsForCall(0)).To(Equal("updated-spec"))
})
Expand All @@ -61,7 +61,7 @@ var _ = Describe("SpecHandler", func() {
handler.ServeHTTP(resp, req)

Expect(fakeLogging.ActivateSpecCallCount()).To(Equal(0))
Expect(resp.Code).To(Equal(http.StatusBadRequest))
Expect(resp.Result().StatusCode).To(Equal(http.StatusBadRequest))
Expect(resp.Body).To(MatchJSON(`{"error": "invalid character 'g' looking for beginning of value"}`))
})
})
Expand All @@ -76,7 +76,7 @@ var _ = Describe("SpecHandler", func() {
resp := httptest.NewRecorder()
handler.ServeHTTP(resp, req)

Expect(resp.Code).To(Equal(http.StatusBadRequest))
Expect(resp.Result().StatusCode).To(Equal(http.StatusBadRequest))
Expect(resp.Body).To(MatchJSON(`{"error": "ewww; that's not right!"}`))
})
})
Expand All @@ -87,7 +87,7 @@ var _ = Describe("SpecHandler", func() {
resp := httptest.NewRecorder()
handler.ServeHTTP(resp, req)

Expect(resp.Code).To(Equal(http.StatusBadRequest))
Expect(resp.Result().StatusCode).To(Equal(http.StatusBadRequest))
Expect(resp.Body).To(MatchJSON(`{"error": "invalid request method: POST"}`))
})

Expand Down
4 changes: 2 additions & 2 deletions core/middleware/request_id_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var _ = Describe("WithRequestID", func() {

It("returns the generated request ID in a header", func() {
chain.ServeHTTP(resp, req)
Expect(resp.Header().Get("X-Request-Id")).To(Equal("generated-id"))
Expect(resp.Result().Header.Get("X-Request-Id")).To(Equal("generated-id"))
})

Context("when a request ID is already present", func() {
Expand All @@ -69,7 +69,7 @@ var _ = Describe("WithRequestID", func() {

It("propagates the request ID to the response", func() {
chain.ServeHTTP(resp, req)
Expect(resp.Header().Get("X-Request-Id")).To(Equal("received-id"))
Expect(resp.Result().Header.Get("X-Request-Id")).To(Equal("received-id"))
})
})
})
10 changes: 5 additions & 5 deletions core/middleware/require_cert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ var _ = Describe("RequireCert", func() {

It("delegates to the next handler when the first verified chain is not empty", func() {
chain.ServeHTTP(resp, req)
Expect(resp.Code).To(Equal(http.StatusOK))
Expect(resp.Result().StatusCode).To(Equal(http.StatusOK))
Expect(handler.ServeHTTPCallCount()).To(Equal(1))
})

Expand All @@ -52,7 +52,7 @@ var _ = Describe("RequireCert", func() {

It("responds with http.StatusUnauthorized", func() {
chain.ServeHTTP(resp, req)
Expect(resp.Code).To(Equal(http.StatusUnauthorized))
Expect(resp.Result().StatusCode).To(Equal(http.StatusUnauthorized))
})

It("does not call the next handler", func() {
Expand All @@ -68,7 +68,7 @@ var _ = Describe("RequireCert", func() {

It("responds with http.StatusUnauthorized", func() {
chain.ServeHTTP(resp, req)
Expect(resp.Code).To(Equal(http.StatusUnauthorized))
Expect(resp.Result().StatusCode).To(Equal(http.StatusUnauthorized))
})

It("does not call the next handler", func() {
Expand All @@ -84,7 +84,7 @@ var _ = Describe("RequireCert", func() {

It("responds with http.StatusUnauthorized", func() {
chain.ServeHTTP(resp, req)
Expect(resp.Code).To(Equal(http.StatusUnauthorized))
Expect(resp.Result().StatusCode).To(Equal(http.StatusUnauthorized))
})

It("does not call the next handler", func() {
Expand All @@ -100,7 +100,7 @@ var _ = Describe("RequireCert", func() {

It("responds with http.StatusUnauthorized", func() {
chain.ServeHTTP(resp, req)
Expect(resp.Code).To(Equal(http.StatusUnauthorized))
Expect(resp.Result().StatusCode).To(Equal(http.StatusUnauthorized))
})

It("does not call the next handler", func() {
Expand Down
8 changes: 4 additions & 4 deletions core/operations/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ func (m *VersionInfoHandler) sendResponse(resp http.ResponseWriter, code int, pa
logger := flogging.MustGetLogger("operations.runner")
logger.Errorw("failed to encode payload", "error", err)
resp.WriteHeader(http.StatusInternalServerError)
} else {
resp.WriteHeader(code)
resp.Header().Set("Content-Type", "application/json")
resp.Write(js)
return
}
resp.Header().Set("Content-Type", "application/json")
resp.WriteHeader(code)
resp.Write(js)
}
1 change: 1 addition & 0 deletions core/operations/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var _ = Describe("Version", func() {
versionInfoHandler := &VersionInfoHandler{Version: "latest"}
versionInfoHandler.ServeHTTP(resp, &http.Request{Method: http.MethodGet})
Expect(resp.Result().StatusCode).To(Equal(http.StatusOK))
Expect(resp.Result().Header.Get("Content-Type")).To(Equal("application/json"))
Expect(resp.Body).To(MatchJSON(`{"Version": "latest"}`))
})

Expand Down

0 comments on commit 4a193ed

Please sign in to comment.