From f17039006681d969066849e4bf51425932785308 Mon Sep 17 00:00:00 2001 From: JP Robinson Date: Mon, 29 Jul 2019 14:47:22 -0400 Subject: [PATCH] =?UTF-8?q?[server]=20fixing=20stdlib=20router=20?= =?UTF-8?q?=F0=9F=98=85=20(#222)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * adding tests to fix stdlib router in server bc we shouldve done it from the start * better test error msgs --- server/router_test.go | 26 ++++++++++- server/server.go | 6 ++- server/simple_server_test.go | 90 +++--------------------------------- 3 files changed, 35 insertions(+), 87 deletions(-) diff --git a/server/router_test.go b/server/router_test.go index 43af3784e..6528d71dc 100644 --- a/server/router_test.go +++ b/server/router_test.go @@ -10,7 +10,7 @@ func TestGorillaRoute(t *testing.T) { cfg := &Config{HealthCheckType: "simple", HealthCheckPath: "/status"} srvr := NewSimpleServer(cfg) RegisterHealthHandler(cfg, srvr.monitor, srvr.mux) - srvr.Register(&benchmarkSimpleService{false}) + srvr.Register(&benchmarkSimpleService{}) w := httptest.NewRecorder() r, _ := http.NewRequest("GET", "/svc/v1/1/blah/:something", nil) @@ -24,6 +24,28 @@ func TestGorillaRoute(t *testing.T) { wantBody := "blah" if gotBody := w.Body.String(); gotBody != wantBody { - t.Errorf("Fast route expected response body to be %q, got %q", wantBody, gotBody) + t.Errorf("Gorilla route expected response body to be %q, got %q", wantBody, gotBody) + } +} + +func TestStdlibRoute(t *testing.T) { + cfg := &Config{RouterType: "stdlib"} + srvr := NewSimpleServer(cfg) + RegisterHealthHandler(cfg, srvr.monitor, srvr.mux) + srvr.Register(&benchmarkSimpleService{}) + + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/svc/v1/2", nil) + r.RemoteAddr = "0.0.0.0:8080" + + srvr.ServeHTTP(w, r) + + if w.Code != http.StatusOK { + t.Errorf("stdlib route expected 200 response code, got %d", w.Code) + } + + wantBody := "ok" + if gotBody := w.Body.String(); gotBody != wantBody { + t.Errorf("stdlib route expected response body to be %q, got %q", wantBody, gotBody) } } diff --git a/server/server.go b/server/server.go index f687f3483..ace7f43ae 100644 --- a/server/server.go +++ b/server/server.go @@ -223,7 +223,11 @@ func RegisterHealthHandler(cfg *Config, monitor *ActivityMonitor, mx Router) Hea Log.Fatal("unable to start the HealthCheckHandler: ", err) } mx.Handle("GET", hch.Path(), hch) - mx.Handle("HEAD", hch.Path(), hch) + // the stdlib's http.ServeMux will panic if the same route is registered twice. + // if we see that router type, we shouldnt use it. + if _, isStdlib := mx.(*stdlibRouter); !isStdlib { + mx.Handle("HEAD", hch.Path(), hch) + } return hch } diff --git a/server/simple_server_test.go b/server/simple_server_test.go index 9f3ec559f..30c9b9f35 100644 --- a/server/simple_server_test.go +++ b/server/simple_server_test.go @@ -12,7 +12,6 @@ import ( ) type benchmarkContextService struct { - fast bool } func (s *benchmarkContextService) Prefix() string { @@ -47,36 +46,6 @@ func (s *benchmarkContextService) GetSimpleNoParam(ctx context.Context, w http.R fmt.Fprint(w, "ok") } -func BenchmarkFastSimpleServer_NoParam(b *testing.B) { - cfg := &Config{RouterType: "fast", HealthCheckType: "simple", HealthCheckPath: "/status"} - srvr := NewSimpleServer(cfg) - RegisterHealthHandler(cfg, srvr.monitor, srvr.mux) - srvr.Register(&benchmarkSimpleService{true}) - - w := httptest.NewRecorder() - r, _ := http.NewRequest("GET", "/svc/v1/2", nil) - r.RemoteAddr = "0.0.0.0:8080" - - for i := 0; i < b.N; i++ { - srvr.ServeHTTP(w, r) - } -} - -func BenchmarkFastSimpleServer_WithParam(b *testing.B) { - cfg := &Config{RouterType: "fast", HealthCheckType: "simple", HealthCheckPath: "/status"} - srvr := NewSimpleServer(cfg) - RegisterHealthHandler(cfg, srvr.monitor, srvr.mux) - srvr.Register(&benchmarkSimpleService{true}) - - w := httptest.NewRecorder() - r, _ := http.NewRequest("GET", "/svc/v1/1/{something}/blah", nil) - r.RemoteAddr = "0.0.0.0:8080" - - for i := 0; i < b.N; i++ { - srvr.ServeHTTP(w, r) - } -} - func BenchmarkSimpleServer_NoParam(b *testing.B) { cfg := &Config{HealthCheckType: "simple", HealthCheckPath: "/status"} srvr := NewSimpleServer(cfg) @@ -108,7 +77,6 @@ func BenchmarkSimpleServer_WithParam(b *testing.B) { } type benchmarkSimpleService struct { - fast bool } func (s *benchmarkSimpleService) Prefix() string { @@ -139,49 +107,6 @@ func (s *benchmarkSimpleService) GetSimpleNoParam(w http.ResponseWriter, r *http fmt.Fprint(w, "ok") } -func BenchmarkFastJSONServer_JSONPayload(b *testing.B) { - cfg := &Config{RouterType: "fast", HealthCheckType: "simple", HealthCheckPath: "/status"} - srvr := NewSimpleServer(cfg) - RegisterHealthHandler(cfg, srvr.monitor, srvr.mux) - srvr.Register(&benchmarkJSONService{true}) - - w := httptest.NewRecorder() - r, _ := http.NewRequest("PUT", "/svc/v1/1", bytes.NewBufferString(`{"hello":"hi","howdy":"yo"}`)) - r.RemoteAddr = "0.0.0.0:8080" - - for i := 0; i < b.N; i++ { - srvr.ServeHTTP(w, r) - } -} -func BenchmarkFastJSONServer_NoParam(b *testing.B) { - cfg := &Config{RouterType: "fast", HealthCheckType: "simple", HealthCheckPath: "/status"} - srvr := NewSimpleServer(cfg) - RegisterHealthHandler(cfg, srvr.monitor, srvr.mux) - srvr.Register(&benchmarkJSONService{true}) - - w := httptest.NewRecorder() - r, _ := http.NewRequest("PUT", "/svc/v1/2", nil) - r.RemoteAddr = "0.0.0.0:8080" - - for i := 0; i < b.N; i++ { - srvr.ServeHTTP(w, r) - } -} -func BenchmarkFastJSONServer_WithParam(b *testing.B) { - cfg := &Config{RouterType: "fast", HealthCheckType: "simple", HealthCheckPath: "/status"} - srvr := NewSimpleServer(cfg) - RegisterHealthHandler(cfg, srvr.monitor, srvr.mux) - srvr.Register(&benchmarkJSONService{true}) - - w := httptest.NewRecorder() - r, _ := http.NewRequest("PUT", "/svc/v1/3/{something}/blah", bytes.NewBufferString(`{"hello":"hi","howdy":"yo"}`)) - r.RemoteAddr = "0.0.0.0:8080" - - for i := 0; i < b.N; i++ { - srvr.ServeHTTP(w, r) - } -} - func BenchmarkJSONServer_JSONPayload(b *testing.B) { cfg := &Config{HealthCheckType: "simple", HealthCheckPath: "/status"} srvr := NewSimpleServer(cfg) @@ -227,7 +152,6 @@ func BenchmarkJSONServer_WithParam(b *testing.B) { } type benchmarkJSONService struct { - fast bool } func (s *benchmarkJSONService) Prefix() string { @@ -275,10 +199,10 @@ func (s *benchmarkJSONService) GetJSONParam(r *http.Request) (int, interface{}, } func BenchmarkFastContextSimpleServer_NoParam(b *testing.B) { - cfg := &Config{RouterType: "fast", HealthCheckType: "simple", HealthCheckPath: "/status"} + cfg := &Config{RouterType: "stdlib", HealthCheckType: "simple", HealthCheckPath: "/status"} srvr := NewSimpleServer(cfg) RegisterHealthHandler(cfg, srvr.monitor, srvr.mux) - srvr.Register(&benchmarkContextService{true}) + srvr.Register(&benchmarkContextService{}) w := httptest.NewRecorder() r, _ := http.NewRequest("GET", "/svc/v1/ctx/2", nil) @@ -290,10 +214,10 @@ func BenchmarkFastContextSimpleServer_NoParam(b *testing.B) { } func BenchmarkFastContextSimpleServer_WithParam(b *testing.B) { - cfg := &Config{RouterType: "fast", HealthCheckType: "simple", HealthCheckPath: "/status"} + cfg := &Config{RouterType: "stdlib", HealthCheckType: "simple", HealthCheckPath: "/status"} srvr := NewSimpleServer(cfg) RegisterHealthHandler(cfg, srvr.monitor, srvr.mux) - srvr.Register(&benchmarkContextService{true}) + srvr.Register(&benchmarkContextService{}) w := httptest.NewRecorder() r, _ := http.NewRequest("GET", "/svc/v1/ctx/1/{something}/blah", nil) @@ -340,7 +264,6 @@ type testJSON struct { } type testMixedService struct { - fast bool } func (s *testMixedService) Prefix() string { @@ -381,7 +304,6 @@ func (s *testMixedService) Middleware(h http.Handler) http.Handler { } type testInvalidService struct { - fast bool } func (s *testInvalidService) Prefix() string { @@ -438,7 +360,7 @@ func TestSimpleServerCORSMiddleware(t *testing.T) { cfg := &Config{HealthCheckType: "simple", HealthCheckPath: "/status"} srvr := NewSimpleServer(cfg) RegisterHealthHandler(cfg, srvr.monitor, srvr.mux) - srvr.Register(&benchmarkSimpleService{false}) + srvr.Register(&benchmarkSimpleService{}) wt := httptest.NewRecorder() // hit the CORS middlware @@ -472,7 +394,7 @@ func TestNotFoundHandler(t *testing.T) { cfg := &Config{HealthCheckType: "simple", HealthCheckPath: "/status", NotFoundHandler: http.HandlerFunc(http.NotFound)} srvr := NewSimpleServer(cfg) RegisterHealthHandler(cfg, srvr.monitor, srvr.mux) - srvr.Register(&benchmarkSimpleService{false}) + srvr.Register(&benchmarkSimpleService{}) wt := httptest.NewRecorder() r := httptest.NewRequest(http.MethodGet, "/svc/v1/1/blah", nil)