Introduce visibility filters - #165
Conversation
8b02f63 to
3a63142
Compare
3a63142 to
4b9bf83
Compare
c5bde13 to
f7a4ce3
Compare
| $source | ||
| ); | ||
| $sourceRelation = $this->getResolver()->getRelations($source)->get($relation); | ||
| $visibilityFilter = FilterProcessor::assembleFilter($this->getResolver()->qualifyFilter( |
There was a problem hiding this comment.
Found a bug with Claude's help: This creates a wrong junction table alias. Using $subQuery instead of $this fixes the issue.
| $visibilityFilter = FilterProcessor::assembleFilter($this->getResolver()->qualifyFilter( | |
| $visibilityFilter = FilterProcessor::assembleFilter($subQuery->getResolver()->qualifyFilter( |
Test for QueryTest class:
public function testDeriveQualifiesTheRelationsOwnFilterAgainstTheDerivedSubQueryNotTheOuterQuery()
{
// Employee::ticket is a hasMany with setFilter(Filter::equal('open', 'y')); Ticket::employee is the
// plain reverse belongsTo - the same shape as the relation pair involved in the bug this guards.
$employee = new Employee();
$employee->id = 1;
// Resolving "employee.ticket" on the outer query's own resolver first (as createSubQuery() does
// internally to read the relation's filter) used to leak an alias for Ticket ("employee_ticket")
// into the outer resolver. derive()'s own re-application of that filter then qualified it against that
// leaked, unrelated alias instead of the derived sub-query's alias for its own base table ("ticket"),
// producing SQL that referenced a table never joined in the actual query.
$query = (new Query())
->setDb(new TestConnection())
->setModel($employee);
$subQuery = $query->derive('ticket', $employee);
// The relation's filter is applied twice by design (once as part of the JOIN createSubQuery() builds,
// once again by derive() itself) - both applications must reference the sub-query's own alias
// ("sub_ticket"), never an alias belonging to a different, unrelated query. Employee's own visibility
// filter (deleted = 'n') also shows up in the JOIN, since Employee is a joined relation here.
$this->assertSql(
'SELECT sub_ticket.id, sub_ticket.subject, sub_ticket.open, sub_ticket.employee_id'
. ' FROM ticket sub_ticket'
. ' INNER JOIN employee sub_ticket_employee'
. ' ON (sub_ticket_employee.id = sub_ticket.employee_id)'
. ' AND ((sub_ticket.open = ?) AND (sub_ticket_employee.deleted = ?))'
. ' WHERE (sub_ticket_employee.id = ?) AND (sub_ticket.open = ?)',
$subQuery->assembleSelect(),
['y', 'n', 1, 'y']
);
}There was a problem hiding this comment.
The bug was the change in derive alone and not the incorrect resolver usage. Your test shows this as well since the ticket.open filter is applied twice. I reverted my changes in derive.
c68fe80 to
cc1a073
Compare
Visibility filters are supposed to constrain a model's query result at all times. Be it by selecting from it or joining it. Usecases such as soft-deletions are thus made possible to guarantee that ORM queries never fetch related rows.
Relation filters are supposed to constrain joined or eager loaded results, but only given the direction they were declared in. For example, if the opposite relation does not declare the same or any filter at all, joining from this side does not constrain the result the same way as the other way round.
…tic` This is the equivalent of `Relation::setFilter()` but for the junction table.
…ubjects): Filter\Chain` Utility method to qualify a simple filter using only base table columns (e.g. `'base.column'`)
…el $source, Model $target): void` Supposed to validate and resolve relation filters so that they reference a model which `qualifyFilter` can work with.
Provides access to a model's visibility filter and resolves it as well, similar to what `resolveRelationFilter` does but for relation filters.
* `getSelectBase` applies the base model's visibility filters. * `assembleSelect` applies visibility and relation filters to joins. This introduces a compatibility related change to `Relation::resolve()` which now yields a key and extending classes need to re-yield it. `BelongsToMany` is such a case and did that already for a long time. A deprecation notice will appear if a relation still yields an int. * `createSubQuery` overrides the filters of the reversed relations with those of the original path, preserving the semantics as if the results were joined normally.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cc1a073 to
b8f1837
Compare
BastianLedererIcinga
left a comment
There was a problem hiding this comment.
To test the cases where a model has a relation to its own table I had claude write
These tests
public function testSelfReferencingRelationAppliesTheTargetsVisibilityFilterToTheTarget() { $query = Node::on(new TestConnection()) ->columns('name') ->utilize('parent'); $this->assertSql(
<<<'SQL'
SELECT node.name
FROM node
LEFT JOIN node node_parent
ON (node_parent.id = node.parent_id)
AND (node_parent.deleted = ?)
WHERE node.deleted = ?
SQL,
$query->assembleSelect(),
['n', 'n']
);
}
public function testSelfReferencingRelationFilterIsAppliedToTheTarget()
{
$query = Node::on(new TestConnection())
->columns('name')
->utilize('child');
$this->assertSql(
<<<'SQL'
SELECT node.name
FROM node
INNER JOIN node node_child
ON (node_child.parent_id = node.id)
AND ((node_child.name = ?) AND (node_child.deleted = ?))
WHERE node.deleted = ?
SQL,
$query->assembleSelect(),
['foo', 'n', 'n']
);
}
Using this model
namespace ipl\Tests\Orm\Lib\Model;use ipl\Orm\Model;
use ipl\Orm\Relations;
use ipl\Stdlib\Filter;
class Node extends Model
{
public function getTableName()
{
return 'node';
}
public function getKeyName()
{
return 'id';
}
public function getColumns()
{
return [
'name',
'parent_id',
'deleted'
];
}
public function createRelations(Relations $relations)
{
$relations->belongsTo('parent', self::class)
->setCandidateKey('parent_id')
->setJoinType('LEFT');
$relations->hasMany('child', self::class)
->setForeignKey('parent_id')
->setFilter(Filter::equal('name', 'foo'));
}
public function createVisibilityFilter(Filter\Chain $filter): void
{
$filter->add(Filter::equal('deleted', 'n'));
}
}
|
|
||
| $visibilityConditions = FilterProcessor::assembleFilter(Filter::all( | ||
| $resolver->qualifyFilter($targetRelation->getFilter(), $source, $target), | ||
| $resolver->qualifyFilter($resolver->getVisibilityFilter($target), $source, $target) |
There was a problem hiding this comment.
| $resolver->qualifyFilter($resolver->getVisibilityFilter($target), $source, $target) | |
| $resolver->qualifyFilter($resolver->getVisibilityFilter($target), $target) |
When a model has a relation to its own table Resolver::qualifyFilter() will always match $source in its $qualifyColumn closure. So the relation alias is not used, instead the exact same visibility filter that is already present in the WHERE clause is added to the ON condition.
I think it's not necessary to pass $source in the first place, since the docstring of Model::createVisibilityFilter() states:
only actual columns of the model's table itself are allowed.
So removing $source should break nothing, but allows the Resolver to use the correct alias.
| } | ||
|
|
||
| $visibilityConditions = FilterProcessor::assembleFilter(Filter::all( | ||
| $resolver->qualifyFilter($targetRelation->getFilter(), $source, $target), |
There was a problem hiding this comment.
When a model has a relation to its own table and uses a relation filter, Resolver::qualifyFilter() will always use the alias of $source.
$relations->hasMany('child', self::class)
->setForeignKey('parent_id')
->setFilter(Filter::equal('name', 'foo'));
will generate this ON:
ON (node_child.parent_id = node.id) AND (node.name = ?)
where I would have expected
ON (node_child.parent_id = node.id) AND (node_child.name = ?)
Swapping the order of arguments:
$resolver->qualifyFilter($targetRelation->getFilter(), $target, $source)
fixes the case above, but it will break for cases that try to reference the source explicitly:
$relations->hasMany('child', self::class)
->setForeignKey('parent_id')
->setFilter(Filter::equal('node.name', 'foo'));
Will generate:
ON (node_child.parent_id = node.id) AND (node_child.name = ?)
But it is probably the better option to swap the arguments, since the target columns appearing in the filter is the more likely of these two cases.
|
|
||
| $originalRelations = iterator_to_array($this->getResolver()->resolveRelations($targetPath, $from), false); | ||
| foreach ($subQuery->getResolver()->resolveRelations($sourcePath) as $relation) { | ||
| $relation->setFilter(array_pop($originalRelations)->getFilter()); |
There was a problem hiding this comment.
In case of a BelongsToMany relation this drops the throughFilter().
| } | ||
|
|
||
| $visibilityConditions = FilterProcessor::assembleFilter(Filter::all( | ||
| $resolver->qualifyFilter($targetRelation->getFilter(), $source, $target), |
There was a problem hiding this comment.
If $relation is a BelongsToMany, the second HasMany relation it yields has the original $relation->filter, but the passed $source will be the junction table, and $target the target table.
So if the setFilter() call on the original BelongsToMany relation used the source table, qualifyFilter() will throw an InvalidArgumentException.
So I would suggest to always pass $relation->getSource() as a model to qualifyFilter().
You can test this by declaring the inverse relation to RestrictedUser in Car:
$relations->belongsToMany('restricted_user', RestrictedUser::class)
->through(CarUser::class);
and then running:
RestrictedUser::on(new TestConnection())
->derive("car", new RestrictedUser(["id" => 1]))
->assembleSelect();
It throws:
InvalidArgumentException: Unknown model alias "car" for filter column "manufacturer"
Overview
Adds a way to always constrain query results, independent of the filters a caller supplies — useful for row-level access rules (soft-deletes, tenant scoping, …). Two kinds of constraint are introduced:
Model::createVisibilityFilter(Filter\Chain $filter). References the model's own columns only. Applied to the base model as aWHEREclause and, whenever the model is joined, added to theJOINcondition.Relation::setFilter(Filter\Rule $filter), plusBelongsToMany::setThroughFilter()for the junction. Applied as extraJOINconditions. A relation filter may reference either the source or the target table by its alias (target by default), and the through filter the source or junction table.Filter columns must be actual columns of the referenced table and comparison values are passed as-is to the query builder (behaviors are not applied);
ExpressionInterfacevalues are supported under the same rules.How it works
Qualification happens in two stages so a relation filter can be applied unchanged regardless of join direction:
Resolver::resolveRelationFilter()(at relation-resolution time) rewrites each column to a logical alias — the source's or target's table alias.Resolver::qualifyFilter(Filter\Chain, Model ...$subjects)(at assembly time) maps those logical aliases to the runtime aliases, with both participating models in scope.Because the filter addresses tables by alias rather than by role, it resolves correctly when the
FilterProcessorreverses a join into a correlated sub-query (filtering on a to-many relation) and whenderive()lazy-loads a relation — the source and target simply swap places andqualifyFilterstill finds them.Changes
Model—createVisibilityFilter()hook (no-op by default).Relation—setFilter()/getFilter();resolve()now yields the relation to join as the generator key (a numeric key still works, with a deprecation notice).BelongsToMany—setThroughFilter()/getThroughFilter(); propagates the through/relation filters and the join type to the junction and target joins.Resolver—getVisibilityFilter()(resolve + cache a model's filter),resolveRelationFilter()andqualifyFilter(); resolves a relation's filters when resolving the relation.Query— applies the visibility filter to the baseWHEREand to joined models, and relation/through filters to theirJOINconditions; carries relation filters into reversed sub-queries andderive().Tests
Relation/BelongsToManyaccessors and forResolver::resolveRelationFilter()/qualifyFilter()/getVisibilityFilter()(target- and source-referencing columns, alias/column validation, junction handling, deep-clone).VisibilityFilterTestfor baseWHERE, joined model visibility filters, relation and through filters, a relation filter referencing the source table, and — via theFilterProcessor— the same filters propagating unchanged through the reversed joins of a sub-query.FilterProcessorTestruns against MySQL and PostgreSQL.🤖 Generated with Claude Code