feat(irs-api):[#199] register policy definition for certain bpnls #802

ds-psosnowski · 2024-03-05T14:26:39Z

story: eclipse-tractusx#199

github-actions · 2024-03-05T15:45:27Z

CHANGELOG file was not updated! Make sure to include important changes.

...lient/src/test/java/org/eclipse/tractusx/irs/edc/client/policy/PolicyCheckerServiceTest.java

irs-api/src/main/java/org/eclipse/tractusx/irs/configuration/RegistryConfiguration.java

...istry-client/src/main/java/org/eclipse/tractusx/irs/registryclient/DefaultConfiguration.java

...-store/src/main/java/org/eclipse/tractusx/irs/policystore/persistence/PolicyPersistence.java

...dc-client/src/main/java/org/eclipse/tractusx/irs/edc/client/policy/PolicyCheckerService.java

...tore/src/test/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreServiceTest.java

1) harden test PolicyStoreServiceTest#whenUpdate by asserting that the creation timestamp is not changed and that the validUntil timestamp is changed as expected 2) clean method signature by moving loop from controller to the service method

inlcuding test for helper class

- updatePolicies_shouldAddPolicyToEachBpn - updatePolicies_shouldAddBpnsToEachPolicy

ds-psosnowski · 2024-03-14T09:01:31Z

Hey, please comment out code so I can reply in relevant topics. According to:

list of bpns in path param - this is get request so you cannot have body for it also array is perfectly fine as parameter. If for some reason we don't want that this method need to be converted to post for example.
There is business partner number when you query for all policies. If you are not (you are passing the bpn as param) then there is no need to return it since you are returning polices for given bnps right?
About return type - original get was returning whole policy not just an id, I don't think it would make sense to return only id's for policies that are for some bpns and whole policy when we use getAll, I believe this is mistake in story definition, but we can confirm it.
We agreed on changing param type and leave returning policy not just an id
~~../policy/... or .../policies/... - do you really think that it was intended to use singular only for one request? I would rather think that this is just typo~~ agreed that we will leave it plural as other controller methods

…certian-bpnls # Conflicts: # CHANGELOG.md

dsmf · 2024-03-15T17:12:23Z

merged main again because of new conflict

dsmf

Hi @ds-psosnowski and @ds-jhartmann ,

There are open questions concerning the "default" and why apiAllowedBpn was removed. This may be fine, but I simply don't understand this. Maybe you can clarify this?

dsmf · 2024-03-13T22:42:56Z

...tore/src/test/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreServiceTest.java

+
+        // assert
+        verify(persistence).delete("bpn2", "testId");
+        verify(persistence).save(eq(List.of("bpn1")), any());
    }
 }


Hi @ds-psosnowski , I think we should also add the following tests:

updatePolicies_shouldAddPolicyToEachBpn

updatePolicies_shouldAddBpnsToEachPolicy

See my review branch: https://github.com/catenax-ng/tx-item-relationship-service/tree/feature/%23199-register-policy-definition-for-certian-bpnls--REVIEW in commit id c999b0a

ok looks good now

dsmf · 2024-03-15T17:48:04Z

irs-common/src/main/java/org/eclipse/tractusx/irs/common/persistence/BlobPersistence.java

    void putBlob(String targetBlobName, byte[] blob) throws BlobPersistenceException;

    Optional<byte[]> getBlob(String sourceBlobName) throws BlobPersistenceException;

+    Map<String, byte[]> getAllBlobs() throws BlobPersistenceException;


Hi @ds-psosnowski , Something similar like my suggestion above in PolicyPersistence?

...dc-client/src/main/java/org/eclipse/tractusx/irs/edc/client/policy/PolicyCheckerService.java

...-store/src/main/java/org/eclipse/tractusx/irs/policystore/persistence/PolicyPersistence.java

irs-api/src/main/java/org/eclipse/tractusx/irs/configuration/RegistryConfiguration.java

charts/irs-helm/templates/configmap-spring-app-config.yaml

dsmf

Clarified apiAllowedBpn and default handling with @ds-psosnowski via msteams. Makes sense to me now. Therefore approved from my side.

…certian-bpnls

sonarqubecloud · 2024-03-22T12:33:06Z

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
82.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

ds-jhartmann

LGTM

ds-psosnowski added 2 commits March 5, 2024 15:26

