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

[Autocomplete] Reset form state on each request for applications that reuse the Symfony application between requests #2094

Merged
merged 1 commit into from
Aug 31, 2024

Conversation

dotdevio
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
Issues
License MIT

Hi there, in ux-autocomplete there is an WrappedEntityTypeAutocompleter::getForm which on first call saves form instance in WrappedEntityTypeAutocompleter $form property, it has method setOptions which is called on each request in EntityAutocompleteController, in PHP-FPM everything will work as expected where container is created on each request, but with Swoole or similar where container is not created on each request rather only first request on first EntityAutocompleteController request everything is also fine, but on second request WrappedEntityTypeAutocompleter will have an instance of form from previous request which is fine but the call of setOptions in controller will throw exception.

So in Swoole on second and next requests $this->form already contains an instance of form from previous request because DI Container is not created on each request as in PHP-FPM (Container is created only once) and exception is thrown. So, Symfony have an kernel.reset / kernel.terminate event which probably can be used here to reset $this->form in WrappedEntityTypeAutocompleter because they are fired after request is completed.

@carsonbot carsonbot added Autocomplete Bug Bug Fix Status: Needs Review Needs to be reviewed labels Aug 23, 2024
@smnandre
Copy link
Member

Thank you @dotdevio for this PR!

Could you maybe add a test?

Also.. I'd see this more as a feature than a bug, as swoole (or similar runners) compatibility was not something supported before.. what do you think ?

@dotdevio
Copy link
Contributor Author

Thank you @dotdevio for this PR!

Could you maybe add a test?

Also.. I'd see this more as a feature than a bug, as swoole (or similar runners) compatibility was not something supported before.. what do you think ?

This is not a feature because Symfony supports multiple runtimes and this bug will be not only in Swoole but any other runtime like FrankenPHP, RoadRunner etc. Service state shoud reset in this case for sure, no BC breaks on PHP-FPM. I would like add a test but i dont see how i can test here and i dont found any tests for WrappedEntityTypeAutocompleter. Actually there is noothing to break the app, just unsetting variable on kernel.reset event.

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Aug 23, 2024
@smnandre
Copy link
Member

Ok! Thank you for the fix @dotdevio :)

@1ed
Copy link
Contributor

1ed commented Aug 26, 2024

This fixes #1649

… reuse the Symfony application between requests
@kbond
Copy link
Member

kbond commented Aug 31, 2024

Perfect, thanks @dotdevio!

@kbond kbond merged commit 83340c3 into symfony:2.x Aug 31, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Autocomplete Bug Bug Fix Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants