Only copy UriBuilderFactory when customized

This commit ensures that, when creating a RestClient.Builder from a
RestTemplate, the UriBuilderFactory is only copied if it has been
changed from the default values.
Before this commit, the UriBuilderFactory was effectively alway copied,
resulting in not being able to use a baseUrl.

This commit also introduces a small memory optimization in
DefaultUriBuilderFactory, so that default environment variables are
created lazily.

Closes gh-32180

Author: poutsma

Diff for: spring-web/src/main/java/org/springframework/web/client/DefaultRestClientBuilder.java

@@ -54,6 +54,7 @@
 import org.springframework.util.CollectionUtils;
 import org.springframework.web.util.DefaultUriBuilderFactory;
 import org.springframework.web.util.UriBuilderFactory;
+import org.springframework.web.util.UriTemplateHandler;
 
 /**
  * Default implementation of {@link RestClient.Builder}.
@@ -172,9 +173,7 @@ public DefaultRestClientBuilder(DefaultRestClientBuilder other) {
 	public DefaultRestClientBuilder(RestTemplate restTemplate) {
 		Assert.notNull(restTemplate, "RestTemplate must not be null");
 
-		if (restTemplate.getUriTemplateHandler() instanceof UriBuilderFactory builderFactory) {
-			this.uriBuilderFactory = builderFactory;
-		}
+		this.uriBuilderFactory = getUriBuilderFactory(restTemplate);
 		this.statusHandlers = new ArrayList<>();
 		this.statusHandlers.add(StatusHandler.fromErrorHandler(restTemplate.getErrorHandler()));
 
@@ -191,6 +190,26 @@ public DefaultRestClientBuilder(RestTemplate restTemplate) {
 		this.observationConvention = restTemplate.getObservationConvention();
 	}
 
+	@Nullable
+	private static UriBuilderFactory getUriBuilderFactory(RestTemplate restTemplate) {
+		UriTemplateHandler uriTemplateHandler = restTemplate.getUriTemplateHandler();
+		if (uriTemplateHandler instanceof DefaultUriBuilderFactory builderFactory) {
+			// only reuse the DefaultUriBuilderFactory if it has been customized
+			if (builderFactory.hasRestTemplateDefaults()) {
+				return null;
+			}
+			else {
+				return builderFactory;
+			}
+		}
+		else if (uriTemplateHandler instanceof UriBuilderFactory builderFactory) {
+			return builderFactory;
+		}
+		else {
+			return null;
+		}
+	}
+
 	private static ClientHttpRequestFactory getRequestFactory(RestTemplate restTemplate) {
 		ClientHttpRequestFactory requestFactory = restTemplate.getRequestFactory();
 		if (requestFactory instanceof InterceptingClientHttpRequestFactory interceptingClientHttpRequestFactory) {

Diff for: spring-web/src/main/java/org/springframework/web/util/DefaultUriBuilderFactory.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
 import java.util.Optional;
 
 import org.springframework.lang.Nullable;
+import org.springframework.util.CollectionUtils;
 import org.springframework.util.MultiValueMap;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
@@ -46,7 +47,8 @@ public class DefaultUriBuilderFactory implements UriBuilderFactory {
 
 	private EncodingMode encodingMode = EncodingMode.TEMPLATE_AND_VALUES;
 
-	private final Map<String, Object> defaultUriVariables = new HashMap<>();
+	@Nullable
+	private Map<String, Object> defaultUriVariables;
 
 	private boolean parsePath = true;
 
@@ -108,17 +110,31 @@ public EncodingMode getEncodingMode() {
 	 * @param defaultUriVariables default URI variable values
 	 */
 	public void setDefaultUriVariables(@Nullable Map<String, ?> defaultUriVariables) {
-		this.defaultUriVariables.clear();
 		if (defaultUriVariables != null) {
-			this.defaultUriVariables.putAll(defaultUriVariables);
+			if (this.defaultUriVariables == null) {
+				this.defaultUriVariables = new HashMap<>(defaultUriVariables);
+			}
+			else {
+				this.defaultUriVariables.putAll(defaultUriVariables);
+			}
+		}
+		else {
+			if (this.defaultUriVariables != null) {
+				this.defaultUriVariables.clear();
+			}
 		}
 	}
 
 	/**
 	 * Return the configured default URI variable values.
 	 */
 	public Map<String, ?> getDefaultUriVariables() {
-		return Collections.unmodifiableMap(this.defaultUriVariables);
+		if (this.defaultUriVariables != null) {
+			return Collections.unmodifiableMap(this.defaultUriVariables);
+		}
+		else {
+			return Collections.emptyMap();
+		}
 	}
 
 	/**
@@ -141,6 +157,20 @@ public boolean shouldParsePath() {
 		return this.parsePath;
 	}
 
+	/**
+	 * Indicates whether this {@code DefaultUriBuilderFactory} uses the default
+	 * {@link org.springframework.web.client.RestTemplate RestTemplate}
+	 * settings.
+	 * @since 6.1.4
+	 */
+	public boolean hasRestTemplateDefaults() {
+		// see RestTemplate::initUriTemplateHandler
+		return this.baseUri == null &&
+				this.encodingMode == EncodingMode.URI_COMPONENT &&
+				CollectionUtils.isEmpty(this.defaultUriVariables) &&
+				this.parsePath;
+	}
+
 
 	// UriTemplateHandler
 
