Skip to content

Commit 10106ce

Browse files
luoxinerluoxiner
committed
fix: refactor MCP endpoint URL extraction
- Refactor Nacos MCP endpoint URL building to use McpEndpointInfo directly - Add unit tests for endpoint URL construction and remote config - Manage Surefire plugin in parent pom so JUnit 5 tests run Fix #121 Change-Id: Ic8287a5f8c1ce946c586d7360eead5f2faf2bceb
1 parent 7b4d16d commit 10106ce

File tree

3 files changed

+232
-106
lines changed

3 files changed

+232
-106
lines changed

‎himarket-server/src/main/java/com/alibaba/himarket/service/impl/NacosServiceImpl.java‎

Lines changed: 91 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import com.alibaba.nacos.api.PropertyKeyConst;
4949
import com.alibaba.nacos.api.ai.model.a2a.AgentCard;
5050
import com.alibaba.nacos.api.ai.model.a2a.AgentCardVersionInfo;
51+
import com.alibaba.nacos.api.ai.model.mcp.McpEndpointInfo;
5152
import com.alibaba.nacos.api.ai.model.mcp.McpServerBasicInfo;
5253
import com.alibaba.nacos.api.ai.model.mcp.McpServerDetailInfo;
5354
import com.alibaba.nacos.api.exception.NacosException;
@@ -61,6 +62,7 @@
6162
import com.aliyun.mse20190531.models.ListClustersResponse;
6263
import com.aliyun.mse20190531.models.ListClustersResponseBody;
6364
import com.aliyun.teautil.models.RuntimeOptions;
65+
import java.net.URI;
6466
import java.util.HashMap;
6567
import java.util.List;
6668
import java.util.Map;
@@ -359,11 +361,10 @@ private MCPConfigResult buildMCPConfigResult(McpServerDetailInfo detail) {
359361
}
360362

