Move bookmark out of the NetworkSession constructor

lutovich · lutovich · commit 69555d08aac6 · 2017-02-27T17:33:26.000+01:00
diff --git a/driver/src/main/java/org/neo4j/driver/internal/LeakLoggingNetworkSession.java b/driver/src/main/java/org/neo4j/driver/internal/LeakLoggingNetworkSession.java
@@ -28,10 +28,9 @@ class LeakLoggingNetworkSession extends NetworkSession
 {
     private final String stackTrace;
 
-    LeakLoggingNetworkSession( ConnectionProvider connectionProvider, AccessMode mode, String bookmark,
-            Logging logging )
+    LeakLoggingNetworkSession( ConnectionProvider connectionProvider, AccessMode mode, Logging logging )
     {
-        super( connectionProvider, mode, bookmark, logging );
+        super( connectionProvider, mode, logging );
         this.stackTrace = captureStackTrace();
     }
 
diff --git a/driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java b/driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java
@@ -53,11 +53,10 @@ public class NetworkSession implements Session, SessionResourcesHandler
 
     private final AtomicBoolean isOpen = new AtomicBoolean( true );
 
-    public NetworkSession( ConnectionProvider connectionProvider, AccessMode mode, String bookmark, Logging logging )
+    public NetworkSession( ConnectionProvider connectionProvider, AccessMode mode, Logging logging )
     {
         this.connectionProvider = connectionProvider;
         this.mode = mode;
-        this.lastBookmark = bookmark;
         this.logger = logging.getLog( "Session-" + hashCode() );
     }
 
@@ -99,7 +98,8 @@ public StatementResult run( Statement statement )
         return run( currentConnection, statement, this );
     }
 
-    public static StatementResult run( Connection connection, Statement statement, SessionResourcesHandler resourcesHandler )
+    public static StatementResult run( Connection connection, Statement statement,
+            SessionResourcesHandler resourcesHandler )
     {
         InternalStatementResult result = new InternalStatementResult( connection, resourcesHandler, null, statement );
         connection.run( statement.text(), statement.parameters().asMap( Values.ofValue() ),
@@ -118,7 +118,7 @@ public synchronized void reset()
         if ( currentTransaction != null )
         {
             currentTransaction.markToClose();
-            lastBookmark = currentTransaction.bookmark();
+            setLastBookmark( currentTransaction.bookmark() );
             currentTransaction = null;
         }
         if ( currentConnection != null )
@@ -157,7 +157,7 @@ public void close()
                 }
             }
         }
-        
+
         syncAndCloseCurrentConnection();
     }
 
@@ -170,7 +170,7 @@ public synchronized Transaction beginTransaction()
     @Override
     public synchronized Transaction beginTransaction( String bookmark )
     {
-        lastBookmark = bookmark;
+        setLastBookmark( bookmark );
         return beginTransaction();
     }
 
@@ -186,6 +186,14 @@ public <T> T writeTransaction( Function<Transaction,T> work )
         return transaction( AccessMode.WRITE, work );
     }
 
+    void setLastBookmark( String bookmark )
+    {
+        if ( bookmark != null )
+        {
+            lastBookmark = bookmark;
+        }
+    }
+
     @Override
     public String lastBookmark()
     {
@@ -210,11 +218,7 @@ public synchronized void onTransactionClosed( ExplicitTransaction tx )
         if ( currentTransaction != null && currentTransaction == tx )
         {
             closeCurrentConnection();
-            String bookmark = currentTransaction.bookmark();
-            if ( bookmark != null )
-            {
-                lastBookmark = bookmark;
-            }
+            setLastBookmark( currentTransaction.bookmark() );
             currentTransaction = null;
         }
     }
diff --git a/driver/src/main/java/org/neo4j/driver/internal/SessionFactoryImpl.java b/driver/src/main/java/org/neo4j/driver/internal/SessionFactoryImpl.java
@@ -40,11 +40,17 @@ public class SessionFactoryImpl implements SessionFactory
     @Override
     public Session newInstance( AccessMode mode, String bookmark )
     {
+        NetworkSession session;
         if ( leakedSessionsLoggingEnabled )
         {
-            return new LeakLoggingNetworkSession( connectionProvider, mode, bookmark, logging );
+            session = new LeakLoggingNetworkSession( connectionProvider, mode, logging );
         }
-        return new NetworkSession( connectionProvider, mode, bookmark, logging );
+        else
+        {
+            session = new NetworkSession( connectionProvider, mode, logging );
+        }
+        session.setLastBookmark( bookmark );
+        return session;
     }
 
     @Override
