]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-9442 Fix wrong notification info when profile inherit from builtin
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Fri, 23 Jun 2017 13:03:16 +0000 (15:03 +0200)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 29 Jun 2017 15:23:19 +0000 (17:23 +0200)
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java
server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesNotificationTest.java
tests/src/test/java/org/sonarqube/tests/qualityProfile/BuiltInQualityProfilesNotificationTest.java

index 2281d186e85d5032966d28391fd510b1f9d213a9..d44eb0d8980212c4911f97c67afb9729f7882fa2 100644 (file)
@@ -53,8 +53,8 @@ public class RegisterQualityProfiles {
   private final System2 system2;
 
   public RegisterQualityProfiles(BuiltInQProfileRepository builtInQProfileRepository,
-                                 DbClient dbClient, BuiltInQProfileInsert builtInQProfileInsert, BuiltInQProfileUpdate builtInQProfileUpdate,
-                                 BuiltInQualityProfilesUpdateListener builtInQualityProfilesNotification, System2 system2) {
+    DbClient dbClient, BuiltInQProfileInsert builtInQProfileInsert, BuiltInQProfileUpdate builtInQProfileUpdate,
+    BuiltInQualityProfilesUpdateListener builtInQualityProfilesNotification, System2 system2) {
     this.builtInQProfileRepository = builtInQProfileRepository;
     this.dbClient = dbClient;
     this.builtInQProfileInsert = builtInQProfileInsert;
@@ -84,7 +84,10 @@ public class RegisterQualityProfiles {
         } else {
           List<ActiveRuleChange> changes = update(dbSession, builtIn, ruleProfile);
           changedProfiles.putAll(builtIn.getQProfileName(), changes.stream()
-            .filter(change -> change.getInheritance() == null || NONE.equals(change.getInheritance()))
+            .filter(change -> {
+              String inheritance = change.getActiveRule().getInheritance();
+              return inheritance == null || NONE.name().equals(inheritance);
+            })
             .collect(MoreCollectors.toList()));
         }
       });
index fd5d65447b23a431c1ad85e944f4d5de9e484f1c..6fbba727fc6521cf8398d04948adc8aa1a8bc022 100644 (file)
@@ -22,11 +22,13 @@ package org.sonar.server.qualityprofile;
 import com.google.common.collect.Multimap;
 import java.util.Arrays;
 import java.util.Random;
+import java.util.function.Consumer;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.mockito.ArgumentCaptor;
 import org.sonar.api.profiles.RulesProfile;
+import org.sonar.api.rule.Severity;
 import org.sonar.api.rules.ActiveRule;
 import org.sonar.api.rules.RulePriority;
 import org.sonar.api.utils.System2;
@@ -61,6 +63,7 @@ import static org.sonar.db.qualityprofile.QualityProfileTesting.newRuleProfileDt
 import static org.sonar.server.language.LanguageTesting.newLanguage;
 import static org.sonar.server.qualityprofile.ActiveRuleChange.Type.ACTIVATED;
 import static org.sonar.server.qualityprofile.ActiveRuleChange.Type.DEACTIVATED;
+import static org.sonar.server.qualityprofile.ActiveRuleChange.Type.UPDATED;
 
 public class RegisterQualityProfilesNotificationTest {
 
@@ -185,29 +188,80 @@ public class RegisterQualityProfilesNotificationTest {
       .extracting(QProfileName::getName, QProfileName::getLanguage)
       .containsExactlyInAnyOrder(
         tuple(dbProfile1.getName(), dbProfile1.getLanguage()),
-        tuple(dbProfile2.getName(), dbProfile2.getLanguage())
-      );
+        tuple(dbProfile2.getName(), dbProfile2.getLanguage()));
     assertThat(updatedProfiles.values())
       .extracting(value -> value.getActiveRule().getRuleId(), ActiveRuleChange::getType)
       .containsExactlyInAnyOrder(
         tuple(newRule1.getId(), ACTIVATED),
-        tuple(newRule2.getId(), ACTIVATED)
-      );
+        tuple(newRule2.getId(), ACTIVATED));
   }
 
   @Test
-  public void do_not_include_inherited_quality_profile_changes() {
+  public void do_not_include_inherited_quality_profile_change_on_new_rule() {
     String language = newLanguageKey();
+    RuleDefinitionDto newRule = db.rules().insert(r -> r.setLanguage(language));
+    OrganizationDto organization = db.organizations().insert();
+
+    QProfileDto builtInQProfileDto = insertProfile(organization, orgQProfile -> orgQProfile.setIsBuiltIn(true).setLanguage(language));
+    QProfileDto childQProfileDto = insertProfile(organization, orgQProfile -> orgQProfile.setIsBuiltIn(false).setLanguage(language).setParentKee(builtInQProfileDto.getKee()));
+    addPluginProfile(builtInQProfileDto, newRule);
+    builtInQProfileRepositoryRule.initialize();
+
+    underTest.start();
 
+    ArgumentCaptor<Multimap> captor = ArgumentCaptor.forClass(Multimap.class);
+    verify(builtInQualityProfilesNotification).onChange(captor.capture(), anyLong(), anyLong());
+    Multimap<QProfileName, ActiveRuleChange> updatedProfiles = captor.<Multimap<QProfileName, ActiveRuleChange>>getValue();
+    assertThat(updatedProfiles.keySet())
+      .extracting(QProfileName::getName, QProfileName::getLanguage)
+      .containsExactlyInAnyOrder(tuple(builtInQProfileDto.getName(), builtInQProfileDto.getLanguage()));
+    assertThat(updatedProfiles.values())
+      .extracting(value -> value.getActiveRule().getRuleId(), ActiveRuleChange::getType)
+      .containsExactlyInAnyOrder(tuple(newRule.getId(), ACTIVATED));
+  }
+
+  @Test
+  public void do_not_include_inherited_quality_profile_change_on_existing_rule() {
+    String language = newLanguageKey();
+    RuleDefinitionDto ruleDefinitionDto = db.rules().insert(r -> r.setLanguage(language).setSeverity(Severity.MINOR));
     OrganizationDto organization = db.organizations().insert();
-    QProfileDto builtInQProfileDto = db.qualityProfiles().insert(organization, orgQProfile -> orgQProfile.setIsBuiltIn(true).setLanguage(language));
-    QProfileDto customQProfileDto = db.qualityProfiles().insert(organization, orgQProfile -> orgQProfile.setIsBuiltIn(false).setLanguage(language).setParentKee(builtInQProfileDto.getKee()));
+
+    QProfileDto builtInQProfileDto = insertProfile(organization, orgQProfile -> orgQProfile.setIsBuiltIn(true).setLanguage(language));
+    ruleActivator.activate(db.getSession(), RuleActivation.create(ruleDefinitionDto.getKey()), builtInQProfileDto);
+    QProfileDto childQProfileDto = insertProfile(organization, orgQProfile -> orgQProfile.setIsBuiltIn(false).setLanguage(language).setParentKee(builtInQProfileDto.getKee()));
+    ruleActivator.activate(db.getSession(), RuleActivation.create(ruleDefinitionDto.getKey()), childQProfileDto);
     db.commit();
-    RuleDefinitionDto newRule = db.rules().insert(r -> r.setLanguage(language));
+    addPluginProfile(builtInQProfileDto, ruleDefinitionDto);
+    builtInQProfileRepositoryRule.initialize();
+
+    underTest.start();
+
+    ArgumentCaptor<Multimap> captor = ArgumentCaptor.forClass(Multimap.class);
+    verify(builtInQualityProfilesNotification).onChange(captor.capture(), anyLong(), anyLong());
+    Multimap<QProfileName, ActiveRuleChange> updatedProfiles = captor.<Multimap<QProfileName, ActiveRuleChange>>getValue();
+    assertThat(updatedProfiles.keySet())
+      .extracting(QProfileName::getName, QProfileName::getLanguage)
+      .containsExactlyInAnyOrder(tuple(builtInQProfileDto.getName(), builtInQProfileDto.getLanguage()));
+    assertThat(updatedProfiles.values())
+      .extracting(value -> value.getActiveRule().getRuleId(), ActiveRuleChange::getType)
+      .containsExactlyInAnyOrder(tuple(ruleDefinitionDto.getId(), UPDATED));
+  }
 
-    RulesProfile pluginProfile = RulesProfile.create(builtInQProfileDto.getName(), builtInQProfileDto.getLanguage());
-    pluginProfile.activateRule(create(newRule.getRepositoryKey(), newRule.getRuleKey()), MAJOR);
-    builtInQProfileRepositoryRule.add(newLanguage(builtInQProfileDto.getLanguage()), builtInQProfileDto.getName(), false, pluginProfile.getActiveRules().toArray(new ActiveRule[0]));
+  @Test
+  public void do_not_include_inherited_quality_profile_change_on_deactivated_rule() {
+    String language = newLanguageKey();
+    RuleDefinitionDto ruleDefinitionDto = db.rules().insert(r -> r.setLanguage(language).setSeverity(Severity.MINOR));
+    OrganizationDto organization = db.organizations().insert();
+
+    QProfileDto builtInQProfileDto = insertProfile(organization,
+      orgQProfile -> orgQProfile.setIsBuiltIn(true).setLanguage(language));
+    ruleActivator.activate(db.getSession(), RuleActivation.create(ruleDefinitionDto.getKey()), builtInQProfileDto);
+    QProfileDto childQProfileDto = insertProfile(organization,
+      orgQProfile -> orgQProfile.setIsBuiltIn(false).setLanguage(language).setParentKee(builtInQProfileDto.getKee()));
+    ruleActivator.activate(db.getSession(), RuleActivation.create(ruleDefinitionDto.getKey()), childQProfileDto);
+    db.commit();
+
+    addPluginProfile(builtInQProfileDto);
     builtInQProfileRepositoryRule.initialize();
 
     underTest.start();
@@ -220,7 +274,7 @@ public class RegisterQualityProfilesNotificationTest {
       .containsExactlyInAnyOrder(tuple(builtInQProfileDto.getName(), builtInQProfileDto.getLanguage()));
     assertThat(updatedProfiles.values())
       .extracting(value -> value.getActiveRule().getRuleId(), ActiveRuleChange::getType)
-      .containsExactlyInAnyOrder(tuple(newRule.getId(), ACTIVATED));
+      .containsExactlyInAnyOrder(tuple(ruleDefinitionDto.getId(), DEACTIVATED));
   }
 
   @Test
@@ -243,10 +297,16 @@ public class RegisterQualityProfilesNotificationTest {
 
   private void addPluginProfile(RulesProfileDto dbProfile, RuleDefinitionDto... dbRules) {
     RulesProfile pluginProfile = RulesProfile.create(dbProfile.getName(), dbProfile.getLanguage());
-    Arrays.stream(dbRules).forEach(dbRule -> pluginProfile.activateRule(create(dbRule.getRepositoryKey(), dbRule.getRuleKey()), MAJOR));
+    Arrays.stream(dbRules).forEach(dbRule -> pluginProfile.activateRule(create(dbRule.getRepositoryKey(), dbRule.getRuleKey()), null));
     builtInQProfileRepositoryRule.add(newLanguage(dbProfile.getLanguage()), dbProfile.getName(), false, pluginProfile.getActiveRules().toArray(new ActiveRule[0]));
   }
 
+  private void addPluginProfile(QProfileDto profile, RuleDefinitionDto... dbRules) {
+    RulesProfile pluginProfile = RulesProfile.create(profile.getName(), profile.getLanguage());
+    Arrays.stream(dbRules).forEach(dbRule -> pluginProfile.activateRule(create(dbRule.getRepositoryKey(), dbRule.getRuleKey()), null));
+    builtInQProfileRepositoryRule.add(newLanguage(profile.getLanguage()), profile.getName(), false, pluginProfile.getActiveRules().toArray(new ActiveRule[0]));
+  }
+
   private RulesProfileDto insertBuiltInProfile(String language) {
     RulesProfileDto ruleProfileDto = newRuleProfileDto(rp -> rp.setIsBuiltIn(true).setLanguage(language));
     db.getDbClient().qualityProfileDao().insert(db.getSession(), ruleProfileDto);
@@ -265,6 +325,12 @@ public class RegisterQualityProfilesNotificationTest {
     db.commit();
   }
 
+  private QProfileDto insertProfile(OrganizationDto organization, Consumer<QProfileDto> consumer) {
+    QProfileDto builtInQProfileDto = db.qualityProfiles().insert(organization, consumer);
+    db.commit();
+    return builtInQProfileDto;
+  }
+
   private static String newLanguageKey() {
     return randomAlphanumeric(20).toLowerCase();
   }
index d11f252ddad3fc368bf5326f388ce5051ef8e839..92a7b8977898b9d9afa8f17f3eb8312f0ce971be 100644 (file)
@@ -33,6 +33,8 @@ import org.sonarqube.ws.client.PostRequest;
 import org.sonarqube.ws.client.WsClient;
 import org.sonarqube.ws.client.permission.AddGroupWsRequest;
 import org.sonarqube.ws.client.permission.AddUserWsRequest;
+import org.sonarqube.ws.client.qualityprofile.ChangeParentRequest;
+import org.sonarqube.ws.client.qualityprofile.CreateRequest;
 import org.subethamail.wiser.Wiser;
 import org.subethamail.wiser.WiserMessage;
 import util.ItUtils;
@@ -72,21 +74,11 @@ public class BuiltInQualityProfilesNotificationTest {
       .setServerProperty("email.smtp_port.secured", Integer.toString(smtpServer.getServer().getPort()))
       .build();
     orchestrator.start();
-
     userRule = UserRule.from(orchestrator);
-
     WsUsers.CreateWsResponse.User profileAdmin1 = userRule.generate();
     WsClient wsClient = ItUtils.newAdminWsClient(orchestrator);
     wsClient.permissions().addUser(new AddUserWsRequest().setLogin(profileAdmin1.getLogin()).setPermission("profileadmin"));
 
-    WsUsers.CreateWsResponse.User profileAdmin2 = userRule.generate();
-    String groupName = randomAlphanumeric(20);
-    wsClient.wsConnector().call(new PostRequest("api/user_groups/create").setParam("name", groupName)).failIfNotSuccessful();
-    wsClient.permissions().addGroup(new AddGroupWsRequest().setPermission("profileadmin").setGroupName(groupName));
-    wsClient.wsConnector().call(new PostRequest("api/user_groups/add_user").setParam("name", groupName).setParam("login", profileAdmin2.getLogin())).failIfNotSuccessful();
-
-    WsUsers.CreateWsResponse.User noProfileAdmin = userRule.generate();
-
     orchestrator.restartServer();
 
     waitUntilAllNotificationsAreDelivered(1, 10, 100);
@@ -104,21 +96,25 @@ public class BuiltInQualityProfilesNotificationTest {
 
     userRule = UserRule.from(orchestrator);
 
+    // Create a quality profile administrator (user having direct permission)
     WsUsers.CreateWsResponse.User profileAdmin1 = userRule.generate();
     WsClient wsClient = ItUtils.newAdminWsClient(orchestrator);
     wsClient.permissions().addUser(new AddUserWsRequest().setLogin(profileAdmin1.getLogin()).setPermission("profileadmin"));
-
+    // Create a quality profile administrator (user having permission from a group)
     WsUsers.CreateWsResponse.User profileAdmin2 = userRule.generate();
     String groupName = randomAlphanumeric(20);
     wsClient.wsConnector().call(new PostRequest("api/user_groups/create").setParam("name", groupName)).failIfNotSuccessful();
     wsClient.permissions().addGroup(new AddGroupWsRequest().setPermission("profileadmin").setGroupName(groupName));
     wsClient.wsConnector().call(new PostRequest("api/user_groups/add_user").setParam("name", groupName).setParam("login", profileAdmin2.getLogin())).failIfNotSuccessful();
-
+    // Create a user not being quality profile administrator
     WsUsers.CreateWsResponse.User noProfileAdmin = userRule.generate();
 
+    // Create a child profile on the built-in profile => The notification should not take into account updates of this profile
+    wsClient.qualityProfiles().create(CreateRequest.builder().setLanguage("foo").setProfileName("child").build());
+    wsClient.qualityProfiles().changeParent(ChangeParentRequest.builder().setProfileName("child").setParentName("Basic").setLanguage("foo").build());
+
     // uninstall plugin V1
     wsClient.wsConnector().call(new PostRequest("api/plugins/uninstall").setParam("key", "foo")).failIfNotSuccessful();
-
     // install plugin V2
     File pluginsDir = new File(orchestrator.getServer().getHome() + "/extensions/plugins");
     orchestrator.getConfiguration().fileSystem().copyToDirectory(pluginArtifact("foo-plugin-v2"), pluginsDir);
@@ -156,24 +152,13 @@ public class BuiltInQualityProfilesNotificationTest {
       .setServerProperty("email.smtp_port.secured", Integer.toString(smtpServer.getServer().getPort()))
       .build();
     orchestrator.start();
-
     userRule = UserRule.from(orchestrator);
-
     WsUsers.CreateWsResponse.User profileAdmin1 = userRule.generate();
     WsClient wsClient = ItUtils.newAdminWsClient(orchestrator);
     wsClient.permissions().addUser(new AddUserWsRequest().setLogin(profileAdmin1.getLogin()).setPermission("profileadmin"));
 
-    WsUsers.CreateWsResponse.User profileAdmin2 = userRule.generate();
-    String groupName = randomAlphanumeric(20);
-    wsClient.wsConnector().call(new PostRequest("api/user_groups/create").setParam("name", groupName)).failIfNotSuccessful();
-    wsClient.permissions().addGroup(new AddGroupWsRequest().setPermission("profileadmin").setGroupName(groupName));
-    wsClient.wsConnector().call(new PostRequest("api/user_groups/add_user").setParam("name", groupName).setParam("login", profileAdmin2.getLogin())).failIfNotSuccessful();
-
-    WsUsers.CreateWsResponse.User noProfileAdmin = userRule.generate();
-
     // uninstall plugin V1
     wsClient.wsConnector().call(new PostRequest("api/plugins/uninstall").setParam("key", "foo")).failIfNotSuccessful();
-
     // install plugin V2
     File pluginsDir = new File(orchestrator.getServer().getHome() + "/extensions/plugins");
     orchestrator.getConfiguration().fileSystem().copyToDirectory(pluginArtifact("foo-plugin-v2"), pluginsDir);