Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow lenient mode for connection properties #671

Merged
merged 2 commits into from Nov 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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
Expand Down Expand Up @@ -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:";
Expand All @@ -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 =
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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));
Expand All @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down
Expand Up @@ -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");
}
}
}