Replace the factory with an instance or Closure (#365)

yiisoft · Jun 23, 2024 · e3c7c6e · e3c7c6e
1 parent b000730
commit e3c7c6e
Show file tree

Hide file tree

Showing 7 changed files with 109 additions and 27 deletions.
diff --git a/src/AbstractActiveRecord.php b/src/AbstractActiveRecord.php
@@ -38,6 +38,8 @@
  * ActiveRecord is the base class for classes representing relational data in terms of objects.
  *
  * See {@see ActiveRecord} for a concrete implementation.
+ *
+ * @psalm-import-type ARClass from ActiveQueryInterface
  */
 abstract class AbstractActiveRecord implements ActiveRecordInterface
 {
@@ -48,7 +50,6 @@ abstract class AbstractActiveRecord implements ActiveRecordInterface
 
     public function __construct(
         private ConnectionInterface $db,
-        private ActiveRecordFactory|null $arFactory = null,
         private string $tableName = ''
     ) {
     }
@@ -261,16 +262,17 @@ public function hasAttribute(string $name): bool
      *
      * Call methods declared in {@see ActiveQuery} to further customize the relation.
      *
-     * @param string $class The class name of the related record
+     * @param ActiveRecordInterface|Closure|string $class The class name of the related record, or an instance of the
+     * related record, or a Closure to create an {@see ActiveRecordInterface} object.
      * @param array $link The primary-foreign key constraint. The keys of the array refer to the attributes of the
      * record associated with the `$class` model, while the values of the array refer to the corresponding attributes in
      * **this** AR class.
      *
      * @return ActiveQueryInterface The relational query object.
      *
-     * @psalm-param class-string<ActiveRecordInterface> $class
+     * @psalm-param ARClass $class
      */
-    public function hasMany(string $class, array $link): ActiveQueryInterface
+    public function hasMany(string|ActiveRecordInterface|Closure $class, array $link): ActiveQueryInterface
     {
         return $this->createRelationQuery($class, $link, true);
     }
@@ -299,16 +301,17 @@ public function hasMany(string $class, array $link): ActiveQueryInterface
      *
      * Call methods declared in {@see ActiveQuery} to further customize the relation.
      *
-     * @param string $class The class name of the related record.
+     * @param ActiveRecordInterface|Closure|string $class The class name of the related record, or an instance of the
+     *  related record, or a Closure to create an {@see ActiveRecordInterface} object.
      * @param array $link The primary-foreign key constraint. The keys of the array refer to the attributes of the
      * record associated with the `$class` model, while the values of the array refer to the corresponding attributes in
      * **this** AR class.
      *
      * @return ActiveQueryInterface The relational query object.
      *
-     * @psalm-param class-string<ActiveRecordInterface> $class
+     * @psalm-param ARClass $class
      */
-    public function hasOne(string $class, array $link): ActiveQueryInterface
+    public function hasOne(string|ActiveRecordInterface|Closure $class, array $link): ActiveQueryInterface
     {
         return $this->createRelationQuery($class, $link, false);
     }
@@ -319,11 +322,14 @@ public function insert(array $attributes = null): bool
     }
 
     /**
-     * @psalm-param class-string<ActiveRecordInterface> $arClass
+     * @param ActiveRecordInterface|Closure|string $arClass The class name of the related record, or an instance of the
+     * related record, or a Closure to create an {@see ActiveRecordInterface} object.
+     *
+     * @psalm-param ARClass $arClass
      */
-    public function instantiateQuery(string $arClass): ActiveQueryInterface
+    public function instantiateQuery(string|ActiveRecordInterface|Closure $arClass): ActiveQueryInterface
     {
-        return new ActiveQuery($arClass, $this->db, $this->arFactory);
+        return new ActiveQuery($arClass, $this->db);
     }
 
     /**
@@ -1071,18 +1077,18 @@ protected function setRelationDependencies(
     /**
      * Creates a query instance for `has-one` or `has-many` relation.
      *
-     * @param string $arClass The class name of the related record.
+     * @param ActiveRecordInterface|Closure|string $arClass The class name of the related record.
      * @param array $link The primary-foreign key constraint.
      * @param bool $multiple Whether this query represents a relation to more than one record.
      *
      * @return ActiveQueryInterface The relational query object.
      *
-     * @psalm-param class-string<ActiveRecordInterface> $arClass
+     * @psalm-param ARClass $arClass
 
      * {@see hasOne()}
      * {@see hasMany()}
      */
-    protected function createRelationQuery(string $arClass, array $link, bool $multiple): ActiveQueryInterface
+    protected function createRelationQuery(string|ActiveRecordInterface|Closure $arClass, array $link, bool $multiple): ActiveQueryInterface
     {
         return $this->instantiateQuery($arClass)->primaryModel($this)->link($link)->multiple($multiple);
     }

diff --git a/src/ActiveQuery.php b/src/ActiveQuery.php
@@ -102,6 +102,8 @@
  * These methods may only be called in a relational context. Same is true for {@see inverseOf()}, which marks a relation
  * as inverse of another relation and {@see onCondition()} which adds a condition that's to be added to relational
  * query join condition.
+ *
+ * @psalm-import-type ARClass from ActiveQueryInterface
  */
 class ActiveQuery extends Query implements ActiveQueryInterface
 {
@@ -111,15 +113,13 @@ class ActiveQuery extends Query implements ActiveQueryInterface
     private string|null $sql = null;
     private array|string|null $on = null;
     private array $joinWith = [];
-    private ActiveRecordInterface|null $arInstance = null;
 
     /**
-     * @psalm-param class-string<ActiveRecordInterface> $arClass
+     * @psalm-param ARClass $arClass
      */
     final public function __construct(
-        protected string $arClass,
+        protected string|ActiveRecordInterface|Closure $arClass,
         protected ConnectionInterface $db,
-        private ActiveRecordFactory|null $arFactory = null,
         private string $tableName = ''
     ) {
         parent::__construct($db);
@@ -301,7 +301,7 @@ private function removeDuplicatedModels(array $models): array
             $pks = $this->getARInstance()->primaryKey();
 
             if (empty($pks)) {
-                throw new InvalidConfigException("Primary key of '$this->arClass' can not be empty.");
+                throw new InvalidConfigException('Primary key of "' . $this->getARClassName() . '" can not be empty.');
             }
 
             foreach ($pks as $pk) {
@@ -323,7 +323,7 @@ private function removeDuplicatedModels(array $models): array
             $pks = $model->getPrimaryKey(true);
 
             if (empty($pks)) {
-                throw new InvalidConfigException("Primary key of '$this->arClass' can not be empty.");
+                throw new InvalidConfigException('Primary key of "' . $this->getARClassName() . '" can not be empty.');
             }
 
             foreach ($pks as $pk) {
@@ -803,7 +803,7 @@ public function orOnCondition(array|string $condition, array $params = []): self
 
     public function viaTable(string $tableName, array $link, callable $callable = null): self
     {
-        $arClass = $this->primaryModel ? $this->primaryModel::class : $this->arClass;
+        $arClass = $this->primaryModel ?? $this->arClass;
         $arClassInstance = new self($arClass, $this->db);
 
         /** @psalm-suppress UndefinedMethod */
@@ -882,7 +882,7 @@ public function getSql(): string|null
         return $this->sql;
     }
 
-    public function getARClass(): string|null
+    public function getARClass(): string|ActiveRecordInterface|Closure
     {
         return $this->arClass;
     }
@@ -976,16 +976,33 @@ public function sql(string|null $value): self
         return $this;
     }
 
+    public function getARClassName(): string
+    {
+        if ($this->arClass instanceof ActiveRecordInterface) {
+            return $this->arClass::class;
+        }
+
+        if ($this->arClass instanceof Closure) {
+            return ($this->arClass)($this->db)::class;
+        }
+
+        return $this->arClass;
+    }
+
     public function getARInstance(): ActiveRecordInterface
     {
-        if ($this->arFactory !== null) {
-            return $this->arFactory->createAR($this->arClass, $this->tableName, $this->db);
+        if ($this->arClass instanceof ActiveRecordInterface) {
+            return clone $this->arClass;
+        }
+
+        if ($this->arClass instanceof Closure) {
+            return ($this->arClass)($this->db);
         }
 
         /** @psalm-var class-string<ActiveRecordInterface> $class */
         $class = $this->arClass;
 
-        return new $class($this->db, null, $this->tableName);
+        return new $class($this->db, $this->tableName);
     }
 
     private function createInstance(): static

diff --git a/src/ActiveQueryInterface.php b/src/ActiveQueryInterface.php
@@ -7,6 +7,7 @@
 use Closure;
 use ReflectionException;
 use Throwable;
+use Yiisoft\Db\Connection\ConnectionInterface;
 use Yiisoft\Db\Exception\Exception;
 use Yiisoft\Db\Exception\InvalidArgumentException;
 use Yiisoft\Db\Exception\InvalidConfigException;
@@ -22,6 +23,8 @@
  * represents a relation between two active record classes and will return related records only.
  *
  * A class implementing this interface should also use {@see ActiveQueryTrait} and {@see ActiveRelationTrait}.
+ *
+ * @psalm-type ARClass = class-string<ActiveRecordInterface>|ActiveRecordInterface|Closure(ConnectionInterface):ActiveRecordInterface
  */
 interface ActiveQueryInterface extends QueryInterface
 {
@@ -298,7 +301,12 @@ public function getTablesUsedInFrom(): array;
      */
     public function getSql(): string|null;
 
-    public function getARClass(): string|null;
+    /**
+     * @return ActiveRecordInterface|Closure|string The AR class associated with this query.
+     *
+     * @psalm-return ARClass
+     */
+    public function getARClass(): string|ActiveRecordInterface|Closure;
 
     /**
      * Creates an {@see ActiveQuery} instance with a given SQL statement.

diff --git a/tests/ActiveQueryTest.php b/tests/ActiveQueryTest.php
@@ -23,6 +23,7 @@
 use Yiisoft\ActiveRecord\Tests\Support\Assert;
 use Yiisoft\ActiveRecord\Tests\Support\DbHelper;
 use Yiisoft\Db\Command\AbstractCommand;
+use Yiisoft\Db\Connection\ConnectionInterface;
 use Yiisoft\Db\Exception\Exception;
 use Yiisoft\Db\Exception\InvalidArgumentException;
 use Yiisoft\Db\Exception\InvalidCallException;
@@ -2664,4 +2665,33 @@ public function testEqual(): void
         $customerB = (new ActiveQuery(Item::class, $this->db))->findOne(1);
         $this->assertFalse($customerA->equals($customerB));
     }
+
+    public function testARClassAsString(): void
+    {
+        $query = new ActiveQuery(Customer::class, $this->db);
+
+        $this->assertSame($query->getARClass(), Customer::class);
+        $this->assertSame($query->getARClassName(), Customer::class);
+        $this->assertInstanceOf(Customer::class, $query->getARInstance());
+    }
+
+    public function testARClassAsInstance(): void
+    {
+        $customer = new Customer($this->db);
+        $query = new ActiveQuery($customer, $this->db);
+
+        $this->assertSame($query->getARClass(), $customer);
+        $this->assertSame($query->getARClassName(), Customer::class);
+        $this->assertInstanceOf(Customer::class, $query->getARInstance());
+    }
+
+    public function testARClassAsClosure(): void
+    {
+        $closure = fn (ConnectionInterface $db): Customer => new Customer($db);
+        $query = new ActiveQuery($closure, $this->db);
+
+        $this->assertSame($query->getARClass(), $closure);
+        $this->assertSame($query->getARClassName(), Customer::class);
+        $this->assertInstanceOf(Customer::class, $query->getARInstance());
+    }
 }
diff --git a/tests/ActiveRecordTest.php b/tests/ActiveRecordTest.php
@@ -948,4 +948,19 @@ public function testGetDirtyAttributesAfterFind(): void
             $customer->getDirtyAttributes(['id', 'email', 'address', 'status', 'unknown']),
         );
     }
+
+    public function testRelationWithInstance(): void
+    {
+        $this->checkFixture($this->db, 'customer');
+
+        $customerQuery = new ActiveQuery(Customer::class, $this->db);
+        $customer = $customerQuery->findOne(2);
+
+        $orders = $customer->getOrdersUsingInstance();
+
+        $this->assertTrue($customer->isRelationPopulated('ordersUsingInstance'));
+        $this->assertCount(2, $orders);
+        $this->assertSame(2, $orders[0]->getId());
+        $this->assertSame(3, $orders[1]->getId());
+    }
 }
diff --git a/tests/Stubs/ActiveRecord/Customer.php b/tests/Stubs/ActiveRecord/Customer.php
@@ -53,6 +53,7 @@ public function relationQuery(string $name): ActiveQueryInterface
             'orderItems' => $this->getOrderItemsQuery(),
             'orderItems2' => $this->getOrderItems2Query(),
             'items2' => $this->getItems2Query(),
+            'ordersUsingInstance' => $this->hasMany(new Order($this->db()), ['customer_id' => 'id']),
             default => parent::relationQuery($name),
         };
     }
@@ -261,4 +262,9 @@ public function getItems2Query(): ActiveQuery
         return $this->hasMany(Item::class, ['id' => 'item_id'])
             ->via('orderItems2');
     }
+
+    public function getOrdersUsingInstance(): array
+    {
+        return $this->relation('ordersUsingInstance');
+    }
 }
diff --git a/tests/Stubs/ActiveRecord/CustomerWithConstructor.php b/tests/Stubs/ActiveRecord/CustomerWithConstructor.php
@@ -23,9 +23,9 @@ final class CustomerWithConstructor extends ActiveRecord
     protected bool|string|null $bool_status = false;
     protected int|null $profile_id = null;
 
-    public function __construct(ConnectionInterface $db, private Aliases $aliases)
+    public function __construct(ConnectionInterface $db, string $tableName = '', private Aliases|null $aliases = null)
     {
-        parent::__construct($db);
+        parent::__construct($db, $tableName);
     }
 
     public function getTableName(): string