Made TinkerGraph validate that ids be homogeneous up front.

Take this approach rather than doing it during iteration so that TinkerPop 
semantics are respected and appropriate validation exceptions are thrown at the 
right time.


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/e59b93ca
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/e59b93ca
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/e59b93ca

Branch: refs/heads/tp31
Commit: e59b93ca4ec7ffbde4a4b91319f35f5d3fd9caeb
Parents: 70dd324
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Thu Jun 16 15:08:30 2016 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Thu Jun 16 15:08:30 2016 -0400

----------------------------------------------------------------------
 .../tinkergraph/structure/TinkerGraph.java      | 41 ++++++++------------
 1 file changed, 17 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/e59b93ca/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java
----------------------------------------------------------------------
diff --git 
a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java
 
b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java
index 168dd7f..edffab9 100644
--- 
a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java
+++ 
b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java
@@ -326,30 +326,15 @@ public final class TinkerGraph implements Graph {
             return elements.values().iterator();
         } else {
             final List<Object> idList = Arrays.asList(ids);
-            // base the conversion function on the first item in the id list 
as the expectation is that these
-            // id values will be a uniform list
-            if (clazz.isAssignableFrom(ids[0].getClass())) {
-                // got a bunch of Elements - have to look each up because it 
might be an Attachable instance or
-                // other implementation. the assumption is that id conversion 
is not required for detached
-                // stuff - doesn't seem likely someone would detach a Titan 
vertex then try to expect that
-                // vertex to be findable in OrientDB
-                return IteratorUtils.filter(IteratorUtils.map(idList, id -> {
-                    // based on the first item assume all vertices in the 
argument list
-                    if (!clazz.isAssignableFrom(id.getClass()))
-                        throw Graph.Exceptions.idArgsMustBeEitherIdOrElement();
-
-                    return elements.get(clazz.cast(id).id());
-                }).iterator(), Objects::nonNull);
-            } else {
-                final Class<?> firstClass = ids[0].getClass();
-                return IteratorUtils.filter(IteratorUtils.map(idList, id -> {
-                    // all items in the list should be of the same class - 
don't get fancy on a Graph
-                    if (!id.getClass().equals(firstClass))
-                        throw Graph.Exceptions.idArgsMustBeEitherIdOrElement();
-
-                    return elements.get(idManager.convert(id));
-                }).iterator(), Objects::nonNull);
-            }
+            validateHomogenousIds(idList);
+
+            // if the type is of Element - have to look each up because it 
might be an Attachable instance or
+            // other implementation. the assumption is that id conversion is 
not required for detached
+            // stuff - doesn't seem likely someone would detach a Titan vertex 
then try to expect that
+            // vertex to be findable in OrientDB
+            return clazz.isAssignableFrom(ids[0].getClass()) ?
+               IteratorUtils.filter(IteratorUtils.map(idList, id -> 
elements.get(clazz.cast(id).id())).iterator(), Objects::nonNull)
+                : IteratorUtils.filter(IteratorUtils.map(idList, id -> 
elements.get(idManager.convert(id))).iterator(), Objects::nonNull);
         }
     }
 
@@ -365,6 +350,14 @@ public final class TinkerGraph implements Graph {
         return features;
     }
 
+    private void validateHomogenousIds(final List<Object> ids) {
+        final Class firstClass = ids.get(0).getClass();
+        for (Object id : ids) {
+            if (!id.getClass().equals(firstClass))
+                throw Graph.Exceptions.idArgsMustBeEitherIdOrElement();
+        }
+    }
+
     public class TinkerGraphFeatures implements Features {
 
         private final TinkerGraphGraphFeatures graphFeatures = new 
TinkerGraphGraphFeatures();

Reply via email to