Delete project member when delete project (#19523)

Signed-off-by: stonezdj <daojunz@vmware.com>

src/controller/event/handler/internal/project.go

@@ -21,6 +21,7 @@ import (
 	"github.com/goharbor/harbor/src/controller/immutable"
 	"github.com/goharbor/harbor/src/controller/retention"
 	"github.com/goharbor/harbor/src/lib/log"
+	"github.com/goharbor/harbor/src/pkg/member"
 )
 
 // ProjectEventHandler process project event data
@@ -39,16 +40,15 @@ func (a *ProjectEventHandler) IsStateful() bool {
 
 func (a *ProjectEventHandler) onProjectDelete(ctx context.Context, event *event.DeleteProjectEvent) error {
 	log.Infof("delete project id: %d", event.ProjectID)
-	// delete tag immutable
-	err := immutable.Ctr.DeleteImmutableRuleByProject(ctx, event.ProjectID)
-	if err != nil {
+	if err := immutable.Ctr.DeleteImmutableRuleByProject(ctx, event.ProjectID); err != nil {
 		log.Errorf("failed to delete immutable rule, error %v", err)
 	}
-	// delete tag retention
-	err = retention.Ctl.DeleteRetentionByProject(ctx, event.ProjectID)
-	if err != nil {
+	if err := retention.Ctl.DeleteRetentionByProject(ctx, event.ProjectID); err != nil {
 		log.Errorf("failed to delete retention rule, error %v", err)
 	}
+	if err := member.Mgr.DeleteMemberByProjectID(ctx, event.ProjectID); err != nil {
+		log.Errorf("failed to delete project member, error %v", err)
+	}
 	return nil
 }
 

src/controller/event/handler/internal/project_test.go

@@ -21,7 +21,9 @@ import (
 	beegoorm "github.com/beego/beego/v2/client/orm"
 	"github.com/stretchr/testify/suite"
 
+	"github.com/goharbor/harbor/src/common"
 	common_dao "github.com/goharbor/harbor/src/common/dao"
+	commonmodels "github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/controller/event"
 	"github.com/goharbor/harbor/src/controller/immutable"
 	"github.com/goharbor/harbor/src/lib/config"
@@ -29,11 +31,14 @@ import (
 	"github.com/goharbor/harbor/src/pkg"
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	immutableModel "github.com/goharbor/harbor/src/pkg/immutable/model"
+	"github.com/goharbor/harbor/src/pkg/member"
+	memberModels "github.com/goharbor/harbor/src/pkg/member/models"
 	"github.com/goharbor/harbor/src/pkg/project"
 	"github.com/goharbor/harbor/src/pkg/project/models"
 	"github.com/goharbor/harbor/src/pkg/repository/model"
 	"github.com/goharbor/harbor/src/pkg/tag"
 	tagmodel "github.com/goharbor/harbor/src/pkg/tag/model/tag"
+	"github.com/goharbor/harbor/src/pkg/user"
 )
 
 // ProjectHandlerTestSuite is test suite for artifact handler.
@@ -81,6 +86,18 @@ func (suite *ProjectHandlerTestSuite) TestOnProjectDelete() {
 	projID, err := project.New().Create(suite.ctx, &models.Project{Name: "test-project", OwnerID: 1})
 	suite.Nil(err)
 
+	userID, err := user.Mgr.Create(suite.ctx, &commonmodels.User{Username: "test-user-event", Email: "test-user-event@example.com"})
+	defer user.Mgr.Delete(suite.ctx, userID)
+
+	// create project member
+	_, err = member.Mgr.AddProjectMember(suite.ctx, memberModels.Member{ProjectID: projID, EntityType: common.UserMember, EntityID: userID, Role: 1})
+	suite.Nil(err)
+
+	// verify project member
+	members, err := member.Mgr.SearchMemberByName(suite.ctx, projID, "test-user-event")
+	suite.Nil(err)
+	suite.Equal(1, len(members))
+
 	defer project.New().Delete(suite.ctx, projID)
 	immutableRule := &immutableModel.Metadata{
 		ProjectID: projID,
@@ -116,6 +133,11 @@ func (suite *ProjectHandlerTestSuite) TestOnProjectDelete() {
 	// check if immutable rule is deleted
 	_, err = immutable.Ctr.GetImmutableRule(suite.ctx, immutableID)
 	suite.NotNil(err)
+
+	// check if project member is deleted
+	mbs, err := member.Mgr.SearchMemberByName(suite.ctx, projID, "test-user-event")
+	suite.Nil(err)
+	suite.Equal(0, len(mbs))
 }
 
 // TestArtifactHandler tests ArtifactHandler.

src/pkg/member/dao/dao.go

@@ -44,6 +44,8 @@ type DAO interface {
 	DeleteProjectMemberByID(ctx context.Context, projectID int64, pmid int) error
 	// DeleteProjectMemberByUserID -- Delete project member by user id
 	DeleteProjectMemberByUserID(ctx context.Context, uid int) error
+	// DeleteProjectMemberByProjectID -- Delete project member by project id
+	DeleteProjectMemberByProjectID(ctx context.Context, projectID int64) error
 	// SearchMemberByName search members of the project by entity_name
 	SearchMemberByName(ctx context.Context, projectID int64, entityName string) ([]*models.Member, error)
 	// ListRoles lists the roles of user for the specific project
@@ -261,3 +263,15 @@ func (d *dao) ListRoles(ctx context.Context, user *models.User, projectID int64)
 	}
 	return roles, nil
 }
+
+func (d *dao) DeleteProjectMemberByProjectID(ctx context.Context, projectID int64) error {
+	o, err := orm.FromContext(ctx)
+	if err != nil {
+		return err
+	}
+	sql := "delete from project_member where project_id = ?"
+	if _, err := o.Raw(sql, projectID).Exec(); err != nil {
+		return err
+	}
+	return nil
+}

src/pkg/member/dao/dao_test.go

@@ -16,6 +16,7 @@ package dao
 
 import (
 	"database/sql"
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/suite"
@@ -293,6 +294,59 @@ func (s *DaoTestSuite) TestDeleteProjectMemberByUserId() {
 	s.Nil(err)
 }
 
+func (s *DaoTestSuite) TestDeleteProjectMemberByProjectID() {
+	s.WithUser(func(userID int64, username string) {
+		proj2, err := s.projectMgr.Get(s.Context(), "member_test_02")
+		s.Nil(err)
+		s.NotNil(proj2)
+		var addMember = models.Member{
+			ProjectID:  proj2.ProjectID,
+			EntityID:   int(userID),
+			EntityType: common.UserMember,
+			Role:       common.RoleDeveloper,
+		}
+		pmid, err := s.dao.AddProjectMember(s.Context(), addMember)
+		s.Nil(err)
+		s.True(pmid > 0)
+
+		err = s.dao.DeleteProjectMemberByProjectID(s.Context(), proj2.ProjectID)
+		s.Nil(err)
+
+		queryMember := models.Member{ProjectID: proj2.ProjectID, EntityID: int(userID), EntityType: common.UserMember}
+
+		// not exist
+		members, err := s.dao.GetProjectMember(s.Context(), queryMember, nil)
+		s.True(len(members) == 0)
+		s.Nil(err)
+	}, "test_project_member_delete")
+}
+
+func (s *DaoTestSuite) WithUser(f func(int64, string), usernames ...string) {
+	var username string
+	if len(usernames) > 0 {
+		username = usernames[0]
+	} else {
+		username = s.RandString(5)
+	}
+
+	o, err := orm.FromContext(orm.Context())
+	if err != nil {
+		s.Fail("got error %v", err)
+	}
+
+	var userID int64
+
+	email := fmt.Sprintf("%s@example.com", username)
+	sql := "INSERT INTO harbor_user (username, realname, email, password) VALUES (?, ?, ?, 'Harbor12345') RETURNING user_id"
+	s.Nil(o.Raw(sql, username, username, email).QueryRow(&userID))
+
+	defer func() {
+		o.Raw("delete from harbor_user WHERE user_id = ?", userID).Exec()
+	}()
+
+	f(userID, username)
+}
+
 func TestDaoTestSuite(t *testing.T) {
 	suite.Run(t, &DaoTestSuite{})
 }

src/pkg/member/manager.go

@@ -44,6 +44,8 @@ type Manager interface {
 	SearchMemberByName(ctx context.Context, projectID int64, entityName string) ([]*models.Member, error)
 	// DeleteMemberByUserID delete project member by user id
 	DeleteMemberByUserID(ctx context.Context, uid int) error
+	// DeleteMemberByProjectID delete project member by project id
+	DeleteMemberByProjectID(ctx context.Context, projectID int64) error
 	// GetTotalOfProjectMembers get the total amount of project members
 	GetTotalOfProjectMembers(ctx context.Context, projectID int64, query *q.Query, roles ...int) (int, error)
 	// ListRoles list project roles
@@ -101,6 +103,9 @@ func (m *manager) Delete(ctx context.Context, projectID int64, memberID int) err
 func (m *manager) DeleteMemberByUserID(ctx context.Context, uid int) error {
 	return m.dao.DeleteProjectMemberByUserID(ctx, uid)
 }
+func (m *manager) DeleteMemberByProjectID(ctx context.Context, projectID int64) error {
+	return m.dao.DeleteProjectMemberByProjectID(ctx, projectID)
+}
 
 // NewManager ...
 func NewManager() Manager {

src/testing/pkg/member/fake_member_manager.go

@@ -55,6 +55,20 @@ func (_m *Manager) Delete(ctx context.Context, projectID int64, memberID int) er
 	return r0
 }
 
+// DeleteMemberByProjectID provides a mock function with given fields: ctx, projectID
+func (_m *Manager) DeleteMemberByProjectID(ctx context.Context, projectID int64) error {
+	ret := _m.Called(ctx, projectID)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, int64) error); ok {
+		r0 = rf(ctx, projectID)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // DeleteMemberByUserID provides a mock function with given fields: ctx, uid
 func (_m *Manager) DeleteMemberByUserID(ctx context.Context, uid int) error {
 	ret := _m.Called(ctx, uid)