Ticket #1064 (closed defect: fixed)

Opened 14 months ago

Last modified 9 months ago

generateFiles option to Doctrine_Record_Generator does not regenerate existing files

Reported by: Jon.Collins Owned by: romanb
Priority: minor Milestone: 1.0.3
Component: Record Version: 1.0.0
Severity: Keywords:
Cc: Has Test: no
Status: Pending Core Response Has Patch: no

Description

Line 160 of Record/Generator.php:

        if (class_exists($this->_options['className'])) {
            return false;
        }

If files were previously generated into an autoloaded folder, class_exists() autoloads the class and breaks out of the Doctrine_Record_Generator::initialize() method.

The goal of the statement appears to be to avoid redefining the class in the case that generateFiles = false and the class definition is eval()'d into the script by Doctrine_Record_Generator::generateClass().

At first glance it looks like the check is not necessary if generateFiles = true, in fact we do want the new class definition to be loaded eventually by Doctrine_Import_Builder::writeDefinition().

Possible fix:

        if ($this->_options['generateFiles'] == false && class_exists($this->_options['className'])) {
            return false;
        }

This will fail at the && and avoid autoloading the class. It seems appropriate to move this to a refactored Doctrine_Record_Generator::generateClass().

Change History

Changed 14 months ago by jwage

  • status changed from new to closed
  • resolution set to fixed

(In [4410]) fixes #1064

Changed 10 months ago by anonymous

  • milestone New deleted

Milestone New deleted

Changed 9 months ago by ggoodman

  • status changed from closed to reopened
  • has_test unset
  • mystatus set to Pending Core Response
  • milestone set to 1.0.1
  • has_patch unset
  • resolution fixed deleted

There is a slight bug in this that prevents integration with spl_autoload. Lines 161-163 of Doctrine/Record/Generator.php should read:

        if ($this->_options['generateFiles'] === false && class_exists($this->_options['className'], false)) {
            return false;
        }

The fix is to prevent class_exists from triggering an autoload call by passing 'false' as the 2nd parameter.

Changed 9 months ago by jwage

  • status changed from reopened to closed
  • resolution set to fixed

Fixed by r4991. Thanks for the report.

Changed 9 months ago by jwage

  • milestone changed from 1.0.1 to 1.0.3

Changed 9 months ago by jwage

  • version changed from 0.11 to 1.0
Note: See TracTickets for help on using tickets.