diff options
37 files changed, 1195 insertions, 574 deletions
diff --git a/it/it-tests/src/test/java/it/qualityProfile/CustomQualityProfilesTest.java b/it/it-tests/src/test/java/it/qualityProfile/CustomQualityProfilesTest.java index 2de56eaf33b..adf96f5fa1c 100644 --- a/it/it-tests/src/test/java/it/qualityProfile/CustomQualityProfilesTest.java +++ b/it/it-tests/src/test/java/it/qualityProfile/CustomQualityProfilesTest.java @@ -46,6 +46,7 @@ import util.QualityProfileRule; import util.QualityProfileSupport; import util.user.UserRule; +import static org.apache.commons.lang3.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; import static util.ItUtils.expectForbiddenError; import static util.ItUtils.expectMissingError; @@ -254,14 +255,15 @@ public class CustomQualityProfilesTest { QualityProfileSupport adminProfiles = profiles.as(user.getLogin(), A_PASSWORD); - String projectKey = "test-project"; + String projectKey = randomAlphanumeric(10); + String projectName = randomAlphanumeric(10); orchestrator.executeBuild( SonarScanner.create(projectDir("shared/xoo-sample"), "sonar.login", user.getLogin(), "sonar.password", A_PASSWORD, "sonar.organization", org.getKey()) .setProjectKey(projectKey) - .setProjectName("my-project") + .setProjectName(projectName) ); QualityProfiles.SearchWsResponse.QualityProfile defaultProfile = getProfile(org, p -> "xoo".equals(p.getLanguage()) && p.getIsDefault()); @@ -276,7 +278,7 @@ public class CustomQualityProfilesTest { "sonar.password", A_PASSWORD, "sonar.organization", org.getKey()) .setProjectKey(projectKey) - .setProjectName("my-project") + .setProjectName(projectName) ); assertThatQualityProfileIsUsedFor(projectKey, newXooProfile.getKey()); @@ -286,8 +288,8 @@ public class CustomQualityProfilesTest { public void analysis_must_use_associated_profile() { Organization org = organizations.create(); User user = users.createAdministrator(org, A_PASSWORD); - String projectKey = "test-project"; - String projectName = "my-project"; + String projectKey = randomAlphanumeric(10); + String projectName = randomAlphanumeric(10); QualityProfileSupport adminProfiles = profiles.as(user.getLogin(), A_PASSWORD); QualityProfile newXooProfile = adminProfiles.createXooProfile(org); diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java index 71002640fad..e62d881de6d 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java @@ -71,6 +71,10 @@ public class ActiveRuleDao implements Dao { return selectByProfileUuid(dbSession, profile.getKee()); } + public List<ActiveRuleDto> selectByRuleProfile(DbSession dbSession, RulesProfileDto ruleProfileDto) { + return mapper(dbSession).selectByRuleProfileUuid(ruleProfileDto.getKee()); + } + public ActiveRuleDto insert(DbSession dbSession, ActiveRuleDto item) { Preconditions.checkArgument(item.getProfileId() != null, QUALITY_PROFILE_IS_NOT_PERSISTED); Preconditions.checkArgument(item.getRuleId() != null, RULE_IS_NOT_PERSISTED); @@ -101,6 +105,11 @@ public class ActiveRuleDao implements Dao { DatabaseUtils.executeLargeUpdates(rulesProfileUuids, mapper::deleteByRuleProfileUuids); } + public void deleteByIds(DbSession dbSession, List<Integer> activeRuleIds) { + ActiveRuleMapper mapper = mapper(dbSession); + DatabaseUtils.executeLargeUpdates(activeRuleIds, mapper::deleteByIds); + } + public void deleteParametersByRuleProfileUuids(DbSession dbSession, Collection<String> rulesProfileUuids) { ActiveRuleMapper mapper = mapper(dbSession); DatabaseUtils.executeLargeUpdates(rulesProfileUuids, mapper::deleteParametersByRuleProfileUuids); @@ -152,6 +161,11 @@ public class ActiveRuleDao implements Dao { } } + public void deleteParamsByActiveRuleIds(DbSession dbSession, List<Integer> activeRuleIds) { + ActiveRuleMapper mapper = mapper(dbSession); + DatabaseUtils.executeLargeUpdates(activeRuleIds, mapper::deleteParamsByActiveRuleIds); + } + /** * Active rule on removed rule are NOT taken into account */ @@ -176,5 +190,4 @@ public class ActiveRuleDao implements Dao { private static ActiveRuleMapper mapper(DbSession dbSession) { return dbSession.getMapper(ActiveRuleMapper.class); } - } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java index 9e58d80c3d9..5c384a20ccd 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java @@ -36,6 +36,8 @@ public interface ActiveRuleMapper { void deleteByRuleProfileUuids(@Param("rulesProfileUuids") Collection<String> rulesProfileUuids); + void deleteByIds(@Param("ids") Collection<Integer> ids); + @CheckForNull ActiveRuleDto selectByKey(@Param("ruleProfileUuid") String ruleProfileUuid, @Param("repository") String repository, @Param("rule") String rule); @@ -49,6 +51,8 @@ public interface ActiveRuleMapper { List<OrgActiveRuleDto> selectByProfileUuid(String uuid); + List<ActiveRuleDto> selectByRuleProfileUuid(@Param("ruleProfileUuid") String uuid); + void insertParameter(ActiveRuleParamDto dto); void updateParameter(ActiveRuleParamDto dto); @@ -59,6 +63,8 @@ public interface ActiveRuleMapper { void deleteParameter(int activeRuleParamId); + void deleteParamsByActiveRuleIds(@Param("activeRuleIds") Collection<Integer> activeRuleIds); + List<ActiveRuleParamDto> selectParamsByActiveRuleId(int activeRuleId); List<ActiveRuleParamDto> selectParamsByActiveRuleIds(@Param("ids") List<Integer> ids); diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileDao.java index 14b53010472..6f82c926bd1 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileDao.java @@ -109,9 +109,15 @@ public class QualityProfileDao implements Dao { } } + public void update(DbSession dbSession, RulesProfileDto rulesProfile) { + QualityProfileMapper mapper = mapper(dbSession); + long now = system.now(); + mapper.updateRuleProfile(rulesProfile, new Date(now)); + } + private void doUpdate(QualityProfileMapper mapper, QProfileDto profile, long now) { - mapper.updateRuleProfile(profile, new Date(now)); - mapper.updateOrgQProfile(profile, now); + mapper.updateRuleProfile(RulesProfileDto.from(profile), new Date(now)); + mapper.updateOrgQProfile(OrgQProfileDto.from(profile), now); } public List<QProfileDto> selectDefaultProfiles(DbSession dbSession, OrganizationDto organization, Collection<String> languages) { @@ -220,6 +226,10 @@ public class QualityProfileDao implements Dao { DatabaseUtils.executeLargeUpdates(rulesProfileUuids, mapper::deleteRuleProfilesByUuids); } + public List<QProfileDto> selectChildrenOfBuiltInRulesProfile(DbSession dbSession, RulesProfileDto rulesProfile) { + return mapper(dbSession).selectChildrenOfBuiltInRulesProfile(rulesProfile.getKee()); + } + private static String sqlQueryString(@Nullable String query) { if (query == null) { return "%"; diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileMapper.java index 9121d9a40b4..758786dc15c 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualityprofile/QualityProfileMapper.java @@ -32,9 +32,9 @@ public interface QualityProfileMapper { void insertRuleProfile(@Param("dto") RulesProfileDto dto, @Param("now") Date now); - void updateRuleProfile(@Param("dto") QProfileDto dto, @Param("now") Date now); + void updateRuleProfile(@Param("dto") RulesProfileDto dto, @Param("now") Date now); - void updateOrgQProfile(@Param("dto") QProfileDto dto, @Param("now") long now); + void updateOrgQProfile(@Param("dto") OrgQProfileDto dto, @Param("now") long now); void deleteRuleProfilesByUuids(@Param("uuids") Collection<String> uuids); @@ -121,4 +121,6 @@ public interface QualityProfileMapper { List<String> selectUuidsOfCustomRuleProfiles(@Param("language") String language, @Param("name") String name); void renameRuleProfiles(@Param("newName") String newName, @Param("updatedAt") Date updatedAt, @Param("uuids") Collection<String> uuids); + + List<QProfileDto> selectChildrenOfBuiltInRulesProfile(@Param("rulesProfileUuid") String rulesProfileUuid); } diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml index 9948c028ebe..840467e7e8b 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml @@ -81,6 +81,13 @@ ) </delete> + <delete id="deleteByIds" parameterType="Integer"> + delete from active_rules + where + id in + <foreach collection="ids" open="(" close=")" item="id" separator=",">#{id, jdbcType=INTEGER}</foreach> + </delete> + <select id="selectByKey" parameterType="map" resultType="ActiveRule"> select <include refid="activeRuleColumns"/> @@ -116,6 +123,15 @@ where oqp.uuid = #{id, jdbcType=VARCHAR} </select> + <select id="selectByRuleProfileUuid" parameterType="string" resultType="org.sonar.db.qualityprofile.ActiveRuleDto"> + select + <include refid="activeRuleColumns"/> + from active_rules a + <include refid="activeRuleKeyJoin"/> + where + rp.kee = #{ruleProfileUuid, jdbcType=VARCHAR} + </select> + <select id="selectByRuleId" parameterType="map" resultType="org.sonar.db.qualityprofile.OrgActiveRuleDto"> select <include refid="orgActiveRuleColumns"/> @@ -203,6 +219,13 @@ DELETE FROM active_rule_parameters WHERE id=#{id, jdbcType=BIGINT} </delete> + <delete id="deleteParamsByActiveRuleIds" parameterType="Integer"> + delete from active_rule_parameters + where + active_rule_id in + <foreach collection="activeRuleIds" open="(" close=")" item="activeRuleId" separator=",">#{activeRuleId, jdbcType=INTEGER}</foreach> + </delete> + <select id="selectParamsByActiveRuleId" parameterType="Integer" resultType="ActiveRuleParam"> select <include refid="activeRuleParamColumns"/> diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/QualityProfileMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/QualityProfileMapper.xml index 35b8cbdfcb0..1f8add3639b 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/QualityProfileMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/qualityprofile/QualityProfileMapper.xml @@ -77,18 +77,18 @@ rules_updated_at = #{dto.rulesUpdatedAt, jdbcType=VARCHAR}, is_built_in = #{dto.isBuiltIn, jdbcType=BOOLEAN} where - kee = #{dto.rulesProfileUuid, jdbcType=VARCHAR} + kee = #{dto.kee, jdbcType=VARCHAR} </update> <update id="updateOrgQProfile" parameterType="map"> update org_qprofiles set - parent_uuid = #{dto.parentKee, jdbcType=VARCHAR}, + parent_uuid = #{dto.parentUuid, jdbcType=VARCHAR}, last_used = #{dto.lastUsed, jdbcType=BIGINT}, user_updated_at = #{dto.userUpdatedAt, jdbcType=BIGINT}, updated_at = #{now, jdbcType=BIGINT} where - uuid = #{dto.kee, jdbcType=VARCHAR} + uuid = #{dto.uuid, jdbcType=VARCHAR} </update> <delete id="deleteRuleProfilesByUuids" parameterType="String"> @@ -335,5 +335,18 @@ where kee in <foreach collection="uuids" open="(" close=")" item="uuid" separator=",">#{uuid, jdbcType=VARCHAR}</foreach> </update> + + <select id="selectChildrenOfBuiltInRulesProfile" parameterType="string" resultType="org.sonar.db.qualityprofile.QProfileDto"> + select + <include refid="qProfileColumns"/> + from org_qprofiles oqp + inner join rules_profiles rp on oqp.rules_profile_uuid = rp.kee + inner join org_qprofiles parentoqp on parentoqp.uuid = oqp.parent_uuid + inner join rules_profiles parentrp on parentoqp.rules_profile_uuid = parentrp.kee + where + parentrp.kee = #{rulesProfileUuid, jdbcType=VARCHAR} + and parentrp.is_built_in = ${_true} + and oqp.parent_uuid is not null + </select> </mapper> diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java index fd293237cda..62c4c989d77 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java @@ -151,7 +151,7 @@ public class ActiveRuleDaoTest { } @Test - public void selectByProfileUuid_ignores_removed_rules() throws Exception { + public void selectByProfileUuid_ignores_removed_rules() { ActiveRuleDto activeRule = createFor(profile1, removedRule).setSeverity(BLOCKER); underTest.insert(dbSession, activeRule); @@ -159,6 +159,22 @@ public class ActiveRuleDaoTest { } @Test + public void selectByRuleProfileUuid() { + ActiveRuleDto activeRule1 = createFor(profile1, rule1).setSeverity(BLOCKER); + ActiveRuleDto activeRule2 = createFor(profile1, rule2).setSeverity(MAJOR); + underTest.insert(dbSession, activeRule1); + underTest.insert(dbSession, activeRule2); + + List<ActiveRuleDto> result = underTest.selectByRuleProfile(dbSession, RulesProfileDto.from(profile1)); + assertThat(result) + .hasSize(2) + .extracting(ActiveRuleDto::getProfileId, ActiveRuleDto::getRuleKey, ActiveRuleDto::getSeverityString) + .containsOnly(tuple(profile1.getId(), rule1.getKey(), BLOCKER), tuple(profile1.getId(), rule2.getKey(), MAJOR)); + + assertThat(underTest.selectByProfile(dbSession, profile2)).isEmpty(); + } + + @Test public void insert() { ActiveRuleDto activeRule = createFor(profile1, rule1) .setSeverity(BLOCKER) @@ -297,6 +313,29 @@ public class ActiveRuleDaoTest { assertThat(db.countRowsOfTable(dbSession, "active_rules")).isEqualTo(1); } + @Test + public void deleteByIds() { + ActiveRuleDto ar1 = underTest.insert(dbSession, newRow(profile1, rule1)); + ActiveRuleDto ar2 = underTest.insert(dbSession, newRow(profile1, rule2)); + ActiveRuleDto ar3 = underTest.insert(dbSession, newRow(profile2, rule1)); + + underTest.deleteByIds(dbSession, asList(ar1.getId(), ar3.getId())); + + assertThat(db.countRowsOfTable(dbSession, "active_rules")).isEqualTo(1); + assertThat(underTest.selectByProfile(dbSession, profile1)) + .extracting(ActiveRuleDto::getId) + .containsExactly(ar2.getId()); + } + + @Test + public void deleteByIds_does_nothing_if_empty_list_of_ids() { + underTest.insert(dbSession, newRow(profile1, rule1)); + + underTest.deleteByIds(dbSession, emptyList()); + + assertThat(db.countRowsOfTable(dbSession, "active_rules")).isEqualTo(1); + } + private static ActiveRuleDto newRow(QProfileDto profile, RuleDefinitionDto rule) { return createFor(profile, rule).setSeverity(BLOCKER); } @@ -461,6 +500,22 @@ public class ActiveRuleDaoTest { } @Test + public void deleteParamsByActiveRuleIds() { + ActiveRuleDto ar1 = underTest.insert(dbSession, newRow(profile1, rule1)); + ActiveRuleParamDto param = ActiveRuleParamDto.createFor(rule1Param1).setValue("foo"); + underTest.insertParam(dbSession, ar1, param); + + ActiveRuleDto ar2 = underTest.insert(dbSession, newRow(profile1, rule2)); + ActiveRuleParamDto param2 = ActiveRuleParamDto.createFor(rule2Param1).setValue("bar"); + underTest.insertParam(dbSession, ar2, param2); + + underTest.deleteParamsByActiveRuleIds(dbSession, asList(ar1.getId())); + + assertThat(underTest.selectParamsByActiveRuleId(dbSession, ar1.getId())).hasSize(0); + assertThat(underTest.selectParamsByActiveRuleId(dbSession, ar2.getId())).hasSize(1); + } + + @Test public void test_countActiveRulesByProfileKey_for_a_specified_organization() { db.qualityProfiles().activateRule(profile1, rule1); db.qualityProfiles().activateRule(profile1, rule2); diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileDaoTest.java index 3fb9e58832a..48ae7bd63ef 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileDaoTest.java @@ -49,7 +49,9 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.sonar.core.util.stream.MoreCollectors.toList; import static org.sonar.db.qualityprofile.QualityProfileTesting.newQualityProfileDto; +import static org.sonar.db.qualityprofile.RulesProfileDto.from; public class QualityProfileDaoTest { @@ -721,4 +723,35 @@ public class QualityProfileDaoTest { return Arrays.asList(dto1, dto2); } + @Test + public void selectChildrenOfBuiltInRulesProfile_must_return_only_inherited_profiles() { + OrganizationDto org1 = db.organizations().insert(); + OrganizationDto org2 = db.organizations().insert(); + OrganizationDto org3 = db.organizations().insert(); + + QProfileDto builtInProfile = db.qualityProfiles().insert(org1, p -> p.setIsBuiltIn(true).setLanguage("java").setName("foo")); + QProfileDto javaProfileOrg2 = db.qualityProfiles().insert(org2, p -> p.setIsBuiltIn(false).setLanguage("java").setName("foo")); + QProfileDto inheritedJavaProfileOrg2 = db.qualityProfiles().insert(org2, p -> p.setIsBuiltIn(false).setLanguage("java").setName("foo").setParentKee(builtInProfile.getKee())); + QProfileDto differentLanguage = db.qualityProfiles().insert(org2, p -> p.setIsBuiltIn(false).setLanguage("cobol").setName("foo")); + QProfileDto differentName = db.qualityProfiles().insert(org2, p -> p.setIsBuiltIn(false).setLanguage("java").setName("bar")); + QProfileDto javaProfileOrg3 = db.qualityProfiles().insert(org3, p -> p.setIsBuiltIn(false).setLanguage("java").setName("foo")); + QProfileDto inheritedJavaProfileOrg3 = db.qualityProfiles().insert(org3, p -> p.setIsBuiltIn(false).setLanguage("java").setName("foo").setParentKee(builtInProfile.getKee())); + + List<QProfileDto> children = db.getDbClient().qualityProfileDao().selectChildrenOfBuiltInRulesProfile(db.getSession(), from(builtInProfile)); + + assertThat(children.stream().map(qp -> qp.getId()).collect(toList())).containsExactlyInAnyOrder( + inheritedJavaProfileOrg2.getId(), inheritedJavaProfileOrg3.getId()); + } + + @Test + public void selectChildrenOfBuiltInRulesProfile_must_return_empty_list_if_not_built_in() { + OrganizationDto org = db.organizations().insert(); + + QProfileDto notBuiltInProfile = db.qualityProfiles().insert(org, p -> p.setIsBuiltIn(false).setLanguage("java").setName("foo")); + QProfileDto inheritedProfile = db.qualityProfiles().insert(org, p -> p.setIsBuiltIn(false).setLanguage("java").setName("foo").setParentKee(notBuiltInProfile.getKee())); + + List<QProfileDto> children = db.getDbClient().qualityProfileDao().selectChildrenOfBuiltInRulesProfile(db.getSession(), from(notBuiltInProfile)); + + assertThat(children).isEmpty(); + } } diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileTesting.java b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileTesting.java index 10958208dbc..fac9f8c8a33 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileTesting.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/qualityprofile/QualityProfileTesting.java @@ -19,8 +19,11 @@ */ package org.sonar.db.qualityprofile; +import java.util.function.Consumer; import org.sonar.core.util.Uuids; +import static java.util.Arrays.stream; +import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.apache.commons.lang.math.RandomUtils.nextLong; @@ -56,4 +59,17 @@ public class QualityProfileTesting { .setChangeType("ACTIVATED") .setLogin(randomAlphanumeric(10)); } + + /** + * Create an instance of {@link RulesProfileDto} with most of random field values. + */ + public static RulesProfileDto newRuleProfileDto(Consumer<RulesProfileDto>... populators) { + RulesProfileDto dto = new RulesProfileDto() + .setKee("uuid" + randomAlphabetic(10)) + .setName("name" + randomAlphabetic(10)) + .setLanguage("lang" + randomAlphabetic(5)) + .setIsBuiltIn(false); + stream(populators).forEach(p -> p.accept(dto)); + return dto; + } } diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/DbVersion65.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/DbVersion65.java index 0f13d2d420a..b8647194efd 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/DbVersion65.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/DbVersion65.java @@ -52,7 +52,6 @@ public class DbVersion65 implements DbVersion { .add(1723, "Populate table qprofiles", PopulateOrgQProfiles.class) .add(1724, "Drop columns organization_uuid and parent_kee from rules_profiles", DropOrgColumnsFromRulesProfiles.class) .add(1725, "Mark rules_profiles.is_built_in to true for default organization", SetRulesProfilesIsBuiltInToTrueForDefaultOrganization.class) - .add(1726, "Update OrgQProfiles to point to built-in profiles", UpdateOrgQProfilesToPointToBuiltInProfiles.class) - .add(1727, "Delete orphans rules_profiles table and associated tables", DeleteOrphansFromRulesProfiles.class); + .add(1726, "Update OrgQProfiles to point to built-in profiles", UpdateOrgQProfilesToPointToBuiltInProfiles.class); } } diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/DeleteOrphansFromRulesProfiles.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/DeleteOrphansFromRulesProfiles.java index 133e6de6e51..a63fd8fd0ba 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/DeleteOrphansFromRulesProfiles.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v65/DeleteOrphansFromRulesProfiles.java @@ -21,90 +21,32 @@ package org.sonar.server.platform.db.migration.version.v65; import java.sql.SQLException; +import org.sonar.api.utils.log.Logger; +import org.sonar.api.utils.log.Loggers; import org.sonar.db.Database; import org.sonar.server.platform.db.migration.step.DataChange; -import org.sonar.server.platform.db.migration.step.MassUpdate; public class DeleteOrphansFromRulesProfiles extends DataChange { + private static final Logger LOG = Loggers.get(DeleteOrphansFromRulesProfiles.class); + public DeleteOrphansFromRulesProfiles(Database db) { super(db); } @Override protected void execute(Context context) throws SQLException { - deleteOrphansFromRulesProfiles(context); - deleteOrphansFromActiveRules(context); - deleteOrphansFromActiveRuleParameters(context); - deleteOrphansFromQProfileChanges(context); - } - - private static void deleteOrphansFromRulesProfiles(Context context) throws SQLException { - MassUpdate massUpdate = context.prepareMassUpdate() - .rowPluralName("rules profiles"); - - massUpdate.select("select rp.kee " + - " from rules_profiles rp" + - " where not exists " + - " ( select 1 from org_qprofiles oqp where oqp.rules_profile_uuid = rp.kee )"); - - massUpdate.update("delete from rules_profiles where kee = ?") - .execute((row, update) -> { - String kee = row.getString(1); - update.setString(1, kee); - return true; - }); - } - - private static void deleteOrphansFromActiveRules(Context context) throws SQLException { - MassUpdate massUpdate = context.prepareMassUpdate() - .rowPluralName("active rules"); - - massUpdate.select("select ar.id " + - " from active_rules ar " + - " where not exists " + - " ( select 1 from rules_profiles rp where ar.profile_id = rp.id )"); - - massUpdate.update("delete from active_rules where id = ?") - .execute((row, update) -> { - int id = row.getInt(1); - update.setInt(1, id); - return true; - }); - } - - private static void deleteOrphansFromActiveRuleParameters(Context context) throws SQLException { - MassUpdate massUpdate = context.prepareMassUpdate() - .rowPluralName("active rule parameters"); - - massUpdate.select("select arp.id " + - " from active_rule_parameters arp " + - " where not exists " + - " ( select 1 from active_rules ar where ar.id = arp.active_rule_id )"); - - massUpdate.update("delete from active_rule_parameters where id = ?") - .execute((row, update) -> { - int id = row.getInt(1); - update.setInt(1, id); - return true; - }); + execute(context, "rules_profiles", "delete from rules_profiles where not exists( select 1 from org_qprofiles oqp where oqp.rules_profile_uuid = kee)"); + execute(context, "active_rules", "delete from active_rules where not exists ( select 1 from rules_profiles rp where rp.id = profile_id)"); + execute(context, "active_rule_parameters", "delete from active_rule_parameters where not exists ( select 1 from active_rules ar where ar.id = active_rule_id)"); + execute(context, "qprofile_changes", "delete from qprofile_changes where not exists ( select 1 from rules_profiles rp where rp.kee = qprofile_key)"); } - private static void deleteOrphansFromQProfileChanges(Context context) throws SQLException { - MassUpdate massUpdate = context.prepareMassUpdate() - .rowPluralName("qprofile changes"); - - massUpdate.select("select qpc.kee " + - " from qprofile_changes qpc" + - " where not exists " + - " ( select 1 from rules_profiles rp where qpc.qprofile_key = rp.kee )"); - - massUpdate.update("delete from qprofile_changes where kee = ?") - .execute((row, update) -> { - String kee = row.getString(1); - update.setString(1, kee); - return true; - }); + private void execute(Context context, String tableName, String sql) throws SQLException { + LOG.info("Deleting orphans from " + tableName); + context + .prepareUpsert(sql) + .execute() + .commit(); } - } diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v65/DbVersion65Test.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v65/DbVersion65Test.java index ff124e18248..551158fabc0 100644 --- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v65/DbVersion65Test.java +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v65/DbVersion65Test.java @@ -35,6 +35,6 @@ public class DbVersion65Test { @Test public void verify_migration_count() { - verifyMigrationCount(underTest, 28); + verifyMigrationCount(underTest, 27); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java index ff7a5e5633e..d627e3c40ab 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java @@ -27,6 +27,7 @@ import org.sonar.server.platform.web.RegisterServletFilters; import org.sonar.server.qualitygate.RegisterQualityGates; import org.sonar.server.qualityprofile.BuiltInQProfileInsertImpl; import org.sonar.server.qualityprofile.BuiltInQProfileLoader; +import org.sonar.server.qualityprofile.BuiltInQProfileUpdateImpl; import org.sonar.server.qualityprofile.RegisterQualityProfiles; import org.sonar.server.rule.RegisterRules; import org.sonar.server.rule.WebServerRuleFinder; @@ -58,6 +59,7 @@ public class PlatformLevelStartup extends PlatformLevel { add(BuiltInQProfileLoader.class); addIfStartupLeader( BuiltInQProfileInsertImpl.class, + BuiltInQProfileUpdateImpl.class, RegisterQualityProfiles.class, RegisterPermissionTemplates.class, RenameDeprecatedPropertyKeys.class, diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdate.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdate.java new file mode 100644 index 00000000000..885833d9d37 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdate.java @@ -0,0 +1,31 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.qualityprofile; + +import org.sonar.db.DbSession; +import org.sonar.db.qualityprofile.RulesProfileDto; + +public interface BuiltInQProfileUpdate { + /** + * Persist a built-in profile and associate it to all existing organizations. + * Db sessions are committed. + */ + void update(DbSession dbSession, BuiltInQProfile builtInQProfile, RulesProfileDto ruleProfile); +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImpl.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImpl.java new file mode 100644 index 00000000000..222b5f1b736 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImpl.java @@ -0,0 +1,77 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.qualityprofile; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.ActiveRuleParam; +import org.sonar.core.util.stream.MoreCollectors; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.qualityprofile.ActiveRuleDto; +import org.sonar.db.qualityprofile.RulesProfileDto; +import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; + +public class BuiltInQProfileUpdateImpl implements BuiltInQProfileUpdate { + + private final DbClient dbClient; + private final RuleActivator ruleActivator; + private final ActiveRuleIndexer activeRuleIndexer; + + public BuiltInQProfileUpdateImpl(DbClient dbClient, RuleActivator ruleActivator, ActiveRuleIndexer activeRuleIndexer) { + this.dbClient = dbClient; + this.ruleActivator = ruleActivator; + this.activeRuleIndexer = activeRuleIndexer; + } + + public void update(DbSession dbSession, BuiltInQProfile builtIn, RulesProfileDto ruleProfile) { + // Keep reference to all the activated rules before update + Set<RuleKey> toBeDeactivated = dbClient.activeRuleDao().selectByRuleProfile(dbSession, ruleProfile) + .stream() + .map(ActiveRuleDto::getRuleKey) + .collect(MoreCollectors.toHashSet()); + + List<ActiveRuleChange> changes = new ArrayList<>(); + builtIn.getActiveRules().forEach(ar -> { + RuleActivation activation = convert(ar); + toBeDeactivated.remove(activation.getRuleKey()); + changes.addAll(ruleActivator.activateOnBuiltInRulesProfile(dbSession, activation, ruleProfile)); + }); + + // these rules are not part of the built-in profile anymore + toBeDeactivated.forEach(ruleKey -> { + changes.addAll(ruleActivator.deactivateOnBuiltInRulesProfile(dbSession, ruleProfile, ruleKey, false)); + }); + + dbSession.commit(); + activeRuleIndexer.indexChanges(dbSession, changes); + } + + private static RuleActivation convert(org.sonar.api.rules.ActiveRule ar) { + String severity = ar.getSeverity() != null ? ar.getSeverity().name() : null; + Map<String, String> params = ar.getActiveRuleParams().stream() + .collect(MoreCollectors.uniqueIndex(ActiveRuleParam::getKey, ActiveRuleParam::getValue)); + return RuleActivation.create(ar.getRule().ruleKey(), severity, params); + } + +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java index 3623aac1a65..b8173696254 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java @@ -201,10 +201,7 @@ public class QProfileBackuperImpl implements QProfileBackuper { duplicatedKeys.add(ruleKey); } activatedKeys.add(ruleKey); - RuleActivation activation = new RuleActivation(ruleKey); - activation.setSeverity(severity); - activation.setParameters(parameters); - activations.add(activation); + activations.add(RuleActivation.create(ruleKey, severity, parameters)); } if (!duplicatedKeys.isEmpty()) { throw new IllegalArgumentException("The quality profile cannot be restored as it contains duplicates for the following rules: " + diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileExporters.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileExporters.java index 2ba5204c4a9..ea89de0c9db 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileExporters.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileExporters.java @@ -29,17 +29,20 @@ import java.io.Writer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; +import java.util.Map; import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.StringUtils; import org.sonar.api.profiles.ProfileExporter; import org.sonar.api.profiles.ProfileImporter; import org.sonar.api.profiles.RulesProfile; +import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.ActiveRuleParam; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; import org.sonar.api.rules.RulePriority; import org.sonar.api.server.ServerSide; import org.sonar.api.utils.ValidationMessages; +import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.qualityprofile.ActiveRuleDto; @@ -175,12 +178,11 @@ public class QProfileExporters { } private static RuleActivation toRuleActivation(org.sonar.api.rules.ActiveRule activeRule) { - RuleActivation ruleActivation = new RuleActivation(activeRule.getRule().ruleKey()); - ruleActivation.setSeverity(activeRule.getSeverity().name()); - for (ActiveRuleParam activeRuleParam : activeRule.getActiveRuleParams()) { - ruleActivation.setParameter(activeRuleParam.getKey(), activeRuleParam.getValue()); - } - return ruleActivation; + RuleKey ruleKey = activeRule.getRule().ruleKey(); + String severity = activeRule.getSeverity().name(); + Map<String, String> params = activeRule.getActiveRuleParams().stream() + .collect(MoreCollectors.uniqueIndex(ActiveRuleParam::getKey, ActiveRuleParam::getValue)); + return RuleActivation.create(ruleKey, severity, params); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileName.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileName.java index 0a0a084138a..e32b5fc3b28 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileName.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileName.java @@ -66,6 +66,6 @@ public class QProfileName { @Override public String toString() { - return String.format("{lang=%s, name=%s}", lang, name); + return String.format("%s/%s", lang, name); } } 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 084c7348e69..c6b9593f13b 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 @@ -21,7 +21,7 @@ package org.sonar.server.qualityprofile; import java.util.Collection; import java.util.List; -import java.util.Set; +import java.util.Map; import org.sonar.api.server.ServerSide; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; @@ -29,6 +29,7 @@ import org.sonar.api.utils.log.Profiler; import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; +import org.sonar.db.qualityprofile.RulesProfileDto; import static java.lang.String.format; @@ -43,12 +44,14 @@ public class RegisterQualityProfiles { private final BuiltInQProfileRepository builtInQProfileRepository; private final DbClient dbClient; private final BuiltInQProfileInsert builtInQProfileInsert; + private final BuiltInQProfileUpdate builtInQProfileUpdate; public RegisterQualityProfiles(BuiltInQProfileRepository builtInQProfileRepository, - DbClient dbClient, BuiltInQProfileInsert builtInQProfileInsert) { + DbClient dbClient, BuiltInQProfileInsert builtInQProfileInsert, BuiltInQProfileUpdate builtInQProfileUpdate) { this.builtInQProfileRepository = builtInQProfileRepository; this.dbClient = dbClient; this.builtInQProfileInsert = builtInQProfileInsert; + this.builtInQProfileUpdate = builtInQProfileUpdate; } public void start() { @@ -61,23 +64,37 @@ public class RegisterQualityProfiles { try (DbSession dbSession = dbClient.openSession(false); DbSession batchDbSession = dbClient.openSession(true)) { - Set<QProfileName> namesExistingInDb = dbClient.qualityProfileDao().selectBuiltInRulesProfiles(dbSession).stream() - .map(dto -> new QProfileName(dto.getLanguage(), dto.getName())) - .collect(MoreCollectors.toSet()); + Map<QProfileName, RulesProfileDto> persistedRuleProfiles = loadPersistedProfiles(dbSession); - builtInQProfiles.stream() - .filter(p -> !namesExistingInDb.contains(p.getQProfileName())) - .forEach(profile -> register(dbSession, batchDbSession, profile)); + builtInQProfiles.forEach(builtIn -> { + RulesProfileDto ruleProfile = persistedRuleProfiles.get(builtIn.getQProfileName()); + if (ruleProfile == null) { + register(dbSession, batchDbSession, builtIn); + } else { + update(dbSession, builtIn, ruleProfile); + } + }); } profiler.stopDebug(); } - private void register(DbSession dbSession, DbSession batchDbSession, BuiltInQProfile builtInProfile) { - LOGGER.info("Register profile {}", builtInProfile.getQProfileName()); + private Map<QProfileName, RulesProfileDto> loadPersistedProfiles(DbSession dbSession) { + return dbClient.qualityProfileDao().selectBuiltInRulesProfiles(dbSession).stream() + .collect(MoreCollectors.uniqueIndex(rp -> new QProfileName(rp.getLanguage(), rp.getName()))); + } + + private void register(DbSession dbSession, DbSession batchDbSession, BuiltInQProfile builtIn) { + LOGGER.info("Register profile {}", builtIn.getQProfileName()); + + renameOutdatedProfiles(dbSession, builtIn); + + builtInQProfileInsert.create(dbSession, batchDbSession, builtIn); + } - renameOutdatedProfiles(dbSession, builtInProfile); + private void update(DbSession dbSession, BuiltInQProfile builtIn, RulesProfileDto ruleProfile) { + LOGGER.info("Update profile {}", builtIn.getQProfileName()); - builtInQProfileInsert.create(dbSession, batchDbSession, builtInProfile); + builtInQProfileUpdate.update(dbSession, builtIn, ruleProfile); } /** diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java index abfc56f1ee4..0fbe9f7695d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java @@ -20,57 +20,46 @@ package org.sonar.server.qualityprofile; import com.google.common.base.Strings; -import com.google.common.collect.Maps; -import org.sonar.api.rule.RuleKey; -import org.sonar.api.rule.Severity; - +import java.util.HashMap; +import java.util.Map; import javax.annotation.CheckForNull; import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.rule.Severity; -import java.util.Map; - +@Immutable public class RuleActivation { private final RuleKey ruleKey; - private final Map<String, String> parameters; - private String severity = null; - private boolean cascade = false; - private boolean reset = false; + private final boolean reset; + private final String severity; + private final Map<String, String> parameters = new HashMap<>(); - public RuleActivation(RuleKey ruleKey) { + private RuleActivation(RuleKey ruleKey, boolean reset, @Nullable String severity, @Nullable Map<String, String> parameters) { this.ruleKey = ruleKey; - this.parameters = Maps.newHashMap(); + this.reset = reset; + this.severity = severity; + if (severity != null && !Severity.ALL.contains(severity)) { + throw new IllegalArgumentException("Unknown severity: " + severity); + } + if (parameters != null) { + for (Map.Entry<String, String> entry : parameters.entrySet()) { + this.parameters.put(entry.getKey(), Strings.emptyToNull(entry.getValue())); + } + } } - public RuleActivation(RuleActivation other) { - this.ruleKey = other.ruleKey; - this.parameters = Maps.newHashMap(other.parameters); - this.severity = other.severity; - this.reset = other.reset; - this.cascade = other.cascade; + public static RuleActivation createReset(RuleKey ruleKey) { + return new RuleActivation(ruleKey, true, null, null); } - /** - * For internal use - */ - boolean isCascade() { - return this.cascade; + public static RuleActivation create(RuleKey ruleKey, @Nullable String severity, @Nullable Map<String, String> parameters) { + return new RuleActivation(ruleKey, false, severity, parameters); } - /** - * For internal use - */ - RuleActivation setCascade(boolean b) { - this.cascade = b; - return this; - } - - public RuleActivation setSeverity(@Nullable String s) { - if (s != null && !Severity.ALL.contains(s)) { - throw new IllegalArgumentException("Unknown severity: " + s); - } - this.severity = s; - return this; + public static RuleActivation create(RuleKey ruleKey) { + return create(ruleKey, null, null); } /** @@ -81,34 +70,20 @@ public class RuleActivation { return severity; } - public RuleActivation setParameter(String key, @Nullable String value) { - String sanitizedValue = Strings.emptyToNull(value); - parameters.put(key, sanitizedValue); - return this; - } - - public RuleActivation setParameters(Map<String, String> m) { - parameters.clear(); - for (Map.Entry<String, String> entry : m.entrySet()) { - setParameter(entry.getKey(), entry.getValue()); - } - return this; - } - public RuleKey getRuleKey() { return ruleKey; } - public Map<String, String> getParameters() { - return parameters; + @CheckForNull + public String getParameter(String key) { + return parameters.get(key); } - public boolean isReset() { - return reset; + public boolean hasParameter(String key) { + return parameters.containsKey(key); } - public RuleActivation setReset(boolean b) { - this.reset = b; - return this; + public boolean isReset() { + return reset; } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java index ee553bdbd57..a643716fc38 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java @@ -35,8 +35,10 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.qualityprofile.ActiveRuleDao; import org.sonar.db.qualityprofile.ActiveRuleDto; +import org.sonar.db.qualityprofile.ActiveRuleKey; import org.sonar.db.qualityprofile.ActiveRuleParamDto; import org.sonar.db.qualityprofile.QProfileDto; +import org.sonar.db.qualityprofile.RulesProfileDto; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleParamDto; import org.sonar.server.exceptions.BadRequestException; @@ -46,7 +48,7 @@ import org.sonar.server.rule.index.RuleQuery; import org.sonar.server.user.UserSession; import org.sonar.server.util.TypeValidations; -import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.base.Preconditions.checkArgument; import static org.sonar.server.ws.WsUtils.checkRequest; /** @@ -75,8 +77,14 @@ public class RuleActivator { this.userSession = userSession; } - public List<ActiveRuleChange> activate(DbSession dbSession, RuleActivation activation, QProfileDto profileDto) { - RuleActivatorContext context = contextFactory.create(profileDto, activation.getRuleKey(), dbSession); + public List<ActiveRuleChange> activateOnBuiltInRulesProfile(DbSession dbSession, RuleActivation activation, RulesProfileDto rulesProfile) { + checkArgument(rulesProfile.isBuiltIn(), "Rules profile must be a built-in profile: " + rulesProfile.getKee()); + RuleActivatorContext context = contextFactory.createForBuiltIn(dbSession, activation.getRuleKey(), rulesProfile); + return doActivate(dbSession, activation, context); + } + + public List<ActiveRuleChange> activate(DbSession dbSession, RuleActivation activation, QProfileDto profile) { + RuleActivatorContext context = contextFactory.create(dbSession, activation.getRuleKey(), profile, false); return doActivate(dbSession, activation, context); } @@ -95,17 +103,17 @@ public class RuleActivator { // new activation change = new ActiveRuleChange(ActiveRuleChange.Type.ACTIVATED, context.activeRuleKey()); applySeverityAndParamToChange(activation, context, change); - if (activation.isCascade() || context.isSameAsParent(change)) { + if (context.isCascade() || context.isSameAsParent(change)) { change.setInheritance(ActiveRule.Inheritance.INHERITED); } } else { // already activated - if (activation.isCascade() && activeRule.doesOverride()) { + if (context.isCascade() && activeRule.doesOverride()) { // propagating to descendants, but child profile already overrides rule -> stop propagation return changes; } change = new ActiveRuleChange(ActiveRuleChange.Type.UPDATED, context.activeRuleKey()); - if (activation.isCascade() && activeRule.getInheritance() == null) { + if (context.isCascade() && activeRule.getInheritance() == null) { // activate on child, then on parent -> mark child as overriding parent change.setInheritance(ActiveRule.Inheritance.OVERRIDES); change.setSeverity(context.currentSeverity()); @@ -113,7 +121,7 @@ public class RuleActivator { stopPropagation = true; } else { applySeverityAndParamToChange(activation, context, change); - if (!activation.isCascade() && context.parentActiveRule() != null) { + if (!context.isCascade() && context.parentActiveRule() != null) { // override rule which is already declared on parents change.setInheritance(context.isSameAsParent(change) ? ActiveRule.Inheritance.INHERITED : ActiveRule.Inheritance.OVERRIDES); } @@ -129,7 +137,7 @@ public class RuleActivator { } if (!stopPropagation) { - changes.addAll(cascadeActivation(dbSession, activation, context.profile())); + changes.addAll(cascadeActivation(dbSession, activation, context)); } if (!changes.isEmpty()) { @@ -139,12 +147,19 @@ public class RuleActivator { } private void updateProfileDates(DbSession dbSession, RuleActivatorContext context) { - QProfileDto profile = context.profile(); - profile.setRulesUpdatedAtAsDate(context.getInitDate()); - if (userSession.isLoggedIn()) { - profile.setUserUpdatedAt(context.getInitDate().getTime()); + QProfileDto profile = context.getProfile(); + if (profile != null) { + profile.setRulesUpdatedAtAsDate(context.getInitDate()); + if (userSession.isLoggedIn()) { + profile.setUserUpdatedAt(context.getInitDate().getTime()); + } + db.qualityProfileDao().update(dbSession, profile); + } else { + // built-in profile, change rules_profiles.rules_updated_at + RulesProfileDto rulesProfile = context.getRulesProfile(); + rulesProfile.setRulesUpdatedAtAsDate(context.getInitDate()); + db.qualityProfileDao().update(dbSession, rulesProfile); } - db.qualityProfileDao().update(dbSession, profile); } /** @@ -208,7 +223,7 @@ public class RuleActivator { } @CheckForNull - String firstNonNull(String... strings) { + private String firstNonNull(String... strings) { for (String s : strings) { if (s != null) { return s; @@ -217,22 +232,25 @@ public class RuleActivator { return null; } - private List<ActiveRuleChange> cascadeActivation(DbSession dbSession, RuleActivation activation, QProfileDto profile) { + private List<ActiveRuleChange> cascadeActivation(DbSession dbSession, RuleActivation activation, RuleActivatorContext context) { List<ActiveRuleChange> changes = new ArrayList<>(); // get all inherited profiles - getChildren(dbSession, profile).forEach(child -> { - RuleActivation childActivation = new RuleActivation(activation).setCascade(true); - changes.addAll(activate(dbSession, childActivation, child)); + getChildren(dbSession, context).forEach(child -> { + RuleActivatorContext childContext = contextFactory.create(dbSession, activation.getRuleKey(), child, true); + changes.addAll(doActivate(dbSession, activation, childContext)); }); return changes; } - protected List<QProfileDto> getChildren(DbSession session, QProfileDto profile) { - return db.qualityProfileDao().selectChildren(session, profile); + protected List<QProfileDto> getChildren(DbSession session, RuleActivatorContext context) { + if (context.getProfile() != null) { + return db.qualityProfileDao().selectChildren(session, context.getProfile()); + } + return db.qualityProfileDao().selectChildrenOfBuiltInRulesProfile(session, context.getRulesProfile()); } - private ActiveRuleDto persist(ActiveRuleChange change, RuleActivatorContext context, DbSession dbSession) { + private void persist(ActiveRuleChange change, RuleActivatorContext context, DbSession dbSession) { ActiveRuleDto activeRule = null; if (change.getType() == ActiveRuleChange.Type.ACTIVATED) { activeRule = doInsert(change, context, dbSession); @@ -245,13 +263,14 @@ public class RuleActivator { } change.setActiveRule(activeRule); db.qProfileChangeDao().insert(dbSession, change.toDto(userSession.getLogin())); - return activeRule; } private ActiveRuleDto doInsert(ActiveRuleChange change, RuleActivatorContext context, DbSession dbSession) { - ActiveRuleDto activeRule; ActiveRuleDao dao = db.activeRuleDao(); - activeRule = ActiveRuleDto.createFor(context.profile(), context.rule()); + ActiveRuleDto activeRule = new ActiveRuleDto(); + activeRule.setProfileId(context.getRulesProfile().getId()); + activeRule.setRuleId(context.getRule().getId()); + activeRule.setKey(ActiveRuleKey.of(context.getRulesProfile(), context.getRule().getKey())); String severity = change.getSeverity(); if (severity != null) { activeRule.setSeverity(severity); @@ -328,14 +347,20 @@ public class RuleActivator { } /** - * Deactivate a rule on a Quality profile WITHOUT committing db session, WITHOUT checking permissions, and forcing removal of inherited rules + * Deletes a rule from all Quality profiles. */ - public List<ActiveRuleChange> deactivateOfAllOrganizations(DbSession dbSession, RuleDefinitionDto rule) { + public List<ActiveRuleChange> delete(DbSession dbSession, RuleDefinitionDto rule) { List<ActiveRuleChange> changes = new ArrayList<>(); - List<ActiveRuleDto> activeRules = db.activeRuleDao().selectByRuleIdOfAllOrganizations(dbSession, rule.getId()); - for (ActiveRuleDto activeRule : activeRules) { - // FIXME changes.addAll(deactivate(dbSession, activeRule.getKey(), rule.getKey(), true)); - } + List<Integer> activeRuleIds = new ArrayList<>(); + + db.activeRuleDao().selectByRuleIdOfAllOrganizations(dbSession, rule.getId()).forEach(ar -> { + activeRuleIds.add(ar.getId()); + changes.add(new ActiveRuleChange(ActiveRuleChange.Type.DEACTIVATED, ar)); + }); + + db.activeRuleDao().deleteByIds(dbSession, activeRuleIds); + db.activeRuleDao().deleteParamsByActiveRuleIds(dbSession, activeRuleIds); + return changes; } @@ -343,25 +368,34 @@ public class RuleActivator { * @param force if true then inherited rules are deactivated */ public List<ActiveRuleChange> deactivate(DbSession dbSession, QProfileDto profile, RuleKey ruleKey, boolean force) { - return cascadeDeactivation(dbSession, profile, ruleKey, false, force); + RuleActivatorContext context = contextFactory.create(dbSession, ruleKey, profile, false); + return cascadeDeactivation(dbSession, context, ruleKey, force); } - private List<ActiveRuleChange> cascadeDeactivation(DbSession dbSession, QProfileDto profile, RuleKey ruleKey, boolean isCascade, boolean force) { + public List<ActiveRuleChange> deactivateOnBuiltInRulesProfile(DbSession dbSession, RulesProfileDto rulesProfile, RuleKey ruleKey, boolean force) { + checkArgument(rulesProfile.isBuiltIn(), "Rules profile must be a built-in profile: " + rulesProfile.getKee()); + RuleActivatorContext context = contextFactory.createForBuiltIn(dbSession, ruleKey, rulesProfile); + return cascadeDeactivation(dbSession, context, ruleKey, force); + } + + private List<ActiveRuleChange> cascadeDeactivation(DbSession dbSession, RuleActivatorContext context, RuleKey ruleKey, boolean force) { List<ActiveRuleChange> changes = new ArrayList<>(); - RuleActivatorContext context = contextFactory.create(profile, ruleKey, dbSession); ActiveRuleChange change; ActiveRuleDto activeRuleDto = context.activeRule(); if (activeRuleDto == null) { return changes; } - checkRequest(force || isCascade || activeRuleDto.getInheritance() == null, "Cannot deactivate inherited rule '%s'", ruleKey); + checkRequest(force || context.isCascade() || activeRuleDto.getInheritance() == null, "Cannot deactivate inherited rule '%s'", ruleKey); change = new ActiveRuleChange(ActiveRuleChange.Type.DEACTIVATED, activeRuleDto); changes.add(change); persist(change, context, dbSession); - // get all inherited profiles + // get all inherited profiles (they are not built-in by design) - getChildren(dbSession, profile).forEach(child -> changes.addAll(cascadeDeactivation(dbSession, child, ruleKey, true, force))); + getChildren(dbSession, context).forEach(child -> { + RuleActivatorContext childContext = contextFactory.create(dbSession, ruleKey, child, true); + changes.addAll(cascadeDeactivation(dbSession, childContext, ruleKey, force)); + }); if (!changes.isEmpty()) { updateProfileDates(dbSession, context); @@ -375,7 +409,7 @@ public class RuleActivator { if (value != null) { RuleParamType ruleParamType = RuleParamType.parse(ruleParam.getType()); if (ruleParamType.multiple()) { - List<String> values = newArrayList(Splitter.on(",").split(value)); + List<String> values = Splitter.on(",").splitToList(value); typeValidations.validate(values, ruleParamType.type(), ruleParamType.values()); } else { typeValidations.validate(value, ruleParamType.type(), ruleParamType.values()); @@ -390,8 +424,7 @@ public class RuleActivator { while (rules.hasNext()) { RuleKey ruleKey = rules.next(); try { - RuleActivation activation = new RuleActivation(ruleKey); - activation.setSeverity(severity); + RuleActivation activation = RuleActivation.create(ruleKey, severity, null); List<ActiveRuleChange> changes = activate(dbSession, activation, profile); result.addChanges(changes); if (!changes.isEmpty()) { @@ -451,7 +484,7 @@ public class RuleActivator { db.qualityProfileDao().update(dbSession, profile); for (ActiveRuleDto parentActiveRule : db.activeRuleDao().selectByProfile(dbSession, parent)) { try { - RuleActivation activation = new RuleActivation(parentActiveRule.getRuleKey()); + RuleActivation activation = RuleActivation.create(parentActiveRule.getRuleKey(), null, null); changes.addAll(activate(dbSession, activation, profile)); } catch (BadRequestException e) { // for example because rule status is REMOVED diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java index 1605b31125d..74c9a8d4a50 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java @@ -22,6 +22,7 @@ package org.sonar.server.qualityprofile; import com.google.common.collect.Maps; import java.util.Collection; import java.util.Date; +import java.util.HashMap; import java.util.Map; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -31,30 +32,61 @@ import org.sonar.db.qualityprofile.ActiveRuleDto; import org.sonar.db.qualityprofile.ActiveRuleKey; import org.sonar.db.qualityprofile.ActiveRuleParamDto; import org.sonar.db.qualityprofile.QProfileDto; +import org.sonar.db.qualityprofile.RulesProfileDto; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleParamDto; +import static com.google.common.base.Preconditions.checkArgument; import static org.sonar.server.ws.WsUtils.checkRequest; class RuleActivatorContext { + private final QProfileDto profile; + private final RulesProfileDto rulesProfile; private final Date initDate = new Date(); private RuleDefinitionDto rule; - private final Map<String, RuleParamDto> ruleParams = Maps.newHashMap(); - private QProfileDto profile; + private final Map<String, RuleParamDto> ruleParams = new HashMap<>(); private ActiveRuleDto activeRule; private ActiveRuleDto parentActiveRule; - private final Map<String, ActiveRuleParamDto> activeRuleParams = Maps.newHashMap(); - private final Map<String, ActiveRuleParamDto> parentActiveRuleParams = Maps.newHashMap(); + private final Map<String, ActiveRuleParamDto> activeRuleParams = new HashMap<>(); + private final Map<String, ActiveRuleParamDto> parentActiveRuleParams = new HashMap<>(); + private final boolean isCascade; - RuleActivatorContext() { + RuleActivatorContext(QProfileDto profile, boolean isCascade) { + this.profile = profile; + this.rulesProfile = RulesProfileDto.from(profile); + this.isCascade = isCascade; + } + + RuleActivatorContext(RulesProfileDto rulesProfile) { + checkArgument(rulesProfile.isBuiltIn(), "Rules profile must be a built-in profile: " + rulesProfile.getKee()); + this.profile = null; + this.rulesProfile = rulesProfile; + this.isCascade = false; + } + + @CheckForNull + QProfileDto getProfile() { + return profile; + } + + RulesProfileDto getRulesProfile() { + return rulesProfile; + } + + boolean isBuiltIn() { + return profile == null; + } + + boolean isCascade() { + return isCascade; } ActiveRuleKey activeRuleKey() { - return ActiveRuleKey.of(profile, rule.getKey()); + return ActiveRuleKey.of(rulesProfile, rule.getKey()); } - RuleDefinitionDto rule() { + RuleDefinitionDto getRule() { return rule; } @@ -63,6 +95,8 @@ class RuleActivatorContext { return this; } + + Date getInitDate() { return initDate; } @@ -83,15 +117,6 @@ class RuleActivatorContext { return this; } - QProfileDto profile() { - return profile; - } - - RuleActivatorContext setProfile(QProfileDto profile) { - this.profile = profile; - return this; - } - @CheckForNull ActiveRuleDto activeRule() { return activeRule; @@ -117,11 +142,11 @@ class RuleActivatorContext { if (rule.isCustomRule()) { return null; } - return request.getParameters().get(key); + return request.getParameter(key); } boolean hasRequestParamValue(RuleActivation request, String key) { - return request.getParameters().containsKey(key); + return request.hasParameter(key); } @CheckForNull @@ -231,6 +256,6 @@ class RuleActivatorContext { void verifyForActivation() { checkRequest(RuleStatus.REMOVED != rule.getStatus(), "Rule was removed: %s", rule.getKey()); checkRequest(!rule.isTemplate(), "Rule template can't be activated on a Quality profile: %s", rule.getKey()); - checkRequest(profile.getLanguage().equals(rule.getLanguage()), "Rule %s and profile %s have different languages", rule.getKey(), profile.getKee()); + checkRequest(rulesProfile.getLanguage().equals(rule.getLanguage()), "Rule %s and profile %s have different languages", rule.getKey(), profile != null ? profile.getKee() : rulesProfile.getKee()); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContextFactory.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContextFactory.java index 22ef2fa85dc..d0ea1013d8e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContextFactory.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContextFactory.java @@ -30,6 +30,7 @@ import org.sonar.db.qualityprofile.ActiveRuleDto; import org.sonar.db.qualityprofile.ActiveRuleKey; import org.sonar.db.qualityprofile.ActiveRuleParamDto; import org.sonar.db.qualityprofile.QProfileDto; +import org.sonar.db.qualityprofile.RulesProfileDto; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleParamDto; @@ -44,18 +45,27 @@ public class RuleActivatorContextFactory { this.db = db; } - RuleActivatorContext create(QProfileDto profile, RuleKey ruleKey, DbSession session) { - return create(ruleKey, session, new RuleActivatorContext().setProfile(profile)); + RuleActivatorContext createForBuiltIn(DbSession dbSession, RuleKey ruleKey, RulesProfileDto rulesProfile) { + RuleActivatorContext context = new RuleActivatorContext(rulesProfile); + return init(dbSession, ruleKey, context); } - private RuleActivatorContext create(RuleKey ruleKey, DbSession dbSession, RuleActivatorContext context) { + RuleActivatorContext create(DbSession dbSession, RuleKey ruleKey, QProfileDto profile, boolean cascade) { + RuleActivatorContext context = new RuleActivatorContext(profile, cascade); + return init(dbSession, ruleKey, context); + } + + private RuleActivatorContext init(DbSession dbSession, RuleKey ruleKey, RuleActivatorContext context) { initRule(ruleKey, context, dbSession); - initActiveRules(context.profile(), ruleKey, context, dbSession, false); - String parentKee = context.profile().getParentKee(); - if (parentKee != null) { - QProfileDto parent = getQualityProfileDto(dbSession, parentKee); - initActiveRules(parent, ruleKey, context, dbSession, true); + initActiveRules(context.getRulesProfile(), ruleKey, context, dbSession, false); + + if (context.getProfile() != null && context.getProfile().getParentKee() != null) { + QProfileDto parent = db.qualityProfileDao().selectByUuid(dbSession, context.getProfile().getParentKee()); + if (parent != null) { + initActiveRules(RulesProfileDto.from(parent), ruleKey, context, dbSession, true); + } } + return context; } @@ -68,14 +78,14 @@ public class RuleActivatorContextFactory { return ruleDefinitionDto; } - private void initActiveRules(QProfileDto profile, RuleKey ruleKey, RuleActivatorContext context, DbSession session, boolean parent) { - ActiveRuleKey key = ActiveRuleKey.of(profile, ruleKey); - Optional<ActiveRuleDto> activeRule = getActiveRule(session, key); + private void initActiveRules(RulesProfileDto rulesProfile, RuleKey ruleKey, RuleActivatorContext context, DbSession dbSession, boolean isParent) { + ActiveRuleKey key = ActiveRuleKey.of(rulesProfile, ruleKey); + Optional<ActiveRuleDto> activeRule = getActiveRule(dbSession, key); Collection<ActiveRuleParamDto> activeRuleParams = null; if (activeRule.isPresent()) { - activeRuleParams = getActiveRuleParams(session, activeRule.get()); + activeRuleParams = getActiveRuleParams(dbSession, activeRule.get()); } - if (parent) { + if (isParent) { context.setParentActiveRule(activeRule.orElse(null)); context.setParentActiveRuleParams(activeRuleParams); } else { @@ -84,10 +94,6 @@ public class RuleActivatorContextFactory { } } - QProfileDto getQualityProfileDto(DbSession session, String profileKey) { - return db.qualityProfileDao().selectByUuid(session, profileKey); - } - Optional<RuleDefinitionDto> getRule(DbSession dbSession, RuleKey ruleKey) { return Optional.ofNullable(db.ruleDao().selectDefinitionByKey(dbSession, ruleKey).orElse(null)); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java index 7ae5fbed8a7..48b4a941a93 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java @@ -20,6 +20,7 @@ package org.sonar.server.qualityprofile.ws; import java.util.List; +import java.util.Map; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.Severity; import org.sonar.api.server.ServerSide; @@ -86,7 +87,7 @@ public class ActivateRuleAction implements QProfileWsAction { activate.createParam(PARAM_PARAMS) .setDescription(format("Parameters as semi-colon list of <key>=<value>. Ignored if parameter %s is true.", PARAM_RESET)) - .setExampleValue("params=key1=v1;key2=v2"); + .setExampleValue("params=key1=v1;key2=v2"); activate.createParam(PARAM_RESET) .setDescription("Reset severity and parameters of activated rule. Set the values defined on parent profile " + @@ -96,20 +97,13 @@ public class ActivateRuleAction implements QProfileWsAction { @Override public void handle(Request request, Response response) throws Exception { - RuleKey ruleKey = readRuleKey(request); - RuleActivation activation = new RuleActivation(ruleKey); - activation.setSeverity(request.param(PARAM_SEVERITY)); - String params = request.param(PARAM_PARAMS); - if (params != null) { - activation.setParameters(KeyValueFormat.parse(params)); - } - activation.setReset(Boolean.TRUE.equals(request.paramAsBoolean(PARAM_RESET))); - String profileKey = request.mandatoryParam(PARAM_PROFILE_KEY); userSession.checkLoggedIn(); try (DbSession dbSession = dbClient.openSession(false)) { + String profileKey = request.mandatoryParam(PARAM_PROFILE_KEY); QProfileDto profile = wsSupport.getProfile(dbSession, QProfileReference.fromKey(profileKey)); wsSupport.checkPermission(dbSession, profile); wsSupport.checkNotBuiltInt(profile); + RuleActivation activation = readActivation(request); List<ActiveRuleChange> changes = ruleActivator.activate(dbSession, activation, profile); dbSession.commit(); activeRuleIndexer.indexChanges(dbSession, changes); @@ -117,7 +111,19 @@ public class ActivateRuleAction implements QProfileWsAction { response.noContent(); } - private static RuleKey readRuleKey(Request request) { - return RuleKey.parse(request.mandatoryParam(PARAM_RULE_KEY)); + private RuleActivation readActivation(Request request) { + RuleKey ruleKey = RuleKey.parse(request.mandatoryParam(PARAM_RULE_KEY)); + boolean reset = Boolean.TRUE.equals(request.paramAsBoolean(PARAM_RESET)); + if (reset) { + return RuleActivation.createReset(ruleKey); + } + String severity = request.param(PARAM_SEVERITY); + Map<String, String> params = null; + String paramsAsString = request.param(PARAM_PARAMS); + if (paramsAsString != null) { + params = KeyValueFormat.parse(paramsAsString); + } + return RuleActivation.create(ruleKey, severity, params); } + } diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java b/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java index ec1e973189f..c714e91142d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java @@ -506,7 +506,7 @@ public class RegisterRules implements Startable { // SONAR-4642 Remove active rules only when repository still exists if (repositoryKeys.contains(rule.getRepositoryKey())) { profiler.start(); - changes.addAll(ruleActivator.deactivateOfAllOrganizations(session, rule)); + changes.addAll(ruleActivator.delete(session, rule)); profiler.stopDebug(format("Remove active rule for rule %s", rule.getKey())); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java index e1fb1e43655..41932387d03 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java @@ -419,7 +419,7 @@ public class RuleIndex { private static void addActiveSeverityFacetIfNeeded(RuleQuery query, SearchOptions options, Map<String, AbstractAggregationBuilder> aggregations, StickyFacetBuilder stickyFacetBuilder) { - if (options.getFacets().contains(FACET_ACTIVE_SEVERITIES)) { + if (options.getFacets().contains(FACET_ACTIVE_SEVERITIES) && query.getQProfile() != null) { // We are building a children aggregation on active rules // so the rule filter has to be used as parent filter for active rules // from which we remove filters that concern active rules ("activation") @@ -431,12 +431,7 @@ public class RuleIndex { BoolQueryBuilder childrenFilter = boolQuery(); addTermFilter(childrenFilter, FIELD_ACTIVE_RULE_PROFILE_UUID, query.getQProfile().getRulesProfileUuid()); RuleIndex.addTermFilter(childrenFilter, FIELD_ACTIVE_RULE_INHERITANCE, query.getInheritance()); - QueryBuilder activeRuleFilter; - if (childrenFilter.hasClauses()) { - activeRuleFilter = childrenFilter.must(ruleFilter); - } else { - activeRuleFilter = ruleFilter; - } + QueryBuilder activeRuleFilter = childrenFilter.must(ruleFilter); AbstractAggregationBuilder activeSeverities = AggregationBuilders.children(FACET_ACTIVE_SEVERITIES + "_children") .childType(INDEX_TYPE_ACTIVE_RULE.getType()) diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java index 2cf1fe90848..2837318a565 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java @@ -30,6 +30,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import org.apache.commons.lang.StringUtils; import org.sonar.api.resources.Language; @@ -189,7 +190,7 @@ public class ActiveRuleCompleter { // load profiles Map<String, QProfileDto> profilesByUuid = dbClient.qualityProfileDao().selectByUuids(dbSession, new ArrayList<>(profileUuids)) .stream() - .collect(uniqueIndex(QProfileDto::getKee)); + .collect(Collectors.toMap(QProfileDto::getKee, Function.identity())); // load associated parents List<String> parentUuids = profilesByUuid.values().stream() diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/DeleteAction.java b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/DeleteAction.java index 2371bd07729..85da5137d03 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/DeleteAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/DeleteAction.java @@ -82,7 +82,7 @@ public class DeleteAction implements RulesWsAction { // For custom rule, first deactivate the rule on all profiles if (rule.isCustomRule()) { - ruleActivator.deactivateOfAllOrganizations(dbSession, rule); + ruleActivator.delete(dbSession, rule); } rule.setStatus(RuleStatus.REMOVED); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImplTest.java new file mode 100644 index 00000000000..6a0642b003b --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImplTest.java @@ -0,0 +1,249 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.qualityprofile; + +import java.util.List; +import java.util.Optional; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.api.profiles.RulesProfile; +import org.sonar.api.rules.RulePriority; +import org.sonar.api.utils.System2; +import org.sonar.api.utils.internal.TestSystem2; +import org.sonar.db.DbTester; +import org.sonar.db.qualityprofile.ActiveRuleDto; +import org.sonar.db.qualityprofile.RulesProfileDto; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; +import org.sonar.server.tester.UserSessionRule; +import org.sonar.server.util.IntegerTypeValidation; +import org.sonar.server.util.StringTypeValidation; +import org.sonar.server.util.TypeValidations; + +import static java.util.Arrays.asList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.sonar.api.rules.RulePriority.BLOCKER; +import static org.sonar.api.rules.RulePriority.CRITICAL; +import static org.sonar.api.rules.RulePriority.MAJOR; +import static org.sonar.db.qualityprofile.QualityProfileTesting.newRuleProfileDto; + +public class BuiltInQProfileUpdateImplTest { + + private static final long NOW = 1_000; + private static final long PAST = NOW - 100; + + @Rule + public BuiltInQProfileRepositoryRule builtInProfileRepository = new BuiltInQProfileRepositoryRule(); + @Rule + public DbTester db = DbTester.create().setDisableDefaultOrganization(true); + @Rule + public UserSessionRule userSession = UserSessionRule.standalone(); + private System2 system2 = new TestSystem2().setNow(NOW); + private ActiveRuleIndexer activeRuleIndexer = mock(ActiveRuleIndexer.class); + private RuleActivatorContextFactory contextFactory = new RuleActivatorContextFactory(db.getDbClient()); + private TypeValidations typeValidations = new TypeValidations(asList(new StringTypeValidation(), new IntegerTypeValidation())); + private RuleActivator ruleActivator = new RuleActivator(system2, db.getDbClient(), null, contextFactory, typeValidations, activeRuleIndexer, userSession); + + private BuiltInQProfileUpdateImpl underTest = new BuiltInQProfileUpdateImpl(db.getDbClient(), ruleActivator, activeRuleIndexer); + + private RulesProfileDto persistedProfile; + + @Before + public void setUp() { + persistedProfile = newRuleProfileDto(rp -> rp + .setIsBuiltIn(true) + .setLanguage("xoo") + .setRulesUpdatedAt(null)); + db.getDbClient().qualityProfileDao().insert(db.getSession(), persistedProfile); + db.commit(); + } + + @Test + public void activate_new_rules() { + RuleDefinitionDto rule1 = db.rules().insert(r -> r.setLanguage("xoo")); + RuleDefinitionDto rule2 = db.rules().insert(r -> r.setLanguage("xoo")); + RulesProfile apiProfile = RulesProfile.create("Sonar way", "xoo"); + activateRuleInDef(apiProfile, rule1, CRITICAL); + activateRuleInDef(apiProfile, rule2, MAJOR); + BuiltInQProfile builtIn = builtInProfileRepository.create(apiProfile); + + underTest.update(db.getSession(), builtIn, persistedProfile); + + List<ActiveRuleDto> activeRules = db.getDbClient().activeRuleDao().selectByRuleProfile(db.getSession(), persistedProfile); + assertThat(activeRules).hasSize(2); + assertThatRuleIsNewlyActivated(activeRules, rule1, CRITICAL); + assertThatRuleIsNewlyActivated(activeRules, rule2, MAJOR); + assertThatProfileIsMarkedAsUpdated(persistedProfile); + } + + @Test + public void already_activated_rule_is_updated_in_case_of_differences() { + RuleDefinitionDto rule = db.rules().insert(r -> r.setLanguage("xoo")); + RulesProfile apiProfile = RulesProfile.create("Sonar way", "xoo"); + activateRuleInDef(apiProfile, rule, CRITICAL); + BuiltInQProfile builtIn = builtInProfileRepository.create(apiProfile); + + activateRuleInDb(persistedProfile, rule, BLOCKER); + + underTest.update(db.getSession(), builtIn, persistedProfile); + + List<ActiveRuleDto> activeRules = db.getDbClient().activeRuleDao().selectByRuleProfile(db.getSession(), persistedProfile); + assertThat(activeRules).hasSize(1); + assertThatRuleIsUpdated(activeRules, rule, CRITICAL); + assertThatProfileIsMarkedAsUpdated(persistedProfile); + } + + @Test + public void already_activated_rule_is_not_touched_if_no_differences() { + RuleDefinitionDto rule = db.rules().insert(r -> r.setLanguage("xoo")); + RulesProfile apiProfile = RulesProfile.create("Sonar way", "xoo"); + activateRuleInDef(apiProfile, rule, CRITICAL); + BuiltInQProfile builtIn = builtInProfileRepository.create(apiProfile); + + activateRuleInDb(persistedProfile, rule, CRITICAL); + + underTest.update(db.getSession(), builtIn, persistedProfile); + + List<ActiveRuleDto> activeRules = db.getDbClient().activeRuleDao().selectByRuleProfile(db.getSession(), persistedProfile); + assertThat(activeRules).hasSize(1); + assertThatRuleIsUntouched(activeRules, rule, CRITICAL); + assertThatProfileIsNotMarkedAsUpdated(persistedProfile); + } + + @Test + public void deactivate_rule_that_is_not_in_built_in_definition_anymore() { + RuleDefinitionDto rule1 = db.rules().insert(r -> r.setLanguage("xoo")); + RuleDefinitionDto rule2 = db.rules().insert(r -> r.setLanguage("xoo")); + RulesProfile apiProfile = RulesProfile.create("Sonar way", "xoo"); + activateRuleInDef(apiProfile, rule2, CRITICAL); + BuiltInQProfile builtIn = builtInProfileRepository.create(apiProfile); + + // built-in definition contains only rule2 + // so rule1 must be deactivated + activateRuleInDb(persistedProfile, rule1, CRITICAL); + + underTest.update(db.getSession(), builtIn, persistedProfile); + + List<ActiveRuleDto> activeRules = db.getDbClient().activeRuleDao().selectByRuleProfile(db.getSession(), persistedProfile); + assertThat(activeRules).hasSize(1); + assertThatRuleIsDeactivated(activeRules, rule1); + assertThatProfileIsMarkedAsUpdated(persistedProfile); + } + + @Test + public void activate_deactivate_and_update_three_rules_at_the_same_time() { + RuleDefinitionDto rule1 = db.rules().insert(r -> r.setLanguage("xoo")); + RuleDefinitionDto rule2 = db.rules().insert(r -> r.setLanguage("xoo")); + RuleDefinitionDto rule3 = db.rules().insert(r -> r.setLanguage("xoo")); + RulesProfile apiProfile = RulesProfile.create("Sonar way", "xoo"); + activateRuleInDef(apiProfile, rule1, CRITICAL); + activateRuleInDef(apiProfile, rule2, MAJOR); + BuiltInQProfile builtIn = builtInProfileRepository.create(apiProfile); + + // rule1 must be updated (blocker to critical) + // rule2 must be activated + // rule3 must be deactivated + activateRuleInDb(persistedProfile, rule1, BLOCKER); + activateRuleInDb(persistedProfile, rule3, BLOCKER); + + underTest.update(db.getSession(), builtIn, persistedProfile); + + List<ActiveRuleDto> activeRules = db.getDbClient().activeRuleDao().selectByRuleProfile(db.getSession(), persistedProfile); + assertThat(activeRules).hasSize(2); + assertThatRuleIsUpdated(activeRules, rule1, CRITICAL); + assertThatRuleIsNewlyActivated(activeRules, rule2, MAJOR); + assertThatRuleIsDeactivated(activeRules, rule3); + assertThatProfileIsMarkedAsUpdated(persistedProfile); + } + + + private static void assertThatRuleIsNewlyActivated(List<ActiveRuleDto> activeRules, RuleDefinitionDto rule, RulePriority severity) { + ActiveRuleDto activeRule = findRule(activeRules, rule).get(); + + assertThat(activeRule.getInheritance()).isNull(); + assertThat(activeRule.getSeverityString()).isEqualTo(severity.name()); + assertThat(activeRule.getCreatedAt()).isEqualTo(NOW); + assertThat(activeRule.getUpdatedAt()).isEqualTo(NOW); + } + + private static void assertThatRuleIsUpdated(List<ActiveRuleDto> activeRules, RuleDefinitionDto rule, RulePriority severity) { + ActiveRuleDto activeRule = findRule(activeRules, rule).get(); + + assertThat(activeRule.getInheritance()).isNull(); + assertThat(activeRule.getSeverityString()).isEqualTo(severity.name()); + assertThat(activeRule.getCreatedAt()).isEqualTo(PAST); + assertThat(activeRule.getUpdatedAt()).isEqualTo(NOW); + } + + private static void assertThatRuleIsUntouched(List<ActiveRuleDto> activeRules, RuleDefinitionDto rule, RulePriority severity) { + ActiveRuleDto activeRule = findRule(activeRules, rule).get(); + + assertThat(activeRule.getInheritance()).isNull(); + assertThat(activeRule.getSeverityString()).isEqualTo(severity.name()); + assertThat(activeRule.getCreatedAt()).isEqualTo(PAST); + assertThat(activeRule.getUpdatedAt()).isEqualTo(PAST); + } + + private static void assertThatRuleIsDeactivated(List<ActiveRuleDto> activeRules, RuleDefinitionDto rule) { + assertThat(findRule(activeRules, rule)).isEmpty(); + } + + private void assertThatProfileIsMarkedAsUpdated(RulesProfileDto dto) { + RulesProfileDto reloaded = db.getDbClient().qualityProfileDao().selectBuiltInRulesProfiles(db.getSession()) + .stream() + .filter(p -> p.getKee().equals(dto.getKee())) + .findFirst() + .get(); + assertThat(reloaded.getRulesUpdatedAt()).isNotEmpty(); + } + + private void assertThatProfileIsNotMarkedAsUpdated(RulesProfileDto dto) { + RulesProfileDto reloaded = db.getDbClient().qualityProfileDao().selectBuiltInRulesProfiles(db.getSession()) + .stream() + .filter(p -> p.getKee().equals(dto.getKee())) + .findFirst() + .get(); + assertThat(reloaded.getRulesUpdatedAt()).isNull(); + } + + private static Optional<ActiveRuleDto> findRule(List<ActiveRuleDto> activeRules, RuleDefinitionDto rule) { + return activeRules.stream() + .filter(ar -> ar.getRuleKey().equals(rule.getKey())) + .findFirst(); + } + + private static void activateRuleInDef(RulesProfile apiProfile, RuleDefinitionDto rule, RulePriority severity) { + apiProfile.activateRule(org.sonar.api.rules.Rule.create(rule.getRepositoryKey(), rule.getRuleKey()), severity); + } + + private void activateRuleInDb(RulesProfileDto profile, RuleDefinitionDto rule, RulePriority severity) { + ActiveRuleDto dto = new ActiveRuleDto() + .setProfileId(profile.getId()) + .setSeverity(severity.name()) + .setRuleId(rule.getId()) + .setCreatedAt(PAST) + .setUpdatedAt(PAST); + db.getDbClient().activeRuleDao().insert(db.getSession(), dto); + db.commit(); + } + +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileComparisonMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileComparisonMediumTest.java index 31e4c19eba8..ddd99d77964 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileComparisonMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileComparisonMediumTest.java @@ -19,6 +19,7 @@ */ package org.sonar.server.qualityprofile; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.MapDifference.ValueDifference; import org.assertj.core.data.MapEntry; import org.junit.After; @@ -102,10 +103,8 @@ public class QProfileComparisonMediumTest { @Test public void compare_same() { - RuleActivation commonActivation = new RuleActivation(xooRule1.getKey()) - .setSeverity(Severity.CRITICAL) - .setParameter("min", "7") - .setParameter("max", "42"); + RuleActivation commonActivation = RuleActivation.create(xooRule1.getKey(), Severity.CRITICAL, + ImmutableMap.of("min", "7", "max", "42")); ruleActivator.activate(dbSession, commonActivation, left); ruleActivator.activate(dbSession, commonActivation, right); dbSession.commit(); @@ -122,7 +121,8 @@ public class QProfileComparisonMediumTest { @Test public void compare_only_left() { - ruleActivator.activate(dbSession, new RuleActivation(xooRule1.getKey()), left); + RuleActivation activation = RuleActivation.create(xooRule1.getKey()); + ruleActivator.activate(dbSession, activation, left); dbSession.commit(); QProfileComparisonResult result = comparison.compare(dbSession, left, right); @@ -137,7 +137,7 @@ public class QProfileComparisonMediumTest { @Test public void compare_only_right() { - ruleActivator.activate(dbSession, new RuleActivation(xooRule1.getKey()), right); + ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey()), right); dbSession.commit(); QProfileComparisonResult result = comparison.compare(dbSession, left, right); @@ -152,8 +152,8 @@ public class QProfileComparisonMediumTest { @Test public void compare_disjoint() { - ruleActivator.activate(dbSession, new RuleActivation(xooRule1.getKey()), left); - ruleActivator.activate(dbSession, new RuleActivation(xooRule2.getKey()), right); + ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey()), left); + ruleActivator.activate(dbSession, RuleActivation.create(xooRule2.getKey()), right); dbSession.commit(); QProfileComparisonResult result = comparison.compare(dbSession, left, right); @@ -168,8 +168,8 @@ public class QProfileComparisonMediumTest { @Test public void compare_modified_severity() { - ruleActivator.activate(dbSession, new RuleActivation(xooRule1.getKey()).setSeverity(Severity.CRITICAL), left); - ruleActivator.activate(dbSession, new RuleActivation(xooRule1.getKey()).setSeverity(Severity.BLOCKER), right); + ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey(), Severity.CRITICAL, null), left); + ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey(), Severity.BLOCKER, null), right); dbSession.commit(); QProfileComparisonResult result = comparison.compare(dbSession, left, right); @@ -189,8 +189,8 @@ public class QProfileComparisonMediumTest { @Test public void compare_modified_param() { - ruleActivator.activate(dbSession, new RuleActivation(xooRule1.getKey()).setParameter("max", "20"), left); - ruleActivator.activate(dbSession, new RuleActivation(xooRule1.getKey()).setParameter("max", "30"), right); + ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey(), null, ImmutableMap.of("max", "20")), left); + ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey(), null, ImmutableMap.of("max", "30")), right); dbSession.commit(); QProfileComparisonResult result = comparison.compare(dbSession, left, right); @@ -213,8 +213,8 @@ public class QProfileComparisonMediumTest { @Test public void compare_different_params() { - ruleActivator.activate(dbSession, new RuleActivation(xooRule1.getKey()).setParameter("max", "20"), left); - ruleActivator.activate(dbSession, new RuleActivation(xooRule1.getKey()).setParameter("min", "5"), right); + ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey(), null, ImmutableMap.of("max", "20")), left); + ruleActivator.activate(dbSession, RuleActivation.create(xooRule1.getKey(), null, ImmutableMap.of("min", "5")), right); dbSession.commit(); QProfileComparisonResult result = comparison.compare(dbSession, left, right); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileFactoryTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileFactoryTest.java index 6c7afb66e4e..687064fa21c 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileFactoryTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileFactoryTest.java @@ -120,7 +120,7 @@ public class QProfileFactoryTest { underTest.checkAndCreateCustom(dbSession, organization, name); dbSession.commit(); - expectBadRequestException("Quality profile already exists: {lang=xoo, name=P1}"); + expectBadRequestException("Quality profile already exists: xoo/P1"); underTest.checkAndCreateCustom(dbSession, organization, name); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesTest.java index 2a4a3674528..b8aa907ad31 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesTest.java @@ -21,7 +21,6 @@ package org.sonar.server.qualityprofile; import java.util.ArrayList; import java.util.List; -import org.apache.commons.lang.RandomStringUtils; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -39,6 +38,7 @@ import org.sonar.server.language.LanguageTesting; import org.sonar.server.tester.UserSessionRule; import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.db.qualityprofile.QualityProfileTesting.newRuleProfileDto; public class RegisterQualityProfilesTest { private static final Language FOO_LANGUAGE = LanguageTesting.newLanguage("foo"); @@ -57,10 +57,11 @@ public class RegisterQualityProfilesTest { private DbClient dbClient = db.getDbClient(); private DummyBuiltInQProfileInsert insert = new DummyBuiltInQProfileInsert(); + private DummyBuiltInQProfileUpdate update = new DummyBuiltInQProfileUpdate(); private RegisterQualityProfiles underTest = new RegisterQualityProfiles( builtInQProfileRepositoryRule, dbClient, - insert); + insert, update); @Test public void start_fails_if_BuiltInQProfileRepository_has_not_been_initialized() { @@ -77,8 +78,9 @@ public class RegisterQualityProfilesTest { underTest.start(); - assertThat(insert.callLogs) - .containsExactly(builtInQProfile); + assertThat(insert.callLogs).containsExactly(builtInQProfile); + assertThat(update.callLogs).isEmpty(); + assertThat(logTester.logs(LoggerLevel.INFO)).contains("Register profile foo/Sonar way"); } @Test @@ -92,8 +94,8 @@ public class RegisterQualityProfilesTest { underTest.start(); - assertThat(insert.callLogs) - .containsExactly(nonPersistedBuiltIn); + assertThat(insert.callLogs).containsExactly(nonPersistedBuiltIn); + assertThat(update.callLogs).containsExactly(persistedBuiltIn); } @Test @@ -115,16 +117,31 @@ public class RegisterQualityProfilesTest { assertThat(logTester.logs(LoggerLevel.INFO)).contains("Rename Quality profiles [foo/Sonar way] to [Sonar way (outdated copy)] in 2Â organizations"); } + @Test + public void update_built_in_profile_if_it_already_exists() { + RulesProfileDto ruleProfile = newRuleProfileDto(rp -> rp.setIsBuiltIn(true).setName("Sonar way").setLanguage(FOO_LANGUAGE.getKey())); + db.getDbClient().qualityProfileDao().insert(db.getSession(), ruleProfile); + db.commit(); + + BuiltInQProfile builtIn = builtInQProfileRepositoryRule.add(FOO_LANGUAGE, ruleProfile.getName(), false); + builtInQProfileRepositoryRule.initialize(); + + underTest.start(); + + assertThat(insert.callLogs).isEmpty(); + assertThat(update.callLogs).containsExactly(builtIn); + assertThat(logTester.logs(LoggerLevel.INFO)).contains("Update profile foo/Sonar way"); + } + private String selectPersistedName(QProfileDto profile) { return db.qualityProfiles().selectByUuid(profile.getKee()).get().getName(); } private void insertRulesProfile(BuiltInQProfile builtIn) { - RulesProfileDto dto = new RulesProfileDto() - .setIsBuiltIn(true) - .setKee(RandomStringUtils.randomAlphabetic(40)) - .setLanguage(builtIn.getLanguage()) - .setName(builtIn.getName()); + RulesProfileDto dto = newRuleProfileDto(rp -> rp + .setIsBuiltIn(true) + .setLanguage(builtIn.getLanguage()) + .setName(builtIn.getName())); dbClient.qualityProfileDao().insert(db.getSession(), dto); db.commit(); } @@ -137,4 +154,13 @@ public class RegisterQualityProfilesTest { callLogs.add(builtIn); } } + + private static class DummyBuiltInQProfileUpdate implements BuiltInQProfileUpdate { + private final List<BuiltInQProfile> callLogs = new ArrayList<>(); + + @Override + public void update(DbSession dbSession, BuiltInQProfile builtIn, RulesProfileDto ruleProfile) { + callLogs.add(builtIn); + } + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java index fbab477c55e..7553421f401 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java @@ -85,85 +85,6 @@ public class RuleActivatorMediumTest { // // // - - -// -// @Test -// public void ignore_parameters_when_activating_custom_rule() { -// // initial activation -// ActiveRuleKey activeRuleKey = ActiveRuleKey.of(XOO_P1_KEY, CUSTOM_RULE_KEY); -// RuleActivation activation = new RuleActivation(CUSTOM_RULE_KEY); -// activate(activation, XOO_P1_KEY); -// -// // update -// RuleActivation update = new RuleActivation(CUSTOM_RULE_KEY) -// .setParameter("format", "xls"); -// activate(update, XOO_P1_KEY); -// -// assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(1); -// verifyHasActiveRuleInDb(activeRuleKey, MINOR, null, ImmutableMap.of("format", "txt")); -// } -// -// -// @Test -// public void deactivation_fails_if_profile_not_found() { -// ActiveRuleKey key = ActiveRuleKey.of("unknown", XOO_X1); -// try { -// ruleActivator.deactivateAndUpdateIndex(dbSession, key); -// fail(); -// } catch (BadRequestException e) { -// assertThat(e).hasMessage("Quality profile not found: unknown"); -// } -// } -// -// -// @Test -// public void bulk_activation() { -// // Generate more rules than the search's max limit -// int bulkSize = SearchOptions.MAX_LIMIT + 10; -// List<RuleKey> keys = new ArrayList<>(); -// for (int i = 0; i < bulkSize; i++) { -// RuleDefinitionDto ruleDefinitionDto = newDto(RuleKey.of("bulk", "r_" + i)).setLanguage("xoo").getDefinition(); -// db.ruleDao().insert(dbSession, ruleDefinitionDto); -// keys.add(ruleDefinitionDto.getKey()); -// } -// dbSession.commit(); -// ruleIndexer.indexRuleDefinitions(keys); -// -// // 0. No active rules so far (base case) and plenty rules available -// verifyZeroActiveRules(XOO_P1_KEY); -// assertThat(tester.get(RuleIndex.class) -// .search(new RuleQuery().setRepositories(singletonList("bulk")), new SearchOptions()).getTotal()) -// .isEqualTo(bulkSize); -// -// // 1. bulk activate all the rules -// RuleQuery ruleQuery = new RuleQuery().setRepositories(singletonList("bulk")); -// BulkChangeResult result = ruleActivator.bulkActivate(dbSession, ruleQuery, selectProfile(XOO_P1_KEY), "MINOR"); -// -// // 2. assert that all activation has been commit to DB and ES -// dbSession.commit(); -// assertThat(db.activeRuleDao().selectByProfileUuid(dbSession, XOO_P1_KEY)).hasSize(bulkSize); -// assertThat(result.countSucceeded()).isEqualTo(bulkSize); -// assertThat(result.countFailed()).isEqualTo(0); -// } -// -// private QProfileDto selectProfile(String uuid) { -// return db.qualityProfileDao().selectByUuid(dbSession, uuid); -// } -// -// @Test -// public void bulk_activation_ignores_errors() { -// // 1. bulk activate all the rules, even non xoo-rules and xoo templates -// BulkChangeResult result = ruleActivator.bulkActivate(dbSession, new RuleQuery(), selectProfile(XOO_P1_KEY), "MINOR"); -// -// // 2. assert that all activations have been commit to DB and ES -// // -> xoo rules x1, x2 and custom1 -// dbSession.commit(); -// assertThat(db.activeRuleDao().selectByProfileUuid(dbSession, XOO_P1_KEY)).hasSize(3); -// assertThat(result.countSucceeded()).isEqualTo(3); -// assertThat(result.countFailed()).isGreaterThan(0); -// } -// // // @Test // public void ignore_activation_errors_when_setting_parent() { @@ -192,61 +113,6 @@ public class RuleActivatorMediumTest { // verifyHasActiveRuleInDbAndIndex(ActiveRuleKey.of(XOO_P2_KEY, XOO_X2), MAJOR, INHERITED, Collections.emptyMap()); // } // -// @Test -// public void bulk_deactivate() { -// activate(new RuleActivation(XOO_X1), XOO_P1_KEY); -// activate(new RuleActivation(XOO_X2), XOO_P1_KEY); -// assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(2); -// -// BulkChangeResult result = ruleActivator.bulkDeactivate(new RuleQuery().setActivation(true).setQProfileKey(XOO_P1_KEY), XOO_P1_KEY); -// -// dbSession.clearCache(); -// assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(0); -// assertThat(result.countFailed()).isEqualTo(0); -// assertThat(result.countSucceeded()).isEqualTo(2); -// assertThat(result.getChanges()).hasSize(2); -// } -// -// @Test -// public void bulk_deactivation_ignores_errors() { -// // activate on parent profile P1 -// createChildProfiles(); -// activate(new RuleActivation(XOO_X1), XOO_P1_KEY); -// assertThat(countActiveRules(XOO_P2_KEY)).isEqualTo(1); -// -// // bulk deactivate on child profile P2 -> not possible -// BulkChangeResult result = ruleActivator.bulkDeactivate(new RuleQuery().setActivation(true).setQProfileKey(XOO_P2_KEY), XOO_P2_KEY); -// -// dbSession.clearCache(); -// assertThat(countActiveRules(XOO_P2_KEY)).isEqualTo(1); -// assertThat(result.countFailed()).isEqualTo(1); -// assertThat(result.countSucceeded()).isEqualTo(0); -// assertThat(result.getChanges()).hasSize(0); -// } -// -// @Test -// public void bulk_change_severity() { -// createChildProfiles(); -// -// // activate two rules on root profile P1 (propagated to P2 and P3) -// RuleActivation activation = new RuleActivation(XOO_X1).setSeverity(INFO).setParameter("max", "7"); -// activate(activation, XOO_P1_KEY); -// activation = new RuleActivation(XOO_X2).setSeverity(INFO); -// activate(activation, XOO_P1_KEY); -// -// // bulk change severity to BLOCKER. Parameters are not set. -// RuleQuery query = new RuleQuery().setActivation(true).setQProfileKey(XOO_P1_KEY); -// BulkChangeResult result = ruleActivator.bulkActivate(dbSession, query, selectProfile(XOO_P1_KEY), "BLOCKER"); -// dbSession.commit(); -// assertThat(result.countSucceeded()).isEqualTo(2); -// -// verifyHasActiveRuleInDbAndIndex(ActiveRuleKey.of(XOO_P1_KEY, XOO_X1), BLOCKER, null, ImmutableMap.of("max", "7")); -// verifyHasActiveRuleInDbAndIndex(ActiveRuleKey.of(XOO_P1_KEY, XOO_X2), BLOCKER, null, Collections.<String, String>emptyMap()); -// verifyHasActiveRuleInDbAndIndex(ActiveRuleKey.of(XOO_P2_KEY, XOO_X1), BLOCKER, INHERITED, ImmutableMap.of("max", "7")); -// verifyHasActiveRuleInDbAndIndex(ActiveRuleKey.of(XOO_P2_KEY, XOO_X2), BLOCKER, INHERITED, Collections.<String, String>emptyMap()); -// verifyHasActiveRuleInDbAndIndex(ActiveRuleKey.of(XOO_P3_KEY, XOO_X1), BLOCKER, INHERITED, ImmutableMap.of("max", "7")); -// verifyHasActiveRuleInDbAndIndex(ActiveRuleKey.of(XOO_P3_KEY, XOO_X2), BLOCKER, INHERITED, Collections.<String, String>emptyMap()); -// } // // private int countActiveRules(String profileKey) { // List<ActiveRuleDto> activeRuleDtos = db.activeRuleDao().selectByProfileUuid(dbSession, profileKey); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java index 021f0b53a06..d75471f8ec5 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java @@ -19,9 +19,12 @@ */ package org.sonar.server.qualityprofile; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Random; +import java.util.stream.IntStream; import javax.annotation.Nullable; import org.junit.Rule; import org.junit.Test; @@ -37,14 +40,19 @@ import org.sonar.db.DbTester; import org.sonar.db.qualityprofile.ActiveRuleParamDto; import org.sonar.db.qualityprofile.OrgActiveRuleDto; import org.sonar.db.qualityprofile.QProfileDto; +import org.sonar.db.qualityprofile.RulesProfileDto; import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.db.rule.RuleDto; import org.sonar.db.rule.RuleParamDto; import org.sonar.server.es.EsTester; +import org.sonar.server.es.SearchOptions; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.qualityprofile.index.ActiveRuleIteratorFactory; import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.rule.index.RuleIndexDefinition; +import org.sonar.server.rule.index.RuleIndexer; +import org.sonar.server.rule.index.RuleQuery; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.util.IntegerTypeValidation; import org.sonar.server.util.StringTypeValidation; @@ -53,11 +61,16 @@ import org.sonar.server.util.TypeValidations; import static com.google.common.collect.ImmutableMap.of; import static java.util.Arrays.asList; import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; +import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; import static org.sonar.api.rule.Severity.BLOCKER; import static org.sonar.api.rule.Severity.CRITICAL; import static org.sonar.api.rule.Severity.MAJOR; +import static org.sonar.api.rule.Severity.MINOR; +import static org.sonar.db.rule.RuleTesting.newCustomRule; +import static org.sonar.server.qualityprofile.ActiveRule.Inheritance.INHERITED; public class RuleActivatorTest { @@ -75,6 +88,7 @@ public class RuleActivatorTest { private RuleActivatorContextFactory contextFactory = new RuleActivatorContextFactory(db.getDbClient()); private ActiveRuleIteratorFactory activeRuleIteratorFactory = new ActiveRuleIteratorFactory(db.getDbClient()); private ActiveRuleIndexer activeRuleIndexer = new ActiveRuleIndexer(db.getDbClient(), es.client(), activeRuleIteratorFactory); + private RuleIndexer ruleIndexer = new RuleIndexer(es.client(), db.getDbClient()); private TypeValidations typeValidations = new TypeValidations(asList(new StringTypeValidation(), new IntegerTypeValidation())); private RuleActivator underTest = new RuleActivator(system2, db.getDbClient(), ruleIndex, contextFactory, typeValidations, activeRuleIndexer, @@ -84,8 +98,7 @@ public class RuleActivatorTest { public void system_activates_rule_without_parameters() { RuleDefinitionDto rule = createRule(); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()); - activation.setSeverity(BLOCKER); + RuleActivation activation = RuleActivation.create(rule.getKey(), BLOCKER, null); List<ActiveRuleChange> changes = activate(profile, activation); assertThatRuleIsActivated(profile, rule, changes, BLOCKER, null, emptyMap()); @@ -97,8 +110,7 @@ public class RuleActivatorTest { userSession.logIn(); RuleDefinitionDto rule = createRule(); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()); - activation.setSeverity(BLOCKER); + RuleActivation activation = RuleActivation.create(rule.getKey(), BLOCKER, null); List<ActiveRuleChange> changes = activate(profile, activation); assertThatRuleIsActivated(profile, rule, changes, BLOCKER, null, emptyMap()); @@ -111,7 +123,7 @@ public class RuleActivatorTest { RuleParamDto ruleParam = db.rules().insertRuleParam(rule, p -> p.setName("min").setDefaultValue("10")); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); List<ActiveRuleChange> changes = activate(profile, activation); assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, of("min", "10")); @@ -124,8 +136,7 @@ public class RuleActivatorTest { RuleParamDto ruleParam = db.rules().insertRuleParam(rule, p -> p.setName("min").setDefaultValue("10")); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()) - .setParameter(ruleParam.getName(), "15"); + RuleActivation activation = RuleActivation.create(rule.getKey(), null, of(ruleParam.getName(), "15")); List<ActiveRuleChange> changes = activate(profile, activation); assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, of("min", "15")); @@ -137,7 +148,7 @@ public class RuleActivatorTest { RuleDefinitionDto rule = createRule(); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); List<ActiveRuleChange> changes = activate(profile, activation); assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, emptyMap()); @@ -153,8 +164,7 @@ public class RuleActivatorTest { RuleParamDto ruleParam = db.rules().insertRuleParam(rule, p -> p.setName("min").setDefaultValue("10")); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()) - .setParameter("min", ""); + RuleActivation activation = RuleActivation.create(rule.getKey(), null, of("min", "")); List<ActiveRuleChange> changes = activate(profile, activation); assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, of("min", "10")); @@ -171,8 +181,7 @@ public class RuleActivatorTest { RuleParamDto paramWithDefault = db.rules().insertRuleParam(rule, p -> p.setName("max").setDefaultValue("10")); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()) - .setParameter(paramWithoutDefault.getName(), "-10"); + RuleActivation activation = RuleActivation.create(rule.getKey(), null, of(paramWithoutDefault.getName(), "-10")); List<ActiveRuleChange> changes = activate(profile, activation); assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, @@ -186,8 +195,7 @@ public class RuleActivatorTest { RuleParamDto param = db.rules().insertRuleParam(rule, p -> p.setName("max").setDefaultValue("10")); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()) - .setParameter("xxx", "yyy"); + RuleActivation activation = RuleActivation.create(rule.getKey(), null, of("xxx", "yyy")); List<ActiveRuleChange> changes = activate(profile, activation); assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, of(param.getName(), param.getDefaultValue())); @@ -201,14 +209,11 @@ public class RuleActivatorTest { QProfileDto profile = createProfile(rule); // initial activation - RuleActivation activation = new RuleActivation(rule.getKey()) - .setSeverity(MAJOR); + RuleActivation activation = RuleActivation.create(rule.getKey(), MAJOR, null); activate(profile, activation); // update - RuleActivation updateActivation = new RuleActivation(rule.getKey()) - .setSeverity(CRITICAL) - .setParameter(param.getName(), "20"); + RuleActivation updateActivation = RuleActivation.create(rule.getKey(), CRITICAL, of(param.getName(), "20")); List<ActiveRuleChange> changes = activate(profile, updateActivation); assertThatRuleIsUpdated(profile, rule, CRITICAL, null, of(param.getName(), "20")); @@ -223,13 +228,11 @@ public class RuleActivatorTest { QProfileDto profile = createProfile(rule); // initial activation -> param "max" has a default value - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); activate(profile, activation); // update param "min", which has no default value - RuleActivation updateActivation = new RuleActivation(rule.getKey()) - .setSeverity(MAJOR) - .setParameter(paramWithoutDefault.getName(), "3"); + RuleActivation updateActivation = RuleActivation.create(rule.getKey(), MAJOR, of(paramWithoutDefault.getName(), "3")); List<ActiveRuleChange> changes = activate(profile, updateActivation); assertThatRuleIsUpdated(profile, rule, MAJOR, null, of(paramWithDefault.getName(), "10", paramWithoutDefault.getName(), "3")); @@ -243,13 +246,11 @@ public class RuleActivatorTest { QProfileDto profile = createProfile(rule); // initial activation -> param "max" has a default value - RuleActivation activation = new RuleActivation(rule.getKey()) - .setParameter(paramWithDefault.getName(), "20"); + RuleActivation activation = RuleActivation.create(rule.getKey(), null, of(paramWithDefault.getName(), "20")); activate(profile, activation); // reset to default_value - RuleActivation updateActivation = new RuleActivation(rule.getKey()) - .setParameter(paramWithDefault.getName(), null); + RuleActivation updateActivation = RuleActivation.create(rule.getKey(), null, of(paramWithDefault.getName(), "")); List<ActiveRuleChange> changes = activate(profile, updateActivation); assertThatRuleIsUpdated(profile, rule, rule.getSeverityString(), null, of(paramWithDefault.getName(), "10")); @@ -264,13 +265,11 @@ public class RuleActivatorTest { QProfileDto profile = createProfile(rule); // initial activation -> param "max" has a default value - RuleActivation activation = new RuleActivation(rule.getKey()) - .setParameter(paramWithoutDefault.getName(), "20"); + RuleActivation activation = RuleActivation.create(rule.getKey(), null, of(paramWithoutDefault.getName(), "20")); activate(profile, activation); // remove parameter - RuleActivation updateActivation = new RuleActivation(rule.getKey()) - .setParameter(paramWithoutDefault.getName(), null); + RuleActivation updateActivation = RuleActivation.create(rule.getKey(), null, of(paramWithoutDefault.getName(), "")); List<ActiveRuleChange> changes = activate(profile, updateActivation); assertThatRuleIsUpdated(profile, rule, rule.getSeverityString(), null, of(paramWithDefault.getName(), paramWithDefault.getDefaultValue())); @@ -284,14 +283,13 @@ public class RuleActivatorTest { QProfileDto profile = createProfile(rule); // initial activation -> param "max" has a default value - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); List<ActiveRuleChange> changes = activate(profile, activation); db.getDbClient().activeRuleDao().deleteParametersByRuleProfileUuids(db.getSession(), asList(profile.getRulesProfileUuid())); assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, emptyMap()); // contrary to activerule, the param is supposed to be inserted but not updated - RuleActivation updateActivation = new RuleActivation(rule.getKey()) - .setParameter(param.getName(), null); + RuleActivation updateActivation = RuleActivation.create(rule.getKey(), null, of(param.getName(), "")); changes = activate(profile, updateActivation); assertThatRuleIsUpdated(profile, rule, rule.getSeverityString(), null, of(param.getName(), param.getDefaultValue())); @@ -304,11 +302,11 @@ public class RuleActivatorTest { QProfileDto profile = createProfile(rule); // initial activation - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); activate(profile, activation); // update with exactly the same severity and params - activation = new RuleActivation(rule.getKey()); + activation = RuleActivation.create(rule.getKey()); List<ActiveRuleChange> changes = activate(profile, activation); assertThat(changes).isEmpty(); @@ -321,13 +319,11 @@ public class RuleActivatorTest { QProfileDto profile = createProfile(rule); // initial activation -> param "max" has a default value - RuleActivation activation = new RuleActivation(rule.getKey()) - .setSeverity(BLOCKER) - .setParameter(param.getName(), "20"); + RuleActivation activation = RuleActivation.create(rule.getKey(), BLOCKER, of(param.getName(), "20")); activate(profile, activation); // update without any severity or params => keep - RuleActivation update = new RuleActivation(rule.getKey()); + RuleActivation update = RuleActivation.create(rule.getKey()); List<ActiveRuleChange> changes = activate(profile, update); assertThat(changes).isEmpty(); @@ -338,7 +334,7 @@ public class RuleActivatorTest { RuleDefinitionDto rule = createRule(); QProfileDto profile = createProfile(rule); RuleKey ruleKey = RuleKey.parse("unknown:xxx"); - RuleActivation activation = new RuleActivation(ruleKey); + RuleActivation activation = RuleActivation.create(ruleKey); expectFailure("Rule not found: " + ruleKey, () -> activate(profile, activation)); } @@ -347,7 +343,7 @@ public class RuleActivatorTest { public void fail_to_activate_rule_if_profile_is_on_different_languages() { RuleDefinitionDto rule = createJavaRule(); QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p.setLanguage("js")); - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); expectFailure("Rule " + rule.getKey() + " and profile " + profile.getKee() + " have different languages", () -> activate(profile, activation)); } @@ -356,7 +352,7 @@ public class RuleActivatorTest { public void fail_to_activate_rule_if_rule_has_REMOVED_status() { RuleDefinitionDto rule = db.rules().insert(r -> r.setStatus(RuleStatus.REMOVED)); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); expectFailure("Rule was removed: " + rule.getKey(), () -> activate(profile, activation)); } @@ -365,7 +361,7 @@ public class RuleActivatorTest { public void fail_to_activate_if_template() { RuleDefinitionDto rule = db.rules().insert(r -> r.setIsTemplate(true)); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); expectFailure("Rule template can't be activated on a Quality profile: " + rule.getKey(), () -> activate(profile, activation)); } @@ -376,17 +372,36 @@ public class RuleActivatorTest { RuleParamDto param = db.rules().insertRuleParam(rule, p -> p.setName("max").setDefaultValue("10").setType(PropertyType.INTEGER.name())); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()) - .setParameter(param.getName(), "foo"); + RuleActivation activation = RuleActivation.create(rule.getKey(), null, of(param.getName(), "foo")); expectFailure("Value 'foo' must be an integer.", () -> activate(profile, activation)); } + + @Test + public void ignore_parameters_when_activating_custom_rule() { + RuleDefinitionDto templateRule = db.rules().insert(r -> r.setIsTemplate(true)); + RuleParamDto templateParam = db.rules().insertRuleParam(templateRule, p -> p.setName("format")); + RuleDefinitionDto customRule = db.rules().insert(newCustomRule(templateRule)); + RuleParamDto customParam = db.rules().insertRuleParam(customRule, p -> p.setName("format").setDefaultValue("txt")); + QProfileDto profile = createProfile(customRule); + + // initial activation + RuleActivation activation = RuleActivation.create(customRule.getKey(), MAJOR, emptyMap()); + activate(profile, activation); + assertThatRuleIsActivated(profile, customRule, null, MAJOR, null, of("format", "txt")); + + // update -> parameter is not changed + RuleActivation updateActivation = RuleActivation.create(customRule.getKey(), BLOCKER, of("format", "xml")); + activate(profile, updateActivation); + assertThatRuleIsActivated(profile, customRule, null, BLOCKER, null, of("format", "txt")); + } + @Test public void user_deactivates_a_rule() { userSession.logIn(); RuleDefinitionDto rule = createRule(); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); activate(profile, activation); List<ActiveRuleChange> changes = deactivate(profile, rule); @@ -400,7 +415,7 @@ public class RuleActivatorTest { public void system_deactivates_a_rule() { RuleDefinitionDto rule = createRule(); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); activate(profile, activation); List<ActiveRuleChange> changes = deactivate(profile, rule); @@ -439,7 +454,7 @@ public class RuleActivatorTest { public void deactivate_rule_that_has_REMOVED_status() { RuleDefinitionDto rule = createRule(); QProfileDto profile = createProfile(rule); - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); activate(profile, activation); rule.setStatus(RuleStatus.REMOVED); @@ -457,10 +472,10 @@ public class RuleActivatorTest { QProfileDto childProfile = createChildProfile(parentProfile); QProfileDto grandChildProfile = createChildProfile(childProfile); - List<ActiveRuleChange> changes = activate(childProfile, new RuleActivation(rule.getKey())); + List<ActiveRuleChange> changes = activate(childProfile, RuleActivation.create(rule.getKey())); assertThatProfileHasNoActiveRules(parentProfile); assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), null, emptyMap()); - assertThatRuleIsActivated(grandChildProfile, rule, changes, rule.getSeverityString(), ActiveRule.Inheritance.INHERITED, emptyMap()); + assertThatRuleIsActivated(grandChildProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); } @Test @@ -471,19 +486,15 @@ public class RuleActivatorTest { QProfileDto childProfile = createChildProfile(parentProfile); QProfileDto grandChildProfile = createChildProfile(childProfile); - RuleActivation initialActivation = new RuleActivation(rule.getKey()) - .setSeverity(MAJOR) - .setParameter(param.getName(), "foo"); + RuleActivation initialActivation = RuleActivation.create(rule.getKey(), MAJOR, of(param.getName(), "foo")); activate(childProfile, initialActivation); - RuleActivation updateActivation = new RuleActivation(rule.getKey()) - .setSeverity(CRITICAL) - .setParameter(param.getName(), "bar"); + RuleActivation updateActivation = RuleActivation.create(rule.getKey(), CRITICAL, of(param.getName(), "bar")); List<ActiveRuleChange> changes = activate(childProfile, updateActivation); assertThatProfileHasNoActiveRules(parentProfile); assertThatRuleIsUpdated(childProfile, rule, CRITICAL, null, of(param.getName(), "bar")); - assertThatRuleIsUpdated(grandChildProfile, rule, CRITICAL, ActiveRule.Inheritance.INHERITED, of(param.getName(), "bar")); + assertThatRuleIsUpdated(grandChildProfile, rule, CRITICAL, INHERITED, of(param.getName(), "bar")); assertThat(changes).hasSize(2); } @@ -495,14 +506,10 @@ public class RuleActivatorTest { QProfileDto childProfile = createChildProfile(parentProfile); QProfileDto grandChildProfile = createChildProfile(childProfile); - RuleActivation initialActivation = new RuleActivation(rule.getKey()) - .setSeverity(MAJOR) - .setParameter(param.getName(), "foo"); + RuleActivation initialActivation = RuleActivation.create(rule.getKey(), MAJOR, of(param.getName(), "foo")); activate(childProfile, initialActivation); - RuleActivation overrideActivation = new RuleActivation(rule.getKey()) - .setSeverity(CRITICAL) - .setParameter(param.getName(), "bar"); + RuleActivation overrideActivation = RuleActivation.create(rule.getKey(), CRITICAL, of(param.getName(), "bar")); List<ActiveRuleChange> changes = activate(grandChildProfile, overrideActivation); assertThatProfileHasNoActiveRules(parentProfile); @@ -519,20 +526,14 @@ public class RuleActivatorTest { QProfileDto childProfile = createChildProfile(parentProfile); QProfileDto grandChildProfile = createChildProfile(childProfile); - RuleActivation initialActivation = new RuleActivation(rule.getKey()) - .setSeverity(MAJOR) - .setParameter(param.getName(), "foo"); + RuleActivation initialActivation = RuleActivation.create(rule.getKey(), MAJOR, of(param.getName(), "foo")); activate(childProfile, initialActivation); - RuleActivation overrideActivation = new RuleActivation(rule.getKey()) - .setSeverity(CRITICAL) - .setParameter(param.getName(), "bar"); + RuleActivation overrideActivation = RuleActivation.create(rule.getKey(), CRITICAL, of(param.getName(), "bar")); activate(grandChildProfile, overrideActivation); // update child --> do not touch grandChild - RuleActivation updateActivation = new RuleActivation(rule.getKey()) - .setSeverity(BLOCKER) - .setParameter(param.getName(), "baz"); + RuleActivation updateActivation = RuleActivation.create(rule.getKey(), BLOCKER, of(param.getName(), "baz")); List<ActiveRuleChange> changes = activate(childProfile, updateActivation); assertThatProfileHasNoActiveRules(parentProfile); @@ -549,23 +550,18 @@ public class RuleActivatorTest { QProfileDto childProfile = createChildProfile(parentProfile); QProfileDto grandChildProfile = createChildProfile(childProfile); - RuleActivation initialActivation = new RuleActivation(rule.getKey()) - .setSeverity(MAJOR) - .setParameter(param.getName(), "foo"); + RuleActivation initialActivation = RuleActivation.create(rule.getKey(), MAJOR, of(param.getName(), "foo")); activate(parentProfile, initialActivation); - RuleActivation overrideActivation = new RuleActivation(rule.getKey()) - .setSeverity(CRITICAL) - .setParameter(param.getName(), "bar"); + RuleActivation overrideActivation = RuleActivation.create(rule.getKey(), CRITICAL, of(param.getName(), "bar")); activate(grandChildProfile, overrideActivation); // reset parent --> touch child but not grandChild - RuleActivation updateActivation = new RuleActivation(rule.getKey()) - .setReset(true); + RuleActivation updateActivation = RuleActivation.createReset(rule.getKey()); List<ActiveRuleChange> changes = activate(parentProfile, updateActivation); assertThatRuleIsUpdated(parentProfile, rule, rule.getSeverityString(), null, of(param.getName(), param.getDefaultValue())); - assertThatRuleIsUpdated(childProfile, rule, rule.getSeverityString(), ActiveRule.Inheritance.INHERITED, of(param.getName(), param.getDefaultValue())); + assertThatRuleIsUpdated(childProfile, rule, rule.getSeverityString(), INHERITED, of(param.getName(), param.getDefaultValue())); assertThatRuleIsUpdated(grandChildProfile, rule, CRITICAL, ActiveRule.Inheritance.OVERRIDES, of(param.getName(), "bar")); assertThat(changes).hasSize(2); } @@ -577,14 +573,10 @@ public class RuleActivatorTest { QProfileDto parentProfile = createProfile(rule); QProfileDto childProfile = createChildProfile(parentProfile); - RuleActivation childActivation = new RuleActivation(rule.getKey()) - .setSeverity(MAJOR) - .setParameter(param.getName(), "foo"); + RuleActivation childActivation = RuleActivation.create(rule.getKey(), MAJOR, of(param.getName(), "foo")); activate(childProfile, childActivation); - RuleActivation parentActivation = new RuleActivation(rule.getKey()) - .setSeverity(CRITICAL) - .setParameter(param.getName(), "bar"); + RuleActivation parentActivation = RuleActivation.create(rule.getKey(), CRITICAL, of(param.getName(), "bar")); List<ActiveRuleChange> changes = activate(parentProfile, parentActivation); assertThatRuleIsUpdated(parentProfile, rule, CRITICAL, null, of(param.getName(), "bar")); @@ -599,17 +591,13 @@ public class RuleActivatorTest { QProfileDto parentProfile = createProfile(rule); QProfileDto childProfile = createChildProfile(parentProfile); - RuleActivation parentActivation = new RuleActivation(rule.getKey()) - .setSeverity(MAJOR) - .setParameter(param.getName(), "foo"); + RuleActivation parentActivation = RuleActivation.create(rule.getKey(), MAJOR, of(param.getName(), "foo")); activate(parentProfile, parentActivation); - RuleActivation overrideActivation = new RuleActivation(rule.getKey()) - .setSeverity(MAJOR) - .setParameter(param.getName(), "foo"); + RuleActivation overrideActivation = RuleActivation.create(rule.getKey(), MAJOR, of(param.getName(), "foo")); List<ActiveRuleChange> changes = activate(childProfile, overrideActivation); - assertThatRuleIsUpdated(childProfile, rule, MAJOR, ActiveRule.Inheritance.INHERITED, of(param.getName(), "foo")); + assertThatRuleIsUpdated(childProfile, rule, MAJOR, INHERITED, of(param.getName(), "foo")); assertThat(changes).hasSize(0); } @@ -619,10 +607,10 @@ public class RuleActivatorTest { QProfileDto parentProfile = createProfile(rule); QProfileDto childProfile = createChildProfile(parentProfile); - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); List<ActiveRuleChange> changes = activate(parentProfile, activation); assertThatRuleIsActivated(parentProfile, rule, changes, rule.getSeverityString(), null, emptyMap()); - assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), ActiveRule.Inheritance.INHERITED, emptyMap()); + assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); changes = deactivate(parentProfile, rule); assertThatProfileHasNoActiveRules(parentProfile); @@ -636,13 +624,12 @@ public class RuleActivatorTest { QProfileDto parentProfile = createProfile(rule); QProfileDto childProfile = createChildProfile(parentProfile); - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); List<ActiveRuleChange> changes = activate(parentProfile, activation); assertThatRuleIsActivated(parentProfile, rule, changes, rule.getSeverityString(), null, emptyMap()); - assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), ActiveRule.Inheritance.INHERITED, emptyMap()); + assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); - activation = new RuleActivation(rule.getKey()) - .setSeverity(CRITICAL); + activation = RuleActivation.create(rule.getKey(), CRITICAL, null); activate(childProfile, activation); changes = deactivate(parentProfile, rule); @@ -657,10 +644,10 @@ public class RuleActivatorTest { QProfileDto parentProfile = createProfile(rule); QProfileDto childProfile = createChildProfile(parentProfile); - RuleActivation activation = new RuleActivation(rule.getKey()); + RuleActivation activation = RuleActivation.create(rule.getKey()); List<ActiveRuleChange> changes = activate(parentProfile, activation); assertThatRuleIsActivated(parentProfile, rule, changes, rule.getSeverityString(), null, emptyMap()); - assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), ActiveRule.Inheritance.INHERITED, emptyMap()); + assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); expectedException.expect(BadRequestException.class); expectedException.expectMessage("Cannot deactivate inherited rule"); @@ -673,22 +660,20 @@ public class RuleActivatorTest { QProfileDto parentProfile = createProfile(rule); QProfileDto childProfile = createChildProfile(parentProfile); - RuleActivation activation = new RuleActivation(rule.getKey()) - .setSeverity(CRITICAL); + RuleActivation activation = RuleActivation.create(rule.getKey(), CRITICAL, null); List<ActiveRuleChange> changes = activate(parentProfile, activation); assertThatRuleIsActivated(parentProfile, rule, changes, CRITICAL, null, emptyMap()); - assertThatRuleIsActivated(childProfile, rule, changes, CRITICAL, ActiveRule.Inheritance.INHERITED, emptyMap()); + assertThatRuleIsActivated(childProfile, rule, changes, CRITICAL, INHERITED, emptyMap()); assertThat(changes).hasSize(2); - RuleActivation childActivation = new RuleActivation(rule.getKey()) - .setSeverity(BLOCKER); + RuleActivation childActivation = RuleActivation.create(rule.getKey(), BLOCKER, null); changes = activate(childProfile, childActivation); assertThatRuleIsUpdated(childProfile, rule, BLOCKER, ActiveRule.Inheritance.OVERRIDES, emptyMap()); assertThat(changes).hasSize(1); - RuleActivation resetActivation = new RuleActivation(rule.getKey()).setReset(true); + RuleActivation resetActivation = RuleActivation.createReset(rule.getKey()); changes = activate(childProfile, resetActivation); - assertThatRuleIsUpdated(childProfile, rule, CRITICAL, ActiveRule.Inheritance.INHERITED, emptyMap()); + assertThatRuleIsUpdated(childProfile, rule, CRITICAL, INHERITED, emptyMap()); assertThatRuleIsUpdated(parentProfile, rule, CRITICAL, null, emptyMap()); assertThat(changes).hasSize(1); } @@ -700,52 +685,50 @@ public class RuleActivatorTest { QProfileDto childProfile = createChildProfile(parentProfile); QProfileDto grandchildProfile = createChildProfile(childProfile); - RuleActivation activation = new RuleActivation(rule.getKey()) - .setSeverity(CRITICAL); + RuleActivation activation = RuleActivation.create(rule.getKey(), CRITICAL, null); List<ActiveRuleChange> changes = activate(parentProfile, activation); assertThatRuleIsActivated(parentProfile, rule, changes, CRITICAL, null, emptyMap()); - assertThatRuleIsActivated(childProfile, rule, changes, CRITICAL, ActiveRule.Inheritance.INHERITED, emptyMap()); - assertThatRuleIsActivated(grandchildProfile, rule, changes, CRITICAL, ActiveRule.Inheritance.INHERITED, emptyMap()); + assertThatRuleIsActivated(childProfile, rule, changes, CRITICAL, INHERITED, emptyMap()); + assertThatRuleIsActivated(grandchildProfile, rule, changes, CRITICAL, INHERITED, emptyMap()); assertThat(changes).hasSize(3); - RuleActivation childActivation = new RuleActivation(rule.getKey()) - .setSeverity(BLOCKER); + RuleActivation childActivation = RuleActivation.create(rule.getKey(), BLOCKER, null); changes = activate(childProfile, childActivation); assertThatRuleIsUpdated(childProfile, rule, BLOCKER, ActiveRule.Inheritance.OVERRIDES, emptyMap()); - assertThatRuleIsUpdated(grandchildProfile, rule, BLOCKER, ActiveRule.Inheritance.INHERITED, emptyMap()); + assertThatRuleIsUpdated(grandchildProfile, rule, BLOCKER, INHERITED, emptyMap()); assertThat(changes).hasSize(2); // Reset on parent do not change child nor grandchild - RuleActivation resetActivation = new RuleActivation(rule.getKey()).setReset(true); + RuleActivation resetActivation = RuleActivation.createReset(rule.getKey()); changes = activate(parentProfile, resetActivation); assertThatRuleIsUpdated(parentProfile, rule, rule.getSeverityString(), null, emptyMap()); assertThatRuleIsUpdated(childProfile, rule, BLOCKER, ActiveRule.Inheritance.OVERRIDES, emptyMap()); - assertThatRuleIsUpdated(grandchildProfile, rule, BLOCKER, ActiveRule.Inheritance.INHERITED, emptyMap()); + assertThatRuleIsUpdated(grandchildProfile, rule, BLOCKER, INHERITED, emptyMap()); assertThat(changes).hasSize(1); // Reset on child change grandchild - resetActivation = new RuleActivation(rule.getKey()).setReset(true); + resetActivation = RuleActivation.createReset(rule.getKey()); changes = activate(childProfile, resetActivation); assertThatRuleIsUpdated(parentProfile, rule, rule.getSeverityString(), null, emptyMap()); - assertThatRuleIsUpdated(childProfile, rule, rule.getSeverityString(), ActiveRule.Inheritance.INHERITED, emptyMap()); - assertThatRuleIsUpdated(grandchildProfile, rule, rule.getSeverityString(), ActiveRule.Inheritance.INHERITED, emptyMap()); + assertThatRuleIsUpdated(childProfile, rule, rule.getSeverityString(), INHERITED, emptyMap()); + assertThatRuleIsUpdated(grandchildProfile, rule, rule.getSeverityString(), INHERITED, emptyMap()); assertThat(changes).hasSize(2); } @Test - public void ignore_reset_if_not_activated() { + public void ignore_reset_if_not_activated() { RuleDefinitionDto rule = createRule(); QProfileDto parentProfile = createProfile(rule); QProfileDto childProfile = createChildProfile(parentProfile); - RuleActivation resetActivation = new RuleActivation(rule.getKey()).setReset(true); + RuleActivation resetActivation = RuleActivation.createReset(rule.getKey()); List<ActiveRuleChange> changes = activate(parentProfile, resetActivation); verifyNoActiveRules(); assertThat(changes).hasSize(0); } @Test - public void unset_parent_when_no_parent_does_not_fail() { + public void unset_parent_when_no_parent_does_not_fail() { RuleDefinitionDto rule = createRule(); QProfileDto profile = createProfile(rule); underTest.setParent(db.getSession(), profile, null); @@ -785,16 +768,16 @@ public class RuleActivatorTest { } @Test - public void cannot_set_parent_if_language_is_different() { + public void cannot_set_parent_if_language_is_different() { RuleDefinitionDto rule1 = db.rules().insert(r -> r.setLanguage("foo")); RuleDefinitionDto rule2 = db.rules().insert(r -> r.setLanguage("bar")); QProfileDto parentProfile = createProfile(rule1); - List<ActiveRuleChange> changes = activate(parentProfile, new RuleActivation(rule1.getKey())); + List<ActiveRuleChange> changes = activate(parentProfile, RuleActivation.create(rule1.getKey())); assertThat(changes).hasSize(1); QProfileDto childProfile = createProfile(rule2); - changes = activate(childProfile, new RuleActivation(rule2.getKey())); + changes = activate(childProfile, RuleActivation.create(rule2.getKey())); assertThat(changes).hasSize(1); expectedException.expect(BadRequestException.class); @@ -804,21 +787,21 @@ public class RuleActivatorTest { } @Test - public void set_then_unset_parent() { + public void set_then_unset_parent() { RuleDefinitionDto rule1 = createJavaRule(); RuleDefinitionDto rule2 = createJavaRule(); QProfileDto profile1 = createProfile(rule1); - List<ActiveRuleChange> changes = activate(profile1, new RuleActivation(rule1.getKey())); + List<ActiveRuleChange> changes = activate(profile1, RuleActivation.create(rule1.getKey())); assertThat(changes).hasSize(1); QProfileDto profile2 = createProfile(rule2); - changes = activate(profile2, new RuleActivation(rule2.getKey())); + changes = activate(profile2, RuleActivation.create(rule2.getKey())); assertThat(changes).hasSize(1); changes = underTest.setParent(db.getSession(), profile2, profile1); assertThat(changes).hasSize(1); - assertThatRuleIsActivated(profile2, rule1, changes, rule1.getSeverityString(), ActiveRule.Inheritance.INHERITED, emptyMap()); + assertThatRuleIsActivated(profile2, rule1, changes, rule1.getSeverityString(), INHERITED, emptyMap()); assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); changes = underTest.setParent(db.getSession(), profile2, null); @@ -828,24 +811,23 @@ public class RuleActivatorTest { } @Test - public void set_then_unset_parent_keep_overridden_rules() { + public void set_then_unset_parent_keep_overridden_rules() { RuleDefinitionDto rule1 = createJavaRule(); RuleDefinitionDto rule2 = createJavaRule(); QProfileDto profile1 = createProfile(rule1); - List<ActiveRuleChange> changes = activate(profile1, new RuleActivation(rule1.getKey())); + List<ActiveRuleChange> changes = activate(profile1, RuleActivation.create(rule1.getKey())); assertThat(changes).hasSize(1); QProfileDto profile2 = createProfile(rule2); - changes = activate(profile2, new RuleActivation(rule2.getKey())); + changes = activate(profile2, RuleActivation.create(rule2.getKey())); assertThat(changes).hasSize(1); changes = underTest.setParent(db.getSession(), profile2, profile1); assertThat(changes).hasSize(1); - assertThatRuleIsActivated(profile2, rule1, changes, rule1.getSeverityString(), ActiveRule.Inheritance.INHERITED, emptyMap()); + assertThatRuleIsActivated(profile2, rule1, changes, rule1.getSeverityString(), INHERITED, emptyMap()); assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); - RuleActivation activation = new RuleActivation(rule1.getKey()) - .setSeverity(BLOCKER); + RuleActivation activation = RuleActivation.create(rule1.getKey(), BLOCKER, null); changes = activate(profile2, activation); assertThat(changes).hasSize(1); assertThatRuleIsUpdated(profile2, rule1, BLOCKER, ActiveRule.Inheritance.OVERRIDES, emptyMap()); @@ -858,6 +840,212 @@ public class RuleActivatorTest { assertThatRuleIsActivated(profile2, rule2, null, rule2.getSeverityString(), null, emptyMap()); } + @Test + public void bulk_activation() { + int bulkSize = SearchOptions.MAX_LIMIT + 10 + new Random().nextInt(100); + String language = randomAlphanumeric(10); + String repositoryKey = randomAlphanumeric(10); + QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p.setLanguage(language)); + + List<RuleDto> rules = new ArrayList<>(); + IntStream.rangeClosed(1, bulkSize).forEach( + i -> rules.add(db.rules().insertRule(r -> r.setLanguage(language).setRepositoryKey(repositoryKey)))); + + verifyNoActiveRules(); + ruleIndexer.indexOnStartup(ruleIndexer.getIndexTypes()); + + RuleQuery ruleQuery = new RuleQuery() + .setRepositories(singletonList(repositoryKey)); + + BulkChangeResult bulkChangeResult = underTest.bulkActivate(db.getSession(), ruleQuery, profile, MINOR); + + assertThat(bulkChangeResult.countFailed()).isEqualTo(0); + assertThat(bulkChangeResult.countSucceeded()).isEqualTo(bulkSize); + assertThat(bulkChangeResult.getChanges()).hasSize(bulkSize); + assertThat(db.getDbClient().activeRuleDao().selectByProfile(db.getSession(), profile)).hasSize(bulkSize); + rules.stream().forEach( + r -> assertThatRuleIsActivated(profile, r.getDefinition(), null, MINOR, null, emptyMap())); + } + + @Test + public void bulk_deactivation() { + int bulkSize = SearchOptions.MAX_LIMIT + 10 + new Random().nextInt(100); + String language = randomAlphanumeric(10); + String repositoryKey = randomAlphanumeric(10); + QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p.setLanguage(language)); + + List<RuleDto> rules = new ArrayList<>(); + IntStream.rangeClosed(1, bulkSize).forEach( + i -> rules.add(db.rules().insertRule(r -> r.setLanguage(language).setRepositoryKey(repositoryKey)))); + + verifyNoActiveRules(); + ruleIndexer.indexOnStartup(ruleIndexer.getIndexTypes()); + + RuleQuery ruleQuery = new RuleQuery() + .setRepositories(singletonList(repositoryKey)); + + BulkChangeResult bulkChangeResult = underTest.bulkActivate(db.getSession(), ruleQuery, profile, MINOR); + + assertThat(bulkChangeResult.countFailed()).isEqualTo(0); + assertThat(bulkChangeResult.countSucceeded()).isEqualTo(bulkSize); + assertThat(bulkChangeResult.getChanges()).hasSize(bulkSize); + assertThat(db.getDbClient().activeRuleDao().selectByProfile(db.getSession(), profile)).hasSize(bulkSize); + + // Now deactivate all rules + bulkChangeResult = underTest.bulkDeactivate(db.getSession(), ruleQuery, profile); + + assertThat(bulkChangeResult.countFailed()).isEqualTo(0); + assertThat(bulkChangeResult.countSucceeded()).isEqualTo(bulkSize); + assertThat(bulkChangeResult.getChanges()).hasSize(bulkSize); + assertThat(db.getDbClient().activeRuleDao().selectByProfile(db.getSession(), profile)).hasSize(0); + rules.stream().forEach( + r -> assertThatRuleIsNotPresent(profile, r.getDefinition())); + } + + @Test + public void bulk_deactivation_ignores_errors() { + RuleDefinitionDto rule = createRule(); + QProfileDto parentProfile = createProfile(rule); + QProfileDto childProfile = createChildProfile(parentProfile); + + List<ActiveRuleChange> changes = activate(parentProfile, RuleActivation.create(rule.getKey())); + assertThatRuleIsActivated(parentProfile, rule, null, rule.getSeverityString(), null, emptyMap()); + assertThatRuleIsActivated(childProfile, rule, null, rule.getSeverityString(), INHERITED, emptyMap()); + + ruleIndexer.indexOnStartup(ruleIndexer.getIndexTypes()); + + RuleQuery ruleQuery = new RuleQuery() + .setQProfile(childProfile); + BulkChangeResult bulkChangeResult = underTest.bulkDeactivate(db.getSession(), ruleQuery, childProfile); + + assertThat(bulkChangeResult.countFailed()).isEqualTo(1); + assertThat(bulkChangeResult.countSucceeded()).isEqualTo(0); + assertThat(bulkChangeResult.getChanges()).hasSize(0); + assertThatRuleIsActivated(parentProfile, rule, null, rule.getSeverityString(), null, emptyMap()); + assertThatRuleIsActivated(childProfile, rule, null, rule.getSeverityString(), INHERITED, emptyMap()); + } + + @Test + public void bulk_change_severity() { + RuleDefinitionDto rule1 = createJavaRule(); + RuleDefinitionDto rule2 = createJavaRule(); + QProfileDto parentProfile = createProfile(rule1); + QProfileDto childProfile = createChildProfile(parentProfile); + QProfileDto grandchildProfile = createChildProfile(childProfile); + + activate(parentProfile, RuleActivation.create(rule1.getKey())); + activate(parentProfile, RuleActivation.create(rule2.getKey())); + + ruleIndexer.indexOnStartup(ruleIndexer.getIndexTypes()); + + RuleQuery query = new RuleQuery() + .setRuleKey(rule1.getRuleKey()) + .setQProfile(parentProfile); + BulkChangeResult result = underTest.bulkActivate(db.getSession(), query, parentProfile, "BLOCKER"); + + assertThat(result.getChanges()).hasSize(3); + assertThat(result.countSucceeded()).isEqualTo(1); + assertThat(result.countFailed()).isEqualTo(0); + + // Rule1 must be activated with BLOCKER on all profiles + assertThatRuleIsActivated(parentProfile, rule1, null, BLOCKER, null, emptyMap()); + assertThatRuleIsActivated(childProfile, rule1, null, BLOCKER, INHERITED, emptyMap()); + assertThatRuleIsActivated(grandchildProfile, rule1, null, BLOCKER, INHERITED, emptyMap()); + + // Rule2 did not changed + assertThatRuleIsActivated(parentProfile, rule2, null, rule2.getSeverityString(), null, emptyMap()); + assertThatRuleIsActivated(childProfile, rule2, null, rule2.getSeverityString(), INHERITED, emptyMap()); + assertThatRuleIsActivated(grandchildProfile, rule2, null, rule2.getSeverityString(), INHERITED, emptyMap()); + } + + @Test + public void activateOnBuiltInProfile_throws_IAE_when_profile_is_not_built_in() { + RuleDefinitionDto rule = createJavaRule(); + QProfileDto profile = createProfile(rule); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Rules profile must be a built-in profile: " + profile.getRulesProfileUuid()); + + underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), RulesProfileDto.from(profile)); + } + + @Test + public void activateOnBuiltInProfile_activate_rule_on_child_profiles() { + RuleDefinitionDto rule = createJavaRule(); + QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(), + p -> p.setLanguage(rule.getLanguage()) + .setIsBuiltIn(true)); + QProfileDto childProfile = createChildProfile(profile); + QProfileDto grandchildProfile = createChildProfile(childProfile); + + List<ActiveRuleChange> changes = underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), RulesProfileDto.from(profile)); + + assertThat(changes).hasSize(3); + assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, emptyMap()); + assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); + assertThatRuleIsActivated(grandchildProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); + } + + @Test + public void deactivateOnBuiltInProfile_throws_IAE_when_profile_is_not_built_in() { + RuleDefinitionDto rule = createJavaRule(); + QProfileDto profile = createProfile(rule); + activate(profile, RuleActivation.create(rule.getKey())); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Rules profile must be a built-in profile: " + profile.getRulesProfileUuid()); + + underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), RulesProfileDto.from(profile)); + } + + @Test + public void deactivateOnBuiltInProfile_activate_rule_on_child_profiles() { + RuleDefinitionDto rule = createJavaRule(); + QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(), + p -> p.setLanguage(rule.getLanguage()) + .setIsBuiltIn(true)); + QProfileDto childProfile = createChildProfile(profile); + QProfileDto grandchildProfile = createChildProfile(childProfile); + + List<ActiveRuleChange> changes = underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), RulesProfileDto.from(profile)); + + assertThatRuleIsActivated(profile, rule, changes, rule.getSeverityString(), null, emptyMap()); + assertThatRuleIsActivated(childProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); + assertThatRuleIsActivated(grandchildProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap()); + + changes = underTest.deactivateOnBuiltInRulesProfile(db.getSession(), RulesProfileDto.from(profile), rule.getKey(), false); + + assertThat(changes).hasSize(3); + assertThatRuleIsNotPresent(profile, rule); + assertThatRuleIsNotPresent(childProfile, rule); + assertThatRuleIsNotPresent(grandchildProfile, rule); + } + + @Test + public void delete_rule_from_all_profiles() { + RuleDefinitionDto rule = createRule(); + QProfileDto parentProfile = createProfile(rule); + QProfileDto childProfile = createChildProfile(parentProfile); + QProfileDto grandChildProfile = createChildProfile(childProfile); + + RuleActivation activation = RuleActivation.create(rule.getKey(), CRITICAL, null); + activate(parentProfile, activation); + + RuleActivation overrideActivation = RuleActivation.create(rule.getKey(), BLOCKER, null); + activate(grandChildProfile, overrideActivation); + + // Reset on parent do not change child nor grandchild + List<ActiveRuleChange> changes = underTest.delete(db.getSession(), rule); + + assertThatRuleIsNotPresent(parentProfile, rule); + assertThatRuleIsNotPresent(childProfile, rule); + assertThatRuleIsNotPresent(grandChildProfile, rule); + assertThat(changes) + .extracting(ActiveRuleChange::getType) + .containsOnly(ActiveRuleChange.Type.DEACTIVATED) + .hasSize(3); + } + private void assertThatProfileHasNoActiveRules(QProfileDto profile) { List<OrgActiveRuleDto> activeRules = db.getDbClient().activeRuleDao().selectByProfile(db.getSession(), profile); assertThat(activeRules).isEmpty(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java index e193ad12d64..1530907b0e3 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java @@ -128,7 +128,7 @@ public class RegisterRulesMediumTest { db.qualityProfileDao().insert(dbSession, profile); dbSession.commit(); dbSession.clearCache(); - RuleActivation activation = new RuleActivation(RuleTesting.XOO_X1); + RuleActivation activation = RuleActivation.create(RuleTesting.XOO_X1, null, null); TESTER.get(RuleActivator.class).activate(dbSession, activation, profile); // Restart, repo xoo still exists -> deactivate x1 @@ -160,7 +160,7 @@ public class RegisterRulesMediumTest { db.qualityProfileDao().insert(dbSession, profile); dbSession.commit(); dbSession.clearCache(); - RuleActivation activation = new RuleActivation(RuleTesting.XOO_X1); + RuleActivation activation = RuleActivation.create(RuleTesting.XOO_X1, null, null); TESTER.get(RuleActivator.class).activate(dbSession, activation, profile); dbSession.commit(); @@ -197,8 +197,7 @@ public class RegisterRulesMediumTest { db.qualityProfileDao().insert(dbSession, profile); dbSession.commit(); dbSession.clearCache(); - RuleActivation activation = new RuleActivation(RuleTesting.XOO_X1); - activation.setParameter("format", "txt"); + RuleActivation activation = RuleActivation.create(RuleTesting.XOO_X1, null, ImmutableMap.of("format", "txt")); TESTER.get(RuleActivator.class).activate(dbSession, activation, profile); dbSession.commit(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java index 889b0f51ebb..b4f3d8e912e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java @@ -502,10 +502,6 @@ public class RuleIndexTest { return db.qualityProfiles().insert(db.getDefaultOrganization(), p -> p.setLanguage("java")); } - private QProfileDto createProfile(OrganizationDto organization) { - return db.qualityProfiles().insert(organization); - } - @Test public void search_by_activation_and_inheritance() { RuleDefinitionDto rule1 = createJavaRule(); @@ -563,12 +559,28 @@ public class RuleIndexTest { verifyFacet(query, RuleIndex.FACET_ACTIVE_SEVERITIES, entry(BLOCKER, 1L), entry(CRITICAL, 1L)); } + @Test + public void facet_by_activation_severity_is_ignored_when_profile_is_not_specified() { + RuleDefinitionDto rule = createJavaRule(); + QProfileDto profile = createJavaProfile(); + db.qualityProfiles().activateRule(profile, rule); + index(); + + RuleQuery query = newRuleQuery(); + verifyNoFacet(query, RuleIndex.FACET_ACTIVE_SEVERITIES); + } + private void verifyFacet(RuleQuery query, String facet, Map.Entry<String, Long>... expectedBuckets) { SearchIdResult<RuleKey> result = underTest.search(query, new SearchOptions().addFacets(facet)); assertThat(result.getFacets().get(facet)) .containsOnly(expectedBuckets); } + private void verifyNoFacet(RuleQuery query, String facet) { + SearchIdResult<RuleKey> result = underTest.search(query, new SearchOptions().addFacets(facet)); + assertThat(result.getFacets().get(facet)).isNull(); + } + @Test public void listTags_should_return_both_system_tags_and_organization_specific_tags() { OrganizationDto organization = db.organizations().insert(); |