Change of PhoneGap SQLitePlugin API to be closer to the W3 SQL API

[brodycj]

From issue #11: it would be great to use almost the same API as the HTML5/W3 SQL API.

I hereby propose a couple small changes to the plugin in the executeSql() member function of the transaction object and the PGSQLitePlugin database object, so that the array of values is kept separate from the SQL string itself. This change makes the SQLitePlugin API closer to the HTML5/W3 SQL API.

This change would also make the application code more portable to the existing Android Storage plugin from: https://github.com/apache/incubator-cordova-android/blob/master/framework/src/org/apache/cordova/Storage.java

I made some changes to the lawnchair adapter but I did not have a chance to test them. Perhaps it would be better to put these changes together with the Cordova version.

[brodycj]

This pull request is now merged with #23 and is hereby closed.

[brodycj]

Thanks @marcucio I will adapt the CoffeeScript version myself.

[brodycj]

@marcucio I do not have much time to test these changes, however I found a few things that I would like to do differently. I made these changes in the CoffeeScript version and compiled to Javascript but I do not have time to do much testing. Here are some of the things I would do differently:

• I checked a few places on the Internet and I believe the success and error callbacks were already in the right order. Please take a quick look at the example in: http://www.w3.org/TR/offline-webapps/#sql
• In PGSQLitePluginTransaction.prototype.executeSql it is possible to save the reference to the current transaction object in a "self" variable and pass that reference into the success and error callback functions.
• In PGSQLitePluginTransaction.prototype.complete incude the tx reference in the callbacks
• In PGSQLitePlugin.prototype.executeSql returning the result object just like PGSQLitePluginTransaction.prototype.executeSql

Following your suggestion, I tried making the following changes to pgsqlite_plugin.coffee and I hope you can follow them. Please let me know if you want me to post a special commit with the changes to the Javascript version.

diff --git a/src/pgsqlite_plugin.coffee b/src/pgsqlite_plugin.coffee
index a3f230f..26ed6e7 100644
--- a/src/pgsqlite_plugin.coffee
+++ b/src/pgsqlite_plugin.coffee
@@ -50,7 +50,13 @@ class root.PGSQLitePlugin

   executeSql: (sql, params, success, error) ->
     throw new Error "Cannot executeSql without a query" unless sql
-    opts = getOptions({ query: [sql].concat(params || []), path: @dbPath }, success, error)
+    successcb = (execres) ->
+      res =
+        item: (i) ->
+          execres[i]
+        length: execres.length
+      success(res)
+    opts = getOptions({ query: [sql].concat(params || []), path: @dbPath }, successcb, error)
     PhoneGap.exec("PGSQLitePlugin.backgroundExecuteSql", opts)
     return

@@ -79,14 +85,28 @@ class root.PGSQLitePluginTransaction
     @executes = []

   executeSql: (sql, params, success, error) ->
-    @executes.push getOptions({ query: [sql].concat(params || []), path: @dbPath }, success, error)
+    txself = @
+    successcb = (execres) ->
+      res =
+        item: (i) ->
+          execres[i]
+        length: execres.length
+      success(txself, res)
+    errorcb = (res) ->
+      error(txself, res)
+    @executes.push getOptions({ query: [sql].concat(params || []), path: @dbPath }, successcb, errorcb)
     return

   complete: (success, error) ->
     throw new Error "Transaction already run" if @__completed
     @__completed = true
+    txself = @
+    successcb = (res) ->
+      success(txself, res)
+    errorcb = (res) ->
+      error(txself, res)
     begin_opts = getOptions({ query: [ "BEGIN;" ], path: @dbPath })
-    commit_opts = getOptions({ query: [ "COMMIT;" ], path: @dbPath }, success, error)
+    commit_opts = getOptions({ query: [ "COMMIT;" ], path: @dbPath }, successcb, errorcb)
     executes = [ begin_opts ].concat(@executes).concat([ commit_opts ])
     opts = { executes: executes }
     PhoneGap.exec("PGSQLitePlugin.backgroundExecuteSqlBatch", opts)

Chris

[marcucio]

I agree with all of your changes except for one, here is the spec I was going off of (and webkit follows it):

http://www.w3.org/TR/webdatabase/

Only in the transaction is the error callback before the success callback, everywhere else success comes before error:

4.3 Asynchronous database API

interface Database {
void transaction(in SQLTransactionCallback callback, in optional SQLTransactionErrorCallback errorCallback, in optional SQLVoidCallback successCallback);

I am not sure why in this one situation it is backwards but this is how it works in webkit

[marcucio]

if you want to build the .js I can test it out

[marcucio]

I know why it seems backwards to me,

void transaction(in SQLTransactionCallback callback, in optional SQLTransactionErrorCallback errorCallback, in optional SQLVoidCallback successCallback);

it has the usual SQLTransactionCallback and SQLTransactionErrorCallback but there is an optional successCallback as the third callback that is called after the entire transaction completes.

[brodycj]

Sorry Mike, I missed that one. Give me a few minutes and I will post a version for you to test.

[brodycj]

Can you please check the changes from the following commit: https://github.com/chbrody/Phonegap-SQLitePlugin/commit/d864bae2eb80a005ac327053983b36ec5e4d79b4 (W3-SQL-API-test-1 branch)?

[brodycj]

@marcucio I have reorganized my fork quite a bit and it has a new location: https://github.com/chbrody/Cordova-SQLiteNative so just look at its default master branch.

You will see both "Legacy PhoneGap" and new Cordova versions, and I have checked in Lawnchair automatic testing suites to help verify that I don't break anything major. I have checked in a couple API changes in the Legacy-PhoneGap-iPhone subdirectory to make db.transaction() and tx.executeSql() follow the HTML5 SQL API more closely. Here are the changes in the CoffeeScript:

--- a/Legacy-PhoneGap-iPhone/src/pgsqlite_plugin.coffee
+++ b/Legacy-PhoneGap-iPhone/src/pgsqlite_plugin.coffee
@@ -54,7 +54,7 @@ class root.PGSQLitePlugin
     PhoneGap.exec("PGSQLitePlugin.backgroundExecuteSql", opts)
     return

-  transaction: (fn, success, error) ->
+  transaction: (fn, error, success) ->
     t = new root.PGSQLitePluginTransaction(@dbPath)
     fn(t)
     t.complete(success, error)
@@ -79,14 +79,36 @@ class root.PGSQLitePluginTransaction
     @executes = []

   executeSql: (sql, values, success, error) ->
-    @executes.push getOptions({ query: [sql].concat(values || []), path: @dbPath }, success, error)
+    txself = @
+    successcb = null
+    if success
+      successcb = (execres) ->
+        saveres = execres
+        res =
+          rows:
+            item: (i) ->
+              saveres.rows[i]
+            length: saveres.rows.length
+          rowsAffected: saveres.rowsAffected
+          insertId: saveres.insertId || null
+        success(txself, res)
+    errorcb = null
+    if error
+      errorcb = (res) ->
+        error(txself, res)
+    @executes.push getOptions({ query: [sql].concat(values || []), path: @dbPath }, successcb, errorcb)
     return

   complete: (success, error) ->
     throw new Error "Transaction already run" if @__completed
     @__completed = true
+    txself = @
+    successcb = (res) ->
+      success(txself, res)
+    errorcb = (res) ->
+      error(txself, res)
     begin_opts = getOptions({ query: [ "BEGIN;" ], path: @dbPath })
-    commit_opts = getOptions({ query: [ "COMMIT;" ], path: @dbPath }, success, error)
+    commit_opts = getOptions({ query: [ "COMMIT;" ], path: @dbPath }, successcb, errorcb)
     executes = [ begin_opts ].concat(@executes).concat([ commit_opts ])
     opts = { executes: executes }
     PhoneGap.exec("PGSQLitePlugin.backgroundExecuteSqlBatch", opts)

There is a section in the README to describe the change to tx.executeSql(). Can you take a look at these changes and perhaps test Legacy-PhoneGap-iPhone/build/pgsqlite_plugin.js whenever you get a chance?

Thanks,
Chris

[marcucio]

ok, I will check it out this afternoon, thanks

[brodycj]

@marcucio did you get a chance to test it? Can I continue with the new Cordova 1.5 version or do you need support for a legacy PhoneGap version?

[marcucio]

I tested it out briefly but I would like to do more in depth testing. This is a busy day but hopefully I can get to it today, if not, tomorrow I have all day to look into it.

You can continue with Cordova, I plan on taking your .m and .js files and renaming them to be consistent with the other phonegap plugins (maybe SQLitePlugin?) and just maintaining those files in my githib. I will be on the old PhoneGap for a while because I am at the tail end of a release and do not want to switch over yet for this version.

I will let you know if I find any bugs so that you can fix it in your branch, thanks

On Apr 3, 2012, at 6:00 AM, Chris Brody wrote:

@marcucio did you get a chance to test it? Can I continue with the new Cordova 1.5 version or do you need support for a legacy PhoneGap version?

---

Reply to this email directly or view it on GitHub:
#24 (comment)

[brodycj]

@marcucio I really appreciate your efforts to test the changes for me. We are all of us busy so we just do things the best we can. Your comment is a confirmation to me to keep the old Legacy PhoneGap version for a while, for those who still need it.

[brodycj]

@marcucio I have just pushed some more changes to my fork, namely to use sqliteNative.openDatabase() instead of a constructor, and with some added parameters, to maintain better consistency with the HTML5/W3 SQL API. I am happy whether or not you want to test this change.

[brodycj]

And I want to rename (PG)SQLitePlugin to SQLiteNative also in the Objective-C class and in [PhoneGap|Cordova].plist. I had trouble doing this in the past and don't have much time to figure this one out right now.

[marcucio]

I was debating on using the term 'Plugin' or 'Native'. I think native is assumed by being a phonegap plugin, it looks like most people just use the name for what it is without 'Native' or 'Plugin':

https://github.com/phonegap/phonegap-plugins/tree/master/iOS

On Apr 3, 2012, at 8:44 AM, Chris Brody wrote:

And I want to rename (PG)SQLitePlugin to SQLiteNative also in the Objective-C class and in [PhoneGap|Cordova].plist. I had trouble doing this in the past and don't have much time to figure this one out right now.

---

Reply to this email directly or view it on GitHub:
#24 (comment)

[brodycj]

Whatever makes the most sense for people to use. The idea is that people know they are getting SQLite, they are getting something that is "almost like" the HTML5 SQL API but in a form in which they can control where the data is being stored. The only thing I don't like about using the term 'Plugin' in the name is that most of the plugins are being "installed" in the window.plugins object. BTW I was a little too fast to rename my fork, I may change it back sometime.

[brodycj]

Also, do you see any reason to keep the db.executeSql() function? It is not like the HTML5/W3 standard and I don't really want to support that one in the Android version.

[brodycj]

@marcucio my idea right now is to rename back to SQLitePlugin, this will make its purpose very clear, and I still want to get rid of db.executeSql() since it is non-standard. I have no time right now, maybe later.

[marcucio]

I agree with removing db.executeSql() I do not this this is in the spec. SQLitePlugin seems like the best name. We might also want to put in the read me something like this:

The reason why this plugin is necessary is because iOS does not store its web SQL database in a location which will be backed up. If the user upgrades to a newer version of iOS the databases will be lost. This plugin gives you the web SQL API to store data on the device where it will be backed up by iOS.

[marcucio]

https://github.com/chbrody/Cordova-SQLiteNative/blob/master/Legacy-PhoneGap-iPhone/build/pgsqlite_plugin.js seems to work pretty good for my app, I think we got a winner.

[brodycj]

@marcucio thank you for your test and your feedback, the Cordova version is working well for me too so I plan to make another baseline later today or tonight. In #23 people want me to get rid of the Legacy PG version but I am not ready for this if people like you still want to use this for pre-Cordova. Are you willing to add a comment to #23 that you still need the legacy PhoneGap version?

[marcucio]

added comment... one bug I am looking into. when I save null values into the db and then retrieving them I get back the string "". I am trying to find out if this is a bug in getting the value or setting the value, I will report back anything I find.

[marcucio]

I really don't like this fix but it works for me:

https://github.com/marcucio/Phonegap-SQLitePlugin/commit/8660ba16fd796ad764e0c8a1eda906c47dbf8b57

I don't have time to debug the phonegap code to see exactly where the null value from the js sql parameters becomes "" in the .m but this is the first place in the plugin code where it happens.

Also since I do not have 1.5 installed I cannot see if this is an issue in that version too or if it is just an issue in 1.4

[brodycj]

@marcucio thank you for your time and attention to do the testing, fixing and reporting. To be honest, we really should be using automatic testing to check these kinds of problems. I had already included a test suite from Lawnchair that should be able to test both the Lawnchair adapter and the plugin itself at a basic level. I am thinking about making a special test to reproduce this problem and verify the fix.

[marcucio]

@chbrody I agree, setting/getting null values should be in the test suite. This fix probably shouldn't be put in until someone else can repro the issue.

[brodycj]

And I do not do anything with this one for legacy PG 1.4-, I am sure it should not be a problem for you. I suspect you could do a workaround at the application level though it is probably better to just to keep the fix this one in your own fork.

[brodycj]

I took a quick look at the issue list and found #15 seems to report this one. I do want to come up with a test in the Cordova 1.5+ version but probably not for today.

[brodycj]

For your work are you good to keep it under the mit license?

On Sunday, April 8, 2012, Mike wrote:

FYI I my code is now located here:
https://github.com/marcucio/Cordova-Plugins

I also am in the middle of an Android version of this this plugin, I tried
your Android version but it did not take advantage of transactions on the
native side so it was too slow for me.

---

Reply to this email directly or view it on GitHub:

#24 (comment)

[brodycj]

@marcucio a few issues. I tried running my small test program (see https://github.com/chbrody/Cordova-SQLitePlugin/blob/master/DroidGap/assets/www/index.html lines 25-46, ignore lines 50-62), changing from my_openDatabase() to new SQLitePlugin() and also from res.rows.item(0).cnt to res.rows[0].cnt, and it looks like some plugin SQL is executed but I do not end getting any response to the select SQL. Can you post a small test program so I can check how to run your version of the Android plugin?

Also I looked again through some plugins under https://github.com/phonegap/phonegap-plugins/tree/master/Android and it looked like they to use src.com.phonegap.plugin (no 's'). Doesn't make such a big difference to me, I am just as happy if we use something like com.phonegap.sqlitePlugin.SQLitePlugin.

Finally, it looks like you are taking generated Javascript and making adaptations for the Android version. If I get this working, I would like to readapt from CoffeeScript sometime in the near future. I am happy to do this, probably sometime next weekend.

Chris

[marcucio]

I'm fine with MIT licence, dosen't matter to me.

I'll check out your tests, we should be able to use the same exact test code from iOS since the API should be exactly the same (except that I took out the executeSql function that wasn't part of transaction). I really didn't test it except for using it in my app so I will try to make it pass the same test that we use for iOS.

As far as the plugin (or plugins) name, I guess the random projects I looked at had plugins, but I just checked out the phoneGap wiki and they had it your way with plugin:

http://wiki.phonegap.com/w/page/36753494/How%20to%20Create%20a%20PhoneGap%20Plugin%20for%20Android

I figured you were going to make the .coffee for this, it would probably clean up some inconsistencies with my coding style.

[marcucio]

@chbrody I investigated that bug that you talked about, I fixed one small bug which prevented it from running but it seems like there is a larger issue with nested callbacks with both the iOS and Android plugins. The situation is as follows:

the following code should output:

222
333
444
555

but with the android AND the iOS plugin it outputs:

222
333
555

db.transaction(function(tx) 
{
    write_log('app->do_test - 222');
    tx.executeSql('DROP TABLE IF EXISTS test_table');
    tx.executeSql('CREATE TABLE IF NOT EXISTS test_table (id integer primary key, data text, data_num integer)');
    return tx.executeSql("INSERT INTO test_table (data, data_num) VALUES (?,?)", ["test", 100], function(tx, res) 
    {
        write_log('app->do_test - 333');
        tx.executeSql("select count(id) as cnt from test_table;", [], function(tx, res) 
        {
            write_log('app->do_test - 444');
            console.log("rows.length: " + res.rows.length + " -- should be 1");
            return console.log("rows[0].cnt: " + res.rows.item(0).cnt + " -- should be 1");
        });

    }, function(e) 
    {
        write_log('app->do_test error - 666');
        return console.log("ERROR: " + e.message);
    });

}, function()
{
    write_log('app->do_test error - 777');
}, function()
{
    write_log('app->do_test success - 555');
});

The problem is that by the time we get to the callback with 444 the transaction is already submitted via PhoneGap.exec.

This is a tricky issue because we want to batch our transactions together before sending them to PhoneGap for speed reasons but it makes this situation tricky to code.

I don't need this functionality for my apps but both the iOS and Android plugins do not do this correctly. I am looking into a few ways to fix this but I do not have time to fix it if it is going to take a few days to come up with a solution.

[marcucio]

ps, sorry about "write_log" it is my internal logging function

[brodycj]

@marcucio don't worry about write_log() any half-decent hacker should be able to figure that one out LOL. I was thinking about this issue and to be honest I am questioning if my sample is really supported by the H5 SQL API. I will take another look and maybe do some more testing sometime later today.

[brodycj]

Hi @marcucio I tried running my test on normal HTML5/Web SQL API, using the database object returned by window.openDatabase() (iOS version only) and my test was working OK. So perhaps you should make a clear label on your version that it is working only with batches. I am thinking about how I can change my version to work between individual queries and batches to get the best of both worlds, sometime in the future.

Chris

[marcucio]

I think i have a fix for Android but I have to test it out, hopefully I will have time today

[brodycj]

Awesome, please keep me posted, I will use it if it is working.

[marcucio]

Here is my fix:

marcucio/Cordova-Plugins@0351174

basically when the final transaction callback is run i check to see if there were any more queries that have not been run yet and if so I submit the transaction again with the queries that were not run yet.

[brodycj]

Mike @marcucio that sounds very good. I have just finished some changes to my fork to use sqlitePlugin.openDatabase() for both iOS and Android versions, and also a common Lawnchair adapter for both iOS and Android, so I will try your change and incorporate it if it is working for me.

[brodycj]

Good news and bad news. My test program worked, and all but one of the Lawnchair test suites are working. There is a test with combined batch set and get that is not working:

test('get batch functionality', function() {
    QUnit.expect(3);
    QUnit.stop(500);

    var t = [{key:'test-get'},{key:'test-get-1'}]
    store.batch(t, function() {
        this.get(['test-get','test-get-1'], function(r) {
            equals(r[0].key, 'test-get', "get first object");
            equals(r[1].key, 'test-get-1', "get second object");
            equals(r.length, t.length, "should batch get")
            QUnit.start()
        })
    }) 
});

To be honest, I do not really feel ready to include your version until I am more confident that absolutely nothing will go wrong. I am trying to get as close an API match as possible and I do not really want to take a risk without a better understanding of how these changes are working. I do not really have much time so later this week I will just issue a pull request with what I have and look at your changes in the future.

I do want to fork your version and do some more testing in a separate repository sometime in the future. Maybe if you want to give your repository a more descriptive name, to indicate that this is for native SQLite transactions, I will make a fork and propose some changes for the future.

Chris

[marcucio]

No problem, how are you testing this, are you setting up a new Android project with the lawnchair files? If so what files are you including, I see you have a lawnchair-adapter folder but also a lawnchair-adapter-test folder in your Droid gap folder.

I woud like to get set up with this so that I can fix this bug.

On Apr 9, 2012, at 3:51 PM, Chris Brody wrote:

Good news and bad news. My test program worked, and all but one of the Lawnchair test suites are working. There is a test with combined batch set and get that is not working:

test('get batch functionality', function() {
QUnit.expect(3);
QUnit.stop(500);

   var t = [{key:'test-get'},{key:'test-get-1'}]
   store.batch(t, function() {
       this.get(['test-get','test-get-1'], function(r) {
           equals(r[0].key, 'test-get', "get first object");
           equals(r[1].key, 'test-get-1', "get second object");
           equals(r.length, t.length, "should batch get")
           QUnit.start()
       })
   }) 

});

To be honest, I do not really feel ready to include your version until I am more confident that absolutely nothing will go wrong. I am trying to get as close an API match as possible and I do not really want to take a risk without a better understanding of how these changes are working. I do not really have much time so later this week I will just issue a pull request with what I have and look at your changes in the future.

I do want to fork your version and do some more testing in a separate repository sometime in the future. Maybe if you want to give your repository a more descriptive name, to indicate that this is for native SQLite transactions, I will make a fork and propose some changes for the future.

Chris

---

Reply to this email directly or view it on GitHub:
#24 (comment)

[marcucio]

we should also probably file a bug under the iPhone/iOS versions so that people know that they do not handle nested callbacks

[brodycj]

we should also probably file a bug under the iPhone/iOS versions so that people know that this does not handle nested callbacks
I think that was only in the version with your fixes, I will test the
new API version again.

[brodycj]

Copy the contents of Lawnchair-adapter/test-www into the assets/www
folder, make sure you got the right version of cordova-1.5.0.js, your
version of SQLitePlugin.js, and you have to make the following changes
to connect the lawnchair adapter to your version of SQLitePlugin.js:

$ diff -u Cordova-SQLitePlugin/Lawnchair-adapter/test-www
example/assets/www | sed "s/^/ /"

diff -u Cordova-SQLitePlugin/Lawnchair-adapter/test-www/Lawnchair-sqlitePlugin.js

example/assets/www/Lawnchair-sqlitePlugin.js

--- Cordova-SQLitePlugin/Lawnchair-adapter/test-www/Lawnchair-sqlitePlugin.js   2012-04-09

20:24:30.000000000 +0200
+++ example/assets/www/Lawnchair-sqlitePlugin.js 2012-04-09
21:05:43.000000000 +0200
@@ -23,7 +23,8 @@

         //valid: function() { return !!(window.openDatabase) },
         //valid: function() { return !!(window.my_openDatabase) },
-        valid: function() { return !!(sqlitePlugin.openDatabase) },
+        //valid: function() { return !!(sqlitePlugin.openDatabase) },
+        valid: function() { return !!(window.SQLitePlugin) },

         init: function (options, callback) {
             var that   = this
@@ -34,8 +35,10 @@
             //this.db = openDatabase(this.name, '1.0.0', this.name, 65536)
             //this.db = my_openDatabase(this.name, '1.0.0',

this.name, 65536)
//this.db = window.my_openDatabase("Database", "1.0",
"PhoneGap Demo", 200000);
- this.db = sqlitePlugin.openDatabase(this.name,
'1.0.0', this.name, 65536)
+ //this.db = sqlitePlugin.openDatabase(this.name,
'1.0.0', this.name, 65536)
//this.db = sqlitePlugin.openDatabase("Database",
"1.0", "PhoneGap Demo", 200000);
+ //this.db = sqlitePlugin.openDatabase(this.name,
'1.0.0', this.name, 65536)
+ this.db = new SQLitePlugin(this.name, '1.0.0',
this.name, 65536)
this.db.transaction(function (t) {
t.executeSql(create, [], win, fail)
})
Only in example/assets/www: SQLitePlugin.js
Only in example/assets/www: cordova-1.5.0.js
Common subdirectories:
Cordova-SQLitePlugin/Lawnchair-adapter/test-www/lib and
example/assets/www/lib

You should be able to see the Lawnchair test suite running in the
Android window.

On Mon, Apr 9, 2012 at 10:33 PM, Mike
[email protected]
wrote:

No problem, how are you testing this, are you setting up a new Android project with the lawnchair files? If so what files are you including,  I see you have a lawnchair-adapter folder but also a lawnchair-adapter-test folder in your Droid gap folder.

I woud like to get set up with this so that I can fix this bug.

On Apr 9, 2012, at 3:51 PM, Chris Brody wrote:

Good news and bad news. My test program worked, and all but one of the Lawnchair test suites are working. There is a test with combined batch set and get that is not working:

   test('get batch functionality', function() {
       QUnit.expect(3);
       QUnit.stop(500);

       var t = [{key:'test-get'},{key:'test-get-1'}]
       store.batch(t, function() {
           this.get(['test-get','test-get-1'], function(r) {
               equals(r[0].key, 'test-get', "get first object");
               equals(r[1].key, 'test-get-1', "get second object");
               equals(r.length, t.length, "should batch get")
               QUnit.start()
           })
       })
   });

To be honest, I do not really feel ready to include your version until I am more confident that absolutely nothing will go wrong. I am trying to get as close an API match as possible and I do not really want to take a risk without a better understanding of how these changes are working. I do not really have much time so later this week I will just issue a pull request with what I have and look at your changes in the future.

