DDC-2494: Proxy getId not invoke convertToPHPValue on custom type #3214

Closed
doctrinebot opened this Issue Jun 8, 2013 · 8 comments

1 participant

@doctrinebot

Jira issue originally created by user entering:

I have a custom type tinyint:
https://gist.github.com/entering/3503d7458e5fbe2f6e02

I was using it a lot and when I updated to doctrine 2.4 beta it break some stuff. At the time i turn all on smallint and solve the problem, now I had time to look into it.

Example, entity Currency:

    /****
     * @var integer
     *
     * @ORM\Column(name="id", type="tinyint", nullable=false, options={"unsigned": true})
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    protected $id;

    /****
     * @ORM\Column(name="temp", type="tinyint", nullable=false)
     */
    protected $temp;

Entity Campaign:

    /****
     * @var Currency
     *
     * @ORM\ManyToOne(targetEntity="Currency", inversedBy="campaigns")
     * @ORM\JoinColumns({
     *   @ORM\JoinColumn(name="currency_id", referencedColumnName="id", nullable=false)
     * })
     */
    protected $currency;

When i have this piece of code:

var_dump($campaign->getCurrency()->getId());
var_dump($campaign->getCurrency()->getTemp());

I get:
string(1) "1"
int(5)

If I turn id into small int:
int(1)
int(5)

If I switch the order to:

var_dump($campaign->getCurrency()->getTemp());
var_dump($campaign->getCurrency()->getId());

As expected:
int(5)
int(1)

If I place a var_dump on convertToPHPValue I can see that is not being called on getId when getId is lazy.

This looks like a bug introduced when getId started being lazy to save queries.

@doctrinebot

Comment created by @FabioBatSilva:

Before DCOM-96 we ignore custom type fields while looking for identifier getters.
It mean that, when an identifier getter is called the entity will be loaded from database, even though the identifier value is already known.
Then the Type#convertToPHPValue will be invoked..

After we move the proxy generation to common custom types are ignored and the database load its no longer triggered.

@doctrinebot

Comment created by entering:

I understand the problem, because I was aware that getId() now doesn't load entity from DB, what is great in performance, I was expecting this for a while.

For me is really bad I cannot have ID's as tinyint (custom type), i suppose Doctrine doesn't support tinyint because is not present in all DB's, but some DB's like MySQL support it, I agree that Doctrine doesn't need to support all types out the box, but custom types should work exactly like native types, shouldn't be second class citizens.

If this is a limitation that is difficult to overpass and there are no plans to "fix" it, should be listed here: http://docs.doctrine-project.org/en/latest/reference/limitations-and-known-issues.html

And should be listed as BC in here: https://github.com/doctrine/doctrine2/blob/master/UPGRADE.md

If there are plans to fix this problem and is just a case of someone put hands on the code I can give it a shot even I'm not completely familiar with Doctrine code.

@doctrinebot

Comment created by @ocramius:

I don't think the bug is related with lazy loading. Identifiers are never hydrated into proxies anyway.

What the problem here seems to be is that the type conversion is not applied to meta columns.

You can check and see if there's code about type conversion in meta columns.

@doctrinebot

Comment created by entering:

"What the problem here seems to be is that the type conversion is not applied to meta columns."

Marco I'm not sure If I follow you.

So when ID of entity Currency is a smallint a var_dump($campaign->getCurrency()) give this: https://gist.github.com/entering/5751908

  protected $id =>
  string(1) "1"

Is a string, the cast is done on getId()

Looking at the proxy created:

    /****
     * {@inheritDoc}
     */
    public function getId()
    {
        if ($this->*_isInitialized_* === false) {
            return (int)  parent::getId();
        }

        $this->*_initializer__ && $this->__initializer__->_*invoke($this, 'getId', array());

        return parent::getId();

The cast is written here: https://github.com/doctrine/doctrine2/blob/2.3/lib/Doctrine/ORM/Proxy/ProxyFactory.php#L259

Just to be sure I placed a var_dump inside convertToPHPValue of SmallIntType and on getId() is not called, but if I force the load of entity from DB (code below) converToPHPValue is called.

var_dump($campaign->getCurrency()->getCode());
var_dump($campaign->getCurrency()->getId());

So the problem here is that convertToPHPValue is never called on getId() on proxy when entity is not initialized, the problem is masked with the cast "written on hand" inside the getId().

The way I see it the getId() on proxy should all the time call convertToPHPValue, that way it would be correct with all types (native and custom).

The proxy with tinyint:

    /****
     * {@inheritDoc}
     */
    public function getId()
    {
        if ($this->*_isInitialized_* === false) {
            return  parent::getId();
        }


        $this->*_initializer__ && $this->__initializer__->_*invoke($this, 'getId', array());

        return parent::getId();
    }

Before custom tinyint was working on Identifiers because getId() would need to load entity from DB, the entity would be hydrate and the convertToPHPValue called at that time, now getId() doesn't load entity from DB so is never called.

To me a cast (int) on proxy that is decided according the name type is a ugly hack, inside if ($this->isInitialized === false) it should call all the time convertToPHPValue.

@doctrinebot

Comment created by @FabioBatSilva:

A possible solution : #690

@doctrinebot

Comment created by @FabioBatSilva:

Fixed : 6937061

@doctrinebot

Issue was closed with resolution "Fixed"

@doctrinebot doctrinebot added this to the 2.4 milestone Dec 6, 2015
@doctrinebot doctrinebot closed this Dec 6, 2015
@doctrinebot doctrinebot added the Bug label Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment