From 3e95ff00461772ebb536b8f43cd9bbf6f779852f Mon Sep 17 00:00:00 2001 From: Eric Giffon Date: Thu, 25 Jan 2024 16:54:45 +0100 Subject: SONAR-21455 Live update should not recompute measures not existing on branch --- .../measure/live/LiveMeasureTreeUpdaterImplIT.java | 37 ++++++++++++++++++++-- .../measure/live/LiveMeasureTreeUpdaterImpl.java | 14 ++++++-- .../server/measure/live/MeasureUpdateFormula.java | 17 ++++++++-- .../live/MeasureUpdateFormulaFactoryImpl.java | 20 ++++++------ 4 files changed, 70 insertions(+), 18 deletions(-) diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/measure/live/LiveMeasureTreeUpdaterImplIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/measure/live/LiveMeasureTreeUpdaterImplIT.java index df5064c0402..23fbe967485 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/measure/live/LiveMeasureTreeUpdaterImplIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/measure/live/LiveMeasureTreeUpdaterImplIT.java @@ -247,10 +247,43 @@ public class LiveMeasureTreeUpdaterImplIT { assertThat(context.getChildrenHotspotsReviewed()).isEqualTo(10); } + @Test + public void update_whenFormulaIsOnlyIfComputedOnBranchAndMetricNotComputedOnBranch_shouldNotCompute() { + snapshot = db.components().insertSnapshot(project); + treeUpdater = new LiveMeasureTreeUpdaterImpl(db.getDbClient(), new AggregateValuesFormula()); + + componentIndex.load(db.getSession(), List.of(file1)); + List initialValues = + List.of(new LiveMeasureDto().setComponentUuid(file1.uuid()).setValue(1d).setMetricUuid(metricDto.getUuid())); + matrix = new MeasureMatrix(List.of(project, dir, file1, file2), List.of(metricDto), initialValues); + treeUpdater.update(db.getSession(), snapshot, config, componentIndex, branch, matrix); + + assertThat(matrix.getChanged()).isEmpty(); + assertThat(matrix.getMeasure(project, metric.getKey())).isEmpty(); + } + + @Test + public void update_whenFormulaIsNotOnlyIfComputedOnBranchAndMetricNotComputedOnBranch_shouldCompute() { + snapshot = db.components().insertSnapshot(project); + treeUpdater = new LiveMeasureTreeUpdaterImpl(db.getDbClient(), new SetValuesFormula()); + + componentIndex.load(db.getSession(), List.of(file1)); + List initialValues = + List.of(new LiveMeasureDto().setComponentUuid(file1.uuid()).setValue(1d).setMetricUuid(metricDto.getUuid())); + matrix = new MeasureMatrix(List.of(project, dir, file1, file2), List.of(metricDto), initialValues); + treeUpdater.update(db.getSession(), snapshot, config, componentIndex, branch, matrix); + + assertThat(matrix.getChanged()).extracting(LiveMeasureDto::getComponentUuid).containsOnly(project.uuid(), dir.uuid()); + assertThat(matrix.getMeasure(project, metric.getKey()).get().getValue()).isEqualTo(1d); + assertThat(matrix.getMeasure(dir, metric.getKey()).get().getValue()).isEqualTo(1d); + assertThat(matrix.getMeasure(file1, metric.getKey()).get().getValue()).isEqualTo(1d); + assertThat(matrix.getMeasure(file2, metric.getKey())).isEmpty(); + } + private class AggregateValuesFormula implements MeasureUpdateFormulaFactory { @Override public List getFormulas() { - return List.of(new MeasureUpdateFormula(metric, false, new MeasureUpdateFormulaFactoryImpl.AddChildren(), (c, i) -> { + return List.of(new MeasureUpdateFormula(metric, false, true, new MeasureUpdateFormulaFactoryImpl.AddChildren(), (c, i) -> { })); } @@ -263,7 +296,7 @@ public class LiveMeasureTreeUpdaterImplIT { private class SetValuesFormula implements MeasureUpdateFormulaFactory { @Override public List getFormulas() { - return List.of(new MeasureUpdateFormula(metric, false, (c, m) -> { + return List.of(new MeasureUpdateFormula(metric, false, false, (c, m) -> { }, (c, i) -> c.setValue(1d))); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/LiveMeasureTreeUpdaterImpl.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/LiveMeasureTreeUpdaterImpl.java index 4ddf4b42829..71eb40833c8 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/LiveMeasureTreeUpdaterImpl.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/LiveMeasureTreeUpdaterImpl.java @@ -70,7 +70,7 @@ public class LiveMeasureTreeUpdaterImpl implements LiveMeasureTreeUpdater { FormulaContextImpl context = new FormulaContextImpl(matrix, components, debtRatingGrid); components.getSortedTree().forEach(c -> { for (MeasureUpdateFormula formula : formulaFactory.getFormulas()) { - if (useLeakFormulas || !formula.isOnLeak()) { + if (shouldComputeMetric(formula, useLeakFormulas, components.getBranch(), matrix)) { context.change(c, formula); try { formula.computeHierarchy(context); @@ -91,8 +91,7 @@ public class LiveMeasureTreeUpdaterImpl implements LiveMeasureTreeUpdater { IssueCounter issueCounter = new IssueCounter(dbClient.issueDao().selectIssueGroupsByComponent(dbSession, c, beginningOfLeak), dbClient.issueDao().selectIssueImpactGroupsByComponent(dbSession, c)); for (MeasureUpdateFormula formula : formulaFactory.getFormulas()) { - // use formulas when the leak period is defined, it's a PR, or the formula is not about the leak period - if (useLeakFormulas || !formula.isOnLeak()) { + if (shouldComputeMetric(formula, useLeakFormulas, components.getBranch(), matrix)) { context.change(c, formula); try { formula.compute(context, issueCounter); @@ -105,6 +104,15 @@ public class LiveMeasureTreeUpdaterImpl implements LiveMeasureTreeUpdater { }); } + private static boolean shouldComputeMetric(MeasureUpdateFormula formula, boolean useLeakFormulas, ComponentDto branchComponent, + MeasureMatrix matrix) { + // Use formula when the leak period is defined, it's a PR, or the formula is not about the leak period + return (useLeakFormulas || !formula.isOnLeak()) + // Some metrics should only be computed if the metric has been computed on the branch before (during analysis). + // Otherwise, the computed measure would only apply to the touched components and be incomplete. + && (!formula.isOnlyIfComputedOnBranch() || matrix.getMeasure(branchComponent, formula.getMetric().getKey()).isPresent()); + } + private static long getBeginningOfLeakPeriod(SnapshotDto lastAnalysis, BranchDto branch) { if (isPR(branch)) { return 0L; diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/MeasureUpdateFormula.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/MeasureUpdateFormula.java index e00d5c2948b..6ff161f49de 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/MeasureUpdateFormula.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/MeasureUpdateFormula.java @@ -34,6 +34,7 @@ class MeasureUpdateFormula { private final Metric metric; private final boolean onLeak; + private final boolean onlyIfComputedOnBranch; private final BiConsumer hierarchyFormula; private final BiConsumer formula; private final Collection dependentMetrics; @@ -44,13 +45,19 @@ class MeasureUpdateFormula { * @param formula Used to calculate new values for a metric for each component, based on the issue counts */ MeasureUpdateFormula(Metric metric, boolean onLeak, BiConsumer hierarchyFormula, BiConsumer formula) { - this(metric, onLeak, hierarchyFormula, formula, emptyList()); + this(metric, onLeak, false, hierarchyFormula, formula, emptyList()); } - MeasureUpdateFormula(Metric metric, boolean onLeak, BiConsumer hierarchyFormula, BiConsumer formula, - Collection dependentMetrics) { + MeasureUpdateFormula(Metric metric, boolean onLeak, boolean onlyIfComputedOnBranch, BiConsumer hierarchyFormula, + BiConsumer formula) { + this(metric, onLeak, onlyIfComputedOnBranch, hierarchyFormula, formula, emptyList()); + } + + MeasureUpdateFormula(Metric metric, boolean onLeak, boolean onlyIfComputedOnBranch, BiConsumer hierarchyFormula, + BiConsumer formula, Collection dependentMetrics) { this.metric = metric; this.onLeak = onLeak; + this.onlyIfComputedOnBranch = onlyIfComputedOnBranch; this.hierarchyFormula = hierarchyFormula; this.formula = formula; this.dependentMetrics = dependentMetrics; @@ -64,6 +71,10 @@ class MeasureUpdateFormula { return onLeak; } + public boolean isOnlyIfComputedOnBranch() { + return onlyIfComputedOnBranch; + } + Collection getDependentMetrics() { return dependentMetrics; } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/MeasureUpdateFormulaFactoryImpl.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/MeasureUpdateFormulaFactoryImpl.java index df159b8fcb1..4b877d8c535 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/MeasureUpdateFormulaFactoryImpl.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/MeasureUpdateFormulaFactoryImpl.java @@ -65,13 +65,13 @@ public class MeasureUpdateFormulaFactoryImpl implements MeasureUpdateFormulaFact new MeasureUpdateFormula(CoreMetrics.SECURITY_HOTSPOTS, false, new AddChildren(), (context, issues) -> context.setValue(issues.countUnresolvedByType(RuleType.SECURITY_HOTSPOT, false))), - new MeasureUpdateFormula(CoreMetrics.RELIABILITY_ISSUES, false, new ImpactAddChildren(), + new MeasureUpdateFormula(CoreMetrics.RELIABILITY_ISSUES, false, true, new ImpactAddChildren(), (context, issues) -> context.setValue(issues.getBySoftwareQuality(SoftwareQuality.RELIABILITY))), - new MeasureUpdateFormula(CoreMetrics.MAINTAINABILITY_ISSUES, false, new ImpactAddChildren(), + new MeasureUpdateFormula(CoreMetrics.MAINTAINABILITY_ISSUES, false, true, new ImpactAddChildren(), (context, issues) -> context.setValue(issues.getBySoftwareQuality(SoftwareQuality.MAINTAINABILITY))), - new MeasureUpdateFormula(CoreMetrics.SECURITY_ISSUES, false, new ImpactAddChildren(), + new MeasureUpdateFormula(CoreMetrics.SECURITY_ISSUES, false, true, new ImpactAddChildren(), (context, issues) -> context.setValue(issues.getBySoftwareQuality(SoftwareQuality.SECURITY))), new MeasureUpdateFormula(CoreMetrics.VIOLATIONS, false, new AddChildren(), @@ -98,7 +98,7 @@ public class MeasureUpdateFormulaFactoryImpl implements MeasureUpdateFormulaFact new MeasureUpdateFormula(CoreMetrics.ACCEPTED_ISSUES, false, new AddChildren(), (context, issues) -> context.setValue(issues.countByResolution(Issue.RESOLUTION_WONT_FIX, false))), - new MeasureUpdateFormula(CoreMetrics.HIGH_IMPACT_ACCEPTED_ISSUES, false, new AddChildren(), + new MeasureUpdateFormula(CoreMetrics.HIGH_IMPACT_ACCEPTED_ISSUES, false, true, new AddChildren(), (context, issues) -> context.setValue(issues.countHighImpactAccepted(false))), new MeasureUpdateFormula(CoreMetrics.OPEN_ISSUES, false, new AddChildren(), @@ -119,17 +119,17 @@ public class MeasureUpdateFormulaFactoryImpl implements MeasureUpdateFormulaFact new MeasureUpdateFormula(CoreMetrics.SECURITY_REMEDIATION_EFFORT, false, new AddChildren(), (context, issues) -> context.setValue(issues.sumEffortOfUnresolved(RuleType.VULNERABILITY, false))), - new MeasureUpdateFormula(CoreMetrics.SQALE_DEBT_RATIO, false, + new MeasureUpdateFormula(CoreMetrics.SQALE_DEBT_RATIO, false, false, (context, formula) -> context.setValue(100.0 * debtDensity(context)), (context, issues) -> context.setValue(100.0 * debtDensity(context)), asList(CoreMetrics.TECHNICAL_DEBT, CoreMetrics.DEVELOPMENT_COST)), - new MeasureUpdateFormula(CoreMetrics.SQALE_RATING, false, + new MeasureUpdateFormula(CoreMetrics.SQALE_RATING, false, false, (context, issues) -> context.setValue(context.getDebtRatingGrid().getRatingForDensity(debtDensity(context))), (context, issues) -> context.setValue(context.getDebtRatingGrid().getRatingForDensity(debtDensity(context))), asList(CoreMetrics.TECHNICAL_DEBT, CoreMetrics.DEVELOPMENT_COST)), - new MeasureUpdateFormula(CoreMetrics.EFFORT_TO_REACH_MAINTAINABILITY_RATING_A, false, + new MeasureUpdateFormula(CoreMetrics.EFFORT_TO_REACH_MAINTAINABILITY_RATING_A, false, false, (context, formula) -> context.setValue(effortToReachMaintainabilityRatingA(context)), (context, issues) -> context.setValue(effortToReachMaintainabilityRatingA(context)), asList(CoreMetrics.TECHNICAL_DEBT, CoreMetrics.DEVELOPMENT_COST)), @@ -194,7 +194,7 @@ public class MeasureUpdateFormulaFactoryImpl implements MeasureUpdateFormulaFact new MeasureUpdateFormula(CoreMetrics.NEW_INFO_VIOLATIONS, true, new AddChildren(), (context, issues) -> context.setValue(issues.countUnresolvedBySeverity(Severity.INFO, true))), - new MeasureUpdateFormula(CoreMetrics.NEW_ACCEPTED_ISSUES, true, new AddChildren(), + new MeasureUpdateFormula(CoreMetrics.NEW_ACCEPTED_ISSUES, true, true, new AddChildren(), (context, issues) -> context.setValue(issues.countByResolution(Issue.RESOLUTION_WONT_FIX, true))), new MeasureUpdateFormula(CoreMetrics.NEW_TECHNICAL_DEBT, true, new AddChildren(), @@ -243,12 +243,12 @@ public class MeasureUpdateFormulaFactoryImpl implements MeasureUpdateFormulaFact context.setValue(computeRating(percent.orElse(null))); }), - new MeasureUpdateFormula(CoreMetrics.NEW_SQALE_DEBT_RATIO, true, + new MeasureUpdateFormula(CoreMetrics.NEW_SQALE_DEBT_RATIO, true, false, (context, formula) -> context.setValue(100.0D * newDebtDensity(context)), (context, issues) -> context.setValue(100.0D * newDebtDensity(context)), asList(CoreMetrics.NEW_TECHNICAL_DEBT, CoreMetrics.NEW_DEVELOPMENT_COST)), - new MeasureUpdateFormula(CoreMetrics.NEW_MAINTAINABILITY_RATING, true, + new MeasureUpdateFormula(CoreMetrics.NEW_MAINTAINABILITY_RATING, true, false, (context, formula) -> context.setValue(context.getDebtRatingGrid().getRatingForDensity(newDebtDensity(context))), (context, issues) -> context.setValue(context.getDebtRatingGrid().getRatingForDensity(newDebtDensity(context))), asList(CoreMetrics.NEW_TECHNICAL_DEBT, CoreMetrics.NEW_DEVELOPMENT_COST))); -- cgit v1.2.3