Skip to content

Commit

Permalink
bug #278 Fix self-referencing entity associations (codedmonkey)
Browse files Browse the repository at this point in the history
This PR was squashed before being merged into the 1.0-dev branch (closes #278).

Discussion
----------

Fix self-referencing entity associations

Fixes faulty generation of entities with self-referencing associations #166. It looks like changes for the owning entity were overwritten by the changes for the inversed entity.

Commits
-------

bed3b44 Fix self-referencing entity associations
  • Loading branch information
weaverryan committed Oct 13, 2018
2 parents 60b0e27 + bed3b44 commit 705d3a2
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 3 deletions.
13 changes: 13 additions & 0 deletions src/Doctrine/BaseRelation.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ abstract class BaseRelation
private $propertyName;
private $targetClassName;
private $targetPropertyName;
private $isSelfReferencing = false;
private $mapInverseRelation = true;

abstract public function isOwning(): bool;
Expand Down Expand Up @@ -59,6 +60,18 @@ public function setTargetPropertyName($targetPropertyName)
return $this;
}

public function isSelfReferencing(): bool
{
return $this->isSelfReferencing;
}

public function setIsSelfReferencing(bool $isSelfReferencing)
{
$this->isSelfReferencing = $isSelfReferencing;

return $this;
}

public function getMapInverseRelation(): bool
{
return $this->mapInverseRelation;
Expand Down
14 changes: 14 additions & 0 deletions src/Doctrine/EntityRelation.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ final class EntityRelation

private $isNullable = false;

private $isSelfReferencing = false;

private $orphanRemoval = false;

private $mapInverseRelation = true;
Expand All @@ -50,6 +52,7 @@ public function __construct(string $type, string $owningClass, string $inverseCl
$this->type = $type;
$this->owningClass = $owningClass;
$this->inverseClass = $inverseClass;
$this->isSelfReferencing = $owningClass === $inverseClass;
}

public function setOwningProperty(string $owningProperty)
Expand Down Expand Up @@ -95,6 +98,7 @@ public function getOwningRelation()
->setTargetClassName($this->inverseClass)
->setTargetPropertyName($this->inverseProperty)
->setIsNullable($this->isNullable)
->setIsSelfReferencing($this->isSelfReferencing)
->setMapInverseRelation($this->mapInverseRelation)
;
break;
Expand All @@ -104,6 +108,7 @@ public function getOwningRelation()
->setTargetClassName($this->inverseClass)
->setTargetPropertyName($this->inverseProperty)
->setIsOwning(true)->setMapInverseRelation($this->mapInverseRelation)
->setIsSelfReferencing($this->isSelfReferencing)
;
break;
case self::ONE_TO_ONE:
Expand All @@ -113,6 +118,7 @@ public function getOwningRelation()
->setTargetPropertyName($this->inverseProperty)
->setIsNullable($this->isNullable)
->setIsOwning(true)
->setIsSelfReferencing($this->isSelfReferencing)
->setMapInverseRelation($this->mapInverseRelation)
;
break;
Expand All @@ -130,6 +136,7 @@ public function getInverseRelation()
->setTargetClassName($this->owningClass)
->setTargetPropertyName($this->owningProperty)
->setOrphanRemoval($this->orphanRemoval)
->setIsSelfReferencing($this->isSelfReferencing)
;
break;
case self::MANY_TO_MANY:
Expand All @@ -138,6 +145,7 @@ public function getInverseRelation()
->setTargetClassName($this->owningClass)
->setTargetPropertyName($this->owningProperty)
->setIsOwning(false)
->setIsSelfReferencing($this->isSelfReferencing)
;
break;
case self::ONE_TO_ONE:
Expand All @@ -147,6 +155,7 @@ public function getInverseRelation()
->setTargetPropertyName($this->owningProperty)
->setIsNullable($this->isNullable)
->setIsOwning(false)
->setIsSelfReferencing($this->isSelfReferencing)
;
break;
default:
Expand Down Expand Up @@ -184,6 +193,11 @@ public function isNullable(): bool
return $this->isNullable;
}

public function isSelfReferencing(): bool
{
return $this->isSelfReferencing;
}

public function getMapInverseRelation(): bool
{
return $this->mapInverseRelation;
Expand Down
9 changes: 7 additions & 2 deletions src/Maker/MakeEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,13 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
} elseif ($newField instanceof EntityRelation) {
// both overridden below for OneToMany
$newFieldName = $newField->getOwningProperty();
$otherManipulatorFilename = $this->getPathOfClass($newField->getInverseClass());
$otherManipulator = $this->createClassManipulator($otherManipulatorFilename, $io, $overwrite);
if ($newField->isSelfReferencing()) {
$otherManipulatorFilename = $entityPath;
$otherManipulator = $manipulator;
} else {
$otherManipulatorFilename = $this->getPathOfClass($newField->getInverseClass());
$otherManipulator = $this->createClassManipulator($otherManipulatorFilename, $io, $overwrite);
}
switch ($newField->getType()) {
case EntityRelation::MANY_TO_ONE:
if ($newField->getOwningClass() === $entityClassDetails->getFullName()) {
Expand Down
2 changes: 1 addition & 1 deletion src/Util/ClassSourceManipulator.php
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ private function addSingularRelation(BaseRelation $relation)

private function addCollectionRelation(BaseCollectionRelation $relation)
{
$typeHint = $this->addUseStatementIfNecessary($relation->getTargetClassName());
$typeHint = $relation->isSelfReferencing() ? 'self' : $this->addUseStatementIfNecessary($relation->getTargetClassName());

$arrayCollectionTypeHint = $this->addUseStatementIfNecessary(ArrayCollection::class);
$collectionTypeHint = $this->addUseStatementIfNecessary(Collection::class);
Expand Down
30 changes: 30 additions & 0 deletions tests/Maker/FunctionalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,36 @@ public function getCommandEntityTests()
->setRequiredPhpVersion(70100)
];

yield 'entity_many_to_one_self_referencing' => [MakerTestDetails::createTest(
$this->getMakerInstance(MakeEntity::class),
[
// entity class name
'User',
// field name
'guardian',
// add a relationship field
'relation',
// the target entity
'User',
// relation type
'ManyToOne',
// nullable
'y',
// do you want to generate an inverse relation? (default to yes)
'',
// field name on opposite side
'dependants',
// orphanRemoval (default to no)
'',
// finish adding fields
'',
])
->setFixtureFilesPath(__DIR__.'/../fixtures/MakeEntitySelfReferencing')
->configureDatabase()
->updateSchemaAfterCommand()
->setRequiredPhpVersion(70100)
];

yield 'entity_one_to_many_simple' => [MakerTestDetails::createTest(
$this->getMakerInstance(MakeEntity::class),
[
Expand Down
53 changes: 53 additions & 0 deletions tests/fixtures/MakeEntitySelfReferencing/src/Entity/User.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;

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

/**
* @ORM\Column(type="string", length=255, nullable=true)
*/
private $firstName;

/**
* @ORM\Column(type="datetime", nullable=true)
*/
private $createdAt;

public function getId()
{
return $this->id;
}

public function getFirstName()
{
return $this->firstName;
}

public function setFirstName(?string $firstName)
{
$this->firstName = $firstName;
}

public function getCreatedAt(): ?\DateTimeInterface
{
return $this->createdAt;
}

public function setCreatedAt(?\DateTimeInterface $createdAt)
{
$this->createdAt = $createdAt;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php

namespace App\Tests;

use Doctrine\Common\Collections\ArrayCollection;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
use Doctrine\ORM\EntityManager;
use App\Entity\User;

class GeneratedEntityTest extends KernelTestCase
{
public function testGeneratedEntity()
{
self::bootKernel();
/** @var EntityManager $em */
$em = self::$kernel->getContainer()
->get('doctrine')
->getManager();

$em->createQuery('DELETE FROM App\\Entity\\User u')->execute();

$user = new User();
// check that the constructor was instantiated properly
$this->assertInstanceOf(ArrayCollection::class, $user->getDependants());
// set existing field
$user->setFirstName('Ryan');
$em->persist($user);

$ward = new User();
$ward->setFirstName('Tim');
$ward->setGuardian($user);
$em->persist($ward);

// set via the inverse side
$ward2 = new User();
$ward2->setFirstName('Fabien');
$user->addDependant($ward2);
$em->persist($ward2);

$em->flush();
$em->refresh($user);

$actualUser = $em->getRepository(User::class)
->findAll();

$this->assertCount(3, $actualUser);
$this->assertCount(2, $actualUser[0]->getDependants());
}
}

0 comments on commit 705d3a2

Please sign in to comment.