Stream limiting doesn't work

[mariuszs]

Using code like this:

Stream<Project> projects = gitlabApi.getProjectApi().getProjectsStream();
projects.limit(1).map(Project::getName).forEach(name -> System.out.println(name));

should results in getting only limited number of projects from gitlab server (not all of them), in this case only 1.

[gmessner]

@mariuszs
Per the Java documentation, Stream.limit() is not intended to limit the number of items a the source Stream, rather it limits the number of items in a truncated version of the source stream. Form the Javadocs for Stream.limit():

Stream<T> limit(long maxSize)
Returns a stream consisting of the elements of this stream, truncated to be no longer than maxSize in length.

This library is set up so that when a Stream is returned it will always contain all available items, so all projects are fetched from the server and then the limit(1) will be applied later in the pipeline, resulting in only 1 name being printed, but all projects being written. It would be very difficult to implement this any other way, and would also provide non-predictable results from the GitLab server if all items are not read from the server.

I will make this fact that all items are read clearer in the documentation.

[mariuszs]

Thanks for your answer, this kind of implementation makes it not very usable. I was trying to implement PagerSplitter to improve this behaviour. Please take a look.

[gmessner]

@mariuszs
Thank you for you contribution of PR #296, however it will not be able to be used.

Because of the way many of the GitLab server endpoints are implemented, it makes it completely unpredictable to operate on streams the way you propose, This is known from experience. Depending on how the Stream is used, it is susceptible to items being repeated or actually left out. The original implementation was very similar to your pull request and after significant testing demonstrated that it would not work reliably, which is why the final implementation uses a StreamBuilder and basically fetches all the elements to construct the Stream.

In the end reliability and predicatabilty are the overriding concerns.

[mariuszs]

This is why this splitter has no characteristics like DISTINCT or ORDERED. Now when you stream using getProjectsStream(), then this method probably will never end, because it is iterating all projects. And this method is just as unpredictable like iteration using Pager (but Pager is usable because you don't need to iterate all projects).

I don't see any enhancements in this method getProjectsStream(), it can be simply replaced with getProjects().all().stream(), but it is confusing user that maybe this API provides something more and allow to lazy load data from gitlab.

[gmessner]

@mariuszs
You are correct that Pager can be unpredictable when used if there is enough time between calls to next().

The way Stream is currently implemented allows for uses of a Stream that the Spliterator approach does not. I'm open to supporting both ways of producing a Stream, allowing the consumer the ability to select whether it is a paged stream or not, that way more uses of Stream are supported not less. This could be implemented by adding an additional parameter to the getXXXXStream() methods indicating whether the Stream should be paged or not

EIther way, I will be updating the documentation about the implementation and use of Stream.

I will also be setting the default page size to 20, which is the GitLab default. It was niavely set to 100 long ago to allow a single call to fetch as many items as allowed by GitLab, but the single call now fetches all elements using Pager.all(). Many of GitLab's endpoints work better and more efficiently when fetching in increments of 20.

[mariuszs]

Because these are network operations, and there is no transaction, then all operations are unpredictable. Also, the current getXXXXStream() method can result in record duplication or missing elements. So I don't see any disadvantage in using Spliterator.
Fortunately, it is very easy to use your library with custom stream implementation, because Pager class is public. So anyone who wants can use lazy streams for fetching data. For me, this is enough. Thanks.

[gmessner]

@mariuszs
To make it easier to use a lazy Stream, It would great if you could modify your PR so that a consumer can make a call to any method that returns a Pager instance and then call a new method on that Pager instance which would return the lazy Stream instance. Something like this:

public Stream<T> lazyStream() throws GitLabApiException {
    // Make sure that current page is 0, this will ensure the whole list is streamed
    // regardless of what page the instance is currently on.
    currentPage = 0;

    return StreamSupport.stream(new PagerSpliterator(this), false);
}

We should consider only allowing a single Stream to be issued from the Pager, otherwise it could be abused and then the results would certainly be unpredicatable.