]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-4831 Fix issue when dealing with removed rules
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Wed, 27 Nov 2013 15:28:08 +0000 (16:28 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Wed, 27 Nov 2013 15:28:29 +0000 (16:28 +0100)
sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtFinder.java
sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtManager.java
sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtFinderTest.java
sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtManagerTest.java
sonar-server/src/main/webapp/WEB-INF/app/models/characteristic_property.rb
sonar-server/src/main/webapp/WEB-INF/app/models/quality_model.rb

index 58148c866e39416eb79e57b552d8830f50534939..def60839db31ee6789ebd65ae65acfa2ca49a47d 100644 (file)
 
 package org.sonar.core.technicaldebt;
 
+import com.google.common.collect.Maps;
 import org.sonar.api.BatchComponent;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rules.Rule;
+import org.sonar.api.rules.RuleFinder;
+import org.sonar.api.rules.RuleQuery;
 import org.sonar.api.technicaldebt.Characteristic;
-import org.sonar.core.rule.DefaultRuleFinder;
 import org.sonar.core.technicaldebt.db.CharacteristicDao;
 import org.sonar.core.technicaldebt.db.CharacteristicDto;
 
@@ -33,15 +35,14 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 
-import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Maps.newHashMap;
 
 public class TechnicalDebtFinder implements ServerComponent, BatchComponent {
 
   private final CharacteristicDao dao;
-  private final DefaultRuleFinder ruleFinder;
+  private final RuleFinder ruleFinder;
 
-  public TechnicalDebtFinder(CharacteristicDao dao, DefaultRuleFinder ruleFinder) {
+  public TechnicalDebtFinder(CharacteristicDao dao, RuleFinder ruleFinder) {
     this.dao = dao;
     this.ruleFinder = ruleFinder;
   }
@@ -50,9 +51,21 @@ public class TechnicalDebtFinder implements ServerComponent, BatchComponent {
     TechnicalDebtModel model = new TechnicalDebtModel();
     List<CharacteristicDto> dtos = dao.selectEnabledCharacteristics();
     Map<Integer, Characteristic> characteristicsById = newHashMap();
-    List<Integer> ruleIds = newArrayList();
 
-    // Root characteristics
+    addRootCharacteristics(model, dtos, characteristicsById);
+    addCharacteristics(model, dtos, characteristicsById);
+    addRequirements(model, dtos, characteristicsById);
+    return model;
+  }
+
+  public TechnicalDebtModel findRootCharacteristics() {
+    TechnicalDebtModel model = new TechnicalDebtModel();
+    List<CharacteristicDto> dtos = dao.selectEnabledRootCharacteristics();
+    addRootCharacteristics(model, dtos, Maps.<Integer, Characteristic>newHashMap());
+    return model;
+  }
+
+  private void addRootCharacteristics(TechnicalDebtModel model, List<CharacteristicDto> dtos, Map<Integer, Characteristic> characteristicsById){
     for (CharacteristicDto dto : dtos) {
       if (dto.getParentId() == null) {
         Characteristic rootCharacteristic = dto.toCharacteristic(null);
@@ -60,8 +73,9 @@ public class TechnicalDebtFinder implements ServerComponent, BatchComponent {
         characteristicsById.put(dto.getId(), rootCharacteristic);
       }
     }
+  }
 
-    // Characteristics
+  private void addCharacteristics(TechnicalDebtModel model, List<CharacteristicDto> dtos, Map<Integer, Characteristic> characteristicsById){
     for (CharacteristicDto dto : dtos) {
       if (dto.getParentId() != null && dto.getRuleId() == null) {
         Characteristic parent = characteristicsById.get(dto.getParentId());
@@ -69,15 +83,10 @@ public class TechnicalDebtFinder implements ServerComponent, BatchComponent {
         characteristicsById.put(dto.getId(), characteristic);
       }
     }
+  }
 
-    for (CharacteristicDto dto : dtos) {
-      if (dto.getRuleId() != null) {
-        ruleIds.add(dto.getRuleId());
-      }
-    }
-
-    Map<Integer, Rule> rulesById = findRules(ruleIds);
-    // Requirements
+  private void addRequirements(TechnicalDebtModel model, List<CharacteristicDto> dtos, Map<Integer, Characteristic> characteristicsById){
+    Map<Integer, Rule> rulesById = rulesById(ruleFinder.findAll(RuleQuery.create()));
     for (CharacteristicDto dto : dtos) {
       Integer ruleId = dto.getRuleId();
       if (ruleId != null) {
@@ -87,25 +96,9 @@ public class TechnicalDebtFinder implements ServerComponent, BatchComponent {
         dto.toRequirement(ruleKey, characteristic);
       }
     }
-
-    return model;
-  }
-
-  public TechnicalDebtModel findRootCharacteristics() {
-    TechnicalDebtModel model = new TechnicalDebtModel();
-    List<CharacteristicDto> dtos = dao.selectEnabledRootCharacteristics();
-    // Root characteristics
-    for (CharacteristicDto dto : dtos) {
-      if (dto.getParentId() == null) {
-        Characteristic rootCharacteristic = dto.toCharacteristic(null);
-        model.addRootCharacteristic(rootCharacteristic);
-      }
-    }
-    return model;
   }
 
-  private Map<Integer, Rule> findRules(List<Integer> ruleIds) {
-    Collection<Rule> rules = ruleFinder.findByIds(ruleIds);
+  private Map<Integer, Rule> rulesById(Collection<Rule> rules) {
     Map<Integer, Rule> rulesById = newHashMap();
     for (Rule rule : rules) {
       rulesById.put(rule.getId(), rule);
index cd3fec937c4f20255553e7ed6bba66cca5f4068a..6e3568b8f0c77abfa445d7bce6bec856acfeed7f 100644 (file)
@@ -71,7 +71,7 @@ public class TechnicalDebtManager implements ServerExtension {
   public TechnicalDebtModel initAndMergePlugins(ValidationMessages messages, TechnicalDebtRuleCache rulesCache, SqlSession session) {
     TechnicalDebtModel defaultModel = loadModelFromXml(TechnicalDebtModelRepository.DEFAULT_MODEL, messages, rulesCache);
     TechnicalDebtModel model = loadOrCreateModelFromDb(defaultModel, messages, session);
-    disableRequirementsOnRemovedRules(model, rulesCache, session);
+    disableRequirementsOnRemovedRules(model, session);
     mergePlugins(model, defaultModel, messages, rulesCache, session);
     messages.log(LOG);
 
@@ -139,9 +139,9 @@ public class TechnicalDebtManager implements ServerExtension {
     }
   }
 
-  private void disableRequirementsOnRemovedRules(TechnicalDebtModel model, TechnicalDebtRuleCache rulesCache, SqlSession session) {
+  private void disableRequirementsOnRemovedRules(TechnicalDebtModel model, SqlSession session) {
     for (Requirement requirement : model.requirements()) {
-      if (!rulesCache.exists(requirement.ruleKey())) {
+      if (requirement.ruleKey() == null) {
         requirement.characteristic().removeRequirement(requirement);
         service.disable(requirement, session);
       }
index f13f8e59a182fe93d61192e8aa8a81be396829e0..f183bf04c8a529e6ad2e3e68ba700a6ee6ea6ca5 100644 (file)
@@ -27,15 +27,17 @@ import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rules.Rule;
+import org.sonar.api.rules.RuleFinder;
+import org.sonar.api.rules.RuleQuery;
 import org.sonar.api.technicaldebt.Characteristic;
 import org.sonar.api.technicaldebt.Requirement;
 import org.sonar.api.technicaldebt.WorkUnit;
-import org.sonar.core.rule.DefaultRuleFinder;
 import org.sonar.core.technicaldebt.db.CharacteristicDao;
 import org.sonar.core.technicaldebt.db.CharacteristicDto;
 
 import static com.google.common.collect.Lists.newArrayList;
 import static org.fest.assertions.Assertions.assertThat;
+import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -45,7 +47,7 @@ public class TechnicalDebtFinderTest {
   CharacteristicDao dao;
 
   @Mock
-  DefaultRuleFinder ruleFinder;
+  RuleFinder ruleFinder;
 
   TechnicalDebtFinder finder;
 
@@ -80,7 +82,7 @@ public class TechnicalDebtFinderTest {
     RuleKey ruleKey = RuleKey.of("checkstyle", "Regexp");
     Rule rule = Rule.create(ruleKey.repository(), ruleKey.rule());
     rule.setId(100);
-    when(ruleFinder.findByIds(newArrayList(100))).thenReturn(newArrayList(rule));
+    when(ruleFinder.findAll(any(RuleQuery.class))).thenReturn(newArrayList(rule));
     when(dao.selectEnabledCharacteristics()).thenReturn(newArrayList(rootCharacteristicDto, characteristicDto, requirementDto));
 
     TechnicalDebtModel result = finder.findAll();
index 5d2ad1fe87216d7f3035e86e700293d017f3af25..bb994ed396f2dfede64d70134618cb8b755ca166 100644 (file)
@@ -191,10 +191,8 @@ public class TechnicalDebtManagerTest {
     Characteristic dbCharacteristic = new Characteristic().setKey("COMPILER_RELATED_PORTABILITY").setParent(dbRootCharacteristic);
     dbModel.addRootCharacteristic(dbRootCharacteristic);
 
-    RuleKey ruleKey1 = RuleKey.of("checkstyle", "import");
-    when(ruleCache.exists(ruleKey1)).thenReturn(false);
     // To be disabled as rule does not exists
-    Requirement dbRequirement = new Requirement().setRuleKey(ruleKey1)
+    Requirement dbRequirement = new Requirement().setRuleKey(null)
       .setFunction("linear").setFactor(WorkUnit.create(30.0, WorkUnit.MINUTES)).setCharacteristic(dbCharacteristic);
 
     when(sqaleModelFinder.findAll()).thenReturn(dbModel);
index d095232edf6b7f84fc1a93f4cc0b21e69e262005..c407614bf3770f5cd9a787d2e0068320c7638543 100644 (file)
@@ -17,6 +17,8 @@
 # along with this program; if not, write to the Free Software Foundation,
 # Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #
+
+# TODO delete it
 class CharacteristicProperty < ActiveRecord::Base
 
 
index 05bb61424feefcbee3679fc17a9805583f05534d..0e5a1e1fa7e981fd70e7a2fb2bff43dd4ede261a 100644 (file)
@@ -17,9 +17,8 @@
 # along with this program; if not, write to the Free Software Foundation,
 # Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #
-class QualityModel < ActiveRecord::Base
 
-  validates_length_of :name, :within => 1..100
-  validates_uniqueness_of :name
+# TODO delete it
+class QualityModel < ActiveRecord::Base
 
 end