diff options
author | Jacek <jacek.poreda@sonarsource.com> | 2022-06-02 15:29:08 +0200 |
---|---|---|
committer | sonartech <sonartech@sonarsource.com> | 2022-06-03 20:03:04 +0000 |
commit | 83ae2064eca6777fe0247625d0ce8f4346a99d0f (patch) | |
tree | 8029a7f2fd2cea262b5e03c74a5b4d44f746f23c /server | |
parent | e6b8b9337e494224c278ab53efdd7722cedad809 (diff) | |
download | sonarqube-83ae2064eca6777fe0247625d0ce8f4346a99d0f.tar.gz sonarqube-83ae2064eca6777fe0247625d0ce8f4346a99d0f.zip |
SONAR-16388 Fix live measure computation on reference branch
Diffstat (limited to 'server')
5 files changed, 140 insertions, 15 deletions
diff --git a/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java b/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java index 710e707f9bd..52a73f6e1ab 100644 --- a/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java +++ b/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java @@ -61,6 +61,7 @@ public final class SqTables { "live_measures", "metrics", "new_code_periods", + "new_code_reference_issues", "notifications", "org_qprofiles", "permission_templates", diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml index f3a69e4aff0..5851f05ab2c 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml @@ -348,9 +348,16 @@ </select> <select id="selectIssueGroupsByBaseComponent" resultType="org.sonar.db.issue.IssueGroupDto" parameterType="map"> - select i.issue_type as ruleType, i.severity as severity, i.resolution as resolution, i.status as status, sum(i.effort) as effort, count(i.issue_type) as "count", (i.issue_creation_date > #{leakPeriodBeginningDate,jdbcType=BIGINT}) as inLeak + select i.issue_type as ruleType, i.severity as severity, i.resolution as resolution, i.status as status, sum(i.effort) as effort, count(i.issue_type) as "count", + <if test="leakPeriodBeginningDate >= 0"> + (i.issue_creation_date > #{leakPeriodBeginningDate,jdbcType=BIGINT}) as inLeak + </if> + <if test="leakPeriodBeginningDate < 0"> + CASE WHEN n.uuid is null THEN false ELSE true END as inLeak + </if> from issues i inner join components p on p.uuid = i.component_uuid and p.project_uuid = i.project_uuid + left join new_code_reference_issues n on n.issue_key = i.kee where i.status !='CLOSED' and i.project_uuid = #{baseComponent.projectUuid,jdbcType=VARCHAR} and (p.uuid_path like #{baseComponent.uuidPathLikeIncludingSelf,jdbcType=VARCHAR} escape '/' or p.uuid = #{baseComponent.uuid,jdbcType=VARCHAR}) @@ -360,9 +367,16 @@ <select id="selectIssueGroupsByBaseComponent" resultType="org.sonar.db.issue.IssueGroupDto" parameterType="map" databaseId="oracle"> select i2.issue_type as ruleType, i2.severity as severity, i2.resolution as resolution, i2.status as status, sum(i2.effort) as effort, count(i2.issue_type) as "count", i2.inLeak as inLeak from ( - select i.issue_type, i.severity, i.resolution, i.status, i.effort, case when i.issue_creation_date > #{leakPeriodBeginningDate,jdbcType=BIGINT} then 1 else 0 end as inLeak + select i.issue_type, i.severity, i.resolution, i.status, i.effort, + <if test="leakPeriodBeginningDate >= 0"> + case when i.issue_creation_date > #{leakPeriodBeginningDate,jdbcType=BIGINT} then 1 else 0 end as inLeak + </if> + <if test="leakPeriodBeginningDate < 0"> + case when n.uuid is null then 0 else 1 end as inLeak + </if> from issues i inner join components p on p.uuid = i.component_uuid and p.project_uuid = i.project_uuid + left join new_code_reference_issues n on n.issue_key = i.kee where i.status !='CLOSED' and i.project_uuid = #{baseComponent.projectUuid,jdbcType=VARCHAR} and (p.uuid_path like #{baseComponent.uuidPathLikeIncludingSelf,jdbcType=VARCHAR} escape '/' or p.uuid = #{baseComponent.uuid,jdbcType=VARCHAR}) @@ -373,9 +387,16 @@ <select id="selectIssueGroupsByBaseComponent" resultType="org.sonar.db.issue.IssueGroupDto" parameterType="map" databaseId="mssql"> select i2.issue_type as ruleType, i2.severity as severity, i2.resolution as resolution, i2.status as status, sum(i2.effort) as effort, count(i2.issue_type) as "count", i2.inLeak as inLeak from ( - select i.issue_type, i.severity, i.resolution, i.status, i.effort, case when i.issue_creation_date > #{leakPeriodBeginningDate,jdbcType=BIGINT} then 1 else 0 end as inLeak + select i.issue_type, i.severity, i.resolution, i.status, i.effort, + <if test="leakPeriodBeginningDate >= 0"> + case when i.issue_creation_date > #{leakPeriodBeginningDate,jdbcType=BIGINT} then 1 else 0 end as inLeak + </if> + <if test="leakPeriodBeginningDate < 0"> + case when n.uuid is null then 0 else 1 end as inLeak + </if> from issues i inner join components p on p.uuid = i.component_uuid and p.project_uuid = i.project_uuid + left join new_code_reference_issues n on n.issue_key = i.kee where i.status !='CLOSED' and i.project_uuid = #{baseComponent.projectUuid,jdbcType=VARCHAR} and (p.uuid_path like #{baseComponent.uuidPathLikeIncludingSelf,jdbcType=VARCHAR} escape '/' or p.uuid = #{baseComponent.uuid,jdbcType=VARCHAR}) diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDaoTest.java index 19a4f58d9e2..393798a7012 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDaoTest.java @@ -351,6 +351,49 @@ public class IssueDaoTest { } @Test + public void selectGroupsOfComponentTreeOnLeak_on_file_new_code_reference_branch() { + ComponentDto project = db.components().insertPublicProject(); + ComponentDto file = db.components().insertComponent(ComponentTesting.newFileDto(project)); + RuleDto rule = db.rules().insert(); + IssueDto fpBug = db.issues().insert(rule, project, file, + i -> i.setStatus("RESOLVED").setResolution("FALSE-POSITIVE").setSeverity("MAJOR").setType(RuleType.BUG)); + IssueDto criticalBug1 = db.issues().insert(rule, project, file, + i -> i.setStatus("OPEN").setResolution(null).setSeverity("CRITICAL").setType(RuleType.BUG)); + IssueDto criticalBug2 = db.issues().insert(rule, project, file, + i -> i.setStatus("OPEN").setResolution(null).setSeverity("CRITICAL").setType(RuleType.BUG)); + + db.issues().insert(rule, project, file, + i -> i.setStatus("OPEN").setResolution(null).setSeverity("CRITICAL").setType(RuleType.BUG)); + + //two issues part of new code period on reference branch + db.issues().insertNewCodeReferenceIssue(fpBug); + db.issues().insertNewCodeReferenceIssue(criticalBug1); + db.issues().insertNewCodeReferenceIssue(criticalBug2); + + Collection<IssueGroupDto> result = underTest.selectIssueGroupsByBaseComponent(db.getSession(), file, -1); + + assertThat(result.stream().mapToLong(IssueGroupDto::getCount).sum()).isEqualTo(4); + + assertThat(result.stream().filter(g -> g.getRuleType() == RuleType.BUG.getDbConstant()).mapToLong(IssueGroupDto::getCount).sum()).isEqualTo(4); + assertThat(result.stream().filter(g -> g.getRuleType() == RuleType.CODE_SMELL.getDbConstant()).mapToLong(IssueGroupDto::getCount).sum()).isZero(); + assertThat(result.stream().filter(g -> g.getRuleType() == RuleType.VULNERABILITY.getDbConstant()).mapToLong(IssueGroupDto::getCount).sum()).isZero(); + + assertThat(result.stream().filter(g -> g.getSeverity().equals("CRITICAL")).mapToLong(IssueGroupDto::getCount).sum()).isEqualTo(3); + assertThat(result.stream().filter(g -> g.getSeverity().equals("MAJOR")).mapToLong(IssueGroupDto::getCount).sum()).isOne(); + assertThat(result.stream().filter(g -> g.getSeverity().equals("MINOR")).mapToLong(IssueGroupDto::getCount).sum()).isZero(); + + assertThat(result.stream().filter(g -> g.getStatus().equals("OPEN")).mapToLong(IssueGroupDto::getCount).sum()).isEqualTo(3); + assertThat(result.stream().filter(g -> g.getStatus().equals("RESOLVED")).mapToLong(IssueGroupDto::getCount).sum()).isOne(); + assertThat(result.stream().filter(g -> g.getStatus().equals("CLOSED")).mapToLong(IssueGroupDto::getCount).sum()).isZero(); + + assertThat(result.stream().filter(g -> "FALSE-POSITIVE".equals(g.getResolution())).mapToLong(IssueGroupDto::getCount).sum()).isOne(); + assertThat(result.stream().filter(g -> g.getResolution() == null).mapToLong(IssueGroupDto::getCount).sum()).isEqualTo(3); + + assertThat(result.stream().filter(IssueGroupDto::isInLeak).mapToLong(IssueGroupDto::getCount).sum()).isEqualTo(3); + assertThat(result.stream().filter(g -> !g.isInLeak()).mapToLong(IssueGroupDto::getCount).sum()).isOne(); + } + + @Test public void selectModuleAndDirComponentUuidsOfOpenIssuesForProjectUuid() { assertThat(underTest.selectModuleAndDirComponentUuidsOfOpenIssuesForProjectUuid(db.getSession(), randomAlphabetic(12))) .isEmpty(); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/LiveMeasureComputerImpl.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/LiveMeasureComputerImpl.java index 6f25080ba76..b7b024a62ef 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/LiveMeasureComputerImpl.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/measure/live/LiveMeasureComputerImpl.java @@ -53,10 +53,12 @@ import org.sonar.server.setting.ProjectConfigurationLoader; import static com.google.common.base.Preconditions.checkState; import static java.util.Collections.emptyList; import static java.util.Collections.singleton; +import static java.util.Optional.ofNullable; import static java.util.stream.Collectors.groupingBy; import static org.sonar.api.measures.CoreMetrics.ALERT_STATUS_KEY; import static org.sonar.core.util.stream.MoreCollectors.toArrayList; import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; +import static org.sonar.db.newcodeperiod.NewCodePeriodType.REFERENCE_BRANCH; public class LiveMeasureComputerImpl implements LiveMeasureComputer { @@ -96,11 +98,13 @@ public class LiveMeasureComputerImpl implements LiveMeasureComputer { ComponentDto branchComponent = findBranchComponent(components); BranchDto branch = loadBranch(dbSession, branchComponent); ProjectDto project = loadProject(dbSession, branch.getProjectUuid()); - Optional<SnapshotDto> lastAnalysis = dbClient.snapshotDao().selectLastAnalysisByRootComponentUuid(dbSession, branchComponent.uuid()); - if (!lastAnalysis.isPresent()) { + Optional<SnapshotDto> lastAnalysisResult = dbClient.snapshotDao().selectLastAnalysisByRootComponentUuid(dbSession, branchComponent.uuid()); + if (lastAnalysisResult.isEmpty()) { return Optional.empty(); } + var lastAnalysis = lastAnalysisResult.get(); + QualityGate qualityGate = qGateComputer.loadQualityGate(dbSession, project, branch); Collection<String> metricKeys = getKeysOfAllInvolvedMetrics(qualityGate); @@ -123,7 +127,7 @@ public class LiveMeasureComputerImpl implements LiveMeasureComputer { IssueCounter issueCounter = new IssueCounter(dbClient.issueDao().selectIssueGroupsByBaseComponent(dbSession, c, beginningOfLeak)); for (IssueMetricFormula 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 (shouldUseLeakFormulas(lastAnalysis.get(), branch) || !formula.isOnLeak()) { + if (shouldUseLeakFormulas(lastAnalysis, branch) || !formula.isOnLeak()) { context.change(c, formula); try { formula.compute(context, issueCounter); @@ -142,16 +146,18 @@ public class LiveMeasureComputerImpl implements LiveMeasureComputer { projectIndexer.commitAndIndexComponents(dbSession, singleton(branchComponent), ProjectIndexer.Cause.MEASURE_CHANGE); return Optional.of( - new QGChangeEvent(project, branch, lastAnalysis.get(), config, previousStatus, () -> Optional.of(evaluatedQualityGate))); + new QGChangeEvent(project, branch, lastAnalysis, config, previousStatus, () -> Optional.of(evaluatedQualityGate))); } - private static long getBeginningOfLeakPeriod(Optional<SnapshotDto> lastAnalysis, BranchDto branch) { + private static long getBeginningOfLeakPeriod(SnapshotDto lastAnalysis, BranchDto branch) { if (isPR(branch)) { return 0L; - } else { - Optional<Long> beginningOfLeakPeriod = lastAnalysis.map(SnapshotDto::getPeriodDate); - return beginningOfLeakPeriod.orElse(Long.MAX_VALUE); + } else if (REFERENCE_BRANCH.name().equals(lastAnalysis.getPeriodMode())) { + return -1; } + return ofNullable(lastAnalysis.getPeriodDate()) + .orElse(Long.MAX_VALUE); + } private static boolean isPR(BranchDto branch) { @@ -159,7 +165,7 @@ public class LiveMeasureComputerImpl implements LiveMeasureComputer { } private static boolean shouldUseLeakFormulas(SnapshotDto lastAnalysis, BranchDto branch) { - return lastAnalysis.getPeriodDate() != null || isPR(branch); + return lastAnalysis.getPeriodDate() != null || isPR(branch) || REFERENCE_BRANCH.name().equals(lastAnalysis.getPeriodMode()); } @CheckForNull diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/measure/live/LiveMeasureComputerImplTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/measure/live/LiveMeasureComputerImplTest.java index 152e4b98555..947ffbf49d4 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/measure/live/LiveMeasureComputerImplTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/measure/live/LiveMeasureComputerImplTest.java @@ -49,6 +49,7 @@ import org.sonar.db.component.ComponentDto; import org.sonar.db.component.ComponentTesting; import org.sonar.db.measure.LiveMeasureDto; import org.sonar.db.metric.MetricDto; +import org.sonar.db.newcodeperiod.NewCodePeriodType; import org.sonar.db.project.ProjectDto; import org.sonar.server.es.ProjectIndexer; import org.sonar.server.es.TestProjectIndexers; @@ -88,6 +89,8 @@ public class LiveMeasureComputerImplTest { private ComponentDto dir; private ComponentDto file1; private ComponentDto file2; + private ComponentDto prBranch; + private ComponentDto prBranchFile; private ComponentDto branch; private ComponentDto branchFile; private final LiveQualityGateComputer qGateComputer = mock(LiveQualityGateComputer.class); @@ -104,7 +107,11 @@ public class LiveMeasureComputerImplTest { dir = db.components().insertComponent(ComponentTesting.newDirectory(project, "src/main/java")); file1 = db.components().insertComponent(ComponentTesting.newFileDto(project, dir)); file2 = db.components().insertComponent(ComponentTesting.newFileDto(project, dir)); - branch = db.components().insertProjectBranch(project, b -> b.setBranchType(BranchType.PULL_REQUEST)); + + prBranch = db.components().insertProjectBranch(project, b -> b.setBranchType(BranchType.PULL_REQUEST)); + prBranchFile = db.components().insertComponent(ComponentTesting.newFileDto(prBranch)); + + branch = db.components().insertProjectBranch(project); branchFile = db.components().insertComponent(ComponentTesting.newFileDto(branch)); } @@ -224,8 +231,50 @@ public class LiveMeasureComputerImplTest { } @Test + public void refresh_after_first_analysis() { + markProjectAsAnalyzed(project, null); + db.measures().insertLiveMeasure(project, intMetric, m -> m.setVariation(null).setValue(42.0)); + db.measures().insertLiveMeasure(project, ratingMetric, m -> m.setValue((double) Rating.E.getIndex()).setData(Rating.E.name())); + db.measures().insertLiveMeasure(dir, intMetric, m -> m.setVariation(null).setValue(42.0)); + db.measures().insertLiveMeasure(dir, ratingMetric, m -> m.setValue((double) Rating.D.getIndex()).setData(Rating.D.name())); + db.measures().insertLiveMeasure(file1, intMetric, m -> m.setVariation(null).setValue(42.0)); + db.measures().insertLiveMeasure(file1, ratingMetric, m -> m.setValue((double) Rating.C.getIndex()).setData(Rating.C.name())); + + List<QGChangeEvent> result = run(file1, newQualifierBasedIntLeakFormula(), newIntConstantFormula(1337)); + + assertThat(db.countRowsOfTable(db.getSession(), "live_measures")).isEqualTo(6); + + assertThatIntMeasureHasValue(file1, 1337); + assertThatRatingMeasureHasValue(file1, Rating.C); + assertThatIntMeasureHasValue(dir, 1337); + assertThatRatingMeasureHasValue(dir, Rating.D); + assertThatIntMeasureHasValue(project, 1337); + assertThatRatingMeasureHasValue(project, Rating.E); + assertThatProjectChanged(result, project); + } + + @Test public void calculate_new_metrics_if_it_is_pr_or_branch() { - markProjectAsAnalyzed(branch, null); + markProjectAsAnalyzed(prBranch, null); + db.measures().insertLiveMeasure(prBranch, intMetric, m -> m.setVariation(42.0).setValue(null)); + db.measures().insertLiveMeasure(prBranchFile, intMetric, m -> m.setVariation(42.0).setValue(null)); + + // generates values 1, 2, 3 on leak measures + List<QGChangeEvent> result = run(prBranchFile, newQualifierBasedIntLeakFormula(), newRatingLeakFormula(Rating.B)); + + assertThat(db.countRowsOfTable(db.getSession(), "live_measures")).isEqualTo(4); + + // Numeric value depends on qualifier (see newQualifierBasedIntLeakFormula()) + assertThatIntMeasureHasLeakValue(prBranchFile, ORDERED_BOTTOM_UP.indexOf(Qualifiers.FILE)); + assertThatRatingMeasureHasLeakValue(prBranchFile, Rating.B); + assertThatIntMeasureHasLeakValue(prBranch, ORDERED_BOTTOM_UP.indexOf(Qualifiers.PROJECT)); + assertThatRatingMeasureHasLeakValue(prBranch, Rating.B); + assertThatProjectChanged(result, prBranch); + } + + @Test + public void calculate_new_metrics_if_it_is_branch_using_new_code_reference() { + markProjectAsAnalyzed(branch, null, NewCodePeriodType.REFERENCE_BRANCH); db.measures().insertLiveMeasure(branch, intMetric, m -> m.setVariation(42.0).setValue(null)); db.measures().insertLiveMeasure(branchFile, intMetric, m -> m.setVariation(42.0).setValue(null)); @@ -411,7 +460,12 @@ public class LiveMeasureComputerImplTest { private void markProjectAsAnalyzed(ComponentDto p, @Nullable Long periodDate) { assertThat(p.qualifier()).isEqualTo(Qualifiers.PROJECT); - db.components().insertSnapshot(p, s -> s.setPeriodDate(periodDate)); + markProjectAsAnalyzed(p, periodDate, null); + } + + private void markProjectAsAnalyzed(ComponentDto p, @Nullable Long periodDate, @Nullable NewCodePeriodType type) { + assertThat(p.qualifier()).isEqualTo(Qualifiers.PROJECT); + db.components().insertSnapshot(p, s -> s.setPeriodDate(periodDate).setPeriodMode(type != null ? type.name() : null)); } private LiveMeasureDto assertThatIntMeasureHasValue(ComponentDto component, double expectedValue) { |