feat(irs-api):[#199] register policy definition for certain bpnls

8028ac4

feat(irs-api):[#199] merge and conflicts resolve

1898634

ds-psosnowski mentioned this pull request Mar 5, 2024

Register policy definition for certain BPNLs eclipse-tractusx/item-relationship-service#199

Closed

6 tasks

ds-psosnowski requested review from dsmf, ds-jhartmann and ds-ext-kmassalski March 5, 2024 16:16

This comment was marked as resolved.

Sign in to view

feat(irs-api):[#199] added changelog entry

c65f58e

ds-psosnowski requested a review from dsmf March 11, 2024 08:48

ds-psosnowski added 2 commits March 12, 2024 15:09

feat(irs-api):[#199] added test

f9a886b

feat(irs-api):[#199] update

4975d01

ds-jhartmann suggested changes Mar 12, 2024

View reviewed changes

feat(irs-api):[#199] added tests

eba7540

dsmf suggested changes Mar 13, 2024

View reviewed changes

...tore/src/test/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreServiceTest.java Outdated Show resolved Hide resolved

ds-psosnowski and others added 3 commits March 13, 2024 13:57

feat(irs-api):[#199] test clean up

dcdd24e

feat(impl): [#199] Fix warnings

9340fb4

This comment was marked as resolved.

Sign in to view

dsmf added 2 commits March 13, 2024 21:26

feat(impl): [#199] Deduplicate by extraction to findBpnsByPolicyId

05ec245

feat(impl): [#199] Avoid too many methods by extraction to helper class

038b85a

inlcuding test for helper class

dsmf mentioned this pull request Mar 13, 2024

Feature/#199 register policy definition for certian bpnls review #816

Merged

feat(impl): [#199] Add tests

c999b0a

- updatePolicies_shouldAddPolicyToEachBpn - updatePolicies_shouldAddBpnsToEachPolicy

ds-psosnowski added 3 commits March 14, 2024 13:29

feat(irs-api):[#199] corrections after review

6a32fde

feat(irs-api):[#199] corrections after review

eea278b

feat(irs-api):[#199] added test

29b7560

ds-psosnowski requested a review from ds-jhartmann March 15, 2024 09:53

ds-psosnowski requested a review from dsmf March 15, 2024 09:53

Merge branch 'main' into feature/#199-register-policy-definition-for-…

f6626c1

…certian-bpnls # Conflicts: # CHANGELOG.md

ds-psosnowski added 3 commits March 18, 2024 08:43

feat(irs-api):[#199] corrections after review

a2f32d9

feat(irs):[#199] corrections after review

be88258

feat(irs):[#199] main merge

c4a4de9

dsmf suggested changes Mar 20, 2024

View reviewed changes

dsmf previously approved these changes Mar 21, 2024

View reviewed changes

feat(irs):[#199] corrections after review

4758554

ds-psosnowski dismissed dsmf’s stale review via 4758554 March 22, 2024 09:22

ds-psosnowski added 7 commits March 22, 2024 10:23

Merge branch 'main' into feature/#199-register-policy-definition-for-…

df7a662

…certian-bpnls

feat(irs):[#199] updated insomnia collection

2d4e1cb

feat(irs):[#199] sonar issues fix

2962615

feat(irs):[#199] sonar issues fix

740b188

feat(irs):[#199] changed spaces in irs request collection

13136f5

feat(irs):[#199] revert white signs in request collection

20c7e49

feat(irs):[#199] changed spaces to tabs in irs request collection

e0ab6b3

ds-jhartmann approved these changes Mar 22, 2024

View reviewed changes

ds-psosnowski merged commit 7703420 into main Mar 22, 2024

ds-psosnowski deleted the feature/#199-register-policy-definition-for-certian-bpnls branch March 22, 2024 12:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(irs-api):[#199] register policy definition for certain bpnls #802

feat(irs-api):[#199] register policy definition for certain bpnls #802

ds-psosnowski commented Mar 5, 2024 •

edited by dsmf

Loading

github-actions bot commented Mar 5, 2024

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

ds-psosnowski commented Mar 14, 2024 •

edited

Loading

dsmf commented Mar 15, 2024

dsmf left a comment

dsmf Mar 13, 2024

dsmf Mar 15, 2024

dsmf Mar 15, 2024

dsmf left a comment

sonarqubecloud bot commented Mar 22, 2024

ds-jhartmann left a comment

feat(irs-api):[#199] register policy definition for certain bpnls #802

feat(irs-api):[#199] register policy definition for certain bpnls #802

Conversation

ds-psosnowski commented Mar 5, 2024 • edited by dsmf Loading

github-actions bot commented Mar 5, 2024

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

ds-psosnowski commented Mar 14, 2024 • edited Loading

dsmf commented Mar 15, 2024

dsmf left a comment

Choose a reason for hiding this comment

dsmf Mar 13, 2024

Choose a reason for hiding this comment

dsmf Mar 15, 2024

Choose a reason for hiding this comment

dsmf Mar 15, 2024

Choose a reason for hiding this comment

dsmf left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 22, 2024

Quality Gate passed

ds-jhartmann left a comment

Choose a reason for hiding this comment

ds-psosnowski commented Mar 5, 2024 •

edited by dsmf

Loading

ds-psosnowski commented Mar 14, 2024 •

edited

Loading