Skip to content

Commit

Permalink
fixed: bad interpretation of http code 201 (#57)
Browse files Browse the repository at this point in the history
* new: added a way to disable gzip

* fixed: bad interpretation of http code 201

* more tests
  • Loading branch information
primalmotion committed May 31, 2019
1 parent c56cba4 commit 9f8df41
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 41 deletions.
1 change: 1 addition & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type config struct {
readTimeout time.Duration
writeTimeout time.Duration
idleTimeout time.Duration
disableCompression bool
disableKeepalive bool
enabled bool
customRootHandlerFunc http.HandlerFunc
Expand Down
2 changes: 0 additions & 2 deletions handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ func makeResponse(ctx *bcontext, response *elemental.Response, cleaner TraceClea
response.StatusCode = ctx.statusCode
if response.StatusCode == 0 {
switch ctx.request.Operation {
case elemental.OperationCreate:
response.StatusCode = http.StatusCreated
case elemental.OperationInfo:
response.StatusCode = http.StatusNoContent
default:
Expand Down
14 changes: 7 additions & 7 deletions handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ func TestHandlers_makeResponse(t *testing.T) {

makeResponse(ctx, response, nil)

Convey("Then response.StatusCode should equal", func() {
So(response.StatusCode, ShouldEqual, http.StatusCreated)
Convey("Then response.StatusCode should be http.StatusOK", func() {
So(response.StatusCode, ShouldEqual, http.StatusOK)
})
})

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

makeResponse(ctx, response, nil)

Convey("Then response.StatusCode should equal", func() {
Convey("Then response.StatusCode should be http.StatusNoContent", func() {
So(response.StatusCode, ShouldEqual, http.StatusNoContent)
})
})
Expand All @@ -152,20 +152,20 @@ func TestHandlers_makeResponse(t *testing.T) {

makeResponse(ctx, response, nil)

Convey("Then response.StatusCode should equal", func() {
Convey("Then response.StatusCode should be http.StatusOK", func() {
So(response.StatusCode, ShouldEqual, http.StatusOK)
})
})

Convey("When I set the operation to Create, status code created, but no data, and I call makeResponse", func() {
Convey("When I set the operation to Create, status code OK, but no data, and I call makeResponse", func() {

ctx.request.Operation = elemental.OperationCreate
ctx.statusCode = http.StatusCreated
ctx.statusCode = http.StatusOK
ctx.outputData = nil

makeResponse(ctx, response, nil)

Convey("Then response.StatusCode should equal", func() {
Convey("Then response.StatusCode should be http.StatusNoContent", func() {
So(response.StatusCode, ShouldEqual, http.StatusNoContent)
})
})
Expand Down
10 changes: 10 additions & 0 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ func OptDisableKeepAlive() Option {
}
}

// OptDisableCompression disables HTTP gzip compression.
//
// This can be useful when your servers run behind
// a proxy for instance.
func OptDisableCompression() Option {
return func(c *config) {
c.restServer.disableCompression = true
}
}

// OptCustomRootHandler configures the custom root (/) handler.
func OptCustomRootHandler(handler http.HandlerFunc) Option {
return func(c *config) {
Expand Down
5 changes: 5 additions & 0 deletions options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ func TestBahamut_Options(t *testing.T) {
So(c.restServer.disableKeepalive, ShouldEqual, true)
})

Convey("Calling OptDisableCompression should work", t, func() {
OptDisableCompression()(&c)
So(c.restServer.disableCompression, ShouldEqual, true)
})

Convey("Calling OptCustomRootHandler should work", t, func() {
h := func(http.ResponseWriter, *http.Request) {}
OptCustomRootHandler(h)(&c)
Expand Down
64 changes: 34 additions & 30 deletions rest_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,44 +232,48 @@ func (a *restServer) stop() context.Context {

func (a *restServer) makeHandler(handler handlerFunc) http.HandlerFunc {

return gziphandler.GzipHandler(
http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
h := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {

var measure FinishMeasurementFunc
if a.cfg.healthServer.metricsManager != nil {
measure = a.cfg.healthServer.metricsManager.MeasureRequest(req.Method, req.URL.Path)
var measure FinishMeasurementFunc
if a.cfg.healthServer.metricsManager != nil {
measure = a.cfg.healthServer.metricsManager.MeasureRequest(req.Method, req.URL.Path)
}

request, err := elemental.NewRequestFromHTTPRequest(req, a.cfg.model.modelManagers[0])
if err != nil {
code := writeHTTPResponse(w, makeErrorResponse(req.Context(), elemental.NewResponse(elemental.NewRequest()), err))
if measure != nil {
measure(code, nil)
}
return
}

request, err := elemental.NewRequestFromHTTPRequest(req, a.cfg.model.modelManagers[0])
if err != nil {
code := writeHTTPResponse(w, makeErrorResponse(req.Context(), elemental.NewResponse(elemental.NewRequest()), err))
setCommonHeader(w, req.Header.Get("Origin"), request.Accept)

ctx := traceRequest(req.Context(), request, a.cfg.opentracing.tracer, a.cfg.opentracing.excludedIdentities, a.cfg.opentracing.traceCleaner)
defer finishTracing(ctx)

if a.cfg.rateLimiting.rateLimiter != nil {
rctx, cancel := context.WithTimeout(req.Context(), 1*time.Second)
defer cancel()
if err = a.cfg.rateLimiting.rateLimiter.Wait(rctx); err != nil {
code := writeHTTPResponse(w, makeErrorResponse(ctx, elemental.NewResponse(request), ErrRateLimit))
if measure != nil {
measure(code, nil)
measure(code, opentracing.SpanFromContext(ctx))
}
return
}
}

setCommonHeader(w, req.Header.Get("Origin"), request.Accept)

ctx := traceRequest(req.Context(), request, a.cfg.opentracing.tracer, a.cfg.opentracing.excludedIdentities, a.cfg.opentracing.traceCleaner)
defer finishTracing(ctx)
code := writeHTTPResponse(w, handler(newContext(ctx, request), a.cfg, a.processorFinder, a.pusher))
if measure != nil {
measure(code, opentracing.SpanFromContext(ctx))
}
})

if a.cfg.rateLimiting.rateLimiter != nil {
rctx, cancel := context.WithTimeout(req.Context(), 1*time.Second)
defer cancel()
if err = a.cfg.rateLimiting.rateLimiter.Wait(rctx); err != nil {
code := writeHTTPResponse(w, makeErrorResponse(ctx, elemental.NewResponse(request), ErrRateLimit))
if measure != nil {
measure(code, opentracing.SpanFromContext(ctx))
}
return
}
}
if a.cfg.restServer.disableCompression {
return h
}

code := writeHTTPResponse(w, handler(newContext(ctx, request), a.cfg, a.processorFinder, a.pusher))
if measure != nil {
measure(code, opentracing.SpanFromContext(ctx))
}
}),
).(http.HandlerFunc)
return gziphandler.GzipHandler(h).(http.HandlerFunc)
}
4 changes: 2 additions & 2 deletions rest_server_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func TestRestServerHelper_writeHTTPResponse(t *testing.T) {
w := httptest.NewRecorder()
r := elemental.NewResponse(elemental.NewRequest())

r.StatusCode = http.StatusCreated
r.StatusCode = http.StatusOK
r.Data = []byte("hello")

Convey("When I call writeHTTPResponse", func() {
Expand All @@ -194,7 +194,7 @@ func TestRestServerHelper_writeHTTPResponse(t *testing.T) {
})

Convey("Then the code should be http.StatusNoContent", func() {
So(code, ShouldEqual, http.StatusCreated)
So(code, ShouldEqual, http.StatusOK)
})
})
})
Expand Down

0 comments on commit 9f8df41

Please sign in to comment.