Skip to content

Commit

Permalink
SQLstore: Fix fetching an inexistent playlist response (grafana#51962)
Browse files Browse the repository at this point in the history
* SQLstore: Fix fetching and deleting an inexistent playlist

xorm's session.Get() does not return an error if the raw does not exist.
It returns a boolean instead.
The playlist `sqlstore.GetPlaylist()` used to check only the error and in case
of inexistent UID didn't return an error.
  • Loading branch information
papagian authored Jul 13, 2022
1 parent 169a1c5 commit 4ff0f00
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
5 changes: 4 additions & 1 deletion pkg/services/sqlstore/playlist.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ func (ss *SQLStore) GetPlaylist(ctx context.Context, query *models.GetPlaylistBy

return ss.WithDbSession(ctx, func(sess *DBSession) error {
playlist := models.Playlist{UID: query.UID, OrgId: query.OrgId}
_, err := sess.Get(&playlist)
exists, err := sess.Get(&playlist)
if !exists {
return models.ErrPlaylistNotFound
}
query.Result = &playlist

return err
Expand Down
20 changes: 18 additions & 2 deletions pkg/services/sqlstore/playlist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ func TestIntegrationPlaylistDataAccess(t *testing.T) {
require.NoError(t, err)
uid := cmd.Result.UID

t.Run("Can get playlist", func(t *testing.T) {
get := &models.GetPlaylistByUidQuery{UID: uid, OrgId: 1}
err = ss.GetPlaylist(context.Background(), get)
require.NoError(t, err)
require.NotNil(t, get.Result)
require.Equal(t, get.Result.Name, "NYC office")
require.Equal(t, get.Result.Interval, "10m")
})

t.Run("Can get playlist items", func(t *testing.T) {
get := &models.GetPlaylistItemsByUidQuery{PlaylistUID: uid, OrgId: 1}
err = ss.GetPlaylistItem(context.Background(), get)
Expand All @@ -49,11 +58,18 @@ func TestIntegrationPlaylistDataAccess(t *testing.T) {

getQuery := models.GetPlaylistByUidQuery{UID: uid, OrgId: 1}
err = ss.GetPlaylist(context.Background(), &getQuery)
require.NoError(t, err)
require.Equal(t, uid, getQuery.Result.UID, "playlist should've been removed")
require.Error(t, err)
require.ErrorIs(t, err, models.ErrPlaylistNotFound)
})
})

t.Run("Get playlist that doesn't exist", func(t *testing.T) {
get := &models.GetPlaylistByUidQuery{UID: "unknown", OrgId: 1}
err := ss.GetPlaylist(context.Background(), get)
require.Error(t, err)
require.ErrorIs(t, err, models.ErrPlaylistNotFound)
})

t.Run("Delete playlist that doesn't exist", func(t *testing.T) {
deleteQuery := models.DeletePlaylistCommand{UID: "654312", OrgId: 1}
err := ss.DeletePlaylist(context.Background(), &deleteQuery)
Expand Down

0 comments on commit 4ff0f00

Please sign in to comment.