From 6ac8a76c55772ce0164fb44152dffca7dcc0b036 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 16 Jun 2015 15:45:56 +0200 Subject: [PATCH] SONAR-6620 add PeriodHolder.getPeriod(int) and hasPeriod(int) also make PeriodsHolderRule wrap a PeriodHolderImpl instance so that PeriodHolder contract is easily complied with by the rule --- .../computation/period/PeriodsHolder.java | 19 ++- .../computation/period/PeriodsHolderImpl.java | 73 +++++++-- .../period/PeriodsHolderImplTest.java | 138 ++++++++++++++++-- .../computation/period/PeriodsHolderRule.java | 23 ++- .../FillMeasuresWithVariationsStepTest.java | 23 +-- .../step/PersistSnapshotsStepTest.java | 7 +- 6 files changed, 238 insertions(+), 45 deletions(-) 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 b01786a83e5..706d6d86fe7 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 @@ -33,10 +33,27 @@ import org.sonar.api.CoreProperties; public interface PeriodsHolder { /** - * Return the list of differential periods. + * Return the list of differential periods, ordered by increasing index. * * @throws IllegalStateException if the periods haven't been initialized */ List getPeriods(); + /** + * Finds out whether the holder contains a Period for the specified index. + * + * @throws IllegalStateException if the periods haven't been initialized + * @throws IndexOutOfBoundsException if i is either < 1 or > 5 + */ + boolean hasPeriod(int i); + + /** + * Retrieves the Period for the specified index from the Holder. + * + * @throws IllegalStateException if the periods haven't been initialized + * @throws IllegalStateException if there is no period for the specified index (see {@link #hasPeriod(int)}) + * @throws IndexOutOfBoundsException if i is either < 1 or > 5 + */ + Period getPeriod(int i); + } 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 2dfbfe570db..3c0a0a301a8 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 @@ -20,26 +20,81 @@ package org.sonar.server.computation.period; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; +import com.google.common.base.Predicate; +import com.google.common.base.Predicates; +import com.google.common.collect.Iterables; +import java.util.Arrays; import java.util.List; import javax.annotation.CheckForNull; +import javax.annotation.Nullable; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.FluentIterable.from; +import static java.util.Objects.requireNonNull; +import static org.sonar.server.computation.period.Period.isValidPeriodIndex; public class PeriodsHolderImpl implements PeriodsHolder { @CheckForNull - private ImmutableList periods; + private Period[] periods = null; + + /** + * Initializes the periods in the holder. + * + * @throws NullPointerException if the specified Iterable is {@code null} + * @throws NullPointerException if the specified Iterable contains a {@code null} + * @throws IllegalArgumentException if the specified Iterable has more than 5 elements + * @throws IllegalStateException if the holder has already been initialized + * @throws IllegalStateException if two Periods have the same index + */ + public void setPeriods(Iterable periods) { + requireNonNull(periods, "Periods cannot be null"); + checkArgument(Iterables.size(periods) <= 5, "There can not be more than 5 periods"); + checkState(this.periods == null, "Periods have already been initialized"); - public void setPeriods(List periods) { - Preconditions.checkNotNull(periods, "Periods cannot be null"); - Preconditions.checkState(this.periods == null, "Periods have already been initialized"); - this.periods = ImmutableList.copyOf(periods); + Period[] newPeriods = new Period[5]; + for (Period period : from(periods).filter(CheckNotNull.INSTANCE)) { + int arrayIndex = period.getIndex() - 1; + checkArgument(newPeriods[(arrayIndex)] == null, "More than one period has the index " + period.getIndex()); + newPeriods[(arrayIndex)] = period; + } + this.periods = newPeriods; } @Override public List getPeriods() { - Preconditions.checkState(periods != null, "Periods have not been initialized yet"); - return periods; + checkHolderIsInitialized(); + return from(Arrays.asList(periods)).filter(Predicates.notNull()).toList(); + } + + @Override + public boolean hasPeriod(int i) { + checkHolderIsInitialized(); + if (!isValidPeriodIndex(i)) { + throw new IndexOutOfBoundsException(String.format("Invalid Period index (%s), must be 0 < x < 6", i)); + } + return periods[i - 1] != null; + } + + @Override + public Period getPeriod(int i) { + checkState(hasPeriod(i), "Holder has no Period for index " + i); + return this.periods[i - 1]; + } + + private void checkHolderIsInitialized() { + checkState(this.periods != null, "Periods have not been initialized yet"); + } + + private enum CheckNotNull implements Predicate { + INSTANCE; + + @Override + public boolean apply(@Nullable Period input) { + requireNonNull(input, "No null Period can be added to the holder"); + return true; + } } } 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 b58212ffc55..8af67b31c63 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 @@ -20,28 +20,32 @@ package org.sonar.server.computation.period; +import com.google.common.collect.ImmutableList; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; public class PeriodsHolderImplTest { @Rule public ExpectedException thrown = ExpectedException.none(); + private PeriodsHolderImpl underTest = new PeriodsHolderImpl(); + @Test public void get_periods() throws Exception { List periods = new ArrayList<>(); - periods.add(new Period(1, "mode", null, 1000L, 10L)); + periods.add(createPeriod(1)); - PeriodsHolderImpl periodsHolder = new PeriodsHolderImpl(); - periodsHolder.setPeriods(periods); + underTest.setPeriods(periods); - assertThat(periodsHolder.getPeriods()).hasSize(1); + assertThat(underTest.getPeriods()).hasSize(1); } @Test @@ -53,36 +57,138 @@ public class PeriodsHolderImplTest { } @Test - public void set_null_periods_trows_null_pointer_exception() throws Exception { + public void setPeriods_throws_NPE_if_arg_is_null() throws Exception { thrown.expect(NullPointerException.class); thrown.expectMessage("Periods cannot be null"); - new PeriodsHolderImpl().setPeriods(null); + underTest.setPeriods(null); + } + + @Test + public void setPeriods_throws_NPE_if_arg_contains_null() throws Exception { + thrown.expect(NullPointerException.class); + thrown.expectMessage("No null Period can be added to the holder"); + + ArrayList periods = new ArrayList<>(); + periods.add(null); + underTest.setPeriods(periods); + } + + @Test + public void setPeriods_throws_NPE_if_arg_contains_null_among_others() throws Exception { + thrown.expect(NullPointerException.class); + thrown.expectMessage("No null Period can be added to the holder"); + + List periods = new ArrayList<>(); + periods.add(createPeriod(1)); + periods.add(createPeriod(2)); + periods.add(null); + periods.add(createPeriod(3)); + underTest.setPeriods(periods); + } + + @Test + public void setPeriods_supports_empty_arg_is_empty() { + underTest.setPeriods(ImmutableList.of()); } @Test - public void set_periods_throws_illegal_state_exception_if_already_initialized() throws Exception { + public void setPeriods_throws_IAE_if_arg_has_more_than_5_elements() { + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("There can not be more than 5 periods"); + + underTest.setPeriods(ImmutableList.of(createPeriod(1), createPeriod(2), createPeriod(3), createPeriod(4), createPeriod(5), createPeriod(5))); + } + + @Test + public void setPeriods_throws_ISE_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, 10L)); + periods.add(createPeriod(1)); + + underTest.setPeriods(periods); + underTest.setPeriods(periods); + } + + @Test + public void setPeriods_throws_IAE_if_two_periods_have_the_same_index() { + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("More than one period has the index 1"); - PeriodsHolderImpl periodsHolder = new PeriodsHolderImpl(); - periodsHolder.setPeriods(periods); - periodsHolder.setPeriods(periods); + underTest.setPeriods(ImmutableList.of(createPeriod(1), createPeriod(2), createPeriod(1))); } @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, 10L)); + underTest.setPeriods(ImmutableList.of(createPeriod(1))); + + underTest.getPeriods().add(createPeriod(2)); + } + + @Test + public void getPeriods_returns_Periods_sorted_by_index_no_matter_order_they_were_added() { + + underTest.setPeriods(ImmutableList.of(createPeriod(2), createPeriod(1), createPeriod(3))); + + assertThat(underTest.getPeriods()).extracting("index").containsOnly(1, 2, 3); + } + + @Test + public void hasPeriod_returns_false_if_holder_is_empty() { + underTest.setPeriods(Collections.emptyList()); - PeriodsHolderImpl periodsHolder = new PeriodsHolderImpl(); - periodsHolder.setPeriods(periods); + for (int i = 1; i < 6; i++) { + assertThat(underTest.hasPeriod(i)).isFalse(); + } + } + + @Test + public void hasPeriod_returns_true_only_if_period_exists_in_holder_with_specific_index() { + for (int i = 1; i < 6; i++) { + PeriodsHolderImpl periodsHolder = new PeriodsHolderImpl(); + periodsHolder.setPeriods(ImmutableList.of(createPeriod(i))); + for (int j = 1; j < 6; j++) { + assertThat(periodsHolder.hasPeriod(j)).isEqualTo(j == i); + } + } + } + + @Test + public void getPeriod_returns_the_period_with_the_right_index() { + underTest.setPeriods(ImmutableList.of(createPeriod(1), createPeriod(2), createPeriod(3), createPeriod(4), createPeriod(5))); + + for (int i = 1; i < 6; i++) { + assertThat(underTest.getPeriod(i).getIndex()).isEqualTo(i); + } + } + + @Test + public void getPeriod_throws_ISE_if_period_does_not_exist_in_holder() { + for (int i = 1; i < 6; i++) { + PeriodsHolderImpl periodsHolder = new PeriodsHolderImpl(); + periodsHolder.setPeriods(ImmutableList.of(createPeriod(i))); + + for (int j = 1; j < 6; j++) { + if (j == i) { + continue; + } + + try { + periodsHolder.getPeriod(j); + fail("an IllegalStateException should have been raised"); + } + catch (IllegalStateException e) { + assertThat(e).hasMessage("Holder has no Period for index " + j); + } + } + } + } - periodsHolder.getPeriods().add(new Period(2, "mode", null, 1001L, 11L)); + private static Period createPeriod(int index) { + return new Period(index, index + "mode", null, 1000L, 11l); } } 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 be6ceeb9ce9..0df6577d6c7 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 @@ -20,14 +20,14 @@ package org.sonar.server.computation.period; -import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import org.junit.rules.TestRule; import org.junit.runner.Description; import org.junit.runners.model.Statement; public class PeriodsHolderRule implements TestRule, PeriodsHolder { - private List periods = new ArrayList<>(); + private PeriodsHolderImpl delegate = new PeriodsHolderImpl(); @Override public Statement apply(final Statement statement, Description description) { @@ -44,15 +44,26 @@ public class PeriodsHolderRule implements TestRule, PeriodsHolder { } private void clear() { - this.periods.clear(); + this.delegate = new PeriodsHolderImpl(); + } + + public void setPeriods(Period... periods) { + delegate = new PeriodsHolderImpl(); + delegate.setPeriods(Arrays.asList(periods)); } @Override public List getPeriods() { - return periods; + return delegate.getPeriods(); + } + + @Override + public boolean hasPeriod(int i) { + return delegate.hasPeriod(i); } - public void addPeriod(Period period) { - this.periods.add(period); + @Override + public Period getPeriod(int i) { + return delegate.getPeriod(i); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/FillMeasuresWithVariationsStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/FillMeasuresWithVariationsStepTest.java index f3f7f24aee2..9d4d6504b99 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/step/FillMeasuresWithVariationsStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/FillMeasuresWithVariationsStepTest.java @@ -136,7 +136,7 @@ public class FillMeasuresWithVariationsStepTest { dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 60d)); session.commit(); - periodsHolder.addPeriod(newPeriod(1, period1ProjectSnapshot)); + periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot)); treeRootHolder.setRoot(PROJECT); @@ -149,6 +149,7 @@ public class FillMeasuresWithVariationsStepTest { public void do_nothing_when_no_period() throws Exception { Component project = DumbComponent.builder(Component.Type.PROJECT, 1).setUuid(PROJECT_DTO.uuid()).build(); treeRootHolder.setRoot(project); + periodsHolder.setPeriods(); sut.execute(); @@ -170,7 +171,7 @@ public class FillMeasuresWithVariationsStepTest { dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), directoryDto.getId(), period1DirectorySnapshot.getId(), 10d)); session.commit(); - periodsHolder.addPeriod(newPeriod(1, period1ProjectSnapshot)); + periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot)); Component directory = DumbComponent.builder(Component.Type.DIRECTORY, 2).setUuid(directoryDto.uuid()).build(); Component project = DumbComponent.builder(Component.Type.PROJECT, 1).setUuid(PROJECT_DTO.uuid()).addChildren(directory).build(); @@ -202,11 +203,11 @@ public class FillMeasuresWithVariationsStepTest { newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period5ProjectSnapshot.getId(), 100d)); session.commit(); - periodsHolder.addPeriod(newPeriod(1, period1ProjectSnapshot)); - periodsHolder.addPeriod(newPeriod(2, period2ProjectSnapshot)); - periodsHolder.addPeriod(newPeriod(3, period3ProjectSnapshot)); - periodsHolder.addPeriod(newPeriod(4, period4ProjectSnapshot)); - periodsHolder.addPeriod(newPeriod(5, period5ProjectSnapshot)); + periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot), + newPeriod(2, period2ProjectSnapshot), + newPeriod(3, period3ProjectSnapshot), + newPeriod(4, period4ProjectSnapshot), + newPeriod(5, period5ProjectSnapshot)); treeRootHolder.setRoot(PROJECT); @@ -237,7 +238,7 @@ public class FillMeasuresWithVariationsStepTest { ); session.commit(); - periodsHolder.addPeriod(newPeriod(1, period1ProjectSnapshot)); + periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot)); treeRootHolder.setRoot(PROJECT); @@ -270,7 +271,7 @@ public class FillMeasuresWithVariationsStepTest { dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 20d).setRuleId(rule2.getId())); session.commit(); - periodsHolder.addPeriod(newPeriod(1, period1ProjectSnapshot)); + periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot)); treeRootHolder.setRoot(PROJECT); @@ -299,7 +300,7 @@ public class FillMeasuresWithVariationsStepTest { dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 20d).setCharacteristicId(char2.getId())); session.commit(); - periodsHolder.addPeriod(newPeriod(1, period1ProjectSnapshot)); + periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot)); treeRootHolder.setRoot(PROJECT); @@ -324,7 +325,7 @@ public class FillMeasuresWithVariationsStepTest { dbClient.measureDao().insert(session, newMeasureDto(ISSUES_METRIC.getId(), PROJECT_DTO.getId(), period1ProjectSnapshot.getId(), 60d)); session.commit(); - periodsHolder.addPeriod(newPeriod(1, period1ProjectSnapshot)); + periodsHolder.setPeriods(newPeriod(1, period1ProjectSnapshot)); treeRootHolder.setRoot(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 3af4fca66ac..fe5b219b4f2 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 @@ -102,6 +102,9 @@ public class PersistSnapshotsStepTest extends BaseStepTest { when(system2.now()).thenReturn(now); sut = new PersistSnapshotsStep(system2, dbClient, treeRootHolder, reportReader, dbIdsRepository, periodsHolderRule); + + // initialize PeriodHolder to empty by default + periodsHolderRule.setPeriods(); } @Override @@ -298,7 +301,7 @@ public class PersistSnapshotsStepTest extends BaseStepTest { SnapshotDto snapshotDto = SnapshotTesting.createForProject(projectDto).setCreatedAt(DateUtils.parseDateQuietly("2015-01-01").getTime()); dbClient.snapshotDao().insert(session, snapshotDto); session.commit(); - periodsHolderRule.addPeriod(new Period(1, CoreProperties.TIMEMACHINE_MODE_DATE, "2015-01-01", analysisDate, 123L)); + periodsHolderRule.setPeriods(new Period(1, CoreProperties.TIMEMACHINE_MODE_DATE, "2015-01-01", analysisDate, 123L)); Component project = DumbComponent.builder(Component.Type.PROJECT, 1).setUuid("ABCD").setKey(PROJECT_KEY).build(); treeRootHolder.setRoot(project); @@ -314,7 +317,7 @@ public class PersistSnapshotsStepTest extends BaseStepTest { @Test public void only_persist_snapshots_with_periods_on_project_and_module() throws Exception { - periodsHolderRule.addPeriod(new Period(1, CoreProperties.TIMEMACHINE_MODE_PREVIOUS_ANALYSIS, null, analysisDate, 123L)); + periodsHolderRule.setPeriods(new Period(1, CoreProperties.TIMEMACHINE_MODE_PREVIOUS_ANALYSIS, null, analysisDate, 123L)); ComponentDto projectDto = ComponentTesting.newProjectDto("ABCD").setKey(PROJECT_KEY).setName("Project"); dbClient.componentDao().insert(session, projectDto); -- 2.39.5