From 7e9af4989431e3007108d38cce9b8f1fbf1d96f5 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 21 May 2014 02:50:46 +0200 Subject: [PATCH] SONAR-5007 clear preview-db cache on qprofile changes --- .../qualityprofile/ActiveRuleService.java | 30 ++++- .../ActiveRuleServiceMediumTest.java | 108 ++++++++++++++++-- 2 files changed, 128 insertions(+), 10 deletions(-) diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleService.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleService.java index 1004e79d519..a32aee5d980 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleService.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleService.java @@ -26,6 +26,7 @@ import org.sonar.api.ServerComponent; import org.sonar.api.server.rule.RuleParamType; import org.sonar.core.permission.GlobalPermissions; import org.sonar.core.persistence.DbSession; +import org.sonar.core.preview.PreviewCache; import org.sonar.core.qualityprofile.db.ActiveRuleDto; import org.sonar.core.qualityprofile.db.ActiveRuleKey; import org.sonar.core.qualityprofile.db.ActiveRuleParamDto; @@ -47,12 +48,15 @@ public class ActiveRuleService implements ServerComponent { private final DbClient db; private final TypeValidations typeValidations; private final RuleActivationContextFactory contextFactory; + private final PreviewCache previewCache; public ActiveRuleService(DbClient db, - RuleActivationContextFactory contextFactory, TypeValidations typeValidations) { + RuleActivationContextFactory contextFactory, TypeValidations typeValidations, + PreviewCache previewCache) { this.db = db; this.contextFactory = contextFactory; this.typeValidations = typeValidations; + this.previewCache = previewCache; } /** @@ -82,6 +86,7 @@ public class ActiveRuleService implements ServerComponent { changes.add(change); persist(changes, context, dbSession); dbSession.commit(); + previewCache.reportGlobalModification(); // TODO filter changes without any differences return changes; @@ -134,11 +139,30 @@ public class ActiveRuleService implements ServerComponent { } /** - * Deactivate a rule on a Quality profile. Does nothing if the rule is not activated. + * Deactivate a rule on a Quality profile. Does nothing if the rule is not activated, but + * fails (fast) if the rule or the profile does not exist. */ public List deactivate(ActiveRuleKey key) { verifyPermission(UserSession.get()); - throw new UnsupportedOperationException("TODO"); + DbSession dbSession = db.openSession(false); + List changes = Lists.newArrayList(); + try { + RuleActivationContext context = contextFactory.create(key, dbSession); + ActiveRuleChange change; + if (context.activeRule() == null) { + // not activated ! + return changes; + } + change = new ActiveRuleChange(ActiveRuleChange.Type.DEACTIVATED, key); + changes.add(change); + persist(changes, context, dbSession); + dbSession.commit(); + previewCache.reportGlobalModification(); + return changes; + + } finally { + dbSession.close(); + } } public List bulkActivate(BulkRuleActivation activation) { diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleServiceMediumTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleServiceMediumTest.java index dda96399720..88a040594b5 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleServiceMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleServiceMediumTest.java @@ -20,6 +20,7 @@ package org.sonar.server.qualityprofile; import org.junit.Before; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.sonar.api.rule.RuleKey; @@ -155,6 +156,43 @@ public class ActiveRuleServiceMediumTest { assertThat(activeRule.params().get("max")).isEqualTo("42"); } + @Test + @Ignore + public void update_activation_but_new_parameter() throws Exception { + // initial activation + grantPermission(); + RuleActivation activation = new RuleActivation(ActiveRuleKey.of(profileKey, RuleKey.of("xoo", "x1"))); + activation.setSeverity(Severity.BLOCKER); + service.activate(activation); + // TODO delete activeruleparam max + + + // update + RuleActivation update = new RuleActivation(ActiveRuleKey.of(profileKey, RuleKey.of("xoo", "x1"))); + update.setSeverity(Severity.CRITICAL); + update.setParameter("max", "42"); + // contrary to activerule, the param 'max' is supposed to be inserted but not updated + service.activate(update); + + // verify db + List activeRuleDtos = dbClient.activeRuleDao().findByProfileKey(profileKey, dbSession); + assertThat(activeRuleDtos).hasSize(1); + assertThat(activeRuleDtos.get(0).getSeverityString()).isEqualTo(Severity.CRITICAL); + assertThat(activeRuleDtos.get(0).getInheritance()).isNull(); + List params = dbClient.activeRuleDao().findParamsByActiveRule(activeRuleDtos.get(0), dbSession); + assertThat(params).hasSize(1); + assertThat(params.get(0).getValue()).isEqualTo("42"); + + // verify es + index.refresh(); + ActiveRule activeRule = index.getByKey(activation.getKey()); + assertThat(activeRule).isNotNull(); + assertThat(activeRule.severity()).isEqualTo(Severity.CRITICAL); + assertThat(activeRule.inheritance()).isEqualTo(ActiveRule.Inheritance.NONE); + assertThat(activeRule.params()).hasSize(1); + assertThat(activeRule.params().get("max")).isEqualTo("42"); + } + @Test public void revert_activation_to_default_severity_and_parameters() throws Exception { // initial activation @@ -196,7 +234,7 @@ public class ActiveRuleServiceMediumTest { service.activate(activation); fail(); } catch (ForbiddenException e) { - verifyZeroActiveRules(activation); + verifyZeroActiveRules(activation.getKey()); } } @@ -211,7 +249,7 @@ public class ActiveRuleServiceMediumTest { fail(); } catch (IllegalArgumentException e) { assertThat(e).hasMessage("Rule squid:j1 and profile MyProfile:xoo have different languages"); - verifyZeroActiveRules(activation); + verifyZeroActiveRules(activation.getKey()); } } @@ -226,7 +264,7 @@ public class ActiveRuleServiceMediumTest { fail(); } catch (IllegalArgumentException e) { assertThat(e).hasMessage("Rule not found: xoo:x3"); - verifyZeroActiveRules(activation); + verifyZeroActiveRules(activation.getKey()); } } @@ -255,7 +293,62 @@ public class ActiveRuleServiceMediumTest { fail(); } catch (BadRequestException e) { assertThat(e.l10nKey()).isEqualTo("errors.type.notInteger"); - verifyZeroActiveRules(activation); + verifyZeroActiveRules(activation.getKey()); + } + } + + @Test + @Ignore + public void deactivate() throws Exception { + // activation + grantPermission(); + ActiveRuleKey key = ActiveRuleKey.of(profileKey, RuleKey.of("xoo", "x1")); + RuleActivation activation = new RuleActivation(key); + activation.setSeverity(Severity.BLOCKER); + activation.setParameter("max", "7"); + service.activate(activation); + + // deactivation + service.deactivate(key); + + verifyZeroActiveRules(key); + } + + @Test + public void ignore_deactivation_if_rule_not_activated() throws Exception { + grantPermission(); + + // deactivation + ActiveRuleKey key = ActiveRuleKey.of(profileKey, RuleKey.of("xoo", "x1")); + service.deactivate(key); + + verifyZeroActiveRules(key); + } + + @Test + public void deactivation_fails_if_rule_not_found() throws Exception { + grantPermission(); + ActiveRuleKey key = ActiveRuleKey.of(profileKey, RuleKey.of("xoo", "x3")); + try { + service.deactivate(key); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage("Rule not found: xoo:x3"); + verifyZeroActiveRules(key); + } + } + + @Test + @Ignore + public void deactivation_fails_if_profile_not_found() throws Exception { + grantPermission(); + ActiveRuleKey key = ActiveRuleKey.of(QualityProfileKey.of("other", "js"), RuleKey.of("xoo", "x1")); + try { + service.deactivate(key); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage("Quality profile not found: other:js"); + verifyZeroActiveRules(key); } } @@ -263,14 +356,15 @@ public class ActiveRuleServiceMediumTest { MockUserSession.set().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN).setLogin("marius"); } - private void verifyZeroActiveRules(RuleActivation activation) { + private void verifyZeroActiveRules(ActiveRuleKey key) { // verify db - List activeRuleDtos = dbClient.activeRuleDao().findByProfileKey(activation.getKey().qProfile(), dbSession); + List activeRuleDtos = dbClient.activeRuleDao().findByProfileKey(key.qProfile(), dbSession); assertThat(activeRuleDtos).isEmpty(); + //TODO test params // verify es index.refresh(); - ActiveRule activeRule = index.getByKey(activation.getKey()); + ActiveRule activeRule = index.getByKey(key); assertThat(activeRule).isNull(); } -- 2.39.5