From 21fc124696e2ff4d78fecb2b614f49939ce57ed2 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 26 Sep 2013 16:16:43 +0200 Subject: [PATCH] SONAR-4714 execute a merge of requirements at server startup instead of doing a reset of all requirements --- .../startup/RegisterTechnicalDebtModel.java | 2 +- .../sonar/server/technicaldebt/RuleCache.java | 11 +- .../technicaldebt/TechnicalDebtManager.java | 61 ++++----- .../technicaldebt/TechnicalDebtModel.java | 40 +++--- .../RegisterTechnicalDebtModelTest.java | 2 +- .../server/technicaldebt/RuleCacheTest.java | 19 ++- .../TechnicalDebtManagerTest.java | 121 ++++++++++-------- .../technicaldebt/TechnicalDebtModelTest.java | 43 +++++-- ...dd_new_requirements_from_plugin-result.xml | 39 ++++++ .../add_new_requirements_from_plugin.xml | 30 +++++ ...rements_from_plugin_on_first_execution.xml | 2 - ..._requirements_on_removed_rules-result.xml} | 28 +--- ...disable_requirements_on_removed_rules.xml} | 6 +- ...model_on_first_execution.xml => empty.xml} | 0 ...cs_not_defined_in_default_model-result.xml | 30 +++++ ...teristics_not_defined_in_default_model.xml | 30 +++++ ...a-model-adding-unknown-characteristic.xml} | 0 ...lugin_define_requirements_on_it-result.xml | 46 +++++++ ..._when_plugin_define_requirements_on_it.xml | 31 +++++ ...arn_when_restoring_unknown_rule-result.xml | 14 -- .../warn_when_restoring_unknown_rule.xml | 9 -- 21 files changed, 402 insertions(+), 162 deletions(-) create mode 100644 sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/add_new_requirements_from_plugin-result.xml create mode 100644 sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/add_new_requirements_from_plugin.xml delete mode 100644 sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/create_model_with_requirements_from_plugin_on_first_execution.xml rename sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/{disable_characteristics_and_remove_requirements_when_resetting_model-result.xml => disable_requirements_on_removed_rules-result.xml} (67%) rename sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/{disable_characteristics_and_remove_requirements_when_resetting_model.xml => disable_requirements_on_removed_rules.xml} (94%) rename sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/{create_default_model_on_first_execution.xml => empty.xml} (100%) create mode 100644 sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/fail_when_plugin_defines_characteristics_not_defined_in_default_model-result.xml create mode 100644 sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/fail_when_plugin_defines_characteristics_not_defined_in_default_model.xml rename sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/{fake-java-model-with-additionnale-characteristic.xml => fake-java-model-adding-unknown-characteristic.xml} (100%) create mode 100644 sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/recreate_previously_deleted_characteristic_from_default_model_when_plugin_define_requirements_on_it-result.xml create mode 100644 sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/recreate_previously_deleted_characteristic_from_default_model_when_plugin_define_requirements_on_it.xml delete mode 100644 sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/warn_when_restoring_unknown_rule-result.xml delete mode 100644 sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/warn_when_restoring_unknown_rule.xml diff --git a/sonar-server/src/main/java/org/sonar/server/startup/RegisterTechnicalDebtModel.java b/sonar-server/src/main/java/org/sonar/server/startup/RegisterTechnicalDebtModel.java index 645082c8a84..44b3f494f52 100644 --- a/sonar-server/src/main/java/org/sonar/server/startup/RegisterTechnicalDebtModel.java +++ b/sonar-server/src/main/java/org/sonar/server/startup/RegisterTechnicalDebtModel.java @@ -41,7 +41,7 @@ public final class RegisterTechnicalDebtModel { public void start() { RuleCache ruleCache = new RuleCache(ruleFinder); - technicalDebtManager.reset(ValidationMessages.create(), ruleCache); + technicalDebtManager.init(ValidationMessages.create(), ruleCache); } } diff --git a/sonar-server/src/main/java/org/sonar/server/technicaldebt/RuleCache.java b/sonar-server/src/main/java/org/sonar/server/technicaldebt/RuleCache.java index dfdce6e3a95..4858fea6dc5 100644 --- a/sonar-server/src/main/java/org/sonar/server/technicaldebt/RuleCache.java +++ b/sonar-server/src/main/java/org/sonar/server/technicaldebt/RuleCache.java @@ -42,10 +42,18 @@ public class RuleCache { @CheckForNull public Rule getRule(String repository, String ruleKey) { + initRules(); + return lookUpRuleInCache(repository, ruleKey); + } + + public boolean exists(Rule rule) { + return getRule(rule.getRepositoryKey(), rule.getKey()) != null; + } + + private void initRules(){ if(cachedRules == null) { loadRules(); } - return lookUpRuleInCache(repository, ruleKey); } private void loadRules() { @@ -62,6 +70,7 @@ public class RuleCache { } } + @CheckForNull private Rule lookUpRuleInCache(String repository, String ruleKey) { Map cachedRepository = cachedRules.get(repository); if(cachedRepository != null) { diff --git a/sonar-server/src/main/java/org/sonar/server/technicaldebt/TechnicalDebtManager.java b/sonar-server/src/main/java/org/sonar/server/technicaldebt/TechnicalDebtManager.java index 3daa1561d70..3d16526965b 100644 --- a/sonar-server/src/main/java/org/sonar/server/technicaldebt/TechnicalDebtManager.java +++ b/sonar-server/src/main/java/org/sonar/server/technicaldebt/TechnicalDebtManager.java @@ -57,67 +57,70 @@ public class TechnicalDebtManager implements ServerExtension { this.importer = importer; } - public Model reset(ValidationMessages messages, RuleCache rulesCache) { + public Model init(ValidationMessages messages, RuleCache rulesCache) { DatabaseSession session = sessionFactory.getSession(); - resetRequirements(); - Model model = loadModelFromDb(); - importDefaultModel(model, messages, rulesCache); - loadRequirementsFromPlugins(model, messages, rulesCache); + disableRequirementsOnRemovedRules(rulesCache); + + Model defaultModel = loadModelFromXml(TechnicalDebtModelFinder.DEFAULT_MODEL, messages, rulesCache); + Model model = loadOrCreateModelFromDb(defaultModel, messages, rulesCache); + loadRequirementsFromPlugins(model, defaultModel, messages, rulesCache); session.save(model); session.commit(); return model; } - private Model loadModelFromDb() { - Model existingModel = modelFinder.findByName(RegisterTechnicalDebtModel.TECHNICAL_DEBT_MODEL); - if (existingModel == null) { - return Model.createByName(RegisterTechnicalDebtModel.TECHNICAL_DEBT_MODEL); - } - return existingModel; - } - - private void importDefaultModel(Model model, ValidationMessages messages, RuleCache rulesCache){ - mergeRequirementsFromPlugin(model, TechnicalDebtModelFinder.DEFAULT_MODEL, messages, rulesCache); - } - - private void loadRequirementsFromPlugins(Model initialModel, ValidationMessages messages, RuleCache ruleCache) { - for (String pluginKey : getContributingPluginListWithoutSqale()) { - mergeRequirementsFromPlugin(initialModel, pluginKey, messages, ruleCache); + private Model loadOrCreateModelFromDb(Model defaultModel, ValidationMessages messages, RuleCache rulesCache) { + Model model = modelFinder.findByName(RegisterTechnicalDebtModel.TECHNICAL_DEBT_MODEL); + if (model == null) { + model = Model.createByName(RegisterTechnicalDebtModel.TECHNICAL_DEBT_MODEL); + merge(defaultModel, model, defaultModel, messages, rulesCache); } + return model; } - private void mergeRequirementsFromPlugin(Model initialModel, String pluginKey, ValidationMessages messages, RuleCache ruleCache) { - Model model = loadModelFromXml(pluginKey, messages, ruleCache); + private void merge(Model pluginModel, Model existingModel, Model defaultModel, ValidationMessages messages, RuleCache rulesCache) { messages.log(LOG); if (!messages.hasErrors()) { - new TechnicalDebtModel(initialModel).mergeWith(model, messages, ruleCache); + new TechnicalDebtModel(existingModel, defaultModel.getCharacteristics()).mergeWith(pluginModel, messages, rulesCache); messages.log(LOG); } } - private Model loadModelFromXml(String pluginKey, ValidationMessages messages, RuleCache ruleCache){ + private void loadRequirementsFromPlugins(Model existingModel, Model defaultModel, ValidationMessages messages, RuleCache rulesCache) { + for (String pluginKey : getContributingPluginListWithoutSqale()) { + Model pluginModel = loadModelFromXml(pluginKey, messages, rulesCache); + merge(pluginModel, existingModel, defaultModel, messages, rulesCache); + } + } + + private Model loadModelFromXml(String pluginKey, ValidationMessages messages, RuleCache rulesCache) { Reader xmlFileReader = null; try { xmlFileReader = languageModelFinder.createReaderForXMLFile(pluginKey); - return importer.importXML(xmlFileReader, messages, ruleCache); + return importer.importXML(xmlFileReader, messages, rulesCache); } finally { IOUtils.closeQuietly(xmlFileReader); } } - private void resetRequirements() { + /** + * Disable requirements linked on removed rules + */ + private void disableRequirementsOnRemovedRules(RuleCache rulesCache) { Model existingModel = modelFinder.findByName(RegisterTechnicalDebtModel.TECHNICAL_DEBT_MODEL); if (existingModel != null) { - for (Characteristic root : existingModel.getCharacteristicsByDepth(REQUIREMENT_LEVEL)) { - existingModel.removeCharacteristic(root); + for (Characteristic requirement : existingModel.getCharacteristicsByDepth(REQUIREMENT_LEVEL)) { + if (!rulesCache.exists(requirement.getRule())) { + existingModel.removeCharacteristic(requirement); + } } sessionFactory.getSession().commit(); } } - private Collection getContributingPluginListWithoutSqale(){ + private Collection getContributingPluginListWithoutSqale() { return languageModelFinder.getContributingPluginList(); } diff --git a/sonar-server/src/main/java/org/sonar/server/technicaldebt/TechnicalDebtModel.java b/sonar-server/src/main/java/org/sonar/server/technicaldebt/TechnicalDebtModel.java index ea288fcfe1b..d7342d6c1ab 100644 --- a/sonar-server/src/main/java/org/sonar/server/technicaldebt/TechnicalDebtModel.java +++ b/sonar-server/src/main/java/org/sonar/server/technicaldebt/TechnicalDebtModel.java @@ -25,47 +25,55 @@ import org.sonar.api.qualitymodel.Model; import org.sonar.api.rules.Rule; import org.sonar.api.utils.ValidationMessages; +import java.util.List; + public class TechnicalDebtModel { private Model model; - public TechnicalDebtModel(Model model) { + private List defaultCharacteristics; + + public TechnicalDebtModel(Model model, List defaultCharacteristics) { this.model = model; + this.defaultCharacteristics = defaultCharacteristics; } public void mergeWith(Model with, ValidationMessages messages, RuleCache ruleCache) { for (Characteristic characteristic : with.getCharacteristics()) { if (isRequirement(characteristic)) { - mergeRequirement(model, characteristic, messages, ruleCache); + mergeRequirement(characteristic, messages, ruleCache); } else { - mergeCharacteristic(model, characteristic, messages); + mergeCharacteristic(characteristic, messages); } } } - private Characteristic mergeCharacteristic(Model target, Characteristic characteristic, ValidationMessages messages) { - Characteristic targetCharacteristic = target.getCharacteristicByKey(characteristic.getKey()); - if (targetCharacteristic == null) { - targetCharacteristic = target.addCharacteristic(clone(characteristic)); - if (!characteristic.getParents().isEmpty()) { - Characteristic parentTargetCharacteristic = mergeCharacteristic(target, characteristic.getParents().get(0), messages); - parentTargetCharacteristic.addChild(targetCharacteristic); + private Characteristic mergeCharacteristic(Characteristic characteristic, ValidationMessages messages) { + Characteristic existingCharacteristic = model.getCharacteristicByKey(characteristic.getKey()); + if (existingCharacteristic == null) { + if (defaultCharacteristics.contains(characteristic)) { + existingCharacteristic = model.addCharacteristic(clone(characteristic)); + if (!characteristic.getParents().isEmpty()) { + Characteristic parentTargetCharacteristic = mergeCharacteristic(characteristic.getParents().get(0), messages); + parentTargetCharacteristic.addChild(existingCharacteristic); + } + } else { + throw new IllegalArgumentException("The characteristic : " + characteristic.getKey() + " cannot be used as it's not available in default ones."); } } - return targetCharacteristic; + return existingCharacteristic; } - private void mergeRequirement(Model target, Characteristic requirement, ValidationMessages messages, - RuleCache ruleCache) { - Characteristic targetRequirement = target.getCharacteristicByRule(requirement.getRule()); + private void mergeRequirement(Characteristic requirement, ValidationMessages messages, RuleCache ruleCache) { + Characteristic targetRequirement = model.getCharacteristicByRule(requirement.getRule()); if (targetRequirement == null && !requirement.getParents().isEmpty()) { Rule rule = ruleCache.getRule(requirement.getRule().getRepositoryKey(), requirement.getRule().getKey()); if (rule == null) { messages.addWarningText("The rule " + requirement.getRule() + " does not exist."); } else { - Characteristic parent = mergeCharacteristic(target, requirement.getParents().get(0), messages); - requirement = target.addCharacteristic(clone(requirement)); + Characteristic parent = mergeCharacteristic(requirement.getParents().get(0), messages); + requirement = model.addCharacteristic(clone(requirement)); requirement.setRule(rule); parent.addChild(requirement); } diff --git a/sonar-server/src/test/java/org/sonar/server/startup/RegisterTechnicalDebtModelTest.java b/sonar-server/src/test/java/org/sonar/server/startup/RegisterTechnicalDebtModelTest.java index e2f5596a562..f65e5d17514 100644 --- a/sonar-server/src/test/java/org/sonar/server/startup/RegisterTechnicalDebtModelTest.java +++ b/sonar-server/src/test/java/org/sonar/server/startup/RegisterTechnicalDebtModelTest.java @@ -38,6 +38,6 @@ public class RegisterTechnicalDebtModelTest { sqaleDefinition.start(); - verify(technicalDebtManager, times(1)).reset(any(ValidationMessages.class), any(RuleCache.class)); + verify(technicalDebtManager, times(1)).init(any(ValidationMessages.class), any(RuleCache.class)); } } diff --git a/sonar-server/src/test/java/org/sonar/server/technicaldebt/RuleCacheTest.java b/sonar-server/src/test/java/org/sonar/server/technicaldebt/RuleCacheTest.java index 9d9a09c78d3..52e439b617f 100644 --- a/sonar-server/src/test/java/org/sonar/server/technicaldebt/RuleCacheTest.java +++ b/sonar-server/src/test/java/org/sonar/server/technicaldebt/RuleCacheTest.java @@ -34,7 +34,7 @@ import static org.mockito.Mockito.*; public class RuleCacheTest { @Test - public void should_lazy_load_rules_on_first_call() throws Exception { + public void lazy_load_rules_on_first_call() throws Exception { RuleFinder ruleFinder = mock(RuleFinder.class); when(ruleFinder.findAll(any(RuleQuery.class))).thenReturn(Collections.EMPTY_LIST); @@ -47,7 +47,7 @@ public class RuleCacheTest { } @Test - public void should_return_matching_rule() throws Exception { + public void return_matching_rule() throws Exception { Rule rule1 = Rule.create("repo1", "rule1"); Rule rule2 = Rule.create("repo2", "rule2"); @@ -62,4 +62,19 @@ public class RuleCacheTest { assertThat(actualRule1).isEqualTo(rule1); assertThat(actualRule2).isEqualTo(rule2); } + + @Test + public void return_if_rule_exists() throws Exception { + + Rule rule1 = Rule.create("repo1", "rule1"); + Rule rule2 = Rule.create("repo2", "rule2"); + + RuleFinder ruleFinder = mock(RuleFinder.class); + when(ruleFinder.findAll(any(RuleQuery.class))).thenReturn(Lists.newArrayList(rule1)); + + RuleCache ruleCache = new RuleCache(ruleFinder); + + assertThat(ruleCache.exists(rule1)).isTrue(); + assertThat(ruleCache.exists(rule2)).isFalse(); + } } diff --git a/sonar-server/src/test/java/org/sonar/server/technicaldebt/TechnicalDebtManagerTest.java b/sonar-server/src/test/java/org/sonar/server/technicaldebt/TechnicalDebtManagerTest.java index 099387fa9d7..2036f2aa19a 100644 --- a/sonar-server/src/test/java/org/sonar/server/technicaldebt/TechnicalDebtManagerTest.java +++ b/sonar-server/src/test/java/org/sonar/server/technicaldebt/TechnicalDebtManagerTest.java @@ -23,10 +23,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.io.Resources; import org.junit.Before; import org.junit.Test; -import org.sonar.api.qualitymodel.Characteristic; import org.sonar.api.qualitymodel.Model; import org.sonar.api.rules.Rule; -import org.sonar.api.rules.RuleFinder; import org.sonar.api.utils.ValidationMessages; import org.sonar.core.qualitymodel.DefaultModelFinder; import org.sonar.core.rule.DefaultRuleFinder; @@ -34,10 +32,9 @@ import org.sonar.jpa.test.AbstractDbUnitTestCase; import java.io.FileNotFoundException; import java.io.FileReader; -import java.util.List; import static org.fest.assertions.Assertions.assertThat; -import static org.mockito.Matchers.anyString; +import static org.fest.assertions.Fail.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -49,10 +46,6 @@ public class TechnicalDebtManagerTest extends AbstractDbUnitTestCase { @Before public void init() throws Exception { technicalDebtModelFinder = mock(TechnicalDebtModelFinder.class); - when(technicalDebtModelFinder.getContributingPluginList()).thenReturn(ImmutableList.of("java")); - when(technicalDebtModelFinder.createReaderForXMLFile("java")).thenReturn( - new FileReader(Resources.getResource(TechnicalDebtManagerTest.class, "TechnicalDebtManagerTest/fake-java-model.xml").getPath())); - // Mock default model when(technicalDebtModelFinder.createReaderForXMLFile("technical-debt")).thenReturn( new FileReader(Resources.getResource(TechnicalDebtManagerTest.class, "TechnicalDebtManagerTest/fake-default-model.xml").getPath())); @@ -61,30 +54,18 @@ public class TechnicalDebtManagerTest extends AbstractDbUnitTestCase { @Test public void create_only_default_model_on_first_execution_when_no_plugin() throws Exception { - setupData("create_default_model_on_first_execution"); + setupData("empty"); - TechnicalDebtModelFinder technicalDebtModelFinder = mock(TechnicalDebtModelFinder.class); - when(technicalDebtModelFinder.createReaderForXMLFile("technical-debt")).thenReturn( - new FileReader(Resources.getResource(TechnicalDebtManagerTest.class, "TechnicalDebtManagerTest/fake-default-model.xml").getPath())); - - TechnicalDebtManager manager = new TechnicalDebtManager(getSessionFactory(), new DefaultModelFinder(getSessionFactory()), technicalDebtModelFinder, new XMLImporter()); - manager.reset(ValidationMessages.create(), defaultRuleCache()); - getSession().commit(); + manager.init(ValidationMessages.create(), defaultRuleCache()); checkTables("create_default_model_on_first_execution", "quality_models", "characteristics", "characteristic_edges"); } @Test public void create_model_with_requirements_from_plugin_on_first_execution() throws Exception { - setupData("create_model_with_requirements_from_plugin_on_first_execution"); - - TechnicalDebtModelFinder technicalDebtModelFinder = mock(TechnicalDebtModelFinder.class); + setupData("empty"); - when(technicalDebtModelFinder.getContributingPluginList()).thenReturn(ImmutableList.of("java")); - when(technicalDebtModelFinder.createReaderForXMLFile("java")).thenReturn( - new FileReader(Resources.getResource(TechnicalDebtManagerTest.class, "TechnicalDebtManagerTest/fake-java-model.xml").getPath())); - when(technicalDebtModelFinder.createReaderForXMLFile("technical-debt")).thenReturn( - new FileReader(Resources.getResource(TechnicalDebtManagerTest.class, "TechnicalDebtManagerTest/fake-default-model.xml").getPath())); + addPluginModel("java", "fake-java-model.xml"); RuleCache ruleCache = mock(RuleCache.class); Rule rule1 = Rule.create("checkstyle", "import", "Regular expression"); @@ -94,35 +75,62 @@ public class TechnicalDebtManagerTest extends AbstractDbUnitTestCase { rule2.setId(2); when(ruleCache.getRule("checkstyle", "export")).thenReturn(rule2); - TechnicalDebtManager manager = new TechnicalDebtManager(getSessionFactory(), new DefaultModelFinder(getSessionFactory()), technicalDebtModelFinder, new XMLImporter()); - manager.reset(ValidationMessages.create(), ruleCache); - getSession().commit(); + manager.init(ValidationMessages.create(), ruleCache); checkTables("create_model_with_requirements_from_plugin_on_first_execution", "quality_models", "characteristics", "characteristic_edges", "characteristic_properties"); } @Test - public void disable_characteristics_and_remove_requirements_when_resetting_model() throws Exception { - setupData("disable_characteristics_and_remove_requirements_when_resetting_model"); + public void add_new_requirements_from_plugin() throws Exception { + setupData("add_new_requirements_from_plugin"); - TechnicalDebtModelFinder technicalDebtModelFinder = mock(TechnicalDebtModelFinder.class); + addPluginModel("java", "fake-java-model.xml"); - when(technicalDebtModelFinder.getContributingPluginList()).thenReturn(ImmutableList.of("java")); - when(technicalDebtModelFinder.createReaderForXMLFile("java")).thenReturn( - new FileReader(Resources.getResource(TechnicalDebtManagerTest.class, "TechnicalDebtManagerTest/fake-java-model.xml").getPath())); - when(technicalDebtModelFinder.createReaderForXMLFile("technical-debt")).thenReturn( - new FileReader(Resources.getResource(TechnicalDebtManagerTest.class, "TechnicalDebtManagerTest/fake-default-model.xml").getPath())); + manager.init(ValidationMessages.create(), defaultRuleCache()); - TechnicalDebtManager manager = new TechnicalDebtManager(getSessionFactory(), new DefaultModelFinder(getSessionFactory()), technicalDebtModelFinder, new XMLImporter()); - manager.reset(ValidationMessages.create(), defaultRuleCache()); - getSession().commit(); + checkTables("add_new_requirements_from_plugin", "quality_models", "characteristics", "characteristic_edges", "characteristic_properties"); + } + + @Test + public void disable_requirements_on_removed_rules() throws Exception { + setupData("disable_requirements_on_removed_rules"); + + addPluginModel("java", "fake-java-model.xml"); - checkTables("disable_characteristics_and_remove_requirements_when_resetting_model", "quality_models", "characteristics", "characteristic_edges", "characteristic_properties"); + manager.init(ValidationMessages.create(), defaultRuleCache()); + + checkTables("disable_requirements_on_removed_rules", "quality_models", "characteristics", "characteristic_edges", "characteristic_properties"); + } + + @Test + public void fail_when_plugin_defines_characteristics_not_defined_in_default_model() throws Exception { + setupData("fail_when_plugin_defines_characteristics_not_defined_in_default_model"); + + addPluginModel("java", "fake-java-model-adding-unknown-characteristic.xml"); + + try { + manager.init(ValidationMessages.create(), defaultRuleCache()); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(IllegalArgumentException.class); + } + checkTables("fail_when_plugin_defines_characteristics_not_defined_in_default_model", "quality_models", "characteristics", "characteristic_edges", "characteristic_properties"); } @Test - public void provided_plugin_should_not_override_default_model_when_resetting_model() throws FileNotFoundException { - Model model = manager.reset(ValidationMessages.create(), defaultRuleCache()); + public void recreate_previously_deleted_characteristic_from_default_model_when_plugin_define_requirements_on_it() throws Exception { + setupData("recreate_previously_deleted_characteristic_from_default_model_when_plugin_define_requirements_on_it"); + + addPluginModel("java", "fake-java-model.xml"); + + manager.init(ValidationMessages.create(), defaultRuleCache()); + + checkTables("recreate_previously_deleted_characteristic_from_default_model_when_plugin_define_requirements_on_it", "quality_models", "characteristics", "characteristic_edges", "characteristic_properties"); + } + + @Test + public void provided_plugin_should_not_override_default_characteristics_name() throws FileNotFoundException { + Model model = manager.init(ValidationMessages.create(), defaultRuleCache()); // Default model values assertThat(model.getCharacteristicByKey("PORTABILITY").getName()).isEqualTo("Portability"); assertThat(model.getCharacteristicByKey("COMPILER_RELATED_PORTABILITY").getName()).isEqualTo("Compiler related portability"); @@ -134,30 +142,35 @@ public class TechnicalDebtManagerTest extends AbstractDbUnitTestCase { } @Test - public void no_failure_on_unknown_rule_when_resetting_model() { - setupData("reset_model"); + public void no_failure_on_unknown_rule() throws FileNotFoundException { + setupData("empty"); - RuleFinder ruleFinder = mock(RuleFinder.class); - when(ruleFinder.findByKey(anyString(), anyString())).thenReturn(null); + addPluginModel("java", "fake-java-model.xml"); - manager = new TechnicalDebtManager(getSessionFactory(), new DefaultModelFinder(getSessionFactory()), technicalDebtModelFinder, new XMLImporter()); + RuleCache ruleCache = mock(RuleCache.class); + Rule rule1 = Rule.create("checkstyle", "import", "Regular expression"); + rule1.setId(1); + when(ruleCache.getRule("checkstyle", "import")).thenReturn(rule1); + Rule rule2 = Rule.create("checkstyle", "export", "Regular expression"); + rule2.setId(2); + when(ruleCache.getRule("checkstyle", "export")).thenReturn(rule2); ValidationMessages messages = ValidationMessages.create(); - Model model = manager.reset(messages, new RuleCache(ruleFinder)); - assertThat(model.getCharacteristics().size()).isGreaterThanOrEqualTo(3); - assertThat(model.getCharacteristics().size()).isGreaterThan(model.getRootCharacteristics().size()); - List hardwareControls = model.getCharacteristicByKey("HARDWARE_RELATED_PORTABILITY").getChildren(); - assertThat(hardwareControls.isEmpty()).isTrue(); + manager.init(messages, ruleCache); - assertThat(messages.getWarnings()).hasSize(3); + assertThat(messages.getWarnings()).hasSize(1); + assertThat(messages.getWarnings().get(0)).isEqualTo("Rule not found: [repository=checkstyle, key=ConstantNameCheck]"); } private RuleCache defaultRuleCache() { return new RuleCache(new DefaultRuleFinder(getSessionFactory())); } - private Rule newRegexpRule() { - return Rule.create("checkstyle", "regexp", "Regular expression"); + private void addPluginModel(String pluginKey, String xmlFile) throws FileNotFoundException { + when(technicalDebtModelFinder.getContributingPluginList()).thenReturn(ImmutableList.of(pluginKey)); + when(technicalDebtModelFinder.createReaderForXMLFile(pluginKey)).thenReturn( + new FileReader(Resources.getResource(TechnicalDebtManagerTest.class, "TechnicalDebtManagerTest/" + xmlFile).getPath())); } + } diff --git a/sonar-server/src/test/java/org/sonar/server/technicaldebt/TechnicalDebtModelTest.java b/sonar-server/src/test/java/org/sonar/server/technicaldebt/TechnicalDebtModelTest.java index a294496300d..1ade07625e5 100644 --- a/sonar-server/src/test/java/org/sonar/server/technicaldebt/TechnicalDebtModelTest.java +++ b/sonar-server/src/test/java/org/sonar/server/technicaldebt/TechnicalDebtModelTest.java @@ -19,7 +19,6 @@ */ package org.sonar.server.technicaldebt; -import com.google.common.collect.Lists; import org.junit.Before; import org.junit.Test; import org.sonar.api.qualitymodel.Characteristic; @@ -30,7 +29,11 @@ import org.sonar.api.rules.RuleQuery; import org.sonar.api.utils.ValidationMessages; import org.sonar.server.startup.RegisterTechnicalDebtModel; +import java.util.List; + +import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; +import static org.fest.assertions.Fail.fail; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -39,22 +42,26 @@ public class TechnicalDebtModelTest { private Model model; private TechnicalDebtModel technicalDebtModel; + private List defaultCharacteristics; @Before public void setUpModel() { model = Model.createByName(RegisterTechnicalDebtModel.TECHNICAL_DEBT_MODEL); - technicalDebtModel = new TechnicalDebtModel(model); + defaultCharacteristics = newArrayList(); + technicalDebtModel = new TechnicalDebtModel(model, defaultCharacteristics); } @Test - public void shouldMergeWithEmptyModel() { + public void merge_with_empty_model() { Model with = Model.createByName(RegisterTechnicalDebtModel.TECHNICAL_DEBT_MODEL); Characteristic efficiency = with.createCharacteristicByKey("efficiency", "Efficiency"); - efficiency.addChild(with.createCharacteristicByKey("ram-efficiency", "RAM Efficiency")); - with.createCharacteristicByKey("usability", "Usability"); + Characteristic ramEfficiency = with.createCharacteristicByKey("ram-efficiency", "RAM Efficiency"); + efficiency.addChild(ramEfficiency); + Characteristic usability = with.createCharacteristicByKey("usability", "Usability"); ValidationMessages messages = ValidationMessages.create(); + defaultCharacteristics.addAll(newArrayList(efficiency, ramEfficiency, usability)); technicalDebtModel.mergeWith(with, messages, mockRuleCache()); assertThat(model.getCharacteristics()).hasSize(3); @@ -64,7 +71,7 @@ public class TechnicalDebtModelTest { } @Test - public void shouldNotUpdateExistingCharacteristics() { + public void not_update_existing_characteristics() { model.createCharacteristicByKey("efficiency", "Efficiency"); Model with = Model.createByName(RegisterTechnicalDebtModel.TECHNICAL_DEBT_MODEL); @@ -78,7 +85,7 @@ public class TechnicalDebtModelTest { } @Test - public void shouldWarnOnMissingRule() { + public void warn_on_missing_rule() { Model with = Model.createByName(RegisterTechnicalDebtModel.TECHNICAL_DEBT_MODEL); Characteristic efficiency = with.createCharacteristicByKey("efficiency", "Efficiency"); Rule fooRule = Rule.create("foo", "bar", "Bar"); @@ -87,6 +94,7 @@ public class TechnicalDebtModelTest { ValidationMessages messages = ValidationMessages.create(); + defaultCharacteristics.add(efficiency); technicalDebtModel.mergeWith(with, messages, mockRuleCache()); assertThat(model.getCharacteristics()).hasSize(1); @@ -96,9 +104,28 @@ public class TechnicalDebtModelTest { assertThat(messages.getWarnings().get(0)).contains("foo"); // warning: the rule foo does not exist } + @Test + public void fail_when_adding_characteristic_not_existing_in_default_characteristics() { + Model with = Model.createByName(RegisterTechnicalDebtModel.TECHNICAL_DEBT_MODEL); + Characteristic efficiency = with.createCharacteristicByKey("efficiency", "Efficiency"); + // usability is not available in default characteristics + with.createCharacteristicByKey("usability", "Usability"); + + ValidationMessages messages = ValidationMessages.create(); + + defaultCharacteristics.add(efficiency); + try { + technicalDebtModel.mergeWith(with, messages, mockRuleCache()); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(IllegalArgumentException.class); + } + assertThat(model.getCharacteristics()).hasSize(1); + } + private RuleCache mockRuleCache() { RuleFinder ruleFinder = mock(RuleFinder.class); - when(ruleFinder.findAll(any(RuleQuery.class))).thenReturn(Lists.newArrayList(newRegexpRule())); + when(ruleFinder.findAll(any(RuleQuery.class))).thenReturn(newArrayList(newRegexpRule())); return new RuleCache(ruleFinder); } diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/add_new_requirements_from_plugin-result.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/add_new_requirements_from_plugin-result.xml new file mode 100644 index 00000000000..3d5786f8cbb --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/add_new_requirements_from_plugin-result.xml @@ -0,0 +1,39 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/add_new_requirements_from_plugin.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/add_new_requirements_from_plugin.xml new file mode 100644 index 00000000000..ab6a26428fc --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/add_new_requirements_from_plugin.xml @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/create_model_with_requirements_from_plugin_on_first_execution.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/create_model_with_requirements_from_plugin_on_first_execution.xml deleted file mode 100644 index fb0854fccbe..00000000000 --- a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/create_model_with_requirements_from_plugin_on_first_execution.xml +++ /dev/null @@ -1,2 +0,0 @@ - - diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/disable_characteristics_and_remove_requirements_when_resetting_model-result.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/disable_requirements_on_removed_rules-result.xml similarity index 67% rename from sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/disable_characteristics_and_remove_requirements_when_resetting_model-result.xml rename to sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/disable_requirements_on_removed_rules-result.xml index 735b6f97e2b..3d387508b11 100644 --- a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/disable_characteristics_and_remove_requirements_when_resetting_model-result.xml +++ b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/disable_requirements_on_removed_rules-result.xml @@ -4,15 +4,15 @@ - + + description="[null]" enabled="true"/> - + @@ -20,37 +20,21 @@ - - - - - - - - - - + - - - - - - - + + diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/disable_characteristics_and_remove_requirements_when_resetting_model.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/disable_requirements_on_removed_rules.xml similarity index 94% rename from sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/disable_characteristics_and_remove_requirements_when_resetting_model.xml rename to sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/disable_requirements_on_removed_rules.xml index be70284b5ab..344e9e2e0c5 100644 --- a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/disable_characteristics_and_remove_requirements_when_resetting_model.xml +++ b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/disable_requirements_on_removed_rules.xml @@ -12,7 +12,7 @@ - + @@ -20,7 +20,6 @@ - @@ -33,6 +32,7 @@ - + + diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/create_default_model_on_first_execution.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/empty.xml similarity index 100% rename from sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/create_default_model_on_first_execution.xml rename to sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/empty.xml diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/fail_when_plugin_defines_characteristics_not_defined_in_default_model-result.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/fail_when_plugin_defines_characteristics_not_defined_in_default_model-result.xml new file mode 100644 index 00000000000..ab6a26428fc --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/fail_when_plugin_defines_characteristics_not_defined_in_default_model-result.xml @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/fail_when_plugin_defines_characteristics_not_defined_in_default_model.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/fail_when_plugin_defines_characteristics_not_defined_in_default_model.xml new file mode 100644 index 00000000000..ab6a26428fc --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/fail_when_plugin_defines_characteristics_not_defined_in_default_model.xml @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/fake-java-model-with-additionnale-characteristic.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/fake-java-model-adding-unknown-characteristic.xml similarity index 100% rename from sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/fake-java-model-with-additionnale-characteristic.xml rename to sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/fake-java-model-adding-unknown-characteristic.xml diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/recreate_previously_deleted_characteristic_from_default_model_when_plugin_define_requirements_on_it-result.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/recreate_previously_deleted_characteristic_from_default_model_when_plugin_define_requirements_on_it-result.xml new file mode 100644 index 00000000000..702f49e5275 --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/recreate_previously_deleted_characteristic_from_default_model_when_plugin_define_requirements_on_it-result.xml @@ -0,0 +1,46 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/recreate_previously_deleted_characteristic_from_default_model_when_plugin_define_requirements_on_it.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/recreate_previously_deleted_characteristic_from_default_model_when_plugin_define_requirements_on_it.xml new file mode 100644 index 00000000000..f78df06bd77 --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/recreate_previously_deleted_characteristic_from_default_model_when_plugin_define_requirements_on_it.xml @@ -0,0 +1,31 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/warn_when_restoring_unknown_rule-result.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/warn_when_restoring_unknown_rule-result.xml deleted file mode 100644 index 1da76f127aa..00000000000 --- a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/warn_when_restoring_unknown_rule-result.xml +++ /dev/null @@ -1,14 +0,0 @@ - - - - - - - - - - - - - - diff --git a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/warn_when_restoring_unknown_rule.xml b/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/warn_when_restoring_unknown_rule.xml deleted file mode 100644 index 619ad9ea4f6..00000000000 --- a/sonar-server/src/test/resources/org/sonar/server/technicaldebt/TechnicalDebtManagerTest/warn_when_restoring_unknown_rule.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - - - - - - -- 2.39.5