refactor: get rid of jakarta-validation framework (#3236)

eclipse-edc · Jun 27, 2023 · 41ab605 · 41ab605
1 parent 83df482
commit 41ab605
Show file tree

Hide file tree

Showing 21 changed files with 93 additions and 219 deletions.
diff --git a/extensions/common/api/api-core/build.gradle.kts b/extensions/common/api/api-core/build.gradle.kts
@@ -26,8 +26,6 @@ dependencies {
     implementation(project(":core:common:util"))
     implementation(project(":core:common:validator-core"))
     implementation(libs.jakarta.rsApi)
-    implementation(libs.jakarta.validation)
-    implementation(libs.jersey.beanvalidation) //for validation
 
     testImplementation(libs.jersey.common)
     testImplementation(libs.jersey.server)

diff --git a/extensions/common/http/jersey-core/build.gradle.kts b/extensions/common/http/jersey-core/build.gradle.kts
@@ -27,7 +27,6 @@ dependencies {
 
     testImplementation(project(":core:common:junit"))
     testImplementation(libs.restAssured)
-    testImplementation(libs.jersey.beanvalidation) //for validation
 
     testFixturesApi(project(":core:common:junit"))
     testFixturesApi(project(":extensions:common:json-ld"))

diff --git a/...s/common/http/jersey-core/src/main/java/org/eclipse/edc/web/jersey/JerseyRestService.java b/...s/common/http/jersey-core/src/main/java/org/eclipse/edc/web/jersey/JerseyRestService.java
@@ -21,7 +21,6 @@
 import org.eclipse.edc.web.jersey.jsonld.ObjectMapperProvider;
 import org.eclipse.edc.web.jersey.mapper.EdcApiExceptionMapper;
 import org.eclipse.edc.web.jersey.mapper.UnexpectedExceptionMapper;
-import org.eclipse.edc.web.jersey.mapper.ValidationExceptionMapper;
 import org.eclipse.edc.web.jetty.JettyService;
 import org.eclipse.edc.web.spi.WebService;
 import org.glassfish.hk2.utilities.binding.AbstractBinder;
@@ -92,7 +91,6 @@ private void registerContext(String contextAlias, List<Object> controllers) {
         resourceConfig.registerInstances(new Binder());
         resourceConfig.registerInstances(new ObjectMapperProvider(typeManager.getMapper()));
         resourceConfig.registerInstances(new EdcApiExceptionMapper());
-        resourceConfig.registerInstances(new ValidationExceptionMapper());
         resourceConfig.registerInstances(new UnexpectedExceptionMapper(monitor));
 
         additionalInstances.forEach(supplier -> resourceConfig.registerInstances(supplier.get()));

diff --git a/...ersey-core/src/main/java/org/eclipse/edc/web/jersey/mapper/ValidationExceptionMapper.java b/...ersey-core/src/main/java/org/eclipse/edc/web/jersey/mapper/ValidationExceptionMapper.java
diff --git a/...core/src/test/java/org/eclipse/edc/web/jersey/mapper/ExceptionMappersIntegrationTest.java b/...core/src/test/java/org/eclipse/edc/web/jersey/mapper/ExceptionMappersIntegrationTest.java
@@ -15,11 +15,6 @@
 package org.eclipse.edc.web.jersey.mapper;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
-import jakarta.validation.MessageInterpolator;
-import jakarta.validation.Valid;
-import jakarta.validation.Validation;
-import jakarta.validation.constraints.NotBlank;
-import jakarta.validation.constraints.Positive;
 import jakarta.ws.rs.Consumes;
 import jakarta.ws.rs.GET;
 import jakarta.ws.rs.POST;
@@ -42,35 +37,24 @@
 import static io.restassured.http.ContentType.JSON;
 import static org.eclipse.edc.junit.testfixtures.TestUtils.getFreePort;
 import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.endsWith;
-import static org.hamcrest.CoreMatchers.hasItems;
 import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.Matchers.hasEntry;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 
 @ApiTest
 @ExtendWith(EdcExtension.class)
 public class ExceptionMappersIntegrationTest {
 
-    private static final String POSITIVE_TEMPLATE = "{jakarta.validation.constraints.Positive.message}";
-    private static final String NOT_BLANK_TEMPLATE = "{jakarta.validation.constraints.NotBlank.message}";
     private final int port = getFreePort();
     private final Runnable runnable = mock(Runnable.class);
 
-    private MessageInterpolator interpolator;
-
     @BeforeEach
     void setUp(EdcExtension extension) {
         extension.setConfiguration(Map.of(
                 "web.http.port", String.valueOf(port),
                 "web.http.path", "/api"
         ));
         extension.registerSystemExtension(ServiceExtension.class, new MyServiceExtension());
-
-        try (var factory = Validation.buildDefaultValidatorFactory()) {
-            interpolator = factory.getMessageInterpolator();
-        }
     }
 
     @Test
@@ -89,31 +73,6 @@ void shouldListEdcApiExceptionMessageInResponseBody() {
                 .body("[0].type", is("ObjectNotFound"));
     }
 
-    @Test
-    void shouldListJakartaValidationMessageInResponseBody() {
-        given()
-                .port(port)
-                .accept(JSON)
-                .contentType(JSON)
-                .body(Map.of("data", "", "number", "-1"))
-                .post("/api/test")
-                .then()
-                .statusCode(400)
-                .body("size()", is(2))
-                .body("", hasItems(
-                        hasEntry("message", interpolator.interpolate(POSITIVE_TEMPLATE, null)),
-                        hasEntry("type", POSITIVE_TEMPLATE),
-                        hasEntry(is("path"), endsWith(".number")),
-                        hasEntry("invalidValue", "-1")
-                ))
-                .body("", hasItems(
-                        hasEntry("message", interpolator.interpolate(NOT_BLANK_TEMPLATE, null)),
-                        hasEntry("type", NOT_BLANK_TEMPLATE),
-                        hasEntry(is("path"), endsWith(".data")),
-                        hasEntry("invalidValue", "")
-                ));
-    }
-
     @Test
     void shouldReturn500ErrorOnJavaLangExceptions() {
         doThrow(new NullPointerException()).when(runnable).run();
@@ -127,11 +86,9 @@ void shouldReturn500ErrorOnJavaLangExceptions() {
     }
 
     private static class RequestPayload {
-        @NotBlank
         @JsonProperty
         private String data;
 
-        @Positive
         @JsonProperty
         private long number;
     }
@@ -148,7 +105,7 @@ public Map<String, String> get() {
 
         @POST
         @Consumes("application/json")
-        public void doAction(@Valid RequestPayload payload) {
+        public void doAction(RequestPayload payload) {
         }
 
     }

diff --git a/extensions/control-plane/api/control-plane-api/build.gradle.kts b/extensions/control-plane/api/control-plane-api/build.gradle.kts
@@ -26,8 +26,6 @@ dependencies {
     implementation(project(":extensions:common:api:control-api-configuration"))
 
     implementation(libs.jakarta.rsApi)
-    implementation(libs.jakarta.validation)
-    implementation(libs.jersey.beanvalidation) //for validation
 
     testImplementation(project(":core:control-plane:control-plane-core"))
     testImplementation(project(":core:data-plane-selector:data-plane-selector-core"))

diff --git a/...rc/main/java/org/eclipse/edc/connector/api/transferprocess/TransferProcessControlApi.java b/...rc/main/java/org/eclipse/edc/connector/api/transferprocess/TransferProcessControlApi.java
@@ -21,8 +21,6 @@
 import io.swagger.v3.oas.annotations.media.Schema;
 import io.swagger.v3.oas.annotations.responses.ApiResponse;
 import io.swagger.v3.oas.annotations.tags.Tag;
-import jakarta.validation.Valid;
-import jakarta.validation.constraints.NotNull;
 import org.eclipse.edc.connector.api.transferprocess.model.TransferProcessFailStateDto;
 import org.eclipse.edc.web.spi.ApiErrorDetail;
 
@@ -47,7 +45,7 @@ public interface TransferProcessControlApi {
                     @ApiResponse(responseCode = "400", description = "Request was malformed, e.g. id was null",
                             content = @Content(array = @ArraySchema(schema = @Schema(implementation = ApiErrorDetail.class))))
             })
-    void fail(String processId, @NotNull @Valid TransferProcessFailStateDto request);
+    void fail(String processId, TransferProcessFailStateDto request);
 
 
 }
diff --git a/...va/org/eclipse/edc/connector/api/transferprocess/TransferProcessControlApiController.java b/...va/org/eclipse/edc/connector/api/transferprocess/TransferProcessControlApiController.java
@@ -14,8 +14,6 @@
 
 package org.eclipse.edc.connector.api.transferprocess;
 
-import jakarta.validation.Valid;
-import jakarta.validation.constraints.NotNull;
 import jakarta.ws.rs.Consumes;
 import jakarta.ws.rs.POST;
 import jakarta.ws.rs.Path;
@@ -25,6 +23,10 @@
 import org.eclipse.edc.connector.api.transferprocess.model.TransferProcessFailStateDto;
 import org.eclipse.edc.connector.spi.transferprocess.TransferProcessService;
 import org.eclipse.edc.connector.transfer.spi.types.TransferProcess;
+import org.eclipse.edc.validator.spi.ValidationResult;
+import org.eclipse.edc.validator.spi.Validator;
+import org.eclipse.edc.validator.spi.Violation;
+import org.eclipse.edc.web.spi.exception.ValidationFailureException;
 
 import static org.eclipse.edc.web.spi.exception.ServiceResultHandler.exceptionMapper;
 
@@ -36,7 +38,7 @@ public class TransferProcessControlApiController implements TransferProcessContr
 
     public static final String PATH = "/transferprocess";
     private final TransferProcessService transferProcessService;
-
+    private final Validator<TransferProcessFailStateDto> validator = new TransferProcessFailStateDtoValidator();
 
     public TransferProcessControlApiController(TransferProcessService transferProcessService) {
         this.transferProcessService = transferProcessService;
@@ -53,8 +55,23 @@ public void complete(@PathParam("processId") String processId) {
     @POST
     @Path("/{processId}/fail")
     @Override
-    public void fail(@PathParam("processId") String processId, @NotNull @Valid TransferProcessFailStateDto request) {
+    public void fail(@PathParam("processId") String processId, TransferProcessFailStateDto request) {
+        validator.validate(request).orElseThrow(ValidationFailureException::new);
+
         transferProcessService.terminate(processId, request.getErrorMessage()).orElseThrow(exceptionMapper(TransferProcess.class, processId));
     }
 
+    private static class TransferProcessFailStateDtoValidator implements Validator<TransferProcessFailStateDto> {
+        @Override
+        public ValidationResult validate(TransferProcessFailStateDto input) {
+            if (input == null) {
+                return ValidationResult.failure(Violation.violation("requestBody cannot be null", ""));
+            }
+
+            if (input.getErrorMessage() == null) {
+                return ValidationResult.failure(Violation.violation("errorMessage cannot be null", "errorMessage"));
+            }
+            return ValidationResult.success();
+        }
+    }
 }
diff --git a/...java/org/eclipse/edc/connector/api/transferprocess/model/TransferProcessFailStateDto.java b/...java/org/eclipse/edc/connector/api/transferprocess/model/TransferProcessFailStateDto.java
@@ -18,11 +18,9 @@
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
-import jakarta.validation.constraints.NotNull;
 
 @JsonDeserialize(builder = TransferProcessFailStateDto.Builder.class)
 public class TransferProcessFailStateDto {
-    @NotNull
     private String errorMessage;
 
     private TransferProcessFailStateDto() {

diff --git a/...l-plane-api/src/test/java/org/eclipse/edc/connector/api/ControlPlaneApiExtensionTest.java b/...l-plane-api/src/test/java/org/eclipse/edc/connector/api/ControlPlaneApiExtensionTest.java
@@ -14,17 +14,12 @@
 
 package org.eclipse.edc.connector.api;
 
-import org.eclipse.edc.api.auth.spi.AuthenticationService;
 import org.eclipse.edc.connector.api.control.configuration.ControlApiConfiguration;
 import org.eclipse.edc.connector.api.transferprocess.TransferProcessControlApiController;
-import org.eclipse.edc.connector.spi.transferprocess.TransferProcessService;
 import org.eclipse.edc.junit.extensions.DependencyInjectionExtension;
-import org.eclipse.edc.spi.monitor.Monitor;
 import org.eclipse.edc.spi.system.Hostname;
 import org.eclipse.edc.spi.system.ServiceExtensionContext;
 import org.eclipse.edc.spi.system.configuration.ConfigFactory;
-import org.eclipse.edc.spi.system.injection.ObjectFactory;
-import org.eclipse.edc.spi.types.TypeManager;
 import org.eclipse.edc.web.spi.WebService;
 import org.eclipse.edc.web.spi.configuration.WebServiceConfiguration;
 import org.junit.jupiter.api.BeforeEach;
@@ -34,41 +29,28 @@
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @ExtendWith(DependencyInjectionExtension.class)
 class ControlPlaneApiExtensionTest {
 
-    private ServiceExtensionContext context;
-    private final WebService webService = mock(WebService.class);
-    private final Monitor monitor = mock(Monitor.class);
-
-    private ControlPlaneApiExtension extension;
+    private final WebService webService = mock();
 
     @BeforeEach
-    void setup(ServiceExtensionContext context, ObjectFactory factory) {
+    void setup(ServiceExtensionContext context) {
         var webServiceConfiguration = WebServiceConfiguration.Builder.newInstance()
                 .contextAlias("control")
                 .path("/control")
                 .port(8888)
                 .build();
         context.registerService(WebService.class, webService);
         context.registerService(Hostname.class, () -> "localhost");
-        context.registerService(TransferProcessService.class, mock(TransferProcessService.class));
-        context.registerService(AuthenticationService.class, mock(AuthenticationService.class));
         context.registerService(ControlApiConfiguration.class, new ControlApiConfiguration(webServiceConfiguration));
-        context.registerService(TypeManager.class, mock(TypeManager.class));
-
-        this.context = spy(context); //used to inject the config
-        when(this.context.getMonitor()).thenReturn(monitor);
-
-        extension = factory.constructInstance(ControlPlaneApiExtension.class);
     }
 
     @Test
-    void initialize_shouldBeRegisteredAsControlApiService() {
+    void initialize_shouldBeRegisteredAsControlApiService(ControlPlaneApiExtension extension, ServiceExtensionContext context) {
         when(context.getConfig()).thenReturn(ConfigFactory.empty());
 
         extension.initialize(context);

diff --git a/extensions/control-plane/provision/provision-http/build.gradle.kts b/extensions/control-plane/provision/provision-http/build.gradle.kts
@@ -32,8 +32,8 @@ dependencies {
     testImplementation(project(":core:data-plane-selector:data-plane-selector-core"))
     testImplementation(project(":extensions:common:http"))
     testImplementation(project(":extensions:common:json-ld"))
+    testImplementation(testFixtures(project(":extensions:common:http:jersey-core")))
     testImplementation(libs.restAssured)
-
     testImplementation(libs.awaitility)
 }
 

diff --git a/...main/java/org/eclipse/edc/connector/provision/http/webhook/HttpProvisionerWebhookApi.java b/...main/java/org/eclipse/edc/connector/provision/http/webhook/HttpProvisionerWebhookApi.java
@@ -16,7 +16,6 @@
 
 import io.swagger.v3.oas.annotations.OpenAPIDefinition;
 import io.swagger.v3.oas.annotations.tags.Tag;
-import jakarta.validation.Valid;
 import org.eclipse.edc.connector.transfer.spi.types.DeprovisionedResource;
 
 /**
@@ -25,7 +24,7 @@
 @OpenAPIDefinition
 @Tag(name = "HTTP Provisioner Webhook")
 public interface HttpProvisionerWebhookApi {
-    void callProvisionWebhook(String transferProcessId, @Valid ProvisionerWebhookRequest request);
+    void callProvisionWebhook(String transferProcessId, ProvisionerWebhookRequest request);
 
-    void callDeprovisionWebhook(String transferProcessId, @Valid DeprovisionedResource resource);
+    void callDeprovisionWebhook(String transferProcessId, DeprovisionedResource resource);
 }