Browse Source

retry uid generation

bergquist 8 years ago
parent
commit
58cfb23625

+ 2 - 0
pkg/middleware/dashboard_redirect_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/bus"
 	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/util"
 	. "github.com/smartystreets/goconvey/convey"
 )
 
@@ -19,6 +20,7 @@ func TestMiddlewareDashboardRedirect(t *testing.T) {
 		fakeDash.Id = 1
 		fakeDash.FolderId = 1
 		fakeDash.HasAcl = false
+		fakeDash.Uid = util.GenerateShortUid()
 
 		bus.AddHandler("test", func(query *m.GetDashboardQuery) error {
 			query.Result = fakeDash

+ 1 - 5
pkg/models/dashboards.go

@@ -9,7 +9,6 @@ import (
 	"github.com/gosimple/slug"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/setting"
-	"github.com/grafana/grafana/pkg/util"
 )
 
 // Typed errors
@@ -23,6 +22,7 @@ var (
 	ErrDashboardFolderCannotHaveParent     = errors.New("A Dashboard Folder cannot be added to another folder")
 	ErrDashboardContainsInvalidAlertData   = errors.New("Invalid alert data. Cannot save dashboard")
 	ErrDashboardFailedToUpdateAlertData    = errors.New("Failed to save alert data")
+	ErrDashboardFailedGenerateUniqueUid    = errors.New("Failed to generate unique dashboard uid.")
 )
 
 type UpdatePluginDashboardError struct {
@@ -66,7 +66,6 @@ type Dashboard struct {
 // NewDashboard creates a new dashboard
 func NewDashboard(title string) *Dashboard {
 	dash := &Dashboard{}
-	dash.Uid = util.GenerateShortUid()
 	dash.Data = simplejson.New()
 	dash.Data.Set("title", title)
 	dash.Title = title
@@ -115,9 +114,6 @@ func NewDashboardFromJson(data *simplejson.Json) *Dashboard {
 
 	if uid, err := dash.Data.Get("uid").String(); err == nil {
 		dash.Uid = uid
-	} else {
-		dash.Uid = util.GenerateShortUid()
-		dash.Data.Set("uid", dash.Uid)
 	}
 
 	return dash

+ 44 - 16
pkg/services/sqlstore/dashboard.go

@@ -7,6 +7,7 @@ import (
 	"github.com/grafana/grafana/pkg/metrics"
 	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/search"
+	"github.com/grafana/grafana/pkg/util"
 )
 
 func init() {
@@ -20,6 +21,8 @@ func init() {
 	bus.AddHandler("sql", GetDashboardsByPluginId)
 }
 
+var generateNewUid func() string = util.GenerateShortUid
+
 func SaveDashboard(cmd *m.SaveDashboardCommand) error {
 	return inTransaction(func(sess *DBSession) error {
 		dash := cmd.GetDashboardModel()
@@ -27,7 +30,7 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error {
 		// try get existing dashboard
 		var existing m.Dashboard
 
-		if dash.Id > 0 {
+		if dash.Id != 0 {
 			dashWithIdExists, err := sess.Where("id=? AND org_id=?", dash.Id, dash.OrgId).Get(&existing)
 			if err != nil {
 				return err
@@ -49,27 +52,36 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error {
 			if existing.PluginId != "" && cmd.Overwrite == false {
 				return m.UpdatePluginDashboardError{PluginId: existing.PluginId}
 			}
-		}
+		} else if dash.Uid != "" {
+			var sameUid m.Dashboard
+			sameUidExists, err := sess.Where("org_id=? AND uid=?", dash.OrgId, dash.Uid).Get(&sameUid)
+			if err != nil {
+				return err
+			}
 
-		var sameUid m.Dashboard
-		sameUidExists, err := sess.Where("org_id=? AND uid=?", dash.OrgId, dash.Uid).Get(&sameUid)
-		if err != nil {
-			return err
+			if sameUidExists {
+				// another dashboard with same uid
+				if dash.Id != sameUid.Id {
+					if cmd.Overwrite {
+						dash.Id = sameUid.Id
+						dash.Version = sameUid.Version
+					} else {
+						return m.ErrDashboardWithSameUIDExists
+					}
+				}
+			}
 		}
 
-		if sameUidExists {
-			// another dashboard with same uid
-			if dash.Id != sameUid.Id {
-				if cmd.Overwrite {
-					dash.Id = sameUid.Id
-					dash.Version = sameUid.Version
-				} else {
-					return m.ErrDashboardWithSameUIDExists
-				}
+		if dash.Uid == "" {
+			uid, err := generateNewDashboardUid(sess, dash.OrgId)
+			if err != nil {
+				return err
 			}
+			dash.Uid = uid
+			dash.Data.Set("uid", uid)
 		}
 
-		err = guaranteeDashboardNameIsUniqueInFolder(sess, dash)
+		err := guaranteeDashboardNameIsUniqueInFolder(sess, dash)
 		if err != nil {
 			return err
 		}
@@ -144,6 +156,22 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error {
 		return err
 	})
 }
+func generateNewDashboardUid(sess *DBSession, orgId int64) (string, error) {
+	for i := 0; i < 3; i++ {
+		uid := generateNewUid()
+
+		exists, err := sess.Where("org_id=? AND uid=?", orgId, uid).Get(&m.Dashboard{})
+		if err != nil {
+			return "", err
+		}
+
+		if !exists {
+			return uid, nil
+		}
+	}
+
+	return "", m.ErrDashboardFailedGenerateUniqueUid
+}
 
 func guaranteeDashboardNameIsUniqueInFolder(sess *DBSession, dash *m.Dashboard) error {
 	var sameNameInFolder m.Dashboard

+ 26 - 2
pkg/services/sqlstore/dashboard_test.go

@@ -5,12 +5,12 @@ import (
 	"testing"
 
 	"github.com/go-xorm/xorm"
-	. "github.com/smartystreets/goconvey/convey"
-
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/search"
 	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/util"
+	. "github.com/smartystreets/goconvey/convey"
 )
 
 func TestDashboardDataAccess(t *testing.T) {
@@ -338,6 +338,30 @@ func TestDashboardDataAccess(t *testing.T) {
 				So(err, ShouldBeNil)
 			})
 
+			Convey("Should retry generation of uid once if it fails.", func() {
+				timesCalled := 0
+				generateNewUid = func() string {
+					timesCalled += 1
+					if timesCalled <= 2 {
+						return savedDash.Uid
+					} else {
+						return util.GenerateShortUid()
+					}
+				}
+				cmd := m.SaveDashboardCommand{
+					OrgId: 1,
+					Dashboard: simplejson.NewFromAny(map[string]interface{}{
+						"title": "new dash 12334",
+						"tags":  []interface{}{},
+					}),
+				}
+
+				err := SaveDashboard(&cmd)
+				So(err, ShouldBeNil)
+
+				generateNewUid = util.GenerateShortUid
+			})
+
 			Convey("Should be able to update dashboard and remove folderId", func() {
 				cmd := m.SaveDashboardCommand{
 					OrgId: 1,