backend/services: Move GetDashboard from sqlstore to dashboard service (#48971)

* rename folder to match package name
* backend/sqlstore: move GetDashboard into DashboardService

This is a stepping-stone commit which copies the GetDashboard function - which lets us remove the sqlstore from the interfaces in dashboards - without changing any other callers.
* checkpoint: moving GetDashboard calls into dashboard service
* finish refactoring api tests for dashboardService.GetDashboard

pkg/services/dashboards/dashboard.go

@@ -14,6 +14,7 @@ type DashboardService interface {
 	MakeUserAdmin(ctx context.Context, orgID int64, userID, dashboardID int64, setViewAndEditPermissions bool) error
 	BuildSaveDashboardCommand(ctx context.Context, dto *SaveDashboardDTO, shouldValidateAlerts bool, validateProvisionedDashboard bool) (*models.SaveDashboardCommand, error)
 	UpdateDashboardACL(ctx context.Context, uid int64, items []*models.DashboardAcl) error
+	GetDashboard(ctx context.Context, query *models.GetDashboardQuery) error
 }
 
 // PluginService is a service for operating on plugin dashboards.
@@ -51,6 +52,7 @@ type Store interface {
 	UnprovisionDashboard(ctx context.Context, id int64) error
 	// GetDashboardsByPluginID retrieves dashboards identified by plugin.
 	GetDashboardsByPluginID(ctx context.Context, query *models.GetDashboardsByPluginIdQuery) error
+	GetDashboard(ctx context.Context, query *models.GetDashboardQuery) error
 	DeleteDashboard(ctx context.Context, cmd *models.DeleteDashboardCommand) error
 	FolderStore
 }

pkg/services/dashboards/dashboard_service_mock.go

@@ -13,6 +13,8 @@ type FakeDashboardService struct {
 	SaveDashboardError  error
 	SavedDashboards     []*SaveDashboardDTO
 	ProvisionedDashData *models.DashboardProvisioning
+
+	GetDashboardFn func(ctx context.Context, cmd *models.GetDashboardQuery) error
 }
 
 func (s *FakeDashboardService) SaveDashboard(ctx context.Context, dto *SaveDashboardDTO, allowUiUpdate bool) (*models.Dashboard, error) {
@@ -45,3 +47,15 @@ func (s *FakeDashboardService) GetProvisionedDashboardDataByDashboardID(id int64
 func (s *FakeDashboardService) DeleteOrphanedProvisionedDashboards(ctx context.Context, cmd *models.DeleteOrphanedProvisionedDashboardsCommand) error {
 	return nil
 }
+
+func (s *FakeDashboardService) GetDashboard(ctx context.Context, cmd *models.GetDashboardQuery) error {
+	if s.GetDashboardFn != nil {
+		return s.GetDashboardFn(ctx, cmd)
+	}
+	// A minimal result for tests that need a valid result, but don't care what's in it.
+	d := models.NewDashboard("mocked")
+	d.Id = 1
+	d.Uid = "1"
+	cmd.Result = d
+	return nil
+}

pkg/services/dashboards/database/database.go

@@ -21,6 +21,9 @@ type DashboardStore struct {
 	log      log.Logger
 }
 
+// DashboardStore implements the Store interface
+var _ dashboards.Store = (*DashboardStore)(nil)
+
 func ProvideDashboardStore(sqlStore *sqlstore.SQLStore) *DashboardStore {
 	return &DashboardStore{sqlStore: sqlStore, log: log.New("dashboard-store")}
 }
@@ -823,3 +826,25 @@ func (d *DashboardStore) deleteAlertDefinition(dashboardId int64, sess *sqlstore
 
 	return nil
 }
+
+func (d *DashboardStore) GetDashboard(ctx context.Context, query *models.GetDashboardQuery) error {
+	return d.sqlStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
+		if query.Id == 0 && len(query.Slug) == 0 && len(query.Uid) == 0 {
+			return models.ErrDashboardIdentifierNotSet
+		}
+
+		dashboard := models.Dashboard{Slug: query.Slug, OrgId: query.OrgId, Id: query.Id, Uid: query.Uid}
+		has, err := sess.Get(&dashboard)
+
+		if err != nil {
+			return err
+		} else if !has {
+			return models.ErrDashboardNotFound
+		}
+
+		dashboard.SetId(dashboard.Id)
+		dashboard.SetUid(dashboard.Uid)
+		query.Result = &dashboard
+		return nil
+	})
+}

pkg/services/dashboards/database/database_dashboard_test.go

@@ -61,7 +61,7 @@ func TestDashboardDataAccess(t *testing.T) {
 			OrgId: 1,
 		}
 
-		err := sqlStore.GetDashboard(context.Background(), &query)
+		err := dashboardStore.GetDashboard(context.Background(), &query)
 		require.NoError(t, err)
 
 		require.Equal(t, query.Result.Title, "test dash 23")
@@ -78,7 +78,7 @@ func TestDashboardDataAccess(t *testing.T) {
 			OrgId: 1,
 		}
 
-		err := sqlStore.GetDashboard(context.Background(), &query)
+		err := dashboardStore.GetDashboard(context.Background(), &query)
 		require.NoError(t, err)
 
 		require.Equal(t, query.Result.Title, "test dash 23")
@@ -95,7 +95,7 @@ func TestDashboardDataAccess(t *testing.T) {
 			OrgId: 1,
 		}
 
-		err := sqlStore.GetDashboard(context.Background(), &query)
+		err := dashboardStore.GetDashboard(context.Background(), &query)
 		require.NoError(t, err)
 
 		require.Equal(t, query.Result.Title, "test dash 23")
@@ -111,7 +111,7 @@ func TestDashboardDataAccess(t *testing.T) {
 			OrgId: 1,
 		}
 
-		err := sqlStore.GetDashboard(context.Background(), &query)
+		err := dashboardStore.GetDashboard(context.Background(), &query)
 		require.Equal(t, err, models.ErrDashboardIdentifierNotSet)
 	})
 
@@ -180,7 +180,7 @@ func TestDashboardDataAccess(t *testing.T) {
 			OrgId: 1,
 		}
 
-		err = sqlStore.GetDashboard(context.Background(), &query)
+		err = dashboardStore.GetDashboard(context.Background(), &query)
 		require.NoError(t, err)
 		require.Equal(t, query.Result.FolderId, int64(0))
 		require.Equal(t, query.Result.CreatedBy, savedDash.CreatedBy)

pkg/services/dashboards/database_mock.go

@@ -1,4 +1,4 @@
-// Code generated by mockery v2.10.0. DO NOT EDIT.
+// Code generated by mockery v2.12.2. DO NOT EDIT.
 
 package dashboards
 
@@ -7,6 +7,8 @@ import (
 
 	models "github.com/grafana/grafana/pkg/models"
 	mock "github.com/stretchr/testify/mock"
+
+	testing "testing"
 )
 
 // FakeDashboardStore is an autogenerated mock type for the Store type
@@ -42,6 +44,20 @@ func (_m *FakeDashboardStore) DeleteOrphanedProvisionedDashboards(ctx context.Co
 	return r0
 }
 
+// GetDashboard provides a mock function with given fields: ctx, query
+func (_m *FakeDashboardStore) GetDashboard(ctx context.Context, query *models.GetDashboardQuery) error {
+	ret := _m.Called(ctx, query)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, *models.GetDashboardQuery) error); ok {
+		r0 = rf(ctx, query)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // GetDashboardsByPluginID provides a mock function with given fields: ctx, query
 func (_m *FakeDashboardStore) GetDashboardsByPluginID(ctx context.Context, query *models.GetDashboardsByPluginIdQuery) error {
 	ret := _m.Called(ctx, query)
@@ -302,3 +318,13 @@ func (_m *FakeDashboardStore) ValidateDashboardBeforeSave(dashboard *models.Dash
 
 	return r0, r1
 }
+
+// NewFakeDashboardStore creates a new instance of FakeDashboardStore. It also registers the testing.TB interface on the mock and a cleanup function to assert the mocks expectations.
+func NewFakeDashboardStore(t testing.TB) *FakeDashboardStore {
+	mock := &FakeDashboardStore{}
+	mock.Mock.Test(t)
+
+	t.Cleanup(func() { mock.AssertExpectations(t) })
+
+	return mock
+}

pkg/services/dashboards/service/dashboard_service.go

@@ -481,3 +481,7 @@ func (dr *DashboardServiceImpl) setDefaultPermissions(ctx context.Context, dto *
 
 	return nil
 }
+
+func (dr *DashboardServiceImpl) GetDashboard(ctx context.Context, query *models.GetDashboardQuery) error {
+	return dr.dashboardStore.GetDashboard(ctx, query)
+}

pkg/services/dashboards/service/dashboard_service_integration_test.go

@@ -7,18 +7,20 @@ import (
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/models"
 	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
 	"github.com/grafana/grafana/pkg/services/alerting"
+	"github.com/grafana/grafana/pkg/services/dashboards"
 	dashbboardservice "github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/dashboards/database"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 const testOrgID int64 = 1
@@ -76,7 +78,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 					res := callSaveWithResult(t, cmd, sc.sqlStore)
 					require.NotNil(t, res)
 
-					err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+					err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 						OrgId: otherOrgId,
 						Uid:   sc.savedDashInFolder.Uid,
 					})
@@ -316,7 +318,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 						res := callSaveWithResult(t, cmd, sc.sqlStore)
 						require.NotNil(t, res)
 
-						err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+						err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 							Id:    res.Id,
 							OrgId: cmd.OrgId,
 						})
@@ -341,7 +343,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 
 						assert.NotEqual(t, sc.savedDashInGeneralFolder.Id, res.Id)
 
-						err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+						err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 							Id:    res.Id,
 							OrgId: cmd.OrgId,
 						})
@@ -366,7 +368,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 						assert.NotEqual(t, sc.savedDashInGeneralFolder.Id, res.Id)
 						assert.True(t, res.IsFolder)
 
-						err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+						err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 							Id:    res.Id,
 							OrgId: cmd.OrgId,
 						})
@@ -388,7 +390,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 
 						assert.Greater(t, res.Id, int64(0))
 						assert.NotEmpty(t, res.Uid)
-						err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+						err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 							Id:    res.Id,
 							OrgId: cmd.OrgId,
 						})
@@ -409,7 +411,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 						res := callSaveWithResult(t, cmd, sc.sqlStore)
 						require.NotNil(t, res)
 
-						err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+						err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 							Id:    res.Id,
 							OrgId: cmd.OrgId,
 						})
@@ -463,7 +465,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 						res := callSaveWithResult(t, cmd, sc.sqlStore)
 						require.NotNil(t, res)
 
-						err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+						err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 							Id:    sc.savedDashInGeneralFolder.Id,
 							OrgId: cmd.OrgId,
 						})
@@ -503,7 +505,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 						res := callSaveWithResult(t, cmd, sc.sqlStore)
 						require.NotNil(t, res)
 
-						err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+						err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 							Id:    sc.savedDashInFolder.Id,
 							OrgId: cmd.OrgId,
 						})
@@ -577,7 +579,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 						res := callSaveWithResult(t, cmd, sc.sqlStore)
 						require.NotNil(t, res)
 
-						err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+						err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 							Id:    sc.savedDashInGeneralFolder.Id,
 							OrgId: cmd.OrgId,
 						})
@@ -599,7 +601,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 						res := callSaveWithResult(t, cmd, sc.sqlStore)
 						require.NotNil(t, res)
 
-						err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+						err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 							Id:    sc.savedDashInFolder.Id,
 							OrgId: cmd.OrgId,
 						})
@@ -623,7 +625,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 						assert.Equal(t, sc.savedDashInFolder.Id, res.Id)
 						assert.Equal(t, "new-uid", res.Uid)
 
-						err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+						err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 							Id:    sc.savedDashInFolder.Id,
 							OrgId: cmd.OrgId,
 						})
@@ -663,7 +665,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 						assert.Equal(t, sc.savedDashInFolder.Id, res.Id)
 						assert.Equal(t, sc.savedDashInFolder.Uid, res.Uid)
 
-						err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+						err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 							Id:    res.Id,
 							OrgId: cmd.OrgId,
 						})
@@ -687,7 +689,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 						assert.Equal(t, sc.savedDashInGeneralFolder.Id, res.Id)
 						assert.Equal(t, sc.savedDashInGeneralFolder.Uid, res.Uid)
 
-						err := sc.sqlStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
+						err := sc.dashboardStore.GetDashboard(context.Background(), &models.GetDashboardQuery{
 							Id:    res.Id,
 							OrgId: cmd.OrgId,
 						})
@@ -795,6 +797,7 @@ func TestIntegratedDashboardService(t *testing.T) {
 type permissionScenarioContext struct {
 	dashboardGuardianMock    *guardian.FakeDashboardGuardian
 	sqlStore                 *sqlstore.SQLStore
+	dashboardStore           dashboards.Store
 	savedFolder              *models.Dashboard
 	savedDashInFolder        *models.Dashboard
 	otherSavedFolder         *models.Dashboard
@@ -813,6 +816,7 @@ func permissionScenario(t *testing.T, desc string, canSave bool, fn permissionSc
 	t.Run(desc, func(t *testing.T) {
 		sqlStore := sqlstore.InitTestDB(t)
 		guardian.InitLegacyGuardian(sqlStore)
+		dashboardStore := database.ProvideDashboardStore(sqlStore)
 
 		savedFolder := saveTestFolder(t, "Saved folder", testOrgID, sqlStore)
 		savedDashInFolder := saveTestDashboard(t, "Saved dash in folder", testOrgID, savedFolder.Id, sqlStore)
@@ -847,6 +851,7 @@ func permissionScenario(t *testing.T, desc string, canSave bool, fn permissionSc
 			otherSavedFolder:         otherSavedFolder,
 			savedDashInGeneralFolder: savedDashInGeneralFolder,
 			savedFolder:              savedFolder,
+			dashboardStore:           dashboardStore,
 		}
 
 		fn(t, sc)

pkg/services/dashboards/service/folder_service.go

@@ -12,7 +12,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/services/search"
-	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
@@ -24,13 +23,12 @@ type FolderServiceImpl struct {
 	searchService    *search.SearchService
 	features         featuremgmt.FeatureToggles
 	permissions      accesscontrol.FolderPermissionsService
-	sqlStore         sqlstore.Store
 }
 
 func ProvideFolderService(
 	cfg *setting.Cfg, dashboardService dashboards.DashboardService, dashboardStore dashboards.Store,
 	searchService *search.SearchService, features featuremgmt.FeatureToggles, folderPermissionsService accesscontrol.FolderPermissionsService,
-	ac accesscontrol.AccessControl, sqlStore sqlstore.Store,
+	ac accesscontrol.AccessControl,
 ) *FolderServiceImpl {
 	ac.RegisterScopeAttributeResolver(dashboards.NewFolderNameScopeResolver(dashboardStore))
 	ac.RegisterScopeAttributeResolver(dashboards.NewFolderIDScopeResolver(dashboardStore))
@@ -43,7 +41,6 @@ func ProvideFolderService(
 		searchService:    searchService,
 		features:         features,
 		permissions:      folderPermissionsService,
-		sqlStore:         sqlStore,
 	}
 }
 
@@ -190,7 +187,7 @@ func (f *FolderServiceImpl) CreateFolder(ctx context.Context, user *models.Signe
 
 func (f *FolderServiceImpl) UpdateFolder(ctx context.Context, user *models.SignedInUser, orgID int64, existingUid string, cmd *models.UpdateFolderCommand) error {
 	query := models.GetDashboardQuery{OrgId: orgID, Uid: existingUid}
-	if err := f.sqlStore.GetDashboard(ctx, &query); err != nil {
+	if err := f.dashboardStore.GetDashboard(ctx, &query); err != nil {
 		return toFolderError(err)
 	}
 

pkg/services/dashboards/service/folder_service_test.go

@@ -18,7 +18,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/guardian"
-	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
 )
@@ -39,7 +38,7 @@ func TestProvideFolderService(t *testing.T) {
 
 		ProvideFolderService(
 			cfg, &dashboards.FakeDashboardService{DashboardService: dashboardService},
-			store, nil, features, folderPermissions, ac, mockstore.NewSQLStoreMock(),
+			store, nil, features, folderPermissions, ac,
 		)
 
 		require.Len(t, ac.Calls.RegisterAttributeScopeResolver, 2)
@@ -55,7 +54,6 @@ func TestFolderService(t *testing.T) {
 		folderPermissions := acmock.NewMockedPermissionsService()
 		dashboardPermissions := acmock.NewMockedPermissionsService()
 		dashboardService := ProvideDashboardService(cfg, store, nil, features, folderPermissions, dashboardPermissions)
-		mockStore := mockstore.NewSQLStoreMock()
 
 		service := FolderServiceImpl{
 			cfg:              cfg,
@@ -65,7 +63,6 @@ func TestFolderService(t *testing.T) {
 			searchService:    nil,
 			features:         features,
 			permissions:      folderPermissions,
-			sqlStore:         mockStore,
 		}
 
 		t.Run("Given user has no permissions", func(t *testing.T) {
@@ -105,7 +102,11 @@ func TestFolderService(t *testing.T) {
 			})
 
 			t.Run("When updating folder should return access denied error", func(t *testing.T) {
-				mockStore.ExpectedDashboard = models.NewDashboardFolder("Folder")
+				store.On("GetDashboard", mock.Anything, mock.AnythingOfType("*models.GetDashboardQuery")).Run(func(args mock.Arguments) {
+					folder := args.Get(1).(*models.GetDashboardQuery)
+					folder.Result = models.NewDashboard("dashboard-test")
+					folder.Result.IsFolder = true
+				}).Return(nil)
 				err := service.UpdateFolder(context.Background(), user, orgID, folderUID, &models.UpdateFolderCommand{
 					Uid:   folderUID,
 					Title: "Folder-TEST",
@@ -156,8 +157,6 @@ func TestFolderService(t *testing.T) {
 				dashboardFolder.Uid = util.GenerateShortUID()
 				f := models.DashboardToFolder(dashboardFolder)
 
-				mockStore.ExpectedDashboard = dashboardFolder
-
 				store.On("ValidateDashboardBeforeSave", mock.Anything, mock.Anything).Return(true, nil)
 				store.On("SaveDashboard", mock.Anything).Return(dashboardFolder, nil)
 				store.On("GetFolderByID", mock.Anything, orgID, dashboardFolder.Id).Return(f, nil)