Merge pull request #633 from zhenlineo/4.0-default-db-for-update-rt

Better error message for default database routing table update

Author: zhenlineo

driver/src/main/java/org/neo4j/driver/SessionConfig.java

@@ -26,7 +26,6 @@
 import org.neo4j.driver.reactive.RxSession;
 
 import static java.util.Objects.requireNonNull;
-import static org.neo4j.driver.internal.messaging.request.MultiDatabaseUtil.ABSENT_DB_NAME;
 
 /**
  * The session configurations used to configure a session.
@@ -222,9 +221,9 @@ public Builder withDefaultAccessMode( AccessMode mode )
         public Builder withDatabase( String database )
         {
             requireNonNull( database, "Database name should not be null." );
-            if ( ABSENT_DB_NAME.equals( database ) )
+            if ( database.isEmpty() )
             {
-                // Disallow users to use bolt internal value directly. To users, this is totally an illegal database name.
+                // Empty string is an illegal database name. Fail fast on client.
                 throw new IllegalArgumentException( String.format( "Illegal database name '%s'.", database ) );
             }
             this.database = database;

driver/src/main/java/org/neo4j/driver/internal/DatabaseName.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2002-2019 "Neo4j,"
+ * Neo4j Sweden AB [http://neo4j.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;
+
+import java.util.Optional;
+
+public interface DatabaseName
+{
+    Optional<String> databaseName();
+
+    String description();
+}

driver/src/main/java/org/neo4j/driver/internal/DatabaseNameUtil.java

@@ -0,0 +1,67 @@
+/*
+ * Copyright (c) 2002-2019 "Neo4j,"
+ * Neo4j Sweden AB [http://neo4j.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;
+
+import java.util.Objects;
+import java.util.Optional;
+
+public final class DatabaseNameUtil
+{
+    static final String DEFAULT_DATABASE_NAME = null;
+    public static final String SYSTEM_DATABASE_NAME = "system";
+
+    private static final DatabaseName DEFAULT_DATABASE = new DatabaseName()
+    {
+        @Override
+        public Optional<String> databaseName()
+        {
+            return Optional.empty();
+        }
+
+        @Override
+        public String description()
+        {
+            return "<default database>";
+        }
+    };
+    private static final DatabaseName SYSTEM_DATABASE = new InternalDatabaseName( SYSTEM_DATABASE_NAME );
+
+    public static DatabaseName defaultDatabase()
+    {
+        return DEFAULT_DATABASE;
+    }
+
+    public static DatabaseName systemDatabase()
+    {
+        return SYSTEM_DATABASE;
+    }
+
+    public static DatabaseName database( String name )
+    {
+        if ( Objects.equals( name, DEFAULT_DATABASE_NAME ) )
+        {
+            return defaultDatabase();
+        }
+        else if ( Objects.equals( name, SYSTEM_DATABASE_NAME ) )
+        {
+            return systemDatabase();
+        }
+        return new InternalDatabaseName( name );
+    }
+}

driver/src/main/java/org/neo4j/driver/internal/InternalDatabaseName.java

@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2002-2019 "Neo4j,"
+ * Neo4j Sweden AB [http://neo4j.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;
+
+import java.util.Objects;
+import java.util.Optional;
+
+import static java.util.Objects.requireNonNull;
+
+public class InternalDatabaseName implements DatabaseName
+{
+    private final String databaseName;
+
+    InternalDatabaseName( String databaseName )
+    {
+        this.databaseName = requireNonNull( databaseName );
+    }
+
+    @Override
+    public Optional<String> databaseName()
+    {
+        return Optional.of( databaseName );
+    }
+
+    @Override
+    public String description()
+    {
+        return databaseName;
+    }
+
+    @Override
+    public boolean equals( Object o )
+    {
+        if ( this == o )
+        {
+            return true;
+        }
+        if ( o == null || getClass() != o.getClass() )
+        {
+            return false;
+        }
+        InternalDatabaseName that = (InternalDatabaseName) o;
+        return databaseName.equals( that.databaseName );
+    }
+
+    @Override
+    public int hashCode()
+    {
+        return Objects.hash( databaseName );
+    }
+
+    @Override
+    public String toString()
+    {
+        return "InternalDatabaseName{" + "databaseName='" + databaseName + '\'' + '}';
+    }
+}

driver/src/main/java/org/neo4j/driver/internal/SessionFactoryImpl.java

@@ -18,19 +18,18 @@
  */
 package org.neo4j.driver.internal;
 
+import java.util.Optional;
 import java.util.concurrent.CompletionStage;
 
 import org.neo4j.driver.AccessMode;
 import org.neo4j.driver.Config;
 import org.neo4j.driver.Logging;
 import org.neo4j.driver.SessionConfig;
