public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Nested folders: Allow creating folders with duplicate names in different locations (#77076)

Browse Source 
* Add API test

* Add move tests

* Fix create folder

* Fix move

* Fix test

* Drop and re-create index so that allows a folder to contain a dashboard and a subfolder with same name

* Get folder by title defaults to root folder and optionally fetches folder by provided parent folder

* Apply suggestions from code review
This commit is contained in:
Sofia Papagiannaki
2024-01-25 11:29:56 +02:00
committed by GitHub
parent 030a68bbf7
commit 478d7d58fa
17 changed files with 330 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/folder/folderimpl/dashboard_folder_store.go
+12 -6
View File
@@ -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 {
pkg/services/folder/folderimpl/dashboard_folder_store_test.go
+26 -13
View File
@@ -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)
pkg/services/folder/folderimpl/folder.go
+9 -3
View File
@@ -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)
}
pkg/services/folder/folderimpl/folder_test.go
+2 -2
View File
@@ -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)
})
pkg/services/folder/folderimpl/sqlstore.go
+10 -1
View File
@@ -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")
}
pkg/services/folder/folderimpl/sqlstore_test.go
+24
View File
@@ -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,