appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.
https://www.appsmith.com
Apache License 2.0
34.21k stars 3.7k forks source link

[Task]: Add mongoDB unique index for slug field in Tenant collection #33377

Closed NilanshBansal closed 4 months ago

NilanshBansal commented 5 months ago

SubTasks

The Tenant domain slug field has an @Unique annotation but there is no unique index for slug field in the mongo database. We should add an index in the database to maintain consistentency.

image
akshayvijayjain commented 5 months ago

@NilanshBansal , we plan to work on this, can we take it up?

NilanshBansal commented 5 months ago

@akshayvijayjain yes definitely.

Naveen-Goud commented 5 months ago

@NilanshBansal can we use@Indexed annotation instead of @Unique, to gererate unique slug id in db. The difference between two annotations is:

The @Unique annotation might be mistakenly thought to enforce uniqueness in the database, but this is not the case. There is no standard @Unique annotation in Spring Data or Java for enforcing database-level constraints. The @Unique annotation you mentioned could be from a custom or non-standard library, but it is not required and not commonly used for this purpose.

@Indexed(unique = true): This annotation tells Spring Data MongoDB to create a unique index on the slug field. This ensures that each document in the Tenant collection will have a unique slug value. MongoDB will enforce this constraint at the database level.

below is the screenshot with the updated change

Screenshot from 2024-05-20 13-07-13

can you review and give feedback whether to use this annotation or not?

nidhi-nair commented 5 months ago

@Naveen-Goud , agreed with your analysis. However, as a practice, we rely on migrations to create indices in the database and not annotations. The ask from this issue is that we create such a migration for this field as well. The reason we opt for migrations and not annotations is that the annotation is a part of compiled code. Spring auto-magically creates the index if it finds such a reference in code. However, if the annotation were to be slipped as part of any refactor, future "fresh DBs" would not have this index at all. This is much harder to do with migrations.

Naveen-Goud commented 5 months ago

hello @nidhi-nair , thanks for the response , I couldn't understand last statement regarding "fresh db's". but I have worked on task , and replaced with @Indexed , and also made a migration file which generate a unigue index for the slug field. image

this migration wil make the slug field unique id and will not accept any id which are similar , if the data s similar then it returns error

Screenshot from 2024-05-23 17-43-13

and also because of the migration step , the slud id are arranged in ascending order.

I also have a question , if we insert data with one field , the slug is considering null as id for first time and not allowing to create another object with inserting one field.

does the task also says that id's should be created in db when we try insert data without slug field?

NilanshBansal commented 4 months ago

This seems like a premature optimization as currently we have not moved to multi-tenancy. Right now, there is only 1 document in the Tenant collection always as there is only a single Tenant. Adding index to that does not make much sense right now. When we implement multi-tenancy we might figure out that ensuring uniqueness only on this field might not suffice. We might need to add other fields along with it to make a composite unique index. Closing this task right now, can reopen when we move to multi-tenancy.