refactor: put DataAddress as Asset field

eclipse-edc · Jun 29, 2023 · 86e62ee · 86e62ee
1 parent 33a22bc
commit 86e62ee
Show file tree

Hide file tree

Showing 42 changed files with 2,231 additions and 750 deletions.
diff --git a/...e/src/test/java/org/eclipse/edc/connector/catalog/DatasetResolverImplIntegrationTest.java b/...e/src/test/java/org/eclipse/edc/connector/catalog/DatasetResolverImplIntegrationTest.java
@@ -29,7 +29,6 @@
 import org.eclipse.edc.spi.query.QuerySpec;
 import org.eclipse.edc.spi.types.domain.DataAddress;
 import org.eclipse.edc.spi.types.domain.asset.Asset;
-import org.eclipse.edc.spi.types.domain.asset.AssetEntry;
 import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -77,9 +76,9 @@ void shouldLimitResult_withHeterogenousChunks() {
         var assets2 = range(24, 113).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());
         var assets3 = range(113, 178).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());
 
-        store(assets1);
-        store(assets2);
-        store(assets3);
+        assets1.forEach(assetIndex::create);
+        assets2.forEach(assetIndex::create);
+        assets3.forEach(assetIndex::create);
 
         var def1 = getContractDefBuilder("def1").assetsSelector(selectorFrom(assets1)).build();
         var def2 = getContractDefBuilder("def2").assetsSelector(selectorFrom(assets2)).build();
@@ -104,8 +103,8 @@ void should_return_offers_subset_when_across_multiple_contract_definitions(int f
         var maximumRange = max(0, (assets1.size() + assets2.size()) - from);
         var requestedRange = to - from;
 
-        store(assets1);
-        store(assets2);
+        assets1.forEach(assetIndex::create);
+        assets2.forEach(assetIndex::create);
 
         var contractDefinition1 = getContractDefBuilder("contract-definition-")
                 .assetsSelector(selectorFrom(assets1)).build();
@@ -125,8 +124,8 @@ void shouldLimitResult_insufficientAssets() {
         var assets1 = range(0, 12).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());
         var assets2 = range(12, 18).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());
 
-        store(assets1);
-        store(assets2);
+        assets1.forEach(assetIndex::create);
+        assets2.forEach(assetIndex::create);
 
         var def1 = getContractDefBuilder("def1").assetsSelector(selectorFrom(assets1)).build();
         var def2 = getContractDefBuilder("def2").assetsSelector(selectorFrom(assets2)).build();
@@ -162,11 +161,6 @@ private ParticipantAgent createAgent() {
         return new ParticipantAgent(emptyMap(), emptyMap());
     }
 
