Merge pull request #1354 from NASA-AMMOS/fix/directive-validations-with-null-model-id

Serialize `NoSuchMissionModel` exception out in activity validations

Author: skovati

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(

e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ActivityValidation.java

@@ -11,6 +11,7 @@ record Success() implements ActivityValidation {}
   record InstantiationFailure(List<String> extraneousArguments, List<String> missingArguments, List<?> unconstructableArguments) implements ActivityValidation {}
   record ValidationFailure(List<ValidationNotice> notices) implements ActivityValidation {}
   record NoSuchActivityTypeFailure(String message, String activityType) implements ActivityValidation {}
+  record NoSuchMissionModelFailure(String message, String modelId) implements ActivityValidation {}
 
   record ValidationNotice(List<String> 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);
     };
   }

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}) {

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)

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

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;

merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/LocalMissionModelService.java

@@ -133,10 +133,22 @@ public List<ValidationNotice> validateActivityArguments(final String missionMode
 
   public List<BulkArgumentValidationResponse> validateActivityArgumentsBulk(
       final MissionModelId modelId,
-      final List<ActivityDirectiveForValidation> activities
-  ) throws NoSuchMissionModelException, MissionModelLoadException {
+      final List<ActivityDirectiveForValidation> 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

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<ValidationNotice> notices) implements BulkArgumentValidationResponse { }
+    record NoSuchMissionModelError(NoSuchMissionModelException ex) implements BulkArgumentValidationResponse { }
     record NoSuchActivityError(NoSuchActivityTypeException ex) implements BulkArgumentValidationResponse { }
     record InstantiationError(InstantiationException ex) implements BulkArgumentValidationResponse { }
   }

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;