Check the existence of name when creating replication rule and fix bugs in testing library (#6381)

1. Fix #5102 by checking the existence of name when creating/editing replication rule
2. Add unique constraint to the name of replication policy and target
3. Fix bugs of testing library

Signed-off-by: Wenkai Yin <yinw@vmware.com>

make/migrations/postgresql/0002_1.7.0_schema.up.sql

@@ -10,3 +10,6 @@ create table job_log (
 );
 
 CREATE UNIQUE INDEX job_log_uuid ON job_log (job_uuid);
+
+ALTER TABLE replication_policy ADD CONSTRAINT unique_policy_name UNIQUE (name);
+ALTER TABLE replication_target ADD CONSTRAINT unique_target_name UNIQUE (name);

src/common/dao/dao_test.go

@@ -875,27 +875,6 @@ func TestGetRepPolicyByName(t *testing.T) {
 
 }
 
-func TestAddRepPolicy2(t *testing.T) {
-	policy2 := models.RepPolicy{
-		ProjectID:   3,
-		TargetID:    3,
-		Description: "whatever",
-		Name:        "mypolicy",
-	}
-	policyID2, err := AddRepPolicy(policy2)
-	t.Logf("added policy, id: %d", policyID2)
-	if err != nil {
-		t.Errorf("Error occurred in AddRepPolicy: %v", err)
-	}
-	p, err := GetRepPolicy(policyID2)
-	if err != nil {
-		t.Errorf("Error occurred in GetPolicy: %v, id: %d", err, policyID2)
-	}
-	if p == nil {
-		t.Errorf("Unable to find a policy with id: %d", policyID2)
-	}
-}
-
 func TestAddRepJob(t *testing.T) {
 	job := models.RepJob{
 		Repository: "library/ubuntu",

src/core/api/replication_policy.go

@@ -128,6 +128,18 @@ func (pa *RepPolicyAPI) Post() {
 	policy := &api_models.ReplicationPolicy{}
 	pa.DecodeJSONReqAndValidate(policy)
 
+	// check the name
+	exist, err := exist(policy.Name)
+	if err != nil {
+		pa.HandleInternalServerError(fmt.Sprintf("failed to check the existence of policy %s: %v", policy.Name, err))
+		return
+	}
+
+	if exist {
+		pa.HandleConflict(fmt.Sprintf("name %s is already used", policy.Name))
+		return
+	}
+
 	// check the existence of projects
 	for _, project := range policy.Projects {
 		pro, err := pa.ProjectMgr.Get(project.ProjectID)
@@ -191,6 +203,22 @@ func (pa *RepPolicyAPI) Post() {
 	pa.Redirect(http.StatusCreated, strconv.FormatInt(id, 10))
 }
 
+func exist(name string) (bool, error) {
+	result, err := core.GlobalController.GetPolicies(rep_models.QueryParameter{
+		Name: name,
+	})
+	if err != nil {
+		return false, err
+	}
+
+	for _, policy := range result.Policies {
+		if policy.Name == name {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
 // Put updates the replication policy
 func (pa *RepPolicyAPI) Put() {
 	id := pa.GetIDFromURL()
@@ -211,6 +239,20 @@ func (pa *RepPolicyAPI) Put() {
 
 	policy.ID = id
 
+	// check the name
+	if policy.Name != originalPolicy.Name {
+		exist, err := exist(policy.Name)
+		if err != nil {
+			pa.HandleInternalServerError(fmt.Sprintf("failed to check the existence of policy %s: %v", policy.Name, err))
+			return
+		}
+
+		if exist {
+			pa.HandleConflict(fmt.Sprintf("name %s is already used", policy.Name))
+			return
+		}
+	}
+
 	// check the existence of projects
 	for _, project := range policy.Projects {
 		pro, err := pa.ProjectMgr.Get(project.ProjectID)

src/core/api/replication_policy_test.go

@@ -311,6 +311,41 @@ func TestRepPolicyAPIPost(t *testing.T) {
 			code:     http.StatusCreated,
 			postFunc: postFunc,
 		},
+		// 409
+		{
+			request: &testingRequest{
+				method: http.MethodPost,
+				url:    repPolicyAPIBasePath,
+				bodyJSON: &api_models.ReplicationPolicy{
+					Name: policyName,
+					Projects: []*models.Project{
+						{
+							ProjectID: projectID,
+						},
+					},
+					Targets: []*models.RepTarget{
+						{
+							ID: targetID,
+						},
+					},
+					Filters: []rep_models.Filter{
+						{
+							Kind:    replication.FilterItemKindRepository,
+							Pattern: "*",
+						},
+						{
+							Kind:  replication.FilterItemKindLabel,
+							Value: labelID2,
+						},
+					},
+					Trigger: &rep_models.Trigger{
+						Kind: replication.TriggerKindManual,
+					},
+				},
+				credential: sysAdmin,
+			},
+			code: http.StatusConflict,
+		},
 	}
 
 	runCodeCheckingCases(t, cases...)

tests/apitests/python/library/label.py

@@ -5,8 +5,10 @@ import base
 import swagger_client
 
 class Label(base.Base):
-    def create_label(self, name=base._random_name("label"), desc="",
+    def create_label(self, name=None, desc="",
         color="", scope="g", project_id=0, **kwargs):
+        if name is None:
+            name = base._random_name("label")
         label = swagger_client.Label(name=name, 
             description=desc, color=color, 
             scope=scope, project_id=project_id)

tests/apitests/python/library/registry.py

@@ -5,8 +5,10 @@ import base
 import swagger_client
 
 class Registry(base.Base):
-    def create_registry(self, endpoint, name = base._random_name("registry"), username="", 
+    def create_registry(self, endpoint, name=None, username="", 
         password="", insecure=True, **kwargs):
+        if name is None:
+            name = base._random_name("registry")
         client = self._get_client(**kwargs)
         registry = swagger_client.RepTargetPost(name=name, endpoint=endpoint, 
             username=username, password=password, insecure=insecure)

tests/apitests/python/library/replication.py

@@ -7,12 +7,14 @@ import swagger_client
 
 class Replication(base.Base):
     def create_replication_rule(self, 
-        projectIDList, targetIDList, name=base._random_name("rule"), desc="", 
+        projectIDList, targetIDList, name=None, desc="", 
         filters=[], trigger=swagger_client.RepTrigger(kind="Manual"), 
         replicate_deletion=True,
         replicate_existing_image_now=False,
         expect_status_code = 201,
         **kwargs):
+        if name is None:
+            name = base._random_name("rule")
         projects = []
         for projectID in projectIDList:
             projects.append(swagger_client.Project(project_id=int(projectID)))

tests/apitests/python/test_add_replication_rule.py

@@ -43,7 +43,7 @@ class TestProjects(unittest.TestCase):
         #4. Delete user(UA);
         self.user.delete_user(TestProjects.user_add_rule_id, **ADMIN_CLIENT)
 
-    def testAddSysLabelToRepo(self):
+    def testAddReplicationRule(self):
         """
         Test case:
             Add Replication Rule