Playlist: Add create+update timestamps to the database (#76295)

.github/CODEOWNERS

@@ -114,7 +114,7 @@
 /pkg/services/navtree/ @grafana/backend-platform
 /pkg/services/notifications/ @grafana/backend-platform
 /pkg/services/org/ @grafana/backend-platform
-/pkg/services/playlist/ @grafana/backend-platform
+/pkg/services/playlist/ @grafana/grafana-app-platform-squad
 /pkg/services/plugindashboards/ @grafana/backend-platform
 /pkg/services/preference/ @grafana/backend-platform
 /pkg/services/provisioning/ @grafana/backend-platform

pkg/services/playlist/model.go

@@ -8,9 +8,8 @@ import (
 
 // Typed errors
 var (
-	ErrPlaylistNotFound                = errors.New("Playlist not found")
+	ErrPlaylistNotFound        = errors.New("Playlist not found")
-	ErrPlaylistFailedGenerateUniqueUid = errors.New("failed to generate unique playlist UID")
+	ErrCommandValidationFailed = errors.New("command missing required fields")
-	ErrCommandValidationFailed         = errors.New("command missing required fields")
 )
 
 // Playlist model
@@ -20,6 +19,12 @@ type Playlist struct {
 	Name     string `json:"name" db:"name"`
 	Interval string `json:"interval" db:"interval"`
 	OrgId    int64  `json:"-" db:"org_id"`
+
+	// Added for kubernetes migration + synchronization
+	// Hidden from json because this is used for openapi generation
+	// Using int64 rather than time.Time to avoid database issues with time support
+	CreatedAt int64 `json:"-" db:"created_at"`
+	UpdatedAt int64 `json:"-" db:"updated_at"`
 }
 
 type PlaylistDTO = playlist.Spec
@@ -54,6 +59,8 @@ type CreatePlaylistCommand struct {
 	Interval string         `json:"interval"`
 	Items    []PlaylistItem `json:"items"`
 	OrgId    int64          `json:"-"`
+	// Used to create playlists from kubectl with a known uid/name
+	UID string `json:"-"`
 }
 
 type DeletePlaylistCommand struct {

pkg/services/playlist/model_test.go

@@ -1,64 +0,0 @@
-package playlist
-
-import (
-	"encoding/json"
-	"fmt"
-	"testing"
-
-	"github.com/grafana/grafana/pkg/kinds/playlist"
-	"github.com/grafana/grafana/pkg/util"
-	"github.com/stretchr/testify/require"
-)
-
-func TestPlaylistConversion(t *testing.T) {
-	src := PlaylistDTO{
-		Uid:      "abc",
-		Name:     "TeamA",
-		Interval: "10s",
-		Items: []playlist.Item{
-			{Title: util.Pointer("First"), Type: playlist.ItemTypeDashboardByUid, Value: "UID0"},
-			{Title: util.Pointer("Second"), Type: playlist.ItemTypeDashboardByTag, Value: "tagA"},
-			{Title: util.Pointer("Third"), Type: playlist.ItemTypeDashboardById, Value: "123"},
-		},
-	}
-
-	dst := PlaylistToResource(src)
-
-	require.Equal(t, "abc", src.Uid)
-	require.Equal(t, "abc", dst.Metadata.Name)
-	require.Equal(t, src.Name, dst.Spec.Name)
-
-	out, err := json.MarshalIndent(dst, "", "  ")
-	require.NoError(t, err)
-	fmt.Printf("%s", string(out))
-	require.JSONEq(t, `{
-		"apiVersion": "v0-0-alpha",
-		"kind": "Playlist",
-		"metadata": {
-		  "name": "abc",
-		  "creationTimestamp": null
-		},
-		"spec": {
-		  "interval": "10s",
-		  "items": [
-			{
-			  "title": "First",
-			  "type": "dashboard_by_uid",
-			  "value": "UID0"
-			},
-			{
-			  "title": "Second",
-			  "type": "dashboard_by_tag",
-			  "value": "tagA"
-			},
-			{
-			  "title": "Third",
-			  "type": "dashboard_by_id",
-			  "value": "123"
-			}
-		  ],
-		  "name": "TeamA",
-		  "uid": ""
-		}
-	  }`, string(out))
-}

pkg/services/playlist/playlistimpl/playlist.go

@@ -16,17 +16,8 @@ type Service struct {
 var _ playlist.Service = &Service{}
 
 func ProvideService(db db.DB, toggles featuremgmt.FeatureToggles, objserver entity.EntityStoreServer) playlist.Service {
-	var sqlstore store
+	sqlstore := &sqlStore{
-
+		db: db,
-	// 🐢🐢🐢 pick the store
-	if toggles.IsEnabled(featuremgmt.FlagNewDBLibrary) {
-		sqlstore = &sqlxStore{
-			sess: db.GetSqlxSession(),
-		}
-	} else {
-		sqlstore = &sqlStore{
-			db: db,
-		}
 	}
 	return &Service{store: sqlstore}
 }

pkg/services/playlist/playlistimpl/sqlx_store.go

@@ -1,201 +0,0 @@
-package playlistimpl
-
-import (
-	"context"
-	"database/sql"
-	"errors"
-
-	"github.com/grafana/grafana/pkg/services/playlist"
-	"github.com/grafana/grafana/pkg/services/sqlstore/session"
-	"github.com/grafana/grafana/pkg/services/star"
-)
-
-type sqlxStore struct {
-	sess *session.SessionDB
-}
-
-func (s *sqlxStore) Insert(ctx context.Context, cmd *playlist.CreatePlaylistCommand) (*playlist.Playlist, error) {
-	p := playlist.Playlist{}
-	var err error
-	uid, err := newGenerateAndValidateNewPlaylistUid(ctx, s.sess, cmd.OrgId)
-	if err != nil {
-		return nil, err
-	}
-
-	p = playlist.Playlist{
-		Name:     cmd.Name,
-		Interval: cmd.Interval,
-		OrgId:    cmd.OrgId,
-		UID:      uid,
-	}
-
-	err = s.sess.WithTransaction(ctx, func(tx *session.SessionTx) error {
-		query := `INSERT INTO playlist (name, "interval", org_id, uid) VALUES (?, ?, ?, ?)`
-		var err error
-		p.Id, err = tx.ExecWithReturningId(ctx, query, p.Name, p.Interval, p.OrgId, p.UID)
-		if err != nil {
-			return err
-		}
-
-		if len(cmd.Items) > 0 {
-			playlistItems := make([]playlist.PlaylistItem, 0)
-			for order, item := range cmd.Items {
-				playlistItems = append(playlistItems, playlist.PlaylistItem{
-					PlaylistId: p.Id,
-					Type:       item.Type,
-					Value:      item.Value,
-					Order:      order + 1,
-					Title:      item.Title,
-				})
-			}
-			query := `INSERT INTO playlist_item (playlist_id, type, value, title, "order") VALUES (:playlist_id, :type, :value, :title, :order)`
-			_, err = tx.NamedExec(ctx, query, playlistItems)
-			if err != nil {
-				return err
-			}
-		}
-		return nil
-	})
-
-	return &p, err
-}
-
-func (s *sqlxStore) Update(ctx context.Context, cmd *playlist.UpdatePlaylistCommand) (*playlist.PlaylistDTO, error) {
-	dto := playlist.PlaylistDTO{}
-
-	// Get the id of playlist to be updated with orgId and UID
-	existingPlaylist, err := s.Get(ctx, &playlist.GetPlaylistByUidQuery{UID: cmd.UID, OrgId: cmd.OrgId})
-	if err != nil {
-		return nil, err
-	}
-
-	// Create object to be update to
-	p := playlist.Playlist{
-		Id:       existingPlaylist.Id,
-		UID:      cmd.UID,
-		OrgId:    cmd.OrgId,
-		Name:     cmd.Name,
-		Interval: cmd.Interval,
-	}
-
-	err = s.sess.WithTransaction(ctx, func(tx *session.SessionTx) error {
-		query := `UPDATE playlist SET uid=:uid, org_id=:org_id, name=:name, "interval"=:interval WHERE id=:id`
-		_, err = tx.NamedExec(ctx, query, p)
-		if err != nil {
-			return err
-		}
-
-		if _, err = tx.Exec(ctx, "DELETE FROM playlist_item WHERE playlist_id = ?", p.Id); err != nil {
-			return err
-		}
-
-		playlistItems := make([]playlist.PlaylistItem, 0)
-
-		for index, item := range cmd.Items {
-			playlistItems = append(playlistItems, playlist.PlaylistItem{
-				PlaylistId: p.Id,
-				Type:       item.Type,
-				Value:      item.Value,
-				Order:      index + 1,
-				Title:      item.Title,
-			})
-		}
-		query = `INSERT INTO playlist_item (playlist_id, type, value, title, "order") VALUES (:playlist_id, :type, :value, :title, :order)`
-		_, err = tx.NamedExec(ctx, query, playlistItems)
-		return err
-	})
-
-	return &dto, err
-}
-
-func (s *sqlxStore) Get(ctx context.Context, query *playlist.GetPlaylistByUidQuery) (*playlist.Playlist, error) {
-	if query.UID == "" || query.OrgId == 0 {
-		return nil, playlist.ErrCommandValidationFailed
-	}
-
-	p := playlist.Playlist{}
-	err := s.sess.Get(ctx, &p, "SELECT * FROM playlist WHERE uid=? AND org_id=?", query.UID, query.OrgId)
-	if err != nil {
-		if errors.Is(err, sql.ErrNoRows) {
-			return nil, playlist.ErrPlaylistNotFound
-		}
-		return nil, err
-	}
-	return &p, err
-}
-
-func (s *sqlxStore) Delete(ctx context.Context, cmd *playlist.DeletePlaylistCommand) error {
-	if cmd.UID == "" || cmd.OrgId == 0 {
-		return playlist.ErrCommandValidationFailed
-	}
-
-	p := playlist.Playlist{}
-	if err := s.sess.Get(ctx, &p, "SELECT * FROM playlist WHERE uid=? AND org_id=?", cmd.UID, cmd.OrgId); err != nil {
-		if errors.Is(err, sql.ErrNoRows) {
-			return nil
-		}
-		return err
-	}
-
-	err := s.sess.WithTransaction(ctx, func(tx *session.SessionTx) error {
-		if _, err := tx.Exec(ctx, "DELETE FROM playlist WHERE uid = ? and org_id = ?", cmd.UID, cmd.OrgId); err != nil {
-			return err
-		}
-
-		if _, err := tx.Exec(ctx, "DELETE FROM playlist_item WHERE playlist_id = ?", p.Id); err != nil {
-			return err
-		}
-		return nil
-	})
-
-	return err
-}
-
-func (s *sqlxStore) List(ctx context.Context, query *playlist.GetPlaylistsQuery) (playlist.Playlists, error) {
-	playlists := make(playlist.Playlists, 0)
-	if query.OrgId == 0 {
-		return playlists, playlist.ErrCommandValidationFailed
-	}
-
-	var err error
-	if query.Name == "" {
-		err = s.sess.Select(
-			ctx, &playlists, "SELECT * FROM playlist WHERE org_id = ? LIMIT ?", query.OrgId, query.Limit)
-	} else {
-		err = s.sess.Select(
-			ctx, &playlists, "SELECT * FROM playlist WHERE org_id = ? AND name LIKE ? LIMIT ?", query.OrgId, "%"+query.Name+"%", query.Limit)
-	}
-	return playlists, err
-}
-
-func (s *sqlxStore) GetItems(ctx context.Context, query *playlist.GetPlaylistItemsByUidQuery) ([]playlist.PlaylistItem, error) {
-	var playlistItems = make([]playlist.PlaylistItem, 0)
-	if query.PlaylistUID == "" || query.OrgId == 0 {
-		return playlistItems, star.ErrCommandValidationFailed
-	}
-
-	var p = playlist.Playlist{}
-	err := s.sess.Get(ctx, &p, "SELECT * FROM playlist WHERE uid=? AND org_id=?", query.PlaylistUID, query.OrgId)
-	if err != nil {
-		return playlistItems, err
-	}
-
-	err = s.sess.Select(ctx, &playlistItems, "SELECT * FROM playlist_item WHERE playlist_id=?", p.Id)
-	return playlistItems, err
-}
-
-func newGenerateAndValidateNewPlaylistUid(ctx context.Context, sess *session.SessionDB, orgId int64) (string, error) {
-	for i := 0; i < 3; i++ {
-		uid := generateNewUid()
-		p := playlist.Playlist{OrgId: orgId, UID: uid}
-		err := sess.Get(ctx, &p, "SELECT * FROM playlist WHERE uid=? AND org_id=?", uid, orgId)
-		if err != nil {
-			if errors.Is(err, sql.ErrNoRows) {
-				return uid, nil
-			}
-			return "", err
-		}
-	}
-
-	return "", playlist.ErrPlaylistFailedGenerateUniqueUid
-}

pkg/services/playlist/playlistimpl/sqlx_store_test.go

@@ -1,16 +0,0 @@
-package playlistimpl
-
-import (
-	"testing"
-
-	"github.com/grafana/grafana/pkg/infra/db"
-)
-
-func TestIntegrationSQLxPlaylistDataAccess(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skipping integration test")
-	}
-	testIntegrationPlaylistDataAccess(t, func(ss db.DB) store {
-		return &sqlxStore{sess: ss.GetSqlxSession()}
-	})
-}

pkg/services/playlist/playlistimpl/store_test.go

@@ -3,6 +3,7 @@ package playlistimpl
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
@@ -15,6 +16,7 @@ type getStore func(db.DB) store
 func testIntegrationPlaylistDataAccess(t *testing.T, fn getStore) {
 	t.Helper()
 
+	start := time.Now().UnixMilli()
 	ss := db.InitTestDB(t)
 	playlistStore := fn(ss)
 
@@ -33,6 +35,8 @@ func testIntegrationPlaylistDataAccess(t *testing.T, fn getStore) {
 			pl, err := playlistStore.Get(context.Background(), get)
 			require.NoError(t, err)
 			require.Equal(t, p.Id, pl.Id)
+			require.GreaterOrEqual(t, pl.CreatedAt, start)
+			require.GreaterOrEqual(t, pl.UpdatedAt, start)
 		})
 
 		t.Run("Can get playlist items", func(t *testing.T) {
@@ -43,6 +47,7 @@ func testIntegrationPlaylistDataAccess(t *testing.T, fn getStore) {
 		})
 
 		t.Run("Can update playlist", func(t *testing.T) {
+			time.Sleep(time.Millisecond * 2)
 			items := []playlist.PlaylistItem{
 				{Title: "influxdb", Value: "influxdb", Type: "dashboard_by_tag"},
 				{Title: "Backend response times", Value: "2", Type: "dashboard_by_id"},
@@ -50,6 +55,14 @@ func testIntegrationPlaylistDataAccess(t *testing.T, fn getStore) {
 			query := playlist.UpdatePlaylistCommand{Name: "NYC office ", OrgId: 1, UID: uid, Interval: "10s", Items: items}
 			_, err = playlistStore.Update(context.Background(), &query)
 			require.NoError(t, err)
+
+			// Now check that UpdatedAt has increased
+			pl, err := playlistStore.Get(context.Background(), &playlist.GetPlaylistByUidQuery{UID: uid, OrgId: 1})
+			require.NoError(t, err)
+			require.Equal(t, p.Id, pl.Id)
+			require.Equal(t, p.CreatedAt, pl.CreatedAt)
+			require.Greater(t, pl.UpdatedAt, p.UpdatedAt)
+			require.Greater(t, pl.UpdatedAt, pl.CreatedAt)
 		})
 
 		t.Run("Can remove playlist", func(t *testing.T) {
@@ -64,6 +77,32 @@ func testIntegrationPlaylistDataAccess(t *testing.T, fn getStore) {
 		})
 	})
 
+	t.Run("Can create playlist with known UID", func(t *testing.T) {
+		items := []playlist.PlaylistItem{
+			{Title: "graphite", Value: "graphite", Type: "dashboard_by_tag"},
+			{Title: "Backend response times", Value: "3", Type: "dashboard_by_id"},
+		}
+		cmd := playlist.CreatePlaylistCommand{Name: "NYC office", Interval: "10m", OrgId: 1,
+			Items: items,
+			UID:   "abcd",
+		}
+		p, err := playlistStore.Insert(context.Background(), &cmd)
+		require.NoError(t, err)
+		require.Equal(t, "abcd", p.UID)
+
+		// Should get an error with an invalid UID
+		cmd.UID = "invalid uid"
+		_, err = playlistStore.Insert(context.Background(), &cmd)
+		require.Error(t, err)
+
+		// cleanup
+		err = playlistStore.Delete(context.Background(), &playlist.DeletePlaylistCommand{
+			OrgId: 1,
+			UID:   "abcd",
+		})
+		require.NoError(t, err)
+	})
+
 	t.Run("Search playlist", func(t *testing.T) {
 		items := []playlist.PlaylistItem{
 			{Title: "graphite", Value: "graphite", Type: "dashboard_by_tag"},

pkg/services/playlist/playlistimpl/xorm_store.go

@@ -2,6 +2,7 @@ package playlistimpl
 
 import (
 	"context"
+	"time"
 
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/services/playlist"
@@ -13,22 +14,31 @@ type sqlStore struct {
 	db db.DB
 }
 
+var _ store = &sqlStore{}
+
 func (s *sqlStore) Insert(ctx context.Context, cmd *playlist.CreatePlaylistCommand) (*playlist.Playlist, error) {
 	p := playlist.Playlist{}
-	err := s.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
+	if cmd.UID == "" {
-		uid, err := generateAndValidateNewPlaylistUid(sess, cmd.OrgId)
+		cmd.UID = util.GenerateShortUID()
+	} else {
+		err := util.ValidateUID(cmd.UID)
 		if err != nil {
-			return err
+			return nil, err
 		}
+	}
 
+	err := s.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
+		ts := time.Now().UnixMilli()
 		p = playlist.Playlist{
-			Name:     cmd.Name,
+			Name:      cmd.Name,
-			Interval: cmd.Interval,
+			Interval:  cmd.Interval,
-			OrgId:    cmd.OrgId,
+			OrgId:     cmd.OrgId,
-			UID:      uid,
+			UID:       cmd.UID,
+			CreatedAt: ts,
+			UpdatedAt: ts,
 		}
 
-		_, err = sess.Insert(&p)
+		_, err := sess.Insert(&p)
 		if err != nil {
 			return err
 		}
@@ -67,6 +77,8 @@ func (s *sqlStore) Update(ctx context.Context, cmd *playlist.UpdatePlaylistComma
 			return err
 		}
 		p.Id = existingPlaylist.Id
+		p.CreatedAt = existingPlaylist.CreatedAt
+		p.UpdatedAt = time.Now().UnixMilli()
 
 		dto = playlist.PlaylistDTO{
 			Uid:      p.UID,
@@ -74,7 +86,7 @@ func (s *sqlStore) Update(ctx context.Context, cmd *playlist.UpdatePlaylistComma
 			Interval: p.Interval,
 		}
 
-		_, err = sess.Where("id=?", p.Id).Cols("name", "interval").Update(&p)
+		_, err = sess.Where("id=?", p.Id).Cols("name", "interval", "updated_at").Update(&p)
 		if err != nil {
 			return err
 		}
@@ -187,26 +199,3 @@ func (s *sqlStore) GetItems(ctx context.Context, query *playlist.GetPlaylistItem
 	})
 	return playlistItems, err
 }
-
-// generateAndValidateNewPlaylistUid generates a playlistUID and verifies that
-// the uid isn't already in use. This is deliberately overly cautious, since users
-// can also specify playlist uids during provisioning.
-func generateAndValidateNewPlaylistUid(sess *db.Session, orgId int64) (string, error) {
-	for i := 0; i < 3; i++ {
-		uid := generateNewUid()
-
-		playlist := playlist.Playlist{OrgId: orgId, UID: uid}
-		exists, err := sess.Get(&playlist)
-		if err != nil {
-			return "", err
-		}
-
-		if !exists {
-			return uid, nil
-		}
-	}
-
-	return "", playlist.ErrPlaylistFailedGenerateUniqueUid
-}
-
-var generateNewUid func() string = util.GenerateShortUID

pkg/services/sqlstore/migrations/playlist_mig.go

@@ -33,6 +33,14 @@ func addPlaylistMigrations(mg *Migrator) {
 		{Name: "value", Type: DB_Text, Nullable: false},
 		{Name: "title", Type: DB_Text, Nullable: false},
 	}))
+
+	// Add columns used for kubernetes dual write synchronization
+	mg.AddMigration("Add playlist column created_at", NewAddColumnMigration(playlistV2(), &Column{
+		Name: "created_at", Type: DB_BigInt, Nullable: false, Default: "0",
+	}))
+	mg.AddMigration("Add playlist column updated_at", NewAddColumnMigration(playlistV2(), &Column{
+		Name: "updated_at", Type: DB_BigInt, Nullable: false, Default: "0",
+	}))
 }
 
 func addPlaylistUIDMigration(mg *Migrator) {