8000 Cache is not invalidated for routes with container parameters · Issue #21426 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

Cache is not invalidated for routes with container parameters #21426

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.

Sign up for GitHub

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

Jump to bottom
Closed
mhujer opened this issue Jan 26, 2017 · 4 comments
Closed

Cache is not invalidated for routes with container parameters #21426

mhujer opened this issue Jan 26, 2017 · 4 comments
Labels
Bug Routing Status: Needs Review

Comments

@mhujer
Copy link
Contributor
mhujer commented Jan 26, 2017
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 3.2.2

While using a @Route annotation in the controller, the dev cache is properly invalidated, if the route requirements are written directly in the annotation. But when using container parameters in requirements, the cache is not invalidated automatically and you need to delete cache/dev directory manually.

See the example: If another option is added to config, the route is not matched until the cache directory is manually deleted:

# config.yml
parameters:
    routing.type_slug: article|category
// DefaultController.php
/**
 * @Route("/{typeSlug}/", name="homepage", requirements={"typeSlug": "%routing.type_slug%"})
 */
public function indexAction(string $typeSlug, Request $request)
@linaori
Copy link
Contributor
linaori commented Jan 27, 2017

I think this is a limitation, as otherwise every single change in your controller would cause a rebuild of the container.

@mhujer
Copy link
Contributor Author
mhujer commented Jan 27, 2017

Could it be the other way around? Change in the container (config.yml) should cause a rebuild of a routing cache.

Before:

# config.yml
parameters:
    routing.type_slug: article|category

After (added comment, which does not work without removing the cache):

# config.yml
parameters:
    routing.type_slug: article|category|comment

I understand that it is trade-off between fast page load in development environment and cache freshness. If it can't be easily fixed, maybe mentioning it in the docs would be enough (as it isn't widely used feature).

@ogizanagi ogizanagi mentioned this issue Jan 28, 2017
Closed
@ogizanagi
Copy link
Contributor

Adding the container class file as a resource of the main RouteCollection does the job. See #21443

@mhujer
Copy link
Contributor Author
mhujer commented Feb 3, 2017

@ogizanagi I just verified that the patch fixes the issue, thanks! 👍

8505
@ogizanagi ogizanagi mentioned this issue Feb 26, 2017
Merged
fabpot added a commit that referenced this issue Mar 5, 2017
@fabpot
feature #21767 [DI][Router][DX] Invalidate routing cache when contain…
0db9723 
…er parameters changed (ogizanagi)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI][Router][DX] Invalidate routing cache when container parameters changed

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21426
| License       | MIT
| Doc PR        | N/A

Supersedes #21443 but only for master.

Indeed, this implementation uses a new feature: a `ContainerParametersResource` which compares cached containers parameters (collected at some point, here by the `Router`) with current ones in the container.

On the contrary of the previous PR targeting 2.7, this will only invalidate routing cache when parameters actually used in the routes changed and will avoid always rebuilding the routing cache when the container is rebuilt, just to catch the edge case of someone modifying a parameter.

Commits
-------

fad4d9e [DI][Router][DX] Invalidate routing cache when container parameters changed
@fabpot fabpot closed this as completed Mar 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Routing Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @javiereguiluz @mhujer @linaori @ogizanagi @carsonbot
0