Merge branch 'master' of https://github.com/grafana/grafana into macros/sql

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

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,
 	})
 

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)
+	})
+}

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)
 	}
 

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)
+			})
+		})
 	})
 }

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 {

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

pkg/api/route_register.go → pkg/api/routing/route_register.go

-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,

pkg/api/route_register_test.go → pkg/api/routing/route_register_test.go

-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)},

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()

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

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

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()
 

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 {

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)

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');
     }
   }

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()]);
       });
 

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'),