From 8b16808a43932555a51d8e6e08a615666d747881 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lievremont Date: Thu, 9 Apr 2015 18:12:28 +0200 Subject: [PATCH] SONAR-6306 SONAR-6307 Apply feedback from PR --- .../ws/QProfileChangeParentAction.java | 14 +++- .../ws/QProfileInheritanceAction.java | 2 + .../QProfileChangeParentActionMediumTest.java | 71 ++++++++++++++++--- .../QProfileInheritanceActionMediumTest.java | 8 +++ .../inheritance-simple.json | 7 ++ 5 files changed, 90 insertions(+), 12 deletions(-) create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/qualityprofile/ws/QProfileInheritanceActionMediumTest/inheritance-simple.json diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileChangeParentAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileChangeParentAction.java index 9c6d01f161e..2401854a00a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileChangeParentAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileChangeParentAction.java @@ -63,7 +63,9 @@ public class QProfileChangeParentAction implements BaseQProfileWsAction { 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 @@ public class QProfileChangeParentAction implements BaseQProfileWsAction { 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; } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileInheritanceAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileInheritanceAction.java index 2212384869c..57ec4fdae77 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileInheritanceAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileInheritanceAction.java @@ -141,6 +141,8 @@ public class QProfileInheritanceAction implements BaseQProfileWsAction { Multimap ancestorStats = profileStats.get(profileKey); json.prop("activeRuleCount", getActiveRuleCount(ancestorStats)); json.prop("overridingRuleCount", getOverridingRuleCount(ancestorStats)); + } else { + json.prop("activeRuleCount", 0); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileChangeParentActionMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileChangeParentActionMediumTest.java index c95c3d9ec47..4258b4a73b9 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileChangeParentActionMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileChangeParentActionMediumTest.java @@ -34,6 +34,7 @@ import org.sonar.core.rule.RuleDto; 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 class QProfileChangeParentActionMediumTest { // 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 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 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 activeRules = db.activeRuleDao().findByProfileKey(session, child.getKey()); assertThat(activeRules).isEmpty(); } @@ -139,7 +140,7 @@ public class QProfileChangeParentActionMediumTest { .execute(); session.clearCache(); - // 1. Assert ActiveRule in DAO + // 1. check rule 1 enabled List 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 class QProfileChangeParentActionMediumTest { .execute(); session.clearCache(); - // 2. Assert ActiveRule in DAO + // 2. check rule 2 enabled List 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 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 activeRules = db.activeRuleDao().findByProfileKey(session, child.getKey()); + assertThat(activeRules).isEmpty(); } @Test(expected = IllegalArgumentException.class) @@ -170,7 +210,20 @@ public class QProfileChangeParentActionMediumTest { .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) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileInheritanceActionMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileInheritanceActionMediumTest.java index 0a6d0c823a7..9d3d15db5a5 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileInheritanceActionMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileInheritanceActionMediumTest.java @@ -97,6 +97,14 @@ public class QProfileInheritanceActionMediumTest { 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(); diff --git a/server/sonar-server/src/test/resources/org/sonar/server/qualityprofile/ws/QProfileInheritanceActionMediumTest/inheritance-simple.json b/server/sonar-server/src/test/resources/org/sonar/server/qualityprofile/ws/QProfileInheritanceActionMediumTest/inheritance-simple.json new file mode 100644 index 00000000000..b5168de4671 --- /dev/null +++ b/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": [] +} -- 2.39.5