@@ -379,8 +409,8 @@ public DefaultUriBuilder fragment(@Nullable String fragment) {
 
 		@Override
 		public URI build(Map<String, ?> uriVars) {
-			if (!defaultUriVariables.isEmpty()) {
-				Map<String, Object> map = new HashMap<>();
+			if (!CollectionUtils.isEmpty(defaultUriVariables)) {
+				Map<String, Object> map = new HashMap<>(defaultUriVariables.size() + uriVars.size());
 				map.putAll(defaultUriVariables);
 				map.putAll(uriVars);
 				uriVars = map;
@@ -394,7 +424,7 @@ public URI build(Map<String, ?> uriVars) {
 
 		@Override
 		public URI build(Object... uriVars) {
-			if (ObjectUtils.isEmpty(uriVars) && !defaultUriVariables.isEmpty()) {
+			if (ObjectUtils.isEmpty(uriVars) && !CollectionUtils.isEmpty(defaultUriVariables)) {
 				return build(Collections.emptyMap());
 			}
 			if (encodingMode.equals(EncodingMode.VALUES_ONLY)) {

Diff for: spring-web/src/test/java/org/springframework/web/client/RestClientBuilderTests.java

@@ -27,9 +27,11 @@
 import org.springframework.http.client.support.BasicAuthenticationInterceptor;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.http.converter.StringHttpMessageConverter;
+import org.springframework.lang.Nullable;
 import org.springframework.web.util.DefaultUriBuilderFactory;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
 
 
 /**
@@ -39,9 +41,9 @@ public class RestClientBuilderTests {
 
 	@SuppressWarnings("unchecked")
 	@Test
-	void createFromRestTemplate() throws NoSuchFieldException, IllegalAccessException {
+	void createFromRestTemplate() {
 		JettyClientHttpRequestFactory requestFactory = new JettyClientHttpRequestFactory();
-		DefaultUriBuilderFactory uriBuilderFactory = new DefaultUriBuilderFactory();
+		DefaultUriBuilderFactory uriBuilderFactory = new DefaultUriBuilderFactory("baseUri");
 		ResponseErrorHandler errorHandler = new DefaultResponseErrorHandler();
 		List<HttpMessageConverter<?>> restTemplateMessageConverters = List.of(new StringHttpMessageConverter());
 		ClientHttpRequestInterceptor interceptor = new BasicAuthenticationInterceptor("foo", "bar");
@@ -77,12 +79,28 @@ void createFromRestTemplate() throws NoSuchFieldException, IllegalAccessExceptio
 		assertThat(initializers).containsExactly(initializer);
 	}
 
-	private static Object fieldValue(String name, DefaultRestClientBuilder instance)
-			throws NoSuchFieldException, IllegalAccessException {
+	@Test
+	void defaultUriBuilderFactory() {
+		RestTemplate restTemplate = new RestTemplate();
+
+		RestClient.Builder builder = RestClient.builder(restTemplate);
+		assertThat(builder).isInstanceOf(DefaultRestClientBuilder.class);
+		DefaultRestClientBuilder defaultBuilder = (DefaultRestClientBuilder) builder;
 
-		Field field = DefaultRestClientBuilder.class.getDeclaredField(name);
-		field.setAccessible(true);
+		assertThat(fieldValue("uriBuilderFactory", defaultBuilder)).isNull();
+	}
 
-		return field.get(instance);
+	@Nullable
+	private static Object fieldValue(String name, DefaultRestClientBuilder instance) {
+		try {
+			Field field = DefaultRestClientBuilder.class.getDeclaredField(name);
+			field.setAccessible(true);
+
+			return field.get(instance);
+		}
+		catch (NoSuchFieldException | IllegalAccessException ex) {
+			fail(ex.getMessage(), ex);
+			return null;
+		}
 	}
 }