From 5b3e6cc67efc2d4787e89911102dacde0f7941cf Mon Sep 17 00:00:00 2001 From: Karthik Ramgopal Date: Mon, 9 Sep 2024 10:48:39 -0700 Subject: [PATCH] Allow for null paging inside Collection response envelopes (#1019) --------- Co-authored-by: Karthik Ramgopal --- CHANGELOG.md | 6 +++- gradle.properties | 2 +- .../response/CollectionResponseBuilder.java | 18 ++++++++++- .../TestCollectionResponseBuilder.java | 31 +++++++++++++++++++ 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2120d7f9d8..adf7ab8170 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ and what APIs have changed, if applicable. ## [Unreleased] +## [29.58.6] - 2024-09-08 +- Allow for null paging inside Collection response envelopes + ## [29.58.5] - 2024-09-04 - Respect glob collection subscriptions on reconnect @@ -5725,7 +5728,8 @@ patch operations can re-use these classes for generating patch messages. ## [0.14.1] -[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.58.5...master +[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.58.6...master +[29.58.6]: https://github.com/linkedin/rest.li/compare/v29.58.5...v29.58.6 [29.58.5]: https://github.com/linkedin/rest.li/compare/v29.58.4...v29.58.5 [29.58.4]: https://github.com/linkedin/rest.li/compare/v29.58.3...v29.58.4 [29.58.3]: https://github.com/linkedin/rest.li/compare/v29.58.2...v29.58.3 diff --git a/gradle.properties b/gradle.properties index ba684a867a..a42923ef16 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -version=29.58.5 +version=29.58.6 group=com.linkedin.pegasus org.gradle.configureondemand=true org.gradle.parallel=true diff --git a/restli-server/src/main/java/com/linkedin/restli/internal/server/response/CollectionResponseBuilder.java b/restli-server/src/main/java/com/linkedin/restli/internal/server/response/CollectionResponseBuilder.java index 2d3a9134ab..ae518fc1c1 100644 --- a/restli-server/src/main/java/com/linkedin/restli/internal/server/response/CollectionResponseBuilder.java +++ b/restli-server/src/main/java/com/linkedin/restli/internal/server/response/CollectionResponseBuilder.java @@ -51,7 +51,23 @@ public RestLiResponse buildResponse(RoutingResult routingResult, D responseData) CollectionResponseEnvelope response = responseData.getResponseEnvelope(); RestLiResponse.Builder builder = new RestLiResponse.Builder(); CollectionResponse collectionResponse = new CollectionResponse<>(AnyRecord.class); - collectionResponse.setPaging(response.getCollectionResponsePaging()); + + // Technically, there is no way to set paging to null in application code since CollectionResult doesn't allow + // it. However, it is possible to use a custom rest.li filter to strip out paging from the ResponseEnvelope in + // case use cases don't want index based paging. This null check detects and elegantly handles such cases. + // + // An alternative would be to support null indexed based paging natively inside CollectionResult and all its + // upstream objects. However, doing so needs several changes inside framework code, and is potentially fragile. + // Hence, we prefer a point fix here. + if (response.getCollectionResponsePaging() != null) + { + collectionResponse.setPaging(response.getCollectionResponsePaging()); + } + else + { + collectionResponse.removePaging(); + } + DataList elementsMap = (DataList) collectionResponse.data().get(CollectionResponse.ELEMENTS); for (RecordTemplate entry : response.getCollectionResponse()) { diff --git a/restli-server/src/test/java/com/linkedin/restli/internal/server/response/TestCollectionResponseBuilder.java b/restli-server/src/test/java/com/linkedin/restli/internal/server/response/TestCollectionResponseBuilder.java index b7dde6a03d..5a53b65d78 100644 --- a/restli-server/src/test/java/com/linkedin/restli/internal/server/response/TestCollectionResponseBuilder.java +++ b/restli-server/src/test/java/com/linkedin/restli/internal/server/response/TestCollectionResponseBuilder.java @@ -29,6 +29,7 @@ import com.linkedin.r2.message.rest.RestRequestBuilder; import com.linkedin.restli.common.CollectionMetadata; import com.linkedin.restli.common.CollectionResponse; +import com.linkedin.restli.common.HttpStatus; import com.linkedin.restli.common.LinkArray; import com.linkedin.restli.common.ResourceMethod; import com.linkedin.restli.internal.server.RoutingResult; @@ -226,6 +227,36 @@ public > void EasyMock.verify(mockContext); } + @DataProvider(name = "testNullPaging") + public Object[][] testNullPaging() + { + return BUILDERS.values().stream().map(builder -> new Object[]{builder}).toArray(Object[][]::new); + } + + @Test(dataProvider = "testNullPaging") + public > + void testNullPaging(CollectionResponseBuilder responseBuilder) + { + Map headers = ResponseBuilderUtil.getHeaders(); + + ServerResourceContext mockContext = EasyMock.createMock(ServerResourceContext.class); + ResourceMethodDescriptor mockDescriptor = EasyMock.createMock(ResourceMethodDescriptor.class); + RoutingResult routingResult = new RoutingResult(mockContext, mockDescriptor); + + Foo metadata = new Foo().setStringField("metadata").setIntField(7); + final List generatedList = generateTestList(); + + RestLiResponse response = responseBuilder.buildResponse(routingResult, + responseBuilder.buildResponseData(HttpStatus.S_200_OK, generatedList, null, metadata, headers, + Collections.emptyList())); + + Assert.assertTrue(response.getEntity() instanceof CollectionResponse); + CollectionResponse collectionResponse = (CollectionResponse) response.getEntity(); + Assert.assertEquals(collectionResponse.getElements(), generatedList); + Assert.assertEquals(collectionResponse.getMetadataRaw(), metadata.data()); + Assert.assertFalse(collectionResponse.hasPaging()); + } + @DataProvider(name = "exceptionTestData") public Object[][] exceptionDataProvider() {