diff --git a/pkg/services/ngalert/api/hooks.go b/pkg/services/ngalert/api/hooks.go index c90e23ec74e..e9341f47784 100644 --- a/pkg/services/ngalert/api/hooks.go +++ b/pkg/services/ngalert/api/hooks.go @@ -1,6 +1,8 @@ package api import ( + "net/url" + "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/infra/log" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" @@ -29,12 +31,19 @@ func (h *Hooks) Set(path string, hook RequestHandlerFunc) { h.hooks[path] = hook } +// Get returns a hook if one is defined for the matching URL. +// Get also returns a bool indicating whether or not a matching hook exists. +func (h *Hooks) Get(url *url.URL) (RequestHandlerFunc, bool) { + hook, ok := h.hooks[url.Path] + return hook, ok +} + // Wrap returns a new handler which will intercept paths with hooks configured, // and invoke the hooked in handler instead. If no hook is configured for a path, // then the given handler is invoked. func (h *Hooks) Wrap(next RequestHandlerFunc) RequestHandlerFunc { return func(req *contextmodel.ReqContext) response.Response { - if hook, ok := h.hooks[req.Context.Req.URL.Path]; ok { + if hook, ok := h.Get(req.Context.Req.URL); ok { h.logger.Debug("Hook defined - invoking new handler", "path", req.Context.Req.URL.Path) return hook(req) } diff --git a/pkg/services/ngalert/api/hooks_test.go b/pkg/services/ngalert/api/hooks_test.go new file mode 100644 index 00000000000..fc70b410276 --- /dev/null +++ b/pkg/services/ngalert/api/hooks_test.go @@ -0,0 +1,128 @@ +package api + +import ( + "net/http" + "net/url" + "testing" + + "github.com/grafana/grafana/pkg/api/response" + "github.com/grafana/grafana/pkg/infra/log" + contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" + "github.com/grafana/grafana/pkg/web" + "github.com/stretchr/testify/require" +) + +func TestHooks(t *testing.T) { + t.Run("Hooks", func(t *testing.T) { + t.Run("yields handlers for paths with hooks", func(t *testing.T) { + hooks := NewHooks(log.NewNopLogger()) + invoked := false + hook := func(*contextmodel.ReqContext) response.Response { invoked = true; return nil } + + hooks.Set("/some/path", hook) + reqURL, _ := url.Parse("http://domain.test/some/path") + handler, ok := hooks.Get(reqURL) + + require.True(t, ok, "hooks did not contain a matching hook for path") + require.False(t, invoked, "hook was invoked earlier than expected") + handler(nil) + require.True(t, invoked, "the hook returned by get() was not invoked as expected") + }) + + t.Run("yields no handlers for paths without hooks", func(t *testing.T) { + hooks := NewHooks(log.NewNopLogger()) + hook := func(*contextmodel.ReqContext) response.Response { return nil } + + hooks.Set("/some/path", hook) + reqURL, _ := url.Parse("http://domain.test/does/not/match") + handler, ok := hooks.Get(reqURL) + + require.False(t, ok, "hooks returned a hook when we expected it not to") + require.Nil(t, handler) + }) + + t.Run("hooks do not match routes with additional subpaths", func(t *testing.T) { + hooks := NewHooks(log.NewNopLogger()) + hook := func(*contextmodel.ReqContext) response.Response { return nil } + + hooks.Set("/some/path", hook) + reqURL, _ := url.Parse("http://domain.test/some/path/with/more") + handler, ok := hooks.Get(reqURL) + + require.False(t, ok, "hooks returned a hook when we expected it not to") + require.Nil(t, handler) + }) + + t.Run("hooks match routes with query parameters", func(t *testing.T) { + hooks := NewHooks(log.NewNopLogger()) + invoked := false + hook := func(*contextmodel.ReqContext) response.Response { invoked = true; return nil } + + hooks.Set("/some/path", hook) + reqURL, _ := url.Parse("http://domain.test/some/path?query=param") + handler, ok := hooks.Get(reqURL) + + require.True(t, ok, "hooks did not contain a matching hook for path") + require.False(t, invoked, "hook was invoked earlier than expected") + handler(nil) + require.True(t, invoked, "the hook returned by get() was not invoked as expected") + }) + + t.Run("hooks do not match routes with path variables", func(t *testing.T) { + hooks := NewHooks(log.NewNopLogger()) + hook := func(*contextmodel.ReqContext) response.Response { return nil } + + hooks.Set("/some/{value}", hook) + reqURL, _ := url.Parse("http://domain.test/some/123") + handler, ok := hooks.Get(reqURL) + + require.False(t, ok, "hooks returned a hook when we expected it not to") + require.Nil(t, handler) + }) + }) + + t.Run("Wrap", func(t *testing.T) { + t.Run("invokes hooks if one is defined", func(t *testing.T) { + defaultInvoked := false + defaultHandler := func(*contextmodel.ReqContext) response.Response { defaultInvoked = true; return nil } + hookInvoked := false + hookHandler := func(*contextmodel.ReqContext) response.Response { hookInvoked = true; return nil } + hooks := NewHooks(log.NewNopLogger()) + hooks.Set("/some/path", hookHandler) + + composed := hooks.Wrap(defaultHandler) + req := createReqWithURL("http://domain.test/some/path") + composed(req) + + require.True(t, hookInvoked, "hook was expected to be invoked, but it was not") + require.False(t, defaultInvoked, "default handler was invoked, but it should not have been") + }) + + t.Run("does not invoke hooks if path has none defined", func(t *testing.T) { + defaultInvoked := false + defaultHandler := func(*contextmodel.ReqContext) response.Response { defaultInvoked = true; return nil } + hookInvoked := false + hookHandler := func(*contextmodel.ReqContext) response.Response { hookInvoked = true; return nil } + hooks := NewHooks(log.NewNopLogger()) + hooks.Set("/some/path", hookHandler) + + composed := hooks.Wrap(defaultHandler) + req := createReqWithURL("http://domain.test/does/not/match") + composed(req) + + require.False(t, hookInvoked, "hook was invoked, but it should not have been") + require.True(t, defaultInvoked, "default handler was expected to be invoked, but it was not") + }) + }) +} + +func createReqWithURL(setupURL string) *contextmodel.ReqContext { + reqURL, _ := url.Parse(setupURL) + return &contextmodel.ReqContext{ + Context: &web.Context{ + Req: &http.Request{ + URL: reqURL, + }, + }, + } +}