From 98fd3e8fe91ff8c1bb54062a47f7115e1b874306 Mon Sep 17 00:00:00 2001 From: owensmallwood Date: Wed, 24 Sep 2025 04:09:20 -0600 Subject: [PATCH] Unified Storage - Fix bug when cleaning up legacy on create (#111511) fix bug - we should be calling s.store.Get() instead of s.Get(). Adds regression test. --- pkg/services/folder/folderimpl/folder.go | 2 +- pkg/services/folder/folderimpl/folder_test.go | 52 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index a709ecd1e40..f56e682283b 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -1227,7 +1227,7 @@ func (s *Service) nestedFolderDelete(ctx context.Context, cmd *folder.DeleteFold return descendantUIDs, folder.ErrBadRequest.Errorf("missing signed in user") } - _, err := s.Get(ctx, &folder.GetFolderQuery{ + _, err := s.store.Get(ctx, folder.GetFolderQuery{ UID: &cmd.UID, OrgID: cmd.OrgID, SignedInUser: cmd.SignedInUser, diff --git a/pkg/services/folder/folderimpl/folder_test.go b/pkg/services/folder/folderimpl/folder_test.go index 9329e87b16d..a3d672ca6f0 100644 --- a/pkg/services/folder/folderimpl/folder_test.go +++ b/pkg/services/folder/folderimpl/folder_test.go @@ -1,11 +1,15 @@ package folderimpl import ( + "context" "encoding/json" "fmt" + "log/slog" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace/noop" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/user" @@ -127,3 +131,51 @@ func TestSplitFullpath(t *testing.T) { }) } } + +func TestNestedFolderDeleteUsesCorrectStorage(t *testing.T) { + // This is a regression test for a bug where nestedFolderDelete was calling s.Get() + // (which goes through the API server/unified storage) instead of s.store.Get() + + ctx := context.Background() + tracer := noop.NewTracerProvider().Tracer("TestNestedFolderDeleteUsesCorrectStorage") + + testUID := "test-folder-uid" + testOrgID := int64(1) + testUser := &user.SignedInUser{UserID: 1, OrgID: testOrgID} + + testFolder := &folder.Folder{ + UID: testUID, + OrgID: testOrgID, + Title: "Test Folder", + } + + // Create fake stores - the direct store will have the folder, unified store will not + store := folder.NewFakeStore() + store.ExpectedFolder = testFolder // Get will return the folder + store.ExpectedFolders = []*folder.Folder{} // GetDescendants returns empty list + store.ExpectedError = nil // No errors + + unifiedStore := folder.NewFakeStore() + unifiedStore.ExpectedFolder = nil // This store doesn't have the folder + unifiedStore.ExpectedError = folder.ErrFolderNotFound // Would return not found + + // Set up the service with both stores + service := &Service{ + store: store, + unifiedStore: unifiedStore, + tracer: tracer, + log: slog.Default(), + } + + cmd := &folder.DeleteFolderCommand{ + UID: testUID, + OrgID: testOrgID, + SignedInUser: testUser, + } + + descendants, err := service.nestedFolderDelete(ctx, cmd) + + require.NoError(t, err, "nestedFolderDelete should succeed") + assert.Empty(t, descendants, "Expected no descendants for this test") + assert.True(t, store.DeleteCalled, "store Delete should have been called") +}