diff --git a/pkg/services/dashboards/accesscontrol.go b/pkg/services/dashboards/accesscontrol.go index 61c19267e53..86161f8c125 100644 --- a/pkg/services/dashboards/accesscontrol.go +++ b/pkg/services/dashboards/accesscontrol.go @@ -49,7 +49,9 @@ func NewFolderNameScopeResolver(folderDB folder.FolderStore, folderSvc folder.Se if len(nsName) == 0 { return nil, ac.ErrInvalidScope } - folder, err := folderDB.GetFolderByTitle(ctx, orgID, nsName) + // this will fetch only root folders + // this is legacy code so most probably it is not used + folder, err := folderDB.GetFolderByTitle(ctx, orgID, nsName, nil) if err != nil { return nil, err } diff --git a/pkg/services/dashboards/accesscontrol_test.go b/pkg/services/dashboards/accesscontrol_test.go index d1b2d9f584b..5d0ec5cc976 100644 --- a/pkg/services/dashboards/accesscontrol_test.go +++ b/pkg/services/dashboards/accesscontrol_test.go @@ -28,7 +28,7 @@ func TestNewFolderNameScopeResolver(t *testing.T) { title := "Very complex :title with: and /" + util.GenerateShortUID() db := &folder.Folder{Title: title, UID: util.GenerateShortUID()} folderStore := foldertest.NewFakeFolderStore(t) - folderStore.On("GetFolderByTitle", mock.Anything, mock.Anything, mock.Anything).Return(db, nil).Once() + folderStore.On("GetFolderByTitle", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(db, nil).Once() scope := "folders:name:" + title @@ -37,7 +37,7 @@ func TestNewFolderNameScopeResolver(t *testing.T) { require.NoError(t, err) require.Len(t, resolvedScopes, 1) require.Equal(t, fmt.Sprintf("folders:uid:%v", db.UID), resolvedScopes[0]) - folderStore.AssertCalled(t, "GetFolderByTitle", mock.Anything, orgId, title) + folderStore.AssertCalled(t, "GetFolderByTitle", mock.Anything, orgId, title, mock.Anything) }) t.Run("resolver should include inherited scopes if any", func(t *testing.T) { orgId := rand.Int63() @@ -46,7 +46,7 @@ func TestNewFolderNameScopeResolver(t *testing.T) { db := &folder.Folder{Title: title, UID: util.GenerateShortUID()} folderStore := foldertest.NewFakeFolderStore(t) - folderStore.On("GetFolderByTitle", mock.Anything, mock.Anything, mock.Anything).Return(db, nil).Once() + folderStore.On("GetFolderByTitle", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(db, nil).Once() scope := "folders:name:" + title @@ -73,7 +73,7 @@ func TestNewFolderNameScopeResolver(t *testing.T) { t.Errorf("Result mismatch (-want +got):\n%s", diff) } - folderStore.AssertCalled(t, "GetFolderByTitle", mock.Anything, orgId, title) + folderStore.AssertCalled(t, "GetFolderByTitle", mock.Anything, orgId, title, mock.Anything) }) t.Run("resolver should fail if input scope is not expected", func(t *testing.T) { _, resolver := NewFolderNameScopeResolver(foldertest.NewFakeFolderStore(t), foldertest.NewFakeService()) @@ -92,7 +92,7 @@ func TestNewFolderNameScopeResolver(t *testing.T) { _, resolver := NewFolderNameScopeResolver(folderStore, foldertest.NewFakeService()) orgId := rand.Int63() - folderStore.On("GetFolderByTitle", mock.Anything, mock.Anything, mock.Anything).Return(nil, ErrDashboardNotFound).Once() + folderStore.On("GetFolderByTitle", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, ErrDashboardNotFound).Once() scope := "folders:name:" + util.GenerateShortUID() diff --git a/pkg/services/dashboards/database/database.go b/pkg/services/dashboards/database/database.go index 9d580d5baa3..4d9d2ce906f 100644 --- a/pkg/services/dashboards/database/database.go +++ b/pkg/services/dashboards/database/database.go @@ -346,9 +346,15 @@ func getExistingDashboardByIDOrUIDForUpdate(sess *db.Session, dash *dashboards.D func getExistingDashboardByTitleAndFolder(sess *db.Session, dash *dashboards.Dashboard, dialect migrator.Dialect, overwrite, isParentFolderChanged bool) (bool, error) { var existing dashboards.Dashboard - // nolint:staticcheck - exists, err := sess.Where("org_id=? AND title=? AND (is_folder=? OR folder_id=?)", dash.OrgID, dash.Title, - dialect.BooleanStr(true), dash.FolderID).Get(&existing) + condition := "org_id=? AND title=?" + args := []any{dash.OrgID, dash.Title} + if dash.FolderUID != "" { + condition += " AND folder_uid=?" + args = append(args, dash.FolderUID) + } else { + condition += " AND folder_uid IS NULL" + } + exists, err := sess.Where(condition, args...).Get(&existing) if err != nil { return isParentFolderChanged, fmt.Errorf("SQL query for existing dashboard by org ID or folder ID failed: %w", err) } diff --git a/pkg/services/dashboards/database/database_test.go b/pkg/services/dashboards/database/database_test.go index 9d3931021ac..75e88deeeb9 100644 --- a/pkg/services/dashboards/database/database_test.go +++ b/pkg/services/dashboards/database/database_test.go @@ -656,7 +656,7 @@ func TestGetExistingDashboardByTitleAndFolder(t *testing.T) { savedDash := insertTestDashboard(t, dashboardStore, "test dash", 1, savedFolder.ID, savedFolder.UID, false, "prod", "webapp") err = sqlStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { // nolint:staticcheck - _, err = getExistingDashboardByTitleAndFolder(sess, &dashboards.Dashboard{Title: savedDash.Title, FolderID: savedFolder.ID, OrgID: 1}, sqlStore.GetDialect(), false, false) + _, err = getExistingDashboardByTitleAndFolder(sess, &dashboards.Dashboard{Title: savedDash.Title, FolderID: savedFolder.ID, FolderUID: savedFolder.UID, OrgID: 1}, sqlStore.GetDialect(), false, false) return err }) require.ErrorIs(t, err, dashboards.ErrDashboardWithSameNameInFolderExists) diff --git a/pkg/services/dashboards/database/migrations/folder_uid_migrator.go b/pkg/services/dashboards/database/migrations/folder_uid_migrator.go index 672279e15a4..f970dcb1730 100644 --- a/pkg/services/dashboards/database/migrations/folder_uid_migrator.go +++ b/pkg/services/dashboards/database/migrations/folder_uid_migrator.go @@ -78,4 +78,16 @@ func AddDashboardFolderMigrations(mg *migrator.Migrator) { mg.AddMigration("Add unique index for dashboard_org_id_folder_uid_title", migrator.NewAddIndexMigration(migrator.Table{Name: "dashboard"}, &migrator.Index{ Cols: []string{"org_id", "folder_uid", "title"}, Type: migrator.UniqueIndex, })) + + mg.AddMigration("Delete unique index for dashboard_org_id_folder_id_title", migrator.NewDropIndexMigration(migrator.Table{Name: "dashboard"}, &migrator.Index{ + Cols: []string{"org_id", "folder_id", "title"}, Type: migrator.UniqueIndex, + })) + + mg.AddMigration("Delete unique index for dashboard_org_id_folder_uid_title", migrator.NewDropIndexMigration(migrator.Table{Name: "dashboard"}, &migrator.Index{ + Cols: []string{"org_id", "folder_uid", "title"}, Type: migrator.UniqueIndex, + })) + + mg.AddMigration("Add unique index for dashboard_org_id_folder_uid_title_is_folder", migrator.NewAddIndexMigration(migrator.Table{Name: "dashboard"}, &migrator.Index{ + Cols: []string{"org_id", "folder_uid", "title", "is_folder"}, Type: migrator.UniqueIndex, + })) } diff --git a/pkg/services/dashboards/errors.go b/pkg/services/dashboards/errors.go index 6cbc6d9d333..307f18fc50e 100644 --- a/pkg/services/dashboards/errors.go +++ b/pkg/services/dashboards/errors.go @@ -122,7 +122,7 @@ var ( ErrFolderTitleEmpty = errors.New("folder title cannot be empty") ErrFolderWithSameUIDExists = errors.New("a folder/dashboard with the same uid already exists") ErrFolderInvalidUID = errors.New("invalid uid for folder provided") - ErrFolderSameNameExists = errors.New("a folder or dashboard in the general folder with the same name already exists") + ErrFolderSameNameExists = errors.New("a folder with the same name already exists in the current location") ErrFolderAccessDenied = errors.New("access denied to folder") ) diff --git a/pkg/services/folder/folderimpl/dashboard_folder_store.go b/pkg/services/folder/folderimpl/dashboard_folder_store.go index aef13d84b0a..b570b7a9e76 100644 --- a/pkg/services/folder/folderimpl/dashboard_folder_store.go +++ b/pkg/services/folder/folderimpl/dashboard_folder_store.go @@ -19,17 +19,23 @@ func ProvideDashboardFolderStore(sqlStore db.DB) *DashboardFolderStoreImpl { return &DashboardFolderStoreImpl{store: sqlStore} } -func (d *DashboardFolderStoreImpl) GetFolderByTitle(ctx context.Context, orgID int64, title string) (*folder.Folder, error) { +func (d *DashboardFolderStoreImpl) GetFolderByTitle(ctx context.Context, orgID int64, title string, folderUID *string) (*folder.Folder, error) { if title == "" { return nil, dashboards.ErrFolderTitleEmpty } - // there is a unique constraint on org_id, folder_id, title + // there is a unique constraint on org_id, folder_uid, title // there are no nested folders so the parent folder id is always 0 // nolint:staticcheck dashboard := dashboards.Dashboard{OrgID: orgID, FolderID: 0, Title: title} err := d.store.WithTransactionalDbSession(ctx, func(sess *db.Session) error { - has, err := sess.Table(&dashboards.Dashboard{}).Where("is_folder = " + d.store.GetDialect().BooleanStr(true)).Where("folder_id=0").Get(&dashboard) + s := sess.Table(&dashboards.Dashboard{}).Where("is_folder = " + d.store.GetDialect().BooleanStr(true)) + if folderUID != nil { + s = s.Where("folder_uid = ?", *folderUID) + } else { + s = s.Where("folder_uid IS NULL") + } + has, err := s.Get(&dashboard) if err != nil { return err } @@ -47,7 +53,7 @@ func (d *DashboardFolderStoreImpl) GetFolderByID(ctx context.Context, orgID int6 // nolint:staticcheck dashboard := dashboards.Dashboard{OrgID: orgID, FolderID: 0, ID: id} err := d.store.WithTransactionalDbSession(ctx, func(sess *db.Session) error { - has, err := sess.Table(&dashboards.Dashboard{}).Where("is_folder = " + d.store.GetDialect().BooleanStr(true)).Where("folder_id=0").Get(&dashboard) + has, err := sess.Table(&dashboards.Dashboard{}).Where("is_folder = " + d.store.GetDialect().BooleanStr(true)).Get(&dashboard) if err != nil { return err } @@ -72,7 +78,7 @@ func (d *DashboardFolderStoreImpl) GetFolderByUID(ctx context.Context, orgID int // nolint:staticcheck dashboard := dashboards.Dashboard{OrgID: orgID, FolderID: 0, UID: uid} err := d.store.WithTransactionalDbSession(ctx, func(sess *db.Session) error { - has, err := sess.Table(&dashboards.Dashboard{}).Where("is_folder = " + d.store.GetDialect().BooleanStr(true)).Where("folder_id=0").Get(&dashboard) + has, err := sess.Table(&dashboards.Dashboard{}).Where("is_folder = " + d.store.GetDialect().BooleanStr(true)).Get(&dashboard) if err != nil { return err } @@ -100,7 +106,7 @@ func (d *DashboardFolderStoreImpl) GetFolders(ctx context.Context, orgID int64, b := strings.Builder{} args := make([]any, 0, len(uids)+1) - b.WriteString("SELECT * FROM dashboard WHERE org_id=?") + b.WriteString("SELECT * FROM dashboard WHERE org_id=? AND is_folder = " + d.store.GetDialect().BooleanStr(true)) args = append(args, orgID) if len(uids) == 1 { diff --git a/pkg/services/folder/folderimpl/dashboard_folder_store_test.go b/pkg/services/folder/folderimpl/dashboard_folder_store_test.go index 4f433e6af99..c456fbbb875 100644 --- a/pkg/services/folder/folderimpl/dashboard_folder_store_test.go +++ b/pkg/services/folder/folderimpl/dashboard_folder_store_test.go @@ -37,22 +37,30 @@ func TestIntegrationDashboardFolderStore(t *testing.T) { var folder1, folder2 *dashboards.Dashboard sqlStore = db.InitTestDB(t) folderStore := ProvideDashboardFolderStore(sqlStore) - folder2 = insertTestFolder(t, dashboardStore, "TEST", orgId, 0, "prod") + folder2 = insertTestFolder(t, dashboardStore, "TEST", orgId, 0, "", "prod") _ = insertTestDashboard(t, dashboardStore, title, orgId, folder2.ID, folder2.UID, "prod") - folder1 = insertTestFolder(t, dashboardStore, title, orgId, 0, "prod") + folder1 = insertTestFolder(t, dashboardStore, title, orgId, 0, "", "prod") t.Run("GetFolderByTitle should find the folder", func(t *testing.T) { - result, err := folderStore.GetFolderByTitle(context.Background(), orgId, title) + result, err := folderStore.GetFolderByTitle(context.Background(), orgId, title, nil) require.NoError(t, err) require.Equal(t, folder1.UID, result.UID) }) + + t.Run("GetFolderByTitle should find the folder by folderUID", func(t *testing.T) { + folder3 := insertTestFolder(t, dashboardStore, title, orgId, folder2.ID, folder2.UID, "prod") + result, err := folderStore.GetFolderByTitle(context.Background(), orgId, title, &folder2.UID) + require.NoError(t, err) + require.Equal(t, folder3.UID, result.UID) + }) }) t.Run("GetFolderByUID", func(t *testing.T) { + setup() var orgId int64 = 1 sqlStore := db.InitTestDB(t) folderStore := ProvideDashboardFolderStore(sqlStore) - folder := insertTestFolder(t, dashboardStore, "TEST", orgId, 0, "prod") + folder := insertTestFolder(t, dashboardStore, "TEST", orgId, 0, "", "prod") dash := insertTestDashboard(t, dashboardStore, "Very Unique Name", orgId, folder.ID, folder.UID, "prod") t.Run("should return folder by UID", func(t *testing.T) { @@ -73,10 +81,11 @@ func TestIntegrationDashboardFolderStore(t *testing.T) { }) t.Run("GetFolderByID", func(t *testing.T) { + setup() var orgId int64 = 1 sqlStore := db.InitTestDB(t) folderStore := ProvideDashboardFolderStore(sqlStore) - folder := insertTestFolder(t, dashboardStore, "TEST", orgId, 0, "prod") + folder := insertTestFolder(t, dashboardStore, "TEST", orgId, 0, "", "prod") dash := insertTestDashboard(t, dashboardStore, "Very Unique Name", orgId, folder.ID, folder.UID, "prod") t.Run("should return folder by ID", func(t *testing.T) { @@ -100,16 +109,18 @@ func TestIntegrationDashboardFolderStore(t *testing.T) { func insertTestDashboard(t *testing.T, dashboardStore dashboards.Store, title string, orgId int64, folderID int64, folderUID string, tags ...any) *dashboards.Dashboard { t.Helper() cmd := dashboards.SaveDashboardCommand{ - OrgID: orgId, - FolderID: folderID, // nolint:staticcheck - FolderUID: folderUID, - IsFolder: false, + OrgID: orgId, + FolderID: folderID, // nolint:staticcheck + IsFolder: false, Dashboard: simplejson.NewFromAny(map[string]any{ "id": nil, "title": title, "tags": tags, }), } + if folderUID != "" { + cmd.FolderUID = folderUID + } dash, err := dashboardStore.SaveDashboard(context.Background(), cmd) require.NoError(t, err) require.NotNil(t, dash) @@ -121,16 +132,18 @@ func insertTestDashboard(t *testing.T, dashboardStore dashboards.Store, title st func insertTestFolder(t *testing.T, dashboardStore dashboards.Store, title string, orgId int64, folderId int64, folderUID string, tags ...any) *dashboards.Dashboard { t.Helper() cmd := dashboards.SaveDashboardCommand{ - OrgID: orgId, - FolderID: folderId, // nolint:staticcheck - FolderUID: folderUID, - IsFolder: true, + OrgID: orgId, + FolderID: folderId, // nolint:staticcheck + IsFolder: true, Dashboard: simplejson.NewFromAny(map[string]any{ "id": nil, "title": title, "tags": tags, }), } + if folderUID != "" { + cmd.FolderUID = folderUID + } dash, err := dashboardStore.SaveDashboard(context.Background(), cmd) require.NoError(t, err) require.NotNil(t, dash) diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index 06fc3b52ac6..3cba1c294e2 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -176,7 +176,7 @@ func (s *Service) Get(ctx context.Context, q *folder.GetFolderQuery) (*folder.Fo return nil, err } case q.Title != nil: - dashFolder, err = s.getFolderByTitle(ctx, q.OrgID, *q.Title) + dashFolder, err = s.getFolderByTitle(ctx, q.OrgID, *q.Title, q.ParentUID) if err != nil { return nil, err } @@ -457,8 +457,8 @@ func (s *Service) getFolderByUID(ctx context.Context, orgID int64, uid string) ( return s.dashboardFolderStore.GetFolderByUID(ctx, orgID, uid) } -func (s *Service) getFolderByTitle(ctx context.Context, orgID int64, title string) (*folder.Folder, error) { - return s.dashboardFolderStore.GetFolderByTitle(ctx, orgID, title) +func (s *Service) getFolderByTitle(ctx context.Context, orgID int64, title string, parentUID *string) (*folder.Folder, error) { + return s.dashboardFolderStore.GetFolderByTitle(ctx, orgID, title, parentUID) } func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (*folder.Folder, error) { @@ -854,6 +854,9 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol NewParentUID: &newParentUID, SignedInUser: cmd.SignedInUser, }); err != nil { + if s.db.GetDialect().IsUniqueConstraintViolation(err) { + return folder.ErrConflict.Errorf("%w", dashboards.ErrFolderSameNameExists) + } return folder.ErrInternal.Errorf("failed to move folder: %w", err) } @@ -865,6 +868,9 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol // bypass optimistic locking used for dashboards Overwrite: true, }); err != nil { + if s.db.GetDialect().IsUniqueConstraintViolation(err) { + return folder.ErrConflict.Errorf("%w", dashboards.ErrFolderSameNameExists) + } return folder.ErrInternal.Errorf("failed to move legacy folder: %w", err) } diff --git a/pkg/services/folder/folderimpl/folder_test.go b/pkg/services/folder/folderimpl/folder_test.go index a4458d675f0..8798658ff9e 100644 --- a/pkg/services/folder/folderimpl/folder_test.go +++ b/pkg/services/folder/folderimpl/folder_test.go @@ -303,9 +303,9 @@ func TestIntegrationFolderService(t *testing.T) { t.Run("When get folder by title should return folder", func(t *testing.T) { expected := folder.NewFolder("TEST-"+util.GenerateShortUID(), "") - folderStore.On("GetFolderByTitle", mock.Anything, orgID, expected.Title).Return(expected, nil) + folderStore.On("GetFolderByTitle", mock.Anything, orgID, expected.Title, mock.Anything).Return(expected, nil) - actual, err := service.getFolderByTitle(context.Background(), orgID, expected.Title) + actual, err := service.getFolderByTitle(context.Background(), orgID, expected.Title, nil) require.Equal(t, expected, actual) require.NoError(t, err) }) diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go index b107222bc59..29137f6f584 100644 --- a/pkg/services/folder/folderimpl/sqlstore.go +++ b/pkg/services/folder/folderimpl/sqlstore.go @@ -172,7 +172,16 @@ func (ss *sqlStore) Get(ctx context.Context, q folder.GetFolderQuery) (*folder.F case q.ID != nil: exists, err = sess.SQL("SELECT * FROM folder WHERE id = ?", q.ID).Get(foldr) case q.Title != nil: - exists, err = sess.SQL("SELECT * FROM folder WHERE title = ? AND org_id = ?", q.Title, q.OrgID).Get(foldr) + s := strings.Builder{} + s.WriteString("SELECT * FROM folder WHERE title = ? AND org_id = ?") + args := []any{*q.Title, q.OrgID} + if q.ParentUID != nil { + s.WriteString(" AND parent_uid = ?") + args = append(args, *q.ParentUID) + } else { + s.WriteString(" AND parent_uid IS NULL") + } + exists, err = sess.SQL(s.String(), args...).Get(foldr) default: return folder.ErrBadRequest.Errorf("one of ID, UID, or Title must be included in the command") } diff --git a/pkg/services/folder/folderimpl/sqlstore_test.go b/pkg/services/folder/folderimpl/sqlstore_test.go index 7729c8fbab6..7d210feb2b2 100644 --- a/pkg/services/folder/folderimpl/sqlstore_test.go +++ b/pkg/services/folder/folderimpl/sqlstore_test.go @@ -384,6 +384,13 @@ func TestIntegrationGet(t *testing.T) { UID: uid1, }) require.NoError(t, err) + subfolderWithSameName, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: folderTitle, + Description: folderDsc, + OrgID: orgID, + UID: util.GenerateShortUID(), + ParentUID: f.UID, + }) t.Cleanup(func() { err := folderStore.Delete(context.Background(), f.UID, orgID) @@ -427,6 +434,23 @@ func TestIntegrationGet(t *testing.T) { assert.NotEmpty(t, ff.URL) }) + t.Run("get folder by title and parent UID should succeed", func(t *testing.T) { + ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{ + Title: &f.Title, + OrgID: orgID, + ParentUID: &uid1, + }) + require.NoError(t, err) + assert.Equal(t, subfolderWithSameName.UID, ff.UID) + assert.Equal(t, subfolderWithSameName.OrgID, ff.OrgID) + assert.Equal(t, subfolderWithSameName.Title, ff.Title) + assert.Equal(t, subfolderWithSameName.Description, ff.Description) + assert.Equal(t, subfolderWithSameName.ParentUID, ff.ParentUID) + assert.NotEmpty(t, ff.Created) + assert.NotEmpty(t, ff.Updated) + assert.NotEmpty(t, ff.URL) + }) + t.Run("get folder by title should succeed", func(t *testing.T) { ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{ UID: &f.UID, diff --git a/pkg/services/folder/foldertest/folder_store_mock.go b/pkg/services/folder/foldertest/folder_store_mock.go index aa5b782a903..93de1422257 100644 --- a/pkg/services/folder/foldertest/folder_store_mock.go +++ b/pkg/services/folder/foldertest/folder_store_mock.go @@ -40,25 +40,25 @@ func (_m *FakeFolderStore) GetFolderByID(ctx context.Context, orgID int64, id in return r0, r1 } -// GetFolderByTitle provides a mock function with given fields: ctx, orgID, title -func (_m *FakeFolderStore) GetFolderByTitle(ctx context.Context, orgID int64, title string) (*folder.Folder, error) { - ret := _m.Called(ctx, orgID, title) +// GetFolderByTitle provides a mock function with given fields: ctx, orgID, title, folderUID +func (_m *FakeFolderStore) GetFolderByTitle(ctx context.Context, orgID int64, title string, folderUID *string) (*folder.Folder, error) { + ret := _m.Called(ctx, orgID, title, folderUID) var r0 *folder.Folder var r1 error - if rf, ok := ret.Get(0).(func(context.Context, int64, string) (*folder.Folder, error)); ok { - return rf(ctx, orgID, title) + if rf, ok := ret.Get(0).(func(context.Context, int64, string, *string) (*folder.Folder, error)); ok { + return rf(ctx, orgID, title, folderUID) } - if rf, ok := ret.Get(0).(func(context.Context, int64, string) *folder.Folder); ok { - r0 = rf(ctx, orgID, title) + if rf, ok := ret.Get(0).(func(context.Context, int64, string, *string) *folder.Folder); ok { + r0 = rf(ctx, orgID, title, folderUID) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*folder.Folder) } } - if rf, ok := ret.Get(1).(func(context.Context, int64, string) error); ok { - r1 = rf(ctx, orgID, title) + if rf, ok := ret.Get(1).(func(context.Context, int64, string, *string) error); ok { + r1 = rf(ctx, orgID, title, folderUID) } else { r1 = ret.Error(1) } diff --git a/pkg/services/folder/model.go b/pkg/services/folder/model.go index da7fa6c837a..5aa996676e9 100644 --- a/pkg/services/folder/model.go +++ b/pkg/services/folder/model.go @@ -13,6 +13,7 @@ import ( var ErrMaximumDepthReached = errutil.BadRequest("folder.maximum-depth-reached", errutil.WithPublicMessage("Maximum nested folder depth reached")) var ErrBadRequest = errutil.BadRequest("folder.bad-request") var ErrDatabaseError = errutil.Internal("folder.database-error") +var ErrConflict = errutil.Conflict("folder.conflict") var ErrInternal = errutil.Internal("folder.internal") var ErrCircularReference = errutil.BadRequest("folder.circular-reference", errutil.WithPublicMessage("Circular reference detected")) var ErrTargetRegistrySrvConflict = errutil.Internal("folder.target-registry-srv-conflict") @@ -144,9 +145,10 @@ type DeleteFolderCommand struct { type GetFolderQuery struct { UID *string // Deprecated: use FolderUID instead - ID *int64 - Title *string - OrgID int64 + ID *int64 + Title *string + ParentUID *string + OrgID int64 SignedInUser identity.Requester `json:"-"` } diff --git a/pkg/services/folder/service.go b/pkg/services/folder/service.go index 242a00e2f0b..a7d09a8393d 100644 --- a/pkg/services/folder/service.go +++ b/pkg/services/folder/service.go @@ -16,6 +16,8 @@ type Service interface { // request. One of UID, ID or Title must be included. If multiple values // are included in the request, Grafana will select one in order of // specificity (UID, ID, Title). + // When fetching a folder by Title, callers can optionally define a ParentUID. + // If ParentUID is not set then the folder will be fetched from the root level. Get(ctx context.Context, q *GetFolderQuery) (*Folder, error) // Update is used to update a folder's UID, Title and Description. To change @@ -39,7 +41,10 @@ type Service interface { //go:generate mockery --name FolderStore --structname FakeFolderStore --outpkg foldertest --output foldertest --filename folder_store_mock.go type FolderStore interface { // GetFolderByTitle retrieves a folder by its title - GetFolderByTitle(ctx context.Context, orgID int64, title string) (*Folder, error) + // It expects a parentUID as last argument. + // If parentUID is empty then the folder will be fetched from the root level + // otherwise it will be fetched from the subfolder under the folder with the given UID. + GetFolderByTitle(ctx context.Context, orgID int64, title string, parentUID *string) (*Folder, error) // GetFolderByUID retrieves a folder by its UID GetFolderByUID(ctx context.Context, orgID int64, uid string) (*Folder, error) // GetFolderByID retrieves a folder by its ID diff --git a/pkg/services/ngalert/migration/migration_test.go b/pkg/services/ngalert/migration/migration_test.go index c95a47aa913..b974dbcb032 100644 --- a/pkg/services/ngalert/migration/migration_test.go +++ b/pkg/services/ngalert/migration/migration_test.go @@ -1359,6 +1359,8 @@ func createAlertWithCond(t *testing.T, orgId int, dashboardId int, panelsId int, // createDashboard creates a folder for inserting into the test database. func createFolder(t *testing.T, id int64, orgId int64, uid string) *dashboards.Dashboard { + // TODO this should create also the entries in the folder table + // or better call the folder service to take care of both f := createDashboard(t, id, orgId, uid, 0, nil) f.IsFolder = true return f @@ -1434,14 +1436,20 @@ func setupLegacyAlertsTables(t *testing.T, x *xorm.Engine, legacyChannels []*mod } // Setup folders. - if len(folders) > 0 { - _, err := x.Insert(folders) + // this loop is required because nullable it does not seem to work + // when inserting multiple rows at once + for _, f := range folders { + // if folder_uid is empty string, it will be set to NULL + _, err := x.NewSession().Nullable("folder_uid").Insert(f) require.NoError(t, err) } // Setup dashboards. - if len(dashes) > 0 { - _, err := x.Insert(dashes) + // this loop is required because nullable it does not seem to work + // when inserting multiple rows at once + for _, d := range dashes { + // if folder_uid is empty string, it will be set to NULL + _, err := x.NewSession().Nullable("folder_uid").Insert(d) require.NoError(t, err) } diff --git a/pkg/tests/api/folders/api_folder_test.go b/pkg/tests/api/folders/api_folder_test.go new file mode 100644 index 00000000000..d9366946a6a --- /dev/null +++ b/pkg/tests/api/folders/api_folder_test.go @@ -0,0 +1,184 @@ +package folders + +import ( + "context" + "errors" + "net/http" + "testing" + + "github.com/go-openapi/runtime" + "github.com/grafana/grafana-openapi-client-go/client/folders" + "github.com/grafana/grafana-openapi-client-go/models" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/services/org/orgimpl" + "github.com/grafana/grafana/pkg/services/quota/quotaimpl" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/services/user/userimpl" + "github.com/grafana/grafana/pkg/tests" + "github.com/grafana/grafana/pkg/tests/testinfra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const orgID = 1 + +func TestIntegrationCreateFolder(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ + DisableAnonymous: true, + EnableQuota: true, + }) + + grafanaListedAddr, store := testinfra.StartGrafana(t, dir, path) + // Create user + createUser(t, store, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleAdmin), + Password: "admin", + Login: "admin", + }) + + adminClient := tests.GetClient(grafanaListedAddr, "admin", "admin") + + t.Run("create folder under root should succeed", func(t *testing.T) { + resp, err := adminClient.Folders.CreateFolder(&models.CreateFolderCommand{ + Title: "folder", + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.Code()) + + t.Run("create folder with same name under root should fail", func(t *testing.T) { + _, err := adminClient.Folders.CreateFolder(&models.CreateFolderCommand{ + Title: "folder", + }) + require.Error(t, err) + var conflict *folders.CreateFolderConflict + assert.True(t, errors.As(err, &conflict)) + assert.Equal(t, http.StatusConflict, conflict.Code()) + }) + }) +} + +func TestIntegrationNestedFoldersOn(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ + DisableAnonymous: true, + EnableQuota: true, + EnableFeatureToggles: []string{featuremgmt.FlagNestedFolders}, + }) + + grafanaListedAddr, store := testinfra.StartGrafana(t, dir, path) + // Create user + createUser(t, store, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleAdmin), + Password: "admin", + Login: "admin", + }) + + adminClient := tests.GetClient(grafanaListedAddr, "admin", "admin") + + t.Run("create folder under root should succeed", func(t *testing.T) { + resp, err := adminClient.Folders.CreateFolder(&models.CreateFolderCommand{ + Title: "folder", + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.Code()) + + t.Run("create folder with same name under root should fail", func(t *testing.T) { + _, err := adminClient.Folders.CreateFolder(&models.CreateFolderCommand{ + Title: "folder", + }) + require.Error(t, err) + var conflict *folders.CreateFolderConflict + assert.True(t, errors.As(err, &conflict)) + assert.Equal(t, http.StatusConflict, conflict.Code()) + }) + }) + + t.Run("create subfolder should succeed", func(t *testing.T) { + resp, err := adminClient.Folders.CreateFolder(&models.CreateFolderCommand{ + Title: "parent", + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.Code()) + parentUID := resp.Payload.UID + + resp, err = adminClient.Folders.CreateFolder(&models.CreateFolderCommand{ + Title: "subfolder", + ParentUID: parentUID, + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.Code()) + + t.Run("create subfolder with same name should fail", func(t *testing.T) { + resp, err = adminClient.Folders.CreateFolder(&models.CreateFolderCommand{ + Title: "subfolder", + ParentUID: parentUID, + }) + require.Error(t, err) + var conflict *folders.CreateFolderConflict + assert.True(t, errors.As(err, &conflict)) + assert.Equal(t, http.StatusConflict, conflict.Code()) + }) + + t.Run("create subfolder with same name under other folder should succeed", func(t *testing.T) { + resp, err := adminClient.Folders.CreateFolder(&models.CreateFolderCommand{ + Title: "other", + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.Code()) + other := resp.Payload.UID + + resp, err = adminClient.Folders.CreateFolder(&models.CreateFolderCommand{ + Title: "subfolder", + ParentUID: other, + }) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.Code()) + assert.Equal(t, other, resp.Payload.ParentUID) + subfolderUnderOther := resp.Payload.UID + + t.Run("move subfolder to other folder containing folder with that name should fail", func(t *testing.T) { + _, err := adminClient.Folders.MoveFolder(subfolderUnderOther, &models.MoveFolderCommand{ + ParentUID: parentUID, + }) + require.Error(t, err) + var apiError *runtime.APIError + assert.True(t, errors.As(err, &apiError)) + assert.Equal(t, http.StatusConflict, apiError.Code) + }) + + t.Run("move subfolder to root should succeed", func(t *testing.T) { + resp, err := adminClient.Folders.MoveFolder(subfolderUnderOther, &models.MoveFolderCommand{}) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.Code()) + assert.Equal(t, "", resp.Payload.ParentUID) + }) + }) + }) +} + +func createUser(t *testing.T, store *sqlstore.SQLStore, cmd user.CreateUserCommand) int64 { + t.Helper() + + store.Cfg.AutoAssignOrg = true + store.Cfg.AutoAssignOrgId = orgID + + quotaService := quotaimpl.ProvideService(store, store.Cfg) + orgService, err := orgimpl.ProvideService(store, store.Cfg, quotaService) + require.NoError(t, err) + usrSvc, err := userimpl.ProvideService(store, orgService, store.Cfg, nil, nil, quotaService, supportbundlestest.NewFakeBundleService()) + require.NoError(t, err) + + u, err := usrSvc.Create(context.Background(), &cmd) + require.NoError(t, err) + return u.ID +}