diff --git a/CHANGELOG.md b/CHANGELOG.md index b77a01a..b04f61c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Allow to set the `last_transaction_id` when outbox is created - Accept Ecto `dynamic/2` expressions for the `:filter` option on `Carbonite.process/4`. +- New `Carbonite.delete_transaction_if_empty/2` deletes the current transaction if no changes have been recorded. ### Changed diff --git a/lib/carbonite.ex b/lib/carbonite.ex index e212060..454dc3d 100644 --- a/lib/carbonite.ex +++ b/lib/carbonite.ex @@ -83,6 +83,45 @@ defmodule Carbonite do ) end + @doc """ + Deletes the current `t:Carbonite.Transaction.t/0` if no changes have been recorded. + + This is sometimes useful to avoid "orphaned" transactions (without change records) + for operations that usually modify any tracked tables but in rare cases do nothing. + + As the INSERT and DELETE on the `transactions` table has a performance cost, it is + usually preferable to skip the transaction entirely, if possible. + + ## Example + + MyApp.Repo.transaction(fn -> + Carbonite.insert_transaction(MyApp.Repo) + do_something_that_may_or_may_not_cause_a_change_to_be_recorded() + Carbonite.delete_transaction_if_empty(MyApp.Repo) + end) + + ## Parameters + + * `repo` - the Ecto repository + * `opts` - optional keyword list + + ## Options + + * `carbonite_prefix` - defines the audit trail's schema, defaults to `"carbonite_default"` + """ + @doc since: "0.16.0" + @spec delete_transaction_if_empty(repo()) :: {:ok, non_neg_integer()} + @spec delete_transaction_if_empty(repo(), [prefix_option()]) :: {:ok, non_neg_integer()} + def delete_transaction_if_empty(repo, opts \\ []) do + {rows_deleted, _} = + opts + |> Query.current_transaction() + |> Query.without_changes() + |> repo.delete_all() + + {:ok, rows_deleted} + end + @doc """ Fetches all changes of the current transaction from the database. diff --git a/lib/carbonite/multi.ex b/lib/carbonite/multi.ex index 0a828c0..2c67aa9 100644 --- a/lib/carbonite/multi.ex +++ b/lib/carbonite/multi.ex @@ -34,18 +34,36 @@ defmodule Carbonite.Multi do @spec insert_transaction(Multi.t(), params()) :: Multi.t() @spec insert_transaction(Multi.t(), params(), [prefix_option()]) :: Multi.t() def insert_transaction(%Multi{} = multi, params \\ %{}, opts \\ []) do - name = - if carbonite_prefix = Keyword.get(opts, :carbonite_prefix) do - {:carbonite_transaction, carbonite_prefix} - else - :carbonite_transaction - end - - Multi.run(multi, name, fn repo, _state -> + Multi.run(multi, maybe_with_prefix(:carbonite_transaction, opts), fn repo, _state -> Carbonite.insert_transaction(repo, params, opts) end) end + @doc """ + Adds an operation `Ecto.Multi` that calls `Carbonite.delete_transaction_if_empty/2`. + + Multi step is called `:delete_carbonite_transaction` if no `:carbonite_prefix` option + is given, otherwise `{:delete_carbonite_transaction, }`. + + See `Carbonite.delete_transaction_if_empty/2` for options. + """ + @doc since: "0.16.0" + @spec delete_transaction_if_empty(Multi.t()) :: Multi.t() + @spec delete_transaction_if_empty(Multi.t(), [prefix_option()]) :: Multi.t() + def delete_transaction_if_empty(%Multi{} = multi, opts \\ []) do + Multi.run(multi, maybe_with_prefix(:delete_carbonite_transaction, opts), fn repo, _state -> + Carbonite.delete_transaction_if_empty(repo, opts) + end) + end + + defp maybe_with_prefix(name, opts) do + if carbonite_prefix = Keyword.get(opts, :carbonite_prefix) do + {name, carbonite_prefix} + else + name + end + end + @doc """ Adds a operation to an `Ecto.Multi` to fetch the changes of the current transaction. diff --git a/lib/carbonite/query.ex b/lib/carbonite/query.ex index c8e169a..c44a6e3 100644 --- a/lib/carbonite/query.ex +++ b/lib/carbonite/query.ex @@ -102,6 +102,24 @@ defmodule Carbonite.Query do |> maybe_preload(opts, :changes, from_with_prefix(Change, opts)) end + @doc """ + Refines a transaction query to only contain transactions without associated changes. + """ + @doc since: "0.16.0" + @spec without_changes(Ecto.Query.t()) :: Ecto.Query.t() + def without_changes( + %Ecto.Query{from: %{source: {"transactions", Transaction}, prefix: prefix}} = query + ) do + changes = + from(c in Change, + prefix: ^prefix, + where: parent_as(:transaction).id == c.transaction_id, + select: 1 + ) + + from(t in query, as: :transaction, where: not exists(subquery(changes))) + end + # Returns all triggers. @doc false @spec triggers() :: Ecto.Query.t() diff --git a/test/carbonite/multi_test.exs b/test/carbonite/multi_test.exs index 4c66516..e6b8c46 100644 --- a/test/carbonite/multi_test.exs +++ b/test/carbonite/multi_test.exs @@ -3,7 +3,7 @@ defmodule Carbonite.MultiTest do use Carbonite.APICase, async: true import Carbonite.Multi - alias Carbonite.{Rabbit, TestRepo} + alias Carbonite.{Query, Rabbit, TestRepo} describe "insert_transaction/3" do test "inserts a transaction within an Ecto.Multi" do @@ -24,6 +24,26 @@ defmodule Carbonite.MultiTest do end end + describe "delete_transaction_if_empty/2" do + test "deletes current transaction if empty within an Ecto.Multi" do + assert {:ok, _} = + Ecto.Multi.new() + |> insert_transaction() + |> delete_transaction_if_empty() + |> TestRepo.transaction() + + refute TestRepo.exists?(Query.current_transaction()) + end + + test "operation names include the given prefix option" do + assert %Ecto.Multi{operations: [{:delete_carbonite_transaction, _}]} = + delete_transaction_if_empty(Ecto.Multi.new()) + + assert %Ecto.Multi{operations: [{{:delete_carbonite_transaction, "custom"}, _}]} = + delete_transaction_if_empty(Ecto.Multi.new(), carbonite_prefix: "custom") + end + end + describe "override_mode/2" do test "enables override mode for the current transaction" do assert {:ok, _} = diff --git a/test/carbonite/query_test.exs b/test/carbonite/query_test.exs index 2449c69..14c0b64 100644 --- a/test/carbonite/query_test.exs +++ b/test/carbonite/query_test.exs @@ -77,6 +77,16 @@ defmodule Carbonite.QueryTest do end end + describe "without_changes/1" do + setup [:insert_past_transactions, :insert_rabbits] + + test "filters a transaction query for orphans" do + # Rabbit transaction has changes, past transactions don't. + assert length(TestRepo.all(Query.transactions())) == 4 + assert length(TestRepo.all(Query.without_changes(Query.transactions()))) == 3 + end + end + describe "outbox/1" do defp outbox(name) do name diff --git a/test/carbonite_test.exs b/test/carbonite_test.exs index a62446b..57346b7 100644 --- a/test/carbonite_test.exs +++ b/test/carbonite_test.exs @@ -3,7 +3,7 @@ defmodule CarboniteTest do use Carbonite.APICase, async: true import Carbonite - alias Carbonite.{Outbox, Rabbit, TestRepo, Transaction} + alias Carbonite.{Outbox, Query, Rabbit, TestRepo, Transaction} alias Ecto.Adapters.SQL defp insert_jack do @@ -58,6 +58,25 @@ defmodule CarboniteTest do end end + describe "delete_transaction_if_empty/2" do + test "deletes the current transaction if no changes were recorded" do + insert_transaction(TestRepo) + + assert delete_transaction_if_empty(TestRepo) == {:ok, 1} + + refute TestRepo.exists?(Query.current_transaction()) + end + + test "keeps the current transaction otherwise" do + insert_transaction(TestRepo) + insert_jack() + + assert delete_transaction_if_empty(TestRepo) == {:ok, 0} + + assert TestRepo.exists?(Query.current_transaction()) + end + end + describe "fetch_changes/2" do test "inserts a transaction" do TestRepo.transaction(fn ->