Move User module's temp stores to core

This looks really good.

Found a few minors:

Drupal\KernelTests\Core\TempStore\TempStoreDatabaseTest:

1. It should be in @group TempStore
2. $storeFactory var annotation need to be adjusted

Drupal\Tests\Core\TempStore\PrivateTempStoreTest:

1. $tempStore var annotation need to be adjusted

Forgot to change the status

Good to see the class_alias() works, this will be our first non-test-framework usage afaik.

This means we can't trigger a deprecation message from the old exception, but we're going to have a subset of API changes from 8.x to 9.x that are documented but not automated, and this was the only idea I could come up with for forwards compatibility.

+++ b/core/modules/user/user.services.yml
@@ -50,12 +50,14 @@ services:
   user.private_tempstore:
     class: Drupal\user\PrivateTempStoreFactory
...
+    arguments: ['@keyvalue.expirable', '@lock', '@current_user', '@request_stack', '%core.tempstore.expire%']
+    deprecated: The "%service_id%" service is deprecated. You should use the 'core.private_tempstore' service instead. See https://www.drupal.org/node/2935639.
     tags:
       - { name: backend_overridable }
   user.shared_tempstore:
     class: Drupal\user\SharedTempStoreFactory
-    arguments: ['@keyvalue.expirable', '@lock', '@request_stack', '%user.tempstore.expire%']
+    arguments: ['@keyvalue.expirable', '@lock', '@request_stack', '%core.tempstore.expire%']
+    deprecated: The "%service_id%" service is deprecated. You should use the 'core.shared_tempstore' service instead. See https://www.drupal.org/node/2935639.
     tags:
       - { name: backend_overridable }

I guess we should add a deprecation comment here as well

CR looks good to me.

Forget about #13. I missed that we have now the deprecated key.

I search in the Drupal codebase and couldn't find any other occurrence of the deprecated classes.

So for me, this is looking really good.

+++ b/core/modules/user/src/PrivateTempStore.php
@@ -2,212 +2,25 @@
-    if (($object = $this->storage->get($key)) && ($object->owner == $this->getOwner())) {

+++ b/core/modules/user/tests/src/Unit/SharedTempStoreTest.php
@@ -322,6 +336,7 @@ public function testDeleteWithNoLockAvailable() {
    * Tests the deleteIfOwner() method.
    *
    * @covers ::deleteIfOwner
+   * @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 testDeleteIfOwner() {
     $this->lock->expects($this->once())

What's the plan with those tests. Wouldn't it make more sense to move them too and use the new class? Don't think we copied them?

+++ b/core/core.services.yml
@@ -1650,3 +1651,13 @@ services:
+  core.private_tempstore:
...
+  core.shared_tempstore:

this is the first instance of a core service being prefixed with `core.`. Are we sure want to introduce that here?

We could instead flip it and make it namespaced to the concept - e.g. `tempstore.private` and `tempstore.shared` instead?

We do similar for other services in core, eg cache, config, entity_type.

Thoughts?

Rest of the patch looks awesome, love the deprecation work.

I checked that all occurrences of core.private_tempstore and core.shared_tempstore were replaced by tempstore.private and tempstore.shared.

Committed 601593f and pushed to 8.5.x. Thanks!

This seems like a cop-out.
Without #2008884: Provide an interface for TempStore implementations this issue makes a bad situation... the same.
This was an opportunity to suggest people typehint by a real interface. Now the deprecation message is prompting them to do the same old thing.

-1

Glad to see #2008884: Provide an interface for TempStore implementations is moving again. If it doesn't make it into 8.5.x, I don't see why this should either (since the deprecation message will point people to do the wrong thing for typehints)

Gave 8.5.x a try on our internal distribution and ctools is very unhappy about this change:

The service "ctools.serializable.tempstore.factory" has a dependency
on a non-existent parameter "user.tempstore.expire". Did you mean
this: "tempstore.expire"? in

I think we need to better BC on the removal of this. my suggestion would be to keep it instead of removing it and copy the value over if it's not the default.

Follow-up?

Edit: Follow-up it is: #2936435: Better BC for deprecated container parameter user.tempstore.expire

how is this fixed? I just downloaded 8.6.1 and still getting the issue. why do I have to perform a patch to the latest version if its fixed? does it not get rolled into the newest version?

what isn't fixed? This was a task, it didn't fix any bugs. And there was an issue with BC but that was fixed a long time ago.

why am I getting the error on a fresh install? The status was set to closed (fixed).

What error? If you have an error message you might want to create a new issue and mention everything you think might be relevant.

same error message that's in this thread.

[Sun Sep 23 23:29:00.196701 2018] [php7:notice] [pid 17992:tid 1944] [client ::1:59714] Symfony\\Component\\DependencyInjection\\Exception\\ParameterNotFoundException: The service "ctools.serializable.tempstore.factory" has a dependency on a non-existent parameter "user.tempstore.expire". Did you mean this: "tempstore.expire"? in C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\ParameterBag\\ParameterBag.php on line 102 #0 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\ParameterBag\\ParameterBag.php(219): Symfony\\Component\\DependencyInjection\\ParameterBag\\ParameterBag->get('user.tempstore....')\n#1 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\ParameterBag\\ParameterBag.php(189): Symfony\\Component\\DependencyInjection\\ParameterBag\\ParameterBag->resolveString('%user.tempstore...', Array)\n#2 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\Compiler\\ResolveParameterPlaceHoldersPass.php(64): Symfony\\Component\\DependencyInjection\\ParameterBag\\ParameterBag->resolveValue('%user.tempstore...')\n#3 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\Compiler\\AbstractRecursivePass.php(60): Symfony\\Component\\DependencyInjection\\Compiler\\ResolveParameterPlaceHoldersPass->processValue('%user.tempstore...', false)\n#4 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\Compiler\\ResolveParameterPlaceHoldersPass.php(79): Symfony\\Component\\DependencyInjection\\Compiler\\AbstractRecursivePass->processValue(Array, false)\n#5 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\Compiler\\AbstractRecursivePass.php(67): Symfony\\Component\\DependencyInjection\\Compiler\\ResolveParameterPlaceHoldersPass->processValue(Array)\n#6 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\Compiler\\ResolveParameterPlaceHoldersPass.php(79): Symfony\\Component\\DependencyInjection\\Compiler\\AbstractRecursivePass->processValue(Object(Symfony\\Component\\DependencyInjection\\Definition), true)\n#7 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\Compiler\\AbstractRecursivePass.php(60): Symfony\\Component\\DependencyInjection\\Compiler\\ResolveParameterPlaceHoldersPass->processValue(Object(Symfony\\Component\\DependencyInjection\\Definition), true)\n#8 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\Compiler\\ResolveParameterPlaceHoldersPass.php(79): Symfony\\Component\\DependencyInjection\\Compiler\\AbstractRecursivePass->processValue(Array, true)\n#9 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\Compiler\\AbstractRecursivePass.php(39): Symfony\\Component\\DependencyInjection\\Compiler\\ResolveParameterPlaceHoldersPass->processValue(Array, true)\n#10 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\Compiler\\ResolveParameterPlaceHoldersPass.php(43): Symfony\\Component\\DependencyInjection\\Compiler\\AbstractRecursivePass->process(Object(Drupal\\Core\\DependencyInjection\\ContainerBuilder))\n#11 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\Compiler\\Compiler.php(141): Symfony\\Component\\DependencyInjection\\Compiler\\ResolveParameterPlaceHoldersPass->process(Object(Drupal\\Core\\DependencyInjection\\ContainerBuilder))\n#12 C:\\xampp\\htdocs\\Drupal8\\vendor\\symfony\\dependency-injection\\ContainerBuilder.php(788): Symfony\\Component\\DependencyInjection\\Compiler\\Compiler->compile(Object(Drupal\\Core\\DependencyInjection\\ContainerBuilder))\n#13 C:\\xampp\\htdocs\\Drupal8\\web\\core\\lib\\Drupal\\Core\\DrupalKernel.php(1318): Symfony\\Component\\DependencyInjection\\ContainerBuilder->compile()\n#14 C:\\xampp\\htdocs\\Drupal8\\web\\core\\lib\\Drupal\\Core\\DrupalKernel.php(892): Drupal\\Core\\DrupalKernel->compileContainer()\n#15 C:\\xampp\\htdocs\\Drupal8\\web\\core\\lib\\Drupal\\Core\\DrupalKernel.php(468): Drupal\\Core\\DrupalKernel->initializeContainer()\n#16 C:\\xampp\\htdocs\\Drupal8\\web\\core\\lib\\Drupal\\Core\\DrupalKernel.php(664): Drupal\\Core\\DrupalKernel->boot()\n#17 C:\\xampp\\htdocs\\Drupal8\\web\\index.php(19): Drupal\\Core\\DrupalKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request))\n#18 {main}

I am getting same error too.

Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException: The service "ctools.serializable.tempstore.factory" has a dependency on a [error]
non-existent parameter "user.tempstore.expire". Did you mean this: "tempstore.expire"? in
/apps/drupal/htdocs/vendor/symfony/dependency-injection/ParameterBag/ParameterBag.php:102

did you update ctools module?

#39, I am also experiencing issues related to user temp store and Chaos Tools. I was good until Chaos Tools updated twice this month.

Any attempts to install ctools 8.x-3.1 or 8.x-3.2 (both released this month) causes any Drupal based page to crash once the code has been changed:

This is the message:
TypeError: Argument 1 passed to Drupal\ctools\ParamConverter\TempstoreConverter::__construct() must be an instance of Drupal\user\SharedTempStoreFactory, instance of Drupal\Core\TempStore\SharedTempStoreFactory given in {full_path_removed}\modules\ctools\src\ParamConverter\TempstoreConverter.php on line 102

The fault here appears to be that Chaos Tools updated to use the deprecated user tempstore.

I have a configuration that does not allow the use of Drush (and, yes, it is unusual but has been working in production for 3 years):

• Windows 10 (or Server 2012 R2)
• Apache 2.4.33 64bit
• PHP 7.2.5 64bit
• Drupal 8.6.10
• MS SQL Server 2012 and 2017

Ctools was updated to use the non-deprecated service. If that's the error then IMHO something went very wrong with your update. Make sure you don't have multiple versions of the ctools module lying around in different folder or so and that you updated all files. You might want to remove the folder completely and replace it.

If it's still not working then post in the ctools issue queue, this isn't a problem in core.

Edit: I just saw you commented there too, but as I said, something with your code base is not correct. Note that you can still use drush (or composer) to manage your files on a local/development system, even if you can't use it on your production server.