[DC-137] Doctrine_Record_Generator does not autoload classes Created: 23/Oct/09  Updated: 02/Dec/09  Resolved: 02/Nov/09

Status: Closed
Project: Doctrine 1
Component/s: Record
Affects Version/s: 1.2.0-ALPHA2
Fix Version/s: 1.2.0-BETA1

Type: Bug Priority: Major
Reporter: Brian Voelschow Assignee: Jonathan H. Wage
Resolution: Fixed Votes: 0
Labels: None

CentOS 5.3, PHP 5.2.5


Using the versionable behavior creates dynamic classes and does not autoload the models that are defined. Removing the false flag from if ($this->_options['generateFiles'] === false && class_exists($this->_options['className']. false )) { line will fix the issue

Comment by Jonathan H. Wage [ 24/Oct/09 ]

Hmm. I remember once upon a time this was this way for a reason. Can you show some code for what you're doing so I can test this myself. Are you writing the generated files to disk somewhere?

Comment by Brian Voelschow [ 28/Oct/09 ]

I wrote a model class so we can access the version history and display it in an application. The code that we are using is just trying to save versions when we have database delegation. We enabled the delegation code in our X_Doctrine_Record class. When debugging the calls we noticed that the correct model does not get loaded, but the generated model did. We need the model that we created so that our delegation will work and save the version to the master DB.

Steps to Reproduce:
1. Add versionable behavior to an existing model
2. Create a model for the version table
3. Try to save an object. Error returned - If you do not allow saves on your slave connection

Comment by Jonathan H. Wage [ 28/Oct/09 ]

I see. The part where you create your own model class is not how this is intended to work. It is intended that you let Doctrine generate the model class for you.

Comment by Brian Voelschow [ 28/Oct/09 ]

Understood that this is not how the code is intended to work, but without having a model there is no way to get the complete history. The only way right now to get the complete history of a record would be to loop through the version records after getting the max version from the original record.

Comment by Jonathan H. Wage [ 28/Oct/09 ]

I'm failing to understand. The behavior generates a model for you named ModelNameVersion and ModelName hasMany ModelNameVersion, no?

Comment by Brian Voelschow [ 28/Oct/09 ]

The relationship was not created. We are adding it and going to some testing. Get back to you

Comment by Brian Voelschow [ 28/Oct/09 ]

The relationship worked on the generated class, but there is still an issue. Without creating a model for the version table, there is no way to use our delegation code. This still causes the problem of saving to the slave. or reading from the master.

Comment by Jonathan H. Wage [ 28/Oct/09 ]

Hmm. The model does exist though. It is called "ModelNameVersion" and it is auto generated for you by Doctrine_Record_Generator. It also has options to write the generated model to a path somewhere.

Comment by Brian Voelschow [ 28/Oct/09 ]

Without autoloading the class by removing the false flag in the generator code, the saved model will never get loaded. Another issue is that when trying to use the saved generated class, we get an error that it cannot find the id field that it created. The id field does not exist in the table.

Comment by Jonathan H. Wage [ 02/Nov/09 ]

We'll give this change a try. If it causes some issues for people we may have to revert it. I remember that false argument being there for a reason but I can't remember and none of our tests pass without it.

Comment by Brian Voelschow [ 03/Nov/09 ]

If your tests are not passing then there must be a reason for it to be set to false. I think a configuration option here would be a better solution. Similar to the Doctrine::ATTR_AUTOLOAD_TABLE_CLASSES setting.

Comment by Adam Jensen [ 20/Nov/09 ]

For what it's worth, I'm running r6785 of the http://svn.doctrine-project.org/tags/1.2.0-BETA3 tag, and the absence of the false argument is causing me some annoying issues.

Based on the documentation/parameters/etc., I'm assuming both of the following use cases are supposed to be possible:

A. The ModelNameVersion class is generated on-the-fly as necessary, and never stored in the filesystem.
B. The ModelNameVersion class is generated once, stored in the filesystem, and subsequently autoloaded from there.

Now, from what I can tell, both use cases work correctly even without the false argument on line 159; however, in case A, a PHP warning can be generated if one of the registered autoloaders doesn't check to see if the file exists before including it. This appears to be the case with Zend_Loader_Autoloader_Resource, for instance; my guess is that it's designed that way such that non-existing files produce the same warnings you'd get if you tried to include them manually ...but it doesn't play nice with Doctrine's expectations here (and heck, it may well be a bug on their part to write it that way).

These warnings don't appear to affect any functionality ...the line 159 conditional still turns up false, allowing the class to be generated on the fly. All the same, I'd prefer not to have to turn off E_WARNING on my development machine just to avoid seeing these. Since Doctrine can't really guarantee that all of the autoloaders registered with SPL are properly avoiding such blind includes, it may make more sense to leave the false argument in place and avoid the warnings.

Of course, I don't know what effect that would have on use case B; my guess is that it'd break, since that's what this issue was originally about Is there another solution available that satisfies both use cases without raising any errors?


Comment by Joël [ 02/Dec/09 ]

Hi !

I'm using Zend Framework and Doctrine and I got warnings with Doctrine_Template_Searchable that was trying to load the Index classes of the Searchable behaviour.
And as you said, with the false argument of get_class() Zend_Framework failed...

I tried with 1.9.6 of Zend_Framework and it still failed, although they seem to say in your external reference ([ZF-8364] Zend_Loader_Autoloader_Resource::autoload() should return false if no match is found) that it would be fixed in release 1.9.6...

Here it is... it was just to point it out

Comment by Jonathan H. Wage [ 02/Dec/09 ]

Doesn't the autoloader have an option to check if the file exists before trying to include it?

Generated at Tue Dec 01 20:41:07 EST 2015 using JIRA 6.4.10#64025-sha1:5b8b74079161cd76a20ab66dda52747ee6701bd6.