From 3650b798ed412350e341ca93c4c02b158cdbbe1b Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Sun, 21 May 2017 18:11:12 -0400 Subject: [PATCH] Use transaction per operation in indexeddb.js, fix broken async tests in fs.stats.spec.js Fixing for review comments Switch to RW or RO transaction per get/put/delete/clear, better error handling for try/catch cases Switch back to transaction-per-context for better atomic fs operations. Move _getObjectStore onto prototype --- src/providers/indexeddb.js | 121 +++++++++++++++++++++--------------- tests/bugs/ls-depth-bug.js | 4 +- tests/lib/indexeddb.js | 24 ++++--- tests/spec/fs.stats.spec.js | 33 ++++++---- 4 files changed, 110 insertions(+), 72 deletions(-) diff --git a/src/providers/indexeddb.js b/src/providers/indexeddb.js index f353075..0e90e7f 100644 --- a/src/providers/indexeddb.js +++ b/src/providers/indexeddb.js @@ -11,43 +11,57 @@ var indexedDB = global.indexedDB || global.msIndexedDB; function IndexedDBContext(db, mode) { - var transaction = db.transaction(FILE_STORE_NAME, mode); - this.objectStore = transaction.objectStore(FILE_STORE_NAME); + this.db = db; + this.mode = mode; } +IndexedDBContext.prototype._getObjectStore = function() { + if(this.objectStore) { + return this.objectStore; + } + + var transaction = this.db.transaction(FILE_STORE_NAME, this.mode); + this.objectStore = transaction.objectStore(FILE_STORE_NAME); + return this.objectStore; +}; + IndexedDBContext.prototype.clear = function(callback) { try { - var request = this.objectStore.clear(); - request.onsuccess = function(event) { + var objectStore = this._getObjectStore(); + var request = objectStore.clear(); + request.onsuccess = function() { callback(); }; - request.onerror = function(error) { - callback(error); + request.onerror = function(event) { + event.preventDefault(); + callback(event.error); }; - } catch(e) { - callback(e); + } catch(err) { + callback(err); } }; -function _get(objectStore, key, callback) { +IndexedDBContext.prototype._get = function(key, callback) { try { + var objectStore = this._getObjectStore(); var request = objectStore.get(key); request.onsuccess = function onsuccess(event) { var result = event.target.result; callback(null, result); }; - request.onerror = function onerror(error) { - callback(error); + request.onerror = function(event) { + event.preventDefault(); + callback(event.error); }; - } catch(e) { - callback(e); + } catch(err) { + callback(err); } -} +}; IndexedDBContext.prototype.getObject = function(key, callback) { - _get(this.objectStore, key, callback); + this._get(key, callback); }; IndexedDBContext.prototype.getBuffer = function(key, callback) { - _get(this.objectStore, key, function(err, arrayBuffer) { + this._get(key, function(err, arrayBuffer) { if(err) { return callback(err); } @@ -55,22 +69,24 @@ IndexedDBContext.prototype.getBuffer = function(key, callback) { }); }; -function _put(objectStore, key, value, callback) { +IndexedDBContext.prototype._put = function(key, value, callback) { try { + var objectStore = this._getObjectStore(); var request = objectStore.put(value, key); request.onsuccess = function onsuccess(event) { var result = event.target.result; callback(null, result); }; - request.onerror = function onerror(error) { - callback(error); + request.onerror = function(event) { + event.preventDefault(); + callback(event.error); }; - } catch(e) { - callback(e); + } catch(err) { + callback(err); } -} +}; IndexedDBContext.prototype.putObject = function(key, value, callback) { - _put(this.objectStore, key, value, callback); + this._put(key, value, callback); }; IndexedDBContext.prototype.putBuffer = function(key, uint8BackedBuffer, callback) { var buf; @@ -79,21 +95,23 @@ IndexedDBContext.prototype.putBuffer = function(key, uint8BackedBuffer, callback } else { buf = uint8BackedBuffer.buffer; } - _put(this.objectStore, key, buf, callback); + this._put(key, buf, callback); }; IndexedDBContext.prototype.delete = function(key, callback) { try { - var request = this.objectStore.delete(key); + var objectStore = this._getObjectStore(); + var request = objectStore.delete(key); request.onsuccess = function onsuccess(event) { var result = event.target.result; callback(null, result); }; - request.onerror = function(error) { - callback(error); + request.onerror = function(event) { + event.preventDefault(); + callback(event.error); }; - } catch(e) { - callback(e); + } catch(err) { + callback(err); } }; @@ -114,32 +132,35 @@ IndexedDB.prototype.open = function(callback) { return callback(); } - // NOTE: we're not using versioned databases. - var openRequest = indexedDB.open(that.name); + try { + // NOTE: we're not using versioned databases. + var openRequest = indexedDB.open(that.name); - // If the db doesn't exist, we'll create it - openRequest.onupgradeneeded = function onupgradeneeded(event) { - var db = event.target.result; + // If the db doesn't exist, we'll create it + openRequest.onupgradeneeded = function onupgradeneeded(event) { + var db = event.target.result; - if(db.objectStoreNames.contains(FILE_STORE_NAME)) { - db.deleteObjectStore(FILE_STORE_NAME); - } - db.createObjectStore(FILE_STORE_NAME); - }; + if(db.objectStoreNames.contains(FILE_STORE_NAME)) { + db.deleteObjectStore(FILE_STORE_NAME); + } + db.createObjectStore(FILE_STORE_NAME); + }; - openRequest.onsuccess = function onsuccess(event) { - that.db = event.target.result; - callback(); - }; - openRequest.onerror = function onerror(error) { - callback(new Errors.EINVAL('IndexedDB cannot be accessed. If private browsing is enabled, disable it.')); - }; + openRequest.onsuccess = function onsuccess(event) { + that.db = event.target.result; + callback(); + }; + openRequest.onerror = function onerror(event) { + event.preventDefault(); + callback(event.error); + }; + } catch(err) { + callback(err); + } }; + IndexedDB.prototype.getReadOnlyContext = function() { - // Due to timing issues in Chrome with readwrite vs. readonly indexeddb transactions - // always use readwrite so we can make sure pending commits finish before callbacks. - // See https://github.com/js-platform/filer/issues/128 - return new IndexedDBContext(this.db, IDB_RW); + return new IndexedDBContext(this.db, IDB_RO); }; IndexedDB.prototype.getReadWriteContext = function() { return new IndexedDBContext(this.db, IDB_RW); diff --git a/tests/bugs/ls-depth-bug.js b/tests/bugs/ls-depth-bug.js index ac2c5ae..ec1a7a1 100644 --- a/tests/bugs/ls-depth-bug.js +++ b/tests/bugs/ls-depth-bug.js @@ -25,7 +25,7 @@ describe('sh.ls and deep directory trees', function() { done(); }); }); - }); + }).timeout(15000); it('should not crash when calling sh.ls() on wide directory layouts', function(done) { var fs = util.fs(); @@ -55,5 +55,5 @@ describe('sh.ls and deep directory trees', function() { }); }); }); - }); + }).timeout(15000); }); diff --git a/tests/lib/indexeddb.js b/tests/lib/indexeddb.js index 77ff3ea..f09ae5b 100644 --- a/tests/lib/indexeddb.js +++ b/tests/lib/indexeddb.js @@ -23,20 +23,26 @@ function IndexedDBTestProvider(name) { return callback(); } - // We have to force any other connections to close - // before we can delete a db. - if(that.provider.db) { - that.provider.db.close(); - } - - var request = indexedDB.deleteDatabase(name); function finished() { that.provider = null; _done = true; callback(); } - request.onsuccess = finished; - request.onerror = finished; + + try { + // We have to force any other connections to close + // before we can delete a db. + if(that.provider.db) { + that.provider.db.close(); + } + + var request = indexedDB.deleteDatabase(name); + request.onsuccess = finished; + request.onerror = finished; + } catch(e) { + console.log("Failed to delete test database", e); + finished(); + } } function init() { diff --git a/tests/spec/fs.stats.spec.js b/tests/spec/fs.stats.spec.js index acb5be8..6094e98 100644 --- a/tests/spec/fs.stats.spec.js +++ b/tests/spec/fs.stats.spec.js @@ -7,11 +7,12 @@ describe('fs.stats', function() { beforeEach(util.setup); afterEach(util.cleanup); - it('should be a function', function() { + it('should be a function', function(done) { var fs = util.fs(); fs.stat('/', function(error, stats) { if(error) throw error; expect(stats.isFile).to.be.a('function'); + done(); }); }); @@ -60,11 +61,12 @@ describe('fs.stats', function() { beforeEach(util.setup); afterEach(util.cleanup); - it('should be a function', function() { + it('should be a function', function(done) { var fs = util.fs(); fs.stat('/', function(error, stats) { if(error) throw error; expect(stats.isDirectory).to.be.a('function'); + done(); }); }); @@ -113,19 +115,21 @@ describe('fs.stats', function() { beforeEach(util.setup); afterEach(util.cleanup); - it('should be a function', function() { + it('should be a function', function(done) { var fs = util.fs(); fs.stat('/', function(error, stats) { if(error) throw error; expect(stats.isBlockDevice).to.be.a('function'); + done(); }); }); - it('should return false', function() { + it('should return false', function(done) { var fs = util.fs(); fs.stat('/', function(error, stats) { if(error) throw error; expect(stats.isBlockDevice()).to.be.false; + done(); }); }); }); @@ -134,19 +138,21 @@ describe('fs.stats', function() { beforeEach(util.setup); afterEach(util.cleanup); - it('should be a function', function() { + it('should be a function', function(done) { var fs = util.fs(); fs.stat('/', function(error, stats) { if(error) throw error; expect(stats.isCharacterDevice).to.be.a('function'); + done(); }); }); - it('should return false', function() { + it('should return false', function(done) { var fs = util.fs(); fs.stat('/', function(error, stats) { if(error) throw error; expect(stats.isCharacterDevice()).to.be.false; + done(); }); }); }); @@ -155,11 +161,12 @@ describe('fs.stats', function() { beforeEach(util.setup); afterEach(util.cleanup); - it('should be a function', function() { + it('should be a function', function(done) { var fs = util.fs(); fs.stat('/', function(error, stats) { if(error) throw error; expect(stats.isSymbolicLink).to.be.a('function'); + done(); }); }); @@ -208,19 +215,21 @@ describe('fs.stats', function() { beforeEach(util.setup); afterEach(util.cleanup); - it('should be a function', function() { + it('should be a function', function(done) { var fs = util.fs(); fs.stat('/', function(error, stats) { if(error) throw error; expect(stats.isFIFO).to.be.a('function'); + done(); }); }); - it('should return false', function() { + it('should return false', function(done) { var fs = util.fs(); fs.stat('/', function(error, stats) { if(error) throw error; expect(stats.isFIFO()).to.be.false; + done(); }); }); }); @@ -229,19 +238,21 @@ describe('fs.stats', function() { beforeEach(util.setup); afterEach(util.cleanup); - it('should be a function', function() { + it('should be a function', function(done) { var fs = util.fs(); fs.stat('/', function(error, stats) { if(error) throw error; expect(stats.isSocket).to.be.a('function'); + done(); }); }); - it('should return false', function() { + it('should return false', function(done) { var fs = util.fs(); fs.stat('/', function(error, stats) { if(error) throw error; expect(stats.isSocket()).to.be.false; + done(); }); }); });