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

538 propagate exceptions via either and add suppressed #737

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
1 change: 1 addition & 0 deletions .github/workflows/owasp.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
name: "OWASP dependency scanner"

on:
workflow_dispatch: # Trigger manually
push:
branches: main
paths-ignore:
Expand Down
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ _**For better traceability add the corresponding GitHub issue number in each cha
or `edc:type`: `data.core.digitalTwinRegistry`. #616
- Fix missing and malformed properties for EDC policy transformation. #648

## Fixed

- Propagates exceptions to have more detail in tombstone. #538


## [5.1.2] - 2024-05-13

### Fixed
Expand Down Expand Up @@ -91,11 +96,11 @@ _**For better traceability add the corresponding GitHub issue number in each cha
## [5.0.0] - 2024-04-16

### Added

- SAMM models can now be added locally #488
- Introduced new Cucumber Tests to cover Industry Core 2.0.0 compatibility #488



### Fixed

- Policy store API fixes. #199, #505
Expand All @@ -111,6 +116,7 @@ _**For better traceability add the corresponding GitHub issue number in each cha
- RestClientExceptions are handled correctly in BpdmFacade now. #405
- Fixed Base64 encoding and decoding for locally provided Semantic Models #488


## [4.9.0] - 2024-04-03
### Added
- Extended EdcPolicyDefinitionService to check if a policy in the edc exists
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,16 @@
********************************************************************************/
package org.eclipse.tractusx.irs.aaswrapper.job.delegate;

import java.util.Collection;
import java.util.List;
import java.util.Objects;

