Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
41 changes: 40 additions & 1 deletion src/Configurator.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,20 @@ public function initFields(EntitySchema $entity, \ReflectionClass $class, string
}

$field = $this->initField($property->getName(), $column, $class, $columnPrefix);
$field->setEntityClass($property->getDeclaringClass()->getName());
$field->setEntityClass($this->findOwningEntity($class, $property->getDeclaringClass())->getName());
$entity->getFields()->set($property->getName(), $field);
}
}

public function initRelations(EntitySchema $entity, \ReflectionClass $class): void
{
foreach ($class->getProperties() as $property) {
// ignore properties declared by parent entities
// otherwise all the relation columns declared in parent would be duplicated across all child tables in JTI
if ($this->findOwningEntity($class, $property->getDeclaringClass())->getName() !== $class->getName()) {
continue;
}
Comment on lines +137 to +139
Copy link
Member

Choose a reason for hiding this comment

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

I've added tests for this case and there is an issue when an entity extended from another entity without STI or JTI.

In thos case all the relations of the parent entity have to be cloned into the child class.

Copy link
Member

Choose a reason for hiding this comment

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

There is TableInheritance generator. It might be better to clean unnecessary relations there in cases STI or JTI

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the review.

You are right, it is quite a nasty use case though. I will take a look at it in the evening,
It makes sense to clear the entity in the inheritance generator, I will try to move the logic there.

The column ownership use cases are alright?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, columns look great. Thank you.


$metadata = $this->getPropertyMetadata($property, RelationAnnotation\RelationInterface::class);

foreach ($metadata as $meta) {
Expand Down Expand Up @@ -425,4 +431,37 @@ private function isOnInsertGeneratedField(Field $field): bool
default => $field->isPrimary(),
};
}

/**
* Function to find an owning entity class in the inheritance hierarchy.
*
* Entity classes may extend a base class and this function is needed route the properties from declaring class to the entity class.
* The function stops only when the declaring class is truly found, it does not naively stop on first entity.
* This behaviour makes it also functional in cases of Joined Table Inheritance on theoretically any number of nesting levels.
*/
private function findOwningEntity(\ReflectionClass $currentClass, \ReflectionClass $declaringClass): \ReflectionClass
{
// latest found entityClass before declaringClass
$latestEntityClass = $currentClass;

do {
// we found declaringClass in the hierarchy
// in most cases the execution will stop here in first loop
if ($currentClass->getName() === $declaringClass->getName()) {
return $latestEntityClass;
}

$currentClass = $currentClass->getParentClass();

// not possible to happen for logical reasons, but defensively check anyway
if (!$currentClass instanceof \ReflectionClass) {
return $latestEntityClass;
}

// if a currentClass in hierarchy is an entity on its own, the property belongs to that entity
if (\count($currentClass->getAttributes(Entity::class)) > 0) {
$latestEntityClass = $currentClass;
}
} while (true); // the inheritance hierarchy cannot be infinite
}
}
6 changes: 5 additions & 1 deletion src/TableInheritance.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ public function run(Registry $registry): Registry

// All child should be presented in a schema as separated entity
// Every child will be handled according its table inheritance type
\assert($child->getRole() !== null && $entity !== null && isset($parent));
// todo should $parent be not null?
// \assert(isset($parent));

\assert($child->getRole() !== null && $entity !== null);

if (!$registry->hasEntity($child->getRole())) {
$registry->register($child);

Expand Down
9 changes: 9 additions & 0 deletions tests/Annotated/Fixtures/Fixtures16/Executive.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Cycle\Annotated\Annotation\Column;
use Cycle\Annotated\Annotation\Entity;
use Cycle\Annotated\Annotation\Inheritance\JoinedTable as InheritanceJoinedTable;
use Cycle\Annotated\Annotation\Relation\HasOne;

/**
* @Entity
Expand All @@ -21,4 +22,12 @@ class Executive extends ExecutiveProxy
/** @Column(type="int") */
#[Column(type: 'int')]
public int $bonus;

/** @Column(type="int", nullable=true, typecast="int") */
#[Column(type: 'int', nullable: true, typecast: 'int')]
public ?int $added_tool_id;

/** @HasOne(target=Tool::class, innerKey="added_tool_id", outerKey="added_tool_id", nullable=true) */
#[HasOne(target: Tool::class, innerKey: 'added_tool_id', outerKey: 'added_tool_id', nullable: true)]
public Tool $addedTool;
}
16 changes: 16 additions & 0 deletions tests/Annotated/Fixtures/Fixtures16/Executive2.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

namespace Cycle\Annotated\Tests\Fixtures\Fixtures16;

use Cycle\Annotated\Annotation\Entity;
use Cycle\Annotated\Annotation\Inheritance\JoinedTable as InheritanceJoinedTable;

/**
* @Entity
* @InheritanceJoinedTable(outerKey="foo_id")
*/
#[Entity]
#[InheritanceJoinedTable(outerKey: 'foo_id')]
class Executive2 extends Executive {}
5 changes: 2 additions & 3 deletions tests/Annotated/Fixtures/Fixtures16/ExecutiveProxy.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@
/**
* This proxy class doesn't have an {@see Entity} annotation (attribute) declaration,
* and it shouldn't be presented in Schema.
* Note: this behavior might be improved. There will be added support for
* annotated base class columns without Entity annotation declaration.
* But all the classes that extend this class should contain all the fields from this class.
*/
class ExecutiveProxy extends Employee
{
/** @Column(type="string") */
/** @Column(type="string", name="proxy") */
#[Column(type: 'string', name: 'proxy')]
public ?string $proxyFieldWithAnnotation = null;

Expand Down
9 changes: 9 additions & 0 deletions tests/Annotated/Fixtures/Fixtures16/Person.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Cycle\Annotated\Annotation\Column;
use Cycle\Annotated\Annotation\Entity;
use Cycle\Annotated\Annotation\Inheritance\DiscriminatorColumn;
use Cycle\Annotated\Annotation\Relation\HasOne;

/**
* @Entity
Expand All @@ -24,6 +25,14 @@ class Person
#[Column(type: 'string')]
public string $type;

/** @Column(type="int", nullable=true, typecast="int") */
#[Column(type: 'int', nullable: true, typecast: 'int')]
public ?int $tool_id;

/** @HasOne(target=Tool::class, innerKey="id", outerKey="tool_id", nullable=true) */
#[HasOne(target: Tool::class, innerKey: 'id', outerKey: 'tool_id', nullable: true)]
public Tool $tool;

/** @Column(type="primary", name="id") */
#[Column(type: 'primary', name: 'id')]
protected int $foo_id;
Expand Down
22 changes: 22 additions & 0 deletions tests/Annotated/Fixtures/Fixtures16/Tool.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

declare(strict_types=1);

namespace Cycle\Annotated\Tests\Fixtures\Fixtures16;

use Cycle\Annotated\Annotation\Column;
use Cycle\Annotated\Annotation\Entity;
use Cycle\Annotated\Annotation\Inheritance\DiscriminatorColumn;

/**
* @Entity
* @DiscriminatorColumn(name="type")
*/
#[Entity]
#[DiscriminatorColumn(name: 'type')]
class Tool
{
/** @Column(type="primary", name="id") */
#[Column(type: 'primary', name: 'id')]
public int $tool_id;
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public function testTableInheritance(ReaderInterface $reader): void
$this->assertSame('secret', $loadedExecutive->hidden);
$this->assertSame(15000, $loadedExecutive->bonus);
$this->assertSame('executive', $loadedExecutive->getType());
$this->assertNull($loadedExecutive->proxyFieldWithAnnotation);
$this->assertSame('value', $loadedExecutive->proxyFieldWithAnnotation);
}

#[DataProvider('allReadersProvider')]
Expand Down
29 changes: 27 additions & 2 deletions tests/Annotated/Functional/Driver/Common/InheritanceTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Cycle\Annotated\Tests\Fixtures\Fixtures16\Employee;
use Cycle\Annotated\Tests\Fixtures\Fixtures16\Executive;
use Cycle\Annotated\Tests\Fixtures\Fixtures16\Person;
use Cycle\Annotated\Tests\Fixtures\Fixtures16\Tool;
use Cycle\ORM\SchemaInterface;
use Cycle\Schema\Compiler;
use Cycle\Schema\Generator\GenerateRelations;
Expand Down Expand Up @@ -69,6 +70,9 @@ public function testTableInheritance(ReaderInterface $reader): void
// Ceo - Single table inheritance {value: ceo}
// Beaver - Separate table

// Tool
$this->assertArrayHasKey('tool', $schema);

// Person
$this->assertCount(3, $schema['person'][SchemaInterface::CHILDREN]);
$this->assertEquals([
Expand All @@ -86,38 +90,56 @@ public function testTableInheritance(ReaderInterface $reader): void
// 'bonus' => 'bonus', // JTI
'preferences' => 'preferences',
'stocks' => 'stocks',
'tool_id' => 'tool_id',
// 'teethAmount' => 'teeth_amount', // Not child
], $schema['person'][SchemaInterface::COLUMNS]);
$this->assertEmpty($schema['person'][SchemaInterface::PARENT] ?? null);
$this->assertEmpty($schema['person'][SchemaInterface::PARENT_KEY] ?? null);
$this->assertSame('people', $schema['person'][SchemaInterface::TABLE]);
$this->assertCount(1, $schema['person'][SchemaInterface::RELATIONS]);

// Employee
$this->assertArrayHasKey('employee', $schema);
$this->assertCount(1, $schema['employee']);
$this->assertSame(Employee::class, $schema['employee'][SchemaInterface::ENTITY]);
$this->assertNull($schema['employee'][SchemaInterface::TABLE] ?? null);
$this->assertCount(0, $schema['employee'][SchemaInterface::RELATIONS] ?? []);

// Customer
$this->assertArrayHasKey('customer', $schema);
$this->assertCount(1, $schema['customer']);
$this->assertSame(Customer::class, $schema['customer'][SchemaInterface::ENTITY]);
$this->assertNull($schema['customer'][SchemaInterface::TABLE] ?? null);
$this->assertCount(0, $schema['customer'][SchemaInterface::RELATIONS] ?? []);

// Executive
$this->assertSame('employee', $schema['executive'][SchemaInterface::PARENT]);
$this->assertSame('foo_id', $schema['executive'][SchemaInterface::PARENT_KEY]);
$this->assertSame('executives', $schema['executive'][SchemaInterface::TABLE]);
$this->assertSame(
['bonus' => 'bonus', 'foo_id' => 'id', 'hidden' => 'hidden'],
$this->assertEquals(
[
'bonus' => 'bonus',
'proxyFieldWithAnnotation' => 'proxy',
'foo_id' => 'id',
'hidden' => 'hidden',
'added_tool_id' => 'added_tool_id',
],
$schema['executive'][SchemaInterface::COLUMNS],
);
$this->assertCount(1, $schema['executive'][SchemaInterface::RELATIONS]);

// Executive2
$this->assertSame('executive', $schema['executive2'][SchemaInterface::PARENT]);
$this->assertSame('foo_id', $schema['executive2'][SchemaInterface::PARENT_KEY]);
$this->assertEquals(['foo_id' => 'id'], $schema['executive2'][SchemaInterface::COLUMNS]);
$this->assertCount(0, $schema['executive2'][SchemaInterface::RELATIONS]);

// Ceo
$this->assertArrayHasKey('ceo', $schema);
$this->assertCount(1, $schema['ceo']);
$this->assertSame(Ceo::class, $schema['ceo'][SchemaInterface::ENTITY]);
$this->assertNull($schema['ceo'][SchemaInterface::TABLE] ?? null);
$this->assertCount(0, $schema['ceo'][SchemaInterface::RELATIONS] ?? []);

// Beaver
$this->assertEmpty($schema['beaver'][SchemaInterface::DISCRIMINATOR] ?? null);
Expand All @@ -131,7 +153,9 @@ public function testTableInheritance(ReaderInterface $reader): void
'name' => 'name',
'type' => 'type',
'hidden' => 'hidden',
'tool_id' => 'tool_id',
], $schema['beaver'][SchemaInterface::COLUMNS]);
$this->assertCount(1, $schema['beaver'][SchemaInterface::RELATIONS] ?? []);
}

public function testTableInheritanceWithIncorrectClassesOrder(): void
Expand All @@ -145,6 +169,7 @@ public function testTableInheritanceWithIncorrectClassesOrder(): void
new \ReflectionClass(Employee::class),
new \ReflectionClass(Executive::class),
new \ReflectionClass(Person::class),
new \ReflectionClass(Tool::class),
]);

$schema = (new Compiler())->compile($r, [
Expand Down