Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ali-ince committed Apr 5, 2018
1 parent 347540f commit a01b005
Show file tree
Hide file tree
Showing 13 changed files with 38 additions and 68 deletions.
Expand Up @@ -27,7 +27,7 @@


import static java.util.Collections.unmodifiableMap; import static java.util.Collections.unmodifiableMap;


public enum KnownType public enum StructType
{ {
NODE( Neo4jPackV1.NODE, "Node" ), NODE( Neo4jPackV1.NODE, "Node" ),
RELATIONSHIP( Neo4jPackV1.RELATIONSHIP, "Relationship" ), RELATIONSHIP( Neo4jPackV1.RELATIONSHIP, "Relationship" ),
Expand All @@ -46,7 +46,7 @@ public enum KnownType
private final byte signature; private final byte signature;
private final String description; private final String description;


KnownType( byte signature, String description ) StructType( byte signature, String description )
{ {
this.signature = signature; this.signature = signature;
this.description = description; this.description = description;
Expand All @@ -62,23 +62,23 @@ public String description()
return description; return description;
} }


private static Map<Byte, KnownType> knownTypesBySignature = knownTypesBySignature(); private static Map<Byte,StructType> knownTypesBySignature = knownTypesBySignature();


public static KnownType valueOf( byte signature ) public static StructType valueOf( byte signature )
{ {
return knownTypesBySignature.get( signature ); return knownTypesBySignature.get( signature );
} }


public static KnownType valueOf( char signature ) public static StructType valueOf( char signature )
{ {
return knownTypesBySignature.get( (byte)signature ); return knownTypesBySignature.get( (byte)signature );
} }


private static Map<Byte,KnownType> knownTypesBySignature() private static Map<Byte,StructType> knownTypesBySignature()
{ {
KnownType[] types = KnownType.values(); StructType[] types = StructType.values();
Map<Byte,KnownType> result = new HashMap<>( types.length * 2 ); Map<Byte,StructType> result = new HashMap<>( types.length * 2 );
for ( KnownType type : types ) for ( StructType type : types )
{ {
result.put( type.signature, type ); result.put( type.signature, type );
} }
Expand Down
Expand Up @@ -30,11 +30,6 @@ public class MessageAccumulator extends ByteToMessageDecoder
{ {
private boolean readMessageBoundary; private boolean readMessageBoundary;


public MessageAccumulator()
{

}

@Override @Override
public void channelRead( ChannelHandlerContext ctx, Object msg ) throws Exception public void channelRead( ChannelHandlerContext ctx, Object msg ) throws Exception
{ {
Expand Down
Expand Up @@ -20,7 +20,6 @@
package org.neo4j.bolt.transport.pipeline; package org.neo4j.bolt.transport.pipeline;


import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.channel.ChannelInboundHandlerAdapter;
Expand Down Expand Up @@ -107,7 +106,7 @@ public void channelRead( ChannelHandlerContext ctx, Object msg )
} }
else else
{ {
ctx.writeAndFlush( Unpooled.wrappedBuffer( new byte[]{0, 0, 0, 0} ) ).addListener( ChannelFutureListener.CLOSE ); ctx.writeAndFlush( ctx.alloc().buffer().writeBytes( new byte[]{0, 0, 0, 0} ) ).addListener( ChannelFutureListener.CLOSE );
} }
} }
else else
Expand Down
Expand Up @@ -30,7 +30,7 @@
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;


import org.neo4j.bolt.messaging.KnownType; import org.neo4j.bolt.messaging.StructType;
import org.neo4j.bolt.v1.packstream.PackInput; import org.neo4j.bolt.v1.packstream.PackInput;
import org.neo4j.bolt.v1.packstream.PackOutput; import org.neo4j.bolt.v1.packstream.PackOutput;
import org.neo4j.bolt.v1.packstream.PackStream; import org.neo4j.bolt.v1.packstream.PackStream;
Expand Down Expand Up @@ -523,15 +523,15 @@ else if ( size == UNKNOWN_SIZE )


protected AnyValue unpackStruct( char signature, long size ) throws IOException protected AnyValue unpackStruct( char signature, long size ) throws IOException
{ {
KnownType knownType = KnownType.valueOf( signature ); StructType structType = StructType.valueOf( signature );
if ( knownType == null ) if ( structType == null )
{ {
throw new BoltIOException( Status.Request.InvalidFormat, throw new BoltIOException( Status.Request.InvalidFormat,
String.format( "Struct types of 0x%s are not recognized.", Integer.toHexString( signature ) ) ); String.format( "Struct types of 0x%s are not recognized.", Integer.toHexString( signature ) ) );
} }


throw new BoltIOException( Status.Statement.TypeError, throw new BoltIOException( Status.Statement.TypeError,
String.format( "%s values cannot be unpacked with this version of bolt.", knownType.description() ) ); String.format( "%s values cannot be unpacked with this version of bolt.", structType.description() ) );
} }


@Override @Override
Expand Down
Expand Up @@ -83,12 +83,6 @@ public byte peekByte()
return buf.getByte( buf.readerIndex() ); return buf.getByte( buf.readerIndex() );
} }


@Override
public void close() throws IOException
{
stop();
}

private void assertNotStarted() private void assertNotStarted()
{ {
if ( buf != null ) if ( buf != null )
Expand Down
Expand Up @@ -19,14 +19,13 @@
*/ */
package org.neo4j.bolt.v1.packstream; package org.neo4j.bolt.v1.packstream;


import java.io.Closeable;
import java.io.IOException; import java.io.IOException;


/** /**
* This is what {@link PackStream} uses to ingest data, implement this on top of any data source of your choice to * This is what {@link PackStream} uses to ingest data, implement this on top of any data source of your choice to
* deserialize the stream with {@link PackStream}. * deserialize the stream with {@link PackStream}.
*/ */
public interface PackInput extends Closeable public interface PackInput
{ {
/** Consume one byte */ /** Consume one byte */
byte readByte() throws IOException; byte readByte() throws IOException;
Expand Down
Expand Up @@ -23,7 +23,7 @@
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;


import org.neo4j.bolt.messaging.KnownType; import org.neo4j.bolt.messaging.StructType;
import org.neo4j.bolt.v1.packstream.utf8.UTF8Encoder; import org.neo4j.bolt.v1.packstream.utf8.UTF8Encoder;


/** /**
Expand Down Expand Up @@ -766,13 +766,13 @@ public PackType peekNextType() throws IOException
return type( markerByte ); return type( markerByte );
} }


public static void ensureCorrectStructSize( KnownType knownType, int expected, long actual ) throws IOException public static void ensureCorrectStructSize( StructType structType, int expected, long actual ) throws IOException
{ {
if ( expected != actual ) if ( expected != actual )
{ {
throw new PackStreamException( throw new PackStreamException(
String.format( "Invalid message received, serialized %s structures should have %d fields, " + "received %s structure has %d fields.", String.format( "Invalid message received, serialized %s structures should have %d fields, " + "received %s structure has %d fields.",
knownType.description(), expected, knownType.description(), actual ) ); structType.description(), expected, structType.description(), actual ) );
} }
} }
} }
Expand Down
Expand Up @@ -81,10 +81,4 @@ public byte peekByte() throws IOException
data.reset(); data.reset();
return value; return value;
} }

@Override
public void close() throws IOException
{
data.close();
}
} }
Expand Up @@ -30,7 +30,7 @@
import java.time.ZonedDateTime; import java.time.ZonedDateTime;
import java.util.Arrays; import java.util.Arrays;


import org.neo4j.bolt.messaging.KnownType; import org.neo4j.bolt.messaging.StructType;
import org.neo4j.bolt.v1.messaging.BoltIOException; import org.neo4j.bolt.v1.messaging.BoltIOException;
import org.neo4j.bolt.v1.messaging.Neo4jPack; import org.neo4j.bolt.v1.messaging.Neo4jPack;
import org.neo4j.bolt.v1.messaging.Neo4jPackV1; import org.neo4j.bolt.v1.messaging.Neo4jPackV1;
Expand Down Expand Up @@ -232,31 +232,31 @@ protected AnyValue unpackStruct( char signature, long size ) throws IOException
switch ( signature ) switch ( signature )
{ {
case POINT_2D: case POINT_2D:
ensureCorrectStructSize( KnownType.POINT_2D, POINT_2D_SIZE, size ); ensureCorrectStructSize( StructType.POINT_2D, POINT_2D_SIZE, size );
return unpackPoint2D(); return unpackPoint2D();
case POINT_3D: case POINT_3D:
ensureCorrectStructSize( KnownType.POINT_3D, POINT_3D_SIZE, size ); ensureCorrectStructSize( StructType.POINT_3D, POINT_3D_SIZE, size );
return unpackPoint3D(); return unpackPoint3D();
case DURATION: case DURATION:
ensureCorrectStructSize( KnownType.DURATION, DURATION_SIZE, size ); ensureCorrectStructSize( StructType.DURATION, DURATION_SIZE, size );
return unpackDuration(); return unpackDuration();
case DATE: case DATE:
ensureCorrectStructSize( KnownType.DATE, DATE_SIZE, size ); ensureCorrectStructSize( StructType.DATE, DATE_SIZE, size );
return unpackDate(); return unpackDate();
case LOCAL_TIME: case LOCAL_TIME:
ensureCorrectStructSize( KnownType.LOCAL_TIME, LOCAL_TIME_SIZE, size ); ensureCorrectStructSize( StructType.LOCAL_TIME, LOCAL_TIME_SIZE, size );
return unpackLocalTime(); return unpackLocalTime();
case TIME: case TIME:
ensureCorrectStructSize( KnownType.TIME, TIME_SIZE, size ); ensureCorrectStructSize( StructType.TIME, TIME_SIZE, size );
return unpackTime(); return unpackTime();
case LOCAL_DATE_TIME: case LOCAL_DATE_TIME:
ensureCorrectStructSize( KnownType.LOCAL_DATE_TIME, LOCAL_DATE_TIME_SIZE, size ); ensureCorrectStructSize( StructType.LOCAL_DATE_TIME, LOCAL_DATE_TIME_SIZE, size );
return unpackLocalDateTime(); return unpackLocalDateTime();
case DATE_TIME_WITH_ZONE_OFFSET: case DATE_TIME_WITH_ZONE_OFFSET:
ensureCorrectStructSize( KnownType.DATE_TIME_WITH_ZONE_OFFSET, DATE_TIME_WITH_ZONE_OFFSET_SIZE, size ); ensureCorrectStructSize( StructType.DATE_TIME_WITH_ZONE_OFFSET, DATE_TIME_WITH_ZONE_OFFSET_SIZE, size );
return unpackDateTimeWithZoneOffset(); return unpackDateTimeWithZoneOffset();
case DATE_TIME_WITH_ZONE_NAME: case DATE_TIME_WITH_ZONE_NAME:
ensureCorrectStructSize( KnownType.DATE_TIME_WITH_ZONE_NAME, DATE_TIME_WITH_ZONE_NAME_SIZE, size ); ensureCorrectStructSize( StructType.DATE_TIME_WITH_ZONE_NAME, DATE_TIME_WITH_ZONE_NAME_SIZE, size );
return unpackDateTimeWithZoneName(); return unpackDateTimeWithZoneName();
default: default:
return super.unpackStruct( signature, size ); return super.unpackStruct( signature, size );
Expand All @@ -268,7 +268,7 @@ protected AnyValue unpackStruct( char signature, long size ) throws IOException
} }
catch ( Throwable ex ) catch ( Throwable ex )
{ {
KnownType type = KnownType.valueOf( signature ); StructType type = StructType.valueOf( signature );
if ( type != null ) if ( type != null )
{ {
throw new BoltIOException( Status.Statement.TypeError, throw new BoltIOException( Status.Statement.TypeError,
Expand Down
Expand Up @@ -137,17 +137,6 @@ public byte peekByte() throws IOException
return buffer.get( buffer.position() ); return buffer.get( buffer.position() );
} }


@Override
public void close() throws IOException
{
buffer.clear();

if ( channel != null )
{
channel.close();
}
}

private void ensure( int numBytes ) throws IOException private void ensure( int numBytes ) throws IOException
{ {
if ( !attempt( numBytes ) ) if ( !attempt( numBytes ) )
Expand Down
Expand Up @@ -63,7 +63,7 @@
import static org.neo4j.values.storable.Values.pointValue; import static org.neo4j.values.storable.Values.pointValue;


@RunWith( Parameterized.class ) @RunWith( Parameterized.class )
public class UnsupportedKnownTypesV1IT public class UnsupportedStructTypesV1IT
{ {
private static final String USER_AGENT = "TestClient/1.0"; private static final String USER_AGENT = "TestClient/1.0";


Expand Down
Expand Up @@ -45,7 +45,7 @@
import static org.neo4j.bolt.v1.transport.integration.TransportTestUtil.eventuallyDisconnects; import static org.neo4j.bolt.v1.transport.integration.TransportTestUtil.eventuallyDisconnects;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.auth_enabled; import static org.neo4j.graphdb.factory.GraphDatabaseSettings.auth_enabled;


public class UnsupportedKnownTypesV1V2IT extends AbstractBoltTransportsTest public class UnsupportedStructTypesV1V2IT extends AbstractBoltTransportsTest
{ {
private static final String USER_AGENT = "TestClient/1.0"; private static final String USER_AGENT = "TestClient/1.0";


Expand Down
Expand Up @@ -30,7 +30,7 @@
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;


import org.neo4j.bolt.messaging.KnownType; import org.neo4j.bolt.messaging.StructType;
import org.neo4j.bolt.v1.messaging.BoltRequestMessage; import org.neo4j.bolt.v1.messaging.BoltRequestMessage;
import org.neo4j.bolt.v1.messaging.Neo4jPack; import org.neo4j.bolt.v1.messaging.Neo4jPack;
import org.neo4j.bolt.v1.packstream.PackedOutputArray; import org.neo4j.bolt.v1.packstream.PackedOutputArray;
Expand All @@ -57,7 +57,7 @@
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.auth_enabled; import static org.neo4j.graphdb.factory.GraphDatabaseSettings.auth_enabled;


@RunWith( Parameterized.class ) @RunWith( Parameterized.class )
public class UnsupportedKnownTypesV2IT public class UnsupportedStructTypesV2IT
{ {
private static final String USER_AGENT = "TestClient/2.0"; private static final String USER_AGENT = "TestClient/2.0";


Expand Down Expand Up @@ -99,7 +99,7 @@ public void shouldFailWhenPoint2DIsSentWithInvalidCrsId() throws Exception
{ {
testFailureWithUnpackableValue( packer -> testFailureWithUnpackableValue( packer ->
{ {
packer.packStructHeader( 3, KnownType.POINT_2D.signature() ); packer.packStructHeader( 3, StructType.POINT_2D.signature() );
packer.pack( Values.of( 5 ) ); packer.pack( Values.of( 5 ) );
packer.pack( Values.of( 3.15 ) ); packer.pack( Values.of( 3.15 ) );
packer.pack( Values.of( 4.012 ) ); packer.pack( Values.of( 4.012 ) );
Expand All @@ -111,7 +111,7 @@ public void shouldFailWhenPoint3DIsSentWithInvalidCrsId() throws Exception
{ {
testFailureWithUnpackableValue( packer -> testFailureWithUnpackableValue( packer ->
{ {
packer.packStructHeader( 4, KnownType.POINT_3D.signature() ); packer.packStructHeader( 4, StructType.POINT_3D.signature() );
packer.pack( Values.of( 1200 ) ); packer.pack( Values.of( 1200 ) );
packer.pack( Values.of( 3.15 ) ); packer.pack( Values.of( 3.15 ) );
packer.pack( Values.of( 4.012 ) ); packer.pack( Values.of( 4.012 ) );
Expand All @@ -124,7 +124,7 @@ public void shouldFailWhenPoint2DDimensionsDoNotMatch() throws Exception
{ {
testDisconnectWithUnpackableValue( packer -> testDisconnectWithUnpackableValue( packer ->
{ {
packer.packStructHeader( 3, KnownType.POINT_3D.signature() ); packer.packStructHeader( 3, StructType.POINT_3D.signature() );
packer.pack( Values.of( CoordinateReferenceSystem.Cartesian_3D.getCode() ) ); packer.pack( Values.of( CoordinateReferenceSystem.Cartesian_3D.getCode() ) );
packer.pack( Values.of( 3.15 ) ); packer.pack( Values.of( 3.15 ) );
packer.pack( Values.of( 4.012 ) ); packer.pack( Values.of( 4.012 ) );
Expand All @@ -136,7 +136,7 @@ public void shouldFailWhenPoint3DDimensionsDoNotMatch() throws Exception
{ {
testFailureWithUnpackableValue( packer -> testFailureWithUnpackableValue( packer ->
{ {
packer.packStructHeader( 4, KnownType.POINT_3D.signature() ); packer.packStructHeader( 4, StructType.POINT_3D.signature() );
packer.pack( Values.of( CoordinateReferenceSystem.Cartesian.getCode() ) ); packer.pack( Values.of( CoordinateReferenceSystem.Cartesian.getCode() ) );
packer.pack( Values.of( 3.15 ) ); packer.pack( Values.of( 3.15 ) );
packer.pack( Values.of( 4.012 ) ); packer.pack( Values.of( 4.012 ) );
Expand All @@ -149,7 +149,7 @@ public void shouldFailWhenZonedDateTimeZoneIdIsNotKnown() throws Exception
{ {
testFailureWithUnpackableValue( packer -> testFailureWithUnpackableValue( packer ->
{ {
packer.packStructHeader( 3, KnownType.DATE_TIME_WITH_ZONE_NAME.signature() ); packer.packStructHeader( 3, StructType.DATE_TIME_WITH_ZONE_NAME.signature() );
packer.pack( Values.of( 0 ) ); packer.pack( Values.of( 0 ) );
packer.pack( Values.of( 0 ) ); packer.pack( Values.of( 0 ) );
packer.pack( Values.of( "Europe/Marmaris" ) ); packer.pack( Values.of( "Europe/Marmaris" ) );
Expand Down

0 comments on commit a01b005

Please sign in to comment.