diff --git a/src/main/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsExceptionMapper.java b/src/main/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsExceptionMapper.java index e2a1e2c..2a19bb0 100644 --- a/src/main/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsExceptionMapper.java +++ b/src/main/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsExceptionMapper.java @@ -5,14 +5,14 @@ package io.github.genomicdatainfrastructure.daam.api; import io.github.genomicdatainfrastructure.daam.exceptions.AcceptTermsException; -import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; - import io.github.genomicdatainfrastructure.daam.model.ErrorResponse; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.ext.ExceptionMapper; import jakarta.ws.rs.ext.Provider; +import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; + @Provider public class AcceptTermsExceptionMapper implements ExceptionMapper { @@ -22,7 +22,7 @@ public Response toResponse(AcceptTermsException exception) { "Could not accept terms", BAD_REQUEST.getStatusCode(), exception.getMessage(), - exception.getErrorMessages() + exception.getWarnings() ); return Response diff --git a/src/main/java/io/github/genomicdatainfrastructure/daam/api/ApplicationSubmissionExceptionMapper.java b/src/main/java/io/github/genomicdatainfrastructure/daam/api/ApplicationSubmissionExceptionMapper.java index 4967a78..940b6f1 100644 --- a/src/main/java/io/github/genomicdatainfrastructure/daam/api/ApplicationSubmissionExceptionMapper.java +++ b/src/main/java/io/github/genomicdatainfrastructure/daam/api/ApplicationSubmissionExceptionMapper.java @@ -4,8 +4,6 @@ package io.github.genomicdatainfrastructure.daam.api; import io.github.genomicdatainfrastructure.daam.exceptions.ApplicationSubmissionException; -import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; - import io.github.genomicdatainfrastructure.daam.model.ErrorResponse; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; @@ -13,6 +11,8 @@ import jakarta.ws.rs.ext.Provider; import lombok.extern.java.Log; +import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; + @Log @Provider public class ApplicationSubmissionExceptionMapper implements @@ -24,10 +24,9 @@ public Response toResponse(ApplicationSubmissionException exception) { "Application could not be submitted", BAD_REQUEST.getStatusCode(), exception.getMessage(), - exception.getErrorMessages() + exception.getWarnings() ); - log.warning("Application could not be submitted: " + String.join(", ", exception - .getErrorMessages())); + return Response .status(BAD_REQUEST) .entity(errorResponse) diff --git a/src/main/java/io/github/genomicdatainfrastructure/daam/exceptions/AcceptTermsException.java b/src/main/java/io/github/genomicdatainfrastructure/daam/exceptions/AcceptTermsException.java index f252454..a386d33 100644 --- a/src/main/java/io/github/genomicdatainfrastructure/daam/exceptions/AcceptTermsException.java +++ b/src/main/java/io/github/genomicdatainfrastructure/daam/exceptions/AcceptTermsException.java @@ -4,19 +4,21 @@ package io.github.genomicdatainfrastructure.daam.exceptions; +import io.github.genomicdatainfrastructure.daam.model.ValidationWarning; +import lombok.Getter; + import java.util.List; +import static java.util.Optional.ofNullable; + +@Getter public class AcceptTermsException extends RuntimeException { - private static final String MESSAGE = "Terms and Licenses of application %s could not be accepted, due to the following errors:"; - private final List errorMessages; + private static final String MESSAGE = "Terms and Licenses of application %s could not be accepted."; + private final transient List warnings; - public AcceptTermsException(Long applicationId, List errorMessages) { + public AcceptTermsException(Long applicationId, List warnings) { super(String.format(MESSAGE, applicationId)); - this.errorMessages = errorMessages; - } - - public List getErrorMessages() { - return errorMessages; + this.warnings = ofNullable(warnings).orElseGet(List::of); } } diff --git a/src/main/java/io/github/genomicdatainfrastructure/daam/exceptions/ApplicationSubmissionException.java b/src/main/java/io/github/genomicdatainfrastructure/daam/exceptions/ApplicationSubmissionException.java index 4f50c16..39ad93e 100644 --- a/src/main/java/io/github/genomicdatainfrastructure/daam/exceptions/ApplicationSubmissionException.java +++ b/src/main/java/io/github/genomicdatainfrastructure/daam/exceptions/ApplicationSubmissionException.java @@ -3,19 +3,21 @@ // SPDX-License-Identifier: Apache-2.0 package io.github.genomicdatainfrastructure.daam.exceptions; +import io.github.genomicdatainfrastructure.daam.model.ValidationWarning; +import lombok.Getter; + import java.util.List; +import static java.util.Optional.ofNullable; + +@Getter public class ApplicationSubmissionException extends RuntimeException { - private static final String MESSAGE = "Application %s could not be submitted due to the following errors:"; - private final List errorMessages; + private static final String MESSAGE = "Application %s could not be submitted."; + private final transient List warnings; - public ApplicationSubmissionException(Long applicationId, List errorMessages) { + public ApplicationSubmissionException(Long applicationId, List warnings) { super(String.format(MESSAGE, applicationId)); - this.errorMessages = errorMessages; - } - - public List getErrorMessages() { - return errorMessages; + this.warnings = ofNullable(warnings).orElseGet(List::of); } -} \ No newline at end of file +} diff --git a/src/main/java/io/github/genomicdatainfrastructure/daam/services/AcceptTermsService.java b/src/main/java/io/github/genomicdatainfrastructure/daam/services/AcceptTermsService.java index af6d035..f009977 100644 --- a/src/main/java/io/github/genomicdatainfrastructure/daam/services/AcceptTermsService.java +++ b/src/main/java/io/github/genomicdatainfrastructure/daam/services/AcceptTermsService.java @@ -7,9 +7,9 @@ import io.github.genomicdatainfrastructure.daam.exceptions.AcceptTermsException; import io.github.genomicdatainfrastructure.daam.gateways.RemsApiQueryGateway; import io.github.genomicdatainfrastructure.daam.model.AcceptTermsCommand; +import io.github.genomicdatainfrastructure.daam.model.ValidationWarning; import io.github.genomicdatainfrastructure.daam.remote.rems.api.RemsApplicationCommandApi; import io.github.genomicdatainfrastructure.daam.remote.rems.model.AcceptLicensesCommand; -import io.github.genomicdatainfrastructure.daam.remote.rems.model.SuccessResponse; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; import lombok.extern.java.Log; @@ -25,8 +25,7 @@ @ApplicationScoped public class AcceptTermsService { - private static final String ERROR_MESSAGE = "Error: %s"; - private static final String ACCEPT_TERMS_LOG = "Terms and Licenses of application %s could not be accepted, due to the following errors: %s"; + private static final String ACCEPT_TERMS_LOG = "Terms and Licenses of application %s could not be accepted: %s"; private final String remsApiKey; private final RemsApplicationCommandApi remsApplicationCommandApi; @@ -46,11 +45,12 @@ public AcceptTermsService( public void acceptTerms(Long id, String userId, AcceptTermsCommand acceptTermsCommand) { remsApiQueryGateway.checkIfApplicationIsEditableByUser(id, userId); - AcceptLicensesCommand remoteAcceptLicensesCommand = new AcceptLicensesCommand(); - remoteAcceptLicensesCommand.setApplicationId(id); - remoteAcceptLicensesCommand.setAcceptedLicenses(acceptTermsCommand.getAcceptedLicenses()); + var remoteAcceptLicensesCommand = AcceptLicensesCommand.builder() + .applicationId(id) + .acceptedLicenses(acceptTermsCommand.getAcceptedLicenses()) + .build(); - SuccessResponse response = remsApplicationCommandApi.apiApplicationsAcceptLicensesPost( + var response = remsApplicationCommandApi.apiApplicationsAcceptLicensesPost( remsApiKey, userId, remoteAcceptLicensesCommand); if (Boolean.FALSE.equals(response.getSuccess())) { @@ -62,11 +62,15 @@ public void acceptTerms(Long id, String userId, AcceptTermsCommand acceptTermsCo log.warning(ACCEPT_TERMS_LOG.formatted(id, concatenatedErrors)); - var errorMessages = nonNullErrors.stream() - .map(it -> ERROR_MESSAGE.formatted(it)) + var warnings = nonNullErrors.stream() + .map(it -> ValidationWarning.builder() + .key(it.getType()) + .formId(it.getFormId()) + .fieldId(it.getFieldId()) + .build()) .toList(); - throw new AcceptTermsException(id, errorMessages); + throw new AcceptTermsException(id, warnings); } } } diff --git a/src/main/java/io/github/genomicdatainfrastructure/daam/services/SubmitApplicationService.java b/src/main/java/io/github/genomicdatainfrastructure/daam/services/SubmitApplicationService.java index a7f88a0..8a7a414 100644 --- a/src/main/java/io/github/genomicdatainfrastructure/daam/services/SubmitApplicationService.java +++ b/src/main/java/io/github/genomicdatainfrastructure/daam/services/SubmitApplicationService.java @@ -5,26 +5,25 @@ import io.github.genomicdatainfrastructure.daam.exceptions.ApplicationSubmissionException; import io.github.genomicdatainfrastructure.daam.gateways.RemsApiQueryGateway; +import io.github.genomicdatainfrastructure.daam.model.ValidationWarning; import io.github.genomicdatainfrastructure.daam.remote.rems.api.RemsApplicationCommandApi; import io.github.genomicdatainfrastructure.daam.remote.rems.model.SubmitApplicationCommand; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; import lombok.extern.java.Log; - -import static java.util.Optional.ofNullable; -import static java.util.stream.Collectors.joining; +import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.eclipse.microprofile.rest.client.inject.RestClient; import java.util.List; -import org.eclipse.microprofile.config.inject.ConfigProperty; -import org.eclipse.microprofile.rest.client.inject.RestClient; +import static java.util.Optional.ofNullable; +import static java.util.stream.Collectors.joining; @Log @ApplicationScoped public class SubmitApplicationService { - private static final String ERROR_MESSAGE = "Field %s %s"; - private static final String SUBMISSION_LOG = "%s failed to submit application %s due to the following errors: %s"; + private static final String SUBMISSION_LOG = "%s failed to submit application %s: %s"; private final String remsApiKey; private final RemsApplicationCommandApi remsApplicationCommandApi; @@ -41,14 +40,6 @@ public SubmitApplicationService( this.gateway = remsApiQueryGateway; } - private String formatError(String error) { - return switch (error) { - case "t.form.validation/required" -> "is required."; - case "t.form.validation/format_error" -> "has invalid format."; - default -> error; - }; - } - public void submitApplication(Long id, String userId) { gateway.checkIfApplicationIsEditableByUser(id, userId); @@ -69,13 +60,15 @@ public void submitApplication(Long id, String userId) { log.warning(SUBMISSION_LOG.formatted(userId, id, concatenatedErrors)); - var errorMessages = nonNullErrors.stream() - .filter(it -> it.getFieldId() != null) - .filter(it -> it.getType() != null) - .map(it -> ERROR_MESSAGE.formatted(it.getFieldId(), formatError(it.getType()))) + var warnings = nonNullErrors.stream() + .map(it -> ValidationWarning.builder() + .key(it.getType()) + .formId(it.getFormId()) + .fieldId(it.getFieldId()) + .build()) .toList(); - throw new ApplicationSubmissionException(id, errorMessages); + throw new ApplicationSubmissionException(id, warnings); } } } diff --git a/src/main/openapi/daam.yaml b/src/main/openapi/daam.yaml index 2a2bd84..7962bf2 100644 --- a/src/main/openapi/daam.yaml +++ b/src/main/openapi/daam.yaml @@ -191,14 +191,6 @@ paths: application/json: schema: $ref: "#/components/schemas/CreateApplicationResponse" - "422": - description: Validation warnings - content: - application/json: - schema: - type: array - items: - $ref: "#/components/schemas/ValidationWarnings" "404": description: Catalogue Item not found content: @@ -976,27 +968,20 @@ components: title: type: string title: dataset title - ValidationWarnings: - properties: - warnings: - type: array - title: validation warnings - items: - $ref: "#/components/schemas/ValidationWarning" ValidationWarning: properties: key: type: string title: validation key formId: - type: string + type: integer + format: int64 title: form id fieldId: title: field id type: string - fieldValidationKey: - type: string - title: field validation key + required: + - key AddedAttachment: properties: id: @@ -1056,11 +1041,11 @@ components: detail: type: string title: Error detail - errorMessages: + validationWarnings: type: array items: - type: string - description: Additional error messages related to the response + $ref: "#/components/schemas/ValidationWarning" + description: List of validation warnings, to be send to the frontend, where the translation will happen. RetrieveGrantedDatasetIdentifiers: type: object properties: diff --git a/src/main/openapi/rems.yaml b/src/main/openapi/rems.yaml index 6ef9d1e..966710b 100644 --- a/src/main/openapi/rems.yaml +++ b/src/main/openapi/rems.yaml @@ -169,7 +169,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/SubmitApplicationResponse" + $ref: "#/components/schemas/SuccessApplicationResponse" /api/applications/{application-id}: get: tags: @@ -335,7 +335,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/SuccessResponse" + $ref: "#/components/schemas/SuccessApplicationResponse" /api/entitlements: get: tags: @@ -409,7 +409,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/SuccessResponse" + $ref: "#/components/schemas/SuccessApplicationResponse" components: schemas: CatalogueItem: @@ -1281,7 +1281,7 @@ components: format: int64 required: - application-id - SubmitApplicationResponse: + SuccessApplicationResponse: type: object properties: success: diff --git a/src/test/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsTestIT.java b/src/test/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsIT.java similarity index 79% rename from src/test/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsTestIT.java rename to src/test/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsIT.java index ac7f392..63f8175 100644 --- a/src/test/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsTestIT.java +++ b/src/test/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsIT.java @@ -7,5 +7,5 @@ import io.quarkus.test.junit.QuarkusIntegrationTest; @QuarkusIntegrationTest -public class AcceptTermsTestIT extends AcceptTermsTest { +public class AcceptTermsIT extends AcceptTermsTest { } diff --git a/src/test/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsTest.java b/src/test/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsTest.java index 6bf95a5..6783b1b 100644 --- a/src/test/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsTest.java +++ b/src/test/java/io/github/genomicdatainfrastructure/daam/api/AcceptTermsTest.java @@ -4,10 +4,15 @@ package io.github.genomicdatainfrastructure.daam.api; +import io.github.genomicdatainfrastructure.daam.model.ErrorResponse; +import io.github.genomicdatainfrastructure.daam.model.ValidationWarning; import io.quarkus.test.junit.QuarkusTest; import org.junit.jupiter.api.Test; +import java.util.List; + import static io.restassured.RestAssured.given; +import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.Matchers.equalTo; @QuarkusTest @@ -70,16 +75,30 @@ void cannot_accept_terms_when_not_in_submittable_state() { @Test void cannot_accept_terms_when_success_false() { - given() + var response = given() .auth() .oauth2(getAccessToken("alice")) .contentType("application/json") .body("{\"acceptedLicenses\": [3, 4]}") .when() - .post("/api/v1/applications/44/accept-terms") - .then() - .statusCode(400) - .body("title", equalTo("Could not accept terms")); + .post("/api/v1/applications/44/accept-terms"); + + var actual = response.getBody().as(ErrorResponse.class); + + var expected = ErrorResponse.builder() + .status(400) + .title("Could not accept terms") + .detail("Terms and Licenses of application 44 could not be accepted.") + .validationWarnings(List.of( + ValidationWarning.builder() + .key("dummy-error") + .build() + )) + .build(); + + assertThat(actual) + .usingRecursiveComparison() + .isEqualTo(expected); } } diff --git a/src/test/java/io/github/genomicdatainfrastructure/daam/api/SubmitApplicationTest.java b/src/test/java/io/github/genomicdatainfrastructure/daam/api/SubmitApplicationTest.java index 0f72843..b4b8529 100644 --- a/src/test/java/io/github/genomicdatainfrastructure/daam/api/SubmitApplicationTest.java +++ b/src/test/java/io/github/genomicdatainfrastructure/daam/api/SubmitApplicationTest.java @@ -4,10 +4,15 @@ package io.github.genomicdatainfrastructure.daam.api; +import io.github.genomicdatainfrastructure.daam.model.ErrorResponse; +import io.github.genomicdatainfrastructure.daam.model.ValidationWarning; import io.quarkus.test.junit.QuarkusTest; import org.junit.jupiter.api.Test; + +import java.util.List; + import static io.restassured.RestAssured.given; -import static org.hamcrest.Matchers.equalTo; +import static org.assertj.core.api.Assertions.assertThat; @QuarkusTest class SubmitApplicationTest extends BaseTest { @@ -67,13 +72,32 @@ void cannot_submit_application_when_anonymous_request() { @Test void cannot_submit_application_due_to_submission_errors() { - given() + var response = given() .auth() .oauth2(getAccessToken("alice")) .when() - .post("api/v1/applications/44/submit") - .then() - .statusCode(400) - .body("title", equalTo("Application could not be submitted")); + .post("api/v1/applications/44/submit"); + + var actual = response.getBody().as(ErrorResponse.class); + + var expected = ErrorResponse.builder() + .status(400) + .title("Application could not be submitted") + .detail("Application 44 could not be submitted.") + .validationWarnings(List.of( + ValidationWarning.builder() + .key("Missing") + .formId(1L) + .fieldId("requiredField") + .build(), + ValidationWarning.builder() + .key("not-accepted-licenses") + .build() + )) + .build(); + + assertThat(actual) + .usingRecursiveComparison() + .isEqualTo(expected); } } diff --git a/src/test/java/io/github/genomicdatainfrastructure/daam/services/AcceptTermsServiceTest.java b/src/test/java/io/github/genomicdatainfrastructure/daam/services/AcceptTermsServiceTest.java index 59d7d21..1eac99e 100644 --- a/src/test/java/io/github/genomicdatainfrastructure/daam/services/AcceptTermsServiceTest.java +++ b/src/test/java/io/github/genomicdatainfrastructure/daam/services/AcceptTermsServiceTest.java @@ -9,7 +9,8 @@ import io.github.genomicdatainfrastructure.daam.model.AcceptTermsCommand; import io.github.genomicdatainfrastructure.daam.remote.rems.api.RemsApplicationCommandApi; import io.github.genomicdatainfrastructure.daam.remote.rems.model.AcceptLicensesCommand; -import io.github.genomicdatainfrastructure.daam.remote.rems.model.SuccessResponse; +import io.github.genomicdatainfrastructure.daam.remote.rems.model.SuccessApplicationResponse; +import io.github.genomicdatainfrastructure.daam.remote.rems.model.ValidationField; import jakarta.ws.rs.WebApplicationException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -41,11 +42,13 @@ void setUp() { void can_accept_terms() { var applicationId = 1L; var userId = "userId"; - AcceptTermsCommand command = new AcceptTermsCommand(); - command.setAcceptedLicenses(List.of(1L, 2L)); + var command = AcceptTermsCommand.builder() + .acceptedLicenses(List.of(1L, 2L)) + .build(); - SuccessResponse response = new SuccessResponse(); - response.setSuccess(true); + var response = SuccessApplicationResponse.builder() + .success(true) + .build(); when(remsApplicationCommandApi.apiApplicationsAcceptLicensesPost( eq(REMS_API_KEY), eq(userId), any(AcceptLicensesCommand.class))) @@ -65,9 +68,14 @@ void throws_exception_when_accept_terms_fails() { AcceptTermsCommand command = new AcceptTermsCommand(); command.setAcceptedLicenses(List.of(1L, 2L)); - SuccessResponse response = new SuccessResponse(); - response.setSuccess(false); - response.setErrors(List.of("Error 1", "Error 2")); + var response = SuccessApplicationResponse.builder() + .success(false) + .errors(List.of( + ValidationField.builder() + .type("test") + .build() + )) + .build(); when(remsApplicationCommandApi.apiApplicationsAcceptLicensesPost( eq(REMS_API_KEY), eq(userId), any(AcceptLicensesCommand.class))) diff --git a/src/test/resources/mappings/accept_terms_success_false.json b/src/test/resources/mappings/accept_terms_success_false.json index 22360a5..d228e13 100644 --- a/src/test/resources/mappings/accept_terms_success_false.json +++ b/src/test/resources/mappings/accept_terms_success_false.json @@ -18,7 +18,11 @@ }, "jsonBody": { "success": false, - "errors": [] + "errors": [ + { + "type": "dummy-error" + } + ] } }, "priority": 1 diff --git a/src/test/resources/mappings/submit_application_for_success_false.json b/src/test/resources/mappings/submit_application_for_success_false.json index ae66a54..a2d6953 100644 --- a/src/test/resources/mappings/submit_application_for_success_false.json +++ b/src/test/resources/mappings/submit_application_for_success_false.json @@ -20,6 +20,9 @@ "field-id": "requiredField", "type": "Missing", "form-id": 1 + }, + { + "type": "not-accepted-licenses" } ], "warnings": [],