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] Nullable embedded objects. #1275

Closed
wants to merge 11 commits into from
3 changes: 2 additions & 1 deletion lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ protected function doLoadMetadata($class, $parent, $rootEntityFound, array $nonS
$this->addNestedEmbeddedClasses($embeddableMetadata, $class, $property);
}

$class->inlineEmbeddable($property, $embeddableMetadata);
$class->inlineEmbeddable($property, $embeddableMetadata, $embeddableClass['nullable']);

unset($this->embeddablesActiveNesting[$class->name]);
}
Expand Down Expand Up @@ -483,6 +483,7 @@ private function addNestedEmbeddedClasses(ClassMetadata $subClass, ClassMetadata
$parentClass->mapEmbedded(array(
'fieldName' => $prefix . '.' . $property,
'class' => $embeddableMetadata->name,
'nullable' => $embeddableClass['nullable'],
'columnPrefix' => $embeddableClass['columnPrefix'],
'declaredField' => $embeddableClass['declaredField']
? $prefix . '.' . $embeddableClass['declaredField']
Expand Down
46 changes: 45 additions & 1 deletion lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -3216,6 +3216,7 @@ public function mapEmbedded(array $mapping)
$this->embeddedClasses[$mapping['fieldName']] = array(
'class' => $this->fullyQualifiedClassName($mapping['class']),
'columnPrefix' => $mapping['columnPrefix'],
'nullable' => isset($mapping['nullable']) ? $mapping['nullable'] : null,
'declaredField' => isset($mapping['declaredField']) ? $mapping['declaredField'] : null,
'originalField' => isset($mapping['originalField']) ? $mapping['originalField'] : null,
);
Expand All @@ -3226,8 +3227,9 @@ public function mapEmbedded(array $mapping)
*
* @param string $property
* @param ClassMetadataInfo $embeddable
* @param boolean $nullable
*/
public function inlineEmbeddable($property, ClassMetadataInfo $embeddable)
public function inlineEmbeddable($property, ClassMetadataInfo $embeddable, $nullable)
Copy link
Member

Choose a reason for hiding this comment

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

Is that extra parameters really required? Can't you use $this->embeddedClasses[$property]['nullable'] in this method instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, my mistake.

{
foreach ($embeddable->fieldMappings as $fieldMapping) {
$fieldMapping['originalClass'] = isset($fieldMapping['originalClass'])
Expand All @@ -3241,6 +3243,10 @@ public function inlineEmbeddable($property, ClassMetadataInfo $embeddable)
: $fieldMapping['fieldName'];
$fieldMapping['fieldName'] = $property . "." . $fieldMapping['fieldName'];

if ($nullable !== null) {
$fieldMapping['nullable'] = $nullable;
}

if (! empty($this->embeddedClasses[$property]['columnPrefix'])) {
$fieldMapping['columnName'] = $this->embeddedClasses[$property]['columnPrefix'] . $fieldMapping['columnName'];
} elseif ($this->embeddedClasses[$property]['columnPrefix'] !== false) {
Expand Down Expand Up @@ -3313,4 +3319,42 @@ public function getSequencePrefix(AbstractPlatform $platform)

return $sequencePrefix;
}

/**
* Configures the entity with the data values according with the class configuration
*
* @param object $entity
* @param array $data
*/
public function populateEntity($entity, array $data)
{
foreach ($data as $field => $value) {
if ($this->shouldUseData($field, $value)) {
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to cache this method call?

Copy link
Member Author

Choose a reason for hiding this comment

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

It can be done, but do you think is needed? I mean, the operations aren't too complex and they're based on attributes that are already caching the information (and only will rely on that attributes when the $value is null)...

$this->reflFields[$field]->setValue($entity, $value);
}
}
}

/**
* Returns if the data should be used to populate the entity
*
* @param string $field
* @param string $value
* @return boolean
*/
private function shouldUseData($field, $value)
{
if ( ! isset($this->fieldMappings[$field])) {
return false;
}

if ($value !== null) {
return true;
}

return isset(
$this->fieldMappings[$field]['declaredField'],
$this->embeddedClasses[$name = $this->fieldMappings[$field]['declaredField']]
) && $this->embeddedClasses[$name]['nullable'] !== true;
}
}
1 change: 1 addition & 0 deletions lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
} else if ($embeddedAnnot = $this->reader->getPropertyAnnotation($property, 'Doctrine\ORM\Mapping\Embedded')) {
$mapping['class'] = $embeddedAnnot->class;
$mapping['columnPrefix'] = $embeddedAnnot->columnPrefix;
$mapping['nullable'] = $embeddedAnnot->nullable;
$metadata->mapEmbedded($mapping);
}

Expand Down
5 changes: 5 additions & 0 deletions lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,14 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
? $this->evaluateBoolean($embeddedMapping['use-column-prefix'])
: true;

$nullable = isset($embeddedMapping['nullable'])
? $this->evaluateBoolean($embeddedMapping['nullable'])
: null;

$mapping = array(
'fieldName' => (string) $embeddedMapping['name'],
'class' => (string) $embeddedMapping['class'],
'nullable' => $nullable,
'columnPrefix' => $useColumnPrefix ? $columnPrefix : false
);

Expand Down
1 change: 1 addition & 0 deletions lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
$mapping = array(
'fieldName' => $name,
'class' => $embeddedMapping['class'],
'nullable' => isset($embeddedMapping['nullable']) ? $embeddedMapping['nullable'] : null,
'columnPrefix' => isset($embeddedMapping['columnPrefix']) ? $embeddedMapping['columnPrefix'] : null,
);
$metadata->mapEmbedded($mapping);
Expand Down
5 changes: 5 additions & 0 deletions lib/Doctrine/ORM/Mapping/Embedded.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,9 @@ final class Embedded implements Annotation
* @var mixed
*/
public $columnPrefix;

/**
* @var boolean
*/
public $nullable;
}
6 changes: 1 addition & 5 deletions lib/Doctrine/ORM/UnitOfWork.php
Original file line number Diff line number Diff line change
Expand Up @@ -2576,11 +2576,7 @@ public function createEntity($className, array $data, &$hints = array())
return $entity;
}

foreach ($data as $field => $value) {
if (isset($class->fieldMappings[$field])) {
$class->reflFields[$field]->setValue($entity, $value);
}
}
$class->populateEntity($entity, $data);

// Loading the entity right here, if its in the eager loading map get rid of it there.
unset($this->eagerLoadingEntities[$class->rootEntityName][$idHash]);
Expand Down
121 changes: 121 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/ValueObjectsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public function setUp()
$this->_em->getClassMetadata(__NAMESPACE__ . '\DDC93Car'),
$this->_em->getClassMetadata(__NAMESPACE__ . '\DDC3027Animal'),
$this->_em->getClassMetadata(__NAMESPACE__ . '\DDC3027Dog'),
$this->_em->getClassMetadata(__NAMESPACE__ . '\DDC3529Event'),
));
} catch(\Exception $e) {
}
Expand Down Expand Up @@ -242,6 +243,85 @@ public function testThrowsExceptionOnInfiniteEmbeddableNesting($embeddableClassN
));
}

public function testThrowsExceptionWhenPersistingWithNullOnNonNullableEmbedded()
{
$this->setExpectedException('Doctrine\DBAL\Exception\NotNullConstraintViolationException');

$event = new DDC3529Event();
$event->name = 'PHP Conference';
$event->period = new DDC3529DateInterval(new \DateTime('2015-01-20 08:00:00'), new \DateTime('2015-01-23 19:00:00'));

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

return $event;
}

public function testNoErrorsShouldHappenWhenPersistingAnEntityWithNullableEmbedded()
{
$event = new DDC3529Event();
$event->name = 'PHP Conference';
$event->country = new DDC93Country('Brazil');
$event->period = new DDC3529DateInterval(new \DateTime('2015-01-20 08:00:00'), new \DateTime('2015-01-23 19:00:00'));

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

return $event;
}

/**
* @depends testNoErrorsShouldHappenWhenPersistingAnEntityWithNullableEmbedded
* @param DDC3529Event $event
*/
public function testEmbeddedObjectShouldNotBeCreatedWhenIsNullableAndHaveNoData(DDC3529Event $event)
{
$event = $this->_em->find(DDC3529Event::CLASSNAME, $event->id);

$this->assertEquals('PHP Conference', $event->name);
$this->assertEquals('Brazil', $event->country->name);
$this->assertEquals('2015-01-20 08:00:00', $event->period->begin->format('Y-m-d H:i:s'));
$this->assertEquals('2015-01-23 19:00:00', $event->period->end->format('Y-m-d H:i:s'));
$this->assertNull($event->submissions);

return $event;
}

