neos / flow-development-collection

The unified repository containing the Flow core packages, used for Flow development.
https://flow.neos.io/
MIT License
137 stars 189 forks source link

BUG: Attribute driven routes are ignored in production context #3400

Open bwaidelich opened 2 days ago

bwaidelich commented 2 days ago

Is there an existing issue for this?

Current Behavior

Routes from attributes are not detected

Expected Behavior

Routes from attributes should be respected and listable via ./flow routing:list

Steps To Reproduce

Add Route attributes to some MVC action and configure the AttributeRoutesProviderFactory as described

Environment

- Flow: 9.0@dev

Anything else?

I could pin point the issue down to this line https://github.com/neos/flow-development-collection/blob/ee8a8127e1d77908b5f44786a1ba4d6f2500c583/Neos.Flow/Classes/Mvc/Routing/AttributeRoutesProvider.php#L81 that returns an empty array in production context, probably because it is invoked before the ReflectionService is fully initialized.

Related: #2059

kitsunet commented 2 days ago

This shoudn't be affected by the ConfigurationManager availability I think?

This is super obscure but I think the problem is that we just do not load method annotation data in production. This is sensible for performance, and we can fix this issue here by using compile static, which we should do anyways for better production performance, BUT fact is, we should have the same behavior in Development and instead only decide between runtime and compiletime behavior.

Atm. \Neos\Flow\Reflection\ReflectionService::initialize has a special handling for production with frozen caches, and I skimmed over this for a bit because I thought we don't freeze anymore, turns out, the reflection service automatically freezes it's own caches for production, so this triggers basically always for production. And in this code branch we just do not load the necessary reflection data (together with below we should probably except method calls that try to access this while not in compiletime or better yet, split the reflectionservice between one for compiletime and one for runtime like the object manager does).

I know from experience that this trickery is necessary as the ram usage for this is huge. But as suggested to make this not only fail in production we should change \Neos\Flow\Reflection\ReflectionService::hasFrozenCacheInProduction to not check for production but for compiletime, trick is that the bootstrap does not expose this state atm, and we need to provide it to the reflectionservice first to test against. Should all be non breaky but a bit complicated. I will have a look. Short term I would really just change this to work with compile static to fix the bug.

Oh and while I can have a look at a PR tomorrow, for sake of clarity as this is a bit of a hidden feature I guess, this is a good example of compile static usage: \Neos\Flow\Cli\CommandManager::getCommandControllerMethodArguments this method will be executed in compiletime and then overwritten in the proxy with one that hardcoded returns whatever was returned during compiletime, cutting off all dependencies used inside the method call and thus never having to use the unavailable reflection data in production.

kitsunet commented 1 day ago

The PR should fix the immediate bug, I will open additional PRs with some ideas about the wider topic of runtime (production) reflection data etc.

kitsunet commented 1 day ago

3402 for the underlaying issue