[jira] [Updated] (TINKERPOP-1356) Several issues in HasContainer

["Marko A. Rodriguez (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/TINKERPOP-1356?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Marko A. Rodriguez updated TINKERPOP-1356:
------------------------------------------
    Assignee:     (was: Marko A. Rodriguez)

> 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.2.0-incubating, 3.1.2-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)