From 2bd126d3ed8c5a6aa2bcbba1c1935b088171a0b0 Mon Sep 17 00:00:00 2001 From: Sean Rankine Date: Wed, 4 Dec 2024 15:28:43 +0000 Subject: [PATCH] Fixup style and missing test This are stylistic changes and missing test for invalid json in the details column. --- cs_integration_tests/http_request_helpers.go | 6 +++--- cs_integration_tests/integration_test.go | 9 +++------ cs_integration_tests/metrics_test.go | 10 ++-------- lib/load_routes.go | 10 +--------- lib/routes.go | 11 ++++++++++- lib/routes_test.go | 9 +++++++++ 6 files changed, 28 insertions(+), 27 deletions(-) diff --git a/cs_integration_tests/http_request_helpers.go b/cs_integration_tests/http_request_helpers.go index 9822f2fa..054ea6c8 100644 --- a/cs_integration_tests/http_request_helpers.go +++ b/cs_integration_tests/http_request_helpers.go @@ -16,11 +16,11 @@ import ( ) func routerRequest(port int, path string) *http.Response { - return doRequest(newRequest("GET", routerURL(port, path))) + return doRequest(newRequest(http.MethodGet, routerURL(port, path))) } func routerRequestWithHeaders(port int, path string, headers map[string]string) *http.Response { - return doRequest(newRequestWithHeaders("GET", routerURL(port, path), headers)) + return doRequest(newRequestWithHeaders(http.MethodGet, routerURL(port, path), headers)) } func newRequest(method, url string) *http.Request { @@ -55,7 +55,7 @@ func doHTTP10Request(req *http.Request) *http.Response { defer conn.Close() if req.Method == "" { - req.Method = "GET" + req.Method = http.MethodGet } req.Proto = "HTTP/1.0" req.ProtoMinor = 0 diff --git a/cs_integration_tests/integration_test.go b/cs_integration_tests/integration_test.go index 984cb4b3..7f44e476 100644 --- a/cs_integration_tests/integration_test.go +++ b/cs_integration_tests/integration_test.go @@ -18,8 +18,7 @@ func TestEverything(t *testing.T) { var _ = BeforeSuite(func() { runtime.GOMAXPROCS(runtime.NumCPU()) - var err error - err = setupTempLogfile() + err := setupTempLogfile() if err != nil { Fail(err.Error()) } @@ -38,12 +37,10 @@ var _ = BeforeSuite(func() { backendEnvVars = append(backendEnvVars, envVar) } - err = startRouter(routerPort, apiPort, backendEnvVars) - if err != nil { + if err := startRouter(routerPort, apiPort, backendEnvVars); err != nil { Fail(err.Error()) } - err = initRouteHelper() - if err != nil { + if err := initRouteHelper(); err != nil { Fail(err.Error()) } }) diff --git a/cs_integration_tests/metrics_test.go b/cs_integration_tests/metrics_test.go index ba4812e9..620c18a8 100644 --- a/cs_integration_tests/metrics_test.go +++ b/cs_integration_tests/metrics_test.go @@ -7,16 +7,10 @@ import ( var _ = Describe("/metrics API endpoint", func() { Context("response body", func() { - var responseBody string - - BeforeEach(func() { + It("should contain at least one metric", func() { resp := doRequest(newRequest("GET", routerURL(apiPort, "/metrics"))) Expect(resp.StatusCode).To(Equal(200)) - responseBody = readBody(resp) - }) - - It("should contain at least one metric", func() { - Expect(responseBody).To(ContainSubstring("router_")) + Expect(readBody(resp)).To(ContainSubstring("router_")) }) }) }) diff --git a/lib/load_routes.go b/lib/load_routes.go index 5e5e83ad..18f04dd0 100644 --- a/lib/load_routes.go +++ b/lib/load_routes.go @@ -163,15 +163,7 @@ func (rt *Router) PeriodicCSRouteUpdates() { func (rt *Router) waitForReload() { for range rt.CsReloadChan { - func() { - defer func() { - if r := recover(); r != nil { - logWarn(r) - } - }() - - rt.reloadCsRoutes(rt.pool) - }() + rt.reloadCsRoutes(rt.pool) } } diff --git a/lib/routes.go b/lib/routes.go index ff6d826e..f7f8fe13 100644 --- a/lib/routes.go +++ b/lib/routes.go @@ -20,6 +20,9 @@ type CsRoute struct { Details *string } +// returns the backend which should be used for this route +// if the route is a gone route, but has an explaination in the details field, +// then route to the backend, or by default to government-frontend func (route *CsRoute) backend() *string { if route.SchemaName != nil && *route.SchemaName == "gone" && !route.gone() { if route.BackendID != nil { @@ -46,18 +49,24 @@ func (route *CsRoute) handlerType() string { func (route *CsRoute) gone() bool { if route.SchemaName != nil && *route.SchemaName == "gone" { if route.Details == nil { + // if the details field is empty, use a standard gone route return true } + // deserialise the details field, which should be JSON var detailsMap map[string]interface{} if err := json.Unmarshal([]byte(*route.Details), &detailsMap); err != nil { - return false + // if the details field is not valid JSON, use a standard gone route + return true } + // check if keys in the details map are not empty for _, value := range detailsMap { if value != nil && value != "" { + // not a standard gone route return false } } + // if the details field is empty, use a standard gone route return true } return false diff --git a/lib/routes_test.go b/lib/routes_test.go index 471c709f..9bf48adf 100644 --- a/lib/routes_test.go +++ b/lib/routes_test.go @@ -86,6 +86,15 @@ var _ = Describe("CsRoute", func() { }) }) + Context("when schema is 'gone' and details is invalid json", func() { + It("should return true", func() { + route.SchemaName = stringPtr("gone") + details := "{invalid}" + route.Details = &details + Expect(route.gone()).To(BeTrue()) + }) + }) + Context("when schema is not 'gone'", func() { It("should return false", func() { Expect(route.gone()).To(BeFalse())