Add PageStreamer to make it simple to fetch paged lists

[jskeet]

Fixes issue #632, as far as we want to for now.

This will only make the PageStreamer available - it won't be used in generated code.
We will want to add examples of how to use this.

Note that the only async option for the moment is to retrieve a whole list asynchronously.
While it would be nice to do this more lazily, that would need something like Ix-Async, and we'd prefer not to
introduce a new dependency for this at the moment.

[gabikliot]

Also, for semantics of async fetch: in addition to the 2 options you provided, don't you think we also want a 3rd option: asynchronously fetch them one by one. Kind of like
Task<TResource> FetchNext and the calling code has to check the token to decide if there is indeed next. Something like Azure Table CloudTable.ExecuteQuerySegmentedAsync method.

This option is important when you can't store all resources in memory at once, so you have to load them one by one, in async way.
Perhaps that is what you meant by Ix-Async? I think you can do something like ExecuteQuerySegmentedAsync without introducing any new dependency.

[jskeet]

That 3rd option is what we would have if we added the dependency to Ix-Async, so we could return an IAsyncEnumerable<T>. But I don't want to either a) make up my own equivalent which would be subtly different and incompatible with Ix-Async or b) take a dependency on it. I don't think the benefit is worth the downside at this point.

We can potentially introduce a separate project which depends on both Ix-Async and this project, and adds extension methods to PageStreamer (which would need to expose a bit more of its functionality, admittedly).

[gabikliot]

I definitely would NOT take dependence on IAsyncEnumerable<T>, not until async RX is an official part of .NET libraries (which may happen at some point, I know some people who are working on that).
But why not to mimic what Azure Storage is doing, like ExecuteQuerySegmentedAsync? You already introduced a Token in the PageStreamer API.

[jskeet]

Which of the 16 overloads of ExecuteQuerySegmentedAsync are you referring to? The point of PageStreamer is to avoid the caller having to deal with the continuation token themselves at all. If they're going to have to manage it, PageStreamer won't provide any actual benefit - they can just call the existing API in a loop. Perhaps if you could provide a sample of what you'd expect the client code to look like - assuming an existing PageStreamer it would clarify things.

[jskeet]

Btw, one downside of this PR compared with the original code is that this was originally written with the expectation of it being called by generated code, mostly. But here's an example of showing the same sort of thing in manual code:

https://github.com/GoogleCloudPlatform/gcloud-dotnet/blob/master/GoogleApiWrappers/src/Google.Apis.Storage.v1.ClientWrapper/StorageClient.ListBuckets.cs#L77

(It's possible that the lambda expression could be replaced by a method group conversion. I haven't checked.)

[gabikliot]

Ohh, I see you point now about "The point of PageStreamer is to avoid the caller having to deal with the continuation token themselves at all".
I definitely did mean the option when the caller deals with tokens himself. That is why I shared the stackoverflow usage example: the caller is calling querySegment.ContinuationToken != null.

I agree that "they can just call the existing API in a loop" and that PageStreamer is an abstraction above that.
We (all Google Apis) just need to make sure we do expose the Token from all the segmented APis, so the caller can check querySegment.ContinuationToken != null.

[jskeet]

Right - I believe we're already good with that. In particular, if the token isn't exposed, we wouldn't be able to construct the PageStreamer to start with :) Sounds like we're on the same page now, no pun intended. Would love Ix-Async to be available already - with C# language support, ideally...

(The gRPC C# tooling already has a dependency on it, btw.)

[jskeet]

Just had another thought about this - as in this location PageStreamer is all about the Google API Client library (rather than, say, gRPC) we can add constraints on the request type, and avoid one parameter. Will have a look at that, and add it as another commit in the same PR.

[gabikliot]

Sounds like we're on the same page now

Yep. 👍

As for Async-Rx: yes, Bart De Smet was promising it to us a long time ago. Can't wait for that to happen.

There is also a new kid on the block: coreclr team it prototyping with a new Async Channels and AsyncEnumerator

[jskeet]

Humbug - can't do that, as IClientServiceRequest is in GoogleApis, not GoogleApis.Core.

@peleyal If I were to move PageStreamer into GoogleApis, would that feel odd? What namespace would you expect it to be in? It would definitely make it easier to call PageStreamer.

[peleyal]

I only reviewed the PageStreamer (not tests yet).
Sorry for joining the discussion a little bit late, I hope that my comments make sense.

[jskeet]

Due to the scale of the changes, I've now squashed and repushed. The good news is that it's now significantly easier to call, and slightly easier to construct.

For example, the GCS client code listing buckets would be:

private static readonly PageStreamer<Bucket, BucketsResource.ListRequest, Buckets, string> bucketPageStreamer =
    new PageStreamer<Bucket, BucketsResource.ListRequest, Buckets, string>(
       (request, token) => { request.PageToken = token; return request; },
        buckets => buckets.NextPageToken,
        buckets => buckets.Items);
...

// Create the initial request as before, then use the page streamer synchronously
// or asynchronously.
var request = ...;
foreach (var bucket in bucketPageStreamer.Fetch(request))
{
    ...
}

// Or
var buckets = await bucketPageStreamer.FetchAllAsync(request, cancellationToken);

[mmdriley]

lgtm

[jskeet]

Fixed using directive order, updated copyright year and added a TODO for a doc comment example.

[jskeet]

I've rebased this to Matt's reorganization, and squashed it down to a single commit. I believe it's all sensible...

[jskeet]

@mmdriley @peleyal Please review the latest commit carefully (before I squash...)

While rewriting the tests slightly, it struck me that within the context of google-api-dotnet-client, it's reasonable to assume that all requests objects are mutable, and so simplify the "provide a request object with this page token" to "just modify the existing request". This is basically what people would be doing anyway, and now it's unambiguous. The setup code is now simplified to:

private static readonly PageStreamer<Bucket, BucketsResource.ListRequest, Buckets, string> bucketPageStreamer =
    new PageStreamer<Bucket, BucketsResource.ListRequest, Buckets, string>(
        (request, token) => request.PageToken = token,
        buckets => buckets.NextPageToken,
        buckets => buckets.Items);

[peleyal]

LGTM, thanks Jon!
I just left one comment regarding TPL, which I'm not actually sure about, but it's worth to mention it here

[mmdriley]

LGTM. Sorry again for pulling the code out from under you mid-PR.

[jskeet]

@mmdriley: No problem at all. Given that this was just new code, it was probably the simplest possible change to make under that scenario :)

Will squash and merge.