Fix NullPointerException in FilterIterator.setNextObject()

Add missing test AbstractIteratorTest.testForEachRemaining()

src/changes/changes.xml

@@ -44,6 +44,7 @@
     <action type="fix" dev="ggregory" due-to="Elia Bertolina, Gary Gregory" issue="COLLECTIONS-815">Javadoc: Update ClosureUtils Javadoc to match runtime.</action>
     <action type="fix" dev="ggregory" due-to="Gary Gregory" issue="COLLECTIONS-815">Javadoc: Update ClosureUtils Javadoc to match runtime.</action>
     <action type="fix" dev="ggregory" due-to="Gary Gregory" issue="COLLECTIONS-777">Migrate to JUnit 5.</action>
+    <action type="fix" dev="ggregory" due-to="Gary Gregory">Fix NullPointerException in FilterIterator.setNextObject().</action>
     <!-- ADD -->
     <action type="add" dev="ggregory" due-to="Gary Gregory">LayerManager.Builder implements Supplier.</action>
     <action type="add" dev="ggregory" due-to="Gary Gregory, hemanth0525">Add CollectionUtils.duplicateList(Collection).</action>
@@ -56,7 +57,8 @@
     <action type="add" issue="COLLECTIONS-700" dev="ggregory" due-to="Gary Gregory">Add ConcurrentReferenceHashMap.</action>
     <action type="add" dev="ggregory" due-to="Gary Gregory">Add commons.easymock.version to parameterize EasyMock version.</action>
     <action type="add" issue="COLLECTIONS-869" dev="ggregory" due-to="Gary Gregory">Add org.apache.commons.collections4.IteratorUtils.chainedIterator(Iterator&lt;? extends Iterator&lt;? extends E&gt;&gt;).</action>
-    <action type="add" dev="ggregory" due-to="Peter De Maeyer" issue="COLLECTIONS-533">Add ArrayListValuedLinkedHashMap #560.</action> 
+    <action type="add" dev="ggregory" due-to="Peter De Maeyer" issue="COLLECTIONS-533">Add ArrayListValuedLinkedHashMap #560.</action>
+    <action type="fix" dev="ggregory" due-to="Gary Gregory">Add missing test AbstractIteratorTest.testForEachRemaining().</action> 
     <!-- UPDATE -->
     <action type="update" dev="ggregory" due-to="Gary Gregory">Bump org.apache.commons:commons-parent from 71 to 78 #534, #545, #550 #555, #566.</action>
     <action type="update" issue="COLLECTIONS-857" dev="ggregory" due-to="Claude Warren">Update bloom filter documentation #508.</action>

src/main/java/org/apache/commons/collections4/iterators/FilterIterator.java

@@ -20,6 +20,7 @@
 import java.util.NoSuchElementException;
 
 import org.apache.commons.collections4.Predicate;
+import org.apache.commons.collections4.functors.TruePredicate;
 
 /**
  * Decorates an {@link Iterator} using an optional predicate to filter elements.
@@ -37,7 +38,7 @@ public class FilterIterator<E> implements Iterator<E> {
     private Iterator<? extends E> iterator;
 
     /** The predicate to filter elements. */
-    private Predicate<? super E> predicate;
+    private Predicate<? super E> predicate = TruePredicate.truePredicate();
 
     /** The next object in the iteration. */
     private E nextObject;
@@ -67,11 +68,11 @@ public FilterIterator(final Iterator<? extends E> iterator) {
      * given iterator and predicate.
      *
      * @param iterator  the iterator to use
-     * @param predicate  the predicate to use
+     * @param predicate  the predicate to use, null accepts all values.
      */
     public FilterIterator(final Iterator<? extends E> iterator, final Predicate<? super E> predicate) {
         this.iterator = iterator;
-        this.predicate = predicate;
+        this.predicate = safePredicate(predicate);
     }
 
     /**
@@ -140,6 +141,10 @@ public void remove() {
         iterator.remove();
     }
 
+    private Predicate<? super E> safePredicate(final Predicate<? super E> predicate) {
+        return predicate != null ? predicate : TruePredicate.truePredicate();
+    }
+
     /**
      * Sets the iterator for this iterator to use.
      * If iteration has started, this effectively resets the iterator.
@@ -169,12 +174,12 @@ private boolean setNextObject() {
     }
 
     /**
-     * Sets the predicate this the iterator to use.
+     * Sets the predicate this the iterator to use where null accepts all values.
      *
-     * @param predicate  the predicate to use
+     * @param predicate  the predicate to use, null accepts all values.
      */
     public void setPredicate(final Predicate<? super E> predicate) {
-        this.predicate = predicate;
+        this.predicate = safePredicate(predicate);
         nextObject = null;
         nextObjectSet = false;
     }

src/test/java/org/apache/commons/collections4/iterators/AbstractIteratorTest.java

@@ -17,15 +17,19 @@
 package org.apache.commons.collections4.iterators;
 
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.List;
 import java.util.NoSuchElementException;
 
 import org.apache.commons.collections4.AbstractObjectTest;
+import org.apache.commons.collections4.IteratorUtils;
 import org.junit.jupiter.api.Test;
 
 /**
@@ -117,6 +121,18 @@ public void testEmptyIterator() {
         assertNotNull(it.toString());
     }
 
+    /**
+     * Tests {@link Iterator#forEachRemaining(java.util.function.Consumer)}.
+     */
+    @Test
+    public void testForEachRemaining() {
+        final List<E> expected = IteratorUtils.toList(makeObject());
+        final Iterator<E> it = makeObject();
+        final List<E> actual = new ArrayList<>();
+        it.forEachRemaining(actual::add);
+        assertEquals(expected, actual);
+    }
+
     /**
      * Test normal iteration behavior.
      */

src/test/java/org/apache/commons/collections4/iterators/FilterIteratorTest.java

@@ -28,7 +28,9 @@
 import java.util.List;
 import java.util.NoSuchElementException;
 
+import org.apache.commons.collections4.IteratorUtils;
 import org.apache.commons.collections4.Predicate;
+import org.apache.commons.collections4.functors.FalsePredicate;
 import org.apache.commons.collections4.functors.NotNullPredicate;
 import org.apache.commons.collections4.functors.TruePredicate;
 import org.apache.commons.lang3.ArrayUtils;
@@ -121,6 +123,34 @@ public void tearDown() throws Exception {
         iterator = null;
     }
 
+    @Test
+    public void testForEachRemainingAcceptAllCtor() {
+        final List<E> expected = IteratorUtils.toList(makeObject());
+        final FilterIterator<E> it = new FilterIterator<>(makeObject(), TruePredicate.truePredicate());
+        final List<E> actual = new ArrayList<>();
+        it.forEachRemaining(actual::add);
+        assertEquals(expected, actual);
+    }
+
+    @Test
+    public void testForEachRemainingDefaultCtor() {
+        final List<E> expected = IteratorUtils.toList(makeObject());
+        final FilterIterator<E> it = new FilterIterator<>();
+        it.setIterator(expected.iterator());
+        final List<E> actual = new ArrayList<>();
+        it.forEachRemaining(actual::add);
+        assertEquals(expected, actual);
+    }
+
+    @Test
+    public void testForEachRemainingRejectAllCtor() {
+        final List<E> expected = IteratorUtils.toList(makeObject());
+        final FilterIterator<E> it = new FilterIterator<>(makeObject(), FalsePredicate.falsePredicate());
+        final List<E> actual = new ArrayList<>();
+        it.forEachRemaining(actual::add);
+        assertTrue(actual.isEmpty());
+    }
+
     @Test
     public void testRepeatedHasNext() {
         for (int i = 0; i <= array.length; i++) {
@@ -149,6 +179,7 @@ public void testReturnValues() {
         verifyElementsInPredicate(new String[] { "a", "b", "c" });
     }
 
+
     /**
      * Test that when the iterator is changed, the hasNext method returns the
      * correct response for the new iterator.

src/test/java/org/apache/commons/collections4/iterators/ObjectGraphIteratorTest.java

@@ -148,6 +148,7 @@ public ObjectGraphIterator<Object> makeEmptyIterator() {
 
     @Override
     public ObjectGraphIterator<Object> makeObject() {
+        setUp();
         return new ObjectGraphIterator<>(iteratorList.iterator());
     }