TINKERPOP-1589 Re-introduced CloseableIterator Made it so that the CloseableIterator is closed by GraphStep if it is provided by the iterator supplier. Furthermore, any steps in a Traversal implement AutoCloseable then its close() method will be called.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/3597dc5f Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/3597dc5f Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/3597dc5f Branch: refs/heads/TINKERPOP-1545-tp32 Commit: 3597dc5f06407a8b1ada0e49a52b36b2f31b8a34 Parents: 3064b93 Author: Stephen Mallette <sp...@genoprime.com> Authored: Wed Jan 4 16:14:23 2017 -0500 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Wed Jan 4 16:14:23 2017 -0500 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../upgrade/release-3.2.x-incubating.asciidoc | 18 +++++ .../gremlin/process/traversal/Traversal.java | 7 +- .../process/traversal/step/map/GraphStep.java | 18 ++++- .../structure/util/CloseableIterator.java | 49 +++++++++++ .../util/DefaultCloseableIterator.java | 45 +++++++++++ .../process/traversal/TraversalTest.java | 44 +++++++++- .../structure/util/CloseableIteratorTest.java | 85 ++++++++++++++++++++ 8 files changed, 260 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 34e81c4..7316d80 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima TinkerPop 3.2.4 (Release Date: NOT OFFICIALLY RELEASED YET) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +* Added `CloseableIterator` to allow `Graph` providers who open expensive resources a way to let users release them. * Fixed minor bug in `gremlin-driver` where closing a session-based `Client` without initializing it could generate an error. * `TinkerGraph` Gryo and GraphSON deserialization is now configured to use multi-properties. * Changed behavior of `ElementHelper.areEqual(Property, Property)` to not throw exceptions with `null` arguments. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/docs/src/upgrade/release-3.2.x-incubating.asciidoc ---------------------------------------------------------------------- diff --git a/docs/src/upgrade/release-3.2.x-incubating.asciidoc b/docs/src/upgrade/release-3.2.x-incubating.asciidoc index 6dcb0a3..8d73baed 100644 --- a/docs/src/upgrade/release-3.2.x-incubating.asciidoc +++ b/docs/src/upgrade/release-3.2.x-incubating.asciidoc @@ -147,6 +147,24 @@ Upgrading for Providers Graph Database Providers ^^^^^^^^^^^^^^^^^^^^^^^^ +CloseableIterator ++++++++++++++++++ + +Prior to TinkerPop 3.x, Blueprints had the notion of a `CloseableIterable` which exposed a way for Graph Providers +to offer a way to release resources that might have been opened when returning vertices and edges. That interface was +never exposed in TinkerPop 3.x, but has now been made available via the new `CloseableIterator`. Providers may choose +to use this interface or not when returning values from `Graph.vertices()` and `Graph.edges()`. + +It will be up to users to know whether or not they need to call `close()`. Of course, users should typically not be +operating with the Graph Structure API, so it's unlikely that they would be calling these methods directly in the +first place. It is more likely that users will be calling `Traversal.close()`. This method will essentially iterate +the steps of the `Traversal` and simply call `close()` on any steps that implement `AutoCloseable`. By default, +`GraphStep` now implements `AutoCloseable` which most Graph Providers will extend upon (as was done with TinkerGraph's +`TinkerGraphStep`), so the integration should largely come for free if the provider simply returns a +`CloseableIterator` from `Graph.vertices()` and `Graph.edges()`. + +See: https://issues.apache.org/jira/browse/TINKERPOP-1589[TINKERPOP-1589] + HasContainer AndP Splitting +++++++++++++++++++++++++++ http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java index 04f5127..13a2b7d 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java @@ -254,9 +254,14 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable, A } } + /** + * Releases resources opened in any steps that implement {@link AutoCloseable}. + */ @Override public default void close() throws Exception { - // do nothing by default + for (AutoCloseable closeableStep : TraversalHelper.getStepsOfAssignableClassRecursively(AutoCloseable.class, asAdmin())) { + closeableStep.close(); + } } /** http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java index 3f169b0..87935d8 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java @@ -32,9 +32,12 @@ import org.apache.tinkerpop.gremlin.structure.Edge; import org.apache.tinkerpop.gremlin.structure.Element; import org.apache.tinkerpop.gremlin.structure.T; import org.apache.tinkerpop.gremlin.structure.Vertex; +import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator; import org.apache.tinkerpop.gremlin.structure.util.StringFactory; import org.apache.tinkerpop.gremlin.util.iterator.EmptyIterator; +import java.io.Closeable; +import java.io.IOException; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -45,7 +48,7 @@ import java.util.function.Supplier; * @author Marko A. Rodriguez (http://markorodriguez.com) * @author Pieter Martin */ -public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implements GraphComputing { +public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implements GraphComputing, AutoCloseable { protected final Class<E> returnClass; protected Object[] ids; @@ -151,7 +154,6 @@ public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implemen this.iterator = EmptyIterator.instance(); } - @Override public int hashCode() { int result = super.hashCode() ^ this.returnClass.hashCode(); @@ -162,6 +164,18 @@ public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implemen } /** + * Attemps to close an underlying iterator if it is of type {@link CloseableIterator}. Graph providers may choose + * to return this interface containing their vertices and edges if there are expensive resources that might need to + * be released at some point. + */ + @Override + public void close() throws Exception { + if (iterator != null && iterator instanceof CloseableIterator) { + ((CloseableIterator) iterator).close(); + } + } + + /** * Helper method for providers that want to "fold in" {@link HasContainer}'s based on id checking into the ids of the {@link GraphStep}. * * @param graphStep the GraphStep to potentially {@link GraphStep#addIds(Object...)}. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIterator.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIterator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIterator.java new file mode 100644 index 0000000..5f334a5 --- /dev/null +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIterator.java @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tinkerpop.gremlin.structure.util; + +import org.apache.tinkerpop.gremlin.structure.Graph; + +import java.io.Closeable; +import java.util.Iterator; + +/** + * An extension of {@code Iterator} that implements {@code Closeable} which allows a {@link Graph} implementation + * that hold open resources to provide the user the option to release those resources. + * + * @author Stephen Mallette (http://stephen.genoprime.com) + */ +public interface CloseableIterator<T> extends Iterator<T>, Closeable { + + /** + * Wraps an existing {@code Iterator} in a {@code CloseableIterator}. If the {@code Iterator} is already of that + * type then it will simply be returned as-is. + */ + public static <T> CloseableIterator<T> asCloseable(final Iterator<T> iterator) { + if (iterator instanceof CloseableIterator) + return (CloseableIterator<T>) iterator; + + return new DefaultCloseableIterator<T>(iterator); + } + + @Override + public default void close() { + // do nothing by default + } +} http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/DefaultCloseableIterator.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/DefaultCloseableIterator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/DefaultCloseableIterator.java new file mode 100644 index 0000000..7e4fb7c --- /dev/null +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/DefaultCloseableIterator.java @@ -0,0 +1,45 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tinkerpop.gremlin.structure.util; + +import java.util.Iterator; + +/** + * A default implementation of {@link CloseableIterator} that simply wraps an existing {@code Iterator}. This + * implementation has a "do nothing" implementation of {@link #close()}. + * + * @author Stephen Mallette (http://stephen.genoprime.com) + */ +public class DefaultCloseableIterator<T> implements CloseableIterator<T> { + protected Iterator<T> iterator; + + public DefaultCloseableIterator(final Iterator<T> iterator) { + this.iterator = iterator; + } + + @Override + public boolean hasNext() { + return iterator.hasNext(); + } + + @Override + public T next() { + return iterator.next(); + } +} http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java index c427d8e..020ed72 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java @@ -112,6 +112,39 @@ public class TraversalTest { assertThat(t.hasNext(), is(false)); } + @Test + public void shouldCloseWithoutACloseableStep() throws Exception { + final MockTraversal<Integer> t = new MockTraversal<>(1, 2, 3, 4, 5, 6, 7); + assertThat(t.hasNext(), is(true)); + t.close(); + assertThat(t.isClosed(), is(false)); + } + + @Test + public void shouldCloseWithCloseableStep() throws Exception { + final MockTraversal<Integer> t = new MockTraversal<>(Arrays.asList(1, 2, 3, 4, 5, 6, 7).iterator(), true); + assertThat(t.hasNext(), is(true)); + t.close(); + assertThat(t.isClosed(), is(true)); + } + + private static class MockCloseStep<E> extends MockStep<E> implements AutoCloseable { + private boolean closed = false; + + public MockCloseStep(final Iterator<E> itty) { + super(itty); + } + + boolean isClosed() { + return closed; + } + + @Override + public void close() throws Exception { + closed = true; + } + } + private static class MockStep<E> implements Step<E,E> { private final Iterator<E> itty; @@ -206,7 +239,6 @@ public class TraversalTest { } } - private static class MockTraversal<T> implements Traversal.Admin<T,T> { private Iterator<T> itty; @@ -220,15 +252,19 @@ public class TraversalTest { } MockTraversal(final List<T> list) { - this(list.iterator()); + this(list.iterator(), false); } - MockTraversal(final Iterator<T> itty) { + MockTraversal(final Iterator<T> itty, final boolean asCloseable) { this.itty = itty; - mockEndStep = new MockStep<>(itty); + mockEndStep = asCloseable ? new MockCloseStep<>(itty) : new MockStep<>(itty); steps = Collections.singletonList(mockEndStep); } + boolean isClosed() { + return mockEndStep instanceof MockCloseStep && ((MockCloseStep) mockEndStep).isClosed(); + } + @Override public Bytecode getBytecode() { return null; http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIteratorTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIteratorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIteratorTest.java new file mode 100644 index 0000000..c30896e --- /dev/null +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIteratorTest.java @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tinkerpop.gremlin.structure.util; + +import org.junit.Test; + +import java.util.Arrays; +import java.util.List; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; + +/** + * @author Stephen Mallette (http://stephen.genoprime.com) + */ +public class CloseableIteratorTest { + @Test + public void shouldWrapIterator() { + final List<String> stuff = Arrays.asList("this stuff", "that stuff", "other stuff"); + final CloseableIterator<String> itty = new DefaultCloseableIterator<>(stuff.iterator()); + assertThat(itty.hasNext(), is(true)); + assertEquals("this stuff", itty.next()); + assertThat(itty.hasNext(), is(true)); + assertEquals("that stuff", itty.next()); + assertThat(itty.hasNext(), is(true)); + assertEquals("other stuff", itty.next()); + assertThat(itty.hasNext(), is(false)); + + // this is a do-nothing, but we should still be able to call it + itty.close(); + } + + @Test + public void shouldWrapIteratorWithHelper() { + final List<String> stuff = Arrays.asList("this stuff", "that stuff", "other stuff"); + final CloseableIterator<String> itty = CloseableIterator.asCloseable(stuff.iterator()); + assertThat(itty.hasNext(), is(true)); + assertEquals("this stuff", itty.next()); + assertThat(itty.hasNext(), is(true)); + assertEquals("that stuff", itty.next()); + assertThat(itty.hasNext(), is(true)); + assertEquals("other stuff", itty.next()); + assertThat(itty.hasNext(), is(false)); + + // this is a do-nothing, but we should still be able to call it + itty.close(); + } + + @Test + public void shouldReturnSameInstance() { + final List<String> stuff = Arrays.asList("this stuff", "that stuff", "other stuff"); + final CloseableIterator<String> itty = new DefaultCloseableIterator<>(stuff.iterator()); + final CloseableIterator<String> same = CloseableIterator.asCloseable(itty); + assertSame(itty, same); + assertThat(itty.hasNext(), is(true)); + assertEquals("this stuff", itty.next()); + assertThat(itty.hasNext(), is(true)); + assertEquals("that stuff", itty.next()); + assertThat(itty.hasNext(), is(true)); + assertEquals("other stuff", itty.next()); + assertThat(itty.hasNext(), is(false)); + assertThat(same.hasNext(), is(false)); + + // this is a do-nothing, but we should still be able to call it + itty.close(); + } +}
