From 295cd390301a169063c78b6e3f39d8bf1844517b Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 18 Feb 2014 11:01:30 +0100 Subject: [PATCH] Fix quality flaws --- .../sonar/batch/debt/RuleDebtCalculator.java | 27 ++---- .../sonar/batch/debt/DebtModelLoaderTest.java | 7 +- .../batch/debt/RuleDebtCalculatorTest.java | 52 +++++----- .../technicaldebt/db/CharacteristicDto.java | 10 +- .../db/CharacteristicDtoTest.java | 96 +++++++++++++++++++ .../internal/DefaultCharacteristic.java | 8 +- 6 files changed, 140 insertions(+), 60 deletions(-) create mode 100644 sonar-core/src/test/java/org/sonar/core/technicaldebt/db/CharacteristicDtoTest.java diff --git a/sonar-batch/src/main/java/org/sonar/batch/debt/RuleDebtCalculator.java b/sonar-batch/src/main/java/org/sonar/batch/debt/RuleDebtCalculator.java index 8913edac04b..56a211e2eaa 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/debt/RuleDebtCalculator.java +++ b/sonar-batch/src/main/java/org/sonar/batch/debt/RuleDebtCalculator.java @@ -27,7 +27,6 @@ import org.sonar.api.technicaldebt.batch.TechnicalDebtModel; import org.sonar.api.technicaldebt.batch.internal.DefaultRequirement; import org.sonar.api.utils.WorkDuration; import org.sonar.api.utils.WorkDurationFactory; -import org.sonar.api.utils.WorkUnit; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -64,31 +63,17 @@ public class RuleDebtCalculator implements BatchExtension { private WorkDuration calculateTechnicalDebt(Requirement requirement, @Nullable Double effortToFix) { WorkDuration result = workDurationFactory.createFromWorkingValue(0, WorkDuration.UNIT.DAYS); - WorkUnit factorUnit = requirement.factor(); - if (factorUnit != null) { - int factorValue = (int) requirement.factor().getValue(); + int factorValue = requirement.factorValue(); + if (factorValue > 0) { int effortToFixValue = Objects.firstNonNull(effortToFix, 1).intValue(); - result = workDurationFactory.createFromWorkingValue(factorValue, toUnit(requirement.factor().getUnit())).multiply(effortToFixValue); + result = workDurationFactory.createFromWorkingValue(factorValue, requirement.factorUnit()).multiply(effortToFixValue); } - WorkUnit offsetUnit = requirement.offset(); - if (offsetUnit != null) { - int offsetValue = (int) requirement.offset().getValue(); - result = result.add(workDurationFactory.createFromWorkingValue(offsetValue, toUnit(requirement.offset().getUnit()))); + int offsetValue = requirement.offsetValue(); + if (offsetValue > 0) { + result = result.add(workDurationFactory.createFromWorkingValue(offsetValue, requirement.offsetUnit())); } - return result; } - private static WorkDuration.UNIT toUnit(String requirementUnit){ - if (requirementUnit.equals(WorkUnit.DAYS)) { - return WorkDuration.UNIT.DAYS; - } else if (requirementUnit.equals(WorkUnit.HOURS)) { - return WorkDuration.UNIT.HOURS; - } else if (requirementUnit.equals(WorkUnit.MINUTES)) { - return WorkDuration.UNIT.MINUTES; - } - throw new IllegalStateException("Invalid unit : " + requirementUnit); - } - } diff --git a/sonar-batch/src/test/java/org/sonar/batch/debt/DebtModelLoaderTest.java b/sonar-batch/src/test/java/org/sonar/batch/debt/DebtModelLoaderTest.java index faa25f1d1a9..e22c6c978f9 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/debt/DebtModelLoaderTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/debt/DebtModelLoaderTest.java @@ -32,7 +32,6 @@ import org.sonar.api.rules.RuleQuery; import org.sonar.api.technicaldebt.batch.internal.DefaultCharacteristic; import org.sonar.api.technicaldebt.batch.internal.DefaultRequirement; import org.sonar.api.utils.WorkDuration; -import org.sonar.api.utils.WorkUnit; import org.sonar.core.technicaldebt.DefaultTechnicalDebtModel; import org.sonar.core.technicaldebt.db.CharacteristicDao; import org.sonar.core.technicaldebt.db.CharacteristicDto; @@ -77,9 +76,9 @@ public class DebtModelLoaderTest { .setRuleId(100) .setFunction("linear") .setFactorValue(2d) - .setFactorUnit(WorkUnit.DAYS) + .setFactorUnit(CharacteristicDto.DAYS) .setOffsetValue(0d) - .setOffsetUnit(WorkUnit.DEFAULT_UNIT); + .setOffsetUnit(CharacteristicDto.MINUTES); RuleKey ruleKey = RuleKey.of("checkstyle", "Regexp"); Rule rule = Rule.create(ruleKey.repository(), ruleKey.rule()); @@ -112,7 +111,7 @@ public class DebtModelLoaderTest { assertThat(requirement.factorValue()).isEqualTo(2); assertThat(requirement.factorUnit()).isEqualTo(WorkDuration.UNIT.DAYS); assertThat(requirement.offsetValue()).isEqualTo(0); - assertThat(requirement.offsetUnit()).isEqualTo(WorkDuration.UNIT.DAYS); + assertThat(requirement.offsetUnit()).isEqualTo(WorkDuration.UNIT.MINUTES); } } diff --git a/sonar-batch/src/test/java/org/sonar/batch/debt/RuleDebtCalculatorTest.java b/sonar-batch/src/test/java/org/sonar/batch/debt/RuleDebtCalculatorTest.java index 820ea6fe6ad..178108b396b 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/debt/RuleDebtCalculatorTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/debt/RuleDebtCalculatorTest.java @@ -23,7 +23,6 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; import org.sonar.api.CoreProperties; import org.sonar.api.config.Settings; @@ -33,23 +32,19 @@ import org.sonar.api.technicaldebt.batch.TechnicalDebtModel; import org.sonar.api.technicaldebt.batch.internal.DefaultRequirement; import org.sonar.api.utils.WorkDuration; import org.sonar.api.utils.WorkDurationFactory; -import org.sonar.api.utils.WorkUnit; import static org.fest.assertions.Assertions.assertThat; import static org.fest.assertions.Fail.fail; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class RuleDebtCalculatorTest { private static final int HOURS_IN_DAY = 8; + @Mock TechnicalDebtModel model; - WorkUnit tenMinutes = WorkUnit.create(10d, WorkUnit.MINUTES); - WorkUnit fiveMinutes = WorkUnit.create(5d, WorkUnit.MINUTES); - RuleDebtCalculator calculator; @Before @@ -64,10 +59,12 @@ public class RuleDebtCalculatorTest { RuleKey ruleKey = RuleKey.of("squid", "AvoidCycle"); DefaultIssue issue = new DefaultIssue().setKey("ABCDE").setRuleKey(ruleKey); - DefaultRequirement requirement = mock(DefaultRequirement.class); - Mockito.when(requirement.function()).thenReturn("constant_issue"); - Mockito.when(requirement.factor()).thenReturn(tenMinutes); - Mockito.when(requirement.offset()).thenReturn(fiveMinutes); + DefaultRequirement requirement = new DefaultRequirement() + .setFunction("constant_issue") + .setFactorValue(10) + .setFactorUnit(WorkDuration.UNIT.MINUTES) + .setOffsetValue(5) + .setOffsetUnit(WorkDuration.UNIT.MINUTES); when(model.requirementsByRule(ruleKey)).thenReturn(requirement); assertThat(calculator.calculateTechnicalDebt(issue.ruleKey(), issue.effortToFix())).isEqualTo( @@ -79,10 +76,12 @@ public class RuleDebtCalculatorTest { RuleKey ruleKey = RuleKey.of("squid", "AvoidCycle"); DefaultIssue issue = new DefaultIssue().setKey("ABCDE").setRuleKey(ruleKey).setEffortToFix(2d); - DefaultRequirement requirement = mock(DefaultRequirement.class); - Mockito.when(requirement.function()).thenReturn("linear_offset"); - Mockito.when(requirement.factor()).thenReturn(tenMinutes); - Mockito.when(requirement.offset()).thenReturn(fiveMinutes); + DefaultRequirement requirement = new DefaultRequirement() + .setFunction("linear_offset") + .setFactorValue(10) + .setFactorUnit(WorkDuration.UNIT.MINUTES) + .setOffsetValue(5) + .setOffsetUnit(WorkDuration.UNIT.MINUTES); when(model.requirementsByRule(ruleKey)).thenReturn(requirement); assertThat(calculator.calculateTechnicalDebt(issue.ruleKey(), issue.effortToFix())).isEqualTo( @@ -94,10 +93,10 @@ public class RuleDebtCalculatorTest { RuleKey ruleKey = RuleKey.of("squid", "AvoidCycle"); DefaultIssue issue = new DefaultIssue().setKey("ABCDE").setRuleKey(ruleKey).setEffortToFix(2d); - DefaultRequirement requirement = mock(DefaultRequirement.class); - Mockito.when(requirement.function()).thenReturn("linear"); - Mockito.when(requirement.factor()).thenReturn(tenMinutes); - Mockito.when(requirement.offset()).thenReturn(null); + DefaultRequirement requirement = new DefaultRequirement() + .setFunction("linear") + .setFactorValue(10) + .setFactorUnit(WorkDuration.UNIT.MINUTES); when(model.requirementsByRule(ruleKey)).thenReturn(requirement); assertThat(calculator.calculateTechnicalDebt(issue.ruleKey(), issue.effortToFix())).isEqualTo( @@ -109,10 +108,11 @@ public class RuleDebtCalculatorTest { RuleKey ruleKey = RuleKey.of("squid", "AvoidCycle"); DefaultIssue issue = new DefaultIssue().setKey("ABCDE").setRuleKey(ruleKey); - DefaultRequirement requirement = mock(DefaultRequirement.class); - Mockito.when(requirement.function()).thenReturn("constant_issue"); - Mockito.when(requirement.factor()).thenReturn(null); - Mockito.when(requirement.offset()).thenReturn(fiveMinutes); + DefaultRequirement requirement = new DefaultRequirement() + .setFunction("constant_issue") + .setOffsetValue(5) + .setOffsetUnit(WorkDuration.UNIT.MINUTES); + when(model.requirementsByRule(ruleKey)).thenReturn(requirement); assertThat(calculator.calculateTechnicalDebt(issue.ruleKey(), issue.effortToFix())).isEqualTo( @@ -133,10 +133,10 @@ public class RuleDebtCalculatorTest { RuleKey ruleKey = RuleKey.of("squid", "AvoidCycle"); DefaultIssue issue = new DefaultIssue().setKey("ABCDE").setRuleKey(ruleKey).setEffortToFix(2d); - DefaultRequirement requirement = mock(DefaultRequirement.class); - Mockito.when(requirement.function()).thenReturn("constant_issue"); - Mockito.when(requirement.factor()).thenReturn(null); - Mockito.when(requirement.offset()).thenReturn(fiveMinutes); + DefaultRequirement requirement = new DefaultRequirement() + .setFunction("constant_issue") + .setOffsetValue(5) + .setOffsetUnit(WorkDuration.UNIT.MINUTES); when(model.requirementsByRule(ruleKey)).thenReturn(requirement); try { 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 d2c76956d1b..acbb5e56245 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 @@ -254,13 +254,13 @@ public class CharacteristicDto implements Serializable { .setUpdatedAt(requirement.updatedAt()); } - public static WorkDuration.UNIT toUnit(@Nullable String requirementUnit) { + private static WorkDuration.UNIT toUnit(@Nullable String requirementUnit) { if (requirementUnit != null) { - if (requirementUnit.equals(DAYS)) { + if (DAYS.equals(requirementUnit)) { return WorkDuration.UNIT.DAYS; - } else if (requirementUnit.equals(HOURS)) { + } else if (HOURS.equals(requirementUnit)) { return WorkDuration.UNIT.HOURS; - } else if (requirementUnit.equals(MINUTES)) { + } else if (MINUTES.equals(requirementUnit)) { return WorkDuration.UNIT.MINUTES; } throw new IllegalStateException("Invalid unit : " + requirementUnit); @@ -268,7 +268,7 @@ public class CharacteristicDto implements Serializable { return null; } - public static String fromUnit(@Nullable WorkDuration.UNIT unit) { + private static String fromUnit(@Nullable WorkDuration.UNIT unit) { if (unit != null) { if (WorkDuration.UNIT.DAYS.equals(unit)) { return DAYS; diff --git a/sonar-core/src/test/java/org/sonar/core/technicaldebt/db/CharacteristicDtoTest.java b/sonar-core/src/test/java/org/sonar/core/technicaldebt/db/CharacteristicDtoTest.java new file mode 100644 index 00000000000..9544b529b4f --- /dev/null +++ b/sonar-core/src/test/java/org/sonar/core/technicaldebt/db/CharacteristicDtoTest.java @@ -0,0 +1,96 @@ +/* + * 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.db; + +import org.junit.Test; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.technicaldebt.batch.internal.DefaultCharacteristic; +import org.sonar.api.technicaldebt.batch.internal.DefaultRequirement; +import org.sonar.api.utils.WorkDuration; + +import java.util.Date; + +import static org.fest.assertions.Assertions.assertThat; + +public class CharacteristicDtoTest { + + @Test + public void to_dto_from_requirement() throws Exception { + DefaultRequirement requirement = new DefaultRequirement() + .setFunction("constant_issue") + .setFactorValue(10) + .setFactorUnit(WorkDuration.UNIT.DAYS) + .setOffsetValue(5) + .setOffsetUnit(WorkDuration.UNIT.MINUTES) + .setCreatedAt(new Date()) + .setUpdatedAt(new Date()); + + CharacteristicDto dto = CharacteristicDto.toDto(requirement, 2, 1, 10); + assertThat(dto.getRuleId()).isEqualTo(10); + assertThat(dto.getParentId()).isEqualTo(2); + assertThat(dto.getRootId()).isEqualTo(1); + assertThat(dto.getFunction()).isEqualTo("constant_issue"); + assertThat(dto.getFactorValue()).isEqualTo(10d); + assertThat(dto.getFactorUnit()).isEqualTo(CharacteristicDto.DAYS); + assertThat(dto.getOffsetValue()).isEqualTo(5d); + assertThat(dto.getOffsetUnit()).isEqualTo(CharacteristicDto.MINUTES); + assertThat(dto.isEnabled()).isTrue(); + assertThat(dto.getCreatedAt()).isNotNull(); + assertThat(dto.getUpdatedAt()).isNotNull(); + } + + @Test + public void to_requirement() throws Exception { + CharacteristicDto requirementDto = new CharacteristicDto() + .setId(3) + .setParentId(2) + .setRuleId(100) + .setFunction("linear") + .setFactorValue(2d) + .setFactorUnit(CharacteristicDto.DAYS) + .setOffsetValue(0d) + .setOffsetUnit(CharacteristicDto.MINUTES) + .setCreatedAt(new Date()) + .setUpdatedAt(new Date()); + + DefaultCharacteristic rootCharacteristic = new DefaultCharacteristic() + .setKey("MEMORY_EFFICIENCY") + .setName("Memory use"); + + DefaultCharacteristic characteristic = new DefaultCharacteristic() + .setKey("EFFICIENCY") + .setName("Efficiency") + .setParent(rootCharacteristic); + + DefaultRequirement requirement = requirementDto.toRequirement(RuleKey.of("squid", "S106"), characteristic, rootCharacteristic); + assertThat(requirement.ruleKey()).isEqualTo(RuleKey.of("squid", "S106")); + assertThat(requirement.characteristic()).isEqualTo(characteristic); + assertThat(requirement.rootCharacteristic()).isEqualTo(rootCharacteristic); + assertThat(requirement.function()).isEqualTo("linear"); + assertThat(requirement.factorValue()).isEqualTo(2); + assertThat(requirement.factorUnit()).isEqualTo(WorkDuration.UNIT.DAYS); + assertThat(requirement.offsetValue()).isEqualTo(0); + assertThat(requirement.offsetUnit()).isEqualTo(WorkDuration.UNIT.MINUTES); + assertThat(requirement.createdAt()).isNotNull(); + assertThat(requirement.updatedAt()).isNotNull(); + + } +} diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/internal/DefaultCharacteristic.java b/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/internal/DefaultCharacteristic.java index f5a107452b3..5f95d54fff7 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/internal/DefaultCharacteristic.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/internal/DefaultCharacteristic.java @@ -133,7 +133,7 @@ public class DefaultCharacteristic implements Characteristic { @Deprecated @CheckForNull public WorkUnit factor() { - if (factorValue!= null && factorUnit!= null) { + if (factorValue != null && factorUnit != null) { return WorkUnit.create((double) factorValue, fromUnit(factorUnit)); } return null; @@ -176,8 +176,8 @@ public class DefaultCharacteristic implements Characteristic { */ @Deprecated public WorkUnit offset() { - if (offsetValue!= null && offsetUnit!= null) { - return WorkUnit.create((double) offsetValue, fromUnit(offsetUnit)); + if (offsetValue != null && offsetUnit != null) { + return WorkUnit.create((double) offsetValue, fromUnit(offsetUnit)); } return null; } @@ -228,7 +228,7 @@ public class DefaultCharacteristic implements Characteristic { return null; } - private static String fromUnit(WorkDuration.UNIT unit){ + private static String fromUnit(WorkDuration.UNIT unit) { if (WorkDuration.UNIT.DAYS.equals(unit)) { return WorkUnit.DAYS; } else if (WorkDuration.UNIT.HOURS.equals(unit)) { -- 2.39.5