Eliminating locking and improve safety with Pagers, Pollers

[heaths]

Resolves #3294

[heaths]

This is a draft to get some earlier feedback. For now I only changed PageIterator since it was simpler - ItemIterator needs to do some extra things - and because one or the other pager has to change first to store the function instead of the stream. PageIterator is the "leaf" in this scenario, so into_pages() and any tests that rely on it were commented out. Module tests were updated, but no examples or other crates' tests have been updated yet.

I'll likely copy the code to ItemIterator as well and get rid of its from_callback since the type declaration will be complex. I'll define a pager::from_callback or something that will return an impl <Trait> where I want to keep the ItemIterator and PageIterator as trait names. Unfortunately, that also means getting rid of Pager because types can't alias traits currently, not without being dyn; though, to just get the stream maybe that's not so bad?

I'll also copy the implementation to Poller. While I will try to find a way to refactor the shared code for pagers, I probably won't for Poller because there's enough difference that it's probably not worth it and will likely have to diverge in the future anyway. Over-abstracting likely isn't worth it.

[heaths]

I can't make this work. The previous BoxedStream solution erased most of the type parameters, but by exposing all those types now, it forces us to use dynamic dispatch which limits a lot of use cases and puts owness on the callers. The goal was eliminate locking but given the network IO, I don't think it's worth the regression to the DX.

[heaths]

Actually, if the type params are the source of "ugliness" and complexity, then the goal is to eliminate them. If I use fewer in the declaration, I think can limit it to P: Page and C: AsRef<str> - the latter because most implements do/will use Url for a "next link" instead of some arbitrary String for an opaque continuation token, but we know the latter exists in Azure.

So, it might mean that Pager gains an extra type param - that part of the implementation leaks out into the API - but we can at least default that as well e.g.,

pub type Pager<P, F = JsonFormat, C = Url> = ItemIterator<Response<P, F>, C>;

Cosmos uses a String but also defines their own type alias anyway, so they could do something like:

pub type FeedPager<P, F = JsonFormat, C = String> = ItemIterator<Response<P, F>, C>;

[analogrelay]

This feels on-track to me. I have some comments that might allow us to reduce clone()s, but I'm not really worried about them.

I do think the C: AsRef<str> bound could be removed, or simplified to C: Debug (for tracing purposes). The Pager doesn't actually care that it's a string. It just flows it back and forth to the actual request function.

[analogrelay]

We should be able to move the continuation_token rather than cloning it, no?

[analogrelay]

Does this actually need to hold the continuation_token itself? Or could the continuation_token functions just call in to PageIterator to get the current value?

[analogrelay]

We could just hold the PagerOptions, and update the continuation token there, to avoid cloning. Or, pull the entire PagerOptions apart in the constructor and only store the things we need here.

[analogrelay]

We probably don't need to store a copy of the continuation on the State anymore. We can just pull it from the fields on PageIterator and pass &C in to the make_request func.

[analogrelay]

Ah, though you might hit issues because the make_request future is probably hanging on to it. This is probably fine then.