diff --git a/common/build.gradle b/common/build.gradle index 1fb575bc1..e23f034a8 100644 --- a/common/build.gradle +++ b/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' diff --git a/common/src/test/java/apoc/util/collection/AbstractResourceIterableTest.java b/common/src/test/java/apoc/util/collection/AbstractResourceIterableTest.java new file mode 100644 index 000000000..767950aa1 --- /dev/null +++ b/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() { + @Override + protected ResourceIterator 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() { + @Override + protected ResourceIterator newIterator() { + iteratorCount.increment(); + return resourceIterator( asIterator(0), Resource.EMPTY); + } + + @Override + protected void onClosed() { + iterableClosed.setTrue(); + } + }; + + final var iterators = new ArrayList>(); + 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() { + private int created; + + @Override + protected ResourceIterator 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() { + @Override + protected ResourceIterator 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() { + @Override + protected ResourceIterator 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() { + @Override + protected ResourceIterator 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) mock(Iterator.class); + when(intIterator.hasNext()).thenReturn(true).thenReturn(true); + when(intIterator.next()).thenReturn(1).thenThrow(IllegalStateException.class); + + // When + final var iterable = new AbstractResourceIterable() { + @Override + protected ResourceIterator newIterator() { + return resourceIterator(intIterator, iteratorClosed::setTrue); + } + + @Override + protected void onClosed() { + iterableClosed.setTrue(); + } + }; + + // Then + final var emitted = new ArrayList(); + 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() { + @Override + protected ResourceIterator newIterator() { + return resourceIterator(items.iterator(), iteratorClosed::setTrue); + } + + @Override + protected void onClosed() { + iterableClosed.setTrue(); + } + }; + + final var emitted = new ArrayList(); + 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() { + @Override + protected ResourceIterator newIterator() { + return resourceIterator; + } + + @Override + protected void onClosed() { + iterableClosed.setTrue(); + } + }; + + // When + try (Stream 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() { + @Override + protected ResourceIterator newIterator() { + return resourceIterator; + } + }; + + // When + final var result = iterable.stream().toList(); + + // Then + assertThat(result).isEqualTo(asList(1, 2, 3)); + assertThat(closed.intValue()).isEqualTo(2); + } +} diff --git a/common/src/test/java/apoc/util/collection/CollectionTestHelper.java b/common/src/test/java/apoc/util/collection/CollectionTestHelper.java new file mode 100644 index 000000000..962023d90 --- /dev/null +++ b/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 ResourceIterator resourceIterator(final Iterator 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 ResourceIterator emptyResourceIterator() { + return (ResourceIterator) EmptyResourceIterator.EMPTY_RESOURCE_ITERATOR; + } + + private static class EmptyResourceIterator implements ResourceIterator { + private static final ResourceIterator 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 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++]; + } + }; + } +} diff --git a/common/src/test/java/apoc/util/collection/IterablesTest.java b/common/src/test/java/apoc/util/collection/IterablesTest.java new file mode 100644 index 000000000..615315bfb --- /dev/null +++ b/common/src/test/java/apoc/util/collection/IterablesTest.java @@ -0,0 +1,93 @@ +package apoc.util.collection; + +import static apoc.util.collection.CollectionTestHelper.emptyResourceIterator; +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 org.apache.commons.lang3.mutable.MutableBoolean; +import org.junit.Test; +import org.neo4j.graphdb.ResourceIterator; + +public class IterablesTest { + + @Test + public void count() { + // Given + final var subjects = asList(1, 2, 3, 4, 5); + final var iteratorClosed = new MutableBoolean(false); + final var iterableClosed = new MutableBoolean(false); + final var iterable = new AbstractResourceIterable() { + @Override + protected ResourceIterator newIterator() { + return newResourceIterator(subjects.iterator(), iteratorClosed::setTrue); + } + + @Override + protected void onClosed() { + iterableClosed.setTrue(); + } + }; + + // when + long count = Iterables.count(iterable); + + // then + assertThat(count).isEqualTo(subjects.size()); + assertThat(iteratorClosed.isTrue()).isTrue(); + assertThat(iterableClosed.isTrue()).isTrue(); + } + + @Test + public void firstNoItems() { + // Given + final var iteratorClosed = new MutableBoolean(false); + final var iterableClosed = new MutableBoolean(false); + final var iterable = new AbstractResourceIterable() { + @Override + protected ResourceIterator newIterator() { + return newResourceIterator( emptyResourceIterator(), iteratorClosed::setTrue); + } + + @Override + protected void onClosed() { + iterableClosed.setTrue(); + } + }; + + // when + assertThatThrownBy(() -> Iterables.first(iterable)); + + // then + assertThat(iteratorClosed.isTrue()).isTrue(); + assertThat(iterableClosed.isTrue()).isTrue(); + } + + @Test + public void firstWithItems() { + // Given + final var subjects = asList(1, 2, 3, 4, 5); + final var iteratorClosed = new MutableBoolean(false); + final var iterableClosed = new MutableBoolean(false); + final var iterable = new AbstractResourceIterable() { + @Override + protected ResourceIterator newIterator() { + return newResourceIterator(subjects.iterator(), iteratorClosed::setTrue); + } + + @Override + protected void onClosed() { + iterableClosed.setTrue(); + } + }; + + // when + long first = Iterables.first(iterable); + + // then + assertThat(first).isEqualTo(1); + assertThat(iteratorClosed.isTrue()).isTrue(); + assertThat(iterableClosed.isTrue()).isTrue(); + } +} diff --git a/common/src/test/java/apoc/util/collection/ResourceClosingIteratorTest.java b/common/src/test/java/apoc/util/collection/ResourceClosingIteratorTest.java new file mode 100644 index 000000000..4e0feedff --- /dev/null +++ b/common/src/test/java/apoc/util/collection/ResourceClosingIteratorTest.java @@ -0,0 +1,38 @@ +package apoc.util.collection; + +import static apoc.util.collection.CollectionTestHelper.resourceIterator; +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import org.apache.commons.lang3.mutable.MutableBoolean; +import org.junit.Test; +import org.neo4j.graphdb.ResourceIterator; + +public class ResourceClosingIteratorTest { + @Test + public void fromResourceIterableShouldCloseParentIterable() { + 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() { + @Override + protected ResourceIterator newIterator() { + return resourceIterator(items.iterator(), iteratorClosed::setTrue); + } + + @Override + protected void onClosed() { + iterableClosed.setTrue(); + } + }; + ResourceIterator iterator = ResourceClosingIterator.fromResourceIterable(iterable); + + // Then + assertThat( Iterators.asList(iterator)).containsExactlyElementsOf(items); + assertThat(iteratorClosed.isTrue()).isTrue(); + assertThat(iterableClosed.isTrue()).isTrue(); + } +}