RFC: Move REST route recognition after routing resolves

[Steveb-p]

🎫 Issue	N/A

Description:

This PR adds a secondary check for REST API route after routing has been resolved.

This allows routes like /admin/api/ibexa/v2 to work.

Note

I removed the original method completely (onKernelRequest) and REST API remained functional.

For QA:

Documentation:

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[alongosz]

It could be a bit confusing. We have X-Siteaccess header. While I get it's not working as expected, is there a possibility to fix that?
Second thing, so now we execute the same check twice in onKernelRequest and addAttributeForIbexaRestRoute. If we go with that solution can we keep just one method?

[Steveb-p]

It could be a bit confusing. We have X-Siteaccess header. While I get it's not working as expected, is there a possibility to fix that?

It is working as expected.

All this does is expose REST routes under site-access aware prefixes, and results in both methods of switching to a different site-access to work.

(X-Siteaccess header takes priority afaik, but haven't checked)

Second thing, so now we execute the same check twice in onKernelRequest and addAttributeForIbexaRestRoute. If we go with that solution can we keep just one method?

Currently yes.

There is an argument to be made to keep the old method still executing since it is "almost" a contract. There might be packages or developers that rely on a particular order of event listeners.

[konradoboza]

Could you please elaborate a bit on the origins of this change? I don't fully get the necessity of it.

[Steveb-p]

Could you please elaborate a bit on the origins of this change? I don't fully get the necessity of it.

Sure.

Our frontend is using - or rather, would like to use - Routing.generate(<route>) on the frontend side (javascript), coming from FOS Routing bundle.

However, route generated there uses /admin prefix when generated from the admin UI. This makes that routing generation unusable, as our REST API does not respect site access prefixes. They need to hardcode REST routes.
The router does match the REST API route, but our view object dispatchers (which are a Symfony concept) do not work, because REST route recognition (putting is_rest_request attribute) is done before the information about semanticPathinfo is put into the Request.

So my suggestion is: move REST route recognition later down in the event chain. That way we will have access to the "actual" route, with site access prefix stripped. I've done basic checks and REST works properly still when this is moved into lower priority - but I've left the original recognition too, because there might be code that relies on it being checked early.

[alongosz]

Ok. But in 5.0 let's get rid of the extra event.