From 5f01c9a6261a259b2fb9658337a46395bf940f06 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Wed, 5 Jun 2024 17:20:04 +0700 Subject: [PATCH 1/6] Refactor `populateRelation()` --- src/ActiveRelationTrait.php | 212 ++++++++++++++---------------------- 1 file changed, 79 insertions(+), 133 deletions(-) diff --git a/src/ActiveRelationTrait.php b/src/ActiveRelationTrait.php index e15d8ddec..fd12a6db9 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,57 @@ 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(); + $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 ($relation->getMultiple() && $relation->getIndexBy() !== null) { + $buckets = $this->indexBuckets($buckets, $relation->getIndexBy()); } + + $relation->populateRelationFromBuckets($models, $buckets, $name, $link); } - private function populateInverseRelationToModels(array &$models, array $primaryModels, string $name): void - { - $model = reset($models); - $isArray = is_array($model); + private function populateRelationFromBuckets( + array &$models, + array $buckets, + string $name, + array $link + ): void { + $indexBy = $this->getIndexBy(); + $default = $this->multiple ? [] : null; - /** @var ActiveQuery $relation */ - $relation = $isArray ? $this->getARInstance()->relationQuery($name) : $model->relationQuery($name); - $buckets = $relation->buildBuckets($primaryModels); - $link = $relation->getLink(); - $default = $relation->getMultiple() ? [] : null; + foreach ($models as &$model) { + $keys = $this->getModelKeys($model, $link); - 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); + $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 ($model instanceof ActiveRecordInterface) { + $model->populateRelation($name, $value); + } else { + $model[$name] = $value; } } } @@ -445,9 +375,17 @@ 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); + $flags = array_fill_keys($key1, true); + + foreach ($key2 as $key) { + if (isset($map[$key])) { + $map[$key] += $flags; + } else { + $map[$key] = $flags; + } + } } if ($viaQuery !== null) { @@ -473,18 +411,22 @@ 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 */ + $keys = $this->getModelKeys($model, $linkKeys); + $filtered = array_intersect_key($map, array_flip($keys)); + + foreach ($filtered as $keys2) { + foreach (array_keys($keys2) 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; + } } } @@ -636,14 +578,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 +595,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)], }; } From 48a060d3d20d4ea4aae1e0cf3144bf57de202355 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Thu, 6 Jun 2024 09:21:11 +0700 Subject: [PATCH 2/6] Fix --- docs/create-model.md | 2 +- src/ActiveRelationTrait.php | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/create-model.md b/docs/create-model.md index 791bea951..8ddc71df4 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 fd12a6db9..be8598303 100644 --- a/src/ActiveRelationTrait.php +++ b/src/ActiveRelationTrait.php @@ -323,10 +323,11 @@ private function populateInverseRelation( : $model->relationQuery($name); $link = $relation->getLink(); + $indexBy = $relation->getIndexBy(); $buckets = $relation->buildBuckets($primaryModels); - if ($relation->getMultiple() && $relation->getIndexBy() !== null) { - $buckets = $this->indexBuckets($buckets, $relation->getIndexBy()); + if ($indexBy !== null && $relation->getMultiple()) { + $buckets = $this->indexBuckets($buckets, $indexBy); } $relation->populateRelationFromBuckets($models, $buckets, $name, $link); @@ -344,6 +345,7 @@ private function populateRelationFromBuckets( foreach ($models as &$model) { $keys = $this->getModelKeys($model, $link); + /** @psalm-suppress NamedArgumentNotAllowed */ $value = match (count($keys)) { 0 => $default, 1 => $buckets[$keys[0]] ?? $default, @@ -412,6 +414,7 @@ private function buildBuckets( if (isset($map)) { foreach ($models as $model) { $keys = $this->getModelKeys($model, $linkKeys); + /** @var bool[][] $filtered */ $filtered = array_intersect_key($map, array_flip($keys)); foreach ($filtered as $keys2) { From 48510178e1691cddcf6e1f575425020616f532a5 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sun, 9 Jun 2024 08:47:10 +0700 Subject: [PATCH 3/6] Add tests --- tests/Driver/Pgsql/ActiveRecordTest.php | 32 ++++++++++++++++++++++ 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 +++++++++ 5 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 tests/Driver/Pgsql/Stubs/Item.php create mode 100644 tests/Driver/Pgsql/Stubs/Promotion.php diff --git a/tests/Driver/Pgsql/ActiveRecordTest.php b/tests/Driver/Pgsql/ActiveRecordTest.php index 74707f18a..82540eb4a 100644 --- a/tests/Driver/Pgsql/ActiveRecordTest.php +++ b/tests/Driver/Pgsql/ActiveRecordTest.php @@ -7,6 +7,9 @@ use ArrayAccess; use Traversable; use Yiisoft\ActiveRecord\ActiveQuery; +use Yiisoft\ActiveRecord\ArArrayHelper; +use Yiisoft\ActiveRecord\Tests\Driver\Pgsql\Stubs\Item; +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; @@ -441,4 +444,33 @@ public function testToArrayWithClosure(): void $customer->toArray(), ); } + + public function testRelationViaArray() + { + $this->checkFixture($this->db, 'promotion'); + + $promotionQuery = new ActiveQuery(Promotion::class, $this->db); + /** @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 */ From 93405ec043af610f2c6cad14a078ca5cb2bbf81e Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Sun, 9 Jun 2024 01:47:32 +0000 Subject: [PATCH 4/6] Apply fixes from StyleCI --- tests/Driver/Pgsql/ActiveRecordTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Driver/Pgsql/ActiveRecordTest.php b/tests/Driver/Pgsql/ActiveRecordTest.php index 82540eb4a..5a30cfb8b 100644 --- a/tests/Driver/Pgsql/ActiveRecordTest.php +++ b/tests/Driver/Pgsql/ActiveRecordTest.php @@ -8,7 +8,6 @@ use Traversable; use Yiisoft\ActiveRecord\ActiveQuery; use Yiisoft\ActiveRecord\ArArrayHelper; -use Yiisoft\ActiveRecord\Tests\Driver\Pgsql\Stubs\Item; use Yiisoft\ActiveRecord\Tests\Driver\Pgsql\Stubs\Promotion; use Yiisoft\ActiveRecord\Tests\Driver\Pgsql\Stubs\Type; use Yiisoft\ActiveRecord\Tests\Stubs\ActiveRecord\ArrayAndJsonTypes; From 7bd8f3830d4d3bdc99b4e176107239320e731f45 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sun, 9 Jun 2024 11:16:53 +0700 Subject: [PATCH 5/6] Improve --- src/ActiveRelationTrait.php | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/ActiveRelationTrait.php b/src/ActiveRelationTrait.php index be8598303..5fa26c548 100644 --- a/src/ActiveRelationTrait.php +++ b/src/ActiveRelationTrait.php @@ -379,15 +379,11 @@ private function buildBuckets( foreach ($viaModels as $viaModel) { $key1 = $this->getModelKeys($viaModel, $viaLinkKeys); $key2 = $this->getModelKeys($viaModel, $linkValues); - $flags = array_fill_keys($key1, true); + $map[] = array_fill_keys($key2, array_fill_keys($key1, true)); + } - foreach ($key2 as $key) { - if (isset($map[$key])) { - $map[$key] += $flags; - } else { - $map[$key] = $flags; - } - } + if (!empty($map)) { + $map = array_replace_recursive(...$map); } if ($viaQuery !== null) { @@ -415,12 +411,10 @@ private function buildBuckets( foreach ($models as $model) { $keys = $this->getModelKeys($model, $linkKeys); /** @var bool[][] $filtered */ - $filtered = array_intersect_key($map, array_flip($keys)); + $filtered = array_intersect_key($map, array_fill_keys($keys, null)); - foreach ($filtered as $keys2) { - foreach (array_keys($keys2) as $key2) { - $buckets[$key2][] = $model; - } + foreach (array_keys(array_replace(...$filtered)) as $key2) { + $buckets[$key2][] = $model; } } } else { @@ -448,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; From ed0adf16945dce812ac586c8dced6b6330f7382a Mon Sep 17 00:00:00 2001 From: Tigrov Date: Thu, 4 Jul 2024 10:13:21 +0700 Subject: [PATCH 6/6] Fix test --- tests/Driver/Pgsql/ActiveRecordTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Driver/Pgsql/ActiveRecordTest.php b/tests/Driver/Pgsql/ActiveRecordTest.php index aee4ce7d6..2ac1a5e95 100644 --- a/tests/Driver/Pgsql/ActiveRecordTest.php +++ b/tests/Driver/Pgsql/ActiveRecordTest.php @@ -441,9 +441,9 @@ public function testToArrayWithClosure(): void public function testRelationViaArray() { - $this->checkFixture($this->db, 'promotion'); + $this->checkFixture($this->db(), 'promotion'); - $promotionQuery = new ActiveQuery(Promotion::class, $this->db); + $promotionQuery = new ActiveQuery(Promotion::class); /** @var Promotion[] $promotions */ $promotions = $promotionQuery->with('items')->all();