Merge pull request #509: change behavior of EntityFactory

cycle · Mar 2, 2025 · 44eaeb0 · 44eaeb0
2 parents a366d01 + cc5fc55
commit 44eaeb0
Show file tree

Hide file tree

Showing 18 changed files with 1,992 additions and 1,046 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -24,6 +24,10 @@ jobs:
           - "8.3"
           - "8.4"
     steps:
+      - name: Install ODBC driver.
+        run: |
+          sudo curl https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/prod.list | sudo tee /etc/apt/sources.list.d/mssql-release.list
+          sudo ACCEPT_EULA=Y apt-get install -y msodbcsql18
       - name: Checkout
         uses: actions/checkout@v2
       - name: Setup DB services

diff --git a/composer.json b/composer.json
@@ -51,10 +51,10 @@
         "mockery/mockery": "^1.1",
         "phpunit/phpunit": "^9.5",
         "ramsey/uuid": "^4.0",
-        "spiral/tokenizer": "^2.8 || ^3.0",
         "spiral/code-style": "~2.2.0",
+        "spiral/tokenizer": "^2.8 || ^3.0",
         "symfony/var-dumper": "^5.2 || ^6.0 || ^7.0",
-        "vimeo/psalm": "5.21"
+        "vimeo/psalm": "5.21 || ^6.8"
     },
     "autoload": {
         "psr-4": {

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
diff --git a/psalm.xml b/psalm.xml
@@ -13,17 +13,14 @@
         </ignoreFiles>
     </projectFiles>
     <issueHandlers>
+        <MissingClassConstType errorLevel="suppress" />
+        <UnusedClass errorLevel="suppress" />
         <UndefinedAttributeClass>
             <errorLevel type="suppress">
                 <referencedClass name="JetBrains\PhpStorm\ExpectedValues" />
                 <referencedClass name="JetBrains\PhpStorm\Deprecated" />
                 <referencedClass name="JetBrains\PhpStorm\Pure" />
             </errorLevel>
         </UndefinedAttributeClass>
-        <UndefinedClass>
-            <errorLevel type="suppress">
-                <referencedClass name="BackedEnum" />
-            </errorLevel>
-        </UndefinedClass>
     </issueHandlers>
 </psalm>
diff --git a/src/ORM.php b/src/ORM.php
@@ -16,12 +16,14 @@
 use Cycle\ORM\Service\Implementation\MapperProvider;
 use Cycle\ORM\Service\Implementation\RelationProvider;
 use Cycle\ORM\Service\Implementation\RepositoryProvider;
+use Cycle\ORM\Service\Implementation\RoleResolver;
 use Cycle\ORM\Service\Implementation\SourceProvider;
 use Cycle\ORM\Service\Implementation\TypecastProvider;
 use Cycle\ORM\Service\IndexProviderInterface;
 use Cycle\ORM\Service\MapperProviderInterface;
 use Cycle\ORM\Service\RelationProviderInterface;
 use Cycle\ORM\Service\RepositoryProviderInterface;
+use Cycle\ORM\Service\RoleResolverInterface;
 use Cycle\ORM\Service\SourceProviderInterface;
 use Cycle\ORM\Service\TypecastProviderInterface;
 use Cycle\ORM\Transaction\CommandGenerator;
@@ -38,11 +40,12 @@ final class ORM implements ORMInterface
     private RelationProvider $relationProvider;
     private SourceProvider $sourceProvider;
     private TypecastProvider $typecastProvider;
-    private EntityFactory $entityFactory;
+    private EntityFactoryInterface $entityFactory;
     private IndexProvider $indexProvider;
     private MapperProvider $mapperProvider;
     private RepositoryProvider $repositoryProvider;
     private EntityProvider $entityProvider;
+    private RoleResolverInterface $roleResolver;
 
     public function __construct(
         private FactoryInterface $factory,
@@ -57,7 +60,7 @@ public function __construct(
 
     public function resolveRole(string|object $entity): string
     {
-        return $this->entityFactory->resolveRole($entity);
+        return $this->roleResolver->resolveRole($entity);
     }
 
     public function get(string $role, array $scope, bool $load = true): ?object
@@ -242,13 +245,15 @@ private function resetRegistry(): void
             $this->factory,
         );
         $this->entityProvider = new EntityProvider($this->heap, $this->repositoryProvider);
+        $this->roleResolver = new RoleResolver($this->schema, $this->heap);
 
         $this->entityFactory = new EntityFactory(
             $this->heap,
             $this->schema,
             $this->mapperProvider,
             $this->relationProvider,
             $this->indexProvider,
+            $this->roleResolver,
         );
     }
 }
diff --git a/src/ORMInterface.php b/src/ORMInterface.php
@@ -12,6 +12,7 @@
 use Cycle\ORM\Service\MapperProviderInterface;
 use Cycle\ORM\Service\RelationProviderInterface;
 use Cycle\ORM\Service\RepositoryProviderInterface;
+use Cycle\ORM\Service\RoleResolverInterface;
 use Cycle\ORM\Service\SourceProviderInterface;
 use Cycle\ORM\Transaction\CommandGeneratorInterface;
 
@@ -25,13 +26,9 @@ interface ORMInterface extends
     MapperProviderInterface,
     RepositoryProviderInterface,
     RelationProviderInterface,
+    RoleResolverInterface,
     IndexProviderInterface
 {
-    /**
-     * Automatically resolve role based on object name or instance.
-     */
-    public function resolveRole(string|object $entity): string;
-
     /**
      * Create new entity based on given role and input data. Method will attempt to re-use
      * already loaded entity.

diff --git a/src/Service/EntityFactoryInterface.php b/src/Service/EntityFactoryInterface.php
@@ -4,16 +4,22 @@
 
 namespace Cycle\ORM\Service;
 
+use Cycle\ORM\Exception\MapperException;
 use Cycle\ORM\Heap\Node;
 
 interface EntityFactoryInterface
 {
     /**
      * Create new entity based on given role and input data.
      *
-     * @param string $role Entity role.
-     * @param array<string, mixed> $data Entity data.
+     * @template T
+     *
+     * @param non-empty-string|class-string<T> $role Entity role.
+     * @param array<non-empty-string, mixed> $data Entity data.
      * @param bool $typecast Indicates that data is raw, and typecasting should be applied.
+     *
+     * @return T
+     * @throws MapperException
      */
     public function make(string $role, array $data = [], int $status = Node::NEW, bool $typecast = false): object;
 }
diff --git a/src/Service/Implementation/EntityFactory.php b/src/Service/Implementation/EntityFactory.php
@@ -4,16 +4,16 @@
 
 namespace Cycle\ORM\Service\Implementation;
 
-use Cycle\ORM\EntityProxyInterface;
-use Cycle\ORM\Exception\ORMException;
 use Cycle\ORM\Heap\HeapInterface;
 use Cycle\ORM\Heap\Node;
+use Cycle\ORM\Reference\ReferenceInterface;
 use Cycle\ORM\Service\EntityFactoryInterface;
 use Cycle\ORM\Service\IndexProviderInterface;
 use Cycle\ORM\Service\MapperProviderInterface;
 use Cycle\ORM\Service\RelationProviderInterface;
 use Cycle\ORM\SchemaInterface;
 use Cycle\ORM\Select\LoaderInterface;
+use Cycle\ORM\Service\RoleResolverInterface;
 
 /**
  * @internal
@@ -26,6 +26,7 @@ public function __construct(
         private MapperProviderInterface $mapperProvider,
         private RelationProviderInterface $relationProvider,
         private IndexProviderInterface $indexProvider,
+        private RoleResolverInterface $roleResolver,
     ) {}
 
     public function make(
@@ -37,7 +38,7 @@ public function make(
         $role = $data[LoaderInterface::ROLE_KEY] ?? $role;
         unset($data[LoaderInterface::ROLE_KEY]);
         // Resolved role
-        $rRole = $this->resolveRole($role);
+        $rRole = $this->roleResolver->resolveRole($role);
         $relMap = $this->relationProvider->getRelationMap($rRole);
         $mapper = $this->mapperProvider->getMapper($rRole);
 
@@ -63,10 +64,25 @@ public function make(
                 $e = $this->heap->find($rRole, $ids);
 
                 if ($e !== null) {
+                    // Get not resolved relations (references)
+                    $refs = \array_filter(
+                        $mapper->fetchRelations($e),
+                        fn($v) => $v instanceof ReferenceInterface,
+                    );
+
+                    if ($refs === []) {
+                        return $e;
+                    }
+
                     $node = $this->heap->get($e);
                     \assert($node !== null);
 
-                    return $mapper->hydrate($e, $relMap->init($this, $node, $castedData));
+                    // Replace references with actual relation data
+                    return $mapper->hydrate($e, $relMap->init(
+                        $this,
+                        $node,
+                        \array_intersect_key($castedData, $refs),
+                    ));
                 }
             }
         }
@@ -79,31 +95,4 @@ public function make(
 
         return $mapper->hydrate($e, $relMap->init($this, $node, $castedData));
     }
-
-    public function resolveRole(object|string $entity): string
-    {
-        if (\is_object($entity)) {
-            $node = $this->heap->get($entity);
-            if ($node !== null) {
-                return $node->getRole();
-            }
-
-            $class = $entity::class;
-            if (!$this->schema->defines($class)) {
-                $parentClass = \get_parent_class($entity);
-
-                if ($parentClass === false
-                    || !$entity instanceof EntityProxyInterface
-                    || !$this->schema->defines($parentClass)
-                ) {
-                    throw new ORMException("Unable to resolve role of `$class`.");
-                }
-                $class = $parentClass;
-            }
-
-            $entity = $class;
-        }
-
-        return $this->schema->resolveAlias($entity) ?? throw new ORMException("Unable to resolve role `$entity`.");
-    }
 }
diff --git a/src/Service/Implementation/RoleResolver.php b/src/Service/Implementation/RoleResolver.php
@@ -0,0 +1,54 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Cycle\ORM\Service\Implementation;
+
+use Cycle\ORM\EntityProxyInterface;
+use Cycle\ORM\Exception\ORMException;
+use Cycle\ORM\Heap\HeapInterface;
+use Cycle\ORM\SchemaInterface;
+use Cycle\ORM\Service\RoleResolverInterface;
+
+/**
+ * @internal
+ */
+final class RoleResolver implements RoleResolverInterface
+{
+    private SchemaInterface $schema;
+    private HeapInterface $heap;
+
+    public function __construct(SchemaInterface $schema, HeapInterface $heap)
+    {
+        $this->schema = $schema;
+        $this->heap = $heap;
+    }
+
+    public function resolveRole(object|string $entity): string
+    {
+        if (\is_object($entity)) {
+            $node = $this->heap->get($entity);
+            if ($node !== null) {
+                return $node->getRole();
+            }
+
+            /** @var class-string $class */
+            $class = $entity::class;
+            if (!$this->schema->defines($class)) {
+                $parentClass = \get_parent_class($entity);
+
+                if ($parentClass === false
+                    || !$entity instanceof EntityProxyInterface
+                    || !$this->schema->defines($parentClass)
+                ) {
+                    throw new ORMException("Unable to resolve role of `$class`.");
+                }
+                $class = $parentClass;
+            }
+
+            $entity = $class;
+        }
+
+        return $this->schema->resolveAlias($entity) ?? throw new ORMException("Unable to resolve role `$entity`.");
+    }
+}
diff --git a/src/Service/RoleResolverInterface.php b/src/Service/RoleResolverInterface.php
@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Cycle\ORM\Service;
+
+interface RoleResolverInterface
+{
+    /**
+     * Automatically resolve role based on object name or instance.
+     *
+     * @return non-empty-string
+     */
+    public function resolveRole(string|object $entity): string;
+}
diff --git a/tests/ORM/Functional/Driver/Common/Mapper/ClasslessMapper/ClasslessMapperTest.php b/tests/ORM/Functional/Driver/Common/Mapper/ClasslessMapper/ClasslessMapperTest.php
@@ -259,11 +259,13 @@ public function testRepositoryFindOneWithWhere(): void
     public function testLoadOverwriteValues(): void
     {
         $u = $this->orm->getRepository('user')->findByPK(1);
+        $this->assertSame('[email protected]', $u->email);
         $u->email = '[email protected]';
         $this->assertSame('[email protected]', $u->email);
 
         $u2 = $this->orm->getRepository('user')->findByPK(1);
-        $this->assertSame('[email protected]', $u2->email);
+        self::assertSame($u, $u2);
+        $this->assertSame('[email protected]', $u2->email);
 
         $u3 = $this->orm->withHeap(new Heap())->getRepository('user')->findByPK(1);
         $this->assertSame('[email protected]', $u3->email);
@@ -272,10 +274,10 @@ public function testLoadOverwriteValues(): void
         $t = new Transaction($this->orm);
         $t->persist($u);
         $t->run();
-        $this->assertNumWrites(0);
+        $this->assertNumWrites(1);
 
         $u4 = $this->orm->withHeap(new Heap())->getRepository('user')->findByPK(1);
-        $this->assertSame('hello@world.com', $u4->email);
+        $this->assertSame('test@email.com', $u4->email);
     }
 
     public function testNullableValuesInASndOut(): void

diff --git a/tests/ORM/Functional/Driver/Common/Mapper/MapperTest.php b/tests/ORM/Functional/Driver/Common/Mapper/MapperTest.php
@@ -328,19 +328,17 @@ public function testLoadOverwriteValues(): void
         $this->assertSame('[email protected]', $u->email);
 
         $u2 = $this->orm->getRepository(User::class)->findByPK(1);
-        $this->assertSame('hello@world.com', $u2->email);
+        $this->assertSame('test@email.com', $u2->email);
 
         $u3 = $this->orm->withHeap(new Heap())->getRepository(User::class)->findByPK(1);
         $this->assertSame('[email protected]', $u3->email);
 
         $this->captureWriteQueries();
-        $t = new Transaction($this->orm);
-        $t->persist($u);
-        $t->run();
-        $this->assertNumWrites(0);
+        $this->save($u);
+        $this->assertNumWrites(1);
 
         $u4 = $this->orm->withHeap(new Heap())->getRepository(User::class)->findByPK(1);
-        $this->assertSame('hello@world.com', $u4->email);
+        $this->assertSame('test@email.com', $u4->email);
     }
 
     public function testNullableValuesInASndOut(): void

diff --git a/tests/ORM/Functional/Driver/Common/Relation/HasOne/HasOneCompositeKeyTest.php b/tests/ORM/Functional/Driver/Common/Relation/HasOne/HasOneCompositeKeyTest.php
@@ -528,6 +528,7 @@ public function testDoNotOverwriteRelation(): void
 
     public function testOverwritePromisedRelation(): void
     {
+        // The relation `child_entity` will not be loaded
         $u = (new Select($this->orm, CompositePK::class))->wherePK([1, 1])->fetchOne();
 
         $newCompositePKChild = new CompositePKChild();
@@ -543,12 +544,13 @@ public function testOverwritePromisedRelation(): void
             ->wherePK([1, 1])->fetchOne();
 
         $this->assertSame($u, $u2);
-        // Overwritten
-        $this->assertSame(self::CHILD_1['key3'], $u2->child_entity->key3);
+        // Relation was not overwritten
+        $this->assertSame('new', $u2->child_entity->key3);
 
         $this->captureWriteQueries();
-        (new Transaction($this->orm))->persist($u)->run();
-        $this->assertNumWrites(0);
+        $this->save($u);
+        // Add a new pivot and delete the old one
+        $this->assertNumWrites(2);
     }
 
     public function setUp(): void