Feedback on current O/R Mapping Unittests

[thr]

First of all, Marcus - thanks once more for SimpleTest .

Second, here are my current testcases for my ORM-implementation:
Notice: In the testcases I use the syntax:
$this->worker->mapper("Type")->findByPk(1);
But it's equally good/shorter to use:
$this->worker->findTypeByPk(1);
It's just that I wrote the testcases before I added most of the syntacitals suger thingies.

<?php
require('Dev/PDOBench.php');
require('Dev/DevFuncs.php');
     ra('../ORM/');
require('simpletest/unit_tester.php');
require('simpletest/reporter.php');
require('simpletest/reporter_showpasses.php');


// Host, Username, Password and Database for tests:
define('DBHOST','localhost');
define('DBUSER','dev');
define('DBPASS','devnull');
define('DBNAME','development');


/* NOTICE: These tests will drop/create the tables named 
   "Groups_test","Persons_test","Memeberships_test" and "Posts_test" on the
   database the tests are pointed at - these tests should
   NOT be run on anything else then a database used for
   testing purposes. If you don't know what something of the above
   means - don't run the tests
   
   NOTICE: These test will currently only work on a mysql database.

   To get the tests to run, set define RUN_TESTS to true
*/
define('RUN_TESTS',true);


if(RUN_TESTS === true){

    class TestObjectRelationalMapping extends UnitTestCase{
    
        function TestObjectRelationalMapping(){
            $this->UnitTestCase("O/R Mapping test");
            $this->pdoBench = new PDOBench("mysql:host=".DBHOST.";dbname=".DBNAME,DBUSER,DBPASS);
            $this->worker = new UnitOfWork($this->pdoBench,'./Classes');
        }
        
        function setUp(){
            $this->pdo = new PDO("mysql:host=".DBHOST.";dbname=".DBNAME,DBUSER,DBPASS);
            $groups = "CREATE TABLE `Groups_test` (
              `id` int(11) NOT NULL auto_increment,
              `name` varchar(25) NOT NULL,
              `comment` varchar(255) NOT NULL,
              PRIMARY KEY  (`id`)
            ) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;";
            $this->pdo->exec($groups);
            
            $groupsData = "INSERT INTO `Groups_test` (`id`, `name`, `comment`) 
            VALUES (1, 'Admin', 'Administrators....'), (2, 'Users', 'Normal users...');";
            $this->pdo->exec($groupsData);
            
            $memberships = "CREATE TABLE `Memberships_test` (
              `id` int(11) NOT NULL auto_increment,
              `person` int(11) NOT NULL,
              `group` int(11) NOT NULL,
              PRIMARY KEY  (`id`)
            ) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;";
            $this->pdo->exec($memberships);
            
            $persons = "CREATE TABLE `Persons_test` (
              `id` int(11) NOT NULL auto_increment,
              `name` varchar(50) NOT NULL,
              `age` int(11) NOT NULL,
              `email` varchar(50) NOT NULL,
              PRIMARY KEY  (`id`)
            ) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;";
            $this->pdo->exec($persons);
            
            $personsData = "INSERT INTO `Persons_test` (`id`, `name`, `age`, `email`) VALUES 
            (1, 'Fredrik', 20, 'thrthr@gmail.com'), (2, 'Madeleine', 19, 'unkown');";
            $this->pdo->exec($personsData);
            
            $posts = "CREATE TABLE `Posts_test` (
              `id` int(11) NOT NULL auto_increment,
              `author` int(11) default NULL,
              `text` text NOT NULL,
              `subject` varchar(255) default NULL,
              PRIMARY KEY  (`id`)
            ) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;";
            $this->pdo->exec($posts);
            $this->worker->unregisterAll();
        }
        
        function tearDown(){
            $this->pdo->exec('DROP TABLE `Groups_test`, `Memberships_test`, `Persons_test`, `Posts_test`;');
        }
        
        function testFind(){
            $result = $this->worker->mapper("Person")->find(array('age' => 20));
            $this->assertEqual(1,count($result));
            $this->assertIsA($result[0],'Person');
        }
        
        function testFindByPk(){
            $person = $this->worker->mapper("Person")->findByPk(1);
            $this->assertIsA($person,'Person');
        }
        
        function testFindBySql(){
            $result = $this->worker->mapper("Person")->findBySql("select * from `Persons_test`");
            $this->assertEqual(2,count($result));
            $this->assertIsA($result[0],'Person');
            $this->assertIsA($result[1],'Person');
            $this->assertNotEqual($result[0]->id,$result[1]->id);
        }
        
        function testFindOne(){
            $result = $this->worker->mapper("Person")->findFirst(array('age' => 20));
            $this->assertIsA($result,'Person');
        }
        
        function testCreate(){
            $person = $this->worker->create("Person");
            $this->assertIsA($person,"Person");
            $this->assertNull($person->id);
        }
        
        function testCreationsAreSaved(){
            $person = $this->worker->create("Person");
            $person->name  = "Dummy";
            $person->age   = 1;
            $person->email = "Unkown...";
            $this->assertIsA($person,"Person");
            $this->assertNull($person->id);
            $this->worker->commit();
            $this->assertEqual($person->id,3);
        }
        
        function testJoinedCreationsAreSaved(){
            $person = $this->worker->create("Person");
            $person->name  = "Dummy";
            $person->age   = 2;
            $person->email = "Unkown...";
            $post1 = $person->posts->create();
            $post2 = $person->posts->create();
            $post3 = $person->posts->create();
            $this->assertIsA($post1,"Post");
            $this->assertIsA($post2,"Post");
            $this->assertIsA($post3,"Post");
            $post1->text = "I ";
            $post2->text = "<3 ";
            $post3->text = " you";
            $this->worker->commit();
            $this->assertNotNull($person->id);
            $this->assertNotNull($post1->id);
            $this->assertNotNull($post2->id);
            $this->assertNotNull($post3->id);
        }
        
        function testRegisterDirty(){
            $person = $this->worker->mapper("Person")->findByPk(1);
            $person->name = "thr";
            $this->worker->registerDirty($person);
            $this->worker->commit();
            
            $newWorker = new UnitOfWork($this->pdoBench,'./Classes');
            $personCopy = $newWorker->mapper("Person")->findByPk(1);
            
            $this->assertEqual($person->name,$personCopy->name);
        }
        
        function testRegisterDelete(){
            $person = $this->worker->mapper("Person")->findByPk(1);
            $this->worker->registerDelete($person);
            $this->worker->commit();
            
            $newWorker = new UnitOfWork($this->pdoBench,'./Classes');
            $personCopy = $newWorker->mapper("Person")->findByPk(1);        
            $this->assertNull($personCopy);
        }
        
        function testLinkedCreationsAreSavedToDb(){
            $person = $this->worker->create("Person");
            $person->name  = "Dummy";
            $person->age   = 2;
            $person->email = "Unkown...";
            $post1 = $person->posts->create();
            $post2 = $person->posts->create();
            $post3 = $person->posts->create();
            $post1->text = "I ";
            $post2->text = "<3 ";
            $post3->text = " you";
            $this->worker->commit();
            
            $newWorker = new UnitOfWork($this->pdoBench,'./Classes');
            $person = $newWorker->mapper("Person")->findByPk(3);
    
            foreach($person->posts as $post){
                $this->assertIsA($post,"Post");
            }
        }
        
        function testCollectionCreationsAutomaticlyAssigned(){
            $person = $this->worker->mapper("Person")->findByPk(1);
            $group  = $this->worker->mapper("Group")->findByPk(1);
            
            $membership = $person->memberships->create();
            $membership->group = $group;
            $this->assertEqual($person->id,$membership->person->id);
            $this->worker->Commit();
            
            $newWorker = new UnitOfWork($this->pdoBench,'./Classes');
            $membership = $newWorker->mapper("Membership")->findByPk(1);
            
            $this->assertEqual($person->id,$membership->person->id);
            $this->assertEqual($group->id,$membership->group->id);
        }
        
        function testThresholdAutoLoad(){
            $person = $this->worker->mapper("Person")->findByPk(1);
            
            for($i = 0; $i < 25;$i++){
                $post = $person->posts->create();
                $post->text = $i;
            }
            
            $this->worker->commit();
            
            $newWorker = new UnitOfWork($this->pdoBench,'./Classes');
            $person = $newWorker->mapper("Person")->findByPk(1);
            $person->posts->setThreshold(5);
            foreach($person->posts as $post){
                $this->assertIsA($post,"Post");
            }
        }
        
        // "aliasing" is actually a normal php reference
        function testAliasing(){
            $person = $this->worker->mapper("Person")->findByPk(1);
            $person->mail = "this is a new email";
            $this->worker->registerDirty($person);
            $this->worker->commit();
            
            // Person::$mail is a reference to Person::$email (sql field)
            // That's defined in Person::__construct();
            $this->assertEqual($person->mail,$person->email);
            
            $newWorker = new UnitOfWork($this->pdoBench,'./Classes');
            $person2 = $newWorker->mapper("Person")->findByPk(1);
            $this->assertEqual($person->mail,$person2->email);
        }
        
        function testSyntaticalSugar(){
            $person = $this->worker->findPersonByPk(1);
            $this->assertIsA($person,"Person");
            $personList = $this->worker->findPerson(array('name' => 'Fredrik'));
            $this->assertEqual(1,count($personList));
            $this->worker->deletePersonByPk(1);
        }
        
        function testDisplayPDO(){
            $this->pdoBench->display();
        }
    
    }
    $testORM = new TestObjectRelationalMapping;
    $testORM->run(new ShowPasses());
}
?>

