From 84c39f50d53f7f4a76fcda6c58b6120875501e40 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Mon, 7 Sep 2015 22:08:45 +0200 Subject: [PATCH] SONAR-6644 fix debt when characteristic is none --- .../server/computation/issue/RuleImpl.java | 17 +----- .../sonar/server/debt/DebtModelBackup.java | 5 +- .../org/sonar/server/rule/RuleOperations.java | 3 +- .../main/java/org/sonar/db/rule/RuleDto.java | 19 +++--- .../java/org/sonar/db/rule/RuleDtoTest.java | 60 +++++++++++++++++++ 5 files changed, 77 insertions(+), 27 deletions(-) create mode 100644 sonar-db/src/test/java/org/sonar/db/rule/RuleDtoTest.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/RuleImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/RuleImpl.java index a30d9cd9d72..483ed15c924 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/RuleImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/RuleImpl.java @@ -48,7 +48,7 @@ public class RuleImpl implements Rule { this.key = dto.getKey(); this.name = dto.getName(); this.status = dto.getStatus(); - this.subCharacteristicId = effectiveCharacteristicId(dto); + this.subCharacteristicId = dto.getEffectiveSubCharacteristicId(); this.tags = union(dto.getSystemTags(), dto.getTags()); this.remediationFunction = effectiveRemediationFunction(dto); } @@ -117,21 +117,6 @@ public class RuleImpl implements Rule { .toString(); } - @CheckForNull - private static Integer effectiveCharacteristicId(RuleDto dto) { - if (isEnabledCharacteristicId(dto.getSubCharacteristicId())) { - return dto.getSubCharacteristicId(); - } - if (isEnabledCharacteristicId(dto.getDefaultSubCharacteristicId())) { - return dto.getDefaultSubCharacteristicId(); - } - return null; - } - - private static boolean isEnabledCharacteristicId(@Nullable Integer id) { - return (id != null) && (id.intValue() != RuleDto.DISABLED_CHARACTERISTIC_ID); - } - @CheckForNull private static DebtRemediationFunction effectiveRemediationFunction(RuleDto dto) { String fn = dto.getRemediationFunction(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/debt/DebtModelBackup.java b/server/sonar-server/src/main/java/org/sonar/server/debt/DebtModelBackup.java index a16ea010fba..f3ab358f804 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/debt/DebtModelBackup.java +++ b/server/sonar-server/src/main/java/org/sonar/server/debt/DebtModelBackup.java @@ -346,9 +346,8 @@ public class DebtModelBackup { @CheckForNull private static RuleDebt toRuleDebt(RuleDto rule, DebtModel debtModel) { RuleDebt ruleDebt = new RuleDebt().setRuleKey(RuleKey.of(rule.getRepositoryKey(), rule.getRuleKey())); - Integer effectiveSubCharacteristicId = rule.getSubCharacteristicId() != null ? rule.getSubCharacteristicId() : rule.getDefaultSubCharacteristicId(); - DebtCharacteristic subCharacteristic = (effectiveSubCharacteristicId != null && !RuleDto.DISABLED_CHARACTERISTIC_ID.equals(effectiveSubCharacteristicId)) ? - debtModel.characteristicById(effectiveSubCharacteristicId) : null; + Integer effectiveSubCharacteristicId = rule.getEffectiveSubCharacteristicId(); + DebtCharacteristic subCharacteristic = effectiveSubCharacteristicId != null ? debtModel.characteristicById(effectiveSubCharacteristicId) : null; if (subCharacteristic != null) { ruleDebt.setSubCharacteristicKey(subCharacteristic.key()); diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/RuleOperations.java b/server/sonar-server/src/main/java/org/sonar/server/rule/RuleOperations.java index 8bbc7521acb..3172f6c9b18 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/RuleOperations.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/RuleOperations.java @@ -111,7 +111,8 @@ public class RuleOperations { // No sub-characteristic is given -> disable rule debt if not already disabled } else { // Rule characteristic is not already disabled -> update it - if (ruleDto.getSubCharacteristicId() == null || !RuleDto.DISABLED_CHARACTERISTIC_ID.equals(ruleDto.getSubCharacteristicId())) { + Integer subCharacteristicId = ruleDto.getSubCharacteristicId(); + if (subCharacteristicId == null || subCharacteristicId != RuleDto.DISABLED_CHARACTERISTIC_ID) { // If default characteristic is not defined, set the overridden characteristic to null in order to be able to track debt plugin // update ruleDto.setSubCharacteristicId(ruleDto.getDefaultSubCharacteristicId() != null ? RuleDto.DISABLED_CHARACTERISTIC_ID : null); diff --git a/sonar-db/src/main/java/org/sonar/db/rule/RuleDto.java b/sonar-db/src/main/java/org/sonar/db/rule/RuleDto.java index d3188ec87db..2c5a17b4f59 100644 --- a/sonar-db/src/main/java/org/sonar/db/rule/RuleDto.java +++ b/sonar-db/src/main/java/org/sonar/db/rule/RuleDto.java @@ -38,7 +38,7 @@ import org.sonar.db.Dto; public class RuleDto extends Dto { - public static final Integer DISABLED_CHARACTERISTIC_ID = -1; + public static final int DISABLED_CHARACTERISTIC_ID = -1; public enum Format { HTML, MARKDOWN @@ -258,6 +258,15 @@ public class RuleDto extends Dto { return this; } + @CheckForNull + public Integer getEffectiveSubCharacteristicId() { + Integer effective = subCharacteristicId == null ? defaultSubCharacteristicId : subCharacteristicId; + if (effective != null && effective != DISABLED_CHARACTERISTIC_ID) { + return effective; + } + return null; + } + @CheckForNull public String getRemediationFunction() { return remediationFunction; @@ -329,15 +338,11 @@ public class RuleDto extends Dto { } public Set getTags() { - return tags == null ? - new HashSet() : - new TreeSet<>(Arrays.asList(StringUtils.split(tags, ','))); + return tags == null ? new HashSet() : new TreeSet<>(Arrays.asList(StringUtils.split(tags, ','))); } public Set getSystemTags() { - return systemTags == null ? - new HashSet() : - new TreeSet<>(Arrays.asList(StringUtils.split(systemTags, ','))); + return systemTags == null ? new HashSet() : new TreeSet<>(Arrays.asList(StringUtils.split(systemTags, ','))); } private String getTagsField() { diff --git a/sonar-db/src/test/java/org/sonar/db/rule/RuleDtoTest.java b/sonar-db/src/test/java/org/sonar/db/rule/RuleDtoTest.java new file mode 100644 index 00000000000..7c170e7c8df --- /dev/null +++ b/sonar-db/src/test/java/org/sonar/db/rule/RuleDtoTest.java @@ -0,0 +1,60 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 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.db.rule; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.db.rule.RuleDto.DISABLED_CHARACTERISTIC_ID; + +public class RuleDtoTest { + + public static final int FAKE_SUB_CHAR_1 = 27; + public static final int FAKE_SUB_CHAR_2 = 42; + + @Test + public void effective_sub_characteristic_id() { + RuleDto dto = new RuleDto(); + + // characteristic is not set + dto.setSubCharacteristicId(null).setDefaultSubCharacteristicId(null); + assertThat(dto.getEffectiveSubCharacteristicId()).isNull(); + + // default characteristic is set + dto.setSubCharacteristicId(null).setDefaultSubCharacteristicId(FAKE_SUB_CHAR_2); + assertThat(dto.getEffectiveSubCharacteristicId()).isEqualTo(FAKE_SUB_CHAR_2); + + // default characteristic is set to "none" + dto.setSubCharacteristicId(null).setDefaultSubCharacteristicId(DISABLED_CHARACTERISTIC_ID); + assertThat(dto.getEffectiveSubCharacteristicId()).isNull(); + + // characteristic is overridden + dto.setSubCharacteristicId(FAKE_SUB_CHAR_1).setDefaultSubCharacteristicId(FAKE_SUB_CHAR_2); + assertThat(dto.getEffectiveSubCharacteristicId()).isEqualTo(FAKE_SUB_CHAR_1); + + // characteristic is set, no defaults + dto.setSubCharacteristicId(FAKE_SUB_CHAR_1).setDefaultSubCharacteristicId(null); + assertThat(dto.getEffectiveSubCharacteristicId()).isEqualTo(FAKE_SUB_CHAR_1); + + // characteristic is set to "none" + dto.setSubCharacteristicId(DISABLED_CHARACTERISTIC_ID).setDefaultSubCharacteristicId(FAKE_SUB_CHAR_2); + assertThat(dto.getEffectiveSubCharacteristicId()).isNull(); + } +} -- 2.39.5