Merge pull request #139 from pontusmelke/1.0-cast-bug

Properly handle collections in `Values`

Author: jakewins

driver/src/main/java/org/neo4j/driver/v1/Values.java

@@ -35,12 +35,12 @@
 import org.neo4j.driver.internal.value.NullValue;
 import org.neo4j.driver.internal.value.StringValue;
 import org.neo4j.driver.v1.exceptions.ClientException;
-import org.neo4j.driver.v1.types.TypeSystem;
-import org.neo4j.driver.v1.util.Function;
 import org.neo4j.driver.v1.types.Entity;
 import org.neo4j.driver.v1.types.Node;
 import org.neo4j.driver.v1.types.Path;
 import org.neo4j.driver.v1.types.Relationship;
+import org.neo4j.driver.v1.types.TypeSystem;
+import org.neo4j.driver.v1.util.Function;
 
 /**
  * Utility for wrapping regular Java types and exposing them as {@link Value}
@@ -78,7 +78,7 @@ public static Value value( Object value )
         if ( value instanceof Double ) { return value( (double) value ); }
         if ( value instanceof Float ) { return value( (float) value ); }
 
-        if ( value instanceof Collection<?> ) { return value( (List<Object>) value ); }
+        if ( value instanceof List<?> ) { return value( (List<Object>) value ); }
         if ( value instanceof Iterable<?> ) { return value( (Iterable<Object>) value ); }
         if ( value instanceof Map<?, ?> ) { return value( (Map<String,Object>) value ); }
 
@@ -171,12 +171,13 @@ public static Value value( float... input )
         return new ListValue( values );
     }
 
-    public static Value value( List<Object> val )
+    public static Value value( List<Object> vals )
     {
-        Value[] values = new Value[val.size()];
-        for ( int i = 0; i < val.size(); i++ )
+        Value[] values = new Value[vals.size()];
+        int i = 0;
+        for ( Object val : vals )
         {
-            values[i] = value( val.get( i ) );
+            values[i++] = value( val );
         }
         return new ListValue( values );
     }

driver/src/test/java/org/neo4j/driver/internal/ValuesTest.java

@@ -18,10 +18,13 @@
  */
 package org.neo4j.driver.internal;
 
+import org.hamcrest.Matchers;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
+import java.util.ArrayDeque;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -241,6 +244,7 @@ public void shouldMapString() throws Throwable
         assertThat( val.asList( ofObject() ), contains((Object)"hello", "world") );
     }
 
+    @SuppressWarnings( "unchecked" )
     @Test
     public void shouldMapMapOfString() throws Throwable
     {
@@ -253,4 +257,17 @@ public void shouldMapMapOfString() throws Throwable
         assertThat( val.asList( ofMap() ), contains(map, map) );
         assertThat( val.asList( ofObject() ), contains((Object)map, map) );
     }
+
+    @Test
+    public void shouldHandleCollection() throws Throwable
+    {
+        // Given
+        Collection<String> collection = new ArrayDeque<>();
+        collection.add( "hello");
+        collection.add( "world");
+        Value val = value( collection );
+
+        // When/Then
+        assertThat( val.asList(), Matchers.<Object>containsInAnyOrder( "hello", "world" ));
+    }
 }

driver/src/test/java/org/neo4j/driver/v1/integration/ScalarTypeIT.java

@@ -54,7 +54,7 @@ public class ScalarTypeIT
     public static Collection<Object[]> typesToTest()
     {
         return Arrays.asList(
-                new Object[]{"RETURN 1 as v", Values.value( 1l )},
+                new Object[]{"RETURN 1 as v", Values.value( 1L )},
                 new Object[]{"RETURN 1.1 as v", Values.value( 1.1d )},
                 new Object[]{"RETURN 'hello' as v", Values.value( "hello" )},
                 new Object[]{"RETURN true as v", Values.value( true )},

driver/src/test/java/org/neo4j/driver/v1/integration/StatementIT.java

@@ -21,6 +21,7 @@
 import org.junit.Rule;
 import org.junit.Test;
 
+import java.util.Collections;
 import java.util.List;
 
 import org.neo4j.driver.v1.Record;
@@ -49,9 +50,9 @@ public void shouldRunWithResult() throws Throwable
         assertThat( result.size(), equalTo( 3 ) );
 
         // And it should allow random access
-        assertThat( result.get( 0 ).get( "k" ).asLong(), equalTo( 1l ) );
-        assertThat( result.get( 1 ).get( "k" ).asLong(), equalTo( 2l ) );
-        assertThat( result.get( 2 ).get( "k" ).asLong(), equalTo( 3l ) );
+        assertThat( result.get( 0 ).get( "k" ).asLong(), equalTo( 1L ) );
+        assertThat( result.get( 1 ).get( "k" ).asLong(), equalTo( 2L ) );
+        assertThat( result.get( 2 ).get( "k" ).asLong(), equalTo( 3L ) );
 
         // And it should allow iteration
         long expected = 0;
@@ -60,7 +61,7 @@ public void shouldRunWithResult() throws Throwable
             expected += 1;
             assertThat( value.get( "k" ), equalTo( Values.value( expected ) ) );
         }
-        assertThat( expected, equalTo( 3l ) );
+        assertThat( expected, equalTo( 3L ) );
     }
 
     @Test
@@ -72,6 +73,15 @@ public void shouldRunWithParameters() throws Throwable
         // Then nothing should've failed
     }
 
+    @Test
+    public void shouldRunWithCollectionAsParameter() throws Throwable
+    {
+        // When
+        session.run( "RETURN {param}", parameters( "param", Collections.singleton( "FOO" ) ) );
+
+        // Then nothing should've failed
+    }
+
     @Test
     public void shouldRun() throws Throwable
     {