Closed
Bug 1365143
Opened 6 years ago
Closed 6 years ago
Add SchemaVersion for each record in ProfileStorage
Categories
(Toolkit :: Form Manager, enhancement, P1)
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: selee, Assigned: selee)
References
(Blocks 1 open bug)
Details
(Whiteboard: [form autofill:M3])
Attachments
(1 file)
When I implement `prefer` function in bug 1361242, the schema version for each record is a must to compare the two records in the different schema format. There are some items related to this change: 1. Add schemaVersion field in the internal field. 2. Implement the migration function to fill the empty schema version of an old record with "1". IMHO, the migration is not serious as a major schema change, so it's fine to just fill the schema version for a record missing it with "1". 3. We can remove the schema version for the whole JSON object [1]. The global schema version looks redundant here. That's sufficient that we have add-on schema version (written in JS codes) and the version for each record. 4. Since the schema version will be for each record, the add-on schema version should be for AddressRecords and CreditCardRecords separately. [1] http://searchfox.org/mozilla-central/rev/ae24a3c83d22e0e35aecfd9049c2b463ca7e045b/browser/extensions/formautofill/ProfileStorage.jsm#13
Comment 1•6 years ago
|
||
(In reply to Sean Lee [:seanlee][:weilonge] from comment #0) > 3. We can remove the schema version for the whole JSON object [1]. The > global schema version looks redundant here. That's sufficient that we have > add-on schema version (written in JS codes) and the version for each record. It will be useful if we want to change the top-level keys e.g. addresses + creditCards… perhaps to support Payment Apps in the future though I guess we could migrate from no version to version 2 at that point?
Assignee | ||
Comment 2•6 years ago
|
||
(In reply to Sean Lee [:seanlee][:weilonge] from comment #0) > 3. We can remove the schema version for the whole JSON object [1]. The > global schema version looks redundant here. That's sufficient that we have > add-on schema version (written in JS codes) and the version for each record. > If we will invoke migration operation during initialization, the schema version for the whole JSON object can be considered as the upgrade-to version. If version 3 addon got a version 2 JSON file, the migration will migrate all records and mark the global version to 3. When initializing in the next time, the global version and add-on version is matched. Then the 2nd time migration can be skipped.
Assignee | ||
Comment 3•6 years ago
|
||
(In reply to Matthew N. [:MattN] (PM if requests are blocking you) from comment #1) > (In reply to Sean Lee [:seanlee][:weilonge] from comment #0) > > 3. We can remove the schema version for the whole JSON object [1]. The > > global schema version looks redundant here. That's sufficient that we have > > add-on schema version (written in JS codes) and the version for each record. > > It will be useful if we want to change the top-level keys e.g. addresses + > creditCards… perhaps to support Payment Apps in the future though I guess we > could migrate from no version to version 2 at that point? Yes, that's the reason to keep the global version. Another reason is at comment 2. I will keep the global version in the JSON file.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → selee
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment 7•6 years ago
|
||
mozreview-review |
Comment on attachment 8868037 [details] Bug 1365143 - Add schema version number for each Form Fill record in ProfileStorage.; https://reviewboard.mozilla.org/r/139626/#review142978 r+ with two nits. ::: browser/extensions/formautofill/ProfileStorage.jsm:100 (Diff revision 3) > "@mozilla.org/uuid-generator;1", > "nsIUUIDGenerator"); > > const PROFILE_JSON_FILE_NAME = "autofill-profiles.json"; > > const SCHEMA_VERSION = 1; nit: `STORAGE_SCHEMA_VERSION` ::: browser/extensions/formautofill/ProfileStorage.jsm:102 (Diff revision 3) > > const PROFILE_JSON_FILE_NAME = "autofill-profiles.json"; > > const SCHEMA_VERSION = 1; > +const ADDRESS_SCHEMA_VERSION = 1; > +const CREDITCARD_SCHEMA_VERSION = 1; nit: `CREDIT_CARD_SCHEMA_VERSION`
Attachment #8868037 -
Flags: review?(lchang) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/53244a64eed9 Add schema version number for each Form Fill record in ProfileStorage.; r=lchang
Keywords: checkin-needed
Comment 10•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/53244a64eed9
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•6 years ago
|
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•