1219621 - Decide how readonly DataAdapters should deal with local deletions on the phone

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Sync, defect, P1)

Description

[Michiel de Jong [:michielbdejong]]

In the readonly DataAdapters for the TV, whenever the DataStore is cleared (this happens on logout), remote data is reimported on next sync.

On the phone, it's also possible that data that was previously imported in readonly mode is later cleared locally. For instance, when the user clears the browser history.

I think it's not entirely "clear" ;) what should happen in this case. Should sync then only ever import records that were newly changed, for the rest of the lifetime of the device? Or reimport everything?

Comment 1

[Michiel de Jong [:michielbdejong]]

As data is imported, lastRevisionId should be updated, so that the sync cursor moves along with the changes caused by the import. Maybe already do a DataStore#sync after each add/update/delete, confirm that the action was successful (no conflict), and update lastRevisionId.

Then, on next sync, the DataAdapter should check for new tasks from DataStore#sync, and:
* update the matching table for record additions (hope we can query the kintoCollection by URL)
* make sure that data that was imported earlier but then destroyed in a local action, is re-imported

For the 2.5 TV the only tasks that come in will be DataStore#clear, and we're already dealing with that, so marking this bug as P3 (from the TV's point of view) / Sprint 11.

Comment 2

[Michiel de Jong [:michielbdejong]]

Just like we're already doing for 'clear', for every 'remove' and every 'update' task that removes the previously synced data, the DataAdapter should look up the record(s) affected and re-establish them in the DataStore. If the user doesn't want this to happen, then they can disable sync for that collection.

The loop that does this (currently called checkIfClearedSince) can be the same loop as the one that syncs up for two-way sync.

Comment 3

[Michiel de Jong [:michielbdejong]]

We would need a two-way matching table to do this properly. For now, I'll just re-run the whole Kinto collection if any records were removed (just like when the collection was cleared), and ignore the situation where DataStore records were updated (I don't think there would ever be an update that removes visits from a history entry, or that removes an entry from fxSyncRecords in a bookmark).

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] michielbdejong:1219621-resync-after-delete > mozilla-b2g:master

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

I'm sorry, I don't follow your reasoning here. IIUC what this patch does is doing a full re-import every time a single item is removed from the bookmarks or history datastores. Is that the case?

Again, in code comments would be really helpful ;)

Comment 6

[Michiel de Jong [:michielbdejong]]

Yes. It assumes that removing history entries one-by-one will not happen often. But I agree it's not ideal once we launch bookmarks UI on the phone. I'll add some code comments, and create a follow-up ticket to remind us that we need something better than this before launching bookmarks sync on the phone.

Comment 7

[Michiel de Jong [:michielbdejong]]

Follow-up for improving efficiency in case of local deletions is bug 1223418.

Comment 8

[Michiel de Jong [:michielbdejong]]

And code comment added.

Comment 9

[Fernando Jiménez Moreno [:ferjm]]

https://github.com/mozilla-b2g/gaia/commit/f8ad4581c5804053afc79ea0ad0a7c692f55ce94