Use PropertyMapper for merging and remove DEFAULT_* constants

Signed-off-by: Oleksandr Shevchenko <oleksandr.shevchenko@datarobot.com>

Author: o-shevchenko

spring-grpc-client-spring-boot-autoconfigure/src/main/java/org/springframework/boot/grpc/client/autoconfigure/GrpcClientProperties.java

@@ -21,12 +21,11 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Optional;
 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;
@@ -37,8 +36,6 @@
 import org.springframework.grpc.client.VirtualTargets;
 import org.springframework.util.unit.DataSize;
 
-import io.grpc.ManagedChannel;
-
 /**
  * Configuration properties for the gRPC client side.
  *
@@ -55,7 +52,8 @@ public class GrpcClientProperties implements EnvironmentAware, VirtualTargets {
 	private final Map<String, ChannelConfig> channels = new HashMap<>();
 
 	/**
-	 * Container for channel-level configuration that can be shared across named channels.
+	 * Container for channel-level default configuration that named channels can inherit
+	 * from.
 	 */
 	private final Channel channel = new Channel();
 
@@ -72,6 +70,7 @@ public class GrpcClientProperties implements EnvironmentAware, VirtualTargets {
 	private Environment environment;
 
 	GrpcClientProperties() {
+		this.defaultChannel.setAddress("static://localhost:9090");
 		this.environment = new StandardEnvironment();
 	}
 
@@ -104,8 +103,8 @@ public void setEnvironment(Environment environment) {
 	 * Gets the configured channel with the given name. If no channel is configured for
 	 * the specified name then one is created using the default channel as a template.
 	 * Named channels inherit settings from the {@code channel.defaults} configuration
-	 * (not from the default-channel) unless {@code inherit-defaults} is set to false on
-	 * the channel.
+	 * (not from the default-channel) if {@code inherit-defaults} is set to true on the
+	 * channel.
 	 * @param name the name of the channel
 	 * @return the configured channel if found (optionally merged with defaults), or a
 	 * newly created channel using the default channel as a template
@@ -149,32 +148,10 @@ public String getTarget(String authority) {
 	}
 
 	/**
-	 * Represents the configuration for a {@link ManagedChannel gRPC channel}.
+	 * Represents the configuration for a gRPC channel.
 	 */
 	public static class ChannelConfig {
 
-		private static final String DEFAULT_ADDRESS = "static://localhost:9090";
-
-		private static final String DEFAULT_LOAD_BALANCING_POLICY = "round_robin";
-
-		private static final boolean DEFAULT_ENABLE_KEEP_ALIVE = false;
-
-		private static final Duration DEFAULT_IDLE_TIMEOUT = Duration.ofSeconds(20);
-
-		private static final Duration DEFAULT_KEEP_ALIVE_TIME = Duration.ofMinutes(5);
-
-		private static final Duration DEFAULT_KEEP_ALIVE_TIMEOUT = Duration.ofSeconds(20);
-
-		private static final boolean DEFAULT_KEEP_ALIVE_WITHOUT_CALLS = false;
-
-		private static final DataSize DEFAULT_MAX_INBOUND_MESSAGE_SIZE = DataSize.ofBytes(4194304);
-
-		private static final DataSize DEFAULT_MAX_INBOUND_METADATA_SIZE = DataSize.ofBytes(8192);
-
-		private static final NegotiationType DEFAULT_NEGOTIATION_TYPE = NegotiationType.PLAINTEXT;
-
-		private static final boolean DEFAULT_SECURE = true;
-
 		/**
 		 * The target address uri to connect to.
 		 */
@@ -261,10 +238,10 @@ public static class ChannelConfig {
 		/**
 		 * Whether to inherit settings from the channel defaults configuration.
 		 */
-		private @Nullable Boolean inheritDefaults;
+		private boolean inheritDefaults;
 
 		public String getAddress() {
-			return Objects.requireNonNullElse(this.address, DEFAULT_ADDRESS);
+			return Objects.requireNonNullElse(this.address, "static://localhost:9090");
 		}
 
 		public void setAddress(final String address) {
@@ -280,15 +257,15 @@ public void setDefaultDeadline(@Nullable Duration defaultDeadline) {
 		}
 
 		public String getDefaultLoadBalancingPolicy() {
-			return Objects.requireNonNullElse(this.defaultLoadBalancingPolicy, DEFAULT_LOAD_BALANCING_POLICY);
+			return Objects.requireNonNullElse(this.defaultLoadBalancingPolicy, "round_robin");
 		}
 
 		public void setDefaultLoadBalancingPolicy(final String defaultLoadBalancingPolicy) {
 			this.defaultLoadBalancingPolicy = defaultLoadBalancingPolicy;
 		}
 
 		public boolean isEnableKeepAlive() {
-			return Objects.requireNonNullElse(this.enableKeepAlive, DEFAULT_ENABLE_KEEP_ALIVE);
+			return Objects.requireNonNullElse(this.enableKeepAlive, false);
 		}
 
 		public void setEnableKeepAlive(boolean enableKeepAlive) {
@@ -300,39 +277,39 @@ public Health getHealth() {
 		}
 
 		public Duration getIdleTimeout() {
-			return Objects.requireNonNullElse(this.idleTimeout, DEFAULT_IDLE_TIMEOUT);
+			return Objects.requireNonNullElse(this.idleTimeout, Duration.ofSeconds(20));
 		}
 
 		public void setIdleTimeout(Duration idleTimeout) {
 			this.idleTimeout = idleTimeout;
 		}
 
 		public Duration getKeepAliveTime() {
-			return Objects.requireNonNullElse(this.keepAliveTime, DEFAULT_KEEP_ALIVE_TIME);
+			return Objects.requireNonNullElse(this.keepAliveTime, Duration.ofMinutes(5));
 		}
 
 		public void setKeepAliveTime(Duration keepAliveTime) {
 			this.keepAliveTime = keepAliveTime;
 		}
 
 		public Duration getKeepAliveTimeout() {
-			return Objects.requireNonNullElse(this.keepAliveTimeout, DEFAULT_KEEP_ALIVE_TIMEOUT);
+			return Objects.requireNonNullElse(this.keepAliveTimeout, Duration.ofSeconds(20));
 		}
 
 		public void setKeepAliveTimeout(Duration keepAliveTimeout) {
 			this.keepAliveTimeout = keepAliveTimeout;
 		}
 
 		public boolean isKeepAliveWithoutCalls() {
-			return Objects.requireNonNullElse(this.keepAliveWithoutCalls, DEFAULT_KEEP_ALIVE_WITHOUT_CALLS);
+			return Objects.requireNonNullElse(this.keepAliveWithoutCalls, false);
 		}
 
 		public void setKeepAliveWithoutCalls(boolean keepAliveWithoutCalls) {
 			this.keepAliveWithoutCalls = keepAliveWithoutCalls;
 		}
 
 		public DataSize getMaxInboundMessageSize() {
-			return Objects.requireNonNullElse(this.maxInboundMessageSize, DEFAULT_MAX_INBOUND_MESSAGE_SIZE);
+			return Objects.requireNonNullElse(this.maxInboundMessageSize, DataSize.ofBytes(4194304));
 		}
 
 		public void setMaxInboundMessageSize(final DataSize maxInboundMessageSize) {
@@ -341,7 +318,7 @@ public void setMaxInboundMessageSize(final DataSize maxInboundMessageSize) {
 		}
 
 		public DataSize getMaxInboundMetadataSize() {
-			return Objects.requireNonNullElse(this.maxInboundMetadataSize, DEFAULT_MAX_INBOUND_METADATA_SIZE);
+			return Objects.requireNonNullElse(this.maxInboundMetadataSize, DataSize.ofBytes(8192));
 		}
 
 		public void setMaxInboundMetadataSize(DataSize maxInboundMetadataSize) {
@@ -362,15 +339,15 @@ else if (maxSize != null && maxSize.toBytes() == -1) {
 		}
 
 		public NegotiationType getNegotiationType() {
-			return Objects.requireNonNullElse(this.negotiationType, DEFAULT_NEGOTIATION_TYPE);
+			return Objects.requireNonNullElse(this.negotiationType, NegotiationType.PLAINTEXT);
 		}
 
 		public void setNegotiationType(NegotiationType negotiationType) {
 			this.negotiationType = negotiationType;
 		}
 
 		public boolean isSecure() {
-			return Objects.requireNonNullElse(this.secure, DEFAULT_SECURE);
+			return Objects.requireNonNullElse(this.secure, true);
 		}
 
 		public void setSecure(boolean secure) {
@@ -394,7 +371,7 @@ public void setUserAgent(@Nullable String userAgent) {
 		}
 
 		public boolean isInheritDefaults() {
-			return Objects.requireNonNullElse(this.inheritDefaults, false);
+			return this.inheritDefaults;
 		}
 
 		public void setInheritDefaults(boolean inheritDefaults) {
@@ -435,27 +412,22 @@ ChannelConfig copy() {
 		 */
 		ChannelConfig mergeWith(ChannelConfig other) {
 			ChannelConfig merged = this.copy();
-			merged.address = Optional.ofNullable(other.address).orElse(merged.address);
-			merged.defaultDeadline = Optional.ofNullable(other.defaultDeadline).orElse(merged.defaultDeadline);
-			merged.defaultLoadBalancingPolicy = Optional.ofNullable(other.defaultLoadBalancingPolicy)
-				.orElse(merged.defaultLoadBalancingPolicy);
-			merged.enableKeepAlive = Optional.ofNullable(other.enableKeepAlive).orElse(merged.enableKeepAlive);
-			merged.idleTimeout = Optional.ofNullable(other.idleTimeout).orElse(merged.idleTimeout);
-			merged.keepAliveTime = Optional.ofNullable(other.keepAliveTime).orElse(merged.keepAliveTime);
-			merged.keepAliveTimeout = Optional.ofNullable(other.keepAliveTimeout).orElse(merged.keepAliveTimeout);
-			merged.keepAliveWithoutCalls = Optional.ofNullable(other.keepAliveWithoutCalls)
-				.orElse(merged.keepAliveWithoutCalls);
-			merged.maxInboundMessageSize = Optional.ofNullable(other.maxInboundMessageSize)
-				.orElse(merged.maxInboundMessageSize);
-			merged.maxInboundMetadataSize = Optional.ofNullable(other.maxInboundMetadataSize)
-				.orElse(merged.maxInboundMetadataSize);
-			merged.negotiationType = Optional.ofNullable(other.negotiationType).orElse(merged.negotiationType);
-			merged.secure = Optional.ofNullable(other.secure).orElse(merged.secure);
-			merged.userAgent = Optional.ofNullable(other.userAgent).orElse(merged.userAgent);
-
+			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);
 			}
@@ -515,8 +487,9 @@ void copyValuesFrom(Health other) {
 			 * @param other the configuration to merge with
 			 */
 			void mergeWith(Health other) {
-				this.enabled = Optional.ofNullable(other.enabled).orElse(this.enabled);
-				this.serviceName = Optional.ofNullable(other.serviceName).orElse(this.serviceName);
+				PropertyMapper map = PropertyMapper.get();
+				map.from(other.enabled).to(this::setEnabled);
+				map.from(other.serviceName).to(this::setServiceName);
 			}
 
 		}
@@ -569,8 +542,9 @@ void copyValuesFrom(Ssl other) {
 			 * @param other the configuration to merge with
 			 */
 			void mergeWith(Ssl other) {
-				this.enabled = Optional.ofNullable(other.enabled).orElse(this.enabled);
-				this.bundle = Optional.ofNullable(other.bundle).orElse(this.bundle);
+				PropertyMapper map = PropertyMapper.get();
+				map.from(other.enabled).to(this::setEnabled);
+				map.from(other.bundle).to(this::setBundle);
 			}
 
 		}