Closed Bug 1219621 Opened 9 years ago Closed 9 years ago

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


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

Gonk (Firefox OS)


(Not tracked)

FxOS-S11 (13Nov)


(Reporter: mbdejong, Assigned: mbdejong)




(1 file)

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?
Assignee: nobody → mbdejong
Blocks: fxos-sync
Priority: -- → P2
Target Milestone: --- → FxOS-S10 (30Oct)
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.
Priority: P2 → P3
Target Milestone: FxOS-S10 (30Oct) → FxOS-S11 (13Nov)
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.
Priority: P3 → P1
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).
Attachment #8683077 - Flags: review?(ferjmoreno)
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 ;)
Flags: needinfo?(mbdejong)
Attachment #8683077 - Flags: review?(ferjmoreno)
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.
Flags: needinfo?(mbdejong)
Follow-up for improving efficiency in case of local deletions is bug 1223418.
Flags: needinfo?(ferjmoreno)
And code comment added.
Flags: needinfo?(ferjmoreno)
Attachment #8683077 - Flags: review+
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.