Skip to content

Commit

Permalink
SONAR-11401 Calculate hotspot measures
Browse files Browse the repository at this point in the history
  • Loading branch information
dbmeneses authored and sonartech committed Jun 10, 2022
1 parent 928f60a commit 144b731
Show file tree
Hide file tree
Showing 14 changed files with 259 additions and 239 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,9 @@ public List<ComponentDto> selectDescendants(DbSession dbSession, ComponentTreeQu
return mapper(dbSession).selectDescendants(query, componentOpt.get().uuid(), query.getUuidPath(component));
}

public List<ComponentDto> selectChildren(DbSession dbSession, Collection<ComponentDto> components) {
public List<ComponentDto> selectChildren(DbSession dbSession, String branchUuid, Collection<ComponentDto> components) {
Set<String> uuidPaths = components.stream().map(c -> c.getUuidPath() + c.uuid() + ".").collect(Collectors.toSet());
return mapper(dbSession).selectChildren(uuidPaths);
return mapper(dbSession).selectChildren(branchUuid, uuidPaths);
}

public ComponentDto selectOrFailByKey(DbSession session, String key) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public interface ComponentMapper {

List<ComponentDto> selectDescendants(@Param("query") ComponentTreeQuery query, @Param("baseUuid") String baseUuid, @Param("baseUuidPath") String baseUuidPath);

List<ComponentDto> selectChildren(@Param("uuidPaths") Set<String> uuidPaths);
List<ComponentDto> selectChildren(@Param("branchUuid") String branchUuid, @Param("uuidPaths") Set<String> uuidPaths);

/**
* Returns all enabled projects (Scope {@link org.sonar.api.resources.Scopes#PROJECT} and qualifier
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,9 @@
select
<include refid="componentColumns"/>
from components p
where p.uuid_path in
where
p.project_uuid = #{branchUuid,jdbcType=VARCHAR}
and p.uuid_path in
<foreach collection="uuidPaths" item="uuidPath" open="(" close=")" separator=",">
#{uuidPath,jdbcType=VARCHAR}
</foreach>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1684,16 +1684,16 @@ public void select_children() {
db.commit();

// test children of root
assertThat(underTest.selectChildren(dbSession, List.of(project))).extracting("uuid").containsOnly(FILE_1_UUID, MODULE_UUID);
assertThat(underTest.selectChildren(dbSession, project.uuid(), List.of(project))).extracting("uuid").containsOnly(FILE_1_UUID, MODULE_UUID);

// test children of intermediate component (module here)
assertThat(underTest.selectChildren(dbSession, List.of(module))).extracting("uuid").containsOnly(FILE_2_UUID, FILE_3_UUID);
assertThat(underTest.selectChildren(dbSession, project.uuid(), List.of(module))).extracting("uuid").containsOnly(FILE_2_UUID, FILE_3_UUID);

// test children of leaf component (file here)
assertThat(underTest.selectChildren(dbSession, List.of(fileInProject))).isEmpty();
assertThat(underTest.selectChildren(dbSession, project.uuid(), List.of(fileInProject))).isEmpty();

// test children of 2 components
assertThat(underTest.selectChildren(dbSession, List.of(project, module))).extracting("uuid").containsOnly(FILE_1_UUID, MODULE_UUID, FILE_2_UUID, FILE_3_UUID);
assertThat(underTest.selectChildren(dbSession, project.uuid(), List.of(project, module))).extracting("uuid").containsOnly(FILE_1_UUID, MODULE_UUID, FILE_2_UUID, FILE_3_UUID);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,17 @@ public static Optional<Double> computePercent(long hotspotsToReview, long hotspo
if (total == 0) {
return Optional.empty();
}
return Optional.of(hotspotsReviewed * 100.0 / total);
return Optional.of(hotspotsReviewed * 100.0D / total);
}

public static Rating computeRating(@Nullable Double percent) {
if (percent == null || percent >= 80.0) {
if (percent == null || percent >= 80.0D) {
return A;
} else if (percent >= 70.0) {
} else if (percent >= 70.0D) {
return B;
} else if (percent >= 50.0) {
} else if (percent >= 50.0D) {
return C;
} else if (percent >= 30.0) {
} else if (percent >= 30.0D) {
return D;
}
return E;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void load(DbSession dbSession, List<ComponentDto> touchedComponents) {
sortedComponentsToRoot = loadTreeOfComponents(dbSession, touchedComponents);
branchComponent = findBranchComponent(sortedComponentsToRoot);
children = new HashMap<>();
List<ComponentDto> childComponents = loadChildren(dbSession, sortedComponentsToRoot);
List<ComponentDto> childComponents = loadChildren(dbSession, branchComponent.uuid(), sortedComponentsToRoot);
for (ComponentDto c : childComponents) {
List<String> uuidPathAsList = c.getUuidPathAsList();
String parentUuid = uuidPathAsList.get(uuidPathAsList.size() - 1);
Expand All @@ -66,8 +66,8 @@ private static ComponentDto findBranchComponent(Collection<ComponentDto> compone
.orElseThrow(() -> new IllegalStateException("No project found in " + components));
}

private List<ComponentDto> loadChildren(DbSession dbSession, Collection<ComponentDto> components) {
return dbClient.componentDao().selectChildren(dbSession, components);
private List<ComponentDto> loadChildren(DbSession dbSession, String branchUuid, Collection<ComponentDto> components) {
return dbClient.componentDao().selectChildren(dbSession, branchUuid, components);
}

private List<ComponentDto> loadTreeOfComponents(DbSession dbSession, List<ComponentDto> touchedComponents) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ protected void configureModule() {
ComponentIndexFactory.class,
LiveMeasureTreeUpdaterImpl.class,
LiveMeasureComputerImpl.class,
HotspotMeasureUpdater.class,
LiveQualityGateComputerImpl.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.sonar.api.config.Configuration;
import org.sonar.api.measures.Metric;
Expand All @@ -36,17 +37,23 @@
import org.sonar.server.measure.Rating;

import static com.google.common.base.Preconditions.checkState;
import static org.sonar.api.measures.CoreMetrics.NEW_SECURITY_HOTSPOTS_KEY;
import static org.sonar.api.measures.CoreMetrics.NEW_SECURITY_HOTSPOTS_REVIEWED_KEY;
import static org.sonar.api.measures.CoreMetrics.NEW_SECURITY_HOTSPOTS_REVIEWED_STATUS_KEY;
import static org.sonar.api.measures.CoreMetrics.NEW_SECURITY_HOTSPOTS_TO_REVIEW_STATUS_KEY;
import static org.sonar.api.measures.CoreMetrics.SECURITY_HOTSPOTS_KEY;
import static org.sonar.api.measures.CoreMetrics.SECURITY_HOTSPOTS_REVIEWED_KEY;
import static org.sonar.api.measures.CoreMetrics.SECURITY_HOTSPOTS_REVIEWED_STATUS_KEY;
import static org.sonar.api.measures.CoreMetrics.SECURITY_HOTSPOTS_TO_REVIEW_STATUS_KEY;
import static org.sonar.db.newcodeperiod.NewCodePeriodType.REFERENCE_BRANCH;

public class LiveMeasureTreeUpdaterImpl implements LiveMeasureTreeUpdater {
private final DbClient dbClient;
private final MeasureUpdateFormulaFactory formulaFactory;
private final HotspotMeasureUpdater hotspotMeasureUpdater;

public LiveMeasureTreeUpdaterImpl(DbClient dbClient, MeasureUpdateFormulaFactory formulaFactory, HotspotMeasureUpdater hotspotMeasureUpdater) {
public LiveMeasureTreeUpdaterImpl(DbClient dbClient, MeasureUpdateFormulaFactory formulaFactory) {
this.dbClient = dbClient;
this.formulaFactory = formulaFactory;
this.hotspotMeasureUpdater = hotspotMeasureUpdater;
}

@Override
Expand All @@ -59,12 +66,6 @@ public void update(DbSession dbSession, SnapshotDto lastAnalysis, Configuration

// 2. aggregate new measures up the component tree
updateMatrixWithHierarchy(measures, components, config, shouldUseLeakFormulas);

// 3. Count hotspots at root level
// this is only necessary because the count of reviewed and to_review hotspots is only saved for the root (not for all components).
// For that reason, we can't incrementally generate the new counts up the tree. To have the correct numbers for the root component, we
// run this extra step that set the hotspots measures to the root based on the total count of hotspots.
hotspotMeasureUpdater.apply(dbSession, measures, components, shouldUseLeakFormulas, beginningOfLeak);
}

private void updateMatrixWithHierarchy(MeasureMatrix matrix, ComponentIndex components, Configuration config, boolean useLeakFormulas) {
Expand Down Expand Up @@ -135,7 +136,7 @@ public FormulaContextImpl(MeasureMatrix matrix, ComponentIndex componentIndex, D
this.debtRatingGrid = debtRatingGrid;
}

private void change(ComponentDto component, MeasureUpdateFormula formula) {
void change(ComponentDto component, MeasureUpdateFormula formula) {
this.currentComponent = component;
this.currentFormula = formula;
}
Expand All @@ -149,6 +150,65 @@ public List<Double> getChildrenValues() {
.collect(Collectors.toList());
}

/**
* Some child components may not have the measures 'SECURITY_HOTSPOTS_TO_REVIEW_STATUS' and 'SECURITY_HOTSPOTS_REVIEWED_STATUS' saved for them,
* so we may need to calculate them based on 'SECURITY_HOTSPOTS_REVIEWED' and 'SECURITY_HOTSPOTS'.
*/
@Override
public long getChildrenHotspotsReviewed() {
return getChildrenHotspotsReviewed(LiveMeasureDto::getValue, SECURITY_HOTSPOTS_REVIEWED_STATUS_KEY, SECURITY_HOTSPOTS_REVIEWED_KEY, SECURITY_HOTSPOTS_KEY);
}

/**
* Some child components may not have the measure 'SECURITY_HOTSPOTS_TO_REVIEW_STATUS_KEY'. We assume that 'SECURITY_HOTSPOTS_KEY' has the same value.
*/
@Override
public long getChildrenHotspotsToReview() {
return componentIndex.getChildren(currentComponent)
.stream()
.map(c -> matrix.getMeasure(c, SECURITY_HOTSPOTS_TO_REVIEW_STATUS_KEY).or(() -> matrix.getMeasure(c, SECURITY_HOTSPOTS_KEY)))
.mapToLong(lmOpt -> lmOpt.flatMap(lm -> Optional.ofNullable(lm.getValue())).orElse(0D).longValue())
.sum();
}

@Override
public long getChildrenNewHotspotsReviewed() {
return getChildrenHotspotsReviewed(LiveMeasureDto::getVariation, NEW_SECURITY_HOTSPOTS_REVIEWED_STATUS_KEY, NEW_SECURITY_HOTSPOTS_REVIEWED_KEY, NEW_SECURITY_HOTSPOTS_KEY);
}

/**
* Some child components may not have the measure 'NEW_SECURITY_HOTSPOTS_TO_REVIEW_STATUS_KEY'. We assume that 'NEW_SECURITY_HOTSPOTS_KEY' has the same value.
*/
@Override
public long getChildrenNewHotspotsToReview() {
return componentIndex.getChildren(currentComponent)
.stream()
.map(c -> matrix.getMeasure(c, NEW_SECURITY_HOTSPOTS_TO_REVIEW_STATUS_KEY).or(() -> matrix.getMeasure(c, NEW_SECURITY_HOTSPOTS_KEY)))
.mapToLong(lmOpt -> lmOpt.flatMap(lm -> Optional.ofNullable(lm.getVariation())).orElse(0D).longValue())
.sum();
}

private long getChildrenHotspotsReviewed(Function<LiveMeasureDto, Double> valueFunc, String metricKey, String percMetricKey, String hotspotsMetricKey) {
return componentIndex.getChildren(currentComponent)
.stream()
.mapToLong(c -> getHotspotsReviewed(c, valueFunc, metricKey, percMetricKey, hotspotsMetricKey))
.sum();
}

private long getHotspotsReviewed(ComponentDto c, Function<LiveMeasureDto, Double> valueFunc, String metricKey, String percMetricKey, String hotspotsMetricKey) {
Optional<LiveMeasureDto> measure = matrix.getMeasure(c, metricKey);
return measure.map(lm -> Optional.ofNullable(valueFunc.apply(lm)).orElse(0D).longValue())
.orElseGet(() -> matrix.getMeasure(c, percMetricKey)
.flatMap(percentage -> matrix.getMeasure(c, hotspotsMetricKey)
.map(hotspots -> {
double perc = Optional.ofNullable(valueFunc.apply(percentage)).orElse(0D) / 100D;
double toReview = Optional.ofNullable(valueFunc.apply(hotspots)).orElse(0D);
double reviewed = (toReview * perc) / (1D - perc);
return Math.round(reviewed);
}))
.orElse(0L));
}

public List<Double> getChildrenLeakValues() {
List<ComponentDto> children = componentIndex.getChildren(currentComponent);
return children.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.List;
import java.util.Optional;
import java.util.function.BiConsumer;
import java.util.stream.DoubleStream;
import org.sonar.api.measures.Metric;
import org.sonar.db.component.ComponentDto;
import org.sonar.server.measure.DebtRatingGrid;
Expand Down Expand Up @@ -79,6 +80,14 @@ void computeHierarchy(Context context) {
interface Context {
List<Double> getChildrenValues();

long getChildrenHotspotsReviewed();

long getChildrenHotspotsToReview();

long getChildrenNewHotspotsReviewed();

long getChildrenNewHotspotsToReview();

List<Double> getChildrenLeakValues();

ComponentDto getComponent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,12 @@ public class MeasureUpdateFormulaFactoryImpl implements MeasureUpdateFormulaFact
new MeasureUpdateFormula(CoreMetrics.SECURITY_RATING, false, new MaxRatingChildren(),
(context, issues) -> context.setValue(RATING_BY_SEVERITY.get(issues.getHighestSeverityOfUnresolved(RuleType.VULNERABILITY, false).orElse(Severity.INFO)))),

new MeasureUpdateFormula(SECURITY_HOTSPOTS_REVIEWED_STATUS, false, new AddChildren(),
new MeasureUpdateFormula(SECURITY_HOTSPOTS_REVIEWED_STATUS, false,
(context, formula) -> context.setValue(context.getValue(SECURITY_HOTSPOTS_REVIEWED_STATUS).orElse(0D) + context.getChildrenHotspotsReviewed()),
(context, issues) -> context.setValue(issues.countHotspotsByStatus(Issue.STATUS_REVIEWED, false))),

new MeasureUpdateFormula(SECURITY_HOTSPOTS_TO_REVIEW_STATUS, false, new AddChildren(),
new MeasureUpdateFormula(SECURITY_HOTSPOTS_TO_REVIEW_STATUS, false,
(context, formula) -> context.setValue(context.getValue(SECURITY_HOTSPOTS_TO_REVIEW_STATUS).orElse(0D) + context.getChildrenHotspotsToReview()),
(context, issues) -> context.setValue(issues.countHotspotsByStatus(Issue.STATUS_TO_REVIEW, false))),

new MeasureUpdateFormula(CoreMetrics.SECURITY_HOTSPOTS_REVIEWED, false,
Expand Down Expand Up @@ -193,10 +195,12 @@ public class MeasureUpdateFormulaFactoryImpl implements MeasureUpdateFormulaFact
context.setLeakValue(RATING_BY_SEVERITY.get(highestSeverity));
}),

new MeasureUpdateFormula(NEW_SECURITY_HOTSPOTS_REVIEWED_STATUS, true, new AddChildren(),
new MeasureUpdateFormula(NEW_SECURITY_HOTSPOTS_REVIEWED_STATUS, true,
(context, formula) -> context.setLeakValue(context.getLeakValue(NEW_SECURITY_HOTSPOTS_REVIEWED_STATUS).orElse(0D) + context.getChildrenNewHotspotsReviewed()),
(context, issues) -> context.setLeakValue(issues.countHotspotsByStatus(Issue.STATUS_REVIEWED, true))),

new MeasureUpdateFormula(NEW_SECURITY_HOTSPOTS_TO_REVIEW_STATUS, true, new AddChildren(),
new MeasureUpdateFormula(NEW_SECURITY_HOTSPOTS_TO_REVIEW_STATUS, true,
(context, formula) -> context.setLeakValue(context.getLeakValue(NEW_SECURITY_HOTSPOTS_TO_REVIEW_STATUS).orElse(0D) + context.getChildrenNewHotspotsToReview()),
(context, issues) -> context.setLeakValue(issues.countHotspotsByStatus(Issue.STATUS_TO_REVIEW, true))),

new MeasureUpdateFormula(NEW_SECURITY_HOTSPOTS_REVIEWED, true,
Expand Down Expand Up @@ -239,7 +243,7 @@ private static double debtDensity(MeasureUpdateFormula.Context context) {

private static double newDebtDensity(MeasureUpdateFormula.Context context) {
double debt = Math.max(context.getLeakValue(CoreMetrics.NEW_TECHNICAL_DEBT).orElse(0.0D), 0.0D);
Optional<Double> devCost = context.getText(CoreMetrics.NEW_DEVELOPMENT_COST).map(Double::parseDouble);
Optional<Double> devCost = context.getLeakValue(CoreMetrics.NEW_DEVELOPMENT_COST);
if (devCost.isPresent() && Double.doubleToRawLongBits(devCost.get()) > 0L) {
return debt / devCost.get();
}
Expand Down
Loading

0 comments on commit 144b731

Please sign in to comment.