diff --git a/src/main/java/org/apache/commons/collections4/bloomfilter/IndexProducer.java b/src/main/java/org/apache/commons/collections4/bloomfilter/IndexProducer.java index ca6ac6e8c..5789285bf 100644 --- a/src/main/java/org/apache/commons/collections4/bloomfilter/IndexProducer.java +++ b/src/main/java/org/apache/commons/collections4/bloomfilter/IndexProducer.java @@ -39,7 +39,7 @@ public interface IndexProducer { * *

Any exceptions thrown by the action are relayed to the caller.

* - *

Indices ordering is not guaranteed

+ *

Indices ordering and uniqueness is not guaranteed.

* * @param predicate the action to be performed for each non-zero bit index. * @return {@code true} if all indexes return true from consumer, {@code false} otherwise. @@ -103,6 +103,9 @@ public interface IndexProducer { /** * Return a copy of the IndexProducer data as an int array. + * + *

Indices ordering and uniqueness is not guaranteed.

+ * *

* The default implementation of this method is slow. It is recommended * that implementing classes reimplement this method. diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/AbstractIndexProducerTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/AbstractIndexProducerTest.java index 54dc01c7d..ac5a6fc23 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/AbstractIndexProducerTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/AbstractIndexProducerTest.java @@ -16,34 +16,60 @@ */ package org.apache.commons.collections4.bloomfilter; -import static org.junit.Assert.assertEquals; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.util.ArrayList; -import java.util.List; +import java.util.Arrays; +import java.util.BitSet; import java.util.function.IntPredicate; - +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Test; public abstract class AbstractIndexProducerTest { - public static final IntPredicate TRUE_PREDICATE = new IntPredicate() { + private static final IntPredicate TRUE_PREDICATE = i -> true; + private static final IntPredicate FALSE_PREDICATE = i -> false; - @Override - public boolean test(int arg0) { + /** Flag to indicate the {@link IndexProducer#forEachIndex(IntPredicate)} is ordered. */ + protected static final int FOR_EACH_ORDERED = 0x1; + /** Flag to indicate the {@link IndexProducer#forEachIndex(IntPredicate)} is distinct. */ + protected static final int FOR_EACH_DISTINCT = 0x2; + /** Flag to indicate the {@link IndexProducer#asIndexArray()} is ordered. */ + protected static final int AS_ARRAY_ORDERED = 0x4; + /** Flag to indicate the {@link IndexProducer#asIndexArray()} is distinct. */ + protected static final int AS_ARRAY_DISTINCT = 0x8; + + /** + * An expandable list of int values. + */ + private static class IntList { + private int size; + private int[] data = {0}; + + /** + * Adds the value to the list. + * + * @param value the value + * @return true if the list was modified + */ + boolean add(int value) { + if (size == data.length) { + data = Arrays.copyOf(data, size << 1); + } + data[size++] = value; return true; } - }; - public static final IntPredicate FALSE_PREDICATE = new IntPredicate() { - - @Override - public boolean test(int arg0) { - return false; + /** + * Convert to an array. + * + * @return the array + */ + int[] toArray() { + return Arrays.copyOf(data, size); } - }; + } /** * Creates a producer with some data. @@ -57,13 +83,22 @@ public abstract class AbstractIndexProducerTest { */ protected abstract IndexProducer createEmptyProducer(); + /** + * Gets the behaviour flags. + * + *

The flags indicate if the methods {@link IndexProducer#forEachIndex(IntPredicate)} + * and {@link IndexProducer#asIndexArray()} output sorted or distinct indices. + * + * @return the behaviour. + */ + protected abstract int getBehaviour(); + @Test public final void testForEachIndex() { - IndexProducer populated = createProducer(); IndexProducer empty = createEmptyProducer(); - assertFalse(populated.forEachIndex(FALSE_PREDICATE), "non-empty should be false"); + assertFalse(populated.forEachIndex(FALSE_PREDICATE), "non-empty should be false"); assertTrue(empty.forEachIndex(FALSE_PREDICATE), "empty should be true"); assertTrue(populated.forEachIndex(TRUE_PREDICATE), "non-empty should be true"); @@ -71,40 +106,77 @@ public abstract class AbstractIndexProducerTest { } @Test - public final void testAsIndexArray() { - int ary[] = createEmptyProducer().asIndexArray(); - assertEquals(0, ary.length); - - IndexProducer producer = createProducer(); - List lst = new ArrayList(); - for (int i : producer.asIndexArray()) { - lst.add(i); - } - assertTrue(producer.forEachIndex(new IntPredicate() { - - @Override - public boolean test(int value) { - assertTrue(lst.remove(Integer.valueOf(value)), - String.format("Instance of %d was not found in lst", value)); - return true; - } + public final void testEmptyProducer() { + IndexProducer empty = createEmptyProducer(); + int ary[] = empty.asIndexArray(); + Assertions.assertEquals(0, ary.length); + assertTrue(empty.forEachIndex(i -> { + throw new AssertionError("forEach predictate should not be called"); })); } + /** + * Test the distinct indices output from the producer are consistent. + */ @Test - public void testForIndexEarlyExit() { + public final void testConsistency() { + IndexProducer producer = createProducer(); + BitSet bs1 = new BitSet(); + BitSet bs2 = new BitSet(); + Arrays.stream(producer.asIndexArray()).forEach(bs1::set); + producer.forEachIndex(i -> { + bs2.set(i); + return true; + }); + Assertions.assertEquals(bs1, bs2); + } + + @Test + public final void testBehaviourAsIndexArray() { + int flags = getBehaviour(); + Assumptions.assumeTrue((flags & (AS_ARRAY_ORDERED | AS_ARRAY_DISTINCT)) != 0); + int[] actual = createProducer().asIndexArray(); + if ((flags & AS_ARRAY_ORDERED) != 0) { + int[] expected = Arrays.stream(actual).sorted().toArray(); + Assertions.assertArrayEquals(expected, actual); + } + if ((flags & AS_ARRAY_DISTINCT) != 0) { + long count = Arrays.stream(actual).distinct().count(); + Assertions.assertEquals(count, actual.length); + } + } + + @Test + public final void testBehaviourForEach() { + int flags = getBehaviour(); + Assumptions.assumeTrue((flags & (FOR_EACH_ORDERED | FOR_EACH_DISTINCT)) != 0); + IntList list = new IntList(); + createProducer().forEachIndex(list::add); + int[] actual = list.toArray(); + if ((flags & FOR_EACH_ORDERED) != 0) { + int[] expected = Arrays.stream(actual).sorted().toArray(); + Assertions.assertArrayEquals(expected, actual); + } + if ((flags & FOR_EACH_DISTINCT) != 0) { + long count = Arrays.stream(actual).distinct().count(); + Assertions.assertEquals(count, actual.length); + } + } + + @Test + public void testForEachIndexEarlyExit() { int[] passes = new int[1]; assertFalse(createProducer().forEachIndex(i -> { passes[0]++; return false; })); - assertEquals(1, passes[0]); + Assertions.assertEquals(1, passes[0]); passes[0] = 0; assertTrue(createEmptyProducer().forEachIndex(i -> { passes[0]++; return false; })); - assertEquals(0, passes[0]); + Assertions.assertEquals(0, passes[0]); } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromArrayCountingBloomFilterTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromArrayCountingBloomFilterTest.java index 7961b6d53..331411436 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromArrayCountingBloomFilterTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromArrayCountingBloomFilterTest.java @@ -32,4 +32,10 @@ public class BitCountProducerFromArrayCountingBloomFilterTest extends AbstractBi protected BitCountProducer createEmptyProducer() { return new ArrayCountingBloomFilter(shape); } + + @Override + protected int getBehaviour() { + // CountingBloomFilter based on an array will be distinct and ordered + return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromIndexProducerTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromIndexProducerTest.java index d4c2be603..8458dddfa 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromIndexProducerTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromIndexProducerTest.java @@ -20,13 +20,14 @@ import static org.junit.Assert.assertEquals; import java.util.HashMap; import java.util.Map; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; public class BitCountProducerFromIndexProducerTest extends AbstractBitCountProducerTest { @Override protected BitCountProducer createProducer() { - return BitCountProducer.from(IndexProducer.fromIndexArray(new int[] { 0, 1, 63, 64, 127, 128 })); + return BitCountProducer.from(IndexProducer.fromIndexArray(new int[] { 0, 63, 1, 1, 64, 127, 128 })); } @Override @@ -34,7 +35,14 @@ public class BitCountProducerFromIndexProducerTest extends AbstractBitCountProdu return BitCountProducer.from(IndexProducer.fromIndexArray(new int[0])); } + @Override + protected int getBehaviour() { + // The default method streams a BitSet so is distinct and ordered. + return AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } + @Test + @Disabled("Current behaviour will return the same index twice, each with a count of 1") public final void testFromIndexProducer() { BitCountProducer producer = createProducer(); @@ -47,7 +55,7 @@ public class BitCountProducerFromIndexProducerTest extends AbstractBitCountProdu assertEquals(6, m.size()); assertEquals(Integer.valueOf(1), m.get(0)); - assertEquals(Integer.valueOf(1), m.get(1)); + assertEquals(Integer.valueOf(2), m.get(1)); assertEquals(Integer.valueOf(1), m.get(63)); assertEquals(Integer.valueOf(1), m.get(64)); assertEquals(Integer.valueOf(1), m.get(127)); diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/DefaultIndexProducerTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/DefaultIndexProducerTest.java index b4558ab9b..dc0ca7f84 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/DefaultIndexProducerTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/DefaultIndexProducerTest.java @@ -45,6 +45,12 @@ public class DefaultIndexProducerTest extends AbstractIndexProducerTest { }; } + @Override + protected int getBehaviour() { + // The default method streams a BitSet so is distinct and ordered. + return AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } + /** * Generates an array of integers. * @param size the size of the array diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasherTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasherTest.java index ef20be9e6..49afb7b28 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasherTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasherTest.java @@ -35,6 +35,12 @@ public class EnhancedDoubleHasherTest extends AbstractHasherTest { return NullHasher.INSTANCE; } + @Override + protected int getBehaviour() { + // Allows duplicates and may be unordered + return 0; + } + @Override protected int getHasherSize(Hasher hasher) { return 1; diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/HasherCollectionTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/HasherCollectionTest.java index 997ee01a2..894115c59 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/HasherCollectionTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/HasherCollectionTest.java @@ -41,14 +41,24 @@ public class HasherCollectionTest extends AbstractHasherTest { return new HasherCollection(); } + @Override + protected int getBehaviour() { + // Allows duplicates and may be unordered + return 0; + } + @Override protected int getHasherSize(Hasher hasher) { return ((HasherCollection) hasher).getHashers().size(); } protected void nestedTest(HasherCollectionTest nestedTest) { - nestedTest.testAsIndexArray(); nestedTest.testForEachIndex(); + nestedTest.testEmptyProducer(); + nestedTest.testConsistency(); + nestedTest.testBehaviourAsIndexArray(); + nestedTest.testBehaviourForEach(); + nestedTest.testForEachIndexEarlyExit(); nestedTest.testAdd(); } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromArrayCountingBloomFilterTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromArrayCountingBloomFilterTest.java index 0ea2c6079..4b7cbb8c8 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromArrayCountingBloomFilterTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromArrayCountingBloomFilterTest.java @@ -32,4 +32,9 @@ public class IndexProducerFromArrayCountingBloomFilterTest extends AbstractIndex protected IndexProducer createEmptyProducer() { return new ArrayCountingBloomFilter(shape); } + + @Override + protected int getBehaviour() { + return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromBitmapProducerTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromBitmapProducerTest.java index a208d3a2f..e844183ef 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromBitmapProducerTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromBitmapProducerTest.java @@ -49,6 +49,12 @@ public class IndexProducerFromBitmapProducerTest extends AbstractIndexProducerTe return IndexProducer.fromBitMapProducer(producer); } + @Override + protected int getBehaviour() { + // Bit maps will be distinct. Conversion to indices should be ordered. + return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } + @Test public final void testFromBitMapProducerTest() { IndexProducer underTest = createProducer(); diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherCollectionTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherCollectionTest.java index 1376a4bfc..044e727b2 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherCollectionTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherCollectionTest.java @@ -27,4 +27,10 @@ public class IndexProducerFromHasherCollectionTest extends AbstractIndexProducer protected IndexProducer createEmptyProducer() { return new HasherCollection().indices(Shape.fromKM(17, 72)); } + + @Override + protected int getBehaviour() { + // HasherCollection allows duplicates and may be unordered + return 0; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherTest.java index 683b3705e..0e7368dee 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherTest.java @@ -27,4 +27,10 @@ public class IndexProducerFromHasherTest extends AbstractIndexProducerTest { protected IndexProducer createEmptyProducer() { return NullHasher.INSTANCE.indices(Shape.fromKM(17, 72)); } + + @Override + protected int getBehaviour() { + // Hasher allows duplicates and may be unordered + return 0; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromIntArrayTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromIntArrayTest.java index 4755d24a8..2ad9ee5c1 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromIntArrayTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromIntArrayTest.java @@ -25,6 +25,12 @@ public class IndexProducerFromIntArrayTest extends AbstractIndexProducerTest { @Override protected IndexProducer createProducer() { - return IndexProducer.fromIndexArray(new int[] { 1, 2, 3, 4, 5 }); + return IndexProducer.fromIndexArray(new int[] { 6, 8, 1, 2, 4, 4, 5 }); + } + + @Override + protected int getBehaviour() { + // Delegates to the default asIndexArray which is distinct and ordered + return AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSimpleBloomFilterTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSimpleBloomFilterTest.java index 40fded6dd..e8da24a8d 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSimpleBloomFilterTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSimpleBloomFilterTest.java @@ -32,4 +32,10 @@ public class IndexProducerFromSimpleBloomFilterTest extends AbstractIndexProduce protected IndexProducer createEmptyProducer() { return new SimpleBloomFilter(shape); } + + @Override + protected int getBehaviour() { + // BloomFilter based on a bit map array will be distinct and ordered + return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSparseBloomFilterTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSparseBloomFilterTest.java index bd1e34836..59823f329 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSparseBloomFilterTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSparseBloomFilterTest.java @@ -33,4 +33,12 @@ public class IndexProducerFromSparseBloomFilterTest extends AbstractIndexProduce protected IndexProducer createEmptyProducer() { return new SparseBloomFilter(shape); } + + @Override + protected int getBehaviour() { + // A sparse BloomFilter will be distinct but it may not be ordered. + // Currently the ordered behaviour is asserted as the implementation uses + // an ordered TreeSet. This may change in the future. + return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherCollectionTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherCollectionTest.java index 99ef6f200..54eeec90d 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherCollectionTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherCollectionTest.java @@ -27,4 +27,13 @@ public class UniqueIndexProducerFromHasherCollectionTest extends AbstractIndexPr protected IndexProducer createEmptyProducer() { return new HasherCollection().uniqueIndices(Shape.fromKM(17, 72)); } + + @Override + protected int getBehaviour() { + // Note: + // Do not return FOR_EACH_DISTINCT | AS_ARRAY_DISTINCT. + // Despite this being a unique index test, the HasherCollection will return a unique + // index from each hasher. The result is there may still be duplicates. + return 0; + } } diff --git a/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherTest.java b/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherTest.java index 84c17b60f..94d13e4d9 100644 --- a/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherTest.java +++ b/src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherTest.java @@ -27,4 +27,10 @@ public class UniqueIndexProducerFromHasherTest extends AbstractIndexProducerTest protected IndexProducer createEmptyProducer() { return NullHasher.INSTANCE.indices(Shape.fromKM(17, 72)); } + + @Override + protected int getBehaviour() { + // Should be unique but may be unordered + return FOR_EACH_DISTINCT | AS_ARRAY_DISTINCT; + } }