Skip to content

Commit 8d91e01

Browse files
committed
refactor: replace individual variables with ConnectionProperty
Replace individual variables in ConnectionOptions and ConnectionImpl with references to ConnectionProperties. This reduces the amount of code bloat, especially in ConnectionOptions, as all connection property parsing is now handled by the ConnectionState class in a generic way. This setup also reduces the amount of code that is needed to add a new connection property, as there is only one source of truth: the list of properties in the ConnectionProperties class. Following steps that will reduce the amount of code bloat further are: 1. Replace all the regular expressions for SET and SHOW statements with a simple token-based parser. 2. Cleaning up ConnectionOptions further by removing the duplicate list of ConnectionProperties there. These can be removed once the JDBC driver has been updated to use the new list of properties.
1 parent 0c61487 commit 8d91e01

15 files changed

+999
-919
lines changed

google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementValueConverters.java

Lines changed: 161 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616

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

19+
import static com.google.cloud.spanner.connection.ReadOnlyStalenessUtil.parseTimeUnit;
20+
import static com.google.cloud.spanner.connection.ReadOnlyStalenessUtil.toChronoUnit;
21+
22+
import com.google.api.gax.core.CredentialsProvider;
23+
import com.google.cloud.spanner.Dialect;
1924
import com.google.cloud.spanner.ErrorCode;
2025
import com.google.cloud.spanner.Options.RpcPriority;
2126
import com.google.cloud.spanner.SpannerException;
@@ -27,16 +32,17 @@
2732
import com.google.common.base.Function;
2833
import com.google.common.base.Preconditions;
2934
import com.google.common.base.Strings;
30-
import com.google.protobuf.Duration;
31-
import com.google.protobuf.util.Durations;
3235
import com.google.spanner.v1.DirectedReadOptions;
3336
import com.google.spanner.v1.RequestOptions.Priority;
37+
import java.lang.reflect.Constructor;
38+
import java.lang.reflect.InvocationTargetException;
39+
import java.time.Duration;
40+
import java.time.temporal.ChronoUnit;
3441
import java.util.Base64;
3542
import java.util.EnumSet;
3643
import java.util.HashMap;
3744
import java.util.Locale;
3845
import java.util.Map;
39-
import java.util.concurrent.TimeUnit;
4046
import java.util.regex.Matcher;
4147
import java.util.regex.Pattern;
4248

@@ -172,8 +178,11 @@ public Integer convert(String value) {
172178
}
173179
}
174180

175-
/** Converter from string to {@link Duration}. */
181+
/** Converter from string to protobuf {@link Duration}. */
176182
static class DurationConverter implements ClientSideStatementValueConverter<Duration> {
183+
static final DurationConverter INSTANCE =
184+
new DurationConverter("('(\\d{1,19})(s|ms|us|ns)'|(^\\d{1,19})|NULL)");
185+
177186
private final Pattern allowedValues;
178187

179188
public DurationConverter(String allowedValues) {
@@ -193,13 +202,16 @@ public Duration convert(String value) {
193202
Matcher matcher = allowedValues.matcher(value);
194203
if (matcher.find()) {
195204
if (matcher.group(0).equalsIgnoreCase("null")) {
196-
return Durations.fromNanos(0L);
205+
return Duration.ofMillis(0L);
197206
} else {
198-
Duration duration =
199-
ReadOnlyStalenessUtil.createDuration(
200-
Long.parseLong(matcher.group(1)),
201-
ReadOnlyStalenessUtil.parseTimeUnit(matcher.group(2)));
202-
if (duration.getSeconds() == 0L && duration.getNanos() == 0) {
207+
Duration duration;
208+
if (matcher.group(3) != null) {
209+
duration = Duration.ofMillis(Long.parseLong(matcher.group(3)));
210+
} else {
211+
ChronoUnit unit = toChronoUnit(parseTimeUnit(matcher.group(2)));
212+
duration = Duration.of(Long.parseLong(matcher.group(1)), unit);
213+
}
214+
if (duration.isZero()) {
203215
return null;
204216
}
205217
return duration;
@@ -231,18 +243,14 @@ public Duration convert(String value) {
231243
if (matcher.find()) {
232244
Duration duration;
233245
if (matcher.group(0).equalsIgnoreCase("default")) {
234-
return Durations.fromNanos(0L);
246+
return Duration.ofMillis(0L);
235247
} else if (matcher.group(2) == null) {
236-
duration =
237-
ReadOnlyStalenessUtil.createDuration(
238-
Long.parseLong(matcher.group(0)), TimeUnit.MILLISECONDS);
248+
duration = Duration.ofMillis(Long.parseLong(matcher.group(0)));
239249
} else {
240-
duration =
241-
ReadOnlyStalenessUtil.createDuration(
242-
Long.parseLong(matcher.group(1)),
243-
ReadOnlyStalenessUtil.parseTimeUnit(matcher.group(2)));
250+
ChronoUnit unit = toChronoUnit(parseTimeUnit(matcher.group(2)));
251+
duration = Duration.of(Long.parseLong(matcher.group(1)), unit);
244252
}
245-
if (duration.getSeconds() == 0L && duration.getNanos() == 0) {
253+
if (duration.isZero()) {
246254
return null;
247255
}
248256
return duration;
@@ -254,6 +262,10 @@ public Duration convert(String value) {
254262
/** Converter from string to possible values for read only staleness ({@link TimestampBound}). */
255263
static class ReadOnlyStalenessConverter
256264
implements ClientSideStatementValueConverter<TimestampBound> {
265+
static final ReadOnlyStalenessConverter INSTANCE =
266+
new ReadOnlyStalenessConverter(
267+
"'((STRONG)|(MIN_READ_TIMESTAMP)[\\t ]+((\\d{4})-(\\d{2})-(\\d{2})([Tt](\\d{2}):(\\d{2}):(\\d{2})(\\.\\d{1,9})?)([Zz]|([+-])(\\d{2}):(\\d{2})))|(READ_TIMESTAMP)[\\t ]+((\\d{4})-(\\d{2})-(\\d{2})([Tt](\\d{2}):(\\d{2}):(\\d{2})(\\.\\d{1,9})?)([Zz]|([+-])(\\d{2}):(\\d{2})))|(MAX_STALENESS)[\\t ]+((\\d{1,19})(s|ms|us|ns))|(EXACT_STALENESS)[\\t ]+((\\d{1,19})(s|ms|us|ns)))'");
268+
257269
private final Pattern allowedValues;
258270
private final CaseInsensitiveEnumMap<Mode> values = new CaseInsensitiveEnumMap<>(Mode.class);
259271

@@ -297,7 +309,7 @@ public TimestampBound convert(String value) {
297309
try {
298310
return TimestampBound.ofExactStaleness(
299311
Long.parseLong(matcher.group(groupIndex + 2)),
300-
ReadOnlyStalenessUtil.parseTimeUnit(matcher.group(groupIndex + 3)));
312+
parseTimeUnit(matcher.group(groupIndex + 3)));
301313
} catch (IllegalArgumentException e) {
302314
throw SpannerExceptionFactory.newSpannerException(
303315
ErrorCode.INVALID_ARGUMENT, e.getMessage());
@@ -306,7 +318,7 @@ public TimestampBound convert(String value) {
306318
try {
307319
return TimestampBound.ofMaxStaleness(
308320
Long.parseLong(matcher.group(groupIndex + 2)),
309-
ReadOnlyStalenessUtil.parseTimeUnit(matcher.group(groupIndex + 3)));
321+
parseTimeUnit(matcher.group(groupIndex + 3)));
310322
} catch (IllegalArgumentException e) {
311323
throw SpannerExceptionFactory.newSpannerException(
312324
ErrorCode.INVALID_ARGUMENT, e.getMessage());
@@ -539,7 +551,12 @@ public PgTransactionMode convert(String value) {
539551
}
540552
}
541553

542-
/** Converter for converting strings to {@link RpcPriority} values. */
554+
/**
555+
* Converter for converting strings to {@link Priority} values.
556+
*
557+
* @deprecated Use {@link RpcPriorityEnumConverter} instead.
558+
*/
559+
@Deprecated
543560
static class RpcPriorityConverter implements ClientSideStatementValueConverter<Priority> {
544561
private final CaseInsensitiveEnumMap<Priority> values =
545562
new CaseInsensitiveEnumMap<>(Priority.class);
@@ -569,12 +586,43 @@ public Priority convert(String value) {
569586
}
570587
}
571588

589+
/** Converter for converting strings to {@link RpcPriority} values. */
590+
static class RpcPriorityEnumConverter implements ClientSideStatementValueConverter<RpcPriority> {
591+
static final RpcPriorityEnumConverter INSTANCE = new RpcPriorityEnumConverter();
592+
593+
private final CaseInsensitiveEnumMap<RpcPriority> values =
594+
new CaseInsensitiveEnumMap<>(RpcPriority.class);
595+
596+
private RpcPriorityEnumConverter() {}
597+
598+
/** Constructor needed for reflection. */
599+
public RpcPriorityEnumConverter(String allowedValues) {}
600+
601+
@Override
602+
public Class<RpcPriority> getParameterClass() {
603+
return RpcPriority.class;
604+
}
605+
606+
@Override
607+
public RpcPriority convert(String value) {
608+
if ("null".equalsIgnoreCase(value)) {
609+
return RpcPriority.UNSPECIFIED;
610+
}
611+
return values.get(value);
612+
}
613+
}
614+
572615
/** Converter for converting strings to {@link SavepointSupport} values. */
573616
static class SavepointSupportConverter
574617
implements ClientSideStatementValueConverter<SavepointSupport> {
618+
static final SavepointSupportConverter INSTANCE = new SavepointSupportConverter();
619+
575620
private final CaseInsensitiveEnumMap<SavepointSupport> values =
576621
new CaseInsensitiveEnumMap<>(SavepointSupport.class);
577622

623+
private SavepointSupportConverter() {}
624+
625+
/** Constructor needed for reflection. */
578626
public SavepointSupportConverter(String allowedValues) {}
579627

580628
@Override
@@ -588,6 +636,30 @@ public SavepointSupport convert(String value) {
588636
}
589637
}
590638

639+
/** Converter for converting strings to {@link DdlInTransactionMode} values. */
640+
static class DdlInTransactionModeConverter
641+
implements ClientSideStatementValueConverter<DdlInTransactionMode> {
642+
static final DdlInTransactionModeConverter INSTANCE = new DdlInTransactionModeConverter();
643+
644+
private final CaseInsensitiveEnumMap<DdlInTransactionMode> values =
645+
new CaseInsensitiveEnumMap<>(DdlInTransactionMode.class);
646+
647+
private DdlInTransactionModeConverter() {}
648+
649+
/** Constructor needed for reflection. */
650+
public DdlInTransactionModeConverter(String allowedValues) {}
651+
652+
@Override
653+
public Class<DdlInTransactionMode> getParameterClass() {
654+
return DdlInTransactionMode.class;
655+
}
656+
657+
@Override
658+
public DdlInTransactionMode convert(String value) {
659+
return values.get(value);
660+
}
661+
}
662+
591663
static class ExplainCommandConverter implements ClientSideStatementValueConverter<String> {
592664
@Override
593665
public Class<String> getParameterClass() {
@@ -648,4 +720,71 @@ public String convert(String filePath) {
648720
return filePath;
649721
}
650722
}
723+
724+
static class CredentialsProviderConverter
725+
implements ClientSideStatementValueConverter<CredentialsProvider> {
726+
static final CredentialsProviderConverter INSTANCE = new CredentialsProviderConverter();
727+
728+
private CredentialsProviderConverter() {}
729+
730+
@Override
731+
public Class<CredentialsProvider> getParameterClass() {
732+
return CredentialsProvider.class;
733+
}
734+
735+
@Override
736+
public CredentialsProvider convert(String credentialsProviderName) {
737+
if (!Strings.isNullOrEmpty(credentialsProviderName)) {
738+
try {
739+
Class<? extends CredentialsProvider> clazz =
740+
(Class<? extends CredentialsProvider>) Class.forName(credentialsProviderName);
741+
Constructor<? extends CredentialsProvider> constructor = clazz.getDeclaredConstructor();
742+
return constructor.newInstance();
743+
} catch (ClassNotFoundException classNotFoundException) {
744+
throw SpannerExceptionFactory.newSpannerException(
745+
ErrorCode.INVALID_ARGUMENT,
746+
"Unknown or invalid CredentialsProvider class name: " + credentialsProviderName,
747+
classNotFoundException);
748+
} catch (NoSuchMethodException noSuchMethodException) {
749+
throw SpannerExceptionFactory.newSpannerException(
750+
ErrorCode.INVALID_ARGUMENT,
751+
"Credentials provider "
752+
+ credentialsProviderName
753+
+ " does not have a public no-arg constructor.",
754+
noSuchMethodException);
755+
} catch (InvocationTargetException
756+
| InstantiationException
757+
| IllegalAccessException exception) {
758+
throw SpannerExceptionFactory.newSpannerException(
759+
ErrorCode.INVALID_ARGUMENT,
760+
"Failed to create an instance of "
761+
+ credentialsProviderName
762+
+ ": "
763+
+ exception.getMessage(),
764+
exception);
765+
}
766+
}
767+
return null;
768+
}
769+
}
770+
771+
/** Converter for converting strings to {@link Dialect} values. */
772+
static class DialectConverter implements ClientSideStatementValueConverter<Dialect> {
773+
static final DialectConverter INSTANCE = new DialectConverter();
774+
775+
private final CaseInsensitiveEnumMap<Dialect> values =
776+
new CaseInsensitiveEnumMap<>(Dialect.class);
777+
778+
private DialectConverter() {}
779+
780+
@Override
781+
public Class<Dialect> getParameterClass() {
782+
return Dialect.class;
783+
}
784+
785+
@Override
786+
public Dialect convert(String value) {
787+
return values.get(value);
788+
}
789+
}
651790
}

0 commit comments

Comments
 (0)