Doctrine 1
  1. Doctrine 1
  2. DC-755

CLONE [DC-558] incorrect handling of MODEL_CLASS_PREFIX causes Doctrine_Migration_Diff to drop the whole database when working from YAML (Regression)

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: 1.2.3
    • Fix Version/s: None
    • Component/s: Migrations
    • Labels:
      None
    • Environment:
      Current HEAD of Doctrine 1.2

      Description

      Replicating the bug:
      1. Set ATTR_MODEL_CLASS_PREFIX non-null
      2. create schema file with entity
      3. run doctrine build-all
      4. copy schema file
      5. edit schema file to add column to entity
      6. run generate-migrations-diff from copy of schema file to edited schema file

      Expected (previous) behaviour:
      Migration is generated to add the new column to entity

      Real behaviour:
      Drops entity from database and creates new

      1. dc755TestCase.diff
        3 kB
        Andrew Coulton
      2. fix755.diff
        0.6 kB
        Andrew Coulton

        Activity

        Andrew Coulton created issue -
        Hide
        Andrew Coulton added a comment -

        This seems to be a regression caused by the fix for DC-558 which added the MODEL_CLASS_PREFIX to the $_toPrefix in Doctrine_Migration_Diff::generateChanges.

        While this fixed the issue when generating diff from models to YAML, it has now created the reverse issue for generating diffs from YAML to YAML - as the models generated for the "from" schema do not get MODEL_CLASS_PREFIX prepended and so now this command will drop all existing tables and recreate.

        I believe the fix may be to amend as:

        Unable to find source-code formatter for language: php. Available languages are: actionscript, html, java, javascript, none, sql, xhtml, xml
                $from = $this->_generateModels(
                    Doctrine_Manager::getInstance()->getAttribute(Doctrine_Core::ATTR_MODEL_CLASS_PREFIX) . self::$_fromPrefix,
                    $this->_from);
                $to = $this->_generateModels(
                    Doctrine_Manager::getInstance()->getAttribute(Doctrine_Core::ATTR_MODEL_CLASS_PREFIX) . self::$_toPrefix,
                    $this->_to                
                );
        

        Since it seems that when presented with a folder of models _generateModels ignores the prefix anyway. However, I'm not sure of other impacts possible as a result?

        Show
        Andrew Coulton added a comment - This seems to be a regression caused by the fix for DC-558 which added the MODEL_CLASS_PREFIX to the $_toPrefix in Doctrine_Migration_Diff::generateChanges. While this fixed the issue when generating diff from models to YAML, it has now created the reverse issue for generating diffs from YAML to YAML - as the models generated for the "from" schema do not get MODEL_CLASS_PREFIX prepended and so now this command will drop all existing tables and recreate. I believe the fix may be to amend as: Unable to find source-code formatter for language: php. Available languages are: actionscript, html, java, javascript, none, sql, xhtml, xml $from = $ this ->_generateModels( Doctrine_Manager::getInstance()->getAttribute(Doctrine_Core::ATTR_MODEL_CLASS_PREFIX) . self::$_fromPrefix, $ this ->_from); $to = $ this ->_generateModels( Doctrine_Manager::getInstance()->getAttribute(Doctrine_Core::ATTR_MODEL_CLASS_PREFIX) . self::$_toPrefix, $ this ->_to ); Since it seems that when presented with a folder of models _generateModels ignores the prefix anyway. However, I'm not sure of other impacts possible as a result?
        Andrew Coulton made changes -
        Field Original Value New Value
        Summary CLONE [DC-558] incorrect handling of MODEL_CLASS_PREFIX causes Doctrine_Migration_Diff to drop the whole database when working from YAML CLONE [DC-558] incorrect handling of MODEL_CLASS_PREFIX causes Doctrine_Migration_Diff to drop the whole database when working from YAML (Regression)
        Affects Version/s 1.2.3 [ 10051 ]
        Affects Version/s 1.2.0 [ 10043 ]
        Affects Version/s 1.2.1 [ 10044 ]
        Environment Vanilla sandbox on version 1.2.1 and 1.2.0
        both sqlite and mysql, so its not driver dependant
        Current HEAD of Doctrine 1.2
        Description Replicating the bug:
        1. unzip vanilla sandbox.
        2. create schema file with entity
        3. run ./doctrine build-all
        4. modify schema, add column to entity.
        5. run ./doctrine generate-migrations-diff

        Expected behaviour:
        generates migration wich adds column to entity.

        Real behaviour:
        Drops entity from database

        Similar issue:
        http://groups.google.com/group/doctrine-user/browse_thread/thread/8b4a0fc0778a388a/79446784623b8497?lnk=gst&q=1.2.1+migration
        Replicating the bug:
        1. Set ATTR_MODEL_CLASS_PREFIX non-null
        2. create schema file with entity
        3. run doctrine build-all
        4. copy schema file
        5. edit schema file to add column to entity
        6. run generate-migrations-diff from copy of schema file to edited schema file

        Expected (previous) behaviour:
        Migration is generated to add the new column to entity

        Real behaviour:
        Drops entity from database and creates new
        Priority Minor [ 4 ] Critical [ 2 ]
        Fix Version/s 1.2.3 [ 10051 ]
        Hide
        Andrew Coulton added a comment -

        I've been using and testing the modified version above locally for some time and it seems to work as expected. Any chance of this making it into core? Otherwise, the migrations feature is completely unusable when working YAML-YAML and using model prefixes on the newly released 1.2.3

        Show
        Andrew Coulton added a comment - I've been using and testing the modified version above locally for some time and it seems to work as expected. Any chance of this making it into core? Otherwise, the migrations feature is completely unusable when working YAML-YAML and using model prefixes on the newly released 1.2.3
        Hide
        Jonathan H. Wage added a comment -

        Has anyone been able to produce this in a test case?

        Show
        Jonathan H. Wage added a comment - Has anyone been able to produce this in a test case?
        Hide
        Andrew Coulton added a comment - - edited

        I've attached a diff file with the DC755TestCase and the required from and to YAML schema files to reproduce this bug. I wasn't sure whether you prefer like this or as a git commit?

        Show
        Andrew Coulton added a comment - - edited I've attached a diff file with the DC755TestCase and the required from and to YAML schema files to reproduce this bug. I wasn't sure whether you prefer like this or as a git commit?
        Andrew Coulton made changes -
        Attachment dc755TestCase.diff [ 10752 ]
        Hide
        Andrew Coulton added a comment -

        Also attached a diff file of my proposed change to Doctrine_Migration_Diff to resolve this, but as I say unsure if it has implications on other migration types.

        Show
        Andrew Coulton added a comment - Also attached a diff file of my proposed change to Doctrine_Migration_Diff to resolve this, but as I say unsure if it has implications on other migration types.
        Andrew Coulton made changes -
        Attachment fix755.diff [ 10753 ]
        Show
        Andrew Coulton added a comment - Fixed by http://github.com/acoulton/doctrine1/tree/DC-755

        This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

        • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={query=DC-755, expand=changesets[0:20].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            Andrew Coulton
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated: