Flourish PHP Unframework
This is an archived copy of the forum for reference purposes

Creating relations between ActiveRecords

posted by mblarsen 9 years ago

Hi

First a huge thank you to wbond and all of the contributors for this great library, I was and still am, very excited when I stumbled upon it.

I have a problem understanding why newly related records does not get saved. I have three class: Shop, Product, !ProductDescription.

!ProductDescription is essentially a join table between Shop and Product, thus shop_id and product_id is combined into the primary key of the product_descriptions table.

A row of Shop exists in the database. I'm trying to create new Products and !ProductDescriptions. When invoking store() on my Product object only the product gets stored. If I invoke store on my !ProductDescription object I get an error stating that shop_id and product_id is not set.

Here is the code: 

$shop = new Shop(1);

$product = new Product();
$product->setProductCode('XFC-1001');
$product->setProductType('office');
$product->setBrand('Relief');
	
$product_description = new ProductDescription();
$product_description->setName('Torch');
$product_description->setShortDescription('short');
$product_description->setLongDescription('long');
	
$product->associateProductDescriptions($product_description);
$shop->associateProductDescriptions($product_description);
	
$product->store();

Passing TRUE to the store method seems to have no effect.

This is the DB scheme: 

CREATE TABLE `product_descriptions` (
 `product_id` int(11) unsigned NOT NULL,
 `shop_id` int(11) unsigned NOT NULL,
 `locale` varchar(255) NOT NULL DEFAULT 'da-DK',
 `name` varchar(255) NOT NULL,
 `short_description` varchar(255) NOT NULL,
 `long_description` text,
 `package_unit` varchar(255) NOT NULL DEFAULT 'stk',
 PRIMARY KEY (`product_id`,`shop_id`),
 KEY `shop_id` (`shop_id`),
 CONSTRAINT `product_descriptions_ibfk_1` FOREIGN KEY (`product_id`) REFERENCES `products` (`product_id`) ON DELETE CASCADE,
 CONSTRAINT `product_descriptions_ibfk_2` FOREIGN KEY (`shop_id`) REFERENCES `shops` (`shop_id`) ON DELETE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8

CREATE TABLE `shops` (
 `shop_id` int(10) unsigned NOT NULL AUTO_INCREMENT,
 `name` varchar(255) NOT NULL,
 PRIMARY KEY (`shop_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8

CREATE TABLE `products` (
 `product_id` int(11) unsigned NOT NULL AUTO_INCREMENT,
 `date_created` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
 `product_code` varchar(255) NOT NULL,
 `package_size` smallint(6) NOT NULL DEFAULT '1',
 `brand` varchar(255) NOT NULL,
 `original` tinyint(1) NOT NULL DEFAULT '0',
 `product_type` varchar(255) NOT NULL,
 PRIMARY KEY (`product_id`),
 KEY `product_code` (`product_code`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8

I have looked through http://flourishlib.com/docs/fActiveRecord#RelatedRecordsOperations and the other ORM pages, but haven't found anything that could help me. These pages seems to have a bias on retrieving records.

I have a feeling my problem has to do with the fact that product_descriptions is a join table between products and shops, but it does not follow that suggest naming scheme: products_shops?

Any help will be most appreciated

In this semi-complex situation, the way that Flourish performs the actions is causing the issue, however I don't know if it can be solved in a generic way. Perhaps an exception can be added that will detect this.

So, when you call associate and then store the parent record, what happens is:

  1. The parent record is stored in the database - any auto-incrementing key is retrieved and stored in the record
  2. The child records are iterated through, and the parent primary key is set on each and then each is saved

Since the child record requires values from two different parents to save, and the parent primary key is set when it saves, it is not possible to save the group of records all at once. In order to do this, Flourish would have to detect that the child record, ProductDescription, requires keys from two parents and would have to trigger store() on the one that wasn't stored. This would be odd, since the programmer never explicitly requested that the Shop be stored.

The way to get around this is to do the following: 

$shop = new Shop(1);
 
$product = new Product();
$product->setProductCode('XFC-1001');
$product->setProductType('office');
$product->setBrand('Relief');
    
$product_description = new ProductDescription();
$product_description->setName('Torch');
$product_description->setShortDescription('short');
$product_description->setLongDescription('long');

$product_description->setShopId($shop->getShopId());    
$product->associateProductDescriptions($product_description);

$product->store();
posted by wbond 9 years ago

"Since the child record requires values from two different parents to save, and the parent primary key is set when it saves"

In this situation the Shop object has been loaded from the db. It already has the id. Does this change anything?

The change you suggested does not seem to work, enabling debug for the db, shows that only one INSERT is created.

Exposing $product shows that the primary key of the related record $product_description is null: 

[related_records:protected] => Array
        (
            [product_descriptions] => Array
                (
                    [product_id] => Array
                        (
                            [record_set] => fRecordSet Object
                                (
                                    [class:private] => ProductDescription
                                    [non_limited_count:private] => {null}
                                    [pointer:private] => 0
                                    [records:private] => Array
                                        (
                                        )
                          
                                )
                    
                            [count] => 0
                            [associate] => {false}
                            [primary_keys] => {null}
                        )
              
                )
        
        )

Or how should this be interpreted?

Thanks

posted by mblarsen 9 years ago

Hi, got the code to save the !ProductDescription. It seems that I have to save invoke store() on $product and then set the id in the same manner that you suggested for the shop. This also makes the product association obsolete.

Note that storing $product again, even with associations set, will not cause the $product_description to be stored, thus storing it "manually".

Problem solved, kind of. Hope you can use the input.

Would it make a difference setting columns specifically in configure()?

See REQUIRED, NOT REQUIRED BELOW. 

	$p = new Product();
	$p->setProductCode('XFC-1001');
	$p->setProductType('office');
	$p->setBrand('Relief');
	
	$p->store(); // REQUIRED for the setting of id below
	
	$pd = new ProductDescription();
	$pd->setShortDescription('short');
	$pd->setName('name');
	$pd->setLongDescription('long');
	
	$pd->setShopId($s->getShopId());
	$pd->setProductId($p->getProductId()); // REQUIRED
	//$p->associateProductDescriptions($pd); // NOT REQUIRED
	
	//$p->store(); // NOT EFFECT even if TRUE is passed
	
	$pd->store(); // REQUIRED
posted by mblarsen 9 years ago

Yes, the method you finalized on will definitely work, however the other methods should also.

I'll open a ticket so that I make sure to look into associate to see what would cause it not to work in this situation.

posted by wbond 9 years ago

The method commented out now works in r819. The issue was that a single fActiveRecord object was being passed instead of an array of fActiveRecord objects. I changed the method to accept a single record.

posted by wbond 9 years ago

Hi, this is great.

However wont I still have to do something like this? 

$o->setShopId($s->getShopId());
$p->associateOfferings($o);

$p->store();

Where this would be the way to go: 

$s->associateOfferings($o);
$p->associateOfferings($o);

$p->store();

or better (as the offering object is main subject of this piece of code): 

$o->associateShop($s);      // $s has been fetch frem db, thus it has an ID
$o->associateProduct($p);   // $p is a new record

$p->store();                // This would store $p and associates
posted by mblarsen 9 years ago

Yes, you need to use the first example as that is what is currently implemented.

The second and third example may seem logical in this specific situation, but they have issues in the more general case. The associate verb not only links two records together, but ties the records together for validation and storing. It removes validation messages about columns that will be filled in upon calling store() since it knows the values will be available later.

So, say you have a User and !UserPreferences. You could populate your user object and associate a number of !UserPreference objects with it. When you save the User object, the associated !UserPreference object would all be validated. If there were any validation errors, other than the user_id, they would be presented as error of the children, such as "User Preference #2 Name: Please enter a value."

In your second example, you are associating a while with two parents. Thus we no longer are dealing with just a parent -> child relationship, but two parents and a child all which must validate each other and produce sane validation messages about what is going on. That combined with the fact that the related records implementation stores information about children, but not parents, makes it really impossible without reimagining how related records are stored and validated.

The third example is basically the same as the second, and suffers from the same issues. Basically you just have to explicitly tell it what is going on via calling set... for one of the parents instead of associate....

posted by wbond 9 years ago

I do see your points in the two-parent scenario with regards to the current implementation of records.

My "ORM-background" comes from Apple's WebObjects in which you opperate with an EditionContext object (a sandbox) in which you at the end commits/stores. Neither child nor parent centric.

I guess it would be similar to implemnting the store() method on fORM (if it had instances) instead of on fActiveRecord.

What are you thought on making object association, say, more liberal. Maybe taking parents into account?

I will look into the code and validation issues. Maybe I could help out.

posted by mblarsen 9 years ago

I can definitely see the usefulness of grouping a number of fActiveRecord objects into a single transaction, but also aggregating the validation message and performing delayed association.

I think a new class would be in order, something such as !fUnitOfWork. This could perform logic such as association, group all of the changes into a single database transaction, but also aggregating validation messages with prefixed field names with object names when there were duplicate field names.

Right now I'm putting some more time into a side project, but it is something that sounds very useful. If you are interested in working on such a class, I would be very appreciative, I just want to let you know that I do require a contributor license agreement for code submitted to Flourish.

The CLA is a legal document that more or less says A) you have the appropriate rights to license the code to Flourish and it users under the MIT license, B) that you are actually licensing it and C) should there be patents or other IP issues with the code, Flourish and its users are not legally responsible.

If that hasn't scared you away, here is the kind of functionality I would minimally expect from the unit of work:

  • A method to add a record to the unit of work, with an optional prefix to use for validation messages.
  • A method to add a record or set of records to the unit of work and associate the record to another record or set of records. This would probably need to handle all types of relationships, including many-to-one, which is what the associate action doesn't do.
  • A method to call validate() on all of the record and create a logical exception message, resolving field name conflicts with the provided prefixed, or a prefix based on the class name.
  • A method to work out what order the record should be saved in, ensure all of the record are using the same database, and then call store() on all of the records and during the process set the appropriate methods to associate the records together.

The only other thing to think about would be if delete operations should be allowed to be mixed into the unit of work. It seems like from a conceptual standpoint it would make sense, I just don't know off of the top of my head if it would require much more work.

What do you think about all of this?

posted by wbond 9 years ago

Hi will, no I'm not scared away, I would be glad to help out.

That said it would be a big rewrite of some of the classes as well as a change of code style. From something like 

$myRecord = new MyRecord(1);
.. make some changes ..
$myRecord->store();

To something like: 

$unitOfWork = new fUnitOfWork();

$myRecord = new MyRecord();

.. make some changes ..

$unitOfWork->add($myRecord);

$unitOfWork->store();

Of couse there are variations on this. Like always passing a fUnitOfWork object to the constructor of a fActiveRecord and the the fRecord::build methods.

Do you see the fUnitOfWork as an option or would it be a central change to the framework?

I've looked at two other ORM frameworks implementing unit of work:

The two uses slightly different syntaxes:

Repose (from their page): 

$project = new Project("Original name");
$project = $session->add($project);
$project->name = "New name"; // Session will NOT lose this change!
$session->flush();

Outlet (from their page): 

$outlet = Outlet::getInstance();
 
$client = new Client;
$client->Name = 'Test Client';
 
$project = new Project;
$project->Name = 'Cool Project';
$project->setClient( $client );
 
$bug = new Bug;
$bug->Title = "Button doesn't work";
 
$project->addBug( $bug );
 
// inserts the project 
// and all of the related entities
// in one transaction
$outlet->save( $project );

The main differences being at which point you "interact" with the unit of work object. Where Repose adds objects upon object creation, adding object by object. Outlet defers interaction with unit of work object till the point at which you commit your changes.

I've previously worked with frameworks using a code style like Repose, and think it works fine, but I do like the simplicity of the style used by Outlet. The code focuses on working with the records/objects (proxies) rather than the unit of work object.

What do you think?

With regards to validation, could you point out to me where work should be done?

posted by mblarsen 9 years ago

Sorry that is has taken so long to get back to this topic. As always, there is way too much to be done, and time is the one thing we are ultimately constrained by.

My vision of an !fUnitOfWork class would be an optional class that could even be named something like !fTransaction. You would create the UOW object and then add records to it, however you could continue using the existing fActiveRecord methods for situations that don't require the extra complexity. Upon further consideration, I would think it should also handle object deletion.

Here is an initial idea for the public API of an !fUnitOfWork class. 

class fUnitOfWork
{
    /**
     *@param fActiveRecord|fRecordSet|array $related_records  The record(s) to associate
     *@param fActiveRecord|fRecordSet|array $records  The record(s) to associate the related records to
     *@param string $route  The relationship route from $records to $related_records
     */
    public function associate($related_records, $records, $route=NULL) { }

    public function commit() { }

    /**
     *@param fActiveRecord|fRecordSet|array $records  The record(s) to delete
     *@param string $validation_message_prefix  A string to prepend before all validation messages for this record or records
     */
    public function delete($records) { }

    /**
     *@param fActiveRecord|fRecordSet|array $records  The record(s) to store
     *@param string $validation_message_prefix  A string to prepend before all validation messages for this record or records
     */
    public function store($records, $validation_message_prefix='') { }

    /**
     *@param boolean $return_messages  If the validation messages should be returned as an array instead of being thrown as an fValidationException
     *@param boolean $remove_column_names  If the column names should be removed from the validation messages
     */
    public function validate($return_messages=FALSE, $remove_column_names=FALSE) { }
}

My main concerns would be calling the methods store and delete. I almost feel like they should be called something like queueStore() and queueDelete().

What are your thoughts on this API?

posted by wbond 9 years ago