Feature/639 handle policy expiration #1041
Conversation
ds-lcapellino
force-pushed
the
feature/639-handle-policy-expiration
branch
from
471ce6b to
88b7fd5
Compare
| @@ -119,7 +119,7 @@ private String negotiateContractAgreement(final String receiverEdcUrl, final Cat | |||
| .orElseThrow() | |||
There was a problem hiding this comment.
Currently the issue is that on the getCatalog call within the class NotificationsEdcFacade between line 157 and 164 we filter out all elements which are invalid.
Instead of filtering them out we need to catch them in a different exception.
e.g. PolicyExpiredException
This policy then will be cached again in EdcNotificationServiceImpl#handleSendingNotification, where we need to enrich the error message into the notification.
At the end we will have the info in the frontend that it was not possible to send it out due to policy issues.
There was a problem hiding this comment.
PolicyCheckerService was adapted according to the requirements in issue #639. Please see eclipse-tractusx/item-relationship-service@6e8880b#diff-b63cc42711ba192e7a2ff818eaad3bf34984ebf1f30d53b2f5df2abfa468e623 fore reference.
This handles "NotificationsEdcFacade between line 157 and 164".
UsagePolicyExpiredException was also added: eclipse-tractusx/item-relationship-service@6e8880b#diff-3db264f1f24f39482d77c13221eaba18eba63de4620a7598ba4a1d404623e8ce
This is thrown, if the policy is expired during contract negotiation.
There was a problem hiding this comment.
Now the filter is gone. That is good. Still some things missing.
When the client library throws an exception within the contract negotiation (the point were you changed the exceptions) it will throw an exception and based on the logic of trace-x it will be catched and always mapped to the generic "NegotiationException" which is for our usage not enough.
We need to know if the policy was expired, or the policy constraints are incorrect or the policy is missing.
Therefore please catch those exceptions and pass them to the errorEnricher. To make sure we have that information in the frontend available.
If this is not manageable within the story - please recheck with product owner if new issue can be created for following up on that one.
|
@ds-mwesener please re-review |
| .post("/api/notifications/{investigationId}/approve", investigationId) | ||
| .then() | ||
| .log().all() | ||
| .statusCode(503); |
There was a problem hiding this comment.
This is the current implementation in case something fails, when trying to send notifications. Maybe HTTP 500 suits better
There was a problem hiding this comment.
In my opinion a 400 would match better, since the policy is part of a notification negotiation.
ds-lcapellino
force-pushed
the
feature/639-handle-policy-expiration
branch
from
6bf890e to
e1eb275
Compare
# Conflicts: # tx-backend/src/main/java/org/eclipse/tractusx/traceability/notification/domain/base/service/NotificationsEDCFacade.java
…-expiration' into feature/639-handle-policy-expiration
Description
Pre-review checks
Please ensure to do as many of the following checks as possible, before asking for committer review: