From bf06b6d1ef2984e2fa4c44e1b18cd9d931841b1b Mon Sep 17 00:00:00 2001 From: Phellippe Lima Date: Mon, 7 Oct 2024 14:25:04 +0100 Subject: [PATCH] feature: paginator tweaks and unit tests --- core/client/README.md | 4 +- core/client/build.gradle | 1 + .../sdk/pagination/Paginator.java | 72 ++++---- .../{ => client}/RateLimitDecoratorTest.java | 8 +- .../sdk/pagination/PaginatorTest.java | 170 ++++++++++++++++++ .../resources/pagination/empty-next-link.json | 35 ++++ .../resources/pagination/empty-response.json | 8 + .../pagination/missing-next-link-cursor.json | 36 ++++ .../resources/pagination/no-next-link.json | 39 ++++ .../resources/pagination/with-next-link.json | 36 ++++ 10 files changed, 367 insertions(+), 42 deletions(-) rename core/client/src/test/java/com/thousandeyes/sdk/{ => client}/RateLimitDecoratorTest.java (92%) create mode 100644 core/client/src/test/java/com/thousandeyes/sdk/pagination/PaginatorTest.java create mode 100644 core/client/src/test/resources/pagination/empty-next-link.json create mode 100644 core/client/src/test/resources/pagination/empty-response.json create mode 100644 core/client/src/test/resources/pagination/missing-next-link-cursor.json create mode 100644 core/client/src/test/resources/pagination/no-next-link.json create mode 100644 core/client/src/test/resources/pagination/with-next-link.json diff --git a/core/client/README.md b/core/client/README.md index f1774347a..864b7bfaa 100644 --- a/core/client/README.md +++ b/core/client/README.md @@ -13,7 +13,7 @@ waiting the appropriate amount of time. E.g: class Example { private static final ApiClient apiClient = NativeApiClient .builder() - .baseUri("https://api.stg.thousandeyes.com") + .baseUri("https://api.thousandeyes.com") .bearerToken("") .build(); @@ -50,7 +50,7 @@ import java.util.stream.Collectors; class Example { private static final ApiClient apiClient = NativeApiClient .builder() - .baseUri("https://api.stg.thousandeyes.com") + .baseUri("https://api.thousandeyes.com") .bearerToken("") .build(); diff --git a/core/client/build.gradle b/core/client/build.gradle index a7bea5a12..d5fe0d6c6 100644 --- a/core/client/build.gradle +++ b/core/client/build.gradle @@ -19,6 +19,7 @@ dependencies { annotationProcessor "org.projectlombok:lombok:1.18.30" testAnnotationProcessor "org.projectlombok:lombok:1.18.30" + testImplementation project(":administrative") testImplementation "org.junit.jupiter:junit-jupiter:$junitVersion" testImplementation "org.mockito:mockito-core:$mockitoVersion" } diff --git a/core/client/src/main/java/com/thousandeyes/sdk/pagination/Paginator.java b/core/client/src/main/java/com/thousandeyes/sdk/pagination/Paginator.java index 8549215b8..69e262348 100644 --- a/core/client/src/main/java/com/thousandeyes/sdk/pagination/Paginator.java +++ b/core/client/src/main/java/com/thousandeyes/sdk/pagination/Paginator.java @@ -26,20 +26,8 @@ public Paginator(PaginatedApiCall apiCall, Function> dataExtractor this.dataExtractor = dataExtractor; } - private String extractCursor(String href) { - if (href == null) { - return null; - } - - var matcher = CURSOR_PATTERN.matcher(href); - if (matcher.find()) { - return URLDecoder.decode(matcher.group(1), StandardCharsets.UTF_8); - } - - return null; - } - - @Override public Iterator iterator() { + @Override + public Iterator iterator() { return new PaginatorIterator(); } @@ -49,17 +37,15 @@ public Stream stream() { private class PaginatorIterator implements Iterator { private String cursor = null; - private Iterator currentBatchIterator = null; - private boolean hasNextBatch = true; + private Iterator currentPageIterator = null; + private boolean hasNextPage = true; @Override public boolean hasNext() { - if (currentBatchIterator == null || !currentBatchIterator.hasNext()) { - if (hasNextBatch) { - fetchNextBatch(); - } + if (!currentPageHasNext() && hasNextPage) { + fetchNextPage(); } - return currentBatchIterator != null && currentBatchIterator.hasNext(); + return currentPageHasNext(); } @Override @@ -67,14 +53,18 @@ public T next() { if (!hasNext()) { throw new NoSuchElementException(); } - return currentBatchIterator.next(); + return currentPageIterator.next(); + } + + private boolean currentPageHasNext() { + return currentPageIterator != null && currentPageIterator.hasNext(); } - private void fetchNextBatch() { + private void fetchNextPage() { try { R result = apiCall.call(cursor); - List currentBatch = dataExtractor.apply(result); - currentBatchIterator = currentBatch.iterator(); + List currentPage = dataExtractor.apply(result); + currentPageIterator = currentPage.iterator(); var clazz = result.getClass(); var getLinks = clazz.getMethod("getLinks"); @@ -83,19 +73,35 @@ private void fetchNextBatch() { var getNext = links.getClass().getMethod("getNext"); var next = getNext.invoke(links); - if (next != null) { - var getHref = next.getClass().getMethod("getHref"); - String nextHref = (String) getHref.invoke(next); - cursor = extractCursor(nextHref); - } - else { - hasNextBatch = false; + cursor = extractCursor(next); + if (cursor == null) { + hasNextPage = false; } } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException | ApiException e) { - throw new RuntimeException("Error fetching next batch", e); + throw new RuntimeException("Error fetching next page", e); + } + } + + private String extractCursor(Object next) + throws InvocationTargetException, IllegalAccessException, NoSuchMethodException + { + if (next != null) { + var getHref = next.getClass().getMethod("getHref"); + String href = (String) getHref.invoke(next); + + if (href == null) { + return null; + } + + var matcher = CURSOR_PATTERN.matcher(href); + if (matcher.find()) { + return URLDecoder.decode(matcher.group(1), StandardCharsets.UTF_8); + } } + return null; } } + } diff --git a/core/client/src/test/java/com/thousandeyes/sdk/RateLimitDecoratorTest.java b/core/client/src/test/java/com/thousandeyes/sdk/client/RateLimitDecoratorTest.java similarity index 92% rename from core/client/src/test/java/com/thousandeyes/sdk/RateLimitDecoratorTest.java rename to core/client/src/test/java/com/thousandeyes/sdk/client/RateLimitDecoratorTest.java index 75264a090..f707e7ab5 100644 --- a/core/client/src/test/java/com/thousandeyes/sdk/RateLimitDecoratorTest.java +++ b/core/client/src/test/java/com/thousandeyes/sdk/client/RateLimitDecoratorTest.java @@ -16,7 +16,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package com.thousandeyes.sdk; +package com.thousandeyes.sdk.client; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -36,12 +36,6 @@ import org.mockito.Mock; import org.mockito.Mockito; -import com.thousandeyes.sdk.client.ApiClient; -import com.thousandeyes.sdk.client.ApiException; -import com.thousandeyes.sdk.client.ApiRequest; -import com.thousandeyes.sdk.client.ApiResponse; -import com.thousandeyes.sdk.client.RateLimitDecorator; - public class RateLimitDecoratorTest { diff --git a/core/client/src/test/java/com/thousandeyes/sdk/pagination/PaginatorTest.java b/core/client/src/test/java/com/thousandeyes/sdk/pagination/PaginatorTest.java new file mode 100644 index 000000000..267843681 --- /dev/null +++ b/core/client/src/test/java/com/thousandeyes/sdk/pagination/PaginatorTest.java @@ -0,0 +1,170 @@ +package com.thousandeyes.sdk.pagination; + +import static com.thousandeyes.sdk.serialization.JSON.getDefault; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.NoSuchElementException; +import java.util.stream.Stream; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.Mockito; + +import com.fasterxml.jackson.databind.ObjectMapper; + +import com.thousandeyes.sdk.account.management.administrative.UserEventsApi; +import com.thousandeyes.sdk.account.management.administrative.model.AuditUserEvents; +import com.thousandeyes.sdk.account.management.administrative.model.UserEvent; +import com.thousandeyes.sdk.client.ApiException; + + + +public class PaginatorTest { + + private static UserEventsApi api; + private static AuditUserEvents withNextLink = null; + private static AuditUserEvents noNextLink = null; + private static final ObjectMapper mapper = getDefault().getMapper(); + + @BeforeAll + public static void setup() throws IOException { + var withNextLinkJson = readJson("with-next-link.json"); + var noNextLinkJson = readJson("no-next-link.json"); + + withNextLink = mapper.readValue(withNextLinkJson, AuditUserEvents.class); + noNextLink = mapper.readValue(noNextLinkJson, AuditUserEvents.class); + } + + @BeforeEach + public void clear() { + api = Mockito.mock(UserEventsApi.class); + } + + @Test + void shouldCallNextLinkWhenNextLinkExists() throws ApiException { + var aid = "1"; + var window = "1w"; + var cursor = "b2Zmc2V0PTUwMTIzMTc5"; + mockApiResponse(withNextLink, aid, window, null); + mockApiResponse(noNextLink, aid, window, cursor); + + var paginator = buildPaginator(aid, window); + var elements = paginator.stream().toList(); + + assertEquals(4, elements.size()); + verify(api).getUserEvents(aid, false, window, null, null, null); + verify(api).getUserEvents(aid, false, window, null, null, cursor); + verifyNoMoreInteractions(api); + } + + @ParameterizedTest + @MethodSource("provideNoNextLinkResponses") + void shouldNotMakeExtraCallsWhenThereIsNoNextLink(AuditUserEvents response) + throws ApiException + { + var aid = "2"; + var window = "2d"; + mockApiResponse(response, aid, window, null); + + var paginator = buildPaginator(aid, window); + var elements = paginator.stream().toList(); + + assertEquals(2, elements.size()); + verify(api).getUserEvents(aid, false, window, null, null, null); + verifyNoMoreInteractions(api); + } + + @Test + void shouldPropagateExceptionWhenApiThrowsException() throws ApiException { + var aid = "3"; + var window = "3h"; + doThrow(ApiException.class) + .when(api) + .getUserEvents(aid, false, window, null, null, null); + + var paginator = buildPaginator(aid, window); + var exception = assertThrows(RuntimeException.class, () -> paginator.iterator().next()); + + assertEquals("Error fetching next page", exception.getMessage()); + assertEquals(ApiException.class, exception.getCause().getClass()); + } + + @Test + void shouldReturnNoElementsWhenEmptyResponse() + throws ApiException, IOException + { + var aid = "4"; + var window = "4m"; + var emptyResponseJson = readJson("empty-response.json"); + var emptyResponse = mapper.readValue(emptyResponseJson, AuditUserEvents.class); + mockApiResponse(emptyResponse, aid, window, null); + + var paginator = buildPaginator(aid, window); + var elements = paginator.stream().toList(); + assertEquals(0, elements.size()); + verify(api).getUserEvents(aid, false, window, null, null, null); + verifyNoMoreInteractions(api); + } + + @Test + void shouldThrowNoSuchElementExceptionWhenNoNextElement() + throws ApiException, IOException + { + var aid = "5"; + var window = "5s"; + var emptyResponseJson = readJson("empty-response.json"); + var emptyResponse = mapper.readValue(emptyResponseJson, AuditUserEvents.class); + mockApiResponse(emptyResponse, aid, window, null); + + var paginator = buildPaginator(aid, window); + var iterator = paginator.iterator(); + + assertThrows(NoSuchElementException.class, iterator::next); + verify(api).getUserEvents(aid, false, window, null, null, null); + verifyNoMoreInteractions(api); + } + + private static Stream provideNoNextLinkResponses() throws IOException { + var emptyNextLinkJson = readJson("empty-next-link.json"); + var missingNextLinkCursorJson = readJson("missing-next-link-cursor.json"); + + var emptyNextLink = mapper.readValue(emptyNextLinkJson, AuditUserEvents.class); + var missingNextLinkCursor = + mapper.readValue(missingNextLinkCursorJson, AuditUserEvents.class); + + return Stream.of(noNextLink, emptyNextLink, missingNextLinkCursor); + } + + private Paginator buildPaginator(String aid, String window) { + return new Paginator<>(cursor -> api.getUserEvents(aid, false, window, null, null, cursor), + AuditUserEvents::getAuditEvents); + } + + private void mockApiResponse(AuditUserEvents response, String aid, String window, String cursor) + throws ApiException + { + doReturn(response) + .when(api) + .getUserEvents(aid, false, window, null, null, cursor); + } + + private static String readJson(String fileName) throws IOException { + return Files.readString(buildResourcesPath(fileName)); + } + + private static Path buildResourcesPath(String filePath) { + return Paths.get("src", "test", "resources", "pagination", filePath); + } +} diff --git a/core/client/src/test/resources/pagination/empty-next-link.json b/core/client/src/test/resources/pagination/empty-next-link.json new file mode 100644 index 000000000..7cc6200dd --- /dev/null +++ b/core/client/src/test/resources/pagination/empty-next-link.json @@ -0,0 +1,35 @@ +{ + "auditEvents": [ + { + "accountGroupName": "group", + "aid": "1", + "event": "Login", + "date": "2024-10-04T08:43:59Z", + "ipAddress": "1.1.1.1", + "uid": "2", + "user": "User (user@user.com)", + "resources": [ + { + "type": "name", + "name": "User" + } + ] + }, + { + "accountGroupName": "group", + "aid": "1", + "event": "Logout", + "date": "2024-10-04T08:42:56Z", + "ipAddress": "1.1.1.1", + "uid": "2", + "user": "User (user@user.com)" + } + ], + "_links": { + "next": { + }, + "self": { + "href": "https://api.com/v7/audit-user-events" + } + } +} diff --git a/core/client/src/test/resources/pagination/empty-response.json b/core/client/src/test/resources/pagination/empty-response.json new file mode 100644 index 000000000..ef11a7e95 --- /dev/null +++ b/core/client/src/test/resources/pagination/empty-response.json @@ -0,0 +1,8 @@ +{ + "auditEvents": [], + "_links": { + "self": { + "href": "https://api.com/v7/audit-user-events" + } + } +} diff --git a/core/client/src/test/resources/pagination/missing-next-link-cursor.json b/core/client/src/test/resources/pagination/missing-next-link-cursor.json new file mode 100644 index 000000000..93af71ab4 --- /dev/null +++ b/core/client/src/test/resources/pagination/missing-next-link-cursor.json @@ -0,0 +1,36 @@ +{ + "auditEvents": [ + { + "accountGroupName": "group", + "aid": "1", + "event": "Login", + "date": "2024-10-04T08:43:59Z", + "ipAddress": "1.1.1.1", + "uid": "2", + "user": "User (user@user.com)", + "resources": [ + { + "type": "name", + "name": "User" + } + ] + }, + { + "accountGroupName": "group", + "aid": "1", + "event": "Logout", + "date": "2024-10-04T08:42:56Z", + "ipAddress": "1.1.1.1", + "uid": "2", + "user": "User (user@user.com)" + } + ], + "_links": { + "next": { + "href": "https://api.com/v7/audit-user-events" + }, + "self": { + "href": "https://api.com/v7/audit-user-events" + } + } +} diff --git a/core/client/src/test/resources/pagination/no-next-link.json b/core/client/src/test/resources/pagination/no-next-link.json new file mode 100644 index 000000000..0db2c6175 --- /dev/null +++ b/core/client/src/test/resources/pagination/no-next-link.json @@ -0,0 +1,39 @@ +{ + "auditEvents": [ + { + "accountGroupName": "group", + "aid": "1", + "event": "Login", + "date": "2024-10-04T08:42:38Z", + "ipAddress": "1.1.1.1", + "uid": "2", + "user": "User (user@user.com)", + "resources": [ + { + "type": "name", + "name": "User" + } + ] + }, + { + "accountGroupName": "group", + "aid": "1", + "event": "Login", + "date": "2024-10-03T07:20:05Z", + "ipAddress": "1.1.1.1", + "uid": "2", + "user": "User (user@user.com)", + "resources": [ + { + "type": "name", + "name": "User" + } + ] + } + ], + "_links": { + "self": { + "href": "https://api.com/v7/audit-user-events" + } + } +} diff --git a/core/client/src/test/resources/pagination/with-next-link.json b/core/client/src/test/resources/pagination/with-next-link.json new file mode 100644 index 000000000..32511be43 --- /dev/null +++ b/core/client/src/test/resources/pagination/with-next-link.json @@ -0,0 +1,36 @@ +{ + "auditEvents": [ + { + "accountGroupName": "group", + "aid": "1", + "event": "Login", + "date": "2024-10-04T08:43:59Z", + "ipAddress": "1.1.1.1", + "uid": "2", + "user": "User (user@user.com)", + "resources": [ + { + "type": "name", + "name": "User" + } + ] + }, + { + "accountGroupName": "group", + "aid": "1", + "event": "Logout", + "date": "2024-10-04T08:42:56Z", + "ipAddress": "1.1.1.1", + "uid": "2", + "user": "User (user@user.com)" + } + ], + "_links": { + "next": { + "href": "https://api.com/v7/audit-user-events?&cursor=b2Zmc2V0PTUwMTIzMTc5" + }, + "self": { + "href": "https://api.com/v7/audit-user-events" + } + } +}