From b6ea76d60a3ebf2196dff819949cd8d0a1619bac Mon Sep 17 00:00:00 2001 From: ndr_brt Date: Wed, 21 Aug 2024 08:59:43 +0200 Subject: [PATCH] fix: consider terminated a DataFlow that cannot be terminated because it does not exist --- .../DataPlaneSignalingFlowController.java | 13 ++++++-- .../DataPlaneSignalingFlowControllerTest.java | 32 +++++++++++++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/extensions/control-plane/transfer/transfer-data-plane-signaling/src/main/java/org/eclipse/edc/connector/controlplane/transfer/dataplane/flow/DataPlaneSignalingFlowController.java b/extensions/control-plane/transfer/transfer-data-plane-signaling/src/main/java/org/eclipse/edc/connector/controlplane/transfer/dataplane/flow/DataPlaneSignalingFlowController.java index 8ff89a979d2..a3e673f26d5 100644 --- a/extensions/control-plane/transfer/transfer-data-plane-signaling/src/main/java/org/eclipse/edc/connector/controlplane/transfer/dataplane/flow/DataPlaneSignalingFlowController.java +++ b/extensions/control-plane/transfer/transfer-data-plane-signaling/src/main/java/org/eclipse/edc/connector/controlplane/transfer/dataplane/flow/DataPlaneSignalingFlowController.java @@ -31,6 +31,7 @@ import org.jetbrains.annotations.NotNull; import java.util.Collection; +import java.util.Optional; import java.util.Set; import java.util.UUID; @@ -114,7 +115,10 @@ public boolean canHandle(TransferProcess transferProcess) { @Override public StatusResult suspend(TransferProcess transferProcess) { - return getClientForDataplane(transferProcess.getDataPlaneId()) + return Optional.ofNullable(transferProcess.getDataPlaneId()) + .map(StatusResult::success) + .orElse(StatusResult.failure(FATAL_ERROR, "DataPlane id is null")) + .compose(this::getClientForDataplane) .map(client -> client.suspend(transferProcess.getId())) .orElse(f -> { var message = "Failed to select the data plane for suspending the transfer process %s. %s" @@ -125,7 +129,12 @@ public StatusResult suspend(TransferProcess transferProcess) { @Override public StatusResult terminate(TransferProcess transferProcess) { - return getClientForDataplane(transferProcess.getDataPlaneId()) + var dataPlaneId = transferProcess.getDataPlaneId(); + if (dataPlaneId == null) { + return StatusResult.success(); + } + + return getClientForDataplane(dataPlaneId) .map(client -> client.terminate(transferProcess.getId())) .orElse(f -> { var message = "Failed to select the data plane for terminating the transfer process %s. %s" diff --git a/extensions/control-plane/transfer/transfer-data-plane-signaling/src/test/java/org/eclipse/edc/connector/controlplane/transfer/dataplane/flow/DataPlaneSignalingFlowControllerTest.java b/extensions/control-plane/transfer/transfer-data-plane-signaling/src/test/java/org/eclipse/edc/connector/controlplane/transfer/dataplane/flow/DataPlaneSignalingFlowControllerTest.java index f135fa1a05f..d18efb055a4 100644 --- a/extensions/control-plane/transfer/transfer-data-plane-signaling/src/test/java/org/eclipse/edc/connector/controlplane/transfer/dataplane/flow/DataPlaneSignalingFlowControllerTest.java +++ b/extensions/control-plane/transfer/transfer-data-plane-signaling/src/test/java/org/eclipse/edc/connector/controlplane/transfer/dataplane/flow/DataPlaneSignalingFlowControllerTest.java @@ -50,6 +50,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; public class DataPlaneSignalingFlowControllerTest { @@ -256,6 +257,20 @@ void shouldFail_whenDataPlaneNotFound() { assertThat(result).isFailed().detail().contains("Failed to select the data plane for terminating the transfer process"); } + + @Test // a null dataPlaneId means that the flow has not been started so it can be considered as already terminated + void shouldReturnSuccess_whenDataPlaneIdIsNull() { + var transferProcess = transferProcessBuilder() + .id("transferProcessId") + .contentDataAddress(testDataAddress()) + .dataPlaneId(null) + .build(); + + var result = flowController.terminate(transferProcess); + + assertThat(result).isSucceeded(); + verifyNoInteractions(dataPlaneClient, dataPlaneClientFactory, selectorService); + } } @Nested @@ -287,13 +302,26 @@ void shouldFail_whenDataPlaneDoesNotExist() { .contentDataAddress(testDataAddress()) .dataPlaneId("invalid") .build(); - when(dataPlaneClient.suspend(any())).thenReturn(StatusResult.success()); - when(dataPlaneClientFactory.createClient(any())).thenReturn(dataPlaneClient); when(selectorService.findById(any())).thenReturn(ServiceResult.notFound("not found")); var result = flowController.suspend(transferProcess); assertThat(result).isFailed().detail().contains("Failed to select the data plane for suspending the transfer process"); + verifyNoInteractions(dataPlaneClient, dataPlaneClientFactory); + } + + @Test + void shouldFail_whenDataPlaneIdIsNull() { + var transferProcess = TransferProcess.Builder.newInstance() + .id("transferProcessId") + .contentDataAddress(testDataAddress()) + .dataPlaneId(null) + .build(); + + var result = flowController.suspend(transferProcess); + + assertThat(result).isFailed().detail().contains("Failed to select the data plane for suspending the transfer process"); + verifyNoInteractions(dataPlaneClient, dataPlaneClientFactory, selectorService); } }