Skip to content
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

Locking is broken when ClassMetadata is cached #2278

Closed
buffcode opened this issue Feb 3, 2021 · 1 comment · Fixed by #2281
Closed

Locking is broken when ClassMetadata is cached #2278

buffcode opened this issue Feb 3, 2021 · 1 comment · Fixed by #2281
Milestone

Comments

@buffcode
Copy link
Contributor

buffcode commented Feb 3, 2021

Bug Report

Q A
BC Break no
Version all

Summary

Lock functionality is broken as soon as a non-volatile ClassMetadata cache is used.

Current behavior

When a metadata_cache_driver is defined, Doctrine MongoDB caches the ClassMetadata using the given driver by serializing all relevant fields. The fields isLockable and lockField are not serialized.

On a second request (or after the cache has been warmed), ClassMetadata has no information about locking-behaviour anymore and calls to ->lock(...) will fail with undefined index, as $this->class->lockField is empty [1] and there is no field mapping for this empty value.

#0 vendor/doctrine/mongodb-odm/lib/Doctrine/ODM/MongoDB/UnitOfWork.php(2325): Doctrine\ODM\MongoDB\Persisters\DocumentPersister->lock(Object(MyDocument), 4)
#1 vendor/doctrine/mongodb-odm/lib/Doctrine/ODM/MongoDB/DocumentManager.php(501): Doctrine\ODM\MongoDB\UnitOfWork->lock(Object(MyDocument), 4, NULL)

How to reproduce

  1. Configure a persistent metadata cache driver:
# config/packages/doctrine_mongodb.yaml 
services:
    app.doctrine_mongodb.redis.cache:
        class: Doctrine\Common\Cache\PredisCache
        arguments:
            - "@app.doctrine_mongodb.redis.inner"

    app.doctrine_mongodb.redis.inner:
        class: Predis\Client
        arguments:
           - "%env(REDIS_URL)%"

doctrine_mongodb:
    document_managers:
        default:
            auto_mapping: true
            metadata_cache_driver:
                type: service
                id:   app.doctrine_mongodb.redis.cache
  1. Warm cache (cache:warmup or trigger application)
  2. Try to lock the document by calling
$documentManager->lock($myDocument, LockMode::PESSIMISTIC_WRITE);
  1. Notice is thrown, no locking occurs.

Expected behavior

Lock functionality should work, even when ClassMetadata is cached.

Solution

Include isLockable and lockField in serialization (PRs incoming).

buffcode added a commit to buffcode/mongodb-odm that referenced this issue Feb 3, 2021
Caching / unserializing ClassMetadata broke locking functionality

Fixes doctrine#2278
buffcode added a commit to buffcode/mongodb-odm that referenced this issue Feb 3, 2021
Caching / unserializing ClassMetadata broke locking functionality

Fixes doctrine#2278
buffcode added a commit to buffcode/mongodb-odm that referenced this issue Feb 3, 2021
Caching / unserializing ClassMetadata broke locking functionality

Fixes doctrine#2278
@malarzm malarzm added this to the 2.2.1 milestone Feb 3, 2021
@malarzm malarzm linked a pull request Feb 3, 2021 that will close this issue
@malarzm
Copy link
Member

malarzm commented Apr 14, 2021

Forgot to link issues before, closing as #2281 was merged

@malarzm malarzm closed this as completed Apr 14, 2021
alcaeus added a commit that referenced this issue May 21, 2021
* Fix locking when ClassMetadata is unserialized

Caching / unserializing ClassMetadata broke locking functionality

Fixes #2278

* Test serialization of lock/version fields

* Update working-with-objects.rst

Detach doc text from code block

* Update storage-strategies.rst

* Fix invalid strict comparison when validating mappings

* Correctly handle write concern specified in defaultCommitOptions (#2294)

* Fix documentation for uploadFromFile

* Fix mapping of the nullable option for XML driver

* Fix query preparation when in elemMatch (#2299)

* Fix preparation of $elemMatch operators in queries (#2298)

* Fix using null values in partial filter expressions (#2300)

* Fix errors with nullable typed associations (#2302)

* Fix initialising nullable associations

* Fix error when merging documents with uninitialised typed properties

Co-authored-by: buffcode <[email protected]>
Co-authored-by: Laurens Stötzel <[email protected]>
Co-authored-by: Maciej Malarz <[email protected]>
Co-authored-by: jeeiex <[email protected]>
Co-authored-by: Claudio Zizza <[email protected]>
Co-authored-by: Andreas Braun <[email protected]>
Co-authored-by: Gocha Ossinkine <[email protected]>
Co-authored-by: Ryan RAJKOMAR <[email protected]>
Co-authored-by: wuchen90 <[email protected]>
Co-authored-by: Andreas Braun <[email protected]>
alcaeus added a commit that referenced this issue Aug 5, 2021
* Fix locking when ClassMetadata is unserialized

Caching / unserializing ClassMetadata broke locking functionality

Fixes #2278

* Test serialization of lock/version fields

* Update working-with-objects.rst

Detach doc text from code block

* Update storage-strategies.rst

* Fix invalid strict comparison when validating mappings

* Correctly handle write concern specified in defaultCommitOptions (#2294)

* Fix documentation for uploadFromFile

* Fix mapping of the nullable option for XML driver

* Fix query preparation when in elemMatch (#2299)

* Fix preparation of $elemMatch operators in queries (#2298)

* Fix using null values in partial filter expressions (#2300)

* Fix errors with nullable typed associations (#2302)

* Fix initialising nullable associations

* Fix error when merging documents with uninitialised typed properties

* Allow mixed value in $not operator (#2307)

* [2.2] Fix builds (#2319)

* Fix wrong handling for nullable fields in upsert and update (#2318)

* Comprehensively test nullable behaviour for embedOne

Co-authored-by: wuchen90 <[email protected]>

* Fix handling of nullable fields for upsert

Co-authored-by: wuchen90 <[email protected]>

* Fix handling of upserts during scheduling for deletion (#2334)

* Fix handling of upserts during scheduling for deletion

* Added test

* Fix wrong assertion (#2335)

This was uncovered by Psalm testing when merging 2.2.x up into 2.3.x.

* Remove psalm-baseline.xml

Co-authored-by: buffcode <[email protected]>
Co-authored-by: Laurens Stötzel <[email protected]>
Co-authored-by: Maciej Malarz <[email protected]>
Co-authored-by: jeeiex <[email protected]>
Co-authored-by: Claudio Zizza <[email protected]>
Co-authored-by: Gocha Ossinkine <[email protected]>
Co-authored-by: Ryan RAJKOMAR <[email protected]>
Co-authored-by: wuchen90 <[email protected]>
Co-authored-by: Fran Moreno <[email protected]>
Co-authored-by: Bernhard Schussek <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants