Bug 1068620 - Provide a Sqlite.wrapStorageConnection method. r=Yoric

toolkit/modules/Sqlite.jsm

@@ -37,6 +37,12 @@ XPCOMUtils.defineLazyServiceGetter(this, "FinalizationWitnessService",
 // used for logging to distinguish connection instances.
 let connectionCounters = new Map();
 
+// Tracks identifiers of wrapped connections, that are Storage connections
+// opened through mozStorage and then wrapped by Sqlite.jsm to use its syntactic
+// sugar API.  Since these connections have an unknown origin, we use this set
+// to differentiate their behavior.
+let wrappedConnections = new Set();
+
 /**
  * Once `true`, reject any attempt to open or close a database.
  */
@@ -66,6 +72,20 @@ function logScriptError(message) {
   }
 }
 
+/**
+ * Gets connection identifier from its database file path.
+ *
+ * @param path
+ *        A file string path pointing to a database file.
+ * @return the connection identifier.
+ */
+function getIdentifierByPath(path) {
+  let basename = OS.Path.basename(path);
+  let number = connectionCounters.get(basename) || 0;
+  connectionCounters.set(basename, number + 1);
+  return basename + "#" + number;
+}
+
 /**
  * Barriers used to ensure that Sqlite.jsm is shutdown after all
  * its clients.
@@ -95,17 +115,17 @@ XPCOMUtils.defineLazyGetter(this, "Barriers", () => {
    * The observer is passed the connection identifier of the database
    * connection that is being finalized.
    */
