Issue #2286235 by andypost, longwave, ericduran, alexpott, voleger, catch:...

Issue #2286235 by andypost, longwave, ericduran, alexpott, voleger, catch: Deprecate db_ignore_replica() and convert it to service

core/core.services.yml

@@ -358,6 +358,11 @@ services:
     class: Drupal\Core\Database\Connection
     factory: Drupal\Core\Database\Database::getConnection
     arguments: [replica]
+  database.replica_kill_switch:
+    class: Drupal\Core\Database\ReplicaKillSwitch
+    arguments: ['@settings', '@datetime.time', '@session']
+    tags:
+      - { name: event_subscriber }
   datetime.time:
     class: Drupal\Component\Datetime\Time
     arguments: ['@request_stack']
@@ -939,7 +944,7 @@ services:
       - { name: needs_destruction }
   menu.rebuild_subscriber:
     class: Drupal\Core\EventSubscriber\MenuRouterRebuildSubscriber
-    arguments: ['@lock', '@plugin.manager.menu.link', '@database']
+    arguments: ['@lock', '@plugin.manager.menu.link', '@database', '@database.replica_kill_switch']
     tags:
       - { name: event_subscriber }
   path.alias_storage:
@@ -1403,10 +1408,6 @@ services:
     tags:
       - { name: backend_overridable }
     lazy: true
-  replica_database_ignore__subscriber:
-    class: Drupal\Core\EventSubscriber\ReplicaDatabaseIgnoreSubscriber
-    tags:
-      - {name: event_subscriber}
   country_manager:
     class: Drupal\Core\Locale\CountryManager
     arguments: ['@module_handler']

core/includes/database.inc

@@ -11,7 +11,6 @@
 
 use Drupal\Core\Database\Database;
 use Drupal\Core\Database\Query\Condition;
-use Drupal\Core\Site\Settings;
 
 /**
  * @addtogroup database
@@ -1089,18 +1088,14 @@ function db_change_field($table, $field, $field_new, $spec, $keys_new = []) {
  * Sets a session variable specifying the lag time for ignoring a replica
  * server (A replica server is traditionally referred to as
  * a "slave" in database server documentation).
+ *
+ * @deprecated as of Drupal 8.7.x, will be removed in Drupal 9.0.0. Use
+ *   \Drupal::service('database.replica_kill_switch')->trigger() instead.
+ *
+ * @see https://www.drupal.org/node/2997500
  * @see https://www.drupal.org/node/2275877
  */
 function db_ignore_replica() {
-  $connection_info = Database::getConnectionInfo();
-  // Only set ignore_replica_server if there are replica servers being used,
-  // which is assumed if there are more than one.
-  if (count($connection_info) > 1) {
-    // Five minutes is long enough to allow the replica to break and resume
-    // interrupted replication without causing problems on the Drupal site from
-    // the old data.
-    $duration = Settings::get('maximum_replication_lag', 300);
-    // Set session variable with amount of time to delay before using replica.
-    $_SESSION['ignore_replica_server'] = REQUEST_TIME + $duration;
-  }
+  @trigger_error('db_ignore_replica() is deprecated in Drupal 8.7.x and will be removed before Drupal 9.0.0. Use \Drupal\Core\Database\ReplicaKillSwitch::trigger() instead. See https://www.drupal.org/node/2997500', E_USER_DEPRECATED);
+  \Drupal::service('database.replica_kill_switch')->trigger();
 }

core/lib/Drupal/Core/EventSubscriber/ReplicaDatabaseIgnoreSubscriber.php → core/lib/Drupal/Core/Database/ReplicaKillSwitch.php

 <?php
 
-namespace Drupal\Core\EventSubscriber;
+namespace Drupal\Core\Database;
 
-use Drupal\Core\Database\Database;
-use Symfony\Component\HttpKernel\KernelEvents;
-use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Drupal\Component\Datetime\TimeInterface;
+use Drupal\Core\Site\Settings;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\HttpFoundation\Session\SessionInterface;
+use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\HttpKernel\KernelEvents;
 
 /**
- * System subscriber for controller requests.
+ * Provides replica server kill switch to ignore it.
  */
-class ReplicaDatabaseIgnoreSubscriber implements EventSubscriberInterface {
+class ReplicaKillSwitch implements EventSubscriberInterface {
+
+  /**
+   * The settings object.
+   *
+   * @var \Drupal\Core\Site\Settings
+   */
+  protected $settings;
+
+  /**
+   * The time service.
+   *
+   * @var \Drupal\Component\Datetime\TimeInterface
+   */
+  protected $time;
+
+  /**
+   * The session.
+   *
+   * @var \Symfony\Component\HttpFoundation\Session\SessionInterface
+   */
+  protected $session;
+
+  /**
+   * Constructs a ReplicaKillSwitch object.
+   *
+   * @param \Drupal\Core\Site\Settings $settings
+   *   The settings object.
+   * @param \Drupal\Component\Datetime\TimeInterface $time
+   *   The time service.
+   * @param \Symfony\Component\HttpFoundation\Session\SessionInterface $session
+   *   The session.
+   */
+  public function __construct(Settings $settings, TimeInterface $time, SessionInterface $session) {
+    $this->settings = $settings;
+    $this->time = $time;
+    $this->session = $session;
+  }
+
+  /**
+   * Denies access to replica database on the current request.
+   *
+   * @see https://www.drupal.org/node/2286193
+   */
+  public function trigger() {
+    $connection_info = Database::getConnectionInfo();
+    // Only set ignore_replica_server if there are replica servers being used,
+    // which is assumed if there are more than one.
+    if (count($connection_info) > 1) {
+      // Five minutes is long enough to allow the replica to break and resume
+      // interrupted replication without causing problems on the Drupal site
+      // from the old data.
+      $duration = $this->settings->get('maximum_replication_lag', 300);
+      // Set session variable with amount of time to delay before using replica.
+      $this->session->set('ignore_replica_server', $this->time->getRequestTime() + $duration);
+    }
+  }
 
   /**
    * Checks and disables the replica database server if appropriate.
@@ -24,22 +82,22 @@ public function checkReplicaServer(GetResponseEvent $event) {
     // In Drupal's distributed database structure, new data is written to the
     // master and then propagated to the replica servers.  This means there is a
     // lag between when data is written to the master and when it is available
-    // on the replica. At these times, we will want to avoid using a replica server
-    // temporarily. For example, if a user posts a new node then we want to
-    // disable the replica server for that user temporarily to allow the replica
-    // server to catch up.
+    // on the replica. At these times, we will want to avoid using a replica
+    // server temporarily. For example, if a user posts a new node then we want
+    // to disable the replica server for that user temporarily to allow the
+    // replica server to catch up.
     // That way, that user will see their changes immediately while for other
     // users we still get the benefits of having a replica server, just with
-    // slightly stale data.  Code that wants to disable the replica server should
-    // use the db_set_ignore_replica() function to set
-    // $_SESSION['ignore_replica_server'] to the timestamp after which the replica
-    // can be re-enabled.
-    if (isset($_SESSION['ignore_replica_server'])) {
-      if ($_SESSION['ignore_replica_server'] >= REQUEST_TIME) {
+    // slightly stale data. Code that wants to disable the replica server should
+    // use the 'database.replica_kill_switch' service's trigger() method to set
+    // 'ignore_replica_server' session flag to the timestamp after which the
+    // replica can be re-enabled.
+    if ($this->session->has('ignore_replica_server')) {
+      if ($this->session->get('ignore_replica_server') >= $this->time->getRequestTime()) {
         Database::ignoreTarget('default', 'replica');
       }
       else {
-        unset($_SESSION['ignore_replica_server']);
+        $this->session->remove('ignore_replica_server');
       }
     }
   }

core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php

@@ -726,7 +726,7 @@ public function delete(array $entities) {
       parent::delete($entities);
 
       // Ignore replica server temporarily.
-      db_ignore_replica();
+      \Drupal::service('database.replica_kill_switch')->trigger();
     }
     catch (\Exception $e) {
       $transaction->rollBack();
@@ -777,7 +777,7 @@ public function save(EntityInterface $entity) {
       $return = parent::save($entity);
 
       // Ignore replica server temporarily.
-      db_ignore_replica();
+      \Drupal::service('database.replica_kill_switch')->trigger();
       return $return;
     }
     catch (\Exception $e) {

core/lib/Drupal/Core/EventSubscriber/MenuRouterRebuildSubscriber.php

@@ -3,6 +3,7 @@
 namespace Drupal\Core\EventSubscriber;
 
 use Drupal\Core\Cache\Cache;
+use Drupal\Core\Database\ReplicaKillSwitch;
 use Drupal\Core\Lock\LockBackendInterface;
 use Drupal\Core\Menu\MenuLinkManagerInterface;
 use Drupal\Core\Routing\RoutingEvents;
@@ -27,6 +28,13 @@ class MenuRouterRebuildSubscriber implements EventSubscriberInterface {
    */
   protected $menuLinkManager;
 
+  /**
+   * The replica kill switch.
+   *
+   * @var \Drupal\Core\Database\ReplicaKillSwitch
+   */
+  protected $replicaKillSwitch;
+
   /**
    * The database connection.
    *
@@ -43,11 +51,14 @@ class MenuRouterRebuildSubscriber implements EventSubscriberInterface {
    *   The menu link plugin manager.
    * @param \Drupal\Core\Database\Connection $connection
    *   The database connection.
+   * @param \Drupal\Core\Database\ReplicaKillSwitch $replica_kill_switch
+   *   The replica kill switch.
    */
-  public function __construct(LockBackendInterface $lock, MenuLinkManagerInterface $menu_link_manager, Connection $connection) {
+  public function __construct(LockBackendInterface $lock, MenuLinkManagerInterface $menu_link_manager, Connection $connection, ReplicaKillSwitch $replica_kill_switch) {
     $this->lock = $lock;
     $this->menuLinkManager = $menu_link_manager;
     $this->connection = $connection;
+    $this->replicaKillSwitch = $replica_kill_switch;
   }
 
   /**
@@ -71,7 +82,7 @@ protected function menuLinksRebuild() {
         // Ensure the menu links are up to date.
         $this->menuLinkManager->rebuild();
         // Ignore any database replicas temporarily.
-        db_ignore_replica();
+        $this->replicaKillSwitch->trigger();
       }
       catch (\Exception $e) {
         $transaction->rollBack();

core/tests/Drupal/KernelTests/Core/Database/DatabaseLegacyTest.php

@@ -488,4 +488,18 @@ public function testDbSelect() {
     $this->assertInstanceOf(Select::class, db_select('test'));
   }
 
+  /**
+   * Tests the db_ignore_replica() function.
+   *
+   * @expectedDeprecation db_ignore_replica() is deprecated in Drupal 8.7.x and will be removed before Drupal 9.0.0. Use \Drupal\Core\Database\ReplicaKillSwitch::trigger() instead. See https://www.drupal.org/node/2997500
+   */
+  public function testDbIgnoreReplica() {
+    $connection = Database::getConnectionInfo('default');
+    Database::addConnectionInfo('default', 'replica', $connection['default']);
+    db_ignore_replica();
+    /** @var \Symfony\Component\HttpFoundation\Session\SessionInterface $session */
+    $session = \Drupal::service('session');
+    $this->assertTrue($session->has('ignore_replica_server'));
+  }
+
 }

core/tests/Drupal/KernelTests/Core/EventSubscriber/IgnoreReplicaSubscriberTest.php → core/tests/Drupal/KernelTests/Core/Database/ReplicaKillSwitchTest.php

 <?php
 
-namespace Drupal\KernelTests\Core\EventSubscriber;
+namespace Drupal\KernelTests\Core\Database;
 
 use Drupal\Core\Database\Database;
-use Drupal\Core\EventSubscriber\ReplicaDatabaseIgnoreSubscriber;
 use Drupal\Core\DrupalKernel;
+use Drupal\Core\Site\Settings;
 use Drupal\KernelTests\KernelTestBase;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 
 /**
- * Tests that ReplicaDatabaseIgnoreSubscriber functions correctly.
+ * Tests that ReplicaKillSwitch functions correctly.
  *
  * @group system
  */
-class IgnoreReplicaSubscriberTest extends KernelTestBase {
+class ReplicaKillSwitchTest extends KernelTestBase {
 
   /**
-   * Tests \Drupal\Core\EventSubscriber\ReplicaDatabaseIgnoreSubscriber::checkReplicaServer().
+   * Tests database.replica_kill_switch service.
    */
   public function testSystemInitIgnoresSecondaries() {
     // Clone the master credentials to a replica connection.
@@ -27,17 +27,24 @@ public function testSystemInitIgnoresSecondaries() {
     $connection_info = Database::getConnectionInfo('default');
     Database::addConnectionInfo('default', 'replica', $connection_info['default']);
 
-    db_ignore_replica();
+    /** @var \Drupal\Core\Database\ReplicaKillSwitch $service */
+    $service = \Drupal::service('database.replica_kill_switch');
+    $service->trigger();
     $class_loader = require $this->root . '/autoload.php';
     $kernel = new DrupalKernel('testing', $class_loader, FALSE);
     $event = new GetResponseEvent($kernel, Request::create('http://example.com'), HttpKernelInterface::MASTER_REQUEST);
-    $subscriber = new ReplicaDatabaseIgnoreSubscriber();
-    $subscriber->checkReplicaServer($event);
+    $service->checkReplicaServer($event);
 
     $db1 = Database::getConnection('default', 'default');
     $db2 = Database::getConnection('replica', 'default');
 
     $this->assertSame($db1, $db2, 'System Init ignores secondaries when requested.');
+
+    // Makes sure that session value set right.
+    $session = \Drupal::service('session');
+    $this->assertTrue($session->has('ignore_replica_server'));
+    $expected = \Drupal::time()->getRequestTime() + Settings::get('maximum_replication_lag', 300);
+    $this->assertEquals($expected, $session->get('ignore_replica_server'));
   }
 
 }