SONAR-6306 SONAR-6307 Apply feedback from PR

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileChangeParentAction.java

@@ -63,7 +63,9 @@ public void define(NewController context) {
     QProfileIdentificationParamUtils.defineProfileParams(inheritance, languages);
 
     inheritance.createParam(PARAM_PARENT_KEY)
-      .setDescription("The key of the new parent profile. If this parameter is set, parentName must not be set. If left empty, ")
+      .setDescription("The key of the new parent profile. If this parameter is set, parentName must not be set. " +
+        "If both are left empty, the inheritance link with current parent profile (if any) is broken, which deactivates all rules " +
+        "which come from the parent and are not overridden.")
       .setExampleValue("sonar-way-java-12345");
     inheritance.createParam(PARAM_PARENT_NAME)
       .setDescription("A quality profile name. If this parameter is set, profileKey must not be set and language must be set to disambiguate.")
@@ -94,9 +96,15 @@ private static String getParentKeyFromParameters(Request request, QProfileFactor
     Preconditions.checkArgument(
       (isEmpty(parentName) || isEmpty(parentKey)), "parentKey and parentName cannot be used simultaneously");
 
-    if (isEmpty(parentKey) && !isEmpty(parentName)) {
-      parentKey = QProfileIdentificationParamUtils.getProfileKeyFromLanguageAndName(language, parentName, profileFactory, session);
+    if (isEmpty(parentKey)) {
+      if (!isEmpty(parentName)) {
+        parentKey = QProfileIdentificationParamUtils.getProfileKeyFromLanguageAndName(language, parentName, profileFactory, session);
+      } else {
+        // Empty parent key is treated as "no more parent"
+        parentKey = null;
+      }
     }
+
     return parentKey;
   }
 }

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileInheritanceAction.java

@@ -141,6 +141,8 @@ private void writeStats(JsonWriter json, String profileKey, Map<String, Multimap
       Multimap<String, FacetValue> ancestorStats = profileStats.get(profileKey);
       json.prop("activeRuleCount", getActiveRuleCount(ancestorStats));
       json.prop("overridingRuleCount", getOverridingRuleCount(ancestorStats));
+    } else {
+      json.prop("activeRuleCount", 0);
     }
   }
 

server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileChangeParentActionMediumTest.java

@@ -34,6 +34,7 @@
 import org.sonar.server.db.DbClient;
 import org.sonar.server.qualityprofile.QProfileName;
 import org.sonar.server.qualityprofile.QProfileTesting;
+import org.sonar.server.qualityprofile.RuleActivator;
 import org.sonar.server.tester.ServerTester;
 import org.sonar.server.user.MockUserSession;
 import org.sonar.server.ws.WsTester;
@@ -84,35 +85,35 @@ public void change_parent_with_keys() throws Exception {
 
     // 1. Set parent 1
     wsTester.newGetRequest(QProfilesWs.API_ENDPOINT, "change_parent")
-      .setParam(QProfileIdentificationParamUtils.PARAM_PROFILE_KEY, child.getKey().toString())
+      .setParam(QProfileIdentificationParamUtils.PARAM_PROFILE_KEY, child.getKey())
       .setParam("parentKey", parent1.getKey().toString())
       .execute();
     session.clearCache();
 
-    // 1. Assert ActiveRule in DAO
+    // 1. Check rule 1 enabled
     List<ActiveRuleDto> activeRules1 = db.activeRuleDao().findByProfileKey(session, child.getKey());
     assertThat(activeRules1).hasSize(1);
     assertThat(activeRules1.get(0).getKey().ruleKey().rule()).isEqualTo("rule1");
 
     // 2. Set parent 2
     wsTester.newGetRequest(QProfilesWs.API_ENDPOINT, "change_parent")
-      .setParam(QProfileIdentificationParamUtils.PARAM_PROFILE_KEY, child.getKey().toString())
+      .setParam(QProfileIdentificationParamUtils.PARAM_PROFILE_KEY, child.getKey())
       .setParam("parentKey", parent2.getKey().toString())
       .execute();
     session.clearCache();
 
-    // 2. Assert ActiveRule in DAO
+    // 2. Check rule 2 enabled
     List<ActiveRuleDto> activeRules2 = db.activeRuleDao().findByProfileKey(session, child.getKey());
     assertThat(activeRules2).hasSize(1);
     assertThat(activeRules2.get(0).getKey().ruleKey().rule()).isEqualTo("rule2");
 
     // 3. Remove parent
     wsTester.newGetRequest(QProfilesWs.API_ENDPOINT, "change_parent")
-      .setParam(QProfileIdentificationParamUtils.PARAM_PROFILE_KEY, child.getKey().toString())
+      .setParam(QProfileIdentificationParamUtils.PARAM_PROFILE_KEY, child.getKey())
       .execute();
     session.clearCache();
 
-    // 3. Assert ActiveRule in DAO
+    // 3. Check no rule enabled
     List<ActiveRuleDto> activeRules = db.activeRuleDao().findByProfileKey(session, child.getKey());
     assertThat(activeRules).isEmpty();
   }
