SONAR-5120 When a rule is linked on a root characteristic, server startup should...

diff --git a/sonar-core/src/main/java/org/sonar/core/debt/TechnicalDebtModelSynchronizer.java b/sonar-core/src/main/java/org/sonar/core/debt/TechnicalDebtModelSynchronizer.java
index a58032fc366f315ff029f2fa8ec967c16f46206d..f5703a43f71cd4fec87900e4846b0c81a035433f 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/debt/TechnicalDebtModelSynchronizer.java
+++ b/sonar-core/src/main/java/org/sonar/core/debt/TechnicalDebtModelSynchronizer.java
@@ -35,6 +35,8 @@ import org.sonar.core.debt.db.CharacteristicDao;
 import org.sonar.core.debt.db.CharacteristicDto;
 import org.sonar.core.persistence.MyBatis;
 
+import javax.annotation.CheckForNull;
+
 import java.io.Reader;
 import java.util.Collection;
 import java.util.List;
@@ -124,12 +126,14 @@ public class TechnicalDebtModelSynchronizer implements ServerExtension {
         Rule rule = rulesCache.getByRuleKey(pluginRequirement.ruleKey());
         if (!isRequirementExists(existingModel, rule)) {
           CharacteristicDto characteristicDto = findCharacteristic(existingModel, pluginRequirement.characteristic().key());
-          Integer rootId = characteristicDto.getRootId();
-          if (rootId == null) {
-            throw new IllegalArgumentException("Requirement on rule '" + pluginRequirement.ruleKey() + "' should not be linked on a root characteristic.");
+          if (characteristicDto != null) {
+            Integer rootId = characteristicDto.getRootId();
+            if (rootId == null) {
+              throw new IllegalArgumentException("Requirement on rule '" + pluginRequirement.ruleKey() + "' should not be linked on a root characteristic.");
+            }
+            CharacteristicDto requirementDto = CharacteristicDto.toDto(pluginRequirement, characteristicDto.getId(), rootId, rule.getId());
+            dao.insert(requirementDto, session);
           }
-          CharacteristicDto requirementDto = CharacteristicDto.toDto(pluginRequirement, characteristicDto.getId(), rootId, rule.getId());
-          dao.insert(requirementDto, session);
         }
       }
     }
@@ -169,6 +173,7 @@ public class TechnicalDebtModelSynchronizer implements ServerExtension {
     return pluginList;
   }
 
+  @CheckForNull
   private CharacteristicDto findCharacteristic(List<CharacteristicDto> existingModel, final String key) {
     return Iterables.find(existingModel, new Predicate<CharacteristicDto>() {
       @Override
@@ -176,7 +181,7 @@ public class TechnicalDebtModelSynchronizer implements ServerExtension {
         String characteristicKey = input.getKey();
         return input.getRuleId() == null && characteristicKey != null && characteristicKey.equals(key);
       }
-    });
+    }, null);
   }
 
   private boolean isRequirementExists(List<CharacteristicDto> existingModel, final Rule rule) {

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java
index 11831d563385e1f5e76196d6249953f2400baeef..4a60de8146472ba2132245e5eb667bf68ad6ec66 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java
@@ -32,6 +32,7 @@ import org.sonar.api.rule.RemediationFunction;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rules.Rule;
 import org.sonar.api.server.rule.RuleDefinitions;
+import org.sonar.api.utils.MessageException;
 import org.sonar.api.utils.System2;
 import org.sonar.api.utils.TimeProfiler;
 import org.sonar.check.Cardinality;
@@ -555,9 +556,8 @@ public class RuleRegistration implements Startable {
       LOG.warn(String.format("Characteristic '%s' has not been found, Technical debt definitions on rule '%s:%s' will be ignored",
         key, ruleDef.repository().name(), ruleDef.key()));
     } else if (characteristicDto.getParentId() == null) {
-      LOG.error(String.format("Rule '%s:%s' should not be linked on the root characteristic '%s'. Technical debt definitions on this rule wll be ignored", key,
-        ruleDef.repository().name(), ruleDef.key()));
-      return null;
+      throw MessageException.of(String.format("Rule '%s:%s' cannot be linked on the root characteristic '%s'",
+        ruleDef.repository().name(), ruleDef.key(), key));
     }
     return characteristicDto;
   }

diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistrationTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistrationTest.java
index 386cfd35aa17518dc51d946ef398739e03d280cf..ead303577bd17ace3fa9563de72789e8d9857ddb 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistrationTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistrationTest.java
@@ -26,6 +26,7 @@ import org.sonar.api.rule.RemediationFunction;
 import org.sonar.api.rule.RuleStatus;
 import org.sonar.api.rule.Severity;
 import org.sonar.api.server.rule.RuleDefinitions;
+import org.sonar.api.utils.MessageException;
 import org.sonar.core.debt.db.CharacteristicDao;
 import org.sonar.core.persistence.AbstractDaoTestCase;
 import org.sonar.core.persistence.MyBatis;
@@ -36,6 +37,7 @@ import org.sonar.server.qualityprofile.ProfilesManager;
 import org.sonar.server.startup.RegisterDebtCharacteristicModel;
 
 import static org.fest.assertions.Assertions.assertThat;
+import static org.fest.assertions.Fail.fail;
 import static org.mockito.Mockito.*;
 
 public class RuleRegistrationTest extends AbstractDaoTestCase {
@@ -174,11 +176,15 @@ public class RuleRegistrationTest extends AbstractDaoTestCase {
   }
 
   @Test
-  public void ignore_rule_debt_definitions_if_rule_is_linked_on_root_characteristic() {
+  public void fail_when_rule_is_linked_on_root_characteristic() {
     setupData("ignore_rule_debt_definitions_if_rule_is_linked_on_root_characteristic");
-    task.start();
 
-    checkTables("ignore_rule_debt_definitions_if_rule_is_linked_on_root_characteristic", EXCLUDED_COLUMN_NAMES, "rules");
+    try {
+      task.start();
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(MessageException.class).hasMessage("Rule 'fake:rule1' cannot be linked on the root characteristic 'MEMORY_EFFICIENCY'");
+    }
   }
 
   @Test

diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/ignore_rule_debt_definitions_if_rule_is_linked_on_root_characteristic-result.xml b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/ignore_rule_debt_definitions_if_rule_is_linked_on_root_characteristic-result.xml
deleted file mode 100644 (file)
index a773983..0000000
--- a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/ignore_rule_debt_definitions_if_rule_is_linked_on_root_characteristic-result.xml
+++ /dev/null
@@ -1,19 +0,0 @@
-<dataset>
-
-  <rules id="1" plugin_rule_key="rule1" plugin_name="fake" plugin_config_key="config1" name="One" description="Description of One"
-         status="READY" priority="4" cardinality="SINGLE" parent_id="[null]" language="java"
-         characteristic_id="[null]" default_characteristic_id="[null]"
-         remediation_function="[null]" default_remediation_function="[null]"
-         remediation_factor="[null]" default_remediation_factor="[null]"
-         remediation_offset="[null]" default_remediation_offset="[null]"
-         effort_to_fix_l10n_key="[null]"/>
-
-  <rules id="2" plugin_rule_key="rule2" plugin_name="fake" plugin_config_key="[null]" name="Two" description="Description of Two"
-         status="DEPRECATED" priority="0" cardinality="SINGLE" parent_id="[null]" language="java"
-         characteristic_id="[null]" default_characteristic_id="[null]"
-         remediation_function="[null]" default_remediation_function="[null]"
-         remediation_factor="[null]" default_remediation_factor="[null]"
-         remediation_offset="[null]" default_remediation_offset="[null]"
-         effort_to_fix_l10n_key="[null]"/>
-
-</dataset>