Doctrine 1

issue with multiple connection handling

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.2.2
  • Fix Version/s: 1.2.2
  • Component/s: Connection
  • Labels:
    None

Description

I've found an issue where doctrine will use the wrong connection for tables under certain conditions.

In a template, I'm doing a $sf_user->hasCredential() - which is causing this to be run in sfGuardSecurityUser,

$this->user = Doctrine::getTable('sfGuardUser')->find($id);

When this execute, the calls find themselves to Doctrine_Manager::getConnectionForComponent($componentName)

This method calls Doctrine_Core::modelsAutoload($componentName);, which fails to load the class, and returns false (no checking is done to see if it should return true).

As this fails to include the sfGuardUser classes wher the component binding goes on, the getTAble call will use the default connection, then create the table fails to use the correct connection

Doctrine_Core::getTable()

return Doctrine_Manager::getInstance()->getConnectionForComponent($componentName)->getTable($componentName);

the binding is done after the call to getConectionForComponent, as it's getTable that will ultimately cause the autoloader to pull in the table classes.

  1. Text File
    doctrine_core.patch
    07/Jul/10 6:05 AM
    0.7 kB
    Marcel Berteler
  2. Text File
    doctrine_manager.patch
    27/Sep/10 3:03 AM
    2 kB
    Marcel Berteler

Activity

Descending order - Click to sort in ascending order
Hide
Permalink
Dean de Bree added a comment -

I found that if I changed the getTable function inside the Core.php file it seemed to work. Basically it forces the autoloader to load the object file, and when it does this it runs the bound connection statement to bind a table to a connection.

Core.php
/**
     * Get the Doctrine_Table object for the passed model
     *
     * @param string $componentName
     * @return Doctrine_Table
     */
    public static function getTable($componentName)
    {
        if (!class_exists($componentName)) {
            new $componentName();
        }

        return Doctrine_Manager::getInstance()->getConnectionForComponent($componentName)->getTable($componentName);
    }
Show
Dean de Bree added a comment - I found that if I changed the getTable function inside the Core.php file it seemed to work. Basically it forces the autoloader to load the object file, and when it does this it runs the bound connection statement to bind a table to a connection. Core.php /** * Get the Doctrine_Table object for the passed model * * @param string $componentName * @ return Doctrine_Table */ public static function getTable($componentName) { if (!class_exists($componentName)) { new $componentName(); } return Doctrine_Manager::getInstance()->getConnectionForComponent($componentName)->getTable($componentName); }
Hide
Permalink
Eugeniy Belyaev added a comment -
Another bad way to get it working in symfony:


ProjectConfiguration.class.php
  public function configureDoctrine(Doctrine_Manager $manager)
  {
    $files = sfFinder::type('file')
      ->maxdepth(0)
      ->not_name('*Table.class.php')
      ->name('*.class.php')
      ->in(sfConfig::get('sf_lib_dir') . '/model/doctrine');

    foreach ($files as $file) {
      $class_name = str_replace('.class.php', '', basename($file));
      Doctrine_Core::loadModel($class_name, $file);
    }
  }
Show
Eugeniy Belyaev added a comment - Another bad way to get it working in symfony: ProjectConfiguration.class.php public function configureDoctrine(Doctrine_Manager $manager) { $files = sfFinder::type('file') ->maxdepth(0) ->not_name('*Table.class.php') ->name('*.class.php') ->in(sfConfig::get('sf_lib_dir') . '/model/doctrine'); foreach ($files as $file) { $class_name = str_replace('.class.php', '', basename($file)); Doctrine_Core::loadModel($class_name, $file); } }
Hide
Permalink
Marcel Berteler added a comment -

second required patch to make Symfony work with 2 concurrent databases

Show
Marcel Berteler added a comment - second required patch to make Symfony work with 2 concurrent databases
Hide
Permalink
Ian P. Christian added a comment - - edited

This effects migrations too it seems:

Even doing this:

    $manager = Doctrine_Manager::getInstance();                                                          
    $manager->bindComponent('ChangeRequest', 'nosp');                                                    
    $manager->bindComponent('change_request', 'nosp');                                                   
