feat: fixes related to DSP conformance (eclipse-edc#4544)

Fixes for DSP conformance
wolf4ood · Oct 13, 2024 · d7b4222 · d7b4222
1 parent 9b94383
commit d7b4222
Show file tree

Hide file tree

Showing 10 changed files with 251 additions and 15 deletions.
diff --git a/...controlplane/services/contractnegotiation/ContractNegotiationProtocolServiceImplTest.java b/...controlplane/services/contractnegotiation/ContractNegotiationProtocolServiceImplTest.java
@@ -488,6 +488,7 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext extensionCo
                             .counterPartyAddress("http://any")
                             .consumerPid("consumerPid")
                             .providerPid("providerPid")
+                            .policy(Policy.Builder.newInstance().build())
                             .build(), PROVIDER, OFFERED),
                     Arguments.of(verified, ContractAgreementVerificationMessage.Builder.newInstance()
                             .protocol("protocol")

diff --git a/...c/connector/controlplane/contract/negotiation/ConsumerContractNegotiationManagerImpl.java b/...c/connector/controlplane/contract/negotiation/ConsumerContractNegotiationManagerImpl.java
@@ -126,17 +126,15 @@ private boolean processRequesting(ContractNegotiation negotiation) {
     }
 
     /**
-     * Processes {@link ContractNegotiation} in state ACCEPTING. Tries to send a dummy contract agreement to
-     * the respective provider in order to approve the last offer sent by the provider. If this succeeds, the
-     * ContractNegotiation is transitioned to state ACCEPTED. Else, it is transitioned to ACCEPTING
-     * for a retry.
+     * Processes {@link ContractNegotiation} in state ACCEPTING. If the dispatch succeeds, the
+     * ContractNegotiation is transitioned to state ACCEPTED. Else, it is transitioned to ACCEPTING for a retry.
      *
      * @return true if processed, false otherwise
      */
     @WithSpan
     private boolean processAccepting(ContractNegotiation negotiation) {
         var messageBuilder = ContractNegotiationEventMessage.Builder.newInstance().type(ACCEPTED);
-
+        messageBuilder.policy(negotiation.getLastContractOffer().getPolicy());
         return dispatch(messageBuilder, negotiation, Object.class)
                 .onSuccess((n, result) -> transitionToAccepted(n))
                 .onFailure((n, throwable) -> transitionToAccepting(n))

diff --git a/...nnector/controlplane/contract/negotiation/ConsumerContractNegotiationManagerImplTest.java b/...nnector/controlplane/contract/negotiation/ConsumerContractNegotiationManagerImplTest.java
@@ -21,6 +21,7 @@
 import org.eclipse.edc.connector.controlplane.contract.spi.negotiation.store.ContractNegotiationStore;
 import org.eclipse.edc.connector.controlplane.contract.spi.types.agreement.ContractAgreement;
 import org.eclipse.edc.connector.controlplane.contract.spi.types.agreement.ContractAgreementVerificationMessage;
+import org.eclipse.edc.connector.controlplane.contract.spi.types.agreement.ContractNegotiationEventMessage;
 import org.eclipse.edc.connector.controlplane.contract.spi.types.negotiation.ContractNegotiation;
 import org.eclipse.edc.connector.controlplane.contract.spi.types.negotiation.ContractNegotiationStates;
 import org.eclipse.edc.connector.controlplane.contract.spi.types.negotiation.ContractRequest;
@@ -210,12 +211,16 @@ void requesting_shouldSendMessageWithId_whenCorrelationIdIsNull_toSupportOldProt
     void accepting_shouldSendAcceptedMessageAndTransitionToApproved() {
         var negotiation = contractNegotiationBuilder().state(ACCEPTING.code()).contractOffer(contractOffer()).build();
         when(store.nextNotLeased(anyInt(), stateIs(ACCEPTING.code()))).thenReturn(List.of(negotiation)).thenReturn(emptyList());
-        when(dispatcherRegistry.dispatch(any(), any())).thenReturn(completedFuture(StatusResult.success("any")));
+
+        var captor = ArgumentCaptor.forClass(ContractNegotiationEventMessage.class);
+        when(dispatcherRegistry.dispatch(any(), captor.capture())).thenReturn(completedFuture(StatusResult.success("any")));
         when(store.findById(negotiation.getId())).thenReturn(negotiation);
 
         manager.start();
 
         await().untilAsserted(() -> {
+            var message = captor.getValue();
+            assertThat(message.getPolicy()).isNotNull();
             verify(store).save(argThat(p -> p.getState() == ACCEPTED.code()));
             verify(dispatcherRegistry, only()).dispatch(any(), any());
             verify(listener).accepted(any());

diff --git a/...g/eclipse/edc/connector/controlplane/transform/odrl/to/JsonObjectToPolicyTransformer.java b/...g/eclipse/edc/connector/controlplane/transform/odrl/to/JsonObjectToPolicyTransformer.java
@@ -85,6 +85,22 @@ public JsonObjectToPolicyTransformer(ParticipantIdMapper participantIdMapper) {
                     .error("Invalid type for ODRL policy, should be one of [%s, %s, %s]".formatted(ODRL_POLICY_TYPE_SET, ODRL_POLICY_TYPE_OFFER, ODRL_POLICY_TYPE_AGREEMENT))
                     .report();
             return null;
+        } else if (policyType == PolicyType.CONTRACT) {
+            if (object.get(ODRL_ASSIGNEE_ATTRIBUTE) == null) {
+                context.problem()
+                        .missingProperty()
+                        .property(ODRL_ASSIGNEE_ATTRIBUTE)
+                        .report();
+                return null;
+            }
+
+            if (object.get(ODRL_ASSIGNER_ATTRIBUTE) == null) {
+                context.problem()
+                        .missingProperty()
+                        .property(ODRL_ASSIGNER_ATTRIBUTE)
+                        .report();
+                return null;
+            }
         }
 
         builder.type(policyType);
@@ -94,8 +110,10 @@ public JsonObjectToPolicyTransformer(ParticipantIdMapper participantIdMapper) {
             case ODRL_PROHIBITION_ATTRIBUTE -> v -> builder.prohibitions(transformArray(v, Prohibition.class, context));
             case ODRL_OBLIGATION_ATTRIBUTE -> v -> builder.duties(transformArray(v, Duty.class, context));
             case ODRL_TARGET_ATTRIBUTE -> v -> builder.target(transformString(v, context));
-            case ODRL_ASSIGNER_ATTRIBUTE -> v -> builder.assigner(participantIdMapper.fromIri(transformString(v, context)));
-            case ODRL_ASSIGNEE_ATTRIBUTE -> v -> builder.assignee(participantIdMapper.fromIri(transformString(v, context)));
+            case ODRL_ASSIGNER_ATTRIBUTE ->
+                    v -> builder.assigner(participantIdMapper.fromIri(transformString(v, context)));
+            case ODRL_ASSIGNEE_ATTRIBUTE ->
+                    v -> builder.assignee(participantIdMapper.fromIri(transformString(v, context)));
             case ODRL_PROFILE_ATTRIBUTE -> v -> builder.profiles(transformProfile(v));
             default -> v -> builder.extensibleProperty(key, transformGenericProperty(v, context));
         });

diff --git a/...lipse/edc/connector/controlplane/transform/odrl/to/JsonObjectToPolicyTransformerTest.java b/...lipse/edc/connector/controlplane/transform/odrl/to/JsonObjectToPolicyTransformerTest.java
@@ -151,6 +151,8 @@ void transform_differentPolicyTypes_returnPolicy(String type, PolicyType policyT
         var policy = jsonFactory.createObjectBuilder()
                 .add(CONTEXT, JsonObject.EMPTY_JSON_OBJECT)
                 .add(TYPE, type)
+                .add(ODRL_ASSIGNEE_ATTRIBUTE, "assignee")
+                .add(ODRL_ASSIGNER_ATTRIBUTE, "assigner")
                 .build();
 
         var result = transformer.transform(TestInput.getExpanded(policy), context);
@@ -179,6 +181,8 @@ void shouldGetTypeFromContext_whenSet() {
 
         var policy = jsonFactory.createObjectBuilder()
                 .add(ODRL_TARGET_ATTRIBUTE, TARGET)
+                .add(ODRL_ASSIGNEE_ATTRIBUTE, "assignee")
+                .add(ODRL_ASSIGNER_ATTRIBUTE, "assigner")
                 .build();
 
         var result = transformer.transform(TestInput.getExpanded(policy), context);

diff --git a/...java/org/eclipse/edc/protocol/dsp/http/dispatcher/DspHttpRemoteMessageDispatcherImpl.java b/...java/org/eclipse/edc/protocol/dsp/http/dispatcher/DspHttpRemoteMessageDispatcherImpl.java
@@ -152,7 +152,7 @@ private <T> StatusResult<T> handleResponse(Response response, String protocol, C
             } else {
                 var stringBody = Optional.ofNullable(responseBody)
                         .map(this::asString)
-                        .orElse("Response body is null");
+                        .orElse("Response body is null. Error code: " + response.code());
 
                 var status = response.code() >= 400 && response.code() < 500 ? FATAL_ERROR : ERROR_RETRY;
 

diff --git a/...p-core/src/main/java/org/eclipse/edc/protocol/dsp/http/message/DspRequestHandlerImpl.java b/...p-core/src/main/java/org/eclipse/edc/protocol/dsp/http/message/DspRequestHandlerImpl.java
@@ -189,7 +189,7 @@ public <I extends RemoteMessage, R> Response updateResource(PostDspRequest<I, R>
                 });
 
         if (inputTransformation.failed()) {
-            monitor.debug(() -> "DSP: Transformation failed: %s".formatted(validation.getFailureMessages()));
+            monitor.debug(() -> "DSP: Transformation failed: %s".formatted(inputTransformation.getFailureMessages()));
             return type(request.getErrorType()).processId(request.getProcessId()).badRequest();
         }
 

diff --git a/...otiation/transform/from/JsonObjectFromContractNegotiationEventMessageTransformerTest.java b/...otiation/transform/from/JsonObjectFromContractNegotiationEventMessageTransformerTest.java
@@ -17,6 +17,7 @@
 import jakarta.json.Json;
 import jakarta.json.JsonBuilderFactory;
 import org.eclipse.edc.connector.controlplane.contract.spi.types.agreement.ContractNegotiationEventMessage;
+import org.eclipse.edc.policy.model.Policy;
 import org.eclipse.edc.transform.spi.TransformerContext;
 import org.junit.jupiter.api.Test;
 
@@ -52,6 +53,7 @@ void transform() {
                 .consumerPid("consumerPid")
                 .providerPid("providerPid")
                 .counterPartyAddress("https://test.com")
+                .policy(Policy.Builder.newInstance().build())
                 .type(ACCEPTED)
                 .build();
 

diff --git a/...clipse/edc/connector/controlplane/contract/spi/types/negotiation/ContractNegotiation.java b/...clipse/edc/connector/controlplane/contract/spi/types/negotiation/ContractNegotiation.java
@@ -38,6 +38,7 @@
 import static java.lang.String.format;
 import static org.eclipse.edc.connector.controlplane.contract.spi.types.negotiation.ContractNegotiation.Type.CONSUMER;
 import static org.eclipse.edc.connector.controlplane.contract.spi.types.negotiation.ContractNegotiation.Type.PROVIDER;
+import static org.eclipse.edc.connector.controlplane.contract.spi.types.negotiation.ContractNegotiationStates.FINALIZED;
 import static org.eclipse.edc.spi.constants.CoreConstants.EDC_NAMESPACE;
 
 /**
@@ -190,7 +191,7 @@ public void transitionRequesting() {
         if (Type.PROVIDER == type) {
             throw new IllegalStateException("Provider processes have no REQUESTING state");
         }
-        transition(ContractNegotiationStates.REQUESTING, ContractNegotiationStates.REQUESTING, ContractNegotiationStates.INITIAL);
+        transition(ContractNegotiationStates.REQUESTING, ContractNegotiationStates.REQUESTING, ContractNegotiationStates.OFFERED, ContractNegotiationStates.INITIAL);
     }
 
     /**
@@ -209,7 +210,7 @@ public void transitionRequested() {
      */
     public void transitionOffering() {
         if (CONSUMER == type) {
-            throw new IllegalStateException("Provider processes have no OFFERING state");
+            throw new IllegalStateException("Consumer processes have no OFFERING state");
         }
 
         transition(ContractNegotiationStates.OFFERING, ContractNegotiationStates.OFFERING, ContractNegotiationStates.OFFERED, ContractNegotiationStates.REQUESTED);
@@ -233,7 +234,7 @@ public void transitionAccepting() {
         if (Type.PROVIDER == type) {
             throw new IllegalStateException("Provider processes have no ACCEPTING state");
         }
-        transition(ContractNegotiationStates.ACCEPTING, ContractNegotiationStates.ACCEPTING, ContractNegotiationStates.REQUESTED);
+        transition(ContractNegotiationStates.ACCEPTING, ContractNegotiationStates.ACCEPTING, ContractNegotiationStates.REQUESTED, ContractNegotiationStates.OFFERED);
     }
 
     /**
@@ -301,7 +302,7 @@ public void transitionFinalizing() {
      * Transition to state FINALIZED.
      */
     public void transitionFinalized() {
-        transition(ContractNegotiationStates.FINALIZED, ContractNegotiationStates.FINALIZED, ContractNegotiationStates.FINALIZING, ContractNegotiationStates.AGREED, ContractNegotiationStates.VERIFIED);
+        transition(FINALIZED, FINALIZED, ContractNegotiationStates.FINALIZING, ContractNegotiationStates.AGREED, ContractNegotiationStates.VERIFIED);
     }
 
     /**
@@ -310,7 +311,7 @@ public void transitionFinalized() {
      * @return true if the negotiation can be terminated, false otherwise
      */
     public boolean canBeTerminated() {
-        return true;
+        return FINALIZED.code() != state;
     }
 
     /**