From c02c8b2567ec543d8eb79d81e4177df986136239 Mon Sep 17 00:00:00 2001 From: Bastian Ike Date: Thu, 19 Jan 2023 14:36:10 +0100 Subject: [PATCH] refactor: fix linter #289 --- .golangci.yml | 1 - app.go | 59 ++++++++-------- core/auth/debug.go | 2 +- core/auth/identity.go | 2 +- core/cache/cache.go | 2 +- core/cache/httpFrontend.go | 2 +- core/locale/application/date_time_service.go | 2 +- .../infrastructure/translation_service.go | 10 +-- .../middleware/securityMiddleware.go | 2 +- framework/cmd/module.go | 2 +- framework/config/config_test.go | 2 +- framework/config/configcmd.go | 6 +- framework/web/context.go | 2 + framework/web/filter.go | 9 +-- framework/web/handler.go | 68 +++++++++++-------- framework/web/path.go | 2 +- framework/web/registry.go | 43 ++++++------ framework/web/request.go | 10 ++- framework/web/result.go | 46 +++++++------ framework/web/router.go | 13 ++-- framework/web/session.go | 1 + framework/web/sessionstore.go | 12 ++-- framework/web/sessionstore_test.go | 4 +- framework/web/templatefunctions.go | 21 ++++-- framework/web/utils.go | 10 +-- framework/web/utils_test.go | 1 + 26 files changed, 187 insertions(+), 147 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 20d69af89..3b49afb25 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -83,7 +83,6 @@ linters: issues: new: false fix: false - new-from-rev: 2399c75fbd6c738c4cddf38a3ad7f5f97367e5ec exclude-rules: - path: _test\.go linters: diff --git a/app.go b/app.go index a0f6a636e..990ce7167 100644 --- a/app.go +++ b/app.go @@ -2,6 +2,7 @@ package flamingo import ( "context" + "errors" "flag" "fmt" "log" @@ -109,12 +110,13 @@ func NewApplication(modules []dingo.Module, options ...ApplicationOption) (*Appl option(app) } + var flamingoConfig arrayFlags + app.flagset = flag.NewFlagSet("flamingo", flag.ContinueOnError) dingoTraceCircular := app.flagset.Bool("dingo-trace-circular", false, "enable dingo circular tracing") flamingoConfigLog := app.flagset.Bool("flamingo-config-log", false, "enable flamingo config logging") flamingoConfigCueDebug := app.flagset.String("flamingo-config-cue-debug", "", "query the flamingo cue config loader (use . for root)") flamingoContext := app.flagset.String("flamingo-context", app.defaultContext, "set flamingo execution context") - var flamingoConfig arrayFlags app.flagset.Var(&flamingoConfig, "flamingo-config", "add additional flamingo yaml config") dingoInspect := app.flagset.Bool("dingo-inspect", false, "inspect dingo") @@ -218,52 +220,54 @@ func (app *Application) Run() error { return fmt.Errorf("get initialized injector: %w", err) } - i, err := injector.GetAnnotatedInstance(new(cobra.Command), "flamingo") + instance, err := injector.GetAnnotatedInstance(new(cobra.Command), "flamingo") if err != nil { return fmt.Errorf("app: get flamingo cobra.Command: %w", err) } - rootCmd := i.(*cobra.Command) + rootCmd := instance.(*cobra.Command) rootCmd.SetArgs(app.flagset.Args()) - i, err = injector.GetInstance(new(eventRouterProvider)) + instance, err = injector.GetInstance(new(eventRouterProvider)) if err != nil { return fmt.Errorf("app: get eventRouterProvider: %w", err) } - i.(eventRouterProvider)().Dispatch(context.Background(), new(flamingo.StartupEvent)) + instance.(eventRouterProvider)().Dispatch(context.Background(), new(flamingo.StartupEvent)) return rootCmd.Execute() } -func typeName(of reflect.Type) string { +func typeName(target reflect.Type) string { var name string - for of.Kind() == reflect.Ptr { - of = of.Elem() + for target.Kind() == reflect.Ptr { + target = target.Elem() } - if of.Kind() == reflect.Slice { + if target.Kind() == reflect.Slice { name += "[]" - of = of.Elem() + target = target.Elem() } - if of.Kind() == reflect.Ptr { + if target.Kind() == reflect.Ptr { name += "*" - of = of.Elem() + target = target.Elem() } - if of.PkgPath() != "" { - name += of.PkgPath() + "." + if target.PkgPath() != "" { + name += target.PkgPath() + "." } - name += of.Name() + name += target.Name() return name } +const truncMax = 25 + func trunc(s string) string { - if len(s) > 25 { - return s[:25] + "..." + if len(s) > truncMax { + return s[:truncMax] + "..." } return s } @@ -297,7 +301,7 @@ func inspect(injector *dingo.Injector) { fmt.Println("\nMultiBindings:") injector.Inspect(dingo.Inspector{ InspectMultiBinding: func(of reflect.Type, index int, annotation string, to reflect.Type, provider, instance *reflect.Value, in dingo.Scope) { - //fmt.Printf("%d: ", index) + // fmt.Printf("%d: ", index) printBinding(of, annotation, to, provider, instance, in) }, }) @@ -305,7 +309,7 @@ func inspect(injector *dingo.Injector) { fmt.Println("\nMapBindings:") injector.Inspect(dingo.Inspector{ InspectMapBinding: func(of reflect.Type, key string, annotation string, to reflect.Type, provider, instance *reflect.Value, in dingo.Scope) { - //fmt.Printf("%s: ", key) + // fmt.Printf("%s: ", key) printBinding(of, annotation, to, provider, instance, in) }, }) @@ -341,7 +345,8 @@ func (a *servemodule) Inject( a.eventRouter = eventRouter a.logger = logger a.server = &http.Server{ - Addr: fmt.Sprintf(":%d", cfg.Port), + Addr: fmt.Sprintf(":%d", cfg.Port), + ReadHeaderTimeout: 10 * time.Second, } a.configuredSampler = configuredSampler a.publicEndpoint = cfg.PublicEndpoint @@ -361,16 +366,16 @@ func (a *servemodule) CueConfig() string { return `core: serve: port: >= 0 & <= 65535 | *3322` } -func serveProvider(a *servemodule, logger flamingo.Logger) *cobra.Command { +func serveProvider(serveModule *servemodule, logger flamingo.Logger) *cobra.Command { serveCmd := &cobra.Command{ Use: "serve", Short: "Default serve command - starts on Port 3322", Run: func(cmd *cobra.Command, args []string) { - a.server.Handler = &ochttp.Handler{IsPublicEndpoint: a.publicEndpoint, Handler: a.router.Handler(), GetStartOptions: a.configuredSampler.GetStartOptions()} + serveModule.server.Handler = &ochttp.Handler{IsPublicEndpoint: serveModule.publicEndpoint, Handler: serveModule.router.Handler(), GetStartOptions: serveModule.configuredSampler.GetStartOptions()} - err := a.listenAndServe() + err := serveModule.listenAndServe() if err != nil { - if err == http.ErrServerClosed { + if errors.Is(err, http.ErrServerClosed) { logger.Info(err) } else { logger.Fatal("unexpected error in serving:", err) @@ -378,9 +383,9 @@ func serveProvider(a *servemodule, logger flamingo.Logger) *cobra.Command { } }, } - serveCmd.Flags().StringVarP(&a.server.Addr, "addr", "a", a.server.Addr, "addr on which flamingo runs") - serveCmd.Flags().StringVarP(&a.certFile, "certFile", "c", "", "certFile to enable HTTPS") - serveCmd.Flags().StringVarP(&a.keyFile, "keyFile", "k", "", "keyFile to enable HTTPS") + serveCmd.Flags().StringVarP(&serveModule.server.Addr, "addr", "a", serveModule.server.Addr, "addr on which flamingo runs") + serveCmd.Flags().StringVarP(&serveModule.certFile, "certFile", "c", "", "certFile to enable HTTPS") + serveCmd.Flags().StringVarP(&serveModule.keyFile, "keyFile", "k", "", "keyFile to enable HTTPS") return serveCmd } diff --git a/core/auth/debug.go b/core/auth/debug.go index eb358fb7a..669420f9c 100644 --- a/core/auth/debug.go +++ b/core/auth/debug.go @@ -24,7 +24,7 @@ func (c *debugController) Inject(responder *web.Responder, identityService *WebI } var tpl = template.Must(template.New("debug").Parse( - //language=gohtml + // language=gohtml `

