diff --git a/core/core.services.yml b/core/core.services.yml
index 193ffbb7dacd4b7749586672d0cb20293cabd0b9..aa3961dff4496f987f64d03c80d3cd42fc84ed84 100644
--- a/core/core.services.yml
+++ b/core/core.services.yml
@@ -1548,7 +1548,7 @@ services:
Drupal\Core\Utility\Token: '@token'
batch.storage:
class: Drupal\Core\Batch\BatchStorage
- arguments: ['@database', '@session', '@csrf_token']
+ arguments: ['@database', '@session', '@csrf_token', '@datetime.time']
tags:
- { name: backend_overridable }
lazy: true
diff --git a/core/includes/form.inc b/core/includes/form.inc
index 4f353d008826baa87ed1c37bf7b5c16a0d5e8376..d6db3ddeec91fc11e75e7f32b6e1756487f41c44 100644
--- a/core/includes/form.inc
+++ b/core/includes/form.inc
@@ -6,6 +6,8 @@
*/
use Drupal\Component\Utility\UrlHelper;
+use Drupal\Core\Batch\BatchStorageInterface;
+use Drupal\Core\Database\IntegrityConstraintViolationException;
use Drupal\Core\Render\Element;
use Drupal\Core\Render\Element\RenderElement;
use Drupal\Core\Template\Attribute;
@@ -897,9 +899,26 @@ function batch_process($redirect = NULL, Url $url = NULL, $redirect_callback = N
// environments.
\Drupal::moduleHandler()->alter('batch', $batch);
- // Assign an arbitrary id: don't rely on a serial column in the 'batch'
- // table, since non-progressive batches skip database storage completely.
- $batch['id'] = \Drupal::database()->nextId();
+ // Assign an id to progressive batches. Non-progressive batches skip
+ // database storage completely.
+ try {
+ $batch['id'] = $batch['progressive'] ? \Drupal::service(BatchStorageInterface::class)->getId() : 'non-progressive';
+ }
+ catch (IntegrityConstraintViolationException $e) {
+ // @todo this is here to support the update path to deprecate
+ // Connection::nextId(). Remove in Drupal 11.
+ $connection = \Drupal::database();
+ $max_bid = (int) $connection->query('SELECT MAX([bid]) FROM {batch}')->fetchField();
+ $batch['id'] = $max_bid + 1;
+ $connection->insert('batch')
+ ->fields([
+ 'bid' => $batch['id'],
+ 'timestamp' => \Drupal::time()->getRequestTime(),
+ 'token' => '',
+ 'batch' => NULL,
+ ])
+ ->execute();
+ }
// Move operations to a job queue. Non-progressive batches will use a
// memory-based queue.
@@ -931,7 +950,7 @@ function batch_process($redirect = NULL, Url $url = NULL, $redirect_callback = N
}
// Store the batch.
- \Drupal::service('batch.storage')->create($batch);
+ \Drupal::service(BatchStorageInterface::class)->create($batch);
// Set the batch number in the session to guarantee that it will stay alive.
$_SESSION['batches'][$batch['id']] = TRUE;
diff --git a/core/lib/Drupal/Core/Batch/BatchStorage.php b/core/lib/Drupal/Core/Batch/BatchStorage.php
index 3c44628ef93d9b75af2523a80b13df038052080e..51ad49ef84e1c132b87088e73397b2dc8cb13dca 100644
--- a/core/lib/Drupal/Core/Batch/BatchStorage.php
+++ b/core/lib/Drupal/Core/Batch/BatchStorage.php
@@ -2,10 +2,11 @@
namespace Drupal\Core\Batch;
+use Drupal\Component\Datetime\TimeInterface;
+use Drupal\Core\Access\CsrfTokenGenerator;
use Drupal\Core\Database\Connection;
use Drupal\Core\Database\DatabaseException;
use Symfony\Component\HttpFoundation\Session\SessionInterface;
-use Drupal\Core\Access\CsrfTokenGenerator;
class BatchStorage implements BatchStorageInterface {
@@ -35,6 +36,11 @@ class BatchStorage implements BatchStorageInterface {
*/
protected $csrfToken;
+ /**
+ * The time service.
+ */
+ protected readonly TimeInterface $time;
+
/**
* Constructs the database batch storage service.
*
@@ -44,11 +50,18 @@ class BatchStorage implements BatchStorageInterface {
* The session.
* @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token
* The CSRF token generator.
+ * @param \Drupal\Component\Datetime\TimeInterface $time
+ * The time service.
*/
- public function __construct(Connection $connection, SessionInterface $session, CsrfTokenGenerator $csrf_token) {
+ public function __construct(Connection $connection, SessionInterface $session, CsrfTokenGenerator $csrf_token, TimeInterface $time = NULL) {
$this->connection = $connection;
$this->session = $session;
$this->csrfToken = $csrf_token;
+ if (!$time) {
+ @trigger_error('Calling ' . __METHOD__ . '() without the $time argument is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. See https://www.drupal.org/node/3220378', E_USER_DEPRECATED);
+ $time = \Drupal::service('datetime.time');
+ }
+ $this->time = $time;
}
/**
@@ -109,7 +122,7 @@ public function cleanup() {
try {
// Cleanup the batch table and the queue for failed batches.
$this->connection->delete('batch')
- ->condition('timestamp', REQUEST_TIME - 864000, '<')
+ ->condition('timestamp', $this->time->getRequestTime() - 864000, '<')
->execute();
}
catch (\Exception $e) {
@@ -121,12 +134,29 @@ public function cleanup() {
* {@inheritdoc}
*/
public function create(array $batch) {
- // Ensure that a session is started before using the CSRF token generator.
+ // Ensure that a session is started before using the CSRF token generator,
+ // and update the database record.
$this->session->start();
+ $this->connection->update('batch')
+ ->fields([
+ 'token' => $this->csrfToken->get($batch['id']),
+ 'batch' => serialize($batch),
+ ])
+ ->condition('bid', $batch['id'])
+ ->execute();
+ }
+
+ /**
+ * Returns a new batch id.
+ *
+ * @return int
+ * A batch id.
+ */
+ public function getId(): int {
$try_again = FALSE;
try {
// The batch table might not yet exist.
- $this->doCreate($batch);
+ return $this->doInsertBatchRecord();
}
catch (\Exception $e) {
// If there was an exception, try to create the table.
@@ -138,23 +168,22 @@ public function create(array $batch) {
}
// Now that the table has been created, try again if necessary.
if ($try_again) {
- $this->doCreate($batch);
+ return $this->doInsertBatchRecord();
}
}
/**
- * Saves a batch.
+ * Inserts a record in the table and returns the batch id.
*
- * @param array $batch
- * The array representing the batch to create.
+ * @return int
+ * A batch id.
*/
- protected function doCreate(array $batch) {
- $this->connection->insert('batch')
+ protected function doInsertBatchRecord(): int {
+ return $this->connection->insert('batch')
->fields([
- 'bid' => $batch['id'],
- 'timestamp' => REQUEST_TIME,
- 'token' => $this->csrfToken->get($batch['id']),
- 'batch' => serialize($batch),
+ 'timestamp' => $this->time->getRequestTime(),
+ 'token' => '',
+ 'batch' => NULL,
])
->execute();
}
@@ -208,9 +237,7 @@ public function schemaDefinition() {
'fields' => [
'bid' => [
'description' => 'Primary Key: Unique batch ID.',
- // This is not a serial column, to allow both progressive and
- // non-progressive batches. See batch_process().
- 'type' => 'int',
+ 'type' => 'serial',
'unsigned' => TRUE,
'not null' => TRUE,
],
diff --git a/core/lib/Drupal/Core/ProxyClass/Batch/BatchStorage.php b/core/lib/Drupal/Core/ProxyClass/Batch/BatchStorage.php
index 5e6fc6df6d216cb7c7e031e2663f85fa2c91de7d..4b54090cabc0add17eeb14f4dda2a0089314adad 100644
--- a/core/lib/Drupal/Core/ProxyClass/Batch/BatchStorage.php
+++ b/core/lib/Drupal/Core/ProxyClass/Batch/BatchStorage.php
@@ -107,6 +107,14 @@ public function create(array $batch)
return $this->lazyLoadItself()->create($batch);
}
+ /**
+ * {@inheritdoc}
+ */
+ public function getId(): int
+ {
+ return $this->lazyLoadItself()->getId();
+ }
+
/**
* {@inheritdoc}
*/
diff --git a/core/modules/system/system.install b/core/modules/system/system.install
index f7be7128f64d9b344585f8b94d272a1ac224b2f2..30aa5b5870fdcc8671f208106959cc57ee71267d 100644
--- a/core/modules/system/system.install
+++ b/core/modules/system/system.install
@@ -1780,3 +1780,22 @@ function system_update_10100(&$sandbox = NULL) {
}
}
+
+/**
+ * Change the {batch} table [bid] field to serial.
+ */
+function system_update_10101(&$sandbox = NULL) {
+ $connection = \Drupal::database();
+ $schema = $connection->schema();
+
+ // Update batch table.
+ if ($schema->tableExists('batch')) {
+ $schema->changeField('batch', 'bid', 'bid', [
+ 'description' => 'Primary Key: Unique batch ID.',
+ 'type' => 'serial',
+ 'unsigned' => TRUE,
+ 'not null' => TRUE,
+ ]);
+ }
+
+}
diff --git a/core/modules/system/tests/src/Functional/Update/BatchBidSerialUpdateTest.php b/core/modules/system/tests/src/Functional/Update/BatchBidSerialUpdateTest.php
new file mode 100644
index 0000000000000000000000000000000000000000..0e5f4d3c2283f4b7332f5a946156ff6efcc442f8
--- /dev/null
+++ b/core/modules/system/tests/src/Functional/Update/BatchBidSerialUpdateTest.php
@@ -0,0 +1,81 @@
+<?php
+
+namespace Drupal\Tests\system\Functional\Update;
+
+use Drupal\Core\Database\IntegrityConstraintViolationException;
+use Drupal\FunctionalTests\Update\UpdatePathTestBase;
+
+/**
+ * Tests system_update_10101() upgrade path.
+ *
+ * @group system
+ * @group legacy
+ */
+class BatchBidSerialUpdateTest extends UpdatePathTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setDatabaseDumpFiles() {
+ $this->databaseDumpFiles = [
+ __DIR__ . '/../../../../../system/tests/fixtures/update/drupal-9.4.0.bare.standard.php.gz',
+ ];
+ }
+
+ /**
+ * Tests the change of the {batch} table [bid] field to serial.
+ */
+ public function testUpdate(): void {
+ /** @var \Drupal\Core\Database\Connection $connection */
+ $connection = \Drupal::service('database');
+
+ // Before the update, inserting a record in the {batch} table without
+ // passing a value for [bid] should fail, with the exception of the SQLite
+ // database where a NOT NULL integer field that is the primary key is set
+ // to automatic increment anyway.
+ //
+ // @see https://www.drupal.org/project/drupal/issues/2665216#comment-14885361
+ try {
+ $connection->insert('batch')
+ ->fields([
+ 'timestamp' => \Drupal::time()->getRequestTime(),
+ 'token' => '',
+ 'batch' => NULL,
+ ])
+ ->execute();
+ if ($connection->databaseType() !== 'sqlite') {
+ $this->fail('Insert to {batch} without bid should have failed, but it did not');
+ }
+ }
+ catch (\Exception $e) {
+ $this->assertInstanceOf(IntegrityConstraintViolationException::class, $e);
+ }
+
+ $this->runUpdates();
+
+ // $bid should be higher than one, since the update process would have
+ // executed a batch already. We look at the records inserted to determine
+ // the value of $bid, instead of relying on the value returned by the
+ // INSERT, because in PostgreSql the test connection gets confused by the
+ // ::changeField() executed in the SUT and keeps returning 0 instead of
+ // lastId as result of the insert.
+ $connection->insert('batch')
+ ->fields([
+ 'timestamp' => \Drupal::time()->getRequestTime(),
+ 'token' => '',
+ 'batch' => NULL,
+ ])
+ ->execute();
+ $bid = (int) $connection->query('SELECT MAX([bid]) FROM {batch}')->fetchField();
+ $this->assertGreaterThan(1, $bid);
+ $connection->insert('batch')
+ ->fields([
+ 'timestamp' => \Drupal::time()->getRequestTime(),
+ 'token' => '',
+ 'batch' => NULL,
+ ])
+ ->execute();
+ $this->assertEquals($bid + 1, (int) $connection->query('SELECT MAX([bid]) FROM {batch}')->fetchField());
+ }
+
+}
diff --git a/core/phpstan-baseline.neon b/core/phpstan-baseline.neon
index 4b0b775af4c08d0a6ee61b157bce3ec1ae87edf4..e74767498466018598a6325e31a652d3655d5ff2 100644
--- a/core/phpstan-baseline.neon
+++ b/core/phpstan-baseline.neon
@@ -230,11 +230,6 @@ parameters:
count: 1
path: lib/Drupal/Core/Asset/JsCollectionRenderer.php
- -
- message: "#^Call to deprecated constant REQUEST_TIME\\: Deprecated in drupal\\:8\\.3\\.0 and is removed from drupal\\:11\\.0\\.0\\. Use \\\\Drupal\\:\\:time\\(\\)\\-\\>getRequestTime\\(\\); $#"
- count: 2
- path: lib/Drupal/Core/Batch/BatchStorage.php
-
-
message: "#^Call to method getDefinitions\\(\\) on an unknown class Drupal\\\\Core\\\\Plugin\\\\CategorizingPluginManagerTrait\\.$#"
count: 1