ApolloPagination: Various API Improvements, refactor into actor

[Iron-Ham]

What are you trying to accomplish?

1. Support for many potential simultaneous triggers of loadMore across many threads.
2. Adds support for throwing transforms in type erased AnyGraphQLQueryPagers.
3. Changes the API to use a canLoadNext function that can be properly used in a type erased pager.
4. Allow AnyGraphQLQueryPager to operate over an individual model as well as arrays. I found that some instances of pagination involved updating a singular model, vs keeping an array of models.
5. Ease of use for both UIKit and SwiftUI contexts.

What approach did you choose and why?

Thread Safety

I kept bashing my head into this towards the tail end of last week. On the morning of 10/15, the idea just came to me:

Instead of trying to use async/await APIs, or locks, or what have you, you can use a combination of Combine and async/await conversion APIs to achieve the goal.

This API is deceptively challenging and non-trivial to make thread safe. It boils down to the following set of requirements:

1. We want to block the loadMore action if there is already an extant loadMore in flight.
2. A loadMore isn't complete until it triggers the callback of its underlying watcher.
3. A watcher can trigger that callback many times: Once or twice on initial call (depending on the cachePolicy), and then again if it observes any cache changes. This means that we want to block it only until the "canonical" load has landed.

A continuation can only be continued once. Continuing it more than once will trigger a crash.
A Combine Publisher can continuously receive values, until it receives a completion event, at which point it will no longer receive or publish values.

My thought boiled down to the following:

1. Create a Combine publisher with the intent of only sending a completion when the "canonical load" has returned, or upon error.
2. Use an async/await continuation to await on the result of the canonical load.
3. Defer to the task that contains that async/await continuation if the loadMore function is called again.

By then turning the Pager into an actor, we can ensure thread safety as it runs all of its operations in a thread isolated context

Support for throwing transforms in type erased AnyGraphQLQueryPagers

Some models don't necessarily always initialize. I'm choosing to support throwing transforms here as opposed to (Query.Data) -> ModelType? with the assumption that one can be converted to the other, but throwing transforms provide additional information that can be bubbled up to the pager.

Changes the API to use a canLoadNext function that can be properly used in a type erased pager.

When used as a variable, we couldn't access this value as it required knowledge of the pager used during instantiation. However, as a function, we can pass its reference to the type erasure and access it accordingly.

Allow AnyGraphQLQueryPager to operate over an individual model as well as arrays.

I found that some instances of pagination involved updating a singular model, vs keeping an array of models. Some of our code at GitHub operates over a single model. For example, when rendering a pull request, we operate over a pull request model. When we paginate in that view, we are loading the events, comments, and so on that are associated with that pull request (@so-and-so has marked as draft, @so-and-so-and-so has closed with comment, etc.).

Ease of use for both UIKit and SwiftUI contexts.

The GraphQLQueryPager is easily used in UIKit, whereas SwiftUI can easily use both GraphQLQueryPager and GraphQLQueryPager.Actor (.task { ... } can use an actor easily).

Anything you want to highlight for special attention from reviewers?

Yes! This is pretty wild stuff. I would love a second set of eyes on this.

---

🤖 Generated by Copilot

Summary

🔄🚀🐛

This pull request introduces a new GraphQLQueryPagerWrapper class that adds concurrency and cancellation support to the existing GraphQLQueryPager class for pagination of GraphQL queries. It also updates various classes and tests that use the GraphQLQueryPager class to use the new wrapper class instead. Additionally, it refactors the type-erasing AnyGraphQLQueryPager class to use the new wrapper class and simplifies the pagination logic and interface. Finally, it adds a new error case loadInProgress to the PaginationError enum to handle concurrent load more requests.

There once was a class called GraphQLQueryPager
That needed some updates to make it much safer
It got a new wrapper
With actor and publisher
And now it supports concurrency and cancellation behavior

Walkthrough

• Replace GraphQLQueryPager type with GraphQLQueryPagerWrapper type in various pagination scenarios and tests
• Add a new case loadInProgress to the PaginationError enum
• Add a new function test_concurrentFetches_nonisolated to the ConcurrencyTests class
• Refactor the logic of the fetch and loadMore methods of the GraphQLQueryPager class into smaller methods that handle the initial fetch, the subsequent fetch, and the task cancellation
• Remove the subscribe method from the GraphQLQueryPager class and the AnyGraphQLQueryPager class
• Remove the fetch method from the GraphQLQueryPager class and the AnyGraphQLQueryPager class
• Remove the _subject property and the subject property from the AnyGraphQLQueryPager class
• Remove the _subject.send(completion: .finished) line from the cancel method of the GraphQLQueryPager class
• Remove an empty line from the AnyGraphQLQueryPagerTests class

[AnthonyMDev]

Can we add a function here for easily converting a GraphQLPager to an AnyGraphQLPager like a map function? Not sure what to call it, maybe mappingResults?

So you can do this:

let pager: AnyGraphQLPager<CustomModel> = makeQueryPager(
    client: client,
    queryProvider: { _ in PageQuery() },
    extractPageInfo: { data in CustomPage(data) }
).mappingResults { result in
 CustomModel(result)
}

[Iron-Ham]

I have those over in AnyGraphQLPager.swift -- extensions on GraphQLQueryPager named eraseToAnyPager, which allow you to supply a function mapping Output to your custom model.

[AnthonyMDev]

This is coming along so nicely! Thanks!

[BobaFetters]

@Iron-Ham any idea why the CI tests etc arent running anymore for this PR? Doesn't look like the additions you made should have affected that but maybe they did?

[Iron-Ham]

@BobaFetters No clue -- the CI changes in here shouldn't have an impact.

I imagine it's related to the verified commit accepting Anthony's suggestion? I can double check when I'm back in office 😜