Skip to content
Permalink
Browse files
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 dropwizard/dropwizard#3461
Fixes googleapis/google-cloud-java#6671
Fixes googleapis/java-spanner-jdbc#283

* fix: add credentials to prevent tests from trying to use env credentials
  • Loading branch information
olavloite committed Nov 30, 2020
1 parent 3f9f74a commit f6a8ba6baff53ededf890e3f22a8e49402c98775
@@ -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<ConnectionProperty> 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<ConnectionProperty> 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<String> 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;
@@ -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");
}
}
}

0 comments on commit f6a8ba6

Please sign in to comment.