Skip to content

Commit

Permalink
JCO-19 Support standard connection string parameter names
Browse files Browse the repository at this point in the history
Motivation
----------
Standardize connection string parameter names across Columnar SDKs

Modifications
-------------
Throw if connection string parameter has uppercase letter.

BuilderPropertySetter now applies a function that transforms
each path component (for example, from lower_snake_case
to lowerCamelCase).

Remove the `tls_verify` alias in favor of standardizing on
`security.disable_server_certificate_verification`.

Change-Id: Id66d5bcae951898915638cfab60aede120f72b69
Reviewed-on: https://review.couchbase.org/c/couchbase-jvm-clients/+/217176
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: David Nault <david.nault@couchbase.com>
  • Loading branch information
dnault committed Oct 2, 2024
1 parent fbc926d commit bb8a318
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.io.Closeable;
import java.time.Duration;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
Expand Down Expand Up @@ -121,11 +120,13 @@ public static Cluster newInstance(
throw new IllegalArgumentException("Invalid connection string; must start with secure scheme \"couchbases://\" (note the final 's') but got: " + redactUser(cs.original()));
}

checkParameterNamesAreLowercase(cs);

ClusterOptions builder = new ClusterOptions();
optionsCustomizer.accept(builder);

BuilderPropertySetter propertySetter = new BuilderPropertySetter("", Collections.emptyMap());
propertySetter.set(builder, translateTlsVerify(cs.params()));
BuilderPropertySetter propertySetter = new BuilderPropertySetter("", Collections.emptyMap(), Cluster::lowerSnakeCaseToLowerCamelCase);
propertySetter.set(builder, cs.params());

// do we really want to allow a system property to disable server certificate verification?
//propertySetter.set(builder, systemPropertyMap(SYSTEM_PROPERTY_PREFIX));
Expand Down Expand Up @@ -175,27 +176,38 @@ public static Cluster newInstance(
return new Cluster(cs, credential.toInternalAuthenticator(), env);
}

private static Map<String, String> translateTlsVerify(Map<String, String> connectionStringProperties) {
Map<String, String> properties = new LinkedHashMap<>(connectionStringProperties);
String tlsVerify = properties.remove("tls_verify");
if (tlsVerify == null) {
return properties;
}
private static void checkParameterNamesAreLowercase(ConnectionString cs) {
cs.params().keySet().stream()
.filter(Cluster::hasUppercase)
.findFirst()
.ifPresent(badName -> {
throw new IllegalArgumentException("Invalid connection string parameter '" + badName + "'. Please use lower_snake_case in connection string parameter names.");
});
}

private static boolean hasUppercase(String s) {
return s.codePoints().anyMatch(Character::isUpperCase);
}

String javaName = "security.verifyServerCertificate";
switch (tlsVerify) {
case "none":
properties.put(javaName, "false");
break;
case "peer":
properties.put(javaName, "true");
break;
default:
throw new IllegalArgumentException(
"Unexpected value for connection string parameter 'tls_verify'; expected 'none' or 'peer', but got: '" + tlsVerify + "'");
private static String lowerSnakeCaseToLowerCamelCase(String s) {
StringBuilder sb = new StringBuilder();
int[] codePoints = s.codePoints().toArray();

boolean prevWasUnderscore = false;
for (int i : codePoints) {
if (i == '_') {
prevWasUnderscore = true;
continue;
}

if (prevWasUnderscore) {
i = Character.toUpperCase(i);
}
sb.appendCodePoint(i);
prevWasUnderscore = false;
}

return properties;
return sb.toString();
}

private static CoreTransactionsConfig disableTransactionsCleanup() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
public class Sandbox {

public static void main(String[] args) throws Exception {
String connectionString = "couchbases://127.0.0.1?security.disableServerCertificateVerification=true";
String connectionString = "couchbases://127.0.0.1?security.disable_server_certificate_verification=true&srv=0";
String username = "Administrator";
String password = "password";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Collectors;

import static com.couchbase.client.core.util.CbCollections.mapCopyOf;
import static com.couchbase.client.core.util.CbCollections.mapOf;
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toList;

@SuppressWarnings("rawtypes")
@Stability.Internal
Expand All @@ -60,16 +60,22 @@ public class BuilderPropertySetter {
// Escape hatch in case some accessors don't follow the convention.
private final Map<String, String> irregularChildBuilderAccessors;

// Converts an input path component to match the Java method name,
// for translating case conventions.
private final Function<String, String> pathComponentTransformer;

public BuilderPropertySetter() {
this("Config", mapOf("ioEnvironment", "ioEnvironment"));
this("Config", mapOf("ioEnvironment", "ioEnvironment"), name -> name);
}

public BuilderPropertySetter(
String childBuilderAccessorSuffix,
Map<String, String> irregularChildBuilderAccessors
Map<String, String> irregularChildBuilderAccessors,
Function<String, String> pathComponentTransformer
) {
this.childBuilderAccessorSuffix = requireNonNull(childBuilderAccessorSuffix);
this.irregularChildBuilderAccessors = mapCopyOf(irregularChildBuilderAccessors);
this.pathComponentTransformer = requireNonNull(pathComponentTransformer);
}

/**
Expand All @@ -83,10 +89,11 @@ public void set(Object builder, Map<String, String> properties) {
* @throws InvalidPropertyException if the property could not be applied to the builder
*/
public void set(Object builder, String propertyName, String propertyValue) {


try {
final List<String> propertyComponents = Arrays.asList(propertyName.split("\\.", -1));
final List<String> propertyComponents = Arrays.stream(propertyName.split("\\.", -1))
.map(pathComponentTransformer)
.collect(toList());

final List<String> pathToBuilder = propertyComponents.subList(0, propertyComponents.size() - 1);
final String setterName = propertyComponents.get(propertyComponents.size() - 1);

Expand All @@ -111,7 +118,7 @@ public void set(Object builder, String propertyName, String propertyValue) {
final List<Method> candidates = Arrays.stream(builder.getClass().getMethods())
.filter(m -> m.getName().equals(setterName))
.filter(m -> m.getParameterCount() == 1)
.collect(Collectors.toList());
.collect(toList());

if (candidates.isEmpty()) {
throw InvalidArgumentException.fromMessage("No one-arg setter for property \"" + propertyName + "\" in " + builder.getClass());
Expand Down Expand Up @@ -256,7 +263,7 @@ private static <E extends Enum> E convertEnum(Class<E> enumClass, String value)
try {
return (E) Enum.valueOf(enumClass, value);
} catch (IllegalArgumentException e) {
List<String> enumValueNames = Arrays.stream(enumClass.getEnumConstants()).map(Enum::name).collect(Collectors.toList());
List<String> enumValueNames = Arrays.stream(enumClass.getEnumConstants()).map(Enum::name).collect(toList());
throw InvalidArgumentException.fromMessage("Expected one of " + enumValueNames + " but got \"" + value + "\"");
}
}
Expand Down

0 comments on commit bb8a318

Please sign in to comment.