Skip to content

Commit

Permalink
COLLECTIONS-853: Change LayerManager to use List and added generics t…
Browse files Browse the repository at this point in the history 
…o LayerdedBloomFilter (#481)

* Added generics to LayeredBloomFilter, modified WrappedBloomfFilter to requrie copy() implementation and changed LayerManager LinkedList declaration to List.

* removed wildcard include

* Placed NumberedBloomFilter class into LayeredBloomFilterTest where it is used and fixed implementation

* made wrapped Bloom filter private with protected access member

* removed null checks from LayerManager first() and last()

* fixed generics

* removed LayerdBloomFilter.fixed()  methods

* changed to Deque in API

* fixed issue with advanceOnCount implementation
  • Loading branch information
@Claudenw
Claudenw committed May 12, 2024
1 parent bd8e950 commit 56da869
Show file tree
Hide file tree
Showing 10 changed files with 245 additions and 147 deletions.
2 changes: 1 addition & 1 deletion src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ default boolean contains(final Hasher hasher) {
* Creates a new instance of the BloomFilter with the same properties as the current one.
* @return a copy of this BloomFilter
*/
BloomFilter copy();
<T extends BloomFilter> T copy();

// update operations

Expand Down
115 changes: 75 additions & 40 deletions src/main/java/org/apache/commons/collections4/bloomfilter/LayerManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.commons.collections4.bloomfilter;

import java.util.Deque;
import java.util.LinkedList;
import java.util.NoSuchElementException;
import java.util.Objects;
Expand Down Expand Up @@ -50,15 +51,15 @@
*
* @since 4.5
*/
public class LayerManager implements BloomFilterProducer {
public class LayerManager<T extends BloomFilter> implements BloomFilterProducer {

/**
* Builder to create Layer Manager
*/
public static class Builder {
private Predicate<LayerManager> extendCheck;
private Supplier<BloomFilter> supplier;
private Consumer<LinkedList<BloomFilter>> cleanup;
public static class Builder<T extends BloomFilter> {
private Predicate<LayerManager<T>> extendCheck;
private Supplier<T> supplier;
private Consumer<Deque<T>> cleanup;

private Builder() {
extendCheck = ExtendCheck.neverAdvance();
Expand All @@ -70,11 +71,11 @@ private Builder() {
*
* @return a new LayerManager.
*/
public LayerManager build() {
public LayerManager<T> build() {
Objects.requireNonNull(supplier, "Supplier must not be null");
Objects.requireNonNull(extendCheck, "ExtendCheck must not be null");
Objects.requireNonNull(cleanup, "Cleanup must not be null");
return new LayerManager(supplier, extendCheck, cleanup, true);
return new LayerManager<>(supplier, extendCheck, cleanup, true);
}

/**
Expand All @@ -84,7 +85,7 @@ public LayerManager build() {
* dated or stale filters.
* @return this
*/
public Builder setCleanup(Consumer<LinkedList<BloomFilter>> cleanup) {
public Builder<T> setCleanup(Consumer<Deque<T>> cleanup) {
this.cleanup = cleanup;
return this;
}
Expand All @@ -97,7 +98,7 @@ public Builder setCleanup(Consumer<LinkedList<BloomFilter>> cleanup) {
* created.
* @return this for chaining.
*/
public Builder setExtendCheck(Predicate<LayerManager> extendCheck) {
public Builder<T> setExtendCheck(Predicate<LayerManager<T>> extendCheck) {
this.extendCheck = extendCheck;
return this;
}
Expand All @@ -109,22 +110,22 @@ public Builder setExtendCheck(Predicate<LayerManager> extendCheck) {
* @param supplier The supplier of new Bloom filter instances.
* @return this for chaining.
*/
public Builder setSupplier(Supplier<BloomFilter> supplier) {
public Builder<T> setSupplier(Supplier<T> supplier) {
this.supplier = supplier;
return this;
}
}

/**
* Static methods to create a Consumer of a LinkedList of BloomFilter perform
* Static methods to create a Consumer of a List of BloomFilter perform
* tests on whether to reduce the collection of Bloom filters.
*/
public static final class Cleanup {
/**
* A Cleanup that never removes anything.
* @return A Consumer suitable for the LayerManager {@code cleanup} parameter.
*/
public static Consumer<LinkedList<BloomFilter>> noCleanup() {
public static <T extends BloomFilter> Consumer<Deque<T>> noCleanup() {
return x -> {};
}

Expand All @@ -137,7 +138,7 @@ public static Consumer<LinkedList<BloomFilter>> noCleanup() {
* @return A Consumer suitable for the LayerManager {@code cleanup} parameter.
* @throws IllegalArgumentException if {@code maxSize <= 0}.
*/
public static Consumer<LinkedList<BloomFilter>> onMaxSize(int maxSize) {
public static <T extends BloomFilter> Consumer<Deque<T>> onMaxSize(int maxSize) {
if (maxSize <= 0) {
throw new IllegalArgumentException("'maxSize' must be greater than 0");
}
Expand All @@ -154,14 +155,24 @@ public static Consumer<LinkedList<BloomFilter>> onMaxSize(int maxSize) {
*
* @return A Consumer suitable for the LayerManager {@code cleanup} parameter.
*/
public static Consumer<LinkedList<BloomFilter>> removeEmptyTarget() {
public static <T extends BloomFilter> Consumer<Deque<T>> removeEmptyTarget() {
return x -> {
if (x.getLast().cardinality() == 0) {
if (!x.isEmpty() && x.getLast().isEmpty()) {
x.removeLast();
}
};
}

/**
* Removes any layer identified by the predicate.
*
* @param test Predicate.
* @return A Consumer suitable for the LayerManager {@code cleanup} parameter.
*/
public static <T extends BloomFilter> Consumer<Deque<T>> removeIf(Predicate<? super T> test) {
return x -> x.removeIf(test);
}

private Cleanup() {
}
}
Expand All @@ -179,16 +190,20 @@ public static final class ExtendCheck {
* @return A Predicate suitable for the LayerManager {@code extendCheck} parameter.
* @throws IllegalArgumentException if {@code breakAt <= 0}
*/
public static Predicate<LayerManager> advanceOnCount(int breakAt) {
public static <T extends BloomFilter> Predicate<LayerManager<T>> advanceOnCount(int breakAt) {
if (breakAt <= 0) {
throw new IllegalArgumentException("'breakAt' must be greater than 0");
}
return new Predicate<LayerManager>() {
return new Predicate<LayerManager<T>>() {
int count;

@Override
public boolean test(LayerManager filter) {
return ++count % breakAt == 0;
public boolean test(LayerManager<T> filter) {
if (++count == breakAt) {
count = 0;
return true;
}
return false;
}
};
}
Expand All @@ -197,8 +212,8 @@ public boolean test(LayerManager filter) {
* Advances the target once a merge has been performed.
* @return A Predicate suitable for the LayerManager {@code extendCheck} parameter.
*/
public static Predicate<LayerManager> advanceOnPopulated() {
return lm -> !lm.filters.peekLast().isEmpty();
public static <T extends BloomFilter> Predicate<LayerManager<T>> advanceOnPopulated() {
return lm -> !lm.last().isEmpty();
}

/**
Expand All @@ -212,12 +227,12 @@ public static Predicate<LayerManager> advanceOnPopulated() {
* @return A Predicate suitable for the LayerManager {@code extendCheck} parameter.
* @throws IllegalArgumentException if {@code maxN <= 0}
*/
public static Predicate<LayerManager> advanceOnSaturation(double maxN) {
public static <T extends BloomFilter> Predicate<LayerManager<T>> advanceOnSaturation(double maxN) {
if (maxN <= 0) {
throw new IllegalArgumentException("'maxN' must be greater than 0");
}
return manager -> {
BloomFilter bf = manager.filters.peekLast();
BloomFilter bf = manager.last();
return maxN <= bf.getShape().estimateN(bf.cardinality());
};
}
Expand All @@ -227,7 +242,7 @@ public static Predicate<LayerManager> advanceOnSaturation(double maxN) {
* perform the advance.
* @return A Predicate suitable for the LayerManager {@code extendCheck} parameter.
*/
public static Predicate<LayerManager> neverAdvance() {
public static <T extends BloomFilter> Predicate<LayerManager<T>> neverAdvance() {
return x -> false;
}

Expand All @@ -242,15 +257,15 @@ private ExtendCheck() {
* @see ExtendCheck#neverAdvance()
* @see Cleanup#noCleanup()
*/
public static Builder builder() {
return new Builder();
public static <T extends BloomFilter> Builder<T> builder() {
return new Builder<>();
}
private final LinkedList<BloomFilter> filters = new LinkedList<>();
private final Consumer<LinkedList<BloomFilter>> filterCleanup;
private final LinkedList<T> filters = new LinkedList<>();
private final Consumer<Deque<T>> filterCleanup;

private final Predicate<LayerManager> extendCheck;
private final Predicate<LayerManager<T>> extendCheck;

private final Supplier<BloomFilter> filterSupplier;
private final Supplier<T> filterSupplier;

/**
* Constructor.
Expand All @@ -263,8 +278,8 @@ public static Builder builder() {
* list.
* @param initialize true if the filter list should be initialized.
*/
private LayerManager(Supplier<BloomFilter> filterSupplier, Predicate<LayerManager> extendCheck,
Consumer<LinkedList<BloomFilter>> filterCleanup, boolean initialize) {
private LayerManager(Supplier<T> filterSupplier, Predicate<LayerManager<T>> extendCheck,
Consumer<Deque<T>> filterCleanup, boolean initialize) {
this.filterSupplier = filterSupplier;
this.extendCheck = extendCheck;
this.filterCleanup = filterCleanup;
Expand All @@ -277,7 +292,7 @@ private LayerManager(Supplier<BloomFilter> filterSupplier, Predicate<LayerManage
* Adds a new Bloom filter to the list.
*/
private void addFilter() {
BloomFilter bf = filterSupplier.get();
T bf = filterSupplier.get();
if (bf == null) {
throw new NullPointerException("filterSupplier returned null.");
}
Expand All @@ -302,9 +317,9 @@ public final void clear() {
*
* @return a copy of this layer Manager.
*/
public LayerManager copy() {
LayerManager newMgr = new LayerManager(filterSupplier, extendCheck, filterCleanup, false);
for (BloomFilter bf : filters) {
public LayerManager<T> copy() {
LayerManager<T> newMgr = new LayerManager<>(filterSupplier, extendCheck, filterCleanup, false);
for (T bf : filters) {
newMgr.filters.add(bf.copy());
}
return newMgr;
Expand Down Expand Up @@ -337,7 +352,7 @@ public boolean forEachBloomFilter(Predicate<BloomFilter> bloomFilterPredicate) {
* @throws NoSuchElementException if depth is not in the range
* [0,filters.size())
*/
public final BloomFilter get(int depth) {
public final T get(int depth) {
if (depth < 0 || depth >= filters.size()) {
throw new NoSuchElementException(String.format("Depth must be in the range [0,%s)", filters.size()));
}
Expand All @@ -346,25 +361,45 @@ public final BloomFilter get(int depth) {

/**
* Returns the number of filters in the LayerManager. In the default LayerManager implementation
* there is alwasy at least one layer.
* there is always at least one layer.
*
* @return the current depth.
*/
public final int getDepth() {
return filters.size();
}

/**
* Gets the Bloom filter from the first layer.
* No extension check is performed during this call.
* @return The Bloom filter from the first layer.
* @see #getTarget()
*/
public final T first() {
return filters.getFirst();
}

/**
* Gets the Bloom filter from the last layer.
* No extension check is performed during this call.
* @return The Bloom filter from the last layer.
* @see #getTarget()
*/
public final T last() {
return filters.getLast();
}

/**
* Returns the current target filter. If a new filter should be created based on
* {@code extendCheck} it will be created before this method returns.
*
* @return the current target filter after any extension.
*/
public final BloomFilter getTarget() {
public final T getTarget() {
if (extendCheck.test(this)) {
next();
}
return filters.peekLast();
return last();
}

/**
Expand Down
27 changes: 7 additions & 20 deletions src/main/java/org/apache/commons/collections4/bloomfilter/LayeredBloomFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,10 @@
* 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
*/
public class LayeredBloomFilter implements BloomFilter, BloomFilterProducer {
public class LayeredBloomFilter<T extends BloomFilter> implements BloomFilter, BloomFilterProducer {
/**
* A class used to locate matching filters across all the layers.
*/
Expand All @@ -88,32 +89,18 @@ public boolean test(BloomFilter x) {
return true;
}
}
/**
* Creates a fixed size layered bloom filter that adds new filters to the list,
* but never merges them. List will never exceed maxDepth. As additional filters
* are added earlier filters are removed.
*
* @param shape The shape for the enclosed Bloom filters.
* @param maxDepth The maximum depth of layers.
* @return An empty layered Bloom filter of the specified shape and depth.
*/
public static LayeredBloomFilter fixed(final Shape shape, int maxDepth) {
LayerManager manager = LayerManager.builder().setExtendCheck(LayerManager.ExtendCheck.advanceOnPopulated())
.setCleanup(LayerManager.Cleanup.onMaxSize(maxDepth)).setSupplier(() -> new SimpleBloomFilter(shape)).build();
return new LayeredBloomFilter(shape, manager);
}

private final Shape shape;

private LayerManager layerManager;
private final LayerManager<T> layerManager;

/**
* Constructor.
*
* @param shape the Shape of the enclosed Bloom filters
* @param layerManager the LayerManager to manage the layers.
*/
public LayeredBloomFilter(Shape shape, LayerManager layerManager) {
public LayeredBloomFilter(Shape shape, LayerManager<T> layerManager) {
this.shape = shape;
this.layerManager = layerManager;
}
Expand Down Expand Up @@ -184,8 +171,8 @@ public boolean contains(IndexProducer indexProducer) {
}

@Override
public LayeredBloomFilter copy() {
return new LayeredBloomFilter(shape, layerManager.copy());
public LayeredBloomFilter<T> copy() {
return new LayeredBloomFilter<>(shape, layerManager.copy());
}

/**
Expand Down Expand Up @@ -329,7 +316,7 @@ public boolean forEachIndex(IntPredicate predicate) {
* @return the Bloom filter at the specified depth.
* @throws NoSuchElementException if depth is not in the range [0,getDepth())
*/
public BloomFilter get(int depth) {
public T get(int depth) {
return layerManager.get(depth);
}

Expand Down

0 comments on commit 56da869

Please sign in to comment.