From 5c973e58bd1e59dc212067584cf25dd27a3e9472 Mon Sep 17 00:00:00 2001 From: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com> Date: Thu, 3 Nov 2022 15:21:41 +0200 Subject: [PATCH] Nested Folders: Add tests for store methods (#57662) * Nested Folders: Add store tests * Fix parent order * Fix update * skip tests! * Export test helpers for now --- pkg/services/folder/folderimpl/sqlstore.go | 186 +++-- .../folder/folderimpl/sqlstore_test.go | 658 +++++++++++++++++- pkg/services/folder/folderimpl/store.go | 10 +- pkg/services/folder/folderimpl/store_fake.go | 12 +- pkg/services/folder/foldertest/foldertest.go | 2 +- pkg/services/folder/model.go | 28 +- pkg/services/org/orgimpl/org.go | 1 + .../sqlstore/migrations/folder_mig.go | 18 +- pkg/util/reverse.go | 10 + pkg/util/reverse_test.go | 15 + 10 files changed, 854 insertions(+), 86 deletions(-) create mode 100644 pkg/util/reverse.go create mode 100644 pkg/util/reverse_test.go diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go index de4b18348df..2f2eae569a2 100644 --- a/pkg/services/folder/folderimpl/sqlstore.go +++ b/pkg/services/folder/folderimpl/sqlstore.go @@ -2,7 +2,8 @@ package folderimpl import ( "context" - "encoding/binary" + "strings" + "time" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" @@ -10,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util" ) type sqlStore struct { @@ -26,20 +28,44 @@ func ProvideStore(db db.DB, cfg *setting.Cfg, features featuremgmt.FeatureManage return &sqlStore{db: db, log: log.New("folder-store"), cfg: cfg, fm: features} } -func (ss *sqlStore) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (*folder.Folder, error) { - foldr := &folder.Folder{ - OrgID: cmd.OrgID, - UID: cmd.UID, - ParentUID: cmd.ParentUID, - Title: cmd.Title, - Description: cmd.Description, +func (ss *sqlStore) Create(ctx context.Context, cmd folder.CreateFolderCommand) (*folder.Folder, error) { + if cmd.UID == "" { + return nil, folder.ErrBadRequest.Errorf("missing UID") } + + var foldr *folder.Folder err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { - folderID, err := sess.Insert(foldr) + var sqlOrArgs []interface{} + if cmd.ParentUID == "" { + sql := "INSERT INTO folder(org_id, uid, title, description, created, updated) VALUES(?, ?, ?, ?, ?, ?)" + sqlOrArgs = []interface{}{sql, cmd.OrgID, cmd.UID, cmd.Title, cmd.Description, time.Now(), time.Now()} + } else { + if cmd.ParentUID != folder.GeneralFolderUID { + if _, err := ss.Get(ctx, folder.GetFolderQuery{ + UID: &cmd.ParentUID, + OrgID: cmd.OrgID, + }); err != nil { + return err + } + } + sql := "INSERT INTO folder(org_id, uid, parent_uid, title, description, created, updated) VALUES(?, ?, ?, ?, ?, ?, ?)" + sqlOrArgs = []interface{}{sql, cmd.OrgID, cmd.UID, cmd.ParentUID, cmd.Title, cmd.Description, time.Now(), time.Now()} + } + res, err := sess.Exec(sqlOrArgs...) + if err != nil { + return folder.ErrDatabaseError.Errorf("failed to insert folder: %w", err) + } + id, err := res.LastInsertId() + if err != nil { + return folder.ErrDatabaseError.Errorf("failed to get last inserted id: %w", err) + } + + foldr, err = ss.Get(ctx, folder.GetFolderQuery{ + ID: &id, + }) if err != nil { return err } - foldr.ID = folderID return nil }) return foldr, err @@ -47,26 +73,85 @@ func (ss *sqlStore) Create(ctx context.Context, cmd *folder.CreateFolderCommand) func (ss *sqlStore) Delete(ctx context.Context, uid string, orgID int64) error { return ss.db.WithDbSession(ctx, func(sess *db.Session) error { - _, err := sess.Exec("DELETE FROM folder WHERE folder_uid=? AND org_id=?", uid, orgID) - return err + _, err := sess.Exec("DELETE FROM folder WHERE uid=? AND org_id=?", uid, orgID) + if err != nil { + return folder.ErrDatabaseError.Errorf("failed to delete folder: %w", err) + } + /* + affected, err := res.RowsAffected() + if err != nil { + return folder.ErrDatabaseError.Errorf("failed to get affected rows: %w", err) + } + if affected == 0 { + return folder.ErrFolderNotFound.Errorf("folder not found uid:%s org_id:%d", uid, orgID) + } + */ + return nil }) } -func (ss *sqlStore) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (*folder.Folder, error) { +func (ss *sqlStore) Update(ctx context.Context, cmd folder.UpdateFolderCommand) (*folder.Folder, error) { + if cmd.Folder == nil { + return nil, folder.ErrBadRequest.Errorf("invalid update command: missing folder") + } + + cmd.Folder.Updated = time.Now() err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { - _, err := sess.ID(cmd.Folder.ID).AllCols().Update(cmd.Folder) - return err + description := cmd.Folder.Description + if cmd.NewDescription != nil { + description = *cmd.NewDescription + } + + title := cmd.Folder.Title + if cmd.NewTitle != nil { + title = *cmd.NewTitle + } + + uid := cmd.Folder.UID + if cmd.NewUID != nil { + uid = *cmd.NewUID + } + + res, err := sess.Exec("UPDATE folder SET description = ?, title = ?, uid = ?, updated = ? WHERE uid = ? AND org_id = ?", description, title, uid, cmd.Folder.Updated, cmd.Folder.UID, cmd.Folder.OrgID) + if err != nil { + return folder.ErrDatabaseError.Errorf("failed to update folder: %w", err) + } + + affected, err := res.RowsAffected() + if err != nil { + return folder.ErrInternal.Errorf("failed to get affected row: %w", err) + } + if affected == 0 { + return folder.ErrInternal.Errorf("no folders are updated") + } + + cmd.Folder.Description = description + cmd.Folder.Title = title + cmd.Folder.UID = uid + return nil }) return cmd.Folder, err } -func (ss *sqlStore) Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folder.Folder, error) { - var foldr *folder.Folder +func (ss *sqlStore) Get(ctx context.Context, q folder.GetFolderQuery) (*folder.Folder, error) { + foldr := &folder.Folder{} err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { - exists, err := sess.Where("uid=? OR id=? OR title=?", cmd.UID, cmd.ID, cmd.Title).Get(foldr) + exists := false + var err error + + switch { + 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) + case q.UID != nil: + exists, err = sess.SQL("SELECT * FROM folder WHERE uid = ? AND org_id = ?", q.UID, q.OrgID).Get(foldr) + default: + return folder.ErrBadRequest.Errorf("one of ID, UID, or Title must be included in the command") + } if err != nil { - return err + return folder.ErrDatabaseError.Errorf("failed to get folder: %w", err) } if !exists { return folder.ErrFolderNotFound.Errorf("folder not found") @@ -76,65 +161,62 @@ func (ss *sqlStore) Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folde return foldr, err } -func (ss *sqlStore) GetParents(ctx context.Context, cmd *folder.GetParentsQuery) ([]*folder.Folder, error) { +func (ss *sqlStore) GetParents(ctx context.Context, q folder.GetParentsQuery) ([]*folder.Folder, error) { var folders []*folder.Folder if ss.db.GetDBType() == migrator.MySQL { - return ss.getParentsMySQL(ctx, cmd) + return ss.getParentsMySQL(ctx, q) } - recQuery := - `WITH RecQry AS ( - SELECT * - FROM folder - UNION ALL - SELECT f.* - FROM folder f INNER JOIN RecQry r - ON f.parent_uid = r.uid + recQuery := ` + WITH RECURSIVE RecQry AS ( + SELECT * FROM folder WHERE uid = ? AND org_id = ? + UNION ALL SELECT f.* FROM folder f INNER JOIN RecQry r ON f.uid = r.parent_uid and f.org_id = r.org_id ) - SELECT * - FROM RecQry` + SELECT * FROM RecQry; + ` err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { - res, err := sess.Query(recQuery) + err := sess.SQL(recQuery, q.UID, q.OrgID).Find(&folders) if err != nil { - return err - } - - for _, row := range res { - folders = append(folders, &folder.Folder{ - ID: int64(binary.BigEndian.Uint64(row["id"])), - OrgID: int64(binary.BigEndian.Uint64(row["org_id"])), - UID: string(row["uid"]), - ParentUID: string(row["parent_uid"]), - Title: string(row["title"]), - Description: string(row["description"]), - // CreatedBy: int64(binary.BigEndian.Uint64(row["created_by"])), - }) + return folder.ErrDatabaseError.Errorf("failed to get folder parents: %w", err) } return nil }) - return nil, err + return util.Reverse(folders[1:]), err } -func (ss *sqlStore) GetChildren(ctx context.Context, cmd *folder.GetTreeQuery) ([]*folder.Folder, error) { +func (ss *sqlStore) GetChildren(ctx context.Context, q folder.GetTreeQuery) ([]*folder.Folder, error) { var folders []*folder.Folder err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { - err := sess.Where("parent_uid=? AND org_id=?", cmd.UID, cmd.OrgID).Find(folders) - return err + sql := strings.Builder{} + sql.Write([]byte("SELECT * FROM folder WHERE parent_uid=? AND org_id=?")) + + if q.Limit != 0 { + var offset int64 = 1 + if q.Page != 0 { + offset = q.Page + } + sql.Write([]byte(ss.db.GetDialect().LimitOffset(q.Limit, offset))) + } + err := sess.SQL(sql.String(), q.UID, q.OrgID).Find(&folders) + if err != nil { + return folder.ErrDatabaseError.Errorf("failed to get folder children: %w", err) + } + return nil }) return folders, err } -func (ss *sqlStore) getParentsMySQL(ctx context.Context, cmd *folder.GetParentsQuery) ([]*folder.Folder, error) { +func (ss *sqlStore) getParentsMySQL(ctx context.Context, cmd folder.GetParentsQuery) ([]*folder.Folder, error) { var foldrs []*folder.Folder var foldr *folder.Folder err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { uid := cmd.UID for uid != folder.GeneralFolderUID && len(foldrs) < 8 { - err := sess.Where("uid=?", uid).Find(foldr) + err := sess.Where("uid=? AND org_id=>", uid, cmd.OrgID).Find(foldr) if err != nil { - return err + return folder.ErrDatabaseError.Errorf("failed to get folder parents: %w", err) } foldrs = append(foldrs, foldr) uid = foldr.ParentUID diff --git a/pkg/services/folder/folderimpl/sqlstore_test.go b/pkg/services/folder/folderimpl/sqlstore_test.go index 056a906673b..57fe4c7a793 100644 --- a/pkg/services/folder/folderimpl/sqlstore_test.go +++ b/pkg/services/folder/folderimpl/sqlstore_test.go @@ -1,19 +1,659 @@ package folderimpl -import "testing" +import ( + "context" + "fmt" + "testing" -func TestCreate(t *testing.T) {} + "github.com/google/go-cmp/cmp" + "github.com/grafana/grafana/pkg/services/accesscontrol" + "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" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) -func TestDelete(t *testing.T) {} +func TestIntegrationCreate(t *testing.T) { + t.Skip("skipping until folder migration is merged") -func TestUpdate(t *testing.T) {} + db := sqlstore.InitTestDB(t) + folderStore := ProvideStore(db, db.Cfg, *featuremgmt.WithFeatures()) -func TestGet(t *testing.T) {} + orgID := CreateOrg(t, db) -func TestGetParent(t *testing.T) {} + t.Run("creating a folder without providing a UID should fail", func(t *testing.T) { + _, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: "folder1", + Description: "folder desc", + OrgID: orgID, + }) + require.Error(t, err) + }) -func TestGetParents(t *testing.T) {} + t.Run("creating a folder with unknown parent should fail", func(t *testing.T) { + _, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: "folder1", + OrgID: orgID, + ParentUID: "unknown", + Description: "folder desc", + UID: util.GenerateShortUID(), + }) + require.Error(t, err) + }) -func TestGetChildren(t *testing.T) {} + t.Run("creating a folder without providing a parent should default to the general folder", func(t *testing.T) { + uid := util.GenerateShortUID() + f, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: "folder1", + Description: "folder desc", + OrgID: orgID, + UID: uid, + }) + require.NoError(t, err) -func TestGetDescendents(t *testing.T) {} + t.Cleanup(func() { + err := folderStore.Delete(context.Background(), f.UID, orgID) + require.NoError(t, err) + }) + + assert.Equal(t, "folder1", f.Title) + assert.Equal(t, "folder desc", f.Description) + assert.NotEmpty(t, f.ID) + assert.Equal(t, uid, f.UID) + assert.Equal(t, folder.GeneralFolderUID, f.ParentUID) + + ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{ + UID: &f.UID, + OrgID: orgID, + }) + assert.NoError(t, err) + assert.Equal(t, "folder1", ff.Title) + assert.Equal(t, "folder desc", ff.Description) + assert.Equal(t, accesscontrol.GeneralFolderUID, ff.ParentUID) + + assertAncestorUIDs(t, folderStore, f, []string{folder.GeneralFolderUID}) + }) + + t.Run("creating a folder with a known parent should succeed", func(t *testing.T) { + parentUID := util.GenerateShortUID() + parent, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: "parent", + OrgID: orgID, + UID: parentUID, + }) + require.NoError(t, err) + require.Equal(t, "parent", parent.Title) + require.NotEmpty(t, parent.ID) + assert.Equal(t, parentUID, parent.UID) + + t.Cleanup(func() { + err := folderStore.Delete(context.Background(), parent.UID, orgID) + require.NoError(t, err) + }) + assertAncestorUIDs(t, folderStore, parent, []string{folder.GeneralFolderUID}) + + uid := util.GenerateShortUID() + f, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: "folder1", + OrgID: orgID, + ParentUID: parent.UID, + Description: "folder desc", + UID: uid, + }) + require.NoError(t, err) + t.Cleanup(func() { + err := folderStore.Delete(context.Background(), f.UID, orgID) + require.NoError(t, err) + }) + + assert.Equal(t, "folder1", f.Title) + assert.Equal(t, "folder desc", f.Description) + assert.NotEmpty(t, f.ID) + assert.Equal(t, uid, f.UID) + assert.Equal(t, parentUID, f.ParentUID) + + assertAncestorUIDs(t, folderStore, f, []string{folder.GeneralFolderUID, parent.UID}) + assertChildrenUIDs(t, folderStore, parent, []string{f.UID}) + + ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{ + UID: &f.UID, + OrgID: f.OrgID, + }) + assert.NoError(t, err) + assert.Equal(t, "folder1", ff.Title) + assert.Equal(t, "folder desc", ff.Description) + assert.Equal(t, parentUID, ff.ParentUID) + }) + + /* + t.Run("creating a nested folder with the maximum nested folder depth should fail", func(t *testing.T) { + ancestorUIDs := createSubTree(t, folderStore, orgID, accesscontrol.GeneralFolderUID, folder.MaxNestedFolderDepth, "") + + t.Cleanup(func() { + for _, uid := range ancestorUIDs[1:] { + err := folderStore.Delete(context.Background(), uid, orgID) + require.NoError(t, err) + } + }) + + title := fmt.Sprintf("folder-%d", len(ancestorUIDs)) + _, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: "folder1", + OrgID: orgID, + ParentUID: ancestorUIDs[len(ancestorUIDs)-1], + UID: util.GenerateShortUID(), + }) + assert.Error(t, err) + }) + */ +} + +func TestIntegrationDelete(t *testing.T) { + t.Skip("skipping until folder migration is merged") + + db := sqlstore.InitTestDB(t) + folderStore := ProvideStore(db, db.Cfg, *featuremgmt.WithFeatures()) + + orgID := CreateOrg(t, db) + + /* + t.Run("attempt to delete unknown folder should fail", func(t *testing.T) { + err := folderSrore.Delete(context.Background(), "unknown", orgID) + assert.Error(t, err) + }) + */ + + ancestorUIDs := CreateSubTree(t, folderStore, orgID, accesscontrol.GeneralFolderUID, folder.MaxNestedFolderDepth, "") + require.Len(t, ancestorUIDs, folder.MaxNestedFolderDepth+1) + + t.Cleanup(func() { + for _, uid := range ancestorUIDs[1:] { + err := folderStore.Delete(context.Background(), uid, orgID) + require.NoError(t, err) + } + }) + + /* + t.Run("deleting folder with children should fail", func(t *testing.T) { + err = folderSrore.Delete(context.Background(), ancestorUIDs[2], orgID) + require.Error(t, err) + }) + */ + + t.Run("deleting a leaf folder should succeed", func(t *testing.T) { + err := folderStore.Delete(context.Background(), ancestorUIDs[len(ancestorUIDs)-1], orgID) + require.NoError(t, err) + + children, err := folderStore.GetChildren(context.Background(), folder.GetTreeQuery{ + UID: ancestorUIDs[len(ancestorUIDs)-2], + OrgID: orgID, + }) + require.NoError(t, err) + assert.Len(t, children, 0) + }) +} + +func TestIntegrationUpdate(t *testing.T) { + t.Skip("skipping until folder migration is merged") + + db := sqlstore.InitTestDB(t) + folderStore := ProvideStore(db, db.Cfg, *featuremgmt.WithFeatures()) + + orgID := CreateOrg(t, db) + + // create folder + origTitle := "folder1" + origDesc := "folder desc" + f, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: origTitle, + Description: origDesc, + OrgID: orgID, + UID: util.GenerateShortUID(), + }) + require.NoError(t, err) + t.Cleanup(func() { + err := folderStore.Delete(context.Background(), f.UID, orgID) + require.NoError(t, err) + }) + + /* + t.Run("updating an unknown folder should fail", func(t *testing.T) { + newTitle := "new title" + newDesc := "new desc" + _, err := folderSrore.Update(context.Background(), &folder.UpdateFolderCommand{ + Folder: f, + NewTitle: &newTitle, + NewDescription: &newDesc, + }) + require.NoError(t, err) + + ff, err := folderSrore.Get(context.Background(), &folder.GetFolderQuery{ + UID: &f.UID, + }) + require.NoError(t, err) + + assert.Equal(t, origTitle, ff.Title) + assert.Equal(t, origDesc, ff.Description) + }) + */ + + t.Run("should not panic in case of bad requests", func(t *testing.T) { + _, err = folderStore.Update(context.Background(), folder.UpdateFolderCommand{}) + require.Error(t, err) + + _, err = folderStore.Update(context.Background(), folder.UpdateFolderCommand{ + Folder: &folder.Folder{}, + }) + require.Error(t, err) + }) + + t.Run("updating a folder should succeed", func(t *testing.T) { + newTitle := "new title" + newDesc := "new desc" + existingUpdated := f.Updated + updated, err := folderStore.Update(context.Background(), folder.UpdateFolderCommand{ + Folder: f, + NewTitle: &newTitle, + NewDescription: &newDesc, + }) + require.NoError(t, err) + + assert.Equal(t, f.UID, updated.UID) + assert.Equal(t, newTitle, updated.Title) + assert.Equal(t, newDesc, updated.Description) + assert.Greater(t, updated.Updated.UnixNano(), existingUpdated.UnixNano()) + + updated, err = folderStore.Get(context.Background(), folder.GetFolderQuery{ + UID: &updated.UID, + OrgID: orgID, + }) + require.NoError(t, err) + assert.Equal(t, newTitle, updated.Title) + assert.Equal(t, newDesc, updated.Description) + }) + + t.Run("updating folder UID should succeed", func(t *testing.T) { + newUID := "new" + existingTitle := f.Title + existingDesc := f.Description + updated, err := folderStore.Update(context.Background(), folder.UpdateFolderCommand{ + Folder: f, + NewUID: &newUID, + }) + require.NoError(t, err) + + assert.Equal(t, newUID, updated.UID) + + updated, err = folderStore.Get(context.Background(), folder.GetFolderQuery{ + UID: &updated.UID, + OrgID: orgID, + }) + require.NoError(t, err) + assert.Equal(t, newUID, updated.UID) + assert.Equal(t, existingTitle, updated.Title) + assert.Equal(t, existingDesc, updated.Description) + }) +} + +func TestIntegrationGet(t *testing.T) { + t.Skip("skipping until folder migration is merged") + + db := sqlstore.InitTestDB(t) + folderStore := ProvideStore(db, db.Cfg, *featuremgmt.WithFeatures()) + + orgID := CreateOrg(t, db) + + // create folder + title1 := "folder1" + desc1 := "folder desc" + uid1 := util.GenerateShortUID() + f, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: title1, + Description: desc1, + OrgID: orgID, + UID: uid1, + }) + require.NoError(t, err) + + t.Cleanup(func() { + err := folderStore.Delete(context.Background(), f.UID, orgID) + require.NoError(t, err) + }) + + t.Run("should gently fail in case of bad request", func(t *testing.T) { + _, err = folderStore.Get(context.Background(), folder.GetFolderQuery{}) + require.Error(t, err) + }) + + t.Run("get folder by UID should succeed", func(t *testing.T) { + ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{ + UID: &f.UID, + OrgID: orgID, + }) + require.NoError(t, err) + assert.Equal(t, f.ID, ff.ID) + assert.Equal(t, f.UID, ff.UID) + assert.Equal(t, f.OrgID, ff.OrgID) + assert.Equal(t, f.Title, ff.Title) + assert.Equal(t, f.Description, ff.Description) + //assert.Equal(t, folder.GeneralFolderUID, ff.ParentUID) + assert.NotEmpty(t, ff.Created) + assert.NotEmpty(t, ff.Updated) + }) + + t.Run("get folder by title should succeed", func(t *testing.T) { + ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{ + Title: &f.Title, + OrgID: orgID, + }) + require.NoError(t, err) + assert.Equal(t, f.ID, ff.ID) + assert.Equal(t, f.UID, ff.UID) + assert.Equal(t, f.OrgID, ff.OrgID) + assert.Equal(t, f.Title, ff.Title) + assert.Equal(t, f.Description, ff.Description) + //assert.Equal(t, folder.GeneralFolderUID, ff.ParentUID) + assert.NotEmpty(t, ff.Created) + assert.NotEmpty(t, ff.Updated) + }) + + t.Run("get folder by title should succeed", func(t *testing.T) { + ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{ + ID: &f.ID, + }) + require.NoError(t, err) + assert.Equal(t, f.ID, ff.ID) + assert.Equal(t, f.UID, ff.UID) + assert.Equal(t, f.OrgID, ff.OrgID) + assert.Equal(t, f.Title, ff.Title) + assert.Equal(t, f.Description, ff.Description) + //assert.Equal(t, folder.GeneralFolderUID, ff.ParentUID) + assert.NotEmpty(t, ff.Created) + assert.NotEmpty(t, ff.Updated) + }) +} + +func TestIntegrationGetParents(t *testing.T) { + t.Skip("skipping until folder migration is merged") + + db := sqlstore.InitTestDB(t) + folderStore := ProvideStore(db, db.Cfg, *featuremgmt.WithFeatures()) + + orgID := CreateOrg(t, db) + + // create folder + title1 := "folder1" + desc1 := "folder desc" + uid1 := util.GenerateShortUID() + f, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: title1, + Description: desc1, + OrgID: orgID, + UID: uid1, + }) + require.NoError(t, err) + + t.Cleanup(func() { + err := folderStore.Delete(context.Background(), f.UID, orgID) + require.NoError(t, err) + }) + + t.Run("get parents of 1-st level folder should be empty", func(t *testing.T) { + parents, err := folderStore.GetParents(context.Background(), folder.GetParentsQuery{ + UID: f.UID, + OrgID: orgID, + }) + require.NoError(t, err) + require.Empty(t, parents) + }) + + t.Run("get parents of 2-st level folder should not be empty", func(t *testing.T) { + title2 := "folder2" + desc2 := "folder2 desc" + uid2 := util.GenerateShortUID() + + f, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: title2, + Description: desc2, + OrgID: orgID, + UID: uid2, + ParentUID: f.UID, + }) + require.NoError(t, err) + + parents, err := folderStore.GetParents(context.Background(), folder.GetParentsQuery{ + UID: f.UID, + OrgID: orgID, + }) + require.NoError(t, err) + parentUIDs := make([]string, 0) + for _, p := range parents { + parentUIDs = append(parentUIDs, p.UID) + } + require.Equal(t, []string{uid1}, parentUIDs) + }) +} + +func TestIntegrationGetChildren(t *testing.T) { + t.Skip("skipping until folder migration is merged") + + db := sqlstore.InitTestDB(t) + folderStore := ProvideStore(db, db.Cfg, *featuremgmt.WithFeatures()) + + orgID := CreateOrg(t, db) + + // create folder + title1 := "folder1" + desc1 := "folder desc" + uid1 := util.GenerateShortUID() + parent, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: title1, + Description: desc1, + OrgID: orgID, + UID: uid1, + }) + require.NoError(t, err) + + treeLeaves := CreateLeaves(t, folderStore, parent, 4) + + t.Cleanup(func() { + for _, uid := range treeLeaves { + err := folderStore.Delete(context.Background(), uid, orgID) + require.NoError(t, err) + } + }) + + /* + t.Run("should gently fail in case of bad request", func(t *testing.T) { + _, err := folderStore.GetChildren(context.Background(), folder.GetTreeQuery{}) + require.Error(t, err) + }) + */ + + t.Run("should successfully get all children", func(t *testing.T) { + children, err := folderStore.GetChildren(context.Background(), folder.GetTreeQuery{ + UID: parent.UID, + OrgID: orgID, + }) + require.NoError(t, err) + + childrenUIDs := make([]string, 0, len(children)) + for _, c := range children { + childrenUIDs = append(childrenUIDs, c.UID) + } + + if diff := cmp.Diff(treeLeaves, childrenUIDs); diff != "" { + t.Errorf("Result mismatch (-want +got):\n%s", diff) + } + }) + + t.Run("query with pagination should work as expected", func(t *testing.T) { + children, err := folderStore.GetChildren(context.Background(), folder.GetTreeQuery{ + UID: parent.UID, + OrgID: orgID, + Limit: 1, + Page: 1, + }) + require.NoError(t, err) + + childrenUIDs := make([]string, 0, len(children)) + for _, c := range children { + childrenUIDs = append(childrenUIDs, c.UID) + } + + if diff := cmp.Diff(treeLeaves[1:2], childrenUIDs); diff != "" { + t.Errorf("Result mismatch (-want +got):\n%s", diff) + } + + children, err = folderStore.GetChildren(context.Background(), folder.GetTreeQuery{ + UID: parent.UID, + OrgID: orgID, + Limit: 1, + Page: 2, + }) + require.NoError(t, err) + + childrenUIDs = make([]string, 0, len(children)) + for _, c := range children { + childrenUIDs = append(childrenUIDs, c.UID) + } + + if diff := cmp.Diff(treeLeaves[2:3], childrenUIDs); diff != "" { + t.Errorf("Result mismatch (-want +got):\n%s", diff) + } + + // no page is set + children, err = folderStore.GetChildren(context.Background(), folder.GetTreeQuery{ + UID: parent.UID, + OrgID: orgID, + Limit: 1, + }) + require.NoError(t, err) + + childrenUIDs = make([]string, 0, len(children)) + for _, c := range children { + childrenUIDs = append(childrenUIDs, c.UID) + } + + if diff := cmp.Diff(treeLeaves[1:2], childrenUIDs); diff != "" { + t.Errorf("Result mismatch (-want +got):\n%s", diff) + } + + // page is set but limit is not set, it should return them all + children, err = folderStore.GetChildren(context.Background(), folder.GetTreeQuery{ + UID: parent.UID, + OrgID: orgID, + Page: 1, + }) + require.NoError(t, err) + + childrenUIDs = make([]string, 0, len(children)) + for _, c := range children { + childrenUIDs = append(childrenUIDs, c.UID) + } + + if diff := cmp.Diff(treeLeaves, childrenUIDs); diff != "" { + t.Errorf("Result mismatch (-want +got):\n%s", diff) + } + }) +} + +func CreateOrg(t *testing.T, db *sqlstore.SQLStore) int64 { + t.Helper() + + orgService := orgimpl.ProvideService(db, db.Cfg) + orgID, err := orgService.GetOrCreate(context.Background(), "test-org") + require.NoError(t, err) + t.Cleanup(func() { + err = orgService.Delete(context.Background(), &org.DeleteOrgCommand{ID: orgID}) + require.NoError(t, err) + }) + + return orgID +} + +func CreateSubTree(t *testing.T, store *sqlStore, orgID int64, parentUID string, depth int, prefix string) []string { + t.Helper() + + ancestorUIDs := []string{parentUID} + for i := 0; i < depth; i++ { + parentUID := ancestorUIDs[len(ancestorUIDs)-1] + title := fmt.Sprintf("%sfolder-%d", prefix, i) + f, err := store.Create(context.Background(), folder.CreateFolderCommand{ + Title: title, + OrgID: orgID, + ParentUID: parentUID, + UID: util.GenerateShortUID(), + }) + require.NoError(t, err) + require.Equal(t, title, f.Title) + require.NotEmpty(t, f.ID) + require.NotEmpty(t, f.UID) + + parents, err := store.GetParents(context.Background(), folder.GetParentsQuery{ + UID: f.UID, + OrgID: orgID, + }) + require.NoError(t, err) + parentUIDs := []string{folder.GeneralFolderUID} + for _, p := range parents { + parentUIDs = append(parentUIDs, p.UID) + } + require.Equal(t, ancestorUIDs, parentUIDs) + + ancestorUIDs = append(ancestorUIDs, f.UID) + } + + return ancestorUIDs +} + +func CreateLeaves(t *testing.T, store *sqlStore, parent *folder.Folder, num int) []string { + t.Helper() + + leaves := make([]string, 0) + for i := 0; i < num; i++ { + f, err := store.Create(context.Background(), folder.CreateFolderCommand{ + Title: fmt.Sprintf("folder-%d", i), + UID: util.GenerateShortUID(), + OrgID: parent.OrgID, + ParentUID: parent.UID, + }) + require.NoError(t, err) + leaves = append(leaves, f.UID) + } + return leaves +} + +func assertAncestorUIDs(t *testing.T, store *sqlStore, f *folder.Folder, expected []string) { + t.Helper() + + ancestors, err := store.GetParents(context.Background(), folder.GetParentsQuery{ + UID: f.UID, + OrgID: f.OrgID, + }) + require.NoError(t, err) + actualAncestorsUIDs := []string{folder.GeneralFolderUID} + for _, f := range ancestors { + actualAncestorsUIDs = append(actualAncestorsUIDs, f.UID) + } + assert.Equal(t, expected, actualAncestorsUIDs) +} + +func assertChildrenUIDs(t *testing.T, store *sqlStore, f *folder.Folder, expected []string) { + t.Helper() + + ancestors, err := store.GetChildren(context.Background(), folder.GetTreeQuery{ + UID: f.UID, + OrgID: f.OrgID, + }) + require.NoError(t, err) + actualChildrenUIDs := make([]string, 0) + for _, f := range ancestors { + actualChildrenUIDs = append(actualChildrenUIDs, f.UID) + } + assert.Equal(t, expected, actualChildrenUIDs) +} diff --git a/pkg/services/folder/folderimpl/store.go b/pkg/services/folder/folderimpl/store.go index 2a374343592..9ba0933e4ab 100644 --- a/pkg/services/folder/folderimpl/store.go +++ b/pkg/services/folder/folderimpl/store.go @@ -9,22 +9,22 @@ import ( // store is the interface which a folder store must implement. type store interface { // Create creates a folder and returns the newly-created folder. - Create(ctx context.Context, cmd *folder.CreateFolderCommand) (*folder.Folder, error) + Create(ctx context.Context, cmd folder.CreateFolderCommand) (*folder.Folder, error) // Delete deletes a folder from the folder store. Delete(ctx context.Context, uid string, orgID int64) error // Update updates the given folder's UID, Title, and Description. // Use Move to change a dashboard's parent ID. - Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (*folder.Folder, error) + Update(ctx context.Context, cmd folder.UpdateFolderCommand) (*folder.Folder, error) // Get returns a folder. - Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folder.Folder, error) + Get(ctx context.Context, cmd folder.GetFolderQuery) (*folder.Folder, error) // GetParents returns an ordered list of parent folder of the given folder. - GetParents(ctx context.Context, cmd *folder.GetParentsQuery) ([]*folder.Folder, error) + GetParents(ctx context.Context, cmd folder.GetParentsQuery) ([]*folder.Folder, error) // GetChildren returns the set of immediate children folders (depth=1) of the // given folder. - GetChildren(ctx context.Context, cmd *folder.GetTreeQuery) ([]*folder.Folder, error) + GetChildren(ctx context.Context, cmd folder.GetTreeQuery) ([]*folder.Folder, error) } diff --git a/pkg/services/folder/folderimpl/store_fake.go b/pkg/services/folder/folderimpl/store_fake.go index 14069e4b3b2..e74c23784de 100644 --- a/pkg/services/folder/folderimpl/store_fake.go +++ b/pkg/services/folder/folderimpl/store_fake.go @@ -14,7 +14,7 @@ type FakeStore struct { var _ store = (*FakeStore)(nil) -func (f *FakeStore) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (*folder.Folder, error) { +func (f *FakeStore) Create(ctx context.Context, cmd folder.CreateFolderCommand) (*folder.Folder, error) { return f.ExpectedFolder, f.ExpectedError } @@ -22,22 +22,22 @@ func (f *FakeStore) Delete(ctx context.Context, uid string, orgID int64) error { return f.ExpectedError } -func (f *FakeStore) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (*folder.Folder, error) { +func (f *FakeStore) Update(ctx context.Context, cmd folder.UpdateFolderCommand) (*folder.Folder, error) { return f.ExpectedFolder, f.ExpectedError } -func (f *FakeStore) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*folder.Folder, error) { +func (f *FakeStore) Move(ctx context.Context, cmd folder.MoveFolderCommand) (*folder.Folder, error) { return f.ExpectedFolder, f.ExpectedError } -func (f *FakeStore) Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folder.Folder, error) { +func (f *FakeStore) Get(ctx context.Context, cmd folder.GetFolderQuery) (*folder.Folder, error) { return f.ExpectedFolder, f.ExpectedError } -func (f *FakeStore) GetParents(ctx context.Context, cmd *folder.GetParentsQuery) ([]*folder.Folder, error) { +func (f *FakeStore) GetParents(ctx context.Context, cmd folder.GetParentsQuery) ([]*folder.Folder, error) { return f.ExpectedFolders, f.ExpectedError } -func (f *FakeStore) GetChildren(ctx context.Context, cmd *folder.GetTreeQuery) ([]*folder.Folder, error) { +func (f *FakeStore) GetChildren(ctx context.Context, cmd folder.GetTreeQuery) ([]*folder.Folder, error) { return f.ExpectedFolders, f.ExpectedError } diff --git a/pkg/services/folder/foldertest/foldertest.go b/pkg/services/folder/foldertest/foldertest.go index 7780e482873..dedd9e1bf15 100644 --- a/pkg/services/folder/foldertest/foldertest.go +++ b/pkg/services/folder/foldertest/foldertest.go @@ -67,7 +67,7 @@ func modelsToFolders(m []*models.Folder) []*folder.Folder { Description: "", // model.Folder does not have a description Created: f.Created, Updated: f.Updated, - UpdatedBy: f.UpdatedBy, + //UpdatedBy: f.UpdatedBy, } } return ret diff --git a/pkg/services/folder/model.go b/pkg/services/folder/model.go index 2d27d668279..e6f0a963010 100644 --- a/pkg/services/folder/model.go +++ b/pkg/services/folder/model.go @@ -6,6 +6,11 @@ import ( "github.com/grafana/grafana/pkg/util/errutil" ) +var ErrMaximumDepthReached = errutil.NewBase(errutil.StatusBadRequest, "folder.maximum-depth-reached", errutil.WithPublicMessage("Maximum nested folder depth reached")) +var ErrBadRequest = errutil.NewBase(errutil.StatusBadRequest, "folder.bad-request") +var ErrDatabaseError = errutil.NewBase(errutil.StatusInternal, "folder.database-error") +var ErrInternal = errutil.NewBase(errutil.StatusInternal, "folder.internal") + const ( GeneralFolderUID = "general" MaxNestedFolderDepth = 8 @@ -14,10 +19,10 @@ const ( var ErrFolderNotFound = errutil.NewBase(errutil.StatusNotFound, "folder.notFound") type Folder struct { - ID int64 - OrgID int64 - UID string - ParentUID string + ID int64 `xorm:"pk autoincr 'id'"` + OrgID int64 `xorm:"org_id"` + UID string `xorm:"uid"` + ParentUID string `xorm:"parent_uid"` Title string Description string @@ -25,7 +30,8 @@ type Folder struct { Updated time.Time // TODO: validate if this field is required/relevant to folders. - UpdatedBy int64 + // currently there is no such column + // UpdatedBy int64 } // NewFolder tales a title and returns a Folder with the Created and Updated @@ -42,11 +48,11 @@ func NewFolder(title string, description string) *Folder { // CreateFolderCommand captures the information required by the folder service // to create a folder. type CreateFolderCommand struct { - UID string `json:"uid" xorm:"uid"` - OrgID int64 `json:"orgId" xorm:"org_id"` + UID string `json:"uid"` + OrgID int64 `json:"orgId"` Title string `json:"title"` Description string `json:"description"` - ParentUID string `json:"parent_uid" xorm:"parent_uid"` + ParentUID string `json:"parent_uid"` } // UpdateFolderCommand captures the information required by the folder service @@ -77,14 +83,16 @@ type DeleteFolderCommand struct { // Title. type GetFolderQuery struct { UID *string - ID *int + ID *int64 Title *string + OrgID int64 } // GetParentsQuery captures the information required by the folder service to // return a list of all parent folders of a given folder. type GetParentsQuery struct { - UID string `xorm:"uid"` + UID string `xorm:"uid"` + OrgID int64 `xorm:"org_id"` } // GetTreeCommand captures the information required by the folder service to diff --git a/pkg/services/org/orgimpl/org.go b/pkg/services/org/orgimpl/org.go index 8c4f78feb37..ca5539eb9a5 100644 --- a/pkg/services/org/orgimpl/org.go +++ b/pkg/services/org/orgimpl/org.go @@ -141,6 +141,7 @@ func (s *Service) GetOrCreate(ctx context.Context, orgName string) (int64, error orga.Name = MainOrgName orga.ID = int64(s.cfg.AutoAssignOrgId) } else { + orga = &org.Org{} orga.Name = orgName } diff --git a/pkg/services/sqlstore/migrations/folder_mig.go b/pkg/services/sqlstore/migrations/folder_mig.go index 9d08776eb9a..fd9835612dd 100644 --- a/pkg/services/sqlstore/migrations/folder_mig.go +++ b/pkg/services/sqlstore/migrations/folder_mig.go @@ -1,6 +1,8 @@ package migrations import ( + "fmt" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" ) @@ -13,12 +15,22 @@ func addFolderMigrations(mg *migrator.Migrator) { // table. The *legacy* parent folder ID, stored as folder_id in the // dashboard table, is always going to be "0" so it is safe to convert to a parent UID. mg.AddMigration("copy existing folders from dashboard table", migrator.NewRawSQLMigration( - "INSERT INTO folder (id, uid, org_id, title, parent_uid, created, updated) SELECT id, uid, org_id, title, folder_id, created, updated FROM dashboard WHERE is_folder = 1;", - ).Postgres("INSERT INTO folder (id, uid, org_id, title, parent_uid, created, updated) SELECT id, uid, org_id, title, folder_id, created, updated FROM dashboard WHERE is_folder = true;")) + "INSERT INTO folder (id, uid, org_id, title, created, updated) SELECT id, uid, org_id, title, created, updated FROM dashboard WHERE is_folder = 1;", + ).Postgres("INSERT INTO folder (id, uid, org_id, title, created, updated) SELECT id, uid, org_id, title, created, updated FROM dashboard WHERE is_folder = true;")) mg.AddMigration("Add index for parent_uid", migrator.NewAddIndexMigration(folderv1(), &migrator.Index{ Cols: []string{"parent_uid", "org_id"}, })) + + mg.AddMigration("Add unique index for folder.uid and folder.org_id", migrator.NewAddIndexMigration(folderv1(), &migrator.Index{ + Type: migrator.UniqueIndex, + Cols: []string{"uid", "org_id"}, + })) + + mg.AddMigration("Add unique index for folder.title and folder.parent_uid", migrator.NewAddIndexMigration(folderv1(), &migrator.Index{ + Type: migrator.UniqueIndex, + Cols: []string{"title", "parent_uid"}, + })) } // nolint:unused // this is temporarily unused during feature development @@ -31,7 +43,7 @@ func folderv1() migrator.Table { {Name: "org_id", Type: migrator.DB_BigInt, Nullable: false}, {Name: "title", Type: migrator.DB_NVarchar, Length: 255, Nullable: false}, {Name: "description", Type: migrator.DB_NVarchar, Length: 255, Nullable: true}, - {Name: "parent_uid", Type: migrator.DB_NVarchar, Length: 40, Default: folder.GeneralFolderUID}, + {Name: "parent_uid", Type: migrator.DB_NVarchar, Length: 40, Default: fmt.Sprintf("'%s'", folder.GeneralFolderUID)}, {Name: "created", Type: migrator.DB_DateTime, Nullable: false}, {Name: "updated", Type: migrator.DB_DateTime, Nullable: false}, }, diff --git a/pkg/util/reverse.go b/pkg/util/reverse.go new file mode 100644 index 00000000000..9e48b58d22e --- /dev/null +++ b/pkg/util/reverse.go @@ -0,0 +1,10 @@ +package util + +// Reverse returns a new slice with reversed order +func Reverse[T comparable](input []T) []T { + output := make([]T, 0, len(input)) + for i := len(input) - 1; i >= 0; i-- { + output = append(output, input[i]) + } + return output +} diff --git a/pkg/util/reverse_test.go b/pkg/util/reverse_test.go new file mode 100644 index 00000000000..2b8f9ef7111 --- /dev/null +++ b/pkg/util/reverse_test.go @@ -0,0 +1,15 @@ +package util + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestReverse(t *testing.T) { + input := []int{1, 2, 3, 4, 5} + + if diff := cmp.Diff([]int{5, 4, 3, 2, 1}, Reverse(input)); diff != "" { + t.Errorf("Result mismatch (-want +got):\n%s", diff) + } +}