diff --git a/driver/src/test/java/org/neo4j/driver/internal/LeakLoggingNetworkSessionTest.java b/driver/src/test/java/org/neo4j/driver/internal/LeakLoggingNetworkSessionTest.java
@@ -98,7 +98,7 @@ private static void finalize( Session session ) throws Exception
 
     private static LeakLoggingNetworkSession newSession( Logging logging, boolean openConnection )
     {
-        return new LeakLoggingNetworkSession( connectionProviderMock( openConnection ), READ, null, logging );
+        return new LeakLoggingNetworkSession( connectionProviderMock( openConnection ), READ, logging );
     }
 
     private static ConnectionProvider connectionProviderMock( final boolean openConnection )
diff --git a/driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java b/driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java
@@ -758,7 +758,9 @@ private static NetworkSession newSession( ConnectionProvider connectionProvider,
 
     private static NetworkSession newSession( ConnectionProvider connectionProvider, AccessMode mode, String bookmark )
     {
-        return new NetworkSession( connectionProvider, mode, bookmark, DEV_NULL_LOGGING );
+        NetworkSession session = new NetworkSession( connectionProvider, mode, DEV_NULL_LOGGING );
+        session.setLastBookmark( bookmark );
+        return session;
     }
 
     private static PooledConnection openConnectionMock()
diff --git a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java
@@ -438,18 +438,19 @@ private static class NetworkSessionWithAddressFactory extends SessionFactoryImpl
         @Override
         public Session newInstance( AccessMode mode, String bookmark )
         {
-            return new NetworkSessionWithAddress( connectionProvider, mode, bookmark, logging );
+            NetworkSessionWithAddress session = new NetworkSessionWithAddress( connectionProvider, mode, logging );
+            session.setLastBookmark( bookmark );
+            return session;
         }
     }
 
     private static class NetworkSessionWithAddress extends NetworkSession
     {
         final BoltServerAddress address;
 
-        NetworkSessionWithAddress( ConnectionProvider connectionProvider, AccessMode mode, String bookmark,
-                Logging logging )
+        NetworkSessionWithAddress( ConnectionProvider connectionProvider, AccessMode mode, Logging logging )
         {
-            super( connectionProvider, mode, bookmark, logging );
+            super( connectionProvider, mode, logging );
             try ( PooledConnection connection = connectionProvider.acquireConnection( mode ) )
             {
                 this.address = connection.boltServerAddress();
diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/LoadBalancerTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/LoadBalancerTest.java
@@ -33,6 +33,7 @@
 import org.neo4j.driver.internal.spi.ConnectionPool;
 import org.neo4j.driver.internal.spi.PooledConnection;
 import org.neo4j.driver.v1.AccessMode;
+import org.neo4j.driver.v1.Session;
 import org.neo4j.driver.v1.Transaction;
 import org.neo4j.driver.v1.Value;
 import org.neo4j.driver.v1.exceptions.ServiceUnavailableException;
@@ -174,7 +175,7 @@ public void shouldForgetAddressAndItsConnectionsOnServiceUnavailableWhileClosing
         Rediscovery rediscovery = mock( Rediscovery.class );
         LoadBalancer loadBalancer = new LoadBalancer( routingTable, connectionPool, rediscovery, DEV_NULL_LOGGER );
 
-        NetworkSession session = new NetworkSession( loadBalancer, AccessMode.WRITE, null, DEV_NULL_LOGGING );
+        Session session = newSession( loadBalancer );
         // begin transaction to make session obtain a connection
         session.beginTransaction();
 
@@ -206,6 +207,11 @@ private LoadBalancer setupLoadBalancer( PooledConnection writerConn, PooledConne
         return new LoadBalancer( routingTable, connPool, mock( Rediscovery.class ), DEV_NULL_LOGGER );
     }
 
+    private static Session newSession( LoadBalancer loadBalancer )
+    {
+        return new NetworkSession( loadBalancer, AccessMode.WRITE, DEV_NULL_LOGGING );
+    }
+
     private static PooledConnection newConnectionWithFailingSync( BoltServerAddress address )
     {
         PooledConnection connection = mock( PooledConnection.class );

Original file line number	Diff line number	Diff line change
`@@ -28,10 +28,9 @@ class LeakLoggingNetworkSession extends NetworkSession`
`28`	`28`	`{`
`29`	`29`	`private final String stackTrace;`
`30`	`30`
`31`		`- LeakLoggingNetworkSession( ConnectionProvider connectionProvider, AccessMode mode, String bookmark,`
`32`		`- Logging logging )`
	`31`	`+ LeakLoggingNetworkSession( ConnectionProvider connectionProvider, AccessMode mode, Logging logging )`
`33`	`32`	`{`
`34`		`- super( connectionProvider, mode, bookmark, logging );`
	`33`	`+ super( connectionProvider, mode, logging );`
`35`	`34`	`this.stackTrace = captureStackTrace();`
`36`	`35`	`}`
`37`	`36`
Original file line number	Diff line number	Diff line change
`@@ -53,11 +53,10 @@ public class NetworkSession implements Session, SessionResourcesHandler`
`53`	`53`
`54`	`54`	`private final AtomicBoolean isOpen = new AtomicBoolean( true );`
`55`	`55`
`56`		`- public NetworkSession( ConnectionProvider connectionProvider, AccessMode mode, String bookmark, Logging logging )`
	`56`	`+ public NetworkSession( ConnectionProvider connectionProvider, AccessMode mode, Logging logging )`
`57`	`57`	`{`
`58`	`58`	`this.connectionProvider = connectionProvider;`
`59`	`59`	`this.mode = mode;`
`60`		`- this.lastBookmark = bookmark;`
`61`	`60`	`this.logger = logging.getLog( "Session-" + hashCode() );`
`62`	`61`	`}`
`63`	`62`
`@@ -99,7 +98,8 @@ public StatementResult run( Statement statement )`
`99`	`98`	`return run( currentConnection, statement, this );`
`100`	`99`	`}`
`101`	`100`
`102`		`- public static StatementResult run( Connection connection, Statement statement, SessionResourcesHandler resourcesHandler )`
	`101`	`+ public static StatementResult run( Connection connection, Statement statement,`
	`102`	`+ SessionResourcesHandler resourcesHandler )`
`103`	`103`	`{`
`104`	`104`	`InternalStatementResult result = new InternalStatementResult( connection, resourcesHandler, null, statement );`
`105`	`105`	`connection.run( statement.text(), statement.parameters().asMap( Values.ofValue() ),`
`@@ -118,7 +118,7 @@ public synchronized void reset()`
`118`	`118`	`if ( currentTransaction != null )`
`119`	`119`	`{`
`120`	`120`	`currentTransaction.markToClose();`
`121`		`- lastBookmark = currentTransaction.bookmark();`
	`121`	`+ setLastBookmark( currentTransaction.bookmark() );`
`122`	`122`	`currentTransaction = null;`
`123`	`123`	`}`
`124`	`124`	`if ( currentConnection != null )`
`@@ -157,7 +157,7 @@ public void close()`
`157`	`157`	`}`
`158`	`158`	`}`
`159`	`159`	`}`
`160`		`-`
	`160`	`+`
`161`	`161`	`syncAndCloseCurrentConnection();`
`162`	`162`	`}`
`163`	`163`
`@@ -170,7 +170,7 @@ public synchronized Transaction beginTransaction()`
`170`	`170`	`@Override`
`171`	`171`	`public synchronized Transaction beginTransaction( String bookmark )`
`172`	`172`	`{`
`173`		`- lastBookmark = bookmark;`
	`173`	`+ setLastBookmark( bookmark );`
`174`	`174`	`return beginTransaction();`
`175`	`175`	`}`
`176`	`176`
`@@ -186,6 +186,14 @@ public <T> T writeTransaction( Function<Transaction,T> work )`
`186`	`186`	`return transaction( AccessMode.WRITE, work );`
`187`	`187`	`}`
`188`	`188`
	`189`	`+ void setLastBookmark( String bookmark )`
	`190`	`+ {`
	`191`	`+ if ( bookmark != null )`
	`192`	`+ {`
	`193`	`+ lastBookmark = bookmark;`
	`194`	`+ }`
	`195`	`+ }`
	`196`	`+`
`189`	`197`	`@Override`
`190`	`198`	`public String lastBookmark()`
`191`	`199`	`{`
`@@ -210,11 +218,7 @@ public synchronized void onTransactionClosed( ExplicitTransaction tx )`
`210`	`218`	`if ( currentTransaction != null && currentTransaction == tx )`
`211`	`219`	`{`
`212`	`220`	`closeCurrentConnection();`
`213`		`- String bookmark = currentTransaction.bookmark();`
`214`		`- if ( bookmark != null )`
`215`		`- {`
`216`		`- lastBookmark = bookmark;`
`217`		`- }`
	`221`	`+ setLastBookmark( currentTransaction.bookmark() );`
`218`	`222`	`currentTransaction = null;`
`219`	`223`	`}`
`220`	`224`	`}`
Original file line number	Diff line number	Diff line change
`@@ -40,11 +40,17 @@ public class SessionFactoryImpl implements SessionFactory`
`40`	`40`	`@Override`
`41`	`41`	`public Session newInstance( AccessMode mode, String bookmark )`
`42`	`42`	`{`
	`43`	`+ NetworkSession session;`
`43`	`44`	`if ( leakedSessionsLoggingEnabled )`
`44`	`45`	`{`
`45`		`- return new LeakLoggingNetworkSession( connectionProvider, mode, bookmark, logging );`
	`46`	`+ session = new LeakLoggingNetworkSession( connectionProvider, mode, logging );`
`46`	`47`	`}`
`47`		`- return new NetworkSession( connectionProvider, mode, bookmark, logging );`
	`48`	`+ else`
	`49`	`+ {`
	`50`	`+ session = new NetworkSession( connectionProvider, mode, logging );`
	`51`	`+ }`
	`52`	`+ session.setLastBookmark( bookmark );`
	`53`	`+ return session;`
`48`	`54`	`}`
`49`	`55`
`50`	`56`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ private static void finalize( Session session ) throws Exception`
`98`	`98`
`99`	`99`	`private static LeakLoggingNetworkSession newSession( Logging logging, boolean openConnection )`
`100`	`100`	`{`
`101`		`- return new LeakLoggingNetworkSession( connectionProviderMock( openConnection ), READ, null, logging );`
	`101`	`+ return new LeakLoggingNetworkSession( connectionProviderMock( openConnection ), READ, logging );`
`102`	`102`	`}`
`103`	`103`
`104`	`104`	`private static ConnectionProvider connectionProviderMock( final boolean openConnection )`
Original file line number	Diff line number	Diff line change
`@@ -758,7 +758,9 @@ private static NetworkSession newSession( ConnectionProvider connectionProvider,`
`758`	`758`
`759`	`759`	`private static NetworkSession newSession( ConnectionProvider connectionProvider, AccessMode mode, String bookmark )`
`760`	`760`	`{`
`761`		`- return new NetworkSession( connectionProvider, mode, bookmark, DEV_NULL_LOGGING );`
	`761`	`+ NetworkSession session = new NetworkSession( connectionProvider, mode, DEV_NULL_LOGGING );`
	`762`	`+ session.setLastBookmark( bookmark );`
	`763`	`+ return session;`
`762`	`764`	`}`
`763`	`765`
`764`	`766`	`private static PooledConnection openConnectionMock()`
Original file line number	Diff line number	Diff line change
`@@ -438,18 +438,19 @@ private static class NetworkSessionWithAddressFactory extends SessionFactoryImpl`
`438`	`438`	`@Override`
`439`	`439`	`public Session newInstance( AccessMode mode, String bookmark )`
`440`	`440`	`{`
`441`		`- return new NetworkSessionWithAddress( connectionProvider, mode, bookmark, logging );`
	`441`	`+ NetworkSessionWithAddress session = new NetworkSessionWithAddress( connectionProvider, mode, logging );`
	`442`	`+ session.setLastBookmark( bookmark );`
	`443`	`+ return session;`
`442`	`444`	`}`
`443`	`445`	`}`
`444`	`446`
`445`	`447`	`private static class NetworkSessionWithAddress extends NetworkSession`
`446`	`448`	`{`
`447`	`449`	`final BoltServerAddress address;`
`448`	`450`
`449`		`- NetworkSessionWithAddress( ConnectionProvider connectionProvider, AccessMode mode, String bookmark,`
`450`		`- Logging logging )`
	`451`	`+ NetworkSessionWithAddress( ConnectionProvider connectionProvider, AccessMode mode, Logging logging )`
`451`	`452`	`{`
`452`		`- super( connectionProvider, mode, bookmark, logging );`
	`453`	`+ super( connectionProvider, mode, logging );`
`453`	`454`	`try ( PooledConnection connection = connectionProvider.acquireConnection( mode ) )`
`454`	`455`	`{`
`455`	`456`	`this.address = connection.boltServerAddress();`