import io.github.resilience4j.core.functions.Either;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.eclipse.tractusx.irs.aaswrapper.job.AASTransferProcess;
import org.eclipse.tractusx.irs.aaswrapper.job.ItemContainer;
import org.eclipse.tractusx.irs.common.ExceptionUtils;
import org.eclipse.tractusx.irs.component.JobParameter;
import org.eclipse.tractusx.irs.component.PartChainIdentificationKey;
import org.eclipse.tractusx.irs.component.Shell;
Expand Down Expand Up @@ -69,10 +73,14 @@ public ItemContainer process(final ItemContainer.ItemContainerBuilder itemContai

try {
final var dtrKeys = List.of(new DigitalTwinRegistryKey(itemId.getGlobalAssetId(), itemId.getBpn()));
final Shell shell = digitalTwinRegistryService.fetchShells(dtrKeys).stream()
// we use findFirst here, because we query only for one
// DigitalTwinRegistryKey here
.findFirst().orElseThrow();
final var eithers = digitalTwinRegistryService.fetchShells(dtrKeys);
final var shell = eithers.stream()
// we use findFirst here, because we query only for one
// DigitalTwinRegistryKey here
.map(Either::getOrNull)
.filter(Objects::nonNull)
.findFirst()
.orElseThrow(() -> shellNotFound(eithers));

itemContainerBuilder.shell(
jobData.isAuditContractNegotiation() ? shell : shell.withoutContractAgreementId());
Expand All @@ -92,6 +100,12 @@ public ItemContainer process(final ItemContainer.ItemContainerBuilder itemContai
return itemContainerBuilder.build();
}

private static RegistryServiceException shellNotFound(final Collection<Either<Exception, Shell>> eithers) {
final RegistryServiceException shellNotFound = new RegistryServiceException("Shell not found");
ExceptionUtils.addSuppressedExceptions(eithers, shellNotFound);
return shellNotFound;
}

private boolean expectedDepthOfTreeIsNotReached(final int expectedDepth, final int currentDepth) {
log.info("Expected tree depth is {}, current depth is {}", expectedDepth, currentDepth);
return currentDepth < expectedDepth;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

import java.util.List;

import io.github.resilience4j.core.functions.Either;
import io.github.resilience4j.retry.RetryRegistry;
import org.eclipse.tractusx.irs.aaswrapper.job.AASTransferProcess;
import org.eclipse.tractusx.irs.aaswrapper.job.ItemContainer;
Expand All @@ -55,7 +56,7 @@ class DigitalTwinDelegateTest {
void shouldFillItemContainerWithShell() throws RegistryServiceException {
// given
when(digitalTwinRegistryService.fetchShells(any())).thenReturn(
List.of(shell("", shellDescriptor(List.of(submodelDescriptorWithoutHref("any"))))));
List.of(Either.right(shell("", shellDescriptor(List.of(submodelDescriptorWithoutHref("any")))))));

// when
final ItemContainer result = digitalTwinDelegate.process(ItemContainer.builder(), jobParameter(),
Expand All @@ -72,11 +73,11 @@ void shouldFillItemContainerWithShell() throws RegistryServiceException {
void shouldFillItemContainerWithShellAndContractAgreementIdWhenAuditFlag() throws RegistryServiceException {
// given
when(digitalTwinRegistryService.fetchShells(any())).thenReturn(
List.of(shell("", shellDescriptor(List.of(submodelDescriptorWithoutHref("any"))))));
List.of(Either.right(shell("", shellDescriptor(List.of(submodelDescriptorWithoutHref("any")))))));

// when
final ItemContainer result = digitalTwinDelegate.process(ItemContainer.builder(), jobParameterAuditContractNegotiation(),
new AASTransferProcess("id", 0), createKey());
final ItemContainer result = digitalTwinDelegate.process(ItemContainer.builder(),
jobParameterAuditContractNegotiation(), new AASTransferProcess("id", 0), createKey());

// then
assertThat(result).isNotNull();
Expand All @@ -86,10 +87,11 @@ void shouldFillItemContainerWithShellAndContractAgreementIdWhenAuditFlag() throw
}

@Test
void shouldFillItemContainerWithShellAndSubmodelDescriptorsWhenDepthReached() throws RegistryServiceException {
void shouldFillItemContainerWithShellAndSubmodelDescriptorsWhenDepthReached()
throws RegistryServiceException {
// given
when(digitalTwinRegistryService.fetchShells(any())).thenReturn(
List.of(shell("", shellDescriptor(List.of(submodelDescriptorWithoutHref("any"))))));
List.of(Either.right(shell("", shellDescriptor(List.of(submodelDescriptorWithoutHref("any")))))));
final JobParameter jobParameter = JobParameter.builder().depth(1).aspects(List.of()).build();

// when
Expand Down Expand Up @@ -132,14 +134,16 @@ void shouldCreateTombstoneIfBPNEmpty() {
assertThat(result).isNotNull();
assertThat(result.getTombstones()).hasSize(1);
assertThat(result.getTombstones().get(0).getCatenaXId()).isEqualTo("itemId");
assertThat(result.getTombstones().get(0).getProcessingError().getErrorDetail()).isEqualTo("Can't get relationship without a BPN");
assertThat(result.getTombstones().get(0).getProcessingError().getErrorDetail()).isEqualTo(
"Can't get relationship without a BPN");
assertThat(result.getTombstones().get(0).getProcessingError().getProcessStep()).isEqualTo(
ProcessStep.DIGITAL_TWIN_REQUEST);
}

private static PartChainIdentificationKey createKey() {
return PartChainIdentificationKey.builder().globalAssetId("itemId").bpn("bpn123").build();
}

private static PartChainIdentificationKey createKeyWithoutBpn() {
return PartChainIdentificationKey.builder().globalAssetId("itemId").build();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/********************************************************************************
* Copyright (c) 2022,2024 Bayerische Motoren Werke Aktiengesellschaft (BMW AG)
* Copyright (c) 2021,2024 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
*
* This program and the accompanying materials are made available under the
* terms of the Apache License, Version 2.0 which is available at
* https://www.apache.org/licenses/LICENSE-2.0.
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
* SPDX-License-Identifier: Apache-2.0
********************************************************************************/
package org.eclipse.tractusx.irs.common;

import java.util.Collection;

import io.github.resilience4j.core.functions.Either;

/**
* Utilities for exception handling
*/
public final class ExceptionUtils {

private ExceptionUtils() {
// private constructor, utility class
}

/**
* Adds all exceptions from left side of the given Eithers to the exception.
*
* @param eithers the {@link Either}s
* @param exception the exception
* @param <E> the exception type (left-hand side of {@link Either})
* @param <T> the object type (right-hand side of {@link Either})
*/
public static <E extends Exception, T> void addSuppressedExceptions(final Collection<Either<E, T>> eithers,
final Exception exception) {
for (final Either<E, T> either : eithers) {
if (either.isLeft() && either.getLeft() != null) {
exception.addSuppressed(either.getLeft());
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,19 @@ public <T> CompletableFuture<T> getFastestResult(final List<CompletableFuture<T>
log.debug("All of the futures completed");

if (ex != null) {
log.warn("All failed: " + System.lineSeparator() //
log.warn("All failed: " + System.lineSeparator()
// TODO (#538) can we remove logging here and log only up in call hierarchy
+ exceptions.stream()
.map(ExceptionUtils::getStackTrace)
.collect(Collectors.joining(System.lineSeparator())), ex);

overallFuture.completeExceptionally(new CompletionExceptions("None successful", exceptions));
final CompletionExceptions noneSuccessful = new CompletionExceptions("None successful");
for (final Throwable exception : exceptions) {
noneSuccessful.addSuppressed(exception);
}

overallFuture.completeExceptionally(noneSuccessful);

} else {
overallFuture.complete(null);
}
Expand Down Expand Up @@ -139,11 +146,8 @@ private static <T> Function<Throwable, T> collectingExceptionsAndThrow(final Lis
@ToString
public static class CompletionExceptions extends CompletionException {

private final List<Throwable> causes;

public CompletionExceptions(final String msg, final List<Throwable> causes) {
public CompletionExceptions(final String msg) {
super(msg);
this.causes = causes;
}

}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/********************************************************************************
* Copyright (c) 2022,2024 Bayerische Motoren Werke Aktiengesellschaft (BMW AG)
* Copyright (c) 2021,2024 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
*
* This program and the accompanying materials are made available under the
* terms of the Apache License, Version 2.0 which is available at
* https://www.apache.org/licenses/LICENSE-2.0.
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
* SPDX-License-Identifier: Apache-2.0
********************************************************************************/
package org.eclipse.tractusx.irs.common;

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.ArrayList;
import java.util.List;

import io.github.resilience4j.core.functions.Either;
import org.junit.jupiter.api.Test;

/**
* Test for class {@link ExceptionUtils}
*/
class ExceptionUtilsTest {

@Test
void addSuppressedExceptionsTest() {

// ARRANGE
final List<Either<Exception, String>> eithers = new ArrayList<>();
eithers.add(Either.left(new RuntimeException("Some runtime exception")));
eithers.add(Either.right("Some string"));
eithers.add(Either.left(new IllegalArgumentException("Another exception")));
eithers.add(Either.left(null));

final Exception mainException = new Exception("Main exception");

// ACT
ExceptionUtils.addSuppressedExceptions(eithers, mainException);

// ASSERT
final Throwable[] suppressedExceptions = mainException.getSuppressed();
assertEquals(2, suppressedExceptions.length); // Expecting two suppressed exceptions
assertEquals("Some runtime exception", suppressedExceptions[0].getMessage());
assertEquals("Another exception", suppressedExceptions[1].getMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.util.Arrays;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
Expand Down Expand Up @@ -106,10 +107,11 @@ void withAllCompletableFuturesFailing_itShouldThrow() {
.extracting(Throwable::getCause)
.isInstanceOf(ResultFinder.CompletionExceptions.class)
.extracting(collectedFailures -> (ResultFinder.CompletionExceptions) collectedFailures)
.extracting(ResultFinder.CompletionExceptions::getCauses)
.extracting(ResultFinder.CompletionExceptions::getSuppressed)
.describedAs("should have collected all exceptions")
.satisfies(causes -> assertThat(
causes.stream().map(Throwable::getMessage).toList()).containsExactlyInAnyOrder(
.satisfies(causes -> assertThat(Arrays.stream(causes)
.map(Throwable::getMessage)
.toList()).containsExactlyInAnyOrder(
"java.lang.RuntimeException: failing 1",
"java.lang.RuntimeException: failing 2",
"java.lang.RuntimeException: failing 3"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,16 @@ public List<CompletableFuture<EndpointDataReference>> getEndpointReferencesForAs

final String storageId = getStorageId(endpointDataReferenceStatus, negotiationResponse);

// TODO (#405) Remove this comment and code later.
dsmf marked this conversation as resolved.
Show resolved Hide resolved
// This is just for quickly simulating and experimenting with negotiation
// failed during development.
// It should later be replaced with a real automated test in
// IrsWireMockIntegrationTest that simulates this
// (this test could probably be based on shouldStartRecursiveProcesses
// but needs different setup throwing an exception during negotiation).
// if (true) {
// throw new EdcClientException("negotiation failed");
// }
return pollingService.<EndpointDataReference>createJob()
.action(() -> retrieveEndpointReference(storageId, stopWatch))
.timeToLive(config.getSubmodel().getRequestTtl())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import java.util.Collection;

import io.github.resilience4j.core.functions.Either;
import org.eclipse.tractusx.irs.component.Shell;
import org.eclipse.tractusx.irs.registryclient.exceptions.RegistryServiceException;

Expand All @@ -39,7 +40,7 @@ public interface DigitalTwinRegistryService {
* @param bpn the BPN to retrieve the shells for
* @return the collection of asset administration shells
*/
default Collection<Shell> lookupShellsByBPN(final String bpn) throws RegistryServiceException {
default Collection<Either<Exception, Shell>> lookupShellsByBPN(final String bpn) throws RegistryServiceException {
Copy link
Contributor

Choose a reason for hiding this comment

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

When changing this interface, we need to make sure to document how to use this.
This interface is used by trace-x

return fetchShells(lookupShellIdentifiers(bpn)).stream().toList();
}

Expand Down Expand Up @@ -69,6 +70,6 @@ default Collection<DigitalTwinRegistryKey> lookupShells(final String bpn) throws
* @param identifiers the shell identifiers
* @return the shell descriptors
*/
Collection<Shell> fetchShells(Collection<DigitalTwinRegistryKey> identifiers)
Collection<Either<Exception, Shell>> fetchShells(Collection<DigitalTwinRegistryKey> identifiers)
throws RegistryServiceException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.Collections;
import java.util.List;

import io.github.resilience4j.core.functions.Either;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.eclipse.tractusx.irs.component.Shell;
Expand All @@ -44,12 +45,13 @@ public class CentralDigitalTwinRegistryService implements DigitalTwinRegistrySer
private final DigitalTwinRegistryClient digitalTwinRegistryClient;

@Override
public Collection<Shell> fetchShells(final Collection<DigitalTwinRegistryKey> keys) {
public Collection<Either<Exception, Shell>> fetchShells(final Collection<DigitalTwinRegistryKey> keys) {
return keys.stream().map(key -> {
final String aaShellIdentification = getAAShellIdentificationOrGlobalAssetId(key.shellId());
log.info("Retrieved AAS Identification {} for globalAssetId {}", aaShellIdentification, key.shellId());

return new Shell("", digitalTwinRegistryClient.getAssetAdministrationShellDescriptor(aaShellIdentification));
return Either.<Exception, Shell>right(new Shell("",
digitalTwinRegistryClient.getAssetAdministrationShellDescriptor(aaShellIdentification)));
}).toList();
}

Expand Down
Loading
Loading