From 8b686a097a65e9906f9a5ff1f1d96218447c1dce Mon Sep 17 00:00:00 2001 From: Sergei Tigrov Date: Thu, 4 Jul 2024 13:16:41 +0700 Subject: [PATCH] Refactor `populateRelation()` (#361) --- docs/create-model.md | 2 +- src/ActiveRelationTrait.php | 221 +++++++++--------------- tests/Driver/Pgsql/ActiveRecordTest.php | 31 ++++ tests/Driver/Pgsql/Stubs/Item.php | 27 +++ tests/Driver/Pgsql/Stubs/Promotion.php | 35 ++++ tests/Stubs/ActiveRecord/Item.php | 2 +- tests/data/pgsql.sql | 12 ++ 7 files changed, 187 insertions(+), 143 deletions(-) create mode 100644 tests/Driver/Pgsql/Stubs/Item.php create mode 100644 tests/Driver/Pgsql/Stubs/Promotion.php diff --git a/docs/create-model.md b/docs/create-model.md index 1c6158377..152806c1e 100644 --- a/docs/create-model.md +++ b/docs/create-model.md @@ -277,7 +277,7 @@ final class User extends ActiveRecord } ``` -Now you can use `$user->getProfile()` and `$user->getOrders()` to access the relation. +Now you can use `$user->getProfile()` and `$user->getOrders()` to access the relations. ```php use Yiisoft\ActiveRecord\ActiveQuery; diff --git a/src/ActiveRelationTrait.php b/src/ActiveRelationTrait.php index e15d8ddec..5fa26c548 100644 --- a/src/ActiveRelationTrait.php +++ b/src/ActiveRelationTrait.php @@ -17,6 +17,7 @@ use function array_diff_key; use function array_fill_keys; use function array_filter; +use function array_flip; use function array_intersect_key; use function array_keys; use function array_merge; @@ -248,20 +249,18 @@ public function populateRelation(string $name, array &$primaryModels): array } if (!$this->multiple && count($primaryModels) === 1) { - $model = $this->onePopulate(); + $models = [$this->onePopulate()]; + $this->populateInverseRelation($models, $primaryModels); + $primaryModel = reset($primaryModels); if ($primaryModel instanceof ActiveRecordInterface) { - $primaryModel->populateRelation($name, $model); + $primaryModel->populateRelation($name, $models[0]); } else { - $primaryModels[key($primaryModels)][$name] = $model; - } - - if ($this->inverseOf !== null) { - $this->populateInverseRelation($primaryModels, [$model], $name, $this->inverseOf); + $primaryModels[key($primaryModels)][$name] = $models[0]; } - return [$model]; + return $models; } /** @@ -273,6 +272,8 @@ public function populateRelation(string $name, array &$primaryModels): array $this->indexBy(null); $models = $this->all(); + $this->populateInverseRelation($models, $primaryModels); + if (isset($viaModels, $viaQuery)) { $buckets = $this->buildBuckets($models, $viaModels, $viaQuery); } else { @@ -297,51 +298,7 @@ public function populateRelation(string $name, array &$primaryModels): array $link = $this->link; } - foreach ($primaryModels as $i => $primaryModel) { - $keys = null; - - if ($this->multiple && count($link) === 1) { - $primaryModelKey = reset($link); - - if ($primaryModel instanceof ActiveRecordInterface) { - $keys = $primaryModel->getAttribute($primaryModelKey); - } else { - $keys = $primaryModel[$primaryModelKey] ?? null; - } - } - - if (is_array($keys)) { - $value = []; - - foreach ($keys as $key) { - $key = (string) $key; - - if (isset($buckets[$key])) { - $value[] = $buckets[$key]; - } - } - - if ($indexBy !== null) { - /** if indexBy is set, array_merge will cause renumbering of numeric array */ - $value = array_replace(...$value); - } else { - $value = array_merge(...$value); - } - } else { - $key = $this->getModelKey($primaryModel, $link); - $value = $buckets[$key] ?? ($this->multiple ? [] : null); - } - - if ($primaryModel instanceof ActiveRecordInterface) { - $primaryModel->populateRelation($name, $value); - } else { - $primaryModels[$i][$name] = $value; - } - } - - if ($this->inverseOf !== null) { - $this->populateInverseRelation($primaryModels, $models, $name, $this->inverseOf); - } + $this->populateRelationFromBuckets($primaryModels, $buckets, $name, $link); return $models; } @@ -350,84 +307,59 @@ public function populateRelation(string $name, array &$primaryModels): array * @throws \Yiisoft\Definitions\Exception\InvalidConfigException */ private function populateInverseRelation( - array &$primaryModels, - array $models, - string $primaryName, - string $name + array &$models, + array $primaryModels, ): void { - if (empty($models) || empty($primaryModels)) { + if ($this->inverseOf === null || empty($models) || empty($primaryModels)) { return; } + $name = $this->inverseOf; $model = reset($models); - if ($model instanceof ActiveRecordInterface) { - $this->populateInverseRelationToModels($models, $primaryModels, $name); - return; - } + /** @var ActiveQuery $relation */ + $relation = is_array($model) + ? $this->getARInstance()->relationQuery($name) + : $model->relationQuery($name); - $primaryModel = reset($primaryModels); + $link = $relation->getLink(); + $indexBy = $relation->getIndexBy(); + $buckets = $relation->buildBuckets($primaryModels); - if ($primaryModel instanceof ActiveRecordInterface) { - if ($this->multiple) { - foreach ($primaryModels as $primaryModel) { - $models = $primaryModel->relation($primaryName); - if (!empty($models)) { - $this->populateInverseRelationToModels($models, $primaryModels, $name); - $primaryModel->populateRelation($primaryName, $models); - } - } - } else { - foreach ($primaryModels as $primaryModel) { - $model = $primaryModel->relation($primaryName); - if (!empty($model)) { - $models = [$model]; - $this->populateInverseRelationToModels($models, $primaryModels, $name); - $primaryModel->populateRelation($primaryName, $models[0]); - } - } - } - } else { - if ($this->multiple) { - foreach ($primaryModels as &$primaryModel) { - if (!empty($primaryModel[$primaryName])) { - $this->populateInverseRelationToModels($primaryModel[$primaryName], $primaryModels, $name); - } - } - } else { - foreach ($primaryModels as &$primaryModel) { - if (!empty($primaryModel[$primaryName])) { - $models = [$primaryModel[$primaryName]]; - $this->populateInverseRelationToModels($models, $primaryModels, $name); - $primaryModel[$primaryName] = $models[0]; - } - } - } + if ($indexBy !== null && $relation->getMultiple()) { + $buckets = $this->indexBuckets($buckets, $indexBy); } - } - private function populateInverseRelationToModels(array &$models, array $primaryModels, string $name): void - { - $model = reset($models); - $isArray = is_array($model); + $relation->populateRelationFromBuckets($models, $buckets, $name, $link); + } - /** @var ActiveQuery $relation */ - $relation = $isArray ? $this->getARInstance()->relationQuery($name) : $model->relationQuery($name); - $buckets = $relation->buildBuckets($primaryModels); - $link = $relation->getLink(); - $default = $relation->getMultiple() ? [] : null; + private function populateRelationFromBuckets( + array &$models, + array $buckets, + string $name, + array $link + ): void { + $indexBy = $this->getIndexBy(); + $default = $this->multiple ? [] : null; + + foreach ($models as &$model) { + $keys = $this->getModelKeys($model, $link); + + /** @psalm-suppress NamedArgumentNotAllowed */ + $value = match (count($keys)) { + 0 => $default, + 1 => $buckets[$keys[0]] ?? $default, + default => !$this->multiple + ? $default + : ($indexBy !== null + ? array_replace(...array_intersect_key($buckets, array_flip($keys))) + : array_merge(...array_intersect_key($buckets, array_flip($keys)))), + }; - if ($isArray) { - /** @var array $model */ - foreach ($models as &$model) { - $key = $this->getModelKey($model, $link); - $model[$name] = $buckets[$key] ?? $default; - } - } else { - /** @var ActiveRecordInterface $model */ - foreach ($models as $model) { - $key = $this->getModelKey($model, $link); - $model->populateRelation($name, $buckets[$key] ?? $default); + if ($model instanceof ActiveRecordInterface) { + $model->populateRelation($name, $value); + } else { + $model[$name] = $value; } } } @@ -445,9 +377,13 @@ private function buildBuckets( $viaVia = null; foreach ($viaModels as $viaModel) { - $key1 = $this->getModelKey($viaModel, $viaLinkKeys); - $key2 = $this->getModelKey($viaModel, $linkValues); - $map[$key2][$key1] = true; + $key1 = $this->getModelKeys($viaModel, $viaLinkKeys); + $key2 = $this->getModelKeys($viaModel, $linkValues); + $map[] = array_fill_keys($key2, array_fill_keys($key1, true)); + } + + if (!empty($map)) { + $map = array_replace_recursive(...$map); } if ($viaQuery !== null) { @@ -473,18 +409,21 @@ private function buildBuckets( if (isset($map)) { foreach ($models as $model) { - $key = $this->getModelKey($model, $linkKeys); - if (isset($map[$key])) { - foreach (array_keys($map[$key]) as $key2) { - /** @psalm-suppress InvalidArrayOffset */ - $buckets[$key2][] = $model; - } + $keys = $this->getModelKeys($model, $linkKeys); + /** @var bool[][] $filtered */ + $filtered = array_intersect_key($map, array_fill_keys($keys, null)); + + foreach (array_keys(array_replace(...$filtered)) as $key2) { + $buckets[$key2][] = $model; } } } else { foreach ($models as $model) { - $key = $this->getModelKey($model, $linkKeys); - $buckets[$key][] = $model; + $keys = $this->getModelKeys($model, $linkKeys); + + foreach ($keys as $key) { + $buckets[$key][] = $model; + } } } @@ -503,11 +442,7 @@ private function mapVia(array $map, array $viaMap): array $resultMap = []; foreach ($map as $key => $linkKeys) { - $resultMap[$key] = []; - foreach (array_keys($linkKeys) as $linkKey) { - /** @psalm-suppress InvalidArrayOffset */ - $resultMap[$key] += $viaMap[$linkKey]; - } + $resultMap[$key] = array_replace(...array_intersect_key($viaMap, $linkKeys)); } return $resultMap; @@ -636,14 +571,16 @@ protected function filterByModels(array $models): void $this->andWhere(['in', $attributes, $values]); } - private function getModelKey(ActiveRecordInterface|array $activeRecord, array $attributes): string + private function getModelKeys(ActiveRecordInterface|array $activeRecord, array $attributes): array { $key = []; if (is_array($activeRecord)) { foreach ($attributes as $attribute) { if (isset($activeRecord[$attribute])) { - $key[] = (string) $activeRecord[$attribute]; + $key[] = is_array($activeRecord[$attribute]) + ? $activeRecord[$attribute] + : (string) $activeRecord[$attribute]; } } } else { @@ -651,15 +588,17 @@ private function getModelKey(ActiveRecordInterface|array $activeRecord, array $a $value = $activeRecord->getAttribute($attribute); if ($value !== null) { - $key[] = (string) $value; + $key[] = is_array($value) + ? $value + : (string) $value; } } } return match (count($key)) { - 0 => '', - 1 => $key[0], - default => serialize($key), + 0 => [], + 1 => is_array($key[0]) ? $key[0] : [$key[0]], + default => [serialize($key)], }; } diff --git a/tests/Driver/Pgsql/ActiveRecordTest.php b/tests/Driver/Pgsql/ActiveRecordTest.php index c6f1f37da..2ac1a5e95 100644 --- a/tests/Driver/Pgsql/ActiveRecordTest.php +++ b/tests/Driver/Pgsql/ActiveRecordTest.php @@ -7,6 +7,8 @@ use ArrayAccess; use Traversable; use Yiisoft\ActiveRecord\ActiveQuery; +use Yiisoft\ActiveRecord\ArArrayHelper; +use Yiisoft\ActiveRecord\Tests\Driver\Pgsql\Stubs\Promotion; use Yiisoft\ActiveRecord\Tests\Driver\Pgsql\Stubs\Type; use Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord\ArrayAndJsonTypes; use Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord\Beta; @@ -436,4 +438,33 @@ public function testToArrayWithClosure(): void $customer->toArray(), ); } + + public function testRelationViaArray() + { + $this->checkFixture($this->db(), 'promotion'); + + $promotionQuery = new ActiveQuery(Promotion::class); + /** @var Promotion[] $promotions */ + $promotions = $promotionQuery->with('items')->all(); + + $this->assertSame([1, 2], ArArrayHelper::getColumn($promotions[0]->getItems(), 'id')); + $this->assertSame([3, 4, 5], ArArrayHelper::getColumn($promotions[1]->getItems(), 'id')); + $this->assertSame([1, 3], ArArrayHelper::getColumn($promotions[2]->getItems(), 'id')); + $this->assertCount(0, $promotions[3]->getItems()); + + /** Test inverse relation */ + foreach ($promotions as $promotion) { + foreach ($promotion->getItems() as $item) { + $this->assertTrue($item->isRelationPopulated('promotions')); + } + } + + $this->assertSame([1, 3], ArArrayHelper::getColumn($promotions[0]->getItems()[0]->getPromotions(), 'id')); + $this->assertSame([1], ArArrayHelper::getColumn($promotions[0]->getItems()[1]->getPromotions(), 'id')); + $this->assertSame([2, 3], ArArrayHelper::getColumn($promotions[1]->getItems()[0]->getPromotions(), 'id')); + $this->assertSame([2], ArArrayHelper::getColumn($promotions[1]->getItems()[1]->getPromotions(), 'id')); + $this->assertSame([2], ArArrayHelper::getColumn($promotions[1]->getItems()[2]->getPromotions(), 'id')); + $this->assertSame([1, 3], ArArrayHelper::getColumn($promotions[2]->getItems()[0]->getPromotions(), 'id')); + $this->assertSame([2, 3], ArArrayHelper::getColumn($promotions[2]->getItems()[1]->getPromotions(), 'id')); + } } diff --git a/tests/Driver/Pgsql/Stubs/Item.php b/tests/Driver/Pgsql/Stubs/Item.php new file mode 100644 index 000000000..fcb19dc49 --- /dev/null +++ b/tests/Driver/Pgsql/Stubs/Item.php @@ -0,0 +1,27 @@ + $this->hasMany(Promotion::class, ['item_ids' => 'id']), + default => parent::relationQuery($name), + }; + } + + /** @return Promotion[] */ + public function getPromotions(): array + { + return $this->relation('promotions'); + } +} diff --git a/tests/Driver/Pgsql/Stubs/Promotion.php b/tests/Driver/Pgsql/Stubs/Promotion.php new file mode 100644 index 000000000..730728bd5 --- /dev/null +++ b/tests/Driver/Pgsql/Stubs/Promotion.php @@ -0,0 +1,35 @@ + $this->hasMany(Item::class, ['id' => 'item_ids'])->inverseOf('promotions'), + default => parent::relationQuery($name), + }; + } + + /** @return Item[] */ + public function getItems(): array + { + return $this->relation('items'); + } +} diff --git a/tests/Stubs/ActiveRecord/Item.php b/tests/Stubs/ActiveRecord/Item.php index 27c4635fb..40d149586 100644 --- a/tests/Stubs/ActiveRecord/Item.php +++ b/tests/Stubs/ActiveRecord/Item.php @@ -11,7 +11,7 @@ /** * Class Item. */ -final class Item extends ActiveRecord +class Item extends ActiveRecord { protected int $id; protected string $name; diff --git a/tests/data/pgsql.sql b/tests/data/pgsql.sql index 2ce4fe6b0..8083b5b16 100644 --- a/tests/data/pgsql.sql +++ b/tests/data/pgsql.sql @@ -28,6 +28,7 @@ DROP TABLE IF EXISTS "employee" CASCADE; DROP TABLE IF EXISTS "department" CASCADE; DROP TABLE IF EXISTS "alpha" CASCADE; DROP TABLE IF EXISTS "beta" CASCADE; +DROP TABLE IF EXISTS "promotion" CASCADE; DROP VIEW IF EXISTS "animal_view" CASCADE; DROP TABLE IF EXISTS "T_constraints_4" CASCADE; DROP TABLE IF EXISTS "T_constraints_3" CASCADE; @@ -224,6 +225,12 @@ CREATE TABLE "beta" ( PRIMARY KEY (id) ); +CREATE TABLE "promotion" ( + id serial primary key, + item_ids integer[] not null, + title varchar(126) not null +); + CREATE VIEW "animal_view" AS SELECT * FROM "animal"; INSERT INTO "animal" (type) VALUES ('Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord\Cat'); @@ -302,6 +309,11 @@ INSERT INTO "beta" (id, alpha_string_identifier) VALUES (6, '2b'); INSERT INTO "beta" (id, alpha_string_identifier) VALUES (7, '2b'); INSERT INTO "beta" (id, alpha_string_identifier) VALUES (8, '02'); +INSERT INTO "promotion" (item_ids, title) VALUES ('{1,2}', 'Discounted items'); +INSERT INTO "promotion" (item_ids, title) VALUES ('{3,4,5}', 'New arrivals'); +INSERT INTO "promotion" (item_ids, title) VALUES ('{1,3}', 'Free shipping'); +INSERT INTO "promotion" (item_ids, title) VALUES ('{}', 'Free!'); + /** * (Postgres-)Database Schema for validator tests */