Bugfix: check to determine if asset can be deleted

common/util/build.gradle.kts

@@ -35,6 +35,7 @@ publishing {
         create<MavenPublication>("common-util") {
             artifactId = "common-util"
             from(components["java"])
+            artifacts.forEach { a -> println(a.file) }
         }
     }
 }

common/util/src/main/java/org/eclipse/dataspaceconnector/common/reflection/ReflectionUtil.java

@@ -33,13 +33,14 @@ public class ReflectionUtil {
     private static final String CLOSING_BRACKET = "]";
 
     /**
-     * Utility function to get value of a field from an object. For field names currently the dot notation and array indexers are supported:
+     * Utility function to get value of a field from an object. For field names currently the dot notation and array
+     * indexers are supported:
      * <pre>
      *     someObject.someValue
      *     someObject[2].someValue //someObject must impement the List interface
      * </pre>
      *
-     * @param object       The object
+     * @param object The object
      * @param propertyName The name of the field
      * @return The field's value.
      * @throws ReflectionException if the field does not exist or is not accessible
@@ -53,6 +54,9 @@ public static <T> T getFieldValue(String propertyName, Object object) {
             var field = propertyName.substring(0, dotIx);
             var rest = propertyName.substring(dotIx + 1);
             object = getFieldValue(field, object);
+            if (object == null) {
+                return null;
+            }
             return getFieldValue(rest, object);
         } else if (propertyName.matches(ARRAY_INDEXER_REGEX)) { //array indexer
             var openingBracketIx = propertyName.indexOf(OPENING_BRACKET);
@@ -82,10 +86,10 @@ public static <T> T getFieldValue(String propertyName, Object object) {
 
 
     /**
-     * Utility function to get value of a field from an object. Essentially the same as {@link ReflectionUtil#getFieldValue(String, Object)}
-     * but it does not throw an exception
+     * Utility function to get value of a field from an object. Essentially the same as
+     * {@link ReflectionUtil#getFieldValue(String, Object)} but it does not throw an exception
      *
-     * @param object       The object
+     * @param object The object
      * @param propertyName The name of the field
      * @return The field's value. Returns null if the field does not exist or is inaccessible.
      */
@@ -118,9 +122,10 @@ public static <T> Comparator<T> propertyComparator(boolean isAscending, String p
 
 
     /**
-     * Gets a field with a given name from all declared fields of a class including supertypes. Will include protected and private fields.
+     * Gets a field with a given name from all declared fields of a class including supertypes. Will include protected
+     * and private fields.
      *
-     * @param clazz     The class of the object
+     * @param clazz The class of the object
      * @param fieldName The fieldname
      * @return A field with the given name, null if the field does not exist
      */

common/util/src/test/java/org/eclipse/dataspaceconnector/common/reflection/AnotherObject.java

@@ -0,0 +1,23 @@
+/*
+ *  Copyright (c) 2020 - 2022 Microsoft Corporation
+ *
+ *  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
+ *
+ *  SPDX-License-Identifier: Apache-2.0
+ *
+ *  Contributors:
+ *       Microsoft Corporation - initial API and implementation
+ *
+ */
+
+package org.eclipse.dataspaceconnector.common.reflection;
+
+public class AnotherObject {
+    private final String anotherDescription;
+
+    public AnotherObject(String desc) {
+        anotherDescription = desc;
+    }
+}

common/util/src/test/java/org/eclipse/dataspaceconnector/common/reflection/ReflectionUtilTest.java

@@ -158,6 +158,24 @@ void getFieldRecursive_whenNotDeclared() {
         assertThat(ReflectionUtil.getFieldRecursive(to.getClass(), "notExist")).isNull();
     }
 
+    @Test
+    void getFieldValue_whenParentExist() {
+        var to = new TestObjectSubSubclass("test-desc", 1, "foobar");
+        to.setAnotherObject(new AnotherObject("another-desc"));
+
+        String fieldValue = ReflectionUtil.getFieldValue("anotherObject.anotherDescription", to);
+        assertThat(fieldValue).isEqualTo("another-desc");
+    }
+
+    @Test
+    void getFieldValue_whenParentNotExist() {
+        var to = new TestObjectSubSubclass("test-desc", 1, "foobar");
+        to.setAnotherObject(null);
+
+        String fieldValue = ReflectionUtil.getFieldValue("anotherObject.anotherDescription", to);
+        assertThat(fieldValue).isNull();
+    }
+
     @Test
     void getFieldValue_withArrayIndex() {
         var to1 = new TestObject("to1", 420);

common/util/src/test/java/org/eclipse/dataspaceconnector/common/reflection/TestObjectSubSubclass.java

@@ -16,6 +16,7 @@
 
 public class TestObjectSubSubclass extends TestObjectSubclass {
     private final String description;
+    private AnotherObject anotherObject;
 
     public TestObjectSubSubclass(String description, int priority, String testProperty) {
         super(description, priority, testProperty);
@@ -26,4 +27,12 @@ public TestObjectSubSubclass(String description, int priority, String testProper
     public String getDescription() {
         return description;
     }
+
+    public AnotherObject getAnotherObject() {
+        return anotherObject;
+    }
+
+    public void setAnotherObject(AnotherObject anotherObject) {
+        this.anotherObject = anotherObject;
+    }
 }

core/defaults/src/main/java/org/eclipse/dataspaceconnector/core/defaults/negotiationstore/InMemoryContractNegotiationStore.java

@@ -36,11 +36,11 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static java.lang.String.format;
 import static java.util.stream.Collectors.toList;
 
 /**
- * An in-memory, threadsafe process store.
- * This implementation is intended for testing purposes only.
+ * An in-memory, threadsafe process store. This implementation is intended for testing purposes only.
  */
 public class InMemoryContractNegotiationStore implements ContractNegotiationStore {
 
@@ -124,6 +124,14 @@ public void delete(String processId) {
         return lockManager.readLock(() -> agreementQueryResolver.query(getAgreements(), querySpec));
     }
 
+    @Override
+    public Stream<ContractNegotiation> getNegotiationsWithAgreementOnAsset(String assetId) {
+        var filter = format("contractAgreement.assetId = %s", assetId);
+        var query = QuerySpec.Builder.newInstance().filter(filter).build();
+
+        return queryNegotiations(query);
+    }
+
     @Override
     public @NotNull List<ContractNegotiation> nextForState(int state, int max) {
         return lockManager.readLock(() -> {

core/defaults/src/test/java/org/eclipse/dataspaceconnector/core/defaults/negotiationstore/InMemoryContractNegotiationStoreTest.java

@@ -20,6 +20,7 @@
 import org.eclipse.dataspaceconnector.spi.query.SortOrder;
 import org.eclipse.dataspaceconnector.spi.types.domain.contract.agreement.ContractAgreement;
 import org.eclipse.dataspaceconnector.spi.types.domain.contract.negotiation.ContractNegotiation;
+import org.eclipse.dataspaceconnector.spi.types.domain.contract.negotiation.ContractNegotiationStates;
 import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
@@ -28,6 +29,7 @@
 import java.util.Comparator;
 import java.util.List;
 import java.util.UUID;
+import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -320,6 +322,57 @@ void queryAgreements_verifySorting_invalidProperty() {
         assertThat(store.queryAgreements(query)).isEmpty();
     }
 
+    @Test
+    void getNegotiationsWithAgreementOnAsset_negotiationWithAgreement() {
+        var agreement = createAgreementBuilder().id("contract1").build();
+        var negotiation = createNegotiationBuilder("negotiation1").contractAgreement(agreement).build();
+        var assetId = agreement.getAssetId();
+
+        store.save(negotiation);
+
+        var result = store.getNegotiationsWithAgreementOnAsset(assetId).collect(Collectors.toList());
+
+        assertThat(result).hasSize(1).usingRecursiveFieldByFieldElementComparator().containsOnly(negotiation);
+    }
+
+    @Test
+    void getNegotiationsWithAgreementOnAsset_negotiationWithoutAgreement() {
+        var assetId = UUID.randomUUID().toString();
+        var negotiation = ContractNegotiation.Builder.newInstance()
+                .type(ContractNegotiation.Type.CONSUMER)
+                .id("negotiation1")
+                .contractAgreement(null)
+                .correlationId("corr-negotiation1")
+                .state(ContractNegotiationStates.REQUESTED.code())
+                .counterPartyAddress("consumer")
+                .counterPartyId("consumerId")
+                .protocol("ids-multipart")
+                .build();
+
+        store.save(negotiation);
+
+        var result = store.getNegotiationsWithAgreementOnAsset(assetId).collect(Collectors.toList());
+
+        assertThat(result).isEmpty();
+        assertThat(store.queryAgreements(QuerySpec.none())).isEmpty();
+    }
+
+    @Test
+    void getNegotiationsWithAgreementOnAsset_multipleNegotiationsSameAsset() {
+        var assetId = UUID.randomUUID().toString();
+        var negotiation1 = createNegotiationBuilder("negotiation1").contractAgreement(createAgreementBuilder().id("contract1").assetId(assetId).build()).build();
+        var negotiation2 = createNegotiationBuilder("negotiation2").contractAgreement(createAgreementBuilder().id("contract2").assetId(assetId).build()).build();
+
+        store.save(negotiation1);
+        store.save(negotiation2);
+
+        var result = store.getNegotiationsWithAgreementOnAsset(assetId).collect(Collectors.toList());
+
+        assertThat(result).hasSize(2)
+                .extracting(ContractNegotiation::getId).containsExactlyInAnyOrder("negotiation1", "negotiation2");
+
+    }
+
     @NotNull
     private ContractNegotiation requestingNegotiation() {
         var negotiation = createNegotiation(UUID.randomUUID().toString());

extensions/api/data-management/asset/src/main/java/org/eclipse/dataspaceconnector/api/datamanagement/asset/service/AssetServiceImpl.java

@@ -51,13 +51,23 @@ public Collection<Asset> query(QuerySpec query) {
         return transactionContext.execute(() -> index.queryAssets(query).collect(toList()));
     }
 
+    @Override
+    public ServiceResult<Asset> create(Asset asset, DataAddress dataAddress) {
+        return transactionContext.execute(() -> {
+            if (findById(asset.getId()) == null) {
+                loader.accept(asset, dataAddress);
+                return ServiceResult.success(asset);
+            } else {
+                return ServiceResult.conflict(format("Asset %s cannot be created because it already exist", asset.getId()));
+            }
+        });
+    }
+
     @Override
     public ServiceResult<Asset> delete(String assetId) {
         return transactionContext.execute(() -> {
-            var filter = format("contractAgreement.assetId = %s", assetId);
-            var query = QuerySpec.Builder.newInstance().filter(filter).build();
 
-            var negotiationsOnAsset = contractNegotiationStore.queryNegotiations(query);
+            var negotiationsOnAsset = contractNegotiationStore.getNegotiationsWithAgreementOnAsset(assetId);
             if (negotiationsOnAsset.findAny().isPresent()) {
                 return ServiceResult.conflict(format("Asset %s cannot be deleted as it is referenced by at least one contract agreement", assetId));
             }
@@ -70,16 +80,4 @@ public ServiceResult<Asset> delete(String assetId) {
             return ServiceResult.success(deleted);
         });
     }
-
-    @Override
-    public ServiceResult<Asset> create(Asset asset, DataAddress dataAddress) {
-        return transactionContext.execute(() -> {
-            if (findById(asset.getId()) == null) {
-                loader.accept(asset, dataAddress);
-                return ServiceResult.success(asset);
-            } else {
-                return ServiceResult.conflict(format("Asset %s cannot be created because it already exist", asset.getId()));
-            }
-        });
-    }
 }

extensions/api/data-management/asset/src/test/java/org/eclipse/dataspaceconnector/api/datamanagement/asset/service/AssetServiceImplTest.java

@@ -40,6 +40,7 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
 class AssetServiceImplTest {
@@ -73,15 +74,17 @@ void query_shouldRelyOnAssetIndex() {
 
     @Test
     void createAsset_shouldCreateAssetIfItDoesNotAlreadyExist() {
-        var asset = createAsset("assetId");
-        when(index.findById("assetId")).thenReturn(null);
-        var dataAddress = DataAddress.Builder.newInstance().type("addressType").build();
+        var assetId = "assetId";
+        var asset = createAsset(assetId);
+        when(index.findById(assetId)).thenReturn(null);
+        var addressType = "addressType";
+        var dataAddress = DataAddress.Builder.newInstance().type(addressType).build();
 
         var inserted = service.create(asset, dataAddress);
 
         assertThat(inserted.succeeded()).isTrue();
-        assertThat(inserted.getContent()).matches(hasId("assetId"));
-        verify(loader).accept(argThat(it -> "assetId".equals(it.getId())), argThat(it -> "addressType".equals(it.getType())));
+        assertThat(inserted.getContent()).matches(hasId(assetId));
+        verify(loader).accept(argThat(it -> assetId.equals(it.getId())), argThat(it -> addressType.equals(it.getType())));
     }
 
     @Test
@@ -120,16 +123,18 @@ void delete_shouldNotDeleteIfAssetIsAlreadyPartOfAnAgreement() {
                         .id(UUID.randomUUID().toString())
                         .providerAgentId(UUID.randomUUID().toString())
                         .consumerAgentId(UUID.randomUUID().toString())
-                        .assetId("assetId")
+                        .assetId(asset.getId())
                         .policy(Policy.Builder.newInstance().build())
                         .build())
                 .build();
-        when(contractNegotiationStore.queryNegotiations(any())).thenReturn(Stream.of(contractNegotiation));
+        when(contractNegotiationStore.getNegotiationsWithAgreementOnAsset(any())).thenReturn(Stream.of(contractNegotiation));
 
         var deleted = service.delete("assetId");
 
         assertThat(deleted.failed()).isTrue();
         assertThat(deleted.getFailure().getReason()).isEqualTo(CONFLICT);
+        verify(contractNegotiationStore).getNegotiationsWithAgreementOnAsset(any());
+        verifyNoMoreInteractions(contractNegotiationStore);
     }
 
     @Test

extensions/azure/cosmos/contract-negotiation-store-cosmos/src/main/java/org/eclipse/dataspaceconnector/contract/negotiation/store/CosmosContractNegotiationStore.java

@@ -42,6 +42,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static java.lang.String.format;
 import static java.util.Optional.ofNullable;
 import static net.jodah.failsafe.Failsafe.with;
 
@@ -162,6 +163,14 @@ public void delete(String negotiationId) {
                 .filter(Objects::nonNull);
     }
 
+    @Override
+    public Stream<ContractNegotiation> getNegotiationsWithAgreementOnAsset(String assetId) {
+        var filter = format("contractAgreement.assetId = %s", assetId);
+        var query = QuerySpec.Builder.newInstance().filter(filter).build();
+
+        return queryNegotiations(query);
+    }
+
     @Override
     public @NotNull List<ContractNegotiation> nextForState(int state, int max) {