From 89923bf77a25cb7b0fbc6473e4afb2e4c3021925 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 19 Oct 2017 17:28:54 +0200 Subject: [PATCH] datasources: change to optimisic concurrency prerequisite for #9504 --- pkg/api/datasources.go | 26 ++++-- pkg/api/dtos/datasource.go | 59 ++++++++++++ pkg/api/dtos/models.go | 50 ----------- pkg/models/datasource.go | 13 +-- pkg/services/sqlstore/datasource.go | 20 ++++- pkg/services/sqlstore/datasource_test.go | 90 +++++++++++++++++-- .../sqlstore/migrations/datasource_mig.go | 6 ++ public/app/features/plugins/ds_edit_ctrl.ts | 4 +- 8 files changed, 196 insertions(+), 72 deletions(-) create mode 100644 pkg/api/dtos/datasource.go diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index 9ffdc4a6d1b..7c509499981 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -119,7 +119,13 @@ func AddDataSource(c *middleware.Context, cmd m.AddDataSourceCommand) { return } - c.JSON(200, util.DynMap{"message": "Datasource added", "id": cmd.Result.Id, "name": cmd.Result.Name}) + ds := convertModelToDtos(cmd.Result) + c.JSON(200, util.DynMap{ + "message": "Datasource added", + "id": cmd.Result.Id, + "name": cmd.Result.Name, + "datasource": ds, + }) } func UpdateDataSource(c *middleware.Context, cmd m.UpdateDataSourceCommand) Response { @@ -133,10 +139,19 @@ func UpdateDataSource(c *middleware.Context, cmd m.UpdateDataSourceCommand) Resp err = bus.Dispatch(&cmd) if err != nil { - return ApiError(500, "Failed to update datasource", err) + if err == m.ErrDataSouceUpdatingOldVersion { + return ApiError(500, "Failed to update datasource. Reload new version and try again", err) + } else { + return ApiError(500, "Failed to update datasource", err) + } } - - return Json(200, util.DynMap{"message": "Datasource updated", "id": cmd.Id, "name": cmd.Name}) + ds := convertModelToDtos(cmd.Result) + return Json(200, util.DynMap{ + "message": "Datasource updated", + "id": cmd.Id, + "name": cmd.Name, + "datasource": ds, + }) } func fillWithSecureJsonData(cmd *m.UpdateDataSourceCommand) error { @@ -158,8 +173,6 @@ func fillWithSecureJsonData(cmd *m.UpdateDataSourceCommand) error { } } - // set version from db - cmd.Version = ds.Version return nil } @@ -228,6 +241,7 @@ func convertModelToDtos(ds *m.DataSource) dtos.DataSource { IsDefault: ds.IsDefault, JsonData: ds.JsonData, SecureJsonFields: map[string]bool{}, + Version: ds.Version, } for k, v := range ds.SecureJsonData { diff --git a/pkg/api/dtos/datasource.go b/pkg/api/dtos/datasource.go new file mode 100644 index 00000000000..7cb36e61ab4 --- /dev/null +++ b/pkg/api/dtos/datasource.go @@ -0,0 +1,59 @@ +package dtos + +import ( + "strings" + + "github.com/grafana/grafana/pkg/components/simplejson" + m "github.com/grafana/grafana/pkg/models" +) + +type DataSource struct { + Id int64 `json:"id"` + OrgId int64 `json:"orgId"` + Name string `json:"name"` + Type string `json:"type"` + TypeLogoUrl string `json:"typeLogoUrl"` + Access m.DsAccess `json:"access"` + Url string `json:"url"` + Password string `json:"password"` + User string `json:"user"` + Database string `json:"database"` + BasicAuth bool `json:"basicAuth"` + BasicAuthUser string `json:"basicAuthUser"` + BasicAuthPassword string `json:"basicAuthPassword"` + WithCredentials bool `json:"withCredentials"` + IsDefault bool `json:"isDefault"` + JsonData *simplejson.Json `json:"jsonData,omitempty"` + SecureJsonFields map[string]bool `json:"secureJsonFields"` + Version int `json:"version"` +} + +type DataSourceListItemDTO struct { + Id int64 `json:"id"` + OrgId int64 `json:"orgId"` + Name string `json:"name"` + Type string `json:"type"` + TypeLogoUrl string `json:"typeLogoUrl"` + Access m.DsAccess `json:"access"` + Url string `json:"url"` + Password string `json:"password"` + User string `json:"user"` + Database string `json:"database"` + BasicAuth bool `json:"basicAuth"` + IsDefault bool `json:"isDefault"` + JsonData *simplejson.Json `json:"jsonData,omitempty"` +} + +type DataSourceList []DataSourceListItemDTO + +func (slice DataSourceList) Len() int { + return len(slice) +} + +func (slice DataSourceList) Less(i, j int) bool { + return strings.ToLower(slice[i].Name) < strings.ToLower(slice[j].Name) +} + +func (slice DataSourceList) Swap(i, j int) { + slice[i], slice[j] = slice[j], slice[i] +} diff --git a/pkg/api/dtos/models.go b/pkg/api/dtos/models.go index d1c346e9539..e2aa12249aa 100644 --- a/pkg/api/dtos/models.go +++ b/pkg/api/dtos/models.go @@ -37,56 +37,6 @@ type CurrentUser struct { HelpFlags1 m.HelpFlags1 `json:"helpFlags1"` } -type DataSource struct { - Id int64 `json:"id"` - OrgId int64 `json:"orgId"` - Name string `json:"name"` - Type string `json:"type"` - TypeLogoUrl string `json:"typeLogoUrl"` - Access m.DsAccess `json:"access"` - Url string `json:"url"` - Password string `json:"password"` - User string `json:"user"` - Database string `json:"database"` - BasicAuth bool `json:"basicAuth"` - BasicAuthUser string `json:"basicAuthUser"` - BasicAuthPassword string `json:"basicAuthPassword"` - WithCredentials bool `json:"withCredentials"` - IsDefault bool `json:"isDefault"` - JsonData *simplejson.Json `json:"jsonData,omitempty"` - SecureJsonFields map[string]bool `json:"secureJsonFields"` -} - -type DataSourceListItemDTO struct { - Id int64 `json:"id"` - OrgId int64 `json:"orgId"` - Name string `json:"name"` - Type string `json:"type"` - TypeLogoUrl string `json:"typeLogoUrl"` - Access m.DsAccess `json:"access"` - Url string `json:"url"` - Password string `json:"password"` - User string `json:"user"` - Database string `json:"database"` - BasicAuth bool `json:"basicAuth"` - IsDefault bool `json:"isDefault"` - JsonData *simplejson.Json `json:"jsonData,omitempty"` -} - -type DataSourceList []DataSourceListItemDTO - -func (slice DataSourceList) Len() int { - return len(slice) -} - -func (slice DataSourceList) Less(i, j int) bool { - return strings.ToLower(slice[i].Name) < strings.ToLower(slice[j].Name) -} - -func (slice DataSourceList) Swap(i, j int) { - slice[i], slice[j] = slice[j], slice[i] -} - type MetricRequest struct { From string `json:"from"` To string `json:"to"` diff --git a/pkg/models/datasource.go b/pkg/models/datasource.go index ee365582734..f600da75a63 100644 --- a/pkg/models/datasource.go +++ b/pkg/models/datasource.go @@ -25,8 +25,9 @@ const ( // Typed errors var ( - ErrDataSourceNotFound = errors.New("Data source not found") - ErrDataSourceNameExists = errors.New("Data source with same name already exists") + ErrDataSourceNotFound = errors.New("Data source not found") + ErrDataSourceNameExists = errors.New("Data source with same name already exists") + ErrDataSouceUpdatingOldVersion = errors.New("Trying to update old version of datasouce") ) type DsAccess string @@ -131,10 +132,12 @@ type UpdateDataSourceCommand struct { IsDefault bool `json:"isDefault"` JsonData *simplejson.Json `json:"jsonData"` SecureJsonData map[string]string `json:"secureJsonData"` + Version int `json:"version"` - OrgId int64 `json:"-"` - Id int64 `json:"-"` - Version int `json:"-"` + OrgId int64 `json:"-"` + Id int64 `json:"-"` + + Result *DataSource } type DeleteDataSourceByIdCommand struct { diff --git a/pkg/services/sqlstore/datasource.go b/pkg/services/sqlstore/datasource.go index 88dc34fd196..3b4bef61a1d 100644 --- a/pkg/services/sqlstore/datasource.go +++ b/pkg/services/sqlstore/datasource.go @@ -3,6 +3,8 @@ package sqlstore import ( "time" + "github.com/go-xorm/xorm" + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/securejsondata" "github.com/grafana/grafana/pkg/metrics" @@ -69,7 +71,6 @@ func DeleteDataSourceByName(cmd *m.DeleteDataSourceByNameCommand) error { } func AddDataSource(cmd *m.AddDataSourceCommand) error { - return inTransaction(func(sess *DBSession) error { existing := m.DataSource{OrgId: cmd.OrgId, Name: cmd.Name} has, _ := sess.Get(&existing) @@ -96,6 +97,7 @@ func AddDataSource(cmd *m.AddDataSourceCommand) error { SecureJsonData: securejsondata.GetEncryptedJsonData(cmd.SecureJsonData), Created: time.Now(), Updated: time.Now(), + Version: 1, } if _, err := sess.Insert(ds); err != nil { @@ -122,7 +124,6 @@ func updateIsDefaultFlag(ds *m.DataSource, sess *DBSession) error { } func UpdateDataSource(cmd *m.UpdateDataSourceCommand) error { - return inTransaction(func(sess *DBSession) error { ds := &m.DataSource{ Id: cmd.Id, @@ -149,12 +150,25 @@ func UpdateDataSource(cmd *m.UpdateDataSourceCommand) error { sess.UseBool("basic_auth") sess.UseBool("with_credentials") - _, err := sess.Where("id=? and org_id=?", ds.Id, ds.OrgId).Update(ds) + var updateSession *xorm.Session + if cmd.Version != 0 { + updateSession = sess.Where("id=? and org_id=? and (version = ? or version < ?)", ds.Id, ds.OrgId, cmd.Version, cmd.Version) + } else { + updateSession = sess.Where("id=? and org_id=?", ds.Id, ds.OrgId) + } + + affected, err := updateSession.Update(ds) if err != nil { return err } + if affected == 0 { + return m.ErrDataSouceUpdatingOldVersion + } + err = updateIsDefaultFlag(ds, sess) + + cmd.Result = ds return err }) } diff --git a/pkg/services/sqlstore/datasource_test.go b/pkg/services/sqlstore/datasource_test.go index 51bc759fdaf..f7a0ab7c1e2 100644 --- a/pkg/services/sqlstore/datasource_test.go +++ b/pkg/services/sqlstore/datasource_test.go @@ -35,12 +35,9 @@ type Test struct { } func TestDataAccess(t *testing.T) { - Convey("Testing DB", t, func() { InitTestDB(t) - Convey("Can add datasource", func() { - err := AddDataSource(&m.AddDataSourceCommand{ OrgId: 10, Name: "laban", @@ -65,7 +62,6 @@ func TestDataAccess(t *testing.T) { }) Convey("Given a datasource", func() { - err := AddDataSource(&m.AddDataSourceCommand{ OrgId: 10, Name: "nisse", @@ -81,6 +77,89 @@ func TestDataAccess(t *testing.T) { ds := query.Result[0] + Convey(" updated ", func() { + cmd := &m.UpdateDataSourceCommand{ + Id: ds.Id, + OrgId: 10, + Name: "nisse", + Type: m.DS_GRAPHITE, + Access: m.DS_ACCESS_PROXY, + Url: "http://test", + Version: ds.Version, + } + + Convey("with same version as source", func() { + err := UpdateDataSource(cmd) + So(err, ShouldBeNil) + }) + + Convey("when someone else updated between read and update", func() { + query := m.GetDataSourcesQuery{OrgId: 10} + err = GetDataSources(&query) + So(err, ShouldBeNil) + + ds := query.Result[0] + intendedUpdate := &m.UpdateDataSourceCommand{ + Id: ds.Id, + OrgId: 10, + Name: "nisse", + Type: m.DS_GRAPHITE, + Access: m.DS_ACCESS_PROXY, + Url: "http://test", + Version: ds.Version, + } + + updateFromOtherUser := &m.UpdateDataSourceCommand{ + Id: ds.Id, + OrgId: 10, + Name: "nisse", + Type: m.DS_GRAPHITE, + Access: m.DS_ACCESS_PROXY, + Url: "http://test", + Version: ds.Version, + } + + err := UpdateDataSource(updateFromOtherUser) + So(err, ShouldBeNil) + + err = UpdateDataSource(intendedUpdate) + So(err, ShouldNotBeNil) + }) + + Convey("updating datasource without version", func() { + cmd := &m.UpdateDataSourceCommand{ + Id: ds.Id, + OrgId: 10, + Name: "nisse", + Type: m.DS_GRAPHITE, + Access: m.DS_ACCESS_PROXY, + Url: "http://test", + } + + Convey("should not raise errors", func() { + err := UpdateDataSource(cmd) + So(err, ShouldBeNil) + }) + }) + + Convey("updating datasource without higher version", func() { + cmd := &m.UpdateDataSourceCommand{ + Id: ds.Id, + OrgId: 10, + Name: "nisse", + Type: m.DS_GRAPHITE, + Access: m.DS_ACCESS_PROXY, + Url: "http://test", + Version: 90000, + } + + Convey("should not raise errors", func() { + err := UpdateDataSource(cmd) + So(err, ShouldBeNil) + }) + }) + }) + Convey("Can delete datasource by id", func() { err := DeleteDataSourceById(&m.DeleteDataSourceByIdCommand{Id: ds.Id, OrgId: ds.OrgId}) So(err, ShouldBeNil) @@ -104,9 +183,6 @@ func TestDataAccess(t *testing.T) { GetDataSources(&query) So(len(query.Result), ShouldEqual, 1) }) - }) - }) - } diff --git a/pkg/services/sqlstore/migrations/datasource_mig.go b/pkg/services/sqlstore/migrations/datasource_mig.go index c694b5b7cfb..cc8a7f05177 100644 --- a/pkg/services/sqlstore/migrations/datasource_mig.go +++ b/pkg/services/sqlstore/migrations/datasource_mig.go @@ -120,4 +120,10 @@ func addDataSourceMigration(mg *Migrator) { {Name: "json_data", Type: DB_Text, Nullable: true}, {Name: "secure_json_data", Type: DB_Text, Nullable: true}, })) + + const setVersionToOneWhereZero = `UPDATE data_source SET version = 1 WHERE version = 0` + mg.AddMigration("Update initial version to 1", new(RawSqlMigration). + Sqlite(setVersionToOneWhereZero). + Postgres(setVersionToOneWhereZero). + Mysql(setVersionToOneWhereZero)) } diff --git a/public/app/features/plugins/ds_edit_ctrl.ts b/public/app/features/plugins/ds_edit_ctrl.ts index 0513487dce7..d388783400d 100644 --- a/public/app/features/plugins/ds_edit_ctrl.ts +++ b/public/app/features/plugins/ds_edit_ctrl.ts @@ -150,13 +150,15 @@ export class DataSourceEditCtrl { } if (this.current.id) { - return this.backendSrv.put('/api/datasources/' + this.current.id, this.current).then(() => { + return this.backendSrv.put('/api/datasources/' + this.current.id, this.current).then((result) => { + this.current = result.datasource; this.updateFrontendSettings().then(() => { this.testDatasource(); }); }); } else { return this.backendSrv.post('/api/datasources', this.current).then(result => { + this.current = result.datasource; this.updateFrontendSettings(); datasourceCreated = true;