Skip to content

Draft: [SPIKE] Refactor routes to make middlewares and handlers more explicit

Kassio Borges requested to merge kassio/new-router into master

What does this MR do?

related to: #763 (closed)

The end goal is to make the application routing more explicit and intentional, separating middlewares from handlers (endpoints). With that some packages, like handlers and routing will be removed. Instead of having these packages based on application structure (what they are) I'd rather have packages by business logic (what they do). What I mean by that is, instead of handlers.AcmeMiddleware() we'd have acme.Middleware().

This MR is to test how we could apply the desired patterns, therefore it doesn't have the final desirable shape. The final should look something like:

// Creates a new router with the default middlewares
// The middlewares are executed in the given order
router := router.NewRouter(
  rejectmethods.Middleware()
  urilimiter.Middleware()
  correlation.Middleware()
  metricsMiddleware.Middleware()
  basicAccessLogger.Middleware()
  panicMiddleware.Middleware()
  customheaders.Middleware()
)

// Each specific endpoint can have its own handler
// that will be called after the default handlers
router.Handle("/-/healthcheck", healthcheck.Handler())

// Each specific endpoint might list extra middlewares
// The middlewares are executed before the given handler, in the given order
router.Handle(
  "/",
  pages.Handler(),
  rateLimiter.Middleware()
  HTTPSRedirect.Middleware()
)

return router

With that in mind, and the iterative approach, what I propose as next steps is:

  1. Introduce the router and keep handling everything the way we're, but passing through the router:
diff --git app.go app.go
index ac3071a9..ad16f136 100644
--- app.go
+++ app.go
@@ -181,7 +181,10 @@ func (a *theApp) buildHandlerPipeline() (http.Handler, error) {
 	handler = urilimiter.NewMiddleware(handler, a.config.General.MaxURILength)
 	handler = rejectmethods.NewMiddleware(handler)
 
-	return handler, nil
+	router := app_router.NewRouter()
+	router.Handle("/", handler)
+
+	return router, nil
 }
 
 // nolint: gocyclo // ignore this
  1. Open one MR to translate each, or a few, middlewares/handlers in each MR to the new pattern. This will be the period where we'd have some confusing names, but it's not too many routes, so I imagine in 1 or two releases we'd be able to translate everything. By translate I mean:
    1. Define if it's a handler or middleware. Where
      • a handler is related to a specific endpoint like healthcheck or acme. (aka: http.Handler)
      • a middleware is an intermediary manipulation of the request, not related to a specific endpoint. (aka type Middleware func(http.Handler) http.Handler)
    2. Define where the handler/middleware will live, based on what it does and not what it's. So, for instance:
      • handlers.RateLimit could become rate_limit.Middleware
      • routing.NewMiddleware could become domain.Middleware
    3. Handler and Middleware can work as the package entry point. Is where the package cycle life starts. IMHO this way of thinking might make the code more approachable/maintainable, as we now where the package work starts.

So, a possible, not exhaustive, list of MRs would be something like:

  1. Create the router
  2. Move all the global/required middlewares to the new format. (rejectmethods, urilimiter, correlationID, metrics, logging, panic, customHeaders)
  3. Translate the healtcheck handler
  4. Translate the acme handler and the middlewares it requires
  5. Translate the serve file handler and the rest of middlewares
Edited by Kassio Borges

Merge request reports