From 001ad17c9c0b32dc0a8511176d96ddf4a7ad2e17 Mon Sep 17 00:00:00 2001 From: Niels Erik Nielsen Date: Wed, 10 Jan 2024 22:15:01 +0100 Subject: [PATCH] Bug-fixes, transformation POST/PUT (MODHAADM-68) - allow (and ignore) property 'acl' for transformations like for other record types - fix class cast exception that prevents optional resolution of steps by unique name (instead of by ID) - resolve/expand associated steps from provided unique names or IDs to complete step association objects in compliance with the legacy API for transformation objects on PUTs (as already done on POSTs) --- .../dataaccess/LegacyHarvesterStorage.java | 123 +++++++++++++----- .../schemas/transformationPostPut.json | 5 + 2 files changed, 93 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/folio/harvesteradmin/dataaccess/LegacyHarvesterStorage.java b/src/main/java/org/folio/harvesteradmin/dataaccess/LegacyHarvesterStorage.java index 6584aff..1f18670 100644 --- a/src/main/java/org/folio/harvesteradmin/dataaccess/LegacyHarvesterStorage.java +++ b/src/main/java/org/folio/harvesteradmin/dataaccess/LegacyHarvesterStorage.java @@ -14,6 +14,8 @@ import static org.folio.harvesteradmin.dataaccess.statics.RequestParameters.supportedGetRequestParameters; import static org.folio.okapi.common.HttpResponse.responseText; +import io.vertx.core.AsyncResult; +import io.vertx.core.CompositeFuture; import io.vertx.core.Future; import io.vertx.core.Promise; import io.vertx.core.Vertx; @@ -140,7 +142,7 @@ public Future getConfigRecordById( return promise.future(); } - private Future getConfigRecordByIdOrName( + private Future getUniqueConfigRecordByIdOrName( String harvesterPath, String id, String name) { Promise promise = Promise.promise(); if (id == null || id.isEmpty()) { @@ -160,13 +162,13 @@ private Future getConfigRecordByIdOrName( promise.complete( new ProcessedHarvesterResponseGetUniqueByName( recordsByName.jsonObject(), - 422, + 404, "Record with name \"" + name + "\" not found", recordsFoundByName)); } else { promise.complete( new ProcessedHarvesterResponseGetUniqueByName( recordsByName.jsonObject(), - 422, + 404, "Found multiple records with name \"" + name + "\"", recordsFoundByName)); } return promise.future(); @@ -249,7 +251,7 @@ public Future resolveReferencedEntities( final String transformationId = entity.getJsonObject("transformation").getString("id"); final String storageName = entity.getJsonObject("storage").getString("name"); final String transformationName = entity.getJsonObject("transformation").getString("name"); - getConfigRecordByIdOrName(HARVESTER_STORAGES_PATH, storageId, storageName) + getUniqueConfigRecordByIdOrName(HARVESTER_STORAGES_PATH, storageId, storageName) .onComplete(storage -> { if (storage.succeeded()) { if (storage.result().wasOK()) { @@ -265,7 +267,7 @@ public Future resolveReferencedEntities( fatalError.add("Error looking up storage by id or name " + storage.cause().getMessage()); } - getConfigRecordByIdOrName(HARVESTER_TRANSFORMATIONS_PATH, + getUniqueConfigRecordByIdOrName(HARVESTER_TRANSFORMATIONS_PATH, transformationId, transformationName) .onComplete(transformation -> { if (transformation.succeeded()) { @@ -278,7 +280,7 @@ public Future resolveReferencedEntities( constraintViolation.add(transformation.result().errorMessage()); } - if (constraintViolation.size() > 0) { + if (!constraintViolation.isEmpty()) { promise.complete( new ProcessedHarvesterResponsePost(422, constraintViolation.toString()) ); @@ -292,7 +294,7 @@ public Future resolveReferencedEntities( + transformation.cause().getMessage()); } }); - if (fatalError.size() > 0) { + if (!fatalError.isEmpty()) { promise.complete(new ProcessedHarvesterResponsePost(500,fatalError.toString())); } }); @@ -408,8 +410,12 @@ public Future putConfigRecord(RoutingContext rout String harvesterPath = mapToHarvesterPath(routingContext); JsonObject jsonToPut = routingContext.body().asJsonObject(); String id = routingContext.request().getParam("id"); - if (harvesterPath != null && harvesterPath.equals(HARVESTER_HARVESTABLES_PATH)) { - jsonToPut.put("lastUpdated", iso_instant.format(Instant.now())); + if (harvesterPath != null) { + if (harvesterPath.equals(HARVESTER_TRANSFORMATIONS_PATH)) { + return putTransformation(routingContext); + } else if (harvesterPath.equals(HARVESTER_HARVESTABLES_PATH)) { + jsonToPut.put("lastUpdated", iso_instant.format(Instant.now())); + } } return putConfigRecord(routingContext, harvesterPath, jsonToPut, id); } @@ -580,12 +586,41 @@ public Future checkForReferencingEntities(String api, String id) { return promise.future(); } - - - private Future doPostAndPutTransformation( - RoutingContext routingContext) { + private Future putTransformation(RoutingContext routingContext) { JsonObject transformationJson = routingContext.body().asJsonObject(); - logger.debug("About to POST-then-PUT " + transformationJson.encodePrettily()); + logger.debug("About to PUT " + transformationJson.encodePrettily()); + List> stepFutures = getStepLookupFutures(transformationJson); + Promise promise = Promise.promise(); + GenericCompositeFuture.all(stepFutures).onComplete(steps -> { + if (steps.succeeded()) { + boolean allStepsFound = true; + for (int h = 0; h < steps.result().size(); h++) { + ProcessedHarvesterResponse stepResponse = steps.result().resultAt(h); + if (stepResponse.statusCode() == NOT_FOUND) { + allStepsFound = false; + promise.complete(new ProcessedHarvesterResponsePut(422, + "Uniquely referenced step not found, cannot store transformation pipeline: " + + stepResponse.errorMessage())); + break; + } + } + if (allStepsFound) { + expandAssociatedSteps(steps, transformationJson); + putConfigRecord( + routingContext, + HARVESTER_TRANSFORMATIONS_PATH, + transformationJson, + transformationJson.getString("id")).onComplete(response -> + promise.complete(response.result()) + ); + } + } + }); + return promise.future(); + } + + private List> getStepLookupFutures( + JsonObject transformationJson) { JsonArray stepsIdsJson = transformationJson.containsKey("stepAssociations") ? transformationJson.getJsonArray( "stepAssociations").copy() : new JsonArray(); @@ -599,19 +634,32 @@ private Future doPostAndPutTransformation( String stepName = step.containsKey("step") ? step.getJsonObject("step").getString("name") : step.getString("stepName"); - stepFutures.add(getConfigRecordByIdOrName(HARVESTER_STEPS_PATH, stepId, stepName)); + stepFutures.add(getUniqueConfigRecordByIdOrName(HARVESTER_STEPS_PATH, stepId, stepName)); } + return stepFutures; + } + + /** + * Checks that referenced steps exist, POSTs the transformation without the steps, + * creates schema compliant step associations in the transformation object, + * PUTs the transformation, checks that a transformation with the given ID exists. + * @return response structure + */ + private Future doPostAndPutTransformation( + RoutingContext routingContext) { + JsonObject transformationJson = routingContext.body().asJsonObject(); + logger.debug("About to POST-then-PUT " + transformationJson.encodePrettily()); + List> stepFutures = getStepLookupFutures(transformationJson); Promise promise = Promise.promise(); GenericCompositeFuture.all(stepFutures).onComplete(steps -> { if (steps.succeeded()) { boolean allStepsFound = true; for (int h = 0; h < steps.result().size(); h++) { - ProcessedHarvesterResponseGetById stepResponse = steps.result().resultAt(h); + ProcessedHarvesterResponse stepResponse = steps.result().resultAt(h); if (stepResponse.statusCode() == NOT_FOUND) { - logger.info("Step not found: " + stepResponse.errorMessage()); allStepsFound = false; promise.complete(new ProcessedHarvesterResponsePost(422, - "Referenced step not found, cannot store transformation pipeline: " + "Uniquely referenced step not found, cannot store transformation pipeline: " + stepResponse.errorMessage())); break; } @@ -625,21 +673,7 @@ private Future doPostAndPutTransformation( if (transformationPost.succeeded() && transformationPost.result().statusCode() == CREATED) { JsonObject createdTransformation = transformationPost.result().jsonObject(); - createdTransformation.put("stepAssociations", new JsonArray()); - for (int i = 0; i < steps.result().size(); i++) { - ProcessedHarvesterResponseGetById stepResponse = steps.result().resultAt(i); - final JsonObject stepJson = stepResponse.jsonObject(); - JsonObject tsaJson = new JsonObject(); - tsaJson.put("id", getRandomFifteenDigitString()); - tsaJson.put("position", Integer.toString(i + 1)); - tsaJson.put("step", new JsonObject()); - tsaJson.getJsonObject("step") - .put("entityType", - typeToEmbeddedTypeMap.get(stepJson.getString("type"))); - tsaJson.getJsonObject("step").put("id", stepJson.getString("id")); - tsaJson.put("transformation", createdTransformation.getString("id")); - createdTransformation.getJsonArray("stepAssociations").add(tsaJson); - } + expandAssociatedSteps(steps, createdTransformation); putConfigRecord( routingContext, HARVESTER_TRANSFORMATIONS_PATH, @@ -687,6 +721,25 @@ private Future doPostAndPutTransformation( return promise.future(); } + private static void expandAssociatedSteps(AsyncResult steps, + JsonObject createdTransformation) { + createdTransformation.put("stepAssociations", new JsonArray()); + for (int i = 0; i < steps.result().size(); i++) { + ProcessedHarvesterResponse stepResponse = steps.result().resultAt(i); + final JsonObject stepJson = stepResponse.jsonObject(); + JsonObject tsaJson = new JsonObject(); + tsaJson.put("id", getRandomFifteenDigitString()); + tsaJson.put("position", Integer.toString(i + 1)); + tsaJson.put("step", new JsonObject()); + tsaJson.getJsonObject("step") + .put("entityType", + typeToEmbeddedTypeMap.get(stepJson.getString("type"))); + tsaJson.getJsonObject("step").put("id", stepJson.getString("id")); + tsaJson.put("transformation", createdTransformation.getString("id")); + createdTransformation.getJsonArray("stepAssociations").add(tsaJson); + } + } + private Future doPostTsaPutTransformation( RoutingContext routingContext) { JsonObject incomingTsa = routingContext.body().asJsonObject(); @@ -695,7 +748,7 @@ private Future doPostTsaPutTransformation( String stepId = incomingTsa.getJsonObject("step").getString("id"); String stepName = incomingTsa.getJsonObject("step").getString("name"); Promise promise = Promise.promise(); - getConfigRecordByIdOrName(HARVESTER_TRANSFORMATIONS_PATH, transId, transName).onComplete( + getUniqueConfigRecordByIdOrName(HARVESTER_TRANSFORMATIONS_PATH, transId, transName).onComplete( theTransformation -> { if (theTransformation.failed()) { promise.complete( @@ -710,7 +763,7 @@ private Future doPostTsaPutTransformation( + transId + " not found.")); } else { JsonObject transformationFound = theTransformation.result().jsonObject(); - getConfigRecordByIdOrName(HARVESTER_STEPS_PATH, stepId, stepName).onComplete( + getUniqueConfigRecordByIdOrName(HARVESTER_STEPS_PATH, stepId, stepName).onComplete( theStep -> { if (!theStep.result().found()) { promise.complete( diff --git a/src/main/resources/openapi/schemas/transformationPostPut.json b/src/main/resources/openapi/schemas/transformationPostPut.json index 6f57b44..27d217a 100644 --- a/src/main/resources/openapi/schemas/transformationPostPut.json +++ b/src/main/resources/openapi/schemas/transformationPostPut.json @@ -6,6 +6,11 @@ "type": "string", "description": "Unique record identifier." }, + "acl": { + "type": "string", + "description": "System controlled access control string.", + "readOnly": true + }, "name": { "type": "string", "description": "Name of the transformation pipeline."