class Addstatetochangerequest extends Doctrine_Migration_Base                                            
{                                                                                                        
  public function up()                                                                                   
  {                                                                                                      
    $this->addColumn('change_request', 'change_state', 'enum', array('values' => array('draft', 'submitted', 'approved', 'rejected', 'closed')));

This results in:

# ./symfony doctrine:migrate
>> doctrine  Migrating from version 0 to 1
                                                                                                                                                                                    
  The following errors occurred:                                                                                                                                                    
                                                                                                                                                                                    
   - SQLSTATE[42S02]: Base table or view not found: 1146 Table 'nosp_test_radius2.change_request' doesn't exist. Failing Query: "ALTER TABLE change_request ADD change_state TEXT"

The database attempted to be used there is not the correct one.

Show
Ian P. Christian added a comment - - edited This effects migrations too it seems: Even doing this: $manager = Doctrine_Manager::getInstance(); $manager->bindComponent('ChangeRequest', 'nosp'); $manager->bindComponent('change_request', 'nosp'); class Addstatetochangerequest extends Doctrine_Migration_Base { public function up() { $ this ->addColumn('change_request', 'change_state', ' enum ', array('values' => array('draft', 'submitted', 'approved', 'rejected', 'closed'))); This results in: # ./symfony doctrine:migrate >> doctrine Migrating from version 0 to 1 The following errors occurred: - SQLSTATE[42S02]: Base table or view not found: 1146 Table 'nosp_test_radius2.change_request' doesn't exist. Failing Query: "ALTER TABLE change_request ADD change_state TEXT" The database attempted to be used there is not the correct one.
Hide
Permalink
Marcel Berteler added a comment -

The patch to Doctrine_Core

This is a hack that only works when used in sfDoctrinePlugin / Symfony

Not intended as the final patch to fix this bug but as a work around to make multiple connections usable.

Show
Marcel Berteler added a comment - The patch to Doctrine_Core This is a hack that only works when used in sfDoctrinePlugin / Symfony Not intended as the final patch to fix this bug but as a work around to make multiple connections usable.
Hide
Permalink
Marcel Berteler added a comment -

Oh, and to make sfDoctrineGuard work properly, you might have to ensure the sfBasicSecurityUser is bound to the correct model.

You can do this in plugins\sfDoctrineGuardPlugin\lib\user\sfGuardSecurityUser.class.php or in apps\xxxxx\lib\myUser.class.php

 
Doctrine_Manager::getInstance()->bindComponent('sfGuardUser', 'connectionName');
Show
Marcel Berteler added a comment - Oh, and to make sfDoctrineGuard work properly, you might have to ensure the sfBasicSecurityUser is bound to the correct model. You can do this in plugins\sfDoctrineGuardPlugin\lib\user\sfGuardSecurityUser.class.php or in apps\xxxxx\lib\myUser.class.php Doctrine_Manager::getInstance()->bindComponent('sfGuardUser', 'connectionName');
Hide
Permalink
Marcel Berteler added a comment -

Sorry... the above does not work.

Doctrine_Core->autoload() is called and not Doctrine->autoload().

sfAutoload::getInstance()->autoload($className);

can be added to Doctrine_Core line 1133

public static function modelsAutoload($className)
    {
        if (class_exists($className, false) || interface_exists($className, false)) {
            return false;
        }

        sfAutoload::getInstance()->autoload($className);
Show
Marcel Berteler added a comment - Sorry... the above does not work. Doctrine_Core->autoload() is called and not Doctrine->autoload(). sfAutoload::getInstance()->autoload($className); can be added to Doctrine_Core line 1133 public static function modelsAutoload($className) { if (class_exists($className, false) || interface_exists($className, false)) { return false; } sfAutoload::getInstance()->autoload($className);
Hide
Permalink
Marcel Berteler added a comment - - edited

After a long and hard look at the sfDoctrinePlugin and Doctrine code I can to the same conclusion. The Doctrine autoload is not working in sfDoctrinePlugin. I think this is more a sfDoctrinePlugin bug than a Doctrine bug.

Instead of manual binding, a better way is the actually make sure the intended flow of the code is working like it should be.

To be able to use sfDoctrineGuard with multiple connections you need to ensure that the connection name is added to the Schema of sfDoctrineGuard. Once this is done, rebuilding the model will put a bindComponent in the class files.

This works fine if the autoload is working like it should.

To get the autoload to work, you can extend the autoload function of Doctrine_Core in Doctrine:

The Doctrine.php file is empty by default, so its easy to add your code to it (until the problem is fixed without having to edit Doctrine code)

lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine.php:

 
class Doctrine extends Doctrine_Core
{
  public static function modelsAutoload($className)
  {
    sfAutoload::getInstance()->autoload($className);

    parent::modelsAutoload($className);
  }
  
}
Show
Marcel Berteler added a comment - - edited After a long and hard look at the sfDoctrinePlugin and Doctrine code I can to the same conclusion. The Doctrine autoload is not working in sfDoctrinePlugin. I think this is more a sfDoctrinePlugin bug than a Doctrine bug. Instead of manual binding, a better way is the actually make sure the intended flow of the code is working like it should be. To be able to use sfDoctrineGuard with multiple connections you need to ensure that the connection name is added to the Schema of sfDoctrineGuard. Once this is done, rebuilding the model will put a bindComponent in the class files. This works fine if the autoload is working like it should. To get the autoload to work, you can extend the autoload function of Doctrine_Core in Doctrine: The Doctrine.php file is empty by default, so its easy to add your code to it (until the problem is fixed without having to edit Doctrine code) lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine.php: class Doctrine extends Doctrine_Core { public static function modelsAutoload($className) { sfAutoload::getInstance()->autoload($className); parent::modelsAutoload($className); } }
Hide
Permalink
Ian P. Christian added a comment -

I've found a work around to this, not sure if it's a desirable fix though...

In the project configuration class, I've added this to the setup()

    $this->dispatcher->connect('doctrine.configure', array($this, 'doctrineBinder'));

and the following method is also added, were I'm manually doing my bindings...

  public function doctrineBinder(sfEvent $event)
  {
    $manager = Doctrine_Manager::getInstance();
    $manager->bindComponent('sfGuardUser', 'nosp');
    $manager->bindComponent('Incident', 'nosp');
    $manager->bindComponent('ServiceIp', 'ip');

  }

The overhead here isn't really that high (it just sets an element in the array) - I'd also wonder if a bindComponents($array); should be added to simplify this call, but that's another method.

Show
Ian P. Christian added a comment - I've found a work around to this, not sure if it's a desirable fix though... In the project configuration class, I've added this to the setup() $ this ->dispatcher->connect('doctrine.configure', array($ this , 'doctrineBinder')); and the following method is also added, were I'm manually doing my bindings... public function doctrineBinder(sfEvent $event) { $manager = Doctrine_Manager::getInstance(); $manager->bindComponent('sfGuardUser', 'nosp'); $manager->bindComponent('Incident', 'nosp'); $manager->bindComponent('ServiceIp', 'ip'); } The overhead here isn't really that high (it just sets an element in the array) - I'd also wonder if a bindComponents($array); should be added to simplify this call, but that's another method.
Hide
Permalink
Ian P. Christian added a comment - - edited

This was not a problem before r7668 (at least, not for most use cases)....

It used to be that null was passed as the first arg in the D_Query::create() method call, causing the query to figure out itself which connection to use, which was done after the component was bound, so that's fine!

However, the code below is how it is in the current head

    public function createQuery($alias = '')
    {
        if ( ! empty($alias)) {
            $alias = ' ' . trim($alias);
        }

        $class = $this->getAttribute(Doctrine_Core::ATTR_QUERY_CLASS);

        return Doctrine_Query::create($this->_conn, $class)
            ->from($this->getComponentName() . $alias);
    }

Here , the connection of the table (as explained above is previously set incorerctly) is passed to the query.

Show
Ian P. Christian added a comment - - edited This was not a problem before r7668 (at least, not for most use cases).... It used to be that null was passed as the first arg in the D_Query::create() method call, causing the query to figure out itself which connection to use, which was done after the component was bound, so that's fine! However, the code below is how it is in the current head public function createQuery($alias = '') { if ( ! empty($alias)) { $alias = ' ' . trim($alias); } $class = $ this ->getAttribute(Doctrine_Core::ATTR_QUERY_CLASS); return Doctrine_Query::create($ this ->_conn, $class) ->from($ this ->getComponentName() . $alias); } Here , the connection of the table (as explained above is previously set incorerctly) is passed to the query.
Hide
Permalink
Ian P. Christian added a comment - - edited

Just to expand on this...

This obviously gets called when a call to getTable is made:

    public function getConnectionForComponent($componentName)
    {
        Doctrine_Core::modelsAutoload($componentName);

        if (isset($this->_bound[$componentName])) {
            return $this->getConnection($this->_bound[$componentName]);
        }

        return $this->getCurrentConnection();
    }

The autoload fails, as you can see from the code...

    public static function modelsAutoload($className)
    {
        if (class_exists($className, false) || interface_exists($className, false)) {
            return false;
        }

        if ( ! self::$_modelsDirectory) {
            $loadedModels = self::$_loadedModelFiles;

            if (isset($loadedModels[$className]) && file_exists($loadedModels[$className])) {
                require $loadedModels[$className];

                return true;
            }
        } else {
            $class = self::$_modelsDirectory . DIRECTORY_SEPARATOR . str_replace('_', DIRECTORY_SEPARATOR, $className) . '.php';

            if (file_exists($class)) {
                require $class;

                return true;
            }
        }
        return false;
    }

$_modelsDirectory is never set, and $_loadedModelFiles is an empty array. The $_modelsDirectory, even if set, wouldn't handle loading for plugins, which put their models in places like lib/model/doctrine/sfDoctrineGuardPlugin/sfGuardUser.class.php.

Show
Ian P. Christian added a comment - - edited Just to expand on this... This obviously gets called when a call to getTable is made: public function getConnectionForComponent($componentName) { Doctrine_Core::modelsAutoload($componentName); if (isset($ this ->_bound[$componentName])) { return $ this ->getConnection($ this ->_bound[$componentName]); } return $ this ->getCurrentConnection(); } The autoload fails, as you can see from the code... public static function modelsAutoload($className) { if (class_exists($className, false ) || interface_exists($className, false )) { return false ; } if ( ! self::$_modelsDirectory) { $loadedModels = self::$_loadedModelFiles; if (isset($loadedModels[$className]) && file_exists($loadedModels[$className])) { require $loadedModels[$className]; return true ; } } else { $class = self::$_modelsDirectory . DIRECTORY_SEPARATOR . str_replace('_', DIRECTORY_SEPARATOR, $className) . '.php'; if (file_exists($class)) { require $class; return true ; } } return false ; } $_modelsDirectory is never set, and $_loadedModelFiles is an empty array. The $_modelsDirectory, even if set, wouldn't handle loading for plugins, which put their models in places like lib/model/doctrine/sfDoctrineGuardPlugin/sfGuardUser.class.php.
Hide
Permalink
Ian P. Christian added a comment - - edited

The reason no autoload error is throw, is because symfony's autoloader loads the class for you, but it does it at the getTable() call, which as seen below from Doctrine_Core::getTable(), it's proxied though the connection - which is created before the gable is instanced, which of when the file is actaully loaded.

return Doctrine_Manager::getInstance()->getConnectionForComponent($componentName)->getTable($componentName);
Show
Ian P. Christian added a comment - - edited The reason no autoload error is throw, is because symfony's autoloader loads the class for you, but it does it at the getTable() call, which as seen below from Doctrine_Core::getTable(), it's proxied though the connection - which is created before the gable is instanced, which of when the file is actaully loaded. return Doctrine_Manager::getInstance()->getConnectionForComponent($componentName)->getTable($componentName);
Hide
Permalink
Jonathan H. Wage added a comment -

Hmm, why is the autoloading of sfGuardUser failing? I don't understand that part. If it is failing do you get a cannot load class error?

Show
Jonathan H. Wage added a comment - Hmm, why is the autoloading of sfGuardUser failing? I don't understand that part. If it is failing do you get a cannot load class error?

People

  • Assignee:
    Jonathan H. Wage
    Reporter:
    Ian P. Christian
Vote (3)
Watch (6)

Dates

  • Created:
    Updated: