From 3c7a09a8c46131f7b4e7dd68f989dfdc45f1709e Mon Sep 17 00:00:00 2001 From: Dmitry Lazurkin Date: Fri, 6 Jan 2017 21:53:29 +0300 Subject: [PATCH] Fix filtering top repositories (#783, #1281) --- src/common/dao/dao_test.go | 7 -- src/common/dao/repository.go | 16 ++- src/common/dao/repository_test.go | 158 ++++++++++++++++++++++++++++++ src/ui/api/repository.go | 7 +- 4 files changed, 177 insertions(+), 11 deletions(-) diff --git a/src/common/dao/dao_test.go b/src/common/dao/dao_test.go index 861bf3ee7..116ccbd72 100644 --- a/src/common/dao/dao_test.go +++ b/src/common/dao/dao_test.go @@ -980,13 +980,6 @@ func TestGetRecentLogs(t *testing.T) { } } -func TestGetTopRepos(t *testing.T) { - _, err := GetTopRepos(10) - if err != nil { - t.Fatalf("error occured in getting top repos, error: %v", err) - } -} - var targetID, policyID, policyID2, policyID3, jobID, jobID2, jobID3 int64 func TestAddRepTarget(t *testing.T) { diff --git a/src/common/dao/repository.go b/src/common/dao/repository.go index cda2f5cba..5d9353a0c 100644 --- a/src/common/dao/repository.go +++ b/src/common/dao/repository.go @@ -102,12 +102,22 @@ func GetRepositoryByProjectName(name string) ([]*models.RepoRecord, error) { } //GetTopRepos returns the most popular repositories -func GetTopRepos(count int) ([]models.TopRepo, error) { +func GetTopRepos(userID int, count int) ([]models.TopRepo, error) { topRepos := []models.TopRepo{} + sql := `select r.name, r.pull_count from repository r + inner join project p on r.project_id = p.project_id + where ( + p.deleted = 0 and ( + p.public = 1 or ( + ? <> ? and exists ( + select 1 from project_member pm + where pm.project_id = p.project_id and pm.user_id = ? + )))) + order by r.pull_count desc, r.name limit ?` repositories := []*models.RepoRecord{} - if _, err := GetOrmer().QueryTable(&models.RepoRecord{}). - OrderBy("-PullCount", "Name").Limit(count).All(&repositories); err != nil { + _, err := GetOrmer().Raw(sql, userID, NonExistUserID, userID, count).QueryRows(&repositories) + if err != nil { return topRepos, err } diff --git a/src/common/dao/repository_test.go b/src/common/dao/repository_test.go index 1cc251f60..e8efc7e6b 100644 --- a/src/common/dao/repository_test.go +++ b/src/common/dao/repository_test.go @@ -16,8 +16,11 @@ package dao import ( + "fmt" "testing" + "time" + "github.com/stretchr/testify/require" "github.com/vmware/harbor/src/common/models" ) @@ -160,6 +163,161 @@ func TestGetTotalOfUserRelevantRepositories(t *testing.T) { } } +func TestGetTopRepos(t *testing.T) { + var err error + require := require.New(t) + + require.NoError(GetOrmer().Begin()) + defer func() { + require.NoError(GetOrmer().Rollback()) + }() + + admin, err := GetUser(models.User{Username: "admin"}) + require.NoError(err) + + user := models.User{ + Username: "user", + Password: "user", + Email: "user@test.com", + } + userID, err := Register(user) + require.NoError(err) + user.UserID = int(userID) + + // + // public project with 1 repository + // non-public project with 2 repositories visible by admin + // non-public project with 1 repository visible by admin and user + // deleted public project with 1 repository + // + + project1 := models.Project{ + OwnerID: admin.UserID, + Name: "project1", + CreationTime: time.Now(), + OwnerName: admin.Username, + Public: 0, + } + project1.ProjectID, err = AddProject(project1) + require.NoError(err) + + project2 := models.Project{ + OwnerID: admin.UserID, + Name: "project2", + CreationTime: time.Now(), + OwnerName: admin.Username, + Public: 0, + } + project2.ProjectID, err = AddProject(project2) + require.NoError(err) + + require.NoError(AddProjectMember(project2.ProjectID, user.UserID, models.PROJECTADMIN)) + + err = AddRepository(*repository) + require.NoError(err) + + repository1 := &models.RepoRecord{ + Name: fmt.Sprintf("%v/repository1", project1.Name), + OwnerName: admin.Username, + ProjectName: project1.Name, + } + err = AddRepository(*repository1) + require.NoError(err) + require.NoError(IncreasePullCount(repository1.Name)) + repository1, err = GetRepositoryByName(repository1.Name) + require.NoError(err) + + repository2 := &models.RepoRecord{ + Name: fmt.Sprintf("%v/repository2", project1.Name), + OwnerName: admin.Username, + ProjectName: project1.Name, + } + err = AddRepository(*repository2) + require.NoError(err) + require.NoError(IncreasePullCount(repository2.Name)) + require.NoError(IncreasePullCount(repository2.Name)) + repository2, err = GetRepositoryByName(repository2.Name) + require.NoError(err) + + repository3 := &models.RepoRecord{ + Name: fmt.Sprintf("%v/repository3", project2.Name), + OwnerName: admin.Username, + ProjectName: project2.Name, + } + err = AddRepository(*repository3) + require.NoError(err) + require.NoError(IncreasePullCount(repository3.Name)) + require.NoError(IncreasePullCount(repository3.Name)) + require.NoError(IncreasePullCount(repository3.Name)) + repository3, err = GetRepositoryByName(repository3.Name) + require.NoError(err) + + deletedPublicProject := models.Project{ + OwnerID: admin.UserID, + Name: "public-deleted", + CreationTime: time.Now(), + OwnerName: admin.Username, + Public: 1, + } + deletedPublicProject.ProjectID, err = AddProject(deletedPublicProject) + require.NoError(err) + deletedPublicRepository1 := &models.RepoRecord{ + Name: fmt.Sprintf("%v/repository1", deletedPublicProject.Name), + OwnerName: admin.Username, + ProjectName: deletedPublicProject.Name, + } + err = AddRepository(*deletedPublicRepository1) + require.NoError(err) + DeleteProject(deletedPublicProject.ProjectID) + + var topRepos []models.TopRepo + + // anonymous should retrieve public non-deleted repositories + topRepos, err = GetTopRepos(NonExistUserID, 3) + require.NoError(err) + require.Len(topRepos, 1) + require.Equal(topRepos, []models.TopRepo{ + models.TopRepo{ + RepoName: repository.Name, + AccessCount: repository.PullCount, + }, + }) + + // admin should retrieve all visible repositories limited by count + topRepos, err = GetTopRepos(admin.UserID, 3) + require.NoError(err) + require.Len(topRepos, 3) + require.Equal(topRepos, []models.TopRepo{ + models.TopRepo{ + RepoName: repository3.Name, + AccessCount: repository3.PullCount, + }, + models.TopRepo{ + RepoName: repository2.Name, + AccessCount: repository2.PullCount, + }, + models.TopRepo{ + RepoName: repository1.Name, + AccessCount: repository1.PullCount, + }, + }) + + // user should retrieve all visible repositories + topRepos, err = GetTopRepos(user.UserID, 3) + require.NoError(err) + require.Len(topRepos, 2) + require.Equal(topRepos, []models.TopRepo{ + models.TopRepo{ + RepoName: repository3.Name, + AccessCount: repository3.PullCount, + }, + models.TopRepo{ + RepoName: repository.Name, + AccessCount: repository.PullCount, + }, + }) +} + func addRepository(repository *models.RepoRecord) error { return AddRepository(*repository) } diff --git a/src/ui/api/repository.go b/src/ui/api/repository.go index 4db0d1eb1..ac5706b2e 100644 --- a/src/ui/api/repository.go +++ b/src/ui/api/repository.go @@ -416,7 +416,12 @@ func (ra *RepositoryAPI) GetTopRepos() { ra.CustomAbort(http.StatusBadRequest, "invalid count") } - repos, err := dao.GetTopRepos(count) + userID, _, ok := ra.GetUserIDForRequest() + if !ok { + userID = dao.NonExistUserID + } + + repos, err := dao.GetTopRepos(userID, count) if err != nil { log.Errorf("failed to get top repos: %v", err) ra.CustomAbort(http.StatusInternalServerError, "internal server error")