diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index ce2ca9d31aa..bd4f5d79a46 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -739,9 +739,10 @@ func (hs *HTTPServer) GetDashboardVersion(c *contextmodel.ReqContext) response.R version, _ := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 32) query := dashver.GetDashboardVersionQuery{ - OrgID: c.OrgID, - DashboardID: dash.ID, - Version: int(version), + OrgID: c.OrgID, + DashboardID: dash.ID, + DashboardUID: dash.UID, + Version: int(version), } res, err := hs.dashboardVersionService.Get(c.Req.Context(), &query) @@ -757,7 +758,7 @@ func (hs *HTTPServer) GetDashboardVersion(c *contextmodel.ReqContext) response.R dashVersionMeta := &dashver.DashboardVersionMeta{ ID: res.ID, DashboardID: res.DashboardID, - DashboardUID: dashUID, + DashboardUID: dash.UID, Data: res.Data, ParentVersion: res.ParentVersion, RestoredFrom: res.RestoredFrom, @@ -989,7 +990,7 @@ func (hs *HTTPServer) RestoreDashboardVersion(c *contextmodel.ReqContext) respon return dashboardGuardianResponse(err) } - versionQuery := dashver.GetDashboardVersionQuery{DashboardID: dashID, Version: apiCmd.Version, OrgID: c.OrgID} + versionQuery := dashver.GetDashboardVersionQuery{DashboardID: dashID, DashboardUID: dash.UID, Version: apiCmd.Version, OrgID: c.OrgID} version, err := hs.dashboardVersionService.Get(c.Req.Context(), &versionQuery) if err != nil { return response.Error(404, "Dashboard version not found", nil) diff --git a/pkg/services/dashboards/dashboard_service_mock.go b/pkg/services/dashboards/dashboard_service_mock.go index a59d811dbbe..3da367b22be 100644 --- a/pkg/services/dashboards/dashboard_service_mock.go +++ b/pkg/services/dashboards/dashboard_service_mock.go @@ -5,9 +5,8 @@ package dashboards import ( context "context" - mock "github.com/stretchr/testify/mock" - folder "github.com/grafana/grafana/pkg/services/folder" + mock "github.com/stretchr/testify/mock" ) // FakeDashboardService is an autogenerated mock type for the DashboardService type diff --git a/pkg/services/dashboards/store_mock.go b/pkg/services/dashboards/store_mock.go index 4b2b5a9bded..f32268a86ce 100644 --- a/pkg/services/dashboards/store_mock.go +++ b/pkg/services/dashboards/store_mock.go @@ -5,10 +5,11 @@ package dashboards import ( context "context" + folder "github.com/grafana/grafana/pkg/services/folder" mock "github.com/stretchr/testify/mock" models "github.com/grafana/grafana/pkg/services/alerting/models" - folder "github.com/grafana/grafana/pkg/services/folder" + quota "github.com/grafana/grafana/pkg/services/quota" ) diff --git a/pkg/services/dashboardversion/dashverimpl/dashver.go b/pkg/services/dashboardversion/dashverimpl/dashver.go index 569c0050a15..f8b090560ba 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver.go @@ -2,8 +2,11 @@ package dashverimpl import ( "context" + "errors" "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/dashboards" dashver "github.com/grafana/grafana/pkg/services/dashboardversion" "github.com/grafana/grafana/pkg/setting" ) @@ -14,27 +17,49 @@ const ( ) type Service struct { - store store + store store + dashSvc dashboards.DashboardService + log log.Logger } -func ProvideService(db db.DB) dashver.Service { +func ProvideService(db db.DB, dashboardService dashboards.DashboardService) dashver.Service { return &Service{ store: &sqlStore{ db: db, dialect: db.GetDialect(), }, + dashSvc: dashboardService, + log: log.New("dashboard-version"), } } func (s *Service) Get(ctx context.Context, query *dashver.GetDashboardVersionQuery) (*dashver.DashboardVersionDTO, error) { + // Get the DashboardUID if not populated + if query.DashboardUID == "" { + u, err := s.getDashUIDMaybeEmpty(ctx, query.DashboardID) + if err != nil { + return nil, err + } + query.DashboardUID = u + } + + // The store methods require the dashboard ID (uid is not in the dashboard + // versions table, at time of this writing), so get the DashboardID if it + // was not populated. + if query.DashboardID == 0 { + id, err := s.getDashIDMaybeEmpty(ctx, query.DashboardUID) + if err != nil { + return nil, err + } + query.DashboardID = id + } + version, err := s.store.Get(ctx, query) if err != nil { return nil, err } version.Data.Set("id", version.DashboardID) - - // FIXME: the next PR will add the dashboardService so we can grab the DashboardUID - return version.ToDTO(""), nil + return version.ToDTO(query.DashboardUID), nil } func (s *Service) DeleteExpired(ctx context.Context, cmd *dashver.DeleteExpiredVersionsCommand) error { @@ -69,6 +94,25 @@ func (s *Service) DeleteExpired(ctx context.Context, cmd *dashver.DeleteExpiredV // List all dashboard versions for the given dashboard ID. func (s *Service) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersionDTO, error) { + // Get the DashboardUID if not populated + if query.DashboardUID == "" { + u, err := s.getDashUIDMaybeEmpty(ctx, query.DashboardID) + if err != nil { + return nil, err + } + query.DashboardUID = u + } + + // The store methods require the dashboard ID (uid is not in the dashboard + // versions table, at time of this writing), so get the DashboardID if it + // was not populated. + if query.DashboardID == 0 { + id, err := s.getDashIDMaybeEmpty(ctx, query.DashboardUID) + if err != nil { + return nil, err + } + query.DashboardID = id + } if query.Limit == 0 { query.Limit = 1000 } @@ -78,8 +122,42 @@ func (s *Service) List(ctx context.Context, query *dashver.ListDashboardVersions } dtos := make([]*dashver.DashboardVersionDTO, len(dvs)) for i, v := range dvs { - // FIXME: the next PR will add the dashboardService so we can grab the DashboardUID - dtos[i] = v.ToDTO("") + dtos[i] = v.ToDTO(query.DashboardUID) } return dtos, nil } + +// getDashUIDMaybeEmpty is a helper function which takes a dashboardID and +// returns the UID. If the dashboard is not found, it will return an empty +// string. +func (s *Service) getDashUIDMaybeEmpty(ctx context.Context, id int64) (string, error) { + q := dashboards.GetDashboardRefByIDQuery{ID: id} + result, err := s.dashSvc.GetDashboardUIDByID(ctx, &q) + if err != nil { + if errors.Is(err, dashboards.ErrDashboardNotFound) { + s.log.Debug("dashboard not found") + return "", nil + } else { + s.log.Error("error getting dashboard", err) + return "", err + } + } + return result.UID, nil +} + +// getDashIDMaybeEmpty is a helper function which takes a dashboardUID and +// returns the ID. If the dashboard is not found, it will return -1. +func (s *Service) getDashIDMaybeEmpty(ctx context.Context, uid string) (int64, error) { + q := dashboards.GetDashboardQuery{UID: uid} + result, err := s.dashSvc.GetDashboard(ctx, &q) + if err != nil { + if errors.Is(err, dashboards.ErrDashboardNotFound) { + s.log.Debug("dashboard not found") + return -1, nil + } else { + s.log.Error("error getting dashboard", err) + return -1, err + } + } + return result.ID, nil +} diff --git a/pkg/services/dashboardversion/dashverimpl/dashver_test.go b/pkg/services/dashboardversion/dashverimpl/dashver_test.go index 480733bd56f..4cac843787a 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver_test.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver_test.go @@ -5,16 +5,20 @@ import ( "errors" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/dashboards" dashver "github.com/grafana/grafana/pkg/services/dashboardversion" "github.com/grafana/grafana/pkg/setting" ) func TestDashboardVersionService(t *testing.T) { dashboardVersionStore := newDashboardVersionStoreFake() - dashboardVersionService := Service{store: dashboardVersionStore} + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService} t.Run("Get dashboard version", func(t *testing.T) { dashboard := &dashver.DashboardVersion{ @@ -22,9 +26,11 @@ func TestDashboardVersionService(t *testing.T) { Data: &simplejson.Json{}, } dashboardVersionStore.ExpectedDashboardVersion = dashboard + dashboardService.On("GetDashboardUIDByID", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardRefByIDQuery")).Return(&dashboards.DashboardRef{UID: "uid"}, nil) + dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(&dashboards.Dashboard{ID: 42}, nil) dashboardVersion, err := dashboardVersionService.Get(context.Background(), &dashver.GetDashboardVersionQuery{}) require.NoError(t, err) - require.Equal(t, dashboard.ToDTO(""), dashboardVersion) + require.Equal(t, dashboard.ToDTO("uid"), dashboardVersion) }) } @@ -33,7 +39,8 @@ func TestDeleteExpiredVersions(t *testing.T) { setting.DashboardVersionsToKeep = versionsToKeep dashboardVersionStore := newDashboardVersionStoreFake() - dashboardVersionService := Service{store: dashboardVersionStore} + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService} t.Run("Don't delete anything if there are no expired versions", func(t *testing.T) { err := dashboardVersionService.DeleteExpired(context.Background(), &dashver.DeleteExpiredVersionsCommand{DeletedRows: 4}) @@ -55,15 +62,85 @@ func TestDeleteExpiredVersions(t *testing.T) { } func TestListDashboardVersions(t *testing.T) { - dashboardVersionStore := newDashboardVersionStoreFake() - dashboardVersionService := Service{store: dashboardVersionStore} + t.Run("List all versions for a given Dashboard ID", func(t *testing.T) { + dashboardVersionStore := newDashboardVersionStoreFake() + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService} + dashboardVersionStore.ExpectedListVersions = []*dashver.DashboardVersion{ + {ID: 1, DashboardID: 42}, + } + dashboardService.On("GetDashboardUIDByID", mock.Anything, + mock.AnythingOfType("*dashboards.GetDashboardRefByIDQuery")). + Return(&dashboards.DashboardRef{UID: "uid"}, nil) - t.Run("Get all versions for a given Dashboard ID", func(t *testing.T) { - query := dashver.ListDashboardVersionsQuery{} - dashboardVersionStore.ExpectedListVersions = []*dashver.DashboardVersion{{}} + query := dashver.ListDashboardVersionsQuery{DashboardID: 42} res, err := dashboardVersionService.List(context.Background(), &query) require.Nil(t, err) require.Equal(t, 1, len(res)) + // validate that the UID was populated + require.EqualValues(t, []*dashver.DashboardVersionDTO{{ID: 1, DashboardID: 42, DashboardUID: "uid"}}, res) + }) + + t.Run("List all versions for a non-existent DashboardID", func(t *testing.T) { + dashboardVersionStore := newDashboardVersionStoreFake() + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, log: log.NewNopLogger()} + dashboardVersionStore.ExpectedListVersions = []*dashver.DashboardVersion{ + {ID: 1, DashboardID: 42}, + } + dashboardService.On("GetDashboardUIDByID", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardRefByIDQuery")). + Return(nil, dashboards.ErrDashboardNotFound).Once() + + query := dashver.ListDashboardVersionsQuery{DashboardID: 42} + res, err := dashboardVersionService.List(context.Background(), &query) + require.Nil(t, err) + require.Equal(t, 1, len(res)) + // The DashboardID remains populated with the given value, even though the dash was not found + require.EqualValues(t, []*dashver.DashboardVersionDTO{{ID: 1, DashboardID: 42}}, res) + }) + + t.Run("List all versions for a given DashboardUID", func(t *testing.T) { + dashboardVersionStore := newDashboardVersionStoreFake() + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, log: log.NewNopLogger()} + dashboardVersionStore.ExpectedListVersions = []*dashver.DashboardVersion{{DashboardID: 42, ID: 1}} + dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")). + Return(&dashboards.Dashboard{ID: 42}, nil) + + query := dashver.ListDashboardVersionsQuery{DashboardUID: "uid"} + res, err := dashboardVersionService.List(context.Background(), &query) + require.Nil(t, err) + require.Equal(t, 1, len(res)) + // validate that the dashboardID was populated from the GetDashboard method call. + require.EqualValues(t, []*dashver.DashboardVersionDTO{{ID: 1, DashboardID: 42, DashboardUID: "uid"}}, res) + }) + + t.Run("List all versions for a given non-existent DashboardUID", func(t *testing.T) { + dashboardVersionStore := newDashboardVersionStoreFake() + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, log: log.NewNopLogger()} + dashboardVersionStore.ExpectedListVersions = []*dashver.DashboardVersion{{DashboardID: 42, ID: 1}} + dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")). + Return(nil, dashboards.ErrDashboardNotFound) + + query := dashver.ListDashboardVersionsQuery{DashboardUID: "uid"} + res, err := dashboardVersionService.List(context.Background(), &query) + require.Nil(t, err) + require.Equal(t, 1, len(res)) + // validate that the dashboardUID & ID are populated, even though the dash was not found + require.EqualValues(t, []*dashver.DashboardVersionDTO{{ID: 1, DashboardID: 42, DashboardUID: "uid"}}, res) + }) + + t.Run("List Dashboard versions - error from store", func(t *testing.T) { + dashboardVersionStore := newDashboardVersionStoreFake() + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardVersionService := Service{store: dashboardVersionStore, dashSvc: dashboardService, log: log.NewNopLogger()} + dashboardVersionStore.ExpectedError = dashver.ErrDashboardVersionNotFound + + query := dashver.ListDashboardVersionsQuery{DashboardID: 42, DashboardUID: "42"} + res, err := dashboardVersionService.List(context.Background(), &query) + require.Nil(t, res) + require.ErrorIs(t, err, dashver.ErrDashboardVersionNotFound) }) } diff --git a/pkg/services/dashboardversion/dashverimpl/store_test.go b/pkg/services/dashboardversion/dashverimpl/store_test.go index 25255828dd8..753a6654809 100644 --- a/pkg/services/dashboardversion/dashverimpl/store_test.go +++ b/pkg/services/dashboardversion/dashverimpl/store_test.go @@ -34,8 +34,8 @@ func testIntegrationGetDashboardVersion(t *testing.T, fn getStore) { res, err := dashVerStore.Get(context.Background(), &query) require.Nil(t, err) - assert.Equal(t, query.DashboardID, savedDash.ID) - assert.Equal(t, query.Version, savedDash.Version) + assert.Equal(t, savedDash.ID, res.ID) + assert.Equal(t, savedDash.Version, res.Version) assert.Equal(t, createdById, res.CreatedBy) dashCmd := &dashboards.Dashboard{ diff --git a/pkg/services/dashboardversion/model.go b/pkg/services/dashboardversion/model.go index b6a00918373..35cb8facc48 100644 --- a/pkg/services/dashboardversion/model.go +++ b/pkg/services/dashboardversion/model.go @@ -46,10 +46,13 @@ func (v *DashboardVersion) ToDTO(dashUid string) *DashboardVersionDTO { } } +// GetDashboardVersionQuery is used to Get a dashboard version. Only one of +// DashboardID and DashboardUID are required. type GetDashboardVersionQuery struct { - DashboardID int64 - OrgID int64 - Version int + DashboardID int64 + DashboardUID string + OrgID int64 + Version int } type DeleteExpiredVersionsCommand struct {