-
Notifications
You must be signed in to change notification settings - Fork 51
feat: add get forwarding functionality #620
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds GET request forwarding functionality to support proxied participants in the ml-api-adapter service. The implementation enables the system to forward GET requests (queries) to proxied participants when there's no response payload, while maintaining backward compatibility for PUT responses when payload data is present.
Changes:
- Added GET request forwarding logic in the notification handler for proxy scenarios
- Modified fulfil transfer validation to skip when 'fspiop-proxy' header is present
- Added comprehensive test coverage for GET and FX-GET forwarding scenarios
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/handlers/notification/index.js | Added GET request forwarding logic with empty payload detection and proxy endpoint resolution |
| src/api/transfers/handler.js | Added conditional validation skip for proxy requests based on header presence |
| test/unit/handlers/notification/index.test.js | Added three new test cases covering GET forwarding success scenarios and error handling |
| package.json | Version bump to 16.7.0-snapshot.3 |
| package-lock.json | Version updates and dependency changes including dev flag removals |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|


This pull request introduces enhancements to the ML API Adapter, focusing on improved handling of GET notification messages (including FX GET) and validation logic for transfer fulfilments. It also adds comprehensive unit tests for the new GET forwarding logic and increments the package version.
Notification GET request forwarding:
src/handlers/notification/index.jsto detect and correctly forward GET and FX_GET notification messages with empty payloads as HTTP GET requests to the appropriate FSP endpoint, ensuring proper header construction, endpoint resolution, and error handling.Transfer fulfilment validation:
src/api/transfers/handler.jsto skip payload validation for transfer fulfilment requests originating from external participants (determined by absence of thefspiop-proxyheader), allowing for more flexible integrations.Testing improvements:
test/unit/handlers/notification/index.test.jsto verify correct forwarding of GET and FX_GET notification messages, successful and failed forwarding scenarios, and appropriate audit tagging.Versioning:
16.7.0-snapshot.3inpackage.json.