I do want to fork your version and do some more testing in a separate repository sometime in the future. Maybe if you want to give your repository a more descriptive name, to indicate that this is for native SQLite transactions, I will make a fork and propose some changes for the future.

Chris

---

Reply to this email directly or view it on GitHub:
#24 (comment)

---

Reply to this email directly or view it on GitHub:
#24 (comment)

[brodycj]

Ignore the commented lines, the point is to check the right object and call the right function to open the database. Maybe you can take the code from the end of DroidGap/assets/www/SQLitePlugin.js and adapt it for your version:

$ tail Cordova-SQLitePlugin/DroidGap/assets/www/SQLitePlugin.js | sed 's/^/ /'

 * @constructor
 */

window.sqlitePlugin = {
    openDatabase: function(name, version, desc, size) {
        window.dddb = new DDB();
                return DDB_openDatabase(name, version, desc, size);
        }
};

[marcucio]

@chbrody I won't be able to do too much more on this for a while but I had some important checkins for the Android plugin today. I'm not sure if anything I did fixed the errors in the lawnchair tests because I din't have time to get set up, but they might have fixed the issues. I also changes the openDatabase API to match yours.

https://github.com/marcucio/Cordova-Plugins/commits/master

[brodycj]

Mike, thank for your efforts, I think none of us really have much
time. I will look at your changes later this morning and let you know
what I find out.

