Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[FrameworkBundle] Fix sorting bug in sorting of tagged services by priority#45399

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:4.4fromAhummeling:fix-sorting-bug
Feb 11, 2022

Conversation

@Ahummeling
Copy link
Contributor

Fixed incorrect assumption that the minimum priority is zero
leading to an unsorted list of negative priorities.

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix#45396
LicenseMIT
Doc PR

Fixed an incorrect assumption that the minimum priority is zero which was leading to an unsorted list of negative priorities.
(See#45396)

@dkarlovi
Copy link
Contributor

@Ahummeling if I understood correctly, it means the positive priorities are sorted correct, but negative are not?

IssortTaggedServicesByPriority used to actually inject the services into the serializer? It seems unfortunate we'd have one method sorting them for display and another sorting them for injection, they'd always be in danger to go out of sync. 🤔

@Ahummeling
Copy link
ContributorAuthor

@Ahummeling if I understood correctly, it means the positive priorities are sorted correct, but negative are not?

IssortTaggedServicesByPriority used to actually inject the services into the serializer? It seems unfortunate we'd have one method sorting them for display and another sorting them for injection, they'd always be in danger to go out of sync. thinking

Yes you are understanding this correctly.

The class responsible for sorting the output of the debug command is theSymfony\Bundle\FrameworkBundle\Console\Descriptor\Descriptor. I can't imagine this is used in the serializer, which would explain the differences.

dkarlovi reacted with thumbs up emoji

@Ahummeling
Copy link
ContributorAuthor

TheSerializerPass seems to callSymfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait::findAndSortTaggedServices
which has a completely different algorithm for sorting the services, but I believe it still initializes default priority to 0 in the case of the serializers. I am not quite sure how it works exactly, as it's quite a bit more complicated

@nicolas-grekas
Copy link
Member

Can you please add a test case?

@Ahummeling
Copy link
ContributorAuthor

Can you please add a test case?

Certainly, sorry should've done that before making the PR

Fixed incorrect assumption that the minimum priority is zeroleading to an unsorted list of negative priorities.
@Ahummeling
Copy link
ContributorAuthor

Added a test case to the relevant provider that would fail without the proposed change and updated the relevant fixtures to reflect this additional testcase.

@nicolas-grekas
Copy link
Member

Thank you@Ahummeling.

Ahummeling reacted with heart emoji

@nicolas-grekasnicolas-grekas merged commita6aecb4 intosymfony:4.4Feb 11, 2022
This was referencedFeb 28, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

4 participants

@Ahummeling@dkarlovi@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp