Alternative approach to unique queue using upsert

Overall the ideal looks good, but I believe it needs more testing and some attention given to the areas noted below:

1. +++ b/src/Plugin/Purge/Queue/DatabaseUniqueUpsertQueue.php
   @@ -0,0 +1,130 @@
   +      // calculate them back by applying subtraction.
   

   My understanding of the docs is that "execute()" will return not ID, but count of created/updated items, and we can't use that to get IDs of the queue items.

2. +++ b/src/Plugin/Purge/Queue/DatabaseUniqueUpsertQueue.php
   @@ -0,0 +1,130 @@
   +    $schema['unique keys']['hash'] = ['hash'];
   

   \Drupal\Core\Database\Query\Upsert doc say "This class can only be used with a table with a single unique index." and MySQL docs https://dev.mysql.com/doc/refman/8.0/en/insert-on-duplicate.html suggest "In general, you should try to avoid using an ON DUPLICATE KEY UPDATE clause on tables with multiple unique indexes." so I believe we can't extent default DB queue here...

Thanks for the review.

I have taken a look through existing plugins and tests and could not see a strong reason why the hash can not be used as the item id. While all the plugins provided by purge use an integer, the only non integer uses I could find were in tests - e.g https://git.drupalcode.org/project/purge/-/blob/8.x-3.x/tests/src/Kernel... So while it would be different, it appears it could still work to get away from the issue you mention with extending the base schema.

This change does introduce duplicating a lot of methods from the parent plugin to remove a lot of casting the item_id to int.

I.e the database queue plugin casts the item_id to an integer in several places: claimItem, claimItemMultiple, selectPage and releaseItemMultiple. Removing this in our plugin appears to work as expected.

• https://git.drupalcode.org/project/purge/-/blob/8.x-3.x/src/Plugin/Purge...
• https://git.drupalcode.org/project/purge/-/blob/8.x-3.x/src/Plugin/Purge...
• https://git.drupalcode.org/project/purge/-/blob/8.x-3.x/src/Plugin/Purge...
• https://git.drupalcode.org/project/purge/-/blob/8.x-3.x/src/Plugin/Purge...

I've added a test extending the base plugin from the purge module and adding a test specifically for adding multiple items via createItem and createItemMultiple.

In doing so I tweaked the get hash method to handle any data. Even though this is a plugin which in practise is used only with invalidation data from the proxy item class, the interface does specify that data can be anything.

Thanks Eric!

This looks good to me, even with a lot of code duplication.

I've done a bit of improvement in the patch:

1. Fixed namespace
2. Flipped compared arguments

Tests are passing locally.

Missed files for previous comment...

We have been using this patch in production since @RoSk0's improvements around 8 months ago.

We have not found any issues with it, and it has greatly improved the performance of the application when large amounts of invalidation occur such using bulk uploads or bulk editing content.

Great work. Thank both. Just created a branch and pushed your patches.

I don't like the code duplication, mainly because it needs to be keep updated. May you provide a patch to purge and remove the casting to integer?

Anyway I'll merge this. Thanks again!

Thanks @jonhattan

I've opened 📌 Allow DatabaseQueue to be extended with non integer based ids Needs review - thanks for suggesting that - completely agree it would be awesome to reduce code the code duplication.

Automatically closed - issue fixed for 2 weeks with no activity.