From 4bfdd2dd9a3932e15687b77919c14cb47fe59915 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 27 Nov 2013 18:19:58 +0100 Subject: [PATCH] SONAR-4831 Improve technical debt model synchronisation --- .../technicaldebt/TechnicalDebtFinder.java | 8 +- .../technicaldebt/TechnicalDebtManager.java | 157 --------------- .../TechnicalDebtModelService.java | 19 +- .../TechnicalDebtModelSynchronizer.java | 186 ++++++++++++++++++ .../technicaldebt/TechnicalDebtRuleCache.java | 9 + .../technicaldebt/db/CharacteristicDto.java | 10 +- ...> TechnicalDebtModelSynchronizerTest.java} | 154 +++++++-------- .../org/sonar/server/platform/Platform.java | 3 +- .../startup/RegisterTechnicalDebtModel.java | 6 +- .../RegisterTechnicalDebtModelTest.java | 4 +- 10 files changed, 290 insertions(+), 266 deletions(-) delete mode 100644 sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtManager.java create mode 100644 sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtModelSynchronizer.java rename sonar-core/src/test/java/org/sonar/core/technicaldebt/{TechnicalDebtManagerTest.java => TechnicalDebtModelSynchronizerTest.java} (61%) diff --git a/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtFinder.java b/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtFinder.java index 06a252e593a..def60839db3 100644 --- a/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtFinder.java +++ b/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtFinder.java @@ -92,12 +92,8 @@ public class TechnicalDebtFinder implements ServerComponent, BatchComponent { if (ruleId != null) { Characteristic characteristic = characteristicsById.get(dto.getParentId()); Rule rule = rulesById.get(ruleId); - if (rule != null) { - RuleKey ruleKey = RuleKey.of(rule.getRepositoryKey(), rule.getKey()); - dto.toRequirement(ruleKey, characteristic); - } else { - dto.toRequirement(null, characteristic); - } + RuleKey ruleKey = RuleKey.of(rule.getRepositoryKey(), rule.getKey()); + dto.toRequirement(ruleKey, characteristic); } } } diff --git a/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtManager.java b/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtManager.java deleted file mode 100644 index 6e3568b8f0c..00000000000 --- a/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtManager.java +++ /dev/null @@ -1,157 +0,0 @@ -/* - * SonarQube, open source software quality management tool. - * Copyright (C) 2008-2013 SonarSource - * mailto:contact AT sonarsource DOT com - * - * SonarQube is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * SonarQube is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ - -package org.sonar.core.technicaldebt; - -import org.apache.commons.io.IOUtils; -import org.apache.ibatis.session.SqlSession; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.sonar.api.ServerExtension; -import org.sonar.api.technicaldebt.Characteristic; -import org.sonar.api.technicaldebt.Requirement; -import org.sonar.api.utils.ValidationMessages; -import org.sonar.core.persistence.MyBatis; - -import java.io.Reader; -import java.util.Collection; -import java.util.List; - -import static com.google.common.collect.Lists.newArrayList; - -public class TechnicalDebtManager implements ServerExtension { - - private static final Logger LOG = LoggerFactory.getLogger(TechnicalDebtManager.class); - - private final MyBatis mybatis; - private final TechnicalDebtModelService service; - private final TechnicalDebtFinder modelFinder; - private final TechnicalDebtModelRepository languageModelFinder; - private final TechnicalDebtXMLImporter importer; - - public TechnicalDebtManager(MyBatis mybatis, TechnicalDebtModelService service, TechnicalDebtFinder modelFinder, - TechnicalDebtModelRepository modelRepository, TechnicalDebtXMLImporter importer) { - this.mybatis = mybatis; - this.service = service; - this.modelFinder = modelFinder; - this.languageModelFinder = modelRepository; - this.importer = importer; - } - - public TechnicalDebtModel initAndMergePlugins(ValidationMessages messages, TechnicalDebtRuleCache rulesCache) { - SqlSession session = mybatis.openSession(); - - TechnicalDebtModel model = null; - try { - model = initAndMergePlugins(messages, rulesCache, session); - session.commit(); - } finally { - MyBatis.closeQuietly(session); - } - return model; - } - - 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, session); - mergePlugins(model, defaultModel, messages, rulesCache, session); - messages.log(LOG); - - return model; - } - - public TechnicalDebtModel loadModel() { - return modelFinder.findAll(); - } - - private TechnicalDebtModel loadOrCreateModelFromDb(TechnicalDebtModel defaultModel, ValidationMessages messages, SqlSession session) { - TechnicalDebtModel model = loadModel(); - if (model.isEmpty()) { - createTechnicalDebtModel(defaultModel, session); - return defaultModel; - } - return model; - } - - private void createTechnicalDebtModel(TechnicalDebtModel defaultModel, SqlSession session) { - for (Characteristic rootCharacteristic : defaultModel.rootCharacteristics()) { - service.create(rootCharacteristic, session); - for (Characteristic characteristic : rootCharacteristic.children()) { - service.create(characteristic, session); - } - } - } - - private void mergePlugins(TechnicalDebtModel existingModel, TechnicalDebtModel defaultModel, ValidationMessages messages, TechnicalDebtRuleCache rulesCache, SqlSession session) { - for (String pluginKey : getContributingPluginListWithoutSqale()) { - TechnicalDebtModel pluginModel = loadModelFromXml(pluginKey, messages, rulesCache); - checkPluginDoNotAddNewCharacteristic(pluginModel, defaultModel); - mergePlugin(pluginModel, existingModel, messages, rulesCache, session); - } - } - - public void mergePlugin(TechnicalDebtModel pluginModel, TechnicalDebtModel existingModel, ValidationMessages messages, TechnicalDebtRuleCache rulesCache, SqlSession session) { - if (!messages.hasErrors()) { - List existingRequirements = existingModel.requirements(); - for (Requirement pluginRequirement : pluginModel.requirements()) { - if (!existingRequirements.contains(pluginRequirement)) { - Characteristic characteristic = existingModel.characteristicByKey(pluginRequirement.characteristic().key()); - service.create(pluginRequirement, characteristic, rulesCache, session); - } - } - } - } - - public TechnicalDebtModel loadModelFromXml(String pluginKey, ValidationMessages messages, TechnicalDebtRuleCache rulesCache) { - Reader xmlFileReader = null; - try { - xmlFileReader = languageModelFinder.createReaderForXMLFile(pluginKey); - return importer.importXML(xmlFileReader, messages, rulesCache); - } finally { - IOUtils.closeQuietly(xmlFileReader); - } - } - - private void checkPluginDoNotAddNewCharacteristic(TechnicalDebtModel pluginModel, TechnicalDebtModel defaultModel) { - List characteristics = defaultModel.characteristics(); - for (Characteristic characteristic : pluginModel.characteristics()) { - if (!characteristics.contains(characteristic)) { - throw new IllegalArgumentException("The characteristic : " + characteristic.key() + " cannot be used as it's not available in default characteristics."); - } - } - } - - private void disableRequirementsOnRemovedRules(TechnicalDebtModel model, SqlSession session) { - for (Requirement requirement : model.requirements()) { - if (requirement.ruleKey() == null) { - requirement.characteristic().removeRequirement(requirement); - service.disable(requirement, session); - } - } - } - - private Collection getContributingPluginListWithoutSqale() { - Collection pluginList = newArrayList(languageModelFinder.getContributingPluginList()); - pluginList.remove(TechnicalDebtModelRepository.DEFAULT_MODEL); - return pluginList; - } - -} diff --git a/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtModelService.java b/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtModelService.java index 7537cd97d57..6a56fb8dcc2 100644 --- a/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtModelService.java +++ b/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtModelService.java @@ -29,6 +29,8 @@ import org.sonar.core.persistence.MyBatis; import org.sonar.core.technicaldebt.db.CharacteristicDao; import org.sonar.core.technicaldebt.db.CharacteristicDto; +import javax.annotation.Nullable; + public class TechnicalDebtModelService implements ServerExtension { private final MyBatis mybatis; @@ -39,34 +41,33 @@ public class TechnicalDebtModelService implements ServerExtension { this.dao = dao; } - public void create(Characteristic characteristic, SqlSession session) { - CharacteristicDto characteristicDto = CharacteristicDto.toDto(characteristic); + public void create(Characteristic characteristic, @Nullable Integer parentId, SqlSession session) { + CharacteristicDto characteristicDto = CharacteristicDto.toDto(characteristic, parentId); dao.insert(characteristicDto, session); characteristic.setId(characteristicDto.getId()); } - public void create(Characteristic characteristic) { + public void create(Characteristic characteristic, @Nullable Integer parentId) { SqlSession session = mybatis.openSession(); try { - create(characteristic, session); + create(characteristic, parentId, session); session.commit(); } finally { MyBatis.closeQuietly(session); } } - public void create(Requirement requirement, Characteristic characteristic, TechnicalDebtRuleCache ruleCache, SqlSession session) { + public void create(Requirement requirement, Integer characteristicId, TechnicalDebtRuleCache ruleCache, SqlSession session) { Rule rule = ruleCache.getByRuleKey(requirement.ruleKey()); - requirement.setCharacteristic(characteristic); - CharacteristicDto requirementDto = CharacteristicDto.toDto(requirement, rule.getId()); + CharacteristicDto requirementDto = CharacteristicDto.toDto(requirement, characteristicId, rule.getId()); dao.insert(requirementDto, session); requirement.setId(requirementDto.getId()); } - public void create(Requirement requirement, Characteristic characteristic, TechnicalDebtRuleCache ruleCache) { + public void create(Requirement requirement, Integer characteristicId, TechnicalDebtRuleCache ruleCache) { SqlSession session = mybatis.openSession(); try { - create(requirement, characteristic, ruleCache, session); + create(requirement, characteristicId, ruleCache, session); session.commit(); } finally { MyBatis.closeQuietly(session); diff --git a/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtModelSynchronizer.java b/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtModelSynchronizer.java new file mode 100644 index 00000000000..a0dd2a03362 --- /dev/null +++ b/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtModelSynchronizer.java @@ -0,0 +1,186 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.core.technicaldebt; + +import com.google.common.base.Predicate; +import com.google.common.collect.Iterables; +import org.apache.commons.io.IOUtils; +import org.apache.ibatis.session.SqlSession; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.sonar.api.ServerExtension; +import org.sonar.api.rules.Rule; +import org.sonar.api.technicaldebt.Characteristic; +import org.sonar.api.technicaldebt.Requirement; +import org.sonar.api.utils.ValidationMessages; +import org.sonar.core.persistence.MyBatis; +import org.sonar.core.technicaldebt.db.CharacteristicDao; +import org.sonar.core.technicaldebt.db.CharacteristicDto; + +import javax.annotation.Nullable; + +import java.io.Reader; +import java.util.Collection; +import java.util.List; + +import static com.google.common.collect.Lists.newArrayList; + +public class TechnicalDebtModelSynchronizer implements ServerExtension { + + private static final Logger LOG = LoggerFactory.getLogger(TechnicalDebtModelSynchronizer.class); + + private final MyBatis mybatis; + private final CharacteristicDao dao; + private final TechnicalDebtModelRepository languageModelFinder; + private final TechnicalDebtXMLImporter importer; + + public TechnicalDebtModelSynchronizer(MyBatis mybatis, CharacteristicDao dao, + TechnicalDebtModelRepository modelRepository, TechnicalDebtXMLImporter importer) { + this.mybatis = mybatis; + this.dao = dao; + this.languageModelFinder = modelRepository; + this.importer = importer; + } + + public List initAndMergePlugins(ValidationMessages messages, TechnicalDebtRuleCache rulesCache) { + SqlSession session = mybatis.openSession(); + + List model = newArrayList(); + try { + model = initAndMergePlugins(messages, rulesCache, session); + session.commit(); + } finally { + MyBatis.closeQuietly(session); + } + return model; + } + + public List initAndMergePlugins(ValidationMessages messages, TechnicalDebtRuleCache rulesCache, SqlSession session) { + TechnicalDebtModel defaultModel = loadModelFromXml(TechnicalDebtModelRepository.DEFAULT_MODEL, messages, rulesCache); + List model = loadOrCreateModelFromDb(defaultModel, messages, session); + disableRequirementsOnRemovedRules(model, rulesCache, session); + mergePlugins(model, defaultModel, messages, rulesCache, session); + messages.log(LOG); + + return model; + } + + private List loadOrCreateModelFromDb(TechnicalDebtModel defaultModel, ValidationMessages messages, SqlSession session) { + List characteristicDtos = loadModel(); + if (characteristicDtos.isEmpty()) { + return createTechnicalDebtModel(defaultModel, session); + } + return characteristicDtos; + } + + private List loadModel() { + return dao.selectEnabledCharacteristics(); + } + + private List createTechnicalDebtModel(TechnicalDebtModel defaultModel, SqlSession session) { + List characteristics = newArrayList(); + for (Characteristic rootCharacteristic : defaultModel.rootCharacteristics()) { + CharacteristicDto rootCharacteristicDto = CharacteristicDto.toDto(rootCharacteristic, null); + dao.insert(rootCharacteristicDto, session); + characteristics.add(rootCharacteristicDto); + for (Characteristic characteristic : rootCharacteristic.children()) { + CharacteristicDto characteristicDto = CharacteristicDto.toDto(characteristic, rootCharacteristicDto.getId()); + dao.insert(characteristicDto, session); + characteristics.add(characteristicDto); + } + } + return characteristics; + } + + private void mergePlugins(List existingModel, TechnicalDebtModel defaultModel, ValidationMessages messages, TechnicalDebtRuleCache rulesCache, SqlSession session) { + for (String pluginKey : getContributingPluginListWithoutSqale()) { + TechnicalDebtModel pluginModel = loadModelFromXml(pluginKey, messages, rulesCache); + checkPluginDoNotAddNewCharacteristic(pluginModel, defaultModel); + mergePlugin(pluginModel, existingModel, messages, rulesCache, session); + } + } + + private void mergePlugin(TechnicalDebtModel pluginModel, List existingModel, ValidationMessages messages, TechnicalDebtRuleCache rulesCache, SqlSession session) { + if (!messages.hasErrors()) { + for (Requirement pluginRequirement : pluginModel.requirements()) { + Rule rule = rulesCache.getByRuleKey(pluginRequirement.ruleKey()); + if (!find(existingModel, rule)) { + CharacteristicDto characteristicDto = find(existingModel, pluginRequirement.characteristic().key()); + CharacteristicDto requirementDto = CharacteristicDto.toDto(pluginRequirement, characteristicDto.getId(), rule.getId()); + dao.insert(requirementDto, session); + } + } + } + } + + private TechnicalDebtModel loadModelFromXml(String pluginKey, ValidationMessages messages, TechnicalDebtRuleCache rulesCache) { + Reader xmlFileReader = null; + try { + xmlFileReader = languageModelFinder.createReaderForXMLFile(pluginKey); + return importer.importXML(xmlFileReader, messages, rulesCache); + } finally { + IOUtils.closeQuietly(xmlFileReader); + } + } + + private void checkPluginDoNotAddNewCharacteristic(TechnicalDebtModel pluginModel, TechnicalDebtModel defaultModel) { + List characteristics = defaultModel.characteristics(); + for (Characteristic characteristic : pluginModel.characteristics()) { + if (!characteristics.contains(characteristic)) { + throw new IllegalArgumentException("The characteristic : " + characteristic.key() + " cannot be used as it's not available in default characteristics."); + } + } + } + + private void disableRequirementsOnRemovedRules(List existingModel, TechnicalDebtRuleCache rulesCache, SqlSession session) { + for (CharacteristicDto characteristicDto : existingModel) { + Integer ruleId = characteristicDto.getRuleId(); + if (ruleId != null && !rulesCache.exists(ruleId)) { + dao.disable(characteristicDto.getId(), session); + } + } + } + + private Collection getContributingPluginListWithoutSqale() { + Collection pluginList = newArrayList(languageModelFinder.getContributingPluginList()); + pluginList.remove(TechnicalDebtModelRepository.DEFAULT_MODEL); + return pluginList; + } + + private CharacteristicDto find(List existingModel, final String key) { + return Iterables.find(existingModel, new Predicate() { + @Override + public boolean apply(@Nullable CharacteristicDto input) { + return input.getKey().equals(key); + } + }); + } + + private boolean find(List existingModel, final Rule rule) { + return Iterables.any(existingModel, new Predicate() { + @Override + public boolean apply(@Nullable CharacteristicDto input) { + Integer ruleId = input.getRuleId(); + return ruleId != null && ruleId.equals(rule.getId()); + } + }); + } +} diff --git a/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtRuleCache.java b/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtRuleCache.java index d447855dcf3..f5b9c06573e 100644 --- a/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtRuleCache.java +++ b/sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtRuleCache.java @@ -36,6 +36,7 @@ public class TechnicalDebtRuleCache { private final RuleFinder ruleFinder; private Map> cachedRules; + private Map cachedRulesId; public TechnicalDebtRuleCache(RuleFinder ruleFinder) { this.ruleFinder = ruleFinder; @@ -57,6 +58,11 @@ public class TechnicalDebtRuleCache { return getRule(rule.getRepositoryKey(), rule.getKey()) != null; } + public boolean exists(Integer ruleId) { + initRules(); + return cachedRulesId.get(ruleId) != null; + } + public boolean exists(RuleKey ruleKey) { return getByRuleKey(ruleKey) != null; } @@ -69,6 +75,7 @@ public class TechnicalDebtRuleCache { private void loadRules() { cachedRules = Maps.newHashMap(); + cachedRulesId = Maps.newHashMap(); Collection rules = ruleFinder.findAll(RuleQuery.create()); for (Rule rule : rules) { if(!cachedRules.containsKey(rule.getRepositoryKey())) { @@ -77,6 +84,7 @@ public class TechnicalDebtRuleCache { Map cachedRepository = cachedRules.get(rule.getRepositoryKey()); if(!cachedRepository.containsKey(rule.getKey())) { cachedRepository.put(rule.getKey(), rule); + cachedRulesId.put(rule.getId(), rule); } } } @@ -89,4 +97,5 @@ public class TechnicalDebtRuleCache { } return null; } + } diff --git a/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDto.java b/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDto.java index 997afb6f1b8..99bc0c1f7be 100644 --- a/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDto.java +++ b/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDto.java @@ -196,13 +196,12 @@ public class CharacteristicDto implements Serializable { .setUpdatedAt(updatedAt); } - public static CharacteristicDto toDto(Characteristic characteristic) { - Characteristic parent = characteristic.parent(); + public static CharacteristicDto toDto(Characteristic characteristic, @Nullable Integer parentId) { return new CharacteristicDto() .setKey(characteristic.key()) .setName(characteristic.name()) .setOrder(characteristic.order()) - .setParentId(parent != null ? parent.id() : null) + .setParentId(parentId) .setEnabled(true) .setCreatedAt(characteristic.createdAt()) .setUpdatedAt(characteristic.updatedAt()); @@ -220,11 +219,10 @@ public class CharacteristicDto implements Serializable { .setUpdatedAt(updatedAt); } - public static CharacteristicDto toDto(Requirement requirement, Integer ruleId) { - Characteristic parent = requirement.characteristic(); + public static CharacteristicDto toDto(Requirement requirement, Integer characteristicId, Integer ruleId) { return new CharacteristicDto() .setRuleId(ruleId) - .setParentId(parent != null ? parent.id() : null) + .setParentId(characteristicId) .setFunction(requirement.function()) .setFactorValue(requirement.factor().getValue()) .setFactorUnit(requirement.factor().getUnit()) diff --git a/sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtManagerTest.java b/sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtModelSynchronizerTest.java similarity index 61% rename from sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtManagerTest.java rename to sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtModelSynchronizerTest.java index bb994ed396f..0fb3f431ec2 100644 --- a/sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtManagerTest.java +++ b/sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtModelSynchronizerTest.java @@ -20,22 +20,27 @@ package org.sonar.core.technicaldebt; +import com.google.common.collect.Lists; import org.apache.ibatis.session.SqlSession; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; 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.technicaldebt.Characteristic; import org.sonar.api.technicaldebt.Requirement; import org.sonar.api.technicaldebt.WorkUnit; import org.sonar.api.utils.ValidationMessages; import org.sonar.core.persistence.MyBatis; +import org.sonar.core.technicaldebt.db.CharacteristicDao; +import org.sonar.core.technicaldebt.db.CharacteristicDto; -import java.io.FileNotFoundException; import java.io.Reader; import java.util.Collections; +import java.util.List; import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; @@ -45,7 +50,7 @@ import static org.mockito.Matchers.eq; import static org.mockito.Mockito.*; @RunWith(MockitoJUnitRunner.class) -public class TechnicalDebtManagerTest { +public class TechnicalDebtModelSynchronizerTest { @Mock MyBatis myBatis; @@ -60,17 +65,14 @@ public class TechnicalDebtManagerTest { TechnicalDebtRuleCache ruleCache; @Mock - TechnicalDebtFinder sqaleModelFinder; - - @Mock - TechnicalDebtModelService service; + CharacteristicDao dao; @Mock TechnicalDebtXMLImporter xmlImporter; private TechnicalDebtModel defaultModel; - private TechnicalDebtManager manager; + private TechnicalDebtModelSynchronizer manager; @Before public void initAndMerge() throws Exception { @@ -81,23 +83,28 @@ public class TechnicalDebtManagerTest { when(technicalDebtModelRepository.createReaderForXMLFile("technical-debt")).thenReturn(defaultModelReader); when(xmlImporter.importXML(eq(defaultModelReader), any(ValidationMessages.class), eq(ruleCache))).thenReturn(defaultModel); - manager = new TechnicalDebtManager(myBatis, service, sqaleModelFinder, technicalDebtModelRepository, xmlImporter); + manager = new TechnicalDebtModelSynchronizer(myBatis, dao, technicalDebtModelRepository, xmlImporter); } @Test public void create_default_model_on_first_execution_when_no_plugin() throws Exception { Characteristic rootCharacteristic = new Characteristic().setKey("PORTABILITY"); - Characteristic characteristic = new Characteristic().setKey("COMPILER_RELATED_PORTABILITY").setParent(rootCharacteristic); + new Characteristic().setKey("COMPILER_RELATED_PORTABILITY").setParent(rootCharacteristic); defaultModel.addRootCharacteristic(rootCharacteristic); when(technicalDebtModelRepository.getContributingPluginList()).thenReturn(Collections.emptyList()); - when(sqaleModelFinder.findAll()).thenReturn(new TechnicalDebtModel()); + when(dao.selectEnabledCharacteristics()).thenReturn(Lists.newArrayList()); manager.initAndMergePlugins(ValidationMessages.create(), ruleCache); - verify(service).create(eq(rootCharacteristic), eq(session)); - verify(service).create(eq(characteristic), eq(session)); - verifyNoMoreInteractions(service); + verify(dao).selectEnabledCharacteristics(); + ArgumentCaptor characteristicCaptor = ArgumentCaptor.forClass(CharacteristicDto.class); + verify(dao, times(2)).insert(characteristicCaptor.capture(), eq(session)); + + List result = characteristicCaptor.getAllValues(); + assertThat(result.get(0).getKey()).isEqualTo("PORTABILITY"); + assertThat(result.get(1).getKey()).isEqualTo("COMPILER_RELATED_PORTABILITY"); + verifyNoMoreInteractions(dao); } @Test @@ -108,7 +115,7 @@ public class TechnicalDebtManagerTest { defaultModel.addRootCharacteristic(defaultRootCharacteristic); // No db model - when(sqaleModelFinder.findAll()).thenReturn(new TechnicalDebtModel()); + when(dao.selectEnabledCharacteristics()).thenReturn(Lists.newArrayList()); // Java model TechnicalDebtModel javaModel = new TechnicalDebtModel(); @@ -116,22 +123,29 @@ public class TechnicalDebtManagerTest { Characteristic javaCharacteristic = new Characteristic().setKey("COMPILER_RELATED_PORTABILITY").setParent(javaRootCharacteristic); javaModel.addRootCharacteristic(javaRootCharacteristic); + Rule rule = Rule.create(); + rule.setId(10); RuleKey ruleKey = RuleKey.of("checkstyle", "import"); - when(ruleCache.exists(ruleKey)).thenReturn(true); - Requirement javaRequirement = new Requirement().setRuleKey(ruleKey) + when(ruleCache.getByRuleKey(ruleKey)).thenReturn(rule); + new Requirement().setRuleKey(ruleKey) .setFunction("linear").setFactor(WorkUnit.create(30.0, WorkUnit.MINUTES)).setCharacteristic(javaCharacteristic); Reader javaModelReader = mock(Reader.class); - when(technicalDebtModelRepository.createReaderForXMLFile("java")).thenReturn(javaModelReader); when(xmlImporter.importXML(eq(javaModelReader), any(ValidationMessages.class), eq(ruleCache))).thenReturn(javaModel); + when(technicalDebtModelRepository.createReaderForXMLFile("java")).thenReturn(javaModelReader); when(technicalDebtModelRepository.getContributingPluginList()).thenReturn(newArrayList("java")); manager.initAndMergePlugins(ValidationMessages.create(), ruleCache); - verify(service).create(eq(defaultRootCharacteristic), eq(session)); - verify(service).create(eq(characteristic), eq(session)); - verify(service).create(eq(javaRequirement), eq(javaCharacteristic), eq(ruleCache), eq(session)); - verifyNoMoreInteractions(service); + verify(dao).selectEnabledCharacteristics(); + ArgumentCaptor characteristicCaptor = ArgumentCaptor.forClass(CharacteristicDto.class); + verify(dao, times(3)).insert(characteristicCaptor.capture(), eq(session)); + + List result = characteristicCaptor.getAllValues(); + assertThat(result.get(0).getKey()).isEqualTo("PORTABILITY"); + assertThat(result.get(1).getKey()).isEqualTo("COMPILER_RELATED_PORTABILITY"); + assertThat(result.get(2).getRuleId()).isEqualTo(10); + verifyNoMoreInteractions(dao); } @Test @@ -142,18 +156,17 @@ public class TechnicalDebtManagerTest { defaultModel.addRootCharacteristic(defaultRootCharacteristic); // Db model - TechnicalDebtModel dbModel = new TechnicalDebtModel(); - Characteristic dbRootCharacteristic = new Characteristic().setKey("PORTABILITY"); - Characteristic dbCharacteristic = new Characteristic().setKey("COMPILER_RELATED_PORTABILITY").setParent(dbRootCharacteristic); - dbModel.addRootCharacteristic(dbRootCharacteristic); + CharacteristicDto dbRootCharacteristic = new CharacteristicDto().setId(1).setKey("PORTABILITY"); + CharacteristicDto dbCharacteristic = new CharacteristicDto().setId(2).setKey("COMPILER_RELATED_PORTABILITY").setParentId(1); + CharacteristicDto requirement = new CharacteristicDto().setId(3) + .setRuleId(10).setParentId(2).setFactorValue(30.0).setFactorUnit("mn"); RuleKey ruleKey1 = RuleKey.of("checkstyle", "import"); - when(ruleCache.exists(ruleKey1)).thenReturn(true); - // Existing requirement - new Requirement().setRuleKey(ruleKey1) - .setFunction("linear").setFactor(WorkUnit.create(30.0, WorkUnit.MINUTES)).setCharacteristic(dbCharacteristic); - - when(sqaleModelFinder.findAll()).thenReturn(dbModel); + Rule rule1 = Rule.create(); + rule1.setId(10); + when(ruleCache.getByRuleKey(ruleKey1)).thenReturn(rule1); + when(ruleCache.exists(10)).thenReturn(true); + when(dao.selectEnabledCharacteristics()).thenReturn(newArrayList(dbRootCharacteristic, dbCharacteristic, requirement)); // Java model TechnicalDebtModel javaModel = new TechnicalDebtModel(); @@ -162,9 +175,12 @@ public class TechnicalDebtManagerTest { javaModel.addRootCharacteristic(javaRootCharacteristic); RuleKey ruleKey2 = RuleKey.of("checkstyle", "export"); - when(ruleCache.exists(ruleKey2)).thenReturn(true); + Rule rule2 = Rule.create(); + rule2.setId(11); + when(ruleCache.getByRuleKey(ruleKey2)).thenReturn(rule2); + // New requirement - Requirement javaRequirement = new Requirement().setRuleKey(ruleKey2) + new Requirement().setRuleKey(ruleKey2) .setFunction("linear").setFactor(WorkUnit.create(1.0, WorkUnit.HOURS)).setCharacteristic(javaCharacteristic); Reader javaModelReader = mock(Reader.class); @@ -174,8 +190,11 @@ public class TechnicalDebtManagerTest { manager.initAndMergePlugins(ValidationMessages.create(), ruleCache); - verify(service).create(eq(javaRequirement), eq(javaCharacteristic), eq(ruleCache), eq(session)); - verifyNoMoreInteractions(service); + verify(dao).selectEnabledCharacteristics(); + ArgumentCaptor characteristicCaptor = ArgumentCaptor.forClass(CharacteristicDto.class); + verify(dao).insert(characteristicCaptor.capture(), eq(session)); + assertThat(characteristicCaptor.getValue().getRuleId()).isEqualTo(11); + verifyNoMoreInteractions(dao); } @Test @@ -186,32 +205,35 @@ public class TechnicalDebtManagerTest { defaultModel.addRootCharacteristic(defaultRootCharacteristic); // Db model - TechnicalDebtModel dbModel = new TechnicalDebtModel(); - Characteristic dbRootCharacteristic = new Characteristic().setKey("PORTABILITY"); - Characteristic dbCharacteristic = new Characteristic().setKey("COMPILER_RELATED_PORTABILITY").setParent(dbRootCharacteristic); - dbModel.addRootCharacteristic(dbRootCharacteristic); - + CharacteristicDto dbRootCharacteristic = new CharacteristicDto().setId(1).setKey("PORTABILITY"); + CharacteristicDto dbCharacteristic = new CharacteristicDto().setId(2).setKey("COMPILER_RELATED_PORTABILITY").setParentId(1); // To be disabled as rule does not exists - Requirement dbRequirement = new Requirement().setRuleKey(null) - .setFunction("linear").setFactor(WorkUnit.create(30.0, WorkUnit.MINUTES)).setCharacteristic(dbCharacteristic); + CharacteristicDto requirement = new CharacteristicDto().setId(3) + .setRuleId(10).setParentId(2).setFactorValue(30.0).setFactorUnit("mn"); + + when(ruleCache.exists(10)).thenReturn(false); - when(sqaleModelFinder.findAll()).thenReturn(dbModel); + when(dao.selectEnabledCharacteristics()).thenReturn(newArrayList(dbRootCharacteristic, dbCharacteristic, requirement)); - TechnicalDebtModel result = manager.initAndMergePlugins(ValidationMessages.create(), ruleCache); + manager.initAndMergePlugins(ValidationMessages.create(), ruleCache); - verify(service).disable(eq(dbRequirement), eq(session)); - verifyNoMoreInteractions(service); - assertThat(result.requirements()).isEmpty(); + verify(dao).selectEnabledCharacteristics(); + verify(dao).disable(eq(3), eq(session)); + verifyNoMoreInteractions(dao); } @Test public void fail_when_plugin_defines_characteristics_not_defined_in_default_model() throws Exception { try { - // Default and db model + // Default model Characteristic defaultRootCharacteristic = new Characteristic().setKey("PORTABILITY"); new Characteristic().setKey("COMPILER_RELATED_PORTABILITY").setParent(defaultRootCharacteristic); defaultModel.addRootCharacteristic(defaultRootCharacteristic); - when(sqaleModelFinder.findAll()).thenReturn(defaultModel); + + // Db model + CharacteristicDto dbRootCharacteristic = new CharacteristicDto().setId(1).setKey("PORTABILITY"); + CharacteristicDto dbCharacteristic = new CharacteristicDto().setId(2).setKey("COMPILER_RELATED_PORTABILITY").setParentId(1); + when(dao.selectEnabledCharacteristics()).thenReturn(newArrayList(dbRootCharacteristic, dbCharacteristic)); // Java model TechnicalDebtModel javaModel = new TechnicalDebtModel(); @@ -229,39 +251,9 @@ public class TechnicalDebtManagerTest { } catch (Exception e) { assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("The characteristic : NEW_CHARACTERISTIC cannot be used as it's not available in default characteristics."); } finally { - verifyZeroInteractions(service); + verify(dao).selectEnabledCharacteristics(); + verifyNoMoreInteractions(dao); } } - @Test - public void provided_plugin_should_not_override_default_characteristics_name() throws FileNotFoundException { - // Default and db model - Characteristic defaultRootCharacteristic = new Characteristic().setKey("PORTABILITY").setName("Portability"); - new Characteristic().setKey("COMPILER_RELATED_PORTABILITY").setName("Compiler").setParent(defaultRootCharacteristic); - defaultModel.addRootCharacteristic(defaultRootCharacteristic); - when(sqaleModelFinder.findAll()).thenReturn(defaultModel); - - // Java model - TechnicalDebtModel javaModel = new TechnicalDebtModel(); - Characteristic javaRootCharacteristic = new Characteristic().setKey("PORTABILITY").setName("New Portability Name"); - new Characteristic().setKey("COMPILER_RELATED_PORTABILITY").setName("New Compiler Name").setParent(javaRootCharacteristic); - javaModel.addRootCharacteristic(javaRootCharacteristic); - - Reader javaModelReader = mock(Reader.class); - when(technicalDebtModelRepository.createReaderForXMLFile("java")).thenReturn(javaModelReader); - when(xmlImporter.importXML(eq(javaModelReader), any(ValidationMessages.class), eq(ruleCache))).thenReturn(javaModel); - when(technicalDebtModelRepository.getContributingPluginList()).thenReturn(newArrayList("java")); - - TechnicalDebtModel model = manager.initAndMergePlugins(ValidationMessages.create(), ruleCache); - - // Default model values - assertThat(model.characteristicByKey("PORTABILITY").name()).isEqualTo("Portability"); - assertThat(model.characteristicByKey("COMPILER_RELATED_PORTABILITY").name()).isEqualTo("Compiler"); - - // Plugin has renamed it, but the value should stay as defined by default model - assertThat(model.characteristicByKey("PORTABILITY").name()).isEqualTo("Portability"); - - verifyZeroInteractions(service); - } - } diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java index f88f77eeea1..d5cc3c8afd4 100644 --- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java +++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java @@ -307,12 +307,11 @@ public final class Platform { // technical debt servicesContainer.addSingleton(InternalRubyTechnicalDebtService.class); - servicesContainer.addSingleton(TechnicalDebtManager.class); + servicesContainer.addSingleton(TechnicalDebtModelSynchronizer.class); servicesContainer.addSingleton(TechnicalDebtModelRepository.class); servicesContainer.addSingleton(TechnicalDebtXMLImporter.class); servicesContainer.addSingleton(TechnicalDebtConverter.class); servicesContainer.addSingleton(TechnicalDebtFormatter.class); - servicesContainer.addSingleton(TechnicalDebtModelService.class); servicesContainer.addSingleton(TechnicalDebtFinder.class); // text 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 c8b05813792..293659b5caa 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 @@ -24,20 +24,20 @@ import org.slf4j.LoggerFactory; import org.sonar.api.rules.RuleFinder; import org.sonar.api.utils.TimeProfiler; import org.sonar.api.utils.ValidationMessages; -import org.sonar.core.technicaldebt.TechnicalDebtManager; +import org.sonar.core.technicaldebt.TechnicalDebtModelSynchronizer; import org.sonar.core.technicaldebt.TechnicalDebtRuleCache; public final class RegisterTechnicalDebtModel { private static final Logger LOGGER = LoggerFactory.getLogger(RegisterTechnicalDebtModel.class); - private final TechnicalDebtManager manager; + private final TechnicalDebtModelSynchronizer manager; private final RuleFinder ruleFinder; /** * @param registerRulesBeforeModels used only to be started after the creation of check templates */ - public RegisterTechnicalDebtModel(TechnicalDebtManager manager, RuleFinder ruleFinder, RegisterRules registerRulesBeforeModels) { + public RegisterTechnicalDebtModel(TechnicalDebtModelSynchronizer manager, RuleFinder ruleFinder, RegisterRules registerRulesBeforeModels) { this.manager = manager; this.ruleFinder = ruleFinder; } 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 673364f74c7..ca8201ba9f6 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 @@ -22,7 +22,7 @@ package org.sonar.server.startup; import org.junit.Test; import org.sonar.api.rules.RuleFinder; import org.sonar.api.utils.ValidationMessages; -import org.sonar.core.technicaldebt.TechnicalDebtManager; +import org.sonar.core.technicaldebt.TechnicalDebtModelSynchronizer; import org.sonar.core.technicaldebt.TechnicalDebtRuleCache; import static org.mockito.Matchers.any; @@ -32,7 +32,7 @@ public class RegisterTechnicalDebtModelTest { @Test public void create_model() throws Exception { - TechnicalDebtManager manger = mock(TechnicalDebtManager.class); + TechnicalDebtModelSynchronizer manger = mock(TechnicalDebtModelSynchronizer.class); RuleFinder ruleFinder = mock(RuleFinder.class); RegisterTechnicalDebtModel sqaleDefinition = new RegisterTechnicalDebtModel(manger, ruleFinder, null); -- 2.39.5