diff --git a/core/data-plane-selector/data-plane-selector-core/build.gradle.kts b/core/data-plane-selector/data-plane-selector-core/build.gradle.kts index b6dd363a019..8f425b7864a 100644 --- a/core/data-plane-selector/data-plane-selector-core/build.gradle.kts +++ b/core/data-plane-selector/data-plane-selector-core/build.gradle.kts @@ -21,6 +21,7 @@ dependencies { implementation(project(":spi:common:transaction-spi")) implementation(project(":core:common:lib:util-lib")) + testImplementation(project(":core:common:junit")) testImplementation(testFixtures(project(":spi:data-plane-selector:data-plane-selector-spi"))) testImplementation(project(":core:common:junit")) } diff --git a/core/data-plane-selector/data-plane-selector-core/src/main/java/org/eclipse/edc/connector/dataplane/selector/service/EmbeddedDataPlaneSelectorService.java b/core/data-plane-selector/data-plane-selector-core/src/main/java/org/eclipse/edc/connector/dataplane/selector/service/EmbeddedDataPlaneSelectorService.java index 67e168add2d..6c8b0814504 100644 --- a/core/data-plane-selector/data-plane-selector-core/src/main/java/org/eclipse/edc/connector/dataplane/selector/service/EmbeddedDataPlaneSelectorService.java +++ b/core/data-plane-selector/data-plane-selector-core/src/main/java/org/eclipse/edc/connector/dataplane/selector/service/EmbeddedDataPlaneSelectorService.java @@ -81,4 +81,9 @@ public ServiceResult addInstance(DataPlaneInstance instance) { return ServiceResult.from(result); }); } + + @Override + public ServiceResult delete(String instanceId) { + return transactionContext.execute(() -> ServiceResult.from(store.deleteById(instanceId))).mapEmpty(); + } } diff --git a/core/data-plane-selector/data-plane-selector-core/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/InMemoryDataPlaneInstanceStore.java b/core/data-plane-selector/data-plane-selector-core/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/InMemoryDataPlaneInstanceStore.java index 1635d488231..24db86bc29c 100644 --- a/core/data-plane-selector/data-plane-selector-core/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/InMemoryDataPlaneInstanceStore.java +++ b/core/data-plane-selector/data-plane-selector-core/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/InMemoryDataPlaneInstanceStore.java @@ -17,7 +17,6 @@ import org.eclipse.edc.connector.dataplane.selector.spi.instance.DataPlaneInstance; import org.eclipse.edc.connector.dataplane.selector.spi.store.DataPlaneInstanceStore; import org.eclipse.edc.spi.result.StoreResult; -import org.eclipse.edc.util.concurrency.LockManager; import java.util.Map; import java.util.Optional; @@ -27,22 +26,18 @@ import static java.lang.String.format; /** - * Default (=in-memory) implementation for the {@link DataPlaneInstanceStore}. All r/w access is secured with a {@link LockManager}. + * Default (=in-memory) implementation for the {@link DataPlaneInstanceStore}. */ public class InMemoryDataPlaneInstanceStore implements DataPlaneInstanceStore { private final Map instances = new ConcurrentHashMap<>(); - public InMemoryDataPlaneInstanceStore() { - } - @Override public StoreResult create(DataPlaneInstance instance) { var prev = instances.putIfAbsent(instance.getId(), instance); return Optional.ofNullable(prev) .map(a -> StoreResult.alreadyExists(format(DATA_PLANE_INSTANCE_EXISTS, instance.getId()))) .orElse(StoreResult.success()); - } @Override @@ -53,6 +48,15 @@ public StoreResult update(DataPlaneInstance instance) { .orElse(StoreResult.notFound(format(DATA_PLANE_INSTANCE_NOT_FOUND, instance.getId()))); } + @Override + public StoreResult deleteById(String instanceId) { + var removed = instances.remove(instanceId); + if (removed == null) { + return StoreResult.notFound("DataPlane instance %s not found".formatted(instanceId)); + } + return StoreResult.success(removed); + } + @Override public DataPlaneInstance findById(String id) { return instances.get(id); diff --git a/core/data-plane-selector/data-plane-selector-core/src/test/java/org/eclipse/edc/connector/dataplane/selector/service/EmbeddedDataPlaneSelectorServiceTest.java b/core/data-plane-selector/data-plane-selector-core/src/test/java/org/eclipse/edc/connector/dataplane/selector/service/EmbeddedDataPlaneSelectorServiceTest.java index 2cc8686cbe4..2a6a53e0963 100644 --- a/core/data-plane-selector/data-plane-selector-core/src/test/java/org/eclipse/edc/connector/dataplane/selector/service/EmbeddedDataPlaneSelectorServiceTest.java +++ b/core/data-plane-selector/data-plane-selector-core/src/test/java/org/eclipse/edc/connector/dataplane/selector/service/EmbeddedDataPlaneSelectorServiceTest.java @@ -20,13 +20,16 @@ import org.eclipse.edc.connector.dataplane.selector.spi.strategy.SelectionStrategy; import org.eclipse.edc.connector.dataplane.selector.spi.strategy.SelectionStrategyRegistry; import org.eclipse.edc.spi.result.ServiceFailure; +import org.eclipse.edc.spi.result.StoreResult; +import org.eclipse.edc.spi.types.domain.DataAddress; import org.eclipse.edc.transaction.spi.NoopTransactionContext; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import java.util.UUID; import java.util.stream.IntStream; import java.util.stream.Stream; -import static org.eclipse.edc.connector.dataplane.selector.spi.testfixtures.TestFunctions.createAddress; import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; import static org.eclipse.edc.spi.result.ServiceFailure.Reason.BAD_REQUEST; import static org.eclipse.edc.spi.result.ServiceFailure.Reason.NOT_FOUND; @@ -39,7 +42,7 @@ public class EmbeddedDataPlaneSelectorServiceTest { private final DataPlaneInstanceStore store = mock(); private final SelectionStrategyRegistry selectionStrategyRegistry = mock(); - private final DataPlaneSelectorService selector = new EmbeddedDataPlaneSelectorService(store, selectionStrategyRegistry, new NoopTransactionContext()); + private final DataPlaneSelectorService service = new EmbeddedDataPlaneSelectorService(store, selectionStrategyRegistry, new NoopTransactionContext()); @Test void select_shouldUseChosenSelector() { @@ -49,7 +52,7 @@ void select_shouldUseChosenSelector() { when(selectionStrategy.apply(any())).thenAnswer(it -> instances.get(0)); when(selectionStrategyRegistry.find(any())).thenReturn(selectionStrategy); - var result = selector.select(createAddress("srcTestType"), "transferType", "strategy"); + var result = service.select(createAddress("srcTestType"), "transferType", "strategy"); assertThat(result).isSucceeded().extracting(DataPlaneInstance::getId).isEqualTo("instance0"); verify(selectionStrategyRegistry).find("strategy"); @@ -61,7 +64,7 @@ void select_shouldReturnBadRequest_whenStrategyNotFound() { when(store.getAll()).thenReturn(instances.stream()); when(selectionStrategyRegistry.find(any())).thenReturn(null); - var result = selector.select(createAddress("srcTestType"), "transferType", "strategy"); + var result = service.select(createAddress("srcTestType"), "transferType", "strategy"); assertThat(result).isFailed().extracting(ServiceFailure::getReason).isEqualTo(BAD_REQUEST); } @@ -71,17 +74,55 @@ void select_shouldReturnNotFound_whenInstanceNotFound() { when(store.getAll()).thenReturn(Stream.empty()); when(selectionStrategyRegistry.find(any())).thenReturn(mock()); - var result = selector.select(createAddress("srcTestType"), "transferType", "strategy"); + var result = service.select(createAddress("srcTestType"), "transferType", "strategy"); assertThat(result).isFailed().extracting(ServiceFailure::getReason).isEqualTo(NOT_FOUND); } + @Nested + class Delete { + + @Test + void shouldDelete() { + var instanceId = UUID.randomUUID().toString(); + var instance = createInstanceBuilder(instanceId).build(); + when(store.deleteById(any())).thenReturn(StoreResult.success(instance)); + + var result = service.delete(instanceId); + + assertThat(result).isSucceeded().isNull(); + } + + @Test + void shouldReturnNotFound_whenInstanceIsNotFound() { + var instanceId = UUID.randomUUID().toString(); + when(store.deleteById(any())).thenReturn(StoreResult.notFound("not found")); + + var result = service.delete(instanceId); + + assertThat(result).isFailed().extracting(ServiceFailure::getReason).isEqualTo(NOT_FOUND); + } + + } + private DataPlaneInstance createInstanceMock(String id, String srcType, String destType) { - return DataPlaneInstance.Builder.newInstance() - .url("http://any") - .id(id) + return createInstanceBuilder(id) .allowedSourceType(srcType) .allowedDestType(destType) .build(); } + + private DataPlaneInstance.Builder createInstanceBuilder(String id) { + return DataPlaneInstance.Builder.newInstance() + .id(id) + .url("http://any"); + } + + private DataAddress createAddress(String type) { + return DataAddress.Builder.newInstance() + .type("test-type") + .keyName(type) + .property("someprop", "someval") + .build(); + } } diff --git a/core/data-plane-selector/data-plane-selector-core/src/test/java/org/eclipse/edc/connector/dataplane/selector/store/InMemoryDataPlaneInstanceStoreTest.java b/core/data-plane-selector/data-plane-selector-core/src/test/java/org/eclipse/edc/connector/dataplane/selector/store/InMemoryDataPlaneInstanceStoreTest.java index 8df329fd772..4ececb3770c 100644 --- a/core/data-plane-selector/data-plane-selector-core/src/test/java/org/eclipse/edc/connector/dataplane/selector/store/InMemoryDataPlaneInstanceStoreTest.java +++ b/core/data-plane-selector/data-plane-selector-core/src/test/java/org/eclipse/edc/connector/dataplane/selector/store/InMemoryDataPlaneInstanceStoreTest.java @@ -26,7 +26,6 @@ void setup() { store = new InMemoryDataPlaneInstanceStore(); } - @Override public InMemoryDataPlaneInstanceStore getStore() { return store; diff --git a/extensions/data-plane-selector/data-plane-selector-client/src/main/java/org/eclipse/edc/connector/dataplane/selector/RemoteDataPlaneSelectorService.java b/extensions/data-plane-selector/data-plane-selector-client/src/main/java/org/eclipse/edc/connector/dataplane/selector/RemoteDataPlaneSelectorService.java index 387566063cd..4d32a1f6ab7 100644 --- a/extensions/data-plane-selector/data-plane-selector-client/src/main/java/org/eclipse/edc/connector/dataplane/selector/RemoteDataPlaneSelectorService.java +++ b/extensions/data-plane-selector/data-plane-selector-client/src/main/java/org/eclipse/edc/connector/dataplane/selector/RemoteDataPlaneSelectorService.java @@ -30,7 +30,6 @@ import org.eclipse.edc.spi.result.ServiceResult; import org.eclipse.edc.spi.types.domain.DataAddress; import org.eclipse.edc.transform.spi.TypeTransformerRegistry; -import org.eclipse.edc.util.string.StringUtils; import org.jetbrains.annotations.Nullable; import java.io.IOException; @@ -97,22 +96,6 @@ public ServiceResult select(DataAddress source, String transf .compose(ServiceResult::from); } - private ServiceResult toJsonObject(String it) { - try { - return ServiceResult.success(mapper.readValue(it, JsonObject.class)); - } catch (JsonProcessingException e) { - return ServiceResult.unexpected("Cannot deserialize response body as JsonObject"); - } - } - - private ServiceResult toJsonArray(String it) { - try { - return ServiceResult.success(mapper.readValue(it, JsonArray.class)); - } catch (JsonProcessingException e) { - return ServiceResult.unexpected("Cannot deserialize response body as JsonObject"); - } - } - @Override public ServiceResult addInstance(DataPlaneInstance instance) { var transform = typeTransformerRegistry.transform(instance, JsonObject.class); @@ -127,7 +110,14 @@ public ServiceResult addInstance(DataPlaneInstance instance) { var request = new Request.Builder().post(body).url(url).build(); - return request(request).map(it -> null); + return request(request).mapEmpty(); + } + + @Override + public ServiceResult delete(String instanceId) { + var request = new Request.Builder().delete().url(url + "/" + instanceId).build(); + + return request(request).mapEmpty(); } private ServiceResult request(Request request) { @@ -137,10 +127,6 @@ private ServiceResult request(Request request) { ) { var bodyAsString = responseBody == null ? null : responseBody.string(); if (response.isSuccessful()) { - if (StringUtils.isNullOrEmpty(bodyAsString)) { - return ServiceResult.badRequest("Response body is null or empty"); - } - return ServiceResult.success(bodyAsString); } else { @@ -157,5 +143,21 @@ private ServiceResult request(Request request) { } } + private ServiceResult toJsonObject(String it) { + try { + return ServiceResult.success(mapper.readValue(it, JsonObject.class)); + } catch (JsonProcessingException e) { + return ServiceResult.unexpected("Cannot deserialize response body as JsonObject"); + } + } + + private ServiceResult toJsonArray(String it) { + try { + return ServiceResult.success(mapper.readValue(it, JsonArray.class)); + } catch (JsonProcessingException e) { + return ServiceResult.unexpected("Cannot deserialize response body as JsonObject"); + } + } + } diff --git a/extensions/data-plane-selector/data-plane-selector-client/src/test/java/org/eclipse/edc/connector/dataplane/selector/RemoteDataPlaneSelectorServiceTest.java b/extensions/data-plane-selector/data-plane-selector-client/src/test/java/org/eclipse/edc/connector/dataplane/selector/RemoteDataPlaneSelectorServiceTest.java index 3569fb6dbb2..d71fffb5bdd 100644 --- a/extensions/data-plane-selector/data-plane-selector-client/src/test/java/org/eclipse/edc/connector/dataplane/selector/RemoteDataPlaneSelectorServiceTest.java +++ b/extensions/data-plane-selector/data-plane-selector-client/src/test/java/org/eclipse/edc/connector/dataplane/selector/RemoteDataPlaneSelectorServiceTest.java @@ -22,6 +22,7 @@ import org.eclipse.edc.connector.dataplane.selector.transformer.JsonObjectToSelectionRequestTransformer; import org.eclipse.edc.jsonld.util.JacksonJsonLd; import org.eclipse.edc.junit.annotations.ComponentTest; +import org.eclipse.edc.spi.result.ServiceFailure; import org.eclipse.edc.spi.result.ServiceResult; import org.eclipse.edc.spi.types.domain.DataAddress; import org.eclipse.edc.transform.TypeTransformerRegistryImpl; @@ -35,13 +36,16 @@ import org.eclipse.edc.validator.spi.ValidationResult; import org.eclipse.edc.web.jersey.testfixtures.RestControllerTestBase; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import java.time.Clock; import java.util.Map; +import java.util.UUID; import static org.eclipse.edc.http.client.testfixtures.HttpTestUtils.testHttpClient; import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; +import static org.eclipse.edc.spi.result.ServiceFailure.Reason.NOT_FOUND; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -94,6 +98,31 @@ void select() { assertThat(result).isSucceeded().usingRecursiveComparison().isEqualTo(expected); } + @Nested + class Delete { + + @Test + void shouldDelete() { + var instanceId = UUID.randomUUID().toString(); + when(serverService.delete(any())).thenReturn(ServiceResult.success()); + + var result = service.delete(instanceId); + + assertThat(result).isSucceeded(); + verify(serverService).delete(instanceId); + } + + @Test + void shouldFail_whenNotFound() { + var instanceId = UUID.randomUUID().toString(); + when(serverService.delete(any())).thenReturn(ServiceResult.notFound("not found")); + + var result = service.delete(instanceId); + + assertThat(result).isFailed().extracting(ServiceFailure::getReason).isEqualTo(NOT_FOUND); + } + } + private DataPlaneInstance createInstance(String id) { return DataPlaneInstance.Builder.newInstance() .id(id) diff --git a/extensions/data-plane-selector/data-plane-selector-control-api/src/main/java/org/eclipse/edc/connector/dataplane/selector/control/api/DataplaneSelectorControlApiController.java b/extensions/data-plane-selector/data-plane-selector-control-api/src/main/java/org/eclipse/edc/connector/dataplane/selector/control/api/DataplaneSelectorControlApiController.java index 227b09f6895..2f00b11d342 100644 --- a/extensions/data-plane-selector/data-plane-selector-control-api/src/main/java/org/eclipse/edc/connector/dataplane/selector/control/api/DataplaneSelectorControlApiController.java +++ b/extensions/data-plane-selector/data-plane-selector-control-api/src/main/java/org/eclipse/edc/connector/dataplane/selector/control/api/DataplaneSelectorControlApiController.java @@ -82,7 +82,7 @@ public JsonObject registerDataplane(JsonObject request) { @DELETE @Path("/{id}") public void unregisterDataplane(@PathParam("id") String id) { - throw new UnsupportedOperationException("not yet implemented"); + service.delete(id).orElseThrow(exceptionMapper(DataPlaneInstance.class)); } @POST diff --git a/extensions/data-plane-selector/data-plane-selector-control-api/src/test/java/org/eclipse/edc/connector/dataplane/selector/control/api/DataplaneSelectorControlApiControllerTest.java b/extensions/data-plane-selector/data-plane-selector-control-api/src/test/java/org/eclipse/edc/connector/dataplane/selector/control/api/DataplaneSelectorControlApiControllerTest.java index e980eba959b..1263f1ca1aa 100644 --- a/extensions/data-plane-selector/data-plane-selector-control-api/src/test/java/org/eclipse/edc/connector/dataplane/selector/control/api/DataplaneSelectorControlApiControllerTest.java +++ b/extensions/data-plane-selector/data-plane-selector-control-api/src/test/java/org/eclipse/edc/connector/dataplane/selector/control/api/DataplaneSelectorControlApiControllerTest.java @@ -32,6 +32,7 @@ import java.time.Clock; import java.util.List; +import java.util.UUID; import static io.restassured.RestAssured.given; import static io.restassured.http.ContentType.JSON; @@ -143,6 +144,36 @@ void shouldFail_whenEgressTransformationFails() { } } + @Nested + class Unregister { + + @Test + void shouldDeleteInstance() { + when(service.delete(any())).thenReturn(ServiceResult.success()); + var instanceId = UUID.randomUUID().toString(); + + given() + .port(port) + .delete("/v1/dataplanes/{id}", instanceId) + .then() + .statusCode(204); + + verify(service).delete(instanceId); + } + + @Test + void shouldReturnNotFound_whenServiceReturnsNotFound() { + when(service.delete(any())).thenReturn(ServiceResult.notFound("not found")); + var instanceId = UUID.randomUUID().toString(); + + given() + .port(port) + .delete("/v1/dataplanes/{id}", instanceId) + .then() + .statusCode(404); + } + } + @Nested class Select { diff --git a/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/sql/SqlDataPlaneInstanceStore.java b/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/sql/SqlDataPlaneInstanceStore.java index 38e83c4ebf7..a843f1d73d5 100644 --- a/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/sql/SqlDataPlaneInstanceStore.java +++ b/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/sql/SqlDataPlaneInstanceStore.java @@ -85,6 +85,24 @@ public StoreResult update(DataPlaneInstance instance) { }); } + @Override + public StoreResult deleteById(String instanceId) { + return transactionContext.execute(() -> { + try (var connection = getConnection()) { + var instance = findById(instanceId); + if (instance == null) { + return StoreResult.notFound("DataPlane instance %s not found".formatted(instanceId)); + } + + queryExecutor.execute(connection, statements.getDeleteByIdTemplate(), instanceId); + + return StoreResult.success(instance); + } catch (SQLException e) { + throw new EdcPersistenceException(e.getMessage(), e); + } + }); + } + @Override public DataPlaneInstance findById(String id) { Objects.requireNonNull(id); @@ -125,11 +143,9 @@ private void update(Connection connection, DataPlaneInstance instance) { queryExecutor.execute(connection, sql, toJson(instance), instance.getId()); } - private DataPlaneInstance mapResultSet(ResultSet resultSet) throws Exception { var json = resultSet.getString(statements.getDataColumn()); return fromJson(json, DataPlaneInstance.class); } - } diff --git a/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/sql/schema/BaseSqlDataPlaneInstanceStatements.java b/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/sql/schema/BaseSqlDataPlaneInstanceStatements.java index 7e39351072b..6bc4c848c23 100644 --- a/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/sql/schema/BaseSqlDataPlaneInstanceStatements.java +++ b/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/sql/schema/BaseSqlDataPlaneInstanceStatements.java @@ -40,4 +40,10 @@ public String getUpdateTemplate() { .jsonColumn(getDataColumn()) .update(getDataPlaneInstanceTable(), getIdColumn()); } + + @Override + public String getDeleteByIdTemplate() { + return executeStatement() + .delete(getDataPlaneInstanceTable(), getIdColumn()); + } } diff --git a/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/sql/schema/DataPlaneInstanceStatements.java b/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/sql/schema/DataPlaneInstanceStatements.java index 2f5cabeebe7..eba14481967 100644 --- a/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/sql/schema/DataPlaneInstanceStatements.java +++ b/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/main/java/org/eclipse/edc/connector/dataplane/selector/store/sql/schema/DataPlaneInstanceStatements.java @@ -39,5 +39,7 @@ default String getDataColumn() { String getUpdateTemplate(); + String getDeleteByIdTemplate(); + } diff --git a/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/test/java/org/eclipse/edc/connector/dataplane/selector/store/sql/PostgresDataPlaneInstanceStoreTest.java b/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/test/java/org/eclipse/edc/connector/dataplane/selector/store/sql/PostgresDataPlaneInstanceStoreTest.java index 665abb4474d..d53748534b7 100644 --- a/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/test/java/org/eclipse/edc/connector/dataplane/selector/store/sql/PostgresDataPlaneInstanceStoreTest.java +++ b/extensions/data-plane-selector/store/sql/data-plane-instance-store-sql/src/test/java/org/eclipse/edc/connector/dataplane/selector/store/sql/PostgresDataPlaneInstanceStoreTest.java @@ -32,17 +32,14 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; -import java.sql.SQLException; @ComponentTest @ExtendWith(PostgresqlStoreSetupExtension.class) public class PostgresDataPlaneInstanceStoreTest extends DataPlaneInstanceStoreTestBase { - private final DataPlaneInstanceStatements statements = new PostgresDataPlaneInstanceStatements(); - - SqlDataPlaneInstanceStore store; + private SqlDataPlaneInstanceStore store; @BeforeAll static void prepare(PostgresqlLocalInstance postgres) { @@ -51,7 +48,6 @@ static void prepare(PostgresqlLocalInstance postgres) { @BeforeEach void setUp(PostgresqlStoreSetupExtension extension, QueryExecutor queryExecutor) throws IOException { - var typeManager = new JacksonTypeManager(); typeManager.registerTypes(DataPlaneInstance.class); @@ -62,7 +58,7 @@ void setUp(PostgresqlStoreSetupExtension extension, QueryExecutor queryExecutor) } @AfterEach - void tearDown(PostgresqlStoreSetupExtension extension) throws SQLException { + void tearDown(PostgresqlStoreSetupExtension extension) { extension.runQuery("DROP TABLE " + statements.getDataPlaneInstanceTable() + " CASCADE"); } diff --git a/extensions/data-plane/data-plane-self-registration/src/main/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtension.java b/extensions/data-plane/data-plane-self-registration/src/main/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtension.java index ac9bc022449..2bd1776cb8a 100644 --- a/extensions/data-plane/data-plane-self-registration/src/main/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtension.java +++ b/extensions/data-plane/data-plane-self-registration/src/main/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtension.java @@ -79,8 +79,16 @@ public void start() { .build(); dataPlaneSelectorService.addInstance(instance) - .onSuccess(it -> context.getMonitor().info("Data plane registered to control-plane")) - .orElseThrow(f -> new EdcException("Cannot register dataplane to the controlplane: " + f.getFailureDetail())); + .onSuccess(it -> context.getMonitor().info("data-plane registered to control-plane")) + .orElseThrow(f -> new EdcException("Cannot register data-plane to the control-plane: " + f.getFailureDetail())); + } + + @Override + public void shutdown() { + dataPlaneSelectorService.delete(context.getRuntimeId()) + .onSuccess(it -> context.getMonitor().info("data-plane successfully unregistered")) + .onFailure(failure -> context.getMonitor().severe("error during data-plane un-registration. %s: %s" + .formatted(failure.getReason(), failure.getFailureDetail()))); } private @NotNull Stream toTransferTypes(FlowType pull, Set types) { diff --git a/extensions/data-plane/data-plane-self-registration/src/test/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtensionTest.java b/extensions/data-plane/data-plane-self-registration/src/test/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtensionTest.java index 792960b2c59..2623671e1cb 100644 --- a/extensions/data-plane/data-plane-self-registration/src/test/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtensionTest.java +++ b/extensions/data-plane/data-plane-self-registration/src/test/java/org/eclipse/edc/connector/dataplane/registration/DataplaneSelfRegistrationExtensionTest.java @@ -88,4 +88,15 @@ void shouldNotStart_whenRegistrationFails(DataplaneSelfRegistrationExtension ext assertThatThrownBy(extension::start).isInstanceOf(EdcException.class); } + + @Test + void shouldUnregisterInstanceAtStartup(DataplaneSelfRegistrationExtension extension, ServiceExtensionContext context) { + when(context.getRuntimeId()).thenReturn("runtimeId"); + when(dataPlaneSelectorService.delete(any())).thenReturn(ServiceResult.success()); + extension.initialize(context); + + extension.shutdown(); + + verify(dataPlaneSelectorService).delete("runtimeId"); + } } diff --git a/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/result/AbstractResult.java b/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/result/AbstractResult.java index ce80ab7242a..82da28c9c39 100644 --- a/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/result/AbstractResult.java +++ b/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/result/AbstractResult.java @@ -148,6 +148,22 @@ public > R2 map(Function mapFunc } } + /** + * Maps this {@link Result} into another. If this {@link Result} is successful, the content is discarded. If this + * {@link Result} failed, the failures are carried over. + */ + public > R2 mapEmpty() { + return map(it -> null); + } + + /** + * Maps this {@link Result} into another. The content gets discarded, this method can be used to forward failures + * to the caller. + */ + public > R2 mapFailure() { + return map(it -> null); + } + /** * Maps one result into another, applying the mapping function. * diff --git a/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/result/Result.java b/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/result/Result.java index 58f48bce8e6..b511f975d3b 100644 --- a/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/result/Result.java +++ b/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/result/Result.java @@ -104,13 +104,11 @@ public Result merge(Result other) { * * @see Result#map(Function) * @see Result#mapTo(Class) + * @deprecated please use {@link #mapEmpty()} or {@link #mapFailure()}. */ + @Deprecated(since = "0.6.4") public Result mapTo() { - if (succeeded()) { - return new Result<>(null, null); - } else { - return Result.failure(getFailureMessages()); - } + return mapEmpty(); } /** @@ -125,9 +123,11 @@ public Result mapTo() { * @param clazz type of the result, with which the resulting {@link Result} should be parameterized * @see Result#map(Function) * @see Result#mapTo() + * @deprecated please use {@link #mapEmpty()} or {@link #mapFailure()}. */ + @Deprecated(since = "0.6.4") public Result mapTo(Class clazz) { - return mapTo(); + return mapEmpty(); } /** diff --git a/spi/common/boot-spi/src/test/java/org/eclipse/edc/spi/result/ResultTest.java b/spi/common/boot-spi/src/test/java/org/eclipse/edc/spi/result/ResultTest.java index 6ed277f206c..05f2443b5de 100644 --- a/spi/common/boot-spi/src/test/java/org/eclipse/edc/spi/result/ResultTest.java +++ b/spi/common/boot-spi/src/test/java/org/eclipse/edc/spi/result/ResultTest.java @@ -14,7 +14,6 @@ package org.eclipse.edc.spi.result; -import org.eclipse.edc.junit.assertions.AbstractResultAssert; import org.eclipse.edc.spi.EdcException; import org.junit.jupiter.api.Test; @@ -24,6 +23,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -97,7 +97,7 @@ void onSuccess_whenSucceeded() { var result = Result.success("foo"); Consumer consumer = mock(); - AbstractResultAssert.assertThat(result.onSuccess(consumer)).isEqualTo(result); + assertThat(result.onSuccess(consumer)).isEqualTo(result); verify(consumer).accept(eq("foo")); } @@ -106,7 +106,7 @@ void onSuccess_whenFailed() { Consumer consumer = mock(); Result result = Result.failure("bar"); - AbstractResultAssert.assertThat(result.onSuccess(consumer)).isEqualTo(result); + assertThat(result.onSuccess(consumer)).isEqualTo(result); verifyNoInteractions(consumer); } @@ -115,7 +115,7 @@ void onFailure_whenSucceeded() { var result = Result.success("foo"); Consumer consumer = mock(); - AbstractResultAssert.assertThat(result.onFailure(consumer)).isEqualTo(result); + assertThat(result.onFailure(consumer)).isEqualTo(result); verifyNoInteractions(consumer); } @@ -124,10 +124,28 @@ void onFailure_whenFailed() { Consumer consumer = mock(); Result result = Result.failure("bar"); - AbstractResultAssert.assertThat(result.onFailure(consumer)).isEqualTo(result); + assertThat(result.onFailure(consumer)).isEqualTo(result); verify(consumer).accept(argThat(f -> f.getMessages().contains("bar"))); } + @Test + void mapToEmpty_succeeded() { + var result = Result.success("foobar"); + + Result mapped = result.mapEmpty(); + + assertThat(mapped).isSucceeded().isNull(); + } + + @Test + void mapToEmpty_failed() { + var result = Result.failure("foobar"); + + Result mapped = result.mapEmpty(); + + assertThat(mapped).isFailed().detail().isEqualTo("foobar"); + } + @Test void mapTo_succeeded() { var res = Result.success("foobar"); @@ -160,7 +178,6 @@ void mapTo_explicitType_failed() { assertThat(mapped.getFailureDetail()).isEqualTo("foobar"); } - @Test void whenSuccess_chainsSuccesses() { var result1 = Result.success("res1"); @@ -226,10 +243,10 @@ void compose_applyMappingFunctionIfSuccessful() { ? Result.success("the content was good") : Result.failure("the content was not good"); - AbstractResultAssert.assertThat(Result.success("right content").compose(mappingFunction)).isSucceeded() + assertThat(Result.success("right content").compose(mappingFunction)).isSucceeded() .isEqualTo("the content was good"); - AbstractResultAssert.assertThat(Result.success("not right content").compose(mappingFunction)).isFailed() + assertThat(Result.success("not right content").compose(mappingFunction)).isFailed() .extracting(Failure::getMessages).asList().contains("the content was not good"); } @@ -237,7 +254,7 @@ void compose_applyMappingFunctionIfSuccessful() { void compose_doNothingIfFailed() { var result = Result.failure("error").compose(it -> Result.success()); - AbstractResultAssert.assertThat(result).isFailed().extracting(Failure::getMessages).asList().contains("error"); + assertThat(result).isFailed().extracting(Failure::getMessages).asList().contains("error"); } @Test @@ -246,8 +263,8 @@ void recover_shouldApplyMappingFunction_whenFailed() { Function> successfulRecoverFunction = f -> Result.success("ok"); Function> failingRecoverFunction = f -> Result.failure("error"); - AbstractResultAssert.assertThat(failedResult.recover(successfulRecoverFunction)).isSucceeded().isEqualTo("ok"); - AbstractResultAssert.assertThat(failedResult.recover(failingRecoverFunction)).isFailed().extracting(Failure::getMessages).asList().contains("error"); + assertThat(failedResult.recover(successfulRecoverFunction)).isSucceeded().isEqualTo("ok"); + assertThat(failedResult.recover(failingRecoverFunction)).isFailed().extracting(Failure::getMessages).asList().contains("error"); } @Test @@ -255,14 +272,14 @@ void recover_shouldDoNothing_whenSucceeded() { var succeededResult = Result.success("ok"); Function> failingRecoverFunction = f -> Result.failure("error"); - AbstractResultAssert.assertThat(succeededResult.recover(failingRecoverFunction)).isSucceeded(); + assertThat(succeededResult.recover(failingRecoverFunction)).isSucceeded(); } @Test void ofThrowable_success() { var result = Result.ofThrowable(String::new); - AbstractResultAssert.assertThat(result).isSucceeded().isEqualTo(""); + assertThat(result).isSucceeded().isEqualTo(""); } @Test @@ -271,6 +288,6 @@ void ofThrowable_failure() { var result = Result.ofThrowable(() -> { throw new EdcException("Exception"); }); - AbstractResultAssert.assertThat(result).isFailed().detail().contains("Exception"); + assertThat(result).isFailed().detail().contains("Exception"); } } diff --git a/spi/data-plane-selector/data-plane-selector-spi/build.gradle.kts b/spi/data-plane-selector/data-plane-selector-spi/build.gradle.kts index 2791a22d4fd..64a2e5c0c43 100644 --- a/spi/data-plane-selector/data-plane-selector-spi/build.gradle.kts +++ b/spi/data-plane-selector/data-plane-selector-spi/build.gradle.kts @@ -24,7 +24,7 @@ dependencies { testImplementation(project(":core:common:lib:json-lib")) - // needed by the abstract test spec located in testFixtures + testFixturesImplementation(testFixtures(project(":core:common:junit"))) testFixturesImplementation(libs.bundles.jupiter) testFixturesImplementation(libs.mockito.core) testFixturesImplementation(libs.assertj) diff --git a/spi/data-plane-selector/data-plane-selector-spi/src/main/java/org/eclipse/edc/connector/dataplane/selector/spi/DataPlaneSelectorService.java b/spi/data-plane-selector/data-plane-selector-spi/src/main/java/org/eclipse/edc/connector/dataplane/selector/spi/DataPlaneSelectorService.java index 74cc4283aa0..02fdbacdb9c 100644 --- a/spi/data-plane-selector/data-plane-selector-spi/src/main/java/org/eclipse/edc/connector/dataplane/selector/spi/DataPlaneSelectorService.java +++ b/spi/data-plane-selector/data-plane-selector-spi/src/main/java/org/eclipse/edc/connector/dataplane/selector/spi/DataPlaneSelectorService.java @@ -88,4 +88,12 @@ default DataPlaneInstance select(DataAddress source, DataAddress destination, @N */ ServiceResult addInstance(DataPlaneInstance instance); + /** + * Delete a Data Plane instance. + * + * @param instanceId the instance id. + * @return successful result if operation completed, failure otherwise. + */ + ServiceResult delete(String instanceId); + } diff --git a/spi/data-plane-selector/data-plane-selector-spi/src/main/java/org/eclipse/edc/connector/dataplane/selector/spi/store/DataPlaneInstanceStore.java b/spi/data-plane-selector/data-plane-selector-spi/src/main/java/org/eclipse/edc/connector/dataplane/selector/spi/store/DataPlaneInstanceStore.java index 96f17313b5d..790cbe1f2d0 100644 --- a/spi/data-plane-selector/data-plane-selector-spi/src/main/java/org/eclipse/edc/connector/dataplane/selector/spi/store/DataPlaneInstanceStore.java +++ b/spi/data-plane-selector/data-plane-selector-spi/src/main/java/org/eclipse/edc/connector/dataplane/selector/spi/store/DataPlaneInstanceStore.java @@ -48,6 +48,14 @@ public interface DataPlaneInstanceStore { */ StoreResult update(DataPlaneInstance instance); + /** + * Delete a data plane instance by its id. + * + * @param instanceId the data plane instance id. + * @return the deleted data plane instance. + */ + StoreResult deleteById(String instanceId); + DataPlaneInstance findById(String id); Stream getAll(); diff --git a/spi/data-plane-selector/data-plane-selector-spi/src/testFixtures/java/org/eclipse/edc/connector/dataplane/selector/spi/testfixtures/TestFunctions.java b/spi/data-plane-selector/data-plane-selector-spi/src/testFixtures/java/org/eclipse/edc/connector/dataplane/selector/spi/testfixtures/TestFunctions.java deleted file mode 100644 index 9c309f3f066..00000000000 --- a/spi/data-plane-selector/data-plane-selector-spi/src/testFixtures/java/org/eclipse/edc/connector/dataplane/selector/spi/testfixtures/TestFunctions.java +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright (c) 2020 - 2022 Microsoft Corporation - * - * This program and the accompanying materials are made available under the - * terms of the Apache License, Version 2.0 which is available at - * https://www.apache.org/licenses/LICENSE-2.0 - * - * SPDX-License-Identifier: Apache-2.0 - * - * Contributors: - * Microsoft Corporation - initial API and implementation - * - */ - -package org.eclipse.edc.connector.dataplane.selector.spi.testfixtures; - -import org.eclipse.edc.connector.dataplane.selector.spi.instance.DataPlaneInstance; -import org.eclipse.edc.spi.types.domain.DataAddress; - -public class TestFunctions { - - public static DataAddress createAddress(String type) { - return DataAddress.Builder.newInstance() - .type("test-type") - .keyName(type) - .property("someprop", "someval") - .build(); - } - - public static DataPlaneInstance createInstance(String id) { - return createInstanceBuilder(id).build(); - } - - public static DataPlaneInstance.Builder createInstanceBuilder(String id) { - return DataPlaneInstance.Builder.newInstance() - .id(id) - .url("http://somewhere.com:1234/api/v1"); - } - - public static DataPlaneInstance createCustomInstance(String id, String name) { - return DataPlaneInstance.Builder.newInstance() - .id(id) - .url("http://somewhere.com:1234/api/v1") - .property("name", "name") - .build(); - } - -} diff --git a/spi/data-plane-selector/data-plane-selector-spi/src/testFixtures/java/org/eclipse/edc/connector/dataplane/selector/spi/testfixtures/store/DataPlaneInstanceStoreTestBase.java b/spi/data-plane-selector/data-plane-selector-spi/src/testFixtures/java/org/eclipse/edc/connector/dataplane/selector/spi/testfixtures/store/DataPlaneInstanceStoreTestBase.java index 2a812ed61e7..b87e08aa670 100644 --- a/spi/data-plane-selector/data-plane-selector-spi/src/testFixtures/java/org/eclipse/edc/connector/dataplane/selector/spi/testfixtures/store/DataPlaneInstanceStoreTestBase.java +++ b/spi/data-plane-selector/data-plane-selector-spi/src/testFixtures/java/org/eclipse/edc/connector/dataplane/selector/spi/testfixtures/store/DataPlaneInstanceStoreTestBase.java @@ -14,36 +14,39 @@ package org.eclipse.edc.connector.dataplane.selector.spi.testfixtures.store; -import org.assertj.core.api.Assertions; import org.eclipse.edc.connector.dataplane.selector.spi.instance.DataPlaneInstance; import org.eclipse.edc.connector.dataplane.selector.spi.store.DataPlaneInstanceStore; -import org.eclipse.edc.connector.dataplane.selector.spi.testfixtures.TestFunctions; +import org.eclipse.edc.spi.result.StoreFailure; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import java.util.UUID; + import static org.assertj.core.api.Assertions.assertThat; +import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; import static org.eclipse.edc.spi.result.StoreFailure.Reason.ALREADY_EXISTS; +import static org.eclipse.edc.spi.result.StoreFailure.Reason.NOT_FOUND; public abstract class DataPlaneInstanceStoreTestBase { - @Test void save() { - var inst = TestFunctions.createInstance("test-id"); + var inst = createInstanceBuilder("test-id").build(); getStore().create(inst); - Assertions.assertThat(getStore().getAll()).usingRecursiveFieldByFieldElementComparator().containsExactly(inst); + assertThat(getStore().getAll()).usingRecursiveFieldByFieldElementComparator().containsExactly(inst); } @Test void save_withAllowedTransferTypes() { - var inst = TestFunctions.createInstanceBuilder("test-id").allowedTransferType("transfer-type").build(); + var inst = createInstanceBuilder("test-id").allowedTransferType("transfer-type").build(); getStore().create(inst); - Assertions.assertThat(getStore().getAll()).usingRecursiveFieldByFieldElementComparator().containsExactly(inst); + assertThat(getStore().getAll()).usingRecursiveFieldByFieldElementComparator().containsExactly(inst); } @Test void save_whenExists_shouldNotUpsert() { - var inst = TestFunctions.createInstance("test-id"); + var inst = createInstanceBuilder("test-id").build(); getStore().create(inst); @@ -57,12 +60,12 @@ void save_whenExists_shouldNotUpsert() { assertThat(result.failed()).isTrue(); assertThat(result.getFailure().getReason()).isEqualTo(ALREADY_EXISTS); - Assertions.assertThat(getStore().getAll()).hasSize(1).usingRecursiveFieldByFieldElementComparator().containsExactly(inst); + assertThat(getStore().getAll()).hasSize(1).usingRecursiveFieldByFieldElementComparator().containsExactly(inst); } @Test void update_whenExists_shouldUpdate() { - var inst = TestFunctions.createInstance("test-id"); + var inst = createInstanceBuilder("test-id").build(); getStore().create(inst); @@ -74,21 +77,18 @@ void update_whenExists_shouldUpdate() { var result = getStore().update(inst2); assertThat(result.succeeded()).isTrue(); - - Assertions.assertThat(getStore().getAll()).hasSize(1).usingRecursiveFieldByFieldElementComparator().containsExactly(inst2); + assertThat(getStore().getAll()).hasSize(1).usingRecursiveFieldByFieldElementComparator().containsExactly(inst2); } - @Test void save_shouldReturnCustomInstance() { - var custom = TestFunctions.createCustomInstance("test-id", "name"); + var custom = createInstanceWithProperty("test-id", "name"); getStore().create(custom); var customInstance = getStore().findById(custom.getId()); - - Assertions.assertThat(customInstance) + assertThat(customInstance) .isInstanceOf(DataPlaneInstance.class) .usingRecursiveComparison() .isEqualTo(custom); @@ -96,21 +96,21 @@ void save_shouldReturnCustomInstance() { @Test void findById() { - var inst = TestFunctions.createInstance("test-id"); + var inst = createInstanceBuilder("test-id").build(); getStore().create(inst); - Assertions.assertThat(getStore().findById("test-id")).usingRecursiveComparison().isEqualTo(inst); + assertThat(getStore().findById("test-id")).usingRecursiveComparison().isEqualTo(inst); } @Test void findById_notExists() { - Assertions.assertThat(getStore().findById("not-exist")).isNull(); + assertThat(getStore().findById("not-exist")).isNull(); } @Test void getAll() { - var doc1 = TestFunctions.createCustomInstance("test-id", "name"); - var doc2 = TestFunctions.createCustomInstance("test-id-2", "name"); + var doc1 = createInstanceWithProperty("test-id", "name"); + var doc2 = createInstanceWithProperty("test-id-2", "name"); var store = getStore(); @@ -122,6 +122,44 @@ void getAll() { assertThat(foundItems).isNotNull().hasSize(2); } + @Nested + class DeleteById { + + @Test + void shouldDeleteDataPlaneInstanceById() { + var id = UUID.randomUUID().toString(); + var instance = createInstanceBuilder(id).build(); + getStore().create(instance); + + var result = getStore().deleteById(id); + + assertThat(result).isSucceeded().usingRecursiveComparison().isEqualTo(instance); + } + + @Test + void shouldFail_whenInstanceDoesNotExist() { + var randomId = UUID.randomUUID().toString(); + + var result = getStore().deleteById(randomId); + + assertThat(result).isFailed().extracting(StoreFailure::getReason).isEqualTo(NOT_FOUND); + } + + } protected abstract DataPlaneInstanceStore getStore(); + + + private DataPlaneInstance createInstanceWithProperty(String id, String name) { + return createInstanceBuilder(id) + .property("name", name) + .build(); + } + + private DataPlaneInstance.Builder createInstanceBuilder(String id) { + return DataPlaneInstance.Builder.newInstance() + .id(id) + .url("http://somewhere.com:1234/api/v1"); + } + }