statamic / eloquent-driver

Provides support for storing your Statamic data in a database, rather than flat files.
https://statamic.dev/tips/storing-content-in-a-database
MIT License
104 stars 75 forks source link

Unique-Constraint on container, folder and basename #256

Closed andreas-eisenmann closed 6 months ago

andreas-eisenmann commented 6 months ago

There's a multi-column index on container, folder and basename in assets_meta:

https://github.com/statamic/eloquent-driver/blob/master/database/migrations/2024_03_07_100000_create_asset_table.php#L22C1-L23C1

$table->index(['container', 'folder', 'basename']);

But there's no unique constraint on this combination of fields. Is there any reason for the missing unique constraint?

When assets are being uploaded via CP, there's a logic inside statamic which ensures uniqueness of asset names. But e.g. if you import data and directly save assets via \Statamic\Eloquent\Assets\Asset in your database, there's neither a validation in the php-side logic, nor a unique constraint in the database which would prevent uploading duplicate assets.

So now I have duplicates in my assets_meta table.

When I call:

$file->move('foo-bar');

the wrong file will be moved because in https://github.com/statamic/eloquent-driver/blob/36ac24d5b23e4aebec0cb0fb0c1e3096ebb46a8e/src/Assets/Asset.php you assume (correctly) that you can identify an asset by its combination of container, folder and basename.

Are there any arguments against changing the above index to a unique index?

$table->unique(['container', 'folder', 'basename']);
ryanmitchell commented 6 months ago

No objections - they should be unique.

ryanmitchell commented 6 months ago

I've opened a PR with the change: https://github.com/statamic/eloquent-driver/pull/257 Note, this will only affect new installs. You'd need to create your own migration to enable it on your existing install.

andreas-eisenmann commented 6 months ago

Thanks a lot! 🚀

You'd need to create your own migration to enable it on your existing install.