From 4e6ac9fc5d259ba3523d29f5c2ae08a9b5383f1e Mon Sep 17 00:00:00 2001 From: Danielwhyte Date: Tue, 11 Dec 2018 16:25:02 +0000 Subject: [PATCH 1/6] adds check for unique indexes when insert or update is called, #21 --- README.md | 16 ++++- lib/alog.ex | 68 +++++++++++++++---- .../20181211161000_unique_index.exs | 15 ++++ test/alog_test.exs | 10 +++ test/support/unique.ex | 20 ++++++ 5 files changed, 114 insertions(+), 15 deletions(-) create mode 100644 priv/repo/test_app/migrations/20181211161000_unique_index.exs create mode 100644 test/support/unique.ex diff --git a/README.md b/README.md index 9e7cf72..5c796b9 100644 --- a/README.md +++ b/README.md @@ -32,5 +32,19 @@ This module provides some helper functions to make it easy to insert and retriev end ``` + ## Repo + Alog expects your `Repo` to belong to the same base module as the schema. - So if your schema is `MyApp.User`, or `MyApp.Accounts.User`, your Repo should be `MyApp.Repo`. \ No newline at end of file + So if your schema is `MyApp.User`, or `MyApp.Accounts.User`, your Repo should be `MyApp.Repo`. + + ## Indexes + + Due to the append only manner in which Alog stores data, it is not compatible with tables that have Unique Indexes applied to any of their columns. If you wish to use alog, you will have to remove these indexes. + + For example, the following in a migration file would remove a unique index on the `email` column from the `users` table. + + ``` + drop(unique_index(:users, :email)) + ``` + + See https://hexdocs.pm/ecto_sql/Ecto.Migration.html#content for more details. \ No newline at end of file diff --git a/lib/alog.ex b/lib/alog.ex index 11b34be..7e76fab 100644 --- a/lib/alog.ex +++ b/lib/alog.ex @@ -101,9 +101,15 @@ defmodule Alog do |> User.insert() """ def insert(struct_or_changeset) do - struct_or_changeset - |> insert_entry_id() - |> @repo.insert() + case check_for_unique_index() do + :ok -> + struct_or_changeset + |> insert_entry_id() + |> @repo.insert() + + {:error, msg} -> + raise msg + end end @doc """ @@ -161,17 +167,23 @@ defmodule Alog do |> User.update() """ def update(%Ecto.Changeset{} = changeset) do - data = - changeset - |> Map.get(:data) - |> @repo.preload(__MODULE__.__schema__(:associations)) - |> Map.put(:id, nil) - |> Map.put(:inserted_at, nil) - |> Map.put(:updated_at, nil) - - changeset - |> Map.put(:data, data) - |> @repo.insert() + case check_for_unique_index() do + :ok -> + data = + changeset + |> Map.get(:data) + |> @repo.preload(__MODULE__.__schema__(:associations)) + |> Map.put(:id, nil) + |> Map.put(:inserted_at, nil) + |> Map.put(:updated_at, nil) + + changeset + |> Map.put(:data, data) + |> @repo.insert() + + {:error, msg} -> + raise msg + end end def update(_) do @@ -326,6 +338,34 @@ defmodule Alog do end) end + defp check_for_unique_index() do + table = __MODULE__.__schema__(:source) + "Elixir." <> module_name = unquote(__MODULE__) |> to_string() + + case @repo.query( + "SELECT * FROM pg_indexes WHERE tablename = $1 and indexname NOT LIKE '%_pkey' AND indexdef LIKE 'CREATE UNIQUE INDEX%';", + [table] + ) do + {:ok, %Postgrex.Result{columns: columns, rows: rows}} when rows != [] -> + unique_index = + rows + |> List.first() + |> Enum.zip(columns) + |> Enum.find(fn {_r, c} -> c == "indexname" end) + |> elem(0) + + {:error, + """ + Unique index '#{unique_index}' found on table '#{table}'. + #{module_name} is not compatible with tables that have a unique index. + Please remove this index if you want to use #{module_name}. + """} + + _ -> + :ok + end + end + defoverridable Alog end end diff --git a/priv/repo/test_app/migrations/20181211161000_unique_index.exs b/priv/repo/test_app/migrations/20181211161000_unique_index.exs new file mode 100644 index 0000000..d43eb85 --- /dev/null +++ b/priv/repo/test_app/migrations/20181211161000_unique_index.exs @@ -0,0 +1,15 @@ +defmodule Alog.Repo.Migrations.UniqueIndex do + use Ecto.Migration + + def change do + create table(:unique) do + add(:name, :string) + add(:entry_id, :string) + add(:deleted, :boolean, default: false) + + timestamps() + end + + create(unique_index(:unique, :name)) + end +end diff --git a/test/alog_test.exs b/test/alog_test.exs index 3312e9b..1aa302b 100644 --- a/test/alog_test.exs +++ b/test/alog_test.exs @@ -61,4 +61,14 @@ defmodule AlogTest do end).() end end + + describe "Not compatible with unique index" do + test "Throws error if unique index exists" do + assert_raise RuntimeError, fn -> + %Alog.TestApp.Unique{} + |> Alog.TestApp.Unique.changeset(%{name: "unique item"}) + |> Alog.TestApp.Unique.insert() + end + end + end end diff --git a/test/support/unique.ex b/test/support/unique.ex new file mode 100644 index 0000000..197bd23 --- /dev/null +++ b/test/support/unique.ex @@ -0,0 +1,20 @@ +defmodule Alog.TestApp.Unique do + use Ecto.Schema + use Alog + import Ecto.Changeset + + schema "unique" do + field(:name, :string) + field(:entry_id, :string) + field(:deleted, :boolean, default: false) + + timestamps() + end + + @doc false + def changeset(unique, attrs) do + unique + |> cast(attrs, [:name]) + |> validate_required([:name]) + end +end From 2a0edb5b6183d01534a30546f2badb762f8ad4c8 Mon Sep 17 00:00:00 2001 From: Danielwhyte Date: Wed, 12 Dec 2018 10:21:53 +0000 Subject: [PATCH 2/6] applies unique_constraint on insert function, #21 --- lib/alog.ex | 17 +++++++++++++++++ mix.lock | 4 +++- test/constraint_test.exs | 21 +++++++++++++++++++++ test/support/user.ex | 1 + test/update_test.exs | 4 ++-- 5 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 test/constraint_test.exs diff --git a/lib/alog.ex b/lib/alog.ex index 7e76fab..f0f8287 100644 --- a/lib/alog.ex +++ b/lib/alog.ex @@ -105,6 +105,7 @@ defmodule Alog do :ok -> struct_or_changeset |> insert_entry_id() + |> apply_constraints() |> @repo.insert() {:error, msg} -> @@ -278,6 +279,22 @@ defmodule Alog do @repo.preload(item, [{assoc, preload_query(assoc)}]) end + defp apply_constraints(%Ecto.Changeset{} = changeset) do + changeset + |> Map.get(:constraints) + |> Enum.reduce(changeset, fn con, acc -> + with :unique <- con.type, + change when not is_nil(change) <- Map.get(changeset.changes, con.field), + existing when not is_nil(existing) <- __MODULE__.get_by([{con.field, change}]) do + Ecto.Changeset.add_error(acc, con.field, Map.get(con, :error) |> elem(0)) + else + _ -> acc + end + end) + end + + defp apply_constraints(struct), do: struct + defp preload_map(assoc, owner) do case assoc do {k, v} -> diff --git a/mix.lock b/mix.lock index 6dc025b..2c326fb 100644 --- a/mix.lock +++ b/mix.lock @@ -1,8 +1,10 @@ %{ "connection": {:hex, :connection, "1.0.4", "a1cae72211f0eef17705aaededacac3eb30e6625b04a6117c1b2db6ace7d5976", [:mix], [], "hexpm"}, "db_connection": {:hex, :db_connection, "1.1.3", "89b30ca1ef0a3b469b1c779579590688561d586694a3ce8792985d4d7e575a61", [:mix], [{:connection, "~> 1.0.2", [hex: :connection, repo: "hexpm", optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, repo: "hexpm", optional: true]}], "hexpm"}, - "decimal": {:hex, :decimal, "1.5.0", "b0433a36d0e2430e3d50291b1c65f53c37d56f83665b43d79963684865beab68", [:mix], [], "hexpm"}, + "decimal": {:hex, :decimal, "1.6.0", "bfd84d90ff966e1f5d4370bdd3943432d8f65f07d3bab48001aebd7030590dcc", [:mix], [], "hexpm"}, "ecto": {:hex, :ecto, "2.2.11", "4bb8f11718b72ba97a2696f65d247a379e739a0ecabf6a13ad1face79844791c", [:mix], [{:db_connection, "~> 1.1", [hex: :db_connection, repo: "hexpm", optional: true]}, {:decimal, "~> 1.2", [hex: :decimal, repo: "hexpm", optional: false]}, {:mariaex, "~> 0.8.0", [hex: :mariaex, repo: "hexpm", optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: true]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.13.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, repo: "hexpm", optional: true]}], "hexpm"}, + "ecto_sql": {:hex, :ecto_sql, "3.0.3", "dd17f2401a69bb2ec91d5564bd259ad0bc63ee32c2cb2e616d04f1559801dba6", [:mix], [{:db_connection, "~> 2.0", [hex: :db_connection, repo: "hexpm", optional: false]}, {:ecto, "~> 3.0.4", [hex: :ecto, repo: "hexpm", optional: false]}, {:mariaex, "~> 0.9.1", [hex: :mariaex, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.14.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.2.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm"}, "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [:rebar], [], "hexpm"}, "postgrex": {:hex, :postgrex, "0.13.5", "3d931aba29363e1443da167a4b12f06dcd171103c424de15e5f3fc2ba3e6d9c5", [:mix], [{:connection, "~> 1.0", [hex: :connection, repo: "hexpm", optional: false]}, {:db_connection, "~> 1.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: false]}], "hexpm"}, + "telemetry": {:hex, :telemetry, "0.2.0", "5b40caa3efe4deb30fb12d7cd8ed4f556f6d6bd15c374c2366772161311ce377", [:mix], [], "hexpm"}, } diff --git a/test/constraint_test.exs b/test/constraint_test.exs new file mode 100644 index 0000000..268c563 --- /dev/null +++ b/test/constraint_test.exs @@ -0,0 +1,21 @@ +defmodule AlogTest.ConstraintTest do + use Alog.TestApp.DataCase + + alias Alog.TestApp.{User, Helpers} + + describe "apply_constraints/1:" do + test "returns error if not unique" do + {:ok, user_1} = %User{} |> User.changeset(Helpers.user_1_params()) |> User.insert() + + assert {:error, user_2} = + %User{} + |> User.changeset( + Helpers.user_2_params() + |> Map.merge(%{username: user_1.username}) + ) + |> User.insert() + + assert user_2.errors == [username: {"has already been taken", []}] + end + end +end diff --git a/test/support/user.ex b/test/support/user.ex index 2e18bcb..dfc77fe 100644 --- a/test/support/user.ex +++ b/test/support/user.ex @@ -20,6 +20,7 @@ defmodule Alog.TestApp.User do user |> cast(attrs, [:name, :username, :postcode, :deleted]) |> validate_required([:name, :username, :postcode]) + |> unique_constraint(:username) end def user_and_item_changeset(user, attrs) do diff --git a/test/update_test.exs b/test/update_test.exs index f7f7feb..076f639 100644 --- a/test/update_test.exs +++ b/test/update_test.exs @@ -28,9 +28,9 @@ defmodule AlogTest.UpdateTest do end test "associations remain after update" do - {:ok, user, item} = Helpers.seed_data() + {:ok, user, _item} = Helpers.seed_data() - {:ok, updated_user} = user |> User.changeset(%{postcode: "W2 3EC"}) |> User.update() + {:ok, _updated_user} = user |> User.changeset(%{postcode: "W2 3EC"}) |> User.update() assert User.get(user.entry_id) |> User.preload(:items) |> Map.get(:items) |> length == 1 end From 49a3beffa8f94a85756104a0e7e7865e200657b7 Mon Sep 17 00:00:00 2001 From: Danielwhyte Date: Wed, 12 Dec 2018 10:48:24 +0000 Subject: [PATCH 3/6] applies unique constraint on update function, #21 --- lib/alog.ex | 1 + test/constraint_test.exs | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/alog.ex b/lib/alog.ex index f0f8287..1ad8fc4 100644 --- a/lib/alog.ex +++ b/lib/alog.ex @@ -179,6 +179,7 @@ defmodule Alog do |> Map.put(:updated_at, nil) changeset + |> apply_constraints() |> Map.put(:data, data) |> @repo.insert() diff --git a/test/constraint_test.exs b/test/constraint_test.exs index 268c563..54f574d 100644 --- a/test/constraint_test.exs +++ b/test/constraint_test.exs @@ -4,7 +4,7 @@ defmodule AlogTest.ConstraintTest do alias Alog.TestApp.{User, Helpers} describe "apply_constraints/1:" do - test "returns error if not unique" do + test "returns error if not unique on insert" do {:ok, user_1} = %User{} |> User.changeset(Helpers.user_1_params()) |> User.insert() assert {:error, user_2} = @@ -17,5 +17,17 @@ defmodule AlogTest.ConstraintTest do assert user_2.errors == [username: {"has already been taken", []}] end + + test "returns error if not unique on update" do + {:ok, user_1} = %User{} |> User.changeset(Helpers.user_1_params()) |> User.insert() + {:ok, user_2} = %User{} |> User.changeset(Helpers.user_2_params()) |> User.insert() + + assert {:error, user_2} = + user_2 + |> User.changeset(%{username: user_1.username}) + |> User.update() + + assert user_2.errors == [username: {"has already been taken", []}] + end end end From 7be7e821f143e1432341441d086c0a2e11952a89 Mon Sep 17 00:00:00 2001 From: Danielwhyte Date: Wed, 12 Dec 2018 10:49:23 +0000 Subject: [PATCH 4/6] bumps version to 0.4 --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index 33138a3..da9383d 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Alog.MixProject do def project do [ app: :alog, - version: "0.3.0", + version: "0.4.0", elixir: "~> 1.7", elixirc_paths: elixirc_paths(Mix.env()), start_permanent: Mix.env() == :prod, From 894e82c3c2cfc3ca5d08a1af861607f2f8582dc3 Mon Sep 17 00:00:00 2001 From: Danielwhyte Date: Wed, 12 Dec 2018 10:50:53 +0000 Subject: [PATCH 5/6] updates README with unique_constraint info --- README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 5c796b9..285586c 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ This module provides some helper functions to make it easy to insert and retriev Alog expects your `Repo` to belong to the same base module as the schema. So if your schema is `MyApp.User`, or `MyApp.Accounts.User`, your Repo should be `MyApp.Repo`. - ## Indexes + ## Uniqueness Due to the append only manner in which Alog stores data, it is not compatible with tables that have Unique Indexes applied to any of their columns. If you wish to use alog, you will have to remove these indexes. @@ -47,4 +47,6 @@ This module provides some helper functions to make it easy to insert and retriev drop(unique_index(:users, :email)) ``` - See https://hexdocs.pm/ecto_sql/Ecto.Migration.html#content for more details. \ No newline at end of file + See https://hexdocs.pm/ecto_sql/Ecto.Migration.html#content for more details. + + If you want to ensure each entry in your database has a unique field, you can use the [`Ecto.Changeset.unique_constraint/3`](https://hexdocs.pm/ecto/Ecto.Changeset.html#unique_constraint/3) function as normal, and Alog will ensure there are no repeated fields, other than those of the same entry, returning an invalid changeset if there are. \ No newline at end of file From 7e72e5503200b3f661a02a5c14dc3f323dbb587d Mon Sep 17 00:00:00 2001 From: Danielwhyte Date: Wed, 12 Dec 2018 11:06:58 +0000 Subject: [PATCH 6/6] removes unnecessary deps from mix.lock --- mix.lock | 2 -- 1 file changed, 2 deletions(-) diff --git a/mix.lock b/mix.lock index 2c326fb..7f16fd1 100644 --- a/mix.lock +++ b/mix.lock @@ -3,8 +3,6 @@ "db_connection": {:hex, :db_connection, "1.1.3", "89b30ca1ef0a3b469b1c779579590688561d586694a3ce8792985d4d7e575a61", [:mix], [{:connection, "~> 1.0.2", [hex: :connection, repo: "hexpm", optional: false]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, repo: "hexpm", optional: true]}], "hexpm"}, "decimal": {:hex, :decimal, "1.6.0", "bfd84d90ff966e1f5d4370bdd3943432d8f65f07d3bab48001aebd7030590dcc", [:mix], [], "hexpm"}, "ecto": {:hex, :ecto, "2.2.11", "4bb8f11718b72ba97a2696f65d247a379e739a0ecabf6a13ad1face79844791c", [:mix], [{:db_connection, "~> 1.1", [hex: :db_connection, repo: "hexpm", optional: true]}, {:decimal, "~> 1.2", [hex: :decimal, repo: "hexpm", optional: false]}, {:mariaex, "~> 0.8.0", [hex: :mariaex, repo: "hexpm", optional: true]}, {:poison, "~> 2.2 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: true]}, {:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.13.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:sbroker, "~> 1.0", [hex: :sbroker, repo: "hexpm", optional: true]}], "hexpm"}, - "ecto_sql": {:hex, :ecto_sql, "3.0.3", "dd17f2401a69bb2ec91d5564bd259ad0bc63ee32c2cb2e616d04f1559801dba6", [:mix], [{:db_connection, "~> 2.0", [hex: :db_connection, repo: "hexpm", optional: false]}, {:ecto, "~> 3.0.4", [hex: :ecto, repo: "hexpm", optional: false]}, {:mariaex, "~> 0.9.1", [hex: :mariaex, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.14.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.2.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm"}, "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [:rebar], [], "hexpm"}, "postgrex": {:hex, :postgrex, "0.13.5", "3d931aba29363e1443da167a4b12f06dcd171103c424de15e5f3fc2ba3e6d9c5", [:mix], [{:connection, "~> 1.0", [hex: :connection, repo: "hexpm", optional: false]}, {:db_connection, "~> 1.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: false]}], "hexpm"}, - "telemetry": {:hex, :telemetry, "0.2.0", "5b40caa3efe4deb30fb12d7cd8ed4f556f6d6bd15c374c2366772161311ce377", [:mix], [], "hexpm"}, }