From: Julien Lancelot Date: Thu, 3 Apr 2014 09:45:52 +0000 (+0200) Subject: SONAR-5056 When update rule, if sub characteristic or remediation function is differe... X-Git-Tag: 4.3~164 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=9fc6df0d576e72ae3a6746868684d2802a1291b8;p=sonarqube.git SONAR-5056 When update rule, if sub characteristic or remediation function is different from default values, update both overridden values --- diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/debt/internal/DefaultDebtRemediationFunction.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/debt/internal/DefaultDebtRemediationFunction.java index f0cbd4e0638..cc8d58b2002 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/debt/internal/DefaultDebtRemediationFunction.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/debt/internal/DefaultDebtRemediationFunction.java @@ -37,7 +37,10 @@ public class DefaultDebtRemediationFunction implements DebtRemediationFunction { private final String coefficient; private final String offset; - public DefaultDebtRemediationFunction(Type type, @Nullable String coefficient, @Nullable String offset) { + public DefaultDebtRemediationFunction(@Nullable Type type, @Nullable String coefficient, @Nullable String offset) { + if (type == null) { + throw new IllegalArgumentException("Remediation function type cannot be null"); + } this.type = type; this.coefficient = sanitizeValue("coefficient", coefficient); this.offset = sanitizeValue("offset", offset); diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/debt/DefaultDebtRemediationFunctionTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/debt/DefaultDebtRemediationFunctionTest.java index 8bdefe18651..b165c6b1556 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/server/debt/DefaultDebtRemediationFunctionTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/debt/DefaultDebtRemediationFunctionTest.java @@ -60,6 +60,16 @@ public class DefaultDebtRemediationFunctionTest { assertThat(function.offset()).isEqualTo("10min"); } + @Test + public void fail_to_when_no_type() { + try { + new DefaultDebtRemediationFunction(null, "5min", "10h"); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage("Remediation function type cannot be null"); + } + } + @Test public void fail_to_create_linear_when_no_coefficient() { try { diff --git a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelBackup.java b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelBackup.java index 40e8cc772db..05663999d7a 100644 --- a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelBackup.java +++ b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelBackup.java @@ -40,6 +40,7 @@ import org.sonar.core.rule.RuleDao; import org.sonar.core.rule.RuleDto; import org.sonar.core.technicaldebt.db.CharacteristicDao; import org.sonar.core.technicaldebt.db.CharacteristicDto; +import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.rule.RuleDefinitionsLoader; import org.sonar.server.rule.RuleRegistry; import org.sonar.server.user.UserSession; @@ -143,7 +144,7 @@ public class DebtModelBackup implements ServerComponent { SqlSession session = mybatis.openSession(); try { // Restore characteristics - List allCharacteristicDtos = restoreCharacteristics(loadModelFromPlugin(DebtModelPluginRepository.DEFAULT_MODEL), true, updateDate, session); + List allCharacteristicDtos = restoreCharacteristics(loadModelFromPlugin(DebtModelPluginRepository.DEFAULT_MODEL), updateDate, session); // Load default rule definitions RulesDefinition.Context context = defLoader.load(); @@ -159,7 +160,7 @@ public class DebtModelBackup implements ServerComponent { RulesDefinition.Rule ruleDef = ruleDef(rule.getRepositoryKey(), rule.getRuleKey(), rules); if (ruleDef != null) { String subCharacteristicKey = ruleDef.debtSubCharacteristic(); - CharacteristicDto subCharacteristicDto = characteristicByKey(subCharacteristicKey, allCharacteristicDtos); + CharacteristicDto subCharacteristicDto = characteristicByKey(subCharacteristicKey, allCharacteristicDtos, false); DebtRemediationFunction remediationFunction = ruleDef.debtRemediationFunction(); boolean hasDebtDefinition = subCharacteristicDto != null && remediationFunction != null; @@ -205,7 +206,7 @@ public class DebtModelBackup implements ServerComponent { Date updateDate = new Date(system2.now()); SqlSession session = mybatis.openSession(); try { - List allCharacteristicDtos = restoreCharacteristics(characteristicsXMLImporter.importXML(xml), languageKey == null, updateDate, session); + List allCharacteristicDtos = restoreCharacteristics(characteristicsXMLImporter.importXML(xml), updateDate, session); restoreRules(allCharacteristicDtos, rules(languageKey, session), rulesXMLImporter.importXML(xml, validationMessages), validationMessages, updateDate, session); session.commit(); @@ -223,21 +224,21 @@ public class DebtModelBackup implements ServerComponent { // rule does not exists in the XML disabledOverriddenRuleDebt(rule); } else { - CharacteristicDto subCharacteristicDto = characteristicByKey(ruleDebt.subCharacteristicKey(), allCharacteristicDtos); - if (subCharacteristicDto == null) { - // TODO not possible, all char should have been created - // rule is linked on a not existing characteristic - disabledOverriddenRuleDebt(rule); + CharacteristicDto subCharacteristicDto = characteristicByKey(ruleDebt.subCharacteristicKey(), allCharacteristicDtos, true); + boolean isSameCharacteristicAsDefault = subCharacteristicDto.getId().equals(rule.getDefaultSubCharacteristicId()); + boolean isSameFunctionAsDefault = isSameRemediationFunction(ruleDebt, rule); + + // Update only if given characteristic is not the same as the default one or if given function is not the same as the default one + if (!isSameCharacteristicAsDefault || !isSameFunctionAsDefault) { + rule.setSubCharacteristicId(subCharacteristicDto.getId()); + rule.setRemediationFunction(ruleDebt.function().name()); + rule.setRemediationCoefficient(ruleDebt.coefficient()); + rule.setRemediationOffset(ruleDebt.offset()); } else { - boolean isSameCharacteristicAsDefault = subCharacteristicDto.getId().equals(rule.getDefaultSubCharacteristicId()); - boolean isSameFunctionAsDefault = isSameRemediationFunction(ruleDebt, rule); - // If given characteristic is the same as the default one, set nothing in overridden characteristic - rule.setSubCharacteristicId(!isSameCharacteristicAsDefault ? subCharacteristicDto.getId() : null); - - // If given function is the same as the default one, set nothing in overridden function - rule.setRemediationFunction(!isSameFunctionAsDefault ? ruleDebt.function().name() : null); - rule.setRemediationCoefficient(!isSameFunctionAsDefault ? ruleDebt.coefficient() : null); - rule.setRemediationOffset(!isSameFunctionAsDefault ? ruleDebt.offset() : null); + rule.setSubCharacteristicId(null); + rule.setRemediationFunction(null); + rule.setRemediationCoefficient(null); + rule.setRemediationOffset(null); } } rule.setUpdatedAt(updateDate); @@ -253,7 +254,7 @@ public class DebtModelBackup implements ServerComponent { } @VisibleForTesting - List restoreCharacteristics(DebtModel targetModel, boolean disableNoMoreExistingCharacteristics, Date updateDate, SqlSession session) { + List restoreCharacteristics(DebtModel targetModel, Date updateDate, SqlSession session) { List sourceCharacteristics = dao.selectEnabledCharacteristics(session); List result = newArrayList(); @@ -266,12 +267,10 @@ public class DebtModelBackup implements ServerComponent { result.add(restoreCharacteristic(subCharacteristic, rootCharacteristicDto.getId(), sourceCharacteristics, updateDate, session)); } } - if (disableNoMoreExistingCharacteristics) { - // Disable no more existing characteristics - for (CharacteristicDto sourceCharacteristic : sourceCharacteristics) { - if (targetModel.characteristicByKey(sourceCharacteristic.getKey()) == null) { - debtModelOperations.delete(sourceCharacteristic, updateDate, session); - } + // Disable no more existing characteristics + for (CharacteristicDto sourceCharacteristic : sourceCharacteristics) { + if (targetModel.characteristicByKey(sourceCharacteristic.getKey()) == null) { + debtModelOperations.delete(sourceCharacteristic, updateDate, session); } } return result; @@ -279,7 +278,7 @@ public class DebtModelBackup implements ServerComponent { private CharacteristicDto restoreCharacteristic(DebtCharacteristic targetCharacteristic, @Nullable Integer parentId, List sourceCharacteristics, Date updateDate, SqlSession session) { - CharacteristicDto sourceCharacteristic = characteristicByKey(targetCharacteristic.key(), sourceCharacteristics); + CharacteristicDto sourceCharacteristic = characteristicByKey(targetCharacteristic.key(), sourceCharacteristics, false); if (sourceCharacteristic == null) { CharacteristicDto newCharacteristic = toDto(targetCharacteristic, parentId).setCreatedAt(updateDate); dao.insert(newCharacteristic, session); @@ -362,17 +361,20 @@ public class DebtModelBackup implements ServerComponent { }, null); } - @CheckForNull - private static CharacteristicDto characteristicByKey(@Nullable final String key, List characteristicDtos) { + private static CharacteristicDto characteristicByKey(@Nullable final String key, List characteristicDtos, boolean failIfNotFound) { if (key == null) { return null; } - return Iterables.find(characteristicDtos, new Predicate() { + CharacteristicDto dto = Iterables.find(characteristicDtos, new Predicate() { @Override public boolean apply(@Nullable CharacteristicDto input) { return input != null && key.equals(input.getKey()); } }, null); + if (dto == null && failIfNotFound) { + throw new NotFoundException(String.format("Characteristic '%s' has not been found", key)); + } + return dto; } private static List subCharacteristics(final Integer parentId, List allCharacteristics) { diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleOperations.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleOperations.java index 17d3b75cadb..5fd1474efe2 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/RuleOperations.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleOperations.java @@ -258,37 +258,40 @@ public class RuleOperations implements ServerComponent { checkPermission(userSession); SqlSession session = myBatis.openSession(); try { - boolean updated = false; + boolean needUpdate = false; RuleDto ruleDto = ruleDao.selectByKey(ruleChange.ruleKey(), session); if (ruleDto == null) { throw new NotFoundException(String.format("Unknown rule '%s'", ruleChange.ruleKey())); } String subCharacteristicKey = ruleChange.debtCharacteristicKey(); + + // A sub-characteristic is given -> update rule debt if given values are different from overridden ones and from default ones if (!Strings.isNullOrEmpty(subCharacteristicKey)) { CharacteristicDto subCharacteristic = characteristicDao.selectByKey(subCharacteristicKey, session); if (subCharacteristic == null) { throw new NotFoundException(String.format("Unknown sub characteristic '%s'", ruleChange.debtCharacteristicKey())); } - // TODO + boolean iSameAsOverriddenValues = subCharacteristic.getId().equals(ruleDto.getSubCharacteristicId()) + && isSameRemediationFunction(ruleChange, ruleDto.getRemediationFunction(), ruleDto.getRemediationCoefficient(), ruleDto.getRemediationOffset()); + boolean iSameAsDefaultValues = subCharacteristic.getId().equals(ruleDto.getDefaultSubCharacteristicId()) + && isSameRemediationFunction(ruleChange, ruleDto.getDefaultRemediationFunction(), ruleDto.getDefaultRemediationCoefficient(), ruleDto.getDefaultRemediationOffset()); - // New sub characteristic is not equals to existing one and not equals to default value -> update it - if (!subCharacteristic.getId().equals(ruleDto.getSubCharacteristicId()) && !subCharacteristic.getId().equals(ruleDto.getDefaultSubCharacteristicId()) ) { + // New sub characteristic is not equals to existing one and not equals to default value + // and new remediation function is not equals to existing one and not equals to default value -> update overridden values + if (!iSameAsOverriddenValues && !iSameAsDefaultValues) { ruleDto.setSubCharacteristicId(subCharacteristic.getId()); - updated = true; - } - // New remediation function is not equals to existing one and not equals to default value -> update it - if (!isSameRemediationFunction(ruleChange, ruleDto.getRemediationFunction(), ruleDto.getRemediationCoefficient(), ruleDto.getRemediationOffset()) - && !isSameRemediationFunction(ruleChange, ruleDto.getDefaultRemediationFunction(), ruleDto.getDefaultRemediationCoefficient(), ruleDto.getDefaultRemediationOffset())) { DefaultDebtRemediationFunction debtRemediationFunction = new DefaultDebtRemediationFunction(DebtRemediationFunction.Type.valueOf(ruleChange.debtRemediationFunction()), ruleChange.debtRemediationCoefficient(), ruleChange.debtRemediationOffset()); ruleDto.setRemediationFunction(debtRemediationFunction.type().name()); ruleDto.setRemediationCoefficient(debtRemediationFunction.coefficient()); ruleDto.setRemediationOffset(debtRemediationFunction.offset()); - updated = true; + needUpdate = true; } + + // No sub-characteristic is given -> disable rule debt if not already disabled } else { // Rule characteristic is not already disabled -> update it if (!ruleDto.getSubCharacteristicId().equals(RuleDto.DISABLED_CHARACTERISTIC_ID)) { @@ -296,11 +299,11 @@ public class RuleOperations implements ServerComponent { ruleDto.setRemediationFunction(null); ruleDto.setRemediationCoefficient(null); ruleDto.setRemediationOffset(null); - updated = true; + needUpdate = true; } } - if (updated) { + if (needUpdate) { ruleDto.setUpdatedAt(new Date(system.now())); ruleDao.update(ruleDto, session); session.commit(); diff --git a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelBackupTest.java b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelBackupTest.java index 272f5c5a57a..1ff603cd064 100644 --- a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelBackupTest.java +++ b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelBackupTest.java @@ -22,7 +22,6 @@ package org.sonar.server.debt; import org.apache.ibatis.session.SqlSession; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; @@ -250,7 +249,6 @@ public class DebtModelBackupTest { new DebtModel() .addRootCharacteristic(new DefaultDebtCharacteristic().setKey("PORTABILITY").setName("Portability").setOrder(1)) .addSubCharacteristic(new DefaultDebtCharacteristic().setKey("COMPILER").setName("Compiler"), "PORTABILITY"), - true, now, session ); @@ -288,7 +286,6 @@ public class DebtModelBackupTest { new DebtModel() .addRootCharacteristic(new DefaultDebtCharacteristic().setKey("PORTABILITY").setName("Portability").setOrder(1)) .addSubCharacteristic(new DefaultDebtCharacteristic().setKey("COMPILER").setName("Compiler"), "PORTABILITY"), - true, now, session ); @@ -326,7 +323,6 @@ public class DebtModelBackupTest { new DebtModel() .addRootCharacteristic(new DefaultDebtCharacteristic().setKey("PORTABILITY").setName("Portability").setOrder(1)) .addSubCharacteristic(new DefaultDebtCharacteristic().setKey("COMPILER").setName("Compiler"), "PORTABILITY"), - true, now, session ); @@ -353,7 +349,7 @@ public class DebtModelBackupTest { when(dao.selectEnabledCharacteristics(session)).thenReturn(newArrayList(dto1, dto2)); - debtModelBackup.restoreCharacteristics(new DebtModel(), true, now, session); + debtModelBackup.restoreCharacteristics(new DebtModel(), now, session); verify(debtModelOperations).delete(dto1, now, session); verify(debtModelOperations).delete(dto2, now, session); @@ -467,81 +463,24 @@ public class DebtModelBackupTest { } @Test - @Ignore("Wait to know how to deal with custom rule") - public void reset_model_for_custom_rules() throws Exception { - when(characteristicsXMLImporter.importXML(any(Reader.class))).thenReturn(new DebtModel() - .addRootCharacteristic(new DefaultDebtCharacteristic().setKey("PORTABILITY").setName("Portability").setOrder(1)) - .addSubCharacteristic(new DefaultDebtCharacteristic().setKey("COMPILER").setName("Compiler"), "PORTABILITY")); - - when(dao.selectEnabledCharacteristics(session)).thenReturn(newArrayList( - new CharacteristicDto().setId(1).setKey("PORTABILITY").setName("Portability updated").setOrder(2).setCreatedAt(oldDate), - new CharacteristicDto().setId(2).setKey("COMPILER").setName("Compiler updated").setParentId(1).setCreatedAt(oldDate) - )); - - when(ruleDao.selectEnablesAndNonManual(session)).thenReturn(newArrayList( - new RuleDto().setId(10).setRepositoryKey("squid").setRuleKey("Xpath").setCreatedAt(oldDate).setUpdatedAt(oldDate), - new RuleDto().setId(11).setRepositoryKey("squid").setRuleKey("XPath_1369910135").setParentId(10) - .setDefaultSubCharacteristicId(10).setDefaultRemediationFunction("LINEAR").setDefaultRemediationCoefficient("2h") - .setSubCharacteristicId(2).setRemediationFunction("LINEAR_OFFSET").setRemediationCoefficient("2h").setRemediationOffset("15min") - .setCreatedAt(oldDate).setUpdatedAt(oldDate) - )); - - RulesDefinition.Context context = new RulesDefinition.Context(); - RulesDefinition.NewRepository repo = context.createRepository("squid", "java").setName("Squid"); - RulesDefinition.NewRule newRule = repo.createRule("Xpath") - .setName("Xpath") - .setHtmlDescription("Xpath") - .setSeverity(Severity.BLOCKER) - .setStatus(RuleStatus.BETA) - .setDebtSubCharacteristic("COMPILER"); - newRule.setDebtRemediationFunction(newRule.debtRemediationFunctions().linearWithOffset("4h", "20min")); - repo.done(); - when(defLoader.load()).thenReturn(context); - - debtModelBackup.reset(); - - verify(dao).selectEnabledCharacteristics(session); - verify(dao, times(2)).update(any(CharacteristicDto.class), eq(session)); - verifyNoMoreInteractions(dao); - - verify(ruleDao).selectEnablesAndNonManual(session); - verify(ruleDao).update(ruleCaptor.capture(), eq(session)); - verifyNoMoreInteractions(ruleDao); - verify(ruleRegistry).reindex(ruleCaptor.getAllValues(), session); - - verify(session).commit(); - - RuleDto rule = ruleCaptor.getValue(); - - assertThat(rule.getDefaultSubCharacteristicId()).isEqualTo(2); - assertThat(rule.getDefaultRemediationFunction()).isEqualTo("LINEAR_OFFSET"); - assertThat(rule.getDefaultRemediationCoefficient()).isEqualTo("4h"); - assertThat(rule.getDefaultRemediationOffset()).isEqualTo("20min"); - assertThat(rule.getUpdatedAt()).isEqualTo(now); - - assertThat(rule.getSubCharacteristicId()).isNull(); - assertThat(rule.getRemediationFunction()).isNull(); - assertThat(rule.getRemediationCoefficient()).isNull(); - assertThat(rule.getRemediationOffset()).isNull(); - assertThat(rule.getUpdatedAt()).isEqualTo(now); - } - - @Test - public void restore_from_xml_with_different_characteristic_and_same_function() throws Exception { + public void restore_from_xml_with_different_characteristic_and_same_function_as_default_values() throws Exception { when(characteristicsXMLImporter.importXML(anyString())).thenReturn(new DebtModel() .addRootCharacteristic(new DefaultDebtCharacteristic().setKey("PORTABILITY").setName("Portability").setOrder(1)) .addSubCharacteristic(new DefaultDebtCharacteristic().setKey("COMPILER").setName("Compiler"), "PORTABILITY")); when(dao.selectEnabledCharacteristics(session)).thenReturn(newArrayList( new CharacteristicDto().setId(1).setKey("PORTABILITY").setName("Portability").setOrder(1).setCreatedAt(oldDate), - new CharacteristicDto().setId(2).setKey("COMPILER").setName("Compiler").setParentId(1).setCreatedAt(oldDate))); + new CharacteristicDto().setId(2).setKey("COMPILER").setName("Compiler").setParentId(1).setCreatedAt(oldDate), + new CharacteristicDto().setId(3).setKey("HARDWARE").setName("Hardware").setParentId(1).setCreatedAt(oldDate))); when(rulesXMLImporter.importXML(anyString(), any(ValidationMessages.class))).thenReturn(newArrayList(new RuleDebt() - .setRuleKey(RuleKey.of("squid", "UselessImportCheck")).setSubCharacteristicKey("COMPILER").setFunction(DebtRemediationFunction.Type.LINEAR).setCoefficient("2h"))); + .setRuleKey(RuleKey.of("squid", "UselessImportCheck")) + .setSubCharacteristicKey("COMPILER") + .setFunction(DebtRemediationFunction.Type.LINEAR).setCoefficient("2h"))); when(ruleDao.selectEnablesAndNonManual(session)).thenReturn(newArrayList( new RuleDto().setId(1).setRepositoryKey("squid").setRuleKey("UselessImportCheck") - .setDefaultSubCharacteristicId(10).setDefaultRemediationFunction("LINEAR").setDefaultRemediationCoefficient("2h") + .setDefaultSubCharacteristicId(3).setDefaultRemediationFunction("LINEAR").setDefaultRemediationCoefficient("2h") .setCreatedAt(oldDate).setUpdatedAt(oldDate) )); @@ -556,14 +495,14 @@ public class DebtModelBackupTest { RuleDto rule = ruleCaptor.getValue(); assertThat(rule.getId()).isEqualTo(1); assertThat(rule.getSubCharacteristicId()).isEqualTo(2); - assertThat(rule.getRemediationFunction()).isNull(); - assertThat(rule.getRemediationCoefficient()).isNull(); + assertThat(rule.getRemediationFunction()).isEqualTo("LINEAR"); + assertThat(rule.getRemediationCoefficient()).isEqualTo("2h"); assertThat(rule.getRemediationOffset()).isNull(); assertThat(rule.getUpdatedAt()).isEqualTo(now); } @Test - public void restore_from_xml_with_same_characteristic_and_different_function() throws Exception { + public void restore_from_xml_with_same_characteristic_and_different_function_as_default_values() throws Exception { when(characteristicsXMLImporter.importXML(anyString())).thenReturn(new DebtModel() .addRootCharacteristic(new DefaultDebtCharacteristic().setKey("PORTABILITY").setName("Portability").setOrder(1)) .addSubCharacteristic(new DefaultDebtCharacteristic().setKey("COMPILER").setName("Compiler"), "PORTABILITY")); @@ -573,7 +512,9 @@ public class DebtModelBackupTest { new CharacteristicDto().setId(2).setKey("COMPILER").setName("Compiler").setParentId(1).setCreatedAt(oldDate))); when(rulesXMLImporter.importXML(anyString(), any(ValidationMessages.class))).thenReturn(newArrayList(new RuleDebt() - .setRuleKey(RuleKey.of("squid", "UselessImportCheck")).setSubCharacteristicKey("COMPILER").setFunction(DebtRemediationFunction.Type.LINEAR_OFFSET).setCoefficient("12h").setOffset("11min"))); + .setRuleKey(RuleKey.of("squid", "UselessImportCheck")) + .setSubCharacteristicKey("COMPILER") + .setFunction(DebtRemediationFunction.Type.LINEAR_OFFSET).setCoefficient("12h").setOffset("11min"))); when(ruleDao.selectEnablesAndNonManual(session)).thenReturn(newArrayList( new RuleDto().setId(1).setRepositoryKey("squid").setRuleKey("UselessImportCheck") @@ -591,7 +532,7 @@ public class DebtModelBackupTest { RuleDto rule = ruleCaptor.getValue(); assertThat(rule.getId()).isEqualTo(1); - assertThat(rule.getSubCharacteristicId()).isNull(); + assertThat(rule.getSubCharacteristicId()).isEqualTo(2); assertThat(rule.getRemediationFunction()).isEqualTo("LINEAR_OFFSET"); assertThat(rule.getRemediationCoefficient()).isEqualTo("12h"); assertThat(rule.getRemediationOffset()).isEqualTo("11min"); @@ -599,7 +540,7 @@ public class DebtModelBackupTest { } @Test - public void restore_from_xml_with_same_characteristic_and_same_function() throws Exception { + public void restore_from_xml_with_same_characteristic_and_same_function_as_default_values() throws Exception { when(characteristicsXMLImporter.importXML(anyString())).thenReturn(new DebtModel() .addRootCharacteristic(new DefaultDebtCharacteristic().setKey("PORTABILITY").setName("Portability").setOrder(1)) .addSubCharacteristic(new DefaultDebtCharacteristic().setKey("COMPILER").setName("Compiler"), "PORTABILITY")); @@ -736,24 +677,23 @@ public class DebtModelBackupTest { } @Test - public void restore_from_xml_and_language_do_not_disable_no_more_existing_characteristics() throws Exception { + public void restore_from_xml_and_language_disable_no_more_existing_characteristics() throws Exception { when(characteristicsXMLImporter.importXML(anyString())).thenReturn(new DebtModel() - .addRootCharacteristic(new DefaultDebtCharacteristic().setKey("PORTABILITY").setName("Portability").setOrder(1)) - // No more existing characteristic - .addSubCharacteristic(new DefaultDebtCharacteristic().setKey("COMPILER").setName("Compiler"), "PORTABILITY")); + .addRootCharacteristic(new DefaultDebtCharacteristic().setKey("PORTABILITY").setName("Portability").setOrder(1))); - when(dao.selectEnabledCharacteristics(session)).thenReturn(newArrayList( - new CharacteristicDto().setId(1).setKey("PORTABILITY").setName("Portability").setOrder(1)) - ); + CharacteristicDto dto1 = new CharacteristicDto().setId(1).setKey("PORTABILITY").setName("Portability").setOrder(1); + // No more existing characteristic + CharacteristicDto dto2 = new CharacteristicDto().setId(2).setKey("COMPILER").setName("Compiler").setParentId(1); + when(dao.selectEnabledCharacteristics(session)).thenReturn(newArrayList(dto1, dto2)); debtModelBackup.restoreFromXml("", "java"); - verify(debtModelOperations, never()).delete(any(CharacteristicDto.class), eq(now), eq(session)); + verify(debtModelOperations).delete(dto2, now, session); verify(session).commit(); } @Test - public void restore_from_xml_and_language_with_rule_not_in_xml_and_linked_on_disabled_default_characteristic() throws Exception { + public void restore_from_xml_and_language_with_rule_not_in_xml() throws Exception { when(characteristicsXMLImporter.importXML(anyString())).thenReturn(new DebtModel() .addRootCharacteristic(new DefaultDebtCharacteristic().setKey("PORTABILITY").setName("Portability").setOrder(1)) .addSubCharacteristic(new DefaultDebtCharacteristic().setKey("COMPILER").setName("Compiler"), "PORTABILITY")); @@ -765,52 +705,13 @@ public class DebtModelBackupTest { when(rulesXMLImporter.importXML(anyString(), any(ValidationMessages.class))).thenReturn(Collections.emptyList()); when(ruleDao.selectEnablesAndNonManual(session)).thenReturn(newArrayList( - // Linked on a default disabled characteristic -> Rule debt should be disabled + // Rule does not exitss in XML -> debt will be disabled new RuleDto().setId(1).setRepositoryKey("squid").setRuleKey("UselessImportCheck").setLanguage("java") - .setDefaultSubCharacteristicId(3).setDefaultRemediationFunction("LINEAR").setDefaultRemediationCoefficient("2h") - .setSubCharacteristicId(2).setRemediationFunction("LINEAR_OFFSET").setRemediationCoefficient("2h").setRemediationOffset("15min") - .setCreatedAt(oldDate).setUpdatedAt(oldDate) - )); - - debtModelBackup.restoreFromXml("", "java"); - - verify(ruleDao).selectEnablesAndNonManual(session); - verify(ruleDao).update(ruleCaptor.capture(), eq(session)); - verifyNoMoreInteractions(ruleDao); - verify(ruleRegistry).reindex(ruleCaptor.getAllValues(), session); - verify(session).commit(); - - RuleDto rule = ruleCaptor.getValue(); - assertThat(rule.getId()).isEqualTo(1); - assertThat(rule.getSubCharacteristicId()).isEqualTo(-1); - assertThat(rule.getRemediationFunction()).isNull(); - assertThat(rule.getRemediationCoefficient()).isNull(); - assertThat(rule.getRemediationOffset()).isNull(); - assertThat(rule.getUpdatedAt()).isEqualTo(now); - } - - @Test - public void restore_from_xml_and_language_with_rule_linked_on_disabled_characteristic2() throws Exception { - when(characteristicsXMLImporter.importXML(anyString())).thenReturn(new DebtModel() - .addRootCharacteristic(new DefaultDebtCharacteristic().setKey("PORTABILITY").setName("Portability").setOrder(1)) - .addSubCharacteristic(new DefaultDebtCharacteristic().setKey("COMPILER").setName("Compiler"), "PORTABILITY")); - - when(dao.selectEnabledCharacteristics(session)).thenReturn(newArrayList( - new CharacteristicDto().setId(1).setKey("PORTABILITY").setName("Portability updated").setOrder(2).setCreatedAt(oldDate), - new CharacteristicDto().setId(2).setKey("COMPILER").setName("Compiler updated").setParentId(1).setCreatedAt(oldDate) - )); - - when(ruleDao.selectEnablesAndNonManual(session)).thenReturn(newArrayList( - new RuleDto().setId(1).setRepositoryKey("squid").setRuleKey("UselessImportCheck").setLanguage("java") - .setDefaultSubCharacteristicId(3).setDefaultRemediationFunction("LINEAR").setDefaultRemediationCoefficient("2h") + .setDefaultSubCharacteristicId(2).setDefaultRemediationFunction("LINEAR").setDefaultRemediationCoefficient("2h") .setSubCharacteristicId(2).setRemediationFunction("LINEAR_OFFSET").setRemediationCoefficient("2h").setRemediationOffset("15min") .setCreatedAt(oldDate).setUpdatedAt(oldDate) )); - when(rulesXMLImporter.importXML(anyString(), any(ValidationMessages.class))).thenReturn(newArrayList(new RuleDebt() - // Linked on a default disabled characteristic -> Rule debt should be disabled - .setRuleKey(RuleKey.of("squid", "UselessImportCheck")).setSubCharacteristicKey("HARDWARE").setFunction(DebtRemediationFunction.Type.LINEAR).setCoefficient("2h"))); - debtModelBackup.restoreFromXml("", "java"); verify(ruleDao).selectEnablesAndNonManual(session); diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RuleOperationsTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RuleOperationsTest.java index 687cb5bae7f..6387d64c40e 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule/RuleOperationsTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule/RuleOperationsTest.java @@ -405,7 +405,29 @@ public class RuleOperationsTest { } @Test - public void update_rule_set_characteristic_only_if_different_from_default_one() throws Exception { + public void not_update_rule_if_same_sub_characteristic_and_function() throws Exception { + RuleDto dto = new RuleDto().setId(1).setRepositoryKey("squid").setRuleKey("UselessImportCheck") + .setSubCharacteristicId(2).setRemediationFunction("CONSTANT_ISSUE").setRemediationOffset("10min"); + RuleKey ruleKey = RuleKey.of("squid", "UselessImportCheck"); + + when(ruleDao.selectByKey(ruleKey, session)).thenReturn(dto); + + CharacteristicDto subCharacteristic = new CharacteristicDto().setId(2).setKey("COMPILER").setName("Compiler").setParentId(1); + when(characteristicDao.selectByKey("COMPILER", session)).thenReturn(subCharacteristic); + + operations.updateRule( + new RuleChange().setRuleKey(ruleKey).setDebtCharacteristicKey("COMPILER") + .setDebtRemediationFunction("CONSTANT_ISSUE").setDebtRemediationOffset("10min"), + authorizedUserSession + ); + + verify(ruleDao, never()).update(any(RuleDto.class), eq(session)); + verify(session, never()).commit(); + verify(ruleRegistry, never()).reindex(any(RuleDto.class), eq(session)); + } + + @Test + public void update_rule_set_characteristic_if_different_from_default_one() throws Exception { RuleDto dto = new RuleDto().setId(1).setRepositoryKey("squid").setRuleKey("UselessImportCheck") .setDefaultSubCharacteristicId(2).setDefaultRemediationFunction("CONSTANT_ISSUE").setDefaultRemediationOffset("10min"); RuleKey ruleKey = RuleKey.of("squid", "UselessImportCheck"); @@ -419,8 +441,7 @@ public class RuleOperationsTest { when(characteristicDao.selectById(1, session)).thenReturn(characteristic); operations.updateRule( - // Characteristic is the same as the default one -> Overridden characteristic should not be set - // Remediation function is not the same as default one -> Overridden remediation function should be set + // Remediation function is not the same as default one -> Overridden value should be set new RuleChange().setRuleKey(ruleKey).setDebtCharacteristicKey("COMPILER") .setDebtRemediationFunction("LINEAR_OFFSET").setDebtRemediationCoefficient("2h").setDebtRemediationOffset("20min"), authorizedUserSession @@ -433,7 +454,7 @@ public class RuleOperationsTest { RuleDto result = ruleCaptor.getValue(); assertThat(result.getId()).isEqualTo(1); - assertThat(result.getSubCharacteristicId()).isNull(); + assertThat(result.getSubCharacteristicId()).isEqualTo(2); assertThat(result.getRemediationFunction()).isEqualTo("LINEAR_OFFSET"); assertThat(result.getRemediationCoefficient()).isEqualTo("2h"); assertThat(result.getRemediationOffset()).isEqualTo("20min"); @@ -441,7 +462,7 @@ public class RuleOperationsTest { } @Test - public void update_rule_set_remediation_function_only_if_different_from_default_one() throws Exception { + public void update_rule_set_remediation_function_if_different_from_default_one() throws Exception { RuleDto dto = new RuleDto().setId(1).setRepositoryKey("squid").setRuleKey("UselessImportCheck") .setDefaultSubCharacteristicId(6).setDefaultRemediationFunction("CONSTANT_ISSUE").setDefaultRemediationOffset("10min"); RuleKey ruleKey = RuleKey.of("squid", "UselessImportCheck"); @@ -455,8 +476,7 @@ public class RuleOperationsTest { when(characteristicDao.selectById(1, session)).thenReturn(characteristic); operations.updateRule( - // Characteristic is the not same as the default one -> Overridden characteristic should be set - // Remediation function is the same as default one -> Overridden remediation function should not be set + // Characteristic is the not same as the default one -> Overridden values should be set new RuleChange().setRuleKey(ruleKey).setDebtCharacteristicKey("COMPILER") .setDebtRemediationFunction("CONSTANT_ISSUE").setDebtRemediationOffset("10min"), authorizedUserSession @@ -470,34 +490,12 @@ public class RuleOperationsTest { assertThat(result.getId()).isEqualTo(1); assertThat(result.getSubCharacteristicId()).isEqualTo(2); - assertThat(result.getRemediationFunction()).isNull(); + assertThat(result.getRemediationFunction()).isEqualTo("CONSTANT_ISSUE"); assertThat(result.getRemediationCoefficient()).isNull(); - assertThat(result.getRemediationOffset()).isNull(); + assertThat(result.getRemediationOffset()).isEqualTo("10min"); assertThat(result.getUpdatedAt()).isEqualTo(now); } - @Test - public void not_update_rule_if_same_sub_characteristic_and_function() throws Exception { - RuleDto dto = new RuleDto().setId(1).setRepositoryKey("squid").setRuleKey("UselessImportCheck") - .setSubCharacteristicId(2).setRemediationFunction("CONSTANT_ISSUE").setRemediationOffset("10min"); - RuleKey ruleKey = RuleKey.of("squid", "UselessImportCheck"); - - when(ruleDao.selectByKey(ruleKey, session)).thenReturn(dto); - - CharacteristicDto subCharacteristic = new CharacteristicDto().setId(2).setKey("COMPILER").setName("Compiler").setParentId(1); - when(characteristicDao.selectByKey("COMPILER", session)).thenReturn(subCharacteristic); - - operations.updateRule( - new RuleChange().setRuleKey(ruleKey).setDebtCharacteristicKey("COMPILER") - .setDebtRemediationFunction("CONSTANT_ISSUE").setDebtRemediationOffset("10min"), - authorizedUserSession - ); - - verify(ruleDao, never()).update(any(RuleDto.class), eq(session)); - verify(session, never()).commit(); - verify(ruleRegistry, never()).reindex(any(RuleDto.class), eq(session)); - } - @Test public void disable_characteristic_and_remove_remediation_function_when_update_rule_with_no_sub_characteristic() throws Exception { RuleDto dto = new RuleDto().setId(1).setRepositoryKey("squid").setRuleKey("UselessImportCheck")