From 55e60bba3b96421c1a2071618343e8211a47b0b7 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 25 May 2015 14:22:56 +0200 Subject: [PATCH] Fix quality flaws --- .../sonar/server/component/ws/AppAction.java | 37 ++++++++------- .../step/PersistComponentsStep.java | 9 ++-- .../server/debt/DebtModelOperations.java | 4 +- .../org/sonar/server/issue/ws/ShowAction.java | 11 +++-- .../server/measure/MeasureFilterSort.java | 2 +- .../ComputeComponentsRefCacheTest.java | 44 ++++++++++++++++++ .../component/DbComponentsRefCacheTest.java | 45 +++++++++++++++++++ .../org/sonar/batch/debt/DebtDecorator.java | 2 +- .../java/org/sonar/api/utils/Durations.java | 10 ++--- 9 files changed, 133 insertions(+), 31 deletions(-) create mode 100644 server/sonar-server/src/test/java/org/sonar/server/computation/component/ComputeComponentsRefCacheTest.java create mode 100644 server/sonar-server/src/test/java/org/sonar/server/computation/component/DbComponentsRefCacheTest.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/component/ws/AppAction.java b/server/sonar-server/src/main/java/org/sonar/server/component/ws/AppAction.java index 6a09d72477e..1609cfd1282 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/component/ws/AppAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/component/ws/AppAction.java @@ -43,6 +43,7 @@ import org.sonar.server.user.UserSession; import javax.annotation.CheckForNull; import javax.annotation.Nullable; + import java.util.List; import java.util.Map; @@ -205,26 +206,32 @@ public class AppAction implements RequestHandler { Metric metric = CoreMetrics.getMetric(measure.getMetricKey()); Metric.ValueType metricType = metric.getType(); - Double value = measure.getValue(); + Double value = getDoubleValue(measure, metric); String data = measure.getData(); - if (BooleanUtils.isTrue(metric.isOptimizedBestValue()) && value == null) { - value = metric.getBestValue(); - } - if (metricType.equals(Metric.ValueType.FLOAT) && value != null) { - return i18n.formatDouble(userSession.locale(), value); - } - if (metricType.equals(Metric.ValueType.INT) && value != null) { - return i18n.formatInteger(userSession.locale(), value.intValue()); - } - if (metricType.equals(Metric.ValueType.PERCENT) && value != null) { - return i18n.formatDouble(userSession.locale(), value) + "%"; - } - if (metricType.equals(Metric.ValueType.WORK_DUR) && value != null) { - return durations.format(userSession.locale(), durations.create(value.longValue()), Durations.DurationFormat.SHORT); + if (value != null) { + switch (metricType) { + case FLOAT: + return i18n.formatDouble(userSession.locale(), value); + case INT: + return i18n.formatInteger(userSession.locale(), value.intValue()); + case PERCENT: + return i18n.formatDouble(userSession.locale(), value) + "%"; + case WORK_DUR: + return durations.format(userSession.locale(), durations.create(value.longValue()), Durations.DurationFormat.SHORT); + } } if ((metricType.equals(Metric.ValueType.STRING) || metricType.equals(Metric.ValueType.RATING)) && data != null) { return data; } return null; } + + @CheckForNull + private static Double getDoubleValue(MeasureDto measure, Metric metric){ + Double value = measure.getValue(); + if (BooleanUtils.isTrue(metric.isOptimizedBestValue()) && value == null) { + value = metric.getBestValue(); + } + return value; + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistComponentsStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistComponentsStep.java index 47e3c3125a4..a6aeba4b13b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistComponentsStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistComponentsStep.java @@ -134,7 +134,7 @@ public class PersistComponentsStep implements ComputationStep { component.setProjectUuid(parentModule.projectUuid()); component.setModuleUuid(parentModule.uuid()); component.setModuleUuidPath(reportComponent.getType().equals(Constants.ComponentType.MODULE) ? - parentModule.moduleUuidPath() + component.uuid() + ComponentDto.MODULE_UUID_PATH_SEP : + (parentModule.moduleUuidPath() + component.uuid() + ComponentDto.MODULE_UUID_PATH_SEP) : parentModule.moduleUuidPath()); } else { component.setProjectUuid(uuid); @@ -179,7 +179,6 @@ public class PersistComponentsStep implements ComputationStep { private static String getScope(BatchReport.Component reportComponent) { switch (reportComponent.getType()) { case PROJECT: - return Scopes.PROJECT; case MODULE: return Scopes.PROJECT; case DIRECTORY: @@ -200,12 +199,16 @@ public class PersistComponentsStep implements ComputationStep { case DIRECTORY: return Qualifiers.DIRECTORY; case FILE: - return !reportComponent.getIsTest() ? Qualifiers.FILE : Qualifiers.UNIT_TEST_FILE; + return getFileQualifier(reportComponent); default : throw new IllegalArgumentException(String.format("Unknown type '%s'", reportComponent.getType())); } } + private static String getFileQualifier(BatchReport.Component reportComponent){ + return !reportComponent.getIsTest() ? Qualifiers.FILE : Qualifiers.UNIT_TEST_FILE; + } + private static String getFileName(BatchReport.Component reportComponent) { String path = reportComponent.getPath(); if (reportComponent.getType() == Constants.ComponentType.PROJECT || reportComponent.getType() == Constants.ComponentType.MODULE) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java b/server/sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java index fcbb6a6f337..b9d79cf1ebd 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java +++ b/server/sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java @@ -169,8 +169,8 @@ public class DebtModelOperations { } }); Integer nextPosition = moveUpOrDown ? - (currentPosition > 0 ? currentPosition - 1 : null) : - ((currentPosition < rootCharacteristics.size() - 1) ? currentPosition + 1 : null); + (currentPosition > 0 ? (currentPosition - 1) : null) : + ((currentPosition < rootCharacteristics.size() - 1) ? (currentPosition + 1) : null); return (nextPosition != null) ? Iterables.get(rootCharacteristics, nextPosition) : null; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/ShowAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/ShowAction.java index bafc9544aea..b2d64407d14 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/ShowAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/ShowAction.java @@ -53,6 +53,7 @@ import org.sonar.server.user.UserSession; import javax.annotation.CheckForNull; import javax.annotation.Nullable; + import java.util.Date; import java.util.List; import java.util.Map; @@ -136,6 +137,8 @@ public class ShowAction implements IssuesWsAction { ActionPlan actionPlan = actionPlanKey == null ? null : actionPlanService.findByKey(actionPlanKey, userSession); Duration debt = issue.debt(); Rule rule = ruleService.getNonNullByKey(issue.ruleKey()); + String actionPlanName = actionPlan != null ? actionPlan.name() : null; + String debtValue = debt != null ? durations.encode(debt) : null; Date updateDate = issue.updateDate(); Date closeDate = issue.closeDate(); @@ -150,14 +153,14 @@ public class ShowAction implements IssuesWsAction { .prop("severity", issue.severity()) .prop("author", issue.authorLogin()) .prop("actionPlan", actionPlanKey) - .prop("actionPlanName", actionPlan != null ? actionPlan.name() : null) - .prop("debt", debt != null ? durations.encode(debt) : null) + .prop("actionPlanName", actionPlanName) + .prop("debt", debtValue) .prop("creationDate", DateUtils.formatDateTime(issue.creationDate())) .prop("fCreationDate", formatDate(issue.creationDate())) - .prop("updateDate", updateDate != null ? DateUtils.formatDateTime(updateDate) : null) + .prop("updateDate", DateUtils.formatDateTimeNullSafe(updateDate)) .prop("fUpdateDate", formatDate(updateDate)) .prop("fUpdateAge", formatAgeDate(updateDate)) - .prop("closeDate", closeDate != null ? DateUtils.formatDateTime(closeDate) : null) + .prop("closeDate", DateUtils.formatDateTimeNullSafe(closeDate)) .prop("fCloseDate", formatDate(issue.closeDate())); addComponents(session, issue, json); diff --git a/server/sonar-server/src/main/java/org/sonar/server/measure/MeasureFilterSort.java b/server/sonar-server/src/main/java/org/sonar/server/measure/MeasureFilterSort.java index 7f64ef28d06..5622f2e5b7a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/measure/MeasureFilterSort.java +++ b/server/sonar-server/src/main/java/org/sonar/server/measure/MeasureFilterSort.java @@ -124,7 +124,7 @@ class MeasureFilterSort { private String getMetricColumn() { if (metric.isNumericType()) { - return period != null ? "pmsort.variation_value_" + period : "pmsort.value"; + return period != null ? ("pmsort.variation_value_" + period) : "pmsort.value"; } else { return "pmsort.text_value"; } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/component/ComputeComponentsRefCacheTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/component/ComputeComponentsRefCacheTest.java new file mode 100644 index 00000000000..92e6ff517a9 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/component/ComputeComponentsRefCacheTest.java @@ -0,0 +1,44 @@ +/* + * 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.server.computation.component; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class ComputeComponentsRefCacheTest { + + @Test + public void add_and_get_component() throws Exception { + ComputeComponentsRefCache cache = new ComputeComponentsRefCache(); + cache.addComponent(1, new ComputeComponentsRefCache.ComputeComponent("Key", "Uuid")); + + assertThat(cache.getByRef(1)).isNotNull(); + assertThat(cache.getByRef(1).getKey()).isEqualTo("Key"); + assertThat(cache.getByRef(1).getUuid()).isEqualTo("Uuid"); + } + + @Test(expected = IllegalArgumentException.class) + public void fail_on_unknown_ref() throws Exception { + new ComputeComponentsRefCache().getByRef(1); + } + +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/component/DbComponentsRefCacheTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/component/DbComponentsRefCacheTest.java new file mode 100644 index 00000000000..67c20dfa9b2 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/component/DbComponentsRefCacheTest.java @@ -0,0 +1,45 @@ +/* + * 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.server.computation.component; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class DbComponentsRefCacheTest { + + @Test + public void add_and_get_component() throws Exception { + DbComponentsRefCache cache = new DbComponentsRefCache(); + cache.addComponent(1, new DbComponentsRefCache.DbComponent(10L, "Key", "Uuid")); + + assertThat(cache.getByRef(1)).isNotNull(); + assertThat(cache.getByRef(1).getId()).isEqualTo(10L); + assertThat(cache.getByRef(1).getKey()).isEqualTo("Key"); + assertThat(cache.getByRef(1).getUuid()).isEqualTo("Uuid"); + } + + @Test(expected = IllegalArgumentException.class) + public void fail_on_unknown_ref() throws Exception { + new DbComponentsRefCache().getByRef(1); + } + +} diff --git a/sonar-batch/src/main/java/org/sonar/batch/debt/DebtDecorator.java b/sonar-batch/src/main/java/org/sonar/batch/debt/DebtDecorator.java index 1c4bf3e019a..c2f4c97b9a6 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/debt/DebtDecorator.java +++ b/sonar-batch/src/main/java/org/sonar/batch/debt/DebtDecorator.java @@ -204,7 +204,7 @@ public final class DebtDecorator implements Decorator { public void add(@Nullable E key, Long value) { if (key != null) { Long currentValue = sumByKeys.get(key); - sumByKeys.put(key, currentValue != null ? currentValue + value : value); + sumByKeys.put(key, currentValue != null ? (currentValue + value) : value); } } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/utils/Durations.java b/sonar-plugin-api/src/main/java/org/sonar/api/utils/Durations.java index 9b2dd159f3d..6f0eccafb45 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/utils/Durations.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/utils/Durations.java @@ -20,11 +20,11 @@ package org.sonar.api.utils; -import org.sonar.api.batch.BatchSide; import org.sonar.api.CoreProperties; -import org.sonar.api.server.ServerSide; +import org.sonar.api.batch.BatchSide; import org.sonar.api.config.Settings; import org.sonar.api.i18n.I18n; +import org.sonar.api.server.ServerSide; import javax.annotation.CheckForNull; @@ -116,15 +116,15 @@ public class Durations { private String format(Locale locale, int days, int hours, int minutes, boolean isNegative) { StringBuilder message = new StringBuilder(); if (days > 0) { - message.append(message(locale, "work_duration.x_days", isNegative ? -1 * days : days)); + message.append(message(locale, "work_duration.x_days", isNegative ? (-1 * days) : days)); } if (displayHours(days, hours)) { addSpaceIfNeeded(message); - message.append(message(locale, "work_duration.x_hours", isNegative && message.length() == 0 ? -1 * hours : hours)); + message.append(message(locale, "work_duration.x_hours", isNegative && message.length() == 0 ? (-1 * hours) : hours)); } if (displayMinutes(days, hours, minutes)) { addSpaceIfNeeded(message); - message.append(message(locale, "work_duration.x_minutes", isNegative && message.length() == 0 ? -1 * minutes : minutes)); + message.append(message(locale, "work_duration.x_minutes", isNegative && message.length() == 0 ? (-1 * minutes) : minutes)); } return message.toString(); } -- 2.39.5