From 250c4c127f75cc4979e511e2459813f22fec67de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Mon, 18 Jan 2021 00:50:09 +0100 Subject: [PATCH] feat: support default ClientInfo properties (#324) --- .../spanner/jdbc/AbstractJdbcConnection.java | 35 +++++++++-- .../cloud/spanner/jdbc/JdbcConnection.java | 2 +- .../spanner/jdbc/JdbcDatabaseMetaData.java | 61 +++++++++++++++++-- .../JdbcConnectionGeneratedSqlScriptTest.java | 20 +++--- .../spanner/jdbc/JdbcConnectionTest.java | 49 +++++++++++++-- .../jdbc/JdbcDatabaseMetaDataTest.java | 12 ++++ 6 files changed, 157 insertions(+), 22 deletions(-) diff --git a/src/main/java/com/google/cloud/spanner/jdbc/AbstractJdbcConnection.java b/src/main/java/com/google/cloud/spanner/jdbc/AbstractJdbcConnection.java index aad5ad82..385a46fa 100644 --- a/src/main/java/com/google/cloud/spanner/jdbc/AbstractJdbcConnection.java +++ b/src/main/java/com/google/cloud/spanner/jdbc/AbstractJdbcConnection.java @@ -45,19 +45,21 @@ abstract class AbstractJdbcConnection extends AbstractJdbcWrapper private static final String ABORT_UNSUPPORTED = "Abort is not supported"; private static final String NETWORK_TIMEOUT_UNSUPPORTED = "Network timeout is not supported"; static final String CLIENT_INFO_NOT_SUPPORTED = - "Cloud Spanner does not support any ClientInfo properties"; + "Cloud Spanner does not support ClientInfo property %s"; private final String connectionUrl; private final ConnectionOptions options; private final com.google.cloud.spanner.connection.Connection spanner; + private final Properties clientInfo; private SQLWarning firstWarning = null; private SQLWarning lastWarning = null; - AbstractJdbcConnection(String connectionUrl, ConnectionOptions options) { + AbstractJdbcConnection(String connectionUrl, ConnectionOptions options) throws SQLException { this.connectionUrl = connectionUrl; this.options = options; this.spanner = options.getConnection(); + this.clientInfo = new Properties(JdbcDatabaseMetaData.getDefaultClientInfoProperties()); } /** Return the corresponding {@link com.google.cloud.spanner.connection.Connection} */ @@ -168,8 +170,10 @@ public SQLXML createSQLXML() throws SQLException { @Override public void setClientInfo(String name, String value) throws SQLClientInfoException { + Properties supported = null; try { checkClosed(); + supported = JdbcDatabaseMetaData.getDefaultClientInfoProperties(); } catch (SQLException e) { if (e instanceof JdbcSqlException) { throw JdbcSqlExceptionFactory.clientInfoException( @@ -178,7 +182,23 @@ public void setClientInfo(String name, String value) throws SQLClientInfoExcepti throw JdbcSqlExceptionFactory.clientInfoException(e.getMessage(), Code.UNKNOWN); } } - pushWarning(new SQLWarning(CLIENT_INFO_NOT_SUPPORTED)); + if (value == null) { + throw JdbcSqlExceptionFactory.clientInfoException( + "Null-value is not allowed for client info.", Code.INVALID_ARGUMENT); + } + if (value.length() > JdbcDatabaseMetaData.MAX_CLIENT_INFO_VALUE_LENGTH) { + throw JdbcSqlExceptionFactory.clientInfoException( + String.format( + "Max length of value is %d characters.", + JdbcDatabaseMetaData.MAX_CLIENT_INFO_VALUE_LENGTH), + Code.INVALID_ARGUMENT); + } + name = name.toUpperCase(); + if (supported.containsKey(name)) { + clientInfo.setProperty(name, value); + } else { + pushWarning(new SQLWarning(String.format(CLIENT_INFO_NOT_SUPPORTED, name))); + } } @Override @@ -193,19 +213,22 @@ public void setClientInfo(Properties properties) throws SQLClientInfoException { throw JdbcSqlExceptionFactory.clientInfoException(e.getMessage(), Code.UNKNOWN); } } - pushWarning(new SQLWarning(CLIENT_INFO_NOT_SUPPORTED)); + clientInfo.clear(); + for (String property : properties.stringPropertyNames()) { + setClientInfo(property, properties.getProperty(property)); + } } @Override public String getClientInfo(String name) throws SQLException { checkClosed(); - return null; + return clientInfo.getProperty(name.toUpperCase()); } @Override public Properties getClientInfo() throws SQLException { checkClosed(); - return null; + return (Properties) clientInfo.clone(); } @Override diff --git a/src/main/java/com/google/cloud/spanner/jdbc/JdbcConnection.java b/src/main/java/com/google/cloud/spanner/jdbc/JdbcConnection.java index 3d79c5ef..6c23db49 100644 --- a/src/main/java/com/google/cloud/spanner/jdbc/JdbcConnection.java +++ b/src/main/java/com/google/cloud/spanner/jdbc/JdbcConnection.java @@ -53,7 +53,7 @@ class JdbcConnection extends AbstractJdbcConnection { private Map> typeMap = new HashMap<>(); - JdbcConnection(String connectionUrl, ConnectionOptions options) { + JdbcConnection(String connectionUrl, ConnectionOptions options) throws SQLException { super(connectionUrl, options); } diff --git a/src/main/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaData.java b/src/main/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaData.java index 508a2c37..a5d79b14 100644 --- a/src/main/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaData.java +++ b/src/main/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaData.java @@ -36,6 +36,7 @@ import java.sql.Types; import java.util.Arrays; import java.util.Collections; +import java.util.Properties; import java.util.Scanner; /** {@link DatabaseMetaData} implementation for Cloud Spanner */ @@ -1495,16 +1496,68 @@ public boolean autoCommitFailureClosesAllResultSets() throws SQLException { return false; } - @Override - public ResultSet getClientInfoProperties() throws SQLException { + /** + * The max length for client info values is 63 to make them fit in Cloud Spanner session labels. + */ + static final int MAX_CLIENT_INFO_VALUE_LENGTH = 63; + + static Properties getDefaultClientInfoProperties() throws SQLException { + Properties info = new Properties(); + try (ResultSet rs = getDefaultClientInfo()) { + while (rs.next()) { + info.put(rs.getString("NAME"), rs.getString("DEFAULT_VALUE")); + } + } + return info; + } + + private static ResultSet getDefaultClientInfo() throws SQLException { return JdbcResultSet.of( ResultSets.forRows( Type.struct( StructField.of("NAME", Type.string()), - StructField.of("MAX_LEN", Type.string()), + StructField.of("MAX_LEN", Type.int64()), StructField.of("DEFAULT_VALUE", Type.string()), StructField.of("DESCRIPTION", Type.string())), - Collections.emptyList())); + Arrays.asList( + Struct.newBuilder() + .set("NAME") + .to("APPLICATIONNAME") + .set("MAX_LEN") + .to(MAX_CLIENT_INFO_VALUE_LENGTH) + .set("DEFAULT_VALUE") + .to("") + .set("DESCRIPTION") + .to("The name of the application currently utilizing the connection.") + .build(), + Struct.newBuilder() + .set("NAME") + .to("CLIENTHOSTNAME") + .set("MAX_LEN") + .to(MAX_CLIENT_INFO_VALUE_LENGTH) + .set("DEFAULT_VALUE") + .to("") + .set("DESCRIPTION") + .to( + "The hostname of the computer the application using the connection is running on.") + .build(), + Struct.newBuilder() + .set("NAME") + .to("CLIENTUSER") + .set("MAX_LEN") + .to(MAX_CLIENT_INFO_VALUE_LENGTH) + .set("DEFAULT_VALUE") + .to("") + .set("DESCRIPTION") + .to( + "The name of the user that the application using the connection is performing work for. " + + "This may not be the same as the user name that was used in establishing the connection.") + .build()))); + } + + @Override + public ResultSet getClientInfoProperties() throws SQLException { + return getDefaultClientInfo(); } @Override diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionGeneratedSqlScriptTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionGeneratedSqlScriptTest.java index b835668b..b818c63f 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionGeneratedSqlScriptTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionGeneratedSqlScriptTest.java @@ -19,12 +19,14 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import com.google.cloud.spanner.SpannerExceptionFactory; import com.google.cloud.spanner.connection.AbstractSqlScriptVerifier.GenericConnection; import com.google.cloud.spanner.connection.AbstractSqlScriptVerifier.GenericConnectionProvider; import com.google.cloud.spanner.connection.ConnectionImplTest; import com.google.cloud.spanner.connection.ConnectionOptions; import com.google.cloud.spanner.connection.SqlScriptVerifier; import com.google.cloud.spanner.jdbc.JdbcSqlScriptVerifier.JdbcGenericConnection; +import java.sql.SQLException; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -46,13 +48,17 @@ public GenericConnection getConnection() { com.google.cloud.spanner.connection.Connection spannerConnection = ConnectionImplTest.createConnection(options); when(options.getConnection()).thenReturn(spannerConnection); - JdbcConnection connection = - new JdbcConnection( - "jdbc:cloudspanner://localhost/projects/project/instances/instance/databases/database;credentialsUrl=url", - options); - JdbcGenericConnection res = JdbcGenericConnection.of(connection); - res.setStripCommentsBeforeExecute(true); - return res; + try { + JdbcConnection connection = + new JdbcConnection( + "jdbc:cloudspanner://localhost/projects/project/instances/instance/databases/database;credentialsUrl=url", + options); + JdbcGenericConnection res = JdbcGenericConnection.of(connection); + res.setStripCommentsBeforeExecute(true); + return res; + } catch (SQLException e) { + throw SpannerExceptionFactory.asSpannerException(e); + } } } diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionTest.java index 82e26d03..27389b4c 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionTest.java @@ -59,7 +59,7 @@ public class JdbcConnectionTest { Type.struct(StructField.of("", Type.int64())), Arrays.asList(Struct.newBuilder().set("").to(1L).build())); - private JdbcConnection createConnection(ConnectionOptions options) { + private JdbcConnection createConnection(ConnectionOptions options) throws SQLException { com.google.cloud.spanner.connection.Connection spannerConnection = ConnectionImplTest.createConnection(options); when(options.getConnection()).thenReturn(spannerConnection); @@ -405,14 +405,24 @@ public void testWarnings() throws SQLException { } @Test - public void testSetClientInfo() throws SQLException { + public void getDefaultClientInfo() throws SQLException { + ConnectionOptions options = mock(ConnectionOptions.class); + try (JdbcConnection connection = createConnection(options)) { + Properties defaultProperties = connection.getClientInfo(); + assertThat(defaultProperties.stringPropertyNames()) + .containsExactly("APPLICATIONNAME", "CLIENTHOSTNAME", "CLIENTUSER"); + } + } + + @Test + public void testSetInvalidClientInfo() throws SQLException { ConnectionOptions options = mock(ConnectionOptions.class); try (JdbcConnection connection = createConnection(options)) { assertThat((Object) connection.getWarnings()).isNull(); connection.setClientInfo("test", "foo"); assertThat((Object) connection.getWarnings()).isNotNull(); assertThat(connection.getWarnings().getMessage()) - .isEqualTo(AbstractJdbcConnection.CLIENT_INFO_NOT_SUPPORTED); + .isEqualTo(String.format(AbstractJdbcConnection.CLIENT_INFO_NOT_SUPPORTED, "TEST")); connection.clearWarnings(); assertThat((Object) connection.getWarnings()).isNull(); @@ -422,7 +432,38 @@ public void testSetClientInfo() throws SQLException { connection.setClientInfo(props); assertThat((Object) connection.getWarnings()).isNotNull(); assertThat(connection.getWarnings().getMessage()) - .isEqualTo(AbstractJdbcConnection.CLIENT_INFO_NOT_SUPPORTED); + .isEqualTo(String.format(AbstractJdbcConnection.CLIENT_INFO_NOT_SUPPORTED, "TEST")); + } + } + + @Test + public void testSetClientInfo() throws SQLException { + ConnectionOptions options = mock(ConnectionOptions.class); + try (JdbcConnection connection = createConnection(options)) { + try (ResultSet validProperties = connection.getMetaData().getClientInfoProperties()) { + while (validProperties.next()) { + assertThat((Object) connection.getWarnings()).isNull(); + String name = validProperties.getString("NAME"); + + connection.setClientInfo(name, "new-client-info-value"); + assertThat((Object) connection.getWarnings()).isNull(); + assertThat(connection.getClientInfo(name)).isEqualTo("new-client-info-value"); + + Properties props = new Properties(); + props.setProperty(name.toLowerCase(), "some-other-value"); + connection.setClientInfo(props); + assertThat((Object) connection.getWarnings()).isNull(); + assertThat(connection.getClientInfo(name)).isEqualTo("some-other-value"); + assertThat(connection.getClientInfo().keySet()).hasSize(1); + for (String key : connection.getClientInfo().stringPropertyNames()) { + if (key.equals(name)) { + assertThat(connection.getClientInfo().getProperty(key)).isEqualTo("some-other-value"); + } else { + assertThat(connection.getClientInfo().getProperty(key)).isEqualTo(""); + } + } + } + } } } diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaDataTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaDataTest.java index b3697a07..ac8ab7bf 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaDataTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaDataTest.java @@ -322,6 +322,18 @@ public void testGetClientInfoProperties() throws SQLException { JdbcConnection connection = mock(JdbcConnection.class); DatabaseMetaData meta = new JdbcDatabaseMetaData(connection); try (ResultSet rs = meta.getClientInfoProperties()) { + assertThat(rs.next(), is(true)); + assertThat(rs.getString("NAME"), is(equalTo("APPLICATIONNAME"))); + assertThat(rs.getString("DEFAULT_VALUE"), is(equalTo(""))); + + assertThat(rs.next(), is(true)); + assertThat(rs.getString("NAME"), is(equalTo("CLIENTHOSTNAME"))); + assertThat(rs.getString("DEFAULT_VALUE"), is(equalTo(""))); + + assertThat(rs.next(), is(true)); + assertThat(rs.getString("NAME"), is(equalTo("CLIENTUSER"))); + assertThat(rs.getString("DEFAULT_VALUE"), is(equalTo(""))); + assertThat(rs.next(), is(false)); ResultSetMetaData rsmd = rs.getMetaData(); assertThat(rsmd.getColumnCount(), is(equalTo(4)));