Skip to content

Commit

Permalink
SONARCLOUD-375 fix NPE in WS api/measures/search_history
Browse files Browse the repository at this point in the history
  • Loading branch information
Simon Brandhof authored and sonartech committed Feb 11, 2019
1 parent 86fd5f9 commit a59930e
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 43 deletions.
4 changes: 4 additions & 0 deletions server/sonar-server/src/main/java/org/sonar/server/measure/ws/MeasureValueFormatter.java
View file
Expand Up @@ -19,6 +19,7 @@
*/ */
package org.sonar.server.measure.ws; package org.sonar.server.measure.ws;


import javax.annotation.CheckForNull;
import javax.annotation.Nullable; import javax.annotation.Nullable;
import org.sonar.api.measures.Metric; import org.sonar.api.measures.Metric;
import org.sonar.db.measure.LiveMeasureDto; import org.sonar.db.measure.LiveMeasureDto;
Expand All @@ -32,18 +33,21 @@ private MeasureValueFormatter() {
// static methods // static methods
} }


@CheckForNull
public static String formatMeasureValue(LiveMeasureDto measure, MetricDto metric) { public static String formatMeasureValue(LiveMeasureDto measure, MetricDto metric) {
Double doubleValue = measure.getValue(); Double doubleValue = measure.getValue();
String stringValue = measure.getDataAsString(); String stringValue = measure.getDataAsString();
return formatMeasureValue(doubleValue == null ? Double.NaN : doubleValue, stringValue, metric); return formatMeasureValue(doubleValue == null ? Double.NaN : doubleValue, stringValue, metric);
} }


@CheckForNull
static String formatMeasureValue(MeasureDto measure, MetricDto metric) { static String formatMeasureValue(MeasureDto measure, MetricDto metric) {
Double doubleValue = measure.getValue(); Double doubleValue = measure.getValue();
String stringValue = measure.getData(); String stringValue = measure.getData();
return formatMeasureValue(doubleValue == null ? Double.NaN : doubleValue, stringValue, metric); return formatMeasureValue(doubleValue == null ? Double.NaN : doubleValue, stringValue, metric);
} }


