From 735d9fa04b22e072a445404879e648cb1cf431c3 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 10 Sep 2015 16:07:03 +0200 Subject: [PATCH] MeasureDtoToMeasure now always allow rule and characteristic measures --- .../measure/MeasureDtoToMeasure.java | 21 ++---- .../measure/MeasureRepositoryImpl.java | 8 ++- .../measure/MeasureDtoToMeasureTest.java | 20 +----- .../measure/MeasureRepositoryImplTest.java | 67 +++++++++++++------ 4 files changed, 59 insertions(+), 57 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureDtoToMeasure.java b/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureDtoToMeasure.java index 15806f274f4..68a598d4d2a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureDtoToMeasure.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureDtoToMeasure.java @@ -25,26 +25,16 @@ import org.sonar.db.measure.MeasureDto; import org.sonar.server.computation.metric.Metric; import static com.google.common.base.Optional.of; -import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; import static org.sonar.server.computation.measure.Measure.Level.toLevel; public class MeasureDtoToMeasure { public Optional toMeasure(@Nullable MeasureDto measureDto, Metric metric) { - return toMeasure(measureDto, metric, false); - } - - public Optional toMeasure(@Nullable MeasureDto measureDto, Metric metric, boolean acceptRuleAndCharacteristicMeasure){ requireNonNull(metric); if (measureDto == null) { return Optional.absent(); } - if (!acceptRuleAndCharacteristicMeasure) { - checkArgument(measureDto.getCharacteristicId() == null, "Measures with characteristicId are not supported"); - checkArgument(measureDto.getRuleId() == null, "Measures with ruleId are not supported"); - } - Double value = measureDto.getValue(); String data = measureDto.getData(); switch (metric.getType().getValueType()) { @@ -150,12 +140,11 @@ public class MeasureDtoToMeasure { private static MeasureVariations createVariations(MeasureDto measureDto) { return new MeasureVariations( - measureDto.getVariation(1), - measureDto.getVariation(2), - measureDto.getVariation(3), - measureDto.getVariation(4), - measureDto.getVariation(5) - ); + measureDto.getVariation(1), + measureDto.getVariation(2), + measureDto.getVariation(3), + measureDto.getVariation(4), + measureDto.getVariation(5)); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureRepositoryImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureRepositoryImpl.java index 67645b49212..7266b2a4b5f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureRepositoryImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureRepositoryImpl.java @@ -35,6 +35,7 @@ import org.sonar.server.computation.metric.Metric; import org.sonar.server.computation.metric.MetricRepository; import org.sonar.server.computation.metric.ReportMetricValidator; +import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; import static org.sonar.server.computation.component.ComponentFunctions.toReportRef; @@ -65,7 +66,12 @@ public class MeasureRepositoryImpl implements MeasureRepository { try (DbSession dbSession = dbClient.openSession(false)) { MeasureDto measureDto = dbClient.measureDao().selectByComponentKeyAndMetricKey(dbSession, component.getKey(), metric.getKey()); - return underTest.toMeasure(measureDto, metric); + Optional measureOptional = underTest.toMeasure(measureDto, metric); + if (measureOptional.isPresent()) { + checkArgument(measureOptional.get().getCharacteristicId() == null, "Measures with characteristicId are not supported"); + checkArgument(measureOptional.get().getRuleId() == null, "Measures with ruleId are not supported"); + } + return measureOptional; } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/measure/MeasureDtoToMeasureTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/measure/MeasureDtoToMeasureTest.java index db1e9a87792..f5acb7726de 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/measure/MeasureDtoToMeasureTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/measure/MeasureDtoToMeasureTest.java @@ -60,14 +60,14 @@ public class MeasureDtoToMeasureTest { @Test public void toMeasure_returns_rule_measure() { - Optional measure = underTest.toMeasure(new MeasureDto().setRuleId(10), SOME_INT_METRIC, true); + Optional measure = underTest.toMeasure(new MeasureDto().setRuleId(10), SOME_INT_METRIC); assertThat(measure).isPresent(); assertThat(measure.get().getRuleId()).isEqualTo(10); } @Test public void toMeasure_returns_characteristic_measure() { - Optional measure = underTest.toMeasure(new MeasureDto().setCharacteristicId(30), SOME_INT_METRIC, true); + Optional measure = underTest.toMeasure(new MeasureDto().setCharacteristicId(30), SOME_INT_METRIC); assertThat(measure).isPresent(); assertThat(measure.get().getCharacteristicId()).isEqualTo(30); } @@ -82,22 +82,6 @@ public class MeasureDtoToMeasureTest { underTest.toMeasure(null, null); } - @Test - public void toMeasure_throws_IAE_if_MeasureDto_has_non_null_ruleId() { - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Measures with ruleId are not supported"); - - underTest.toMeasure(new MeasureDto().setRuleId(12), SOME_STRING_METRIC); - } - - @Test - public void toMeasure_throws_IAE_if_MeasureDto_has_non_null_characteristicId() { - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Measures with characteristicId are not supported"); - - underTest.toMeasure(new MeasureDto().setCharacteristicId(12), SOME_STRING_METRIC); - } - @Test public void toMeasure_returns_no_value_if_dto_has_no_data_for_Level_Metric() { Optional measure = underTest.toMeasure(EMPTY_MEASURE_DTO, SOME_LEVEL_METRIC); diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/measure/MeasureRepositoryImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/measure/MeasureRepositoryImplTest.java index 715bbe1174b..d75b8a93496 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/measure/MeasureRepositoryImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/measure/MeasureRepositoryImplTest.java @@ -29,12 +29,11 @@ import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; import java.util.List; import java.util.Set; -import javax.annotation.CheckForNull; import javax.annotation.Nullable; -import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.sonar.api.rule.RuleKey; import org.sonar.api.utils.System2; @@ -67,6 +66,10 @@ import static org.mockito.Mockito.when; @RunWith(DataProviderRunner.class) public class MeasureRepositoryImplTest { + + @Rule + public final ExpectedException expectedException = ExpectedException.none(); + @Rule public DbTester dbTester = DbTester.create(System2.INSTANCE); @Rule @@ -99,8 +102,7 @@ public class MeasureRepositoryImplTest { private BatchReportReader mockBatchReportReader = mock(BatchReportReader.class); private MeasureRepositoryImpl underTestWithMock = new MeasureRepositoryImpl(mockedDbClient, mockBatchReportReader, metricRepository, reportMetricValidator); - @CheckForNull - private DbSession dbSession; + private DbSession dbSession = dbTester.getSession(); @Before public void setUp() { @@ -114,13 +116,6 @@ public class MeasureRepositoryImplTest { when(metricRepository.getByKey(METRIC_KEY_2)).thenReturn(metric2); } - @After - public void tearDown() { - if (dbSession != null) { - dbSession.close(); - } - } - @Test public void getBaseMeasure_throws_NPE_and_does_not_open_session_if_component_is_null() { try { @@ -151,7 +146,6 @@ public class MeasureRepositoryImplTest { @Test public void getBaseMeasure_returns_Measure_if_measure_of_last_snapshot_only_in_DB() { dbTester.prepareDbUnit(getClass(), "shared.xml"); - dbSession = dbClient.openSession(false); dbClient.measureDao().insert(dbSession, createMeasureDto(METRIC_ID_1, LAST_SNAPSHOT_ID)); dbClient.measureDao().insert(dbSession, createMeasureDto(METRIC_ID_2, OTHER_SNAPSHOT_ID)); dbSession.commit(); @@ -168,44 +162,71 @@ public class MeasureRepositoryImplTest { assertThat(res).isAbsent(); } - @Test(expected = NullPointerException.class) + @Test + public void getBaseMeasure_does_not_return_measure_with_rule() { + dbTester.prepareDbUnit(getClass(), "shared.xml"); + dbClient.measureDao().insert(dbSession, createMeasureDto(METRIC_ID_1, LAST_SNAPSHOT_ID).setRuleId(10)); + dbSession.commit(); + + assertThat(underTest.getBaseMeasure(FILE_COMPONENT, metric1)).isAbsent(); + } + + @Test + public void getBaseMeasure_does_not_return_measure_with_characteristic() { + dbTester.prepareDbUnit(getClass(), "shared.xml"); + dbClient.measureDao().insert(dbSession, createMeasureDto(METRIC_ID_1, LAST_SNAPSHOT_ID).setCharacteristicId(100)); + dbSession.commit(); + + assertThat(underTest.getBaseMeasure(FILE_COMPONENT, metric1)).isAbsent(); + } + + @Test public void add_throws_NPE_if_Component_argument_is_null() { + expectedException.expect(NullPointerException.class); underTest.add(null, metric1, SOME_MEASURE); } - @Test(expected = NullPointerException.class) + @Test public void add_throws_NPE_if_Component_metric_is_null() { + expectedException.expect(NullPointerException.class); underTest.add(FILE_COMPONENT, null, SOME_MEASURE); } - @Test(expected = NullPointerException.class) + @Test public void add_throws_NPE_if_Component_measure_is_null() { + expectedException.expect(NullPointerException.class); underTest.add(FILE_COMPONENT, metric1, null); } - @Test(expected = UnsupportedOperationException.class) + @Test public void add_throws_UOE_if_measure_already_exists() { + expectedException.expect(UnsupportedOperationException.class); underTest.add(FILE_COMPONENT, metric1, SOME_MEASURE); underTest.add(FILE_COMPONENT, metric1, SOME_MEASURE); } - @Test(expected = NullPointerException.class) + @Test public void update_throws_NPE_if_Component_argument_is_null() { + expectedException.expect(NullPointerException.class); underTest.update(null, metric1, SOME_MEASURE); } - @Test(expected = NullPointerException.class) + @Test public void update_throws_NPE_if_Component_metric_is_null() { + expectedException.expect(NullPointerException.class); underTest.update(FILE_COMPONENT, null, SOME_MEASURE); } - @Test(expected = NullPointerException.class) + @Test public void update_throws_NPE_if_Component_measure_is_null() { + expectedException.expect(NullPointerException.class); + expectedException.expect(NullPointerException.class); underTest.update(FILE_COMPONENT, metric1, null); } - @Test(expected = UnsupportedOperationException.class) + @Test public void update_throws_UOE_if_measure_does_not_exists() { + expectedException.expect(UnsupportedOperationException.class); underTest.update(FILE_COMPONENT, metric1, SOME_MEASURE); } @@ -411,13 +432,15 @@ public class MeasureRepositoryImplTest { underTest.update(FILE_COMPONENT, metric1, Measure.updatedMeasureBuilder(measure.get()).create()); } - @Test(expected = NullPointerException.class) + @Test public void getRawMeasures_for_metric_throws_NPE_if_Component_arg_is_null() { + expectedException.expect(NullPointerException.class); underTest.getRawMeasures(null, metric1); } - @Test(expected = NullPointerException.class) + @Test public void getRawMeasures_for_metric_throws_NPE_if_Metric_arg_is_null() { + expectedException.expect(NullPointerException.class); underTest.getRawMeasures(FILE_COMPONENT, null); } -- 2.39.5