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

refactor: supports multi protocols dsp-http #4514

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

wolf4ood
Copy link
Contributor

@wolf4ood wolf4ood commented Sep 30, 2024

What this PR changes/adds

Supports multi protocols in DspRequest and DspHttpRemoteMessageDispatcherImpl

  • Introduced a new ProtocolVersion for v0.8 default one, bind in the default controllers without a version in the path.
  • Introduced DspProtocolTypeTransformerRegistry spi for fetching the right TypeTransformerRegistry given a protocol.
  • Introduced DspProtocolParser for parsing the protocol format dataspace-protocol-http:<version> into a ProtocolVersion
  • Introduced a ProtocolRemoteMessage for holding the protocol and changing it in ingress on DspRequestHandlerImpl
  • PostDspHttpRequestFactory and GetDspHttpRequestFactory uses the DspProtocolParser for extracting the right protocol version from a RemoteMessage#protocol

Why it does that

supporting multiple dsp protocols

Further notes

Currently both v0.8 and 2024/1 have the same transformers registered. In subsequent PRs the 2024/1 should be fixed to point to the right namespace.

The JsonLd context refactor has no been applied in this PR. Currently they still use the same JsonLD configuration under the DSP scope.

Linked Issue(s)

Closes #4507

Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.

@wolf4ood wolf4ood added the refactoring Cleaning up code and dependencies label Sep 30, 2024
@wolf4ood wolf4ood self-assigned this Sep 30, 2024
@wolf4ood wolf4ood force-pushed the refactor/4507_dsp_http_modules branch 15 times, most recently from 140db27 to 89cb2a8 Compare October 1, 2024 10:12
@wolf4ood wolf4ood marked this pull request as ready for review October 1, 2024 10:56
Copy link
Member

@ndr-brt ndr-brt left a comment

Choose a reason for hiding this comment

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

great stuff, just a comment, and agree about the e2e test refactoring pointed by the TODO, I also found out that the current design has limits

@wolf4ood wolf4ood force-pushed the refactor/4507_dsp_http_modules branch from 89cb2a8 to cf18ec7 Compare October 1, 2024 14:25
@wolf4ood wolf4ood force-pushed the refactor/4507_dsp_http_modules branch from cf18ec7 to fcab47f Compare October 1, 2024 14:31
@wolf4ood wolf4ood requested a review from ndr-brt October 2, 2024 08:15
Copy link
Member

@ndr-brt ndr-brt left a comment

Choose a reason for hiding this comment

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

🚀

@wolf4ood wolf4ood merged commit f449c33 into eclipse-edc:main Oct 2, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Cleaning up code and dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor http dsp modules
2 participants