Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ public static class Builder {
private Credentials credentials;
private List<StatementExecutionInterceptor> statementExecutionInterceptors =
Collections.emptyList();
private CredentialsService credentialsService = CredentialsService.INSTANCE;

private Builder() {}

Expand Down Expand Up @@ -317,6 +318,11 @@ Builder setCredentials(Credentials credentials) {
return this;
}

@VisibleForTesting
void setCredentialsService(CredentialsService credentialsService) {
this.credentialsService = credentialsService;
}

/** @return the {@link ConnectionOptions} */
public ConnectionOptions build() {
Preconditions.checkState(this.uri != null, "Connection URI is required");
Expand Down Expand Up @@ -370,10 +376,23 @@ private ConnectionOptions(Builder builder) {
+ matcher.group(Builder.HOST_GROUP);
this.instanceId = matcher.group(Builder.INSTANCE_GROUP);
this.databaseName = matcher.group(Builder.DATABASE_GROUP);
this.credentials =
builder.credentials == null
? getCredentialsService().createCredentials(this.credentialsUrl)
: builder.credentials;
Credentials credentials = null;
if (builder.credentials == null) {
try {
credentials = builder.credentialsService.createCredentials(this.credentialsUrl);
} catch (Exception e) {
// Ignore the error if the user did not set a credentials URL.
// The error is then caused by an invalid environment configuration
// and should not be thrown here.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a test that asserts the correct exception is thrown?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a couple of test cases to cover this.

if (this.credentialsUrl != null) {
throw e;
}
credentials = null;
}
} else {
credentials = builder.credentials;
}
this.credentials = credentials;
String numChannelsValue = parseNumChannels(builder.uri);
if (numChannelsValue != null) {
try {
Expand Down Expand Up @@ -401,11 +420,6 @@ private ConnectionOptions(Builder builder) {
Collections.unmodifiableList(builder.statementExecutionInterceptors);
}

@VisibleForTesting
CredentialsService getCredentialsService() {
return CredentialsService.INSTANCE;
}

@VisibleForTesting
static boolean parseUsePlainText(String uri) {
String value = parseUriProperty(uri, USE_PLAIN_TEXT_PROPERTY_NAME);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,10 @@ Spanner createSpanner(SpannerPoolKey key) {
builder
.setClientLibToken(MoreObjects.firstNonNull(key.userAgent, JdbcDriver.getClientLibToken()))
.setHost(key.host)
.setProjectId(key.projectId)
.setCredentials(key.credentials);
.setProjectId(key.projectId);
if (key.credentials != null) {
builder.setCredentials(key.credentials);
}
if (key.numChannels != null) {
builder.setNumChannels(key.numChannels);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,29 @@
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertThat;
import static org.mockito.Mockito.mock;

import com.google.auth.oauth2.GoogleCredentials;
import com.google.auth.oauth2.ServiceAccountCredentials;
import com.google.cloud.spanner.ErrorCode;
import com.google.cloud.spanner.SpannerExceptionFactory;
import com.google.cloud.spanner.SpannerOptions;
import java.util.Arrays;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.Mockito;

@RunWith(JUnit4.class)
public class ConnectionOptionsTest {
private static final String FILE_TEST_PATH =
ConnectionOptionsTest.class.getResource("test-key.json").getFile();
private static final String DEFAULT_HOST = "https://spanner.googleapis.com";

@Rule public ExpectedException expected = ExpectedException.none();

@Test
public void testBuildWithValidURIAndCredentialsFileURL() {
ConnectionOptions.Builder builder = ConnectionOptions.newBuilder();
Expand All @@ -53,6 +61,65 @@ public void testBuildWithValidURIAndCredentialsFileURL() {
assertThat(options.isReadOnly(), is(equalTo(ConnectionOptions.DEFAULT_READONLY)));
}

@Test
public void testBuildWithValidEnvCredentials() throws Exception {
// Simulate that this is the credentials that have been set for the environment.
GoogleCredentials envCredentials = new CredentialsService().createCredentials(FILE_TEST_PATH);

CredentialsService service = mock(CredentialsService.class);
// Call with null means that the JDBC driver should get the credentials from the environment.
Mockito.when(service.createCredentials((String) Mockito.isNull())).thenReturn(envCredentials);
ConnectionOptions.Builder builder = ConnectionOptions.newBuilder();
builder.setCredentialsService(service);
builder.setUri(
"cloudspanner:/projects/test-project-123/instances/test-instance-123/databases/test-database-123");
ConnectionOptions options = builder.build();
assertThat(
(GoogleCredentials) options.getCredentials(),
is(equalTo(new CredentialsService().createCredentials(FILE_TEST_PATH))));
}

@Test
public void testBuildWithInvalidEnvCredentials() throws Exception {
CredentialsService service = mock(CredentialsService.class);
// Call with null means that the JDBC driver should get the credentials from the environment.
Mockito.when(service.createCredentials((String) Mockito.isNull()))
.thenThrow(
SpannerExceptionFactory.newSpannerException(
ErrorCode.INVALID_ARGUMENT, "file not found"));
ConnectionOptions.Builder builder = ConnectionOptions.newBuilder();
builder.setCredentialsService(service);
builder.setUri(
"cloudspanner:/projects/test-project-123/instances/test-instance-123/databases/test-database-123");
ConnectionOptions options = builder.build();
assertThat((GoogleCredentials) options.getCredentials(), is(nullValue()));
}

@Test
public void testBuildWithInvalidBuilderCredentials() throws Exception {
ConnectionOptions.Builder builder = ConnectionOptions.newBuilder();
// Setting an invalid credentials URL should cause an exception when creating the
// ConnectionOptions.
builder.setCredentialsUrl("/path/to/non-existing-key.json");
builder.setUri(
"cloudspanner:/projects/test-project-123/instances/test-instance-123/databases/test-database-123");
expected.expect(
SpannerExceptionMatcher.matchCodeAndMessage(
ErrorCode.INVALID_ARGUMENT, "Invalid credentials path specified"));
builder.build();
}

@Test
public void testBuildWithInvalidConnectionURLCredentials() throws Exception {
ConnectionOptions.Builder builder = ConnectionOptions.newBuilder();
builder.setUri(
"cloudspanner:/projects/test-project-123/instances/test-instance-123/databases/test-database-123?credentials=/path/to/non-existing-key.json");
expected.expect(
SpannerExceptionMatcher.matchCodeAndMessage(
ErrorCode.INVALID_ARGUMENT, "Invalid credentials path specified"));
builder.build();
}

@Test
public void testBuildWithValidURIAndProperties() {
ConnectionOptions.Builder builder = ConnectionOptions.newBuilder();
Expand Down