Chore: Move folder service into a separate package (grafana#56591)

* Chore: move folder service interface into a separate package * copy implementation into a standalone package * move implementation and tests to the new folder package * remove leftovers from wire * add test doubles for folder service * fix tests in library panels/elements * fix provideservice in ngalert
KFearsoff · Oct 10, 2022 · 53baecd · 53baecd
1 parent eb077db
commit 53baecd
Show file tree

Hide file tree

Showing 18 changed files with 148 additions and 308 deletions.
diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go
@@ -30,6 +30,7 @@ import (
 	dashver "github.com/grafana/grafana/pkg/services/dashboardversion"
 	"github.com/grafana/grafana/pkg/services/dashboardversion/dashvertest"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	"github.com/grafana/grafana/pkg/services/folder"
 	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/services/libraryelements"
 	"github.com/grafana/grafana/pkg/services/live"
@@ -1017,7 +1018,7 @@ func callPostDashboardShouldReturnSuccess(sc *scenarioContext) {
 	assert.Equal(sc.t, 200, sc.resp.Code)
 }
 
-func postDashboardScenario(t *testing.T, desc string, url string, routePattern string, cmd models.SaveDashboardCommand, dashboardService dashboards.DashboardService, folderService dashboards.FolderService, fn scenarioFunc) {
+func postDashboardScenario(t *testing.T, desc string, url string, routePattern string, cmd models.SaveDashboardCommand, dashboardService dashboards.DashboardService, folderService folder.Service, fn scenarioFunc) {
 	t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
 		cfg := setting.NewCfg()
 		hs := HTTPServer{

diff --git a/pkg/api/folder_permission_test.go b/pkg/api/folder_permission_test.go
@@ -17,6 +17,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	service "github.com/grafana/grafana/pkg/services/dashboards/service"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	"github.com/grafana/grafana/pkg/services/folder/foldertest"
 	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
@@ -26,8 +27,7 @@ import (
 func TestFolderPermissionAPIEndpoint(t *testing.T) {
 	settings := setting.NewCfg()
 
-	folderService := &dashboards.FakeFolderService{}
-	defer folderService.AssertExpectations(t)
+	folderService := &foldertest.FakeService{}
 
 	dashboardStore := &dashboards.FakeDashboardStore{}
 	defer dashboardStore.AssertExpectations(t)
@@ -50,7 +50,10 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
 	}
 
 	t.Run("Given folder not exists", func(t *testing.T) {
-		folderService.On("GetFolderByUID", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, dashboards.ErrFolderNotFound).Twice()
+		t.Cleanup(func() {
+			folderService.ExpectedError = nil
+		})
+		folderService.ExpectedError = dashboards.ErrFolderNotFound
 		mockSQLStore := mockstore.NewSQLStoreMock()
 		loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", org.RoleEditor, func(sc *scenarioContext) {
 			callGetFolderPermissions(sc, hs)
@@ -79,10 +82,11 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
 		origNewGuardian := guardian.New
 		t.Cleanup(func() {
 			guardian.New = origNewGuardian
+			folderService.ExpectedError = nil
 		})
 
 		guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanAdminValue: false})
-		folderService.On("GetFolderByUID", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, dashboards.ErrFolderAccessDenied).Twice()
+		folderService.ExpectedError = dashboards.ErrFolderAccessDenied
 		mockSQLStore := mockstore.NewSQLStoreMock()
 
 		loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", org.RoleEditor, func(sc *scenarioContext) {
@@ -126,8 +130,7 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
 			},
 		})
 
-		folderResponse := &models.Folder{Id: 1, Uid: "uid", Title: "Folder"}
-		folderService.On("GetFolderByUID", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(folderResponse, nil).Twice()
+		folderService.ExpectedFolder = &models.Folder{Id: 1, Uid: "uid", Title: "Folder"}
 		dashboardStore.On("UpdateDashboardACL", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
 		mockSQLStore := mockstore.NewSQLStoreMock()
 
@@ -184,8 +187,7 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
 			CheckPermissionBeforeUpdateError: guardian.ErrGuardianPermissionExists,
 		})
 
-		folderResponse := &models.Folder{Id: 1, Uid: "uid", Title: "Folder"}
-		folderService.On("GetFolderByUID", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(folderResponse, nil).Once()
+		folderService.ExpectedFolder = &models.Folder{Id: 1, Uid: "uid", Title: "Folder"}
 
 		cmd := dtos.UpdateDashboardACLCommand{
 			Items: []dtos.DashboardACLUpdateItem{
@@ -249,8 +251,7 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
 			CheckPermissionBeforeUpdateError: guardian.ErrGuardianOverride},
 		)
 
-		folderResponse := &models.Folder{Id: 1, Uid: "uid", Title: "Folder"}
-		folderService.On("GetFolderByUID", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(folderResponse, nil).Once()
+		folderService.ExpectedFolder = &models.Folder{Id: 1, Uid: "uid", Title: "Folder"}
 
 		cmd := dtos.UpdateDashboardACLCommand{
 			Items: []dtos.DashboardACLUpdateItem{
@@ -296,8 +297,7 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
 
 		var gotItems []*models.DashboardACL
 
-		folderResponse := &models.Folder{Id: 1, Uid: "uid", Title: "Folder"}
-		folderService.On("GetFolderByUID", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(folderResponse, nil).Twice()
+		folderService.ExpectedFolder = &models.Folder{Id: 1, Uid: "uid", Title: "Folder"}
 		dashboardStore.On("UpdateDashboardACL", mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
 			gotItems = args.Get(2).([]*models.DashboardACL)
 		}).Return(nil).Once()

diff --git a/pkg/api/folder_test.go b/pkg/api/folder_test.go
@@ -19,6 +19,8 @@ import (
 	acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	"github.com/grafana/grafana/pkg/services/folder"
+	"github.com/grafana/grafana/pkg/services/folder/foldertest"
 	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/services/quota/quotatest"
 	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
@@ -29,17 +31,15 @@ import (
 )
 
 func TestFoldersAPIEndpoint(t *testing.T) {
-	folderService := &dashboards.FakeFolderService{}
-	defer folderService.AssertExpectations(t)
+	folderService := &foldertest.FakeService{}
 
 	t.Run("Given a correct request for creating a folder", func(t *testing.T) {
 		cmd := models.CreateFolderCommand{
 			Uid:   "uid",
 			Title: "Folder",
 		}
 
-		folderResult := &models.Folder{Id: 1, Uid: "uid", Title: "Folder"}
-		folderService.On("CreateFolder", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(folderResult, nil).Once()
+		folderService.ExpectedFolder = &models.Folder{Id: 1, Uid: "uid", Title: "Folder"}
 
 		createFolderScenario(t, "When calling POST on", "/api/folders", "/api/folders", folderService, cmd,
 			func(sc *scenarioContext) {
@@ -55,6 +55,9 @@ func TestFoldersAPIEndpoint(t *testing.T) {
 	})
 
 	t.Run("Given incorrect requests for creating a folder", func(t *testing.T) {
+		t.Cleanup(func() {
+			folderService.ExpectedError = nil
+		})
 		testCases := []struct {
 			Error              error
 			ExpectedStatusCode int
@@ -76,7 +79,7 @@ func TestFoldersAPIEndpoint(t *testing.T) {
 		}
 
 		for _, tc := range testCases {
-			folderService.On("CreateFolder", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, tc.Error).Once()
+			folderService.ExpectedError = tc.Error
 
 			createFolderScenario(t, fmt.Sprintf("Expect '%s' error when calling POST on", tc.Error.Error()),
 				"/api/folders", "/api/folders", folderService, cmd, func(sc *scenarioContext) {
@@ -91,10 +94,7 @@ func TestFoldersAPIEndpoint(t *testing.T) {
 			Title: "Folder upd",
 		}
 
-		folderService.On("UpdateFolder", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
-			cmd := args.Get(4).(*models.UpdateFolderCommand)
-			cmd.Result = &models.Folder{Id: 1, Uid: "uid", Title: "Folder upd"}
-		}).Return(nil).Once()
+		folderService.ExpectedFolder = &models.Folder{Id: 1, Uid: "uid", Title: "Folder upd"}
 
 		updateFolderScenario(t, "When calling PUT on", "/api/folders/uid", "/api/folders/:uid", folderService, cmd,
 			func(sc *scenarioContext) {
@@ -130,7 +130,7 @@ func TestFoldersAPIEndpoint(t *testing.T) {
 		}
 
 		for _, tc := range testCases {
-			folderService.On("UpdateFolder", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tc.Error).Once()
+			folderService.ExpectedError = tc.Error
 			updateFolderScenario(t, fmt.Sprintf("Expect '%s' error when calling PUT on", tc.Error.Error()),
 				"/api/folders/uid", "/api/folders/:uid", folderService, cmd, func(sc *scenarioContext) {
 					callUpdateFolder(sc)
@@ -142,19 +142,15 @@ func TestFoldersAPIEndpoint(t *testing.T) {
 
 func TestHTTPServer_FolderMetadata(t *testing.T) {
 	setUpRBACGuardian(t)
-	folderService := dashboards.NewFakeFolderService(t)
+	folderService := &foldertest.FakeService{}
 	server := SetupAPITestServer(t, func(hs *HTTPServer) {
 		hs.folderService = folderService
 		hs.AccessControl = acmock.New()
 		hs.QuotaService = quotatest.NewQuotaServiceFake()
 	})
 
 	t.Run("Should attach access control metadata to multiple folders", func(t *testing.T) {
-		folderService.On("GetFolders", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*models.Folder{
-			{Uid: "1"},
-			{Uid: "2"},
-			{Uid: "3"},
-		}, nil)
+		folderService.ExpectedFolders = []*models.Folder{{Uid: "1"}, {Uid: "2"}, {Uid: "3"}}
 
 		req := server.NewGetRequest("/api/folders?accesscontrol=true")
 		webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{
@@ -183,7 +179,7 @@ func TestHTTPServer_FolderMetadata(t *testing.T) {
 	})
 
 	t.Run("Should attach access control metadata to folder response", func(t *testing.T) {
-		folderService.On("GetFolderByUID", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&models.Folder{Uid: "folderUid"}, nil)
+		folderService.ExpectedFolder = &models.Folder{Uid: "folderUid"}
 
 		req := server.NewGetRequest("/api/folders/folderUid?accesscontrol=true")
 		webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{
@@ -206,7 +202,7 @@ func TestHTTPServer_FolderMetadata(t *testing.T) {
 	})
 
 	t.Run("Should attach access control metadata to folder response", func(t *testing.T) {
-		folderService.On("GetFolderByUID", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&models.Folder{Uid: "folderUid"}, nil)
+		folderService.ExpectedFolder = &models.Folder{Uid: "folderUid"}
 
 		req := server.NewGetRequest("/api/folders/folderUid")
 		webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{
@@ -233,7 +229,7 @@ func callCreateFolder(sc *scenarioContext) {
 	sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
 }
 
-func createFolderScenario(t *testing.T, desc string, url string, routePattern string, folderService dashboards.FolderService,
+func createFolderScenario(t *testing.T, desc string, url string, routePattern string, folderService folder.Service,
 	cmd models.CreateFolderCommand, fn scenarioFunc) {
 	setUpRBACGuardian(t)
 	t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
@@ -273,7 +269,7 @@ func callUpdateFolder(sc *scenarioContext) {
 	sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec()
 }
 
-func updateFolderScenario(t *testing.T, desc string, url string, routePattern string, folderService dashboards.FolderService,
+func updateFolderScenario(t *testing.T, desc string, url string, routePattern string, folderService folder.Service,
 	cmd models.UpdateFolderCommand, fn scenarioFunc) {
 	setUpRBACGuardian(t)
 	t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
@@ -300,7 +296,7 @@ func updateFolderScenario(t *testing.T, desc string, url string, routePattern st
 }
 
 type fakeFolderService struct {
-	dashboards.FolderService
+	folder.Service
 
 	GetFoldersResult     []*models.Folder
 	GetFoldersError      error

diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go
@@ -15,6 +15,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/middleware/csrf"
+	"github.com/grafana/grafana/pkg/services/folder"
 	"github.com/grafana/grafana/pkg/services/querylibrary"
 	"github.com/grafana/grafana/pkg/services/searchV2"
 	"github.com/grafana/grafana/pkg/services/store/object"
@@ -176,7 +177,7 @@ type HTTPServer struct {
 	NotificationService          *notifications.NotificationService
 	DashboardService             dashboards.DashboardService
 	dashboardProvisioningService dashboards.DashboardProvisioningService
-	folderService                dashboards.FolderService
+	folderService                folder.Service
 	DatasourcePermissionsService permissions.DatasourcePermissionsService
 	commentsService              *comments.Service
 	AlertNotificationService     *alerting.AlertNotificationService
@@ -232,7 +233,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 	ldapGroups ldap.Groups, teamGuardian teamguardian.TeamGuardian, serviceaccountsService serviceaccounts.Service,
 	authInfoService login.AuthInfoService, storageService store.StorageService, httpObjectStore object.HTTPObjectStore,
 	notificationService *notifications.NotificationService, dashboardService dashboards.DashboardService,
-	dashboardProvisioningService dashboards.DashboardProvisioningService, folderService dashboards.FolderService,
+	dashboardProvisioningService dashboards.DashboardProvisioningService, folderService folder.Service,
 	datasourcePermissionsService permissions.DatasourcePermissionsService, alertNotificationService *alerting.AlertNotificationService,
 	dashboardsnapshotsService dashboardsnapshots.Service, commentsService *comments.Service, pluginSettings pluginSettings.Service,
 	avatarCacheServer *avatar.AvatarCacheServer, preferenceService pref.Service,

diff --git a/pkg/cmd/grafana-cli/runner/wire.go b/pkg/cmd/grafana-cli/runner/wire.go
@@ -281,12 +281,10 @@ var wireSet = wire.NewSet(
 	wire.Bind(new(teamguardian.Store), new(*teamguardianDatabase.TeamGuardianStoreImpl)),
 	teamguardianManager.ProvideService,
 	dashboardservice.ProvideDashboardService,
-	dashboardservice.ProvideFolderService,
 	dashboardstore.ProvideDashboardStore,
 	wire.Bind(new(dashboards.DashboardService), new(*dashboardservice.DashboardServiceImpl)),
 	wire.Bind(new(dashboards.DashboardProvisioningService), new(*dashboardservice.DashboardServiceImpl)),
 	wire.Bind(new(dashboards.PluginService), new(*dashboardservice.DashboardServiceImpl)),
-	wire.Bind(new(dashboards.FolderService), new(*dashboardservice.FolderServiceImpl)),
 	wire.Bind(new(dashboards.Store), new(*dashboardstore.DashboardStore)),
 	dashboardimportservice.ProvideService,
 	wire.Bind(new(dashboardimport.Service), new(*dashboardimportservice.ImportDashboardService)),

diff --git a/pkg/server/wire.go b/pkg/server/wire.go
@@ -70,6 +70,7 @@ import (
 	encryptionservice "github.com/grafana/grafana/pkg/services/encryption/service"
 	"github.com/grafana/grafana/pkg/services/export"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	"github.com/grafana/grafana/pkg/services/folder/folderimpl"
 	"github.com/grafana/grafana/pkg/services/grpcserver"
 	grpccontext "github.com/grafana/grafana/pkg/services/grpcserver/context"
 	"github.com/grafana/grafana/pkg/services/grpcserver/interceptors"
@@ -308,12 +309,11 @@ var wireBasicSet = wire.NewSet(
 	featuremgmt.ProvideManagerService,
 	featuremgmt.ProvideToggles,
 	dashboardservice.ProvideDashboardService,
-	dashboardservice.ProvideFolderService,
 	dashboardstore.ProvideDashboardStore,
+	folderimpl.ProvideService,
 	wire.Bind(new(dashboards.DashboardService), new(*dashboardservice.DashboardServiceImpl)),
 	wire.Bind(new(dashboards.DashboardProvisioningService), new(*dashboardservice.DashboardServiceImpl)),
 	wire.Bind(new(dashboards.PluginService), new(*dashboardservice.DashboardServiceImpl)),
-	wire.Bind(new(dashboards.FolderService), new(*dashboardservice.FolderServiceImpl)),
 	wire.Bind(new(dashboards.Store), new(*dashboardstore.DashboardStore)),
 	dashboardimportservice.ProvideService,
 	wire.Bind(new(dashboardimport.Service), new(*dashboardimportservice.ImportDashboardService)),