Add tests for helper methods from Neo4j

common/build.gradle

@@ -84,6 +84,7 @@ dependencies {
     testImplementation group: 'org.neo4j', name: 'neo4j-kernel', version: neo4jVersionEffective, classifier: "tests"
     testImplementation group: 'org.assertj', name: 'assertj-core', version: '3.13.2'
     testImplementation group: 'org.mockito', name: 'mockito-core', version: '4.2.0'
+    testImplementation group: 'pl.pragmatists', name: 'JUnitParams', version: '1.1.1'
 
     configurations.all {
         exclude group: 'org.slf4j', module: 'slf4j-nop'

common/src/test/java/apoc/util/collection/AbstractResourceIterableTest.java

@@ -0,0 +1,320 @@
+package apoc.util.collection;
+
+import static apoc.util.collection.CollectionTestHelper.asIterator;
+import static apoc.util.collection.CollectionTestHelper.emptyResourceIterator;
+import static apoc.util.collection.CollectionTestHelper.resourceIterator;
+import static apoc.util.collection.ResourceClosingIterator.newResourceIterator;
+import static java.util.Arrays.asList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.stream.Stream;
+import junitparams.JUnitParamsRunner;
+import junitparams.Parameters;
+import org.apache.commons.lang3.mutable.MutableBoolean;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.neo4j.graphdb.Resource;
+import org.neo4j.graphdb.ResourceIterator;
+
+@RunWith(JUnitParamsRunner.class)
+public class AbstractResourceIterableTest {
+    @Test
+    public void shouldDelegateToUnderlyingIterableForData() {
+        // Given
+        final var iterableClosed = new MutableBoolean(false);
+        final var iteratorClosed = new MutableBoolean(false);
+
+        final var items = Arrays.asList(0, 1, 2);
+
+        // When
+        final var iterable = new AbstractResourceIterable<Integer>() {
+            @Override
+            protected ResourceIterator<Integer> newIterator() {
+                return resourceIterator(items.iterator(), iteratorClosed::setTrue);
+            }
+
+            @Override
+            protected void onClosed() {
+                iterableClosed.setTrue();
+            }
+        };
+        final var iterator = iterable.iterator();
+
+        // Then
+        assertThat( Iterators.asList(iterator)).containsExactlyElementsOf(items);
+        assertThat(iteratorClosed.isTrue()).isTrue();
+        assertThat(iterableClosed.isTrue()).isFalse();
+    }
+
+    @Test
+    @Parameters({"0", "1", "2", "3", "10"})
+    public void callToIteratorShouldCreateNewIterators(int numberOfIterators) {
+        // Given
+        final var iterableClosed = new MutableBoolean(false);
+        final var iteratorCount = new MutableInt();
+
+        // When
+        final var iterable = new AbstractResourceIterable<Integer>() {
+            @Override
+            protected ResourceIterator<Integer> newIterator() {
+                iteratorCount.increment();
+                return resourceIterator( asIterator(0), Resource.EMPTY);
+            }
+
+            @Override
+            protected void onClosed() {
+                iterableClosed.setTrue();
+            }
+        };
+
+        final var iterators = new ArrayList<ResourceIterator<Integer>>();
+        for (int i = 0; i < numberOfIterators; i++) {
+            iterators.add(iterable.iterator());
+        }
+        iterable.close();
+
+        // Then
+        assertThat(iterableClosed.isTrue()).isTrue();
+        assertThat(iteratorCount.getValue()).isEqualTo(numberOfIterators);
+        assertThat(iterators).containsOnlyOnceElementsOf(new HashSet<>(iterators));
+    }
+
+    @Test
+    public void shouldCloseAllIteratorsIfCloseCalledOnIterable() {
+        // Given
+        final var iteratorsClosed = Arrays.asList(false, false, false, false);
+
+        // When
+        final var iterable = new AbstractResourceIterable<Integer>() {
+            private int created;
+
+            @Override
+            protected ResourceIterator<Integer> newIterator() {
+                var pos = created;
+                created++;
+                return resourceIterator(
+                        asIterator(0), () -> iteratorsClosed.set(pos, true));
+            }
+        };
+        iterable.iterator();
+        iterable.iterator();
+        iterable.iterator();
+        iterable.close();
+
+        // Then
+        assertThat(iteratorsClosed.get(0)).isTrue();
+        assertThat(iteratorsClosed.get(1)).isTrue();
+        assertThat(iteratorsClosed.get(2)).isTrue();
+        assertThat(iteratorsClosed.get(3)).isFalse();
+    }
+
+    @Test
+    public void shouldCloseAllIteratorsEvenIfOnlySomeCloseCalled() {
+        // Given
+        final var iteratorsClosed = new MutableInt();
+
+        // When
+        final var iterable = new AbstractResourceIterable<Integer>() {
+            @Override
+            protected ResourceIterator<Integer> newIterator() {
+                return resourceIterator( asIterator(0), iteratorsClosed::increment);
+            }
+        };
+        final var iterator1 = iterable.iterator();
+        iterable.iterator();
+        final var iterator2 = iterable.iterator();
+        iterable.iterator();
+        final var iterator3 = iterable.iterator();
+        iterable.iterator();
+        iterable.iterator();
+
+        // go out of order
+        iterator3.close();
+        iterator1.close();
+        iterator2.close();
+        iterable.close();
+
+        // Then
+        assertThat(iteratorsClosed.getValue()).isEqualTo(7);
+    }
+
+    @Test
+    public void failIteratorCreationAfterIterableClosed() {
+        // Given
+        final var iteratorCreated = new MutableBoolean(false);
+
+        // When
+        final var iterable = new AbstractResourceIterable<Integer>() {
+            @Override
+            protected ResourceIterator<Integer> newIterator() {
+                iteratorCreated.setTrue();
+                return emptyResourceIterator();
+            }
+        };
+        iterable.close();
+
+        // Then
+        assertThatThrownBy(iterable::iterator);
+        assertThat(iteratorCreated.isTrue()).isFalse();
+    }
+
+    @Test
+    public void shouldCloseIteratorIfCloseCalled() {
+        // Given
+        final var iterableClosed = new MutableBoolean(false);
+        final var iteratorCreated = new MutableBoolean(false);
+        final var iteratorClosed = new MutableBoolean(false);
+
+        // When
+        final var iterable = new AbstractResourceIterable<Integer>() {
+            @Override
+            protected ResourceIterator<Integer> newIterator() {
+                iteratorCreated.setTrue();
+                return resourceIterator(List.of(0).iterator(), iteratorClosed::setTrue);
+            }
+
+            @Override
+            protected void onClosed() {
+                iterableClosed.setTrue();
+            }
+        };
+        assertThat(iterable.iterator().hasNext()).isTrue();
+        iterable.close();
+
+        // Then
+        assertThat(iteratorCreated.isTrue()).isTrue();
+        assertThat(iteratorClosed.isTrue()).isTrue();
+        assertThat(iterableClosed.isTrue()).isTrue();
+    }
+
+    @Test
+    public void shouldCloseIteratorOnForEachFailure() {
+        // Given
+        final var iterableClosed = new MutableBoolean(false);
+        final var iteratorClosed = new MutableBoolean(false);
+
+        @SuppressWarnings("unchecked")
+        final var intIterator = (Iterator<Integer>) mock(Iterator.class);
+        when(intIterator.hasNext()).thenReturn(true).thenReturn(true);
+        when(intIterator.next()).thenReturn(1).thenThrow(IllegalStateException.class);
+
+        // When
+        final var iterable = new AbstractResourceIterable<Integer>() {
+            @Override
+            protected ResourceIterator<Integer> newIterator() {
+                return resourceIterator(intIterator, iteratorClosed::setTrue);
+            }
+
+            @Override
+            protected void onClosed() {
+                iterableClosed.setTrue();
+            }
+        };
+
+        // Then
+        final var emitted = new ArrayList<Integer>();
+        assertThatThrownBy(() -> {
+            try (iterable) {
+                for (var item : iterable) {
+                    emitted.add(item);
+                }
+            }
+        });
+        assertThat(emitted).isEqualTo(List.of(1));
+        assertThat(iteratorClosed.isTrue()).isTrue();
+        assertThat(iterableClosed.isTrue()).isTrue();
+    }
+
+    @Test
+    public void shouldCloseIteratorOnForEachCompletion() {
+        // Given
+        final var iterableClosed = new MutableBoolean(false);
+        final var iteratorClosed = new MutableBoolean(false);
+
+        final var items = Arrays.asList(0, 1, 2);
+
+        // When
+        final var iterable = new AbstractResourceIterable<Integer>() {
+            @Override
+            protected ResourceIterator<Integer> newIterator() {
+                return resourceIterator(items.iterator(), iteratorClosed::setTrue);
+            }
+
+            @Override
+            protected void onClosed() {
+                iterableClosed.setTrue();
+            }
+        };
+
+        final var emitted = new ArrayList<Integer>();
+        for (var item : iterable) {
+            emitted.add(item);
+        }
+
+        // Then
+        assertThat(emitted).isEqualTo(items);
+        assertThat(iteratorClosed.isTrue()).isTrue();
+        assertThat(iterableClosed.isTrue()).isFalse();
+    }
+
+    @Test
+    public void streamShouldCloseIteratorAndIterable() {
+        // Given
+        final var iterableClosed = new MutableBoolean(false);
+        final var iteratorClosed = new MutableBoolean(false);
+        final var resourceIterator = newResourceIterator(asIterator(1, 2, 3), iteratorClosed::setTrue);
+
+        final var iterable = new AbstractResourceIterable<Integer>() {
+            @Override
+            protected ResourceIterator<Integer> newIterator() {
+                return resourceIterator;
+            }
+
+            @Override
+            protected void onClosed() {
+                iterableClosed.setTrue();
+            }
+        };
+
+        // When
+        try (Stream<Integer> stream = iterable.stream()) {
+            final var result = stream.toList();
+            assertThat(result).isEqualTo(asList(1, 2, 3));
+        }
+
+        // Then
+        assertThat(iterableClosed.isTrue()).isTrue();
+        assertThat(iteratorClosed.isTrue()).isTrue();
+    }
+
+    @Test
+    public void streamShouldCloseMultipleOnCompleted() {
+        // Given
+        final var closed = new MutableInt();
+        Resource resource = closed::incrementAndGet;
+        final var resourceIterator = newResourceIterator(asIterator(1, 2, 3), resource, resource);
+
+        final var iterable = new AbstractResourceIterable<Integer>() {
+            @Override
+            protected ResourceIterator<Integer> newIterator() {
+                return resourceIterator;
+            }
+        };
+
+        // When
+        final var result = iterable.stream().toList();
+
+        // Then
+        assertThat(result).isEqualTo(asList(1, 2, 3));
+        assertThat(closed.intValue()).isEqualTo(2);
+    }
+}

common/src/test/java/apoc/util/collection/CollectionTestHelper.java

@@ -0,0 +1,67 @@
+package apoc.util.collection;
+
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+import org.neo4j.graphdb.Resource;
+import org.neo4j.graphdb.ResourceIterator;
+
+/**
+ * Methods from Neo4js Iterators.java which are only needed in tests in APOC
+ */
+public class CollectionTestHelper
+{
+    public static <T> ResourceIterator<T> resourceIterator(final Iterator<T> iterator, final Resource resource) {
+        return new PrefetchingResourceIterator<>() {
+            @Override
+            public void close() {
+                resource.close();
+            }
+
+            @Override
+            protected T fetchNextOrNull() {
+                return iterator.hasNext() ? iterator.next() : null;
+            }
+        };
+    }
+    @SuppressWarnings("unchecked")
+    public static <T> ResourceIterator<T> emptyResourceIterator() {
+        return (ResourceIterator<T>) EmptyResourceIterator.EMPTY_RESOURCE_ITERATOR;
+    }
+
+    private static class EmptyResourceIterator<E> implements ResourceIterator<E> {
+        private static final ResourceIterator<Object> EMPTY_RESOURCE_ITERATOR = new EmptyResourceIterator<>();
+
+        @Override
+        public void close() {}
+
+        @Override
+        public boolean hasNext() {
+            return false;
+        }
+
+        @Override
+        public E next() {
+            throw new NoSuchElementException();
+        }
+    }
+
+    public static Iterator<Integer> asIterator(final int... array) {
+        return new Iterator<>() {
+            private int index;
+
+            @Override
+            public boolean hasNext() {
+                return index < array.length;
+            }
+
+            @Override
+            public Integer next() {
+                if (!hasNext()) {
+                    throw new NoSuchElementException();
+                }
+
+                return array[index++];
+            }
+        };
+    }
+}