Skip to content

Fix locking when ClassMetadata is unserialized #2281

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 4, 2021
Merged

Fix locking when ClassMetadata is unserialized #2281

merged 2 commits into from
Feb 4, 2021

Conversation

buffcode
Copy link
Contributor

@buffcode buffcode commented Feb 3, 2021

Q A
Type bug
BC Break no
Fixed issues #2278

Summary

Caching / unserializing ClassMetadata broke locking functionality

@buffcode
Fix locking when ClassMetadata is unserialized
887a62e 
Caching / unserializing ClassMetadata broke locking functionality

Fixes doctrine#2278
@SenseException
Copy link
Member

@malarzm The website config lists 2.2.x as the maintained current branch and 2.3.x as the upcoming, while the default branch is set to 2.3.x. Which branch should this PR target?

@malarzm
Copy link
Member

malarzm commented Feb 3, 2021

This PR shall prevail :) I'll be closing the other ones and review this one shortly

@SenseException SenseException mentioned this pull request Feb 3, 2021
Closed
malarzm
malarzm requested changes Feb 3, 2021
View reviewed changes
Copy link
Member

@malarzm malarzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! The change on it's own is great however we need to have it covered by a test to prevent regressions in future. At a glance this test here

mongodb-odm/tests/Doctrine/ODM/MongoDB/Tests/Mapping/ClassMetadataTest.php

Line 36 in 06a7f2f

public function testClassMetadataInstanceSerialization()
looks like a good candidate.

@malarzm malarzm added the Bug label Feb 3, 2021
@malarzm malarzm added this to the 2.2.1 milestone Feb 3, 2021
@malarzm malarzm linked an issue Feb 3, 2021 that may be closed by this pull request
Locking is broken when ClassMetadata is cached #2278
Closed
@buffcode
Copy link
Contributor Author

buffcode commented Feb 4, 2021

@malarzm I have added assertions to the ClassMetadataTest for this issue. I've also added assertions for isVersioned and versionField though they are not affected, I hope thats okay.

buffcode
buffcode commented Feb 4, 2021
View reviewed changes
Copy link
Contributor Author

@buffcode buffcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adressed in a97aab0

malarzm
malarzm approved these changes Feb 4, 2021
View reviewed changes
Copy link
Member

@malarzm malarzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's great, thanks for adding versioning to the test as well

@malarzm malarzm changed the title [2.2.x] Fix locking when ClassMetadata is unserialized Fix locking when ClassMetadata is unserialized Feb 4, 2021
@malarzm malarzm merged commit cbe6a2f into doctrine:2.2.x Feb 4, 2021
@malarzm
Copy link
Member

malarzm commented Feb 4, 2021

Thanks @buffcode!

@buffcode buffcode deleted the patch-3 branch February 4, 2021 10:29
@malarzm malarzm mentioned this pull request Apr 14, 2021
Closed
@github-actions github-actions bot mentioned this pull request May 15, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@malarzm malarzm malarzm approved these changes

Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
2.2.1
Development

Successfully merging this pull request may close these issues.

Locking is broken when ClassMetadata is cached
3 participants
@buffcode @SenseException @malarzm