DDC-31: ClassMetadata discriminatorColumn['name'] should always be set when discriminatorColumn is not null #3875

Closed
doctrinebot opened this Issue Oct 3, 2009 · 6 comments

2 participants

@doctrinebot

Jira issue originally created by user itoijala:

When creating an entity, StandardEntityPersister checks if the discriminatorColumn is null. If not it checks if discriminatorColumn['name'] is $column. Sometimes (not sure when), discriminatorColumn['name'] is not set. warning is then emitted:

Notice: Undefined index: name in D:\Projects\NewStein\library\Doctrine\ORM\Persisters\StandardEntityPersister.php on line 504

ClassMetadata should ensure that discriminatorColumn['name'] is always set.

Fix:

OLD (ClassMetadata.php, line 1567):

public function setDiscriminatorColumn($columnDef)
    {
        $this->discriminatorColumn = $columnDef;
        if ( ! isset($columnDef['fieldName'])) {
            $this->discriminatorColumn['fieldName'] = $columnDef['name'];
        }
    }

NEW:

public function setDiscriminatorColumn($columnDef)
    {
        $this->discriminatorColumn = $columnDef;
        if ( ! isset($columnDef['fieldName'])) {
            $this->discriminatorColumn['fieldName'] = $columnDef['name'];
        }
    if (!isset($columnDef['name']))
    {
        $this->discriminatorColumn['name'] = $this->discriminatorColumn['fieldName'];
    }
    }
@doctrinebot

Comment created by @guilhermeblanco:

In changeset http://trac.doctrine-project.org/changeset/6419 this issue is fixed.

Thanks for report and patch!

@doctrinebot

Comment created by @guilhermeblanco:

We didn't feel comfortable with this patch. I've spoken with Roman about it too.

I won't revert it, but I'd like to ask you more information, like your schema and piece of code that you got this error. I'll then wrap it in a test case for more investigation.

Reopened the issue, since solution is not optimal.

@doctrinebot

Comment created by itoijala:

OK.

First the schema (I suppose you mean the entity classes):

Blameable.php

namespace Model;

use \Model\User\User;
use \DateTime;

/****
 * @MappedSuperclass
 */
abstract class Blameable
{
    /****
     *
     * @var DateTime
     *
     * @Column(type="datetime", name="date_created")
     */
    protected $dateCreated;

    /****
     *
     * @var User
     *
     * @OneToOne(targetEntity="Model\User\User")
     * @JoinColumn(name="creator_id", referencedColumnName="id", nullable="true")
     */
    protected $creator;

    /****
     *
     * @var DateTime
     *
     * @Column(type="datetime", name="date_updated", nullable="true")
     */
    protected $dateUpdated;

    /****
     *
     * @var User
     *
     * @OneToOne(targetEntity="Model\User\User")
     * @JoinColumn(name="updater_id", referencedColumnName="id", nullable="true")
     */
    protected $updater;

    /****
     *
     * @var DateTime
     *
     * @Column(type="datetime", name="date_deleted", nullable="true")
     */
    protected $dateDeleted;

    /****
     *
     * @var User
     *
     * @OneToOne(targetEntity="Model\User\User")
     * @JoinColumn(name="deleter_id", referencedColumnName="id", nullable="true")
     */
    protected $deleter;

/** rest omitted (methods) **/
}

User.php

namespace Model\User;

use \Zend*Validate*Alnum;
use \Zend*Validate*StringLength;
use \Zend*Validate*Alpha;
use \Zend*Validate*EmailAddress;
use \Zend_Registry;
use \Model\Blameable;
use \Model\ConstraintException;
use \Itoijala_Singletons;
use \Zend_Auth;
use \Itoijala*Password*Hash;

/****
 *
 *
 * @Entity @Table(name="user_users")
 */
class User extends Blameable
{
    /****
     *
     * @var int
     *
     * @Id @Column(type="integer", name="id")
     * @GeneratedValue(strategy="AUTO")
     */
    private $id;