-    private void store(Collection<Asset> assets) {
-        assets.stream().map(a -> new AssetEntry(a, DataAddress.Builder.newInstance().type("test-type").build()))
-                .forEach(assetIndex::create);
-    }
-
     private List<Criterion> selectorFrom(Collection<Asset> assets1) {
         var ids = assets1.stream().map(Asset::getId).collect(Collectors.toList());
         return List.of(new Criterion(Asset.PROPERTY_ID, "in", ids));
@@ -181,7 +175,10 @@ private ContractDefinition.Builder getContractDefBuilder(String id) {
     }
 
     private Asset.Builder createAsset(String id) {
-        return Asset.Builder.newInstance().id(id).name("test asset " + id);
+        return Asset.Builder.newInstance()
+                .id(id)
+                .name("test asset " + id)
+                .dataAddress(DataAddress.Builder.newInstance().type("test-type").build());
     }
 
     static class RangeProvider implements ArgumentsProvider {

diff --git a/...e/src/test/java/org/eclipse/edc/connector/catalog/DatasetResolverImplPerformanceTest.java b/...e/src/test/java/org/eclipse/edc/connector/catalog/DatasetResolverImplPerformanceTest.java
@@ -29,7 +29,6 @@
 import org.eclipse.edc.spi.query.QuerySpec;
 import org.eclipse.edc.spi.types.domain.DataAddress;
 import org.eclipse.edc.spi.types.domain.asset.Asset;
-import org.eclipse.edc.spi.types.domain.asset.AssetEntry;
 import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -70,7 +69,7 @@ void oneAssetPerDefinition(DatasetResolver datasetResolver, ContractDefinitionSt
                 .contractPolicyId("policy")
                 .assetsSelectorCriterion(criterion(Asset.PROPERTY_ID, "=", String.valueOf(i))).build()
                 ).forEach(contractDefinitionStore::save);
-        range(0, 10000).mapToObj(i -> createAsset(String.valueOf(i)).build()).map(this::createAssetEntry).forEach(assetIndex::create);
+        range(0, 10000).mapToObj(i -> createAsset(String.valueOf(i)).build()).forEach(assetIndex::create);
 
         var firstPageQuery = QuerySpec.Builder.newInstance().offset(0).limit(100).build();
         var firstPageDatasets = queryDatasetsIn(datasetResolver, firstPageQuery, ofSeconds(1));
@@ -87,7 +86,7 @@ void oneAssetPerDefinition(DatasetResolver datasetResolver, ContractDefinitionSt
     void fewDefinitionsSelectAllAssets(DatasetResolver datasetResolver, ContractDefinitionStore contractDefinitionStore, AssetIndex assetIndex, PolicyDefinitionStore policyDefinitionStore) {
         policyDefinitionStore.create(createPolicyDefinition("policy").build());
         range(0, 10).mapToObj(i -> createContractDefinition(String.valueOf(i)).accessPolicyId("policy").contractPolicyId("policy").build()).forEach(contractDefinitionStore::save);
-        range(0, 10000).mapToObj(i -> createAsset(String.valueOf(i)).build()).map(this::createAssetEntry).forEach(assetIndex::create);
+        range(0, 10000).mapToObj(i -> createAsset(String.valueOf(i)).build()).forEach(assetIndex::create);
 
         var firstPageQuery = QuerySpec.Builder.newInstance().offset(0).limit(100).build();
         var firstPageDatasets = queryDatasetsIn(datasetResolver, firstPageQuery, ofSeconds(1));
@@ -116,12 +115,10 @@ private ContractDefinition.Builder createContractDefinition(String id) {
                 .contractPolicyId("contract");
     }
 
-    @NotNull
-    private AssetEntry createAssetEntry(Asset it) {
-        return new AssetEntry(it, DataAddress.Builder.newInstance().type("type").build());
-    }
-
     private Asset.Builder createAsset(String id) {
-        return Asset.Builder.newInstance().id(id).name("test asset " + id);
+        return Asset.Builder.newInstance()
+                .id(id)
+                .name("test asset " + id)
+                .dataAddress(DataAddress.Builder.newInstance().type("type").build());
     }
 }
diff --git a/...va/org/eclipse/edc/connector/contract/negotiation/ContractNegotiationIntegrationTest.java b/...va/org/eclipse/edc/connector/contract/negotiation/ContractNegotiationIntegrationTest.java
@@ -35,6 +35,7 @@
 import org.eclipse.edc.connector.policy.spi.store.PolicyDefinitionStore;
 import org.eclipse.edc.connector.service.contractnegotiation.ContractNegotiationProtocolServiceImpl;
 import org.eclipse.edc.connector.spi.contractnegotiation.ContractNegotiationProtocolService;
+import org.eclipse.edc.junit.annotations.ComponentTest;
 import org.eclipse.edc.policy.model.Action;
 import org.eclipse.edc.policy.model.Duty;
 import org.eclipse.edc.policy.model.Policy;
@@ -76,7 +77,7 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-//@ComponentTest
+@ComponentTest
 class ContractNegotiationIntegrationTest {
     private static final String CONSUMER_ID = "consumer";
     private static final String PROVIDER_ID = "provider";

diff --git a/...va/org/eclipse/edc/connector/contract/offer/ContractOfferResolverImplIntegrationTest.java b/...va/org/eclipse/edc/connector/contract/offer/ContractOfferResolverImplIntegrationTest.java
@@ -32,7 +32,6 @@
 import org.eclipse.edc.spi.query.Criterion;
 import org.eclipse.edc.spi.types.domain.DataAddress;
 import org.eclipse.edc.spi.types.domain.asset.Asset;
-import org.eclipse.edc.spi.types.domain.asset.AssetEntry;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtensionContext;
@@ -90,9 +89,9 @@ void shouldLimitResult_withHeterogenousChunks() {
         var assets2 = range(24, 113).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());
         var assets3 = range(113, 178).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());
 
-        store(assets1);
-        store(assets2);
-        store(assets3);
+        assets1.forEach(assetIndex::create);
+        assets2.forEach(assetIndex::create);
+        assets3.forEach(assetIndex::create);
 
         var def1 = getContractDefBuilder("def1").assetsSelector(selectorFrom(assets1)).build();
         var def2 = getContractDefBuilder("def2").assetsSelector(selectorFrom(assets2)).build();
@@ -123,8 +122,8 @@ void should_return_offers_subset_when_across_multiple_contract_definitions(int f
         var maximumRange = max(0, (assets1.size() + assets2.size()) - from);
         var requestedRange = to - from;
 
-        store(assets1);
-        store(assets2);
+        assets1.forEach(assetIndex::create);
+        assets2.forEach(assetIndex::create);
 
         var contractDefinition1 = getContractDefBuilder("contract-definition-")
                 .assetsSelector(selectorFrom(assets1)).build();
@@ -144,8 +143,8 @@ void shouldLimitResult_insufficientAssets() {
         var assets1 = range(0, 12).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());
         var assets2 = range(12, 18).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());
 
-        store(assets1);
-        store(assets2);
+        assets1.forEach(assetIndex::create);
+        assets2.forEach(assetIndex::create);
 
         var def1 = getContractDefBuilder("def1").assetsSelector(selectorFrom(assets1)).build();
         var def2 = getContractDefBuilder("def2").assetsSelector(selectorFrom(assets2)).build();
@@ -186,11 +185,6 @@ void shouldLimitResult_pageOffsetLargerThanNumAssets() {
         verify(policyStore, never()).findById("contract");
     }
 
-    private void store(Collection<Asset> assets) {
-        assets.stream().map(a -> new AssetEntry(a, DataAddress.Builder.newInstance().type("test-type").build()))
-                .forEach(assetIndex::create);
-    }
-
     private List<Criterion> selectorFrom(Collection<Asset> assets1) {
         var ids = assets1.stream().map(Asset::getId).toList();
         return List.of(new Criterion(Asset.PROPERTY_ID, "in", ids));
@@ -204,7 +198,10 @@ private ContractDefinition.Builder getContractDefBuilder(String id) {
     }
 
     private Asset.Builder createAsset(String id) {
-        return Asset.Builder.newInstance().id(id).name("test asset " + id);
+        return Asset.Builder.newInstance()
+                .id(id)
+                .name("test asset " + id)
+                .dataAddress(DataAddress.Builder.newInstance().type("test-type").build());
     }
 
     static class RangeProvider implements ArgumentsProvider {

diff --git a/...gate-services/src/main/java/org/eclipse/edc/connector/service/asset/AssetServiceImpl.java b/...gate-services/src/main/java/org/eclipse/edc/connector/service/asset/AssetServiceImpl.java
@@ -70,13 +70,18 @@ public ServiceResult<Stream<Asset>> query(QuerySpec query) {
 
     @Override
     public ServiceResult<Asset> create(Asset asset, DataAddress dataAddress) {
-        var validDataAddress = dataAddressValidator.validate(dataAddress);
+        return create(asset.toBuilder().dataAddress(dataAddress).build());
+    }
+
+    @Override
+    public ServiceResult<Asset> create(Asset asset) {
+        var validDataAddress = dataAddressValidator.validate(asset.getDataAddress());
         if (validDataAddress.failed()) {
             return ServiceResult.badRequest(validDataAddress.getFailureMessages());
         }
 
         return transactionContext.execute(() -> {
-            var createResult = index.create(asset, dataAddress);
+            var createResult = index.create(asset);
             if (createResult.succeeded()) {
                 observable.invokeForEach(l -> l.created(asset));
                 return ServiceResult.success(asset);

diff --git a/...-services/src/test/java/org/eclipse/edc/connector/service/asset/AssetServiceImplTest.java b/...-services/src/test/java/org/eclipse/edc/connector/service/asset/AssetServiceImplTest.java
@@ -19,7 +19,9 @@
 import org.eclipse.edc.connector.contract.spi.negotiation.store.ContractNegotiationStore;
 import org.eclipse.edc.connector.contract.spi.types.agreement.ContractAgreement;
 import org.eclipse.edc.connector.contract.spi.types.negotiation.ContractNegotiation;
+import org.eclipse.edc.connector.spi.asset.AssetService;
 import org.eclipse.edc.policy.model.Policy;
+import org.eclipse.edc.service.spi.result.ServiceFailure;
 import org.eclipse.edc.service.spi.result.ServiceResult;
 import org.eclipse.edc.spi.asset.AssetIndex;
 import org.eclipse.edc.spi.dataaddress.DataAddressValidator;
@@ -53,9 +55,11 @@
 import static org.eclipse.edc.service.spi.result.ServiceFailure.Reason.NOT_FOUND;
 import static org.eclipse.edc.spi.query.Criterion.criterion;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
+import static org.mockito.AdditionalMatchers.and;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
@@ -72,7 +76,7 @@ class AssetServiceImplTest {
     private final AssetObservable observable = mock(AssetObservable.class);
     private final DataAddressValidator dataAddressValidator = mock(DataAddressValidator.class);
 
-    private final AssetServiceImpl service = new AssetServiceImpl(index, contractNegotiationStore, dummyTransactionContext,
+    private final AssetService service = new AssetServiceImpl(index, contractNegotiationStore, dummyTransactionContext,
             observable, dataAddressValidator);
 
     @Test
@@ -129,37 +133,77 @@ void query_invalidFilter(Criterion filter) {
 
     @Test
     void createAsset_shouldCreateAssetIfItDoesNotAlreadyExist() {
+        when(dataAddressValidator.validate(any())).thenReturn(Result.success());
+        var assetId = "assetId";
+        var asset = createAsset(assetId);
+        when(index.create(asset)).thenReturn(StoreResult.success());
+
+        var inserted = service.create(asset);
+
+        assertThat(inserted.succeeded()).isTrue();
+        assertThat(inserted.getContent()).matches(hasId(assetId));
+        verify(index).create(and(isA(Asset.class), argThat(it -> assetId.equals(it.getId()))));
+        verifyNoMoreInteractions(index);
+        verify(observable).invokeForEach(any());
+    }
+
+    @Test
+    void createAsset_shouldNotCreateAssetIfItAlreadyExists() {
+        when(dataAddressValidator.validate(any())).thenReturn(Result.success());
+        var asset = createAsset("assetId");
+        when(index.create(asset)).thenReturn(StoreResult.alreadyExists("test"));
+
+        var inserted = service.create(asset);
+
+        assertThat(inserted).isFailed().extracting(ServiceFailure::getReason).isEqualTo(CONFLICT);
+    }
+
+    @Test
+    void createAsset_shouldNotCreateAssetIfDataAddressInvalid() {
+        var asset = createAsset("assetId");
+        when(dataAddressValidator.validate(any())).thenReturn(Result.failure("Data address is invalid"));
+
+        var result = service.create(asset);
+
+        Assertions.assertThat(result).satisfies(ServiceResult::failed)
+                .extracting(ServiceResult::reason)
+                .isEqualTo(BAD_REQUEST);
+        verifyNoInteractions(index);
+    }
+
+    @Test
+    @Deprecated(since = "0.1.2")
+    void createAssetDeprecated_shouldCreateAssetIfItDoesNotAlreadyExist() {
         when(dataAddressValidator.validate(any())).thenReturn(Result.success());
         var assetId = "assetId";
         var asset = createAsset(assetId);
         var addressType = "addressType";
         var dataAddress = DataAddress.Builder.newInstance().type(addressType).build();
-        when(index.create(asset, dataAddress)).thenReturn(StoreResult.success());
+        when(index.create(isA(Asset.class))).thenReturn(StoreResult.success());
 
         var inserted = service.create(asset, dataAddress);
 
         assertThat(inserted.succeeded()).isTrue();
         assertThat(inserted.getContent()).matches(hasId(assetId));
-        verify(index).create(argThat(it -> assetId.equals(it.getId())), argThat(it -> addressType.equals(it.getType())));
-        verifyNoMoreInteractions(index);
         verify(observable).invokeForEach(any());
     }
 
     @Test
-    void createAsset_shouldNotCreateAssetIfItAlreadyExists() {
+    @Deprecated(since = "0.1.2")
+    void createAssetDeprecated_shouldNotCreateAssetIfItAlreadyExists() {
         when(dataAddressValidator.validate(any())).thenReturn(Result.success());
         var asset = createAsset("assetId");
         var dataAddress = DataAddress.Builder.newInstance().type("addressType").build();
-        when(index.create(asset, dataAddress)).thenReturn(StoreResult.alreadyExists("test"));
+        when(index.create(isA(Asset.class))).thenReturn(StoreResult.alreadyExists("test"));
 
         var inserted = service.create(asset, dataAddress);
 
-        assertThat(inserted.succeeded()).isFalse();
-        assertThat(inserted.reason()).isEqualTo(CONFLICT);
+        assertThat(inserted).isFailed().extracting(ServiceFailure::getReason).isEqualTo(CONFLICT);
     }
 
     @Test
-    void createAsset_shouldNotCreateAssetIfDataAddressInvalid() {
+    @Deprecated(since = "0.1.2")
+    void createAssetDeprecated_shouldNotCreateAssetIfDataAddressInvalid() {
         var asset = createAsset("assetId");
         var dataAddress = DataAddress.Builder.newInstance().type("addressType").build();
         when(dataAddressValidator.validate(any())).thenReturn(Result.failure("Data address is invalid"));
@@ -307,6 +351,6 @@ private Predicate<Asset> hasId(String assetId) {
     }
 
     private Asset createAsset(String assetId) {
-        return Asset.Builder.newInstance().id(assetId).build();
+        return Asset.Builder.newInstance().id(assetId).dataAddress(DataAddress.Builder.newInstance().type("any").build()).build();
     }
 }
diff --git a/...lipse/edc/connector/service/contractnegotiation/ContractNegotiationEventDispatchTest.java b/...lipse/edc/connector/service/contractnegotiation/ContractNegotiationEventDispatchTest.java
@@ -98,7 +98,7 @@ void shouldDispatchEventsOnProviderContractNegotiationStateChanges(EventRouter e
                 .build();
         contractDefinitionStore.save(contractDefinition);
         policyDefinitionStore.create(PolicyDefinition.Builder.newInstance().id("policyId").policy(policy).build());
-        assetIndex.create(Asset.Builder.newInstance().id("assetId").build(), DataAddress.Builder.newInstance().type("any").build());
+        assetIndex.create(Asset.Builder.newInstance().id("assetId").dataAddress(DataAddress.Builder.newInstance().type("any").build()).build());
 
         service.notifyRequested(createContractOfferRequest(policy, "assetId"), token);