8000 [Messenger] Add RedeliveryStamp (de)normalizer to avoid deprecations by Jean85 · Pull Request #45701 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Messenger] Add RedeliveryStamp (de)normalizer to avoid deprecations #45701

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
Closed
wants to merge 5 commits into from
Closed

[Messenger] Add RedeliveryStamp (de)normalizer to avoid deprecations #45701

wants to merge 5 commits into from

Conversation

Jean85
Copy link
Contributor
@Jean85 Jean85 commented Mar 10, 2022
Q A
Branch? 5.4
Bug fix? no
New feature? no
Deprecations? no (silencing existing ones)
Tickets N/A
License MIT

While working on the deprecations triggered by my 5.4 app, this is the first one that I stumbled upon:

symfony/src/Symfony/Component/Messenger/Stamp/RedeliveryStamp.php

Lines 30 to 36 in b30675a

public function __construct(int $retryCount, $exceptionMessage = null, FlattenException $flattenException = null, \DateTimeInterface $redeliveredAt = null)
{
$this->retryCount = $retryCount;
$this->redeliveredAt = $redeliveredAt ?? new \DateTimeImmutable();
if (null !== $redeliveredAt) {
trigger_deprecation('symfony/messenger', '5.2', sprintf('Using the "$redeliveredAt" as 4th argument of the "%s::__construct()" is deprecated, pass "$redeliveredAt" as second argument instead.', self::class));
}

This was not intentionally triggered by me, but it stemmed from the normal JSON deserialization process that the Symfony serializer does; if I upgraded to 6.0 today I wouldn't need to do anything about it, hence these deprecations are just noise to me.

By adding this proposed denormalizer, we avoid triggering the deprecation in systems that already handle it correctly due to framework upgrades.

I don't know if existing tests already cover this, please point me to where I can add them if needed.

@carsonbot carsonbot added this to the 5.4 milestone Mar 10, 2022
@Jean85 Jean85 changed the title Add RedeliveryStamp (de)normalizer to avoid deprecations [Messenger] Add RedeliveryStamp (de)normalizer to avoid deprecations Mar 10, 2022
Jean85
Jean85 commented Mar 11, 2022
View reviewed changes
...Symfony/Component/Messenger/Transport/Serialization/Normalizer/RedeliveryStampNormalizer.php
throw new InvalidArgumentException();
}

$dateTimeFormat = $context[DateTimeNormalizer::FORMAT_KEY] ?? \DateTimeInterface::RFC3339;
8000
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this the correct approach? Or is there some solution to delegate this normalization to the default serialization (and to DateTimeNormalizer)?

@nicolas-grekas nicolas-grekas mentioned this pull request Apr 18, 2023
Merged
@nicolas-grekas
Copy link
Member

Closing in favor of #50049, thanks for raising this.

nicolas-grekas added a commit that referenced this pull request Apr 18, 2023
@nicolas-grekas
bug #50049 [Messenger] Fix deprecation layer of RedeliveryStamp (nico…
6366ed2 
…las-grekas)

This PR was merged into the 5.4 branch.

Discussion
----------

[Messenger] Fix deprecation layer of RedeliveryStamp

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #45701
| License       | MIT
| Doc PR        | -

Commits
-------

8b4740d [Messenger] Fix deprecation layer of RedeliveryStamp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Status: Needs Review
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
3 participants
@Jean85 @nicolas-grekas @carsonbot
0