chore: add unique index on organization slug in MongoDB #40198
Conversation
WalkthroughThis update modifies how uniqueness is enforced on the Changes
Sequence Diagram(s)sequenceDiagram
participant Migration as Migration073_AddUniqueIndexOnOrganizationSlug
participant Template as MongoTemplate
participant DB as MongoDB
Migration->>Template: Drop existing index on "slug" (if any)
Template->>DB: Request index drop
DB-->>Template: Confirm drop (or no-op)
Migration->>Template: Create unique index on "slug"
Template->>DB: Ensure unique index creation
DB-->>Template: Return index creation result
Template-->>Migration: Operation completed
Migration->>Migration: Log success or throw exception on failure
Poem
✨ 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:
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
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: 0
🧹 Nitpick comments (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration073_AddUniqueIndexOnOrganizationSlug.java (1)
16-16: Consider adding an author name.The
authorattribute in the@ChangeUnitannotation is empty. Adding the author's name improves traceability and makes it easier to know who to contact if there are questions about this migration in the future.- @ChangeUnit(order = "073", id = "add-unique-index-on-organization-slug", author = "") + @ChangeUnit(order = "073", id = "add-unique-index-on-organization-slug", author = "abhvsn")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Organization.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration073_AddUniqueIndexOnOrganizationSlug.java(1 hunks)
🔇 Additional comments (5)
app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Organization.java (1)
22-22: Removal of @unique annotation is appropriate when moving constraint to database levelThe removal of the
@Uniqueannotation from theslugfield is a good architectural change. This shifts uniqueness enforcement from the application level to the database level via the MongoDB unique index mentioned in the associated migration. Database-level constraints generally provide stronger guarantees and better performance compared to application-level validations.Make sure the migration class (
Migration073_AddUniqueIndexOnOrganizationSlug) properly handles potential duplicate values already in the database to avoid migration failures.app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration073_AddUniqueIndexOnOrganizationSlug.java (4)
25-28: The empty rollback method looks good.Having an empty rollback method with a clear comment is appropriate since there's no need to roll back an index creation - dropping the index would be handled in a separate migration if needed.
31-46: Well-structured migration implementation.The migration properly handles idempotence by first dropping any existing index and then creating a new unique one. The error handling with logging and exception re-throwing is appropriate for a migration task.
38-38: Good use of background indexing.Setting the index creation to run in the background with
.background()is a good practice as it won't block database operations, especially important in production environments.
1-48:❓ Verification inconclusive
Verify data integrity before deployment.
This migration adds a unique constraint on the Organization slug field. Before deploying to production, ensure there are no duplicate slug values in the Organization collection to prevent migration failures.
🏁 Script executed:
#!/bin/bash # Check for duplicate organization slugs that would violate the unique constraint echo "Checking for duplicate organization slugs..." mongosh --eval ' db = db.getSiblingDB("appsmith"); const duplicates = db.organization.aggregate([ { $group: { _id: "$slug", count: { $sum: 1 } } }, { $match: { count: { $gt: 1 } } } ]).toArray(); if (duplicates.length > 0) { print("WARNING: Found duplicate slugs that will violate the unique constraint:"); duplicates.forEach(d => print(`Slug "${d._id}" appears ${d.count} times`)); } else { print("No duplicate slugs found. Migration should proceed without issues."); } 'Length of output: 706
Action Required: Verify Duplicate Organization Slugs
This migration adds a unique index on the Organization slug field. Before deploying, please verify that your production database does not contain duplicate slugs—which would lead to migration failures. The duplicate-check script attempted to run with
mongoshbut failed (command not found).
- Verification Steps:
- Ensure your environment has the appropriate MongoDB shell (such as
mongoshor an equivalent) installed.- Manually verify that no duplicate slugs exist in the Organization collection, or update the script to use a available command.
abhvsn
changed the title
github-actions
bot
added
skip-changelog
Failed server tests
|
…40198) ## Description - Introduced a new migration to add a unique index on the 'slug' field of the Organization collection. - Ensured idempotence by dropping the existing index before creating the new one. - Added logging for successful index creation and error handling for potential issues during the process. ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!CAUTION] > If you modify the content in this section, you are likely to disrupt the CI result for your PR. <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Updated how organization identifiers are validated to ensure each remains unique. - **Chores** - Introduced a database update that enforces uniqueness for organization identifiers, enhancing data consistency and preventing duplicate entries. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit