From: Simon Brandhof <simon.brandhof@gmail.com>
Date: Mon, 22 Jul 2013 15:58:49 +0000 (+0200)
Subject: SONAR-4519 The rule removal phase at startup is not reentrant
X-Git-Tag: 3.7~31
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=a14f013dfcf360b5b75d1f0c1b3ed0041be1f81b;p=sonarqube.git

SONAR-4519 The rule removal phase at startup is not reentrant
---

diff --git a/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java b/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java
index 4c102da2a49..365df45b72f 100644
--- a/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java
+++ b/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java
@@ -101,8 +101,7 @@ public class ProfilesManager extends BaseDao {
    * Deactivate all active rules from profiles using a rule, then remove then.
    * @param ruleId
    */
-  public void removeActivatedRules(int ruleId) {
-    Rule rule = getSession().getEntity(Rule.class, ruleId);
+  public void removeActivatedRules(Rule rule) {
     List<RulesProfile> profiles = getSession().createQuery("FROM " + RulesProfile.class.getSimpleName()).getResultList();
     for (RulesProfile profile : profiles) {
       List<ActiveRule> activeRulesToRemove = newArrayList();
diff --git a/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java b/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java
index 79a340d316b..2078f996fd4 100644
--- a/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java
+++ b/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java
@@ -65,17 +65,18 @@ public final class RegisterRules {
   }
 
   public void start() {
-    TimeProfiler profiler = new TimeProfiler();
     DatabaseSession session = sessionFactory.getSession();
     RulesByRepository existingRules = new RulesByRepository(findAllRules(session));
 
-    List<Rule> registeredRules = registerRules(existingRules, profiler, session);
+    List<Rule> registeredRules = registerRules(existingRules, session);
+
+    LOG.info("Removing deprecated rules");
     disableDeprecatedRules(existingRules, registeredRules, session);
     disableDeprecatedRepositories(existingRules, session);
 
     session.commit();
 
-    notifyForRemovedRules(existingRules);
+    //notifyForRemovedRules(existingRules);
   }
 
   private List<Rule> findAllRules(DatabaseSession session) {
@@ -85,7 +86,8 @@ public final class RegisterRules {
       .getResultList();
   }
 
-  private List<Rule> registerRules(RulesByRepository existingRules, TimeProfiler profiler, DatabaseSession session) {
+  private List<Rule> registerRules(RulesByRepository existingRules, DatabaseSession session) {
+    TimeProfiler profiler = new TimeProfiler();
     List<Rule> registeredRules = newArrayList();
     for (RuleRepository repository : repositories) {
       profiler.start("Register rules [" + repository.getKey() + "/" + StringUtils.defaultString(repository.getLanguage(), "-") + "]");
@@ -229,7 +231,7 @@ public final class RegisterRules {
   private void disableDeprecatedRules(RulesByRepository existingRules, List<Rule> registeredRules, DatabaseSession session) {
     for (Rule rule : existingRules.rules()) {
       if (!registeredRules.contains(rule)) {
-        disable(rule, existingRules, session);
+        disable(rule, session);
       }
     }
   }
@@ -242,67 +244,45 @@ public final class RegisterRules {
         }
       })) {
         for (Rule rule : existingRules.get(repositoryKey)) {
-          disable(rule, existingRules, session);
+          disable(rule, session);
         }
       }
     }
   }
 
-  private void disable(Rule rule, RulesByRepository existingRules, DatabaseSession session) {
+  private void disable(Rule rule, DatabaseSession session) {
     if (!rule.getStatus().equals(Rule.STATUS_REMOVED)) {
-      LOG.debug("Disable rule " + rule);
+      LOG.info("Removing rule " + rule.ruleKey());
+      profilesManager.removeActivatedRules(rule);
+      rule = session.reattach(Rule.class, rule.getId());
       rule.setStatus(Rule.STATUS_REMOVED);
       rule.setUpdatedAt(new Date());
-      session.saveWithoutFlush(rule);
-      existingRules.addRuleToRemove(rule);
-    }
-  }
-
-  private void notifyForRemovedRules(RulesByRepository existingRules) {
-    for (Rule rule : existingRules.getRulesToRemove()) {
-      profilesManager.removeActivatedRules(rule.getId());
+      session.save(rule);
+      session.commit();
     }
   }
 
   static class RulesByRepository {
     Multimap<String, Rule> ruleRepositoryList;
-    List<Rule> rulesToRemove;
 
-    public RulesByRepository() {
+    RulesByRepository(List<Rule> rules) {
       ruleRepositoryList = ArrayListMultimap.create();
-      rulesToRemove = newArrayList();
-    }
-
-    public RulesByRepository(List<Rule> rules) {
-      this();
       for (Rule rule : rules) {
         ruleRepositoryList.put(rule.getRepositoryKey(), rule);
       }
     }
 
-    public void add(Rule rule) {
-      ruleRepositoryList.put(rule.getRepositoryKey(), rule);
-    }
-
-    public Collection<Rule> get(String repositoryKey) {
+    Collection<Rule> get(String repositoryKey) {
       return ruleRepositoryList.get(repositoryKey);
     }
 
-    public Collection<String> repositories() {
+    Collection<String> repositories() {
       return ruleRepositoryList.keySet();
     }
 
-    public Collection<Rule> rules() {
+    Collection<Rule> rules() {
       return ruleRepositoryList.values();
     }
-
-    public void addRuleToRemove(Rule rule) {
-      rulesToRemove.add(rule);
-    }
-
-    public List<Rule> getRulesToRemove() {
-      return rulesToRemove;
-    }
   }
 
 }
diff --git a/sonar-server/src/test/java/org/sonar/server/configuration/RuleChangeTest.java b/sonar-server/src/test/java/org/sonar/server/configuration/RuleChangeTest.java
index 6009e5eb0ac..25a9ba2e7df 100644
--- a/sonar-server/src/test/java/org/sonar/server/configuration/RuleChangeTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/configuration/RuleChangeTest.java
@@ -21,6 +21,7 @@ package org.sonar.server.configuration;
 
 import org.junit.Before;
 import org.junit.Test;
+import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.RulePriority;
 import org.sonar.jpa.test.AbstractDbUnitTestCase;
 
@@ -91,7 +92,8 @@ public class RuleChangeTest extends AbstractDbUnitTestCase {
   @Test
   public void testRemoveActivatedRules() {
     setupData("initialData");
-    profilesManager.removeActivatedRules(1);
+    Rule rule = getSession().reattach(Rule.class, 1);
+    profilesManager.removeActivatedRules(rule);
     checkTables("removeActivatedRules", new String[]{"change_date"}, "active_rule_changes", "active_rules");
   }
 
diff --git a/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java b/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java
index abadc622f82..a3c309ccdc9 100644
--- a/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java
@@ -107,7 +107,7 @@ public class RegisterRulesTest extends AbstractDbUnitTestCase {
     setupData("shared");
     task.start();
 
-    verify(profilesManager).removeActivatedRules(anyInt());
+    verify(profilesManager).removeActivatedRules(any(Rule.class));
   }
 
   @Test