[FrameworkBundle] Fix using annotation_reader in compiler pass to inject configured cache provider #26088
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can you add tests for this? |
nicolas-grekas
approved these changes
Feb 19, 2018
…pass to inject configured cache provider
|
Thank you @Laizerox. |
nicolas-grekas
added a commit
that referenced
this pull request
Feb 19, 2018
…pass to inject configured cache provider (Laizerox) This PR was merged into the 3.4 branch. Discussion ---------- [FrameworkBundle] Fix using annotation_reader in compiler pass to inject configured cache provider | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #26086 | License | MIT | Doc PR | - The compilation pass of AddAnnotationsCachedReaderPass relies on already removed definition `annotations.cached_reader` due to an alias to `annotation_reader`. Since the definition is being removed because of alias, configured annotation cache provider is not injected and will default back to ArrayCache. This PR replaces the use of `annotations.cached_reader` to `annotation_reader` to complete the injection of configured cache provider. Commits ------- dfd93da bug #26086 [FrameworkBundle] Fix using annotation_reader in compiler pass to inject configured cache provider
|
Version 3.3 is affected too, could you please backport? |
|
@Strate 3.3 is EOLed, see supported versions on https://symfony.com/roadmap |
|
For all, who want to get it fixed in 3.3, you can add this compiler pass to your container build: if ($container->hasDefinition('annotation_reader')) {
$reader = $container->getDefinition('annotation_reader');
if (is_a($reader->getClass(), CachedReader::class, true)) {
$properties = $reader->getProperties();
if (isset($properties['cacheProviderBackup'])) {
$provider = $properties['cacheProviderBackup']->getValues()[0];
unset($properties['cacheProviderBackup']);
$reader->setProperties($properties);
$container->set('annotation_reader', null);
$container->setDefinition('annotation_reader', $reader->replaceArgument(1, $provider));
}
}
} |
|
Unfortunatelly, this is still actual for all versions for case, when It happens, because DecoratorServicePass removes any tag from decorated service and run before AddAnnotationsCachedReaderPass, so late pass can't find any service. I think we can just iterate all services to search UPD. Also AddAnnotationsCachedReaderPass run after optimizations, so decorated service no more available in container by id, so I forced to write custom compiler pass special for that case: public function process(ContainerBuilder $container)
{
$def = $container->findDefinition('annotation_reader');
if (is_a($def->getClass(), IgnoreClosureAnnotationsReader::class, true)) {
$inner = $def->getArgument(0);
if ($inner instanceof Definition) {
$properties = $inner->getProperties();
if (isset($properties['cacheProviderBackup'])) {
if (is_a($inner->getClass(), CachedReader::class, true)) {
$provider = $properties['cacheProviderBackup']->getValues()[0];
unset($properties['cacheProviderBackup']);
$inner->setProperties($properties);
$inner->replaceArgument(1, $provider);
}
}
}
}
}Unfortunatelly, after hitting this issue twice, I can consider that something globally get wrong this wiring process of annotations cache :( |
nicolas-grekas
added a commit
that referenced
this pull request
May 23, 2022
…(nicolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- [FrameworkBundle] fix wiring of annotations.cached_reader | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #46202 | License | MIT | Doc PR | - Also fixes #26088 (comment) Instead of trying to hack the compilation of the container to prevent it from inlining `annotations.cached_reader`, this PR relies on an `container.do_not_inline` tag that just does that, prevent inlining so that removing passes can reliably fetch the corresponding services from the container by id or by tag. Commits ------- e32b7b1 [FrameworkBundle] fix wiring of annotations.cached_reader
symfony-splitter
pushed a commit
to symfony/framework-bundle
that referenced
this pull request
May 23, 2022
…(nicolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- [FrameworkBundle] fix wiring of annotations.cached_reader | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #46202 | License | MIT | Doc PR | - Also fixes symfony/symfony#26088 (comment) Instead of trying to hack the compilation of the container to prevent it from inlining `annotations.cached_reader`, this PR relies on an `container.do_not_inline` tag that just does that, prevent inlining so that removing passes can reliably fetch the corresponding services from the container by id or by tag. Commits ------- e32b7b15ec [FrameworkBundle] fix wiring of annotations.cached_reader
symfony-splitter
pushed a commit
to symfony/framework-bundle
that referenced
this pull request
Jul 28, 2023
…(nicolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- [FrameworkBundle] fix wiring of annotations.cached_reader | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #46202 | License | MIT | Doc PR | - Also fixes symfony/symfony#26088 (comment) Instead of trying to hack the compilation of the container to prevent it from inlining `annotations.cached_reader`, this PR relies on an `container.do_not_inline` tag that just does that, prevent inlining so that removing passes can reliably fetch the corresponding services from the container by id or by tag. Commits ------- e32b7b15ec [FrameworkBundle] fix wiring of annotations.cached_reader
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The compilation pass of AddAnnotationsCachedReaderPass relies on already removed definition
annotations.cached_readerdue to an alias toannotation_reader.Since the definition is being removed because of alias, configured annotation cache provider is not injected and will default back to ArrayCache.
This PR replaces the use of
annotations.cached_readertoannotation_readerto complete the injection of configured cache provider.