Folders: Modify folder service Get() to optionally return fullpath (#81972)

* Folders: Modify Get() to optionally return fullpath

* Set FullPath to folder title if feature flag is off

* Apply suggestion from code review
This commit is contained in:
Sofia Papagiannaki
2024-02-13 19:47:46 +02:00
committed by GitHub
parent e6e9d6a782
commit 28de94f6a2
6 changed files with 176 additions and 23 deletions

View File

@@ -227,8 +227,10 @@ func (s *Service) Get(ctx context.Context, q *folder.GetFolderQuery) (*folder.Fo
}
if !s.features.IsEnabled(ctx, featuremgmt.FlagNestedFolders) {
dashFolder.Fullpath = dashFolder.Title
return dashFolder, nil
}
metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Folder).Inc()
// nolint:staticcheck
if q.ID != nil {
@@ -247,6 +249,10 @@ func (s *Service) Get(ctx context.Context, q *folder.GetFolderQuery) (*folder.Fo
f.ID = dashFolder.ID
f.Version = dashFolder.Version
if !s.features.IsEnabled(ctx, featuremgmt.FlagNestedFolders) {
f.Fullpath = f.Title // set full path to the folder title (unescaped)
}
return f, err
}

View File

@@ -1611,6 +1611,106 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
})
}
func TestFolderServiceGetFolder(t *testing.T) {
db := sqlstore.InitTestDB(t)
signedInAdminUser := user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{
orgID: {
dashboards.ActionFoldersCreate: {},
dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll},
dashboards.ActionFoldersRead: {dashboards.ScopeFoldersAll},
},
}}
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{
CanSaveValue: true,
CanViewValue: true,
})
getSvc := func(features featuremgmt.FeatureToggles) Service {
quotaService := quotatest.New(false, nil)
folderStore := ProvideDashboardFolderStore(db)
cfg := setting.NewCfg()
featuresFlagOff := featuremgmt.WithFeatures()
dashStore, err := database.ProvideDashboardStore(db, db.Cfg, featuresFlagOff, tagimpl.ProvideService(db), quotaService)
require.NoError(t, err)
nestedFolderStore := ProvideStore(db, db.Cfg)
b := bus.ProvideBus(tracing.InitializeTracerForTest())
ac := acimpl.ProvideAccessControl(cfg)
return Service{
cfg: cfg,
log: log.New("test-folder-service"),
dashboardStore: dashStore,
dashboardFolderStore: folderStore,
store: nestedFolderStore,
features: features,
bus: b,
db: db,
accessControl: ac,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
}
}
folderSvcOn := getSvc(featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders))
folderSvcOff := getSvc(featuremgmt.WithFeatures())
createCmd := folder.CreateFolderCommand{
OrgID: orgID,
ParentUID: "",
SignedInUser: &signedInAdminUser,
}
depth := 3
folders := CreateSubtreeInStore(t, folderSvcOn.store, &folderSvcOn, depth, "get/folder-", createCmd)
f := folders[1]
testCases := []struct {
name string
svc *Service
WithFullpath bool
expectedFullpath string
}{
{
name: "when flag is off",
svc: &folderSvcOff,
expectedFullpath: f.Title,
},
{
name: "when flag is on and WithFullpath is false",
svc: &folderSvcOn,
WithFullpath: false,
expectedFullpath: "",
},
{
name: "when flag is on and WithFullpath is true",
svc: &folderSvcOn,
WithFullpath: true,
expectedFullpath: "get\\/folder-folder-0/get\\/folder-folder-1",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
q := folder.GetFolderQuery{
OrgID: orgID,
UID: &f.UID,
WithFullpath: tc.WithFullpath,
SignedInUser: &signedInAdminUser,
}
fldr, err := tc.svc.Get(context.Background(), &q)
require.NoError(t, err)
require.Equal(t, f.UID, fldr.UID)
require.Equal(t, tc.expectedFullpath, fldr.Fullpath)
})
}
}
func TestFolderServiceGetFolders(t *testing.T) {
db := sqlstore.InitTestDB(t)
quotaService := quotatest.New(false, nil)
@@ -1687,7 +1787,7 @@ func TestFolderServiceGetFolders(t *testing.T) {
})
}
func CreateSubtreeInStore(t *testing.T, store *sqlStore, service *Service, depth int, prefix string, cmd folder.CreateFolderCommand) []*folder.Folder {
func CreateSubtreeInStore(t *testing.T, store store, service *Service, depth int, prefix string, cmd folder.CreateFolderCommand) []*folder.Folder {
t.Helper()
folders := make([]*folder.Folder, 0, depth)

View File

@@ -171,27 +171,55 @@ func (ss *sqlStore) Update(ctx context.Context, cmd folder.UpdateFolderCommand)
return foldr.WithURL(), err
}
// If WithFullpath is true it computes also the full path of a folder.
// The full path is a string that contains the titles of all parent folders separated by a slash.
// For example, if the folder structure is:
//
// A
// └── B
// └── C
//
// The full path of C is "A/B/C".
// The full path of B is "A/B".
// The full path of A is "A".
// If a folder contains a slash in its title, it is escaped with a backslash.
// For example, if the folder structure is:
//
// A
// └── B/C
//
// The full path of C is "A/B\/C".
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 := false
var err error
s := strings.Builder{}
s.WriteString("SELECT *")
if q.WithFullpath {
s.WriteString(fmt.Sprintf(`, %s AS fullpath`, getFullpathSQL(ss.db.GetDialect())))
}
s.WriteString(" FROM folder f0")
if q.WithFullpath {
s.WriteString(getFullpathJoinsSQL())
}
switch {
case q.UID != nil:
exists, err = sess.SQL("SELECT * FROM folder WHERE uid = ? AND org_id = ?", q.UID, q.OrgID).Get(foldr)
s.WriteString(" WHERE f0.uid = ? AND f0.org_id = ?")
exists, err = sess.SQL(s.String(), q.UID, q.OrgID).Get(foldr)
// nolint:staticcheck
case q.ID != nil:
s.WriteString(" WHERE f0.id = ?")
metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Folder).Inc()
exists, err = sess.SQL("SELECT * FROM folder WHERE id = ?", q.ID).Get(foldr)
exists, err = sess.SQL(s.String(), q.ID).Get(foldr)
case q.Title != nil:
s := strings.Builder{}
s.WriteString("SELECT * FROM folder WHERE title = ? AND org_id = ?")
s.WriteString(" WHERE f0.title = ? AND f0.org_id = ?")
args := []any{*q.Title, q.OrgID}
if q.ParentUID != nil {
s.WriteString(" AND parent_uid = ?")
s.WriteString(" AND f0.parent_uid = ?")
args = append(args, *q.ParentUID)
} else {
s.WriteString(" AND parent_uid IS NULL")
s.WriteString(" AND f0.parent_uid IS NULL")
}
exists, err = sess.SQL(s.String(), args...).Get(foldr)
default:
@@ -207,6 +235,7 @@ func (ss *sqlStore) Get(ctx context.Context, q folder.GetFolderQuery) (*folder.F
return nil
})
foldr.Fullpath = strings.TrimLeft(foldr.Fullpath, "/")
return foldr.WithURL(), err
}
@@ -274,15 +303,16 @@ func (ss *sqlStore) GetChildren(ctx context.Context, q folder.GetChildrenQuery)
args = append(args, q.UID, q.OrgID)
}
if q.FolderUIDs != nil {
sql.WriteString(" AND uid IN (?")
for range q.FolderUIDs[1:] {
sql.WriteString(", ?")
}
sql.WriteString(")")
for _, uid := range q.FolderUIDs {
if len(q.FolderUIDs) > 0 {
sql.WriteString(" AND uid IN (")
for i, uid := range q.FolderUIDs {
if i > 0 {
sql.WriteString(", ")
}
sql.WriteString("?")
args = append(args, uid)
}
sql.WriteString(")")
}
sql.WriteString(" ORDER BY title ASC")

View File

@@ -3,6 +3,7 @@ package folderimpl
import (
"context"
"fmt"
"path"
"slices"
"sort"
"testing"
@@ -391,11 +392,7 @@ func TestIntegrationGet(t *testing.T) {
UID: util.GenerateShortUID(),
ParentUID: f.UID,
})
t.Cleanup(func() {
err := folderStore.Delete(context.Background(), []string{f.UID}, orgID)
require.NoError(t, err)
})
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{})
@@ -466,6 +463,24 @@ func TestIntegrationGet(t *testing.T) {
assert.NotEmpty(t, ff.Updated)
assert.NotEmpty(t, ff.URL)
})
t.Run("get folder with fullpath should set fullpath as expected", func(t *testing.T) {
ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{
UID: &subfolderWithSameName.UID,
OrgID: orgID,
WithFullpath: true,
})
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, path.Join(f.Title, subfolderWithSameName.Title), ff.Fullpath)
assert.Equal(t, f.UID, ff.ParentUID)
assert.NotEmpty(t, ff.Created)
assert.NotEmpty(t, ff.Updated)
assert.NotEmpty(t, ff.URL)
})
}
func TestIntegrationGetParents(t *testing.T) {

View File

@@ -147,10 +147,11 @@ type DeleteFolderCommand struct {
type GetFolderQuery struct {
UID *string
// Deprecated: use FolderUID instead
ID *int64
Title *string
ParentUID *string
OrgID int64
ID *int64
Title *string
ParentUID *string
OrgID int64
WithFullpath bool
SignedInUser identity.Requester `json:"-"`
}

View File

@@ -18,6 +18,7 @@ type Service interface {
// 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.
// If WithFullpath is true it computes also the full path of a folder.
Get(ctx context.Context, q *GetFolderQuery) (*Folder, error)
// Update is used to update a folder's UID, Title and Description. To change