Upsert #19
Upsert #19
Conversation
WalkthroughThe changes update the MongoDB dependency in Changes
Sequence Diagram(s)sequenceDiagram
participant Test as testUpsert()
participant Client as Client::upsert()
participant MongoDB as MongoDB Server
Test->>Client: upsert('movies_upsert', [op1, op2])
loop For each operation
Client->>Client: Clean nulls from update
Client->>Client: Build update operation array
end
Client->>MongoDB: Send bulk update command with upsert=true
MongoDB-->>Client: Return update results
Client-->>Test: Return self
Test->>MongoDB: Fetch all documents
MongoDB-->>Test: Return documents
Test->>Test: Assert document states
Estimated code review effort3 (30–60 minutes) Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/MongoTest.php (1)
230-264: Excellent test coverage for the new bulk upsert functionality.The test effectively validates:
- Updating existing documents with multiple operations ($set and $inc)
- Inserting new documents when filters don't match existing ones
- Proper handling of bulk operations in a single upsert call
Consider adding explicit cleanup for the test collection to ensure test isolation:
public function testUpsert() { + // Clean up any existing test data + $this->getDatabase()->dropCollection('movies_upsert'); + $this->getDatabase()->insert(src/Client.php (1)
548-579: Robust implementation of bulk upsert functionality.The logic correctly:
- Processes multiple operations in a single command
- Maintains null value cleaning for data integrity
- Supports per-operation multi flag configuration
- Constructs proper MongoDB update command structure
Consider adding input validation to improve robustness:
public function upsert(string $collection, array $operations, array $options = []): self { + if (empty($operations)) { + throw new Exception('Operations array cannot be empty'); + } + $updates = []; foreach ($operations as $op) { + if (!isset($op['filter']) || !isset($op['update'])) { + throw new Exception('Each operation must have "filter" and "update" keys'); + } + $cleanUpdate = [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
composer.json(1 hunks)src/Client.php(2 hunks)tests/MongoTest.php(3 hunks)
🔇 Additional comments (5)
composer.json (1)
33-33: Confirm MongoDB PHP Library 2.1.0 upgrade: Security clear, check breaking changesThe bump from 1.10.0 → 2.1.0 carries no known security advisories (all vulnerabilities were patched in ≥ 1.9.2), but it is a major version upgrade that may introduce breaking API changes. Please:
- Security: No active advisories affect the 2.x series (patched in 1.9.2).
- Compatibility: Review the official 2.x upgrade guide and CHANGELOG in the MongoDB PHP Library repo for breaking changes.
- Code audit: Update any deprecated or removed APIs (e.g., Manager/Client instantiation options, BulkWrite behaviors).
- Testing: Run your full test suite—including integration tests against the PHP driver v2.1.0—to confirm nothing regresses.
tests/MongoTest.php (2)
22-22: LGTM! Minor formatting improvement.The added whitespace improves code readability.
113-113: LGTM! Minor formatting improvement.The added whitespace improves code readability.
src/Client.php (2)
40-40: LGTM! Minor formatting improvement.The added whitespace improves code readability.
537-547: Verify all upsert() call sites for the new bulk‐operations signatureThe refactored
upsert(string $collection, array $operations, array $options = [])is a breaking change—existing calls using the old(filter, update[, options])signature must be updated.
Run these commands to locate any outdated usages and correct them:• rg "->upsert(" --type php
• Look for calls still passing two or three arguments
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/Client.php (2)
554-557: Consider using array_filter for cleaner null removal.The null value cleaning logic can be simplified using
array_filter.- $cleanUpdate = []; - foreach ($op['update'] as $k => $v) { - if (!is_null($v)) { - $cleanUpdate[$k] = $v; - } - } + $cleanUpdate = array_filter($op['update'], fn($v) => !is_null($v));
799-811: Consider using 'hello' command for MongoDB 5.0+ compatibility.The
isMastercommand was deprecated in MongoDB 5.0 and replaced withhello. For better forward compatibility, consider detecting the MongoDB version and using the appropriate command.$result = $this->query([ - 'isMaster' => 1, + 'hello' => 1, ], 'admin');Or implement version detection to use the appropriate command:
public function isReplicaSet(): bool { if ($this->replicaSet !== null) { return $this->replicaSet; } // Try 'hello' first (MongoDB 5.0+), fallback to 'isMaster' try { $result = $this->query(['hello' => 1], 'admin'); } catch (Exception $e) { $result = $this->query(['isMaster' => 1], 'admin'); } $this->replicaSet = property_exists($result, 'setName'); return $this->replicaSet; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
composer.json(1 hunks)src/Client.php(5 hunks)
🪛 PHPStan (2.1.17)
src/Client.php
140-140: Function MongoDB\BSON\fromPHPWithOptions not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(function.notFound)
203-203: Function MongoDB\BSON\toPHPWithOptions not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(function.notFound)
🪛 GitHub Actions: CodeQL
src/Client.php
[error] 140-140: PHPStan: Function MongoDB\BSON\fromPHPWithOptions not found. (function.notFound)
[error] 203-203: PHPStan: Function MongoDB\BSON\toPHPWithOptions not found. (function.notFound)
🪛 GitHub Actions: Linter
src/Client.php
[error] 1-1: Laravel Pint PSR-12 linting failed: issues with 'braces' and 'no_whitespace_in_blank_line' rules.
🚧 Files skipped from review as they are similar to previous changes (1)
- composer.json
🧰 Additional context used
🪛 PHPStan (2.1.17)
src/Client.php
140-140: Function MongoDB\BSON\fromPHPWithOptions not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(function.notFound)
203-203: Function MongoDB\BSON\toPHPWithOptions not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(function.notFound)
🪛 GitHub Actions: CodeQL
src/Client.php
[error] 140-140: PHPStan: Function MongoDB\BSON\fromPHPWithOptions not found. (function.notFound)
[error] 203-203: PHPStan: Function MongoDB\BSON\toPHPWithOptions not found. (function.notFound)
🪛 GitHub Actions: Linter
src/Client.php
[error] 1-1: Laravel Pint PSR-12 linting failed: issues with 'braces' and 'no_whitespace_in_blank_line' rules.
⏰ 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). (4)
- GitHub Check: Tests (8.2)
- GitHub Check: Tests (8.3)
- GitHub Check: Tests (8.0)
- GitHub Check: Tests (8.1)
🔇 Additional comments (1)
src/Client.php (1)
791-791: Good caching strategy implementation.The nullable boolean property provides an efficient way to cache the replica set detection result.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/Client.php (3)
140-140: Fix incorrect BSON function callsThe BSON serialization functions are incorrectly called. Based on MongoDB PHP library v2.1.0, the correct functions are
MongoDB\BSON\fromPHPandMongoDB\BSON\toPHP, notMongoDB\BSON\fromPHPWithOptionsandMongoDB\BSON\toPHPWithOptions.Apply this diff:
- $sections = MongoDB\BSON\fromPHP($params); + $sections = BSON\fromPHP($params);- $result = MongoDB\BSON\fromPHP(substr($res, 21, $responseLength - 21)); + $result = BSON\toPHP(substr($res, 21, $responseLength - 21));Note: You already have
use MongoDB\BSON;at the top of the file.Also applies to: 203-203
537-548: Document breaking API change in PHPDocThe method signature has changed from
upsert(string $collection, array $filter, array $update, array $options = [])to support bulk operations. This is a breaking change that requires updates to existing code.Update the PHPDoc to include migration guidance:
/** - * Insert, or update, document(s) with support for bulk operations. + * Insert, or update, document(s) with support for bulk operations. + * + * Breaking Change: This method now accepts bulk operations instead of individual filter/update parameters. + * Migration: Change from upsert($collection, $filter, $update) to upsert($collection, [['filter' => $filter, 'update' => $update]]) + * * https://docs.mongodb.com/manual/reference/command/update/#syntax * * @param string $collection * @param array $operations Array of operations, each with 'filter' and 'update' keys * @param array $options * * @return self * @throws Exception */
560-560: Apply consistent object conversion to filterFor consistency with other methods in the class, convert the filter to an object using
toObject().Apply this diff:
$updateOperation = [ - 'q' => $op['filter'], + 'q' => $this->toObject($op['filter']), 'u' => $cleanUpdate, 'upsert' => true, 'multi' => isset($op['multi']) ? $op['multi'] : false, ];
🧹 Nitpick comments (1)
src/Client.php (1)
805-807: Consider using 'hello' command for future compatibilityThe
isMastercommand is deprecated in MongoDB 5.0+. Consider using thehellocommand for better forward compatibility while maintaining backward compatibility.$result = $this->query([ - 'isMaster' => 1, + 'hello' => 1, ], 'admin');Alternatively, implement a version check to use the appropriate command based on the MongoDB server version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Auth.php(1 hunks)src/Client.php(5 hunks)
🪛 PHPStan (2.1.17)
src/Client.php
140-140: Function Utopia\Mongo\MongoDB\BSON\fromPHP not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(function.notFound)
203-203: Function Utopia\Mongo\MongoDB\BSON\fromPHP not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(function.notFound)
🪛 GitHub Actions: CodeQL
src/Client.php
[error] 140-140: PHPStan: Function Utopia\Mongo\MongoDB\BSON\fromPHP not found. (function.notFound)
[error] 203-203: PHPStan: Function Utopia\Mongo\MongoDB\BSON\fromPHP not found. (function.notFound)
🪛 GitHub Actions: Linter
src/Client.php
[error] 1-1: Laravel Pint PSR-12 linting failed: issues with 'braces' and 'no_whitespace_in_blank_line' rules.
🧰 Additional context used
🪛 PHPStan (2.1.17)
src/Client.php
140-140: Function Utopia\Mongo\MongoDB\BSON\fromPHP not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(function.notFound)
203-203: Function Utopia\Mongo\MongoDB\BSON\fromPHP not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols
(function.notFound)
🪛 GitHub Actions: CodeQL
src/Client.php
[error] 140-140: PHPStan: Function Utopia\Mongo\MongoDB\BSON\fromPHP not found. (function.notFound)
[error] 203-203: PHPStan: Function Utopia\Mongo\MongoDB\BSON\fromPHP not found. (function.notFound)
🪛 GitHub Actions: Linter
src/Client.php
[error] 1-1: Laravel Pint PSR-12 linting failed: issues with 'braces' and 'no_whitespace_in_blank_line' rules.
⏰ 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). (4)
- GitHub Check: Tests (8.2)
- GitHub Check: Tests (8.3)
- GitHub Check: Tests (8.0)
- GitHub Check: Tests (8.1)
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/Client.php (1)
205-234: Add type safety for BSON deserialization result.The
toPHP()method can return either an array or object, but the code assumes it's always an object. This could cause runtime errors when accessing properties.Consider adding type checking or use
toPHP([\MongoDB\BSON\TypeMap::DOCUMENT => 'object'])to ensure consistent object return:- $result = Document::fromBSON($bsonString)->toPHP(); + $result = Document::fromBSON($bsonString)->toPHP([ + 'root' => 'object', + 'array' => 'object', + 'document' => 'object' + ]);
♻️ Duplicate comments (3)
src/Client.php (3)
141-141: BSON serialization correctly updated.The change from the non-existent
fromPHPWithOptionstoDocument::fromPHPproperly addresses the compatibility issue with MongoDB PHP library 2.1.0.
537-548: Document the breaking API change and migration path.The method signature change from single operations to bulk operations is a breaking change. Add a migration example in the PHPDoc.
Add migration documentation:
/** * Insert, or update, document(s) with support for bulk operations. * https://docs.mongodb.com/manual/reference/command/update/#syntax * + * Migration from v1.x: + * Before: $client->upsert('collection', ['_id' => 1], ['$set' => ['name' => 'test']]) + * After: $client->upsert('collection', [['filter' => ['_id' => 1], 'update' => ['$set' => ['name' => 'test']]]]) + * * @param string $collection * @param array $operations Array of operations, each with 'filter' and 'update' keys * @param array $options
560-565: Convert filter and update to objects for consistency.The filter and cleaned update should be converted to objects to maintain consistency with other methods in the class.
Apply this diff:
$updateOperation = [ - 'q' => $op['filter'], - 'u' => $cleanUpdate, + 'q' => $this->toObject($op['filter']), + 'u' => $this->toObject($cleanUpdate), 'upsert' => true, 'multi' => isset($op['multi']) ? $op['multi'] : false, ];
🧹 Nitpick comments (1)
src/Client.php (1)
792-812: Consider using 'hello' command for future compatibility.The
isMastercommand is deprecated in MongoDB 5.0+ in favor of thehellocommand. WhileisMasterstill works, updating tohelloensures future compatibility.Apply this diff for better future compatibility:
$result = $this->query([ - 'isMaster' => 1, + 'hello' => 1, ], 'admin');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Dockerfile-php-8.3(1 hunks)composer.json(1 hunks)src/Client.php(8 hunks)tests/MongoTest.php(1 hunks)
🪛 PHPStan (2.1.17)
src/Client.php
204-204: Variable $result in PHPDoc tag @var does not match assigned variable $bsonString.
(varTag.differentVariable)
🪛 GitHub Actions: CodeQL
src/Client.php
[error] 204-204: PHPStan: Variable $result in PHPDoc tag @var does not match assigned variable $bsonString. (varTag.differentVariable)
[error] 209-210: PHPStan: Cannot access property $writeErrors on array|object. (property.nonObject)
[error] 217-218: PHPStan: Cannot access properties $code, $codeName, $errmsg on array|object. (property.nonObject)
[error] 222-223: PHPStan: Cannot access properties $ok and $n on array|object. (property.nonObject)
[error] 226-226: PHPStan: Cannot access property $ok on array|object. (property.nonObject)
[error] 230-230: PHPStan: Cannot access property $ok on array|object. (property.nonObject)
[error] 234-234: PHPStan: Cannot access property $cursor on array|object. (property.nonObject)
🚧 Files skipped from review as they are similar to previous changes (2)
- composer.json
- tests/MongoTest.php
🧰 Additional context used
🪛 PHPStan (2.1.17)
src/Client.php
204-204: Variable $result in PHPDoc tag @var does not match assigned variable $bsonString.
(varTag.differentVariable)
🪛 GitHub Actions: CodeQL
src/Client.php
[error] 204-204: PHPStan: Variable $result in PHPDoc tag @var does not match assigned variable $bsonString. (varTag.differentVariable)
[error] 209-210: PHPStan: Cannot access property $writeErrors on array|object. (property.nonObject)
[error] 217-218: PHPStan: Cannot access properties $code, $codeName, $errmsg on array|object. (property.nonObject)
[error] 222-223: PHPStan: Cannot access properties $ok and $n on array|object. (property.nonObject)
[error] 226-226: PHPStan: Cannot access property $ok on array|object. (property.nonObject)
[error] 230-230: PHPStan: Cannot access property $ok on array|object. (property.nonObject)
[error] 234-234: PHPStan: Cannot access property $cursor on array|object. (property.nonObject)
⏰ 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). (4)
- GitHub Check: Tests (8.0)
- GitHub Check: Tests (8.2)
- GitHub Check: Tests (8.1)
- GitHub Check: Tests (8.3)
🔇 Additional comments (3)
Dockerfile-php-8.3 (1)
16-16: Version update looks good.The MongoDB PHP driver version update to 2.1.1 aligns with the dependency updates mentioned in the PR objectives.
src/Client.php (2)
5-6: Import statements are correctly added.The new imports for
DocumentandObjectIdproperly support the updated BSON serialization methods used throughout the file.
439-439: ObjectId usage correctly updated.The change to use the imported
ObjectIdclass is consistent with the updated imports.Also applies to: 464-464
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/tests.yml (1)
8-11: Re-evaluating the decision to test only against PHP 8.3Cutting the matrix down to a single PHP version greatly reduces our safety net. Unless the library’s
composer.jsonnow explicitly requiresphp: ^8.3, we’re losing CI coverage for every still-supported PHP minor (8.0–8.2).
Please confirm that:
composer.jsonindeed excludes <8.3, or- we are comfortable shipping untested on those versions.
If backward compatibility is still a goal, consider restoring the full matrix:
- php-versions: ['8.3'] # add PHP versions as required + php-versions: ['8.0', '8.1', '8.2', '8.3'] # expand if BC must be guaranteedTagging for verification rather than a hard refactor, as the intent may be to drop support deliberately.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/tests.yml(1 hunks)
⏰ 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: Tests (8.3)
Co-authored-by: Jake Barnby <jakeb994@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/Client.php (2)
537-548: Document the breaking API change in PHPDoc.The method signature has changed from
upsert(string $collection, array $filter, array $update, array $options = [])to accept bulk operations. This breaking change needs clear documentation.Add a migration note to the PHPDoc:
/** - * Insert, or update, document(s) with support for bulk operations. + * Insert, or update, document(s) with support for bulk operations. + * + * @deprecated The method signature has changed. Previously accepted separate $filter and $update arrays. + * @since 2.0.0 Now accepts an array of operations, each containing 'filter' and 'update' keys. + * + * Migration example: + * Before: $client->upsert('collection', ['_id' => 1], ['$set' => ['name' => 'test']]); + * After: $client->upsert('collection', [['filter' => ['_id' => 1], 'update' => ['$set' => ['name' => 'test']]]]); + * * https://docs.mongodb.com/manual/reference/command/update/#syntax
560-561: Apply consistent object conversion.For consistency with other methods in the class, both filter and update should be converted to objects using
toObject().$updateOperation = [ - 'q' => $op['filter'], - 'u' => $cleanUpdate, + 'q' => $this->toObject($op['filter']), + 'u' => $this->toObject($cleanUpdate), 'upsert' => true, 'multi' => isset($op['multi']) ? $op['multi'] : false, ];
🧹 Nitpick comments (1)
src/Client.php (1)
200-200: Document the wire protocol header offset.The magic number
21represents the MongoDB wire protocol header size (4 bytes each for: message length, request ID, response to, opcode, flags, plus 1 byte section kind). Please add a comment explaining this offset for better code maintainability.+ // Skip 21-byte wire protocol header (4*5 bytes for header fields + 1 byte section kind) $bsonString = substr($res, 21, $responseLength - 21);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Client.php(7 hunks)
⏰ 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: Tests (8.3)
🔇 Additional comments (4)
src/Client.php (4)
5-6: Import statements look good.The addition of
DocumentandObjectIdimports from MongoDB\BSON namespace is appropriate for the BSON serialization and ObjectId instantiation changes in this file.
140-140: BSON serialization update is correct.The change to
Document::fromPHP($params)properly addresses the previous issue with non-existent functions. This is the correct way to serialize PHP data to BSON format.
438-438: ObjectId instantiation is correct.The use of the imported
ObjectIdclass is consistent with the new import statements and improves code readability.Also applies to: 463-463
791-811: Well-implemented replica set detection.The
isReplicaSet()method correctly uses theisMastercommand to detect replica set status and implements proper caching to avoid repeated queries. The implementation is clean and efficient.
… MongoDB response documentation in Client.php
Summary by CodeRabbit
New Features
Tests
Chores
Bug Fixes
CI