Skip to content

Commit

Permalink
Fix generic typing for BloomFilter.copy()
Browse files Browse the repository at this point in the history
Avoids guaranteed exceptions. For example:

SparseBloomFilter filter = new SimpleBloomFilter(Shape.fromNP(10000,
0.01)).copy();

After this commit, this type of broken code won't even compile.
  • Loading branch information
garydgregory committed Oct 18, 2024
1 parent 32a0b77 commit 5479a7d
Show file tree
Hide file tree
Showing 16 changed files with 171 additions and 142 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ public boolean contains(final IndexExtractor indexExtractor) {
return indexExtractor.processIndices(idx -> cells[idx] != 0);
}

/**
* Creates a new instance of this {@link ArrayCountingBloomFilter} with the same properties as the current one.
*
* @return a copy of this BloomFilter.
*/
@Override
public ArrayCountingBloomFilter copy() {
return new ArrayCountingBloomFilter(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@
* <p>
* <em>See implementation notes for {@link BitMapExtractor} and {@link IndexExtractor}.</em>
* </p>
*
* @param <T> The BloomFilter type.
* @see BitMapExtractor
* @see IndexExtractor
* @since 4.5.0
*/
public interface BloomFilter extends IndexExtractor, BitMapExtractor {
public interface BloomFilter<T extends BloomFilter<T>> extends IndexExtractor, BitMapExtractor {

/**
* The sparse characteristic used to determine the best method for matching.
Expand Down Expand Up @@ -84,7 +86,7 @@ default boolean contains(final BitMapExtractor bitMapExtractor) {
* @param other the other Bloom filter
* @return true if all enabled bits in the other filter are enabled in this filter.
*/
default boolean contains(final BloomFilter other) {
default boolean contains(final BloomFilter<?> other) {
Objects.requireNonNull(other, "other");
return (characteristics() & SPARSE) != 0 ? contains((IndexExtractor) other) : contains((BitMapExtractor) other);
}
Expand Down Expand Up @@ -117,12 +119,11 @@ default boolean contains(final Hasher hasher) {
boolean contains(IndexExtractor indexExtractor);

/**
* Creates a new instance of the BloomFilter with the same properties as the current one.
* Creates a new instance of this {@link BloomFilter} with the same properties as the current one.
*
* @param <T> Type of BloomFilter.
* @return a copy of this BloomFilter
* @return a copy of this {@link BloomFilter}.
*/
<T extends BloomFilter> T copy();
T copy();

// update operations

Expand All @@ -142,7 +143,7 @@ default boolean contains(final Hasher hasher) {
* @see #estimateN()
* @see Shape
*/
default int estimateIntersection(final BloomFilter other) {
default int estimateIntersection(final BloomFilter<?> other) {
Objects.requireNonNull(other, "other");
final double eThis = getShape().estimateN(cardinality());
final double eOther = getShape().estimateN(other.cardinality());
Expand All @@ -157,7 +158,7 @@ default int estimateIntersection(final BloomFilter other) {
} else if (Double.isInfinite(eOther)) {
estimate = Math.round(eThis);
} else {
final BloomFilter union = this.copy();
final T union = this.copy();
union.merge(other);
final double eUnion = getShape().estimateN(union.cardinality());
if (Double.isInfinite(eUnion)) {
Expand Down Expand Up @@ -220,11 +221,11 @@ default int estimateN() {
* @see #estimateN()
* @see Shape
*/
default int estimateUnion(final BloomFilter other) {
default int estimateUnion(final BloomFilter<?> other) {
Objects.requireNonNull(other, "other");
final BloomFilter cpy = this.copy();
cpy.merge(other);
return cpy.estimateN();
final T copy = this.copy();
copy.merge(other);
return copy.estimateN();
}

/**
Expand Down Expand Up @@ -290,7 +291,7 @@ default boolean isFull() {
* @param other The bloom filter to merge into this one.
* @return true if the merge was successful
*/
default boolean merge(final BloomFilter other) {
default boolean merge(final BloomFilter<?> other) {
return (characteristics() & SPARSE) != 0 ? merge((IndexExtractor) other) : merge((BitMapExtractor) other);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,14 @@ public interface BloomFilterExtractor {
* </ul>
* <p><em>All modifications to the Bloom filters are reflected in the original filters</em></p>
*
* @param <T> The BloomFilter type.
* @param filters The filters to be returned by the extractor.
* @return THe BloomFilterExtractor containing the filters.
*/
static BloomFilterExtractor fromBloomFilterArray(final BloomFilter... filters) {
static <T extends BloomFilter<T>> BloomFilterExtractor fromBloomFilterArray(final BloomFilter<?>... filters) {
Objects.requireNonNull(filters, "filters");
return new BloomFilterExtractor() {

/**
* This implementation returns a copy the original array, the contained Bloom filters
* are references to the originals, any modifications to them are reflected in the original
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
* @see CellExtractor
* @since 4.5.0
*/
public interface CountingBloomFilter extends BloomFilter, CellExtractor {
public interface CountingBloomFilter extends BloomFilter<CountingBloomFilter>, CellExtractor {

// Query Operations

Expand All @@ -74,13 +74,6 @@ public interface CountingBloomFilter extends BloomFilter, CellExtractor {
*/
boolean add(CellExtractor other);

/**
* Creates a new instance of the CountingBloomFilter with the same properties as the current one.
* @return a copy of this CountingBloomFilter
*/
@Override
CountingBloomFilter copy();

/**
* Returns the maximum allowable value for a cell count in this Counting filter.
* @return the maximum allowable value for a cell count in this Counting filter.
Expand Down Expand Up @@ -114,7 +107,7 @@ default int getMaxInsert(final BitMapExtractor bitMapExtractor) {
* @param bloomFilter the Bloom filter the check for.
* @return the maximum number of times the Bloom filter could have been inserted.
*/
default int getMaxInsert(final BloomFilter bloomFilter) {
default int getMaxInsert(final BloomFilter<?> bloomFilter) {
return getMaxInsert((BitMapExtractor) bloomFilter);
}

Expand Down Expand Up @@ -204,7 +197,7 @@ default boolean merge(final BitMapExtractor bitMapExtractor) {
* @see #add(CellExtractor)
*/
@Override
default boolean merge(final BloomFilter other) {
default boolean merge(final BloomFilter<?> other) {
Objects.requireNonNull(other, "other");
return merge((IndexExtractor) other);
}
Expand Down Expand Up @@ -288,7 +281,7 @@ default boolean remove(final BitMapExtractor bitMapExtractor) {
* @see #isValid()
* @see #subtract(CellExtractor)
*/
default boolean remove(final BloomFilter other) {
default boolean remove(final BloomFilter<?> other) {
return remove((IndexExtractor) other);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@
* @param <T> the {@link BloomFilter} type.
* @since 4.5.0
*/
public class LayerManager<T extends BloomFilter> implements BloomFilterExtractor {
public class LayerManager<T extends BloomFilter<T>> implements BloomFilterExtractor {

/**
* Builds new instances of {@link LayerManager}.
*
* @param <T> the {@link BloomFilter} type.
*/
public static class Builder<T extends BloomFilter> implements Supplier<LayerManager<T>> {
public static class Builder<T extends BloomFilter<T>> implements Supplier<LayerManager<T>> {

private Predicate<LayerManager<T>> extendCheck;
private Supplier<T> supplier;
Expand Down Expand Up @@ -131,7 +131,7 @@ public static final class Cleanup {
* @param <T> Type of BloomFilter.
* @return A Consumer suitable for the LayerManager {@code cleanup} parameter.
*/
public static <T extends BloomFilter> Consumer<Deque<T>> noCleanup() {
public static <T extends BloomFilter<T>> Consumer<Deque<T>> noCleanup() {
return x -> {
// empty
};
Expand All @@ -147,7 +147,7 @@ public static <T extends BloomFilter> Consumer<Deque<T>> noCleanup() {
* @return A Consumer suitable for the LayerManager {@code cleanup} parameter.
* @throws IllegalArgumentException if {@code maxSize <= 0}.
*/
public static <T extends BloomFilter> Consumer<Deque<T>> onMaxSize(final int maxSize) {
public static <T extends BloomFilter<T>> Consumer<Deque<T>> onMaxSize(final int maxSize) {
if (maxSize <= 0) {
throw new IllegalArgumentException("'maxSize' must be greater than 0");
}
Expand All @@ -165,7 +165,7 @@ public static <T extends BloomFilter> Consumer<Deque<T>> onMaxSize(final int max
* @param <T> Type of BloomFilter.
* @return A Consumer suitable for the LayerManager {@code cleanup} parameter.
*/
public static <T extends BloomFilter> Consumer<Deque<T>> removeEmptyTarget() {
public static <T extends BloomFilter<T>> Consumer<Deque<T>> removeEmptyTarget() {
return x -> {
if (!x.isEmpty() && x.getLast().isEmpty()) {
x.removeLast();
Expand All @@ -180,7 +180,7 @@ public static <T extends BloomFilter> Consumer<Deque<T>> removeEmptyTarget() {
* @param test Predicate.
* @return A Consumer suitable for the LayerManager {@code cleanup} parameter.
*/
public static <T extends BloomFilter> Consumer<Deque<T>> removeIf(final Predicate<? super T> test) {
public static <T extends BloomFilter<T>> Consumer<Deque<T>> removeIf(final Predicate<? super T> test) {
return x -> x.removeIf(test);
}

Expand All @@ -202,7 +202,7 @@ public static final class ExtendCheck {
* @return A Predicate suitable for the LayerManager {@code extendCheck} parameter.
* @throws IllegalArgumentException if {@code breakAt <= 0}
*/
public static <T extends BloomFilter> Predicate<LayerManager<T>> advanceOnCount(final int breakAt) {
public static <T extends BloomFilter<T>> Predicate<LayerManager<T>> advanceOnCount(final int breakAt) {
if (breakAt <= 0) {
throw new IllegalArgumentException("'breakAt' must be greater than 0");
}
Expand All @@ -226,7 +226,7 @@ public boolean test(final LayerManager<T> filter) {
* @param <T> Type of BloomFilter.
* @return A Predicate suitable for the LayerManager {@code extendCheck} parameter.
*/
public static <T extends BloomFilter> Predicate<LayerManager<T>> advanceOnPopulated() {
public static <T extends BloomFilter<T>> Predicate<LayerManager<T>> advanceOnPopulated() {
return lm -> !lm.last().isEmpty();
}

Expand All @@ -242,12 +242,12 @@ public static <T extends BloomFilter> Predicate<LayerManager<T>> advanceOnPopula
* @return A Predicate suitable for the LayerManager {@code extendCheck} parameter.
* @throws IllegalArgumentException if {@code maxN <= 0}
*/
public static <T extends BloomFilter> Predicate<LayerManager<T>> advanceOnSaturation(final double maxN) {
public static <T extends BloomFilter<T>> Predicate<LayerManager<T>> advanceOnSaturation(final double maxN) {
if (maxN <= 0) {
throw new IllegalArgumentException("'maxN' must be greater than 0");
}
return manager -> {
final BloomFilter bf = manager.last();
final T bf = manager.last();
return maxN <= bf.getShape().estimateN(bf.cardinality());
};
}
Expand All @@ -259,13 +259,14 @@ public static <T extends BloomFilter> Predicate<LayerManager<T>> advanceOnSatura
* @param <T> Type of BloomFilter.
* @return A Predicate suitable for the LayerManager {@code extendCheck} parameter.
*/
public static <T extends BloomFilter> Predicate<LayerManager<T>> neverAdvance() {
public static <T extends BloomFilter<T>> Predicate<LayerManager<T>> neverAdvance() {
return x -> false;
}

private ExtendCheck() {
}
}

/**
* Creates a new Builder with defaults of {@code ExtendCheck.neverAdvance()} and
* {@code Cleanup.noCleanup()}.
Expand All @@ -275,7 +276,7 @@ private ExtendCheck() {
* @see ExtendCheck#neverAdvance()
* @see Cleanup#noCleanup()
*/
public static <T extends BloomFilter> Builder<T> builder() {
public static <T extends BloomFilter<T>> Builder<T> builder() {
return new Builder<>();
}

Expand Down Expand Up @@ -337,13 +338,15 @@ public final void clear() {
}

/**
* Creates a deep copy of this LayerManager.
* <p><em>Filters in the copy are deep copies, not references, so changes in the copy
* are NOT reflected in the original.</em></p>
* <p>The {@code filterSupplier}, {@code extendCheck}, and the {@code filterCleanup} are shared between
* the copy and this instance.</p>
* Creates a deep copy of this {@link LayerManager}.
* <p>
* <em>Filters in the copy are deep copies, not references, so changes in the copy are NOT reflected in the original.</em>
* </p>
* <p>
* The {@code filterSupplier}, {@code extendCheck}, and the {@code filterCleanup} are shared between the copy and this instance.
* </p>
*
* @return a copy of this layer Manager.
* @return a copy of this {@link LayerManager}.
*/
public LayerManager<T> copy() {
final LayerManager<T> newMgr = new LayerManager<>(filterSupplier, extendCheck, filterCleanup, false);
Expand Down Expand Up @@ -438,7 +441,7 @@ void next() {
*/
@Override
public boolean processBloomFilters(final Predicate<BloomFilter> bloomFilterPredicate) {
for (final BloomFilter bf : filters) {
for (final T bf : filters) {
if (!bloomFilterPredicate.test(bf)) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,20 +59,21 @@
* removes them. It also checks it a new layer should be added, and if so adds
* it and sets the {@code target} before the operation.</li>
* </ul>
*
* @param <T> The type of Bloom Filter that is used for the layers.
* @since 4.5.0
*/
public class LayeredBloomFilter<T extends BloomFilter> implements BloomFilter, BloomFilterExtractor {
public class LayeredBloomFilter<T extends BloomFilter<T>> implements BloomFilter<LayeredBloomFilter<T>>, BloomFilterExtractor {
/**
* A class used to locate matching filters across all the layers.
*/
private class Finder implements Predicate<BloomFilter> {
int[] result = new int[layerManager.getDepth()];
int bfIdx;
int resultIdx;
BloomFilter bf;
BloomFilter<?> bf;

Finder(final BloomFilter bf) {
Finder(final BloomFilter<?> bf) {
this.bf = bf;
}

Expand Down Expand Up @@ -180,6 +181,11 @@ public boolean contains(final IndexExtractor indexExtractor) {
return contains(createFilter(indexExtractor));
}

/**
* Creates a new instance of this {@link LayeredBloomFilter} with the same properties as the current one.
*
* @return a copy of this {@link LayeredBloomFilter}.
*/
@Override
public LayeredBloomFilter<T> copy() {
return new LayeredBloomFilter<>(shape, layerManager.copy());
Expand All @@ -191,7 +197,7 @@ public LayeredBloomFilter<T> copy() {
* @param bitMapExtractor the BitMapExtractor to create the filter from.
* @return the BloomFilter.
*/
private BloomFilter createFilter(final BitMapExtractor bitMapExtractor) {
private SimpleBloomFilter createFilter(final BitMapExtractor bitMapExtractor) {
final SimpleBloomFilter bf = new SimpleBloomFilter(shape);
bf.merge(bitMapExtractor);
return bf;
Expand All @@ -203,7 +209,7 @@ private BloomFilter createFilter(final BitMapExtractor bitMapExtractor) {
* @param hasher the hasher to create the filter from.
* @return the BloomFilter.
*/
private BloomFilter createFilter(final Hasher hasher) {
private SimpleBloomFilter createFilter(final Hasher hasher) {
final SimpleBloomFilter bf = new SimpleBloomFilter(shape);
bf.merge(hasher);
return bf;
Expand All @@ -215,7 +221,7 @@ private BloomFilter createFilter(final Hasher hasher) {
* @param indexExtractor the IndexExtractor to create the filter from.
* @return the BloomFilter.
*/
private BloomFilter createFilter(final IndexExtractor indexExtractor) {
private SimpleBloomFilter createFilter(final IndexExtractor indexExtractor) {
final SimpleBloomFilter bf = new SimpleBloomFilter(shape);
bf.merge(indexExtractor);
return bf;
Expand Down Expand Up @@ -289,8 +295,8 @@ public int[] find(final IndexExtractor indexExtractor) {
* @return the merged bloom filter.
*/
@Override
public BloomFilter flatten() {
final BloomFilter bf = new SimpleBloomFilter(shape);
public SimpleBloomFilter flatten() {
final SimpleBloomFilter bf = new SimpleBloomFilter(shape);
processBloomFilters(bf::merge);
return bf;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public static double cosineSimilarity(final BitMapExtractor first, final BitMapE
* @param second the second Bloom filter.
* @return the Cosine similarity.
*/
public static double cosineSimilarity(final BloomFilter first, final BloomFilter second) {
public static double cosineSimilarity(final BloomFilter<?> first, final BloomFilter<?> second) {
final int numerator = andCardinality(first, second);
// Given that the cardinality is an int then the product as a double will not
// overflow, we can use one sqrt:
Expand Down
Loading

0 comments on commit 5479a7d

Please sign in to comment.