Adding error check of unsupported types #482
Conversation
There was a problem hiding this comment.
@zhenlineo review completed. Made two comments. It would also be nice to add ITs which use nodes, relationships and paths as query parameters.
| @@ -91,6 +94,10 @@ public static Value value( Object value ) | |||
| { | |||
| if ( value == null ) { return NullValue.NULL; } | |||
|
|
|||
| if ( value instanceof NodeValue || value instanceof RelationshipValue || value instanceof PathValue ) | |||
There was a problem hiding this comment.
I think it will be better to prohibit node, relationship and path in PackStreamMessageFormatV1. It is clear that struct packing code is the single point that writes structs to the channel. While Values#value() is just one of many factory methods.
There was a problem hiding this comment.
The reason to do it outside/earlier is because when we got an error there, we will kill the connection, which seems too aggressive. So if we could found the error in advance, then we will simply pop an error to the surface.
| @@ -70,12 +70,12 @@ public void shouldTypeAsNode() | |||
| assertThat( value.typeConstructor(), equalTo( TypeConstructor.NODE ) ); | |||
| } | |||
|
|
|||
| private NodeValue emptyNodeValue() | |||
| public static NodeValue emptyNodeValue() | |||
There was a problem hiding this comment.
Please extract NodeValueTest#emptyNodeValue(), PathValueTest#pathValue() and RelationshipValueTest#emptyRelationshipValue() into a test utility class. Alternatively ValuesTest could just create it's own node, relationship and path values.
zhenlineo
force-pushed
the
1.6-param-types
branch
2 times, most recently
from
5150775 to
6984fa0
Compare
The input parameters could be of type: * Map<String, Object> * Record * MapValue generated via Values#parameters * MapValue If the input parameters are provided by a Map<String, Object> or Record, or Values#parameters, then any unsupported types will be captured via Values#value. If the input parameters are provided by a MapValue directly, then any unsupported types will be captured at PackStream#pack.
zhenlineo
force-pushed
the
1.6-param-types
branch
from
6984fa0 to
af8c5b7
Compare
…est util Some tests needs this server side packer to serialize such types.
zhenlineo
force-pushed
the
1.6-param-types
branch
from
3e9ee16 to
76afa48
Compare
Fixes neo4j/neo4j#10239