Chris

On Tue, Apr 10, 2012 at 2:57 AM, Mike
[email protected]
wrote:

@chbrody I won't be able to do too much more on this for a while but I had some important checkins for the Android plugin today. I'm not sure if anything I did fixed the errors in the lawnchair tests because I din't have time to get set up, but they might have fixed the issues. I also changes the openDatabase API to match yours.

https://github.com/marcucio/Cordova-Plugins/commits/master

---

Reply to this email directly or view it on GitHub:
#24 (comment)

[marcucio]

one last thought, I added a flag to tun on or off my fix for nested queries. Even with it set to false it is much faster than the old version but if you do transactions with 1000s of queries each and if you don't use mulit level callbacks (one level of callback works) then setting this to true will make the app much faster:

this.optimization_no_nested_callbacks = false;//if set to 'true' large batches of queries within a transaction will be much faster but you lose the ability to do muiti level nesting of executeSQL callbacks

[marcucio]

Thanks for your efforts too, I'm happy where the plugins are. They might need a little tweaking here and there but I think they are in a good place.

Were you planning on submitting to the official phonegap plugins repository?

[brodycj]

this.optimization_no_nested_callbacks = false;//if set to 'true' large batches of queries within a transaction will be much faster but you lose the ability to do muiti level nesting of executeSQL callbacks

You are not going to believe: if this is set to false I get 2 test
failures but if this is set to true I am not getting any test
failures. I am investigating this more carefully, maybe we can just
take this option out and always do the faster batching.

[brodycj]

P.S. I was not able to build with the newer version of
SQLitePlugin.java with the cursor field IDs so I am just testing with
the older version.

On Tue, Apr 10, 2012 at 8:00 AM, Chris Brody [email protected] wrote:

this.optimization_no_nested_callbacks = false;//if set to 'true' large batches of queries within a transaction will be much faster but you lose the ability to do muiti level nesting of executeSQL callbacks

You are not going to believe: if this is set to false I get 2 test
failures but if this is set to true I am not getting any test
failures. I am investigating this more carefully, maybe we can just
take this option out and always do the faster batching.

[brodycj]

@marcucio it is my pleasure. As it turns out, it was a matter of a longer timeout when waiting for callbacks during a few batch tests, with or without the optimization for large batches. My idea is to include your changes sometime later today, with your large batch optimization enabled by default and maybe small cleanups, so that people can turn this off if they will encounter problems in the future.

[marcucio]

for the curser field type errors those constants were first introduced with api SDK 11 (Android 3.0.x), we can always just define the constants ourselves. This block of code is the difference between just returning strings or returning the actual types. I ran into this bug because in my code I was expecting an Integer and got back a String.

BTW, I had to do a 'clean' on the project to not get the cursor error.

[brodycj]

On Tue, Apr 10, 2012 at 3:10 PM, Mike
[email protected]
wrote:

for the curser field type errors those constants were first introduced with api SDK 11 (Android 3.0.x), we can always just define the constants ourselves.

Mike, can you define these constants? I would like to target the earlier SDK.

BTW they do not want to accept our changes so I will continue based on
your project, leaving the CoffeeScript version behind.

Chris

[marcucio]

it seems like the getType function is in the newer sdk too, I made that part conditional depending on the api version.

I don't know it there is another way to get the field type in the earlier SDK so we just have to return strings.

marcucio/Cordova-Plugins@9c0f123

In my project I build with the latest sdk but have in the manifest so that it also works on lower sdk devices

[brodycj]

Don't forget we we are in Java-land, no Objective-C conditional
compiling, so I the code still will not compile on an older SDK. I
think we can say something like "if you have an old SDK you have to
change this block of lines" in the README.

In general, I do want to take your changes into my
Cordova-SQLitePlugin fork, maybe sometime tonight.

Chris

On Wed, Apr 11, 2012 at 4:01 AM, Mike
[email protected]
wrote:

it seems like the getType function is in the newer sdk too, I made that part conditional depending on the api version.

I don't know it there is another way to get the field type in the earlier SDK so we just have to return strings.

marcucio/Cordova-Plugins@9c0f123

In my project I build with the latest sdk but have in the manifest so that it also works on lower sdk devices

---

Reply to this email directly or view it on GitHub:
#24 (comment)