diff --git a/action-server/src/listeners/dbListeners.ts b/action-server/src/listeners/dbListeners.ts index 21c35d5537..cf618cdf7b 100644 --- a/action-server/src/listeners/dbListeners.ts +++ b/action-server/src/listeners/dbListeners.ts @@ -5,11 +5,10 @@ import type { PoolClient } from "pg"; import { configuration } from "../config"; import { ActionsDbManager } from "../db"; import { ActionWorkerPool } from "../threads/workerPool"; -import type { ActionDefinitionInsertedPayload, ActionResponse, ActionRunInsertedPayload } from "../type/types"; +import { ActionRunner } from "../type/actionRunner"; +import type { ActionDefinitionInsertedPayload, ActionResponse, ActionRunCancellationRequestPayload, ActionRunInsertedPayload } from "../type/types"; import { extractSchemas } from "../utils/codeRunner"; import logger from "../utils/logger"; -import { ActionRunCancellationRequestPayload } from "../type/types"; -import { ActionRunner } from "../type/actionRunner"; let listenClient: PoolClient | undefined; @@ -30,11 +29,11 @@ async function refreshActionDefinitionSchema(payload: ActionDefinitionInsertedPa const pool = ActionsDbManager.getDb(); const query = ` - UPDATE actions.action_definition + UPDATE actions.action_definition_version SET parameter_schema = $1::jsonb, settings_schema = $2::jsonb - WHERE id = $3 + WHERE action_definition_id = $3 AND revision = $4 RETURNING *; `; @@ -43,9 +42,34 @@ async function refreshActionDefinitionSchema(payload: ActionDefinitionInsertedPa JSON.stringify(schemas.parameterDefinitions), JSON.stringify(schemas.settingDefinitions), payload.action_definition_id, + payload.revision, ]); - logger.info("Updated action_definition:", res.rows[0]); + logger.info("Updated action_definition_version:", res.rows[0]); + + // Strip stale settings from parent action_definition. + // Settings keys no longer in the new version's schema are removed. + const newSettingsKeys = Object.keys(schemas.settingDefinitions || {}); + const stripStaleSettingsQuery = ` + UPDATE actions.action_definition + SET settings = ( + SELECT COALESCE(jsonb_object_agg(key, value), '{}'::jsonb) + FROM jsonb_each(settings) + WHERE key = ANY($1::text[]) + ) + WHERE id = $2 + AND settings IS NOT NULL + AND settings != '{}'::jsonb + AND EXISTS ( + SELECT 1 FROM jsonb_object_keys(settings) k + WHERE k != ALL($1::text[]) + ); + `; + const stripRes = await pool.query(stripStaleSettingsQuery, [newSettingsKeys, payload.action_definition_id]); + + if (stripRes.rowCount && stripRes.rowCount > 0) { + logger.info(`Stripped stale settings for action_definition ${payload.action_definition_id}`); + } } catch (error) { logger.error("Error updating row:", error); } @@ -140,8 +164,8 @@ export async function setupListeners() { // save listenClient as a global so we can close it on cleanup if necessary listenClient = await pool.connect(); - // these occur when user inserts row in `action_definition`, need to pre-process to extract the schemas - listenClient.query("LISTEN action_definition_inserted"); + // these occur when user inserts row in `action_definition_version`, need to pre-process to extract the schemas + listenClient.query("LISTEN action_definition_version_inserted"); // these occur when a user inserts a row in the `action_run` table, signifying a run request listenClient.query("LISTEN action_run_inserted"); // these occur when a user sets the `canceled` of an `action_run` to true, signifying a cancellation request @@ -157,7 +181,7 @@ export async function setupListeners() { const payload = JSON.parse(msg.payload); - if (msg.channel === "action_definition_inserted") { + if (msg.channel === "action_definition_version_inserted") { await refreshActionDefinitionSchema(payload); } else if (msg.channel === "action_run_inserted") { await ActionRunner.addActionRun(payload as ActionRunInsertedPayload); diff --git a/action-server/src/type/types.ts b/action-server/src/type/types.ts index 73cf004a09..876cc80b87 100644 --- a/action-server/src/type/types.ts +++ b/action-server/src/type/types.ts @@ -45,6 +45,7 @@ export type ActionTask = { export type ActionDefinitionInsertedPayload = { action_definition_id: number; action_file_path: string; + revision: number; }; export type ActionRunInsertedPayload = { @@ -52,6 +53,7 @@ export type ActionRunInsertedPayload = { settings: Record; parameters: Record; action_definition_id: number; + action_definition_revision: number; workspace_id: number; action_file_path: string; has_secrets: boolean; diff --git a/db-tests/src/test/java/gov/nasa/jpl/aerie/database/ActionDefinitionVersionTests.java b/db-tests/src/test/java/gov/nasa/jpl/aerie/database/ActionDefinitionVersionTests.java new file mode 100644 index 0000000000..1108cd282b --- /dev/null +++ b/db-tests/src/test/java/gov/nasa/jpl/aerie/database/ActionDefinitionVersionTests.java @@ -0,0 +1,414 @@ +package gov.nasa.jpl.aerie.database; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +import java.io.IOException; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@SuppressWarnings("SqlSourceToSinkFlow") +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class ActionDefinitionVersionTests { + private DatabaseTestHelper helper; + private Connection connection; + + // Shared prerequisite IDs set up in beforeEach + private int commandDictionaryId; + private int parcelId; + private int workspaceId; + private int actionFileId; + + @BeforeAll + void beforeAll() throws SQLException, IOException, InterruptedException { + helper = new DatabaseTestHelper("aerie_action_version_test", "Action Definition Version Tests"); + connection = helper.connection(); + insertUser("TestAdmin"); + } + + @AfterAll + void afterAll() throws SQLException, IOException, InterruptedException { + helper.close(); + connection = null; + helper = null; + } + + @BeforeEach + void beforeEach() throws SQLException { + // Build the dependency chain: user -> command_dictionary -> parcel -> workspace -> action_definition + commandDictionaryId = insertCommandDictionary(); + parcelId = insertParcel(commandDictionaryId); + workspaceId = insertWorkspace(parcelId); + actionFileId = insertFileUpload(); + } + + @AfterEach + void afterEach() throws SQLException { + helper.clearSchema("actions"); + helper.clearSchema("sequencing"); + helper.clearSchema("merlin"); + } + + //region Helper Methods + private void insertUser(String username) throws SQLException { + try (final var statement = connection.createStatement()) { + statement.execute( + //language=sql + """ + INSERT INTO permissions.users (username, default_role) + VALUES ('%s', 'aerie_admin') + ON CONFLICT DO NOTHING; + """.formatted(username)); + } + } + + private int insertCommandDictionary() throws SQLException { + try (final var statement = connection.createStatement()) { + final var res = statement.executeQuery( + //language=sql + """ + INSERT INTO sequencing.command_dictionary (dictionary_path, mission, version) + VALUES ('test-path', 'test-mission', '%s') + RETURNING id; + """.formatted(UUID.randomUUID().toString())); + res.next(); + return res.getInt("id"); + } + } + + private int insertParcel(int cmdDictId) throws SQLException { + try (final var statement = connection.createStatement()) { + final var res = statement.executeQuery( + //language=sql + """ + INSERT INTO sequencing.parcel (name, command_dictionary_id) + VALUES ('test-parcel', %d) + RETURNING id; + """.formatted(cmdDictId)); + res.next(); + return res.getInt("id"); + } + } + + private int insertWorkspace(int parcelId) throws SQLException { + try (final var statement = connection.createStatement()) { + final var res = statement.executeQuery( + //language=sql + """ + INSERT INTO sequencing.workspace (name, disk_location, parcel_id, owner) + VALUES ('test-workspace', '/tmp/test-%s', %d, 'TestAdmin') + RETURNING id; + """.formatted(UUID.randomUUID().toString(), parcelId)); + res.next(); + return res.getInt("id"); + } + } + + private int insertFileUpload() throws SQLException { + try (final var statement = connection.createStatement()) { + final var uuid = UUID.randomUUID().toString(); + final var res = statement.executeQuery( + //language=sql + """ + INSERT INTO merlin.uploaded_file (path, name) + VALUES ('test-action-path-%s', 'test-action-file-%s') + RETURNING id; + """.formatted(uuid, uuid)); + res.next(); + return res.getInt("id"); + } + } + + private int insertActionDefinition(int workspaceId) throws SQLException { + try (final var statement = connection.createStatement()) { + final var res = statement.executeQuery( + //language=sql + """ + INSERT INTO actions.action_definition (name, description, workspace_id, owner) + VALUES ('test-action', 'A test action', %d, 'TestAdmin') + RETURNING id; + """.formatted(workspaceId)); + res.next(); + return res.getInt("id"); + } + } + + private int insertVersion(int actionDefinitionId, int fileId) throws SQLException { + try (final var statement = connection.createStatement()) { + final var res = statement.executeQuery( + //language=sql + """ + INSERT INTO actions.action_definition_version (action_definition_id, action_file_id, author) + VALUES (%d, %d, 'TestAdmin') + RETURNING revision; + """.formatted(actionDefinitionId, fileId)); + res.next(); + return res.getInt("revision"); + } + } + + private int getVersionCount(int actionDefinitionId) throws SQLException { + try (final var statement = connection.createStatement()) { + final var res = statement.executeQuery( + //language=sql + """ + SELECT count(*) FROM actions.action_definition_version + WHERE action_definition_id = %d; + """.formatted(actionDefinitionId)); + res.next(); + return res.getInt(1); + } + } + //endregion + + @Nested + class RevisionAutoIncrement { + @Test + void firstVersionGetsRevisionZero() throws SQLException { + final var defId = insertActionDefinition(workspaceId); + final var revision = insertVersion(defId, actionFileId); + assertEquals(0, revision); + } + + @Test + void secondVersionGetsRevisionOne() throws SQLException { + final var defId = insertActionDefinition(workspaceId); + final var fileId2 = insertFileUpload(); + + final var rev0 = insertVersion(defId, actionFileId); + final var rev1 = insertVersion(defId, fileId2); + + assertEquals(0, rev0); + assertEquals(1, rev1); + } + + @Test + void revisionsIncrementIndependentlyPerDefinition() throws SQLException { + final var defA = insertActionDefinition(workspaceId); + final var defB = insertActionDefinition(workspaceId); + final var fileId2 = insertFileUpload(); + + // Each definition should start its own revision sequence at 0 + assertEquals(0, insertVersion(defA, actionFileId)); + assertEquals(0, insertVersion(defB, actionFileId)); + assertEquals(1, insertVersion(defA, fileId2)); + assertEquals(1, insertVersion(defB, fileId2)); + } + } + + @Nested + class CascadeDelete { + @Test + void deletingDefinitionDeletesVersions() throws SQLException { + final var defId = insertActionDefinition(workspaceId); + insertVersion(defId, actionFileId); + insertVersion(defId, insertFileUpload()); + assertEquals(2, getVersionCount(defId)); + + try (final var statement = connection.createStatement()) { + statement.executeUpdate( + //language=sql + """ + DELETE FROM actions.action_definition WHERE id = %d; + """.formatted(defId)); + } + + assertEquals(0, getVersionCount(defId)); + } + + @Test + void deletingVersionDoesNotDeleteDefinition() throws SQLException { + final var defId = insertActionDefinition(workspaceId); + insertVersion(defId, actionFileId); + + try (final var statement = connection.createStatement()) { + statement.executeUpdate( + //language=sql + """ + DELETE FROM actions.action_definition_version + WHERE action_definition_id = %d AND revision = 0; + """.formatted(defId)); + } + + // The parent definition should still exist + try (final var statement = connection.createStatement()) { + final var res = statement.executeQuery( + //language=sql + """ + SELECT count(*) FROM actions.action_definition WHERE id = %d; + """.formatted(defId)); + res.next(); + assertEquals(1, res.getInt(1)); + } + } + } + + @Nested + class ActionRunDefaultRevision { + @Test + void runDefaultsToLatestRevision() throws SQLException { + final var defId = insertActionDefinition(workspaceId); + insertVersion(defId, actionFileId); + insertVersion(defId, insertFileUpload()); + // Latest revision is 1 + + try (final var statement = connection.createStatement()) { + // Insert a run WITHOUT specifying action_definition_revision; + // the trigger should auto-populate it with the latest (1) + final var res = statement.executeQuery( + //language=sql + """ + INSERT INTO actions.action_run (settings, parameters, action_definition_id, requested_by) + VALUES ('{}', '{}', %d, 'TestAdmin') + RETURNING action_definition_revision; + """.formatted(defId)); + res.next(); + assertEquals(1, res.getInt("action_definition_revision")); + } + } + + @Test + void runRespectsExplicitRevision() throws SQLException { + final var defId = insertActionDefinition(workspaceId); + insertVersion(defId, actionFileId); + insertVersion(defId, insertFileUpload()); + + try (final var statement = connection.createStatement()) { + // Explicitly request revision 0 (not the latest) + final var res = statement.executeQuery( + //language=sql + """ + INSERT INTO actions.action_run (settings, parameters, action_definition_id, action_definition_revision, requested_by) + VALUES ('{}', '{}', %d, 0, 'TestAdmin') + RETURNING action_definition_revision; + """.formatted(defId)); + res.next(); + assertEquals(0, res.getInt("action_definition_revision")); + } + } + } + + @Nested + class Archiving { + @Test + void definitionDefaultsToNotArchived() throws SQLException { + final var defId = insertActionDefinition(workspaceId); + + try (final var statement = connection.createStatement()) { + final var res = statement.executeQuery( + //language=sql + """ + SELECT archived FROM actions.action_definition WHERE id = %d; + """.formatted(defId)); + res.next(); + assertEquals(false, res.getBoolean("archived")); + } + } + + @Test + void versionDefaultsToNotArchived() throws SQLException { + final var defId = insertActionDefinition(workspaceId); + insertVersion(defId, actionFileId); + + try (final var statement = connection.createStatement()) { + final var res = statement.executeQuery( + //language=sql + """ + SELECT archived FROM actions.action_definition_version + WHERE action_definition_id = %d AND revision = 0; + """.formatted(defId)); + res.next(); + assertEquals(false, res.getBoolean("archived")); + } + } + + @Test + void canArchiveDefinition() throws SQLException { + final var defId = insertActionDefinition(workspaceId); + + try (final var statement = connection.createStatement()) { + statement.executeUpdate( + //language=sql + """ + UPDATE actions.action_definition SET archived = true WHERE id = %d; + """.formatted(defId)); + + final var res = statement.executeQuery( + //language=sql + """ + SELECT archived FROM actions.action_definition WHERE id = %d; + """.formatted(defId)); + res.next(); + assertTrue(res.getBoolean("archived")); + } + } + + @Test + void canArchiveVersion() throws SQLException { + final var defId = insertActionDefinition(workspaceId); + insertVersion(defId, actionFileId); + + try (final var statement = connection.createStatement()) { + statement.executeUpdate( + //language=sql + """ + UPDATE actions.action_definition_version SET archived = true + WHERE action_definition_id = %d AND revision = 0; + """.formatted(defId)); + + final var res = statement.executeQuery( + //language=sql + """ + SELECT archived FROM actions.action_definition_version + WHERE action_definition_id = %d AND revision = 0; + """.formatted(defId)); + res.next(); + assertTrue(res.getBoolean("archived")); + } + } + } + + @Nested + class ForeignKeyConstraints { + @Test + void cannotInsertVersionForNonexistentDefinition() { + assertThrows(SQLException.class, () -> { + try (final var statement = connection.createStatement()) { + statement.executeQuery( + //language=sql + """ + INSERT INTO actions.action_definition_version (action_definition_id, action_file_id, author) + VALUES (-999, %d, 'TestAdmin') + RETURNING revision; + """.formatted(actionFileId)); + } + }); + } + + @Test + void cannotDeleteFileReferencedByVersion() throws SQLException { + final var defId = insertActionDefinition(workspaceId); + insertVersion(defId, actionFileId); + + assertThrows(SQLException.class, () -> { + try (final var statement = connection.createStatement()) { + statement.executeUpdate( + //language=sql + """ + DELETE FROM merlin.uploaded_file WHERE id = %d; + """.formatted(actionFileId)); + } + }); + } + } +} diff --git a/deployment/hasura/metadata/databases/tables/actions/action_definition.yaml b/deployment/hasura/metadata/databases/tables/actions/action_definition.yaml index 340157fd5d..3e43a8a839 100644 --- a/deployment/hasura/metadata/databases/tables/actions/action_definition.yaml +++ b/deployment/hasura/metadata/databases/tables/actions/action_definition.yaml @@ -7,9 +7,14 @@ object_relationships: - name: workspace using: foreign_key_constraint_on: workspace_id - - name: action_file +array_relationships: + - name: versions using: - foreign_key_constraint_on: action_file_id + foreign_key_constraint_on: + column: action_definition_id + table: + name: action_definition_version + schema: actions select_permissions: - role: aerie_admin permission: @@ -29,7 +34,7 @@ select_permissions: insert_permissions: - role: aerie_admin permission: - columns: [name, description, action_file_id, workspace_id] + columns: [name, description, workspace_id] check: {} set: owner: "x-hasura-user-id" @@ -37,7 +42,7 @@ insert_permissions: update_permissions: - role: aerie_admin permission: - columns: [name, description, action_file_id, parameter_schema, settings_schema, settings, owner, workspace_id] + columns: [name, description, archived, settings, owner, workspace_id] filter: {} set: updated_by: "x-hasura-user-id" diff --git a/deployment/hasura/metadata/databases/tables/actions/action_definition_version.yaml b/deployment/hasura/metadata/databases/tables/actions/action_definition_version.yaml new file mode 100644 index 0000000000..db30f04b21 --- /dev/null +++ b/deployment/hasura/metadata/databases/tables/actions/action_definition_version.yaml @@ -0,0 +1,40 @@ +table: + name: action_definition_version + schema: actions +configuration: + custom_name: "action_definition_version" +object_relationships: + - name: action_definition + using: + foreign_key_constraint_on: action_definition_id + - name: action_file + using: + foreign_key_constraint_on: action_file_id +select_permissions: + - role: aerie_admin + permission: + columns: "*" + filter: {} + allow_aggregations: true + - role: user + permission: + columns: "*" + filter: {} + allow_aggregations: true + - role: viewer + permission: + columns: "*" + filter: {} + allow_aggregations: true +insert_permissions: + - role: aerie_admin + permission: + columns: [action_definition_id, action_file_id, archived, parameter_schema, settings_schema] + check: {} + set: + author: "x-hasura-user-id" +update_permissions: + - role: aerie_admin + permission: + columns: [archived] + filter: {} diff --git a/deployment/hasura/metadata/databases/tables/actions/action_run.yaml b/deployment/hasura/metadata/databases/tables/actions/action_run.yaml index f05319a520..bff80fb4fe 100644 --- a/deployment/hasura/metadata/databases/tables/actions/action_run.yaml +++ b/deployment/hasura/metadata/databases/tables/actions/action_run.yaml @@ -26,13 +26,13 @@ select_permissions: insert_permissions: - role: aerie_admin permission: - columns: [settings, parameters, action_definition_id, has_secrets] + columns: [settings, parameters, action_definition_id, action_definition_revision, has_secrets] check: {} set: requested_by: "x-hasura-user-id" - role: user permission: - columns: [settings, parameters, action_definition_id, has_secrets] + columns: [settings, parameters, action_definition_id, action_definition_revision, has_secrets] check: { "action_definition": { "workspace": { "_or": [ { "owner": { "_eq": "X-Hasura-User-Id" } },{ "collaborators": { "collaborator": { "_eq": "X-Hasura-User-Id" } } } ] } } } set: requested_by: "x-hasura-user-id" diff --git a/deployment/hasura/metadata/databases/tables/tables.yaml b/deployment/hasura/metadata/databases/tables/tables.yaml index 0868bf1ef5..541e9c2875 100644 --- a/deployment/hasura/metadata/databases/tables/tables.yaml +++ b/deployment/hasura/metadata/databases/tables/tables.yaml @@ -195,4 +195,5 @@ #### Actions #### ################# - "!include actions/action_definition.yaml" +- "!include actions/action_definition_version.yaml" - "!include actions/action_run.yaml" diff --git a/deployment/hasura/migrations/Aerie/31_action_versioning/down.sql b/deployment/hasura/migrations/Aerie/31_action_versioning/down.sql new file mode 100644 index 0000000000..904aba56f6 --- /dev/null +++ b/deployment/hasura/migrations/Aerie/31_action_versioning/down.sql @@ -0,0 +1,112 @@ +-- Drop default revision trigger on action_run +drop trigger action_run_set_default_revision on actions.action_run; +drop function actions.action_run_set_default_revision(); + +-- Restore columns on action_definition +alter table actions.action_definition + add column action_file_id integer, + add column parameter_schema jsonb not null default '{}'::jsonb, + add column settings_schema jsonb not null default '{}'::jsonb; + +-- Populate from latest version +update actions.action_definition ad +set action_file_id = v.action_file_id, + parameter_schema = v.parameter_schema, + settings_schema = v.settings_schema +from ( + select distinct on (def.action_definition_id) + def.action_definition_id, + def.action_file_id, + def.parameter_schema, + def.settings_schema + from actions.action_definition_version def + order by def.action_definition_id, def.revision desc +) v +where ad.id = v.action_definition_id; + +comment on table actions.action_definition is e'' + 'User provided Javascript code that will be invoked by Aerie actions and ran on an Aerie server.'; + +-- Make action_file_id not null +alter table actions.action_definition + alter column action_file_id set not null; + +-- Add FK back +alter table actions.action_definition + add constraint action_definition_references_action_file + foreign key (action_file_id) + references merlin.uploaded_file + on update cascade + on delete restrict; + +-- Drop revision from action_run +alter table actions.action_run drop column action_definition_revision; + +-- Drop archived +alter table actions.action_definition drop column archived; + +-- Restore original notification triggers +drop trigger notify_action_definition_version_inserted on actions.action_definition_version; +drop function actions.notify_action_definition_version_inserted(); + +create function actions.notify_action_definition_inserted() + returns trigger + security definer + language plpgsql as $$ +begin + perform ( + with payload(action_definition_id, action_file_path) as + ( + select NEW.id, + encode(uf.path, 'escape') as path + from merlin.uploaded_file uf + where uf.id = NEW.action_file_id + ) + select pg_notify('action_definition_inserted', json_strip_nulls(row_to_json(payload))::text) + from payload + ); + return null; +end$$; + +create trigger notify_action_definition_inserted + after insert on actions.action_definition + for each row +execute function actions.notify_action_definition_inserted(); + +-- Restore original run trigger +create or replace function actions.notify_action_run_inserted() + returns trigger + security definer + language plpgsql as $$ +begin + perform ( + with payload(action_run_id, + settings, + parameters, + action_definition_id, + has_secrets, + workspace_id, + action_file_path) as + ( + select NEW.id, + NEW.settings, + NEW.parameters, + NEW.action_definition_id, + NEW.has_secrets, + ad.workspace_id, + encode(uf.path, 'escape') as path + from actions.action_definition ad + left join merlin.uploaded_file uf on uf.id = ad.action_file_id + where ad.id = NEW.action_definition_id + ) + select pg_notify('action_run_inserted', json_strip_nulls(row_to_json(payload))::text) + from payload + ); + return null; +end$$; + +-- Drop version table and its trigger function +drop table actions.action_definition_version; +drop function actions.action_definition_version_set_revision(); + +call migrations.mark_migration_rolled_back(31); diff --git a/deployment/hasura/migrations/Aerie/31_action_versioning/up.sql b/deployment/hasura/migrations/Aerie/31_action_versioning/up.sql new file mode 100644 index 0000000000..c5b42bb7d1 --- /dev/null +++ b/deployment/hasura/migrations/Aerie/31_action_versioning/up.sql @@ -0,0 +1,174 @@ +-- 1. Create action_definition_version table +create table actions.action_definition_version ( + action_definition_id integer not null, + revision integer not null default 0, + + action_file_id integer not null, + parameter_schema jsonb not null default '{}'::jsonb, + settings_schema jsonb not null default '{}'::jsonb, + archived boolean not null default false, + author text, + created_at timestamptz not null default now(), + + constraint action_definition_version_pkey + primary key (action_definition_id, revision), + constraint action_definition_version_definition_exists + foreign key (action_definition_id) + references actions.action_definition (id) + on update cascade + on delete cascade, + constraint action_definition_version_author_exists + foreign key (author) + references permissions.users + on update cascade + on delete set null, + constraint action_definition_version_references_action_file + foreign key (action_file_id) + references merlin.uploaded_file + on update cascade + on delete restrict +); + +-- 2. Auto-increment revision trigger (same pattern as constraint_definition) +create function actions.action_definition_version_set_revision() +returns trigger +volatile +language plpgsql as $$ +declare + max_revision integer; +begin + select coalesce((select revision + from actions.action_definition_version + where action_definition_id = new.action_definition_id + order by revision desc + limit 1), -1) + into max_revision; + + new.revision = max_revision + 1; + return new; +end +$$; + +create trigger action_definition_version_set_revision + before insert on actions.action_definition_version + for each row + execute function actions.action_definition_version_set_revision(); + +-- 3. Migrate existing data: create version 0 for each existing action_definition +insert into actions.action_definition_version (action_definition_id, action_file_id, parameter_schema, settings_schema, author) +select id, action_file_id, parameter_schema, settings_schema, owner +from actions.action_definition; + +-- 4. Add archived column to action_definition & update description +alter table actions.action_definition + add column archived boolean not null default false; + +comment on table actions.action_definition is e'' + 'Unversioned user-provided information about a SeqDev action.'; + +-- 5. Add action_definition_revision to action_run backfill existing runs to 0 +-- no default - future inserts w/o explicit revision are auto-set before insert by action_run_set_default_revision +alter table actions.action_run + add column action_definition_revision integer; + +update actions.action_run set action_definition_revision = 0; + +alter table actions.action_run + alter column action_definition_revision set not null; + +-- 6. Move notify_action_definition_inserted trigger to version table +drop trigger notify_action_definition_inserted on actions.action_definition; +drop function actions.notify_action_definition_inserted(); + +create function actions.notify_action_definition_version_inserted() + returns trigger + security definer + language plpgsql as $$ +begin + perform ( + with payload(action_definition_id, revision, action_file_path) as + ( + select NEW.action_definition_id, + NEW.revision, + encode(uf.path, 'escape') as path + from merlin.uploaded_file uf + where uf.id = NEW.action_file_id + ) + select pg_notify('action_definition_version_inserted', json_strip_nulls(row_to_json(payload))::text) + from payload + ); + return null; +end$$; + +create trigger notify_action_definition_version_inserted + after insert on actions.action_definition_version + for each row +execute function actions.notify_action_definition_version_inserted(); + +-- 7. Update notify_action_run_inserted to resolve file from version table +create or replace function actions.notify_action_run_inserted() + returns trigger + security definer + language plpgsql as $$ +begin + perform ( + with payload(action_run_id, + settings, + parameters, + action_definition_id, + action_definition_revision, + has_secrets, + workspace_id, + action_file_path) as + ( + select NEW.id, + NEW.settings, + NEW.parameters, + NEW.action_definition_id, + NEW.action_definition_revision, + NEW.has_secrets, + ad.workspace_id, + encode(uf.path, 'escape') as path + from actions.action_definition ad + left join actions.action_definition_version adv + on adv.action_definition_id = ad.id + and adv.revision = NEW.action_definition_revision + left join merlin.uploaded_file uf on uf.id = adv.action_file_id + where ad.id = NEW.action_definition_id + ) + select pg_notify('action_run_inserted', json_strip_nulls(row_to_json(payload))::text) + from payload + ); + return null; +end$$; + +-- 8. Auto-populate action_definition_revision on run insert (defaults to latest) +create function actions.action_run_set_default_revision() +returns trigger +volatile +language plpgsql as $$ +begin + if new.action_definition_revision is null then + select coalesce( + (select revision from actions.action_definition_version + where action_definition_id = new.action_definition_id + order by revision desc limit 1), + 0 + ) into new.action_definition_revision; + end if; + return new; +end +$$; + +create trigger action_run_set_default_revision + before insert on actions.action_run + for each row + execute function actions.action_run_set_default_revision(); + +-- 9. Drop old columns from action_definition +alter table actions.action_definition + drop column action_file_id, + drop column parameter_schema, + drop column settings_schema; + +call migrations.mark_migration_applied(31); diff --git a/deployment/postgres-init-db/sql/applied_migrations.sql b/deployment/postgres-init-db/sql/applied_migrations.sql index 497aecc507..d0a902bef2 100644 --- a/deployment/postgres-init-db/sql/applied_migrations.sql +++ b/deployment/postgres-init-db/sql/applied_migrations.sql @@ -32,3 +32,4 @@ call migrations.mark_migration_applied(27); call migrations.mark_migration_applied(28); call migrations.mark_migration_applied(29); call migrations.mark_migration_applied(30); +call migrations.mark_migration_applied(31); diff --git a/deployment/postgres-init-db/sql/init.sql b/deployment/postgres-init-db/sql/init.sql index f21ff8bfcc..d684786866 100644 --- a/deployment/postgres-init-db/sql/init.sql +++ b/deployment/postgres-init-db/sql/init.sql @@ -4,6 +4,8 @@ - Tables must be loaded before being referenced by foreign keys. - Functions must be loaded before they're used in triggers, but can be loaded after any functions that call them. - Views must be loaded after all their dependent tables and functions + - Data Preloading should be done after all schemas are created + - Setting up Database Users should be done LAST */ begin; -- Create Non-Public Schemas @@ -40,15 +42,15 @@ begin; -- Tags \ir init_tags.sql + -- Actions + \ir init_actions.sql + -- Hasura \ir init_hasura.sql - -- Preload Data + -- Preload Data (MUST BE DONE SECOND TO LAST) \ir default_user_roles.sql; - -- Initialize DB User permissions + -- Initialize DB User permissions (MUST BE DONE LAST) \ir init_db_users.sql - - -- Actions - \ir init_actions.sql end; diff --git a/deployment/postgres-init-db/sql/init_actions.sql b/deployment/postgres-init-db/sql/init_actions.sql index 33bafaaa2f..290c3b07e2 100644 --- a/deployment/postgres-init-db/sql/init_actions.sql +++ b/deployment/postgres-init-db/sql/init_actions.sql @@ -8,5 +8,6 @@ begin; -- Tables \ir tables/actions/action_definition.sql + \ir tables/actions/action_definition_version.sql \ir tables/actions/action_run.sql end; diff --git a/deployment/postgres-init-db/sql/tables/actions/action_definition.sql b/deployment/postgres-init-db/sql/tables/actions/action_definition.sql index ccfa7c15cf..ac1fa648bf 100644 --- a/deployment/postgres-init-db/sql/tables/actions/action_definition.sql +++ b/deployment/postgres-init-db/sql/tables/actions/action_definition.sql @@ -3,11 +3,9 @@ create table actions.action_definition ( name text not null, description text null, - parameter_schema jsonb not null default '{}'::jsonb, - settings_schema jsonb not null default '{}'::jsonb, settings jsonb not null default '{}'::jsonb, + archived boolean not null default false, - action_file_id integer not null, workspace_id integer not null, created_at timestamptz not null default now(), @@ -25,11 +23,6 @@ create table actions.action_definition ( references permissions.users on update cascade on delete set null, - constraint action_definition_references_action_file - foreign key (action_file_id) - references merlin.uploaded_file - on update cascade - on delete restrict, foreign key (updated_by) references permissions.users on update cascade @@ -37,21 +30,17 @@ create table actions.action_definition ( ); comment on table actions.action_definition is e'' - 'User provided Javascript code that will be invoked by Aerie actions and ran on an Aerie server.'; + 'Unversioned user-provided information about a SeqDev action.'; comment on column actions.action_definition.id is e'' 'The ID of the action.'; comment on column actions.action_definition.name is e'' 'The name of the action.'; comment on column actions.action_definition.description is e'' 'The description of the action.'; -comment on column actions.action_definition.parameter_schema is e'' - 'The JSON schema representing the action''s parameters.'; -comment on column actions.action_definition.settings_schema is e'' - 'The JSON schema representing the action''s settings.'; comment on column actions.action_definition.settings is e'' 'The values provided for the action''s settings.'; -comment on column actions.action_definition.action_file_id is e'' - 'The ID of the uploaded action file.'; +comment on column actions.action_definition.archived is e'' + 'Whether this action definition is archived (soft-deleted).'; comment on column actions.action_definition.workspace_id is e'' 'The ID of the workspace the action is part of.'; comment on column actions.action_definition.created_at is e'' @@ -67,28 +56,3 @@ create trigger set_timestamp before update on actions.action_definition for each row execute function util_functions.set_updated_at(); - -create function actions.notify_action_definition_inserted() - returns trigger - security definer - language plpgsql as $$ -begin - perform ( - with payload(action_definition_id, - action_file_path) as - ( - select NEW.id, - encode(uf.path, 'escape') as path - from merlin.uploaded_file uf - where uf.id = NEW.action_file_id - ) - select pg_notify('action_definition_inserted', json_strip_nulls(row_to_json(payload))::text) - from payload - ); - return null; -end$$; - -create trigger notify_action_definition_inserted - after insert on actions.action_definition - for each row -execute function actions.notify_action_definition_inserted(); diff --git a/deployment/postgres-init-db/sql/tables/actions/action_definition_version.sql b/deployment/postgres-init-db/sql/tables/actions/action_definition_version.sql new file mode 100644 index 0000000000..4bba383737 --- /dev/null +++ b/deployment/postgres-init-db/sql/tables/actions/action_definition_version.sql @@ -0,0 +1,99 @@ +create table actions.action_definition_version ( + action_definition_id integer not null, + revision integer not null default 0, + + action_file_id integer not null, + parameter_schema jsonb not null default '{}'::jsonb, + settings_schema jsonb not null default '{}'::jsonb, + author text, + archived boolean not null default false, + created_at timestamptz not null default now(), + + constraint action_definition_version_pkey + primary key (action_definition_id, revision), + constraint action_definition_version_definition_exists + foreign key (action_definition_id) + references actions.action_definition (id) + on update cascade + on delete cascade, + constraint action_definition_version_author_exists + foreign key (author) + references permissions.users + on update cascade + on delete set null, + constraint action_definition_version_references_action_file + foreign key (action_file_id) + references merlin.uploaded_file + on update cascade + on delete restrict +); + +comment on table actions.action_definition_version is e'' + 'An immutable revision of an action definition''s code and schemas.'; +comment on column actions.action_definition_version.action_definition_id is e'' + 'The ID of the parent action definition.'; +comment on column actions.action_definition_version.revision is e'' + 'The auto-incremented revision number within this action definition.'; +comment on column actions.action_definition_version.action_file_id is e'' + 'The ID of the uploaded action file for this version.'; +comment on column actions.action_definition_version.parameter_schema is e'' + 'The JSON schema representing the action''s parameters for this version.'; +comment on column actions.action_definition_version.settings_schema is e'' + 'The JSON schema representing the action''s settings for this version.'; +comment on column actions.action_definition_version.author is e'' + 'The user who created this version.'; +comment on column actions.action_definition_version.archived is e'' + 'Whether this version is archived (hidden from default version lists).'; +comment on column actions.action_definition_version.created_at is e'' + 'When this version was created.'; + +-- Auto-increment revision per action_definition_id +create function actions.action_definition_version_set_revision() +returns trigger +volatile +language plpgsql as $$ +declare + max_revision integer; +begin + select coalesce((select revision + from actions.action_definition_version + where action_definition_id = new.action_definition_id + order by revision desc + limit 1), -1) + into max_revision; + + new.revision = max_revision + 1; + return new; +end +$$; + +create trigger action_definition_version_set_revision + before insert on actions.action_definition_version + for each row + execute function actions.action_definition_version_set_revision(); + +-- Notify action server when a new version is uploaded +create function actions.notify_action_definition_version_inserted() + returns trigger + security definer + language plpgsql as $$ +begin + perform ( + with payload(action_definition_id, revision, action_file_path) as + ( + select NEW.action_definition_id, + NEW.revision, + encode(uf.path, 'escape') as path + from merlin.uploaded_file uf + where uf.id = NEW.action_file_id + ) + select pg_notify('action_definition_version_inserted', json_strip_nulls(row_to_json(payload))::text) + from payload + ); + return null; +end$$; + +create trigger notify_action_definition_version_inserted + after insert on actions.action_definition_version + for each row +execute function actions.notify_action_definition_version_inserted(); diff --git a/deployment/postgres-init-db/sql/tables/actions/action_run.sql b/deployment/postgres-init-db/sql/tables/actions/action_run.sql index fafc466e21..28b5ca833d 100644 --- a/deployment/postgres-init-db/sql/tables/actions/action_run.sql +++ b/deployment/postgres-init-db/sql/tables/actions/action_run.sql @@ -10,6 +10,7 @@ create table actions.action_run ( has_secrets boolean not null default false, action_definition_id integer not null, + action_definition_revision integer not null, requested_by text, requested_at timestamptz not null default now(), @@ -45,6 +46,8 @@ comment on column actions.action_run.status is e'' 'The status of the action run.'; comment on column actions.action_run.action_definition_id is e'' 'The ID of the definition of the action.'; +comment on column actions.action_run.action_definition_revision is e'' + 'The revision of the action definition version used for this run.'; comment on column actions.action_run.requested_by is e'' 'The username of the requester of the action run.'; comment on column actions.action_run.requested_at is e'' @@ -56,6 +59,7 @@ comment on column actions.action_run.canceled is e'' comment on column actions.action_run.has_secrets is e'' 'A flag that is set to true if the run has secrets, otherwise false.'; +-- Notify action server when a new run is inserted create function actions.notify_action_run_inserted() returns trigger security definer @@ -66,6 +70,7 @@ begin settings, parameters, action_definition_id, + action_definition_revision, has_secrets, workspace_id, action_file_path) as @@ -74,11 +79,15 @@ begin NEW.settings, NEW.parameters, NEW.action_definition_id, + NEW.action_definition_revision, NEW.has_secrets, ad.workspace_id, encode(uf.path, 'escape') as path from actions.action_definition ad - left join merlin.uploaded_file uf on uf.id = ad.action_file_id + left join actions.action_definition_version adv + on adv.action_definition_id = ad.id + and adv.revision = NEW.action_definition_revision + left join merlin.uploaded_file uf on uf.id = adv.action_file_id where ad.id = NEW.action_definition_id ) select pg_notify('action_run_inserted', json_strip_nulls(row_to_json(payload))::text) @@ -112,3 +121,26 @@ create trigger notify_action_run_cancel_requested and OLD.canceled is distinct from NEW.canceled ) execute function actions.notify_action_run_cancel_requested(); + +-- Auto-populate action_definition_revision with latest if not provided +create function actions.action_run_set_default_revision() +returns trigger +volatile +language plpgsql as $$ +begin + if new.action_definition_revision is null then + select coalesce( + (select revision from actions.action_definition_version + where action_definition_id = new.action_definition_id + order by revision desc limit 1), + 0 + ) into new.action_definition_revision; + end if; + return new; +end +$$; + +create trigger action_run_set_default_revision + before insert on actions.action_run + for each row + execute function actions.action_run_set_default_revision();