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

Support for polymorph association through interfaces #6072

Closed
ghost opened this issue Oct 6, 2016 · 21 comments
Closed

Support for polymorph association through interfaces #6072

ghost opened this issue Oct 6, 2016 · 21 comments

Comments

@ghost
Copy link

ghost commented Oct 6, 2016

Currently Doctrine has support for polymorphic associations through inheritance however this use is rather limited. It would be nice if polymorphic associations are also possible through interfaces. That is that you can declare a PHP interface and tag it with an @entity annotation and implement these interfaces in class entities through the implements keyword. Then allowing the targetEntity of associations to be an interface with @entity annotation creating a polymorphic association.

Though in interface languages properties are often defined which must be implemented in classes when implemented. PHP has no such abilities and uses getters and setters instead. I think in the same way a interface entity should not be allowed to define any columns or associations. Instead only define the getters and setters if such are necessary. And leave it to the implementing classes to define the columns or associations on the properties. For DBAL this means the interface entities have no known fields or associations but the INSTANCE OF operator should be usable.

This would fix the rather inconvenience that you either need to implement your own polymorphs when inheritance is not suitable or risk using inheritance which would make things to coupled and create difficulties later on in development.

@Ocramius
Copy link
Member

Ocramius commented Oct 6, 2016

You can already reference interfaces, as long as you provide a concrete implementation at runtime, in a listener that acts during metadata loading.

Is that what you are looking for?

@ghost
Copy link
Author

ghost commented Oct 6, 2016

That depends can I set an interface as a targetEntity for a OneToOne or ManyToOne association? And will such an association create two fields in the database one for the type of entity and another for the id in the table that contains the OneToOne or ManyToOne association?

@Ocramius
Copy link
Member

Ocramius commented Oct 7, 2016

You can indeed set an interface as targetEntity in any association type,
but the ORM still expects a concrete instance once metadata is loaded.

Here are some resources about it (they all involve using the
"ResolveTargetEntityListener":

http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/cookbook/resolve-target-entity-listener.html

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On Thu, Oct 6, 2016 at 8:12 PM, davekok notifications@github.com wrote:

That depends can I set interface as a targetEntity for a OneToOne
or ManyToOne association?
And will such an association create two fields
in the database one for the type of entity and
another for the id in the
table that contains the OneToOne or ManyToOne association?

On Thu, 06
Oct 2016 01:36:18 -0700, Marco Pivetta wrote:

You can already
reference interfaces, as long as you provide a concrete implementation
at runtime, in a listener that acts during metadata loading.

Is
that what you are looking for?

You are receiving this because
you authored the thread.
Reply to this email directly, view it on
GitHub [1], or mute the thread [2].

Links:

[1]
#6072 (comment)
[2]
https://github.com/notifications/unsubscribe-auth/AAZMm6F2-Z6fy6MAV-
bpOEZXY6lVSB00ks5qxLMCgaJpZM4KPqm6


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#6072 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJakIGlYiW7pm_5zui2MlfWu-ANV1R_ks5qxToJgaJpZM4KPqm6
.

@ghost
Copy link
Author

ghost commented Oct 7, 2016

Great but not what I asked. The goal is to have a polymorphic association. Implemented through interfaces. It is already possible through inheritance so it should not be to difficult.

@Ocramius
Copy link
Member

Ocramius commented Oct 7, 2016

Then I'm still not understanding what is going on. An inheritance is the
current way forward, and it works. It is possible to emulate an inheritance
by replacing the root of the inheritance with a fake (non-existing) class,
and an interface as mapped type (to be replaced at runtime).

Is that what you were asking?

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On Fri, Oct 7, 2016 at 2:01 PM, davekok notifications@github.com wrote:

Great but not what I asked. The goal is to have a polymorphic association.
Implemented through interfaces. It is already possible through inheritance
so it should not be to difficult.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#6072 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJakBdfZYnCi15qCVKIs1zLYlfElU_0ks5qxjSDgaJpZM4KPqm6
.

@ghost
Copy link
Author

ghost commented Oct 8, 2016

I am not asking to replace a mapped type at runtime. I am asking to store the type as part of the association as is customary with a polymorph. Basically the same way it is done with the DiscriminatorColumn for inheritance but using interfaces instead. Perhaps you should look up what a polymorph is.

@ghost
Copy link
Author

ghost commented Oct 8, 2016

To further clarrify sometimes single inheritance is not enough. In these cases you use interfaces. As far as I know you can't use interfaces as an alternative to multiple inheritance in Doctrine.

@Ocramius
Copy link
Member

Ocramius commented Oct 8, 2016

Well aware what it is - I gave you a way to implement it, did you try doing
so?

On 8 Oct 2016 11:28 a.m., "davekok" notifications@github.com wrote:

I am not asking to replace a mapped type at runtime. I am asking to store
the type as part of the association as is customary with a polymorph.
Basically the same way it is done with the DiscriminatorColumn for
inheritance but using interfaces instead. Perhaps you should look up what a
polymorph is.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#6072 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJakMHWX-nHOEMvO3Cp60rid_LgT7gbks5qx2JSgaJpZM4KPqm6
.

@ghost
Copy link
Author

ghost commented Oct 9, 2016

Ok how do I resolve one interface to many different classes? The article only mentions resolving one interface to one class.

@ghost
Copy link
Author

ghost commented Oct 17, 2016

So recap, the ResolveTargetEntityListener is great for decoupling bundles allowing the programmer to set a concrete class at runtime. However it is not a polymorph in the sense that multiple concrete classes can be used. You must choose one.

The issue is however not about decoupling bundles at all. The idea is to use interfaces as an alternative to multiple inheritance (not possible anyways in PHP) within the same bundle. Thus adding something like a DiscriminatorColumn annotation to an interface declaration. An entity may then associate to an interface instead of a base class. And when storing the id in one column the actual entity to use is stored in another column.

Is Doctrine team interested in implementing this?

@lcobucci
Copy link
Member

@davekok can you please give us a good use case for this feature? Please also provide an explanation about the database side (especially regarding the foreign keys).

@ghost
Copy link
Author

ghost commented Oct 18, 2016

Here is an abstract use case. As far as I know no foreign keys are possible with polymorphs as it links to multiple tables and SQL does not support that. I think polymorphs using interfaces instead of CTI should only be allowed on ManyToOne or ManyToMany associations. As they can not own what is referenced which should make the necessity of a foreign keys less important. As long as id's are never changed. In the case of deleting a entity referenced through a polymorph a join should not fail but simply not find it as if it had been null. I would leave it to the programmer to decide to implement something to reset polymorph association to null on deletion.

/**
 * @Entity
 * @DiscriminatorColumn(name='a_type', type='int')
 * @DiscriminatorMap({0 = "C", 1 = "D", 2 = "E"})
 */
interface A {}

/**
 * @Entity
 * @InheritanceType("SINGLE_TABLE")
 * @DiscriminatorColumn(name="type", type="integer")
 * @DiscriminatorMap({0 = "C", 1 = "D"})
 */
abstract class B implements A {}

/** @Entity */
class C extends B {}

/** @Entity */
class D extends B {}

/** @Entity */
class E implements A {}

/** @Entity */
class F {
  /** @ManyToOne(targetEntity="A") */
  private $As;
}
create table B (
  id int not null primary key,
  type int not null
);

create table E (
  id int not null primary key
);

create table F (
  id int not null primary key,
  a_id int, -- no foreign key possible
  a_type int
); 

@lcobucci
Copy link
Member

@davekok I was looking for a concrete use case but anyway 😜

Although it looks a good idea at first sight, I find it hard to see a good explanation on why implement this. I mean, there are some trade-offs that I wouldn't allow on things I write (like the lack of foreign keys).

My opinion is that this is a no go, it adds a unnecessary complexity to the ORM and to the schema without adding a huge benefit on design.

@Ocramius what do you think?

@ghost
Copy link
Author

ghost commented Oct 18, 2016

@lcobucci Is there a alternative in case you really do need it without excessive joins? It seems to me this should not be more difficult then joined CTI. Personally I would prefer using either single table CTI or this and drop joined CTI all together.

Still structural problems are best fixed (avoided) through interfaces. Forcing CTI perhaps lessens the burden of doctrine but increases the burden on applications. This seems to me a must have feature. Otherwise I will never be able use more generalized data models. I would also have to use intermediate tables for the sole purpose of using foreign keys and have excessive joins in queries.

@1ed
Copy link
Contributor

1ed commented Oct 18, 2016

I implemented something like this in a project using a lifecycle listener and some custom annotations. In my case I needed a way to add events to an activity stream like

  • patient requested appointment (AppointmentRequest entity)
  • appointment was accepted by the patient (Appointment entity)
  • ...
**
 * Activity entity.
 *
 * For example an activity has the following properties:
 *
 *   - actor: Who did the activity?
 *   - verb: What he did?
 *   - published: When was it done?
 *   - object: What it refers to?
 *
 * <actor>  <verb>                  <published>
 * patient requested appointment on 2016-10-21
 *   message <object>
 *
 * @ORM\Entity(readOnly=true)
 * @HasPolymorphicRelation()
 */
class Activity
{
    /**
     * @var string
     *
     * @ORM\Id()
     * @ORM\Column(type="uuid")
     */
    private $id;

    /**
     * @var Chronos
     *
     * @ORM\Column(type="chronos_datetime")
     */
    private $published;

    /**
     * @var string
     *
     * @ORM\Column()
     */
    private $verb;

    /**
     * @var string
     *
     * @ORM\Column()
     */
    private $actorType;

    /**
     * @var string
     *
     * @ORM\Column()
     */
    private $actorId;

    /**
     * @var mixed
     *
     * @PolymorphicRelation()
     */
    private $actor;

    /**
     * @var string
     *
     * @ORM\Column()
     */
    private $objectType;

    /**
     * @var string
     *
     * @ORM\Column()
     */
    private $objectId;

    /**
     * @var mixed
     *
     * @PolymorphicRelation()
     */
    private $object;

    /**
     * @var string
     *
     * @ORM\Column()
     */
    private $targetType;

    /**
     * @var string
     *
     * @ORM\Column()
     */
    private $targetId;

    /**
     * @var mixed
     *
     * @PolymorphicRelation()
     */
    private $target;
}

The listener looks for HasPolymorphicRelation and PolymorphicRelation annotations and populates type and id fields automatically at prePrestist and hydrates the related entities at postLoad. It works great until I need to update or delete the relations (which was not required in my case, as the activity stream is read only).

As a reference eloquent also has a feature something like this https://laravel.com/docs/5.3/eloquent-relationships#polymorphic-relations.

@ghost ghost closed this as completed Feb 5, 2018
@Ocramius
Copy link
Member

Ocramius commented Feb 5, 2018

To me, this looks like a misuse of types, as the fields become (from the outside) effectively mixed or mixed[].

Take this field for example:

    /**
     * @var mixed
     *
     * @PolymorphicRelation()
     */
    private $actor;

This should simply be mapped as Actor, which should be the root of a Single Table Inheritance (STI) or a Joined Table Inheritance (JTI). The Actor symbol should be defined and referenced, and subtypes should be covariant to it. The Actor symbol may be a class or interface, but in case of an interface, some metadata replacement is required at runtime.

In alternative, I suggest not loading the associations at runtime at all, and instead keeping two split fields (or an Embeddable) for each of these associations, representing:

  • Type of the referenced field
  • Identifier of the referenced field (usually JSON-encoded to support composite identifiers in a simplistic way)

The last solution I proposed is simpler, as it doesn't involve complex reference type variations at runtime, and is simpler to reason about also later on without too much magic going on.

@erop
Copy link

erop commented Apr 13, 2020

To me, this looks like a misuse of types, as the fields become (from the outside) effectively mixed or mixed[].

Take this field for example:

    /**
     * @var mixed
     *
     * @PolymorphicRelation()
     */
    private $actor;

This should simply be mapped as Actor, which should be the root of a Single Table Inheritance (STI) or a Joined Table Inheritance (JTI).

Not sure it is correct to re-open such an old issue but could not find any other info related to the topic.

Here's my real-life use case. I'm building an application which has PolicyApplication entity. This is the one holding a customer request for issuing a car insurance policy. There definitely should be a PolicyHolderInterface implemented both by Person and Organisation entities. Yeah, I could use one of the inheritance mappings documented. But it only applies to classes. Thus I need to “extend” it just once in both of my concrete target classes. But what if nested Vehicle::$owner needed to be either Person or Organisation as well? I.e. I need polymorphic association again in the same PolicyApplication but on the other level in the nested object. And I believe they should implement hm-m-m…. OwnerInterface that time.

I managed to use prePersist and postLoad methods of entity listeners to populate @Embedded into PolicyApplication LinkObject holding type and instanceId on Creation (prePersist) and Retrieving (postLoad) PolicyApplication. But it seems do not work while Updating 😕

@erop
Copy link

erop commented Apr 13, 2020

In a minute after hitting Comment button I started googling hibernate polymorphic association and found this page. I think the topic starter (and me) meant something like that to be available in Doctrine, possibility of having interfaces as an association "root".

@parijke
Copy link

parijke commented Jun 28, 2020

Anyone managed to implement polymorphic associations without extending a class?

@mpdude
Copy link
Contributor

mpdude commented Dec 8, 2020

@Ocramius Should the ResolveTargetEntityListener mechanism work in Discriminator Maps as well?

@parijke
Copy link

parijke commented Jun 4, 2021

I don't fully understand it, but it appears that this is doing what you asked for?
https://github.com/mareg/commentable

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

No branches or pull requests

6 participants