Allow usage records to be registered in background

I'd really like to restrict this as much as possible, only leaving it to people who are really aware of the side effects it might have. In that context, how about we add the setting + the behavior if the setting is turned on, but don't expose it on the settings form?

This way the only way to enable the cron-based processing would be through CMI, programmatically or drush. There is already a similar mechanism if you want to have the warning message in a custom form which is not a delete form (see entity_usage.settings.delete_warning_form_classes).

Here is a patch for adding the new/deleted entities to the queue. This can be set by adding a line to the settings file. Read the docs for more info.

Thanks! Some minor comments:

1. +++ b/README.txt
   @@ -49,6 +49,19 @@ usage of entities on your site.
   +The module support tracking entity usage via a queue. When the entity usage is
   

   typo: s/support/supports

2. +++ b/README.txt
   @@ -49,6 +49,19 @@ usage of entities on your site.
   +This means some references between entities could be missing. Only use this setting
   +when you are sure there are no automatic processes using the tracking information
   

   Nitpick: These 2 lines span over 80 chars.

3. +++ b/README.txt
   @@ -49,6 +49,19 @@ usage of entities on your site.
   +Since this is for advanced users only, this setting is not exposed in the UI.
   +This can be enabled through the settings.php by adding the following line:
   +
   +$config['entity_usage.settings']['queue_tracking'] = TRUE;
   

   Technically this isn't the only way to set it... :) How about we rephrase this with something along the lines of "This can be enabled by importing config, setting the value programmatically with custom code, or overriding it at runtime by adding the following line to your settings.php file:" ?

4. +++ b/entity_usage.module
   @@ -31,35 +31,80 @@ function entity_usage_help($route_name, RouteMatchInterface $route_match) {
   +  if (\Drupal::configFactory()->get('entity_usage.settings')->get('queue_tracking')) {
   

   We need to add queue_tracking to the entity_usage.settings schema as well.

5. +++ b/entity_usage.module
   @@ -31,35 +31,80 @@ function entity_usage_help($route_name, RouteMatchInterface $route_match) {
   +      'operation' => 'insert',
   

   Can insert be a constant in the queue class instead?

6. +++ b/entity_usage.module
   @@ -31,35 +31,80 @@ function entity_usage_help($route_name, RouteMatchInterface $route_match) {
   +      'operation' => 'update',
   

   Same here, I believe a constant would be better.

7. +++ b/entity_usage.module
   @@ -31,35 +31,80 @@ function entity_usage_help($route_name, RouteMatchInterface $route_match) {
   +      'operation' => 'predelete',
   

   Same here.

8. +++ b/entity_usage.module
   @@ -31,35 +31,80 @@ function entity_usage_help($route_name, RouteMatchInterface $route_match) {
   +      'operation' => 'translation_delete',
   

   Same here.

9. +++ b/entity_usage.module
   @@ -31,35 +31,80 @@ function entity_usage_help($route_name, RouteMatchInterface $route_match) {
   +      'operation' => 'revision_delete',
   

   Same here.

10. +++ b/src/Plugin/QueueWorker/EntityUsageTracker.php
    @@ -0,0 +1,97 @@
    +      case 'insert':
    ...
    +      case 'update':
    ...
    +      case 'predelete':
    ...
    +      case 'translation_delete':
    ...
    +      case 'revision_delete':
    

    Here too we would use the constants.

And one relevant consideration:

• +++ b/src/Plugin/QueueWorker/EntityUsageTracker.php
  @@ -0,0 +1,97 @@
  +    $entity = $this->entityTypeManager->getStorage($data['entity_type'])->load($data['entity_id']);
  

  I wonder the side effects of this deferred loading. Kind of an edge case, but what if someone edited the entity between the moment this item was added to the queue, and the moment the item is processed? We might be trying to track something that doesn't actually correspond to what happened in the first save, since the entity is now different. I'd assume it would be safer to add the whole entity object to the queue item, at least this way each processed item will register whatever was being saved by the user. But I don't know if serializing in the queue item a whole entity object has any side effects at all...

Now that I think of it, I realize maybe an alternative approach could be to avoid the use of the queue processing, and rely on a DestructableInterface service. I would still keep the setting (which could be called something like "background_tracking" instead of "queue_tracking"), and when it's switched on, instead of triggering the tracking synchronously, we just register the tracking to be done at destruction time, which is after the request was sent back to the browser. This has the advantage of not needing to rely on cron running, and significantly reduces the time during which there would be a mismatch in usage numbers. (it would actually only be out-of-date during the time needed for the tracking operation itself to take place).

What do you think?

Here is the new version of the patch. Removed the queue method, and use the background tracking instead.

Thanks! I think the approach in #8 is much better. It would be great to have test coverage for the 5 operations we are affecting, making sure they work as expected when working in "background mode".

Re #9, I didn't know about that, thanks for sharing! Yes it's probably better to make this clarification explicit in the readme file.

This looks great!
I have issues on a website where the process is so long that I get a Gateway Timeout and lost all my edits on the content. I have to disable the module to be able to update some nodes, that's painful.

I'd love to see this commited!
I feel that the setting should be added in the settings form and have an upgrade path to preserve BC, though. I could do this but I'd need some guidance to write the tests asked in #10.

I gave this a harder look and even if it does fix the Gateway Timeout issue, it still breaks the max_execution_time.
Would it make sense to delay a bit more those calculations to use a Queue instead?

Is anyone successfully using this patch in Production (or something similar)? The patch looks good to me, just looking for some assurance before we proceed.

Here is a patch against 8.x-2.0-beta6 based on the patch to use a Queue instead of the background track at the end of the request. It checks that storage and entity can be loaded before processing. It might be paranoic, but we are loading things later, so we must check if those things can load.

Rerolled #12

Apparently the Web IDE created the new file in another location than I expected. Fixed.

FYI this patch is also comitted to the entity_track module, which works with the 4.x branch of entity_usage.

Reroll

I've changed @BetoAveiga 's patch to make it work with 2.0.0-beta12 (this patch uses the Drupal queue for updating the usage of entities).

I wonder if the performance issue here still the case. The entity usage plugins are now much more performant.

I think this might still be desirable but for me it is probably not a stable blocker anymore.