Skip to content
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

[FrameworkBundle] Annotation cache does not override default settings #26086

Closed
Laizerox opened this issue Feb 8, 2018 · 1 comment
Closed

Comments

@Laizerox
Copy link
Contributor

Laizerox commented Feb 8, 2018

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 3.4.4

As the title describes when defined in config.yml

framework:
    annotations:
        cache: doctrine_cache.providers.annotations.cache

The annotation_reader does not get injected with proper cache but remains as ArrayCache.

The problem appears to happen when AddAnnotationsCachedReaderPass is being processed definition of annotations.cached_reader is already removed and $container->hasDefinition('annotations.cached_reader') returns false.

Default settings such as file for framework.annotations.cache does not appear to work either.

When default cache is being overridden in annotations.xml of FrameworkBundle manually it appears the cache starts to work.

When using bundles such as FOSRestBundle the lack of cache causes annotation_reader to parse annotations on each request.

Either I'm doing something wrong or this has broken at some stage.

@Laizerox Laizerox changed the title Framework annotation cache does not override default settings [FrameworkBundle] Annotation cache does not override default settings Feb 8, 2018
@Laizerox
Copy link
Contributor Author

Laizerox commented Feb 8, 2018

I've concluded more testing and took a look at 7085569 commit.

It appears that the pass config type was changed from PassConfig::TYPE_BEFORE_REMOVING to PassConfig::TYPE_AFTER_REMOVING.

Now if I revert that change the cache provider will be injected accordingly.

I also noticed that there was a test added in that particular commit. However I believe its looking for a wrong definition and hence doesn't fail. Maybe it should be looking for test.annotations.cached_reader instead of test.annotation_reader @nicolas-grekas ?

nicolas-grekas added a commit that referenced this issue 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
nicolas-grekas added a commit that referenced this issue Feb 19, 2018
* 3.4:
  [Bridge/Twig] fix composer.json
  bug #26086 [FrameworkBundle] Fix using annotation_reader in compiler pass to inject configured cache provider
  [WebProfilerBundle] Fix anchor CSS
  [HttpKernel] Send new session cookie from AbstractTestSessionListener after session invalidation
  [WebProfilerBundle] Tweak default route name
  updated StopwatchEvent phpdoc due to the additional of optional float precision introduced in 0db8d7f
  Retro-fit proxy code to make it deterministic for older proxy manager implementations
  Yaml parser regression with comments and non-strings
  Fix undiscoverablility of SymfonyTestsListenerForV7
  Fixed broken tests
  [TwigBridge] Apply some changes to support Bootstrap4-stable
nicolas-grekas added a commit that referenced this issue Feb 19, 2018
* 4.0:
  [Bridge/Twig] fix composer.json
  bug #26086 [FrameworkBundle] Fix using annotation_reader in compiler pass to inject configured cache provider
  [WebProfilerBundle] Fix anchor CSS
  [HttpKernel] Send new session cookie from AbstractTestSessionListener after session invalidation
  [WebProfilerBundle] Tweak default route name
  updated StopwatchEvent phpdoc due to the additional of optional float precision introduced in 0db8d7f
  Retro-fit proxy code to make it deterministic for older proxy manager implementations
  [Serializer] remove unneeded php doc line
  Yaml parser regression with comments and non-strings
  Fixed broken tests
  [TwigBridge] Apply some changes to support Bootstrap4-stable
ostrolucky pushed a commit to ostrolucky/symfony that referenced this issue Mar 25, 2018
* 3.4:
  [Bridge/Twig] fix composer.json
  bug symfony#26086 [FrameworkBundle] Fix using annotation_reader in compiler pass to inject configured cache provider
  [WebProfilerBundle] Fix anchor CSS
  [HttpKernel] Send new session cookie from AbstractTestSessionListener after session invalidation
  [WebProfilerBundle] Tweak default route name
  updated StopwatchEvent phpdoc due to the additional of optional float precision introduced in 0db8d7f
  Retro-fit proxy code to make it deterministic for older proxy manager implementations
  Yaml parser regression with comments and non-strings
  Fix undiscoverablility of SymfonyTestsListenerForV7
  Fixed broken tests
  [TwigBridge] Apply some changes to support Bootstrap4-stable
ostrolucky pushed a commit to ostrolucky/symfony that referenced this issue Mar 25, 2018
* 4.0:
  [Bridge/Twig] fix composer.json
  bug symfony#26086 [FrameworkBundle] Fix using annotation_reader in compiler pass to inject configured cache provider
  [WebProfilerBundle] Fix anchor CSS
  [HttpKernel] Send new session cookie from AbstractTestSessionListener after session invalidation
  [WebProfilerBundle] Tweak default route name
  updated StopwatchEvent phpdoc due to the additional of optional float precision introduced in 0db8d7f
  Retro-fit proxy code to make it deterministic for older proxy manager implementations
  [Serializer] remove unneeded php doc line
  Yaml parser regression with comments and non-strings
  Fixed broken tests
  [TwigBridge] Apply some changes to support Bootstrap4-stable
sandergo90 pushed a commit to sandergo90/symfony that referenced this issue Jul 4, 2019
* 4.0:
  [Bridge/Twig] fix composer.json
  bug symfony#26086 [FrameworkBundle] Fix using annotation_reader in compiler pass to inject configured cache provider
  [WebProfilerBundle] Fix anchor CSS
  [HttpKernel] Send new session cookie from AbstractTestSessionListener after session invalidation
  [WebProfilerBundle] Tweak default route name
  updated StopwatchEvent phpdoc due to the additional of optional float precision introduced in 0db8d7f
  Retro-fit proxy code to make it deterministic for older proxy manager implementations
  [Serializer] remove unneeded php doc line
  Yaml parser regression with comments and non-strings
  Fixed broken tests
  [TwigBridge] Apply some changes to support Bootstrap4-stable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants