From f6a8ba6baff53ededf890e3f22a8e49402c98775 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Tue, 1 Dec 2020 00:06:20 +0100 Subject: [PATCH] feat: allow lenient mode for connection properties (#671) * feat: allow lenient mode for connection properties Some applications automatically add additional properties to connection strings that are unknown to the Spanner Connection API (and thereby also the Spanner JDBC driver). This causes the connection attempt to fail. This change allows a user to specify 'lenient' mode where unknown properties only generate a warning instead of an error. Fixes https://github.com/dropwizard/dropwizard/issues/3461 Fixes https://github.com/googleapis/google-cloud-java/issues/6671 Fixes https://github.com/googleapis/java-spanner-jdbc/issues/283 * fix: add credentials to prevent tests from trying to use env credentials --- .../spanner/connection/ConnectionOptions.java | 42 ++++++++++++++++--- .../connection/ConnectionOptionsTest.java | 35 ++++++++++++++++ 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java index a8b4b6bdcf..a668342f1b 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java @@ -42,6 +42,7 @@ import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.annotation.Nullable; /** * Internal connection API for Google Cloud Spanner. This class may introduce breaking changes @@ -152,6 +153,7 @@ public String[] getValidValues() { private static final String DEFAULT_NUM_CHANNELS = null; private static final String DEFAULT_USER_AGENT = null; private static final String DEFAULT_OPTIMIZER_VERSION = ""; + private static final boolean DEFAULT_LENIENT = false; private static final String PLAIN_TEXT_PROTOCOL = "http:"; private static final String HOST_PROTOCOL = "https:"; @@ -176,6 +178,8 @@ public String[] getValidValues() { private static final String USER_AGENT_PROPERTY_NAME = "userAgent"; /** Query optimizer version to use for a connection. */ private static final String OPTIMIZER_VERSION_PROPERTY_NAME = "optimizerVersion"; + /** Name of the 'lenientMode' connection property. */ + public static final String LENIENT_PROPERTY_NAME = "lenient"; /** All valid connection properties. */ public static final Set VALID_PROPERTIES = @@ -212,7 +216,11 @@ public String[] getValidValues() { "The custom user-agent property name to use when communicating with Cloud Spanner. This property is intended for internal library usage, and should not be set by applications."), ConnectionProperty.createStringProperty( OPTIMIZER_VERSION_PROPERTY_NAME, - "Sets the default query optimizer version to use for this connection.")))); + "Sets the default query optimizer version to use for this connection."), + ConnectionProperty.createBooleanProperty( + LENIENT_PROPERTY_NAME, + "Silently ignore unknown properties in the connection string/properties (true/false)", + DEFAULT_LENIENT)))); private static final Set INTERNAL_PROPERTIES = Collections.unmodifiableSet( @@ -416,6 +424,7 @@ public static Builder newBuilder() { } private final String uri; + private final String warnings; private final String credentialsUrl; private final String oauthToken; private final Credentials fixedCredentials; @@ -441,7 +450,7 @@ private ConnectionOptions(Builder builder) { Matcher matcher = Builder.SPANNER_URI_PATTERN.matcher(builder.uri); Preconditions.checkArgument( matcher.find(), String.format("Invalid connection URI specified: %s", builder.uri)); - checkValidProperties(builder.uri); + this.warnings = checkValidProperties(builder.uri); this.uri = builder.uri; this.sessionPoolOptions = builder.sessionPoolOptions; @@ -574,6 +583,12 @@ static String parseOptimizerVersion(String uri) { return value != null ? value : DEFAULT_OPTIMIZER_VERSION; } + @VisibleForTesting + static boolean parseLenient(String uri) { + String value = parseUriProperty(uri, LENIENT_PROPERTY_NAME); + return value != null ? Boolean.valueOf(value) : DEFAULT_LENIENT; + } + @VisibleForTesting static String parseUriProperty(String uri, String property) { Pattern pattern = Pattern.compile(String.format("(?is)(?:;|\\?)%s=(.*?)(?:;|$)", property)); @@ -586,9 +601,10 @@ static String parseUriProperty(String uri, String property) { /** Check that only valid properties have been specified. */ @VisibleForTesting - static void checkValidProperties(String uri) { + static String checkValidProperties(String uri) { String invalidProperties = ""; List properties = parseProperties(uri); + boolean lenient = parseLenient(uri); for (String property : properties) { if (!INTERNAL_VALID_PROPERTIES.contains(ConnectionProperty.createEmptyProperty(property))) { if (invalidProperties.length() > 0) { @@ -597,9 +613,17 @@ static void checkValidProperties(String uri) { invalidProperties = invalidProperties + property; } } - Preconditions.checkArgument( - invalidProperties.isEmpty(), - "Invalid properties found in connection URI: " + invalidProperties.toString()); + if (lenient) { + return String.format( + "Invalid properties found in connection URI: %s", invalidProperties.toString()); + } else { + Preconditions.checkArgument( + invalidProperties.isEmpty(), + String.format( + "Invalid properties found in connection URI. Add lenient=true to the connection string to ignore unknown properties. Invalid properties: %s", + invalidProperties.toString())); + return null; + } } @VisibleForTesting @@ -706,6 +730,12 @@ public boolean isRetryAbortsInternally() { return retryAbortsInternally; } + /** Any warnings that were generated while creating the {@link ConnectionOptions} instance. */ + @Nullable + public String getWarnings() { + return warnings; + } + /** Use http instead of https. Only valid for (local) test servers. */ boolean isUsePlainText() { return usePlainText; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java index cb84a635ae..b2f4ea086c 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java @@ -386,4 +386,39 @@ public void testSetOAuthTokenAndCredentials() { assertThat(e.getMessage()).contains("Cannot specify both credentials and an OAuth token"); } } + + @Test + public void testLenient() { + ConnectionOptions options = + ConnectionOptions.newBuilder() + .setUri( + "cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database?lenient=true;foo=bar") + .setCredentialsUrl(FILE_TEST_PATH) + .build(); + assertThat(options.getWarnings()).isNotNull(); + assertThat(options.getWarnings()).contains("foo"); + assertThat(options.getWarnings()).doesNotContain("lenient"); + + options = + ConnectionOptions.newBuilder() + .setUri( + "cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database?bar=foo;lenient=true") + .setCredentialsUrl(FILE_TEST_PATH) + .build(); + assertThat(options.getWarnings()).isNotNull(); + assertThat(options.getWarnings()).contains("bar"); + assertThat(options.getWarnings()).doesNotContain("lenient"); + + try { + options = + ConnectionOptions.newBuilder() + .setUri( + "cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database?bar=foo") + .setCredentialsUrl(FILE_TEST_PATH) + .build(); + fail("missing expected exception"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage()).contains("bar"); + } + } }