Skip to content

Commit 5957008

Browse files
olavloiteolavloite
authored
fix: allow dots and colons in project id (#36)
* fix: allow dots and colons in project id The project ID part of a connection URL should allow the use of dots and colons. Fixes #33 * fix: escaping dot in square brackets is not needed
1 parent 769fba6 commit 5957008

File tree

3 files changed

+124
-133
lines changed

3 files changed

+124
-133
lines changed

‎src/main/java/com/google/cloud/spanner/jdbc/ConnectionOptions.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ private Builder() {}
231231

232232
/** Spanner {@link ConnectionOptions} URI format. */
233233
public static final String SPANNER_URI_FORMAT =
234-
"(?:cloudspanner:)(?<HOSTGROUP>//[\\w.-]+(?:\\.[\\w\\.-]+)*[\\w\\-\\._~:/?#\\[\\]@!\\$&'\\(\\)\\*\\+,;=.]+)?/projects/(?<PROJECTGROUP>(([a-z]|[-]|[0-9])+|(DEFAULT_PROJECT_ID)))(/instances/(?<INSTANCEGROUP>([a-z]|[-]|[0-9])+)(/databases/(?<DATABASEGROUP>([a-z]|[-]|[_]|[0-9])+))?)?(?:[?|;].*)?";
234+
"(?:cloudspanner:)(?<HOSTGROUP>//[\\w.-]+(?:\\.[\\w\\.-]+)*[\\w\\-\\._~:/?#\\[\\]@!\\$&'\\(\\)\\*\\+,;=.]+)?/projects/(?<PROJECTGROUP>(([a-z]|[-.:]|[0-9])+|(DEFAULT_PROJECT_ID)))(/instances/(?<INSTANCEGROUP>([a-z]|[-]|[0-9])+)(/databases/(?<DATABASEGROUP>([a-z]|[-]|[_]|[0-9])+))?)?(?:[?|;].*)?";
235235

236236
private static final String SPANNER_URI_REGEX = "(?is)^" + SPANNER_URI_FORMAT + "$";
237237
private static final Pattern SPANNER_URI_PATTERN = Pattern.compile(SPANNER_URI_REGEX);

‎src/test/java/com/google/cloud/spanner/jdbc/ConnectionOptionsTest.java‎

Lines changed: 118 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,9 @@
1616

1717
package com.google.cloud.spanner.jdbc;
1818

19-
import static org.hamcrest.CoreMatchers.equalTo;
20-
import static org.hamcrest.CoreMatchers.is;
21-
import static org.hamcrest.CoreMatchers.nullValue;
22-
import static org.junit.Assert.assertThat;
19+
import static com.google.common.truth.Truth.assertThat;
20+
import static org.junit.Assert.fail;
2321

24-
import com.google.auth.oauth2.GoogleCredentials;
2522
import com.google.auth.oauth2.ServiceAccountCredentials;
2623
import com.google.cloud.spanner.SpannerOptions;
2724
import java.util.Arrays;
@@ -35,22 +32,38 @@ public class ConnectionOptionsTest {
3532
ConnectionOptionsTest.class.getResource("test-key.json").getFile();
3633
private static final String DEFAULT_HOST = "https://spanner.googleapis.com";
3734

35+
@Test
36+
public void testBuildWithURIWithDots() {
37+
ConnectionOptions.Builder builder = ConnectionOptions.newBuilder();
38+
builder.setUri(
39+
"cloudspanner:/projects/some-company.com:test-project-123/instances/test-instance-123/databases/test-database-123");
40+
builder.setCredentialsUrl(FILE_TEST_PATH);
41+
ConnectionOptions options = builder.build();
42+
assertThat(options.getHost()).isEqualTo(DEFAULT_HOST);
43+
assertThat(options.getProjectId()).isEqualTo("some-company.com:test-project-123");
44+
assertThat(options.getInstanceId()).isEqualTo("test-instance-123");
45+
assertThat(options.getDatabaseName()).isEqualTo("test-database-123");
46+
assertThat(options.getCredentials())
47+
.isEqualTo(new CredentialsService().createCredentials(FILE_TEST_PATH));
48+
assertThat(options.isAutocommit()).isEqualTo(ConnectionOptions.DEFAULT_AUTOCOMMIT);
49+
assertThat(options.isReadOnly()).isEqualTo(ConnectionOptions.DEFAULT_READONLY);
50+
}
51+
3852
@Test
3953
public void testBuildWithValidURIAndCredentialsFileURL() {
4054
ConnectionOptions.Builder builder = ConnectionOptions.newBuilder();
4155
builder.setUri(
4256
"cloudspanner:/projects/test-project-123/instances/test-instance-123/databases/test-database-123");
4357
builder.setCredentialsUrl(FILE_TEST_PATH);
4458
ConnectionOptions options = builder.build();
45-
assertThat(options.getHost(), is(equalTo(DEFAULT_HOST)));
46-
assertThat(options.getProjectId(), is(equalTo("test-project-123")));
47-
assertThat(options.getInstanceId(), is(equalTo("test-instance-123")));
48-
assertThat(options.getDatabaseName(), is(equalTo("test-database-123")));
49-
assertThat(
50-
(GoogleCredentials) options.getCredentials(),
51-
is(equalTo(new CredentialsService().createCredentials(FILE_TEST_PATH))));
52-
assertThat(options.isAutocommit(), is(equalTo(ConnectionOptions.DEFAULT_AUTOCOMMIT)));
53-
assertThat(options.isReadOnly(), is(equalTo(ConnectionOptions.DEFAULT_READONLY)));
59+
assertThat(options.getHost()).isEqualTo(DEFAULT_HOST);
60+
assertThat(options.getProjectId()).isEqualTo("test-project-123");
61+
assertThat(options.getInstanceId()).isEqualTo("test-instance-123");
62+
assertThat(options.getDatabaseName()).isEqualTo("test-database-123");
63+
assertThat(options.getCredentials())
64+
.isEqualTo(new CredentialsService().createCredentials(FILE_TEST_PATH));
65+
assertThat(options.isAutocommit()).isEqualTo(ConnectionOptions.DEFAULT_AUTOCOMMIT);
66+
assertThat(options.isReadOnly()).isEqualTo(ConnectionOptions.DEFAULT_READONLY);
5467
}
5568

5669
@Test
@@ -60,15 +73,14 @@ public void testBuildWithValidURIAndProperties() {
6073
"cloudspanner:/projects/test-project-123/instances/test-instance-123/databases/test-database-123?autocommit=false;readonly=true");
6174
builder.setCredentialsUrl(FILE_TEST_PATH);
6275
ConnectionOptions options = builder.build();
63-
assertThat(options.getHost(), is(equalTo(DEFAULT_HOST)));
64-
assertThat(options.getProjectId(), is(equalTo("test-project-123")));
65-
assertThat(options.getInstanceId(), is(equalTo("test-instance-123")));
66-
assertThat(options.getDatabaseName(), is(equalTo("test-database-123")));
67-
assertThat(
68-
(GoogleCredentials) options.getCredentials(),
69-
is(equalTo(new CredentialsService().createCredentials(FILE_TEST_PATH))));
70-
assertThat(options.isAutocommit(), is(equalTo(false)));
71-
assertThat(options.isReadOnly(), is(equalTo(true)));
76+
assertThat(options.getHost()).isEqualTo(DEFAULT_HOST);
77+
assertThat(options.getProjectId()).isEqualTo("test-project-123");
78+
assertThat(options.getInstanceId()).isEqualTo("test-instance-123");
79+
assertThat(options.getDatabaseName()).isEqualTo("test-database-123");
80+
assertThat(options.getCredentials())
81+
.isEqualTo(new CredentialsService().createCredentials(FILE_TEST_PATH));
82+
assertThat(options.isAutocommit()).isEqualTo(false);
83+
assertThat(options.isReadOnly()).isEqualTo(true);
7284
}
7385

7486
@Test
@@ -78,15 +90,14 @@ public void testBuildWithHostAndValidURI() {
7890
"cloudspanner://test-spanner.googleapis.com/projects/test-project-123/instances/test-instance-123/databases/test-database-123");
7991
builder.setCredentialsUrl(FILE_TEST_PATH);
8092
ConnectionOptions options = builder.build();
81-
assertThat(options.getHost(), is(equalTo("https://test-spanner.googleapis.com")));
82-
assertThat(options.getProjectId(), is(equalTo("test-project-123")));
83-
assertThat(options.getInstanceId(), is(equalTo("test-instance-123")));
84-
assertThat(options.getDatabaseName(), is(equalTo("test-database-123")));
85-
assertThat(
86-
(GoogleCredentials) options.getCredentials(),
87-
is(equalTo(new CredentialsService().createCredentials(FILE_TEST_PATH))));
88-
assertThat(options.isAutocommit(), is(equalTo(ConnectionOptions.DEFAULT_AUTOCOMMIT)));
89-
assertThat(options.isReadOnly(), is(equalTo(ConnectionOptions.DEFAULT_READONLY)));
93+
assertThat(options.getHost()).isEqualTo("https://test-spanner.googleapis.com");
94+
assertThat(options.getProjectId()).isEqualTo("test-project-123");
95+
assertThat(options.getInstanceId()).isEqualTo("test-instance-123");
96+
assertThat(options.getDatabaseName()).isEqualTo("test-database-123");
97+
assertThat(options.getCredentials())
98+
.isEqualTo(new CredentialsService().createCredentials(FILE_TEST_PATH));
99+
assertThat(options.isAutocommit()).isEqualTo(ConnectionOptions.DEFAULT_AUTOCOMMIT);
100+
assertThat(options.isReadOnly()).isEqualTo(ConnectionOptions.DEFAULT_READONLY);
90101
}
91102

92103
@Test
@@ -96,15 +107,14 @@ public void testBuildWithLocalhostPortAndValidURI() {
96107
"cloudspanner://localhost:8443/projects/test-project-123/instances/test-instance-123/databases/test-database-123");
97108
builder.setCredentialsUrl(FILE_TEST_PATH);
98109
ConnectionOptions options = builder.build();
99-
assertThat(options.getHost(), is(equalTo("https://localhost:8443")));
100-
assertThat(options.getProjectId(), is(equalTo("test-project-123")));
101-
assertThat(options.getInstanceId(), is(equalTo("test-instance-123")));
102-
assertThat(options.getDatabaseName(), is(equalTo("test-database-123")));
103-
assertThat(
104-
(GoogleCredentials) options.getCredentials(),
105-
is(equalTo(new CredentialsService().createCredentials(FILE_TEST_PATH))));
106-
assertThat(options.isAutocommit(), is(equalTo(ConnectionOptions.DEFAULT_AUTOCOMMIT)));
107-
assertThat(options.isReadOnly(), is(equalTo(ConnectionOptions.DEFAULT_READONLY)));
110+
assertThat(options.getHost()).isEqualTo("https://localhost:8443");
111+
assertThat(options.getProjectId()).isEqualTo("test-project-123");
112+
assertThat(options.getInstanceId()).isEqualTo("test-instance-123");
113+
assertThat(options.getDatabaseName()).isEqualTo("test-database-123");
114+
assertThat(options.getCredentials())
115+
.isEqualTo(new CredentialsService().createCredentials(FILE_TEST_PATH));
116+
assertThat(options.isAutocommit()).isEqualTo(ConnectionOptions.DEFAULT_AUTOCOMMIT);
117+
assertThat(options.isReadOnly()).isEqualTo(ConnectionOptions.DEFAULT_READONLY);
108118
}
109119

110120
@Test
@@ -114,21 +124,20 @@ public void testBuildWithDefaultProjectPlaceholder() {
114124
"cloudspanner:/projects/default_project_id/instances/test-instance-123/databases/test-database-123");
115125
builder.setCredentialsUrl(FILE_TEST_PATH);
116126
ConnectionOptions options = builder.build();
117-
assertThat(options.getHost(), is(equalTo(DEFAULT_HOST)));
127+
assertThat(options.getHost()).isEqualTo(DEFAULT_HOST);
118128
String projectId = SpannerOptions.getDefaultProjectId();
119129
if (projectId == null) {
120130
projectId =
121131
((ServiceAccountCredentials) new CredentialsService().createCredentials(FILE_TEST_PATH))
122132
.getProjectId();
123133
}
124-
assertThat(options.getProjectId(), is(equalTo(projectId)));
125-
assertThat(options.getInstanceId(), is(equalTo("test-instance-123")));
126-
assertThat(options.getDatabaseName(), is(equalTo("test-database-123")));
127-
assertThat(
128-
(GoogleCredentials) options.getCredentials(),
129-
is(equalTo(new CredentialsService().createCredentials(FILE_TEST_PATH))));
130-
assertThat(options.isAutocommit(), is(equalTo(ConnectionOptions.DEFAULT_AUTOCOMMIT)));
131-
assertThat(options.isReadOnly(), is(equalTo(ConnectionOptions.DEFAULT_READONLY)));
134+
assertThat(options.getProjectId()).isEqualTo(projectId);
135+
assertThat(options.getInstanceId()).isEqualTo("test-instance-123");
136+
assertThat(options.getDatabaseName()).isEqualTo("test-database-123");
137+
assertThat(options.getCredentials())
138+
.isEqualTo(new CredentialsService().createCredentials(FILE_TEST_PATH));
139+
assertThat(options.isAutocommit()).isEqualTo(ConnectionOptions.DEFAULT_AUTOCOMMIT);
140+
assertThat(options.isReadOnly()).isEqualTo(ConnectionOptions.DEFAULT_READONLY);
132141
}
133142

134143
@Test
@@ -203,129 +212,112 @@ public void testBuilderSetUri() {
203212
}
204213

205214
private void setInvalidUri(ConnectionOptions.Builder builder, String uri) {
206-
boolean invalid = false;
207215
try {
208216
builder.setUri(uri);
217+
fail(uri + " should be considered an invalid uri");
209218
} catch (IllegalArgumentException e) {
210-
invalid = true;
211219
}
212-
assertThat(uri + " should be considered an invalid uri", invalid, is(true));
213220
}
214221

215222
private void setInvalidProperty(
216223
ConnectionOptions.Builder builder, String uri, String expectedInvalidProperties) {
217-
boolean invalid = false;
218224
try {
219225
builder.setUri(uri);
226+
fail("missing expected exception");
220227
} catch (IllegalArgumentException e) {
221-
invalid = e.getMessage().contains(expectedInvalidProperties);
228+
assertThat(e.getMessage()).contains(expectedInvalidProperties);
222229
}
223-
assertThat(uri + " should contain invalid properties", invalid, is(true));
224230
}
225231

226232
@Test
227233
public void testParseUriProperty() {
228234
final String baseUri =
229235
"cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database";
230236

231-
assertThat(ConnectionOptions.parseUriProperty(baseUri, "autocommit"), is(nullValue()));
232-
assertThat(
233-
ConnectionOptions.parseUriProperty(baseUri + "?autocommit=true", "autocommit"),
234-
is(equalTo("true")));
237+
assertThat(ConnectionOptions.parseUriProperty(baseUri, "autocommit")).isNull();
238+
assertThat(ConnectionOptions.parseUriProperty(baseUri + "?autocommit=true", "autocommit"))
239+
.isEqualTo("true");
240+
assertThat(ConnectionOptions.parseUriProperty(baseUri + "?autocommit=false", "autocommit"))
241+
.isEqualTo("false");
242+
assertThat(ConnectionOptions.parseUriProperty(baseUri + "?autocommit=true;", "autocommit"))
243+
.isEqualTo("true");
244+
assertThat(ConnectionOptions.parseUriProperty(baseUri + "?autocommit=false;", "autocommit"))
245+
.isEqualTo("false");
235246
assertThat(
236-
ConnectionOptions.parseUriProperty(baseUri + "?autocommit=false", "autocommit"),
237-
is(equalTo("false")));
247+
ConnectionOptions.parseUriProperty(
248+
baseUri + "?autocommit=true;readOnly=false", "autocommit"))
249+
.isEqualTo("true");
238250
assertThat(
239-
ConnectionOptions.parseUriProperty(baseUri + "?autocommit=true;", "autocommit"),
240-
is(equalTo("true")));
251+
ConnectionOptions.parseUriProperty(
252+
baseUri + "?autocommit=false;readOnly=false", "autocommit"))
253+
.isEqualTo("false");
241254
assertThat(
242-
ConnectionOptions.parseUriProperty(baseUri + "?autocommit=false;", "autocommit"),
243-
is(equalTo("false")));
255+
ConnectionOptions.parseUriProperty(
256+
baseUri + "?readOnly=false;autocommit=true", "autocommit"))
257+
.isEqualTo("true");
244258
assertThat(
245-
ConnectionOptions.parseUriProperty(
246-
baseUri + "?autocommit=true;readOnly=false", "autocommit"),
247-
is(equalTo("true")));
259+
ConnectionOptions.parseUriProperty(
260+
baseUri + "?readOnly=false;autocommit=false", "autocommit"))
261+
.isEqualTo("false");
248262
assertThat(
249-
ConnectionOptions.parseUriProperty(
250-
baseUri + "?autocommit=false;readOnly=false", "autocommit"),
251-
is(equalTo("false")));
263+
ConnectionOptions.parseUriProperty(
264+
baseUri + "?readOnly=false;autocommit=true;foo=bar", "autocommit"))
265+
.isEqualTo("true");
252266
assertThat(
253-
ConnectionOptions.parseUriProperty(
254-
baseUri + "?readOnly=false;autocommit=true", "autocommit"),
255-
is(equalTo("true")));
256-
assertThat(
257-
ConnectionOptions.parseUriProperty(
258-
baseUri + "?readOnly=false;autocommit=false", "autocommit"),
259-
is(equalTo("false")));
260-
assertThat(
261-
ConnectionOptions.parseUriProperty(
262-
baseUri + "?readOnly=false;autocommit=true;foo=bar", "autocommit"),
263-
is(equalTo("true")));
264-
assertThat(
265-
ConnectionOptions.parseUriProperty(
266-
baseUri + "?readOnly=false;autocommit=false;foo=bar", "autocommit"),
267-
is(equalTo("false")));
267+
ConnectionOptions.parseUriProperty(
268+
baseUri + "?readOnly=false;autocommit=false;foo=bar", "autocommit"))
269+
.isEqualTo("false");
268270

269271
// case insensitive
270-
assertThat(
271-
ConnectionOptions.parseUriProperty(baseUri + "?AutoCommit=true", "autocommit"),
272-
is(equalTo("true")));
273-
assertThat(
274-
ConnectionOptions.parseUriProperty(baseUri + "?AutoCommit=false", "autocommit"),
275-
is(equalTo("false")));
272+
assertThat(ConnectionOptions.parseUriProperty(baseUri + "?AutoCommit=true", "autocommit"))
273+
.isEqualTo("true");
274+
assertThat(ConnectionOptions.parseUriProperty(baseUri + "?AutoCommit=false", "autocommit"))
275+
.isEqualTo("false");
276276

277277
// ; instead of ? before the properties is ok
278-
assertThat(
279-
ConnectionOptions.parseUriProperty(baseUri + ";autocommit=true", "autocommit"),
280-
is(equalTo("true")));
278+
assertThat(ConnectionOptions.parseUriProperty(baseUri + ";autocommit=true", "autocommit"))
279+
.isEqualTo("true");
281280

282281
// forgot the ? or ; before the properties
283-
assertThat(
284-
ConnectionOptions.parseUriProperty(baseUri + "autocommit=true", "autocommit"),
285-
is(nullValue()));
282+
assertThat(ConnectionOptions.parseUriProperty(baseUri + "autocommit=true", "autocommit"))
283+
.isNull();
286284
// substring is not ok
287-
assertThat(
288-
ConnectionOptions.parseUriProperty(baseUri + "?isautocommit=true", "autocommit"),
289-
is(nullValue()));
285+
assertThat(ConnectionOptions.parseUriProperty(baseUri + "?isautocommit=true", "autocommit"))
286+
.isNull();
290287
}
291288