    /****
     *
     * @var string
     *
     * @Column(type="string", length="20", name="username")
     */
    private $username;

    /****
     *
     * @var string
     *
     * @Column(type="string", length="20", name="first_name")
     */
    private $firstName;

    /****
     *
     * @var string
     *
     * @Column(type="string", length="20", name="last_name")
     */
    private $lastName;

    /****
     *
     * @var string
     *
     * @Column(type="string", length="255", name="email")
     */
    private $email;

    /****
     *
     * @var string
     *
     * @Column(type="string", length="20", name="signature")
     */
    private $signature;

    /****
     *
     * @var string
     *
     * @Column(type="string", length="128", name="password")
     */
    private $password;

    /****
     *
     * @var string
     *
     * @Column(type="string", length="255", name="role")
     */
    private $role;

    /****
     *
     * @var string
     *
     * @Column(type="string", length="32", name="session_id", nullable="true")
     */
    private $sessionId;

    /****
     *
     * @var bool
     *
     * @Column(type="boolean", name="unlocked")
     */
    private $unlocked;
/** rest omitted **/
}

Then the controller action (I'm using the Zend Framework)

public function getAction()
    {
        if (!$this->_isAllowed('user', 'get'))
        {
            $this->_response->setHttpResponseCode(401);
            return;
        }
        $this->_setlayout();
        $this->_helper->viewRenderer->setNoRender(false);

        $user = Itoijala*Singletons::getDoctrine()->find('\Model\User\User', $this->*getParam('id'));
        if ($user === null)
        {
            $this->_response->setHttpResponseCode(404);
            return;
        }
        if ($user->isDeleted() && !$this->_isAllowed('user', 'getDeleted'))
        {
            $this->_response->setHttpResponseCode(401);
            return;
        }
        $this->view->user = $user;
    }

The view code:

<div id="content">
    <?php echo $this->user; ?> // User has a **toString() method
</div>

This outputs the _toString() and emits 3 of the above notices. By echoing the column in StandardEntityPersister I found out that the notices are caused by creator_id, updater_id and deleterid (these are the only associations).

The code in StandardEntityPersister expects the discriminatorColumn to be either null or have a name key.

StandardEntityPersister.php around line 504

foreach ($result as $column => $value) {
            $column = $this->_class->resultColumnNames[$column];
            if (isset($this->_class->fieldNames[$column])) {
                $fieldName = $this->_class->fieldNames[$column];
                $data[$fieldName] = Type::getType($this->_class->fieldMappings[$fieldName]['type'])
                        ->convertToPHPValue($value, $this->_platform);
            } else if ($this->*class->discriminatorColumn !== null && $column == $this->*class->discriminatorColumn['name']) { // outputs the notice
                $entityName = $this->_class->discriminatorMap[$value];
            } else {
                $joinColumnValues[$column] = $value;
            }
        }

Either discriminatorolumn has to have a name key if it is not null (the discriminatorColumn only had an empty fieldName key when print_r'd), or the above code has to be changed.

@doctrinebot

Comment created by romanb:

The question here is why discriminatorColumn is not null. Thats what I'm looking into currently. Of course we can also add a check in setDiscriminatorColumn additionally and throw an exception if name is not set since name is mandatory, only fieldName is optional and defaults to the name if not set. Thats why the current patch doesnt make much sense.

You are not using any discriminator columns actually, are you? (@DiscriminatorColumn(...)) ?

@doctrinebot

Comment created by romanb:

Found the issue. The main reason was that setDiscriminatorColumn was not checking for null, thus classes that did not even have a discriminator column in a mapped hierarchy got an array set with just fieldName set.

Alternatively we could have added the null check to clients that call setDiscriminatorColumn, like ClassMetadataFactory, but I figured it would be safer to do it in the method.

@doctrinebot

Issue was closed with resolution "Fixed"

@doctrinebot doctrinebot added this to the 2.0-ALPHA2 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