Skip to content

Commit

Permalink
Cleaned up version of new BUG 666 WORKAROUND SOLUTION
Browse files Browse the repository at this point in the history
Close db before opening (ignore close error)

Now tested on builtin android.database implementation
(no Android-sqlite-connector / Android-sqlite-native-driver libs).

Ref:
- storesafe/cordova-sqlite-storage#666
- storesafe/cordova-sqlite-storage#730
  • Loading branch information
Christopher J. Brody committed Dec 14, 2017
1 parent 9f1d0a7 commit ec654b6
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 40 deletions.
3 changes: 2 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# Changes

###### cordova-sqlite-legacy-express-core 1.0.4-newdev01
###### cordova-sqlite-legacy-express-core 1.0.4

- Cleaned up workaround solution to BUG 666: close db before opening (ignore close error)
- android.database end transaction if active before closing (needed for new BUG 666 workaround solution to pass selfTest in case of builtin android.database implementation)

###### cordova-sqlite-legacy-express-core 1.0.3
Expand Down
6 changes: 2 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ This version branch contains the source code for the Android/iOS/macOS platforms

This version branch is now used to develop Android platform version without dependency on JAR or NDK library artifacts for testing on `cordova-android@7`.

**NOTICE:** Workaround solution for [BUG 666 (litehelpers/Cordova-sqlite-storage#666)](https://github.com/litehelpers/Cordova-sqlite-storage/issues/666) (possible transaction issue after window.location change change with possible data loss) is incorrect in this version branch as discussed in [this BUG 666 comment](https://github.com/litehelpers/Cordova-sqlite-storage/issues/666#issuecomment-343757398) and may continue to suffer from a possible data loss risk. New workaround solution described in [this newer BUG 666 comment](https://github.com/litehelpers/Cordova-sqlite-storage/issues/666#issuecomment-350159666) is available in newer version branches. New workaround solution to bug 666 will cause selfTest to fail on Android ref: <https://github.com/litehelpers/Cordova-sqlite-storage/pull/730>

NOTE: This version branch has some additional known [issues fixed in newer version branches](#issues-fixed-in-newer-version-branches).

<!-- END About this version branch -->
Expand Down Expand Up @@ -80,8 +78,8 @@ Use the `location` or `iosDatabaseLocation` option in `sqlitePlugin.openDatabase
- Nice overview of this and other alternatives for storing local data at: <https://www.sitepoint.com/storing-local-data-in-a-cordova-app/>
- New alternative solution for small data storage: [TheCocoaProject/ cordova-plugin-nativestorage](https://github.com/TheCocoaProject/cordova-plugin-nativestorage) - simpler "native storage of variables" for Android/iOS/Windows
- Resolved Java 6/7/8 concurrent map compatibility issue reported in [litehelpers/Cordova-sqlite-storage#726](https://github.com/litehelpers/Cordova-sqlite-storage/issues/726), THANKS to pointer by [@NeoLSN (Jason Yang/楊朝傑)](https://github.com/NeoLSN) in [litehelpers/Cordova-sqlite-storage#727](https://github.com/litehelpers/Cordova-sqlite-storage/issues/727).
- Updated workaround solution to [BUG 666 (litehelpers/Cordova-sqlite-storage#666)](https://github.com/litehelpers/Cordova-sqlite-storage/issues/666) (possible transaction issue after window.location change with possible data loss): close database if already open before opening again
- Fixed iOS/macOS platform version to use [PSPDFThreadSafeMutableDictionary.m](https://gist.github.com/steipete/5928916) to avoid threading issue ref: [litehelpers/Cordova-sqlite-storage#716](https://github.com/litehelpers/Cordova-sqlite-storage/issues/716)
- 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 moved to [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 All @@ -106,7 +104,7 @@ Use the `location` or `iosDatabaseLocation` option in `sqlitePlugin.openDatabase
- As described in [this posting](http://brodyspark.blogspot.com/2012/12/cordovaphonegap-sqlite-plugins-offer.html):
- Keeps sqlite database in known, platform specific user data location on all supported platforms (Android/iOS/macOS/Windows), which can be reconfigured on iOS/macOS. Whether or not the database on the iOS platform is synchronized to iCloud depends on the selected database location.
- No arbitrary size limit. SQLite limits described at: <http://www.sqlite.org/limits.html>
- ~~Also tested for multi-page applications with window location changes~~ _(workaround solution for BUG 666 is incorrect in this version branch)_
- Also validated for multi-page applications by internal test selfTest function.
- This project is self-contained: no dependencies on other plugins such as cordova-plugin-file
- Windows 10 UWP platform version available in TBD uses the performant C++ [doo / SQLite3-WinRT](https://github.com/doo/SQLite3-WinRT) component.
- [SQLCipher](https://www.zetetic.net/sqlcipher/) support for Android/iOS/macOS/Windows is available in: [litehelpers / Cordova-sqlcipher-adapter](https://github.com/litehelpers/Cordova-sqlcipher-adapter)
Expand Down
23 changes: 9 additions & 14 deletions SQLitePlugin.coffee.md
Original file line number Diff line number Diff line change
Expand Up @@ -250,21 +250,16 @@
# store initial DB state:
@openDBs[@dbname] = DB_STATE_INIT

# As a WORKAROUND SOLUTION to BUG litehelpers/Cordova-sqlite-storage#666
# (in the next event tick):
# 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.
nextTick =>
if not txLocks[@dbname]
myfn = (tx) ->
tx.addStatement 'ROLLBACK'
return
@addTransaction new SQLitePluginTransaction @, myfn, null, null, false, false

# UPDATED WORKAROUND SOLUTION to cordova-sqlite-storage BUG 666:
# Request to native side to close existing database
# connection in case it is already open.
# Wait for callback before opening the database
# (ignore close error).
step2 = =>
cordova.exec opensuccesscb, openerrorcb, "SQLitePlugin", "open", [ @openargs ]
return

cordova.exec step2, step2, 'SQLitePlugin', 'close', [ { path: @dbname } ]

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.4-newdev01",
"version": "1.0.4",
"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.4-newdev01">
version="1.0.4">

<name>Cordova sqlite storage plugin - legacy express core version</name>

Expand Down
17 changes: 9 additions & 8 deletions spec/www/spec/db-open-close-delete-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -593,10 +593,8 @@ var mytests = function() {
// XXX SEE BELOW: repeat scenario but wait for open callback before close/delete/reopen
// Needed to support some large-scale applications:
test_it(suiteName + ' immediate close, then delete then re-open allows subsequent queries to run', function () {
// TBD POSSIBLY BROKEN on iOS/macOS ...
// if (!isAndroid && !isWindows && !isWP8) pending(...);
// TBD CURRENTLY BROKEN DUE TO BUG 666 WORKAROUND HACK
pending('CURRENTLY BROKEN DUE TO BUG 666 WORKAROUND HACK');
// TBD ...
if (!isAndroid && !isWindows && !isWP8) pending('TBD TEST FAILURE on iOS/macOS (background processing implementation)');

var dbName = "Immediate-close-delete-Reopen.db";
var dbargs = {name: dbName, location: 'default'};
Expand Down Expand Up @@ -742,8 +740,11 @@ var mytests = function() {
});

test_it(suiteName + ' repeatedly open and close database faster (5x)', function () {
// TBD CURRENTLY BROKEN on iOS/macOS due to current background processing implementation:
if (!isAndroid && !isWindows && !isWP8) pending('CURRENTLY BROKEN on iOS/macOS (background processing implementation)');
// TBD BROKEN on iOS/macOS ...
// if (!isAndroid && !isWindows && !isWP8) pending(...);
// TBD SKIP FOR NOW
// (also seems to fail on Android in case of builtin android.database implementation)
pending('SKIP FOR NOW');

var dbName = 'repeatedly-open-and-close-faster-5x.db';
var dbargs = {name: dbName, location: 'default'};
Expand Down Expand Up @@ -866,8 +867,8 @@ var mytests = function() {
test_it(suiteName + ' repeatedly open and delete database faster (5x)', function () {
// TBD POSSIBLY BROKEN on iOS/macOS ...
// if (!isAndroid && !isWindows && !isWP8) pending(...);
// TBD CURRENTLY BROKEN DUE TO BUG 666 WORKAROUND HACK
pending('CURRENTLY BROKEN DUE TO BUG 666 WORKAROUND HACK');
// TBD CURRENTLY BROKEN DUE TO BUG 666 WORKAROUND SOLUTION
pending('CURRENTLY BROKEN DUE TO BUG 666 WORKAROUND SOLUTION');

var dbName = 'repeatedly-open-and-delete-faster-5x.db';
var dbargs = {name: dbName, location: 'default'};
Expand Down
20 changes: 9 additions & 11 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 openerrorcb, opensuccesscb, step2;
if (this.dbname in this.openDBs) {
console.log('database already open: ' + this.dbname);
nextTick((function(_this) {
Expand Down Expand Up @@ -202,18 +202,16 @@
};
})(this);
this.openDBs[this.dbname] = DB_STATE_INIT;
nextTick((function(_this) {
step2 = (function(_this) {
return function() {
var myfn;
if (!txLocks[_this.dbname]) {
myfn = function(tx) {
tx.addStatement('ROLLBACK');
};
_this.addTransaction(new SQLitePluginTransaction(_this, myfn, null, null, false, false));
}
return cordova.exec(opensuccesscb, openerrorcb, "SQLitePlugin", "open", [_this.openargs]);
cordova.exec(opensuccesscb, openerrorcb, "SQLitePlugin", "open", [_this.openargs]);
};
})(this));
})(this);
cordova.exec(step2, step2, 'SQLitePlugin', 'close', [
{
path: this.dbname
}
]);
}
};

Expand Down

0 comments on commit ec654b6

Please sign in to comment.