[thr]

Oh, and here's the generated API documentation(phpDocumentor): http://mirkk.nu/Manual

[Dr Livingston]

Sorry for going off topic, but instead of actually creating the database tables via PHP script, wouldn't it be more convienent, and more pratical just to import from an sql dump?

I know it's just a test, but all the same...

[thr]

Sorry for going off topic, but instead of actually creating the database tables via PHP script, wouldn't it be more convienent, and more pratical just to import from an sql dump?

Yes I know ;p, but as you said.. it's just a test ;P

[Luke Redpath]

Yes I know ;p, but as you said.. it's just a test ;P

I'd still consider refactoring your tests to make them clearer and more concise. Treat them as an equally important part of your production code rather than relegating them to "just tests".

[thr]

I'd still consider refactoring your tests to make them clearer and more concise. Treat them as an equally important part of your production code rather than relegating them to "just tests".

When posting the code here I'd have to say that putting the SQL in the setUp()-method is clearer and more comprehendable then seeing something like:


$sql = file_get_contents("sql.sql");
$this->pdo->exec($sql);

[akrabat]

Is it possible for TearDown() to not actually run when a given test fails?

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

[thr]

Could we skip the whole "sql-in-tests-or-not"-debate? ;p

[kyberfabrikken]

I wonder. Why does the Memberships_test table have a column "id" ? person+group make a good primarykey candidate.

[kyberfabrikken]

More nitpicking :
Would you be able to handle anything but a single integer primarykey properly ?

UnitOfWork seems to be the central element in the API. I find that a bit odd. Wouldn't it be more natural to have a Registry as the central component, and let the uow be a member of the registry ?

I don't see any tests for cyclic dependencies - Have you though of that issue ?

[Ren]

More nitpicking :
Would you be able to handle anything but a single integer primarykey properly ?

UnitOfWork seems to be the central element in the API. I find that a bit odd. Wouldn't it be more natural to have a Registry as the central component, and let the uow be a member of the registry ?

I don't see any tests for cyclic dependencies - Have you though of that issue ?

Think its getting composite primary keys, after a discussion in a previous thread.

[Ren]

Could we skip the whole "sql-in-tests-or-not"-debate? ;p

Heh, was going to post my mysql create table parser tests at some point, might no do now as its littered with SQL

... well couldn't resist atleast one..

	function testMultipleTablesWithReferences()
		{
			$sql1 = 'CREATE TABLE parent(id INT PRIMARY KEY, name NVARCHAR(30));
			   		CREATE TABLE child(id INT PRIMARY KEY, parent_id INT REFERENCES parent(id), name NVARCHAR(40))';
			
			$sql2 = 'CREATE TABLE parent(id INT, name NVARCHAR(30), PRIMARY KEY(id));
						CREATE TABLE child(id INT, parent_id INT, 
						name NVARCHAR(40), PRIMARY KEY(id), FOREIGN KEY (parent_id) REFERENCES parent(id))';
			
			$e = new SqlSchemaEqualExpectation($this->parser, $this->conicaliser, $sql1);
			$this->assert($e, $sql2);
		}

[Luke Redpath]

When posting the code here I'd have to say that putting the SQL in the setUp()-method is clearer and more comprehendable then seeing something like:


$sql = file_get_contents("sql.sql");
$this->pdo->exec($sql);

If that was your reason for doing it, then fair enough

I was mostly making a point about treating your test code the same as you would any other.

[thr]

More nitpicking :
Would you be able to handle anything but a single integer primarykey properly ?

Ah yes - the dreaded joined primary keys. In this version I only have support for single int pk. I have some code which partially support joined pks. Is this crucial to have?

Edit #2: Ok, so I've played around with implementing 100% joined/composite key - and yes it's more then possible to do it, my question is tho - is it realy needed? Yes I know it's a bit bloating of the normalization model to keep an int auto_inc pk in the Membership_tests table in my UnitTests above - but realy, is it THAT bad?

The reason I ask is because well, there's so many things that need to take the composite keys into consideration when doing it with just one pkfield is not just faster, but gives easier to read and maintain code - worth the tradeoff?

UnitOfWork seems to be the central element in the API. I find that a bit odd. Wouldn't it be more natural to have a Registry as the central component, and let the uow be a member of the registry ?

Actually for me it seems more natural to have the uow/worker as the frontend. How would you suggest that this would look code/structure wise?

Edit #1: Gave it some more thought and played around with a small API - this seems more clearer, I'll post some testcode later today.

I don't see any tests for cyclic dependencies - Have you though of that issue ?

About cyclic dependencies - you mean like "One person has many persons" as in, for example, "One child has two parents" ? then yes I have it.

If that was your reason for doing it, then fair enough

I was mostly making a point about treating your test code the same as you would any other.

Ah yes, point taken - but yes that's the main reason I put the SQL in here.

[akrabat]

To me a cyclic dependency is situations like:

• a person's parent is also their child.
• a person's grandparent is himself.

Though this issue may actually have another name!

[thr]

To me a cyclic dependency is situations like:

• a person's parent is also their child.
• a person's grandparent is himself.

Though this issue may actually have another name!

This should be no problem with the current codebase - going to run some tests.

[kyberfabrikken]

Yes I know it's a bit bloating of the normalization model to keep an int auto_inc pk in the Membership_tests table in my UnitTests above - but realy, is it THAT bad?

It's the small things that count, isn't it ? I really dislike the idea that the choice of ORM-layer has impact on my db-schema.

The reason I ask is because well, there's so many things that need to take the composite keys into consideration when doing it with just one pkfield is not just faster, but gives easier to read and maintain code - worth the tradeoff?

I think being correct is worth the hassle, but I can understand where you're comming from ...

About cyclic dependencies - you mean like "One person has many persons" as in, for example, "One child has two parents" ? then yes I have it.

Yes, something like that.

$person = $this->worker->create("Person"); 
$person->parent = $person;
$this->worker->commit(); 

[thr]

It's the small things that count, isn't it ? I really dislike the idea that the choice of ORM-layer has impact on my db-schema.

I think being correct is worth the hassle, but I can understand where you're comming from ...

Ah well, I got composite key support in now ;p

Yes, something like that.

$person = $this->worker->create("Person"); 
$person->parent = $person;
$this->worker->commit(); 

Yes, that's already possible.

[thr]

UnitOfWork seems to be the central element in the API. I find that a bit odd. Wouldn't it be more natural to have a Registry as the central component, and let the uow be a member of the registry ?

Ok, after much ( about 2000 lines of code total) testing and redeploying I've found that the best way (atleast for me) is to keep the UoW as the "main" object. Mostly because the Registry you suggest mainly get's a bunch of "proxy" methods that just relay the call to the Unit of Work. 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.

I also did some reading in PoEAA on DataMappers/Unit of Work/Metadata Mapping and I found that fowler also uses the UoW as main object holding the mappers.

And I got support for cyclic dependecies, composite keys, 1 > 1, 1 < 1, 1 > * and * >< * with named intermediate(the intermediate table is an object), Value Objects. Edit #1: Hopefully I will get support for "CompositeMappers" in - which means you can aggregate two mappers for different types into one single mapper and do complex joins such as: SELECT Person.*, Group.* FROM Person, Group WHERE <Complex Condition> and get both the Person and Group object out with one query. Also support for autoloading in collections and recursion-depth.

Edit #2:I haven't tested on anything exception PostGresql and MySQL but it SHOULD run on all databases supported by PDO atm.

Edit #3: Currently the only thing required to use the implementation is for your domain objects to implement the interface "DomainObject", if you want more functionallity you can extend "GenericMapper" and specify relations, table, value objects and such. Or you can implement the "Snapshotable"-interface which might be important for object with large fields(only the changed fields will be written upon commit, but at the overhead of holding a clone of the object in memory for comparsion).

There's also a AutoInitializing interface for a __initializeField(Array $row = array()) method that will be called when an object is created to populate the fields + do initialization. If that interface is not implemented the object fields will be set from "outside" the object one by one($obj->field1 = val, etc.)

I'll post the code + 500-600 lines of testcases later today or tomorrow, cheers.

[Ren]

Nice.

Are you going to support table/entity inheritance?
Postgres supports it directly CREATE TABLE a (fields); CREATE TABLE b(fields) INHERITS(a);

Or with other rdbms the two tables have the same primary keys, with the base referencing the subclass.

[thr]

Ah yes I know inheritance ;/ I've read all in PoEAA about it (well I've read all of PoEAA like 2 times but hey ;p) articles on the webb, etc. - I just can't decide if it's worth it or not. Currently I can say no, but maybee in the future.

[thr]

Ok I just got the basic layout of the Composite Mapper down - I'm thinking about using something like this for Composite Mappers.

This would look something like this when it's a use:

<?php
$mapper = $reg->newCompositeMapper("Person","Group");
list($persons,$groups) = $mapper->findBySql("select person.*, group.* from person, group where <somecondition>");
?>

Which would return all persons and groups matching that query, in one SQL query. Seems usefull or not?

[kyberfabrikken]

multitable inheritance is something you eventually will need if you have a large domain model.

[Ren]

Ok I just got the basic layout of the Composite Mapper down - I'm thinking about using something like this for Composite Mappers.

This would look something like this when it's a use:

<?php
$mapper = $reg->newCompositeMapper("Person","Group");
list($persons,$groups) = $mapper->findBySql("select person.*, group.* from person, group where <somecondition>");
?>

Which would return all persons and groups matching that query, in one SQL query. Seems usefull or not?

Handy, but I personally would only use something like this if there was only 1-to-1 relationship involved, otherwise more data is being returned from the database that is absolutely necessary.

Can a mapper handle multiple resultsets from a batch of queries, using PDO::nextRowset() ?

<?php
list($person, $orders) = $mapper->findBySQL('SELECT * FROM Person WHERE personId = :personId; SELECT * FROM Orders WHERE personId = :personId');
?>

Might be more useful for quickly getting information, perhaps you use this internally already for pre-fetching related data?

[thr]

Handy, but I personally would only use something like this if there was only 1-to-1 relationship involved, otherwise more data is being returned from the database that is absolutely necessary.

Ok, good point there.

Can a mapper handle multiple resultsets from a batch of queries, using PDO::nextRowset() ?

<?php
list($person, $orders) = $mapper->findBySQL('SELECT * FROM Person WHERE personId = :personId; SELECT * FROM Orders WHERE personId = :personId');
?>

Might be more useful for quickly getting information, perhaps you use this internally already for pre-fetching related data?

Ok, this was a realy good idea - going to have to look into this some more. Right now I'm battling with a LockManager implementation.

multitable inheritance is something you eventually will need if you have a large domain model.

Yes I figured such out on my way home on the buss today =/ Ah well, that's for later anyways.