Skip to content

Commit 9b8e34e

Browse files
committed
fix: Update PushNotificationConfigStore implementations to use new paginated API
Updated all PushNotificationConfigStore implementations and tests to use the new ListTaskPushNotificationConfigParams/ListTaskPushNotificationConfigResult API instead of the old String-based getInfo method. Changes to implementations: - InMemoryPushNotificationConfigStore: Fixed bug where pageSize - 1 was used instead of pageSize, causing one less item to be returned than requested - JpaDatabasePushNotificationConfigStore: Implemented pagination logic matching the in-memory implementation - Added helper methods for pagination: findFirstIndex, convertPushNotificationConfig - Updated all test files with getInfoAsList helper method to maintain compatibility Comprehensive pagination tests added (20 new tests total): - InMemoryPushNotificationConfigStoreTest: 10 pagination tests - JpaDatabasePushNotificationConfigStoreIntegrationTest: 10 pagination tests Test coverage includes: - Basic pagination with pageSize - Multi-page navigation using pageToken - Last page handling (no nextPageToken) - Edge cases: zero/negative pageSize, pageSize larger than available items - Exact pageSize match - Invalid token handling - Empty task pagination - Full iteration across all pages - Explicit overlap detection (verifies no config appears in multiple pages) Signed-off-by: Emmanuel Hugonnet <[email protected]>
1 parent 2334c36 commit 9b8e34e

File tree

10 files changed

+686
-75
lines changed

10 files changed

+686
-75
lines changed

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

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

3+
import java.util.ArrayList;
4+
import java.util.Collections;
35
import java.util.List;
46

57
import jakarta.annotation.Priority;
@@ -11,7 +13,10 @@
1113

1214
import io.a2a.json.JsonProcessingException;
1315
import io.a2a.server.tasks.PushNotificationConfigStore;
16+
import io.a2a.spec.ListTaskPushNotificationConfigParams;
17+
import io.a2a.spec.ListTaskPushNotificationConfigResult;
1418
import io.a2a.spec.PushNotificationConfig;
19+
import io.a2a.spec.TaskPushNotificationConfig;
1520
import org.slf4j.Logger;
1621
import org.slf4j.LoggerFactory;
1722

@@ -65,7 +70,8 @@ public PushNotificationConfig setInfo(String taskId, PushNotificationConfig noti
6570

6671
@Transactional
6772
@Override
68-
public List<PushNotificationConfig> getInfo(String taskId) {
73+
public ListTaskPushNotificationConfigResult getInfo(ListTaskPushNotificationConfigParams params) {
74+
String taskId = params.id();
6975
LOGGER.debug("Retrieving PushNotificationConfigs for Task '{}'", taskId);
7076
try {
7177
List<JpaPushNotificationConfig> jpaConfigs = em.createQuery(
@@ -88,13 +94,58 @@ public List<PushNotificationConfig> getInfo(String taskId) {
8894
.toList();
8995

9096
LOGGER.debug("Successfully retrieved {} PushNotificationConfigs for Task '{}'", configs.size(), taskId);
91-
return configs;
97+
98+
// Handle pagination
99+
if (configs.isEmpty()) {
100+
return new ListTaskPushNotificationConfigResult(Collections.emptyList());
101+
}
102+
103+
if (params.pageSize() <= 0) {
104+
return new ListTaskPushNotificationConfigResult(convertPushNotificationConfig(configs, params), null);
105+
}
106+
107+
// Apply pageToken filtering if provided
108+
List<PushNotificationConfig> paginatedConfigs = configs;
109+
if (params.pageToken() != null && !params.pageToken().isBlank()) {
110+
int index = findFirstIndex(configs, params.pageToken());
111+
if (index < configs.size()) {
112+
paginatedConfigs = configs.subList(index, configs.size());
113+
}
114+
}
115+
116+
// Apply page size limit
117+
if (paginatedConfigs.size() <= params.pageSize()) {
118+
return new ListTaskPushNotificationConfigResult(convertPushNotificationConfig(paginatedConfigs, params), null);
119+
}
120+
121+
String nextToken = paginatedConfigs.get(params.pageSize()).token();
122+
return new ListTaskPushNotificationConfigResult(
123+
convertPushNotificationConfig(paginatedConfigs.subList(0, params.pageSize()), params),
124+
nextToken);
92125
} catch (Exception e) {
93126
LOGGER.error("Failed to retrieve PushNotificationConfigs for Task '{}'", taskId, e);
94127
throw e;
95128
}
96129
}
97130

131+
private int findFirstIndex(List<PushNotificationConfig> configs, String token) {
132+
for (int i = 0; i < configs.size(); i++) {
133+
if (token.equals(configs.get(i).token())) {
134+
return i;
135+
}
136+
}
137+
return configs.size();
138+
}
139+
140+
private List<TaskPushNotificationConfig> convertPushNotificationConfig(List<PushNotificationConfig> pushNotificationConfigList, ListTaskPushNotificationConfigParams params) {
141+
List<TaskPushNotificationConfig> taskPushNotificationConfigList = new ArrayList<>(pushNotificationConfigList.size());
142+
for (PushNotificationConfig pushNotificationConfig : pushNotificationConfigList) {
143+
TaskPushNotificationConfig taskPushNotificationConfig = new TaskPushNotificationConfig(params.id(), pushNotificationConfig, params.tenant());
144+
taskPushNotificationConfigList.add(taskPushNotificationConfig);
145+
}
146+
return taskPushNotificationConfigList;
147+
}
148+
98149
@Transactional
99150
@Override
100151
public void deleteInfo(String taskId, String configId) {

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

Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
import static org.junit.jupiter.api.Assertions.assertEquals;
44
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
55
import static org.junit.jupiter.api.Assertions.assertNotNull;
6+
import static org.junit.jupiter.api.Assertions.assertNull;
67
import static org.junit.jupiter.api.Assertions.assertThrows;
78
import static org.junit.jupiter.api.Assertions.assertTrue;
89

910
import jakarta.inject.Inject;
11+
import jakarta.transaction.Transactional;
1012

1113
import java.util.List;
1214
import java.util.Queue;
@@ -23,6 +25,8 @@
2325
import io.a2a.spec.AgentCard;
2426
import io.a2a.spec.DeleteTaskPushNotificationConfigParams;
2527
import io.a2a.spec.GetTaskPushNotificationConfigParams;
28+
import io.a2a.spec.ListTaskPushNotificationConfigParams;
29+
import io.a2a.spec.ListTaskPushNotificationConfigResult;
2630
import io.a2a.spec.Message;
2731
import io.a2a.spec.PushNotificationConfig;
2832
import io.a2a.spec.Task;
@@ -182,4 +186,258 @@ public void testJpaDatabasePushNotificationConfigStoreIntegration() throws Excep
182186
client.getTaskPushNotificationConfiguration(new GetTaskPushNotificationConfigParams(taskId));
183187
}, "Getting a deleted config should throw an A2AClientException");
184188
}
189+
190+
private PushNotificationConfig createSamplePushConfig(String url, String configId, String token) {
191+
return PushNotificationConfig.builder()
192+
.url(url)
193+
.id(configId)
194+
.token(token)
195+
.build();
196+
}
197+
198+
@Test
199+
@Transactional
200+
public void testPaginationWithPageSize() {
201+
String taskId = "task_pagination_" + System.currentTimeMillis();
202+
// Create 5 configs
203+
for (int i = 0; i < 5; i++) {
204+
PushNotificationConfig config = createSamplePushConfig(
205+
"http://url" + i + ".com/callback", "cfg" + i, "token" + i);
206+
pushNotificationConfigStore.setInfo(taskId, config);
207+
}
208+
209+
// Request first page with pageSize=2
210+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, 2, "", "");
211+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
212+
213+
assertNotNull(result);
214+
assertEquals(2, result.configs().size(), "Should return 2 configs");
215+
assertNotNull(result.nextPageToken(), "Should have nextPageToken when more items exist");
216+
}
217+
218+
@Test
219+
@Transactional
220+
public void testPaginationWithPageToken() {
221+
String taskId = "task_pagination_token_" + System.currentTimeMillis();
222+
// Create 5 configs
223+
for (int i = 0; i < 5; i++) {
224+
PushNotificationConfig config = createSamplePushConfig(
225+
"http://url" + i + ".com/callback", "cfg" + i, "token" + i);
226+
pushNotificationConfigStore.setInfo(taskId, config);
227+
}
228+
229+
// Get first page
230+
ListTaskPushNotificationConfigParams firstPageParams = new ListTaskPushNotificationConfigParams(taskId, 2, "", "");
231+
ListTaskPushNotificationConfigResult firstPage = pushNotificationConfigStore.getInfo(firstPageParams);
232+
assertNotNull(firstPage.nextPageToken());
233+
234+
// Get second page using nextPageToken
235+
ListTaskPushNotificationConfigParams secondPageParams = new ListTaskPushNotificationConfigParams(
236+
taskId, 2, firstPage.nextPageToken(), "");
237+
ListTaskPushNotificationConfigResult secondPage = pushNotificationConfigStore.getInfo(secondPageParams);
238+
239+
assertNotNull(secondPage);
240+
assertEquals(2, secondPage.configs().size(), "Should return 2 configs for second page");
241+
assertNotNull(secondPage.nextPageToken(), "Should have nextPageToken when more items exist");
242+
243+
// Verify NO overlap between pages - collect all IDs from both pages
244+
List<String> firstPageIds = firstPage.configs().stream()
245+
.map(c -> c.pushNotificationConfig().id())
246+
.toList();
247+
List<String> secondPageIds = secondPage.configs().stream()
248+
.map(c -> c.pushNotificationConfig().id())
249+
.toList();
250+
251+
// Check that no ID from first page appears in second page
252+
for (String id : firstPageIds) {
253+
assertTrue(!secondPageIds.contains(id),
254+
"Config " + id + " appears in both pages - overlap detected!");
255+
}
256+
257+
// Also verify the pages are sequential (first page ends before second page starts)
258+
// Since configs are created in order, we can verify the IDs
259+
assertEquals("cfg0", firstPageIds.get(0));
260+
assertEquals("cfg1", firstPageIds.get(1));
261+
assertEquals("cfg2", secondPageIds.get(0));
262+
assertEquals("cfg3", secondPageIds.get(1));
263+
}
264+
265+
@Test
266+
@Transactional
267+
public void testPaginationLastPage() {
268+
String taskId = "task_pagination_last_" + System.currentTimeMillis();
269+
// Create 5 configs
270+
for (int i = 0; i < 5; i++) {
271+
PushNotificationConfig config = createSamplePushConfig(
272+
"http://url" + i + ".com/callback", "cfg" + i, "token" + i);
273+
pushNotificationConfigStore.setInfo(taskId, config);
274+
}
275+
276+
// Get first page (2 items)
277+
ListTaskPushNotificationConfigParams firstPageParams = new ListTaskPushNotificationConfigParams(taskId, 2, "", "");
278+
ListTaskPushNotificationConfigResult firstPage = pushNotificationConfigStore.getInfo(firstPageParams);
279+
280+
// Get second page (2 items)
281+
ListTaskPushNotificationConfigParams secondPageParams = new ListTaskPushNotificationConfigParams(
282+
taskId, 2, firstPage.nextPageToken(), "");
283+
ListTaskPushNotificationConfigResult secondPage = pushNotificationConfigStore.getInfo(secondPageParams);
284+
285+
// Get last page (1 item remaining)
286+
ListTaskPushNotificationConfigParams lastPageParams = new ListTaskPushNotificationConfigParams(
287+
taskId, 2, secondPage.nextPageToken(), "");
288+
ListTaskPushNotificationConfigResult lastPage = pushNotificationConfigStore.getInfo(lastPageParams);
289+
290+
assertNotNull(lastPage);
291+
assertEquals(1, lastPage.configs().size(), "Last page should have 1 remaining config");
292+
assertNull(lastPage.nextPageToken(), "Last page should not have nextPageToken");
293+
}
294+
295+
@Test
296+
@Transactional
297+
public void testPaginationWithZeroPageSize() {
298+
String taskId = "task_pagination_zero_" + System.currentTimeMillis();
299+
// Create 5 configs
300+
for (int i = 0; i < 5; i++) {
301+
PushNotificationConfig config = createSamplePushConfig(
302+
"http://url" + i + ".com/callback", "cfg" + i, "token" + i);
303+
pushNotificationConfigStore.setInfo(taskId, config);
304+
}
305+
306+
// Request with pageSize=0 should return all configs
307+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, 0, "", "");
308+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
309+
310+
assertNotNull(result);
311+
assertEquals(5, result.configs().size(), "Should return all 5 configs when pageSize=0");
312+
assertNull(result.nextPageToken(), "Should not have nextPageToken when returning all");
313+
}
314+
315+
@Test
316+
@Transactional
317+
public void testPaginationWithNegativePageSize() {
318+
String taskId = "task_pagination_negative_" + System.currentTimeMillis();
319+
// Create 3 configs
320+
for (int i = 0; i < 3; i++) {
321+
PushNotificationConfig config = createSamplePushConfig(
322+
"http://url" + i + ".com/callback", "cfg" + i, "token" + i);
323+
pushNotificationConfigStore.setInfo(taskId, config);
324+
}
325+
326+
// Request with negative pageSize should return all configs
327+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, -1, "", "");
328+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
329+
330+
assertNotNull(result);
331+
assertEquals(3, result.configs().size(), "Should return all configs when pageSize is negative");
332+
assertNull(result.nextPageToken(), "Should not have nextPageToken when returning all");
333+
}
334+
335+
@Test
336+
@Transactional
337+
public void testPaginationPageSizeLargerThanConfigs() {
338+
String taskId = "task_pagination_large_" + System.currentTimeMillis();
339+
// Create 3 configs
340+
for (int i = 0; i < 3; i++) {
341+
PushNotificationConfig config = createSamplePushConfig(
342+
"http://url" + i + ".com/callback", "cfg" + i, "token" + i);
343+
pushNotificationConfigStore.setInfo(taskId, config);
344+
}
345+
346+
// Request with pageSize larger than available configs
347+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, 10, "", "");
348+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
349+
350+
assertNotNull(result);
351+
assertEquals(3, result.configs().size(), "Should return all 3 configs");
352+
assertNull(result.nextPageToken(), "Should not have nextPageToken when all configs fit in one page");
353+
}
354+
355+
@Test
356+
@Transactional
357+
public void testPaginationExactlyPageSize() {
358+
String taskId = "task_pagination_exact_" + System.currentTimeMillis();
359+
// Create exactly 3 configs
360+
for (int i = 0; i < 3; i++) {
361+
PushNotificationConfig config = createSamplePushConfig(
362+
"http://url" + i + ".com/callback", "cfg" + i, "token" + i);
363+
pushNotificationConfigStore.setInfo(taskId, config);
364+
}
365+
366+
// Request with pageSize equal to number of configs
367+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, 3, "", "");
368+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
369+
370+
assertNotNull(result);
371+
assertEquals(3, result.configs().size(), "Should return all 3 configs");
372+
assertNull(result.nextPageToken(), "Should not have nextPageToken when configs exactly match pageSize");
373+
}
374+
375+
@Test
376+
@Transactional
377+
public void testPaginationWithInvalidToken() {
378+
String taskId = "task_pagination_invalid_token_" + System.currentTimeMillis();
379+
// Create 5 configs
380+
for (int i = 0; i < 5; i++) {
381+
PushNotificationConfig config = createSamplePushConfig(
382+
"http://url" + i + ".com/callback", "cfg" + i, "token" + i);
383+
pushNotificationConfigStore.setInfo(taskId, config);
384+
}
385+
386+
// Request with invalid pageToken - JPA implementation behavior is to start from beginning
387+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(
388+
taskId, 2, "invalid_token_that_does_not_exist", "");
389+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
390+
391+
assertNotNull(result);
392+
// When token is not found, implementation starts from beginning
393+
assertEquals(2, result.configs().size(), "Should return first page when token is not found");
394+
assertNotNull(result.nextPageToken(), "Should have nextPageToken since more items exist");
395+
}
396+
397+
@Test
398+
@Transactional
399+
public void testPaginationEmptyTaskWithPageSize() {
400+
String taskId = "task_pagination_empty_" + System.currentTimeMillis();
401+
// No configs created
402+
403+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, 2, "", "");
404+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
405+
406+
assertNotNull(result);
407+
assertTrue(result.configs().isEmpty(), "Should return empty list for non-existent task");
408+
assertNull(result.nextPageToken(), "Should not have nextPageToken for empty result");
409+
}
410+
411+
@Test
412+
@Transactional
413+
public void testPaginationFullIteration() {
414+
String taskId = "task_pagination_full_" + System.currentTimeMillis();
415+
// Create 7 configs
416+
for (int i = 0; i < 7; i++) {
417+
PushNotificationConfig config = createSamplePushConfig(
418+
"http://url" + i + ".com/callback", "cfg" + i, "token" + i);
419+
pushNotificationConfigStore.setInfo(taskId, config);
420+
}
421+
422+
// Iterate through all pages with pageSize=3
423+
int totalCollected = 0;
424+
String pageToken = "";
425+
int pageCount = 0;
426+
427+
do {
428+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, 3, pageToken, "");
429+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
430+
431+
totalCollected += result.configs().size();
432+
pageToken = result.nextPageToken();
433+
pageCount++;
434+
435+
// Safety check to prevent infinite loop
436+
assertTrue(pageCount <= 10, "Should not have more than 10 pages for 7 configs");
437+
438+
} while (pageToken != null);
439+
440+
assertEquals(7, totalCollected, "Should collect all 7 configs across all pages");
441+
assertEquals(3, pageCount, "Should have exactly 3 pages (3+3+1)");
442+
}
185443
}

0 commit comments

Comments
 (0)