Daniel Kuppitz created TINKERPOP-1356:
-----------------------------------------
Summary: Several issues in HasContainer
Key: TINKERPOP-1356
URL: https://issues.apache.org/jira/browse/TINKERPOP-1356
Project: TinkerPop
Issue Type: Bug
Components: process
Affects Versions: 3.1.2-incubating, 3.2.0-incubating
Reporter: Daniel Kuppitz
{{HasContainer}} has some issues that are not covered by test cases, but IMO
likely to happen in real world applications.
Empty collections lead to uncaught exceptions with a meaningless error message:
{noformat}
gremlin> g = TinkerFactory.createModern().traversal()
==>graphtraversalsource[tinkergraph[vertices:6 edges:6], standard]
gremlin> g.V().hasId(within(new ArrayList()))
0
Display stack trace? [yN] N
gremlin> g.V().hasId(without(new ArrayList()))
0
Display stack trace? [yN]
{noformat}
In the constructor of {{HasContainer}} we should use:
{code}
((Collection)
this.predicate.getValue()).stream().findFirst().orElseGet(Object::new)
{code}
...instead of:
{code}
((Collection) this.predicate.getValue()).toArray()[0]
{code}
..or anything else that doesn't assume that we will always have a first element.
Furthermore {{enforceHomogenousCollectionIfPresent}} should check for empty
collections:
{code}
...
final Collection collection = (Collection) predicateValue;
if (!collection.isEmpty()) { // <--
final Class<?> first = collection.toArray()[0].getClass();
if (!((Collection)
predicateValue).stream().map(Object::getClass).allMatch(c -> first.equals(c)))
throw new IllegalArgumentException("Has comparisons on a collection of
ids require ids to all be of the same type");
}
...
{code}
And the last issue is this one (from the code snippet above):
{{collection.toArray()\[0\].getClass()}}. What if the first (or any other)
element is actually {{null}}? The check should be more like:
{code}
final Object obj = new Object();
if (((Collection) predicateValue).stream().map(o ->
Optional.ofNullable(o).orElse(obj).getClass()).distinct().count() != 1)
throw ...
{code}
Or something smarter like that, as long as it has a {{null}}-check.
The proposed code changes seem to work fine, except for
{{hasId(within(<empty-collection>))}}, which emits all vertices instead of none.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
