GraphQLQueryWatcher: publisher property

[Iron-Ham]

What are you trying to accomplish?

I noticed that in situations where we have a large number of local cache updates and network cache updates, that we end up refreshing the UI far too often. By being able to treat the results of cache updates as a Publisher, we can place throttle or debounce functionality into the watcher's update stream.

What approach did you choose and why?

This PR introduces the following changes:

• apollo-ios/Sources/Apollo/GraphQLQueryWatcher.swift: The GraphQLQueryWatcher class was extended to include a CachePublisher struct and a WatcherSubscription class, enabling the watcher to publish cache updates to subscribers. A publisher() method was also added to allow access to the CachePublisher.
  • This was chosen as an alternative to making the GraphQLQueryWatcher a Publisher itself, as it is an observer of the underlying cache.
• apollo-ios/Sources/Apollo/GraphQLQueryWatcher.swift: The GraphQLQueryWatcher class was refactored to include convenience initializers and a private initializer. The resultHandler was updated to trigger subscriptions when a result is received.
  • The original initializer was effectively left unchanged, meaning that there is no breaking change to existing consumers.
  • A new initializer was added that does not take a result handler. This initializer expects users to then utilize the publisher property.
• apollo-ios/Sources/Apollo/GraphQLQueryWatcher.swift: The resultHandler property in the GraphQLQueryWatcher class was changed from a constant to an optional variable.
• apollo-ios/Package.swift: The minimum macOS version for the Apollo package was updated from 10.14 to 10.15. This is because Combine is unsupported on 10.14. This constitutes a breaking change for macOS 10.14 deployments.

Anything you want to highlight for special attention from reviewers?

Employing this on the GraphQLQueryWatcher feels like the correct choice, as opposed to doing it in a roundabout manner on the ApolloClient, but would require a version minimum increase to macOS 10_15.

NOTE: I didn't tackle any macOS 10.15 deprecations.

Alternatives Considered:

• As per the OSS ApolloCombine project: an extension on ApolloClient. I chose to avoid this route because: it wouldn't integrate well with ApolloPagination, it would be wasteful if you want multiple subscribers on the same watcher.
• Pulling the behavior of GraphQLQueryWatcher into a protocol / creating a subclass of GraphQLQueryWatcher that has this behavior. I chose not to do this since it seems less discoverable, and perhaps a bit arbitrary. It has the advantage of not needing a version bump.

[netlify]

👷 Deploy request for eclectic-pie-88a2ba pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	872d9fb

[netlify]

👷 Deploy request for apollo-ios-docc pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	872d9fb

[AnthonyMDev]

I'm not opposed to this addition, but I want to make sure we think through it.

I'm unclear on how easy it is to create a retain cycle here. There will definitely be a retain on the watcher, but since the watcher holds on to the subscriptions will that cause a retain cycle in any situation? What if the subscriber block accesses the watcher itself? I guess that would be broken by calling cancel though, right?

[AnthonyMDev]

I would think the handler would need to be @escaping here also. What am I missing?

[AnthonyMDev]

Thinking this should be a let right?

[AnthonyMDev]

Make this let please.

[Iron-Ham]

🤔
While this is potentially directionally correct – I'm not sure it's the best solution for the problem. I spoke with @eliperkins earlier today, who suggested a mechanism like FMDB's queueing mechanism, in conjunction with the concept of a cache update timestamp.

---

Imagine the following scenario:

A user takes five rapid actions, each triggering an optimistic cache update and queuing a network request which makes the mutation. The mutation responds with the updated data, and in the event of failure rolls back the optimistic update. The user actions must have optimistic updates attached, as the UI would otherwise be out-of-sync. The server responses, however, would contain stale data relative to other optimistic updates – which would blow out the user's changes, resulting in a jarring experience. (Especially if the user was deleting/removing items from a list – like when we archive emails)

This PR attempts to solve this problem by giving consumers of a watcher the ability to debounce or throttle outputs. While that may be a desired behavior in another context, it seems like it shouldn't be the primary solution. Instead, what if each updated value had an updated_at timestamp that gets committed by a new Client method? We could have an API that looks something like this:

func perform<Mutation: GraphQLMutation, T>(
  mutation: Mutation,
  publishResultToStore: Bool,
  context: RequestContext?,
  queue: DispatchQueue,
  optimisticUpdate: ((ReadWriteTransaction) throws -> T)?, // NEW: optional `optimisticUpdate` parameter
  resultHandler: GraphQLResultHandler<Mutation.Data>?
) -> Cancellable

This way, the client can:

• Trigger the optimistic update, with an internally stored (or stored within sqlite?) timestamp of when the field was updated.
• When the mutation response is received, it uses the timestamp of the optimistic mutation (if one was provided; else, it uses the current time) to determine if a given value should be overwritten.

With all of this in mind, the (success) path would look something like this:

A user takes five rapid actions, each triggering an optimistic cache update and queuing a network request which makes the mutation. Each optimistic event is tied to a given timestamp, and only data whose timestamp is equal (or later than) the previous data's timestamp can overwrite. Thus, when a mutation responds, it only overwrites data that has no timestamp (existing cached data) or confirms the optimistic update by overwriting data with a timestamp equal-to-or-less-than its timestamp. After five mutations, the cache state remains correct throughout the process, even for actions that have not yet been confirmed by the server. If the consumer of the API wants throttling or debouncing, that seems like a separate (and separately solved) problem.

There's still the matter of rollbacks, and that's something to think about in this API. We should have a way of exposing a rollback to the user: this is a common use-case and I'm certain we can take notes from how apollo-react has gone about this problem.

What do you think, @AnthonyMDev?