From e3c7c6eb311a334f1afba15d5b81d3d3f1b938eb Mon Sep 17 00:00:00 2001 From: Sergei Tigrov Date: Sun, 23 Jun 2024 17:36:38 +0700 Subject: [PATCH] Replace the factory with an instance or Closure (#365) --- src/AbstractActiveRecord.php | 32 ++++++++------- src/ActiveQuery.php | 39 +++++++++++++------ src/ActiveQueryInterface.php | 10 ++++- tests/ActiveQueryTest.php | 30 ++++++++++++++ tests/ActiveRecordTest.php | 15 +++++++ tests/Stubs/ActiveRecord/Customer.php | 6 +++ .../ActiveRecord/CustomerWithConstructor.php | 4 +- 7 files changed, 109 insertions(+), 27 deletions(-) diff --git a/src/AbstractActiveRecord.php b/src/AbstractActiveRecord.php index 0be18c184..7433529fc 100644 --- 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 $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 $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 $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 $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 index b68f9a808..0b2a18567 100644 --- 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 $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 $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 index e058256f9..8fe0ed423 100644 --- 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|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 index 6d604d5af..84174f5f4 100644 --- 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 index 55f797d5b..8447e77d4 100644 --- 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 index 923c28b3e..99dfe9944 100644 --- 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 index 29a310198..48f3e4e3a 100644 --- 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