Optimistic update fails if cache is empty

[awslabs]

Optimistic update fails if cache is empty

[freshtechs]

@rohandubal @palpatim
The only issue solved with previous PRs and merges was the incorrectly print of “missingValue” after the perform mutation operation. But still up to this moment if your very first fetch/request was made while offline you will get no results as response.

Just as @camerona93 explained.

[freshtechs]

@palpatim

Ok from my point of view:

Assuming you are offline, if the first thing you do after opening your app is perform a mutation of X model (i.e. you add your first Todo item) and while still offline you wish to view or edit it.

Expected Behavior: Perform optimistic updates mutations (CreateTodoMutation) and query the local cache for TodoItems to show you a list of your recently added/edited ones.

Actual behavior: Getting a nil result nil error while querying the local-cache (after successfully perform a optimistic-update mutation)

Conclusion: using the code pasted below (mutation() and loadTodos() funcs) you’re unable to actually get results of local cache.

[camerona93]

Is this issue still being investigated? I just encountered this as I was creating a light example app locally.

The repro path is pretty straight forward, I can include my example code here (mostly pulled directly from the integration docs):

In AppDelegate.swift

func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {
    // Override point for customization after application launch.
    do {
        // You can choose the directory in which AppSync stores its persistent cache databases
        let cacheConfiguration = try AWSAppSyncCacheConfiguration()
        
        // AppSync configuration & client initialization
        let appSyncServiceConfig = try AWSAppSyncServiceConfig()
        let appSyncConfig = try AWSAppSyncClientConfiguration(appSyncServiceConfig: appSyncServiceConfig,
                                                              cacheConfiguration: cacheConfiguration)
        self.appSyncClient = try AWSAppSyncClient(appSyncConfig: appSyncConfig)
        // Set id as the cache key for objects. See architecture section for details
        self.appSyncClient?.apolloClient?.cacheKeyForObject = { $0["id"] }
    } catch {
        print("Error initializing appsync client. \(error)")
    }
    // other methods
    return true
}

In ViewController.swift

//Reference AppSync client
var appSyncClient: AWSAppSyncClient?
{
    get
    {
        return (UIApplication.shared.delegate as! AppDelegate).appSyncClient
    }
}

override func viewDidLoad() {
    super.viewDidLoad()
    // Do any additional setup after loading the view, typically from a nib.
    
    let button = UIButton.init(type: .roundedRect)
    button.setTitle("Create Item", for: .normal)
    button.addTarget(self, action: #selector(createItem), for: .touchUpInside)
    button.frame = .init(x: 25, y: 250, width: 200, height: 100)
    
    let findButton = UIButton.init(type: .roundedRect)
    findButton.setTitle("List Items (cached)", for: .normal)
    findButton.addTarget(self, action: #selector(listItemsCached), for: .touchUpInside)
    findButton.frame = .init(x: 25, y: 450, width: 200, height: 100)
    
    self.view.addSubview(button)
    self.view.addSubview(findButton)
}

@objc
func createItem()
{
    let tempID = "temp"+UUID.init().uuidString
    let value = "I have a value of '" + tempID + "'"
    let mutation = CreateItemMutation(id: tempID, value: value)
    self.appSyncClient?.perform(mutation: mutation, optimisticUpdate: { (transaction) in
        do {
            try transaction?.update(query: GetItemsQuery()) { (data: inout GetItemsQuery.Data) in
                print("Handling optimistic update.")
                data.getItems?.append(GetItemsQuery.Data.GetItem.init(id: tempID, value: value))
            }
        } catch {
            print("Error updating cache with optimistic response")
        }
    }, resultHandler: { (result, error) in
        if let result = result {
            print("Added Response from service: \(String(describing: result.data?.createItem?.value))")
            //Now remove the outdated entry in cache from optimistic write
            let _ = self.appSyncClient?.store?.withinReadWriteTransaction { transaction in
                try transaction.update(query: GetItemsQuery())
                { (data: inout GetItemsQuery.Data) in
                    var pos = -1, counter = 0
                    for item in (data.getItems!) {
                        if item?.id == tempID {
                            pos = counter
                            continue
                        }; counter += 1
                    }
                    if pos != -1 { data.getItems?.remove(at: pos) }
                }
            }
        } else if let error = error {
            print("Error adding Item: \(error.localizedDescription)")
        }
    })
}

@objc
func listItemsCached()
{
    appSyncClient?.fetch(query: GetItemsQuery(), cachePolicy: .returnCacheDataAndFetch) {
        (result, error) in
        if error != nil {
            print(error?.localizedDescription ?? "")
            return
        }
        print("Results:")
        result?.data?.getItems?.forEach { print($0!.value) }
    }
}

If you set up a simple iOS app like this, launch it for the first time with internet connectivity disabled, press the "Create Item" button, and then press the "List Items (cached)" button, you'll get no results.

However, if you open the application with internet, press "List Items (cached)", turn off the internet, and then press "Create Item" then subsequent presses of "List Items (cached)" will return results of the optimistic update. I've attached two gifs below showing the difference in behavior with the example code I've provided:

Example with no internet connection for duration of demonstration: Note that the code for listItemsCached is just returning an error, no cached results, even after a mutation is performed.

This below example has internet turned on ONLY for the first press of "List Items (Cached)", then I turned it off and you can observe the expected behavior.

Happy to help provide any additional info if needed!

[sdeff]

I'm facing the same problem.

When I run the Event App sample for the very first time and with airplane mode enabled, I get a "missingValue" after executing "transaction?.update(...)" whenever I try to add a new post:

    appSyncClient?.perform(mutation: addEventMutation, optimisticUpdate: { (transaction) in
        do {
            // Update our normalized local store immediately for a responsive UI.
            try transaction?.update(query: ListEventsQuery()) { (data: inout ListEventsQuery.Data) in
                data.listEvents?.items?.append(
                    ListEventsQuery.Data.ListEvent.Item(id: UUID().uuidString,
                                                        description: descriptionText,
                                                        name: nameText,
                                                        when: whenText,
                                                        where: whereText,
                                                        comments: nil))
            }
        } catch {
            print("Error updating the cache with optimistic response.")
        }
    }

However, when I force close the app and start it with airplane mode disabled, the previously added posts appear.

[freshtechs]

This may be a bug? @rohandubal

[rohandubal]

@sdeff @freshtechs There was a known bug which was fixed in 2.6.22 which did not allow writing to cache for queries with parameters or with objects with null values.

Does this happen only on queries which have never been fetched from the network or for any query?

[freshtechs]

@rohandubal Happens when you try to perform an optimistic update mutation prior a fetch request of the same object/model (from cache or network i tried with all the keys in CachePolicy).

If the first action and first launch of your app involves an optimistic update the perform operation would fail to the “do try transaction?.update...” catch statement printing “missingValue” as error value

[rohandubal]

Thanks for the information @freshtechs

I will look into this.

[larryonoff]

Just checked ApolloStore code. It looks that the behaviour you describe is by design. See a code below

    public func update<Query: GraphQLQuery>(query: Query, _ body: (inout Query.Data) throws -> Void) throws {
      var data = try read(query: query)
      try body(&data)
      try write(data: data, forQuery: query)
    }

It tries to read the query which I assume fails since the query not exist in cache.

PS: I think that this's invalid behaviour and would be great to change this behaviour in Apollo.

[shannon-hager-skookum]

PS: I think that this's invalid behaviour and would be great to change this behaviour in Apollo.

I agree. This behavior is part of why we moved completely away from using AppSync local cache for anything other than offline mutation caching.

[larryonoff]

@shannon-hager-skookum could you share your solution? Our team considers custom cache solution for AppSync if we won't move away from AWS AppSync solution (it really buggy with slow support).

BTW. I've rewritten perform mutation mechanism in PR #109. It is not yet accepted by Amazon.

[shannon-hager-skookum]

We created a CoreData layer for local cache, optimistic updates, etc. There are still issues caused by the appSync local cache (I really wish we could disable it instead of simply ignoring it) but we never check that cache, never manually update it, and are mostly unaffected by it. It is a lot more work to do it the way we did, obviously, but it works.

[larryonoff]

@shannon-hager-skookum it would be great having some generic solution for, with support of CoreData / Realm / SQLite

I assume that to disable local cache you just should just set fileURL to nil for AppSync.

[shannon-hager-skookum]

If that did work, it seems like it would also disable offline mutation caching, which is one of the biggest benefits to AppSync (imo).

[larryonoff]

@shannon-hager-skookum the biggest benefit doesn't work properly. As for me the only thing why AppSync is reasonable to use is that you don't need configure web client.

[freshtechs]

@palpatim why the console logs “missingValue” while doing this?

[palpatim]

@freshtechs

That's the issue I'm looking at. I have a test that replicates this issue and am working on figuring out the proper way to handle it.

[larryonoff]

@palpatim please check my comment #92 (comment)

This the issue from Apollo. So Apollo code needs to be changed.

[palpatim]

I'm concerned about changing the Apollo code for a couple of reasons:

1. Changing it at or near the low-level readObject call doesn't give us insight into the structure that is being read. That means we don't have a good way to return an empty result for whatever query is being requested.
2. Changing it at the execute level or above could hide real problems below, and makes me worry about unintended side effects.
3. I don't have insight into the Apollo team's thought process around this behavior. If it was intentional, it may break something else in some way (see above re: unintended side effects).

Instead, I'm leaning toward prepopulating our records cache with an empty QUERY_ROOT when we create the cache. Since all caches are backed by a SQLite table (either persistent or in-memory), I can localize the change in the createTableIfNeeded method and wrap the prepopulation in a check such that it only happens if there are no "QUERY_ROOT" rows in the table.

[freshtechs]

Ok thanks! How can I use that branch on my existing Xcode project?

[palpatim]

@freshtechs I haven't merged it yet because I'm still testing (and haven't actually confirmed that this fixes the issue properly). Once I'm able to verify behavior, I'll merge to master in preparation for the next release (which would probably be a few days after I merge).

Alternately, you can build AppSync into your project from the master branch. Since you're using CocoaPods, you'd have to download the project to your build system and adjust your Podfile to specify a :path to AppSync, rather than using the normal pod declaration.

[palpatim]

This is now merged into master and pending the next release. We're planning on doing that later this week.

[palpatim]

The fix for this impacts the behavior of the .returnCacheDataElseFetch and .returnCacheDataDontFetch cache policies on queries against empty caches. I'm working on a fix now, tracking on #181

[palpatim]

This fix is released in 2.10.1. Please let us know if you continue to see issues.

[freshtechs]

The optimistic update it doesn't fail anymore to "missingValue" but I will eventually send it to the network instead of saving it first on cache/store.

Im using defaults cache/service config on the client:

let cacheConfiguration = try AWSAppSyncCacheConfiguration()
let appSyncServiceConfig = try AWSAppSyncServiceConfig()

Im creating a list before appending the new item to cache, but when you fetch/query with .returnCacheDataDontFetch as cachePolicy it will return an empty list as result.

appSyncClient?.perform(
    mutation: carMutation,
    optimisticUpdate: { (transaction) in
        do {
            try transaction?.update(query: ListCarsQuery()) { (data: inout ListCarsQuery.Data) in
                var listCars: [ListCarsQuery.Data.ListCar.Item?] = data.listCars?.items ?? []
                let localItem = ListCarsQuery.Data.ListCar.Item(
                    id: id, make: make, model: model, year: year, licensePlate: licensePlate,
                    initialOdometer: initialOdometer, currentOdometer: currentOdometer,
                    imageFileName: imageFileName, isFavorite: isFavorite, timestamp: timestamp)
                listCars.append(localItem)
            }
        } catch {
            print(error)
        }
})
print("succesfully created car about to query it from cache")
appSyncClient?.fetch(query: ListCarsQuery(), cachePolicy: .returnCacheDataDontFetch) {result, error in
    if let error = error {
        print(error.localizedDescription)
        return
    }
    print(result?.data?.listCars?.items ?? [])
}

[freshtechs]

Same results using the docs (Todos) app :

override func viewDidLoad() {
    super.viewDidLoad()
    let appDelegate = UIApplication.shared.delegate as! AppDelegate
    appSyncClient = appDelegate.appSyncClient
    mutation()
}

func mutation() {
    let mutationInput = CreateTodoInput(name: "Use AppSync", description:"Realtime and Offline")
    let createTodoMutation = CreateTodoMutation(input: mutationInput)
    let UUID = NSUUID().uuidString
    
    appSyncClient?.perform(mutation: createTodoMutation, optimisticUpdate: { (transaction) in
        do {
            try transaction?.update(query: ListTodosQuery()) { (data: inout ListTodosQuery.Data) in
                var listTodos: [ListTodosQuery.Data.ListTodo.Item?] = data.listTodos?.items ?? []
                listTodos.append(ListTodosQuery.Data.ListTodo.Item(id: UUID, name: mutationInput.name, description: mutationInput.description!))
                data.listTodos?.items = listTodos
            }
        } catch {
            print("Error updating cache with optimistic response for \(error)")
        }
    })
    loadTodos()
}

func loadTodos() {
    appSyncClient?.fetch(query: ListTodosQuery(), cachePolicy: .returnCacheDataDontFetch)
    { (result, error) in
        if error != nil {
            print(error?.localizedDescription ?? "")
            return
        }
        result?.data?.listTodos?.items!.forEach { print(($0?.name)! + " " + ($0?.description)!) }
    }
}

[palpatim]

I notice you're doing your fetch (in both the first example and the "Todos" app, immediately after invoking the perform. However, since mutations are asynchronous, there's an extremely good chance that the mutation will not have completed (or perhaps even begun) by the time the query starts. Can you try invoking your fetches after the mutation's optimistic update has completed?

Revised example from Todos app:

func mutation() {
    let mutationInput = CreateTodoInput(name: "Use AppSync", description:"Realtime and Offline")
    let createTodoMutation = CreateTodoMutation(input: mutationInput)
    let UUID = NSUUID().uuidString
    
    appSyncClient?.perform(mutation: createTodoMutation, optimisticUpdate: { (transaction) in
        do {
            try transaction?.update(query: ListTodosQuery()) { (data: inout ListTodosQuery.Data) in
                var listTodos: [ListTodosQuery.Data.ListTodo.Item?] = data.listTodos?.items ?? []
                listTodos.append(ListTodosQuery.Data.ListTodo.Item(id: UUID, name: mutationInput.name, description: mutationInput.description!))
                data.listTodos?.items = listTodos
            }
            // Local cache is updated after transaction block (which is synchronous) completes
            loadTodos()
        } catch {
            print("Error updating cache with optimistic response for \(error)")
        }
    })
}

(Your question prompted me to look more closely at my tests--I see that the optimistic update unit test doesn't wait for the block to finish before asserting the cache is updated, which means I've just been lucky that our tests aren't hitting the same problem you describe! Thanks for pointing this out.)

[freshtechs]

Tried with your suggestion, but still the result var of the closure of loadTodos() fetch is nil. Also I had to insert self.loadTodos() because was calling loadTodos() inside of optimisticUpdate/transaction closure.

[freshtechs]

Also is weird that im seeing the todos items being push to the DynamoDB table successfully, without me adding the resultHandler: { (result, error) in optional-closure at the end of the .perform func

[palpatim]

Also I had to insert self.loadTodos() because was calling loadTodos() inside of optimisticUpdate/transaction closure.

Thanks for catching that!

Also is weird that im seeing the todos items being push to the DynamoDB table successfully, without me adding the resultHandler: { (result, error) in optional-closure at the end of the .perform func

The results you're seeing in the local cache may be the result of that update. To disambiguate that, you can make the ID of your optimistically updated Item easy to distinguish (as for example by using an ID of let UUID = "TEMP-\(UUID().uuidString)". That lets you tell at a glance whether an item in your cache is local-only or server-generated.

Also note that the usual pattern for creating new items is to have the server generate the ID, which means that in the result block (assuming it's successful), you would have to reconcile the cache, for example by removing the optimistic-update record and adding server-provided record. If your creation pattern allows for an object to be fully populated by the client and stored-as-is by the server, then no such reconciliation would be necessary.

[freshtechs]

@palpatim

Thanks for guiding me for the ids case, but actually the pattern I’m using would be generating the ids on the client side, so it won’t be necessary to create a temporary id for later replacement.

And about the fetch i think that I miss-explained myself. I’m seeing the results on the DynamoDB instance instead of the fetch-query-result with “only cache” policy passed to it. Anytime I fetch query the local cache i get a nil result.

[palpatim]

@freshtechs

I'm stumped by the behavior you're seeing--we have some tests (some that I recently added, in fact) to verify that query cache policies work as expected. There are some known failures around cache keys with dots that are fixed in #186, which is currently under review and should go out in the next release. Are you able to pull that PR's branch locally and see if that fixes the query behavior you're seeing querying the cache?

If you're not using dots in the cache keys (which includes both locally-generated IDs (e.g., post.id = "123-4567-890ab"), and arguments passed to queries (e.g., let query = ListPostQuery(id: "123-4567-890ab")), could you open a new issue with a code sample that replicates the problem? I'm wondering if there's a use case we're not catching. I do see that our tests for mutations without parameters don't cover optimistic updates and/or caching behavior for those kind of mutations, so that might be where the problem is coming up.

[freshtechs]

Im not using dots and not passing arguments to queries. The mutation it successfully gets its way to the dynamoDB instance but never to the local cache, for reference im using the events app/code used on the docs/unit tests:
(I first pod-update to latest release 2.10.2)

import UIKit
import AWSAppSync

class ViewController: UIViewController {
    
    var appSyncClient: AWSAppSyncClient?

    override func viewDidLoad() {
        super.viewDidLoad()
        let appDelegate = UIApplication.shared.delegate as! AppDelegate
        appSyncClient = appDelegate.appSyncClient
        mutation()
    }
    
    func mutation() {
        let mutationInput = CreateTodoInput(name: "Use AppSync", description:"Realtime and Offline")
        let createTodoMutation = CreateTodoMutation(input: mutationInput)
        let UUID = NSUUID().uuidString
        
        appSyncClient?.perform(mutation: createTodoMutation, optimisticUpdate: { (transaction)  in
            do {
                try transaction?.update(query: ListTodosQuery()) { (data: inout ListTodosQuery.Data) in
                    var listTodos: [ListTodosQuery.Data.ListTodo.Item?] = data.listTodos?.items ?? []
                    listTodos.append(ListTodosQuery.Data.ListTodo.Item(id: UUID, name: mutationInput.name, description: mutationInput.description!))
                    data.listTodos?.items = listTodos
                }
                // Local cache is updated after transaction block (which is synchronous) completes
                self.loadTodos()
            } catch {
                print("Error updating cache with optimistic response for \(error)")
            }
        })
    }
    
    func loadTodos() {
        appSyncClient?.fetch(query: ListTodosQuery(), cachePolicy: .returnCacheDataDontFetch)
        { (result, error) in
            if error != nil {
                print(error?.localizedDescription ?? "")
                return
            }
            result?.data?.listTodos?.items!.forEach { print(($0?.name)! + " " + ($0?.description)!) }
        }
    }
    
}

[freshtechs]

The only way the data its cached to store is when I change the cachePolicy to

CachePolicy.returnCacheDataElseFetch

The results of the mutation are never cached, no matter if it's optimistic or not.

So the only data that gets cached is the results of a fetch query to the network.

[btn52396]

I'm experiencing the same issue as @freshtechs. If the cache is empty, it cannot be written to unless first fetched from the server. Once the data is fetched from the server and the cache is filled, optimistic update works as expected. I'll create a sample replication when I get the chance

[palpatim]

@freshtechs

I'd like to clarify what we mean by "cache". The underlying cache implementation stores results of its operations under three different "roots": one for queries, one for mutations and one for subscriptions. If you're expecting the results of a mutation to update the cache for a query, you would have to manually update that in the mutation's result handler.

Can you confirm that the code you're using above demonstrates the following behavior?

1. Perform mutation
2. Optimistically update cache
3. Query against local cache (query with cache policy of .returnCacheDataDontFetch)
   Expected: receive cached results of optimistic update
   Actual: Receive cached results of optimistic update
4. Receive server-generated results of mutation
5. Query against local cache (query with cache policy of .returnCacheDataDontFetch)
   Expected: receive cached results of mutation received in Step 4
   Actual: Receive cached results of optimistic update, instead of the overwritten results

If so, that's the behavior I'd expect. To update a local queries cache, you'd have to insert a step 4.1:

4.1. Reconcile local "queries" cache with the results of the mutation, similar in structure to that peformed during an optimistic update.

But I may be misunderstanding the behavior you're seeing.

At the risk of making you dive into internals, you can verify cache behavior by inspecting the database(s) used by your AppSync client. Assuming you're using AWSAppSyncCacheConfiguration to configure your client, when you view the queries.db database, you should be able to see results with queries like:

SELECT * FROM records WHERE key='QUERY_ROOT';
-- expected: a field with cache keys that refer to of Query operations from server,
-- and of optimistic updates of mutations. Notably, I would NOT expect to see
-- automatically-populated results of queries as the result of performing a mutation

SELECT * FROM records WHERE key='MUTATION_ROOT';
-- expected: service-provided result sets from Mutation operations

SELECT * FROM records;
-- expected: a list of records including the above QUERY_ROOT and MUTATION_ROOT
-- keys, plus individual object results keyed by their `cacheKeyForObject` results

If I'm understanding the situation accurately, the misunderstanding is the (quite natural) assumption that a mutation which creates/updates an object on the server would automatically update the local cache with the results of that data, such that a query against the local cache would return it. Unfortunately, because GraphQL operates on fairly loosely defined selection sets, it is necessary for you as the programmer to fill in the gap and update local query results. Essentially, you're saying "I know the selection set from this mutation can update the following queries: GetTodo, ListTodos, ... etc".

Does this clarify matters, confuse matters, or am I missing your point? If the last, I'll see if I can run the above code and either replicate, or confirm that it is behaving as I expect.

[palpatim]

@btn52396

Can you confirm that you're still seeing that behavior using the latest version of AppSync? If so, a repro sample would be much appreciated so we can add the case to our tests.

[btn52396]

@palpatim
Yes I'm using the latest version of AppSync. I always use the latest version before reporting a bug.
Here is a sample that reproduces the bug https://github.com/btn52396/aws-mobile-appsync-events-starter-ios

Before running the sample app, make sure that you remove the events sample app from your iPhone simulator if there is a copy there to clear any previous cache. Then just fill in your API info and run it.

I modified the EventListViewController so that it only fetches from the cache. It will never fetch from the server. The purpose of this modification is to mimic the scenario where the user installs the app, goes offline, and then tries to make a mutation without being able to fetch from the server first. If you try to add an event, you will find the that the event was sent to the server but not updated in the cache and will provoke the error message "Local cache unexpectedly has no results for ListEventsQuery."

Let me know if you need anything else :)

[mjaydeep01]

Hey All,

Any updates on this issue of first mutation (create item) while offline, and not able to access it by list query?

[SunandaRamineni]

@rohandubal I am also facing the same issue, when performing mutation of a new record in offline. Cache data is returning nil to update. Can you please reply on how to overcome or any version updates with fix?

[grevolution]

@rohandubal any update on the issue above?

[grevolution]

Hey All,

Any updates on this issue of first mutation (create item) while offline, and not able to access it by list query?

@mjaydeep01 did you get around with this problem?

[lawmicha]

I opened up an new issue to make it easier to track this issue here: #390 forking from the detailed repro steps (thanks @camerona93)