Skip to content

Commit

Permalink
Allow for null paging inside Collection response envelopes (#1019)
Browse files Browse the repository at this point in the history

---------

Co-authored-by: Karthik Ramgopal <kramgopa@linkedin.com>
  • Loading branch information
karthikrg and li-kramgopa authored Sep 9, 2024
1 parent e2f6fa7 commit 5b3e6cc
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 3 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
version=29.58.5
version=29.58.6
group=com.linkedin.pegasus
org.gradle.configureondemand=true
org.gradle.parallel=true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,23 @@ public RestLiResponse buildResponse(RoutingResult routingResult, D responseData)
CollectionResponseEnvelope response = responseData.getResponseEnvelope();
RestLiResponse.Builder builder = new RestLiResponse.Builder();
CollectionResponse<AnyRecord> 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())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -226,6 +227,36 @@ public <D extends RestLiResponseData<? extends CollectionResponseEnvelope>> 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 <D extends RestLiResponseData<? extends CollectionResponseEnvelope>>
void testNullPaging(CollectionResponseBuilder<D> responseBuilder)
{
Map<String, String> 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<Foo> 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()
{
Expand Down

0 comments on commit 5b3e6cc

Please sign in to comment.