Fix golint errors

alphagov · Nov 4, 2024 · cc35993 · cc35993
1 parent 7ba0ed2
commit cc35993
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 40 deletions.
diff --git a/lib/load_routes.go b/lib/load_routes.go
@@ -39,11 +39,11 @@ func addHandler(mux *triemux.Mux, route *CsRoute, backends map[string]http.Handl
 	incomingURL, err := url.Parse(*route.IncomingPath)
 	if err != nil {
 		logWarn(fmt.Sprintf("router: found route %+v with invalid incoming path '%s', skipping!", route, *route.IncomingPath))
-		return nil
+		return nil //nolint:nilerr
 	}
 
 	switch route.handlerType() {
-	case "backend":
+	case HandlerTypeBackend:
 		backend := route.backend()
 		if backend == nil {
 			logWarn(fmt.Sprintf("router: found route %+v with nil backend_id, skipping!", *route.IncomingPath))
@@ -56,14 +56,14 @@ func addHandler(mux *triemux.Mux, route *CsRoute, backends map[string]http.Handl
 			return nil
 		}
 		mux.Handle(incomingURL.Path, prefix, handler)
-	case "redirect":
+	case HandlerTypeRedirect:
 		if route.RedirectTo == nil {
 			logWarn(fmt.Sprintf("router: found route %+v with nil redirect_to, skipping!", route))
 			return nil
 		}
 		handler := handlers.NewRedirectHandler(incomingURL.Path, *route.RedirectTo, shouldPreserveSegments(*route.RouteType, route.segmentsMode()))
 		mux.Handle(incomingURL.Path, prefix, handler)
-	case "gone":
+	case HandlerTypeGone:
 		mux.Handle(incomingURL.Path, prefix, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			http.Error(w, "410 Gone", http.StatusGone)
 		}))
@@ -170,8 +170,6 @@ func (rt *Router) reloadCsRoutes(pool PgxIface) {
 			errorMessage := fmt.Sprintf("panic: %v", r)
 			err := logger.RecoveredError{ErrorMessage: errorMessage}
 			logger.NotifySentry(logger.ReportableError{Error: err})
-		} else {
-			// rt.csMuxUpdatedAt = time.Now()
 		}
 		timer.ObserveDuration()
 	}()
@@ -180,7 +178,12 @@ func (rt *Router) reloadCsRoutes(pool PgxIface) {
 	newmux := triemux.NewMux()
 
 	backends := rt.loadBackendsFromEnv()
-	loadRoutesFromCS(pool, newmux, backends)
+	err := loadRoutesFromCS(pool, newmux, backends)
+	if err != nil {
+		logWarn(fmt.Sprintf("router: error reloading routes from content store: %v", err))
+		return
+	}
+
 	routeCount := newmux.RouteCount()
 
 	rt.lock.Lock()

diff --git a/lib/load_routes_test.go b/lib/load_routes_test.go
@@ -29,15 +29,15 @@ var _ = Describe("loadRoutesFromCS", func() {
 		backends = map[string]http.Handler{
 			"backend1": http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusOK)
-				w.Write([]byte("backend1"))
+				w.Write([]byte("backend1")) //nolint:errcheck
 			}),
 			"backend2": http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusOK)
-				w.Write([]byte("backend2"))
+				w.Write([]byte("backend2")) //nolint:errcheck
 			}),
 			"government-frontend": http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusOK)
-				w.Write([]byte("government-frontend"))
+				w.Write([]byte("government-frontend")) //nolint:errcheck
 			}),
 		}
 	})
