Skip to content

Commit d121d5c

Browse files
fjumafjuma
authored
fix: Incorporate latest feedback from #138 (#182)
# Description This PR incorporates the latest feedback from #138. - [x] Follow the [`CONTRIBUTING` Guide](../CONTRIBUTING.md). - [x] Make your Pull Request title in the <https://www.conventionalcommits.org/> specification. - Important Prefixes for [release-please](https://github.com/googleapis/release-please): - `fix:` which represents bug fixes, and correlates to a [SemVer](https://semver.org/) patch. - `feat:` represents a new feature, and correlates to a SemVer minor. - `feat!:`, or `fix!:`, `refactor!:`, etc., which represent a breaking change (indicated by the `!`) and will result in a SemVer major. - [x] Ensure the tests pass - [x] Appropriate READMEs were updated (if necessary)
1 parent 8d31ddf commit d121d5c

File tree

4 files changed

+46
-37
lines changed

4 files changed

+46
-37
lines changed

‎client/src/main/java/io/a2a/client/A2ACardResolver.java‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import static io.a2a.util.Utils.unmarshalFrom;
44

55
import java.io.IOException;
6+
import java.net.URI;
7+
import java.net.URISyntaxException;
68
import java.util.Map;
79

810
import com.fasterxml.jackson.core.JsonProcessingException;
@@ -18,14 +20,16 @@ public class A2ACardResolver {
1820
private final String url;
1921
private final Map<String, String> authHeaders;
2022

21-
static String DEFAULT_AGENT_CARD_PATH = "/.well-known/agent.json";
23+
private static final String DEFAULT_AGENT_CARD_PATH = "/.well-known/agent.json";
24+
25+
private static final TypeReference<AgentCard> AGENT_CARD_TYPE_REFERENCE = new TypeReference<>() {};
2226

23-
static final TypeReference<AgentCard> AGENT_CARD_TYPE_REFERENCE = new TypeReference<>() {};
2427
/**
2528
* @param httpClient the http client to use
2629
* @param baseUrl the base URL for the agent whose agent card we want to retrieve
30+
* @throws A2AClientError if the URL for the agent is invalid
2731
*/
28-
public A2ACardResolver(A2AHttpClient httpClient, String baseUrl) {
32+
public A2ACardResolver(A2AHttpClient httpClient, String baseUrl) throws A2AClientError {
2933
this(httpClient, baseUrl, null, null);
3034
}
3135

@@ -34,8 +38,9 @@ public A2ACardResolver(A2AHttpClient httpClient, String baseUrl) {
3438
* @param baseUrl the base URL for the agent whose agent card we want to retrieve
3539
* @param agentCardPath optional path to the agent card endpoint relative to the base
3640
* agent URL, defaults to ".well-known/agent.json"
41+
* @throws A2AClientError if the URL for the agent is invalid
3742
*/
38-
public A2ACardResolver(A2AHttpClient httpClient, String baseUrl, String agentCardPath) {
43+
public A2ACardResolver(A2AHttpClient httpClient, String baseUrl, String agentCardPath) throws A2AClientError {
3944
this(httpClient, baseUrl, agentCardPath, null);
4045
}
4146

@@ -45,17 +50,17 @@ public A2ACardResolver(A2AHttpClient httpClient, String baseUrl, String agentCar
4550
* @param agentCardPath optional path to the agent card endpoint relative to the base
4651
* agent URL, defaults to ".well-known/agent.json"
4752
* @param authHeaders the HTTP authentication headers to use. May be {@code null}
53+
* @throws A2AClientError if the URL for the agent is invalid
4854
*/
49-
public A2ACardResolver(A2AHttpClient httpClient, String baseUrl, String agentCardPath, Map<String, String> authHeaders) {
55+
public A2ACardResolver(A2AHttpClient httpClient, String baseUrl, String agentCardPath,
56+
Map<String, String> authHeaders) throws A2AClientError {
5057
this.httpClient = httpClient;
51-
if (!baseUrl.endsWith("/")) {
52-
baseUrl += "/";
53-
}
5458
agentCardPath = agentCardPath == null || agentCardPath.isEmpty() ? DEFAULT_AGENT_CARD_PATH : agentCardPath;
55-
if (agentCardPath.startsWith("/")) {
56-
agentCardPath = agentCardPath.substring(1);
59+
try {
60+
this.url = new URI(baseUrl).resolve(agentCardPath).toString();
61+
} catch (URISyntaxException e) {
62+
throw new A2AClientError("Invalid agent URL", e);
5763
}
58-
this.url = baseUrl + agentCardPath;
5964
this.authHeaders = authHeaders;
6065
}
6166

‎client/src/test/java/io/a2a/client/A2ACardResolverTest.java‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package io.a2a.client;
22

3-
import static io.a2a.client.A2ACardResolver.AGENT_CARD_TYPE_REFERENCE;
43
import static io.a2a.util.Utils.OBJECT_MAPPER;
54
import static io.a2a.util.Utils.unmarshalFrom;
65
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -11,6 +10,7 @@
1110
import java.util.concurrent.CompletableFuture;
1211
import java.util.function.Consumer;
1312

13+
import com.fasterxml.jackson.core.type.TypeReference;
1414
import io.a2a.http.A2AHttpClient;
1515
import io.a2a.http.A2AHttpResponse;
1616
import io.a2a.spec.A2AClientError;
@@ -19,6 +19,10 @@
1919
import org.junit.jupiter.api.Test;
2020

2121
public class A2ACardResolverTest {
22+
23+
private static final String AGENT_CARD_PATH = "/.well-known/agent.json";
24+
private static final TypeReference<AgentCard> AGENT_CARD_TYPE_REFERENCE = new TypeReference<>() {};
25+
2226
@Test
2327
public void testConstructorStripsSlashes() throws Exception {
2428
TestHttpClient client = new TestHttpClient();
@@ -27,33 +31,37 @@ public void testConstructorStripsSlashes() throws Exception {
2731
A2ACardResolver resolver = new A2ACardResolver(client, "http://example.com/");
2832
AgentCard card = resolver.getAgentCard();
2933

30-
assertEquals("http://example.com" + A2ACardResolver.DEFAULT_AGENT_CARD_PATH, client.url);
34+
assertEquals("http://example.com" + AGENT_CARD_PATH, client.url);
3135

3236

3337
resolver = new A2ACardResolver(client, "http://example.com");
3438
card = resolver.getAgentCard();
3539

36-
assertEquals("http://example.com" + A2ACardResolver.DEFAULT_AGENT_CARD_PATH, client.url);
40+
assertEquals("http://example.com" + AGENT_CARD_PATH, client.url);
3741

38-
resolver = new A2ACardResolver(client, "http://example.com/", A2ACardResolver.DEFAULT_AGENT_CARD_PATH);
42+
// baseUrl with trailing slash, agentCardParth with leading slash
43+
resolver = new A2ACardResolver(client, "http://example.com/", AGENT_CARD_PATH);
3944
card = resolver.getAgentCard();
4045

41-
assertEquals("http://example.com" + A2ACardResolver.DEFAULT_AGENT_CARD_PATH, client.url);
46+
assertEquals("http://example.com" + AGENT_CARD_PATH, client.url);
4247

43-
resolver = new A2ACardResolver(client, "http://example.com", A2ACardResolver.DEFAULT_AGENT_CARD_PATH);
48+
// baseUrl without trailing slash, agentCardPath with leading slash
49+
resolver = new A2ACardResolver(client, "http://example.com", AGENT_CARD_PATH);
4450
card = resolver.getAgentCard();
4551

46-
assertEquals("http://example.com" + A2ACardResolver.DEFAULT_AGENT_CARD_PATH, client.url);
52+
assertEquals("http://example.com" + AGENT_CARD_PATH, client.url);
4753

48-
resolver = new A2ACardResolver(client, "http://example.com/", A2ACardResolver.DEFAULT_AGENT_CARD_PATH.substring(0));
54+
// baseUrl with trailing slash, agentCardPath without leading slash
55+
resolver = new A2ACardResolver(client, "http://example.com/", AGENT_CARD_PATH.substring(1));
4956
card = resolver.getAgentCard();
5057

51-
assertEquals("http://example.com" + A2ACardResolver.DEFAULT_AGENT_CARD_PATH, client.url);
58+
assertEquals("http://example.com" + AGENT_CARD_PATH, client.url);
5259

53-
resolver = new A2ACardResolver(client, "http://example.com", A2ACardResolver.DEFAULT_AGENT_CARD_PATH.substring(0));
60+
// baseUrl without trailing slash, agentCardPath without leading slash
61+
resolver = new A2ACardResolver(client, "http://example.com", AGENT_CARD_PATH.substring(1));
5462
card = resolver.getAgentCard();
5563

56-
assertEquals("http://example.com" + A2ACardResolver.DEFAULT_AGENT_CARD_PATH, client.url);
64+
assertEquals("http://example.com" + AGENT_CARD_PATH, client.url);
5765
}
5866

5967

‎reference-impl/src/main/java/io/a2a/server/apps/quarkus/A2AServerRoutes.java‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import java.util.concurrent.atomic.AtomicLong;
99
import java.util.function.Function;
1010

11+
import com.fasterxml.jackson.databind.JsonNode;
1112
import jakarta.enterprise.inject.Instance;
1213
import jakarta.inject.Inject;
1314
import jakarta.inject.Singleton;
@@ -212,16 +213,14 @@ private JSONRPCResponse<?> generateErrorResponse(JSONRPCRequest<?> request, JSON
212213
}
213214

214215
private static boolean isStreamingRequest(String requestBody) {
215-
return requestBody.contains(SendStreamingMessageRequest.METHOD) ||
216-
requestBody.contains(TaskResubscriptionRequest.METHOD);
217-
}
218-
219-
private static boolean isNonStreamingRequest(String requestBody) {
220-
return requestBody.contains(GetTaskRequest.METHOD) ||
221-
requestBody.contains(CancelTaskRequest.METHOD) ||
222-
requestBody.contains(SendMessageRequest.METHOD) ||
223-
requestBody.contains(SetTaskPushNotificationConfigRequest.METHOD) ||
224-
requestBody.contains(GetTaskPushNotificationConfigRequest.METHOD);
216+
try {
217+
JsonNode node = Utils.OBJECT_MAPPER.readTree(requestBody);
218+
JsonNode method = node != null ? node.get("method") : null;
219+
return method != null && (SendStreamingMessageRequest.METHOD.equals(method.asText())
220+
|| TaskResubscriptionRequest.METHOD.equals(method.asText()));
221+
} catch (Exception e) {
222+
return false;
223+
}
225224
}
226225

227226
static void setStreamingMultiSseSupportSubscribedRunnable(Runnable runnable) {

‎sdk-server-common/src/main/java/io/a2a/server/tasks/ResultAggregator.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import static io.a2a.server.util.async.AsyncUtils.createTubeConfig;
55
import static io.a2a.server.util.async.AsyncUtils.processor;
66

7+
import java.util.concurrent.CompletableFuture;
78
import java.util.concurrent.Flow;
89
import java.util.concurrent.atomic.AtomicBoolean;
910
import java.util.concurrent.atomic.AtomicReference;
@@ -100,11 +101,7 @@ public EventTypeAndInterrupt consumeAndBreakOnInterrupt(EventConsumer consumer)
100101
// new request is expected in order for the agent to make progress,
101102
// so the agent should exit.
102103

103-
// TODO There is the following line in the Python code I don't totally get
104-
// asyncio.create_task(self._continue_consuming(event_stream))
105-
// I think it means the continueConsuming() call should be done in another thread
106-
continueConsuming(all);
107-
104+
CompletableFuture.runAsync(() -> continueConsuming(all));
108105
interrupted.set(true);
109106
return false;
110107
}

0 commit comments

Comments
 (0)