-import org.neo4j.driver.internal.async.NetworkSession;
 import org.neo4j.driver.internal.async.LeakLoggingNetworkSession;
+import org.neo4j.driver.internal.async.NetworkSession;
 import org.neo4j.driver.internal.retry.RetryLogic;
 import org.neo4j.driver.internal.spi.ConnectionProvider;
 
-import static org.neo4j.driver.internal.messaging.request.MultiDatabaseUtil.ABSENT_DB_NAME;
-
 public class SessionFactoryImpl implements SessionFactory
 {
     private final ConnectionProvider connectionProvider;
@@ -50,10 +49,17 @@ public class SessionFactoryImpl implements SessionFactory
     public NetworkSession newInstance( SessionConfig sessionConfig )
     {
         BookmarkHolder bookmarkHolder = new DefaultBookmarkHolder( InternalBookmark.from( sessionConfig.bookmarks() ) );
-        return createSession( connectionProvider, retryLogic, sessionConfig.database().orElse( ABSENT_DB_NAME ),
+        return createSession( connectionProvider, retryLogic, parseDatabaseName( sessionConfig ),
                 sessionConfig.defaultAccessMode(), bookmarkHolder, logging );
     }
 
+    private DatabaseName parseDatabaseName( SessionConfig sessionConfig )
+    {
+        return sessionConfig.database()
+                .flatMap( name -> Optional.of( DatabaseNameUtil.database( name ) ) )
+                .orElse( DatabaseNameUtil.defaultDatabase() );
+    }
+
     @Override
     public CompletionStage<Void> verifyConnectivity()
     {
@@ -78,7 +84,7 @@ public ConnectionProvider getConnectionProvider()
         return connectionProvider;
     }
 
-    private NetworkSession createSession( ConnectionProvider connectionProvider, RetryLogic retryLogic, String databaseName, AccessMode mode,
+    private NetworkSession createSession( ConnectionProvider connectionProvider, RetryLogic retryLogic, DatabaseName databaseName, AccessMode mode,
             BookmarkHolder bookmarkHolder, Logging logging )
     {
         return leakedSessionsLoggingEnabled

driver/src/main/java/org/neo4j/driver/internal/async/ConnectionContext.java

@@ -19,11 +19,12 @@
 package org.neo4j.driver.internal.async;
 
 import org.neo4j.driver.AccessMode;
+import org.neo4j.driver.internal.DatabaseName;
 import org.neo4j.driver.internal.InternalBookmark;
 
 public interface ConnectionContext
 {
-    String databaseName();
+    DatabaseName databaseName();
 
     AccessMode mode();
 

driver/src/main/java/org/neo4j/driver/internal/async/ImmutableConnectionContext.java

@@ -19,32 +19,33 @@
 package org.neo4j.driver.internal.async;
 
 import org.neo4j.driver.AccessMode;
+import org.neo4j.driver.internal.DatabaseName;
 import org.neo4j.driver.internal.InternalBookmark;
 import org.neo4j.driver.internal.spi.Connection;
 
+import static org.neo4j.driver.internal.DatabaseNameUtil.defaultDatabase;
 import static org.neo4j.driver.internal.InternalBookmark.empty;
-import static org.neo4j.driver.internal.messaging.request.MultiDatabaseUtil.ABSENT_DB_NAME;
 
 /**
  * A {@link Connection} shall fulfil this {@link ImmutableConnectionContext} when acquired from a connection provider.
  */
 public class ImmutableConnectionContext implements ConnectionContext
 {
-    private static final ConnectionContext SIMPLE = new ImmutableConnectionContext( ABSENT_DB_NAME, empty(), AccessMode.READ );
+    private static final ConnectionContext SIMPLE = new ImmutableConnectionContext( defaultDatabase(), empty(), AccessMode.READ );
 
-    private final String databaseName;
+    private final DatabaseName databaseName;
     private final AccessMode mode;
     private final InternalBookmark rediscoveryBookmark;
 
-    public ImmutableConnectionContext( String databaseName, InternalBookmark bookmark, AccessMode mode )
+    public ImmutableConnectionContext( DatabaseName databaseName, InternalBookmark bookmark, AccessMode mode )
     {
         this.databaseName = databaseName;
         this.rediscoveryBookmark = bookmark;
         this.mode = mode;
     }
 
     @Override
-    public String databaseName()
+    public DatabaseName databaseName()
     {
         return databaseName;
     }

driver/src/main/java/org/neo4j/driver/internal/async/LeakLoggingNetworkSession.java

@@ -21,6 +21,7 @@
 import org.neo4j.driver.AccessMode;
 import org.neo4j.driver.Logging;
 import org.neo4j.driver.internal.BookmarkHolder;
+import org.neo4j.driver.internal.DatabaseName;
 import org.neo4j.driver.internal.retry.RetryLogic;
 import org.neo4j.driver.internal.spi.ConnectionProvider;
 import org.neo4j.driver.internal.util.Futures;
@@ -31,7 +32,7 @@ public class LeakLoggingNetworkSession extends NetworkSession
 {
     private final String stackTrace;
 
-    public LeakLoggingNetworkSession( ConnectionProvider connectionProvider, RetryLogic retryLogic, String databaseName, AccessMode mode,
+    public LeakLoggingNetworkSession( ConnectionProvider connectionProvider, RetryLogic retryLogic, DatabaseName databaseName, AccessMode mode,
             BookmarkHolder bookmarkHolder, Logging logging )
     {
         super( connectionProvider, retryLogic, databaseName, mode, bookmarkHolder, logging );

driver/src/main/java/org/neo4j/driver/internal/async/NetworkSession.java

@@ -23,14 +23,15 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.neo4j.driver.AccessMode;
+import org.neo4j.driver.Bookmark;
 import org.neo4j.driver.Logger;
 import org.neo4j.driver.Logging;
 import org.neo4j.driver.Statement;
 import org.neo4j.driver.TransactionConfig;
 import org.neo4j.driver.async.StatementResultCursor;
 import org.neo4j.driver.exceptions.ClientException;
-import org.neo4j.driver.Bookmark;
 import org.neo4j.driver.internal.BookmarkHolder;
+import org.neo4j.driver.internal.DatabaseName;
 import org.neo4j.driver.internal.FailableCursor;
 import org.neo4j.driver.internal.InternalBookmark;
 import org.neo4j.driver.internal.cursor.InternalStatementResultCursor;
@@ -62,7 +63,7 @@ public class NetworkSession
 
     private final AtomicBoolean open = new AtomicBoolean( true );
 
-    public NetworkSession( ConnectionProvider connectionProvider, RetryLogic retryLogic, String databaseName, AccessMode mode,
+    public NetworkSession( ConnectionProvider connectionProvider, RetryLogic retryLogic, DatabaseName databaseName, AccessMode mode,
             BookmarkHolder bookmarkHolder, Logging logging )
     {
         this.connectionProvider = connectionProvider;
@@ -344,17 +345,17 @@ private void ensureSessionIsOpen()
     /**
      * A {@link Connection} shall fulfil this {@link ImmutableConnectionContext} when acquired from a connection provider.
      */
-    private class NetworkSessionConnectionContext implements ConnectionContext
+    private static class NetworkSessionConnectionContext implements ConnectionContext
     {
-        private final String databaseName;
+        private final DatabaseName databaseName;
         private AccessMode mode;
 
         // This bookmark is only used for rediscovery.
         // It has to be the initial bookmark given at the creation of the session.
         // As only that bookmark could carry extra system bookmarks
         private final InternalBookmark rediscoveryBookmark;
 
-        private NetworkSessionConnectionContext( String databaseName, InternalBookmark bookmark )
+        private NetworkSessionConnectionContext( DatabaseName databaseName, InternalBookmark bookmark )
         {
             this.databaseName = databaseName;
             this.rediscoveryBookmark = bookmark;
@@ -367,7 +368,7 @@ private ConnectionContext contextWithMode( AccessMode mode )
         }
 
         @Override
-        public String databaseName()
+        public DatabaseName databaseName()
         {
             return databaseName;
         }

driver/src/main/java/org/neo4j/driver/internal/async/connection/DirectConnection.java

@@ -21,6 +21,7 @@
 import java.util.concurrent.CompletionStage;
 
 import org.neo4j.driver.internal.BoltServerAddress;
+import org.neo4j.driver.internal.DatabaseName;
 import org.neo4j.driver.internal.DirectConnectionProvider;
 import org.neo4j.driver.internal.messaging.BoltProtocol;
 import org.neo4j.driver.internal.messaging.Message;
@@ -36,9 +37,9 @@ public class DirectConnection implements Connection
 {
     private final Connection delegate;
     private final AccessMode mode;
-    private final String databaseName;
+    private final DatabaseName databaseName;
 
-    public DirectConnection( Connection delegate, String databaseName, AccessMode mode )
+    public DirectConnection( Connection delegate, DatabaseName databaseName, AccessMode mode )
     {
         this.delegate = delegate;
         this.mode = mode;
@@ -135,7 +136,7 @@ public AccessMode mode()
     }
 
     @Override
-    public String databaseName()
+    public DatabaseName databaseName()
     {
         return this.databaseName;
     }