Odd validation behaviour using annotations #5156

Closed
gremo opened this Issue Aug 2, 2012 · 6 comments

Comments

Projects
None yet
5 participants
@gremo

gremo commented Aug 2, 2012

I don't know if this is an unwanted behaviour or it's supposed to be as-is. Subclasses cannot override validation of inherited fields.

A mapped AbstractMessage superclass for common fields:

/** @ORM\MappedSuperclass */
abstract class AbstractMessage
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    private $id;

    /**
     * @ORM\Column(type="text", nullable=true)
     */
    private $content;
}

An abstract AbstractOutgoingMessage class with Doctrine CTI for outgoing messages (additional fields not shown here):

/**
 * @ORM\Entity
 * @ORM\Table(name="outgoing_message")
 * @ORM\InheritanceType("JOINED")
 * @ORM\DiscriminatorColumn(name="type", type="string")
 * @ORM\DiscriminatorMap({"small_text_message" = "SmallTextMessage", "newsletter" = "Newsletter"})
 */
abstract class AbstractOutgoingMessage extends AbstractMessage { }

A concrete class Newsletter class, adding validation by annotations:

/**
 * @ORM\Entity
 * @ORM\Table(name="newsletter")
 */
class Newsletter extends AbstractOutgoingMessage
{
    /**
     * @Assert\NotBlank(message="You must provide a content.")
     */
    private $content;
}

With this hierarchy, validation of Newsletter simply doesn't work. When adding setContent and getContent copied straight from AbstractMessage to Newsletter class, validation works, but fields are not persisted (got null content).

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Aug 3, 2012

Member

This is logical: the private property in the child class is not the same one than the private property in the parent. If you want to redefine the properties in child classes, use protected properties so that they are considered the same.

Member

stof commented Aug 3, 2012

This is logical: the private property in the child class is not the same one than the private property in the parent. If you want to redefine the properties in child classes, use protected properties so that they are considered the same.

@gremo

This comment has been minimized.

Show comment
Hide comment
@gremo

gremo Aug 3, 2012

stof, i appreciated your answer. But after making protected properties in the parent class, doctrine will generate private properties. So an error occurred: access level should be weaker in the child class. If you make then protected also, you'll get an error: Class XXX has no field or association named "content".

gremo commented Aug 3, 2012

stof, i appreciated your answer. But after making protected properties in the parent class, doctrine will generate private properties. So an error occurred: access level should be weaker in the child class. If you make then protected also, you'll get an error: Class XXX has no field or association named "content".

@gremo

This comment has been minimized.

Show comment
Hide comment
@gremo

gremo Aug 4, 2012

AS proof, a little example. As you said, using protected properties:

/** @ORM\MappedSuperclass */
abstract class AbstractMessage
{
    /** @ORM\Column(type="text", nullable=true) */
    protected $content;
}
/** @ORM\Entity */
class InternalMessage extends AbstractMessage
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    protected $id;

    /** @Assert\NotBlank(message="Internal message title is required.")  */
    protected $content;

    public function setContent($content)
    {
        $this->content = $content;
        return $this;
    }

    public function getContent() { return $this->content; }
}

Validation works, BUT content is not persisted. While removing content from InternalMessage, content itself is persisted while validation does not work.

gremo commented Aug 4, 2012

AS proof, a little example. As you said, using protected properties:

/** @ORM\MappedSuperclass */
abstract class AbstractMessage
{
    /** @ORM\Column(type="text", nullable=true) */
    protected $content;
}
/** @ORM\Entity */
class InternalMessage extends AbstractMessage
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    protected $id;

    /** @Assert\NotBlank(message="Internal message title is required.")  */
    protected $content;

    public function setContent($content)
    {
        $this->content = $content;
        return $this;
    }

    public function getContent() { return $this->content; }
}

Validation works, BUT content is not persisted. While removing content from InternalMessage, content itself is persisted while validation does not work.

@webmozart

This comment has been minimized.

Show comment
Hide comment
@webmozart

webmozart Aug 4, 2012

Contributor

You might have to redeclare the ORM mapping information on your field.

/**
 * @ORM\Column(type="text", nullable=true)
 * @Assert\NotBlank(message="Internal message title is required.")
 */
protected $content;

If that works, you should consult the Doctrine team what the issue is here.

Contributor

webmozart commented Aug 4, 2012

You might have to redeclare the ORM mapping information on your field.

/**
 * @ORM\Column(type="text", nullable=true)
 * @Assert\NotBlank(message="Internal message title is required.")
 */
protected $content;

If that works, you should consult the Doctrine team what the issue is here.

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Aug 4, 2012

Member

Well, the issue is that declaring the property twice means that Reflection considers that there is 2 different properties (with different docComments). For protected properties, Doctrine reads it at the bottom of the hierarchy to take care of the potential overwriting of the property. It does not read all docComments in the hierarchy (this would be a pain in the ass to merge them together).

Anyway, redefining properties is likely to be a pain when using Reflection (so Doctrine, Validator, JMSSerializerBundle and so on). It may be easier to add the constraint through a YAML mapping instead (but it would still require to have the property protected if you want to have the constraint only in the child class)

Member

stof commented Aug 4, 2012

Well, the issue is that declaring the property twice means that Reflection considers that there is 2 different properties (with different docComments). For protected properties, Doctrine reads it at the bottom of the hierarchy to take care of the potential overwriting of the property. It does not read all docComments in the hierarchy (this would be a pain in the ass to merge them together).

Anyway, redefining properties is likely to be a pain when using Reflection (so Doctrine, Validator, JMSSerializerBundle and so on). It may be easier to add the constraint through a YAML mapping instead (but it would still require to have the property protected if you want to have the constraint only in the child class)

@cordoval

This comment has been minimized.

Show comment
Hide comment
@cordoval

cordoval Dec 4, 2013

Contributor

@gremo

when you split classes like above, and you have a mapped property on a superclass then you have to use annotations to override mapping @override, check doctrine documentation. I think between @stof et all explanations and this that should get you going in the right direction.

Please use the mailing list for these problems. Thanks.

ping @fabpot we can close this safely i think

Contributor

cordoval commented Dec 4, 2013

@gremo

when you split classes like above, and you have a mapped property on a superclass then you have to use annotations to override mapping @override, check doctrine documentation. I think between @stof et all explanations and this that should get you going in the right direction.

Please use the mailing list for these problems. Thanks.

ping @fabpot we can close this safely i think

@fabpot fabpot closed this Dec 5, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment