[streams] review updates

atp-mipt · Dec 17, 2023 · 752e9bc · 752e9bc
1 parent 0639c52
commit 752e9bc
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 78 deletions.
diff --git a/src/main/java/com/github/fppt/jedismock/datastructures/streams/StreamId.java b/src/main/java/com/github/fppt/jedismock/datastructures/streams/StreamId.java
@@ -3,13 +3,15 @@
 import com.github.fppt.jedismock.datastructures.Slice;
 import com.github.fppt.jedismock.exception.WrongStreamKeyException;
 
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 import static com.github.fppt.jedismock.datastructures.streams.StreamErrors.RANGES_END_ID_ERROR;
 import static com.github.fppt.jedismock.datastructures.streams.StreamErrors.RANGES_START_ID_ERROR;
 import static java.lang.Long.compareUnsigned;
 import static java.lang.Long.parseUnsignedLong;
 import static java.lang.Long.toUnsignedString;
 import static com.github.fppt.jedismock.datastructures.streams.StreamErrors.INVALID_ID_ERROR;
-import static com.github.fppt.jedismock.datastructures.streams.StreamErrors.ZERO_ERROR;
 
 public final class StreamId implements Comparable<StreamId> {
     private final long firstPart;
@@ -35,48 +37,25 @@ public StreamId(long firstPart, long secondPart) {
     }
 
     public StreamId(String key) throws WrongStreamKeyException {
-        String[] parsed = key.split("-");
-
-        switch (parsed.length) {
-            case 2:
-                try {
-                    secondPart = parseUnsignedLong(parsed[1]);
-                } catch (NumberFormatException e) {
-                    throw new WrongStreamKeyException(INVALID_ID_ERROR);
-                }
-
-                try {
-                    firstPart = parseUnsignedLong(parsed[0]);
-                } catch (NumberFormatException e) {
-                    throw new WrongStreamKeyException(INVALID_ID_ERROR);
-                }
-
-                break;
-            case 1:
-                secondPart = 0;
-
-                try {
-                    firstPart = parseUnsignedLong(parsed[0]);
-                } catch (NumberFormatException e) {
-                    throw new WrongStreamKeyException(INVALID_ID_ERROR);
-                }
-
-                break;
-            default:
+        Matcher matcher = Pattern.compile("(\\d+)(?:-(\\d+))?").matcher(key);
+        if (matcher.matches()) {
+            try {
+                firstPart = parseUnsignedLong(matcher.group(1));
+                secondPart = matcher.group(2) == null ? 0 : parseUnsignedLong(matcher.group(2));
+            } catch (NumberFormatException e) {
                 throw new WrongStreamKeyException(INVALID_ID_ERROR);
+            }
+        } else {
+            throw new WrongStreamKeyException(INVALID_ID_ERROR);
         }
     }
 
     public StreamId(Slice slice) throws WrongStreamKeyException {
         this(slice.toString());
     }
 
-    public StreamId compareToZero() throws WrongStreamKeyException {
-        if (secondPart == 0 && firstPart == 0) {
-            throw new WrongStreamKeyException(ZERO_ERROR);
-        }
-
-        return this;
+    public boolean isZero() {
+        return secondPart == 0 && firstPart == 0;
     }
 
     public StreamId increment() throws WrongStreamKeyException {
@@ -111,8 +90,9 @@ public StreamId decrement() throws WrongStreamKeyException {
 
     @Override
     public int compareTo(StreamId other) {
-        return compareUnsigned(firstPart, other.firstPart) != 0
-                ? compareUnsigned(firstPart, other.firstPart)
+        int firstPartComparison = compareUnsigned(firstPart, other.firstPart);
+        return firstPartComparison != 0
+                ? firstPartComparison
                 : compareUnsigned(secondPart, other.secondPart);
     }
 

diff --git a/src/main/java/com/github/fppt/jedismock/operations/streams/XAdd.java b/src/main/java/com/github/fppt/jedismock/operations/streams/XAdd.java
@@ -12,10 +12,7 @@
 
 import java.util.List;
 
-import static com.github.fppt.jedismock.datastructures.streams.StreamErrors.LIMIT_OPTION_ERROR;
-import static com.github.fppt.jedismock.datastructures.streams.StreamErrors.NOT_AN_INTEGER_ERROR;
-import static com.github.fppt.jedismock.datastructures.streams.StreamErrors.SYNTAX_ERROR;
-import static com.github.fppt.jedismock.datastructures.streams.StreamErrors.TOP_ERROR;
+import static com.github.fppt.jedismock.datastructures.streams.StreamErrors.*;
 import static com.github.fppt.jedismock.operations.streams.XTrim.trimID;
 import static com.github.fppt.jedismock.operations.streams.XTrim.trimLen;
 
@@ -31,14 +28,15 @@ public XAdd(RedisBase base, List<Slice> params) {
         super(base, params);
     }
 
-    public StreamId compareWithTopKey(StreamId key) throws WrongStreamKeyException {
-        RMStream stream = getStreamFromBaseOrCreateEmpty(params().get(0));
+    void validate(StreamId key) throws WrongStreamKeyException {
+        if (key.isZero()) {
+            throw new WrongStreamKeyException(ZERO_ERROR);
+        }
 
-        if (key.compareTo(stream.getLastId()) <= 0) {
+        StreamId lastId = getStreamFromBaseOrCreateEmpty(params().get(0)).getLastId();
+        if (key.compareTo(lastId) <= 0) {
             throw new WrongStreamKeyException(TOP_ERROR);
         }
-
-        return key;
     }
 
     @Override
@@ -104,7 +102,8 @@ protected Slice response() {
         StreamId nodeId;
 
         try {
-           nodeId = compareWithTopKey(new StreamId(stream.replaceAsterisk(id)).compareToZero());
+           nodeId = new StreamId(stream.replaceAsterisk(id));
+           validate(nodeId);
         } catch (WrongStreamKeyException e) {
             return Response.error(e.getMessage());
         }

diff --git a/src/test/java/com/github/fppt/jedismock/comparisontests/sortedsets/TestZDiff.java b/src/test/java/com/github/fppt/jedismock/comparisontests/sortedsets/TestZDiff.java
@@ -130,7 +130,7 @@ public static String randstring(int min, int max, String type) {
     }
 
     @TestTemplate
-    public void testZDiffFuzzing(Jedis jedis) {
+    public void testZDiffStress(Jedis jedis) {
         for (int j = 0; j < 100; j++) {
             Map<String, String> s = new HashMap<>();
             List<String> argsList = new ArrayList<>();

diff --git a/.../java/com/github/fppt/jedismock/datastructures/streams/LinkedMapForwardIteratorTests.java b/.../java/com/github/fppt/jedismock/datastructures/streams/LinkedMapForwardIteratorTests.java
@@ -271,7 +271,7 @@ void findFirstWhenKeyDoesNotExistTest() {
     }
 
     @Test
-    void findFirstSuitableFuzzingTest() {
+    void findFirstSuitableStressTest() {
         LinkedMap<Integer, Integer> map = new LinkedMap<>();
         LinkedMapForwardIterator<Integer, Integer> it = map.iterator();
 

diff --git a/src/test/java/com/github/fppt/jedismock/datastructures/streams/RMStreamTests.java b/src/test/java/com/github/fppt/jedismock/datastructures/streams/RMStreamTests.java
@@ -100,7 +100,7 @@ void whenKeyIsIncorrect_ensureReplaceAsteriskDoesNotChangeIt() {
     }
 
     @Test
-    void fuzzingAsteriskTest() {
+    void stressAsteriskTest() {
         for (int i = 0; i < 1000; ++i) {
             if (Math.random() >= 0.9) {
                 long second = (long) (Math.random() * Long.MAX_VALUE) * (Math.random() >= 0.5 ? 1 : -1);

diff --git a/src/test/java/com/github/fppt/jedismock/datastructures/streams/StreamIdTests.java b/src/test/java/com/github/fppt/jedismock/datastructures/streams/StreamIdTests.java
@@ -17,35 +17,31 @@ void equalsHashCodeTest(){
     @Test
     void zeroComparisonWithZeroTest() {
         StreamId zero = new StreamId(0, 0);
-        assertThrows(
-                WrongStreamKeyException.class,
-                zero::compareToZero,
-                "ERR The ID specified in XADD must be greater than 0-0"
-        );
+        assertTrue(zero.isZero());
     }
 
     @Test
     void zeroComparisonWithPositiveKeysTest() {
         StreamId other = new StreamId(0, 1);
-        assertDoesNotThrow(() -> assertSame(other, other.compareToZero()));
+        assertFalse(other.isZero());
 
         StreamId newOther = new StreamId(1, 0);
-        assertDoesNotThrow(() -> assertSame(newOther, newOther.compareToZero()));
+        assertFalse(newOther.isZero());
 
         StreamId newestOther = new StreamId(1, 1);
-        assertDoesNotThrow(() -> assertSame(newestOther, newestOther.compareToZero()));
+        assertFalse(newestOther.isZero());
     }
 
     @Test
     void zeroComparisonWithNegativeKeysTest() {
         StreamId other = new StreamId(0, -1);
-        assertDoesNotThrow(() -> assertSame(other, other.compareToZero()));
+        assertFalse(other.isZero());
 
         StreamId newOther = new StreamId(-1, 0);
-        assertDoesNotThrow(() -> assertSame(newOther, newOther.compareToZero()));
+        assertFalse(newOther.isZero());
 
         StreamId newestOther = new StreamId(-1, -1);
-        assertDoesNotThrow(() -> assertSame(newestOther, newestOther.compareToZero()));
+        assertFalse(newestOther.isZero());
     }
 
     @Test
@@ -105,7 +101,7 @@ void decrementTest() {
     }
 
     @Test
-    void incrementDecrementFuzzingTest() {
+    void incrementDecrementStressTest() {
         for (int i = 0; i < 1000; ++i) {
             if (Math.random() >= 0.9) {
                 long first = (long) (Math.random() * Long.MAX_VALUE) * (Math.random() >= 0.5 ? 1 : -1);
@@ -145,7 +141,7 @@ void incrementDecrementFuzzingTest() {
     }
 
     @Test
-    void toStringFuzzingTest() {
+    void toStringStressTest() {
         assertEquals(
                 "18446744072474983726-18446744072474983726",
                 new StreamId(-1234567890, -1234567890).toString()
@@ -242,7 +238,7 @@ void constructorInvalidIdsTest() {
     }
 
     @Test
-    void constructorFuzzingTest() {
+    void constructorStressTest() {
         for (int i = 0; i < 1000; ++i) {
             long first = (long) (Math.random() * (Long.MAX_VALUE - 1) + 1) * (Math.random() >= 0.5 ? 1 : -1);
             long second = (long) (Math.random() * (Long.MAX_VALUE - 1) + 1) * (Math.random() >= 0.5 ? 1 : -1);

diff --git a/...ntests/streams/CompareToTopItemTests.java → ...k/operations/streams/ValidationTests.java b/...ntests/streams/CompareToTopItemTests.java → ...k/operations/streams/ValidationTests.java
@@ -1,21 +1,64 @@
-package com.github.fppt.jedismock.comparisontests.streams;
+package com.github.fppt.jedismock.operations.streams;
 
 import com.github.fppt.jedismock.datastructures.Slice;
 import com.github.fppt.jedismock.datastructures.streams.RMStream;
 import com.github.fppt.jedismock.datastructures.streams.StreamId;
 import com.github.fppt.jedismock.exception.WrongStreamKeyException;
-import com.github.fppt.jedismock.operations.streams.XAdd;
 import com.github.fppt.jedismock.storage.RedisBase;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 
 import java.util.Collections;
 
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
-import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
-public class CompareToTopItemTests {
+public class ValidationTests {
+    static RMStream stream = new RMStream();
+    static XAdd xAdd;
+
+    @BeforeAll
+    static void setUp() {
+        RedisBase mock = Mockito.mock(RedisBase.class);
+        Mockito.when(mock.getStream(Mockito.any())).thenReturn(stream);
+        xAdd = new XAdd(mock, Collections.singletonList(Slice.create("mock")));
+    }
+
+    @Test
+    void zeroComparisonWithZeroTest() {
+        StreamId zero = new StreamId(0, 0);
+        assertThrows(
+                WrongStreamKeyException.class,
+                () -> xAdd.validate(zero),
+                "ERR The ID specified in XADD must be greater than 0-0"
+        );
+    }
+
+    @Test
+    void zeroComparisonWithPositiveKeysTest() {
+        StreamId other = new StreamId(0, 1);
+        assertDoesNotThrow(() -> xAdd.validate(other));
+
+        StreamId newOther = new StreamId(1, 0);
+        assertDoesNotThrow(() -> xAdd.validate(newOther));
+
+        StreamId newestOther = new StreamId(1, 1);
+        assertDoesNotThrow(() -> xAdd.validate(newestOther));
+    }
+
+    @Test
+    void zeroComparisonWithNegativeKeysTest() {
+        StreamId other = new StreamId(0, -1);
+        assertDoesNotThrow(() -> xAdd.validate(other));
+
+        StreamId newOther = new StreamId(-1, 0);
+        assertDoesNotThrow(() -> xAdd.validate(newOther));
+
+        StreamId newestOther = new StreamId(-1, -1);
+        assertDoesNotThrow(() -> xAdd.validate(newestOther));
+    }
+
     @Test
     void compareTopIdTest() {
         RMStream stream = new RMStream();
@@ -29,30 +72,30 @@ void compareTopIdTest() {
 
         assertThrows(
                 WrongStreamKeyException.class,
-                () -> xAdd.compareWithTopKey(new StreamId(3, 3)),
+                () -> xAdd.validate(new StreamId(3, 3)),
                 "ERR The ID specified in XADD is equal or smaller than the target stream top item"
         );
 
         assertThrows(
                 WrongStreamKeyException.class,
-                () -> xAdd.compareWithTopKey(new StreamId(3, 2)),
+                () -> xAdd.validate(new StreamId(3, 2)),
                 "ERR The ID specified in XADD is equal or smaller than the target stream top item"
         );
 
         assertThrows(
                 WrongStreamKeyException.class,
-                () -> xAdd.compareWithTopKey(new StreamId(2, -1)),
+                () -> xAdd.validate(new StreamId(2, -1)),
                 "ERR The ID specified in XADD is equal or smaller than the target stream top item"
         );
 
         StreamId key = new StreamId(3, 4);
-        assertDoesNotThrow(() -> assertSame(key, xAdd.compareWithTopKey(key)));
+        assertDoesNotThrow(() -> xAdd.validate(key));
         StreamId newKey = new StreamId(4, 0);
-        assertDoesNotThrow(() -> assertSame(newKey, xAdd.compareWithTopKey(newKey)));
+        assertDoesNotThrow(() -> xAdd.validate(newKey));
     }
 
     @Test
-    void fuzzingTest() {
+    void stressTest() {
         RMStream stream = new RMStream();
 
         RedisBase mock = Mockito.mock(RedisBase.class);
@@ -69,20 +112,20 @@ void fuzzingTest() {
             stream.updateLastId(id);
 
             StreamId key = new StreamId(first, second + 1);
-            assertDoesNotThrow(() -> assertSame(key, xAdd.compareWithTopKey(key)));
+            assertDoesNotThrow(() -> xAdd.validate(key));
 
             StreamId newKey = new StreamId(first + 1, 0);
-            assertDoesNotThrow(() -> assertSame(newKey, xAdd.compareWithTopKey(newKey)));
+            assertDoesNotThrow(() -> xAdd.validate(newKey));
 
             assertThrows(
                     WrongStreamKeyException.class,
-                    () -> xAdd.compareWithTopKey(new StreamId(first, second - 1)),
+                    () -> xAdd.validate(new StreamId(first, second - 1)),
                     "ERR The ID specified in XADD is equal or smaller than the target stream top item"
             );
 
             assertThrows(
                     WrongStreamKeyException.class,
-                    () -> xAdd.compareWithTopKey(new StreamId(first - 1, -1)),
+                    () -> xAdd.validate(new StreamId(first - 1, -1)),
                     "ERR The ID specified in XADD is equal or smaller than the target stream top item"
             );
         }