Skip to content

Commit a5fd351

Browse files
committed
chore: 553: Addressed PR comments from Pull request 564. Changes related to PushNotificationConfig pagination feature
1 parent 882f064 commit a5fd351

File tree

8 files changed

+155
-45
lines changed

8 files changed

+155
-45
lines changed

extras/push-notification-config-store-database-jpa/src/main/java/io/a2a/extras/pushnotificationconfigstore/database/jpa/JpaDatabasePushNotificationConfigStore.java

Lines changed: 58 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package io.a2a.extras.pushnotificationconfigstore.database.jpa;
22

3+
import io.a2a.server.config.A2AConfigProvider;
4+
import jakarta.annotation.PostConstruct;
5+
import jakarta.inject.Inject;
36
import jakarta.persistence.TypedQuery;
47
import java.time.Instant;
58
import java.util.List;
@@ -29,18 +32,42 @@ public class JpaDatabasePushNotificationConfigStore implements PushNotificationC
2932
private static final Logger LOGGER = LoggerFactory.getLogger(JpaDatabasePushNotificationConfigStore.class);
3033

3134
private static final Instant NULL_TIMESTAMP_SENTINEL = Instant.EPOCH;
35+
private static final String A2A_PUSH_NOTIFICATION_MAX_PAGE_SIZE_CONFIG = "a2a.push-notification-config.max-page-size";
36+
private static final int A2A_PUSH_NOTIFICATION_DEFAULT_MAX_PAGE_SIZE = 100;
3237

3338
@PersistenceContext(unitName = "a2a-java")
3439
EntityManager em;
3540

41+
@Inject
42+
A2AConfigProvider configProvider;
43+
44+
/**
45+
* Maximum page size when listing push notification configurations for a task.
46+
* Requested page sizes exceeding this value will be capped to this limit.
47+
* <p>
48+
* Property: {@code a2a.push-notification-config.max-page-size}<br>
49+
* Default: 100<br>
50+
* Note: Property override requires a configurable {@link A2AConfigProvider} on the classpath.
51+
*/
52+
int maxPageSize;
53+
54+
@PostConstruct
55+
void initConfig() {
56+
try {
57+
maxPageSize = Integer.parseInt(configProvider.getValue(A2A_PUSH_NOTIFICATION_MAX_PAGE_SIZE_CONFIG));
58+
} catch (Exception e) {
59+
LOGGER.warn("Failed to read '{}' configuration, falling back to default page size of {}.",
60+
A2A_PUSH_NOTIFICATION_MAX_PAGE_SIZE_CONFIG, A2A_PUSH_NOTIFICATION_DEFAULT_MAX_PAGE_SIZE, e);
61+
maxPageSize = A2A_PUSH_NOTIFICATION_DEFAULT_MAX_PAGE_SIZE;
62+
}
63+
}
64+
3665
@Transactional
3766
@Override
3867
public PushNotificationConfig setInfo(String taskId, PushNotificationConfig notificationConfig) {
3968
// Ensure config has an ID - default to taskId if not provided (mirroring InMemoryPushNotificationConfigStore behavior)
4069
PushNotificationConfig.Builder builder = PushNotificationConfig.builder(notificationConfig);
4170
if (notificationConfig.id() == null || notificationConfig.id().isEmpty()) {
42-
// This means the taskId and configId are same. This will not allow having multiple configs for a single Task.
43-
// The configId is a required field in the spec and should not be empty
4471
builder.id(taskId);
4572
}
4673
notificationConfig = builder.build();
@@ -80,44 +107,48 @@ public ListTaskPushNotificationConfigResult getInfo(ListTaskPushNotificationConf
80107
LOGGER.debug("Retrieving PushNotificationConfigs for Task '{}' with params: pageSize={}, pageToken={}",
81108
taskId, params.pageSize(), params.pageToken());
82109
try {
83-
StringBuilder queryBuilder = new StringBuilder("SELECT c FROM JpaPushNotificationConfig c WHERE c.id.taskId = :taskId");
110+
// Parse pageToken once upfront
111+
Instant tokenTimestamp = null;
112+
String tokenId = null;
84113

85114
if (params.pageToken() != null && !params.pageToken().isEmpty()) {
86-
String[] tokenParts = params.pageToken().split(":", 2);
87-
if (tokenParts.length == 2) {
88-
// Keyset pagination: get tasks where timestamp < tokenTimestamp OR (timestamp = tokenTimestamp AND id > tokenId)
89-
// All tasks have timestamps (TaskStatus canonical constructor ensures this)
115+
String[] tokenParts = params.pageToken().split(":", 2);
116+
if (tokenParts.length != 2) {
117+
throw new io.a2a.spec.InvalidParamsError(null,
118+
"Invalid pageToken format: pageToken must be in 'timestamp_millis:configId' format", null);
119+
}
120+
121+
try {
122+
long timestampMillis = Long.parseLong(tokenParts[0]);
123+
tokenTimestamp = Instant.ofEpochMilli(timestampMillis);
124+
tokenId = tokenParts[1];
125+
} catch (NumberFormatException e) {
126+
throw new io.a2a.spec.InvalidParamsError(null,
127+
"Invalid pageToken format: timestamp must be numeric milliseconds", null);
128+
}
129+
}
130+
131+
// Build query using the parsed values
132+
StringBuilder queryBuilder = new StringBuilder("SELECT c FROM JpaPushNotificationConfig c WHERE c.id.taskId = :taskId");
133+
134+
if (tokenTimestamp != null) {
135+
// Keyset pagination: get notifications where timestamp < tokenTimestamp OR (timestamp = tokenTimestamp AND id > tokenId)
90136
queryBuilder.append(" AND (COALESCE(c.createdAt, :nullSentinel) < :tokenTimestamp OR (COALESCE(c.createdAt, :nullSentinel) = :tokenTimestamp AND c.id.configId > :tokenId))");
91-
} else {
92-
// Based on the comments in the test case, if the pageToken is invalid start from the beginning.
93-
}
94137
}
95138

96139
queryBuilder.append(" ORDER BY COALESCE(c.createdAt, :nullSentinel) DESC, c.id.configId ASC");
97140

141+
// Create query and set parameters
98142
TypedQuery<JpaPushNotificationConfig> query = em.createQuery(queryBuilder.toString(), JpaPushNotificationConfig.class);
99143
query.setParameter("taskId", taskId);
100144
query.setParameter("nullSentinel", NULL_TIMESTAMP_SENTINEL);
101145

102-
if (params.pageToken() != null && !params.pageToken().isEmpty()) {
103-
String[] tokenParts = params.pageToken().split(":", 2);
104-
if (tokenParts.length == 2) {
105-
try {
106-
long timestampMillis = Long.parseLong(tokenParts[0]);
107-
String tokenId = tokenParts[1];
108-
109-
Instant tokenTimestamp = Instant.ofEpochMilli(timestampMillis);
110-
query.setParameter("tokenTimestamp", tokenTimestamp);
111-
query.setParameter("tokenId", tokenId);
112-
} catch (NumberFormatException e) {
113-
// Malformed timestamp in pageToken
114-
throw new io.a2a.spec.InvalidParamsError(null,
115-
"Invalid pageToken format: timestamp must be numeric milliseconds", null);
116-
}
117-
}
146+
if (tokenTimestamp != null) {
147+
query.setParameter("tokenTimestamp", tokenTimestamp);
148+
query.setParameter("tokenId", tokenId);
118149
}
119150

120-
int pageSize = params.getEffectivePageSize();
151+
int pageSize = params.getEffectivePageSize(maxPageSize);
121152
query.setMaxResults(pageSize + 1);
122153
List<JpaPushNotificationConfig> jpaConfigsPage = query.getResultList();
123154

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# A2A JPA Database Push Notification Config Store Default Configuration
2+
3+
# Maximum page size when listing push notification configurations for a task
4+
# Requested page sizes exceeding this value will be capped to this limit
5+
# Used as default when pageSize parameter is not specified or is invalid
6+
a2a.push-notification-config.max-page-size=100
7+

extras/push-notification-config-store-database-jpa/src/test/java/io/a2a/extras/pushnotificationconfigstore/database/jpa/JpaDatabasePushNotificationConfigStoreIntegrationTest.java

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import static org.junit.jupiter.api.Assertions.assertThrows;
88
import static org.junit.jupiter.api.Assertions.assertTrue;
99

10+
import io.a2a.spec.InvalidParamsError;
1011
import java.util.List;
1112
import java.util.Queue;
1213
import java.util.concurrent.CountDownLatch;
@@ -293,7 +294,7 @@ public void testPaginationWithZeroPageSize() {
293294
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
294295

295296
assertNotNull(result);
296-
assertEquals(5, result.configs().size(), "Should return all 5 configs when pageSize=0");
297+
assertEquals(5, result.configs().size(), "Should return all default capped 5 configs when pageSize=0");
297298
assertNull(result.nextPageToken(), "Should not have nextPageToken when returning all");
298299
}
299300

@@ -355,12 +356,10 @@ public void testPaginationWithInvalidToken() {
355356
// Request with invalid pageToken - JPA implementation behavior is to start from beginning
356357
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(
357358
taskId, 2, "invalid_token_that_does_not_exist", "");
358-
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
359359

360-
assertNotNull(result);
361-
// When token is not found, implementation starts from beginning
362-
assertEquals(2, result.configs().size(), "Should return first page when token is not found");
363-
assertNotNull(result.nextPageToken(), "Should have nextPageToken since more items exist");
360+
assertThrows(InvalidParamsError.class, () ->
361+
pushNotificationConfigStore.getInfo(params),
362+
"Invalid pageToken format: pageToken must be in 'timestamp_millis:configId' format");
364363
}
365364

366365
@Test
@@ -428,12 +427,10 @@ public void testPageTokenWithMissingColon() {
428427

429428
ListTaskPushNotificationConfigParams params =
430429
new ListTaskPushNotificationConfigParams(taskId, 2, "123456789cfg1", "");
431-
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
432430

433-
assertNotNull(result);
434-
assertEquals(2, result.configs().size(),
435-
"Should return first page when pageToken format is invalid (missing colon)");
436-
assertNotNull(result.nextPageToken(), "Should have nextPageToken since more items exist");
431+
assertThrows(InvalidParamsError.class, () ->
432+
pushNotificationConfigStore.getInfo(params),
433+
"Invalid pageToken format: pageToken must be in 'timestamp_millis:configId' format");
437434
}
438435

439436
@Test
@@ -552,6 +549,47 @@ public void testPaginationOrderingConsistency() {
552549
assertEquals("cfg0", allConfigIds.get(14),
553550
"Last config should be oldest created");
554551
}
552+
553+
@Test
554+
@Transactional
555+
public void testPageSizeExceedingConfiguredMaxLimit() {
556+
String taskId = "task_max_page_size_" + System.currentTimeMillis();
557+
// Create 5 configs (more than test max page size of 5)
558+
createSamples(taskId, 7);
559+
560+
// Request with pageSize=7 (exceeds configured max of 5 in test application.properties)
561+
// Should be capped to maxPageSize (5) from config
562+
ListTaskPushNotificationConfigParams params =
563+
new ListTaskPushNotificationConfigParams(taskId, 7, "", "");
564+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
565+
566+
assertNotNull(result);
567+
// Should return 5 configs (capped to maxPageSize from test config), not 7
568+
assertEquals(5, result.configs().size(),
569+
"Page size should be capped to configured maxPageSize (5 in tests) when requested size exceeds limit");
570+
assertNotNull(result.nextPageToken(),
571+
"Should have nextPageToken since more configs remain");
572+
573+
// Verify we can iterate through all pages and get all 7 configs
574+
List<String> allConfigIds = new java.util.ArrayList<>();
575+
result.configs().forEach(c -> allConfigIds.add(c.pushNotificationConfig().id()));
576+
577+
String nextToken = result.nextPageToken();
578+
while (nextToken != null) {
579+
ListTaskPushNotificationConfigParams nextParams =
580+
new ListTaskPushNotificationConfigParams(taskId, 7, nextToken, "");
581+
ListTaskPushNotificationConfigResult nextResult = pushNotificationConfigStore.getInfo(nextParams);
582+
583+
nextResult.configs().forEach(c -> allConfigIds.add(c.pushNotificationConfig().id()));
584+
nextToken = nextResult.nextPageToken();
585+
}
586+
587+
assertEquals(7, allConfigIds.size(),
588+
"Should retrieve all 7 configs across multiple pages");
589+
assertEquals(7, new java.util.HashSet<>(allConfigIds).size(),
590+
"All config IDs should be unique - no duplicates");
591+
}
592+
555593
private void createSamples(String taskId, int size) {
556594
// Create configs with slight delays to ensure unique timestamps for deterministic ordering
557595
for (int i = 0; i < size; i++) {
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# A2A SDK Default Configuration Values
2+
# These values are used when no other configuration source provides them
3+
4+
# DefaultRequestHandler - Blocking call timeouts
5+
# Timeout for agent execution to complete (seconds)
6+
# Increase for slow agents: LLM-based, data processing, external APIs
7+
a2a.blocking.agent.timeout.seconds=30
8+
9+
# Timeout for event consumption/persistence to complete (seconds)
10+
# Ensures TaskStore is fully updated before returning to client
11+
a2a.blocking.consumption.timeout.seconds=5
12+
13+
# AsyncExecutorProducer - Thread pool configuration
14+
# Core pool size for async agent execution
15+
a2a.executor.core-pool-size=5
16+
17+
# Maximum pool size for async agent execution
18+
a2a.executor.max-pool-size=50
19+
20+
# Keep-alive time for idle threads (seconds)
21+
a2a.executor.keep-alive-seconds=60
22+
23+
# A2A JPA Database Push Notification Config Store Default Configuration
24+
25+
# A2A Configuration - Override max page size for testing
26+
# Set to a lower value (2) to make tests faster and verify capping behavior
27+
a2a.push-notification-config.max-page-size=5
28+

extras/push-notification-config-store-database-jpa/src/test/resources/application.properties

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ quarkus.hibernate-orm.log.format-sql=true
1212

1313
# Transaction timeout (set to 30 minutes for debugging - 1800 seconds)
1414
# quarkus.transaction-manager.default-transaction-timeout=1800s
15+
a2a.defaults.resource=META-INF/a2a-test-defaults.properties

server-common/pom.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@
6262
<dependency>
6363
<groupId>io.quarkus</groupId>
6464
<artifactId>quarkus-arc</artifactId>
65-
<scope>test</scope>
6665
<exclusions>
6766
<exclusion>
6867
<groupId>org.jboss.logging</groupId>

server-common/src/main/java/io/a2a/server/config/DefaultValuesConfigProvider.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import jakarta.annotation.PostConstruct;
1313
import jakarta.enterprise.context.ApplicationScoped;
1414

15+
import org.eclipse.microprofile.config.inject.ConfigProperty;
1516
import org.slf4j.Logger;
1617
import org.slf4j.LoggerFactory;
1718

@@ -31,6 +32,9 @@ public class DefaultValuesConfigProvider implements A2AConfigProvider {
3132
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultValuesConfigProvider.class);
3233
private static final String DEFAULTS_RESOURCE = "META-INF/a2a-defaults.properties";
3334

35+
@ConfigProperty(name = "a2a.defaults.resource", defaultValue = DEFAULTS_RESOURCE)
36+
String defaultsResource = DEFAULTS_RESOURCE;
37+
3438
private final Map<String, String> defaults = new HashMap<>();
3539

3640
@PostConstruct
@@ -42,7 +46,7 @@ private void loadDefaultsFromClasspath() {
4246
try {
4347
Enumeration<URL> resources = Thread.currentThread()
4448
.getContextClassLoader()
45-
.getResources(DEFAULTS_RESOURCE);
49+
.getResources(defaultsResource);
4650

4751
Map<String, String> sourceTracker = new HashMap<>(); // Track which file each key came from
4852

spec/src/main/java/io/a2a/spec/ListTaskPushNotificationConfigParams.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,15 @@ public ListTaskPushNotificationConfigParams(String id) {
3939
}
4040

4141
/**
42-
* Validates and returns the effective page size (between 1 and 100, defaults to 100).
42+
* Validates and returns the effective page size.
43+
* If the requested pageSize is invalid (≤ 0 or > maxPageSize), returns maxPageSize.
4344
*
44-
* @return the effective page size
45+
* @param maxPageSize the maximum allowed page size
46+
* @return the effective page size (between 1 and maxPageSize)
4547
*/
46-
public int getEffectivePageSize() {
47-
if (pageSize <= 0 || pageSize > 100) {
48-
return 100;
48+
public int getEffectivePageSize(int maxPageSize) {
49+
if (pageSize <= 0 || pageSize > maxPageSize) {
50+
return maxPageSize;
4951
}
5052
return pageSize;
5153
}

0 commit comments

Comments
 (0)