diff --git a/src/Query.php b/src/Query.php index e1b47f31..d83b1019 100644 --- a/src/Query.php +++ b/src/Query.php @@ -333,7 +333,7 @@ public function getSelectBase(): Select $visibilityFilter = FilterProcessor::assembleFilter( $this->getResolver()->qualifyFilter( $this->getResolver()->getVisibilityFilter($this->getModel()), - $this->getModel() + ...[$this->getModel()->getTableAlias() => $this->getModel()] ) ); if ($visibilityFilter) { @@ -516,15 +516,21 @@ public function assembleSelect(): Select foreach ($relation->resolve() as $targetRelation => [$source, $target, $relatedKeys]) { if (is_int($targetRelation)) { $targetRelation = $relation; + $relationFilter = Filter::any(); trigger_error(sprintf( 'Relation implementation of %s::resolve() returned a numeric key for the target' . ' relation. This is deprecated and will be removed in a future version. Please return' . ' the target relation as key instead.', $relation::class ), E_USER_DEPRECATED); + } else { + /** @var Relation $targetRelation */ + $relationFilter = $resolver->qualifyFilter( + $targetRelation->getFilter(), + ...$targetRelation->getFilterSubjects() + ); } - /** @var Relation $targetRelation */ /** @var Model $source */ /** @var Model $target */ @@ -541,8 +547,11 @@ public function assembleSelect(): Select } $visibilityConditions = FilterProcessor::assembleFilter(Filter::all( - $resolver->qualifyFilter($targetRelation->getFilter(), $targetRelation), - $resolver->qualifyFilter($resolver->getVisibilityFilter($target), $target) + $relationFilter, + $resolver->qualifyFilter( + $resolver->getVisibilityFilter($target), + ...[$target->getTableAlias() => $target] + ) )); if ($visibilityConditions) { $conditions[] = $visibilityConditions; @@ -642,27 +651,38 @@ public function createSubQuery(Model $target, string $targetPath, ?Model $from = ->setDb($this->getDb()) ->setModel($target); - $sourceParts = array_reverse(explode('.', $targetPath)); - $sourceParts[0] = $target->getTableAlias(); - $subQueryResolver = $subQuery->getResolver(); - $sourcePath = join('.', $sourceParts); - - $originalRelations = iterator_to_array($this->getResolver()->resolveRelations($targetPath, $from), false); - foreach ($subQuery->getResolver()->resolveRelations($sourcePath) as $relation) { - $original = array_pop($originalRelations); - if ($relation instanceof BelongsToMany) { - $relation->setFilter($original->getThroughFilter()); - $relation->setThroughFilter($original->getFilter()); - } else { - $relation->setFilter($original->getFilter()); + $sourceParts = []; + foreach ($this->getResolver()->resolveRelations($targetPath, $from) as $relationPath => $relation) { + $predecessor = array_slice(explode('.', $relationPath), -2, 1)[0]; + foreach ($relation->reverse($subQueryResolver) as $oppositeRelation) { + if ( + $relation->getReverseName() === null + && $predecessor !== $oppositeRelation->getName() + && $oppositeRelation->getName() === $oppositeRelation->getTarget()->getTableAlias() + ) { + trigger_error(sprintf( + 'Relation "%s" still uses the default table alias during reversal.' + . ' Use `%s::setReverseName("%s")` to get rid of this deprecation notice.', + $relationPath, + $relation::class, + $predecessor + ), E_USER_DEPRECATED); + $oppositeRelation->setName($predecessor); + array_unshift($sourceParts, $predecessor); + } else { + array_unshift($sourceParts, $oppositeRelation->getName()); + } } - - $subQueryTarget = $relation->getTarget(); } - $subQuery->utilize($sourcePath); // TODO: Don't join if there's a matching foreign key + array_unshift($sourceParts, $target->getTableAlias()); + $sourcePath = join('.', $sourceParts); + $subQueryTarget = $subQueryResolver->resolveRelation($sourcePath)->getTarget(); + + // Up until here only the required relations are eagerly registered but not used yet + $subQuery->utilize($sourcePath); if (! $link) { $subQuery->columns(array_map(function ($keyName) use ($sourcePath) { diff --git a/src/Relation.php b/src/Relation.php index d9a9d9f8..578463ca 100644 --- a/src/Relation.php +++ b/src/Relation.php @@ -5,6 +5,8 @@ use Generator; use ipl\Stdlib\Filter; use ipl\Stdlib\Filter\Rule; +use LogicException; +use RuntimeException; use UnexpectedValueException; /** @@ -16,6 +18,12 @@ class Relation /** @var string Name of the relation */ protected $name; + /** @var ?string Name of the reversed relation */ + protected ?string $reverseName = null; + + /** @var ?class-string The class to reverse the relation */ + protected ?string $reverseClass = null; + /** @var Model Source model */ protected $source; @@ -43,6 +51,12 @@ class Relation /** @var ?Filter\Chain Additional JOIN conditions */ protected ?Filter\Chain $filter = null; + /** @var ?array Models additional JOIN conditions may reference, keyed by their alias */ + protected ?array $filterSubjects = null; + + /** @var ?string The name of the relation prior reversal */ + private ?string $forwardRelationName = null; + /** * Get the default column name(s) in the source table used to match the foreign key * @@ -112,6 +126,56 @@ public function setName(string $name): static return $this; } + /** + * Get the reverse name of the relation + * + * @return ?string + */ + public function getReverseName(): ?string + { + return $this->reverseName; + } + + /** + * Set the reverse name of the relation + * + * The source's table alias is used by default. + * + * @param string $name + * + * @return $this + */ + public function setReverseName(string $name): static + { + $this->reverseName = $name; + + return $this; + } + + /** + * Get the class to reverse the relation + * + * @return class-string + */ + public function getReverseClass(): string + { + return $this->reverseClass ?? static::class; + } + + /** + * Set the class to reverse the relation + * + * @param class-string $reverseClass + * + * @return $this + */ + public function setReverseClass(string $reverseClass): static + { + $this->reverseClass = $reverseClass; + + return $this; + } + /** * Get the source model of the relation * @@ -298,6 +362,34 @@ public function setFilter(Filter\Rule $filter): static return $this; } + /** + * Get subjects the relation filter may reference + * + * @return array + */ + public function getFilterSubjects(): array + { + return $this->filterSubjects ?? throw new LogicException(sprintf( + 'Cannot get filter subjects of an unbound relation. Please call %s::bindTo() first.', + static::class + )); + } + + /** + * Add subjects the relation filter may reference, while keeping existing ones + * + * @param array ...$subjects + * + * @return $this + */ + public function addFilterSubjects(Model ...$subjects): static + { + $this->filterSubjects ??= []; + $this->filterSubjects += $subjects; + + return $this; + } + /** * Determine the candidate key-foreign key construct of the relation * @@ -348,13 +440,45 @@ public function determineKeys(Model $source): array return array_combine($foreignKey, $candidateKey); } + /** + * Bind the relation to the given source using the passed resolver + * + * @param Model $source The model to use as source + * @param string $path The path the relation has been resolved at + * @param Resolver $resolver The resolver to register the relation's target alias + * + * @return $this + */ + public function bindTo(Model $source, string $path, Resolver $resolver): static + { + $this->setSource($source); + $target = $this->getTarget(); + + $subjects = [ + $this->getName() => $target, + $target->getTableAlias() => $target, + $source->getTableAlias() => $source + ]; + if ($this->forwardRelationName !== null) { + $subjects[$this->forwardRelationName] = $source; + } + + $this->addFilterSubjects(...$subjects); + + $resolver->resolveRelationFilter($this->getFilter(), $this->getName(), ...$subjects); + $resolver->setAlias($target, str_replace('.', '_', $path)); + + return $this; + } + /** * Resolve the relation * * Yields the relation to join as key and a three-element array consisting of the source model, * target model and the join keys as value. * - * @return Generator}, void> + * @return Generator}, void> + * @phpstan-return Generator}, mixed, void> */ public function resolve(): Generator { @@ -362,4 +486,78 @@ public function resolve(): Generator yield $this => [$source, $this->getTarget(), $this->determineKeys($source)]; } + + /** + * Reverse the relation + * + * Uses the passed resolver to eagerly register missing relations on the reversed path. + * + * @param Resolver $resolver + * + * @return Generator + * @phpstan-return Generator + * + * @throws LogicException In case the relation is not bound yet (has no source) or has already been reversed + * @throws RuntimeException In case the model of the forward relation is incompatible with the reversed relation's + */ + public function reverse(Resolver $resolver): Generator + { + if ($this->getSource() === null) { + throw new LogicException('Cannot reverse an unbound relation.'); + } elseif (isset($this->forwardRelationName)) { + throw new LogicException('Cannot undo a reverse.'); + } + + $reverseName = $this->getReverseName() ?? $this->getSource()->getTableAlias(); + + $targetRelations = $resolver->getRelations($this->getTarget()); + if ($targetRelations->has($reverseName)) { + // Explicit reverse relations must be properly set up with corresponding key pairs + $relation = $targetRelations->get($reverseName); + + if (! $this->getSource() instanceof ($relation->getTargetClass())) { + throw new RuntimeException(sprintf( + 'The source model of the relation "%s" (%s) is not compatible' + . ' with the target model of the inverse relation (%s)', + $this->getName(), + get_class($this->getSource()), + $relation->getTargetClass() + )); + } + } else { + // Eagerly create the relation in case it's only necessary during reversal + $relation = $targetRelations->create( + $this->getReverseClass(), + $reverseName, + get_class($this->getSource()) + ); + + // Pass on custom configuration + $relation->setCandidateKey($this->getForeignKey()); + $relation->setForeignKey($this->getCandidateKey()); + $relation->setJoinType($this->getJoinType()); + } + + // The previous relation name must be kept for reference as relation filters + // may require it but need to be resolved to the source model instead. + $relation->forwardRelationName = $this->getName(); + + $relation->setTarget($this->getSource()); // Propagates the same instance + + if (! $this->getFilter()->isEmpty()) { + // Do not override set filters with an empty set, however, if the set is not empty + // the forward relation is expected to carry the same semantics as the inverse. + $relation->setFilter(clone $this->getFilter()); + } + + yield $relation; + + if (! $targetRelations->has($relation->getName())) { + /** + * This is done after `yield` so that the backwards compatibility branch + * of {@see Query::createSubQuery()} is able to change the name. + */ + $targetRelations->add($relation); + } + } } diff --git a/src/Relation/BelongsTo.php b/src/Relation/BelongsTo.php index 1982f10b..61419fce 100644 --- a/src/Relation/BelongsTo.php +++ b/src/Relation/BelongsTo.php @@ -10,4 +10,6 @@ class BelongsTo extends Relation { protected bool $inverse = true; + + protected ?string $reverseClass = HasMany::class; } diff --git a/src/Relation/BelongsToMany.php b/src/Relation/BelongsToMany.php index bf570f63..9aa161a6 100644 --- a/src/Relation/BelongsToMany.php +++ b/src/Relation/BelongsToMany.php @@ -6,9 +6,11 @@ use ipl\Orm\Model; use ipl\Orm\Relation; use ipl\Orm\Relations; +use ipl\Orm\Resolver; use ipl\Stdlib\Filter; use ipl\Stdlib\Filter\Rule; use LogicException; +use RuntimeException; /** * Many-to-many relationship @@ -38,6 +40,9 @@ class BelongsToMany extends Relation /** @var ?Filter\Chain Additional JOIN conditions for the join table */ protected ?Filter\Chain $throughFilter = null; + /** @var ?array Models additional join table conditions may reference, keyed by their alias */ + protected ?array $throughFilterSubjects = null; + /** * Get the name of the join table or junction model class * @@ -142,11 +147,11 @@ public function getTargetForeignKey(): string|array|null /** * Set the column name(s) of the target model's foreign key found in the join table * - * @param string|array $targetForeignKey Array if the foreign key is compound, string otherwise + * @param string|array|null $targetForeignKey Array if the foreign key is compound, string otherwise * * @return $this */ - public function setTargetForeignKey(string|array $targetForeignKey): static + public function setTargetForeignKey(string|array|null $targetForeignKey): static { $this->targetForeignKey = $targetForeignKey; @@ -166,11 +171,11 @@ public function getTargetCandidateKey(): string|array|null /** * Set the candidate key column name(s) in the target table which references the target foreign key * - * @param string|array $targetCandidateKey Array if the foreign key is compound, string otherwise + * @param string|array|null $targetCandidateKey Array if the foreign key is compound, string otherwise * * @return $this */ - public function setTargetCandidateKey(string|array $targetCandidateKey): static + public function setTargetCandidateKey(string|array|null $targetCandidateKey): static { $this->targetCandidateKey = $targetCandidateKey; @@ -211,6 +216,61 @@ public function setThroughFilter(Filter\Rule $filter): static return $this; } + /** + * Get subjects the join table filter may reference + * + * @return array + */ + public function getThroughFilterSubjects(): array + { + return $this->throughFilterSubjects ?? throw new LogicException(sprintf( + 'Cannot get filter subjects of an unbound relation. Please call %s::bindTo() first.', + static::class + )); + } + + /** + * Add subjects the join table filter may reference, while keeping existing ones + * + * @param array ...$subjects + * + * @return $this + */ + public function addThroughFilterSubjects(Model ...$subjects): static + { + $this->throughFilterSubjects ??= []; + $this->throughFilterSubjects += $subjects; + + return $this; + } + + public function bindTo(Model $source, string $path, Resolver $resolver): static + { + // Allow to reference the join table in the second hop + $this->addFilterSubjects(...[$this->getThroughAlias() => $this->getThrough()]); + + parent::bindTo($source, $path, $resolver); + + $this->addThroughFilterSubjects(...[ + $this->getSource()->getTableAlias() => $this->getSource(), + $this->getThrough()->getTableAlias() => $this->getThrough(), + $this->getThroughAlias() => $this->getThrough() + ]); + + $resolver->resolveRelationFilter( + $this->getThroughFilter(), + $this->getThroughAlias(), + ...$this->getThroughFilterSubjects() + ); + + $resolver->setAlias($this->getThrough(), join('_', array_merge( + array_slice(explode('.', $path), 0, -1), + [$this->getThroughAlias()] + ))); + + return $this; + } + public function resolve(): Generator { $source = $this->getSource(); @@ -250,6 +310,7 @@ public function resolve(): Generator ->setSource($source) ->setTarget($junction) ->setFilter($this->getThroughFilter()) + ->addFilterSubjects(...$this->getThroughFilterSubjects()) ->setCandidateKey($this->extractKey($possibleCandidateKey)) ->setForeignKey($this->extractKey($possibleForeignKey)) ->setJoinType($this->getJoinType()); @@ -262,6 +323,7 @@ public function resolve(): Generator ->setSource($junction) ->setTarget($target) ->setFilter($this->getFilter()) + ->addFilterSubjects(...$this->getFilterSubjects()) ->setCandidateKey($this->extractKey($possibleTargetCandidateKey)) ->setForeignKey($this->extractKey($possibleTargetForeignKey)) ->setJoinType($this->getJoinType()); @@ -269,6 +331,41 @@ public function resolve(): Generator yield from $toTarget->resolve(); } + public function reverse(Resolver $resolver): Generator + { + foreach (parent::reverse($resolver) as $relation) { + if ($relation->getThroughClass() !== null && $relation->getThroughClass() !== $this->getThroughClass()) { + throw new RuntimeException(sprintf( + 'The junction model of the relation "%s" (%s) is not compatible' + . ' with the junction model of the inverse relation (%s != %s)', + $this->getName(), + get_class($relation->getSource()), + $relation->getThroughClass(), + $this->getThroughClass() + )); + } + + $relation->through($this->getThroughClass()); + $relation->setThrough($this->getThrough()); + $relation->setThroughAlias($this->getThroughAlias()); + + if (! $this->getThroughFilter()->isEmpty()) { + $relation->setThroughFilter(clone $this->getThroughFilter()); + } + + yield $relation; + + if (! $resolver->getRelations($this->getTarget())->has($relation->getName())) { + // The relation is eagerly set up and thus needs proper key pairs, + // but reversed as only the forward relation's pairs are known. + $relation->setCandidateKey($this->getTargetCandidateKey()); + $relation->setForeignKey($this->getTargetForeignKey()); + $relation->setTargetCandidateKey($this->getCandidateKey()); + $relation->setTargetForeignKey($this->getForeignKey()); + } + } + } + protected function extractKey(array $possibleKey): string|array|null { $filtered = array_filter($possibleKey); diff --git a/src/Relation/HasMany.php b/src/Relation/HasMany.php index 13d1abc4..82ca669e 100644 --- a/src/Relation/HasMany.php +++ b/src/Relation/HasMany.php @@ -10,4 +10,6 @@ class HasMany extends Relation { protected bool $isOne = false; + + protected ?string $reverseClass = BelongsTo::class; } diff --git a/src/Relation/HasOne.php b/src/Relation/HasOne.php index 8f7a802a..fe56aa4b 100644 --- a/src/Relation/HasOne.php +++ b/src/Relation/HasOne.php @@ -9,4 +9,5 @@ */ class HasOne extends Relation { + protected ?string $reverseClass = BelongsTo::class; } diff --git a/src/Resolver.php b/src/Resolver.php index 68cf0902..fb2fb862 100644 --- a/src/Resolver.php +++ b/src/Resolver.php @@ -500,37 +500,32 @@ public function qualifyPath(string $path, string $tableName): string /** * Resolve the given relation filter * - * Resolves each condition's column according to the referenced subject or, by default, the target. - * The target may also be referenced by the relation's name. + * Resolves each condition's column according to the referenced models or to the given default. * * @param Filter\Chain $filter - * @param string $name The name of the relation - * @param Model $source - * @param Model $target + * @param string $default Must be a valid subject + * @param array $subjects Models keyed by their name * * @throws InvalidArgumentException If a non-condition rule or invalid column is used in the filter */ - public function resolveRelationFilter(Filter\Chain $filter, string $name, Model $source, Model $target): void + public function resolveRelationFilter(Filter\Chain $filter, string $default, Model ...$subjects): void { - $resolveColumn = function (string $column) use ($name, $source, $target): string { + $resolveColumn = function (string $column) use ($default, $subjects): string { // A column may reference the source or target table by its alias, defaulting to the target if (str_contains($column, '.')) { [$alias, $column] = explode('.', $column, 2); } else { - $alias = $target->getTableAlias(); + $alias = $default; } - $subject = match ($alias) { - $name => $target, - $source->getTableAlias() => $source, - $target->getTableAlias() => $target, - default => throw new InvalidArgumentException(sprintf( - 'Invalid relation alias "%s" for models "%s" and "%s"', - $alias, - get_class($source), - get_class($target) + $subject = $subjects[$alias] ?? throw new InvalidArgumentException(sprintf( + 'Invalid relation alias "%s". Available options are: %s', + $alias, + join(', ', array_map( + fn($k) => sprintf('%s => %s', $k, get_class($subjects[$k])), + array_keys($subjects) )) - }; + )); if (! $subject instanceof Junction && ! $this->hasSelectableColumn($subject, $column)) { throw new InvalidArgumentException(sprintf( @@ -546,8 +541,7 @@ public function resolveRelationFilter(Filter\Chain $filter, string $name, Model foreach ($filter->yieldRules() as $rule) { if (! $rule instanceof Filter\Condition) { throw new InvalidArgumentException(sprintf( - 'Relation filter for model "%s" contains a non-condition rule of type "%s"', - get_class($target), + 'Relation filter contains a non-condition rule of type "%s"', get_class($rule) )); } @@ -566,42 +560,24 @@ public function resolveRelationFilter(Filter\Chain $filter, string $name, Model * Qualify the columns of the given filter * * @param Filter\Chain $filter - * @param Model|Relation $subject + * @param array $subjects Models keyed by their name * * @return Filter\Chain * * @throws InvalidArgumentException If a non-condition rule is used or an unknown model is referenced */ - public function qualifyFilter(Filter\Chain $filter, Model|Relation $subject): Filter\Chain + public function qualifyFilter(Filter\Chain $filter, Model ...$subjects): Filter\Chain { - $qualifyColumn = function (string $column) use ($subject): string { + $qualifyColumn = function (string $column) use ($subjects): string { [$alias, $column] = explode('.', $column, 2); - if ($subject instanceof Model) { - if ($subject->getTableAlias() !== $alias) { - throw new InvalidArgumentException(sprintf( - 'Unknown model alias "%s" for filter column "%s"', - $alias, - $column - )); - } - - return $this->qualifyColumn($column, $this->getAlias($subject)); - } + $subject = $subjects[$alias] ?? throw new InvalidArgumentException(sprintf( + 'Unknown model alias "%s" for filter column "%s"', + $alias, + $column + )); - return $this->qualifyColumn( - $column, - match ($alias) { - $subject->getSource()->getTableAlias() => $this->getAlias($subject->getSource()), - $subject->getTarget()->getTableAlias() => $this->getAlias($subject->getTarget()), - $subject->getName() => $this->getAlias($subject->getTarget()), - default => throw new InvalidArgumentException(sprintf( - 'Unknown model alias "%s" for filter column "%s"', - $alias, - $column - )) - } - ); + return $this->qualifyColumn($column, $this->getAlias($subject)); }; $filter = clone $filter; // Deep clone @@ -670,7 +646,9 @@ public function resolveRelation(string $path, ?Model $subject = null): Relation * @param string $path * @param ?Model $subject * - * @return Generator + * @return Generator + * @phpstan-return Generator + * * @throws InvalidArgumentException In case $path is not fully qualified * @throws InvalidRelationException In case a relation is unknown */ @@ -715,32 +693,10 @@ public function resolveRelations(string $path, ?Model $subject = null): Generato throw new InvalidRelationException($relationName, $target); } - $relation = $targetRelations->get($relationName); - $relation->setSource($target); - $this->resolveRelationFilter( - $relation->getFilter(), - $relationName, - $target, - $relation->getTarget() - ); + $relation = $targetRelations->get($relationName) + ->bindTo($target, $relationPath, $this); $resolvedRelations[$relationPath] = $relation; - - if ($relation instanceof BelongsToMany) { - $this->resolveRelationFilter( - $relation->getThroughFilter(), - $relationName, - $target, - $relation->getThrough() - ); - - $this->setAlias($relation->getThrough(), join('_', array_merge( - array_slice($segments, 0, -1), - [$relation->getThroughAlias()] - ))); - } - - $this->setAlias($relation->getTarget(), join('_', $segments)); } yield $relationPath => $relation; diff --git a/tests/BelongsToManyTest.php b/tests/BelongsToManyTest.php index cf37846a..0e8a4934 100644 --- a/tests/BelongsToManyTest.php +++ b/tests/BelongsToManyTest.php @@ -5,8 +5,10 @@ use ipl\Orm\Query; use ipl\Orm\Relation\BelongsToMany; use ipl\Orm\Relations; +use ipl\Orm\Resolver; use ipl\Sql\Test\SqlAssertions; use ipl\Stdlib\Filter; +use ipl\Tests\Orm\Lib\Model\Book; class BelongsToManyTest extends \PHPUnit\Framework\TestCase { @@ -40,7 +42,7 @@ public function testResolveDefaultKeys() foreach ( $relations ->get('user') - ->setSource($model) + ->bindTo($model, 'car.user', $this->createStub(Resolver::class)) ->resolve() as [$from, $to, $keys] ) { reset($keys); @@ -77,7 +79,7 @@ public function testResolveRespectsCustomKeysInTroughModels() foreach ( $relations ->get('user_custom_keys') - ->setSource($model) + ->bindTo($model, 'car.user_custom_keys', $this->createStub(Resolver::class)) ->resolve() as [$from, $to, $keys] ) { reset($keys); @@ -133,22 +135,17 @@ public function testSetThroughFilterWrapsABareConditionInAnAllChain() public function testResolveYieldsJunctionAndTargetRelationsWithTheirFiltersAndJoinType() { - $model = new Car(); - $relations = new Relations(); - $model->createRelations($relations); - - $throughFilter = Filter::equal('user_id', 5); - $targetFilter = Filter::equal('username', 'root'); + $query = (new Query())->setModel(new Car()); + $resolver = $query->getResolver(); - $relation = $relations - ->get('user') - ->setSource($model) + $resolver->getRelations($query->getModel())->get('user') ->setJoinType('LEFT') - ->setThroughFilter($throughFilter) - ->setFilter($targetFilter); + ->setThroughFilter(Filter::equal('user_id', 5)) + ->setFilter(Filter::equal('username', 'root')) + ->bindTo($query->getModel(), 'car.user', $resolver); $resolved = []; - foreach ($relation->resolve() as $key => $_) { + foreach ($resolver->resolveRelation('car.user')->resolve() as $key => $_) { $resolved[] = $key; } @@ -160,18 +157,66 @@ public function testResolveYieldsJunctionAndTargetRelationsWithTheirFiltersAndJo $this->assertSame('LEFT', $toJunction->getJoinType()); $this->assertSame('LEFT', $toTarget->getJoinType()); + // The junction sits between source and target + $this->assertSame('car', $toJunction->getSource()->getTableName()); + $this->assertSame('car_user', $toJunction->getTarget()->getTableName()); + $this->assertSame('car_user', $toTarget->getSource()->getTableName()); + $this->assertSame('user', $toTarget->getTarget()->getTableName()); + // The junction join carries the through filter ... + $throughFilter = iterator_to_array($toJunction->getFilter()->yieldRules()); + $this->assertNotEmpty($throughFilter, 'The junction join does not carry the through filter'); $this->assertSame( - [$throughFilter], - iterator_to_array($toJunction->getFilter()), - 'The junction join does not carry the through filter' + 'car_user.user_id', + $throughFilter[0]->getColumn(), + 'The through filter column is incorrectly resolved' ); // ... and the target join carries the relation filter + $relationFilter = iterator_to_array($toTarget->getFilter()->yieldRules()); + $this->assertNotEmpty($relationFilter, 'The target join does not carry the relation filter'); $this->assertSame( - [$targetFilter], - iterator_to_array($toTarget->getFilter()), - 'The target join does not carry the relation filter' + 'user.username', + $relationFilter[0]->getColumn(), + 'The relation filter column is incorrectly resolved' ); } + + public function testSetTargetForeignKeyAcceptsNull() + { + $this->assertNull((new BelongsToMany())->setTargetForeignKey(null)->getTargetForeignKey()); + } + + public function testSetTargetCandidateKeyAcceptsNull() + { + $this->assertNull((new BelongsToMany())->setTargetCandidateKey(null)->getTargetCandidateKey()); + } + + public function testReverseYieldsAnInverseBelongsToManyPreservingTheJunctionAndSwappingTheKeys() + { + $source = new Book(); + $resolver = (new Query())->setModel($source)->getResolver(); + // Book->author: many-to-many through a plain junction with explicit keys; Author declares no inverse, + // so it is created eagerly during reversal (which is where the key pairs must be exchanged) + $forward = $resolver->getRelations($source)->get('author')->bindTo($source, 'book.author', $resolver); + + $reversed = iterator_to_array($forward->reverse($resolver)); + + $this->assertCount(1, $reversed); + $inverse = $reversed[0]; + + $this->assertInstanceOf(BelongsToMany::class, $inverse); + $this->assertSame('book', $inverse->getName()); + $this->assertSame($source, $inverse->getTarget()); + + // The junction is preserved ... + $this->assertSame($forward->getThroughClass(), $inverse->getThroughClass()); + $this->assertSame($forward->getThroughAlias(), $inverse->getThroughAlias()); + + // ... and the source-side and target-side key pairs are exchanged as a whole + $this->assertSame($forward->getTargetCandidateKey(), $inverse->getCandidateKey()); + $this->assertSame($forward->getTargetForeignKey(), $inverse->getForeignKey()); + $this->assertSame($forward->getCandidateKey(), $inverse->getTargetCandidateKey()); + $this->assertSame($forward->getForeignKey(), $inverse->getTargetForeignKey()); + } } diff --git a/tests/Lib/Model/Author.php b/tests/Lib/Model/Author.php new file mode 100644 index 00000000..34c53af3 --- /dev/null +++ b/tests/Lib/Model/Author.php @@ -0,0 +1,33 @@ +author must create it eagerly, and there is + // no junction model to re-derive the keys from, so the reversed keys come solely from reverse(). + } +} diff --git a/tests/Lib/Model/Book.php b/tests/Lib/Model/Book.php new file mode 100644 index 00000000..f4910dcf --- /dev/null +++ b/tests/Lib/Model/Book.php @@ -0,0 +1,40 @@ +belongsToMany('author', Author::class) + ->through('authorship') + ->setCandidateKey('book_no') // book column + ->setForeignKey('authored_book') // junction column referencing the book + ->setTargetForeignKey('authoring') // junction column referencing the author + ->setTargetCandidateKey('author_ref'); // author column + } +} diff --git a/tests/RelationTest.php b/tests/RelationTest.php index c9ceadb3..17dfc137 100644 --- a/tests/RelationTest.php +++ b/tests/RelationTest.php @@ -2,8 +2,17 @@ namespace ipl\Tests\Orm; +use ipl\Orm\Query; use ipl\Orm\Relation; +use ipl\Orm\Relation\BelongsTo; +use ipl\Orm\Relation\HasMany; +use ipl\Orm\Relation\HasOne; +use ipl\Orm\Resolver; use ipl\Stdlib\Filter; +use ipl\Tests\Orm\Lib\Model\Department; +use ipl\Tests\Orm\Lib\Model\RestrictedUser; +use LogicException; +use RuntimeException; class RelationTest extends \PHPUnit\Framework\TestCase { @@ -206,4 +215,98 @@ public function testResolveYieldsTheRelationItselfAsKey() $this->assertSame([$relation], $keys); } + + public function testGetReverseNameReturnsNullByDefault() + { + $this->assertNull((new Relation())->getReverseName()); + } + + public function testSetReverseNameSetsTheReverseName() + { + $this->assertSame('foo', (new Relation())->setReverseName('foo')->getReverseName()); + } + + public function testGetReverseClassFallsBackToTheRelationsOwnClass() + { + $this->assertSame(Relation::class, (new Relation())->getReverseClass()); + // Subclasses provide sensible defaults + $this->assertSame(BelongsTo::class, (new HasMany())->getReverseClass()); + $this->assertSame(HasMany::class, (new BelongsTo())->getReverseClass()); + } + + public function testSetReverseClassOverridesTheDefault() + { + $this->assertSame( + HasOne::class, + (new BelongsTo())->setReverseClass(HasOne::class)->getReverseClass() + ); + } + + public function testReverseThrowsIfTheRelationIsUnbound() + { + $this->expectException(LogicException::class); + $this->expectExceptionMessage('Cannot reverse an unbound relation'); + + iterator_to_array((new HasMany())->reverse($this->createStub(Resolver::class))); + } + + public function testReverseReusesADeclaredInverseRelation() + { + $source = new Department(); + $resolver = (new Query())->setModel($source)->getResolver(); + // Binding qualifies the filter and registers the target alias, as resolveRelations() would + $forward = $resolver->getRelations($source) + ->get('employee') + ->bindTo($source, 'department.employee', $resolver); + + $resolver->getRelations($forward->getTarget()) + ->get('department') + ->setCandidateKey('office_id'); // Silly, but must be retained + + $reversed = iterator_to_array($forward->reverse($resolver)); + + $this->assertCount(1, $reversed); + $inverse = $reversed[0]; + + // Employee declares a matching belongsTo 'department' (named after the source's table alias) which + // is reused as the inverse and re-targeted at the very source instance + $this->assertSame($resolver->getRelations($forward->getTarget())->get('department'), $inverse); + $this->assertSame('office_id', $inverse->getCandidateKey()); + $this->assertSame('department', $inverse->getName()); + $this->assertSame($source, $inverse->getTarget()); + } + + public function testReverseCreatesAnInverseRelationWhenNoneIsDeclared() + { + $source = new RestrictedUser(); + $resolver = (new Query())->setModel($source)->getResolver(); + // RestrictedGroup declares no relations, so the inverse has to be created eagerly + $forward = $resolver->getRelations($source) + ->get('restricted_group') + ->bindTo($source, 'restricted_user.restricted_group', $resolver); + + $reversed = iterator_to_array($forward->reverse($resolver)); + + $this->assertCount(1, $reversed); + $inverse = $reversed[0]; + + $this->assertInstanceOf(BelongsTo::class, $inverse); + $this->assertSame('restricted_user', $inverse->getName()); + $this->assertSame($source, $inverse->getTarget()); + $this->assertInstanceOf(RestrictedUser::class, $inverse->getTarget()); + } + + public function testReverseThrowsIfADeclaredInverseTargetsAnIncompatibleModel() + { + $source = new Department(); + $forward = (new Query())->setModel($source)->getResolver()->getRelations($source)->get('employee') + ->setSource($source) + // Employee.office targets Office, but the source of this relation is a Department + ->setReverseName('office'); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('is not compatible with the target model of the inverse relation'); + + iterator_to_array($forward->reverse((new Query())->getResolver())); + } } diff --git a/tests/ResolverTest.php b/tests/ResolverTest.php index 633cdc6a..a47cd8f7 100644 --- a/tests/ResolverTest.php +++ b/tests/ResolverTest.php @@ -226,10 +226,14 @@ public function testResolveRelationFilterQualifiesTargetColumnsByDefault() $resolver = (new Query())->setModel(new Department())->getResolver(); $filter = Filter::all(Filter::equal('active', 'y'), Filter::equal('employee.role', 'lead')); - $resolver->resolveRelationFilter($filter, 'relation', new Department(), new Employee()); + $resolver->resolveRelationFilter($filter, 'relation', ...[ + 'relation' => new Employee(), + 'employee' => new Employee(), + 'department' => new Department() + ]); $columns = array_map(fn ($rule) => $rule->getColumn(), iterator_to_array($filter->yieldRules())); - $this->assertSame(['employee.active', 'employee.role'], $columns); + $this->assertSame(['relation.active', 'employee.role'], $columns); } public function testResolveRelationFilterQualifiesSourceColumns() @@ -237,7 +241,11 @@ public function testResolveRelationFilterQualifiesSourceColumns() $resolver = (new Query())->setModel(new Department())->getResolver(); $filter = Filter::all(Filter::equal('department.name', 'Engineering')); - $resolver->resolveRelationFilter($filter, 'relation', new Department(), new Employee()); + $resolver->resolveRelationFilter($filter, 'relation', ...[ + 'relation' => new Employee(), + 'employee' => new Employee(), + 'department' => new Department() + ]); $this->assertSame('department.name', iterator_to_array($filter->yieldRules())[0]->getColumn()); } @@ -247,7 +255,10 @@ public function testResolveRelationFilterQualifiesRelationColumns() $resolver = (new Query())->setModel(new Department())->getResolver(); $filter = Filter::all(Filter::equal('supplementary.name', 'Q/A')); - $resolver->resolveRelationFilter($filter, 'supplementary', new Department(), new Department()); + $resolver->resolveRelationFilter($filter, 'supplementary', ...[ + 'supplementary' => new Department(), + 'department' => new Department() + ]); $this->assertSame('supplementary.name', iterator_to_array($filter->yieldRules())[0]->getColumn()); } @@ -262,8 +273,11 @@ public function testResolveRelationFilterThrowsForAnUnknownAlias() $resolver->resolveRelationFilter( Filter::all(Filter::equal('office.city', 'London')), 'relation', - new Department(), - new Employee() + ...[ + 'relation' => new Employee(), + 'employee' => new Employee(), + 'department' => new Department() + ] ); } @@ -277,8 +291,11 @@ public function testResolveRelationFilterThrowsForANonSelectableColumn() $resolver->resolveRelationFilter( Filter::all(Filter::equal('unknown', 'x')), 'relation', - new Department(), - new Employee() + ...[ + 'relation' => new Employee(), + 'employee' => new Employee(), + 'department' => new Department() + ] ); } @@ -288,7 +305,11 @@ public function testResolveRelationFilterDoesNotValidateJunctionColumns() $junction = (new Junction())->setTableName('membership'); $filter = Filter::all(Filter::equal('membership.since', '2020')); - $resolver->resolveRelationFilter($filter, 'relation', new Department(), $junction); + $resolver->resolveRelationFilter($filter, 'relation', ...[ + 'relation' => $junction, + 'membership' => $junction, + 'department' => new Department() + ]); $this->assertSame('membership.since', iterator_to_array($filter->yieldRules())[0]->getColumn()); } @@ -302,7 +323,7 @@ public function testQualifyFilterThrowsForAnUnknownModelAlias() $query->getResolver()->qualifyFilter( Filter::all(Filter::equal('employee.active', 'y')), - $query->getModel() + ...[$query->getModel()->getTableAlias() => $query->getModel()] ); } @@ -311,7 +332,9 @@ public function testQualifyFilterDoesNotModifyTheGivenFilter() $query = (new Query())->setModel(new Department()); $original = Filter::all(Filter::equal('department.name', 'Engineering')); - $qualified = $query->getResolver()->qualifyFilter($original, $query->getModel()); + $qualified = $query->getResolver()->qualifyFilter($original, ...[ + $query->getModel()->getTableAlias() => $query->getModel() + ]); // The chain is deep cloned, hence the original is left untouched $this->assertNotSame($original, $qualified, 'The given filter has not been cloned'); diff --git a/tests/VisibilityFilterTest.php b/tests/VisibilityFilterTest.php index 26a7fcbc..10e7f958 100644 --- a/tests/VisibilityFilterTest.php +++ b/tests/VisibilityFilterTest.php @@ -5,6 +5,7 @@ use ipl\Orm\Query; use ipl\Sql\Test\SqlAssertions; use ipl\Stdlib\Filter; +use ipl\Tests\Orm\Lib\Model\Book; use ipl\Tests\Orm\Lib\Model\Department; use ipl\Tests\Orm\Lib\Model\Node; use ipl\Tests\Orm\Lib\Model\RestrictedGroup; @@ -108,6 +109,27 @@ public function testSelfReferencingRelationFilterIsAppliedToTheTarget() ); } + public function testSelfReferencingRelationFilterCanBeFilteredByItsName() + { + $query = Node::on(new TestConnection()) + ->columns('name') + ->filter(Filter::equal('child.name', 'John Doe')); + + $this->assertSql( + <<<'SQL' + SELECT node.name + FROM node + WHERE (node.deleted = ?) AND (node.id IN ((SELECT sub_node_node.id AS sub_node_node_id + FROM node sub_node + INNER JOIN node sub_node_node ON (sub_node_node.id = sub_node.parent_id) + AND ((sub_node.name = ?) AND (sub_node_node.deleted = ?)) + WHERE (sub_node.deleted = ?) AND (sub_node.name = ?)))) + SQL, + $query->assembleSelect(), + ['n', 'foo', 'n', 'n', 'John Doe'] + ); + } + public function testRelationFilterIsAppliedToJoinCondition() { $query = (new Query()) @@ -207,14 +229,54 @@ public function testBelongsToManyThroughAndRelationFiltersAreAppliedToReversedJo FROM car sub_car INNER JOIN car_user sub_car_car_user ON (sub_car_car_user.car_id = sub_car.id) - AND (sub_car.manufacturer = ?) + AND (sub_car_car_user.user_id = ?) INNER JOIN restricted_user sub_car_restricted_user ON (sub_car_restricted_user.id = sub_car_car_user.restricted_user_id) - AND (sub_car_car_user.user_id = ?) + AND (sub_car.manufacturer = ?) WHERE sub_car.model_name = ?)) SQL, $query->assembleSelect(), - ['Icinga', 5, 'volkswagen'] + [5, 'Icinga', 'volkswagen'] + ); + } + + public function testBelongsToManyWithMandatoryKeysJoinsCorrectly() + { + // Sanity anchor for the forward direction of the reversal regression below + $query = (new Query()) + ->setModel(new Book()) + ->columns('title') + ->utilize('author'); + + $this->assertSql( + 'SELECT book.title FROM book' + . ' INNER JOIN authorship book_authorship ON book_authorship.authored_book = book.book_no' + . ' INNER JOIN author book_author ON book_author.author_ref = book_authorship.authoring', + $query->assembleSelect() + ); + } + + public function testBelongsToManyWithMandatoryKeysReversesThemCorrectlyInASubQuery() + { + // Book->author uses a plain junction and non-conventional keys that must be declared explicitly. + // When reversed for the sub-query there is no junction model or default to re-derive them from, so + // the source-side and target-side key pairs must be exchanged as a whole. Regression for the bug + // where BelongsToMany::reverse() only flipped candidate<->foreign within each side. + $query = (new Query()) + ->setDb(new TestConnection()) + ->setModel(new Book()) + ->columns('title') + ->filter(Filter::equal('author.name', 'x')); + + $this->assertSql( + 'SELECT book.title FROM book WHERE book.book_no IN ((SELECT' + . ' sub_author_book.book_no AS sub_author_book_book_no FROM author sub_author' + . ' INNER JOIN authorship sub_author_authorship' + . ' ON sub_author_authorship.authoring = sub_author.author_ref' + . ' INNER JOIN book sub_author_book ON sub_author_book.book_no = sub_author_authorship.authored_book' + . ' WHERE sub_author.name = ?))', + $query->assembleSelect(), + ['x'] ); } @@ -336,6 +398,51 @@ public function testDeriveAppliesARelationFilterThatReferencesTheSourceTable() ); } + public function testSubQueryReversalEmitsADeprecationWhenARelationUsesTheDefaultReverseName() + { + // Filtering through "lead" (whose name differs from its target's table alias "employee") into the + // deeper to-many "ticket" reverses two hops. Reversing the ticket hop falls back to the source's + // table alias ("employee"), which differs from the path segment ("lead"), so a deprecation nudges + // towards setReverseName(). The produced SQL still uses the path segment for backwards compatibility. + $query = (new Query()) + ->setDb(new TestConnection()) + ->setModel(new Department()) + ->columns('name') + ->filter(Filter::equal('lead.ticket.subject', 'x')); + + $deprecations = []; + set_error_handler(function ($_, $message) use (&$deprecations) { + $deprecations[] = $message; + + return true; + }, E_USER_DEPRECATED); + + try { + $select = $query->assembleSelect(); + } finally { + restore_error_handler(); + } + + $this->assertNotEmpty($deprecations, 'Reversal did not emit a deprecation'); + $this->assertStringContainsString( + 'Relation "department.lead.ticket" still uses the default table alias during reversal', + $deprecations[0] + ); + + $this->assertSql( + 'SELECT department.name FROM department WHERE department.id IN ((SELECT' + . ' sub_ticket_lead_department.id AS sub_ticket_lead_department_id FROM ticket sub_ticket' + . ' INNER JOIN employee sub_ticket_lead ON (sub_ticket_lead.id = sub_ticket.employee_id)' + . ' AND ((sub_ticket.open = ?) AND (sub_ticket_lead.deleted = ?))' + . ' INNER JOIN department sub_ticket_lead_department' + . ' ON (sub_ticket_lead_department.id = sub_ticket_lead.department_id)' + . ' AND ((sub_ticket_lead.role = ?) AND (sub_ticket_lead_department.name = ?))' + . ' WHERE sub_ticket.subject = ?))', + $select, + ['y', 'n', 'lead', 'Engineering', 'x'] + ); + } + public function testModelVisibilityFilterColumnsAreNotValidated() { // Unlike relation filters, a model's visibility filter is not validated against selectable columns;