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

[DependencyInjection] Apply attribute configurator to child classes#54365

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
fabpot merged 1 commit intosymfony:7.1fromGromNaN:nearest-attribute-configurator
Mar 23, 2024

Conversation

@GromNaN
Copy link
Member

QA
Branch?7.1
Bug fix?no
New feature?yes
Deprecations?no
IssuesFix#52898
LicenseMIT

This allows extending attribute classes registered for autoconfiguration.

Use-case: Share configuration between several classes with pre-configured attribute classes. As described in#52898 (bus name forAsMessageHandler, schedule name forAsCronTask andAsPeriodicTask)

The child-class attribute is handled by the same function as it's parent class that is registered for autoconfiguration. This means any additional property will be ignored (unless supported, which could be new feature for theAsTaggedItem attribute configurator).

If there is a configurator for the child class, the configurator for the parent class is not called.

@GromNaNGromNaNforce-pushed thenearest-attribute-configurator branch from2c896cb to2390a26CompareMarch 21, 2024 11:13
Copy link
Member

@chalasrchalasr left a comment

Choose a reason for hiding this comment

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

Looks sensible to me. I'd suggest checking our current attribute list to make sure there's no blocker in extending each of them (and make those final if any, or hard-skip them)

@GromNaN
Copy link
MemberAuthor

GromNaN commentedMar 21, 2024•
edited
Loading

The list of autoconfigured attributes is inFrameworkExtension

  • AsEventListener
  • AsController
  • AsRemoteEventConsumer
  • AsMessageHandler
  • AsTargetedValueResolver
  • AsSchedule
  • AsPeriodicTask
  • AsCronTask
  • Workflow\Attribute\AsAnnounceListener
  • Workflow\Attribute\AsCompletedListener
  • Workflow\Attribute\AsEnterListener
  • Workflow\Attribute\AsEnteredListener
  • Workflow\Attribute\AsGuardListener
  • Workflow\Attribute\AsLeaveListener
  • Workflow\Attribute\AsTransitionListener

@chalasr
Copy link
Member

The use case forAsMessageHandler makes sense to me (see linked issue) so I'd be fine reverting the change that made it final, likely for another PR.
For the rest of the attributes I'll let other mergers chime in, but to me all of them feel ok-ish to extend.

@GromNaN
Copy link
MemberAuthor

It will be also possible to remove the handling for all theWorkflow attributes, since it duplicatesAsEventListener handler.

$listenerAttributes = [
Workflow\Attribute\AsAnnounceListener::class,
Workflow\Attribute\AsCompletedListener::class,
Workflow\Attribute\AsEnterListener::class,
Workflow\Attribute\AsEnteredListener::class,
Workflow\Attribute\AsGuardListener::class,
Workflow\Attribute\AsLeaveListener::class,
Workflow\Attribute\AsTransitionListener::class,
];
foreach ($listenerAttributesas$attribute) {
$container->registerAttributeForAutoconfiguration($attribute,staticfunction (ChildDefinition$definition,AsEventListener$attribute,\ReflectionClass|\ReflectionMethod$reflector) {
$tagAttributes =get_object_vars($attribute);
if ($reflectorinstanceof \ReflectionMethod) {
if (isset($tagAttributes['method'])) {
thrownewLogicException(sprintf('"%s" attribute cannot declare a method on "%s::%s()".',$attribute::class,$reflector->class,$reflector->name));
}
$tagAttributes['method'] =$reflector->getName();
}
$definition->addTag('kernel.event_listener',$tagAttributes);
});
}

chalasr reacted with hooray emoji

@GromNaN
Copy link
MemberAuthor

GromNaN commentedMar 22, 2024•
edited
Loading

All theinternal attributes can be extended and we already have aninternal use-case with Workflow attributes.

Ready for review.

@fabpotfabpotforce-pushed thenearest-attribute-configurator branch fromb11cb5b to69dc71bCompareMarch 23, 2024 07:14
@fabpot
Copy link
Member

Thank you@GromNaN.

nicolas-grekas added a commit that referenced this pull requestMar 23, 2024
…Handler` (GromNaN)This PR was merged into the 7.1 branch.Discussion----------[Messenger] Allow extending attribute class `AsMessageHandler`| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Issues        | -| License       | MITRevert#52971 which made `AsMessageHandler` final.Discussed in#54365 (comment)Commits-------9479563 Allow extending AsMessageHandler
nicolas-grekas added a commit that referenced this pull requestMar 23, 2024
…tener attributes (GromNaN)This PR was merged into the 7.1 branch.Discussion----------[FrameworkBundle] Remove custom handler for Workflow listener attributes| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        | -| License       | MITAll the `Workflow\Attribute\As*Listener` classes extend `AsEventListener`. Since the attribute handler are now applied to child classes (#54365), it's no longer necessary to declare an attribute handler for each attribute.Discussed in#54365 (comment)Commits-------db7004c Remove custom handler for Workflow listener attributes
@fabpotfabpot mentioned this pull requestMay 2, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@chalasrchalasrchalasr approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

7.1

Development

Successfully merging this pull request may close these issues.

[Messenger] Support extendingAsMessageHandler attribute

5 participants

@GromNaN@chalasr@fabpot@nicolas-grekas@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp