Add UUID generation to Client class and update document ID handling #25
Conversation
This commit introduces a new method `createUuid` in the Client class to generate UUIDs using the Ramsey UUID library. The document ID assignment in the insert methods has been updated to use UUIDs instead of MongoDB's ObjectId, enhancing the uniqueness of document identifiers.
WalkthroughSwitch default document ID generation from MongoDB ObjectId to UUID v7 (ramsey/uuid). Add createUuid() to Client. insert() and insertMany() now assign string UUIDs when _id is absent. Add ramsey/uuid dependency to composer.json. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Client
participant Uuid as Ramsey\Uuid
participant Mongo as MongoDB
App->>Client: insert(document)
alt _id not provided
Client->>Uuid: uuid7()
Uuid-->>Client: UUID string
Client->>Mongo: insertOne(document with _id=UUID)
else _id provided
Client->>Mongo: insertOne(document)
end
Mongo-->>Client: result
Client-->>App: result
sequenceDiagram
participant App
participant Client
participant Uuid as Ramsey\Uuid
participant Mongo as MongoDB
App->>Client: insertMany(documents)
loop for each doc
alt _id not provided
Client->>Uuid: uuid7()
Uuid-->>Client: UUID string
Client->>Mongo: prepare doc with _id=UUID
else _id provided
Client->>Mongo: prepare doc
end
end
Client->>Mongo: insertMany(prepared docs)
Mongo-->>Client: result
Client-->>App: result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Client.php (1)
136-144: UUID v7 generator is fine; tiny style nitThe method is straightforward and does what we need. Minor nit: double space before Uuid.
Apply this micro-fix:
- return Uuid::uuid7()->toString(); + return Uuid::uuid7()->toString();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
composer.json(1 hunks)src/Client.php(4 hunks)
🔇 Additional comments (2)
composer.json (1)
33-34: Dependency addition for UUIDs looks goodAdding ramsey/uuid (^4.9.0) is appropriate for UUIDv7 generation and compatible with PHP 8.0. No issues spotted in require block structure.
src/Client.php (1)
8-8: Correct import for Ramsey UUIDImporting Ramsey\Uuid\Uuid aligns with the new UUID generation method.
src/Client.php
Outdated
| } | ||
|
|
||
| $docObj->_id ??= new ObjectId(); | ||
| $docObj->_id ??= $this->createUuid(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Default _id now a UUID string — check downstream expectations; harden the “empty id” check
Changing default _id from ObjectId to string UUID is a breaking behavioral change for consumers that:
- Assume _id is a MongoDB ObjectId.
- Sort by _id to infer recency across mixed types (ObjectId vs string) — Mongo sorts by BSON type first, which may affect methods that rely on _id ordering if collections contain both old (ObjectId) and new (string) docs.
Actionable suggestions:
- Consider treating empty string/null as “missing id” so we don’t persist an empty _id inadvertently.
- Audit call sites that rely on _id ordering (e.g., lastDocument) or expect ObjectId. If mixed types exist, consider migrating existing _id values, or use an explicit createdAt field for recency.
Proposed hardening for both insert paths:
- $docObj->_id ??= $this->createUuid();
+ if (!isset($docObj->_id) || $docObj->_id === '' || $docObj->_id === null) {
+ $docObj->_id = $this->createUuid();
+ }And likewise in insertMany:
- $docObj->_id ??= $this->createUuid();
+ if (!isset($docObj->_id) || $docObj->_id === '' || $docObj->_id === null) {
+ $docObj->_id = $this->createUuid();
+ }Follow-ups:
- Update PHPDoc for insert/insertMany to explicitly state that, by default, _id is generated as a UUIDv7 string.
- If needed for backward compatibility, consider an optional toggle or injectable ID generator (ObjectId vs UUID) to avoid surprising downstreams.
Run this repo audit to surface potential hotspots that assume ObjectId or rely on _id ordering:
Expected outcome:
- No remaining hard assumptions on ObjectId type.
- Any usage that sorts by _id for recency is reviewed, especially in collections with mixed _id types.
Also applies to: 500-500
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for ObjectId usages (construction, type hints, or imports)..."
rg -nP -C3 --type=php '\bMongoDB\\BSON\\ObjectId\b|new\s+ObjectId\s*\(|\bObjectId\b'
echo
echo "Searching for code relying on _id sort order (descending or ascending)..."
rg -nP -C3 --type=php "(?:'|\")sort(?:'|\")\s*=>\s*\[\s*(?:'|\")_id(?:'|\")\s*=>\s*-?1"
echo
echo "Searching for lastDocument call sites..."
rg -nP -C3 --type=php '\blastDocument\s*\('Length of output: 2539
Action Required: Harden UUID _id assignment & Audit ObjectId dependencies
We’ve verified that:
- The code now defaults
_idto a UUID string via$docObj->_id ??= $this->createUuid(). lastDocument()still sorts by_id('sort' => ['_id' => -1]), which will mix ObjectId vs UUID order.- Several tests in
tests/MongoTest.phpexplicitly construct or type‐hintMongoDB\BSON\ObjectId, so they’ll break against UUID IDs.
Please address the following before merging:
• Harden “empty id” check
Replace both coalesce assignments in src/Client.php with an explicit guard:
// Before (around line 479 and in insertMany)
- $docObj->_id ??= $this->createUuid();
+ if (!isset($docObj->_id) || $docObj->_id === '' || $docObj->_id === null) {
+ $docObj->_id = $this->createUuid();
+ }• Audit ObjectId assumptions
- Update or remove references in
tests/MongoTest.phpthat donew ObjectId(...)or expect_idto be an ObjectId. - Review any production filtering by
_id(e.g.find(…, ['_id' => new ObjectId($id)])) to ensure UUID strings are handled. - Consider migrating existing collections or providing a compatibility layer if consumers still rely on BSON ObjectIds.
• Review recency logic
- Sorting by
_idis no longer a reliable proxy for creation time when mixing types. Consider adding or using an explicitcreatedAttimestamp.
• API & docs updates
- Update PHPDoc for
insert()/insertMany()to state that_iddefaults to a UUIDv7 string. - Optionally inject the ID generator (ObjectId vs UUID) via configuration or a strategy interface to maintain backward compatibility.
📝 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.
| $docObj->_id ??= $this->createUuid(); | |
| // Replace this line in both insert() (around line 479) and insertMany(): | |
| // $docObj->_id ??= $this->createUuid(); | |
| if (!isset($docObj->_id) || $docObj->_id === '' || $docObj->_id === null) { | |
| $docObj->_id = $this->createUuid(); | |
| } |
🤖 Prompt for AI Agents
In src/Client.php around line 479, replace the null coalesce shorthand id
assignment with an explicit check that treats empty/invalid values (null, '', 0)
as missing and only then assigns a generated UUID; audit and update
tests/MongoTest.php to remove or adapt any new ObjectId(...) usage or
expectations of BSON ObjectId types, update production code paths that cast or
filter by new ObjectId($id) to accept string UUIDs (or branch to construct
ObjectId when needed), stop relying on sorting by '_id' for recency and add/use
an explicit createdAt timestamp for ordering, update PHPDoc for
insert()/insertMany() to state _id defaults to a UUIDv7 string, and consider
adding a configurable ID-generator/strategy interface to toggle between ObjectId
and UUID for backward compatibility.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
…tion is only applied when necessary This update modifies the document ID assignment logic in the Client class to check for existing IDs before generating a new UUID. Additionally, it corrects the expected length of UUIDs in the MongoTest class, ensuring consistency in ID handling across tests.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/MongoTest.php (3)
65-65: Strengthen UUID assertion: validate format and version, not just length.Length-only can pass false positives. Assert RFC-4122 validity and version 7.
Apply this diff:
- self::assertEquals(36, strlen($id)); + self::assertTrue(\Ramsey\Uuid\Uuid::isValid($id)); + self::assertSame(7, \Ramsey\Uuid\Uuid::fromString($id)->getVersion());Add the import near the other uses:
use Ramsey\Uuid\Uuid;
70-72: LGTM on querying by string _id; add a round‑trip assertion.This correctly aligns with string UUID storage. Optionally verify the returned _id matches.
- $doc = $this->getDatabase()->find('movies', ['_id' => $id])->cursor->firstBatch ?? []; - self::assertCount(1, $doc); + $doc = $this->getDatabase()->find('movies', ['_id' => $id])->cursor->firstBatch ?? []; + self::assertCount(1, $doc); + self::assertSame($id, (string)($doc[0]->_id ?? ''));
75-76: Repeat robust UUID checks here as well.Mirror the validity + version checks to avoid length-only assertions.
- self::assertEquals(36, strlen($id)); + self::assertTrue(\Ramsey\Uuid\Uuid::isValid($id)); + self::assertSame(7, \Ramsey\Uuid\Uuid::fromString($id)->getVersion());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Client.php(4 hunks)tests/MongoTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Client.php
🧰 Additional context used
🧬 Code graph analysis (1)
tests/MongoTest.php (1)
src/Client.php (2)
find(619-629)insert(471-489)
⏰ 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)
This commit introduces a new method
createUuidin the Client class to generate UUIDs using the Ramsey UUID library. The document ID assignment in the insert methods has been updated to use UUIDs instead of MongoDB's ObjectId, enhancing the uniqueness of document identifiers.Summary by CodeRabbit