Closed Bug 1207468 Opened 4 years ago Closed 4 years ago

Prefix asyncStorage entries with userid in DataAdapters

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(Not tracked)

RESOLVED FIXED
FxOS-S10 (30Oct)

People

(Reporter: mbdejong, Assigned: mbdejong)

References

Details

Attachments

(1 file)

This is a follow-up to bug 1191773.

The way it uses an asyncStorage data base to map FxSync Id <-> DataStore Id takes up disk space and processing time, so maybe we can do something smarter.

Suggestion 1: Implement and use https://github.com/Kinto/kinto.js/issues/173
Suggestion 2: Store records in the DataStore by their FxSync Id (what would be the implications of that for other apps? would a data migration be needed)?
Suggestion 3: ... ?
Assignee: nobody → mbdejong
Blocks: 1195647
Target Milestone: --- → FxOS-S9 (16Oct)
Re suggestion 2: We're currently using the URL as the id in DataStore, so that's obviously not going to fly if we change that. https://github.com/weilonge/gaia/blob/seanlee/DataSync/master/Bug1191773/apps/sync/js/adapters/history.js#L114
Possible suggestion 3: A 'dry run' option in Kinto.js `Collection#sync`. That would allow us to inspect the SyncResults object from Kinto.js to know what to change in the DataStore (while still having access to the unchanged records). We could then apply the incoming changes one-by-one as we process them, meaning that if something goes wrong before we processed the whole SyncResults (e.g. app is brutally killed), we will still get those unprocessed changes on next sync. The ones that we did apply will show up as 'skipped' in the next sync. It's not so efficient in updating Kinto.js's lastModified timestamp, and will lead to extra bandwidth usage (which is not nice on mobile), but maybe we can iterate over this thought.
Suggestion 4 (variation on suggestion 3): Pass a second RemoteTransformer to Kinto.js, from which the adapter can inspect each incoming change before it gets committed.
Hey Michiel,

Thanks for your suggestions. :)
For suggestion 4, I checked RemoteTransformer example http://kintojs.readthedocs.org/en/latest/api/#listing-records .
Is it possible to get the original URL (aka. DataStoreID) for deleted: true record?

class MyRemoteTransformer extends Kinto.transformers.RemoteTransformer {
  encode(record) { <<<  Will it contain URL information here?
    return update(record, {title: record.title + "!"});
  }

  decode(record) {
    return update(record, {title: record.title.slice(0, -1)});
  }
}
Correct my above comment, I think the URL information would be at decode function rather than encode, and I suppose that URL will be in payload object:

  Delete Records from History Collection (HC): {
    "id": "_Avscjx5srFy",
    "sortindex": 100,
    "last_modified": 1441985077970,
    "payload": {
      "url": "http://mozilla.org",
      "id": "_Avscjx5srFy",
      "deleted": true
    },
    "_status": "synced"
  }
New insight: https://github.com/Kinto/kinto.js/issues/173#issuecomment-145007083

Seems nicer than spying through a RemoteTransformer.
Depends on: 1210356
Target Milestone: FxOS-S9 (16Oct) → FxOS-S10 (30Oct)
Priority: -- → P3
As discussed today, we will store this mapping in localStorage, and prefixed with the email address of the logged in user. On the TV (but not on the phone), this data will be deleted when the user logs out.
Summary: Make FxSync Id <-> DataStore Id mapping more efficient in History Adapter → Store FxSync Id <-> DataStore Id mapping prefixed with user email in History Adapter
Duplicate of this bug: 1214496
Attachment #8676186 - Flags: review?(ferjmoreno)
Summary: Store FxSync Id <-> DataStore Id mapping prefixed with user email in History Adapter → Prefix asyncStorage entries with userid in DataAdapters
Priority: P3 → P1
Comment on attachment 8676186 [details] [review]
[gaia] michielbdejong:1207468-fxsyncid-datastoreid-mapping > mozilla-b2g:master

Treeherder shows an ESL error, but I can't see why?
(In reply to Michiel de Jong [:michielbdejong] from comment #10)
> Comment on attachment 8676186 [details] [review]
> [gaia] michielbdejong:1207468-fxsyncid-datastoreid-mapping >
> mozilla-b2g:master
> 
> Treeherder shows an ESL error, but I can't see why?

Ah, green after a rebase.

Please review.
Flags: needinfo?(ferjmoreno)
Flags: needinfo?(ferjmoreno)
Attachment #8676186 - Flags: review?(ferjmoreno) → review+
IIRC, when the Sync server is flushed, kB and xClientState will change, right? We should find out if the fxSyncIds also change in this case. If they do, then using xClientState here is correct, but if not then it's more correct to use the email address of the current user.
(In reply to Michiel de Jong [:michielbdejong] from comment #12)
> IIRC, when the Sync server is flushed, kB and xClientState will change,
> right? We should find out if the fxSyncIds also change in this case. If they
> do, then using xClientState here is correct, but if not then it's more
> correct to use the email address of the current user.

Created P2 follow-up bug 1218286 about this.
Incorporated your first review comment https://github.com/mozilla-b2g/gaia/pull/32596#discussion_r42936908 and created a follow-up bug for your second review comment https://github.com/mozilla-b2g/gaia/pull/32596#discussion_r42936922. Rebased, tests are green.
Flags: needinfo?(ferjmoreno)
https://github.com/mozilla-b2g/gaia/commit/4c78c9ea90b1a89734df246ccfa374f4f0aafe5b
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(ferjmoreno)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.