]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-21455 Live update should not recompute measures not existing on branch
authorEric Giffon <eric.giffon@sonarsource.com>
Thu, 25 Jan 2024 15:54:45 +0000 (16:54 +0100)
committersonartech <sonartech@sonarsource.com>
Wed, 31 Jan 2024 20:03:36 +0000 (20:03 +0000)
server/sonar-webserver-webapi/src/it/java/org/sonar/server/measure/live/LiveMeasureTreeUpdaterImplIT.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/LiveMeasureTreeUpdaterImpl.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/MeasureUpdateFormula.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/MeasureUpdateFormulaFactoryImpl.java

index df5064c040240e6615bdced94c71131203ec602e..23fbe967485f54ea720100ebc96c8343017db51f 100644 (file)
@@ -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<LiveMeasureDto> 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<LiveMeasureDto> 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<MeasureUpdateFormula> 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<MeasureUpdateFormula> 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)));
     }
 
index 4ddf4b428299469da187cdc1802676f0134b3686..71eb40833c831ddb8cb53270bdabd75958d704a4 100644 (file)
@@ -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;
index e00d5c2948be7e02c0e0df081b097eecf9367ed9..6ff161f49de56adab351ef18df0b59142ded2a68 100644 (file)
@@ -34,6 +34,7 @@ class MeasureUpdateFormula {
 
   private final Metric metric;
   private final boolean onLeak;
+  private final boolean onlyIfComputedOnBranch;
   private final BiConsumer<Context, MeasureUpdateFormula> hierarchyFormula;
   private final BiConsumer<Context, IssueCounter> formula;
   private final Collection<Metric> 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<Context, MeasureUpdateFormula> hierarchyFormula, BiConsumer<Context, IssueCounter> formula) {
-    this(metric, onLeak, hierarchyFormula, formula, emptyList());
+    this(metric, onLeak, false, hierarchyFormula, formula, emptyList());
   }
 
-  MeasureUpdateFormula(Metric metric, boolean onLeak, BiConsumer<Context, MeasureUpdateFormula> hierarchyFormula, BiConsumer<Context, IssueCounter> formula,
-    Collection<Metric> dependentMetrics) {
+  MeasureUpdateFormula(Metric metric, boolean onLeak, boolean onlyIfComputedOnBranch, BiConsumer<Context, MeasureUpdateFormula> hierarchyFormula,
+                       BiConsumer<Context, IssueCounter> formula) {
+    this(metric, onLeak, onlyIfComputedOnBranch, hierarchyFormula, formula, emptyList());
+  }
+
+  MeasureUpdateFormula(Metric metric, boolean onLeak, boolean onlyIfComputedOnBranch, BiConsumer<Context, MeasureUpdateFormula> hierarchyFormula,
+                       BiConsumer<Context, IssueCounter> formula, Collection<Metric> 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<Metric> getDependentMetrics() {
     return dependentMetrics;
   }
index df159b8fcb16800e07a532da4afb9d8f5f5b4de3..4b877d8c53536f21c1141382e71c2a5f2715de71 100644 (file)
@@ -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)));