From 4d1d2120587325b551250af7b6c2e0986f37f5c3 Mon Sep 17 00:00:00 2001 From: zimt28 <1764689+zimt28@users.noreply.github.com> Date: Sat, 9 Jan 2021 20:13:16 +0100 Subject: [PATCH 1/5] Omit field opts if they are default values --- lib/migration_generator/operation.ex | 48 ++++++++++++++++++---------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/lib/migration_generator/operation.ex b/lib/migration_generator/operation.ex index 63c5a56a..45fac8c3 100644 --- a/lib/migration_generator/operation.ex +++ b/lib/migration_generator/operation.ex @@ -1,5 +1,17 @@ defmodule AshPostgres.MigrationGenerator.Operation do @moduledoc false + + defmodule Helper do + def maybe_add_default(nil), do: "" + def maybe_add_default(value), do: ", default: #{value}" + + def maybe_add_primary_key(true), do: ", primary_key: true" + def maybe_add_primary_key(_), do: "" + + def maybe_add_null(false), do: ", null: false" + def maybe_add_null(_), do: "" + end + defmodule CreateTable do @moduledoc false defstruct [:table, :multitenancy, :old_multitenancy] @@ -9,6 +21,8 @@ defmodule AshPostgres.MigrationGenerator.Operation do @moduledoc false defstruct [:attribute, :table, :multitenancy, :old_multitenancy] + import Helper + def up(%{ multitenancy: %{strategy: :attribute, attribute: source_attribute}, attribute: @@ -24,7 +38,7 @@ defmodule AshPostgres.MigrationGenerator.Operation do inspect(attribute.type) }, column: #{inspect(destination_field)}, with: [#{source_attribute}: :#{ destination_attribute - }]), default: #{attribute.default}, primary_key: #{attribute.primary_key?}" + }]) #{maybe_add_default(attribute.default)} #{maybe_add_primary_key(attribute.primary_key?)}" end def up(%{ @@ -40,9 +54,9 @@ defmodule AshPostgres.MigrationGenerator.Operation do }) do "add #{inspect(attribute.name)}, references(#{inspect(table)}, type: #{ inspect(attribute.type) - }, column: #{inspect(destination_field)}, name: \"\#\{prefix\}_#{table}_#{attribute.name}_fkey\", prefix: \"public\"), default: #{ - attribute.default - }, primary_key: #{attribute.primary_key?}" + }, column: #{inspect(destination_field)}, name: \"\#\{prefix\}_#{table}_#{attribute.name}_fkey\", prefix: \"public\") #{ + maybe_add_default(attribute.default) + } #{maybe_add_primary_key(attribute.primary_key?)}" end def up(%{ @@ -62,9 +76,9 @@ defmodule AshPostgres.MigrationGenerator.Operation do you should be aware of """) - "add #{inspect(attribute.name)}, #{inspect(attribute.type)}, default: #{attribute.default}, primary_key: #{ - attribute.primary_key? - }" + "add #{inspect(attribute.name)}, #{inspect(attribute.type)} #{ + maybe_add_default(attribute.default) + } #{maybe_add_primary_key(attribute.primary_key?)}" end def up(%{ @@ -74,9 +88,9 @@ defmodule AshPostgres.MigrationGenerator.Operation do }) do "add #{inspect(attribute.name)}, references(#{inspect(table)}, type: #{ inspect(attribute.type) - }, column: #{inspect(destination_field)}, name: \"\#\{prefix\}_#{table}_#{attribute.name}_fkey\"), default: #{ - attribute.default - }, primary_key: #{attribute.primary_key?}" + }, column: #{inspect(destination_field)}, name: \"\#\{prefix\}_#{table}_#{attribute.name}_fkey\") #{ + maybe_add_default(attribute.default) + } #{maybe_add_primary_key(attribute.primary_key?)}" end def up(%{ @@ -85,21 +99,21 @@ defmodule AshPostgres.MigrationGenerator.Operation do }) do "add #{inspect(attribute.name)}, references(#{inspect(table)}, type: #{ inspect(attribute.type) - }, column: #{inspect(destination_field)}), default: #{attribute.default}, primary_key: #{ - attribute.primary_key? + }, column: #{inspect(destination_field)}) #{maybe_add_default(attribute.default)} #{ + maybe_add_primary_key(attribute.primary_key?) }" end def up(%{attribute: %{type: :integer, default: "nil", generated?: true} = attribute}) do - "add #{inspect(attribute.name)}, :serial, null: #{attribute.allow_nil?}, primary_key: #{ - attribute.primary_key? + "add #{inspect(attribute.name)}, :serial #{maybe_add_null(attribute.allow_nil?)} #{ + maybe_add_primary_key(attribute.primary_key?) }" end def up(%{attribute: attribute}) do - "add #{inspect(attribute.name)}, #{inspect(attribute.type)}, null: #{attribute.allow_nil?}, default: #{ - attribute.default - }, primary_key: #{attribute.primary_key?}" + "add #{inspect(attribute.name)}, #{inspect(attribute.type)} #{ + maybe_add_null(attribute.allow_nil?) + } #{maybe_add_default(attribute.default)} #{maybe_add_primary_key(attribute.primary_key?)}" end def down( From 517ac5b70ee7fbb2f2462e482376200271765f2d Mon Sep 17 00:00:00 2001 From: zimt28 <1764689+zimt28@users.noreply.github.com> Date: Sat, 9 Jan 2021 20:17:42 +0100 Subject: [PATCH 2/5] Fix match --- lib/migration_generator/operation.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/migration_generator/operation.ex b/lib/migration_generator/operation.ex index 45fac8c3..68822042 100644 --- a/lib/migration_generator/operation.ex +++ b/lib/migration_generator/operation.ex @@ -2,7 +2,7 @@ defmodule AshPostgres.MigrationGenerator.Operation do @moduledoc false defmodule Helper do - def maybe_add_default(nil), do: "" + def maybe_add_default("nil"), do: "" def maybe_add_default(value), do: ", default: #{value}" def maybe_add_primary_key(true), do: ", primary_key: true" From 5c2dd1611ef47ebd6c6bf7d7915d3bd1fad54996 Mon Sep 17 00:00:00 2001 From: zimt28 <1764689+zimt28@users.noreply.github.com> Date: Sun, 10 Jan 2021 01:02:43 +0100 Subject: [PATCH 3/5] Adapt tests --- test/migration_generator_test.exs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/migration_generator_test.exs b/test/migration_generator_test.exs index 01ee0f49..9a7f7250 100644 --- a/test/migration_generator_test.exs +++ b/test/migration_generator_test.exs @@ -101,14 +101,14 @@ defmodule AshPostgres.MigrationGeneratorTest do assert [file] = Path.wildcard("test_migration_path/**/*_migrate_resources*.exs") assert File.read!(file) =~ - ~S[add :id, :binary_id, null: true, default: fragment("uuid_generate_v4()"), primary_key: true] + ~S[add :id, :binary_id, default: fragment("uuid_generate_v4()"), primary_key: true] end test "the migration adds other attributes" do assert [file] = Path.wildcard("test_migration_path/**/*_migrate_resources*.exs") assert File.read!(file) =~ - ~S[add :title, :text, null: true, default: nil, primary_key: false] + ~S[add :title, :text] end test "the migration creates unique_indexes based on the identities of the resource" do @@ -181,7 +181,7 @@ defmodule AshPostgres.MigrationGeneratorTest do Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) assert File.read!(file2) =~ - ~S[add :name, :text, null: false, default: nil, primary_key: false] + ~S[add :name, :text, null: false] end test "when renaming a field, it asks if you are renaming it, and renames it if you are" do @@ -232,7 +232,7 @@ defmodule AshPostgres.MigrationGeneratorTest do Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) assert File.read!(file2) =~ - ~S[add :name, :text, null: false, default: nil, primary_key: false] + ~S[add :name, :text, null: false] end test "when renaming a field, it asks which field you are renaming it to, and renames it if you are" do @@ -286,7 +286,7 @@ defmodule AshPostgres.MigrationGeneratorTest do Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) assert File.read!(file2) =~ - ~S[add :subject, :text, null: false, default: nil, primary_key: false] + ~S[add :subject, :text, null: false] end test "when changing the primary key, it changes properly" do @@ -311,7 +311,7 @@ defmodule AshPostgres.MigrationGeneratorTest do Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) assert File.read!(file2) =~ - ~S[add :guid, :binary_id, null: true, default: fragment("uuid_generate_v4()"), primary_key: true] + ~S[add :guid, :binary_id, default: fragment("uuid_generate_v4()"), primary_key: true] end test "when multiple schemas apply to the same table, all attributes are added" do @@ -343,10 +343,10 @@ defmodule AshPostgres.MigrationGeneratorTest do Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) assert File.read!(file2) =~ - ~S[add :foobar, :text, null: true, default: nil, primary_key: false] + ~S[add :foobar, :text] assert File.read!(file2) =~ - ~S[add :foobar, :text, null: true, default: nil, primary_key: false] + ~S[add :foobar, :text] end end @@ -360,7 +360,7 @@ defmodule AshPostgres.MigrationGeneratorTest do defposts do attributes do attribute(:id, :integer, generated?: true, allow_nil?: false, primary_key?: true) - attribute(:views, :integer, default: nil) + attribute(:views, :integer) end end @@ -385,7 +385,7 @@ defmodule AshPostgres.MigrationGeneratorTest do ~S[add :id, :serial, null: false, primary_key: true] assert File.read!(file) =~ - ~S[add :views, :integer, null: true, default: nil, primary_key: false] + ~S[add :views, :integer] end end end From 0db3e5e4eff11a20bb56ed830fab26701e91f045 Mon Sep 17 00:00:00 2001 From: zimt28 <1764689+zimt28@users.noreply.github.com> Date: Sun, 10 Jan 2021 01:04:23 +0100 Subject: [PATCH 4/5] Improve readability --- lib/migration_generator/operation.ex | 113 +++++++++++++++++++-------- 1 file changed, 80 insertions(+), 33 deletions(-) diff --git a/lib/migration_generator/operation.ex b/lib/migration_generator/operation.ex index 68822042..8f27b612 100644 --- a/lib/migration_generator/operation.ex +++ b/lib/migration_generator/operation.ex @@ -2,14 +2,16 @@ defmodule AshPostgres.MigrationGenerator.Operation do @moduledoc false defmodule Helper do - def maybe_add_default("nil"), do: "" - def maybe_add_default(value), do: ", default: #{value}" + def join(list), do: list |> List.flatten() |> Enum.reject(&is_nil/1) |> Enum.join(", ") - def maybe_add_primary_key(true), do: ", primary_key: true" - def maybe_add_primary_key(_), do: "" + def maybe_add_default("nil"), do: nil + def maybe_add_default(value), do: "default: #{value}" - def maybe_add_null(false), do: ", null: false" - def maybe_add_null(_), do: "" + def maybe_add_primary_key(true), do: "primary_key: true" + def maybe_add_primary_key(_), do: nil + + def maybe_add_null(false), do: "null: false" + def maybe_add_null(_), do: nil end defmodule CreateTable do @@ -34,11 +36,19 @@ defmodule AshPostgres.MigrationGenerator.Operation do } } = attribute }) do - "add #{inspect(attribute.name)}, references(#{inspect(table)}, type: #{ - inspect(attribute.type) - }, column: #{inspect(destination_field)}, with: [#{source_attribute}: :#{ - destination_attribute - }]) #{maybe_add_default(attribute.default)} #{maybe_add_primary_key(attribute.primary_key?)}" + [ + "add #{inspect(attribute.name)}", + "references(#{inspect(table)}", + [ + "type: #{inspect(attribute.type)}", + "column: #{inspect(destination_field)}", + "with: [#{source_attribute}: :#{destination_attribute}]" + ], + ")", + maybe_add_default(attribute.default), + maybe_add_primary_key(attribute.primary_key?) + ] + |> join() end def up(%{ @@ -52,11 +62,20 @@ defmodule AshPostgres.MigrationGenerator.Operation do } } = attribute }) do - "add #{inspect(attribute.name)}, references(#{inspect(table)}, type: #{ - inspect(attribute.type) - }, column: #{inspect(destination_field)}, name: \"\#\{prefix\}_#{table}_#{attribute.name}_fkey\", prefix: \"public\") #{ - maybe_add_default(attribute.default) - } #{maybe_add_primary_key(attribute.primary_key?)}" + [ + "add #{inspect(attribute.name)}", + "references(#{inspect(table)}", + [ + "type: #{inspect(attribute.type)}", + "column: #{inspect(destination_field)}", + "name: \"\#\{prefix\}_#{table}_#{attribute.name}_fkey\"", + "prefix: \"public\"" + ], + ")", + maybe_add_default(attribute.default), + maybe_add_primary_key(attribute.primary_key?) + ] + |> join() end def up(%{ @@ -76,9 +95,13 @@ defmodule AshPostgres.MigrationGenerator.Operation do you should be aware of """) - "add #{inspect(attribute.name)}, #{inspect(attribute.type)} #{ - maybe_add_default(attribute.default) - } #{maybe_add_primary_key(attribute.primary_key?)}" + [ + "add #{inspect(attribute.name)}", + inspect(attribute.type), + maybe_add_default(attribute.default), + maybe_add_primary_key(attribute.primary_key?) + ] + |> join() end def up(%{ @@ -86,34 +109,58 @@ defmodule AshPostgres.MigrationGenerator.Operation do attribute: %{references: %{table: table, destination_field: destination_field}} = attribute }) do - "add #{inspect(attribute.name)}, references(#{inspect(table)}, type: #{ - inspect(attribute.type) - }, column: #{inspect(destination_field)}, name: \"\#\{prefix\}_#{table}_#{attribute.name}_fkey\") #{ - maybe_add_default(attribute.default) - } #{maybe_add_primary_key(attribute.primary_key?)}" + [ + "add #{inspect(attribute.name)}", + "references(#{inspect(table)}", + [ + "type: #{inspect(attribute.type)}", + "column: #{inspect(destination_field)}", + "name: \"\#\{prefix\}_#{table}_#{attribute.name}_fkey\"" + ], + ")", + maybe_add_default(attribute.default), + maybe_add_primary_key(attribute.primary_key?) + ] + |> join() end def up(%{ attribute: %{references: %{table: table, destination_field: destination_field}} = attribute }) do - "add #{inspect(attribute.name)}, references(#{inspect(table)}, type: #{ - inspect(attribute.type) - }, column: #{inspect(destination_field)}) #{maybe_add_default(attribute.default)} #{ + [ + "add #{inspect(attribute.name)}", + "references(#{inspect(table)}", + [ + "type: #{inspect(attribute.type)}", + "column: #{inspect(destination_field)}" + ], + ")", + maybe_add_default(attribute.default), maybe_add_primary_key(attribute.primary_key?) - }" + ] + |> join() end def up(%{attribute: %{type: :integer, default: "nil", generated?: true} = attribute}) do - "add #{inspect(attribute.name)}, :serial #{maybe_add_null(attribute.allow_nil?)} #{ + [ + "add #{inspect(attribute.name)}", + ":serial", + maybe_add_null(attribute.allow_nil?), maybe_add_primary_key(attribute.primary_key?) - }" + ] + |> join() end def up(%{attribute: attribute}) do - "add #{inspect(attribute.name)}, #{inspect(attribute.type)} #{ - maybe_add_null(attribute.allow_nil?) - } #{maybe_add_default(attribute.default)} #{maybe_add_primary_key(attribute.primary_key?)}" + [ + "add #{inspect(attribute.name)}", + "#{inspect(attribute.type)}", + maybe_add_null(attribute.allow_nil?), + maybe_add_default(attribute.default), + maybe_add_primary_key(attribute.primary_key?) + ] + |> join() end def down( From eb78856743727795703f66e356102fa0123a4772 Mon Sep 17 00:00:00 2001 From: zimt28 <1764689+zimt28@users.noreply.github.com> Date: Sun, 10 Jan 2021 01:12:05 +0100 Subject: [PATCH 5/5] Add missing moduledoc --- lib/migration_generator/operation.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/migration_generator/operation.ex b/lib/migration_generator/operation.ex index 8f27b612..9af704ab 100644 --- a/lib/migration_generator/operation.ex +++ b/lib/migration_generator/operation.ex @@ -2,6 +2,7 @@ defmodule AshPostgres.MigrationGenerator.Operation do @moduledoc false defmodule Helper do + @moduledoc false def join(list), do: list |> List.flatten() |> Enum.reject(&is_nil/1) |> Enum.join(", ") def maybe_add_default("nil"), do: nil