Workaround solution to BUG storesafe#666

Internally verified by selfTest function.
brodycj · Apr 20, 2017 · 284abb3 · 284abb3
1 parent 6b3b443
commit 284abb3
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 75 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,8 +1,9 @@
 # Changes
 
-### cordova-sqlite-legacy-express-core 1.0.0-pre2
+### cordova-sqlite-legacy-express-core 1.0.0-pre3
 
-- selfTest simulate scenario in BUG litehelpers/Cordova-sqlite-storage#666 (includes string test and test of effects of location reload/change in this version branch along with another internal check)
+- Workaround solution to BUG litehelpers/Cordova-sqlite-storage#666 (hanging transaction in case of location reload/change)
+- selfTest simulate scenario & test solution to BUG litehelpers/Cordova-sqlite-storage#666 (also includes string test and test of effects of location reload/change in this version branch, along with another internal check)
 - Drop engine constraints in package.json & plugin.xml (in this version branch)
 - Support macOS platform with builtin libsqlite3.dylib framework in this version branch
 

diff --git a/README.md b/README.md
@@ -78,6 +78,7 @@ Some other projects by [@brodybits](https://github.com/brodybits):
 
 ## Announcements
 
+- Resolved transaction problem after window.location (page) change with possible data loss ref: [litehelpers/Cordova-sqlite-storage#666](https://github.com/litehelpers/Cordova-sqlite-storage/issues/666)
 - [brodybits / cordova-sqlite-test-app](https://github.com/brodybits/cordova-sqlite-test-app) project is a CC0 (public domain) starting point (NOTE that this plugin must be added) and may also be used to reproduce issues with this plugin.
 - The Lawnchair adapter is now maintained in [litehelpers / cordova-sqlite-lawnchair-adapter](https://github.com/litehelpers/cordova-sqlite-lawnchair-adapter).
 - [litehelpers / cordova-sqlite-ext](https://github.com/litehelpers/cordova-sqlite-ext) now supports SELECT BLOB data in Base64 format on all platforms in addition to REGEXP (Android/iOS/macOS) and pre-populated database (all platforms).
@@ -125,7 +126,6 @@ Some other projects by [@brodybits](https://github.com/brodybits):
 
 ## Known issues
 
-- Transaction problem after page change WITH POSSIBLE DATA LOSS ref: [litehelpers/Cordova-sqlite-storage#666](https://github.com/litehelpers/Cordova-sqlite-storage/issues/666)
 - iOS/macOS platform version does not support certain rapidly repeated open-and-close or open-and-delete test scenarios due to how the implementation handles background processing
 - As described below, auto-vacuum is NOT enabled by default.
 - INSERT statement that affects multiple rows (due to SELECT cause or using TRIGGER(s), for example) does not report proper rowsAffected on Android

diff --git a/SQLitePlugin.coffee.md b/SQLitePlugin.coffee.md
@@ -84,7 +84,7 @@
     SQLitePlugin = (openargs, openSuccess, openError) ->
       # console.log "SQLitePlugin openargs: #{JSON.stringify openargs}"
 
-      # _should_ already be checked by openDatabase:
+      # SHOULD already be checked by openDatabase:
       if !(openargs and openargs['name'])
         throw newSQLError "Cannot create a SQLitePlugin db instance without a db name"
 
@@ -115,8 +115,8 @@
     SQLitePlugin::databaseFeatures = isSQLitePluginDatabase: true
 
     # Keep track of state of open db connections
-    # XXX TBD this will be moved and renamed or
-    # combined with txLocks.
+    # XXX FUTURE TBD this *may* be moved and renamed,
+    # or even combined with txLocks if possible.
     # NOTE: In case txLocks is renamed or replaced the selfTest has to be adapted as well.
     SQLitePlugin::openDBs = {}
 
@@ -128,14 +128,17 @@
         }
       txLocks[@dbname].queue.push t
       if @dbname of @openDBs && @openDBs[@dbname] isnt DB_STATE_INIT
-        # XXX TODO: only when queue has length of 1 [and test it!!]
+        # FUTURE TBD: rename startNextTransaction to something like
+        # triggerTransactionQueue
+        # ALT TBD: only when queue has length of 1 (and test)??
         @startNextTransaction()
 
       else
         if @dbname of @openDBs
           console.log 'new transaction is waiting for open operation'
         else
-          # XXX TBD TODO: in this case (which should not happen), should abort and discard the transaction.
+          # XXX SHOULD NOT GET HERE.
+          # FUTURE TBD TODO: in this exceptional case abort and discard the transaction.
           console.log 'database is closed, new transaction is [stuck] waiting until db is opened again!'
       return
 
@@ -220,7 +223,7 @@
           #if !@openDBs[@dbname] then call open error cb, and abort pending tx if any
           if !@openDBs[@dbname]
             console.log 'database was closed during open operation'
-            # XXX TODO [BUG #210] (and test!!):
+            # XXX TODO (WITH TEST) ref BUG litehelpers/Cordova-sqlite-storage#210:
             # if !!error then error newSQLError 'database closed during open operation'
             # @abortAllPendingTransactions()
 
@@ -245,27 +248,50 @@
         # store initial DB state:
         @openDBs[@dbname] = DB_STATE_INIT
 
+        # As a WORKAROUND SOLUTION to BUG litehelpers/Cordova-sqlite-storage#666:
+        # If the database was never opened on the JavaScript side
+        # start an extra ROLLBACK statement to abort any pending transaction
+        # (does not matter whether it succeeds or fails here).
+        # FUTURE TBD a better solution would be to send a special signal or parameter
+        # if the database was never opened on the JavaScript side.
+        if not txLocks[@dbname]
+          myfn = (tx) ->
+            #tx.addStatement 'ROLLBACK', null, null, null
+            tx.addStatement 'ROLLBACK'
+            return
+          @addTransaction new SQLitePluginTransaction @, myfn, null, null, false, false
+
         cordova.exec opensuccesscb, openerrorcb, "SQLitePlugin", "open", [ @openargs ]
 
       return
 
     SQLitePlugin::close = (success, error) ->
       if @dbname of @openDBs
         if txLocks[@dbname] && txLocks[@dbname].inProgress
-          # XXX TBD: wait for current tx then close (??)
+          # FUTURE TBD TODO ref BUG litehelpers/Cordova-sqlite-storage#210:
+          # Wait for current tx to finish then close,
+          # then abort any other pending transactions
+          # (and cleanup any other internal resources).
+          # (This would need testing!!)
           console.log 'cannot close: transaction is in progress'
           error newSQLError 'database cannot be closed while a transaction is in progress'
           return
 
         console.log 'CLOSE database: ' + @dbname
 
-        # XXX [BUG #209] closing one db handle disables other handles to same db
+        # NOTE: closing one db handle disables other handles to same db
+        # FUTURE TBD TODO ref litehelpers/Cordova-sqlite-storage#210:
+        # Add a dispose method to simply invalidate the
+        # current database object ("this")
         delete @openDBs[@dbname]
 
         if txLocks[@dbname] then console.log 'closing db with transaction queue length: ' + txLocks[@dbname].queue.length
         else console.log 'closing db with no transaction lock state'
 
-        # XXX [BUG #210] TODO: when closing or deleting a db, abort any pending transactions [and test it!!]
+        # XXX TODO BUG litehelpers/Cordova-sqlite-storage#210:
+        # abort all pending transactions (with error callback)
+        # when closing a database (needs testing!!)
+        # (and cleanup any other internal resources)
 
         cordova.exec success, error, "SQLitePlugin", "close", [ { path: @dbname } ]
 
@@ -645,6 +671,12 @@
         new SQLitePlugin openargs, okcb, errorcb
 
       deleteDatabase: (first, success, error) ->
+        # XXX TODO BUG litehelpers/Cordova-sqlite-storage#367:
+        # abort all pending transactions (with error callback)
+        # when deleting a database
+        # (and cleanup any other internal resources)
+        # NOTE: This should properly close the database
+        # (at least on the JavaScript side) before deleting.
         args = {}
 
         if first.constructor == String
@@ -681,7 +713,10 @@
 
         args.dblocation = dblocation
 
-        # XXX [BUG #210] TODO: when closing or deleting a db, abort any pending transactions (with error callback)
+        # XXX TODO BUG litehelpers/Cordova-sqlite-storage#367 (repeated here):
+        # abort all pending transactions (with error callback)
+        # when deleting a database
+        # (and cleanup any other internal resources)
         delete SQLitePlugin::openDBs[args.path]
         cordova.exec success, error, "SQLitePlugin", "delete", [ args ]
 
@@ -770,45 +805,23 @@
 
       step2: (successcb, errorcb) ->
         SQLiteFactory.openDatabase {name: SelfTest.DBNAME, location: 'default'}, (db) ->
-          # TX FAILURE EXPECTED DUE TO BUG litehelpers/Cordova-sqlite-storage#666:
+          # TX SHOULD SUCCEED to demonstrate solution to BUG litehelpers/Cordova-sqlite-storage#666:
           db.transaction (tx) ->
             tx.executeSql 'SELECT ? AS myResult', [null], (ignored, resutSet) ->
-              # Extra sql success callback ignored:
+              if !resutSet.rows
+                return SelfTest.finishWithError errorcb, 'Missing resutSet.rows'
+              if !resutSet.rows.length
+                return SelfTest.finishWithError errorcb, 'Missing resutSet.rows.length'
+              if resutSet.rows.length isnt 1
+                return SelfTest.finishWithError errorcb,
+                  "Incorrect resutSet.rows.length value: #{resutSet.rows.length} (expected: 1)"
+              SelfTest.step3 successcb, errorcb
               return
             return
-
           , (txError) ->
-            # EXPECTED RESULT DUE TO BUG litehelpers/Cordova-sqlite-storage#666:
-            if !txError
-              return SelfTest.finishWithError errorcb, 'Missing txError object'
-            # second try should work:
-            db.transaction (tx2) ->
-              tx2.executeSql 'SELECT ? AS myResult', [null], (ignored, resutSet) ->
-                if !resutSet.rows
-                  return SelfTest.finishWithError errorcb, 'Missing resutSet.rows'
-                if !resutSet.rows.length
-                  return SelfTest.finishWithError errorcb, 'Missing resutSet.rows.length'
-                if resutSet.rows.length isnt 1
-                  return SelfTest.finishWithError errorcb,
-                SelfTest.step3 successcb, errorcb
-                return
-              return
-            , (tx2_err) ->
-              return SelfTest.finishWithError errorcb, "UNEXPECTED TRANSACTION ERROR: #{tx2_err}"
-            return
-
-          , () ->
-            # TX SUCCESS POSSIBLE FOR Android (android.database) ONLY
-            # NOTE: Windows 10 (UWP) mobile platform also shows "Android" in navigator.userAgent,
-            # filtered out here.
-            # FUTURE TBD android.database implementation should be fixed to report error in this case.
-            if /Android/.test(navigator.userAgent) and not /Windows /.test(navigator.userAgent)
-              return SelfTest.step3 successcb, errorcb
-            # OTHERWISE:
-            # TX SUCCESS NOT EXPECTED DUE TO BUG litehelpers/Cordova-sqlite-storage#666:
-            return SelfTest.finishWithError errorcb, 'UNEXPECTED SUCCESS ref: litehelpers/Cordova-sqlite-storage#666'
+            # NOT EXPECTED:
+            return SelfTest.finishWithError errorcb, "UNEXPECTED TRANSACTION ERROR: #{txError}"
           return
-
         , (open_err) ->
           SelfTest.finishWithError errorcb, "Open database error: #{open_err}"
         return

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "cordova-sqlite-legacy-express-core",
-  "version": "1.0.0-pre2",
+  "version": "1.0.0-pre3",
   "description": "Native interface to SQLite for PhoneGap/Cordova (legacy express core version)",
   "cordova": {
     "id": "cordova-sqlite-legacy-express-core",

diff --git a/plugin.xml b/plugin.xml
@@ -2,9 +2,9 @@
 <plugin xmlns="http://www.phonegap.com/ns/plugins/1.0"
     xmlns:android="http://schemas.android.com/apk/res/android"
     id="cordova-sqlite-legacy-express-core"
-    version="1.0.0-pre2">
+    version="1.0.0-pre3">
 
-    <name>Cordova sqlite storage plugin - express core version</name>
+    <name>Cordova sqlite storage plugin - XXX legacy express core version</name>
 
     <license>MIT</license>
 

diff --git a/www/SQLitePlugin.js b/www/SQLitePlugin.js
@@ -162,7 +162,7 @@
   };
 
   SQLitePlugin.prototype.open = function(success, error) {
-    var openerrorcb, opensuccesscb;
+    var myfn, openerrorcb, opensuccesscb;
     if (this.dbname in this.openDBs) {
       console.log('database already open: ' + this.dbname);
       nextTick((function(_this) {
@@ -201,6 +201,12 @@
         };
       })(this);
       this.openDBs[this.dbname] = DB_STATE_INIT;
+      if (!txLocks[this.dbname]) {
+        myfn = function(tx) {
+          tx.addStatement('ROLLBACK');
+        };
+        this.addTransaction(new SQLitePluginTransaction(this, myfn, null, null, false, false));
+      }
       cordova.exec(opensuccesscb, openerrorcb, "SQLitePlugin", "open", [this.openargs]);
     }
   };
@@ -681,32 +687,20 @@
         location: 'default'
       }, function(db) {
         db.transaction(function(tx) {
-          tx.executeSql('SELECT ? AS myResult', [null], function(ignored, resutSet) {});
-        }, function(txError) {
-          if (!txError) {
-            return SelfTest.finishWithError(errorcb, 'Missing txError object');
-          }
-          db.transaction(function(tx2) {
-            tx2.executeSql('SELECT ? AS myResult', [null], function(ignored, resutSet) {
-              if (!resutSet.rows) {
-                return SelfTest.finishWithError(errorcb, 'Missing resutSet.rows');
-              }
-              if (!resutSet.rows.length) {
-                return SelfTest.finishWithError(errorcb, 'Missing resutSet.rows.length');
-              }
-              if (resutSet.rows.length !== 1) {
-                return SelfTest.finishWithError(errorcb);
-              }
-              SelfTest.step3(successcb, errorcb);
-            });
-          }, function(tx2_err) {
-            return SelfTest.finishWithError(errorcb, "UNEXPECTED TRANSACTION ERROR: " + tx2_err);
+          tx.executeSql('SELECT ? AS myResult', [null], function(ignored, resutSet) {
+            if (!resutSet.rows) {
+              return SelfTest.finishWithError(errorcb, 'Missing resutSet.rows');
+            }
+            if (!resutSet.rows.length) {
+              return SelfTest.finishWithError(errorcb, 'Missing resutSet.rows.length');
+            }
+            if (resutSet.rows.length !== 1) {
+              return SelfTest.finishWithError(errorcb, "Incorrect resutSet.rows.length value: " + resutSet.rows.length + " (expected: 1)");
+            }
+            SelfTest.step3(successcb, errorcb);
           });
-        }, function() {
-          if (/Android/.test(navigator.userAgent) && !/Windows /.test(navigator.userAgent)) {
-            return SelfTest.step3(successcb, errorcb);
-          }
-          return SelfTest.finishWithError(errorcb, 'UNEXPECTED SUCCESS ref: litehelpers/Cordova-sqlite-storage#666');
+        }, function(txError) {
+          return SelfTest.finishWithError(errorcb, "UNEXPECTED TRANSACTION ERROR: " + txError);
         });
       }, function(open_err) {
         return SelfTest.finishWithError(errorcb, "Open database error: " + open_err);