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: split Dtos in InputDto and OutputDto for Asset and ContractDefinition #1797

Merged
merged 7 commits into from
Aug 10, 2022
Merged

feat: split Dtos in InputDto and OutputDto for Asset and ContractDefinition #1797

merged 7 commits into from
Aug 10, 2022

Conversation

ndr-brt
Copy link
Member

@ndr-brt ndr-brt commented Aug 8, 2022

What this PR changes/adds

Split

  • AssetDto in AssetInputDto and AssetOutputDto
  • ContractDefinitionDto in ContractDefinitionInputDto and ContractDefinitionOutputDto

Why it does that

To improve APIs, as currently the spec are stating that a createdAt field can be passed from the client, but that's not true as that field is valued by the EDC itself

Further notes

  • remove duplication between TransformerContextImpl and DtoTransformerContext that were the pretty much the same class.
  • improve validation on the ids, as they shouldn't be mandatory from the client but they should be created by the EDC as random UUID by default (as done for PolicyDefinition, ContractNegotiation, TransferProcess and DataRequest)
  • make the asset and contract definition transformers "NPE proof". This would lead to a subsequent refactor in another PR as all the transformer should handle the "null input" correctly

Linked Issue(s)

Closes #1796

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 styleguide for details)

@ndr-brt ndr-brt added enhancement New feature or request api Feature related to the (REST) api labels Aug 8, 2022
@paullatzelsperger
Copy link
Member

paullatzelsperger commented Aug 9, 2022

I have not yet looked at this in greater detail, but my inner monk somehow would immediately feel more comfortable if they were named RequestDto and ResponseDto

@ndr-brt
Copy link
Member Author

ndr-brt commented Aug 10, 2022

I have not yet looked at this in greater detail, but my inner monk somehow would immediately feel more comfortable if they were named RequestDto and ResponseDto

@paullatzelsperger makes total sense to me. Done

@codecov-commenter
Copy link

Codecov Report

Merging #1797 (2f523f5) into main (fca34e5) will decrease coverage by 57.14%.
The diff coverage is 0.00%.

@@              Coverage Diff              @@
##               main    #1797       +/-   ##
=============================================
- Coverage     67.78%   10.63%   -57.15%     
- Complexity        0      485      +485     
=============================================
  Files           800      802        +2     
  Lines         16977    17022       +45     
  Branches       1080     1080               
=============================================
- Hits          11508     1811     -9697     
- Misses         4999    15126    +10127     
+ Partials        470       85      -385     
Impacted Files Coverage Δ
...ids/core/transform/IdsTransformerRegistryImpl.java 0.00% <0.00%> (-65.22%) ⬇️
.../dataspaceconnector/api/model/BaseResponseDto.java 0.00% <0.00%> (ø)
...lipse/dataspaceconnector/api/model/MutableDto.java 0.00% <0.00%> (ø)
...or/api/transformer/DtoTransformerRegistryImpl.java 0.00% <0.00%> (-88.00%) ⬇️
...r/api/datamanagement/asset/AssetApiController.java 0.00% <0.00%> (-97.73%) ⬇️
...or/api/datamanagement/asset/AssetApiExtension.java 0.00% <0.00%> (-100.00%) ⬇️
.../api/datamanagement/asset/model/AssetEntryDto.java 0.00% <ø> (-100.00%) ⬇️
...pi/datamanagement/asset/model/AssetRequestDto.java 0.00% <0.00%> (ø)
...i/datamanagement/asset/model/AssetResponseDto.java 0.00% <0.00%> (ø)
...t/transform/AssetRequestDtoToAssetTransformer.java 0.00% <0.00%> (ø)
... and 550 more

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

@ndr-brt ndr-brt merged commit 797b8ff into eclipse-edc:main Aug 10, 2022
@ndr-brt ndr-brt deleted the feature/1796-separate-dto-input-output branch August 10, 2022 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Feature related to the (REST) api enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Separate ContractDefinitionDto and AssetDto in input and output parts
3 participants