From 12af365eaa3cf93a561305fe99897dae87e0626f Mon Sep 17 00:00:00 2001 From: Eric Giffon Date: Wed, 9 Oct 2024 10:17:13 +0200 Subject: [PATCH] SONAR-23213 Measures double write - live update --- .../org/sonar/db/component/BranchDao.java | 4 +- .../org/sonar/db/measure/JsonMeasureDao.java | 22 +++++ .../org/sonar/db/component/BranchDaoTest.java | 23 +++++ .../sonar/db/measure/JsonMeasureDaoTest.java | 54 +++++++++++ .../measure/live/LiveMeasureComputerImpl.java | 53 +++++++++-- .../live/LiveMeasureComputerImplTest.java | 90 +++++++++++++++++-- 6 files changed, 232 insertions(+), 14 deletions(-) diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/component/BranchDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/component/BranchDao.java index 402e09abe5f..d7e24c7aa54 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/component/BranchDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/component/BranchDao.java @@ -19,7 +19,6 @@ */ package org.sonar.db.component; -import com.google.common.annotations.VisibleForTesting; import java.util.Collection; import java.util.LinkedList; import java.util.List; @@ -150,8 +149,7 @@ public class BranchDao implements Dao { return mapper(dbSession).updateMeasuresMigrated(branchUuid, measuresMigrated, now); } - @VisibleForTesting - boolean isMeasuresMigrated(DbSession dbSession, String uuid) { + public boolean isMeasuresMigrated(DbSession dbSession, String uuid) { return mapper(dbSession).isMeasuresMigrated(uuid); } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/measure/JsonMeasureDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/measure/JsonMeasureDao.java index 49b9aa77b8b..d760cd38862 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/measure/JsonMeasureDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/measure/JsonMeasureDao.java @@ -37,10 +37,32 @@ public class JsonMeasureDao implements Dao { return mapper(dbSession).insert(dto, system2.now()); } + /** + * Update a measure. The measure json value will be overwritten. + */ public int update(DbSession dbSession, JsonMeasureDto dto) { return mapper(dbSession).update(dto, system2.now()); } + /** + * Unlike {@link #update(DbSession, JsonMeasureDto)}, this method will not overwrite the entire json value, + * but will update the measures inside the json. + */ + public int insertOrUpdate(DbSession dbSession, JsonMeasureDto dto) { + long now = system2.now(); + Optional existingMeasureOpt = selectByComponentUuid(dbSession, dto.getComponentUuid()); + if (existingMeasureOpt.isPresent()) { + JsonMeasureDto existingDto = existingMeasureOpt.get(); + existingDto.getMetricValues().putAll(dto.getMetricValues()); + dto.getMetricValues().putAll(existingDto.getMetricValues()); + dto.computeJsonValueHash(); + return mapper(dbSession).update(dto, now); + } else { + dto.computeJsonValueHash(); + return mapper(dbSession).insert(dto, now); + } + } + public Optional selectByComponentUuid(DbSession dbSession, String componentUuid) { return Optional.ofNullable(mapper(dbSession).selectByComponentUuid(componentUuid)); } diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/component/BranchDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/component/BranchDaoTest.java index f11f90f271c..5c374541c90 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/component/BranchDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/component/BranchDaoTest.java @@ -712,6 +712,29 @@ public class BranchDaoTest { assertThat(underTest.isMeasuresMigrated(dbSession, uuid2)).isFalse(); } + @Test + public void isMeasuresMigrated() throws SQLException { + createMeasuresMigratedColumn(); + + // master branch with flag set to false + ComponentDto project = db.components().insertPrivateProject(); + // branches & PRs + ComponentDto branch1 = db.components().insertProjectBranch(project, b -> b.setBranchType(BRANCH)); + ComponentDto branch2 = db.components().insertProjectBranch(project, b -> b.setBranchType(BranchType.BRANCH)); + ComponentDto branch3 = db.components().insertProjectBranch(project, b -> b.setBranchType(BranchType.PULL_REQUEST)); + ComponentDto branch4 = db.components().insertProjectBranch(project, b -> b.setBranchType(BranchType.PULL_REQUEST)); + + db.getDbClient().branchDao().updateMeasuresMigrated(dbSession, branch1.branchUuid(), true); + db.getDbClient().branchDao().updateMeasuresMigrated(dbSession, branch2.branchUuid(), false); + db.getDbClient().branchDao().updateMeasuresMigrated(dbSession, branch3.branchUuid(), true); + + assertThat(underTest.isMeasuresMigrated(dbSession, project.uuid())).isFalse(); + assertThat(underTest.isMeasuresMigrated(dbSession, branch1.uuid())).isTrue(); + assertThat(underTest.isMeasuresMigrated(dbSession, branch2.uuid())).isFalse(); + assertThat(underTest.isMeasuresMigrated(dbSession, branch3.uuid())).isTrue(); + assertThat(underTest.isMeasuresMigrated(dbSession, branch4.uuid())).isFalse(); + } + private void createMeasuresMigratedColumn() throws SQLException { AddMeasuresMigratedColumnToProjectBranchesTable migration = new AddMeasuresMigratedColumnToProjectBranchesTable(db.getDbClient().getDatabase()); migration.execute(); diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/measure/JsonMeasureDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/measure/JsonMeasureDaoTest.java index f0b6d913c6b..3841aeeef9b 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/measure/JsonMeasureDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/measure/JsonMeasureDaoTest.java @@ -30,6 +30,7 @@ import org.sonar.server.platform.db.migration.adhoc.CreateIndexOnPortfoliosMeasu import org.sonar.server.platform.db.migration.adhoc.CreateIndexOnProjectBranchesMeasuresMigrated; import org.sonar.server.platform.db.migration.adhoc.CreateMeasuresTable; +import static java.util.Map.entry; import static org.assertj.core.api.Assertions.assertThat; import static org.sonar.db.measure.MeasureTesting.newJsonMeasure; @@ -72,6 +73,59 @@ public class JsonMeasureDaoTest { verifyPersisted(dto); } + @Test + public void insertOrUpdate_inserts_or_updates_measure() { + // insert + JsonMeasureDto dto = newJsonMeasure(); + int count = underTest.insertOrUpdate(db.getSession(), dto); + assertThat(count).isEqualTo(1); + verifyTableSize(1); + verifyPersisted(dto); + + // update + String key = dto.getMetricValues().keySet().stream().findFirst().orElseThrow(); + dto.addValue(key, 10d); + count = underTest.insertOrUpdate(db.getSession(), dto); + assertThat(count).isEqualTo(1); + verifyTableSize(1); + verifyPersisted(dto); + } + + @Test + public void insertOrUpdate_merges_measures() { + // insert + Double value2 = 10d; + JsonMeasureDto dto = newJsonMeasure(); + dto.getMetricValues().clear(); + dto.addValue("key1", 11d) + .addValue("key2", value2); + int count = underTest.insert(db.getSession(), dto); + verifyPersisted(dto); + verifyTableSize(1); + assertThat(count).isEqualTo(1); + + // update key1 value, remove key2 (must not disappear from DB) and add key3 + Double value1 = 12d; + Double value3 = 13d; + dto.addValue("key1", value1) + .addValue("key3", value3) + .getMetricValues().remove("key2"); + count = underTest.insertOrUpdate(db.getSession(), dto); + assertThat(count).isEqualTo(1); + verifyTableSize(1); + + assertThat(underTest.selectByComponentUuid(db.getSession(), dto.getComponentUuid())) + .hasValueSatisfying(selected -> { + assertThat(selected.getComponentUuid()).isEqualTo(dto.getComponentUuid()); + assertThat(selected.getBranchUuid()).isEqualTo(dto.getBranchUuid()); + assertThat(selected.getMetricValues()).contains( + entry("key1", value1), + entry("key2", value2), + entry("key3", value3)); + assertThat(selected.getJsonValueHash()).isEqualTo(dto.computeJsonValueHash()); + }); + } + @Test public void select_measure() { JsonMeasureDto measure1 = newJsonMeasure(); 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 1686d945afc..4448d99ba66 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 @@ -21,6 +21,7 @@ package org.sonar.server.measure.live; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -35,10 +36,12 @@ import org.sonar.db.DbSession; import org.sonar.db.component.BranchDto; import org.sonar.db.component.ComponentDto; import org.sonar.db.component.SnapshotDto; +import org.sonar.db.measure.JsonMeasureDto; import org.sonar.db.measure.LiveMeasureComparator; import org.sonar.db.measure.LiveMeasureDto; import org.sonar.db.metric.MetricDto; import org.sonar.db.project.ProjectDto; +import org.sonar.db.property.PropertyDto; import org.sonar.server.es.ProjectIndexer; import org.sonar.server.es.ProjectIndexers; import org.sonar.server.qualitygate.EvaluatedQualityGate; @@ -50,10 +53,13 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singleton; import static java.util.stream.Collectors.groupingBy; import static org.sonar.api.measures.CoreMetrics.ALERT_STATUS_KEY; +import static org.sonar.core.config.CorePropertyDefinitions.SYSTEM_MEASURES_MIGRATION_ENABLED; import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; public class LiveMeasureComputerImpl implements LiveMeasureComputer { + private static final Set TEXT_VALUE_TYPES = Set.of("STRING", "LEVEL", "DATA", "DISTRIB"); + private final DbClient dbClient; private final MeasureUpdateFormulaFactory formulaFactory; private final ComponentIndexFactory componentIndexFactory; @@ -100,30 +106,65 @@ public class LiveMeasureComputerImpl implements LiveMeasureComputer { Configuration config = projectConfigurationLoader.loadProjectConfiguration(dbSession, branchComponent); ProjectDto project = loadProject(dbSession, branch.getProjectUuid()); QualityGate qualityGate = qGateComputer.loadQualityGate(dbSession, project, branch); - MeasureMatrix matrix = loadMeasureMatrix(dbSession, components.getAllUuids(), qualityGate); + Collection metricKeys = getKeysOfAllInvolvedMetrics(qualityGate); + Map metricsPerUuid = dbClient.metricDao().selectByKeys(dbSession, metricKeys).stream().collect(uniqueIndex(MetricDto::getUuid)); + MeasureMatrix matrix = loadMeasureMatrix(dbSession, components.getAllUuids(), metricsPerUuid); treeUpdater.update(dbSession, lastAnalysis.get(), config, components, branch, matrix); Metric.Level previousStatus = loadPreviousStatus(dbSession, branchComponent); EvaluatedQualityGate evaluatedQualityGate = qGateComputer.refreshGateStatus(branchComponent, qualityGate, matrix, config); - persistAndIndex(dbSession, matrix, branchComponent); + persistAndIndex(dbSession, matrix, branchComponent, metricsPerUuid); return Optional.of(new QGChangeEvent(project, branch, lastAnalysis.get(), config, previousStatus, () -> Optional.of(evaluatedQualityGate))); } - private MeasureMatrix loadMeasureMatrix(DbSession dbSession, Set componentUuids, QualityGate qualityGate) { - Collection metricKeys = getKeysOfAllInvolvedMetrics(qualityGate); - Map metricsPerUuid = dbClient.metricDao().selectByKeys(dbSession, metricKeys).stream().collect(uniqueIndex(MetricDto::getUuid)); + private MeasureMatrix loadMeasureMatrix(DbSession dbSession, Set componentUuids, Map metricsPerUuid) { List measures = dbClient.liveMeasureDao().selectByComponentUuidsAndMetricUuids(dbSession, componentUuids, metricsPerUuid.keySet()); return new MeasureMatrix(componentUuids, metricsPerUuid.values(), measures); } - private void persistAndIndex(DbSession dbSession, MeasureMatrix matrix, ComponentDto branchComponent) { + private void persistAndIndex(DbSession dbSession, MeasureMatrix matrix, ComponentDto branchComponent, + Map metricsPerUuid) { // persist the measures that have been created or updated matrix.getChanged().sorted(LiveMeasureComparator.INSTANCE).forEach(m -> dbClient.liveMeasureDao().insertOrUpdate(dbSession, m)); + + if (isMeasuresMigrationEnabled() && dbClient.branchDao().isMeasuresMigrated(dbSession, branchComponent.uuid())) { + Map measureDtoPerComponent = new HashMap<>(); + matrix.getChanged().sorted(LiveMeasureComparator.INSTANCE).forEach(m -> { + MetricDto metricDto = metricsPerUuid.get(m.getMetricUuid()); + Object metricValue = getMetricValue(m, metricDto.getValueType()); + if (metricValue != null) { + measureDtoPerComponent.compute(m.getComponentUuid(), (componentUuid, measureDto) -> { + if (measureDto == null) { + measureDto = new JsonMeasureDto() + .setComponentUuid(componentUuid) + .setBranchUuid(m.getProjectUuid()); + } + return measureDto.addValue( + metricDto.getKey(), + metricValue + ); + }); + } + }); + measureDtoPerComponent.values().forEach(m -> dbClient.jsonMeasureDao().insertOrUpdate(dbSession, m)); + } + projectIndexer.commitAndIndexComponents(dbSession, singleton(branchComponent), ProjectIndexer.Cause.MEASURE_CHANGE); } + private boolean isMeasuresMigrationEnabled() { + return Optional.ofNullable(dbClient.propertiesDao().selectGlobalProperty(SYSTEM_MEASURES_MIGRATION_ENABLED)) + .map(PropertyDto::getValue) + .map(Boolean::valueOf) + .orElse(false); + } + + private static Object getMetricValue(LiveMeasureDto measure, String valueType) { + return TEXT_VALUE_TYPES.contains(valueType) ? measure.getDataAsString() : measure.getValue(); + } + @CheckForNull private Metric.Level loadPreviousStatus(DbSession dbSession, ComponentDto branchComponent) { Optional measure = dbClient.liveMeasureDao().selectMeasure(dbSession, branchComponent.uuid(), ALERT_STATUS_KEY); 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 78848e3f9dc..555775b8870 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 @@ -19,7 +19,9 @@ */ package org.sonar.server.measure.live; +import com.google.common.collect.ImmutableMap; import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import java.sql.SQLException; import java.util.List; import java.util.Set; import javax.annotation.Nullable; @@ -41,12 +43,15 @@ import org.sonar.db.component.ComponentDto; import org.sonar.db.component.SnapshotDto; import org.sonar.db.metric.MetricDto; import org.sonar.server.es.TestProjectIndexers; +import org.sonar.server.platform.db.migration.adhoc.AddMeasuresMigratedColumnToProjectBranchesTable; +import org.sonar.server.platform.db.migration.adhoc.CreateMeasuresTable; import org.sonar.server.qualitygate.EvaluatedQualityGate; import org.sonar.server.qualitygate.QualityGate; import org.sonar.server.qualitygate.changeevent.QGChangeEvent; import org.sonar.server.setting.ProjectConfigurationLoader; import org.sonar.server.setting.TestProjectConfigurationLoader; +import static java.util.Map.entry; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -54,6 +59,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.sonar.api.measures.CoreMetrics.ALERT_STATUS_KEY; +import static org.sonar.core.config.CorePropertyDefinitions.SYSTEM_MEASURES_MIGRATION_ENABLED; @RunWith(DataProviderRunner.class) public class LiveMeasureComputerImplTest { @@ -80,13 +86,18 @@ public class LiveMeasureComputerImplTest { private BranchDto branch; @Before - public void setUp() { - metric1 = db.measures().insertMetric(); - metric2 = db.measures().insertMetric(); + public void setUp() throws SQLException { + createMeasuresTable(); + createMeasuresMigratedColumn(); + + metric1 = db.measures().insertMetric(m -> m.setValueType("INT")); + metric2 = db.measures().insertMetric(m -> m.setValueType("INT")); project = db.components().insertPublicProject(); branch = db.getDbClient().branchDao().selectByUuid(db.getSession(), project.uuid()).get(); + db.getDbClient().branchDao().updateMeasuresMigrated(db.getSession(), branch.getUuid(), true); db.measures().insertLiveMeasure(project, metric2, lm -> lm.setValue(1d)); + db.measures().insertJsonMeasure(project, m -> m.addValue(metric2.getKey(), 1d)); when(componentIndexFactory.create(any(), any())).thenReturn(componentIndex); when(measureUpdateFormulaFactory.getFormulaMetrics()).thenReturn(Set.of(toMetric(metric1), toMetric(metric2))); @@ -95,7 +106,9 @@ public class LiveMeasureComputerImplTest { @Test public void loads_measure_matrix_and_calls_tree_updater() { - SnapshotDto snapshot = markProjectAsAnalyzed(project); + db.properties().insertProperty(SYSTEM_MEASURES_MIGRATION_ENABLED, "true", null); + + markProjectAsAnalyzed(project); when(componentIndex.getAllUuids()).thenReturn(Set.of(project.uuid())); liveMeasureComputer.refresh(db.getSession(), List.of(project)); @@ -108,7 +121,64 @@ public class LiveMeasureComputerImplTest { assertThat(treeUpdater.getMeasureMatrix().getMeasure(project, metric2.getKey()).get().getValue()).isEqualTo(1d); // new measures were persisted - assertThat(db.getDbClient().liveMeasureDao().selectMeasure(db.getSession(), project.uuid(), metric1.getKey()).get().getValue()).isEqualTo(2d); + assertThat(db.getDbClient().liveMeasureDao().selectMeasure(db.getSession(), project.uuid(), metric1.getKey())) + .hasValueSatisfying(m -> assertThat(m.getValue()).isEqualTo(2d)); + assertThat(db.getDbClient().liveMeasureDao().selectMeasure(db.getSession(), project.uuid(), metric2.getKey())) + .hasValueSatisfying(m -> assertThat(m.getValue()).isEqualTo(1d)); + assertThat(db.getDbClient().jsonMeasureDao().selectByComponentUuid(db.getSession(), project.uuid())) + .hasValueSatisfying(m -> assertThat(m.getMetricValues()).containsExactlyInAnyOrderEntriesOf( + ImmutableMap.of(metric1.getKey(), 2d, metric2.getKey(), 1d))); + } + + @Test + public void no_json_measure_update_when_migration_disabled() { + db.properties().insertProperty(SYSTEM_MEASURES_MIGRATION_ENABLED, "false", null); + + markProjectAsAnalyzed(project); + when(componentIndex.getAllUuids()).thenReturn(Set.of(project.uuid())); + + liveMeasureComputer.refresh(db.getSession(), List.of(project)); + + // tree updater was called + assertThat(treeUpdater.getMeasureMatrix()).isNotNull(); + + // measure matrix was loaded with formula's metrics and measures + assertThat(treeUpdater.getMeasureMatrix().getMetricByUuid(metric2.getUuid())).isNotNull(); + assertThat(treeUpdater.getMeasureMatrix().getMeasure(project, metric2.getKey()).get().getValue()).isEqualTo(1d); + + // no json measures were persisted + assertThat(db.getDbClient().liveMeasureDao().selectMeasure(db.getSession(), project.uuid(), metric1.getKey())) + .hasValueSatisfying(m -> assertThat(m.getValue()).isEqualTo(2d)); + assertThat(db.getDbClient().liveMeasureDao().selectMeasure(db.getSession(), project.uuid(), metric2.getKey())) + .hasValueSatisfying(m -> assertThat(m.getValue()).isEqualTo(1d)); + assertThat(db.getDbClient().jsonMeasureDao().selectByComponentUuid(db.getSession(), project.uuid())) + .hasValueSatisfying(m -> assertThat(m.getMetricValues()).containsExactly(entry(metric2.getKey(), 1d))); + } + + @Test + public void no_json_measure_update_when_migration_enabled_and_branch_not_migrated() { + db.properties().insertProperty(SYSTEM_MEASURES_MIGRATION_ENABLED, "true", null); + db.getDbClient().branchDao().updateMeasuresMigrated(db.getSession(), branch.getUuid(), false); + + markProjectAsAnalyzed(project); + when(componentIndex.getAllUuids()).thenReturn(Set.of(project.uuid())); + + liveMeasureComputer.refresh(db.getSession(), List.of(project)); + + // tree updater was called + assertThat(treeUpdater.getMeasureMatrix()).isNotNull(); + + // measure matrix was loaded with formula's metrics and measures + assertThat(treeUpdater.getMeasureMatrix().getMetricByUuid(metric2.getUuid())).isNotNull(); + assertThat(treeUpdater.getMeasureMatrix().getMeasure(project, metric2.getKey()).get().getValue()).isEqualTo(1d); + + // no json measures were persisted + assertThat(db.getDbClient().liveMeasureDao().selectMeasure(db.getSession(), project.uuid(), metric1.getKey())) + .hasValueSatisfying(m -> assertThat(m.getValue()).isEqualTo(2d)); + assertThat(db.getDbClient().liveMeasureDao().selectMeasure(db.getSession(), project.uuid(), metric2.getKey())) + .hasValueSatisfying(m -> assertThat(m.getValue()).isEqualTo(1d)); + assertThat(db.getDbClient().jsonMeasureDao().selectByComponentUuid(db.getSession(), project.uuid())) + .hasValueSatisfying(m -> assertThat(m.getMetricValues()).containsExactly(entry(metric2.getKey(), 1d))); } @Test @@ -150,6 +220,16 @@ public class LiveMeasureComputerImplTest { assertThat(qgChangeEvents.get(0).getQualityGateSupplier().get()).contains(newQualityGate); } + private void createMeasuresTable() throws SQLException { + new CreateMeasuresTable(db.getDbClient().getDatabase()).execute(); + db.executeDdl("truncate table measures"); + } + + private void createMeasuresMigratedColumn() throws SQLException { + AddMeasuresMigratedColumnToProjectBranchesTable migration = new AddMeasuresMigratedColumnToProjectBranchesTable(db.getDbClient().getDatabase()); + migration.execute(); + } + private SnapshotDto markProjectAsAnalyzed(ComponentDto p) { return markProjectAsAnalyzed(p, 1_490_000_000L); } -- 2.39.5