This is an automated email from the ASF dual-hosted git repository. arnold pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/fineract.git
The following commit(s) were added to refs/heads/develop by this push: new 20d96d954 [FINERACT-1852] Swagger schema inconsistency for Business Step Config APIs 20d96d954 is described below commit 20d96d954295dc6394475a7fdd3165bdcb62184a Author: taskain7 <task...@gmail.com> AuthorDate: Fri Jan 13 02:17:46 2023 +0100 [FINERACT-1852] Swagger schema inconsistency for Business Step Config APIs --- .../cob/api/ConfigureBusinessStepResource.java | 25 +++++++++++----------- .../data/ConfiguredJobNamesDTO.java} | 17 +++++++-------- .../SynchronousCommandProcessingService.java | 2 +- .../IdempotentCommandProcessSucceedException.java | 9 +++++++- ...potentCommandProcessSucceedExceptionMapper.java | 9 +++++--- .../core/filters/IdempotencyStoreFilter.java | 5 +++-- .../BusinessConfigurationApiTest.java | 9 ++++---- .../fineract/integrationtests/IdempotencyTest.java | 17 ++++++++------- 8 files changed, 52 insertions(+), 41 deletions(-) diff --git a/fineract-provider/src/main/java/org/apache/fineract/cob/api/ConfigureBusinessStepResource.java b/fineract-provider/src/main/java/org/apache/fineract/cob/api/ConfigureBusinessStepResource.java index 9fa37171b..7d1d6f40b 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/cob/api/ConfigureBusinessStepResource.java +++ b/fineract-provider/src/main/java/org/apache/fineract/cob/api/ConfigureBusinessStepResource.java @@ -18,10 +18,8 @@ */ package org.apache.fineract.cob.api; -import com.google.gson.Gson; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Parameter; -import io.swagger.v3.oas.annotations.media.ArraySchema; import io.swagger.v3.oas.annotations.media.Content; import io.swagger.v3.oas.annotations.media.Schema; import io.swagger.v3.oas.annotations.parameters.RequestBody; @@ -40,8 +38,10 @@ import javax.ws.rs.PathParam; import javax.ws.rs.Produces; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import lombok.RequiredArgsConstructor; +import org.apache.fineract.cob.data.ConfiguredJobNamesDTO; import org.apache.fineract.cob.data.JobBusinessStepConfigData; import org.apache.fineract.cob.data.JobBusinessStepDetail; import org.apache.fineract.cob.service.ConfigJobParameterService; @@ -49,7 +49,6 @@ import org.apache.fineract.commands.domain.CommandWrapper; import org.apache.fineract.commands.service.CommandWrapperBuilder; import org.apache.fineract.commands.service.PortfolioCommandSourceWritePlatformService; import org.apache.fineract.infrastructure.core.api.ApiRequestParameterHelper; -import org.apache.fineract.infrastructure.core.data.CommandProcessingResult; import org.apache.fineract.infrastructure.core.serialization.ApiRequestJsonSerializationSettings; import org.apache.fineract.infrastructure.core.serialization.DefaultToApiJsonSerializer; import org.springframework.stereotype.Component; @@ -67,6 +66,7 @@ public class ConfigureBusinessStepResource { private final DefaultToApiJsonSerializer<JobBusinessStepConfigData> businessStepConfigSerializeService; private final DefaultToApiJsonSerializer<JobBusinessStepDetail> businessStepDetailSerializeService; + private final DefaultToApiJsonSerializer<ConfiguredJobNamesDTO> configuredJobNamesSerializer; private final ApiRequestParameterHelper apiRequestParameterHelper; private final ConfigJobParameterService configJobParameterService; private final PortfolioCommandSourceWritePlatformService commandWritePlatformService; @@ -76,13 +76,13 @@ public class ConfigureBusinessStepResource { @Produces({ MediaType.APPLICATION_JSON }) @Operation(summary = "List Business Jobs", description = "Returns the configured Business Jobs") @ApiResponses({ - @ApiResponse(responseCode = "200", description = "OK", content = @Content(array = @ArraySchema(schema = @Schema(implementation = ConfigureBusinessStepResourceSwagger.GetBusinessJobConfigResponse.class)))) }) + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = ConfigureBusinessStepResourceSwagger.GetBusinessJobConfigResponse.class))) }) public String retrieveAllConfiguredBusinessJobs(@Context final UriInfo uriInfo) { List<String> businessJobNames = configJobParameterService.getAllConfiguredJobNames(); - final Gson gson = new Gson(); + ConfiguredJobNamesDTO result = new ConfiguredJobNamesDTO(businessJobNames); - return gson.toJson(businessJobNames); + return configuredJobNamesSerializer.serialize(result); } @GET @@ -91,7 +91,7 @@ public class ConfigureBusinessStepResource { @Produces({ MediaType.APPLICATION_JSON }) @Operation(summary = "List Business Step Configurations for a Job", description = "Returns the configured Business Steps for a job") @ApiResponses({ - @ApiResponse(responseCode = "200", description = "OK", content = @Content(array = @ArraySchema(schema = @Schema(implementation = ConfigureBusinessStepResourceSwagger.GetBusinessStepConfigResponse.class)))) }) + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = ConfigureBusinessStepResourceSwagger.GetBusinessStepConfigResponse.class))) }) public String retrieveAllConfiguredBusinessStep(@Context final UriInfo uriInfo, @PathParam("jobName") @Parameter(description = "jobName") final String jobName) { @@ -108,15 +108,14 @@ public class ConfigureBusinessStepResource { @Produces({ MediaType.APPLICATION_JSON }) @Operation(summary = "List Business Step Configurations for a Job", description = "Updates the Business steps execution order for a job") @RequestBody(content = @Content(schema = @Schema(implementation = ConfigureBusinessStepResourceSwagger.UpdateBusinessStepConfigRequest.class))) - @ApiResponses({ - @ApiResponse(responseCode = "204", description = "OK", content = @Content(array = @ArraySchema(schema = @Schema(implementation = ConfigureBusinessStepResourceSwagger.GetBusinessStepConfigResponse.class)))) }) - public String updateJobBusinessStepConfig(@PathParam("jobName") @Parameter(description = "jobName") final String jobName, + @ApiResponses({ @ApiResponse(responseCode = "204", description = "NO_CONTENT") }) + public Response updateJobBusinessStepConfig(@PathParam("jobName") @Parameter(description = "jobName") final String jobName, @Parameter(hidden = true) final String jsonRequestBody) { final CommandWrapper commandRequest = new CommandWrapperBuilder().updateBusinessStepConfig(jobName).withJson(jsonRequestBody) .build(); - CommandProcessingResult result = commandWritePlatformService.logCommandSource(commandRequest); - return businessStepConfigSerializeService.serialize(result); + commandWritePlatformService.logCommandSource(commandRequest); + return Response.status(Response.Status.NO_CONTENT).build(); } @GET @@ -125,7 +124,7 @@ public class ConfigureBusinessStepResource { @Produces({ MediaType.APPLICATION_JSON }) @Operation(summary = "List Business Step Configurations for a Job", description = "Returns the available Business Steps for a job") @ApiResponses({ - @ApiResponse(responseCode = "200", description = "OK", content = @Content(array = @ArraySchema(schema = @Schema(implementation = ConfigureBusinessStepResourceSwagger.GetBusinessStepConfigResponse.class)))) }) + @ApiResponse(responseCode = "200", description = "OK", content = @Content(schema = @Schema(implementation = ConfigureBusinessStepResourceSwagger.GetBusinessStepConfigResponse.class))) }) public String retrieveAllAvailableBusinessStep(@Context final UriInfo uriInfo, @PathParam("jobName") @Parameter(description = "jobName") final String jobName) { diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exception/IdempotentCommandProcessSucceedException.java b/fineract-provider/src/main/java/org/apache/fineract/cob/data/ConfiguredJobNamesDTO.java similarity index 61% copy from fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exception/IdempotentCommandProcessSucceedException.java copy to fineract-provider/src/main/java/org/apache/fineract/cob/data/ConfiguredJobNamesDTO.java index 5b84ad550..60951f98a 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exception/IdempotentCommandProcessSucceedException.java +++ b/fineract-provider/src/main/java/org/apache/fineract/cob/data/ConfiguredJobNamesDTO.java @@ -16,16 +16,15 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.fineract.infrastructure.core.exception; +package org.apache.fineract.cob.data; -import org.apache.fineract.commands.domain.CommandWrapper; +import java.util.List; +import lombok.AllArgsConstructor; +import lombok.Data; -/** - * Exception thrown when command is sent with same action, entity and idempotency key - */ -public class IdempotentCommandProcessSucceedException extends AbstractIdempotentCommandException { +@Data +@AllArgsConstructor +public class ConfiguredJobNamesDTO { - public IdempotentCommandProcessSucceedException(CommandWrapper wrapper, String response) { - super(wrapper.actionName(), wrapper.entityName(), wrapper.getIdempotencyKey(), response); - } + private List<String> businessJobs; } diff --git a/fineract-provider/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java b/fineract-provider/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java index 941d6869b..e95a702d1 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java +++ b/fineract-provider/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java @@ -165,7 +165,7 @@ public class SynchronousCommandProcessingService implements CommandProcessingSer command -> new IdempotentCommandProcessUnderProcessingException(wrapper)); idempotentExceptionByStatus(ERROR, existingCommand, command -> new IdempotentCommandProcessFailedException(wrapper, command)); idempotentExceptionByStatus(PROCESSED, existingCommand, - command -> new IdempotentCommandProcessSucceedException(wrapper, command.getResult())); + command -> new IdempotentCommandProcessSucceedException(wrapper, command.getResult(), command.getResultStatusCode())); } } diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exception/IdempotentCommandProcessSucceedException.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exception/IdempotentCommandProcessSucceedException.java index 5b84ad550..d7437275b 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exception/IdempotentCommandProcessSucceedException.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exception/IdempotentCommandProcessSucceedException.java @@ -25,7 +25,14 @@ import org.apache.fineract.commands.domain.CommandWrapper; */ public class IdempotentCommandProcessSucceedException extends AbstractIdempotentCommandException { - public IdempotentCommandProcessSucceedException(CommandWrapper wrapper, String response) { + private final Integer statusCode; + + public IdempotentCommandProcessSucceedException(CommandWrapper wrapper, String response, Integer statusCode) { super(wrapper.actionName(), wrapper.entityName(), wrapper.getIdempotencyKey(), response); + this.statusCode = statusCode; + } + + public Integer getStatusCode() { + return statusCode; } } diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/IdempotentCommandProcessSucceedExceptionMapper.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/IdempotentCommandProcessSucceedExceptionMapper.java index 7d5494a90..a1778cafa 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/IdempotentCommandProcessSucceedExceptionMapper.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/IdempotentCommandProcessSucceedExceptionMapper.java @@ -20,7 +20,6 @@ package org.apache.fineract.infrastructure.core.exceptionmapper; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; -import javax.ws.rs.core.Response.Status; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; import lombok.extern.slf4j.Slf4j; @@ -36,7 +35,11 @@ public class IdempotentCommandProcessSucceedExceptionMapper implements Exception @Override public Response toResponse(final IdempotentCommandProcessSucceedException exception) { log.debug("Idempotent processing success request: {}", exception.getMessage()); - return Response.status(Status.OK).entity(exception.getResponse()) - .header(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER, "true").type(MediaType.APPLICATION_JSON).build(); + Response.ResponseBuilder responseBuilder = Response.status(exception.getStatusCode()); + if (exception.getResponse() != null) { + responseBuilder.entity(exception.getResponse()); + } + return responseBuilder.header(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER, "true").type(MediaType.APPLICATION_JSON) + .build(); } } diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilter.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilter.java index 5072e2710..52d4d2f97 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilter.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilter.java @@ -70,7 +70,8 @@ public class IdempotencyStoreFilter extends OncePerRequestFilter implements Batc boolean isSuccessWithoutStored = isStoreIdempotencyKey(request) && commandId.isPresent() && isAllowedContentTypeResponse(response) && wrapper.getValue() != null; if (isSuccessWithoutStored) { - storeCommandResult(response.getStatus(), new String(wrapper.getValue().getContentAsByteArray(), StandardCharsets.UTF_8), + storeCommandResult(response.getStatus(), Optional.ofNullable(wrapper.getValue()) + .map(ContentCachingResponseWrapper::getContentAsByteArray).map(s -> new String(s, StandardCharsets.UTF_8)).orElse(null), commandId); } if (wrapper.getValue() != null) { @@ -92,7 +93,7 @@ public class IdempotencyStoreFilter extends OncePerRequestFilter implements Batc private boolean isAllowedContentTypeResponse(HttpServletResponse response) { return Optional.ofNullable(response.getContentType()).map(String::toLowerCase).map(ct -> ct.contains("application/json")) - .orElse(false); + .orElse(false) || (response.getStatus() > 200 && response.getStatus() < 300); } private boolean isAllowedContentTypeRequest(HttpServletRequest request) { diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/BusinessConfigurationApiTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/BusinessConfigurationApiTest.java index d6e9ef859..a0fdafcdd 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/BusinessConfigurationApiTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/BusinessConfigurationApiTest.java @@ -83,12 +83,13 @@ public class BusinessConfigurationApiTest { @Test public void shouldUpdateStepOrder() { + ResponseSpecification updateResponseSpec = new ResponseSpecBuilder().expectStatusCode(204).build(); JobBusinessStepConfigData originalStepConfig = BusinessStepConfigurationHelper.getConfiguredBusinessStepsByJobName(requestSpec, responseSpec, LOAN_JOB_NAME); List<BusinessStep> requestBody = new ArrayList<>(); requestBody.add(getBusinessSteps(1L, APPLY_CHARGE_TO_OVERDUE_LOANS)); - BusinessStepConfigurationHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + BusinessStepConfigurationHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME, BusinessStepConfigurationHelper.toJsonString(requestBody)); JobBusinessStepConfigData newStepConfig = BusinessStepConfigurationHelper.getConfiguredBusinessStepsByJobName(requestSpec, @@ -100,7 +101,7 @@ public class BusinessConfigurationApiTest { requestBody.add(getBusinessSteps(2L, LOAN_DELINQUENCY_CLASSIFICATION)); - BusinessStepConfigurationHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + BusinessStepConfigurationHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME, BusinessStepConfigurationHelper.toJsonString(requestBody)); newStepConfig = BusinessStepConfigurationHelper.getConfiguredBusinessStepsByJobName(requestSpec, responseSpec, LOAN_JOB_NAME); applyChargeStep = newStepConfig.getBusinessSteps().stream() @@ -112,7 +113,7 @@ public class BusinessConfigurationApiTest { assertEquals(2L, loanDelinquencyStep.getOrder()); requestBody.remove(1); - BusinessStepConfigurationHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + BusinessStepConfigurationHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME, BusinessStepConfigurationHelper.toJsonString(requestBody)); newStepConfig = BusinessStepConfigurationHelper.getConfiguredBusinessStepsByJobName(requestSpec, responseSpec, LOAN_JOB_NAME); @@ -121,7 +122,7 @@ public class BusinessConfigurationApiTest { assertEquals(1, newStepConfig.getBusinessSteps().size()); assertEquals(1L, applyChargeStep.getOrder()); - BusinessStepConfigurationHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + BusinessStepConfigurationHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME, BusinessStepConfigurationHelper.toJsonString(originalStepConfig.getBusinessSteps())); } diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/IdempotencyTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/IdempotencyTest.java index f550fdb3a..ee2b2c1a6 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/IdempotencyTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/IdempotencyTest.java @@ -58,6 +58,7 @@ public class IdempotencyTest { @Test public void shouldUpdateStepOrder() { + ResponseSpecification updateResponseSpec = new ResponseSpecBuilder().expectStatusCode(204).build(); JobBusinessStepConfigData originalStepConfig = IdempotencyHelper.getConfiguredBusinessStepsByJobName(requestSpec, responseSpec, LOAN_JOB_NAME); @@ -65,9 +66,9 @@ public class IdempotencyTest { List<BusinessStep> requestBody = new ArrayList<>(); requestBody.add(getBusinessSteps(1L, APPLY_CHARGE_TO_OVERDUE_LOANS)); - Response response = IdempotencyHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + Response response = IdempotencyHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME, IdempotencyHelper.toJsonString(requestBody), idempotencyKeyHeader); - Response responseSecond = IdempotencyHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + Response responseSecond = IdempotencyHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME, IdempotencyHelper.toJsonString(requestBody), idempotencyKeyHeader); Assertions.assertEquals(response.getBody().asString(), responseSecond.getBody().asString()); Assertions.assertNull(response.header(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER)); @@ -84,9 +85,9 @@ public class IdempotencyTest { requestBody.add(getBusinessSteps(2L, LOAN_DELINQUENCY_CLASSIFICATION)); - Response update = IdempotencyHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + Response update = IdempotencyHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME, IdempotencyHelper.toJsonString(requestBody), idempotencyKeyHeader); - Response updateSecond = IdempotencyHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + Response updateSecond = IdempotencyHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME, IdempotencyHelper.toJsonString(requestBody), idempotencyKeyHeader); Assertions.assertNull(update.header(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER)); Assertions.assertNotNull(updateSecond.header(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER)); @@ -103,9 +104,9 @@ public class IdempotencyTest { requestBody.remove(1); idempotencyKeyHeader = UUID.randomUUID().toString(); - update = IdempotencyHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + update = IdempotencyHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME, IdempotencyHelper.toJsonString(requestBody), idempotencyKeyHeader); - updateSecond = IdempotencyHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + updateSecond = IdempotencyHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME, IdempotencyHelper.toJsonString(requestBody), idempotencyKeyHeader); Assertions.assertNull(update.header(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER)); @@ -120,9 +121,9 @@ public class IdempotencyTest { idempotencyKeyHeader = UUID.randomUUID().toString(); - update = IdempotencyHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + update = IdempotencyHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME, IdempotencyHelper.toJsonString(originalStepConfig.getBusinessSteps()), idempotencyKeyHeader); - updateSecond = IdempotencyHelper.updateBusinessStepOrder(requestSpec, responseSpec, LOAN_JOB_NAME, + updateSecond = IdempotencyHelper.updateBusinessStepOrder(requestSpec, updateResponseSpec, LOAN_JOB_NAME, IdempotencyHelper.toJsonString(originalStepConfig.getBusinessSteps()), idempotencyKeyHeader); Assertions.assertNull(update.header(AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER));