Minor update to the e2e test

* We are always returning an object now

* Update error messages.

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

@@ -2,7 +2,6 @@
 
 import com.microsoft.playwright.Playwright;
 import gov.nasa.jpl.aerie.e2e.types.ConstraintError;
-import gov.nasa.jpl.aerie.e2e.types.ConstraintResult;
 import gov.nasa.jpl.aerie.e2e.types.ExternalDataset.ProfileInput;
 import gov.nasa.jpl.aerie.e2e.types.ExternalDataset.ProfileInput.ProfileSegmentInput;
 import gov.nasa.jpl.aerie.e2e.types.ValueSchema;
@@ -107,8 +106,9 @@ void constraintsSucceedOneViolation() throws IOException {
 
     // Check the Result
     final var constraintResponse = constraintsResponses.get(0);
-    assertEquals(true,constraintResponse.success());
-    final ConstraintResult constraintResult = constraintResponse.result();
+    assertTrue(constraintResponse.success());
+    assertTrue(constraintResponse.result().isPresent());
+    final var constraintResult = constraintResponse.result().get();
     assertEquals(constraintId, constraintResult.constraintId());
     assertEquals(constraintName, constraintResult.constraintName());
 
@@ -136,19 +136,23 @@ void constraintsSucceedNoViolations() throws IOException {
     // Delete activity to avoid violation
     hasura.deleteActivity(planId, activityId);
     hasura.awaitSimulation(planId);
-    final var constraintsResults = hasura.checkConstraints(planId);
-    assertEquals(0, constraintsResults.size());
+    final var constraintResponses = hasura.checkConstraints(planId);
+
+    assertEquals(1, constraintResponses.size());
+    assertTrue( constraintResponses.get(0).success());
+    assertTrue( constraintResponses.get(0).result().isPresent());
+    assertEquals(0, constraintResponses.get(0).result().get().violations().size());
   }
 
   @Test
   void constraintCachedViolation() throws IOException {
     final var simDatasetId = hasura.awaitSimulation(planId).simDatasetId();
-    final var constraintRuns = hasura.checkConstraints(planId);
+    final var constraintResponses = hasura.checkConstraints(planId);
     final var cachedRuns = hasura.getConstraintRuns(simDatasetId);
 
     // There's the correct number of results
     assertEquals(1, cachedRuns.size());
-    assertEquals(1, constraintRuns.size());
+    assertEquals(1, constraintResponses.size());
 
     // Check properties
     final var cachedRun = cachedRuns.get(0);
@@ -159,9 +163,10 @@ void constraintCachedViolation() throws IOException {
 
     // Check results
     assertTrue(cachedRun.results().isPresent());
-    final var constraintResponse = constraintRuns.get(0);
-    assertEquals(true,constraintResponse.success());
-    assertEquals(constraintResponse.result(), cachedRun.results().get());
+    final var constraintResponse = constraintResponses.get(0);
+    assertTrue(constraintResponse.success());
+    assertTrue(constraintResponse.result().isPresent());
+    assertEquals(constraintResponse.result().get(), cachedRun.results().get());
   }
 
   @Test
@@ -183,7 +188,16 @@ void constraintCachedNoViolations() throws IOException {
     assertEquals(constraintDefinition, cachedRun.constraintDefinition());
 
     // Check results
-    assertTrue(cachedRun.results().isEmpty());
+    assertTrue(cachedRun.results().isPresent());
+    final var results = cachedRun.results().get();
+    assertEquals(constraintId, results.constraintId());
+    assertEquals("fruit_equal_peel",results.constraintName());
+    assertEquals("plan",results.type());
+    assertEquals(2,results.resourceIds().size());
+    assertEquals("/peel",results.resourceIds().get(0));
+    assertEquals("/fruit",results.resourceIds().get(1));
+    assertEquals(0,results.violations().size());
+    assertEquals(0,results.gaps().size());
   }
 
   @Test
@@ -228,8 +242,9 @@ void constraintsWorkMonthLongActivity() throws IOException {
 
     // Check the Result
     final var constraintResponse = constraintsResponses.get(0);
-    assertEquals(true,constraintResponse.success());
-    final var constraintResult = constraintResponse.result();
+    assertTrue(constraintResponse.success());
+    assertTrue(constraintResponse.result().isPresent());
+    final var constraintResult = constraintResponse.result().get();
     assertEquals(constraintId, constraintResult.constraintId());
     assertEquals(constraintName, constraintResult.constraintName());
 
@@ -257,17 +272,22 @@ void runConstraintsOnOldSimulation() throws IOException {
     final int newSimDatasetId = hasura.awaitSimulation(planId).simDatasetId();
 
     // Expect no violations on the new simulation
-    final var newConstraintResults = hasura.checkConstraints(planId, newSimDatasetId);
-    assertEquals(0, newConstraintResults.size());
+    final var newConstraintResponses = hasura.checkConstraints(planId, newSimDatasetId);
+    assertEquals(1, newConstraintResponses.size());
+    assertTrue(newConstraintResponses.get(0).result().isPresent());
+    final var newConstraintResult = newConstraintResponses.get(0).result().get();
+    assertTrue(newConstraintResult.violations().isEmpty());
+
 
     // Expect one violation on the old simulation
     final var oldConstraintsResponses = hasura.checkConstraints(planId, oldSimDatasetId);
     assertEquals(1, oldConstraintsResponses.size());
 
     // Check the Result
     final var oldConstraintResponse = oldConstraintsResponses.get(0);
-    assertEquals(true,oldConstraintResponse.success());
-    final var constraintResult = oldConstraintResponse.result();
+    assertTrue(oldConstraintResponse.success());
+    assertTrue(oldConstraintResponse.result().isPresent());
+    final var constraintResult = oldConstraintResponse.result().get();
 
     assertEquals(constraintId, constraintResult.constraintId());
     assertEquals(constraintName, constraintResult.constraintName());
@@ -320,12 +340,14 @@ class WithExternalDatasets {
             new ProfileSegmentInput(3600000000L, JsonValue.TRUE), // 1hr in micros
             new ProfileSegmentInput(3600000000L, JsonValue.FALSE)));
 
+    public static final String constraintDefinition = "export default (): Constraint => Discrete.Resource(\"/my_boolean\").equal(true)";
+
     @BeforeEach
     void beforeEach() throws IOException {
       // Change Constraint to be about MyBoolean
       hasura.updateConstraint(
           constraintId,
-          "export default (): Constraint => Discrete.Resource(\"/my_boolean\").equal(true)");
+          constraintDefinition);
       // Simulate Plan
       simDatasetId = hasura.awaitSimulation(planId).simDatasetId();
       // Add Simulation-Associated External Dataset
@@ -347,32 +369,33 @@ void afterEach() throws IOException {
     @DisplayName("Simulation-Associated External Datasets are loaded when Simulation is current")
     void oneViolationCurrentSimulation() throws IOException {
       // Constraint Results w/o SimDatasetId
-      final var noDatasetResults = hasura.checkConstraints(planId);
-      assertEquals(1, noDatasetResults.size());
-      assertEquals(true,  noDatasetResults.get(0).success());
-
-      final var ndRecord = noDatasetResults.get(0).result();
+      final var noDatasetResponses = hasura.checkConstraints(planId);
+      assertEquals(1, noDatasetResponses.size());
+      assertTrue(noDatasetResponses.get(0).success());
+      assertTrue(noDatasetResponses.get(0).result().isPresent());
+      final var nRecordResults = noDatasetResponses.get(0).result().get();
 
       // Constraint Results w/ SimDatasetId
-      final var withDatasetResults = hasura.checkConstraints(planId, simDatasetId);
-      assertEquals(1, withDatasetResults.size());
-      assertEquals(true,  withDatasetResults.get(0).success());
-      final var wdRecord = withDatasetResults.get(0).result();
+      final var withDatasetResponses = hasura.checkConstraints(planId, simDatasetId);
+      assertEquals(1, withDatasetResponses.size());
+      assertTrue(  withDatasetResponses.get(0).success());
+      assertTrue(withDatasetResponses.get(0).result().isPresent());
+      final var wRecordResults = withDatasetResponses.get(0).result().get();
 
       // The results should be the same
-      assertEquals(ndRecord, wdRecord);
+      assertEquals(nRecordResults, wRecordResults);
 
       // Check the Result
-      assertEquals(constraintName, ndRecord.constraintName());
-      assertEquals(constraintId, ndRecord.constraintId());
+      assertEquals(constraintName, nRecordResults.constraintName());
+      assertEquals(constraintId, nRecordResults.constraintId());
 
       // Resources
-      assertEquals(1, ndRecord.resourceIds().size());
-      assertTrue(ndRecord.resourceIds().contains("/my_boolean"));
+      assertEquals(1, nRecordResults.resourceIds().size());
+      assertTrue(nRecordResults.resourceIds().contains("/my_boolean"));
 
       // Violation
-      assertEquals(1, ndRecord.violations().size());
-      final var violation = ndRecord.violations().get(0);
+      assertEquals(1, nRecordResults.violations().size());
+      final var violation = nRecordResults.violations().get(0);
       assertEquals(1, violation.windows().size());
 
       final long falseBeginMicros = 7 * 60 * 60 * 1000000L; // 7h in micros
@@ -384,15 +407,15 @@ void oneViolationCurrentSimulation() throws IOException {
       // Gaps
       // Two are expected: 1 from plan start to the external dataset start
       // and 1 from the external dataset end to plan end
-      assertEquals(2, ndRecord.gaps().size());
+      assertEquals(2, nRecordResults.gaps().size());
       final long datasetOffsetMicros = 6 * 60 * 60 * 1000000L; // 6h in micros
       final long planDuration = 1212 * 60 * 60 * 1000000L; // 1212h in micros
 
-      final var firstGap = ndRecord.gaps().get(0);
+      final var firstGap = nRecordResults.gaps().get(0);
       assertEquals(0, firstGap.start());
       assertEquals(datasetOffsetMicros, firstGap.end());
 
-      final var secondGap = ndRecord.gaps().get(1);
+      final var secondGap = nRecordResults.gaps().get(1);
       assertEquals(datasetEndMicros, secondGap.start());
       assertEquals(planDuration, secondGap.end());
     }
@@ -408,8 +431,9 @@ void oneViolationOutdatedSimIdPassed() throws IOException {
       assertEquals(1, constraintResponses.size());
 
       final var constraintResponse = constraintResponses.get(0);
-      assertEquals(true,constraintResponse.success());
-      final var record = constraintResponse.result();
+      assertTrue(constraintResponse.success());
+      assertTrue(constraintResponse.result().isPresent());
+      final var record = constraintResponse.result().get();
 
       // Check the Result
       assertEquals(constraintName, record.constraintName());
@@ -454,13 +478,15 @@ void compilationFailsOutdatedSimulationSimDatasetId() throws IOException {
       final int newSimDatasetId = hasura.awaitSimulation(planId).simDatasetId();
       // This test causes the endpoint to throw an exception when it fails to compile the constraint,
       // as it cannot find the external dataset resource in the set of known resource types.
-      final var constraintResponses = hasura.checkConstraints(planId);
+      final var constraintResponses = hasura.checkConstraints(planId, newSimDatasetId);
       assertEquals(1,constraintResponses.size());
-      final var constraintRepsonse = constraintResponses.get(0);
-      assertEquals(false,constraintRepsonse.success());
-      assertEquals(1,constraintRepsonse.errors().size());
-      final ConstraintError error = constraintRepsonse.errors().get(0);
-      assertEquals("Constraint 'fruit_equal_peel': TypeError: TS2345 Argument of type '\"/my_boolean\"' is not assignable to parameter of type 'ResourceName'.",error.message());
+      final var constraintResponse = constraintResponses.get(0);
+      assertFalse(constraintResponse.success());
+      assertTrue(constraintResponse.result().isEmpty());
+      assertEquals(1,constraintResponse.errors().size());
+      final var error = constraintResponse.errors().get(0);
+      assertEquals("Constraint 'fruit_equal_peel' compilation failed:\n"
+                   + " TypeError: TS2345 Argument of type '\"/my_boolean\"' is not assignable to parameter of type 'ResourceName'.",error.message());
     }
 
     @Test
@@ -474,12 +500,58 @@ void compilationFailsOutdatedSimulationNoSimDataset() throws IOException {
       // DSL isn't being generated for datasets that are outdated.
       final var constraintResponses = hasura.checkConstraints(planId);
       assertEquals(1,constraintResponses.size());
-      final var constraintRepsonse = constraintResponses.get(0);
-      assertEquals(false,constraintRepsonse.success());
-      assertEquals(1,constraintRepsonse.errors().size());
-      final ConstraintError error = constraintRepsonse.errors().get(0);
-      assertEquals("Constraint 'fruit_equal_peel': TypeError: TS2345 Argument of type '\"/my_boolean\"' is not assignable to parameter of type 'ResourceName'.",error.message());
+      final var constraintResponse = constraintResponses.get(0);
+      assertFalse(constraintResponse.success());
+      assertTrue(constraintResponse.result().isEmpty());
+      assertEquals(1,constraintResponse.errors().size());
+      final ConstraintError error = constraintResponse.errors().get(0);
+      assertEquals("Constraint 'fruit_equal_peel' compilation failed:\n"
+                   + " TypeError: TS2345 Argument of type '\"/my_boolean\"' is not assignable to parameter of type 'ResourceName'.",error.message());
 
     }
+
+    /**
+     *  This test case evaluates the scenario where a user initially has a functioning constraint.
+     *  However, during the modification process, they introduce a compilation error.
+     *  The goal is to confirm that the user can successfully fix the compilation error,
+     *  ensuring that the constraint compiles correctly and returns a valid response.
+     */
+    @Test
+    @DisplayName("If a constraint is updated and then reverted, Constraints will load the cached value from before the update.")
+    void constraintInvalidModification() throws IOException {
+      // Simulate the plan to insure a new simulation set
+      hasura.awaitSimulation(planId);
+
+      // Check the initial constraints responses it should compile
+      final var constraintsResponses = hasura.checkConstraints(planId);
+      assertEquals(1, constraintsResponses.size());
+      assertTrue(constraintsResponses.get(0).success());
+
+      // Attempt to update a constraint with an invalid syntax and capture the error response
+      hasura.updateConstraint(
+          constraintId,
+          WithExternalDatasets.constraintDefinition + "; error");
+
+      // Check the constraints response after the invalid modification
+      final var constraintsErrorResponses = hasura.checkConstraints(planId);
+      assertEquals(1, constraintsErrorResponses.size());
+      assertFalse(constraintsErrorResponses.get(0).success());
+      assertTrue(constraintsErrorResponses.get(0).result().isEmpty());
+      assertEquals(1, constraintsErrorResponses.get(0).errors().size());
+
+      // Restore the original valid constraint
+      hasura.updateConstraint(
+          constraintId,
+          WithExternalDatasets.constraintDefinition);
+
+      // Check the constraints response after reverting to the valid constraint
+      final var constraintsCacheResponses = hasura.checkConstraints(planId);
+      assertEquals(1, constraintsCacheResponses.size());
+      assertTrue(constraintsCacheResponses.get(0).success());
+
+      // Ensure that the constraints responses are the same before and after the invalid modification
+      assertEquals(constraintsResponses, constraintsCacheResponses);
+    }
+
   }
 }

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

@@ -1,20 +1,19 @@
 package gov.nasa.jpl.aerie.e2e.types;
 
-import javax.json.JsonNumber;
 import javax.json.JsonObject;
-import javax.json.JsonString;
 import java.util.List;
+import java.util.Optional;
 
 public record ConstraintRecord(
     boolean success,
-    ConstraintResult result,
+    Optional<ConstraintResult> result,
     List<ConstraintError> errors
 
   ) {
   public static ConstraintRecord fromJSON(JsonObject json){
     return new ConstraintRecord(
         json.getBoolean("success"),
-        json.getJsonObject("results").isEmpty() ? null : ConstraintResult.fromJSON(json.getJsonObject("results")),
+        json.getJsonObject("results").isEmpty() ? Optional.empty() : Optional.of(ConstraintResult.fromJSON(json.getJsonObject("results"))),
         json.getJsonArray("errors").getValuesAs(ConstraintError::fromJSON));
   }
 }