doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.9k stars 2.5k forks source link

BasicEntityPersister confuses association types with PDO datatypes #7062

Closed mariusklocke closed 6 years ago

mariusklocke commented 6 years ago

Hey folks,

i think i found a bug when combining one-to-one associations with association-key identifiers. My application deals with soccer leagues and the issue should become visible with the following 3 related entities:

<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                  xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
                    http://raw.github.com/doctrine/doctrine2/master/doctrine-mapping.xsd">

    <entity name="HexagonalPlayground\Domain\Ranking" table="rankings">
        <id name="season" association-key="true" />
        <field name="updatedAt" column="updated_at" type="datetime_immutable" nullable="true" />
        <one-to-one field="season" target-entity="HexagonalPlayground\Domain\Season" inversed-by="ranking">
            <join-column name="season_id" referenced-column-name="id" />
        </one-to-one>
        <one-to-many target-entity="HexagonalPlayground\Domain\RankingPosition" mapped-by="ranking" field="positions" index-by="team_id" orphan-removal="true">
            <cascade>
                <cascade-all/>
            </cascade>
        </one-to-many>
    </entity>
</doctrine-mapping>
<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                  xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
                    http://raw.github.com/doctrine/doctrine2/master/doctrine-mapping.xsd">

    <entity name="HexagonalPlayground\Domain\RankingPosition" table="ranking_positions">
        <indexes>
            <index columns="sort_index" />
        </indexes>
        <id name="ranking" association-key="true" />
        <id name="team" association-key="true" />
        <field name="sortIndex" type="integer" column="sort_index" />
        <field name="number" type="integer" />
        <field name="matches" type="integer" />
        <field name="wins" type="integer" />
        <field name="draws" type="integer" />
        <field name="losses" type="integer" />
        <field name="scoredGoals" type="integer" column="scored_goals" />
        <field name="concededGoals" type="integer" column="conceded_goals" />
        <field name="points" type="integer" />
        <many-to-one field="ranking" target-entity="HexagonalPlayground\Domain\Ranking">
            <join-column name="season_id" referenced-column-name="season_id" nullable="false" />
        </many-to-one>
        <many-to-one field="team" target-entity="HexagonalPlayground\Domain\Team">
            <join-column name="team_id" referenced-column-name="id" nullable="false" />
        </many-to-one>
    </entity>
</doctrine-mapping>
<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                  xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
                    http://raw.github.com/doctrine/doctrine2/master/doctrine-mapping.xsd">

    <entity name="HexagonalPlayground\Domain\Season" table="seasons">
        <id name="id" type="string" />
        <field name="name" type="string" />
        <field name="state" type="string" />
        <one-to-one field="ranking" target-entity="HexagonalPlayground\Domain\Ranking" mapped-by="season" orphan-removal="true">
            <cascade>
                <cascade-all/>
            </cascade>
        </one-to-one>
        <one-to-many field="matches" target-entity="HexagonalPlayground\Domain\Match" mapped-by="season" orphan-removal="true">
            <order-by>
                <order-by-field name="matchDay" direction="ASC" />
            </order-by>
        </one-to-many>
        <many-to-many target-entity="HexagonalPlayground\Domain\Team" field="teams">
            <join-table name="seasons_teams_link">
                <join-columns>
                    <join-column name="season_id" referenced-column-name="id" />
                </join-columns>
                <inverse-join-columns>
                    <join-column name="team_id" referenced-column-name="id"/>
                </inverse-join-columns>
            </join-table>
        </many-to-many>
    </entity>
</doctrine-mapping>

The issue arises when changes to RankingPosition need to be persisted. Updating this entity does not work, because the resulting query sends a wrong parameter type for the season_id

[2018-02-13 20:53:45] logger.INFO: Executing query UPDATE ranking_positions SET sort_index = ?, number = ? WHERE season_id = ? AND team_id = ?
[2018-02-13 20:53:45] logger.INFO: With parameter values array (   0 => 7,   1 => 2,   2 => '481185d3-9d35-428b-9900-c073238eb9dc',   3 => 'e0b23bd3-3a6d-4a15-b601-6de8f2fc43df', )
[2018-02-13 20:53:45] logger.INFO: With parameter types array (   0 => 'integer',   1 => 'integer',   2 => 1,   3 => 'string', )
[2018-02-13 20:53:45] logger.INFO: Finished query after 0.001 seconds

Doctrine determines 1 as parameter type (corresponds to PDO::PARAM_INT) for the season_id, but it should be string.

I did a little debugging and i think i found an implementation flaw in Doctrine\ORM\Persisters\Entity\BasicEntityPersister::updateTable(). The method tries to determine the parameter type for each identifier column, by using the type field in the target entities associationMappings. This fields value does NOT represent one of the PDO::PARAM_* constants. I think this field reflects the association type and the value 1 here represents the constant Doctrine\ORM\Mapping\ClassMetadataInfo::ONE_TO_ONE.

Can anybody closer to the project confirm my assumptions?

You can find my application here on Github: https://github.com/mariusklocke/liga-manager-api

Ocramius commented 6 years ago

@mariusklocke can this be reproduced in isolation (see https://github.com/doctrine/doctrine2/tree/master/tests/Doctrine/Tests/ORM/Functional/Ticket for examples)? Is this affecting latest ORM?

mariusklocke commented 6 years ago

I'll try to provide a Testcase. By "latest" you mean master branch? It's definitely happing on 2.6.0

Are there any conventions i should follow for naming my branch?

Ocramius commented 6 years ago

@mariusklocke no convention - feel free to pick anything. We'll adjust the patch anyway once we have a clue of what is going on in an isolated scenario. My suggestion is to name the test DDC7062Test.

mariusklocke commented 6 years ago

@Ocramius Hey, i created a PR with a failing test, but i am unsure whether i got the branches right. I created my branch from 2.6 branch, but the contribution guidelines say all PRs have to go to master? That seems weird to me, because i thought bugfixes for 2.6 should go into 2.6 branch? Can you help me out here?

Ocramius commented 6 years ago

@mariusklocke send it against 2.6 for now - we'll figure it out :) You can just edit the open pull request

Ocramius commented 6 years ago

Handled in #7082