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

III-4896 Add PSR router #1102

Merged
merged 40 commits into from
Aug 23, 2022
Merged

III-4896 Add PSR router #1102

merged 40 commits into from
Aug 23, 2022

Conversation

bertramakers
Copy link
Contributor

@bertramakers bertramakers commented Aug 22, 2022

Added

  • PSR router that gets used if a request cannot be matched by the Silex router
  • Support for OPTIONS requests on routes registered on the PSR router (but we still also need a catch-all OPTIONS route in Silex to support CORS on the Silex routes for now). The necessary CORS headers are still added to every response by the middleware in Silex, even for routes matched by the new router. This will be moved in a later ticket once all routes are moved to the new router. (We cannot do it earlier since we also need the CORS headers on the routes in the Silex router)

Changed

  • Registered the GetDetailRequestHandler for events and places on the new router as a proof of concept
  • Move path rewriting to a LegacyPathRewriter class
  • Renamed LegacyRoutesServiceProvider to CatchAllRouteServiceProvider as it is also used to dispatch requests to the new PSR router (until we can by-pass the Silex router completely once all routes, middlewares etc are moved)

Ticket: https://jira.uitdatabank.be/browse/III-4896

All acceptance tests are green, and the CORS requests still work in the UI, also for event details. I also tested the OPTIONS request via curl like this (make sure to change the event URL):

curl -i 'https://io.uitdatabank.dev/event/6c1c3991-0e8f-480d-9c02-16eadb62dae2' -X OPTIONS -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:103.0) Gecko/20100101 Firefox/103.0' -H 'Accept: */*' -H 'Accept-Language: en,en-US;q=0.7,nl;q=0.3' -H 'Accept-Encoding: gzip, deflate, br' -H 'Access-Control-Request-Method: GET' -H 'Access-Control-Request-Headers: authorization,x-api-key' -H 'Referer: https://old.uitdatabank.dev/' -H 'Origin: https://old.uitdatabank.dev' -H 'Connection: keep-alive' -H 'Sec-Fetch-Dest: empty' -H 'Sec-Fetch-Mode: cors' -H 'Sec-Fetch-Site: same-site' -H 'Pragma: no-cache' -H 'Cache-Control: no-cache'

Notes:

It still works for all routes, also those matched by the new router. And if we move it now it won't work anymore for routes matched by the Silex router. So we need to move all the routes to the new router first
Copy link
Contributor

@LucWollants LucWollants left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the test for path rewrites!

@bertramakers bertramakers merged commit 7fda250 into master Aug 23, 2022
@bertramakers bertramakers deleted the III-4896-psr-router branch August 23, 2022 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants