Skip to content

[Lock] Add LockKeyNormalizer #60023

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
Open
wants to merge 1 commit into
base: 7.4
Choose a base branch
from
Open

[Lock] Add LockKeyNormalizer #60023

wants to merge 1 commit into from
+121 −1

Conversation

valtzu
Copy link
Contributor

@valtzu valtzu commented Mar 23, 2025 •
edited
Loading

Q A
Branch? 7.4
Bug fix? no
New feature? yes
Deprecations? no
License MIT

The Lock documentation mentions that the lock keys are serializable – however, it seems that's the case only if you're using native php serializer.

I'm proposing to add LockKeyNormalizer to make the example in the docs work the same when using Symfony Serializer too.

@valtzu valtzu requested a review from jderusse as a code owner March 23, 2025 12:41
@carsonbot carsonbot added this to the 7.3 milestone Mar 23, 2025
@valtzu valtzu force-pushed the lock-normalizer branch 2 times, most recently from 89d05b7 to d1cd6e1 Compare March 23, 2025 12:51
nicolas-grekas
nicolas-grekas reviewed Mar 23, 2025
View reviewed changes
stof
stof reviewed Mar 28, 2025
View reviewed changes
@fabpot fabpot modified the milestones: 7.3, 7.4 May 26, 2025
@stof stof mentioned this pull request Jun 4, 2025
Open
@Steveb-p Steveb-p mentioned this pull request Jul 18, 2025
Merged
andersonamuller
andersonamuller reviewed Jul 21, 2025
View reviewed changes
OskarStark
OskarStark reviewed Jul 21, 2025
View reviewed changes
@valtzu valtzu force-pushed the lock-normalizer branch from ab729b1 to a9201d2 Compare July 21, 2025 10:33
@valtzu valtzu force-pushed the lock-normalizer branch 2 times, most recently from 090db9a to be1f46f Compare August 23, 2025 15:15
@valtzu
Copy link
Contributor Author

valtzu commented Aug 23, 2025 •
edited
Loading

Since native serialization is handled within Key class, maybe it'd make sense to do the same for JSON? Just remove the LockKeyNormalizer and implement \JsonSerializable on Key instead. What do you think?

EDIT: right, it would not work on deserialization..

@valtzu valtzu force-pushed the lock-normalizer branch 2 times, most recently from 4264be0 to 9a45e82 Compare August 23, 2025 15:32
@valtzu valtzu changed the title [Lock] Add LockKeyNormalizer [Lock][FrameworkBundle] Add LockKeyNormalizer Aug 23, 2025
fabpot
fabpot requested changes Sep 4, 2025
View reviewed changes
src/Symfony/Component/Lock/Tests/Serializer/LockKeyNormalizerTest.php Outdated

protected function setUp(): void
{
$this->normalizer = new LockKeyNormalizer();
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove this setup and create new instances in each test for more isolation and less memory consumption.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, done

@carsonbot carsonbot changed the title [Lock][FrameworkBundle] Add LockKeyNormalizer [Lock] Add LockKeyNormalizer Sep 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@stof stof stof left review comments

@OskarStark OskarStark OskarStark left review comments

@fabpot fabpot fabpot requested changes

@jderusse jderusse Awaiting requested review from jderusse jderusse is a code owner

+1 more reviewer

@andersonamuller andersonamuller andersonamuller left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Feature Lock Status: Needs Work
Projects
None yet
Milestone
7.4
Development

Successfully merging this pull request may close these issues.
7 participants
@valtzu @fabpot @nicolas-grekas @stof @OskarStark @andersonamuller @carsonbot