From 2098eed4a2169456009f65aa4dbeaeb03fd74b11 Mon Sep 17 00:00:00 2001 From: Manuel Odendahl Date: Wed, 8 May 2024 15:58:45 -0400 Subject: [PATCH] :sparkles: Fix the router middleware hacks --- pkg/handlers/template-dir/template-dir.go | 6 +- pkg/handlers/template/template.go | 3 +- pkg/render/renderer.go | 92 +++++++++++------------ pkg/server/server.go | 4 +- pkg/{render => utils}/errors.go | 2 +- pkg/utils/http.go | 30 ++++++++ 6 files changed, 80 insertions(+), 57 deletions(-) rename pkg/{render => utils}/errors.go (95%) diff --git a/pkg/handlers/template-dir/template-dir.go b/pkg/handlers/template-dir/template-dir.go index 4d6fd75..bc88bfe 100644 --- a/pkg/handlers/template-dir/template-dir.go +++ b/pkg/handlers/template-dir/template-dir.go @@ -128,11 +128,7 @@ func NewTemplateDirHandlerFromConfig(td *config.TemplateDir, options ...Template } func (td *TemplateDirHandler) Serve(server *server.Server, path string) error { - // TODO(manuel, 2023-05-26) This is a hack because we currently mix and match content with commands. - // The use of a middleware to handle something that could be handled by the routing framework itself - // is because gin (which really should get replaced because we actually go against its grain heavily) - // does not allow routes to overlap. - server.Router.Use(td.renderer.HandleWithTrimPrefixMiddleware(path, nil)) + server.Router.GET(path+"/:page", td.renderer.WithTrimPrefixHandler("", nil)) return nil } diff --git a/pkg/handlers/template/template.go b/pkg/handlers/template/template.go index d2ff96a..5cd0b86 100644 --- a/pkg/handlers/template/template.go +++ b/pkg/handlers/template/template.go @@ -86,8 +86,7 @@ func NewTemplateHandlerFromConfig( } func (t *TemplateHandler) Serve(server_ *server.Server, path string) error { - - server_.Router.Pre(t.renderer.HandleWithTrimPrefixMiddleware(path, nil)) + server_.Router.Pre(t.renderer.WithTrimPrefixMiddleware(path, "", nil)) return nil } diff --git a/pkg/render/renderer.go b/pkg/render/renderer.go index 0adac76..72f6822 100644 --- a/pkg/render/renderer.go +++ b/pkg/render/renderer.go @@ -1,6 +1,7 @@ package render import ( + "github.com/go-go-golems/parka/pkg/utils" "github.com/labstack/echo/v4" "github.com/pkg/errors" "github.com/rs/zerolog/log" @@ -227,7 +228,7 @@ func (r *Renderer) Render( return errors.Wrap(err, "error looking up template") } if t == nil { - return &NoPageFoundError{Page: page} + return &utils.NoPageFoundError{Page: page} } c.Response().WriteHeader(http.StatusOK) @@ -240,64 +241,61 @@ func (r *Renderer) Render( return nil } -func (r *Renderer) HandleWithTemplateMiddleware( - path string, - templateName string, - data map[string]interface{}, -) echo.MiddlewareFunc { - return func(next echo.HandlerFunc) echo.HandlerFunc { - return func(c echo.Context) error { - if c.Response().Committed { - return next(c) - } +func (r *Renderer) WithTemplateHandler(path string, templateName string, data map[string]interface{}) echo.HandlerFunc { + return func(c echo.Context) error { + if c.Response().Committed { + return nil + } - if c.Request().URL.Path == path { - err := r.Render(c, templateName, data) - if err != nil { - if _, ok := err.(*NoPageFoundError); ok { - return next(c) - } - return echo.NewHTTPError(http.StatusInternalServerError, err.Error()) + if c.Request().URL.Path == path { + err := r.Render(c, templateName, data) + if err != nil { + if _, ok := err.(*utils.NoPageFoundError); ok { + return c.NoContent(http.StatusNotFound) } - return nil + return echo.NewHTTPError(http.StatusInternalServerError, err.Error()) } - - return next(c) + return nil } + + return c.NoContent(http.StatusOK) } } -func (r *Renderer) HandleWithTrimPrefixMiddleware(prefix string, data map[string]interface{}) echo.MiddlewareFunc { +func (r *Renderer) WithTemplateMiddleware( + path string, + templateName string, + data map[string]interface{}, +) echo.MiddlewareFunc { + return utils.WithPathPrefixMiddleware(path, r.WithTemplateHandler(path, templateName, data)) +} + +func (r *Renderer) WithTrimPrefixHandler(prefix string, data map[string]interface{}) echo.HandlerFunc { prefix = strings.TrimPrefix(prefix, "/") - return func(next echo.HandlerFunc) echo.HandlerFunc { - return func(c echo.Context) error { - if c.Response().Committed { - return next(c) + return func(c echo.Context) error { + rawPath := c.Request().URL.Path + + if len(rawPath) > 0 && rawPath[0] == '/' { + trimmedPath := rawPath[1:] + trimmedPath = strings.TrimPrefix(trimmedPath, prefix) + if trimmedPath == "" || strings.HasSuffix(trimmedPath, "/") { + trimmedPath += "index" } - rawPath := c.Request().URL.Path - - if len(rawPath) > 0 && rawPath[0] == '/' { - trimmedPath := rawPath[1:] - trimmedPath = strings.TrimPrefix(trimmedPath, prefix) - if trimmedPath == "" || strings.HasSuffix(trimmedPath, "/") { - trimmedPath += "index" - } - - err := r.Render(c, trimmedPath, data) - if err != nil { - if _, ok := err.(*NoPageFoundError); ok { - return next(c) - } - return echo.NewHTTPError(http.StatusInternalServerError, err.Error()) - } - - return nil + err := r.Render(c, trimmedPath, data) + if err != nil { + return err } - // TODO(manuel, 2024-05-07) I'm not entirely sure this is the correct way of doing things - // this is if the rawPath is empty? I'm not sure I understand the logic here - return c.NoContent(http.StatusOK) + return nil } + + // TODO(manuel, 2024-05-07) I'm not entirely sure this is the correct way of doing things + // this is if the rawPath is empty? I'm not sure I understand the logic here + return c.NoContent(http.StatusOK) } } + +func (r *Renderer) WithTrimPrefixMiddleware(path string, prefix string, data map[string]interface{}) echo.MiddlewareFunc { + return utils.WithPathPrefixMiddleware(path, r.WithTrimPrefixHandler(prefix, data)) +} diff --git a/pkg/server/server.go b/pkg/server/server.go index 109345b..fd10305 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -213,8 +213,8 @@ func (s *Server) Run(ctx context.Context) error { // match all remaining paths to the templates if s.DefaultRenderer != nil { - s.Router.Pre(s.DefaultRenderer.HandleWithTemplateMiddleware("/", "index", nil)) - s.Router.Pre(s.DefaultRenderer.HandleWithTrimPrefixMiddleware("", nil)) + s.Router.GET("/", s.DefaultRenderer.WithTemplateHandler("/", "index", nil)) + s.Router.GET("/:path", s.DefaultRenderer.WithTemplateHandler("/", "index", nil)) } addr := fmt.Sprintf("%s:%d", s.Address, s.Port) diff --git a/pkg/render/errors.go b/pkg/utils/errors.go similarity index 95% rename from pkg/render/errors.go rename to pkg/utils/errors.go index 0b5a0cd..b752981 100644 --- a/pkg/render/errors.go +++ b/pkg/utils/errors.go @@ -1,4 +1,4 @@ -package render +package utils import "fmt" diff --git a/pkg/utils/http.go b/pkg/utils/http.go index cb054de..eddd5a3 100644 --- a/pkg/utils/http.go +++ b/pkg/utils/http.go @@ -1,3 +1,33 @@ package utils +import ( + "github.com/labstack/echo/v4" + "net/http" + "strings" +) + type H map[string]interface{} + +func WithPathPrefixMiddleware(path string, h echo.HandlerFunc) echo.MiddlewareFunc { + return func(next echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + if c.Response().Committed { + return next(c) + } + + if !strings.HasPrefix(c.Request().URL.Path, path) { + return next(c) + } + + err := h(c) + if err != nil { + if _, ok := err.(*NoPageFoundError); ok { + return next(c) + } + return echo.NewHTTPError(http.StatusInternalServerError, err.Error()) + } + + return nil + } + } +}