improve IndexedDB use

2 files changed, 276 insertions, 88 deletions

diff --git a/common/indexeddb.js b/common/indexeddb.js
index d0bacc4..c6302fa 100644
--- a/common/indexeddb.js
+++ b/common/indexeddb.js
@@ -55,8 +55,8 @@ const nr_reductor = ([i, s], num) => [i - 1, s + num * 1024 ** i];
 const version_nr = ver => Array.reduce(ver.slice(0, 3), nr_reductor, [2, 0])[1];
 
 const stores = 	[
-    ["files",     {keyPath: "sha256"}],
-    ["file_uses", {keyPath: "sha256"}],
+    ["files",     {keyPath: "hash_key"}],
+    ["file_uses", {keyPath: "hash_key"}],
     ["resources", {keyPath: "identifier"}],
     ["mappings",  {keyPath: "identifier"}]
 ];
@@ -92,7 +92,7 @@ async function idb_del(transaction, store_name, key)
 }
 
 /* Open haketilo database, asynchronously return an IDBDatabase object. */
-async function get_db(initialization_data=initial_data)
+async function get_db(data=initial_data)
 {
     if (db)
 	return db;
@@ -120,9 +120,9 @@ async function get_db(initialization_data=initial_data)
 	for (const [store_name, key_mode] of stores)
 	    store = opened_db.createObjectStore(store_name, key_mode);
 
-	await new Promise(resolve => store.transaction.oncomplete = resolve);
+	const context = make_context(store.transaction, data.files);
 
-	save_items(db, initialization_data);
+	await _save_items(data.resources, data.mappings, context);
     }
 
     db = opened_db;
@@ -130,6 +130,105 @@ async function get_db(initialization_data=initial_data)
     return db;
 }
 
+/* Helper function used by start_items_transaction() and get_db(). */
+function make_context(transaction, files)
+{
+    files = files || {};
+    const context = {transaction, files, file_uses: {}};
+
+    let resolve, reject;
+    context.result = new Promise((...cbs) => [resolve, reject] = cbs);
+
+    context.transaction.oncomplete = resolve;
+    context.transaction.onerror = reject;
+
+    return context;
+}
+
+/*
+ * item_store_names should be an array with either string "mappings", string
+ * "resources" or both. files should be a dict with values being contents of
+ * files that are to be possibly saved in this transaction and keys of the form
+ * `sha256-<file's-sha256-sum>`.
+ *
+ * Returned is a context object wrapping the transaction and handling the
+ * counting of file references in IndexedDB.
+ */
+async function start_items_transaction(item_store_names, files)
+{
+    const db = await haketilodb.get();
+    const scope = [...item_store_names, "files", "file_uses"];
+    return make_context(db.transaction(scope, "readwrite"), files);
+}
+
+async function incr_file_uses(context, file_ref, by=1)
+{
+    const hash_key = file_ref.hash_key;
+    let uses = context.file_uses[hash_key];
+    if (uses === undefined) {
+	uses = await idb_get(context.transaction, "file_uses", hash_key);
+	if (uses)
+	    [uses.new, uses.initial] = [false, uses.uses];
+	else
+	    uses = {hash_key, uses: 0, new: true, initial: 0};
+
+	context.file_uses[hash_key] = uses;
+    }
+
+    uses.uses = uses.uses + by;
+}
+
+const decr_file_uses = (ctx, file_ref) => incr_file_uses(ctx, file_ref, -1);
+
+async function finalize_items_transaction(context)
+{
+    for (const uses of Object.values(context.file_uses)) {
+	if (uses.uses < 0)
+	    console.error("internal error: uses < 0 for file " + uses.hash_key);
+
+	const is_new       = uses.new;
+	const initial_uses = uses.initial;
+	const hash_key     = uses.hash_key;
+
+	delete uses.new;
+	delete uses.initial;
+
+	if (uses.uses < 1) {
+	    if (!is_new) {
+		idb_del(context.transaction, "file_uses", hash_key);
+		idb_del(context.transaction, "files",     hash_key);
+	    }
+
+	    continue;
+	}
+
+	if (uses.uses === initial_uses)
+	    continue;
+
+	idb_put(context.transaction, "file_uses", uses);
+
+	if (initial_uses > 0)
+	    continue;
+
+	const file = context.files[hash_key];
+	if (file === undefined) {
+	    context.transaction.abort();
+	    throw "file not present: " + hash_key;
+	}
+
+	idb_put(context.transaction, "files", {hash_key, contents: file});
+    }
+
+    return context.result;
+}
+
+async function with_items_transaction(cb, item_store_names, files={})
+{
+    const context = await start_items_transaction(item_store_names, files);
+    await cb(context);
+    await finalize_items_transaction(context);
+}
+
 /*
  * How a sample data argument to the function below might look like:
  *
@@ -167,97 +266,84 @@ async function get_db(initialization_data=initial_data)
  *     }
  * }
  */