/**
* @depends testEmbeddedObjectShouldNotBeCreatedWhenIsNullableAndHaveNoData
* @param DDC3529Event $event
*/
public function testEmbeddedObjectShouldBeCreatedWhenIsNullableButHaveData(DDC3529Event $event)
{
$event->submissions = new DDC3529DateInterval(new \DateTime('2014-11-20 08:00:00'), new \DateTime('2014-12-23 19:00:00'));

$this->_em->persist($event);
$this->_em->flush();
$this->_em->clear();

$event = $this->_em->find(DDC3529Event::CLASSNAME, $event->id);

$this->assertEquals('2014-11-20 08:00:00', $event->submissions->begin->format('Y-m-d H:i:s'));
$this->assertEquals('2014-12-23 19:00:00', $event->submissions->end->format('Y-m-d H:i:s'));

return $event;
}

/**
* @depends testEmbeddedObjectShouldBeCreatedWhenIsNullableButHaveData
* @param DDC3529Event $event
*/
public function testFindShouldReturnNullAfterTheObjectWasRemoved(DDC3529Event $event)
{
$eventId = $event->id;

$event = $this->_em->find(DDC3529Event::CLASSNAME, $eventId);
$this->_em->remove($event);
$this->_em->flush();

$this->assertNull($this->_em->find(DDC3529Event::CLASSNAME, $eventId));
}

public function getInfiniteEmbeddableNestingData()
{
return array(
Expand Down Expand Up @@ -279,6 +359,47 @@ public function __construct($name = null, DDC93Address $address = null)
}
}

/**
* @Embeddable
*/
class DDC3529DateInterval
{
/** @Column(type = "datetime") */
public $begin;

/** @Column(type = "datetime") */
public $end;

public function __construct(\DateTime $begin, \DateTime $end)
{
$this->begin = $begin;
$this->end = $end;
}
}

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

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

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

/** @Embedded(class = "DDC3529DateInterval") */
public $period;

/** @Embedded(class = "DDC3529DateInterval", nullable = true) */
public $submissions;

/** @Embedded(class = "DDC93Country", nullable = false) */
public $country;
}

/**
* @Embeddable
*/
Expand Down
8 changes: 6 additions & 2 deletions tests/Doctrine/Tests/ORM/Mapping/ClassMetadataBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public function testAddEmbeddedWithOnlyRequiredParams()
'columnPrefix' => null,
'declaredField' => null,
'originalField' => null,
'nullable' => null
)
), $this->cm->embeddedClasses);
}
Expand All @@ -93,6 +94,7 @@ public function testAddEmbeddedWithPrefix()
'columnPrefix' => 'nm_',
'declaredField' => null,
'originalField' => null,
'nullable' => null
)
), $this->cm->embeddedClasses);
}
Expand All @@ -110,7 +112,8 @@ public function testCreateEmbeddedWithoutExtraParams()
'class' => 'Doctrine\Tests\Models\ValueObjects\Name',
'columnPrefix' => null,
'declaredField' => null,
'originalField' => null
'originalField' => null,
'nullable' => null
),
$this->cm->embeddedClasses['name']
);
Expand All @@ -129,7 +132,8 @@ public function testCreateEmbeddedWithColumnPrefix()
'class' => 'Doctrine\Tests\Models\ValueObjects\Name',
'columnPrefix' => 'nm_',
'declaredField' => null,
'originalField' => null
'originalField' => null,
'nullable' => null
),
$this->cm->embeddedClasses['name']
);
Expand Down
1 change: 1 addition & 0 deletions tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ public function testEmbeddedMapping()
'columnPrefix' => 'nm_',
'declaredField' => null,
'originalField' => null,
'nullable' => null,
)
),
$class->embeddedClasses
Expand Down