@CheckForNull
static String formatMeasureValue(double doubleValue, @Nullable String stringValue, MetricDto metric) { static String formatMeasureValue(double doubleValue, @Nullable String stringValue, MetricDto metric) {
Metric.ValueType metricType = Metric.ValueType.valueOf(metric.getValueType()); Metric.ValueType metricType = Metric.ValueType.valueOf(metric.getValueType());
switch (metricType) { switch (metricType) {
Expand Down
4 changes: 3 additions & 1 deletion .../sonar-server/src/main/java/org/sonar/server/measure/ws/SearchHistoryResponseFactory.java
View file
Expand Up @@ -108,7 +108,9 @@ private SnapshotDto addValue(SnapshotDto analysis, MetricDto dbMetric, @Nullable
String measureValue = dbMetric.getKey().startsWith("new_") String measureValue = dbMetric.getKey().startsWith("new_")
? formatNumericalValue(dbMeasure.getVariation(), dbMetric) ? formatNumericalValue(dbMeasure.getVariation(), dbMetric)
: formatMeasureValue(dbMeasure, dbMetric); : formatMeasureValue(dbMeasure, dbMetric);
value.setValue(measureValue); if (measureValue != null) {
value.setValue(measureValue);
}
} }


return analysis; return analysis;
Expand Down
148 changes: 106 additions & 42 deletions server/sonar-server/src/test/java/org/sonar/server/measure/ws/SearchHistoryActionTest.java
View file
Expand Up @@ -19,7 +19,6 @@
*/ */
package org.sonar.server.measure.ws; package org.sonar.server.measure.ws;


import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.stream.LongStream; import java.util.stream.LongStream;
import org.junit.Before; import org.junit.Before;
Expand All @@ -43,7 +42,6 @@
import org.sonar.server.component.TestComponentFinder; import org.sonar.server.component.TestComponentFinder;
import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.ForbiddenException;
import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.NotFoundException;
import org.sonar.server.measure.ws.SearchHistoryAction.Builder;
import org.sonar.server.measure.ws.SearchHistoryAction.SearchHistoryRequest; import org.sonar.server.measure.ws.SearchHistoryAction.SearchHistoryRequest;
import org.sonar.server.tester.UserSessionRule; import org.sonar.server.tester.UserSessionRule;
import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.TestRequest;
Expand All @@ -53,9 +51,9 @@
import org.sonarqube.ws.Measures.SearchHistoryResponse.HistoryMeasure; import org.sonarqube.ws.Measures.SearchHistoryResponse.HistoryMeasure;
import org.sonarqube.ws.Measures.SearchHistoryResponse.HistoryValue; import org.sonarqube.ws.Measures.SearchHistoryResponse.HistoryValue;


import static com.google.common.collect.Lists.newArrayList;
import static java.lang.Double.parseDouble; import static java.lang.Double.parseDouble;
import static java.lang.String.format; import static java.lang.String.format;
import static java.util.Arrays.asList;
import static java.util.Collections.singletonList; import static java.util.Collections.singletonList;
import static java.util.Optional.ofNullable; import static java.util.Optional.ofNullable;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -69,13 +67,13 @@
import static org.sonar.db.component.SnapshotTesting.newAnalysis; import static org.sonar.db.component.SnapshotTesting.newAnalysis;
import static org.sonar.db.measure.MeasureTesting.newMeasureDto; import static org.sonar.db.measure.MeasureTesting.newMeasureDto;
import static org.sonar.db.metric.MetricTesting.newMetricDto; import static org.sonar.db.metric.MetricTesting.newMetricDto;
import static org.sonar.server.component.ws.MeasuresWsParameters.PARAM_PULL_REQUEST;
import static org.sonar.test.JsonAssert.assertJson;
import static org.sonar.server.component.ws.MeasuresWsParameters.PARAM_BRANCH; import static org.sonar.server.component.ws.MeasuresWsParameters.PARAM_BRANCH;
import static org.sonar.server.component.ws.MeasuresWsParameters.PARAM_COMPONENT; import static org.sonar.server.component.ws.MeasuresWsParameters.PARAM_COMPONENT;
import static org.sonar.server.component.ws.MeasuresWsParameters.PARAM_FROM; import static org.sonar.server.component.ws.MeasuresWsParameters.PARAM_FROM;
import static org.sonar.server.component.ws.MeasuresWsParameters.PARAM_METRICS; import static org.sonar.server.component.ws.MeasuresWsParameters.PARAM_METRICS;
import static org.sonar.server.component.ws.MeasuresWsParameters.PARAM_PULL_REQUEST;
import static org.sonar.server.component.ws.MeasuresWsParameters.PARAM_TO; import static org.sonar.server.component.ws.MeasuresWsParameters.PARAM_TO;
import static org.sonar.test.JsonAssert.assertJson;


public class SearchHistoryActionTest { public class SearchHistoryActionTest {


Expand All @@ -93,11 +91,10 @@ public class SearchHistoryActionTest {


private ComponentDto project; private ComponentDto project;
private SnapshotDto analysis; private SnapshotDto analysis;
private List<String> metrics;
private MetricDto complexityMetric; private MetricDto complexityMetric;
private MetricDto nclocMetric; private MetricDto nclocMetric;
private MetricDto newViolationMetric; private MetricDto newViolationMetric;
private Builder wsRequest; private MetricDto stringMetric;


@Before @Before
public void setUp() { public void setUp() {
Expand All @@ -107,19 +104,19 @@ public void setUp() {
nclocMetric = insertNclocMetric(); nclocMetric = insertNclocMetric();
complexityMetric = insertComplexityMetric(); complexityMetric = insertComplexityMetric();
newViolationMetric = insertNewViolationMetric(); newViolationMetric = insertNewViolationMetric();
metrics = newArrayList(nclocMetric.getKey(), complexityMetric.getKey(), newViolationMetric.getKey()); stringMetric = insertStringMetric();
wsRequest = SearchHistoryRequest.builder().setComponent(project.getDbKey()).setMetrics(metrics);
} }


@Test @Test
public void empty_response() { public void empty_response() {
project = db.components().insertPrivateProject(); project = db.components().insertPrivateProject();
userSession.addProjectPermission(UserRole.USER, project); userSession.addProjectPermission(UserRole.USER, project);
wsRequest SearchHistoryRequest request = SearchHistoryRequest.builder()
.setComponent(project.getDbKey()) .setComponent(project.getDbKey())
.setMetrics(singletonList(complexityMetric.getKey())); .setMetrics(singletonList(complexityMetric.getKey()))
.build();


SearchHistoryResponse result = call(); SearchHistoryResponse result = call(request);


assertThat(result.getMeasuresList()).hasSize(1); assertThat(result.getMeasuresList()).hasSize(1);
assertThat(result.getMeasures(0).getHistoryCount()).isEqualTo(0); assertThat(result.getMeasures(0).getHistoryCount()).isEqualTo(0);
Expand All @@ -134,11 +131,13 @@ public void analyses_but_no_measure() {
project = db.components().insertPrivateProject(); project = db.components().insertPrivateProject();
analysis = db.components().insertSnapshot(project); analysis = db.components().insertSnapshot(project);
userSession.addProjectPermission(UserRole.USER, project); userSession.addProjectPermission(UserRole.USER, project);
wsRequest
SearchHistoryRequest request = SearchHistoryRequest.builder()
.setComponent(project.getDbKey()) .setComponent(project.getDbKey())
.setMetrics(singletonList(complexityMetric.getKey())); .setMetrics(singletonList(complexityMetric.getKey()))
.build();


SearchHistoryResponse result = call(); SearchHistoryResponse result = call(request);


assertThat(result.getPaging()).extracting(Paging::getPageIndex, Paging::getPageSize, Paging::getTotal).containsExactly(1, 100, 1); assertThat(result.getPaging()).extracting(Paging::getPageIndex, Paging::getPageSize, Paging::getTotal).containsExactly(1, 100, 1);
assertThat(result.getMeasuresList()).hasSize(1); assertThat(result.getMeasuresList()).hasSize(1);
Expand All @@ -150,7 +149,12 @@ public void return_metrics() {
dbClient.measureDao().insert(dbSession, newMeasureDto(complexityMetric, project, analysis).setValue(42.0d)); dbClient.measureDao().insert(dbSession, newMeasureDto(complexityMetric, project, analysis).setValue(42.0d));
db.commit(); db.commit();


SearchHistoryResponse result = call(); SearchHistoryRequest request = SearchHistoryRequest.builder()
.setComponent(project.getDbKey())
.setMetrics(asList(complexityMetric.getKey(), nclocMetric.getKey(), newViolationMetric.getKey()))
.build();

SearchHistoryResponse result = call(request);


assertThat(result.getMeasuresList()).hasSize(3) assertThat(result.getMeasuresList()).hasSize(3)
.extracting(HistoryMeasure::getMetric) .extracting(HistoryMeasure::getMetric)
Expand All @@ -170,7 +174,11 @@ public void return_measures() {
newMeasureDto(newViolationMetric, project, laterAnalysis).setVariation(10d)); newMeasureDto(newViolationMetric, project, laterAnalysis).setVariation(10d));
db.commit(); db.commit();


SearchHistoryResponse result = call(); SearchHistoryRequest request = SearchHistoryRequest.builder()
.setComponent(project.getDbKey())
.setMetrics(asList(complexityMetric.getKey(), nclocMetric.getKey(), newViolationMetric.getKey()))
.build();
SearchHistoryResponse result = call(request);


assertThat(result.getPaging()).extracting(Paging::getPageIndex, Paging::getPageSize, Paging::getTotal) assertThat(result.getPaging()).extracting(Paging::getPageIndex, Paging::getPageSize, Paging::getTotal)
.containsExactly(1, 100, 2); .containsExactly(1, 100, 2);
Expand Down Expand Up @@ -205,9 +213,14 @@ public void pagination_applies_to_analyses() {
.map(a -> formatDateTime(a.getCreatedAt())) .map(a -> formatDateTime(a.getCreatedAt()))
.collect(MoreCollectors.toList()); .collect(MoreCollectors.toList());
db.commit(); db.commit();
wsRequest.setComponent(project.getDbKey()).setPage(2).setPageSize(3);


SearchHistoryResponse result = call(); SearchHistoryRequest request = SearchHistoryRequest.builder()
.setComponent(project.getDbKey())
.setMetrics(asList(complexityMetric.getKey(), nclocMetric.getKey(), newViolationMetric.getKey()))
.setPage(2)
.setPageSize(3)
.build();
SearchHistoryResponse result = call(request);


assertThat(result.getPaging()).extracting(Paging::getPageIndex, Paging::getPageSize, Paging::getTotal).containsExactly(2, 3, 9); assertThat(result.getPaging()).extracting(Paging::getPageIndex, Paging::getPageSize, Paging::getTotal).containsExactly(2, 3, 9);
assertThat(result.getMeasures(0).getHistoryList()).extracting(HistoryValue::getDate).containsExactly( assertThat(result.getMeasures(0).getHistoryList()).extracting(HistoryValue::getDate).containsExactly(
Expand All @@ -224,9 +237,14 @@ public void inclusive_from_and_to_dates() {
.map(a -> formatDateTime(a.getCreatedAt())) .map(a -> formatDateTime(a.getCreatedAt()))
.collect(MoreCollectors.toList()); .collect(MoreCollectors.toList());
db.commit(); db.commit();
wsRequest.setComponent(project.getDbKey()).setFrom(analysisDates.get(1)).setTo(analysisDates.get(3));


SearchHistoryResponse result = call(); SearchHistoryRequest request = SearchHistoryRequest.builder()
.setComponent(project.getDbKey())
.setMetrics(asList(complexityMetric.getKey(), nclocMetric.getKey(), newViolationMetric.getKey()))
.setFrom(analysisDates.get(1))
.setTo(analysisDates.get(3))
.build();
SearchHistoryResponse result = call(request);


assertThat(result.getPaging()).extracting(Paging::getPageIndex, Paging::getPageSize, Paging::getTotal).containsExactly(1, 100, 3); assertThat(result.getPaging()).extracting(Paging::getPageIndex, Paging::getPageSize, Paging::getTotal).containsExactly(1, 100, 3);
assertThat(result.getMeasures(0).getHistoryList()).extracting(HistoryValue::getDate).containsExactly( assertThat(result.getMeasures(0).getHistoryList()).extracting(HistoryValue::getDate).containsExactly(
Expand All @@ -239,17 +257,23 @@ public void return_best_values_for_files() {
dbClient.metricDao().insert(dbSession, newMetricDto().setKey("new_optimized").setValueType(ValueType.INT.name()).setOptimizedBestValue(true).setBestValue(789d)); dbClient.metricDao().insert(dbSession, newMetricDto().setKey("new_optimized").setValueType(ValueType.INT.name()).setOptimizedBestValue(true).setBestValue(789d));
db.commit(); db.commit();
ComponentDto file = db.components().insertComponent(newFileDto(project)); ComponentDto file = db.components().insertComponent(newFileDto(project));
wsRequest.setComponent(file.getDbKey()).setMetrics(Arrays.asList("optimized", "new_optimized"));


SearchHistoryResponse result = call(); SearchHistoryRequest request = SearchHistoryRequest.builder()
.setComponent(file.getDbKey())
.setMetrics(asList("optimized", "new_optimized"))
.build();
SearchHistoryResponse result = call(request);


assertThat(result.getMeasuresCount()).isEqualTo(2); assertThat(result.getMeasuresCount()).isEqualTo(2);
assertThat(result.getMeasuresList().get(0).getHistoryList()).extracting(HistoryValue::getValue).containsExactly("789"); assertThat(result.getMeasuresList().get(0).getHistoryList()).extracting(HistoryValue::getValue).containsExactly("789");
assertThat(result.getMeasuresList().get(1).getHistoryList()).extracting(HistoryValue::getValue).containsExactly("456"); assertThat(result.getMeasuresList().get(1).getHistoryList()).extracting(HistoryValue::getValue).containsExactly("456");


// Best value is not applied to project // Best value is not applied to project
wsRequest.setComponent(project.getDbKey()); request = SearchHistoryRequest.builder()
result = call(); .setComponent(project.getDbKey())
.setMetrics(asList("optimized", "new_optimized"))
.build();
result = call(request);
assertThat(result.getMeasuresList().get(0).getHistoryCount()).isEqualTo(1); assertThat(result.getMeasuresList().get(0).getHistoryCount()).isEqualTo(1);
assertThat(result.getMeasuresList().get(0).getHistory(0).hasDate()).isTrue(); assertThat(result.getMeasuresList().get(0).getHistory(0).hasDate()).isTrue();
assertThat(result.getMeasuresList().get(0).getHistory(0).hasValue()).isFalse(); assertThat(result.getMeasuresList().get(0).getHistory(0).hasValue()).isFalse();
Expand All @@ -260,7 +284,11 @@ public void do_not_return_unprocessed_analyses() {
dbClient.snapshotDao().insert(dbSession, newAnalysis(project).setStatus(STATUS_UNPROCESSED)); dbClient.snapshotDao().insert(dbSession, newAnalysis(project).setStatus(STATUS_UNPROCESSED));
db.commit(); db.commit();


SearchHistoryResponse result = call(); SearchHistoryRequest request = SearchHistoryRequest.builder()
.setComponent(project.getDbKey())
.setMetrics(asList(complexityMetric.getKey(), nclocMetric.getKey(), newViolationMetric.getKey()))
.build();
SearchHistoryResponse result = call(request);


// one analysis in setUp method // one analysis in setUp method
assertThat(result.getPaging().getTotal()).isEqualTo(1); assertThat(result.getPaging().getTotal()).isEqualTo(1);
Expand Down Expand Up @@ -330,30 +358,40 @@ public void fail_when_using_branch_db_key() throws Exception {


@Test @Test
public void fail_if_unknown_metric() { public void fail_if_unknown_metric() {
wsRequest.setMetrics(newArrayList(complexityMetric.getKey(), nclocMetric.getKey(), "METRIC_42", "42_METRIC")); SearchHistoryRequest request = SearchHistoryRequest.builder()
.setComponent(project.getDbKey())
.setMetrics(asList(complexityMetric.getKey(), nclocMetric.getKey(), "METRIC_42", "42_METRIC"))
.build();


expectedException.expect(IllegalArgumentException.class); expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage("Metrics 42_METRIC, METRIC_42 are not found"); expectedException.expectMessage("Metrics 42_METRIC, METRIC_42 are not found");


call(); call(request);
} }


@Test @Test
public void fail_if_not_enough_permissions() { public void fail_if_not_enough_permissions() {
userSession.logIn().addProjectPermission(UserRole.ADMIN, project); userSession.logIn().addProjectPermission(UserRole.ADMIN, project);
SearchHistoryRequest request = SearchHistoryRequest.builder()
.setComponent(project.getDbKey())
.setMetrics(singletonList(complexityMetric.getKey()))
.build();


expectedException.expect(ForbiddenException.class); expectedException.expect(ForbiddenException.class);


call(); call(request);
} }


@Test @Test
public void fail_if_unknown_component() { public void fail_if_unknown_component() {
wsRequest.setComponent("PROJECT_42"); SearchHistoryRequest request = SearchHistoryRequest.builder()
.setComponent("__UNKNOWN__")
.setMetrics(singletonList(complexityMetric.getKey()))
.build();


expectedException.expect(NotFoundException.class); expectedException.expect(NotFoundException.class);


call(); call(request);
} }


@Test @Test
Expand Down Expand Up @@ -420,25 +458,42 @@ public void json_example() {


String result = ws.newRequest() String result = ws.newRequest()
.setParam(PARAM_COMPONENT, project.getDbKey()) .setParam(PARAM_COMPONENT, project.getDbKey())
.setParam(PARAM_METRICS, String.join(",", metrics)) .setParam(PARAM_METRICS, String.join(",", asList(complexityMetric.getKey(), nclocMetric.getKey(), newViolationMetric.getKey())))
.execute().getInput(); .execute().getInput();


assertJson(result).isSimilarTo(ws.getDef().responseExampleAsString()); assertJson(result).isSimilarTo(ws.getDef().responseExampleAsString());
} }


private SearchHistoryResponse call() { @Test
SearchHistoryRequest wsRequest = this.wsRequest.build(); public void measure_without_values() {
dbClient.measureDao().insert(dbSession, newMeasureDto(stringMetric, project, analysis).setValue(null).setData(null));
db.commit();

SearchHistoryRequest request = SearchHistoryRequest.builder()
.setComponent(project.getDbKey())
.setMetrics(singletonList(stringMetric.getKey()))
.build();
SearchHistoryResponse result = call(request);

HistoryMeasure measure = result.getMeasuresList().stream()
.filter(m -> m.getMetric().equals(stringMetric.getKey()))
.findFirst()
.get();
assertThat(measure.getHistoryList()).hasSize(1);
assertThat(measure.getHistory(0).hasValue()).isFalse();
}


TestRequest request = ws.newRequest(); private SearchHistoryResponse call(SearchHistoryRequest request) {
TestRequest testRequest = ws.newRequest();


request.setParam(PARAM_COMPONENT, wsRequest.getComponent()); testRequest.setParam(PARAM_COMPONENT, request.getComponent());
request.setParam(PARAM_METRICS, String.join(",", wsRequest.getMetrics())); testRequest.setParam(PARAM_METRICS, String.join(",", request.getMetrics()));
ofNullable(wsRequest.getFrom()).ifPresent(from -> request.setParam(PARAM_FROM, from)); ofNullable(request.getFrom()).ifPresent(from -> testRequest.setParam(PARAM_FROM, from));
ofNullable(wsRequest.getTo()).ifPresent(to -> request.setParam(PARAM_TO, to)); ofNullable(request.getTo()).ifPresent(to -> testRequest.setParam(PARAM_TO, to));
ofNullable(wsRequest.getPage()).ifPresent(p -> request.setParam(Param.PAGE, String.valueOf(p))); ofNullable(request.getPage()).ifPresent(p -> testRequest.setParam(Param.PAGE, String.valueOf(p)));
ofNullable(wsRequest.getPageSize()).ifPresent(ps -> request.setParam(Param.PAGE_SIZE, String.valueOf(ps))); ofNullable(request.getPageSize()).ifPresent(ps -> testRequest.setParam(Param.PAGE_SIZE, String.valueOf(ps)));


return request.executeProtobuf(SearchHistoryResponse.class); return testRequest.executeProtobuf(SearchHistoryResponse.class);
} }


private static MetricDto newMetricDtoWithoutOptimization() { private static MetricDto newMetricDtoWithoutOptimization() {
Expand Down Expand Up @@ -493,4 +548,13 @@ private MetricDto insertNewViolationMetric() {
db.commit(); db.commit();
return metric; return metric;
} }

private MetricDto insertStringMetric() {
MetricDto metric = dbClient.metricDao().insert(dbSession, newMetricDtoWithoutOptimization()
.setKey("a_string")
.setShortName("A String")
.setValueType("STRING"));
db.commit();
return metric;
}
} }

0 comments on commit a59930e

Please sign in to comment.