-
Notifications
You must be signed in to change notification settings - Fork 21
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: consume offering #491
Conversation
* Add missing annotations * Align class names * Add null checks
…ensions into feat/182-consume-offering feat: merge latest upstream changes
…sferProcessOutputDto
# Conflicts: # extensions/wrapper/wrapper-common-api/src/main/java/de/sovity/edc/ext/wrapper/api/common/model/CriterionDto.java # extensions/wrapper/wrapper/build.gradle.kts # extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtension.java # extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtensionContextBuilder.java # extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/usecase/model/ContractDefinitionRequestDto.java # extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/usecase/services/OfferingService.java # extensions/wrapper/wrapper/src/test/java/de/sovity/edc/ext/wrapper/api/usecase/services/OfferingServiceTest.java # extensions/wrapper/wrapper/src/test/resources/usecase/contract-offer-valid.json # gradle.properties # settings.gradle.kts
…rns and problem reporting
import lombok.RequiredArgsConstructor; | ||
import lombok.Setter; | ||
import lombok.ToString; | ||
import lombok.*; |
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.
convention: no asterisk imports (requires fix in intellij settings)
@@ -15,6 +15,7 @@ | |||
|
|||
import io.swagger.v3.oas.annotations.media.Schema; | |||
import lombok.AllArgsConstructor; | |||
import lombok.Builder; |
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.
unused import?
doLast { | ||
println(sourceSets["main"].runtimeClasspath.asPath) | ||
} | ||
} |
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.
This seems to be for local debugging only, right?
private String id; | ||
private String providerId; | ||
private String consumerId; | ||
private long contractSigningDate; |
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.
convention: API should use real date classes, e.g. OffsetDateTime
transformerRegistry.register(new PermissionToPermissionDtoTransformer()); | ||
transformerRegistry.register(new PolicyToPolicyDtoTransformer()); | ||
transformerRegistry.register(new ContractAgreementToContractAgreementDtoTransformer()); | ||
transformerRegistry.register(new DataRequestToDataRequestDtoTransformer()); | ||
transformerRegistry.register(new ContractNegotiationToContractNegotiationOutputDtoTransformer()); | ||
transformerRegistry.register(new TransferProcessToTransferProcessOutputDtoTransformer()); | ||
var consumptionService = new ConsumptionService(contractNegotiationService, transferProcessService, | ||
contractNegotiationStore, transferProcessStore, transformerRegistry, policyMappingService); | ||
|
||
negotiationObservable.registerListener(new ContractNegotiationConsumptionListener(consumptionService)); |
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.
As long as we can we should try to avoid using the transformer registry to allow to see the code flow, calling functions depending on what's necessary, rather than registering global resources / mappings.
violates clean code / YAGNI
private String id; | ||
|
||
/** The input that was used to start this process. */ | ||
private ConsumptionInputDto input; |
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.
Is this required?
@Getter | ||
public class ContractNegotiationOutputDto { | ||
private String id; | ||
private String state; |
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.
String state, what information does this encode?
private String counterPartyId; | ||
private String counterPartyAddress; | ||
private String protocol; | ||
private ContractAgreementDto contractAgreement; |
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.
should this be unnested?
* A DTO Class for DataRequests | ||
* | ||
* @author Haydar Qarawlus | ||
*/ |
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.
what does this class do?
private String state; | ||
private DataRequestDto dataRequest; | ||
private Map<String, String> contentDataAddress; | ||
private Map<String, String> privateProperties = new HashMap<>(); |
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.
should we really output the data address and private properties here?
In general, I'd recommend taking a look once more at these topics:
|
closed the PR, the general API model of the Use Case API is still on a MS8-State. Using EDC 0 we'll create a new simplified Use Case API that also re-uses the classes |
Pull Request
Implements the workflow for consuming an offering (asset, policy, contract definition) as stated in #182. Adds an API endpoint for consuming an offering with a single API call.
Will start a contract negotiation and once that is finisehd it will start a data transfer.
How Has This Been Tested?
All new classes are tested via Unit Testing, in addition to this there is an integration test covering the whole consumption process.
Linked Issue(s)
Part of #182 .
Checklist