Doctrine 1
  1. Doctrine 1
  2. DC-740

issue with multiple connection handling

    Details

    • Type: Bug Bug
    • Status: 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. doctrine_core.patch
        0.7 kB
        Marcel Berteler
      2. doctrine_manager.patch
        2 kB
        Marcel Berteler

        Activity

        Hide
        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?
        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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); }

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            Ian P. Christian
          • Votes:
            3 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated: