Skip to content

Commit b741fd5

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 b741fd5

File tree

10 files changed

+699
-107
lines changed

10 files changed

+699
-107
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: 235 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,235 @@ 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+
createSamples(taskId, 5);
224+
225+
// Get first page
226+
ListTaskPushNotificationConfigParams firstPageParams = new ListTaskPushNotificationConfigParams(taskId, 2, "", "");
227+
ListTaskPushNotificationConfigResult firstPage = pushNotificationConfigStore.getInfo(firstPageParams);
228+
assertNotNull(firstPage.nextPageToken());
229+
230+
// Get second page using nextPageToken
231+
ListTaskPushNotificationConfigParams secondPageParams = new ListTaskPushNotificationConfigParams(
232+
taskId, 2, firstPage.nextPageToken(), "");
233+
ListTaskPushNotificationConfigResult secondPage = pushNotificationConfigStore.getInfo(secondPageParams);
234+
235+
assertNotNull(secondPage);
236+
assertEquals(2, secondPage.configs().size(), "Should return 2 configs for second page");
237+
assertNotNull(secondPage.nextPageToken(), "Should have nextPageToken when more items exist");
238+
239+
// Verify NO overlap between pages - collect all IDs from both pages
240+
List<String> firstPageIds = firstPage.configs().stream()
241+
.map(c -> c.pushNotificationConfig().id())
242+
.toList();
243+
List<String> secondPageIds = secondPage.configs().stream()
244+
.map(c -> c.pushNotificationConfig().id())
245+
.toList();
246+
247+
// Check that no ID from first page appears in second page
248+
for (String id : firstPageIds) {
249+
assertTrue(!secondPageIds.contains(id),
250+
"Config " + id + " appears in both pages - overlap detected!");
251+
}
252+
253+
// Also verify the pages are sequential (first page ends before second page starts)
254+
// Since configs are created in order, we can verify the IDs
255+
assertEquals("cfg0", firstPageIds.get(0));
256+
assertEquals("cfg1", firstPageIds.get(1));
257+
assertEquals("cfg2", secondPageIds.get(0));
258+
assertEquals("cfg3", secondPageIds.get(1));
259+
}
260+
261+
@Test
262+
@Transactional
263+
public void testPaginationLastPage() {
264+
String taskId = "task_pagination_last_" + System.currentTimeMillis();
265+
// Create 5 configs
266+
createSamples(taskId, 5);
267+
268+
// Get first page (2 items)
269+
ListTaskPushNotificationConfigParams firstPageParams = new ListTaskPushNotificationConfigParams(taskId, 2, "", "");
270+
ListTaskPushNotificationConfigResult firstPage = pushNotificationConfigStore.getInfo(firstPageParams);
271+
272+
// Get second page (2 items)
273+
ListTaskPushNotificationConfigParams secondPageParams = new ListTaskPushNotificationConfigParams(
274+
taskId, 2, firstPage.nextPageToken(), "");
275+
ListTaskPushNotificationConfigResult secondPage = pushNotificationConfigStore.getInfo(secondPageParams);
276+
277+
// Get last page (1 item remaining)
278+
ListTaskPushNotificationConfigParams lastPageParams = new ListTaskPushNotificationConfigParams(
279+
taskId, 2, secondPage.nextPageToken(), "");
280+
ListTaskPushNotificationConfigResult lastPage = pushNotificationConfigStore.getInfo(lastPageParams);
281+
282+
assertNotNull(lastPage);
283+
assertEquals(1, lastPage.configs().size(), "Last page should have 1 remaining config");
284+
assertNull(lastPage.nextPageToken(), "Last page should not have nextPageToken");
285+
}
286+
287+
@Test
288+
@Transactional
289+
public void testPaginationWithZeroPageSize() {
290+
String taskId = "task_pagination_zero_" + System.currentTimeMillis();
291+
// Create 5 configs
292+
createSamples(taskId, 5);
293+
294+
// Request with pageSize=0 should return all configs
295+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, 0, "", "");
296+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
297+
298+
assertNotNull(result);
299+
assertEquals(5, result.configs().size(), "Should return all 5 configs when pageSize=0");
300+
assertNull(result.nextPageToken(), "Should not have nextPageToken when returning all");
301+
}
302+
303+
@Test
304+
@Transactional
305+
public void testPaginationWithNegativePageSize() {
306+
String taskId = "task_pagination_negative_" + System.currentTimeMillis();
307+
// Create 3 configs
308+
createSamples(taskId, 3);
309+
310+
// Request with negative pageSize should return all configs
311+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, -1, "", "");
312+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
313+
314+
assertNotNull(result);
315+
assertEquals(3, result.configs().size(), "Should return all configs when pageSize is negative");
316+
assertNull(result.nextPageToken(), "Should not have nextPageToken when returning all");
317+
}
318+
319+
@Test
320+
@Transactional
321+
public void testPaginationPageSizeLargerThanConfigs() {
322+
String taskId = "task_pagination_large_" + System.currentTimeMillis();
323+
// Create 3 configs
324+
createSamples(taskId, 3);
325+
326+
// Request with pageSize larger than available configs
327+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, 10, "", "");
328+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
329+
330+
assertNotNull(result);
331+
assertEquals(3, result.configs().size(), "Should return all 3 configs");
332+
assertNull(result.nextPageToken(), "Should not have nextPageToken when all configs fit in one page");
333+
}
334+
335+
@Test
336+
@Transactional
337+
public void testPaginationExactlyPageSize() {
338+
String taskId = "task_pagination_exact_" + System.currentTimeMillis();
339+
// Create exactly 3 configs
340+
createSamples(taskId, 3);
341+
342+
// Request with pageSize equal to number of configs
343+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, 3, "", "");
344+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
345+
346+
assertNotNull(result);
347+
assertEquals(3, result.configs().size(), "Should return all 3 configs");
348+
assertNull(result.nextPageToken(), "Should not have nextPageToken when configs exactly match pageSize");
349+
}
350+
351+
@Test
352+
@Transactional
353+
public void testPaginationWithInvalidToken() {
354+
String taskId = "task_pagination_invalid_token_" + System.currentTimeMillis();
355+
// Create 5 configs
356+
createSamples(taskId, 5);
357+
358+
// Request with invalid pageToken - JPA implementation behavior is to start from beginning
359+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(
360+
taskId, 2, "invalid_token_that_does_not_exist", "");
361+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
362+
363+
assertNotNull(result);
364+
// When token is not found, implementation starts from beginning
365+
assertEquals(2, result.configs().size(), "Should return first page when token is not found");
366+
assertNotNull(result.nextPageToken(), "Should have nextPageToken since more items exist");
367+
}
368+
369+
@Test
370+
@Transactional
371+
public void testPaginationEmptyTaskWithPageSize() {
372+
String taskId = "task_pagination_empty_" + System.currentTimeMillis();
373+
// No configs created
374+
375+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, 2, "", "");
376+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
377+
378+
assertNotNull(result);
379+
assertTrue(result.configs().isEmpty(), "Should return empty list for non-existent task");
380+
assertNull(result.nextPageToken(), "Should not have nextPageToken for empty result");
381+
}
382+
383+
@Test
384+
@Transactional
385+
public void testPaginationFullIteration() {
386+
String taskId = "task_pagination_full_" + System.currentTimeMillis();
387+
// Create 7 configs
388+
createSamples(taskId, 7);
389+
390+
// Iterate through all pages with pageSize=3
391+
int totalCollected = 0;
392+
String pageToken = "";
393+
int pageCount = 0;
394+
395+
do {
396+
ListTaskPushNotificationConfigParams params = new ListTaskPushNotificationConfigParams(taskId, 3, pageToken, "");
397+
ListTaskPushNotificationConfigResult result = pushNotificationConfigStore.getInfo(params);
398+
399+
totalCollected += result.configs().size();
400+
pageToken = result.nextPageToken();
401+
pageCount++;
402+
403+
// Safety check to prevent infinite loop
404+
assertTrue(pageCount <= 7, "Should not have more than 7 pages for 7 configs");
405+
406+
} while (pageToken != null);
407+
408+
assertEquals(7, totalCollected, "Should collect all 7 configs across all pages");
409+
assertEquals(3, pageCount, "Should have exactly 3 pages (3+3+1)");
410+
}
411+
412+
private void createSamples(String taskId, int size) {
413+
// Create 7 configs
414+
for (int i = 0; i < size; i++) {
415+
PushNotificationConfig config = createSamplePushConfig(
416+
"http://url" + i + ".com/callback", "cfg" + i, "token" + i);
417+
pushNotificationConfigStore.setInfo(taskId, config);
418+
}
419+
}
185420
}

0 commit comments

Comments
 (0)