Auth Debug

Registered RequestIdentifier:

diff --git a/core/auth/identity.go b/core/auth/identity.go index 521dc1b7a..2b17e5f72 100644 --- a/core/auth/identity.go +++ b/core/auth/identity.go @@ -34,7 +34,7 @@ func (p *securityRoleProvider) All(ctx context.Context, _ *web.Session) []domain for _, identity := range p.service.IdentifyAll(ctx, request) { _ = identity identified = true - //if roler, ok := identity.(hasRoles); ok { + // if roler, ok := identity.(hasRoles); ok { //roles = append(roles, roler.Roles()...) //} } diff --git a/core/cache/cache.go b/core/cache/cache.go index e6576926b..d656abe0e 100644 --- a/core/cache/cache.go +++ b/core/cache/cache.go @@ -24,7 +24,7 @@ type ( Backend interface { Get(key string) (entry *Entry, found bool) // Get a cache entry Set(key string, entry *Entry) error // Set a cache entry - //Peek(key string) (entry *CacheEntry, found bool) // Peek for a cache entry, this should not trigger key-updates or weight/priorities to be changed + // Peek(key string) (entry *CacheEntry, found bool) // Peek for a cache entry, this should not trigger key-updates or weight/priorities to be changed Purge(key string) error PurgeTags(tags []string) error Flush() error diff --git a/core/cache/httpFrontend.go b/core/cache/httpFrontend.go index 371900539..936ef792c 100644 --- a/core/cache/httpFrontend.go +++ b/core/cache/httpFrontend.go @@ -198,7 +198,7 @@ func (hf *HTTPFrontend) load(ctx context.Context, key string, loader HTTPLoader, span.AddAttributes(trace.StringAttribute("parenttrace", response.span.TraceID.String())) span.AddAttributes(trace.StringAttribute("parentspan", response.span.SpanID.String())) - //span.AddLink(trace.Link{ + // span.AddLink(trace.Link{ // SpanID: data.(loaderResponse).span.SpanID, // TraceID: data.(loaderResponse).span.TraceID, // Type: trace.LinkTypeChild, diff --git a/core/locale/application/date_time_service.go b/core/locale/application/date_time_service.go index a906ce6a2..fcbb2080f 100644 --- a/core/locale/application/date_time_service.go +++ b/core/locale/application/date_time_service.go @@ -50,7 +50,7 @@ func (dts *DateTimeService) Inject( // GetDateTimeFormatterFromIsoString Need string in format ISO: "2017-11-25T06:30:00Z" func (dts *DateTimeService) GetDateTimeFormatterFromIsoString(dateTimeString string) (*domain.DateTimeFormatter, error) { - timeResult, err := time.Parse(time.RFC3339, dateTimeString) //"2006-01-02T15:04:05Z" + timeResult, err := time.Parse(time.RFC3339, dateTimeString) // "2006-01-02T15:04:05Z" if err != nil { return nil, fmt.Errorf("could not parse date in defined format: %v: %w", dateTimeString, err) } diff --git a/core/locale/infrastructure/translation_service.go b/core/locale/infrastructure/translation_service.go index a519e6586..4446fd2b1 100644 --- a/core/locale/infrastructure/translation_service.go +++ b/core/locale/infrastructure/translation_service.go @@ -61,17 +61,17 @@ func (ts *TranslationService) TranslateLabel(label domain.Label) string { ts.reloadFilesIfNecessary() translatedString, err := ts.translateWithLib(label.GetLocaleCode(), label.GetKey(), label.GetCount(), label.GetTranslationArguments()) - //while there is an error check fallBacks + // while there is an error check fallBacks for _, fallbackLocale := range label.GetFallbackLocaleCodes() { if err != nil { translatedString, err = ts.translateWithLib(fallbackLocale, label.GetKey(), label.GetCount(), label.GetTranslationArguments()) } } if err != nil { - //default to key (=untranslated) if still an error + // default to key (=untranslated) if still an error translatedString = label.GetKey() } - //Fallback if label was not translated + // Fallback if label was not translated if translatedString == label.GetKey() && label.GetDefaultLabel() != "" { return ts.parseDefaultLabel(label.GetDefaultLabel(), label.GetKey(), label.GetTranslationArguments()) } @@ -84,11 +84,11 @@ func (ts *TranslationService) Translate(key string, defaultLabel string, localeC label, err := ts.translateWithLib(localeCode, key, count, translationArguments) if err != nil { - //default to key (=untranslated) on error + // default to key (=untranslated) on error label = key } - //Fallback if label was not translated + // Fallback if label was not translated if label == key && defaultLabel != "" { return ts.parseDefaultLabel(defaultLabel, key, translationArguments) } diff --git a/core/security/interface/middleware/securityMiddleware.go b/core/security/interface/middleware/securityMiddleware.go index 8741ea6dc..7a768dd93 100644 --- a/core/security/interface/middleware/securityMiddleware.go +++ b/core/security/interface/middleware/securityMiddleware.go @@ -16,7 +16,7 @@ import ( const ( // ReferrerRedirectStrategy strategy to redirect to the supplied referrer ReferrerRedirectStrategy = "referrer" - //PathRedirectStrategy strategy to redirect to the supplied path + // PathRedirectStrategy strategy to redirect to the supplied path PathRedirectStrategy = "path" ) diff --git a/framework/cmd/module.go b/framework/cmd/module.go index 1235c6bf6..f8ce4eca1 100644 --- a/framework/cmd/module.go +++ b/framework/cmd/module.go @@ -67,7 +67,7 @@ func (m *Module) Configure(injector *dingo.Injector) { }, Example: `Run with -h or -help to see global debug flags`, } - //rootCmd.SetHelpTemplate() + // rootCmd.SetHelpTemplate() rootCmd.FParseErrWhitelist.UnknownFlags = true for _, set := range flagSetProvider() { rootCmd.PersistentFlags().AddFlagSet(set) diff --git a/framework/config/config_test.go b/framework/config/config_test.go index d0056bcb7..697416fa2 100644 --- a/framework/config/config_test.go +++ b/framework/config/config_test.go @@ -130,7 +130,7 @@ func TestMapMapInto(t *testing.T) { } } - //fill the config map according to the resultType struct + // fill the config map according to the resultType struct m := make(Map) assert.NoError(t, m.Add(Map{ diff --git a/framework/config/configcmd.go b/framework/config/configcmd.go index 9f4d0228a..7f4842727 100644 --- a/framework/config/configcmd.go +++ b/framework/config/configcmd.go @@ -56,9 +56,9 @@ func dumpConfigArea(a *Area) { fmt.Println("**************************") fmt.Println("Area: ", a.Name) fmt.Println("**************************") - if false { //cuedump { + if false { // cuedump { // build a cue runtime to verify the config - //cueRuntime := new(cue.Runtime) + // cueRuntime := new(cue.Runtime) //ci, err := cueRuntime.Build(a.cueBuildInstance) //if err != nil { // panic(err) @@ -71,7 +71,7 @@ func dumpConfigArea(a *Area) { fmt.Println("") } - //d, _ := format.Node(ci.Value().Syntax(), format.Simplify()) + // d, _ := format.Node(ci.Value().Syntax(), format.Simplify()) //fmt.Println(string(d)) } else { x, _ := json.MarshalIndent(a.Configuration, "", " ") diff --git a/framework/web/context.go b/framework/web/context.go index 48917b664..47d69e472 100644 --- a/framework/web/context.go +++ b/framework/web/context.go @@ -13,6 +13,7 @@ func RunWithDetachedContext(origCtx context.Context, fnc func(ctx context.Contex request := RequestFromContext(origCtx) session := SessionFromContext(origCtx) + if request != nil && session == nil { session = request.Session() } @@ -20,5 +21,6 @@ func RunWithDetachedContext(origCtx context.Context, fnc func(ctx context.Contex ctx := ContextWithRequest(trace.NewContext(context.Background(), span), request) ctx = ContextWithSession(ctx, session) + //nolint:contextcheck // we want a new context here fnc(ctx) } diff --git a/framework/web/filter.go b/framework/web/filter.go index ec8cfd035..53cdd44e8 100644 --- a/framework/web/filter.go +++ b/framework/web/filter.go @@ -60,6 +60,7 @@ func (fc *FilterChain) Next(ctx context.Context, req *Request, w http.ResponseWr next := fc.filters[0] fc.filters = fc.filters[1:] + return next.Filter(ctx, req, w, fc) } @@ -74,18 +75,18 @@ func (sf sortableFilers) Len() int { } // Less supports implementation for sort.Interface -func (sf sortableFilers) Less(i, j int) bool { +func (sf sortableFilers) Less(indexLeft, indexRight int) bool { firstPriority := 0 - if filter, ok := sf[i].filter.(PrioritizedFilter); ok { + if filter, ok := sf[indexLeft].filter.(PrioritizedFilter); ok { firstPriority = filter.Priority() } secondPriority := 0 - if filter, ok := sf[j].filter.(PrioritizedFilter); ok { + if filter, ok := sf[indexRight].filter.(PrioritizedFilter); ok { secondPriority = filter.Priority() } - return firstPriority < secondPriority || (firstPriority == secondPriority && sf[i].index > sf[j].index) + return firstPriority < secondPriority || (firstPriority == secondPriority && sf[indexLeft].index > sf[indexRight].index) } // Swap supports implementation for sort.Interface diff --git a/framework/web/handler.go b/framework/web/handler.go index 99cdf48b8..5e6b48505 100644 --- a/framework/web/handler.go +++ b/framework/web/handler.go @@ -67,19 +67,21 @@ func (e *panicError) Unwrap() error { return e.err } -func (e *panicError) Format(s fmt.State, verb rune) { +func (e *panicError) Format(state fmt.State, verb rune) { switch verb { case 'v': - if s.Flag('+') { - _, _ = io.WriteString(s, e.err.Error()) - _, _ = fmt.Fprintf(s, "\n%s", e.stack) + if state.Flag('+') { + _, _ = io.WriteString(state, e.err.Error()) + _, _ = fmt.Fprintf(state, "\n%s", e.stack) + return } + fallthrough case 's': - _, _ = io.WriteString(s, e.err.Error()) + _, _ = io.WriteString(state, e.err.Error()) case 'q': - _, _ = fmt.Fprintf(s, "%q", e.err) + _, _ = fmt.Fprintf(state, "%q", e.err) } } @@ -91,7 +93,7 @@ func panicToError(p interface{}) error { var err error switch errIface := p.(type) { case error: - //err = fmt.Errorf("controller panic: %w", errIface) + // err = fmt.Errorf("controller panic: %w", errIface) err = &panicError{err: fmt.Errorf("controller panic: %w", errIface), stack: debug.Stack()} case string: err = &panicError{err: fmt.Errorf("controller panic: %s", errIface), stack: debug.Stack()} @@ -123,6 +125,7 @@ func (h *handler) ServeHTTP(rw http.ResponseWriter, httpRequest *http.Request) { ctx, _ = tag.New(ctx, tag.Upsert(ControllerKey, handler.GetHandlerName()), tag.Insert(opencensus.KeyArea, "-")) httpRequest = httpRequest.WithContext(ctx) start := time.Now() + defer func() { stats.Record(ctx, rt.M(time.Since(start).Nanoseconds()/1000000)) }() @@ -151,26 +154,26 @@ func (h *handler) ServeHTTP(rw http.ResponseWriter, httpRequest *http.Request) { chain := &FilterChain{ filters: h.filter, - final: func(ctx context.Context, r *Request, rw http.ResponseWriter) (response Result) { + final: func(ctx context.Context, r *Request, writer http.ResponseWriter) (response Result) { ctx, span := trace.StartSpan(ctx, "router/controller") defer span.End() defer func() { if err := panicToError(recover()); err != nil { - response = h.routerRegistry.handler[FlamingoError].any(context.WithValue(ctx, RouterError, err), r) + response = h.routerRegistry.handler[FlamingoError].anyAction(context.WithValue(ctx, RouterError, err), r) span.SetStatus(trace.Status{Code: trace.StatusCodeAborted, Message: "controller panic"}) } }() - defer h.eventRouter.Dispatch(ctx, &OnResponseEvent{OnRequestEvent{req, rw}, response}) + defer h.eventRouter.Dispatch(ctx, &OnResponseEvent{OnRequestEvent{req, writer}, response}) if c, ok := controller.method[req.Request().Method]; ok && c != nil { response = c(ctx, r) - } else if controller.any != nil { - response = controller.any(ctx, r) + } else if controller.anyAction != nil { + response = controller.anyAction(ctx, r) } else { err := fmt.Errorf("action for method %q not found and no \"any\" fallback", req.Request().Method) - response = h.routerRegistry.handler[FlamingoNotfound].any(context.WithValue(ctx, RouterError, err), r) + response = h.routerRegistry.handler[FlamingoNotfound].anyAction(context.WithValue(ctx, RouterError, err), r) span.SetStatus(trace.Status{Code: trace.StatusCodeNotFound, Message: "action not found"}) } @@ -187,16 +190,18 @@ func (h *handler) ServeHTTP(rw http.ResponseWriter, httpRequest *http.Request) { } var finalErr error + if result != nil { ctx, span := trace.StartSpan(ctx, "router/responseApply") func() { - //catch panic in Apply only + // catch panic in Apply only defer func() { if err := panicToError(recover()); err != nil { finalErr = err } }() + finalErr = result.Apply(ctx, rw) }() @@ -212,23 +217,28 @@ func (h *handler) ServeHTTP(rw http.ResponseWriter, httpRequest *http.Request) { cb(finalErr, result) } - if finalErr != nil { - finishErr = finalErr - defer func() { - if err := panicToError(recover()); err != nil { - if errors.Is(err, context.Canceled) { - h.logger.WithContext(ctx).Debug(err) - } else { - h.logger.WithContext(ctx).Error(err) - } - finishErr = err - rw.WriteHeader(http.StatusInternalServerError) - _, _ = fmt.Fprintf(rw, "%+v", err) + if finalErr == nil { + return + } + + finishErr = finalErr + + defer func() { + if err := panicToError(recover()); err != nil { + if errors.Is(err, context.Canceled) { + h.logger.WithContext(ctx).Debug(err) + } else { + h.logger.WithContext(ctx).Error(err) } - }() - if err := h.routerRegistry.handler[FlamingoError].any(context.WithValue(ctx, RouterError, finalErr), req).Apply(ctx, rw); err != nil { - panic(err) + finishErr = err + + rw.WriteHeader(http.StatusInternalServerError) + _, _ = fmt.Fprintf(rw, "%+v", err) } + }() + + if err := h.routerRegistry.handler[FlamingoError].anyAction(context.WithValue(ctx, RouterError, finalErr), req).Apply(ctx, rw); err != nil { + panic(err) } } diff --git a/framework/web/path.go b/framework/web/path.go index f2cc65d01..2fd2e0f1c 100644 --- a/framework/web/path.go +++ b/framework/web/path.go @@ -269,7 +269,7 @@ func (p *Path) Match(path string) *Match { matched, key, value, length := part.match(path) - //log.Printf("%#v == %v (%d) %s", part, matched, length, value) + // log.Printf("%#v == %v (%d) %s", part, matched, length, value) if !matched { return nil diff --git a/framework/web/registry.go b/framework/web/registry.go index a4b1f5109..6c6f724b4 100644 --- a/framework/web/registry.go +++ b/framework/web/registry.go @@ -33,9 +33,9 @@ type ( } handlerAction struct { - method map[string]Action - any Action - data DataAction + method map[string]Action + anyAction Action + data DataAction } matchedHandler struct { @@ -77,8 +77,8 @@ func (ha *handlerAction) set(method string, action Action) { ha.method[method] = action } -func (ha *handlerAction) setAny(any Action) { - ha.any = any +func (ha *handlerAction) setAny(anyAction Action) { + ha.anyAction = anyAction } func (ha *handlerAction) setData(data DataAction) { @@ -87,7 +87,7 @@ func (ha *handlerAction) setData(data DataAction) { func (mh matchedHandlers) getHandleAny() *matchedHandler { for _, matched := range mh { - if matched.handlerAction.any != nil { + if matched.handlerAction.anyAction != nil { return matched } } @@ -166,7 +166,7 @@ func (registry *RouterRegistry) Has(method, name string) bool { // HasAny checks if an any handler is set for a given name func (registry *RouterRegistry) HasAny(name string) bool { la, ok := registry.handler[name] - return ok && la.any != nil + return ok && la.anyAction != nil } // HasData checks if a data handler is set for a given name @@ -190,20 +190,20 @@ func (registry *RouterRegistry) MustRoute(path, handler string) *Handler { // Route assigns a route to a Handler func (registry *RouterRegistry) Route(path, handler string) (*Handler, error) { - var h = parseHandler(handler) var err error - h.path, err = NewPath(path) + parsedHandler := parseHandler(handler) + parsedHandler.path, err = NewPath(path) if err != nil { return nil, err } - if len(h.params) == 0 { - h.params, h.catchall = parseParams(strings.Join(h.path.params, ", ")) + if len(parsedHandler.params) == 0 { + parsedHandler.params, parsedHandler.catchall = parseParams(strings.Join(parsedHandler.path.params, ", ")) } - registry.routes = append(registry.routes, h) - return h, nil + registry.routes = append(registry.routes, parsedHandler) + return parsedHandler, nil } // GetRoutes returns registered Routes @@ -237,10 +237,12 @@ func parseParams(list string) (params map[string]*param, catchall bool) { // try to get enough space for the list params = make(map[string]*param, strings.Count(list, ",")) - var name, val string - var optional bool - var quote byte - var readto = &name + var ( + name, val string + optional bool + quote byte + ) + readto := &name for i := 0; i < len(list); i++ { if list[i] != quote && quote != 0 { @@ -306,16 +308,18 @@ func parseParams(list string) (params map[string]*param, catchall bool) { func (registry *RouterRegistry) Reverse(name string, params map[string]string) (string, error) { if alias, ok := registry.alias[name]; ok { name = alias.handler + if params == nil { params = make(map[string]string, len(alias.params)) } + for name, param := range alias.params { params[name] = param.value } } - var keys = make([]string, len(params)) - var i = 0 + keys := make([]string, len(params)) + i := 0 for k := range params { keys[i] = k i++ @@ -413,6 +417,7 @@ func (registry *RouterRegistry) match(path string) (handler handlerAction, param if match := route.path.Match(path); match != nil { handler = registry.handler[route.handler] params = make(map[string]string) + for k, param := range route.params { params[k] = param.value } diff --git a/framework/web/request.go b/framework/web/request.go index b27c4c6c2..73ac65c87 100644 --- a/framework/web/request.go +++ b/framework/web/request.go @@ -3,6 +3,7 @@ package web import ( "context" "errors" + "fmt" "net/http" "net/url" "strings" @@ -45,6 +46,7 @@ func CreateRequest(r *http.Request, s *Session) *Request { r, _ := http.NewRequest(http.MethodGet, "", nil) req.request = *r } + if s != nil { req.session.s = s.s } else { @@ -85,7 +87,6 @@ func (r *Request) RemoteAddress() []string { for _, ip := range ips { remoteAddress = append(remoteAddress, strings.TrimSpace(ip)) } - } remoteAddress = append(remoteAddress, strings.TrimSpace(r.request.RemoteAddr)) @@ -112,6 +113,7 @@ func (r *Request) Form1(name string) (string, error) { if err != nil { return "", err } + if len(f) > 0 { return f[0], nil } @@ -121,8 +123,10 @@ func (r *Request) Form1(name string) (string, error) { // FormAll get all POST values func (r *Request) FormAll() (map[string][]string, error) { - err := r.Request().ParseForm() - return r.Request().Form, err + if err := r.Request().ParseForm(); err != nil { + return nil, fmt.Errorf("unable to parse request form: %w", err) + } + return r.Request().Form, nil } // Query looks up Raw Query map for Param diff --git a/framework/web/result.go b/framework/web/result.go index 911a462b7..09189df4e 100644 --- a/framework/web/result.go +++ b/framework/web/result.go @@ -180,7 +180,7 @@ func (r *Response) Apply(_ context.Context, responseWriter http.ResponseWriter) } // SetNoCache helper -// deprecated: use CacheControlHeader instead +// Deprecated: use CacheControlHeader instead func (r *Response) SetNoCache() *Response { r.CacheDirective = CacheDirectiveBuilder{IsReusable: false}.Build() return r @@ -200,7 +200,7 @@ func (r *Responder) RouteRedirect(to string, data map[string]string) *RouteRedir } // Apply response -func (r *RouteRedirectResponse) Apply(c context.Context, w http.ResponseWriter) error { +func (r *RouteRedirectResponse) Apply(ctx context.Context, w http.ResponseWriter) error { if r.router == nil { return errors.New("no reverserouter available") } @@ -211,7 +211,7 @@ func (r *RouteRedirectResponse) Apply(c context.Context, w http.ResponseWriter) } to.Fragment = r.fragment w.Header().Set("Location", to.String()) - return r.Response.Apply(c, w) + return r.Response.Apply(ctx, w) } // Fragment adds a fragment to the resulting URL, argument must be given without '#' @@ -281,12 +281,13 @@ func (r *Responder) Data(data interface{}) *DataResponse { func (r *DataResponse) Apply(c context.Context, w http.ResponseWriter) error { buf := new(bytes.Buffer) if err := json.NewEncoder(buf).Encode(r.Data); err != nil { - return err + return fmt.Errorf("unable to encode response: %w", err) } r.Body = buf if r.Response.Header == nil { r.Response.Header = make(http.Header) } + r.Response.Header.Set("Content-Type", "application/json; charset=utf-8") return r.Response.Apply(c, w) } @@ -330,48 +331,49 @@ func (r *Responder) Render(tpl string, data interface{}) *RenderResponse { } // Apply response -func (r *RenderResponse) Apply(c context.Context, w http.ResponseWriter) error { +func (response *RenderResponse) Apply(c context.Context, w http.ResponseWriter) error { var err error - if r.engine == nil { - return r.DataResponse.Apply(c, w) + if response.engine == nil { + return response.DataResponse.Apply(c, w) } - if req := RequestFromContext(c); req != nil && r.engine != nil { - partialRenderer, ok := r.engine.(flamingo.PartialTemplateEngine) + if req := RequestFromContext(c); req != nil && response.engine != nil { + partialRenderer, ok := response.engine.(flamingo.PartialTemplateEngine) if partials := req.Request().Header.Get("X-Partial"); partials != "" && ok { - content, err := partialRenderer.RenderPartials(c, r.Template, r.Data, strings.Split(partials, ",")) + content, err := partialRenderer.RenderPartials(c, response.Template, response.Data, strings.Split(partials, ",")) if err != nil { - return err + return fmt.Errorf("unable to render partials: %w", err) } result := make(map[string]string, len(content)) for k, v := range content { buf, err := io.ReadAll(v) if err != nil { - return err + return fmt.Errorf("unabelt to read partial: %w", err) } result[k] = string(buf) } body, err := json.Marshal(map[string]interface{}{"partials": result, "data": new(GetPartialDataFunc).Func(c).(func() map[string]interface{})()}) if err != nil { - return err + return fmt.Errorf("unable to marshal response body: %w", err) } - r.Body = bytes.NewBuffer(body) - r.Header.Set("Content-Type", "application/json; charset=utf-8") - return r.Response.Apply(c, w) + response.Body = bytes.NewBuffer(body) + response.Header.Set("Content-Type", "application/json; charset=utf-8") + return response.Response.Apply(c, w) } } - if r.Header == nil { - r.Header = make(http.Header) + if response.Header == nil { + response.Header = make(http.Header) } - r.Header.Set("Content-Type", "text/html; charset=utf-8") - r.Body, err = r.engine.Render(c, r.Template, r.Data) + + response.Header.Set("Content-Type", "text/html; charset=utf-8") + response.Body, err = response.engine.Render(c, response.Template, response.Data) if err != nil { - return err + return fmt.Errorf("unable to render response: %w", err) } - return r.Response.Apply(c, w) + return response.Response.Apply(c, w) } // SetNoCache helper diff --git a/framework/web/router.go b/framework/web/router.go index c069eb754..01b11db2d 100644 --- a/framework/web/router.go +++ b/framework/web/router.go @@ -181,8 +181,8 @@ func (r *Router) relative(to string, params map[string]string) (string, error) { } // Relative returns a root-relative URL, starting with `/` -func (r *Router) Relative(to string, params map[string]string) (*url.URL, error) { - if to == "" { +func (r *Router) Relative(target string, params map[string]string) (*url.URL, error) { + if target == "" { relativePath := r.Base().Path if r.external != nil { relativePath = r.external.Path @@ -193,7 +193,7 @@ func (r *Router) Relative(to string, params map[string]string) (*url.URL, error) }, nil } - p, err := r.relative(to, params) + p, err := r.relative(target, params) if err != nil { return nil, err } @@ -208,10 +208,10 @@ func (r *Router) Relative(to string, params map[string]string) (*url.URL, error) // Absolute returns an absolute URL, with scheme and host. // It takes the request to construct as many information as possible -func (r *Router) Absolute(req *Request, to string, params map[string]string) (*url.URL, error) { +func (r *Router) Absolute(req *Request, target string, params map[string]string) (*url.URL, error) { if r.external != nil { e := *r.external - p, err := r.relative(to, params) + p, err := r.relative(target, params) if err != nil { return nil, err } @@ -234,7 +234,7 @@ func (r *Router) Absolute(req *Request, to string, params map[string]string) (*u host = req.request.Host } - u, err := r.Relative(to, params) + u, err := r.Relative(target, params) if err != nil { return u, err } @@ -269,6 +269,7 @@ func dataParams(params map[interface{}]interface{}) RequestParams { func (r *Router) Data(ctx context.Context, handler string, params map[interface{}]interface{}) interface{} { ctx, span := trace.StartSpan(ctx, "flamingo/router/data") span.Annotate(nil, handler) + defer span.End() req := RequestFromContext(ctx) diff --git a/framework/web/session.go b/framework/web/session.go index 3ad06a28f..ca08a956e 100644 --- a/framework/web/session.go +++ b/framework/web/session.go @@ -66,6 +66,7 @@ func (s *Session) Load(key interface{}) (data interface{}, ok bool) { defer s.mu.Unlock() data, ok = s.s.Values[key] + if s.sessionSaveMode <= sessionSaveOnRead { s.markDirty(key) } diff --git a/framework/web/sessionstore.go b/framework/web/sessionstore.go index 96a67e389..0d6a13be1 100644 --- a/framework/web/sessionstore.go +++ b/framework/web/sessionstore.go @@ -58,7 +58,7 @@ func (s *SessionStore) LoadByRequest(ctx context.Context, req *http.Request) (*S span.AddAttributes(trace.StringAttribute(string(flamingo.LogKeySession), hashID(gs.ID))) - return &Session{s: gs, sessionSaveMode: s.sessionSaveMode}, err + return &Session{s: gs, sessionSaveMode: s.sessionSaveMode}, fmt.Errorf("unable to create new session: %w", err) } // LoadByID loads a Session from a provided session id @@ -112,7 +112,7 @@ func (s *SessionStore) Save(ctx context.Context, session *Session) (http.Header, session.mu.Lock() defer session.mu.Unlock() - gs := session.s + gorillaSession := session.s // copy dirty values to new instance and move Values to original session if s.sessionSaveMode != sessionSaveAlways && !session.dirtyAll && session.s.ID != "" { @@ -127,20 +127,20 @@ func (s *SessionStore) Save(ctx context.Context, session *Session) (http.Header, } var ok bool for k := range session.dirty { - newGs.s.Values[k], ok = gs.Values[k] + newGs.s.Values[k], ok = gorillaSession.Values[k] if !ok { delete(newGs.s.Values, k) } } - gs.Values = newGs.s.Values + gorillaSession.Values = newGs.s.Values session.dirty = nil } _, span := trace.StartSpan(ctx, "flamingo/web/session/save") defer span.End() rw := headerResponseWriter(make(http.Header)) - if err := s.sessionStore.Save(s.requestFromID(session.s.ID), rw, gs); err != nil { - return nil, err + if err := s.sessionStore.Save(s.requestFromID(session.s.ID), rw, gorillaSession); err != nil { + return nil, fmt.Errorf("unable to save session: %w", err) } return rw.Header(), nil diff --git a/framework/web/sessionstore_test.go b/framework/web/sessionstore_test.go index 53fa75e12..51f995e06 100644 --- a/framework/web/sessionstore_test.go +++ b/framework/web/sessionstore_test.go @@ -91,7 +91,7 @@ func TestSessionSaveOnRead(t *testing.T) { assert.Equal(t, map[interface{}]interface{}{ "key1": "val0", // from session2 "key2": "val1", // from session1 - //"key3": nil, // deleted + // "key3": nil, // deleted "key4": "val2", // from session2 "key5": "val0", // from session2 read // no flashes because session2 did not touch it, the deletion from session1 wins @@ -127,7 +127,7 @@ func TestSessionSaveOnWrite(t *testing.T) { assert.Equal(t, map[interface{}]interface{}{ "key1": "val2", // from session2 "key2": "val1", // from session1 - //"key3": nil, // deleted + // "key3": nil, // deleted // no flashes because session2 did not touch it, the deletion from session1 wins }, session.s.Values) } diff --git a/framework/web/templatefunctions.go b/framework/web/templatefunctions.go index 327f26899..22adf95b7 100644 --- a/framework/web/templatefunctions.go +++ b/framework/web/templatefunctions.go @@ -20,19 +20,24 @@ const ctxKey partialDataContextKey = "partialData" // Func getter to bind the context func (*SetPartialDataFunc) Func(c context.Context) interface{} { return func(key string, val interface{}) interface{} { - r := RequestFromContext(c) - if r == nil { + request := RequestFromContext(c) + if request == nil { return nil } - data, ok := r.Values.Load(ctxKey) + data, ok := request.Values.Load(ctxKey) if !ok || data == nil { data = make(map[string]interface{}) } - data.(map[string]interface{})[key] = val + rawData, ok := data.(map[string]interface{}) + if !ok { + rawData = make(map[string]interface{}) + } - r.Values.Store(ctxKey, data) + rawData[key] = val + + request.Values.Store(ctxKey, rawData) return nil } @@ -51,7 +56,11 @@ func (*GetPartialDataFunc) Func(c context.Context) interface{} { return nil } - return data.(map[string]interface{}) + rawData, ok := data.(map[string]interface{}) + if !ok { + return nil + } + return rawData } } diff --git a/framework/web/utils.go b/framework/web/utils.go index 53be9f129..e7913be08 100644 --- a/framework/web/utils.go +++ b/framework/web/utils.go @@ -6,12 +6,12 @@ import ( // URLTitle normalizes a title for nice usage in URLs func URLTitle(title string) string { - url := strings.ToLower(strings.Replace(strings.Replace(title, "/", "_", -1), " ", "-", -1)) - url = strings.Replace(url, "-_", "-", -1) - url = strings.Replace(url, "%", "-", -1) + url := strings.ToLower(strings.ReplaceAll(strings.ReplaceAll(title, "/", "_"), " ", "-")) + url = strings.ReplaceAll(url, "-_", "-") + url = strings.ReplaceAll(url, "%", "-") + for strings.Contains(url, "--") { - url = strings.Replace(url, "--", "-", -1) + url = strings.ReplaceAll(url, "--", "-") } - return url } diff --git a/framework/web/utils_test.go b/framework/web/utils_test.go index 5b191e235..059e68ec1 100644 --- a/framework/web/utils_test.go +++ b/framework/web/utils_test.go @@ -8,6 +8,7 @@ import ( ) func TestURLTitle(t *testing.T) { + t.Parallel() assert.Equal(t, "test_a-123-name-test", URLTitle("test/a 123 name % / _ - _ test")) }