fix: Address CodeRabbit review comments

- Fix OR precheck bug: skip attribute existence check when OR conditions
  exist, since OR can match with partial attributes present
- Remove dead code in app/realtime.php (unused $names and $channels variables)
- Add tests for OR queries with missing attributes in one branch

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Jake Barnby
2026-02-05 22:24:26 +13:00
parent 18125156d2
commit e21614088f
3 changed files with 40 additions and 8 deletions
-2
View File
@@ -797,8 +797,6 @@ $server->onMessage(function (int $connection, string $message) use ($server, $re
}
$roles = $user->getRoles($database->getAuthorization());
$names = $realtime->connections[$connection]['channels'] ?? [];
$channels = Realtime::convertChannels(array_flip($names), $user->getId());
$authorization = $realtime->connections[$connection]['authorization'] ?? null;
$projectId = $realtime->connections[$connection]['projectId'] ?? null;
+14 -6
View File
@@ -95,12 +95,13 @@ class RuntimeQuery extends Query
'type' => 'filter',
'conditions' => [],
'attributes' => [],
'hasOr' => false,
];
foreach ($queries as $query) {
$condition = self::compileCondition($query);
$compiled['conditions'][] = $condition;
self::extractAttributes($condition, $compiled['attributes']);
self::extractAttributes($condition, $compiled['attributes'], $compiled['hasOr']);
}
$compiled['attributes'] = array_unique($compiled['attributes']);
@@ -138,15 +139,19 @@ class RuntimeQuery extends Query
/**
* Extract all attribute names from a compiled condition tree.
* Also tracks whether any OR conditions exist.
*/
private static function extractAttributes(array $condition, array &$attributes): void
private static function extractAttributes(array $condition, array &$attributes, bool &$hasOr): void
{
if (isset($condition['op']) && $condition['op'] === 'OR') {
$hasOr = true;
}
if (isset($condition['attr'])) {
$attributes[] = $condition['attr'];
}
if (isset($condition['conditions'])) {
foreach ($condition['conditions'] as $sub) {
self::extractAttributes($sub, $attributes);
self::extractAttributes($sub, $attributes, $hasOr);
}
}
}
@@ -166,9 +171,12 @@ class RuntimeQuery extends Query
}
// Quick rejection: if payload is missing any required attribute, fail fast
foreach ($compiled['attributes'] as $attr) {
if (!isset($payload[$attr]) && !\array_key_exists($attr, $payload)) {
return null;
// Skip this optimization when OR conditions exist (OR can match with partial attributes)
if (empty($compiled['hasOr'])) {
foreach ($compiled['attributes'] as $attr) {
if (!isset($payload[$attr]) && !\array_key_exists($attr, $payload)) {
return null;
}
}
}
@@ -463,6 +463,32 @@ class RuntimeQueryTest extends TestCase
$this->assertEquals($payload, $result);
}
public function testOrWithMissingAttributeInOneBranch(): void
{
// OR should match when one branch's attribute is missing but another branch matches
$query = Query::or([
Query::equal('name', ['John']),
Query::equal('email', ['john@example.com'])
]);
// Payload only has email, not name - should still match via email branch
$payload = ['email' => 'john@example.com'];
$result = $this->compileAndFilter([$query], $payload);
$this->assertEquals($payload, $result);
}
public function testOrWithMissingAttributeNoMatch(): void
{
// OR should not match when the only matching branch has missing attribute
$query = Query::or([
Query::equal('name', ['John']),
Query::equal('email', ['john@example.com'])
]);
// Payload only has name but it doesn't match - should not match
$payload = ['name' => 'Jane'];
$result = $this->compileAndFilter([$query], $payload);
$this->assertNull($result);
}
// Complex combinations
public function testAndOrCombination(): void
{