From 90a1685a2638342f7c9f6e06aed21d687106439d Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Thu, 19 Jul 2018 20:59:05 +0200 Subject: [PATCH] Fix after review --- .../v1/runtime/TransactionStateMachine.java | 22 ++++++++-------- .../v3/messaging/decoder/StatementMode.java | 6 ++--- .../v3/messaging/request/BeginMessage.java | 25 +++++++++++++++---- .../v3/runtime/TransactionReadyState.java | 8 +++--- .../BoltStateMachineFactoryImplTest.java | 2 +- .../integration/BoltV3TransportIT.java | 8 +++--- .../runtime/integration/ConnectedStateIT.java | 2 +- 7 files changed, 45 insertions(+), 28 deletions(-) diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/TransactionStateMachine.java b/community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/TransactionStateMachine.java index b62728acfc00..10be240e5a21 100644 --- a/community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/TransactionStateMachine.java +++ b/community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/TransactionStateMachine.java @@ -248,27 +248,18 @@ enum State @Override State beginTransaction( MutableTransactionState ctx, TransactionStateMachineSPI spi, Bookmark bookmark ) throws KernelException { - waitForBookmark( ctx, spi, bookmark ); + waitForBookmark( spi, bookmark ); ctx.currentResult = BoltResult.EMPTY; ctx.currentTransaction = spi.beginTransaction( ctx.loginContext ); return EXPLICIT_TRANSACTION; } - private void waitForBookmark( MutableTransactionState ctx, TransactionStateMachineSPI spi, Bookmark bookmark ) - throws TransactionFailureException - { - if ( bookmark != null ) - { - spi.awaitUpToDate( bookmark.txId() ); - } - } - @Override State run( MutableTransactionState ctx, TransactionStateMachineSPI spi, String statement, MapValue params, Bookmark bookmark ) throws KernelException { statement = parseStatement( ctx, statement ); - waitForBookmark( ctx, spi, bookmark ); + waitForBookmark( spi, bookmark ); execute( ctx, spi, statement, params, spi.isPeriodicCommit( statement ) ); return AUTO_COMMIT; } @@ -506,6 +497,15 @@ void startExecution( MutableTransactionState ctx, BoltResultHandle resultHandle } + private static void waitForBookmark( TransactionStateMachineSPI spi, Bookmark bookmark ) + throws TransactionFailureException + { + if ( bookmark != null ) + { + spi.awaitUpToDate( bookmark.txId() ); + } + } + private static Bookmark newestBookmark( TransactionStateMachineSPI spi ) { long txId = spi.newestEncounteredTxId(); diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/decoder/StatementMode.java b/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/decoder/StatementMode.java index 85a7c6cd8b3f..2889848c304d 100644 --- a/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/decoder/StatementMode.java +++ b/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/decoder/StatementMode.java @@ -22,7 +22,7 @@ import org.neo4j.bolt.messaging.BoltIOException; import org.neo4j.kernel.api.exceptions.Status; -public enum StatementMode //TODO is this already somewhere? +public enum StatementMode { READ( "R" ), WRITE( "W" ); @@ -41,11 +41,11 @@ public String signature() public static StatementMode parseMode( String str ) throws BoltIOException { - if ( str.equalsIgnoreCase( READ.signature() ) ) + if ( READ.signature().equalsIgnoreCase( str ) ) { return READ; } - else if ( str.equalsIgnoreCase( WRITE.signature() ) ) + else if ( WRITE.signature().equalsIgnoreCase( str ) ) { return WRITE; } diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/request/BeginMessage.java b/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/request/BeginMessage.java index 9068ed7e005f..f4a197ab4b50 100644 --- a/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/request/BeginMessage.java +++ b/community/bolt/src/main/java/org/neo4j/bolt/v3/messaging/request/BeginMessage.java @@ -31,6 +31,7 @@ import org.neo4j.values.AnyValue; import org.neo4j.values.storable.LongValue; import org.neo4j.values.storable.TextValue; +import org.neo4j.values.storable.Values; import org.neo4j.values.virtual.MapValue; import org.neo4j.values.virtual.VirtualValues; @@ -72,24 +73,38 @@ static Bookmark parseBookmark( MapValue meta ) throws BoltIOException } } - static Duration parseTransactionTimeout( MapValue meta ) + static Duration parseTransactionTimeout( MapValue meta ) throws BoltIOException { AnyValue anyValue = meta.get( TX_TIMEOUT_KEY ); - if ( anyValue instanceof LongValue ) + if ( anyValue == Values.NO_VALUE ) + { + return null; + } + else if ( anyValue instanceof LongValue ) { return Duration.ofMillis( ((LongValue) anyValue).longValue() ); } - return null; + else + { + throw new BoltIOException( Status.Request.InvalidFormat, "Expecting transaction timeout value to be a Long value, but got: " + anyValue ); + } } static StatementMode parseStatementMode( MapValue meta ) throws BoltIOException { AnyValue anyValue = meta.get( MODE_KEY ); - if ( anyValue instanceof TextValue ) + if ( anyValue == Values.NO_VALUE ) + { + return null; + } + else if ( anyValue instanceof TextValue ) { return StatementMode.parseMode( ((TextValue) anyValue).stringValue() ); } - return null; + else + { + throw new BoltIOException( Status.Request.InvalidFormat, "Expecting transaction statement mode value to be a String value, but got: " + anyValue ); + } } public Bookmark bookmark() diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v3/runtime/TransactionReadyState.java b/community/bolt/src/main/java/org/neo4j/bolt/v3/runtime/TransactionReadyState.java index 738cdbb9bf41..52cec5658202 100644 --- a/community/bolt/src/main/java/org/neo4j/bolt/v3/runtime/TransactionReadyState.java +++ b/community/bolt/src/main/java/org/neo4j/bolt/v3/runtime/TransactionReadyState.java @@ -40,7 +40,7 @@ public class TransactionReadyState extends FailSafeBoltStateMachineState { - private BoltStateMachineState streaming; + private BoltStateMachineState streamingState; private BoltStateMachineState interruptedState; private BoltStateMachineState readyState; @@ -75,7 +75,7 @@ public String name() public void setTransactionStreamingState( BoltStateMachineState streamingState ) { - this.streaming = streamingState; + this.streamingState = streamingState; } public void setInterruptedState( BoltStateMachineState interruptedState ) @@ -97,7 +97,7 @@ private BoltStateMachineState processRunMessage( RunMessage message, StateMachin context.connectionState().onMetadata( FIELDS_KEY, stringArray( statementMetadata.fieldNames() ) ); context.connectionState().onMetadata( FIRST_RECORD_AVAILABLE_KEY, Values.longValue( end - start ) ); - return streaming; + return streamingState; } private BoltStateMachineState processCommitMessage( StateMachineContext context ) throws Exception @@ -117,7 +117,7 @@ private BoltStateMachineState processRollbackMessage( StateMachineContext contex private void assertInitialized() { - checkState( streaming != null, "Streaming state not set" ); + checkState( streamingState != null, "Streaming state not set" ); checkState( interruptedState != null, "Interrupted state not set" ); checkState( readyState != null, "Ready state not set" ); checkState( failedState != null, "Failed state not set" ); diff --git a/community/bolt/src/test/java/org/neo4j/bolt/v1/runtime/BoltStateMachineFactoryImplTest.java b/community/bolt/src/test/java/org/neo4j/bolt/v1/runtime/BoltStateMachineFactoryImplTest.java index b7fca634938f..b151c02bffcc 100644 --- a/community/bolt/src/test/java/org/neo4j/bolt/v1/runtime/BoltStateMachineFactoryImplTest.java +++ b/community/bolt/src/test/java/org/neo4j/bolt/v1/runtime/BoltStateMachineFactoryImplTest.java @@ -58,7 +58,7 @@ class BoltStateMachineFactoryImplTest @ParameterizedTest( name = "V{0}" ) @ValueSource( longs = {BoltProtocolV1.VERSION, BoltProtocolV2.VERSION} ) - void shouldCreateBoltStateMachines( long protocolVersion ) throws Throwable + void shouldCreateBoltStateMachinesV1( long protocolVersion ) throws Throwable { BoltStateMachineFactoryImpl factory = newBoltFactory(); diff --git a/community/community-it/bolt-it/src/test/java/org/neo4j/bolt/v3/runtime/integration/BoltV3TransportIT.java b/community/community-it/bolt-it/src/test/java/org/neo4j/bolt/v3/runtime/integration/BoltV3TransportIT.java index 63ec67c5dcc2..63cb6b68c6ba 100644 --- a/community/community-it/bolt-it/src/test/java/org/neo4j/bolt/v3/runtime/integration/BoltV3TransportIT.java +++ b/community/community-it/bolt-it/src/test/java/org/neo4j/bolt/v3/runtime/integration/BoltV3TransportIT.java @@ -38,8 +38,11 @@ import org.neo4j.bolt.v1.messaging.request.ResetMessage; import org.neo4j.bolt.v1.transport.integration.Neo4jWithSocket; import org.neo4j.bolt.v1.transport.integration.TransportTestUtil; +import org.neo4j.bolt.v1.transport.socket.client.SecureSocketConnection; +import org.neo4j.bolt.v1.transport.socket.client.SecureWebSocketConnection; import org.neo4j.bolt.v1.transport.socket.client.SocketConnection; import org.neo4j.bolt.v1.transport.socket.client.TransportConnection; +import org.neo4j.bolt.v1.transport.socket.client.WebSocketConnection; import org.neo4j.bolt.v3.messaging.request.BeginMessage; import org.neo4j.bolt.v3.messaging.request.HelloMessage; import org.neo4j.bolt.v3.messaging.request.RunMessage; @@ -90,8 +93,7 @@ public class BoltV3TransportIT @Parameters( name = "{0}" ) public static List> transports() { - return asList( SocketConnection.class ); - //, WebSocketConnection.class, SecureSocketConnection.class, SecureWebSocketConnection.class ); + return asList( SocketConnection.class, WebSocketConnection.class, SecureSocketConnection.class, SecureWebSocketConnection.class ); } @Before @@ -123,7 +125,7 @@ public void shouldNegotiateProtocolV3() throws Exception } @Test - public void shouldNegotiateProtocolV2WhenClientSupportsBothV1V2AndV3() throws Exception + public void shouldNegotiateProtocolV3WhenClientSupportsBothV1V2AndV3() throws Exception { connection.connect( address ) .send( util.acceptedVersions( 3, 2, 1, 0 ) ) diff --git a/community/community-it/bolt-it/src/test/java/org/neo4j/bolt/v3/runtime/integration/ConnectedStateIT.java b/community/community-it/bolt-it/src/test/java/org/neo4j/bolt/v3/runtime/integration/ConnectedStateIT.java index abdf5cd81cc8..830cd0d1d3da 100644 --- a/community/community-it/bolt-it/src/test/java/org/neo4j/bolt/v3/runtime/integration/ConnectedStateIT.java +++ b/community/community-it/bolt-it/src/test/java/org/neo4j/bolt/v3/runtime/integration/ConnectedStateIT.java @@ -54,7 +54,7 @@ class ConnectedStateIT extends BoltStateMachineStateTestBase { @Test - void shouldExecuteStatement() throws Throwable + void shouldHandleHelloMessage() throws Throwable { // Given BoltStateMachineV3 machine = newStateMachine();