-  let finalizationObserver = function (subject, topic, connectionIdentifier) {
-    let connectionData = ConnectionData.byId.get(connectionIdentifier);
+  let finalizationObserver = function (subject, topic, identifier) {
+    let connectionData = ConnectionData.byId.get(identifier);
 
     if (connectionData === undefined) {
       logScriptError("Error: Attempt to finalize unknown Sqlite connection: " +
-                     connectionIdentifier + "\n");
+                     identifier + "\n");
       return;
     }
 
-    ConnectionData.byId.delete(connectionIdentifier);
-    logScriptError("Warning: Sqlite connection '" + connectionIdentifier +
+    ConnectionData.byId.delete(identifier);
+    logScriptError("Warning: Sqlite connection '" + identifier +
                    "' was not properly closed. Auto-close triggered by garbage collection.\n");
     connectionData.close();
   };
@@ -170,13 +190,17 @@ XPCOMUtils.defineLazyGetter(this, "Barriers", () => {
  * OpenedConnection needs to use the methods in this object, it will
  * dispatch its method calls here.
  */
-function ConnectionData(connection, basename, number, options) {
-  this._log = Log.repository.getLoggerWithMessagePrefix("Sqlite.Connection." + basename,
-                                                        "Conn #" + number + ": ");
+function ConnectionData(connection, identifier, options={}) {
+  this._log = Log.repository.getLoggerWithMessagePrefix("Sqlite.Connection." +
+                                                        identifier + ": ");
   this._log.info("Opened");
 
   this._dbConn = connection;
-  this._connectionIdentifier = basename + " Conn #" + number;
+
+  // This is a unique identifier for the connection, generated through
+  // getIdentifierByPath.  It may be used for logging or as a key in Maps.
+  this._identifier = identifier;
+
   this._open = true;
 
   this._cachedStatements = new Map();
@@ -204,10 +228,10 @@ function ConnectionData(connection, basename, number, options) {
   this._closeRequested = false;
 
   Barriers.connections.client.addBlocker(
-    this._connectionIdentifier + ": waiting for shutdown",
+    this._identifier + ": waiting for shutdown",
     this._deferredClose.promise,
     () =>  ({
-      identifier: this._connectionIdentifier,
+      identifier: this._identifier,
       isCloseRequested: this._closeRequested,
       hasDbConn: !!this._dbConn,
       hasInProgressTransaction: !!this._inProgressTransaction,
@@ -224,7 +248,7 @@ function ConnectionData(connection, basename, number, options) {
  * database. Used by finalization witnesses to be able to close opened
  * connections on garbage collection.
  *
- * Key: _connectionIdentifier of ConnectionData
+ * Key: _identifier of ConnectionData
  * Value: ConnectionData object
  */
 ConnectionData.byId = new Map();
@@ -304,15 +328,23 @@ ConnectionData.prototype = Object.freeze({
     // function and asyncClose() finishing. See also bug 726990.
     this._open = false;
 
-    this._log.debug("Calling asyncClose().");
-    this._dbConn.asyncClose(() => {
+    // We must always close the connection at the Sqlite.jsm-level, not
+    // necessarily at the mozStorage-level.
+    let markAsClosed = () => {
       this._log.info("Closed");
       this._dbConn = null;
       // Now that the connection is closed, no need to keep
       // a blocker for Barriers.connections.
       Barriers.connections.client.removeBlocker(deferred.promise);
       deferred.resolve();
-    });
+    }
+    if (wrappedConnections.has(this._identifier)) {
+      wrappedConnections.delete(this._identifier);
+      markAsClosed();
+    } else {
+      this._log.debug("Calling asyncClose().");
+      this._dbConn.asyncClose(markAsClosed);
+    }
   },
 
   executeCached: function (sql, params=null, onRow=null) {
@@ -722,12 +754,7 @@ function openConnection(options) {
   }
 
   let file = FileUtils.File(path);
-
-  let basename = OS.Path.basename(path);
-  let number = connectionCounters.get(basename) || 0;
-  connectionCounters.set(basename, number + 1);
-
-  let identifier = basename + "#" + number;
+  let identifier = getIdentifierByPath(path);
 
   log.info("Opening database: " + path + " (" + identifier + ")");
   let deferred = Promise.defer();
@@ -746,8 +773,8 @@ function openConnection(options) {
     log.info("Connection opened");
     try {
       deferred.resolve(
-        new OpenedConnection(connection.QueryInterface(Ci.mozIStorageAsyncConnection), basename, number,
-        openedOptions));
+        new OpenedConnection(connection.QueryInterface(Ci.mozIStorageAsyncConnection),
+                            identifier, openedOptions));
     } catch (ex) {
       log.warn("Could not open database: " + CommonUtils.exceptionStr(ex));
       deferred.reject(ex);
@@ -759,7 +786,7 @@ function openConnection(options) {
 /**
  * Creates a clone of an existing and open Storage connection.  The clone has
  * the same underlying characteristics of the original connection and is
- * returned in form of on OpenedConnection handle.
+ * returned in form of an OpenedConnection handle.
  *
  * The following parameters can control the cloned connection:
  *
@@ -812,10 +839,7 @@ function cloneStorageConnection(options) {
   }
 
   let path = source.databaseFile.path;
-  let basename = OS.Path.basename(path);
-  let number = connectionCounters.get(basename) || 0;
-  connectionCounters.set(basename, number + 1);
-  let identifier = basename + "#" + number;
+  let identifier = getIdentifierByPath(path);
 
   log.info("Cloning database: " + path + " (" + identifier + ")");
   let deferred = Promise.defer();
@@ -828,8 +852,7 @@ function cloneStorageConnection(options) {
     log.info("Connection cloned");
     try {
       let conn = connection.QueryInterface(Ci.mozIStorageAsyncConnection);
-      deferred.resolve(new OpenedConnection(conn, basename, number,
-                                            openedOptions));
+      deferred.resolve(new OpenedConnection(conn, identifier, openedOptions));
     } catch (ex) {
       log.warn("Could not clone database: " + CommonUtils.exceptionStr(ex));
       deferred.reject(ex);
@@ -838,6 +861,55 @@ function cloneStorageConnection(options) {
   return deferred.promise;
 }
 
+/**
+ * Wraps an existing and open Storage connection with Sqlite.jsm API.  The
+ * wrapped connection clone has the same underlying characteristics of the
+ * original connection and is returned in form of an OpenedConnection handle.
+ *
+ * Clients are responsible for closing both the Sqlite.jsm wrapper and the
+ * underlying mozStorage connection.
+ *
+ * The following parameters can control the wrapped connection:
+ *
+ *   connection -- (mozIStorageAsyncConnection) The original Storage connection
+ *       to wrap.
+ *
+ * @param options
+ *        (Object) Parameters to control connection and wrap options.
+ *
+ * @return Promise<OpenedConnection>
+ */
+function wrapStorageConnection(options) {
+  let log = Log.repository.getLogger("Sqlite.ConnectionWrapper");
+
+  let connection = options && options.connection;
+  if (!connection || !(connection instanceof Ci.mozIStorageAsyncConnection)) {
+    throw new TypeError("connection not specified or invalid.");
+  }
+
+  if (isClosed) {
+    throw new Error("Sqlite.jsm has been shutdown. Cannot wrap connection to: " + connection.database.path);
+  }
+
+  let path = connection.databaseFile.path;
+  let identifier = getIdentifierByPath(path);
+
+  log.info("Wrapping database: " + path + " (" + identifier + ")");
+  return new Promise(resolve => {
+    try {
+      let conn = connection.QueryInterface(Ci.mozIStorageAsyncConnection);
+      let wrapper = new OpenedConnection(conn, identifier);
+      // We must not handle shutdown of a wrapped connection, since that is
+      // already handled by the opener.
+      wrappedConnections.add(identifier);
+      resolve(wrapper);
+    } catch (ex) {
+      log.warn("Could not wrap database: " + CommonUtils.exceptionStr(ex));
+      throw ex;
+    }
+  });
+}
+
 /**
  * Handle on an opened SQLite database.
  *
@@ -877,25 +949,24 @@ function cloneStorageConnection(options) {
  *
  * @param connection
  *        (mozIStorageConnection) Underlying SQLite connection.
- * @param basename
- *        (string) The basename of this database name. Used for logging.
- * @param number
- *        (Number) The connection number to this database.
- * @param options
+ * @param identifier
+ *        (string) The unique identifier of this database. It may be used for
+ *        logging or as a key in Maps.
+ * @param options [optional]
  *        (object) Options to control behavior of connection. See
  *        `openConnection`.
  */
-function OpenedConnection(connection, basename, number, options) {
+function OpenedConnection(connection, identifier, options={}) {
   // Store all connection data in a field distinct from the
   // witness. This enables us to store an additional reference to this
   // field without preventing garbage collection of
   // OpenedConnection. On garbage collection, we will still be able to
   // close the database using this extra reference.
-  this._connectionData = new ConnectionData(connection, basename, number, options);
+  this._connectionData = new ConnectionData(connection, identifier, options);
 
   // Store the extra reference in a map with connection identifier as
   // key.
-  ConnectionData.byId.set(this._connectionData._connectionIdentifier,
+  ConnectionData.byId.set(this._connectionData._identifier,
                           this._connectionData);
 
   // Make a finalization witness. If this object is garbage collected
@@ -904,7 +975,7 @@ function OpenedConnection(connection, basename, number, options) {
   // connection identifier string of the database.
   this._witness = FinalizationWitnessService.make(
     "sqlite-finalization-witness",
-    this._connectionData._connectionIdentifier);
+    this._connectionData._identifier);
 }
 
 OpenedConnection.prototype = Object.freeze({
@@ -964,8 +1035,8 @@ OpenedConnection.prototype = Object.freeze({
     // Unless cleanup has already been done by a previous call to
     // `close`, delete the database entry from map and tell the
     // finalization witness to forget.
-    if (ConnectionData.byId.has(this._connectionData._connectionIdentifier)) {
-      ConnectionData.byId.delete(this._connectionData._connectionIdentifier);
+    if (ConnectionData.byId.has(this._connectionData._identifier)) {
+      ConnectionData.byId.delete(this._connectionData._identifier);
       this._witness.forget();
     }
     return this._connectionData.close();
@@ -1175,6 +1246,7 @@ OpenedConnection.prototype = Object.freeze({
 this.Sqlite = {
   openConnection: openConnection,
   cloneStorageConnection: cloneStorageConnection,
+  wrapStorageConnection: wrapStorageConnection,
   /**
    * Shutdown barrier client. May be used by clients to perform last-minute
    * cleanup prior to the shutdown of this module.

toolkit/modules/tests/xpcshell/test_sqlite.js

@@ -845,12 +845,12 @@ add_task(function test_direct() {
 add_task(function* test_cloneStorageConnection() {
   let file = new FileUtils.File(OS.Path.join(OS.Constants.Path.profileDir,
                                              "test_cloneStorageConnection.sqlite"));
-  let c = yield new Promise((success, failure) => {
+  let c = yield new Promise((resolve, reject) => {
     Services.storage.openAsyncDatabase(file, null, (status, db) => {
       if (Components.isSuccessCode(status)) {
-        success(db.QueryInterface(Ci.mozIStorageAsyncConnection));
+        resolve(db.QueryInterface(Ci.mozIStorageAsyncConnection));
       } else {
-        failure(new Error(status));
+        reject(new Error(status));
       }
     });
   });
@@ -913,6 +913,33 @@ add_task(function* test_readOnly_clone() {
   yield clone.close();
 });
 
+/**
+ * Test Sqlite.wrapStorageConnection.
+ */
+add_task(function* test_wrapStorageConnection() {
+  let file = new FileUtils.File(OS.Path.join(OS.Constants.Path.profileDir,
+                                             "test_wrapStorageConnection.sqlite"));
+  let c = yield new Promise((resolve, reject) => {
+    Services.storage.openAsyncDatabase(file, null, (status, db) => {
+      if (Components.isSuccessCode(status)) {
+        resolve(db.QueryInterface(Ci.mozIStorageAsyncConnection));
+      } else {
+        reject(new Error(status));
+      }
+    });
+  });
+
+  let wrapper = yield Sqlite.wrapStorageConnection({ connection: c });
+  // Just check that it works.
+  yield wrapper.execute("SELECT 1");
+  yield wrapper.executeCached("SELECT 1");
+
+  // Closing the wrapper should just finalize statements but not close the
+  // database.
+  yield wrapper.close();
+  yield c.asyncClose();
+});
+
 /**
  * Test finalization
  */
@@ -921,7 +948,7 @@ add_task(function* test_closed_by_witness() {
   let c = yield getDummyDatabase("closed_by_witness");
 
   Services.obs.notifyObservers(null, "sqlite-finalization-witness",
-                               c._connectionData._connectionIdentifier);
+                               c._connectionData._identifier);
   // Since we triggered finalization ourselves, tell the witness to
   // forget the connection so it does not trigger a finalization again
   c._witness.forget();
@@ -933,7 +960,7 @@ add_task(function* test_closed_by_witness() {
 add_task(function* test_warning_message_on_finalization() {
   failTestsOnAutoClose(false);
   let c = yield getDummyDatabase("warning_message_on_finalization");
-  let connectionIdentifier = c._connectionData._connectionIdentifier;
+  let identifier = c._connectionData._identifier;
   let deferred = Promise.defer();
 
   let listener = {
@@ -941,14 +968,14 @@ add_task(function* test_warning_message_on_finalization() {
       let messageText = msg.message;
       // Make sure the message starts with a warning containing the
       // connection identifier
-      if (messageText.indexOf("Warning: Sqlite connection '" + connectionIdentifier + "'") !== -1) {
+      if (messageText.indexOf("Warning: Sqlite connection '" + identifier + "'") !== -1) {
         deferred.resolve();
       }
     }
   };
   Services.console.registerListener(listener);
 
-  Services.obs.notifyObservers(null, "sqlite-finalization-witness", connectionIdentifier);
+  Services.obs.notifyObservers(null, "sqlite-finalization-witness", identifier);
   // Since we triggered finalization ourselves, tell the witness to
   // forget the connection so it does not trigger a finalization again
   c._witness.forget();