From cfff31fc008de455c0cda8f1a0712e4daeeba122 Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Wed, 17 Apr 2019 14:48:02 +0200 Subject: [PATCH 1/4] Added database name in the ResultSummary. --- .../summary/InternalDatabaseInfo.java | 41 +++++++++++++++++++ .../summary/InternalResultSummary.java | 14 +++++-- .../internal/util/MetadataExtractor.java | 25 +++++++++-- .../neo4j/driver/summary/DatabaseInfo.java | 31 ++++++++++++++ .../neo4j/driver/summary/ResultSummary.java | 8 +++- .../async/AsyncStatementResultCursorTest.java | 6 +-- .../internal/util/MetadataExtractorTest.java | 29 +++++++++++++ 7 files changed, 143 insertions(+), 11 deletions(-) create mode 100644 driver/src/main/java/org/neo4j/driver/internal/summary/InternalDatabaseInfo.java create mode 100644 driver/src/main/java/org/neo4j/driver/summary/DatabaseInfo.java diff --git a/driver/src/main/java/org/neo4j/driver/internal/summary/InternalDatabaseInfo.java b/driver/src/main/java/org/neo4j/driver/internal/summary/InternalDatabaseInfo.java new file mode 100644 index 0000000000..6e2b314b2b --- /dev/null +++ b/driver/src/main/java/org/neo4j/driver/internal/summary/InternalDatabaseInfo.java @@ -0,0 +1,41 @@ +/* + * 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.summary; + +import org.neo4j.driver.summary.DatabaseInfo; + +import static org.neo4j.driver.internal.messaging.request.MultiDatabaseUtil.ABSENT_DB_NAME; + +public class InternalDatabaseInfo implements DatabaseInfo +{ + public static DatabaseInfo DEFAULT_DATABASE_INFO = new InternalDatabaseInfo( ABSENT_DB_NAME ); + + private final String name; + + public InternalDatabaseInfo( String name ) + { + this.name = name; + } + + @Override + public String name() + { + return this.name; + } +} diff --git a/driver/src/main/java/org/neo4j/driver/internal/summary/InternalResultSummary.java b/driver/src/main/java/org/neo4j/driver/internal/summary/InternalResultSummary.java index bab1155a22..3d00836f0f 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/summary/InternalResultSummary.java +++ b/driver/src/main/java/org/neo4j/driver/internal/summary/InternalResultSummary.java @@ -24,6 +24,7 @@ import java.util.concurrent.TimeUnit; import org.neo4j.driver.Statement; +import org.neo4j.driver.summary.DatabaseInfo; import org.neo4j.driver.summary.Notification; import org.neo4j.driver.summary.Plan; import org.neo4j.driver.summary.ProfiledPlan; @@ -43,13 +44,14 @@ public class InternalResultSummary implements ResultSummary private final List notifications; private final long resultAvailableAfter; private final long resultConsumedAfter; + private final DatabaseInfo databaseInfo; - public InternalResultSummary( Statement statement, ServerInfo serverInfo, StatementType statementType, - SummaryCounters counters, Plan plan, ProfiledPlan profile, List notifications, - long resultAvailableAfter, long resultConsumedAfter ) + public InternalResultSummary( Statement statement, ServerInfo serverInfo, DatabaseInfo databaseInfo, StatementType statementType, + SummaryCounters counters, Plan plan, ProfiledPlan profile, List notifications, long resultAvailableAfter, long resultConsumedAfter ) { this.statement = statement; this.serverInfo = serverInfo; + this.databaseInfo = databaseInfo; this.statementType = statementType; this.counters = counters; this.plan = resolvePlan( plan, profile ); @@ -127,6 +129,12 @@ public ServerInfo server() return serverInfo; } + @Override + public DatabaseInfo database() + { + return databaseInfo; + } + @Override public boolean equals( Object o ) { diff --git a/driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java b/driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java index d39b7192d3..228705a168 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java +++ b/driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java @@ -25,6 +25,7 @@ import org.neo4j.driver.internal.Bookmarks; import org.neo4j.driver.internal.spi.Connection; +import org.neo4j.driver.internal.summary.InternalDatabaseInfo; import org.neo4j.driver.internal.summary.InternalNotification; import org.neo4j.driver.internal.summary.InternalPlan; import org.neo4j.driver.internal.summary.InternalProfiledPlan; @@ -34,6 +35,7 @@ import org.neo4j.driver.Statement; import org.neo4j.driver.Value; import org.neo4j.driver.exceptions.UntrustedServerException; +import org.neo4j.driver.summary.DatabaseInfo; import org.neo4j.driver.summary.Notification; import org.neo4j.driver.summary.Plan; import org.neo4j.driver.summary.ProfiledPlan; @@ -42,6 +44,7 @@ import org.neo4j.driver.summary.StatementType; import static java.util.Collections.emptyList; +import static org.neo4j.driver.internal.summary.InternalDatabaseInfo.DEFAULT_DATABASE_INFO; import static org.neo4j.driver.internal.types.InternalTypeSystem.TYPE_SYSTEM; public class MetadataExtractor @@ -99,12 +102,26 @@ public long extractResultAvailableAfter( Map metadata ) public ResultSummary extractSummary( Statement statement, Connection connection, long resultAvailableAfter, Map metadata ) { ServerInfo serverInfo = new InternalServerInfo( connection.serverAddress(), connection.serverVersion() ); - return new InternalResultSummary( statement, serverInfo, extractStatementType( metadata ), - extractCounters( metadata ), extractPlan( metadata ), extractProfiledPlan( metadata ), - extractNotifications( metadata ), resultAvailableAfter, extractResultConsumedAfter( metadata, resultConsumedAfterMetadataKey ) ); + DatabaseInfo dbInfo = extractDatabaseInfo( metadata ); + return new InternalResultSummary( statement, serverInfo, dbInfo, extractStatementType( metadata ), extractCounters( metadata ), extractPlan( metadata ), + extractProfiledPlan( metadata ), extractNotifications( metadata ), resultAvailableAfter, + extractResultConsumedAfter( metadata, resultConsumedAfterMetadataKey ) ); } - public Bookmarks extractBookmarks( Map metadata ) + public static DatabaseInfo extractDatabaseInfo( Map metadata ) + { + Value dbValue = metadata.get( "db" ); + if ( dbValue == null || dbValue.isNull() || !dbValue.hasType( TYPE_SYSTEM.STRING() ) ) + { + return DEFAULT_DATABASE_INFO; + } + else + { + return new InternalDatabaseInfo( dbValue.asString() ); + } + } + + public static Bookmarks extractBookmarks( Map metadata ) { Value bookmarkValue = metadata.get( "bookmark" ); if ( bookmarkValue != null && !bookmarkValue.isNull() && bookmarkValue.hasType( TYPE_SYSTEM.STRING() ) ) diff --git a/driver/src/main/java/org/neo4j/driver/summary/DatabaseInfo.java b/driver/src/main/java/org/neo4j/driver/summary/DatabaseInfo.java new file mode 100644 index 0000000000..eeab3e4268 --- /dev/null +++ b/driver/src/main/java/org/neo4j/driver/summary/DatabaseInfo.java @@ -0,0 +1,31 @@ +/* + * 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.summary; + +/** + * Provides basic information about where a {@link ResultSummary} is obtained from. + */ +public interface DatabaseInfo +{ + /** + * The name of the database where a {@link ResultSummary} is obtained from. + * @return the name of the database where a {@link ResultSummary} is obtained from + */ + String name(); +} diff --git a/driver/src/main/java/org/neo4j/driver/summary/ResultSummary.java b/driver/src/main/java/org/neo4j/driver/summary/ResultSummary.java index 9915616fe4..afa1260740 100644 --- a/driver/src/main/java/org/neo4j/driver/summary/ResultSummary.java +++ b/driver/src/main/java/org/neo4j/driver/summary/ResultSummary.java @@ -110,7 +110,13 @@ public interface ResultSummary /** * The basic information of the server where the result is obtained from - * @return basic information of the server where the result is obtain from + * @return basic information of the server where the result is obtained from */ ServerInfo server(); + + /** + * The basic information of the database where the result is obtained from + * @return the basic information of the database where the result is obtained from + */ + DatabaseInfo database(); } diff --git a/driver/src/test/java/org/neo4j/driver/internal/async/AsyncStatementResultCursorTest.java b/driver/src/test/java/org/neo4j/driver/internal/async/AsyncStatementResultCursorTest.java index 1bf48c77f1..6246343ae3 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/async/AsyncStatementResultCursorTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/async/AsyncStatementResultCursorTest.java @@ -59,6 +59,7 @@ import static org.mockito.Mockito.when; import static org.neo4j.driver.Values.value; import static org.neo4j.driver.Values.values; +import static org.neo4j.driver.internal.summary.InternalDatabaseInfo.DEFAULT_DATABASE_INFO; import static org.neo4j.driver.internal.util.Futures.completedWithNull; import static org.neo4j.driver.internal.util.Futures.failedFuture; import static org.neo4j.driver.util.TestUtil.await; @@ -85,9 +86,8 @@ void shouldReturnSummary() PullAllResponseHandler pullAllHandler = mock( PullAllResponseHandler.class ); ResultSummary summary = new InternalResultSummary( new Statement( "RETURN 42" ), - new InternalServerInfo( BoltServerAddress.LOCAL_DEFAULT, ServerVersion.v3_1_0 ), - StatementType.SCHEMA_WRITE, new InternalSummaryCounters( 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 ), - null, null, emptyList(), 42, 42 ); + new InternalServerInfo( BoltServerAddress.LOCAL_DEFAULT, ServerVersion.v3_1_0 ), DEFAULT_DATABASE_INFO, StatementType.SCHEMA_WRITE, + new InternalSummaryCounters( 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 ), null, null, emptyList(), 42, 42 ); when( pullAllHandler.summaryAsync() ).thenReturn( completedFuture( summary ) ); AsyncStatementResultCursor cursor = newCursor( pullAllHandler ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java b/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java index 9305410bbd..058d1ca1b8 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java @@ -32,6 +32,7 @@ import org.neo4j.driver.Value; import org.neo4j.driver.Values; import org.neo4j.driver.exceptions.UntrustedServerException; +import org.neo4j.driver.summary.DatabaseInfo; import org.neo4j.driver.summary.Notification; import org.neo4j.driver.summary.Plan; import org.neo4j.driver.summary.ProfiledPlan; @@ -49,6 +50,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.neo4j.driver.internal.summary.InternalSummaryCounters.EMPTY_STATS; +import static org.neo4j.driver.internal.util.MetadataExtractor.extractDatabaseInfo; import static org.neo4j.driver.internal.util.MetadataExtractor.extractNeo4jServerVersion; import static org.neo4j.driver.Values.parameters; import static org.neo4j.driver.Values.value; @@ -390,6 +392,33 @@ void shouldExtractServerVersion() assertEquals( ServerVersion.v3_5_0, version ); } + + @Test + void shouldExtractDatabase() + { + // Given + Map metadata = singletonMap( "db", value( "MyAwesomeDatabase" ) ); + + // When + DatabaseInfo db = extractDatabaseInfo( metadata ); + + // Then + assertEquals( "MyAwesomeDatabase", db.name() ); + } + + @Test + void shouldDefaultToEmptyDatabaseName() + { + // Given + Map metadata = singletonMap( "no_db", value( "no_db" ) ); + + // When + DatabaseInfo db = extractDatabaseInfo( metadata ); + + // Then + assertEquals( "", db.name() ); + } + @Test void shouldFailToExtractServerVersionWhenMetadataDoesNotContainIt() { From bae2507a0de0be60236690d7a72c801252a21504 Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Mon, 29 Apr 2019 11:54:54 +0200 Subject: [PATCH 2/4] Default to null when database name is absent for server versions lower than 4.0 --- .../summary/InternalDatabaseInfo.java | 33 +++++++++++++++++-- .../summary/InternalResultSummary.java | 1 + .../internal/summary/InternalServerInfo.java | 29 ++++++++++++++++ .../neo4j/driver/summary/DatabaseInfo.java | 1 + .../internal/util/MetadataExtractorTest.java | 4 +-- 5 files changed, 63 insertions(+), 5 deletions(-) diff --git a/driver/src/main/java/org/neo4j/driver/internal/summary/InternalDatabaseInfo.java b/driver/src/main/java/org/neo4j/driver/internal/summary/InternalDatabaseInfo.java index 6e2b314b2b..039d1bf529 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/summary/InternalDatabaseInfo.java +++ b/driver/src/main/java/org/neo4j/driver/internal/summary/InternalDatabaseInfo.java @@ -18,13 +18,13 @@ */ package org.neo4j.driver.internal.summary; -import org.neo4j.driver.summary.DatabaseInfo; +import java.util.Objects; -import static org.neo4j.driver.internal.messaging.request.MultiDatabaseUtil.ABSENT_DB_NAME; +import org.neo4j.driver.summary.DatabaseInfo; public class InternalDatabaseInfo implements DatabaseInfo { - public static DatabaseInfo DEFAULT_DATABASE_INFO = new InternalDatabaseInfo( ABSENT_DB_NAME ); + public static DatabaseInfo DEFAULT_DATABASE_INFO = new InternalDatabaseInfo( null ); private final String name; @@ -38,4 +38,31 @@ public String name() { return this.name; } + + @Override + public boolean equals( Object o ) + { + if ( this == o ) + { + return true; + } + if ( o == null || getClass() != o.getClass() ) + { + return false; + } + InternalDatabaseInfo that = (InternalDatabaseInfo) o; + return Objects.equals( name, that.name ); + } + + @Override + public int hashCode() + { + return Objects.hash( name ); + } + + @Override + public String toString() + { + return "InternalDatabaseInfo{" + "name='" + name + '\'' + '}'; + } } diff --git a/driver/src/main/java/org/neo4j/driver/internal/summary/InternalResultSummary.java b/driver/src/main/java/org/neo4j/driver/internal/summary/InternalResultSummary.java index 3d00836f0f..ba7484109d 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/summary/InternalResultSummary.java +++ b/driver/src/main/java/org/neo4j/driver/internal/summary/InternalResultSummary.java @@ -171,6 +171,7 @@ public String toString() return "InternalResultSummary{" + "statement=" + statement + ", serverInfo=" + serverInfo + + ", databaseInfo=" + databaseInfo + ", statementType=" + statementType + ", counters=" + counters + ", plan=" + plan + diff --git a/driver/src/main/java/org/neo4j/driver/internal/summary/InternalServerInfo.java b/driver/src/main/java/org/neo4j/driver/internal/summary/InternalServerInfo.java index 34c41000ab..58b5db8dbb 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/summary/InternalServerInfo.java +++ b/driver/src/main/java/org/neo4j/driver/internal/summary/InternalServerInfo.java @@ -18,6 +18,8 @@ */ package org.neo4j.driver.internal.summary; +import java.util.Objects; + import org.neo4j.driver.internal.BoltServerAddress; import org.neo4j.driver.internal.util.ServerVersion; import org.neo4j.driver.summary.ServerInfo; @@ -44,4 +46,31 @@ public String version() { return version; } + + @Override + public boolean equals( Object o ) + { + if ( this == o ) + { + return true; + } + if ( o == null || getClass() != o.getClass() ) + { + return false; + } + InternalServerInfo that = (InternalServerInfo) o; + return Objects.equals( address, that.address ) && Objects.equals( version, that.version ); + } + + @Override + public int hashCode() + { + return Objects.hash( address, version ); + } + + @Override + public String toString() + { + return "InternalServerInfo{" + "address='" + address + '\'' + ", version='" + version + '\'' + '}'; + } } diff --git a/driver/src/main/java/org/neo4j/driver/summary/DatabaseInfo.java b/driver/src/main/java/org/neo4j/driver/summary/DatabaseInfo.java index eeab3e4268..b646e603bd 100644 --- a/driver/src/main/java/org/neo4j/driver/summary/DatabaseInfo.java +++ b/driver/src/main/java/org/neo4j/driver/summary/DatabaseInfo.java @@ -25,6 +25,7 @@ public interface DatabaseInfo { /** * The name of the database where a {@link ResultSummary} is obtained from. + * Default to {@code null} if servers does not support multi-databases. * @return the name of the database where a {@link ResultSummary} is obtained from */ String name(); diff --git a/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java b/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java index 058d1ca1b8..67355ab5a6 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java @@ -407,7 +407,7 @@ void shouldExtractDatabase() } @Test - void shouldDefaultToEmptyDatabaseName() + void shouldDefaultToNullDatabaseName() { // Given Map metadata = singletonMap( "no_db", value( "no_db" ) ); @@ -416,7 +416,7 @@ void shouldDefaultToEmptyDatabaseName() DatabaseInfo db = extractDatabaseInfo( metadata ); // Then - assertEquals( "", db.name() ); + assertNull( db.name() ); } @Test From a37b98f0153c50a7aa956e656b5615223ee0b651 Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Mon, 6 May 2019 13:37:11 +0200 Subject: [PATCH 3/4] Fix after review --- .../internal/util/MetadataExtractor.java | 8 ++--- .../internal/util/MetadataExtractorTest.java | 30 ++++++++++++++----- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java b/driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java index 228705a168..e68aca9648 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java +++ b/driver/src/main/java/org/neo4j/driver/internal/util/MetadataExtractor.java @@ -23,6 +23,9 @@ import java.util.List; import java.util.Map; +import org.neo4j.driver.Statement; +import org.neo4j.driver.Value; +import org.neo4j.driver.exceptions.UntrustedServerException; import org.neo4j.driver.internal.Bookmarks; import org.neo4j.driver.internal.spi.Connection; import org.neo4j.driver.internal.summary.InternalDatabaseInfo; @@ -32,9 +35,6 @@ import org.neo4j.driver.internal.summary.InternalResultSummary; import org.neo4j.driver.internal.summary.InternalServerInfo; import org.neo4j.driver.internal.summary.InternalSummaryCounters; -import org.neo4j.driver.Statement; -import org.neo4j.driver.Value; -import org.neo4j.driver.exceptions.UntrustedServerException; import org.neo4j.driver.summary.DatabaseInfo; import org.neo4j.driver.summary.Notification; import org.neo4j.driver.summary.Plan; @@ -111,7 +111,7 @@ public ResultSummary extractSummary( Statement statement, Connection connection, public static DatabaseInfo extractDatabaseInfo( Map metadata ) { Value dbValue = metadata.get( "db" ); - if ( dbValue == null || dbValue.isNull() || !dbValue.hasType( TYPE_SYSTEM.STRING() ) ) + if ( dbValue == null || dbValue.isNull() ) { return DEFAULT_DATABASE_INFO; } diff --git a/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java b/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java index 67355ab5a6..bf17bbd56a 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/util/MetadataExtractorTest.java @@ -24,14 +24,15 @@ import java.util.Map; import java.util.concurrent.TimeUnit; -import org.neo4j.driver.internal.BoltServerAddress; -import org.neo4j.driver.internal.Bookmarks; -import org.neo4j.driver.internal.spi.Connection; -import org.neo4j.driver.internal.summary.InternalInputPosition; import org.neo4j.driver.Statement; import org.neo4j.driver.Value; import org.neo4j.driver.Values; import org.neo4j.driver.exceptions.UntrustedServerException; +import org.neo4j.driver.exceptions.value.Uncoercible; +import org.neo4j.driver.internal.BoltServerAddress; +import org.neo4j.driver.internal.Bookmarks; +import org.neo4j.driver.internal.spi.Connection; +import org.neo4j.driver.internal.summary.InternalInputPosition; import org.neo4j.driver.summary.DatabaseInfo; import org.neo4j.driver.summary.Notification; import org.neo4j.driver.summary.Plan; @@ -42,6 +43,8 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; +import static org.hamcrest.CoreMatchers.startsWith; +import static org.hamcrest.junit.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; @@ -49,12 +52,12 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import static org.neo4j.driver.internal.summary.InternalSummaryCounters.EMPTY_STATS; -import static org.neo4j.driver.internal.util.MetadataExtractor.extractDatabaseInfo; -import static org.neo4j.driver.internal.util.MetadataExtractor.extractNeo4jServerVersion; import static org.neo4j.driver.Values.parameters; import static org.neo4j.driver.Values.value; import static org.neo4j.driver.Values.values; +import static org.neo4j.driver.internal.summary.InternalSummaryCounters.EMPTY_STATS; +import static org.neo4j.driver.internal.util.MetadataExtractor.extractDatabaseInfo; +import static org.neo4j.driver.internal.util.MetadataExtractor.extractNeo4jServerVersion; import static org.neo4j.driver.summary.StatementType.READ_ONLY; import static org.neo4j.driver.summary.StatementType.READ_WRITE; import static org.neo4j.driver.summary.StatementType.SCHEMA_WRITE; @@ -419,6 +422,19 @@ void shouldDefaultToNullDatabaseName() assertNull( db.name() ); } + @Test + void shouldErrorWhenTypeIsWrong() + { + // Given + Map metadata = singletonMap( "db", value( 10L ) ); + + // When + Uncoercible error = assertThrows( Uncoercible.class, () -> extractDatabaseInfo( metadata ) ); + + // Then + assertThat( error.getMessage(), startsWith( "Cannot coerce INTEGER to Java String" ) ); + } + @Test void shouldFailToExtractServerVersionWhenMetadataDoesNotContainIt() { From 80203b3f19cd3a4535ac6cd907218164df5c7ed1 Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Mon, 6 May 2019 15:06:26 +0200 Subject: [PATCH 4/4] 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. --- .../driver/SessionParametersTemplate.java | 5 ++-- .../driver/internal/SessionFactoryImpl.java | 4 +++- .../driver/internal/SessionParameters.java | 15 ++++++++---- .../internal/SessionParametersTest.java | 24 ++++++++++++++----- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/driver/src/main/java/org/neo4j/driver/SessionParametersTemplate.java b/driver/src/main/java/org/neo4j/driver/SessionParametersTemplate.java index 0cd0108f83..a0b979549e 100644 --- a/driver/src/main/java/org/neo4j/driver/SessionParametersTemplate.java +++ b/driver/src/main/java/org/neo4j/driver/SessionParametersTemplate.java @@ -58,8 +58,9 @@ public interface SessionParametersTemplate /** * Set the database that the newly created session is going to connect to. - * The given database name cannot be null. - * If the database name is not set, then the default database configured on the server configuration will be connected when the session established. + * If the database name is not set or the database name is set to null, + * then the default database configured in the server configuration will be connected when the session is established. + * For servers that do not support multi-databases, this database value should not be set or could only be set to null. * * @param database the database the session going to connect to. * @return this builder. diff --git a/driver/src/main/java/org/neo4j/driver/internal/SessionFactoryImpl.java b/driver/src/main/java/org/neo4j/driver/internal/SessionFactoryImpl.java index 37ee254375..ed552c77e1 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/SessionFactoryImpl.java +++ b/driver/src/main/java/org/neo4j/driver/internal/SessionFactoryImpl.java @@ -28,6 +28,8 @@ 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; @@ -47,7 +49,7 @@ public class SessionFactoryImpl implements SessionFactory public NetworkSession newInstance( SessionParameters parameters ) { BookmarksHolder bookmarksHolder = new DefaultBookmarksHolder( Bookmarks.from( parameters.bookmarks() ) ); - return createSession( connectionProvider, retryLogic, parameters.database(), parameters.defaultAccessMode(), bookmarksHolder, logging ); + return createSession( connectionProvider, retryLogic, parameters.database().orElse( ABSENT_DB_NAME ), parameters.defaultAccessMode(), bookmarksHolder, logging ); } @Override diff --git a/driver/src/main/java/org/neo4j/driver/internal/SessionParameters.java b/driver/src/main/java/org/neo4j/driver/internal/SessionParameters.java index 8a245a81a3..d6b2257fa3 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/SessionParameters.java +++ b/driver/src/main/java/org/neo4j/driver/internal/SessionParameters.java @@ -21,6 +21,7 @@ import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.Optional; import org.neo4j.driver.AccessMode; import org.neo4j.driver.SessionParametersTemplate; @@ -87,11 +88,11 @@ public AccessMode defaultAccessMode() /** * The database where the session is going to connect to. - * @return the database name where the session is going to connect to. + * @return the nullable database name where the session is going to connect to. */ - public String database() + public Optional database() { - return database; + return Optional.ofNullable( database ); } @Override @@ -125,7 +126,7 @@ public static class Template implements SessionParametersTemplate { private List bookmarks = null; private AccessMode defaultAccessMode = AccessMode.WRITE; - private String database = ABSENT_DB_NAME; + private String database = null; private Template() { @@ -162,7 +163,11 @@ public Template withDefaultAccessMode( AccessMode mode ) @Override public Template withDatabase( String database ) { - Objects.requireNonNull( database, "Database cannot be null." ); + if ( ABSENT_DB_NAME.equals( database ) ) + { + // Disallow users to use bolt internal value directly. To users, this is totally an illegal database name. + throw new IllegalArgumentException( String.format( "Illegal database name '%s'.", database ) ); + } this.database = database; return this; } diff --git a/driver/src/test/java/org/neo4j/driver/internal/SessionParametersTest.java b/driver/src/test/java/org/neo4j/driver/internal/SessionParametersTest.java index 97543346e5..9a002e64d5 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/SessionParametersTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/SessionParametersTest.java @@ -34,7 +34,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.neo4j.driver.internal.SessionParameters.empty; import static org.neo4j.driver.internal.SessionParameters.template; import static org.neo4j.driver.internal.messaging.request.MultiDatabaseUtil.ABSENT_DB_NAME; @@ -47,7 +49,7 @@ void shouldReturnDefaultValues() throws Throwable SessionParameters parameters = empty(); Assert.assertEquals( AccessMode.WRITE, parameters.defaultAccessMode() ); - assertEquals( ABSENT_DB_NAME, parameters.database() ); + assertFalse( parameters.database().isPresent() ); assertNull( parameters.bookmarks() ); } @@ -60,18 +62,28 @@ void shouldChangeAccessMode( AccessMode mode ) throws Throwable } @ParameterizedTest - @ValueSource( strings = {"", "foo", "data", ABSENT_DB_NAME} ) + @ValueSource( strings = {"foo", "data", "my awesome database", " "} ) void shouldChangeDatabaseName( String databaseName ) { SessionParameters parameters = template().withDatabase( databaseName ).build(); - assertEquals( databaseName, parameters.database() ); + assertTrue( parameters.database().isPresent() ); + assertEquals( databaseName, parameters.database().get() ); } @Test - void shouldForbiddenNullDatabaseName() throws Throwable + void shouldAllowNullDatabaseName() throws Throwable { - NullPointerException error = assertThrows( NullPointerException.class, () -> template().withDatabase( null ).build()); - assertThat( error.getMessage(), equalTo( "Database cannot be null." ) ); + SessionParameters parameters = template().withDatabase( null ).build(); + assertFalse( parameters.database().isPresent() ); + assertEquals( "", parameters.database().orElse( ABSENT_DB_NAME ) ); + } + + @ParameterizedTest + @ValueSource( strings = {"", ABSENT_DB_NAME} ) + void shouldForbiddenEmptyStringDatabaseName( String databaseName ) throws Throwable + { + IllegalArgumentException error = assertThrows( IllegalArgumentException.class, () -> template().withDatabase( databaseName ).build()); + assertThat( error.getMessage(), equalTo( "Illegal database name ''." ) ); } @Test