From 8bf1e2ede986fed0cd002740ac0c751ea974398d Mon Sep 17 00:00:00 2001 From: Emad Shaaban Date: Wed, 19 Jun 2024 21:22:21 +0300 Subject: [PATCH] fix: ensure index keys are atoms in generated migrations (#332) --------- Co-authored-by: Zach Daniel --- .../migration_generator.ex | 30 ++++++++++++++++++- .../20240618102809_migrate_resources30.exs | 2 +- test/migration_generator_test.exs | 24 +++++++-------- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/lib/migration_generator/migration_generator.ex b/lib/migration_generator/migration_generator.ex index 2d96b5d..b586f6d 100644 --- a/lib/migration_generator/migration_generator.ex +++ b/lib/migration_generator/migration_generator.ex @@ -2884,7 +2884,7 @@ defmodule AshPostgres.MigrationGenerator do Enum.map(keys, fn key -> case Ash.Resource.Info.field(resource, key) do %Ash.Resource.Attribute{} = attribute -> - to_string(attribute.source || attribute.name) + attribute.source || attribute.name %Ash.Resource.Calculation{} -> sql = @@ -3000,6 +3000,17 @@ defmodule AshPostgres.MigrationGenerator do %{index | fields: fields} end) end) + |> Map.update!(:identities, fn identities -> + Enum.map(identities, fn identity -> + keys = + Enum.map(identity.keys, fn + value when is_binary(value) -> %{"type" => "string", "value" => value} + value when is_atom(value) -> %{"type" => "atom", "value" => value} + end) + + %{identity | keys: keys} + end) + end) |> Jason.encode!(pretty: true) end @@ -3251,6 +3262,23 @@ defmodule AshPostgres.MigrationGenerator do |> Map.put_new(:all_tenants?, false) |> Map.put_new(:where, nil) |> Map.put_new(:nils_distinct?, true) + |> Map.update!( + :keys, + &Enum.map(&1, fn + %{type: "atom", value: value} -> + maybe_to_atom(value) + + %{type: "string", value: value} -> + value + + value -> + if String.contains?(value, "(") do + value + else + maybe_to_atom(value) + end + end) + ) end defp add_index_name(%{name: name} = index, table) do diff --git a/priv/test_repo/migrations/20240618102809_migrate_resources30.exs b/priv/test_repo/migrations/20240618102809_migrate_resources30.exs index 5a4aa79..f3cec44 100644 --- a/priv/test_repo/migrations/20240618102809_migrate_resources30.exs +++ b/priv/test_repo/migrations/20240618102809_migrate_resources30.exs @@ -1,4 +1,4 @@ -defmodule AshPostgres.TestRepo.Migrations.MigrateResources29 do +defmodule AshPostgres.TestRepo.Migrations.MigrateResources30 do @moduledoc """ Updates resources based on their most recent snapshots. diff --git a/test/migration_generator_test.exs b/test/migration_generator_test.exs index 804ac50..40c8ea3 100644 --- a/test/migration_generator_test.exs +++ b/test/migration_generator_test.exs @@ -156,15 +156,15 @@ defmodule AshPostgres.MigrationGeneratorTest do # the migration creates unique_indexes based on the identities of the resource assert file_contents =~ - ~S{create unique_index(:posts, ["title"], name: "posts_title_index")} + ~S{create unique_index(:posts, [:title], name: "posts_title_index")} # the migration creates unique_indexes based on the identities of the resource assert file_contents =~ - ~S{create unique_index(:posts, ["title", "second_title"], name: "posts_thing_index")} + ~S{create unique_index(:posts, [:title, :second_title], name: "posts_thing_index")} # the migration creates unique_indexes using the `source` on the attributes of the identity on the resource assert file_contents =~ - ~S{create unique_index(:posts, ["title", "t_w_s"], name: "posts_thing_with_source_index")} + ~S{create unique_index(:posts, [:title, :t_w_s], name: "posts_thing_with_source_index")} end end @@ -241,7 +241,7 @@ defmodule AshPostgres.MigrationGeneratorTest do assert file_contents =~ ~S{create index(:posts, ["id"]} assert file_contents =~ - ~S{create unique_index(:posts, ["second_title"], name: "posts_second_title_index", prefix: "example", nulls_distinct: false, where: "((second_title like '%foo%'))")} + ~S{create unique_index(:posts, [:second_title], name: "posts_second_title_index", prefix: "example", nulls_distinct: false, where: "((second_title like '%foo%'))")} # the migration adds the id, with its default assert file_contents =~ @@ -255,7 +255,7 @@ defmodule AshPostgres.MigrationGeneratorTest do # the migration creates unique_indexes based on the identities of the resource assert file_contents =~ - ~S{create unique_index(:posts, ["title"], name: "posts_title_index", prefix: "example")} + ~S{create unique_index(:posts, [:title], name: "posts_title_index", prefix: "example")} end end @@ -535,11 +535,11 @@ defmodule AshPostgres.MigrationGeneratorTest do assert [file_before, _] = String.split( File.read!(file2), - ~S{create unique_index(:posts, ["title"], name: "posts_title_index", where: "(title != 'fred' and title != 'george')")} + ~S{create unique_index(:posts, [:title], name: "posts_title_index", where: "(title != 'fred' and title != 'george')")} ) assert file_before =~ - ~S{drop_if_exists unique_index(:posts, ["title"], name: "posts_title_index")} + ~S{drop_if_exists unique_index(:posts, [:title], name: "posts_title_index")} end test "when adding a field, it adds the field" do @@ -753,18 +753,18 @@ defmodule AshPostgres.MigrationGeneratorTest do file1_content = File.read!(file1) assert file1_content =~ - "create unique_index(:posts, [\"title\"], name: \"posts_title_index\")" + "create unique_index(:posts, [:title], name: \"posts_title_index\")" file2_content = File.read!(file2) assert file2_content =~ - "drop_if_exists unique_index(:posts, [\"title\"], name: \"posts_title_index\")" + "drop_if_exists unique_index(:posts, [:title], name: \"posts_title_index\")" assert file2_content =~ - "create unique_index(:posts, [\"name\"], name: \"posts_unique_name_index\")" + "create unique_index(:posts, [:name], name: \"posts_unique_name_index\")" assert file2_content =~ - "create unique_index(:posts, [\"title\"], name: \"posts_unique_title_index\")" + "create unique_index(:posts, [:title], name: \"posts_unique_title_index\")" end test "when an attribute exists only on some of the resources that use the same table, it isn't marked as null: false" do @@ -1241,7 +1241,7 @@ defmodule AshPostgres.MigrationGeneratorTest do assert [file] = Path.wildcard("test_migration_path/**/*_migrate_resources*.exs") assert File.read!(file) =~ - ~S{create unique_index(:users, ["name"], name: "users_unique_name_index")} + ~S{create unique_index(:users, [:name], name: "users_unique_name_index")} end test "when modified, the foreign key is dropped before modification" do