Doctrine 1
  1. Doctrine 1
  2. DC-137

Doctrine_Record_Generator does not autoload classes

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0-ALPHA2
    • Fix Version/s: 1.2.0-BETA1
    • Component/s: Record
    • Labels:
      None
    • Environment:
      CentOS 5.3, PHP 5.2.5

      Description

      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

        Activity

        Hide
        Jonathan H. Wage added a comment -

        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?

        Show
        Jonathan H. Wage added a comment - 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?
        Hide
        Brian Voelschow added a comment -

        Jonathan,
        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

        Show
        Brian Voelschow added a comment - Jonathan, 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
        Hide
        Jonathan H. Wage added a comment -

        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.

        Show
        Jonathan H. Wage added a comment - 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.
        Hide
        Brian Voelschow added a comment -

        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.

        Show
        Brian Voelschow added a comment - 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.
        Hide
        Jonathan H. Wage added a comment -

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

        Show
        Jonathan H. Wage added a comment - I'm failing to understand. The behavior generates a model for you named ModelNameVersion and ModelName hasMany ModelNameVersion, no?
        Hide
        Brian Voelschow added a comment -

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

        Show
        Brian Voelschow added a comment - The relationship was not created. We are adding it and going to some testing. Get back to you
        Hide
        Brian Voelschow added a comment -

        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.

        Show
        Brian Voelschow added a comment - 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.
        Hide
        Jonathan H. Wage added a comment -

        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.

        Show
        Jonathan H. Wage added a comment - 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.
        Hide
        Brian Voelschow added a comment -

        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.

        Show
        Brian Voelschow added a comment - 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.
        Hide
        Jonathan H. Wage added a comment -

        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.

        Show
        Jonathan H. Wage added a comment - 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.
        Hide
        Brian Voelschow added a comment -

        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.

        Show
        Brian Voelschow added a comment - 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.
        Hide
        Adam Jensen added a comment -

        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?

        Thanks!
        Adam

        Show
        Adam Jensen added a comment - 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? Thanks! Adam
        Hide
        Joël added a comment -

        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

        Show
        Joël added a comment - 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
        Hide
        Jonathan H. Wage added a comment -

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

        Show
        Jonathan H. Wage added a comment - Doesn't the autoloader have an option to check if the file exists before trying to include it?

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            Brian Voelschow
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: