feat(unified-storage): build full path internally when getting folders (#105316)

pkg/services/folder/folderimpl/unifiedstore.go

@@ -2,9 +2,7 @@ package folderimpl
 
 import (
 	"context"
-	"errors"
 	"fmt"
-	"net/http"
 	"strings"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -153,13 +151,11 @@ func (ss *FolderUnifiedStoreImpl) Get(ctx context.Context, q folder.GetFolderQue
 func (ss *FolderUnifiedStoreImpl) GetParents(ctx context.Context, q folder.GetParentsQuery) ([]*folder.Folder, error) {
 	hits := []*folder.Folder{}
 
-	parentUid := q.UID
-
-	for parentUid != "" {
-		folder, err := ss.Get(ctx, folder.GetFolderQuery{UID: &parentUid, OrgID: q.OrgID})
+	parentUID := q.UID
+	for parentUID != "" {
+		folder, err := ss.Get(ctx, folder.GetFolderQuery{UID: &parentUID, OrgID: q.OrgID})
 		if err != nil {
-			var statusError *apierrors.StatusError
-			if errors.As(err, &statusError) && statusError.ErrStatus.Code == http.StatusForbidden {
+			if apierrors.IsForbidden(err) {
 				// If we get a Forbidden error when requesting the parent folder, it means the user does not have access
 				// to it, nor its parents. So we can stop looping
 				break
@@ -167,15 +163,15 @@ func (ss *FolderUnifiedStoreImpl) GetParents(ctx context.Context, q folder.GetPa
 			return nil, err
 		}
 
-		parentUid = folder.ParentUID
+		parentUID = folder.ParentUID
 		hits = append(hits, folder)
 	}
 
-	if len(hits) > 0 {
-		return util.Reverse(hits[1:]), nil
+	if len(hits) == 0 {
+		return hits, nil
 	}
 
-	return hits, nil
+	return util.Reverse(hits[1:]), nil
 }
 
 func (ss *FolderUnifiedStoreImpl) GetChildren(ctx context.Context, q folder.GetChildrenQuery) ([]*folder.FolderReference, error) {
@@ -340,46 +336,37 @@ func (ss *FolderUnifiedStoreImpl) GetFolders(ctx context.Context, q folder.GetFo
 		return nil, err
 	}
 
-	m := map[string]*folder.Folder{}
-	for _, f := range folders {
-		if (q.WithFullpath || q.WithFullpathUIDs) && f.Fullpath == "" {
-			parents, err := ss.GetParents(ctx, folder.GetParentsQuery{UID: f.UID, OrgID: q.OrgID})
-			if err != nil {
-				return nil, fmt.Errorf("failed to get parents for folder %s: %w", f.UID, err)
-			}
-			// If we don't have a parent, we just return the current folder as the full path
-			f.Fullpath, f.FullpathUIDs = computeFullPath(append(parents, f))
-		}
-
-		m[f.UID] = f
+	filters := make(map[string]struct{}, len(q.UIDs))
+	for _, uid := range q.UIDs {
+		filters[uid] = struct{}{}
 	}
 
-	hits := []*folder.Folder{}
-
-	if len(q.UIDs) > 0 {
-		//return only the specified q.UIDs
-		for _, uid := range q.UIDs {
-			f, ok := m[uid]
-			if ok {
-				hits = append(hits, f)
-			}
+	folderMap := make(map[string]*folder.Folder)
+	relations := make(map[string]string)
+	if q.WithFullpath || q.WithFullpathUIDs {
+		for _, folder := range folders {
+			folderMap[folder.UID] = folder
+			relations[folder.UID] = folder.ParentUID
 		}
-
-		return hits, nil
 	}
 
-	/*
-		if len(q.AncestorUIDs) > 0 {
-			// TODO
-			//return all nodes under those ancestors, requires building a tree
+	hits := make([]*folder.Folder, 0, len(folders))
+	for _, f := range folders {
+		if shouldSkipFolder(f, filters) {
+			continue
+		}
+
+		if (q.WithFullpath || q.WithFullpathUIDs) && f.Fullpath == "" {
+			buildFolderFullPaths(f, relations, folderMap)
 		}
-	*/
 
-	//return everything
-	for _, f := range m {
 		hits = append(hits, f)
 	}
 
+	// TODO: return all nodes under those ancestors, requires building a tree
+	// if len(q.AncestorUIDs) > 0 {
+	// }
+
 	return hits, nil
 }
 
@@ -490,3 +477,42 @@ func computeFullPath(parents []*folder.Folder) (string, string) {
 	}
 	return strings.Join(fullpath, "/"), strings.Join(fullpathUIDs, "/")
 }
+
+func buildFolderFullPaths(f *folder.Folder, relations map[string]string, folderMap map[string]*folder.Folder) {
+	titles := make([]string, 0)
+	uids := make([]string, 0)
+
+	titles = append(titles, f.Title)
+	uids = append(uids, f.UID)
+
+	currentUID := f.UID
+	for currentUID != "" {
+		parentUID, exists := relations[currentUID]
+		if !exists {
+			break
+		}
+
+		if parentUID == "" {
+			break
+		}
+
+		parentFolder, exists := folderMap[parentUID]
+		if !exists {
+			break
+		}
+		titles = append(titles, parentFolder.Title)
+		uids = append(uids, parentFolder.UID)
+		currentUID = parentFolder.UID
+	}
+
+	f.Fullpath = strings.Join(util.Reverse(titles), "/")
+	f.FullpathUIDs = strings.Join(util.Reverse(uids), "/")
+}
+
+func shouldSkipFolder(f *folder.Folder, filterUIDs map[string]struct{}) bool {
+	if len(filterUIDs) == 0 {
+		return false
+	}
+	_, exists := filterUIDs[f.UID]
+	return !exists
+}

pkg/services/folder/folderimpl/unifiedstore_test.go

@@ -174,6 +174,7 @@ func TestGetParents(t *testing.T) {
 		require.Len(t, result, 1)
 		require.Equal(t, "parenttwo", result[0].UID)
 	})
+
 	t.Run("should stop if parent folder is not found", func(t *testing.T) {
 		mockCli.On("Get", mock.Anything, "parentone", orgID, mock.Anything, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "folders.folder.grafana.app", Resource: "folder"}, "parentone")).Once()
 
@@ -439,3 +440,443 @@ func TestGetChildren(t *testing.T) {
 		require.Equal(t, "folder3", result[1].UID)
 	})
 }
+
+func TestGetFolders(t *testing.T) {
+	type args struct {
+		ctx context.Context
+		q   folder.GetFoldersFromStoreQuery
+	}
+	tests := []struct {
+		name    string
+		args    args
+		mock    func(mockCli *client.MockK8sHandler)
+		want    []*folder.Folder
+		wantErr bool
+	}{
+		{
+			name: "should return all folders from k8s",
+			args: args{
+				ctx: context.Background(),
+				q: folder.GetFoldersFromStoreQuery{
+					GetFoldersQuery: folder.GetFoldersQuery{
+						OrgID: orgID,
+					},
+				},
+			},
+			mock: func(mockCli *client.MockK8sHandler) {
+				mockCli.On("List", mock.Anything, orgID, metav1.ListOptions{
+					Limit:    100000,
+					TypeMeta: metav1.TypeMeta{},
+				}).Return(&unstructured.UnstructuredList{
+					Items: []unstructured.Unstructured{
+						{
+							Object: map[string]interface{}{
+								"metadata": map[string]interface{}{
+									"name": "folder1",
+									"uid":  "folder1",
+								},
+								"spec": map[string]interface{}{
+									"title": "folder1",
+								},
+							},
+						},
+						{
+							Object: map[string]interface{}{
+								"metadata": map[string]interface{}{
+									"name": "folder2",
+									"uid":  "folder2",
+								},
+								"spec": map[string]interface{}{
+									"title": "folder2",
+								},
+							},
+						},
+					},
+				}, nil).Once()
+			},
+			want: []*folder.Folder{
+				{
+					UID:   "folder1",
+					Title: "folder1",
+					OrgID: orgID,
+				},
+				{
+					UID:   "folder2",
+					Title: "folder2",
+					OrgID: orgID,
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "should return folders from k8s by uid",
+			args: args{
+				ctx: context.Background(),
+				q: folder.GetFoldersFromStoreQuery{
+					GetFoldersQuery: folder.GetFoldersQuery{
+						OrgID: orgID,
+						UIDs:  []string{"folder1", "folder2"},
+					},
+				},
+			},
+			mock: func(mockCli *client.MockK8sHandler) {
+				mockCli.On("List", mock.Anything, orgID, metav1.ListOptions{
+					Limit:    100000,
+					TypeMeta: metav1.TypeMeta{},
+				}).Return(&unstructured.UnstructuredList{
+					Items: []unstructured.Unstructured{
+						{
+							Object: map[string]interface{}{
+								"metadata": map[string]interface{}{
+									"name": "folder1",
+									"uid":  "folder1",
+								},
+								"spec": map[string]interface{}{
+									"title": "folder1",
+								},
+							},
+						},
+						{
+							Object: map[string]interface{}{
+								"metadata": map[string]interface{}{
+									"name": "folder2",
+									"uid":  "folder2",
+								},
+								"spec": map[string]interface{}{
+									"title": "folder2",
+								},
+							},
+						},
+						{
+							Object: map[string]interface{}{
+								"metadata": map[string]interface{}{
+									"name": "folder3",
+									"uid":  "folder3",
+								},
+								"spec": map[string]interface{}{
+									"title": "folder3",
+								},
+							},
+						},
+					},
+				}, nil).Once()
+			},
+			want: []*folder.Folder{
+				{
+					UID:   "folder1",
+					Title: "folder1",
+					OrgID: orgID,
+				},
+				{
+					UID:   "folder2",
+					Title: "folder2",
+					OrgID: orgID,
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "should return all folders from k8s with fullpath enabled",
+			args: args{
+				ctx: context.Background(),
+				q: folder.GetFoldersFromStoreQuery{
+					GetFoldersQuery: folder.GetFoldersQuery{
+						OrgID:        orgID,
+						WithFullpath: true,
+					},
+				},
+			},
+			mock: func(mockCli *client.MockK8sHandler) {
+				mockCli.On("List", mock.Anything, orgID, metav1.ListOptions{
+					Limit:         100000,
+					TypeMeta:      metav1.TypeMeta{},
+					LabelSelector: "grafana.app/fullpath=true",
+				}).Return(&unstructured.UnstructuredList{
+					Items: []unstructured.Unstructured{
+						{
+							Object: map[string]interface{}{
+								"metadata": map[string]interface{}{
+									"name": "root1",
+									"uid":  "root1",
+								},
+								"spec": map[string]interface{}{
+									"title": "root1",
+								},
+							},
+						},
+						{
+							Object: map[string]interface{}{
+								"metadata": map[string]interface{}{
+									"name": "root2",
+									"uid":  "root2",
+								},
+								"spec": map[string]interface{}{
+									"title": "root2",
+								},
+							},
+						},
+					},
+				}, nil).Once()
+			},
+			want: []*folder.Folder{
+				{
+					UID:          "root1",
+					Title:        "root1",
+					OrgID:        orgID,
+					Fullpath:     "root1",
+					FullpathUIDs: "root1",
+				},
+				{
+					UID:          "root2",
+					Title:        "root2",
+					OrgID:        orgID,
+					Fullpath:     "root2",
+					FullpathUIDs: "root2",
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "should return folders from k8s by uid with fullpath enabled",
+			args: args{
+				ctx: context.Background(),
+				q: folder.GetFoldersFromStoreQuery{
+					GetFoldersQuery: folder.GetFoldersQuery{
+						OrgID:        orgID,
+						UIDs:         []string{"folder1", "folder2"},
+						WithFullpath: true,
+					},
+				},
+			},
+			mock: func(mockCli *client.MockK8sHandler) {
+				mockCli.On("List", mock.Anything, orgID, metav1.ListOptions{
+					Limit:         100000,
+					TypeMeta:      metav1.TypeMeta{},
+					LabelSelector: "grafana.app/fullpath=true",
+				}).Return(&unstructured.UnstructuredList{
+					Items: []unstructured.Unstructured{
+						{
+							Object: map[string]interface{}{
+								"metadata": map[string]interface{}{
+									"name": "parentcommon",
+									"uid":  "parentcommon",
+								},
+								"spec": map[string]interface{}{
+									"title": "parentcommon",
+								},
+							},
+						},
+						{
+							Object: map[string]interface{}{
+								"metadata": map[string]interface{}{
+									"name":        "folder1",
+									"uid":         "folder1",
+									"annotations": map[string]interface{}{"grafana.app/folder": "parentcommon"},
+								},
+								"spec": map[string]interface{}{
+									"title": "folder1",
+								},
+							},
+						},
+						{
+							Object: map[string]interface{}{
+								"metadata": map[string]interface{}{
+									"name":        "folder2",
+									"uid":         "folder2",
+									"annotations": map[string]interface{}{"grafana.app/folder": "parentcommon"},
+								},
+								"spec": map[string]interface{}{
+									"title": "folder2",
+								},
+							},
+						},
+					},
+				}, nil).Once()
+			},
+			want: []*folder.Folder{
+				{
+					UID:          "folder1",
+					Title:        "folder1",
+					OrgID:        orgID,
+					Fullpath:     "parentcommon/folder1",
+					FullpathUIDs: "parentcommon/folder1",
+				},
+				{
+					UID:          "folder2",
+					Title:        "folder2",
+					OrgID:        orgID,
+					Fullpath:     "parentcommon/folder2",
+					FullpathUIDs: "parentcommon/folder2",
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "should return error if k8s returns error",
+			args: args{
+				ctx: context.Background(),
+				q: folder.GetFoldersFromStoreQuery{
+					GetFoldersQuery: folder.GetFoldersQuery{
+						OrgID: orgID,
+						UIDs:  []string{"folder1", "folder2"},
+					},
+				},
+			},
+			mock: func(mockCli *client.MockK8sHandler) {
+				mockCli.On("List", mock.Anything, orgID, metav1.ListOptions{
+					Limit:    100000,
+					TypeMeta: metav1.TypeMeta{},
+				}).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "folders.folder.grafana.app", Resource: "folder"}, "folder1")).Once()
+			},
+			want:    nil,
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockCLI := new(client.MockK8sHandler)
+			tt.mock(mockCLI)
+			ss := &FolderUnifiedStoreImpl{
+				k8sclient:   mockCLI,
+				userService: usertest.NewUserServiceFake(),
+			}
+			got, err := ss.GetFolders(tt.args.ctx, tt.args.q)
+			require.Equal(t, tt.wantErr, err != nil, "GetFolders() error = %v, wantErr %v", err, tt.wantErr)
+			if !tt.wantErr {
+				require.Len(t, got, len(tt.want), "GetFolders() = %v, want %v", got, tt.want)
+				for i, folder := range got {
+					require.Equal(t, tt.want[i].UID, folder.UID, "GetFolders() = %v, want %v", got, tt.want)
+					require.Equal(t, tt.want[i].OrgID, folder.OrgID, "GetFolders() = %v, want %v", got, tt.want)
+					require.Equal(t, tt.want[i].Fullpath, folder.Fullpath, "GetFolders() = %v, want %v", got, tt.want)
+					require.Equal(t, tt.want[i].FullpathUIDs, folder.FullpathUIDs, "GetFolders() = %v, want %v", got, tt.want)
+				}
+			}
+		})
+	}
+}
+
+func TestBuildFolderFullPaths(t *testing.T) {
+	type args struct {
+		f         *folder.Folder
+		relations map[string]string
+		folderMap map[string]*folder.Folder
+	}
+	tests := []struct {
+		name string
+		args args
+		want *folder.Folder
+	}{
+		{
+			name: "should build full path for a folder with no parents",
+			args: args{
+				f: &folder.Folder{
+					Title: "Root",
+					UID:   "root-uid",
+				},
+				relations: map[string]string{},
+				folderMap: map[string]*folder.Folder{},
+			},
+			want: &folder.Folder{
+				Title:        "Root",
+				UID:          "root-uid",
+				Fullpath:     "Root",
+				FullpathUIDs: "root-uid",
+			},
+		},
+		{
+			name: "should build full path for a folder with one parent",
+			args: args{
+				f: &folder.Folder{
+					Title:     "Child",
+					UID:       "child-uid",
+					ParentUID: "parent-uid",
+				},
+				relations: map[string]string{
+					"child-uid": "parent-uid",
+				},
+				folderMap: map[string]*folder.Folder{
+					"parent-uid": {
+						Title: "Parent",
+						UID:   "parent-uid",
+					},
+				},
+			},
+			want: &folder.Folder{
+				Title:        "Child",
+				UID:          "child-uid",
+				ParentUID:    "parent-uid",
+				Fullpath:     "Parent/Child",
+				FullpathUIDs: "parent-uid/child-uid",
+			},
+		},
+		{
+			name: "should build full path for a folder with multiple parents",
+			args: args{
+				f: &folder.Folder{
+					Title:     "Child",
+					UID:       "child-uid",
+					ParentUID: "parent-uid",
+				},
+				relations: map[string]string{
+					"child-uid":       "parent-uid",
+					"parent-uid":      "grandparent-uid",
+					"grandparent-uid": "",
+				},
+				folderMap: map[string]*folder.Folder{
+					"child-uid": {
+						Title:     "Child",
+						UID:       "child-uid",
+						ParentUID: "parent-uid",
+					},
+					"parent-uid": {
+						Title:     "Parent",
+						UID:       "parent-uid",
+						ParentUID: "grandparent-uid",
+					},
+					"grandparent-uid": {
+						Title: "Grandparent",
+						UID:   "grandparent-uid",
+					},
+				},
+			},
+			want: &folder.Folder{
+				Title:        "Child",
+				UID:          "child-uid",
+				ParentUID:    "parent-uid",
+				Fullpath:     "Grandparent/Parent/Child",
+				FullpathUIDs: "grandparent-uid/parent-uid/child-uid",
+			},
+		},
+		{
+			name: "should build full path for a folder with no parents in the map",
+			args: args{
+				f: &folder.Folder{
+					Title:     "Child",
+					UID:       "child-uid",
+					ParentUID: "parent-uid",
+				},
+				relations: map[string]string{
+					"child-uid": "parent-uid",
+				},
+				folderMap: map[string]*folder.Folder{},
+			},
+			want: &folder.Folder{
+				Title:        "Child",
+				UID:          "child-uid",
+				ParentUID:    "parent-uid",
+				Fullpath:     "Child",
+				FullpathUIDs: "child-uid",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			buildFolderFullPaths(tt.args.f, tt.args.relations, tt.args.folderMap)
+			require.Equal(t, tt.want.Fullpath, tt.args.f.Fullpath, "BuildFolderFullPaths() = %v, want %v", tt.args.f.Fullpath, tt.want.Fullpath)
+			require.Equal(t, tt.want.FullpathUIDs, tt.args.f.FullpathUIDs, "BuildFolderFullPaths() = %v, want %v", tt.args.f.FullpathUIDs, tt.want.FullpathUIDs)
+			require.Equal(t, tt.want.Title, tt.args.f.Title, "BuildFolderFullPaths() = %v, want %v", tt.args.f.Title, tt.want.Title)
+			require.Equal(t, tt.want.UID, tt.args.f.UID, "BuildFolderFullPaths() = %v, want %v", tt.args.f.UID, tt.want.UID)
+			require.Equal(t, tt.want.ParentUID, tt.args.f.ParentUID, "BuildFolderFullPaths() = %v, want %v", tt.args.f.ParentUID, tt.want.ParentUID)
+		})
+	}
+}