Feedback on current O/R Mapping Unittests

[lastcraft]

Hi...

Clearly I'm missing something, but what's the problem with having the SQL that the test depends on being in the test itself?

Nothing. In fact it's often a good thing, as it allows the test to tell the full story.

yours, Marcus

[lastcraft]

Hi...

I also looked at marcus changes-lib last night and realized he had a UoW(I think?) as the main class(which he calls Changes ?). The UoW mainly acts as a registry of changed objects and a holder for the mappers connected to this UoW.

That's correct. The generated classes are DAO's and I find the biggest weakness of ActiveRecord/DAO type libraries is the need to call save() all over the place and suddenly having no specific point to commit a transaction. This is something that Rails doesn't seem to solve satisfactorily either. The UnitOfWork solves these problems and more.

Anyway, looks like you have surpassed Changes some time ago .

yours, Marcus

[thr]

Nothing. In fact it's often a good thing, as it allows the test to tell the full story.

Yes, I couldn't see the reason as to why they were bashing(ok maybee it wasn't that bad ;p) me at the start for putting the SQL in the tests, ah well.

That's correct. The generated classes are DAO's and I find the biggest weakness of ActiveRecord/DAO type libraries is the need to call save() all over the place and suddenly having no specific point to commit a transaction. This is something that Rails doesn't seem to solve satisfactorily either. The UnitOfWork solves these problems and more.

Yes, I tend to agree about this and well, I don't like AR/DAOs as they polute the Domain Objects. The only thing your Domain Objects should care about (imho) thier own data and what they can do with that - not how to persist it or how to find themselves, etc. That's the main reason I've worked pretty hard towards the goal that to be able to persist your domainobject all you have to do is to implement the DomainObject interface, which is just a symbolic interface as it has no methods defined.

Anyway, looks like you have surpassed Changes some time ago :).

yours, Marcus

Ah well, thank you :) I've read more of changes code and tbh. I gotta say that it's realy realy clever ;).

It is perfectly reasonable to build a more generalized layer on top of the ORM layer, so I tend to agree: validation issues should be outside of the ORM implementation. Whether the application factorizes validation or not, it is still up to the app to issue/call validation from within the normal insert/update/delete hooks.

Totaly correct imho and that's why I'm only going to implement "onDelete", "onUpdate" and "onInsert" which will be called after a valid insert, delete, update.

The main danger I see with pre-methods is that when they're executed you assume that the query will go thru and thus make changes to other DomainObjects / The database - but what happends if the query fail and you've already made changes in say beforeInsert(); to other DomainObjects?

[thr]

How does this look for a standard mapper interface:

<?php
interface Mapper{

    const HAS_MANY = 0x0;
    const HAS_ONE = 0x1;
    const BELONGS_TO = 0x2;
    
    const CFG_TYPE = 0x1;
    const CFG_TABLE = 0x2;
    const CFG_ID = 0x4;
    const CFG_KEYS = 0x8;
    const CFG_FIELDS = 0x10;
    const CFG_RELATIONS = 0x20;
    const CFG_VALUEOBJECTS = 0x40;
    const CFG_ALL = 0x7F;
    
    public function getConfigurationDirective($directive = Mapper::CFG_ALL);
    public function generateArrayKey(Array $result);
    public function generateObjectKey(DomainObject $object);
    public function findBySql($sql, Array $values = array(), $recursion = null);
    public function findByStmt(PDOStatement $stmt);
    public function update(DomainObject $object);
    public function delete(DomainObject $object);
    public function insert(DomainObject $object);
    public function create();
    
}
?>

[thr]

Ok, so I was thinking about performance and how to improve it earlier today (yes I spend alot of time on this ORM code but I enjoy it.), this idea popped into my head:

Often when you do a query that spans over multiple objects(such as select * from person where age > 20), many of those objects are already in memory and the data isn't realy required to be fetched from the db. Just executing this query and then compare the result vs. what you got inmemory and figure out what objects to build and what objects already exists is the "normal" way. I was thinking about trying to do some optimization.

Take the above query select * from person where age > 20 and turn it into select id from person where age > 20, now compare the ids you get back to the objects you already got in memory. Remove all the ids that already have object representing them and only fetch the ids without objects with a query such as: select * from person where id = 1 or id = 2 to fetch object 1 and 2. Would this improve performance? I can see that memory wise it would improve performance if you're fetching very large rows from the db. And even if it doesn't effect performance that much is it worth it anyways due to not fetching "old" data again?

[Ren]

Not sure its worth it.

If eventually write a OQL parser, and evaluator to run against the in-mem objects you could feasibly write

SELECT * Person WHERE age > 20 AND Id NOT IN (1, 2)

Where 1 & 2 are in-mem persons that statisfy the condition(s), but I'm guessing probably more expensive in the vast majority of cases. Edit: Plus composite pks get messy.

Back to optimistic locking, if your using a config file, why not have a section for columns that need to be compared when doing U&D operations. Would be more independant I think, MySQL allows tables with multiple timestamps, and only the first gets updated automatically, MSSQL's timestamp is non-standard and have a rowversion datatype, Sqlite doesn't have a timestamp datatype.

[kyberfabrikken]

Take the above query select * from person where age > 20 and turn it into select id from person where age > 20, now compare the ids you get back to the objects you already got in memory. Remove all the ids that already have object representing them and only fetch the ids without objects with a query such as: select * from person where id = 1 or id = 2 to fetch object 1 and 2. Would this improve performance? I can see that memory wise it would improve performance if you're fetching very large rows from the db. And even if it doesn't effect performance that much is it worth it anyways due to not fetching "old" data again?

I don't think it's worth it, but it probably depends on the context. As a general rule of thumb, you'd want to minimize the number of calls to an external service, rather than minimize amount of data transferred. The reason is that establishing the connection takes a lot of time. Unless you have really many records in memory already, I think the extra call by far outweights the saved bandwidth. Since rdbms's mostly run at localhost anyway, bandwidth is probably not your biggest issue at that point. (Rather the memory-footprint of thoose thousands of in-memory objects should concern you)

[thr]

I don't think it's worth it, but it probably depends on the context. As a general rule of thumb, you'd want to minimize the number of calls to an external service, rather than minimize amount of data transferred. The reason is that establishing the connection takes a lot of time. Unless you have really many records in memory already, I think the extra call by far outweights the saved bandwidth. Since rdbms's mostly run at localhost anyway, bandwidth is probably not your biggest issue at that point. (Rather the memory-footprint of thoose thousands of in-memory objects should concern you)

Ah yes, both you and Ren got good points - ah well it was just an idea that popped into my head. It might be worth it if you're dealing with BLOB values tho? for example images, etc.?

Back to optimistic locking, if your using a config file, why not have a section for columns that need to be compared when doing U&D operations. Would be more independant I think, MySQL allows tables with multiple timestamps, and only the first gets updated automatically, MSSQL's timestamp is non-standard and have a rowversion datatype, Sqlite doesn't have a timestamp datatype.

About the optimistic locking I've decided to go for a versioning or timestamp field depending on what your DB supports. I think that the "compare all fields"-idea might be to system resource heavy when updating multiple objects.

[Ren]

Ah yes, both you and Ren got good points - ah well it was just an idea that popped into my head. It might be worth it if you're dealing with BLOB values tho? for example images, etc.?

If dealing with BLOB types, then you'd be lazy loading fields. Once start on that path, you can lazy load all fields (except the PK of course) So can just use the query

SELECT Id FROM Persons WHERE Age > 20

And create Person objects directly from the result, and the rest of the Person fields get lazy loaded, guess the trick maybe to merge SELECT * FROM Persons WHERE Id = 1 into SELECT * Persons WHERE Id IN (...) when performing lazy loading, so can reduce the number of SQL calls.

[33degrees]

Ok some realy good points there again, and I agree that the onSave (as I said above) feels way to generic. And about validation? Shouldn't data validation be put somewhere else except in the ORM layer? Seems odd to have it there.

Well, for me having onSave makes things a bit simpler; in my eyes, the alternatives are duplicating code in both onInsert and onUpdate, or having them call a common function, which is exactly what onSave is.

As for Data validation, it goes in the model layer, which just happens to be the same as the orm layer for me because i'm using active record. The advantage of putting it there is that there is no way to persist data that doesn't meet the validation rules, which could happen if validation wasn't being performed in the save call.

[thr]

Edit: Rephrased + added some more text.

Well, for me having onSave makes things a bit simpler; in my eyes, the alternatives are duplicating code in both onInsert and onUpdate, or having them call a common function, which is exactly what onSave is.

Ok, suggesting that I'd put both onSave and onUpdate/Insert in, which one would go first if both are present? onSave or onUpdate/Insert

As for Data validation, it goes in the model layer, which just happens to be the same as the orm layer for me because i'm using active record. The advantage of putting it there is that there is no way to persist data that doesn't meet the validation rules, which could happen if validation wasn't being performed in the save call.

Imho, data validation should go in the Domain Object itself, if your using AR that's perfectly fine but with Mappers imho it's not.

[33degrees]