361363
private Object buildRemoteConnectionConfig(McpServerDetailInfo detail) {
362-
List<?> backendEndpoints = detail.getBackendEndpoints();
364+
List<McpEndpointInfo> backendEndpoints = detail.getBackendEndpoints();
363365

364366
if (backendEndpoints != null && !backendEndpoints.isEmpty()) {
365-
Object firstEndpoint = backendEndpoints.get(0);
366-
367+
McpEndpointInfo firstEndpoint = backendEndpoints.get(0);
367368
Map<String, Object> connectionConfig = new HashMap<>();
368369
Map<String, Object> mcpServers = new HashMap<>();
369370
Map<String, Object> serverConfig = new HashMap<>();
@@ -386,132 +387,117 @@ private Object buildRemoteConnectionConfig(McpServerDetailInfo detail) {
386387
return basicConfig;
387388
}
388389

389-
private String extractEndpointUrl(Object endpoint) {
390+
private String extractEndpointUrl(McpEndpointInfo endpoint) {
390391
if (endpoint == null) {
391392
return null;
392393
}
393394

394-
if (endpoint instanceof String) {
395-
return (String) endpoint;
395+
String address = endpoint.getAddress();
396+
if (StrUtil.isBlank(address)) {
397+
return null;
396398
}
397399

398-
if (endpoint instanceof Map) {
399-
Map<?, ?> endpointMap = (Map<?, ?>) endpoint;
400-
401-
String url = getStringValue(endpointMap, "url");
402-
if (url != null) return url;
403-
404-
String endpointUrl = getStringValue(endpointMap, "endpointUrl");
405-
if (endpointUrl != null) return endpointUrl;
406-
407-
String host = getStringValue(endpointMap, "host");
408-
String port = getStringValue(endpointMap, "port");
409-
String path = getStringValue(endpointMap, "path");
410-
411-
if (host != null) {
412-
StringBuilder urlBuilder = new StringBuilder();
413-
String protocol = getStringValue(endpointMap, "protocol");
414-
urlBuilder.append(protocol != null ? protocol : "http").append("://");
415-
urlBuilder.append(host);
416-
417-
if (port != null && !port.isEmpty()) {
418-
urlBuilder.append(":").append(port);
419-
}
400+
String protocol = StrUtil.blankToDefault(endpoint.getProtocol(), "http");
401+
int endpointPort = endpoint.getPort() > 0 ? endpoint.getPort() : -1;
402+
String normalizedPath = normalizeUrlPath(endpoint.getPath());
420403

421-
if (path != null && !path.isEmpty()) {
422-
if (!path.startsWith("/")) {
423-
urlBuilder.append("/");
424-
}
425-
urlBuilder.append(path);
404+
// If address is already a full URL, prefer it and only fill missing port/path.
405+
if (address.contains("://")) {
406+
try {
407+
URI uri = URI.create(address);
408+
String scheme = StrUtil.blankToDefault(uri.getScheme(), protocol);
409+
String host = uri.getHost();
410+
int port = uri.getPort() != -1 ? uri.getPort() : endpointPort;
411+
String path = StrUtil.isBlank(uri.getPath()) ? normalizedPath : uri.getPath();
412+
if (host != null) {
413+
return new URI(scheme, null, host, port, path, null, null).toString();
426414
}
427-
428-
return urlBuilder.toString();
415+
} catch (Exception ignored) {
416+
// fall back to simple concatenation
429417
}
430-
}
431-
432-
if (endpoint.getClass().getName().contains("McpEndpointInfo")) {
433-
return extractUrlFromMcpEndpointInfo(endpoint);
434-
}
435-
436-
return endpoint.toString();
437-
}
438-
439-
private String getStringValue(Map<?, ?> map, String key) {
440-
Object value = map.get(key);
441-
return value != null ? value.toString() : null;
442-
}
443-
444-
private String extractUrlFromMcpEndpointInfo(Object endpoint) {
445-
String[] possibleFieldNames = {"url", "endpointUrl", "address", "host", "endpoint"};
446-
447-
for (String fieldName : possibleFieldNames) {
448-
try {
449-
java.lang.reflect.Field field = endpoint.getClass().getDeclaredField(fieldName);
450-
field.setAccessible(true);
451-
Object value = field.get(endpoint);
452-
if (value != null && !value.toString().trim().isEmpty()) {
453-
if (value.toString().contains("://") || value.toString().contains(":")) {
454-
return value.toString();
418+
return appendPath(address, normalizedPath);
419+
}
420+
421+
// Address might be host or host:port (IPv4/hostname). Handle simple host:port.
422+
String host = address;
423+
int port = endpointPort;
424+
425+
// Handle bracketed IPv6 forms like "[::1]" or "[::1]:8848".
426+
if (address.startsWith("[") && address.contains("]")) {
427+
int endBracket = address.indexOf(']');
428+
host = address.substring(1, endBracket);
429+
if (port == -1) {
430+
String rest = address.substring(endBracket + 1);
431+
if (rest.startsWith(":")) {
432+
Integer parsedPort = tryParsePort(rest.substring(1));
433+
if (parsedPort != null) {
434+
port = parsedPort;
455435
}
456436
}
457-
} catch (Exception e) {
458-
continue;
459437
}
460-
}
461-
462-
java.lang.reflect.Field[] fields = endpoint.getClass().getDeclaredFields();
463-
464-
String host = null;
465-
String port = null;
466-
String path = null;
467-
String protocol = null;
468-
469-
for (java.lang.reflect.Field field : fields) {
470-
try {
471-
field.setAccessible(true);
472-
Object value = field.get(endpoint);
473-
if (value != null && !value.toString().trim().isEmpty()) {
474-
String fieldName = field.getName().toLowerCase();
475-
476-
if (fieldName.contains("host")
477-
|| fieldName.contains("ip")
478-
|| fieldName.contains("address")) {
479-
host = value.toString();
480-
} else if (fieldName.contains("port")) {
481-
port = value.toString();
482-
} else if (fieldName.contains("path")
483-
|| fieldName.contains("endpoint")
484-
|| fieldName.contains("uri")) {
485-
path = value.toString();
486-
} else if (fieldName.contains("protocol") || fieldName.contains("scheme")) {
487-
protocol = value.toString();
438+
} else {
439+
int lastColon = address.lastIndexOf(':');
440+
if (lastColon > 0 && address.indexOf(':') == lastColon) {
441+
Integer parsedPort = tryParsePort(address.substring(lastColon + 1));
442+
if (parsedPort != null) {
443+
host = address.substring(0, lastColon);
444+
if (port == -1) {
445+
port = parsedPort;
488446
}
489447
}
490-
} catch (Exception e) {
491-
continue;
492448
}
493449
}
494450

495-
if (host != null) {
451+
try {
452+
return new URI(protocol, null, host, port, normalizedPath, null, null).toString();
453+
} catch (Exception e) {
496454
StringBuilder urlBuilder = new StringBuilder();
497-
urlBuilder.append(protocol != null ? protocol : "http").append("://");
498-
urlBuilder.append(host);
499-
500-
if (port != null && !port.isEmpty()) {
455+
urlBuilder.append(protocol).append("://").append(address);
456+
if (port > 0 && !address.contains(":")) {
501457
urlBuilder.append(":").append(port);
502458
}
503-
504-
if (path != null && !path.isEmpty()) {
505-
if (!path.startsWith("/")) {
506-
urlBuilder.append("/");
507-
}
508-
urlBuilder.append(path);
459+
if (normalizedPath != null) {
460+
urlBuilder.append(normalizedPath);
509461
}
510-
511462
return urlBuilder.toString();
512463
}
464+
}
465+
466+
private String normalizeUrlPath(String path) {
467+
if (StrUtil.isBlank(path)) {
468+
return null;
469+
}
470+
return path.startsWith("/") ? path : "/" + path;
471+
}
472+
473+
private Integer tryParsePort(String portStr) {
474+
if (StrUtil.isBlank(portStr)) {
475+
return null;
476+
}
477+
for (int i = 0; i < portStr.length(); i++) {
478+
if (!Character.isDigit(portStr.charAt(i))) {
479+
return null;
480+
}
481+
}
482+
try {
483+
int port = Integer.parseInt(portStr);
484+
return port > 0 && port <= 65535 ? port : null;
485+
} catch (NumberFormatException e) {
486+
return null;
487+
}
488+
}
513489

514-
return endpoint.toString();
490+
private String appendPath(String baseUrl, String normalizedPath) {
491+
if (normalizedPath == null) {
492+
return baseUrl;
493+
}
494+
if (baseUrl.endsWith("/") && normalizedPath.startsWith("/")) {
495+
return baseUrl + normalizedPath.substring(1);
496+
}
497+
if (!baseUrl.endsWith("/") && !normalizedPath.startsWith("/")) {
498+
return baseUrl + "/" + normalizedPath;
499+
}
500+
return baseUrl + normalizedPath;
515501
}
516502

517503
private NacosInstance findNacosInstance(String nacosId) {

‎himarket-server/src/test/java/com/alibaba/himarket/service/impl/NacosServiceImplEndpointTest.java‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
package com.alibaba.himarket.service.impl;
2+
3+
import static org.junit.jupiter.api.Assertions.*;
4+
5+
import com.alibaba.nacos.api.ai.model.mcp.McpEndpointInfo;
6+
import com.alibaba.nacos.api.ai.model.mcp.McpServerDetailInfo;
7+
import java.lang.reflect.Method;
8+
import java.util.List;
9+
import java.util.Map;
10+
import org.junit.jupiter.api.Test;
11+
12+
class NacosServiceImplEndpointTest {
13+
14+
private static NacosServiceImpl newService() {
15+
return new NacosServiceImpl(null, null, null);
16+
}
17+
18+
private static McpEndpointInfo endpoint(
19+
String protocol, String address, int port, String path) {
20+
McpEndpointInfo info = new McpEndpointInfo();
21+
info.setProtocol(protocol);
22+
info.setAddress(address);
23+
info.setPort(port);
24+
info.setPath(path);
25+
return info;
26+
}
27+
28+
private static String invokeExtractEndpointUrl(
29+
NacosServiceImpl service, McpEndpointInfo endpoint) throws Exception {
30+
Method method =
31+
NacosServiceImpl.class.getDeclaredMethod(
32+
"extractEndpointUrl", McpEndpointInfo.class);
33+
method.setAccessible(true);
34+
return (String) method.invoke(service, endpoint);
35+
}
36+
37+
private static Object invokeBuildRemoteConnectionConfig(
38+
NacosServiceImpl service, McpServerDetailInfo detail) throws Exception {
39+
Method method =
40+
NacosServiceImpl.class.getDeclaredMethod(
41+
"buildRemoteConnectionConfig", McpServerDetailInfo.class);
42+
method.setAccessible(true);
43+
return method.invoke(service, detail);
44+
}
45+
46+
@Test
47+
void extractEndpointUrl_nullEndpoint_returnsNull() throws Exception {
48+
assertNull(invokeExtractEndpointUrl(newService(), null));
49+
}
50+
51+
@Test
52+
void extractEndpointUrl_blankAddress_returnsNull() throws Exception {
53+
McpEndpointInfo info = endpoint("http", " ", 8080, "/mcp");
54+
assertNull(invokeExtractEndpointUrl(newService(), info));
55+
}
56+
57+
@Test
58+
void extractEndpointUrl_fullUrl_fillsMissingPortAndPath() throws Exception {
59+
McpEndpointInfo info = endpoint("https", "http://example.com", 8443, "/mcp");
60+
assertEquals("http://example.com:8443/mcp", invokeExtractEndpointUrl(newService(), info));
61+
}
62+
63+
@Test
64+
void extractEndpointUrl_fullUrl_keepsExistingPath_andFillsMissingPort() throws Exception {
65+
McpEndpointInfo info = endpoint("http", "https://example.com/api", 1234, "/ignored");
66+
assertEquals("https://example.com:1234/api", invokeExtractEndpointUrl(newService(), info));
67+
}
68+
69+
@Test
70+
void extractEndpointUrl_hostPort_parsesPortWhenEndpointPortMissing() throws Exception {
71+
McpEndpointInfo info = endpoint(null, "example.com:8080", -1, "x");
72+
assertEquals("http://example.com:8080/x", invokeExtractEndpointUrl(newService(), info));
73+
}
74+
75+
@Test
76+
void extractEndpointUrl_hostPort_prefersEndpointPortWhenProvided() throws Exception {
77+
McpEndpointInfo info = endpoint(null, "example.com:8080", 9090, "x");
78+
assertEquals("http://example.com:9090/x", invokeExtractEndpointUrl(newService(), info));
79+
}
80+
81+
@Test
82+
void extractEndpointUrl_ipv6BracketWithPort_buildsCorrectUrl() throws Exception {
83+
McpEndpointInfo info = endpoint("http", "[::1]:8848", -1, "nacos");
84+
assertEquals("http://[::1]:8848/nacos", invokeExtractEndpointUrl(newService(), info));
85+
}
86+
87+
@Test
88+
@SuppressWarnings("unchecked")
89+
void buildRemoteConnectionConfig_withBackendEndpoint_setsUrlInMcpServers() throws Exception {
90+
NacosServiceImpl service = newService();
91+
92+
McpEndpointInfo backend = endpoint("http", "example.com:8080", -1, "/mcp");
93+
McpServerDetailInfo detail = new McpServerDetailInfo();
94+
detail.setName("demo");
95+
detail.setBackendEndpoints(List.of(backend));
96+
97+
Object config = invokeBuildRemoteConnectionConfig(service, detail);
98+
assertNotNull(config);
99+
assertTrue(config instanceof Map);
100+
101+
Map<String, Object> connectionConfig = (Map<String, Object>) config;
102+
assertTrue(connectionConfig.containsKey("mcpServers"));
103+
104+
Map<String, Object> mcpServers = (Map<String, Object>) connectionConfig.get("mcpServers");
105+
assertTrue(mcpServers.containsKey("demo"));
106+
107+
Map<String, Object> serverConfig = (Map<String, Object>) mcpServers.get("demo");
108+
assertEquals("http://example.com:8080/mcp", serverConfig.get("url"));
109+
}
110+
111+
@Test
112+
@SuppressWarnings("unchecked")
113+
void buildRemoteConnectionConfig_withoutEndpoints_returnsBasicRemoteConfig() throws Exception {
114+
NacosServiceImpl service = newService();
115+
116+
McpServerDetailInfo detail = new McpServerDetailInfo();
117+
detail.setName("demo");
118+
detail.setBackendEndpoints(List.of());
119+
120+
Object config = invokeBuildRemoteConnectionConfig(service, detail);
121+
assertNotNull(config);
122+
assertTrue(config instanceof Map);
123+
124+
Map<String, Object> basicConfig = (Map<String, Object>) config;
125+
assertEquals("remote", basicConfig.get("type"));
126+
assertEquals("demo", basicConfig.get("name"));
127+
assertEquals("http", basicConfig.get("protocol"));
128+
}
129+
}

0 commit comments

Comments
 (0)