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

refactor: clean up :ids-transform and :ids-spi #1750

Conversation

juliapampus
Copy link
Contributor

@juliapampus juliapampus commented Jul 27, 2022

What this PR changes/adds

Refactors modules :ids-transform and :ids-spi.

  • Introduces package structure to :ids-transform
  • Refactores package structure in :ids-spi
  • Unifies DefaultValues and IdsProtocol to IdsConstants in :ids-spi, removes IdsProtocol from :ids-transform
  • Deletes IdsIdParser and introduces methods toUri, from(URI) and from(String) to IdsId
  • Removes transformer classes for IdsId to URI and vice versa
  • Renames transformer classes to "toIds" and "fromIds" for a better class name ordering
  • Refactors all transformer classes
  • Merge ContractOfferFromIdsContractOfferTransformer and ContractOfferFromIdsContractRequestTransformer to ContractOfferFromIdsContractOfferOrRequestTransformer
  • Cleans up some transformer tests
  • Adds mapping of edc-sepcific policy properties to and from IDS contract objects

Why it does that

Cleaning up ids modules

Further notes

core clean up will follow, e.g. for better readability

Linked Issue(s)

Closes #1473
Relates #1351
Relates #1761

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)

@juliapampus juliapampus added the refactoring Cleaning up code and dependencies label Jul 27, 2022
@juliapampus juliapampus requested a review from ndr-brt July 27, 2022 17:19
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2022

Codecov Report

Merging #1750 (939bcfe) into main (33803c4) will decrease coverage by 57.18%.
The diff coverage is 6.79%.

@@              Coverage Diff              @@
##               main    #1750       +/-   ##
=============================================
- Coverage     67.77%   10.59%   -57.19%     
- Complexity        0      477      +477     
=============================================
  Files           795      793        -2     
  Lines         16968    16840      -128     
  Branches       1079     1076        -3     
=============================================
- Hits          11500     1784     -9716     
- Misses         5005    14973     +9968     
+ Partials        463       83      -380     
Impacted Files Coverage Δ
...ispatcher/IdsMultipartRemoteMessageDispatcher.java 50.00% <0.00%> (ø)
...nder/MultipartCatalogDescriptionRequestSender.java 5.88% <0.00%> (ø)
...r/MultipartEndpointDataReferenceRequestSender.java 21.42% <0.00%> (-71.43%) ⬇️
.../api/multipart/handler/ArtifactRequestHandler.java 14.28% <0.00%> (-51.85%) ⬇️
...pi/multipart/handler/ContractAgreementHandler.java 70.58% <ø> (ø)
...aceconnector/ids/core/IdsCoreServiceExtension.java 0.00% <0.00%> (ø)
...tor/ids/core/serialization/IdsTypeManagerUtil.java 0.00% <0.00%> (-75.61%) ⬇️
...nnector/ids/core/service/ConnectorServiceImpl.java 0.00% <ø> (-100.00%) ⬇️
...tor/ids/core/service/ConnectorServiceSettings.java 0.00% <0.00%> (-1.08%) ⬇️
...ataspaceconnector/ids/spi/domain/IdsConstants.java 0.00% <0.00%> (ø)
... and 590 more

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

@juliapampus juliapampus force-pushed the refactor/clean_up_ids_transformers branch 10 times, most recently from f1f9dc8 to bac3e68 Compare July 28, 2022 14:10
@juliapampus juliapampus changed the title refactor(ids): clean up ids-transform and ids-spi refactor: clean up :ids-transform and :ids-spi Jul 28, 2022
@juliapampus juliapampus requested a review from ndr-brt July 28, 2022 14:46
@juliapampus juliapampus force-pushed the refactor/clean_up_ids_transformers branch 5 times, most recently from 058285f to 046df2d Compare August 2, 2022 16:15
@juliapampus juliapampus requested a review from ndr-brt August 2, 2022 16:18
@juliapampus juliapampus force-pushed the refactor/clean_up_ids_transformers branch 2 times, most recently from 5c093b6 to 939bcfe Compare August 3, 2022 09:27
@juliapampus juliapampus requested a review from ndr-brt August 3, 2022 09:32
@juliapampus
Copy link
Contributor Author

@paullatzelsperger @alexandrudanciu Added the mapping of whatever Policy properties to/from the ids ContractOffer and ContractAgreement. See PropertyUtilTest for a legalText example (you may call it differently).

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.

I missed a possible NPE, after this I think we are ready to go from my side

@juliapampus juliapampus force-pushed the refactor/clean_up_ids_transformers branch from 939bcfe to 34f90fc Compare August 3, 2022 09:59
@juliapampus juliapampus force-pushed the refactor/clean_up_ids_transformers branch from 34f90fc to 322d1c8 Compare August 3, 2022 10:00
@juliapampus juliapampus requested a review from ndr-brt August 3, 2022 10:12
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.

🚀

@juliapampus juliapampus merged commit 8eb991d into eclipse-edc:main Aug 3, 2022
@juliapampus juliapampus deleted the refactor/clean_up_ids_transformers branch August 3, 2022 12:32
diegogomez-zf pushed a commit to diegogomez-zf/DataSpaceConnector that referenced this pull request Aug 3, 2022
* refactor: clean up `ids-transform` and `ids-spi`

* chore: add edc policy property mapping to/from ids contract objects
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Cleaning up code and dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move IdsProtocol from ids-transform
3 participants