1196239 - Sync engine for sync app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Sync, defect)

Description

[Michiel de Jong [:michielbdejong]]

The main sync cycle loop of the Synchronizer app (bug 1195647), that calls kinto to fetch data, then calls the adapters to update, then calls kinto again to push data.

Comment 1

[Michiel de Jong [:michielbdejong]]

I am working on this in https://github.com/michielbdejong/sync-engine/issues

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] michielbdejong:1196239-sync-engine > mozilla-b2g:master

Comment 3

[Sean Lee [:seanlee][:weilonge]]

I would like to test history adapter based on the demo code in Paris WW with your latest sync engine and crypto library. So it won't be blocked until all codes are landed.

Here are the files should be updated:
* sync engine - syncengine.js
* crypto library - fxsyncwebcrypto.js, keyderivation.js ,stringconversion.js
* kinto.js - kinto.dev.js (https://github.com/Kinto/kinto.js/blob/master/dist/kinto.dev.js)
* syncto in mozilla-services (https://github.com/mozilla-services/syncto @ branch implement_history_handling)

Could you help to check the list? Thank you. :)

Comment 4

[Michiel de Jong [:michielbdejong]]

(In reply to Sean Lee [:seanlee] from comment #3)
> I would like to test history adapter based on the demo code in Paris WW with
> your latest sync engine and crypto library. So it won't be blocked until all
> codes are landed.
> 
> Here are the files should be updated:
> * sync engine - syncengine.js
> * crypto library - fxsyncwebcrypto.js, keyderivation.js ,stringconversion.js
> * kinto.js - kinto.dev.js
> (https://github.com/Kinto/kinto.js/blob/master/dist/kinto.dev.js)
> * syncto in mozilla-services (https://github.com/mozilla-services/syncto @
> branch implement_history_handling)
> 
> Could you help to check the list? Thank you. :)

I have bug 1200539 for 'putting it all together'. The branch is michielbdejong/1200539-sync-app-integration-tests. Make sure to hard-code an assertion in https://github.com/michielbdejong/gaia/blob/1200539-sync-app-integration-tests/apps/sync/js/sync-credentials/sync-credentials-mock.js since the SyncCredentials part is not working yet.

Comment 5

[Michiel de Jong [:michielbdejong]]

Right, and as I said on irc, the other thing is that you currently need to run the 6-original-ids branch of syncto on http://localhost:8000/v1/ for this to work.

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8657304 [details] [review]
[gaia] michielbdejong:1196239-sync-engine > mozilla-b2g:master

Excellent work as usual :)

I left a few comments on the PR. I'd like to see them applied or discussed before looking into the tests.

Thank you!

Comment 7

[Michiel de Jong [:michielbdejong]]

@ferjm thanks a lot for the useful comments! I now made it so that you can do:

    let syncEngine = new SyncEngine(options);
    syncEngine.syncNow([ 'history' ]);

I fixed all your comments, except where I responded otherwise.

Comment 8

[Sean Lee [:seanlee][:weilonge]]

Thanks a lot for Michiel's integration branch, so I can base on the latest SyncEngine to develop History Adapter.
The one issue is that the new history records updated by Desktop won't be synced to the history collection at FxOS side when 'sync' button is clicked couple times.

If we do 'make clean && make' to rebuild gaia, the new records will be in the history collection. But it's only at the first time sync. Any following sync request from clicking 'sync' button won't change the history collection.

Michiel and I have some discussion about this on IRC already. We can have the solution discussion here as well.

Comment 9

[Michiel de Jong [:michielbdejong]]

(In reply to Sean Lee [:seanlee] from comment #8)
> the new history records updated by Desktop won't be
> synced to the history collection at FxOS side when 'sync' button is clicked
> couple times.

This is now fixed, you can either use the work-around in current https://github.com/michielbdejong/gaia/tree/1200539-sync-app-integration-tests (which simply leaves out the ?_since= header), or by cherrypicking Natim's timestamps patch from Friday on top of the 6-original-ids branch of Syncto. It will also work without workarounds and with Syncto master soon (probably this week).

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8657304 [details] [review]
[gaia] michielbdejong:1196239-sync-engine > mozilla-b2g:master

Thank you Michiel. I left some comments in the PR, but I think this is good to land. We can work on polishing it in follow up bugs once we start integrating all the parts.

Comment 11

[Fernando Jiménez Moreno [:ferjm]]

BTW, in the future, use the review flag instead of needinfo, please. I have different email filters for that :) Thanks!

Comment 12

[Michiel de Jong [:michielbdejong]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/31702

Fixed the else-after-return-or-throw thing, and replied on github to your other comments.

Comment 13

[Michiel de Jong [:michielbdejong]]

Rebased and squashed.

Comment 14

[Fernando Jiménez Moreno [:ferjm]]

https://github.com/mozilla-b2g/gaia/commit/994ff1537c2d7ca4d1658806c50f3ceba1053f9b