Regression in behavior of queries against empty cache

[palpatim]

Describe the bug

PR #177, which is intended to fix #92 and #101, regresses the behavior of queries against an empty cache.

To Reproduce
Steps to reproduce the behavior:

• Duplicate the existing test FetchQueryTests.testReturnCacheDataAndFetch to a test named testReturnCacheDataAndFetchWithMissingData
• Modify all FetchQueryTests test*WithMissingData to set the initialData to:

      let initialRecords: RecordSet = ["QUERY_ROOT": [:]]

• The tests now fail because they are returning a result instead of nil. That causes the .returnCacheDataDontFetch policy to return an empty result instead of a nil, and causes .returnCacheDataElseFetch to return an empty result instead of falling through to server results. (This last is similar behavior as described in returnCacheDataElseFetch returning nil instead of fetching #90, except that it was previously returning nil and not falling through to server results)

Expected behavior

• Querying against a cache that does not produce results (either a nil result, or an empty result set) for a query should behave as follows:
  • .returnCacheDataAndFetch: Does not return a value from cache, returns results from server
  • .returnCacheDataDontFetch: Return a nil value, cease processing
  • .returnCacheDataElseFetch: Does not return a value from cache, returns results from server

Obviously, whatever fix is in place should also maintain the correct behavior for a cache that contains a query hit:

• .returnCacheDataAndFetch: Returns a value from cache, and returns results from server
• .returnCacheDataDontFetch: Return a value from cache, cease processing
• .returnCacheDataElseFetch: Return a value from cache, cease processing

Essentially we now have different conditions leading to a cache miss that we need to handle:

• Cache is completely empty (this shouldn't happen in the current code if we initialize the cache via the AppSync code paths, but we should test it to ensure correct behavior):

  let initialRecords: RecordSet = [:]

• Cache is populated with an initial QUERY_ROOT, to support optimistic updates:

  let initialRecords: RecordSet = ["QUERY_ROOT":[:]]

• Cache is populated with results that don't fulfill the existing query:

  let initialRecords: RecordSet = [
      "QUERY_ROOT": ["hero": Reference(key: "hero")],
          "hero": [
              "name": "R2-D2",
              "__typename": "Droid",
      ]
  ]

[freshtechs]

Hey @palpatim

One of our app models is intended to be used offline-only and because of that, we use the .returnCacheDataDontFetch cache-policy combined with optimistic updates for requests and queries.

Can we assert that once this issue gets fixed that could be possible?

[palpatim]

@freshtechs

I don't have specific tests around that use case, but yes, that should work as expected. Do note that optimistic updates against an empty cache will still require you to populate a list structure to begin with, as in this example from MutationOptimisticUpdateTests:

appSyncClient.perform(
    mutation: addPost,
    queue: MutationOptimisticUpdateTests.mutationQueue,
    optimisticUpdate: { transaction in
        guard let transaction = transaction else {
            XCTFail("Optimistic update transaction unexpectedly nil")
            return
        }
        do {
            try transaction.update(query: ListPostsQuery()) { data in
                // Note this line checks for a `nil` list structure -----v
                var listPosts: [ListPostsQuery.Data.ListPost?] = data.listPosts ?? []
                listPosts.append(newPost)
                data.listPosts = listPosts
            }
            // The `update` is synchronous, so we can fulfill after the block completes
            optimisticUpdatePerformed.fulfill()
        } catch {
            XCTFail("Unexpected error performing optimistic update: \(error)")
        }
})

[palpatim]

This fix is released in 2.10.1.