diff --git a/core/core.services.yml b/core/core.services.yml index 5e84779315..ce0e6d47e3 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -1718,6 +1718,6 @@ services: - { name: backend_overridable } tempstore.shared: class: Drupal\Core\TempStore\SharedTempStoreFactory - arguments: ['@keyvalue.expirable', '@lock', '@request_stack', '%tempstore.expire%'] + arguments: ['@keyvalue.expirable', '@lock', '@request_stack', '@current_user', '@session', '%tempstore.expire%'] tags: - { name: backend_overridable } diff --git a/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php b/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php index 40a48f87f9..bccd0f01a4 100644 --- a/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php +++ b/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php @@ -24,11 +24,7 @@ public static function getLabel() { public function getContext() { $request = $this->requestStack->getCurrentRequest(); if ($request->hasSession()) { - $id = $request->getSession()->getId(); - if (!$id) { - throw new Exception('No session ID'); - } - return Crypt::hashBase64($id); + return Crypt::hashBase64($request->getSession()->getId()); } return 'none'; } diff --git a/core/lib/Drupal/Core/TempStore/SharedTempStore.php b/core/lib/Drupal/Core/TempStore/SharedTempStore.php index aeaa2d811b..bcb6697cf3 100644 --- a/core/lib/Drupal/Core/TempStore/SharedTempStore.php +++ b/core/lib/Drupal/Core/TempStore/SharedTempStore.php @@ -66,6 +66,13 @@ class SharedTempStore { */ protected $owner; + /** + * The current user. + * + * @var \Drupal\Core\Session\AccountProxyInterface + */ + protected $currentUser; + /** * The time to live for items in seconds. * @@ -75,13 +82,6 @@ class SharedTempStore { */ protected $expire; - /** - * The current user. - * - * @var \Drupal\Core\Session\AccountProxyInterface - */ - protected $currentUser; - /** * Constructs a new object for accessing data from a key/value store. * @@ -95,7 +95,7 @@ class SharedTempStore { * The owner key to store along with the data (e.g. a user or session ID). * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack * The request stack. - * @param \Drupal\Core\Session\AccountProxyInterface|null $current_user + * @param \Drupal\Core\Session\AccountProxyInterface $current_user * The current user. * @param int $expire * The time to live for items, in seconds. @@ -109,7 +109,7 @@ public function __construct(KeyValueStoreExpirableInterface $storage, LockBacken $this->currentUser = $current_user; } else { - @trigger_error('@todo', E_USER_DEPRECATED); + @trigger_error(__CLASS__ . '::__construct() now requires the current_user service to be injected. See https://www.drupal.org/node/3006268', E_USER_DEPRECATED); $this->currentUser = \Drupal::currentUser(); if (is_int($current_user)) { // If the $current_user argument is numeric then this object has been diff --git a/core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php b/core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php index 29745a1769..9d0040205b 100644 --- a/core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php +++ b/core/lib/Drupal/Core/TempStore/SharedTempStoreFactory.php @@ -5,7 +5,9 @@ use Drupal\Component\Utility\Crypt; use Drupal\Core\KeyValueStore\KeyValueExpirableFactoryInterface; use Drupal\Core\Lock\LockBackendInterface; +use Drupal\Core\Session\AccountProxyInterface; use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpFoundation\Session\SessionInterface; /** * Creates a shared temporary storage for a collection. @@ -33,6 +35,20 @@ class SharedTempStoreFactory { */ protected $requestStack; + /** + * The current user. + * + * @var \Drupal\Core\Session\AccountProxyInterface + */ + protected $currentUser; + + /** + * The session service. + * + * @var \Symfony\Component\HttpFoundation\Session\SessionInterface + */ + protected $session; + /** * The time to live for items in seconds. * @@ -49,13 +65,38 @@ class SharedTempStoreFactory { * The lock object used for this data. * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack * The request stack. + * @param \Drupal\Core\Session\AccountProxyInterface $current_user + * The current user. + * @param \Symfony\Component\HttpFoundation\Session\SessionInterface $session + * The session service. * @param int $expire * The time to live for items, in seconds. */ - public function __construct(KeyValueExpirableFactoryInterface $storage_factory, LockBackendInterface $lock_backend, RequestStack $request_stack, $expire = 604800) { + public function __construct(KeyValueExpirableFactoryInterface $storage_factory, LockBackendInterface $lock_backend, RequestStack $request_stack, $current_user = NULL, SessionInterface $session = NULL, $expire = 604800) { $this->storageFactory = $storage_factory; $this->lockBackend = $lock_backend; $this->requestStack = $request_stack; + + if ($current_user instanceof AccountProxyInterface) { + $this->currentUser = $current_user; + } + else { + $this->currentUser = \Drupal::currentUser(); + } + if ($session instanceof SessionInterface) { + $this->session = $session; + } + else { + $this->session = \Drupal::service('session'); + } + if (!($current_user instanceof AccountProxyInterface) || !($session instanceof SessionInterface)) { + @trigger_error(__CLASS__ . '::__construct() now requires the current_user and session services to be injected. See https://www.drupal.org/node/3006268', E_USER_DEPRECATED); + if (is_int($current_user)) { + // If the $current_user argument is numeric then this object has been + // instantiated with the old constructor signature. + $expire = $current_user; + } + } $this->expire = $expire; } @@ -76,26 +117,24 @@ public function __construct(KeyValueExpirableFactoryInterface $storage_factory, public function get($collection, $owner = NULL) { // Use the currently authenticated user ID or the active user ID unless // the owner is overridden. - $account = \Drupal::currentUser(); if (!isset($owner)) { - $owner = $account->id(); - if ($account->isAnonymous()) { + $owner = $this->currentUser->id(); + if ($this->currentUser->isAnonymous()) { $has_session = $this->requestStack ->getCurrentRequest() ->hasSession(); if (!$has_session) { - /** @var \Symfony\Component\HttpFoundation\Session\SessionInterface $session */ - $session = \Drupal::service('session'); - $this->requestStack->getCurrentRequest()->setSession($session); - $session->start(); + $this->requestStack->getCurrentRequest()->setSession($this->session); + $this->session->start(); } + // Anonymous users store a random identifier in session. $owner = $this->requestStack->getCurrentRequest()->getSession()->get('core.tempstore.shared.owner', Crypt::randomBytesBase64()); } } // Store the data for this collection in the database. $storage = $this->storageFactory->get("tempstore.shared.$collection"); - return new SharedTempStore($storage, $this->lockBackend, $owner, $this->requestStack, $account, $this->expire); + return new SharedTempStore($storage, $this->lockBackend, $owner, $this->requestStack, $this->currentUser, $this->expire); } } diff --git a/core/modules/user/src/SharedTempStoreFactory.php b/core/modules/user/src/SharedTempStoreFactory.php index c955f07f7a..fe4deb00d3 100644 --- a/core/modules/user/src/SharedTempStoreFactory.php +++ b/core/modules/user/src/SharedTempStoreFactory.php @@ -35,12 +35,23 @@ public function get($collection, $owner = NULL) { // Use the currently authenticated user ID or the active user ID unless // the owner is overridden. if (!isset($owner)) { - $owner = \Drupal::currentUser()->id() ?: session_id(); + $owner = $this->currentUser->id(); + if ($this->currentUser->isAnonymous()) { + $has_session = $this->requestStack + ->getCurrentRequest() + ->hasSession(); + if (!$has_session) { + $this->requestStack->getCurrentRequest()->setSession($this->session); + $this->session->start(); + } + // Anonymous users store a random identifier in session. + $owner = $this->requestStack->getCurrentRequest()->getSession()->get('core.tempstore.shared.owner', Crypt::randomBytesBase64()); + } } // Store the data for this collection in the database. $storage = $this->storageFactory->get("user.shared_tempstore.$collection"); - return new SharedTempStore($storage, $this->lockBackend, $owner, $this->requestStack, $this->expire); + return new SharedTempStore($storage, $this->lockBackend, $owner, $this->requestStack, $this->currentUser, $this->expire); } } diff --git a/core/modules/user/tests/src/Kernel/TempStoreDatabaseTest.php b/core/modules/user/tests/src/Kernel/TempStoreDatabaseTest.php index 9e4ea6ed70..ce2ea64764 100644 --- a/core/modules/user/tests/src/Kernel/TempStoreDatabaseTest.php +++ b/core/modules/user/tests/src/Kernel/TempStoreDatabaseTest.php @@ -3,6 +3,7 @@ namespace Drupal\Tests\user\Kernel; use Drupal\Core\KeyValueStore\KeyValueExpirableFactory; +use Drupal\Core\Session\AccountProxyInterface; use Drupal\KernelTests\KernelTestBase; use Drupal\user\SharedTempStoreFactory; use Drupal\Core\Lock\DatabaseLockBackend; @@ -73,12 +74,17 @@ protected function setUp() { * @expectedDeprecation \Drupal\user\SharedTempStore is scheduled for removal in Drupal 9.0.0. Use \Drupal\Core\TempStore\SharedTempStore instead. See https://www.drupal.org/node/2935639. */ public function testUserTempStore() { - $session = \Drupal::service('session'); - $session->start(); - $request_stack = $this->container->get('request_stack'); - $request_stack->getCurrentRequest()->setSession($session); + // Mock the current user service so that isAnonymous returns FALSE. + $current_user = $this->prophesize(AccountProxyInterface::class); + // Create a key/value collection. - $factory = new SharedTempStoreFactory(new KeyValueExpirableFactory(\Drupal::getContainer()), new DatabaseLockBackend(Database::getConnection()), $request_stack); + $factory = new SharedTempStoreFactory( + new KeyValueExpirableFactory(\Drupal::getContainer()), + new DatabaseLockBackend(Database::getConnection()), + $this->container->get('request_stack'), + $current_user->reveal(), + $this->container->get('session') + ); $collection = $this->randomMachineName(); // Create two mock users. diff --git a/core/tests/Drupal/KernelTests/Core/TempStore/TempStoreDatabaseTest.php b/core/tests/Drupal/KernelTests/Core/TempStore/TempStoreDatabaseTest.php index a936a431a3..391bb0c53c 100644 --- a/core/tests/Drupal/KernelTests/Core/TempStore/TempStoreDatabaseTest.php +++ b/core/tests/Drupal/KernelTests/Core/TempStore/TempStoreDatabaseTest.php @@ -3,6 +3,7 @@ namespace Drupal\KernelTests\Core\TempStore; use Drupal\Core\KeyValueStore\KeyValueExpirableFactory; +use Drupal\Core\Session\AccountProxyInterface; use Drupal\KernelTests\KernelTestBase; use Drupal\Core\TempStore\SharedTempStoreFactory; use Drupal\Core\Lock\DatabaseLockBackend; @@ -64,11 +65,15 @@ protected function setUp() { public function testSharedTempStore() { // Create a key/value collection. $database = Database::getConnection(); - $session = \Drupal::service('session'); - $session->start(); - $request_stack = $this->container->get('request_stack'); - $request_stack->getCurrentRequest()->setSession($session); - $factory = new SharedTempStoreFactory(new KeyValueExpirableFactory(\Drupal::getContainer()), new DatabaseLockBackend($database), $request_stack); + // Mock the current user service so that isAnonymous returns FALSE. + $current_user = $this->prophesize(AccountProxyInterface::class); + $factory = new SharedTempStoreFactory( + new KeyValueExpirableFactory(\Drupal::getContainer()), + new DatabaseLockBackend($database), + $this->container->get('request_stack'), + $current_user->reveal(), + $this->container->get('session') + ); $collection = $this->randomMachineName(); // Create two mock users. diff --git a/core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreFactoryTest.php b/core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreFactoryTest.php new file mode 100644 index 0000000000..2874b35c83 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreFactoryTest.php @@ -0,0 +1,48 @@ +prophesize(AccountProxyInterface::class)->reveal(); + $session = $this->prophesize(SessionInterface::class)->reveal(); + $container->set('current_user', $current_user); + $container->set('session', $session); + \Drupal::setContainer($container); + + $key_value_factory = $this->prophesize(KeyValueExpirableFactoryInterface::class)->reveal(); + $lock = $this->prophesize(LockBackendInterface::class)->reveal(); + $request_stack = $this->prophesize(RequestStack::class)->reveal(); + $store = new SharedTempStoreFactory($key_value_factory, $lock, $request_stack, 1000); + + $reflection_class = new \ReflectionClass(SharedTempStoreFactory::class); + + $current_user_property = $reflection_class->getProperty('currentUser'); + $current_user_property->setAccessible(TRUE); + $this->assertSame($current_user, $current_user_property->getValue($store)); + + $expire_property = $reflection_class->getProperty('expire'); + $expire_property->setAccessible(TRUE); + $this->assertSame(1000, $expire_property->getValue($store)); + } + +} diff --git a/core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreTest.php b/core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreTest.php index f8edcd066c..ba70003ef3 100644 --- a/core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreTest.php +++ b/core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreTest.php @@ -358,7 +358,7 @@ public function testDeleteIfOwner() { /** * @group legacy - * @expectedDeprecation @todo + * @expectedDeprecation Drupal\Core\TempStore\SharedTempStore::__construct() now requires the current_user service to be injected. See https://www.drupal.org/node/3006268 */ public function testLegacyConstructor() { $container = new ContainerBuilder();