Skip to content

Addressing the remaining checkstyle failures #2738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -175,22 +175,6 @@ public List<McpSyncClient> mcpSyncClients(McpSyncClientConfigurer mcpSyncClientC
return mcpSyncClients;
}

/**
* Record class that implements {@link AutoCloseable} to ensure proper cleanup of MCP
* clients.
*
* <p>
* This class is responsible for closing all MCP sync clients when the application
* context is closed, preventing resource leaks.
*/
public record CloseableMcpSyncClients(List<McpSyncClient> clients) implements AutoCloseable {

@Override
public void close() {
this.clients.forEach(McpSyncClient::close);
}
}

/**
* Creates a closeable wrapper for MCP sync clients to ensure proper resource cleanup.
* @param clients the list of MCP sync clients to manage
Expand Down Expand Up @@ -258,13 +242,6 @@ public List<McpAsyncClient> mcpAsyncClients(McpAsyncClientConfigurer mcpSyncClie
return mcpSyncClients;
}

public record CloseableMcpAsyncClients(List<McpAsyncClient> clients) implements AutoCloseable {
@Override
public void close() {
this.clients.forEach(McpAsyncClient::close);
}
}

@Bean
@ConditionalOnProperty(prefix = McpClientCommonProperties.CONFIG_PREFIX, name = "type", havingValue = "ASYNC")
public CloseableMcpAsyncClients makeAsynClientsClosable(List<McpAsyncClient> clients) {
Expand All @@ -278,4 +255,27 @@ McpAsyncClientConfigurer mcpAsyncClientConfigurer(ObjectProvider<McpAsyncClientC
return new McpAsyncClientConfigurer(customizerProvider.orderedStream().toList());
}

/**
* Record class that implements {@link AutoCloseable} to ensure proper cleanup of MCP
* clients.
*
* <p>
* This class is responsible for closing all MCP sync clients when the application
* context is closed, preventing resource leaks.
*/
public record CloseableMcpSyncClients(List<McpSyncClient> clients) implements AutoCloseable {

@Override
public void close() {
this.clients.forEach(McpSyncClient::close);
}
}

public record CloseableMcpAsyncClients(List<McpAsyncClient> clients) implements AutoCloseable {
@Override
public void close() {
this.clients.forEach(McpAsyncClient::close);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,26 +40,6 @@
@Conditional(McpToolCallbackAutoConfiguration.McpToolCallbackAutoconfigurationCondition.class)
public class McpToolCallbackAutoConfiguration {

public static class McpToolCallbackAutoconfigurationCondition extends AllNestedConditions {

public McpToolCallbackAutoconfigurationCondition() {
super(ConfigurationPhase.PARSE_CONFIGURATION);
}

@ConditionalOnProperty(prefix = McpClientCommonProperties.CONFIG_PREFIX, name = "enabled", havingValue = "true",
matchIfMissing = true)
static class McpAutoConfigEnabled {

}

@ConditionalOnProperty(prefix = McpClientCommonProperties.CONFIG_PREFIX + ".toolcallback", name = "enabled",
havingValue = "true", matchIfMissing = false)
static class ToolCallbackProviderEnabled {

}

}

/**
* Creates tool callbacks for all configured MCP clients.
*
Expand All @@ -84,4 +64,24 @@ public ToolCallbackProvider mcpAsyncToolCallbacks(ObjectProvider<List<McpAsyncCl
return new AsyncMcpToolCallbackProvider(mcpClients);
}

public static class McpToolCallbackAutoconfigurationCondition extends AllNestedConditions {

public McpToolCallbackAutoconfigurationCondition() {
super(ConfigurationPhase.PARSE_CONFIGURATION);
}

@ConditionalOnProperty(prefix = McpClientCommonProperties.CONFIG_PREFIX, name = "enabled", havingValue = "true",
matchIfMissing = true)
static class McpAutoConfigEnabled {

}

@ConditionalOnProperty(prefix = McpClientCommonProperties.CONFIG_PREFIX + ".toolcallback", name = "enabled",
havingValue = "true", matchIfMissing = false)
static class ToolCallbackProviderEnabled {

}

}

}
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
/*
* Copyright 2024 - 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
* Copyright 2025-2025 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.ai.mcp.client.autoconfigure;

import io.modelcontextprotocol.spec.McpClientTransport;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@
import io.modelcontextprotocol.client.transport.HttpClientSseClientTransport;
import io.modelcontextprotocol.spec.McpSchema;

import org.springframework.beans.factory.ObjectProvider;
import org.springframework.ai.mcp.client.autoconfigure.properties.McpClientCommonProperties;
import org.springframework.ai.mcp.client.autoconfigure.properties.McpSseClientProperties;
import org.springframework.ai.mcp.client.autoconfigure.properties.McpSseClientProperties.SseParameters;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.AutoConfiguration;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import io.modelcontextprotocol.client.transport.WebFluxSseClientTransport;

import org.springframework.beans.factory.ObjectProvider;
import org.springframework.ai.mcp.client.autoconfigure.properties.McpClientCommonProperties;
import org.springframework.ai.mcp.client.autoconfigure.properties.McpSseClientProperties;
import org.springframework.ai.mcp.client.autoconfigure.properties.McpSseClientProperties.SseParameters;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.AutoConfiguration;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
/*
* Copyright 2024 - 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
* Copyright 2025-2025 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.ai.mcp.client.autoconfigure.properties;

import java.util.HashMap;
Expand Down Expand Up @@ -46,14 +47,6 @@ public class McpSseClientProperties {

public static final String CONFIG_PREFIX = "spring.ai.mcp.client.sse";

/**
* Parameters for configuring an SSE connection to an MCP server.
*
* @param url the URL endpoint for SSE communication with the MCP server
*/
public record SseParameters(String url) {
}

/**
* Map of named SSE connection configurations.
* <p>
Expand All @@ -70,4 +63,12 @@ public Map<String, SseParameters> getConnections() {
return this.connections;
}

/**
* Parameters for configuring an SSE connection to an MCP server.
*
* @param url the URL endpoint for SSE communication with the MCP server
*/
public record SseParameters(String url) {
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -73,32 +73,6 @@ public Map<String, Parameters> getConnections() {
return this.connections;
}

/**
* Record representing the parameters for an MCP server connection.
* <p>
* Includes the command to execute, command arguments, and environment variables.
*/
@JsonInclude(JsonInclude.Include.NON_ABSENT)
public record Parameters(
/**
* The command to execute for the MCP server.
*/
@JsonProperty("command") String command,
/**
* List of command arguments.
*/
@JsonProperty("args") List<String> args,
/**
* Map of environment variables for the server process.
*/
@JsonProperty("env") Map<String, String> env) {

public ServerParameters toServerParameters() {
return ServerParameters.builder(this.command()).args(this.args()).env(this.env()).build();
}

}

private Map<String, ServerParameters> resourceToServerParameters() {
try {
Map<String, Map<String, Parameters>> stdioConnection = new ObjectMapper().readValue(
Expand Down Expand Up @@ -133,4 +107,30 @@ public Map<String, ServerParameters> toServerParameters() {
return serverParameters;
}

/**
* Record representing the parameters for an MCP server connection.
* <p>
* Includes the command to execute, command arguments, and environment variables.
*/
@JsonInclude(JsonInclude.Include.NON_ABSENT)
public record Parameters(
/**
* The command to execute for the MCP server.
*/
@JsonProperty("command") String command,
/**
* List of command arguments.
*/
@JsonProperty("args") List<String> args,
/**
* Map of environment variables for the server process.
*/
@JsonProperty("env") Map<String, String> env) {

public ServerParameters toServerParameters() {
return ServerParameters.builder(this.command()).args(this.args()).env(this.env()).build();
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ void toolCallbacksCreation() {

@Test
void closeableWrappersCreation() {
this.contextRunner.withUserConfiguration(TestTransportConfiguration.class).run(context -> {
assertThat(context).hasSingleBean(McpClientAutoConfiguration.CloseableMcpSyncClients.class);
});
this.contextRunner.withUserConfiguration(TestTransportConfiguration.class)
.run(context -> assertThat(context)
.hasSingleBean(McpClientAutoConfiguration.CloseableMcpSyncClients.class));
}

@Configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,21 @@ public class McpToolCallbackAutoConfigurationTests {
@Test
void disabledByDeafault() {

this.applicationContext.run((context) -> {
this.applicationContext.run(context -> {
assertThat(context).doesNotHaveBean("mcpToolCallbacks");
assertThat(context).doesNotHaveBean("mcpAsyncToolCallbacks");
});

this.applicationContext
.withPropertyValues("spring.ai.mcp.client.enabled=true", "spring.ai.mcp.client.type=SYNC")
.run((context) -> {
.run(context -> {
assertThat(context).doesNotHaveBean("mcpToolCallbacks");
assertThat(context).doesNotHaveBean("mcpAsyncToolCallbacks");
});

this.applicationContext
.withPropertyValues("spring.ai.mcp.client.enabled=true", "spring.ai.mcp.client.type=ASYNC")
.run((context) -> {
.run(context -> {
assertThat(context).doesNotHaveBean("mcpToolCallbacks");
assertThat(context).doesNotHaveBean("mcpAsyncToolCallbacks");
});
Expand All @@ -55,31 +55,31 @@ void disabledByDeafault() {
void enabledMcpToolCallbackAutoconfiguration() {

// sync
this.applicationContext.withPropertyValues("spring.ai.mcp.client.toolcallback.enabled=true").run((context) -> {
this.applicationContext.withPropertyValues("spring.ai.mcp.client.toolcallback.enabled=true").run(context -> {
assertThat(context).hasBean("mcpToolCallbacks");
assertThat(context).doesNotHaveBean("mcpAsyncToolCallbacks");
});

this.applicationContext
.withPropertyValues("spring.ai.mcp.client.enabled=true", "spring.ai.mcp.client.toolcallback.enabled=true",
"spring.ai.mcp.client.type=SYNC")
.run((context) -> {
.run(context -> {
assertThat(context).hasBean("mcpToolCallbacks");
assertThat(context).doesNotHaveBean("mcpAsyncToolCallbacks");
});

// Async
this.applicationContext
.withPropertyValues("spring.ai.mcp.client.toolcallback.enabled=true", "spring.ai.mcp.client.type=ASYNC")
.run((context) -> {
.run(context -> {
assertThat(context).doesNotHaveBean("mcpToolCallbacks");
assertThat(context).hasBean("mcpAsyncToolCallbacks");
});

this.applicationContext
.withPropertyValues("spring.ai.mcp.client.enabled=true", "spring.ai.mcp.client.toolcallback.enabled=true",
"spring.ai.mcp.client.type=ASYNC")
.run((context) -> {
.run(context -> {
assertThat(context).doesNotHaveBean("mcpToolCallbacks");
assertThat(context).hasBean("mcpAsyncToolCallbacks");
});
Expand Down
Loading