From c4af6ff82417b83cac01418bbcb34d4a2c7d5883 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Wenkai=20Yin=28=E5=B0=B9=E6=96=87=E5=BC=80=29?=
 <yinw@vmware.com>
Date: Mon, 23 Mar 2020 10:38:47 +0800
Subject: [PATCH] Fix bug when deleting the repository (#11121)

Fixes #10997 by looping the artifact candidates until all of them are deleted

Signed-off-by: Wenkai Yin <yinw@vmware.com>
---
 src/api/repository/controller.go      | 34 +++++++++++++++++++++++----
 src/api/repository/controller_test.go |  5 ++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/src/api/repository/controller.go b/src/api/repository/controller.go
index 55157828c..8b6cd7636 100644
--- a/src/api/repository/controller.go
+++ b/src/api/repository/controller.go
@@ -16,11 +16,14 @@ package repository
 
 import (
 	"context"
+
 	"github.com/goharbor/harbor/src/api/artifact"
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
+	"github.com/goharbor/harbor/src/common/utils/log"
 	ierror "github.com/goharbor/harbor/src/internal/error"
 	"github.com/goharbor/harbor/src/internal/orm"
+	art "github.com/goharbor/harbor/src/pkg/artifact"
 	"github.com/goharbor/harbor/src/pkg/project"
 	"github.com/goharbor/harbor/src/pkg/q"
 	"github.com/goharbor/harbor/src/pkg/repository"
@@ -58,6 +61,7 @@ func NewController() Controller {
 	return &controller{
 		proMgr:  project.Mgr,
 		repoMgr: repository.Mgr,
+		artMgr:  art.Mgr,
 		artCtl:  artifact.Ctl,
 	}
 }
@@ -65,6 +69,7 @@ func NewController() Controller {
 type controller struct {
 	proMgr  project.Manager
 	repoMgr repository.Manager
+	artMgr  art.Manager
 	artCtl  artifact.Controller
 }
 
@@ -137,7 +142,7 @@ func (c *controller) GetByName(ctx context.Context, name string) (*models.RepoRe
 }
 
 func (c *controller) Delete(ctx context.Context, id int64) error {
-	artifacts, err := c.artCtl.List(ctx, &q.Query{
+	candidates, err := c.artCtl.List(ctx, &q.Query{
 		Keywords: map[string]interface{}{
 			"RepositoryID": id,
 		},
@@ -145,9 +150,30 @@ func (c *controller) Delete(ctx context.Context, id int64) error {
 	if err != nil {
 		return err
 	}
-	for _, artifact := range artifacts {
-		if err = c.artCtl.Delete(ctx, artifact.ID); err != nil {
-			return err
+	for len(candidates) > 0 {
+		artifacts := candidates
+		candidates = nil
+		for _, artifact := range artifacts {
+			parents, err := c.artMgr.ListReferences(ctx, &q.Query{
+				Keywords: map[string]interface{}{
+					"ChildID": artifact.ID,
+				},
+			})
+			if err != nil {
+				return err
+			}
+			if len(parents) > 0 {
+				// if the artifact is referenced by others, put it into the artifacts
+				// list again, and try to delete it in the next loop
+				log.Debugf("the artifact %d is referenced by others, put it into the backlog and try to delete it in the next loop",
+					artifact.ID)
+				candidates = append(candidates, artifact)
+				continue
+			}
+			if err = c.artCtl.Delete(ctx, artifact.ID); err != nil {
+				return err
+			}
+			log.Debugf("the artifact %d is deleted", artifact.ID)
 		}
 	}
 	return c.repoMgr.Delete(ctx, id)
diff --git a/src/api/repository/controller_test.go b/src/api/repository/controller_test.go
index 9153a45d5..2c3aa12c2 100644
--- a/src/api/repository/controller_test.go
+++ b/src/api/repository/controller_test.go
@@ -22,6 +22,7 @@ import (
 	artifacttesting "github.com/goharbor/harbor/src/testing/api/artifact"
 	ormtesting "github.com/goharbor/harbor/src/testing/lib/orm"
 	"github.com/goharbor/harbor/src/testing/mock"
+	arttesting "github.com/goharbor/harbor/src/testing/pkg/artifact"
 	"github.com/goharbor/harbor/src/testing/pkg/project"
 	"github.com/goharbor/harbor/src/testing/pkg/repository"
 	"github.com/stretchr/testify/suite"
@@ -33,16 +34,19 @@ type controllerTestSuite struct {
 	ctl     *controller
 	proMgr  *project.FakeManager
 	repoMgr *repository.FakeManager
+	argMgr  *arttesting.FakeManager
 	artCtl  *artifacttesting.Controller
 }
 
 func (c *controllerTestSuite) SetupTest() {
 	c.proMgr = &project.FakeManager{}
 	c.repoMgr = &repository.FakeManager{}
+	c.argMgr = &arttesting.FakeManager{}
 	c.artCtl = &artifacttesting.Controller{}
 	c.ctl = &controller{
 		proMgr:  c.proMgr,
 		repoMgr: c.repoMgr,
+		artMgr:  c.argMgr,
 		artCtl:  c.artCtl,
 	}
 }
@@ -119,6 +123,7 @@ func (c *controllerTestSuite) TestGetByName() {
 func (c *controllerTestSuite) TestDelete() {
 	art := &artifact.Artifact{}
 	art.ID = 1
+	c.argMgr.On("ListReferences").Return(nil, nil)
 	mock.OnAnything(c.artCtl, "List").Return([]*artifact.Artifact{art}, nil)
 	mock.OnAnything(c.artCtl, "Delete").Return(nil)
 	c.repoMgr.On("Delete").Return(nil)