From 530ba1d27b49d7b3167d239cf6ff7c8366343f83 Mon Sep 17 00:00:00 2001
From: Wenkai Yin <yinw@vmware.com>
Date: Wed, 13 Feb 2019 16:12:35 +0800
Subject: [PATCH] Fix #6698

This commit fixes the issue #6698: cannot create a same name replication policy after deleting it

Signed-off-by: Wenkai Yin <yinw@vmware.com>
---
 src/common/dao/dao_test.go             |  8 +---
 src/common/dao/replication_job.go      | 16 ++++----
 src/common/dao/replication_job_test.go | 56 ++++++++++++++++++++++++++
 src/replication/policy/manager.go      |  5 +++
 4 files changed, 72 insertions(+), 13 deletions(-)
 create mode 100644 src/common/dao/replication_job_test.go

diff --git a/src/common/dao/dao_test.go b/src/common/dao/dao_test.go
index c69e492a4..9b83301d1 100644
--- a/src/common/dao/dao_test.go
+++ b/src/common/dao/dao_test.go
@@ -1098,12 +1098,8 @@ func TestDeleteRepPolicy(t *testing.T) {
 	}
 	t.Logf("delete rep policy, id: %d", policyID)
 	p, err := GetRepPolicy(policyID)
-	if err != nil && err != orm.ErrNoRows {
-		t.Errorf("Error occurred in GetRepPolicy:%v", err)
-	}
-	if p != nil && !p.Deleted {
-		t.Errorf("Able to find rep policy after deletion, id: %d", policyID)
-	}
+	require.Nil(t, err)
+	assert.Nil(t, p)
 }
 
 func TestGetOrmer(t *testing.T) {
diff --git a/src/common/dao/replication_job.go b/src/common/dao/replication_job.go
index c25eccc1f..e5cd2b109 100644
--- a/src/common/dao/replication_job.go
+++ b/src/common/dao/replication_job.go
@@ -286,13 +286,9 @@ func UpdateRepPolicy(policy *models.RepPolicy) error {
 
 // DeleteRepPolicy ...
 func DeleteRepPolicy(id int64) error {
-	o := GetOrmer()
-	policy := &models.RepPolicy{
-		ID:         id,
-		Deleted:    true,
-		UpdateTime: time.Now(),
-	}
-	_, err := o.Update(policy, "Deleted")
+	_, err := GetOrmer().Delete(&models.RepPolicy{
+		ID: id,
+	})
 	return err
 }
 
@@ -384,6 +380,12 @@ func DeleteRepJob(id int64) error {
 	return err
 }
 
+// DeleteRepJobs deletes replication jobs by policy ID
+func DeleteRepJobs(policyID int64) error {
+	_, err := GetOrmer().QueryTable(&models.RepJob{}).Filter("PolicyID", policyID).Delete()
+	return err
+}
+
 // UpdateRepJobStatus ...
 func UpdateRepJobStatus(id int64, status string) error {
 	o := GetOrmer()
diff --git a/src/common/dao/replication_job_test.go b/src/common/dao/replication_job_test.go
new file mode 100644
index 000000000..9bfb38689
--- /dev/null
+++ b/src/common/dao/replication_job_test.go
@@ -0,0 +1,56 @@
+// Copyright Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package dao
+
+import (
+	"testing"
+
+	"github.com/goharbor/harbor/src/common/models"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDeleteRepJobs(t *testing.T) {
+	var policyID int64 = 999
+	_, err := AddRepJob(models.RepJob{
+		PolicyID:   policyID,
+		Repository: "library/hello-world",
+		Operation:  "delete",
+		Status:     "success",
+	})
+	require.Nil(t, err)
+	_, err = AddRepJob(models.RepJob{
+		PolicyID:   policyID,
+		Repository: "library/hello-world",
+		Operation:  "delete",
+		Status:     "success",
+	})
+	require.Nil(t, err)
+
+	jobs, err := GetRepJobs(&models.RepJobQuery{
+		PolicyID: policyID,
+	})
+	require.Nil(t, err)
+	require.Equal(t, 2, len(jobs))
+
+	err = DeleteRepJobs(policyID)
+	require.Nil(t, err)
+
+	jobs, err = GetRepJobs(&models.RepJobQuery{
+		PolicyID: policyID,
+	})
+	require.Nil(t, err)
+	assert.Equal(t, 0, len(jobs))
+}
diff --git a/src/replication/policy/manager.go b/src/replication/policy/manager.go
index 824885a9e..7210a1782 100644
--- a/src/replication/policy/manager.go
+++ b/src/replication/policy/manager.go
@@ -196,5 +196,10 @@ func (m *DefaultManager) UpdatePolicy(policy models.ReplicationPolicy) error {
 // RemovePolicy removes the specified policy;
 // If removing failed, error will be returned.
 func (m *DefaultManager) RemovePolicy(policyID int64) error {
+	// delete replication jobs
+	if err := dao.DeleteRepJobs(policyID); err != nil {
+		return err
+	}
+	// delete the replication policy
 	return dao.DeleteRepPolicy(policyID)
 }