Proper reversal of relations in sub queries

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) {

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<self> 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<string, Model> 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<self>
+     */
+    public function getReverseClass(): string
+    {
+        return $this->reverseClass ?? static::class;
+    }
+
+    /**
+     * Set the class to reverse the relation
+     *
+     * @param class-string<self> $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<string, Model>
+     */
+    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<string, Model> ...$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,18 +440,124 @@ 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, static, array{0: Model, 1: Model, 2: array<string, string>}, void>
+     * @return Generator<mixed, static, array{0: Model, 1: Model, 2: array<string, string>}, void>
+     * @phpstan-return Generator<static, array{0: Model, 1: Model, 2: array<string, string>}, mixed, void>
      */
     public function resolve(): Generator
     {
         $source = $this->getSource();
 
         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<mixed, void, static, void>
+     * @phpstan-return Generator<void, static, mixed, void>
+     *
+     * @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);
+        }
+    }
 }

src/Relation/BelongsTo.php

@@ -10,4 +10,6 @@
 class BelongsTo extends Relation
 {
     protected bool $inverse = true;
+
+    protected ?string $reverseClass = HasMany::class;
 }