doctrine/archived-orm
1
0
Fork 0
mirror of https://github.com/doctrine/orm.git synced 2026-03-24 06:52:09 +01:00
Code Issues 5.3k Packages Projects Releases 100 Wiki Activity

Cache: READ_WRITE failing Argument 2 passed to... #6384

New Issue
Closed
opened 2026-01-22 15:32:09 +01:00 by admin · 10 comments
admin commented 2026-01-22 15:32:09 +01:00
Owner
Copy Link

Originally created by @NikoGrano on GitHub (Jan 14, 2020).

Bug Report

Q A
BC Break ?
Version 2.7.0

Summary

Entity:

 * @ORM\Entity
 * @ORM\Table(name="stock_inventory")
 * @ORM\Cache(usage="READ_WRITE", region="locking")

Configuration (Bundle for Symfony)

    orm:
        auto_generate_proxy_classes: '%kernel.debug%'
        entity_managers:
            default:
                naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
                auto_mapping: true
                second_level_cache:
                    region_cache_driver:
                        type: service
                        id: cache.doctrine.redis
                        pool: 'dl2_'
                    enabled: true
                    regions:
                        locking:
                            cache_driver:
                                type: service
                                id: cache.doctrine.redis
                                pool: 'dl2l_'
                            lock_path: '%kernel.cache_dir%/doctrine/orm/lock'

While correct configuration would be

    orm:
        ...
        entity_managers:
            default:
                ...
                second_level_cache:
                    ...
                    regions:
                        locking:
                            lock_path: '%kernel.cache_dir%/doctrine/orm/lock'
                            type: filelock

Current behavior

Running a write into the database will cause

 [TypeError]                                                                                                                                                                                                     
  Argument 2 passed to Doctrine\ORM\Cache\Persister\Entity\ReadWriteCachedEntityPersister::__construct() must be an instance of Doctrine\ORM\Cache\ConcurrentRegion, instance of Doctrine\ORM\Cache\Region\Defau  
  ltRegion given, called in /app/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php on line 132  

Exception trace:
  at /app/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php:44
 Doctrine\ORM\Cache\Persister\Entity\ReadWriteCachedEntityPersister->__construct() at /app/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php:132
 Doctrine\ORM\Cache\DefaultCacheFactory->buildCachedEntityPersister() at /app/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:3133
 Doctrine\ORM\UnitOfWork->getEntityPersister() at /app/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:1065
 Doctrine\ORM\UnitOfWork->executeInserts() at /app/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:400
 Doctrine\ORM\UnitOfWork->commit() at /app/vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php:368
 Doctrine\ORM\EntityManager->flush() at projectRelatedFile.php:50

Digging the issue

So this is caused due here we are expecting Region interface. This is propably correct functionality. Here we pass it to the next class, which causes this error. The problem is, in the Persister constructor we are declaring we want this one specific type.

This should be fixed with a interface, abstract class or something. Any toughts? Or is this something for 3.0/wontfix?

How to reproduce

Configure previous settings and try persist something to the database.

Expected behavior

Throw a handled exception, not a type error.

Notes

I'm not sure how this architecture fully works, so I would like if somebody else familiar with this would take this. With correct configuration this works, but I wasn't sure if I should post bug about this, however I decided to post it just to make sure this is intended functionality.

Originally created by @NikoGrano on GitHub (Jan 14, 2020). ### Bug Report | Q | A |------------ | ------ | BC Break | ? | Version | 2.7.0 #### Summary Entity: ``` * @ORM\Entity * @ORM\Table(name="stock_inventory") * @ORM\Cache(usage="READ_WRITE", region="locking") ``` Configuration (Bundle for Symfony) ```yaml orm: auto_generate_proxy_classes: '%kernel.debug%' entity_managers: default: naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware auto_mapping: true second_level_cache: region_cache_driver: type: service id: cache.doctrine.redis pool: 'dl2_' enabled: true regions: locking: cache_driver: type: service id: cache.doctrine.redis pool: 'dl2l_' lock_path: '%kernel.cache_dir%/doctrine/orm/lock' ``` **While correct configuration would be** ```yaml orm: ... entity_managers: default: ... second_level_cache: ... regions: locking: lock_path: '%kernel.cache_dir%/doctrine/orm/lock' type: filelock ``` #### Current behavior Running a write into the database will cause ```php [TypeError] Argument 2 passed to Doctrine\ORM\Cache\Persister\Entity\ReadWriteCachedEntityPersister::__construct() must be an instance of Doctrine\ORM\Cache\ConcurrentRegion, instance of Doctrine\ORM\Cache\Region\Defau ltRegion given, called in /app/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php on line 132 Exception trace: at /app/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php:44 Doctrine\ORM\Cache\Persister\Entity\ReadWriteCachedEntityPersister->__construct() at /app/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php:132 Doctrine\ORM\Cache\DefaultCacheFactory->buildCachedEntityPersister() at /app/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:3133 Doctrine\ORM\UnitOfWork->getEntityPersister() at /app/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:1065 Doctrine\ORM\UnitOfWork->executeInserts() at /app/vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:400 Doctrine\ORM\UnitOfWork->commit() at /app/vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php:368 Doctrine\ORM\EntityManager->flush() at projectRelatedFile.php:50 ``` #### Digging the issue So this is caused due [here](https://github.com/doctrine/orm/blob/master/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php#L72) we are expecting Region interface. This is propably correct functionality. [Here](https://github.com/doctrine/orm/blob/master/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php#L100) we pass it to the next class, which causes this error. The problem is, in the Persister [constructor](https://github.com/doctrine/orm/blob/master/lib/Doctrine/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php#L24) we are declaring we want this one specific type. This should be fixed with a interface, abstract class or something. Any toughts? Or is this something for 3.0/wontfix? #### How to reproduce Configure previous settings and try persist something to the database. #### Expected behavior Throw a handled exception, not a type error. #### Notes I'm not sure how this architecture fully works, so I would like if somebody else familiar with this would take this. With correct configuration this works, but I wasn't sure if I should post bug about this, however I decided to post it just to make sure this is intended functionality.
admin closed this issue 2026-01-22 15:32:09 +01:00
admin commented 2026-01-22 15:32:10 +01:00
Author
Owner
Copy Link

@lcobucci commented on GitHub (Jan 14, 2020):

@NikoGrano did you manage to reproduce this using the ORM only? This seems to be an integration (or configuration) issue since the code that creates the decorator seems to be bypassed... 04036f2244/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php (L178-L188)

@lcobucci commented on GitHub (Jan 14, 2020): @NikoGrano did you manage to reproduce this using the ORM only? This seems to be an integration (or configuration) issue since the code that creates the decorator seems to be bypassed... https://github.com/doctrine/orm/blob/04036f22446b7df7cca6c57ba16d01c65b8099bf/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php#L178-L188
admin commented 2026-01-22 15:32:10 +01:00
Author
Owner
Copy Link

@NikoGrano commented on GitHub (Jan 15, 2020):

Hey!

I agree on that this is more configuration error on bundle side, than a issue. However, I think there should be \LogicException thrown always instead of hard debuggable type-error.

The previous code you linked, it gets by passed due the region is set by using following method. Then after in second block, we are trying to get region, however, due it is already set, it will take it from the regions array instead of creating a new one.
04036f2244/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php (L72-L75)

04036f2244/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php (L109-L117)

04036f2244/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php (L164-L171)

So setting a region with setRegion is correct usage. However, the given DefaultRegion is valid based on the interface, but it is not valid in case of READ_WRITE action, due it does require ConcurrentRegion.

04036f2244/lib/Doctrine/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php (L16-L27)

Adding a check instaceof check here would fix this and make debugging much easier, but is it correct approach? More worried about that.

04036f2244/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php (L123-L125)

@NikoGrano commented on GitHub (Jan 15, 2020): Hey! I agree on that this is more configuration error on bundle side, than a issue. However, I think there should be `\LogicException` thrown always instead of hard debuggable type-error. The previous code you linked, it gets by passed due the region is set by using following method. Then after in second block, we are trying to get region, however, due it is already set, it will take it from the regions array instead of creating a new one. https://github.com/doctrine/orm/blob/04036f22446b7df7cca6c57ba16d01c65b8099bf/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php#L72-L75 https://github.com/doctrine/orm/blob/04036f22446b7df7cca6c57ba16d01c65b8099bf/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php#L109-L117 https://github.com/doctrine/orm/blob/04036f22446b7df7cca6c57ba16d01c65b8099bf/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php#L164-L171 So setting a region with `setRegion` is correct usage. However, the given `DefaultRegion` is valid based on the interface, but it is not valid in case of `READ_WRITE` action, due it does require `ConcurrentRegion`. https://github.com/doctrine/orm/blob/04036f22446b7df7cca6c57ba16d01c65b8099bf/lib/Doctrine/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php#L16-L27 Adding a check `instaceof` check here would fix this and make debugging much easier, but is it correct approach? More worried about that. https://github.com/doctrine/orm/blob/04036f22446b7df7cca6c57ba16d01c65b8099bf/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php#L123-L125
admin commented 2026-01-22 15:32:10 +01:00
Author
Owner
Copy Link

@NikoGrano commented on GitHub (Jan 15, 2020):

Checking this futher, for example I would propose following patch for this:
See #7984

I would request comments for this. If it's fine, I will create UnitTests.

diff --git a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php
index bfc6aa29a..ce2ac4d4e 100644
--- a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php
+++ b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php
@@ -196,7 +196,14 @@ class DefaultCacheFactory implements CacheFactory
     public function getRegion(array $cache)
     {
         if (isset($this->regions[$cache['region']])) {
-            return $this->regions[$cache['region']];
+            $region = $this->regions[$cache['region']];
+            if ($cache['usage'] === ClassMetadata::CACHE_USAGE_READ_WRITE &&
+                !($region instanceof ConcurrentRegion)
+            ) {
+                throw new \LogicException('If you want to use a "READ_WRITE" cache an implementation of "Doctrine\ORM\Cache\ConcurrentRegion" is required');
+            }
+
+            return $region;
         }
 
         $name         = $cache['region'];
@NikoGrano commented on GitHub (Jan 15, 2020): Checking this futher, for example I would propose following patch for this: See #7984 I would request comments for this. If it's fine, I will create UnitTests. ```diff diff --git a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php index bfc6aa29a..ce2ac4d4e 100644 --- a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php +++ b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php @@ -196,7 +196,14 @@ class DefaultCacheFactory implements CacheFactory public function getRegion(array $cache) { if (isset($this->regions[$cache['region']])) { - return $this->regions[$cache['region']]; + $region = $this->regions[$cache['region']]; + if ($cache['usage'] === ClassMetadata::CACHE_USAGE_READ_WRITE && + !($region instanceof ConcurrentRegion) + ) { + throw new \LogicException('If you want to use a "READ_WRITE" cache an implementation of "Doctrine\ORM\Cache\ConcurrentRegion" is required'); + } + + return $region; } $name = $cache['region']; ```
admin commented 2026-01-22 15:32:11 +01:00
Author
Owner
Copy Link

@NikoGrano commented on GitHub (Jan 17, 2020):

@lcobucci Sorry for pining. Does this look a good solution, worth of continuing working on PR?

@NikoGrano commented on GitHub (Jan 17, 2020): @lcobucci Sorry for pining. Does this look a good solution, worth of continuing working on PR?
admin commented 2026-01-22 15:32:11 +01:00
Author
Owner
Copy Link

@lcobucci commented on GitHub (Jan 18, 2020):

@NikoGrano the problem is that (as far as I remember) this method is used in places where performance is needed and adding any additional check might mess that up.

I do agree that the design is fragile, though.

@lcobucci commented on GitHub (Jan 18, 2020): @NikoGrano the problem is that (as far as I remember) this method is used in places where performance is needed and adding any additional check might mess that up. I do agree that the design is fragile, though.
admin commented 2026-01-22 15:32:11 +01:00
Author
Owner
Copy Link

@NikoGrano commented on GitHub (Jan 18, 2020):

@lcobucci I was worrying about exactly about same. Maybe we should check if the flow changes in 3.0 or close in as "Won't Fix" and Known Issue. It is anyways configuration issue.

Anyways, when somebody sees this issue in future, he/she can work the solution based on this issue.

@NikoGrano commented on GitHub (Jan 18, 2020): @lcobucci I was worrying about exactly about same. Maybe we should check if the flow changes in 3.0 or close in as "Won't Fix" and Known Issue. It is anyways configuration issue. Anyways, when somebody sees this issue in future, he/she can work the solution based on this issue.
admin commented 2026-01-22 15:32:11 +01:00
Author
Owner
Copy Link

@andreybolonin commented on GitHub (Dec 7, 2021):

@lcobucci the same error

@andreybolonin commented on GitHub (Dec 7, 2021): @lcobucci the same error
admin commented 2026-01-22 15:32:12 +01:00
Author
Owner
Copy Link

@NikoGrano commented on GitHub (Dec 7, 2021):

Still would recommend implementing #7984 or something similar to avoid pings like this. Clear error what you are doing wrong instead of type error makes stuff easier to figure out.

@NikoGrano commented on GitHub (Dec 7, 2021): Still would recommend implementing #7984 or something similar to avoid pings like this. Clear error what you are doing wrong instead of type error makes stuff easier to figure out.
admin commented 2026-01-22 15:32:12 +01:00
Author
Owner
Copy Link

@lcobucci commented on GitHub (Dec 7, 2021):

@andreybolonin please send a patch that fixes without introducing performance issues. I'm pretty sure that @greg0ire and the other folks from the ORM team will be happy to help out.

@lcobucci commented on GitHub (Dec 7, 2021): @andreybolonin please send a patch that fixes without introducing performance issues. I'm pretty sure that @greg0ire and the other folks from the ORM team will be happy to help out.
admin commented 2026-01-22 15:32:12 +01:00
Author
Owner
Copy Link

@NikoGrano commented on GitHub (Nov 25, 2024):

Closed as stale.

@NikoGrano commented on GitHub (Nov 25, 2024): Closed as stale.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
BC Break
Bug
Can't Fix
Deprecation
Documentation
DQL
Duplicate
Failing Test
Hacktoberfest
Improvement
Improvement
Incomplete
Invalid
Missing Tests
New Feature
PR Created
pull-request
Mirrored from GitHub Pull Request
 Question
Regression
Status: Waiting feedback
Waiting feedback
WIP
Won't Fix
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
admin
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6384
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?