Merge pull request #1258 from NASA-AMMOS/feature/allow-validation-fai…

…lure-reasons Added a new ValidationResult type that can be used to return more spe…
NASA-AMMOS · Dec 21, 2023 · 3972ab2 · 3972ab2
2 parents ec9f2c9 + 1ac4f00
commit 3972ab2
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 35 deletions.
diff --git a/contrib/src/main/java/gov/nasa/jpl/aerie/contrib/models/ValidationResult.java b/contrib/src/main/java/gov/nasa/jpl/aerie/contrib/models/ValidationResult.java
@@ -0,0 +1,5 @@
+package gov.nasa.jpl.aerie.contrib.models;
+
+import java.util.Optional;
+
+public record ValidationResult(boolean success, String subject, String message) {}
diff --git a/...ion/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java b/...ion/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java
@@ -1,6 +1,7 @@
 package gov.nasa.jpl.aerie.banananation.activities;
 
 import gov.nasa.jpl.aerie.banananation.Mission;
+import gov.nasa.jpl.aerie.contrib.models.ValidationResult;
 import gov.nasa.jpl.aerie.merlin.framework.annotations.ActivityType;
 import gov.nasa.jpl.aerie.merlin.framework.annotations.ActivityType.EffectModel;
 import gov.nasa.jpl.aerie.merlin.framework.annotations.Export.Validation;
@@ -9,16 +10,15 @@
 @ActivityType("BakeBananaBread")
 public record BakeBananaBreadActivity(double temperature, int tbSugar, boolean glutenFree) {
 
-  @Validation("Temperature must be positive")
-  @Validation.Subject("temperature")
-  public boolean validateTemperature() {
-    return this.temperature() > 0;
-  }
+  @Validation
+  public ValidationResult validateTemperatures() {
+    if (this.temperature < 0) {
+      return new ValidationResult(false, "temperature", "Temperature must be positive");
+    }
 
-  @Validation("Gluten-free bread must be baked at a temperature >= 100")
-  @Validation.Subject({"glutenFree", "temperature"})
-  public boolean validateGlutenFreeTemperature() {
-    return !glutenFree || temperature >= 100;
+    return new ValidationResult(!glutenFree || temperature >= 100,
+      "glutenFree",
+      "Gluten-free bread must be baked at a temperature >= 100");
   }
 
   @EffectModel

diff --git a/...nanation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java b/...nanation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java
@@ -3,6 +3,7 @@
 import gov.nasa.jpl.aerie.banananation.Flag;
 import gov.nasa.jpl.aerie.banananation.Mission;
 import gov.nasa.jpl.aerie.contrib.metadata.Unit;
+import gov.nasa.jpl.aerie.contrib.models.ValidationResult;
 import gov.nasa.jpl.aerie.merlin.framework.annotations.ActivityType;
 import gov.nasa.jpl.aerie.merlin.framework.annotations.ActivityType.EffectModel;
 import gov.nasa.jpl.aerie.merlin.framework.annotations.AutoValueMapper;
@@ -25,10 +26,9 @@ public final class BiteBananaActivity {
   @Unit("m")
   public double biteSize = 1.0;
 
-  @Validation("bite size must be positive")
-  @Validation.Subject("biteSize")
-  public boolean validateBiteSize() {
-    return this.biteSize > 0;
+  @Validation
+  public ValidationResult validateBiteSize() {
+    return new ValidationResult(this.biteSize > 0, "biteSize", "bite size must be positive");
   }
 
   @EffectModel

diff --git a/...ework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/MissionModelParser.java b/...ework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/MissionModelParser.java
@@ -1,6 +1,7 @@
 package gov.nasa.jpl.aerie.merlin.processor;
 
 import com.squareup.javapoet.ClassName;
+import gov.nasa.jpl.aerie.contrib.models.ValidationResult;
 import gov.nasa.jpl.aerie.merlin.framework.MetadataValueMapper;
 import gov.nasa.jpl.aerie.merlin.framework.Registrar;
 import gov.nasa.jpl.aerie.merlin.framework.ValueMapper;
@@ -502,6 +503,9 @@ private List<ParameterValidationRecord> getExportValidations(
     for (final var element : exportTypeElement.getEnclosedElements()) {
       if (element.getAnnotation(Export.Validation.class) == null) continue;
 
+      boolean isSimpleValidation = !(element.getKind() == ElementKind.METHOD &&
+          ((ExecutableElement) element).getReturnType().toString().equals(ValidationResult.class.getTypeName()));
+
       final var name = element.getSimpleName().toString();
       final var message = element.getAnnotation(Export.Validation.class).value();
       final var subjects = element.getAnnotation(Export.Validation.Subject.class) == null ?
@@ -519,7 +523,7 @@ private List<ParameterValidationRecord> getExportValidations(
                 .formatted(name, missingSubjects$.get()), exportTypeElement);
       }
 
-      validations.add(new ParameterValidationRecord(name, subjects, message));
+      validations.add(new ParameterValidationRecord(name, subjects, message, isSimpleValidation));
     }
 
     return validations;

diff --git a/...cessor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/generator/MapperMethodMaker.java b/...cessor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/generator/MapperMethodMaker.java
@@ -12,8 +12,10 @@
 import gov.nasa.jpl.aerie.merlin.protocol.types.UnconstructableArgumentException;
 
 import javax.lang.model.element.Modifier;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 import java.util.function.BiFunction;
 import java.util.stream.Collectors;
 
@@ -151,26 +153,58 @@ public final MethodSpec makeGetValidationFailuresMethod() {
             inputType.validations()
                 .stream()
                 .map(validation -> {
-                    final var subjects = Arrays.stream(validation.subjects())
-                        .map(subject -> CodeBlock.builder().add("$S", subject))
-                        .reduce((x, y) -> x.add(", ").add(y.build()))
-                        .orElse(CodeBlock.builder())
-                        .build();
+                    CodeBlock.Builder codeBlock = CodeBlock.builder().beginControlFlow("try");
 
-                    return CodeBlock
-                        .builder()
-                        .beginControlFlow("try")
-                        .addStatement(
-                            "if (!$L.$L()) notices.add(new $T($T.of($L), $S))",
-                            "input",
-                            validation.methodName(),
-                            ValidationNotice.class,
-                            List.class,
-                            subjects,
-                            validation.failureMessage())
-                        .nextControlFlow("catch ($T ex)", Throwable.class)
-                        .addStatement("notices.add(new $T($T.of($L), ex.getMessage()))", ValidationNotice.class, List.class, subjects)
-                        .endControlFlow();
+                    if (validation.isSimpleValidation()) {
+                        final var subjects = Arrays.stream(validation.subjects())
+                          .map(subject -> CodeBlock.builder().add("$S", subject))
+                          .reduce((x, y) -> x.add(", ").add(y.build()))
+                          .orElse(CodeBlock.builder())
+                          .build();
+
+                        codeBlock.addStatement(
+                          "if (!$L.$L()) notices.add(new $T($T.of($L), $S))",
+                          "input",
+                          validation.methodName(),
+                          ValidationNotice.class,
+                          List.class,
+                          subjects,
+                          validation.failureMessage());
+
+                        return codeBlock
+                          .nextControlFlow("catch ($T ex)", Throwable.class)
+                          .addStatement(
+                              "notices.add(new $T($T.of($L), ex.getMessage()))",
+                              ValidationNotice.class,
+                              List.class,
+                              subjects)
+                          .endControlFlow();
+                    }
+
+                    codeBlock.addStatement(
+                      "if (!$L.$L().$L()) notices.add(new $T($T.of($L.$L().$L()), $L.$L().$L()))",
+                      "input",
+                      validation.methodName(),
+                      "success",
+                      ValidationNotice.class,
+                      List.class,
+                      "input",
+                      validation.methodName(),
+                      "subject",
+                      "input",
+                      validation.methodName(),
+                      "message");
+
+                    return codeBlock
+                      .nextControlFlow("catch ($T ex)", Throwable.class)
+                      .addStatement(
+                          "notices.add(new $T($T.of($L.$L().$L()), ex.getMessage()))",
+                          ValidationNotice.class,
+                          List.class,
+                          "input",
+                          validation.methodName(),
+                          "subject")
+                      .endControlFlow();
                 })
                 .reduce(CodeBlock.builder(), (x, y) -> x.add(y.build()))
                 .build())

diff --git a/...rc/main/java/gov/nasa/jpl/aerie/merlin/processor/metamodel/ParameterValidationRecord.java b/...rc/main/java/gov/nasa/jpl/aerie/merlin/processor/metamodel/ParameterValidationRecord.java
@@ -1,3 +1,3 @@
 package gov.nasa.jpl.aerie.merlin.processor.metamodel;
 
-public record ParameterValidationRecord(String methodName, String[] subjects, String failureMessage) { }
+public record ParameterValidationRecord(String methodName, String[] subjects, String failureMessage, boolean isSimpleValidation) { }
diff --git a/merlin-framework/src/main/java/gov/nasa/jpl/aerie/merlin/framework/annotations/Export.java b/merlin-framework/src/main/java/gov/nasa/jpl/aerie/merlin/framework/annotations/Export.java
@@ -31,7 +31,7 @@
   @Retention(RetentionPolicy.CLASS)
   @Target(ElementType.METHOD)
   @interface Validation {
-    String value();
+    String value() default "";
 
     @Retention(RetentionPolicy.CLASS)
     @Target(ElementType.METHOD)