diff --git a/driver/src/main/java/org/neo4j/driver/internal/messaging/PackStreamMessageFormatV1.java b/driver/src/main/java/org/neo4j/driver/internal/messaging/PackStreamMessageFormatV1.java index c5124ad257..130ad9beb7 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/messaging/PackStreamMessageFormatV1.java +++ b/driver/src/main/java/org/neo4j/driver/internal/messaging/PackStreamMessageFormatV1.java @@ -43,7 +43,6 @@ import org.neo4j.driver.internal.value.RelationshipValue; import org.neo4j.driver.v1.Value; import org.neo4j.driver.v1.exceptions.ClientException; -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; @@ -252,79 +251,8 @@ void packInternalValue( InternalValue value ) throws IOException } break; - case NODE: - { - Node node = value.asNode(); - packNode( node ); - } - break; - - case RELATIONSHIP: - { - Relationship rel = value.asRelationship(); - packer.packStructHeader( 5, RELATIONSHIP ); - packer.pack( rel.id() ); - packer.pack( rel.startNodeId() ); - packer.pack( rel.endNodeId() ); - - packer.pack( rel.type() ); - - packProperties( rel ); - } - break; - - case PATH: - Path path = value.asPath(); - packer.packStructHeader( 3, PATH ); - - // Unique nodes - Map nodeIdx = Iterables.newLinkedHashMapWithSize( path.length() + 1 ); - for ( Node node : path.nodes() ) - { - if ( !nodeIdx.containsKey( node ) ) - { - nodeIdx.put( node, nodeIdx.size() ); - } - } - packer.packListHeader( nodeIdx.size() ); - for ( Node node : nodeIdx.keySet() ) - { - packNode( node ); - } - - // Unique rels - Map relIdx = Iterables.newLinkedHashMapWithSize( path.length() ); - for ( Relationship rel : path.relationships() ) - { - if ( !relIdx.containsKey( rel ) ) - { - relIdx.put( rel, relIdx.size() + 1 ); - } - } - packer.packListHeader( relIdx.size() ); - for ( Relationship rel : relIdx.keySet() ) - { - packer.packStructHeader( 3, UNBOUND_RELATIONSHIP ); - packer.pack( rel.id() ); - packer.pack( rel.type() ); - packProperties( rel ); - } - - // Sequence - packer.packListHeader( path.length() * 2 ); - for ( Path.Segment seg : path ) - { - Relationship rel = seg.relationship(); - long relEndId = rel.endNodeId(); - long segEndId = seg.end().id(); - int size = relEndId == segEndId ? relIdx.get( rel ) : -relIdx.get( rel ); - packer.pack( size ); - packer.pack( nodeIdx.get( seg.end() ) ); - } - break; - default: - throw new IOException( "Unknown type: " + value ); + throw new IOException( "Unknown type: " + value.type().name() ); } } @@ -333,32 +261,6 @@ public void write( Message msg ) throws IOException { msg.dispatch( this ); } - - private void packNode( Node node ) throws IOException - { - packer.packStructHeader( NODE_FIELDS, NODE ); - packer.pack( node.id() ); - - Iterable labels = node.labels(); - packer.packListHeader( Iterables.count( labels ) ); - for ( String label : labels ) - { - packer.pack( label ); - } - - packProperties( node ); - } - - private void packProperties( Entity entity ) throws IOException - { - Iterable keys = entity.keys(); - packer.packMapHeader( entity.size() ); - for ( String propKey : keys ) - { - packer.pack( propKey ); - packValue( entity.get( propKey ) ); - } - } } static class ReaderV1 implements MessageFormat.Reader diff --git a/driver/src/main/java/org/neo4j/driver/internal/messaging/PackStreamMessageFormatV2.java b/driver/src/main/java/org/neo4j/driver/internal/messaging/PackStreamMessageFormatV2.java index 480b6b266c..0bcd71970e 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/messaging/PackStreamMessageFormatV2.java +++ b/driver/src/main/java/org/neo4j/driver/internal/messaging/PackStreamMessageFormatV2.java @@ -86,7 +86,7 @@ public MessageFormat.Reader newReader( PackInput input ) return new ReaderV2( input ); } - private static class WriterV2 extends WriterV1 + static class WriterV2 extends WriterV1 { WriterV2( PackOutput output ) { @@ -226,7 +226,7 @@ private void packPoint3D( Point point ) throws IOException } } - private static class ReaderV2 extends ReaderV1 + static class ReaderV2 extends ReaderV1 { ReaderV2( PackInput input ) { diff --git a/driver/src/main/java/org/neo4j/driver/v1/Statement.java b/driver/src/main/java/org/neo4j/driver/v1/Statement.java index b1dd85cb1a..08aae48637 100644 --- a/driver/src/main/java/org/neo4j/driver/v1/Statement.java +++ b/driver/src/main/java/org/neo4j/driver/v1/Statement.java @@ -20,6 +20,8 @@ import java.util.Map; +import org.neo4j.driver.internal.value.MapValue; +import org.neo4j.driver.v1.exceptions.ClientException; import org.neo4j.driver.v1.summary.ResultSummary; import org.neo4j.driver.v1.util.Immutable; @@ -52,7 +54,18 @@ public class Statement public Statement( String text, Value parameters ) { this.text = text; - this.parameters = parameters == null ? Values.EmptyMap : parameters; + if( parameters == null ) + { + this.parameters = Values.EmptyMap; + } + else if ( parameters instanceof MapValue ) + { + this.parameters = parameters; + } + else + { + throw new ClientException( "The parameters should be provided as Map type. Unsupported parameters type: " + parameters.type().name() ); + } } /** diff --git a/driver/src/main/java/org/neo4j/driver/v1/Values.java b/driver/src/main/java/org/neo4j/driver/v1/Values.java index 0868efd307..5ede4ace5a 100644 --- a/driver/src/main/java/org/neo4j/driver/v1/Values.java +++ b/driver/src/main/java/org/neo4j/driver/v1/Values.java @@ -49,8 +49,11 @@ import org.neo4j.driver.internal.value.LocalDateTimeValue; import org.neo4j.driver.internal.value.LocalTimeValue; import org.neo4j.driver.internal.value.MapValue; +import org.neo4j.driver.internal.value.NodeValue; import org.neo4j.driver.internal.value.NullValue; +import org.neo4j.driver.internal.value.PathValue; import org.neo4j.driver.internal.value.PointValue; +import org.neo4j.driver.internal.value.RelationshipValue; import org.neo4j.driver.internal.value.StringValue; import org.neo4j.driver.internal.value.TimeValue; import org.neo4j.driver.v1.exceptions.ClientException; @@ -91,6 +94,7 @@ public static Value value( Object value ) { if ( value == null ) { return NullValue.NULL; } + assertParameter( value ); if ( value instanceof AsValue ) { return ((AsValue) value).asValue(); } if ( value instanceof Boolean ) { return value( (boolean) value ); } if ( value instanceof String ) { return value( (String) value ); } @@ -381,7 +385,6 @@ public static Value parameters( Object... keysAndValues ) for ( int i = 0; i < keysAndValues.length; i += 2 ) { Object value = keysAndValues[i + 1]; - assertParameter( value ); map.put( keysAndValues[i].toString(), value( value ) ); } return value( map ); @@ -666,15 +669,15 @@ public static Function> ofList( final Function innerM private static void assertParameter( Object value ) { - if ( value instanceof Node ) + if ( value instanceof Node || value instanceof NodeValue ) { throw new ClientException( "Nodes can't be used as parameters." ); } - if ( value instanceof Relationship ) + if ( value instanceof Relationship || value instanceof RelationshipValue ) { throw new ClientException( "Relationships can't be used as parameters." ); } - if ( value instanceof Path ) + if ( value instanceof Path || value instanceof PathValue ) { throw new ClientException( "Paths can't be used as parameters." ); } diff --git a/driver/src/test/java/org/neo4j/driver/internal/ValuesTest.java b/driver/src/test/java/org/neo4j/driver/internal/ValuesTest.java index 29ff8368b5..08c9b0dbd2 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/ValuesTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/ValuesTest.java @@ -46,12 +46,18 @@ import org.neo4j.driver.internal.value.LocalDateTimeValue; import org.neo4j.driver.internal.value.LocalTimeValue; import org.neo4j.driver.internal.value.MapValue; +import org.neo4j.driver.internal.value.NodeValue; +import org.neo4j.driver.internal.value.PathValue; +import org.neo4j.driver.internal.value.RelationshipValue; import org.neo4j.driver.internal.value.TimeValue; import org.neo4j.driver.v1.Value; import org.neo4j.driver.v1.Values; import org.neo4j.driver.v1.exceptions.ClientException; import org.neo4j.driver.v1.types.IsoDuration; +import org.neo4j.driver.v1.types.Node; +import org.neo4j.driver.v1.types.Path; import org.neo4j.driver.v1.types.Point; +import org.neo4j.driver.v1.types.Relationship; import static java.util.Arrays.asList; import static org.hamcrest.CoreMatchers.equalTo; @@ -60,6 +66,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertThat; +import static org.neo4j.driver.internal.util.ValueFactory.emptyNodeValue; +import static org.neo4j.driver.internal.util.ValueFactory.emptyRelationshipValue; +import static org.neo4j.driver.internal.util.ValueFactory.filledPathValue; import static org.neo4j.driver.v1.Values.isoDuration; import static org.neo4j.driver.v1.Values.ofDouble; import static org.neo4j.driver.v1.Values.ofFloat; @@ -483,4 +492,76 @@ public void shouldCreateValueFromPoint3D() assertEquals( point3D, point3DValue2.asPoint() ); assertEquals( point3DValue1, point3DValue2 ); } + + @Test + public void shouldComplainAboutNodeValueType() throws Throwable + { + // Expect + exception.expect( ClientException.class ); + exception.expectMessage( "Nodes can't be used as parameters." ); + + // When + NodeValue node = emptyNodeValue(); + value( node ); + } + + @Test + public void shouldComplainAboutNodeType() throws Throwable + { + // Expect + exception.expect( ClientException.class ); + exception.expectMessage( "Nodes can't be used as parameters." ); + + // When + Node node = emptyNodeValue().asNode(); + value( node ); + } + + @Test + public void shouldComplainAboutRelationshipValueType() throws Throwable + { + // Expect + exception.expect( ClientException.class ); + exception.expectMessage( "Relationships can't be used as parameters." ); + + // When + RelationshipValue rel = emptyRelationshipValue(); + value( rel ); + } + + @Test + public void shouldComplainAboutRelationshipType() throws Throwable + { + // Expect + exception.expect( ClientException.class ); + exception.expectMessage( "Relationships can't be used as parameters." ); + + // When + Relationship rel = emptyRelationshipValue().asRelationship(); + value( rel ); + } + + @Test + public void shouldComplainAboutPathValueType() throws Throwable + { + // Expect + exception.expect( ClientException.class ); + exception.expectMessage( "Paths can't be used as parameters." ); + + // When + PathValue path = filledPathValue(); + value( path ); + } + + @Test + public void shouldComplainAboutPathType() throws Throwable + { + // Expect + exception.expect( ClientException.class ); + exception.expectMessage( "Paths can't be used as parameters." ); + + // When + Path path = filledPathValue().asPath(); + value( path ); + } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/messaging/KnowledgeablePackStreamMessageFormat.java b/driver/src/test/java/org/neo4j/driver/internal/messaging/KnowledgeablePackStreamMessageFormat.java new file mode 100644 index 0000000000..6b18ec8729 --- /dev/null +++ b/driver/src/test/java/org/neo4j/driver/internal/messaging/KnowledgeablePackStreamMessageFormat.java @@ -0,0 +1,164 @@ +/* + * Copyright (c) 2002-2018 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Licensed 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.neo4j.driver.internal.messaging; + +import java.io.IOException; +import java.util.Map; + +import org.neo4j.driver.internal.packstream.PackOutput; +import org.neo4j.driver.internal.types.TypeConstructor; +import org.neo4j.driver.internal.util.Iterables; +import org.neo4j.driver.internal.value.InternalValue; +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; + +/** + * This class provides the missing server side packing methods to serialize Node, Relationship and Path. + */ +public class KnowledgeablePackStreamMessageFormat extends PackStreamMessageFormatV2 +{ + @Override + public MessageFormat.Writer newWriter( PackOutput output, boolean byteArraySupportEnabled ) + { + return new KnowledgeablePackStreamMessageFormat.Writer( output ); + } + + private static class Writer extends WriterV2 + { + Writer( PackOutput output ) + { + super( output ); + } + + @Override + void packInternalValue( InternalValue value ) throws IOException + { + TypeConstructor typeConstructor = value.typeConstructor(); + switch ( typeConstructor ) + { + case NODE: + Node node = value.asNode(); + packNode( node ); + break; + + case RELATIONSHIP: + Relationship rel = value.asRelationship(); + packRelationship( rel ); + break; + + case PATH: + Path path = value.asPath(); + packPath( path ); + break; + default: + super.packInternalValue( value ); + } + } + + private void packPath( Path path ) throws IOException + { + packer.packStructHeader( 3, PATH ); + + // Unique nodes + Map nodeIdx = Iterables.newLinkedHashMapWithSize( path.length() + 1 ); + for ( Node node : path.nodes() ) + { + if ( !nodeIdx.containsKey( node ) ) + { + nodeIdx.put( node, nodeIdx.size() ); + } + } + packer.packListHeader( nodeIdx.size() ); + for ( Node node : nodeIdx.keySet() ) + { + packNode( node ); + } + + // Unique rels + Map relIdx = Iterables.newLinkedHashMapWithSize( path.length() ); + for ( Relationship rel : path.relationships() ) + { + if ( !relIdx.containsKey( rel ) ) + { + relIdx.put( rel, relIdx.size() + 1 ); + } + } + packer.packListHeader( relIdx.size() ); + for ( Relationship rel : relIdx.keySet() ) + { + packer.packStructHeader( 3, UNBOUND_RELATIONSHIP ); + packer.pack( rel.id() ); + packer.pack( rel.type() ); + packProperties( rel ); + } + + // Sequence + packer.packListHeader( path.length() * 2 ); + for ( Path.Segment seg : path ) + { + Relationship rel = seg.relationship(); + long relEndId = rel.endNodeId(); + long segEndId = seg.end().id(); + int size = relEndId == segEndId ? relIdx.get( rel ) : -relIdx.get( rel ); + packer.pack( size ); + packer.pack( nodeIdx.get( seg.end() ) ); + } + } + + private void packRelationship( Relationship rel ) throws IOException + { + packer.packStructHeader( 5, RELATIONSHIP ); + packer.pack( rel.id() ); + packer.pack( rel.startNodeId() ); + packer.pack( rel.endNodeId() ); + + packer.pack( rel.type() ); + + packProperties( rel ); + } + + private void packNode( Node node ) throws IOException + { + packer.packStructHeader( NODE_FIELDS, NODE ); + packer.pack( node.id() ); + + Iterable labels = node.labels(); + packer.packListHeader( Iterables.count( labels ) ); + for ( String label : labels ) + { + packer.pack( label ); + } + + packProperties( node ); + } + + private void packProperties( Entity entity ) throws IOException + { + Iterable keys = entity.keys(); + packer.packMapHeader( entity.size() ); + for ( String propKey : keys ) + { + packer.pack( propKey ); + packInternalValue( (InternalValue) entity.get( propKey ) ); + } + } + } +} diff --git a/driver/src/test/java/org/neo4j/driver/internal/messaging/MessageFormatTest.java b/driver/src/test/java/org/neo4j/driver/internal/messaging/MessageFormatTest.java index eca8a25d7d..dfe47b316e 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/messaging/MessageFormatTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/messaging/MessageFormatTest.java @@ -21,17 +21,15 @@ import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.embedded.EmbeddedChannel; +import io.netty.handler.codec.EncoderException; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import java.util.Collections; +import java.io.IOException; import java.util.HashMap; import java.util.List; -import org.neo4j.driver.internal.InternalNode; -import org.neo4j.driver.internal.InternalPath; -import org.neo4j.driver.internal.InternalRelationship; import org.neo4j.driver.internal.async.BoltProtocolUtil; import org.neo4j.driver.internal.async.ChannelPipelineBuilderImpl; import org.neo4j.driver.internal.async.inbound.InboundMessageDispatcher; @@ -41,13 +39,22 @@ import org.neo4j.driver.v1.exceptions.ClientException; import static java.util.Arrays.asList; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.neo4j.driver.internal.async.ChannelAttributes.messageDispatcher; import static org.neo4j.driver.internal.async.ChannelAttributes.setMessageDispatcher; import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING; -import static org.neo4j.driver.v1.Values.EmptyMap; +import static org.neo4j.driver.internal.util.ValueFactory.emptyNodeValue; +import static org.neo4j.driver.internal.util.ValueFactory.emptyPathValue; +import static org.neo4j.driver.internal.util.ValueFactory.emptyRelationshipValue; +import static org.neo4j.driver.internal.util.ValueFactory.filledNodeValue; +import static org.neo4j.driver.internal.util.ValueFactory.filledPathValue; +import static org.neo4j.driver.internal.util.ValueFactory.filledRelationshipValue; import static org.neo4j.driver.v1.Values.ofValue; import static org.neo4j.driver.v1.Values.parameters; import static org.neo4j.driver.v1.Values.value; @@ -80,33 +87,51 @@ public void shouldUnpackAllResponses() throws Throwable } @Test - public void shouldUnpackAllValues() throws Throwable + public void shouldPackUnpackValidValues() throws Throwable { assertSerializesValue( value( parameters( "cat", null, "dog", null ) ) ); assertSerializesValue( value( parameters( "k", 12, "a", "banana" ) ) ); assertSerializesValue( value( asList( "k", 12, "a", "banana" ) ) ); - assertSerializesValue( value( - new InternalNode( 1, Collections.singletonList( "User" ), parameters( "name", "Bob", "age", 45 ).asMap( - ofValue() ) ) - ) ); - assertSerializesValue( value( new InternalNode( 1 ) ) ); - assertSerializesValue( value( - new InternalRelationship( 1, 1, 1, - "KNOWS", - parameters( "name", "Bob", "age", 45 ).asMap( ofValue() ) ) ) ); - assertSerializesValue( value( - new InternalPath( - new InternalNode( 1 ), - new InternalRelationship( 2, 1, 3, - "KNOWS", EmptyMap.asMap( ofValue() ) ), - new InternalNode( 3 ), - new InternalRelationship( 4, 3, 5, - "LIKES", EmptyMap.asMap( ofValue() ) ), - new InternalNode( 5 ) - ) ) ); - assertSerializesValue( value( new InternalPath( new InternalNode( 1 ) ) ) ); } + @Test + public void shouldUnpackNodeRelationshipAndPath() throws Throwable + { + // Given + assertOnlyDeserializesValue( emptyNodeValue() ); + assertOnlyDeserializesValue( filledNodeValue() ); + assertOnlyDeserializesValue( emptyRelationshipValue() ); + assertOnlyDeserializesValue( filledRelationshipValue() ); + assertOnlyDeserializesValue( emptyPathValue() ); + assertOnlyDeserializesValue( filledPathValue() ); + } + + + @Test + public void shouldErrorPackingNode() throws Throwable + { + // Given + Value value = filledNodeValue(); + expectIOExceptionWithMessage( value, "Unknown type: NODE" ); + } + + @Test + public void shouldErrorPackingRelationship() throws Throwable + { + // Given + Value value = filledRelationshipValue(); + expectIOExceptionWithMessage( value, "Unknown type: RELATIONSHIP" ); + } + + @Test + public void shouldErrorPackingPath() throws Throwable + { + // Given + Value value = filledPathValue(); + expectIOExceptionWithMessage( value, "Unknown type: PATH" ); + } + + @Test public void shouldGiveHelpfulErrorOnMalformedNodeStruct() throws Throwable { @@ -149,6 +174,11 @@ private void assertSerializes( Message message ) throws Throwable } private EmbeddedChannel newEmbeddedChannel() + { + return newEmbeddedChannel( format ); + } + + private EmbeddedChannel newEmbeddedChannel( MessageFormat format ) { EmbeddedChannel channel = new EmbeddedChannel(); setMessageDispatcher( channel, new MemorizingInboundMessageDispatcher( channel, DEV_NULL_LOGGING ) ); @@ -186,4 +216,51 @@ private Message unpack( ByteBuf packed, EmbeddedChannel channel ) throws Throwab assertEquals( 1, unpackedMessages.size() ); return unpackedMessages.get( 0 ); } + + private void assertOnlyDeserializesValue( Value value ) throws Throwable + { + RecordMessage message = new RecordMessage( new Value[]{value} ); + ByteBuf packed = knowledgeablePack( message ); + + EmbeddedChannel channel = newEmbeddedChannel(); + Message unpackedMessage = unpack( packed, channel ); + + assertEquals( message, unpackedMessage ); + } + + private ByteBuf knowledgeablePack( Message message ) throws IOException + { + EmbeddedChannel channel = newEmbeddedChannel( new KnowledgeablePackStreamMessageFormat() ); + assertTrue( channel.writeOutbound( message ) ); + + ByteBuf[] packedMessages = channel.outboundMessages() + .stream() + .map( msg -> (ByteBuf) msg ) + .toArray( ByteBuf[]::new ); + + return Unpooled.wrappedBuffer( packedMessages ); + } + + private void expectIOExceptionWithMessage( Value value, String errorMessage ) + { + RecordMessage message = new RecordMessage( new Value[]{value} ); + EmbeddedChannel channel = newEmbeddedChannel(); + + try + { + pack( message, channel ); + fail( "Expecting a EncoderException" ); + } + catch ( EncoderException e ) + { + Throwable cause = e.getCause(); + assertThat( cause, instanceOf( IOException.class ) ); + assertThat( cause.getMessage(), equalTo( errorMessage ) ); + } + catch ( Exception e ) + { + fail( "Expecting a EncoderException but got " + e ); + } + } + } diff --git a/driver/src/test/java/org/neo4j/driver/internal/util/ValueFactory.java b/driver/src/test/java/org/neo4j/driver/internal/util/ValueFactory.java new file mode 100644 index 0000000000..be58244bc6 --- /dev/null +++ b/driver/src/test/java/org/neo4j/driver/internal/util/ValueFactory.java @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2002-2018 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Licensed 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.neo4j.driver.internal.util; + +import java.util.HashMap; + +import org.neo4j.driver.internal.InternalNode; +import org.neo4j.driver.internal.InternalPath; +import org.neo4j.driver.internal.InternalRelationship; +import org.neo4j.driver.internal.value.NodeValue; +import org.neo4j.driver.internal.value.PathValue; +import org.neo4j.driver.internal.value.RelationshipValue; +import org.neo4j.driver.v1.Value; + +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static org.neo4j.driver.v1.Values.value; + +public class ValueFactory +{ + public static NodeValue emptyNodeValue() + { + return new NodeValue( new InternalNode( 1234, singletonList( "User" ), new HashMap() ) ); + } + + public static NodeValue filledNodeValue() + { + return new NodeValue( new InternalNode( 1234, singletonList( "User" ), singletonMap( "name", value( "Dodo" ) ) ) ); + } + + public static RelationshipValue emptyRelationshipValue() + { + return new RelationshipValue( new InternalRelationship( 1234, 1, 2, "KNOWS" ) ); + } + + public static RelationshipValue filledRelationshipValue() + { + return new RelationshipValue( new InternalRelationship( 1234, 1, 2, "KNOWS", singletonMap( "name", value( "Dodo" ) ) ) ); + } + + public static PathValue filledPathValue() + { + return new PathValue( new InternalPath( new InternalNode(42L), new InternalRelationship( 43L, 42L, 44L, "T" ), new InternalNode( 44L ) ) ); + } + + public static PathValue emptyPathValue() + { + return new PathValue( new InternalPath( new InternalNode( 1 ) ) ); + } +} diff --git a/driver/src/test/java/org/neo4j/driver/internal/value/NodeValueTest.java b/driver/src/test/java/org/neo4j/driver/internal/value/NodeValueTest.java index f01c252267..c568032b6e 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/value/NodeValueTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/value/NodeValueTest.java @@ -20,20 +20,15 @@ import org.junit.Test; -import java.util.HashMap; - -import org.neo4j.driver.internal.InternalNode; import org.neo4j.driver.internal.types.InternalTypeSystem; import org.neo4j.driver.internal.types.TypeConstructor; -import org.neo4j.driver.v1.Value; -import static java.util.Collections.singletonList; -import static java.util.Collections.singletonMap; import static org.hamcrest.CoreMatchers.equalTo; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; -import static org.neo4j.driver.v1.Values.value; +import static org.neo4j.driver.internal.util.ValueFactory.emptyNodeValue; +import static org.neo4j.driver.internal.util.ValueFactory.filledNodeValue; public class NodeValueTest { @@ -69,14 +64,4 @@ public void shouldTypeAsNode() InternalValue value = emptyNodeValue(); assertThat( value.typeConstructor(), equalTo( TypeConstructor.NODE ) ); } - - private NodeValue emptyNodeValue() - { - return new NodeValue( new InternalNode( 1234, singletonList( "User" ), new HashMap() ) ); - } - - private NodeValue filledNodeValue() - { - return new NodeValue( new InternalNode( 1234, singletonList( "User" ), singletonMap( "name", value( "Dodo" ) ) ) ); - } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/value/PathValueTest.java b/driver/src/test/java/org/neo4j/driver/internal/value/PathValueTest.java index 9d66a6a4b6..1713b5dde9 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/value/PathValueTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/value/PathValueTest.java @@ -20,9 +20,6 @@ import org.junit.Test; -import org.neo4j.driver.internal.InternalNode; -import org.neo4j.driver.internal.InternalPath; -import org.neo4j.driver.internal.InternalRelationship; import org.neo4j.driver.internal.types.InternalTypeSystem; import org.neo4j.driver.v1.Value; @@ -30,19 +27,20 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; +import static org.neo4j.driver.internal.util.ValueFactory.filledPathValue; public class PathValueTest { @Test public void shouldHaveSensibleToString() throws Throwable { - assertEquals("path[(42)-[43:T]->(44)]", pathValue().toString()); + assertEquals("path[(42)-[43:T]->(44)]", filledPathValue().toString()); } @Test public void shouldNotBeNull() { - Value value = pathValue(); + Value value = filledPathValue(); assertFalse( value.isNull() ); } @@ -51,11 +49,6 @@ public void shouldNotBeNull() public void shouldHaveCorrectType() throws Throwable { - assertThat(pathValue().type(), equalTo( InternalTypeSystem.TYPE_SYSTEM.PATH() )); - } - - private PathValue pathValue() - { - return new PathValue( new InternalPath( new InternalNode(42L), new InternalRelationship( 43L, 42L, 44L, "T" ), new InternalNode( 44L ) ) ); + assertThat( filledPathValue().type(), equalTo( InternalTypeSystem.TYPE_SYSTEM.PATH() )); } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/value/RelationshipValueTest.java b/driver/src/test/java/org/neo4j/driver/internal/value/RelationshipValueTest.java index 724a244887..631c4bc586 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/value/RelationshipValueTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/value/RelationshipValueTest.java @@ -20,16 +20,15 @@ import org.junit.Test; -import org.neo4j.driver.internal.InternalRelationship; import org.neo4j.driver.internal.types.TypeConstructor; import org.neo4j.driver.v1.Value; -import static java.util.Collections.singletonMap; import static org.hamcrest.CoreMatchers.equalTo; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; -import static org.neo4j.driver.v1.Values.value; +import static org.neo4j.driver.internal.util.ValueFactory.emptyRelationshipValue; +import static org.neo4j.driver.internal.util.ValueFactory.filledRelationshipValue; public class RelationshipValueTest { @@ -60,14 +59,4 @@ public void shouldTypeAsRelationship() InternalValue value = emptyRelationshipValue(); assertThat( value.typeConstructor(), equalTo( TypeConstructor.RELATIONSHIP ) ); } - - private RelationshipValue emptyRelationshipValue() - { - return new RelationshipValue( new InternalRelationship( 1234, 1, 2, "KNOWS" ) ); - } - - private RelationshipValue filledRelationshipValue() - { - return new RelationshipValue( new InternalRelationship( 1234, 1, 2, "KNOWS", singletonMap( "name", value( "Dodo" ) ) ) ); - } } diff --git a/driver/src/test/java/org/neo4j/driver/v1/ParametersTest.java b/driver/src/test/java/org/neo4j/driver/v1/ParametersTest.java index 6944c21a0d..d131e83727 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/ParametersTest.java +++ b/driver/src/test/java/org/neo4j/driver/v1/ParametersTest.java @@ -21,14 +21,56 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import java.util.HashMap; +import java.util.Map; + +import org.neo4j.driver.internal.InternalRecord; +import org.neo4j.driver.internal.NetworkSession; +import org.neo4j.driver.internal.retry.RetryLogic; +import org.neo4j.driver.internal.spi.ConnectionProvider; import org.neo4j.driver.v1.exceptions.ClientException; +import static java.util.Collections.singletonList; +import static org.junit.Assume.assumeTrue; +import static org.mockito.Mockito.mock; +import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING; +import static org.neo4j.driver.internal.util.ValueFactory.emptyNodeValue; +import static org.neo4j.driver.internal.util.ValueFactory.emptyRelationshipValue; +import static org.neo4j.driver.internal.util.ValueFactory.filledPathValue; +import static org.neo4j.driver.v1.Values.parameters; + +@RunWith(Parameterized.class) public class ParametersTest { @Rule public ExpectedException exception = ExpectedException.none(); + @Parameterized.Parameter + public Object obj; + @Parameterized.Parameter( 1 ) + public String expectedMsg; + + @Parameterized.Parameters( name = "{0}" ) + public static Object[][] addressesToParse() + { + return new Object[][]{ + // Node + {emptyNodeValue(), "Nodes can't be used as parameters."}, + {emptyNodeValue().asNode(), "Nodes can't be used as parameters."}, + + // Rel + {emptyRelationshipValue(), "Relationships can't be used as parameters."}, + {emptyRelationshipValue().asRelationship(), "Relationships can't be used as parameters."}, + + // Path + {filledPathValue(), "Paths can't be used as parameters."}, + {filledPathValue().asPath(), "Paths can't be used as parameters."}, + }; + } + @Test public void shouldGiveHelpfulMessageOnMisalignedInput() throws Throwable { @@ -38,6 +80,57 @@ public void shouldGiveHelpfulMessageOnMisalignedInput() throws Throwable "alternating key and value." ); // When - Values.parameters( "1", new Object(), "2" ); + Values.parameters( "1", obj, "2" ); + } + + @Test + public void shouldNotBePossibleToUseInvalidParameterTypesViaParameters() + { + //Expect + exception.expect( ClientException.class ); + exception.expectMessage( expectedMsg ); + + // WHEN + Session session = mockedSession(); + session.run( "RETURN {a}", parameters( "a", obj ) ); + } + + @Test + public void shouldNotBePossibleToUseInvalidParametersViaMap() + { + // GIVEN + Map params = new HashMap<>(); + params.put( "a", obj ); + + //Expect + exception.expect( ClientException.class ); + exception.expectMessage( expectedMsg ); + + // WHEN + Session session = mockedSession(); + session.run( "RETURN {a}", params ); + } + + @Test + public void shouldNotBePossibleToUseInvalidParametersViaRecord() + { + // GIVEN + assumeTrue(obj instanceof Value); + Record record = new InternalRecord( singletonList( "a" ), new Value[]{(Value) obj} ); + + //Expect + exception.expect( ClientException.class ); + exception.expectMessage( expectedMsg ); + + // WHEN + Session session = mockedSession(); + session.run( "RETURN {a}", record ); + } + + private Session mockedSession() + { + ConnectionProvider provider = mock( ConnectionProvider.class ); + RetryLogic retryLogic = mock( RetryLogic.class ); + return new NetworkSession( provider, AccessMode.WRITE, retryLogic, DEV_NULL_LOGGING ); } } diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/ParametersIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/ParametersIT.java index cf43a05489..3dc36d38b8 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/ParametersIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/ParametersIT.java @@ -22,26 +22,31 @@ import org.junit.Test; import org.junit.rules.ExpectedException; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.ThreadLocalRandom; import org.neo4j.driver.internal.util.ServerVersion; +import org.neo4j.driver.internal.value.MapValue; import org.neo4j.driver.v1.Record; import org.neo4j.driver.v1.StatementResult; import org.neo4j.driver.v1.Value; import org.neo4j.driver.v1.exceptions.ClientException; import org.neo4j.driver.v1.exceptions.ServiceUnavailableException; -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.util.TestNeo4jSession; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; import static org.neo4j.driver.internal.util.ServerVersion.version; +import static org.neo4j.driver.internal.util.ValueFactory.emptyNodeValue; +import static org.neo4j.driver.internal.util.ValueFactory.emptyRelationshipValue; +import static org.neo4j.driver.internal.util.ValueFactory.filledPathValue; import static org.neo4j.driver.v1.Values.ofValue; import static org.neo4j.driver.v1.Values.parameters; @@ -397,48 +402,53 @@ public void settingInvalidParameterTypeShouldThrowHelpfulError() throws Throwabl } @Test - public void shouldNotBePossibleToUseNodeAsParameter() + public void settingInvalidParameterTypeDirectlyShouldThrowHelpfulError() throws Throwable { - // GIVEN - StatementResult cursor = session.run( "CREATE (a:Node) RETURN a" ); - Node node = cursor.single().get( 0 ).asNode(); - - //Expect + // Expect exception.expect( ClientException.class ); - exception.expectMessage( "Nodes can't be used as parameters" ); + exception.expectMessage( "The parameters should be provided as Map type. Unsupported parameters type: NODE" ); - // WHEN - session.run( "RETURN {a}", parameters( "a", node ) ); + // When + session.run( "anything", emptyNodeValue() ); } @Test - public void shouldNotBePossibleToUseRelationshipAsParameter() + public void shouldNotBePossibleToUseNodeAsParameterInMapValue() { // GIVEN - StatementResult cursor = session.run( "CREATE (a:Node), (b:Node), (a)-[r:R]->(b) RETURN r" ); - Relationship relationship = cursor.single().get( 0 ).asRelationship(); - - //Expect - exception.expect( ClientException.class ); - exception.expectMessage( "Relationships can't be used as parameters" ); + Value node = emptyNodeValue(); + Map params = new HashMap<>(); + params.put( "a", node ); + MapValue mapValue = new MapValue( params ); // WHEN - session.run( "RETURN {a}", parameters( "a", relationship ) ); + expectIOExceptionWithMessage( mapValue, "Unknown type: NODE" ); } @Test - public void shouldNotBePossibleToUsePathAsParameter() + public void shouldNotBePossibleToUseRelationshipAsParameterViaMapValue() { // GIVEN - StatementResult cursor = session.run( "CREATE (a:Node), (b:Node), p=(a)-[r:R]->(b) RETURN p" ); - Path path = cursor.single().get( 0 ).asPath(); + Value relationship = emptyRelationshipValue(); + Map params = new HashMap<>(); + params.put( "a", relationship ); + MapValue mapValue = new MapValue( params ); - //Expect - exception.expect( ClientException.class ); - exception.expectMessage( "Paths can't be used as parameters" ); + // WHEN + expectIOExceptionWithMessage( mapValue, "Unknown type: RELATIONSHIP" ); + } + + @Test + public void shouldNotBePossibleToUsePathAsParameterViaMapValue() + { + // GIVEN + Value path = filledPathValue(); + Map params = new HashMap<>(); + params.put( "a", path ); + MapValue mapValue = new MapValue( params ); // WHEN - session.run( "RETURN {a}", parameters( "a", path ) ); + expectIOExceptionWithMessage( mapValue, "Unknown type: PATH" ); } private void testBytesProperty( byte[] array ) @@ -480,4 +490,23 @@ private static byte[] randomByteArray( int length ) ThreadLocalRandom.current().nextBytes( result ); return result; } + + private void expectIOExceptionWithMessage( Value value, String message ) + { + try + { + session.run( "RETURN {a}", value ).consume(); + fail( "Expecting a ServiceUnavailableException" ); + } + catch ( ServiceUnavailableException e ) + { + Throwable cause = e.getCause(); + assertThat( cause, instanceOf( IOException.class ) ); + assertThat( cause.getMessage(), equalTo( message ) ); + } + catch ( Exception e ) + { + fail( "Expecting a ServiceUnavailableException but got " + e ); + } + } } diff --git a/driver/src/test/java/org/neo4j/driver/v1/types/TypeSystemTest.java b/driver/src/test/java/org/neo4j/driver/v1/types/TypeSystemTest.java index d3485419ce..1f21a1cf04 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/types/TypeSystemTest.java +++ b/driver/src/test/java/org/neo4j/driver/v1/types/TypeSystemTest.java @@ -32,6 +32,9 @@ import org.neo4j.driver.internal.InternalPath; import org.neo4j.driver.internal.InternalRelationship; import org.neo4j.driver.internal.types.InternalTypeSystem; +import org.neo4j.driver.internal.value.NodeValue; +import org.neo4j.driver.internal.value.PathValue; +import org.neo4j.driver.internal.value.RelationshipValue; import org.neo4j.driver.v1.Value; import static org.hamcrest.CoreMatchers.not; @@ -48,10 +51,10 @@ public class TypeSystemTest private Value integerValue = value( 13 ); private Value floatValue = value( 13.1 ); private Value stringValue = value( "Lalala " ); - private Value nodeValue = value( node ); - private Value relationshipValue = value( relationship ); + private Value nodeValue = new NodeValue( node ); + private Value relationshipValue = new RelationshipValue( relationship ); private Value mapValue = value( Collections.singletonMap( "type", "r" ) ); - private Value pathValue = value( new InternalPath( Arrays.asList( node, relationship, node ) ) ); + private Value pathValue = new PathValue( new InternalPath( Arrays.asList( node, relationship, node ) ) ); private Value booleanValue = value( true ); private Value listValue = value( Arrays.asList( 1, 2, 3 ) ); private Value nullValue = value( (Object) null );