This PR adds database constraints that guarantee no two books can have the same slug, and updates the application logic for checking slugs and handling duplicate key errors (e.g. trying to insert a record that violates a unique constraint).
We encountered this issue while using S3 file storage. Even though this issue will probably not affect installations using the filesystem-based storage, these updates should guarantee they don't.
Here's a complete rundown of the changes included in the PR.
Database Changes
Added unique constraint to the scalar_db_books table on the slug column. This enforces the existing application logic and serves as an additional layer of protection.
Added unique constraint to the scalar_db_content table on the (book_id, slug) columns. Given a book, the slug should be unique. This simply enforces the existing application logic and adds a layer of protection.
Code Changes
Moved the slug_exists() and safe_slug() code from MY_Model.php into the page_model.php, where they are being used. Especially since the book_model.php simply overrides the slug_exists() method. The intended use is the same, but their implementations are different, so it seems best to put them in their respective classes.
Fixed the slug_exists() method in book_model.php so that it checks the database for the existence of a slug, rather than checking the filesystem for a folder. This is the crux of the issue when using an alternative storage backend. It doesn't seem like there should be any situation where a slug folder would exist on the filesystem without a corresponding record in the database.
Added error handling in case the db->insert() method fails due to a unique key constraint error. It should never get there, because the application is checking, but just in case, an error message will be displayed to the user.
Addendums
You may notice that the length of the slug column has been modified in scalar_store_utf8mb4.sql. This was done to account for the maximum key length in MySQL MyISAM, which is 1000 bytes. The utf8 character set (actually an alias for utf8mb3), is only a 3-byte character encoding while utf8mb4 is a 4-byte character encoding. So for the scalar_db_books table, a length of 250 is necessary, and for scalar_db_content, a length of 249 is necessary since it also accounts for the book_id (INT = 4 bytes), which is included in the unique key.
craigdietrich
commented
4 years ago
This PR adds database constraints that guarantee no two books can have the same slug, and updates the application logic for checking slugs and handling duplicate key errors (e.g. trying to insert a record that violates a unique constraint).
We encountered this issue while using S3 file storage. Even though this issue will probably not affect installations using the filesystem-based storage, these updates should guarantee they don't.
Here's a complete rundown of the changes included in the PR.
Database Changes
scalar_db_bookstable on theslugcolumn. This enforces the existing application logic and serves as an additional layer of protection.scalar_db_contenttable on the(book_id, slug)columns. Given a book, the slug should be unique. This simply enforces the existing application logic and adds a layer of protection.Code Changes
slug_exists()andsafe_slug()code fromMY_Model.phpinto thepage_model.php, where they are being used. Especially since thebook_model.phpsimply overrides theslug_exists()method. The intended use is the same, but their implementations are different, so it seems best to put them in their respective classes.slug_exists()method inbook_model.phpso that it checks the database for the existence of a slug, rather than checking the filesystem for a folder. This is the crux of the issue when using an alternative storage backend. It doesn't seem like there should be any situation where a slug folder would exist on the filesystem without a corresponding record in the database.db->insert()method fails due to a unique key constraint error. It should never get there, because the application is checking, but just in case, an error message will be displayed to the user.Addendums
slugcolumn has been modified inscalar_store_utf8mb4.sql. This was done to account for the maximum key length in MySQL MyISAM, which is 1000 bytes. The utf8 character set (actually an alias for utf8mb3), is only a 3-byte character encoding while utf8mb4 is a 4-byte character encoding. So for thescalar_db_bookstable, a length of 250 is necessary, and forscalar_db_content, a length of 249 is necessary since it also accounts for the book_id (INT= 4 bytes), which is included in the unique key.Thoughts, questions, concerns?