Take the above query select * from person where age > 20 and turn it into select id from person where age > 20, now compare the ids you get back to the objects you already got in memory. Remove all the ids that already have object representing them and only fetch the ids without objects with a query such as: select * from person where id = 1 or id = 2 to fetch object 1 and 2. Would this improve performance?

Actually, it may very well end up slowing down performance, because although you're transfering less data, you're connecting to the db twice, and that may end up being worse.

Really, the only way to optimise sensibly is to profile your classes and identify the bottlenecks; don't bother implementing something like this unless you're absolutely sure it's worth it.

And even if it doesn't effect performance that much is it worth it anyways due to not fetching "old" data again?

Nothing wrong with fetching the data, but it would be wise to check if you have an in-memory object and return that instead; are you using an Identity Map?

[33degrees]

Edit: Rephrased + added some more text.

Ok, suggesting that I'd put both onSave and onUpdate/Insert in, which one would go first if both are present? onSave or onUpdate/Insert

I would put onSave first, I think it's more intuitive there.

Imho, data validation should go in the Domain Object itself, if your using AR that's perfectly fine but with Mappers imho it's not.

Is your mapper making sure the domain object is valid before persisting it? That is the most important thing, IMO.

[thr]

Nothing wrong with fetching the data, but it would be wise to check if you have an in-memory object and return that instead; are you using an Identity Map?

Yes ofc. I'm using an ID Map ;P

Is your mapper making sure the domain object is valid before persisting it? That is the most important thing, IMO.

Exactly what do you mean by valid?

[thr]

(to mention something else) Found a rather annoying thing in PHP today, namely this:

<?php
$names = array("fredrik","madeleine");
foreach($names as $name){
    echo $name . "<br />";
    if($name == 'fredrik')
        $names[]  = 'thr';
}
?>

will print:

fredrik<br />madeleine<br />

I was trying to add objects to the delete que while being inside the delete loop(ie doing cascading deletes)


Edit: Ok, found a workaround (rather annoying tho):

<?php
class ObjectList implements Iterator{

    protected $objects = array();
    protected $pos;

    public function next(){
        $this->pos++;
        next($this->objects);
    }
    
    public function valid(){
        return ($this->pos < count($this->objects));
    }
    
    public function key(){
        return key($this->objects);
    }
    
    public function current(){
        return current($this->objects);
    }
    
    public function rewind(){
        $this->pos = 0;
        reset($this->objects);
    }
    
    public function insert($object){
        $this->objects[strval($object)] = $object;
    }
    
    public function clear(){
        $this->objects = array();
    }
    

}

$o = new ObjectList;
$o->insert("hej");
$o->insert(" ");
$o->insert("älskling");
foreach($o as $v){
    var_dump($v);
    if($v == " ")
        $o->insert(" <3 ");
}
?>

[thr]

Sigh, three posts in a row no, ah well - another thing on my mind is this:

If a transaction fail, should the ORM automaticly rollback the transaction or let the user decide what to do?

[Ren]

(to mention something else) Found a rather annoying thing in PHP today, namely this:

<?php
$names = array("fredrik","madeleine");
foreach($names as $name){
    echo $name . "<br />";
    if($name == 'fredrik')
        $names[]  = 'thr';
}
?>

will print:

fredrik<br />madeleine<br />

I was trying to add objects to the delete que while being inside the delete loop(ie doing cascading deletes)


Edit: Ok, found a workaround (rather annoying tho):

<?php
class ObjectList implements Iterator{

    protected $objects = array();
    protected $pos;

    public function next(){
        $this->pos++;
        next($this->objects);
    }
    
    public function valid(){
        return ($this->pos < count($this->objects));
    }
    
    public function key(){
        return key($this->objects);
    }
    
    public function current(){
        return current($this->objects);
    }
    
    public function rewind(){
        $this->pos = 0;
        reset($this->objects);
    }
    
    public function insert($object){
        $this->objects[strval($object)] = $object;
    }
    
    public function clear(){
        $this->objects = array();
    }
    

}

$o = new ObjectList;
$o->insert("hej");
$o->insert(" ");
$o->insert("älskling");
foreach($o as $v){
    var_dump($v);
    if($v == " ")
        $o->insert(" <3 ");
}
?>

$names = new ArrayObject(array("fredrik","madeleine"));
foreach($names as $name)
{ 
    echo $name . "<br />"; 
    if($name == 'fredrik') 
        $names[]  = 'thr'; 
} 

Also there is an AppendIterator interface.
Edit scratch that, it appends another iterator rather than value.

[thr]

haha, how could i miss that ;p thanks ren.

[Dr Livingston]

