From 142340e0ff035c01c66143c44b457928ba153195 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Philippe=20Qu=C3=A9m=C3=A9ner?= Date: Mon, 10 Nov 2025 20:37:02 +0100 Subject: [PATCH] refactor(folders): use set to detect circular references (#113665) --- .../folder/folderimpl/unifiedstore.go | 9 +- .../folder/folderimpl/unifiedstore_test.go | 135 ++++++++++++++++++ 2 files changed, 139 insertions(+), 5 deletions(-) diff --git a/pkg/services/folder/folderimpl/unifiedstore.go b/pkg/services/folder/folderimpl/unifiedstore.go index 8c30024eefb..4c76b219873 100644 --- a/pkg/services/folder/folderimpl/unifiedstore.go +++ b/pkg/services/folder/folderimpl/unifiedstore.go @@ -568,14 +568,13 @@ func buildFolderFullPaths(f *folder.Folder, relations map[string]string, folderM titles = append(titles, f.Title) uids = append(uids, f.UID) - i := 0 + seen := make(map[string]bool) currentUID := f.UID for currentUID != "" { - // This is just a circuit breaker to prevent infinite loops. We should never reach this limit. - if i > 1000 { - return fmt.Errorf("folder depth exceeds the maximum allowed depth, You might have a circular reference") + if seen[currentUID] { + return folder.ErrCircularReference.Errorf("circular reference detected for folder %s", currentUID) } - i++ + seen[currentUID] = true parentUID, exists := relations[currentUID] if !exists { break diff --git a/pkg/services/folder/folderimpl/unifiedstore_test.go b/pkg/services/folder/folderimpl/unifiedstore_test.go index 5ffc1d12947..b31db85227e 100644 --- a/pkg/services/folder/folderimpl/unifiedstore_test.go +++ b/pkg/services/folder/folderimpl/unifiedstore_test.go @@ -891,6 +891,141 @@ func TestBuildFolderFullPaths(t *testing.T) { } } +func TestBuildFolderFullPaths_CircularReference(t *testing.T) { + type args struct { + f *folder.Folder + relations map[string]string + folderMap map[string]*folder.Folder + } + tests := []struct { + name string + args args + expectedErr string + }{ + { + name: "should detect direct circular reference (A -> B -> A)", + args: args{ + f: &folder.Folder{ + Title: "FolderA", + UID: "folder-a", + ParentUID: "folder-b", + }, + relations: map[string]string{ + "folder-a": "folder-b", + "folder-b": "folder-a", // circular: B points back to A + }, + folderMap: map[string]*folder.Folder{ + "folder-a": { + Title: "FolderA", + UID: "folder-a", + ParentUID: "folder-b", + }, + "folder-b": { + Title: "FolderB", + UID: "folder-b", + ParentUID: "folder-a", + }, + }, + }, + expectedErr: "circular reference detected", + }, + { + name: "should detect self-reference (A -> A)", + args: args{ + f: &folder.Folder{ + Title: "FolderA", + UID: "folder-a", + ParentUID: "folder-a", // points to itself + }, + relations: map[string]string{ + "folder-a": "folder-a", + }, + folderMap: map[string]*folder.Folder{ + "folder-a": { + Title: "FolderA", + UID: "folder-a", + ParentUID: "folder-a", + }, + }, + }, + expectedErr: "circular reference detected", + }, + { + name: "should detect longer circular reference (A -> B -> C -> A)", + args: args{ + f: &folder.Folder{ + Title: "FolderA", + UID: "folder-a", + ParentUID: "folder-b", + }, + relations: map[string]string{ + "folder-a": "folder-b", + "folder-b": "folder-c", + "folder-c": "folder-a", // circular: C points back to A + }, + folderMap: map[string]*folder.Folder{ + "folder-a": { + Title: "FolderA", + UID: "folder-a", + ParentUID: "folder-b", + }, + "folder-b": { + Title: "FolderB", + UID: "folder-b", + ParentUID: "folder-c", + }, + "folder-c": { + Title: "FolderC", + UID: "folder-c", + ParentUID: "folder-a", + }, + }, + }, + expectedErr: "circular reference detected", + }, + { + name: "should detect circular reference starting from middle (B in A -> B -> C -> A)", + args: args{ + f: &folder.Folder{ + Title: "FolderB", + UID: "folder-b", + ParentUID: "folder-c", + }, + relations: map[string]string{ + "folder-a": "folder-b", + "folder-b": "folder-c", + "folder-c": "folder-a", + }, + folderMap: map[string]*folder.Folder{ + "folder-a": { + Title: "FolderA", + UID: "folder-a", + ParentUID: "folder-b", + }, + "folder-b": { + Title: "FolderB", + UID: "folder-b", + ParentUID: "folder-c", + }, + "folder-c": { + Title: "FolderC", + UID: "folder-c", + ParentUID: "folder-a", + }, + }, + }, + expectedErr: "circular reference detected", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := buildFolderFullPaths(tt.args.f, tt.args.relations, tt.args.folderMap) + require.Error(t, err) + require.Contains(t, err.Error(), tt.expectedErr) + }) + } +} + func TestList(t *testing.T) { type args struct { ctx context.Context