@@ -139,7 +140,7 @@ public void change_parent_with_names() throws Exception {
       .execute();
     session.clearCache();
 
-    // 1. Assert ActiveRule in DAO
+    // 1. check rule 1 enabled
     List<ActiveRuleDto> activeRules1 = db.activeRuleDao().findByProfileKey(session, child.getKey());
     assertThat(activeRules1).hasSize(1);
     assertThat(activeRules1.get(0).getKey().ruleKey().rule()).isEqualTo("rule1");
@@ -152,10 +153,49 @@ public void change_parent_with_names() throws Exception {
       .execute();
     session.clearCache();
 
-    // 2. Assert ActiveRule in DAO
+    // 2. check rule 2 enabled
     List<ActiveRuleDto> activeRules2 = db.activeRuleDao().findByProfileKey(session, child.getKey());
     assertThat(activeRules2).hasSize(1);
     assertThat(activeRules2.get(0).getKey().ruleKey().rule()).isEqualTo("rule2");
+
+    // 3. Remove parent
+    wsTester.newGetRequest(QProfilesWs.API_ENDPOINT, "change_parent")
+      .setParam(QProfileIdentificationParamUtils.PARAM_LANGUAGE, "xoo")
+      .setParam(QProfileIdentificationParamUtils.PARAM_PROFILE_NAME, child.getName())
+      .setParam("parentName", "")
+      .execute();
+    session.clearCache();
+
+    // 3. check no rule enabled
+    List<ActiveRuleDto> activeRules = db.activeRuleDao().findByProfileKey(session, child.getKey());
+    assertThat(activeRules).isEmpty();
+  }
+
+  @Test
+  public void remove_parent_with_empty_key() throws Exception {
+    QualityProfileDto parent = createProfile("xoo", "Parent 1");
+    QualityProfileDto child = createProfile("xoo", "Child");
+
+    RuleDto rule1 = createRule("xoo", "rule1");
+    createActiveRule(rule1, parent);
+    session.commit();
+
+    assertThat(db.activeRuleDao().findByProfileKey(session, child.getKey())).isEmpty();
+
+    // Set parent
+    tester.get(RuleActivator.class).setParent(child.getKey(), parent.getKey());
+    session.clearCache();
+
+    // Remove parent
+    wsTester.newGetRequest(QProfilesWs.API_ENDPOINT, "change_parent")
+      .setParam(QProfileIdentificationParamUtils.PARAM_PROFILE_KEY, child.getKey())
+      .setParam("parentKey", "")
+      .execute();
+    session.clearCache();
+
+    // Check no rule enabled
+    List<ActiveRuleDto> activeRules = db.activeRuleDao().findByProfileKey(session, child.getKey());
+    assertThat(activeRules).isEmpty();
   }
 
   @Test(expected = IllegalArgumentException.class)
@@ -170,7 +210,20 @@ public void fail_if_parent_key_and_name_both_set() throws Exception {
       .setParam("parentName", "polop")
       .setParam("parentKey", "palap")
       .execute();
-    session.clearCache();
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void fail_if_profile_key_and_name_both_set() throws Exception {
+    QualityProfileDto child = createProfile("xoo", "Child");
+    session.commit();
+
+    assertThat(db.activeRuleDao().findByProfileKey(session, child.getKey())).isEmpty();
+
+    wsTester.newGetRequest(QProfilesWs.API_ENDPOINT, "change_parent")
+      .setParam(QProfileIdentificationParamUtils.PARAM_PROFILE_KEY, child.getKee())
+      .setParam(QProfileIdentificationParamUtils.PARAM_PROFILE_NAME, child.getName())
+      .setParam("parentKey", "palap")
+      .execute();
   }
 
   private QualityProfileDto createProfile(String lang, String name) {

server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileInheritanceActionMediumTest.java

@@ -97,6 +97,14 @@ public void inheritance_nominal() throws Exception {
     wsTester.newGetRequest("api/qualityprofiles", "inheritance").setParam("profileKey", buWide.getKee()).execute().assertJson(getClass(), "inheritance-buWide.json");
   }
 
+  @Test
+  public void inheritance_no_family() throws Exception {
+    // Simple profile, no parent, no child
+    QualityProfileDto remi = createProfile("xoo", "Nobodys Boy", "xoo-nobody-s-boy-01234");
+
+    wsTester.newGetRequest("api/qualityprofiles", "inheritance").setParam("profileKey", remi.getKee()).execute().assertJson(getClass(), "inheritance-simple.json");
+  }
+
   @Test(expected = NotFoundException.class)
   public void fail_if_not_found() throws Exception {
     wsTester.newGetRequest("api/qualityprofiles", "inheritance").setParam("profileKey", "polop").execute();

server/sonar-server/src/test/resources/org/sonar/server/qualityprofile/ws/QProfileInheritanceActionMediumTest/inheritance-simple.json

@@ -0,0 +1,7 @@
+{
+  "profile": {
+    "key": "xoo-nobody-s-boy-01234", "name": "Nobodys Boy", "activeRuleCount": 0
+  },
+  "ancestors": [],
+  "children": []
+}