[DDC-2494] Proxy getId not invoke convertToPHPValue on custom type Created: 08/Jun/13  Updated: 14/Jul/14  Resolved: 12/Jun/13

Status: Resolved
Project: Doctrine 2 - ORM
Component/s: None
Affects Version/s: None
Fix Version/s: 2.4
Security Level: All

Type: Bug Priority: Critical
Reporter: Eduardo Oliveira Assignee: Fabio B. Silva
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Reference
is referenced by DDC-3192 Custom types do not get converted to ... Resolved

 Description   

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.



 Comments   
Comment by Fabio B. Silva [ 10/Jun/13 ]

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.

Comment by Eduardo Oliveira [ 10/Jun/13 ]

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.

Comment by Marco Pivetta [ 10/Jun/13 ]

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.

Comment by Eduardo Oliveira [ 10/Jun/13 ]

"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.

Comment by Fabio B. Silva [ 11/Jun/13 ]

A possible solution : https://github.com/doctrine/doctrine2/pull/690

Comment by Fabio B. Silva [ 12/Jun/13 ]

Fixed : https://github.com/doctrine/doctrine2/commit/6937061b23ec4de63081efac800415e09dcbcb4f

Generated at Thu Oct 23 09:35:34 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.