From 93f85261cd74c1bc50dc6da02106fabc606f7322 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Thu, 21 Sep 2017 15:06:49 +0200 Subject: [PATCH] support condition on deleted metric when resolving QG in CE --- .../metric/MetricRepository.java | 7 ++++ .../metric/MetricRepositoryImpl.java | 13 ++++--- .../qualitygate/QualityGateServiceImpl.java | 10 +++--- .../metric/MetricRepositoryImplTest.java | 34 +++++++++++++++++++ .../metric/MetricRepositoryRule.java | 6 ++++ .../QualityGateServiceImplTest.java | 21 ++++++++++-- 6 files changed, 79 insertions(+), 12 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepository.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepository.java index be719973f8b..551ccb2db1d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepository.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepository.java @@ -19,6 +19,8 @@ */ package org.sonar.server.computation.task.projectanalysis.metric; +import java.util.Optional; + public interface MetricRepository { /** @@ -39,6 +41,11 @@ public interface MetricRepository { */ Metric getById(long id); + /** + * Gets the {@link Metric} with the specific id if it exists in the repository. + */ + Optional getOptionalById(long id); + /** * Get iterable of all {@link Metric}. */ diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepositoryImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepositoryImpl.java index 16a30e5d312..ddba6c1e8e4 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepositoryImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepositoryImpl.java @@ -23,6 +23,7 @@ import com.google.common.base.Function; import com.google.common.collect.FluentIterable; import java.util.List; import java.util.Map; +import java.util.Optional; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import org.picocontainer.Startable; @@ -76,13 +77,15 @@ public class MetricRepositoryImpl implements MetricRepository, Startable { @Override public Metric getById(long id) { + return getOptionalById(id) + .orElseThrow(() -> new IllegalStateException(String.format("Metric with id '%s' does not exist", id))); + } + + @Override + public Optional getOptionalById(long id) { verifyMetricsInitialized(); - Metric res = this.metricsById.get(id); - if (res == null) { - throw new IllegalStateException(String.format("Metric with id '%s' does not exist", id)); - } - return res; + return Optional.ofNullable(this.metricsById.get(id)); } @Override diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/qualitygate/QualityGateServiceImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/qualitygate/QualityGateServiceImpl.java index 2ad4bb9fb39..9b73a508859 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/qualitygate/QualityGateServiceImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/qualitygate/QualityGateServiceImpl.java @@ -21,12 +21,12 @@ package org.sonar.server.computation.task.projectanalysis.qualitygate; import com.google.common.base.Optional; import java.util.Collection; +import java.util.Objects; import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.qualitygate.QualityGateConditionDto; import org.sonar.db.qualitygate.QualityGateDto; -import org.sonar.server.computation.task.projectanalysis.metric.Metric; import org.sonar.server.computation.task.projectanalysis.metric.MetricRepository; public class QualityGateServiceImpl implements QualityGateService { @@ -54,10 +54,10 @@ public class QualityGateServiceImpl implements QualityGateService { Collection dtos = dbClient.gateConditionDao().selectForQualityGate(dbSession, qualityGateDto.getId()); Iterable conditions = dtos.stream() - .map(input -> { - Metric metric = metricRepository.getById(input.getMetricId()); - return new Condition(metric, input.getOperator(), input.getErrorThreshold(), input.getWarningThreshold(), input.getPeriod() != null); - }) + .map(input -> metricRepository.getOptionalById(input.getMetricId()) + .map(metric -> new Condition(metric, input.getOperator(), input.getErrorThreshold(), input.getWarningThreshold(), input.getPeriod() != null)) + .orElse(null)) + .filter(Objects::nonNull) .collect(MoreCollectors.toList(dtos.size())); return new QualityGate(qualityGateDto.getId(), qualityGateDto.getName(), conditions); diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepositoryImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepositoryImplTest.java index 5d4822cd51c..8b95dd70145 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepositoryImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepositoryImplTest.java @@ -120,6 +120,40 @@ public class MetricRepositoryImplTest { assertThat(underTest.getById(2).getKey()).isEqualTo("coverage"); } + @Test + public void getOptionalById_throws_ISE_if_start_has_not_been_called() { + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("Metric cache has not been initialized"); + + underTest.getOptionalById(SOME_ID); + } + + @Test + public void getOptionalById_returns_empty_of_Metric_does_not_exist() { + underTest.start(); + + assertThat(underTest.getOptionalById(SOME_ID)).isEmpty(); + } + + @Test + public void getOptionalById_returns_empty_of_Metric_is_disabled() { + dbTester.prepareDbUnit(getClass(), "shared.xml"); + + underTest.start(); + + assertThat(underTest.getOptionalById(100)).isEmpty(); + } + + @Test + public void getOptionalById_find_enabled_Metrics() { + dbTester.prepareDbUnit(getClass(), "shared.xml"); + + underTest.start(); + + assertThat(underTest.getOptionalById(1).get().getKey()).isEqualTo("ncloc"); + assertThat(underTest.getOptionalById(2).get().getKey()).isEqualTo("coverage"); + } + @Test public void get_all_metrics() { dbTester.prepareDbUnit(getClass(), "shared.xml"); diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepositoryRule.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepositoryRule.java index 813e25abe3f..a7ae455f3f8 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepositoryRule.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/metric/MetricRepositoryRule.java @@ -21,6 +21,7 @@ package org.sonar.server.computation.task.projectanalysis.metric; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.junit.rules.ExternalResource; import static com.google.common.base.Preconditions.checkState; @@ -103,6 +104,11 @@ public class MetricRepositoryRule extends ExternalResource implements MetricRepo return res; } + @Override + public Optional getOptionalById(long id) { + return Optional.of(metricsById.get(id)); + } + @Override public Iterable getAll() { return metricsByKey.values(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/qualitygate/QualityGateServiceImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/qualitygate/QualityGateServiceImplTest.java index 60cd97e5646..c41dce6e17e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/qualitygate/QualityGateServiceImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/qualitygate/QualityGateServiceImplTest.java @@ -87,8 +87,8 @@ public class QualityGateServiceImplTest { when(qualityGateDao.selectById(any(DbSession.class), eq(SOME_ID))).thenReturn(QUALITY_GATE_DTO); when(qualityGateConditionDao.selectForQualityGate(any(DbSession.class), eq(SOME_ID))).thenReturn(ImmutableList.of(CONDITION_1, CONDITION_2)); // metrics are always supposed to be there - when(metricRepository.getById(METRIC_ID_1)).thenReturn(METRIC_1); - when(metricRepository.getById(METRIC_ID_2)).thenReturn(METRIC_2); + when(metricRepository.getOptionalById(METRIC_ID_1)).thenReturn(java.util.Optional.of(METRIC_1)); + when(metricRepository.getOptionalById(METRIC_ID_2)).thenReturn(java.util.Optional.of(METRIC_2)); Optional res = underTest.findById(SOME_ID); @@ -99,4 +99,21 @@ public class QualityGateServiceImplTest { new Condition(METRIC_1, CONDITION_1.getOperator(), CONDITION_1.getErrorThreshold(), CONDITION_1.getWarningThreshold(), true), new Condition(METRIC_2, CONDITION_2.getOperator(), CONDITION_2.getErrorThreshold(), CONDITION_2.getWarningThreshold(), false)); } + + @Test + public void findById_ignores_conditions_on_missing_metrics() { + when(qualityGateDao.selectById(any(DbSession.class), eq(SOME_ID))).thenReturn(QUALITY_GATE_DTO); + when(qualityGateConditionDao.selectForQualityGate(any(DbSession.class), eq(SOME_ID))).thenReturn(ImmutableList.of(CONDITION_1, CONDITION_2)); + // metrics are always supposed to be there + when(metricRepository.getOptionalById(METRIC_ID_1)).thenReturn(java.util.Optional.empty()); + when(metricRepository.getOptionalById(METRIC_ID_2)).thenReturn(java.util.Optional.of(METRIC_2)); + + Optional res = underTest.findById(SOME_ID); + + assertThat(res).isPresent(); + assertThat(res.get().getId()).isEqualTo(SOME_ID); + assertThat(res.get().getName()).isEqualTo(SOME_NAME); + assertThat(res.get().getConditions()).containsOnly( + new Condition(METRIC_2, CONDITION_2.getOperator(), CONDITION_2.getErrorThreshold(), CONDITION_2.getWarningThreshold(), false)); + } } -- 2.39.5