Fixed Symfony PurgeListener was pruning instead of purging #475
Conversation
|
Okay, so this PR also fixes an issue with Symfony 3/4/5 compat. One failing test is not related to this PR and the other one is PHP 7.0 compat because the PSR6 store is not compatible with 7.0. |
There was a problem hiding this comment.
just a nitpick on the header handling for symfony 5.
the build now fails because we can't install with php 7.0. i would be ok if you want to drop the 7.0 build here. i think the only other solution would be to make the toflar/store allow php 7.0 and that might be difficult.
please add a note to the changelog about this bug, people might be glad to know about it.
| foreach ($request->headers->get($this->tagsHeader, '', false) as $v) { | ||
| foreach (explode(',', $v) as $tag) { | ||
| // Compatibility with Symfony 3+ | ||
| $allHeaders = $request->headers->all(); |
There was a problem hiding this comment.
i would prefer to have a reflection check if all takes a parameter and if it does, you can $request->headers->all($this->tagsHeader); and otherwise keep the old way with get. and add a note that when we drop support for symfony < 4.4 we can drop the check.
i prefer that over this code that is harder to read and won't get cleaned up.
dbu
force-pushed
the
hotfix/fix-symfony-not-clearing
branch
from
e408f4e to
4fa32cc
Compare
Toflar
force-pushed
the
hotfix/fix-symfony-not-clearing
branch
from
8a9103d to
b7cf2c1
Compare
What a stupid mistake and why did nobody notice this so far? 😄
Basically the clear-all mode of the Symfony integration only pruned (cleaned up expired entries) up until now. But it should actually clear it = deleting all the entries!
This wasn't even possible with my store implementation so I quickly fixed this and released v2.2.0.