RollCycleEncodeSequence fix

src/main/java/net/openhft/chronicle/queue/impl/single/RollCycleEncodeSequence.java

@@ -7,6 +7,7 @@
 
 
 class RollCycleEncodeSequence implements Sequence {
+    private static final long THIRTY_ONE_BITS = (1 << 31) - 1;
     private final TwoLongValue writePositionAndSequence;
     private final int cycleShift;
     private final long sequenceMask;
@@ -29,12 +30,10 @@ public void setSequence(long sequence, long position) {
 
     @Override
     public long toIndex(long headerNumber, long sequence) {
-        int cycle = Maths.toUInt31(headerNumber >> cycleShift);
+        int cycle = toCycle(headerNumber);
         return toLongValue(cycle, sequence);
     }
 
-
-
     /**
      * gets the sequence for a writePosition
      * <p>
@@ -57,7 +56,9 @@ public long getSequence(long forWritePosition) {
         if (sequenceValue == 0)
             return Sequence.NOT_FOUND;
 
-        final int lowerBitsOfWp = toLowerBitsWritePosition(toLongValue((int) forWritePosition, 0));
+        // the below cast is safe as cycleMask always returns a number guaranteed within int range
+        int writePositionCycle = (int) cycleMask(forWritePosition);
+        final int lowerBitsOfWp = toLowerBitsWritePosition(toLongValue(writePositionCycle, 0));
         final int toLowerBitsWritePosition = toLowerBitsWritePosition(sequenceValue);
 
         if (lowerBitsOfWp == toLowerBitsWritePosition)
@@ -66,6 +67,10 @@ public long getSequence(long forWritePosition) {
         return Sequence.NOT_FOUND_RETRY;
     }
 
+    private long cycleMask(long number) {
+        return number & THIRTY_ONE_BITS;
+    }
+
     private long toLongValue(int cycle, long sequenceNumber) {
         return ((long) cycle << cycleShift) + (sequenceNumber & sequenceMask);
     }
@@ -75,6 +80,10 @@ public long toSequenceNumber(long index) {
     }
 
     private int toLowerBitsWritePosition(long index) {
-        return Maths.toUInt31(index >> cycleShift);
+        return toCycle(cycleMask(index));
+    }
+
+    private int toCycle(long number) {
+        return Maths.toUInt31(number >> cycleShift);
     }
 }

src/test/java/net/openhft/chronicle/queue/impl/single/RollCycleEncodeSequenceTest.java

@@ -0,0 +1,48 @@
+package net.openhft.chronicle.queue.impl.single;
+
+import net.openhft.chronicle.bytes.Bytes;
+import net.openhft.chronicle.bytes.ref.BinaryTwoLongReference;
+import net.openhft.chronicle.queue.RollCycles;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.util.Arrays;
+import java.util.Collection;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Created by Jerry Shea on 22/11/17.
+ */
+@RunWith(Parameterized.class)
+public class RollCycleEncodeSequenceTest {
+    private final BinaryTwoLongReference longValue;
+    private final RollCycleEncodeSequence rollCycleEncodeSequence;
+
+    @Parameterized.Parameters(name = "{0}")
+    public static Collection<Object[]> data() {
+        return Arrays.asList(new Object[][]{
+                {RollCycles.DAILY},
+                {RollCycles.HOURLY},
+                {RollCycles.MINUTELY},
+                {RollCycles.HUGE_DAILY}
+        });
+    }
+
+    public RollCycleEncodeSequenceTest(final RollCycles cycle) {
+        longValue = new BinaryTwoLongReference();
+        longValue.bytesStore(Bytes.elasticByteBuffer(),0, 16);
+        rollCycleEncodeSequence = new RollCycleEncodeSequence(longValue, cycle.defaultIndexCount(), cycle.defaultIndexSpacing());
+    }
+
+    @Test
+    public void forWritePosition() {
+        longValue.setOrderedValue(1);
+        longValue.setOrderedValue2(2);
+        // a cast to int of this magic number was causing problems
+        long forWritePosition = 0x8001cc54L;
+        long sequence = rollCycleEncodeSequence.getSequence(forWritePosition);
+        assertEquals(longValue.getValue2(), sequence);
+    }
+}