Feat mongo tmp #647
Feat mongo tmp #647
Conversation
Feat mongo index validation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/e2e/Adapter/Scopes/AttributeTests.php (1)
520-556: GuardtestUpdateAttributeStructure()for adapters without attributesWhen
getSupportForAttributes()is false (e.g., Mongo),testUpdateAttributeFormat()exits early and never seeds thepriceattribute. This dependent test still runs, then dereferences$collection->getAttribute('attributes')[4], which is undefined and crashes the suite. Add the same capability guard here so the test is skipped when attributes aren’t supported./** @var Database $database */ $database = static::getDatabase(); + if (!$database->getAdapter()->getSupportForAttributes()) { + $this->expectNotToPerformAssertions(); + return; + } + // price attribute $collection = $database->getCollection('flowers');
🧹 Nitpick comments (7)
tests/e2e/Adapter/Scopes/SpatialTests.php (1)
24-26: Prefer skipping unsupported paths over “no‑assertion” passesReplacing markTestSkipped with expectNotToPerformAssertions changes reporting semantics (passes vs. skipped). Use markTestSkipped to reflect adapter capability gaps.
Apply this pattern (example for Lines 24–26; repeat similarly in other locations):
- $this->expectNotToPerformAssertions(); - return; + $this->markTestSkipped('Adapter does not support spatial attributes');Also applies to: 98-100, 923-925, 1354-1356, 1486-1488, 1788-1790, 2068-2070, 2174-2176, 2250-2252, 2382-2384, 2462-2464, 2653-2655, 2685-2687
tests/e2e/Adapter/Scopes/IndexTests.php (3)
261-276: Tighten assertions for failure branchWhen attributes are supported and validation fails, assert the specific message for determinism.
- } catch (Exception $e) { - $this->assertEquals('Attribute "integer" cannot be part of a fulltext index, must be of type string', $e->getMessage()); + } catch (Exception $e) { + $this->assertSame('Attribute "integer" cannot be part of a fulltext index, must be of type string', $e->getMessage());
510-553: Multiple fulltext index test: consider asserting the validator errorWhen unsupported, also assert the exception message “There is already a fulltext index in the collection” to ensure parity with validator behavior.
555-633: Minor test description fixesFix typos (“faliure” -> “failure”) and align comments with assertions (e.g., blocks labeled failure currently assert success).
- // Test with different attributes order - faliure + // Test with different attributes order - success (attributes order differs) ... - // Test with different orders order - faliure + // Test with different orders - success (orders differ)src/Database/Adapter.php (2)
1184-1187: Document expected projection shapes for adaptersReturn type is mixed; clarify expected forms (e.g., SQL: string of columns; Mongo: array projection) in PHPDoc.
- /** - * Get an attribute projection given a list of selected attributes - * - * @param array<string> $selections - * @param string $prefix - * @return mixed - */ + /** + * Get an attribute projection given a list of selected attributes. + * + * @param array<string> $selections + * @param string $prefix + * @return string|array<string,int> + * SQL adapters: string of quoted column projections (e.g., "t.col1,t.col2"). + * Mongo adapters: projection map (e.g., ['field' => 1, ...]). + */
1341-1355: Clarify casting hook docs“Before” hook should state it runs pre-adapter encode; “After” runs post-adapter decode.
- /** - * Returns the document after casting + /** + * Casts fields before persistence/driver encoding. * @param Document $collection * @param Document $document * @return Document */src/Database/Adapter/SQL.php (1)
1565-1573: Silence unused$collectionparameters in casting hooks.PHPMD is flagging the newly added
castingBefore/castingAftermethods because$collectionisn’t referenced. Please mark the argument as intentionally unused (e.g., viaunset($collection);) so static analysis stays green while we keep the signature required by the interface.
Based on static analysis hintspublic function castingBefore(Document $collection, Document $document): Document { + unset($collection); return $document; } public function castingAfter(Document $collection, Document $document): Document { + unset($collection); return $document; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
src/Database/Adapter.php(3 hunks)src/Database/Adapter/Mongo.php(1 hunks)src/Database/Adapter/Pool.php(3 hunks)src/Database/Adapter/SQL.php(2 hunks)src/Database/Database.php(24 hunks)src/Database/Validator/Index.php(6 hunks)tests/e2e/Adapter/Scopes/AttributeTests.php(18 hunks)tests/e2e/Adapter/Scopes/DocumentTests.php(22 hunks)tests/e2e/Adapter/Scopes/IndexTests.php(5 hunks)tests/e2e/Adapter/Scopes/SpatialTests.php(15 hunks)tests/unit/Validator/IndexTest.php(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-30T19:17:53.630Z
Learnt from: ArnabChatterjee20k
PR: utopia-php/database#642
File: src/Database/Validator/PartialStructure.php:43-52
Timestamp: 2025-07-30T19:17:53.630Z
Learning: In PartialStructure validator, when filtering for required attributes validation using the $requiredAttributes parameter, $this->attributes should be used instead of the merged $attributes array because this validation is specifically for internal attributes like $createdAt and $updatedAt that are defined in the base Structure class, not collection-specific attributes.
Applied to files:
src/Database/Adapter/Mongo.php
🧬 Code graph analysis (10)
src/Database/Adapter.php (3)
src/Database/Adapter/Pool.php (9)
getSupportForMultipleFulltextIndexes(543-546)getSupportForIdenticalIndexes(548-551)getSupportForOrderRandom(553-556)getAttributeProjection(473-476)castingBefore(573-576)castingAfter(578-581)isMongo(583-586)getSupportForInternalCasting(588-591)setUTCDatetime(593-596)src/Database/Adapter/Mongo.php (9)
getSupportForMultipleFulltextIndexes(2565-2568)getSupportForIdenticalIndexes(2574-2577)getSupportForOrderRandom(2584-2587)getAttributeProjection(2148-2173)castingBefore(957-1006)castingAfter(898-948)isMongo(2259-2262)getSupportForInternalCasting(2253-2256)setUTCDatetime(2264-2267)src/Database/Adapter/SQL.php (9)
getSupportForMultipleFulltextIndexes(1530-1533)getSupportForIdenticalIndexes(1540-1543)getSupportForOrderRandom(1550-1553)getAttributeProjection(1949-1978)castingBefore(1565-1568)castingAfter(1570-1573)isMongo(1555-1558)getSupportForInternalCasting(1520-1523)setUTCDatetime(1560-1563)
src/Database/Adapter/Pool.php (3)
src/Database/Adapter.php (11)
getAttributeProjection(1186-1186)getSupportForDistanceBetweenMultiDimensionGeometryInMeters(1099-1099)getSupportForSpatialAxisOrder(1085-1085)getSupportForMultipleFulltextIndexes(1106-1106)getSupportForIdenticalIndexes(1114-1114)getSupportForOrderRandom(1121-1121)castingBefore(1347-1347)castingAfter(1355-1355)isMongo(1362-1362)getSupportForInternalCasting(1369-1369)setUTCDatetime(1377-1377)src/Database/Mirror.php (1)
delegate(88-103)src/Database/Document.php (1)
Document(12-470)
tests/e2e/Adapter/Scopes/SpatialTests.php (7)
src/Database/Database.php (1)
getAdapter(1201-1204)src/Database/Adapter/Pool.php (1)
getSupportForDistanceBetweenMultiDimensionGeometryInMeters(528-531)src/Database/Adapter.php (1)
getSupportForDistanceBetweenMultiDimensionGeometryInMeters(1099-1099)src/Database/Adapter/MariaDB.php (1)
getSupportForDistanceBetweenMultiDimensionGeometryInMeters(1889-1892)src/Database/Adapter/SQLite.php (1)
getSupportForDistanceBetweenMultiDimensionGeometryInMeters(1273-1276)src/Database/Adapter/MySQL.php (1)
getSupportForDistanceBetweenMultiDimensionGeometryInMeters(189-192)src/Database/Adapter/Postgres.php (1)
getSupportForDistanceBetweenMultiDimensionGeometryInMeters(1999-2002)
src/Database/Database.php (9)
src/Database/Adapter/Pool.php (15)
getMaxIndexLength(308-311)getInternalIndexesKeys(488-491)getSupportForIndexArray(338-341)getSupportForSpatialAttributes(428-431)getSupportForSpatialIndexNull(433-436)getSupportForSpatialIndexOrder(523-526)getSupportForAttributes(323-326)getSupportForMultipleFulltextIndexes(543-546)getSupportForIdenticalIndexes(548-551)castingAfter(578-581)castingBefore(573-576)createDocument(228-231)updateDocument(238-241)isMongo(583-586)setUTCDatetime(593-596)src/Database/Adapter.php (15)
getMaxIndexLength(876-876)getInternalIndexesKeys(1291-1291)getSupportForIndexArray(935-935)getSupportForSpatialAttributes(1057-1057)getSupportForSpatialIndexNull(1064-1064)getSupportForSpatialIndexOrder(1078-1078)getSupportForAttributes(914-914)getSupportForMultipleFulltextIndexes(1106-1106)getSupportForIdenticalIndexes(1114-1114)castingAfter(1355-1355)castingBefore(1347-1347)createDocument(701-701)updateDocument(725-725)isMongo(1362-1362)setUTCDatetime(1377-1377)src/Database/Adapter/Mongo.php (15)
getMaxIndexLength(2693-2696)getInternalIndexesKeys(2703-2706)getSupportForIndexArray(2243-2246)getSupportForSpatialAttributes(2500-2503)getSupportForSpatialIndexNull(2510-2513)getSupportForSpatialIndexOrder(2529-2532)getSupportForAttributes(2275-2278)getSupportForMultipleFulltextIndexes(2565-2568)getSupportForIdenticalIndexes(2574-2577)castingAfter(898-948)castingBefore(957-1006)createDocument(862-890)updateDocument(1103-1126)isMongo(2259-2262)setUTCDatetime(2264-2267)src/Database/Adapter/SQL.php (13)
getMaxIndexLength(1845-1851)getInternalIndexesKeys(1897-1900)getSupportForIndexArray(1460-1463)getSupportForSpatialAttributes(1490-1493)getSupportForSpatialIndexNull(1500-1503)getSupportForSpatialIndexOrder(1510-1513)getSupportForAttributes(896-899)getSupportForMultipleFulltextIndexes(1530-1533)getSupportForIdenticalIndexes(1540-1543)castingAfter(1570-1573)castingBefore(1565-1568)isMongo(1555-1558)setUTCDatetime(1560-1563)src/Database/Adapter/MariaDB.php (7)
getInternalIndexesKeys(1772-1775)getSupportForIndexArray(1845-1848)getSupportForSpatialAttributes(1850-1853)getSupportForSpatialIndexNull(1860-1863)getSupportForSpatialIndexOrder(1879-1882)createDocument(822-933)updateDocument(948-1170)src/Database/Adapter/MySQL.php (2)
getSupportForIndexArray(130-136)getSupportForSpatialIndexOrder(179-182)src/Database/Document.php (4)
getAttribute(224-231)setAttribute(244-261)Document(12-470)getArrayCopy(423-458)src/Database/Validator/Sequence.php (1)
isValid(38-59)src/Database/DateTime.php (2)
DateTime(7-86)setTimezone(58-67)
tests/e2e/Adapter/Scopes/DocumentTests.php (5)
src/Database/Database.php (2)
getAdapter(1201-1204)Database(37-7421)src/Database/Adapter/Pool.php (4)
getIdAttributeType(508-511)getSupportForAttributes(323-326)getSupportForOrderRandom(553-556)getSupportForFulltextIndex(353-356)src/Database/Adapter.php (4)
getIdAttributeType(890-890)getSupportForAttributes(914-914)getSupportForOrderRandom(1121-1121)getSupportForFulltextIndex(956-956)src/Database/Adapter/Mongo.php (4)
getIdAttributeType(2685-2688)getSupportForAttributes(2275-2278)getSupportForOrderRandom(2584-2587)getSupportForFulltextIndex(2295-2298)src/Database/Adapter/SQL.php (4)
getIdAttributeType(1837-1840)getSupportForAttributes(896-899)getSupportForOrderRandom(1550-1553)getSupportForFulltextIndex(916-919)
tests/unit/Validator/IndexTest.php (2)
src/Database/Validator/Index.php (3)
Index(10-489)isValid(311-354)getDescription(97-100)src/Database/Document.php (1)
getAttribute(224-231)
src/Database/Adapter/SQL.php (4)
src/Database/Adapter/Pool.php (9)
getSupportForInternalCasting(588-591)getSupportForMultipleFulltextIndexes(543-546)getSupportForIdenticalIndexes(548-551)getSupportForOrderRandom(553-556)isMongo(583-586)setUTCDatetime(593-596)castingBefore(573-576)castingAfter(578-581)getAttributeProjection(473-476)src/Database/Adapter.php (9)
getSupportForInternalCasting(1369-1369)getSupportForMultipleFulltextIndexes(1106-1106)getSupportForIdenticalIndexes(1114-1114)getSupportForOrderRandom(1121-1121)isMongo(1362-1362)setUTCDatetime(1377-1377)castingBefore(1347-1347)castingAfter(1355-1355)getAttributeProjection(1186-1186)src/Database/Adapter/Mongo.php (9)
getSupportForInternalCasting(2253-2256)getSupportForMultipleFulltextIndexes(2565-2568)getSupportForIdenticalIndexes(2574-2577)getSupportForOrderRandom(2584-2587)isMongo(2259-2262)setUTCDatetime(2264-2267)castingBefore(957-1006)castingAfter(898-948)getAttributeProjection(2148-2173)src/Database/Document.php (1)
Document(12-470)
tests/e2e/Adapter/Scopes/AttributeTests.php (7)
src/Database/Database.php (6)
updateAttribute(2250-2460)Database(37-7421)deleteIndex(3429-3454)createIndex(3255-3415)createAttribute(1698-1753)createDocument(3862-3958)src/Database/Adapter/Pool.php (8)
getSupportForAttributes(323-326)getSupportForIdenticalIndexes(548-551)updateAttribute(178-181)getMaxIndexLength(308-311)deleteIndex(218-221)createIndex(213-216)createAttribute(168-171)createDocument(228-231)src/Database/Adapter.php (8)
getSupportForAttributes(914-914)getSupportForIdenticalIndexes(1114-1114)updateAttribute(584-584)getMaxIndexLength(876-876)deleteIndex(680-680)createIndex(670-670)createAttribute(557-557)createDocument(701-701)src/Database/Adapter/Mongo.php (8)
getSupportForAttributes(2275-2278)getSupportForIdenticalIndexes(2574-2577)updateAttribute(1465-1471)getMaxIndexLength(2693-2696)deleteIndex(802-809)createIndex(667-733)createAttribute(441-444)createDocument(862-890)src/Database/Adapter/SQL.php (4)
getSupportForAttributes(896-899)getSupportForIdenticalIndexes(1540-1543)getMaxIndexLength(1845-1851)createAttribute(234-248)src/Database/Adapter/MariaDB.php (4)
updateAttribute(417-438)deleteIndex(789-809)createIndex(715-778)createDocument(822-933)src/Database/Mirror.php (5)
updateAttribute(393-444)deleteIndex(514-538)createIndex(472-512)createAttribute(295-357)createDocument(540-585)
tests/e2e/Adapter/Scopes/IndexTests.php (9)
src/Database/Database.php (6)
getAdapter(1201-1204)createCollection(1330-1468)createAttribute(1698-1753)Database(37-7421)getDatabase(779-782)createIndex(3255-3415)src/Database/Adapter/Pool.php (13)
getMaxIndexLength(308-311)getInternalIndexesKeys(488-491)getSupportForIndexArray(338-341)getSupportForSpatialAttributes(428-431)getSupportForSpatialIndexNull(433-436)getSupportForSpatialIndexOrder(523-526)getSupportForAttributes(323-326)getSupportForMultipleFulltextIndexes(543-546)getSupportForIdenticalIndexes(548-551)createCollection(153-156)createAttribute(168-171)getSupportForFulltextIndex(353-356)createIndex(213-216)src/Database/Adapter.php (14)
getMaxIndexLength(876-876)getInternalIndexesKeys(1291-1291)getSupportForIndexArray(935-935)getSupportForSpatialAttributes(1057-1057)getSupportForSpatialIndexNull(1064-1064)getSupportForSpatialIndexOrder(1078-1078)getSupportForAttributes(914-914)getSupportForMultipleFulltextIndexes(1106-1106)getSupportForIdenticalIndexes(1114-1114)createCollection(525-525)createAttribute(557-557)getDatabase(160-163)getSupportForFulltextIndex(956-956)createIndex(670-670)src/Database/Adapter/Mongo.php (13)
getMaxIndexLength(2693-2696)getInternalIndexesKeys(2703-2706)getSupportForIndexArray(2243-2246)getSupportForSpatialAttributes(2500-2503)getSupportForSpatialIndexNull(2510-2513)getSupportForSpatialIndexOrder(2529-2532)getSupportForAttributes(2275-2278)getSupportForMultipleFulltextIndexes(2565-2568)getSupportForIdenticalIndexes(2574-2577)createCollection(199-347)createAttribute(441-444)getSupportForFulltextIndex(2295-2298)createIndex(667-733)src/Database/Adapter/SQL.php (11)
getMaxIndexLength(1845-1851)getInternalIndexesKeys(1897-1900)getSupportForIndexArray(1460-1463)getSupportForSpatialAttributes(1490-1493)getSupportForSpatialIndexNull(1500-1503)getSupportForSpatialIndexOrder(1510-1513)getSupportForAttributes(896-899)getSupportForMultipleFulltextIndexes(1530-1533)getSupportForIdenticalIndexes(1540-1543)createAttribute(234-248)getSupportForFulltextIndex(916-919)src/Database/Adapter/MySQL.php (2)
getSupportForIndexArray(130-136)getSupportForSpatialIndexOrder(179-182)src/Database/Validator/Index.php (2)
isValid(311-354)getDescription(97-100)src/Database/Validator/Query/Select.php (1)
isValid(51-99)tests/e2e/Adapter/Base.php (1)
getDatabase(36-36)
src/Database/Adapter/Mongo.php (8)
src/Database/Database.php (38)
Database(37-7421)setTimeout(872-877)clearTimeout(885-888)ping(1224-1227)reconnect(1229-1232)create(1243-1262)exists(1273-1278)list(1285-1292)listCollections(1549-1559)delete(1301-1315)createCollection(1330-1468)getSizeOfCollectionOnDisk(1591-1608)getSizeOfCollection(1569-1582)deleteCollection(1629-1672)analyzeCollection(1616-1619)createAttribute(1698-1753)createAttributes(1769-1861)deleteAttribute(2506-2574)renameAttribute(2589-2653)createRelationship(2673-2850)updateRelationship(2865-3052)getDocument(3467-3590)deleteRelationship(3066-3180)createIndex(3255-3415)renameIndex(3196-3234)deleteIndex(3429-3454)find(6406-6535)createDocument(3862-3958)createDocuments(3974-4077)updateDocument(4414-4621)updateDocuments(4644-4838)upsertDocuments(5275-5290)increaseDocumentAttribute(5546-5627)deleteDocument(5741-5793)deleteDocuments(6192-6345)count(6632-6668)updateAttribute(2250-2460)sum(6683-6712)src/Database/Adapter.php (42)
Adapter(16-1379)setTimeout(309-309)clearTimeout(322-326)startTransaction(336-336)commitTransaction(348-348)rollbackTransaction(359-359)ping(474-474)reconnect(479-479)create(488-488)exists(499-499)list(506-506)delete(515-515)createCollection(525-525)filter(1218-1227)getSizeOfCollectionOnDisk(843-843)getSizeOfCollection(834-834)deleteCollection(534-534)analyzeCollection(542-542)createAttribute(557-557)createAttributes(568-568)deleteAttribute(594-594)renameAttribute(604-604)createRelationship(615-615)updateRelationship(631-631)getDocument(691-691)deleteRelationship(645-645)createIndex(670-670)renameIndex(655-655)deleteIndex(680-680)find(802-802)createDocument(701-701)createDocuments(713-713)updateDocument(725-725)updateDocuments(740-740)upsertDocuments(752-756)getSequences(763-763)increaseDocumentAttribute(1269-1277)deleteDocument(773-773)deleteDocuments(784-784)count(825-825)updateAttribute(584-584)sum(814-814)src/Database/Change.php (1)
Change(5-32)src/Database/DateTime.php (2)
DateTime(7-86)format(29-32)src/Database/Document.php (6)
Document(12-470)getAttribute(224-231)getSequence(71-80)find(304-322)removeAttribute(287-293)setAttribute(244-261)src/Database/Query.php (6)
Query(8-1091)getAttribute(158-161)setAttribute(199-204)equal(434-437)limit(622-625)offset(633-636)src/Database/Validator/Authorization.php (1)
Authorization(7-225)src/Database/Adapter/Pool.php (39)
setTimeout(98-101)startTransaction(103-106)commitTransaction(108-111)rollbackTransaction(113-116)ping(123-126)reconnect(128-131)create(133-136)exists(138-141)list(143-146)delete(148-151)createCollection(153-156)getSizeOfCollectionOnDisk(283-286)getSizeOfCollection(278-281)deleteCollection(158-161)analyzeCollection(163-166)createAttribute(168-171)createAttributes(173-176)deleteAttribute(183-186)renameAttribute(188-191)createRelationship(193-196)updateRelationship(198-201)getDocument(223-226)deleteRelationship(203-206)createIndex(213-216)renameIndex(208-211)deleteIndex(218-221)find(263-266)createDocument(228-231)createDocuments(233-236)updateDocument(238-241)updateDocuments(243-246)upsertDocuments(248-251)getSequences(513-516)increaseDocumentAttribute(478-481)deleteDocument(253-256)deleteDocuments(258-261)count(273-276)updateAttribute(178-181)sum(268-271)
🪛 PHPMD (2.15.0)
src/Database/Adapter/SQL.php
1565-1565: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
1570-1570: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
src/Database/Adapter/Mongo.php
68-68: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
123-123: Avoid unused parameters such as '$name'. (undefined)
(UnusedFormalParameter)
138-138: Avoid unused parameters such as '$database'. (undefined)
(UnusedFormalParameter)
425-425: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
441-441: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
441-441: Avoid unused parameters such as '$id'. (undefined)
(UnusedFormalParameter)
441-441: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
441-441: Avoid unused parameters such as '$size'. (undefined)
(UnusedFormalParameter)
441-441: Avoid unused parameters such as '$signed'. (undefined)
(UnusedFormalParameter)
441-441: Avoid unused parameters such as '$array'. (undefined)
(UnusedFormalParameter)
441-441: Avoid unused parameters such as '$required'. (undefined)
(UnusedFormalParameter)
454-454: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
454-454: Avoid unused parameters such as '$attributes'. (undefined)
(UnusedFormalParameter)
512-512: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
512-512: Avoid unused parameters such as '$relatedCollection'. (undefined)
(UnusedFormalParameter)
512-512: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
512-512: Avoid unused parameters such as '$twoWay'. (undefined)
(UnusedFormalParameter)
512-512: Avoid unused parameters such as '$id'. (undefined)
(UnusedFormalParameter)
512-512: Avoid unused parameters such as '$twoWayKey'. (undefined)
(UnusedFormalParameter)
538-538: Avoid unused parameters such as '$side'. (undefined)
(UnusedFormalParameter)
667-667: Avoid unused parameters such as '$lengths'. (undefined)
(UnusedFormalParameter)
820-820: Avoid unused parameters such as '$forUpdate'. (undefined)
(UnusedFormalParameter)
1103-1103: Avoid unused parameters such as '$skipPermissions'. (undefined)
(UnusedFormalParameter)
1421-1421: Avoid unused parameters such as '$permissionIds'. (undefined)
(UnusedFormalParameter)
1465-1465: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
1465-1465: Avoid unused parameters such as '$size'. (undefined)
(UnusedFormalParameter)
1465-1465: Avoid unused parameters such as '$signed'. (undefined)
(UnusedFormalParameter)
1465-1465: Avoid unused parameters such as '$array'. (undefined)
(UnusedFormalParameter)
1465-1465: Avoid unused parameters such as '$required'. (undefined)
(UnusedFormalParameter)
2148-2148: Avoid unused parameters such as '$prefix'. (undefined)
(UnusedFormalParameter)
2481-2481: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
2668-2668: Avoid unused parameters such as '$string'. (undefined)
(UnusedFormalParameter)
2677-2677: Avoid unused parameters such as '$stmt'. (undefined)
(UnusedFormalParameter)
2708-2708: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
2747-2747: Avoid unused parameters such as '$wkb'. (undefined)
(UnusedFormalParameter)
2758-2758: Avoid unused parameters such as '$wkb'. (undefined)
(UnusedFormalParameter)
2769-2769: Avoid unused parameters such as '$wkb'. (undefined)
(UnusedFormalParameter)
2781-2781: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
2781-2781: Avoid unused parameters such as '$alias'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Adapter Tests (SharedTables/MongoDB)
- GitHub Check: Adapter Tests (SQLite)
- GitHub Check: Adapter Tests (SharedTables/Postgres)
- GitHub Check: Adapter Tests (SharedTables/SQLite)
- GitHub Check: Adapter Tests (SharedTables/MySQL)
- GitHub Check: Adapter Tests (SharedTables/MariaDB)
- GitHub Check: Adapter Tests (Mirror)
- GitHub Check: Adapter Tests (Pool)
- GitHub Check: Adapter Tests (Postgres)
- GitHub Check: Adapter Tests (MySQL)
- GitHub Check: Adapter Tests (MariaDB)
- GitHub Check: Adapter Tests (MongoDB)
- GitHub Check: Unit Test
🔇 Additional comments (40)
tests/e2e/Adapter/Scopes/AttributeTests.php (1)
1331-1333: Replace unexplained index-overhead magic numberThe hard-coded
68offset is still here with no adapter-backed explanation. Please switch to an adapter-provided constant/method (e.g.,getIndexOverhead()) so the intent is explicit and per-adapter configuration lives with the adapter implementation. This mirrors the prior review feedback.tests/e2e/Adapter/Scopes/DocumentTests.php (1)
6419-6439: Add explicit failure whencreateDocumentsdoesn’t throw.Both
tryblocks expect aStructureException, but ifcreateDocuments()ever stops throwing, the test will silently pass. Please add$this->fail('Failed to throw exception');immediately after eachcreateDocuments()call so the test fails on regressions.src/Database/Database.php (16)
1409-1420: IndexValidator capability flags: LGTMPassing adapter capability flags (attributes, index array, spatial, identical, multi-fulltext) improves validation accuracy across engines.
2253-2259: Preserving original indexes before attribute mutation: LGTMCloning and keeping the pre-change index list is necessary for correct revalidation later.
2430-2440: Using $originalIndexes in re-validation: LGTMFeeding original indexes into IndexValidator during attribute updates is correct.
Please confirm IndexValidator’s constructor order matches these added params across all call sites.
3392-3393: Append new index to collection metadata before adapter call: LGTM
3551-3552: Apply adapter castingAfter on read before decode: LGTMCorrect placement to normalize engine-specific types before decoding/filters.
3931-3931: Create path casting order improvements: LGTM
- Structure validator receives supportForAttributes: OK
- castingBefore prior to adapter->createDocument: OK
- castingAfter after the transaction: OK
Also applies to: 3937-3937, 3946-3946
4053-4053: Batch castingAfter on inserted docs: LGTM
4586-4586: Update path casting order improvements: LGTM
- Structure validator param: OK
- castingBefore just before adapter update: OK
- castingAfter immediately after: OK
Also applies to: 4597-4597, 4601-4601
4714-4715: No‑op: $updates castingBefore happens after update; move it right after encode$updates is cast only after adapter->updateDocuments has executed, so it has no effect. Cast earlier.
Apply this diff:
$updates = $this->encode($collection, $updates); + $updates = $this->adapter->castingBefore($collection, $updates); @@ - $updates = $this->adapter->castingBefore($collection, $updates);Also applies to: 4807-4808
4721-4721: Batch update castingAfter on returned docs: LGTMEnsures engine-specific types are normalized post-update.
Also applies to: 4811-4811
5434-5434: Upsert path casting: LGTM
- Structure validator param: OK
- castingBefore on old/new before upsert: OK
- castingAfter on results: OK
Also applies to: 5459-5461, 5495-5495
6428-6428: Find path: validator capability, cursor castingBefore, and result castingAfter: LGTM
- DocumentsValidator receives supportForAttributes: OK
- Cursor encode + castingBefore: correct for engine-specific cursors
- castingAfter per node before decode/relations: OK
Also applies to: 6483-6489, 6516-6516
7134-7145: Move Mongo date handling into adapter; also resolve attributes by both id and key
- Avoid isMongo() branching here; delegate datetime conversion to adapter for better encapsulation.
- Attribute lookup uses only $id; match by both id and key (case-insensitive) to avoid misses when keys differ.
Apply this diff:
public function convertQuery(Document $collection, Query $query): Query { @@ - $attribute = new Document(); - foreach ($attributes as $attr) { - if ($attr->getId() === $query->getAttribute()) { - $attribute = $attr; - } - } + $attribute = new Document(); + $lookup = $query->getAttribute(); + foreach ($attributes as $attr) { + $id = $attr->getId(); + $key = (string)$attr->getAttribute('key', $id); + if (\strcasecmp($id, $lookup) === 0 || \strcasecmp($key, $lookup) === 0) { + $attribute = $attr; + break; + } + } @@ if ($attribute->getAttribute('type') == Database::VAR_DATETIME) { $values = $query->getValues(); foreach ($values as $valueIndex => $value) { try { - $values[$valueIndex] = $this->adapter->isMongo() - ? $this->adapter->setUTCDatetime($value) - : DateTime::setTimezone($value); + // Delegate engine-specific normalization to the adapter + $values[$valueIndex] = $this->adapter->convertDatetimeForQuery($value); } catch (\Throwable $e) { throw new QueryException($e->getMessage(), $e->getCode(), $e); } } $query->setValues($values); }Note: This requires adding convertDatetimeForQuery(string $value): mixed to Adapter and implementing it per adapter (Mongo: UTCDateTime; SQL: DateTime::setTimezone). As per previous review comment.
Also applies to: 7157-7195
4033-4033: Potentially lost castingBefore when iterating by value — assign back to $documentsIf castingBefore returns a new Document instance, it won’t be written back to the $documents array.
Apply this diff:
- foreach ($documents as $document) { + foreach ($documents as $i => $document) { ... - $document = $this->adapter->castingBefore($collection, $document); + $documents[$i] = $this->adapter->castingBefore($collection, $document);Also applies to: 4042-4043
4797-4798: Bug: CastingBefore uses the wrong variable (drops prior encoding)You encode into $batch[$index] but then cast $document, discarding the encoded value.
Apply this diff:
- $batch[$index] = $this->encode($collection, $document); - $batch[$index] = $this->adapter->castingBefore($collection, $document); + $batch[$index] = $this->encode($collection, $document); + $batch[$index] = $this->adapter->castingBefore($collection, $batch[$index]);
46-47: Add deprecated alias for removed VAR_OBJECT_ID to avoid BC breakPublic API rename risks downstream breakage. Preserve the old constant as an alias.
Apply this diff:
public const VAR_ID = 'id'; - public const VAR_UUID7 = 'uuid7'; + public const VAR_UUID7 = 'uuid7'; + /** @deprecated Use VAR_UUID7 */ + public const VAR_OBJECT_ID = self::VAR_UUID7;src/Database/Adapter/Mongo.php (12)
140-156: Fix fatal database existence check
selectDatabase()requires the database name and returns a manager, notnull. Calling it without arguments will throw immediately. Please list the available database names and check membership instead.- return $this->getClient()->selectDatabase() != null; + $databases = (array)$this->getClient()->listDatabaseNames(); + + return \in_array($database, $databases, true);
280-302: Use per-attribute order slot when building index keysInside the attribute loop we still read
$orders[$i], which is the index number, not the position of the attribute inside the index. Multi-column indexes therefore apply the first order to every column. Iterate with the attribute index instead and pick$orders[$j].- foreach ($attributes as $attribute) { - $attribute = $this->filter($this->getInternalKeyForAttribute($attribute)); + foreach ($attributes as $j => $attributeId) { + $attribute = $this->filter($this->getInternalKeyForAttribute($attributeId)); @@ - case Database::INDEX_KEY: - $order = $this->getOrder($this->filter($orders[$i] ?? Database::ORDER_ASC)); + case Database::INDEX_KEY: + $order = $this->getOrder($this->filter($orders[$j] ?? Database::ORDER_ASC)); @@ - case Database::INDEX_UNIQUE: - $order = $this->getOrder($this->filter($orders[$i] ?? Database::ORDER_ASC)); + case Database::INDEX_UNIQUE: + $order = $this->getOrder($this->filter($orders[$j] ?? Database::ORDER_ASC));
311-330: Partial filter must target the stored field namesThe partial filter currently uses the raw attribute ids (
$attr), but the documents store the filtered/internal key, so the filter never matches. Resolve the internal key before adding it to the filter.- foreach ($attributes as $attr) { + foreach ($index->getAttribute('attributes') as $attr) { + $attrKey = $this->filter($this->getInternalKeyForAttribute($attr)); @@ - $partialFilter[$attr] = ['$exists' => true, '$type' => $attrType]; + $partialFilter[$attrKey] = ['$exists' => true, '$type' => $attrType];
469-476: Unset attributes via their internal key
$idis the external attribute identifier. Writing it straight into$unsetbreaks when the stored key differs (internal/reserved attributes, filtered chars). Always map throughgetInternalKeyForAttribute()(and filter) before issuing the update.- ['$unset' => [$id => '']], + [ + '$unset' => [ + $this->filter($this->getInternalKeyForAttribute($id)) => '' + ] + ],
493-498: Rename attributes using sanitized field namesThe
$renamemap also needs the stored field names. Passing the external ids risks renaming the wrong field or failing altogether.- ['$rename' => [$id => $name]], + [ + '$rename' => [ + $this->filter($this->getInternalKeyForAttribute($id)) => + $this->filter($this->getInternalKeyForAttribute($name)) + ] + ],
623-641: Filter attribute keys before $unset in deleteRelationshipEvery
$unsetstill uses the external key ($key/$twoWayKey). When the stored name differs (internal attributes, filtered chars, tenant prefixes), the update misses the target field. Resolve and filter the internal key once and reuse it in each branch.- switch ($type) { + $filteredKey = $this->filter($this->getInternalKeyForAttribute($key)); + $filteredTwoWayKey = $this->filter($this->getInternalKeyForAttribute($twoWayKey)); + + switch ($type) { @@ - $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$filteredKey => '']], multi: true); if ($twoWay) { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$filteredTwoWayKey => '']], multi: true); @@ - $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$filteredKey => '']], multi: true); @@ - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$filteredTwoWayKey => '']], multi: true);
1117-1123: Don’t replace entire documents on updatePassing the raw
$recordtoupdate()instructs MongoDB to replace the document, dropping any missing fields. Wrap the payload in$setto only mutate the intended attributes.- unset($record['_id']); // Don't update _id - - $this->client->update($name, $filters, $record); + unset($record['_id']); // Don't update _id + + $this->client->update($name, $filters, ['$set' => $record]);
1440-1448: Propagate delete errors instead of swallowing them
processException()returns the mapped exception, but we drop it here, so callers think the batch delete succeeded. Re-throw the processed exception.- } catch (MongoException $e) { - $this->processException($e); + } catch (MongoException $e) { + throw $this->processException($e); }
1524-1527: Escape roles before embedding them in the permission regexRoles can contain regex meta characters. Concatenating them directly enables regex injection and corrupts matching. Use
preg_quoteon each role first.- $roles = \implode('|', Authorization::getRoles()); - $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')]; + $roles = \implode('|', \array_map('preg_quote', Authorization::getRoles())); + $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')];
1771-1773: Harden count() permission filterSame regex-injection issue as above—escape each role before building the pattern.
- $roles = \implode('|', Authorization::getRoles()); - $filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')]; + $roles = \implode('|', \array_map('preg_quote', Authorization::getRoles())); + $filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')];
1849-1888: Sum pipeline must target the stored field and escape roles
$attributeis still the external name, so$sumreferences a field that doesn’t exist once the document is stored. Also, escape the roles in the permission regex like infind()/count().- if (Authorization::$status) { // skip if authorization is disabled - $roles = \implode('|', Authorization::getRoles()); - $filters['_permissions']['$in'] = [new Regex("read\(\".*(?:{$roles}).*\"\)", 'i')]; + if (Authorization::$status) { // skip if authorization is disabled + $roles = \implode('|', \array_map('preg_quote', Authorization::getRoles())); + $filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')]; } @@ - $pipeline[] = [ + $attributeKey = $this->filter($this->getInternalKeyForAttribute($attribute)); + + $pipeline[] = [ '$group' => [ '_id' => null, - 'total' => ['$sum' => '$' . $attribute], + 'total' => ['$sum' => '$' . $attributeKey], ], ];
2148-2171: Projection should use internal field namesSelections such as
$id/custom attributes are still exposed under their external names. You have to translate them to the stored field name before adding them to the projection; otherwise MongoDB omits the requested column.- $projection[$selection] = 1; + $projection[$this->filter($this->getInternalKeyForAttribute($selection))] = 1;src/Database/Adapter/Pool.php (3)
473-476: Delegated projection with widened return type looks goodSignature matches Adapter::getAttributeProjection and delegation is correct.
543-556: New capability delegations are correctDelegating getSupportForMultipleFulltextIndexes / getSupportForIdenticalIndexes / getSupportForOrderRandom is consistent with Adapter and concrete adapters.
573-596: Casting hooks and Mongo/casting capability delegations look goodMethod set is complete and returns are forwarded as expected.
tests/unit/Validator/IndexTest.php (1)
54-55: Updated Index validator constructor usage is correctPassing $indexes as the second argument aligns with the new Index signature and capability checks.
Also applies to: 103-107, 141-141, 188-188, 235-235, 273-273, 311-311, 348-348
tests/e2e/Adapter/Scopes/IndexTests.php (4)
165-177: Validator construction matches adapter capability surfaceSupplying adapter-driven flags into Index is correct and future-proof.
247-259: Second validator construction also correctConsistently supplies indexes and capability flags.
329-336: Index length zero test logic is validRelying on empty(0) to treat 0 as “unspecified” and trigger max-length validation is intentional and clear.
343-347: Update-attribute over limit test is validAssertion aligns with validator error path.
src/Database/Adapter.php (2)
1101-1121: New capability flags added to Adapter interface look goodSurface area (multiple fulltext, identical indexes, random order) is coherent with tests and adapters.
1357-1377: Narrow return documentation for setUTCDatetimeImprove PHPDoc to convey concrete return types across adapters (keeps signature unchanged).
- /** - * Set UTC Datetime - * - * @param string $value - * @return mixed - */ + /** + * Set UTC Datetime + * + * @param string $value ISO8601 or equivalent + * @return string|\DateTime|\MongoDB\BSON\UTCDateTime Adapter-specific UTC datetime representation + */ abstract public function setUTCDatetime(string $value): mixed;
| $name = $this->getNamespace() . '_' . $this->filter($collection->getId()); | ||
|
|
||
| $sequence = $document->getSequence(); | ||
|
|
||
| $document->removeAttribute('$sequence'); | ||
|
|
||
| if ($this->sharedTables) { | ||
| $document->setAttribute('$tenant', $this->getTenant()); | ||
| } | ||
|
|
||
| $record = $this->replaceChars('$', '_', (array)$document); | ||
|
|
||
| // Insert manual id if set | ||
| if (!empty($sequence)) { | ||
| $record['_id'] = $sequence; | ||
| } | ||
|
|
||
| $result = $this->insertDocument($name, $record); | ||
| $result = $this->replaceChars('_', '$', $result); |
There was a problem hiding this comment.
Fix tenant filters during insert
insertDocument() currently receives the fully-qualified collection name and then passes it to getTenantFilters(), which expects the plain collection id. As soon as shared tables are enabled the tenant filter becomes db_namespace_collection, so the inserted document cannot be read back. Pass the logical collection id instead and compute the namespaced collection inside the helper.
- $result = $this->insertDocument($name, $record);
+ $result = $this->insertDocument($collection->getId(), $record);- private function insertDocument(string $name, array $document): array
+ private function insertDocument(string $collection, array $document): array
{
+ $name = $this->getNamespace() . '_' . $this->filter($collection);
try {
$this->client->insert($name, $document);
@@
- if ($this->sharedTables) {
- $filters['_tenant'] = $this->getTenantFilters($name);
+ if ($this->sharedTables) {
+ $filters['_tenant'] = $this->getTenantFilters($collection);
}Also applies to: 1064-1074
🤖 Prompt for AI Agents
In src/Database/Adapter/Mongo.php around lines 865-883 (and likewise at
1064-1074), the code is passing the fully-qualified namespaced collection name
to functions that expect the logical collection id which breaks tenant filters
when sharedTables is enabled; change the calls so getTenantFilters and any
tenant-related logic receive the plain collection id (e.g.,
$collection->getId()), and only compute/concatenate the namespaced collection
name (namespace + '_' + filtered id) inside the helper or immediately before the
actual DB insert; apply the same adjustment in the other block at 1064-1074 so
tenant filters are derived from the logical id and the physical collection name
is computed solely for the insert operation.
| if ($max) { | ||
| $filters[$attribute] = ['$lte' => $max]; | ||
| } | ||
|
|
||
| if ($min) { | ||
| $filters[$attribute] = ['$gte' => $min]; | ||
| } |
There was a problem hiding this comment.
Preserve both min and max constraints in increment
When both $min and $max are provided, the second assignment overwrites the first, so only one bound is enforced. Merge them into the same constraint array.
- if ($max) {
- $filters[$attribute] = ['$lte' => $max];
- }
-
- if ($min) {
- $filters[$attribute] = ['$gte' => $min];
- }
+ if ($max !== null || $min !== null) {
+ $filters[$attribute] = [];
+ if ($max !== null) {
+ $filters[$attribute]['$lte'] = $max;
+ }
+ if ($min !== null) {
+ $filters[$attribute]['$gte'] = $min;
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ($max) { | |
| $filters[$attribute] = ['$lte' => $max]; | |
| } | |
| if ($min) { | |
| $filters[$attribute] = ['$gte' => $min]; | |
| } | |
| // Preserve both min and max constraints when incrementing | |
| - if ($max) { | |
| - $filters[$attribute] = ['$lte' => $max]; | |
| - } | |
| - | |
| - if ($min) { | |
| - $filters[$attribute] = ['$gte' => $min]; | |
| if ($max !== null || $min !== null) { | |
| $filters[$attribute] = []; | |
| if ($max !== null) { | |
| $filters[$attribute]['$lte'] = $max; | |
| } | |
| if ($min !== null) { | |
| $filters[$attribute]['$gte'] = $min; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/Database/Adapter/Mongo.php around lines 1368 to 1374, the code assigns
$filters[$attribute] twice so when both $min and $max are provided the second
assignment overwrites the first; modify the logic so both bounds are merged into
the same constraint array for that attribute (e.g. ensure $filters[$attribute]
contains both '$lte' => $max and '$gte' => $min by creating or merging into the
existing array rather than replacing it).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Database/Adapter/SQL.php (1)
518-527: Fix spatial binding: incorrect check and missing WKT conversion
- Using isset($spatialAttributes[$attributeName]) is wrong; $spatialAttributes is a list.
- Spatial arrays must be converted to WKT for ST_GeomFromText placeholders; JSON-encoding breaks writes.
Apply both fixes.
- $attributeIndex = 0; - foreach ($attributes as $attributeName => $value) { - if (!isset($spatialAttributes[$attributeName]) && is_array($value)) { - $value = json_encode($value); - } - - $bindKey = 'key_' . $attributeIndex; - $value = (is_bool($value)) ? (int)$value : $value; - $stmt->bindValue(':' . $bindKey, $value, $this->getPDOType($value)); - $attributeIndex++; - } + $attributeIndex = 0; + foreach ($attributes as $attributeName => $value) { + $isSpatial = \in_array($attributeName, $spatialAttributes, true); + if ($isSpatial && \is_array($value)) { + $value = $this->convertArrayToWKT($value); + } elseif (!$isSpatial && \is_array($value)) { + $value = \json_encode($value); + } + + $bindKey = 'key_' . $attributeIndex; + $value = (\is_bool($value)) ? (int)$value : $value; + $stmt->bindValue(':' . $bindKey, $value, $this->getPDOType($value)); + $attributeIndex++; + }src/Database/Database.php (1)
4714-4723: Cast updates payload before batch update (missing currently)
$updatesshould be passed throughcastingBeforeimmediately after encode so adapters (e.g., Mongo) receive correctly-typed values.Apply this diff:
$updates = $this->encode($collection, $updates); + $updates = $this->adapter->castingBefore($collection, $updates); // Check new document structure $validator = new PartialStructure(
🧹 Nitpick comments (5)
Dockerfile (1)
23-40: Trim image size: use virtual build deps and remove them after buildBuild toolchains remain in the final image because final is FROM compile. Install build deps with a virtual package and delete them after compiling extensions.
Apply this diff:
-RUN apk update && apk add --no-cache \ - postgresql-libs \ - postgresql-dev \ - make \ - automake \ - autoconf \ - gcc \ - g++ \ - git \ - brotli-dev \ - linux-headers \ - docker-cli \ - docker-cli-compose \ - && pecl install mongodb-$PHP_MONGODB_VERSION \ - && docker-php-ext-enable mongodb \ - && docker-php-ext-install opcache pgsql pdo_mysql pdo_pgsql \ - && apk del postgresql-dev \ - && rm -rf /var/cache/apk/* +RUN apk update \ + && apk add --no-cache \ + postgresql-libs \ + git \ + brotli-dev \ + docker-cli \ + docker-cli-compose \ + && apk add --no-cache --virtual .build-deps \ + postgresql-dev \ + make \ + automake \ + autoconf \ + gcc \ + g++ \ + linux-headers \ + && pecl install mongodb-$PHP_MONGODB_VERSION \ + && docker-php-ext-enable mongodb \ + && docker-php-ext-install opcache pgsql pdo_mysql pdo_pgsql \ + && apk del .build-deps \ + && rm -rf /var/cache/apk/*src/Database/Adapter/SQL.php (1)
2005-2008: Normalize PDOException to domain exceptionsMap driver codes to Duplicate/Timeout/Database exceptions for better error semantics.
- protected function processException(PDOException $e): \Exception - { - return $e; - } + protected function processException(PDOException $e): \Exception + { + $code = (string)$e->getCode(); + $message = $e->getMessage(); + + // Unique/duplicate constraint violations (MySQL/MariaDB/Postgres) + if (in_array($code, ['23000', '23505'], true)) { + return new DuplicateException($message, $e->getCode(), $e); + } + + // Timeouts (heuristic) + if (str_contains(strtolower($message), 'timeout')) { + return new TimeoutException($message, $e->getCode(), $e); + } + + return new DatabaseException($message, $e->getCode(), $e); + }docker-compose.yml (2)
86-117: Healthcheck robustness and keyfile handling
- Healthcheck uses bash; ensure the mongo image includes bash (it usually does, but verify).
- Consider using service hostname in rs.initiate host to avoid localhost-only membership.
If needed, switch to CMD-SHELL without bash and use service DNS:
healthcheck: test: > mongosh -u root -p password --authenticationDatabase admin --quiet --eval " try { rs.status().ok } catch (e) { rs.initiate({_id: 'rs0', members: [{_id: 0, host: 'mongo:27017'}]}) } " interval: 10s timeout: 10s retries: 10 start_period: 30s
95-97: Prefer Docker secrets for the Mongo keyfile over bind mountYou declared a secret but aren’t using it. Using secrets avoids world-readable files and manages permissions cleanly.
Example change:
services: mongo: secrets: - source: mongo_keyfile target: /etc/mongo-keyfile mode: 0400 volumes: - mongo-data:/data/db # remove the ./tests/resources/mongo/mongo-keyfile bind mount secrets: mongo_keyfile: file: ./tests/resources/mongo/mongo-keyfileNote: Update entrypoint.sh to skip copying from /tmp/keyfile when using secrets.
Also applies to: 187-190
src/Database/Database.php (1)
2346-2350: Fix method casing for readabilityUse the canonical method name casing.
Apply this diff:
- ->setattribute('key', $newKey ?? $id) + ->setAttribute('key', $newKey ?? $id)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
Dockerfile(2 hunks)composer.json(2 hunks)docker-compose.yml(3 hunks)src/Database/Adapter.php(3 hunks)src/Database/Adapter/Mongo.php(1 hunks)src/Database/Adapter/Pool.php(3 hunks)src/Database/Adapter/SQL.php(2 hunks)src/Database/Database.php(24 hunks)tests/resources/mongo/entrypoint.sh(1 hunks)tests/resources/mongo/mongo-keyfile(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/resources/mongo/mongo-keyfile
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Database/Adapter/Pool.php
- composer.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-30T19:17:53.630Z
Learnt from: ArnabChatterjee20k
PR: utopia-php/database#642
File: src/Database/Validator/PartialStructure.php:43-52
Timestamp: 2025-07-30T19:17:53.630Z
Learning: In PartialStructure validator, when filtering for required attributes validation using the $requiredAttributes parameter, $this->attributes should be used instead of the merged $attributes array because this validation is specifically for internal attributes like $createdAt and $updatedAt that are defined in the base Structure class, not collection-specific attributes.
Applied to files:
src/Database/Adapter/Mongo.php
🧬 Code graph analysis (4)
src/Database/Adapter.php (3)
src/Database/Adapter/Mongo.php (9)
getSupportForMultipleFulltextIndexes(2720-2723)getSupportForIdenticalIndexes(2729-2732)getSupportForOrderRandom(2739-2742)getAttributeProjection(2304-2329)castingBefore(1102-1151)castingAfter(1043-1093)getSupportForInternalCasting(2409-2412)getSupportForUTCCasting(2414-2417)setUTCDatetime(2419-2422)src/Database/Adapter/Pool.php (9)
getSupportForMultipleFulltextIndexes(543-546)getSupportForIdenticalIndexes(548-551)getSupportForOrderRandom(553-556)getAttributeProjection(473-476)castingBefore(573-576)castingAfter(578-581)getSupportForInternalCasting(583-586)getSupportForUTCCasting(588-591)setUTCDatetime(593-596)src/Database/Adapter/SQL.php (9)
getSupportForMultipleFulltextIndexes(1530-1533)getSupportForIdenticalIndexes(1540-1543)getSupportForOrderRandom(1550-1553)getAttributeProjection(1949-1978)castingBefore(1565-1568)castingAfter(1570-1573)getSupportForInternalCasting(1520-1523)getSupportForUTCCasting(1555-1558)setUTCDatetime(1560-1563)
src/Database/Adapter/SQL.php (4)
src/Database/Adapter.php (9)
getSupportForInternalCasting(1362-1362)getSupportForMultipleFulltextIndexes(1106-1106)getSupportForIdenticalIndexes(1114-1114)getSupportForOrderRandom(1121-1121)getSupportForUTCCasting(1369-1369)setUTCDatetime(1377-1377)castingBefore(1347-1347)castingAfter(1355-1355)getAttributeProjection(1186-1186)src/Database/Adapter/Mongo.php (9)
getSupportForInternalCasting(2409-2412)getSupportForMultipleFulltextIndexes(2720-2723)getSupportForIdenticalIndexes(2729-2732)getSupportForOrderRandom(2739-2742)getSupportForUTCCasting(2414-2417)setUTCDatetime(2419-2422)castingBefore(1102-1151)castingAfter(1043-1093)getAttributeProjection(2304-2329)src/Database/Adapter/Pool.php (9)
getSupportForInternalCasting(583-586)getSupportForMultipleFulltextIndexes(543-546)getSupportForIdenticalIndexes(548-551)getSupportForOrderRandom(553-556)getSupportForUTCCasting(588-591)setUTCDatetime(593-596)castingBefore(573-576)castingAfter(578-581)getAttributeProjection(473-476)src/Database/Document.php (1)
Document(12-470)
src/Database/Database.php (9)
src/Database/Adapter.php (14)
getMaxIndexLength(876-876)getInternalIndexesKeys(1291-1291)getSupportForIndexArray(935-935)getSupportForSpatialAttributes(1057-1057)getSupportForSpatialIndexNull(1064-1064)getSupportForSpatialIndexOrder(1078-1078)getSupportForAttributes(914-914)getSupportForMultipleFulltextIndexes(1106-1106)getSupportForIdenticalIndexes(1114-1114)castingAfter(1355-1355)castingBefore(1347-1347)withTransaction(377-424)getSupportForUTCCasting(1369-1369)setUTCDatetime(1377-1377)src/Database/Adapter/Mongo.php (14)
getMaxIndexLength(2848-2851)getInternalIndexesKeys(2858-2861)getSupportForIndexArray(2399-2402)getSupportForSpatialAttributes(2655-2658)getSupportForSpatialIndexNull(2665-2668)getSupportForSpatialIndexOrder(2684-2687)getSupportForAttributes(2430-2433)getSupportForMultipleFulltextIndexes(2720-2723)getSupportForIdenticalIndexes(2729-2732)castingAfter(1043-1093)castingBefore(1102-1151)withTransaction(99-124)getSupportForUTCCasting(2414-2417)setUTCDatetime(2419-2422)src/Database/Adapter/Pool.php (13)
getMaxIndexLength(308-311)getInternalIndexesKeys(488-491)getSupportForIndexArray(338-341)getSupportForSpatialAttributes(428-431)getSupportForSpatialIndexNull(433-436)getSupportForSpatialIndexOrder(523-526)getSupportForAttributes(323-326)getSupportForMultipleFulltextIndexes(543-546)getSupportForIdenticalIndexes(548-551)castingAfter(578-581)castingBefore(573-576)getSupportForUTCCasting(588-591)setUTCDatetime(593-596)src/Database/Adapter/SQL.php (13)
getMaxIndexLength(1845-1851)getInternalIndexesKeys(1897-1900)getSupportForIndexArray(1460-1463)getSupportForSpatialAttributes(1490-1493)getSupportForSpatialIndexNull(1500-1503)getSupportForSpatialIndexOrder(1510-1513)getSupportForAttributes(896-899)getSupportForMultipleFulltextIndexes(1530-1533)getSupportForIdenticalIndexes(1540-1543)castingAfter(1570-1573)castingBefore(1565-1568)getSupportForUTCCasting(1555-1558)setUTCDatetime(1560-1563)src/Database/Adapter/MariaDB.php (5)
getInternalIndexesKeys(1772-1775)getSupportForIndexArray(1845-1848)getSupportForSpatialAttributes(1850-1853)getSupportForSpatialIndexNull(1860-1863)getSupportForSpatialIndexOrder(1879-1882)src/Database/Adapter/MySQL.php (2)
getSupportForIndexArray(130-136)getSupportForSpatialIndexOrder(179-182)src/Database/Adapter/SQLite.php (3)
getSupportForSpatialAttributes(971-974)getSupportForSpatialIndexNull(976-979)getSupportForSpatialIndexOrder(1259-1262)src/Database/Adapter/Postgres.php (3)
getSupportForSpatialAttributes(1959-1962)getSupportForSpatialIndexNull(1969-1972)getSupportForSpatialIndexOrder(1989-1992)src/Database/DateTime.php (2)
DateTime(7-86)setTimezone(58-67)
src/Database/Adapter/Mongo.php (10)
src/Database/Database.php (29)
Database(37-7421)setTimeout(872-877)clearTimeout(885-888)withTransaction(1214-1217)ping(1224-1227)create(1243-1262)getNamespace(751-754)delete(1301-1315)createCollection(1330-1468)createAttribute(1698-1753)createAttributes(1769-1861)deleteAttribute(2506-2574)renameAttribute(2589-2653)createRelationship(2673-2850)updateRelationship(2865-3052)getDocument(3467-3590)deleteRelationship(3066-3180)createIndex(3255-3415)renameIndex(3196-3234)deleteIndex(3429-3454)find(6406-6535)createDocument(3862-3958)getTenant(1054-1057)createDocuments(3974-4077)updateDocument(4414-4621)updateDocuments(4644-4838)upsertDocuments(5275-5290)count(6632-6668)sum(6683-6712)src/Database/Adapter.php (39)
Adapter(16-1379)setTimeout(309-309)clearTimeout(322-326)withTransaction(377-424)startTransaction(336-336)commitTransaction(348-348)rollbackTransaction(359-359)ping(474-474)create(488-488)getNamespace(107-110)delete(515-515)createCollection(525-525)filter(1218-1227)createAttribute(557-557)createAttributes(568-568)deleteAttribute(594-594)renameAttribute(604-604)createRelationship(615-615)updateRelationship(631-631)getDocument(691-691)deleteRelationship(645-645)createIndex(670-670)renameIndex(655-655)deleteIndex(680-680)find(802-802)createDocument(701-701)getTenant(216-219)castingAfter(1355-1355)castingBefore(1347-1347)createDocuments(713-713)updateDocument(725-725)updateDocuments(740-740)upsertDocuments(752-756)count(825-825)sum(814-814)getLimitForString(850-850)getLimitForInt(857-857)getSupportForAttributes(914-914)getIdAttributeType(890-890)src/Database/Exception.php (1)
Exception(7-21)src/Database/Change.php (1)
Change(5-32)src/Database/DateTime.php (2)
DateTime(7-86)format(29-32)src/Database/Document.php (4)
Document(12-470)getId(63-66)find(304-322)getTenant(178-187)src/Database/Query.php (3)
Query(8-1091)equal(434-437)limit(622-625)src/Database/Validator/Authorization.php (1)
Authorization(7-225)src/Database/Adapter/Pool.php (33)
setTimeout(98-101)startTransaction(103-106)commitTransaction(108-111)rollbackTransaction(113-116)ping(123-126)create(133-136)delete(148-151)createCollection(153-156)createAttribute(168-171)createAttributes(173-176)deleteAttribute(183-186)renameAttribute(188-191)createRelationship(193-196)updateRelationship(198-201)getDocument(223-226)deleteRelationship(203-206)createIndex(213-216)renameIndex(208-211)deleteIndex(218-221)find(263-266)createDocument(228-231)castingAfter(578-581)castingBefore(573-576)createDocuments(233-236)updateDocument(238-241)updateDocuments(243-246)upsertDocuments(248-251)count(273-276)sum(268-271)getLimitForString(288-291)getLimitForInt(293-296)getSupportForAttributes(323-326)getIdAttributeType(508-511)src/Database/Adapter/SQL.php (21)
startTransaction(59-82)commitTransaction(87-113)rollbackTransaction(118-137)ping(146-151)createAttribute(234-248)createAttributes(258-285)deleteAttribute(326-339)renameAttribute(297-314)getDocument(351-417)find(2416-2595)castingAfter(1570-1573)castingBefore(1565-1568)createDocuments(2030-2163)updateDocuments(453-693)upsertDocuments(2172-2329)count(2607-2669)sum(2682-2745)getLimitForString(833-836)getLimitForInt(843-846)getSupportForAttributes(896-899)getIdAttributeType(1837-1840)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Setup & Build Docker Image
🔇 Additional comments (50)
src/Database/Adapter.php (3)
1101-1121: New capability flags look consistentThe added capability methods are clear and align with adapter-specific behavior.
1184-1187: Projection return type widened appropriatelyReturning mixed enables adapters (SQL string vs Mongo array) without breaking callers.
1341-1377: Doc polish: clarify castingBefore summary and document union return for setUTCDatetimeTighten PHPDoc without changing signatures.
Apply this diff:
- /** - * Returns the document after casting - * @param Document $collection - * @param Document $document - * @return Document - */ + /** + * Returns the document before adapter-specific casting (pre-insert/update) + * @param Document $collection + * @param Document $document + * @return Document + */ abstract public function castingBefore(Document $collection, Document $document): Document; @@ - /** - * Set UTC Datetime - * - * @param string $value - * @return mixed - */ + /** + * Set UTC Datetime + * + * @param string $value + * @return string|\DateTimeInterface|\MongoDB\BSON\UTCDateTime + */ abstract public function setUTCDatetime(string $value): mixed;src/Database/Adapter/SQL.php (4)
1515-1573: Adapter capability stubs are sane defaultsReturning false for internal/UTC casting and true for the index-related flags matches SQL behavior.
1946-1950: Projection API matches Adapter contractMixed return type retained while SQL implementation keeps returning string, which is correct.
2048-2051: Use strict comparison for sequence presence checkAvoid type juggling in a boolean comparison that can throw incorrectly.
- } elseif ($hasSequence == empty($document->getSequence())) { + } elseif ($hasSequence === empty($document->getSequence())) {
1994-2003: Remove duplicate escapeWildcards; inherit from AdapterThis duplicates Adapter::escapeWildcards with identical logic.
- protected function escapeWildcards(string $value): string - { - $wildcards = ['%', '_', '[', ']', '^', '-', '.', '*', '+', '?', '(', ')', '{', '}', '|']; - - foreach ($wildcards as $wildcard) { - $value = \str_replace($wildcard, "\\$wildcard", $value); - } - - return $value; - }docker-compose.yml (2)
118-132: ME_CONFIG_MONGODB_URL aligns with credentialsURL matches root/password and admin authSource; looks good.
Ensure mongo-express loads when replica set is healthy; if it fails on rs init delay, consider adding ME_CONFIG_MONGODB_URL with replicaSet=rs0 or directConnection=true (already set).
134-135: Version bumps for MySQL/RedisImage upgrades look fine. Validate any test flakiness due to version changes (MySQL defaults can shift between patch releases).
Also applies to: 150-151, 166-166, 174-174
Dockerfile (1)
19-20: No action needed: mongodb PECL 2.1.1 supports PHP 8.3 (min PHP 8.1)
No bump required.src/Database/Database.php (17)
46-46: Add deprecated alias to avoid BC break for renamed object-id constantAdd a deprecated alias
VAR_OBJECT_IDpointing toVAR_UUID7to preserve public API compatibility.Apply this diff:
public const VAR_UUID7 = 'uuid7'; + /** @deprecated Use VAR_UUID7 */ + public const VAR_OBJECT_ID = self::VAR_UUID7;
2252-2260: Capturing original indexes before modifications is correctStoring a deep copy of indexes before altering attributes avoids “validating against already-mutated metadata.” Good change.
3392-3393: Pre-append index to collection metadata before persistence: OKThe in-memory append is fine, given persistence to METADATA happens afterwards.
3551-3552: Post-read casting hook is placed correctlyApplying adapter->castingAfter before decode/relationship resolution is appropriate.
3937-3947: Pre-/post-write casting hooks around createDocument are correctGood ordering: encode -> validate -> castingBefore -> adapter create -> castingAfter -> decode.
4001-4043: Foreach by value risks losing the casted instance; assign back into $documentsIf adapter->castingBefore returns a new Document, the new instance won’t be persisted into $documents, and the batch may use the old instance.
Apply this diff:
- foreach ($documents as $document) { + foreach ($documents as $i => $document) { ... - $document = $this->adapter->castingBefore($collection, $document); + $documents[$i] = $this->adapter->castingBefore($collection, $document);
4053-4053: Post-batch castingAfter before decode is correctHook placement looks good.
4596-4602: Pre-/post-write casting in updateDocument is correctGood ordering around adapter->updateDocument.
4793-4799: Bug: castingBefore uses wrong variable and overwrites encoded docYou encode into
$batch[$index], then cast$documentand assign it over$batch[$index], dropping the encoding. Cast the encoded instance instead.Apply this diff:
- $batch[$index] = $this->encode($collection, $document); - $batch[$index] = $this->adapter->castingBefore($collection, $document); + $batch[$index] = $this->encode($collection, $document); + $batch[$index] = $this->adapter->castingBefore($collection, $batch[$index]);
5459-5461: Upsert: pre-cast old/new before adapter call is correctThis ensures adapter receives payload in its native types.
5495-5496: Upsert: post-cast after adapter returns is correctGood consistency with other paths.
6483-6489: Cursor normalization is correctEncoding + castingBefore + array copy for cursor passed to adapter is appropriate.
6516-6517: Find: apply castingAfter before decodeHook placement looks correct.
7157-7195: Attribute lookup in convertQuery is O(n) and matches only by $id
- Functional: if Query::getAttribute() ever returns “key” instead of “$id”, the lookup may miss. Consider matching by both
$idandkeycase-insensitively.- Perf: build a map once per call to avoid repeated scans.
Apply this diff:
- $attribute = new Document(); - - foreach ($attributes as $attr) { - if ($attr->getId() === $query->getAttribute()) { - $attribute = $attr; - } - } + $attribute = new Document(); + $needle = \strtolower($query->getAttribute()); + foreach ($attributes as $attr) { + $id = \strtolower($attr->getId()); + $key = \strtolower($attr->getAttribute('key', $attr->getId())); + if ($id === $needle || $key === $needle) { + $attribute = $attr; + break; + } + }
4807-4808: No-op: late casting of $updates happens after update execution
$updates = adapter->castingBefore(...)must occur beforeadapter->updateDocuments(...). Remove here to avoid confusion and duplicate work.Apply this diff:
- $updates = $this->adapter->castingBefore($collection, $updates);And rely on the earlier added pre-call cast.
1407-1421: Validator args extension: looks good; confirm adapter API coveragePassing additional adapter capability flags into IndexValidator is sound. Please confirm all adapters implement these getters to prevent fatal errors at runtime.
Run:
7134-7145: Public static → instance method change: verify no external static callsChanging
convertQueriesto an instance method is a BC break if used statically. Please verify no static invocations remain.Generate evidence:
src/Database/Adapter/Mongo.php (23)
1177-1178: Use strict comparison for type safety.Line 1177 uses loose comparison (
==) which could lead to unexpected type coercion between boolean and other types.Apply this diff to use strict comparison:
- } elseif ($hasSequence == empty($sequence)) { + } elseif ($hasSequence === empty($sequence)) {
2505-2508: Enable batch operations support flag.The method returns
falsebut batch operations likecreateDocumentsandupsertDocumentsare implemented, so this capability should be advertised.Apply this diff to enable the flag:
public function getSupportForBatchOperations(): bool { - return false; + return true; }
53-53: Fix uncommented$timeoutproperty declaration.Line 53 declares the
$timeoutproperty as commented out, but Lines 83 and 90 access this property directly. This will cause "undefined property" errors.Apply this diff to uncomment the property declaration:
- //protected ?int $timeout = null; + protected ?int $timeout = null;
71-75: Add error handling for MongoDB connection failures.The constructor connects immediately without handling potential connection failures, which could cause unhandled exceptions.
Apply this diff to add proper error handling:
public function __construct(Client $client) { $this->client = $client; - $this->client->connect(); + try { + $this->client->connect(); + } catch (MongoException $e) { + throw new DatabaseException('Failed to connect to MongoDB: ' . $e->getMessage(), $e->getCode(), $e); + } }
300-300: Fix incorrect database existence check.Line 300 calls
selectDatabase()without parameters and compares to null, which is incorrect.selectDatabase()requires a database name and always returns a database object.Apply this diff to fix the database existence check:
- return $this->getClient()->selectDatabase() != null; + $databases = $this->getClient()->listDatabaseNames(); + return in_array($database, $databases);
425-447: Fix incorrect array indexing in attribute order processing.The variable
$ifrom the outer$indexesloop is incorrectly used to access the$ordersarray, which corresponds to the inner$attributesloop. This causes incorrect order assignments.Apply this diff to fix the indexing:
- foreach ($attributes as $attribute) { + foreach ($attributes as $j => $attribute) { $attribute = $this->filter($this->getInternalKeyForAttribute($attribute)); switch ($index->getAttribute('type')) { case Database::INDEX_KEY: - $order = $this->getOrder($this->filter($orders[$i] ?? Database::ORDER_ASC)); + $order = $this->getOrder($this->filter($orders[$j] ?? Database::ORDER_ASC)); break; case Database::INDEX_FULLTEXT: // MongoDB fulltext index is just 'text' // Not using Database::INDEX_KEY for clarity $order = 'text'; break; case Database::INDEX_UNIQUE: - $order = $this->getOrder($this->filter($orders[$i] ?? Database::ORDER_ASC)); + $order = $this->getOrder($this->filter($orders[$j] ?? Database::ORDER_ASC)); $unique = true; break; default: // index not supported return false; } $key[$attribute] = $order; }
461-471: Use internal filtered attribute names in partial filters.Line 471 uses the raw
$attrvariable in the partial filter, but it should use the filtered internal attribute name to match the stored keys.Apply this diff to fix the partial filter keys:
foreach ($attributes as $attr) { + $attrKey = $this->filter($this->getInternalKeyForAttribute($attr)); // Find the matching attribute in collectionAttributes to get its type $attrType = 'string'; // Default fallback foreach ($collectionAttributes as $collectionAttr) { if ($collectionAttr->getId() === $attr) { $attrType = $this->getMongoTypeCode($collectionAttr->getAttribute('type')); break; } } // Use both $exists: true and $type to exclude nulls and ensure correct type - $partialFilter[$attr] = ['$exists' => true, '$type' => $attrType]; + $partialFilter[$attrKey] = ['$exists' => true, '$type' => $attrType]; }
612-624: Filter attribute name in deleteAttribute operation.Lines 619 uses the raw attribute ID in the
$unsetoperation, which may not match the stored internal key format.Apply this diff to filter the attribute name:
$this->getClient()->update( $collection, [], - ['$unset' => [$id => '']], + ['$unset' => [$this->filter($this->getInternalKeyForAttribute($id)) => '']], multi: true );
634-646: Filter both source and destination attribute names in renameAttribute.Line 641 uses raw attribute names in the
$renameoperation, which may not match stored internal keys.Apply this diff to filter both attribute names:
$this->getClient()->update( $collection, [], - ['$rename' => [$id => $name]], + ['$rename' => [ + $this->filter($this->getInternalKeyForAttribute($id)) => + $this->filter($this->getInternalKeyForAttribute($name)) + ]], multi: true );
754-796: Filter attribute keys in deleteRelationship $unset operations.Lines 769, 771, 776, 778, 783, and 785 use raw attribute keys in
$unsetoperations without filtering them to match stored internal keys.Apply this diff to filter all attribute keys:
case Database::RELATION_ONE_TO_ONE: - $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($key)) => '']], multi: true); if ($twoWay) { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($twoWayKey)) => '']], multi: true); } break; case Database::RELATION_ONE_TO_MANY: if ($side === Database::RELATION_SIDE_PARENT) { - $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($key)) => '']], multi: true); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($twoWayKey)) => '']], multi: true); } break; case Database::RELATION_MANY_TO_ONE: if ($side === Database::RELATION_SIDE_CHILD) { - $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($key)) => '']], multi: true); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($twoWayKey)) => '']], multi: true); } break;
864-872: Fix createIndex to use attribute-keyed indexAttributeTypes map.Lines 864-867 use
$indexAttributeTypes[$i]with numeric indexing, but the parameter should be keyed by attribute names according to the PHPDoc and usage patterns.Apply this diff to use attribute-name keys:
foreach ($attributes as $i => $attr) { - $attrType = $indexAttributeTypes[$i] ?? Database::VAR_STRING; // Default to string if type not provided + $attrType = $indexAttributeTypes[$attr] ?? Database::VAR_STRING; // Default to string if type not provided $attrType = $this->getMongoTypeCode($attrType); $partialFilter[$attr] = ['$exists' => true, '$type' => $attrType]; }
1023-1025: Convert sequence to ObjectId for proper MongoDB _id format.Line 1024 sets the
_idfield directly from a raw string sequence, but MongoDB expects_idto be a BSON ObjectId type for consistency.Apply this diff to convert the sequence to ObjectId:
// Insert manual id if set if (!empty($sequence)) { - $record['_id'] = $sequence; + $record['_id'] = new ObjectId($sequence); }
1027-1027: Fix insertDocument call to pass collection ID instead of full name.Line 1027 passes
$name(the full namespaced collection name) toinsertDocument, but the method expects the collection ID for proper tenant filter handling.Apply this diff to pass the collection ID:
- $result = $this->insertDocument($name, $this->removeNullKeys($record), $options); + $result = $this->insertDocument($collection->getId(), $this->removeNullKeys($record), $options);
1211-1236: Refactor insertDocument to properly handle collection-based tenant filters.The
insertDocumentmethod incorrectly passes the full namespaced collection name togetTenantFilters, which breaks tenant filtering logic for shared tables.Apply this diff to fix tenant filter handling:
- private function insertDocument(string $name, array $document, array $options = []): array + private function insertDocument(string $collection, array $document, array $options = []): array { try { + $name = $this->getNamespace() . '_' . $this->filter($collection); $result = $this->client->insert($name, $document, $options); $filters = []; $filters['_uid'] = $document['_uid']; if ($this->sharedTables) { - $filters['_tenant'] = $this->getTenantFilters($name); + $filters['_tenant'] = $this->getTenantFilters($collection); }
1267-1267: Use $set operator for MongoDB updates to avoid replacing entire document.Line 1267 passes the raw
$recorddirectly to the update operation, which risks replacing the entire document instead of updating specific fields.Apply this diff to wrap the update in
$set:- $this->client->update($name, $filters, $record, $options); + $this->client->update($name, $filters, ['$set' => $record], $options);
1288-1318: Remove stray semicolon and return actual modified count.Line 1290 has a stray semicolon, and Line 1317 always returns 1 instead of the actual number of modified documents.
Apply this diff to fix both issues:
public function updateDocuments(Document $collection, Document $updates, array $documents): int { - ; $name = $this->getNamespace() . '_' . $this->filter($collection->getId()); $options = $this->getTransactionOptions(); $queries = [ Query::equal('$sequence', \array_map(fn ($document) => $document->getSequence(), $documents)) ]; $filters = $this->buildFilters($queries); if ($this->sharedTables) { $filters['_tenant'] = $this->getTenantFilters($collection->getId()); } $record = $updates->getArrayCopy(); $record = $this->replaceChars('$', '_', $record); $updateQuery = [ '$set' => $record, ]; try { - $this->client->update($name, $filters, $updateQuery, multi: true, options: $options); + $modified = $this->client->update($name, $filters, $updateQuery, multi: true, options: $options); } catch (MongoException $e) { throw $this->processException($e); } - return 1; + return (int)($modified ?? 0); }
1519-1525: Preserve both min and max constraints in increment operation.When both
$minand$maxare provided, the second assignment on Line 1524 overwrites the first on Line 1520, so only one constraint is enforced.Apply this diff to merge both constraints:
- if ($max) { - $filters[$attribute] = ['$lte' => $max]; - } - - if ($min) { - $filters[$attribute] = ['$gte' => $min]; - } + if ($max !== null || $min !== null) { + $filters[$attribute] = []; + if ($max !== null) { + $filters[$attribute]['$lte'] = $max; + } + if ($min !== null) { + $filters[$attribute]['$gte'] = $min; + } + }
1594-1604: Propagate deleteDocuments errors instead of swallowing them.The catch block on Line 1601 calls
processExceptionbut doesn't re-throw the exception, causing silent failures.Apply this diff to re-throw processed exceptions:
} catch (MongoException $e) { - $this->processException($e); + throw $this->processException($e); }
1678-1681: Escape regex metacharacters in roles for permission filters.Lines 1679 and 2018 join roles directly into regex patterns without escaping, which could cause regex injection or malformed patterns if roles contain special characters.
Apply this diff to escape roles in both locations:
if (Authorization::$status) { - $roles = \implode('|', Authorization::getRoles()); + $roles = \implode('|', \array_map('preg_quote', Authorization::getRoles())); $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')]; }And also fix it in the
sum()method:if (Authorization::$status) { // skip if authorization is disabled - $roles = \implode('|', Authorization::getRoles()); + $roles = \implode('|', \array_map('preg_quote', Authorization::getRoles())); $filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')]; }
2004-2040: Map attribute to internal key and escape roles in sum method.Line 2040 uses the raw
$attributedirectly in the aggregation pipeline, which may not match the stored internal key format. Also contains the same regex injection issue as noted above.Apply this diff to fix both issues:
public function sum(Document $collection, string $attribute, array $queries = [], ?int $max = null): float|int { $name = $this->getNamespace() . '_' . $this->filter($collection->getId()); + $attribute = $this->filter($this->getInternalKeyForAttribute($attribute)); // queries $queries = array_map(fn ($query) => clone $query, $queries); $filters = $this->buildFilters($queries); if ($this->sharedTables) { $filters['_tenant'] = $this->getTenantFilters($collection->getId()); } // permissions if (Authorization::$status) { // skip if authorization is disabled - $roles = \implode('|', Authorization::getRoles()); + $roles = \implode('|', \array_map('preg_quote', Authorization::getRoles())); $filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')]; }
2304-2329: Use internal/stored keys in projection instead of external attribute names.Line 2319 uses external attribute selections directly in projection, but MongoDB queries should use the actual stored field names (internal keys).
Apply this diff to map selections to internal keys:
foreach ($selections as $selection) { // Skip internal attributes since all are selected by default if (\in_array($selection, $internalKeys)) { continue; } - $projection[$selection] = 1; + $projection[$this->filter($this->getInternalKeyForAttribute($selection))] = 1; }
2823-2826: Implement or document the quote method properly.The
quotemethod always returns an empty string regardless of input. If quoting is not needed for MongoDB, this should be documented or throw an exception to prevent misuse.Apply this diff to clarify the method's purpose:
protected function quote(string $string): string { - return ""; + // MongoDB doesn't require SQL-style quoting + throw new DatabaseException('Quote method is not applicable for MongoDB adapter'); }
1839-1851: BSON type mappings are correct. The aliases ‘string’, ‘int’, ‘double’, ‘bool’, and ‘date’ align with MongoDB’s official BSON string identifiers (2, 16, 1, 8, 9 respectively).
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Database/Adapter/Mongo.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-30T19:17:53.630Z
Learnt from: ArnabChatterjee20k
PR: utopia-php/database#642
File: src/Database/Validator/PartialStructure.php:43-52
Timestamp: 2025-07-30T19:17:53.630Z
Learning: In PartialStructure validator, when filtering for required attributes validation using the $requiredAttributes parameter, $this->attributes should be used instead of the merged $attributes array because this validation is specifically for internal attributes like $createdAt and $updatedAt that are defined in the base Structure class, not collection-specific attributes.
Applied to files:
src/Database/Adapter/Mongo.php
🧬 Code graph analysis (1)
src/Database/Adapter/Mongo.php (10)
src/Database/Database.php (15)
Database(37-7421)setTimeout(872-877)clearTimeout(885-888)withTransaction(1214-1217)ping(1224-1227)reconnect(1229-1232)create(1243-1262)delete(1301-1315)createCollection(1330-1468)updateRelationship(2865-3052)deleteRelationship(3066-3180)find(6406-6535)upsertDocuments(5275-5290)count(6632-6668)sum(6683-6712)src/Database/Adapter.php (27)
Adapter(16-1379)setTimeout(309-309)getSupportForTimeouts(985-985)clearTimeout(322-326)withTransaction(377-424)startTransaction(336-336)commitTransaction(348-348)rollbackTransaction(359-359)ping(474-474)reconnect(479-479)create(488-488)delete(515-515)createCollection(525-525)filter(1218-1227)updateRelationship(631-631)deleteRelationship(645-645)find(802-802)upsertDocuments(752-756)getSequences(763-763)count(825-825)sum(814-814)escapeWildcards(1229-1254)getSupportForAttributes(914-914)getSupportForRelationships(992-992)decodePoint(1323-1323)decodeLinestring(1331-1331)decodePolygon(1339-1339)src/Database/Exception.php (1)
Exception(7-21)src/Database/Change.php (1)
Change(5-32)src/Database/DateTime.php (2)
DateTime(7-86)format(29-32)src/Database/Document.php (6)
Document(12-470)getAttribute(224-231)getSequence(71-80)find(304-322)removeAttribute(287-293)setAttribute(244-261)src/Database/Query.php (6)
Query(8-1091)getAttribute(158-161)setAttribute(199-204)toArray(389-412)equal(434-437)limit(622-625)src/Database/Validator/Authorization.php (1)
Authorization(7-225)src/Database/Adapter/Pool.php (22)
setTimeout(98-101)getSupportForTimeouts(373-376)startTransaction(103-106)commitTransaction(108-111)rollbackTransaction(113-116)ping(123-126)reconnect(128-131)create(133-136)delete(148-151)createCollection(153-156)updateRelationship(198-201)deleteRelationship(203-206)find(263-266)upsertDocuments(248-251)getSequences(513-516)count(273-276)sum(268-271)getSupportForAttributes(323-326)getSupportForRelationships(378-381)decodePoint(558-561)decodeLinestring(563-566)decodePolygon(568-571)src/Database/Adapter/SQL.php (16)
startTransaction(59-82)commitTransaction(87-113)rollbackTransaction(118-137)ping(146-151)reconnect(153-156)find(2416-2595)upsertDocuments(2172-2329)getSequences(775-826)count(2607-2669)sum(2682-2745)escapeWildcards(1994-2003)getSupportForAttributes(896-899)getSupportForRelationships(1470-1473)decodePoint(2757-2799)decodeLinestring(2801-2841)decodePolygon(2843-2946)
🪛 PHPMD (2.15.0)
src/Database/Adapter/Mongo.php
77-77: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
175-175: Avoid unused local variables such as '$result'. (undefined)
(UnusedLocalVariable)
208-208: Avoid unused local variables such as '$result'. (undefined)
(UnusedLocalVariable)
274-274: Avoid unused parameters such as '$name'. (undefined)
(UnusedFormalParameter)
289-289: Avoid unused parameters such as '$database'. (undefined)
(UnusedFormalParameter)
576-576: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
592-592: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
592-592: Avoid unused parameters such as '$id'. (undefined)
(UnusedFormalParameter)
592-592: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
592-592: Avoid unused parameters such as '$size'. (undefined)
(UnusedFormalParameter)
592-592: Avoid unused parameters such as '$signed'. (undefined)
(UnusedFormalParameter)
592-592: Avoid unused parameters such as '$array'. (undefined)
(UnusedFormalParameter)
592-592: Avoid unused parameters such as '$required'. (undefined)
(UnusedFormalParameter)
605-605: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
605-605: Avoid unused parameters such as '$attributes'. (undefined)
(UnusedFormalParameter)
663-663: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
663-663: Avoid unused parameters such as '$relatedCollection'. (undefined)
(UnusedFormalParameter)
663-663: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
663-663: Avoid unused parameters such as '$twoWay'. (undefined)
(UnusedFormalParameter)
663-663: Avoid unused parameters such as '$id'. (undefined)
(UnusedFormalParameter)
663-663: Avoid unused parameters such as '$twoWayKey'. (undefined)
(UnusedFormalParameter)
689-689: Avoid unused parameters such as '$side'. (undefined)
(UnusedFormalParameter)
818-818: Avoid unused parameters such as '$lengths'. (undefined)
(UnusedFormalParameter)
971-971: Avoid unused parameters such as '$forUpdate'. (undefined)
(UnusedFormalParameter)
1255-1255: Avoid unused parameters such as '$skipPermissions'. (undefined)
(UnusedFormalParameter)
1581-1581: Avoid unused parameters such as '$permissionIds'. (undefined)
(UnusedFormalParameter)
1625-1625: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
1625-1625: Avoid unused parameters such as '$size'. (undefined)
(UnusedFormalParameter)
1625-1625: Avoid unused parameters such as '$signed'. (undefined)
(UnusedFormalParameter)
1625-1625: Avoid unused parameters such as '$array'. (undefined)
(UnusedFormalParameter)
1625-1625: Avoid unused parameters such as '$required'. (undefined)
(UnusedFormalParameter)
2310-2310: Avoid unused parameters such as '$prefix'. (undefined)
(UnusedFormalParameter)
2642-2642: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
2829-2829: Avoid unused parameters such as '$string'. (undefined)
(UnusedFormalParameter)
2838-2838: Avoid unused parameters such as '$stmt'. (undefined)
(UnusedFormalParameter)
2869-2869: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
2908-2908: Avoid unused parameters such as '$wkb'. (undefined)
(UnusedFormalParameter)
2919-2919: Avoid unused parameters such as '$wkb'. (undefined)
(UnusedFormalParameter)
2930-2930: Avoid unused parameters such as '$wkb'. (undefined)
(UnusedFormalParameter)
2942-2942: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
2942-2942: Avoid unused parameters such as '$alias'. (undefined)
(UnusedFormalParameter)
🪛 GitHub Actions: Linter
src/Database/Adapter/Mongo.php
[error] 1-1: PSR-12: statement_indentation violation.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Adapter Tests (SharedTables/MariaDB)
- GitHub Check: Adapter Tests (SharedTables/SQLite)
- GitHub Check: Adapter Tests (SharedTables/MySQL)
- GitHub Check: Adapter Tests (Postgres)
- GitHub Check: Adapter Tests (Mirror)
- GitHub Check: Adapter Tests (SQLite)
- GitHub Check: Adapter Tests (SharedTables/MongoDB)
- GitHub Check: Adapter Tests (SharedTables/Postgres)
- GitHub Check: Adapter Tests (MySQL)
- GitHub Check: Adapter Tests (MariaDB)
- GitHub Check: Adapter Tests (Pool)
- GitHub Check: Adapter Tests (MongoDB)
- GitHub Check: Unit Test
🔇 Additional comments (20)
src/Database/Adapter/Mongo.php (20)
2511-2514: Advertise batch operations supportBatch create/update are implemented; return true.
[uggest_recommended_refactor]public function getSupportForBatchOperations(): bool { - return false; + return true; }
2241-2265: Error message lists invalid operator (TYPE_SELECT)Remove
TYPE_SELECTand include the logical operators for clarity.- default => throw new DatabaseException('Unknown operator:' . $operator . '. Must be one of ' . Query::TYPE_EQUAL . ', ' . Query::TYPE_NOT_EQUAL . ', ' . Query::TYPE_LESSER . ', ' . Query::TYPE_LESSER_EQUAL . ', ' . Query::TYPE_GREATER . ', ' . Query::TYPE_GREATER_EQUAL . ', ' . Query::TYPE_IS_NULL . ', ' . Query::TYPE_IS_NOT_NULL . ', ' . Query::TYPE_BETWEEN . ', ' . Query::TYPE_NOT_BETWEEN . ', ' . Query::TYPE_STARTS_WITH . ', ' . Query::TYPE_NOT_STARTS_WITH . ', ' . Query::TYPE_ENDS_WITH . ', ' . Query::TYPE_NOT_ENDS_WITH . ', ' . Query::TYPE_CONTAINS . ', ' . Query::TYPE_NOT_CONTAINS . ', ' . Query::TYPE_SEARCH . ', ' . Query::TYPE_NOT_SEARCH . ', ' . Query::TYPE_SELECT), + default => throw new DatabaseException('Unknown operator:' . $operator . '. Must be one of ' . Query::TYPE_EQUAL . ', ' . Query::TYPE_NOT_EQUAL . ', ' . Query::TYPE_LESSER . ', ' . Query::TYPE_LESSER_EQUAL . ', ' . Query::TYPE_GREATER . ', ' . Query::TYPE_GREATER_EQUAL . ', ' . Query::TYPE_IS_NULL . ', ' . Query::TYPE_IS_NOT_NULL . ', ' . Query::TYPE_BETWEEN . ', ' . Query::TYPE_NOT_BETWEEN . ', ' . Query::TYPE_STARTS_WITH . ', ' . Query::TYPE_NOT_STARTS_WITH . ', ' . Query::TYPE_ENDS_WITH . ', ' . Query::TYPE_NOT_ENDS_WITH . ', ' . Query::TYPE_CONTAINS . ', ' . Query::TYPE_NOT_CONTAINS . ', ' . Query::TYPE_SEARCH . ', ' . Query::TYPE_NOT_SEARCH . ', ' . Query::TYPE_OR . ', ' . Query::TYPE_AND),
2829-2832: quote(): returning empty string is misleadingIf quoting is not applicable for Mongo, throw to make misuse obvious (also fixes unused-param warning).
protected function quote(string $string): string { - return ""; + throw new DatabaseException('Quote method is not applicable for MongoDB adapter'); }
53-54: Define $timeout to avoid undefined-property errors
setTimeout(),clearTimeout(), and read sites assume$this->timeoutexists. Uncomment and initialize it.- //protected ?int $timeout = null; + protected ?int $timeout = null;
431-447: Index order bug: use attribute loop index, not outer index
$orders[$i]pulls the outer index; should use the inner attribute index.- foreach ($attributes as $attribute) { + foreach ($attributes as $j => $attribute) { $attribute = $this->filter($this->getInternalKeyForAttribute($attribute)); switch ($index->getAttribute('type')) { case Database::INDEX_KEY: - $order = $this->getOrder($this->filter($orders[$i] ?? Database::ORDER_ASC)); + $order = $this->getOrder($this->filter($orders[$j] ?? Database::ORDER_ASC)); break; case Database::INDEX_FULLTEXT: // MongoDB fulltext index is just 'text' // Not using Database::INDEX_KEY for clarity $order = 'text'; break; case Database::INDEX_UNIQUE: - $order = $this->getOrder($this->filter($orders[$i] ?? Database::ORDER_ASC)); + $order = $this->getOrder($this->filter($orders[$j] ?? Database::ORDER_ASC)); $unique = true; break;
466-481: Partial filter must use stored/internal keysCurrently uses raw attribute ids; use filtered internal keys to match stored fields.
- foreach ($attributes as $attr) { + foreach ($attributes as $attr) { + $attrKey = $this->filter($this->getInternalKeyForAttribute($attr)); // Find the matching attribute in collectionAttributes to get its type $attrType = 'string'; // Default fallback foreach ($collectionAttributes as $collectionAttr) { if ($collectionAttr->getId() === $attr) { $attrType = $this->getMongoTypeCode($collectionAttr->getAttribute('type')); break; } } // Use both $exists: true and $type to exclude nulls and ensure correct type - $partialFilter[$attr] = ['$exists' => true, '$type' => $attrType]; + $partialFilter[$attrKey] = ['$exists' => true, '$type' => $attrType]; }
618-628: deleteAttribute(): filter the attribute key before $unsetRaw keys can be invalid; also map external -> internal field.
$this->getClient()->update( $collection, [], - ['$unset' => [$id => '']], + ['$unset' => [$this->filter($this->getInternalKeyForAttribute($id)) => '']], multi: true );
644-649: renameAttribute(): filter both source and destination keysAvoid writing unfiltered field names.
$this->getClient()->update( $collection, [], - ['$rename' => [$id => $name]], + ['$rename' => [ + $this->filter($this->getInternalKeyForAttribute($id)) => + $this->filter($this->getInternalKeyForAttribute($name)) + ]], multi: true );
773-799: deleteRelationship(): filter attribute keys in $unsetUse internal filtered keys for all branches.
- $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($key)) => '']], multi: true); if ($twoWay) { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($twoWayKey)) => '']], multi: true); } @@ if ($side === Database::RELATION_SIDE_PARENT) { - $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($key)) => '']], multi: true); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($twoWayKey)) => '']], multi: true); } @@ if ($side === Database::RELATION_SIDE_CHILD) { - $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($key)) => '']], multi: true); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($twoWayKey)) => '']], multi: true); }
831-851: createIndex(): fix attribute typing map and use internal keys in partial filter
$indexAttributeTypesshould be keyed by attribute name, not numeric.- Use original attribute names for type lookup and internal keys for filter/index.
- foreach ($attributes as $i => $attribute) { + $origAttributes = $attributes; + foreach ($attributes as $i => $attribute) { - - $attributes[$i] = $this->filter($this->getInternalKeyForAttribute($attribute)); + $attributes[$i] = $this->filter($this->getInternalKeyForAttribute($attribute)); $orderType = $this->getOrder($this->filter($orders[$i] ?? Database::ORDER_ASC)); $indexes['key'][$attributes[$i]] = $orderType; @@ - if (in_array($type, [Database::INDEX_UNIQUE, Database::INDEX_KEY])) { + if (in_array($type, [Database::INDEX_UNIQUE, Database::INDEX_KEY], true)) { $partialFilter = []; - foreach ($attributes as $i => $attr) { - $attrType = $indexAttributeTypes[$i] ?? Database::VAR_STRING; // Default to string if type not provided - $attrType = $this->getMongoTypeCode($attrType); - $partialFilter[$attr] = ['$exists' => true, '$type' => $attrType]; + foreach ($origAttributes as $i => $attrName) { + $mongoKey = $this->filter($this->getInternalKeyForAttribute($attrName)); + $attrType = $indexAttributeTypes[$attrName] ?? Database::VAR_STRING; + $partialFilter[$mongoKey] = [ + '$exists' => true, + '$type' => $this->getMongoTypeCode($attrType) + ]; }Also applies to: 867-878
896-942: renameIndex(): wrong lookup; add rollback on failureCompare by index id/name (not key array) and restore old index if create fails.
- foreach ($indexes as $node) { - if ($node['key'] === $old) { + foreach ($indexes as $node) { + if (($node['$id'] ?? $node['name'] ?? null) === $old) { $index = $node; break; } } @@ - try { - $deletedindex = $this->deleteIndex($collection, $old); - $createdindex = $this->createIndex($collection, $new, $index['type'], $index['attributes'], $index['lengths'] ?? [], $index['orders'] ?? [], $indexAttributeTypes); - } catch (\Exception $e) { - throw $this->processException($e); - } - - if ($index && $deletedindex && $createdindex) { - return true; - } - - return false; + if (!$index) { + return false; + } + try { + if (!$this->deleteIndex($collection, $old)) { + return false; + } + return $this->createIndex( + $collection, + $new, + $index['type'], + $index['attributes'], + $index['lengths'] ?? [], + $index['orders'] ?? [], + $indexAttributeTypes + ); + } catch (\Exception $e) { + // best-effort restore + try { + $this->createIndex( + $collection, + $old, + $index['type'], + $index['attributes'], + $index['lengths'] ?? [], + $index['orders'] ?? [], + $indexAttributeTypes + ); + } finally { + throw $this->processException($e); + } + }
1028-1034: Insert with shared tables: tenant filters computed from logical collection idPassing
$name(namespaced) intogetTenantFilters()breaks reads under shared tables. Pass logical id; compute namespaced name inside helper.- $result = $this->insertDocument($name, $this->removeNullKeys($record), $options); + $result = $this->insertDocument($collection->getId(), $this->removeNullKeys($record), $options);- private function insertDocument(string $name, array $document, array $options = []): array + private function insertDocument(string $collection, array $document, array $options = []): array { try { - $result = $this->client->insert($name, $document, $options); + $name = $this->getNamespace() . '_' . $this->filter($collection); + $result = $this->client->insert($name, $document, $options); $filters = []; $filters['_uid'] = $document['_uid']; if ($this->sharedTables) { - $filters['_tenant'] = $this->getTenantFilters($name); + $filters['_tenant'] = $this->getTenantFilters($collection); } try { $result = $this->client->find( $name, $filters, array_merge(['limit' => 1], $options) )->cursor->firstBatch[0];Also applies to: 1217-1242
1269-1274: updateDocument(): wrap updates in $set to avoid replacing documentCurrent call risks full-document replacement.
- unset($record['_id']); // Don't update _id - - $options = $this->getTransactionOptions(); - $this->client->update($name, $filters, $record, $options); + unset($record['_id']); // Don't update _id + $options = $this->getTransactionOptions(); + $this->client->update($name, $filters, ['$set' => $record], $options);
1296-1324: updateDocuments(): stray semicolon; return actual modified countReturn the driver’s modified count instead of a hardcoded
1.- { - ; + { $name = $this->getNamespace() . '_' . $this->filter($collection->getId()); @@ - try { - $this->client->update($name, $filters, $updateQuery, multi: true, options: $options); + try { + $modified = $this->client->update($name, $filters, $updateQuery, multi: true, options: $options); } catch (MongoException $e) { throw $this->processException($e); } - return 1; + return (int) ($modified ?? 0);
1525-1531: increaseDocumentAttribute(): preserve both min and maxSecond assignment overwrites the first; merge constraints.
- if ($max) { - $filters[$attribute] = ['$lte' => $max]; - } - - if ($min) { - $filters[$attribute] = ['$gte' => $min]; - } + if ($max !== null || $min !== null) { + $filters[$attribute] = []; + if ($max !== null) { + $filters[$attribute]['$lte'] = $max; + } + if ($min !== null) { + $filters[$attribute]['$gte'] = $min; + } + }
1600-1607: deleteDocuments(): exceptions are swallowedRe-throw processed exceptions to avoid silent failures.
} catch (MongoException $e) { - $this->processException($e); + throw $this->processException($e); }
1683-1687: Escape roles in permission regex (regex injection)Roles may contain metacharacters; escape them.
- if (Authorization::$status) { - $roles = \implode('|', Authorization::getRoles()); - $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')]; + if (Authorization::$status) { + $roles = \implode('|', \array_map('preg_quote', Authorization::getRoles())); + $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')]; }
1932-1936: Escape roles in count() permission regex tooKeep consistent and safe.
- if (Authorization::$status) { - $roles = \implode('|', Authorization::getRoles()); - $filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')]; + if (Authorization::$status) { + $roles = \implode('|', \array_map('preg_quote', Authorization::getRoles())); + $filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')]; }
2010-2052: sum(): map attribute to internal key and escape rolesUse stored field name in
$groupand escape roles in permissions filter.public function sum(Document $collection, string $attribute, array $queries = [], ?int $max = null): float|int { $name = $this->getNamespace() . '_' . $this->filter($collection->getId()); + $attribute = $this->filter($this->getInternalKeyForAttribute($attribute)); @@ - if (Authorization::$status) { // skip if authorization is disabled - $roles = \implode('|', Authorization::getRoles()); - $filters['_permissions']['$in'] = [new Regex("read\(\".*(?:{$roles}).*\"\)", 'i')]; + if (Authorization::$status) { // skip if authorization is disabled + $roles = \implode('|', \array_map('preg_quote', Authorization::getRoles())); + $filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')]; } @@ - if (!empty($filters)) { - $pipeline[] = ['$match' => $filters]; + if (!empty($filters)) { + $pipeline[] = ['$match' => $this->client->toObject($filters)]; } @@ $pipeline[] = [ '$group' => [ '_id' => null, 'total' => ['$sum' => '$' . $attribute], ], ];
2319-2335: Projection should use stored/internal keysMap external selections to the stored field names.
- foreach ($selections as $selection) { + foreach ($selections as $selection) { // Skip internal attributes since all are selected by default if (\in_array($selection, $internalKeys)) { continue; } - - $projection[$selection] = 1; + $projection[$this->filter($this->getInternalKeyForAttribute($selection))] = 1; }
Update MongoDB client
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (16)
src/Database/Adapter/Mongo.php (16)
615-627: Filter attribute name before $unset operation.The
deleteAttributemethod uses the raw$idparameter in the$unsetoperation (line 622), which could contain special characters that are unsafe for MongoDB field names. The attribute should be filtered using$this->filter($this->getInternalKeyForAttribute($id)).Apply this diff:
public function deleteAttribute(string $collection, string $id): bool { $collection = $this->getNamespace() . '_' . $this->filter($collection); $this->getClient()->update( $collection, [], - ['$unset' => [$id => '']], + ['$unset' => [$this->filter($this->getInternalKeyForAttribute($id)) => '']], multi: true ); return true; }
1671-1686: Escape roles in permission regex to prevent injection.Line 1684 concatenates roles directly into a regex pattern without escaping special characters. If roles contain regex metacharacters, this could cause pattern errors or security issues.
Apply this diff:
// permissions if (Authorization::$status) { - $roles = \implode('|', Authorization::getRoles()); - $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')]; + $roles = \implode('|', \array_map('preg_quote', Authorization::getRoles())); + $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')]; }Apply the same fix in
count()(line 1933) andsum()(line 2023).
637-649: Filter both source and destination attribute names in rename operation.The
renameAttributemethod uses raw$idand$nameparameters (line 644), which should be filtered to ensure they're safe MongoDB field names.Apply this diff:
public function renameAttribute(string $collection, string $id, string $name): bool { $collection = $this->getNamespace() . '_' . $this->filter($collection); $this->getClient()->update( $collection, [], - ['$rename' => [$id => $name]], + ['$rename' => [ + $this->filter($this->getInternalKeyForAttribute($id)) => + $this->filter($this->getInternalKeyForAttribute($name)) + ]], multi: true ); return true; }
2309-2334: Map selections to internal attribute keys in projection.The projection uses raw
$selectionvalues (line 2324) directly from queries, but MongoDB expects the internal/stored field names. Map external attribute identifiers to their stored keys before building the projection.Apply this diff:
foreach ($selections as $selection) { // Skip internal attributes since all are selected by default if (\in_array($selection, $internalKeys)) { continue; } - $projection[$selection] = 1; + $filteredKey = $this->filter($this->getInternalKeyForAttribute($selection)); + $projection[$filteredKey] = 1; }
815-881: Fix $indexAttributeTypes array indexing in createIndex.Line 868 uses
$indexAttributeTypes[$i]where$iis the loop index over$attributes, but$indexAttributeTypesshould be an associative array keyed by attribute names, not numeric indexes. This causes incorrect type lookups.Apply this diff:
foreach ($attributes as $i => $attribute) { $attributes[$i] = $this->filter($this->getInternalKeyForAttribute($attribute)); $orderType = $this->getOrder($this->filter($orders[$i] ?? Database::ORDER_ASC)); $indexes['key'][$attributes[$i]] = $orderType; switch ($type) { case Database::INDEX_KEY: break; case Database::INDEX_FULLTEXT: $indexes['key'][$attributes[$i]] = 'text'; break; case Database::INDEX_UNIQUE: $indexes['unique'] = true; break; default: return false; } } // ... collation section ... // Add partial filter for indexes to avoid indexing null values if (in_array($type, [Database::INDEX_UNIQUE, Database::INDEX_KEY])) { $partialFilter = []; - foreach ($attributes as $i => $attr) { - $attrType = $indexAttributeTypes[$i] ?? Database::VAR_STRING; + foreach ($attributes as $attr) { + // $attr here is already the filtered internal key from line 830 + // We need to look up the original attribute name to find its type + $originalAttr = array_search($attr, $attributes, true); + $attrType = $indexAttributeTypes[$originalAttr] ?? Database::VAR_STRING; $attrType = $this->getMongoTypeCode($attrType); $partialFilter[$attr] = ['$exists' => true, '$type' => $attrType]; }Note: The PHPDoc should also be updated to indicate
$indexAttributeTypesisarray<string, string>(attribute name => type).
1515-1544: Merge min and max constraints to avoid overwriting.Lines 1524-1530 assign
$filters[$attribute]twice, so when both$minand$maxare provided, the second assignment overwrites the first, causing only one bound to be enforced.Apply this diff:
$attribute = $this->filter($attribute); $filters = ['_uid' => $id]; if ($this->sharedTables) { $filters['_tenant'] = $this->getTenantFilters($collection); } - if ($max) { - $filters[$attribute] = ['$lte' => $max]; - } - - if ($min) { - $filters[$attribute] = ['$gte' => $min]; - } + if ($max !== null || $min !== null) { + $filters[$attribute] = []; + if ($max !== null) { + $filters[$attribute]['$lte'] = $max; + } + if ($min !== null) { + $filters[$attribute]['$gte'] = $min; + } + }
1293-1323: Remove stray semicolon and return actual modified count.Line 1295 has a stray semicolon, and line 1322 always returns 1 instead of the actual number of modified documents.
Apply this diff:
public function updateDocuments(Document $collection, Document $updates, array $documents): int { - ; $name = $this->getNamespace() . '_' . $this->filter($collection->getId()); $options = $this->getTransactionOptions(); $queries = [ Query::equal('$sequence', \array_map(fn ($document) => $document->getSequence(), $documents)) ]; $filters = $this->buildFilters($queries); if ($this->sharedTables) { $filters['_tenant'] = $this->getTenantFilters($collection->getId()); } $record = $updates->getArrayCopy(); $record = $this->replaceChars('$', '_', $record); $updateQuery = [ '$set' => $record, ]; try { - $this->client->update($name, $filters, $updateQuery, multi: true, options: $options); + $result = $this->client->update($name, $filters, $updateQuery, multi: true, options: $options); } catch (MongoException $e) { throw $this->processException($e); } - return 1; + return (int)($result ?? 0); }
2009-2051: Filter attribute and escape roles in sum() aggregation.Line 2023 has the same regex injection issue, and line 2045 uses the raw
$attributeparameter in the MongoDB aggregation pipeline without filtering or mapping to the internal key.Apply this diff:
public function sum(Document $collection, string $attribute, array $queries = [], ?int $max = null): float|int { $name = $this->getNamespace() . '_' . $this->filter($collection->getId()); + $attribute = $this->filter($this->getInternalKeyForAttribute($attribute)); // queries $queries = array_map(fn ($query) => clone $query, $queries); $filters = $this->buildFilters($queries); if ($this->sharedTables) { $filters['_tenant'] = $this->getTenantFilters($collection->getId()); } // permissions if (Authorization::$status) { // skip if authorization is disabled - $roles = \implode('|', Authorization::getRoles()); - $filters['_permissions']['$in'] = [new Regex("read\(\".*(?:{$roles}).*\"\)", 'i')]; + $roles = \implode('|', \array_map('preg_quote', Authorization::getRoles())); + $filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')]; }
1580-1610: Re-throw exception after processing in deleteDocuments.The catch block at line 1606 calls
processException()but doesn't re-throw the result, causing silent failures.Apply this diff:
try { $count = $this->client->delete( collection: $name, filters: $filters, limit: 0, options: $options ); } catch (MongoException $e) { - $this->processException($e); + throw $this->processException($e); }
276-294: Fix broken database and collection existence checks.The
exists()method has critical logic errors:
Collection check (lines 278-290): Uses
flattenArray($this->listCollections())[0]->firstBatch, butlistCollections()returns an array of strings (line 507), not a cursor withfirstBatch.Database check (line 293): Calls
selectDatabase()without a database name parameter and compares tonull, which is incorrect.Apply this fix:
public function exists(string $database, ?string $collection = null): bool { if (!\is_null($collection)) { - $collection = $this->getNamespace() . "_" . $collection; - $list = $this->flattenArray($this->listCollections())[0]->firstBatch; - foreach ($list as $obj) { - if (\is_object($obj) - && isset($obj->name) - && $obj->name === $collection - ) { - return true; - } - } - - return false; + $qualified = $this->getNamespace() . '_' . $this->filter($collection); + $collections = $this->listCollections(); + return \in_array($qualified, $collections, true); } - return $this->getClient()->selectDatabase() != null; + $databases = $this->list(); + return \in_array($database, $databases, true); }
757-799: Filter relationship attribute keys in $unset operations.The
deleteRelationshipmethod uses raw$keyand$twoWayKeyvariables in multiple$unsetoperations (lines 772, 774, 779, 781, 786, 788), which should be filtered for safety.Apply this diff to filter all attribute keys:
case Database::RELATION_ONE_TO_ONE: - $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($key)) => '']], multi: true); if ($twoWay) { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($twoWayKey)) => '']], multi: true); } break; case Database::RELATION_ONE_TO_MANY: if ($side === Database::RELATION_SIDE_PARENT) { - $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($key)) => '']], multi: true); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($twoWayKey)) => '']], multi: true); } break; case Database::RELATION_MANY_TO_ONE: if ($side === Database::RELATION_SIDE_CHILD) { - $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($key)) => '']], multi: true); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$this->filter($this->getInternalKeyForAttribute($twoWayKey)) => '']], multi: true); } break;
1907-1994: Escape roles in count() permission regex.Line 1933 has the same regex injection vulnerability as
find(). Apply the same fix usingpreg_quote.Apply this diff:
// Add permissions filter if authorization is enabled if (Authorization::$status) { - $roles = \implode('|', Authorization::getRoles()); - $filters['_permissions']['$in'] = [new Regex("read\(\".*(?:{$roles}).*\"\)", 'i')]; + $roles = \implode('|', \array_map('preg_quote', Authorization::getRoles())); + $filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')]; }
1254-1278: Wrap update record in $set operator.Line 1272 passes the raw
$recordto the update operation, which replaces the entire document. MongoDB updates should use the$setoperator to update only specified fields.Apply this diff:
try { unset($record['_id']); // Don't update _id $options = $this->getTransactionOptions(); - $this->client->update($name, $filters, $record, $options); + $this->client->update($name, $filters, ['$set' => $record], $options); } catch (MongoException $e) { throw $this->processException($e); }
413-447: Fix incorrect order array indexing in createCollection.Lines 430 and 438 use
$orders[$i]where$iis the outer loop index over$indexes, but$orderscorresponds to the inner loop over$attributes. This causes incorrect order assignments or array access errors.Apply this diff:
- foreach ($indexes as $i => $index) { + foreach ($indexes as $indexIdx => $index) { $key = []; $unique = false; $attributes = $index->getAttribute('attributes'); $orders = $index->getAttribute('orders'); // If sharedTables, always add _tenant as the first key if ($this->sharedTables) { $key['_tenant'] = $this->getOrder(Database::ORDER_ASC); } - foreach ($attributes as $attribute) { + foreach ($attributes as $attrIdx => $attribute) { $attribute = $this->filter($this->getInternalKeyForAttribute($attribute)); switch ($index->getAttribute('type')) { case Database::INDEX_KEY: - $order = $this->getOrder($this->filter($orders[$i] ?? Database::ORDER_ASC)); + $order = $this->getOrder($this->filter($orders[$attrIdx] ?? Database::ORDER_ASC)); break; case Database::INDEX_FULLTEXT: // MongoDB fulltext index is just 'text' // Not using Database::INDEX_KEY for clarity $order = 'text'; break; case Database::INDEX_UNIQUE: - $order = $this->getOrder($this->filter($orders[$i] ?? Database::ORDER_ASC)); + $order = $this->getOrder($this->filter($orders[$attrIdx] ?? Database::ORDER_ASC)); $unique = true; break; default: // index not supported return false; } $key[$attribute] = $order; } - $newIndexes[$i] = [ + $newIndexes[$indexIdx] = [ 'key' => $key, 'name' => $this->filter($index->getId()), 'unique' => $unique ];
679-743: Fix many-to-many metadata lookup using namespaced names.Lines 690-691 overwrite the
$collectionand$relatedCollectionvariables with namespaced names before the many-to-many case (lines 724-737) attempts to use them for metadata lookups. This causesgetDocument()to fail finding the metadata.Apply this diff:
public function updateRelationship( string $collection, string $relatedCollection, string $type, bool $twoWay, string $key, string $twoWayKey, string $side, ?string $newKey = null, ?string $newTwoWayKey = null ): bool { - $collection = $this->getNamespace() . '_' . $this->filter($collection); - $relatedCollection = $this->getNamespace() . '_' . $this->filter($relatedCollection); + $collectionId = $collection; + $relatedCollectionId = $relatedCollection; + $namespacedCollection = $this->getNamespace() . '_' . $this->filter($collection); + $namespacedRelatedCollection = $this->getNamespace() . '_' . $this->filter($relatedCollection); $renameKey = [ '$rename' => [ $key => $newKey, ] ]; $renameTwoWayKey = [ '$rename' => [ $twoWayKey => $newTwoWayKey, ] ]; switch ($type) { case Database::RELATION_ONE_TO_ONE: if (!\is_null($newKey)) { - $this->getClient()->update($collection, updates: $renameKey, multi: true); + $this->getClient()->update($namespacedCollection, updates: $renameKey, multi: true); } if ($twoWay && !\is_null($newTwoWayKey)) { - $this->getClient()->update($relatedCollection, updates: $renameTwoWayKey, multi: true); + $this->getClient()->update($namespacedRelatedCollection, updates: $renameTwoWayKey, multi: true); } break; case Database::RELATION_ONE_TO_MANY: if ($twoWay && !\is_null($newTwoWayKey)) { - $this->getClient()->update($relatedCollection, updates: $renameTwoWayKey, multi: true); + $this->getClient()->update($namespacedRelatedCollection, updates: $renameTwoWayKey, multi: true); } break; case Database::RELATION_MANY_TO_ONE: if (!\is_null($newKey)) { - $this->getClient()->update($collection, updates: $renameKey, multi: true); + $this->getClient()->update($namespacedCollection, updates: $renameKey, multi: true); } break; case Database::RELATION_MANY_TO_MANY: $metadataCollection = new Document(['$id' => Database::METADATA]); - $collection = $this->getDocument($metadataCollection, $collection); - $relatedCollection = $this->getDocument($metadataCollection, $relatedCollection); + $collectionDoc = $this->getDocument($metadataCollection, $collectionId); + $relatedCollectionDoc = $this->getDocument($metadataCollection, $relatedCollectionId); - $junction = $this->getNamespace() . '_' . $this->filter('_' . $collection->getSequence() . '_' . $relatedCollection->getSequence()); + $junction = $this->getNamespace() . '_' . $this->filter('_' . $collectionDoc->getSequence() . '_' . $relatedCollectionDoc->getSequence()); if (!\is_null($newKey)) { $this->getClient()->update($junction, updates: $renameKey, multi: true); } if ($twoWay && !\is_null($newTwoWayKey)) { $this->getClient()->update($junction, updates: $renameTwoWayKey, multi: true); } break; default: throw new DatabaseException('Invalid relationship type'); } return true; }
1010-1038: Fix tenant filter scope in createDocument.Line 1030 calls
insertDocument($name, ...)where$nameis the fully namespaced collection name. However,insertDocumentat line 1224 passes this namespaced name togetTenantFilters($name), which expects the logical collection ID for proper shared tables handling.Apply this diff:
public function createDocument(Document $collection, Document $document): Document { $name = $this->getNamespace() . '_' . $this->filter($collection->getId()); $sequence = $document->getSequence(); $document->removeAttribute('$sequence'); if ($this->sharedTables) { $document->setAttribute('$tenant', $this->getTenant()); } $record = $this->replaceChars('$', '_', (array)$document); // Insert manual id if set if (!empty($sequence)) { $record['_id'] = $sequence; } $options = $this->getTransactionOptions(); - $result = $this->insertDocument($name, $this->removeNullKeys($record), $options); + $result = $this->insertDocument($collection->getId(), $this->removeNullKeys($record), $options);Also update the
insertDocumentsignature and implementation:-private function insertDocument(string $name, array $document, array $options = []): array +private function insertDocument(string $collection, array $document, array $options = []): array { + $name = $this->getNamespace() . '_' . $this->filter($collection); try { $result = $this->client->insert($name, $document, $options); $filters = []; $filters['_uid'] = $document['_uid']; if ($this->sharedTables) { - $filters['_tenant'] = $this->getTenantFilters($name); + $filters['_tenant'] = $this->getTenantFilters($collection); }
🧹 Nitpick comments (2)
src/Database/Adapter/Mongo.php (2)
1179-1183: Use strict comparison for type safety.Line 1181 uses loose comparison (
==) which can lead to unexpected type coercion. Use strict comparison (===) instead.Apply this diff:
if ($hasSequence === null) { $hasSequence = !empty($sequence); - } elseif ($hasSequence == empty($sequence)) { + } elseif ($hasSequence === empty($sequence)) { throw new DatabaseException('All documents must have an sequence if one is set'); }
2510-2513: Update batch operations support flag.The adapter implements
createDocuments(),updateDocuments(), andupsertDocuments()methods, sogetSupportForBatchOperations()should returntrue.Apply this diff:
public function getSupportForBatchOperations(): bool { - return false; + return true; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
composer.json(2 hunks)src/Database/Adapter/Mongo.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-30T19:17:53.630Z
Learnt from: ArnabChatterjee20k
PR: utopia-php/database#642
File: src/Database/Validator/PartialStructure.php:43-52
Timestamp: 2025-07-30T19:17:53.630Z
Learning: In PartialStructure validator, when filtering for required attributes validation using the $requiredAttributes parameter, $this->attributes should be used instead of the merged $attributes array because this validation is specifically for internal attributes like $createdAt and $updatedAt that are defined in the base Structure class, not collection-specific attributes.
Applied to files:
src/Database/Adapter/Mongo.php
🧬 Code graph analysis (1)
src/Database/Adapter/Mongo.php (13)
src/Database/Database.php (38)
Database(37-7421)setTimeout(872-877)clearTimeout(885-888)withTransaction(1214-1217)ping(1224-1227)reconnect(1229-1232)create(1243-1262)exists(1273-1278)list(1285-1292)delete(1301-1315)createCollection(1330-1468)getSizeOfCollectionOnDisk(1591-1608)getSizeOfCollection(1569-1582)deleteCollection(1629-1672)analyzeCollection(1616-1619)createAttribute(1698-1753)createAttributes(1769-1861)deleteAttribute(2506-2574)renameAttribute(2589-2653)createRelationship(2673-2850)updateRelationship(2865-3052)getDocument(3467-3590)deleteRelationship(3066-3180)createIndex(3255-3415)renameIndex(3196-3234)deleteIndex(3429-3454)find(6406-6535)createDocument(3862-3958)createDocuments(3974-4077)updateDocument(4414-4621)updateDocuments(4644-4838)upsertDocuments(5275-5290)increaseDocumentAttribute(5546-5627)deleteDocument(5741-5793)deleteDocuments(6192-6345)count(6632-6668)updateAttribute(2250-2460)sum(6683-6712)src/Database/Adapter.php (45)
Adapter(16-1379)setTimeout(309-309)clearTimeout(322-326)withTransaction(377-424)startTransaction(336-336)commitTransaction(348-348)rollbackTransaction(359-359)ping(474-474)reconnect(479-479)create(488-488)exists(499-499)list(506-506)delete(515-515)createCollection(525-525)filter(1218-1227)getSizeOfCollectionOnDisk(843-843)getSizeOfCollection(834-834)deleteCollection(534-534)analyzeCollection(542-542)createAttribute(557-557)createAttributes(568-568)deleteAttribute(594-594)renameAttribute(604-604)createRelationship(615-615)updateRelationship(631-631)getDocument(691-691)deleteRelationship(645-645)createIndex(670-670)renameIndex(655-655)deleteIndex(680-680)find(802-802)createDocument(701-701)createDocuments(713-713)updateDocument(725-725)updateDocuments(740-740)upsertDocuments(752-756)getSequences(763-763)increaseDocumentAttribute(1269-1277)deleteDocument(773-773)deleteDocuments(784-784)count(825-825)updateAttribute(584-584)sum(814-814)getLimitForString(850-850)getLimitForInt(857-857)src/Database/Exception.php (1)
Exception(7-21)src/Database/Change.php (1)
Change(5-32)src/Database/DateTime.php (2)
DateTime(7-86)format(29-32)src/Database/Document.php (8)
Document(12-470)getAttribute(224-231)getId(63-66)getSequence(71-80)find(304-322)removeAttribute(287-293)setAttribute(244-261)getPermissions(93-96)src/Database/Query.php (5)
Query(8-1091)getAttribute(158-161)setAttribute(199-204)equal(434-437)limit(622-625)src/Database/Validator/Authorization.php (1)
Authorization(7-225)src/Database/Adapter/SQL.php (23)
startTransaction(59-82)commitTransaction(87-113)rollbackTransaction(118-137)processException(2005-2008)ping(146-151)reconnect(153-156)exists(167-209)list(216-219)createAttribute(234-248)createAttributes(258-285)deleteAttribute(326-339)renameAttribute(297-314)getDocument(351-417)find(2416-2595)createDocuments(2030-2163)updateDocuments(453-693)upsertDocuments(2172-2329)getSequences(775-826)deleteDocuments(706-765)count(2607-2669)sum(2682-2745)getLimitForString(833-836)getLimitForInt(843-846)src/Database/Adapter/MariaDB.php (20)
setTimeout(1745-1761)processException(1777-1833)create(28-43)delete(53-64)createCollection(76-225)getSizeOfCollectionOnDisk(234-266)getSizeOfCollection(275-310)deleteCollection(320-335)analyzeCollection(344-352)createRelationship(450-491)updateRelationship(506-591)deleteRelationship(604-676)createIndex(715-778)renameIndex(687-700)deleteIndex(789-809)createDocument(822-933)updateDocument(948-1170)increaseDocumentAttribute(1249-1293)deleteDocument(1304-1354)updateAttribute(417-438)src/Database/Adapter/Postgres.php (25)
setTimeout(115-125)startTransaction(33-59)rollbackTransaction(64-82)processException(1910-1943)create(135-162)delete(172-180)createCollection(191-333)getSizeOfCollectionOnDisk(341-367)getSizeOfCollection(376-402)deleteCollection(410-418)analyzeCollection(426-429)createAttribute(444-463)deleteAttribute(475-497)renameAttribute(509-524)createRelationship(599-639)updateRelationship(654-738)deleteRelationship(751-824)createIndex(839-899)renameIndex(934-948)deleteIndex(909-922)createDocument(958-1058)updateDocument(1072-1272)increaseDocumentAttribute(1351-1383)deleteDocument(1393-1443)updateAttribute(541-587)src/Database/Adapter/MySQL.php (3)
setTimeout(22-41)processException(147-164)getSizeOfCollectionOnDisk(49-81)src/Database/Adapter/SQLite.php (17)
startTransaction(37-62)processException(1244-1257)create(113-116)exists(73-103)delete(126-129)createCollection(141-227)getSizeOfCollectionOnDisk(276-279)getSizeOfCollection(237-268)deleteCollection(288-307)analyzeCollection(315-318)deleteAttribute(354-392)createIndex(455-480)renameIndex(404-439)deleteIndex(491-510)createDocument(522-630)updateDocument(644-852)updateAttribute(335-342)
🪛 PHPMD (2.15.0)
src/Database/Adapter/Mongo.php
74-74: Avoid unused parameters such as '$event'. (undefined)
(UnusedFormalParameter)
161-161: Avoid unused local variables such as '$result'. (undefined)
(UnusedLocalVariable)
203-203: Avoid unused local variables such as '$result'. (undefined)
(UnusedLocalVariable)
261-261: Avoid unused parameters such as '$name'. (undefined)
(UnusedFormalParameter)
276-276: Avoid unused parameters such as '$database'. (undefined)
(UnusedFormalParameter)
573-573: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
589-589: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
589-589: Avoid unused parameters such as '$id'. (undefined)
(UnusedFormalParameter)
589-589: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
589-589: Avoid unused parameters such as '$size'. (undefined)
(UnusedFormalParameter)
589-589: Avoid unused parameters such as '$signed'. (undefined)
(UnusedFormalParameter)
589-589: Avoid unused parameters such as '$array'. (undefined)
(UnusedFormalParameter)
589-589: Avoid unused parameters such as '$required'. (undefined)
(UnusedFormalParameter)
602-602: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
602-602: Avoid unused parameters such as '$attributes'. (undefined)
(UnusedFormalParameter)
660-660: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
660-660: Avoid unused parameters such as '$relatedCollection'. (undefined)
(UnusedFormalParameter)
660-660: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
660-660: Avoid unused parameters such as '$twoWay'. (undefined)
(UnusedFormalParameter)
660-660: Avoid unused parameters such as '$id'. (undefined)
(UnusedFormalParameter)
660-660: Avoid unused parameters such as '$twoWayKey'. (undefined)
(UnusedFormalParameter)
686-686: Avoid unused parameters such as '$side'. (undefined)
(UnusedFormalParameter)
815-815: Avoid unused parameters such as '$lengths'. (undefined)
(UnusedFormalParameter)
968-968: Avoid unused parameters such as '$forUpdate'. (undefined)
(UnusedFormalParameter)
1254-1254: Avoid unused parameters such as '$skipPermissions'. (undefined)
(UnusedFormalParameter)
1580-1580: Avoid unused parameters such as '$permissionIds'. (undefined)
(UnusedFormalParameter)
1624-1624: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
1624-1624: Avoid unused parameters such as '$size'. (undefined)
(UnusedFormalParameter)
1624-1624: Avoid unused parameters such as '$signed'. (undefined)
(UnusedFormalParameter)
1624-1624: Avoid unused parameters such as '$array'. (undefined)
(UnusedFormalParameter)
1624-1624: Avoid unused parameters such as '$required'. (undefined)
(UnusedFormalParameter)
2309-2309: Avoid unused parameters such as '$prefix'. (undefined)
(UnusedFormalParameter)
2641-2641: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
2832-2832: Avoid unused parameters such as '$string'. (undefined)
(UnusedFormalParameter)
2841-2841: Avoid unused parameters such as '$stmt'. (undefined)
(UnusedFormalParameter)
2872-2872: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
2911-2911: Avoid unused parameters such as '$wkb'. (undefined)
(UnusedFormalParameter)
2922-2922: Avoid unused parameters such as '$wkb'. (undefined)
(UnusedFormalParameter)
2933-2933: Avoid unused parameters such as '$wkb'. (undefined)
(UnusedFormalParameter)
2945-2945: Avoid unused parameters such as '$collection'. (undefined)
(UnusedFormalParameter)
2945-2945: Avoid unused parameters such as '$alias'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Adapter Tests (SharedTables/Postgres)
- GitHub Check: Adapter Tests (SharedTables/SQLite)
- GitHub Check: Adapter Tests (SharedTables/MySQL)
- GitHub Check: Adapter Tests (SharedTables/MariaDB)
- GitHub Check: Adapter Tests (SQLite)
- GitHub Check: Adapter Tests (Mirror)
- GitHub Check: Adapter Tests (MongoDB)
- GitHub Check: Adapter Tests (Postgres)
- GitHub Check: Adapter Tests (SharedTables/MongoDB)
- GitHub Check: Adapter Tests (MySQL)
- GitHub Check: Adapter Tests (Pool)
- GitHub Check: Adapter Tests (MariaDB)
- GitHub Check: Unit Test
🔇 Additional comments (4)
composer.json (2)
35-44: LGTM: MongoDB dependencies properly added.The new dependencies
ext-mongodbandutopia-php/mongoare correctly specified to enable MongoDB adapter functionality. Version constraints are appropriate.
55-59: LGTM: Helpful suggestion added for MongoDB support.The suggestion to install
mongodb/mongodbpackage appropriately guides users who need MongoDB adapter support.src/Database/Adapter/Mongo.php (2)
96-217: LGTM: Transaction support properly implemented.The transaction methods now include proper session management and replica set detection. The implementation correctly:
- Checks for replica set support before attempting transactions
- Manages nested transactions with a counter
- Handles session lifecycle
- Includes appropriate error handling and rollback logic
46-72: No action required:$timeoutproperty is inherited from the parentAdapterclass.
| // Add partial filter for indexes to avoid indexing null values | ||
| if (in_array($index->getAttribute('type'), [ | ||
| Database::INDEX_UNIQUE, | ||
| Database::INDEX_KEY | ||
| ])) { | ||
| $partialFilter = []; | ||
| foreach ($attributes as $attr) { | ||
| // Find the matching attribute in collectionAttributes to get its type | ||
| $attrType = 'string'; // Default fallback | ||
| foreach ($collectionAttributes as $collectionAttr) { | ||
| if ($collectionAttr->getId() === $attr) { | ||
| $attrType = $this->getMongoTypeCode($collectionAttr->getAttribute('type')); | ||
| break; | ||
| } | ||
| } | ||
| // Use both $exists: true and $type to exclude nulls and ensure correct type | ||
| $partialFilter[$attr] = ['$exists' => true, '$type' => $attrType]; | ||
| } | ||
| if (!empty($partialFilter)) { | ||
| $newIndexes[$i]['partialFilterExpression'] = $partialFilter; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Use filtered attribute keys in partial filter expression.
The partial filter builds keys using the raw $attr from the original attribute names (line 461), but MongoDB indexes use filtered internal keys. The partial filter expression should use the same filtered keys as the index itself.
Apply this diff:
if (in_array($index->getAttribute('type'), [
Database::INDEX_UNIQUE,
Database::INDEX_KEY
])) {
$partialFilter = [];
foreach ($attributes as $attr) {
+ $filteredAttr = $this->filter($this->getInternalKeyForAttribute($attr));
// Find the matching attribute in collectionAttributes to get its type
$attrType = 'string'; // Default fallback
foreach ($collectionAttributes as $collectionAttr) {
if ($collectionAttr->getId() === $attr) {
$attrType = $this->getMongoTypeCode($collectionAttr->getAttribute('type'));
break;
}
}
// Use both $exists: true and $type to exclude nulls and ensure correct type
- $partialFilter[$attr] = ['$exists' => true, '$type' => $attrType];
+ $partialFilter[$filteredAttr] = ['$exists' => true, '$type' => $attrType];
}
if (!empty($partialFilter)) {
- $newIndexes[$i]['partialFilterExpression'] = $partialFilter;
+ $newIndexes[$indexIdx]['partialFilterExpression'] = $partialFilter;
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/Database/Adapter/Mongo.php around lines 455 to 477, the
partialFilterExpression is built using the raw attribute names ($attr) but the
index keys use filtered/internal attribute keys; change the code to look up and
use the same filtered key for each attribute when building $partialFilter (i.e.,
compute the filtered/internal attribute key the same way the index key was
generated and use that as the array key for $partialFilter instead of $attr),
keep the same $exists and $type checks, and ensure the resulting $newIndexes
entry uses those filtered keys so the partialFilterExpression matches the actual
index keys.
Summary by CodeRabbit
New Features
Infrastructure
Validation
Tests