refactor: make resource manifest generator handle TransferProcess (#3979

) * refactor: make resource manifest generator deal with transfer process * refactor: make resource manifest generator accept TransferProcess instead of DataRequest
eclipse-edc · Mar 8, 2024 · 2887028 · 2887028
1 parent 1fe0c95
commit 2887028
Show file tree

Hide file tree

Showing 10 changed files with 71 additions and 106 deletions.
diff --git a/.../src/main/java/org/eclipse/edc/connector/transfer/process/TransferProcessManagerImpl.java b/.../src/main/java/org/eclipse/edc/connector/transfer/process/TransferProcessManagerImpl.java
@@ -192,7 +192,7 @@ private boolean processInitial(TransferProcess process) {
 
         ResourceManifest manifest;
         if (process.getType() == CONSUMER) {
-            var manifestResult = manifestGenerator.generateConsumerResourceManifest(process.getDataRequest(), policy);
+            var manifestResult = manifestGenerator.generateConsumerResourceManifest(process, policy);
             if (manifestResult.failed()) {
                 transitionToTerminated(process, format("Resource manifest for process %s cannot be modified to fulfil policy. %s", process.getId(), manifestResult.getFailureMessages()));
                 return true;
@@ -214,7 +214,7 @@ private boolean processInitial(TransferProcess process) {
                 vault.storeSecret(dataDestination.getKeyName(), secret);
             }
 
-            manifest = manifestGenerator.generateProviderResourceManifest(process.getDataRequest(), dataAddress, policy);
+            manifest = manifestGenerator.generateProviderResourceManifest(process, dataAddress, policy);
         }
 
         process.transitionProvisioning(manifest);

diff --git a/...main/java/org/eclipse/edc/connector/transfer/provision/ResourceManifestGeneratorImpl.java b/...main/java/org/eclipse/edc/connector/transfer/provision/ResourceManifestGeneratorImpl.java
@@ -19,8 +19,8 @@
 import org.eclipse.edc.connector.transfer.spi.provision.ProviderResourceDefinitionGenerator;
 import org.eclipse.edc.connector.transfer.spi.provision.ResourceManifestContext;
 import org.eclipse.edc.connector.transfer.spi.provision.ResourceManifestGenerator;
-import org.eclipse.edc.connector.transfer.spi.types.DataRequest;
 import org.eclipse.edc.connector.transfer.spi.types.ResourceManifest;
+import org.eclipse.edc.connector.transfer.spi.types.TransferProcess;
 import org.eclipse.edc.policy.engine.spi.PolicyContext;
 import org.eclipse.edc.policy.engine.spi.PolicyContextImpl;
 import org.eclipse.edc.policy.engine.spi.PolicyEngine;
@@ -37,6 +37,7 @@
  * Default implementation.
  */
 public class ResourceManifestGeneratorImpl implements ResourceManifestGenerator {
+
     private final List<ConsumerResourceDefinitionGenerator> consumerGenerators = new ArrayList<>();
     private final List<ProviderResourceDefinitionGenerator> providerGenerators = new ArrayList<>();
     private final PolicyEngine policyEngine;
@@ -56,10 +57,10 @@ public void registerGenerator(ProviderResourceDefinitionGenerator generator) {
     }
 
     @Override
-    public Result<ResourceManifest> generateConsumerResourceManifest(DataRequest dataRequest, Policy policy) {
+    public Result<ResourceManifest> generateConsumerResourceManifest(TransferProcess transferProcess, Policy policy) {
         var definitions = consumerGenerators.stream()
-                .filter(generator -> generator.canGenerate(dataRequest, policy))
-                .map(generator -> generator.generate(dataRequest, policy))
+                .filter(generator -> generator.canGenerate(transferProcess, policy))
+                .map(generator -> generator.generate(transferProcess, policy))
                 .filter(Objects::nonNull).collect(Collectors.toList());
 
         var manifest = ResourceManifest.Builder.newInstance().definitions(definitions).build();
@@ -77,10 +78,10 @@ public Result<ResourceManifest> generateConsumerResourceManifest(DataRequest dat
     }
 
     @Override
-    public ResourceManifest generateProviderResourceManifest(DataRequest dataRequest, DataAddress assetAddress, Policy policy) {
+    public ResourceManifest generateProviderResourceManifest(TransferProcess transferProcess, DataAddress assetAddress, Policy policy) {
         var definitions = providerGenerators.stream()
-                .filter(generator -> generator.canGenerate(dataRequest, assetAddress, policy))
-                .map(generator -> generator.generate(dataRequest, assetAddress, policy))
+                .filter(generator -> generator.canGenerate(transferProcess, assetAddress, policy))
+                .map(generator -> generator.generate(transferProcess, assetAddress, policy))
                 .filter(Objects::nonNull).collect(Collectors.toList());
 
         return ResourceManifest.Builder.newInstance().definitions(definitions).build();

diff --git a/...org/eclipse/edc/connector/transfer/process/TransferProcessManagerImplIntegrationTest.java b/...org/eclipse/edc/connector/transfer/process/TransferProcessManagerImplIntegrationTest.java
@@ -103,7 +103,7 @@ class TransferProcessManagerImplIntegrationTest {
     @BeforeEach
     void setup() {
         var resourceManifest = ResourceManifest.Builder.newInstance().definitions(List.of(new TestResourceDefinition())).build();
-        when(manifestGenerator.generateConsumerResourceManifest(any(DataRequest.class), any(Policy.class))).thenReturn(Result.success(resourceManifest));
+        when(manifestGenerator.generateConsumerResourceManifest(any(TransferProcess.class), any(Policy.class))).thenReturn(Result.success(resourceManifest));
 
         var policyArchive = mock(PolicyArchive.class);
         when(policyArchive.findPolicyForContract(anyString())).thenReturn(Policy.Builder.newInstance().build());

diff --git a/.../test/java/org/eclipse/edc/connector/transfer/process/TransferProcessManagerImplTest.java b/.../test/java/org/eclipse/edc/connector/transfer/process/TransferProcessManagerImplTest.java
@@ -203,7 +203,7 @@ void initial_consumer_shouldTransitionToProvisioning() {
                 .thenReturn(List.of(transferProcess))
                 .thenReturn(emptyList());
         var resourceManifest = ResourceManifest.Builder.newInstance().definitions(List.of(new TestResourceDefinition())).build();
-        when(manifestGenerator.generateConsumerResourceManifest(any(DataRequest.class), any(Policy.class)))
+        when(manifestGenerator.generateConsumerResourceManifest(any(TransferProcess.class), any(Policy.class)))
                 .thenReturn(Result.success(resourceManifest));
 
         manager.start();
@@ -222,7 +222,7 @@ void initial_consumer_manifestEvaluationFailed_shouldTransitionToTerminated() {
         when(transferProcessStore.nextNotLeased(anyInt(), stateIs(INITIAL.code())))
                 .thenReturn(List.of(transferProcess))
                 .thenReturn(emptyList());
-        when(manifestGenerator.generateConsumerResourceManifest(any(DataRequest.class), any(Policy.class)))
+        when(manifestGenerator.generateConsumerResourceManifest(any(TransferProcess.class), any(Policy.class)))
                 .thenReturn(Result.failure("error"));
 
         manager.start();
@@ -259,7 +259,7 @@ void initial_provider_shouldTransitionToProvisioning() {
         var contentDataAddress = DataAddress.Builder.newInstance().type("type").build();
         when(addressResolver.resolveForAsset(any())).thenReturn(contentDataAddress);
         var resourceManifest = ResourceManifest.Builder.newInstance().definitions(List.of(new TestResourceDefinition())).build();
-        when(manifestGenerator.generateProviderResourceManifest(any(DataRequest.class), any(), any()))
+        when(manifestGenerator.generateProviderResourceManifest(any(TransferProcess.class), any(), any()))
                 .thenReturn(resourceManifest);
 
         manager.start();
@@ -290,7 +290,7 @@ void initial_provider_shouldStoreSecret_whenItIsFoundInTheDataAddress() {
         when(transferProcessStore.nextNotLeased(anyInt(), stateIs(INITIAL.code()))).thenReturn(List.of(transferProcess)).thenReturn(emptyList());
         when(addressResolver.resolveForAsset(any())).thenReturn(DataAddress.Builder.newInstance().type("type").build());
         var resourceManifest = ResourceManifest.Builder.newInstance().definitions(List.of(new TestResourceDefinition())).build();
-        when(manifestGenerator.generateProviderResourceManifest(any(DataRequest.class), any(), any()))
+        when(manifestGenerator.generateProviderResourceManifest(any(TransferProcess.class), any(), any()))
                 .thenReturn(resourceManifest);
 
         manager.start();

diff --git a/.../java/org/eclipse/edc/connector/transfer/provision/ResourceManifestGeneratorImplTest.java b/.../java/org/eclipse/edc/connector/transfer/provision/ResourceManifestGeneratorImplTest.java
@@ -18,6 +18,7 @@
 import org.eclipse.edc.connector.transfer.spi.provision.ConsumerResourceDefinitionGenerator;
 import org.eclipse.edc.connector.transfer.spi.provision.ProviderResourceDefinitionGenerator;
 import org.eclipse.edc.connector.transfer.spi.types.DataRequest;
+import org.eclipse.edc.connector.transfer.spi.types.TransferProcess;
 import org.eclipse.edc.policy.engine.spi.PolicyContext;
 import org.eclipse.edc.policy.engine.spi.PolicyEngine;
 import org.eclipse.edc.policy.model.Policy;
@@ -40,28 +41,25 @@ class ResourceManifestGeneratorImplTest {
     private final ConsumerResourceDefinitionGenerator consumerGenerator = mock();
     private final ProviderResourceDefinitionGenerator providerGenerator = mock();
     private final PolicyEngine policyEngine = mock();
-    private ResourceManifestGeneratorImpl generator;
-    private Policy policy;
-    private DataAddress dataAddress;
+    private final ResourceManifestGeneratorImpl generator = new ResourceManifestGeneratorImpl(policyEngine);
+    private final Policy policy = Policy.Builder.newInstance().build();
+    private final DataAddress dataAddress = DataAddress.Builder.newInstance().type("test").build();
 
     @BeforeEach
     void setUp() {
-        generator = new ResourceManifestGeneratorImpl(policyEngine);
         generator.registerGenerator(consumerGenerator);
         generator.registerGenerator(providerGenerator);
-        policy = Policy.Builder.newInstance().build();
-        dataAddress = DataAddress.Builder.newInstance().type("test").build();
     }
 
     @Test
     void shouldGenerateResourceManifestForConsumerManagedTransferProcess() {
-        var dataRequest = createDataRequest();
+        var transferProcess = TransferProcess.Builder.newInstance().dataRequest(createDataRequest()).build();
         var resourceDefinition = TestResourceDefinition.Builder.newInstance().id(UUID.randomUUID().toString()).build();
         when(consumerGenerator.canGenerate(any(), any())).thenReturn(true);
         when(consumerGenerator.generate(any(), any())).thenReturn(resourceDefinition);
         when(policyEngine.evaluate(any(), any(), isA(PolicyContext.class))).thenReturn(Result.success());
 
-        var result = generator.generateConsumerResourceManifest(dataRequest, policy);
+        var result = generator.generateConsumerResourceManifest(transferProcess, policy);
 
         assertThat(result.succeeded()).isTrue();
         assertThat(result.getContent().getDefinitions()).hasSize(1).containsExactly(resourceDefinition);
@@ -70,47 +68,47 @@ void shouldGenerateResourceManifestForConsumerManagedTransferProcess() {
 
     @Test
     void shouldGenerateEmptyResourceManifestForNotGeneratedFilter() {
-        var dataRequest = createDataRequest();
+        var transferProcess = TransferProcess.Builder.newInstance().dataRequest(createDataRequest()).build();
         when(consumerGenerator.canGenerate(any(), any())).thenReturn(false);
         when(policyEngine.evaluate(any(), any(), isA(PolicyContext.class))).thenReturn(Result.success());
 
-        var result = generator.generateConsumerResourceManifest(dataRequest, policy);
+        var result = generator.generateConsumerResourceManifest(transferProcess, policy);
 
         assertThat(result.getContent().getDefinitions()).hasSize(0);
         verifyNoInteractions(providerGenerator);
     }
 
     @Test
     void shouldReturnFailedResultForConsumerWhenPolicyEvaluationFailed() {
-        var dataRequest = createDataRequest();
+        var transferProcess = TransferProcess.Builder.newInstance().dataRequest(createDataRequest()).build();
         var resourceDefinition = TestResourceDefinition.Builder.newInstance().id(UUID.randomUUID().toString()).build();
         when(consumerGenerator.generate(any(), any())).thenReturn(resourceDefinition);
         when(policyEngine.evaluate(any(), any(), isA(PolicyContext.class))).thenReturn(Result.failure("error"));
 
-        var result = generator.generateConsumerResourceManifest(dataRequest, policy);
+        var result = generator.generateConsumerResourceManifest(transferProcess, policy);
 
         assertThat(result.succeeded()).isFalse();
     }
 
     @Test
     void shouldGenerateResourceManifestForProviderTransferProcess() {
-        var process = createDataRequest();
+        var transferProcess = TransferProcess.Builder.newInstance().dataRequest(createDataRequest()).build();
         var resourceDefinition = TestResourceDefinition.Builder.newInstance().id(UUID.randomUUID().toString()).build();
         when(providerGenerator.canGenerate(any(), any(), any())).thenReturn(true);
         when(providerGenerator.generate(any(), any(), any())).thenReturn(resourceDefinition);
 
-        var resourceManifest = generator.generateProviderResourceManifest(process, dataAddress, policy);
+        var resourceManifest = generator.generateProviderResourceManifest(transferProcess, dataAddress, policy);
 
         assertThat(resourceManifest.getDefinitions()).hasSize(1).containsExactly(resourceDefinition);
         verifyNoInteractions(consumerGenerator);
     }
 
     @Test
     void shouldGenerateEmptyResourceManifestForProviderTransferProcess() {
-        var process = createDataRequest();
+        var transferProcess = TransferProcess.Builder.newInstance().dataRequest(createDataRequest()).build();
         when(providerGenerator.canGenerate(any(), any(), any())).thenReturn(false);
 
-        var resourceManifest = generator.generateProviderResourceManifest(process, dataAddress, policy);
+        var resourceManifest = generator.generateProviderResourceManifest(transferProcess, dataAddress, policy);
 
         assertThat(resourceManifest.getDefinitions()).hasSize(0);
         verifyNoInteractions(consumerGenerator);

diff --git a/...rg/eclipse/edc/connector/provision/http/impl/HttpProviderResourceDefinitionGenerator.java b/...rg/eclipse/edc/connector/provision/http/impl/HttpProviderResourceDefinitionGenerator.java
@@ -17,10 +17,9 @@
 package org.eclipse.edc.connector.provision.http.impl;
 
 import org.eclipse.edc.connector.transfer.spi.provision.ProviderResourceDefinitionGenerator;
-import org.eclipse.edc.connector.transfer.spi.types.DataRequest;
 import org.eclipse.edc.connector.transfer.spi.types.ResourceDefinition;
+import org.eclipse.edc.connector.transfer.spi.types.TransferProcess;
 import org.eclipse.edc.policy.model.Policy;
-import org.eclipse.edc.spi.EdcException;
 import org.eclipse.edc.spi.types.domain.DataAddress;
 import org.jetbrains.annotations.Nullable;
 
@@ -39,23 +38,17 @@ public HttpProviderResourceDefinitionGenerator(String dataAddressType) {
     }
 
     @Override
-    public @Nullable ResourceDefinition generate(DataRequest dataRequest, DataAddress assetAddress, Policy policy) {
-        var assetId = dataRequest.getAssetId();
-
-        if (assetId == null) {
-            // programming error
-            throw new EdcException("Asset id was null for request: " + dataRequest.getId());
-        }
+    public @Nullable ResourceDefinition generate(TransferProcess transferProcess, DataAddress assetAddress, Policy policy) {
         return HttpProviderResourceDefinition.Builder.newInstance()
                 .id(UUID.randomUUID().toString())
                 .dataAddressType(dataAddressType)
-                .transferProcessId(dataRequest.getProcessId())
-                .assetId(assetId)
+                .transferProcessId(transferProcess.getId())
+                .assetId(transferProcess.getAssetId())
                 .build();
     }
 
     @Override
-    public boolean canGenerate(DataRequest dataRequest, DataAddress assetAddress, Policy policy) {
+    public boolean canGenerate(TransferProcess transferProcess, DataAddress assetAddress, Policy policy) {
         return dataAddressType.equals(assetAddress.getType());
     }
 }
diff --git a/...clipse/edc/connector/provision/http/impl/HttpProviderResourceDefinitionGeneratorTest.java b/...clipse/edc/connector/provision/http/impl/HttpProviderResourceDefinitionGeneratorTest.java
@@ -18,34 +18,27 @@
 
 
 import org.eclipse.edc.connector.transfer.spi.types.DataRequest;
+import org.eclipse.edc.connector.transfer.spi.types.TransferProcess;
 import org.eclipse.edc.policy.model.Policy;
-import org.eclipse.edc.spi.EdcException;
 import org.eclipse.edc.spi.types.domain.DataAddress;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
 class HttpProviderResourceDefinitionGeneratorTest {
-    private static final String DATA_ADDRESS_TYPE = "test-address";
-
-    private HttpProviderResourceDefinitionGenerator generator;
 
-    @BeforeEach
-    void setUp() {
-        generator = new HttpProviderResourceDefinitionGenerator(DATA_ADDRESS_TYPE);
-    }
+    private static final String DATA_ADDRESS_TYPE = "test-address";
 
+    private final HttpProviderResourceDefinitionGenerator generator = new HttpProviderResourceDefinitionGenerator(DATA_ADDRESS_TYPE);
 
     @Test
     void generate() {
         var dataRequest = DataRequest.Builder.newInstance().destinationType("destination").assetId("asset-id").processId("process-id").build();
+        var transferProcess = TransferProcess.Builder.newInstance().id("process-id").dataRequest(dataRequest).build();
         var policy = Policy.Builder.newInstance().build();
-
         var assetAddress1 = DataAddress.Builder.newInstance().type(DATA_ADDRESS_TYPE).build();
 
-        var definition = generator.generate(dataRequest, assetAddress1, policy);
+        var definition = generator.generate(transferProcess, assetAddress1, policy);
 
         assertThat(definition).isInstanceOf(HttpProviderResourceDefinition.class);
         var objectDef = (HttpProviderResourceDefinition) definition;
@@ -54,46 +47,27 @@ void generate() {
         assertThat(objectDef.getAssetId()).isEqualTo("asset-id");
     }
 
-    @Test
-    void generate_noDataRequestAsParameter() {
-        var assetAddress1 = DataAddress.Builder.newInstance().type(DATA_ADDRESS_TYPE).build();
-        var policy = Policy.Builder.newInstance().build();
-
-        assertThatExceptionOfType(NullPointerException.class).isThrownBy(() -> generator.generate(null, assetAddress1, policy));
-    }
-
-    @Test
-    void generate_assetIdIsNull() {
-        var dataRequest = DataRequest.Builder.newInstance().destinationType("destination").processId("process-id").build();
-        var policy = Policy.Builder.newInstance().build();
-
-        var assetAddress1 = DataAddress.Builder.newInstance().type(DATA_ADDRESS_TYPE).build();
-
-        assertThatExceptionOfType(EdcException.class).isThrownBy(() -> generator.generate(dataRequest, assetAddress1, policy));
-
-    }
-
     @Test
     void canGenerate() {
-
         var dataRequest = DataRequest.Builder.newInstance().destinationType("destination").assetId("asset-id").processId("process-id").build();
+        var transferProcess = TransferProcess.Builder.newInstance().dataRequest(dataRequest).build();
         var policy = Policy.Builder.newInstance().build();
-
         var assetAddress1 = DataAddress.Builder.newInstance().type(DATA_ADDRESS_TYPE).build();
 
-        var definition = generator.canGenerate(dataRequest, assetAddress1, policy);
+        var definition = generator.canGenerate(transferProcess, assetAddress1, policy);
+
         assertThat(definition).isTrue();
     }
 
     @Test
     void canGenerate_dataAddressTypeDifferentThanAssetAddressType() {
-
         var dataRequest = DataRequest.Builder.newInstance().destinationType("destination").assetId("asset-id").processId("process-id").build();
+        var transferProcess = TransferProcess.Builder.newInstance().dataRequest(dataRequest).build();
         var policy = Policy.Builder.newInstance().build();
-
         var assetAddress1 = DataAddress.Builder.newInstance().type("a-different-addressType").build();
 
-        var definition = generator.canGenerate(dataRequest, assetAddress1, policy);
+        var definition = generator.canGenerate(transferProcess, assetAddress1, policy);
+
         assertThat(definition).isFalse();
     }