search

spring-projects / spring-graphql

Spring Integration for GraphQL
https://spring.io/projects/spring-graphql
Apache License 2.0
1.5k stars 297 forks source link

Use either `first` or `after` to determine forward pagination #929

Closed rstoyanchev closed 3 months ago

rstoyanchev commented 4 months ago

While investigating #925, I noticed we only check for the presence of last to assume backward pagination, so for a query with only before, we assume forward, and as a result ignore the before argument. The pagination spec does expect both last and before to enable backward pagination, but nevertheless we should check the presence of either last or before

GoncaloPT commented 3 months ago

@rstoyanchev my understanding of what we need has increased since you've closed the other issue.

Taking into account that you will be dealing with pagination; I would (re)ask: Can we have the total element/count information on Connection?

After looking into the pagination package I can now see I would need to reimplement a lot to provide a total count on a new type of connection that supports pagination...

Could we introduce some other ConnectionAdapterSupport for org.springframework.data.domain.Page type? I see we have SliceConnectionAdapter and WindowConnectionAdapter... Perhaps we could have a PageConnectionAdapter.

We could reuse/compose with SliceConnectionAdapter since it already provides most of what we need, other than the totalCount which is provided by the Page ( Slice subclass )

Sorry for hijacking this issue but seems related to the work you will be doing.

Edit: We would probably need: Optional<Integer> getTotalCount(Object container); at ConnectionAdapter interface as well.

rstoyanchev commented 3 months ago

The steps to check have been updated to start by looking for either "after" or "first", leading to forward pagination. Then for the presence of either "before" or "last" for backward pagination.