386371 - Use getters and setters for database schema version

Status: RESOLVED FIXED

(Toolkit :: Preferences, defect)

Description

[Shawn Wilsher :sdwilsh]

Here are the places that it'll be useful:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/components/contentprefs/src/nsContentPrefService.js&rev=1.7#692
(not as useful on these lines, but best to just use the api)
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/components/contentprefs/src/nsContentPrefService.js&rev=1.7#735 
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/components/contentprefs/src/nsContentPrefService.js&rev=1.7#787
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/components/contentprefs/src/nsContentPrefService.js&rev=1.7#803

Comment 1

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

not sure who is good to review this...

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 270556 [details] [diff] [review]
v1.0

>Index: toolkit/components/contentprefs/src/nsContentPrefService.js
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/components/contentprefs/src/nsContentPrefService.js,v
>retrieving revision 1.7
>diff -u -8 -p -r1.7 nsContentPrefService.js
>--- toolkit/components/contentprefs/src/nsContentPrefService.js	26 Jun 2007 04:57:30 -0000	1.7
>+++ toolkit/components/contentprefs/src/nsContentPrefService.js	2 Jul 2007 08:48:36 -0000
>@@ -683,25 +683,17 @@ ContentPrefService.prototype = {
>       this._log("no database file; creating database");
>       dbConnection = this._dbCreate(dbService, dbFile);
>     }
>     else {
>       try {
>         dbConnection = dbService.openDatabase(dbFile);
> 
>         // Get the version of the database in the file.
>-        var statement, version;
>-        try {
>-          statement = dbConnection.createStatement("PRAGMA user_version");
>-          statement.executeStep();
>-          version = statement.getInt32(0);
>-        }
>-        finally {
>-          statement.reset();
>-        }
>+        var version = dbConnection.schemaVersion;
> 
>         if (version != this._dbVersion) {
>           this._log("database: v" + version + ", application: v" +
>                     this._dbVersion + "; migrating database");
>           this._dbMigrate(dbConnection, version, this._dbVersion);
>         }

Seems like we could drop the temporary variable entirely here and just use dbConnection.schemaVersion wherever needed.  But it's ok this way too, and the rest of the patch looks great.  Thanks for adding this feature!

Comment 3

[Shawn Wilsher :sdwilsh]

It still executes the query each time, so the member variable is probably a good thing to use.

Comment 4

[Shawn Wilsher :sdwilsh]

Checking in toolkit/components/contentprefs/src/nsContentPrefService.js;
new revision: 1.8; previous revision: 1.7