Few notes on BaseORMService

1) The deleteByID() function is throwing an error for me. tx is
undefined. It's coming from the convertIDValueToJavaType() function,
line 895

2) When passing in an array of ids into the getAll() function, the
BaseORMService is doing an entityLoadByPK for each id in the array.
This strikes me being very inefficient since each entityLoadByPK call
will run a SQL query if the entity is not in the current session. Was
there a reason for doing it this way over using a single query?
Something similar to what deleteById() does.

3) I had to overload the get() function cause it was causing me
frustration. The problem is that it always returns an entity. Most of
the time I don't want a new entity. I could just call entityLoadByPK
and get the behavior I want but I'd rather keep all my ORM interaction
going through the service. Maybe we can add a load() function that
does this?

Also the test for the presence of the id is tickling my OCD bone...
The whole ID EQ 0 check..... I have compound keys and I never get a
new object back when I would expect it. Maybe we can change the
function to:

any function get(required string entityName,required any id) {

  var entity = entityLoadByPK(arguments.entityName, arguments.id);

  if (isNull( entity ) ) {
    return new(arguments.entityName);
  } else {
    return entity;
  }

}

Sure this will always execute a query whereas the current one doesn't
when there isn't a "valid" id, but I fell strong enough that testing
the presence of an id value should not be implied by the service.

If the change would be too much impact, just adding this load()
function would make me happy:

any function load (required string entityName, required any id,
boolean returnNew=false) {

  var entity = entityLoadByPK(arguments.entityName, arguments.id);

  if (isNull( entity ) AND arguments.returnNew ) {
    return new(arguments.entityName);
  } else if (!isNull(entity)){
    return entity;
  }

}

thanks,

.brett

I also noticed that the save method still has this problem in it.

    var tx = ORMGetSession().beginTransaction();
    try{
      entitySave(arguments.entity, arguments.forceInsert);

      tx.commit();
    }
    catch(Any e){
      tx.rollback();
      throw(e);
    }

That code is actually redundant, and causes session closures in my
application. It only needs to be

entitySave(arguments.entity, arguments.forceInsert);

Regards,
Andrew Scott
http://www.andyscott.id.au/

From: coldbox@googlegroups.com [mailto:coldbox@googlegroups.com] On
Behalf Of Brett
Sent: Thursday, 20 January 2011 11:24 AM
To: ColdBox Platform
Subject: [coldbox:7755] Few notes on BaseORMService

1) The deleteByID() function is throwing an error for me. tx is
undefined. It's coming from the convertIDValueToJavaType() function,
line 895

2) When passing in an array of ids into the getAll() function, the
BaseORMService is doing an entityLoadByPK for each id in the array.
This strikes me being very inefficient since each entityLoadByPK call will

run a

SQL query if the entity is not in the current session. Was there a reason

for

doing it this way over using a single query?
Something similar to what deleteById() does.

3) I had to overload the get() function cause it was causing me

frustration.

The problem is that it always returns an entity. Most of the time I don't

want

a new entity. I could just call entityLoadByPK and get the behavior I

want but

I'd rather keep all my ORM interaction going through the service. Maybe

we

can add a load() function that does this?

Also the test for the presence of the id is tickling my OCD bone...
The whole ID EQ 0 check..... I have compound keys and I never get a new
object back when I would expect it. Maybe we can change the function to:

any function get(required string entityName,required any id) {

  var entity = entityLoadByPK(arguments.entityName, arguments.id);

  if (isNull( entity ) ) {
    return new(arguments.entityName);
  } else {
    return entity;
  }

}

Sure this will always execute a query whereas the current one doesn't when
there isn't a "valid" id, but I fell strong enough that testing the

presence of an

Hey Andrew,

I haven't run into that with the save() method, and I have some pretty
convoluted models. Most often for me I get a session close if I am
saving an entity with an invalid property value (a required value with
a NULL value or an invalid property value). All of my save routines
have a validate call so that is super important. If nothing else...
just to just get around the super generic session closed error...

thinking... CB 3.1 should also include a generic baseEntity.cfc that
all persistent models inherit from. I'm a big fan of Fowler's fat
model, and I have a base cfc that is taking on a lot of useful
functionality. For example, I use a generic get() function on all
entities that takes a property path (dot notation) and returns a
default value if it can not be resolved:

resource.get(path="publisher.organization.name", default="")

very useful for forms when working with new vs persisted entities. As
soon as the get() function can't resolve the next path property it
returns the default.

another good one is
resource.escape(path="publisher.organization.name", default="")

to escape the property value and safe guard against XSS.

.brett

Brett,

1) can you send me the stack trace and what type you ID is and I can see if I can fix that up.
2) Good idea. I will work on this.
3) Anyone else have opinions on that. I was never in love with the fact that if sent in 0 it returned new, but that is also handy sometimes. Let us know your thoughts.

Curt

This was done to make the save transactional. You can set flushatrequestend to false and control the session(hibernate session) yourself.

Curt

Trust me, I have a conversion utility (see a previous thread on that in
here) where I am converting data from an older application over to a new
format. This is fairly involved work, but it does about 6 tables and uses
ORM in some places to setup defaults, then uses a standard query to then
convert the data into a struct which is then populated into an Entity.

At some point without these changes this code WILL never run, and always
throw a session is closed error. As soon as I removed this code in the save
method (Which really does nothing because you are only wrapping a
transaction around one DB call and not multiple as it is intended), I am
able to run this conversion utility with no problems.

So the question remains why was a transaction put in place like that, it
makes no sense at all. Especially when you are opening and saving upto 4-5
Entities inside a loop. Because on the very first save, goodbye session.

Regards,
Andrew Scott
http://www.andyscott.id.au/

From: coldbox@googlegroups.com [mailto:coldbox@googlegroups.com] On
Behalf Of Brett
Sent: Thursday, 20 January 2011 2:06 PM
To: ColdBox Platform
Subject: [coldbox:7762] Re: Few notes on BaseORMService

Hey Andrew,

I haven't run into that with the save() method, and I have some pretty
convoluted models. Most often for me I get a session close if I am saving

an

entity with an invalid property value (a required value with a NULL value

or

an invalid property value). All of my save routines have a validate call

so that

is super important. If nothing else...
just to just get around the super generic session closed error...

thinking... CB 3.1 should also include a generic baseEntity.cfc that all
persistent models inherit from. I'm a big fan of Fowler's fat model, and

I

have a base cfc that is taking on a lot of useful functionality. For

example, I

use a generic get() function on all entities that takes a property path

(dot

notation) and returns a default value if it can not be resolved:

resource.get(path="publisher.organization.name", default="")

very useful for forms when working with new vs persisted entities. As

soon

Trust me

I trust you. The baseORMService shouldn't imply transactions on us,
especially if it is saving only one entity. I could see it on a
saveAll([arrayOfEntities]).

Hey Curt,

1) I don't have a running computer in front of me, but look at the
convertIDValueToJavaType() function in baseORMService.cfc and you'll
see the tx variable isn't scoped in any way:

https://github.com/ColdBox/coldbox-platform/blob/master/system/orm/hibernate/BaseORMService.cfc

2) If that make sense and I can contribute a patch let me know. I
spent some time logging what the ORM was doing and seeing how it was
functioning. The ORM service taught me a lot! but I think it needs
some rework.

3) Not a fan of "0" at all... I'd rather default the id to an empty
string and evaluate it as not having an id. That is what CF taught me
anyway...

Thanks for replying!

.brett

I have to agree with the default to 0 for an ID, especially since some of my
tables are not integer ID's but string.

Regards,
Andrew Scott
http://www.andyscott.id.au/

From: coldbox@googlegroups.com [mailto:coldbox@googlegroups.com] On
Behalf Of Brett
Sent: Thursday, 20 January 2011 3:09 PM
To: ColdBox Platform
Subject: [coldbox:7773] Re: Few notes on BaseORMService

Hey Curt,

1) I don't have a running computer in front of me, but look at the
convertIDValueToJavaType() function in baseORMService.cfc and you'll see
the tx variable isn't scoped in any way:

https://github.com/ColdBox/coldbox-
platform/blob/master/system/orm/hibernate/BaseORMService.cfc

2) If that make sense and I can contribute a patch let me know. I spent

some

time logging what the ORM was doing and seeing how it was functioning.
The ORM service taught me a lot! but I think it needs some rework.

3) Not a fan of "0" at all... I'd rather default the id to an empty

string and

yep. I'm all guid here....

sorry, this is much better for load():

any function load (required string entityName, required any id,
boolean returnNew=false) {
        var entity = entityLoadByPK(arguments.entityName,
arguments.id);
        if (!isNull( entity ) {
                return entity;
        } else if ( arguments.returnNew ) ){
                return new(arguments.entityName);
        }
}

1) deleteByID
This has been fixed

2) getAll()
I just updated it to use DLM style executions for listing the queries
out. This was on the todo and I never saw it, but now it has been
updated.

3) get() problem with ID
I totally see the point and I reworked it where we do the entityload
first
and if not null, we return. If null, then we check if the id=0 or
empty and if it is
then we return a new entity by convenience. This should only impact
users who send in 0 or '' as the simple id.

4) The save method was done to provide a transaction safe save method.
However, I do see the point in it.

I created a new method called saveAll() which takes in an array of
entities and tries
to persist them in order in an orderly fashion. I think this makes
much more sense
for creating a transaction approach, where you create a buffer of
entities to be saved
sequentially and then send the batch for saving.

As for the save() method I added an argument called: transactional
which defaults to true.
If you do not want it transactional, just
save(entity,transactional=false);

That's it.

By the way, great feedback guys. I think we are really improving the
ORM services and making ORM integration much nicer and cleaner and
much more powerful!!

I am also very interested in a BaseEntity class but maybe for 3.1

I disagree with transactional for one entity, am I really the only person
who sees that.

Regards,
Andrew Scott
http://www.andyscott.id.au/

From: coldbox@googlegroups.com [mailto:coldbox@googlegroups.com] On
Behalf Of ColdBox Manager
Sent: Thursday, 20 January 2011 5:16 PM
To: ColdBox Platform
Subject: [coldbox:7784] Re: Few notes on BaseORMService

1) deleteByID
This has been fixed

2) getAll()
I just updated it to use DLM style executions for listing the queries out.

This

was on the todo and I never saw it, but now it has been updated.

3) get() problem with ID
I totally see the point and I reworked it where we do the entityload first

and

if not null, we return. If null, then we check if the id=0 or empty and

if it is

then we return a new entity by convenience. This should only impact users
who send in 0 or '' as the simple id.

4) The save method was done to provide a transaction safe save method.
However, I do see the point in it.

I created a new method called saveAll() which takes in an array of

entities

and tries to persist them in order in an orderly fashion. I think this

makes

much more sense for creating a transaction approach, where you create a
buffer of entities to be saved sequentially and then send the batch for
saving.

As for the save() method I added an argument called: transactional which
defaults to true.
If you do not want it transactional, just

save(entity,transactional=false);

That's it.

> sorry, this is much better for load():
>
> any function load (required string entityName, required any id,
> boolean returnNew=false) {
> var entity = entityLoadByPK(arguments.entityName,
> arguments.id);
> if (!isNull( entity ) {
> return entity;
> } else if ( arguments.returnNew ) ){
> return new(arguments.entityName);
> }
>
> }
>
>
>
>
>
>
>
>
> > yep. I'm all guid here....
>
>
> > > I have to agree with the default to 0 for an ID, especially since
> > > some of my tables are not integer ID's but string.
>
> > > Regards,
> > > Andrew Scotthttp://www.andyscott.id.au/
>
> > > > From: coldbox@googlegroups.com
[mailto:coldbox@googlegroups.com]
> > > > On Behalf Of Brett
> > > > Sent: Thursday, 20 January 2011 3:09 PM
> > > > To: ColdBox Platform
> > > > Subject: [coldbox:7773] Re: Few notes on BaseORMService
>
> > > > Hey Curt,
>
> > > > 1) I don't have a running computer in front of me, but look at
> > > > the
> > > > convertIDValueToJavaType() function in baseORMService.cfc and
> > > > you'll see the tx variable isn't scoped in any way:
>
> > > >https://github.com/ColdBox/coldbox-
> > > > platform/blob/master/system/orm/hibernate/BaseORMService.cfc
>
> > > > 2) If that make sense and I can contribute a patch let me know.
> > > > I spent
> > > some
> > > > time logging what the ORM was doing and seeing how it was
functioning.
> > > > The ORM service taught me a lot! but I think it needs some

rework.

>
> > > > 3) Not a fan of "0" at all... I'd rather default the id to an
> > > > empty
> > > string and
> > > > evaluate it as not having an id. That is what CF taught me

anyway...

If you don’t want to use the transaction then use transactional=false or override the method so it always uses non transactions. Why you need transactions is because you can turn the orm flussion and session close OFF and you need a transaction in order for the committ to be flushed to the database, if not, it will never be sent to the database.

Again, options options options.

No, you are not. I am in 100% agreement with you, as are many others (who may or may not be ColdBox users).

again, read my post, if you do NOT have a transactional commit when you auto manage your hibernate session, objects will NOT be persisted back to the db.

Wait. Damn. I hate when I do that. I misread something. Or my memory failed me. Hell, I don’t know. Can I go back to sleep yet?

LOL, I am, baby is awake again, dang!! Ok, laterz, baby duty is ON!!

Why the hell do I keep hitting [send] without actually writing the whole message?

Anyways, unless you are allowing CF to manage all of your Hibernate sessions for you, you want to use a transaction. So, yeah, while it seems on the surface like a waste, it’s a good thing.

Oh, and by the way, for whatever it’s worth, if you are allowing CF to manage all of your Hibernate sessions for you, you are virtually guaranteed to have issues in your application. The following two settings are your friend:

this.ormSettings = {
flushatrequestend = false,
automanageSession = false
};