[DC-740] issue with multiple connection handling Created: 16/Jun/10  Updated: 16/Nov/10

Status: Open
Project: Doctrine 1
Component/s: Connection
Affects Version/s: 1.2.2
Fix Version/s: 1.2.2

Type: Bug Priority: Major
Reporter: Ian P. Christian Assignee: Jonathan H. Wage
Resolution: Unresolved Votes: 3
Labels: None

Attachments: Text File doctrine_core.patch     Text File doctrine_manager.patch    
Sub-Tasks:
Key
Summary
Type
Status
Assignee
DC-618 [PATCH] Local key relations without m... Sub-task Open Jonathan H. Wage  

 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.



 Comments   
Comment by Jonathan H. Wage [ 16/Jun/10 ]

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?

Comment by Ian P. Christian [ 16/Jun/10 ]

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);
Comment by Ian P. Christian [ 16/Jun/10 ]

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.

Comment by Ian P. Christian [ 16/Jun/10 ]

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.

Comment by Ian P. Christian [ 16/Jun/10 ]

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.

Comment by Marcel Berteler [ 06/Jul/10 ]

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);
  }
  
}
Comment by Marcel Berteler [ 06/Jul/10 ]

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);
Comment by Marcel Berteler [ 06/Jul/10 ]

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');
Comment by Marcel Berteler [ 07/Jul/10 ]

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.

Comment by Ian P. Christian [ 08/Jul/10 ]

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.

Comment by Marcel Berteler [ 27/Sep/10 ]

second required patch to make Symfony work with 2 concurrent databases

Comment by Eugeniy Belyaev [ 21/Oct/10 ]
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);
    }
  }
Comment by Dean de Bree [ 16/Nov/10 ]

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);
    }
Generated at Thu Dec 18 19:37:15 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.