diff --git a/deployment/hasura/metadata/actions.graphql b/deployment/hasura/metadata/actions.graphql index 1bbb07da81..cadf914d9a 100644 --- a/deployment/hasura/metadata/actions.graphql +++ b/deployment/hasura/metadata/actions.graphql @@ -255,6 +255,7 @@ type ResourceSamplesResponse { type ConstraintResponse { success: String! constraintId: Int!, + constraintInvocationId: Int!, constraintRevision: Int!, constraintName: String!, errors: [UserCodeError!]! diff --git a/deployment/hasura/metadata/databases/tables/merlin/constraints/constraint_definition.yaml b/deployment/hasura/metadata/databases/tables/merlin/constraints/constraint_definition.yaml index 036c67d448..d64396d56e 100644 --- a/deployment/hasura/metadata/databases/tables/merlin/constraints/constraint_definition.yaml +++ b/deployment/hasura/metadata/databases/tables/merlin/constraints/constraint_definition.yaml @@ -75,13 +75,13 @@ select_permissions: insert_permissions: - role: aerie_admin permission: - columns: [constraint_id, definition] + columns: [constraint_id, definition, type, uploaded_jar_id] check: {} set: author: "x-hasura-user-id" - role: user permission: - columns: [constraint_id, definition] + columns: [constraint_id, definition, type, uploaded_jar_id] check: {"_or":[{"metadata":{"public":{"_eq":true}}},{"metadata":{"owner":{"_eq":"X-Hasura-User-Id"}}}]} set: author: "x-hasura-user-id" diff --git a/deployment/hasura/metadata/databases/tables/merlin/constraints/constraint_specification.yaml b/deployment/hasura/metadata/databases/tables/merlin/constraints/constraint_specification.yaml index ec6b9a392e..893727efaa 100644 --- a/deployment/hasura/metadata/databases/tables/merlin/constraints/constraint_specification.yaml +++ b/deployment/hasura/metadata/databases/tables/merlin/constraints/constraint_specification.yaml @@ -34,11 +34,11 @@ select_permissions: insert_permissions: - role: aerie_admin permission: - columns: [plan_id, constraint_id, constraint_revision, enabled] + columns: [plan_id, constraint_id, constraint_revision, enabled, arguments] check: {} - role: user permission: - columns: [plan_id, constraint_id, constraint_revision, enabled] + columns: [plan_id, constraint_id, constraint_revision, enabled, arguments] check: { "_and": [ # User is a plan owner/collaborator { "plan": { "_or": [ { "owner": { "_eq": "X-Hasura-User-Id" } }, @@ -50,11 +50,11 @@ insert_permissions: update_permissions: - role: aerie_admin permission: - columns: [constraint_revision, enabled] + columns: [constraint_revision, enabled, arguments] filter: {} - role: user permission: - columns: [constraint_revision, enabled] + columns: [constraint_revision, enabled, arguments] filter: { "plan": { "_or": [ { "owner": { "_eq": "X-Hasura-User-Id" } },{ "collaborators": { "collaborator": { "_eq": "X-Hasura-User-Id" }}}]}} delete_permissions: - role: aerie_admin diff --git a/deployment/hasura/migrations/Aerie/12_procedural_constraints/down.sql b/deployment/hasura/migrations/Aerie/12_procedural_constraints/down.sql new file mode 100644 index 0000000000..0c24144f78 --- /dev/null +++ b/deployment/hasura/migrations/Aerie/12_procedural_constraints/down.sql @@ -0,0 +1,63 @@ +-- remove procedural constraints + +alter table merlin.constraint_run + drop column arguments; + +alter table merlin.constraint_specification + drop column arguments; + +-- delete all procedural constraints from specs (cascade is restricted) +delete from merlin.constraint_specification cs + using merlin.constraint_definition cd +where cd.constraint_id = cs.constraint_id + and cd.type = 'JAR'::merlin.constraint_type; + +-- delete all procedural constraint metadata and definitions +delete from merlin.constraint_metadata cm + using merlin.constraint_definition cd +where cd.constraint_id = cm.id + and cd.type = 'JAR'::merlin.constraint_type; + +alter table merlin.constraint_definition + drop constraint constraint_procedure_has_uploaded_jar, + drop constraint check_constraint_definition_type_consistency, + + alter column definition set not null, + + drop column type, + drop column uploaded_jar_id, + drop column parameter_schema; + +drop type merlin.constraint_type; + +-- revert constraint invocations + +-- remove all but the first invocation of a constraint +-- we need to revert to a more constrained primary key, so we need to delete data +-- keeping the first invocation is just a convention +delete +from merlin.constraint_specification +where invocation_id not in (select min(invocation_id) + from merlin.constraint_specification + group by plan_id, constraint_id); + +alter table merlin.constraint_specification + drop constraint constraint_specification_pkey, + add constraint constraint_specification_pkey + primary key (plan_id, constraint_id), + + drop column invocation_id; + +-- similar action with constraint_runs +delete +from merlin.constraint_run +where constraint_run.constraint_invocation_id not in (select min(constraint_invocation_id) + from merlin.constraint_run + group by constraint_id, constraint_revision, simulation_dataset_id); + +alter table merlin.constraint_run + drop constraint constraint_run_key, + add constraint constraint_run_key + primary key (constraint_id, constraint_revision, simulation_dataset_id), + + drop column constraint_invocation_id; diff --git a/deployment/hasura/migrations/Aerie/12_procedural_constraints/up.sql b/deployment/hasura/migrations/Aerie/12_procedural_constraints/up.sql new file mode 100644 index 0000000000..c1bd228323 --- /dev/null +++ b/deployment/hasura/migrations/Aerie/12_procedural_constraints/up.sql @@ -0,0 +1,63 @@ +-- constraint invocation support +alter table merlin.constraint_specification + add column invocation_id integer generated by default as identity, + + drop constraint constraint_specification_pkey, + add constraint constraint_specification_pkey + primary key (invocation_id); + +comment on column merlin.constraint_specification.invocation_id is e'' + 'The id of a specific constraint invocation in the specification. Primary key.'; + +-- update constraint_run PK +alter table merlin.constraint_run + -- temp set as nullable so we can insert, made explictly not null below + add column constraint_invocation_id integer null, + + drop constraint constraint_run_key; + +-- copy invocation_ids from spec +update merlin.constraint_run cr +set constraint_invocation_id = cs.invocation_id +from merlin.constraint_specification cs +where cs.constraint_id = cr.constraint_id; + +-- delete cached constraint runs that aren't referenced by a spec anymore (this isn't ideal) +delete from merlin.constraint_run where constraint_invocation_id is null; + +alter table merlin.constraint_run + add constraint constraint_run_key + primary key (constraint_invocation_id, simulation_dataset_id); + + +-- procedural constraints support +create type merlin.constraint_type as enum ('EDSL', 'JAR'); + +alter table merlin.constraint_definition + add column type merlin.constraint_type not null default 'EDSL', + add column uploaded_jar_id integer, + add column parameter_schema jsonb, + + alter column definition drop not null, + + add constraint constraint_procedure_has_uploaded_jar + foreign key (uploaded_jar_id) + references merlin.uploaded_file + on update cascade + on delete restrict, + + add constraint check_constraint_definition_type_consistency + check ( + (type = 'EDSL' and definition is not null and uploaded_jar_id is null) + or + (type = 'JAR' and uploaded_jar_id is not null and definition is null) + ); + +alter table merlin.constraint_specification + add column arguments jsonb not null default '{}'::jsonb; + +alter table merlin.constraint_run + add column arguments jsonb not null; + + +call migrations.mark_migration_applied('12'); diff --git a/deployment/postgres-init-db/sql/init_merlin.sql b/deployment/postgres-init-db/sql/init_merlin.sql index eca054c4c5..c159a2b3e2 100644 --- a/deployment/postgres-init-db/sql/init_merlin.sql +++ b/deployment/postgres-init-db/sql/init_merlin.sql @@ -10,6 +10,7 @@ begin; \ir types/merlin/merlin-arguments.sql \ir types/merlin/activity-directive-metadata.sql \ir types/merlin/plan-merge-types.sql + \ir types/merlin/constraint_type.sql ------------ -- Tables diff --git a/deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_definition.sql b/deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_definition.sql index 92bcfb4d2c..58dc041350 100644 --- a/deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_definition.sql +++ b/deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_definition.sql @@ -1,7 +1,11 @@ create table merlin.constraint_definition( constraint_id integer not null, revision integer not null default 0, - definition text not null, + + type merlin.constraint_type not null default 'EDSL', + definition text, + uploaded_jar_id integer, + parameter_schema jsonb, author text, created_at timestamptz not null default now(), @@ -16,7 +20,18 @@ create table merlin.constraint_definition( foreign key (author) references permissions.users on update cascade - on delete set null + on delete set null, + constraint constraint_procedure_has_uploaded_jar + foreign key (uploaded_jar_id) + references merlin.uploaded_file + on update cascade + on delete restrict, + constraint check_constraint_definition_type_consistency + check ( + (type = 'EDSL' and definition is not null and uploaded_jar_id is null) + or + (type = 'JAR' and uploaded_jar_id is not null and definition is null) + ) ); comment on table merlin.constraint_definition is e'' diff --git a/deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_run.sql b/deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_run.sql index 3cf066a5b4..481da89f90 100644 --- a/deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_run.sql +++ b/deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_run.sql @@ -2,6 +2,8 @@ create table merlin.constraint_run ( constraint_id integer not null, constraint_revision integer not null, simulation_dataset_id integer not null, + constraint_invocation_id integer not null, + arguments jsonb not null, results jsonb not null default '{}', @@ -10,7 +12,7 @@ create table merlin.constraint_run ( requested_at timestamptz not null default now(), constraint constraint_run_key - primary key (constraint_id, constraint_revision, simulation_dataset_id), + primary key (constraint_invocation_id, simulation_dataset_id), constraint constraint_run_to_constraint_definition foreign key (constraint_id, constraint_revision) references merlin.constraint_definition diff --git a/deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_specification.sql b/deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_specification.sql index def39d9239..e72a7127e6 100644 --- a/deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_specification.sql +++ b/deployment/postgres-init-db/sql/tables/merlin/constraints/constraint_specification.sql @@ -1,4 +1,5 @@ create table merlin.constraint_specification( + invocation_id integer generated by default as identity, plan_id integer not null references merlin.plan on update cascade @@ -6,9 +7,10 @@ create table merlin.constraint_specification( constraint_id integer not null, constraint_revision integer, -- latest is NULL enabled boolean not null default true, + arguments jsonb not null default '{}'::jsonb, constraint constraint_specification_pkey - primary key (plan_id, constraint_id), + primary key (invocation_id), constraint plan_spec_constraint_exists foreign key (constraint_id) references merlin.constraint_metadata(id) diff --git a/deployment/postgres-init-db/sql/types/merlin/constraint_type.sql b/deployment/postgres-init-db/sql/types/merlin/constraint_type.sql new file mode 100644 index 0000000000..514d52f758 --- /dev/null +++ b/deployment/postgres-init-db/sql/types/merlin/constraint_type.sql @@ -0,0 +1 @@ +create type merlin.constraint_type as enum ('EDSL', 'JAR'); diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/ConstraintsTests.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/ConstraintsTests.java index 50aa3728f6..c3e428ba63 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/ConstraintsTests.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/ConstraintsTests.java @@ -2,6 +2,7 @@ import com.microsoft.playwright.Playwright; import gov.nasa.jpl.aerie.e2e.types.ConstraintError; +import gov.nasa.jpl.aerie.e2e.types.ConstraintInvocationId; import gov.nasa.jpl.aerie.e2e.types.ConstraintRecord; import gov.nasa.jpl.aerie.e2e.types.ExternalDataset.ProfileInput; import gov.nasa.jpl.aerie.e2e.types.ExternalDataset.ProfileInput.ProfileSegmentInput; @@ -32,6 +33,7 @@ public class ConstraintsTests { private int planId; private int activityId; private int constraintId; + private int invocationId; private final String constraintName = "fruit_equal_peel"; private final String constraintDefinition = @@ -74,11 +76,13 @@ void beforeEach() throws IOException, InterruptedException { "1h", Json.createObjectBuilder().add("biteSize", 1).build()); // Insert the Constraint - constraintId = hasura.insertPlanConstraint( + final var insertResp = hasura.insertPlanConstraint( constraintName, planId, constraintDefinition, ""); + constraintId = insertResp.id(); + invocationId = insertResp.invocationId(); } @AfterEach @@ -156,7 +160,7 @@ void constraintsSucceedAgainstVariantWithUnits() throws IOException { // Check the Response final var constraintResponse = constraintsResponses.get(0); assertTrue(constraintResponse.success()); - assertEquals(fruitConstraintId, constraintResponse.constraintId()); + assertEquals(fruitConstraintId.id(), constraintResponse.constraintId()); assertEquals(fruitConstraintName, constraintResponse.constraintName()); // Check the Result assertTrue(constraintResponse.result().isPresent()); @@ -358,7 +362,7 @@ void constraintVersionLocking() throws IOException { hasura.awaitSimulation(planId); // Update the plan's constraint specification to use a specific version - hasura.updatePlanConstraintSpecVersion(planId, constraintId, 0); + hasura.updatePlanConstraintSpecVersion(constraintId, 0); // Update definition to have invalid syntax final int newRevision = hasura.updateConstraintDefinition( @@ -376,7 +380,7 @@ void constraintVersionLocking() throws IOException { assertTrue(initConstraint.result().isPresent()); // Update constraint spec to use invalid definition - hasura.updatePlanConstraintSpecVersion(planId, constraintId, newRevision); + hasura.updatePlanConstraintSpecVersion(invocationId, newRevision); // Check constraints -- should fail final var badDefinitionResults = hasura.checkConstraints(planId); @@ -394,7 +398,7 @@ void constraintVersionLocking() throws IOException { badConstraint.errors().get(1).message()); // Update constraint spec to use initial definition - hasura.updatePlanConstraintSpecVersion(planId, constraintId, 0); + hasura.updatePlanConstraintSpecVersion(invocationId, 0); // Check constraints -- should match assertEquals(initResults, hasura.checkConstraints(planId)); @@ -406,13 +410,16 @@ void constraintIgnoreDisabled() throws IOException { hasura.awaitSimulation(planId); // Add a problematic constraint to the spec, then disable it final String problemConstraintName = "bad constraint"; - final int problemConstraintId = hasura.insertPlanConstraint( + final var problemConstraintId = hasura.insertPlanConstraint( problemConstraintName, planId, "error :-(", "constraint that shouldn't compile"); + + final var invocationId = problemConstraintId.invocationId(); + try { - hasura.updatePlanConstraintSpecEnabled(planId, problemConstraintId, false); + hasura.updatePlanConstraintSpecEnabled(invocationId, false); // Check constraints -- Validate that only the enabled constraint is included final var initResults = hasura.checkConstraints(planId); @@ -421,7 +428,7 @@ void constraintIgnoreDisabled() throws IOException { assertTrue(initResults.get(0).success()); // Enable disabled constraint - hasura.updatePlanConstraintSpecEnabled(planId, problemConstraintId, true); + hasura.updatePlanConstraintSpecEnabled(invocationId, true); // Check constraints -- Validate that the other constraint is present and a failure final var results = hasura.checkConstraints(planId); @@ -431,7 +438,7 @@ void constraintIgnoreDisabled() throws IOException { assertTrue(results.get(0).success()); final var problemResults = results.get(1); - assertEquals(problemConstraintId, problemResults.constraintId()); + assertEquals(problemConstraintId.id(), problemResults.constraintId()); assertFalse(problemResults.success()); assertEquals(problemConstraintName, problemResults.constraintName()); assertEquals(2, problemResults.errors().size()); @@ -442,7 +449,7 @@ void constraintIgnoreDisabled() throws IOException { assertEquals("Constraint 'bad constraint' compilation failed:\n TypeError: TS1109 Expression expected.", problemResults.errors().get(1).message()); } finally { - hasura.deleteConstraint(problemConstraintId); + hasura.deleteConstraint(problemConstraintId.id()); } } diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ConstraintInvocationId.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ConstraintInvocationId.java new file mode 100644 index 0000000000..2ebea49632 --- /dev/null +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/ConstraintInvocationId.java @@ -0,0 +1,3 @@ +package gov.nasa.jpl.aerie.e2e.types; + +public record ConstraintInvocationId(int id, int invocationId) {} 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 8d07e3fa95..a5c72bf639 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 @@ -581,6 +581,7 @@ query GetTopicsEvents($datasetId: Int!) { mutation insertConstraintAssignToPlanSpec($constraint: constraint_specification_insert_input!) { constraint: insert_constraint_specification_one(object: $constraint){ constraint_id + invocation_id } }"""), INSERT_PROFILE(""" @@ -655,9 +656,9 @@ mutation updateConstraint($constraintId: Int!, $constraintDefinition: String!) { } }"""), UPDATE_CONSTRAINT_SPEC_VERSION(""" - mutation updateConstraintSpecVersion($plan_id: Int!, $constraint_id: Int!, $constraint_revision: Int!) { + mutation updateConstraintSpecVersion($invocation_id: Int!, $constraint_revision: Int!) { update_constraint_specification_by_pk( - pk_columns: {constraint_id: $constraint_id, plan_id: $plan_id}, + pk_columns: {invocation_id: $invocation_id}, _set: {constraint_revision: $constraint_revision} ) { plan_id @@ -667,13 +668,14 @@ mutation updateConstraintSpecVersion($plan_id: Int!, $constraint_id: Int!, $cons } }"""), UPDATE_CONSTRAINT_SPEC_ENABLED(""" - mutation updateConstraintSpecVersion($plan_id: Int!, $constraint_id: Int!, $enabled: Boolean!) { + mutation updateConstraintSpecVersion($invocation_id: Int!, $enabled: Boolean!) { update_constraint_specification_by_pk( - pk_columns: {constraint_id: $constraint_id, plan_id: $plan_id}, + pk_columns: {invocation_id: $invocation_id}, _set: {enabled: $enabled} ) { plan_id constraint_id + invocation_id constraint_revision enabled } diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/HasuraRequests.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/HasuraRequests.java index 71b5dff661..ba441ef134 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/HasuraRequests.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/HasuraRequests.java @@ -1166,7 +1166,7 @@ public List getConstraintRuns(int simulationDatasetId) thro return cachedRuns.getValuesAs(e -> CachedConstraintRun.fromJSON(e.asJsonObject())); } - public int insertPlanConstraint(String name, int planId, String definition, String description) throws IOException { + public ConstraintInvocationId insertPlanConstraint(String name, int planId, String definition, String description) throws IOException { final var constraintInsertBuilder = Json.createObjectBuilder() .add("plan_id", planId) .add("constraint_metadata", @@ -1181,22 +1181,24 @@ public int insertPlanConstraint(String name, int planId, String definition, Stri Json.createObjectBuilder() .add("definition", definition))))); final var variables = Json.createObjectBuilder().add("constraint", constraintInsertBuilder).build(); - return makeRequest(GQL.INSERT_PLAN_SPEC_CONSTRAINT, variables).getJsonObject("constraint").getInt("constraint_id"); + final var resp = makeRequest(GQL.INSERT_PLAN_SPEC_CONSTRAINT, variables).getJsonObject("constraint"); + return new ConstraintInvocationId( + resp.getInt("constraint_id"), + resp.getInt("invocation_id") + ); } - public void updatePlanConstraintSpecVersion(int planId, int constraintId, int constraintRevision) throws IOException { + public void updatePlanConstraintSpecVersion(int invocationId, int constraintRevision) throws IOException { final var variables = Json.createObjectBuilder() - .add("plan_id", planId) - .add("constraint_id", constraintId) + .add("invocation_id", invocationId) .add("constraint_revision", constraintRevision) .build(); makeRequest(GQL.UPDATE_CONSTRAINT_SPEC_VERSION, variables); } - public void updatePlanConstraintSpecEnabled(int planId, int constraintId, boolean enabled) throws IOException { + public void updatePlanConstraintSpecEnabled(int invocationId, boolean enabled) throws IOException { final var variables = Json.createObjectBuilder() - .add("plan_id", planId) - .add("constraint_id", constraintId) + .add("invocation_id", invocationId) .add("enabled", enabled) .build(); makeRequest(GQL.UPDATE_CONSTRAINT_SPEC_ENABLED, variables); diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/MerlinBindings.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/MerlinBindings.java index 2558cd3278..368773fe85 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/MerlinBindings.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/MerlinBindings.java @@ -32,6 +32,8 @@ import java.util.Map; import java.util.stream.Collectors; +import static gov.nasa.jpl.aerie.merlin.server.http.MerlinParsers.parseJson; + import static gov.nasa.jpl.aerie.merlin.server.http.HasuraParsers.hasuraActivityActionP; import static gov.nasa.jpl.aerie.merlin.server.http.HasuraParsers.hasuraActivityBulkActionP; import static gov.nasa.jpl.aerie.merlin.server.http.HasuraParsers.hasuraConstraintsCodeAction; @@ -513,18 +515,6 @@ private void getConstraintsDslTypescript(final Context ctx) { } } - private T parseJson(final String subject, final JsonParser parser) - throws InvalidJsonException, InvalidEntityException - { - try { - final var requestJson = Json.createReader(new StringReader(subject)).readValue(); - final var result = parser.parse(requestJson); - return result.getSuccessOrThrow($ -> new InvalidEntityException(List.of($))); - } catch (JsonParsingException e) { - throw new InvalidJsonException(e); - } - } - private void checkPermissions( final Action action, final HasuraAction.Session session, diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/MerlinParsers.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/MerlinParsers.java index 12501047cf..e6a78c5d71 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/MerlinParsers.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/http/MerlinParsers.java @@ -16,7 +16,10 @@ import javax.json.Json; import javax.json.JsonObject; import javax.json.JsonValue; +import javax.json.stream.JsonParsingException; +import java.io.StringReader; import java.time.format.DateTimeParseException; +import java.util.List; import java.util.Optional; import static gov.nasa.jpl.aerie.json.BasicParsers.*; import static gov.nasa.jpl.aerie.json.Uncurry.tuple; @@ -103,4 +106,17 @@ public JsonValue unparse(final Timestamp value) { untuple((type, message, data, trace, timestamp) -> new SimulationFailure(type, message, data, trace.orElse(""), timestamp.toInstant())), failure -> tuple(failure.type(), failure.message(), failure.data(), Optional.ofNullable(failure.trace()), new Timestamp(failure.timestamp())) ); + + public static T parseJson(final String subject, final JsonParser parser) + throws InvalidJsonException, InvalidEntityException + { + try { + final var requestJson = Json.createReader(new StringReader(subject)).readValue(); + final var result = parser.parse(requestJson); + return result.getSuccessOrThrow($ -> new InvalidEntityException(List.of($))); + } catch (JsonParsingException e) { + throw new InvalidJsonException(e); + } + } + } 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 a47e503e6c..26a9bf7e0a 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 @@ -334,6 +334,7 @@ public static JsonValue serializeConstraintResults(final Map invocationId, Map arguments, String name, String description, String definition) { + public Constraint(Long id, Long revision, String name, String description, String definition) { + this(id, revision, Optional.empty(), Map.of(), name, description, definition); + } + public Constraint(Long id, Long revision, Long invocationId, Map arguments, String name, String description, String definition) { + this(id, revision, Optional.of(invocationId), arguments, name, description, definition); + } } diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/ConstraintRecord.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/ConstraintRecord.java index 097b0c886f..cedef07999 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/ConstraintRecord.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/ConstraintRecord.java @@ -1,8 +1,14 @@ package gov.nasa.jpl.aerie.merlin.server.remotes.postgres; +import gov.nasa.jpl.aerie.merlin.protocol.types.SerializedValue; + +import java.util.Map; + public record ConstraintRecord( long id, long revision, + long invocationId, String name, String description, - String definition) {} + String definition, + Map arguments) {} diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/ConstraintRunRecord.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/ConstraintRunRecord.java index e66bb4d7b2..36d81d9b22 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/ConstraintRunRecord.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/ConstraintRunRecord.java @@ -1,8 +1,13 @@ package gov.nasa.jpl.aerie.merlin.server.remotes.postgres; import gov.nasa.jpl.aerie.constraints.model.ConstraintResult; +import gov.nasa.jpl.aerie.merlin.protocol.types.SerializedValue; + +import java.util.Map; public record ConstraintRunRecord( long constraintId, + long constraintInvocationId, + Map arguments, ConstraintResult result ) {} diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetPlanConstraintsAction.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetPlanConstraintsAction.java index 4acd8ce212..0d2e0fd416 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetPlanConstraintsAction.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetPlanConstraintsAction.java @@ -1,5 +1,8 @@ package gov.nasa.jpl.aerie.merlin.server.remotes.postgres; +import gov.nasa.jpl.aerie.merlin.driver.json.SerializedValueJsonParser; +import gov.nasa.jpl.aerie.merlin.server.http.InvalidEntityException; +import gov.nasa.jpl.aerie.merlin.server.http.InvalidJsonException; import org.intellij.lang.annotations.Language; import java.sql.Connection; @@ -9,14 +12,16 @@ import java.util.List; import java.util.Optional; +import static gov.nasa.jpl.aerie.merlin.server.http.MerlinParsers.parseJson; + /*package local*/ final class GetPlanConstraintsAction implements AutoCloseable { // We left join through the plan table in order to distinguish // a plan without any enabled constraints from a non-existent plan. // A plan without any enabled constraints will produce a placeholder row with nulls. private static final @Language("SQL") String sql = """ - select c.constraint_id, c.revision, c.name, c.description, c.definition + select c.constraint_id, c.revision, c.invocation_id, c.name, c.description, c.definition, c.arguments from merlin.plan p - left join (select cs.plan_id, cs.constraint_id, cd.revision, cm.name, cm.description, cd.definition + left join (select cs.plan_id, cs.constraint_id, cs.invocation_id, cs.arguments, cd.revision, cm.name, cm.description, cd.definition from merlin.constraint_specification cs left join merlin.constraint_definition cd using (constraint_id) left join merlin.constraint_metadata cm on cs.constraint_id = cm.id @@ -52,14 +57,18 @@ public Optional> get(final long planId) throws SQLExcepti final var constraint = new ConstraintRecord( results.getLong(1), results.getLong(2), - results.getString(3), + results.getLong(3), results.getString(4), - results.getString(5)); + results.getString(5), + results.getString(6), + parseJson(results.getString(7), new SerializedValueJsonParser()).asMap().get()); constraints.add(constraint); } while (results.next()); return Optional.of(constraints); + } catch (InvalidJsonException | InvalidEntityException e) { + throw new SQLException(e); } } diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetValidConstraintRunsAction.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetValidConstraintRunsAction.java index 5d2b910ba1..e1074a577b 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetValidConstraintRunsAction.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/GetValidConstraintRunsAction.java @@ -1,5 +1,8 @@ package gov.nasa.jpl.aerie.merlin.server.remotes.postgres; +import gov.nasa.jpl.aerie.merlin.driver.json.SerializedValueJsonParser; +import gov.nasa.jpl.aerie.merlin.server.http.InvalidEntityException; +import gov.nasa.jpl.aerie.merlin.server.http.InvalidJsonException; import gov.nasa.jpl.aerie.merlin.server.models.Constraint; import gov.nasa.jpl.aerie.merlin.server.models.SimulationDatasetId; import org.intellij.lang.annotations.Language; @@ -14,13 +17,17 @@ import static gov.nasa.jpl.aerie.constraints.json.ConstraintParsers.constraintResultP; import static gov.nasa.jpl.aerie.merlin.server.remotes.postgres.PostgresParsers.getJsonColumn; +import static gov.nasa.jpl.aerie.merlin.server.http.MerlinParsers.parseJson; + final class GetValidConstraintRunsAction implements AutoCloseable { private static final @Language("SQL") String sql = """ select cr.constraint_id, cr.constraint_revision, cr.simulation_dataset_id, - cr.results + cr.results, + cr.constraint_invocation_id, + cr.arguments from merlin.constraint_run as cr where cr.constraint_id = any(?) and cr.simulation_dataset_id = ?; @@ -45,8 +52,11 @@ public List get() throws SQLException { while (results.next()) { final var constraintId = results.getLong("constraint_id"); + final var constraintInvocationId = results.getLong("constraint_invocation_id"); final var constraintRevision = results.getLong("constraint_revision"); + final var constraintArguments = parseJson(results.getString("arguments"), new SerializedValueJsonParser()).asMap().get(); + // The cached result wasn't for the correct revision if(constraints.get(constraintId).revision() != constraintRevision) continue; @@ -54,16 +64,20 @@ public List get() throws SQLException { // The constraint run didn't have any violations if (resultString.equals("{}")) { - constraintRuns.add(new ConstraintRunRecord(constraintId, null)); + constraintRuns.add(new ConstraintRunRecord(constraintId, constraintInvocationId, constraintArguments, null)); } else { constraintRuns.add(new ConstraintRunRecord( constraintId, + constraintInvocationId, + constraintArguments, getJsonColumn(results, "results", constraintResultP) .getSuccessOrThrow($ -> new Error("Corrupt results cannot be parsed: " + $.reason())))); } } return constraintRuns; + } catch (InvalidJsonException | InvalidEntityException e) { + throw new SQLException(e); } } diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/InsertConstraintRunsAction.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/InsertConstraintRunsAction.java index 3546fd8c25..1a56e4ade6 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/InsertConstraintRunsAction.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/InsertConstraintRunsAction.java @@ -9,12 +9,13 @@ import java.sql.SQLException; import java.util.Map; +import static gov.nasa.jpl.aerie.merlin.server.remotes.postgres.PostgresParsers.constraintArgumentsP; import static gov.nasa.jpl.aerie.constraints.json.ConstraintParsers.constraintResultP; /* package local */ class InsertConstraintRunsAction implements AutoCloseable { private static final @Language("SQL") String sql = """ - insert into merlin.constraint_run (constraint_id, constraint_revision, simulation_dataset_id, results) - values (?, ?, ?, ?::json) + insert into merlin.constraint_run (constraint_id, constraint_revision, constraint_invocation_id, arguments, simulation_dataset_id, results) + values (?, ?, ?, ?::jsonb, ?, ?::jsonb) """; private final PreparedStatement statement; @@ -30,12 +31,14 @@ public void apply( for (Constraint constraint : constraintMap.values()) { statement.setLong(1, constraint.id()); statement.setLong(2, constraint.revision()); - statement.setLong(3, simulationDatasetId); + statement.setLong(3, constraint.invocationId().get()); + statement.setString(4, constraintArgumentsP.unparse(constraint.arguments()).toString()); + statement.setLong(5, simulationDatasetId); if (results.get(constraint.id()) != null) { - statement.setString(4, constraintResultP.unparse(results.get(constraint.id())).toString()); + statement.setString(6, constraintResultP.unparse(results.get(constraint.id())).toString()); } else { - statement.setString(4, "{}"); + statement.setString(6, "{}"); } this.statement.addBatch(); diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresConstraintRepository.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresConstraintRepository.java index edf533d459..7ab9ae9bb0 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresConstraintRepository.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresConstraintRepository.java @@ -40,7 +40,7 @@ public Map getValidConstraintRuns(Map(); for (final var constraintRun : constraintRuns) { - validConstraintRuns.put(constraintRun.constraintId(), constraintRun); + validConstraintRuns.put(constraintRun.constraintInvocationId(), constraintRun); } return validConstraintRuns; diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresParsers.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresParsers.java index 87cf3d3057..b0fe1252d0 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresParsers.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresParsers.java @@ -116,6 +116,7 @@ public static Duration parseDurationISO8601(final String iso8601String){ return Duration.of((javaDuration.getSeconds() * 1_000_000L) + (javaDuration.getNano() / 1000L), Duration.MICROSECONDS); } + public static final JsonParser> constraintArgumentsP = mapP(serializedValueP); public static final JsonParser> activityArgumentsP = mapP(serializedValueP); public static final JsonParser> simulationArgumentsP = mapP(serializedValueP); diff --git a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresPlanRepository.java b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresPlanRepository.java index 10a496fe4f..01003a7281 100644 --- a/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresPlanRepository.java +++ b/merlin-server/src/main/java/gov/nasa/jpl/aerie/merlin/server/remotes/postgres/PostgresPlanRepository.java @@ -188,10 +188,12 @@ public Map getPlanConstraints(final PlanId planId) throws NoSu .orElseThrow(() -> new NoSuchPlanException(planId)) .stream() .collect(Collectors.toMap( - ConstraintRecord::id, + ConstraintRecord::invocationId, r -> new Constraint( r.id(), r.revision(), + r.invocationId(), + r.arguments(), r.name(), r.description(), r.definition()))); 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 3b16377768..2426047e0a 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 @@ -64,7 +64,7 @@ public Map> getViolations(final PlanId planId, final Opt // Remove any constraints that we've already checked, so they aren't rechecked. for (ConstraintRunRecord constraintRun : validConstraintRuns.values()) { - constraintResultMap.put(constraintCode.remove(constraintRun.constraintId()), Fallible.of(constraintRun.result())); + constraintResultMap.put(constraintCode.remove(constraintRun.constraintInvocationId()), Fallible.of(constraintRun.result())); } // If the lengths don't match we need check the left-over constraints.