Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
],
"require": {
"php": ">=8.1",
"cycle/database": "^2.17.0",
"cycle/database": "^2.18.0",
"doctrine/instantiator": "^1.3.1 || ^2.0",
"spiral/core": "^2.8 || ^3.0"
},
Expand Down
1 change: 1 addition & 0 deletions src/Select/QueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* @method QueryBuilder where(...$args);
* @method QueryBuilder andWhere(...$args);
* @method QueryBuilder orWhere(...$args);
* @method QueryBuilder wrapWhere()
Comment thread
roxblnfk marked this conversation as resolved.
* @method QueryBuilder having(...$args);
* @method QueryBuilder andHaving(...$args);
* @method QueryBuilder orHaving(...$args);
Expand Down
89 changes: 87 additions & 2 deletions src/Select/ScopeInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,97 @@
namespace Cycle\ORM\Select;

/**
* Provides the ability to modify the selector and/or entity loader. Can be used to implement multi-table inheritance.
* Scopes attach extra query criteria to every `Select` for a given entity — typical
* use cases are soft-delete filtering, tenant isolation, default ordering, or
* inheritance discriminators.
*
* A scope is invoked once per query build, **after** any user-supplied WHERE
* conditions have already been registered on the underlying SelectQuery. That
* timing is important: see "Avoiding scope bypass via orWhere" below.
*
* The {@see apply()} method receives a {@see QueryBuilder} that proxies the
* underlying query. For scopes attached to joined loaders the builder forwards
* `where*` calls to the JOIN ON tokens (`onWhere`) rather than the top-level
* WHERE — handle accordingly.
*
* ## Registering a scope
*
* Either via schema (applied automatically by `getRepository()`):
*
* Schema::SCOPE => SoftDeleteScope::class,
*
* Or per-query on a `Select`:
*
* $select->scope(new SoftDeleteScope());
*
* ## Recommended pattern: always call `wrapWhere()` first
*
* Start `apply()` with a {@see \Cycle\Database\Query\Traits\WhereTrait::wrapWhere()}
* call, then add your conditions. This protects the scope from being bypassed by
Comment thread
roxblnfk marked this conversation as resolved.
* user-supplied `orWhere` (explained below):
*
* final class SoftDeleteScope implements ScopeInterface
* {
* public function apply(QueryBuilder $query): void
* {
* $query->wrapWhere(); // enclose user wheres
* $query->where('deleted_at', null); // scope condition outside
* }
* }
*
* `wrapWhere()` is a no-op when no WHERE tokens have been registered yet, so
* it is always safe to call.
*
* ## Why `wrapWhere()` is needed
*
* A plain top-level WHERE added by a scope can be defeated by a user `orWhere`
* because of SQL operator precedence — AND binds tighter than OR. Without
* `wrapWhere()`, given a scope that simply does `$query->where('deleted_at', null)`
* and a user query
*
* $select->scope(new SoftDeleteScope())
* ->where('id', 1)
* ->orWhere('id', 2);
*
* the resulting SQL is
*
* WHERE {id} = 1 OR {id} = 2 AND {deleted_at} IS NULL
* ≡ WHERE {id} = 1 OR ({id} = 2 AND {deleted_at} IS NULL)
*
* which returns rows matching `id = 1` regardless of `deleted_at` — the scope
* is silently bypassed on the first OR arm. With the recommended pattern above
* the same query compiles to
*
* WHERE ({id} = 1 OR {id} = 2) AND {deleted_at} IS NULL
*
* and the scope holds regardless of how user code mixes AND/OR.
*
* ## Stacking multiple scopes
*
* Each scope in a chain can call `wrapWhere()` independently — every layer
* encloses the previous accumulation, producing
*
* WHERE ((user_wheres) AND scope1_conds) AND scope2_conds
*
* which is logically equivalent to `user AND scope1 AND scope2`. Stack scopes
* via a composite/aggregating scope or by registering them at the appropriate
* layer of your application.
*
* ## Other usage
*
* Scopes are not limited to WHERE — `apply()` may also call `orderBy()`,
* `having()`, `limit()`, etc. on the builder. Loader-aware scopes can use the
* builder's `resolve()` to translate `relation.column` identifiers to proper
* SQL aliases.
*/
interface ScopeInterface
{
/**
* Configure query and loader pair using proxy strategy.
* Apply scope-specific modifications to the query builder.
*
* Called during query compilation, after any user-supplied WHERE conditions
* have been registered on the underlying SelectQuery. See the interface-level
* docblock for guidance on avoiding scope bypass via {@see QueryBuilder::wrapWhere()}.
*/
public function apply(QueryBuilder $query): void;
}
36 changes: 36 additions & 0 deletions tests/ORM/Fixtures/NotDeletedWrappedScope.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

// phpcs:ignoreFile
declare(strict_types=1);

namespace Cycle\ORM\Tests\Fixtures;

