From f09e2b348d9193aa5c6cd48c823c5d27e24a84c5 Mon Sep 17 00:00:00 2001 From: "Stern, Ittay (is9613)" Date: Wed, 1 Jun 2022 16:42:46 +0300 Subject: [PATCH 1/3] Parameters in spring.data.cassandra.config file will override defaults This commit changes two things: 1. Any primitive on CassandraProperties are replaced with object values. This allows distinguishing between defaults values and no-values. Then CassandraAutoConfiguration.mapConfig() can use whenNonNull() predicate to ignore those. 2. CassandraProperties no longer populate default values on any property. With that, the defaults can be applied on top of the file spring.data.cassandra.config; i.e. the config file have higher precedence than the defaults, but lower that any spring.data.cassandra.* property. On the test side, the file `override-defaults.conf` is defined to override the values that were non-overridable with values which are different from the default. Closes gh-31025 --- .../cassandra/CassandraAutoConfiguration.java | 13 ++--- .../cassandra/CassandraProperties.java | 48 ++++++++++++------- .../CassandraAutoConfigurationTests.java | 40 ++++++++++++++++ .../cassandra/override-defaults.conf | 20 ++++++++ 4 files changed, 99 insertions(+), 22 deletions(-) create mode 100644 spring-boot-project/spring-boot-autoconfigure/src/test/resources/org/springframework/boot/autoconfigure/cassandra/override-defaults.conf diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraAutoConfiguration.java index 3b37e2e026a0..0ef07d6cb76b 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraAutoConfiguration.java @@ -126,7 +126,7 @@ private Config cassandraConfiguration(CassandraProperties properties) { private Config applyDefaultFallback(Config config) { ConfigFactory.invalidateCaches(); - return ConfigFactory.defaultOverrides().withFallback(config).withFallback(ConfigFactory.defaultReference()) + return ConfigFactory.defaultOverrides().withFallback(config).withFallback(mapConfig(CassandraProperties.defaults())).withFallback(ConfigFactory.defaultReference()) .resolve(); } @@ -153,10 +153,10 @@ private Config mapConfig(CassandraProperties properties) { mapPoolingOptions(properties, options); mapRequestOptions(properties, options); mapControlConnectionOptions(properties, options); - map.from(mapContactPoints(properties)) + map.from(mapContactPoints(properties)).whenNonNull() .to((contactPoints) -> options.add(DefaultDriverOption.CONTACT_POINTS, contactPoints)); - map.from(properties.getLocalDatacenter()).to( - (localDatacenter) -> options.add(DefaultDriverOption.LOAD_BALANCING_LOCAL_DATACENTER, localDatacenter)); + map.from(properties.getLocalDatacenter()).whenHasText() + .to((localDatacenter) -> options.add(DefaultDriverOption.LOAD_BALANCING_LOCAL_DATACENTER, localDatacenter)); return options.build(); } @@ -210,8 +210,9 @@ private void mapControlConnectionOptions(CassandraProperties properties, Cassand } private List mapContactPoints(CassandraProperties properties) { - return properties.getContactPoints().stream() - .map((candidate) -> formatContactPoint(candidate, properties.getPort())).collect(Collectors.toList()); + List contactPoints = properties.getContactPoints(); + return (contactPoints == null) ? null : contactPoints.stream() + .map((candidate) -> formatContactPoint(candidate, properties.getPort())).toList(); } private String formatContactPoint(String candidate, int port) { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraProperties.java index 17d99cf7696a..0f1ae8f5d1f5 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraProperties.java @@ -38,6 +38,18 @@ @ConfigurationProperties(prefix = "spring.data.cassandra") public class CassandraProperties { + static CassandraProperties defaults() { + var properties = new CassandraProperties(); + properties.setContactPoints(new ArrayList<>(Collections.singleton("127.0.0.1:9042"))); + properties.setPort(9042); + properties.setCompression(Compression.NONE); + properties.setSchemaAction("none"); + properties.setSsl(false); + + properties.getControlconnection().setTimeout(Duration.ofSeconds(5)); + + return properties; + } /** * Location of the configuration file to use. */ @@ -57,12 +69,12 @@ public class CassandraProperties { * Cluster node addresses in the form 'host:port', or a simple 'host' to use the * configured port. */ - private final List contactPoints = new ArrayList<>(Collections.singleton("127.0.0.1:9042")); + private List contactPoints; /** * Port to use if a contact point does not specify one. */ - private int port = 9042; + private Integer port; /** * Datacenter that is considered "local". Contact points should be from this @@ -83,17 +95,17 @@ public class CassandraProperties { /** * Compression supported by the Cassandra binary protocol. */ - private Compression compression = Compression.NONE; + private Compression compression; /** * Schema action to take at startup. */ - private String schemaAction = "none"; + private String schemaAction; /** * Enable SSL support. */ - private boolean ssl = false; + private Boolean ssl; /** * Connection configuration. @@ -143,7 +155,11 @@ public List getContactPoints() { return this.contactPoints; } - public int getPort() { + public void setContactPoints(List contactPoints) { + this.contactPoints = contactPoints; + } + + public Integer getPort() { return this.port; } @@ -183,7 +199,7 @@ public void setCompression(Compression compression) { this.compression = compression; } - public boolean isSsl() { + public Boolean isSsl() { return this.ssl; } @@ -266,7 +282,7 @@ public static class Request { /** * How many rows will be retrieved simultaneously in a single network round-trip. */ - private int pageSize; + private Integer pageSize; private final Throttler throttler = new Throttler(); @@ -294,7 +310,7 @@ public void setSerialConsistency(DefaultConsistencyLevel serialConsistency) { this.serialConsistency = serialConsistency; } - public int getPageSize() { + public Integer getPageSize() { return this.pageSize; } @@ -347,7 +363,7 @@ public static class Controlconnection { /** * Timeout to use for control queries. */ - private Duration timeout = Duration.ofSeconds(5); + private Duration timeout; public Duration getTimeout() { return this.timeout; @@ -370,17 +386,17 @@ public static class Throttler { * Maximum number of requests that can be enqueued when the throttling threshold * is exceeded. */ - private int maxQueueSize; + private Integer maxQueueSize; /** * Maximum number of requests that are allowed to execute in parallel. */ - private int maxConcurrentRequests; + private Integer maxConcurrentRequests; /** * Maximum allowed request rate. */ - private int maxRequestsPerSecond; + private Integer maxRequestsPerSecond; /** * How often the throttler attempts to dequeue requests. Set this high enough that @@ -397,7 +413,7 @@ public void setType(ThrottlerType type) { this.type = type; } - public int getMaxQueueSize() { + public Integer getMaxQueueSize() { return this.maxQueueSize; } @@ -405,7 +421,7 @@ public void setMaxQueueSize(int maxQueueSize) { this.maxQueueSize = maxQueueSize; } - public int getMaxConcurrentRequests() { + public Integer getMaxConcurrentRequests() { return this.maxConcurrentRequests; } @@ -413,7 +429,7 @@ public void setMaxConcurrentRequests(int maxConcurrentRequests) { this.maxConcurrentRequests = maxConcurrentRequests; } - public int getMaxRequestsPerSecond() { + public Integer getMaxRequestsPerSecond() { return this.maxRequestsPerSecond; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/cassandra/CassandraAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/cassandra/CassandraAutoConfigurationTests.java index ed05b182aa52..e3127b7c8d81 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/cassandra/CassandraAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/cassandra/CassandraAutoConfigurationTests.java @@ -17,6 +17,7 @@ package org.springframework.boot.autoconfigure.cassandra; import java.time.Duration; +import java.util.List; import com.datastax.oss.driver.api.core.CqlIdentifier; import com.datastax.oss.driver.api.core.CqlSession; @@ -28,6 +29,7 @@ import com.datastax.oss.driver.internal.core.session.throttling.ConcurrencyLimitingRequestThrottler; import com.datastax.oss.driver.internal.core.session.throttling.PassThroughRequestThrottler; import com.datastax.oss.driver.internal.core.session.throttling.RateLimitingRequestThrottler; +import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; import org.springframework.boot.autoconfigure.AutoConfigurations; @@ -244,6 +246,44 @@ void driverConfigLoaderWithConfigComplementSettings() { }); } + @Test // gh-31025 + void driverConfigLoaderWithConfigOverridesDefaults() { + String configLocation = "org/springframework/boot/autoconfigure/cassandra/override-defaults.conf"; + this.contextRunner.withPropertyValues("spring.data.cassandra.config=" + configLocation).run((context) -> { + assertThat(context).hasSingleBean(DriverConfigLoader.class); + + SoftAssertions softly = new SoftAssertions(); + + softly.assertThat(context.getBean(DriverConfigLoader.class).getInitialConfig().getDefaultProfile() + .getString(DefaultDriverOption.SESSION_NAME)).isEqualTo("advanced session"); + + softly.assertThat(context.getBean(DriverConfigLoader.class).getInitialConfig().getDefaultProfile() + .getDuration(DefaultDriverOption.REQUEST_TIMEOUT)).isEqualTo(Duration.ofSeconds(2)); // default + + softly.assertThat(context.getBean(DriverConfigLoader.class).getInitialConfig().getDefaultProfile() + .getStringList(DefaultDriverOption.CONTACT_POINTS)).isEqualTo(List.of("1.2.3.4:5678")); + softly.assertThat(context.getBean(DriverConfigLoader.class).getInitialConfig().getDefaultProfile() + .getBoolean(DefaultDriverOption.RESOLVE_CONTACT_POINTS)).isFalse(); + softly.assertThat(context.getBean(DriverConfigLoader.class).getInitialConfig().getDefaultProfile() + .getInt(DefaultDriverOption.REQUEST_PAGE_SIZE)).isEqualTo(11); + softly.assertThat(context.getBean(DriverConfigLoader.class).getInitialConfig().getDefaultProfile() + .getString(DefaultDriverOption.LOAD_BALANCING_LOCAL_DATACENTER)).isEqualTo("datacenter1"); + + softly.assertThat(context.getBean(DriverConfigLoader.class).getInitialConfig().getDefaultProfile() + .getInt(DefaultDriverOption.REQUEST_THROTTLER_MAX_CONCURRENT_REQUESTS)).isEqualTo(22); + softly.assertThat(context.getBean(DriverConfigLoader.class).getInitialConfig().getDefaultProfile() + .getInt(DefaultDriverOption.REQUEST_THROTTLER_MAX_REQUESTS_PER_SECOND)).isEqualTo(33); + softly.assertThat(context.getBean(DriverConfigLoader.class).getInitialConfig().getDefaultProfile() + .getInt(DefaultDriverOption.REQUEST_THROTTLER_MAX_QUEUE_SIZE)).isEqualTo(44); + softly.assertThat(context.getBean(DriverConfigLoader.class).getInitialConfig().getDefaultProfile() + .getDuration(DefaultDriverOption.CONTROL_CONNECTION_TIMEOUT)).isEqualTo(Duration.ofMillis(5555)); + softly.assertThat(context.getBean(DriverConfigLoader.class).getInitialConfig().getDefaultProfile() + .getString(DefaultDriverOption.PROTOCOL_COMPRESSION)).isEqualTo("SNAPPY"); + + softly.assertAll(); + }); + } + @Test void driverConfigLoaderWithConfigCreateProfiles() { String configLocation = "org/springframework/boot/autoconfigure/cassandra/profiles.conf"; diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/resources/org/springframework/boot/autoconfigure/cassandra/override-defaults.conf b/spring-boot-project/spring-boot-autoconfigure/src/test/resources/org/springframework/boot/autoconfigure/cassandra/override-defaults.conf new file mode 100644 index 000000000000..52115731ed3d --- /dev/null +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/resources/org/springframework/boot/autoconfigure/cassandra/override-defaults.conf @@ -0,0 +1,20 @@ +datastax-java-driver { + basic { + session-name = advanced session + load-balancing-policy { + local-datacenter = datacenter1 + } + request.page-size = 11 + contact-points = [ "1.2.3.4:5678" ] + } + advanced { + throttler { + max-concurrent-requests = 22 + max-requests-per-second = 33 + max-queue-size = 44 + } + control-connection.timeout = 5555 + protocol.compression = SNAPPY + resolve-contact-points = false + } +} From 62d58de736f5cb3623e147d60cc5b79805b6c3e9 Mon Sep 17 00:00:00 2001 From: "Stern, Ittay (is9613)" Date: Thu, 2 Jun 2022 12:49:05 +0300 Subject: [PATCH 2/3] Set default properties where values are not overridable port, SSL, schema-action are all not defined in the `spring.data.cassandra.config` file, so we can initialize those on the first configuration layer (default `CassandraProperties` field values). This simplifies the case of port configuration: 1. In case `spring.data.cassandra.contact-points` is configured without a port - the value of `spring.data.cassandra.port` (which defaults to 9042) is used. 2. Otherwise, in case `contact-points` on spring.data.cassandra.config file are defined -- then it must be configured with a port (no separate port property). 3. Otherwise, `contact-points` defaults to `127.0.0.1:9042` regardless of `spring.data.cassandra.port` configuration. --- .../cassandra/CassandraProperties.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraProperties.java index 0f1ae8f5d1f5..03c8afe2ec83 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraProperties.java @@ -40,16 +40,14 @@ public class CassandraProperties { static CassandraProperties defaults() { var properties = new CassandraProperties(); + properties.setContactPoints(new ArrayList<>(Collections.singleton("127.0.0.1:9042"))); - properties.setPort(9042); properties.setCompression(Compression.NONE); - properties.setSchemaAction("none"); - properties.setSsl(false); - properties.getControlconnection().setTimeout(Duration.ofSeconds(5)); return properties; } + /** * Location of the configuration file to use. */ @@ -74,7 +72,7 @@ static CassandraProperties defaults() { /** * Port to use if a contact point does not specify one. */ - private Integer port; + private int port = 9042; /** * Datacenter that is considered "local". Contact points should be from this @@ -100,12 +98,12 @@ static CassandraProperties defaults() { /** * Schema action to take at startup. */ - private String schemaAction; + private String schemaAction = "none"; /** * Enable SSL support. */ - private Boolean ssl; + private boolean ssl = false; /** * Connection configuration. @@ -159,7 +157,7 @@ public void setContactPoints(List contactPoints) { this.contactPoints = contactPoints; } - public Integer getPort() { + public int getPort() { return this.port; } @@ -199,7 +197,7 @@ public void setCompression(Compression compression) { this.compression = compression; } - public Boolean isSsl() { + public boolean isSsl() { return this.ssl; } From 590e3165f367435fd82c56547ed30fed11c390b8 Mon Sep 17 00:00:00 2001 From: "Stern, Ittay (is9613)" Date: Thu, 2 Jun 2022 12:49:22 +0300 Subject: [PATCH 3/3] Polish --- .../autoconfigure/cassandra/CassandraAutoConfiguration.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraAutoConfiguration.java index 0ef07d6cb76b..3f43c82e37a6 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cassandra/CassandraAutoConfiguration.java @@ -23,7 +23,6 @@ import java.util.List; import java.util.Map; import java.util.function.Supplier; -import java.util.stream.Collectors; import javax.net.ssl.SSLContext; @@ -126,7 +125,10 @@ private Config cassandraConfiguration(CassandraProperties properties) { private Config applyDefaultFallback(Config config) { ConfigFactory.invalidateCaches(); - return ConfigFactory.defaultOverrides().withFallback(config).withFallback(mapConfig(CassandraProperties.defaults())).withFallback(ConfigFactory.defaultReference()) + return ConfigFactory.defaultOverrides() + .withFallback(config) + .withFallback(mapConfig(CassandraProperties.defaults())) + .withFallback(ConfigFactory.defaultReference()) .resolve(); }