Enhancements for server side use

[bjouhier]

Hi Zef,

These are the changes I described in my post 2 days ago. I did a bit more cleanup and testing.

I did not introduce a Persistence class because this would have meant changing a lot more code. I just introduced a createPersistence() function. I think that this is sufficient and that a class would be confusing because you already provide an alternative to subclassing (the config functions).

For commit/rollback, I had originally modified persistence.flush to execute a COMMIT at the end but this did not work because flush is used internally by persistence, for example to update the db before any select statement. So, I had to introduce a separate persistence.commit call which does a flush and then sends the COMMIT. All this is controlled by a flag called explicitCommit that you can set on the transaction object. So, the usage is the following:

If you want to work in autocommit mode, you use the normal persistence API. My changes should be fully backward compatible.

If you want to work in explicit commit mode, you should set transaction.explicitCommit = true on your transactions and you should call session.commit(tx, callback) instead of session.flush(tx, callback) to complete the transaction.

I have put the rollback in the exception handling block of transaction(conn). I am not too sure about this because there are probably cases where you would not want to rollback systematically as soon as an error occurs. Another way to do it would be to introduce a persistence.rollback function and to give the caller the freedom to call it or not in his error handler.

I have also added the following line to the error handling block in transaction(conn): that.errorHandler && that.errorHandler(err); This allows me to associate an error handler with the transaction. I did not see how I could catch the errors otherwise because executeSql is almost always called without the errorFn parameter. Maybe I missed a trick here and my errorHandler might not be the best way to trap the errors (a listener?)

The type mapping stuff is not very complicated but it leads to lots of one liner changes. The default type mapper is in persistence.store.sql. My override enables BINARY(16) on id columns. It looks like the following:

         var tm = persistence.typeMapper = {
            idType: "BINARY(16)",
            columnType: function(type){
                return type;
            },
            inVar: function(str, type){
                return type.indexOf("BINARY(16)") == 0 ? "hex(" + str + ")" : str;
            },
            inId: function(str){
                var tm = persistence.typeMapper;
                return tm.inVar(str, tm.idType);
            },
            outVar: function(str, type){
                return type.indexOf("BINARY(16)") == 0 ? "unhex(" + str + ")" : str;
            },
            outIdVar: function(str) {
                return "unhex(" + str + ")";
            },
            outId: function(id){
                return "unhex('" + id.replace(/-/g, '') + "')"
            },
            entityIdToDbId: function(id){
                var tm = persistence.typeMapper;
                return tm.entityValToDbVal(id, tm.idType);
            },
            newUuid: helpers.uuid.generate, // generates UUIDs with hyphens
            entityValToDbVal: function(val, type) { ... }, // I can send you the details
            dbValToEntityVal: function(val, type) { ... } // I can send you the details
          };

With the typeMapper, the values stored in meta.fields become completely opaque to the persistence object, they are always passed to typeMapper methods. At this stage I am still using simple strings as types but my intent is to change this and use a small type structure instead, something like { sqlName: "TINYINT", nullable: false, ... }. I am that far yet but I should be able to do it without further changes to persistencejs itself.

Also, I get an error when I run the persistencejs unit test. Test #9 (Entity manipulation module: many to many) fails because table Tag_tasks_Task does not exist. This error does not come from my changes because I got it before applying my changes. Do you reproduce it?

Bruno.

[zefhemel]

Hi Bruno,

Thanks a lot! I merged in your changes, with some minor stylistic (indentation etc.) code changes.

I'm thinking about persistence.commit and rolling back. How about the idea to add a commit and rollback function to the transaction object instead of the persistence/session object? That seems a little more useful to me. And indeed, persistence.js is still a bit lacking when it comes to error handling. It would, however, be probably a good thing to be able to rollback a transaction manually (tx.rollback(callback)?).

I'm also interested in your typeMapper implementation, using BINARY(16) for storing IDs. It will be more difficult to read, but it may save quite some storage (and memory) space. It would be interesting to offer it as an option, I suppose.

Thanks for the great work!

[bjouhier]

Hi Zef,

Yes, I have different editor settings (Aptana with default settings). I'll try to do a better job next time.

I agree that commit and rollback would make more sense in the transaction object. And manual rollback gets my vote too. Also, the rollback should clear the session cache. I did not do it because in my case the session always gets released after the rollback but this might not always be the case, especially with a manual rollback. Do you want to do the change, or do you want me to do it?

Regarding exception handling, I find it painful to have to check for errors in every callback. This is why I associated the error handler with the transaction. But I'm not sure that I have set things up the right way and that my error handler will catch all errors. Also, the API that I am proposing (transaction.errorHandler) is probably very naive (I'm new to node). Emitting an error event would probably be much better.

I'll add the alternate typeMapper for binary ids. Also, I'd like to move the dbValToEntityVal and entityValToDbVal functions inside the typeMapper object (I kept them where they were because I wanted to disturb the code as little as possible).

I also need to do another pass on the typeMapper API (some calls are a bit redundant) and document it.

Bruno

[zefhemel]

Hi Bruno,

Don't worry about the code lay-out, that's easy enough to fix. If you'd like to make the changes to the commit/rollback code, that's be great, I'm currently more focussed on the browser use -- so it's great that people like you are also using it on the server and improving that part. Let me know of any progress.

Thanks for the help!

Zef