diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 012380ef0bf..86243b18d3d 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -411,10 +411,7 @@ func setupHTTPServerWithCfgDb(t *testing.T, useFakeAccessControl, enableAccessCo m.Use(func(c *web.Context) { initCtx.Context = c initCtx.Logger = log.New("api-test") - c.Map(initCtx) - c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), initCtx)) - c.Map(c.Req) }) m.Use(accesscontrol.LoadPermissionsMiddleware(hs.AccessControl)) diff --git a/pkg/infra/usagestats/service/api_test.go b/pkg/infra/usagestats/service/api_test.go index ea67c868442..b48dfe0ab0f 100644 --- a/pkg/infra/usagestats/service/api_test.go +++ b/pkg/infra/usagestats/service/api_test.go @@ -103,9 +103,6 @@ func contextProvider(tc *testContext) web.Handler { SkipCache: true, Logger: log.New("test"), } - c.Map(reqCtx) - c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), reqCtx)) - c.Map(c.Req) } } diff --git a/pkg/middleware/request_metrics.go b/pkg/middleware/request_metrics.go index 6f9b2618993..fd17e256a1f 100644 --- a/pkg/middleware/request_metrics.go +++ b/pkg/middleware/request_metrics.go @@ -56,7 +56,6 @@ func RequestMetrics(features featuremgmt.FeatureToggles) web.Handler { now := time.Now() httpRequestsInFlight.Inc() defer httpRequestsInFlight.Dec() - c.Map(c.Req) c.Next() handler := "unknown" diff --git a/pkg/middleware/request_tracing.go b/pkg/middleware/request_tracing.go index f877a1f37ef..e477749ce9a 100644 --- a/pkg/middleware/request_tracing.go +++ b/pkg/middleware/request_tracing.go @@ -55,8 +55,6 @@ func RequestTracing(tracer tracing.Tracer) web.Handler { ctx, span := tracer.Start(req.Context(), fmt.Sprintf("HTTP %s %s", req.Method, req.URL.Path), trace.WithLinks(trace.LinkFromContext(wireContext))) c.Req = req.WithContext(ctx) - c.Map(c.Req) - c.Next() // Only call span.Finish when a route operation name have been set, diff --git a/pkg/services/accesscontrol/middleware_test.go b/pkg/services/accesscontrol/middleware_test.go index 887390f34b8..e9d64c2cb91 100644 --- a/pkg/services/accesscontrol/middleware_test.go +++ b/pkg/services/accesscontrol/middleware_test.go @@ -90,9 +90,6 @@ func contextProvider() web.Handler { IsSignedIn: true, SkipCache: true, } - c.Map(reqCtx) - c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), reqCtx)) - c.Map(c.Req) } } diff --git a/pkg/services/accesscontrol/resourcepermissions/api_test.go b/pkg/services/accesscontrol/resourcepermissions/api_test.go index c1786f435b5..38d25552d21 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/api_test.go @@ -500,10 +500,7 @@ func contextProvider(tc *testContext) web.Handler { SkipCache: true, Logger: log.New("test"), } - c.Map(reqCtx) - c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), reqCtx)) - c.Map(c.Req) } } diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index 4e09458bd5b..d01a4158dbd 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -95,9 +95,8 @@ func (h *ContextHandler) Middleware(mContext *web.Context) { Logger: log.New("context"), } - // Inject ReqContext into a request context and replace the request instance in the macaron context + // Inject ReqContext into http.Request.Context mContext.Req = mContext.Req.WithContext(ctxkey.Set(mContext.Req.Context(), reqContext)) - mContext.Map(mContext.Req) traceID := tracing.TraceIDFromContext(mContext.Req.Context(), false) if traceID != "" { @@ -153,8 +152,6 @@ func (h *ContextHandler) Middleware(mContext *web.Context) { {Num: reqContext.UserId}}, ) - mContext.Map(reqContext) - // update last seen every 5min if reqContext.ShouldUpdateLastSeenAt() { reqContext.Logger.Debug("Updating last user_seen_at", "user_id", reqContext.UserId) diff --git a/pkg/services/ngalert/metrics/ngalert.go b/pkg/services/ngalert/metrics/ngalert.go index 6bbf6a601b9..916ac41a21f 100644 --- a/pkg/services/ngalert/metrics/ngalert.go +++ b/pkg/services/ngalert/metrics/ngalert.go @@ -12,7 +12,6 @@ import ( "github.com/prometheus/client_golang/prometheus/promauto" "github.com/grafana/grafana/pkg/api/response" - "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/models" legacyMetrics "github.com/grafana/grafana/pkg/services/alerting/metrics" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" @@ -279,20 +278,14 @@ func (m *OrgRegistries) RemoveOrgRegistry(org int64) { func Instrument( method, path string, - action interface{}, + action func(*models.ReqContext) response.Response, metrics *API, ) web.Handler { normalizedPath := MakeLabelValue(path) return func(c *models.ReqContext) { start := time.Now() - var res response.Response - val, err := c.Invoke(action) - if err == nil && val != nil && len(val) > 0 { - res = val[0].Interface().(response.Response) - } else { - res = routing.ServerError(err) - } + res := action(c) // TODO: We could look up the datasource type via our datasource service var backend string diff --git a/pkg/services/serviceaccounts/api/api_test.go b/pkg/services/serviceaccounts/api/api_test.go index c6db4b27b23..e13968e2e5c 100644 --- a/pkg/services/serviceaccounts/api/api_test.go +++ b/pkg/services/serviceaccounts/api/api_test.go @@ -238,10 +238,7 @@ func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock, SignedInUser: signedUser, Logger: log.New("serviceaccounts-test"), } - c.Map(ctx) - c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), ctx)) - c.Map(c.Req) }) a.RouterRegister.Register(m.Router) return m, a diff --git a/pkg/web/context.go b/pkg/web/context.go index 8cea4cec9cb..06095e70aa6 100644 --- a/pkg/web/context.go +++ b/pkg/web/context.go @@ -20,27 +20,16 @@ import ( "net" "net/http" "net/url" - "reflect" "strconv" "strings" "github.com/grafana/grafana/pkg/infra/log" ) -// ContextInvoker is an inject.FastInvoker wrapper of func(ctx *Context). -type ContextInvoker func(ctx *Context) - -// Invoke implements inject.FastInvoker which simplifies calls of `func(ctx *Context)` function. -func (invoke ContextInvoker) Invoke(params []interface{}) ([]reflect.Value, error) { - invoke(params[0].(*Context)) - return nil, nil -} - // Context represents the runtime context of current request of Macaron instance. // It is the integration of most frequently used middlewares and helper methods. type Context struct { - Injector - handlers []Handler + handlers []http.Handler index int *Router @@ -50,12 +39,12 @@ type Context struct { logger log.Logger } -func (ctx *Context) handler() Handler { +func (ctx *Context) handler() http.Handler { if ctx.index < len(ctx.handlers) { return ctx.handlers[ctx.index] } if ctx.index == len(ctx.handlers) { - return func() {} + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) } panic("invalid index for context handler") } @@ -68,9 +57,8 @@ func (ctx *Context) Next() { func (ctx *Context) run() { for ctx.index <= len(ctx.handlers) { - if _, err := ctx.Invoke(ctx.handler()); err != nil { - panic(err) - } + ctx.handler().ServeHTTP(ctx.Resp, ctx.Req) + ctx.index++ if ctx.Resp.Written() { return diff --git a/pkg/web/inject.go b/pkg/web/inject.go deleted file mode 100644 index fb5c27efe87..00000000000 --- a/pkg/web/inject.go +++ /dev/null @@ -1,192 +0,0 @@ -// Copyright 2013 Jeremy Saenz -// Copyright 2015 The Macaron Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"): you may -// not use this file except in compliance with the License. You may obtain -// a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -// License for the specific language governing permissions and limitations -// under the License. - -// Package inject provides utilities for mapping and injecting dependencies in various ways. -package web - -import ( - "fmt" - "reflect" -) - -// Injector represents an interface for mapping and injecting dependencies into structs -// and function arguments. -type Injector interface { - Invoker - TypeMapper -} - -// Invoker represents an interface for calling functions via reflection. -type Invoker interface { - // Invoke attempts to call the interface{} provided as a function, - // providing dependencies for function arguments based on Type. Returns - // a slice of reflect.Value representing the returned values of the function. - // Returns an error if the injection fails. - Invoke(interface{}) ([]reflect.Value, error) -} - -// FastInvoker represents an interface in order to avoid the calling function via reflection. -// -// example: -// type handlerFuncHandler func(http.ResponseWriter, *http.Request) error -// func (f handlerFuncHandler)Invoke([]interface{}) ([]reflect.Value, error){ -// ret := f(p[0].(http.ResponseWriter), p[1].(*http.Request)) -// return []reflect.Value{reflect.ValueOf(ret)}, nil -// } -// -// type funcHandler func(int, string) -// func (f funcHandler)Invoke([]interface{}) ([]reflect.Value, error){ -// f(p[0].(int), p[1].(string)) -// return nil, nil -// } -type FastInvoker interface { - // Invoke attempts to call the ordinary functions. If f is a function - // with the appropriate signature, f.Invoke([]interface{}) is a Call that calls f. - // Returns a slice of reflect.Value representing the returned values of the function. - // Returns an error if the injection fails. - Invoke([]interface{}) ([]reflect.Value, error) -} - -// IsFastInvoker check interface is FastInvoker -func IsFastInvoker(h interface{}) bool { - _, ok := h.(FastInvoker) - return ok -} - -// TypeMapper represents an interface for mapping interface{} values based on type. -type TypeMapper interface { - // Maps the interface{} value based on its immediate type from reflect.TypeOf. - Map(interface{}) TypeMapper - // Maps the interface{} value based on the pointer of an Interface provided. - // This is really only useful for mapping a value as an interface, as interfaces - // cannot at this time be referenced directly without a pointer. - MapTo(interface{}, interface{}) TypeMapper - // Returns the Value that is mapped to the current type. Returns a zeroed Value if - // the Type has not been mapped. - GetVal(reflect.Type) reflect.Value -} - -type injector struct { - values map[reflect.Type]reflect.Value -} - -// InterfaceOf dereferences a pointer to an Interface type. -// It panics if value is not an pointer to an interface. -func InterfaceOf(value interface{}) reflect.Type { - t := reflect.TypeOf(value) - - for t.Kind() == reflect.Ptr { - t = t.Elem() - } - - if t.Kind() != reflect.Interface { - panic("Called inject.InterfaceOf with a value that is not a pointer to an interface. (*MyInterface)(nil)") - } - - return t -} - -// New returns a new Injector. -func NewInjector() Injector { - return &injector{ - values: make(map[reflect.Type]reflect.Value), - } -} - -// Invoke attempts to call the interface{} provided as a function, -// providing dependencies for function arguments based on Type. -// Returns a slice of reflect.Value representing the returned values of the function. -// Returns an error if the injection fails. -// It panics if f is not a function -func (inj *injector) Invoke(f interface{}) ([]reflect.Value, error) { - t := reflect.TypeOf(f) - switch v := f.(type) { - case FastInvoker: - return inj.fastInvoke(v, t, t.NumIn()) - default: - return inj.callInvoke(f, t, t.NumIn()) - } -} - -func (inj *injector) fastInvoke(f FastInvoker, t reflect.Type, numIn int) ([]reflect.Value, error) { - var in []interface{} - if numIn > 0 { - in = make([]interface{}, numIn) // Panic if t is not kind of Func - var argType reflect.Type - var val reflect.Value - for i := 0; i < numIn; i++ { - argType = t.In(i) - val = inj.GetVal(argType) - if !val.IsValid() { - return nil, fmt.Errorf("value not found for type %v", argType) - } - - in[i] = val.Interface() - } - } - return f.Invoke(in) -} - -// callInvoke reflect.Value.Call -func (inj *injector) callInvoke(f interface{}, t reflect.Type, numIn int) ([]reflect.Value, error) { - var in []reflect.Value - if numIn > 0 { - in = make([]reflect.Value, numIn) - var argType reflect.Type - var val reflect.Value - for i := 0; i < numIn; i++ { - argType = t.In(i) - val = inj.GetVal(argType) - if !val.IsValid() { - return nil, fmt.Errorf("value not found for type %v", argType) - } - - in[i] = val - } - } - return reflect.ValueOf(f).Call(in), nil -} - -// Maps the concrete value of val to its dynamic type using reflect.TypeOf, -// It returns the TypeMapper registered in. -func (inj *injector) Map(val interface{}) TypeMapper { - inj.values[reflect.TypeOf(val)] = reflect.ValueOf(val) - return inj -} - -func (inj *injector) MapTo(val interface{}, ifacePtr interface{}) TypeMapper { - inj.values[InterfaceOf(ifacePtr)] = reflect.ValueOf(val) - return inj -} - -func (inj *injector) GetVal(t reflect.Type) reflect.Value { - val := inj.values[t] - - if val.IsValid() { - return val - } - - // no concrete types found, try to find implementors - // if t is an interface - if t.Kind() == reflect.Interface { - for k, v := range inj.values { - if k.Implements(t) { - val = v - break - } - } - } - return val -} diff --git a/pkg/web/macaron.go b/pkg/web/macaron.go index e6d4a90a7d8..edec73da060 100644 --- a/pkg/web/macaron.go +++ b/pkg/web/macaron.go @@ -56,14 +56,14 @@ func hack_wrap(Handler) http.HandlerFunc // validateAndWrapHandler makes sure a handler is a callable function, it panics if not. // When the handler is also potential to be any built-in inject.FastInvoker, // it wraps the handler automatically to have some performance gain. -func validateAndWrapHandler(h Handler) Handler { +func validateAndWrapHandler(h Handler) http.Handler { return hack_wrap(h) } // validateAndWrapHandlers preforms validation and wrapping for each input handler. // It accepts an optional wrapper function to perform custom wrapping on handlers. -func validateAndWrapHandlers(handlers []Handler) []Handler { - wrappedHandlers := make([]Handler, len(handlers)) +func validateAndWrapHandlers(handlers []Handler) []http.Handler { + wrappedHandlers := make([]http.Handler, len(handlers)) for i, h := range handlers { wrappedHandlers[i] = validateAndWrapHandler(h) } @@ -74,7 +74,7 @@ func validateAndWrapHandlers(handlers []Handler) []Handler { // Macaron represents the top level web application. // Injector methods can be invoked to map services on a global level. type Macaron struct { - handlers []Handler + handlers []http.Handler urlPrefix string // For suburl support. *Router @@ -136,35 +136,29 @@ func (m *Macaron) UseMiddleware(middleware func(http.Handler) http.Handler) { } else { c.Resp = NewResponseWriter(req.Method, rw) } - c.Map(req) - c.MapTo(rw, (*http.ResponseWriter)(nil)) c.Next() }) - m.handlers = append(m.handlers, Handler(middleware(next))) + m.handlers = append(m.handlers, middleware(next)) } // Use adds a middleware Handler to the stack, // and panics if the handler is not a callable func. // Middleware Handlers are invoked in the order that they are added. func (m *Macaron) Use(handler Handler) { - handler = validateAndWrapHandler(handler) - m.handlers = append(m.handlers, handler) + h := validateAndWrapHandler(handler) + m.handlers = append(m.handlers, h) } func (m *Macaron) createContext(rw http.ResponseWriter, req *http.Request) *Context { c := &Context{ - Injector: NewInjector(), handlers: m.handlers, index: 0, Router: m.Router, Resp: NewResponseWriter(req.Method, rw), logger: log.New("macaron.context"), } - req = req.WithContext(context.WithValue(req.Context(), macaronContextKey{}, c)) - c.Map(c) - c.MapTo(c.Resp, (*http.ResponseWriter)(nil)) - c.Map(req) - c.Req = req + + c.Req = req.WithContext(context.WithValue(req.Context(), macaronContextKey{}, c)) return c } diff --git a/pkg/web/router.go b/pkg/web/router.go index feed5e27556..3f32d428c29 100644 --- a/pkg/web/router.go +++ b/pkg/web/router.go @@ -146,13 +146,13 @@ func (r *Router) Handle(method string, pattern string, handlers []Handler) { h = append(h, handlers...) handlers = h } - handlers = validateAndWrapHandlers(handlers) + httpHandlers := validateAndWrapHandlers(handlers) r.handle(method, pattern, func(resp http.ResponseWriter, req *http.Request, params map[string]string) { c := r.m.createContext(resp, SetURLParams(req, params)) - c.handlers = make([]Handler, 0, len(r.m.handlers)+len(handlers)) + c.handlers = make([]http.Handler, 0, len(r.m.handlers)+len(handlers)) c.handlers = append(c.handlers, r.m.handlers...) - c.handlers = append(c.handlers, handlers...) + c.handlers = append(c.handlers, httpHandlers...) c.run() }) } @@ -194,12 +194,12 @@ func (r *Router) Any(pattern string, h ...Handler) { r.Handle("*", pattern, h) } // found. If it is not set, http.NotFound is used. // Be sure to set 404 response code in your handler. func (r *Router) NotFound(handlers ...Handler) { - handlers = validateAndWrapHandlers(handlers) + httpHandlers := validateAndWrapHandlers(handlers) r.notFound = func(rw http.ResponseWriter, req *http.Request) { c := r.m.createContext(rw, req) - c.handlers = make([]Handler, 0, len(r.m.handlers)+len(handlers)) + c.handlers = make([]http.Handler, 0, len(r.m.handlers)+len(handlers)) c.handlers = append(c.handlers, r.m.handlers...) - c.handlers = append(c.handlers, handlers...) + c.handlers = append(c.handlers, httpHandlers...) c.run() } } diff --git a/pkg/web/webtest/webtest.go b/pkg/web/webtest/webtest.go index 8645914ebaf..849a7128d91 100644 --- a/pkg/web/webtest/webtest.go +++ b/pkg/web/webtest/webtest.go @@ -33,10 +33,8 @@ func NewServer(t testing.TB, routeRegister routing.RouteRegister) *Server { m.Use(func(c *web.Context) { initCtx.Context = c initCtx.Logger = log.New("api-test") - c.Map(initCtx) c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), initCtx)) - c.Map(c.Req) }) m.Use(requestContextMiddleware()) @@ -129,7 +127,9 @@ func requestContextFromRequest(req *http.Request) *models.ReqContext { } func requestContextMiddleware() web.Handler { - return func(res http.ResponseWriter, req *http.Request, c *models.ReqContext) { + return func(res http.ResponseWriter, req *http.Request) { + c := ctxkey.Get(req.Context()).(*models.ReqContext) + ctx := requestContextFromRequest(req) if ctx == nil { c.Next() @@ -145,6 +145,5 @@ func requestContextMiddleware() web.Handler { c.RequestNonce = ctx.RequestNonce c.PerfmonTimer = ctx.PerfmonTimer c.LookupTokenErr = ctx.LookupTokenErr - c.Map(c) } }