Fix mutating global RestClient and WebClient builders (#3020)
Since the builders for HTTP clients are mutable and shared, they should only be configured with globally applicable settings. The current use leaks specific details into other usages and affects newly instantiated clients. This PR applies the clone() method right before mutation happens as it probably is the strategy that avoids multiple unnecessary copies. feat: improve RestClient and WebClient instantiation and configuration across AI modules - Ensure RestClient.Builder and WebClient.Builder are properly cloned before setting base URLs and headers in Mistral, OpenAI, and Ollama API classes to prevent side effects. - Add WebClientAutoConfiguration to auto-configuration classes for Anthropic and Mistral AI modules. - Update Mistral AI auto-configuration to inject and use WebClient.Builder. - Clean up test and annotation usage (remove unnecessary @Nullable, fix test builder usage in AnthropicApiIT). - Improve consistency and reliability of HTTP client configuration across model integrations. Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com> Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com> Co-authored-by: Christian Tzolov <christian.tzolov@broadcom.com>
This commit is contained in:
committed by
Christian Tzolov
parent
09a6a6e1e5
commit
97f90b19d6
@@ -52,7 +52,7 @@ import org.springframework.web.reactive.function.client.WebClient;
|
||||
* @since 1.0.0
|
||||
*/
|
||||
@AutoConfiguration(after = { RestClientAutoConfiguration.class, SpringAiRetryAutoConfiguration.class,
|
||||
ToolCallingAutoConfiguration.class })
|
||||
ToolCallingAutoConfiguration.class, WebClientAutoConfiguration.class })
|
||||
@EnableConfigurationProperties({ AnthropicChatProperties.class, AnthropicConnectionProperties.class })
|
||||
@ConditionalOnClass(AnthropicApi.class)
|
||||
@ConditionalOnProperty(name = SpringAIModelProperties.CHAT_MODEL, havingValue = SpringAIModels.ANTHROPIC,
|
||||
|
||||
@@ -35,6 +35,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
|
||||
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
|
||||
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
|
||||
import org.springframework.boot.autoconfigure.web.client.RestClientAutoConfiguration;
|
||||
import org.springframework.boot.autoconfigure.web.reactive.function.client.WebClientAutoConfiguration;
|
||||
import org.springframework.boot.context.properties.EnableConfigurationProperties;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.retry.support.RetryTemplate;
|
||||
@@ -42,6 +43,7 @@ import org.springframework.util.Assert;
|
||||
import org.springframework.util.StringUtils;
|
||||
import org.springframework.web.client.ResponseErrorHandler;
|
||||
import org.springframework.web.client.RestClient;
|
||||
import org.springframework.web.reactive.function.client.WebClient;
|
||||
|
||||
/**
|
||||
* Chat {@link AutoConfiguration Auto-configuration} for Mistral AI.
|
||||
@@ -52,28 +54,30 @@ import org.springframework.web.client.RestClient;
|
||||
* @author Ilayaperumal Gopinathan
|
||||
* @since 0.8.1
|
||||
*/
|
||||
@AutoConfiguration(after = { RestClientAutoConfiguration.class, SpringAiRetryAutoConfiguration.class,
|
||||
ToolCallingAutoConfiguration.class })
|
||||
@AutoConfiguration(after = { RestClientAutoConfiguration.class, WebClientAutoConfiguration.class,
|
||||
SpringAiRetryAutoConfiguration.class, ToolCallingAutoConfiguration.class })
|
||||
@EnableConfigurationProperties({ MistralAiCommonProperties.class, MistralAiChatProperties.class })
|
||||
@ConditionalOnProperty(name = SpringAIModelProperties.CHAT_MODEL, havingValue = SpringAIModels.MISTRAL,
|
||||
matchIfMissing = true)
|
||||
@ConditionalOnClass(MistralAiApi.class)
|
||||
@ImportAutoConfiguration(classes = { SpringAiRetryAutoConfiguration.class, RestClientAutoConfiguration.class,
|
||||
ToolCallingAutoConfiguration.class })
|
||||
WebClientAutoConfiguration.class, ToolCallingAutoConfiguration.class })
|
||||
public class MistralAiChatAutoConfiguration {
|
||||
|
||||
@Bean
|
||||
@ConditionalOnMissingBean
|
||||
public MistralAiChatModel mistralAiChatModel(MistralAiCommonProperties commonProperties,
|
||||
MistralAiChatProperties chatProperties, ObjectProvider<RestClient.Builder> restClientBuilderProvider,
|
||||
ToolCallingManager toolCallingManager, RetryTemplate retryTemplate,
|
||||
ResponseErrorHandler responseErrorHandler, ObjectProvider<ObservationRegistry> observationRegistry,
|
||||
ObjectProvider<WebClient.Builder> webClientBuilderProvider, ToolCallingManager toolCallingManager,
|
||||
RetryTemplate retryTemplate, ResponseErrorHandler responseErrorHandler,
|
||||
ObjectProvider<ObservationRegistry> observationRegistry,
|
||||
ObjectProvider<ChatModelObservationConvention> observationConvention,
|
||||
ObjectProvider<ToolExecutionEligibilityPredicate> mistralAiToolExecutionEligibilityPredicate) {
|
||||
|
||||
var mistralAiApi = mistralAiApi(chatProperties.getApiKey(), commonProperties.getApiKey(),
|
||||
chatProperties.getBaseUrl(), commonProperties.getBaseUrl(),
|
||||
restClientBuilderProvider.getIfAvailable(RestClient::builder), responseErrorHandler);
|
||||
restClientBuilderProvider.getIfAvailable(RestClient::builder),
|
||||
webClientBuilderProvider.getIfAvailable(WebClient::builder), responseErrorHandler);
|
||||
|
||||
var chatModel = MistralAiChatModel.builder()
|
||||
.mistralAiApi(mistralAiApi)
|
||||
@@ -91,7 +95,8 @@ public class MistralAiChatAutoConfiguration {
|
||||
}
|
||||
|
||||
private MistralAiApi mistralAiApi(String apiKey, String commonApiKey, String baseUrl, String commonBaseUrl,
|
||||
RestClient.Builder restClientBuilder, ResponseErrorHandler responseErrorHandler) {
|
||||
RestClient.Builder restClientBuilder, WebClient.Builder webClientBuilder,
|
||||
ResponseErrorHandler responseErrorHandler) {
|
||||
|
||||
var resolvedApiKey = StringUtils.hasText(apiKey) ? apiKey : commonApiKey;
|
||||
var resoledBaseUrl = StringUtils.hasText(baseUrl) ? baseUrl : commonBaseUrl;
|
||||
@@ -99,7 +104,8 @@ public class MistralAiChatAutoConfiguration {
|
||||
Assert.hasText(resolvedApiKey, "Mistral API key must be set");
|
||||
Assert.hasText(resoledBaseUrl, "Mistral base URL must be set");
|
||||
|
||||
return new MistralAiApi(resoledBaseUrl, resolvedApiKey, restClientBuilder, responseErrorHandler);
|
||||
return new MistralAiApi(resoledBaseUrl, resolvedApiKey, restClientBuilder, webClientBuilder,
|
||||
responseErrorHandler);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -119,12 +119,14 @@ public class AnthropicApi {
|
||||
|
||||
this.completionsPath = completionsPath;
|
||||
|
||||
this.restClient = restClientBuilder.baseUrl(baseUrl)
|
||||
this.restClient = restClientBuilder.clone()
|
||||
.baseUrl(baseUrl)
|
||||
.defaultHeaders(jsonContentHeaders)
|
||||
.defaultStatusHandler(responseErrorHandler)
|
||||
.build();
|
||||
|
||||
this.webClient = webClientBuilder.baseUrl(baseUrl)
|
||||
this.webClient = webClientBuilder.clone()
|
||||
.baseUrl(baseUrl)
|
||||
.defaultHeaders(jsonContentHeaders)
|
||||
.defaultStatusHandler(HttpStatusCode::isError,
|
||||
resp -> resp.bodyToMono(String.class)
|
||||
|
||||
@@ -110,7 +110,7 @@ public class AnthropicApiIT {
|
||||
void chatCompletionStreamError() {
|
||||
AnthropicMessage chatCompletionMessage = new AnthropicMessage(List.of(new ContentBlock("Tell me a Joke?")),
|
||||
Role.USER);
|
||||
AnthropicApi api = AnthropicApi.builder().baseUrl("FAKE_KEY_FOR_ERROR_RESPONSE").build();
|
||||
AnthropicApi api = AnthropicApi.builder().apiKey("FAKE_KEY_FOR_ERROR_RESPONSE").build();
|
||||
|
||||
Flux<ChatCompletionResponse> response = api.chatCompletionStream(new ChatCompletionRequest(
|
||||
AnthropicApi.ChatModel.CLAUDE_3_OPUS.getValue(), List.of(chatCompletionMessage), null, 100, 0.8, true));
|
||||
|
||||
@@ -115,7 +115,7 @@ public class MiniMaxApi {
|
||||
.defaultStatusHandler(responseErrorHandler)
|
||||
.build();
|
||||
|
||||
this.webClient = WebClient.builder()
|
||||
this.webClient = WebClient.builder() // FIXME: use a bean instead
|
||||
.baseUrl(baseUrl)
|
||||
.defaultHeaders(authHeaders)
|
||||
.build();
|
||||
|
||||
@@ -101,18 +101,31 @@ public class MistralAiApi {
|
||||
*/
|
||||
public MistralAiApi(String baseUrl, String mistralAiApiKey, RestClient.Builder restClientBuilder,
|
||||
ResponseErrorHandler responseErrorHandler) {
|
||||
this(baseUrl, mistralAiApiKey, restClientBuilder, WebClient.builder(), responseErrorHandler);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new client api.
|
||||
* @param baseUrl api base URL.
|
||||
* @param mistralAiApiKey Mistral api Key.
|
||||
* @param restClientBuilder RestClient builder.
|
||||
* @param responseErrorHandler Response error handler.
|
||||
*/
|
||||
public MistralAiApi(String baseUrl, String mistralAiApiKey, RestClient.Builder restClientBuilder,
|
||||
WebClient.Builder webClientBuilder, ResponseErrorHandler responseErrorHandler) {
|
||||
|
||||
Consumer<HttpHeaders> jsonContentHeaders = headers -> {
|
||||
headers.setBearerAuth(mistralAiApiKey);
|
||||
headers.setContentType(MediaType.APPLICATION_JSON);
|
||||
};
|
||||
|
||||
this.restClient = restClientBuilder.baseUrl(baseUrl)
|
||||
this.restClient = restClientBuilder.clone()
|
||||
.baseUrl(baseUrl)
|
||||
.defaultHeaders(jsonContentHeaders)
|
||||
.defaultStatusHandler(responseErrorHandler)
|
||||
.build();
|
||||
|
||||
this.webClient = WebClient.builder().baseUrl(baseUrl).defaultHeaders(jsonContentHeaders).build();
|
||||
this.webClient = webClientBuilder.clone().baseUrl(baseUrl).defaultHeaders(jsonContentHeaders).build();
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -79,12 +79,15 @@ public class OllamaApi {
|
||||
headers.setAccept(List.of(MediaType.APPLICATION_JSON));
|
||||
};
|
||||
|
||||
this.restClient = restClientBuilder.baseUrl(baseUrl)
|
||||
this.restClient = restClientBuilder
|
||||
.clone()
|
||||
.baseUrl(baseUrl)
|
||||
.defaultHeaders(defaultHeaders)
|
||||
.defaultStatusHandler(responseErrorHandler)
|
||||
.build();
|
||||
|
||||
this.webClient = webClientBuilder
|
||||
.clone()
|
||||
.baseUrl(baseUrl)
|
||||
.defaultHeaders(defaultHeaders)
|
||||
.build();
|
||||
|
||||
@@ -115,12 +115,13 @@ public class OpenAiApi {
|
||||
h.setContentType(MediaType.APPLICATION_JSON);
|
||||
h.addAll(headers);
|
||||
};
|
||||
this.restClient = restClientBuilder.baseUrl(baseUrl)
|
||||
this.restClient = restClientBuilder.clone()
|
||||
.baseUrl(baseUrl)
|
||||
.defaultHeaders(finalHeaders)
|
||||
.defaultStatusHandler(responseErrorHandler)
|
||||
.build();
|
||||
|
||||
this.webClient = webClientBuilder
|
||||
this.webClient = webClientBuilder.clone()
|
||||
.baseUrl(baseUrl)
|
||||
.defaultHeaders(finalHeaders)
|
||||
.build(); // @formatter:on
|
||||
|
||||
@@ -77,12 +77,13 @@ public class OpenAiAudioApi {
|
||||
// h.setContentType(MediaType.APPLICATION_JSON);
|
||||
};
|
||||
|
||||
this.restClient = restClientBuilder.baseUrl(baseUrl)
|
||||
this.restClient = restClientBuilder.clone()
|
||||
.baseUrl(baseUrl)
|
||||
.defaultHeaders(authHeaders)
|
||||
.defaultStatusHandler(responseErrorHandler)
|
||||
.build();
|
||||
|
||||
this.webClient = webClientBuilder.baseUrl(baseUrl).defaultHeaders(authHeaders).build();
|
||||
this.webClient = webClientBuilder.clone().baseUrl(baseUrl).defaultHeaders(authHeaders).build();
|
||||
}
|
||||
|
||||
public static Builder builder() {
|
||||
|
||||
@@ -116,7 +116,7 @@ public class ZhiPuAiApi {
|
||||
.defaultStatusHandler(responseErrorHandler)
|
||||
.build();
|
||||
|
||||
this.webClient = WebClient.builder()
|
||||
this.webClient = WebClient.builder() // FIXME: use a builder instead
|
||||
.baseUrl(baseUrl)
|
||||
.defaultHeaders(authHeaders)
|
||||
.build();
|
||||
|
||||
@@ -113,7 +113,6 @@ public class ChromaApi {
|
||||
return result;
|
||||
}
|
||||
|
||||
@Nullable
|
||||
public void createTenant(String tenantName) {
|
||||
|
||||
this.restClient.post()
|
||||
@@ -144,7 +143,6 @@ public class ChromaApi {
|
||||
}
|
||||
}
|
||||
|
||||
@Nullable
|
||||
public void createDatabase(String tenantName, String databaseName) {
|
||||
|
||||
this.restClient.post()
|
||||
|
||||
Reference in New Issue
Block a user