Merge pull request #2367 from ywk253100/170524_search

Refactor search API

src/common/dao/dao_test.go

@@ -504,7 +504,7 @@ func TestChangeUserPasswordWithIncorrectOldPassword(t *testing.T) {
 }
 
 func TestQueryRelevantProjectsWhenNoProjectAdded(t *testing.T) {
-	projects, err := SearchProjects(currentUser.UserID)
+	projects, err := GetHasReadPermProjects(currentUser.Username)
 	if err != nil {
 		t.Errorf("Error occurred in QueryRelevantProjects: %v", err)
 	}

src/common/dao/project.go

@@ -156,11 +156,18 @@ func ToggleProjectPublicity(projectID int64, publicity int) error {
 	return err
 }
 
-// SearchProjects returns a project list,
+// GetHasReadPermProjects returns a project list,
 // which satisfies the following conditions:
 // 1. the project is not deleted
 // 2. the prject is public or the user is a member of the project
-func SearchProjects(userID int) ([]*models.Project, error) {
+func GetHasReadPermProjects(username string) ([]*models.Project, error) {
+	user, err := GetUser(models.User{
+		Username: username,
+	})
+	if err != nil {
+		return nil, err
+	}
+
 	o := GetOrmer()
 
 	sql :=
@@ -174,7 +181,7 @@ func SearchProjects(userID int) ([]*models.Project, error) {
 
 	var projects []*models.Project
 
-	if _, err := o.Raw(sql, userID).QueryRows(&projects); err != nil {
+	if _, err := o.Raw(sql, user.UserID).QueryRows(&projects); err != nil {
 		return nil, err
 	}
 

src/common/security/rbac/context_test.go

@@ -110,6 +110,11 @@ func (f *fakePM) GetAll(*models.QueryParam) ([]*models.Project, error) {
 	return []*models.Project{}, nil
 }
 
+// nil implement
+func (f *fakePM) GetHasReadPerm(username ...string) ([]*models.Project, error) {
+	return []*models.Project{}, nil
+}
+
 // nil implement
 func (f *fakePM) GetTotal(*models.QueryParam) (int64, error) {
 	return 0, nil

src/ui/api/search.go

@@ -15,11 +15,12 @@
 package api
 
 import (
+	"fmt"
 	"net/http"
 	"sort"
 	"strings"
 
-	"github.com/vmware/harbor/src/common/api"
+	"github.com/vmware/harbor/src/common"
 	"github.com/vmware/harbor/src/common/dao"
 	"github.com/vmware/harbor/src/common/models"
 	"github.com/vmware/harbor/src/common/utils"
@@ -29,7 +30,7 @@ import (
 
 // SearchAPI handles requesst to /api/search
 type SearchAPI struct {
-	api.BaseAPI
+	BaseController
 }
 
 type searchResult struct {
@@ -39,33 +40,25 @@ type searchResult struct {
 
 // Get ...
 func (s *SearchAPI) Get() {
-	userID, _, ok := s.GetUserIDForRequest()
-	if !ok {
-		userID = dao.NonExistUserID
-	}
-
 	keyword := s.GetString("q")
-
-	isSysAdmin, err := dao.IsAdminRole(userID)
-	if err != nil {
-		log.Errorf("failed to check whether the user %d is system admin: %v", userID, err)
-		s.CustomAbort(http.StatusInternalServerError, "internal error")
-	}
+	isAuthenticated := s.SecurityCtx.IsAuthenticated()
+	username := s.SecurityCtx.GetUsername()
+	isSysAdmin := s.SecurityCtx.IsSysAdmin()
 
 	var projects []*models.Project
+	var err error
 
-	if isSysAdmin {
-		projects, err = dao.GetProjects(nil)
-		if err != nil {
-			log.Errorf("failed to get all projects: %v", err)
-			s.CustomAbort(http.StatusInternalServerError, "internal error")
-		}
+	if !isAuthenticated {
+		projects, err = s.ProjectMgr.GetPublic()
+	} else if isSysAdmin {
+		projects, err = s.ProjectMgr.GetAll(nil)
 	} else {
-		projects, err = dao.SearchProjects(userID)
-		if err != nil {
-			log.Errorf("failed to get user %d 's relevant projects: %v", userID, err)
-			s.CustomAbort(http.StatusInternalServerError, "internal error")
-		}
+		projects, err = s.ProjectMgr.GetHasReadPerm(username)
+	}
+	if err != nil {
+		s.HandleInternalServerError(fmt.Sprintf(
+			"failed to get projects: %v", err))
+		return
 	}
 
 	projectSorter := &models.ProjectSorter{Projects: projects}
@@ -76,17 +69,19 @@ func (s *SearchAPI) Get() {
 			continue
 		}
 
-		if userID != dao.NonExistUserID {
-			roles, err := dao.GetUserProjectRoles(userID, p.ProjectID)
+		if isAuthenticated {
+			roles, err := s.ProjectMgr.GetRoles(username, p.ProjectID)
 			if err != nil {
-				log.Errorf("failed to get user's project role: %v", err)
-				s.CustomAbort(http.StatusInternalServerError, "")
-			}
-			if len(roles) != 0 {
-				p.Role = roles[0].RoleID
+				s.HandleInternalServerError(fmt.Sprintf("failed to get roles of user %s to project %d: %v",
+					username, p.ProjectID, err))
+				return
 			}
 
-			if p.Role == models.PROJECTADMIN || isSysAdmin {
+			if len(roles) != 0 {
+				p.Role = roles[0]
+			}
+
+			if p.Role == common.RoleProjectAdmin || isSysAdmin {
 				p.Togglable = true
 			}
 		}

src/ui/projectmanager/db/pm.go

@@ -200,3 +200,13 @@ func (p *ProjectManager) GetTotal(query *models.QueryParam) (
 	int64, error) {
 	return dao.GetTotalOfProjects(query)
 }
+
+// GetHasReadPerm returns projects which are public or the user is a member of
+func (p *ProjectManager) GetHasReadPerm(username ...string) (
+	[]*models.Project, error) {
+	if len(username) == 0 || len(username[0]) == 0 {
+		return p.GetPublic()
+	}
+
+	return dao.GetHasReadPermProjects(username[0])
+}

src/ui/projectmanager/db/pm_test.go

@@ -298,3 +298,58 @@ func TestGetAll(t *testing.T) {
 	}
 	assert.True(t, exist)
 }
+
+func TestGetHasReadPerm(t *testing.T) {
+	pm := &ProjectManager{}
+
+	// do not pass username
+	projects, err := pm.GetHasReadPerm()
+	assert.Nil(t, err)
+	assert.NotEqual(t, 0, len(projects))
+	exist := false
+	for _, project := range projects {
+		if project.ProjectID == 1 {
+			exist = true
+			break
+		}
+	}
+	assert.True(t, exist)
+
+	// username is nil
+	projects, err = pm.GetHasReadPerm("")
+	assert.Nil(t, err)
+	assert.NotEqual(t, 0, len(projects))
+	exist = false
+	for _, project := range projects {
+		if project.ProjectID == 1 {
+			exist = true
+			break
+		}
+	}
+	assert.True(t, exist)
+
+	// valid username
+	id, err := pm.Create(&models.Project{
+		Name:    "get_has_read_perm_test",
+		OwnerID: 1,
+		Public:  0,
+	})
+	assert.Nil(t, err)
+	defer pm.Delete(id)
+
+	projects, err = pm.GetHasReadPerm("admin")
+	assert.Nil(t, err)
+	assert.NotEqual(t, 0, len(projects))
+	exist1 := false
+	exist2 := false
+	for _, project := range projects {
+		if project.ProjectID == 1 {
+			exist1 = true
+		}
+		if project.ProjectID == id {
+			exist2 = true
+		}
+	}
+	assert.True(t, exist1)
+	assert.True(t, exist2)
+}

src/ui/projectmanager/pm.go

@@ -36,4 +36,8 @@ type ProjectManager interface {
 	GetAll(query *models.QueryParam) ([]*models.Project, error)
 	// GetTotal returns the total count according to the query parameters
 	GetTotal(query *models.QueryParam) (int64, error)
+	// GetHasReadPerm returns a project list which the user has read
+	// permission of. The list should contains all public projects and
+	// projects which the user is a member of if the username is not nil
+	GetHasReadPerm(username ...string) ([]*models.Project, error)
 }