diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index 24b15f05adb..043f5c348d3 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "strings" + "time" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/events" @@ -29,14 +30,13 @@ import ( type Service struct { store store - log log.Logger - cfg *setting.Cfg - dashboardService dashboards.DashboardService - dashboardStore dashboards.Store - searchService *search.SearchService - features featuremgmt.FeatureToggles - permissions accesscontrol.FolderPermissionsService - accessControl accesscontrol.AccessControl + log log.Logger + cfg *setting.Cfg + dashboardStore dashboards.Store + searchService *search.SearchService + features featuremgmt.FeatureToggles + permissions accesscontrol.FolderPermissionsService + accessControl accesscontrol.AccessControl // bus is currently used to publish events that cause scheduler to update rules. bus bus.Bus @@ -46,7 +46,6 @@ func ProvideService( ac accesscontrol.AccessControl, bus bus.Bus, cfg *setting.Cfg, - dashboardService dashboards.DashboardService, dashboardStore dashboards.Store, db db.DB, // DB for the (new) nested folder store features featuremgmt.FeatureToggles, @@ -57,16 +56,15 @@ func ProvideService( ac.RegisterScopeAttributeResolver(dashboards.NewFolderIDScopeResolver(dashboardStore)) store := ProvideStore(db, cfg, features) svr := &Service{ - cfg: cfg, - log: log.New("folder-service"), - dashboardService: dashboardService, - dashboardStore: dashboardStore, - store: store, - searchService: searchService, - features: features, - permissions: folderPermissionsService, - accessControl: ac, - bus: bus, + cfg: cfg, + log: log.New("folder-service"), + dashboardStore: dashboardStore, + store: store, + searchService: searchService, + features: features, + permissions: folderPermissionsService, + accessControl: ac, + bus: bus, } if features.IsEnabled(featuremgmt.FlagNestedFolders) { svr.DBMigration(db) @@ -321,7 +319,7 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) ( User: user, } - saveDashboardCmd, err := s.dashboardService.BuildSaveDashboardCommand(ctx, dto, false, false) + saveDashboardCmd, err := s.BuildSaveDashboardCommand(ctx, dto) if err != nil { return nil, toFolderError(err) } @@ -466,7 +464,7 @@ func (s *Service) legacyUpdate(ctx context.Context, cmd *folder.UpdateFolderComm Overwrite: cmd.Overwrite, } - saveDashboardCmd, err := s.dashboardService.BuildSaveDashboardCommand(ctx, dto, false, false) + saveDashboardCmd, err := s.BuildSaveDashboardCommand(ctx, dto) if err != nil { return nil, toFolderError(err) } @@ -648,8 +646,154 @@ func (s *Service) nestedFolderDelete(ctx context.Context, cmd *folder.DeleteFold return nil } +// MakeUserAdmin is copy of DashboardServiceImpl.MakeUserAdmin func (s *Service) MakeUserAdmin(ctx context.Context, orgID int64, userID, folderID int64, setViewAndEditPermissions bool) error { - return s.dashboardService.MakeUserAdmin(ctx, orgID, userID, folderID, setViewAndEditPermissions) + rtEditor := org.RoleEditor + rtViewer := org.RoleViewer + + items := []*models.DashboardACL{ + { + OrgID: orgID, + DashboardID: folderID, + UserID: userID, + Permission: models.PERMISSION_ADMIN, + Created: time.Now(), + Updated: time.Now(), + }, + } + + if setViewAndEditPermissions { + items = append(items, + &models.DashboardACL{ + OrgID: orgID, + DashboardID: folderID, + Role: &rtEditor, + Permission: models.PERMISSION_EDIT, + Created: time.Now(), + Updated: time.Now(), + }, + &models.DashboardACL{ + OrgID: orgID, + DashboardID: folderID, + Role: &rtViewer, + Permission: models.PERMISSION_VIEW, + Created: time.Now(), + Updated: time.Now(), + }, + ) + } + + if err := s.dashboardStore.UpdateDashboardACL(ctx, folderID, items); err != nil { + return err + } + + return nil +} + +// BuildSaveDashboardCommand is a simplified version on DashboardServiceImpl.BuildSaveDashboardCommand +// keeping only the meaningful functionality for folders +func (s *Service) BuildSaveDashboardCommand(ctx context.Context, dto *dashboards.SaveDashboardDTO) (*dashboards.SaveDashboardCommand, error) { + dash := dto.Dashboard + + dash.OrgID = dto.OrgID + dash.Title = strings.TrimSpace(dash.Title) + dash.Data.Set("title", dash.Title) + dash.SetUID(strings.TrimSpace(dash.UID)) + + if dash.Title == "" { + return nil, dashboards.ErrDashboardTitleEmpty + } + + if dash.IsFolder && dash.FolderID > 0 { + return nil, dashboards.ErrDashboardFolderCannotHaveParent + } + + if dash.IsFolder && strings.EqualFold(dash.Title, dashboards.RootFolderName) { + return nil, dashboards.ErrDashboardFolderNameExists + } + + if !util.IsValidShortUID(dash.UID) { + return nil, dashboards.ErrDashboardInvalidUid + } else if util.IsShortUIDTooLong(dash.UID) { + return nil, dashboards.ErrDashboardUidTooLong + } + + isParentFolderChanged, err := s.dashboardStore.ValidateDashboardBeforeSave(ctx, dash, dto.Overwrite) + if err != nil { + return nil, err + } + + if isParentFolderChanged { + // Check that the user is allowed to add a dashboard to the folder + guardian, err := guardian.NewByDashboard(ctx, dash, dto.OrgID, dto.User) + if err != nil { + return nil, err + } + if canSave, err := guardian.CanCreate(dash.FolderID, dash.IsFolder); err != nil || !canSave { + if err != nil { + return nil, err + } + return nil, dashboards.ErrDashboardUpdateAccessDenied + } + } + + guard, err := getGuardianForSavePermissionCheck(ctx, dash, dto.User) + if err != nil { + return nil, err + } + + if dash.ID == 0 { + if canCreate, err := guard.CanCreate(dash.FolderID, dash.IsFolder); err != nil || !canCreate { + if err != nil { + return nil, err + } + return nil, dashboards.ErrDashboardUpdateAccessDenied + } + } else { + if canSave, err := guard.CanSave(); err != nil || !canSave { + if err != nil { + return nil, err + } + return nil, dashboards.ErrDashboardUpdateAccessDenied + } + } + + cmd := &dashboards.SaveDashboardCommand{ + Dashboard: dash.Data, + Message: dto.Message, + OrgID: dto.OrgID, + Overwrite: dto.Overwrite, + UserID: dto.User.UserID, + FolderID: dash.FolderID, + IsFolder: dash.IsFolder, + PluginID: dash.PluginID, + } + + if !dto.UpdatedAt.IsZero() { + cmd.UpdatedAt = dto.UpdatedAt + } + + return cmd, nil +} + +// getGuardianForSavePermissionCheck returns the guardian to be used for checking permission of dashboard +// It replaces deleted Dashboard.GetDashboardIdForSavePermissionCheck() +func getGuardianForSavePermissionCheck(ctx context.Context, d *dashboards.Dashboard, user *user.SignedInUser) (guardian.DashboardGuardian, error) { + newDashboard := d.ID == 0 + + if newDashboard { + // if it's a new dashboard/folder check the parent folder permissions + guard, err := guardian.New(ctx, d.FolderID, d.OrgID, user) + if err != nil { + return nil, err + } + return guard, nil + } + guard, err := guardian.NewByDashboard(ctx, d, d.OrgID, user) + if err != nil { + return nil, err + } + return guard, nil } func (s *Service) nestedFolderCreate(ctx context.Context, cmd *folder.CreateFolderCommand) (*folder.Folder, error) { diff --git a/pkg/services/folder/folderimpl/folder_test.go b/pkg/services/folder/folderimpl/folder_test.go index 9b58dee7b7b..611dfa27d89 100644 --- a/pkg/services/folder/folderimpl/folder_test.go +++ b/pkg/services/folder/folderimpl/folder_test.go @@ -19,7 +19,6 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol/actest" acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/dashboards" - dashboardsvc "github.com/grafana/grafana/pkg/services/dashboards/service" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/guardian" @@ -39,7 +38,7 @@ func TestIntegrationProvideFolderService(t *testing.T) { t.Run("should register scope resolvers", func(t *testing.T) { cfg := setting.NewCfg() ac := acmock.New() - ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), cfg, nil, nil, nil, &featuremgmt.FeatureManager{}, nil, nil) + ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), cfg, nil, nil, &featuremgmt.FeatureManager{}, nil, nil) require.Len(t, ac.Calls.RegisterAttributeScopeResolver, 2) }) @@ -58,19 +57,16 @@ func TestIntegrationFolderService(t *testing.T) { cfg.RBACEnabled = false features := featuremgmt.WithFeatures() folderPermissions := acmock.NewMockedPermissionsService() - dashboardPermissions := acmock.NewMockedPermissionsService() - dashboardService := dashboardsvc.ProvideDashboardService(cfg, dashStore, nil, features, folderPermissions, dashboardPermissions, acmock.New()) service := &Service{ - cfg: cfg, - log: log.New("test-folder-service"), - dashboardService: dashboardService, - dashboardStore: dashStore, - store: store, - searchService: nil, - features: features, - permissions: folderPermissions, - bus: bus.ProvideBus(tracing.InitializeTracerForTest()), + cfg: cfg, + log: log.New("test-folder-service"), + dashboardStore: dashStore, + store: store, + searchService: nil, + features: features, + permissions: folderPermissions, + bus: bus.ProvideBus(tracing.InitializeTracerForTest()), } t.Run("Given user has no permissions", func(t *testing.T) { @@ -320,28 +316,30 @@ func TestIntegrationFolderService(t *testing.T) { } func TestNestedFolderServiceFeatureToggle(t *testing.T) { + g := guardian.New + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) + t.Cleanup(func() { + guardian.New = g + }) + folderStore := NewFakeStore() - dashboardsvc := dashboards.FakeDashboardService{} - dashboardsvc.On("BuildSaveDashboardCommand", - mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"), - mock.AnythingOfType("bool"), mock.AnythingOfType("bool")).Return(&dashboards.SaveDashboardCommand{}, nil) dashStore := dashboards.FakeDashboardStore{} + 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(&dashboards.Dashboard{}, nil) dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil) cfg := setting.NewCfg() cfg.RBACEnabled = false folderService := &Service{ - cfg: cfg, - store: folderStore, - dashboardStore: &dashStore, - dashboardService: &dashboardsvc, - features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), - log: log.New("test-folder-service"), + cfg: cfg, + store: folderStore, + dashboardStore: &dashStore, + features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), + log: log.New("test-folder-service"), } t.Run("create folder", func(t *testing.T) { folderStore.ExpectedFolder = &folder.Folder{ParentUID: util.GenerateShortUID()} - res, err := folderService.Create(context.Background(), &folder.CreateFolderCommand{SignedInUser: usr}) + res, err := folderService.Create(context.Background(), &folder.CreateFolderCommand{SignedInUser: usr, Title: "my folder"}) require.NoError(t, err) require.NotNil(t, res.UID) require.NotEmpty(t, res.ParentUID) @@ -351,19 +349,21 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) { func TestNestedFolderService(t *testing.T) { t.Run("with feature flag unset", func(t *testing.T) { t.Run("When create folder, no create in folder table done", func(t *testing.T) { - // dashboard store & service commands that should be called. - dashboardsvc := dashboards.FakeDashboardService{} - dashboardsvc.On("BuildSaveDashboardCommand", - mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"), - mock.AnythingOfType("bool"), mock.AnythingOfType("bool")).Return(&dashboards.SaveDashboardCommand{}, nil) + g := guardian.New + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) + t.Cleanup(func() { + guardian.New = g + }) + // dashboard store & service commands that should be called. dashStore := &dashboards.FakeDashboardStore{} + 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(&dashboards.Dashboard{}, nil) dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil) folderStore := NewFakeStore() - folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures(), nil, &dashboardsvc) + folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures(), nil) _, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{ OrgID: orgID, Title: "myFolder", @@ -376,8 +376,6 @@ func TestNestedFolderService(t *testing.T) { }) t.Run("When delete folder, no delete in folder table done", func(t *testing.T) { - dashboardsvc := dashboards.FakeDashboardService{} - var actualCmd *dashboards.DeleteDashboardCommand dashStore := &dashboards.FakeDashboardStore{} dashStore.On("DeleteDashboard", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { @@ -385,32 +383,28 @@ func TestNestedFolderService(t *testing.T) { }).Return(nil).Once() dashStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&folder.Folder{}, nil) - g := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) - folderStore := NewFakeStore() - folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures(), nil, &dashboardsvc) + folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures(), nil) err := folderSvc.Delete(context.Background(), &folder.DeleteFolderCommand{UID: "myFolder", OrgID: orgID, SignedInUser: usr}) require.NoError(t, err) require.NotNil(t, actualCmd) - t.Cleanup(func() { - guardian.New = g - }) require.False(t, folderStore.DeleteCalled) }) }) t.Run("with nested folder feature flag on", func(t *testing.T) { t.Run("create, no error", func(t *testing.T) { - dashboardsvc := dashboards.FakeDashboardService{} - // dashboard store & service commands that should be called. - dashboardsvc.On("BuildSaveDashboardCommand", - mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"), - mock.AnythingOfType("bool"), mock.AnythingOfType("bool")).Return(&dashboards.SaveDashboardCommand{}, nil) + g := guardian.New + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) + t.Cleanup(func() { + guardian.New = g + }) + // dashboard store commands that should be called. dashStore := &dashboards.FakeDashboardStore{} + 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(&dashboards.Dashboard{}, nil) dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil) @@ -418,7 +412,7 @@ func TestNestedFolderService(t *testing.T) { folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{ ExpectedEvaluate: true, - }, &dashboardsvc) + }) _, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{ OrgID: orgID, Title: "myFolder", @@ -431,13 +425,15 @@ func TestNestedFolderService(t *testing.T) { }) t.Run("create without UID, no error", func(t *testing.T) { - // dashboard store & service commands that should be called. - dashboardsvc := dashboards.FakeDashboardService{} - dashboardsvc.On("BuildSaveDashboardCommand", - mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"), - mock.AnythingOfType("bool"), mock.AnythingOfType("bool")).Return(&dashboards.SaveDashboardCommand{}, nil) + g := guardian.New + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) + t.Cleanup(func() { + guardian.New = g + }) + // dashboard store commands that should be called. dashStore := &dashboards.FakeDashboardStore{} + 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(&dashboards.Dashboard{UID: "newUID"}, nil) dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil) @@ -445,7 +441,7 @@ func TestNestedFolderService(t *testing.T) { folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{ ExpectedEvaluate: true, - }, &dashboardsvc) + }) f, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{ OrgID: orgID, Title: "myFolder", @@ -458,17 +454,18 @@ func TestNestedFolderService(t *testing.T) { }) t.Run("create failed because of circular reference", func(t *testing.T) { - // dashboard store & service commands that should be called. + g := guardian.New + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) + t.Cleanup(func() { + guardian.New = g + }) + dashboardFolder := dashboards.NewDashboardFolder("myFolder") dashboardFolder.ID = rand.Int63() dashboardFolder.UID = "myFolder" f := dashboards.FromDashboard(dashboardFolder) - dashboardsvc := dashboards.FakeDashboardService{} - dashboardsvc.On("BuildSaveDashboardCommand", - mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"), - mock.AnythingOfType("bool"), mock.AnythingOfType("bool")).Return(&dashboards.SaveDashboardCommand{}, nil) - + // dashboard store commands that should be called. dashStore := &dashboards.FakeDashboardStore{} 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) @@ -496,7 +493,7 @@ func TestNestedFolderService(t *testing.T) { folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{ ExpectedEvaluate: true, - }, &dashboardsvc) + }) _, err := folderSvc.Create(context.Background(), &cmd) require.Error(t, err, folder.ErrCircularReference) // CreateFolder should not call the folder store's create method. @@ -507,15 +504,14 @@ func TestNestedFolderService(t *testing.T) { t.Run("create returns error from nested folder service", func(t *testing.T) { // This test creates and deletes the dashboard, so needs some extra setup. g := guardian.New - guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{}) - - // dashboard store & service commands that should be called. - dashboardsvc := dashboards.FakeDashboardService{} - dashboardsvc.On("BuildSaveDashboardCommand", - mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"), - mock.AnythingOfType("bool"), mock.AnythingOfType("bool")).Return(&dashboards.SaveDashboardCommand{}, nil) + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true}) + t.Cleanup(func() { + guardian.New = g + }) + // dashboard store commands that should be called. dashStore := &dashboards.FakeDashboardStore{} + 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(&dashboards.Dashboard{}, nil) dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil) dashStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&folder.Folder{}, nil) @@ -531,7 +527,7 @@ func TestNestedFolderService(t *testing.T) { // the service return success as long as the legacy create succeeds folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{ ExpectedEvaluate: true, - }, &dashboardsvc) + }) _, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{ OrgID: orgID, Title: "myFolder", @@ -543,10 +539,6 @@ func TestNestedFolderService(t *testing.T) { // CreateFolder should also call the folder store's create method. require.True(t, folderStore.CreateCalled) require.NotNil(t, actualCmd) - - t.Cleanup(func() { - guardian.New = g - }) }) t.Run("move, no view permission should fail", func(t *testing.T) { @@ -557,7 +549,6 @@ func TestNestedFolderService(t *testing.T) { guardian.New = g }) - dashboardsvc := dashboards.FakeDashboardService{} dashStore := &dashboards.FakeDashboardStore{} folderStore := NewFakeStore() @@ -565,7 +556,7 @@ func TestNestedFolderService(t *testing.T) { folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{ ExpectedEvaluate: true, - }, &dashboardsvc) + }) _, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: usr}) require.Error(t, err, dashboards.ErrFolderAccessDenied) }) @@ -578,7 +569,6 @@ func TestNestedFolderService(t *testing.T) { guardian.New = g }) - dashboardsvc := dashboards.FakeDashboardService{} dashStore := &dashboards.FakeDashboardStore{} folderStore := NewFakeStore() @@ -586,7 +576,7 @@ func TestNestedFolderService(t *testing.T) { folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{ ExpectedEvaluate: true, - }, &dashboardsvc) + }) _, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: usr}) require.Error(t, err, dashboards.ErrFolderAccessDenied) }) @@ -599,7 +589,6 @@ func TestNestedFolderService(t *testing.T) { guardian.New = g }) - dashboardsvc := dashboards.FakeDashboardService{} dashStore := &dashboards.FakeDashboardStore{} folderStore := NewFakeStore() @@ -612,7 +601,7 @@ func TestNestedFolderService(t *testing.T) { folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{ ExpectedEvaluate: true, - }, &dashboardsvc) + }) f, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: usr}) require.NoError(t, err) require.NotNil(t, f) @@ -625,7 +614,6 @@ func TestNestedFolderService(t *testing.T) { guardian.New = g }) - dashboardsvc := dashboards.FakeDashboardService{} dashStore := &dashboards.FakeDashboardStore{} folderStore := NewFakeStore() @@ -634,7 +622,7 @@ func TestNestedFolderService(t *testing.T) { folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{ ExpectedEvaluate: true, - }, &dashboardsvc) + }) f, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder", OrgID: orgID, SignedInUser: usr}) require.Error(t, err, folder.ErrCircularReference) require.Nil(t, f) @@ -647,7 +635,6 @@ func TestNestedFolderService(t *testing.T) { guardian.New = g }) - dashboardsvc := dashboards.FakeDashboardService{} dashStore := &dashboards.FakeDashboardStore{} folderStore := NewFakeStore() @@ -660,7 +647,7 @@ func TestNestedFolderService(t *testing.T) { folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{ ExpectedEvaluate: true, - }, &dashboardsvc) + }) f, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder2", OrgID: orgID, SignedInUser: usr}) require.Error(t, err, folder.ErrMaximumDepthReached) require.Nil(t, f) @@ -673,7 +660,6 @@ func TestNestedFolderService(t *testing.T) { guardian.New = g }) - dashboardsvc := dashboards.FakeDashboardService{} dashStore := &dashboards.FakeDashboardStore{} folderStore := NewFakeStore() @@ -682,7 +668,7 @@ func TestNestedFolderService(t *testing.T) { folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{ ExpectedEvaluate: true, - }, &dashboardsvc) + }) f, err := folderSvc.Move(context.Background(), &folder.MoveFolderCommand{UID: "myFolder", NewParentUID: "newFolder2", OrgID: orgID, SignedInUser: usr}) require.Error(t, err, folder.ErrCircularReference) require.Nil(t, f) @@ -695,8 +681,6 @@ func TestNestedFolderService(t *testing.T) { guardian.New = g }) - dashboardsvc := dashboards.FakeDashboardService{} - dashStore := &dashboards.FakeDashboardStore{} var actualCmd *dashboards.DeleteDashboardCommand dashStore.On("DeleteDashboard", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { @@ -709,7 +693,7 @@ func TestNestedFolderService(t *testing.T) { folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{ ExpectedEvaluate: true, - }, &dashboardsvc) + }) err := folderSvc.Delete(context.Background(), &folder.DeleteFolderCommand{UID: "myFolder", OrgID: orgID, SignedInUser: usr}) require.NoError(t, err) require.NotNil(t, actualCmd) @@ -725,13 +709,9 @@ func TestNestedFolderService(t *testing.T) { guardian.New = g }) - dashboardsvc := dashboards.FakeDashboardService{} - + // dashboard store commands that should be called. dashStore := &dashboards.FakeDashboardStore{} - // dashboard store & service commands that should be called. - dashboardsvc.On("BuildSaveDashboardCommand", - mock.Anything, mock.AnythingOfType("*dashboards.SaveDashboardDTO"), - mock.AnythingOfType("bool"), mock.AnythingOfType("bool")).Return(&dashboards.SaveDashboardCommand{}, nil) + dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil).Times(2) dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{}, nil) dashStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil) dashStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&folder.Folder{}, nil) @@ -751,7 +731,7 @@ func TestNestedFolderService(t *testing.T) { folderSvc := setup(t, dashStore, folderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{ ExpectedEvaluate: true, - }, &dashboardsvc) + }) _, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{ Title: "folder", OrgID: orgID, @@ -765,19 +745,18 @@ func TestNestedFolderService(t *testing.T) { }) } -func setup(t *testing.T, dashStore dashboards.Store, folderStore store, features featuremgmt.FeatureToggles, ac accesscontrol.AccessControl, dashboardService dashboards.DashboardService) folder.Service { +func setup(t *testing.T, dashStore dashboards.Store, folderStore store, features featuremgmt.FeatureToggles, ac accesscontrol.AccessControl) folder.Service { t.Helper() // nothing enabled yet cfg := setting.NewCfg() cfg.RBACEnabled = false return &Service{ - cfg: cfg, - log: log.New("test-folder-service"), - dashboardService: dashboardService, - dashboardStore: dashStore, - store: folderStore, - features: features, - accessControl: ac, + cfg: cfg, + log: log.New("test-folder-service"), + dashboardStore: dashStore, + store: folderStore, + features: features, + accessControl: ac, } } diff --git a/pkg/services/libraryelements/libraryelements_test.go b/pkg/services/libraryelements/libraryelements_test.go index 1ca4a0c8304..d293d57d6c7 100644 --- a/pkg/services/libraryelements/libraryelements_test.go +++ b/pkg/services/libraryelements/libraryelements_test.go @@ -310,16 +310,11 @@ func createFolderWithACL(t *testing.T, sqlStore db.DB, title string, user user.S cfg.IsFeatureToggleEnabled = features.IsEnabled ac := acmock.New() folderPermissions := acmock.NewMockedPermissionsService() - dashboardPermissions := acmock.NewMockedPermissionsService() quotaService := quotatest.New(false, nil) dashboardStore, err := database.ProvideDashboardStore(sqlStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore, cfg), quotaService) require.NoError(t, err) - d := dashboardservice.ProvideDashboardService( - cfg, dashboardStore, nil, - features, folderPermissions, dashboardPermissions, ac, - ) - s := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), cfg, d, dashboardStore, nil, features, folderPermissions, nil) + s := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), cfg, dashboardStore, nil, features, folderPermissions, nil) t.Logf("Creating folder with title and UID %q", title) ctx := appcontext.WithUser(context.Background(), &user) folder, err := s.Create(ctx, &folder.CreateFolderCommand{ @@ -447,7 +442,7 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo service := LibraryElementService{ Cfg: sqlStore.Cfg, SQLStore: sqlStore, - folderService: folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), sqlStore.Cfg, dashboardService, dashboardStore, nil, features, folderPermissions, nil), + folderService: folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), sqlStore.Cfg, dashboardStore, nil, features, folderPermissions, nil), } // deliberate difference between signed in user and user in db to make it crystal clear diff --git a/pkg/services/librarypanels/librarypanels_test.go b/pkg/services/librarypanels/librarypanels_test.go index 8f9287735ef..586a3800dd0 100644 --- a/pkg/services/librarypanels/librarypanels_test.go +++ b/pkg/services/librarypanels/librarypanels_test.go @@ -723,12 +723,10 @@ func createFolderWithACL(t *testing.T, sqlStore db.DB, title string, user *user. cfg.IsFeatureToggleEnabled = featuremgmt.WithFeatures().IsEnabled features := featuremgmt.WithFeatures() folderPermissions := acmock.NewMockedPermissionsService() - dashboardPermissions := acmock.NewMockedPermissionsService() quotaService := quotatest.New(false, nil) dashboardStore, err := database.ProvideDashboardStore(sqlStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore, cfg), quotaService) require.NoError(t, err) - d := dashboardservice.ProvideDashboardService(cfg, dashboardStore, nil, features, folderPermissions, dashboardPermissions, ac) - s := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), cfg, d, dashboardStore, nil, features, folderPermissions, nil) + s := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), cfg, dashboardStore, nil, features, folderPermissions, nil) t.Logf("Creating folder with title and UID %q", title) ctx := appcontext.WithUser(context.Background(), user) @@ -836,13 +834,8 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo features := featuremgmt.WithFeatures() ac := acmock.New() folderPermissions := acmock.NewMockedPermissionsService() - dashboardPermissions := acmock.NewMockedPermissionsService() - dashboardService := dashboardservice.ProvideDashboardService( - cfg, dashboardStore, &alerting.DashAlertExtractorService{}, - features, folderPermissions, dashboardPermissions, ac, - ) - folderService := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), cfg, dashboardService, dashboardStore, nil, features, folderPermissions, nil) + folderService := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), cfg, dashboardStore, nil, features, folderPermissions, nil) elementService := libraryelements.ProvideService(cfg, sqlStore, routing.NewRouteRegister(), folderService) service := LibraryPanelService{ diff --git a/pkg/services/ngalert/tests/util.go b/pkg/services/ngalert/tests/util.go index 098a4af36aa..4c8e0207214 100644 --- a/pkg/services/ngalert/tests/util.go +++ b/pkg/services/ngalert/tests/util.go @@ -85,7 +85,7 @@ func SetupTestEnv(tb testing.TB, baseInterval time.Duration) (*ngalert.AlertNG, tracer := tracing.InitializeTracerForTest() bus := bus.ProvideBus(tracer) - folderService := folderimpl.ProvideService(ac, bus, cfg, dashboardService, dashboardStore, nil, features, folderPermissions, nil) + folderService := folderimpl.ProvideService(ac, bus, cfg, dashboardStore, nil, features, folderPermissions, nil) ng, err := ngalert.ProvideService( cfg, featuremgmt.WithFeatures(), nil, nil, routing.NewRouteRegister(), sqlStore, nil, nil, nil, quotatest.New(false, nil),