In v1.3 all FB Data will live in a DataStore. For users convenience We will need to migrate older versions data present in the old indexedDB to the new DataStore. We need to decide how and when to do this: A/ At runtime B/ At any point in build time / OS update time Just asking first for feedback on them best way to do it

> A/ At runtime This requires some UI... because all the work has to be done in the main thread. Then, how to be sure that the user doesn't kill the app before the ending of the operation? > B/ At any point in build time / OS update time If we can, I prefer this. Fabrice, what do you think about this?

I can't see how this can happen at build time, since users won't rebuild themselves. So the best option is to the migration at first run after update. Since it's application specific this should happen in gaia though...

triage: this is targeted for 1.3. would not block 1.3. please land it in master directly when ready

(In reply to Joe Cheng [:jcheng] from comment #3) > triage: this is targeted for 1.3. would not block 1.3. please land it in > master directly when ready if we don't have this in v1.3 th euser impact would be high as he will lose all his Facebook Data, she will have to re-import re-link

Comment on attachment 8338344 [details] 14051.html Jose, this is looking pretty good, but I have a few concerns I put on Github. In particular, I think given the amount of code here we probably should have a unit test for datastore_migrator.js. I'd like to look at it again before landing, so r- for now. I also agree this should be a blocker for 1.3 if FB datastore is shipping in 1.3. Thanks!

triage; per comment 5, 1.3+ to get it into v1.3

Comment on attachment 8338344 [details] 14051.html Hi Ben, Now we have tests and the code is much more robust. Please could you re-review it? thanks!

Thanks Jose. I will review this first thing tomorrow morning. I was caught up in a long bisection today that would have been painful to interrupt.

Comment on attachment 8338344 [details] 14051.html While I had difficulty testing this on my device due to bug 938406, the code looks good and the tests pass. I did have a question about how the user recovers when errors occur. Should they just re-sync with FB? Can we automatically do that in the background if they hit an error during migration? Also, I think we should start using the following pattern in async test code: done(function() { // test some code that might assert or throw }); This makes sure mocha properly reports errors even if its in an async context. Anyway, if we can address those items, r=me. Also, I think we should add the verifyme keyword after this lands to have QA double check that a migration from a 1.2 build to 1.3 works as expected. Thanks!

(In reply to Ben Kelly [:bkelly] from comment #10) > Comment on attachment 8338344 [details] > 14051.html > > While I had difficulty testing this on my device due to bug 938406, the code > looks good and the tests pass. > > I did have a question about how the user recovers when errors occur. Should > they just re-sync with FB? Can we automatically do that in the background > if they hit an error during migration? > Yes, we can recover from errors. See for instance this test https://github.com/mozilla-b2g/gaia/pull/14051/files#diff-ee495aa1f222b7770cd216c23d62cddeR236 > Also, I think we should start using the following pattern in async test code: > > done(function() { > // test some code that might assert or throw > }); > I've never seeing that pattern in Gaia code nor in Mocha examples I know of. Probably you are meaning that we should assign error callbacks to done as done can take an error object as parameter and avoid calling extra assert.fail. That's what I actually have done. > This makes sure mocha properly reports errors even if its in an async > context. > > Anyway, if we can address those items, r=me. > > Also, I think we should add the verifyme keyword after this lands to have QA > double check that a migration from a 1.2 build to 1.3 works as expected. > > Thanks!

QA. To verify this bug the following steps must be done: A/ Have a Firefox OS 1.2 device. Import FB Contacts B/ Trigger a FOTA for v1.3. C/ Open the Contacts App and stay idle for about 5 seconds. Initially you will see that FB data is not loaded. Then the migration should start (see adb logcat) and finally the FB data should be appearing for those contacts on the screen and the rest. Ni Isabel to make her aware of this.

(In reply to Jose M. Cantera from comment #11) > > Also, I think we should start using the following pattern in async test code: > > > > done(function() { > > // test some code that might assert or throw > > }); > > > > I've never seeing that pattern in Gaia code nor in Mocha examples I know of. > Probably you are meaning that we should assign error callbacks to done as > done can take an error object as parameter and avoid calling extra > assert.fail. That's what I actually have done. I had not seen in widespread use in gaia, but I also see plenty of cases in gaia where an assert fires and we don't get a stack trace. Its kind of annoying to solve the problem when that happens. This pattern was recently shown to me by gsvelto. It basically wraps the function passed to done() in a try/catch so any exceptions are properly reported. It seems a bit more elegant than re-writing try/catch everywhere.

ni to John Ford, in case he can help us with the uplift to v1.3 branch. It seems the specific v1.3 status flag has not been created yet ("status-b2g-v1.3") so we can't mark v1.3 as "affected" Thanks!

(In reply to Noemí Freire (:noemi) from comment #15) > ni to John Ford, in case he can help us with the uplift to v1.3 branch. It > seems the specific v1.3 status flag has not been created yet > ("status-b2g-v1.3") so we can't mark v1.3 as "affected" > > Thanks! setting "status-firefox28" flag, please clear it if it is not the one used for referring to v1.3 branch.

[v1.3 022cf2f] Merge pull request #14051 from jmcanterafonseca/fb_migration_datastore

Cannot verify the bug from comment 12. B/ Trigger a FOTA for v1.3. Cannot FOTA to 1.3 from 1.2

I need build VIA OTA. For this reason this bug isn't tested still.

Tested 1.1 -> 1.3 on a Unagi: 1.1 Build ID: 20140303131755 Platfomr version: 18.1 Git Commint: c5cb252e5d1aa45d14f3a2ea182e73e 1.3 Build ID: 20140303045237 Platform version: 28.0 Git Commit: d51d2e09 Tested 1.2 -> 1.3 on a Unagi: 1.2 Build ID: 20140303105246 Platfomr version: 26.0 Git Commit: 539a25e1887b902b8b25038c547048e691 1.3 Build ID: 20140303045237 Platform version: 28.0 Git Commit: d51d2e09 (In a test with 1900 contacts, you will see that FB data is not loaded. If you want to know the result for contact you have to hope more 1 minute this time depends of number of contacts to load.)