-async function save_items(db, data)
+async function save_items(transaction, data)
 {
-    const files = data.files;
-    const resources =
-	  Object.values(data.resources || []).map(entities.get_newest);
-    const mappings =
-	  Object.values(data.mappings || []).map(entities.get_newest);
+    const items_store_names = ["resources", "mappings"];
+    const context = start_items_transaction(items_store_names, data.files);
 
-    resources.concat(mappings).forEach(i => save_item(i, data.files, db));
+    return _save_items(data.resources, data.mappings, context);
 }
 
-/* helper function of save_item() */
-async function get_file_uses(transaction, file_uses_sha256, file_ref)
+async function _save_items(resources, mappings, context)
 {
-    let uses = file_uses_sha256[file_ref.sha256];
-    if (uses === undefined) {
-	uses = await idb_get(transaction, "file_uses", file_ref.sha256);
-	if (uses)
-	    [uses.new, uses.initial] = [false, uses.uses];
-	else
-	    uses = {sha256: file_ref.sha256, uses: 0, new: true, initial: 0};
+    resources = Object.values(resources || {}).map(entities.get_newest);
+    mappings  = Object.values(mappings  || {}).map(entities.get_newest);
 
-	file_uses_sha256[file_ref.sha256] = uses;
-    }
+    for (const item of resources.concat(mappings))
+	await save_item(item, context);
 
-    return uses;
+    await finalize_items_transaction(context);
 }
 
 /*
  * Save given definition of a resource/mapping to IndexedDB. If the definition
  * (passed as `item`) references files that are not already present in
  * IndexedDB, those files should be present as values of the `files_sha256`
- * object with keys being their sha256 sums.
+ * object with keys being of the form `sha256-<file's-sha256-sum>`.
+ *
+ * context should be one returned from start_items_transaction() and should be
+ * later passed to finalize_items_transaction() so that files depended on are
+ * added to IndexedDB and files that are no longer depended on after this
+ * operation are removed from IndexedDB.
  */
