IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Nested Folders: Wrap create/update operations with transactions (#78000)

Browse Source 
* Nested Folders: Add transaction to create and update methods

* Update tests

* Make IncreaseVersionForAllRulesInNamespace synchronous

* Resolve merge conflicts
This commit is contained in:
Tania 2023-11-21 13:06:20 -08:00 committed by GitHub
parent 9c5daed336
commit 39754ba2d6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 108 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

131
pkg/services/folder/folderimpl/folder.go
View File

@ -416,43 +416,35 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
return nil, toFolderError(err)
}
dash, err := s.dashboardStore.SaveDashboard(ctx, *saveDashboardCmd)
if err != nil {
return nil, toFolderError(err)
}
var nestedFolder *folder.Folder
var dash *dashboards.Dashboard
err = s.db.InTransaction(ctx, func(ctx context.Context) error {
if dash, err = s.dashboardStore.SaveDashboard(ctx, *saveDashboardCmd); err != nil {
return toFolderError(err)
}
var createdFolder *folder.Folder
createdFolder, err = s.dashboardFolderStore.GetFolderByID(ctx, cmd.OrgID, dash.ID)
cmd = &folder.CreateFolderCommand{
// TODO: Today, if a UID isn't specified, the dashboard store
// generates a new UID. The new folder store will need to do this as
// well, but for now we take the UID from the newly created folder.
UID: dash.UID,
OrgID: cmd.OrgID,
Title: cmd.Title,
Description: cmd.Description,
ParentUID: cmd.ParentUID,
}
if nestedFolder, err = s.nestedFolderCreate(ctx, cmd); err != nil {
logger.Error("error saving folder to nested folder store", "error", err)
return err
}
return nil
})
if err != nil {
return nil, err
}
var nestedFolder *folder.Folder
cmd = &folder.CreateFolderCommand{
// TODO: Today, if a UID isn't specified, the dashboard store
// generates a new UID. The new folder store will need to do this as
// well, but for now we take the UID from the newly created folder.
UID: dash.UID,
OrgID: cmd.OrgID,
Title: cmd.Title,
Description: cmd.Description,
ParentUID: cmd.ParentUID,
}
nestedFolder, err = s.nestedFolderCreate(ctx, cmd)
if err != nil {
// We'll log the error and also roll back the previously-created
// (legacy) folder.
logger.Error("error saving folder to nested folder store", "error", err)
// do not shallow create error if the legacy folder delete fails
if deleteErr := s.dashboardStore.DeleteDashboard(ctx, &dashboards.DeleteDashboardCommand{
ID: createdFolder.ID, // nolint:staticcheck
OrgID: createdFolder.OrgID,
}); deleteErr != nil {
logger.Error("error deleting folder after failed save to nested folder store", "error", err)
}
return dashboards.FromDashboard(dash), err
}
f := dashboards.FromDashboard(dash)
if nestedFolder != nil && nestedFolder.ParentUID != "" {
f.ParentUID = nestedFolder.ParentUID
@ -468,25 +460,48 @@ func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (
}
user := cmd.SignedInUser
dashFolder, err := s.legacyUpdate(ctx, cmd)
var dashFolder, foldr *folder.Folder
var err error
err = s.db.InTransaction(ctx, func(ctx context.Context) error {
if dashFolder, err = s.legacyUpdate(ctx, cmd); err != nil {
return err
}
if foldr, err = s.store.Update(ctx, folder.UpdateFolderCommand{
UID: cmd.UID,
OrgID: cmd.OrgID,
NewTitle: cmd.NewTitle,
NewDescription: cmd.NewDescription,
SignedInUser: user,
}); err != nil {
return err
}
if cmd.NewTitle != nil {
namespace, id := cmd.SignedInUser.GetNamespacedID()
if err := s.bus.Publish(context.Background(), &events.FolderTitleUpdated{
Timestamp: foldr.Updated,
Title: foldr.Title,
ID: dashFolder.ID, // nolint:staticcheck
UID: dashFolder.UID,
OrgID: cmd.OrgID,
}); err != nil {
logger.Error("failed to publish FolderTitleUpdated event", "folder", foldr.Title, "user", id, "namespace", namespace, "error", err)
return err
}
}
return nil
})
if err != nil {
logger.Error("folder update failed", "folderUID", cmd.UID, "error", err)
return nil, err
}
foldr, err := s.store.Update(ctx, folder.UpdateFolderCommand{
UID: cmd.UID,
OrgID: cmd.OrgID,
NewTitle: cmd.NewTitle,
NewDescription: cmd.NewDescription,
SignedInUser: user,
})
if err != nil {
if errors.Is(err, folder.ErrFolderNotFound) {
logger.Warn("attempt to update folder that does not exist in the folders table", "folderUID", cmd.UID)
return dashFolder, nil
}
return nil, err
if !s.features.IsEnabled(ctx, featuremgmt.FlagNestedFolders) {
return dashFolder, nil
}
// always expose the dashboard store sequential ID
@ -511,8 +526,6 @@ func (s *Service) legacyUpdate(ctx context.Context, cmd *folder.UpdateFolderComm
dashFolder.FolderUID = *cmd.NewParentUID
}
currentTitle := dashFolder.Title
if !dashFolder.IsFolder {
return nil, dashboards.ErrFolderNotFound
}
@ -555,17 +568,6 @@ func (s *Service) legacyUpdate(ctx context.Context, cmd *folder.UpdateFolderComm
return nil, err
}
if currentTitle != foldr.Title {
if err := s.bus.Publish(ctx, &events.FolderTitleUpdated{
Timestamp: foldr.Updated,
Title: foldr.Title,
ID: dash.ID,
UID: dash.UID,
OrgID: cmd.OrgID,
}); err != nil {
logger.Error("failed to publish FolderTitleUpdated event", "folder", foldr.Title, "user", id, "namespace", namespace, "error", err)
}
}
return foldr, nil
}
@ -630,10 +632,10 @@ func (s *Service) Delete(ctx context.Context, cmd *folder.DeleteFolderCommand) e
return folder.ErrInternal.Errorf("failed to fetch subfolders from dashboard store: %w", err)
}
for _, folder := range result {
dashFolder, ok := dashFolders[folder]
for _, foldr := range result {
dashFolder, ok := dashFolders[foldr]
if !ok {
return err
return folder.ErrInternal.Errorf("folder does not exist in dashboard store")
}
if cmd.ForceDeleteRules {
@ -642,8 +644,7 @@ func (s *Service) Delete(ctx context.Context, cmd *folder.DeleteFolderCommand) e
}
}
err = s.legacyDelete(ctx, cmd, dashFolder)
if err != nil {
if err = s.legacyDelete(ctx, cmd, dashFolder); err != nil {
return err
}
}

75
pkg/services/folder/folderimpl/folder_test.go
View File

@ -191,7 +191,6 @@ func TestIntegrationFolderService(t *testing.T) {
dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil)
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(dash, nil).Once()
folderStore.On("GetFolderByID", mock.Anything, orgID, dash.ID).Return(f, nil)
actualFolder, err := service.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
@ -220,10 +219,21 @@ func TestIntegrationFolderService(t *testing.T) {
dashboardFolder := dashboards.NewDashboardFolder("Folder")
dashboardFolder.ID = rand.Int63()
dashboardFolder.UID = util.GenerateShortUID()
dashboardFolder.OrgID = orgID
f := dashboards.FromDashboard(dashboardFolder)
_, err := service.store.Create(context.Background(), folder.CreateFolderCommand{
OrgID: orgID,
Title: dashboardFolder.Title,
UID: dashboardFolder.UID,
SignedInUser: usr,
})
require.NoError(t, err)
dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil)
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(dashboardFolder, nil)
dashStore.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(dashboardFolder, nil)
folderStore.On("GetFolderByID", mock.Anything, orgID, dashboardFolder.ID).Return(f, nil)
title := "TEST-Folder"
@ -714,12 +724,12 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) {
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{}, nil)
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
dashboardFolderStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
cfg := setting.NewCfg()
folderService := &Service{
cfg: cfg,
store: nestedFolderStore,
db: sqlstore.InitTestDB(t),
dashboardStore: &dashStore,
dashboardFolderStore: dashboardFolderStore,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
@ -744,21 +754,25 @@ func TestNestedFolderService(t *testing.T) {
guardian.New = g
})
// dash is needed here because folderSvc.Create expects SaveDashboard to return it
dash := dashboards.NewDashboardFolder("myFolder")
dash.ID = rand.Int63()
dash.UID = "some_uid"
// dashboard store & service commands that should be called.
dashStore := &dashboards.FakeDashboardStore{}
dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil)
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{}, nil)
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(dash, nil)
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
dashboardFolderStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
nestedFolderStore := NewFakeStore()
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures(), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures(), acimpl.ProvideAccessControl(setting.NewCfg()), sqlstore.InitTestDB(t))
_, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
Title: "myFolder",
UID: "myFolder",
Title: dash.Title,
UID: dash.UID,
SignedInUser: usr,
})
require.NoError(t, err)
@ -774,21 +788,23 @@ func TestNestedFolderService(t *testing.T) {
guardian.New = g
})
dash := dashboards.NewDashboardFolder("myFolder")
dash.ID = rand.Int63()
dash.UID = "some_uid"
// dashboard store commands that should be called.
dashStore := &dashboards.FakeDashboardStore{}
dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil)
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{}, nil)
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(dash, nil)
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
dashboardFolderStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
nestedFolderStore := NewFakeStore()
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), sqlstore.InitTestDB(t))
_, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
Title: "myFolder",
UID: "myFolder",
Title: dash.Title,
UID: dash.UID,
SignedInUser: usr,
})
require.NoError(t, err)
@ -840,17 +856,16 @@ func TestNestedFolderService(t *testing.T) {
// dashboard store commands that should be called.
dashStore := &dashboards.FakeDashboardStore{}
dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil)
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{}, nil)
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(dash, nil)
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
dashboardFolderStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
dashboardFolderStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&folder.Folder{}, nil)
nestedFolderUser := &user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{}}
nestedFolderUser.Permissions[orgID] = map[string][]string{dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersProvider.GetResourceScopeUID("some_parent")}}
nestedFolderStore := NewFakeStore()
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), dbtest.NewFakeDB())
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), acimpl.ProvideAccessControl(setting.NewCfg()), sqlstore.InitTestDB(t))
_, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
Title: dash.Title,
@ -875,13 +890,11 @@ func TestNestedFolderService(t *testing.T) {
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{UID: "newUID"}, nil)
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
dashboardFolderStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
nestedFolderStore := NewFakeStore()
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{
ExpectedEvaluate: true,
}, dbtest.NewFakeDB())
}, sqlstore.InitTestDB(t))
f, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
Title: "myFolder",
@ -909,13 +922,8 @@ func TestNestedFolderService(t *testing.T) {
dashStore := &dashboards.FakeDashboardStore{}
dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil)
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(dashboardFolder, nil)
var actualCmd *dashboards.DeleteDashboardCommand
dashStore.On("DeleteDashboard", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
actualCmd = args.Get(1).(*dashboards.DeleteDashboardCommand)
}).Return(nil).Once()
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
dashboardFolderStore.On("GetFolderByID", mock.Anything, orgID, dashboardFolder.ID).Return(f, nil)
dashboardFolderStore.On("GetFolderByUID", mock.Anything, orgID, dashboardFolder.UID).Return(f, nil)
nestedFolderStore := NewFakeStore()
@ -936,12 +944,11 @@ func TestNestedFolderService(t *testing.T) {
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{
ExpectedEvaluate: true,
}, dbtest.NewFakeDB())
}, sqlstore.InitTestDB(t))
_, err := folderSvc.Create(context.Background(), &cmd)
require.Error(t, err, folder.ErrCircularReference)
// CreateFolder should not call the folder store's create method.
require.False(t, nestedFolderStore.CreateCalled)
require.NotNil(t, actualCmd)
})
t.Run("create returns error from nested folder service", func(t *testing.T) {
@ -956,13 +963,8 @@ func TestNestedFolderService(t *testing.T) {
dashStore := &dashboards.FakeDashboardStore{}
dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil)
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{}, nil)
var actualCmd *dashboards.DeleteDashboardCommand
dashStore.On("DeleteDashboard", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
actualCmd = args.Get(1).(*dashboards.DeleteDashboardCommand)
}).Return(nil).Once()
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
dashboardFolderStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
// return an error from the folder store
nestedFolderStore := NewFakeStore()
@ -971,7 +973,7 @@ func TestNestedFolderService(t *testing.T) {
// the service return success as long as the legacy create succeeds
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{
ExpectedEvaluate: true,
}, dbtest.NewFakeDB())
}, sqlstore.InitTestDB(t))
_, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
OrgID: orgID,
Title: "myFolder",
@ -982,7 +984,6 @@ func TestNestedFolderService(t *testing.T) {
// CreateFolder should also call the folder store's create method.
require.True(t, nestedFolderStore.CreateCalled)
require.NotNil(t, actualCmd)
})
t.Run("move without the right permissions should fail", func(t *testing.T) {
@ -1141,13 +1142,8 @@ func TestNestedFolderService(t *testing.T) {
dashStore := &dashboards.FakeDashboardStore{}
dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil).Times(2)
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&dashboards.Dashboard{}, nil)
var actualCmd *dashboards.DeleteDashboardCommand
dashStore.On("DeleteDashboard", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
actualCmd = args.Get(1).(*dashboards.DeleteDashboardCommand)
}).Return(nil).Once()
dashboardFolderStore := foldertest.NewFakeFolderStore(t)
dashboardFolderStore.On("GetFolderByID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("int64")).Return(&folder.Folder{}, nil)
dashboardFolderStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(&folder.Folder{}, nil)
parents := make([]*folder.Folder, 0, folder.MaxNestedFolderDepth)
@ -1161,7 +1157,7 @@ func TestNestedFolderService(t *testing.T) {
folderSvc := setup(t, dashStore, dashboardFolderStore, nestedFolderStore, featuremgmt.WithFeatures("nestedFolders"), actest.FakeAccessControl{
ExpectedEvaluate: true,
}, dbtest.NewFakeDB())
}, sqlstore.InitTestDB(t))
_, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
Title: "folder",
OrgID: orgID,
@ -1170,7 +1166,6 @@ func TestNestedFolderService(t *testing.T) {
SignedInUser: usr,
})
assert.ErrorIs(t, err, folder.ErrMaximumDepthReached)
require.NotNil(t, actualCmd)
})
t.Run("get default folder, no error", func(t *testing.T) {

17
pkg/services/ngalert/ngalert.go
View File

@ -329,16 +329,13 @@ func (ng *AlertNG) init() error {
func subscribeToFolderChanges(logger log.Logger, bus bus.Bus, dbStore api.RuleStore) {
// if folder title is changed, we update all alert rules in that folder to make sure that all peers (in HA mode) will update folder title and
// clean up the current state
bus.AddEventListener(func(ctx context.Context, e *events.FolderTitleUpdated) error {
// do not block the upstream execution
go func(evt *events.FolderTitleUpdated) {
logger.Info("Got folder title updated event. updating rules in the folder", "folderUID", evt.UID)
_, err := dbStore.IncreaseVersionForAllRulesInNamespace(ctx, evt.OrgID, evt.UID)
if err != nil {
logger.Error("Failed to update alert rules in the folder after its title was changed", "error", err, "folderUID", evt.UID, "folder", evt.Title)
return
}
}(e)
bus.AddEventListener(func(ctx context.Context, evt *events.FolderTitleUpdated) error {
logger.Info("Got folder title updated event. updating rules in the folder", "folderUID", evt.UID)
_, err := dbStore.IncreaseVersionForAllRulesInNamespace(ctx, evt.OrgID, evt.UID)
if err != nil {
logger.Error("Failed to update alert rules in the folder after its title was changed", "error", err, "folderUID", evt.UID, "folder", evt.Title)
return err
}
return nil
})
}