Skip to content

Commit

Permalink
Workaround solution to BUG storesafe#666
Browse files Browse the repository at this point in the history
Internally verified by selfTest function.
  • Loading branch information
Christopher J. Brody committed Apr 20, 2017
1 parent 037f0c6 commit 5933cb7
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 74 deletions.
5 changes: 3 additions & 2 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -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 (also 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

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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
Expand Down
98 changes: 55 additions & 43 deletions SQLitePlugin.coffee.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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 = {}

Expand All @@ -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

Expand Down Expand Up @@ -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()

Expand All @@ -245,27 +248,49 @@
# 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'
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 } ]

Expand Down Expand Up @@ -645,6 +670,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
Expand Down Expand Up @@ -681,7 +712,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 ]

Expand Down Expand Up @@ -770,45 +804,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
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
2 changes: 1 addition & 1 deletion plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<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 - legacy express core version</name>

Expand Down
46 changes: 20 additions & 26 deletions www/SQLitePlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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]);
}
};
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 5933cb7

Please sign in to comment.