diff --git a/constraints/src/main/java/gov/nasa/jpl/aerie/constraints/json/ConstraintParsers.java b/constraints/src/main/java/gov/nasa/jpl/aerie/constraints/json/ConstraintParsers.java index 073d5213c3..683cc68a18 100644 --- a/constraints/src/main/java/gov/nasa/jpl/aerie/constraints/json/ConstraintParsers.java +++ b/constraints/src/main/java/gov/nasa/jpl/aerie/constraints/json/ConstraintParsers.java @@ -313,9 +313,12 @@ static JsonParser keepTrueSegmentP(JsonParser tuple($.windows(), $.activityInstanceIds()) + untuple((windows, aids, message) -> message + .map(s -> new Violation(windows, aids, s)) + .orElseGet(() -> new Violation(windows, aids))), + $ -> tuple($.windows(), $.activityInstanceIds(), $.message()) ); public static final JsonParser edslConstraintResultP = diff --git a/constraints/src/main/java/gov/nasa/jpl/aerie/constraints/model/Violation.java b/constraints/src/main/java/gov/nasa/jpl/aerie/constraints/model/Violation.java index a200640595..1fdb324ede 100644 --- a/constraints/src/main/java/gov/nasa/jpl/aerie/constraints/model/Violation.java +++ b/constraints/src/main/java/gov/nasa/jpl/aerie/constraints/model/Violation.java @@ -7,10 +7,15 @@ import java.util.ArrayList; import java.util.List; +import java.util.Optional; -public record Violation(List windows, ArrayList activityInstanceIds) { +public record Violation(List windows, ArrayList activityInstanceIds, Optional message) { public Violation(List windows, List activityInstanceIds) { - this(windows, new ArrayList<>(activityInstanceIds)); + this(windows, new ArrayList<>(activityInstanceIds), Optional.empty()); + } + + public Violation(List windows, List activityInstanceIds, String message) { + this(windows, new ArrayList<>(activityInstanceIds), Optional.ofNullable(message)); } public static List fromProceduralViolations(Violations violations, gov.nasa.jpl.aerie.merlin.driver.SimulationResults simResults) { @@ -42,7 +47,10 @@ public static List fromProceduralViolations(Violations violations, go } } - constraintViolations.add(new Violation(List.of(Interval.fromProceduralInterval(v.getInterval())), activityInstanceIds)); + constraintViolations.add(new Violation( + List.of(Interval.fromProceduralInterval(v.getInterval())), + activityInstanceIds, + v.getMessage())); } return constraintViolations; } diff --git a/deployment/hasura/metadata/actions.graphql b/deployment/hasura/metadata/actions.graphql index a75616d162..71a7448790 100644 --- a/deployment/hasura/metadata/actions.graphql +++ b/deployment/hasura/metadata/actions.graphql @@ -343,6 +343,7 @@ type ConstraintResult { } type ConstraintViolation { + message: String, windows: [Interval!]!, activityInstanceIds: [Int!]! } diff --git a/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/FruitThresholdConstraint.java b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/FruitThresholdConstraint.java index d5da47ef55..25c0eb1c87 100644 --- a/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/FruitThresholdConstraint.java +++ b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/FruitThresholdConstraint.java @@ -19,7 +19,7 @@ public Violations run(@NotNull Plan plan, @NotNull SimulationResults simResults) return Violations.on( fruit.lessThan(upperBound).and(fruit.greaterThan(lowerBound)), false - ); + ).withDefaultMessage("Fruit count is outside of boundaries: [%d, %d]".formatted(lowerBound, upperBound)); } @WithDefaults diff --git a/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/NoMessageConstraint.java b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/NoMessageConstraint.java new file mode 100644 index 0000000000..743e09b108 --- /dev/null +++ b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/NoMessageConstraint.java @@ -0,0 +1,32 @@ +package gov.nasa.jpl.aerie.e2e.procedural.scheduling.procedures; + +import gov.nasa.ammos.aerie.procedural.constraints.Constraint; +import gov.nasa.ammos.aerie.procedural.constraints.Violations; +import gov.nasa.ammos.aerie.procedural.constraints.annotations.ConstraintProcedure; +import gov.nasa.ammos.aerie.procedural.scheduling.annotations.WithDefaults; +import gov.nasa.ammos.aerie.procedural.timeline.collections.profiles.Real; +import gov.nasa.ammos.aerie.procedural.timeline.plan.Plan; +import gov.nasa.ammos.aerie.procedural.timeline.plan.SimulationResults; +import org.jetbrains.annotations.NotNull; + +/** + * Equivalent to FruitThresholdConstraint, except it does not include a message in the violations + */ +@ConstraintProcedure +public record NoMessageConstraint(int lowerBound, int upperBound) implements Constraint { + @NotNull + @Override + public Violations run(@NotNull Plan plan, @NotNull SimulationResults simResults) { + final var fruit = simResults.resource("/fruit", Real.deserializer()); + + return Violations.on( + fruit.lessThan(upperBound).and(fruit.greaterThan(lowerBound)), + false + ); + } + + @WithDefaults + public static class Template{ + public int lowerBound = 5; + } +} diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/constraints/BasicConstraintTests.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/constraints/BasicConstraintTests.java index 21ed2f24a0..3fbc1a4c4e 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/constraints/BasicConstraintTests.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/constraints/BasicConstraintTests.java @@ -18,24 +18,35 @@ import static org.junit.jupiter.api.Assertions.assertTrue; public class BasicConstraintTests extends ProceduralSchedulingSetup { - private ConstraintInvocationId fruitTresholdConstraintId; + private ConstraintInvocationId fruitThresholdConstraintId; + private ConstraintInvocationId noMessageConstraintId; @BeforeEach void localBeforeEach() throws IOException { try (final var gateway = new GatewayRequests(playwright)) { - final int fruitTresholdConstraintJarId = gateway.uploadJarFile("build/libs/FruitThresholdConstraint.jar"); - // Add Scheduling Procedure - fruitTresholdConstraintId = hasura.createConstraintSpecProcedure( - "Test Constraint Procedure 1", - fruitTresholdConstraintJarId, + final int fruitThresholdConstraintJarId = gateway.uploadJarFile("build/libs/FruitThresholdConstraint.jar"); + final int noMessageConstraintJarId = gateway.uploadJarFile("build/libs/NoMessageConstraint.jar"); + // Add Constraint Procedures + fruitThresholdConstraintId = hasura.createConstraintSpecProcedure( + "Fruit Threshold Constraint", + fruitThresholdConstraintJarId, planId ); + noMessageConstraintId = hasura.createConstraintSpecProcedure( + "No Message Constraint", + noMessageConstraintJarId, + planId + ); + + // Disable the noMessageConstraint by default + hasura.updatePlanConstraintSpecEnabled(noMessageConstraintId.invocationId(), false); } } @AfterEach void localAfterEach() throws IOException { - hasura.deleteConstraint(fruitTresholdConstraintId.id()); + hasura.deleteConstraint(fruitThresholdConstraintId.id()); + hasura.deleteConstraint(noMessageConstraintId.id()); } /** @@ -47,8 +58,15 @@ void executeConstraintRunWithoutArguments() throws IOException { hasura.awaitSimulation(planId); final var resp = hasura.checkConstraints(planId); assertEquals(1, resp.constraintsRun().size()); - assertEquals(1, resp.constraintsRun().getFirst().errors().size()); - assertTrue(resp.constraintsRun().getFirst().errors().getFirst().message().contains("gov.nasa.jpl.aerie.merlin.protocol.types.InstantiationException: Invalid arguments for input type \"FruitThresholdConstraint\": extraneous arguments: [], unconstructable arguments: [], missing arguments: [MissingArgument[parameterName=upperBound, schema=IntSchema[]]], valid arguments: [ValidArgument[parameterName=lowerBound, serializedValue=NumericValue[value=5]]]")); + final var constraint = resp.constraintsRun().getFirst(); + assertEquals(1, constraint.errors().size()); + assertTrue(constraint.errors().getFirst().message().contains( + "gov.nasa.jpl.aerie.merlin.protocol.types.InstantiationException: Invalid arguments for input type " + + "\"FruitThresholdConstraint\": " + + "extraneous arguments: [], " + + "unconstructable arguments: [], " + + "missing arguments: [MissingArgument[parameterName=upperBound, schema=IntSchema[]]], " + + "valid arguments: [ValidArgument[parameterName=lowerBound, serializedValue=NumericValue[value=5]]]")); } /** @@ -57,7 +75,7 @@ void executeConstraintRunWithoutArguments() throws IOException { @Test void executeConstraintRunWithArguments() throws IOException { final var args = Json.createObjectBuilder().add("upperBound", 10).build(); - hasura.updateConstraintArguments(fruitTresholdConstraintId.invocationId(), args); + hasura.updateConstraintArguments(fruitThresholdConstraintId.invocationId(), args); hasura.awaitSimulation(planId); final var resp = hasura.checkConstraints(planId); assertTrue(resp.constraintsRun().getFirst().success()); @@ -68,20 +86,55 @@ void executeConstraintRunWithArguments() throws IOException { assertEquals(violation.windows(), List.of(new ConstraintResult.Interval(0, Duration.hours(48).micros()))); } + /** + * Test that constraints with a violation message include said violation message. + */ + @Test + void messageReturnedIfPresent() throws IOException { + // Enable the NoMessageConstraint + hasura.updatePlanConstraintSpecEnabled(noMessageConstraintId.invocationId(), true); + + // Assign args to the constraints + final var args = Json.createObjectBuilder().add("upperBound", 10).build(); + hasura.updateConstraintArguments(fruitThresholdConstraintId.invocationId(), args); + hasura.updateConstraintArguments(noMessageConstraintId.invocationId(), args); + + // Get Constraint Results + hasura.awaitSimulation(planId); + final var resp = hasura.checkConstraints(planId); + + assertEquals(2, resp.constraintsRun().size()); + + final var firstConstraint = resp.constraintsRun().getFirst(); + assertEquals(fruitThresholdConstraintId.invocationId(), firstConstraint.constraintInvocationId()); + assertTrue(firstConstraint.success()); + assertEquals(1, firstConstraint.result().get().violations().size()); + final var firstConstraintViolation = firstConstraint.result().get().violations().getFirst(); + assertTrue(firstConstraintViolation.message().isPresent()); + assertEquals("Fruit count is outside of boundaries: [5, 10]", firstConstraintViolation.message().get()); + + final var secondConstraint = resp.constraintsRun().getLast(); + assertEquals(noMessageConstraintId.invocationId(), secondConstraint.constraintInvocationId()); + assertTrue(secondConstraint.success()); + assertEquals(1, secondConstraint.result().get().violations().size()); + final var secondConstraintViolation = secondConstraint.result().get().violations().getFirst(); + assertTrue(secondConstraintViolation.message().isEmpty()); + } + /** * Queries the procedural constraints arguments. */ @Test void effectiveArgumentsQuery() throws IOException { final var effectiveArgs = hasura.getEffectiveProceduralConstraintsArgumentsBulk( - List.of(Pair.of(fruitTresholdConstraintId.id(), Json.createObjectBuilder().add("upperBound", 10).build()))); + List.of(Pair.of(fruitThresholdConstraintId.id(), Json.createObjectBuilder().add("upperBound", 10).build()))); assertEquals(1, effectiveArgs.size()); - assertTrue(effectiveArgs.get(0).success()); - assertTrue(effectiveArgs.get(0).arguments().isPresent()); - assertTrue(effectiveArgs.get(0).errors().isEmpty()); + assertTrue(effectiveArgs.getFirst().success()); + assertTrue(effectiveArgs.getFirst().arguments().isPresent()); + assertTrue(effectiveArgs.getFirst().errors().isEmpty()); // Check returned Arguments - final var args = effectiveArgs.get(0).arguments().get(); + final var args = effectiveArgs.getFirst().arguments().get(); assertEquals(2, args.size()); assertEquals(10, args.getInt("upperBound")); assertEquals(5, args.getInt("lowerBound")); diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ConstraintResult.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ConstraintResult.java index 4f7cd4cdee..85b28b56c2 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ConstraintResult.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ConstraintResult.java @@ -4,20 +4,23 @@ import javax.json.JsonObject; import javax.json.JsonString; import java.util.List; +import java.util.Optional; public record ConstraintResult( List resourceIds, List violations, List gaps ) { - public record ConstraintViolation(List activityInstanceIds, List windows) { + public record ConstraintViolation(List activityInstanceIds, List windows, Optional message) { public static ConstraintViolation fromJSON(JsonObject json) { return new ConstraintViolation( json.getJsonArray("activityInstanceIds") .getValuesAs(JsonNumber::intValue), json.getJsonArray("windows") - .getValuesAs(Interval::fromJSON)); + .getValuesAs(Interval::fromJSON), + Optional.ofNullable(json.getString("message", null)) + ); } } 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 34ec961f21..f72f9a043b 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 @@ -68,6 +68,7 @@ query checkConstraints($planId: Int!, $simulationDatasetId: Int) { start } violations { + message activityInstanceIds windows { end 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 c7805c376c..0b65c0c82a 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 @@ -35,6 +35,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.SortedMap; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -287,7 +288,7 @@ public static JsonValue serializeResourceSamples(final Map>> resultMap) { + public static JsonValue serializeConstraintResults(final int requestId, final SortedMap>> resultMap) { var results = resultMap.entrySet().stream().map(entry -> { final var constraint = entry.getKey(); @@ -323,7 +324,7 @@ public static JsonValue serializeConstraintResults(final int requestId, final Ma } // successful runs - var constraintResult = (ConstraintResult) fallible.getOptional().get(); + var constraintResult = fallible.getOptional().get(); return Json.createObjectBuilder() .add("success", JsonValue.TRUE) .add("constraintId", constraint.constraintId()) diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/models/ConstraintRecord.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/models/ConstraintRecord.java index ee0ff27744..13c4053e88 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/models/ConstraintRecord.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/models/ConstraintRecord.java @@ -23,4 +23,10 @@ public record ConstraintRecord( String description, ConstraintType type, Map arguments -) {} +) implements Comparable { + + @Override + public int compareTo(final ConstraintRecord o) { + return Long.compare(this.priority, o.priority); + } +} diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/ConstraintAction.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/ConstraintAction.java index 6ae7bad3e6..e764b1c6bb 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/ConstraintAction.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/services/ConstraintAction.java @@ -89,7 +89,7 @@ public List getConstraintProcedureEffec * @throws MissionModelService.NoSuchMissionModelException If the plan's mission model does not exist. * @throws SimulationDatasetMismatchException If the specified simulation is not a simulation of the specified plan. */ - public Pair>>> getViolations( + public Pair>>> getViolations( final PlanId planId, final Optional simulationDatasetId, final boolean force, @@ -117,7 +117,7 @@ public Pair(this.planService.getConstraintsForPlan(planId)); - final var constraintResultMap = new HashMap>>(); + final var constraintResultMap = new TreeMap>>(); // Load cached results if the force rerun flag is not set final var validConstraintRuns = force ? new HashMap() :