chore(dashboard version service): make method sigs more consistent (#60736)

The DashboardVersion struct is the database object; the DashboardVersionDTO is the object that should be sent to the API layer.

In the future I'd like to move DashboardVersion to dashverimpl and un-export it, but there are a few places that Insert directly into that table, not all of which are test fixtures, so that should wait until we clean up at least the DashboardService's use of it.
This commit is contained in:
Kristin Laemmert
2022-12-27 17:17:24 +01:00
committed by GitHub
parent 29276581d2
commit 6e9419ea80
9 changed files with 67 additions and 37 deletions

View File

@@ -133,7 +133,7 @@ func TestDashboardAPIEndpoint(t *testing.T) {
fakeDash.FolderId = 1
fakeDash.HasACL = false
fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake()
fakeDashboardVersionService.ExpectedDashboardVersion = &dashver.DashboardVersion{}
fakeDashboardVersionService.ExpectedDashboardVersion = &dashver.DashboardVersionDTO{}
teamService := &teamtest.FakeService{}
dashboardService := dashboards.NewFakeDashboardService(t)
dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*models.GetDashboardQuery")).Run(func(args mock.Arguments) {
@@ -241,7 +241,7 @@ func TestDashboardAPIEndpoint(t *testing.T) {
fakeDash.FolderId = 1
fakeDash.HasACL = true
fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake()
fakeDashboardVersionService.ExpectedDashboardVersion = &dashver.DashboardVersion{}
fakeDashboardVersionService.ExpectedDashboardVersion = &dashver.DashboardVersionDTO{}
teamService := &teamtest.FakeService{}
dashboardService := dashboards.NewFakeDashboardService(t)
dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*models.GetDashboardQuery")).Run(func(args mock.Arguments) {
@@ -787,7 +787,7 @@ func TestDashboardAPIEndpoint(t *testing.T) {
t.Run("Given two dashboards being compared", func(t *testing.T) {
fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake()
fakeDashboardVersionService.ExpectedDashboardVersions = []*dashver.DashboardVersion{
fakeDashboardVersionService.ExpectedDashboardVersions = []*dashver.DashboardVersionDTO{
{
DashboardID: 1,
Version: 1,
@@ -874,7 +874,7 @@ func TestDashboardAPIEndpoint(t *testing.T) {
Version: 1,
}
fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake()
fakeDashboardVersionService.ExpectedDashboardVersions = []*dashver.DashboardVersion{
fakeDashboardVersionService.ExpectedDashboardVersions = []*dashver.DashboardVersionDTO{
{
DashboardID: 2,
Version: 1,
@@ -908,7 +908,7 @@ func TestDashboardAPIEndpoint(t *testing.T) {
}).Return(nil, nil)
fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake()
fakeDashboardVersionService.ExpectedDashboardVersions = []*dashver.DashboardVersion{
fakeDashboardVersionService.ExpectedDashboardVersions = []*dashver.DashboardVersionDTO{
{
DashboardID: 2,
Version: 1,

View File

@@ -5,7 +5,7 @@ import (
)
type Service interface {
Get(context.Context, *GetDashboardVersionQuery) (*DashboardVersion, error)
Get(context.Context, *GetDashboardVersionQuery) (*DashboardVersionDTO, error)
DeleteExpired(context.Context, *DeleteExpiredVersionsCommand) error
List(context.Context, *ListDashboardVersionsQuery) ([]*DashboardVersionDTO, error)
}

View File

@@ -26,13 +26,15 @@ func ProvideService(db db.DB) dashver.Service {
}
}
func (s *Service) Get(ctx context.Context, query *dashver.GetDashboardVersionQuery) (*dashver.DashboardVersion, error) {
func (s *Service) Get(ctx context.Context, query *dashver.GetDashboardVersionQuery) (*dashver.DashboardVersionDTO, error) {
version, err := s.store.Get(ctx, query)
if err != nil {
return nil, err
}
version.Data.Set("id", version.DashboardID)
return version, nil
// FIXME: the next PR will add the dashboardService so we can grab the DashboardUID
return version.ToDTO(""), nil
}
func (s *Service) DeleteExpired(ctx context.Context, cmd *dashver.DeleteExpiredVersionsCommand) error {
@@ -70,5 +72,14 @@ func (s *Service) List(ctx context.Context, query *dashver.ListDashboardVersions
if query.Limit == 0 {
query.Limit = 1000
}
return s.store.List(ctx, query)
dvs, err := s.store.List(ctx, query)
if err != nil {
return nil, err
}
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("")
}
return dtos, nil
}

View File

@@ -5,10 +5,11 @@ import (
"errors"
"testing"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/components/simplejson"
dashver "github.com/grafana/grafana/pkg/services/dashboardversion"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/require"
)
func TestDashboardVersionService(t *testing.T) {
@@ -23,7 +24,7 @@ func TestDashboardVersionService(t *testing.T) {
dashboardVersionStore.ExpectedDashboardVersion = dashboard
dashboardVersion, err := dashboardVersionService.Get(context.Background(), &dashver.GetDashboardVersionQuery{})
require.NoError(t, err)
require.Equal(t, dashboardVersion, dashboard)
require.Equal(t, dashboard.ToDTO(""), dashboardVersion)
})
}
@@ -59,7 +60,7 @@ func TestListDashboardVersions(t *testing.T) {
t.Run("Get all versions for a given Dashboard ID", func(t *testing.T) {
query := dashver.ListDashboardVersionsQuery{}
dashboardVersionStore.ExpectedListVersions = []*dashver.DashboardVersionDTO{{}}
dashboardVersionStore.ExpectedListVersions = []*dashver.DashboardVersion{{}}
res, err := dashboardVersionService.List(context.Background(), &query)
require.Nil(t, err)
require.Equal(t, 1, len(res))
@@ -70,7 +71,7 @@ type FakeDashboardVersionStore struct {
ExpectedDashboardVersion *dashver.DashboardVersion
ExptectedDeletedVersions int64
ExpectedVersions []interface{}
ExpectedListVersions []*dashver.DashboardVersionDTO
ExpectedListVersions []*dashver.DashboardVersion
ExpectedError error
}
@@ -90,6 +91,6 @@ func (f *FakeDashboardVersionStore) DeleteBatch(ctx context.Context, cmd *dashve
return f.ExptectedDeletedVersions, f.ExpectedError
}
func (f *FakeDashboardVersionStore) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersionDTO, error) {
func (f *FakeDashboardVersionStore) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersion, error) {
return f.ExpectedListVersions, f.ExpectedError
}

View File

@@ -59,16 +59,15 @@ func (ss *sqlxStore) DeleteBatch(ctx context.Context, cmd *dashver.DeleteExpired
return deleted, err
}
func (ss *sqlxStore) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersionDTO, error) {
var dashboardVersion []*dashver.DashboardVersionDTO
func (ss *sqlxStore) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersion, error) {
var dashboardVersion []*dashver.DashboardVersion
qr := `SELECT dashboard_version.id,
dashboard_version.dashboard_id,
dashboard_version.parent_version,
dashboard_version.restored_from,
dashboard_version.version,
dashboard_version.created,
dashboard_version.message,
"user".login as created_by_login
dashboard_version.message
FROM dashboard_version
LEFT JOIN "user" ON "user".id = dashboard_version.created_by
LEFT JOIN dashboard ON dashboard.id = dashboard_version.dashboard_id

View File

@@ -10,5 +10,5 @@ type store interface {
Get(context.Context, *dashver.GetDashboardVersionQuery) (*dashver.DashboardVersion, error)
GetBatch(context.Context, *dashver.DeleteExpiredVersionsCommand, int, int) ([]interface{}, error)
DeleteBatch(context.Context, *dashver.DeleteExpiredVersionsCommand, []interface{}) (int64, error)
List(context.Context, *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersionDTO, error)
List(context.Context, *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersion, error)
}

View File

@@ -71,8 +71,8 @@ func (ss *sqlStore) DeleteBatch(ctx context.Context, cmd *dashver.DeleteExpiredV
return deleted, err
}
func (ss *sqlStore) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersionDTO, error) {
var dashboardVersion []*dashver.DashboardVersionDTO
func (ss *sqlStore) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersion, error) {
var dashboardVersion []*dashver.DashboardVersion
err := ss.db.WithDbSession(ctx, func(sess *db.Session) error {
err := sess.Table("dashboard_version").
Select(`dashboard_version.id,

View File

@@ -7,8 +7,8 @@ import (
)
type FakeDashboardVersionService struct {
ExpectedDashboardVersion *dashver.DashboardVersion
ExpectedDashboardVersions []*dashver.DashboardVersion
ExpectedDashboardVersion *dashver.DashboardVersionDTO
ExpectedDashboardVersions []*dashver.DashboardVersionDTO
ExpectedListDashboarVersions []*dashver.DashboardVersionDTO
counter int
ExpectedError error
@@ -18,7 +18,7 @@ func NewDashboardVersionServiceFake() *FakeDashboardVersionService {
return &FakeDashboardVersionService{}
}
func (f *FakeDashboardVersionService) Get(ctx context.Context, query *dashver.GetDashboardVersionQuery) (*dashver.DashboardVersion, error) {
func (f *FakeDashboardVersionService) Get(ctx context.Context, query *dashver.GetDashboardVersionQuery) (*dashver.DashboardVersionDTO, error) {
if len(f.ExpectedDashboardVersions) == 0 {
return f.ExpectedDashboardVersion, f.ExpectedError
}

View File

@@ -12,6 +12,10 @@ var (
ErrNoVersionsForDashboardID = errors.New("no dashboard versions found for the given DashboardId")
)
// DashboardVersion represents a dashboard version in the database. Ideally this
// will be moved into dashverimpl and unexported, but there are a few test
// fixtures that insert DashboardVersions directly into a database which must be
// refactored first.
type DashboardVersion struct {
ID int64 `json:"id" xorm:"pk autoincr 'id'" db:"id"`
DashboardID int64 `json:"dashboardId" xorm:"dashboard_id" db:"dashboard_id"`
@@ -26,6 +30,22 @@ type DashboardVersion struct {
Data *simplejson.Json `json:"data" db:"data"`
}
// ToDTO converts a DashboardVersion to a DashboardVersionDTO.
func (v *DashboardVersion) ToDTO(dashUid string) *DashboardVersionDTO {
return &DashboardVersionDTO{
ID: v.ID,
DashboardID: v.DashboardID,
DashboardUID: dashUid,
ParentVersion: v.ParentVersion,
RestoredFrom: v.RestoredFrom,
Version: v.Version,
Created: v.Created,
CreatedBy: v.CreatedBy,
Message: v.Message,
Data: v.Data,
}
}
type GetDashboardVersionQuery struct {
DashboardID int64
OrgID int64
@@ -45,22 +65,21 @@ type ListDashboardVersionsQuery struct {
}
type DashboardVersionDTO struct {
ID int64 `json:"id" xorm:"id" db:"id"`
DashboardID int64 `json:"dashboardId" xorm:"dashboard_id" db:"dashboard_id"`
DashboardUID string `json:"dashboardUid" xorm:"dashboard_uid" db:"dashboard_uid"`
ParentVersion int `json:"parentVersion" db:"parent_version"`
RestoredFrom int `json:"restoredFrom" db:"restored_from"`
Version int `json:"version" db:"version"`
Created time.Time `json:"created" db:"created"`
// Since we get created by with left join user table, this can be null technically,
// but in reality it will always be set, when database is not corrupted.
CreatedBy *string `json:"createdBy" db:"created_by_login"`
Message string `json:"message" db:"message"`
ID int64 `json:"id"`
DashboardID int64 `json:"dashboardId"`
DashboardUID string `json:"dashboardUid"`
ParentVersion int `json:"parentVersion"`
RestoredFrom int `json:"restoredFrom"`
Version int `json:"version"`
Created time.Time `json:"created"`
CreatedBy int64 `json:"createdBy"`
Message string `json:"message"`
Data *simplejson.Json `json:"data" db:"data"`
}
// DashboardVersionMeta extends the dashboard version model with the names
// DashboardVersionMeta extends the DashboardVersionDTO with the names
// associated with the UserIds, overriding the field with the same name from
// the DashboardVersion model.
// the DashboardVersionDTO model.
type DashboardVersionMeta struct {
ID int64 `json:"id"`
DashboardID int64 `json:"dashboardId"`