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

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 b01786a83e562e10e38d3ce4fdd1316feaee75ca..706d6d86fe71f21027be2285e22777536071c79a 100644 (file)
--- 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<Period> 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 &lt; 1 or &gt; 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 &lt; 1 or &gt; 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 2dfbfe570dbe1107e5b37b03a03b2b2484cfe932..3c0a0a301a822ed18b17c4d635268573dcaa7c6e 100644 (file)
--- 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<Period> 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<Period> 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<Period> 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<Period> 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<Period> {
+    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 b58212ffc5545aa8ce055863ae8539e902dc32f8..8af67b31c63d7cf25cc916f2fcfd0cfec38802ff 100644 (file)
--- 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<Period> 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<Period> 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<Period> 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.<Period>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<Period> 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<Period> 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.<Period>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 be6ceeb9ce9128f23ac6e4940041a6382ba94bd5..0df6577d6c7572555ae22d8135f5bdfb03307acb 100644 (file)
--- 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<Period> 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<Period> 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 f3f7f24aee245821324cd04091c78896343e3451..9d4d6504b9971807cdc5a4e342cbd70bb2989588 100644 (file)
--- 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 3af4fca66ac7f304cb18fff371efc8fe9d1b429b..fe5b219b4f20e72038335a14ee5fbc81fe734357 100644 (file)
--- 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);