Skip to content

Commit 5d56ed5

Browse files
Sh1bariSh1bari
authored
feat: harden spec immutability (#968)
* fix metadata and error details to allow null map values
1 parent b6e2b87 commit 5d56ed5

44 files changed

Lines changed: 1207 additions & 53 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎compat-0.3/server-conversion/src/test/java/org/a2aproject/sdk/compat03/conversion/mappers/domain/TaskStatusUpdateEventMapper_v0_3_Test.java‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package org.a2aproject.sdk.compat03.conversion.mappers.domain;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertThrows;
5+
6+
import java.util.HashMap;
7+
import java.util.Map;
8+
9+
import org.a2aproject.sdk.compat03.spec.TaskState_v0_3;
10+
import org.a2aproject.sdk.compat03.spec.TaskStatusUpdateEvent_v0_3;
11+
import org.a2aproject.sdk.compat03.spec.TaskStatus_v0_3;
12+
import org.a2aproject.sdk.spec.TaskState;
13+
import org.a2aproject.sdk.spec.TaskStatus;
14+
import org.a2aproject.sdk.spec.TaskStatusUpdateEvent;
15+
import org.junit.jupiter.api.Test;
16+
17+
class TaskStatusUpdateEventMapper_v0_3_Test {
18+
19+
@Test
20+
void toV10_preservesMetadataAndDefensiveCopy() {
21+
Map<String, Object> mutableMetadata = new HashMap<>();
22+
mutableMetadata.put("source", "sensor");
23+
24+
TaskStatusUpdateEvent_v0_3 v03 = new TaskStatusUpdateEvent_v0_3.Builder()
25+
.taskId("task-123")
26+
.status(new TaskStatus_v0_3(TaskState_v0_3.WORKING))
27+
.contextId("context-456")
28+
.isFinal(false)
29+
.metadata(mutableMetadata)
30+
.build();
31+
32+
TaskStatusUpdateEvent v10 = TaskStatusUpdateEventMapper_v0_3.INSTANCE.toV10(v03);
33+
mutableMetadata.put("write", "should-not-leak");
34+
35+
assertEquals("task-123", v10.taskId());
36+
assertEquals(TaskState.TASK_STATE_WORKING, v10.status().state());
37+
assertEquals("context-456", v10.contextId());
38+
assertEquals(Map.of("source", "sensor"), v10.metadata());
39+
assertThrows(UnsupportedOperationException.class, () -> v10.metadata().put("another", "value"));
40+
}
41+
42+
@Test
43+
void fromV10_preservesMetadata() {
44+
TaskStatusUpdateEvent v10 = new TaskStatusUpdateEvent(
45+
"task-123",
46+
new TaskStatus(TaskState.TASK_STATE_WORKING),
47+
"context-456",
48+
Map.of("source", "sensor"));
49+
50+
TaskStatusUpdateEvent_v0_3 v03 = TaskStatusUpdateEventMapper_v0_3.INSTANCE.fromV10(v10);
51+
52+
assertEquals("task-123", v03.taskId());
53+
assertEquals(TaskState_v0_3.WORKING, v03.status().state());
54+
assertEquals("context-456", v03.contextId());
55+
assertEquals(Map.of("source", "sensor"), v03.metadata());
56+
}
57+
}

‎server-common/src/test/java/org/a2aproject/sdk/server/tasks/InMemoryTaskStoreTest.java‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package org.a2aproject.sdk.server.tasks;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertNull;
5+
import static org.junit.jupiter.api.Assertions.assertSame;
6+
import static org.junit.jupiter.api.Assertions.assertTrue;
7+
8+
import java.util.List;
9+
import java.util.Map;
10+
11+
import org.a2aproject.sdk.spec.Artifact;
12+
import org.a2aproject.sdk.spec.ListTasksParams;
13+
import org.a2aproject.sdk.spec.Message;
14+
import org.a2aproject.sdk.spec.Task;
15+
import org.a2aproject.sdk.spec.TaskState;
16+
import org.a2aproject.sdk.spec.TaskStatus;
17+
import org.a2aproject.sdk.spec.TextPart;
18+
import org.junit.jupiter.api.Test;
19+
20+
public class InMemoryTaskStoreTest {
21+
22+
@Test
23+
public void testSaveAndGet() {
24+
InMemoryTaskStore store = new InMemoryTaskStore();
25+
Task task = sampleTask("task-abc");
26+
27+
store.save(task, false);
28+
29+
Task retrieved = store.get(task.id());
30+
assertSame(task, retrieved);
31+
}
32+
33+
@Test
34+
public void testGetNonExistent() {
35+
InMemoryTaskStore store = new InMemoryTaskStore();
36+
37+
Task retrieved = store.get("nonexistent");
38+
assertNull(retrieved);
39+
}
40+
41+
@Test
42+
public void testDelete() {
43+
InMemoryTaskStore store = new InMemoryTaskStore();
44+
Task task = sampleTask("task-abc");
45+
46+
store.save(task, false);
47+
store.delete(task.id());
48+
49+
Task retrieved = store.get(task.id());
50+
assertNull(retrieved);
51+
}
52+
53+
@Test
54+
public void testDeleteNonExistent() {
55+
InMemoryTaskStore store = new InMemoryTaskStore();
56+
57+
store.delete("non-existent");
58+
}
59+
60+
@Test
61+
public void testListTransformsHistoryAndArtifacts() {
62+
InMemoryTaskStore store = new InMemoryTaskStore();
63+
Task task = Task.builder()
64+
.id("task-abc")
65+
.contextId("session-xyz")
66+
.status(new TaskStatus(TaskState.TASK_STATE_WORKING))
67+
.history(List.of(sampleMessage("msg-1"), sampleMessage("msg-2")))
68+
.artifacts(List.of(sampleArtifact("artifact-1")))
69+
.metadata(Map.of("origin", "test"))
70+
.build();
71+
72+
store.save(task, false);
73+
74+
ListTasksParams params = ListTasksParams.builder().build();
75+
List<Task> tasks = store.list(params, null).tasks();
76+
77+
assertEquals(1, tasks.size());
78+
Task listed = tasks.get(0);
79+
assertEquals(task.id(), listed.id());
80+
assertEquals(task.contextId(), listed.contextId());
81+
assertTrue(listed.history().isEmpty(), "Default list() should omit history");
82+
assertTrue(listed.artifacts().isEmpty(), "Default list() should omit artifacts");
83+
assertEquals(task.metadata(), listed.metadata());
84+
}
85+
86+
private static Task sampleTask(String id) {
87+
return Task.builder()
88+
.id(id)
89+
.contextId("session-xyz")
90+
.status(new TaskStatus(TaskState.TASK_STATE_SUBMITTED))
91+
.build();
92+
}
93+
94+
private static Message sampleMessage(String messageId) {
95+
return Message.builder()
96+
.role(Message.Role.ROLE_USER)
97+
.parts(List.of(new TextPart("content")))
98+
.messageId(messageId)
99+
.build();
100+
}
101+
102+
private static Artifact sampleArtifact(String artifactId) {
103+
return Artifact.builder()
104+
.artifactId(artifactId)
105+
.parts(List.of(new TextPart("artifact content")))
106+
.build();
107+
}
108+
}
109+

‎server-common/src/test/java/org/a2aproject/sdk/server/tasks/TaskManagerTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,6 @@ public void testSaveTaskEventStatusUpdate() throws A2AServerException {
9595

9696
assertEquals(initialTask.id(), updated.id());
9797
assertEquals(initialTask.contextId(), updated.contextId());
98-
// TODO type does not get unmarshalled
99-
//assertEquals(initialTask.getType(), updated.getType());
10098
assertSame(newStatus, updated.status());
10199
}
102100

‎spec/src/main/java/org/a2aproject/sdk/spec/A2AError.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.util.Map;
44

55
import org.a2aproject.sdk.util.Assert;
6+
import org.a2aproject.sdk.util.CollectionCopies;
67
import org.jspecify.annotations.Nullable;
78

89
/**
@@ -54,7 +55,7 @@ public class A2AError extends RuntimeException implements Event {
5455
public A2AError(Integer code, String message, @Nullable Map<String, Object> details) {
5556
super(Assert.checkNotNullParam("message", message));
5657
this.code = Assert.checkNotNullParam("code", code);
57-
this.details = details == null ? Map.of() : Map.copyOf(details);
58+
this.details = details == null ? Map.of() : CollectionCopies.unmodifiableShallowMap(details);
5859
}
5960

6061
/**

‎spec/src/main/java/org/a2aproject/sdk/spec/AgentCapabilities.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package org.a2aproject.sdk.spec;
22

3-
import java.util.List;
3+
import org.a2aproject.sdk.util.CollectionCopies;
44
import org.jspecify.annotations.Nullable;
55

6+
import java.util.List;
7+
68
/**
79
* Defines optional capabilities supported by an agent in the A2A Protocol.
810
* <p>
@@ -35,6 +37,10 @@ public record AgentCapabilities(boolean streaming,
3537
boolean extendedAgentCard,
3638
@Nullable List<AgentExtension> extensions) {
3739

40+
public AgentCapabilities {
41+
extensions = CollectionCopies.immutableNullableList(extensions);
42+
}
43+
3844
/**
3945
* Create a new Builder
4046
*

‎spec/src/main/java/org/a2aproject/sdk/spec/AgentCard.java‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
package org.a2aproject.sdk.spec;
22

3-
import java.util.ArrayList;
4-
import java.util.Collections;
5-
import java.util.List;
6-
import java.util.Map;
7-
83
import org.a2aproject.sdk.util.Assert;
4+
import org.a2aproject.sdk.util.CollectionCopies;
95
import org.jspecify.annotations.Nullable;
106

7+
import java.util.List;
8+
import java.util.Map;
9+
1110
/**
1211
* The AgentCard is a self-describing manifest for an agent in the A2A Protocol.
1312
* <p>
@@ -89,6 +88,14 @@ public record AgentCard(
8988
Assert.checkNotNullParam("skills", skills);
9089
Assert.checkNotNullParam("supportedInterfaces", supportedInterfaces);
9190
Assert.checkNotNullParam("version", version);
91+
defaultInputModes = CollectionCopies.immutableList(defaultInputModes);
92+
defaultOutputModes = CollectionCopies.immutableList(defaultOutputModes);
93+
skills = CollectionCopies.immutableList(skills);
94+
securitySchemes = CollectionCopies.immutableNullableMap(securitySchemes);
95+
securityRequirements = CollectionCopies.immutableNullableList(securityRequirements);
96+
supportedInterfaces = CollectionCopies.immutableList(supportedInterfaces);
97+
signatures = CollectionCopies.immutableNullableList(signatures);
98+
additionalInterfaces = CollectionCopies.immutableNullableList(additionalInterfaces);
9299
}
93100

94101
/**
@@ -184,17 +191,17 @@ private Builder(AgentCard card) {
184191
this.version = card.version();
185192
this.documentationUrl = card.documentationUrl();
186193
this.capabilities = card.capabilities();
187-
this.defaultInputModes = card.defaultInputModes() != null ? new ArrayList<>(card.defaultInputModes()) : Collections.emptyList();
188-
this.defaultOutputModes = card.defaultOutputModes() != null ? new ArrayList<>(card.defaultOutputModes()) : Collections.emptyList();
189-
this.skills = card.skills() != null ? new ArrayList<>(card.skills()) : Collections.emptyList();
190-
this.securitySchemes = card.securitySchemes() != null ? Map.copyOf(card.securitySchemes()) : Collections.emptyMap();
191-
this.securityRequirements = card.securityRequirements() != null ? new ArrayList<>(card.securityRequirements()) : Collections.emptyList();
194+
this.defaultInputModes = CollectionCopies.mutableListCopyOrEmpty(card.defaultInputModes());
195+
this.defaultOutputModes = CollectionCopies.mutableListCopyOrEmpty(card.defaultOutputModes());
196+
this.skills = CollectionCopies.mutableListCopyOrEmpty(card.skills());
197+
this.securitySchemes = CollectionCopies.mutableMapCopyOrEmpty(card.securitySchemes());
198+
this.securityRequirements = CollectionCopies.mutableListCopyOrEmpty(card.securityRequirements());
192199
this.iconUrl = card.iconUrl();
193-
this.supportedInterfaces = card.supportedInterfaces() != null ? new ArrayList<>(card.supportedInterfaces()) : Collections.emptyList();
194-
this.signatures = card.signatures() != null ? new ArrayList<>(card.signatures()) : null;
195-
this.url = card.url();
196-
this.preferredTransport= card.preferredTransport();
197-
this.additionalInterfaces = card.additionalInterfaces() != null ? new ArrayList<>(card.additionalInterfaces()) : null;
200+
this.supportedInterfaces = CollectionCopies.mutableListCopyOrEmpty(card.supportedInterfaces());
201+
this.signatures = CollectionCopies.immutableNullableList(card.signatures());
202+
this.url = card.url();
203+
this.preferredTransport = card.preferredTransport();
204+
this.additionalInterfaces = CollectionCopies.immutableNullableList(card.additionalInterfaces());
198205
}
199206

200207
/**

‎spec/src/main/java/org/a2aproject/sdk/spec/AgentCardSignature.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import com.google.gson.annotations.SerializedName;
66
import org.a2aproject.sdk.util.Assert;
7+
import org.a2aproject.sdk.util.CollectionCopies;
78
import org.jspecify.annotations.Nullable;
89

910
/**
@@ -46,6 +47,7 @@ public record AgentCardSignature(@Nullable Map<String, Object> header, @Serializ
4647
public AgentCardSignature {
4748
Assert.checkNotNullParam("protectedHeader", protectedHeader);
4849
Assert.checkNotNullParam("signature", signature);
50+
header = CollectionCopies.unmodifiableNullableShallowMap(header);
4951
}
5052

5153
/**

‎spec/src/main/java/org/a2aproject/sdk/spec/AgentExtension.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.util.Map;
44

55
import org.a2aproject.sdk.util.Assert;
6+
import org.a2aproject.sdk.util.CollectionCopies;
67
import org.jspecify.annotations.Nullable;
78

89
/**
@@ -37,6 +38,7 @@ public record AgentExtension (@Nullable String description, @Nullable Map<String
3738
*/
3839
public AgentExtension {
3940
Assert.checkNotNullParam("uri", uri);
41+
params = CollectionCopies.unmodifiableNullableShallowMap(params);
4042
}
4143

4244
/**

‎spec/src/main/java/org/a2aproject/sdk/spec/AgentSkill.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.util.List;
44

55
import org.a2aproject.sdk.util.Assert;
6+
import org.a2aproject.sdk.util.CollectionCopies;
67
import org.jspecify.annotations.Nullable;
78

89
/**
@@ -61,6 +62,11 @@ public record AgentSkill(String id, String name, String description, List<String
6162
Assert.checkNotNullParam("name", name);
6263
Assert.checkNotNullParam("description", description);
6364
Assert.checkNotNullParam("tags", tags);
65+
tags = CollectionCopies.immutableList(tags);
66+
examples = CollectionCopies.immutableNullableList(examples);
67+
inputModes = CollectionCopies.immutableNullableList(inputModes);
68+
outputModes = CollectionCopies.immutableNullableList(outputModes);
69+
securityRequirements = CollectionCopies.immutableNullableList(securityRequirements);
6470
}
6571

6672
/**
@@ -237,7 +243,7 @@ public AgentSkill build() {
237243
Assert.checkNotNullParam("id", id),
238244
Assert.checkNotNullParam("name", name),
239245
Assert.checkNotNullParam("description", description),
240-
Assert.checkNotNullParam("tags", tags),
246+
Assert.checkNotNullParam("tags", tags),
241247
examples,
242248
inputModes,
243249
outputModes,

‎spec/src/main/java/org/a2aproject/sdk/spec/Artifact.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.util.Map;
55

66
import org.a2aproject.sdk.util.Assert;
7+
import org.a2aproject.sdk.util.CollectionCopies;
78
import org.jspecify.annotations.Nullable;
89

910
/**
@@ -50,6 +51,9 @@ public record Artifact(String artifactId, @Nullable String name, @Nullable Strin
5051
if (parts.isEmpty()) {
5152
throw new IllegalArgumentException("Parts cannot be empty");
5253
}
54+
parts = CollectionCopies.immutableList(parts);
55+
metadata = CollectionCopies.unmodifiableNullableShallowMap(metadata);
56+
extensions = CollectionCopies.immutableNullableList(extensions);
5357
}
5458

5559
/**
@@ -169,7 +173,7 @@ public Builder parts(List<Part<?>> parts) {
169173
* @return this builder for method chaining
170174
*/
171175
public Builder parts(Part<?>... parts) {
172-
this.parts = List.of(parts);
176+
this.parts = CollectionCopies.immutableList(List.of(parts));
173177
return this;
174178
}
175179

@@ -191,7 +195,7 @@ public Builder metadata(@Nullable Map<String, Object> metadata) {
191195
* @return this builder for method chaining
192196
*/
193197
public Builder extensions(@Nullable List<String> extensions) {
194-
this.extensions = (extensions == null) ? null : List.copyOf(extensions);
198+
this.extensions = CollectionCopies.immutableNullableList(extensions);
195199
return this;
196200
}
197201

0 commit comments

Comments
 (0)