-async function save_item(item, files_sha256, db)
+async function save_item(item, context)
 {
     const store_name = {resource: "resources", mapping: "mappings"}[item.type];
-    const transaction =
-	  db.transaction([store_name, "files", "file_uses"], "readwrite");
-
-    let resolve, reject;
-    const result = new Promise((...cbs) => [resolve, reject] = cbs);
-    transaction.oncomplete = resolve;
-    transaction.onerror = reject;
 
-    const uses_sha256 = {};
     for (const file_ref of entities.get_files(item))
-	(await get_file_uses(transaction, uses_sha256, file_ref)).uses++;
+	await incr_file_uses(context, file_ref);
 
-    const old_item = await idb_get(transaction, store_name, item.identifier);
-    if (old_item !== undefined) {
-	for (const file_ref of entities.get_files(old_item))
-	    (await get_file_uses(transaction, uses_sha256, file_ref)).uses--;
-    }
-
-    for (const uses of Object.values(uses_sha256)) {
-	if (uses.uses < 0)
-	    console.error("internal error: uses < 0 for file " + uses.sha256);
-
-	const [is_new, initial_uses] = [uses.new, uses.initial];
-	delete uses.new;
-	delete uses.initial;
-
-	if (uses.uses < 1) {
-	    if (!is_new) {
-		idb_del(transaction, "file_uses", uses.sha256);
-		idb_del(transaction, "files",     uses.sha256);
-	    }
-
-	    continue;
-	}
-
-	if (uses.uses === initial_uses)
-	    continue;
-
-	const file = files_sha256[uses.sha256];
-	if (file === undefined)
-	    throw "file not present: " + uses.sha256;
+    await _remove_item(store_name, item.identifier, context, false);
+    await idb_put(context.transaction, store_name, item);
+}
 
-	idb_put(transaction, "files", {sha256: uses.sha256, contents: file});
-	idb_put(transaction, "file_uses", uses);
+/* Helper function used by remove_item() and save_item(). */
+async function _remove_item(store_name, identifier, context)
+{
+    const item = await idb_get(context.transaction, store_name, identifier);
+    if (item !== undefined) {
+	for (const file_ref of entities.get_files(item))
+	    await decr_file_uses(context, file_ref);
     }
+}
 
-    idb_put(transaction, store_name, item);
-
-    return result;
+/*
+ * Remove definition of a resource/mapping from IndexedDB.
+ *
+ * context should be one returned from start_items_transaction() and should be
+ * later passed to finalize_items_transaction() so that files depended on are
+ * added to IndexedDB and files that are no longer depended on after this
+ * operation are removed from IndexedDB.
+ */
+async function remove_item(store_name, identifier, context)
+{
+    await _remove_item(store_name, identifier, context);
+    await idb_del(context.transaction, store_name, identifier);
 }
 
+const remove_resource =
+      (identifier, ctx) => remove_item("resources", identifier, ctx);
+const remove_mapping =
+      (identifier, ctx) => remove_item("mappings",  identifier, ctx);
+
 const haketilodb = {
-    get:       get_db,
-    save_item: save_item
+    get: get_db,
+    save_items,
+    save_item,
+    remove_resource,
+    remove_mapping,
+    start_items_transaction,
+    finalize_items_transaction
 };
 
 /*
diff --git a/test/unit/test_indexeddb.py b/test/unit/test_indexeddb.py
index e5e1626..f1322fb 100644
--- a/test/unit/test_indexeddb.py
+++ b/test/unit/test_indexeddb.py
@@ -28,8 +28,8 @@ def indexeddb_code():
 
 def sample_file(contents):
     return {
-        'sha256': sha256(contents.encode()).digest().hex(),
-        contents: contents
+        'hash_key': f'sha256-{sha256(contents.encode()).digest().hex()}',
+        'contents': contents
     }
 
 sample_files = {
@@ -37,16 +37,17 @@ sample_files = {
     'LICENSES/somelicense.txt': sample_file('Permission is granted...'),
     'hello.js':                 sample_file('console.log("hello!");\n'),
     'bye.js':                   sample_file('console.log("bye!");\n'),
+    'combined.js':              sample_file('console.log("hello!\\nbye!");\n'),
     'README.md':                sample_file('# Python Frobnicator\n...')
 }
 
-sample_files_sha256 = \
-    dict([[file['sha256'], file] for file in sample_files.values()])
+sample_files_by_hash = dict([[file['hash_key'], file['contents']]
+                             for file in sample_files.values()])
 
 def file_ref(file_name):
-    return {'file': file_name, 'sha256': sample_files[file_name]['sha256']}
+    return {'file': file_name, 'hash_key': sample_files[file_name]['hash_key']}
 
-def test_save_item(execute_in_page, indexeddb_code):
+def test_save_remove_item(execute_in_page, indexeddb_code):
     """
     indexeddb.js facilitates operating on Haketilo's internal database.
     Verify database operations work properly.
@@ -79,11 +80,8 @@ def test_save_item(execute_in_page, indexeddb_code):
     # Facilitate retrieving all IndexedDB contents.
     execute_in_page(
         '''
-        async function get_database_contents(promise=Promise.resolve())
+        async function get_database_contents()
         {
-            if (promise)
-                await promise;
-
             const db = await haketilodb.get();
 
             const transaction = db.transaction(db.objectStoreNames);
@@ -110,25 +108,129 @@ def test_save_item(execute_in_page, indexeddb_code):
         'type': 'resource',
         'identifier': 'helloapple',
         'scripts': [file_ref('hello.js'), file_ref('bye.js')],
-        'type': 'resource'
     }
     next(iter(sample_item['source_copyright']))['ugly_extra_property'] = True
 
     database_contents = execute_in_page(
         '''{
-        const prom = haketilodb.get().then(db => save_item(...arguments, db));
-        returnval(get_database_contents(prom));
+        const promise = start_items_transaction(["resources"], arguments[1])
+            .then(ctx => save_item(arguments[0], ctx).then(() => ctx))
+            .then(finalize_items_transaction)
+            .then(get_database_contents);
+        returnval(promise);
         }''',
-        sample_item, sample_files_sha256)
+        sample_item, sample_files_by_hash)
     assert len(database_contents['files']) == 4
-    assert all([sample_files_sha256[file['sha256']] == file['contents']
+    assert all([sample_files_by_hash[file['hash_key']] == file['contents']
                 for file in database_contents['files']])
     assert all([len(file) == 2 for file in database_contents['files']])
 
     assert len(database_contents['file_uses']) == 4
     assert all([uses['uses'] == 1 for uses in database_contents['file_uses']])
-    assert set([uses['sha256'] for uses in database_contents['file_uses']]) \
-        == set([file['sha256'] for file in database_contents['files']])
+    assert set([uses['hash_key'] for uses in database_contents['file_uses']]) \
+        == set([file['hash_key'] for file in database_contents['files']])
 
     assert database_contents['mappings'] == []
     assert database_contents['resources'] == [sample_item]
+
+    # See if trying to add an item without providing all its files ends in an
+    # exception and aborts the transaction as it should.
+    sample_item['scripts'].append(file_ref('combined.js'))
+    incomplete_files = {**sample_files_by_hash}
+    incomplete_files.pop(sample_files['combined.js']['hash_key'])
+    print ('incomplete files:', incomplete_files)
+    print ('sample item:', sample_item)
+    result = execute_in_page(
+        '''{
+        console.log('sample item', arguments[0]);
+        const promise = (async () => {
+            const context =
+                await start_items_transaction(["resources"], arguments[1]);
+            try {
+                await save_item(arguments[0], context);
+                await finalize_items_transaction(context);
+                return {};
+            } catch(e) {
+                var exception = e;
+            }
+
+            return {exception, db_contents: await get_database_contents()};
+        })();
+        returnval(promise);
+        }''',
+        sample_item, incomplete_files)
+
+    assert result
+    assert 'file not present' in result['exception']
+    for key, val in database_contents.items():
+        keyfun = lambda item: item.get('hash_key') or item['identifier']
+        assert sorted(result['db_contents'][key], key=keyfun) \
+            == sorted(val,                        key=keyfun)
+
+    # See if adding another item that partially uses first's files works OK.
+    sample_item = {
+        'source_copyright': [
+            file_ref('report.spdx'),
+            file_ref('README.md')
+        ],
+        'type': 'mapping',
+        'identifier': 'helloapple',
+    }
+    database_contents = execute_in_page(
+        '''{
+        const promise = start_items_transaction(["mappings"], arguments[1])
+            .then(ctx => save_item(arguments[0], ctx).then(() => ctx))
+            .then(finalize_items_transaction)
+            .then(get_database_contents);
+        returnval(promise);
+        }''',
+        sample_item, sample_files_by_hash)
+
+    names = ['README.md', 'report.spdx', 'LICENSES/somelicense.txt', 'hello.js',
+             'bye.js']
+    sample_files_list = [sample_files[name] for name in names]
+    uses_list = [1, 2, 1, 1, 1]
+
+    uses = dict([(uses['hash_key'], uses['uses'])
+                 for uses in database_contents['file_uses']])
+    assert uses  == dict([(file['hash_key'], nr)
+                          for file, nr in zip(sample_files_list, uses_list)])
+
+    files = dict([(file['hash_key'], file['contents'])
+                  for file in database_contents['files']])
+    assert files == dict([(file['hash_key'], file['contents'])
+                          for file in sample_files_list])
+
+    assert database_contents['mappings'] == [sample_item]
+
+    # Try removing the items to get an empty database again.
+    results = [None, None]
+    for i, item_type in enumerate(['resource', 'mapping']):
+         results[i] = execute_in_page(
+            f'''{{
+            const remover = remove_{item_type};
+            const promise =
+                start_items_transaction(["{item_type}s"], {{}})
+                .then(ctx => remover('helloapple', ctx).then(() => ctx))
+                .then(finalize_items_transaction)
+                .then(get_database_contents);
+            returnval(promise);
+            }}''')
+
+    names = ['README.md', 'report.spdx']
+    sample_files_list = [sample_files[name] for name in names]
+    uses_list = [1, 1]
+
+    uses = dict([(uses['hash_key'], uses['uses'])
+                 for uses in results[0]['file_uses']])
+    assert uses  == dict([(file['hash_key'], 1) for file in sample_files_list])
+
+    files = dict([(file['hash_key'], file['contents'])
+                  for file in results[0]['files']])
+    assert files == dict([(file['hash_key'], file['contents'])
+                          for file in sample_files_list])
+
+    assert results[0]['resources'] == []
+    assert results[0]['mappings'] == [sample_item]
+
+    assert results[1] == dict([(key, []) for key in  results[0].keys()])