From f597278c7f06a738d57146ba155cf3cff2b688d6 Mon Sep 17 00:00:00 2001 From: Theresa Kamerman Date: Tue, 3 Oct 2023 11:36:44 -0700 Subject: [PATCH] Adjust ConstraintAction error messages The errors will now correctly distinguish between a plan never being simulated and a specified sim dataset not existing. Additionally, the serialializer for SimDatasetMismatch now refers to the correct extension. --- e2e-tests/src/tests/bindings.test.ts | 18 ++++++++++++++++-- .../server/http/ResponseSerializers.java | 2 +- .../server/services/ConstraintAction.java | 10 +++++++--- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/e2e-tests/src/tests/bindings.test.ts b/e2e-tests/src/tests/bindings.test.ts index 8bf55c4641..4de62840dc 100644 --- a/e2e-tests/src/tests/bindings.test.ts +++ b/e2e-tests/src/tests/bindings.test.ts @@ -182,6 +182,20 @@ test.describe('Merlin Bindings', () => { expect(response.status()).toEqual(404); expect((await response.json()).message).toEqual('no such plan'); + // Returns a 404 if the SimulationDataset id is invalid + // message is "input mismatch exception" + response = await request.post(`${urls.MERLIN_URL}/constraintViolations`, { + data: { + action: {name: "check_constraints"}, + input: {planId: plan_id, simulationDatasetId: -1}, + request_query: "", + session_variables: admin.session}}); + expect(response.status()).toEqual(404); + expect((await response.json())).toEqual({ + message: "input mismatch exception", + cause: "simulation dataset with id `-1` does not exist" + }); + // Returns a 403 if unauthorized response = await request.post(`${urls.MERLIN_URL}/constraintViolations`, { data: { @@ -204,7 +218,7 @@ test.describe('Merlin Bindings', () => { expect(response.status()).toEqual(404); expect((await response.json())).toEqual({ message: "input mismatch exception", - cause: "no simulation datasets found for plan id " + plan_id + cause: "plan with id " + plan_id +" has not yet been simulated at its current revision" }); // Returns a 404 if given an invalid simulation dataset id @@ -218,7 +232,7 @@ test.describe('Merlin Bindings', () => { session_variables: admin.session}}); expect(response.status()).toEqual(404); expect((await response.json())).toEqual({ - message: "input mismatch exception", + message: "simulation dataset mismatch exception", cause: "Simulation Dataset with id `" + invalidSimDatasetId + "` does not belong to Plan with id `"+plan_id+"`" }); 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 5a66e90e69..185b5f3143 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 @@ -471,7 +471,7 @@ public static JsonValue serializeInputMismatchException(final InputMismatchExcep public static JsonValue serializeSimulationDatasetMismatchException(final SimulationDatasetMismatchException ex){ return Json.createObjectBuilder() - .add("message", "input mismatch exception") + .add("message", "simulation dataset mismatch exception") .add("cause", ex.getMessage()) .build(); } 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 cfffe041e4..4a5a442f12 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 @@ -51,11 +51,18 @@ public List getViolations(final PlanId planId, final Optional< throws NoSuchPlanException, MissionModelService.NoSuchMissionModelException, SimulationDatasetMismatchException { final var plan = this.planService.getPlanForValidation(planId); final Optional resultsHandle$; + final SimulationDatasetId simDatasetId; if (simulationDatasetId.isPresent()) { resultsHandle$ = this.simulationService.get(planId, simulationDatasetId.get()); + simDatasetId = resultsHandle$ + .map(SimulationResultsHandle::getSimulationDatasetId) + .orElseThrow(() -> new InputMismatchException("simulation dataset with id `" + simulationDatasetId.get().id() + "` does not exist")); } else { final var revisionData = this.planService.getPlanRevisionData(planId); resultsHandle$ = this.simulationService.get(planId, revisionData); + simDatasetId = resultsHandle$ + .map(SimulationResultsHandle::getSimulationDatasetId) + .orElseThrow(() -> new InputMismatchException("plan with id " + planId.id() +" has not yet been simulated at its current revision")); } @@ -68,9 +75,6 @@ public List getViolations(final PlanId planId, final Optional< throw new RuntimeException("Assumption falsified -- mission model for existing plan does not exist"); } - final var simDatasetId = resultsHandle$ - .map(SimulationResultsHandle::getSimulationDatasetId) - .orElseThrow(() -> new InputMismatchException("no simulation datasets found for plan id " + planId.id())); final var violations = new HashMap(); final var validConstraintRuns = this.constraintService.getValidConstraintRuns(constraintCode.values().stream().toList(), simDatasetId);