diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index a5c15b246af..b2094dff7df 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -56,7 +56,7 @@ func ProvideService( features featuremgmt.FeatureToggles, r prometheus.Registerer, ) folder.Service { - store := ProvideStore(db, cfg, features) + store := ProvideStore(db, cfg) srv := &Service{ cfg: cfg, log: log.New("folder-service"), @@ -457,7 +457,7 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) ( // well, but for now we take the UID from the newly created folder. UID: dash.UID, OrgID: cmd.OrgID, - Title: cmd.Title, + Title: dashFolder.Title, Description: cmd.Description, ParentUID: cmd.ParentUID, } @@ -498,7 +498,7 @@ func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) ( if foldr, err = s.store.Update(ctx, folder.UpdateFolderCommand{ UID: cmd.UID, OrgID: cmd.OrgID, - NewTitle: cmd.NewTitle, + NewTitle: &dashFolder.Title, NewDescription: cmd.NewDescription, SignedInUser: user, }); err != nil { diff --git a/pkg/services/folder/folderimpl/folder_test.go b/pkg/services/folder/folderimpl/folder_test.go index 28f2c3d9120..86371a13fbf 100644 --- a/pkg/services/folder/folderimpl/folder_test.go +++ b/pkg/services/folder/folderimpl/folder_test.go @@ -69,7 +69,7 @@ func TestIntegrationFolderService(t *testing.T) { t.Run("Folder service tests", func(t *testing.T) { dashStore := &dashboards.FakeDashboardStore{} db := sqlstore.InitTestDB(t) - nestedFolderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures([]interface{}{"nestedFolders"})) + nestedFolderStore := ProvideStore(db, db.Cfg) folderStore := foldertest.NewFakeFolderStore(t) @@ -219,23 +219,30 @@ func TestIntegrationFolderService(t *testing.T) { dashboardFolder.ID = rand.Int63() dashboardFolder.UID = util.GenerateShortUID() dashboardFolder.OrgID = orgID - f := dashboards.FromDashboard(dashboardFolder) - _, err := service.store.Create(context.Background(), folder.CreateFolderCommand{ + f, err := service.store.Create(context.Background(), folder.CreateFolderCommand{ OrgID: orgID, Title: dashboardFolder.Title, UID: dashboardFolder.UID, SignedInUser: usr, }) require.NoError(t, err) + assert.Equal(t, "Folder", f.Title) dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil) - dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(dashboardFolder, nil) - dashStore.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(dashboardFolder, nil) - - folderStore.On("GetFolderByID", mock.Anything, orgID, dashboardFolder.ID).Return(f, nil) - title := "TEST-Folder" + updatedDashboardFolder := *dashboardFolder + updatedDashboardFolder.Title = title + dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&updatedDashboardFolder, nil) + dashStore.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(&updatedDashboardFolder, nil) + + folderStore.On("GetFolderByID", mock.Anything, orgID, dashboardFolder.ID).Return(&folder.Folder{ + OrgID: orgID, + ID: dashboardFolder.ID, + UID: dashboardFolder.UID, + Title: title, + }, nil) + req := &folder.UpdateFolderCommand{ UID: dashboardFolder.UID, OrgID: orgID, @@ -245,7 +252,7 @@ func TestIntegrationFolderService(t *testing.T) { reqResult, err := service.Update(context.Background(), req) require.NoError(t, err) - require.Equal(t, f, reqResult) + assert.Equal(t, title, reqResult.Title) }) t.Run("When deleting folder by uid should not return access denied error", func(t *testing.T) { @@ -343,7 +350,7 @@ func TestIntegrationNestedFolderService(t *testing.T) { featuresFlagOn := featuremgmt.WithFeatures("nestedFolders") dashStore, err := database.ProvideDashboardStore(db, db.Cfg, featuresFlagOn, tagimpl.ProvideService(db), quotaService) require.NoError(t, err) - nestedFolderStore := ProvideStore(db, db.Cfg, featuresFlagOn) + nestedFolderStore := ProvideStore(db, db.Cfg) b := bus.ProvideBus(tracing.InitializeTracerForTest()) ac := acimpl.ProvideAccessControl(cfg) @@ -459,7 +466,7 @@ func TestIntegrationNestedFolderService(t *testing.T) { featuresFlagOff := featuremgmt.WithFeatures() dashStore, err := database.ProvideDashboardStore(db, db.Cfg, featuresFlagOff, tagimpl.ProvideService(db), quotaService) require.NoError(t, err) - nestedFolderStore := ProvideStore(db, db.Cfg, featuresFlagOff) + nestedFolderStore := ProvideStore(db, db.Cfg) serviceWithFlagOff := &Service{ cfg: cfg, @@ -622,7 +629,7 @@ func TestIntegrationNestedFolderService(t *testing.T) { dashStore, err := database.ProvideDashboardStore(db, db.Cfg, tc.featuresFlag, tagimpl.ProvideService(db), quotaService) require.NoError(t, err) - nestedFolderStore := ProvideStore(db, db.Cfg, tc.featuresFlag) + nestedFolderStore := ProvideStore(db, db.Cfg) tc.service.dashboardStore = dashStore tc.service.store = nestedFolderStore @@ -732,6 +739,70 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) { }) } +func TestFolderServiceDualWrite(t *testing.T) { + g := guardian.New + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) + t.Cleanup(func() { + guardian.New = g + }) + + db := sqlstore.InitTestDB(t) + cfg := setting.NewCfg() + features := featuremgmt.WithFeatures() + nestedFolderStore := ProvideStore(db, cfg) + + dashStore, err := database.ProvideDashboardStore(db, cfg, features, tagimpl.ProvideService(db), "atest.FakeQuotaService{}) + require.NoError(t, err) + + dashboardFolderStore := ProvideDashboardFolderStore(db) + + folderService := &Service{ + cfg: setting.NewCfg(), + store: nestedFolderStore, + db: sqlstore.InitTestDB(t), + dashboardStore: dashStore, + dashboardFolderStore: dashboardFolderStore, + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + log: log.New("test-folder-service"), + accessControl: acimpl.ProvideAccessControl(cfg), + metrics: newFoldersMetrics(nil), + bus: bus.ProvideBus(tracing.InitializeTracerForTest()), + } + + t.Run("When creating a folder it should trim leading and trailing spaces in both dashboard and folder tables", func(t *testing.T) { + f, err := folderService.Create(context.Background(), &folder.CreateFolderCommand{SignedInUser: usr, OrgID: orgID, Title: " my folder "}) + require.NoError(t, err) + + assert.Equal(t, "my folder", f.Title) + + dashFolder, err := dashboardFolderStore.GetFolderByUID(context.Background(), orgID, f.UID) + require.NoError(t, err) + + nestedFolder, err := nestedFolderStore.Get(context.Background(), folder.GetFolderQuery{UID: &f.UID, OrgID: orgID}) + require.NoError(t, err) + + assert.Equal(t, dashFolder.Title, nestedFolder.Title) + }) + + t.Run("When updating a folder it should trim leading and trailing spaces in both dashboard and folder tables", func(t *testing.T) { + f, err := folderService.Create(context.Background(), &folder.CreateFolderCommand{SignedInUser: usr, OrgID: orgID, Title: "my folder 2"}) + require.NoError(t, err) + + f, err = folderService.Update(context.Background(), &folder.UpdateFolderCommand{SignedInUser: usr, OrgID: orgID, UID: f.UID, NewTitle: util.Pointer(" my updated folder 2 "), Version: f.Version}) + require.NoError(t, err) + + assert.Equal(t, "my updated folder 2", f.Title) + + dashFolder, err := dashboardFolderStore.GetFolderByUID(context.Background(), orgID, f.UID) + require.NoError(t, err) + + nestedFolder, err := nestedFolderStore.Get(context.Background(), folder.GetFolderQuery{UID: &f.UID, OrgID: orgID}) + require.NoError(t, err) + + assert.Equal(t, dashFolder.Title, nestedFolder.Title) + }) +} + func TestNestedFolderService(t *testing.T) { t.Run("with feature flag unset", func(t *testing.T) { t.Run("Should create a folder in both dashboard and folders tables", func(t *testing.T) { @@ -1196,7 +1267,7 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) { featuresFlagOn := featuremgmt.WithFeatures("nestedFolders") dashStore, err := database.ProvideDashboardStore(db, db.Cfg, featuresFlagOn, tagimpl.ProvideService(db), quotaService) require.NoError(t, err) - nestedFolderStore := ProvideStore(db, db.Cfg, featuresFlagOn) + nestedFolderStore := ProvideStore(db, db.Cfg) b := bus.ProvideBus(tracing.InitializeTracerForTest()) ac := acimpl.ProvideAccessControl(cfg) diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go index 5f526da5801..c7e8033bff7 100644 --- a/pkg/services/folder/folderimpl/sqlstore.go +++ b/pkg/services/folder/folderimpl/sqlstore.go @@ -11,7 +11,6 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/dashboards" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" @@ -21,14 +20,13 @@ type sqlStore struct { db db.DB log log.Logger cfg *setting.Cfg - fm featuremgmt.FeatureToggles } // sqlStore implements the store interface. var _ store = (*sqlStore)(nil) -func ProvideStore(db db.DB, cfg *setting.Cfg, features featuremgmt.FeatureToggles) *sqlStore { - return &sqlStore{db: db, log: log.New("folder-store"), cfg: cfg, fm: features} +func ProvideStore(db db.DB, cfg *setting.Cfg) *sqlStore { + return &sqlStore{db: db, log: log.New("folder-store"), cfg: cfg} } func (ss *sqlStore) Create(ctx context.Context, cmd folder.CreateFolderCommand) (*folder.Folder, error) { diff --git a/pkg/services/folder/folderimpl/sqlstore_test.go b/pkg/services/folder/folderimpl/sqlstore_test.go index 9b81f92df00..6d2151aec80 100644 --- a/pkg/services/folder/folderimpl/sqlstore_test.go +++ b/pkg/services/folder/folderimpl/sqlstore_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org/orgimpl" @@ -29,7 +28,7 @@ func TestIntegrationCreate(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) + folderStore := ProvideStore(db, db.Cfg) orgID := CreateOrg(t, db) @@ -149,7 +148,7 @@ func TestIntegrationDelete(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) + folderStore := ProvideStore(db, db.Cfg) orgID := CreateOrg(t, db) @@ -196,7 +195,7 @@ func TestIntegrationUpdate(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) + folderStore := ProvideStore(db, db.Cfg) orgID := CreateOrg(t, db) @@ -371,7 +370,7 @@ func TestIntegrationGet(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) + folderStore := ProvideStore(db, db.Cfg) orgID := CreateOrg(t, db) @@ -450,7 +449,7 @@ func TestIntegrationGetParents(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) + folderStore := ProvideStore(db, db.Cfg) orgID := CreateOrg(t, db) @@ -518,7 +517,7 @@ func TestIntegrationGetChildren(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) + folderStore := ProvideStore(db, db.Cfg) orgID := CreateOrg(t, db) @@ -698,7 +697,7 @@ func TestIntegrationGetHeight(t *testing.T) { } db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) + folderStore := ProvideStore(db, db.Cfg) orgID := CreateOrg(t, db) @@ -731,7 +730,7 @@ func TestIntegrationGetFolders(t *testing.T) { foldersNum := 10 db := sqlstore.InitTestDB(t) - folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)) + folderStore := ProvideStore(db, db.Cfg) orgID := CreateOrg(t, db)