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
14 changes: 14 additions & 0 deletions src/Database/Adapter/Mongo.php
Original file line number Diff line number Diff line change
Expand Up @@ -2473,6 +2473,20 @@ protected function buildFilters(array $queries, string $separator = '$and'): arr
*/
protected function buildFilter(Query $query): array
{
// Normalize extended ISO 8601 datetime strings in query values to UTCDateTime
// so they can be correctly compared against datetime fields stored in MongoDB.
$values = $query->getValues();
foreach ($values as $k => $value) {
if (is_string($value) && $this->isExtendedISODatetime($value)) {
try {
$values[$k] = $this->toMongoDatetime($value);
} catch (\Throwable $th) {
// Leave value as-is if it cannot be parsed as a datetime
}
}
}
$query->setValues($values);
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard datetime normalization by attribute type to avoid schemaful regressions.

Line 2479 converts all ISO-like strings to UTCDateTime. In schemaful mode, string attributes can store ISO-looking values; coercing the query value will stop those rows from matching (type mismatch). Please only normalize when the attribute is known to be a datetime (or when running schemaless). Otherwise this is a query correctness regression.

💡 Possible guard (stopgap)
-        $values = $query->getValues();
-        foreach ($values as $k => $value) {
-            if (is_string($value) && $this->isExtendedISODatetime($value)) {
-                try {
-                    $values[$k] = $this->toMongoDatetime($value);
-                } catch (\Throwable $th) {
-                    // Leave value as-is if it cannot be parsed as a datetime
-                }
-            }
-        }
-        $query->setValues($values);
+        // Only normalize when we can be sure the attribute represents a datetime
+        if (!$this->getSupportForAttributes() || \in_array($query->getAttribute(), ['$createdAt', '$updatedAt'], true)) {
+            $values = $query->getValues();
+            foreach ($values as $k => $value) {
+                if (is_string($value) && $this->isExtendedISODatetime($value)) {
+                    try {
+                        $values[$k] = $this->toMongoDatetime($value);
+                    } catch (\Throwable $th) {
+                        // Leave value as-is if it cannot be parsed as a datetime
+                    }
+                }
+            }
+            $query->setValues($values);
+        }

Consider wiring attribute type metadata into buildFilter/Query so custom datetime attributes are safely normalized as well.

🤖 Prompt for AI Agents
In `@src/Database/Adapter/Mongo.php` around lines 2476 - 2488, The current loop in
the Mongo adapter unconditionally converts ISO-like strings to UTCDateTime
(using isExtendedISODatetime and toMongoDatetime) which breaks schemaful
queries; change the logic to only normalize values when the query is schemaless
or when the corresponding attribute is known to be a datetime type—e.g. call a
metadata accessor on the Query (or buildFilter) to get the attribute type for
key $k and skip conversion unless the type === 'datetime' (or equivalent) or
$query->isSchemaless() returns true; keep the try/catch around to leave
unparsable values unchanged and then call $query->setValues($values) as before.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in schemaless we dont have support for attribute types, but in queries it will not be an issue reason being if string the check would be o(1) as the normalization doing it and if you see the logic you will get it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ArnabChatterjee20k, I understand the focus on schemaless mode, but I'd like to clarify the concern about schemaful mode:

In schemaful mode (when getSupportForAttributes() returns true), a collection could have a regular string attribute that legitimately stores ISO-formatted datetime strings as text data (e.g., event logs, API responses, human-readable timestamps).

The issue:

  • Database stores: { "logMessage": "2024-01-01T00:00:00Z" } (as a string)
  • User queries: Query::equal("logMessage", "2024-01-01T00:00:00Z")
  • Your code converts the query value to UTCDateTime object
  • MongoDB compares: string (in DB) vs UTCDateTime (in query) → no match

Pattern in your own codebase:

In castingBefore (lines 1398-1413), you already guard datetime normalization:

if (!$this->getSupportForAttributes()) {  // ← Only in schemaless mode
    foreach ($document->getArrayCopy() as $key => $value) {
        if (is_string($value) && ... && $this->isExtendedISODatetime($value)) {
            // normalize to UTCDateTime
        }
    }
}

Suggestion:
Add the same guard to buildFilter to maintain consistency and avoid breaking schemaful string queries:

// Only normalize in schemaless mode or for internal datetime attributes
if (!$this->getSupportForAttributes() || \in_array($query->getAttribute(), ['$createdAt', '$updatedAt'], true)) {
    $values = $query->getValues();
    foreach ($values as $k => $value) {
        if (is_string($value) && $this->isExtendedISODatetime($value)) {
            try {
                $values[$k] = $this->toMongoDatetime($value);
            } catch (\Throwable $th) {
                // Leave value as-is
            }
        }
    }
    $query->setValues($values);
}

Could you confirm whether schemaful collections with string attributes containing ISO-formatted text are a supported use case? If so, the guard would prevent regressions. 🤔


🧠 Learnings used
Learnt from: abnegate
Repo: utopia-php/database PR: 721
File: tests/e2e/Adapter/Scopes/DocumentTests.php:6418-6439
Timestamp: 2025-10-03T02:04:17.803Z
Learning: In tests/e2e/Adapter/Scopes/DocumentTests::testSchemalessDocumentInvalidInteralAttributeValidation (PHP), when the adapter reports getSupportForAttributes() === false (schemaless), the test should not expect exceptions from createDocuments for “invalid” internal attributes; remove try/catch and ensure the test passes without exceptions, keeping at least one assertion.

Learnt from: ArnabChatterjee20k
Repo: utopia-php/database PR: 747
File: src/Database/Adapter/Mongo.php:1449-1453
Timestamp: 2025-10-29T12:27:57.071Z
Learning: In src/Database/Adapter/Mongo.php, when getSupportForAttributes() returns false (schemaless mode), the updateDocument method intentionally uses a raw document without $set operator for replacement-style updates, as confirmed by the repository maintainer ArnabChatterjee20k.


if ($query->getAttribute() === '$id') {
$query->setAttribute('_uid');
} elseif ($query->getAttribute() === '$sequence') {
Expand Down
128 changes: 128 additions & 0 deletions tests/e2e/Adapter/Scopes/SchemalessTests.php
Original file line number Diff line number Diff line change
Expand Up @@ -3242,4 +3242,132 @@ public function testSchemalessMongoDotNotationIndexes(): void

$database->deleteCollection($col);
}

public function testQueryWithDatetime(): void
{
/** @var Database $database */
$database = static::getDatabase();

if ($database->getAdapter()->getSupportForAttributes()) {
$this->expectNotToPerformAssertions();
return;
}

$col = uniqid('sl_query_datetime');
$database->createCollection($col);

$permissions = [
Permission::read(Role::any()),
Permission::write(Role::any()),
Permission::update(Role::any()),
Permission::delete(Role::any())
];

// Documents with datetime field (ISO 8601) for query tests
// Dates: Jan 15 2024, Feb 20 2024, Mar 25 2024, Jun 15 2024, Dec 31 2024
$docs = [
new Document([
'$id' => 'dt1',
'$permissions' => $permissions,
'name' => 'January',
'datetime' => '2024-01-15T10:30:00.000+00:00'
]),
new Document([
'$id' => 'dt2',
'$permissions' => $permissions,
'name' => 'February',
'datetime' => '2024-02-20T14:45:30.123Z'
]),
new Document([
'$id' => 'dt3',
'$permissions' => $permissions,
'name' => 'March',
// Use a valid extended ISO 8601 datetime that will be normalized
// to MongoDB UTCDateTime for comparison queries.
'datetime' => '2024-03-25T08:15:45.000+00:00'
]),
new Document([
'$id' => 'dt4',
'$permissions' => $permissions,
'name' => 'June',
'datetime' => '2024-06-15T12:00:00.000Z'
]),
new Document([
'$id' => 'dt5',
'$permissions' => $permissions,
'name' => 'December',
'datetime' => '2024-12-31T23:59:59.999+00:00'
]),
];

$createdCount = $database->createDocuments($col, $docs);
$this->assertEquals(5, $createdCount);

// Query: equal - find document with exact datetime (Jan 15 2024)
$equalResults = $database->find($col, [
Query::equal('datetime', ['2024-01-15T10:30:00.000+00:00'])
]);
$this->assertCount(1, $equalResults);
$this->assertEquals('dt1', $equalResults[0]->getId());
$this->assertEquals('January', $equalResults[0]->getAttribute('name'));

// Query: greaterThan - datetimes after 2024-03-01 (dt3, dt4, dt5)
$greaterResults = $database->find($col, [
Query::greaterThan('datetime', '2024-03-01T00:00:00.000Z')
]);
$this->assertCount(3, $greaterResults);
$greaterIds = array_map(fn ($d) => $d->getId(), $greaterResults);
$this->assertContains('dt3', $greaterIds);
$this->assertContains('dt4', $greaterIds);
$this->assertContains('dt5', $greaterIds);

// Query: lessThan - datetimes before 2024-03-01 (dt1, dt2)
$lessResults = $database->find($col, [
Query::lessThan('datetime', '2024-03-01T00:00:00.000Z')
]);
$this->assertCount(2, $lessResults);
$lessIds = array_map(fn ($d) => $d->getId(), $lessResults);
$this->assertContains('dt1', $lessIds);
$this->assertContains('dt2', $lessIds);

// Query: greaterThanEqual - datetimes on or after 2024-02-20 (dt2, dt3, dt4, dt5)
$gteResults = $database->find($col, [
Query::greaterThanEqual('datetime', '2024-02-20T14:45:30.123Z')
]);
$this->assertCount(4, $gteResults);
$gteIds = array_map(fn ($d) => $d->getId(), $gteResults);
$this->assertContains('dt2', $gteIds);
$this->assertContains('dt3', $gteIds);
$this->assertContains('dt4', $gteIds);
$this->assertContains('dt5', $gteIds);

// Query: lessThanEqual - datetimes on or before 2024-06-15 (dt1, dt2, dt3, dt4)
$lteResults = $database->find($col, [
Query::lessThanEqual('datetime', '2024-06-15T12:00:00.000Z')
]);
$this->assertCount(4, $lteResults);
$lteIds = array_map(fn ($d) => $d->getId(), $lteResults);
$this->assertContains('dt1', $lteIds);
$this->assertContains('dt2', $lteIds);
$this->assertContains('dt3', $lteIds);
$this->assertContains('dt4', $lteIds);

// Query: between - datetimes in range [2024-02-01, 2024-07-01) (dt2, dt3, dt4)
$betweenResults = $database->find($col, [
Query::between('datetime', '2024-02-01T00:00:00.000Z', '2024-07-01T00:00:00.000Z')
]);
$this->assertCount(3, $betweenResults);
$betweenIds = array_map(fn ($d) => $d->getId(), $betweenResults);
$this->assertContains('dt2', $betweenIds);
$this->assertContains('dt3', $betweenIds);
$this->assertContains('dt4', $betweenIds);

// Query: equal with no match
$noneResults = $database->find($col, [
Query::equal('datetime', ['2020-01-01T00:00:00.000Z'])
]);
$this->assertCount(0, $noneResults);

$database->deleteCollection($col);
}
}