diff --git a/documentation/topics/development/migrations-and-tasks.md b/documentation/topics/development/migrations-and-tasks.md index e94f53e..b101135 100644 --- a/documentation/topics/development/migrations-and-tasks.md +++ b/documentation/topics/development/migrations-and-tasks.md @@ -37,6 +37,63 @@ For single-step changes or when you know the final feature name: For more information on generating migrations, run `mix help ash_sqlite.generate_migrations` (the underlying task that is called by `mix ash.codegen`) +### Changes SQLite cannot make in place + +SQLite can mostly add, rename and drop a column in place. Changing a column's type or default, adding or dropping a foreign key, changing a primary key, making a table `STRICT` and, as the adapters write it, making a column required or optional all need the table to be created again in its new shape, with the rows copied over. + +By default the generator writes a statement that fails when the migration runs, or leaves the change out (a renamed attribute that also changes, a table that becomes `STRICT`). With `--rebuild-tables`, or `rebuild_tables: true` in the repo's config, it writes a `rebuild_table` instead, which shows the table's new shape and what triggered it: + +```elixir +def up do + # SQLite cannot change `comments` in place, so it is rebuilt from the resource's + # snapshot. Anything on the table that the resource does not describe (a column, + # index or trigger added by hand) is not kept. Changes: + # - dropping the foreign key comments_post_id_fkey + rebuild_table :comments do + add :id, :uuid, null: false, primary_key: true + add :post_id, :uuid + end +end +``` + +A rebuild needs an `ecto_sql` that runs the `before_transaction/1` and `after_transaction/1` migration callbacks, which is how foreign keys are switched off outside the transaction. Without them, asking for rebuilds raises, and the generator only prints its hint, without suggesting one. + +The rebuild runs in one transaction with foreign keys off, and changes nothing if it fails. Columns both tables have are copied as they are. When the right value for the rows depends on your data, as when a column becomes required and has no default, a `REVIEW` comment says so and shows how to give them one. + +The custom statements of the resource are dropped (their `down`) before the rebuild and recreated (their `up`) after it, so what they hold is lost. That suits what can be dropped and recreated without losing anything, such as a trigger, a view or an index, and a full-text index, which is rebuilt from its table. Data that exists nowhere else does not belong in a custom statement. The full-text index needs a command that you add to the generated migration, after the statement's `up`: + +```elixir +statement :items_fts do + up "CREATE VIRTUAL TABLE items_fts USING fts5(name, content='items', content_rowid='rowid')" + down "DROP TABLE items_fts" +end +``` + +```elixir +def up do + # SQLite cannot change `items` in place, so it is rebuilt ... Changes: + # ... + execute(""" + DROP TABLE items_fts + """) + + rebuild_table :items do + add :id, :uuid, null: false, primary_key: true + add :name, :text + end + + execute(""" + CREATE VIRTUAL TABLE items_fts USING fts5(name, content='items', content_rowid='rowid') + """) + + execute("INSERT INTO items_fts(items_fts) VALUES('rebuild')") # added +end +``` + +The `default:` of an attribute (a number, string, boolean, atom, decimal, date or time) is then also the default of its column. SQLite cannot change a default in place either, so the first run rebuilds the tables whose attributes have one: `--dry-run` shows which. + +Snapshots keep the defaults, so a run without the option after one with it sees them as removed. If you use rebuilds, set `rebuild_tables: true` in the repo's config instead of passing the flag, so that every run agrees, `mix ash.codegen --check` in CI included. + ### Regenerating Migrations Often, you will run into a situation where you want to make a slight change to a resource after you've already generated and run migrations. If you are using git and would like to undo those changes, then regenerate the migrations, this script may prove useful: diff --git a/lib/migration.ex b/lib/migration.ex new file mode 100644 index 0000000..f9b173d --- /dev/null +++ b/lib/migration.ex @@ -0,0 +1,242 @@ +# SPDX-FileCopyrightText: 2023 ash_sqlite contributors +# +# SPDX-License-Identifier: MIT + +defmodule AshSqlite.Migration do + @moduledoc """ + Helpers for migrations that rebuild a table. + + SQLite can mostly add, rename and drop columns in place. For most other changes it is + [documented](https://www.sqlite.org/lang_altertable.html#otheralter) to create the table in + its new shape, copy the rows, drop the old table and rename the new one: + + use AshSqlite.Migration + + def up do + rebuild_table :posts do + add :id, :uuid, null: false, primary_key: true + add :title, :text + end + end + + `use AshSqlite.Migration` does what `use Ecto.Migration` does, and adds what makes a rebuild + safe: foreign keys are off while it runs (dropping a table would otherwise run the `ON DELETE` + actions of the tables that point to it) and they are checked before it commits. A rebuild + either completes or changes nothing. + + Foreign keys can only be switched off by an `ecto_sql` that runs the `before_transaction/1` + and `after_transaction/1` migration callbacks. Without them, a rebuild refuses to run + unless the connection has foreign keys off already. + """ + + @pragmas_key :ash_sqlite_rebuild_pragmas + + @doc false + defmacro __using__(_opts) do + quote do + use Ecto.Migration + import AshSqlite.Migration, only: [rebuild_table: 2, rebuild_table: 3] + + defdelegate before_transaction(repo), to: AshSqlite.Migration, as: :__before_transaction__ + defdelegate after_transaction(repo), to: AshSqlite.Migration, as: :__after_transaction__ + + def before_commit, do: AshSqlite.Migration.__check_foreign_keys__(repo()) + end + end + + # An `ecto_sql` without the callbacks never calls the ones `use AshSqlite.Migration` adds. + @doc false + def rebuild_supported? do + {:before_transaction, 1} in Ecto.Migration.behaviour_info(:optional_callbacks) + end + + # `PRAGMA foreign_keys` does nothing inside a transaction, so it is set before Ecto opens one, + # on the connection it will use. The previous values are kept to put them back afterwards. + @doc false + def __before_transaction__(repo) do + Process.put(@pragmas_key, {pragma(repo, "foreign_keys"), pragma(repo, "legacy_alter_table")}) + quiet!(repo, "PRAGMA foreign_keys = OFF") + end + + # `legacy_alter_table` is put back too: a rebuild that fails does not turn it off itself. + @doc false + def __after_transaction__(repo) do + case Process.delete(@pragmas_key) do + nil -> + :ok + + {foreign_keys, legacy_alter_table} -> + quiet!(repo, "PRAGMA legacy_alter_table = #{legacy_alter_table}") + quiet!(repo, "PRAGMA foreign_keys = #{foreign_keys}") + end + end + + @doc false + def __check_can_rebuild__(repo, table) do + check_foreign_keys_off!(repo, table) + check_in_transaction!(repo, table) + end + + # Ecto does not run the callbacks for a migration without a transaction + # (`@disable_ddl_transaction true`), and dropping a table with foreign keys on deletes + # from the tables that reference it. + defp check_foreign_keys_off!(repo, table) do + if pragma(repo, "foreign_keys") != 0 do + raise "refusing to rebuild #{table}: foreign keys are on, so dropping it would run " <> + "the ON DELETE actions of the tables that reference it. They are switched " <> + "off by the callbacks `use AshSqlite.Migration` adds. Ecto runs them only when " <> + "the migration runs in its own transaction (remove `@disable_ddl_transaction " <> + "true`), and only in an `ecto_sql` that has them." + end + end + + # Without one nothing keeps other connections from writing during the rebuild, and a failure + # leaves the table half rebuilt. + defp check_in_transaction!(repo, table) do + unless repo.in_transaction?() do + raise "refusing to rebuild #{table}: the migration is not in a transaction, so a rebuild " <> + "could lose concurrent writes or fail halfway. Remove `@disable_ddl_transaction true`." + end + end + + @doc false + def __legacy_alter_table__(repo, value), + do: quiet!(repo, "PRAGMA legacy_alter_table = #{value}") + + @doc """ + Rebuilds `table` with the shape `block` describes, as the block of a `create table`. + + Every column the old and the new table both have is copied as it is. A column the + old table did not have gets the `DEFAULT` of the new table. + + Options: + + * `:copy` - what to copy differently, as a keyword list from the new column to + where its value comes from: `new: :old` copies the column `old` into `new` (a + rename), and `new: "sql"` copies an SQL expression, which can look at the old + table's columns (for example `COALESCE(name, 'unknown')` for a column that is + becoming required, or `'unknown'` for a new required one). + * any other option is an option of the temporary `table/2`, for example + `options: "STRICT"`. + """ + defmacro rebuild_table(table, opts \\ [], do: block) do + quote do + table = unquote(table) + {copy, opts} = Keyword.pop(unquote(opts), :copy, []) + temporary = "#{table}_rebuild" + + execute(fn -> AshSqlite.Migration.__check_can_rebuild__(repo(), table) end) + + create table(temporary, [primary_key: false] ++ opts) do + unquote(block) + end + + # the columns of both tables are only known once the new one exists + execute(fn -> AshSqlite.Migration.__copy_rows__(repo(), table, temporary, copy) end) + + # While the old table is dropped, a view or trigger that mentions it points at nothing, and + # the rename that follows refuses. The legacy behaviour leaves them alone. It is not on for + # the rest of the migration, so that a `rename table` in it still updates what mentions it. + execute(fn -> AshSqlite.Migration.__legacy_alter_table__(repo(), 1) end) + drop(table(table)) + rename(table(temporary), to: table(table)) + execute(fn -> AshSqlite.Migration.__legacy_alter_table__(repo(), 0) end) + end + end + + @doc false + def __copy_rows__(repo, table, temporary, copy) do + copy_sequence(repo, table, temporary) + + old_columns = column_names(repo, table) + new_columns = column_names(repo, temporary) + + unknown = + copy |> Keyword.keys() |> Enum.map(&to_string/1) |> Enum.reject(&(&1 in new_columns)) + + if unknown != [] do + raise ArgumentError, + "`copy:` for #{table} names #{Enum.join(unknown, ", ")}, which the new table " <> + "does not have (it has #{Enum.join(new_columns, ", ")})" + end + + expressions = + for name <- new_columns, + expression = copy_expression(copy, name, old_columns), + do: {name, expression} + + if expressions == [] do + raise ArgumentError, + "`rebuild_table` for #{table} would copy nothing: the new table has no column in " <> + "common with the old one and no `copy:`, so its rows would be lost" + end + + names = Enum.map_join(expressions, ", ", fn {name, _} -> quote_name(name) end) + selected = Enum.map_join(expressions, ", ", fn {_, expression} -> expression end) + + query!( + repo, + "INSERT INTO #{quote_name(temporary)} (#{names}) SELECT #{selected} FROM #{quote_name(table)}" + ) + end + + # An AUTOINCREMENT table keeps the highest id it ever handed out in `sqlite_sequence`, which + # dropping the table deletes: ids that were used and deleted would be handed out again. + defp copy_sequence(repo, table, temporary) do + %{rows: [[autoincrement]]} = + quiet!( + repo, + "SELECT count(*) FROM sqlite_master WHERE name = ? AND sql GLOB '*PRIMARY KEY AUTOINCREMENT*'", + [temporary] + ) + + if autoincrement > 0 do + query!( + repo, + "INSERT INTO sqlite_sequence (name, seq) SELECT ?, seq FROM sqlite_sequence WHERE name = ?", + [temporary, to_string(table)] + ) + end + end + + defp copy_expression(copy, name, old_columns) do + case Enum.find(copy, fn {new, _} -> to_string(new) == name end) do + {_, from} when is_atom(from) -> quote_name(from) + {_, sql} when is_binary(sql) -> sql + nil -> if name in old_columns, do: quote_name(name) + end + end + + defp column_names(repo, table) do + %{rows: rows} = quiet!(repo, "PRAGMA table_info(#{quote_name(table)})") + Enum.map(rows, fn [_cid, name | _] -> name end) + end + + defp quote_name(name), do: ~s("#{name}") + + # not `repo.query!/1`: ecto_libsql's repo has none + # sobelow_skip ["SQL.Query"] + defp query!(repo, statement, params \\ []), + do: Ecto.Adapters.SQL.query!(repo, statement, params) + + # sobelow_skip ["SQL.Query"] + defp quiet!(repo, statement, params \\ []), + do: Ecto.Adapters.SQL.query!(repo, statement, params, log: false) + + defp pragma(repo, name) do + %{rows: [[value]]} = quiet!(repo, "PRAGMA #{name}") + value + end + + @doc false + def __check_foreign_keys__(repo) do + case quiet!(repo, "PRAGMA foreign_key_check") do + %{rows: []} -> + :ok + + %{rows: rows} -> + raise "foreign key check failed after rebuilding tables (table, rowid, parent, constraint): " <> + inspect(rows) + end + end +end diff --git a/lib/migration_generator/migration_generator.ex b/lib/migration_generator/migration_generator.ex index 067a445..6565bb7 100644 --- a/lib/migration_generator/migration_generator.ex +++ b/lib/migration_generator/migration_generator.ex @@ -21,7 +21,8 @@ defmodule AshSqlite.MigrationGenerator do check: false, dev: false, auto_name: false, - drop_columns: false + drop_columns: false, + rebuild_tables: nil def generate(domains, opts \\ []) do domains = List.wrap(domains) @@ -35,7 +36,7 @@ defmodule AshSqlite.MigrationGenerator do Ash.DataLayer.data_layer(resource) == AshSqlite.DataLayer && AshSqlite.DataLayer.Info.migrate?(resource) end) - |> Enum.flat_map(&get_snapshots(&1, all_resources)) + |> Enum.flat_map(&get_snapshots(&1, all_resources, opts)) repos = snapshots @@ -361,11 +362,12 @@ defmodule AshSqlite.MigrationGenerator do end end + phases = organize_operations(operations) + migration_files = - operations - |> organize_operations + phases |> build_up_and_down() - |> migration(repo, opts) + |> migration(repo, rebuilds?(phases), opts) snapshot_files = create_new_snapshot(snapshots, repo_name(repo), opts) @@ -403,41 +405,13 @@ defmodule AshSqlite.MigrationGenerator do end) with_repo_not_in_test(repo, fn repo -> - {repo, query, opts} = Ecto.Migration.SchemaMigration.versions(repo, [], nil) - - repo.transaction(fn -> - Ecto.Migration.SchemaMigration.ensure_schema_migrations_table!( - repo, - repo.config(), - [] - ) - - versions = repo.all(query, opts) - - dev_migrations - |> Enum.map(&extract_migration_info/1) - |> Enum.filter(& &1) - |> Enum.map(&load_migration!/1) - |> Enum.sort() - |> Enum.reverse() - |> Enum.filter(fn {version, _} -> - version in versions - end) - |> Enum.each(fn {version, mod} -> - Ecto.Migration.Runner.run( - repo, - [], - version, - mod, - :forward, - :down, - :down, - all: true - ) - - Ecto.Migration.SchemaMigration.down(repo, repo.config(), version, []) - end) - end) + dev_migrations + |> Enum.map(&extract_migration_info/1) + |> Enum.filter(& &1) + |> Enum.map(&load_migration!/1) + |> Enum.sort() + |> Enum.reverse() + |> Enum.each(fn {version, mod} -> Ecto.Migrator.down(repo, version, mod) end) end) # Remove dev migration files @@ -853,7 +827,7 @@ defmodule AshSqlite.MigrationGenerator do repo |> Module.split() |> List.last() |> Macro.underscore() end - defp migration({up, down}, repo, opts) do + defp migration({up, down}, repo, rebuilds?, opts) do migration_path = migration_path(opts, repo) require_name!(opts) @@ -901,7 +875,7 @@ defmodule AshSqlite.MigrationGenerator do This file was autogenerated with `mix ash_sqlite.generate_migrations` \"\"\" - use Ecto.Migration + #{if rebuilds?, do: "use AshSqlite.Migration", else: "use Ecto.Migration"} def up do #{up} @@ -1009,6 +983,8 @@ defmodule AshSqlite.MigrationGenerator do {up, down} end + defp rebuilds?(phases), do: Enum.any?(phases, &match?(%Operation.RebuildTable{}, &1)) + defp maybe_comment(text, %{commented?: true}) do text |> String.split("\n") @@ -1711,8 +1687,110 @@ defmodule AshSqlite.MigrationGenerator do |> Enum.concat() |> Enum.map(&Map.put(&1, :multitenancy, snapshot.multitenancy)) |> Enum.map(&Map.put(&1, :old_multitenancy, old_snapshot.multitenancy)) + |> add_strict_change(snapshot, old_snapshot, opts) + |> rebuild_if_needed(snapshot, old_snapshot, opts) end + defp rebuild_tables?(opts, repo) do + rebuild_tables = rebuild_requested?(opts, repo) + + if rebuild_tables and !AshSqlite.Migration.rebuild_supported?() do + raise "rebuilding tables needs an `ecto_sql` that runs the `before_transaction/1` and " <> + "`after_transaction/1` migration callbacks, and the one installed does not. " <> + "Turn `rebuild_tables` off in the repo's config, or pass `--no-rebuild-tables`." + end + + rebuild_tables + end + + # `--rebuild-tables` (or `--no-rebuild-tables`) for this run, else the repo's `rebuild_tables`. + defp rebuild_requested?(%{rebuild_tables: nil}, repo), + do: repo.config()[:rebuild_tables] == true + + defp rebuild_requested?(%{rebuild_tables: rebuild_tables}, _repo), do: rebuild_tables + + defp add_strict_change(operations, snapshot, old_snapshot, opts) do + from = !!old_snapshot[:strict?] + + if !old_snapshot[:empty?] and from != snapshot.strict? do + change = %Operation.AlterStrict{table: snapshot.table, from: from, to: snapshot.strict?} + + if rebuild_tables?(opts, snapshot.repo), + do: [change | operations], + else: [%Operation.Omitted{operation: change, table: snapshot.table} | operations] + else + operations + end + end + + defp rebuild_if_needed(operations, snapshot, old_snapshot, opts) do + if !old_snapshot[:empty?] and + Enum.any?(operations, &Operation.RebuildTable.requires_rebuild?/1) do + {omitted, operations} = Enum.split_with(operations, &match?(%Operation.Omitted{}, &1)) + + if rebuild_tables?(opts, snapshot.repo) do + [ + %Operation.RebuildTable{ + table: snapshot.table, + old_snapshot: old_snapshot, + snapshot: snapshot, + changes: operations + } + ] + else + hint_rebuild( + operations, + Enum.map(omitted, &Operation.Omitted.reason(&1, snapshot.multitenancy)), + snapshot, + opts + ) + + operations + end + else + operations + end + end + + defp hint_rebuild(operations, omitted_reasons, snapshot, opts) do + unless opts.quiet do + fails = + for {reason, blocks} <- Operation.RebuildTable.blocking_reasons(operations), + do: " - #{reason} (#{blocked_when(blocks)})" + + omitted_lines = for reason <- omitted_reasons, do: " - #{reason} (not in the migration)" + + advice = + if AshSqlite.Migration.rebuild_supported?(), + do: "To rebuild the table instead, #{rebuild_advice(opts)}\n", + else: "" + + warn(""" + SQLite cannot make these changes to `#{snapshot.table}` in place: + #{Enum.join(fails ++ omitted_lines, "\n")} + #{advice}\ + """) + end + end + + defp blocked_when(:run), do: "fails when the migration runs" + defp blocked_when(:rollback), do: "fails when rolled back" + + defp rebuild_advice(%{dry_run: true}), do: "run again with `--rebuild-tables`." + defp rebuild_advice(%{check: true}), do: "run again with `--rebuild-tables`." + + defp rebuild_advice(%{dev: true}), + do: "make the final run (`mix ash.codegen `) with `--rebuild-tables`." + + defp rebuild_advice(_opts) do + "undo this run (roll the migrations back if you ran them, editing any `down` that " <> + "raises, then delete them and the snapshots they wrote) and generate again with " <> + "`--rebuild-tables`, or with `rebuild_tables: true` in the repo's config." + end + + # yellow, to stand out in a `--dry-run` + defp warn(message), do: Mix.shell().info([:yellow, message]) + defp indexes_match?(table, left, right) do left = left @@ -1792,18 +1870,48 @@ defmodule AshSqlite.MigrationGenerator do {attributes_to_add, attributes_to_remove, attributes_to_rename} = resolve_renames(snapshot.table, attributes_to_add, attributes_to_remove, opts) + old_names = Map.new(attributes_to_rename, fn {new, old} -> {new.source, old.source} end) + attributes_to_alter = snapshot.attributes |> Enum.map(fn attribute -> - {attribute, - Enum.find( - old_snapshot.attributes, - &(&1.source == attribute.source && - attributes_unequal?(&1, attribute, snapshot.repo, old_snapshot, snapshot)) - )} + old_name = Map.get(old_names, attribute.source, attribute.source) + old_attribute = Enum.find(old_snapshot.attributes, &(&1.source == old_name)) + + changed? = + old_attribute && + attributes_unequal?( + old_attribute, + attribute, + snapshot.repo, + old_snapshot, + snapshot, + old_name != attribute.source + ) + + {attribute, changed? && old_attribute} end) |> Enum.filter(&elem(&1, 1)) + # Only a rebuild can make the change of a renamed attribute, SQLite refuses the statement. + {renamed_and_changed, attributes_to_alter} = + if rebuild_tables?(opts, snapshot.repo) do + {[], attributes_to_alter} + else + Enum.split_with(attributes_to_alter, fn {new, old} -> new.source != old.source end) + end + + omitted = + for {new, old} <- renamed_and_changed, + do: %Operation.Omitted{ + table: snapshot.table, + operation: %Operation.AlterAttribute{ + table: snapshot.table, + new_attribute: new, + old_attribute: old + } + } + rename_attribute_events = Enum.map(attributes_to_rename, fn {new, old} -> %Operation.RenameAttribute{ @@ -1925,7 +2033,7 @@ defmodule AshSqlite.MigrationGenerator do end) add_attribute_events ++ - alter_attribute_events ++ remove_attribute_events ++ rename_attribute_events + alter_attribute_events ++ remove_attribute_events ++ rename_attribute_events ++ omitted end defp differently_deferrable?(%{references: %{deferrable: left}}, %{ @@ -1950,15 +2058,18 @@ defmodule AshSqlite.MigrationGenerator do # This exists to handle the fact that the remapping of the key name -> source caused attributes # to be considered unequal. We ignore things that only differ in that way using this function. - defp attributes_unequal?(left, right, repo, _old_snapshot, _new_snapshot) do - left = clean_for_equality(left, repo) - - right = clean_for_equality(right, repo) - - left != right + defp attributes_unequal?( + left, + right, + _repo, + _old_snapshot, + _new_snapshot, + renaming? + ) do + clean_for_equality(left, renaming?) != clean_for_equality(right, renaming?) end - defp clean_for_equality(attribute, _repo) do + defp clean_for_equality(attribute, ignore_names?) do cond do attribute[:source] -> Map.put(attribute, :name, attribute[:source]) @@ -1986,6 +2097,21 @@ defmodule AshSqlite.MigrationGenerator do attribute -> attribute end) + |> without_names(ignore_names?) + end + + defp without_names(attribute, false), do: attribute + + defp without_names(attribute, true) do + attribute = Map.drop(attribute, [:source, :name]) + + case attribute do + %{references: %{} = references} -> + %{attribute | references: Map.delete(references, :name)} + + attribute -> + attribute + end end defp add_ignore(%{references: references} = attribute) when is_map(references) do @@ -2158,7 +2284,7 @@ defmodule AshSqlite.MigrationGenerator do defp pad(i) when i < 10, do: <> defp pad(i), do: to_string(i) - def get_snapshots(resource, all_resources) do + def get_snapshots(resource, all_resources, opts \\ %__MODULE__{}) do Code.ensure_compiled!(AshSqlite.DataLayer.Info.repo(resource)) if AshSqlite.DataLayer.Info.polymorphic?(resource) do @@ -2170,7 +2296,7 @@ defmodule AshSqlite.MigrationGenerator do |> Enum.uniq() |> Enum.map(fn relationship -> resource - |> do_snapshot(relationship.context[:data_layer][:table]) + |> do_snapshot(relationship.context[:data_layer][:table], opts) |> Map.update!(:identities, fn identities -> identity_index_names = AshSqlite.DataLayer.Info.identity_index_names(resource) @@ -2200,7 +2326,8 @@ defmodule AshSqlite.MigrationGenerator do default( source_attribute, relationship.destination, - AshSqlite.DataLayer.Info.repo(relationship.destination) + AshSqlite.DataLayer.Info.repo(relationship.destination), + opts ), deferrable: false, match_tenant?: false, @@ -2221,13 +2348,13 @@ defmodule AshSqlite.MigrationGenerator do end) end) else - [do_snapshot(resource, AshSqlite.DataLayer.Info.table(resource))] + [do_snapshot(resource, AshSqlite.DataLayer.Info.table(resource), opts)] end end - defp do_snapshot(resource, table) do + defp do_snapshot(resource, table, opts) do snapshot = %{ - attributes: attributes(resource, table), + attributes: attributes(resource, table, opts), identities: identities(resource), table: table || AshSqlite.DataLayer.Info.table(resource), custom_indexes: custom_indexes(resource), @@ -2281,7 +2408,7 @@ defmodule AshSqlite.MigrationGenerator do } end - defp attributes(resource, table) do + defp attributes(resource, table, opts) do repo = AshSqlite.DataLayer.Info.repo(resource) ignored = AshSqlite.DataLayer.Info.migration_ignore_attributes(resource) || [] @@ -2301,7 +2428,7 @@ defmodule AshSqlite.MigrationGenerator do ]) ) |> Enum.map(fn attribute -> - default = default(attribute, resource, repo) + default = default(attribute, resource, repo, opts) type = AshSqlite.DataLayer.Info.migration_types(resource)[attribute.name] || @@ -2613,23 +2740,24 @@ defmodule AshSqlite.MigrationGenerator do |> Enum.map(&Map.put(&1, :base_filter, AshSqlite.DataLayer.Info.base_filter_sql(resource))) end - defp default(%{name: name, default: default}, resource, _repo) when is_function(default) do + defp default(%{name: name, default: default}, resource, _repo, _opts) + when is_function(default) do configured_default(resource, name) || "nil" end - defp default(%{name: name, default: {_, _, _}}, resource, _), + defp default(%{name: name, default: {_, _, _}}, resource, _, _), do: configured_default(resource, name) || "nil" - defp default(%{name: name, default: nil}, resource, _), + defp default(%{name: name, default: nil}, resource, _, _), do: configured_default(resource, name) || "nil" - defp default(%{name: name, default: []}, resource, _), + defp default(%{name: name, default: []}, resource, _, _), do: configured_default(resource, name) || "[]" - defp default(%{name: name, default: default}, resource, _) when default == %{}, + defp default(%{name: name, default: default}, resource, _, _) when default == %{}, do: configured_default(resource, name) || "%{}" - defp default(%{name: name, default: value, type: type} = attr, resource, _) do + defp default(%{name: name, default: value, type: type} = attr, resource, repo, opts) do case configured_default(resource, name) do nil -> case migration_default(type, Map.get(attr, :constraints, []), value) do @@ -2637,7 +2765,7 @@ defmodule AshSqlite.MigrationGenerator do default :error -> - "nil" + literal_default(value, repo, opts) end default -> @@ -2645,6 +2773,34 @@ defmodule AshSqlite.MigrationGenerator do end end + defp literal_default(value, repo, opts) do + if rebuild_tables?(opts, repo), + do: literal_default(value), + else: "nil" + end + + defp literal_default(value) when is_number(value), do: to_string(value) + defp literal_default(value) when is_boolean(value), do: to_string(value) + defp literal_default(value) when is_atom(value), do: inspect(to_string(value)) + defp literal_default(value) when is_binary(value), do: inspect(value) + defp literal_default(%Decimal{} = value), do: inspect(to_string(value)) + + defp literal_default(%module{} = value) when module in [Date, Time, DateTime, NaiveDateTime], + do: ~s[fragment("'#{module.to_iso8601(value)}'")] + + defp literal_default(value) do + Logger.warning(""" + You have specified a default value that cannot be explicitly converted to a column default: + + `#{inspect(value)}` + + The default value in the migration will be set to `nil`. To set one, use + `migration_defaults` in the sqlite section of the resource. + """) + + "nil" + end + defp migration_default(type, constraints, value) do type = type diff --git a/lib/migration_generator/operation.ex b/lib/migration_generator/operation.ex index bef9ecf..d1b2b04 100644 --- a/lib/migration_generator/operation.ex +++ b/lib/migration_generator/operation.ex @@ -820,4 +820,391 @@ defmodule AshSqlite.MigrationGenerator.Operation do end end end + + defmodule AlterStrict do + @moduledoc false + # The table became STRICT, or stopped being. There is no statement for that: it takes a + # rebuild. + defstruct [:table, :from, :to] + + def reason(%{table: table, to: true}), do: "making `#{table}` a STRICT table" + def reason(%{table: table, to: false}), do: "`#{table}` is no longer a STRICT table" + end + + defmodule Omitted do + @moduledoc false + # A change SQLite cannot make in place, when tables are not rebuilt: only the hint knows of it. + defstruct [:operation, :table, :multitenancy, :old_multitenancy] + + alias AshSqlite.MigrationGenerator.Operation + + def reason(%{operation: %Operation.AlterStrict{} = operation}, _multitenancy), + do: Operation.AlterStrict.reason(operation) + + def reason(%{operation: operation}, multitenancy) do + operation + |> Map.put(:multitenancy, multitenancy) + |> operation.__struct__.up() + |> String.trim() + end + end + + defmodule RebuildTable do + @moduledoc false + # The operations of a table when one of them cannot be done in place (`requires_rebuild?/1`): + # a `rebuild_table` of the table's new shape, followed by its indexes. + defstruct [:table, :old_snapshot, :snapshot, :changes, no_phase: true] + + import Helper + + alias AshSqlite.MigrationGenerator.Operation + + def requires_rebuild?(%Operation.AlterAttribute{}), do: true + def requires_rebuild?(%Operation.DropForeignKey{}), do: true + def requires_rebuild?(%Operation.RemovePrimaryKey{}), do: true + def requires_rebuild?(%Operation.Omitted{}), do: true + def requires_rebuild?(%Operation.AlterStrict{}), do: true + + def requires_rebuild?(%Operation.AddAttribute{attribute: attribute}), + do: attribute.allow_nil? == false and attribute.default == "nil" + + def requires_rebuild?(_), do: false + + def up(%{old_snapshot: old, snapshot: new, changes: changes}) do + renames = renames(changes) + leftover = leftover_columns(old, new, renames, changes) + + render(%{ + table: new.table, + from: old, + from_columns: old.attributes, + to: new, + to_columns: new.attributes ++ Enum.map(leftover, &as_nullable/1), + renames: renames, + direction: :up, + reasons: reasons(changes), + notes: type_notes(old, new, changes) ++ leftover_notes(leftover) ++ statement_notes(old) + }) + end + + def down(%{old_snapshot: old, snapshot: new, changes: changes}) do + up_renames = renames(changes) + leftover = leftover_columns(old, new, up_renames, changes) + + render(%{ + table: new.table, + from: new, + from_columns: new.attributes ++ Enum.map(leftover, &as_nullable/1), + to: old, + to_columns: old.attributes, + renames: Map.new(up_renames, fn {to, from} -> {from, to} end), + direction: :down, + reasons: ["undoing the rebuild above (rows in columns it added are not kept)"], + notes: statement_notes(new) + }) + end + + defp render(%{table: table} = plan) do + columns = plan.to_columns + + lines = plan.reasons ++ plan.notes ++ review_notes(columns, plan) + + options = + [ + copy_option(columns, plan), + if(plan.to.strict?, do: ~s|options: "STRICT"|) + ] + |> Enum.reject(&is_nil/1) + |> Enum.map_join("", &(", " <> &1)) + + adds = + Enum.map_join(columns, "\n", fn attribute -> + Operation.AddAttribute.up(%Operation.AddAttribute{ + attribute: attribute, + table: table, + multitenancy: plan.to.multitenancy, + old_multitenancy: plan.from.multitenancy + }) + end) + + [ + header(table, lines), + Enum.map(plan.from.custom_statements, fn statement -> + Operation.AddCustomStatement.down(%{statement: statement, table: table}) + end), + "rebuild_table :#{as_atom(table)}#{options} do\n#{adds}\nend", + Enum.map(plan.to.identities, fn identity -> + Operation.AddUniqueIndex.up(%{ + identity: identity, + table: table, + multitenancy: plan.to.multitenancy + }) + end), + Enum.map(plan.to.custom_indexes, fn index -> + Operation.AddCustomIndex.up(%{ + index: index, + table: table, + base_filter: plan.to.base_filter, + multitenancy: plan.to.multitenancy + }) + end), + Enum.map(plan.to.custom_statements, fn statement -> + Operation.AddCustomStatement.up(%{statement: statement, table: table}) + end) + ] + |> List.flatten() + |> Enum.join("\n") + end + + defp header(table, lines) do + Enum.join( + [ + "# SQLite cannot change `#{table}` in place, so it is rebuilt from the resource's", + "# snapshot. Anything on the table that the resource does not describe (a column,", + "# index or trigger added by hand) is not kept. Changes:" + | Enum.flat_map(lines, &entry_lines/1) + ], + "\n" + ) + end + + defp entry_lines(entry) do + case String.split(entry, "\n") do + [first | rest] -> ["# - #{first}" | Enum.map(rest, &"# #{&1}")] + end + end + + # `copy: [subject: :title, name: "COALESCE(name, 'x')"]`: only what differs from copying + # the column of the same name; nil when nothing does. + defp copy_option(columns, plan) do + entries = + for column <- columns, + from_name = Map.get(plan.renames, column.source, column.source), + from_column = Enum.find(plan.from_columns, &(&1.source == from_name)), + entry = copy_entry(column, from_name, from_column), + do: entry + + if entries != [], do: "copy: [#{Enum.join(entries, ", ")}]" + end + + defp copy_entry(column, from_name, from_column) do + cond do + expression = backfill(from_column, column, quote_name(from_name)) -> + "#{atom_key(column.source)} #{inspect(expression)}" + + from_name == column.source -> + nil + + true -> + "#{atom_key(column.source)} :#{as_atom(from_name)}" + end + end + + defp atom_key(name), do: Macro.inspect_atom(:key, atom(name)) + + defp atom(name) when is_atom(name), do: name + # sobelow_skip ["DOS.StringToAtom"] + defp atom(name), do: String.to_atom(name) + + defp backfill(%{allow_nil?: true}, %{allow_nil?: false} = column, expression) do + case default_sql(column.default) do + nil -> nil + default -> "COALESCE(#{expression}, #{default})" + end + end + + defp backfill(_from, _to, _expression), do: nil + + defp review_notes(_columns, %{direction: :down}), do: [] + + defp review_notes(columns, plan) do + for column <- columns, + note = review_note(column, plan) do + note + end + end + + defp review_note(%{allow_nil?: false} = column, plan) do + from_name = Map.get(plan.renames, column.source, column.source) + + case Enum.find(plan.from_columns, &(&1.source == from_name)) do + nil -> + if unfilled?(column), + do: review_new_column(plan.table, column.source) + + %{allow_nil?: true} -> + if default_sql(column.default) == nil, + do: review_required_column(plan.table, column.source, from_name) + + _ -> + nil + end + end + + defp review_note(_column, _plan), do: nil + + defp unfilled?(attribute), do: attribute.default == "nil" and !attribute.generated? + + defp review_required_column(table, name, from) do + replaces = if from != name, do: " (in place of its `#{atom_key(name)} #{inspect(from)}`)" + + """ + REVIEW: #{inspect(name)} becomes required and has no default, so a row without a value for it + stops this migration (nothing is changed). Give those rows one with the `copy:` option + of `rebuild_table`#{replaces}, for example + rebuild_table :#{as_atom(table)}, copy: [#{atom_key(name)} "COALESCE(#{from}, 'your value')"] do + To give new rows one too, set a `default` on the attribute (or use `migration_defaults`) + and regenerate.\ + """ + end + + defp review_new_column(table, name) do + """ + REVIEW: #{inspect(name)} is new, required and has no default, so if the table has rows this + migration stops (nothing is changed). Give the existing rows a value with the `copy:` + option of `rebuild_table`, for example + rebuild_table :#{as_atom(table)}, copy: [#{atom_key(name)} "'your value'"] do + or set a `default` on the attribute (or use `migration_defaults`) and regenerate.\ + """ + end + + defp renames(changes) do + for %Operation.RenameAttribute{old_attribute: old, new_attribute: new} <- changes, + into: %{}, + do: {new.source, old.source} + end + + # The columns of the old table that the resource no longer describes. They stay, as nullable + # columns, unless the author asked for them to be dropped (`--drop-columns`). + defp leftover_columns(old, new, renames, changes) do + new_names = MapSet.new(new.attributes, & &1.source) + renamed_away = MapSet.new(Map.values(renames)) + + dropped = + for %Operation.RemoveAttribute{attribute: attribute, commented?: false} <- changes, + into: MapSet.new(), + do: attribute.source + + Enum.reject(old.attributes, fn attribute -> + MapSet.member?(new_names, attribute.source) or + MapSet.member?(renamed_away, attribute.source) or + MapSet.member?(dropped, attribute.source) + end) + end + + defp type_notes(_old, %{strict?: true}, _changes), do: [] + + defp type_notes(old, _new, changes) do + for %Operation.AlterAttribute{old_attribute: from, new_attribute: to} <- changes, + from.type != to.type, + Enum.any?(old.attributes, &(&1.source == from.source)) do + "REVIEW: `#{to.source}` changes type in a table that is not STRICT, so values that " <> + "do not convert are copied as they are, not rejected" + end + end + + defp statement_notes(%{custom_statements: []}), do: [] + + defp statement_notes(%{custom_statements: statements}) do + names = Enum.map_join(statements, ", ", &inspect(&1.name)) + + [ + "REVIEW: the custom statements #{names} are dropped (their `down`) before and " <> + "recreated (their `up`) after the rebuild.\n" <> + "What they hold, such as the rows of a table or the contents of a full-text index, " <> + "is not kept: to keep it, copy it before and restore it after, in this migration" + ] + end + + defp leftover_notes([]), do: [] + + defp leftover_notes(leftover) do + names = Enum.map_join(leftover, ", ", &inspect(&1.source)) + + [ + "#{names} no longer in the resource, kept as nullable so no data is lost " <> + "(to drop one, delete its line from the `rebuild_table` below)" + ] + end + + defp as_nullable(attribute) do + %{attribute | allow_nil?: true, default: "nil", primary_key?: false, references: nil} + end + + def reasons(changes), do: changes |> blocking_reasons() |> Enum.map(&elem(&1, 0)) + + def blocking_reasons(changes) do + for change <- changes, + requires_rebuild?(change), + !covered_by_other_change?(change, changes), + reason = reason(change), + uniq: true, + do: {reason, blocks(change)} + end + + defp blocks(%Operation.DropForeignKey{direction: :down}), do: :rollback + defp blocks(_change), do: :run + + # Changing a foreign key is two operations, dropping it and altering the column. One line + # says it: the drop when the column no longer has a foreign key (and the alteration is only + # that), the alteration when it has another one. + defp covered_by_other_change?( + %Operation.DropForeignKey{attribute: %{source: source}}, + changes + ) do + Enum.any?( + changes, + &match?(%Operation.AlterAttribute{new_attribute: %{source: ^source, references: %{}}}, &1) + ) + end + + defp covered_by_other_change?( + %Operation.AlterAttribute{old_attribute: old, new_attribute: %{references: nil} = new}, + changes + ) do + Enum.any?( + changes, + &match?( + %Operation.DropForeignKey{attribute: %{source: source}} when source == new.source, + &1 + ) + ) and %{old | references: nil} == new + end + + defp covered_by_other_change?(_change, _changes), do: false + + defp reason(%Operation.AlterAttribute{} = change), + do: change |> Operation.AlterAttribute.up() |> String.trim() + + defp reason(%Operation.AddAttribute{attribute: attribute}) do + "add #{inspect(attribute.source)} as NOT NULL without a default, " <> + "which SQLite refuses on a table with rows" + end + + defp reason(%Operation.AlterStrict{} = change), do: Operation.AlterStrict.reason(change) + + defp reason(%Operation.DropForeignKey{direction: :down, attribute: %{references: reference}}), + do: "dropping the foreign key #{reference.name} when rolling back" + + defp reason(%Operation.DropForeignKey{attribute: %{references: reference}}), + do: "dropping the foreign key #{reference.name}" + + defp reason(%Operation.RemovePrimaryKey{}), do: "changing the primary key" + + defp quote_name(name), do: ~s("#{name}") + + defp default_sql(nil), do: nil + defp default_sql("nil"), do: nil + + defp default_sql(code) when is_binary(code) do + case Code.string_to_quoted(code) do + {:ok, value} when is_binary(value) -> "'" <> String.replace(value, "'", "''") <> "'" + {:ok, value} when is_number(value) -> to_string(value) + {:ok, true} -> "1" + {:ok, false} -> "0" + {:ok, {:fragment, _, [sql]}} when is_binary(sql) -> sql + _ -> nil + end + end + end end diff --git a/lib/mix/tasks/ash_sqlite.generate_migrations.ex b/lib/mix/tasks/ash_sqlite.generate_migrations.ex index 1213209..d87efb5 100644 --- a/lib/mix/tasks/ash_sqlite.generate_migrations.ex +++ b/lib/mix/tasks/ash_sqlite.generate_migrations.ex @@ -13,6 +13,10 @@ defmodule Mix.Tasks.AshSqlite.GenerateMigrations do * `migration-path` - a custom path to store the migrations, defaults to "priv". Migrations are stored in a folder for each repo, so `priv/repo_name/migrations` * `drop-columns` - whether or not to drop columns as attributes are removed. See below for more + * `rebuild-tables` - rebuild a table when SQLite cannot make a change in place. Can be set for + every run with `rebuild_tables: true` in the repo's config, and turned off for one run with + `--no-rebuild-tables`. It needs an `ecto_sql` with the `before_transaction/1` and + `after_transaction/1` migration callbacks. See "Rebuilding tables" below * `name` - names the generated migrations, prepending with the timestamp. The default is `migrate_resources_`, where `` is the count of migrations matching `*migrate_resources*` plus one. @@ -46,6 +50,18 @@ defmodule Mix.Tasks.AshSqlite.GenerateMigrations do to tell it to uncomment those statements. Additionally, you can just uncomment that code on a case by case basis. + #### Rebuilding tables + + SQLite and its Ecto adapters cannot change a column's type, default or nullability, or a foreign key, + in place. By default the generator writes a statement that fails when the migration runs, or leaves + the change out (a renamed attribute that also changes, a table that becomes `STRICT`), and prints a + hint saying so. With `--rebuild-tables` it writes a `rebuild_table` instead, which creates the table + again in its new shape and copies the rows. See the "Migrations" guide. + + The `default:` of an attribute that is a plain value (a number, string, boolean, atom, decimal, + date or time) then also becomes the default of its column. Set `rebuild_tables: true` in the + repo's config rather than passing the flag, so that every run agrees. See the "Migrations" guide. + #### Conflicts/Multiple Resources It will raise on conflicts that it can't resolve, like the same field with different @@ -117,7 +133,8 @@ defmodule Mix.Tasks.AshSqlite.GenerateMigrations do check: :boolean, dev: :boolean, auto_name: :boolean, - drop_columns: :boolean + drop_columns: :boolean, + rebuild_tables: :boolean ] ) diff --git a/lib/repo.ex b/lib/repo.ex index b067efd..d783974 100644 --- a/lib/repo.ex +++ b/lib/repo.ex @@ -20,6 +20,11 @@ defmodule AshSqlite.Repo do - `:tenant_migrations_path` - The path where your tenant migrations are stored (only relevant for a multitenant implementation) - `:snapshots_path` - The path where the resource snapshots for the migration generator are stored. + - `:rebuild_tables` - Set to `true` to rebuild tables SQLite cannot change in place, as if + `--rebuild-tables` were passed to every run (`--no-rebuild-tables` turns it off for one run). + Prefer it to the flag: the defaults it applies are kept in the snapshots, so every run has to agree. + It needs an `ecto_sql` with the `before_transaction/1` and `after_transaction/1` migration + callbacks, and raises without them. """ @doc "Use this to inform the data layer about what extensions are installed" diff --git a/mix.exs b/mix.exs index 120b48c..3a928d2 100644 --- a/mix.exs +++ b/mix.exs @@ -123,7 +123,8 @@ defmodule AshSqlite.MixProject do AshSqlite.TenantRepo ], Utilities: [ - AshSqlite.ManualRelationship + AshSqlite.ManualRelationship, + AshSqlite.Migration ], Introspection: [ AshSqlite.DataLayer.Info, @@ -150,10 +151,10 @@ defmodule AshSqlite.MixProject do # Run "mix help deps" to learn about dependencies. defp deps do [ - {:ecto_sql, "~> 3.13"}, + {:ecto_sql, ecto_sql_version("~> 3.13")}, {:ecto_sqlite3, "~> 0.12"}, {:ecto_libsql, "~> 0.9", optional: true}, - {:ecto, "~> 3.13"}, + {:ecto, "~> 3.13", override: System.get_env("ECTO_SQL_VERSION") == "main"}, {:jason, "~> 1.0"}, {:ash, ash_version("~> 3.33 and >= 3.33.2")}, {:ash_sql, ash_sql_version("~> 0.2 and >= 0.6.9")}, @@ -169,6 +170,21 @@ defmodule AshSqlite.MixProject do ] end + # `main` has migration callbacks that are not released yet. It asks for `ecto` from git, which + # the `ecto` entry above overrides to stay on Hex. + defp ecto_sql_version(default_version) do + case System.get_env("ECTO_SQL_VERSION") do + nil -> + default_version + + "main" -> + [git: "https://github.com/elixir-ecto/ecto_sql.git", override: true] + + version -> + "~> #{version}" + end + end + defp ash_version(default_version) do case System.get_env("ASH_VERSION") do nil -> diff --git a/test/migration_generator_test.exs b/test/migration_generator_test.exs index 0f131da..be6b05f 100644 --- a/test/migration_generator_test.exs +++ b/test/migration_generator_test.exs @@ -9,6 +9,9 @@ defmodule AshSqlite.MigrationGeneratorTest do import ExUnit.CaptureLog + import AshSqlite.RebuildHelper, + only: [generate: 2, generate: 3, last_migration: 1, migrations: 1] + setup %{tmp_dir: tmp_dir} do current_shell = Mix.shell() :ok = Mix.shell(Mix.Shell.Process) @@ -1596,4 +1599,558 @@ defmodule AshSqlite.MigrationGeneratorTest do assert File.read!(file2) =~ ~S[rename table(:posts), :contributer2_id, to: :contributer_id] end end + + describe "rebuilding tables" do + @describetag :rebuild + + defmacrop defitem(do: body) do + quote do + defresource Item, "items" do + unquote(body) + end + end + end + + defmacrop defparent_and_child do + quote do + defresource Parent, "parents" do + attributes do + uuid_primary_key(:id) + end + end + + defresource Child, "children" do + attributes do + uuid_primary_key(:id) + end + + relationships do + belongs_to(:parent, Parent) + end + end + end + end + + # `name` was required and is now optional, which SQLite cannot do in place + defp name_required_then_optional(ctx) do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + end + end + + defdomain([Item]) + generate(Domain, ctx, rebuild_tables: nil) + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + end + + # the alias made by `defdomain` only lives in this function + Domain + end + + test "without the option, a change SQLite cannot make is written as it always was", ctx do + domain = name_required_then_optional(ctx) + generate(domain, ctx, rebuild_tables: nil, quiet: false) + + migration = last_migration(ctx) + assert migration =~ "modify :name, :text, null: true" + refute migration =~ "rebuild_table" + refute migration =~ "AshSqlite.Migration" + + assert_received {:mix_shell, :info, + ["SQLite cannot make these changes to `items`" <> _ = hint]} + + assert hint =~ " - modify :name, :text, null: true (fails when the migration runs)" + assert hint =~ "--rebuild-tables" + assert hint =~ "rebuild_tables: true" + end + + test "without the option, the hint's advice depends on the run", ctx do + domain = name_required_then_optional(ctx) + + generate(domain, ctx, rebuild_tables: nil, quiet: false, dry_run: true) + assert_received {:mix_shell, :info, ["SQLite cannot make" <> _ = hint]} + assert hint =~ "run again with `--rebuild-tables`." + + # the dev migration may have been run: the final run takes care of it + generate(domain, ctx, rebuild_tables: nil, quiet: false, dev: true) + assert_received {:mix_shell, :info, ["SQLite cannot make" <> _ = hint]} + assert hint =~ "make the final run" + + generate(domain, ctx, rebuild_tables: nil, quiet: false) + assert_received {:mix_shell, :info, ["SQLite cannot make" <> _ = hint]} + assert hint =~ "undo this run" + end + + test "without the option, a dropped foreign key still raises with guidance", ctx do + defparent_and_child() + defdomain([Parent, Child]) + generate(Domain, ctx, rebuild_tables: nil) + + defresource Child, "children" do + attributes do + uuid_primary_key(:id) + attribute(:parent_id, :uuid) + end + end + + defdomain([Parent, Child]) + generate(Domain, ctx, rebuild_tables: nil) + + migration = last_migration(ctx) + assert migration =~ "SQLite does not support dropping foreign key constraints." + refute migration =~ "rebuild_table" + end + + test "without the option, the hint says that a new required column is refused", ctx do + defitem do + attributes do + uuid_primary_key(:id) + end + end + + defdomain([Item]) + generate(Domain, ctx, rebuild_tables: nil) + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:code, :string, allow_nil?: false) + end + end + + generate(Domain, ctx, rebuild_tables: nil, quiet: false) + + assert_received {:mix_shell, :info, + ["SQLite cannot make these changes to `items`" <> _ = hint]} + + assert hint =~ " - add :code as NOT NULL without a default, which SQLite refuses" + end + + test "without the option, a new foreign key column says that rolling back is blocked", ctx do + defresource Owner, "owners" do + attributes do + uuid_primary_key(:id) + end + end + + defresource Pet, "pets" do + attributes do + uuid_primary_key(:id) + end + end + + defdomain([Owner, Pet]) + generate(Domain, ctx, rebuild_tables: nil) + + defresource Pet, "pets" do + attributes do + uuid_primary_key(:id) + end + + relationships do + belongs_to(:owner, Owner) + end + end + + generate(Domain, ctx, rebuild_tables: nil, quiet: false) + + # the way up works, as it always did + migration = last_migration(ctx) + assert migration =~ "add :owner_id, references(:owners" + refute migration =~ "rebuild_table" + + assert_received {:mix_shell, :info, + ["SQLite cannot make these changes to `pets`" <> _ = hint]} + + assert hint =~ + " - dropping the foreign key pets_owner_id_fkey when rolling back (fails when rolled back)" + + refute hint =~ "fails when the migration runs" + end + + test "with the option, a new foreign key column is rebuilt, so that it can be rolled back", + ctx do + defresource Owner, "owners" do + attributes do + uuid_primary_key(:id) + end + end + + defresource Pet, "pets" do + attributes do + uuid_primary_key(:id) + end + end + + defdomain([Owner, Pet]) + generate(Domain, ctx) + + defresource Pet, "pets" do + attributes do + uuid_primary_key(:id) + end + + relationships do + belongs_to(:owner, Owner) + end + end + + generate(Domain, ctx) + + [up, down] = String.split(last_migration(ctx), "def down do") + assert up =~ "# - dropping the foreign key pets_owner_id_fkey when rolling back" + assert up =~ "rebuild_table :pets" + assert down =~ "rebuild_table :pets" + end + + test "with the option, there is no hint and the table is rebuilt", ctx do + domain = name_required_then_optional(ctx) + generate(domain, ctx, rebuild_tables: true, quiet: false) + + refute_received {:mix_shell, :info, ["SQLite cannot make these changes" <> _]} + assert last_migration(ctx) =~ "rebuild_table :items" + end + + test "the copy is not written when every column is simply copied", ctx do + domain = name_required_then_optional(ctx) + generate(domain, ctx) + + refute last_migration(ctx) =~ "copy:" + refute last_migration(ctx) =~ "REVIEW" + end + + test "the copy lists only the rename", ctx do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:title, :string) + attribute(:body, :string) + end + end + + defdomain([Item]) + generate(Domain, ctx) + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:subject, :string) + attribute(:body, :string, allow_nil?: false) + end + end + + send(self(), {:mix_shell_input, :yes?, true}) + send(self(), {:mix_shell_input, :prompt, "subject"}) + generate(Domain, ctx) + assert last_migration(ctx) =~ "rebuild_table :items, copy: [subject: :title] do" + end + + test "a default fills the rows of a required column, so there is no REVIEW", ctx do + defitem do + sqlite do + migration_defaults(name: "\"unnamed\"") + end + + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + end + + defdomain([Item]) + generate(Domain, ctx) + + defitem do + sqlite do + migration_defaults(name: "\"unnamed\"") + end + + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + end + end + + generate(Domain, ctx) + refute last_migration(ctx) =~ "REVIEW" + assert last_migration(ctx) =~ "COALESCE" + end + + test "the comment says once that a foreign key is dropped", ctx do + defparent_and_child() + defdomain([Parent, Child]) + generate(Domain, ctx) + + defresource Child, "children" do + attributes do + uuid_primary_key(:id) + attribute(:parent_id, :uuid) + end + end + + defdomain([Parent, Child]) + generate(Domain, ctx) + + [up, _down] = String.split(last_migration(ctx), "def down do") + assert up =~ "# - dropping the foreign key children_parent_id_fkey" + refute up =~ "modify :parent_id" + end + + test "the comment says once that a foreign key is changed", ctx do + defparent_and_child() + defdomain([Parent, Child]) + generate(Domain, ctx) + + defresource Child, "children" do + sqlite do + references do + reference(:parent, on_delete: :delete) + end + end + + attributes do + uuid_primary_key(:id) + end + + relationships do + belongs_to(:parent, Parent) + end + end + + defdomain([Parent, Child]) + generate(Domain, ctx) + + [up, _down] = String.split(last_migration(ctx), "def down do") + assert up =~ "# - modify :parent_id, references(:parents" + refute up =~ "dropping the foreign key" + end + + test "a plain rename of a foreign key column stays a plain rename", ctx do + defresource Parent, "parents" do + attributes do + uuid_primary_key(:id) + end + end + + defresource Child, "children" do + attributes do + uuid_primary_key(:id) + end + + relationships do + belongs_to(:parent, Parent, source_attribute: :owner_id) + end + end + + defdomain([Parent, Child]) + generate(Domain, ctx) + + defresource Child, "children" do + attributes do + uuid_primary_key(:id) + end + + relationships do + belongs_to(:parent, Parent, source_attribute: :guardian_id) + end + end + + send(self(), {:mix_shell_input, :yes?, true}) + send(self(), {:mix_shell_input, :prompt, "guardian_id"}) + generate(Domain, ctx) + migration = last_migration(ctx) + refute migration =~ "use AshSqlite.Migration" + assert migration =~ "rename table(:children), :owner_id, to: :guardian_id" + end + + test "without the option, a rename together with a change is only a rename", ctx do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:title, :string) + end + end + + defdomain([Item]) + generate(Domain, ctx, rebuild_tables: nil) + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:subject, :string, allow_nil?: false) + end + end + + send(self(), {:mix_shell_input, :yes?, true}) + generate(Domain, ctx, rebuild_tables: nil, quiet: false) + migration = last_migration(ctx) + + assert migration =~ "rename table(:items), :title, to: :subject" + refute migration =~ "rebuild_table" + refute migration =~ "modify" + + # the change that is left out is in the hint + assert_received {:mix_shell, :info, + ["SQLite cannot make these changes to `items`" <> _ = hint]} + + assert hint =~ " - modify :subject, :text, null: false (not in the migration)" + end + + test "an attribute's default is not a column default when rebuilding is off", ctx do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:count, :integer, default: 0) + end + end + + defdomain([Item]) + generate(Domain, ctx, rebuild_tables: nil) + + refute last_migration(ctx) =~ "default:" + end + + test "a value that cannot be a column default is nil, with a warning", ctx do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:tags, {:array, :string}, default: ["a"]) + end + end + + defdomain([Item]) + log = capture_log(fn -> generate(Domain, ctx) end) + + assert log =~ "cannot be explicitly converted to a column default" + assert log =~ ~S|["a"]| + assert last_migration(ctx) =~ "add :tags, {:array, :text}" + end + + test "a migration_defaults entry still wins over an attribute's default", ctx do + defitem do + sqlite do + migration_defaults(title: "\"from migration_defaults\"") + end + + attributes do + uuid_primary_key(:id) + attribute(:title, :string, default: "from the attribute") + end + end + + defdomain([Item]) + generate(Domain, ctx) + assert last_migration(ctx) =~ ~S|default: "from migration_defaults"| + end + + test "generating again after the defaults were applied finds nothing to do", ctx do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:count, :integer, default: 3) + end + end + + defdomain([Item]) + generate(Domain, ctx) + assert last_migration(ctx) =~ "default: 3" + + generate(Domain, ctx) + assert length(migrations(ctx)) == 1 + end + + test "a snapshot from before there was strict? is a table that is not", ctx do + defitem do + sqlite do + strict?(false) + end + + attributes do + uuid_primary_key(:id) + end + end + + defdomain([Item]) + generate(Domain, ctx) + + for file <- Path.wildcard(Path.join(ctx.snapshot_path, "**/*.json")) do + snapshot = file |> File.read!() |> Jason.decode!() |> Map.delete("strict?") + File.write!(file, Jason.encode!(snapshot)) + end + + generate(Domain, ctx) + assert length(migrations(ctx)) == 1 + end + + test "without the option, a table that becomes STRICT is only in the hint", ctx do + defitem do + sqlite do + strict?(false) + end + + attributes do + uuid_primary_key(:id) + end + end + + defdomain([Item]) + generate(Domain, ctx, rebuild_tables: nil) + + defitem do + sqlite do + strict?(true) + end + + attributes do + uuid_primary_key(:id) + end + end + + generate(Domain, ctx, rebuild_tables: nil, quiet: false) + + assert length(migrations(ctx)) == 1 + + assert_received {:mix_shell, :info, + ["SQLite cannot make these changes to `items`" <> _ = hint]} + + assert hint =~ "making `items` a STRICT table" + assert hint =~ "(not in the migration)" + end + end + + describe "without the migration callbacks of ecto_sql" do + @describetag :no_rebuild_support + + test "asking for rebuilds raises", ctx do + defitem do + attributes do + uuid_primary_key(:id) + end + end + + defdomain([Item]) + + assert_raise RuntimeError, ~r/needs an `ecto_sql` that runs/, fn -> + generate(Domain, ctx, rebuild_tables: true) + end + end + + test "the hint lists what SQLite cannot do and does not suggest a rebuild", ctx do + domain = name_required_then_optional(ctx) + generate(domain, ctx, rebuild_tables: nil, quiet: false) + + assert_received {:mix_shell, :info, + ["SQLite cannot make these changes to `items`" <> _ = hint]} + + assert hint =~ " - modify :name, :text, null: true (fails when the migration runs)" + refute hint =~ "rebuild" + end + end end diff --git a/test/migration_test.exs b/test/migration_test.exs new file mode 100644 index 0000000..7290e05 --- /dev/null +++ b/test/migration_test.exs @@ -0,0 +1,321 @@ +# SPDX-FileCopyrightText: 2023 ash_sqlite contributors +# +# SPDX-License-Identifier: MIT + +defmodule AshSqlite.MigrationTest do + @moduledoc """ + Runs migrations that use `AshSqlite.Migration` on a real SQLite file. + """ + use ExUnit.Case, async: false + @moduletag :migration + @moduletag :rebuild + @moduletag :tmp_dir + + import AshSqlite.RebuildHelper + + setup %{tmp_dir: tmp_dir} = context do + repo = Map.get(context, :repo, AshSqlite.RebuildTestRepo) + %{ctx: start(tmp_dir, Map.get(context, :repo_config, []), repo)} + end + + # posts, and comments that are deleted with their post, with a row each + defp blog(ctx) do + write_migration(ctx, 1, "create_blog", """ + def up do + create table(:posts, primary_key: false) do + add :id, :text, primary_key: true + add :title, :text, null: false + add :subtitle, :text + end + + create table(:comments, primary_key: false) do + add :id, :text, primary_key: true + add :post_id, references(:posts, type: :text, on_delete: :delete_all) + end + end + + def down do + drop table(:comments) + drop table(:posts) + end + """) + + migrate(ctx) + sql("INSERT INTO posts (id, title) VALUES ('p', 'hello')") + sql("INSERT INTO comments (id, post_id) VALUES ('c', 'p')") + end + + @tag repo_config: [pool_size: 1] + test "rebuilds a table: its rows stay, and so do the rows that point at it", %{ctx: ctx} do + blog(ctx) + + write_migration(ctx, 2, "subtitle_required", """ + def up do + rebuild_table :posts do + add :id, :text, primary_key: true + add :title, :text + add :subtitle, :text + end + end + + def down, do: :ok + """) + + migrate(ctx) + + assert [%{name: "title", notnull: false}] = + Enum.filter(columns("posts"), &(&1.name == "title")) + + assert sql("SELECT id, title FROM posts") == [["p", "hello"]] + # dropping the old table did not run ON DELETE CASCADE on the comments + assert sql("SELECT id FROM comments") == [["c"]] + assert sql("PRAGMA foreign_keys") == [[1]] + end + + test "copy: renames a column and computes a value", %{ctx: ctx} do + blog(ctx) + + write_migration(ctx, 2, "rename_and_slug", """ + def up do + rebuild_table :posts, copy: [name: :title, slug: "lower(title) || '!'"] do + add :id, :text, primary_key: true + add :name, :text + add :slug, :text + end + end + + def down, do: :ok + """) + + migrate(ctx) + + assert sql("SELECT id, name, slug FROM posts") == [["p", "hello", "hello!"]] + end + + test "a copy: that names a column the new table does not have says so", %{ctx: ctx} do + blog(ctx) + + write_migration(ctx, 2, "typo", """ + def up do + rebuild_table :posts, copy: [nme: "'x'"] do + add :id, :text, primary_key: true + add :name, :text + end + end + + def down, do: :ok + """) + + error = migrate_error(ctx) + assert error =~ "names nme, which the new table does not have" + assert error =~ "it has id, name" + end + + test "a rebuild that would copy nothing refuses, rather than lose the rows", %{ctx: ctx} do + write_migration(ctx, 1, "create_items", """ + def up do + execute "CREATE TABLE items (old TEXT)" + execute "INSERT INTO items VALUES ('a'), ('b')" + end + + def down, do: :ok + """) + + write_migration(ctx, 2, "replace_items", """ + def up do + rebuild_table :items do + add :new, :text, default: "new" + end + end + + def down, do: :ok + """) + + assert migrate_error(ctx) =~ "would copy nothing" + assert sql("SELECT old FROM items ORDER BY old") == [["a"], ["b"]] + assert versions() == [1] + end + + @tag repo_config: [pool_size: 1] + test "when it fails, nothing is changed and foreign keys are back on", %{ctx: ctx} do + blog(ctx) + before = columns("posts") + + write_migration(ctx, 2, "subtitle_required", """ + def up do + rebuild_table :posts do + add :id, :text, primary_key: true + add :title, :text, null: false + add :subtitle, :text, null: false + end + end + + def down, do: :ok + """) + + assert migrate_error(ctx) =~ "NOT NULL constraint failed" + + assert columns("posts") == before + assert sql("SELECT id FROM comments") == [["c"]] + assert leftovers() == [] + assert versions() == [1] + assert sql("PRAGMA foreign_keys") == [[1]] + end + + @tag repo_config: [pool_size: 1] + test "a foreign key that is already violated stops it", %{ctx: ctx} do + blog(ctx) + sql("PRAGMA foreign_keys = OFF") + sql("INSERT INTO comments (id, post_id) VALUES ('orphan', 'nobody')") + + write_migration(ctx, 2, "anything", """ + def up do + rebuild_table :posts do + add :id, :text, primary_key: true + add :title, :text + add :subtitle, :text + end + end + + def down, do: :ok + """) + + assert migrate_error(ctx) =~ "foreign key check failed" + assert versions() == [1] + assert leftovers() == [] + end + + test "a migration without a transaction refuses, rather than drop with foreign keys on", %{ + ctx: ctx + } do + blog(ctx) + + write_migration(ctx, 2, "no_transaction", """ + @disable_ddl_transaction true + + def up do + rebuild_table :posts do + add :id, :text, primary_key: true + add :title, :text + add :subtitle, :text + end + end + + def down, do: :ok + """) + + assert migrate_error(ctx) =~ "refusing to rebuild posts: foreign keys are on" + assert sql("SELECT id FROM comments") == [["c"]] + end + + # a repo can have foreign keys off for every connection, which is all the check above looks at + @tag repo_config: [foreign_keys: :off, pool_size: 1] + test "a migration without a transaction refuses, with foreign keys off", %{ctx: ctx} do + blog(ctx) + + write_migration(ctx, 2, "no_transaction", """ + @disable_ddl_transaction true + + def up do + rebuild_table :posts do + add :id, :text, primary_key: true + add :title, :text + add :subtitle, :text + end + end + + def down, do: :ok + """) + + assert migrate_error(ctx) =~ + "refusing to rebuild posts: the migration is not in a transaction" + + assert versions() == [1] + assert sql("SELECT id FROM comments") == [["c"]] + end + + test "a table that stops being AUTOINCREMENT and becomes one again has one row", %{ctx: ctx} do + write_migration(ctx, 1, "create_things", """ + def up do + execute "CREATE TABLE things (id INTEGER PRIMARY KEY AUTOINCREMENT)" + execute "INSERT INTO things DEFAULT VALUES" + execute "INSERT INTO things DEFAULT VALUES" + end + + def down, do: :ok + """) + + write_migration(ctx, 2, "there_and_back", """ + def up do + rebuild_table :things do + add :id, :integer, primary_key: true + end + + rebuild_table :things do + add :id, :bigserial, primary_key: true + end + end + + def down, do: :ok + """) + + migrate(ctx) + + assert [["things", 2]] = sql("SELECT name, seq FROM sqlite_sequence") + end + + describe "with ecto_libsql" do + @describetag repo: AshSqlite.RebuildLibSqlRepo + + # it has no `foreign_keys:` connection option, so what the migration does to the PRAGMA is + # all there is: a rebuild that left them on would have deleted the comments + @tag repo_config: [pool_size: 1] + test "a rebuild keeps the rows that point at the table", %{ctx: ctx} do + blog(ctx) + + write_migration(ctx, 2, "title_optional", """ + def up do + rebuild_table :posts do + add :id, :text, primary_key: true + add :title, :text + add :subtitle, :text + end + end + + def down, do: :ok + """) + + migrate(ctx) + + assert sql("SELECT id FROM comments") == [["c"]] + assert sql("PRAGMA foreign_keys") == [[1]] + end + + @tag repo_config: [pool_size: 1] + test "a rebuild works when the database has an AUTOINCREMENT table", %{ctx: ctx} do + write_migration(ctx, 1, "create_tables", """ + def up do + execute "CREATE TABLE counters (id INTEGER PRIMARY KEY AUTOINCREMENT)" + execute "CREATE TABLE plain (id INTEGER PRIMARY KEY, name TEXT NOT NULL)" + end + + def down, do: :ok + """) + + write_migration(ctx, 2, "name_optional", """ + def up do + rebuild_table :plain do + add :id, :integer, primary_key: true + add :name, :text + end + end + + def down, do: :ok + """) + + migrate(ctx) + + assert versions() == [1, 2] + end + end +end diff --git a/test/rebuild_unsupported_test.exs b/test/rebuild_unsupported_test.exs new file mode 100644 index 0000000..32cda51 --- /dev/null +++ b/test/rebuild_unsupported_test.exs @@ -0,0 +1,53 @@ +# SPDX-FileCopyrightText: 2023 ash_sqlite contributors +# +# SPDX-License-Identifier: MIT + +defmodule AshSqlite.RebuildUnsupportedTest do + @moduledoc """ + What a rebuild does on an `ecto_sql` without the migration callbacks: it refuses. + """ + use ExUnit.Case, async: false + @moduletag :no_rebuild_support + @moduletag :tmp_dir + + import AshSqlite.RebuildHelper + + setup %{tmp_dir: tmp_dir}, do: %{ctx: start(tmp_dir)} + + test "a rebuild refuses while foreign keys are on, and the rows that point at the table stay", + %{ctx: ctx} do + write_migration(ctx, 1, "create_blog", """ + def up do + create table(:posts, primary_key: false) do + add :id, :text, primary_key: true + end + + create table(:comments, primary_key: false) do + add :id, :text, primary_key: true + add :post_id, references(:posts, type: :text, on_delete: :delete_all) + end + end + + def down, do: :ok + """) + + migrate(ctx) + sql("INSERT INTO posts (id) VALUES ('p')") + sql("INSERT INTO comments (id, post_id) VALUES ('c', 'p')") + + write_migration(ctx, 2, "rebuild_posts", """ + def up do + rebuild_table :posts do + add :id, :text, primary_key: true + add :title, :text + end + end + + def down, do: :ok + """) + + assert migrate_error(ctx) =~ "only in an `ecto_sql` that has them" + assert sql("SELECT id FROM comments") == [["c"]] + assert versions() == [1] + end +end diff --git a/test/support/rebuild_helper.ex b/test/support/rebuild_helper.ex new file mode 100644 index 0000000..d3822eb --- /dev/null +++ b/test/support/rebuild_helper.ex @@ -0,0 +1,145 @@ +# SPDX-FileCopyrightText: 2023 ash_sqlite contributors +# +# SPDX-License-Identifier: MIT + +defmodule AshSqlite.RebuildHelper do + @moduledoc """ + Runs generated migrations against a real SQLite file, for the table rebuild tests. + """ + alias AshSqlite.RebuildTestRepo + + # The repo the current test started (the process dictionary: the tests are not async) + defp repo, do: Process.get(:rebuild_repo, RebuildTestRepo) + + @doc """ + Points `AshSqlite.RebuildTestRepo` at a new database file in `tmp_dir` and starts it. + `repo_config` is merged into the repo's configuration (`pool_size: 2`, ...). + Returns a context map the other helpers take. + """ + def start(tmp_dir, repo_config \\ [], repo \\ RebuildTestRepo) do + Process.put(:rebuild_repo, repo) + + ctx = %{ + db: Path.join(tmp_dir, "test.db"), + snapshot_path: Path.join(tmp_dir, "snapshots"), + migration_path: Path.join(tmp_dir, "migrations") + } + + config = + Keyword.merge( + [ + database: ctx.db, + pool_size: 1, + migration_lock: false + ], + repo_config + ) + + Application.put_env(:ash_sqlite, repo, config) + ExUnit.Callbacks.start_supervised!(repo) + ExUnit.Callbacks.on_exit(fn -> Application.delete_env(:ash_sqlite, repo) end) + + ctx + end + + @doc "Generates migrations for `domain` (as `mix ash_sqlite.generate_migrations` would)." + def generate(domain, ctx, opts \\ []) do + AshSqlite.MigrationGenerator.generate( + domain, + Keyword.merge( + [ + snapshot_path: ctx.snapshot_path, + migration_path: ctx.migration_path, + quiet: true, + format: false, + auto_name: true, + rebuild_tables: true + ], + opts + ) + ) + + migrations(ctx) + end + + @doc "The generated migration files, oldest first." + def migrations(ctx) do + Enum.sort(Path.wildcard("#{ctx.migration_path}/**/*_migrate_resources*.exs")) + end + + def last_migration(ctx), do: ctx |> migrations() |> List.last() |> File.read!() + + @doc "Writes a migration, `body` being what goes in the module after `use AshSqlite.Migration`." + def write_migration(ctx, version, name, body) do + File.mkdir_p!(ctx.migration_path) + module = Module.concat([repo(), Migrations, Macro.camelize(name)]) + + File.write!(Path.join(ctx.migration_path, "#{version}_#{name}.exs"), """ + defmodule #{inspect(module)} do + use AshSqlite.Migration + + #{body} + end + """) + end + + @doc "Runs all pending migrations. Returns the versions run, or raises." + def migrate(ctx) do + Code.compiler_options(ignore_module_conflict: true) + + try do + Ecto.Migrator.run(repo(), ctx.migration_path, :up, all: true, log: false) + after + Code.compiler_options(ignore_module_conflict: false) + end + end + + @doc "Rolls back the last `n` migrations." + def rollback(ctx, n \\ 1) do + Code.compiler_options(ignore_module_conflict: true) + + try do + Ecto.Migrator.run(repo(), ctx.migration_path, :down, step: n, log: false) + after + Code.compiler_options(ignore_module_conflict: false) + end + end + + @doc "Runs SQL; returns the rows (lists)." + def sql(statement, params \\ []) do + Ecto.Adapters.SQL.query!(repo(), statement, params).rows + end + + @doc "`PRAGMA foreign_key_check` rows (an empty list means all foreign keys hold)." + def fk_violations, do: sql("PRAGMA foreign_key_check") + + def columns(table) do + "SELECT name, type, \"notnull\", dflt_value, pk FROM pragma_table_info('#{table}')" + |> sql() + |> Enum.map(fn [name, type, notnull, default, pk] -> + %{name: name, type: type, notnull: notnull == 1, default: default, pk: pk} + end) + end + + @doc "Runs the pending migrations, which must fail. Returns the exception's message." + def migrate_error(ctx) do + migrate(ctx) + flunk("expected the migration to fail") + rescue + error in ExUnit.AssertionError -> reraise error, __STACKTRACE__ + error -> Exception.message(error) + end + + defp flunk(message), do: raise(ExUnit.AssertionError, message: message) + + @doc "The versions recorded in schema_migrations." + def versions, + do: "SELECT version FROM schema_migrations ORDER BY version" |> sql() |> List.flatten() + + @doc "Tables left over from a rebuild (named `*_rebuild`)." + def leftovers do + "SELECT name FROM sqlite_master WHERE type = 'table' AND name LIKE '%\\_rebuild' ESCAPE '\\'" + |> sql() + |> List.flatten() + end +end diff --git a/test/support/rebuild_test_repo.ex b/test/support/rebuild_test_repo.ex new file mode 100644 index 0000000..e6851e7 --- /dev/null +++ b/test/support/rebuild_test_repo.ex @@ -0,0 +1,27 @@ +# SPDX-FileCopyrightText: 2023 ash_sqlite contributors +# +# SPDX-License-Identifier: MIT + +defmodule AshSqlite.RebuildTestRepo do + @moduledoc """ + A repo with an ordinary pool (no sandbox) whose database file is chosen per test + through the application environment, see `AshSqlite.RebuildHelper`. + """ + use AshSqlite.Repo, + otp_app: :ash_sqlite + + def min_sqlite_version do + %Version{major: 3, minor: 38, patch: 0} + end +end + +defmodule AshSqlite.RebuildLibSqlRepo do + @moduledoc "Like `AshSqlite.RebuildTestRepo`, on the optional ecto_libsql adapter." + use AshSqlite.Repo, + otp_app: :ash_sqlite, + adapter: Ecto.Adapters.LibSql + + def min_sqlite_version do + %Version{major: 3, minor: 38, patch: 0} + end +end diff --git a/test/table_rebuild_test.exs b/test/table_rebuild_test.exs new file mode 100644 index 0000000..1eb3dd3 --- /dev/null +++ b/test/table_rebuild_test.exs @@ -0,0 +1,1244 @@ +# SPDX-FileCopyrightText: 2023 ash_sqlite contributors +# +# SPDX-License-Identifier: MIT + +defmodule AshSqlite.TableRebuildTest do + @moduledoc """ + Generates migrations for resources that change in ways SQLite cannot do in place, runs them + on a real SQLite file and checks what happens to the data. + """ + use ExUnit.Case, async: false + @moduletag :migration + @moduletag :rebuild + @moduletag :tmp_dir + + import AshSqlite.RebuildHelper + + setup %{tmp_dir: tmp_dir} = context do + current_shell = Mix.shell() + :ok = Mix.shell(Mix.Shell.Process) + on_exit(fn -> Mix.shell(current_shell) end) + + %{ + ctx: + start( + tmp_dir, + Map.get(context, :repo_config, []), + Map.get(context, :repo, AshSqlite.RebuildTestRepo) + ) + } + end + + # Defines a resource on the rebuild test repo. Redefining a module is how a test + # moves a resource from one version to the next. + defmacrop defres(mod, table, do: body) do + quote do + defres_on(AshSqlite.RebuildTestRepo, unquote(mod), unquote(table), do: unquote(body)) + end + end + + defmacrop defres_on(repo, mod, table, do: body) do + quote do + Code.compiler_options(ignore_module_conflict: true) + + defmodule unquote(mod) do + use Ash.Resource, + domain: nil, + data_layer: AshSqlite.DataLayer + + sqlite do + table unquote(table) + repo(unquote(repo)) + end + + actions do + defaults([:create, :read, :update, :destroy]) + end + + unquote(body) + end + + Code.compiler_options(ignore_module_conflict: false) + end + end + + defmacrop defdomain(resources) do + quote do + Code.compiler_options(ignore_module_conflict: true) + + defmodule Domain do + use Ash.Domain, validate_config_inclusion?: false + + resources do + for resource <- unquote(resources) do + resource(resource) + end + end + end + + Code.compiler_options(ignore_module_conflict: false) + end + end + + describe "making a column nullable" do + test "keeps the rows and the child rows that cascade from them", %{ctx: ctx} do + defres Parent, "parents" do + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + end + end + + defres Child, "children" do + attributes do + uuid_primary_key(:id) + end + + relationships do + belongs_to(:parent, Parent, allow_nil?: false) + end + + sqlite do + references do + reference(:parent, on_delete: :delete) + end + end + end + + defdomain([Parent, Child]) + generate(Domain, ctx) + migrate(ctx) + + sql("INSERT INTO parents (id, name) VALUES ('p1', 'one'), ('p2', 'two')") + sql("INSERT INTO children (id, parent_id) VALUES ('c1', 'p1'), ('c2', 'p2')") + + defres Parent, "parents" do + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + end + + generate(Domain, ctx) + migration = last_migration(ctx) + assert migration =~ "use AshSqlite.Migration" + migrate(ctx) + + assert sql("SELECT id, name FROM parents ORDER BY id") == [["p1", "one"], ["p2", "two"]] + assert sql("SELECT id FROM children ORDER BY id") == [["c1"], ["c2"]] + assert fk_violations() == [] + + assert [%{name: "name", notnull: false}] = + Enum.filter(columns("parents"), &(&1.name == "name")) + end + end + + # A resource on the table "items", the one most scenarios change. + defmacrop defitem(do: body) do + quote do + defres Item, "items" do + unquote(body) + end + + defdomain([Item]) + end + end + + describe "making a column required" do + test "without a default, fails loudly when a row holds NULL and leaves nothing behind", %{ + ctx: ctx + } do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, name) VALUES ('a', 'x'), ('b', NULL)") + versions_before = versions() + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + end + end + + generate(Domain, ctx) + assert migrate_error(ctx) =~ "NOT NULL constraint failed" + + assert leftovers() == [] + assert versions() == versions_before + assert sql("SELECT id, name FROM items ORDER BY id") == [["a", "x"], ["b", nil]] + + assert [%{name: "name", notnull: false}] = + Enum.filter(columns("items"), &(&1.name == "name")) + end + + test "with a default, fills the NULLs with it", %{ctx: ctx} do + defitem do + sqlite do + migration_defaults(name: "\"unnamed\"") + end + + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, name) VALUES ('a', 'x'), ('b', NULL)") + + defitem do + sqlite do + migration_defaults(name: "\"unnamed\"") + end + + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false, default: "unnamed") + end + end + + generate(Domain, ctx) + assert last_migration(ctx) =~ "COALESCE" + migrate(ctx) + + assert sql("SELECT id, name FROM items ORDER BY id") == [["a", "x"], ["b", "unnamed"]] + end + end + + describe "adding a required column without a default" do + test "works on an empty table and fails loudly on a table with rows", %{ctx: ctx} do + defitem do + attributes do + uuid_primary_key(:id) + end + end + + generate(Domain, ctx) + migrate(ctx) + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + end + end + + generate(Domain, ctx) + sql("INSERT INTO items (id) VALUES ('a')") + assert migrate_error(ctx) =~ "NOT NULL constraint failed: items_rebuild.name" + assert leftovers() == [] + assert sql("SELECT id FROM items") == [["a"]] + + sql("DELETE FROM items") + migrate(ctx) + + assert [%{name: "name", notnull: true}] = + Enum.filter(columns("items"), &(&1.name == "name")) + end + end + + describe "defaults that are not literals" do + test "a new column keeps its SQL default for the rows that exist", %{ctx: ctx} do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, name) VALUES ('a', 'x'), ('b', 'y')") + + defitem do + sqlite do + migration_defaults(stamp: "fragment(\"(datetime('now'))\")") + end + + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false, default: "z") + attribute(:stamp, :string) + end + end + + generate(Domain, ctx) + migrate(ctx) + + assert [[stamp_a], [stamp_b]] = sql("SELECT stamp FROM items ORDER BY id") + assert is_binary(stamp_a) and is_binary(stamp_b) + end + end + + describe "things in the database that refer to the rebuilt table" do + test "a view and a trigger on another table that mention it survive", %{ctx: ctx} do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("CREATE TABLE audit (msg TEXT)") + sql("CREATE TABLE other (id TEXT)") + sql("CREATE VIEW item_names AS SELECT name FROM items") + + sql( + "CREATE TRIGGER other_audit AFTER INSERT ON other BEGIN INSERT INTO audit SELECT name FROM items; END" + ) + + sql("INSERT INTO items (id, name) VALUES ('a', 'x')") + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + end + + generate(Domain, ctx) + migrate(ctx) + + assert sql("SELECT name FROM item_names") == [["x"]] + sql("INSERT INTO other (id) VALUES ('o')") + assert sql("SELECT msg FROM audit") == [["x"]] + end + end + + describe "integer primary keys with AUTOINCREMENT" do + test "ids that were used and deleted are not handed out again", %{ctx: ctx} do + defitem do + attributes do + integer_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (name) VALUES ('a'), ('b'), ('c')") + sql("DELETE FROM items WHERE id = 3") + + defitem do + attributes do + integer_primary_key(:id) + attribute(:name, :string) + end + end + + generate(Domain, ctx) + migrate(ctx) + + sql("INSERT INTO items (name) VALUES ('d')") + assert sql("SELECT id, name FROM items ORDER BY id") == [[1, "a"], [2, "b"], [4, "d"]] + end + end + + describe "foreign key topology" do + test "a self-referencing table", %{ctx: ctx} do + defres Node, "nodes" do + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + end + + relationships do + belongs_to(:parent, Node) + end + + sqlite do + references do + reference(:parent, on_delete: :delete) + end + end + end + + defdomain([Node]) + generate(Domain, ctx) + migrate(ctx) + + sql( + "INSERT INTO nodes (id, name, parent_id) VALUES ('root', 'r', NULL), ('kid', 'k', 'root')" + ) + + defres Node, "nodes" do + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + + relationships do + belongs_to(:parent, Node) + end + + sqlite do + references do + reference(:parent, on_delete: :delete) + end + end + end + + generate(Domain, ctx) + migrate(ctx) + + assert sql("SELECT id, parent_id FROM nodes ORDER BY id") == [ + ["kid", "root"], + ["root", nil] + ] + + assert fk_violations() == [] + # still a working cascade pointing at the table itself + sql("DELETE FROM nodes WHERE id = 'root'") + assert sql("SELECT id FROM nodes") == [] + end + + test "a new column with a foreign key can be rolled back", %{ctx: ctx} do + defres Owner, "owners" do + attributes do + uuid_primary_key(:id) + end + end + + defres Pet, "pets" do + attributes do + uuid_primary_key(:id) + end + end + + defdomain([Owner, Pet]) + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO pets (id) VALUES ('p')") + + defres Pet, "pets" do + attributes do + uuid_primary_key(:id) + end + + relationships do + belongs_to(:owner, Owner) + end + end + + generate(Domain, ctx) + migrate(ctx) + assert [[_, _, "owners" | _]] = sql("PRAGMA foreign_key_list(pets)") + + rollback(ctx) + assert Enum.map(columns("pets"), & &1.name) == ["id"] + assert sql("SELECT id FROM pets") == [["p"]] + assert fk_violations() == [] + end + + test "a table created in the same migration that references the rebuilt one", %{ctx: ctx} do + defres Parent, "parents" do + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + end + end + + defdomain([Parent]) + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO parents (id, name) VALUES ('p', 'n')") + + defres Parent, "parents" do + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + end + + defres Child, "children" do + attributes do + uuid_primary_key(:id) + end + + relationships do + belongs_to(:parent, Parent, allow_nil?: false) + end + + sqlite do + references do + reference(:parent, on_delete: :delete) + end + end + end + + defdomain([Parent, Child]) + generate(Domain, ctx) + migrate(ctx) + + sql("INSERT INTO children (id, parent_id) VALUES ('c', 'p')") + assert fk_violations() == [] + sql("DELETE FROM parents") + assert sql("SELECT id FROM children") == [] + end + end + + # Two versions of the resource on "items": `name` required, then optional. + defmacrop items_v1 do + quote do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + end + end + end + end + + defmacrop items_v2 do + quote do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + end + end + end + + describe "how the migration is run" do + test "rolling back the rebuild with data that fits", %{ctx: ctx} do + items_v1() + generate(Domain, ctx) + migrate(ctx) + + items_v2() + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, name) VALUES ('a', 'x')") + rollback(ctx) + + assert [%{name: "name", notnull: true}] = + Enum.filter(columns("items"), &(&1.name == "name")) + + assert sql("SELECT id, name FROM items") == [["a", "x"]] + assert versions() |> length() == 1 + end + end + + defp assert_raise_on_rollback(ctx, message) do + error = + try do + rollback(ctx) + flunk("expected the rollback to fail") + rescue + error in ExUnit.AssertionError -> reraise error, __STACKTRACE__ + error -> Exception.message(error) + end + + assert error =~ message + end + + describe "the generator" do + test "a rename together with a change is both a rename and a change", %{ctx: ctx} do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:title, :string) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, title) VALUES ('a', 'x')") + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:subject, :string, allow_nil?: false) + end + end + + send(self(), {:mix_shell_input, :yes?, true}) + send(self(), {:mix_shell_input, :prompt, "subject"}) + generate(Domain, ctx) + assert last_migration(ctx) =~ "use AshSqlite.Migration" + migrate(ctx) + + assert sql("SELECT id, subject FROM items") == [["a", "x"]] + + assert [%{name: "subject", notnull: true}] = + Enum.filter(columns("items"), &(&1.name == "subject")) + end + + test "generating again after a rebuild migration finds nothing to do (codegen --check)", %{ + ctx: ctx + } do + items_v1() + generate(Domain, ctx) + migrate(ctx) + items_v2() + generate(Domain, ctx) + migrate(ctx) + + files = migrations(ctx) + assert generate(Domain, ctx) == files + assert generate(Domain, ctx, check: true) == files + end + + test "the formatted migration is valid and runs", %{ctx: ctx} do + items_v1() + generate(Domain, ctx, format: true) + migrate(ctx) + sql("INSERT INTO items (id, name) VALUES ('a', 'x')") + items_v2() + generate(Domain, ctx, format: true) + migrate(ctx) + assert sql("SELECT id, name FROM items") == [["a", "x"]] + end + + test "a column removed from the resource is kept (nullable) by the rebuild", %{ctx: ctx} do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + attribute(:legacy, :string, allow_nil?: false) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, name, legacy) VALUES ('a', 'x', 'old data')") + + items_v2() + generate(Domain, ctx) + migrate(ctx) + + assert sql("SELECT id, name, legacy FROM items") == [["a", "x", "old data"]] + assert [%{notnull: false}] = Enum.filter(columns("items"), &(&1.name == "legacy")) + + # a row written after: the resource no longer knows `legacy`, so it is NULL + sql("INSERT INTO items (id, name) VALUES ('b', 'y')") + # going back would need a value for the required column `legacy` + assert_raise_on_rollback(ctx, "NOT NULL constraint failed") + assert length(sql("SELECT id FROM items")) == 2 + end + end + + defp index_sql(table) do + sql( + "SELECT name, sql FROM sqlite_master WHERE type = 'index' AND tbl_name = ? AND sql IS NOT NULL ORDER BY name", + [ + table + ] + ) + end + + defmacrop indexed_items(nullable?) do + quote do + defitem do + sqlite do + custom_indexes do + index([:name], where: "name IS NOT NULL", name: "items_partial_index") + end + end + + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: unquote(!nullable?), public?: true) + attribute(:code, :string, public?: true) + end + + identities do + identity(:unique_code, [:code]) + identity(:unique_live_name, [:name], where: expr(not is_nil(^ref(:code)))) + end + end + end + end + + defmacrop retyped_items(type, strict?) do + quote do + defitem do + sqlite do + strict?(unquote(strict?)) + end + + attributes do + uuid_primary_key(:id) + attribute(:amount, unquote(type)) + end + end + end + end + + describe "resource shapes" do + test "a composite primary key", %{ctx: ctx} do + defitem do + attributes do + attribute(:org, :string, primary_key?: true, allow_nil?: false) + attribute(:code, :string, primary_key?: true, allow_nil?: false) + attribute(:name, :string, allow_nil?: false) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (org, code, name) VALUES ('o', 'c', 'x')") + + defitem do + attributes do + attribute(:org, :string, primary_key?: true, allow_nil?: false) + attribute(:code, :string, primary_key?: true, allow_nil?: false) + attribute(:name, :string) + end + end + + generate(Domain, ctx) + migrate(ctx) + + assert sql("SELECT org, code, name FROM items") == [["o", "c", "x"]] + assert Enum.map(columns("items"), & &1.pk) |> Enum.sort() == [0, 1, 2] + + assert_raise Exqlite.Error, ~r/UNIQUE constraint failed/, fn -> + sql("INSERT INTO items (org, code, name) VALUES ('o', 'c', 'dup')") + end + end + + test "a primary key that changes is listed as such", %{ctx: ctx} do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:code, :string, allow_nil?: false) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, code) VALUES ('a', 'c')") + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:code, :string, primary_key?: true, allow_nil?: false) + end + end + + generate(Domain, ctx) + assert last_migration(ctx) =~ "# - changing the primary key" + refute last_migration(ctx) =~ "Operation" + + migrate(ctx) + assert Enum.map(columns("items"), & &1.pk) |> Enum.sort() == [1, 2] + end + + test "identities with a where, custom indexes with where", + %{ctx: ctx} do + indexed_items(false) + generate(Domain, ctx) + migrate(ctx) + before = index_sql("items") + assert length(before) == 3 + + indexed_items(true) + generate(Domain, ctx) + migrate(ctx) + + assert index_sql("items") == before + end + + test "a table named like the temporary one is refused loudly, not overwritten", %{ctx: ctx} do + defres Other, "items_rebuild" do + attributes do + uuid_primary_key(:id) + end + end + + items_v1() + defdomain([Item, Other]) + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items_rebuild (id) VALUES ('keep me')") + + items_v2() + defdomain([Item, Other]) + generate(Domain, ctx) + assert migrate_error(ctx) =~ "already exists" + assert sql("SELECT id FROM items_rebuild") == [["keep me"]] + end + end + + describe "changing a column's type" do + defp retype(ctx, strict?) do + retyped_items(:string, strict?) + + generate(__MODULE__.Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, amount) VALUES ('a', '12'), ('b', 'twelve')") + + retyped_items(:integer, strict?) + + generate(__MODULE__.Domain, ctx) + end + + test "in a STRICT table a value that is not a number is loud", %{ctx: ctx} do + retype(ctx, true) + refute last_migration(ctx) =~ "not STRICT" + assert migrate_error(ctx) =~ "cannot store TEXT value in INTEGER column" + assert sql("SELECT id, amount FROM items ORDER BY id") == [["a", "12"], ["b", "twelve"]] + end + + test "in a table that is not STRICT it is copied as it is, text in an integer column", %{ + ctx: ctx + } do + retype(ctx, false) + assert last_migration(ctx) =~ "REVIEW: `amount` changes type in a table that is not STRICT" + migrate(ctx) + + assert sql("SELECT id, amount, typeof(amount) FROM items ORDER BY id") == [ + ["a", 12, "integer"], + ["b", "twelve", "text"] + ] + end + end + + defmacrop trigger_items(nullable?) do + quote do + defitem do + sqlite do + custom_statements do + statement :items_audit do + up "CREATE TRIGGER items_audit AFTER INSERT ON items BEGIN INSERT INTO audit VALUES (new.id); END" + down "DROP TRIGGER items_audit" + end + end + end + + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: unquote(!nullable?)) + end + end + end + end + + describe "custom statements on the rebuilt table" do + test "a trigger is dropped before the rebuild, recreated after, and still fires", %{ctx: ctx} do + sql("CREATE TABLE IF NOT EXISTS audit (msg TEXT)") + trigger_items(false) + generate(Domain, ctx) + migrate(ctx) + sql("CREATE TABLE IF NOT EXISTS audit (msg TEXT)") + sql("INSERT INTO items (id, name) VALUES ('a', 'x')") + assert sql("SELECT msg FROM audit") == [["a"]] + + trigger_items(true) + generate(Domain, ctx) + assert last_migration(ctx) =~ "REVIEW: the custom statements :items_audit" + migrate(ctx) + + sql("INSERT INTO items (id, name) VALUES ('b', 'y')") + assert sql("SELECT msg FROM audit ORDER BY msg") == [["a"], ["b"]] + assert [["items_audit"]] = sql("SELECT name FROM sqlite_master WHERE type = 'trigger'") + end + end + + describe "dev migrations" do + test "dev migrations that rebuild can be replaced by the real migration", %{ctx: ctx} do + items_v1() + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, name) VALUES ('a', 'x')") + + items_v2() + generate(Domain, ctx, dev: true) + migrate(ctx) + assert [_, dev] = migrations_with_dev(ctx) + assert dev =~ "_dev.exs" + + # `ash.codegen name` rolls the dev migrations back and writes the real one + generate(Domain, ctx, name: "make_name_optional") + + assert [_, real] = migrations_with_dev(ctx) + refute real =~ "_dev.exs" + assert File.read!(real) =~ "use AshSqlite.Migration" + # the dev migration was rolled back: the column is required again until the real one runs + assert [%{notnull: true}] = Enum.filter(columns("items"), &(&1.name == "name")) + migrate(ctx) + assert [%{notnull: false}] = Enum.filter(columns("items"), &(&1.name == "name")) + assert sql("SELECT id, name FROM items") == [["a", "x"]] + end + end + + defp migrations_with_dev(ctx), do: Enum.sort(Path.wildcard("#{ctx.migration_path}/**/*.exs")) + + describe "dropping columns" do + test "--drop-columns drops the column instead of keeping it", %{ctx: ctx} do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + attribute(:legacy, :string) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, name, legacy) VALUES ('a', 'x', 'old')") + + items_v2() + generate(Domain, ctx, drop_columns: true) + refute last_migration(ctx) =~ "no longer in the resource" + migrate(ctx) + + assert Enum.map(columns("items"), & &1.name) == ["id", "name"] + end + end + + defmacrop filtered_items(nullable?) do + quote do + defitem do + sqlite do + base_filter_sql("name IS NOT NULL") + end + + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: unquote(!nullable?), public?: true) + end + + identities do + identity(:unique_name, [:name]) + end + end + end + end + + describe "keys and filters" do + test "a resource's base filter on its identities", %{ctx: ctx} do + filtered_items(false) + generate(Domain, ctx) + migrate(ctx) + before = index_sql("items") + assert [[_, index_sql]] = before + assert index_sql =~ "WHERE" + + filtered_items(true) + generate(Domain, ctx) + migrate(ctx) + assert index_sql("items") == before + end + end + + describe "opting in through the repo's config" do + @tag repo_config: [rebuild_tables: true] + test "the repo's config turns it on for every run", %{ctx: ctx} do + domain = make_name_optional_from_required(ctx) + + generate(domain, ctx, rebuild_tables: nil) + + assert last_migration(ctx) =~ "rebuild_table :items" + end + + @tag repo_config: [rebuild_tables: true] + test "--no-rebuild-tables turns it off for one run", %{ctx: ctx} do + domain = make_name_optional_from_required(ctx) + + generate(domain, ctx, rebuild_tables: false) + + migration = last_migration(ctx) + refute migration =~ "rebuild_table" + assert migration =~ "modify :name, :text, null: true" + end + + defp make_name_optional_from_required(ctx) do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + end + end + + generate(Domain, ctx, rebuild_tables: nil) + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + end + + # the alias made by `defdomain` only lives in this function + Domain + end + end + + describe "the copy" do + defp edit_last_migration(ctx, from, to) do + file = ctx |> migrations() |> List.last() + contents = File.read!(file) + assert contents =~ from + File.write!(file, String.replace(contents, from, to, global: false)) + end + + test "a required column without a default: the comment's remedy gives the rows a value", + %{ctx: ctx} do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, name) VALUES ('a', 'x'), ('b', NULL)") + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:name, :string, allow_nil?: false) + end + end + + generate(Domain, ctx) + assert last_migration(ctx) =~ "REVIEW: :name becomes required and has no default" + assert last_migration(ctx) =~ ~S|copy: [name: "COALESCE(name, 'your value')"]| + + # as it is, it stops, as the comment says + assert migrate_error(ctx) =~ "NOT NULL constraint failed" + + edit_last_migration( + ctx, + "rebuild_table :items do", + ~S|rebuild_table :items, copy: [name: "COALESCE(name, 'unnamed')"] do| + ) + + migrate(ctx) + assert sql("SELECT id, name FROM items ORDER BY id") == [["a", "x"], ["b", "unnamed"]] + end + + test "a new required column: the comment's remedy gives the existing rows a value", %{ + ctx: ctx + } do + defitem do + attributes do + uuid_primary_key(:id) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id) VALUES ('a')") + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:code, :string, allow_nil?: false) + end + end + + generate(Domain, ctx) + assert last_migration(ctx) =~ "REVIEW: :code is new, required and has no default" + assert last_migration(ctx) =~ ~S|copy: [code: "'your value'"]| + + assert migrate_error(ctx) =~ "NOT NULL constraint failed" + + edit_last_migration( + ctx, + "rebuild_table :items do", + ~S|rebuild_table :items, copy: [code: "'none'"] do| + ) + + migrate(ctx) + assert sql("SELECT id, code FROM items") == [["a", "none"]] + end + + test "a renamed column that becomes required: the remedy replaces the rename", %{ctx: ctx} do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:title, :string) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, title) VALUES ('a', NULL)") + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:subject, :string, allow_nil?: false) + end + end + + send(self(), {:mix_shell_input, :yes?, true}) + generate(Domain, ctx) + assert last_migration(ctx) =~ ~S|copy: [subject: "COALESCE(title, 'your value')"]| + assert last_migration(ctx) =~ "in place of its `subject: :title`" + + edit_last_migration( + ctx, + "copy: [subject: :title]", + ~S|copy: [subject: "COALESCE(title, 'untitled')"]| + ) + + migrate(ctx) + assert sql("SELECT id, subject FROM items") == [["a", "untitled"]] + end + end + + describe "default values" do + @describetag repo_config: [rebuild_tables: true] + + test "a new table gets the defaults of its attributes", %{ctx: ctx} do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:title, :string, default: "it's untitled") + attribute(:count, :integer, default: 0) + attribute(:ratio, :float, default: 1.5) + attribute(:done, :boolean, default: false) + attribute(:price, :decimal, default: Decimal.new("9.50")) + attribute(:on, :date, default: ~D[2020-01-02]) + attribute(:at, :utc_datetime, default: ~U[2020-01-02 03:04:05Z]) + end + end + + generate(Domain, ctx) + migration = last_migration(ctx) + assert migration =~ ~S|add :title, :text, default: "it's untitled"| + assert migration =~ "add :count, :bigint, default: 0" + assert migration =~ "add :done, :boolean, default: false" + + migrate(ctx) + sql("INSERT INTO items (id) VALUES ('a')") + + assert [["a", "it's untitled", 0, 1.5, 0, price, "2020-01-02", at]] = + sql("SELECT id, title, count, ratio, done, price, \"on\", at FROM items") + + assert to_string(price) =~ "9.5" + # the text Ecto stores, so that comparing with a value it dumps finds the row + assert at == "2020-01-02T03:04:05Z" + end + + test "a default added to an existing attribute rebuilds the table to get it", %{ctx: ctx} do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:title, :string) + end + end + + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, title) VALUES ('a', NULL)") + + defitem do + attributes do + uuid_primary_key(:id) + attribute(:title, :string, default: "untitled") + end + end + + generate(Domain, ctx) + assert last_migration(ctx) =~ ~S|add :title, :text, default: "untitled"| + migrate(ctx) + + sql("INSERT INTO items (id) VALUES ('b')") + # the rows that were there keep what they had + assert sql("SELECT id, title FROM items ORDER BY id") == [["a", nil], ["b", "untitled"]] + end + + test "--no-rebuild-tables turns them off for a run, whatever the config says", %{ctx: ctx} do + defitem do + attributes do + uuid_primary_key(:id) + attribute(:count, :integer, default: 0) + end + end + + generate(Domain, ctx, rebuild_tables: false) + + refute last_migration(ctx) =~ "default:" + end + end + + describe "STRICT" do + defmacrop strict_items(strict?) do + quote do + defitem do + sqlite do + strict?(unquote(strict?)) + end + + attributes do + uuid_primary_key(:id) + attribute(:count, :integer) + attribute(:name, :string) + end + end + end + end + + defp table_sql, do: sql("SELECT sql FROM sqlite_master WHERE name = 'items'") |> hd() |> hd() + + test "making a table STRICT rebuilds it and keeps the rows", %{ctx: ctx} do + strict_items(false) + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, count, name) VALUES ('a', 1, 'x')") + refute table_sql() =~ "STRICT" + + strict_items(true) + generate(Domain, ctx) + assert last_migration(ctx) =~ "# - making `items` a STRICT table" + + assert last_migration(ctx) =~ ~S|rebuild_table :items, options: "STRICT" do| + + migrate(ctx) + assert table_sql() =~ "STRICT" + assert sql("SELECT id, count, name FROM items") == [["a", 1, "x"]] + end + + test "a value that does not fit its column stops it, and nothing is changed", %{ctx: ctx} do + strict_items(false) + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, count) VALUES ('a', 'many')") + versions_before = versions() + + strict_items(true) + generate(Domain, ctx) + + assert migrate_error(ctx) =~ "cannot store TEXT value in INTEGER column" + assert leftovers() == [] + assert versions() == versions_before + refute table_sql() =~ "STRICT" + assert sql("SELECT count FROM items") == [["many"]] + end + + test "making a table not STRICT, and rolling that back", %{ctx: ctx} do + strict_items(true) + generate(Domain, ctx) + migrate(ctx) + sql("INSERT INTO items (id, count) VALUES ('a', 1)") + assert table_sql() =~ "STRICT" + + strict_items(false) + generate(Domain, ctx) + assert last_migration(ctx) =~ "# - `items` is no longer a STRICT table" + migrate(ctx) + refute table_sql() =~ "STRICT" + + rollback(ctx) + assert table_sql() =~ "STRICT" + assert sql("SELECT id, count FROM items") == [["a", 1]] + end + end +end diff --git a/test/test_helper.exs b/test/test_helper.exs index 30925ec..eff3b12 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -5,6 +5,13 @@ ExUnit.start() ExUnit.configure(stacktrace_depth: 100) +# A rebuild needs the migration callbacks of an `ecto_sql` that has them +if AshSqlite.Migration.rebuild_supported?() do + ExUnit.configure(exclude: [:no_rebuild_support]) +else + ExUnit.configure(exclude: [:rebuild]) +end + AshSqlite.TestRepo.start_link() AshSqlite.TransactionTestRepo.start_link() AshSqlite.DevTestRepo.start_link()