diff --git a/server/opensharing-server-core/src/main/java/io/opensharing/http/Pagination.java b/server/opensharing-server-core/src/main/java/io/opensharing/http/Pagination.java new file mode 100644 index 00000000..f625c97e --- /dev/null +++ b/server/opensharing-server-core/src/main/java/io/opensharing/http/Pagination.java @@ -0,0 +1,71 @@ +package io.opensharing.http; + +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.List; +import java.util.function.Function; + +/** + * Pages list endpoints. A page holds {@link #MAX_RESULTS} items when the request sets no + * maxResults or sets 0, and never more. A page token is an opaque encoding of the offset of the + * next page in the list's order, so that order must be total. + */ +public final class Pagination { + + public static final int MAX_RESULTS = 500; + + private static final String TOKEN_PREFIX = "os1:"; + + private Pagination() {} + + /** Rows {@code offset} to {@code offset + limit - 1} of a list. */ + @FunctionalInterface + public interface Slice { + List fetch(int offset, int limit); + } + + /** One page of {@code slice} starting at {@code pageToken}. */ + public static ListResponse page( + Integer maxResults, String pageToken, Slice slice, Function mapper) { + int size = pageSize(maxResults); + int offset = offsetOf(pageToken); + // One extra row tells whether another page follows. + List rows = slice.fetch(offset, size + 1); + List items = rows.stream().limit(size).map(mapper).toList(); + return new ListResponse<>(items, rows.size() > size ? encode(offset + size) : null); + } + + private static int pageSize(Integer maxResults) { + if (maxResults == null || maxResults == 0) { + return MAX_RESULTS; + } + if (maxResults < 0) { + throw ApiException.invalidParameter("maxResults must not be negative"); + } + return Math.min(maxResults, MAX_RESULTS); + } + + private static int offsetOf(String pageToken) { + if (pageToken == null || pageToken.isEmpty()) { + return 0; + } + try { + String decoded = new String(Base64.getUrlDecoder().decode(pageToken), StandardCharsets.UTF_8); + if (decoded.startsWith(TOKEN_PREFIX)) { + int offset = Integer.parseInt(decoded.substring(TOKEN_PREFIX.length())); + if (offset >= 0) { + return offset; + } + } + } catch (IllegalArgumentException invalid) { + // Reported below like any other token this server did not issue. + } + throw ApiException.invalidParameter("pageToken is invalid"); + } + + private static String encode(int offset) { + return Base64.getUrlEncoder() + .withoutPadding() + .encodeToString((TOKEN_PREFIX + offset).getBytes(StandardCharsets.UTF_8)); + } +} diff --git a/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientService.java b/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientService.java index 8674ff64..27e02509 100644 --- a/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientService.java +++ b/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientService.java @@ -8,6 +8,7 @@ import io.opensharing.catalog.CatalogConnector; import io.opensharing.http.ApiException; import io.opensharing.http.ListResponse; +import io.opensharing.http.Pagination; import java.security.SecureRandom; import java.time.Duration; import java.time.Instant; @@ -71,12 +72,12 @@ public RecipientResponse create( return toResponse(recipient, activationBaseUrl); } - /** Lists every recipient by name, unpaged. */ - public ListResponse list(String activationBaseUrl) { - // TODO: page with maxResults and pageToken. + /** Lists one page of recipients by name. */ + public ListResponse list( + Integer maxResults, String pageToken, String activationBaseUrl) { // TODO: introduce USE_RECIPIENT and list only the caller's recipients unless they hold it. - return ListResponse.of( - recipients.list().stream().map(r -> toResponse(r, activationBaseUrl)).toList()); + return Pagination.page( + maxResults, pageToken, recipients::list, r -> toResponse(r, activationBaseUrl)); } /** Gets a recipient by name in any case. */ diff --git a/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientStore.java b/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientStore.java index e99f0d4d..050c46a7 100644 --- a/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientStore.java +++ b/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientStore.java @@ -92,13 +92,15 @@ public String findActivationCode(RecipientEntity recipient) { }); } - /** Every recipient, regardless of owner, ordered by name. */ - public List list() { + /** Recipients of any owner, ordered by name; names are unique, so the order is total. */ + public List list(int offset, int limit) { return tx.inTransaction( true, em -> em.createQuery( "select r from RecipientEntity r order by r.name", RecipientEntity.class) + .setFirstResult(offset) + .setMaxResults(limit) .getResultList()); } diff --git a/server/opensharing-server-core/src/main/java/io/opensharing/share/SharePermissionStore.java b/server/opensharing-server-core/src/main/java/io/opensharing/share/SharePermissionStore.java index c8674487..55c4fb19 100644 --- a/server/opensharing-server-core/src/main/java/io/opensharing/share/SharePermissionStore.java +++ b/server/opensharing-server-core/src/main/java/io/opensharing/share/SharePermissionStore.java @@ -51,17 +51,22 @@ public void revoke( em.remove(permission); } - /** The share's permissions, ordered by recipient name. */ - public List list(ShareEntity share) { + /** + * The share's permissions, ordered by recipient name and then privilege. A recipient holds + * each privilege at most once, so the order is total. + */ + public List list(ShareEntity share, int offset, int limit) { return tx.inTransaction( true, em -> em.createQuery( "select p from SharePermissionEntity p join fetch p.share" + " join fetch p.recipient r where p.share.id = :shareId" - + " order by r.name", + + " order by r.name, p.privilege", SharePermissionEntity.class) .setParameter("shareId", share.getId()) + .setFirstResult(offset) + .setMaxResults(limit) .getResultList()); } diff --git a/server/opensharing-server-core/src/main/java/io/opensharing/share/ShareService.java b/server/opensharing-server-core/src/main/java/io/opensharing/share/ShareService.java index 849840b9..4edb3860 100644 --- a/server/opensharing-server-core/src/main/java/io/opensharing/share/ShareService.java +++ b/server/opensharing-server-core/src/main/java/io/opensharing/share/ShareService.java @@ -13,6 +13,7 @@ import io.opensharing.exception.CatalogException; import io.opensharing.http.ApiException; import io.opensharing.http.ListResponse; +import io.opensharing.http.Pagination; import io.opensharing.recipient.RecipientEntity; import io.opensharing.recipient.RecipientStore; import jakarta.persistence.EntityManager; @@ -65,11 +66,10 @@ public ShareResponse create(UserContext user, CreateShareRequest request) { validateProperties(request.properties()))); } - /** Lists every share by name, unpaged. */ - public ListResponse list() { - // TODO: page with maxResults and pageToken. + /** Lists one page of shares by name. */ + public ListResponse list(Integer maxResults, String pageToken) { // TODO: check USE_SHARE through catalog.authorize. - return ListResponse.of(shares.list().stream().map(ShareResponse::from).toList()); + return Pagination.page(maxResults, pageToken, shares::list, ShareResponse::from); } /** Gets a share by name in any case, with the objects in it when {@code includeSharedData}. */ @@ -135,18 +135,20 @@ public void delete(UserContext user, String share) { shares.delete(share, user); } - /** Lists who holds which privilege on the share, by recipient name, unpaged. */ - public ListResponse listPermissions(String share) { - // TODO: page with maxResults and pageToken. - return ListResponse.of( - permissions.list(shares.getOrThrow(share)).stream() - .map(SharePermissionResponse::from) - .toList()); + /** Lists one page of who holds which privilege on the share, by recipient name. */ + public ListResponse listPermissions( + String share, Integer maxResults, String pageToken) { + ShareEntity entity = shares.getOrThrow(share); + return Pagination.page( + maxResults, + pageToken, + (offset, limit) -> permissions.list(entity, offset, limit), + SharePermissionResponse::from); } /** * Applies the changes in order, each revoking then granting, all in one transaction: either every - * change commits or none does. Returns the share's permissions. Owner only. + * change commits or none does. Returns the first page of the share's permissions. Owner only. */ public ListResponse updatePermissions( UserContext user, String share, UpdateSharePermissionsRequest request) { @@ -178,7 +180,7 @@ public ListResponse updatePermissions( } return null; }); - return listPermissions(share); + return listPermissions(share, null, null); } // Resolves the object in the catalog on behalf of the caller and checks the declared type. diff --git a/server/opensharing-server-core/src/main/java/io/opensharing/share/ShareStore.java b/server/opensharing-server-core/src/main/java/io/opensharing/share/ShareStore.java index 7e2f489b..52df108e 100644 --- a/server/opensharing-server-core/src/main/java/io/opensharing/share/ShareStore.java +++ b/server/opensharing-server-core/src/main/java/io/opensharing/share/ShareStore.java @@ -83,12 +83,14 @@ public ShareEntity getOwnedOrThrow(String name, UserContext user) { return tx.inTransaction(true, em -> checkOwner(getOrThrow(em, name, LockModeType.NONE), user)); } - /** Every share, regardless of owner, ordered by name. */ - public List list() { + /** Shares of any owner, ordered by name; names are unique, so the order is total. */ + public List list(int offset, int limit) { return tx.inTransaction( true, em -> em.createQuery("select s from ShareEntity s order by s.name", ShareEntity.class) + .setFirstResult(offset) + .setMaxResults(limit) .getResultList()); } diff --git a/server/opensharing-server-core/src/test/java/io/opensharing/http/PaginationTest.java b/server/opensharing-server-core/src/test/java/io/opensharing/http/PaginationTest.java new file mode 100644 index 00000000..cc1c2245 --- /dev/null +++ b/server/opensharing-server-core/src/test/java/io/opensharing/http/PaginationTest.java @@ -0,0 +1,57 @@ +package io.opensharing.http; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.List; +import java.util.function.Function; +import java.util.stream.IntStream; +import org.junit.jupiter.api.Test; + +class PaginationTest { + + private static final List ROWS = IntStream.range(0, 1200).boxed().toList(); + + @Test + void pagesThroughTheListWithTokens() { + ListResponse first = page(null, null); + assertEquals(ROWS.subList(0, 500), first.items()); + + ListResponse second = page(null, first.nextPageToken()); + assertEquals(ROWS.subList(500, 1000), second.items()); + + ListResponse last = page(null, second.nextPageToken()); + assertEquals(ROWS.subList(1000, 1200), last.items()); + assertNull(last.nextPageToken()); + } + + @Test + void capsRequestedPageSizesAtTheMaximum() { + assertEquals(500, page(1000, null).items().size()); + assertEquals(List.of(0, 1), page(2, null).items()); + } + + @Test + void treatsAZeroPageSizeAsUnset() { + ListResponse first = page(0, null); + assertEquals(ROWS.subList(0, 500), first.items()); + assertEquals(ROWS.subList(500, 1000), page(0, first.nextPageToken()).items()); + } + + @Test + void rejectsNegativePageSizesAndForeignTokens() { + assertThrows(ApiException.class, () -> page(-1, null)); + assertThrows(ApiException.class, () -> page(null, "not-a-token")); + assertThrows(ApiException.class, () -> page(null, "b3MxOi0x")); + } + + private static ListResponse page(Integer maxResults, String pageToken) { + return Pagination.page(maxResults, pageToken, PaginationTest::slice, Function.identity()); + } + + private static List slice(int offset, int limit) { + int end = Math.min(offset + limit, ROWS.size()); + return ROWS.subList(Math.min(offset, end), end); + } +} diff --git a/server/opensharing-server/src/main/java/io/opensharing/recipient/RecipientAdminService.java b/server/opensharing-server/src/main/java/io/opensharing/recipient/RecipientAdminService.java index eafaae1a..ae26c3c5 100644 --- a/server/opensharing-server/src/main/java/io/opensharing/recipient/RecipientAdminService.java +++ b/server/opensharing-server/src/main/java/io/opensharing/recipient/RecipientAdminService.java @@ -2,6 +2,7 @@ import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.server.ServiceRequestContext; import com.linecorp.armeria.server.annotation.Blocking; import com.linecorp.armeria.server.annotation.Delete; @@ -45,10 +46,11 @@ public RecipientResponse create(ServiceRequestContext ctx, CreateRecipientReques return recipients.create(ProviderTokenDecorator.user(ctx), request, activationBaseUrl); } - /** {@code GET /recipients}: lists every recipient by name, unpaged. */ + /** {@code GET /recipients}: lists one page of recipients by name. */ @Get("/recipients") - public ListResponse list() { - return recipients.list(activationBaseUrl); + public ListResponse list( + @Param @Nullable Integer maxResults, @Param @Nullable String pageToken) { + return recipients.list(maxResults, pageToken, activationBaseUrl); } /** {@code GET /recipients/{recipient}}: gets a recipient by name in any case. */ diff --git a/server/opensharing-server/src/main/java/io/opensharing/share/ShareAdminService.java b/server/opensharing-server/src/main/java/io/opensharing/share/ShareAdminService.java index a0cf52e3..973c2b98 100644 --- a/server/opensharing-server/src/main/java/io/opensharing/share/ShareAdminService.java +++ b/server/opensharing-server/src/main/java/io/opensharing/share/ShareAdminService.java @@ -2,6 +2,7 @@ import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.server.ServiceRequestContext; import com.linecorp.armeria.server.annotation.Blocking; import com.linecorp.armeria.server.annotation.Default; @@ -36,10 +37,11 @@ public ShareResponse create(ServiceRequestContext ctx, CreateShareRequest reques return shares.create(ProviderTokenDecorator.user(ctx), request); } - /** {@code GET /shares}: lists every share by name, unpaged. */ + /** {@code GET /shares}: lists one page of shares by name. */ @Get("/shares") - public ListResponse list() { - return shares.list(); + public ListResponse list( + @Param @Nullable Integer maxResults, @Param @Nullable String pageToken) { + return shares.list(maxResults, pageToken); } /** @@ -70,15 +72,21 @@ public HttpResponse delete(ServiceRequestContext ctx, @Param String share) { return HttpResponse.of(HttpStatus.NO_CONTENT); } - /** {@code GET /shares/{share}/permissions}: lists who holds which privilege on the share. */ + /** + * {@code GET /shares/{share}/permissions}: lists one page of who holds which privilege on the + * share. + */ @Get("/shares/{share}/permissions") - public ListResponse listPermissions(@Param String share) { - return shares.listPermissions(share); + public ListResponse listPermissions( + @Param String share, + @Param @Nullable Integer maxResults, + @Param @Nullable String pageToken) { + return shares.listPermissions(share, maxResults, pageToken); } /** * {@code PATCH /shares/{share}/permissions}: grants and revokes privileges in order, all or - * nothing, then returns the share's permissions. Owner only. + * nothing, then returns the first page of the share's permissions. Owner only. */ @Patch("/shares/{share}/permissions") public ListResponse updatePermissions( diff --git a/server/opensharing-server/src/test/java/io/opensharing/http/ProviderListPaginationTest.java b/server/opensharing-server/src/test/java/io/opensharing/http/ProviderListPaginationTest.java new file mode 100644 index 00000000..e03db188 --- /dev/null +++ b/server/opensharing-server/src/test/java/io/opensharing/http/ProviderListPaginationTest.java @@ -0,0 +1,161 @@ +package io.opensharing.http; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.fasterxml.jackson.databind.JsonNode; +import io.opensharing.TestServer; +import io.opensharing.TestServer.Response; +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +class ProviderListPaginationTest { + + private static final String SHARES = "/api/1.0/opensharing/provider/shares"; + private static final String RECIPIENTS = "/api/1.0/opensharing/provider/recipients"; + + private static TestServer server; + + @BeforeAll + static void startServer() { + server = TestServer.start(); + } + + @AfterAll + static void stopServer() { + server.close(); + } + + @Test + void pagesSharesByName() { + for (String name : List.of("paged-c", "paged-a", "paged-b")) { + create(SHARES, "{\"name\":\"" + name + "\"}"); + } + List names = walk(SHARES, "name"); + assertTrue(names.containsAll(List.of("paged-a", "paged-b", "paged-c")), names.toString()); + } + + @Test + void pagesRecipientsByName() { + for (String name : List.of("partner-c", "partner-a", "partner-b")) { + create(RECIPIENTS, recipient(name)); + } + List names = walk(RECIPIENTS, "name"); + assertTrue(names.containsAll(List.of("partner-a", "partner-b", "partner-c")), names.toString()); + } + + @Test + void pagesSharePermissionsByRecipientName() { + create(SHARES, "{\"name\":\"granted\"}"); + for (String name : List.of("grantee-c", "grantee-a", "grantee-b")) { + create(RECIPIENTS, recipient(name)); + } + + // Updating returns the first page of the share's permissions, which holds all three. + Response updated = + server.patch( + SHARES + "/granted/permissions", + "alice-token", + """ + {"changes":[ + {"recipientName":"grantee-c","add":["SELECT"]}, + {"recipientName":"grantee-a","add":["SELECT"]}, + {"recipientName":"grantee-b","add":["SELECT"]}]} + """); + assertEquals(200, updated.status()); + assertEquals(3, updated.body().path("items").size()); + assertTrue(updated.body().path("nextPageToken").isMissingNode()); + + assertEquals( + List.of("grantee-a", "grantee-b", "grantee-c"), + walk(SHARES + "/granted/permissions", "recipientName")); + } + + @Test + void resumesFromPageTokens() { + create(SHARES, "{\"name\":\"tokened\"}"); + for (String name : List.of("holder-a", "holder-b", "holder-c")) { + create(RECIPIENTS, recipient(name)); + Response granted = + server.patch( + SHARES + "/tokened/permissions", + "alice-token", + "{\"changes\":[{\"recipientName\":\"" + name + "\",\"add\":[\"SELECT\"]}]}"); + assertEquals(200, granted.status()); + } + String url = SHARES + "/tokened/permissions"; + + JsonNode first = fetch(url + "?maxResults=1"); + assertEquals(List.of("holder-a"), values(first, "recipientName")); + String token = first.path("nextPageToken").asText(); + + JsonNode rest = fetch(url + "?pageToken=" + token); + assertEquals(List.of("holder-b", "holder-c"), values(rest, "recipientName")); + assertTrue(rest.path("nextPageToken").isMissingNode()); + + // A token marks a position, not a page size, so it can be replayed with another maxResults. + JsonNode replayed = fetch(url + "?pageToken=" + token + "&maxResults=1"); + assertEquals(List.of("holder-b"), values(replayed, "recipientName")); + assertFalse(replayed.path("nextPageToken").isMissingNode()); + + for (String list : List.of(SHARES, RECIPIENTS, url)) { + Response invalid = server.get(list + "?pageToken=not-a-token", "alice-token"); + assertEquals(400, invalid.status()); + assertEquals(ErrorCodes.INVALID_PARAMETER_VALUE, invalid.errorCode()); + } + } + + @Test + void validatesPageSizes() { + create(SHARES, "{\"name\":\"capped\"}"); + JsonNode unset = fetch(SHARES + "?maxResults=0"); + assertEquals(fetch(SHARES).path("items"), unset.path("items")); + assertTrue(unset.path("items").size() > 0); + + Response negative = server.get(SHARES + "?maxResults=-1", "alice-token"); + assertEquals(400, negative.status()); + assertEquals(ErrorCodes.INVALID_PARAMETER_VALUE, negative.errorCode()); + } + + /** + * Follows nextPageToken two items at a time from the first page to the last and returns {@code + * field} of every item, checking that pages come in ascending order. + */ + private static List walk(String url, String field) { + List values = new ArrayList<>(); + String token = null; + do { + JsonNode body = fetch(url + "?maxResults=2" + (token == null ? "" : "&pageToken=" + token)); + List page = values(body, field); + assertTrue(page.size() <= 2, body.toString()); + values.addAll(page); + JsonNode next = body.path("nextPageToken"); + token = next.isMissingNode() ? null : next.asText(); + } while (token != null); + assertEquals(values.stream().sorted().distinct().toList(), values); + return values; + } + + private static JsonNode fetch(String url) { + Response response = server.get(url, "alice-token"); + assertEquals(200, response.status(), () -> response.body().toString()); + return response.body(); + } + + private static List values(JsonNode body, String field) { + return body.path("items").findValuesAsText(field); + } + + private static void create(String url, String body) { + assertEquals(201, server.post(url, "alice-token", body).status()); + } + + private static String recipient(String name) { + return "{\"name\":\"%s\",\"authenticationType\":\"TOKEN\",\"tokenExpirationDays\":30}" + .formatted(name); + } +}