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

Runtime action argument type information should override static type information #1905

Closed
albe opened this issue Jan 31, 2020 · 1 comment · Fixed by #1906
Closed

Runtime action argument type information should override static type information #1905

albe opened this issue Jan 31, 2020 · 1 comment · Fixed by #1906

Comments

@albe
Copy link
Member

albe commented Jan 31, 2020

Currently, the action method arguments are initialized early inside the controller, before initializing the property mapping and calling initialize*Action(). That way the argument only has type information from static reflection. The validation chain is then built based on that argument type information ($argument->getDataType()) and in case of an action receiving an interface with dynamic type overrides (__type or inside initialize*Action()), the validation chain is not correctly built, missing validation information from the actual class property annotations.

The issue in solving this however, is that we need to check runtime type information only after initialize*Action() / propertyMappingConfiguration (the latter needs to explicitly allow overriding the type). That in turn expects the action arguments to be already available.

So there are three possible ways to solve this (assuming allowing dynamic typing of action arguments is valid):

  • action argument type information and validators are not (fully) available inside initialize*Action(), moving initializeActionMethodValidators() after that
  • move __type override from the ObjectConverter lower down into the Framework stack, so that the argument type is overriden earlier (before property mapping)
  • completely rebuilding the valdation chain building to be lazy and only work from actual data types given into the validator

The latter is at least an order of magnitude more complex, but generally desirable - the first is potentially a bit breaky.

@albe
Copy link
Member Author

albe commented Dec 14, 2020

This change has been adjusted to be hidden behind a feature flag, so it can end up in the next minor version.

@albe albe added this to To do in Neos & Flow 7.1 Release Board via automation Mar 6, 2021
@albe albe moved this from To do to Review in progress in Neos & Flow 7.1 Release Board Mar 6, 2021
@albe albe removed this from Review in progress in Neos & Flow 7.1 Release Board May 5, 2021
@albe albe added this to To do in Neos & Flow 7.2 Release Board via automation Jul 17, 2021
@albe albe moved this from To do to In progress in Neos & Flow 7.2 Release Board Jul 17, 2021
@Sebobo Sebobo moved this from In progress to In Review in Neos & Flow 7.2 Release Board Jul 27, 2021
@kdambekalns kdambekalns removed this from Needs Review in Neos & Flow 7.2 Release Board Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants