diff --git a/CHANGELOG.md b/CHANGELOG.md index a00936090a4..19daed035c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,18 @@ * **Dataproxy**: Pass configured/auth headers to a Datasource [#10971](https://github.com/grafana/grafana/issues/10971), thx [@mrsiano](https://github.com/mrsiano) * **Cleanup**: Make temp file time to live configurable [#11607](https://github.com/grafana/grafana/issues/11607), thx [@xapon](https://github.com/xapon) +### Minor + +* **Api**: Delete nonexistent datasource should return 404 [#12313](https://github.com/grafana/grafana/issues/12313), thx [@AustinWinstanley](https://github.com/AustinWinstanley) +* **Dashboard**: Fix selecting current dashboard from search should not reload dashboard [#12248](https://github.com/grafana/grafana/issues/12248) + # 5.2.0 (unreleased) +### Minor + +* **Plugins**: Handle errors correctly when loading datasource plugin [#12383](https://github.com/grafana/grafana/pull/12383) thx [@rozetko](https://github.com/rozetko) +* **Render**: Enhance error message if phantomjs executable is not found [#11868](https://github.com/grafana/grafana/issues/11868) + # 5.2.0-beta3 (2018-06-21) ### Minor diff --git a/pkg/api/annotations.go b/pkg/api/annotations.go index 52eeb57dbb9..a44108537cb 100644 --- a/pkg/api/annotations.go +++ b/pkg/api/annotations.go @@ -37,7 +37,6 @@ func GetAnnotations(c *m.ReqContext) Response { if item.Email != "" { item.AvatarUrl = dtos.GetGravatarUrl(item.Email) } - item.Time = item.Time } return JSON(200, items) @@ -214,7 +213,9 @@ func DeleteAnnotations(c *m.ReqContext, cmd dtos.DeleteAnnotationsCmd) Response repo := annotations.GetRepository() err := repo.Delete(&annotations.DeleteParams{ - AlertId: cmd.PanelId, + OrgId: c.OrgId, + Id: cmd.AnnotationId, + RegionId: cmd.RegionId, DashboardId: cmd.DashboardId, PanelId: cmd.PanelId, }) @@ -235,7 +236,8 @@ func DeleteAnnotationByID(c *m.ReqContext) Response { } err := repo.Delete(&annotations.DeleteParams{ - Id: annotationID, + OrgId: c.OrgId, + Id: annotationID, }) if err != nil { @@ -254,6 +256,7 @@ func DeleteAnnotationRegion(c *m.ReqContext) Response { } err := repo.Delete(&annotations.DeleteParams{ + OrgId: c.OrgId, RegionId: regionID, }) diff --git a/pkg/api/annotations_test.go b/pkg/api/annotations_test.go index 9fe96245b9b..e5f63ce022b 100644 --- a/pkg/api/annotations_test.go +++ b/pkg/api/annotations_test.go @@ -100,6 +100,11 @@ func TestAnnotationsApiEndpoint(t *testing.T) { Id: 1, } + deleteCmd := dtos.DeleteAnnotationsCmd{ + DashboardId: 1, + PanelId: 1, + } + viewerRole := m.ROLE_VIEWER editorRole := m.ROLE_EDITOR @@ -171,6 +176,25 @@ func TestAnnotationsApiEndpoint(t *testing.T) { }) }) }) + + Convey("When user is an Admin", func() { + role := m.ROLE_ADMIN + Convey("Should be able to do anything", func() { + postAnnotationScenario("When calling POST on", "/api/annotations", "/api/annotations", role, cmd, func(sc *scenarioContext) { + sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() + So(sc.resp.Code, ShouldEqual, 200) + }) + + putAnnotationScenario("When calling PUT on", "/api/annotations/1", "/api/annotations/:annotationId", role, updateCmd, func(sc *scenarioContext) { + sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec() + So(sc.resp.Code, ShouldEqual, 200) + }) + deleteAnnotationsScenario("When calling POST on", "/api/annotations/mass-delete", "/api/annotations/mass-delete", role, deleteCmd, func(sc *scenarioContext) { + sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() + So(sc.resp.Code, ShouldEqual, 200) + }) + }) + }) }) } @@ -239,3 +263,26 @@ func putAnnotationScenario(desc string, url string, routePattern string, role m. fn(sc) }) } + +func deleteAnnotationsScenario(desc string, url string, routePattern string, role m.RoleType, cmd dtos.DeleteAnnotationsCmd, fn scenarioFunc) { + Convey(desc+" "+url, func() { + defer bus.ClearBusHandlers() + + sc := setupScenarioContext(url) + sc.defaultHandler = wrap(func(c *m.ReqContext) Response { + sc.context = c + sc.context.UserId = TestUserID + sc.context.OrgId = TestOrgID + sc.context.OrgRole = role + + return DeleteAnnotations(c, cmd) + }) + + fakeAnnoRepo = &fakeAnnotationsRepo{} + annotations.SetRepository(fakeAnnoRepo) + + sc.m.Post(routePattern, sc.defaultHandler) + + fn(sc) + }) +} diff --git a/pkg/api/api.go b/pkg/api/api.go index c205e7d3e2f..39dae56eb69 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -4,6 +4,7 @@ import ( "github.com/go-macaron/binding" "github.com/grafana/grafana/pkg/api/avatar" "github.com/grafana/grafana/pkg/api/dtos" + "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/middleware" m "github.com/grafana/grafana/pkg/models" ) @@ -117,10 +118,10 @@ func (hs *HTTPServer) registerRoutes() { r.Get("/api/login/ping", quota("session"), LoginAPIPing) // authed api - r.Group("/api", func(apiRoute RouteRegister) { + r.Group("/api", func(apiRoute routing.RouteRegister) { // user (signed in) - apiRoute.Group("/user", func(userRoute RouteRegister) { + apiRoute.Group("/user", func(userRoute routing.RouteRegister) { userRoute.Get("/", wrap(GetSignedInUser)) userRoute.Put("/", bind(m.UpdateUserCommand{}), wrap(UpdateSignedInUser)) userRoute.Post("/using/:id", wrap(UserSetUsingOrg)) @@ -140,7 +141,7 @@ func (hs *HTTPServer) registerRoutes() { }) // users (admin permission required) - apiRoute.Group("/users", func(usersRoute RouteRegister) { + apiRoute.Group("/users", func(usersRoute routing.RouteRegister) { usersRoute.Get("/", wrap(SearchUsers)) usersRoute.Get("/search", wrap(SearchUsersWithPaging)) usersRoute.Get("/:id", wrap(GetUserByID)) @@ -152,7 +153,7 @@ func (hs *HTTPServer) registerRoutes() { }, reqGrafanaAdmin) // team (admin permission required) - apiRoute.Group("/teams", func(teamsRoute RouteRegister) { + apiRoute.Group("/teams", func(teamsRoute routing.RouteRegister) { teamsRoute.Post("/", bind(m.CreateTeamCommand{}), wrap(CreateTeam)) teamsRoute.Put("/:teamId", bind(m.UpdateTeamCommand{}), wrap(UpdateTeam)) teamsRoute.Delete("/:teamId", wrap(DeleteTeamByID)) @@ -162,19 +163,19 @@ func (hs *HTTPServer) registerRoutes() { }, reqOrgAdmin) // team without requirement of user to be org admin - apiRoute.Group("/teams", func(teamsRoute RouteRegister) { + apiRoute.Group("/teams", func(teamsRoute routing.RouteRegister) { teamsRoute.Get("/:teamId", wrap(GetTeamByID)) teamsRoute.Get("/search", wrap(SearchTeams)) }) // org information available to all users. - apiRoute.Group("/org", func(orgRoute RouteRegister) { + apiRoute.Group("/org", func(orgRoute routing.RouteRegister) { orgRoute.Get("/", wrap(GetOrgCurrent)) orgRoute.Get("/quotas", wrap(GetOrgQuotas)) }) // current org - apiRoute.Group("/org", func(orgRoute RouteRegister) { + apiRoute.Group("/org", func(orgRoute routing.RouteRegister) { orgRoute.Put("/", bind(dtos.UpdateOrgForm{}), wrap(UpdateOrgCurrent)) orgRoute.Put("/address", bind(dtos.UpdateOrgAddressForm{}), wrap(UpdateOrgAddressCurrent)) orgRoute.Post("/users", quota("user"), bind(m.AddOrgUserCommand{}), wrap(AddOrgUserToCurrentOrg)) @@ -192,7 +193,7 @@ func (hs *HTTPServer) registerRoutes() { }, reqOrgAdmin) // current org without requirement of user to be org admin - apiRoute.Group("/org", func(orgRoute RouteRegister) { + apiRoute.Group("/org", func(orgRoute routing.RouteRegister) { orgRoute.Get("/users", wrap(GetOrgUsersForCurrentOrg)) }) @@ -203,7 +204,7 @@ func (hs *HTTPServer) registerRoutes() { apiRoute.Get("/orgs", reqGrafanaAdmin, wrap(SearchOrgs)) // orgs (admin routes) - apiRoute.Group("/orgs/:orgId", func(orgsRoute RouteRegister) { + apiRoute.Group("/orgs/:orgId", func(orgsRoute routing.RouteRegister) { orgsRoute.Get("/", wrap(GetOrgByID)) orgsRoute.Put("/", bind(dtos.UpdateOrgForm{}), wrap(UpdateOrg)) orgsRoute.Put("/address", bind(dtos.UpdateOrgAddressForm{}), wrap(UpdateOrgAddress)) @@ -217,24 +218,24 @@ func (hs *HTTPServer) registerRoutes() { }, reqGrafanaAdmin) // orgs (admin routes) - apiRoute.Group("/orgs/name/:name", func(orgsRoute RouteRegister) { + apiRoute.Group("/orgs/name/:name", func(orgsRoute routing.RouteRegister) { orgsRoute.Get("/", wrap(GetOrgByName)) }, reqGrafanaAdmin) // auth api keys - apiRoute.Group("/auth/keys", func(keysRoute RouteRegister) { + apiRoute.Group("/auth/keys", func(keysRoute routing.RouteRegister) { keysRoute.Get("/", wrap(GetAPIKeys)) keysRoute.Post("/", quota("api_key"), bind(m.AddApiKeyCommand{}), wrap(AddAPIKey)) keysRoute.Delete("/:id", wrap(DeleteAPIKey)) }, reqOrgAdmin) // Preferences - apiRoute.Group("/preferences", func(prefRoute RouteRegister) { + apiRoute.Group("/preferences", func(prefRoute routing.RouteRegister) { prefRoute.Post("/set-home-dash", bind(m.SavePreferencesCommand{}), wrap(SetHomeDashboard)) }) // Data sources - apiRoute.Group("/datasources", func(datasourceRoute RouteRegister) { + apiRoute.Group("/datasources", func(datasourceRoute routing.RouteRegister) { datasourceRoute.Get("/", wrap(GetDataSources)) datasourceRoute.Post("/", quota("data_source"), bind(m.AddDataSourceCommand{}), wrap(AddDataSource)) datasourceRoute.Put("/:id", bind(m.UpdateDataSourceCommand{}), wrap(UpdateDataSource)) @@ -250,7 +251,7 @@ func (hs *HTTPServer) registerRoutes() { apiRoute.Get("/plugins/:pluginId/settings", wrap(GetPluginSettingByID)) apiRoute.Get("/plugins/:pluginId/markdown/:name", wrap(GetPluginMarkdown)) - apiRoute.Group("/plugins", func(pluginRoute RouteRegister) { + apiRoute.Group("/plugins", func(pluginRoute routing.RouteRegister) { pluginRoute.Get("/:pluginId/dashboards/", wrap(GetPluginDashboards)) pluginRoute.Post("/:pluginId/settings", bind(m.UpdatePluginSettingCmd{}), wrap(UpdatePluginSetting)) }, reqOrgAdmin) @@ -260,17 +261,17 @@ func (hs *HTTPServer) registerRoutes() { apiRoute.Any("/datasources/proxy/:id", reqSignedIn, hs.ProxyDataSourceRequest) // Folders - apiRoute.Group("/folders", func(folderRoute RouteRegister) { + apiRoute.Group("/folders", func(folderRoute routing.RouteRegister) { folderRoute.Get("/", wrap(GetFolders)) folderRoute.Get("/id/:id", wrap(GetFolderByID)) folderRoute.Post("/", bind(m.CreateFolderCommand{}), wrap(CreateFolder)) - folderRoute.Group("/:uid", func(folderUidRoute RouteRegister) { + folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) { folderUidRoute.Get("/", wrap(GetFolderByUID)) folderUidRoute.Put("/", bind(m.UpdateFolderCommand{}), wrap(UpdateFolder)) folderUidRoute.Delete("/", wrap(DeleteFolder)) - folderUidRoute.Group("/permissions", func(folderPermissionRoute RouteRegister) { + folderUidRoute.Group("/permissions", func(folderPermissionRoute routing.RouteRegister) { folderPermissionRoute.Get("/", wrap(GetFolderPermissionList)) folderPermissionRoute.Post("/", bind(dtos.UpdateDashboardAclCommand{}), wrap(UpdateFolderPermissions)) }) @@ -278,7 +279,7 @@ func (hs *HTTPServer) registerRoutes() { }) // Dashboard - apiRoute.Group("/dashboards", func(dashboardRoute RouteRegister) { + apiRoute.Group("/dashboards", func(dashboardRoute routing.RouteRegister) { dashboardRoute.Get("/uid/:uid", wrap(GetDashboard)) dashboardRoute.Delete("/uid/:uid", wrap(DeleteDashboardByUID)) @@ -292,12 +293,12 @@ func (hs *HTTPServer) registerRoutes() { dashboardRoute.Get("/tags", GetDashboardTags) dashboardRoute.Post("/import", bind(dtos.ImportDashboardCommand{}), wrap(ImportDashboard)) - dashboardRoute.Group("/id/:dashboardId", func(dashIdRoute RouteRegister) { + dashboardRoute.Group("/id/:dashboardId", func(dashIdRoute routing.RouteRegister) { dashIdRoute.Get("/versions", wrap(GetDashboardVersions)) dashIdRoute.Get("/versions/:id", wrap(GetDashboardVersion)) dashIdRoute.Post("/restore", bind(dtos.RestoreDashboardVersionCommand{}), wrap(RestoreDashboardVersion)) - dashIdRoute.Group("/permissions", func(dashboardPermissionRoute RouteRegister) { + dashIdRoute.Group("/permissions", func(dashboardPermissionRoute routing.RouteRegister) { dashboardPermissionRoute.Get("/", wrap(GetDashboardPermissionList)) dashboardPermissionRoute.Post("/", bind(dtos.UpdateDashboardAclCommand{}), wrap(UpdateDashboardPermissions)) }) @@ -305,12 +306,12 @@ func (hs *HTTPServer) registerRoutes() { }) // Dashboard snapshots - apiRoute.Group("/dashboard/snapshots", func(dashboardRoute RouteRegister) { + apiRoute.Group("/dashboard/snapshots", func(dashboardRoute routing.RouteRegister) { dashboardRoute.Get("/", wrap(SearchDashboardSnapshots)) }) // Playlist - apiRoute.Group("/playlists", func(playlistRoute RouteRegister) { + apiRoute.Group("/playlists", func(playlistRoute routing.RouteRegister) { playlistRoute.Get("/", wrap(SearchPlaylists)) playlistRoute.Get("/:id", ValidateOrgPlaylist, wrap(GetPlaylist)) playlistRoute.Get("/:id/items", ValidateOrgPlaylist, wrap(GetPlaylistItems)) @@ -329,7 +330,7 @@ func (hs *HTTPServer) registerRoutes() { apiRoute.Get("/tsdb/testdata/gensql", reqGrafanaAdmin, wrap(GenerateSQLTestData)) apiRoute.Get("/tsdb/testdata/random-walk", wrap(GetTestDataRandomWalk)) - apiRoute.Group("/alerts", func(alertsRoute RouteRegister) { + apiRoute.Group("/alerts", func(alertsRoute routing.RouteRegister) { alertsRoute.Post("/test", bind(dtos.AlertTestCommand{}), wrap(AlertTest)) alertsRoute.Post("/:alertId/pause", reqEditorRole, bind(dtos.PauseAlertCommand{}), wrap(PauseAlert)) alertsRoute.Get("/:alertId", ValidateOrgAlert, wrap(GetAlert)) @@ -340,7 +341,7 @@ func (hs *HTTPServer) registerRoutes() { apiRoute.Get("/alert-notifications", wrap(GetAlertNotifications)) apiRoute.Get("/alert-notifiers", wrap(GetAlertNotifiers)) - apiRoute.Group("/alert-notifications", func(alertNotifications RouteRegister) { + apiRoute.Group("/alert-notifications", func(alertNotifications routing.RouteRegister) { alertNotifications.Post("/test", bind(dtos.NotificationTestCommand{}), wrap(NotificationTest)) alertNotifications.Post("/", bind(m.CreateAlertNotificationCommand{}), wrap(CreateAlertNotification)) alertNotifications.Put("/:notificationId", bind(m.UpdateAlertNotificationCommand{}), wrap(UpdateAlertNotification)) @@ -351,7 +352,7 @@ func (hs *HTTPServer) registerRoutes() { apiRoute.Get("/annotations", wrap(GetAnnotations)) apiRoute.Post("/annotations/mass-delete", reqOrgAdmin, bind(dtos.DeleteAnnotationsCmd{}), wrap(DeleteAnnotations)) - apiRoute.Group("/annotations", func(annotationsRoute RouteRegister) { + apiRoute.Group("/annotations", func(annotationsRoute routing.RouteRegister) { annotationsRoute.Post("/", bind(dtos.PostAnnotationsCmd{}), wrap(PostAnnotation)) annotationsRoute.Delete("/:annotationId", wrap(DeleteAnnotationByID)) annotationsRoute.Put("/:annotationId", bind(dtos.UpdateAnnotationsCmd{}), wrap(UpdateAnnotation)) @@ -365,7 +366,7 @@ func (hs *HTTPServer) registerRoutes() { }, reqSignedIn) // admin api - r.Group("/api/admin", func(adminRoute RouteRegister) { + r.Group("/api/admin", func(adminRoute routing.RouteRegister) { adminRoute.Get("/settings", AdminGetSettings) adminRoute.Post("/users", bind(dtos.AdminCreateUserForm{}), AdminCreateUser) adminRoute.Put("/users/:id/password", bind(dtos.AdminUpdateUserPasswordForm{}), AdminUpdateUserPassword) diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index 99677a93ee6..6ffefea991a 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -103,6 +103,9 @@ func DeleteDataSourceByName(c *m.ReqContext) Response { getCmd := &m.GetDataSourceByNameQuery{Name: name, OrgId: c.OrgId} if err := bus.Dispatch(getCmd); err != nil { + if err == m.ErrDataSourceNotFound { + return Error(404, "Data source not found", nil) + } return Error(500, "Failed to delete datasource", err) } diff --git a/pkg/api/datasources_test.go b/pkg/api/datasources_test.go index 490393727d6..6e52a27758b 100644 --- a/pkg/api/datasources_test.go +++ b/pkg/api/datasources_test.go @@ -46,5 +46,13 @@ func TestDataSourcesProxy(t *testing.T) { So(respJSON[3]["name"], ShouldEqual, "ZZZ") }) }) + + Convey("Should be able to save a data source", func() { + loggedInUserScenario("When calling DELETE on non-existing", "/api/datasources/name/12345", func(sc *scenarioContext) { + sc.handlerFunc = DeleteDataSourceByName + sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() + So(sc.resp.Code, ShouldEqual, 404) + }) + }) }) } diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 371c500a73e..e1a10fb468f 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -11,6 +11,7 @@ import ( "path" "time" + "github.com/grafana/grafana/pkg/api/routing" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -43,10 +44,10 @@ type HTTPServer struct { cache *gocache.Cache httpSrv *http.Server - RouteRegister RouteRegister `inject:""` - Bus bus.Bus `inject:""` - RenderService rendering.Service `inject:""` - Cfg *setting.Cfg `inject:""` + RouteRegister routing.RouteRegister `inject:""` + Bus bus.Bus `inject:""` + RenderService rendering.Service `inject:""` + Cfg *setting.Cfg `inject:""` } func (hs *HTTPServer) Init() error { diff --git a/pkg/api/render.go b/pkg/api/render.go index f6f149980d6..b8ef6cc5cb6 100644 --- a/pkg/api/render.go +++ b/pkg/api/render.go @@ -3,7 +3,9 @@ package api import ( "fmt" "net/http" + "runtime" "strconv" + "strings" "time" m "github.com/grafana/grafana/pkg/models" @@ -55,6 +57,15 @@ func (hs *HTTPServer) RenderToPng(c *m.ReqContext) { return } + if err != nil && err == rendering.ErrPhantomJSNotInstalled { + if strings.HasPrefix(runtime.GOARCH, "arm") { + c.Handle(500, "Rendering failed - PhantomJS isn't included in arm build per default", err) + } else { + c.Handle(500, "Rendering failed - PhantomJS isn't installed correctly", err) + } + return + } + if err != nil { c.Handle(500, "Rendering failed.", err) return diff --git a/pkg/api/route_register.go b/pkg/api/routing/route_register.go similarity index 70% rename from pkg/api/route_register.go rename to pkg/api/routing/route_register.go index 926de13c546..47531732564 100644 --- a/pkg/api/route_register.go +++ b/pkg/api/routing/route_register.go @@ -1,9 +1,10 @@ -package api +package routing import ( "net/http" + "strings" - macaron "gopkg.in/macaron.v1" + "gopkg.in/macaron.v1" ) type Router interface { @@ -14,15 +15,33 @@ type Router interface { // RouteRegister allows you to add routes and macaron.Handlers // that the web server should serve. type RouteRegister interface { + // Get adds a list of handlers to a given route with a GET HTTP verb Get(string, ...macaron.Handler) + + // Post adds a list of handlers to a given route with a POST HTTP verb Post(string, ...macaron.Handler) + + // Delete adds a list of handlers to a given route with a DELETE HTTP verb Delete(string, ...macaron.Handler) + + // Put adds a list of handlers to a given route with a PUT HTTP verb Put(string, ...macaron.Handler) + + // Patch adds a list of handlers to a given route with a PATCH HTTP verb Patch(string, ...macaron.Handler) + + // Any adds a list of handlers to a given route with any HTTP verb Any(string, ...macaron.Handler) + // Group allows you to pass a function that can add multiple routes + // with a shared prefix route. Group(string, func(RouteRegister), ...macaron.Handler) + // Insert adds more routes to an existing Group. + Insert(string, func(RouteRegister), ...macaron.Handler) + + // Register iterates over all routes added to the RouteRegister + // and add them to the `Router` pass as an parameter. Register(Router) *macaron.Router } @@ -52,6 +71,24 @@ type routeRegister struct { groups []*routeRegister } +func (rr *routeRegister) Insert(pattern string, fn func(RouteRegister), handlers ...macaron.Handler) { + + //loop over all groups at current level + for _, g := range rr.groups { + + // apply routes if the prefix matches the pattern + if g.prefix == pattern { + g.Group("", fn) + break + } + + // go down one level if the prefix can be find in the pattern + if strings.HasPrefix(pattern, g.prefix) { + g.Insert(pattern, fn) + } + } +} + func (rr *routeRegister) Group(pattern string, fn func(rr RouteRegister), handlers ...macaron.Handler) { group := &routeRegister{ prefix: rr.prefix + pattern, @@ -92,6 +129,12 @@ func (rr *routeRegister) route(pattern, method string, handlers ...macaron.Handl h = append(h, rr.subfixHandlers...) h = append(h, handlers...) + for _, r := range rr.routes { + if r.pattern == rr.prefix+pattern && r.method == method { + panic("cannot add duplicate route") + } + } + rr.routes = append(rr.routes, route{ method: method, pattern: rr.prefix + pattern, diff --git a/pkg/api/route_register_test.go b/pkg/api/routing/route_register_test.go similarity index 72% rename from pkg/api/route_register_test.go rename to pkg/api/routing/route_register_test.go index 3b5d79599a8..62e8989ff92 100644 --- a/pkg/api/route_register_test.go +++ b/pkg/api/routing/route_register_test.go @@ -1,11 +1,11 @@ -package api +package routing import ( "net/http" "strconv" "testing" - macaron "gopkg.in/macaron.v1" + "gopkg.in/macaron.v1" ) type fakeRouter struct { @@ -33,7 +33,7 @@ func (fr *fakeRouter) Get(pattern string, handlers ...macaron.Handler) *macaron. } func emptyHandlers(n int) []macaron.Handler { - res := []macaron.Handler{} + var res []macaron.Handler for i := 1; n >= i; i++ { res = append(res, emptyHandler(strconv.Itoa(i))) } @@ -138,7 +138,78 @@ func TestRouteGroupedRegister(t *testing.T) { } } } +func TestRouteGroupInserting(t *testing.T) { + testTable := []route{ + {method: http.MethodGet, pattern: "/api/", handlers: emptyHandlers(1)}, + {method: http.MethodPost, pattern: "/api/group/endpoint", handlers: emptyHandlers(1)}, + {method: http.MethodGet, pattern: "/api/group/inserted", handlers: emptyHandlers(1)}, + {method: http.MethodDelete, pattern: "/api/inserted-endpoint", handlers: emptyHandlers(1)}, + } + + // Setup + rr := NewRouteRegister() + + rr.Group("/api", func(api RouteRegister) { + api.Get("/", emptyHandler("1")) + + api.Group("/group", func(group RouteRegister) { + group.Post("/endpoint", emptyHandler("1")) + }) + }) + + rr.Insert("/api", func(api RouteRegister) { + api.Delete("/inserted-endpoint", emptyHandler("1")) + }) + + rr.Insert("/api/group", func(group RouteRegister) { + group.Get("/inserted", emptyHandler("1")) + }) + + fr := &fakeRouter{} + rr.Register(fr) + + // Validation + if len(fr.route) != len(testTable) { + t.Fatalf("want %v routes, got %v", len(testTable), len(fr.route)) + } + + for i := range testTable { + if testTable[i].method != fr.route[i].method { + t.Errorf("want %s got %v", testTable[i].method, fr.route[i].method) + } + + if testTable[i].pattern != fr.route[i].pattern { + t.Errorf("want %s got %v", testTable[i].pattern, fr.route[i].pattern) + } + + if len(testTable[i].handlers) != len(fr.route[i].handlers) { + t.Errorf("want %d handlers got %d handlers \ntestcase: %v\nroute: %v\n", + len(testTable[i].handlers), + len(fr.route[i].handlers), + testTable[i], + fr.route[i]) + } + } +} + +func TestDuplicateRoutShouldPanic(t *testing.T) { + defer func() { + if recover() != "cannot add duplicate route" { + t.Errorf("Should cause panic if duplicate routes are added ") + } + }() + + rr := NewRouteRegister(func(name string) macaron.Handler { + return emptyHandler(name) + }) + + rr.Get("/api", emptyHandler("1")) + rr.Get("/api", emptyHandler("1")) + + fr := &fakeRouter{} + rr.Register(fr) +} func TestNamedMiddlewareRouteRegister(t *testing.T) { testTable := []route{ {method: "DELETE", pattern: "/admin", handlers: emptyHandlers(2)}, diff --git a/pkg/cmd/grafana-server/server.go b/pkg/cmd/grafana-server/server.go index 4abdb3fb442..6444528f7f0 100644 --- a/pkg/cmd/grafana-server/server.go +++ b/pkg/cmd/grafana-server/server.go @@ -12,6 +12,7 @@ import ( "time" "github.com/facebookgo/inject" + "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/middleware" "github.com/grafana/grafana/pkg/registry" @@ -61,8 +62,8 @@ type GrafanaServerImpl struct { shutdownReason string shutdownInProgress bool - RouteRegister api.RouteRegister `inject:""` - HttpServer *api.HTTPServer `inject:""` + RouteRegister routing.RouteRegister `inject:""` + HttpServer *api.HTTPServer `inject:""` } func (g *GrafanaServerImpl) Run() error { @@ -75,7 +76,7 @@ func (g *GrafanaServerImpl) Run() error { serviceGraph := inject.Graph{} serviceGraph.Provide(&inject.Object{Value: bus.GetBus()}) serviceGraph.Provide(&inject.Object{Value: g.cfg}) - serviceGraph.Provide(&inject.Object{Value: api.NewRouteRegister(middleware.RequestMetrics, middleware.RequestTracing)}) + serviceGraph.Provide(&inject.Object{Value: routing.NewRouteRegister(middleware.RequestMetrics, middleware.RequestTracing)}) // self registered services services := registry.GetServices() diff --git a/pkg/services/annotations/annotations.go b/pkg/services/annotations/annotations.go index 5cebb3d2df9..9b490169d3b 100644 --- a/pkg/services/annotations/annotations.go +++ b/pkg/services/annotations/annotations.go @@ -35,11 +35,12 @@ type PostParams struct { } type DeleteParams struct { - Id int64 `json:"id"` - AlertId int64 `json:"alertId"` - DashboardId int64 `json:"dashboardId"` - PanelId int64 `json:"panelId"` - RegionId int64 `json:"regionId"` + OrgId int64 + Id int64 + AlertId int64 + DashboardId int64 + PanelId int64 + RegionId int64 } var repositoryInstance Repository diff --git a/pkg/services/rendering/interface.go b/pkg/services/rendering/interface.go index 9498071f264..85c139cfc04 100644 --- a/pkg/services/rendering/interface.go +++ b/pkg/services/rendering/interface.go @@ -10,6 +10,7 @@ import ( var ErrTimeout = errors.New("Timeout error. You can set timeout in seconds with &timeout url parameter") var ErrNoRenderer = errors.New("No renderer plugin found nor is an external render server configured") +var ErrPhantomJSNotInstalled = errors.New("PhantomJS executable not found") type Opts struct { Width int diff --git a/pkg/services/rendering/phantomjs.go b/pkg/services/rendering/phantomjs.go index d4ceac0ed43..8e06b5fed9d 100644 --- a/pkg/services/rendering/phantomjs.go +++ b/pkg/services/rendering/phantomjs.go @@ -24,6 +24,11 @@ func (rs *RenderingService) renderViaPhantomJS(ctx context.Context, opts Opts) ( url := rs.getURL(opts.Path) binPath, _ := filepath.Abs(filepath.Join(rs.Cfg.PhantomDir, executable)) + if _, err := os.Stat(binPath); os.IsNotExist(err) { + rs.log.Error("executable not found", "executable", binPath) + return nil, ErrPhantomJSNotInstalled + } + scriptPath, _ := filepath.Abs(filepath.Join(rs.Cfg.PhantomDir, "render.js")) pngPath := rs.getFilePathForNewImage() diff --git a/pkg/services/sqlstore/annotation.go b/pkg/services/sqlstore/annotation.go index 52da7a99516..a65bc136554 100644 --- a/pkg/services/sqlstore/annotation.go +++ b/pkg/services/sqlstore/annotation.go @@ -238,18 +238,19 @@ func (r *SqlAnnotationRepo) Delete(params *annotations.DeleteParams) error { queryParams []interface{} ) + sqlog.Info("delete", "orgId", params.OrgId) if params.RegionId != 0 { - annoTagSql = "DELETE FROM annotation_tag WHERE annotation_id IN (SELECT id FROM annotation WHERE region_id = ?)" - sql = "DELETE FROM annotation WHERE region_id = ?" - queryParams = []interface{}{params.RegionId} + annoTagSql = "DELETE FROM annotation_tag WHERE annotation_id IN (SELECT id FROM annotation WHERE region_id = ? AND org_id = ?)" + sql = "DELETE FROM annotation WHERE region_id = ? AND org_id = ?" + queryParams = []interface{}{params.RegionId, params.OrgId} } else if params.Id != 0 { - annoTagSql = "DELETE FROM annotation_tag WHERE annotation_id IN (SELECT id FROM annotation WHERE id = ?)" - sql = "DELETE FROM annotation WHERE id = ?" - queryParams = []interface{}{params.Id} + annoTagSql = "DELETE FROM annotation_tag WHERE annotation_id IN (SELECT id FROM annotation WHERE id = ? AND org_id = ?)" + sql = "DELETE FROM annotation WHERE id = ? AND org_id = ?" + queryParams = []interface{}{params.Id, params.OrgId} } else { - annoTagSql = "DELETE FROM annotation_tag WHERE annotation_id IN (SELECT id FROM annotation WHERE dashboard_id = ? AND panel_id = ?)" - sql = "DELETE FROM annotation WHERE dashboard_id = ? AND panel_id = ?" - queryParams = []interface{}{params.DashboardId, params.PanelId} + annoTagSql = "DELETE FROM annotation_tag WHERE annotation_id IN (SELECT id FROM annotation WHERE dashboard_id = ? AND panel_id = ? AND org_id = ?)" + sql = "DELETE FROM annotation WHERE dashboard_id = ? AND panel_id = ? AND org_id = ?" + queryParams = []interface{}{params.DashboardId, params.PanelId, params.OrgId} } if _, err := sess.Exec(annoTagSql, queryParams...); err != nil { diff --git a/pkg/services/sqlstore/annotation_test.go b/pkg/services/sqlstore/annotation_test.go index 01a95c7db7b..c0d267f2578 100644 --- a/pkg/services/sqlstore/annotation_test.go +++ b/pkg/services/sqlstore/annotation_test.go @@ -268,7 +268,7 @@ func TestAnnotations(t *testing.T) { annotationId := items[0].Id - err = repo.Delete(&annotations.DeleteParams{Id: annotationId}) + err = repo.Delete(&annotations.DeleteParams{Id: annotationId, OrgId: 1}) So(err, ShouldBeNil) items, err = repo.Find(query) diff --git a/public/app/core/components/search/search_results.ts b/public/app/core/components/search/search_results.ts index 273af224660..35ee1365e22 100644 --- a/public/app/core/components/search/search_results.ts +++ b/public/app/core/components/search/search_results.ts @@ -63,7 +63,8 @@ export class SearchResultsCtrl { } onItemClick(item) { - if (this.$location.path().indexOf(item.url) > -1) { + //Check if one string can be found in the other + if (this.$location.path().indexOf(item.url) > -1 || item.url.indexOf(this.$location.path()) > -1) { appEvents.emit('hide-dash-search'); } } diff --git a/public/app/features/plugins/datasource_srv.ts b/public/app/features/plugins/datasource_srv.ts index aef43a4760b..19bdf599ce1 100644 --- a/public/app/features/plugins/datasource_srv.ts +++ b/public/app/features/plugins/datasource_srv.ts @@ -7,7 +7,7 @@ export class DatasourceSrv { datasources: any; /** @ngInject */ - constructor(private $q, private $injector, $rootScope, private templateSrv) { + constructor(private $q, private $injector, private $rootScope, private templateSrv) { this.init(); } @@ -61,7 +61,7 @@ export class DatasourceSrv { this.datasources[name] = instance; deferred.resolve(instance); }) - .catch(function(err) { + .catch(err => { this.$rootScope.appEvent('alert-error', [dsConfig.name + ' plugin failed', err.toString()]); }); diff --git a/scripts/webpack/webpack.dev.js b/scripts/webpack/webpack.dev.js index 7e43e8179ac..22df4fa3adf 100644 --- a/scripts/webpack/webpack.dev.js +++ b/scripts/webpack/webpack.dev.js @@ -65,7 +65,7 @@ module.exports = merge(common, { }, plugins: [ - new CleanWebpackPlugin('../public/build', { allowExternal: true }), + new CleanWebpackPlugin('../../public/build', { allowExternal: true }), extractSass, new HtmlWebpackPlugin({ filename: path.resolve(__dirname, '../../public/views/index.html'),