-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection] Add support for excluding services with declared custom attribute #50447
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 7.3
Are you sure you want to change the base?
Conversation
7abe927
to
e46ab36
Compare
aac7e01
to
86d4e3a
Compare
The failures in the checks seem unrelated. |
namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Utils; | ||
|
||
#[\Attribute(\Attribute::TARGET_CLASS)] | ||
class ToBeExcluded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be great to allow this feature to work even if the attribute class doesn't not exist.
Can you remove this one and make the feature still work?
if ($r->getAttributes(Exclude::class)[0] ?? null) { | ||
$this->addContainerExcludedTag($class, $source); | ||
continue; | ||
$exclusionAttributes = $this->container->getExclusionAttributes(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, file loaders are run on separate container instances, right? This would mean that a bundle cannot expect registerAttributeForExclusion to have any effect on classes in src/
. Am I correct? If yes, don't we want to make these app-wide instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about this? did you give it a try? configuring an exclusion within a bundle and seeing it work on the app?
d8675d7
to
86d4e3a
Compare
86d4e3a
to
cdb6eb3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small rebase needed, but more importantly: did you try using registerForAutoconfiguration to define the container.exclude tag?
7.1 | ||
--- | ||
|
||
* Add support for excluding services with declared custom attribute (`ContainerBuilder::registerAttributeForExclusion`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Add support for excluding services with declared custom attribute (`ContainerBuilder::registerAttributeForExclusion`) | |
* Add support for excluding services by attribute with `ContainerBuilder::registerAttributeForExclusion()` |
@@ -131,6 +132,11 @@ class ContainerBuilder extends Container implements TaggedContainerInterface | ||
*/ | ||
private array $autoconfiguredAttributes = []; | ||
|
||
/** | ||
* @var array<class-string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
* @var array<class-string> | |
* @var array<class-string, true> |
if ($r->getAttributes(Exclude::class)[0] ?? null) { | ||
$this->addContainerExcludedTag($class, $source); | ||
continue; | ||
$exclusionAttributes = $this->container->getExclusionAttributes(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about this? did you give it a try? configuring an exclusion within a bundle and seeing it work on the app?
Currently, if we want to exclude classes from registering as services in the container, we can add their namespace(s) in the app configuration, or use the
#[Exclude]
/#[When('never')]
attributes on each of the classes.When it comes to certain types of classes, eg. entities, they can be placed all around the apps in many different folders, which can result in many entries under the
exclude
configuration. It is also usual for us to have many entity classes, so adding the attribute to each of them can be error-prone.However, classes of a same type may already have a common attribute declared on them - such as
Doctrine\ORM\Mapping\Entity
.Because of that, I'm proposing a new way for excluding classes by making it possible to register any attribute for exclusion.
Underneath, this is just extending the functionality for handling the classes with
#[Exclude]
attribute by additionally checking the list of attributes marked for exclusion.Usage:
To exclude all classes with the
#[Entity]
attribute: