Stars: include query history (#111979)

pkg/registry/apis/preferences/admission.go

@@ -0,0 +1,35 @@
+package preferences
+
+import (
+	"context"
+	"fmt"
+
+	"k8s.io/apiserver/pkg/admission"
+
+	preferences "github.com/grafana/grafana/apps/preferences/pkg/apis/preferences/v1alpha1"
+)
+
+func (b *APIBuilder) Mutate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) {
+	switch a.GetOperation() {
+	case admission.Create, admission.Update:
+		// ignore anything that is not CREATE | UPDATE
+	default:
+		return nil
+	}
+
+	obj := a.GetObject()
+	if obj == nil {
+		return nil
+	}
+
+	switch a.GetResource().Resource {
+	case "stars":
+		stars, ok := obj.(*preferences.Stars)
+		if !ok {
+			return fmt.Errorf("expected stars object: (%T)", obj)
+		}
+		stars.Spec.Normalize()
+		return nil
+	}
+	return nil
+}

pkg/registry/apis/preferences/legacy/queries.go

@@ -26,21 +26,29 @@ func mustTemplate(filename string) *template.Template {
 
 // Templates.
 var (
-	sqlStarsQuery       = mustTemplate("sql_stars_query.sql")
-	sqlStarsRV          = mustTemplate("sql_stars_rv.sql")
-	sqlPreferencesQuery = mustTemplate("sql_preferences_query.sql")
-	sqlPreferencesRV    = mustTemplate("sql_preferences_rv.sql")
-	sqlTeams            = mustTemplate("sql_teams.sql")
+	sqlDashboardStarsQuery = mustTemplate("sql_dashboard_stars.sql")
+	sqlDashboardStarsRV    = mustTemplate("sql_dashboard_stars_rv.sql")
+	sqlHistoryStarsQuery   = mustTemplate("sql_history_stars.sql")
+	sqlHistoryStarsInsert  = mustTemplate("sql_history_stars_insert.sql")
+	sqlHistoryStarsDelete  = mustTemplate("sql_history_stars_delete.sql")
+	sqlPreferencesQuery    = mustTemplate("sql_preferences_query.sql")
+	sqlPreferencesRV       = mustTemplate("sql_preferences_rv.sql")
+	sqlTeams               = mustTemplate("sql_teams.sql")
 )
 
 type starQuery struct {
 	sqltemplate.SQLTemplate
 
-	OrgID   int64 // >= 1 if UserID != ""
-	UserUID string
+	OrgID     int64 // >= 1 if UserID != ""
+	UserUID   string
+	UserID    int64 // for stars
+	QueryUIDs []string
+	QueryUID  string
 
-	StarTable string
-	UserTable string
+	StarTable              string
+	UserTable              string
+	QueryHistoryStarsTable string
+	QueryHistoryTable      string
 }
 
 func (r starQuery) Validate() error {
@@ -57,8 +65,10 @@ func newStarQueryReq(sql *legacysql.LegacyDatabaseHelper, user string, orgId int
 		UserUID: user,
 		OrgID:   orgId,
 
-		StarTable: sql.Table("star"),
-		UserTable: sql.Table("user"),
+		StarTable:              sql.Table("star"),
+		UserTable:              sql.Table("user"),
+		QueryHistoryStarsTable: sql.Table("query_history_star"),
+		QueryHistoryTable:      sql.Table("query_history"),
 	}
 }
 

pkg/registry/apis/preferences/legacy/queries_test.go

@@ -23,6 +23,15 @@ func TestStarsQueries(t *testing.T) {
 		return &v
 	}
 
+	getHistoryReq := func(orgId int64, userId int64, stars []string, star string) sqltemplate.SQLTemplate {
+		v := newStarQueryReq(nodb, "", orgId)
+		v.UserID = userId
+		v.QueryUIDs = stars
+		v.QueryUID = star
+		v.SQLTemplate = mocks.NewTestingSQLTemplate()
+		return &v
+	}
+
 	getPreferencesQuery := func(orgId int64, cb func(q *preferencesQuery)) sqltemplate.SQLTemplate {
 		v := newPreferencesQueryReq(nodb, orgId)
 		v.SQLTemplate = mocks.NewTestingSQLTemplate()
@@ -40,7 +49,7 @@ func TestStarsQueries(t *testing.T) {
 		RootDir:        "testdata",
 		SQLTemplatesFS: sqlTemplatesFS,
 		Templates: map[*template.Template][]mocks.TemplateTestCase{
-			sqlStarsQuery: {
+			sqlDashboardStarsQuery: {
 				{
 					Name: "all",
 					Data: getStarQuery(0, ""),
@@ -54,12 +63,42 @@ func TestStarsQueries(t *testing.T) {
 					Data: getStarQuery(3, "abc"),
 				},
 			},
-			sqlStarsRV: {
+			sqlDashboardStarsRV: {
 				{
 					Name: "get",
 					Data: getStarQuery(0, ""),
 				},
 			},
+			sqlHistoryStarsQuery: {
+				{
+					Name: "user",
+					Data: getStarQuery(1, "abc"),
+				},
+			},
+			sqlHistoryStarsQuery: {
+				{
+					Name: "org",
+					Data: getStarQuery(1, ""),
+				},
+			},
+			sqlHistoryStarsInsert: {
+				{
+					Name: "add star",
+					Data: getHistoryReq(1, 3, nil, "XXX"),
+				},
+			},
+			sqlHistoryStarsDelete: {
+				{
+					Name: "remove star",
+					Data: getHistoryReq(1, 3, []string{"xxx", "yyy"}, ""),
+				},
+			},
+			sqlHistoryStarsDelete: {
+				{
+					Name: "remove all star",
+					Data: getHistoryReq(1, 3, nil, ""),
+				},
+			},
 			sqlPreferencesQuery: {
 				{
 					Name: "all",

pkg/registry/apis/preferences/legacy/sql.go

@@ -12,6 +12,7 @@ import (
 	preferences "github.com/grafana/grafana/apps/preferences/pkg/apis/preferences/v1alpha1"
 	"github.com/grafana/grafana/pkg/apimachinery/identity"
 	pref "github.com/grafana/grafana/pkg/services/preference"
+	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/storage/legacysql"
 	"github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate"
 )
@@ -58,13 +59,16 @@ func (s *LegacySQL) getDashboardStars(ctx context.Context, orgId int64, user str
 
 	req := newStarQueryReq(sql, user, orgId)
 
-	q, err := sqltemplate.Execute(sqlStarsQuery, req)
+	q, err := sqltemplate.Execute(sqlDashboardStarsQuery, req)
 	if err != nil {
-		return nil, 0, fmt.Errorf("execute template %q: %w", sqlStarsQuery.Name(), err)
+		return nil, 0, fmt.Errorf("execute template %q: %w", sqlDashboardStarsQuery.Name(), err)
 	}
 
 	sess := sql.DB.GetSqlxSession()
 	rows, err := sess.Query(ctx, q, req.GetArgs()...)
+	if err != nil {
+		return nil, 0, err
+	}
 	defer func() {
 		if rows != nil {
 			_ = rows.Close()
@@ -111,7 +115,7 @@ func (s *LegacySQL) getDashboardStars(ctx context.Context, orgId int64, user str
 	// Find the RV unless it is a user query
 	if userUID == "" {
 		req.Reset()
-		q, err = sqltemplate.Execute(sqlStarsRV, req)
+		q, err = sqltemplate.Execute(sqlDashboardStarsRV, req)
 		if err != nil {
 			return nil, 0, fmt.Errorf("execute template %q: %w", sqlPreferencesRV.Name(), err)
 		}
@@ -132,6 +136,90 @@ func (s *LegacySQL) getDashboardStars(ctx context.Context, orgId int64, user str
 	return stars, updated.UnixMilli(), err
 }
 
+func (s *LegacySQL) getHistoryStars(ctx context.Context, orgId int64, user string) (map[string][]string, error) {
+	sql, err := s.db(ctx)
+	if err != nil {
+		return nil, err
+	}
+	req := newStarQueryReq(sql, user, orgId)
+
+	q, err := sqltemplate.Execute(sqlHistoryStarsQuery, req)
+	if err != nil {
+		return nil, fmt.Errorf("execute template %q: %w", sqlHistoryStarsQuery.Name(), err)
+	}
+
+	sess := sql.DB.GetSqlxSession()
+	rows, err := sess.Query(ctx, q, req.GetArgs()...)
+	if err != nil {
+		return nil, err
+	}
+	defer func() {
+		if rows != nil {
+			_ = rows.Close()
+		}
+	}()
+
+	last := user
+	res := make(map[string][]string)
+	buffer := make([]string, 0, 10)
+	var uid string
+
+	for rows.Next() {
+		err := rows.Scan(&uid, &user)
+		if err != nil {
+			return nil, err
+		}
+		if user != last && len(buffer) > 0 {
+			res[last] = buffer
+			buffer = make([]string, 0, 10)
+		}
+		buffer = append(buffer, uid)
+		last = user
+	}
+	res[last] = buffer
+	return res, nil
+}
+
+func (s *LegacySQL) removeHistoryStar(ctx context.Context, user *user.User, stars []string) error {
+	sql, err := s.db(ctx)
+	if err != nil {
+		return err
+	}
+	req := newStarQueryReq(sql, "", user.OrgID)
+	req.UserID = user.ID
+	if len(stars) > 0 {
+		req.QueryUIDs = stars
+	}
+
+	q, err := sqltemplate.Execute(sqlHistoryStarsDelete, req)
+	if err != nil {
+		return fmt.Errorf("execute template %q: %w", sqlHistoryStarsDelete.Name(), err)
+	}
+
+	sess := sql.DB.GetSqlxSession()
+	_, err = sess.Exec(ctx, q, req.GetArgs()...)
+	return err
+}
+
+func (s *LegacySQL) addHistoryStar(ctx context.Context, user *user.User, star string) error {
+	sql, err := s.db(ctx)
+	if err != nil {
+		return err
+	}
+	req := newStarQueryReq(sql, "", user.OrgID)
+	req.UserID = user.ID
+	req.QueryUID = star
+
+	q, err := sqltemplate.Execute(sqlHistoryStarsDelete, req)
+	if err != nil {
+		return fmt.Errorf("execute template %q: %w", sqlHistoryStarsDelete.Name(), err)
+	}
+
+	sess := sql.DB.GetSqlxSession()
+	_, err = sess.Exec(ctx, q, req.GetArgs()...)
+	return err
+}
+
 // List all defined preferences in an org (valid for admin users only)
 func (s *LegacySQL) listPreferences(ctx context.Context,
 	ns string, orgId int64,

pkg/registry/apis/preferences/legacy/sql_history_stars.sql

@@ -0,0 +1,9 @@
+SELECT s.query_uid, u.uid as user_uid 
+  FROM {{ .Ident .QueryHistoryStarsTable }} as s 
+  JOIN {{ .Ident .QueryHistoryTable }} as h ON s.query_uid = h.uid
+  JOIN {{ .Ident .UserTable }} as u ON s.user_id  = u.id
+ WHERE s.org_id = {{ .Arg .OrgID }} 
+ {{ if .UserUID }}
+   AND u.uid = {{ .Arg .UserUID }}
+{{ end }}
+ ORDER BY s.org_id asc, s.user_id asc, s.query_uid asc 

pkg/registry/apis/preferences/legacy/sql_history_stars_delete.sql

@@ -0,0 +1,6 @@
+DELETE FROM {{ .Ident .QueryHistoryStarsTable }}
+ WHERE org_id = {{ .Arg .OrgID }} 
+   AND user_id = {{ .Arg .UserID }} 
+ {{ if .QueryUIDs }}
+   AND query_uid IN ({{ .ArgList .QueryUIDs }})
+{{ end }}

pkg/registry/apis/preferences/legacy/sql_history_stars_insert.sql

@@ -0,0 +1,4 @@
+INSERT INTO {{ .Ident .QueryHistoryStarsTable }}
+ ( query_uid, user_id, org_id )
+VALUES 
+ ( {{ .Arg .QueryUID }}, {{ .Arg .UserID }}, {{ .Arg .OrgID }} )

pkg/registry/apis/preferences/legacy/stars.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"math/rand"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -12,6 +13,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/internalversion"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/utils/ptr"
 
@@ -107,8 +109,13 @@ func (s *DashboardStarsStorage) List(ctx context.Context, options *internalversi
 	if err != nil {
 		return nil, err
 	}
+	history, err := s.sql.getHistoryStars(ctx, ns.OrgID, "")
+	if err != nil {
+		return nil, err
+	}
 	for _, v := range found {
-		list.Items = append(list.Items, asStarsResource(s.namespacer(v.OrgID), &v))
+		list.Items = append(list.Items,
+			asStarsResource(s.namespacer(v.OrgID), &v, history[v.UserUID]))
 	}
 	if rv > 0 {
 		list.ResourceVersion = strconv.FormatInt(rv, 10)
@@ -141,19 +148,25 @@ func (s *DashboardStarsStorage) Get(ctx context.Context, name string, options *m
 	if err != nil {
 		return nil, err
 	}
+
+	history, err := s.sql.getHistoryStars(ctx, ns.OrgID, owner.Identifier)
+	if err != nil {
+		return nil, err
+	}
+
 	if len(found) == 0 || len(found[0].Dashboards) == 0 {
 		return nil, apiserrors.NewNotFound(preferences.StarsResourceInfo.GroupResource(), name)
 	}
-	obj := asStarsResource(ns.Value, &found[0])
+	obj := asStarsResource(ns.Value, &found[0], history[owner.Identifier])
 	return &obj, nil
 }
 
-func getDashboardStars(stars *preferences.Stars) []string {
+func getStars(stars *preferences.Stars, gk schema.GroupKind) []string {
 	if stars == nil || len(stars.Spec.Resource) == 0 {
 		return []string{}
 	}
 	for _, r := range stars.Spec.Resource {
-		if r.Group == "dashboard.grafana.app" && r.Kind == "Dashboard" {
+		if r.Group == gk.Group && r.Kind == gk.Kind {
 			return r.Names
 		}
 	}
@@ -161,7 +174,7 @@ func getDashboardStars(stars *preferences.Stars) []string {
 }
 
 // Create implements rest.Creater.
-func (s *DashboardStarsStorage) write(ctx context.Context, obj *preferences.Stars, old *preferences.Stars) (runtime.Object, error) {
+func (s *DashboardStarsStorage) write(ctx context.Context, obj *preferences.Stars) (runtime.Object, error) {
 	ns, owner, err := getNamespaceAndOwner(ctx, obj.Name)
 	if err != nil {
 		return nil, err
@@ -177,7 +190,7 @@ func (s *DashboardStarsStorage) write(ctx context.Context, obj *preferences.Star
 		return nil, fmt.Errorf("namespace mismatch")
 	}
 
-	stars := getDashboardStars(obj)
+	stars := getStars(obj, schema.GroupKind{Group: "dashboard.grafana.app", Kind: "Dashboard"})
 	if len(stars) == 0 {
 		err = s.stars.DeleteByUser(ctx, user.ID)
 		return &preferences.Stars{ObjectMeta: metav1.ObjectMeta{
@@ -232,6 +245,31 @@ func (s *DashboardStarsStorage) write(ctx context.Context, obj *preferences.Star
 		changed = true
 	}
 
+	// Apply history stars
+	stars = getStars(obj, schema.GroupKind{Group: "history.grafana.app", Kind: "Query"})
+	res, err := s.sql.getHistoryStars(ctx, user.OrgID, user.UID)
+	if err != nil {
+		return nil, err
+	}
+	history := res[user.UID]
+	if !slices.Equal(stars, history) {
+		changed = true
+		if len(stars) == 0 {
+			err = s.sql.removeHistoryStar(ctx, user, nil)
+			if err != nil {
+				return nil, err
+			}
+		} else {
+			added, removed, _ := preferences.Changes(history, stars)
+			if len(removed) > 0 {
+				_ = s.sql.removeHistoryStar(ctx, user, nil)
+			}
+			for _, v := range added {
+				_ = s.sql.addHistoryStar(ctx, user, v) // one at a time so duplicates do not fail everything
+			}
+		}
+	}
+
 	if changed {
 		return s.Get(ctx, obj.Name, &metav1.GetOptions{})
 	}
@@ -245,7 +283,7 @@ func (s *DashboardStarsStorage) Create(ctx context.Context, obj runtime.Object,
 		return nil, fmt.Errorf("expected stars object")
 	}
 
-	return s.write(ctx, stars, nil)
+	return s.write(ctx, stars)
 }
 
 // Update implements rest.Updater.
@@ -265,13 +303,13 @@ func (s *DashboardStarsStorage) Update(ctx context.Context, name string, objInfo
 		return nil, false, fmt.Errorf("expected stars object")
 	}
 
-	obj, err = s.write(ctx, stars, old.(*preferences.Stars))
+	obj, err = s.write(ctx, stars)
 	return obj, false, err
 }
 
 // Delete implements rest.GracefulDeleter.
 func (s *DashboardStarsStorage) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) {
-	obj, err := s.write(ctx, &preferences.Stars{ObjectMeta: metav1.ObjectMeta{Name: name}}, nil)
+	obj, err := s.write(ctx, &preferences.Stars{ObjectMeta: metav1.ObjectMeta{Name: name}})
 	if err != nil {
 		return nil, false, err
 	}
@@ -283,8 +321,8 @@ func (s *DashboardStarsStorage) DeleteCollection(ctx context.Context, deleteVali
 	return nil, fmt.Errorf("not implemented yet")
 }
 
-func asStarsResource(ns string, v *dashboardStars) preferences.Stars {
-	return preferences.Stars{
+func asStarsResource(ns string, v *dashboardStars, history []string) preferences.Stars {
+	stars := preferences.Stars{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:              fmt.Sprintf("user-%s", v.UserUID),
 			Namespace:         ns,
@@ -299,4 +337,13 @@ func asStarsResource(ns string, v *dashboardStars) preferences.Stars {
 			}},
 		},
 	}
+	if len(history) > 0 {
+		stars.Spec.Resource = append(stars.Spec.Resource, preferences.StarsResource{
+			Group: "history.grafana.app",
+			Kind:  "Query",
+			Names: history,
+		})
+	}
+	stars.Spec.Normalize()
+	return stars
 }

pkg/registry/apis/preferences/legacy/testdata/mysql--sql_history_stars-org.sql

@@ -0,0 +1,6 @@
+SELECT s.query_uid, u.uid as user_uid 
+  FROM `grafana`.`query_history_star` as s 
+  JOIN `grafana`.`query_history` as h ON s.query_uid = h.uid
+  JOIN `grafana`.`user` as u ON s.user_id  = u.id
+ WHERE s.org_id = 1 
+ ORDER BY s.org_id asc, s.user_id asc, s.query_uid asc 

pkg/registry/apis/preferences/legacy/testdata/mysql--sql_history_stars_delete-remove all star.sql

@@ -0,0 +1,3 @@
+DELETE FROM `grafana`.`query_history_star`
+ WHERE org_id = 1 
+   AND user_id = 3 

pkg/registry/apis/preferences/legacy/testdata/mysql--sql_history_stars_insert-add star.sql

@@ -0,0 +1,4 @@
+INSERT INTO `grafana`.`query_history_star`
+ ( query_uid, user_id, org_id )
+VALUES 
+ ( 'XXX', 3, 1 )

pkg/registry/apis/preferences/legacy/testdata/postgres--sql_history_stars-org.sql

@@ -0,0 +1,6 @@
+SELECT s.query_uid, u.uid as user_uid 
+  FROM "grafana"."query_history_star" as s 
+  JOIN "grafana"."query_history" as h ON s.query_uid = h.uid
+  JOIN "grafana"."user" as u ON s.user_id  = u.id
+ WHERE s.org_id = 1 
+ ORDER BY s.org_id asc, s.user_id asc, s.query_uid asc 

pkg/registry/apis/preferences/legacy/testdata/postgres--sql_history_stars_delete-remove all star.sql

@@ -0,0 +1,3 @@
+DELETE FROM "grafana"."query_history_star"
+ WHERE org_id = 1 
+   AND user_id = 3 

pkg/registry/apis/preferences/legacy/testdata/postgres--sql_history_stars_insert-add star.sql

@@ -0,0 +1,4 @@
+INSERT INTO "grafana"."query_history_star"
+ ( query_uid, user_id, org_id )
+VALUES 
+ ( 'XXX', 3, 1 )

pkg/registry/apis/preferences/legacy/testdata/sqlite--sql_history_stars-org.sql

@@ -0,0 +1,6 @@
+SELECT s.query_uid, u.uid as user_uid 
+  FROM "grafana"."query_history_star" as s 
+  JOIN "grafana"."query_history" as h ON s.query_uid = h.uid
+  JOIN "grafana"."user" as u ON s.user_id  = u.id
+ WHERE s.org_id = 1 
+ ORDER BY s.org_id asc, s.user_id asc, s.query_uid asc 

pkg/registry/apis/preferences/legacy/testdata/sqlite--sql_history_stars_delete-remove all star.sql

@@ -0,0 +1,3 @@
+DELETE FROM "grafana"."query_history_star"
+ WHERE org_id = 1 
+   AND user_id = 3 

pkg/registry/apis/preferences/legacy/testdata/sqlite--sql_history_stars_insert-add star.sql

@@ -0,0 +1,4 @@
+INSERT INTO "grafana"."query_history_star"
+ ( query_uid, user_id, org_id )
+VALUES 
+ ( 'XXX', 3, 1 )

pkg/registry/apis/preferences/register.go

@@ -29,7 +29,10 @@ import (
 	"github.com/grafana/grafana/pkg/storage/legacysql"
 )
 
-var _ builder.APIGroupBuilder = (*APIBuilder)(nil)
+var (
+	_ builder.APIGroupBuilder  = (*APIBuilder)(nil)
+	_ builder.APIGroupMutation = (*APIBuilder)(nil)
+)
 
 type APIBuilder struct {
 	authorizer  authorizer.Authorizer
@@ -112,7 +115,7 @@ func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupI
 	if err != nil {
 		return err
 	}
-	stars = &starStorage{store: stars} // wrap List so we only return one value
+	stars = &starStorage{Storage: stars} // wrap List so we only return one value
 	if b.legacyStars != nil && opts.DualWriteBuilder != nil {
 		stars, err = opts.DualWriteBuilder(resource.GroupResource(), b.legacyStars, stars)
 		if err != nil {

pkg/registry/apis/preferences/stars.go

@@ -6,7 +6,6 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/internalversion"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apiserver/pkg/registry/rest"
 
 	authlib "github.com/grafana/authlib/types"
 	preferences "github.com/grafana/grafana/apps/preferences/pkg/apis/preferences/v1alpha1"
@@ -17,7 +16,7 @@ import (
 var _ grafanarest.Storage = (*starStorage)(nil)
 
 type starStorage struct {
-	store grafanarest.Storage
+	grafanarest.Storage
 }
 
 // When using list, we really just want to get the value for the single user
@@ -34,7 +33,7 @@ func (s *starStorage) List(ctx context.Context, options *internalversion.ListOpt
 	// Get the single user stars
 	case authlib.TypeUser:
 		stars := &preferences.StarsList{}
-		obj, _ := s.store.Get(ctx, "user-"+user.GetIdentifier(), &v1.GetOptions{})
+		obj, _ := s.Get(ctx, "user-"+user.GetIdentifier(), &v1.GetOptions{})
 		if obj != nil {
 			s, ok := obj.(*preferences.Stars)
 			if ok {
@@ -44,61 +43,6 @@ func (s *starStorage) List(ctx context.Context, options *internalversion.ListOpt
 		return stars, nil
 
 	default:
-		return s.store.List(ctx, options)
+		return s.Storage.List(ctx, options)
 	}
 }
-
-// ConvertToTable implements rest.Storage.
-func (s *starStorage) ConvertToTable(ctx context.Context, obj runtime.Object, tableOptions runtime.Object) (*v1.Table, error) {
-	return s.store.ConvertToTable(ctx, obj, tableOptions)
-}
-
-// Create implements rest.Storage.
-func (s *starStorage) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *v1.CreateOptions) (runtime.Object, error) {
-	return s.store.Create(ctx, obj, createValidation, options)
-}
-
-// Delete implements rest.Storage.
-func (s *starStorage) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *v1.DeleteOptions) (runtime.Object, bool, error) {
-	return s.store.Delete(ctx, name, deleteValidation, options)
-}
-
-// DeleteCollection implements rest.Storage.
-func (s *starStorage) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *v1.DeleteOptions, listOptions *internalversion.ListOptions) (runtime.Object, error) {
-	return s.store.DeleteCollection(ctx, deleteValidation, options, listOptions)
-}
-
-// Destroy implements rest.Storage.
-func (s *starStorage) Destroy() {
-	s.store.Destroy()
-}
-
-// Get implements rest.Storage.
-func (s *starStorage) Get(ctx context.Context, name string, options *v1.GetOptions) (runtime.Object, error) {
-	return s.store.Get(ctx, name, options)
-}
-
-// GetSingularName implements rest.Storage.
-func (s *starStorage) GetSingularName() string {
-	return s.store.GetSingularName()
-}
-
-// NamespaceScoped implements rest.Storage.
-func (s *starStorage) NamespaceScoped() bool {
-	return s.store.NamespaceScoped()
-}
-
-// New implements rest.Storage.
-func (s *starStorage) New() runtime.Object {
-	return s.store.New()
-}
-
-// NewList implements rest.Storage.
-func (s *starStorage) NewList() runtime.Object {
-	return s.store.NewList()
-}
-
-// Update implements rest.Storage.
-func (s *starStorage) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *v1.UpdateOptions) (runtime.Object, bool, error) {
-	return s.store.Update(ctx, name, objInfo, createValidation, updateValidation, forceAllowCreate, options)
-}

pkg/registry/apis/preferences/stars_update.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"net/http"
-	"slices"
 	"strings"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -110,11 +109,10 @@ func (r *starsREST) Connect(ctx context.Context, name string, _ runtime.Object,
 			return
 		}
 
-		if !apply(&obj.Spec, item, remove) {
-			responder.Object(http.StatusNoContent, &v1.Status{
-				Code: http.StatusNoContent,
-			})
-			return
+		if remove {
+			obj.Spec.Remove(item.group, item.kind, item.id)
+		} else {
+			obj.Spec.Add(item.group, item.kind, item.id)
 		}
 
 		if len(obj.Spec.Resource) == 0 {
@@ -128,9 +126,7 @@ func (r *starsREST) Connect(ctx context.Context, name string, _ runtime.Object,
 			responder.Error(err)
 			return
 		}
-		responder.Object(http.StatusOK, &v1.Status{
-			Code: http.StatusOK,
-		})
+		responder.Object(http.StatusOK, &v1.Status{Code: http.StatusOK})
 	}), nil
 }
 
@@ -151,49 +147,3 @@ func itemFromPath(urlPath, prefix string) (starItem, error) {
 		id:    parts[2],
 	}, nil
 }
-
-func apply(spec *preferences.StarsSpec, item starItem, remove bool) bool {
-	var stars *preferences.StarsResource
-	for idx, v := range spec.Resource {
-		if v.Group == item.group && v.Kind == item.kind {
-			stars = &spec.Resource[idx]
-		}
-	}
-	if stars == nil {
-		if remove {
-			return false
-		}
-		spec.Resource = append(spec.Resource, preferences.StarsResource{
-			Group: item.group,
-			Kind:  item.kind,
-			Names: []string{},
-		})
-		stars = &spec.Resource[len(spec.Resource)-1]
-	}
-
-	idx := slices.Index(stars.Names, item.id)
-	if idx < 0 { // not found
-		if remove {
-			return false
-		}
-		stars.Names = append(stars.Names, item.id)
-	} else if remove {
-		stars.Names = append(stars.Names[:idx], stars.Names[idx+1:]...)
-	} else {
-		return false
-	}
-	slices.Sort(stars.Names)
-
-	// Remove the slot if only one value
-	if len(stars.Names) == 0 {
-		tmp := preferences.StarsSpec{}
-		for _, v := range spec.Resource {
-			if v.Group == item.group && v.Kind == item.kind {
-				continue
-			}
-			tmp.Resource = append(tmp.Resource, v)
-		}
-		spec.Resource = tmp.Resource
-	}
-	return true
-}

pkg/registry/apis/preferences/stars_update_test.go

@@ -4,194 +4,9 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-
-	preferences "github.com/grafana/grafana/apps/preferences/pkg/apis/preferences/v1alpha1"
 )
 
 func TestStarsWrite(t *testing.T) {
-	t.Run("apply", func(t *testing.T) {
-		tests := []struct {
-			name    string
-			spec    preferences.StarsSpec
-			item    starItem
-			remove  bool
-			changed bool
-			expect  preferences.StarsSpec
-		}{{
-			name: "add to an existing array",
-			spec: preferences.StarsSpec{
-				Resource: []preferences.StarsResource{{
-					Group: "g",
-					Kind:  "k",
-					Names: []string{"a", "b", "c"},
-				}},
-			},
-			item: starItem{
-				group: "g",
-				kind:  "k",
-				id:    "x",
-			},
-			remove:  false,
-			changed: true,
-			expect: preferences.StarsSpec{
-				Resource: []preferences.StarsResource{{
-					Group: "g",
-					Kind:  "k",
-					Names: []string{"a", "b", "c", "x"}, // added "x"
-				}},
-			},
-		}, {
-			name: "remove from an existing array",
-			spec: preferences.StarsSpec{
-				Resource: []preferences.StarsResource{{
-					Group: "g",
-					Kind:  "k",
-					Names: []string{"a", "b", "c"},
-				}},
-			},
-			item: starItem{
-				group: "g",
-				kind:  "k",
-				id:    "b",
-			},
-			remove:  true,
-			changed: true,
-			expect: preferences.StarsSpec{
-				Resource: []preferences.StarsResource{{
-					Group: "g",
-					Kind:  "k",
-					Names: []string{"a", "c"}, // removed "b"
-				}},
-			},
-		}, {
-			name: "add to empty spec",
-			spec: preferences.StarsSpec{},
-			item: starItem{
-				group: "g",
-				kind:  "k",
-				id:    "a",
-			},
-			remove:  false,
-			changed: true,
-			expect: preferences.StarsSpec{
-				Resource: []preferences.StarsResource{{
-					Group: "g",
-					Kind:  "k",
-					Names: []string{"a"},
-				}},
-			},
-		}, {
-			name: "remove item that does not exist",
-			spec: preferences.StarsSpec{
-				Resource: []preferences.StarsResource{{
-					Group: "g",
-					Kind:  "k",
-					Names: []string{"x"},
-				}},
-			},
-			item: starItem{
-				group: "g",
-				kind:  "k",
-				id:    "a",
-			},
-			remove:  true,
-			changed: false,
-		}, {
-			name: "add item that already exist",
-			spec: preferences.StarsSpec{
-				Resource: []preferences.StarsResource{{
-					Group: "g",
-					Kind:  "k",
-					Names: []string{"x"},
-				}},
-			},
-			item: starItem{
-				group: "g",
-				kind:  "k",
-				id:    "x",
-			},
-			remove:  false,
-			changed: false,
-		}, {
-			name: "remove from empty",
-			spec: preferences.StarsSpec{},
-			item: starItem{
-				group: "g",
-				kind:  "k",
-				id:    "a",
-			},
-			remove:  true,
-			changed: false,
-		}, {
-			name: "remove item that does not exist",
-			spec: preferences.StarsSpec{
-				Resource: []preferences.StarsResource{{
-					Group: "g",
-					Kind:  "k",
-					Names: []string{"a", "b", "c"},
-				}},
-			},
-			item: starItem{
-				group: "g",
-				kind:  "k",
-				id:    "X",
-			},
-			remove:  true,
-			changed: false,
-		}, {
-			name: "remove last item",
-			spec: preferences.StarsSpec{
-				Resource: []preferences.StarsResource{{
-					Group: "g",
-					Kind:  "k",
-					Names: []string{"a"},
-				}},
-			},
-			item: starItem{
-				group: "g",
-				kind:  "k",
-				id:    "a",
-			},
-			remove:  true,
-			changed: true,
-			expect:  preferences.StarsSpec{},
-		}, {
-			name: "remove last item (with others)",
-			spec: preferences.StarsSpec{
-				Resource: []preferences.StarsResource{{
-					Group: "g",
-					Kind:  "k",
-					Names: []string{"a"},
-				}, {
-					Group: "g2",
-					Kind:  "k2",
-					Names: []string{"a"},
-				}}},
-			item: starItem{
-				group: "g",
-				kind:  "k",
-				id:    "a",
-			},
-			remove:  true,
-			changed: true,
-			expect: preferences.StarsSpec{
-				Resource: []preferences.StarsResource{{
-					Group: "g2",
-					Kind:  "k2",
-					Names: []string{"a"},
-				}}},
-		}}
-		for _, tt := range tests {
-			t.Run(tt.name, func(t *testing.T) {
-				changed := apply(&tt.spec, tt.item, tt.remove)
-				require.Equal(t, tt.changed, changed)
-				if changed {
-					require.Equal(t, tt.expect, tt.spec)
-				}
-			})
-		}
-	})
-
 	t.Run("path", func(t *testing.T) {
 		tests := []struct {
 			name   string