Issue #3510939 by mikelutz, balazswmann, godotislate, mxr576: Avoid registering multiple batch sets with node_access_rebuild()'s batch mode

Author: catch

lib/Drupal/Core/Batch/BatchBuilder.php

@@ -19,6 +19,23 @@
  * }
  * batch_set($batch_builder->toArray());
  * @endcode
+ *
+ * To prevent duplicate batches from being created inadvertently in the same
+ * page request, you can use ::isSetIdRegistered and ::registerSetId to check
+ * and see if this batch has been built before.
+ * @code
+ * if (!BatchBuilder::isSetIdRegistered('my_unique_id')) {
+ *   $batch_builder = (new BatchBuilder())
+ *     ->registerSetId('my_unique_id')
+ *     ->setTitle(t('Batch Title'))
+ *     ->setFinishCallback('batch_example_finished_callback')
+ *     ->setInitMessage(t('The initialization message (optional)'));
+ *   foreach ($ids as $id) {
+ *     $batch_builder->addOperation('batch_example_callback', [$id]);
+ *   }
+ *   batch_set($batch_builder->toArray());
+ * }
+ * @endcode
  */
 class BatchBuilder {
 
@@ -110,6 +127,13 @@ class BatchBuilder {
    */
   protected $queue;
 
+  /**
+   * A static array of custom batch ids.
+   *
+   * @var string[]
+   */
+  protected static array $registeredSetIds = [];
+
   /**
    * Sets the default values for the batch builder.
    */
@@ -317,6 +341,32 @@ public function addOperation(callable $callback, array $arguments = []) {
     return $this;
   }
 
+  /**
+   * Checks if a set ID has been registered during this request.
+   *
+   * @param string $setId
+   *   The set ID to check.
+   *
+   * @return bool
+   *   True if this set ID has been registered.
+   */
+  public static function isSetIdRegistered(string $setId): bool {
+    return isset(static::$registeredSetIds[$setId]);
+  }
+
+  /**
+   * Registers a set ID for this batch.
+   *
+   * @param string $setId
+   *   The set ID to register.
+   *
+   * @return $this
+   */
+  public function registerSetId(string $setId): self {
+    static::$registeredSetIds[$setId] = TRUE;
+    return $this;
+  }
+
   /**
    * Converts a \Drupal\Core\Batch\Batch object into an array.
    *

modules/node/node.module

@@ -532,7 +532,9 @@ function node_access_needs_rebuild($rebuild = NULL) {
  *   has a large number of nodes). hook_update_N() and any form submit handler
  *   are safe contexts to use the 'batch mode'. Less decidable cases (such as
  *   calls from hook_user(), hook_taxonomy(), etc.) might consider using the
- *   non-batch mode. Defaults to FALSE.
+ *   non-batch mode. Defaults to FALSE. Calling this method multiple times in
+ *   the same request with $batch_mode set to TRUE will only result in one batch
+ *   set being added.
  *
  * @see node_access_needs_rebuild()
  */
@@ -544,11 +546,14 @@ function node_access_rebuild($batch_mode = FALSE) {
   // Only recalculate if the site is using a node_access module.
   if (\Drupal::moduleHandler()->hasImplementations('node_grants')) {
     if ($batch_mode) {
-      $batch_builder = (new BatchBuilder())
-        ->setTitle(t('Rebuilding content access permissions'))
-        ->addOperation('_node_access_rebuild_batch_operation', [])
-        ->setFinishCallback('_node_access_rebuild_batch_finished');
-      batch_set($batch_builder->toArray());
+      if (!BatchBuilder::isSetIdRegistered(__FUNCTION__)) {
+        $batch_builder = (new BatchBuilder())
+          ->setTitle(t('Rebuilding content access permissions'))
+          ->addOperation('_node_access_rebuild_batch_operation', [])
+          ->setFinishCallback('_node_access_rebuild_batch_finished')
+          ->registerSetId(__FUNCTION__);
+        batch_set($batch_builder->toArray());
+      }
     }
     else {
       // Try to allocate enough time to rebuild node grants

modules/node/tests/src/Kernel/NodeAccessTest.php

@@ -156,4 +156,19 @@ public function testQueryWithBaseTableJoin(): void {
     $this->assertEquals(4, $query->countQuery()->execute()->fetchField());
   }
 
+  /**
+   * Tests that multiple calls to node_access_rebuild only result in one batch.
+   */
+  public function testDuplicateBatchRebuild(): void {
+    $this->enableModules(['node_access_test']);
+    $batch = batch_get();
+    $this->assertEmpty($batch);
+    node_access_rebuild(TRUE);
+    $batch = batch_get();
+    $this->assertCount(1, $batch['sets']);
+    node_access_rebuild(TRUE);
+    $batch = batch_get();
+    $this->assertCount(1, $batch['sets']);
+  }
+
 }

tests/Drupal/Tests/Core/Batch/BatchBuilderTest.php

@@ -248,6 +248,19 @@ public function testAddOperation(): void {
     ], $batch['operations']);
   }
 
+  /**
+   * Tests registering IDs of built batches.
+   *
+   * @covers ::isSetIdRegistered
+   * @covers ::registerSetId
+   */
+  public function testRegisterIds(): void {
+    $setId = $this->randomMachineName();
+    $this->assertFalse(BatchBuilder::isSetIdRegistered($setId));
+    (new BatchBuilder())->registerSetId($setId);
+    $this->assertTrue(BatchBuilder::isSetIdRegistered($setId));
+  }
+
   /**
    * Empty callback for the tests.
    *