feat: replace contract dates with policy #2950

paullatzelsperger · 2023-05-11T09:09:59Z

What this PR changes/adds

This PR does a couple of things:

removes start and end date from ContractOffer
removes validity from ContractDefinition and related DTOs
adds a function to evaluate in-force period policies

Why it does that

validity periods of contracts should be represented by policies

Further notes

I did add an e2e test in AbsractEndToEndTransfer.java, but because Rejection Messages are not properly handled in IDS, I @Disabled those tests. Once the tests use the new DSP, we should be able to re-enable them.
This is a very large changeset, so reviewers are advised to focus mainly on the following classes:
- ContractExpiryCheckFunction.java: contains the permission function to validate a time-limiting policy
- ContractValidationServiceImpl.java: where the policy is supposed to be evaluated
- ContractOffer.java: removed the start- and end-date as well as the offerstart and end date
- ContractAgreement.java: removed the start- and end-date

Linked Issue(s)

Closes #2758

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 Etiquette for pull requests for details)

codecov-commenter · 2023-05-11T09:36:27Z

Codecov Report

Patch coverage: 64.10% and project coverage change: +0.05 🎉

Comparison is base (ad9ed18) 66.19% compared to head (fdc439f) 66.24%.

❗ Current head fdc439f differs from pull request most recent head 754dd77. Consider uploading reports for the commit 754dd77 to get more accurate results

❗ 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    #2950      +/-   ##
==========================================
+ Coverage   66.19%   66.24%   +0.05%     
==========================================
  Files        1039     1041       +2     
  Lines       21281    21227      -54     
  Branches     1259     1258       -1     
==========================================
- Hits        14086    14062      -24     
+ Misses       6672     6642      -30     
  Partials      523      523

Impacted Files	Coverage Δ
...iation/ConsumerContractNegotiationManagerImpl.java	`96.61% <ø> (-0.06%)`	⬇️
...iation/ProviderContractNegotiationManagerImpl.java	`96.03% <ø> (-0.08%)`	⬇️
...tAgreementFromIdsContractAgreementTransformer.java	`81.63% <ø> (+2.38%)`	⬆️
...actAgreementToIdsContractAgreementTransformer.java	`80.48% <ø> (+4.97%)`	⬆️
...OfferFromIdsContractOfferOrRequestTransformer.java	`76.31% <ø> (+2.50%)`	⬆️
...ct/ContractOfferToIdsContractOfferTransformer.java	`83.33% <ø> (+10.83%)`	⬆️
.../contractagreement/model/ContractAgreementDto.java	`100.00% <ø> (ø)`
...actAgreementToContractAgreementDtoTransformer.java	`100.00% <ø> (ø)`
...JsonObjectFromContractAgreementDtoTransformer.java	`100.00% <ø> (ø)`
...definition/model/ContractDefinitionRequestDto.java	`100.00% <ø> (ø)`
... and 26 more

... and 13 files 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.

jimmarino · 2023-05-12T07:50:58Z

...src/main/java/org/eclipse/edc/connector/contract/validation/ContractExpiryCheckFunction.java

+    private Duration asDuration(String rightValueStr) {
+        var matcher = Pattern.compile(EXPRESSION_REGEX).matcher(rightValueStr);
+        if (matcher.matches()) {
+            var number = Integer.parseInt(matcher.group(REGEX_GROUP_NUMERIC));


@paullatzelsperger should we catch and handle this or let it bubble? We could also address in another PR

jimmarino

Awesome stuff, especially removing code ;-) One small comment but that can either stay the same or be addressed later.

jimmarino · 2023-05-12T07:50:58Z

...src/main/java/org/eclipse/edc/connector/contract/validation/ContractExpiryCheckFunction.java

+    private Duration asDuration(String rightValueStr) {
+        var matcher = Pattern.compile(EXPRESSION_REGEX).matcher(rightValueStr);
+        if (matcher.matches()) {
+            var number = Integer.parseInt(matcher.group(REGEX_GROUP_NUMERIC));


@paullatzelsperger should we catch and handle this or let it bubble? We could also address in another PR

paullatzelsperger · 2023-05-12T07:57:25Z

@paullatzelsperger should we catch and handle this or let it bubble? We could also address in another PR

I think it's not an issue, since it would never match the Regex unless its a numeric value. Maybe another test for this would be in order though :)

wolf4ood

LGTM 🪓

paullatzelsperger added enhancement New feature or request core feature labels May 11, 2023

paullatzelsperger self-assigned this May 11, 2023

paullatzelsperger temporarily deployed to Azure-dev May 11, 2023 09:11 — with GitHub Actions Inactive

paullatzelsperger temporarily deployed to Azure-dev May 11, 2023 09:40 — with GitHub Actions Inactive

paullatzelsperger temporarily deployed to Azure-dev May 11, 2023 10:07 — with GitHub Actions Inactive

paullatzelsperger temporarily deployed to Azure-dev May 11, 2023 17:32 — with GitHub Actions Inactive

github-advanced-security bot found potential problems May 11, 2023

View reviewed changes

paullatzelsperger force-pushed the feature/2758_replace_contractdates_with_policy branch from 776554a to 7bc6624 Compare May 11, 2023 17:47

paullatzelsperger temporarily deployed to Azure-dev May 11, 2023 17:48 — with GitHub Actions Inactive

paullatzelsperger added 7 commits May 11, 2023 20:36

removed contractoffer dates

33ec236

remove validity field from ContractDefinition

c7ea695

removed dates from DTOs

8a07ef2

fix openapi, checkstyle

b44ec38

support for fixed date ranges

53cb5fe

add support for duration policies

970551d

fixed some SQL bugs

269ce7e

paullatzelsperger force-pushed the feature/2758_replace_contractdates_with_policy branch from 7bc6624 to 269ce7e Compare May 11, 2023 19:02

paullatzelsperger temporarily deployed to Azure-dev May 11, 2023 19:03 — with GitHub Actions Inactive

openapi

3f0b75d

paullatzelsperger temporarily deployed to Azure-dev May 11, 2023 19:09 — with GitHub Actions Inactive

add dummy for an e2e test

68f788c

paullatzelsperger temporarily deployed to Azure-dev May 12, 2023 06:36 — with GitHub Actions Inactive

documentation

3497547

paullatzelsperger marked this pull request as ready for review May 12, 2023 06:57

paullatzelsperger requested review from ndr-brt, ronjaquensel and juliapampus as code owners May 12, 2023 06:57

paullatzelsperger requested a review from jimmarino May 12, 2023 06:57

paullatzelsperger temporarily deployed to Azure-dev May 12, 2023 07:13 — with GitHub Actions Inactive

javadoc

fdc439f

paullatzelsperger temporarily deployed to Azure-dev May 12, 2023 07:43 — with GitHub Actions Inactive

paullatzelsperger requested a review from wolf4ood May 12, 2023 07:46

jimmarino approved these changes May 12, 2023

View reviewed changes

wolf4ood approved these changes May 12, 2023

View reviewed changes

more tests

754dd77

ronjaquensel approved these changes May 12, 2023

View reviewed changes

paullatzelsperger temporarily deployed to Azure-dev May 12, 2023 08:05 — with GitHub Actions Inactive

paullatzelsperger merged commit 3acbbb4 into eclipse-edc:main May 12, 2023

bscholtes1A mentioned this pull request Jun 9, 2023

bug: ContractAgreement validity is not longer asserted when validating token received by Data Plane #3160

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: replace contract dates with policy #2950

feat: replace contract dates with policy #2950

paullatzelsperger commented May 11, 2023 •

edited

Loading

codecov-commenter commented May 11, 2023 •

edited

Loading

jimmarino May 12, 2023

jimmarino left a comment

jimmarino May 12, 2023

paullatzelsperger commented May 12, 2023

wolf4ood left a comment

feat: replace contract dates with policy #2950

feat: replace contract dates with policy #2950

Conversation

paullatzelsperger commented May 11, 2023 • edited Loading

What this PR changes/adds

Why it does that

Further notes

Linked Issue(s)

Checklist

codecov-commenter commented May 11, 2023 • edited Loading

Codecov Report

jimmarino May 12, 2023

Choose a reason for hiding this comment

jimmarino left a comment

Choose a reason for hiding this comment

jimmarino May 12, 2023

Choose a reason for hiding this comment

paullatzelsperger commented May 12, 2023

wolf4ood left a comment

Choose a reason for hiding this comment

paullatzelsperger commented May 11, 2023 •

edited

Loading

codecov-commenter commented May 11, 2023 •

edited

Loading