rafiki

rafiki copied to clipboard

• Calling the List Incoming Payments endpoint with forward pagination ("{pagination: {first: 2}}") gives you all the incoming payments
• Calling Calling the List Incoming Payments endpoint with backwards pagination ("{pagination: {last: 2}}") gives you all the incoming payments, and does not respect required parameters (cursor is required in this case, the endpoint should throw an error response)

The Open Payments OpenAPI spec defines the pagination query parameters as oneOf either forward or backwards pagination: https://github.com/interledger/open-payments/blob/f1b51c3c051c9030cea1f3d2542cfb492c4330d3/openapi/resource-server.yaml#L1203-L1207

I think that the libraries Rafiki's openapi validator middleware is using to both coerce the query parameter types and set their defaults is unable to handle the oneOf

@AlexLakatos @sabineschaller would either of you mind testing the endpoint with Rafiki using a version of the openapi with pagination as only forward or backwards (rename one of them as pagination)? https://github.com/interledger/rafiki/blob/main/packages/backend/src/config/app.ts#L103

I was a bit wrong when I tested this. I was using body params, didn't notice the spec said they're query params. With query params for first or last, pagination works, but it still doesn't enforce required for cursor on last pagination.

Another possible cause is that the openapi-request-coercer doesn't support "exploding" object parameter properties: https://github.com/kogosoftwarellc/open-api/blob/0ec45af8c35779b6aae611f8a13196b7d2bf75b9/packages/openapi-request-coercer/index.ts#L220-L222

Currently related to what I was working on for listing outgoing and incoming payments, going to double check whether https://github.com/interledger/rafiki/issues/443#issuecomment-1220477823 is still relevant

Things are better, but we're not all the way there yet. I've updated the issue description/comment.

I believe this is the main problem: https://github.com/interledger/rafiki/issues/443#issuecomment-1220445410

https://github.com/interledger/rafiki/blob/b70323a6abc8b712487891dd7ee4f21b212d125e/packages/backend/src/shared/baseModel.ts#L48-L54 I'm seeing pagination's first/last being a string (not coerced to number) or undefined (not set to default when appropriate). last as a string causes use to not error both here and in openapi validation when attempting backwards pagination without a cursor (before).

• https://github.com/interledger/open-payments/issues/136

I had the thought :point_up_2: could help, but I imagine it would also use oneOf...

@wilsonianb what if we had:

direction: 'forwards' | 'backwards'
count: number
cursor: string

I think the issue is having multiple query params defined in an object (not necessarily an object with oneOf) https://github.com/interledger/open-payments/blob/f1b51c3c051c9030cea1f3d2542cfb492c4330d3/openapi/resource-server.yaml#L1336-L1342 https://github.com/interledger/rafiki/blob/8687571fbf7a5f421a98a9d5567b992462ad96bd/packages/open-payments/src/openapi/resource-server.yaml#L1336-L1342

The openapi coercer can't handle "exploding" the fields into distinct parameters https://github.com/interledger/rafiki/issues/443#issuecomment-1231888626

Something like this would work, and then we'd just have the RS (instead of OpenAPI) enforce things like gotta have a cursor with backwards https://github.com/wilsonianb/rafiki/pull/221/commits/6f221e7280bec4c28c1d7cce2e73090ff241e309