Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hydratation error when two classes with the same parent class both define the same property with different types. #7505

Closed
m-r-r opened this issue Dec 6, 2018 · 8 comments
Labels
Milestone

Comments

@m-r-r
Copy link
Contributor

m-r-r commented Dec 6, 2018

Bug Report

Q A
BC Break no
Version 2.6.3

Summary

SimpleObjectHydrator produces a type error when two classes with the same parent class both define a property with the same name and different types.

I think this may be related to issue #5989.

Current behavior

When two classes inherit from the same parent using SINGLE_TABLE inheritance, and both classes define the same property with different types, SimpleObjectHydrator produce a type error if one of the properties is of type simple_array.

How to reproduce

Create an abstract class with two child classes:

use Doctrine\ORM\Annotation as ORM;

/**
 * @ORM\Entity()
 * @ORM\Table(name="test_responses")
 * @ORM\InheritanceType("SINGLE_TABLE")
 * @ORM\DiscriminatorColumn(name="discr", type="string")
 * @ORM\DiscriminatorMap({
 *     "array"       = ArrayResponse::class,
 *     "text"        = TextResponse::class,
 * })
 */
abstract class AbstractResponse {
    /**
     * @ORM\Id @ORM\GeneratedValue
     * @ORM\Column(type="integer")
     */
    public $id;
}

Create two child classes. Each class defines the same property with different types.
The first class use the type simple_array and the second class has a string property :

/**
 * @ORM\Entity()
 */
class ArrayResponse extends AbstractResponse {
    /**
     * @ORM\Column(name="value_array", type="simple_array")
     *
     * @var array
     */
    public $value = [];
}

/**
 * @ORM\Entity()
 */
class TextResponse extends AbstractResponse {
    /**
     * @ORM\Column(name="value_string", type="string")
     *
     * @var string|null
     */
    public $value = null;
}

Create an instance of the second class and flush the EntityManager:

$textResponse = new TextResponse();
$em->persist($textResponse);
$em->flush();

Clear the EntityManager and fetch the instance:

$em->clear();

/** @var TextResponse $arrayResponse */
$textResponse = $em->getRepository(AbstractResponse::class)
                   ->find($textResponse->id);

var_dump($textResponse->value); // Display an empty array (type error)

Expected behavior

The value of $textResponse->value should be NULL, because the property $value of class TextResponse has type string|null.

@m-r-r
Copy link
Contributor Author

m-r-r commented Dec 6, 2018

I'm creating a pull request with a failing test case.

@Ocramius
Copy link
Member

Ocramius commented Dec 6, 2018

This should be a validation error: these columns MUST have different names.

@m-r-r
Copy link
Contributor Author

m-r-r commented Dec 6, 2018

@Ocramius Thanks for answering so quickly and noticing that: I made an error when writing the issue and the names of the columns are missing in the annotation.

The behavior is the same when the columns have different names, I updated the code in the issue to fix the column names.

@m-r-r m-r-r closed this as completed Dec 6, 2018
@m-r-r m-r-r reopened this Dec 6, 2018
@Ocramius
Copy link
Member

Ocramius commented Dec 6, 2018

@m-r-r there are two issues then:

  1. two ReflectionProperty with same #getName(), same #getDeclaringClass()->getParentClass(), but same #getName() lead to incorrect hydration
  2. two ReflectionProperty with same #getName(), same #getDeclaringClass()->getParentClass(), but same #getName() and mapped with same @Column(name=) should lead to the SchemaValidator rejecting them.

@m-r-r
Copy link
Contributor Author

m-r-r commented Dec 7, 2018

@Ocramius Ok ! Should I open a second issue for the validation problem ?

@Ocramius
Copy link
Member

Ocramius commented Dec 7, 2018

@m-r-r yes please, or maybe just create a failing test case involving the schema validator 👍

@lcobucci lcobucci added this to the 2.7.1 milestone Nov 15, 2019
@lcobucci lcobucci added the Bug label Nov 15, 2019
@beberlei
Copy link
Member

beberlei commented Jan 8, 2020

@Ocramius the hydrator code has multiple paths where it has comments mentioning the handling of duplicate field names across the hierachy, so it is actually supported to do this right now.

beberlei added a commit to beberlei/doctrine2 that referenced this issue Jan 15, 2020
beberlei added a commit that referenced this issue Jan 15, 2020
[GH-7505] Bug in SimpleObjectHydrator when using inheritance with same field
@beberlei
Copy link
Member

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants