From 091ec857d24bfe139d2a5ce143ffc9b32b21cd7c Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 10 Jun 2015 12:12:31 +0200 Subject: [PATCH] SONAR-6260 Apply PR feedback --- .../server/component/db/SnapshotDao.java | 2 +- .../computation/period/PeriodsHolder.java | 5 +++ .../computation/period/PeriodsHolderImpl.java | 14 ++++--- .../computation/step/FeedPeriodsStep.java | 10 ++--- .../step/PersistComponentsStep.java | 11 +++--- .../server/component/db/SnapshotDaoTest.java | 8 ++-- .../period/PeriodsHolderImplTest.java | 38 ++++++++++++++++++- .../computation/period/PeriodsHolderRule.java | 2 +- .../step/PersistComponentsStepTest.java | 2 +- .../step/PersistSnapshotsStepTest.java | 2 +- .../qualitygate/QualityGateVerifier.java | 18 +++------ .../qualitygate/QualityGateVerifierTest.java | 13 ++----- .../core/component/db/SnapshotMapper.java | 1 - .../component/{ => db}/SnapshotQuery.java | 2 +- .../component/{ => db}/SnapshotQueryTest.java | 6 +-- 15 files changed, 80 insertions(+), 54 deletions(-) rename sonar-core/src/main/java/org/sonar/core/component/{ => db}/SnapshotQuery.java (98%) rename sonar-core/src/test/java/org/sonar/core/component/{ => db}/SnapshotQueryTest.java (90%) diff --git a/server/sonar-server/src/main/java/org/sonar/server/component/db/SnapshotDao.java b/server/sonar-server/src/main/java/org/sonar/server/component/db/SnapshotDao.java index fde3d9be2e7..e5268ddebb3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/component/db/SnapshotDao.java +++ b/server/sonar-server/src/main/java/org/sonar/server/component/db/SnapshotDao.java @@ -25,8 +25,8 @@ import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.api.resources.Scopes; import org.sonar.core.component.SnapshotDto; -import org.sonar.core.component.SnapshotQuery; import org.sonar.core.component.db.SnapshotMapper; +import org.sonar.core.component.db.SnapshotQuery; import org.sonar.core.persistence.DaoComponent; import org.sonar.core.persistence.DbSession; import org.sonar.server.exceptions.NotFoundException; diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/period/PeriodsHolder.java b/server/sonar-server/src/main/java/org/sonar/server/computation/period/PeriodsHolder.java index 622922f23ce..b01786a83e5 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/period/PeriodsHolder.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/period/PeriodsHolder.java @@ -32,6 +32,11 @@ import org.sonar.api.CoreProperties; */ public interface PeriodsHolder { + /** + * Return the list of differential periods. + * + * @throws IllegalStateException if the periods haven't been initialized + */ List getPeriods(); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/period/PeriodsHolderImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/period/PeriodsHolderImpl.java index f11422f33ce..2dfbfe570db 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/period/PeriodsHolderImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/period/PeriodsHolderImpl.java @@ -21,22 +21,24 @@ package org.sonar.server.computation.period; import com.google.common.base.Preconditions; -import java.util.ArrayList; +import com.google.common.collect.ImmutableList; import java.util.List; +import javax.annotation.CheckForNull; public class PeriodsHolderImpl implements PeriodsHolder { - private boolean isPeriodsInitialized = false; - private List periods = new ArrayList<>(); + @CheckForNull + private ImmutableList periods; public void setPeriods(List periods) { - this.periods = periods; - isPeriodsInitialized = true; + Preconditions.checkNotNull(periods, "Periods cannot be null"); + Preconditions.checkState(this.periods == null, "Periods have already been initialized"); + this.periods = ImmutableList.copyOf(periods); } @Override public List getPeriods() { - Preconditions.checkArgument(isPeriodsInitialized, "Periods have not been initialized yet"); + Preconditions.checkState(periods != null, "Periods have not been initialized yet"); return periods; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/FeedPeriodsStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/FeedPeriodsStep.java index c820ea4299f..885a97769aa 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/FeedPeriodsStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/FeedPeriodsStep.java @@ -37,7 +37,7 @@ import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.core.component.ComponentDto; import org.sonar.core.component.SnapshotDto; -import org.sonar.core.component.SnapshotQuery; +import org.sonar.core.component.db.SnapshotQuery; import org.sonar.core.persistence.DbSession; import org.sonar.server.computation.batch.BatchReportReader; import org.sonar.server.computation.component.Component; @@ -46,9 +46,9 @@ import org.sonar.server.computation.period.Period; import org.sonar.server.computation.period.PeriodsHolderImpl; import org.sonar.server.db.DbClient; -import static org.sonar.core.component.SnapshotQuery.SORT_FIELD.BY_DATE; -import static org.sonar.core.component.SnapshotQuery.SORT_ORDER.ASC; -import static org.sonar.core.component.SnapshotQuery.SORT_ORDER.DESC; +import static org.sonar.core.component.db.SnapshotQuery.SORT_FIELD.BY_DATE; +import static org.sonar.core.component.db.SnapshotQuery.SORT_ORDER.ASC; +import static org.sonar.core.component.db.SnapshotQuery.SORT_ORDER.DESC; /** * Populates the {@link org.sonar.server.computation.period.PeriodsHolder} @@ -71,7 +71,7 @@ public class FeedPeriodsStep implements ComputationStep { private final PeriodsHolderImpl periodsHolder; public FeedPeriodsStep(DbClient dbClient, Settings settings, TreeRootHolder treeRootHolder, BatchReportReader batchReportReader, - PeriodsHolderImpl periodsHolder) { + PeriodsHolderImpl periodsHolder) { this.dbClient = dbClient; this.settings = settings; this.treeRootHolder = treeRootHolder; diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistComponentsStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistComponentsStep.java index fc44bc1cdb8..ce2f0d2c3cb 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistComponentsStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistComponentsStep.java @@ -68,16 +68,15 @@ public class PersistComponentsStep implements ComputationStep { org.sonar.server.computation.component.Component root = treeRootHolder.getRoot(); List existingComponents = dbClient.componentDao().selectComponentsFromProjectKey(session, root.getKey()); Map existingComponentDtosByKey = componentDtosByKey(existingComponents); - PersisComponent persisComponent = new PersisComponent(session, existingComponentDtosByKey, reportReader); - - persisComponent.recursivelyProcessComponent(root, null); + PersistComponentExecutor persistComponentExecutor = new PersistComponentExecutor(session, existingComponentDtosByKey, reportReader); + persistComponentExecutor.recursivelyProcessComponent(root, null); session.commit(); } finally { session.close(); } } - private class PersisComponent { + private class PersistComponentExecutor { private final BatchReportReader reportReader; private final Map existingComponentDtosByKey; @@ -85,13 +84,13 @@ public class PersistComponentsStep implements ComputationStep { private ComponentDto project; - public PersisComponent(DbSession dbSession, Map existingComponentDtosByKey, BatchReportReader reportReader) { + public PersistComponentExecutor(DbSession dbSession, Map existingComponentDtosByKey, BatchReportReader reportReader) { this.reportReader = reportReader; this.existingComponentDtosByKey = existingComponentDtosByKey; this.dbSession = dbSession; } - private void recursivelyProcessComponent(Component component, @Nullable ComponentDto lastModule) { + public void recursivelyProcessComponent(Component component, @Nullable ComponentDto lastModule) { BatchReport.Component reportComponent = reportReader.readComponent(component.getRef()); switch (component.getType()) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/component/db/SnapshotDaoTest.java b/server/sonar-server/src/test/java/org/sonar/server/component/db/SnapshotDaoTest.java index 2000c1b1ba7..ceedae09448 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/component/db/SnapshotDaoTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/component/db/SnapshotDaoTest.java @@ -27,14 +27,14 @@ import org.junit.Before; import org.junit.Test; import org.sonar.api.utils.DateUtils; import org.sonar.core.component.SnapshotDto; -import org.sonar.core.component.SnapshotQuery; +import org.sonar.core.component.db.SnapshotQuery; import org.sonar.core.persistence.AbstractDaoTestCase; import org.sonar.core.persistence.DbSession; import static org.assertj.core.api.Assertions.assertThat; -import static org.sonar.core.component.SnapshotQuery.SORT_FIELD.BY_DATE; -import static org.sonar.core.component.SnapshotQuery.SORT_ORDER.ASC; -import static org.sonar.core.component.SnapshotQuery.SORT_ORDER.DESC; +import static org.sonar.core.component.db.SnapshotQuery.SORT_FIELD.BY_DATE; +import static org.sonar.core.component.db.SnapshotQuery.SORT_ORDER.ASC; +import static org.sonar.core.component.db.SnapshotQuery.SORT_ORDER.DESC; public class SnapshotDaoTest extends AbstractDaoTestCase { diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/period/PeriodsHolderImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/period/PeriodsHolderImplTest.java index cf5dacc8ec9..d82ff0ed899 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/period/PeriodsHolderImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/period/PeriodsHolderImplTest.java @@ -45,10 +45,44 @@ public class PeriodsHolderImplTest { } @Test - public void fail_to_get_periods_if_not_initialized() throws Exception { - thrown.expect(IllegalArgumentException.class); + public void get_periods_throws_illegal_state_exception_if_not_initialized() throws Exception { + thrown.expect(IllegalStateException.class); thrown.expectMessage("Periods have not been initialized yet"); new PeriodsHolderImpl().getPeriods(); } + + @Test + public void set_null_periods_trows_null_pointer_exception() throws Exception { + thrown.expect(NullPointerException.class); + thrown.expectMessage("Periods cannot be null"); + + new PeriodsHolderImpl().setPeriods(null); + } + + @Test + public void set_periods_throws_illegal_state_exception_if_already_initialized() throws Exception { + thrown.expect(IllegalStateException.class); + thrown.expectMessage("Periods have already been initialized"); + + List periods = new ArrayList<>(); + periods.add(new Period(1, "mode", null, 1000L)); + + PeriodsHolderImpl periodsHolder = new PeriodsHolderImpl(); + periodsHolder.setPeriods(periods); + periodsHolder.setPeriods(periods); + } + + @Test + public void update_periods_throws_unsupported_operation_exception() throws Exception { + thrown.expect(UnsupportedOperationException.class); + + List periods = new ArrayList<>(); + periods.add(new Period(1, "mode", null, 1000L)); + + PeriodsHolderImpl periodsHolder = new PeriodsHolderImpl(); + periodsHolder.setPeriods(periods); + + periodsHolder.getPeriods().add(new Period(2, "mode", null, 1001L)); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/period/PeriodsHolderRule.java b/server/sonar-server/src/test/java/org/sonar/server/computation/period/PeriodsHolderRule.java index a13a20e84cb..be6ceeb9ce9 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/period/PeriodsHolderRule.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/period/PeriodsHolderRule.java @@ -44,7 +44,7 @@ public class PeriodsHolderRule implements TestRule, PeriodsHolder { } private void clear() { - this.periods = new ArrayList<>(); + this.periods.clear(); } @Override diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistComponentsStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistComponentsStepTest.java index c4e95d99ced..0c1c27ba460 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistComponentsStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistComponentsStepTest.java @@ -781,7 +781,7 @@ public class PersistComponentsStepTest extends BaseStepTest { } @Test - public void not_update_create_at() throws Exception { + public void do_not_update_created_at_on_existing_component() throws Exception { Date oldDate = DateUtils.parseDate("2015-01-01"); ComponentDto project = ComponentTesting.newProjectDto("ABCD").setKey(PROJECT_KEY).setName("Project").setCreatedAt(oldDate); dbClient.componentDao().insert(session, project); diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistSnapshotsStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistSnapshotsStepTest.java index db3f96e9d11..9c6832c9702 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistSnapshotsStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistSnapshotsStepTest.java @@ -33,7 +33,7 @@ import org.sonar.api.utils.System2; import org.sonar.batch.protocol.output.BatchReport; import org.sonar.core.component.ComponentDto; import org.sonar.core.component.SnapshotDto; -import org.sonar.core.component.SnapshotQuery; +import org.sonar.core.component.db.SnapshotQuery; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.DbTester; import org.sonar.server.component.ComponentTesting; diff --git a/sonar-batch/src/main/java/org/sonar/batch/qualitygate/QualityGateVerifier.java b/sonar-batch/src/main/java/org/sonar/batch/qualitygate/QualityGateVerifier.java index 5e300001222..b6f67629881 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/qualitygate/QualityGateVerifier.java +++ b/sonar-batch/src/main/java/org/sonar/batch/qualitygate/QualityGateVerifier.java @@ -42,9 +42,7 @@ import org.sonar.api.resources.Resource; import org.sonar.api.resources.ResourceUtils; import org.sonar.api.utils.Duration; import org.sonar.api.utils.Durations; -import org.sonar.batch.index.BatchComponentCache; import org.sonar.core.qualitygate.db.QualityGateConditionDto; -import org.sonar.core.timemachine.Periods; public class QualityGateVerifier implements Decorator { @@ -58,15 +56,11 @@ public class QualityGateVerifier implements Decorator { private QualityGate qualityGate; - private Periods periods; private I18n i18n; private Durations durations; - private BatchComponentCache resourceCache; - public QualityGateVerifier(QualityGate qualityGate, BatchComponentCache resourceCache, Periods periods, I18n i18n, Durations durations) { + public QualityGateVerifier(QualityGate qualityGate, I18n i18n, Durations durations) { this.qualityGate = qualityGate; - this.resourceCache = resourceCache; - this.periods = periods; this.i18n = i18n; this.durations = durations; } @@ -166,11 +160,11 @@ public class QualityGateVerifier implements Decorator { .append(" ").append(operatorLabel(condition.operator())).append(" ") .append(alertValue(condition, level)); - // Disabled because snapshot is no more created by the batch -// if (alertPeriod != null) { -// Snapshot snapshot = resourceCache.get(project).snapshot(); -// stringBuilder.append(" ").append(periods.label(snapshot, alertPeriod)); -// } + // TODO Disabled because snapshot is no more created by the batch, but should be reactivated when the decorator will be moved to the batch + // if (alertPeriod != null) { + // Snapshot snapshot = resourceCache.get(project).snapshot(); + // stringBuilder.append(" ").append(periods.label(snapshot, alertPeriod)); + // } return stringBuilder.toString(); } diff --git a/sonar-batch/src/test/java/org/sonar/batch/qualitygate/QualityGateVerifierTest.java b/sonar-batch/src/test/java/org/sonar/batch/qualitygate/QualityGateVerifierTest.java index 1579a537074..8404b09e65d 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/qualitygate/QualityGateVerifierTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/qualitygate/QualityGateVerifierTest.java @@ -42,9 +42,7 @@ import org.sonar.api.resources.Resource; import org.sonar.api.test.IsMeasure; import org.sonar.api.utils.Duration; import org.sonar.api.utils.Durations; -import org.sonar.batch.index.BatchComponentCache; import org.sonar.core.qualitygate.db.QualityGateConditionDto; -import org.sonar.core.timemachine.Periods; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.any; @@ -67,14 +65,12 @@ public class QualityGateVerifierTest { Measure measureComplexity; Resource project; Snapshot snapshot; - Periods periods; I18n i18n; Durations durations; @Before public void before() { context = mock(DecoratorContext.class); - periods = mock(Periods.class); i18n = mock(I18n.class); when(i18n.message(any(Locale.class), eq("variation"), eq("variation"))).thenReturn("variation"); durations = mock(Durations.class); @@ -93,10 +89,7 @@ public class QualityGateVerifierTest { project = new Project("foo"); - BatchComponentCache resourceCache = new BatchComponentCache(); - resourceCache.add(project, null).setSnapshot(snapshot); - - verifier = new QualityGateVerifier(qualityGate, resourceCache, periods, i18n, durations); + verifier = new QualityGateVerifier(qualityGate, i18n, durations); } @Test @@ -361,7 +354,7 @@ public class QualityGateVerifierTest { measureClasses.setVariation1(40d); when(i18n.message(any(Locale.class), eq("metric.classes.name"), anyString())).thenReturn("Classes"); - when(periods.label(snapshot, 1)).thenReturn("since someday"); + // when(periods.label(snapshot, 1)).thenReturn("since someday"); ArrayList conditions = Lists.newArrayList( mockCondition(CoreMetrics.CLASSES, QualityGateConditionDto.OPERATOR_GREATER_THAN, null, "30", 1) // generates warning because classes @@ -385,7 +378,7 @@ public class QualityGateVerifierTest { measureClasses.setVariation1(40d); when(i18n.message(any(Locale.class), eq("metric.new_metric_key.name"), anyString())).thenReturn("New Measure"); - when(periods.label(snapshot, 1)).thenReturn("since someday"); + // when(periods.label(snapshot, 1)).thenReturn("since someday"); ArrayList conditions = Lists.newArrayList( mockCondition(newMetric, QualityGateConditionDto.OPERATOR_GREATER_THAN, null, "30", 1) // generates warning because classes increases diff --git a/sonar-core/src/main/java/org/sonar/core/component/db/SnapshotMapper.java b/sonar-core/src/main/java/org/sonar/core/component/db/SnapshotMapper.java index 4f047976ab1..8abac141a46 100644 --- a/sonar-core/src/main/java/org/sonar/core/component/db/SnapshotMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/component/db/SnapshotMapper.java @@ -24,7 +24,6 @@ import java.util.List; import javax.annotation.CheckForNull; import org.apache.ibatis.annotations.Param; import org.sonar.core.component.SnapshotDto; -import org.sonar.core.component.SnapshotQuery; public interface SnapshotMapper { diff --git a/sonar-core/src/main/java/org/sonar/core/component/SnapshotQuery.java b/sonar-core/src/main/java/org/sonar/core/component/db/SnapshotQuery.java similarity index 98% rename from sonar-core/src/main/java/org/sonar/core/component/SnapshotQuery.java rename to sonar-core/src/main/java/org/sonar/core/component/db/SnapshotQuery.java index 4525a0a1b03..db8c996d970 100644 --- a/sonar-core/src/main/java/org/sonar/core/component/SnapshotQuery.java +++ b/sonar-core/src/main/java/org/sonar/core/component/db/SnapshotQuery.java @@ -18,7 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -package org.sonar.core.component; +package org.sonar.core.component.db; import javax.annotation.CheckForNull; import javax.annotation.Nullable; diff --git a/sonar-core/src/test/java/org/sonar/core/component/SnapshotQueryTest.java b/sonar-core/src/test/java/org/sonar/core/component/db/SnapshotQueryTest.java similarity index 90% rename from sonar-core/src/test/java/org/sonar/core/component/SnapshotQueryTest.java rename to sonar-core/src/test/java/org/sonar/core/component/db/SnapshotQueryTest.java index 6c76f162ec9..9ab0af5aa8e 100644 --- a/sonar-core/src/test/java/org/sonar/core/component/SnapshotQueryTest.java +++ b/sonar-core/src/test/java/org/sonar/core/component/db/SnapshotQueryTest.java @@ -18,13 +18,13 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -package org.sonar.core.component; +package org.sonar.core.component.db; import org.junit.Test; import static org.assertj.core.api.Assertions.assertThat; -import static org.sonar.core.component.SnapshotQuery.SORT_FIELD.BY_DATE; -import static org.sonar.core.component.SnapshotQuery.SORT_ORDER.ASC; +import static org.sonar.core.component.db.SnapshotQuery.SORT_FIELD.BY_DATE; +import static org.sonar.core.component.db.SnapshotQuery.SORT_ORDER.ASC; public class SnapshotQueryTest { -- 2.39.5