If a transaction fail, should the ORM automaticly rollback the transaction or let the user decide what to do?

I would leave the transaction to rollback gracefully... Why leave it to a user to muck it up even more, if something goes wrong? But that's not to say that there are, or will be those occasions when intervention is required, it's just not a common event

[33degrees]

Exactly what do you mean by valid?

I mean that it has passed the validation tests; I think it's important to make sure your domain object has been validated before you can save it, which is why I hook in the validation calls into the on_save, on_insert and on_update calls, as appropriate. If you're using a data mapper, it would then be the mapper's responsibility to make sure the domain object has been validated before saving. In either case, the ORM layer has to get involved in validation to some degree.

[thr]

I mean that it has passed the validation tests; I think it's important to make sure your domain object has been validated before you can save it, which is why I hook in the validation calls into the on_save, on_insert and on_update calls, as appropriate. If you're using a data mapper, it would then be the mapper's responsibility to make sure the domain object has been validated before saving. In either case, the ORM layer has to get involved in validation to some degree.

Yes but that depends on how you define "valid". If you mean stuff like "username only containz a-z 0-9 - and _" then no, I don't think that's the Mappers responsibility and it should be put in the Domain Objects imho.

If you mean that it's a valid domain object that got all required fields set(no NULLs, etc.) and doesn't already exists in the db, etc - then yes.

[33degrees]

Yes but that depends on how you define "valid". If you mean stuff like "username only containz a-z 0-9 - and _" then no, I don't think that's the Mappers responsibility and it should be put in the Domain Objects imho.

If you mean that it's a valid domain object that got all required fields set(no NULLs, etc.) and doesn't already exists in the db, etc - then yes.

What I mean is that, although the validation logic would be contained in the model, the mapper should be sure that the validation has passed before persisting the model (i.e. by calling the model's isValid method)

[thr]

Just a quick question to all of you: Which type of inheritance mapping would you prefer?
1. Single Table Inhertiance (PoEAA p.278) (One table for all classes)
2. Class Table Inheritance (PoEAA p.285) (Different tables for all classes with only the "extra" fields that are added in a subclass in each sub-class table)
3. Concrete Table Inheritance (PoEAA p.293) (Different tables for all classes with all fields for one class in that table)

Personally Single(1) or Concrete(3) seems the easiest to implement and also the fastest ones in terms of mysql queries. Class(2) seems like the "purest" choice but also the hardest to implement. 1 Has the drawback that the table you're using easily could get cluttered with all types of fields for different subclasses. 2. Has the drawback that it requires quite an amount of queries/subqueries if you get deep inheritance, 3. Has the drawback that one change in the top class requires a table schema change in every other also.

So which one is the lesser of three evils?

What I mean is that, although the validation logic would be contained in the model, the mapper should be sure that the validation has passed before persisting the model (i.e. by calling the model's isValid method)

Now I'm with ya, but I don't think I agree on it - from my PoV an object shouldn't even get into the mapper if it's not valid. But I see your point and I'll look into post/pre hooks.

[thr]

I ran into another problem with Optimistic Offlinelocking and php today, more specific this: It's not possible to use "standard" offline locking with php as the client doesn't hold the state of an object. That means that if you do the "normal" route with just one version field and you "lock()" an object, on the next page/application run that object will be fetched again and the version field will be updated to the latest version meaning that the "lock" will be valid at all times.

Now there's two workarounds for this(as far as I can see):

1. Save the object you're updating in the current session(you store the object in $_SESSION['locked'] and you can fetch your locked objects thru the LockingManager for example)

2. Use a lockingtable for Optimistic Offlinelocking also(where you store the key for the object, the type for the object, who locked it and what version he had when he locked it)

Or am I missing something crucial and just confusing myself?

[Ren]

I ran into another problem with Optimistic Offlinelocking and php today, more specific this: It's not possible to use "standard" offline locking with php as the client doesn't hold the state of an object. That means that if you do the "normal" route with just one version field and you "lock()" an object, on the next page/application run that object will be fetched again and the version field will be updated to the latest version meaning that the "lock" will be valid at all times.

Now there's two workarounds for this(as far as I can see):

1. Save the object you're updating in the current session(you store the object in $_SESSION['locked'] and you can fetch your locked objects thru the LockingManager for example)

2. Use a lockingtable for Optimistic Offlinelocking also(where you store the key for the object, the type for the object, who locked it and what version he had when he locked it)

Or am I missing something crucial and just confusing myself?

Simplest case is the rowversion becomes a hidden field in the form (along with the PK), if editting a row, and thus beyond the scope of the ORM.