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

feat: dataspace protocol negotiation transformer #2771

Merged

Conversation

juliapampus
Copy link
Contributor

@juliapampus juliapampus commented Apr 19, 2023

What this PR changes/adds

Implements contract negotiation spi and transformers:

  • :data-protocols:dsp:dsp-negotiation:dsp-negotiation-spi
  • :data-protocols:dsp:dsp-negotiation:dsp-negotiation-transform

spec alignments:

  • remove null check for callback in ContractAgreementMessage
  • add checksum to ContractNegotiationEventMessage
  • add code in ContractNegotiationTerminationMessage
  • remove null check for rejectionReason in ContractNegotiationTerminationMessage
  • deprecate contractStart and contractEnd in ContractOffer, see feat: implement policies for contract start/end date #2758
  • add checksum to ContractNegotiation

Why it does that

Support the dataspace protocol specification.

Further notes

Derived issue #2775.

Linked Issue(s)

Relates #2474
Closes #2764

Checklist

  • added appropriate tests?
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • assigned appropriate label? (exclude from changelog with label no-changelog)
  • formatted title correctly? (take a look at the CONTRIBUTING and Etiquette for pull requests for details)

@juliapampus juliapampus added enhancement New feature or request dataspace-protocol related to the dataspace protocol labels Apr 19, 2023
@juliapampus juliapampus temporarily deployed to Azure-dev April 19, 2023 18:55 — with GitHub Actions Inactive
@juliapampus juliapampus changed the title feat: dataspace protocol transformer feat: dataspace protocol negotiation transformer Apr 19, 2023
@codecov-commenter
Copy link

codecov-commenter commented Apr 19, 2023

Codecov Report

Patch coverage: 77.15% and project coverage change: +0.12 🎉

Comparison is base (65479dc) 64.79% compared to head (2112f7e) 64.91%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2771      +/-   ##
==========================================
+ Coverage   64.79%   64.91%   +0.12%     
==========================================
  Files         953      965      +12     
  Lines       19561    19754     +193     
  Branches     1169     1178       +9     
==========================================
+ Hits        12674    12824     +150     
- Misses       6434     6471      +37     
- Partials      453      459       +6     
Impacted Files Coverage Δ
...atalog/transform/DspCatalogTransformExtension.java 0.00% <ø> (ø)
...on.transform/DspNegotiationTransformExtension.java 0.00% <0.00%> (ø)
.../spi/types/agreement/ContractAgreementMessage.java 0.00% <ø> (ø)
...pes/agreement/ContractNegotiationEventMessage.java 0.00% <0.00%> (ø)
...act/spi/types/negotiation/ContractNegotiation.java 0.00% <0.00%> (ø)
...tiation/ContractNegotiationTerminationMessage.java 0.00% <0.00%> (ø)
...nector/contract/spi/types/offer/ContractOffer.java 37.77% <ø> (-2.65%) ⬇️
.../JsonObjectFromContractNegotiationTransformer.java 55.00% <55.00%> (ø)
...romContractNegotiationEventMessageTransformer.java 66.66% <66.66%> (ø)
...tractNegotiationTerminationMessageTransformer.java 76.92% <76.92%> (ø)
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@juliapampus juliapampus force-pushed the feat/dsp-negotiation-transform branch from 68f35e7 to 2e8192d Compare April 19, 2023 19:08
@juliapampus juliapampus temporarily deployed to Azure-dev April 19, 2023 19:08 — with GitHub Actions Inactive
@juliapampus juliapampus force-pushed the feat/dsp-negotiation-transform branch from 2e8192d to b39c129 Compare April 20, 2023 08:17
@juliapampus juliapampus temporarily deployed to Azure-dev April 20, 2023 08:18 — with GitHub Actions Inactive
@juliapampus juliapampus force-pushed the feat/dsp-negotiation-transform branch from b39c129 to e05ee9e Compare April 20, 2023 08:41
@juliapampus juliapampus marked this pull request as ready for review April 20, 2023 08:42
@juliapampus juliapampus temporarily deployed to Azure-dev April 20, 2023 08:43 — with GitHub Actions Inactive
@juliapampus juliapampus temporarily deployed to Azure-dev April 20, 2023 08:43 — with GitHub Actions Inactive
@juliapampus
Copy link
Contributor Author

Open question: How to set providerAgentId and consumerAgentId in ContractAgreement? @jimmarino

@juliapampus juliapampus temporarily deployed to Azure-dev April 20, 2023 09:10 — with GitHub Actions Inactive
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.

pretty much ok, apart from that potential bug in JsonObjectToContractNegotiationEventMessageTransformer, we should remove the side effect from transformString and make it return the string instead of evaluating a function passed.
Maybe first adding a new method, to avoid bloating this PR.

Copy link
Contributor

@jimmarino jimmarino left a comment

Choose a reason for hiding this comment

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

Just some specific changes, otherwise for me OK pending others' comments.

@juliapampus juliapampus force-pushed the feat/dsp-negotiation-transform branch from e39de4a to 1366240 Compare April 25, 2023 08:07
@juliapampus juliapampus temporarily deployed to Azure-dev April 25, 2023 08:07 — with GitHub Actions Inactive
@juliapampus juliapampus force-pushed the feat/dsp-negotiation-transform branch from 1366240 to ca0292d Compare April 25, 2023 08:17
@juliapampus juliapampus temporarily deployed to Azure-dev April 25, 2023 08:18 — with GitHub Actions Inactive
@juliapampus juliapampus temporarily deployed to Azure-dev April 25, 2023 08:22 — with GitHub Actions Inactive
@juliapampus juliapampus temporarily deployed to Azure-dev April 25, 2023 08:22 — with GitHub Actions Inactive
@juliapampus juliapampus force-pushed the feat/dsp-negotiation-transform branch from ed166dc to 9c73f2d Compare April 25, 2023 08:34
@juliapampus juliapampus temporarily deployed to Azure-dev April 25, 2023 08:35 — with GitHub Actions Inactive
@juliapampus juliapampus temporarily deployed to Azure-dev April 25, 2023 08:35 — with GitHub Actions Inactive
@juliapampus juliapampus temporarily deployed to Azure-dev April 25, 2023 08:42 — with GitHub Actions Inactive
@ronjaquensel ronjaquensel temporarily deployed to Azure-dev April 25, 2023 14:03 — with GitHub Actions Inactive
@@ -17,5 +17,4 @@ plugins {
}

dependencies {
Copy link
Member

Choose a reason for hiding this comment

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

the whole dependencies block can be removed

@janpmeyer janpmeyer force-pushed the feat/dsp-negotiation-transform branch from 364b1f1 to c1c79d7 Compare April 26, 2023 12:44
@janpmeyer janpmeyer temporarily deployed to Azure-dev April 26, 2023 12:45 — with GitHub Actions Inactive
@ronjaquensel ronjaquensel merged commit b19024e into eclipse-edc:main Apr 26, 2023
@ronjaquensel ronjaquensel deleted the feat/dsp-negotiation-transform branch April 26, 2023 13:25
majadlymhmd pushed a commit to FraunhoferISST/edc-connector that referenced this pull request May 10, 2023
---------

Co-authored-by: Jan Peter Meyer <[email protected]>
Co-authored-by: Ronja Quensel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dataspace-protocol related to the dataspace protocol enhancement New feature or request
Projects
None yet
6 participants