Skip to content

Commit

Permalink
SONAR-7237 Variation measures are set to current value when no past m…
Browse files Browse the repository at this point in the history 
…easures
  • Loading branch information
@julienlancelot
julienlancelot committed May 19, 2016
1 parent b70f290 commit e51c45d
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 53 deletions.
76 changes: 29 additions & 47 deletions ...-server/src/main/java/org/sonar/server/computation/step/ComputeMeasureVariationsStep.java
View file
Expand Up @@ -20,6 +20,7 @@
package org.sonar.server.computation.step;

import com.google.common.base.Function;
import com.google.common.base.Optional;
import com.google.common.base.Predicate;
import java.util.Collection;
import java.util.HashMap;
Expand Down Expand Up @@ -48,6 +49,7 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.FluentIterable.from;
import static java.lang.String.format;
import static org.sonar.server.computation.component.Component.Type.DIRECTORY;
import static org.sonar.server.computation.component.Component.Type.SUBVIEW;
import static org.sonar.server.computation.component.ComponentVisitor.Order.PRE_ORDER;
Expand Down Expand Up @@ -100,14 +102,14 @@ private class VariationMeasuresVisitor extends TypeAwareVisitorAdapter {

private final DbSession session;
private final Set<Integer> metricIds;
private final Map<String, Metric> metricByKeys;
private final List<Metric> metrics;

public VariationMeasuresVisitor(DbSession session, Iterable<Metric> metrics) {
VariationMeasuresVisitor(DbSession session, List<Metric> metrics) {
// measures on files are currently purged, so past measures are not available on files
super(CrawlerDepthLimit.reportMaxDepth(DIRECTORY).withViewsMaxDepth(SUBVIEW), PRE_ORDER);
this.session = session;
this.metricIds = from(metrics).transform(MetricDtoToMetricId.INSTANCE).toSet();
this.metricByKeys = from(metrics).uniqueIndex(MetricToKey.INSTANCE);
this.metrics = metrics;
}

@Override
Expand All @@ -126,35 +128,19 @@ private MeasuresWithVariationRepository computeMeasuresWithVariations(Component
return measuresWithVariationRepository;
}

private void processMeasuresWithVariation(Component component, MeasuresWithVariationRepository measuresWithVariationRepository) {
for (MeasureWithVariations measureWithVariations : measuresWithVariationRepository.measures()) {
Metric metric = measureWithVariations.getMetric();
Measure measure = Measure.updatedMeasureBuilder(measureWithVariations.getMeasure())
.setVariations(new MeasureVariations(
measureWithVariations.getVariation(1),
measureWithVariations.getVariation(2),
measureWithVariations.getVariation(3),
measureWithVariations.getVariation(4),
measureWithVariations.getVariation(5)))
.create();
measureRepository.update(component, metric, measure);
}
}

private void setVariationMeasures(Component component, List<PastMeasureDto> pastMeasures, int period, MeasuresWithVariationRepository measuresWithVariationRepository) {
Map<MeasureKey, PastMeasureDto> pastMeasuresByMeasureKey = from(pastMeasures).uniqueIndex(pastMeasureToMeasureKey);
for (Map.Entry<String, Measure> entry : from(measureRepository.getRawMeasures(component).entries()).filter(NotDeveloperMeasure.INSTANCE)) {
String metricKey = entry.getKey();
Measure measure = entry.getValue();
PastMeasureDto pastMeasure = pastMeasuresByMeasureKey.get(new MeasureKey(metricKey, null));
if (pastMeasure != null && pastMeasure.hasValue()) {
Metric metric = metricByKeys.get(metricKey);
measuresWithVariationRepository.add(metric, measure, period, computeVariation(measure, pastMeasure.getValue()));
for (Metric metric : metrics) {
Optional<Measure> measure = measureRepository.getRawMeasure(component, metric);
if (measure.isPresent() && !measure.get().hasVariations()) {
PastMeasureDto pastMeasure = pastMeasuresByMeasureKey.get(new MeasureKey(metric.getKey(), null));
double pastValue = (pastMeasure != null && pastMeasure.hasValue()) ? pastMeasure.getValue() : 0d;
measuresWithVariationRepository.add(metric, measure.get(), period, computeVariation(measure.get(), pastValue));
}
}
}

private double computeVariation(Measure measure, Double pastValue) {
private double computeVariation(Measure measure, double pastValue) {
switch (measure.getValueType()) {
case INT:
return measure.getIntValue() - pastValue;
Expand All @@ -165,7 +151,22 @@ private double computeVariation(Measure measure, Double pastValue) {
case BOOLEAN:
return (measure.getBooleanValue() ? 1d : 0d) - pastValue;
default:
throw new IllegalArgumentException("Unsupported Measure.ValueType " + measure.getValueType());
throw new IllegalArgumentException(format("Unsupported Measure.ValueType on measure '%s'", measure));
}
}

private void processMeasuresWithVariation(Component component, MeasuresWithVariationRepository measuresWithVariationRepository) {
for (MeasureWithVariations measureWithVariations : measuresWithVariationRepository.measures()) {
Metric metric = measureWithVariations.getMetric();
Measure measure = Measure.updatedMeasureBuilder(measureWithVariations.getMeasure())
.setVariations(new MeasureVariations(
measureWithVariations.getVariation(1),
measureWithVariations.getVariation(2),
measureWithVariations.getVariation(3),
measureWithVariations.getVariation(4),
measureWithVariations.getVariation(5)))
.create();
measureRepository.update(component, metric, measure);
}
}
}
Expand Down Expand Up @@ -195,7 +196,7 @@ private static final class MeasureWithVariations {
private final Measure measure;
private final Double[] variations = new Double[5];

public MeasureWithVariations(Metric metric, Measure measure) {
MeasureWithVariations(Metric metric, Measure measure) {
this.metric = metric;
this.measure = measure;
}
Expand Down Expand Up @@ -228,16 +229,6 @@ public Integer apply(@Nonnull Metric metric) {
}
}

private enum MetricToKey implements Function<Metric, String> {
INSTANCE;

@Nullable
@Override
public String apply(@Nonnull Metric metric) {
return metric.getKey();
}
}

private enum NumericMetric implements Predicate<Metric> {
INSTANCE;

Expand All @@ -251,15 +242,6 @@ public boolean apply(@Nonnull Metric metric) {
}
}

private enum NotDeveloperMeasure implements Predicate<Map.Entry<String, Measure>> {
INSTANCE;

@Override
public boolean apply(@Nonnull Map.Entry<String, Measure> input) {
return input.getValue().getDeveloper() == null;
}
}

@Override
public String getDescription() {
return "Compute measure variations";
Expand Down
87 changes: 81 additions & 6 deletions ...c/test/java/org/sonar/server/computation/step/ReportComputeMeasureVariationsStepTest.java
View file
Expand Up @@ -37,6 +37,7 @@
import org.sonar.server.computation.component.ReportComponent;
import org.sonar.server.computation.measure.Measure;
import org.sonar.server.computation.measure.MeasureRepositoryRule;
import org.sonar.server.computation.measure.MeasureVariations;
import org.sonar.server.computation.metric.Metric;
import org.sonar.server.computation.metric.MetricImpl;
import org.sonar.server.computation.metric.MetricRepositoryRule;
Expand All @@ -48,13 +49,13 @@
import static org.sonar.db.component.SnapshotTesting.newSnapshotForProject;
import static org.sonar.server.computation.measure.Measure.newMeasureBuilder;


public class ReportComputeMeasureVariationsStepTest {

static final Metric ISSUES_METRIC = new MetricImpl(1, "violations", "violations", Metric.MetricType.INT);
static final Metric DEBT_METRIC = new MetricImpl(2, "sqale_index", "sqale_index", Metric.MetricType.WORK_DUR);
static final Metric FILE_COMPLEXITY_METRIC = new MetricImpl(3, "file_complexity", "file_complexity", Metric.MetricType.FLOAT);
static final Metric BUILD_BREAKER_METRIC = new MetricImpl(4, "build_breaker", "build_breaker", Metric.MetricType.BOOL);
static final Metric NEW_DEBT = new MetricImpl(5, "new_debt", "new_debt", Metric.MetricType.WORK_DUR);

static final ComponentDto PROJECT_DTO = ComponentTesting.newProjectDto();

Expand All @@ -74,7 +75,8 @@ public class ReportComputeMeasureVariationsStepTest {
.add(ISSUES_METRIC)
.add(DEBT_METRIC)
.add(FILE_COMPLEXITY_METRIC)
.add(BUILD_BREAKER_METRIC);
.add(BUILD_BREAKER_METRIC)
.add(NEW_DEBT);
@Rule
public MeasureRepositoryRule measureRepository = MeasureRepositoryRule.create(treeRootHolder, metricRepository);

Expand Down Expand Up @@ -150,6 +152,64 @@ public void set_variation() {
assertThat(measureRepository.getRawMeasure(directory, ISSUES_METRIC).get().getVariations().getVariation1()).isEqualTo(10d);
}

@Test
public void set_zero_variation_when_no_change() {
// Project
SnapshotDto period1ProjectSnapshot = newSnapshotForProject(PROJECT_DTO);
dbClient.snapshotDao().insert(session, period1ProjectSnapshot);
dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 60d));

// Directory
ComponentDto directoryDto = ComponentTesting.newDirectory(PROJECT_DTO, "dir");
dbClient.componentDao().insert(session, directoryDto);
SnapshotDto period1DirectorySnapshot = createForComponent(directoryDto, period1ProjectSnapshot);
dbClient.snapshotDao().insert(session, period1DirectorySnapshot);
dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), directoryDto.getId(), period1DirectorySnapshot.getId(), 10d));
session.commit();

periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot));

Component directory = ReportComponent.builder(Component.Type.DIRECTORY, 2).setUuid(directoryDto.uuid()).build();
Component project = ReportComponent.builder(Component.Type.PROJECT, 1).setUuid(PROJECT_DTO.uuid()).addChildren(directory).build();
treeRootHolder.setRoot(project);

addRawMeasure(project, ISSUES_METRIC, newMeasureBuilder().create(60, null));
addRawMeasure(directory, ISSUES_METRIC, newMeasureBuilder().create(10, null));

underTest.execute();

assertThat(measureRepository.getRawMeasure(project, ISSUES_METRIC).get().getVariations().getVariation1()).isEqualTo(0d);
assertThat(measureRepository.getRawMeasure(directory, ISSUES_METRIC).get().getVariations().getVariation1()).isEqualTo(0d);
}

@Test
public void set_variation_to_raw_value_on_new_component() throws Exception {
// Project
SnapshotDto past1ProjectSnapshot = newSnapshotForProject(PROJECT_DTO).setCreatedAt(1000_000_000L);
SnapshotDto currentProjectSnapshot = newSnapshotForProject(PROJECT_DTO).setCreatedAt(2000_000_000L);
dbClient.snapshotDao().insert(session, past1ProjectSnapshot);
dbClient.snapshotDao().insert(session, currentProjectSnapshot);
dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), past1ProjectSnapshot.getId(), 60d));
dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), currentProjectSnapshot.getId(), 60d));
session.commit();

