]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-5007 complete activation of rule
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Tue, 20 May 2014 22:51:01 +0000 (00:51 +0200)
committerSimon Brandhof <simon.brandhof@sonarsource.com>
Tue, 20 May 2014 22:51:01 +0000 (00:51 +0200)
16 files changed:
sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java
sonar-server/src/main/java/org/sonar/server/db/DbClient.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleService.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RuleActivationActions.java
sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleServiceMediumTest.java
sonar-server/src/test/java/org/sonar/server/qualityprofile/ESActiveRuleTest.java
sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java
sonar-server/src/test/java/org/sonar/server/qualityprofile/index/ActiveRuleIndexMediumTest.java
sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java
sonar-server/src/test/java/org/sonar/server/rule2/RegisterRulesMediumTest.java
sonar-server/src/test/java/org/sonar/server/rule2/RuleDataMediumTest.java
sonar-server/src/test/java/org/sonar/server/rule2/RuleServiceMediumTest.java
sonar-server/src/test/java/org/sonar/server/rule2/index/RuleIndexMediumTest.java
sonar-server/src/test/java/org/sonar/server/rule2/ws/RulesWebServiceTest.java
sonar-server/src/test/java/org/sonar/server/tester/ServerTester.java

index 1682757237a836c88606cccaf7a7d3f14712dbe5..0d95e568856f5e1b68dda63381db83041cee3d6c 100644 (file)
@@ -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<ActiveRuleKey> {
 
@@ -48,10 +47,6 @@ public class ActiveRuleDto extends Dto<ActiveRuleKey> {
   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<ActiveRuleKey> {
   }
 
   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<ActiveRuleKey> {
     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;
index 8ec9db3176074566a1ea2035b06d4ad99e3fb5ab..15d2de0afe9a0598e9d3a18cb04637b262b6bba8 100644 (file)
@@ -40,7 +40,6 @@ public class DbClient implements ServerComponent {
   private final MyBatis myBatis;
   private final Map<Class<?>, DaoComponent> daoComponents;
 
-
   public DbClient(Database db, MyBatis myBatis, DaoComponent... daoComponents) {
     this.db = db;
     this.myBatis = myBatis;
index 431204c833815bdf67eb74c76c4e9230af3c3554..88a9dab620efa977ad2ce1b5e16e682cf19e1f12 100644 (file)
@@ -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<ActiveRuleChange> changes, DbSession dbSession) {
+  private void persist(Collection<ActiveRuleChange> 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
       }
     }
   }
index 2791d17da9f0e562591b3d8d3f543ce8568ca7bf..6b60de6aceb13dc75278791a30bcf02cbd0343f8 100644 (file)
@@ -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<ActiveRuleParamDto> params, SqlSession session, UserSession userSession) {
     ProfilesManager.RuleInheritanceActions actions = new ProfilesManager.RuleInheritanceActions();
     for (ActiveRuleParamDto activeRuleParam : params) {
index daeebfddad183e0ec7139f734732fcebb1621471..5e673d3608c1022fd1e7b468ec1ba94ee7335ee4 100644 (file)
@@ -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")));
   }
 }
index 20e647eb757320cf325405d1828b807884514638..fce8c323a67c818bc5e76fd9c10af17811f21f94 100644 (file)
  */
 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<ActiveRuleDto> 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<ActiveRuleDto> 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<ActiveRuleDto> 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();
+    }
   }
 }
index 0991fff678ef7e70e40714f068e11140d91cd557..863bbb2417affa3a365b584d5182edcd7cb1ff5f 100644 (file)
@@ -225,8 +225,7 @@ public class ESActiveRuleTest {
 
   @Test
   public void bulk_index_active_rules_checking_into_db() throws IOException {
-    List<ActiveRuleDto> activeRules = newArrayList(new ActiveRuleDto().setId(1).setProfileId(10).setRuleId(1).setSeverity(Severity.MAJOR).setParentId(5)
-      .setNoteData("polop").setNoteCreatedAt(new Date()).setNoteUserLogin("godin"));
+    List<ActiveRuleDto> 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);
index e879fa662f3e2526c886152a9eda7963add7ea2d..0a92b1d0cfbcf81a5e88270f37a92ddfaa8f3387 100644 (file)
@@ -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(){
index bc90b112394ee159d918a8f1d8623e63d7dd6ada..3fdd7ab81638c077b874dac52904cca0acef0283 100644 (file)
@@ -60,7 +60,7 @@ public class ActiveRuleIndexMediumTest {
 
   @Before
   public void before() {
-    tester.clearDataStores();
+    tester.clearDbAndEs();
     dbSession = myBatis.openSession(false);
   }
 
index 125f40fafb32dc0b08d5b6c7d45cc39b84b33136..7b89c45aee1ec16a9675130053792b619c27f5cd 100644 (file)
@@ -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);
   }
 
 }
index aaa75d55226c031c904e2b820de11cef423b0928..111ecb4f6c7ef8b57c1b0f8dc827673bd03b03a1 100644 (file)
@@ -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")
index 17cbb750990ab4c52235d0abefe524e3f8e8cbdd..87c28da4bc0a484b2fbc684b85f06384685b6683 100644 (file)
@@ -57,7 +57,7 @@ public class RuleDataMediumTest {
 
   @Before
   public void before() {
-    tester.clearDataStores();
+    tester.clearDbAndEs();
     dbSession = tester.get(MyBatis.class).openSession(false);
   }
 
index d5bb11cffd43e74dfb082c8a63f3b658c6c999cc..02e20aeef1dc1573e5e699622e60528d42a3cae0 100644 (file)
@@ -67,7 +67,7 @@ public class RuleServiceMediumTest {
 
   @Before
   public void before() {
-    tester.clearDataStores();
+    tester.clearDbAndEs();
     dbSession = tester.get(MyBatis.class).openSession(false);
   }
 
index 6046b3545c6ff9cdae636c9111dc7aa5093f9b81..57c0b16dcee8568f23c6a49864b6267421f5ceef 100644 (file)
@@ -58,7 +58,7 @@ public class RuleIndexMediumTest {
 
   @Before
   public void before() {
-    tester.clearDataStores();
+    tester.clearDbAndEs();
     dbSession = myBatis.openSession(false);
   }
 
index 1b8c308dc826c1d65b5c1c26a57a08375d7c0bca..6dbc8d03e423e7b105fc00a2c5d39f449b6223a7 100644 (file)
@@ -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);
index 5e5c1d4dc281c5611aa301a0935917672c19b5db..6fa838d176577a3c8935f85a0342546e650d98ff 100644 (file)
@@ -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();
   }