fix(repoinfo): fix userprefs values for repos returned by expanded repo info (#1413)

- now isBookmarked and isStarred are updated correctly

Signed-off-by: Laurentiu Niculae <niculae.laurentiu1@gmail.com>

pkg/extensions/search/resolver.go

@@ -1096,7 +1096,7 @@ func expandedRepoInfo(ctx context.Context, repo string, repoDB repodb.RepoDB, cv
 		return &gql_generated.RepoInfo{}, nil //nolint:nilerr // don't give details to a potential attacker
 	}
 
-	repoMeta, err := repoDB.GetRepoMeta(repo)
+	repoMeta, err := repoDB.GetUserRepoMeta(ctx, repo)
 	if err != nil {
 		log.Error().Err(err).Str("repository", repo).Msg("resolver: can't retrieve repoMeta for repo")
 

pkg/extensions/search/resolver_test.go

@@ -3274,7 +3274,7 @@ func TestExpandedRepoInfo(t *testing.T) {
 			graphql.DefaultRecover)
 
 		repoDB := mocks.RepoDBMock{
-			GetRepoMetaFn: func(repo string) (repodb.RepoMetadata, error) {
+			GetUserRepoMetaFn: func(ctx context.Context, repo string) (repodb.RepoMetadata, error) {
 				return repodb.RepoMetadata{
 					Tags: map[string]repodb.Descriptor{
 						"tagManifest": {

pkg/extensions/search/search_test.go

@@ -4699,7 +4699,7 @@ func TestRepoDBIndexOperations(t *testing.T) {
 
 func RunRepoDBIndexTests(baseURL, port string) {
 	Convey("Push test index", func() {
-		repo := "repo"
+		const repo = "repo"
 
 		multiarchImage, err := GetRandomMultiarchImage("tag1")
 		So(err, ShouldBeNil)

pkg/extensions/search/userprefs_test.go

@@ -806,6 +806,198 @@ func TestGlobalSearchWithUserPrefFiltering(t *testing.T) {
 	})
 }
 
+func TestExpandedRepoInfoWithUserPrefs(t *testing.T) {
+	Convey("ExpandedRepoInfo with User Prefs", t, func() {
+		dir := t.TempDir()
+		port := GetFreePort()
+		baseURL := GetBaseURL(port)
+		conf := config.New()
+		conf.HTTP.Port = port
+		conf.Storage.RootDirectory = dir
+
+		simpleUser := "simpleUser"
+		simpleUserPassword := "simpleUserPass"
+		credTests := fmt.Sprintf("%s\n\n", getCredString(simpleUser, simpleUserPassword))
+
+		htpasswdPath := MakeHtpasswdFileFromString(credTests)
+		defer os.Remove(htpasswdPath)
+
+		conf.HTTP.Auth = &config.AuthConfig{
+			HTPasswd: config.AuthHTPasswd{
+				Path: htpasswdPath,
+			},
+		}
+
+		conf.HTTP.AccessControl = &config.AccessControlConfig{
+			Repositories: config.Repositories{
+				"**": config.PolicyGroup{
+					Policies: []config.Policy{
+						{
+							Users:   []string{simpleUser},
+							Actions: []string{"read", "create"},
+						},
+					},
+				},
+			},
+		}
+
+		defaultVal := true
+		conf.Extensions = &extconf.ExtensionConfig{
+			Search: &extconf.SearchConfig{BaseConfig: extconf.BaseConfig{Enable: &defaultVal}},
+		}
+
+		ctlr := api.NewController(conf)
+
+		ctlrManager := NewControllerManager(ctlr)
+		ctlrManager.StartAndWait(port)
+		defer ctlrManager.StopServer()
+
+		preferencesBaseURL := baseURL + constants.FullUserPreferencesPrefix
+		simpleUserClient := resty.R().SetBasicAuth(simpleUser, simpleUserPassword)
+
+		// ------ Add sbrepo and star/bookmark it
+		sbrepo := "sbrepo"
+		img, err := GetRandomImage("tag")
+		So(err, ShouldBeNil)
+		err = UploadImageWithBasicAuth(img, baseURL, sbrepo, simpleUser, simpleUserPassword)
+		So(err, ShouldBeNil)
+
+		resp, err := simpleUserClient.Put(preferencesBaseURL + PutRepoStarURL(sbrepo))
+		So(resp.StatusCode(), ShouldEqual, http.StatusOK)
+		So(err, ShouldBeNil)
+
+		resp, err = simpleUserClient.Put(preferencesBaseURL + PutRepoBookmarkURL(sbrepo))
+		So(resp.StatusCode(), ShouldEqual, http.StatusOK)
+		So(err, ShouldBeNil)
+
+		// ExpandedRepoinfo
+
+		query := `
+		{
+			ExpandedRepoInfo(repo:"sbrepo"){
+				Summary {
+					Name IsStarred IsBookmarked 
+				}
+			}
+		}`
+
+		resp, err = simpleUserClient.Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query))
+		So(resp, ShouldNotBeNil)
+		So(err, ShouldBeNil)
+		So(resp.StatusCode(), ShouldEqual, 200)
+
+		responseStruct := ExpandedRepoInfoResp{}
+
+		err = json.Unmarshal(resp.Body(), &responseStruct)
+		So(err, ShouldBeNil)
+
+		repoInfo := responseStruct.ExpandedRepoInfo.RepoInfo
+		So(repoInfo.Summary.IsBookmarked, ShouldBeTrue)
+		So(repoInfo.Summary.IsStarred, ShouldBeTrue)
+
+		// ------ Add srepo and star it
+		srepo := "srepo"
+		img, err = GetRandomImage("tag")
+		So(err, ShouldBeNil)
+		err = UploadImageWithBasicAuth(img, baseURL, srepo, simpleUser, simpleUserPassword)
+		So(err, ShouldBeNil)
+
+		resp, err = simpleUserClient.Put(preferencesBaseURL + PutRepoStarURL(srepo))
+		So(resp.StatusCode(), ShouldEqual, http.StatusOK)
+		So(err, ShouldBeNil)
+
+		// ExpandedRepoinfo
+		query = `
+		{
+			ExpandedRepoInfo(repo:"srepo"){
+				Summary {
+					Name IsStarred IsBookmarked 
+				}
+			}
+		}`
+
+		resp, err = simpleUserClient.Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query))
+		So(resp, ShouldNotBeNil)
+		So(err, ShouldBeNil)
+		So(resp.StatusCode(), ShouldEqual, 200)
+
+		responseStruct = ExpandedRepoInfoResp{}
+
+		err = json.Unmarshal(resp.Body(), &responseStruct)
+		So(err, ShouldBeNil)
+
+		repoInfo = responseStruct.ExpandedRepoInfo.RepoInfo
+		So(repoInfo.Summary.IsBookmarked, ShouldBeFalse)
+		So(repoInfo.Summary.IsStarred, ShouldBeTrue)
+
+		// ------ Add brepo and bookmark it
+		brepo := "brepo"
+		img, err = GetRandomImage("tag")
+		So(err, ShouldBeNil)
+		err = UploadImageWithBasicAuth(img, baseURL, brepo, simpleUser, simpleUserPassword)
+		So(err, ShouldBeNil)
+
+		resp, err = simpleUserClient.Put(preferencesBaseURL + PutRepoBookmarkURL(brepo))
+		So(resp.StatusCode(), ShouldEqual, http.StatusOK)
+		So(err, ShouldBeNil)
+
+		// ExpandedRepoinfo
+		query = `
+		{
+			ExpandedRepoInfo(repo:"brepo"){
+				Summary {
+					Name IsStarred IsBookmarked 
+				}
+			}
+		}`
+
+		resp, err = simpleUserClient.Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query))
+		So(resp, ShouldNotBeNil)
+		So(err, ShouldBeNil)
+		So(resp.StatusCode(), ShouldEqual, 200)
+
+		responseStruct = ExpandedRepoInfoResp{}
+
+		err = json.Unmarshal(resp.Body(), &responseStruct)
+		So(err, ShouldBeNil)
+
+		repoInfo = responseStruct.ExpandedRepoInfo.RepoInfo
+		So(repoInfo.Summary.IsBookmarked, ShouldBeTrue)
+		So(repoInfo.Summary.IsStarred, ShouldBeFalse)
+
+		// ------ Add repo without star/bookmark
+		repo := "repo"
+		img, err = GetRandomImage("tag")
+		So(err, ShouldBeNil)
+		err = UploadImageWithBasicAuth(img, baseURL, repo, simpleUser, simpleUserPassword)
+		So(err, ShouldBeNil)
+
+		// ExpandedRepoinfo
+		query = `
+		{
+			ExpandedRepoInfo(repo:"repo"){
+				Summary {
+					Name IsStarred IsBookmarked 
+				}
+			}
+		}`
+
+		resp, err = simpleUserClient.Get(baseURL + graphqlQueryPrefix + "?query=" + url.QueryEscape(query))
+		So(resp, ShouldNotBeNil)
+		So(err, ShouldBeNil)
+		So(resp.StatusCode(), ShouldEqual, 200)
+
+		responseStruct = ExpandedRepoInfoResp{}
+
+		err = json.Unmarshal(resp.Body(), &responseStruct)
+		So(err, ShouldBeNil)
+
+		repoInfo = responseStruct.ExpandedRepoInfo.RepoInfo
+		So(repoInfo.Summary.IsBookmarked, ShouldBeFalse)
+		So(repoInfo.Summary.IsStarred, ShouldBeFalse)
+	})
+}
+
 func PutRepoStarURL(repo string) string {
 	return fmt.Sprintf("?repo=%s&action=toggleStar", repo)
 }

pkg/meta/repodb/boltdb-wrapper/boltdb_wrapper.go

@@ -533,6 +533,36 @@ func (bdw *DBWrapper) GetRepoMeta(repo string) (repodb.RepoMetadata, error) {
 	return repoMeta, err
 }
 
+func (bdw *DBWrapper) GetUserRepoMeta(ctx context.Context, repo string) (repodb.RepoMetadata, error) {
+	var repoMeta repodb.RepoMetadata
+
+	err := bdw.DB.Update(func(tx *bbolt.Tx) error {
+		buck := tx.Bucket([]byte(bolt.RepoMetadataBucket))
+		userBookmarks := getUserBookmarks(ctx, tx)
+		userStars := getUserStars(ctx, tx)
+
+		repoMetaBlob := buck.Get([]byte(repo))
+
+		// object not found
+		if repoMetaBlob == nil {
+			return zerr.ErrRepoMetaNotFound
+		}
+
+		// object found
+		err := json.Unmarshal(repoMetaBlob, &repoMeta)
+		if err != nil {
+			return err
+		}
+
+		repoMeta.IsBookmarked = zcommon.Contains(userBookmarks, repo)
+		repoMeta.IsStarred = zcommon.Contains(userStars, repo)
+
+		return nil
+	})
+
+	return repoMeta, err
+}
+
 func (bdw *DBWrapper) SetRepoMeta(repo string, repoMeta repodb.RepoMetadata) error {
 	err := bdw.DB.Update(func(tx *bbolt.Tx) error {
 		buck := tx.Bucket([]byte(bolt.RepoMetadataBucket))

pkg/meta/repodb/boltdb-wrapper/boltdb_wrapper_test.go

@@ -922,6 +922,30 @@ func TestWrapperErrors(t *testing.T) {
 			)
 			So(err, ShouldBeNil)
 		})
+
+		Convey("GetUserRepoMeta unmarshal error", func() {
+			acCtx := localCtx.AccessControlContext{
+				ReadGlobPatterns: map[string]bool{
+					"repo": true,
+				},
+				Username: "username",
+			}
+			authzCtxKey := localCtx.GetContextKey()
+			ctx := context.WithValue(context.Background(), authzCtxKey, acCtx)
+
+			err = boltdbWrapper.DB.Update(func(tx *bbolt.Tx) error {
+				repoBuck := tx.Bucket([]byte(bolt.RepoMetadataBucket))
+
+				err := repoBuck.Put([]byte("repo"), []byte("bad repo"))
+				So(err, ShouldBeNil)
+
+				return nil
+			})
+			So(err, ShouldBeNil)
+
+			_, err := boltdbWrapper.GetUserRepoMeta(ctx, "repo")
+			So(err, ShouldNotBeNil)
+		})
 	})
 }
 

pkg/meta/repodb/dynamodb-wrapper/dynamo_test.go

@@ -1037,6 +1037,55 @@ func TestWrapperErrors(t *testing.T) {
 			err := dynamoWrapper.PatchDB()
 			So(err, ShouldNotBeNil)
 		})
+
+		Convey("GetUserRepoMeta client.GetItem error", func() {
+			dynamoWrapper.RepoMetaTablename = badTablename
+
+			_, err = dynamoWrapper.GetUserRepoMeta(ctx, "repo")
+			So(err, ShouldNotBeNil)
+		})
+
+		Convey("GetUserRepoMeta repoMeta not found", func() {
+			_, err = dynamoWrapper.GetUserRepoMeta(ctx, "unknown-repo-meta")
+			So(err, ShouldNotBeNil)
+		})
+
+		Convey("GetUserRepoMeta userMeta not found", func() {
+			err := dynamoWrapper.SetRepoReference("repo", "tag", digest.FromString("1"), ispec.MediaTypeImageManifest)
+			So(err, ShouldBeNil)
+			dynamoWrapper.UserDataTablename = badTablename
+
+			acCtx := localCtx.AccessControlContext{
+				ReadGlobPatterns: map[string]bool{
+					"repo": true,
+				},
+				Username: "username",
+			}
+
+			authzCtxKey := localCtx.GetContextKey()
+			ctx := context.WithValue(context.Background(), authzCtxKey, acCtx)
+
+			_, err = dynamoWrapper.GetUserRepoMeta(ctx, "repo")
+			So(err, ShouldNotBeNil)
+		})
+
+		Convey("GetUserRepoMeta unmarshal error", func() {
+			err := setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo")
+			So(err, ShouldBeNil)
+
+			acCtx := localCtx.AccessControlContext{
+				ReadGlobPatterns: map[string]bool{
+					"repo": true,
+				},
+				Username: "username",
+			}
+
+			authzCtxKey := localCtx.GetContextKey()
+			ctx := context.WithValue(context.Background(), authzCtxKey, acCtx)
+
+			_, err = dynamoWrapper.GetUserRepoMeta(ctx, "repo")
+			So(err, ShouldNotBeNil)
+		})
 	})
 
 	Convey("NewDynamoDBWrapper errors", t, func() {

pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go

@@ -616,6 +616,39 @@ func (dwr *DBWrapper) GetRepoMeta(repo string) (repodb.RepoMetadata, error) {
 	return repoMeta, nil
 }
 
+func (dwr *DBWrapper) GetUserRepoMeta(ctx context.Context, repo string) (repodb.RepoMetadata, error) {
+	resp, err := dwr.Client.GetItem(ctx, &dynamodb.GetItemInput{
+		TableName: aws.String(dwr.RepoMetaTablename),
+		Key: map[string]types.AttributeValue{
+			"RepoName": &types.AttributeValueMemberS{Value: repo},
+		},
+	})
+	if err != nil {
+		return repodb.RepoMetadata{}, err
+	}
+
+	if resp.Item == nil {
+		return repodb.RepoMetadata{}, zerr.ErrRepoMetaNotFound
+	}
+
+	var repoMeta repodb.RepoMetadata
+
+	err = attributevalue.Unmarshal(resp.Item["RepoMetadata"], &repoMeta)
+	if err != nil {
+		return repodb.RepoMetadata{}, err
+	}
+
+	userMeta, err := dwr.GetUserMeta(ctx)
+	if err != nil {
+		return repodb.RepoMetadata{}, err
+	}
+
+	repoMeta.IsBookmarked = zcommon.Contains(userMeta.BookmarkedRepos, repo)
+	repoMeta.IsStarred = zcommon.Contains(userMeta.StarredRepos, repo)
+
+	return repoMeta, nil
+}
+
 func (dwr *DBWrapper) IncrementImageDownloads(repo string, reference string) error {
 	repoMeta, err := dwr.GetRepoMeta(repo)
 	if err != nil {
@@ -1926,7 +1959,7 @@ func (dwr *DBWrapper) GetStarredRepos(ctx context.Context) ([]string, error) {
 	return userMeta.StarredRepos, err
 }
 
-func (dwr DBWrapper) GetUserMeta(ctx context.Context) (repodb.UserData, error) {
+func (dwr *DBWrapper) GetUserMeta(ctx context.Context) (repodb.UserData, error) {
 	acCtx, err := localCtx.GetAccessControlContext(ctx)
 	if err != nil {
 		return repodb.UserData{}, err
@@ -1963,7 +1996,7 @@ func (dwr DBWrapper) GetUserMeta(ctx context.Context) (repodb.UserData, error) {
 	return userMeta, nil
 }
 
-func (dwr DBWrapper) createUserDataTable() error {
+func (dwr *DBWrapper) createUserDataTable() error {
 	_, err := dwr.Client.CreateTable(context.Background(), &dynamodb.CreateTableInput{
 		TableName: aws.String(dwr.UserDataTablename),
 		AttributeDefinitions: []types.AttributeDefinition{
@@ -1988,7 +2021,7 @@ func (dwr DBWrapper) createUserDataTable() error {
 	return dwr.waitTableToBeCreated(dwr.UserDataTablename)
 }
 
-func (dwr DBWrapper) SetUserMeta(ctx context.Context, userMeta repodb.UserData) error {
+func (dwr *DBWrapper) SetUserMeta(ctx context.Context, userMeta repodb.UserData) error {
 	acCtx, err := localCtx.GetAccessControlContext(ctx)
 	if err != nil {
 		return err

pkg/meta/repodb/repodb.go

@@ -47,6 +47,10 @@ type RepoDB interface { //nolint:interfacebloat
 	// GetRepoMeta returns RepoMetadata of a repo from the database
 	GetRepoMeta(repo string) (RepoMetadata, error)
 
+	// GetUserRepometa return RepoMetadata of a repo from the database along side specific information about the
+	// user
+	GetUserRepoMeta(ctx context.Context, repo string) (RepoMetadata, error)
+
 	// GetRepoMeta returns RepoMetadata of a repo from the database
 	SetRepoMeta(repo string, repoMeta RepoMetadata) error
 

pkg/meta/repodb/repodb_test.go

@@ -2511,6 +2511,35 @@ func RunRepoDBTests(repoDB repodb.RepoDB, preparationFuncs ...func() error) {
 			So(repoMetas[0].IsBookmarked, ShouldBeTrue)
 			So(repoMetas[0].IsStarred, ShouldBeTrue)
 		})
+
+		Convey("Test GetUserRepoMeta", func() {
+			authzCtxKey := localCtx.GetContextKey()
+
+			acCtx := localCtx.AccessControlContext{
+				ReadGlobPatterns: map[string]bool{
+					"repo": true,
+				},
+				Username: "user1",
+			}
+			ctx := context.WithValue(context.Background(), authzCtxKey, acCtx)
+
+			digest := godigest.FromString("1")
+
+			err := repoDB.SetRepoReference("repo", "tag", digest, ispec.MediaTypeImageManifest)
+			So(err, ShouldBeNil)
+
+			_, err = repoDB.ToggleBookmarkRepo(ctx, "repo")
+			So(err, ShouldBeNil)
+
+			_, err = repoDB.ToggleStarRepo(ctx, "repo")
+			So(err, ShouldBeNil)
+
+			repoMeta, err := repoDB.GetUserRepoMeta(ctx, "repo")
+			So(err, ShouldBeNil)
+			So(repoMeta.IsBookmarked, ShouldBeTrue)
+			So(repoMeta.IsStarred, ShouldBeTrue)
+			So(repoMeta.Tags, ShouldContainKey, "tag")
+		})
 	})
 }
 

pkg/test/mocks/repo_db_mock.go

@@ -25,6 +25,8 @@ type RepoDBMock struct {
 
 	GetRepoMetaFn func(repo string) (repodb.RepoMetadata, error)
 
+	GetUserRepoMetaFn func(ctx context.Context, repo string) (repodb.RepoMetadata, error)
+
 	SetRepoMetaFn func(repo string, repoMeta repodb.RepoMetadata) error
 
 	GetMultipleRepoMetaFn func(ctx context.Context, filter func(repoMeta repodb.RepoMetadata) bool,
@@ -155,6 +157,14 @@ func (sdm RepoDBMock) GetRepoMeta(repo string) (repodb.RepoMetadata, error) {
 	return repodb.RepoMetadata{}, nil
 }
 
+func (sdm RepoDBMock) GetUserRepoMeta(ctx context.Context, repo string) (repodb.RepoMetadata, error) {
+	if sdm.GetUserRepoMetaFn != nil {
+		return sdm.GetUserRepoMetaFn(ctx, repo)
+	}
+
+	return repodb.RepoMetadata{}, nil
+}
+
 func (sdm RepoDBMock) SetRepoMeta(repo string, repoMeta repodb.RepoMetadata) error {
 	if sdm.SetRepoMetaFn != nil {
 		return sdm.SetRepoMetaFn(repo, repoMeta)