Doctrine 1
  1. Doctrine 1
  2. DC-373

Relating to Translation table (generated by Doctrine_I18n) doesn't work correctly after resetting connection

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.2.1, 1.2.2, 1.2.3
    • Fix Version/s: None
    • Component/s: Connection, I18n
    • Labels:
      None
    • Environment:
      PHP 5.2.11 (with Suhosin-Patch 0.9.7)
      PHP 5.3.6

      Description

      Relating to Translation table (generated by Doctrine_I18n) doesn't work correctly after resetting connection.

      First, I defined the following schema.yml::

       
        Example:
          actAs:
            I18n:
              fields: [title]
          columns:
              title: string(128)
      

      Next, I prepared the following fixture file::

       
        Example:
          Example_1:
            Translation:
              en:
                title: "Title"
              ja:
                title: "題名"
      

      Next, I run "build-all-reload" task. The table was created and data was loaded.

      And I write the following code::

       
        Doctrine_Core::loadModels('models');
        $example = Doctrine_Core::getTable('Example')->find(1);
        
        var_dump(
          $example->Translation['en']->title,
          $example->Translation['ja']->title
        );
        
        Doctrine_Manager::resetInstance();
        Doctrine_Manager::getInstance()->openConnection(DSN, 'doctrine');
        
        $example2 = Doctrine_Core::getTable('Example')->find(1);
        var_dump(
          $example2->Translation['en']->title,
          $example2->Translation['ja']->title
        );
      

      I try executing my code, but I got the following error::

        string(5) "Title"
        string(6) "題名"
        
        Doctrine_Connection_Mysql_Exception: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'e.title' in 'field list' in /path/to/doctrine/Doctrine/Connection.php on line 1082
        
        Call Stack:
            0.0055      65352   1. {main}() /path/to/my/code/test.php:0
            0.2776    5873388   2. Doctrine_Table->find() /path/to/my/code/test.php:52
            0.2793    5881152   3. Doctrine_Query->fetchOne() /path/to/doctrine/Doctrine/Table.php:1611
            0.2793    5881296   4. Doctrine_Query_Abstract->execute() /path/to/doctrine/Doctrine/Query.php:281
            0.2793    5881892   5. Doctrine_Query_Abstract->_execute() /path/to/doctrine/Doctrine/Query/Abstract.php:1026
            0.3127    5890712   6. Doctrine_Connection->execute() /path/to/doctrine/Doctrine/Query/Abstract.php:976
            0.3164    5892632   7. Doctrine_Connection_Statement->execute() /path/to/doctrine/Doctrine/Connection.php:1006
            0.3181    5907408   8. Doctrine_Connection->rethrowException() /path/to/doctrine/Doctrine/Connection/Statement.php:269
      

        Activity

        Hide
        Arnaud Charlier added a comment -

        Good Morning,

        First, thanks a lot for your work on Doctrine. It's a real great tool we love to use.

        Currently in one of our big application we have exactly the same issue.
        It seems when we close the connection, the reference with the objectTranslation is lost.
        Re-open the connection seems not able to reinstantiate this Translation link to the objectTranslation.

        Currently we have no solution, but we are still investigating this.

        I'm available to go deep in the code with you, but any Doctrine Team help will be really nice.

        Thanks to let me know as soon as possible.

        Arnaud

        Show
        Arnaud Charlier added a comment - Good Morning, First, thanks a lot for your work on Doctrine. It's a real great tool we love to use. Currently in one of our big application we have exactly the same issue. It seems when we close the connection, the reference with the objectTranslation is lost. Re-open the connection seems not able to reinstantiate this Translation link to the objectTranslation. Currently we have no solution, but we are still investigating this. I'm available to go deep in the code with you, but any Doctrine Team help will be really nice. Thanks to let me know as soon as possible. Arnaud
        Hide
        Joe Siponen added a comment - - edited

        I've just now battled with the very same problem in Doctrine 1.2 (the version bundled with symfony 1.4) and the problem seems to be caused by the fact that Doctrine_Record_Generator simply isn't written such that it is able to reinitialize generators for unloaded table instances after a connection is closed. This problem also manifests itself after a table has been loaded in a connection and one tries retrieve a table again after Doctrine_Connection->evictTables() has been called. This makes it impossible to to open more than one connection at a time in a request/script when using behaviors that dynamically modify table instances (such as the i18n behavior).

        Doctrine_Record_Generator determines if it needs to run its initialization methods simply by checking if the to-be generated class, as defined by the className option, exists using a class_exists call. This means that the first time this method is called the initialization happens but for every subsequent call no initialization is made. Now, in the i18m behavior, the important initialization happens in its setTableDefinition method in which it removes any of the translated fields from the table instance that is been setup and redefines them as relations on the to-be-created Translation class. It then finishes off by dynamically declaring the new class for the translation record using its generateClassFromTable method.

        Thus, the first time everything goes smoothly and the i18n generator's setTableDefinition is called and the table instance is properly initialized. Everything will now work as expected while the current connection is open since the connection instance keeps the i18n modified table instances alive and well for callers.

        But, when the current connection is closed the i18n modified table instances it holds are also removed (goes out of scope). Then, when a new connection is opened, this new connection will start without having any table instances. This means that the next time one asks the new connection for a table instance of the same class with the i18n behavior the i18n behaviors will fail to initialize because the generator at this time believes its class has actually been initialized which, in turn, means that the table using the i18n behavior isn't properly initialized. No initialization means that this table will now include the non-existant i18n fields in the select part of its queries (those are in the translation table) causing those queries to fail miserably.

        I believe this could be fixed by adding a static attribute to Doctrine_Record_Generator that tracks the spl_object_hash of the underlying dbh instance variable of the doctrine connection of the table parameter. If the hash is the same the next time that the initialize method is called the generator can decide not to reinitialize itself but if it detects that the hash of the current connection is different then that is definitely a clue to the generator that it needs to reinitialize itself (i.e. run all of the initialization methods but generateClassFromTable which should't be called more than once).

        Maybe do it like this perhaps:

         
        abstract class Doctrine_Record_Generator extends Doctrine_Record_Abstract
        {
          public function initialize(Doctrine_Table $table)
          {
            /* ... */ 
          
            $currentConnectionHash = spl_object_hash($table->getConnection()->getDbh());
            
            //Next part is called if this is the first connection made or if this is a new open connection with new table instances
            if ($currentConnectionHash != self::$lastConnectionHash)
            {
              self::$lastConnectionHash = $currentConnectionHash;
              
              $this->buildTable();
        
              $fk = $this->buildForeignKeys($this->_options['table']);
        
              $this->_table->setColumns($fk);
        
              $this->buildRelation();
        
              $this->setTableDefinition();
              $this->setUp();
              
              if ($this->_options['generateFiles'] === false && class_exists($this->_options['className'])) {
                $this->generateClassFromTable($this->_table); //Don't generate the class more than once ever
              }
              
              $this->buildChildDefinitions();
        
              $this->_table->initIdentifier();
            }
          }
        }
        
        Show
        Joe Siponen added a comment - - edited I've just now battled with the very same problem in Doctrine 1.2 (the version bundled with symfony 1.4) and the problem seems to be caused by the fact that Doctrine_Record_Generator simply isn't written such that it is able to reinitialize generators for unloaded table instances after a connection is closed. This problem also manifests itself after a table has been loaded in a connection and one tries retrieve a table again after Doctrine_Connection->evictTables() has been called. This makes it impossible to to open more than one connection at a time in a request/script when using behaviors that dynamically modify table instances (such as the i18n behavior). Doctrine_Record_Generator determines if it needs to run its initialization methods simply by checking if the to-be generated class, as defined by the className option, exists using a class_exists call. This means that the first time this method is called the initialization happens but for every subsequent call no initialization is made. Now, in the i18m behavior, the important initialization happens in its setTableDefinition method in which it removes any of the translated fields from the table instance that is been setup and redefines them as relations on the to-be-created Translation class. It then finishes off by dynamically declaring the new class for the translation record using its generateClassFromTable method. Thus, the first time everything goes smoothly and the i18n generator's setTableDefinition is called and the table instance is properly initialized. Everything will now work as expected while the current connection is open since the connection instance keeps the i18n modified table instances alive and well for callers. But, when the current connection is closed the i18n modified table instances it holds are also removed (goes out of scope). Then, when a new connection is opened, this new connection will start without having any table instances. This means that the next time one asks the new connection for a table instance of the same class with the i18n behavior the i18n behaviors will fail to initialize because the generator at this time believes its class has actually been initialized which, in turn, means that the table using the i18n behavior isn't properly initialized. No initialization means that this table will now include the non-existant i18n fields in the select part of its queries (those are in the translation table) causing those queries to fail miserably. I believe this could be fixed by adding a static attribute to Doctrine_Record_Generator that tracks the spl_object_hash of the underlying dbh instance variable of the doctrine connection of the table parameter. If the hash is the same the next time that the initialize method is called the generator can decide not to reinitialize itself but if it detects that the hash of the current connection is different then that is definitely a clue to the generator that it needs to reinitialize itself (i.e. run all of the initialization methods but generateClassFromTable which should't be called more than once). Maybe do it like this perhaps: abstract class Doctrine_Record_Generator extends Doctrine_Record_Abstract { public function initialize(Doctrine_Table $table) { /* ... */ $currentConnectionHash = spl_object_hash($table->getConnection()->getDbh()); //Next part is called if this is the first connection made or if this is a new open connection with new table instances if ($currentConnectionHash != self::$lastConnectionHash) { self::$lastConnectionHash = $currentConnectionHash; $this->buildTable(); $fk = $this->buildForeignKeys($this->_options['table']); $this->_table->setColumns($fk); $this->buildRelation(); $this->setTableDefinition(); $this->setUp(); if ($this->_options['generateFiles'] === false && class_exists($this->_options['className'])) { $this->generateClassFromTable($this->_table); //Don't generate the class more than once ever } $this->buildChildDefinitions(); $this->_table->initIdentifier(); } } }
        Hide
        Joe Siponen added a comment -

        Failing test case attached

        Show
        Joe Siponen added a comment - Failing test case attached
        Hide
        Kousuke Ebihara added a comment -

        Good job, Joe Siponen! Thanks for your nice patch!

        I can't understand why the Doctrine team discontinued maintenance of Doctrine 1 without any fixes for some serious issues like this problem...

        Joe, would you send your patch by pull-request in GitHub? They might notice this problem and try to fix in official repository by your request. If you cannnot, I will do it as your proxy. (Of course I'm going to describe your credit)

        Show
        Kousuke Ebihara added a comment - Good job, Joe Siponen! Thanks for your nice patch! I can't understand why the Doctrine team discontinued maintenance of Doctrine 1 without any fixes for some serious issues like this problem... Joe, would you send your patch by pull-request in GitHub? They might notice this problem and try to fix in official repository by your request. If you cannnot, I will do it as your proxy. (Of course I'm going to describe your credit)
        Hide
        Kousuke Ebihara added a comment -

        I've tested Joe Siponen's patch, it would be good in a situation of single connection, but it doesn't work in multiple connections.

        So I modified your patch to fit my needs. I'm testing my arranged patch and I want to publish it ASAP...

        Show
        Kousuke Ebihara added a comment - I've tested Joe Siponen's patch, it would be good in a situation of single connection, but it doesn't work in multiple connections. So I modified your patch to fit my needs. I'm testing my arranged patch and I want to publish it ASAP...
        Hide
        Baptiste SIMON - Libre Informatique added a comment - - edited

        Hi guys,

        I found a workaround for this bug on multiple DB connections :
        In my *-schema.yml I have defined my i18n table as :

        (...)
        actAs:
        I18n:
        fields: [xxx, yyy, zzz]
        generateFiles: true
        generatePath: <?php echo sfConfig::get('sf_lib_dir'); ?>/model/doctrine/i18n
        (...)

        (notice the generateFiles & generatePath...)

        This (alone) seems to solve my issue. I'll give more feed back if there is any.
        FEEDBACK: do not work with inheritated behaviors... so it's not correct for our needs.

        sources: http://forum.symfony-project.org/viewtopic.php?t=24071

        Show
        Baptiste SIMON - Libre Informatique added a comment - - edited Hi guys, I found a workaround for this bug on multiple DB connections : In my *-schema.yml I have defined my i18n table as : (...) actAs: I18n: fields: [xxx, yyy, zzz] generateFiles: true generatePath: <?php echo sfConfig::get('sf_lib_dir'); ?>/model/doctrine/i18n (...) (notice the generateFiles & generatePath...) This (alone) seems to solve my issue. I'll give more feed back if there is any. FEEDBACK: do not work with inheritated behaviors... so it's not correct for our needs. sources: http://forum.symfony-project.org/viewtopic.php?t=24071
        Hide
        Baptiste SIMON - Libre Informatique added a comment -

        eventually, for a portable solution (web), you can use this path :

        (...)
        generatePath: ../<?php echo str_replace(sfConfig::get('sf_root_dir').'/', '', sfConfig::get('sf_lib_dir')) ?>/model/doctrine/i18n
        (...)

        this is a trick for file replication between servers that have different DOCUMENT_ROOT path.

        Show
        Baptiste SIMON - Libre Informatique added a comment - eventually, for a portable solution (web), you can use this path : (...) generatePath: ../<?php echo str_replace(sfConfig::get('sf_root_dir').'/', '', sfConfig::get('sf_lib_dir')) ?>/model/doctrine/i18n (...) this is a trick for file replication between servers that have different DOCUMENT_ROOT path.
        Hide
        Baptiste SIMON - Libre Informatique added a comment - - edited

        ok sorry for the multiple posts... I maybe have found a smart solution by patching Doctrine/Record/Generator.php that way :

        Index: Doctrine/Record/Generator.php
        ===================================================================
        — Doctrine/Record/Generator.php (révision 4690)
        +++ Doctrine/Record/Generator.php (copie de travail)
        @@ -163,7 +163,7 @@
        // check that class doesn't exist (otherwise we cannot create it)
        if ($this->_options['generateFiles'] === false && class_exists($this->_options['className']))

        { $this->_table = Doctrine_Core::getTable($this->_options['className']); - return false; + //return false; }

        $this->buildTable();
        @@ -461,10 +461,10 @@
        } else

        { throw new Doctrine_Record_Exception('If you wish to generate files then you must specify the path to generate the files in.'); }
        • } else { + }

          elseif ( !class_exists($definition['className']) )

          { $def = $builder->buildDefinition($definition); eval($def); }

          }

        This implicates multiple rebuilding of the same class, but at least, it works... without any modifications in the schema.

        Show
        Baptiste SIMON - Libre Informatique added a comment - - edited ok sorry for the multiple posts... I maybe have found a smart solution by patching Doctrine/Record/Generator.php that way : Index: Doctrine/Record/Generator.php =================================================================== — Doctrine/Record/Generator.php (révision 4690) +++ Doctrine/Record/Generator.php (copie de travail) @@ -163,7 +163,7 @@ // check that class doesn't exist (otherwise we cannot create it) if ($this->_options ['generateFiles'] === false && class_exists($this->_options ['className'] )) { $this->_table = Doctrine_Core::getTable($this->_options['className']); - return false; + //return false; } $this->buildTable(); @@ -461,10 +461,10 @@ } else { throw new Doctrine_Record_Exception('If you wish to generate files then you must specify the path to generate the files in.'); } } else { + } elseif ( !class_exists($definition ['className'] ) ) { $def = $builder->buildDefinition($definition); eval($def); } } This implicates multiple rebuilding of the same class, but at least, it works... without any modifications in the schema.

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            Kousuke Ebihara
          • Votes:
            2 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated: