From 630e663df681eddbd2efb59d66c68442388271c0 Mon Sep 17 00:00:00 2001 From: Cody Hansen Date: Thu, 14 Dec 2023 06:28:06 -1000 Subject: [PATCH 1/4] Added a new ValidationResult type that can be used to return more specific validation notices --- .../contrib/models/ValidationResult.java | 5 ++ deployment/hasura/metadata/actions.graphql | 2 +- .../activities/BakeBananaBreadActivity.java | 20 +++--- .../activities/BiteBananaActivity.java | 10 +-- .../merlin/processor/MissionModelParser.java | 6 +- .../generator/MapperMethodMaker.java | 72 ++++++++++++++----- .../metamodel/ParameterValidationRecord.java | 2 +- .../merlin/framework/annotations/Export.java | 2 +- 8 files changed, 83 insertions(+), 36 deletions(-) create mode 100644 contrib/src/main/java/gov/nasa/jpl/aerie/contrib/models/ValidationResult.java 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 new file mode 100644 index 0000000000..6855bda959 --- /dev/null +++ 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, Optional message) {} diff --git a/deployment/hasura/metadata/actions.graphql b/deployment/hasura/metadata/actions.graphql index 8349d072b6..d8ef9314e5 100644 --- a/deployment/hasura/metadata/actions.graphql +++ b/deployment/hasura/metadata/actions.graphql @@ -191,7 +191,7 @@ type MerlinSimulationResponse { } type ValidationResponse { - errors: [ValidationNotice!] + errors: ValidationNotice! success: Boolean! } diff --git a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java index 3cc285590b..b456fc4fb8 100644 --- a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java +++ b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java @@ -1,24 +1,26 @@ 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; import gov.nasa.jpl.aerie.merlin.framework.annotations.Export.WithDefaults; +import java.util.Optional; + @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", Optional.of("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", + Optional.of("Gluten-free bread must be baked at a temperature >= 100")); } @EffectModel diff --git a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java index 440e7cc43d..82696425b0 100644 --- a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java +++ b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java @@ -3,12 +3,15 @@ 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; import gov.nasa.jpl.aerie.merlin.framework.annotations.Export.Parameter; import gov.nasa.jpl.aerie.merlin.framework.annotations.Export.Validation; +import java.util.Optional; + /** * Bite a banana. * @@ -25,10 +28,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", Optional.of("bite size must be positive")); } @EffectModel diff --git a/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/MissionModelParser.java b/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/MissionModelParser.java index 65cdee299e..2feba504b1 100644 --- a/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/MissionModelParser.java +++ b/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/MissionModelParser.java @@ -502,6 +502,10 @@ private List getExportValidations( for (final var element : exportTypeElement.getEnclosedElements()) { if (element.getAnnotation(Export.Validation.class) == null) continue; + // Is there a way to tie this to the actual class rather than the string? + boolean isSimpleValidation = !(element.getKind() == ElementKind.METHOD && + ((ExecutableElement) element).getReturnType().toString().equals("gov.nasa.jpl.aerie.contrib.models.ValidationResult")); + 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 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/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/generator/MapperMethodMaker.java b/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/generator/MapperMethodMaker.java index 7226c983c5..ba7b76744f 100644 --- a/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/generator/MapperMethodMaker.java +++ b/merlin-framework-processor/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().orElse(null)))", + "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/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/metamodel/ParameterValidationRecord.java b/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/metamodel/ParameterValidationRecord.java index 1862f43b3e..51721917cc 100644 --- a/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/metamodel/ParameterValidationRecord.java +++ b/merlin-framework-processor/src/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 index 3a48b3d3ab..2f8c126194 100644 --- 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) From df4ad7ac045879be62c85ceef94cf04b0bd15a36 Mon Sep 17 00:00:00 2001 From: Cody Hansen Date: Wed, 20 Dec 2023 06:59:50 -1000 Subject: [PATCH 2/4] Removed optional from ValidationResult, fixed validation Hasura action --- .../gov/nasa/jpl/aerie/contrib/models/ValidationResult.java | 2 +- deployment/hasura/metadata/actions.graphql | 2 +- .../banananation/activities/BakeBananaBreadActivity.java | 4 ++-- .../jpl/aerie/banananation/activities/BiteBananaActivity.java | 2 +- .../nasa/jpl/aerie/merlin/processor/MissionModelParser.java | 4 ++-- .../aerie/merlin/processor/generator/MapperMethodMaker.java | 2 +- 6 files changed, 8 insertions(+), 8 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 index 6855bda959..0a40521d4e 100644 --- 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 @@ -2,4 +2,4 @@ import java.util.Optional; -public record ValidationResult(boolean success, String subject, Optional message) {} +public record ValidationResult(boolean success, String subject, String message) {} diff --git a/deployment/hasura/metadata/actions.graphql b/deployment/hasura/metadata/actions.graphql index d8ef9314e5..359c7c257a 100644 --- a/deployment/hasura/metadata/actions.graphql +++ b/deployment/hasura/metadata/actions.graphql @@ -191,7 +191,7 @@ type MerlinSimulationResponse { } type ValidationResponse { - errors: ValidationNotice! + errors: [ValidationNotice]! success: Boolean! } diff --git a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java index b456fc4fb8..30a728645e 100644 --- a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java +++ b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java @@ -15,12 +15,12 @@ public record BakeBananaBreadActivity(double temperature, int tbSugar, boolean g @Validation public ValidationResult validateTemperatures() { if (this.temperature < 0) { - return new ValidationResult(false, "temperature", Optional.of("Temperature must be positive")); + return new ValidationResult(false, "temperature", "Temperature must be positive"); } return new ValidationResult(!glutenFree || temperature >= 100, "glutenFree", - Optional.of("Gluten-free bread must be baked at a temperature >= 100")); + "Gluten-free bread must be baked at a temperature >= 100"); } @EffectModel diff --git a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java index 82696425b0..404dd6d018 100644 --- a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java +++ b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java @@ -30,7 +30,7 @@ public final class BiteBananaActivity { @Validation public ValidationResult validateBiteSize() { - return new ValidationResult(this.biteSize > 0, "biteSize", Optional.of("bite size must be positive")); + return new ValidationResult(this.biteSize > 0, "biteSize", "bite size must be positive"); } @EffectModel diff --git a/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/MissionModelParser.java b/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/MissionModelParser.java index 2feba504b1..06742b0d1b 100644 --- a/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/MissionModelParser.java +++ b/merlin-framework-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,9 +503,8 @@ private List getExportValidations( for (final var element : exportTypeElement.getEnclosedElements()) { if (element.getAnnotation(Export.Validation.class) == null) continue; - // Is there a way to tie this to the actual class rather than the string? boolean isSimpleValidation = !(element.getKind() == ElementKind.METHOD && - ((ExecutableElement) element).getReturnType().toString().equals("gov.nasa.jpl.aerie.contrib.models.ValidationResult")); + ((ExecutableElement) element).getReturnType().toString().equals(ValidationResult.class.getTypeName())); final var name = element.getSimpleName().toString(); final var message = element.getAnnotation(Export.Validation.class).value(); diff --git a/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/generator/MapperMethodMaker.java b/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/generator/MapperMethodMaker.java index ba7b76744f..d466eb7bf4 100644 --- a/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/generator/MapperMethodMaker.java +++ b/merlin-framework-processor/src/main/java/gov/nasa/jpl/aerie/merlin/processor/generator/MapperMethodMaker.java @@ -182,7 +182,7 @@ public final MethodSpec makeGetValidationFailuresMethod() { } codeBlock.addStatement( - "if (!$L.$L().$L()) notices.add(new $T($T.of($L.$L().$L()), $L.$L().$L().orElse(null)))", + "if (!$L.$L().$L()) notices.add(new $T($T.of($L.$L().$L()), $L.$L().$L()))", "input", validation.methodName(), "success", From cb4ee5a8797cd0fa5847cc5df67687ba4d6b976b Mon Sep 17 00:00:00 2001 From: Cody Hansen Date: Wed, 20 Dec 2023 07:21:03 -1000 Subject: [PATCH 3/4] Properly reverted actions.graphql change --- deployment/hasura/metadata/actions.graphql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deployment/hasura/metadata/actions.graphql b/deployment/hasura/metadata/actions.graphql index 359c7c257a..8349d072b6 100644 --- a/deployment/hasura/metadata/actions.graphql +++ b/deployment/hasura/metadata/actions.graphql @@ -191,7 +191,7 @@ type MerlinSimulationResponse { } type ValidationResponse { - errors: [ValidationNotice]! + errors: [ValidationNotice!] success: Boolean! } From 1ac4f00a202c4ed5fc90d2fbc899d3f9614daa2a Mon Sep 17 00:00:00 2001 From: Cody Hansen Date: Wed, 20 Dec 2023 07:22:16 -1000 Subject: [PATCH 4/4] Removed unused imports --- .../aerie/banananation/activities/BakeBananaBreadActivity.java | 2 -- .../jpl/aerie/banananation/activities/BiteBananaActivity.java | 2 -- 2 files changed, 4 deletions(-) diff --git a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java index 30a728645e..c95fae56c1 100644 --- a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java +++ b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BakeBananaBreadActivity.java @@ -7,8 +7,6 @@ import gov.nasa.jpl.aerie.merlin.framework.annotations.Export.Validation; import gov.nasa.jpl.aerie.merlin.framework.annotations.Export.WithDefaults; -import java.util.Optional; - @ActivityType("BakeBananaBread") public record BakeBananaBreadActivity(double temperature, int tbSugar, boolean glutenFree) { diff --git a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java index 404dd6d018..264a9af2eb 100644 --- a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java +++ b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/BiteBananaActivity.java @@ -10,8 +10,6 @@ import gov.nasa.jpl.aerie.merlin.framework.annotations.Export.Parameter; import gov.nasa.jpl.aerie.merlin.framework.annotations.Export.Validation; -import java.util.Optional; - /** * Bite a banana. *