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(dsp): implement GET /id negotiation endpoint #3212

Conversation

ronjaquensel
Copy link
Contributor

What this PR changes/adds

Adds a method findById to the ContractNegotiationProtocolService, that takes a ClaimToken as parameter and returns a negotiation only if the requesting party is authorized. Using this method, implements the GET /id endpoint in the DspNegotiationApiController.

Why it does that

To support the protocol specification

Linked Issue(s)

Closes #2795

@ronjaquensel ronjaquensel added enhancement New feature or request dataspace-protocol related to the dataspace protocol labels Jun 22, 2023
@ronjaquensel ronjaquensel added this to the Milestone 10 milestone Jun 22, 2023
private ServiceResult<ContractNegotiation> validateGetRequest(ClaimToken claimToken, String id, ContractNegotiation negotiation) {
var result = validationService.validateRequest(claimToken, negotiation);
if (result.failed()) {
// should return not found if counter-party not authorized
Copy link
Member

Choose a reason for hiding this comment

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

I know that this is stated in the specs, but shouldn't be more http-like to return 403?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the idea behind returning the 404 is to not give out information about the existence of processes to unauthorized parties.

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 88.46% and no project coverage change.

Comparison is base (1b6162a) 65.60% compared to head (9217597) 65.60%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3212   +/-   ##
=======================================
  Coverage   65.60%   65.60%           
=======================================
  Files         844      844           
  Lines       16902    16914   +12     
  Branches      931      933    +2     
=======================================
+ Hits        11089    11097    +8     
- Misses       5441     5444    +3     
- Partials      372      373    +1     
Impacted Files Coverage Δ
.../dsp/api/configuration/error/DspErrorResponse.java 0.00% <0.00%> (ø)
...on/api/controller/DspNegotiationApiController.java 94.28% <88.23%> (-1.80%) ⬇️
...iation/ContractNegotiationProtocolServiceImpl.java 96.80% <100.00%> (+0.29%) ⬆️

... and 1 file with indirect coverage changes

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

@ronjaquensel ronjaquensel merged commit 36aa899 into eclipse-edc:main Jun 23, 2023
@ronjaquensel ronjaquensel deleted the feat/2795-protcol-negotiation-findById branch June 23, 2023 09:56
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
Development

Successfully merging this pull request may close these issues.

dataspace protocol: add method findById to negotiation protocol service
4 participants