diff --git a/composer.json b/composer.json index 36f0364..41887ef 100644 --- a/composer.json +++ b/composer.json @@ -16,6 +16,7 @@ "symfony/dependency-injection": "^6.4 || ^7.4 || ^8.0", "symfony/form": "^6.4 || ^7.4 || ^8.0", "symfony/http-kernel": "^6.4 || ^7.4 || ^8.0", + "symfony/service-contracts": "^2.5 || ^3.0", "twig/twig": "^3.11" }, "require-dev": { diff --git a/config/services.php b/config/services.php index 50d5e24..c15de6c 100644 --- a/config/services.php +++ b/config/services.php @@ -10,6 +10,7 @@ $services->set(Filter::class) ->arg('$formFactory', service('form.factory')) ->arg('$requestStack', service('request_stack')) + ->tag('kernel.reset', ['method' => 'reset']) ; $services->set(\PUGX\FilterBundle\Twig\Filter::class) diff --git a/src/Filter.php b/src/Filter.php index 858047e..29f9a0f 100644 --- a/src/Filter.php +++ b/src/Filter.php @@ -9,10 +9,15 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\SessionInterface; +use Symfony\Contracts\Service\ResetInterface; -final class Filter +/** + * Forms are kept only for the current request: reset() clears them, so the service + * holds no state across requests in long-running processes (e.g. worker mode). + */ +final class Filter implements ResetInterface { - /** @var array */ + /** @var array */ private array $forms = []; public function __construct( @@ -31,11 +36,10 @@ public function __construct( public function filter(string $name): array { $filter = []; - $fname = $name.$this->getSession()->getId(); /** @var array|null $values */ $values = $this->getSession()->get('filter.'.$name); if (null !== $values) { - if ($this->forms[$fname]->isSubmitted() || $this->forms[$fname]->submit($values)->isValid()) { + if ($this->forms[$name]->isSubmitted() || $this->forms[$name]->submit($values)->isValid()) { $filter = \array_filter($values, static fn ($value): bool => '' !== $value); } } @@ -76,8 +80,7 @@ public function getFormView(string $name, ?string $type = null): FormView */ public function saveFilter(string $type, string $name, array $defaults = [], array $options = []): bool { - $fname = $name.$this->getSession()->getId(); - $this->forms[$fname] = $this->formFactory->create($type, null, $options); + $this->forms[$name] = $this->formFactory->create($type, null, $options); if ($this->getRequest()->query->has('reset-filter')) { $this->getSession()->set('filter.'.$name, null); @@ -91,9 +94,9 @@ public function saveFilter(string $type, string $name, array $defaults = [], arr if (!$this->getRequest()->query->has('submit-filter')) { return false; } - $this->forms[$fname]->handleRequest($this->getRequest()); - if ($this->forms[$fname]->isSubmitted() && $this->forms[$fname]->isValid()) { - $this->getSession()->set('filter.'.$name, $this->getRequest()->query->all()[$this->forms[$fname]->getName()]); + $this->forms[$name]->handleRequest($this->getRequest()); + if ($this->forms[$name]->isSubmitted() && $this->forms[$name]->isValid()) { + $this->getSession()->set('filter.'.$name, $this->getRequest()->query->all()[$this->forms[$name]->getName()]); return true; } @@ -114,11 +117,14 @@ public function sort(string $name, string $field, string $direction = 'ASC'): vo */ private function getForm(string $name, ?string $type = null): FormInterface { - $name .= $this->getSession()->getId(); - return $this->forms[$name] ?? $this->formFactory->create($type ?? FormType::class); } + public function reset(): void + { + $this->forms = []; + } + private function getRequest(): Request { return $this->requestStack->getMainRequest(); diff --git a/tests/FilterTest.php b/tests/FilterTest.php index 3ff6aa5..b51e085 100644 --- a/tests/FilterTest.php +++ b/tests/FilterTest.php @@ -4,6 +4,7 @@ use PHPUnit\Framework\TestCase; use PUGX\FilterBundle\Filter; +use Symfony\Component\Form\Extension\Core\Type\FormType; use Symfony\Component\Form\FormFactoryInterface; use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\FormView; @@ -70,6 +71,48 @@ public function testFormViewWithoutPreviousForm(): void self::assertEquals($view, $formView); } + public function testFormViewWhenSessionStartsDuringRequest(): void + { + // stored filter values, session not started yet: its id is empty until filter() reads it + $storage = new MockArraySessionStorage(); + $storage->setSessionData(['_sf2_attributes' => ['filter.foo' => ['bar' => 'baz']]]); + $session = new Session($storage); + $request = Request::create('/'); + $request->setSession($session); + $stack = new RequestStack(); + $stack->push($request); + + $view = $this->createStub(FormView::class); + $form = $this->createStub(FormInterface::class); + $form->method('createView')->willReturn($view); + $form->method('submit')->willReturnSelf(); + $form->method('isValid')->willReturn(true); + $factory = $this->createMock(FormFactoryInterface::class); + $factory->expects($this->once())->method('create')->with(StubFormType::class)->willReturn($form); + $filter = new Filter($factory, $stack); + + self::assertSame('', $session->getId()); + $filter->saveFilter(StubFormType::class, 'foo'); + $filter->filter('foo'); // starts the session + self::assertSame($view, $filter->getFormView('foo'), 'same form, even if the session id changed'); + } + + public function testReset(): void + { + $types = []; + $form = $this->createStub(FormInterface::class); + $form->method('createView')->willReturn($this->createStub(FormView::class)); + $this->factory->expects($this->exactly(2))->method('create')->willReturnCallback(static function (string $type) use (&$types, $form): FormInterface { + $types[] = $type; + + return $form; + }); + $this->filter->saveFilter(StubFormType::class, 'foo'); + $this->filter->reset(); + $this->filter->getFormView('foo'); + self::assertSame([StubFormType::class, FormType::class], $types, 'saved form is gone after reset'); + } + public function testSort(): void { $this->filter->sort('foo', 'bar');