292289
@Test
293290
public void testParseProperties() {
294291
final String baseUri =
295292
"cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database";
296-
assertThat(
297-
ConnectionOptions.parseProperties(baseUri + "?autocommit=true"),
298-
is(equalTo(Arrays.asList("autocommit"))));
299-
assertThat(
300-
ConnectionOptions.parseProperties(baseUri + "?autocommit=true;readonly=false"),
301-
is(equalTo(Arrays.asList("autocommit", "readonly"))));
302-
assertThat(
303-
ConnectionOptions.parseProperties(baseUri + "?autocommit=true;READONLY=false"),
304-
is(equalTo(Arrays.asList("autocommit", "READONLY"))));
305-
assertThat(
306-
ConnectionOptions.parseProperties(baseUri + ";autocommit=true;readonly=false"),
307-
is(equalTo(Arrays.asList("autocommit", "readonly"))));
308-
assertThat(
309-
ConnectionOptions.parseProperties(baseUri + ";autocommit=true;readonly=false;"),
310-
is(equalTo(Arrays.asList("autocommit", "readonly"))));
293+
assertThat(ConnectionOptions.parseProperties(baseUri + "?autocommit=true"))
294+
.isEqualTo(Arrays.asList("autocommit"));
295+
assertThat(ConnectionOptions.parseProperties(baseUri + "?autocommit=true;readonly=false"))
296+
.isEqualTo(Arrays.asList("autocommit", "readonly"));
297+
assertThat(ConnectionOptions.parseProperties(baseUri + "?autocommit=true;READONLY=false"))
298+
.isEqualTo(Arrays.asList("autocommit", "READONLY"));
299+
assertThat(ConnectionOptions.parseProperties(baseUri + ";autocommit=true;readonly=false"))
300+
.isEqualTo(Arrays.asList("autocommit", "readonly"));
301+
assertThat(ConnectionOptions.parseProperties(baseUri + ";autocommit=true;readonly=false;"))
302+
.isEqualTo(Arrays.asList("autocommit", "readonly"));
311303
}
312304

313305
@Test
314306
public void testParsePropertiesSpecifiedMultipleTimes() {
315307
final String baseUri =
316308
"cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database";
317309
assertThat(
318-
ConnectionOptions.parseUriProperty(
319-
baseUri + "?autocommit=true;autocommit=false", "autocommit"),
320-
is(equalTo("true")));
310+
ConnectionOptions.parseUriProperty(
311+
baseUri + "?autocommit=true;autocommit=false", "autocommit"))
312+
.isEqualTo("true");
321313
assertThat(
322-
ConnectionOptions.parseUriProperty(
323-
baseUri + "?autocommit=false;autocommit=true", "autocommit"),
324-
is(equalTo("false")));
314+
ConnectionOptions.parseUriProperty(
315+
baseUri + "?autocommit=false;autocommit=true", "autocommit"))
316+
.isEqualTo("false");
325317
assertThat(
326-
ConnectionOptions.parseUriProperty(
327-
baseUri + ";autocommit=false;readonly=false;autocommit=true", "autocommit"),
328-
is(equalTo("false")));
318+
ConnectionOptions.parseUriProperty(
319+
baseUri + ";autocommit=false;readonly=false;autocommit=true", "autocommit"))
320+
.isEqualTo("false");
329321
ConnectionOptions.newBuilder()
330322
.setUri(
331323
"cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database"

0 commit comments

Comments
 (0)