Issue 6621

lib/ioIndexedDB.js

Index: lib/ioIndexedDB.js
diff --git a/lib/ioIndexedDB.js b/lib/ioIndexedDB.js
new file mode 100644
index 0000000000000000000000000000000000000000..0dcb99179367fa7d0887d425d4512126159e44a9
--- /dev/null
+++ b/lib/ioIndexedDB.js
@@ -0,0 +1,300 @@
+/*
+ * This file is part of Adblock Plus <https://adblockplus.org/>,
+ * Copyright (C) 2006-present eyeo GmbH
+ *
+ * Adblock Plus is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Adblock Plus is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+"use strict";
+
+
+// values from the DefaultConfig https://github.com/localForage/localForage/blob/2cdbd74/src/localforage.js#L42-L51
+const localForageDbConfig = {
+  dbName: "localforage",
+  storeName: "keyvaluepairs",
+  version: 2
+};
+
+const dbConfig = {
+  dbName: "adblockplus",
+  storeName: "file",
+  keyPath: "fileName",
+  version: 1
+};
+
+let db;

[kzar, 2018/06/05 12:38:53]

I'd rather we stored the connection promise here, the APIs we export are all
asynchronous anyway. Something like this:

  let db = openDB({
    dbName: "adblockplus",
    ...
  );

[piscoi.georgiana, 2018/06/11 07:02:45]

Done.

+let localForageDb;

[kzar, 2018/06/05 12:38:53]

We don't use the localforage database connection outside of the migration
function, so I figure we can get rid of this variable.

[piscoi.georgiana, 2018/06/11 07:02:44]

Done.

+let migrationDone = migrateFiles();
+
+const keyPrefix = "file:";
+
+/**
+ * Handles migrating all files from localforage db
+ * used in the previous implementation by the localForage library
+ * to the new adblockplus db that we use as a replacement
+ * @return {Promise}
+ *    Promise to be resolved or rejected once the operation is completed
+ */
+function migrateFiles()
+{
+  return openDB(dbConfig)

[kzar, 2018/06/05 12:38:53]

Nit: I think we should move the openDB function's definition above, since we're
calling it here.

[piscoi.georgiana, 2018/06/11 07:02:45]

Is there any specific rule/preference regarding reading direction? Normally, I
like to put my exports at the top, and everything else, in a top-bottom order,
with functions that are called from multiple places last. 
I'm only asking because, I think I should probably move more than just this one
function. 

[kzar, 2018/06/12 10:46:16]

No rule that I know of, just something I try to do since personally I find it
kind of confusing to use something above where it's defined. I don't insist
anyway, if you prefer it this way around that's fair enough.

[Sebastian Noack, 2018/06/13 00:55:11]

The idea is to increase code locality, i.e. so that when someone studies the
code, following the path of execution across functions defined in here, you have
to scroll as less as possible. There is no strict rule though.

[piscoi.georgiana, 2018/06/14 13:41:26]

Done.

+    .then(dbInstance =>
+    {
+      db = dbInstance;

[kzar, 2018/06/05 12:38:53]

(I think assigning something from a promise which we later use synchronously is
worth avoiding if possible, which is why I suggested to just store the promise
in the comment above.)

[piscoi.georgiana, 2018/06/11 07:02:44]

Done.

+      return openDB(localForageDbConfig);
+    })
+    .then(dbInstance =>
+    {
+      localForageDb = dbInstance;
+      return getAllFiles(localForageDb, localForageDbConfig.storeName);
+    })
+    .then(files =>
+      files.map(file =>
+        Promise.all(saveFile(file, db, dbConfig.storeName))))

[kzar, 2018/06/05 12:38:53]

Supposing a file exists in both databases wouldn't this logic overwrite the one
stored in the new database? I think we should probably skip files which already
exist in the new database.

[piscoi.georgiana, 2018/06/11 07:02:45]

Yes, it will override any file that is present in both databases, however that
scenario is possible, only if the user, somehow used the new version, without
localForage, then used the version with localForage, and finally back to the new
version. I think this scenario is unlikely, so I didn't treat it, but if you
think that it might happen, I'll change the implementation. 

[kzar, 2018/06/12 10:46:16]

What do you think Sebastian? I think I'd prefer that the file is only copied
from the old database if it doesn't exist in the new database yet.

[Sebastian Noack, 2018/06/13 00:55:11]

I think it's fine to copy the file over from localForage without checking if it
already exists in the new database (if that keeps the logic simpler). There is
no practical scenario in which users would/could downgrade to an older version,
and we never bothered about that scenario when we did similar migrations before.

+    .then(() =>
+      clearObjectStore(localForageDb, localForageDbConfig.storeName));

[kzar, 2018/06/05 12:38:53]

I wonder what will happen the next time we try to open this database after we've
cleared it? Will it throw?

[piscoi.georgiana, 2018/06/11 07:02:44]

No, it won't throw. The cursor value will be null, so the getAllFiles will
resolve with an empty array. 

[kzar, 2018/06/12 10:46:16]

Acknowledged.

+}
+
+function getAllFiles(dbInstance, storeName)
+{
+  return new Promise((resolve, reject) =>
+  {
+    // edge doesn't currently support getAll method on IDBObjectStore interface
+    // so a cursor is used to iterate over all objects from the store
+    let transaction = dbInstance
+      .transaction([storeName], IDBTransaction.READ_ONLY);
+
+    let store = transaction.objectStore(storeName);
+    let cursorReq = store.openCursor();
+    let filesData = [];
+
+    transaction.oncomplete = event =>
+    {
+      resolve(filesData);
+    };
+
+    cursorReq.onsuccess = event =>
+    {
+      let cursor = event.currentTarget.result;
+      if (cursor)
+      {
+        let value = cursor.value;
+
+        filesData.push({
+          fileName: cursor.key,
+          content: value.content,
+          lastModified: value.lastModified
+        });
+        cursor.continue();
+      }
+    };
+
+    cursorReq.onerror = reject;
+  });
+}
+
+function clearObjectStore(dbInstance, storeName)
+{
+  return new Promise((resolve, reject) =>
+  {
+    let store = getObjectStore(dbInstance, storeName);
+    let req = store.clear();
+
+    req.onsuccess = resolve;
+    req.onerror = reject;
+  });
+}
+
+function fileToKey(fileName)
+{
+  return keyPrefix + fileName;
+}
+
+function formatFile(name, data)
+{
+  return {
+    fileName: fileToKey(name),
+    content: Array.from(data),
+    lastModified: Date.now()
+  };
+}
+
+function openDB({dbName, storeName, version, keyPath})
+{
+  return new Promise((resolve, reject) =>
+  {
+    let req = indexedDB.open(dbName, version);
+
+    req.onsuccess = event =>
+    {
+      return resolve(event.currentTarget.result);
+    };
+
+    req.onerror = reject;
+
+    req.onupgradeneeded = event =>
+    {
+      event
+      .currentTarget
+      .result
+      .createObjectStore(storeName,
+        {
+          keyPath,
+          autoIncrement: true
+        });
+    };
+  });
+}
+
+function getObjectStore(dbInstance, storeName)
+{
+  return dbInstance
+    .transaction([storeName], IDBTransaction.READ_WRITE)
+    .objectStore(storeName);
+}
+
+function getFile(fileName, dbInstance, storeName)
+{
+  return new Promise((resolve, reject) =>
+  {
+    let store = getObjectStore(dbInstance, storeName);
+    let req = store.get(fileToKey(fileName));
+
+    req.onsuccess = event =>
+    {
+      let result = event.currentTarget.result;
+
+      if (result)
+        resolve(result);
+      else
+        reject({type: "NoSuchFile"});
+    };
+    req.onerror = reject;
+  });
+}
+
+function saveFile(data, dbInstance, storeName)
+{
+  return new Promise((resolve, reject) =>
+  {
+    let store = getObjectStore(dbInstance, storeName);
+    let req = store.put(data);
+
+    req.onsuccess = resolve;
+    req.onerror = reject;
+  });
+}
+
+function deleteFile(fileName, dbInstance, storeName)
+{
+  return new Promise((resolve, reject) =>
+  {
+    let store = getObjectStore(dbInstance, storeName);
+    let req = store.delete(fileToKey(fileName));
+
+    req.onsuccess = resolve;
+    req.onerror = reject;
+  });
+}
+
+exports.IO =
+{
+  /**
+   * Writes text lines to a file.
+   * @param {string} fileName
+   *    Name of the file to be written
+   * @param {Iterable.<string>} data
+   *    An array-like or iterable object containing the lines (without line
+   *    endings)
+   * @return {Promise}
+   *    Promise to be resolved or rejected once the operation is completed
+   */
+  writeToFile(fileName, data)
+  {
+    return migrationDone
+      .then(() => saveFile(formatFile(fileName, data), db, dbConfig.storeName));
+  },
+
+  /**
+   * Reads text lines from a file.
+   * @param {string} fileName
+   *    Name of the file to be read
+   * @param {TextSink} listener
+   *    Function that will be called for each line in the file
+   * @return {Promise}
+   *    Promise to be resolved or rejected once the operation is completed
+   */
+  readFromFile(fileName, listener)
+  {
+    return migrationDone
+      .then(() => getFile(fileName, db, dbConfig.storeName))
+      .then(entry =>
+      {
+        for (let line of entry.content)
+          listener(line);
+      });
+  },
+
+  /**
+   * Retrieves file metadata.
+   * @param {string} fileName
+   *    Name of the file to be looked up
+   * @return {Promise.<StatData>}
+   *    Promise to be resolved with file metadata once the operation is
+   *    completed
+   */
+  statFile(fileName)
+  {
+    return migrationDone
+      .then(() => getFile(fileName, db, dbConfig.storeName))
+      .then(entry =>
+      {
+        return {
+          exists: true,
+          lastModified: entry.lastModified
+        };
+      })
+      .catch(error =>
+      {
+        if (error.type == "NoSuchFile")
+          return {exists: false};
+        throw error;
+      });
+  },
+
+  /**
+   * Renames a file.
+   * @param {string} fromFile
+   *    Name of the file to be renamed
+   * @param {string} newName
+   *    New file name, will be overwritten if exists
+   * @return {Promise}
+   *    Promise to be resolved or rejected once the operation is completed
+   */
+  renameFile(fromFile, newName)
+  {
+    return migrationDone
+      .then(() => getFile(fromFile, db, dbConfig.storeName))
+      .then(fileData =>
+        saveFile(
+          {
+            fileName: fileToKey(newName),
+            content: fileData.content,
+            lastModified: fileData.lastModified
+          },
+          db,
+          dbConfig.storeName))
+      .then(() => deleteFile(fromFile, db, dbConfig.storeName));
+  }
+};
+