-
Notifications
You must be signed in to change notification settings - Fork 252
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: clean up ids-api-multipart-endpoint-v1
#1715
Merged
juliapampus
merged 45 commits into
eclipse-edc:main
from
FraunhoferISST:chore/1610-clean-up-multipart-endpoint
Jul 22, 2022
+1,384
−3,014
Merged
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
0ef385c
chore: remove IdsResponseMessageFactory
ronjaquensel d815e25
chore: remove unused exceptions
ronjaquensel 1ab2fc5
chore: merge response and rejection util classes
ronjaquensel 034dc73
chore: remove class IdsClientCredentialsScope
ronjaquensel 42b1156
chore: do not use description sub-handlers
ronjaquensel 6cfd5d7
chore: remove sub-handler classes
ronjaquensel 5f32658
chore: remove DescriptionRequestHandler interface
ronjaquensel be3c483
chore: remove DescriptionResponseMessageUtil
ronjaquensel d90f447
chore: move util classes to util package
ronjaquensel 7582399
chore: add token to all response headers
ronjaquensel fc11082
chore: add Javadoc to MultipartController
ronjaquensel 1896f7b
chore: move method for building description response to util
ronjaquensel 84f70e5
chore: return RequestInProcessMessage for artifact requests
ronjaquensel 20df603
chore: make response depend on status result from state machine
ronjaquensel eb07868
chore: do not build message in ArtifactRequestHandler
ronjaquensel 7a05a51
chore: rename DescriptionRequestHandler
ronjaquensel 761f359
chore: align parameter order in ResponseMessageUtil
ronjaquensel 4786e6b
docs: add Javadoc for ResponseMessageUtil
ronjaquensel 5d6651c
chore: remove unused method
ronjaquensel 1344b2d
docs: add Javadoc for MultipartResponseUtil
ronjaquensel 4db8bf7
refactor: remove duplicated methods from handlers
ronjaquensel 83f5e59
refactor: remove MultipartResponseUtil
ronjaquensel 0dad8e2
refactor: process status result in contract handlers
ronjaquensel 62b30c8
refactor: remove notification sub-handlers
ronjaquensel 39fb36c
chore: remove check for connector payload in MultipartController
ronjaquensel 755e5d7
refactor: remove superfluous claim token parameter
ronjaquensel a61cdf9
refactor: remove internal handle method from DescriptionRequestHandler
ronjaquensel dea5fcf
docs: add comments and Javadoc
ronjaquensel 199ff97
refactor: make handleRequest in Handler @NotNull
ronjaquensel 0a0b08f
chore: change rejection reason in EDR handler
ronjaquensel 39a9d1d
chore: add required fields to test messages
ronjaquensel 24b2045
test: update handler tests
ronjaquensel 96697d2
test: replace tests for DescriptionRequestHandler
ronjaquensel 2dbcbea
refactor: rename RequestUtil
ronjaquensel fd28727
chore: only store secret if TP initialized successfully
ronjaquensel 118d3f0
docs: update CHANGELOG.md
ronjaquensel 833f521
test: update ResponseUtilTest
ronjaquensel 189ede4
chore: fix import order
ronjaquensel ec60ced
chore: fix import order
ronjaquensel 818269e
refactor: return FormDataMultiPart from MultipartController
ronjaquensel cb3fe43
refactor: return token from method in MultipartController
ronjaquensel 48103c3
chore: remove unused contract offer handler
ronjaquensel 95f3634
chore: remove superfluous requireNonNull checks
ronjaquensel 7f4dfa7
refactor: inject monitor into IdsMultipartApiServiceExtension
ronjaquensel d7cbdd5
chore: add TODOs for after project structure review
ronjaquensel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
docs: add comments and Javadoc
commit dea5fcf493f3d5e163bdc4dac1c707c4066947e3
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,6 +76,7 @@ public boolean canHandle(@NotNull MultipartRequest multipartRequest) { | |
public @Nullable MultipartResponse handleRequest(@NotNull MultipartRequest multipartRequest) { | ||
Objects.requireNonNull(multipartRequest); | ||
|
||
// Read and transform the endpoint data reference from the request payload | ||
var edr = typeManager.readValue(multipartRequest.getPayload(), EndpointDataReference.class); | ||
var transformationResult = transformerRegistry.transform(edr); | ||
if (transformationResult.failed()) { | ||
|
@@ -85,6 +86,7 @@ public boolean canHandle(@NotNull MultipartRequest multipartRequest) { | |
|
||
var transformedEdr = transformationResult.getContent(); | ||
|
||
// Apply all endpoint data reference receivers to the endpoint data reference | ||
var receiveResult = receiverRegistry.receiveAll(transformedEdr).join(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not for this PR, but |
||
if (receiveResult.failed()) { | ||
monitor.severe("EDR dispatch failed: " + String.join(", ", receiveResult.getFailureMessages())); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
note for the future (after project structure review), here the
TransferProcessService
should be used instead of the managerThere 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.
Hey, @ronjaquensel, this aligns with our discussion to encapsulate some of the policy evaluation as well.
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.
@ronjaquensel Can we either create an issue or add a TODO for this comment and the one regarding the
ContractNegotiationService
?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.
good point, I added TODOs.