Skip to content

Commit 80203b3

Browse files
author
Zhen Li
committed
Unified the input and output value of database name.
The database name default to `null` if the server does not support multi-databases when the database name is returned in result summary. However `null` is a forbidden value for database in session parameters. Then for user who want to connect to default db but do not want ot omit invoke `withDatabase` method on session parameter, they have to use empty string to work aroud our API design. This PR allows `null` as a valid database name for session parameters, while disallows empty string as it is an invalid database name to users.
1 parent a37b98f commit 80203b3

File tree

4 files changed

+34
-14
lines changed

4 files changed

+34
-14
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,9 @@ public interface SessionParametersTemplate
5858

5959
/**
6060
* Set the database that the newly created session is going to connect to.
61-
* The given database name cannot be <code>null</code>.
62-
* If the database name is not set, then the default database configured on the server configuration will be connected when the session established.
61+
* If the database name is not set or the database name is set to <code>null</code>,
62+
* then the default database configured in the server configuration will be connected when the session is established.
63+
* For servers that do not support multi-databases, this database value should not be set or could only be set to <code>null</code>.
6364
*
6465
* @param database the database the session going to connect to.
6566
* @return this builder.

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import org.neo4j.driver.internal.retry.RetryLogic;
2929
import org.neo4j.driver.internal.spi.ConnectionProvider;
3030

31+
import static org.neo4j.driver.internal.messaging.request.MultiDatabaseUtil.ABSENT_DB_NAME;
32+
3133
public class SessionFactoryImpl implements SessionFactory
3234
{
3335
private final ConnectionProvider connectionProvider;
@@ -47,7 +49,7 @@ public class SessionFactoryImpl implements SessionFactory
4749
public NetworkSession newInstance( SessionParameters parameters )
4850
{
4951
BookmarksHolder bookmarksHolder = new DefaultBookmarksHolder( Bookmarks.from( parameters.bookmarks() ) );
50-
return createSession( connectionProvider, retryLogic, parameters.database(), parameters.defaultAccessMode(), bookmarksHolder, logging );
52+
return createSession( connectionProvider, retryLogic, parameters.database().orElse( ABSENT_DB_NAME ), parameters.defaultAccessMode(), bookmarksHolder, logging );
5153
}
5254

5355
@Override

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.Arrays;
2222
import java.util.List;
2323
import java.util.Objects;
24+
import java.util.Optional;
2425

2526
import org.neo4j.driver.AccessMode;
2627
import org.neo4j.driver.SessionParametersTemplate;
@@ -87,11 +88,11 @@ public AccessMode defaultAccessMode()
8788

8889
/**
8990
* The database where the session is going to connect to.
90-
* @return the database name where the session is going to connect to.
91+
* @return the nullable database name where the session is going to connect to.
9192
*/
92-
public String database()
93+
public Optional<String> database()
9394
{
94-
return database;
95+
return Optional.ofNullable( database );
9596
}
9697

9798
@Override
@@ -125,7 +126,7 @@ public static class Template implements SessionParametersTemplate
125126
{
126127
private List<String> bookmarks = null;
127128
private AccessMode defaultAccessMode = AccessMode.WRITE;
128-
private String database = ABSENT_DB_NAME;
129+
private String database = null;
129130

130131
private Template()
131132
{
@@ -162,7 +163,11 @@ public Template withDefaultAccessMode( AccessMode mode )
162163
@Override
163164
public Template withDatabase( String database )
164165
{
165-
Objects.requireNonNull( database, "Database cannot be null." );
166+
if ( ABSENT_DB_NAME.equals( database ) )
167+
{
168+
// Disallow users to use bolt internal value directly. To users, this is totally an illegal database name.
169+
throw new IllegalArgumentException( String.format( "Illegal database name '%s'.", database ) );
170+
}
166171
this.database = database;
167172
return this;
168173
}

driver/src/test/java/org/neo4j/driver/internal/SessionParametersTest.java

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@
3434
import static org.junit.Assert.assertEquals;
3535
import static org.junit.Assert.assertNull;
3636
import static org.junit.Assert.assertThat;
37+
import static org.junit.jupiter.api.Assertions.assertFalse;
3738
import static org.junit.jupiter.api.Assertions.assertThrows;
39+
import static org.junit.jupiter.api.Assertions.assertTrue;
3840
import static org.neo4j.driver.internal.SessionParameters.empty;
3941
import static org.neo4j.driver.internal.SessionParameters.template;
4042
import static org.neo4j.driver.internal.messaging.request.MultiDatabaseUtil.ABSENT_DB_NAME;
@@ -47,7 +49,7 @@ void shouldReturnDefaultValues() throws Throwable
4749
SessionParameters parameters = empty();
4850

4951
Assert.assertEquals( AccessMode.WRITE, parameters.defaultAccessMode() );
50-
assertEquals( ABSENT_DB_NAME, parameters.database() );
52+
assertFalse( parameters.database().isPresent() );
5153
assertNull( parameters.bookmarks() );
5254
}
5355

@@ -60,18 +62,28 @@ void shouldChangeAccessMode( AccessMode mode ) throws Throwable
6062
}
6163

6264
@ParameterizedTest
63-
@ValueSource( strings = {"", "foo", "data", ABSENT_DB_NAME} )
65+
@ValueSource( strings = {"foo", "data", "my awesome database", " "} )
6466
void shouldChangeDatabaseName( String databaseName )
6567
{
6668
SessionParameters parameters = template().withDatabase( databaseName ).build();
67-
assertEquals( databaseName, parameters.database() );
69+
assertTrue( parameters.database().isPresent() );
70+
assertEquals( databaseName, parameters.database().get() );
6871
}
6972

7073
@Test
71-
void shouldForbiddenNullDatabaseName() throws Throwable
74+
void shouldAllowNullDatabaseName() throws Throwable
7275
{
73-
NullPointerException error = assertThrows( NullPointerException.class, () -> template().withDatabase( null ).build());
74-
assertThat( error.getMessage(), equalTo( "Database cannot be null." ) );
76+
SessionParameters parameters = template().withDatabase( null ).build();
77+
assertFalse( parameters.database().isPresent() );
78+
assertEquals( "", parameters.database().orElse( ABSENT_DB_NAME ) );
79+
}
80+
81+
@ParameterizedTest
82+
@ValueSource( strings = {"", ABSENT_DB_NAME} )
83+
void shouldForbiddenEmptyStringDatabaseName( String databaseName ) throws Throwable
84+
{
85+
IllegalArgumentException error = assertThrows( IllegalArgumentException.class, () -> template().withDatabase( databaseName ).build());
86+
assertThat( error.getMessage(), equalTo( "Illegal database name ''." ) );
7587
}
7688

7789
@Test

0 commit comments

Comments
 (0)