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

Feature/639 handle policy expiration #1041

Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
3b6739f
feature: #639 update irs-lib
ds-lcapellino Jun 11, 2024
30c995a
Merge branch 'main' into feature/639-handle-policy-expiration
ds-lcapellino Jun 12, 2024
88b7fd5
feature: #639 adapt integration tests
ds-lcapellino Jun 17, 2024
e292065
feature: #639 adapt integration tests
ds-lcapellino Jun 17, 2024
e1eb275
feature: #639 fix integration tests
ds-lcapellino Jun 18, 2024
81d839a
feature: #639 remove PolicyCheckerService.isValid() check
ds-lcapellino Jun 18, 2024
ef77bc0
Merge branch 'main' into feature/639-handle-policy-expiration
ds-lcapellino Jun 19, 2024
fc5f75b
Merge branch 'main' into feature/639-handle-policy-expiration
ds-lcapellino Jun 19, 2024
8efea7e
Merge branch 'main' into feature/639-handle-policy-expiration
ds-lcapellino Jun 27, 2024
32536c8
feature: #639 test
ds-lcapellino Jun 28, 2024
e86bfba
feature: #639 test
ds-lcapellino Jun 28, 2024
1059d7e
Merge branch 'main' into feature/639-handle-policy-expiration
ds-lcapellino Jun 28, 2024
19fc8e9
feature: #639 test
ds-lcapellino Jun 28, 2024
5568cbf
feature: #639 fix integration tests
ds-lcapellino Jun 28, 2024
fa8349b
feature: #639 don't show sql
ds-lcapellino Jun 28, 2024
bf9db9f
feature: #639 refactor
ds-lcapellino Jul 1, 2024
6540dd2
Merge branch 'main' into feature/639-handle-policy-expiration
ds-lcapellino Jul 1, 2024
8e229fd
feature: #639 update irs-client-lib.version
ds-lcapellino Jul 1, 2024
932eedc
Merge remote-tracking branch 'ds-lcapellino/feature/639-handle-policy…
ds-lcapellino Jul 1, 2024
1d7215e
feature: #639 update irs-client-lib.version
ds-lcapellino Jul 2, 2024
5659c08
feature: #639 fix integration tests
ds-lcapellino Jul 2, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ _**For better traceability add the corresponding GitHub issue number in each cha
- #1037 extended autocomplete api by contractAgreementId
- #985 Added function to save Contracts based on notification contractAgreementIds into the database
- #985 Added function to filter notifications for contractAgreementIds
- XXX updated JsonSchemaTest now the test pulls the latest version of the json file
- XXX deactivated a test class in tx-backend which behaved undesirably
- #1017 updated contributing, notice, and readme files for TRG 7
- #639 handle expired or incorrect policies when sending notifications
- #786 Added authorization as admin for submodel api & registry api

### Added
Expand All @@ -25,6 +29,7 @@ _**For better traceability add the corresponding GitHub issue number in each cha
- XXX Added interceptor to EdcRestTemplates to log requests
- #915 Added section to documentation: EDC-BPN configuration
- #1037 Added link from contracts view to the corresponding filtered part table view
- #1017 added file for CC BY 4.0 license for TRG 7
- #985 Added reference to part/notification under contract
- #786 Added icons on part table to let admin reload registry / sync assets via IRS

Expand All @@ -33,19 +38,6 @@ _**For better traceability add the corresponding GitHub issue number in each cha
- XXX Removed EdcNotifiactionMockServiceImpl class and replaced with mocks
- #1033 removed action jira-publish-release workflow

### Changed

- XXX updated JsonSchemaTest now the test pulls the latest version of the json file
- XXX deactivated a test class in tx-backend which behaved undesirably

### Added

- #1017 added file for CC BY 4.0 license for TRG 7

### Changed

- #1017 updated contributing, notice, and readme files for TRG 7

## [11.0.2 - 29.05.2024]
### Added
- #1010 Made submodel path configurable
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ SPDX-License-Identifier: Apache-2.0
<junit-bom.version>5.10.2</junit-bom.version>
<awaitility.version>4.2.1</awaitility.version>
<!-- TODO https://github.com/eclipse-tractusx/traceability-foss/issues/978 update to the cx release version of irs lib IMPORTANT NO SNAPSHOT-->
<irs-client-lib.version>2.0.5</irs-client-lib.version>
<irs-client-lib.version>2.1.0</irs-client-lib.version>
<json-schema-validator.version>5.4.0</json-schema-validator.version>
<!-- Sonar related properties -->
<sonar.java.coveragePlugin>jacoco</sonar.java.coveragePlugin>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ private static List<DetailAspectModel> extractDetailAspectModelsPartSiteInformat
List<DetailAspectModel> detailAspectModels = new ArrayList<>();
emptyIfNull(sites).forEach(site -> {
DetailAspectDataPartSiteInformationAsPlanned detailAspectDataPartSiteInformationAsPlanned = DetailAspectDataPartSiteInformationAsPlanned.builder()
.catenaXSiteId(site.catenaXSiteId())
.catenaXSiteId(site.catenaXsiteId())
.functionValidFrom(site.functionValidFrom().toOffsetDateTime())
.function(site.function())
.functionValidUntil(site.functionValidUntil().toOffsetDateTime())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public void approve(Long notificationId) {
approvedInvestigation = notificationPublisherService.approveNotification(notification);
} catch (SendNotificationException exception) {
log.info("Notification status rollback", exception);
throw new SendNotificationException(exception.getMessage());
throw new SendNotificationException(exception.getMessage(), exception);
}
getNotificationRepository().updateNotification(approvedInvestigation);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public CompletableFuture<NotificationMessage> asyncNotificationMessageExecutor(N
return CompletableFuture.completedFuture(null);

} catch (DiscoveryFinderException discoveryFinderException) {
enrichNotificationByError(discoveryFinderException, notification);
enrichNotificationByError(discoveryFinderException, notification, message);
return CompletableFuture.completedFuture(null);
}
}
Expand All @@ -97,30 +97,25 @@ private boolean handleSendingNotification(NotificationMessage message, String se
return true;
} catch (NoCatalogItemException e) {
log.warn("Could not send message to {} no catalog item found. ", receiverUrl, e);
enrichNotificationByError(e, notification);
enrichNotificationByError(e, notification, message);
} catch (SendNotificationException e) {
log.warn("Could not send message to {} ", receiverUrl, e);
enrichNotificationByError(e, notification);
enrichNotificationByError(e, notification, message);
} catch (NoEndpointDataReferenceException e) {
log.warn("Could not send message to {} no endpoint data reference found", receiverUrl, e);
enrichNotificationByError(e, notification);
enrichNotificationByError(e, notification, message);
} catch (ContractNegotiationException e) {
log.warn("Could not send message to {} could not negotiate contract agreement", receiverUrl, e);
enrichNotificationByError(e, notification);
enrichNotificationByError(e, notification, message);
}
return false;
}

private void enrichNotificationByError(Exception e, Notification notification) {
private void enrichNotificationByError(Exception e, Notification notification, NotificationMessage message) {

log.info("Notification for error message enrichment {}", notification);
notification.getNotifications().forEach(message1 -> log.info("Message found {}", message1));
notification.secondLatestNotifications().forEach(qmMessage -> {
log.info("Message from second latest notification {}", qmMessage);
qmMessage.setErrorMessage(e.getMessage());
});

notificationRepository.updateErrorMessage(notification);
log.info("Notification for error message enrichment {}", message);
message.setErrorMessage(e.getMessage());
notificationRepository.updateErrorMessage(notification, message);

}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ private String negotiateContractAgreement(final String receiverEdcUrl, final Cat
.orElseThrow()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@ds-mwesener ds-mwesener Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

.getContractAgreementId();
} catch (Exception e) {
throw new ContractNegotiationException("Failed to negotiate contract agreement. ", e);
throw new ContractNegotiationException("Failed to negotiate contract agreement: " + e.getMessage(), e);
}
}

Expand Down Expand Up @@ -164,14 +164,6 @@ private CatalogItem getCatalogItem(final NotificationMessage notification, final
.build())
.build()
).stream()
.filter(catalogItem -> {
log.info("-- catalog item check --");
log.info("Item {}: {}", catalogItem.getItemId(), catalogItem);
boolean isValid = policyCheckerService.isValid(catalogItem.getPolicy(), notification.getSentTo()
);
log.info("IsValid : {}", isValid);
return isValid;
})
.findFirst()
.orElseThrow();
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.eclipse.tractusx.traceability.common.model.SearchCriteria;
import org.eclipse.tractusx.traceability.notification.domain.base.model.Notification;
import org.eclipse.tractusx.traceability.notification.domain.base.model.NotificationId;
import org.eclipse.tractusx.traceability.notification.domain.base.model.NotificationMessage;
import org.eclipse.tractusx.traceability.notification.domain.base.model.NotificationSide;
import org.eclipse.tractusx.traceability.notification.domain.base.model.NotificationType;
import org.springframework.data.domain.Pageable;
Expand Down Expand Up @@ -53,7 +54,7 @@ public interface NotificationRepository {

List<String> getDistinctFieldValues(String fieldName, String startWith, Integer resultLimit, NotificationSide owner);

void updateErrorMessage(Notification investigation);
void updateErrorMessage(Notification notification, NotificationMessage message);

void deleteByIdIn(List<String> messageIds);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@
import org.eclipse.tractusx.traceability.notification.infrastructure.notification.model.NotificationMessageEntity;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;
import org.springframework.transaction.annotation.Transactional;

@Repository
public interface JpaNotificationMessageRepository extends JpaRepository<NotificationMessageEntity, String> {
@Transactional
default NotificationMessageEntity updateOrInsert(NotificationMessageEntity notificationMessageEntity) {
return save(notificationMessageEntity);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import jakarta.persistence.EntityManager;
import jakarta.persistence.PersistenceContext;
import jakarta.transaction.Transactional;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.eclipse.tractusx.traceability.assets.domain.base.model.Owner;
Expand All @@ -48,6 +47,7 @@
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.domain.Specification;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;

import java.time.Clock;
import java.time.LocalDateTime;
Expand Down Expand Up @@ -163,19 +163,16 @@ public List<String> getDistinctFieldValues(String fieldName, String startWith, I
}

@Override
public void updateErrorMessage(Notification notification) {
public void updateErrorMessage(Notification notification, NotificationMessage message) {
log.info("Starting update of error message with notification {}", notification);
NotificationEntity notificationEntity = jpaNotificationRepository.findById(notification.getNotificationId().value())
.orElseThrow(() -> new IllegalArgumentException(String.format("Notification with id %s not found!", notification.getNotificationId().value())));

for (NotificationMessage notificationMessage : notification.getNotifications()) {
List<AssetAsBuiltEntity> assetEntitiesByNotification = getAssetEntitiesByNotification(notification);
NotificationMessageEntity notificationMessageEntity = toNotificationMessageEntity(notificationEntity, notificationMessage, assetEntitiesByNotification);
notificationMessageEntity.setErrorMessage(notificationMessage.getErrorMessage());
notificationMessageEntity.setUpdated(LocalDateTime.ofInstant(clock.instant(), clock.getZone()));
jpaNotificationMessageRepository.save(notificationMessageEntity);
}
jpaNotificationRepository.save(notificationEntity);
List<AssetAsBuiltEntity> assetEntitiesByNotification = getAssetEntitiesByNotification(notification);
NotificationMessageEntity notificationMessageEntity = toNotificationMessageEntity(notificationEntity, message, assetEntitiesByNotification);
notificationMessageEntity.setErrorMessage(message.getErrorMessage());
notificationMessageEntity.setUpdated(LocalDateTime.ofInstant(clock.instant(), clock.getZone()));
jpaNotificationMessageRepository.updateOrInsert(notificationMessageEntity);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,8 @@ class NotificationsEDCFacadeTest {
@Mock
EndpointDataReferenceStorage endpointDataReferenceStorage;
@Mock
PolicyCheckerService policyCheckerService;
@Mock
EndpointDataReference endpointDataReference;
@Mock
NotificationRepository notificationRepository;
@Mock
Notification notification;
@InjectMocks
NotificationsEDCFacade notificationsEDCFacade;
Expand All @@ -92,7 +88,6 @@ void givenCorrectInvestigationMessageButSendRequestThrowsException_whenStartEdcT
when(notification.getSeverity()).thenReturn(NotificationSeverity.MAJOR);
when(edcProperties.getIdsPath()).thenReturn(idsPath);
when(edcCatalogFacade.fetchCatalogItems(any())).thenReturn(List.of(catalogItem));
when(policyCheckerService.isValid(null, null)).thenReturn(true);
when(contractNegotiationService.negotiate(receiverEdcUrl + idsPath, catalogItem, null, null))
.thenReturn(NegotiationResponse.builder().contractAgreementId(agreementId).build());
when(endpointDataReference.getEndpoint()).thenReturn("endpoint");
Expand Down Expand Up @@ -121,7 +116,6 @@ void givenCorrectInvestigationMessageButNegotiateContractAgreementHasNoCatalogIt
final String idsPath = "/api/v1/dsp";
when(edcProperties.getIdsPath()).thenReturn(idsPath);
when(edcCatalogFacade.fetchCatalogItems(any())).thenReturn(List.of(catalogItem));
when(policyCheckerService.isValid(null, null)).thenReturn(true);
when(contractNegotiationService.negotiate(receiverEdcUrl + idsPath, catalogItem, null, null))
.thenReturn(null);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.eclipse.tractusx.traceability.integration;

import groovy.json.JsonBuilder;
import io.restassured.RestAssured;
import org.awaitility.Awaitility;
import org.eclipse.tractusx.traceability.integration.common.config.PostgreSQLConfig;
import org.eclipse.tractusx.traceability.integration.common.config.RestAssuredConfig;
Expand All @@ -33,6 +34,7 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.web.server.LocalServerPort;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.ContextConfiguration;

Expand All @@ -56,8 +58,12 @@ public class IntegrationTestSpecification {
@Autowired
DatabaseSupport databaseSupport;

@LocalServerPort
private Integer port;

@BeforeEach
void beforeEach() throws JoseException {
RestAssured.port = port;
oAuth2ApiSupport.oauth2ApiReturnsTechnicalUserToken();
oAuth2ApiSupport.oauth2ApiReturnsJwkCerts(oAuth2Support.jwk());
}
Expand Down
Loading
Loading