diff --git a/pkg/services/datasources/fakes/fake_datasource_service.go b/pkg/services/datasources/fakes/fake_datasource_service.go index c23024b4e5b..4c5fd79173f 100644 --- a/pkg/services/datasources/fakes/fake_datasource_service.go +++ b/pkg/services/datasources/fakes/fake_datasource_service.go @@ -20,9 +20,9 @@ var _ datasources.DataSourceService = &FakeDataSourceService{} func (s *FakeDataSourceService) GetDataSource(ctx context.Context, query *datasources.GetDataSourceQuery) (*datasources.DataSource, error) { for _, dataSource := range s.DataSources { - idMatch := query.ID != 0 && query.ID == dataSource.ID + idMatch := query.ID != 0 && query.ID == dataSource.ID // nolint:staticcheck uidMatch := query.UID != "" && query.UID == dataSource.UID - nameMatch := query.Name != "" && query.Name == dataSource.Name + nameMatch := query.Name != "" && query.Name == dataSource.Name // nolint:staticcheck if idMatch || nameMatch || uidMatch { return dataSource, nil } diff --git a/pkg/services/datasources/models.go b/pkg/services/datasources/models.go index d81e99bdf11..329d936dedb 100644 --- a/pkg/services/datasources/models.go +++ b/pkg/services/datasources/models.go @@ -156,8 +156,8 @@ type AddDataSourceCommand struct { Type string `json:"type" binding:"Required"` Access DsAccess `json:"access" binding:"Required"` URL string `json:"url"` - Database string `json:"database"` User string `json:"user"` + Database string `json:"database"` BasicAuth bool `json:"basicAuth"` BasicAuthUser string `json:"basicAuthUser"` WithCredentials bool `json:"withCredentials"` @@ -166,9 +166,9 @@ type AddDataSourceCommand struct { SecureJsonData map[string]string `json:"secureJsonData"` UID string `json:"uid"` // swagger:ignore - APIVersion string `json:"apiVersion"` + APIVersion string `json:"apiVersion,omitempty"` // swagger:ignore - IsPrunable bool + IsPrunable bool `json:"-"` OrgID int64 `json:"-"` UserID int64 `json:"-"` @@ -191,12 +191,15 @@ type UpdateDataSourceCommand struct { IsDefault bool `json:"isDefault"` JsonData *simplejson.Json `json:"jsonData"` SecureJsonData map[string]string `json:"secureJsonData"` - Version int `json:"version"` UID string `json:"uid"` // swagger:ignore - APIVersion string `json:"apiVersion"` + APIVersion string `json:"apiVersion,omitempty"` // swagger:ignore - IsPrunable bool + IsPrunable bool `json:"-"` + // Everything above is identical in AddDataSourceCommand + + // The previous version -- used for optimistic locking + Version int `json:"version"` OrgID int64 `json:"-"` ID int64 `json:"-"` @@ -251,10 +254,16 @@ type GetDataSourcesByTypeQuery struct { // GetDataSourceQuery will get a DataSource based on OrgID as well as the UID (preferred), ID, or Name. // At least one of the UID, ID, or Name properties must be set in addition to OrgID. type GetDataSourceQuery struct { - ID int64 - UID string + // Deprecated: use UID + ID int64 + + // The datasource unique id + UID string + + // Deprecated: Use UID Name string + // Required OrgID int64 } diff --git a/pkg/services/datasources/service/datasource.go b/pkg/services/datasources/service/datasource.go index bbaa4ab0822..f5cbe96225b 100644 --- a/pkg/services/datasources/service/datasource.go +++ b/pkg/services/datasources/service/datasource.go @@ -481,12 +481,15 @@ func (s *Service) UpdateDataSource(ctx context.Context, cmd *datasources.UpdateD query := &datasources.GetDataSourceQuery{ ID: cmd.ID, + UID: cmd.UID, OrgID: cmd.OrgID, } dataSource, err = s.SQLStore.GetDataSource(ctx, query) if err != nil { return err } + cmd.UID = dataSource.UID + cmd.ID = dataSource.ID // Validate the command jd, err := cmd.JsonData.ToDB() diff --git a/pkg/services/datasources/service/datasource_test.go b/pkg/services/datasources/service/datasource_test.go index 138b88bfa7b..a21854821f2 100644 --- a/pkg/services/datasources/service/datasource_test.go +++ b/pkg/services/datasources/service/datasource_test.go @@ -12,13 +12,13 @@ import ( "time" "github.com/google/uuid" - "github.com/grafana/grafana-plugin-sdk-go/backend" - sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "gopkg.in/ini.v1" + "github.com/grafana/grafana-plugin-sdk-go/backend" + sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/httpclient" @@ -40,7 +40,6 @@ import ( secretsmng "github.com/grafana/grafana/pkg/services/secrets/manager" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tests/testsuite" - // testdatasource "github.com/grafana/grafana/pkg/tsdb/grafana-testdata-datasource" ) func TestMain(m *testing.M) { @@ -53,9 +52,9 @@ type dataSourceMockRetriever struct { func (d *dataSourceMockRetriever) GetDataSource(ctx context.Context, query *datasources.GetDataSourceQuery) (*datasources.DataSource, error) { for _, dataSource := range d.res { - idMatch := query.ID != 0 && query.ID == dataSource.ID + idMatch := query.ID != 0 && query.ID == dataSource.ID // nolint:staticcheck uidMatch := query.UID != "" && query.UID == dataSource.UID - nameMatch := query.Name != "" && query.Name == dataSource.Name + nameMatch := query.Name != "" && query.Name == dataSource.Name // nolint:staticcheck if idMatch || nameMatch || uidMatch { return dataSource, nil } @@ -420,6 +419,27 @@ func TestIntegrationService_UpdateDataSource(t *testing.T) { require.NoError(t, err) }) + t.Run("should update with UID", func(t *testing.T) { + dsService := initDSService(t) + + ds, err := dsService.AddDataSource(context.Background(), &datasources.AddDataSourceCommand{ + OrgID: 1, + Name: "test-datasource", + URL: "http://before", + }) + require.NoError(t, err) + + cmd := &datasources.UpdateDataSourceCommand{ + UID: ds.UID, + OrgID: ds.OrgID, + URL: "http://after", + } + + after, err := dsService.UpdateDataSource(context.Background(), cmd) + require.NoError(t, err) + require.Equal(t, "http://after", after.URL) + }) + t.Run("should return error if datasource with same name exist", func(t *testing.T) { dsService := initDSService(t) @@ -759,6 +779,28 @@ func TestIntegrationService_UpdateDataSource(t *testing.T) { require.False(t, ok) require.Nil(t, updatedRules) }) + + t.Run("Should update with UID", func(t *testing.T) { + dsService := initDSService(t) + + ds, err := dsService.AddDataSource(context.Background(), &datasources.AddDataSourceCommand{ + OrgID: 1, + Name: "test-datasource", + Type: "test", + URL: "http://before", + }) + require.NoError(t, err) + + updateCmd := &datasources.UpdateDataSourceCommand{ + UID: ds.UID, + OrgID: ds.OrgID, + URL: "http://after", + } + + updatedDS, err := dsService.UpdateDataSource(context.Background(), updateCmd) + require.NoError(t, err) + require.Equal(t, "http://after", updatedDS.URL) + }) } func TestIntegrationService_DeleteDataSource(t *testing.T) { diff --git a/pkg/services/datasources/service/store.go b/pkg/services/datasources/service/store.go index d60c0a26c3f..d7a4056f165 100644 --- a/pkg/services/datasources/service/store.go +++ b/pkg/services/datasources/service/store.go @@ -61,21 +61,26 @@ func (ss *SqlStore) GetDataSource(ctx context.Context, query *datasources.GetDat } func (ss *SqlStore) getDataSource(_ context.Context, query *datasources.GetDataSourceQuery, sess *db.Session) (*datasources.DataSource, error) { - if query.OrgID == 0 || (query.ID == 0 && len(query.Name) == 0 && len(query.UID) == 0) { + if query.OrgID == 0 || (query.ID == 0 && len(query.Name) == 0 && len(query.UID) == 0) { // nolint:staticcheck return nil, datasources.ErrDataSourceIdentifierNotSet } if len(query.UID) > 0 { if err := util.ValidateUID(query.UID); err != nil { - logDeprecatedInvalidDsUid(ss.logger, query.UID, query.Name, "read", fmt.Errorf("invalid UID")) + logDeprecatedInvalidDsUid(ss.logger, query.UID, query.Name, "read", fmt.Errorf("invalid UID")) // nolint:staticcheck } } - datasource := &datasources.DataSource{Name: query.Name, OrgID: query.OrgID, ID: query.ID, UID: query.UID} + datasource := &datasources.DataSource{ + OrgID: query.OrgID, + UID: query.UID, + Name: query.Name, // nolint:staticcheck + ID: query.ID, // nolint:staticcheck + } has, err := sess.Get(datasource) if err != nil { - ss.logger.Error("Failed getting data source", "err", err, "uid", query.UID, "id", query.ID, "name", query.Name, "orgId", query.OrgID) + ss.logger.Error("Failed getting data source", "err", err, "uid", query.UID, "id", query.ID, "name", query.Name, "orgId", query.OrgID) // nolint:staticcheck return nil, err } else if !has { return nil, datasources.ErrDataSourceNotFound @@ -324,7 +329,11 @@ func (ss *SqlStore) UpdateDataSource(ctx context.Context, cmd *datasources.Updat cmd.JsonData = simplejson.New() } - if cmd.UID != "" { + if cmd.ID == 0 || cmd.OrgID == 0 { + return datasources.ErrDataSourceIdentifierNotSet + } + + if len(cmd.UID) > 0 { if err := util.ValidateUID(cmd.UID); err != nil { logDeprecatedInvalidDsUid(ss.logger, cmd.UID, cmd.Name, "update", err) return datasources.ErrDataSourceUIDInvalid.Errorf("invalid UID for datasource %s: %w", cmd.Name, err) diff --git a/pkg/services/datasources/service/store_test.go b/pkg/services/datasources/service/store_test.go index 2540c2e9f4f..9e8b64b44ec 100644 --- a/pkg/services/datasources/service/store_test.go +++ b/pkg/services/datasources/service/store_test.go @@ -247,8 +247,8 @@ func TestIntegrationDataAccess(t *testing.T) { }) }) - t.Run("DeleteDataSourceById", func(t *testing.T) { - t.Run("can delete datasource", func(t *testing.T) { + t.Run("DeleteDataSource", func(t *testing.T) { + t.Run("can delete datasource with ID", func(t *testing.T) { db := db.InitTestDB(t) ds := initDatasource(db) ss := SqlStore{db: db} @@ -263,6 +263,21 @@ func TestIntegrationDataAccess(t *testing.T) { require.Equal(t, 0, len(dataSources)) }) + t.Run("can delete datasource with UID", func(t *testing.T) { + db := db.InitTestDB(t) + ds := initDatasource(db) + ss := SqlStore{db: db} + + err := ss.DeleteDataSource(context.Background(), &datasources.DeleteDataSourceCommand{UID: ds.UID, OrgID: ds.OrgID}) + require.NoError(t, err) + + query := datasources.GetDataSourcesQuery{OrgID: 10} + dataSources, err := ss.GetDataSources(context.Background(), &query) + require.NoError(t, err) + + require.Equal(t, 0, len(dataSources)) + }) + t.Run("Can not delete datasource with wrong orgID", func(t *testing.T) { db := db.InitTestDB(t) ds := initDatasource(db) diff --git a/pkg/services/provisioning/datasources/config_reader_test.go b/pkg/services/provisioning/datasources/config_reader_test.go index e18f96639de..35083d8d009 100644 --- a/pkg/services/provisioning/datasources/config_reader_test.go +++ b/pkg/services/provisioning/datasources/config_reader_test.go @@ -457,7 +457,7 @@ type spyStore struct { func (s *spyStore) GetDataSource(ctx context.Context, query *datasources.GetDataSourceQuery) (*datasources.DataSource, error) { for _, v := range s.items { - if query.Name == v.Name && query.OrgID == v.OrgID { + if query.Name == v.Name && query.OrgID == v.OrgID { // nolint:staticcheck return v, nil } } diff --git a/public/api-enterprise-spec.json b/public/api-enterprise-spec.json index 6f4adb798a2..d431451f492 100644 --- a/public/api-enterprise-spec.json +++ b/public/api-enterprise-spec.json @@ -5487,6 +5487,7 @@ "format": "int64" }, "id": { + "description": "Deprecated: this field will be removed in the future", "type": "integer", "format": "int64" }, @@ -6128,15 +6129,15 @@ "language": { "type": "string" }, - "locale": { - "type": "string" - }, "navbar": { "$ref": "#/definitions/NavbarPreference" }, "queryHistory": { "$ref": "#/definitions/QueryHistoryPreference" }, + "regionalFormat": { + "type": "string" + }, "theme": { "type": "string", "enum": [ @@ -6397,16 +6398,16 @@ "description": "Selected language (beta)", "type": "string" }, - "locale": { - "description": "Selected locale (beta)", - "type": "string" - }, "navbar": { "$ref": "#/definitions/NavbarPreference" }, "queryHistory": { "$ref": "#/definitions/QueryHistoryPreference" }, + "regionalFormat": { + "description": "Selected locale (beta)", + "type": "string" + }, "theme": { "description": "light, dark, empty is default", "type": "string" @@ -8442,6 +8443,7 @@ "type": "string" }, "version": { + "description": "The previous version -- used for optimistic locking", "type": "integer", "format": "int64" }, @@ -8559,15 +8561,15 @@ "language": { "type": "string" }, - "locale": { - "type": "string" - }, "navbar": { "$ref": "#/definitions/NavbarPreference" }, "queryHistory": { "$ref": "#/definitions/QueryHistoryPreference" }, + "regionalFormat": { + "type": "string" + }, "theme": { "type": "string", "enum": [ diff --git a/public/api-merged.json b/public/api-merged.json index 7c8e7873c42..0706252d551 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -22192,6 +22192,7 @@ "type": "string" }, "version": { + "description": "The previous version -- used for optimistic locking", "type": "integer", "format": "int64" }, diff --git a/public/openapi3.json b/public/openapi3.json index 66de8bb8da4..f44e48250d6 100644 --- a/public/openapi3.json +++ b/public/openapi3.json @@ -12238,6 +12238,7 @@ "type": "string" }, "version": { + "description": "The previous version -- used for optimistic locking", "format": "int64", "type": "integer" },