From: Julien Lancelot Date: Fri, 23 Jun 2017 13:03:16 +0000 (+0200) Subject: SONAR-9442 Fix wrong notification info when profile inherit from builtin X-Git-Tag: 6.5-M2~63 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=40977f259975faee0c9e5f59ae1228ed89138bb9;p=sonarqube.git SONAR-9442 Fix wrong notification info when profile inherit from builtin --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java index 2281d186e85..d44eb0d8980 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java @@ -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 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())); } }); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesNotificationTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesNotificationTest.java index fd5d65447b2..6fbba727fc6 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesNotificationTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesNotificationTest.java @@ -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 captor = ArgumentCaptor.forClass(Multimap.class); + verify(builtInQualityProfilesNotification).onChange(captor.capture(), anyLong(), anyLong()); + Multimap updatedProfiles = captor.>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 captor = ArgumentCaptor.forClass(Multimap.class); + verify(builtInQualityProfilesNotification).onChange(captor.capture(), anyLong(), anyLong()); + Multimap updatedProfiles = captor.>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 consumer) { + QProfileDto builtInQProfileDto = db.qualityProfiles().insert(organization, consumer); + db.commit(); + return builtInQProfileDto; + } + private static String newLanguageKey() { return randomAlphanumeric(20).toLowerCase(); } diff --git a/tests/src/test/java/org/sonarqube/tests/qualityProfile/BuiltInQualityProfilesNotificationTest.java b/tests/src/test/java/org/sonarqube/tests/qualityProfile/BuiltInQualityProfilesNotificationTest.java index d11f252ddad..92a7b897789 100644 --- a/tests/src/test/java/org/sonarqube/tests/qualityProfile/BuiltInQualityProfilesNotificationTest.java +++ b/tests/src/test/java/org/sonarqube/tests/qualityProfile/BuiltInQualityProfilesNotificationTest.java @@ -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);