diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/AutomaticValidationTests.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/AutomaticValidationTests.java index d59018b489..b86b03fe57 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/AutomaticValidationTests.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/AutomaticValidationTests.java @@ -12,6 +12,7 @@ import org.junit.jupiter.api.TestInstance; import javax.json.Json; +import javax.json.JsonValue; import java.io.IOException; import java.util.List; @@ -130,6 +131,30 @@ void instantiationError() throws IOException, InterruptedException { ), activityValidation); } + @Test + void noSuchMissionModelError() throws IOException, InterruptedException { + final var activityId = hasura.insertActivity( + planId, + "BiteBanana", + "1h", + JsonValue.EMPTY_JSON_OBJECT + ); + Thread.sleep(1000); // TODO consider a while loop here + + hasura.deleteMissionModel(modelId); + + final var arguments = Json.createObjectBuilder().add("biteSize", 2).build(); + hasura.updateActivityDirectiveArguments(planId, activityId, arguments); + Thread.sleep(1000); // TODO consider a while loop here + + final var activityValidations = hasura.getActivityValidations(planId); + final ActivityValidation activityValidation = activityValidations.get((long) activityId); + assertEquals( + new ActivityValidation.NoSuchMissionModelFailure("no such mission model", "0"), + activityValidation + ); + } + @Test void exceptionDuringValidationHandled() throws IOException, InterruptedException { final var exceptionActivityId = hasura.insertActivity( diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ActivityValidation.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ActivityValidation.java index 599e84d394..63fd59f92d 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ActivityValidation.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ActivityValidation.java @@ -11,6 +11,7 @@ record Success() implements ActivityValidation {} record InstantiationFailure(List extraneousArguments, List missingArguments, List unconstructableArguments) implements ActivityValidation {} record ValidationFailure(List notices) implements ActivityValidation {} record NoSuchActivityTypeFailure(String message, String activityType) implements ActivityValidation {} + record NoSuchMissionModelFailure(String message, String modelId) implements ActivityValidation {} record ValidationNotice(List subjects, String message) { } record UnconstructableArgument(String name, String failure) { } @@ -44,6 +45,10 @@ static ActivityValidation fromJSON(JsonObject obj) { getStringArray($, "subjects"), $.asJsonObject().getString("message")))); case "NO_SUCH_ACTIVITY_TYPE" -> new NoSuchActivityTypeFailure(errors.getJsonObject("noSuchActivityError").getString("message"), errors.getJsonObject("noSuchActivityError").getString("activity_type")); + case "NO_SUCH_MISSION_MODEL" -> new NoSuchMissionModelFailure( + errors.getJsonObject("noSuchMissionModelError").getString("message"), + errors.getJsonObject("noSuchMissionModelError").getString("mission_model_id") + ); default -> throw new RuntimeException("Unhandled error type: " + type); }; } diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/GQL.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/GQL.java index e2f6ace7ec..401a6ef8af 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/GQL.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/GQL.java @@ -501,6 +501,15 @@ query Simulate($plan_id: Int!) { simulationDatasetId } }"""), + UPDATE_ACTIVITY_DIRECTIVE_ARGUMENTS(""" + mutation updateActivityDirectiveArguments($id: Int!, $plan_id: Int!, $arguments: jsonb!) { + updateActivityDirectiveArguments: update_activity_directive_by_pk( + pk_columns: {id: $id, plan_id: $plan_id}, + _set: {arguments: $arguments} + ) { + id + } + }"""), UPDATE_CONSTRAINT(""" mutation updateConstraint($constraintId: Int!, $constraintDefinition: String!) { constraint: insert_constraint_definition_one(object: {constraint_id: $constraintId, definition: $constraintDefinition}) { diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/HasuraRequests.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/HasuraRequests.java index aa5bf80841..d854205198 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/HasuraRequests.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/HasuraRequests.java @@ -192,6 +192,16 @@ public int insertActivity(int planId, String type, String startOffset, JsonObjec return makeRequest(GQL.CREATE_ACTIVITY_DIRECTIVE, variables).getJsonObject("createActivityDirective").getInt("id"); } + public void updateActivityDirectiveArguments(int planId, int activityId, JsonObject arguments) throws IOException { + final var variables = Json.createObjectBuilder() + .add("plan_id", planId) + .add("id", activityId) + .add("arguments", arguments) + .build(); + + makeRequest(GQL.UPDATE_ACTIVITY_DIRECTIVE_ARGUMENTS, variables); + } + public void deleteActivity(int planId, int activityId) throws IOException { final var variables = Json.createObjectBuilder() .add("plan_id", planId) diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/ResponseSerializers.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/ResponseSerializers.java index 6935fc4604..f9bf187f58 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/ResponseSerializers.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/ResponseSerializers.java @@ -191,6 +191,14 @@ public static JsonValue serializeBulkArgumentValidationResponse(BulkArgumentVali return Json.createObjectBuilder(serializeInstantiationException(f.ex()).asJsonObject()) .add("type", "INSTANTIATION_ERRORS") .build(); + } else if (response instanceof BulkArgumentValidationResponse.NoSuchMissionModelError m) { + return Json.createObjectBuilder() + .add("success", JsonValue.FALSE) + .add("type", "NO_SUCH_MISSION_MODEL") + .add("errors", Json.createObjectBuilder() + .add("noSuchMissionModelError", serializeNoSuchMissionModelException(m.ex())) + .build()) + .build(); } // This should never happen, but we don't have exhaustive pattern matching diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetUnvalidatedDirectivesAction.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetUnvalidatedDirectivesAction.java index 0697906f63..1439ac1e74 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetUnvalidatedDirectivesAction.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetUnvalidatedDirectivesAction.java @@ -5,7 +5,6 @@ import gov.nasa.jpl.aerie.merlin.server.models.ActivityDirectiveId; import gov.nasa.jpl.aerie.merlin.server.models.MissionModelId; import gov.nasa.jpl.aerie.merlin.server.models.PlanId; -import gov.nasa.jpl.aerie.merlin.server.models.Timestamp; import java.sql.Connection; import java.sql.PreparedStatement; diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/LocalMissionModelService.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/LocalMissionModelService.java index ba5873a833..6c1c2b4c65 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/LocalMissionModelService.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/LocalMissionModelService.java @@ -133,10 +133,22 @@ public List validateActivityArguments(final String missionMode public List validateActivityArgumentsBulk( final MissionModelId modelId, - final List activities - ) throws NoSuchMissionModelException, MissionModelLoadException { + final List activities) { // load mission model once for all activities - final var modelType = this.loadMissionModelType(modelId.toString()); + ModelType modelType; + try { + modelType = this.loadMissionModelType(modelId.toString()); + // try and catch NoSuchMissionModel here, so we can serialize it out to each activity validation + // rather than catching it at a higher level in the workerLoop itself + } catch (NoSuchMissionModelException e) { + return activities.stream() + .map(directive -> new BulkArgumentValidationResponse.NoSuchMissionModelError(e)) + .collect(Collectors.toList()); + } catch (MissionModelLoadException e) { + log.error("Caught MissionModelLoadException, skipping this batch but leaving validations pending..."); + log.error(e.toString()); + return List.of(); + } final var registry = DirectiveTypeRegistry.extract(modelType); // map all directives to validation response diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/MissionModelService.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/MissionModelService.java index 6d18cf9229..8d9203fe8c 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/MissionModelService.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/MissionModelService.java @@ -105,6 +105,7 @@ record InstantiationFailure(InstantiationException ex) implements BulkEffective sealed interface BulkArgumentValidationResponse { record Success() implements BulkArgumentValidationResponse { } record Validation(List notices) implements BulkArgumentValidationResponse { } + record NoSuchMissionModelError(NoSuchMissionModelException ex) implements BulkArgumentValidationResponse { } record NoSuchActivityError(NoSuchActivityTypeException ex) implements BulkArgumentValidationResponse { } record InstantiationError(InstantiationException ex) implements BulkArgumentValidationResponse { } } diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/ValidationWorker.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/ValidationWorker.java index 62ff4a87bb..b2d55afe23 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/ValidationWorker.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/ValidationWorker.java @@ -1,7 +1,6 @@ package gov.nasa.jpl.aerie.merlin.server.services; import gov.nasa.jpl.aerie.merlin.server.models.ActivityDirectiveForValidation; -import gov.nasa.jpl.aerie.merlin.server.services.MissionModelService.NoSuchMissionModelException; import gov.nasa.jpl.aerie.merlin.server.services.MissionModelService.BulkArgumentValidationResponse; import org.apache.commons.lang3.tuple.Pair; import org.slf4j.Logger; @@ -49,9 +48,6 @@ public void workerLoop() { final var duration = (endTime - beginTime) / 1_000_000.0; logger.debug("processed model batch of size {} in {} ms", unvalidatedDirectives.size(), duration); } - - } catch (NoSuchMissionModelException ex) { - logger.error("Validation request failed due to no such mission model: {}", ex.toString()); } catch (InterruptedException ex) { // we were interrupted, so exit gracefully return;