From efd8da349cc488281fe2ef3b1e9c5fb83bb77a75 Mon Sep 17 00:00:00 2001 From: Rory McKinley Date: Mon, 27 Nov 2023 13:45:26 +0200 Subject: [PATCH 1/8] WIP - remove FKs --- ...27112907_remove_run_fk_on_attempt_runs.exs | 19 +++++++++++++++++++ ...31127112928_remove_run_fk_on_log_lines.exs | 19 +++++++++++++++++++ ...43_remove_run_fk_on_log_lines_monolith.exs | 19 +++++++++++++++++++ .../20231127114133_remove_run_fk_on_runs.exs | 19 +++++++++++++++++++ 4 files changed, 76 insertions(+) create mode 100644 priv/repo/migrations/20231127112907_remove_run_fk_on_attempt_runs.exs create mode 100644 priv/repo/migrations/20231127112928_remove_run_fk_on_log_lines.exs create mode 100644 priv/repo/migrations/20231127112943_remove_run_fk_on_log_lines_monolith.exs create mode 100644 priv/repo/migrations/20231127114133_remove_run_fk_on_runs.exs diff --git a/priv/repo/migrations/20231127112907_remove_run_fk_on_attempt_runs.exs b/priv/repo/migrations/20231127112907_remove_run_fk_on_attempt_runs.exs new file mode 100644 index 0000000000..a241a8ecac --- /dev/null +++ b/priv/repo/migrations/20231127112907_remove_run_fk_on_attempt_runs.exs @@ -0,0 +1,19 @@ +defmodule Lightning.Repo.Migrations.RemoveRunFkOnAttemptRuns do + use Ecto.Migration + + def up do + execute(""" + ALTER TABLE attempt_runs + DROP CONSTRAINT attempt_runs_run_id_fkey + """) + end + + def down do + execute(""" + ALTER TABLE attempt_runs + ADD CONSTRAINT attempt_runs_run_id_fkey + FOREIGN KEY (run_id) + REFERENCES runs(id) ON DELETE CASCADE + """) + end +end diff --git a/priv/repo/migrations/20231127112928_remove_run_fk_on_log_lines.exs b/priv/repo/migrations/20231127112928_remove_run_fk_on_log_lines.exs new file mode 100644 index 0000000000..ce0d53340c --- /dev/null +++ b/priv/repo/migrations/20231127112928_remove_run_fk_on_log_lines.exs @@ -0,0 +1,19 @@ +defmodule Lightning.Repo.Migrations.RemoveRunFkOnLogLines do + use Ecto.Migration + + def up do + execute(""" + ALTER TABLE log_lines + DROP CONSTRAINT log_lines_run_id_fkey + """) + end + + def down do + execute(""" + ALTER TABLE log_lines + ADD CONSTRAINT log_lines_run_id_fkey + FOREIGN KEY (run_id) + REFERENCES runs(id) ON DELETE CASCADE + """) + end +end diff --git a/priv/repo/migrations/20231127112943_remove_run_fk_on_log_lines_monolith.exs b/priv/repo/migrations/20231127112943_remove_run_fk_on_log_lines_monolith.exs new file mode 100644 index 0000000000..9c93ebe94d --- /dev/null +++ b/priv/repo/migrations/20231127112943_remove_run_fk_on_log_lines_monolith.exs @@ -0,0 +1,19 @@ +defmodule Lightning.Repo.Migrations.RemoveRunFkOnLogLinesMonolith do + use Ecto.Migration + + def up do + execute(""" + ALTER TABLE log_lines_monolith + DROP CONSTRAINT log_lines_monolith_run_id_fkey + """) + end + + def down do + execute(""" + ALTER TABLE log_lines_monolith + ADD CONSTRAINT "log_lines_monolith_run_id_fkey" + FOREIGN KEY (run_id) + REFERENCES runs(id) ON DELETE CASCADE + """) + end +end diff --git a/priv/repo/migrations/20231127114133_remove_run_fk_on_runs.exs b/priv/repo/migrations/20231127114133_remove_run_fk_on_runs.exs new file mode 100644 index 0000000000..2a75c83973 --- /dev/null +++ b/priv/repo/migrations/20231127114133_remove_run_fk_on_runs.exs @@ -0,0 +1,19 @@ +defmodule Lightning.Repo.Migrations.RemoveRunFkOnRuns do + use Ecto.Migration + + def up do + execute(""" + ALTER TABLE runs + DROP CONSTRAINT runs_previous_id_fkey + """) + end + + def down do + execute(""" + ALTER TABLE runs + ADD CONSTRAINT runs_previous_id_fkey + FOREIGN KEY (previous_id) + REFERENCES runs(id) ON DELETE CASCADE + """) + end +end From 3966f6e495f8836701c2f969529fc4cd6e1bfb6c Mon Sep 17 00:00:00 2001 From: Rory McKinley Date: Mon, 27 Nov 2023 14:04:12 +0200 Subject: [PATCH 2/8] Add shim migration --- ...7115754_shim_migration_not_for_merging.exs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 priv/repo/migrations/20231127115754_shim_migration_not_for_merging.exs diff --git a/priv/repo/migrations/20231127115754_shim_migration_not_for_merging.exs b/priv/repo/migrations/20231127115754_shim_migration_not_for_merging.exs new file mode 100644 index 0000000000..e361278e36 --- /dev/null +++ b/priv/repo/migrations/20231127115754_shim_migration_not_for_merging.exs @@ -0,0 +1,33 @@ +# NBNB For WIP branch only - this is just a shim branch until +# the branch that removes attempts FKs is merged in. +defmodule Lightning.Repo.Migrations.ShimMigrationNotForMerging do + use Ecto.Migration + + def up do + execute(""" + ALTER TABLE attempt_runs + DROP CONSTRAINT attempt_runs_attempt_id_fkey + """) + execute(""" + ALTER TABLE log_lines + DROP CONSTRAINT log_lines_attempt_id_fkey + """) + end + + def change do + execute(""" + ALTER TABLE attempt_runs + ADD CONSTRAINT attempt_runs_attempt_id_fkey + FOREIGN KEY (attempt_id) + REFERENCES attempts(id) ON DELETE CASCADE + """) + + execute(""" + ALTER TABLE log_lines + ADD CONSTRAINT log_lines_attempt_id_fkey + FOREIGN KEY (attempt_id) + REFERENCES attempts(id) + ON DELETE CASCADE + """) + end +end From a1dac685178a048b826b86623526d4b77376b609 Mon Sep 17 00:00:00 2001 From: Rory McKinley Date: Mon, 27 Nov 2023 14:05:09 +0200 Subject: [PATCH 3/8] Add explicit deletion of LogLine records --- lib/lightning/setup_utils.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/lightning/setup_utils.ex b/lib/lightning/setup_utils.ex index f85448ef6b..d84ac3f81f 100644 --- a/lib/lightning/setup_utils.ex +++ b/lib/lightning/setup_utils.ex @@ -893,6 +893,7 @@ defmodule Lightning.SetupUtils do Lightning.Projects.ProjectCredential, Lightning.WorkOrder, Lightning.Invocation.Run, + Lightning.Invocation.LogLine, Lightning.Credentials.Credential, Lightning.Workflows.Job, Lightning.Workflows.Trigger, From 4b746ea3aae33abb08414eb5fe019bf864914e51 Mon Sep 17 00:00:00 2001 From: Rory McKinley Date: Mon, 27 Nov 2023 14:26:05 +0200 Subject: [PATCH 4/8] Add explicit deletion of LogLine records --- lib/lightning/projects.ex | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/lightning/projects.ex b/lib/lightning/projects.ex index 1d747443be..034d4a1522 100644 --- a/lib/lightning/projects.ex +++ b/lib/lightning/projects.ex @@ -19,7 +19,7 @@ defmodule Lightning.Projects do alias Lightning.Accounts.User alias Lightning.ExportUtils alias Lightning.Workflows.Workflow - alias Lightning.Invocation.{Run, Dataclip} + alias Lightning.Invocation.{Run, Dataclip, LogLine} alias Lightning.WorkOrder require Logger @@ -210,6 +210,8 @@ defmodule Lightning.Projects do end) Repo.transaction(fn -> + project_log_lines_query(project) |> Repo.delete_all() + project_attempts_query(project) |> Repo.delete_all() project_attempt_run_query(project) |> Repo.delete_all() @@ -259,6 +261,15 @@ defmodule Lightning.Projects do ) end + def project_log_lines_query(project) do + from(ll in LogLine, + join: att in assoc(ll, :attempt), + join: wo in assoc(att, :work_order), + join: w in assoc(wo, :workflow), + where: w.project_id == ^project.id + ) + end + def project_workorders_query(project) do from(wo in WorkOrder, join: w in assoc(wo, :workflow), From 70f055c2f36d6793b9a846cf80d953e5c86796df Mon Sep 17 00:00:00 2001 From: Rory McKinley Date: Mon, 27 Nov 2023 14:42:38 +0200 Subject: [PATCH 5/8] Fix delete sequence bug --- lib/lightning/projects.ex | 4 ++-- test/lightning/projects_test.exs | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/lightning/projects.ex b/lib/lightning/projects.ex index 034d4a1522..2752ed9d7e 100644 --- a/lib/lightning/projects.ex +++ b/lib/lightning/projects.ex @@ -212,10 +212,10 @@ defmodule Lightning.Projects do Repo.transaction(fn -> project_log_lines_query(project) |> Repo.delete_all() - project_attempts_query(project) |> Repo.delete_all() - project_attempt_run_query(project) |> Repo.delete_all() + project_attempts_query(project) |> Repo.delete_all() + project_workorders_query(project) |> Repo.delete_all() project_runs_query(project) |> Repo.delete_all() diff --git a/test/lightning/projects_test.exs b/test/lightning/projects_test.exs index c1c47d9acf..ccbe2a0ff9 100644 --- a/test/lightning/projects_test.exs +++ b/test/lightning/projects_test.exs @@ -280,6 +280,8 @@ defmodule Lightning.ProjectsTest do ) ) + p2_attempt_run = Repo.get_by(Lightning.AttemptRun, run_id: p2_run.id) + runs_query = Lightning.Projects.project_runs_query(p1) work_order_query = Lightning.Projects.project_workorders_query(p1) @@ -325,7 +327,7 @@ defmodule Lightning.ProjectsTest do assert attempt_query |> Repo.aggregate(:count, :id) == 0 - assert attempt_run_query |> Repo.aggregate(:count, :id) == 0 + assert only_record_for_type?(p2_attempt_run) assert pu_query |> Repo.aggregate(:count, :id) == 0 From 13a2ce78468aac291b8e19266a9aa2f73f090531 Mon Sep 17 00:00:00 2001 From: Rory McKinley Date: Mon, 27 Nov 2023 16:59:26 +0200 Subject: [PATCH 6/8] Fix bug in helper --- test/support/model_helpers.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/support/model_helpers.ex b/test/support/model_helpers.ex index f2fc9b46df..480d201476 100644 --- a/test/support/model_helpers.ex +++ b/test/support/model_helpers.ex @@ -83,7 +83,7 @@ defmodule Lightning.ModelHelpers do where: r.id == ^expected_instance.id, left_join: others in ^model, on: others.id != ^expected_instance.id, - select: [count(r.id), count(others.id)] + select: [count(r.id, :distinct), count(others.id)] ) |> Lightning.Repo.one!() |> case do From 0df982354a02b21312288642d55aed01356e0e10 Mon Sep 17 00:00:00 2001 From: Rory McKinley Date: Mon, 27 Nov 2023 16:59:59 +0200 Subject: [PATCH 7/8] Tweaking test coverage --- lib/lightning/projects.ex | 24 ++++++++++++------------ test/lightning/projects_test.exs | 12 ++++++++++-- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/lib/lightning/projects.ex b/lib/lightning/projects.ex index 2752ed9d7e..6661cc8b15 100644 --- a/lib/lightning/projects.ex +++ b/lib/lightning/projects.ex @@ -221,18 +221,18 @@ defmodule Lightning.Projects do project_runs_query(project) |> Repo.delete_all() project_jobs_query(project) |> Repo.delete_all() - - project_triggers_query(project) |> Repo.delete_all() - - project_workflows_query(project) |> Repo.delete_all() - - project_users_query(project) |> Repo.delete_all() - - project_credentials_query(project) |> Repo.delete_all() - - project_dataclips_query(project) |> Repo.delete_all() - - {:ok, project} = Repo.delete(project) + # + # project_triggers_query(project) |> Repo.delete_all() + # + # project_workflows_query(project) |> Repo.delete_all() + # + # project_users_query(project) |> Repo.delete_all() + # + # project_credentials_query(project) |> Repo.delete_all() + # + # project_dataclips_query(project) |> Repo.delete_all() + + # {:ok, project} = Repo.delete(project) Logger.debug(fn -> # coveralls-ignore-start diff --git a/test/lightning/projects_test.exs b/test/lightning/projects_test.exs index ccbe2a0ff9..e6f7d3dac5 100644 --- a/test/lightning/projects_test.exs +++ b/test/lightning/projects_test.exs @@ -267,7 +267,7 @@ defmodule Lightning.ProjectsTest do p2_log_line = build(:log_line, run: p2_run) - insert(:workorder, + p2_workorder = insert(:workorder, trigger: t2, workflow: w2, dataclip: p2_dataclip, @@ -321,7 +321,15 @@ defmodule Lightning.ProjectsTest do assert {:ok, %Project{}} = Projects.delete_project(p1) - assert runs_query |> Repo.aggregate(:count, :id) == 0 + # IO.inspect(p2_run, label: :highlander) + # + # Repo.all(Lightning.Invocation.Run) |> IO.inspect() + + assert only_record_for_type?(p2_run) + + assert only_record_for_type?(p2_workorder) + + assert only_record_for_type?(e2.target_job) assert work_order_query |> Repo.aggregate(:count, :id) == 0 From 0979a958aa4295b35c62eeb0674004f7219c30f8 Mon Sep 17 00:00:00 2001 From: Rory McKinley Date: Tue, 28 Nov 2023 11:28:32 +0200 Subject: [PATCH 8/8] Tighten down some screws --- lib/lightning/projects.ex | 26 ++++++++++++++------------ test/lightning/projects_test.exs | 22 ++++++++++++++-------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/lib/lightning/projects.ex b/lib/lightning/projects.ex index 6661cc8b15..abd06a1a95 100644 --- a/lib/lightning/projects.ex +++ b/lib/lightning/projects.ex @@ -221,18 +221,20 @@ defmodule Lightning.Projects do project_runs_query(project) |> Repo.delete_all() project_jobs_query(project) |> Repo.delete_all() - # - # project_triggers_query(project) |> Repo.delete_all() - # - # project_workflows_query(project) |> Repo.delete_all() - # - # project_users_query(project) |> Repo.delete_all() - # - # project_credentials_query(project) |> Repo.delete_all() - # - # project_dataclips_query(project) |> Repo.delete_all() - - # {:ok, project} = Repo.delete(project) + + # No explicit test for triggers? Confirm when the noise from + # attempts has been silenced + project_triggers_query(project) |> Repo.delete_all() + + project_workflows_query(project) |> Repo.delete_all() + + project_users_query(project) |> Repo.delete_all() + + project_credentials_query(project) |> Repo.delete_all() + + project_dataclips_query(project) |> Repo.delete_all() + + {:ok, project} = Repo.delete(project) Logger.debug(fn -> # coveralls-ignore-start diff --git a/test/lightning/projects_test.exs b/test/lightning/projects_test.exs index e6f7d3dac5..d379a32eee 100644 --- a/test/lightning/projects_test.exs +++ b/test/lightning/projects_test.exs @@ -261,6 +261,7 @@ defmodule Lightning.ProjectsTest do ] ) + p2_dataclip = insert(:dataclip, body: %{foo: "bar"}, project: p2) p2_run = insert(:run, input_dataclip: p2_dataclip, job: e2.target_job) @@ -280,6 +281,15 @@ defmodule Lightning.ProjectsTest do ) ) + {:ok, p2_pu} = p2.project_users |> Enum.fetch(0) + + {:ok, p2_pc} = Repo.all(Ecto.assoc(p2, :project_credentials)) |> Enum.fetch(0) + + p2_dataclip = Repo.get_by( + Lightning.Invocation.Dataclip, + project_id: p2.id + ) + p2_attempt_run = Repo.get_by(Lightning.AttemptRun, run_id: p2_run.id) runs_query = Lightning.Projects.project_runs_query(p1) @@ -321,25 +331,19 @@ defmodule Lightning.ProjectsTest do assert {:ok, %Project{}} = Projects.delete_project(p1) - # IO.inspect(p2_run, label: :highlander) - # - # Repo.all(Lightning.Invocation.Run) |> IO.inspect() - assert only_record_for_type?(p2_run) assert only_record_for_type?(p2_workorder) - assert only_record_for_type?(e2.target_job) - assert work_order_query |> Repo.aggregate(:count, :id) == 0 assert attempt_query |> Repo.aggregate(:count, :id) == 0 assert only_record_for_type?(p2_attempt_run) - assert pu_query |> Repo.aggregate(:count, :id) == 0 + assert only_record_for_type?(p2_pu) - assert pc_query |> Repo.aggregate(:count, :id) == 0 + assert only_record_for_type?(p2_pc) assert workflows_query |> Repo.aggregate(:count, :id) == 0 @@ -347,6 +351,8 @@ defmodule Lightning.ProjectsTest do assert only_record_for_type?(p2_log_line) + assert only_record_for_type?(p2_dataclip) + assert_raise Ecto.NoResultsError, fn -> Projects.get_project!(p1.id) end