Page.getNextPage returns an empty Page object

[davejohnclark]

Calling Page.getNextPage returns an empty Page object when even when there is more content. It looks like the getNextPage method doesn't have enough type information to construct the correct TypeToken (the type T has been erased by this point, so the TypeToken will just be TypeToken<Page> rather than e.g. TypeToken<Page<OperationResponse>>).

The first Page response is request with the correct concrete type, and the TypeToken is available in the ResponseHandler.handleResponse method. I've put together a branch with a quick-fix for this that preserves the input TypeToken on any object implementing a new TypedResponse interface. This lets it be re-used within getNextPage.

You can see it at https://github.com/davejohnclark/java-stellar-sdk/commit/7960f78c603f2726c814a96f6a1e305160d15b40 if this looks reasonable I'm happy to tidy this up and get a PR raised.

[bartekn]

@davejohnclark thanks for the report. It looks like you are correct, I confirmed that Page.getNextPage does not work as expected. I'll be happy to check and merge a PR fixing this.

[robert-de-vries]

@davejohnclark @bartekn I noticed the same problem with getNextPage and was busy fixing it until I saw this issue. Can I expect this fix to be part of the sdk soon? The solution seems correct to me and has my approval.

[robert-de-vries]

Although this fix works and really is an improvement, still an empty page is returned after calling getNextPage on the last page. Also in case there is no first page, an empty page is returned. Returning null is more appropriate.

Replacing the last line of the try block in ResponseHandler.handleResponse with beneath code will fix this. I think this additional fix should be part of this issue, so I did not create a new issue. Correct me if I'm wrong.

      if (object instanceof Page) {
        Page page = (Page)object;
        // If a page has to be returned and there is no data available the endpoint returns an empty page. It should
        // return 'null' (meaning no page) instead of an empty page. Let's fix it here.
        if (page == null || page.getRecords() == null || page.getRecords().isEmpty()) {
          return null;
        }
      }
      return object;

[davejohnclark]

@robert-de-vries Seems interest has picked up again in this, so I'm happy to continue this discussion as it looks like this fix will get merged in some form.

Having not looked at this code for a while I was about to disagree with your suggestion, personally I would prefer to handle an empty Page object than have to worry about nulls, but when I looked back over the getNextPage implementation two things stuck out:

1. The javadoc on the method does in fact declare that null will be returned if there are no more results.
2. If there is no no next link, it is already supposed to return null.

This begs the question, why, when there are no more results, is there a next link? To me this suggests a divergence between the underlying api and the expectations this sdk is built on.

Either the api surfaces a next link, and links to an empty page, in which case I'd expect the behaviour I indicated a preference for above where an empty Page object would be returned (this means that the java sdk and the underling api behave the same - follow the link, get empty results), or there is no next link and null is returned when getNextPage is called (to be picky my preference here would in fact be that getNextPage return an Optional but this isn't the right place to get into that discussion - I'm happy to follow the standards of the rest of the sdk's api).

@nullstyle Any chance you'd care to weigh in on this? Is one of these approaches more consistent with the expected behaviour of the sdk, or the stellar api in general? I'm happy to include the change suggested by @robert-de-vries if that's the desired behaviour (I'd probably put it in the getNextPage method directly, rather than the ResponseHandler I think - otherwise that ResponseHandler will start to know a lot about the types it's handling - but happy to discuss further if we go down that route).

[nullstyle]

At present the end of a collection is signified by one of two things: A returned page smaller than the requested limit or an empty page is returned. There is no end of collection hinting and in general even an empty page will have a next link to provide a continuation of the traversal.

[robert-de-vries]

@davejohnclark My preference was first to put this code directly into the getNextPage method, but the first page that is retrieved may also be empty. Putting this code directly into ResponseHandler also fixes this case. Maybe we need another refactoring of the code, but that needs some more brain power. A utility method <T> boolean isEmpty(Page<T> page) may also be an option.