[WIP] Mapping support for Embeddables (VOs).

[guilhermeblanco]

This patch is going to address: http://www.doctrine-project.org/jira/browse/DDC-93

Embeddable classes are a good way for organizing your domain model. Specially larger systems, which contains similar domain objects with similar properties.
This patch proposes a cleaner way to structure these duplicating attributes (and logic) into an @embeddable object. Abstract the common information into an @Embeddedable object and use it as @Embedded.

/**
 * @Embeddable
 */
class Location
{
    /** @Column(type="decimal", scale=8, precision=12, nullable=true) */
    protected $latitude;

    /** @Column(type="decimal", scale=8, precision=12, nullable=true) */
    protected $longitude;
}

/**
 * @Entity 
 */
class User
{
    /** @Id @GeneratedValue @Column(type="integer") */
    protected $id;

    // ...

    /** @Embedded(class="Location") */
    protected $location;
}

Querying:

$query = $entityManager->createQuery('SELECT u FROM User u WHERE u.location.latitude = ?1');

[guilhermeblanco]

I'll change these flags to an entityType property and assign a value for it in an easier way. That way, we could always inspect for entity type by doing:

$metadata->entityType === ClassMetadata::TYPE_ENTITY
$metadata->entityType === ClassMetadata::TYPE_MAPPEDSUPERCLASS
$metadata->entityType === ClassMetadata::TYPE_EMBEDDABLE

[guilhermeblanco]

According to @jwage suggest, we should use @EmbedOne instead of @Embedded.
This is valid in the case we want to support @EmbedMany in the future.

[stof]

just a note before looking at the diff: you should not use public properties when giving an example in a PR (or elsewhere). It would make it even more difficult when explaining to users that using public properties are forbidden because of proxies

[guilhermeblanco]

@stof changed. =P

[Ocramius]

$class->isMappedSuperclass && ! $class->isEmbeddable

This check is used also later on. Could it be implemented as a method of ClassMetadata?

[guilhermeblanco]

This will be changed soon with ->entityType refactoring.

[stof]

@guilhermeblanco how would these be stored ? a separate table in the DB ?

[Ocramius]

Additional columns on the same table...

[Ocramius]

We surely could need an additional property to allow the user to define what the prefix used when building embedded fields would look like.

By prefix, I would also suggest it is not just "fieldName" in "fieldName_embeddableField", but I'd also include the "_", allowing the user not to depend on a pre-defined separator

[michelsalib]

Considering the fix to use @EmbedOne, this is a very nice feature.
It will clearly easy some stuffs and improve performance (in some of my use cases).

[beberlei]

Regarding the implementation in Hydration i think it would make sense to "simulate" as if the value object were an entity, that means:

1. SQLWalker creates DQL Alias for VOs, i.e. "ParentEntityAlias__VO_fieldName".
2. ResultSetMapping has this DQL Alias
3. Value Objects have all their fields defined as identifier. So they are used to generate an ID hash.
4. ObjectHydrator::createEntity has a shortcut for value objects, so the UnitOfWork does not need to know about them.

This assumes that value objects are ALWAYS read-only. This would simplify this patch considerably. Updating "values" has to be done by switching the whole object. Then only the persisters have to be updated, not the Hydrators.

[jonathaningram]

Does anyone know the status of this PR? Is there any way I could contribute to get the ball rolling on this feature?

[lstrojny]

Very cool feature. Would love to see it making its way into Doctrine. Two things would be cool: optionally define the mapping in the embedding class and to pass value objects as query parameters.

[german-bortoli]

+1 to this feature

[rouffj]

+1

[marijn]

Why are these and other @Embeddable fields marked as public. A value object should be immutable, right?

[lstrojny]

Immutability is based on the value object itself. Being public doesn’t make the value object itself mutable.

[stof]

however, public properties break Doctrine proxies so they should be avoided in tests too IMO (this object will work in the cases used by the tests but they will confuse users looking at it)

[lstrojny]

Is that because public properties don’t trigger the internal lazy loading?

[guilhermeblanco]

In tests they are public for simplicity... I can mark as protected if you have stronger arguments.

[stof]

@lstrojny the proxy factory cannot edit the code of your own classes. We don't want this at all

[guilhermeblanco]

That's a problem of PHP. @lstrojny maybe you could help PHP core to implement a concept that Java has called Interceptors.
That's how Hibernate solves clearly the issue in Java. The get/set property patch (similar to .NET which was proposed) may also fix, but it needs approval afaik.

[lstrojny]

I know we are getting more and more OT :)

@stof Editing is not necessary. It could be done dynamically in the proxy itself.

@guilhermeblanco I'm not sure we need this, support for better accessors is indeed quite an interesting patch, nevertheless we don’t need it for this issue. What’s not very well known is that unset() works on object properties and after unset() is called magic methods are called again. This is something Doctrine could use. See this gist for illustration: https://gist.github.com/2662665

[stof]

but it could cause some issues if someone uses both public properties and __call in a class

[marijn]

What simplicity is associated with marking these properties as public?

[marijn]

Nice work @guilhermeblanco! 👍 👍 👍

What is the status of this PR?

[stof]

CS issue (and for next methods too)

[mhlavac]

What is the status? Is there a need for this PR any longer? Same functionality can be achieved with traits in PHP 5.4?

trait Location
{
    /** @Column(type="decimal", scale=8, precision=12, nullable=true) */
    protected $latitude;

    /** @Column(type="decimal", scale=8, precision=12, nullable=true) */
    protected $longitude;
}

/**
 * @Entity 
 */
class User
{
    /** @Id @GeneratedValue @Column(type="integer") */
    protected $id;

    // ...

    use Location;
}

It's always good to implement an interface when using trait. Also you can't access embedded variable with DQL like this:

SELECT u FROM User u WHERE u.location.latitude = 1234.00

You must use original names instead:

SELECT u FROM User u WHERE u.latitude = 1234.00

[stof]

@mhlavac traits are not the same. They would put the property in the entity itself, not in an embedded value object

[FZ14]

Hi!
Will Doctrine\ORM 2.3 adds the ability to use Value Objects?
If not, when do you plan to add this feature?

[stof]

@FZ14 no as Doctrine 2.3 is RC3 (so no new features in it anymore) and is due in a few days, whereas this feature is far from being ready.

[beberlei]

Closing this for now as a rebase will be virtually impossible given recent changes and what Fabio plans.

[FZ14]

@beberlei Why is it impossible? What recent changes did you mean, and what Fabio plans?
Thanks.

[stof]

@FZ14 A big part of the ClassMetadataFactory has been moved to Doctrine Common in 2.3

[gseric]

Any ETA or target version for this feature? IMHO it's essential for any medium to large scale application.

[beberlei]

I plan to POC this feature for the next release (2.5), but i cannot make any promises. I agree that its very important.

[masi]

2.5? What happened to 2.4? Sorry for crashing this issue, but I was looking for this feature in TYPO3 Flow which is based on Doctrine2. Too bad that the latter seems to lack a custom support and Doctrine istelf isn't ready yet for that.

[stof]

@masi it is too late to start a new big feature for 2.4 now, as its beta release is scheduled for December

[Majkl578]

@stof: I'm wondering if there is something new for 2.4? According to diff between 2.3.0 and master, I don't see anything big, only some small changes and some BC breaks...

[kdambekalns]

@masi Well, down in the internals we handle VOs like entities, but some optimization is done - if you create the "same" object it will be "normalized" when being saved, so no duplication in that regard at least.