From 58cfb236250e0d5b3bac72c1d261abcfb5572435 Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 31 Jan 2018 17:27:28 +0100 Subject: [PATCH] retry uid generation --- pkg/middleware/dashboard_redirect_test.go | 2 + pkg/models/dashboards.go | 6 +-- pkg/services/sqlstore/dashboard.go | 62 ++++++++++++++++------- pkg/services/sqlstore/dashboard_test.go | 28 +++++++++- 4 files changed, 74 insertions(+), 24 deletions(-) diff --git a/pkg/middleware/dashboard_redirect_test.go b/pkg/middleware/dashboard_redirect_test.go index bff4ee2253c..21fc12e5e84 100644 --- a/pkg/middleware/dashboard_redirect_test.go +++ b/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 diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index c4361eefd7d..e06bc0d1f0d 100644 --- a/pkg/models/dashboards.go +++ b/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 diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 4c04fe1251a..f93e7f402ae 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/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 + } } } } - err = guaranteeDashboardNameIsUniqueInFolder(sess, dash) + 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) 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 diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 87010849a77..6e240b9585e 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/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,