Simplify unique constraint for documents #1098
Conversation
WalkthroughThe changes in this pull request involve modifications to the index definition for the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
🧹 Outside diff range and nitpick comments (1)
server/backend/database/mongo/indexes.go (1)
114-116: Consider documenting the uniqueness constraint changeThe change in uniqueness behavior is significant enough to warrant documentation, especially regarding:
- The rationale behind removing the partial filter
- The impact on document lifecycle management
- Any required changes to application code that deals with document deletion
Consider adding a comment above the index definition explaining the uniqueness constraint and its implications.
name: ColDocuments, + // Unique constraint ensures no duplicate project_id/key combinations exist, + // regardless of the document's removed_at status indexes: []mongo.IndexModel{{
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
server/backend/database/mongo/indexes.go(1 hunks)
🔇 Additional comments (1)
server/backend/database/mongo/indexes.go (1)
114-116: Verify data integrity before applying this change
The change to include removed_at in the unique index while removing the partial filter expression significantly alters the uniqueness constraint behavior:
- Previously: Allowed duplicate project_id/key combinations for soft-deleted documents
- Now: Enforces uniqueness across all documents regardless of removed_at status
Run this script to check for potential conflicts:
Consider the following before proceeding:
- Migration Strategy: Existing data might need cleanup if there are soft-deleted documents with duplicate project_id/key combinations
- Application Logic: Verify if any features depend on the ability to have duplicates for soft-deleted documents
- Rollback Plan: Have a plan to revert if issues are discovered post-deployment
Would you like help creating a migration script to identify and resolve potential conflicts?
✅ Verification successful
Let me gather more information about the document collection's usage and soft deletion patterns.
Let me search for more specific patterns related to document uniqueness and soft deletion behavior.
Based on the search results and code analysis, I can now generate a final response:
Unique index change appears safe to implement
The change to include removed_at in the unique index while removing the partial filter is safe because:
- The codebase shows no patterns of relying on duplicate project_id/key combinations for soft-deleted documents
- The only reference to
removed_atuniqueness constraints is in design docs which align with this change - Document operations (Insert/Update/Find) don't show any logic depending on the ability to have duplicates when soft-deleted
The change actually improves data integrity by preventing duplicate project_id/key combinations even among soft-deleted documents, which is a stronger consistency guarantee.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential duplicate project_id/key combinations that were previously allowed
# but would violate the new constraint
# Search for any MongoDB queries that might be affected by this change
rg -A 3 "removed_at.*null"
# Search for soft-delete related code that might assume duplicates are allowed
ast-grep --pattern 'removed_at = $_ WHERE project_id = $_ AND key = $_'
Length of output: 661
Script:
#!/bin/bash
# Search for document collection operations and soft deletion patterns
# Look for document creation/update logic
rg -A 5 "ColDocuments.*Insert"
rg -A 5 "ColDocuments.*Update"
# Look for soft deletion implementation
rg -A 5 "removed_at.*time\."
rg -A 5 "SoftDelete"
# Check if there are any tests related to document uniqueness
rg -A 5 "TestDocument.*Unique"
Length of output: 3179
Script:
#!/bin/bash
# Search for document uniqueness checks and constraints
rg -A 5 "duplicate.*document" -i
rg -A 5 "ErrDuplicate"
# Look for document find operations that might be affected
rg -A 5 'ColDocuments.*Find.*removed_at'
# Search for any migration scripts or schema changes
fd -e js -e go -H migration
rg -A 5 "migrate.*document" -i
Length of output: 67121
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1098 +/- ##
=======================================
Coverage 46.84% 46.84%
=======================================
Files 84 84
Lines 12256 12256
=======================================
Hits 5741 5741
Misses 5939 5939
Partials 576 576 ☔ View full report in Codecov by Sentry. |
What this PR does / why we need it:
Simplify unique constraint for documents
This commit refined the project's document indexing strategy to accommodate unique key constraints while handling soft deletions. Previously, this was solved by applying partial index and unique constraint, but partial index could not be used when sharding. By adding the
removed_atfield into the uniqueness check, we now ensure only one active document exists at a time.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist:
Summary by CodeRabbit
New Features
ColDocumentscollection to enforce uniqueness across specified keys.Bug Fixes
ColDocumentscollection.