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

@@ -11,43 +11,57 @@ var indexedDB = global.indexedDB       ||
                 global.msIndexedDB;
 
 function IndexedDBContext(db, mode) {
-  var transaction = db.transaction(FILE_STORE_NAME, mode);
+  this.db = db;
-  this.objectStore = transaction.objectStore(FILE_STORE_NAME);
+  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();
+    var objectStore = this._getObjectStore();
-    request.onsuccess = function(event) {
+    var request = objectStore.clear();
+    request.onsuccess = function() {
       callback();
     };
-    request.onerror = function(error) {
+    request.onerror = function(event) {
-      callback(error);
+      event.preventDefault();
+      callback(event.error);
     };
-  } catch(e) {
+  } catch(err) {
-    callback(e);
+    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) {
+    request.onerror = function(event) {
-      callback(error);
+      event.preventDefault();
+      callback(event.error);
     };
-  } catch(e) {
+  } catch(err) {
-    callback(e);
+    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) {
+    request.onerror = function(event) {
-      callback(error);
+      event.preventDefault();
+      callback(event.error);
     };
-  } catch(e) {
+  } catch(err) {
-    callback(e);
+    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) {
+    request.onerror = function(event) {
-      callback(error);
+      event.preventDefault();
+      callback(event.error);
     };
-  } catch(e) {
+  } catch(err) {
-    callback(e);
+    callback(err);
   }
 };
 
@@ -114,6 +132,7 @@ IndexedDB.prototype.open = function(callback) {
     return callback();
   }
 
+  try {
     // NOTE: we're not using versioned databases.
     var openRequest = indexedDB.open(that.name);
 
@@ -131,15 +150,17 @@ IndexedDB.prototype.open = function(callback) {
       that.db = event.target.result;
       callback();
     };
-  openRequest.onerror = function onerror(error) {
+    openRequest.onerror = function onerror(event) {
-    callback(new Errors.EINVAL('IndexedDB cannot be accessed. If private browsing is enabled, disable it.'));
+      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
+  return new IndexedDBContext(this.db, IDB_RO);
-  // 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);
 };
 IndexedDB.prototype.getReadWriteContext = function() {
   return new IndexedDBContext(this.db, IDB_RW);

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);
 });

tests/lib/indexeddb.js

@@ -23,6 +23,13 @@ function IndexedDBTestProvider(name) {
       return callback();
     }
 
+    function finished() {
+      that.provider = null;
+      _done = true;
+      callback();
+    }
+
+    try {
       // We have to force any other connections to close
       // before we can delete a db.
       if(that.provider.db) {
@@ -30,13 +37,12 @@ function IndexedDBTestProvider(name) {
       }
 
       var request = indexedDB.deleteDatabase(name);
-    function finished() {
-      that.provider = null;
-      _done = true;
-      callback();
-    }
       request.onsuccess = finished;
       request.onerror = finished;
+    } catch(e) {
+      console.log("Failed to delete test database", e);
+      finished();
+    }
   }
 
   function init() {

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();
       });
     });
   });