@@ -54,11 +54,12 @@ var _ = Describe("loadRoutesFromCS", func() {
 
 			mockPool.ExpectQuery("WITH").WillReturnRows(rows)
 
-			loadRoutesFromCS(mockPool, mux, backends)
+			err := loadRoutesFromCS(mockPool, mux, backends)
+			Expect(err).NotTo(HaveOccurred())
 		})
 
 		It("should load backend exact routes correctly", func() {
-			req, _ := http.NewRequest("GET", "/path1", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/path1", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 
@@ -67,7 +68,7 @@ var _ = Describe("loadRoutesFromCS", func() {
 		})
 
 		It("should load backend prefix routes correctly", func() {
-			req, _ := http.NewRequest("GET", "/path2/foo/bar", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/path2/foo/bar", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 
@@ -88,35 +89,36 @@ var _ = Describe("loadRoutesFromCS", func() {
 
 			mockPool.ExpectQuery("WITH").WillReturnRows(rows)
 
-			loadRoutesFromCS(mockPool, mux, backends)
+			err := loadRoutesFromCS(mockPool, mux, backends)
+			Expect(err).NotTo(HaveOccurred())
 		})
 
 		It("should load gone route with description with empty fields", func() {
-			req, _ := http.NewRequest("GET", "/gone-empty-attributes", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/gone-empty-attributes", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 
 			Expect(rr.Code).To(Equal(http.StatusGone))
 		})
 
 		It("should load gone route with empty description", func() {
-			req, _ := http.NewRequest("GET", "/gone-empty-details", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/gone-empty-details", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 
 			Expect(rr.Code).To(Equal(http.StatusGone))
 		})
 
 		It("should load gone route with nil description", func() {
-			req, _ := http.NewRequest("GET", "/gone-nil-details", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/gone-nil-details", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 
 			Expect(rr.Code).To(Equal(http.StatusGone))
 		})
 
 		It("should load backend route with description and backend", func() {
-			req, _ := http.NewRequest("GET", "/backend-gone", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/backend-gone", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 
@@ -125,7 +127,7 @@ var _ = Describe("loadRoutesFromCS", func() {
 		})
 
 		It("should load government-frontend backend route with description and without backend", func() {
-			req, _ := http.NewRequest("GET", "/government-frontend-gone", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/government-frontend-gone", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 
@@ -145,11 +147,12 @@ var _ = Describe("loadRoutesFromCS", func() {
 				AddRow(nil, stringPtr("/redirect-prefix-preserve"), stringPtr("prefix"), stringPtr("/redirected-prefix-preserve"), stringPtr("preserve"), stringPtr("redirect"), nil)
 			mockPool.ExpectQuery("WITH").WillReturnRows(rows)
 
-			loadRoutesFromCS(mockPool, mux, backends)
+			err := loadRoutesFromCS(mockPool, mux, backends)
+			Expect(err).NotTo(HaveOccurred())
 		})
 
 		It("should load exact redirect route", func() {
-			req, _ := http.NewRequest("GET", "/redirect-exact", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/redirect-exact", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 
@@ -158,7 +161,7 @@ var _ = Describe("loadRoutesFromCS", func() {
 		})
 
 		It("should load prefix redirect route", func() {
-			req, _ := http.NewRequest("GET", "/redirect-prefix/foo/bar", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/redirect-prefix/foo/bar", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 
@@ -167,7 +170,7 @@ var _ = Describe("loadRoutesFromCS", func() {
 		})
 
 		It("should load exact redirect route that ignores suffix segments", func() {
-			req, _ := http.NewRequest("GET", "/redirect-exact-ignore", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/redirect-exact-ignore", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 
@@ -176,7 +179,7 @@ var _ = Describe("loadRoutesFromCS", func() {
 		})
 
 		It("should load prefix redirect route that ignores suffix segments", func() {
-			req, _ := http.NewRequest("GET", "/redirect-prefix-ignore/foo/bar", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/redirect-prefix-ignore/foo/bar", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 
@@ -185,7 +188,7 @@ var _ = Describe("loadRoutesFromCS", func() {
 		})
 
 		It("should load exact redirect route that preserves suffix segments", func() {
-			req, _ := http.NewRequest("GET", "/redirect-exact-preserve", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/redirect-exact-preserve", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 
@@ -194,7 +197,7 @@ var _ = Describe("loadRoutesFromCS", func() {
 		})
 
 		It("should load prefix redirect route that preserves suffix segments", func() {
-			req, _ := http.NewRequest("GET", "/redirect-prefix-preserve/foo/bar", nil)
+			req, _ := http.NewRequest(http.MethodGet, "/redirect-prefix-preserve/foo/bar", nil)
 			rr := httptest.NewRecorder()
 			mux.ServeHTTP(rr, req)
 

diff --git a/lib/router.go b/lib/router.go
@@ -153,7 +153,13 @@ func NewRouter(o Options) (rt *Router, err error) {
 
 	if csMuxSampleRate != 0.0 {
 		rt.reloadCsRoutes(pool)
-		go rt.listenForContentStoreUpdates(context.Background())
+
+		go func() {
+			if err := rt.listenForContentStoreUpdates(context.Background()); err != nil {
+				logWarn(fmt.Sprintf("router: error in listenForContentStoreUpdates: %v", err))
+			}
+		}()
+
 		go rt.waitForReload()
 	}
 
@@ -184,7 +190,7 @@ func (rt *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		}
 	}()
 
-	useContentStoreMux := rt.csMuxSampleRate > 0 && rand.Float64() < rt.csMuxSampleRate
+	useContentStoreMux := rt.csMuxSampleRate > 0 && rand.Float64() < rt.csMuxSampleRate //nolint:gosec
 	var mux *triemux.Mux
 
 	rt.lock.RLock()
@@ -358,7 +364,7 @@ func loadRoutes(c *mgo.Collection, mux *triemux.Mux, backends map[string]http.Ha
 		}
 
 		switch route.Handler {
-		case "backend":
+		case HandlerTypeBackend:
 			handler, ok := backends[route.BackendID]
 			if !ok {
 				logWarn(fmt.Sprintf("router: found route %+v which references unknown backend "+
@@ -368,12 +374,12 @@ func loadRoutes(c *mgo.Collection, mux *triemux.Mux, backends map[string]http.Ha
 			mux.Handle(incomingURL.Path, prefix, handler)
 			logDebug(fmt.Sprintf("router: registered %s (prefix: %v) for %s",
 				incomingURL.Path, prefix, route.BackendID))
-		case "redirect":
+		case HandlerTypeRedirect:
 			handler := handlers.NewRedirectHandler(incomingURL.Path, route.RedirectTo, shouldPreserveSegments(route.RouteType, route.SegmentsMode))
 			mux.Handle(incomingURL.Path, prefix, handler)
 			logDebug(fmt.Sprintf("router: registered %s (prefix: %v) -> %s",
 				incomingURL.Path, prefix, route.RedirectTo))
-		case "gone":
+		case HandlerTypeGone:
 			mux.Handle(incomingURL.Path, prefix, goneHandler)
 			logDebug(fmt.Sprintf("router: registered %s (prefix: %v) -> Gone", incomingURL.Path, prefix))
 		case "boom":

diff --git a/lib/routes.go b/lib/routes.go
@@ -4,6 +4,12 @@ import (
 	"encoding/json"
 )
 
+const (
+	HandlerTypeBackend  = "backend"
+	HandlerTypeRedirect = "redirect"
+	HandlerTypeGone     = "gone"
+)
+
 type CsRoute struct {
 	IncomingPath *string
 	RouteType    *string
@@ -27,12 +33,13 @@ func (route *CsRoute) backend() *string {
 }
 
 func (route *CsRoute) handlerType() string {
-	if route.redirect() {
-		return "redirect"
-	} else if route.gone() {
-		return "gone"
-	} else {
-		return "backend"
+	switch {
+	case route.redirect():
+		return HandlerTypeRedirect
+	case route.gone():
+		return HandlerTypeGone
+	default:
+		return HandlerTypeBackend
 	}
 }
 

diff --git a/lib/routes_test.go b/lib/routes_test.go
@@ -42,20 +42,20 @@ var _ = Describe("CsRoute", func() {
 		Context("when route is a redirect", func() {
 			It("should return 'redirect'", func() {
 				route.SchemaName = stringPtr("redirect")
-				Expect(route.handlerType()).To(Equal("redirect"))
+				Expect(route.handlerType()).To(Equal(HandlerTypeRedirect))
 			})
 		})
 
 		Context("when route is gone", func() {
 			It("should return 'gone'", func() {
 				route.SchemaName = stringPtr("gone")
-				Expect(route.handlerType()).To(Equal("gone"))
+				Expect(route.handlerType()).To(Equal(HandlerTypeGone))
 			})
 		})
 
 		Context("when route is neither redirect nor gone", func() {
 			It("should return 'backend'", func() {
-				Expect(route.handlerType()).To(Equal("backend"))
+				Expect(route.handlerType()).To(Equal(HandlerTypeBackend))
 			})
 		})
 	})