MeasureDtoToMeasure now always allow rule and characteristic measures

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<Measure> toMeasure(@Nullable MeasureDto measureDto, Metric metric) {
-    return toMeasure(measureDto, metric, false);
-  }
-
-  public Optional<Measure> 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<Measure> 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> measure = underTest.toMeasure(new MeasureDto().setRuleId(10), SOME_INT_METRIC, true);
+    Optional<Measure> 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> measure = underTest.toMeasure(new MeasureDto().setCharacteristicId(30), SOME_INT_METRIC, true);
+    Optional<Measure> measure = underTest.toMeasure(new MeasureDto().setCharacteristicId(30), SOME_INT_METRIC);
     assertThat(measure).isPresent();
     assertThat(measure.get().getCharacteristicId()).isEqualTo(30);
   }
@@ -83,22 +83,6 @@ public class MeasureDtoToMeasureTest {
   }
 
   @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> measure = underTest.toMeasure(EMPTY_MEASURE_DTO, SOME_LEVEL_METRIC);
     assertThat(measure).isPresent();
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);
   }