From: Simon Brandhof Date: Tue, 20 May 2014 22:51:01 +0000 (+0200) Subject: SONAR-5007 complete activation of rule X-Git-Tag: 4.4-RC1~959 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=dafeac5cf4dd8dba0133e783651ef25a6a76e654;p=sonarqube.git SONAR-5007 complete activation of rule --- diff --git a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java index 1682757237a..0d95e568856 100644 --- a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java +++ b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java @@ -31,7 +31,6 @@ import org.sonar.core.rule.SeverityUtil; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import javax.persistence.Transient; -import java.util.Date; public class ActiveRuleDto extends Dto { @@ -48,10 +47,6 @@ public class ActiveRuleDto extends Dto { private Integer ruleId; private Integer severity; private String inheritance; - private Date noteCreatedAt; - private Date noteUpdatedAt; - private String noteUserLogin; - private String noteData; @Deprecated public ActiveRuleDto setKey(ActiveRuleKey key) { @@ -63,8 +58,8 @@ public class ActiveRuleDto extends Dto { } public ActiveRuleKey getKey() { - return ActiveRuleKey.of(QualityProfileKey.of(this.profile, this.language), - RuleKey.of(this.repository, this.rule)); + return ActiveRuleKey.of(QualityProfileKey.of(this.profile, this.language), + RuleKey.of(this.repository, this.rule)); } // This field do not exists in db, it's only retrieve by joins @@ -128,47 +123,6 @@ public class ActiveRuleDto extends Dto { return this; } - @CheckForNull - public Date getNoteCreatedAt() { - return noteCreatedAt; - } - - public ActiveRuleDto setNoteCreatedAt(@Nullable Date noteCreatedAt) { - this.noteCreatedAt = noteCreatedAt; - return this; - } - - @CheckForNull - public Date getNoteUpdatedAt() { - return noteUpdatedAt; - } - - public ActiveRuleDto setNoteUpdatedAt(@Nullable Date noteUpdatedAt) { - this.noteUpdatedAt = noteUpdatedAt; - return this; - } - - @CheckForNull - public String getNoteUserLogin() { - return noteUserLogin; - } - - public ActiveRuleDto setNoteUserLogin(@Nullable String noteUserLogin) { - this.noteUserLogin = noteUserLogin; - return this; - } - - @CheckForNull - public String getNoteData() { - return noteData; - } - - public ActiveRuleDto setNoteData(@Nullable String noteData) { - this.noteData = noteData; - return this; - } - - @CheckForNull public Integer getParentId() { return parentId; diff --git a/sonar-server/src/main/java/org/sonar/server/db/DbClient.java b/sonar-server/src/main/java/org/sonar/server/db/DbClient.java index 8ec9db31760..15d2de0afe9 100644 --- a/sonar-server/src/main/java/org/sonar/server/db/DbClient.java +++ b/sonar-server/src/main/java/org/sonar/server/db/DbClient.java @@ -40,7 +40,6 @@ public class DbClient implements ServerComponent { private final MyBatis myBatis; private final Map, DaoComponent> daoComponents; - public DbClient(Database db, MyBatis myBatis, DaoComponent... daoComponents) { this.db = db; this.myBatis = myBatis; 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 431204c8338..88a9dab620e 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 @@ -74,7 +74,7 @@ public class ActiveRuleService implements ServerComponent { // TODO apply changes to children - persist(changes, dbSession); + persist(changes, context, dbSession); dbSession.commit(); // TODO filter changes without any differences @@ -85,11 +85,10 @@ public class ActiveRuleService implements ServerComponent { } } - private void persist(Collection changes, DbSession dbSession) { + private void persist(Collection changes, RuleActivationContext context, DbSession dbSession) { for (ActiveRuleChange change : changes) { if (change.getType() == ActiveRuleChange.Type.ACTIVATED) { - ActiveRuleDto activeRule = ActiveRuleDto.createFor(null, null /* TODO */) - .setKey(change.getKey()) + ActiveRuleDto activeRule = ActiveRuleDto.createFor(context.profile(), context.rule()) .setSeverity(change.getSeverity()); db.activeRuleDao().insert(activeRule, dbSession); @@ -99,7 +98,10 @@ public class ActiveRuleService implements ServerComponent { db.activeRuleDao().deleteByKey(change.getKey(), dbSession); } else if (change.getType() == ActiveRuleChange.Type.UPDATED) { - + ActiveRuleDto activeRule = context.activeRule(); + activeRule.setSeverity(change.getSeverity()); + db.activeRuleDao().update(activeRule, dbSession); + // TODO insert activeruelparams } } } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java index 2791d17da9f..6b60de6aceb 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java @@ -374,51 +374,6 @@ public class QProfileActiveRuleOperations implements ServerComponent { } } - public void updateActiveRuleNote(int activeRuleId, String note, UserSession userSession) { - validatePermission(userSession); - DbSession session = myBatis.openSession(false); - - try { - ActiveRuleDto activeRule = findActiveRuleNotNull(activeRuleId, session); - String sanitizedNote = Strings.emptyToNull(note); - if (sanitizedNote != null) { - Date now = new Date(system.now()); - if (activeRule.getNoteData() == null) { - activeRule.setNoteCreatedAt(now); - activeRule.setNoteUserLogin(userSession.login()); - } - activeRule.setNoteUpdatedAt(now); - activeRule.setNoteData(note); - activeRuleDao.update(activeRule, session); - session.commit(); - - reindexActiveRule(activeRule, session); - } - } finally { - MyBatis.closeQuietly(session); - } - } - - public void deleteActiveRuleNote(int activeRuleId, UserSession userSession) { - validatePermission(userSession); - - DbSession session = myBatis.openSession(false); - try { - ActiveRuleDto activeRule = findActiveRuleNotNull(activeRuleId, session); - - activeRule.setNoteData(null); - activeRule.setNoteUserLogin(null); - activeRule.setNoteCreatedAt(null); - activeRule.setNoteUpdatedAt(null); - activeRuleDao.update(activeRule, session); - session.commit(); - - reindexActiveRule(activeRule, session); - } finally { - MyBatis.closeQuietly(session); - } - } - private void notifyParamsDeleted(ActiveRuleDto activeRule, List params, SqlSession session, UserSession userSession) { ProfilesManager.RuleInheritanceActions actions = new ProfilesManager.RuleInheritanceActions(); for (ActiveRuleParamDto activeRuleParam : params) { diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RuleActivationActions.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RuleActivationActions.java index daeebfddad1..5e673d3608c 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RuleActivationActions.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RuleActivationActions.java @@ -84,25 +84,15 @@ public class RuleActivationActions implements ServerComponent { } private void defineActiveRuleKeyParameters(WebService.NewAction action) { - action.createParam("profile_lang") - .setDescription("Profile language") + action.createParam("profile_key") + .setDescription("Key of Quality profile") .setRequired(true) - .setExampleValue("java"); - - action.createParam("profile_name") - .setDescription("Profile name") - .setRequired(true) - .setExampleValue("My profile"); - - action.createParam("rule_repo") - .setDescription("Rule repository") - .setRequired(true) - .setExampleValue("squid"); + .setExampleValue("Sonar way:java"); action.createParam("rule_key") - .setDescription("Rule key") + .setDescription("Key of the rule to activate") .setRequired(true) - .setExampleValue("AvoidCycles"); + .setExampleValue("squid:AvoidCycles"); } private void activate(Request request, Response response) throws Exception { @@ -122,7 +112,7 @@ public class RuleActivationActions implements ServerComponent { private ActiveRuleKey readKey(Request request) { return ActiveRuleKey.of( - QualityProfileKey.of(request.mandatoryParam("profile_name"), request.mandatoryParam("profile_lang")), - RuleKey.of(request.mandatoryParam("rule_repo"), request.mandatoryParam("rule_key"))); + QualityProfileKey.parse(request.mandatoryParam("profile_key")), + RuleKey.parse(request.mandatoryParam("rule_key"))); } } 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 20e647eb757..fce8c323a67 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 @@ -19,20 +19,166 @@ */ package org.sonar.server.qualityprofile; -import org.junit.ClassRule; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.rule.Severity; +import org.sonar.api.server.rule.RuleParamType; +import org.sonar.api.server.rule.RulesDefinition; +import org.sonar.core.permission.GlobalPermissions; +import org.sonar.core.persistence.DbSession; +import org.sonar.core.qualityprofile.db.ActiveRuleDto; +import org.sonar.core.qualityprofile.db.ActiveRuleKey; +import org.sonar.core.qualityprofile.db.QualityProfileDto; +import org.sonar.core.qualityprofile.db.QualityProfileKey; +import org.sonar.server.db.DbClient; +import org.sonar.server.qualityprofile.index.ActiveRuleIndex; import org.sonar.server.tester.ServerTester; +import org.sonar.server.user.MockUserSession; + +import java.util.List; import static org.fest.assertions.Assertions.assertThat; public class ActiveRuleServiceMediumTest { - @ClassRule - public static ServerTester tester = new ServerTester(); + @Rule + public ServerTester tester = new ServerTester().addComponents(XooRulesDefinition.class); + + DbClient dbClient; + DbSession dbSession; + ActiveRuleService service; + + @Before + public void before() { + dbClient = tester.get(DbClient.class); + dbSession = dbClient.openSession(false); + service = tester.get(ActiveRuleService.class); + + // create quality profile + dbClient.qualityProfileDao().insert(QualityProfileDto.createFor("MyProfile", "xoo"), dbSession); + dbSession.commit(); + } @Test public void activate() throws Exception { + MockUserSession.set().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN).setLogin("marius"); + QualityProfileKey profileKey = QualityProfileKey.of("MyProfile", "xoo"); + RuleActivation activation = new RuleActivation(ActiveRuleKey.of(profileKey, RuleKey.of("xoo", "x1"))); + activation.setSeverity(Severity.BLOCKER); + service.activate(activation); + + // verify db + List activeRuleDtos = dbClient.activeRuleDao().findByProfileKey(profileKey, dbSession); + assertThat(activeRuleDtos).hasSize(1); + assertThat(activeRuleDtos.get(0).getSeverityString()).isEqualTo(Severity.BLOCKER); + assertThat(activeRuleDtos.get(0).getInheritance()).isNull(); + + // verify es + ActiveRuleIndex index = tester.get(ActiveRuleIndex.class); + index.refresh(); + ActiveRule activeRule = index.getByKey(activation.getKey()); + assertThat(activeRule).isNotNull(); + assertThat(activeRule.severity()).isEqualTo(Severity.BLOCKER); + assertThat(activeRule.inheritance()).isEqualTo(ActiveRule.Inheritance.NONE); + } + + @Test + public void activate_with_default_severity() throws Exception { + MockUserSession.set().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN).setLogin("marius"); + QualityProfileKey profileKey = QualityProfileKey.of("MyProfile", "xoo"); + RuleActivation activation = new RuleActivation(ActiveRuleKey.of(profileKey, RuleKey.of("xoo", "x1"))); + service.activate(activation); + + // verify db + List activeRuleDtos = dbClient.activeRuleDao().findByProfileKey(profileKey, dbSession); + assertThat(activeRuleDtos).hasSize(1); + assertThat(activeRuleDtos.get(0).getSeverityString()).isEqualTo(Severity.MINOR); + assertThat(activeRuleDtos.get(0).getInheritance()).isNull(); + + // verify es + ActiveRuleIndex index = tester.get(ActiveRuleIndex.class); + index.refresh(); + ActiveRule activeRule = index.getByKey(activation.getKey()); + assertThat(activeRule).isNotNull(); + assertThat(activeRule.severity()).isEqualTo(Severity.MINOR); + assertThat(activeRule.inheritance()).isEqualTo(ActiveRule.Inheritance.NONE); + } + + @Test + public void update_activation_severity_and_parameters() throws Exception { + // initial activation + MockUserSession.set().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN).setLogin("marius"); + QualityProfileKey profileKey = QualityProfileKey.of("MyProfile", "xoo"); + RuleActivation activation = new RuleActivation(ActiveRuleKey.of(profileKey, RuleKey.of("xoo", "x1"))); + activation.setSeverity(Severity.BLOCKER); + service.activate(activation); + + // update + RuleActivation update = new RuleActivation(ActiveRuleKey.of(profileKey, RuleKey.of("xoo", "x1"))); + update.setSeverity(Severity.CRITICAL); + 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(); + + // verify es + ActiveRuleIndex index = tester.get(ActiveRuleIndex.class); + index.refresh(); + ActiveRule activeRule = index.getByKey(activation.getKey()); + assertThat(activeRule).isNotNull(); + assertThat(activeRule.severity()).isEqualTo(Severity.CRITICAL); + assertThat(activeRule.inheritance()).isEqualTo(ActiveRule.Inheritance.NONE); + } + @Test + @Ignore + public void activate_with_params() throws Exception { + + } + + @Test + @Ignore + public void activate_with_default_params() throws Exception { + + } + + @Test + @Ignore + public void fail_to_activate_if_not_granted() throws Exception { + + } + + @Test + @Ignore + public void fail_to_activate_if_different_languages() throws Exception { + // profile and rule have different languages + } + + @Test + @Ignore + public void fail_to_activate_if_invalid_parameter() throws Exception { + + } + public static class XooRulesDefinition implements RulesDefinition { + @Override + public void define(Context context) { + NewRepository repository = context.createRepository("xoo", "xoo").setName("Xoo Repo"); + repository.createRule("x1") + .setName("x1 name") + .setHtmlDescription("x1 desc") + .setSeverity(Severity.MINOR) + .createParam("max") + .setDefaultValue("10") + .setType(RuleParamType.INTEGER) + .setDescription("Maximum"); + repository.done(); + } } } diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ESActiveRuleTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ESActiveRuleTest.java index 0991fff678e..863bbb2417a 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ESActiveRuleTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ESActiveRuleTest.java @@ -225,8 +225,7 @@ public class ESActiveRuleTest { @Test public void bulk_index_active_rules_checking_into_db() throws IOException { - List activeRules = newArrayList(new ActiveRuleDto().setId(1).setProfileId(10).setRuleId(1).setSeverity(Severity.MAJOR).setParentId(5) - .setNoteData("polop").setNoteCreatedAt(new Date()).setNoteUserLogin("godin")); + List activeRules = newArrayList(new ActiveRuleDto().setId(1).setProfileId(10).setRuleId(1).setSeverity(Severity.MAJOR).setParentId(5)); DbSession session = mock(DbSession.class); when(myBatis.openSession(false)).thenReturn(session); diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java index e879fa662f3..0a92b1d0cfb 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java @@ -57,7 +57,7 @@ public class RegisterQualityProfilesMediumTest { @org.junit.Rule public ServerTester serverTester = new ServerTester().addComponents(XooRulesDefinition.class, XooProfileDefinition.class); - private DbSession session; + DbSession session; @Before public void setup(){ diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/index/ActiveRuleIndexMediumTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/index/ActiveRuleIndexMediumTest.java index bc90b112394..3fdd7ab8163 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/index/ActiveRuleIndexMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/index/ActiveRuleIndexMediumTest.java @@ -60,7 +60,7 @@ public class ActiveRuleIndexMediumTest { @Before public void before() { - tester.clearDataStores(); + tester.clearDbAndEs(); dbSession = myBatis.openSession(false); } diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java index 125f40fafb3..7b89c45aee1 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java @@ -65,7 +65,7 @@ public class QProfilesWsTest { WebService.Action restoreProfiles = controller.action("activate_rule"); assertThat(restoreProfiles).isNotNull(); assertThat(restoreProfiles.isPost()).isTrue(); - assertThat(restoreProfiles.params()).hasSize(6); + assertThat(restoreProfiles.params()).hasSize(4); } @Test @@ -73,7 +73,7 @@ public class QProfilesWsTest { WebService.Action restoreProfiles = controller.action("deactivate_rule"); assertThat(restoreProfiles).isNotNull(); assertThat(restoreProfiles.isPost()).isTrue(); - assertThat(restoreProfiles.params()).hasSize(4); + assertThat(restoreProfiles.params()).hasSize(2); } } diff --git a/sonar-server/src/test/java/org/sonar/server/rule2/RegisterRulesMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule2/RegisterRulesMediumTest.java index aaa75d55226..111ecb4f6c7 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule2/RegisterRulesMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule2/RegisterRulesMediumTest.java @@ -105,8 +105,6 @@ public class RegisterRulesMediumTest { .setType(RuleParamType.BOOLEAN) .setDescription("Accept whitespaces on the line"); - - repository.createRule("x2") .setName("x2 name") .setHtmlDescription("x2 desc") diff --git a/sonar-server/src/test/java/org/sonar/server/rule2/RuleDataMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule2/RuleDataMediumTest.java index 17cbb750990..87c28da4bc0 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule2/RuleDataMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule2/RuleDataMediumTest.java @@ -57,7 +57,7 @@ public class RuleDataMediumTest { @Before public void before() { - tester.clearDataStores(); + tester.clearDbAndEs(); dbSession = tester.get(MyBatis.class).openSession(false); } diff --git a/sonar-server/src/test/java/org/sonar/server/rule2/RuleServiceMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule2/RuleServiceMediumTest.java index d5bb11cffd4..02e20aeef1d 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule2/RuleServiceMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule2/RuleServiceMediumTest.java @@ -67,7 +67,7 @@ public class RuleServiceMediumTest { @Before public void before() { - tester.clearDataStores(); + tester.clearDbAndEs(); dbSession = tester.get(MyBatis.class).openSession(false); } diff --git a/sonar-server/src/test/java/org/sonar/server/rule2/index/RuleIndexMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule2/index/RuleIndexMediumTest.java index 6046b3545c6..57c0b16dcee 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule2/index/RuleIndexMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule2/index/RuleIndexMediumTest.java @@ -58,7 +58,7 @@ public class RuleIndexMediumTest { @Before public void before() { - tester.clearDataStores(); + tester.clearDbAndEs(); dbSession = myBatis.openSession(false); } diff --git a/sonar-server/src/test/java/org/sonar/server/rule2/ws/RulesWebServiceTest.java b/sonar-server/src/test/java/org/sonar/server/rule2/ws/RulesWebServiceTest.java index 1b8c308dc82..6dbc8d03e42 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule2/ws/RulesWebServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule2/ws/RulesWebServiceTest.java @@ -62,7 +62,7 @@ public class RulesWebServiceTest { @Before public void setUp() throws Exception { - tester.clearDataStores(); + tester.clearDbAndEs(); ruleDao = tester.get(RuleDao.class); ws = tester.get(RulesWebService.class); wsTester = new WsTester(ws); diff --git a/sonar-server/src/test/java/org/sonar/server/tester/ServerTester.java b/sonar-server/src/test/java/org/sonar/server/tester/ServerTester.java index 5e5c1d4dc28..6fa838d1765 100644 --- a/sonar-server/src/test/java/org/sonar/server/tester/ServerTester.java +++ b/sonar-server/src/test/java/org/sonar/server/tester/ServerTester.java @@ -157,7 +157,7 @@ public class ServerTester extends ExternalResource { /** * Truncate all db tables and es indices. Can be executed only if ServerTester is started. */ - public void clearDataStores() { + public void clearDbAndEs() { checkStarted(); get(DataStoreCleanup.class).clearAll(); }