Improve routing API

[fpseverino]

• Make PassesService and OrdersService conform to RouteCollection
  • Services can now be registered as controllers on any RoutesBuilder
• Use the request's Logger inside routes (instead of one provided by the user)
• Remove push routes and pushRoutesMiddleware
  • Those were somewhat useful in early versions of the package when methods like sendPushNotifications were not public

[ptoffy]

Where'd the tests go?

[fpseverino]

Where'd the tests go?

I removed only the tests related to the two "push routes", which I would like to remove

The first one when called sent a push notification to all devices registered to a specific pass, the other one returned as a response the push tokens of all devices registered to a pass

Their usefulness is questionable in my opinion, and their initialization required (rightly) a middleware, that with this refactoring of the routes into a RouteCollection should have been saved inside PassesService/OrdersService

P.S. Of course we already provide the methods that these routes used internally and they are easily reimplementable by the user if they need them

[ptoffy]

I'm not against removing useless things but we should be testing what's public so if you remove tests I'd remove the tested routes too. When reviewing it seemed like you only removed the tests and not the routes

[0xTim]

Definitely need better variable names. I'm assuming all the new push stuff is tested?

One thing I want to check now that we're relying on route registration is that it works with nested stuff unlike the Imperial issues?

[fpseverino]

I'm not against removing useless things but we should be testing what's public so if you remove tests I'd remove the tested routes too. When reviewing it seemed like you only removed the tests and not the routes

Yes, I also removed push routes, you can see that service initializers no longer accept the pushRoutesMiddleware argument.
I also moved all routes to a new file, so maybe it's hard to notice that the push ones are no longer there

[codecov]

Codecov Report

Attention: Patch coverage is 93.98907% with 22 lines in your changes missing coverage. Please review.

Project coverage is 88.90%. Comparing base (900653c) to head (ff82700).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...etPasses/PassesServiceCustom+RouteCollection.swift	95.55%	8 Missing ⚠️
...etOrders/OrdersServiceCustom+RouteCollection.swift	95.89%	6 Missing ⚠️
...iddleware/OrdersService+AsyncModelMiddleware.swift	42.85%	4 Missing ⚠️
...iddleware/PassesService+AsyncModelMiddleware.swift	42.85%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #20      +/-   ##
==========================================
- Coverage   92.47%   88.90%   -3.58%     
==========================================
  Files          10       11       +1     
  Lines         718      613     -105     
==========================================
- Hits          664      545     -119     
- Misses         54       68      +14     

Files with missing lines	Coverage Δ
...WalletOrders/Middleware/AppleOrderMiddleware.swift	100.00% <100.00%> (ø)
Sources/VaporWalletOrders/OrdersService.swift	100.00% <100.00%> (ø)
...ources/VaporWalletOrders/OrdersServiceCustom.swift	82.43% <100.00%> (-13.09%)	⬇️
...rWalletPasses/Middleware/ApplePassMiddleware.swift	100.00% <100.00%> (ø)
Sources/VaporWalletPasses/PassesService.swift	100.00% <100.00%> (ø)
...ources/VaporWalletPasses/PassesServiceCustom.swift	84.88% <100.00%> (-10.70%)	⬇️
...iddleware/OrdersService+AsyncModelMiddleware.swift	50.00% <42.85%> (ø)
...iddleware/PassesService+AsyncModelMiddleware.swift	50.00% <42.85%> (ø)
...etOrders/OrdersServiceCustom+RouteCollection.swift	95.89% <95.89%> (ø)
...etPasses/PassesServiceCustom+RouteCollection.swift	95.55% <95.55%> (ø)

[fpseverino]

I'm assuming all the new push stuff is tested?

The push notifications public methods haven't changed (they were already tested), I just removed the so called "push routes" which were a duplicate way to send notifications (not required by Apple Wallet API specification)

[fpseverino]

One thing I want to check now that we're relying on route registration is that it works with nested stuff unlike the Imperial issues?

The difference between this package and Imperial is that Imperial automatically returns the URL to the API based on the registered route, whereas here the route registration and the URL encoding are two completely separate things.

So it works perfectly in nested routes, but you have to manually build the URL when encoding the pass.

For example, if you register the routes like this:

try app.grouped("api", "passes").register(collection: passesService)

you have to set the webServiceURL of the pass like this:

let webServiceURL = "\(Environment.get("WEBSITE_URL")!)/api/passes/"

If we find a solution to this common problem we could think of automatically injecting the path