1365143 - Add SchemaVersion for each record in ProfileStorage

Status: RESOLVED FIXED

(Toolkit :: Form Manager, enhancement, P1)

Description

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

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

[Matthew N. [:MattN]]

(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?

Comment 2

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

(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.

Comment 3

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

(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 4

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

Attachment: Bug 1365143 - Add schema version number for each Form Fill record in ProfileStorage.;

Review commit: https://reviewboard.mozilla.org/r/139626/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139626/

Comment 5

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

Comment on attachment 8868037 [details]
Bug 1365143 - Add schema version number for each Form Fill record in ProfileStorage.;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139626/diff/1-2/

Comment 6

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

Comment on attachment 8868037 [details]
Bug 1365143 - Add schema version number for each Form Fill record in ProfileStorage.;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139626/diff/2-3/

Comment 7

[Luke Chang [:lchang] (inactive)]

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`

Comment 8

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

Comment on attachment 8868037 [details]
Bug 1365143 - Add schema version number for each Form Fill record in ProfileStorage.;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139626/diff/3-4/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53244a64eed9
Add schema version number for each Form Fill record in ProfileStorage.; r=lchang

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/53244a64eed9