Mongo upsert #619
Mongo upsert #619
Conversation
…-mongo-2 # Conflicts: # composer.lock
WalkthroughThis update introduces a MongoDB adapter to the database abstraction layer, adds MongoDB and mongo-express services to the Docker Compose setup, and adjusts the build and dependencies to support MongoDB. The core Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant DB as Database
participant Adapter as Mongo/MongoDB Adapter
participant Mongo as MongoDB Server
App->>DB: createDocument(document)
DB->>Adapter: castingBefore(document)
Adapter->>Mongo: Insert document
Mongo-->>Adapter: Insert result
Adapter->>DB: castingAfter(document)
DB-->>App: Document (casted)
App->>DB: find(collection, queries)
DB->>Adapter: find(collection, queries)
Adapter->>Mongo: Query with filters
Mongo-->>Adapter: Documents
Adapter->>DB: castingAfter(documents)
DB-->>App: Documents (casted)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🧹 Nitpick comments (14)
phpunit.xml (1)
10-10: Consider the impact on CI workflows.Setting
stopOnFailure="true"is useful for quick feedback during development but will prevent discovering multiple test failures in CI runs. Consider if this should be configurable or environment-specific.tests/e2e/Adapter/Scopes/AttributeTests.php (1)
1296-1296: Address the uncertainty in the comment.The comment "Is this hack valid?" suggests uncertainty about the approach. If this is a legitimate concern, it should be addressed properly rather than left as a questioning comment in the code.
Consider either:
- Removing the comment if the approach is valid
- Documenting why this calculation is necessary for MongoDB compatibility
- Implementing a more robust solution if this is indeed a hack
- /** Is this hack valid? */ + // Dynamic sizing needed for MongoDB compatibility - different adapters have different max index lengthstests/e2e/Adapter/Scopes/DocumentTests.php (1)
1983-1987: Remove forgotten debug statementThe commented
var_dumpleft behind in the test pollutes the diff and might get uncommented by mistake.- //var_dump($movieDocuments);tests/e2e/Adapter/Scopes/CollectionTests.php (1)
689-691: Consider making the index length constraint configurable.The comment suggests that increasing the maximum index length to 1024 would allow tests to pass but break others. Consider making this constraint adapter-specific or configurable to avoid this trade-off.
docker-compose.yml (1)
75-85: Consider using a specific MongoDB version instead of 'latest'.Using
mongo:latestcan lead to unpredictable behavior when the latest version changes. Consider pinning to a specific version for reproducible builds.- image: mongo:latest + image: mongo:7.0tests/e2e/Adapter/MongoDBTest.php (2)
79-97: Consider adding meaningful test stubs or skipping tests.Instead of returning
truefor unsupported operations, consider using PHPUnit'smarkTestSkipped()to make it clear these features aren't supported by MongoDB.public function testRenameAttribute(): void { - $this->assertTrue(true); + $this->markTestSkipped('MongoDB adapter does not support renaming attributes'); }
99-107: Address static analysis warnings for unused parameters.The static analysis correctly identifies unused parameters in the stubbed methods. Consider adding parameter documentation or using parameter name prefixes to indicate they're intentionally unused.
-protected static function deleteColumn(string $collection, string $column): bool +protected static function deleteColumn(string $_collection, string $_column): bool { return true; } -protected static function deleteIndex(string $collection, string $index): bool +protected static function deleteIndex(string $_collection, string $_index): bool { return true; }tests/e2e/Adapter/SharedTables/MongoDBTest.php (2)
82-100: Consider using test skipping instead of trivial assertions.Similar to the non-shared tables version, these stubbed tests would be clearer if they used
markTestSkipped()to indicate unsupported MongoDB features.public function testRenameAttribute(): void { - $this->assertTrue(true); + $this->markTestSkipped('MongoDB adapter does not support renaming attributes'); }
102-110: Address static analysis warnings for unused parameters.The same unused parameter warnings apply here. Consider prefixing parameter names with underscore to indicate they're intentionally unused.
-protected static function deleteColumn(string $collection, string $column): bool +protected static function deleteColumn(string $_collection, string $_column): bool { return true; } -protected static function deleteIndex(string $collection, string $index): bool +protected static function deleteIndex(string $_collection, string $_index): bool { return true; }src/Database/Adapter/Mongo.php (5)
109-119: Document that MongoDB creates databases automatically.The
create()method always returnstruewithout performing any operation. While this aligns with MongoDB's behavior of creating databases on first write, it should be documented./** * Create Database + * + * Note: MongoDB creates databases automatically when data is first written. + * This method exists for interface compatibility and always returns true. * * @param string $name * * @return bool */ public function create(string $name): bool { return true; }
1018-1018: Fix typo in TODO comment.- ["ordered" => false] // TODO Do we want to continue if an error is thrown? + ["ordered" => false] // TODO: Do we want to continue if an error is thrown?
1309-1309: Add space after if keyword.Follow PSR-12 coding standards for control structure spacing.
- if($originalPrev === '$sequence'){ + if ($originalPrev === '$sequence') { $tmp = new ObjectId($tmp); } // ... later in code ... - if($originalAttribute === '$sequence'){ + if ($originalAttribute === '$sequence') { $tmp = new ObjectId($tmp); /** If there is only $sequence attribute in $orderAttributes skip Or And operators **/ - if(count($orderAttributes) === 1){ + if (count($orderAttributes) === 1) {Also applies to: 1324-1324
2063-2068: Fix typo in PHPDoc comment./** * Returns number of attributes used by default. - *p + * * @return int */
952-1037: Consider optimizing the createOrUpdateDocuments implementation.The current implementation could be optimized:
- The function uses
bulkUpsertwhich is good for performance- However, fetching sequences after upsert might be inefficient for large batches
Consider implementing a more efficient approach:
- Use MongoDB's
findAndModifywithupsert: trueandnew: trueto get the document after upsert in one operation- Or leverage the write result from
bulkWriteto identify which documents were inserted vs updated
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Dockerfile(2 hunks)composer.json(2 hunks)docker-compose.yml(2 hunks)phpunit.xml(1 hunks)src/Database/Adapter.php(1 hunks)src/Database/Adapter/Mongo.php(1 hunks)tests/e2e/Adapter/Base.php(1 hunks)tests/e2e/Adapter/MongoDBTest.php(1 hunks)tests/e2e/Adapter/Scopes/AttributeTests.php(2 hunks)tests/e2e/Adapter/Scopes/CollectionTests.php(3 hunks)tests/e2e/Adapter/Scopes/DocumentTests.php(8 hunks)tests/e2e/Adapter/Scopes/GeneralTests.php(2 hunks)tests/e2e/Adapter/SharedTables/MongoDBTest.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
tests/e2e/Adapter/Scopes/AttributeTests.php (10)
src/Database/Adapter/Mongo.php (3)
createAttribute(368-371)getMaxIndexLength(2192-2195)deleteIndex(693-700)src/Database/Adapter.php (3)
createAttribute(541-541)getMaxIndexLength(852-852)deleteIndex(663-663)src/Database/Adapter/SQL.php (2)
createAttribute(216-231)getMaxIndexLength(1583-1589)src/Database/Adapter/Postgres.php (2)
createAttribute(441-460)deleteIndex(894-907)src/Database/Adapter/Pool.php (3)
createAttribute(168-171)getMaxIndexLength(308-311)deleteIndex(218-221)src/Database/Database.php (3)
createAttribute(1552-1603)Database(35-6790)deleteIndex(3145-3170)src/Database/Mirror.php (2)
createAttribute(295-357)deleteIndex(514-538)tests/e2e/Adapter/Base.php (1)
deleteIndex(50-50)src/Database/Adapter/MariaDB.php (1)
deleteIndex(783-803)src/Database/Adapter/SQLite.php (1)
deleteIndex(488-507)
tests/e2e/Adapter/SharedTables/MongoDBTest.php (4)
tests/e2e/Adapter/Base.php (4)
Base(18-64)getDatabase(34-34)deleteColumn(42-42)deleteIndex(50-50)src/Database/Database.php (8)
Database(35-6790)getDatabase(672-675)setDatabase(657-662)setNamespace(630-635)exists(1148-1153)delete(1176-1190)create(1118-1137)deleteIndex(3145-3170)src/Database/Adapter/Mongo.php (5)
Mongo(22-2216)exists(131-149)delete(176-181)create(116-119)deleteIndex(693-700)tests/e2e/Adapter/MongoDBTest.php (10)
MongoDBTest(13-108)getAdapterName(23-26)getDatabase(32-65)testCreateExistsDelete(70-77)testRenameAttribute(79-82)testRenameAttributeExisting(84-87)testUpdateAttributeStructure(89-92)testKeywords(94-97)deleteColumn(99-102)deleteIndex(104-107)
src/Database/Adapter/Mongo.php (16)
src/Database/Database.php (42)
Database(35-6790)__construct(383-460)setTimeout(765-770)clearTimeout(778-781)ping(1099-1102)reconnect(1104-1107)create(1118-1137)exists(1148-1153)list(1160-1167)delete(1176-1190)createCollection(1205-1322)getSizeOfCollectionOnDisk(1445-1462)getSizeOfCollection(1423-1436)deleteCollection(1483-1526)analyzeCollection(1470-1473)createAttribute(1552-1603)createAttributes(1619-1711)deleteAttribute(2265-2333)renameAttribute(2348-2412)createRelationship(2432-2609)updateRelationship(2624-2811)getDocument(3183-3340)deleteRelationship(2825-2939)createIndex(3014-3131)renameIndex(2955-2993)deleteIndex(3145-3170)getTenant(929-932)find(5971-6132)createDocument(3587-3672)createDocuments(3687-3771)updateDocument(4108-4293)updateDocuments(4315-4474)createOrUpdateDocuments(4879-4892)count(6229-6264)increaseDocumentAttribute(5121-5202)deleteDocument(5316-5366)deleteDocuments(5764-5910)sum(6279-6307)getLimitForAttributes(6641-6648)getLimitForIndexes(6655-6658)getConnectionId(536-539)getSchemaAttributes(6726-6729)src/Database/Adapter.php (76)
Adapter(11-1207)setTimeout(304-304)getSupportForTimeouts(940-940)clearTimeout(317-321)startTransaction(331-331)commitTransaction(343-343)rollbackTransaction(354-354)ping(458-458)reconnect(463-463)create(472-472)exists(483-483)list(490-490)delete(499-499)createCollection(509-509)filter(1109-1118)getSizeOfCollectionOnDisk(819-819)getSizeOfCollection(810-810)deleteCollection(518-518)analyzeCollection(526-526)createAttribute(541-541)createAttributes(552-552)deleteAttribute(577-577)renameAttribute(587-587)createRelationship(598-598)updateRelationship(614-614)getDocument(674-674)deleteRelationship(628-628)createIndex(653-653)renameIndex(638-638)deleteIndex(663-663)getTenant(211-214)getAttributeSelections(1085-1100)find(778-778)createDocument(684-684)createDocuments(696-696)updateDocument(707-707)updateDocuments(722-722)createOrUpdateDocuments(734-738)count(801-801)increaseDocumentAttribute(1160-1168)deleteDocument(748-748)deleteDocuments(759-759)sum(790-790)escapeWildcards(1120-1145)getLimitForString(826-826)getLimitForInt(833-833)getLimitForAttributes(840-840)getLimitForIndexes(847-847)getMinDateTime(859-859)getSupportForSchemas(876-876)getSupportForAttributes(883-883)getSupportForIndex(897-897)getSupportForUniqueIndex(904-904)getSupportForFulltextIndex(911-911)getSupportForFulltextWildcardIndex(918-918)getSupportForQueryContains(933-933)getSupportForRelationships(947-947)getSupportForUpdateLock(949-949)getSupportForAttributeResizing(963-963)getSupportForBatchOperations(956-956)getSupportForGetConnectionId(970-970)getSupportForCacheSkipOnFailure(991-991)getSupportForHostname(1005-1005)getSupportForSchemaAttributes(890-890)getSupportForCastIndexArray(977-977)getSupportForUpserts(984-984)getSupportForReconnection(998-998)getSupportForBatchCreateAttributes(1012-1012)getSupportForCasting(926-926)quote(451-451)execute(1206-1206)getMaxIndexLength(852-852)getConnectionId(1175-1175)getInternalIndexesKeys(1182-1182)getSchemaAttributes(1191-1191)getTenantQuery(1200-1200)src/Database/Exception.php (2)
Exception(7-21)__construct(9-20)src/Database/DateTime.php (2)
DateTime(7-86)format(29-32)src/Database/Document.php (14)
Document(12-460)__construct(28-54)getAttribute(212-219)getId(59-62)getSequence(67-70)getTenant(168-175)find(294-312)removeAttribute(275-283)setAttribute(232-249)getArrayCopy(413-448)getAttributes(182-200)getCreatedAt(152-155)getUpdatedAt(160-163)getPermissions(83-86)src/Database/Validator/Queries/Document.php (1)
Document(10-43)src/Database/Exception/Timeout.php (1)
Timeout(7-9)src/Database/Query.php (10)
Query(8-730)__construct(89-98)getAttribute(120-123)setAttribute(161-166)equal(355-358)limit(496-499)offset(507-510)groupByType(619-698)getMethod(112-115)getValues(128-131)src/Database/Validator/Authorization.php (3)
Authorization(7-225)__construct(29-32)getRoles(101-104)src/Database/Adapter/SQL.php (45)
__construct(27-30)startTransaction(35-60)commitTransaction(65-91)rollbackTransaction(96-119)ping(128-133)reconnect(135-138)exists(149-191)list(198-201)createAttribute(216-231)createAttributes(241-267)deleteAttribute(308-321)renameAttribute(279-296)getDocument(333-397)createDocuments(1762-1903)updateDocuments(412-638)processException(1737-1740)deleteDocuments(651-710)getInternalKeyForAttribute(1712-1724)escapeWildcards(1726-1735)getLimitForString(763-766)getLimitForInt(773-776)getLimitForAttributes(785-788)getLimitForIndexes(796-799)getSupportForSchemas(806-809)getSupportForAttributes(826-829)getSupportForIndex(816-819)getSupportForUniqueIndex(836-839)getSupportForFulltextIndex(846-849)getSupportForQueryContains(1365-1368)getSupportForRelationships(1382-1385)getSupportForUpdateLock(856-859)getSupportForAttributeResizing(866-869)getSupportForBatchOperations(876-879)getSupportForGetConnectionId(886-889)getSupportForCacheSkipOnFailure(896-899)getSupportForHostname(906-909)getSupportForCastIndexArray(1377-1380)getSupportForReconnection(1387-1390)getSupportForBatchCreateAttributes(1392-1395)getSupportForCasting(1349-1352)execute(1746-1749)getMaxIndexLength(1583-1589)getInternalIndexesKeys(1633-1636)getSchemaAttributes(1638-1641)getTenantQuery(1643-1675)src/Database/Validator/Queries/Documents.php (1)
__construct(25-72)src/Database/Validator/Structure.php (1)
__construct(104-109)src/Database/Validator/Query/Filter.php (1)
__construct(27-36)src/Database/Validator/Datetime.php (1)
__construct(18-28)src/Database/Adapter/MySQL.php (4)
setTimeout(20-39)getSizeOfCollectionOnDisk(47-79)processException(86-99)getSupportForCastIndexArray(81-84)src/Database/Change.php (1)
getNew(23-26)
🪛 GitHub Actions: Linter
tests/e2e/Adapter/Scopes/GeneralTests.php
[error] 1-1: PHP CS Fixer (PSR 12) style issue: no_whitespace_in_blank_line.
tests/e2e/Adapter/Scopes/DocumentTests.php
[error] 1-1: PHP CS Fixer (PSR 12) style issue: no_whitespace_in_blank_line.
src/Database/Adapter/Mongo.php
[error] 1-1: PHP CS Fixer (PSR 12) style issues: single_space_around_construct, method_argument_spacing.
🪛 PHPMD (2.15.0)
tests/e2e/Adapter/SharedTables/MongoDBTest.php
102-102: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
102-102: Avoid unused parameters such as '$column'. (Unused Code Rules)
(UnusedFormalParameter)
107-107: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
107-107: Avoid unused parameters such as '$index'. (Unused Code Rules)
(UnusedFormalParameter)
tests/e2e/Adapter/MongoDBTest.php
99-99: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
99-99: Avoid unused parameters such as '$column'. (Unused Code Rules)
(UnusedFormalParameter)
104-104: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
104-104: Avoid unused parameters such as '$index'. (Unused Code Rules)
(UnusedFormalParameter)
src/Database/Adapter/Mongo.php
61-61: Avoid unused parameters such as '$event'. (Unused Code Rules)
(UnusedFormalParameter)
116-116: Avoid unused parameters such as '$name'. (Unused Code Rules)
(UnusedFormalParameter)
131-131: Avoid unused parameters such as '$database'. (Unused Code Rules)
(UnusedFormalParameter)
351-351: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
368-368: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
368-368: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
368-368: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
368-368: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
368-368: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
368-368: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
381-381: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
381-381: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
439-439: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
439-439: Avoid unused parameters such as '$relatedCollection'. (Unused Code Rules)
(UnusedFormalParameter)
439-439: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
439-439: Avoid unused parameters such as '$twoWay'. (Unused Code Rules)
(UnusedFormalParameter)
439-439: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
439-439: Avoid unused parameters such as '$twoWayKey'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$side'. (Unused Code Rules)
(UnusedFormalParameter)
592-592: Avoid unused parameters such as '$lengths'. (Unused Code Rules)
(UnusedFormalParameter)
711-711: Avoid unused parameters such as '$forUpdate'. (Unused Code Rules)
(UnusedFormalParameter)
846-846: Avoid unused local variables such as '$bla'. (Unused Code Rules)
(UnusedLocalVariable)
1153-1153: Avoid unused parameters such as '$permissionIds'. (Unused Code Rules)
(UnusedFormalParameter)
1194-1194: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
1194-1194: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
1194-1194: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
1194-1194: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
1797-1797: Avoid unused parameters such as '$prefix'. (Unused Code Rules)
(UnusedFormalParameter)
2103-2103: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2175-2175: Avoid unused parameters such as '$string'. (Unused Code Rules)
(UnusedFormalParameter)
2184-2184: Avoid unused parameters such as '$stmt'. (Unused Code Rules)
(UnusedFormalParameter)
2207-2207: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2212-2212: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2212-2212: Avoid unused parameters such as '$parentAlias'. (Unused Code Rules)
(UnusedFormalParameter)
🪛 GitHub Actions: CodeQL
src/Database/Adapter/Mongo.php
[error] 951-951: PHPStan: Parameter $changes of method Utopia\Database\Adapter\Mongo::createOrUpdateDocuments() has invalid type Utopia\Database\Adapter\Change.
[error] 966-966: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1026-1026: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1027-1027: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1036-1036: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1153-1153: PHPStan: PHPDoc tag @param references unknown parameter: $ids.
[error] 2214-2214: PHPStan: Method Utopia\Database\Adapter\Mongo::getTenantQuery() should return string but returns int|null.
⏰ 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 (14)
composer.json (1)
57-57: LGTM! MongoDB package suggestion is appropriate.The suggestion for
mongodb/mongodbpackage correctly indicates the runtime dependency needed for the MongoDB adapter.Dockerfile (1)
23-40: LGTM! MongoDB extension installation is well-implemented.The consolidated RUN command efficiently installs the MongoDB PHP extension with proper version pinning (1.17.0) and cleanup. This change appropriately supports the new MongoDB adapter functionality.
tests/e2e/Adapter/Base.php (1)
25-25: LGTM: Appropriate adaptation for MongoDB support.Commenting out the
RelationshipTeststrait usage is a reasonable approach to accommodate MongoDB's different relationship handling capabilities while maintaining the base test structure for other adapters.tests/e2e/Adapter/Scopes/AttributeTests.php (2)
1301-1301: Good adaptation for multi-adapter support.The dynamic calculation
$database->getAdapter()->getMaxIndexLength() - 68properly adapts to different database adapters' index length constraints, replacing the hardcoded 700 value. This accommodates MongoDB's 1024 max index length while maintaining compatibility with SQL adapters.
1476-1476: Good practice: Clean up index before creating new one.Adding
$database->deleteIndex($collection, 'indx1');before creating 'indx2' prevents index conflicts when creating multiple indexes on the same attribute. This is a solid improvement for test reliability.tests/e2e/Adapter/Scopes/DocumentTests.php (1)
4237-4243: Verify$sequencechange to a non-numeric ObjectId-like stringChanging
'$sequence'from'200'to'507f1f77bcf86cd799439011'might be fine for the new MongoDB adapter, but be aware that:
- SQL-based adapters typically persist
sequencein anINTcolumn; inserting a 24-hex string is truncated/converted and can silently end up as507, breaking ordering & uniqueness assumptions.- The surrounding test never asserts on the sequence value, so the intent (testing a case-insensitive $id collision) is still covered if the field is simply omitted.
Please double-check that this does not introduce cross-adapter failures (run the full test-matrix).
If portability is required, consider reverting to a purely-numeric value or skipping this attribute for adapters that don’t need it.tests/e2e/Adapter/Scopes/CollectionTests.php (3)
652-654: Good documentation of the change rationale.The comment clearly explains the purpose of updating the array length check to 255, which helps maintainers understand the MongoDB-specific constraint being accommodated.
693-694: Excellent use of constant for maintainability.Using
Database::ARRAY_INDEX_LENGTHinstead of a hardcoded value improves maintainability and makes the constraint source clear.
673-674: Verify multi‐attribute index prefix lengths on all adaptersWe’ve updated the e2e test in tests/e2e/Adapter/Scopes/CollectionTests.php to use a two-column index (
['username','cards']) with lengths[99,255]. Different adapters may treat column-prefix lengths differently (e.g. trimming lengths equal to the column’s defined size, ignoring them entirely in SQLite, etc.). Please confirm that:
- MariaDB (
src/Database/Adapter/MariaDB.php) applies both prefixes and returns them in listIndices- PostgreSQL (
src/Database/Adapter/Postgres.php) supports and reports the same prefix lengths- SQLite (
src/Database/Adapter/SQLite.php) behaves as expected (it may drop/ignore prefixes)- MongoDB (
src/Database/Adapter/Mongo.php) stores and exposes the fulllengthsarrayIf any adapter alters or omits one of the prefixes, adjust the test’s expected metadata or add adapter-specific assertions.
docker-compose.yml (1)
87-99: MongoDB administration interface is properly configured.The mongo-express service is well-configured with proper authentication and connects correctly to the MongoDB service.
tests/e2e/Adapter/MongoDBTest.php (2)
33-65: Well-structured MongoDB database setup.The database initialization properly configures MongoDB client with Redis cache, handles namespace generation, and ensures clean state by deleting existing databases before creation.
70-77: Appropriate override for MongoDB's behavior.The comment and implementation correctly address MongoDB's on-the-fly database creation behavior by skipping the existence check while maintaining the test's intent.
tests/e2e/Adapter/SharedTables/MongoDBTest.php (2)
54-59: Proper shared tables configuration for MongoDB.The database setup correctly enables shared tables, sets a tenant ID, and uses a fixed namespace appropriate for shared table testing.
73-80: Consistent override for MongoDB behavior.The testCreateExistsDelete override is consistent with the non-shared tables version and appropriately handles MongoDB's on-the-fly database creation.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/Database/Adapter/Mongo.php (4)
115-118: Document or implement actual database creation behavior.The
createmethod always returnstruewithout performing any database creation operation. While MongoDB creates databases on demand, this behavior should be documented or the method should attempt to create a collection to force database creation.Consider adding a comment to clarify the behavior:
public function create(string $name): bool { + // MongoDB creates databases on demand when collections are created + // This method returns true to indicate the operation can proceed return true; }
320-325: Add proper validation for query result.The code uses a generic
is_object($result)check and assumes the object has atotalSizeproperty without validation.Add proper validation:
$result = $this->getClient()->query($command); -if (is_object($result)) { +if (is_object($result) && property_exists($result, 'totalSize')) { return $result->totalSize; } else { throw new DatabaseException('No size found'); }
1763-1769: Ensure proper regex escaping for all metacharacters.The code escapes wildcards but may not escape all regex metacharacters properly when constructing patterns.
Consider using
preg_quotefor complete regex escaping:case Query::TYPE_STARTS_WITH: - $value = $this->escapeWildcards($value); - return $value.'.*'; + $value = preg_quote($value, '/'); + return '^'.$value; case Query::TYPE_ENDS_WITH: - $value = $this->escapeWildcards($value); - return '.*'.$value; + $value = preg_quote($value, '/'); + return $value.'$';
2063-2065: Fix typo in PHPDoc comment./** * Returns number of attributes used by default. - *p + * * @return int */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Database/Adapter/Mongo.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Database/Adapter/Mongo.php (9)
src/Database/Database.php (42)
Database(35-6790)setTimeout(765-770)clearTimeout(778-781)ping(1099-1102)reconnect(1104-1107)create(1118-1137)exists(1148-1153)list(1160-1167)delete(1176-1190)createCollection(1205-1322)getSizeOfCollectionOnDisk(1445-1462)getSizeOfCollection(1423-1436)deleteCollection(1483-1526)analyzeCollection(1470-1473)createAttribute(1552-1603)createAttributes(1619-1711)deleteAttribute(2265-2333)renameAttribute(2348-2412)createRelationship(2432-2609)updateRelationship(2624-2811)getDocument(3183-3340)deleteRelationship(2825-2939)createIndex(3014-3131)renameIndex(2955-2993)deleteIndex(3145-3170)getTenant(929-932)find(5971-6132)createDocument(3587-3672)createDocuments(3687-3771)updateDocument(4108-4293)updateDocuments(4315-4474)createOrUpdateDocuments(4879-4892)count(6229-6264)deleteDocument(5316-5366)deleteDocuments(5764-5910)updateAttribute(2072-2219)sum(6279-6307)getLimitForAttributes(6641-6648)getLimitForIndexes(6655-6658)getKeywords(1066-1069)getConnectionId(536-539)getSchemaAttributes(6726-6729)src/Database/Adapter.php (83)
Adapter(11-1203)setTimeout(304-304)getSupportForTimeouts(936-936)clearTimeout(317-321)startTransaction(331-331)commitTransaction(343-343)rollbackTransaction(354-354)ping(454-454)reconnect(459-459)create(468-468)exists(479-479)list(486-486)delete(495-495)createCollection(505-505)filter(1105-1114)getSizeOfCollectionOnDisk(815-815)getSizeOfCollection(806-806)deleteCollection(514-514)analyzeCollection(522-522)createAttribute(537-537)createAttributes(548-548)deleteAttribute(573-573)renameAttribute(583-583)createRelationship(594-594)updateRelationship(610-610)getDocument(670-670)deleteRelationship(624-624)createIndex(649-649)renameIndex(634-634)deleteIndex(659-659)getTenant(211-214)getAttributeSelections(1081-1096)getAttributeProjection(1073-1073)find(774-774)createDocument(680-680)createDocuments(692-692)updateDocument(703-703)updateDocuments(718-718)createOrUpdateDocuments(730-734)count(797-797)deleteDocument(744-744)deleteDocuments(755-755)updateAttribute(563-563)sum(786-786)escapeWildcards(1116-1141)getLimitForString(822-822)getLimitForInt(829-829)getLimitForAttributes(836-836)getLimitForIndexes(843-843)getMinDateTime(855-855)getSupportForSchemas(872-872)getSupportForAttributes(879-879)getSupportForIndex(893-893)getSupportForUniqueIndex(900-900)getSupportForFulltextIndex(907-907)getSupportForFulltextWildcardIndex(914-914)getSupportForQueryContains(929-929)getSupportForRelationships(943-943)getSupportForUpdateLock(945-945)getSupportForAttributeResizing(959-959)getSupportForBatchOperations(952-952)getSupportForGetConnectionId(966-966)getSupportForCacheSkipOnFailure(987-987)getSupportForHostname(1001-1001)getSupportForSchemaAttributes(886-886)getSupportForCastIndexArray(973-973)getSupportForUpserts(980-980)getSupportForReconnection(994-994)getSupportForBatchCreateAttributes(1008-1008)getCountOfAttributes(1016-1016)getCountOfDefaultAttributes(1031-1031)getCountOfIndexes(1024-1024)getCountOfDefaultIndexes(1038-1038)getDocumentSizeLimit(1046-1046)getAttributeWidth(1057-1057)getSupportForCasting(922-922)getKeywords(1064-1064)quote(447-447)execute(1202-1202)getConnectionId(1171-1171)getInternalIndexesKeys(1178-1178)getSchemaAttributes(1187-1187)getTenantQuery(1196-1196)src/Database/DateTime.php (2)
DateTime(7-86)format(29-32)src/Database/Document.php (13)
Document(12-460)getAttribute(212-219)getId(59-62)getSequence(67-70)getTenant(168-175)find(294-312)removeAttribute(275-283)setAttribute(232-249)getArrayCopy(413-448)getAttributes(182-200)getCreatedAt(152-155)getUpdatedAt(160-163)getPermissions(83-86)src/Database/Validator/Queries/Document.php (1)
Document(10-43)src/Database/Exception/Timeout.php (1)
Timeout(7-9)src/Database/Query.php (9)
Query(8-730)getAttribute(120-123)setAttribute(161-166)equal(355-358)limit(496-499)offset(507-510)groupByType(619-698)isNested(705-712)onArray(717-720)src/Database/Validator/Authorization.php (1)
Authorization(7-225)src/Database/Change.php (1)
getNew(23-26)
🪛 PHPMD (2.15.0)
src/Database/Adapter/Mongo.php
60-60: Avoid unused parameters such as '$event'. (Unused Code Rules)
(UnusedFormalParameter)
115-115: Avoid unused parameters such as '$name'. (Unused Code Rules)
(UnusedFormalParameter)
130-130: Avoid unused parameters such as '$database'. (Unused Code Rules)
(UnusedFormalParameter)
350-350: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
367-367: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
367-367: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
367-367: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
367-367: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
367-367: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
367-367: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
380-380: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
380-380: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
438-438: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
438-438: Avoid unused parameters such as '$relatedCollection'. (Unused Code Rules)
(UnusedFormalParameter)
438-438: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
438-438: Avoid unused parameters such as '$twoWay'. (Unused Code Rules)
(UnusedFormalParameter)
438-438: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
438-438: Avoid unused parameters such as '$twoWayKey'. (Unused Code Rules)
(UnusedFormalParameter)
464-464: Avoid unused parameters such as '$side'. (Unused Code Rules)
(UnusedFormalParameter)
591-591: Avoid unused parameters such as '$lengths'. (Unused Code Rules)
(UnusedFormalParameter)
710-710: Avoid unused parameters such as '$forUpdate'. (Unused Code Rules)
(UnusedFormalParameter)
845-845: Avoid unused local variables such as '$bla'. (Unused Code Rules)
(UnusedLocalVariable)
1152-1152: Avoid unused parameters such as '$permissionIds'. (Unused Code Rules)
(UnusedFormalParameter)
1193-1193: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
1193-1193: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
1193-1193: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
1193-1193: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
1796-1796: Avoid unused parameters such as '$prefix'. (Unused Code Rules)
(UnusedFormalParameter)
2102-2102: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2174-2174: Avoid unused parameters such as '$string'. (Unused Code Rules)
(UnusedFormalParameter)
2183-2183: Avoid unused parameters such as '$stmt'. (Unused Code Rules)
(UnusedFormalParameter)
2206-2206: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2211-2211: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2211-2211: Avoid unused parameters such as '$parentAlias'. (Unused Code Rules)
(UnusedFormalParameter)
🪛 GitHub Actions: CodeQL
src/Database/Adapter/Mongo.php
[error] 950-950: PHPStan: Parameter $changes of method Utopia\Database\Adapter\Mongo::createOrUpdateDocuments() has invalid type Utopia\Database\Adapter\Change.
[error] 965-965: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1025-1025: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1026-1026: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1035-1035: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1152-1152: PHPStan: PHPDoc tag @param references unknown parameter: $ids.
[error] 2213-2213: PHPStan: Method Utopia\Database\Adapter\Mongo::getTenantQuery() should return string but returns int|null.
🔇 Additional comments (1)
src/Database/Adapter/Mongo.php (1)
1017-1017: Resolve TODO regarding error handling strategy.The TODO comment questions whether to continue operations on error. This decision impacts data consistency and should be resolved.
Consider making this configurable or following the pattern used in other adapters. Setting
ordered: falsemeans operations continue even if some fail, which could lead to partial updates.
…into feat-mongo-2
…into feat-mongo-2
…dependencies in composer.lock
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (19)
src/Database/Adapter/Mongo.php (19)
46-46: Uncomment the timeout property declaration.The
setTimeout()method on line 68 assigns to$this->timeout, but the property is commented out. This will cause a runtime error.protected Client $client; -//protected ?int $timeout = null; +protected ?int $timeout = null;
78-91: Transaction methods should throw exceptions if not supported.The transaction methods always return
truewithout implementing actual transaction logic. This could mislead callers into thinking transactions are working when they're not.Consider throwing an exception to make it clear that transactions aren't supported:
public function startTransaction(): bool { - return true; + throw new DatabaseException('Transactions are not supported in MongoDB adapter'); } public function commitTransaction(): bool { - return true; + throw new DatabaseException('Transactions are not supported in MongoDB adapter'); } public function rollbackTransaction(): bool { - return true; + throw new DatabaseException('Transactions are not supported in MongoDB adapter'); }Alternatively, if MongoDB transactions are planned for future implementation, add a TODO comment.
149-149: Fix incorrect database existence check.The check
$this->getClient()->selectDatabase() != nulldoesn't verify database existence. MongoDB always returns a database object even for non-existent databases.To properly check database existence, query the list of databases:
- return $this->getClient()->selectDatabase() != null; + $databases = $this->getClient()->listDatabaseNames(); + return in_array($database, $databases);
255-294: Fix array index bug in index creation loop.The code uses
$ito access the$ordersarray inside the attributes loop, but$iis from the outer indexes loop. This will cause incorrect order assignments.// using $i and $j as counters to distinguish from $key foreach ($indexes as $i => $index) { $key = []; $unique = false; $attributes = $index->getAttribute('attributes'); $orders = $index->getAttribute('orders'); - foreach ($attributes as $attribute) { + foreach ($attributes as $j => $attribute) { $attribute = $this->filter($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;
379-382: Throw exception for unsupported operation instead of returning false.The
analyzeCollectionmethod returnsfalsewithout explanation. This could mislead callers into thinking the operation failed rather than being unsupported.Throw an exception to clearly indicate this operation is not supported:
public function analyzeCollection(string $collection): bool { - return false; + throw new DatabaseException('Analyze collection is not supported in MongoDB adapter'); }
426-434: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.$this->getClient()->update( $collection, [], ['$unset' => [$id => '']], - multi: true + ['multi' => true] );
448-455: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.$this->getClient()->update( $collection, [], ['$rename' => [$id => $name]], - multi: true + ['multi' => true] );
515-542: Fix incorrect parameter syntax for update operations.Multiple update calls use incorrect
multiparameter syntax and named parameters.- $this->getClient()->update($collection, updates: $renameKey, multi: true); + $this->getClient()->update($collection, [], $renameKey, ['multi' => true]); } if ($twoWay && !\is_null($newTwoWayKey)) { - $this->getClient()->update($relatedCollection, updates: $renameTwoWayKey, multi: true); + $this->getClient()->update($relatedCollection, [], $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($relatedCollection, [], $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($collection, [], $renameKey, ['multi' => true]); } break; case Database::RELATION_MANY_TO_MANY: $collection = $this->getDocument(Database::METADATA, $collection); $relatedCollection = $this->getDocument(Database::METADATA, $relatedCollection); $junction = $this->getNamespace() . '_' . $this->filter('_' . $collection->getSequence() . '_' . $relatedCollection->getSequence()); if (!\is_null($newKey)) { - $this->getClient()->update($junction, updates: $renameKey, multi: true); + $this->getClient()->update($junction, [], $renameKey, ['multi' => true]); } if ($twoWay && !\is_null($newTwoWayKey)) { - $this->getClient()->update($junction, updates: $renameTwoWayKey, multi: true); + $this->getClient()->update($junction, [], $renameTwoWayKey, ['multi' => true]); }
572-572: Fix junction table naming inconsistency.The junction table name construction is inconsistent with the
updateRelationshipmethod. It should use document sequences, not collection names.- $junction = $this->getNamespace() . '_' . $this->filter('_' . $collection . '_' . $relatedCollection); + $collectionDoc = $this->getDocument(Database::METADATA, $collection); + $relatedCollectionDoc = $this->getDocument(Database::METADATA, $relatedCollection); + $junction = $this->getNamespace() . '_' . $this->filter('_' . $collectionDoc->getSequence() . '_' . $relatedCollectionDoc->getSequence()); $collection = $this->getNamespace() . '_' . $this->filter($collection); $relatedCollection = $this->getNamespace() . '_' . $this->filter($relatedCollection);
578-596: Fix incorrect parameter syntax for update operations.- $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$key => '']], ['multi' => true]); if ($twoWay) { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$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' => [$key => '']], ['multi' => true]); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$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' => [$key => '']], ['multi' => true]); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], ['multi' => true]); }
607-621: Fix method signature to match parent interface.The parameter name should be
$indexAttributeTypesinstead of$collationto match the abstract method signature.public function createIndex( string $collection, string $id, string $type, array $attributes, array $lengths, array $orders, - array $collation = [] + array $indexAttributeTypes = [] ): bool
1090-1090: Fix incorrect parameter syntax for update operation.- $this->client->update($name, $filters, $updateQuery, multi: true); + $this->client->update($name, $filters, $updateQuery, ['multi' => true]);
1098-1104: Fix incorrect type hint for $changes parameter.The parameter type should be
array<Change>but theChangeclass is not imported.Add the import at the top of the file:
use Utopia\Database\Adapter; +use Utopia\Database\Change; use Utopia\Database\Database;And update the PHPDoc:
/** * @param string $collection * @param string $attribute - * @param array<Change> $changes + * @param array<\Utopia\Database\Change> $changes * @return array<Document> */
1295-1304: Fix PHPDoc parameter name.The PHPDoc references
$idsbut the parameter is named$sequences./** * Delete Documents * * @param string $collection - * @param array<string> $ids + * @param array<string> $sequences + * @param array<string> $permissionIds * * @return int */
1403-1406: Escape dynamic role values in the Mongo regex to prevent injection and ReDoSThe current code builds a regex from the raw role names without escaping, which allows unintended regex meta-characters and risks both incorrect matches and ReDoS attacks if roles become complex.
if (Authorization::$status) { - $roles = \implode('|', Authorization::getRoles()); + // Quote each role to neutralize regex metacharacters + $quoted = \array_map(fn($r) => \preg_quote($r, '/'), Authorization::getRoles()); + $roles = \implode('|', $quoted); $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')]; }
1602-1605: Add error handling for DateTime creation.Creating DateTime from string without validation could throw exceptions on invalid date formats.
Add error handling:
private function toMongoDatetime(string $dt): UTCDateTime { - return new UTCDateTime(new \DateTime($dt)); + try { + return new UTCDateTime(new \DateTime($dt)); + } catch (\Exception $e) { + throw new DatabaseException('Invalid datetime format: ' . $dt); + } }
1779-1782: Validate ObjectId format before creation.Creating ObjectId without validation could throw exceptions if the sequence is not a valid 24-character hex string.
Add validation:
if (array_key_exists('$sequence', $array)) { - $result['_id'] = new ObjectId($array['$sequence']); + try { + $result['_id'] = new ObjectId($array['$sequence']); + } catch (\Exception $e) { + // Handle invalid ObjectId format + throw new DatabaseException('Invalid sequence format: ' . $array['$sequence']); + } unset($result['$sequence']); }
2343-2346: Implement quote method or throw exception.The
quote()method returns an empty string, which could cause issues if called. It should either be properly implemented or throw an exception.protected function quote(string $string): string { - return ""; + // MongoDB doesn't require quoting identifiers like SQL databases + return $string; }
2380-2383: Fix return type mismatch.The method is declared to return
stringbut returnsint|nullfromgetTenant().public function getTenantQuery(string $collection, string $parentAlias = ''): string { - return $this->getTenant(); + return (string) $this->getTenant(); }
🧹 Nitpick comments (5)
src/Database/Adapter/SQL.php (2)
821-824: Consider suppressing the unused parameter warning.Since this is a no-op implementation of an abstract method, the
$collectionparameter is intentionally unused. Consider adding a PHPDoc annotation to suppress the static analysis warning.+/** + * @inheritDoc + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ public function internalCastingFrom(Document $collection, Document $document): Document { return $document; }
826-829: Consider suppressing the unused parameter warning.Since this is a no-op implementation of an abstract method, the
$collectionparameter is intentionally unused. Consider adding a PHPDoc annotation to suppress the static analysis warning.+/** + * @inheritDoc + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ public function internalCastingTo(Document $collection, Document $document): Document { return $document; }src/Database/Adapter.php (1)
1205-1239: Fix docblock formatting.The abstract method declarations look good for supporting MongoDB-specific features. However, there's a formatting issue in the docblock.
- /** + /** * Set UTC Datetime * * @param string $value * @return mixed */src/Database/Database.php (2)
6147-6147: Remove commented debug code.The commented
//var_dump($node);should be removed entirely rather than left as a comment.- //var_dump($node);
6722-6722: Remove commented debug statement.The commented
//var_dump($query->getAttribute() );should be removed entirely.- //var_dump($query->getAttribute() );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
Dockerfile(2 hunks)composer.json(2 hunks)src/Database/Adapter.php(2 hunks)src/Database/Adapter/Mongo.php(1 hunks)src/Database/Adapter/SQL.php(1 hunks)src/Database/Database.php(26 hunks)tests/e2e/Adapter/Scopes/CollectionTests.php(3 hunks)tests/e2e/Adapter/Scopes/DocumentTests.php(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- composer.json
🚧 Files skipped from review as they are similar to previous changes (3)
- Dockerfile
- tests/e2e/Adapter/Scopes/CollectionTests.php
- tests/e2e/Adapter/Scopes/DocumentTests.php
🧰 Additional context used
🧠 Learnings (1)
src/Database/Adapter/SQL.php (1)
Learnt from: ArnabChatterjee20k
PR: #613
File: src/Database/Adapter/Postgres.php:1254-1319
Timestamp: 2025-07-01T11:31:37.438Z
Learning: In PostgreSQL adapter methods like getUpsertStatement, complexity for database-specific SQL generation is acceptable when the main business logic is properly separated in the parent SQL adapter class, following the adapter pattern where each database adapter handles its own SQL syntax requirements.
🧬 Code Graph Analysis (2)
src/Database/Adapter.php (3)
src/Database/Adapter/SQL.php (5)
internalCastingFrom(821-824)internalCastingTo(826-829)isMongo(831-834)getSupportForInternalCasting(816-819)setUTCDatetime(836-839)src/Database/Adapter/Mongo.php (5)
internalCastingFrom(819-869)internalCastingTo(880-928)isMongo(2039-2042)getSupportForInternalCasting(2033-2036)setUTCDatetime(2044-2047)src/Database/Document.php (1)
Document(12-460)
src/Database/Adapter/SQL.php (4)
src/Database/Adapter.php (5)
getSupportForInternalCasting(1230-1230)internalCastingFrom(1211-1211)internalCastingTo(1219-1219)isMongo(1223-1223)setUTCDatetime(1238-1238)src/Database/Adapter/Mongo.php (5)
getSupportForInternalCasting(2033-2036)internalCastingFrom(819-869)internalCastingTo(880-928)isMongo(2039-2042)setUTCDatetime(2044-2047)src/Database/Document.php (1)
Document(12-460)src/Database/Validator/Queries/Document.php (1)
Document(10-43)
🪛 PHPMD (2.15.0)
src/Database/Adapter/Mongo.php
62-62: Avoid unused parameters such as '$event'. (Unused Code Rules)
(UnusedFormalParameter)
117-117: Avoid unused parameters such as '$name'. (Unused Code Rules)
(UnusedFormalParameter)
132-132: Avoid unused parameters such as '$database'. (Unused Code Rules)
(UnusedFormalParameter)
379-379: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
396-396: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
396-396: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
396-396: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
396-396: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
396-396: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
396-396: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
409-409: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
409-409: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
467-467: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
467-467: Avoid unused parameters such as '$relatedCollection'. (Unused Code Rules)
(UnusedFormalParameter)
467-467: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
467-467: Avoid unused parameters such as '$twoWay'. (Unused Code Rules)
(UnusedFormalParameter)
467-467: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
467-467: Avoid unused parameters such as '$twoWayKey'. (Unused Code Rules)
(UnusedFormalParameter)
493-493: Avoid unused parameters such as '$side'. (Unused Code Rules)
(UnusedFormalParameter)
620-620: Avoid unused parameters such as '$lengths'. (Unused Code Rules)
(UnusedFormalParameter)
743-743: Avoid unused parameters such as '$forUpdate'. (Unused Code Rules)
(UnusedFormalParameter)
1303-1303: Avoid unused parameters such as '$permissionIds'. (Unused Code Rules)
(UnusedFormalParameter)
1343-1343: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
1343-1343: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
1343-1343: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
1343-1343: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
1565-1571: Avoid unused private methods such as 'timeToDocument'. (Unused Code Rules)
(UnusedPrivateMethod)
1581-1593: Avoid unused private methods such as 'timeToMongo'. (Unused Code Rules)
(UnusedPrivateMethod)
1943-1943: Avoid unused parameters such as '$prefix'. (Unused Code Rules)
(UnusedFormalParameter)
2271-2271: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2343-2343: Avoid unused parameters such as '$string'. (Unused Code Rules)
(UnusedFormalParameter)
2352-2352: Avoid unused parameters such as '$stmt'. (Unused Code Rules)
(UnusedFormalParameter)
2375-2375: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2380-2380: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2380-2380: Avoid unused parameters such as '$parentAlias'. (Unused Code Rules)
(UnusedFormalParameter)
src/Database/Adapter/SQL.php
821-821: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
826-826: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (9)
src/Database/Database.php (9)
3299-3305: LGTM! Internal casting integration for document retrieval.The addition of
internalCastingFrom()after retrieving a document from the adapter is correct and aligns with the MongoDB adapter's need to handle BSON/MongoDB-specific data conversions.
3661-3672: LGTM! Proper internal casting for document creation.The bidirectional casting pattern (to/from) ensures documents are properly formatted for MongoDB storage and retrieval.
3758-3770: LGTM! Consistent internal casting for batch document creation.The implementation correctly applies internal casting to each document in the batch, maintaining consistency with single document operations.
4125-4298: LGTM! Internal casting properly implemented for document updates.The casting pattern is consistently applied before and after the update operation.
4461-4473: LGTM! Batch update operations properly handle internal casting.The implementation maintains consistency with other batch operations.
5069-5104: LGTM! Upsert operations properly implement internal casting.The implementation correctly handles casting for both old and new documents in the upsert operation.
6073-6148: LGTM! Internal casting properly implemented for find operations.The implementation correctly handles:
- Cursor casting before encoding
- Result casting after retrieval
- Proper conversion from static to instance method for
convertQueries()
6293-6293: LGTM! Consistent use of instance method for query conversion.The changes properly update
count()andsum()methods to use the instance version ofconvertQueries().Also applies to: 6337-6337
6704-6732: LGTM! MongoDB-specific datetime handling properly implemented.The conversion to an instance method and the conditional datetime handling based on adapter type (MongoDB vs others) is correctly implemented. This ensures proper timezone handling for different database systems.
src/Database/Adapter/Mongo.php
Outdated
| switch ($type) { | ||
| case Database::VAR_DATETIME : | ||
| $node = new UTCDateTime(new \DateTime($node)); | ||
| break; | ||
| default: | ||
| break; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for DateTime creation.
Creating DateTime from string without validation could throw exceptions on invalid date formats.
foreach ($value as &$node) {
switch ($type) {
case Database::VAR_DATETIME :
- $node = new UTCDateTime(new \DateTime($node));
+ try {
+ $node = new UTCDateTime(new \DateTime($node));
+ } catch (\Exception $e) {
+ throw new DatabaseException('Invalid datetime format: ' . $node);
+ }
break;
default:
break;
}
}📝 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.
| switch ($type) { | |
| case Database::VAR_DATETIME : | |
| $node = new UTCDateTime(new \DateTime($node)); | |
| break; | |
| default: | |
| break; | |
| } | |
| foreach ($value as &$node) { | |
| switch ($type) { | |
| case Database::VAR_DATETIME : | |
| - $node = new UTCDateTime(new \DateTime($node)); | |
| + try { | |
| + $node = new UTCDateTime(new \DateTime($node)); | |
| + } catch (\Exception $e) { | |
| + throw new DatabaseException('Invalid datetime format: ' . $node); | |
| + } | |
| break; | |
| default: | |
| break; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/Database/Adapter/Mongo.php around lines 915 to 921, the code creates a
new DateTime object from a string without validating the input, which can throw
exceptions if the date format is invalid. To fix this, wrap the DateTime
creation in a try-catch block to handle potential exceptions gracefully. If an
exception occurs, log the error or handle it appropriately to prevent the
application from crashing.
…rom/to to castingBefore/After for clarity
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (20)
src/Database/Adapter/Mongo.php (20)
42-44: Uncomment the timeout property declaration.The
setTimeout()method on line 67 assigns to$this->timeout, but the property is commented out. This will cause a runtime error.protected Client $client; -//protected ?int $timeout = null; +protected ?int $timeout = null;
76-89: Transaction methods should throw exceptions if not supported.The transaction methods always return
truewithout implementing actual transaction logic. This could mislead callers into thinking transactions are working when they're not.public function startTransaction(): bool { - return true; + throw new DatabaseException('Transactions are not supported in MongoDB adapter'); } public function commitTransaction(): bool { - return true; + throw new DatabaseException('Transactions are not supported in MongoDB adapter'); } public function rollbackTransaction(): bool { - return true; + throw new DatabaseException('Transactions are not supported in MongoDB adapter'); }Alternatively, if MongoDB transactions are planned for future implementation, add a TODO comment.
147-147: Fix incorrect database existence check.The check
$this->getClient()->selectDatabase() != nulldoesn't verify database existence. MongoDB always returns a database object even for non-existent databases.- return $this->getClient()->selectDatabase() != null; + $databases = $this->getClient()->listDatabaseNames(); + return in_array($database, $databases);
254-291: Fix array index bug in index creation loop.The code uses
$ito access the$ordersarray inside the attributes loop, but$iis from the outer indexes loop. This will cause incorrect order assignments.// using $i and $j as counters to distinguish from $key foreach ($indexes as $i => $index) { $key = []; $unique = false; $attributes = $index->getAttribute('attributes'); $orders = $index->getAttribute('orders'); - foreach ($attributes as $attribute) { + foreach ($attributes as $j => $attribute) { $attribute = $this->filter($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;
377-380: Throw exception for unsupported operation instead of returning false.The
analyzeCollectionmethod returnsfalsewithout explanation. This could mislead callers into thinking the operation failed rather than being unsupported.public function analyzeCollection(string $collection): bool { - return false; + throw new DatabaseException('Analyze collection is not supported in MongoDB adapter'); }
424-432: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.$this->getClient()->update( $collection, [], ['$unset' => [$id => '']], - multi: true + ['multi' => true] );
446-453: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.$this->getClient()->update( $collection, [], ['$rename' => [$id => $name]], - multi: true + ['multi' => true] );
513-540: Fix incorrect parameter syntax for update operations.Multiple update calls use incorrect
multiparameter syntax and named parameters.- $this->getClient()->update($collection, updates: $renameKey, multi: true); + $this->getClient()->update($collection, [], $renameKey, ['multi' => true]); } if ($twoWay && !\is_null($newTwoWayKey)) { - $this->getClient()->update($relatedCollection, updates: $renameTwoWayKey, multi: true); + $this->getClient()->update($relatedCollection, [], $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($relatedCollection, [], $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($collection, [], $renameKey, ['multi' => true]); } break; case Database::RELATION_MANY_TO_MANY: $collection = $this->getDocument(Database::METADATA, $collection); $relatedCollection = $this->getDocument(Database::METADATA, $relatedCollection); $junction = $this->getNamespace() . '_' . $this->filter('_' . $collection->getSequence() . '_' . $relatedCollection->getSequence()); if (!\is_null($newKey)) { - $this->getClient()->update($junction, updates: $renameKey, multi: true); + $this->getClient()->update($junction, [], $renameKey, ['multi' => true]); } if ($twoWay && !\is_null($newTwoWayKey)) { - $this->getClient()->update($junction, updates: $renameTwoWayKey, multi: true); + $this->getClient()->update($junction, [], $renameTwoWayKey, ['multi' => true]); }
570-572: Fix junction table naming inconsistency.The junction table name construction is inconsistent with the
updateRelationshipmethod. It should use document sequences, not collection names.- $junction = $this->getNamespace() . '_' . $this->filter('_' . $collection . '_' . $relatedCollection); + $collectionDoc = $this->getDocument(Database::METADATA, $collection); + $relatedCollectionDoc = $this->getDocument(Database::METADATA, $relatedCollection); + $junction = $this->getNamespace() . '_' . $this->filter('_' . $collectionDoc->getSequence() . '_' . $relatedCollectionDoc->getSequence()); $collection = $this->getNamespace() . '_' . $this->filter($collection); $relatedCollection = $this->getNamespace() . '_' . $this->filter($relatedCollection);
576-594: Fix incorrect parameter syntax for update operations.All update calls use incorrect
multiparameter syntax.- $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$key => '']], ['multi' => true]); if ($twoWay) { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$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' => [$key => '']], ['multi' => true]); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$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' => [$key => '']], ['multi' => true]); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], ['multi' => true]); }
605-619: Fix method signature to match parent interface.The parameter name should be
$indexAttributeTypesinstead of$collationto match the abstract method signature.public function createIndex( string $collection, string $id, string $type, array $attributes, array $lengths, array $orders, - array $collation = [] + array $indexAttributeTypes = [] ): bool
905-914: Add error handling for DateTime creation.Creating DateTime from string without validation could throw exceptions on invalid date formats.
foreach ($value as &$node) { switch ($type) { case Database::VAR_DATETIME : - $node = new UTCDateTime(new \DateTime($node)); + try { + $node = new UTCDateTime(new \DateTime($node)); + } catch (\Exception $e) { + throw new DatabaseException('Invalid datetime format: ' . $node); + } break; default: break; } }
1078-1078: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.- $this->client->update($name, $filters, $updateQuery, multi: true); + $this->client->update($name, $filters, $updateQuery, ['multi' => true]);
1086-1092: Fix incorrect type hint for $changes parameter.The parameter type should be
array<Change>but the import is missing.Add the import at the top of the file:
use Utopia\Database\Adapter; +use Utopia\Database\Change; use Utopia\Database\Database;And update the PHPDoc:
/** * @param string $collection * @param string $attribute - * @param array<Change> $changes + * @param array<\Utopia\Database\Change> $changes * @return array<Document> */
1282-1291: Fix PHPDoc parameter names.The PHPDoc references
$idsbut the parameter is named$sequences. Also missing documentation for$permissionIds./** * Delete Documents * * @param string $collection - * @param array<string> $ids + * @param array<string> $sequences + * @param array<string> $permissionIds * * @return int */
1391-1394: Escape dynamic role values in the regex to prevent injection and ReDoS.The current code builds a regex from raw role names without escaping, which allows unintended regex meta-characters and risks both incorrect matches and ReDoS attacks.
if (Authorization::$status) { - $roles = \implode('|', Authorization::getRoles()); + // Quote each role to neutralize regex metacharacters + $quoted = \array_map(fn($r) => \preg_quote($r, '/'), Authorization::getRoles()); + $roles = \implode('|', $quoted); $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')]; }
1589-1592: Add error handling for DateTime creation.Creating DateTime from string without validation could throw exceptions on invalid date formats.
private function toMongoDatetime(string $dt): UTCDateTime { - return new UTCDateTime(new \DateTime($dt)); + try { + return new UTCDateTime(new \DateTime($dt)); + } catch (\Exception $e) { + throw new DatabaseException('Invalid datetime format: ' . $dt); + } }
1766-1769: Validate ObjectId format before creation.Creating ObjectId without validation could throw exceptions if the sequence is not a valid 24-character hex string.
if (array_key_exists('$sequence', $array)) { - $result['_id'] = new ObjectId($array['$sequence']); + try { + $result['_id'] = new ObjectId($array['$sequence']); + } catch (\Exception $e) { + // Handle invalid ObjectId format + throw new DatabaseException('Invalid sequence format: ' . $array['$sequence']); + } unset($result['$sequence']); }
2330-2333: Implement quote method or throw exception.The
quote()method returns an empty string, which could cause issues if called. It should either be properly implemented or throw an exception.protected function quote(string $string): string { - return ""; + // MongoDB doesn't require quoting identifiers like SQL databases + return $string; }
2367-2370: Fix return type mismatch.The method is declared to return
stringbut returnsint|nullfromgetTenant().public function getTenantQuery(string $collection, string $parentAlias = ''): string { - return $this->getTenant(); + return (string) $this->getTenant(); }
🧹 Nitpick comments (5)
src/Database/Adapter.php (3)
1204-1210: Fix documentation formatting inconsistencies.The method documentation has formatting issues that are inconsistent with the rest of the codebase.
- /** - * Returns the document after casting - * @param Document $collection - * @param Document $document - * @return Document - */ + /** + * Returns the document after casting + * @param Document $collection + * @param Document $document + * @return Document + */
1220-1225: Fix documentation formatting inconsistency.The method documentation has an indentation issue.
- /** - * Is Mongo? - * - * @return bool - */ + /** + * Is Mongo? + * + * @return bool + */
1234-1240: Fix documentation formatting inconsistency.The method documentation has an indentation issue.
- /** - * Set UTC Datetime - * - * @param string $value - * @return mixed - */ + /** + * Set UTC Datetime + * + * @param string $value + * @return mixed + */src/Database/Adapter/SQL.php (2)
821-830: Fix documentation formatting and consider parameter documentation.The method documentation has formatting inconsistencies, and while the unused parameter is expected for interface compliance, it should be documented.
- /** - * Returns the document after casting - * @param Document $collection - * @param Document $document - * @return Document - */ + /** + * Returns the document after casting + * @param Document $collection (unused in SQL adapters) + * @param Document $document + * @return Document + */
843-851: Fix documentation formatting inconsistency.The method implementation is correct, but the documentation formatting needs adjustment.
- /** - * Is Mongo? - * - * @return bool - */ + /** + * Is Mongo? + * + * @return bool + */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/Database/Adapter.php(1 hunks)src/Database/Adapter/Mongo.php(1 hunks)src/Database/Adapter/SQL.php(1 hunks)src/Database/Database.php(18 hunks)tests/e2e/Adapter/Scopes/CollectionTests.php(3 hunks)tests/e2e/Adapter/Scopes/DocumentTests.php(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/e2e/Adapter/Scopes/CollectionTests.php
- tests/e2e/Adapter/Scopes/DocumentTests.php
- src/Database/Database.php
🧰 Additional context used
🧠 Learnings (1)
src/Database/Adapter/SQL.php (1)
Learnt from: ArnabChatterjee20k
PR: #613
File: src/Database/Adapter/Postgres.php:1254-1319
Timestamp: 2025-07-01T11:31:37.438Z
Learning: In PostgreSQL adapter methods like getUpsertStatement, complexity for database-specific SQL generation is acceptable when the main business logic is properly separated in the parent SQL adapter class, following the adapter pattern where each database adapter handles its own SQL syntax requirements.
🪛 PHPMD (2.15.0)
src/Database/Adapter/Mongo.php
60-60: Avoid unused parameters such as '$event'. (Unused Code Rules)
(UnusedFormalParameter)
115-115: Avoid unused parameters such as '$name'. (Unused Code Rules)
(UnusedFormalParameter)
130-130: Avoid unused parameters such as '$database'. (Unused Code Rules)
(UnusedFormalParameter)
377-377: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
407-407: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
407-407: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$relatedCollection'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$twoWay'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$twoWayKey'. (Unused Code Rules)
(UnusedFormalParameter)
491-491: Avoid unused parameters such as '$side'. (Unused Code Rules)
(UnusedFormalParameter)
618-618: Avoid unused parameters such as '$lengths'. (Unused Code Rules)
(UnusedFormalParameter)
740-740: Avoid unused parameters such as '$forUpdate'. (Unused Code Rules)
(UnusedFormalParameter)
1290-1290: Avoid unused parameters such as '$permissionIds'. (Unused Code Rules)
(UnusedFormalParameter)
1330-1330: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
1330-1330: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
1330-1330: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
1330-1330: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
1552-1558: Avoid unused private methods such as 'timeToDocument'. (Unused Code Rules)
(UnusedPrivateMethod)
1568-1580: Avoid unused private methods such as 'timeToMongo'. (Unused Code Rules)
(UnusedPrivateMethod)
1930-1930: Avoid unused parameters such as '$prefix'. (Unused Code Rules)
(UnusedFormalParameter)
2258-2258: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2330-2330: Avoid unused parameters such as '$string'. (Unused Code Rules)
(UnusedFormalParameter)
2339-2339: Avoid unused parameters such as '$stmt'. (Unused Code Rules)
(UnusedFormalParameter)
2362-2362: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2367-2367: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2367-2367: Avoid unused parameters such as '$parentAlias'. (Unused Code Rules)
(UnusedFormalParameter)
src/Database/Adapter/SQL.php
827-827: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
838-838: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (5)
src/Database/Adapter.php (2)
1212-1218: LGTM - Method signature and documentation are correct.The
castingAftermethod provides a clean hook for post-processing documents after casting operations.
1227-1232: LGTM - Follows established patterns.The method correctly follows the established
getSupportFor*pattern used throughout the adapter class.src/Database/Adapter/SQL.php (3)
811-819: LGTM - Appropriate default for SQL adapters.Returning
falseis the correct default for SQL adapters that don't require internal casting support.
832-841: LGTM - Appropriate no-op implementation for SQL adapters.The implementation correctly returns the document unchanged, which is appropriate for SQL adapters that don't require post-casting modifications. The unused
$collectionparameter is expected for interface compliance.
853-862: LGTM - Appropriate pass-through implementation for SQL adapters.Returning the value unchanged is the correct behavior for SQL adapters that don't require special UTC datetime processing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Database/Database.php (1)
6145-6148: Remove commented debug codeThe commented
//var_dump($node);on line 6145 should be removed before merging. Debug statements should not be committed to production code.foreach ($results as &$node) { - //var_dump($node); $node = $this->adapter->castingAfter($collection, $node);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Database/Database.php(16 hunks)tests/e2e/Adapter/Scopes/CollectionTests.php(1 hunks)tests/e2e/Adapter/Scopes/DocumentTests.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/e2e/Adapter/Scopes/CollectionTests.php
- tests/e2e/Adapter/Scopes/DocumentTests.php
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ArnabChatterjee20k
PR: utopia-php/database#613
File: src/Database/Adapter/Postgres.php:1254-1319
Timestamp: 2025-07-01T11:31:37.438Z
Learning: In PostgreSQL adapter methods like getUpsertStatement, complexity for database-specific SQL generation is acceptable when the main business logic is properly separated in the parent SQL adapter class, following the adapter pattern where each database adapter handles its own SQL syntax requirements.
🔇 Additional comments (12)
src/Database/Database.php (12)
3303-3304: LGTM: Consistent casting implementationThe addition of
castingAfter()call is appropriately placed after document retrieval and before further processing, ensuring proper document casting for adapter-specific needs.
3658-3659: LGTM: Proper pre-creation castingThe
castingBefore()call is correctly positioned before document creation operations, ensuring the document is properly formatted for the adapter.
3667-3668: LGTM: Consistent post-creation castingThe
castingAfter()call is properly placed after document creation to ensure correct document formatting before further processing.
3753-3755: LGTM: Proper batch document castingThe
castingBefore()call is correctly applied to each document in the batch processing loop, ensuring all documents are properly formatted before creation.
3764-3764: LGTM: Consistent batch post-creation castingThe
castingAfter()call is appropriately applied to each document after batch creation, maintaining consistency with the casting pattern.
4288-4293: LGTM: Complete update operation castingThe implementation correctly applies
castingBefore()prior to the update operation andcastingAfter()following it, ensuring proper document formatting throughout the update process.
4456-4461: LGTM: Comprehensive batch update castingThe implementation correctly applies
castingBefore()to both individual documents and the updates object in batch operations, ensuring proper formatting for all components of the batch update.
4472-4472: LGTM: Proper batch update post-processingThe
castingAfter()call correctly processes each document after batch updates, maintaining consistency with the casting pattern.
5068-5070: LGTM: Proper upsert operation castingThe implementation correctly applies
castingBefore()to both old and new documents in upsert operations, ensuring proper formatting for the adapter's requirements.
5101-5103: LGTM: Complete upsert post-processingThe
castingAfter()call properly processes each document after upsert operations, maintaining the established casting pattern.
6072-6075: LGTM: Proper cursor casting implementationThe cursor casting before query operations ensures proper formatting for database queries, maintaining consistency with the casting pattern.
6702-6740: LGTM: Proper method conversion for adapter integrationThe conversion of
convertQueries()from static to instance method is correct and necessary for MongoDB adapter integration. The conditional logic properly handles different adapter types for datetime processing.
…and composer.lock
# Conflicts: # docker-compose.yml # src/Database/Database.php # tests/e2e/Adapter/Scopes/AttributeTests.php
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (20)
src/Database/Adapter/Mongo.php (20)
42-45: Uncomment the timeout property declaration.The
setTimeout()andclearTimeout()methods assign to$this->timeout, but the property is commented out.protected Client $client; -//protected ?int $timeout = null; +protected ?int $timeout = null;
76-89: Transaction methods should throw exceptions if not supported.The transaction methods always return
truewithout implementing actual transaction logic. This could mislead callers into thinking transactions are working when they're not.Consider throwing an exception to make it clear that transactions aren't supported:
public function startTransaction(): bool { - return true; + throw new DatabaseException('Transactions are not supported in MongoDB adapter'); } public function commitTransaction(): bool { - return true; + throw new DatabaseException('Transactions are not supported in MongoDB adapter'); } public function rollbackTransaction(): bool { - return true; + throw new DatabaseException('Transactions are not supported in MongoDB adapter'); }Alternatively, if MongoDB transactions are planned for future implementation, add a TODO comment.
146-148: Fix incorrect database existence check.The check
$this->getClient()->selectDatabase() != nulldoesn't verify database existence. MongoDB always returns a database object even for non-existent databases.To properly check database existence, query the list of databases:
- return $this->getClient()->selectDatabase() != null; + $databases = $this->getClient()->listDatabaseNames(); + return in_array($database, $databases);
253-291: Fix array index bug in index creation loop.The code uses
$ito access the$ordersarray inside the attributes loop, but$iis from the outer indexes loop. This will cause incorrect order assignments.// using $i and $j as counters to distinguish from $key foreach ($indexes as $i => $index) { $key = []; $unique = false; $attributes = $index->getAttribute('attributes'); $orders = $index->getAttribute('orders'); - foreach ($attributes as $attribute) { + foreach ($attributes as $j => $attribute) { $attribute = $this->filter($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;
377-380: Throw exception for unsupported operation instead of returning false.The
analyzeCollectionmethod returnsfalsewithout explanation. This could mislead callers into thinking the operation failed rather than being unsupported.Throw an exception to clearly indicate this operation is not supported:
public function analyzeCollection(string $collection): bool { - return false; + throw new DatabaseException('Analyze collection is not supported in MongoDB adapter'); }
424-432: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.$this->getClient()->update( $collection, [], ['$unset' => [$id => '']], - multi: true + ['multi' => true] );
446-453: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.$this->getClient()->update( $collection, [], ['$rename' => [$id => $name]], - multi: true + ['multi' => true] );
510-540: Fix incorrect parameter syntax for update operations.Multiple update calls use incorrect
multiparameter syntax and missing filter parameter.- $this->getClient()->update($collection, updates: $renameKey, multi: true); + $this->getClient()->update($collection, [], $renameKey, ['multi' => true]); } if ($twoWay && !\is_null($newTwoWayKey)) { - $this->getClient()->update($relatedCollection, updates: $renameTwoWayKey, multi: true); + $this->getClient()->update($relatedCollection, [], $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($relatedCollection, [], $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($collection, [], $renameKey, ['multi' => true]); } break; case Database::RELATION_MANY_TO_MANY: $collection = $this->getDocument(Database::METADATA, $collection); $relatedCollection = $this->getDocument(Database::METADATA, $relatedCollection); $junction = $this->getNamespace() . '_' . $this->filter('_' . $collection->getSequence() . '_' . $relatedCollection->getSequence()); if (!\is_null($newKey)) { - $this->getClient()->update($junction, updates: $renameKey, multi: true); + $this->getClient()->update($junction, [], $renameKey, ['multi' => true]); } if ($twoWay && !\is_null($newTwoWayKey)) { - $this->getClient()->update($junction, updates: $renameTwoWayKey, multi: true); + $this->getClient()->update($junction, [], $renameTwoWayKey, ['multi' => true]); }
570-572: Fix junction table naming inconsistency.The junction table name construction is inconsistent with the
updateRelationshipmethod. It should use document sequences, not collection names.- $junction = $this->getNamespace() . '_' . $this->filter('_' . $collection . '_' . $relatedCollection); + $collectionDoc = $this->getDocument(Database::METADATA, $collection); + $relatedCollectionDoc = $this->getDocument(Database::METADATA, $relatedCollection); + $junction = $this->getNamespace() . '_' . $this->filter('_' . $collectionDoc->getSequence() . '_' . $relatedCollectionDoc->getSequence()); $collection = $this->getNamespace() . '_' . $this->filter($collection); $relatedCollection = $this->getNamespace() . '_' . $this->filter($relatedCollection);
574-594: Fix incorrect parameter syntax for update operations.- $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$key => '']], ['multi' => true]); if ($twoWay) { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$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' => [$key => '']], ['multi' => true]); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$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' => [$key => '']], ['multi' => true]); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], ['multi' => true]); }
605-619: Fix method signature to match parent interface.The parameter name should be
$indexAttributeTypesinstead of$collationto match the abstract method signature.public function createIndex( string $collection, string $id, string $type, array $attributes, array $lengths, array $orders, - array $collation = [] + array $indexAttributeTypes = [] ): bool
905-913: Add error handling for DateTime creation.Creating DateTime from string without validation could throw exceptions on invalid date formats.
foreach ($value as &$node) { switch ($type) { case Database::VAR_DATETIME : - $node = new UTCDateTime(new \DateTime($node)); + try { + $node = new UTCDateTime(new \DateTime($node)); + } catch (\Exception $e) { + throw new DatabaseException('Invalid datetime format: ' . $node); + } break; default: break; } }
1077-1079: Fix incorrect parameter syntax for update operation.- $this->client->update($name, $filters, $updateQuery, multi: true); + $this->client->update($name, $filters, $updateQuery, ['multi' => true]);
1282-1291: Fix PHPDoc parameter name.The PHPDoc references
$idsbut the parameter is named$sequences./** * Delete Documents * * @param string $collection - * @param array<string> $ids + * @param array<string> $sequences + * @param array<string> $permissionIds * * @return int */
1391-1394: Escape dynamic role values in the Mongo regex to prevent injection and ReDoSThe current code builds a regex from the raw role names without escaping, which allows unintended regex meta-characters and risks both incorrect matches and ReDoS attacks if roles become complex.
if (Authorization::$status) { - $roles = \implode('|', Authorization::getRoles()); + // Quote each role to neutralize regex metacharacters + $quoted = \array_map(fn($r) => \preg_quote($r, '/'), Authorization::getRoles()); + $roles = \implode('|', $quoted); $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')]; }
1589-1592: Add error handling for DateTime creation.Creating DateTime from string without validation could throw exceptions on invalid date formats.
private function toMongoDatetime(string $dt): UTCDateTime { - return new UTCDateTime(new \DateTime($dt)); + try { + return new UTCDateTime(new \DateTime($dt)); + } catch (\Exception $e) { + throw new DatabaseException('Invalid datetime format: ' . $dt); + } }
1766-1769: Validate ObjectId format before creation.Creating ObjectId without validation could throw exceptions if the sequence is not a valid 24-character hex string.
if (array_key_exists('$sequence', $array)) { - $result['_id'] = new ObjectId($array['$sequence']); + try { + $result['_id'] = new ObjectId($array['$sequence']); + } catch (\Exception $e) { + // Handle invalid ObjectId format + throw new DatabaseException('Invalid sequence format: ' . $array['$sequence']); + } unset($result['$sequence']); }
2335-2338: Implement quote method or throw exception.The
quote()method returns an empty string, which could cause issues if called.protected function quote(string $string): string { - return ""; + // MongoDB doesn't require quoting identifiers like SQL databases + return $string; }
2372-2375: Fix return type mismatch.The method is declared to return
stringbut returnsint|nullfromgetTenant().public function getTenantQuery(string $collection, string $parentAlias = ''): string { - return $this->getTenant(); + return (string) $this->getTenant(); }
3-20: Add missing import for Change class.The
Changeclass is used in thecreateOrUpdateDocumentsmethod but is not imported.Add the missing import:
use Utopia\Database\Adapter; +use Utopia\Database\Change; use Utopia\Database\Database;
🧹 Nitpick comments (2)
src/Database/Database.php (2)
6076-6079: Clone cursor before mutating to avoid surprising caller side-effects
castingBefore()rewrites attribute values (e.g. converts strings toUTCDateTime).
Because the sameDocumentinstance is supplied by the caller, its state changes after this call.-if (!empty($cursor)) { - $cursor = $this->adapter->castingBefore($collection, $cursor); +if (!empty($cursor)) { + $cursor = $this->adapter->castingBefore($collection, clone $cursor); }This keeps the caller’s object intact.
6685-6689: Minor: avoid per-itemisMongo()checks
isMongo()is evaluated inside a tight inner loop. Caching the result once before the loops saves repeated virtual calls:$forMongo = $this->adapter->isMongo(); ... if ($forMongo) { $values[$valueIndex] = $this->adapter->setUTCDatetime($value); } else { $values[$valueIndex] = DateTime::setTimezone($value); }Pure micro-optimisation – feel free to ignore.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docker-compose.yml(2 hunks)src/Database/Adapter.php(1 hunks)src/Database/Adapter/Mongo.php(1 hunks)src/Database/Adapter/SQL.php(1 hunks)src/Database/Database.php(13 hunks)tests/e2e/Adapter/Scopes/AttributeTests.php(2 hunks)tests/e2e/Adapter/Scopes/CollectionTests.php(1 hunks)tests/e2e/Adapter/Scopes/DocumentTests.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/e2e/Adapter/Scopes/CollectionTests.php
- src/Database/Adapter.php
- tests/e2e/Adapter/Scopes/AttributeTests.php
- tests/e2e/Adapter/Scopes/DocumentTests.php
- docker-compose.yml
🧰 Additional context used
🧠 Learnings (1)
src/Database/Adapter/SQL.php (1)
Learnt from: ArnabChatterjee20k
PR: #613
File: src/Database/Adapter/Postgres.php:1254-1319
Timestamp: 2025-07-01T11:31:37.438Z
Learning: In PostgreSQL adapter methods like getUpsertStatement, complexity for database-specific SQL generation is acceptable when the main business logic is properly separated in the parent SQL adapter class, following the adapter pattern where each database adapter handles its own SQL syntax requirements.
🧬 Code Graph Analysis (1)
src/Database/Database.php (5)
src/Database/Adapter/Mongo.php (6)
castingAfter(813-862)castingBefore(871-919)createDocument(778-803)updateDocument(1018-1039)isMongo(2041-2044)setUTCDatetime(2046-2049)src/Database/Adapter.php (7)
castingAfter(1232-1232)castingBefore(1224-1224)withTransaction(372-404)createDocument(680-680)updateDocument(703-703)isMongo(1239-1239)setUTCDatetime(1254-1254)src/Database/Adapter/SQL.php (4)
castingAfter(854-857)castingBefore(843-846)isMongo(864-867)setUTCDatetime(875-878)src/Database/Mirror.php (3)
silent(172-175)createDocument(540-585)updateDocument(662-708)src/Database/DateTime.php (2)
DateTime(7-86)setTimezone(58-67)
🪛 PHPMD (2.15.0)
src/Database/Adapter/Mongo.php
60-60: Avoid unused parameters such as '$event'. (Unused Code Rules)
(UnusedFormalParameter)
115-115: Avoid unused parameters such as '$name'. (Unused Code Rules)
(UnusedFormalParameter)
130-130: Avoid unused parameters such as '$database'. (Unused Code Rules)
(UnusedFormalParameter)
377-377: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
407-407: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
407-407: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$relatedCollection'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$twoWay'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$twoWayKey'. (Unused Code Rules)
(UnusedFormalParameter)
491-491: Avoid unused parameters such as '$side'. (Unused Code Rules)
(UnusedFormalParameter)
618-618: Avoid unused parameters such as '$lengths'. (Unused Code Rules)
(UnusedFormalParameter)
740-740: Avoid unused parameters such as '$forUpdate'. (Unused Code Rules)
(UnusedFormalParameter)
1290-1290: Avoid unused parameters such as '$permissionIds'. (Unused Code Rules)
(UnusedFormalParameter)
1330-1330: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
1330-1330: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
1330-1330: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
1330-1330: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
1552-1558: Avoid unused private methods such as 'timeToDocument'. (Unused Code Rules)
(UnusedPrivateMethod)
1568-1580: Avoid unused private methods such as 'timeToMongo'. (Unused Code Rules)
(UnusedPrivateMethod)
1930-1930: Avoid unused parameters such as '$prefix'. (Unused Code Rules)
(UnusedFormalParameter)
2263-2263: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2335-2335: Avoid unused parameters such as '$string'. (Unused Code Rules)
(UnusedFormalParameter)
2344-2344: Avoid unused parameters such as '$stmt'. (Unused Code Rules)
(UnusedFormalParameter)
2367-2367: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2372-2372: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2372-2372: Avoid unused parameters such as '$parentAlias'. (Unused Code Rules)
(UnusedFormalParameter)
src/Database/Adapter/SQL.php
843-843: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
854-854: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
🪛 GitHub Actions: CodeQL
src/Database/Adapter/Mongo.php
[error] 1092-1092: Parameter $changes of method Utopia\Database\Adapter\Mongo::createOrUpdateDocuments() has invalid type Utopia\Database\Adapter\Change.
[error] 1107-1107: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1163-1163: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1164-1164: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1164-1164: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1173-1173: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1184-1184: Parameter #2 $documentIds (array) of method Utopia\Database\Adapter\Mongo::getSequences() should be compatible with parameter $documents (array<Utopia\Database\Document>) of method Utopia\Database\Adapter::getSequences().
[error] 1184-1184: Return type (array<string, string>) of method Utopia\Database\Adapter\Mongo::getSequences() should be compatible with return type (array<Utopia\Database\Document>) of method Utopia\Database\Adapter::getSequences().
[error] 1290-1290: PHPDoc tag @param references unknown parameter: $ids.
[error] 1522-1522: Method Utopia\Database\Adapter\Mongo::timeFilter() is unused.
[error] 1552-1552: Method Utopia\Database\Adapter\Mongo::timeToDocument() is unused.
[error] 1568-1568: Method Utopia\Database\Adapter\Mongo::timeToMongo() is unused.
[error] 2374-2374: Method Utopia\Database\Adapter\Mongo::getTenantQuery() should return string but returns int|null.
⏰ 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 (7)
src/Database/Adapter/SQL.php (5)
827-835: LGTM! Correct default implementation for SQL adapters.The method appropriately returns
falseindicating that SQL adapters don't support internal casting, which aligns with the adapter pattern where MongoDB-specific features are disabled in SQL adapters.
837-846: LGTM! Appropriate no-op implementation for SQL adapters.The method correctly provides a default implementation that returns the document unchanged. The unused
$collectionparameter flagged by static analysis is expected here - it's part of the interface contract and will be used in other adapter implementations like MongoDB.
848-857: LGTM! Consistent no-op implementation for SQL adapters.The method provides the correct default behavior for SQL adapters. The unused
$collectionparameter warning from static analysis is expected and can be ignored - it's part of the interface contract for adapter implementations.
859-867: LGTM! Correct adapter type identification.The method appropriately returns
falsefor SQL adapters, providing clear type identification for the adapter pattern implementation.
869-878: LGTM! Appropriate datetime handling for SQL adapters.The method correctly returns the datetime value unchanged, which is the appropriate behavior for SQL adapters that don't require special UTC datetime transformation.
src/Database/Database.php (2)
3271-3272: Good placement of adapter-level post-castingIntroducing
castingAfter()right after the raw fetch ensures Mongo-specific type hydration happens before any permission, decoding or relationship logic runs.
3643-3653: Pre- and post-casting integrated correctly
castingBefore()is executed immediately before persistence andcastingAfter()right after the adapter write – this keeps the in-memory object consistent with the stored representation.
src/Database/Database.php
Outdated
| $document = $this->adapter->castingBefore($collection, $document); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Assign the possibly new instance back into the documents array
castingBefore() might return a new Document instance (adapters are free to clone).
At the moment the returned object is stored only in $document, leaving $documents[$key] pointing at the pre-cast object.
foreach ($documents as $key => $document) {
...
- $document = $this->adapter->castingBefore($collection, $document);
+ $document = $this->adapter->castingBefore($collection, $document);
+ $documents[$key] = $document; // keep array in sync
}📝 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.
| $document = $this->adapter->castingBefore($collection, $document); | |
| foreach ($documents as $key => $document) { | |
| // … any other processing … | |
| $document = $this->adapter->castingBefore($collection, $document); | |
| $documents[$key] = $document; // keep array in sync | |
| } |
🤖 Prompt for AI Agents
In src/Database/Database.php around lines 3740 to 3741, the result of calling
castingBefore() is assigned to $document but not saved back into the $documents
array. Since castingBefore() may return a new Document instance, update the
$documents array at the current key with the returned $document to ensure the
array holds the casted objects.
…ocuments, improving sequence retrieval logic and preserving document structure.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (20)
src/Database/Adapter/Mongo.php (20)
42-45: Uncomment the timeout property declaration.The
setTimeout()method on line 66 assigns to$this->timeout, but the property is commented out.
76-89: Transaction methods should throw exceptions if not supported.The transaction methods always return
truewithout implementing actual transaction logic. This could mislead callers into thinking transactions are working when they're not.
147-147: Fix incorrect database existence check.The check
$this->getClient()->selectDatabase() != nulldoesn't verify database existence. MongoDB always returns a database object even for non-existent databases.
266-289: Fix array index bug in index creation loop.The code uses
$ito access the$ordersarray inside the attributes loop, but$iis from the outer indexes loop. This will cause incorrect order assignments.
377-380: Throw exception for unsupported operation instead of returning false.The
analyzeCollectionmethod returnsfalsewithout explanation. This could mislead callers into thinking the operation failed rather than being unsupported.
424-432: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.
446-453: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.
513-540: Fix incorrect parameter syntax for update operations.Multiple update calls use incorrect
multiparameter syntax.
570-570: Fix junction table naming inconsistency.The junction table name construction is inconsistent with the
updateRelationshipmethod. It should use document sequences, not collection names.
576-594: Fix incorrect parameter syntax for update operations.
605-619: Fix method signature to match parent interface.The parameter name should be
$indexAttributeTypesinstead of$collationto match the abstract method signature.
905-914: Add error handling for DateTime creation.Creating DateTime from string without validation could throw exceptions on invalid date formats.
1078-1078: Fix incorrect parameter syntax for update operation.
1302-1311: Fix PHPDoc parameter name.The PHPDoc references
$idsbut the parameter is named$sequences.
1411-1414: Security consideration for permission regex.The permission filtering uses regex with user-provided roles. While the roles are escaped, consider the performance impact and potential ReDoS attacks with complex permission strings.
1609-1612: Add error handling for DateTime creation.Creating DateTime from string without validation could throw exceptions on invalid date formats.
1786-1789: Validate ObjectId format before creation.Creating ObjectId without validation could throw exceptions if the sequence is not a valid 24-character hex string.
2355-2358: Implement quote method or throw exception.The
quote()method returns an empty string, which could cause issues if called. It should either be properly implemented or throw an exception.
2392-2395: Fix return type mismatch.The method is declared to return
stringbut returnsint|nullfromgetTenant().
5-19: Add missing import for the Change class.The
Changeclass is used in thecreateOrUpdateDocumentsmethod but not imported.Add the import after line 11:
use Utopia\Database\Database; use Utopia\Database\DateTime; +use Utopia\Database\Change; use Utopia\Database\Document;
🧹 Nitpick comments (3)
src/Database/Adapter/Mongo.php (3)
1-1: Fix whitespace in blank line.PSR-12 style violation: remove whitespace from blank line.
2245-2245: Fix typo in comment.Remove the stray "p" character.
/** * Returns number of attributes used by default. - *p + * * @return int */
2364-2367: Implement execute method or document its purpose.The method always returns
truewithout executing anything. Consider adding a comment explaining why this is a no-op for MongoDB or throw an exception if it shouldn't be called.protected function execute(mixed $stmt): bool { + // MongoDB doesn't use prepared statements like SQL databases return true; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Database/Adapter/Mongo.php(1 hunks)
🧰 Additional context used
🪛 PHPMD (2.15.0)
src/Database/Adapter/Mongo.php
60-60: Avoid unused parameters such as '$event'. (Unused Code Rules)
(UnusedFormalParameter)
115-115: Avoid unused parameters such as '$name'. (Unused Code Rules)
(UnusedFormalParameter)
130-130: Avoid unused parameters such as '$database'. (Unused Code Rules)
(UnusedFormalParameter)
377-377: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
407-407: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
407-407: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$relatedCollection'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$twoWay'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$twoWayKey'. (Unused Code Rules)
(UnusedFormalParameter)
491-491: Avoid unused parameters such as '$side'. (Unused Code Rules)
(UnusedFormalParameter)
618-618: Avoid unused parameters such as '$lengths'. (Unused Code Rules)
(UnusedFormalParameter)
740-740: Avoid unused parameters such as '$forUpdate'. (Unused Code Rules)
(UnusedFormalParameter)
1310-1310: Avoid unused parameters such as '$permissionIds'. (Unused Code Rules)
(UnusedFormalParameter)
1350-1350: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
1350-1350: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
1350-1350: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
1350-1350: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
1572-1578: Avoid unused private methods such as 'timeToDocument'. (Unused Code Rules)
(UnusedPrivateMethod)
1588-1600: Avoid unused private methods such as 'timeToMongo'. (Unused Code Rules)
(UnusedPrivateMethod)
1950-1950: Avoid unused parameters such as '$prefix'. (Unused Code Rules)
(UnusedFormalParameter)
2283-2283: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2355-2355: Avoid unused parameters such as '$string'. (Unused Code Rules)
(UnusedFormalParameter)
2364-2364: Avoid unused parameters such as '$stmt'. (Unused Code Rules)
(UnusedFormalParameter)
2387-2387: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2392-2392: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2392-2392: Avoid unused parameters such as '$parentAlias'. (Unused Code Rules)
(UnusedFormalParameter)
🪛 GitHub Actions: Linter
src/Database/Adapter/Mongo.php
[error] 1-1: PHP CS Fixer (pint) PSR-12 style issue: 'no_whitespace_in_blank_line' violation.
🪛 GitHub Actions: CodeQL
src/Database/Adapter/Mongo.php
[error] 1092-1092: Parameter $changes of method Utopia\Database\Adapter\Mongo::createOrUpdateDocuments() has invalid type Utopia\Database\Adapter\Change.
[error] 1107-1107: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1160-1160: Method Utopia\Database\Adapter\Mongo::getSequences() invoked with 3 parameters, 2 required.
[error] 1163-1163: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1164-1164: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1164-1164: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1173-1173: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1184-1184: PHPDoc tag @param references unknown parameter: $documentIds.
[error] 1184-1184: PHPDoc tag @param references unknown parameter: $documentTenants.
[error] 1184-1184: Return type (array<string, string>) of method Utopia\Database\Adapter\Mongo::getSequences() should be compatible with return type (array<Utopia\Database\Document>) of method Utopia\Database\Adapter::getSequences().
[error] 1200-1200: Method Utopia\Database\Adapter\Mongo::getSequences() should return array<string, string> but returns array<Utopia\Database\Document>.
[error] 1231-1231: Method Utopia\Database\Adapter\Mongo::getSequences() should return array<string, string> but returns array<Utopia\Database\Document>.
[error] 1310-1310: PHPDoc tag @param references unknown parameter: $ids.
[error] 1542-1542: Method Utopia\Database\Adapter\Mongo::timeFilter() is unused.
[error] 1572-1572: Method Utopia\Database\Adapter\Mongo::timeToDocument() is unused.
[error] 1588-1588: Method Utopia\Database\Adapter\Mongo::timeToMongo() is unused.
[error] 2394-2394: Method Utopia\Database\Adapter\Mongo::getTenantQuery() should return string but returns int|null.
| case Database::VAR_DATETIME : | ||
| $node = DateTime::format($node->toDateTime()); | ||
| break; |
There was a problem hiding this comment.
Fix DateTime conversion logic.
The code attempts to call toDateTime() on $node, but based on the castingBefore method, $node should already be a UTCDateTime object after retrieval from MongoDB.
case Database::VAR_DATETIME :
- $node = DateTime::format($node->toDateTime());
+ if ($node instanceof UTCDateTime) {
+ $node = DateTime::format($node->toDateTime());
+ } elseif ($node instanceof \DateTime) {
+ $node = DateTime::format($node);
+ }
break;📝 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.
| case Database::VAR_DATETIME : | |
| $node = DateTime::format($node->toDateTime()); | |
| break; | |
| case Database::VAR_DATETIME : | |
| if ($node instanceof UTCDateTime) { | |
| $node = DateTime::format($node->toDateTime()); | |
| } elseif ($node instanceof \DateTime) { | |
| $node = DateTime::format($node); | |
| } | |
| break; |
🤖 Prompt for AI Agents
In src/Database/Adapter/Mongo.php around lines 850 to 852, the code incorrectly
calls toDateTime() on $node, assuming it is not already a UTCDateTime object.
Since $node should already be a UTCDateTime instance, remove the toDateTime()
call and directly format $node using DateTime::format after converting $node to
a DateTime object properly.
…uments for sequences without existing IDs, and update index length checks in tests to align with maximum index length constraints.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (23)
src/Database/Adapter/Mongo.php (23)
44-44: Uncomment the timeout property declaration.The
setTimeout()method on line 67 assigns to$this->timeout, but the property is commented out.
76-90: Transaction methods should throw exceptions if not supported.The transaction methods always return
truewithout implementing actual transaction logic. This could mislead callers into thinking transactions are working when they're not.
147-147: Fix incorrect database existence check.The check
$this->getClient()->selectDatabase() != nulldoesn't verify database existence. MongoDB always returns a database object even for non-existent databases.
266-288: Fix array index bug in index creation loop.The code uses
$ito access the$ordersarray inside the attributes loop, but$iis from the outer indexes loop. This will cause incorrect order assignments.
377-380: Throw exception for unsupported operation instead of returning false.The
analyzeCollectionmethod returnsfalsewithout explanation. This could mislead callers into thinking the operation failed rather than being unsupported.
424-432: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.
446-453: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.
513-540: Fix incorrect parameter syntax for update operations.Multiple update calls use incorrect
multiparameter syntax.
570-570: Fix junction table naming inconsistency.The junction table name construction is inconsistent with the
updateRelationshipmethod. It should use document sequences, not collection names.
576-594: Fix incorrect parameter syntax for update operations.
605-619: Fix method signature to match parent interface.The parameter name should be
$indexAttributeTypesinstead of$collationto match the abstract method signature.
850-852: Fix DateTime conversion logic.The code attempts to call
toDateTime()on$node, but based on the castingBefore method,$nodeshould already be aUTCDateTimeobject after retrieval from MongoDB.
905-913: Add error handling for DateTime creation.Creating DateTime from string without validation could throw exceptions on invalid date formats.
985-985: Remove or rename unused variable.The variable
$blais assigned but never used, and has a non-descriptive name.
1078-1078: Fix incorrect parameter syntax for update operation.
1086-1092: Fix incorrect type hint for $changes parameter.The parameter type should be
array<Change>notarray<Change>(the import is missing).
1183-1191: Fix PHPDoc to match actual parameters and return type.The PHPDoc references wrong parameter names and incorrect return type.
1308-1317: Fix PHPDoc parameter name.The PHPDoc references
$idsbut the parameter is named$sequences.
1417-1420: Security consideration for permission regex.The permission filtering uses regex with user-provided roles. While the roles are escaped, consider the performance impact and potential ReDoS attacks with complex permission strings.
1615-1618: Add error handling for DateTime creation.Creating DateTime from string without validation could throw exceptions on invalid date formats.
1792-1795: Validate ObjectId format before creation.Creating ObjectId without validation could throw exceptions if the sequence is not a valid 24-character hex string.
2361-2364: Implement quote method or throw exception.The
quote()method returns an empty string, which could cause issues if called. It should either be properly implemented or throw an exception.
2398-2401: Fix return type mismatch.The method is declared to return
stringbut returnsint|nullfromgetTenant().
🧹 Nitpick comments (1)
src/Database/Adapter/Mongo.php (1)
2404-2404: Add newline at end of file.Add a newline character at the end of the file to comply with PSR-12 coding standards.
} -} +} +
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Database/Adapter/Mongo.php(1 hunks)src/Database/Database.php(14 hunks)tests/e2e/Adapter/Scopes/IndexTests.php(2 hunks)tests/e2e/Adapter/Scopes/PermissionTests.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Database/Database.php
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/e2e/Adapter/Scopes/PermissionTests.php (2)
src/Database/Database.php (1)
getAdapter(1077-1080)src/Database/Adapter/Mongo.php (1)
getSupportForRelationships(2138-2141)
src/Database/Adapter/Mongo.php (9)
src/Database/Database.php (42)
Database(36-6864)__construct(384-461)setTimeout(766-771)clearTimeout(779-782)ping(1100-1103)reconnect(1105-1108)create(1119-1138)exists(1149-1154)list(1161-1168)delete(1177-1191)createCollection(1206-1324)deleteCollection(1485-1528)analyzeCollection(1472-1475)createAttribute(1554-1605)createAttributes(1621-1713)deleteAttribute(2268-2336)renameAttribute(2351-2415)createRelationship(2435-2612)updateRelationship(2627-2814)getDocument(3187-3310)deleteRelationship(2828-2942)createIndex(3017-3135)renameIndex(2958-2996)deleteIndex(3149-3174)getTenant(930-933)find(6001-6127)createDocument(3574-3664)createDocuments(3679-3770)updateDocument(4107-4296)updateDocuments(4319-4490)createOrUpdateDocuments(4895-4908)count(6224-6259)increaseDocumentAttribute(5145-5226)deleteDocument(5340-5390)deleteDocuments(5789-5940)updateAttribute(2074-2222)sum(6274-6302)getLimitForAttributes(6638-6645)getLimitForIndexes(6652-6655)getKeywords(1067-1070)getConnectionId(537-540)getSchemaAttributes(6727-6730)src/Database/Adapter.php (85)
Adapter(11-1256)setTimeout(304-304)getSupportForTimeouts(957-957)clearTimeout(317-321)startTransaction(331-331)commitTransaction(343-343)rollbackTransaction(354-354)ping(454-454)reconnect(459-459)create(468-468)exists(479-479)list(486-486)delete(495-495)createCollection(505-505)filter(1119-1128)deleteCollection(514-514)analyzeCollection(522-522)createAttribute(537-537)createAttributes(548-548)deleteAttribute(573-573)renameAttribute(583-583)createRelationship(594-594)updateRelationship(610-610)getDocument(670-670)deleteRelationship(624-624)createIndex(649-649)renameIndex(634-634)deleteIndex(659-659)getTenant(211-214)getAttributeProjection(1087-1087)find(781-781)createDocument(680-680)getSupportForInternalCasting(1246-1246)createDocuments(692-692)updateDocument(703-703)updateDocuments(718-718)createOrUpdateDocuments(730-734)getSequences(741-741)count(804-804)increaseDocumentAttribute(1170-1178)deleteDocument(751-751)deleteDocuments(762-762)updateAttribute(563-563)sum(793-793)getLimitForString(829-829)getLimitForInt(836-836)getLimitForAttributes(843-843)getLimitForIndexes(850-850)getMinDateTime(862-862)getSupportForSchemas(879-879)getSupportForIndex(900-900)getSupportForIndexArray(907-907)isMongo(1239-1239)setUTCDatetime(1254-1254)getSupportForAttributes(886-886)getSupportForUniqueIndex(921-921)getSupportForFulltextIndex(928-928)getSupportForFulltextWildcardIndex(935-935)getSupportForQueryContains(950-950)getSupportForRelationships(964-964)getSupportForUpdateLock(966-966)getSupportForAttributeResizing(980-980)getSupportForBatchOperations(973-973)getSupportForGetConnectionId(987-987)getSupportForCacheSkipOnFailure(1001-1001)getSupportForHostname(1015-1015)getSupportForSchemaAttributes(893-893)getSupportForCastIndexArray(914-914)getSupportForUpserts(994-994)getSupportForReconnection(1008-1008)getSupportForBatchCreateAttributes(1022-1022)getCountOfAttributes(1030-1030)getCountOfDefaultAttributes(1045-1045)getCountOfIndexes(1038-1038)getCountOfDefaultIndexes(1052-1052)getDocumentSizeLimit(1060-1060)getAttributeWidth(1071-1071)getKeywords(1078-1078)quote(447-447)execute(1216-1216)getMaxIndexLength(855-855)getConnectionId(1185-1185)getInternalIndexesKeys(1192-1192)getSchemaAttributes(1201-1201)getTenantQuery(1210-1210)src/Database/DateTime.php (2)
DateTime(7-86)format(29-32)src/Database/Document.php (8)
Document(12-462)__construct(28-56)getAttribute(214-221)getSequence(69-72)getTenant(170-177)find(296-314)isEmpty(388-391)getArrayCopy(415-450)src/Database/Exception/Timeout.php (1)
Timeout(7-9)src/Database/Query.php (9)
Query(8-730)__construct(89-98)getAttribute(120-123)equal(355-358)limit(496-499)offset(507-510)groupByType(619-698)isNested(705-712)onArray(717-720)src/Database/Validator/Authorization.php (3)
Authorization(7-225)__construct(29-32)getRoles(101-104)src/Database/Validator/Queries/Documents.php (1)
__construct(25-72)src/Database/Change.php (1)
getNew(23-26)
🪛 GitHub Actions: Linter
tests/e2e/Adapter/Scopes/IndexTests.php
[error] 1-1: Laravel Pint PSR-12 style check failed: no_whitespace_in_blank_line.
tests/e2e/Adapter/Scopes/PermissionTests.php
[error] 1-1: Laravel Pint PSR-12 style check failed: no_whitespace_in_blank_line.
src/Database/Adapter/Mongo.php
[error] 1-1: Laravel Pint PSR-12 style check failed: no_whitespace_in_blank_line.
🪛 PHPMD (2.15.0)
src/Database/Adapter/Mongo.php
60-60: Avoid unused parameters such as '$event'. (Unused Code Rules)
(UnusedFormalParameter)
115-115: Avoid unused parameters such as '$name'. (Unused Code Rules)
(UnusedFormalParameter)
130-130: Avoid unused parameters such as '$database'. (Unused Code Rules)
(UnusedFormalParameter)
377-377: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
394-394: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
407-407: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
407-407: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$relatedCollection'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$twoWay'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
465-465: Avoid unused parameters such as '$twoWayKey'. (Unused Code Rules)
(UnusedFormalParameter)
491-491: Avoid unused parameters such as '$side'. (Unused Code Rules)
(UnusedFormalParameter)
618-618: Avoid unused parameters such as '$lengths'. (Unused Code Rules)
(UnusedFormalParameter)
740-740: Avoid unused parameters such as '$forUpdate'. (Unused Code Rules)
(UnusedFormalParameter)
1316-1316: Avoid unused parameters such as '$permissionIds'. (Unused Code Rules)
(UnusedFormalParameter)
1356-1356: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
1356-1356: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
1356-1356: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
1356-1356: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
1578-1584: Avoid unused private methods such as 'timeToDocument'. (Unused Code Rules)
(UnusedPrivateMethod)
1594-1606: Avoid unused private methods such as 'timeToMongo'. (Unused Code Rules)
(UnusedPrivateMethod)
1956-1956: Avoid unused parameters such as '$prefix'. (Unused Code Rules)
(UnusedFormalParameter)
2289-2289: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2361-2361: Avoid unused parameters such as '$string'. (Unused Code Rules)
(UnusedFormalParameter)
2370-2370: Avoid unused parameters such as '$stmt'. (Unused Code Rules)
(UnusedFormalParameter)
2393-2393: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2398-2398: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2398-2398: Avoid unused parameters such as '$parentAlias'. (Unused Code Rules)
(UnusedFormalParameter)
🪛 GitHub Actions: CodeQL
src/Database/Adapter/Mongo.php
[error] 1092-1092: PHPStan: Parameter $changes of method Utopia\Database\Adapter\Mongo::createOrUpdateDocuments() has invalid type Utopia\Database\Adapter\Change.
[error] 1107-1107: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1162-1162: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1163-1163: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1170-1170: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1171-1171: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1171-1171: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1180-1180: PHPStan: Call to method getNew() on an unknown class Utopia\Database\Adapter\Change.
[error] 1191-1191: PHPStan: PHPDoc tag @param references unknown parameter: $documentIds.
[error] 1191-1191: PHPStan: PHPDoc tag @param references unknown parameter: $documentTenants.
[error] 1191-1191: PHPStan: Return type (array<string, string>) of method Utopia\Database\Adapter\Mongo::getSequences() should be compatible with return type (array<Utopia\Database\Document>) of method Utopia\Database\Adapter::getSequences().
[error] 1206-1206: PHPStan: Method Utopia\Database\Adapter\Mongo::getSequences() should return array<string, string> but returns array<Utopia\Database\Document>.
[error] 1237-1237: PHPStan: Method Utopia\Database\Adapter\Mongo::getSequences() should return array<string, string> but returns array<Utopia\Database\Document>.
[error] 1316-1316: PHPStan: PHPDoc tag @param references unknown parameter: $ids.
[error] 1548-1548: PHPStan: Method Utopia\Database\Adapter\Mongo::timeFilter() is unused.
[error] 1578-1578: PHPStan: Method Utopia\Database\Adapter\Mongo::timeToDocument() is unused.
[error] 1594-1594: PHPStan: Method Utopia\Database\Adapter\Mongo::timeToMongo() is unused.
[error] 2400-2400: PHPStan: Method Utopia\Database\Adapter\Mongo::getTenantQuery() should return string but returns int|null.
🔇 Additional comments (4)
tests/e2e/Adapter/Scopes/IndexTests.php (2)
308-308: LGTM! Improved adapter compatibility.The dynamic calculation of attribute size based on the adapter's maximum index length makes this test more robust and compatible with different database adapters, particularly the newly introduced MongoDB adapter.
322-322: LGTM! Consistent with the previous improvement.This change maintains consistency with the dynamic size calculation approach, ensuring both attribute creation and update scenarios are tested with adapter-appropriate values.
tests/e2e/Adapter/Scopes/PermissionTests.php (2)
758-761: LGTM! Proper adapter capability checking.The early exit condition based on relationship support prevents relationship-dependent tests from running on adapters that don't support this feature, ensuring compatibility with the MongoDB adapter.
840-843: LGTM! Consistent relationship support handling.The implementation correctly checks for relationship support and returns an empty array when not supported, maintaining compatibility with dependent test methods that expect an array return value.
| @@ -0,0 +1,2403 @@ | |||
| <?php | |||
There was a problem hiding this comment.
Fix whitespace issue.
Remove trailing whitespace on line 1 to comply with PSR-12 coding standards.
🧰 Tools
🪛 GitHub Actions: Linter
[error] 1-1: Laravel Pint PSR-12 style check failed: no_whitespace_in_blank_line.
🤖 Prompt for AI Agents
In src/Database/Adapter/Mongo.php at line 1, remove any trailing whitespace
after the opening PHP tag to comply with PSR-12 coding standards. Ensure the
line contains only the "<?php" tag without extra spaces or tabs following it.
…files to improve code readability and maintainability.
…adapter for tenant compatibility
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (24)
src/Database/Adapter/Mongo.php (24)
1-1: Remove trailing whitespace
45-45: Uncomment the timeout property declarationThe
setTimeout()method on line 67 assigns to$this->timeout, but the property is commented out.
77-90: Transaction methods should throw exceptions if not supportedThe transaction methods always return
truewithout implementing actual transaction logic. This could mislead callers into thinking transactions are working when they're not.
148-148: Fix incorrect database existence checkThe check
$this->getClient()->selectDatabase() != nulldoesn't verify database existence. MongoDB always returns a database object even for non-existent databases.
267-290: Fix array index bug in index creation loopThe code uses
$ito access the$ordersarray inside the attributes loop, but$iis from the outer indexes loop. This will cause incorrect order assignments.
378-381: Throw exception for unsupported operation instead of returning falseThe
analyzeCollectionmethod returnsfalsewithout explanation. This could mislead callers into thinking the operation failed rather than being unsupported.
425-433: Fix incorrect parameter syntax for update operationThe
multiparameter should be passed in the options array.
447-454: Fix incorrect parameter syntax for update operationThe
multiparameter should be passed in the options array.
514-541: Fix incorrect parameter syntax for update operationsMultiple update calls use incorrect
multiparameter syntax.
571-571: Fix junction table naming inconsistencyThe junction table name construction is inconsistent with the
updateRelationshipmethod. It should use document sequences, not collection names.
577-595: Fix incorrect parameter syntax for update operations
606-620: Fix method signature to match parent interfaceThe parameter name should be
$indexAttributeTypesinstead of$collationto match the abstract method signature.
986-986: Remove unused variable assignment
1079-1079: Fix incorrect parameter syntax for update operation
1087-1093: Fix incorrect type hint for $changes parameterThe parameter type should be
array<Change>notarray<Change>(the import is missing).
1184-1190: Fix PHPDoc to match actual parameters and return type
1309-1316: Fix PHPDoc parameter name
1615-1618: Add error handling for DateTime creationCreating DateTime from string without validation could throw exceptions on invalid date formats.
1792-1795: Validate ObjectId format before creationCreating ObjectId without validation could throw exceptions if the sequence is not a valid 24-character hex string.
906-910: Add error handling for DateTime creationCreating DateTime from string without validation could throw exceptions on invalid date formats.
2361-2364: Implement quote method or throw exceptionThe
quote()method returns an empty string, which could cause issues if called. It should either be properly implemented or throw an exception.
2398-2402: Fix return type mismatchThe method is declared to return
stringbut returnsint|nullfromgetTenant().
1417-1420: Escape dynamic role values in regex to prevent injectionThe current code builds a regex from raw role names without escaping, which could allow regex injection and ReDoS attacks.
if (Authorization::$status) { - $roles = \implode('|', Authorization::getRoles()); + $escapedRoles = \array_map(fn($r) => \preg_quote($r, '/'), Authorization::getRoles()); + $roles = \implode('|', $escapedRoles); $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')]; }
1708-1711: Escape role values in permissions regexSame regex injection vulnerability. Apply role escaping here as well.
if (Authorization::$status) { // skip if authorization is disabled - $roles = \implode('|', Authorization::getRoles()); + $escapedRoles = \array_map(fn($r) => \preg_quote($r, '/'), Authorization::getRoles()); + $roles = \implode('|', $escapedRoles); $filters['_permissions']['$in'] = [new Regex("read\(\".*(?:{$roles}).*\"\)", 'i')]; }
🧹 Nitpick comments (6)
src/Database/Adapter/Mongo.php (6)
851-853: Verify DateTime conversion logicThe code assumes
$nodeis aUTCDateTimeobject and callstoDateTime()on it. Consider adding type checking to handle edge cases gracefully.case Database::VAR_DATETIME : - $node = DateTime::format($node->toDateTime()); + if ($node instanceof UTCDateTime) { + $node = DateTime::format($node->toDateTime()); + } break;
946-946: Fix grammatical error in exception message- throw new DatabaseException('All documents must have an sequence if one is set'); + throw new DatabaseException('All documents must have a sequence if one is set');
1057-1057: Remove empty statement- ; $name = $this->getNamespace() . '_' . $this->filter($collection);
1548-1569: Remove unused timeFilter methodPHPStan reports this method as unused. If it's not needed, consider removing it to reduce code complexity.
1578-1584: Remove unused timeToDocument methodBoth PHPMD and PHPStan report this method as unused. Consider removing it if not needed.
1594-1606: Remove unused timeToMongo methodBoth PHPMD and PHPStan report this method as unused. Consider removing it if not needed.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Database/Adapter/Mongo.php(1 hunks)src/Database/Adapter/Pool.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/Database/Adapter/Pool.php (5)
src/Database/Adapter/Mongo.php (5)
castingBefore(872-920)castingAfter(814-863)getSupportForInternalCasting(2061-2064)isMongo(2067-2070)setUTCDatetime(2072-2075)src/Database/Adapter.php (5)
castingBefore(1224-1224)castingAfter(1232-1232)getSupportForInternalCasting(1246-1246)isMongo(1239-1239)setUTCDatetime(1254-1254)src/Database/Adapter/SQL.php (5)
castingBefore(843-846)castingAfter(854-857)getSupportForInternalCasting(832-835)isMongo(864-867)setUTCDatetime(875-878)src/Database/Document.php (1)
Document(12-462)src/Database/Mirror.php (1)
delegate(88-103)
src/Database/Adapter/Mongo.php (9)
src/Database/Database.php (42)
Database(36-6864)__construct(384-461)setTimeout(766-771)clearTimeout(779-782)ping(1100-1103)reconnect(1105-1108)create(1119-1138)exists(1149-1154)list(1161-1168)delete(1177-1191)createCollection(1206-1324)getSizeOfCollectionOnDisk(1447-1464)getSizeOfCollection(1425-1438)deleteCollection(1485-1528)analyzeCollection(1472-1475)createAttribute(1554-1605)createAttributes(1621-1713)deleteAttribute(2268-2336)renameAttribute(2351-2415)createRelationship(2435-2612)updateRelationship(2627-2814)getDocument(3187-3310)deleteRelationship(2828-2942)createIndex(3017-3135)renameIndex(2958-2996)deleteIndex(3149-3174)getTenant(930-933)find(6001-6127)createDocument(3574-3664)createDocuments(3679-3770)updateDocument(4107-4296)updateDocuments(4319-4490)createOrUpdateDocuments(4895-4908)count(6224-6259)increaseDocumentAttribute(5145-5226)deleteDocument(5340-5390)deleteDocuments(5789-5940)updateAttribute(2074-2222)sum(6274-6302)getLimitForAttributes(6638-6645)getLimitForIndexes(6652-6655)getKeywords(1067-1070)src/Database/Adapter.php (85)
Adapter(11-1256)setTimeout(304-304)getSupportForTimeouts(957-957)clearTimeout(317-321)startTransaction(331-331)commitTransaction(343-343)rollbackTransaction(354-354)ping(454-454)reconnect(459-459)create(468-468)exists(479-479)list(486-486)delete(495-495)createCollection(505-505)filter(1119-1128)getSizeOfCollectionOnDisk(822-822)getSizeOfCollection(813-813)deleteCollection(514-514)analyzeCollection(522-522)createAttribute(537-537)createAttributes(548-548)deleteAttribute(573-573)renameAttribute(583-583)createRelationship(594-594)updateRelationship(610-610)getDocument(670-670)deleteRelationship(624-624)createIndex(649-649)renameIndex(634-634)deleteIndex(659-659)getTenant(211-214)find(781-781)createDocument(680-680)castingAfter(1232-1232)castingBefore(1224-1224)createDocuments(692-692)updateDocument(703-703)updateDocuments(718-718)createOrUpdateDocuments(730-734)getSequences(741-741)count(804-804)increaseDocumentAttribute(1170-1178)deleteDocument(751-751)deleteDocuments(762-762)updateAttribute(563-563)sum(793-793)escapeWildcards(1130-1155)getLimitForString(829-829)getLimitForInt(836-836)getLimitForAttributes(843-843)getLimitForIndexes(850-850)getMinDateTime(862-862)getSupportForSchemas(879-879)getSupportForIndex(900-900)getSupportForIndexArray(907-907)isMongo(1239-1239)setUTCDatetime(1254-1254)getSupportForAttributes(886-886)getSupportForUniqueIndex(921-921)getSupportForFulltextIndex(928-928)getSupportForFulltextWildcardIndex(935-935)getSupportForQueryContains(950-950)getSupportForRelationships(964-964)getSupportForUpdateLock(966-966)getSupportForAttributeResizing(980-980)getSupportForBatchOperations(973-973)getSupportForGetConnectionId(987-987)getSupportForCacheSkipOnFailure(1001-1001)getSupportForHostname(1015-1015)getSupportForSchemaAttributes(893-893)getSupportForCastIndexArray(914-914)getSupportForReconnection(1008-1008)getSupportForBatchCreateAttributes(1022-1022)getCountOfAttributes(1030-1030)getCountOfDefaultAttributes(1045-1045)getCountOfIndexes(1038-1038)getCountOfDefaultIndexes(1052-1052)getDocumentSizeLimit(1060-1060)getAttributeWidth(1071-1071)getSupportForCasting(943-943)getKeywords(1078-1078)quote(447-447)execute(1216-1216)getMaxIndexLength(855-855)getTenantQuery(1210-1210)src/Database/Exception.php (2)
Exception(7-21)__construct(9-20)src/Database/Change.php (2)
Change(5-32)getNew(23-26)src/Database/DateTime.php (2)
DateTime(7-86)format(29-32)src/Database/Document.php (13)
Document(12-462)__construct(28-56)getAttribute(214-221)getId(61-64)getSequence(69-72)getTenant(170-177)find(296-314)removeAttribute(277-285)setAttribute(234-251)isEmpty(388-391)getArrayCopy(415-450)getAttributes(184-202)getPermissions(85-88)src/Database/Exception/Timeout.php (1)
Timeout(7-9)src/Database/Query.php (14)
Query(8-730)__construct(89-98)getAttribute(120-123)setAttribute(161-166)toArray(310-333)equal(355-358)limit(496-499)offset(507-510)groupByType(619-698)isNested(705-712)getMethod(112-115)getValues(128-131)setValues(174-179)onArray(717-720)src/Database/Validator/Authorization.php (3)
Authorization(7-225)__construct(29-32)getRoles(101-104)
🪛 PHPMD (2.15.0)
src/Database/Adapter/Mongo.php
61-61: Avoid unused parameters such as '$event'. (Unused Code Rules)
(UnusedFormalParameter)
116-116: Avoid unused parameters such as '$name'. (Unused Code Rules)
(UnusedFormalParameter)
131-131: Avoid unused parameters such as '$database'. (Unused Code Rules)
(UnusedFormalParameter)
378-378: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
408-408: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
408-408: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$relatedCollection'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$twoWay'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$twoWayKey'. (Unused Code Rules)
(UnusedFormalParameter)
492-492: Avoid unused parameters such as '$side'. (Unused Code Rules)
(UnusedFormalParameter)
619-619: Avoid unused parameters such as '$lengths'. (Unused Code Rules)
(UnusedFormalParameter)
741-741: Avoid unused parameters such as '$forUpdate'. (Unused Code Rules)
(UnusedFormalParameter)
1316-1316: Avoid unused parameters such as '$permissionIds'. (Unused Code Rules)
(UnusedFormalParameter)
1356-1356: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
1356-1356: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
1356-1356: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
1356-1356: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
1578-1584: Avoid unused private methods such as 'timeToDocument'. (Unused Code Rules)
(UnusedPrivateMethod)
1594-1606: Avoid unused private methods such as 'timeToMongo'. (Unused Code Rules)
(UnusedPrivateMethod)
1956-1956: Avoid unused parameters such as '$prefix'. (Unused Code Rules)
(UnusedFormalParameter)
2289-2289: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2361-2361: Avoid unused parameters such as '$string'. (Unused Code Rules)
(UnusedFormalParameter)
2370-2370: Avoid unused parameters such as '$stmt'. (Unused Code Rules)
(UnusedFormalParameter)
2393-2393: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2398-2398: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2398-2398: Avoid unused parameters such as '$parentAlias'. (Unused Code Rules)
(UnusedFormalParameter)
🪛 GitHub Actions: CodeQL
src/Database/Adapter/Mongo.php
[error] 1548-1548: PHPStan: Method Utopia\Database\Adapter\Mongo::timeFilter() is unused.
[error] 1578-1578: PHPStan: Method Utopia\Database\Adapter\Mongo::timeToDocument() is unused.
[error] 1594-1594: PHPStan: Method Utopia\Database\Adapter\Mongo::timeToMongo() is unused.
⏰ 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). (8)
- GitHub Check: Adapter Tests (SharedTables/Postgres)
- GitHub Check: Adapter Tests (SharedTables/MariaDB)
- GitHub Check: Adapter Tests (SharedTables/SQLite)
- GitHub Check: Adapter Tests (SQLite)
- GitHub Check: Adapter Tests (Mirror)
- GitHub Check: Adapter Tests (MariaDB)
- GitHub Check: Adapter Tests (MySQL)
- GitHub Check: Unit Test
🔇 Additional comments (1)
src/Database/Adapter/Pool.php (1)
503-526: LGTM! Consistent implementation of new adapter methods.The five new methods (
castingBefore,castingAfter,getSupportForInternalCasting,isMongo, andsetUTCDatetime) correctly follow the established delegation pattern used throughout the Pool class. Each method properly delegates to the underlying adapter instance using thedelegatemethod with__FUNCTION__andfunc_get_args(), maintaining consistency with the existing codebase architecture.The method signatures align with the abstract declarations in the base
Adapterclass, ensuring proper interface compliance.
| if (Authorization::$status) { // skip if authorization is disabled | ||
| $roles = \implode('|', Authorization::getRoles()); | ||
| $filters['_permissions']['$in'] = [new Regex("read\(\".*(?:{$roles}).*\"\)", 'i')]; | ||
| } |
There was a problem hiding this comment.
Escape role values in permissions regex
Same regex injection vulnerability as in the find method. Apply the same escaping fix here.
if (Authorization::$status) { // skip if authorization is disabled
- $roles = \implode('|', Authorization::getRoles());
+ $escapedRoles = \array_map(fn($r) => \preg_quote($r, '/'), Authorization::getRoles());
+ $roles = \implode('|', $escapedRoles);
$filters['_permissions']['$in'] = [new Regex("read\(\".*(?:{$roles}).*\"\)", 'i')];
}📝 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 (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 | |
| $escapedRoles = \array_map(fn($r) => \preg_quote($r, '/'), Authorization::getRoles()); | |
| $roles = \implode('|', $escapedRoles); | |
| $filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')]; | |
| } |
🤖 Prompt for AI Agents
In src/Database/Adapter/Mongo.php around lines 1680 to 1683, the roles used in
the regex for permissions filtering are not escaped, causing a regex injection
vulnerability similar to the one in the find method. Fix this by properly
escaping each role value before including it in the regex pattern, ensuring
special characters in roles do not break the regex or introduce security issues.
| public function setUTCDatetime(string $value): mixed | ||
| { | ||
| return new UTCDateTime(new \DateTime($value)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for DateTime creation
public function setUTCDatetime(string $value): mixed
{
- return new UTCDateTime(new \DateTime($value));
+ try {
+ return new UTCDateTime(new \DateTime($value));
+ } catch (\Exception $e) {
+ throw new DatabaseException('Invalid datetime format: ' . $value);
+ }
}📝 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.
| public function setUTCDatetime(string $value): mixed | |
| { | |
| return new UTCDateTime(new \DateTime($value)); | |
| } | |
| public function setUTCDatetime(string $value): mixed | |
| { | |
| try { | |
| return new UTCDateTime(new \DateTime($value)); | |
| } catch (\Exception $e) { | |
| throw new DatabaseException('Invalid datetime format: ' . $value); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/Database/Adapter/Mongo.php around lines 2072 to 2075, the setUTCDatetime
method creates a new DateTime object without handling potential errors from
invalid date strings. Modify the method to catch exceptions thrown by the
DateTime constructor, handle the error gracefully (e.g., return null or throw a
custom exception), and ensure the method does not fail silently or cause
unhandled exceptions.
…ecks for UTCDateTime instances and removing unused time conversion methods, improving code clarity and performance.
…tential conflicts during development.
# Conflicts: # src/Database/Database.php
…t updates; change Redis cache to Memory cache as fallback in tests; update Docker port mapping.
…into feat-mongo-2
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
tests/e2e/Adapter/MongoDBTest.php (3)
42-48: Consider externalizing database connection parameters.The MongoDB connection parameters are hardcoded. Consider using environment variables or configuration files for flexibility across different test environments.
$client = new Client( $schema, - 'mongo', - 27017, - 'root', - 'password', + $_ENV['MONGO_HOST'] ?? 'mongo', + (int)($_ENV['MONGO_PORT'] ?? 27017), + $_ENV['MONGO_USER'] ?? 'root', + $_ENV['MONGO_PASSWORD'] ?? 'password', false );
77-95: Implement or document stub test methods.These test methods contain only trivial assertions and don't test any functionality. Either implement proper tests or add TODO comments explaining why these tests are skipped for MongoDB.
Would you like me to help implement these test methods or open an issue to track this task?
97-105: Address unused parameters in stub methods.The parameters in
deleteColumnanddeleteIndexare unused. Consider adding underscore prefix to indicate they're intentionally unused, or add a comment explaining why these operations always return true for MongoDB.- protected static function deleteColumn(string $collection, string $column): bool + protected static function deleteColumn(string $_collection, string $_column): bool { + // MongoDB doesn't support column deletion - always return true return true; } - protected static function deleteIndex(string $collection, string $index): bool + protected static function deleteIndex(string $_collection, string $_index): bool { + // Index deletion is handled differently in MongoDB tests - always return true return true; }src/Database/Adapter/Mongo.php (1)
2294-2301: Consider enhancing error handling for more MongoDB-specific errors.Currently only timeout errors are specifically handled. Consider adding cases for other common MongoDB errors like duplicate key violations, connection errors, etc.
protected function processException(Exception $e): \Exception { if ($e->getCode() === 50) { return new Timeout('Query timed out', $e->getCode(), $e); } + + // Handle duplicate key errors + if ($e->getCode() === 11000) { + return new Duplicate('Duplicate key error', $e->getCode(), $e); + } + + // Add more MongoDB-specific error handling as needed return $e; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docker-compose.yml(2 hunks)src/Database/Adapter/Mongo.php(1 hunks)src/Database/Database.php(14 hunks)tests/e2e/Adapter/MongoDBTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docker-compose.yml
- src/Database/Database.php
🧰 Additional context used
🪛 PHPMD (2.15.0)
src/Database/Adapter/Mongo.php
61-61: Avoid unused parameters such as '$event'. (Unused Code Rules)
(UnusedFormalParameter)
116-116: Avoid unused parameters such as '$name'. (Unused Code Rules)
(UnusedFormalParameter)
131-131: Avoid unused parameters such as '$database'. (Unused Code Rules)
(UnusedFormalParameter)
378-378: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
408-408: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
408-408: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$relatedCollection'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$twoWay'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$twoWayKey'. (Unused Code Rules)
(UnusedFormalParameter)
492-492: Avoid unused parameters such as '$side'. (Unused Code Rules)
(UnusedFormalParameter)
619-619: Avoid unused parameters such as '$lengths'. (Unused Code Rules)
(UnusedFormalParameter)
741-741: Avoid unused parameters such as '$forUpdate'. (Unused Code Rules)
(UnusedFormalParameter)
1325-1325: Avoid unused parameters such as '$permissionIds'. (Unused Code Rules)
(UnusedFormalParameter)
1365-1365: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
1365-1365: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
1365-1365: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
1365-1365: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
1898-1898: Avoid unused parameters such as '$prefix'. (Unused Code Rules)
(UnusedFormalParameter)
2231-2231: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2303-2303: Avoid unused parameters such as '$string'. (Unused Code Rules)
(UnusedFormalParameter)
2312-2312: Avoid unused parameters such as '$stmt'. (Unused Code Rules)
(UnusedFormalParameter)
2335-2335: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2340-2340: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2340-2340: Avoid unused parameters such as '$parentAlias'. (Unused Code Rules)
(UnusedFormalParameter)
tests/e2e/Adapter/MongoDBTest.php
97-97: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
97-97: Avoid unused parameters such as '$column'. (Unused Code Rules)
(UnusedFormalParameter)
102-102: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
102-102: Avoid unused parameters such as '$index'. (Unused Code Rules)
(UnusedFormalParameter)
🪛 GitHub Actions: Linter
tests/e2e/Adapter/MongoDBTest.php
[error] 1-1: PHP CS Fixer (pint) PSR-12 style check failed: 'no_unused_imports' rule violation.
🔇 Additional comments (4)
tests/e2e/Adapter/MongoDBTest.php (2)
23-26: LGTM!The adapter name method is correctly implemented.
68-75: LGTM!The test correctly handles MongoDB's dynamic database creation behavior by removing the exists check.
src/Database/Adapter/Mongo.php (2)
936-976: Well-implemented batch document creation.Good practices observed:
- Cloning documents to avoid side effects
- Validating consistent sequence usage across all documents
- Proper batch insertion with error handling
2252-2266: LGTM!The
flattenArraymethod is well-implemented with proper recursion and base case handling.
| public function reconnect(): void | ||
| { | ||
| $this->client->connect(); | ||
| } |
There was a problem hiding this comment.
Inconsistent reconnection support declaration.
The adapter implements a reconnect() method (line 104) but getSupportForReconnection() returns false (line 2155). This is contradictory - either remove the reconnect method or return true for reconnection support.
public function getSupportForReconnection(): bool
{
- return false;
+ return true;
}Also applies to: 2155-2158
🤖 Prompt for AI Agents
In src/Database/Adapter/Mongo.php around lines 104 to 107 and 2155 to 2158,
there is a contradiction where the reconnect() method is implemented but
getSupportForReconnection() returns false. To fix this, either remove the
reconnect() method if reconnection is not supported, or update
getSupportForReconnection() to return true to indicate that reconnection is
supported, ensuring consistency between the method implementation and the
support declaration.
| */ | ||
| public function updateDocuments(string $collection, Document $updates, array $documents): int | ||
| { | ||
| ; |
There was a problem hiding this comment.
Remove unnecessary semicolon.
There's an empty statement (standalone semicolon) at the beginning of the method.
public function updateDocuments(string $collection, Document $updates, array $documents): int
{
- ;
$name = $this->getNamespace() . '_' . $this->filter($collection);📝 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.
| ; | |
| public function updateDocuments(string $collection, Document $updates, array $documents): int | |
| { | |
| $name = $this->getNamespace() . '_' . $this->filter($collection); | |
| // … | |
| } |
🤖 Prompt for AI Agents
In src/Database/Adapter/Mongo.php at line 1066, there is an unnecessary
standalone semicolon at the beginning of the method. Remove this empty statement
to clean up the code and avoid redundant syntax.
|
|
||
| namespace Tests\E2E\Adapter; | ||
|
|
||
| use Exception; |
There was a problem hiding this comment.
Remove unused imports.
The Exception and NoCache imports are not used in this file.
-use Exception;
use Utopia\Cache\Adapter\Memory;
-use Utopia\Cache\Adapter\None as NoCache;
use Utopia\Cache\Cache;Also applies to: 7-7
🤖 Prompt for AI Agents
In tests/e2e/Adapter/MongoDBTest.php at lines 5 and 7, the imports for Exception
and NoCache are not used anywhere in the file. Remove these unused import
statements to clean up the code and avoid unnecessary dependencies.
…ns handling; update Docker configuration for client mapping; enhance tests with Redis cache integration.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (28)
tests/e2e/Adapter/MongoDBTest.php (1)
5-5: Remove unused imports.The
ExceptionandNoCacheimports are not used in this file.Also applies to: 8-8
src/Database/Database.php (1)
3739-3741: Still losing the casted instance inside the documents loop.
$documentis modified (including$this->adapter->castingBefore) but, because the loop iterates by value, the mutated object is never written back to$documents, so the adapter may receive stale data.-foreach ($documents as $document) { +foreach ($documents as $idx => $document) { ... - $document = $this->adapter->castingBefore($collection, $document); + $document = $this->adapter->castingBefore($collection, $document); + $documents[$idx] = $document; // keep array in sync }src/Database/Adapter/Mongo.php (26)
1066-1066: Remove unnecessary semicolon.There's an empty statement (standalone semicolon) at the beginning of the method.
public function updateDocuments(string $collection, Document $updates, array $documents): int { - ; $name = $this->getNamespace() . '_' . $this->filter($collection);
1-1: Fix trailing whitespace.Remove the trailing whitespace after the opening PHP tag to comply with PSR-12 coding standards.
45-45: Uncomment the timeout property declaration.The
setTimeout()method on line 67 assigns to$this->timeout, but the property is commented out. This will cause a runtime error.-//protected ?int $timeout = null; +protected ?int $timeout = null;
77-90: Transaction methods should throw exceptions if not supported.The transaction methods always return
truewithout implementing actual transaction logic. This could mislead callers into thinking transactions are working when they're not.Consider throwing an exception to make it clear that transactions aren't supported:
public function startTransaction(): bool { - return true; + throw new DatabaseException('Transactions are not supported in MongoDB adapter'); } public function commitTransaction(): bool { - return true; + throw new DatabaseException('Transactions are not supported in MongoDB adapter'); } public function rollbackTransaction(): bool { - return true; + throw new DatabaseException('Transactions are not supported in MongoDB adapter'); }Alternatively, if MongoDB transactions are planned for future implementation, add a TODO comment.
104-107: Inconsistent reconnection support declaration.The adapter implements a
reconnect()method butgetSupportForReconnection()returns false. This is contradictory - either remove the reconnect method or return true for reconnection support.public function getSupportForReconnection(): bool { - return false; + return true; }Also applies to: 2139-2142
148-148: Fix incorrect database existence check.The check
$this->getClient()->selectDatabase() != nulldoesn't verify database existence. MongoDB always returns a database object even for non-existent databases.To properly check database existence, query the list of databases:
- return $this->getClient()->selectDatabase() != null; + $databases = $this->getClient()->listDatabaseNames(); + return in_array($database, $databases);
267-289: Fix array index bug in index creation loop.The code uses
$ito access the$ordersarray inside the attributes loop, but$iis from the outer indexes loop. This will cause incorrect order assignments.- foreach ($attributes as $attribute) { + foreach ($attributes as $j => $attribute) { $attribute = $this->filter($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;
378-381: Throw exception for unsupported operation instead of returning false.The
analyzeCollectionmethod returnsfalsewithout explanation. This could mislead callers into thinking the operation failed rather than being unsupported.Throw an exception to clearly indicate this operation is not supported:
public function analyzeCollection(string $collection): bool { - return false; + throw new DatabaseException('Analyze collection is not supported in MongoDB adapter'); }
425-433: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.$this->getClient()->update( $collection, [], ['$unset' => [$id => '']], - multi: true + ['multi' => true] );
447-454: Fix incorrect parameter syntax for update operation.The
multiparameter should be passed in the options array.$this->getClient()->update( $collection, [], ['$rename' => [$id => $name]], - multi: true + ['multi' => true] );
514-541: Fix incorrect parameter syntax for update operations.Multiple update calls use incorrect
multiparameter syntax.- $this->getClient()->update($collection, updates: $renameKey, multi: true); + $this->getClient()->update($collection, [], $renameKey, ['multi' => true]); } if ($twoWay && !\is_null($newTwoWayKey)) { - $this->getClient()->update($relatedCollection, updates: $renameTwoWayKey, multi: true); + $this->getClient()->update($relatedCollection, [], $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($relatedCollection, [], $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($collection, [], $renameKey, ['multi' => true]); } break; case Database::RELATION_MANY_TO_MANY: $collection = $this->getDocument(Database::METADATA, $collection); $relatedCollection = $this->getDocument(Database::METADATA, $relatedCollection); $junction = $this->getNamespace() . '_' . $this->filter('_' . $collection->getSequence() . '_' . $relatedCollection->getSequence()); if (!\is_null($newKey)) { - $this->getClient()->update($junction, updates: $renameKey, multi: true); + $this->getClient()->update($junction, [], $renameKey, ['multi' => true]); } if ($twoWay && !\is_null($newTwoWayKey)) { - $this->getClient()->update($junction, updates: $renameTwoWayKey, multi: true); + $this->getClient()->update($junction, [], $renameTwoWayKey, ['multi' => true]); }
571-571: Fix junction table naming inconsistency.The junction table name construction is inconsistent with the
updateRelationshipmethod. It should use document sequences, not collection names.- $junction = $this->getNamespace() . '_' . $this->filter('_' . $collection . '_' . $relatedCollection); + $collectionDoc = $this->getDocument(Database::METADATA, $collection); + $relatedCollectionDoc = $this->getDocument(Database::METADATA, $relatedCollection); + $junction = $this->getNamespace() . '_' . $this->filter('_' . $collectionDoc->getSequence() . '_' . $relatedCollectionDoc->getSequence());
577-595: Fix incorrect parameter syntax for update operations.- $this->getClient()->update($collection, [], ['$unset' => [$key => '']], multi: true); + $this->getClient()->update($collection, [], ['$unset' => [$key => '']], ['multi' => true]); if ($twoWay) { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$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' => [$key => '']], ['multi' => true]); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$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' => [$key => '']], ['multi' => true]); } else { - $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], multi: true); + $this->getClient()->update($relatedCollection, [], ['$unset' => [$twoWayKey => '']], ['multi' => true]); }
606-620: Fix method signature to match parent interface.The parameter name should be
$indexAttributeTypesinstead of$collationto match the abstract method signature.public function createIndex( string $collection, string $id, string $type, array $attributes, array $lengths, array $orders, - array $collation = [] + array $indexAttributeTypes = [] ): bool
850-863: Fix DateTime conversion logic.The code attempts to call
toDateTime()on$node, but based on the castingBefore method,$nodeshould already be aUTCDateTimeobject after retrieval from MongoDB.case Database::VAR_DATETIME : - $node = DateTime::format($node->toDateTime()); + if ($node instanceof UTCDateTime) { + $node = DateTime::format($node->toDateTime()); + } elseif ($node instanceof \DateTime) { + $node = DateTime::format($node); + } break;
912-923: Add error handling for DateTime creation.Creating DateTime from string without validation could throw exceptions on invalid date formats.
foreach ($value as &$node) { switch ($type) { case Database::VAR_DATETIME : - $node = new UTCDateTime(new \DateTime($node)); + try { + $node = new UTCDateTime(new \DateTime($node)); + } catch (\Exception $e) { + throw new DatabaseException('Invalid datetime format: ' . $node); + } break; default: break; } }
1088-1088: Fix incorrect parameter syntax for update operation.- $this->client->update($name, $filters, $updateQuery, multi: true); + $this->client->update($name, $filters, $updateQuery, ['multi' => true]);
1177-1184: Fix PHPDoc to match actual parameters and return type.The PHPDoc references wrong parameter names and incorrect return type.
/** * Get sequences for documents that were created * * @param string $collection - * @param array<string> $documentIds - * @param array<int> $documentTenants - * @return array<string, string> + * @param array<Document> $documents + * @return array<Document> */
1302-1309: Fix PHPDoc parameter name.The PHPDoc references
$idsbut the parameter is named$sequences./** * Delete Documents * * @param string $collection - * @param array<string> $ids + * @param array<string> $sequences + * @param array<string> $permissionIds * * @return int */
1410-1413: Escape dynamic role values in the Mongo regex to prevent injection and ReDoSThe current code builds a regex from the raw role names without escaping, which allows unintended regex meta-characters and risks both incorrect matches and ReDoS attacks if roles become complex.
if (Authorization::$status) { - $roles = \implode('|', Authorization::getRoles()); + // Quote each role to neutralize regex metacharacters + $quoted = \array_map(fn($r) => \preg_quote($r, '/'), Authorization::getRoles()); + $roles = \implode('|', $quoted); $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')]; }
1541-1544: Add error handling for DateTime creation.Creating DateTime from string without validation could throw exceptions on invalid date formats.
private function toMongoDatetime(string $dt): UTCDateTime { - return new UTCDateTime(new \DateTime($dt)); + try { + return new UTCDateTime(new \DateTime($dt)); + } catch (\Exception $e) { + throw new DatabaseException('Invalid datetime format: ' . $dt); + } }
1606-1609: Escape role values in permissions regexSame regex injection vulnerability as in the
findmethod. Apply the same escaping fix here.if (Authorization::$status) { // skip if authorization is disabled - $roles = \implode('|', Authorization::getRoles()); + $escapedRoles = \array_map(fn($r) => \preg_quote($r, '/'), Authorization::getRoles()); + $roles = \implode('|', $escapedRoles); $filters['_permissions']['$in'] = [new Regex("read\(\".*(?:{$roles}).*\"\)", 'i')]; }
1718-1721: Validate ObjectId format before creation.Creating ObjectId without validation could throw exceptions if the sequence is not a valid 24-character hex string.
if (array_key_exists('$sequence', $array)) { - $result['_id'] = new ObjectId($array['$sequence']); + try { + $result['_id'] = new ObjectId($array['$sequence']); + } catch (\Exception $e) { + // Handle invalid ObjectId format + throw new DatabaseException('Invalid sequence format: ' . $array['$sequence']); + } unset($result['$sequence']); }
1998-2001: Add error handling for DateTime creationpublic function setUTCDatetime(string $value): mixed { - return new UTCDateTime(new \DateTime($value)); + try { + return new UTCDateTime(new \DateTime($value)); + } catch (\Exception $e) { + throw new DatabaseException('Invalid datetime format: ' . $value); + } }
2324-2328: Fix return type mismatch.The method is declared to return
stringbut returnsint|nullfromgetTenant().public function getTenantQuery(string $collection, string $parentAlias = ''): string { - return $this->getTenant(); + return (string) $this->getTenant(); }
2287-2290: Implement quote method or throw exception.The
quote()method returns an empty string, which could cause issues if called. It should either be properly implemented or throw an exception.protected function quote(string $string): string { - return ""; + // MongoDB doesn't require quoting identifiers like SQL databases + return $string; }
🧹 Nitpick comments (1)
tests/e2e/Adapter/MongoDBTest.php (1)
100-108: Add documentation explaining why these methods are no-ops.These methods appear to override abstract methods from the parent class but don't use their parameters. Add comments explaining why these operations are no-ops for MongoDB.
+/** + * MongoDB doesn't support explicit column deletion like SQL databases. + * Fields are removed by unsetting them from documents. + * + * @param string $collection Collection name (unused) + * @param string $column Column name (unused) + * @return bool Always returns true + */ protected static function deleteColumn(string $collection, string $column): bool { return true; } +/** + * Index deletion is handled differently in MongoDB. + * This method is a no-op to satisfy the parent interface. + * + * @param string $collection Collection name (unused) + * @param string $index Index name (unused) + * @return bool Always returns true + */ protected static function deleteIndex(string $collection, string $index): bool { return true; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docker-compose.yml(2 hunks)src/Database/Adapter/Mongo.php(1 hunks)src/Database/Database.php(20 hunks)tests/e2e/Adapter/MongoDBTest.php(1 hunks)tests/e2e/Adapter/Scopes/CollectionTests.php(1 hunks)tests/e2e/Adapter/Scopes/DocumentTests.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/e2e/Adapter/Scopes/CollectionTests.php
- docker-compose.yml
- tests/e2e/Adapter/Scopes/DocumentTests.php
🧰 Additional context used
🪛 GitHub Actions: Linter
src/Database/Database.php
[error] 1-1: PHP CS Fixer (laravel/pint) PSR-12 style issue: no_whitespace_in_blank_line
tests/e2e/Adapter/MongoDBTest.php
[error] 1-1: PHP CS Fixer (laravel/pint) PSR-12 style issue: no_unused_imports
src/Database/Adapter/Mongo.php
[error] 1-1: PHP CS Fixer (laravel/pint) PSR-12 style issue: no_whitespace_in_blank_line
🪛 PHPMD (2.15.0)
tests/e2e/Adapter/MongoDBTest.php
100-100: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
100-100: Avoid unused parameters such as '$column'. (Unused Code Rules)
(UnusedFormalParameter)
105-105: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
105-105: Avoid unused parameters such as '$index'. (Unused Code Rules)
(UnusedFormalParameter)
src/Database/Adapter/Mongo.php
61-61: Avoid unused parameters such as '$event'. (Unused Code Rules)
(UnusedFormalParameter)
116-116: Avoid unused parameters such as '$name'. (Unused Code Rules)
(UnusedFormalParameter)
131-131: Avoid unused parameters such as '$database'. (Unused Code Rules)
(UnusedFormalParameter)
378-378: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
408-408: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
408-408: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$relatedCollection'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$twoWay'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$twoWayKey'. (Unused Code Rules)
(UnusedFormalParameter)
492-492: Avoid unused parameters such as '$side'. (Unused Code Rules)
(UnusedFormalParameter)
619-619: Avoid unused parameters such as '$lengths'. (Unused Code Rules)
(UnusedFormalParameter)
741-741: Avoid unused parameters such as '$forUpdate'. (Unused Code Rules)
(UnusedFormalParameter)
1027-1027: Avoid unused parameters such as '$skipPermissions'. (Unused Code Rules)
(UnusedFormalParameter)
1309-1309: Avoid unused parameters such as '$permissionIds'. (Unused Code Rules)
(UnusedFormalParameter)
1349-1349: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
1349-1349: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
1349-1349: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
1349-1349: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
1882-1882: Avoid unused parameters such as '$prefix'. (Unused Code Rules)
(UnusedFormalParameter)
2215-2215: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2287-2287: Avoid unused parameters such as '$string'. (Unused Code Rules)
(UnusedFormalParameter)
2296-2296: Avoid unused parameters such as '$stmt'. (Unused Code Rules)
(UnusedFormalParameter)
2319-2319: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2324-2324: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2324-2324: Avoid unused parameters such as '$parentAlias'. (Unused Code Rules)
(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). (2)
- GitHub Check: Adapter Tests (SQLite)
- GitHub Check: Unit Test
🔇 Additional comments (19)
tests/e2e/Adapter/MongoDBTest.php (2)
19-28: LGTM!The
getAdapterNamemethod correctly returns the adapter name.
68-78: LGTM!The test correctly handles MongoDB's on-the-fly database creation behavior by removing the exists check.
src/Database/Database.php (17)
3271-3272: Post-fetch casting hook added – looks correct.
No further action required.
3643-3644: Pre-insert casting hook added – looks correct.
3652-3653: Post-insert casting hook added – looks correct.
3751-3753: Post-batch casting hook – OK.
4120-4120: Minor: flag initialised – OK.
4287-4287: Pre-update casting hook – OK.
4291-4291: Post-update casting hook – OK.
4953-4954: Book-keeping array introduced – sensible.
4990-4992: Tracks processed IDs – LGTM.
5088-5090: Casts both old & new before upsert – OK.
5113-5113: Sequence retrieval unaffected – fine.
5124-5126: Post-upsert casting hook – OK.
5141-5144: Guarding callback with processed set – good safety improvement.
6104-6106: Casting cursor before encode – sound.
6134-6136: Post-fetch casting in find() – OK.
6150-6150: Unsetting reference – negligible but harmless.
6724-6728: Adapter-specific UTC handling – well-structured.
| $redis = new Redis(); | ||
| $redis->connect('redis', 6379); | ||
| $redis->flushAll(); | ||
| $cache = new Cache(new RedisAdapter($redis)); |
There was a problem hiding this comment.
Critical: Redis flushAll() will clear all data.
The flushAll() command will clear ALL data in Redis, not just test data. This could affect other tests or services using the same Redis instance.
Consider using a dedicated Redis database or prefixed keys:
$redis = new Redis();
$redis->connect('redis', 6379);
-$redis->flushAll();
+$redis->select(1); // Use a dedicated test database
+$redis->flushDB(); // Clear only the selected database
$cache = new Cache(new RedisAdapter($redis));📝 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.
| $redis = new Redis(); | |
| $redis->connect('redis', 6379); | |
| $redis->flushAll(); | |
| $cache = new Cache(new RedisAdapter($redis)); | |
| $redis = new Redis(); | |
| $redis->connect('redis', 6379); | |
| $redis->select(1); // Use a dedicated test database | |
| $redis->flushDB(); // Clear only the selected database | |
| $cache = new Cache(new RedisAdapter($redis)); |
🤖 Prompt for AI Agents
In tests/e2e/Adapter/MongoDBTest.php around lines 39 to 42, the use of
redis->flushAll() clears all data in the Redis instance, which can disrupt other
tests or services. To fix this, configure Redis to use a dedicated database
index for testing or modify the cache keys to include a unique prefix to isolate
test data. This prevents interference with other data while maintaining test
isolation.
| $schema = 'utopiaTests'; // same as $this->testDatabase | ||
| $client = new Client( | ||
| $schema, | ||
| 'mongo', | ||
| 27017, | ||
| 'root', | ||
| 'password', | ||
| false | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract hardcoded values to configuration.
The database schema and connection parameters are hardcoded. Consider making these configurable for better test flexibility.
-$schema = 'utopiaTests'; // same as $this->testDatabase
+$schema = getenv('TEST_MONGODB_DATABASE') ?: 'utopiaTests';
$client = new Client(
$schema,
- 'mongo',
- 27017,
- 'root',
- 'password',
+ getenv('TEST_MONGODB_HOST') ?: 'mongo',
+ (int) (getenv('TEST_MONGODB_PORT') ?: 27017),
+ getenv('TEST_MONGODB_USER') ?: 'root',
+ getenv('TEST_MONGODB_PASSWORD') ?: 'password',
false
);📝 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.
| $schema = 'utopiaTests'; // same as $this->testDatabase | |
| $client = new Client( | |
| $schema, | |
| 'mongo', | |
| 27017, | |
| 'root', | |
| 'password', | |
| false | |
| ); | |
| $schema = getenv('TEST_MONGODB_DATABASE') ?: 'utopiaTests'; | |
| $client = new Client( | |
| $schema, | |
| getenv('TEST_MONGODB_HOST') ?: 'mongo', | |
| (int) (getenv('TEST_MONGODB_PORT') ?: 27017), | |
| getenv('TEST_MONGODB_USER') ?: 'root', | |
| getenv('TEST_MONGODB_PASSWORD') ?: 'password', | |
| false | |
| ); |
🤖 Prompt for AI Agents
In tests/e2e/Adapter/MongoDBTest.php around lines 44 to 52, the database schema
and connection parameters are hardcoded, reducing test flexibility. Refactor the
code to extract these values into configuration variables or environment
variables, then use those variables to initialize the Client. This will allow
easy modification of connection details without changing the test code.
| public function testRenameAttribute(): void | ||
| { | ||
| $this->assertTrue(true); | ||
| } | ||
|
|
||
| public function testRenameAttributeExisting(): void | ||
| { | ||
| $this->assertTrue(true); | ||
| } | ||
|
|
||
| public function testUpdateAttributeStructure(): void | ||
| { | ||
| $this->assertTrue(true); | ||
| } | ||
|
|
||
| public function testKeywords(): void | ||
| { | ||
| $this->assertTrue(true); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mark stub tests as incomplete or add TODO comments.
These test methods are not implemented and only assert true. Consider marking them as incomplete or adding TODO comments to indicate they need implementation.
public function testRenameAttribute(): void
{
- $this->assertTrue(true);
+ $this->markTestIncomplete('MongoDB attribute renaming tests not yet implemented');
}
public function testRenameAttributeExisting(): void
{
- $this->assertTrue(true);
+ $this->markTestIncomplete('MongoDB existing attribute renaming tests not yet implemented');
}
public function testUpdateAttributeStructure(): void
{
- $this->assertTrue(true);
+ $this->markTestIncomplete('MongoDB attribute structure update tests not yet implemented');
}
public function testKeywords(): void
{
- $this->assertTrue(true);
+ $this->markTestIncomplete('MongoDB keywords tests not yet implemented');
}📝 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.
| public function testRenameAttribute(): void | |
| { | |
| $this->assertTrue(true); | |
| } | |
| public function testRenameAttributeExisting(): void | |
| { | |
| $this->assertTrue(true); | |
| } | |
| public function testUpdateAttributeStructure(): void | |
| { | |
| $this->assertTrue(true); | |
| } | |
| public function testKeywords(): void | |
| { | |
| $this->assertTrue(true); | |
| } | |
| public function testRenameAttribute(): void | |
| { | |
| $this->markTestIncomplete('MongoDB attribute renaming tests not yet implemented'); | |
| } | |
| public function testRenameAttributeExisting(): void | |
| { | |
| $this->markTestIncomplete('MongoDB existing attribute renaming tests not yet implemented'); | |
| } | |
| public function testUpdateAttributeStructure(): void | |
| { | |
| $this->markTestIncomplete('MongoDB attribute structure update tests not yet implemented'); | |
| } | |
| public function testKeywords(): void | |
| { | |
| $this->markTestIncomplete('MongoDB keywords tests not yet implemented'); | |
| } |
🤖 Prompt for AI Agents
In tests/e2e/Adapter/MongoDBTest.php between lines 80 and 98, the test methods
are stubs that only assert true. Replace these placeholder assertions with calls
to mark the tests as incomplete or add TODO comments inside each method to
clearly indicate that the tests need to be implemented.
…and optimizing document ID retrieval; streamline sequence fetching logic for improved performance.
…nd tenants; streamline upsert filter construction for improved clarity and performance.
…and adding MongoDB initialization credentials for username and password.
…ttribute handling during upserts; ensure correct incrementing of specified attributes while maintaining other fields.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/Database/Adapter/Mongo.php (2)
1143-1144: Fix typos in comment.The comment contains spelling errors.
// Remove the attribute from $set since we're incrementing it - // it is requierd to mimic the behaver of SQL on duplicate key update + // it is required to mimic the behavior of SQL on duplicate key update unset($record[$attribute]);
2169-2171: Remove stray character in PHPDoc.The PHPDoc contains a stray 'p' character.
/** * Returns number of attributes used by default. - *p + * * @return int */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Database/Adapter/Mongo.php(1 hunks)
🧰 Additional context used
🪛 PHPMD (2.15.0)
src/Database/Adapter/Mongo.php
61-61: Avoid unused parameters such as '$event'. (Unused Code Rules)
(UnusedFormalParameter)
116-116: Avoid unused parameters such as '$name'. (Unused Code Rules)
(UnusedFormalParameter)
131-131: Avoid unused parameters such as '$database'. (Unused Code Rules)
(UnusedFormalParameter)
378-378: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
395-395: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
408-408: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
408-408: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$relatedCollection'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$twoWay'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$id'. (Unused Code Rules)
(UnusedFormalParameter)
466-466: Avoid unused parameters such as '$twoWayKey'. (Unused Code Rules)
(UnusedFormalParameter)
492-492: Avoid unused parameters such as '$side'. (Unused Code Rules)
(UnusedFormalParameter)
619-619: Avoid unused parameters such as '$lengths'. (Unused Code Rules)
(UnusedFormalParameter)
741-741: Avoid unused parameters such as '$forUpdate'. (Unused Code Rules)
(UnusedFormalParameter)
1023-1023: Avoid unused parameters such as '$skipPermissions'. (Unused Code Rules)
(UnusedFormalParameter)
1119-1119: Avoid unused local variables such as '$documentIds'. (Unused Code Rules)
(UnusedLocalVariable)
1302-1302: Avoid unused parameters such as '$permissionIds'. (Unused Code Rules)
(UnusedFormalParameter)
1342-1342: Avoid unused parameters such as '$type'. (Unused Code Rules)
(UnusedFormalParameter)
1342-1342: Avoid unused parameters such as '$size'. (Unused Code Rules)
(UnusedFormalParameter)
1342-1342: Avoid unused parameters such as '$signed'. (Unused Code Rules)
(UnusedFormalParameter)
1342-1342: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
1875-1875: Avoid unused parameters such as '$prefix'. (Unused Code Rules)
(UnusedFormalParameter)
2208-2208: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2280-2280: Avoid unused parameters such as '$string'. (Unused Code Rules)
(UnusedFormalParameter)
2289-2289: Avoid unused parameters such as '$stmt'. (Unused Code Rules)
(UnusedFormalParameter)
2312-2312: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2317-2317: Avoid unused parameters such as '$collection'. (Unused Code Rules)
(UnusedFormalParameter)
2317-2317: Avoid unused parameters such as '$parentAlias'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (1)
src/Database/Adapter/Mongo.php (1)
634-639: Fix array index when accessing orders in createIndex.The code uses
$ito access the$ordersarray, but$iis the loop variable for iterating over$attributes. This will cause an index out of bounds error when there are more attributes than orders.foreach ($attributes as $i => $attribute) { $attribute = $this->filter($attribute); - $orderType = $this->getOrder($this->filter($orders[$i] ?? Database::ORDER_ASC)); + $orderType = $this->getOrder($this->filter($orders[$i] ?? Database::ORDER_ASC)); $indexes['key'][$attribute] = $orderType;Note: The array index is correctly using
$ihere since it corresponds to the attribute index. However, ensure that the$ordersarray has the same number of elements as$attributesor handle the case gracefully.Likely an incorrect or invalid review comment.
| try { | ||
| $result = $this->getClient()->query($command); | ||
| if (is_object($result)) { | ||
| return $result->totalSize; | ||
| } else { | ||
| throw new DatabaseException('No size found'); | ||
| } | ||
| } catch (Exception $e) { | ||
| throw new DatabaseException('Failed to get collection size: ' . $e->getMessage()); | ||
| } |
There was a problem hiding this comment.
Add null check before accessing result properties.
The code accesses $result->totalSize without first checking if $result is null, which could cause a null pointer error.
try {
$result = $this->getClient()->query($command);
- if (is_object($result)) {
+ if (is_object($result) && isset($result->totalSize)) {
return $result->totalSize;
} else {
throw new DatabaseException('No size found');
}
} catch (Exception $e) {
throw new DatabaseException('Failed to get collection size: ' . $e->getMessage());
}📝 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.
| try { | |
| $result = $this->getClient()->query($command); | |
| if (is_object($result)) { | |
| return $result->totalSize; | |
| } else { | |
| throw new DatabaseException('No size found'); | |
| } | |
| } catch (Exception $e) { | |
| throw new DatabaseException('Failed to get collection size: ' . $e->getMessage()); | |
| } | |
| try { | |
| $result = $this->getClient()->query($command); | |
| if (is_object($result) && isset($result->totalSize)) { | |
| return $result->totalSize; | |
| } else { | |
| throw new DatabaseException('No size found'); | |
| } | |
| } catch (Exception $e) { | |
| throw new DatabaseException('Failed to get collection size: ' . $e->getMessage()); | |
| } |
🤖 Prompt for AI Agents
In src/Database/Adapter/Mongo.php around lines 347 to 356, the code accesses
$result->totalSize without verifying if $result is null, risking a null pointer
error. Add a null check to ensure $result is not null before accessing its
totalSize property. If $result is null, handle it appropriately, such as
throwing a DatabaseException or returning a default value.
| $this->client->update($name, $filters, $updateQuery, multi: true); | ||
| } catch (MongoException $e) { | ||
| throw new Duplicate($e->getMessage()); | ||
| } | ||
|
|
||
| return 1; | ||
| } |
There was a problem hiding this comment.
Return actual update count instead of hardcoded value.
The method always returns 1 regardless of how many documents were actually updated. The MongoDB update operation should return the number of modified documents.
try {
- $this->client->update($name, $filters, $updateQuery, multi: true);
+ $result = $this->client->update($name, $filters, $updateQuery, ['multi' => true]);
} catch (MongoException $e) {
throw new Duplicate($e->getMessage());
}
- return 1;
+ return $result->modifiedCount ?? 0;Note: Also fix the incorrect parameter syntax for multi as mentioned in other review comments.
📝 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.
| $this->client->update($name, $filters, $updateQuery, multi: true); | |
| } catch (MongoException $e) { | |
| throw new Duplicate($e->getMessage()); | |
| } | |
| return 1; | |
| } | |
| try { | |
| - $this->client->update($name, $filters, $updateQuery, multi: true); | |
| + $result = $this->client->update($name, $filters, $updateQuery, ['multi' => true]); | |
| } catch (MongoException $e) { | |
| throw new Duplicate($e->getMessage()); | |
| } | |
| - return 1; | |
| + return $result->modifiedCount ?? 0; | |
| } |
🤖 Prompt for AI Agents
In src/Database/Adapter/Mongo.php around lines 1083 to 1089, the method
currently returns a hardcoded value of 1 instead of the actual number of
documents updated by the MongoDB update operation. Modify the code to capture
the result of the update call and return the count of modified documents from
that result. Also, correct the usage of the multi parameter by passing it as
part of an options array or using the correct syntax supported by the MongoDB
driver instead of the invalid named argument syntax.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores