diff --git a/spring-grpc-client-spring-boot-autoconfigure/src/main/java/org/springframework/boot/grpc/client/autoconfigure/GrpcClientProperties.java b/spring-grpc-client-spring-boot-autoconfigure/src/main/java/org/springframework/boot/grpc/client/autoconfigure/GrpcClientProperties.java index 84c9cb22..24796a13 100644 --- a/spring-grpc-client-spring-boot-autoconfigure/src/main/java/org/springframework/boot/grpc/client/autoconfigure/GrpcClientProperties.java +++ b/spring-grpc-client-spring-boot-autoconfigure/src/main/java/org/springframework/boot/grpc/client/autoconfigure/GrpcClientProperties.java @@ -20,11 +20,13 @@ import java.time.temporal.ChronoUnit; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.function.Consumer; import org.jspecify.annotations.Nullable; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.PropertyMapper; import org.springframework.boot.convert.DurationUnit; import org.springframework.context.EnvironmentAware; import org.springframework.core.env.Environment; @@ -52,6 +54,11 @@ public class GrpcClientProperties implements EnvironmentAware, VirtualTargets { */ private final Map channels = new HashMap<>(); + /** + * Default configuration that named channels can inherit from. + */ + private final Channel channel = new Channel(); + /** * The default channel configuration to use for new channels. */ @@ -73,6 +80,10 @@ public Map getChannels() { return this.channels; } + public Channel getChannel() { + return this.channel; + } + public ChannelConfig getDefaultChannel() { return this.defaultChannel; } @@ -101,11 +112,14 @@ public ChannelConfig getChannel(String name) { if ("default".equals(name)) { return this.defaultChannel; } - ChannelConfig channel = this.channels.get(name); - if (channel != null) { - return channel; + ChannelConfig namedChannel = this.channels.get(name); + if (namedChannel != null) { + if (namedChannel.isInheritDefaults()) { + return this.channel.getDefaults().mergeWith(namedChannel); + } + return namedChannel; } - channel = this.defaultChannel.copy(); + ChannelConfig newChannel = this.defaultChannel.copy(); String address = name; if (!name.contains(":/") && !name.startsWith("unix:")) { if (name.contains(":")) { @@ -118,8 +132,8 @@ public ChannelConfig getChannel(String name) { } } } - channel.setAddress(address); - return channel; + newChannel.setAddress(address); + return newChannel; } @Override @@ -140,7 +154,7 @@ public static class ChannelConfig { /** * The target address uri to connect to. */ - private String address = "static://localhost:9090"; + private @Nullable String address; /** * The default deadline for RPCs performed on this channel. @@ -150,12 +164,12 @@ public static class ChannelConfig { /** * The load balancing policy the channel should use. */ - private String defaultLoadBalancingPolicy = "round_robin"; + private @Nullable String defaultLoadBalancingPolicy; /** * Whether keep alive is enabled on the channel. */ - private boolean enableKeepAlive; + private @Nullable Boolean enableKeepAlive; private final Health health = new Health(); @@ -163,7 +177,7 @@ public static class ChannelConfig { * The duration without ongoing RPCs before going to idle mode. */ @DurationUnit(ChronoUnit.SECONDS) - private Duration idleTimeout = Duration.ofSeconds(20); + private @Nullable Duration idleTimeout; /** * The delay before sending a keepAlive. Note that shorter intervals increase the @@ -171,42 +185,42 @@ public static class ChannelConfig { * 'permitKeepAliveTime' on the server. */ @DurationUnit(ChronoUnit.SECONDS) - private Duration keepAliveTime = Duration.ofMinutes(5); + private @Nullable Duration keepAliveTime; /** * The default timeout for a keepAlives ping request. */ @DurationUnit(ChronoUnit.SECONDS) - private Duration keepAliveTimeout = Duration.ofSeconds(20); + private @Nullable Duration keepAliveTimeout; /** * Whether a keepAlive will be performed when there are no outstanding RPC on a * connection. */ - private boolean keepAliveWithoutCalls; + private @Nullable Boolean keepAliveWithoutCalls; /** * Maximum message size allowed to be received by the channel (default 4MiB). Set * to '-1' to use the highest possible limit (not recommended). */ - private DataSize maxInboundMessageSize = DataSize.ofBytes(4194304); + private @Nullable DataSize maxInboundMessageSize; /** * Maximum metadata size allowed to be received by the channel (default 8KiB). Set * to '-1' to use the highest possible limit (not recommended). */ - private DataSize maxInboundMetadataSize = DataSize.ofBytes(8192); + private @Nullable DataSize maxInboundMetadataSize; /** * The negotiation type for the channel. */ - private NegotiationType negotiationType = NegotiationType.PLAINTEXT; + private @Nullable NegotiationType negotiationType; /** * Flag to say that strict SSL checks are not enabled (so the remote certificate * could be anonymous). */ - private boolean secure = true; + private @Nullable Boolean secure; /** * Map representation of the service config to use for the channel. @@ -220,8 +234,13 @@ public static class ChannelConfig { */ private @Nullable String userAgent; + /** + * Whether to inherit settings from the channel defaults configuration. + */ + private boolean inheritDefaults; + public String getAddress() { - return this.address; + return Objects.requireNonNullElse(this.address, "static://localhost:9090"); } public void setAddress(final String address) { @@ -237,7 +256,7 @@ public void setDefaultDeadline(@Nullable Duration defaultDeadline) { } public String getDefaultLoadBalancingPolicy() { - return this.defaultLoadBalancingPolicy; + return Objects.requireNonNullElse(this.defaultLoadBalancingPolicy, "round_robin"); } public void setDefaultLoadBalancingPolicy(final String defaultLoadBalancingPolicy) { @@ -245,7 +264,7 @@ public void setDefaultLoadBalancingPolicy(final String defaultLoadBalancingPolic } public boolean isEnableKeepAlive() { - return this.enableKeepAlive; + return Objects.requireNonNullElse(this.enableKeepAlive, false); } public void setEnableKeepAlive(boolean enableKeepAlive) { @@ -257,7 +276,7 @@ public Health getHealth() { } public Duration getIdleTimeout() { - return this.idleTimeout; + return Objects.requireNonNullElse(this.idleTimeout, Duration.ofSeconds(20)); } public void setIdleTimeout(Duration idleTimeout) { @@ -265,7 +284,7 @@ public void setIdleTimeout(Duration idleTimeout) { } public Duration getKeepAliveTime() { - return this.keepAliveTime; + return Objects.requireNonNullElse(this.keepAliveTime, Duration.ofMinutes(5)); } public void setKeepAliveTime(Duration keepAliveTime) { @@ -273,7 +292,7 @@ public void setKeepAliveTime(Duration keepAliveTime) { } public Duration getKeepAliveTimeout() { - return this.keepAliveTimeout; + return Objects.requireNonNullElse(this.keepAliveTimeout, Duration.ofSeconds(20)); } public void setKeepAliveTimeout(Duration keepAliveTimeout) { @@ -281,7 +300,7 @@ public void setKeepAliveTimeout(Duration keepAliveTimeout) { } public boolean isKeepAliveWithoutCalls() { - return this.keepAliveWithoutCalls; + return Objects.requireNonNullElse(this.keepAliveWithoutCalls, false); } public void setKeepAliveWithoutCalls(boolean keepAliveWithoutCalls) { @@ -289,7 +308,7 @@ public void setKeepAliveWithoutCalls(boolean keepAliveWithoutCalls) { } public DataSize getMaxInboundMessageSize() { - return this.maxInboundMessageSize; + return Objects.requireNonNullElse(this.maxInboundMessageSize, DataSize.ofBytes(4194304)); } public void setMaxInboundMessageSize(final DataSize maxInboundMessageSize) { @@ -298,7 +317,7 @@ public void setMaxInboundMessageSize(final DataSize maxInboundMessageSize) { } public DataSize getMaxInboundMetadataSize() { - return this.maxInboundMetadataSize; + return Objects.requireNonNullElse(this.maxInboundMetadataSize, DataSize.ofBytes(8192)); } public void setMaxInboundMetadataSize(DataSize maxInboundMetadataSize) { @@ -319,7 +338,7 @@ else if (maxSize != null && maxSize.toBytes() == -1) { } public NegotiationType getNegotiationType() { - return this.negotiationType; + return Objects.requireNonNullElse(this.negotiationType, NegotiationType.PLAINTEXT); } public void setNegotiationType(NegotiationType negotiationType) { @@ -327,7 +346,7 @@ public void setNegotiationType(NegotiationType negotiationType) { } public boolean isSecure() { - return this.secure; + return Objects.requireNonNullElse(this.secure, true); } public void setSecure(boolean secure) { @@ -350,6 +369,14 @@ public void setUserAgent(@Nullable String userAgent) { this.userAgent = userAgent; } + public boolean isInheritDefaults() { + return this.inheritDefaults; + } + + public void setInheritDefaults(boolean inheritDefaults) { + this.inheritDefaults = inheritDefaults; + } + /** * Provide a copy of the channel instance. * @return a copy of the channel instance. @@ -368,6 +395,7 @@ ChannelConfig copy() { copy.maxInboundMetadataSize = this.maxInboundMetadataSize; copy.userAgent = this.userAgent; copy.defaultDeadline = this.defaultDeadline; + copy.inheritDefaults = this.inheritDefaults; copy.health.copyValuesFrom(this.getHealth()); copy.secure = this.secure; copy.ssl.copyValuesFrom(this.getSsl()); @@ -375,6 +403,30 @@ ChannelConfig copy() { return copy; } + ChannelConfig mergeWith(ChannelConfig other) { + ChannelConfig merged = this.copy(); + PropertyMapper map = PropertyMapper.get(); + map.from(other.address).to(merged::setAddress); + map.from(other.defaultDeadline).to(merged::setDefaultDeadline); + map.from(other.defaultLoadBalancingPolicy).to(merged::setDefaultLoadBalancingPolicy); + map.from(other.enableKeepAlive).to(merged::setEnableKeepAlive); + map.from(other.idleTimeout).to(merged::setIdleTimeout); + map.from(other.keepAliveTime).to(merged::setKeepAliveTime); + map.from(other.keepAliveTimeout).to(merged::setKeepAliveTimeout); + map.from(other.keepAliveWithoutCalls).to(merged::setKeepAliveWithoutCalls); + map.from(other.maxInboundMessageSize).to(merged::setMaxInboundMessageSize); + map.from(other.maxInboundMetadataSize).to(merged::setMaxInboundMetadataSize); + map.from(other.negotiationType).to(merged::setNegotiationType); + map.from(other.secure).to(merged::setSecure); + map.from(other.userAgent).to(merged::setUserAgent); + merged.health.mergeWith(other.health); + merged.ssl.mergeWith(other.ssl); + if (!other.serviceConfig.isEmpty()) { + merged.serviceConfig.putAll(other.serviceConfig); + } + return merged; + } + /** * Extracts the service configuration from the client properties, respecting the * yaml lists (e.g. `retryPolicy`). @@ -390,7 +442,7 @@ public static class Health { /** * Whether to enable client-side health check for the channel. */ - private boolean enabled; + private @Nullable Boolean enabled; /** * Name of the service to check health on. @@ -398,7 +450,7 @@ public static class Health { private @Nullable String serviceName; public boolean isEnabled() { - return this.enabled; + return Objects.requireNonNullElse(this.enabled, false); } public void setEnabled(boolean enabled) { @@ -422,6 +474,12 @@ void copyValuesFrom(Health other) { this.serviceName = other.serviceName; } + void mergeWith(Health other) { + PropertyMapper map = PropertyMapper.get(); + map.from(other.enabled).to(this::setEnabled); + map.from(other.serviceName).to(this::setServiceName); + } + } public static class Ssl { @@ -466,6 +524,25 @@ void copyValuesFrom(Ssl other) { this.bundle = other.bundle; } + void mergeWith(Ssl other) { + PropertyMapper map = PropertyMapper.get(); + map.from(other.enabled).to(this::setEnabled); + map.from(other.bundle).to(this::setBundle); + } + + } + + } + + /** + * Container for channel defaults configuration. + */ + public static class Channel { + + private final ChannelConfig defaults = new ChannelConfig(); + + public ChannelConfig getDefaults() { + return this.defaults; } } diff --git a/spring-grpc-client-spring-boot-autoconfigure/src/test/java/org/springframework/boot/grpc/client/autoconfigure/GrpcClientPropertiesTests.java b/spring-grpc-client-spring-boot-autoconfigure/src/test/java/org/springframework/boot/grpc/client/autoconfigure/GrpcClientPropertiesTests.java index cc2890d0..6c0997a5 100644 --- a/spring-grpc-client-spring-boot-autoconfigure/src/test/java/org/springframework/boot/grpc/client/autoconfigure/GrpcClientPropertiesTests.java +++ b/spring-grpc-client-spring-boot-autoconfigure/src/test/java/org/springframework/boot/grpc/client/autoconfigure/GrpcClientPropertiesTests.java @@ -209,14 +209,14 @@ void withDefaultNameReturnsDefaultChannel() { } @Test - void withKnownNameReturnsKnownChannel() { + void withKnownNameReturnsMergedChannel() { Map map = new HashMap<>(); // we have to at least bind one property or bind() fails map.put("spring.grpc.client.channels.c1.enable-keep-alive", "false"); GrpcClientProperties properties = bindProperties(map); var channel = properties.getChannel("c1"); - assertThat(properties).extracting("channels", InstanceOfAssertFactories.MAP) - .containsExactly(entry("c1", channel)); + assertThat(properties).extracting("channels", InstanceOfAssertFactories.MAP).containsKey("c1"); + assertThat(channel.isEnableKeepAlive()).isFalse(); } @Test @@ -310,4 +310,167 @@ void copyFromDefaultChannel() { } + @Nested + class ChannelInheritanceAPI { + + @Test + void namedChannelInheritsFromChannelDefaultsWhenOptedIn() { + Map map = new HashMap<>(); + map.put("spring.grpc.client.channel.defaults.max-inbound-message-size", "5MB"); + map.put("spring.grpc.client.channel.defaults.max-inbound-metadata-size", "1MB"); + map.put("spring.grpc.client.channel.defaults.enable-keep-alive", "true"); + map.put("spring.grpc.client.channels.service-a.address", "static://service-a:9090"); + map.put("spring.grpc.client.channels.service-a.inherit-defaults", "true"); + GrpcClientProperties properties = bindProperties(map); + + var channel = properties.getChannel("service-a"); + + assertThat(channel.getMaxInboundMessageSize()).isEqualTo(DataSize.ofMegabytes(5)); + assertThat(channel.getMaxInboundMetadataSize()).isEqualTo(DataSize.ofMegabytes(1)); + assertThat(channel.isEnableKeepAlive()).isTrue(); + assertThat(channel.getAddress()).isEqualTo("static://service-a:9090"); + } + + @Test + void namedChannelOverridesChannelDefaultsSettings() { + Map map = new HashMap<>(); + map.put("spring.grpc.client.channel.defaults.max-inbound-message-size", "5MB"); + map.put("spring.grpc.client.channel.defaults.enable-keep-alive", "true"); + map.put("spring.grpc.client.channel.defaults.idle-timeout", "30s"); + map.put("spring.grpc.client.channels.service-a.address", "static://service-a:9090"); + map.put("spring.grpc.client.channels.service-a.inherit-defaults", "true"); + map.put("spring.grpc.client.channels.service-a.max-inbound-message-size", "10MB"); + map.put("spring.grpc.client.channels.service-a.enable-keep-alive", "false"); + GrpcClientProperties properties = bindProperties(map); + + var channel = properties.getChannel("service-a"); + + assertThat(channel.getMaxInboundMessageSize()).isEqualTo(DataSize.ofMegabytes(10)); + assertThat(channel.isEnableKeepAlive()).isFalse(); + assertThat(channel.getIdleTimeout()).isEqualTo(Duration.ofSeconds(30)); + } + + @Test + void namedChannelMergesNestedHealthConfig() { + Map map = new HashMap<>(); + map.put("spring.grpc.client.channel.defaults.health.enabled", "true"); + map.put("spring.grpc.client.channel.defaults.health.service-name", "default-service"); + map.put("spring.grpc.client.channels.service-a.address", "static://service-a:9090"); + map.put("spring.grpc.client.channels.service-a.inherit-defaults", "true"); + map.put("spring.grpc.client.channels.service-a.health.service-name", "service-a-health"); + GrpcClientProperties properties = bindProperties(map); + + var channel = properties.getChannel("service-a"); + + assertThat(channel.getHealth().isEnabled()).isTrue(); + assertThat(channel.getHealth().getServiceName()).isEqualTo("service-a-health"); + } + + @Test + void namedChannelMergesNestedSslConfig() { + Map map = new HashMap<>(); + map.put("spring.grpc.client.channel.defaults.ssl.enabled", "true"); + map.put("spring.grpc.client.channel.defaults.ssl.bundle", "default-bundle"); + map.put("spring.grpc.client.channels.service-a.address", "static://service-a:9090"); + map.put("spring.grpc.client.channels.service-a.inherit-defaults", "true"); + map.put("spring.grpc.client.channels.service-a.ssl.bundle", "service-a-bundle"); + GrpcClientProperties properties = bindProperties(map); + + var channel = properties.getChannel("service-a"); + + assertThat(channel.getSsl().isEnabled()).isTrue(); + assertThat(channel.getSsl().getBundle()).isEqualTo("service-a-bundle"); + } + + @Test + void namedChannelMergesServiceConfig() { + Map map = new HashMap<>(); + map.put("spring.grpc.client.channel.defaults.service-config.default-key", "default-value"); + map.put("spring.grpc.client.channels.service-a.address", "static://service-a:9090"); + map.put("spring.grpc.client.channels.service-a.inherit-defaults", "true"); + map.put("spring.grpc.client.channels.service-a.service-config.custom-key", "custom-value"); + GrpcClientProperties properties = bindProperties(map); + + var channel = properties.getChannel("service-a"); + + assertThat(channel.getServiceConfig()).containsKey("default-key"); + assertThat(channel.getServiceConfig()).containsKey("custom-key"); + } + + @Test + void multipleNamedChannelsAllInheritFromDefaults() { + Map map = new HashMap<>(); + map.put("spring.grpc.client.channel.defaults.max-inbound-message-size", "5MB"); + map.put("spring.grpc.client.channel.defaults.enable-keep-alive", "true"); + map.put("spring.grpc.client.channels.service-a.address", "static://service-a:9090"); + map.put("spring.grpc.client.channels.service-a.inherit-defaults", "true"); + map.put("spring.grpc.client.channels.service-b.address", "static://service-b:9090"); + map.put("spring.grpc.client.channels.service-b.inherit-defaults", "true"); + GrpcClientProperties properties = bindProperties(map); + + var channelA = properties.getChannel("service-a"); + var channelB = properties.getChannel("service-b"); + + assertThat(channelA.getMaxInboundMessageSize()).isEqualTo(DataSize.ofMegabytes(5)); + assertThat(channelA.isEnableKeepAlive()).isTrue(); + assertThat(channelA.getAddress()).isEqualTo("static://service-a:9090"); + + assertThat(channelB.getMaxInboundMessageSize()).isEqualTo(DataSize.ofMegabytes(5)); + assertThat(channelB.isEnableKeepAlive()).isTrue(); + assertThat(channelB.getAddress()).isEqualTo("static://service-b:9090"); + } + + @Test + void namedChannelDoesNotInheritByDefault() { + Map map = new HashMap<>(); + map.put("spring.grpc.client.channel.defaults.max-inbound-message-size", "5MB"); + map.put("spring.grpc.client.channel.defaults.enable-keep-alive", "true"); + map.put("spring.grpc.client.channels.service-a.address", "static://service-a:9090"); + GrpcClientProperties properties = bindProperties(map); + + var channel = properties.getChannel("service-a"); + + // Should NOT inherit from defaults by default - uses property defaults + // instead + assertThat(channel.getMaxInboundMessageSize()).isEqualTo(DataSize.ofBytes(4194304)); + assertThat(channel.isEnableKeepAlive()).isFalse(); + assertThat(channel.getAddress()).isEqualTo("static://service-a:9090"); + } + + @Test + void defaultChannelDoesNotInheritFromChannelDefaults() { + Map map = new HashMap<>(); + map.put("spring.grpc.client.channel.defaults.max-inbound-message-size", "5MB"); + map.put("spring.grpc.client.channel.defaults.enable-keep-alive", "true"); + map.put("spring.grpc.client.default-channel.address", "static://default-server:9090"); + GrpcClientProperties properties = bindProperties(map); + + var defaultChannel = properties.getDefaultChannel(); + + // default-channel should NOT inherit from channel.defaults + assertThat(defaultChannel.getMaxInboundMessageSize()).isEqualTo(DataSize.ofBytes(4194304)); + assertThat(defaultChannel.isEnableKeepAlive()).isFalse(); + assertThat(defaultChannel.getAddress()).isEqualTo("static://default-server:9090"); + } + + @Test + void channelDefaultsAreIndependentFromDefaultChannel() { + Map map = new HashMap<>(); + map.put("spring.grpc.client.channel.defaults.max-inbound-message-size", "5MB"); + map.put("spring.grpc.client.default-channel.max-inbound-message-size", "10MB"); + map.put("spring.grpc.client.channels.service-a.address", "static://service-a:9090"); + map.put("spring.grpc.client.channels.service-a.inherit-defaults", "true"); + GrpcClientProperties properties = bindProperties(map); + + var defaultChannel = properties.getDefaultChannel(); + var namedChannel = properties.getChannel("service-a"); + + // default-channel has its own setting + assertThat(defaultChannel.getMaxInboundMessageSize()).isEqualTo(DataSize.ofMegabytes(10)); + // named channel inherits from channel.defaults, not default-channel + assertThat(namedChannel.getMaxInboundMessageSize()).isEqualTo(DataSize.ofMegabytes(5)); + } + + } + }