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

[WIP] Value objects #634

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ protected function doLoadMetadata($class, $parent, $rootEntityFound, array $nonS
$this->completeIdGeneratorMapping($class);
}

foreach ($class->embeddedClasses as $property => $embeddableClass) {
$embeddableMetadata = $this->getMetadataFor($embeddableClass);
$class->inlineEmbeddable($property, $embeddableMetadata);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing line break

}

if ($parent && $parent->isInheritanceTypeSingleTable()) {
$class->setPrimaryTable($parent->table);
}
Expand Down
83 changes: 76 additions & 7 deletions lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,13 @@ class ClassMetadataInfo implements ClassMetadata
*/
public $isMappedSuperclass = false;

/**
* READ-ONLY: Wheather this class describes the mapping of an embeddable class.
*
* @var boolean
*/
public $isEmbeddedClass = false;

/**
* READ-ONLY: The names of the parent classes (ancestors).
*
Expand All @@ -260,6 +267,13 @@ class ClassMetadataInfo implements ClassMetadata
*/
public $subClasses = array();

/**
* READ-ONLY: The names of all embedded classes based on properties.
*
* @var array
*/
public $embeddedClasses = array();

/**
* READ-ONLY: The named queries allowed to be called directly from Repository.
*
Expand Down Expand Up @@ -880,6 +894,15 @@ public function wakeupReflection($reflService)
$this->reflClass = $reflService->getClass($this->name);

foreach ($this->fieldMappings as $field => $mapping) {
if (isset($mapping['declaredField'])) {
$this->reflFields[$field] = new ReflectionProxy(
$reflService->getAccessibleProperty($this->name, $mapping['declaredField']),
$reflService->getAccessibleProperty($this->embeddedClasses[$mapping['declaredField']], $mapping['originalField']),
$this->embeddedClasses[$mapping['declaredField']]
);
continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing line break

}

$this->reflFields[$field] = isset($mapping['declared'])
? $reflService->getAccessibleProperty($mapping['declared'], $field)
: $reflService->getAccessibleProperty($this->name, $field);
Expand Down Expand Up @@ -921,8 +944,12 @@ public function initializeReflection($reflService)
*/
public function validateIdentifier()
{
if ($this->isMappedSuperclass || $this->isEmbeddedClass) {
return;
}

// Verify & complete identifier mapping
if ( ! $this->identifier && ! $this->isMappedSuperclass) {
if ( ! $this->identifier) {
throw MappingException::identifierRequired($this->name);
}

Expand Down Expand Up @@ -2155,9 +2182,8 @@ private function _isInheritanceType($type)
public function mapField(array $mapping)
{
$this->_validateAndCompleteFieldMapping($mapping);
if (isset($this->fieldMappings[$mapping['fieldName']]) || isset($this->associationMappings[$mapping['fieldName']])) {
throw MappingException::duplicateFieldMapping($this->name, $mapping['fieldName']);
}
$this->assertFieldNotMapped($mapping['fieldName']);

$this->fieldMappings[$mapping['fieldName']] = $mapping;
}

Expand Down Expand Up @@ -2405,9 +2431,7 @@ protected function _storeAssociationMapping(array $assocMapping)
{
$sourceFieldName = $assocMapping['fieldName'];

if (isset($this->fieldMappings[$sourceFieldName]) || isset($this->associationMappings[$sourceFieldName])) {
throw MappingException::duplicateFieldMapping($this->name, $sourceFieldName);
}
$this->assertFieldNotMapped($sourceFieldName);

$this->associationMappings[$sourceFieldName] = $assocMapping;
}
Expand Down Expand Up @@ -3019,4 +3043,49 @@ public function fullyQualifiedClassName($className)

return $className;
}

/**
* Map Embedded Class
*
* @array $mapping
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@var array $mapping

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@param, not @var actually

* @return void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing line break

*/
public function mapEmbedded(array $mapping)
{
$this->assertFieldNotMapped($mapping['fieldName']);

$this->embeddedClasses[$mapping['fieldName']] = $this->fullyQualifiedClassName($mapping['class']);
}

/**
* Inline the embeddable class
*
* @param string $property
* @param ClassMetadataInfo $embeddable
*/
public function inlineEmbeddable($property, ClassMetadataInfo $embeddable)
{
foreach ($embeddable->fieldMappings as $fieldMapping) {
$fieldMapping['declaredField'] = $property;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align = signs

$fieldMapping['originalField'] = $fieldMapping['fieldName'];
$fieldMapping['fieldName'] = $property . "." . $fieldMapping['fieldName']; // TODO: Change DQL parser to accept this dot notation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems wrong at first glance to me... it would make near to impossible to handle this in DQL. =
Also, you have to consider that according to DQL original EBNF, it should be possible to have nested embeddables. Something like: user.address.geolocation.latitude

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't touched DQL yet, my feeling was that this would not lead to more code in the DQL parts than your approach with explicit sub classes.

The dot is actually a pretty nice trick to prevent collisions, because its not a valid field name sign, and reads naturally in our DQL syntax.

$fieldMapping['columnName'] = $property . "_" . $fieldMapping['columnName']; // TODO: Use naming strategy

$this->mapField($fieldMapping);
}
}

/**
* @param string $fieldName
* @throws MappingException
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use FQCN for Exception

*/
private function assertFieldNotMapped($fieldName)
{
if (isset($this->fieldMappings[$fieldName]) ||
isset($this->associationMappings[$fieldName]) ||
isset($this->embeddedClasses[$fieldName])) {

throw MappingException::duplicateFieldMapping($this->name, $fieldName);
}
}
}
5 changes: 5 additions & 0 deletions lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
$mappedSuperclassAnnot = $classAnnotations['Doctrine\ORM\Mapping\MappedSuperclass'];
$metadata->setCustomRepositoryClass($mappedSuperclassAnnot->repositoryClass);
$metadata->isMappedSuperclass = true;
} else if (isset($classAnnotations['Doctrine\ORM\Mapping\Embeddable'])) {
$metadata->isEmbeddedClass = true;
} else {
throw MappingException::classIsNotAValidEntityOrMappedSuperClass($className);
}
Expand Down Expand Up @@ -364,6 +366,9 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
}

$metadata->mapManyToMany($mapping);
} else if ($embeddedAnnot = $this->reader->getPropertyAnnotation($property, 'Doctrine\ORM\Mapping\Embedded')) {
$mapping['class'] = $embeddedAnnot->class;
$metadata->mapEmbedded($mapping);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing line break

}
}

Expand Down
4 changes: 3 additions & 1 deletion lib/Doctrine/ORM/Mapping/Driver/DoctrineAnnotations.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

require_once __DIR__.'/../Annotation.php';
require_once __DIR__.'/../Entity.php';
require_once __DIR__.'/../Embeddable.php';
require_once __DIR__.'/../Embedded.php';
require_once __DIR__.'/../MappedSuperclass.php';
require_once __DIR__.'/../InheritanceType.php';
require_once __DIR__.'/../DiscriminatorColumn.php';
Expand Down Expand Up @@ -64,4 +66,4 @@
require_once __DIR__.'/../AssociationOverrides.php';
require_once __DIR__.'/../AttributeOverride.php';
require_once __DIR__.'/../AttributeOverrides.php';
require_once __DIR__.'/../EntityListeners.php';
require_once __DIR__.'/../EntityListeners.php';
28 changes: 28 additions & 0 deletions lib/Doctrine/ORM/Mapping/Embeddable.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php
/*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
* This software consists of voluntary contributions made by many individuals
* and is licensed under the MIT license. For more information, see
* <http://www.doctrine-project.org>.
*/

namespace Doctrine\ORM\Mapping;

/**
* @Annotation
* @Target("PROPERTY")
*/
final class Embeddable implements Annotation
{
}
32 changes: 32 additions & 0 deletions lib/Doctrine/ORM/Mapping/Embedded.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php
/*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
* This software consists of voluntary contributions made by many individuals
* and is licensed under the MIT license. For more information, see
* <http://www.doctrine-project.org>.
*/

namespace Doctrine\ORM\Mapping;

/**
* @Annotation
* @Target("CLASS")
*/
final class Embedded implements Annotation
{
/**
* @var string
*/
public $class;
}
64 changes: 64 additions & 0 deletions lib/Doctrine/ORM/Mapping/ReflectionProxy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php
/*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
* This software consists of voluntary contributions made by many individuals
* and is licensed under the MIT license. For more information, see
* <http://www.doctrine-project.org>.
*/

namespace Doctrine\ORM\Mapping;

/**
* Acts as a proxy to a nested Property structure, making it look like
* just a single scalar property.
*
* This way value objects "just work" without UnitOfWork, Persisters or Hydrators
* needing any changes.
*/
class ReflectionProxy
{
private $parentProperty;
private $childProperty;
private $class;

public function __construct($parentProperty, $childProperty, $class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should typehint the ReflectionProperty arguments

{
$this->parentProperty = $parentProperty;
$this->childProperty = $childProperty;
$this->class = $class;
}

public function getValue($object)
{
$embeddedObject = $this->parentProperty->getValue($object);

if ($embeddedObject === null) {
return null;
}

return $this->childProperty->getValue($embeddedObject);
}

public function setValue($object, $value)
{
$embeddedObject = $this->parentProperty->getValue($object);

if ($embeddedObject === null) {
$embeddedObject = new $this->class; // TODO
$this->parentProperty->setValue($object, $embeddedObject);
}

$this->childProperty->setValue($embeddedObject, $value);
}
}
1 change: 1 addition & 0 deletions lib/Doctrine/ORM/Tools/SchemaTool.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ private function processingNotRequired($class, array $processedClasses)
return (
isset($processedClasses[$class->name]) ||
$class->isMappedSuperclass ||
$class->isEmbeddedClass ||
($class->isInheritanceTypeSingleTable() && $class->name != $class->rootEntityName)
);
}
Expand Down
80 changes: 80 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/ValueObjectsTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<?php

namespace Doctrine\Tests\ORM\Functional;

/**
* @group DDC-93
*/
class ValueObjectsTest extends \Doctrine\Tests\OrmFunctionalTestCase
{
public function setUp()
{
parent::setUp();

$this->_schemaTool->createSchema(array(
$this->_em->getClassMetadata(__NAMESPACE__ . '\DDC93Person'),
$this->_em->getClassMetadata(__NAMESPACE__ . '\DDC93Address'),
));
}

public function testMetadata()
{
$person = new DDC93Person();
$person->name = "Tara";
$person->address = new DDC93Address();
$person->address->street = "United States of Tara Street";
$person->address->zip = "12345";
$person->address->city = "funkytown";

$this->_em->persist($person);
$this->_em->flush();

$this->_em->clear();

$person = $this->_em->find(DDC93Person::CLASSNAME, $person->id);

$this->assertInstanceOf(DDC93Address::CLASSNAME, $person->address);
$this->assertEquals('United States of Tara Street', $person->address->street);
$this->assertEquals('12345', $person->address->zip);
$this->assertEquals('funkytown', $person->address->city);
}
}

/**
* @Entity
*/
class DDC93Person
{
const CLASSNAME = __CLASS__;

/** @Id @GeneratedValue @Column(type="integer") */
public $id;

/** @Column(type="string") */
public $name;

/** @Embedded(class="DDC93Address") */
public $address;
}

/**
* @Embeddable
*/
class DDC93Address
{
const CLASSNAME = __CLASS__;

/**
* @Column(type="string")
*/
public $street;
/**
* @Column(type="string")
*/
public $zip;
/**
* @Column(type="string")
*/
public $city;
}