Chore: Remove bus from dashboards provisioning (#47495)

* Chore: Remove bus from dashboards provisioning

* fix symlink test, make it run on darwin

* remove unused mock
This commit is contained in:
Serge Zaitsev 2022-04-08 13:56:38 +02:00 committed by GitHub
parent 3df625e9f4
commit ad432108e6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 59 additions and 53 deletions

View File

@ -23,7 +23,7 @@ type DashboardProvisioner interface {
}
// DashboardProvisionerFactory creates DashboardProvisioners based on input
type DashboardProvisionerFactory func(context.Context, string, dashboards.DashboardProvisioningService, utils.OrgStore) (DashboardProvisioner, error)
type DashboardProvisionerFactory func(context.Context, string, dashboards.DashboardProvisioningService, utils.OrgStore, utils.DashboardStore) (DashboardProvisioner, error)
// Provisioner is responsible for syncing dashboard from disk to Grafana's database.
type Provisioner struct {
@ -35,7 +35,7 @@ type Provisioner struct {
}
// New returns a new DashboardProvisioner
func New(ctx context.Context, configDirectory string, provisioner dashboards.DashboardProvisioningService, orgStore utils.OrgStore) (DashboardProvisioner, error) {
func New(ctx context.Context, configDirectory string, provisioner dashboards.DashboardProvisioningService, orgStore utils.OrgStore, dashboardStore utils.DashboardStore) (DashboardProvisioner, error) {
logger := log.New("provisioning.dashboard")
cfgReader := &configReader{path: configDirectory, log: logger, orgStore: orgStore}
configs, err := cfgReader.readConfig(ctx)
@ -43,7 +43,7 @@ func New(ctx context.Context, configDirectory string, provisioner dashboards.Das
return nil, errutil.Wrap("Failed to read dashboards config", err)
}
fileReaders, err := getFileReaders(configs, logger, provisioner)
fileReaders, err := getFileReaders(configs, logger, provisioner, dashboardStore)
if err != nil {
return nil, errutil.Wrap("Failed to initialize file readers", err)
}
@ -123,14 +123,14 @@ func (provider *Provisioner) GetAllowUIUpdatesFromConfig(name string) bool {
}
func getFileReaders(
configs []*config, logger log.Logger, service dashboards.DashboardProvisioningService,
configs []*config, logger log.Logger, service dashboards.DashboardProvisioningService, store utils.DashboardStore,
) ([]*FileReader, error) {
var readers []*FileReader
for _, config := range configs {
switch config.Type {
case "file":
fileReader, err := NewDashboardFileReader(config, logger.New("type", config.Type, "name", config.Name), service)
fileReader, err := NewDashboardFileReader(config, logger.New("type", config.Type, "name", config.Name), service, store)
if err != nil {
return nil, errutil.Wrapf(err, "Failed to create file reader for config %v", config.Name)
}

View File

@ -11,12 +11,12 @@ import (
"sync"
"time"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/provisioning/utils"
"github.com/grafana/grafana/pkg/util"
)
@ -33,6 +33,7 @@ type FileReader struct {
Path string
log log.Logger
dashboardProvisioningService dashboards.DashboardProvisioningService
dashboardStore utils.DashboardStore
FoldersFromFilesStructure bool
mux sync.RWMutex
@ -41,7 +42,7 @@ type FileReader struct {
}
// NewDashboardFileReader returns a new filereader based on `config`
func NewDashboardFileReader(cfg *config, log log.Logger, service dashboards.DashboardProvisioningService) (*FileReader, error) {
func NewDashboardFileReader(cfg *config, log log.Logger, service dashboards.DashboardProvisioningService, dashboardStore utils.DashboardStore) (*FileReader, error) {
var path string
path, ok := cfg.Options["path"].(string)
if !ok {
@ -63,6 +64,7 @@ func NewDashboardFileReader(cfg *config, log log.Logger, service dashboards.Dash
Path: path,
log: log,
dashboardProvisioningService: service,
dashboardStore: dashboardStore,
FoldersFromFilesStructure: foldersFromFilesStructure,
usageTracker: newUsageTracker(),
}, nil
@ -298,7 +300,7 @@ func (fr *FileReader) getOrCreateFolderID(ctx context.Context, cfg *config, serv
}
cmd := &models.GetDashboardQuery{Slug: models.SlugifyTitle(folderName), OrgId: cfg.OrgID}
err := bus.Dispatch(ctx, cmd)
err := fr.dashboardStore.GetDashboard(ctx, cmd)
if err != nil && !errors.Is(err, models.ErrDashboardNotFound) {
return 0, err

View File

@ -1,5 +1,5 @@
//go:build linux
// +build linux
//go:build linux || darwin
// +build linux darwin
package dashboards
@ -25,7 +25,7 @@ func TestProvisionedSymlinkedFolder(t *testing.T) {
Options: map[string]interface{}{"path": symlinkedFolder},
}
reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil)
reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil, nil)
if err != nil {
t.Error("expected err to be nil")
}

View File

@ -8,7 +8,6 @@ import (
"testing"
"time"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/dashboards"
@ -42,7 +41,7 @@ func TestCreatingNewDashboardFileReader(t *testing.T) {
t.Run("using path parameter", func(t *testing.T) {
cfg := setup()
cfg.Options["path"] = defaultDashboards
reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil)
reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil, nil)
require.NoError(t, err)
require.NotEqual(t, reader.Path, "")
})
@ -50,7 +49,7 @@ func TestCreatingNewDashboardFileReader(t *testing.T) {
t.Run("using folder as options", func(t *testing.T) {
cfg := setup()
cfg.Options["folder"] = defaultDashboards
reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil)
reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil, nil)
require.NoError(t, err)
require.NotEqual(t, reader.Path, "")
})
@ -59,7 +58,7 @@ func TestCreatingNewDashboardFileReader(t *testing.T) {
cfg := setup()
cfg.Options["path"] = foldersFromFilesStructure
cfg.Options["foldersFromFilesStructure"] = true
reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil)
reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil, nil)
require.NoError(t, err)
require.NotEqual(t, reader.Path, "")
})
@ -72,7 +71,7 @@ func TestCreatingNewDashboardFileReader(t *testing.T) {
}
cfg.Options["folder"] = fullPath
reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil)
reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil, nil)
require.NoError(t, err)
require.Equal(t, reader.Path, fullPath)
@ -82,7 +81,7 @@ func TestCreatingNewDashboardFileReader(t *testing.T) {
t.Run("using relative path", func(t *testing.T) {
cfg := setup()
cfg.Options["folder"] = defaultDashboards
reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil)
reader, err := NewDashboardFileReader(cfg, log.New("test-logger"), nil, nil)
require.NoError(t, err)
resolvedPath := reader.resolvedPath()
@ -96,9 +95,8 @@ func TestDashboardFileReader(t *testing.T) {
fakeService := &dashboards.FakeDashboardProvisioning{}
defer fakeService.AssertExpectations(t)
fakeStore := &fakeDashboardStore{}
setup := func() {
bus.ClearBusHandlers()
bus.AddHandler("test", mockGetDashboardQuery)
cfg = &config{
Name: configName,
Type: "file",
@ -118,7 +116,7 @@ func TestDashboardFileReader(t *testing.T) {
fakeService.On("SaveFolderForProvisionedDashboards", mock.Anything, mock.Anything).Return(&models.Dashboard{Id: 1}, nil).Once()
fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{Id: 2}, nil).Times(2)
reader, err := NewDashboardFileReader(cfg, logger, nil)
reader, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
reader.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -138,7 +136,7 @@ func TestDashboardFileReader(t *testing.T) {
inserted++
})
reader, err := NewDashboardFileReader(cfg, logger, nil)
reader, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
reader.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -175,7 +173,7 @@ func TestDashboardFileReader(t *testing.T) {
fakeService.On("GetProvisionedDashboardData", configName).Return(provisionedDashboard, nil).Once()
reader, err := NewDashboardFileReader(cfg, logger, nil)
reader, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
reader.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -203,7 +201,7 @@ func TestDashboardFileReader(t *testing.T) {
fakeService.On("GetProvisionedDashboardData", configName).Return(provisionedDashboard, nil).Once()
fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Once()
reader, err := NewDashboardFileReader(cfg, logger, nil)
reader, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
reader.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -238,7 +236,7 @@ func TestDashboardFileReader(t *testing.T) {
fakeService.On("GetProvisionedDashboardData", configName).Return(provisionedDashboard, nil).Once()
reader, err := NewDashboardFileReader(cfg, logger, nil)
reader, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
reader.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -266,7 +264,7 @@ func TestDashboardFileReader(t *testing.T) {
fakeService.On("GetProvisionedDashboardData", configName).Return(provisionedDashboard, nil).Once()
fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Once()
reader, err := NewDashboardFileReader(cfg, logger, nil)
reader, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
reader.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -281,7 +279,7 @@ func TestDashboardFileReader(t *testing.T) {
fakeService.On("GetProvisionedDashboardData", configName).Return(nil, nil).Once()
fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Once()
reader, err := NewDashboardFileReader(cfg, logger, nil)
reader, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
reader.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -298,7 +296,7 @@ func TestDashboardFileReader(t *testing.T) {
fakeService.On("SaveFolderForProvisionedDashboards", mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Times(2)
fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Times(3)
reader, err := NewDashboardFileReader(cfg, logger, nil)
reader, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
reader.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -315,7 +313,7 @@ func TestDashboardFileReader(t *testing.T) {
Folder: "",
}
_, err := NewDashboardFileReader(cfg, logger, nil)
_, err := NewDashboardFileReader(cfg, logger, nil, nil)
require.NotNil(t, err)
})
@ -323,7 +321,7 @@ func TestDashboardFileReader(t *testing.T) {
setup()
cfg.Options["path"] = brokenDashboards
_, err := NewDashboardFileReader(cfg, logger, nil)
_, err := NewDashboardFileReader(cfg, logger, nil, nil)
require.NoError(t, err)
})
@ -336,14 +334,14 @@ func TestDashboardFileReader(t *testing.T) {
fakeService.On("SaveFolderForProvisionedDashboards", mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Times(2)
fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Times(2)
reader1, err := NewDashboardFileReader(cfg1, logger, nil)
reader1, err := NewDashboardFileReader(cfg1, logger, nil, fakeStore)
reader1.dashboardProvisioningService = fakeService
require.NoError(t, err)
err = reader1.walkDisk(context.Background())
require.NoError(t, err)
reader2, err := NewDashboardFileReader(cfg2, logger, nil)
reader2, err := NewDashboardFileReader(cfg2, logger, nil, fakeStore)
reader2.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -363,7 +361,7 @@ func TestDashboardFileReader(t *testing.T) {
"folder": defaultDashboards,
},
}
r, err := NewDashboardFileReader(cfg, logger, nil)
r, err := NewDashboardFileReader(cfg, logger, nil, nil)
require.NoError(t, err)
_, err = r.getOrCreateFolderID(context.Background(), cfg, fakeService, cfg.Folder)
@ -383,7 +381,7 @@ func TestDashboardFileReader(t *testing.T) {
}
fakeService.On("SaveFolderForProvisionedDashboards", mock.Anything, mock.Anything).Return(&models.Dashboard{Id: 1}, nil).Once()
r, err := NewDashboardFileReader(cfg, logger, nil)
r, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
require.NoError(t, err)
_, err = r.getOrCreateFolderID(context.Background(), cfg, fakeService, cfg.Folder)
@ -403,7 +401,7 @@ func TestDashboardFileReader(t *testing.T) {
},
}
r, err := NewDashboardFileReader(cfg, logger, nil)
r, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
require.NoError(t, err)
_, err = r.getOrCreateFolderID(context.Background(), cfg, fakeService, cfg.Folder)
@ -458,7 +456,7 @@ func TestDashboardFileReader(t *testing.T) {
cfg.DisableDeletion = true
reader, err := NewDashboardFileReader(cfg, logger, nil)
reader, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
reader.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -473,7 +471,7 @@ func TestDashboardFileReader(t *testing.T) {
fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Once()
fakeService.On("DeleteProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
reader, err := NewDashboardFileReader(cfg, logger, nil)
reader, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
reader.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -512,6 +510,8 @@ func (ffi FakeFileInfo) Sys() interface{} {
return nil
}
func mockGetDashboardQuery(_ context.Context, _ *models.GetDashboardQuery) error {
type fakeDashboardStore struct{}
func (fds *fakeDashboardStore) GetDashboard(_ context.Context, _ *models.GetDashboardQuery) error {
return models.ErrDashboardNotFound
}

View File

@ -5,7 +5,6 @@ import (
"sort"
"testing"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/dashboards"
@ -19,11 +18,9 @@ const (
)
func TestDuplicatesValidator(t *testing.T) {
bus.ClearBusHandlers()
fakeService := &dashboards.FakeDashboardProvisioning{}
defer fakeService.AssertExpectations(t)
bus.AddHandler("test", mockGetDashboardQuery)
cfg := &config{
Name: "Default",
Type: "file",
@ -36,7 +33,8 @@ func TestDuplicatesValidator(t *testing.T) {
t.Run("Duplicates validator should collect info about duplicate UIDs and titles within folders", func(t *testing.T) {
const folderName = "duplicates-validator-folder"
r, err := NewDashboardFileReader(cfg, logger, nil)
fakeStore := &fakeDashboardStore{}
r, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
require.NoError(t, err)
fakeService.On("SaveFolderForProvisionedDashboards", mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Times(6)
fakeService.On("GetProvisionedDashboardData", mock.Anything).Return([]*models.DashboardProvisioning{}, nil).Times(4)
@ -55,11 +53,11 @@ func TestDuplicatesValidator(t *testing.T) {
Options: map[string]interface{}{"path": dashboardContainingUID},
}
reader1, err := NewDashboardFileReader(cfg1, logger, nil)
reader1, err := NewDashboardFileReader(cfg1, logger, nil, fakeStore)
reader1.dashboardProvisioningService = fakeService
require.NoError(t, err)
reader2, err := NewDashboardFileReader(cfg2, logger, nil)
reader2, err := NewDashboardFileReader(cfg2, logger, nil, fakeStore)
reader2.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -91,7 +89,8 @@ func TestDuplicatesValidator(t *testing.T) {
t.Run("Duplicates validator should not collect info about duplicate UIDs and titles within folders for different orgs", func(t *testing.T) {
const folderName = "duplicates-validator-folder"
r, err := NewDashboardFileReader(cfg, logger, nil)
fakeStore := &fakeDashboardStore{}
r, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
require.NoError(t, err)
folderID, err := r.getOrCreateFolderID(context.Background(), cfg, fakeService, folderName)
require.NoError(t, err)
@ -107,11 +106,11 @@ func TestDuplicatesValidator(t *testing.T) {
Options: map[string]interface{}{"path": dashboardContainingUID},
}
reader1, err := NewDashboardFileReader(cfg1, logger, nil)
reader1, err := NewDashboardFileReader(cfg1, logger, nil, fakeStore)
reader1.dashboardProvisioningService = fakeService
require.NoError(t, err)
reader2, err := NewDashboardFileReader(cfg2, logger, nil)
reader2, err := NewDashboardFileReader(cfg2, logger, nil, fakeStore)
reader2.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -154,6 +153,7 @@ func TestDuplicatesValidator(t *testing.T) {
fakeService.On("SaveFolderForProvisionedDashboards", mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Times(5)
fakeService.On("GetProvisionedDashboardData", mock.Anything).Return([]*models.DashboardProvisioning{}, nil).Times(3)
fakeService.On("SaveProvisionedDashboard", mock.Anything, mock.Anything, mock.Anything).Return(&models.Dashboard{}, nil).Times(5)
fakeStore := &fakeDashboardStore{}
cfg1 := &config{
Name: "first", Type: "file", OrgID: 1, Folder: "duplicates-validator-folder",
@ -167,15 +167,15 @@ func TestDuplicatesValidator(t *testing.T) {
Name: "third", Type: "file", OrgID: 2, Folder: "duplicates-validator-folder",
Options: map[string]interface{}{"path": twoDashboardsWithUID},
}
reader1, err := NewDashboardFileReader(cfg1, logger, nil)
reader1, err := NewDashboardFileReader(cfg1, logger, nil, fakeStore)
reader1.dashboardProvisioningService = fakeService
require.NoError(t, err)
reader2, err := NewDashboardFileReader(cfg2, logger, nil)
reader2, err := NewDashboardFileReader(cfg2, logger, nil, fakeStore)
reader2.dashboardProvisioningService = fakeService
require.NoError(t, err)
reader3, err := NewDashboardFileReader(cfg3, logger, nil)
reader3, err := NewDashboardFileReader(cfg3, logger, nil, fakeStore)
reader3.dashboardProvisioningService = fakeService
require.NoError(t, err)
@ -192,7 +192,7 @@ func TestDuplicatesValidator(t *testing.T) {
duplicates := duplicateValidator.getDuplicates()
r, err := NewDashboardFileReader(cfg, logger, nil)
r, err := NewDashboardFileReader(cfg, logger, nil, fakeStore)
require.NoError(t, err)
folderID, err := r.getOrCreateFolderID(context.Background(), cfg, fakeService, cfg1.Folder)
require.NoError(t, err)
@ -209,7 +209,7 @@ func TestDuplicatesValidator(t *testing.T) {
sort.Strings(titleUsageReaders)
require.Equal(t, []string{"first"}, titleUsageReaders)
r, err = NewDashboardFileReader(cfg3, logger, nil)
r, err = NewDashboardFileReader(cfg3, logger, nil, fakeStore)
require.NoError(t, err)
folderID, err = r.getOrCreateFolderID(context.Background(), cfg3, fakeService, cfg3.Folder)
require.NoError(t, err)

View File

@ -187,7 +187,7 @@ func (ps *ProvisioningServiceImpl) ProvisionNotifications(ctx context.Context) e
func (ps *ProvisioningServiceImpl) ProvisionDashboards(ctx context.Context) error {
dashboardPath := filepath.Join(ps.Cfg.ProvisioningPath, "dashboards")
dashProvisioner, err := ps.newDashboardProvisioner(ctx, dashboardPath, ps.dashboardService, ps.SQLStore)
dashProvisioner, err := ps.newDashboardProvisioner(ctx, dashboardPath, ps.dashboardService, ps.SQLStore, ps.SQLStore)
if err != nil {
return errutil.Wrap("Failed to create provisioner", err)
}

View File

@ -93,7 +93,7 @@ func setup() *serviceTestStruct {
}
serviceTest.service = newProvisioningServiceImpl(
func(context.Context, string, dashboardstore.DashboardProvisioningService, utils.OrgStore) (dashboards.DashboardProvisioner, error) {
func(context.Context, string, dashboardstore.DashboardProvisioningService, utils.OrgStore, utils.DashboardStore) (dashboards.DashboardProvisioner, error) {
return serviceTest.mock, nil
},
nil,

View File

@ -12,6 +12,10 @@ type OrgStore interface {
GetOrgById(context.Context, *models.GetOrgByIdQuery) error
}
type DashboardStore interface {
GetDashboard(context.Context, *models.GetDashboardQuery) error
}
func CheckOrgExists(ctx context.Context, store OrgStore, orgID int64) error {
query := models.GetOrgByIdQuery{Id: orgID}
if err := store.GetOrgById(ctx, &query); err != nil {