Skip to content

Fix a deprecation message about PdoCacheAdapterDoctrineSchemaSubscriber #1417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

javiereguiluz
Copy link
Contributor

This tries to fix the following deprecation:

  1x: Since symfony/doctrine-bridge 5.4: The "Symfony\Bridge\Doctrine\SchemaListener\PdoCacheAdapterDoctrineSchemaSubscriber" class is deprecated, use "Symfony\Bridge\Doctrine\SchemaListener\DoctrineDbalCacheAdapterSchemaSubscriber" instead.
    1x in AddUserCommandTest::testCreateUserNonInteractive from App\Tests\Command

which we found when upgrading Symfony Demo app to the upcoming Symfony 5.4. See symfony/demo#1268 (comment)

@ostrolucky
Copy link
Member

Hmm according CI failing it looks like this needs more work

@javiereguiluz
Copy link
Contributor Author

Yes. To be honest, I don't know what I'm doing here ... so I need help to solve this. Thanks!

@stof
Copy link
Member

stof commented Nov 9, 2021

Well, what you need to do is replacing the doctrine.orm.listeners.pdo_cache_adapter_doctrine_schema_subscriber by a doctrine.orm.listeners.dbal_cache_adapter_doctrine_schema_subscriber service using the new class.

And then, if the new class exists, removing the doctrine.orm.listeners.pdo_cache_adapter_doctrine_schema_subscriber. Otherwise, remove the new service, and remove the old service if none of the class exist.

@jt2k
Copy link

jt2k commented Nov 11, 2021

I was looking to see if anyone was addressing this deprecation yet, and found this issue. @javiereguiluz, are you planning to continue working on this? If not, I'd be happy to work on a new PR.

@ostrolucky
Copy link
Member

Yeah something like this needs to be done, but that's still incomplete, as CacheSchemaSubscriberPass needs to be updated too.

Index: Tests/CacheSchemaSubscriberTest.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Tests/CacheSchemaSubscriberTest.php b/Tests/CacheSchemaSubscriberTest.php
--- a/Tests/CacheSchemaSubscriberTest.php	
+++ b/Tests/CacheSchemaSubscriberTest.php	
@@ -5,7 +5,7 @@
 use Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler\CacheSchemaSubscriberPass;
 use Doctrine\Bundle\DoctrineBundle\DependencyInjection\DoctrineExtension;
 use Doctrine\ORM\EntityManagerInterface;
-use Symfony\Bridge\Doctrine\SchemaListener\PdoCacheAdapterDoctrineSchemaSubscriber;
+use Symfony\Bridge\Doctrine\SchemaListener\DoctrineDbalCacheAdapterSchemaSubscriber;
 use Symfony\Bundle\FrameworkBundle\DependencyInjection\FrameworkExtension;
 use Symfony\Component\DependencyInjection\Alias;
 use Symfony\Component\DependencyInjection\Compiler\PassConfig;
@@ -21,8 +21,8 @@
 {
     public function testSchemaSubscriberWiring(): void
     {
-        if (! class_exists(PdoCacheAdapterDoctrineSchemaSubscriber::class)) {
-            $this->markTestSkipped('This test requires Symfony 5.1 or higher');
+        if (! class_exists(DoctrineDbalCacheAdapterSchemaSubscriber::class)) {
+            $this->markTestSkipped('This test requires Symfony 5.4 or higher');
         }
 
         if (! interface_exists(EntityManagerInterface::class)) {
@@ -51,7 +51,7 @@
             'framework' => [
                 'cache' => [
                     'pools' => [
-                        'my_cache_adapter' => ['adapter' => 'cache.adapter.pdo'],
+                        'my_cache_adapter' => ['adapter' => 'cache.adapter.dbal'],
                     ],
                 ],
             ],
Index: Resources/config/orm.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Resources/config/orm.xml b/Resources/config/orm.xml
--- a/Resources/config/orm.xml	
+++ b/Resources/config/orm.xml	
@@ -134,7 +134,7 @@
 
         <!-- listeners -->
         <service id="doctrine.orm.listeners.resolve_target_entity" class="%doctrine.orm.listeners.resolve_target_entity.class%" public="false" />
-        <service id="doctrine.orm.listeners.pdo_cache_adapter_doctrine_schema_subscriber" class="Symfony\Bridge\Doctrine\SchemaListener\PdoCacheAdapterDoctrineSchemaSubscriber">
+        <service id="doctrine.orm.listeners.pdo_cache_adapter_doctrine_schema_subscriber" class="Symfony\Bridge\Doctrine\SchemaListener\DoctrineDbalCacheAdapterSchemaSubscriber">
             <argument type="collection" /> <!-- PdoAdapter instances -->
             <tag name="doctrine.event_subscriber" />
         </service>
Index: DependencyInjection/DoctrineExtension.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/DependencyInjection/DoctrineExtension.php b/DependencyInjection/DoctrineExtension.php
--- a/DependencyInjection/DoctrineExtension.php	
+++ b/DependencyInjection/DoctrineExtension.php	
@@ -27,6 +27,7 @@
 use Symfony\Bridge\Doctrine\Messenger\DoctrineClearEntityManagerWorkerSubscriber;
 use Symfony\Bridge\Doctrine\Messenger\DoctrineTransactionMiddleware;
 use Symfony\Bridge\Doctrine\PropertyInfo\DoctrineExtractor;
+use Symfony\Bridge\Doctrine\SchemaListener\DoctrineDbalCacheAdapterSchemaSubscriber;
 use Symfony\Bridge\Doctrine\SchemaListener\MessengerTransportDoctrineSchemaSubscriber;
 use Symfony\Bridge\Doctrine\SchemaListener\PdoCacheAdapterDoctrineSchemaSubscriber;
 use Symfony\Bridge\Doctrine\SchemaListener\RememberMeTokenProviderDoctrineSchemaSubscriber;
@@ -435,9 +436,16 @@
             $container->getDefinition('form.type.entity')->addTag('kernel.reset', ['method' => 'reset']);
         }
 
-        // available in Symfony 5.1 and higher
-        if (! class_exists(PdoCacheAdapterDoctrineSchemaSubscriber::class)) {
-            $container->removeDefinition('doctrine.orm.listeners.pdo_cache_adapter_doctrine_schema_subscriber');
+
+        // available in Symfony 5.4 and higher
+        if (! class_exists(DoctrineDbalCacheAdapterSchemaSubscriber::class)) {
+            $schemaSubscriberId = 'doctrine.orm.listeners.pdo_cache_adapter_doctrine_schema_subscriber';
+            if (class_exists(PdoCacheAdapterDoctrineSchemaSubscriber::class)) {
+                // available in Symfony 5.1 and higher
+                $container->getDefinition($schemaSubscriberId)->setClass(PdoCacheAdapterDoctrineSchemaSubscriber::class);
+            } else {
+                $container->removeDefinition($schemaSubscriberId);
+            }
         }
 
         // available in Symfony 5.3 and higher

@jt2k would be great if you finished this!

@GromNaN
Copy link
Member

GromNaN commented Nov 16, 2021

Thank you @javiereguiluz spotting this issue. I missed a major part during the split between PdoAdapter and DoctrineDbalAdapter.
Here is an alternative to this PR: #1421

fabpot added a commit to symfony/symfony that referenced this pull request Nov 17, 2021
…ter (GromNaN)

This PR was merged into the 5.4 branch.

Discussion
----------

[FrameworkBundle] Add framework config for DBAL cache adapter

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | doctrine/DoctrineBundle#1417
| License       | MIT
| Doc PR        | -

The framework configuration was missing from #43362.

Additionnaly, the depreciation message on `PdoCacheAdapterDoctrineSchemaSubscriber` must be removed. This class needs to be used in 5.4 whenever a `PdoAdapter` is used, because it could have a DBAL connection and we need to keep the deprecated behavior. A depreciation message is already triggered in the `PdoAdapter` itself when it gets a DBAL connection.

Commits
-------

672545d Add framework config for DBAL cache adapter
@ostrolucky ostrolucky closed this Nov 19, 2021
@jt2k
Copy link

jt2k commented Nov 19, 2021

@GromNaN Thanks you! I started in on this last weekend but got tripped up with the corresponding changes that needed to happen in the FrameworkBundle.

@javiereguiluz
Copy link
Contributor Author

@GromNaN thanks for properly fixing this issue 🙏

@javiereguiluz javiereguiluz deleted the patch-2 branch November 24, 2021 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants