Pagination with "after" cursor results in ScrollPosition that skips one extra item

[tata-dr-dk]

Using Graphql Query 'after' option with a cursor of position N, yields a result-set starting from position N+2 upwards, skipping the document right after the cursor that should have been included (N+1).

Using the latest graphql and mongodb starters

	implementation 'org.springframework.boot:spring-boot-starter-data-mongodb'
	implementation 'org.springframework.boot:spring-boot-starter-graphql'

Using graphql @QueryMapping annotation and ScrollSubrange on a controller

    @QueryMapping
    public Window<Customer> customers(@Argument String storeNumber, ScrollSubrange subrange) {
       //...
    }

and a query:

query MyQuery {
  customers(storeNumber: "123", after: "T18x") {
    edges {
      cursor
      node {
        name
        storeNumber
      }
    }
  }
}

where 'T18x' is the cursor of the first document, results in subrange.position() of 2.

Using that position in a mongodb Query:

        var query = Query.query(criteria)
                .with(scrollPosition);

would actually skip 2 documents, and not start from document of position 2 (thus not inclusive).
Seems to be related to #916

A project reproducing the bug can be found here: graphql-bug

[JIngrahamTG]

I've noticed the same issue. Also, the last and before includes the cursor provided as the "before", so last:2 before:"cursor" only includes one node before the cursor as well as the before cursor itself. I am also using the latest released version of spring graphql.

[pax95]

It seems to be a regression from #916.
Downgrading to spring-graphql:1.2.4 solves the pagination problem.

[rstoyanchev]

@tata-dr-dk, thanks for the sample.

Indeed #916 was a breaking change, but it was necessary to fix a problem that had been there from the start, and reverse another that was introduced in 1.2.4. That's pretty confusing I'll admit, but I believe the current behavior is correct.

In the sample, there are 3 customers. Cursor "T18x" decodes to "O_1", which is offset 1. That points to the 2nd customer, and the server correctly returns only the 3rd customer.

If I use cursor "T18w" instead, or "O_0", which is offset 0, that points to the first customer, and the server returns the 2nd and 3rd customers.

only includes one node before the cursor as well as the before cursor itself

If I use last:2 before:"T18x" in same the sample, the resulting ScrollSubrange has offset 0 and count 1 as expected. However, the controller uses 10, in effect ignoring the count, and the server returns all 3 customers. If I change the count to subrange.count().orElse(10), then the server returns only the 1st customer.

[tata-dr-dk]

Thanks @rstoyanchev for your response

I am totally new to GraphQL, but for me there still seems to be a incompatible behavior. I would expect the cursor X returned for any document to be usable in the after clause, which then would result in the next page starting from the document that exactly follows it, e.g. with cursor Y.

I updated the example unit tests with two cases, one using the first returned document.cursor, the other using the pageInfo.endCursor to query the next page. Both seem to show that there is a missing document, and an extra erroneous skip indeed is happening.

It would surprise me if that is indeed the correct behavior. In that case, requesting the next page would require the client to use the cursor with index "pageSize - 1", and the endCursor would not be of any value

The first document cursor returned is T18x and not T18w

[rstoyanchev]

That helps better to demonstrate the issue, thanks.

It looks like OffsetScrollPosition adds 1 to its offset, so when we render cursors starting from 1. However, when the cursor is sent back and used, the repository seems to treat the offset as zero based. I'm not sure if this is expected behavior for Spring Data or not, but I will investigate.

[rstoyanchev]

After some discussions with the Spring Data team, there is now a change planned on their side, see spring-projects/spring-data-commons#3070. After that we will see if we need any follow-up changes here.

[rstoyanchev]

We've decided to apply a temporary fix in spring-graphql, which allows us to release a fix for this regression without delay, and gives Spring Data a bit more time to work out how to proceed. Once there is a clear direction under spring-projects/spring-data-commons#3070, we'll adapt here as needed.

@tata-dr-dk, @JIngrahamTG, @pax95, if you are able to confirm the fix that's in 1.2.6-SNAPSHOT via https://repo.spring.io/snapshot that would be much appreciated.

[tata-dr-dk]

@rstoyanchev The fix works - thanks a lot!
Apologies for the late reply - was absent for quite long time and could not check the fix out.

[rstoyanchev]

Thanks for the confirmation.

For the upcoming 1.3 (RC1 released today, GA on May 21), there are underlying changes in Spring Data 2024 where offset positions start from 0 (rather than 1), and data stores treat offset positions as exclusive. We have adapted accordingly in #946.