fix: enforce contract negotiation request and transfer request consis…

…tency
bscholtes1A · Jun 11, 2024 · 770b99c · 770b99c
1 parent 6034040
commit 770b99c
Show file tree

Hide file tree

Showing 18 changed files with 162 additions and 75 deletions.
diff --git a/...controlplane/services/contractnegotiation/ContractNegotiationProtocolServiceImplTest.java b/...controlplane/services/contractnegotiation/ContractNegotiationProtocolServiceImplTest.java
@@ -437,10 +437,11 @@ private interface MethodCall<M extends RemoteMessage> {
 
     interface TestFunctions {
         static ContractOffer contractOffer() {
+            var assetId = "test-asset-id";
             return ContractOffer.Builder.newInstance()
-                    .id(ContractOfferId.create("1", "test-asset-id").toString())
+                    .id(ContractOfferId.create("1", assetId).toString())
                     .policy(createPolicy())
-                    .assetId("assetId")
+                    .assetId(assetId)
                     .build();
         }
 

diff --git a/...nnector/controlplane/services/contractnegotiation/ContractNegotiationServiceImplTest.java b/...nnector/controlplane/services/contractnegotiation/ContractNegotiationServiceImplTest.java
@@ -14,6 +14,7 @@
 
 package org.eclipse.edc.connector.controlplane.services.contractnegotiation;
 
+import org.eclipse.edc.connector.controlplane.contract.spi.ContractOfferId;
 import org.eclipse.edc.connector.controlplane.contract.spi.negotiation.ConsumerContractNegotiationManager;
 import org.eclipse.edc.connector.controlplane.contract.spi.negotiation.store.ContractNegotiationStore;
 import org.eclipse.edc.connector.controlplane.contract.spi.types.agreement.ContractAgreement;
@@ -269,10 +270,11 @@ private ContractNegotiation.Builder createContractNegotiationBuilder(String nego
     }
 
     private ContractOffer createContractOffer() {
+        var assetId = "test-asset";
         return ContractOffer.Builder.newInstance()
-                .id(UUID.randomUUID().toString())
+                .id(ContractOfferId.create("1", assetId).toString())
                 .policy(Policy.Builder.newInstance().build())
-                .assetId("test-asset")
+                .assetId(assetId)
                 .build();
     }
 }
diff --git a/...nnector/controlplane/contract/negotiation/ConsumerContractNegotiationManagerImplTest.java b/...nnector/controlplane/contract/negotiation/ConsumerContractNegotiationManagerImplTest.java
@@ -16,6 +16,7 @@
 package org.eclipse.edc.connector.controlplane.contract.negotiation;
 
 import org.eclipse.edc.connector.controlplane.contract.observe.ContractNegotiationObservableImpl;
+import org.eclipse.edc.connector.controlplane.contract.spi.ContractOfferId;
 import org.eclipse.edc.connector.controlplane.contract.spi.negotiation.ContractNegotiationPendingGuard;
 import org.eclipse.edc.connector.controlplane.contract.spi.negotiation.observe.ContractNegotiationListener;
 import org.eclipse.edc.connector.controlplane.contract.spi.negotiation.store.ContractNegotiationStore;
@@ -85,6 +86,7 @@
 
 class ConsumerContractNegotiationManagerImplTest {
 
+    private static final String ASSET_ID = "assetId";
     private static final String PARTICIPANT_ID = "participantId";
     private static final int RETRY_LIMIT = 1;
 
@@ -307,7 +309,7 @@ void dispatchException(ContractNegotiationStates starting, ContractNegotiationSt
     }
 
     private Criterion[] stateIs(int state) {
-        return aryEq(new Criterion[]{ hasState(state), isNotPending(), new Criterion("type", "=", "CONSUMER") });
+        return aryEq(new Criterion[] {hasState(state), isNotPending(), new Criterion("type", "=", "CONSUMER")});
     }
 
     private ContractNegotiation.Builder contractNegotiationBuilder() {
@@ -321,9 +323,10 @@ private ContractNegotiation.Builder contractNegotiationBuilder() {
     }
 
     private ContractOffer contractOffer() {
-        return ContractOffer.Builder.newInstance().id("id:assetId:random")
+        return ContractOffer.Builder.newInstance()
+                .id(ContractOfferId.create("1", ASSET_ID).toString())
                 .policy(Policy.Builder.newInstance().assigner("providerId").build())
-                .assetId("assetId")
+                .assetId(ASSET_ID)
                 .build();
     }
 
@@ -332,7 +335,7 @@ private ContractAgreement createContractAgreement() {
                 .id("contractId")
                 .consumerId("consumerId")
                 .providerId("providerId")
-                .assetId("assetId")
+                .assetId(ASSET_ID)
                 .policy(Policy.Builder.newInstance().build())
                 .build();
     }
@@ -368,15 +371,16 @@ private ContractAgreement createContractAgreement() {
                     .id("contractId")
                     .consumerId("consumerId")
                     .providerId("providerId")
-                    .assetId("assetId")
+                    .assetId(ASSET_ID)
                     .policy(Policy.Builder.newInstance().build())
                     .build();
         }
 
         private ContractOffer contractOffer() {
-            return ContractOffer.Builder.newInstance().id("id:assetId:random")
+            return ContractOffer.Builder.newInstance()
+                    .id(ContractOfferId.create("1", ASSET_ID).toString())
                     .policy(Policy.Builder.newInstance().build())
-                    .assetId("assetId")
+                    .assetId(ASSET_ID)
                     .build();
         }
     }

diff --git a/...nnector/controlplane/contract/negotiation/ProviderContractNegotiationManagerImplTest.java b/...nnector/controlplane/contract/negotiation/ProviderContractNegotiationManagerImplTest.java
@@ -88,6 +88,7 @@
 
 class ProviderContractNegotiationManagerImplTest {
 
+    private static final String ASSET_ID = "assetId";
     private static final String PROVIDER_ID = "provider";
     private static final int RETRY_LIMIT = 1;
     private final ContractNegotiationStore store = mock();
@@ -301,15 +302,16 @@ private ContractAgreement.Builder contractAgreementBuilder() {
     }
 
     private ContractOffer contractOffer() {
+        var assetId = "test-asset-id";
         return ContractOffer.Builder.newInstance()
-                .id(ContractOfferId.create("1", "test-asset-id").toString())
+                .id(ContractOfferId.create("1", assetId).toString())
                 .policy(Policy.Builder.newInstance().build())
-                .assetId("assetId")
+                .assetId(assetId)
                 .build();
     }
 
     private Criterion[] stateIs(int state) {
-        return aryEq(new Criterion[]{ hasState(state), isNotPending(), new Criterion("type", "=", "PROVIDER") });
+        return aryEq(new Criterion[] {hasState(state), isNotPending(), new Criterion("type", "=", "PROVIDER")});
     }
 
     private static class DispatchFailureArguments implements ArgumentsProvider {
@@ -339,9 +341,10 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext extensionCo
         }
 
         private ContractOffer contractOffer() {
-            return ContractOffer.Builder.newInstance().id("id:assetId:random")
+            return ContractOffer.Builder.newInstance()
+                    .id(ContractOfferId.create("1", ASSET_ID).toString())
                     .policy(Policy.Builder.newInstance().build())
-                    .assetId("assetId")
+                    .assetId(ASSET_ID)
                     .build();
         }
 
@@ -350,7 +353,7 @@ private ContractAgreement createContractAgreement() {
                     .id("contractId")
                     .consumerId("consumerId")
                     .providerId("providerId")
-                    .assetId("assetId")
+                    .assetId(ASSET_ID)
                     .policy(Policy.Builder.newInstance().build())
                     .build();
         }

diff --git a/...a/org/eclipse/edc/connector/controlplane/transfer/process/TransferProcessManagerImpl.java b/...a/org/eclipse/edc/connector/controlplane/transfer/process/TransferProcessManagerImpl.java
@@ -192,6 +192,11 @@ private boolean processInitial(TransferProcess process) {
             return true;
         }
 
+        if (!process.getAssetId().equals(policy.getTarget())) {
+            transitionToTerminated(process, "Transfer Process asset id %s does not match asset id in contract %s".formatted(process.getAssetId(), policy.getTarget()));
+            return true;
+        }
+
         ResourceManifest manifest;
         if (process.getType() == CONSUMER) {
             var manifestResult = manifestGenerator.generateConsumerResourceManifest(process, policy);
@@ -539,17 +544,17 @@ private <T> void handleResult(TransferProcess transferProcess, List<StatusResult
     }
 
     private Processor processConsumerTransfersInState(TransferProcessStates state, Function<TransferProcess, Boolean> function) {
-        var filter = new Criterion[]{ hasState(state.code()), isNotPending(), Criterion.criterion("type", "=", CONSUMER.name()) };
+        var filter = new Criterion[] {hasState(state.code()), isNotPending(), Criterion.criterion("type", "=", CONSUMER.name())};
         return createProcessor(function, filter);
     }
 
     private Processor processProviderTransfersInState(TransferProcessStates state, Function<TransferProcess, Boolean> function) {
-        var filter = new Criterion[]{ hasState(state.code()), isNotPending(), Criterion.criterion("type", "=", PROVIDER.name()) };
+        var filter = new Criterion[] {hasState(state.code()), isNotPending(), Criterion.criterion("type", "=", PROVIDER.name())};
         return createProcessor(function, filter);
     }
 
     private Processor processTransfersInState(TransferProcessStates state, Function<TransferProcess, Boolean> function) {
-        var filter = new Criterion[]{ hasState(state.code()), isNotPending() };
+        var filter = new Criterion[] {hasState(state.code()), isNotPending()};
         return createProcessor(function, filter);
     }