use Cycle\ORM\Select\QueryBuilder;
use Cycle\ORM\Select\ScopeInterface;

/**
* Soft-delete scope that protects itself from user-added OR conditions by wrapping
* already-registered WHERE tokens into a parenthesized group before adding its own
* filter. With a plain {@see NotDeletedScope} a query like
*
* WHERE id = 1 OR id = 2 AND deleted_at IS NULL
*
* is parsed by SQL as
*
* WHERE id = 1 OR (id = 2 AND deleted_at IS NULL)
*
* which bypasses the scope on the first OR arm. With wrapWhere() the resulting
* SQL becomes
*
* WHERE (id = 1 OR id = 2) AND deleted_at IS NULL
*
* which keeps the scope effective regardless of what user code added.
*/
class NotDeletedWrappedScope implements ScopeInterface
{
public function apply(QueryBuilder $query): void
{
$query->wrapWhere();
$query->where('deleted_at', '=', null);
}
}
84 changes: 84 additions & 0 deletions tests/ORM/Functional/Driver/Common/Mapper/SoftDeletesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Cycle\ORM\Select;
use Cycle\ORM\Tests\Functional\Driver\Common\BaseTest;
use Cycle\ORM\Tests\Fixtures\NotDeletedScope;
use Cycle\ORM\Tests\Fixtures\NotDeletedWrappedScope;
use Cycle\ORM\Tests\Fixtures\SoftDeletedMapper;
use Cycle\ORM\Tests\Fixtures\User;
use Cycle\ORM\Tests\Traits\TableTrait;
Expand Down Expand Up @@ -63,6 +64,71 @@ public function testDelete(): void
$this->assertNotNull($s->fetchOne());
}

/**
* Demonstrates the historic problem: a plain scope that adds a top-level
* WHERE is bypassed by a user-added orWhere due to AND-over-OR precedence.
*/
public function testScopeBypassedByOrWhereWithoutWrapWhere(): void
{
$this->seedAliceDeletedAndBobAlive();

// No wrapWhere here — the scope contributes a top-level AND that ORs reach over.
$orm = $this->orm->withHeap(new Heap());
$rows = (new Select($orm, User::class))
->scope(new NotDeletedScope())
->where('id', 1)
->orWhere('id', 2)
->fetchAll();

// SQL: WHERE {id} = ? OR {id} = ? AND {deleted_at} IS NULL
// ≡ WHERE {id} = 1 OR ({id} = 2 AND {deleted_at} IS NULL)
// Alice (id=1) is returned despite being soft-deleted — scope is bypassed.
$this->assertCount(2, $rows);
$emails = \array_map(static fn($u) => $u->email, $rows);
\sort($emails);
$this->assertSame(['alice@test.com', 'bob@test.com'], $emails);
}

/**
* Same query as above but the scope calls wrapWhere() before adding its
* condition. The user OR is enclosed in a group, the scope stays effective.
*/
public function testScopeProtectedByWrapWhereAgainstOrWhere(): void
{
$this->seedAliceDeletedAndBobAlive();

$orm = $this->orm->withHeap(new Heap());
$rows = (new Select($orm, User::class))
->scope(new NotDeletedWrappedScope())
->where('id', 1)
->orWhere('id', 2)
->fetchAll();

// SQL: WHERE ({id} = ? OR {id} = ?) AND {deleted_at} IS NULL
// Only Bob (id=2, alive) survives the scope.
$this->assertCount(1, $rows);
$this->assertSame('bob@test.com', $rows[0]->email);
}

/**
* Wrap-aware scope must remain a no-op when the user supplies no WHERE at all —
* wrapWhere() short-circuits on empty token state, scope's own condition is the
* only thing left.
*/
public function testWrappedScopeWithNoUserWhere(): void
{
$this->seedAliceDeletedAndBobAlive();

$orm = $this->orm->withHeap(new Heap());
$rows = (new Select($orm, User::class))
->scope(new NotDeletedWrappedScope())
->fetchAll();

// SQL: WHERE {deleted_at} IS NULL
$this->assertCount(1, $rows);
$this->assertSame('bob@test.com', $rows[0]->email);
}

public function setUp(): void
{
parent::setUp();
Expand Down Expand Up @@ -93,4 +159,22 @@ public function setUp(): void
],
]));
}

private function seedAliceDeletedAndBobAlive(): void
{
$alice = new User();
$alice->email = 'alice@test.com';
$alice->balance = 100;

$bob = new User();
$bob->email = 'bob@test.com';
$bob->balance = 200;

(new Transaction($this->orm))->persist($alice)->persist($bob)->run();

// Soft-delete Alice; Bob stays alive.
$orm = $this->orm->withHeap(new Heap());
$alice = (new Select($orm, User::class))->wherePK(1)->fetchOne();
(new Transaction($orm))->delete($alice)->run();
}
}
Loading