periodsHolder.setPeriods(newPeriod(1, past1ProjectSnapshot));

// Directory has just been added => no snapshot
Component directory = ReportComponent.builder(Component.Type.DIRECTORY, 2).setUuid("DIRECTORY").build();
Component project = ReportComponent.builder(Component.Type.PROJECT, 1).setUuid(PROJECT_DTO.uuid()).addChildren(directory).build();
treeRootHolder.setRoot(project);

addRawMeasure(project, ISSUES_METRIC, newMeasureBuilder().create(90, null));
addRawMeasure(directory, ISSUES_METRIC, newMeasureBuilder().create(10, null));

underTest.execute();

assertThat(measureRepository.getRawMeasure(project, ISSUES_METRIC).get().getVariations().getVariation1()).isEqualTo(30d);
// Variation should be the raw value
assertThat(measureRepository.getRawMeasure(directory, ISSUES_METRIC).get().getVariations().getVariation1()).isEqualTo(10d);
}

@Test
public void set_variations_on_all_periods() {
SnapshotDto period1ProjectSnapshot = newSnapshotForProject(PROJECT_DTO).setLast(false);
Expand Down Expand Up @@ -198,8 +258,7 @@ public void set_variation_on_all_numeric_metrics() {
newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 60d),
newMeasureDto(DEBT_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 10d),
newMeasureDto(FILE_COMPLEXITY_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 2d),
newMeasureDto(BUILD_BREAKER_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 1d)
);
newMeasureDto(BUILD_BREAKER_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 1d));
session.commit();

periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot));
Expand All @@ -226,8 +285,7 @@ public void do_not_set_variations_on_numeric_metric_for_developer() {
SnapshotDto period1ProjectSnapshot = newSnapshotForProject(PROJECT_DTO);
dbClient.snapshotDao().insert(session, period1ProjectSnapshot);
dbClient.measureDao().insert(session,
newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 60d)
);
newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 60d));
session.commit();

periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot));
Expand All @@ -244,6 +302,23 @@ public void do_not_set_variations_on_numeric_metric_for_developer() {
assertThat(measureRepository.getRawMeasure(PROJECT, ISSUES_METRIC, developer).get().hasVariations()).isFalse();
}

@Test
public void does_not_update_existing_variations() throws Exception {
SnapshotDto period1ProjectSnapshot = newSnapshotForProject(PROJECT_DTO);
dbClient.snapshotDao().insert(session, period1ProjectSnapshot);
dbClient.measureDao().insert(session, newMeasureDto(NEW_DEBT.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 60d));
session.commit();
periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot));
treeRootHolder.setRoot(PROJECT);

addRawMeasure(PROJECT, NEW_DEBT, newMeasureBuilder().setVariations(new MeasureVariations(10d)).createNoValue());

underTest.execute();

// As the measure has already variations it has been ignored, then variations will be the same
assertThat(measureRepository.getRawMeasure(PROJECT, NEW_DEBT).get().getVariations().getVariation1()).isEqualTo(10d);
}

private static MeasureDto newMeasureDto(int metricId, long projectId, long snapshotId, double value) {
return new MeasureDto().setMetricId(metricId).setComponentId(projectId).setSnapshotId(snapshotId).setValue(value);
}
Expand Down

0 comments on commit e51c45d

Please sign in to comment.