Destroyed model not being cleared after online sync #146

Open
pushkin- opened this Issue Jul 30, 2015 · 2 comments

Projects

None yet

2 participants

@pushkin-

Let's say I have POSTed an item and am online.
I go offline and delete it, and then go back online.
The item is deleted from the server. So far so good.

Then I go offline and back online. A syncDirtyAndDestroyed is triggered and the server tries to delete the item that it already deleted.

So it looks like after the online sync, the destroyed model id isn't being cleared from the store.

I will post more info later today.
Edit:
syncDirtyAndDestroyed is called which in turn calls syncDestroyed. After the model has been destroyed on the server, I would expect storeName_destroyed to not exist.

syncDestroyed gets the destroyed models (local not online it seems) by calling localStorage.getItem((getStoreName(this)) + "_destroyed"); under Backbone.Collection.prototype.syncDestroyed.

This shouldn't find anything, but it does, suggesting that localStorage.removeItem is not called.

I found localStorage.removeItem in two places - once when Store.prototype.clear is called and once when Store.prototype.destroy(model) is called, the latter I think is the one that is important here.

So when is store.destroy being called?

I would expect this to be called after the online sync is complete. (delete offline - removed from localstorage; go online - syncDestroyed - finds a _destroyed item and deletes from server; that model is removed so no model belongs to storeName_destroyed anymore; offline; online - syncDestroyed - find nothing).

It is being called in two places: Backbone.LocalStorage.sync and localsync.
I don't quite understand why a local sync should remove that item rather than an online sync.

What am I missing?
Though I understand you're busy, comments in the code would help.

@nilbus nilbus added Bug and removed Bug labels Aug 12, 2015
@nilbus nilbus self-assigned this Aug 12, 2015
@nilbus nilbus closed this in a6615a3 Aug 12, 2015
@nilbus
Owner
nilbus commented Aug 12, 2015

Mentioned the wrong issue in the commit that closed this. Whoops!

@nilbus nilbus reopened this Aug 12, 2015
@nilbus
Owner
nilbus commented Aug 12, 2015

store.clean is the call you're looking for. It wasn't obvious, because it can clean both dirty and destroyed records and doesn't contain either string.

Here's what generally happens (I'll go through syncDestoryed, but the same applies to syncDirty:

  1. syncDestroyed calls model.destroy()

  2. Destroying the model leads to dualsync's when 'delete'

  3. For models that have been successfully saved, localsync('delete', model, options)

    options.success = (resp, _status, _xhr) ->
      return useOfflineStorage() if hasOfflineStatusCode options.xhr
      localsync(method, model, options)
  4. localsync when 'delete' calls store.destroy(model) and then store.clean to clean either 1) 'dirty' because it had never been created online and was marked dirty for creation, or 2) 'destroyed' because it was created online and deleted offline. See this annotated source:

    when 'delete'
      store.destroy(model)
      # options.dirty implies that this delete is using offline storage
      if options.dirty && !model.hasTempId()
        store.destroyed(model)
      else # !options.dirty || model.hasTempId()
        if model.hasTempId()
          store.clean(model, 'dirty')
        else # !options.dirty
          store.clean(model, 'destroyed')

The two places I can see this might be going wrong:

  1. In step 3, return useOfflineStorage() if hasOfflineStatusCode options.xhr. I suspect that hasOfflineStatusCode(options.xhr) is returning for you, because it seems hasOfflineStatusCode was also triggering #145 for you. Let me know if you can confirm this in debugging. If hasOfflineStatusCode is returning true, then leaving the records marked dirty/destroyed is the intended behavior.
  2. There could be some edge case that I'm not thinking of for the logic in step 4 that causes something not to be cleaned that should.

comments in the code would help

noted. I also would like to improve the ease of understanding.

@nilbus nilbus removed the Bug label Aug 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment