From 6ac894a38efcaa1ae010403240ba7edddada81a6 Mon Sep 17 00:00:00 2001 From: Numfor Mbiziwo-Tiapo Date: Thu, 13 Aug 2020 13:58:31 -0700 Subject: [PATCH] [IndexedDB] Implement putAllValues with correct error handling Implements putAllValues where any errors cause the whole request to fail, instead of just the value/s that failed. Explainer: https://github.com/nums11/idb-putall/blob/master/Explainer.md Design Doc: https://docs.google.com/document/d/1wawQ8Pl-Vi6GQN5-y2UVkcghipcOGg0WRAC0ZyBkezg/edit I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/TPd_rtgO3_k/m/qZmPy1dfAgAJ Change-Id: I976db2fe4c441f0ae288e63deb4c69c148721a18 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2324146 Reviewed-by: Christian Biesinger Reviewed-by: Joshua Bell Reviewed-by: Chris Palmer Reviewed-by: Daniel Murphy Reviewed-by: Olivier Yiptong Commit-Queue: Daniel Murphy Cr-Commit-Position: refs/heads/master@{#797837} --- .../idbobjectstore_putall.tentative.any.js | 155 +++++++++++++++++- IndexedDB/support-promises.js | 14 +- 2 files changed, 165 insertions(+), 4 deletions(-) diff --git a/IndexedDB/idbobjectstore_putall.tentative.any.js b/IndexedDB/idbobjectstore_putall.tentative.any.js index a312d71d03d59c..26bcc87a4c3234 100644 --- a/IndexedDB/idbobjectstore_putall.tentative.any.js +++ b/IndexedDB/idbobjectstore_putall.tentative.any.js @@ -6,12 +6,13 @@ promise_test(async testCase => { }); const txn = db.transaction(['books'], 'readwrite'); const objectStore = txn.objectStore('books'); - let values = [ + const values = [ {isbn: 'one', title: 'title1'}, {isbn: 'two', title: 'title2'}, {isbn: 'three', title: 'title3'} ]; - let putAllRequest = objectStore.putAll(values); + const putAllRequest = objectStore.putAllValues(values); + // TODO(nums): Check that correct keys are returned. await promiseForRequest(testCase, putAllRequest); await promiseForTransaction(testCase, txn); @@ -28,4 +29,152 @@ promise_test(async testCase => { ['title1', 'title2', 'title3'], 'All three retrieved titles should match those that were put.'); db.close(); -}, 'Data can be successfully inputted into an object store using putAll.'); +}, 'Data can be successfully inserted into an object store using putAll.'); + +promise_test(async testCase => { + const db = await createDatabase(testCase, db => { + const store = createBooksStore(testCase, db); + }); + const txn = db.transaction(['books'], 'readwrite'); + const objectStore = txn.objectStore('books'); + const values = [ + {isbn: ['one', 'two', 'three'], title: 'title1'}, + {isbn: ['four', 'five', 'six'], title: 'title2'}, + {isbn: ['seven', 'eight', 'nine'], title: 'title3'} + ]; + const putAllRequest = objectStore.putAllValues(values); + // TODO(nums): Check that correct keys are returned. + await promiseForRequest(testCase, putAllRequest); + await promiseForTransaction(testCase, txn); + + const txn2 = db.transaction(['books'], 'readonly'); + const objectStore2 = txn2.objectStore('books'); + const getRequest1 = objectStore2.get(['one', 'two', 'three']); + const getRequest2 = objectStore2.get(['four', 'five', 'six']); + const getRequest3 = objectStore2.get(['seven', 'eight', 'nine']); + await promiseForTransaction(testCase, txn2); + assert_array_equals( + [getRequest1.result.title, + getRequest2.result.title, + getRequest3.result.title], + ['title1', 'title2', 'title3'], + 'All three retrieved titles should match those that were put.'); + db.close(); +}, 'Values with array keys can be successfully inserted into an object' + + ' store using putAll.'); + +promise_test(async testCase => { + const db = await createDatabase(testCase, db => { + const store = createBooksStore(testCase, db); + }); + const txn = db.transaction(['books'], 'readwrite'); + const objectStore = txn.objectStore('books'); + const putAllRequest = objectStore.putAllValues([]); + await promiseForRequest(testCase, putAllRequest); + await promiseForTransaction(testCase, txn); + // TODO(nums): Check that an empty key array is returned. + db.close(); +}, 'Inserting an empty list using putAll.'); + +promise_test(async testCase => { + const db = await createDatabase(testCase, db => { + const store = createBooksStore(testCase, db); + }); + const txn = db.transaction(['books'], 'readwrite'); + const objectStore = txn.objectStore('books'); + const putAllRequest = objectStore.putAllValues([{}, {}, {}]); + // TODO(nums): Check that correct keys are returned. + await promiseForRequest(testCase, putAllRequest); + await promiseForTransaction(testCase, txn); + + const txn2 = db.transaction(['books'], 'readonly'); + const objectStore2 = txn2.objectStore('books'); + const getRequest1 = objectStore2.get(1); + const getRequest2 = objectStore2.get(2); + const getRequest3 = objectStore2.get(3); + await Promise.all([ + promiseForRequest(testCase, getRequest1), + promiseForRequest(testCase, getRequest2), + promiseForRequest(testCase, getRequest3), + ]); + db.close(); +}, 'Empty values can be inserted into an objectstore' + + ' with a key generator using putAll.'); + +promise_test(async testCase => { + const db = await createDatabase(testCase, db => { + const store = createBooksStore(testCase, db); + }); + const txn = db.transaction(['books'], 'readonly'); + const objectStore = txn.objectStore('books'); + assert_throws_dom('ReadOnlyError', + () => { objectStore.putAllValues([{}]); }, + 'The transaction is readonly'); + db.close(); +}, 'Attempting to insert with a read only transaction using putAll throws a ' + + 'ReadOnlyError.'); + +promise_test(async testCase => { + const db = await createDatabase(testCase, db => { + const store = createBooksStore(testCase, db); + }); + const txn = db.transaction(['books'], 'readwrite'); + const objectStore = txn.objectStore('books'); + const putRequest = await objectStore.put({isbn: 1, title: "duplicate"}); + await promiseForRequest(testCase, putRequest); + const putAllRequest = objectStore.putAllValues([ + {isbn: 2, title: "duplicate"}, + {isbn: 3, title: "duplicate"} + ]); + const errorEvent = await requestWatcher(testCase, + putAllRequest).wait_for('error'); + assert_equals(errorEvent.target.error.name, "ConstraintError"); + errorEvent.preventDefault(); + // The transaction still receives the error event even though it + // isn't aborted. + await transactionWatcher(testCase, txn).wait_for(['error', 'complete']); + + const txn2 = db.transaction(['books'], 'readonly'); + const objectStore2 = txn2.objectStore('books'); + const getRequest1 = objectStore2.get(1); + const getRequest2 = objectStore2.get(2); + const getRequest3 = objectStore2.get(3); + await promiseForTransaction(testCase, txn2); + assert_array_equals( + [getRequest1.result.title, getRequest2.result, getRequest3.result], + ["duplicate", undefined, undefined], + 'None of the values should have been inserted.'); + db.close(); +}, 'Inserting duplicate unique keys into a store that already has the key' + + 'using putAll throws a ConstraintError.'); + +promise_test(async testCase => { + const db = await createDatabase(testCase, db => { + const store = createBooksStoreWithoutAutoIncrement(testCase, db); + }); + const txn = db.transaction(['books'], 'readwrite'); + const objectStore = txn.objectStore('books'); + const values = [ + {title: "title1", isbn: 1}, + {title: "title2"} + ]; + assert_throws_dom('DataError', + () => { const putAllRequest = objectStore.putAllValues(values); }, + "Evaluating the object store's key path did not yield a value"); + + const txn2 = db.transaction(['books'], 'readonly'); + const objectStore2 = txn2.objectStore('books'); + const getRequest1 = objectStore2.get(1); + const getRequest2 = objectStore2.get(2); + await promiseForTransaction(testCase, txn2); + assert_array_equals( + [getRequest1.result, getRequest2.result], + [undefined, undefined], + 'No data should have been inserted'); + db.close(); +}, 'Inserting values without the key into an object store that' + + ' does not have generated keys throws an exception.'); + +// TODO(nums): Add test for insertion into multi entry indexes +// TODO(nums): Add test for inserting unique keys into a store +// that doesn't already have the key https://crbug.com/1115649 diff --git a/IndexedDB/support-promises.js b/IndexedDB/support-promises.js index 8195be341e1886..9128bfe151ab9c 100644 --- a/IndexedDB/support-promises.js +++ b/IndexedDB/support-promises.js @@ -196,7 +196,19 @@ const createBooksStore = (testCase, database) => { { keyPath: 'isbn', autoIncrement: true }); store.createIndex('by_author', 'author'); store.createIndex('by_title', 'title', { unique: true }); - for (let record of BOOKS_RECORD_DATA) + for (const record of BOOKS_RECORD_DATA) + store.put(record); + return store; +} + +// Creates a 'books' object store whose contents closely resembles the first +// example in the IndexedDB specification, just without autoincrementing. +const createBooksStoreWithoutAutoIncrement = (testCase, database) => { + const store = database.createObjectStore('books', + { keyPath: 'isbn' }); + store.createIndex('by_author', 'author'); + store.createIndex('by_title', 'title', { unique: true }); + for (const record of BOOKS_RECORD_DATA) store.put(record); return store; }