From a7b47fb70f752e7d9dfb9dd5318887aab6a191ae Mon Sep 17 00:00:00 2001 From: Paul Harter Date: Mon, 12 Sep 2022 18:14:02 +0100 Subject: [PATCH 01/18] refactoring with a struct --- lib/electric/migration.ex | 107 +++++++++++++++++ lib/electric/migrations.ex | 239 ++++++++++++++++++------------------- test/migration_test.exs | 88 ++++++++++---- 3 files changed, 287 insertions(+), 147 deletions(-) create mode 100644 lib/electric/migration.ex diff --git a/lib/electric/migration.ex b/lib/electric/migration.ex new file mode 100644 index 0000000..502ab7b --- /dev/null +++ b/lib/electric/migration.ex @@ -0,0 +1,107 @@ +defmodule Electric.Migration do + @moduledoc """ + + """ + + + @migration_file_name "migration.sql" + @satellite_file_name "satellite.sql" + @enforce_keys [:name] + defstruct name: "noname", src_folder: nil, original_body: nil, satellite_body: nil, error: nil + + + def original_file_path(migration) do + Path.join([migration.src_folder, migration.name, @migration_file_name]) + end + + def satellite_file_path(migration) do + Path.join([migration.src_folder, migration.name, @satellite_file_name]) + end + + def ensure_original_body(migration) do + if migration.original_body == nil do + sql = File.read!(original_file_path(migration)) + %{migration | original_body: sql} + else + migration + end + end + + def ensure_satellite_body(migration) do + if migration.satellite_body == nil do + sql = File.read!(satellite_file_path(migration)) + %{migration | satellite_body: sql} + else + migration + end + end + + + def ensure_and_validate_original_sql(migration) do + with_body = ensure_original_body(migration) + {:ok, conn} = Exqlite.Sqlite3.open(":memory:") + case Exqlite.Sqlite3.execute(conn, with_body.original_body) do + :ok -> with_body + {:error, reason} -> + %{with_body | error: reason} + end + end + + def as_json_map(migration, with_body) do + with_satellite_body = ensure_satellite_body(migration) + metadata = get_satellite_metadata(with_satellite_body) + if with_body do + # At the moment just using elixir jason's default escaping of the SQL text - maybe switch to base64 if causes issues + # see here for json escaping https://www.ietf.org/rfc/rfc4627.txt + Map.merge(metadata, %{"body" => with_satellite_body.satellite_body, "encoding" => "escaped"}) + else + metadata + end + end + + def get_satellite_metadata(migration) do + with_satellite = ensure_satellite_body(migration) + get_body_metadata(with_satellite.satellite_body) + end + + def get_original_metadata(migration) do + with_original = ensure_original_body(migration) + get_body_metadata(with_original.original_body) + end + + defp get_body_metadata(body) do + regex = ~r/ElectricDB Migration[\s]*(.*?)[\s]*\*/ + matches = Regex.run(regex, body) + + if matches == nil do + {:error, "no header"} + else + case Jason.decode(List.last(matches)) do + {:ok, metadata} -> metadata["metadata"] + {:error, reason} -> {:error, reason} + end + end + end + + def file_header(migration, hash, title) do + case title do + nil -> + """ + /* + ElectricDB Migration + {"metadata": {"name": "#{migration.name}", "sha256": "#{hash}"}} + */ + """ + + _ -> + """ + /* + ElectricDB Migration + {"metadata": {"title": "#{title}", "name": "#{migration.name}", "sha256": "#{hash}"}} + */ + """ + end + end + + +end \ No newline at end of file diff --git a/lib/electric/migrations.ex b/lib/electric/migrations.ex index b1d0bc3..e599a6e 100644 --- a/lib/electric/migrations.ex +++ b/lib/electric/migrations.ex @@ -78,12 +78,18 @@ defmodule Electric.Migrations do case check_migrations_folder(options) do {:ok, migrations_folder} -> template = Map.get(options, :template, @satellite_template) - ordered_migration_paths(migrations_folder) |> add_triggers_to_migrations(template) + ordered_migrations(migrations_folder) |> add_triggers_to_migrations(template) + + if flags[:manifest] do write_manifest(migrations_folder) end + if flags[:json] do + write_bundle(migrations_folder) + end + if flags[:bundle] do write_js_bundle(migrations_folder) end @@ -144,13 +150,7 @@ defmodule Electric.Migrations do @doc false def write_manifest(src_folder) do manifest_path = Path.join(src_folder, @manifest_file_name) - - migration_names = - for migration_folder <- ordered_migration_paths(src_folder) do - Path.basename(migration_folder) - end - - manifest = Jason.encode!(%{"migrations" => migration_names}) |> Jason.Formatter.pretty_print() + manifest = create_bundle(src_folder, false) if File.exists?(manifest_path) do File.rm(manifest_path) @@ -161,7 +161,7 @@ defmodule Electric.Migrations do @doc false def write_bundle(src_folder) do - migrations = create_bundle(src_folder) + migrations = create_bundle(src_folder, true) bundle_path = Path.join(src_folder, @bundle_file_name) if File.exists?(bundle_path) do @@ -173,7 +173,7 @@ defmodule Electric.Migrations do @doc false def write_js_bundle(src_folder) do - migrations = create_bundle(src_folder) + migrations = create_bundle(src_folder, true) {result, _bindings} = Code.eval_quoted(@bundle_template, migrations: migrations) bundle_path = Path.join(src_folder, @js_bundle_file_name) @@ -184,157 +184,153 @@ defmodule Electric.Migrations do File.write!(bundle_path, result) end - defp ordered_migration_paths(src_folder) do + defp ordered_migrations(src_folder) do sql_file_paths = Path.join([src_folder, "*", @migration_file_name]) |> Path.wildcard() - migration_names = - for file_path <- sql_file_paths do + migration_names = for file_path <- sql_file_paths do Path.dirname(file_path) |> Path.basename() end for migration_name <- Enum.sort(migration_names) do - Path.join(src_folder, migration_name) + %Electric.Migration{name: migration_name, src_folder: src_folder} end end - defp create_bundle(src_folder) do - migrations = - for migration_folder <- ordered_migration_paths(src_folder) do - satellite_sql_path = Path.join(migration_folder, @satellite_file_name) - migration_text = File.read!(satellite_sql_path) - migration_name = Path.basename(migration_folder) - %{"name" => migration_name, "body" => migration_text} - end - + defp create_bundle(src_folder, with_body) do + migrations = all_migrations_as_maps(src_folder, with_body) Jason.encode!(%{"migrations" => migrations}) |> Jason.Formatter.pretty_print() end + defp all_migrations_as_maps(src_folder, with_body) do + for migration <- ordered_migrations(src_folder) do + Electric.Migration.as_json_map(migration, with_body) + end + end + + +# defp diff_migrations(server_migrations, src_folder) do +# +# missing_locally = for server_migration_info <- server_migrations if !File.exists?(Path.join([src_folder, server_migration_info["name"]])) do +# server_migration_info["name"] +# end +# +# if length(missing_locally) != 0 do +# {:missing_error, conflicted_migrations} +# else +# local_migrations = all_migrations_as_maps(src_folder, true) +# server_migrations_lookup = Enum.into(server_migrations, %{}, fn info -> {info["name"], info} end) +# +# migration_statuses = for local_migration <- local_migrations do +# migration_name = local_migration["name"] +# if Map.has_key?(server_migrations_lookup, migration_name) do +# server_migration_info = server_migrations_lookup[migration_name] +# if server_migration_info["sha256"] != local_migration_info["sha256"] do +# if server_migration_info["applied"] == true do +# {:conflicted, local_migration} +# else +# {:modified, local_migration} +# end +# else +# [:matches, local_migration] +# end +# else +# {:new, local_migration} +# end +# end +# +# conflicted_migrations = for {status, local_migration} <- local_migrations if status == :conflicted do +# local_migration +# end +# +# if length(conflicted_migrations) != 0 do +# {:conflict_error, conflicted_migrations} +# else +# new_migrations = for {status, local_migration} <- local_migrations if status != :conflicted do +# if status == :modified do +# IO.puts("The migration #{local_migration["name"]} will be changed on the server.") +# end +# local_migration +# end +# {:ok, new_migrations} +# end +# end +# end + defp calc_hash(with_triggers) do sha = :crypto.hash(:sha256, with_triggers) base16 = Base.encode16(sha) String.downcase(base16) end - defp get_metadata(file_path) do - case File.read(file_path) do - {:ok, body} -> - get_body_metadata(body) + + defp add_triggers_to_migrations(ordered_migrations, template) do - {:error, reason} -> - {:error, reason} + validated_migrations = for migration <- ordered_migrations do + Electric.Migration.ensure_and_validate_original_sql(migration) end - end - defp get_body_metadata(body) do - regex = ~r/ElectricDB Migration[\s]*(.*?)[\s]*\*/ - matches = Regex.run(regex, body) + failed_validation = Enum.filter(validated_migrations, fn migration -> migration.error != nil end) - if matches == nil do - {:error, "no header"} + if length(failed_validation) > 0 do + {:error, failed_validation} else - case Jason.decode(List.last(matches)) do - {:ok, metadata} -> metadata["metadata"] + try do + for {_migration, i} <- Enum.with_index(validated_migrations) do + subset_of_migrations = Enum.take(validated_migrations, i + 1) + # this is using a migration file path and all the migrations up to, an including this migration + case add_triggers_to_migration( + subset_of_migrations, + template + ) do + :ok -> :ok + {:error, reason} -> throw({:error, reason}) + end + end + catch {:error, reason} -> {:error, reason} end end end - defp file_header(hash, name, title) do - case title do - nil -> - """ - /* - ElectricDB Migration - {"metadata": {"name": "#{name}", "sha256": "#{hash}"}} - */ - """ - - _ -> - """ - /* - ElectricDB Migration - {"metadata": {"title": "#{title}", "name": "#{name}", "sha256": "#{hash}"}} - */ - """ - end - end - defp add_triggers_to_migrations(ordered_migration_paths, template) do - ## throwing tuples funky! - try do - ordered_migrations = - for migration_folder_path <- ordered_migration_paths do - migration_file_path = Path.join([migration_folder_path, @migration_file_name]) - - case File.read(migration_file_path) do - {:ok, sql} -> - case validate_sql(sql) do - :ok -> sql - {:error, reason} -> throw({:error, reason}) - end - - {:error, reason} -> - throw({:error, reason}) - end - end + @doc false + def add_triggers_to_migration(ordered_migrations, template) do - # needs to fail early so has to start at the first migration and go through - for {_migration_folder_path, i} <- Enum.with_index(ordered_migration_paths) do - subset_of_migrations = Enum.take(ordered_migrations, i + 1) - subset_of_migration_folders = Enum.take(ordered_migration_paths, i + 1) - - # this is using a migration file path and all the migrations up to, an including this migration - case add_triggers_to_migration_folder( - subset_of_migration_folders, - subset_of_migrations, - template - ) do - :ok -> :ok - {:error, reason} -> throw({:error, reason}) - end - end - catch - {:error, reason} -> {:error, reason} + migration = List.last(ordered_migrations) + + ordered_sql = for migration <- ordered_migrations do + migration.original_body end - end - @doc false - def add_triggers_to_migration_folder(ordered_folder_paths, ordered_migrations, template) do - migration_folder_path = List.last(ordered_folder_paths) - migration_name = Path.basename(migration_folder_path) - satellite_file_path = Path.join(migration_folder_path, @satellite_file_name) - with_triggers = add_triggers_to_last_migration(ordered_migrations, template) + with_triggers = add_triggers_to_last_sql(ordered_sql, template) migration_fingerprint = if length(ordered_migrations) > 1 do - previous_satellite_migration_file_path = - Path.join(Enum.at(ordered_folder_paths, -2), @satellite_file_name) - - previous_metadata = get_metadata(previous_satellite_migration_file_path) - "#{with_triggers}#{previous_metadata["sha256"]}" + previous_migration = Enum.at(ordered_migrations, -2) + previous_metadata = Electric.Migration.get_satellite_metadata(previous_migration) + "#{migration.original_body}#{previous_metadata["sha256"]}" else - with_triggers + migration.original_body end hash = calc_hash(migration_fingerprint) + satellite_file_path = Electric.Migration.satellite_file_path(migration) if File.exists?(satellite_file_path) do - metadata = get_metadata(satellite_file_path) - + metadata = Electric.Migration.get_satellite_metadata(migration) if metadata["sha256"] != hash do - IO.puts("Warning: The migration #{migration_name} has been modified.") + IO.puts("Warning: The migration #{migration.name} has been modified.") File.rm(satellite_file_path) end end if !File.exists?(satellite_file_path) do header = - case get_body_metadata(List.last(ordered_migrations)) do + case Electric.Migration.get_original_metadata(migration) do {:error, _reason} -> - file_header(hash, migration_name, nil) - + Electric.Migration.file_header(migration, hash, nil) existing_metadata -> - file_header(hash, migration_name, existing_metadata["title"]) + Electric.Migration.file_header(migration, hash, existing_metadata["title"]) end File.write!(satellite_file_path, header <> with_triggers) @@ -343,26 +339,18 @@ defmodule Electric.Migrations do :ok end end + @doc false - def add_triggers_to_last_migration(ordered_migrations, template) do + def add_triggers_to_last_sql(ordered_sql, template) do + # adds triggers for all tables to the end of the last migration - table_infos = all_tables_info(ordered_migrations) - sql_in = List.last(ordered_migrations) - is_init = length(ordered_migrations) == 1 + table_infos = all_tables_info(ordered_sql) + sql_in = List.last(ordered_sql) + is_init = length(ordered_sql) == 1 template_all_the_things(sql_in, table_infos, template, is_init) end - @doc false - def validate_sql(sql_in) do - {:ok, conn} = Exqlite.Sqlite3.open(":memory:") - - case Exqlite.Sqlite3.execute(conn, sql_in) do - :ok -> :ok - {:error, reason} -> {:error, reason} - end - end - @doc false def created_table_names(sql_in) do info = all_tables_info(sql_in) @@ -371,6 +359,7 @@ defmodule Electric.Migrations do @doc false def all_tables_info(all_migrations) do + namespace = "main" # get all the table names {:ok, conn} = Exqlite.Sqlite3.open(":memory:") diff --git a/test/migration_test.exs b/test/migration_test.exs index 6517085..8455a6c 100644 --- a/test/migration_test.exs +++ b/test/migration_test.exs @@ -13,7 +13,10 @@ defmodule MigrationsTest do ); """ - assert Electric.Migrations.validate_sql(sql) == :ok + migration = %Electric.Migration{name: "test1", original_body: sql} +# IO.inspect(migration) + + assert Electric.Migration.ensure_and_validate_original_sql(migration).error == nil end test "tests nonsense SQL fails" do @@ -24,7 +27,9 @@ defmodule MigrationsTest do SOME BOLLOCKS; """ - assert Electric.Migrations.validate_sql(sql) == {:error, "near \"SOME\": syntax error"} + migration = %Electric.Migration{name: "test1", original_body: sql} + + assert Electric.Migration.ensure_and_validate_original_sql(migration).error == "near \"SOME\": syntax error" end end @@ -69,7 +74,7 @@ defmodule MigrationsTest do --ADD A TRIGGER FOR main.fish; """ - assert Electric.Migrations.add_triggers_to_last_migration([sql], @trigger_template) == + assert Electric.Migrations.add_triggers_to_last_sql([sql], @trigger_template) == expected end @@ -80,16 +85,26 @@ defmodule MigrationsTest do ); """ +# migration = %Electric.Migration{name: "test1", original_body: sql} + sql = - Electric.Migrations.add_triggers_to_last_migration( + Electric.Migrations.add_triggers_to_last_sql( [sql], Electric.Migrations.get_template() ) - assert Electric.Migrations.validate_sql(sql) == :ok + assert is_valid_sql(sql) == :ok end end + + def is_valid_sql(sql) do + {:ok, conn} = Exqlite.Sqlite3.open(":memory:") + Exqlite.Sqlite3.execute(conn, sql) + end + + + describe "Triggers works as expected" do test "templating" do original_sql = """ @@ -284,7 +299,7 @@ defmodule MigrationsTest do """ sql = - Electric.Migrations.add_triggers_to_last_migration( + Electric.Migrations.add_triggers_to_last_sql( [sql], Electric.Migrations.get_template() ) @@ -335,7 +350,7 @@ defmodule MigrationsTest do """ sql = - Electric.Migrations.add_triggers_to_last_migration( + Electric.Migrations.add_triggers_to_last_sql( [sql], Electric.Migrations.get_template() ) @@ -368,7 +383,7 @@ defmodule MigrationsTest do """ sql = - Electric.Migrations.add_triggers_to_last_migration( + Electric.Migrations.add_triggers_to_last_sql( [sql], Electric.Migrations.get_template() ) @@ -418,13 +433,13 @@ defmodule MigrationsTest do """ sql_out1 = - Electric.Migrations.add_triggers_to_last_migration( + Electric.Migrations.add_triggers_to_last_sql( [sql1], Electric.Migrations.get_template() ) sql_out2 = - Electric.Migrations.add_triggers_to_last_migration( + Electric.Migrations.add_triggers_to_last_sql( [sql1, sql2], Electric.Migrations.get_template() ) @@ -634,7 +649,18 @@ defmodule MigrationsFileTest do assert File.exists?(Path.join([migrations_path, "manifest.json"])) end - test "test can build with bundle" do + test "test can build with json bundle" do + temp = temp_folder() + migrations_path = Path.join([temp, "migrations"]) + init_and_add_migration(temp) + + {:success, _msg} = + Electric.Migrations.build_migrations(%{:bundle => true}, %{:migrations => migrations_path}) + + assert File.exists?(Path.join([migrations_path, "index.js"])) + end + + test "test can build with js bundle" do temp = temp_folder() migrations_path = Path.join([temp, "migrations"]) init_and_add_migration(temp) @@ -686,16 +712,17 @@ defmodule MigrationsFileTest do {:ok, migration} = File.read(migration_file_path) - Electric.Migrations.add_triggers_to_migration_folder( - [migration_folder], - [migration], + m = %Electric.Migration{name: migration_name, original_body: migration, src_folder: Path.join([temp, "migrations"])} + + Electric.Migrations.add_triggers_to_migration( + [m], @trigger_template ) expected = """ /* ElectricDB Migration - {"metadata": {"name": "#{migration_name}", "sha256": "7aeaa635b94e7def04a4f9dc5ac730353d4c45cf65aa2574cee58e1154d3ed43"}} + {"metadata": {"name": "#{migration_name}", "sha256": "0f0eb722f4d27646a93dcd6fd645b16b158cf1e40455544e158ef71972226e00"}} */ CREATE TABLE IF NOT EXISTS items ( value TEXT PRIMARY KEY @@ -732,7 +759,7 @@ defmodule MigrationsFileTest do expected = """ /* ElectricDB Migration - {"metadata": {"name": "#{migration_name}", "sha256": "7aeaa635b94e7def04a4f9dc5ac730353d4c45cf65aa2574cee58e1154d3ed43"}} + {"metadata": {"name": "#{migration_name}", "sha256": "0f0eb722f4d27646a93dcd6fd645b16b158cf1e40455544e158ef71972226e00"}} */ CREATE TABLE IF NOT EXISTS items ( value TEXT PRIMARY KEY @@ -767,11 +794,24 @@ defmodule MigrationsFileTest do manifest_path = Path.join([migrations_folder, "manifest.json"]) - Electric.Migrations.write_manifest(migrations_folder) + Electric.Migrations.build_migrations( + %{}, + %{ + :migrations => migrations_folder + } + ) + Electric.Migrations.write_manifest(migrations_folder) manifest = Jason.decode!(File.read!(manifest_path)) - assert manifest == %{"migrations" => [migration_name, migration_name_2]} + expected = %{ + "migrations" => [ + %{"name" => migration_name, "sha256" => "0f0eb722f4d27646a93dcd6fd645b16b158cf1e40455544e158ef71972226e00"}, + %{"name" => migration_name_2, "sha256" => "3da01b8ef4f3845fdc3cb3619c6ba75538988d6eec816f3734a7c36383874266"} + ] + } + + assert manifest == expected end test "creates a bundle" do @@ -818,13 +858,17 @@ defmodule MigrationsFileTest do "migrations" => [ %{ "body" => - "/*\nElectricDB Migration\n{\"metadata\": {\"name\": \"#{migration_name}\", \"sha256\": \"7aeaa635b94e7def04a4f9dc5ac730353d4c45cf65aa2574cee58e1154d3ed43\"}}\n*/\nCREATE TABLE IF NOT EXISTS items (\n value TEXT PRIMARY KEY\n);\n--ADD A TRIGGER FOR main.items;\n", - "name" => migration_name + "/*\nElectricDB Migration\n{\"metadata\": {\"name\": \"#{migration_name}\", \"sha256\": \"0f0eb722f4d27646a93dcd6fd645b16b158cf1e40455544e158ef71972226e00\"}}\n*/\nCREATE TABLE IF NOT EXISTS items (\n value TEXT PRIMARY KEY\n);\n--ADD A TRIGGER FOR main.items;\n", + "name" => migration_name, + "encoding" => "escaped", + "sha256" => "0f0eb722f4d27646a93dcd6fd645b16b158cf1e40455544e158ef71972226e00" }, %{ "body" => - "/*\nElectricDB Migration\n{\"metadata\": {\"name\": \"#{migration_name_2}\", \"sha256\": \"4b29f7cab89d0f0e37985280f9d311d5bf1d43132fa0183ed865a3a9b1428aa7\"}}\n*/\nCREATE TABLE IF NOT EXISTS cat (\n value TEXT PRIMARY KEY\n);\n--ADD A TRIGGER FOR main.cat;\n\n--ADD A TRIGGER FOR main.items;\n", - "name" => migration_name_2 + "/*\nElectricDB Migration\n{\"metadata\": {\"name\": \"#{migration_name_2}\", \"sha256\": \"3da01b8ef4f3845fdc3cb3619c6ba75538988d6eec816f3734a7c36383874266\"}}\n*/\nCREATE TABLE IF NOT EXISTS cat (\n value TEXT PRIMARY KEY\n);\n--ADD A TRIGGER FOR main.cat;\n\n--ADD A TRIGGER FOR main.items;\n", + "name" => migration_name_2, + "encoding" => "escaped", + "sha256" => "3da01b8ef4f3845fdc3cb3619c6ba75538988d6eec816f3734a7c36383874266" } ] } From bb5b0feaedb02a08a7717954c1966bc189ca4ca9 Mon Sep 17 00:00:00 2001 From: Paul Harter Date: Wed, 28 Sep 2022 16:46:29 +0100 Subject: [PATCH 02/18] generates postgres too --- README.md | 5 + lib/electric/migrations/generation.ex | 188 ++++++++++ lib/electric/{ => migrations}/migration.ex | 34 +- lib/electric/{ => migrations}/migrations.ex | 249 +++---------- lib/electric/migrations/parse.ex | 258 +++++++++++++ lib/electric/migrations/triggers.ex | 33 ++ lib/electric/migrations/validation.ex | 29 ++ test/migration_generation_test.exs | 154 ++++++++ test/migration_parse_test.exs | 384 ++++++++++++++++++++ test/migration_test.exs | 186 +++------- test/support/migration.sql | 2 +- test/support/migration2.sql | 2 +- 12 files changed, 1175 insertions(+), 349 deletions(-) create mode 100644 lib/electric/migrations/generation.ex rename lib/electric/{ => migrations}/migration.ex (83%) rename lib/electric/{ => migrations}/migrations.ex (51%) create mode 100644 lib/electric/migrations/parse.ex create mode 100644 lib/electric/migrations/triggers.ex create mode 100644 lib/electric/migrations/validation.ex create mode 100644 test/migration_generation_test.exs create mode 100644 test/migration_parse_test.exs diff --git a/README.md b/README.md index ba66fe2..b19d958 100644 --- a/README.md +++ b/README.md @@ -57,3 +57,8 @@ And run it: This creates a binary at `./dist/electric` and needs to be run on the OS you are targetting. See also https://github.com/bake-bake-bake/bakeware#static-compiling-openssl-into-erlang-distribution + + +## Migration commands + + diff --git a/lib/electric/migrations/generation.ex b/lib/electric/migrations/generation.ex new file mode 100644 index 0000000..a43f639 --- /dev/null +++ b/lib/electric/migrations/generation.ex @@ -0,0 +1,188 @@ +defmodule Electric.Migrations.Generation do + @moduledoc """ + + """ + + def postgres_for_ordered_migrations(ordered_migrations) do + {before_ast, after_ast} = before_and_after_ast(ordered_migrations) + get_postgres_for_infos(before_ast, after_ast) + end + + defp before_and_after_ast(migration_set) do + after_ast = Electric.Migrations.Parse.sql_ast_from_migration_set(migration_set) + all_but_last_migration_set = Enum.take(migration_set, Enum.count(migration_set) - 1) + before_ast = Electric.Migrations.Parse.sql_ast_from_migration_set(all_but_last_migration_set) + {before_ast, after_ast} + end + + defp get_postgres_for_infos(before_ast, after_ast) do + get_sql_for_infos(before_ast, after_ast, "postgres") + + end + + defp get_sqlite_for_infos(before_ast, after_ast) do + get_sql_for_infos(before_ast, after_ast, "sqlite") + + end + + defp get_sql_for_infos(before_ast, after_ast, flavour) do + + statements = for change <- table_changes(before_ast, after_ast) do + case change do + {nil, table_after} -> + ## https://www.sqlite.org/syntax/column-constraint.html + ## %{cid: 0, dflt_value: nil, name: "id", notnull: 0, pk: 1, type: "INTEGER"} + + column_definitions = for {column_id, column_info} <- table_after.column_infos do + "\n " <> column_def_sql_from_info(column_info, flavour) + end + + foreign_key_clauses = for foreign_key_info <- table_after.foreign_keys_info do + "\n " <> foreign_key_sql_from_info(foreign_key_info, flavour) + end + + columns_and_keys = column_definitions ++ foreign_key_clauses + + case flavour do + "postgres" -> + "\nCREATE TABLE #{table_after.namespace}.#{table_after.table_name} (#{Enum.join(columns_and_keys, ",")});\n" + "sqlite" -> + "\nCREATE TABLE #{table_after.namespace}.#{table_after.table_name} (#{Enum.join(columns_and_keys, ",")})STRICT;\n" + end + + {table_before, nil} -> + "DROP TABLE #{table_before.namespace}.#{table_before.table_name};\n" + {table_before, table_after} -> + + ## add columns + added_colums_lines = for {column_id, column_info} <- table_after.column_infos, !Map.has_key?(table_before.column_infos, column_id) do + "ALTER TABLE #{table_after.namespace}.#{table_after.table_name} ADD COLUMN #{column_def_sql_from_info(column_info, flavour)};\n" + end + + ## delete columns + dropped_colums_lines = for {column_id, column_info} <- table_before.column_infos, !Map.has_key?(table_after.column_infos, column_id) do + "ALTER TABLE #{table_before.namespace}.#{table_before.table_name} DROP COLUMN #{column_info.name};\n" + end + + ## rename columns + rename_colums_lines = for {column_id, column_info} <- table_after.column_infos, Map.has_key?(table_before.column_infos, column_id) && column_info.name != table_before.column_infos[column_id].name do + "ALTER TABLE #{table_after.namespace}.#{table_after.table_name} RENAME COLUMN #{table_before.column_infos[column_id].name} TO #{column_info.name};\n" + end + all_change_lines = added_colums_lines ++ dropped_colums_lines ++ rename_colums_lines + Enum.join(all_change_lines, " ") + end + end + Enum.join(statements, "") + end + + defp table_changes(before_ast, after_ast) do + if before_ast == nil do + for {table_name, table_info} <- after_ast do + {nil, table_info} + end + else + new_and_changed = for {table_name, table_info} <- after_ast, table_info != before_ast[table_name] do + {before_ast[table_name], table_info} + end + dropped = for {table_name, table_info} <- before_ast, !Map.has_key?(after_ast, table_name) do + {table_info, nil} + end + new_and_changed ++ dropped + end + end + + defp foreign_key_sql_from_info(key_info, flavour) do + + # DEFERRABLE + + # %{from: "daddy", id: 0, match: "NONE", on_delete: "NO ACTION", on_update: "NO ACTION", seq: 0, table: "parent", to: "id"} + + elements = [] + + # force postgres to MATCH SIMPLE as sqlite always does this + elements = case flavour do + "postgres" -> + ["MATCH SIMPLE" | elements] + _ -> + elements + end + + elements = if key_info.on_delete != "NO ACTION" do + ["ON DELETE #{key_info.on_delete}" | elements] + else + elements + end + + elements = if key_info.on_update != "NO ACTION" do + ["ON UPDATE #{key_info.on_update}" | elements] + else + elements + end + + elements = ["FOREIGN KEY(#{key_info.from}) REFERENCES #{key_info.table}(#{key_info.to})" | elements] + + Enum.join(elements, " ") + end + + + defp column_def_sql_from_info(column_info, flavour) do + + # DONE + # name + # type + + # CONSTRAINTs done + # PRIMARY KEY + # NOT NULL + # DEFAULT + # PRIMARY KEY ASC + # PRIMARY KEY DESC + # UNIQUE + + # CONSTRAINTs TODO + + # AUTOINCREMENT disallow + # COLLATE read from sql + # GENERATED pass through somehow read from sql? + # CHECK pass through somehow read from sql? + + # PRIMARY KEY conflict-clause this is sqlite stuff + # NOT NULL conflict-clause this is sqlite stuff + + type_lookup = case flavour do + "postgres" -> + %{ "TEXT" => "text", + "NUMERIC" => "numeric", + "INTEGER" => "integer", + "REAL" => "real", + "BLOB" => "blob" + } + _ -> + %{ "TEXT" => "TEXT", + "NUMERIC" => "NUMERIC", + "INTEGER" => "INTEGER", + "REAL" => "REAL", + "BLOB" => "BLOB" + } + end + + elements = [] + elements = if column_info.dflt_value != nil do + ["DEFAULT #{column_info.dflt_value}" | elements] + else + elements + end + elements = if column_info.unique do ["UNIQUE" | elements] else elements end + elements = if column_info.notnull != 0 && column_info.pk == 0 do ["NOT NULL" | elements] else elements end + elements = if column_info.pk != 0 do + ["PRIMARY KEY#{ if column_info.pk_desc do " DESC" else "" end}" | elements] + else + elements + end + elements = [type_lookup[column_info.type] | elements] + elements = [column_info.name | elements] + Enum.join(elements, " ") + end + + +end \ No newline at end of file diff --git a/lib/electric/migration.ex b/lib/electric/migrations/migration.ex similarity index 83% rename from lib/electric/migration.ex rename to lib/electric/migrations/migration.ex index 502ab7b..37bb0cb 100644 --- a/lib/electric/migration.ex +++ b/lib/electric/migrations/migration.ex @@ -3,13 +3,12 @@ defmodule Electric.Migration do """ - @migration_file_name "migration.sql" @satellite_file_name "satellite.sql" + @postgres_file_name "postgres.sql" @enforce_keys [:name] defstruct name: "noname", src_folder: nil, original_body: nil, satellite_body: nil, error: nil - def original_file_path(migration) do Path.join([migration.src_folder, migration.name, @migration_file_name]) end @@ -18,6 +17,10 @@ defmodule Electric.Migration do Path.join([migration.src_folder, migration.name, @satellite_file_name]) end + def postgres_file_path(migration) do + Path.join([migration.src_folder, migration.name, @postgres_file_name]) + end + def ensure_original_body(migration) do if migration.original_body == nil do sql = File.read!(original_file_path(migration)) @@ -36,20 +39,23 @@ defmodule Electric.Migration do end end - - def ensure_and_validate_original_sql(migration) do - with_body = ensure_original_body(migration) - {:ok, conn} = Exqlite.Sqlite3.open(":memory:") - case Exqlite.Sqlite3.execute(conn, with_body.original_body) do - :ok -> with_body - {:error, reason} -> - %{with_body | error: reason} - end - end +# def ensure_and_validate_original_sql(migration) do +# with_body = ensure_original_body(migration) +# {:ok, conn} = Exqlite.Sqlite3.open(":memory:") +# +# case Exqlite.Sqlite3.execute(conn, with_body.original_body) do +# :ok -> +# with_body +# +# {:error, reason} -> +# %{with_body | error: reason} +# end +# end def as_json_map(migration, with_body) do with_satellite_body = ensure_satellite_body(migration) metadata = get_satellite_metadata(with_satellite_body) + if with_body do # At the moment just using elixir jason's default escaping of the SQL text - maybe switch to base64 if causes issues # see here for json escaping https://www.ietf.org/rfc/rfc4627.txt @@ -102,6 +108,4 @@ defmodule Electric.Migration do """ end end - - -end \ No newline at end of file +end diff --git a/lib/electric/migrations.ex b/lib/electric/migrations/migrations.ex similarity index 51% rename from lib/electric/migrations.ex rename to lib/electric/migrations/migrations.ex index e599a6e..df6669f 100644 --- a/lib/electric/migrations.ex +++ b/lib/electric/migrations/migrations.ex @@ -23,7 +23,6 @@ defmodule Electric.Migrations do case root_directory = Map.get(options, :dir) do nil -> "migrations" - _ -> if Path.basename(root_directory) == "migrations" do root_directory @@ -78,8 +77,8 @@ defmodule Electric.Migrations do case check_migrations_folder(options) do {:ok, migrations_folder} -> template = Map.get(options, :template, @satellite_template) - ordered_migrations(migrations_folder) |> add_triggers_to_migrations(template) - + migration_set = ordered_migrations(migrations_folder) + add_triggers_to_migrations(migration_set, template) if flags[:manifest] do @@ -187,7 +186,8 @@ defmodule Electric.Migrations do defp ordered_migrations(src_folder) do sql_file_paths = Path.join([src_folder, "*", @migration_file_name]) |> Path.wildcard() - migration_names = for file_path <- sql_file_paths do + migration_names = + for file_path <- sql_file_paths do Path.dirname(file_path) |> Path.basename() end @@ -207,69 +207,19 @@ defmodule Electric.Migrations do end end - -# defp diff_migrations(server_migrations, src_folder) do -# -# missing_locally = for server_migration_info <- server_migrations if !File.exists?(Path.join([src_folder, server_migration_info["name"]])) do -# server_migration_info["name"] -# end -# -# if length(missing_locally) != 0 do -# {:missing_error, conflicted_migrations} -# else -# local_migrations = all_migrations_as_maps(src_folder, true) -# server_migrations_lookup = Enum.into(server_migrations, %{}, fn info -> {info["name"], info} end) -# -# migration_statuses = for local_migration <- local_migrations do -# migration_name = local_migration["name"] -# if Map.has_key?(server_migrations_lookup, migration_name) do -# server_migration_info = server_migrations_lookup[migration_name] -# if server_migration_info["sha256"] != local_migration_info["sha256"] do -# if server_migration_info["applied"] == true do -# {:conflicted, local_migration} -# else -# {:modified, local_migration} -# end -# else -# [:matches, local_migration] -# end -# else -# {:new, local_migration} -# end -# end -# -# conflicted_migrations = for {status, local_migration} <- local_migrations if status == :conflicted do -# local_migration -# end -# -# if length(conflicted_migrations) != 0 do -# {:conflict_error, conflicted_migrations} -# else -# new_migrations = for {status, local_migration} <- local_migrations if status != :conflicted do -# if status == :modified do -# IO.puts("The migration #{local_migration["name"]} will be changed on the server.") -# end -# local_migration -# end -# {:ok, new_migrations} -# end -# end -# end - defp calc_hash(with_triggers) do sha = :crypto.hash(:sha256, with_triggers) base16 = Base.encode16(sha) String.downcase(base16) end - defp add_triggers_to_migrations(ordered_migrations, template) do - - validated_migrations = for migration <- ordered_migrations do - Electric.Migration.ensure_and_validate_original_sql(migration) - end - - failed_validation = Enum.filter(validated_migrations, fn migration -> migration.error != nil end) + validated_migrations = + for migration <- ordered_migrations do + Electric.Migration.ensure_original_body(migration) + end + failed_validation = + Enum.filter(validated_migrations, fn migration -> migration.error != nil end) if length(failed_validation) > 0 do {:error, failed_validation} @@ -277,6 +227,7 @@ defmodule Electric.Migrations do try do for {_migration, i} <- Enum.with_index(validated_migrations) do subset_of_migrations = Enum.take(validated_migrations, i + 1) + # this is using a migration file path and all the migrations up to, an including this migration case add_triggers_to_migration( subset_of_migrations, @@ -294,156 +245,56 @@ defmodule Electric.Migrations do @doc false - def add_triggers_to_migration(ordered_migrations, template) do - - migration = List.last(ordered_migrations) - - ordered_sql = for migration <- ordered_migrations do - migration.original_body - end - - with_triggers = add_triggers_to_last_sql(ordered_sql, template) - - migration_fingerprint = - if length(ordered_migrations) > 1 do - previous_migration = Enum.at(ordered_migrations, -2) - previous_metadata = Electric.Migration.get_satellite_metadata(previous_migration) - "#{migration.original_body}#{previous_metadata["sha256"]}" - else - migration.original_body - end - - hash = calc_hash(migration_fingerprint) - satellite_file_path = Electric.Migration.satellite_file_path(migration) - - if File.exists?(satellite_file_path) do - metadata = Electric.Migration.get_satellite_metadata(migration) - if metadata["sha256"] != hash do - IO.puts("Warning: The migration #{migration.name} has been modified.") - File.rm(satellite_file_path) - end - end - - if !File.exists?(satellite_file_path) do - header = - case Electric.Migration.get_original_metadata(migration) do - {:error, _reason} -> - Electric.Migration.file_header(migration, hash, nil) - existing_metadata -> - Electric.Migration.file_header(migration, hash, existing_metadata["title"]) + def add_triggers_to_migration(migration_set, template) do + migration = List.last(migration_set) + + case Electric.Migrations.Triggers.add_triggers_to_last_migration(migration_set, template) do + {:error, reasons} -> + {:error, reasons} + with_triggers -> + migration_fingerprint = + if length(migration_set) > 1 do + previous_migration = Enum.at(migration_set, -2) + previous_metadata = Electric.Migration.get_satellite_metadata(previous_migration) + "#{migration.original_body}#{previous_metadata["sha256"]}" + else + migration.original_body end - File.write!(satellite_file_path, header <> with_triggers) - :ok - else - :ok - end - end - - - @doc false - def add_triggers_to_last_sql(ordered_sql, template) do - - # adds triggers for all tables to the end of the last migration - table_infos = all_tables_info(ordered_sql) - sql_in = List.last(ordered_sql) - is_init = length(ordered_sql) == 1 - template_all_the_things(sql_in, table_infos, template, is_init) - end - - @doc false - def created_table_names(sql_in) do - info = all_tables_info(sql_in) - Map.keys(info) - end + postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations(migration_set) - @doc false - def all_tables_info(all_migrations) do - - namespace = "main" - # get all the table names - {:ok, conn} = Exqlite.Sqlite3.open(":memory:") + hash = calc_hash(migration_fingerprint) + satellite_file_path = Electric.Migration.satellite_file_path(migration) + postgres_file_path = Electric.Migration.postgres_file_path(migration) - for migration <- all_migrations do - :ok = Exqlite.Sqlite3.execute(conn, migration) - end + if File.exists?(satellite_file_path) do + metadata = Electric.Migration.get_satellite_metadata(migration) - {:ok, statement} = - Exqlite.Sqlite3.prepare( - conn, - "SELECT name, sql FROM sqlite_master WHERE type='table' AND name!='_electric_oplog';" - ) - - info = get_rows_while(conn, statement, []) - :ok = Exqlite.Sqlite3.release(conn, statement) - - # for each table - infos = - for [table_name, _sql] <- info do - # column names - {:ok, info_statement} = Exqlite.Sqlite3.prepare(conn, "PRAGMA table_info(#{table_name});") - columns = Enum.reverse(get_rows_while(conn, info_statement, [])) - - column_names = - for [_cid, name, _type, _notnull, _dflt_value, _pk] <- columns do - name - end + if metadata["sha256"] != hash do + IO.puts("Warning: The migration #{migration.name} has been modified.") + File.rm(satellite_file_path) + File.rm(postgres_file_path) + end + end - # private keys columns - private_key_column_names = - for [_cid, name, _type, _notnull, _dflt_value, pk] when pk == 1 <- columns do - name - end - # foreign keys - {:ok, foreign_statement} = - Exqlite.Sqlite3.prepare(conn, "PRAGMA foreign_key_list(#{table_name});") - foreign_keys = get_rows_while(conn, foreign_statement, []) + if !File.exists?(satellite_file_path) do + header = + case Electric.Migration.get_original_metadata(migration) do + {:error, _reason} -> + Electric.Migration.file_header(migration, hash, nil) - foreign_keys = - for [_a, _b, parent_table, child_key, parent_key, _c, _d, _e] <- foreign_keys do - %{ - :child_key => child_key, - :parent_key => parent_key, - :table => "#{namespace}.#{parent_table}" - } + existing_metadata -> + Electric.Migration.file_header(migration, hash, existing_metadata["title"]) end - %{ - :table_name => table_name, - :columns => column_names, - :namespace => namespace, - :primary => private_key_column_names, - :foreign_keys => foreign_keys - } + File.write!(satellite_file_path, header <> with_triggers) + File.write!(postgres_file_path, header <> postgres_version) + :ok + else + :ok end - - Enum.into(infos, %{}, fn info -> {"#{namespace}.#{info.table_name}", info} end) - end - - defp get_rows_while(conn, statement, rows) do - case Exqlite.Sqlite3.step(conn, statement) do - {:row, row} -> - get_rows_while(conn, statement, [row | rows]) - - :done -> - rows end - end - - @doc false - def template_all_the_things(original_sql, tables, template, is_init) do - ## strip the old header - patched_sql = String.replace(original_sql, ~r/\A\/\*((?s).*)\*\/\n/, "") - ## template - {result, _bindings} = - Code.eval_quoted(template, - is_init: is_init, - original_sql: patched_sql, - tables: tables - ) - - result - end + end #function end diff --git a/lib/electric/migrations/parse.ex b/lib/electric/migrations/parse.ex new file mode 100644 index 0000000..e52f90c --- /dev/null +++ b/lib/electric/migrations/parse.ex @@ -0,0 +1,258 @@ +defmodule Electric.Migrations.Parse do + @moduledoc """ + hello + """ + + @doc false + def sql_ast_from_migration_set(migrations) do + case ast_from_ordered_migrations(migrations) do + {ast, []} -> + ast + {ast, errors} -> + {:error, errors} + end + end + + + @doc false + def ast_from_ordered_migrations(migrations) do + + namespace = "main" + # get all the table names + {:ok, conn} = Exqlite.Sqlite3.open(":memory:") + + sql_errors = Enum.flat_map(migrations, fn migration -> + case Exqlite.Sqlite3.execute(conn, migration.original_body) do + {:error, reason} -> ["In migration #{migration.name} SQL error: #{reason}"] + :ok -> [] + end + end) + + if length(sql_errors) > 0 do + {:error, sql_errors} + else + index_info = all_index_info_from_connection(conn) + + {:ok, statement} = + Exqlite.Sqlite3.prepare( + conn, + "SELECT * FROM sqlite_master WHERE type='table' AND name!='_electric_oplog';" + ) + + info = get_rows_while(conn, statement, []) + :ok = Exqlite.Sqlite3.release(conn, statement) + + # for each table + infos = + for [type, name, tbl_name, rootpage, sql] <- info do + + validation_fails = check_sql(tbl_name, sql) + + # column names + {:ok, info_statement} = Exqlite.Sqlite3.prepare(conn, "PRAGMA table_info(#{tbl_name});") + columns = Enum.reverse(get_rows_while(conn, info_statement, [])) + + column_names = + for [_cid, name, _type, _notnull, _dflt_value, _pk] <- columns do + name + end + + column_infos = + for [cid, name, type, notnull, dflt_value, pk] <- columns, into: %{} do + {cid, %{:cid => cid, + :name => name, + :type => type, + :notnull => notnull, + :unique => is_unique(name, index_info["#{namespace}.#{tbl_name}"]), + :pk_desc => is_primary_desc(name, index_info["#{namespace}.#{tbl_name}"]), + :dflt_value => dflt_value, + :pk => pk}} + end + + # private keys columns + private_key_column_names = + for [_cid, name, _type, _notnull, _dflt_value, pk] when pk == 1 <- columns do + name + end + + # foreign keys + {:ok, foreign_statement} = Exqlite.Sqlite3.prepare(conn, "PRAGMA foreign_key_list(#{tbl_name});") + foreign_keys_rows = get_rows_while(conn, foreign_statement, []) + + foreign_keys = + for [id, seq, table, from, to, on_update, on_delete, match] <- foreign_keys_rows do + %{ + :child_key => from, + :parent_key => to, + :table => "#{namespace}.#{table}" + } + end + + foreign_keys_info = + for [id, seq, table, from, to, on_update, on_delete, match] <- foreign_keys_rows do + %{ + :id => id, + :seq => seq, + :table => table, + :from => from, + :to => to, + :on_update => on_update, + :on_delete => on_delete, + :match => match + } + end + + %{ + :table_name => tbl_name, + :table_info => %{type: type, name: name, tbl_name: tbl_name, rootpage: rootpage, sql: sql}, + :columns => column_names, + :namespace => namespace, + :primary => private_key_column_names, + :foreign_keys => foreign_keys, + :column_infos => column_infos, + :foreign_keys_info => foreign_keys_info, + :validation_fails => validation_fails + } + end + + ast = Enum.into(infos, %{}, fn info -> {"#{namespace}.#{info.table_name}", info} end) + validation_fails = for info <- infos, length(info.validation_fails) > 0 do + info.validation_fails + end + {ast, List.flatten(validation_fails)} + end + end + + + def check_sql(table_name, sql) do + validation_fails = [] + lower = String.downcase(sql) + validation_fails = if !String.contains?(lower, "strict") do + ["The table #{table_name} is not STRICT. Add the STRICT option at the end of the create table statement" | validation_fails] + else + validation_fails + end + + validation_fails = if !String.contains?(lower, "without rowid") do + ["The table #{table_name} is not WITHOUT ROWID. Add the WITHOUT ROWID option at the end of the create table statement and make sure you also specify a primary key" | validation_fails] + else + validation_fails + end + end + + + def all_index_info(all_migrations) do + + namespace = "main" + # get all the table names + {:ok, conn} = Exqlite.Sqlite3.open(":memory:") + + for migration <- all_migrations do + :ok = Exqlite.Sqlite3.execute(conn, migration) + end + all_index_info_from_connection(conn) + + end + + + defp all_index_info_from_connection(conn) do + + namespace = "main" + + {:ok, statement} = + Exqlite.Sqlite3.prepare( + conn, + "SELECT * FROM sqlite_master WHERE type='index';" + ) + + info = get_rows_while(conn, statement, []) + :ok = Exqlite.Sqlite3.release(conn, statement) + + # for each table + infos = + for [type, name, tbl_name, rootpage, sql] <- info, into: %{} do + # column names + {:ok, info_statement} = Exqlite.Sqlite3.prepare(conn, "PRAGMA index_list(#{tbl_name});") + indexes = Enum.reverse(get_rows_while(conn, info_statement, [])) + + index_infos = + for [seq, name, unique, origin, partial] <- indexes, into: %{} do + + {:ok, col_info_statement} = Exqlite.Sqlite3.prepare(conn, "PRAGMA index_xinfo(#{name});") + index_columns = Enum.reverse(get_rows_while(conn, col_info_statement, [])) + + index_column_infos = for [seqno, cid, name, desc, coll, key] <- index_columns do + %{ + :seqno => seqno, + :cid => cid, + :name => name, + :desc => desc, + :coll => coll, + :key => key + } + end + + {seq, %{ + :seq => seq, + :name => name, + :unique => unique, + :origin => origin, + :partial => partial, + :columns => index_column_infos + }} + end + {"main.#{tbl_name}", index_infos} + end + end + + + defp get_rows_while(conn, statement, rows) do + case Exqlite.Sqlite3.step(conn, statement) do + {:row, row} -> + get_rows_while(conn, statement, [row | rows]) + :done -> + rows + end + end + + defp is_unique(column_name, indexes) do + case indexes do + nil -> + false + _ -> + matching_unique_indexes = for {seq, info} <- indexes do + case info.origin do + "u" -> + cols = for key_column <- info.columns do + key_column.key == 1 && key_column.name == column_name + end + Enum.any?(cols) + _ -> + false + end + end + Enum.any?(matching_unique_indexes) + end + end + + defp is_primary_desc(column_name, indexes) do + case indexes do + nil -> + false + _ -> + matching_desc_indexes = for {seq, info} <- indexes do + case info.origin do + "pk" -> + cols = for key_column <- info.columns do + key_column.key == 1 && key_column.name == column_name && key_column.desc == 1 + end + Enum.any?(cols) + _ -> + false + end + end + Enum.any?(matching_desc_indexes) + end + end + +end \ No newline at end of file diff --git a/lib/electric/migrations/triggers.ex b/lib/electric/migrations/triggers.ex new file mode 100644 index 0000000..fd54d67 --- /dev/null +++ b/lib/electric/migrations/triggers.ex @@ -0,0 +1,33 @@ +defmodule Electric.Migrations.Triggers do + @moduledoc """ + + """ + + + def add_triggers_to_last_migration(migration_set, template) do + case Electric.Migrations.Parse.sql_ast_from_migration_set(migration_set) do + {:error, reasons} -> + {:error, reasons} + ast -> + sql_in = List.last(migration_set).original_body + is_init = length(migration_set) == 1 + template_all_the_things(sql_in, ast, template, is_init) + end + end + + + @doc false + def template_all_the_things(original_sql, tables, template, is_init) do + ## strip the old header + patched_sql = String.replace(original_sql, ~r/\A\/\*((?s).*)\*\/\n/, "") + ## template + {result, _bindings} = + Code.eval_quoted(template, + is_init: is_init, + original_sql: patched_sql, + tables: tables + ) + result + end + +end \ No newline at end of file diff --git a/lib/electric/migrations/validation.ex b/lib/electric/migrations/validation.ex new file mode 100644 index 0000000..c1d1701 --- /dev/null +++ b/lib/electric/migrations/validation.ex @@ -0,0 +1,29 @@ +defmodule Electric.Migrations.Validation do + @moduledoc """ + + """ + + def validate(ordered_migrations) do + +# ast = case sql_ast_from_ordered_migrations(ordered_migrations) do +# {:error, reason} +# end + + + end + + +# def ensure_and_validate_original_sql(migration) do +# with_body = ensure_original_body(migration) +# {:ok, conn} = Exqlite.Sqlite3.open(":memory:") +# +# case Exqlite.Sqlite3.execute(conn, with_body.original_body) do +# :ok -> +# with_body +# +# {:error, reason} -> +# %{with_body | error: reason} +# end +# end + +end \ No newline at end of file diff --git a/test/migration_generation_test.exs b/test/migration_generation_test.exs new file mode 100644 index 0000000..497aa9e --- /dev/null +++ b/test/migration_generation_test.exs @@ -0,0 +1,154 @@ +defmodule MigrationsPostgresTest do + use ExUnit.Case + + describe "Generate PostgresSQL SQL text" do + test "Test create a new table" do + + sql = """ + CREATE TABLE IF NOT EXISTS fish ( + value TEXT PRIMARY KEY + ) STRICT, WITHOUT ROWID; + """ + + migration = %Electric.Migration{name: "test1", original_body: sql} + postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) + + expected = "\nCREATE TABLE main.fish (\n value text PRIMARY KEY);\n" + + assert expected == postgres_version + end + + test "Test two migrations and remove not null" do + + sql1 = """ + CREATE TABLE IF NOT EXISTS fish ( + value TEXT PRIMARY KEY + ) STRICT, WITHOUT ROWID;; + """ + + sql2 = """ + CREATE TABLE IF NOT EXISTS goat ( + name TEXT PRIMARY KEY NOT NULL + ) STRICT, WITHOUT ROWID; + """ + + migration_1 = %Electric.Migration{name: "test_1", original_body: sql1} + migration_2 = %Electric.Migration{name: "test_2", original_body: sql2} + postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration_1, migration_2]) + + expected = "\nCREATE TABLE main.goat (\n name text PRIMARY KEY);\n" + + assert expected == postgres_version + end + + test "Test add col" do + sql1 = """ + CREATE TABLE IF NOT EXISTS fish ( + value TEXT PRIMARY KEY + ) STRICT, WITHOUT ROWID; + """ + + sql2 = """ + CREATE TABLE IF NOT EXISTS goat ( + name TEXT PRIMARY KEY NOT NULL + ) STRICT, WITHOUT ROWID; + ALTER TABLE fish ADD COLUMN eyes INTEGER DEFAULT 2; + """ + + migration_1 = %Electric.Migration{name: "test_1", original_body: sql1} + migration_2 = %Electric.Migration{name: "test_2", original_body: sql2} + postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration_1, migration_2]) + expected = """ + ALTER TABLE main.fish ADD COLUMN eyes integer DEFAULT 2; + + CREATE TABLE main.goat ( + name text PRIMARY KEY); + """ + + assert expected == postgres_version + end + + test "foreign keys" do + + sql_in = """ + CREATE TABLE IF NOT EXISTS parent ( + id INTEGER PRIMARY KEY, + value TEXT + ) STRICT, WITHOUT ROWID; + + CREATE TABLE IF NOT EXISTS child ( + id INTEGER PRIMARY KEY, + daddy INTEGER NOT NULL, + FOREIGN KEY(daddy) REFERENCES parent(id) + ) STRICT, WITHOUT ROWID; + """ + + migration = %Electric.Migration{name: "test1", original_body: sql_in} + postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) + + expected = """ + + CREATE TABLE main.child ( + id integer PRIMARY KEY, + daddy integer NOT NULL, + FOREIGN KEY(daddy) REFERENCES parent(id) MATCH SIMPLE); + + CREATE TABLE main.parent ( + id integer PRIMARY KEY, + value text); + """ + + assert expected == postgres_version + end + + test "unique keys" do + + sql_in = """ + CREATE TABLE IF NOT EXISTS parent ( + id INTEGER PRIMARY KEY, + value TEXT UNIQUE + ) STRICT, WITHOUT ROWID; + + """ + + migration = %Electric.Migration{name: "test1", original_body: sql_in} + postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) + + expected = """ + + CREATE TABLE main.parent ( + id integer PRIMARY KEY, + value text UNIQUE); + """ + + assert expected == postgres_version + end + + + test "desc primary keys" do + + sql_in = """ + CREATE TABLE IF NOT EXISTS parent ( + id INTEGER PRIMARY KEY DESC, + value TEXT UNIQUE + ) STRICT, WITHOUT ROWID; + + """ + + migration = %Electric.Migration{name: "test1", original_body: sql_in} + postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) + + expected = """ + + CREATE TABLE main.parent ( + id integer PRIMARY KEY DESC, + value text UNIQUE); + """ + + assert expected == postgres_version + end + + + + end +end \ No newline at end of file diff --git a/test/migration_parse_test.exs b/test/migration_parse_test.exs new file mode 100644 index 0000000..8e4f77c --- /dev/null +++ b/test/migration_parse_test.exs @@ -0,0 +1,384 @@ +defmodule MigrationsParseTest do + use ExUnit.Case + + + describe "Parse sql" do + + test "tests can get column names" do + sql_in = """ + CREATE TABLE IF NOT EXISTS fish ( + value TEXT PRIMARY KEY, + colour TEXT + ) STRICT, WITHOUT ROWID;; + """ + + migration = %Electric.Migration{name: "test1", original_body: sql_in} + info = Electric.Migrations.Parse.sql_ast_from_migration_set([migration]) + + column_names = info["main.fish"][:columns] + assert column_names == ["value", "colour"] + end + + test "tests nonsense SQL fails" do + sql_in = """ + CREATE TABLE IF NOT EXISTS fish ( + value TEXT PRIMARY KEY + ); + SOME BOLLOCKS; + """ + + {:error, [reason]} = Electric.Migrations.Parse.sql_ast_from_migration_set([%Electric.Migration{name: "test1", original_body: sql_in}]) + + assert reason == "In migration test1 SQL error: near \"SOME\": syntax error" + + end + + + test "tests can check for strictness and rowid" do + + sql_in = """ + CREATE TABLE IF NOT EXISTS fish ( + value TEXT PRIMARY KEY, + colour TEXT + ); + """ + + {:error, [reason1, reason2]} = Electric.Migrations.Parse.sql_ast_from_migration_set([%Electric.Migration{name: "test1", original_body: sql_in}]) + + assert reason1 == "The table fish is not WITHOUT ROWID. Add the WITHOUT ROWID option at the end of the create table statement and make sure you also specify a primary key" + assert reason2 == "The table fish is not STRICT. Add the STRICT option at the end of the create table statement" + + end + + + test "tests getting SQL structure for templating" do + sql_in = """ + CREATE TABLE IF NOT EXISTS parent ( + id INTEGER PRIMARY KEY, + value TEXT + ) STRICT, WITHOUT ROWID; + + CREATE TABLE IF NOT EXISTS child ( + id INTEGER PRIMARY KEY, + daddy INTEGER NOT NULL, + FOREIGN KEY(daddy) REFERENCES parent(id) + ) STRICT, WITHOUT ROWID; + """ + + info = Electric.Migrations.Parse.sql_ast_from_migration_set([%Electric.Migration{name: "test1", original_body: sql_in}]) + + expected_info = %{ + "main.parent" => %{ + :namespace => "main", + :table_name => "parent", + :validation_fails => [], + :primary => ["id"], + :foreign_keys => [], + :columns => ["id", "value"], + column_infos: %{ + 0 => %{ + cid: 0, + dflt_value: nil, + name: "id", + notnull: 1, + pk: 1, + type: "INTEGER", + pk_desc: false, + unique: false + }, + 1 => %{ + cid: 1, + dflt_value: nil, + name: "value", + notnull: 0, + pk: 0, + type: "TEXT", + pk_desc: false, + unique: false + } + }, + foreign_keys_info: [], + table_info: %{ + name: "parent", + rootpage: 2, + sql: "CREATE TABLE parent (\n id INTEGER PRIMARY KEY,\n value TEXT\n) STRICT, WITHOUT ROWID", + tbl_name: "parent", + type: "table" + } + }, + "main.child" => %{ + :namespace => "main", + :table_name => "child", + :validation_fails => [], + :primary => ["id"], + :foreign_keys => [ + %{:child_key => "daddy", :parent_key => "id", :table => "main.parent"} + ], + :columns => ["id", "daddy"], + column_infos: %{ + 0 => %{ + cid: 0, + dflt_value: nil, + name: "id", + notnull: 1, + pk: 1, + type: "INTEGER", + pk_desc: false, + unique: false + }, + 1 => %{ + cid: 1, + dflt_value: nil, + name: "daddy", + notnull: 1, + pk: 0, + type: "INTEGER", + pk_desc: false, + unique: false + } + }, + foreign_keys_info: [ + %{ + from: "daddy", + id: 0, + match: "NONE", + on_delete: "NO ACTION", + on_update: "NO ACTION", + seq: 0, + table: "parent", + to: "id" + } + ], + table_info: %{ + name: "child", + rootpage: 3, + sql: "CREATE TABLE child (\n id INTEGER PRIMARY KEY,\n daddy INTEGER NOT NULL,\n FOREIGN KEY(daddy) REFERENCES parent(id)\n) STRICT, WITHOUT ROWID", + tbl_name: "child", + type: "table" + } + } + } + + assert info == expected_info + end + + + test "tests getting uniques" do + sql_in = """ + CREATE TABLE IF NOT EXISTS parent ( + id INTEGER PRIMARY KEY DESC, + value TEXT, + email TEXT UNIQUE + ) STRICT, WITHOUT ROWID; + + CREATE TABLE IF NOT EXISTS child ( + id INTEGER PRIMARY KEY, + daddy INTEGER NOT NULL, + FOREIGN KEY(daddy) REFERENCES parent(id) + ) STRICT, WITHOUT ROWID; + """ + + info = Electric.Migrations.Parse.sql_ast_from_migration_set([%Electric.Migration{name: "test1", original_body: sql_in}]) + + expected_info = %{ + "main.child" => %{ + column_infos: %{ + 0 => %{ + cid: 0, + dflt_value: nil, + name: "id", + notnull: 1, + pk: 1, + type: "INTEGER", + unique: false, + pk_desc: false + }, + 1 => %{ + cid: 1, + dflt_value: nil, + name: "daddy", + notnull: 1, + pk: 0, + type: "INTEGER", + unique: false, + pk_desc: false + } + }, + columns: ["id", "daddy"], + foreign_keys: [%{child_key: "daddy", parent_key: "id", table: "main.parent"}], + foreign_keys_info: [ + %{ + from: "daddy", + id: 0, + match: "NONE", + on_delete: "NO ACTION", + on_update: "NO ACTION", + seq: 0, + table: "parent", + to: "id" + } + ], + namespace: "main", + validation_fails: [], + primary: ["id"], + table_info: %{ + name: "child", + rootpage: 4, + sql: "CREATE TABLE child (\n id INTEGER PRIMARY KEY,\n daddy INTEGER NOT NULL,\n FOREIGN KEY(daddy) REFERENCES parent(id)\n) STRICT, WITHOUT ROWID", + tbl_name: "child", + type: "table" + }, + table_name: "child" + }, + "main.parent" => %{ + column_infos: %{ + 0 => %{ + cid: 0, + dflt_value: nil, + name: "id", + notnull: 1, + pk: 1, + type: "INTEGER", + unique: false, + pk_desc: true + }, + 1 => %{ + cid: 1, + dflt_value: nil, + name: "value", + notnull: 0, + pk: 0, + type: "TEXT", + unique: false, + pk_desc: false + }, + 2 => %{ + cid: 2, + dflt_value: nil, + name: "email", + notnull: 0, + pk: 0, + type: "TEXT", + unique: true, + pk_desc: false + } + }, + columns: ["id", "value", "email"], + foreign_keys: [], + validation_fails: [], + foreign_keys_info: [], + namespace: "main", + primary: ["id"], + table_info: %{ + name: "parent", + rootpage: 2, + sql: "CREATE TABLE parent (\n id INTEGER PRIMARY KEY DESC,\n value TEXT,\n email TEXT UNIQUE\n) STRICT, WITHOUT ROWID", + tbl_name: "parent", + type: "table" + }, + table_name: "parent" + } + } + + assert info == expected_info + end + + test "tests getting SQL index structure" do + + sql_in = """ + CREATE TABLE IF NOT EXISTS parent ( + id INTEGER PRIMARY KEY, + value TEXT, + email TEXT UNIQUE + ) STRICT, WITHOUT ROWID; + + CREATE TABLE IF NOT EXISTS child ( + id INTEGER PRIMARY KEY, + daddy INTEGER NOT NULL, + FOREIGN KEY(daddy) REFERENCES parent(id) + ) STRICT, WITHOUT ROWID; + """ + + table_info = Electric.Migrations.Parse.sql_ast_from_migration_set([%Electric.Migration{name: "test1", original_body: sql_in}]) + ##IO.inspect(table_info) + + index_info = Electric.Migrations.Parse.all_index_info([sql_in]) + + assert index_info == %{ + "main.parent" => %{ + 0 => %{ + columns: [ + %{cid: 0, coll: "BINARY", desc: 0, key: 1, name: "id", seqno: 0}, + %{cid: 1, coll: "BINARY", desc: 0, key: 0, name: "value", seqno: 1}, + %{cid: 2, coll: "BINARY", desc: 0, key: 0, name: "email", seqno: 2} + ], + name: "sqlite_autoindex_parent_2", + origin: "pk", + partial: 0, + seq: 0, + unique: 1 + }, + 1 => %{ + columns: [ + %{cid: 2, coll: "BINARY", desc: 0, key: 1, name: "email", seqno: 0}, + %{cid: 0, coll: "BINARY", desc: 0, key: 0, name: "id", seqno: 1} + ], + name: "sqlite_autoindex_parent_1", + origin: "u", + partial: 0, + seq: 1, + unique: 1 + } + } + } + end + + test "tests getting SQL conflict" do + + sql_in = """ + CREATE TABLE IF NOT EXISTS parent ( + id INTEGER PRIMARY KEY, + value TEXT, + email TEXT UNIQUE ON CONFLICT ROLLBACK + ); + + CREATE TABLE IF NOT EXISTS child ( + id INTEGER PRIMARY KEY, + daddy INTEGER NOT NULL, + FOREIGN KEY(daddy) REFERENCES parent(id) + ); + """ + + table_info = Electric.Migrations.Parse.sql_ast_from_migration_set([%Electric.Migration{name: "test1", original_body: sql_in}]) + ##IO.inspect(table_info) + + index_info = Electric.Migrations.Parse.all_index_info([sql_in]) + + assert index_info == %{ + "main.parent" => %{ + 0 => %{ + columns: [ + %{cid: 2, coll: "BINARY", desc: 0, key: 1, name: "email", seqno: 0}, + %{cid: -1, coll: "BINARY", desc: 0, key: 0, name: nil, seqno: 1} + ], + name: "sqlite_autoindex_parent_1", + origin: "u", + partial: 0, + seq: 0, + unique: 1 + } + } + } + end + + end + + + + + + + + + +end \ No newline at end of file diff --git a/test/migration_test.exs b/test/migration_test.exs index 8455a6c..0b00998 100644 --- a/test/migration_test.exs +++ b/test/migration_test.exs @@ -10,53 +10,15 @@ defmodule MigrationsTest do sql = """ CREATE TABLE IF NOT EXISTS fish ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; """ migration = %Electric.Migration{name: "test1", original_body: sql} -# IO.inspect(migration) - assert Electric.Migration.ensure_and_validate_original_sql(migration).error == nil + assert Electric.Migration.ensure_original_body(migration).error == nil end - test "tests nonsense SQL fails" do - sql = """ - CREATE TABLE IF NOT EXISTS fish ( - value TEXT PRIMARY KEY - ); - SOME BOLLOCKS; - """ - migration = %Electric.Migration{name: "test1", original_body: sql} - - assert Electric.Migration.ensure_and_validate_original_sql(migration).error == "near \"SOME\": syntax error" - end - end - - describe "Finds create table instructions in sql" do - test "find single create" do - sql = """ - CREATE TABLE IF NOT EXISTS fish ( - value TEXT PRIMARY KEY - ); - """ - - assert Electric.Migrations.created_table_names([sql]) == ["main.fish"] - end - - test "find multiple creates" do - sql = """ - CREATE TABLE IF NOT EXISTS fish ( - value TEXT PRIMARY KEY - ); - CREATE TABLE IF NOT EXISTS dogs ( - value TEXT PRIMARY KEY - ); - """ - - assert MapSet.new(Electric.Migrations.created_table_names([sql])) == - MapSet.new(["main.fish", "main.dogs"]) - end end describe "adds_triggers to sql" do @@ -64,17 +26,17 @@ defmodule MigrationsTest do sql = """ CREATE TABLE IF NOT EXISTS fish ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; """ expected = """ CREATE TABLE IF NOT EXISTS fish ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; --ADD A TRIGGER FOR main.fish; """ - assert Electric.Migrations.add_triggers_to_last_sql([sql], @trigger_template) == + assert Electric.Migrations.Triggers.add_triggers_to_last_migration([%Electric.Migration{name: "test1", original_body: sql}], @trigger_template) == expected end @@ -82,14 +44,13 @@ defmodule MigrationsTest do sql = """ CREATE TABLE IF NOT EXISTS fish ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; """ -# migration = %Electric.Migration{name: "test1", original_body: sql} + # migration = %Electric.Migration{name: "test1", original_body: sql} sql = - Electric.Migrations.add_triggers_to_last_sql( - [sql], + Electric.Migrations.Triggers.add_triggers_to_last_migration([%Electric.Migration{name: "test1", original_body: sql}], Electric.Migrations.get_template() ) @@ -97,14 +58,11 @@ defmodule MigrationsTest do end end - def is_valid_sql(sql) do {:ok, conn} = Exqlite.Sqlite3.open(":memory:") Exqlite.Sqlite3.execute(conn, sql) end - - describe "Triggers works as expected" do test "templating" do original_sql = """ @@ -131,7 +89,7 @@ defmodule MigrationsTest do } templated = - Electric.Migrations.template_all_the_things( + Electric.Migrations.Triggers.template_all_the_things( original_sql, tables, Electric.Migrations.get_template(), @@ -295,12 +253,11 @@ defmodule MigrationsTest do sql = """ CREATE TABLE IF NOT EXISTS fish ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; """ sql = - Electric.Migrations.add_triggers_to_last_sql( - [sql], + Electric.Migrations.Triggers.add_triggers_to_last_migration([%Electric.Migration{name: "test1", original_body: sql}], Electric.Migrations.get_template() ) @@ -328,7 +285,7 @@ defmodule MigrationsTest do sql = """ CREATE TABLE IF NOT EXISTS main.fish ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; """ {:ok, conn} = Exqlite.Sqlite3.open(":memory:") @@ -346,12 +303,11 @@ defmodule MigrationsTest do sql = """ CREATE TABLE IF NOT EXISTS fish ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; """ sql = - Electric.Migrations.add_triggers_to_last_sql( - [sql], + Electric.Migrations.Triggers.add_triggers_to_last_migration([%Electric.Migration{name: "test1", original_body: sql}], Electric.Migrations.get_template() ) @@ -379,12 +335,11 @@ defmodule MigrationsTest do CREATE TABLE IF NOT EXISTS fish ( value TEXT PRIMARY KEY, colour TEXT - ); + ) STRICT, WITHOUT ROWID; """ sql = - Electric.Migrations.add_triggers_to_last_sql( - [sql], + Electric.Migrations.Triggers.add_triggers_to_last_migration([%Electric.Migration{name: "test1", original_body: sql}], Electric.Migrations.get_template() ) @@ -424,7 +379,7 @@ defmodule MigrationsTest do sql1 = """ CREATE TABLE IF NOT EXISTS fish ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; """ sql2 = """ @@ -433,14 +388,17 @@ defmodule MigrationsTest do """ sql_out1 = - Electric.Migrations.add_triggers_to_last_sql( - [sql1], + Electric.Migrations.Triggers.add_triggers_to_last_migration([%Electric.Migration{name: "test1", original_body: sql1}], Electric.Migrations.get_template() ) + + migration_1 = %Electric.Migration{name: "test1", original_body: sql1} + migration_2 = %Electric.Migration{name: "test2", original_body: sql2} + sql_out2 = - Electric.Migrations.add_triggers_to_last_sql( - [sql1, sql2], + Electric.Migrations.Triggers.add_triggers_to_last_migration( + [migration_1, migration_2], Electric.Migrations.get_template() ) @@ -471,58 +429,10 @@ defmodule MigrationsTest do assert oldRow == nil end - test "tests can get column names" do - sql_in = """ - CREATE TABLE IF NOT EXISTS fish ( - value TEXT PRIMARY KEY, - colour TEXT - ); - """ - - info = Electric.Migrations.all_tables_info([sql_in]) - - column_names = info["main.fish"][:columns] - assert column_names == ["value", "colour"] - end - - test "tests getting SQL structure for templating" do - sql_in = """ - CREATE TABLE IF NOT EXISTS parent ( - id INTEGER PRIMARY KEY, - value TEXT - ); - - CREATE TABLE IF NOT EXISTS child ( - id INTEGER PRIMARY KEY, - daddy INTEGER NOT NULL, - FOREIGN KEY(daddy) REFERENCES parent(id) - ); - """ + end - info = Electric.Migrations.all_tables_info([sql_in]) - expected_info = %{ - "main.parent" => %{ - :namespace => "main", - :table_name => "parent", - :primary => ["id"], - :foreign_keys => [], - :columns => ["id", "value"] - }, - "main.child" => %{ - :namespace => "main", - :table_name => "child", - :primary => ["id"], - :foreign_keys => [ - %{:child_key => "daddy", :parent_key => "id", :table => "main.parent"} - ], - :columns => ["id", "daddy"] - } - } - assert info == expected_info - end - end def get_while(conn, statement, names) do case Exqlite.Sqlite3.step(conn, statement) do @@ -581,7 +491,7 @@ defmodule MigrationsFileTest do new_content = """ CREATE TABLE IF NOT EXISTS items ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; """ File.write!(my_new_migration, new_content, [:append]) @@ -601,7 +511,7 @@ defmodule MigrationsFileTest do new_content = """ CREATE TABLE IF NOT EXISTS items ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; """ File.write!(my_new_migration, new_content, [:append]) @@ -613,7 +523,7 @@ defmodule MigrationsFileTest do cats_content = """ CREATE TABLE IF NOT EXISTS cats ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; """ second_migration = most_recent_migration_file(migrations_path) @@ -684,7 +594,7 @@ defmodule MigrationsFileTest do dogs_content = """ CREATE TABLE IF NOT EXISTS dogs ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; """ File.write!(migration, dogs_content, [:append]) @@ -712,7 +622,11 @@ defmodule MigrationsFileTest do {:ok, migration} = File.read(migration_file_path) - m = %Electric.Migration{name: migration_name, original_body: migration, src_folder: Path.join([temp, "migrations"])} + m = %Electric.Migration{ + name: migration_name, + original_body: migration, + src_folder: Path.join([temp, "migrations"]) + } Electric.Migrations.add_triggers_to_migration( [m], @@ -722,11 +636,11 @@ defmodule MigrationsFileTest do expected = """ /* ElectricDB Migration - {"metadata": {"name": "#{migration_name}", "sha256": "0f0eb722f4d27646a93dcd6fd645b16b158cf1e40455544e158ef71972226e00"}} + {"metadata": {"name": "#{migration_name}", "sha256": "211b1e2b203d1fcac6ccb526d2775ec1f5575d4018ab1a33272948ce0ae76775"}} */ CREATE TABLE IF NOT EXISTS items ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; --ADD A TRIGGER FOR main.items; """ @@ -759,11 +673,11 @@ defmodule MigrationsFileTest do expected = """ /* ElectricDB Migration - {"metadata": {"name": "#{migration_name}", "sha256": "0f0eb722f4d27646a93dcd6fd645b16b158cf1e40455544e158ef71972226e00"}} + {"metadata": {"name": "#{migration_name}", "sha256": "211b1e2b203d1fcac6ccb526d2775ec1f5575d4018ab1a33272948ce0ae76775"}} */ CREATE TABLE IF NOT EXISTS items ( value TEXT PRIMARY KEY - ); + ) STRICT, WITHOUT ROWID; --ADD A TRIGGER FOR main.items; """ @@ -805,11 +719,17 @@ defmodule MigrationsFileTest do manifest = Jason.decode!(File.read!(manifest_path)) expected = %{ - "migrations" => [ - %{"name" => migration_name, "sha256" => "0f0eb722f4d27646a93dcd6fd645b16b158cf1e40455544e158ef71972226e00"}, - %{"name" => migration_name_2, "sha256" => "3da01b8ef4f3845fdc3cb3619c6ba75538988d6eec816f3734a7c36383874266"} - ] - } + "migrations" => [ + %{ + "name" => migration_name, + "sha256" => "211b1e2b203d1fcac6ccb526d2775ec1f5575d4018ab1a33272948ce0ae76775" + }, + %{ + "name" => migration_name_2, + "sha256" => "946f0f3a0d0338fa486d3d7da35c3b6032f837336fb9a08f933d44675bb264d3" + } + ] + } assert manifest == expected end @@ -858,17 +778,17 @@ defmodule MigrationsFileTest do "migrations" => [ %{ "body" => - "/*\nElectricDB Migration\n{\"metadata\": {\"name\": \"#{migration_name}\", \"sha256\": \"0f0eb722f4d27646a93dcd6fd645b16b158cf1e40455544e158ef71972226e00\"}}\n*/\nCREATE TABLE IF NOT EXISTS items (\n value TEXT PRIMARY KEY\n);\n--ADD A TRIGGER FOR main.items;\n", + "/*\nElectricDB Migration\n{\"metadata\": {\"name\": \"#{migration_name}\", \"sha256\": \"211b1e2b203d1fcac6ccb526d2775ec1f5575d4018ab1a33272948ce0ae76775\"}}\n*/\nCREATE TABLE IF NOT EXISTS items (\n value TEXT PRIMARY KEY\n) STRICT, WITHOUT ROWID;\n--ADD A TRIGGER FOR main.items;\n", "name" => migration_name, "encoding" => "escaped", - "sha256" => "0f0eb722f4d27646a93dcd6fd645b16b158cf1e40455544e158ef71972226e00" + "sha256" => "211b1e2b203d1fcac6ccb526d2775ec1f5575d4018ab1a33272948ce0ae76775" }, %{ "body" => - "/*\nElectricDB Migration\n{\"metadata\": {\"name\": \"#{migration_name_2}\", \"sha256\": \"3da01b8ef4f3845fdc3cb3619c6ba75538988d6eec816f3734a7c36383874266\"}}\n*/\nCREATE TABLE IF NOT EXISTS cat (\n value TEXT PRIMARY KEY\n);\n--ADD A TRIGGER FOR main.cat;\n\n--ADD A TRIGGER FOR main.items;\n", + "/*\nElectricDB Migration\n{\"metadata\": {\"name\": \"#{migration_name_2}\", \"sha256\": \"946f0f3a0d0338fa486d3d7da35c3b6032f837336fb9a08f933d44675bb264d3\"}}\n*/\nCREATE TABLE IF NOT EXISTS cat (\n value TEXT PRIMARY KEY\n) STRICT, WITHOUT ROWID;\n--ADD A TRIGGER FOR main.cat;\n\n--ADD A TRIGGER FOR main.items;\n", "name" => migration_name_2, "encoding" => "escaped", - "sha256" => "3da01b8ef4f3845fdc3cb3619c6ba75538988d6eec816f3734a7c36383874266" + "sha256" => "946f0f3a0d0338fa486d3d7da35c3b6032f837336fb9a08f933d44675bb264d3" } ] } diff --git a/test/support/migration.sql b/test/support/migration.sql index a352200..ece25f3 100644 --- a/test/support/migration.sql +++ b/test/support/migration.sql @@ -1,3 +1,3 @@ CREATE TABLE IF NOT EXISTS items ( value TEXT PRIMARY KEY -); \ No newline at end of file +) STRICT, WITHOUT ROWID; \ No newline at end of file diff --git a/test/support/migration2.sql b/test/support/migration2.sql index 798d809..aa5f440 100644 --- a/test/support/migration2.sql +++ b/test/support/migration2.sql @@ -1,3 +1,3 @@ CREATE TABLE IF NOT EXISTS cat ( value TEXT PRIMARY KEY -); \ No newline at end of file +) STRICT, WITHOUT ROWID; \ No newline at end of file From c2755dd8c73130c91a277573e05be8dc46f519f2 Mon Sep 17 00:00:00 2001 From: Paul Harter Date: Wed, 28 Sep 2022 16:47:03 +0100 Subject: [PATCH 03/18] formmating --- lib/electric/migrations/generation.ex | 240 +++++++++++++++----------- lib/electric/migrations/migration.ex | 24 +-- lib/electric/migrations/migrations.ex | 84 ++++----- lib/electric/migrations/parse.ex | 200 ++++++++++++--------- lib/electric/migrations/triggers.ex | 19 +- lib/electric/migrations/validation.ex | 37 ++-- test/migration_generation_test.exs | 36 ++-- test/migration_parse_test.exs | 129 +++++++------- test/migration_test.exs | 27 +-- 9 files changed, 438 insertions(+), 358 deletions(-) diff --git a/lib/electric/migrations/generation.ex b/lib/electric/migrations/generation.ex index a43f639..8599434 100644 --- a/lib/electric/migrations/generation.ex +++ b/lib/electric/migrations/generation.ex @@ -17,61 +17,71 @@ defmodule Electric.Migrations.Generation do defp get_postgres_for_infos(before_ast, after_ast) do get_sql_for_infos(before_ast, after_ast, "postgres") - end defp get_sqlite_for_infos(before_ast, after_ast) do get_sql_for_infos(before_ast, after_ast, "sqlite") - end defp get_sql_for_infos(before_ast, after_ast, flavour) do - - statements = for change <- table_changes(before_ast, after_ast) do - case change do - {nil, table_after} -> - ## https://www.sqlite.org/syntax/column-constraint.html - ## %{cid: 0, dflt_value: nil, name: "id", notnull: 0, pk: 1, type: "INTEGER"} - - column_definitions = for {column_id, column_info} <- table_after.column_infos do - "\n " <> column_def_sql_from_info(column_info, flavour) - end - - foreign_key_clauses = for foreign_key_info <- table_after.foreign_keys_info do - "\n " <> foreign_key_sql_from_info(foreign_key_info, flavour) - end - - columns_and_keys = column_definitions ++ foreign_key_clauses - - case flavour do - "postgres" -> - "\nCREATE TABLE #{table_after.namespace}.#{table_after.table_name} (#{Enum.join(columns_and_keys, ",")});\n" - "sqlite" -> - "\nCREATE TABLE #{table_after.namespace}.#{table_after.table_name} (#{Enum.join(columns_and_keys, ",")})STRICT;\n" - end - - {table_before, nil} -> - "DROP TABLE #{table_before.namespace}.#{table_before.table_name};\n" - {table_before, table_after} -> - - ## add columns - added_colums_lines = for {column_id, column_info} <- table_after.column_infos, !Map.has_key?(table_before.column_infos, column_id) do - "ALTER TABLE #{table_after.namespace}.#{table_after.table_name} ADD COLUMN #{column_def_sql_from_info(column_info, flavour)};\n" - end - - ## delete columns - dropped_colums_lines = for {column_id, column_info} <- table_before.column_infos, !Map.has_key?(table_after.column_infos, column_id) do - "ALTER TABLE #{table_before.namespace}.#{table_before.table_name} DROP COLUMN #{column_info.name};\n" + statements = + for change <- table_changes(before_ast, after_ast) do + case change do + {nil, table_after} -> + ## https://www.sqlite.org/syntax/column-constraint.html + ## %{cid: 0, dflt_value: nil, name: "id", notnull: 0, pk: 1, type: "INTEGER"} + + column_definitions = + for {column_id, column_info} <- table_after.column_infos do + "\n " <> column_def_sql_from_info(column_info, flavour) + end + + foreign_key_clauses = + for foreign_key_info <- table_after.foreign_keys_info do + "\n " <> foreign_key_sql_from_info(foreign_key_info, flavour) + end + + columns_and_keys = column_definitions ++ foreign_key_clauses + + case flavour do + "postgres" -> + "\nCREATE TABLE #{table_after.namespace}.#{table_after.table_name} (#{Enum.join(columns_and_keys, ",")});\n" + + "sqlite" -> + "\nCREATE TABLE #{table_after.namespace}.#{table_after.table_name} (#{Enum.join(columns_and_keys, ",")})STRICT;\n" + end + + {table_before, nil} -> + "DROP TABLE #{table_before.namespace}.#{table_before.table_name};\n" + + {table_before, table_after} -> + ## add columns + added_colums_lines = + for {column_id, column_info} <- table_after.column_infos, + !Map.has_key?(table_before.column_infos, column_id) do + "ALTER TABLE #{table_after.namespace}.#{table_after.table_name} ADD COLUMN #{column_def_sql_from_info(column_info, flavour)};\n" + end + + ## delete columns + dropped_colums_lines = + for {column_id, column_info} <- table_before.column_infos, + !Map.has_key?(table_after.column_infos, column_id) do + "ALTER TABLE #{table_before.namespace}.#{table_before.table_name} DROP COLUMN #{column_info.name};\n" + end + + ## rename columns + rename_colums_lines = + for {column_id, column_info} <- table_after.column_infos, + Map.has_key?(table_before.column_infos, column_id) && + column_info.name != table_before.column_infos[column_id].name do + "ALTER TABLE #{table_after.namespace}.#{table_after.table_name} RENAME COLUMN #{table_before.column_infos[column_id].name} TO #{column_info.name};\n" + end + + all_change_lines = added_colums_lines ++ dropped_colums_lines ++ rename_colums_lines + Enum.join(all_change_lines, " ") end - - ## rename columns - rename_colums_lines = for {column_id, column_info} <- table_after.column_infos, Map.has_key?(table_before.column_infos, column_id) && column_info.name != table_before.column_infos[column_id].name do - "ALTER TABLE #{table_after.namespace}.#{table_after.table_name} RENAME COLUMN #{table_before.column_infos[column_id].name} TO #{column_info.name};\n" - end - all_change_lines = added_colums_lines ++ dropped_colums_lines ++ rename_colums_lines - Enum.join(all_change_lines, " ") end - end + Enum.join(statements, "") end @@ -81,18 +91,21 @@ defmodule Electric.Migrations.Generation do {nil, table_info} end else - new_and_changed = for {table_name, table_info} <- after_ast, table_info != before_ast[table_name] do - {before_ast[table_name], table_info} - end - dropped = for {table_name, table_info} <- before_ast, !Map.has_key?(after_ast, table_name) do - {table_info, nil} - end + new_and_changed = + for {table_name, table_info} <- after_ast, table_info != before_ast[table_name] do + {before_ast[table_name], table_info} + end + + dropped = + for {table_name, table_info} <- before_ast, !Map.has_key?(after_ast, table_name) do + {table_info, nil} + end + new_and_changed ++ dropped end end defp foreign_key_sql_from_info(key_info, flavour) do - # DEFERRABLE # %{from: "daddy", id: 0, match: "NONE", on_delete: "NO ACTION", on_update: "NO ACTION", seq: 0, table: "parent", to: "id"} @@ -100,33 +113,37 @@ defmodule Electric.Migrations.Generation do elements = [] # force postgres to MATCH SIMPLE as sqlite always does this - elements = case flavour do - "postgres" -> - ["MATCH SIMPLE" | elements] - _ -> - elements - end + elements = + case flavour do + "postgres" -> + ["MATCH SIMPLE" | elements] - elements = if key_info.on_delete != "NO ACTION" do - ["ON DELETE #{key_info.on_delete}" | elements] - else - elements - end + _ -> + elements + end - elements = if key_info.on_update != "NO ACTION" do - ["ON UPDATE #{key_info.on_update}" | elements] - else - elements - end + elements = + if key_info.on_delete != "NO ACTION" do + ["ON DELETE #{key_info.on_delete}" | elements] + else + elements + end - elements = ["FOREIGN KEY(#{key_info.from}) REFERENCES #{key_info.table}(#{key_info.to})" | elements] + elements = + if key_info.on_update != "NO ACTION" do + ["ON UPDATE #{key_info.on_update}" | elements] + else + elements + end + + elements = [ + "FOREIGN KEY(#{key_info.from}) REFERENCES #{key_info.table}(#{key_info.to})" | elements + ] Enum.join(elements, " ") end - defp column_def_sql_from_info(column_info, flavour) do - # DONE # name # type @@ -149,40 +166,63 @@ defmodule Electric.Migrations.Generation do # PRIMARY KEY conflict-clause this is sqlite stuff # NOT NULL conflict-clause this is sqlite stuff - type_lookup = case flavour do + type_lookup = + case flavour do "postgres" -> - %{ "TEXT" => "text", - "NUMERIC" => "numeric", - "INTEGER" => "integer", - "REAL" => "real", - "BLOB" => "blob" - } + %{ + "TEXT" => "text", + "NUMERIC" => "numeric", + "INTEGER" => "integer", + "REAL" => "real", + "BLOB" => "blob" + } + _ -> - %{ "TEXT" => "TEXT", - "NUMERIC" => "NUMERIC", - "INTEGER" => "INTEGER", - "REAL" => "REAL", - "BLOB" => "BLOB" - } + %{ + "TEXT" => "TEXT", + "NUMERIC" => "NUMERIC", + "INTEGER" => "INTEGER", + "REAL" => "REAL", + "BLOB" => "BLOB" + } end elements = [] - elements = if column_info.dflt_value != nil do - ["DEFAULT #{column_info.dflt_value}" | elements] - else - elements - end - elements = if column_info.unique do ["UNIQUE" | elements] else elements end - elements = if column_info.notnull != 0 && column_info.pk == 0 do ["NOT NULL" | elements] else elements end - elements = if column_info.pk != 0 do - ["PRIMARY KEY#{ if column_info.pk_desc do " DESC" else "" end}" | elements] - else - elements - end + + elements = + if column_info.dflt_value != nil do + ["DEFAULT #{column_info.dflt_value}" | elements] + else + elements + end + + elements = + if column_info.unique do + ["UNIQUE" | elements] + else + elements + end + + elements = + if column_info.notnull != 0 && column_info.pk == 0 do + ["NOT NULL" | elements] + else + elements + end + + elements = + if column_info.pk != 0 do + ["PRIMARY KEY#{if column_info.pk_desc do + " DESC" + else + "" + end}" | elements] + else + elements + end + elements = [type_lookup[column_info.type] | elements] elements = [column_info.name | elements] Enum.join(elements, " ") end - - -end \ No newline at end of file +end diff --git a/lib/electric/migrations/migration.ex b/lib/electric/migrations/migration.ex index 37bb0cb..a240dad 100644 --- a/lib/electric/migrations/migration.ex +++ b/lib/electric/migrations/migration.ex @@ -39,18 +39,18 @@ defmodule Electric.Migration do end end -# def ensure_and_validate_original_sql(migration) do -# with_body = ensure_original_body(migration) -# {:ok, conn} = Exqlite.Sqlite3.open(":memory:") -# -# case Exqlite.Sqlite3.execute(conn, with_body.original_body) do -# :ok -> -# with_body -# -# {:error, reason} -> -# %{with_body | error: reason} -# end -# end + # def ensure_and_validate_original_sql(migration) do + # with_body = ensure_original_body(migration) + # {:ok, conn} = Exqlite.Sqlite3.open(":memory:") + # + # case Exqlite.Sqlite3.execute(conn, with_body.original_body) do + # :ok -> + # with_body + # + # {:error, reason} -> + # %{with_body | error: reason} + # end + # end def as_json_map(migration, with_body) do with_satellite_body = ensure_satellite_body(migration) diff --git a/lib/electric/migrations/migrations.ex b/lib/electric/migrations/migrations.ex index df6669f..c4fe0aa 100644 --- a/lib/electric/migrations/migrations.ex +++ b/lib/electric/migrations/migrations.ex @@ -23,6 +23,7 @@ defmodule Electric.Migrations do case root_directory = Map.get(options, :dir) do nil -> "migrations" + _ -> if Path.basename(root_directory) == "migrations" do root_directory @@ -80,7 +81,6 @@ defmodule Electric.Migrations do migration_set = ordered_migrations(migrations_folder) add_triggers_to_migrations(migration_set, template) - if flags[:manifest] do write_manifest(migrations_folder) end @@ -218,6 +218,7 @@ defmodule Electric.Migrations do for migration <- ordered_migrations do Electric.Migration.ensure_original_body(migration) end + failed_validation = Enum.filter(validated_migrations, fn migration -> migration.error != nil end) @@ -243,58 +244,59 @@ defmodule Electric.Migrations do end end - @doc false def add_triggers_to_migration(migration_set, template) do migration = List.last(migration_set) case Electric.Migrations.Triggers.add_triggers_to_last_migration(migration_set, template) do - {:error, reasons} -> - {:error, reasons} - with_triggers -> - migration_fingerprint = - if length(migration_set) > 1 do - previous_migration = Enum.at(migration_set, -2) - previous_metadata = Electric.Migration.get_satellite_metadata(previous_migration) - "#{migration.original_body}#{previous_metadata["sha256"]}" - else - migration.original_body - end + {:error, reasons} -> + {:error, reasons} + + with_triggers -> + migration_fingerprint = + if length(migration_set) > 1 do + previous_migration = Enum.at(migration_set, -2) + previous_metadata = Electric.Migration.get_satellite_metadata(previous_migration) + "#{migration.original_body}#{previous_metadata["sha256"]}" + else + migration.original_body + end - postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations(migration_set) + postgres_version = + Electric.Migrations.Generation.postgres_for_ordered_migrations(migration_set) - hash = calc_hash(migration_fingerprint) - satellite_file_path = Electric.Migration.satellite_file_path(migration) - postgres_file_path = Electric.Migration.postgres_file_path(migration) + hash = calc_hash(migration_fingerprint) + satellite_file_path = Electric.Migration.satellite_file_path(migration) + postgres_file_path = Electric.Migration.postgres_file_path(migration) - if File.exists?(satellite_file_path) do - metadata = Electric.Migration.get_satellite_metadata(migration) + if File.exists?(satellite_file_path) do + metadata = Electric.Migration.get_satellite_metadata(migration) - if metadata["sha256"] != hash do - IO.puts("Warning: The migration #{migration.name} has been modified.") - File.rm(satellite_file_path) - File.rm(postgres_file_path) + if metadata["sha256"] != hash do + IO.puts("Warning: The migration #{migration.name} has been modified.") + File.rm(satellite_file_path) + File.rm(postgres_file_path) + end end - end + if !File.exists?(satellite_file_path) do + header = + case Electric.Migration.get_original_metadata(migration) do + {:error, _reason} -> + Electric.Migration.file_header(migration, hash, nil) + existing_metadata -> + Electric.Migration.file_header(migration, hash, existing_metadata["title"]) + end - if !File.exists?(satellite_file_path) do - header = - case Electric.Migration.get_original_metadata(migration) do - {:error, _reason} -> - Electric.Migration.file_header(migration, hash, nil) - - existing_metadata -> - Electric.Migration.file_header(migration, hash, existing_metadata["title"]) - end - - File.write!(satellite_file_path, header <> with_triggers) - File.write!(postgres_file_path, header <> postgres_version) - :ok - else - :ok - end + File.write!(satellite_file_path, header <> with_triggers) + File.write!(postgres_file_path, header <> postgres_version) + :ok + else + :ok + end end - end #function + end + + # function end diff --git a/lib/electric/migrations/parse.ex b/lib/electric/migrations/parse.ex index e52f90c..a2cb629 100644 --- a/lib/electric/migrations/parse.ex +++ b/lib/electric/migrations/parse.ex @@ -8,25 +8,25 @@ defmodule Electric.Migrations.Parse do case ast_from_ordered_migrations(migrations) do {ast, []} -> ast + {ast, errors} -> {:error, errors} end end - @doc false def ast_from_ordered_migrations(migrations) do - namespace = "main" # get all the table names {:ok, conn} = Exqlite.Sqlite3.open(":memory:") - sql_errors = Enum.flat_map(migrations, fn migration -> - case Exqlite.Sqlite3.execute(conn, migration.original_body) do - {:error, reason} -> ["In migration #{migration.name} SQL error: #{reason}"] - :ok -> [] - end - end) + sql_errors = + Enum.flat_map(migrations, fn migration -> + case Exqlite.Sqlite3.execute(conn, migration.original_body) do + {:error, reason} -> ["In migration #{migration.name} SQL error: #{reason}"] + :ok -> [] + end + end) if length(sql_errors) > 0 do {:error, sql_errors} @@ -45,7 +45,6 @@ defmodule Electric.Migrations.Parse do # for each table infos = for [type, name, tbl_name, rootpage, sql] <- info do - validation_fails = check_sql(tbl_name, sql) # column names @@ -59,14 +58,17 @@ defmodule Electric.Migrations.Parse do column_infos = for [cid, name, type, notnull, dflt_value, pk] <- columns, into: %{} do - {cid, %{:cid => cid, - :name => name, - :type => type, - :notnull => notnull, - :unique => is_unique(name, index_info["#{namespace}.#{tbl_name}"]), - :pk_desc => is_primary_desc(name, index_info["#{namespace}.#{tbl_name}"]), - :dflt_value => dflt_value, - :pk => pk}} + {cid, + %{ + :cid => cid, + :name => name, + :type => type, + :notnull => notnull, + :unique => is_unique(name, index_info["#{namespace}.#{tbl_name}"]), + :pk_desc => is_primary_desc(name, index_info["#{namespace}.#{tbl_name}"]), + :dflt_value => dflt_value, + :pk => pk + }} end # private keys columns @@ -76,7 +78,9 @@ defmodule Electric.Migrations.Parse do end # foreign keys - {:ok, foreign_statement} = Exqlite.Sqlite3.prepare(conn, "PRAGMA foreign_key_list(#{tbl_name});") + {:ok, foreign_statement} = + Exqlite.Sqlite3.prepare(conn, "PRAGMA foreign_key_list(#{tbl_name});") + foreign_keys_rows = get_rows_while(conn, foreign_statement, []) foreign_keys = @@ -104,7 +108,13 @@ defmodule Electric.Migrations.Parse do %{ :table_name => tbl_name, - :table_info => %{type: type, name: name, tbl_name: tbl_name, rootpage: rootpage, sql: sql}, + :table_info => %{ + type: type, + name: name, + tbl_name: tbl_name, + rootpage: rootpage, + sql: sql + }, :columns => column_names, :namespace => namespace, :primary => private_key_column_names, @@ -116,33 +126,42 @@ defmodule Electric.Migrations.Parse do end ast = Enum.into(infos, %{}, fn info -> {"#{namespace}.#{info.table_name}", info} end) - validation_fails = for info <- infos, length(info.validation_fails) > 0 do - info.validation_fails - end + + validation_fails = + for info <- infos, length(info.validation_fails) > 0 do + info.validation_fails + end + {ast, List.flatten(validation_fails)} end end - def check_sql(table_name, sql) do validation_fails = [] lower = String.downcase(sql) - validation_fails = if !String.contains?(lower, "strict") do - ["The table #{table_name} is not STRICT. Add the STRICT option at the end of the create table statement" | validation_fails] - else - validation_fails - end - validation_fails = if !String.contains?(lower, "without rowid") do - ["The table #{table_name} is not WITHOUT ROWID. Add the WITHOUT ROWID option at the end of the create table statement and make sure you also specify a primary key" | validation_fails] - else - validation_fails - end - end + validation_fails = + if !String.contains?(lower, "strict") do + [ + "The table #{table_name} is not STRICT. Add the STRICT option at the end of the create table statement" + | validation_fails + ] + else + validation_fails + end + validation_fails = + if !String.contains?(lower, "without rowid") do + [ + "The table #{table_name} is not WITHOUT ROWID. Add the WITHOUT ROWID option at the end of the create table statement and make sure you also specify a primary key" + | validation_fails + ] + else + validation_fails + end + end def all_index_info(all_migrations) do - namespace = "main" # get all the table names {:ok, conn} = Exqlite.Sqlite3.open(":memory:") @@ -150,13 +169,11 @@ defmodule Electric.Migrations.Parse do for migration <- all_migrations do :ok = Exqlite.Sqlite3.execute(conn, migration) end - all_index_info_from_connection(conn) + all_index_info_from_connection(conn) end - defp all_index_info_from_connection(conn) do - namespace = "main" {:ok, statement} = @@ -176,40 +193,44 @@ defmodule Electric.Migrations.Parse do indexes = Enum.reverse(get_rows_while(conn, info_statement, [])) index_infos = - for [seq, name, unique, origin, partial] <- indexes, into: %{} do + for [seq, name, unique, origin, partial] <- indexes, into: %{} do + {:ok, col_info_statement} = + Exqlite.Sqlite3.prepare(conn, "PRAGMA index_xinfo(#{name});") - {:ok, col_info_statement} = Exqlite.Sqlite3.prepare(conn, "PRAGMA index_xinfo(#{name});") index_columns = Enum.reverse(get_rows_while(conn, col_info_statement, [])) - index_column_infos = for [seqno, cid, name, desc, coll, key] <- index_columns do - %{ - :seqno => seqno, - :cid => cid, - :name => name, - :desc => desc, - :coll => coll, - :key => key - } - end - - {seq, %{ - :seq => seq, - :name => name, - :unique => unique, - :origin => origin, - :partial => partial, - :columns => index_column_infos - }} + index_column_infos = + for [seqno, cid, name, desc, coll, key] <- index_columns do + %{ + :seqno => seqno, + :cid => cid, + :name => name, + :desc => desc, + :coll => coll, + :key => key + } + end + + {seq, + %{ + :seq => seq, + :name => name, + :unique => unique, + :origin => origin, + :partial => partial, + :columns => index_column_infos + }} end + {"main.#{tbl_name}", index_infos} end end - defp get_rows_while(conn, statement, rows) do case Exqlite.Sqlite3.step(conn, statement) do {:row, row} -> get_rows_while(conn, statement, [row | rows]) + :done -> rows end @@ -219,19 +240,25 @@ defmodule Electric.Migrations.Parse do case indexes do nil -> false + _ -> - matching_unique_indexes = for {seq, info} <- indexes do - case info.origin do - "u" -> - cols = for key_column <- info.columns do - key_column.key == 1 && key_column.name == column_name + matching_unique_indexes = + for {seq, info} <- indexes do + case info.origin do + "u" -> + cols = + for key_column <- info.columns do + key_column.key == 1 && key_column.name == column_name + end + + Enum.any?(cols) + + _ -> + false end - Enum.any?(cols) - _ -> - false - end - end - Enum.any?(matching_unique_indexes) + end + + Enum.any?(matching_unique_indexes) end end @@ -239,20 +266,25 @@ defmodule Electric.Migrations.Parse do case indexes do nil -> false + _ -> - matching_desc_indexes = for {seq, info} <- indexes do - case info.origin do - "pk" -> - cols = for key_column <- info.columns do - key_column.key == 1 && key_column.name == column_name && key_column.desc == 1 + matching_desc_indexes = + for {seq, info} <- indexes do + case info.origin do + "pk" -> + cols = + for key_column <- info.columns do + key_column.key == 1 && key_column.name == column_name && key_column.desc == 1 + end + + Enum.any?(cols) + + _ -> + false end - Enum.any?(cols) - _ -> - false - end - end - Enum.any?(matching_desc_indexes) + end + + Enum.any?(matching_desc_indexes) end end - -end \ No newline at end of file +end diff --git a/lib/electric/migrations/triggers.ex b/lib/electric/migrations/triggers.ex index fd54d67..76e5015 100644 --- a/lib/electric/migrations/triggers.ex +++ b/lib/electric/migrations/triggers.ex @@ -3,19 +3,18 @@ defmodule Electric.Migrations.Triggers do """ - def add_triggers_to_last_migration(migration_set, template) do case Electric.Migrations.Parse.sql_ast_from_migration_set(migration_set) do - {:error, reasons} -> - {:error, reasons} - ast -> - sql_in = List.last(migration_set).original_body - is_init = length(migration_set) == 1 - template_all_the_things(sql_in, ast, template, is_init) + {:error, reasons} -> + {:error, reasons} + + ast -> + sql_in = List.last(migration_set).original_body + is_init = length(migration_set) == 1 + template_all_the_things(sql_in, ast, template, is_init) end end - @doc false def template_all_the_things(original_sql, tables, template, is_init) do ## strip the old header @@ -27,7 +26,7 @@ defmodule Electric.Migrations.Triggers do original_sql: patched_sql, tables: tables ) + result end - -end \ No newline at end of file +end diff --git a/lib/electric/migrations/validation.ex b/lib/electric/migrations/validation.ex index c1d1701..2165298 100644 --- a/lib/electric/migrations/validation.ex +++ b/lib/electric/migrations/validation.ex @@ -4,26 +4,21 @@ defmodule Electric.Migrations.Validation do """ def validate(ordered_migrations) do - -# ast = case sql_ast_from_ordered_migrations(ordered_migrations) do -# {:error, reason} -# end - - + # ast = case sql_ast_from_ordered_migrations(ordered_migrations) do + # {:error, reason} + # end end - -# def ensure_and_validate_original_sql(migration) do -# with_body = ensure_original_body(migration) -# {:ok, conn} = Exqlite.Sqlite3.open(":memory:") -# -# case Exqlite.Sqlite3.execute(conn, with_body.original_body) do -# :ok -> -# with_body -# -# {:error, reason} -> -# %{with_body | error: reason} -# end -# end - -end \ No newline at end of file + # def ensure_and_validate_original_sql(migration) do + # with_body = ensure_original_body(migration) + # {:ok, conn} = Exqlite.Sqlite3.open(":memory:") + # + # case Exqlite.Sqlite3.execute(conn, with_body.original_body) do + # :ok -> + # with_body + # + # {:error, reason} -> + # %{with_body | error: reason} + # end + # end +end diff --git a/test/migration_generation_test.exs b/test/migration_generation_test.exs index 497aa9e..4c5b21e 100644 --- a/test/migration_generation_test.exs +++ b/test/migration_generation_test.exs @@ -3,7 +3,6 @@ defmodule MigrationsPostgresTest do describe "Generate PostgresSQL SQL text" do test "Test create a new table" do - sql = """ CREATE TABLE IF NOT EXISTS fish ( value TEXT PRIMARY KEY @@ -11,7 +10,9 @@ defmodule MigrationsPostgresTest do """ migration = %Electric.Migration{name: "test1", original_body: sql} - postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) + + postgres_version = + Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) expected = "\nCREATE TABLE main.fish (\n value text PRIMARY KEY);\n" @@ -19,7 +20,6 @@ defmodule MigrationsPostgresTest do end test "Test two migrations and remove not null" do - sql1 = """ CREATE TABLE IF NOT EXISTS fish ( value TEXT PRIMARY KEY @@ -34,7 +34,9 @@ defmodule MigrationsPostgresTest do migration_1 = %Electric.Migration{name: "test_1", original_body: sql1} migration_2 = %Electric.Migration{name: "test_2", original_body: sql2} - postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration_1, migration_2]) + + postgres_version = + Electric.Migrations.Generation.postgres_for_ordered_migrations([migration_1, migration_2]) expected = "\nCREATE TABLE main.goat (\n name text PRIMARY KEY);\n" @@ -57,7 +59,10 @@ defmodule MigrationsPostgresTest do migration_1 = %Electric.Migration{name: "test_1", original_body: sql1} migration_2 = %Electric.Migration{name: "test_2", original_body: sql2} - postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration_1, migration_2]) + + postgres_version = + Electric.Migrations.Generation.postgres_for_ordered_migrations([migration_1, migration_2]) + expected = """ ALTER TABLE main.fish ADD COLUMN eyes integer DEFAULT 2; @@ -69,7 +74,6 @@ defmodule MigrationsPostgresTest do end test "foreign keys" do - sql_in = """ CREATE TABLE IF NOT EXISTS parent ( id INTEGER PRIMARY KEY, @@ -84,7 +88,9 @@ defmodule MigrationsPostgresTest do """ migration = %Electric.Migration{name: "test1", original_body: sql_in} - postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) + + postgres_version = + Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) expected = """ @@ -102,7 +108,6 @@ defmodule MigrationsPostgresTest do end test "unique keys" do - sql_in = """ CREATE TABLE IF NOT EXISTS parent ( id INTEGER PRIMARY KEY, @@ -112,7 +117,9 @@ defmodule MigrationsPostgresTest do """ migration = %Electric.Migration{name: "test1", original_body: sql_in} - postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) + + postgres_version = + Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) expected = """ @@ -124,9 +131,7 @@ defmodule MigrationsPostgresTest do assert expected == postgres_version end - test "desc primary keys" do - sql_in = """ CREATE TABLE IF NOT EXISTS parent ( id INTEGER PRIMARY KEY DESC, @@ -136,7 +141,9 @@ defmodule MigrationsPostgresTest do """ migration = %Electric.Migration{name: "test1", original_body: sql_in} - postgres_version = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) + + postgres_version = + Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) expected = """ @@ -147,8 +154,5 @@ defmodule MigrationsPostgresTest do assert expected == postgres_version end - - - end -end \ No newline at end of file +end diff --git a/test/migration_parse_test.exs b/test/migration_parse_test.exs index 8e4f77c..c6e2b01 100644 --- a/test/migration_parse_test.exs +++ b/test/migration_parse_test.exs @@ -1,9 +1,7 @@ defmodule MigrationsParseTest do use ExUnit.Case - - - describe "Parse sql" do + describe "Parse sql" do test "tests can get column names" do sql_in = """ CREATE TABLE IF NOT EXISTS fish ( @@ -27,15 +25,15 @@ defmodule MigrationsParseTest do SOME BOLLOCKS; """ - {:error, [reason]} = Electric.Migrations.Parse.sql_ast_from_migration_set([%Electric.Migration{name: "test1", original_body: sql_in}]) + {:error, [reason]} = + Electric.Migrations.Parse.sql_ast_from_migration_set([ + %Electric.Migration{name: "test1", original_body: sql_in} + ]) assert reason == "In migration test1 SQL error: near \"SOME\": syntax error" - end - test "tests can check for strictness and rowid" do - sql_in = """ CREATE TABLE IF NOT EXISTS fish ( value TEXT PRIMARY KEY, @@ -43,14 +41,18 @@ defmodule MigrationsParseTest do ); """ - {:error, [reason1, reason2]} = Electric.Migrations.Parse.sql_ast_from_migration_set([%Electric.Migration{name: "test1", original_body: sql_in}]) + {:error, [reason1, reason2]} = + Electric.Migrations.Parse.sql_ast_from_migration_set([ + %Electric.Migration{name: "test1", original_body: sql_in} + ]) - assert reason1 == "The table fish is not WITHOUT ROWID. Add the WITHOUT ROWID option at the end of the create table statement and make sure you also specify a primary key" - assert reason2 == "The table fish is not STRICT. Add the STRICT option at the end of the create table statement" + assert reason1 == + "The table fish is not WITHOUT ROWID. Add the WITHOUT ROWID option at the end of the create table statement and make sure you also specify a primary key" + assert reason2 == + "The table fish is not STRICT. Add the STRICT option at the end of the create table statement" end - test "tests getting SQL structure for templating" do sql_in = """ CREATE TABLE IF NOT EXISTS parent ( @@ -65,7 +67,10 @@ defmodule MigrationsParseTest do ) STRICT, WITHOUT ROWID; """ - info = Electric.Migrations.Parse.sql_ast_from_migration_set([%Electric.Migration{name: "test1", original_body: sql_in}]) + info = + Electric.Migrations.Parse.sql_ast_from_migration_set([ + %Electric.Migration{name: "test1", original_body: sql_in} + ]) expected_info = %{ "main.parent" => %{ @@ -101,7 +106,8 @@ defmodule MigrationsParseTest do table_info: %{ name: "parent", rootpage: 2, - sql: "CREATE TABLE parent (\n id INTEGER PRIMARY KEY,\n value TEXT\n) STRICT, WITHOUT ROWID", + sql: + "CREATE TABLE parent (\n id INTEGER PRIMARY KEY,\n value TEXT\n) STRICT, WITHOUT ROWID", tbl_name: "parent", type: "table" } @@ -152,7 +158,8 @@ defmodule MigrationsParseTest do table_info: %{ name: "child", rootpage: 3, - sql: "CREATE TABLE child (\n id INTEGER PRIMARY KEY,\n daddy INTEGER NOT NULL,\n FOREIGN KEY(daddy) REFERENCES parent(id)\n) STRICT, WITHOUT ROWID", + sql: + "CREATE TABLE child (\n id INTEGER PRIMARY KEY,\n daddy INTEGER NOT NULL,\n FOREIGN KEY(daddy) REFERENCES parent(id)\n) STRICT, WITHOUT ROWID", tbl_name: "child", type: "table" } @@ -162,7 +169,6 @@ defmodule MigrationsParseTest do assert info == expected_info end - test "tests getting uniques" do sql_in = """ CREATE TABLE IF NOT EXISTS parent ( @@ -178,7 +184,10 @@ defmodule MigrationsParseTest do ) STRICT, WITHOUT ROWID; """ - info = Electric.Migrations.Parse.sql_ast_from_migration_set([%Electric.Migration{name: "test1", original_body: sql_in}]) + info = + Electric.Migrations.Parse.sql_ast_from_migration_set([ + %Electric.Migration{name: "test1", original_body: sql_in} + ]) expected_info = %{ "main.child" => %{ @@ -224,7 +233,8 @@ defmodule MigrationsParseTest do table_info: %{ name: "child", rootpage: 4, - sql: "CREATE TABLE child (\n id INTEGER PRIMARY KEY,\n daddy INTEGER NOT NULL,\n FOREIGN KEY(daddy) REFERENCES parent(id)\n) STRICT, WITHOUT ROWID", + sql: + "CREATE TABLE child (\n id INTEGER PRIMARY KEY,\n daddy INTEGER NOT NULL,\n FOREIGN KEY(daddy) REFERENCES parent(id)\n) STRICT, WITHOUT ROWID", tbl_name: "child", type: "table" }, @@ -272,7 +282,8 @@ defmodule MigrationsParseTest do table_info: %{ name: "parent", rootpage: 2, - sql: "CREATE TABLE parent (\n id INTEGER PRIMARY KEY DESC,\n value TEXT,\n email TEXT UNIQUE\n) STRICT, WITHOUT ROWID", + sql: + "CREATE TABLE parent (\n id INTEGER PRIMARY KEY DESC,\n value TEXT,\n email TEXT UNIQUE\n) STRICT, WITHOUT ROWID", tbl_name: "parent", type: "table" }, @@ -284,7 +295,6 @@ defmodule MigrationsParseTest do end test "tests getting SQL index structure" do - sql_in = """ CREATE TABLE IF NOT EXISTS parent ( id INTEGER PRIMARY KEY, @@ -299,42 +309,45 @@ defmodule MigrationsParseTest do ) STRICT, WITHOUT ROWID; """ - table_info = Electric.Migrations.Parse.sql_ast_from_migration_set([%Electric.Migration{name: "test1", original_body: sql_in}]) - ##IO.inspect(table_info) + table_info = + Electric.Migrations.Parse.sql_ast_from_migration_set([ + %Electric.Migration{name: "test1", original_body: sql_in} + ]) + + ## IO.inspect(table_info) index_info = Electric.Migrations.Parse.all_index_info([sql_in]) assert index_info == %{ - "main.parent" => %{ - 0 => %{ - columns: [ - %{cid: 0, coll: "BINARY", desc: 0, key: 1, name: "id", seqno: 0}, - %{cid: 1, coll: "BINARY", desc: 0, key: 0, name: "value", seqno: 1}, - %{cid: 2, coll: "BINARY", desc: 0, key: 0, name: "email", seqno: 2} - ], - name: "sqlite_autoindex_parent_2", - origin: "pk", - partial: 0, - seq: 0, - unique: 1 - }, - 1 => %{ - columns: [ - %{cid: 2, coll: "BINARY", desc: 0, key: 1, name: "email", seqno: 0}, - %{cid: 0, coll: "BINARY", desc: 0, key: 0, name: "id", seqno: 1} - ], - name: "sqlite_autoindex_parent_1", - origin: "u", - partial: 0, - seq: 1, - unique: 1 - } - } - } + "main.parent" => %{ + 0 => %{ + columns: [ + %{cid: 0, coll: "BINARY", desc: 0, key: 1, name: "id", seqno: 0}, + %{cid: 1, coll: "BINARY", desc: 0, key: 0, name: "value", seqno: 1}, + %{cid: 2, coll: "BINARY", desc: 0, key: 0, name: "email", seqno: 2} + ], + name: "sqlite_autoindex_parent_2", + origin: "pk", + partial: 0, + seq: 0, + unique: 1 + }, + 1 => %{ + columns: [ + %{cid: 2, coll: "BINARY", desc: 0, key: 1, name: "email", seqno: 0}, + %{cid: 0, coll: "BINARY", desc: 0, key: 0, name: "id", seqno: 1} + ], + name: "sqlite_autoindex_parent_1", + origin: "u", + partial: 0, + seq: 1, + unique: 1 + } + } + } end test "tests getting SQL conflict" do - sql_in = """ CREATE TABLE IF NOT EXISTS parent ( id INTEGER PRIMARY KEY, @@ -349,8 +362,12 @@ defmodule MigrationsParseTest do ); """ - table_info = Electric.Migrations.Parse.sql_ast_from_migration_set([%Electric.Migration{name: "test1", original_body: sql_in}]) - ##IO.inspect(table_info) + table_info = + Electric.Migrations.Parse.sql_ast_from_migration_set([ + %Electric.Migration{name: "test1", original_body: sql_in} + ]) + + ## IO.inspect(table_info) index_info = Electric.Migrations.Parse.all_index_info([sql_in]) @@ -370,15 +387,5 @@ defmodule MigrationsParseTest do } } end - end - - - - - - - - - -end \ No newline at end of file +end diff --git a/test/migration_test.exs b/test/migration_test.exs index 0b00998..3ff79b8 100644 --- a/test/migration_test.exs +++ b/test/migration_test.exs @@ -17,8 +17,6 @@ defmodule MigrationsTest do assert Electric.Migration.ensure_original_body(migration).error == nil end - - end describe "adds_triggers to sql" do @@ -36,7 +34,10 @@ defmodule MigrationsTest do --ADD A TRIGGER FOR main.fish; """ - assert Electric.Migrations.Triggers.add_triggers_to_last_migration([%Electric.Migration{name: "test1", original_body: sql}], @trigger_template) == + assert Electric.Migrations.Triggers.add_triggers_to_last_migration( + [%Electric.Migration{name: "test1", original_body: sql}], + @trigger_template + ) == expected end @@ -50,7 +51,8 @@ defmodule MigrationsTest do # migration = %Electric.Migration{name: "test1", original_body: sql} sql = - Electric.Migrations.Triggers.add_triggers_to_last_migration([%Electric.Migration{name: "test1", original_body: sql}], + Electric.Migrations.Triggers.add_triggers_to_last_migration( + [%Electric.Migration{name: "test1", original_body: sql}], Electric.Migrations.get_template() ) @@ -257,7 +259,8 @@ defmodule MigrationsTest do """ sql = - Electric.Migrations.Triggers.add_triggers_to_last_migration([%Electric.Migration{name: "test1", original_body: sql}], + Electric.Migrations.Triggers.add_triggers_to_last_migration( + [%Electric.Migration{name: "test1", original_body: sql}], Electric.Migrations.get_template() ) @@ -307,7 +310,8 @@ defmodule MigrationsTest do """ sql = - Electric.Migrations.Triggers.add_triggers_to_last_migration([%Electric.Migration{name: "test1", original_body: sql}], + Electric.Migrations.Triggers.add_triggers_to_last_migration( + [%Electric.Migration{name: "test1", original_body: sql}], Electric.Migrations.get_template() ) @@ -339,7 +343,8 @@ defmodule MigrationsTest do """ sql = - Electric.Migrations.Triggers.add_triggers_to_last_migration([%Electric.Migration{name: "test1", original_body: sql}], + Electric.Migrations.Triggers.add_triggers_to_last_migration( + [%Electric.Migration{name: "test1", original_body: sql}], Electric.Migrations.get_template() ) @@ -388,11 +393,11 @@ defmodule MigrationsTest do """ sql_out1 = - Electric.Migrations.Triggers.add_triggers_to_last_migration([%Electric.Migration{name: "test1", original_body: sql1}], + Electric.Migrations.Triggers.add_triggers_to_last_migration( + [%Electric.Migration{name: "test1", original_body: sql1}], Electric.Migrations.get_template() ) - migration_1 = %Electric.Migration{name: "test1", original_body: sql1} migration_2 = %Electric.Migration{name: "test2", original_body: sql2} @@ -428,12 +433,8 @@ defmodule MigrationsTest do assert newRow == "{\"value\":\"abcdefg\",\"colour\":\"red\"}" assert oldRow == nil end - end - - - def get_while(conn, statement, names) do case Exqlite.Sqlite3.step(conn, statement) do {:row, result} -> From ea2c7995c4727f61fac07af94fb4190ca315863c Mon Sep 17 00:00:00 2001 From: Paul Harter Date: Wed, 28 Sep 2022 18:22:19 +0100 Subject: [PATCH 04/18] tidy up warnings and doc strings --- lib/electric/migrations/generation.ex | 7 +- lib/electric/migrations/migration.ex | 40 ++++--- lib/electric/migrations/migrations.ex | 9 +- lib/electric/migrations/parse.ex | 109 +++++++++--------- .../{ => migrations}/templates/bundle_js.eex | 0 .../{ => migrations}/templates/migration.eex | 0 .../{ => migrations}/templates/triggers.eex | 0 lib/electric/migrations/triggers.ex | 6 +- lib/electric/migrations/validation.ex | 24 ---- test/migration_parse_test.exs | 14 --- 10 files changed, 93 insertions(+), 116 deletions(-) rename lib/electric/{ => migrations}/templates/bundle_js.eex (100%) rename lib/electric/{ => migrations}/templates/migration.eex (100%) rename lib/electric/{ => migrations}/templates/triggers.eex (100%) delete mode 100644 lib/electric/migrations/validation.ex diff --git a/lib/electric/migrations/generation.ex b/lib/electric/migrations/generation.ex index 8599434..16b81fa 100644 --- a/lib/electric/migrations/generation.ex +++ b/lib/electric/migrations/generation.ex @@ -1,8 +1,11 @@ defmodule Electric.Migrations.Generation do @moduledoc """ - + Generates PostgreSQL text from SQLite text """ + @doc """ + Given an ordered list of Electric.Migration objects creates a PostgreSQL file for the last migration in the list + """ def postgres_for_ordered_migrations(ordered_migrations) do {before_ast, after_ast} = before_and_after_ast(ordered_migrations) get_postgres_for_infos(before_ast, after_ast) @@ -32,7 +35,7 @@ defmodule Electric.Migrations.Generation do ## %{cid: 0, dflt_value: nil, name: "id", notnull: 0, pk: 1, type: "INTEGER"} column_definitions = - for {column_id, column_info} <- table_after.column_infos do + for {_column_id, column_info} <- table_after.column_infos do "\n " <> column_def_sql_from_info(column_info, flavour) end diff --git a/lib/electric/migrations/migration.ex b/lib/electric/migrations/migration.ex index a240dad..b24063a 100644 --- a/lib/electric/migrations/migration.ex +++ b/lib/electric/migrations/migration.ex @@ -1,6 +1,6 @@ defmodule Electric.Migration do @moduledoc """ - + A struct to hold information about a single migration """ @migration_file_name "migration.sql" @@ -9,18 +9,30 @@ defmodule Electric.Migration do @enforce_keys [:name] defstruct name: "noname", src_folder: nil, original_body: nil, satellite_body: nil, error: nil + @doc """ + The file path for this migration's original source within the migrations folder + """ def original_file_path(migration) do Path.join([migration.src_folder, migration.name, @migration_file_name]) end + @doc """ + The file path for this migration's satellite SQL file within the migrations folder + """ def satellite_file_path(migration) do Path.join([migration.src_folder, migration.name, @satellite_file_name]) end + @doc """ + The file path for this migration's PostgreSQL file within the migrations folder + """ def postgres_file_path(migration) do Path.join([migration.src_folder, migration.name, @postgres_file_name]) end + @doc """ + reads the original source from file + """ def ensure_original_body(migration) do if migration.original_body == nil do sql = File.read!(original_file_path(migration)) @@ -30,6 +42,9 @@ defmodule Electric.Migration do end end + @doc """ + reads the satellite source from file + """ def ensure_satellite_body(migration) do if migration.satellite_body == nil do sql = File.read!(satellite_file_path(migration)) @@ -39,19 +54,10 @@ defmodule Electric.Migration do end end - # def ensure_and_validate_original_sql(migration) do - # with_body = ensure_original_body(migration) - # {:ok, conn} = Exqlite.Sqlite3.open(":memory:") - # - # case Exqlite.Sqlite3.execute(conn, with_body.original_body) do - # :ok -> - # with_body - # - # {:error, reason} -> - # %{with_body | error: reason} - # end - # end - + @doc """ + reads the satellite metadata from the file header and returns the metadata as a json serialisable map + with_body: is a bool to ask for the satellite migration body itself to be included + """ def as_json_map(migration, with_body) do with_satellite_body = ensure_satellite_body(migration) metadata = get_satellite_metadata(with_satellite_body) @@ -65,11 +71,17 @@ defmodule Electric.Migration do end end + @doc """ + reads the satellite metadata from the file header + """ def get_satellite_metadata(migration) do with_satellite = ensure_satellite_body(migration) get_body_metadata(with_satellite.satellite_body) end + @doc """ + reads the original metadata from the file header + """ def get_original_metadata(migration) do with_original = ensure_original_body(migration) get_body_metadata(with_original.original_body) diff --git a/lib/electric/migrations/migrations.ex b/lib/electric/migrations/migrations.ex index c4fe0aa..c99aa40 100644 --- a/lib/electric/migrations/migrations.ex +++ b/lib/electric/migrations/migrations.ex @@ -5,13 +5,12 @@ defmodule Electric.Migrations do """ @migration_file_name "migration.sql" - @satellite_file_name "satellite.sql" @manifest_file_name "manifest.json" @bundle_file_name "manifest.bundle.json" @js_bundle_file_name "index.js" - @migration_template EEx.compile_file("lib/electric/templates/migration.eex") - @satellite_template EEx.compile_file("lib/electric/templates/triggers.eex") - @bundle_template EEx.compile_file("lib/electric/templates/bundle_js.eex") + @migration_template EEx.compile_file("lib/electric/migrations/templates/migration.eex") + @satellite_template EEx.compile_file("lib/electric/migrations/templates/triggers.eex") + @bundle_template EEx.compile_file("lib/electric/migrations/templates/bundle_js.eex") @doc """ Creates the migrations folder and adds in initial migration to it. @@ -297,6 +296,4 @@ defmodule Electric.Migrations do end end end - - # function end diff --git a/lib/electric/migrations/parse.ex b/lib/electric/migrations/parse.ex index a2cb629..8f54e1b 100644 --- a/lib/electric/migrations/parse.ex +++ b/lib/electric/migrations/parse.ex @@ -1,15 +1,18 @@ defmodule Electric.Migrations.Parse do @moduledoc """ - hello + Creates an AST from SQL migrations """ - @doc false + @doc """ + Given a set of Electric.Migration and returns an ugly map of maps containing info about the DB structure. + Also validates the SQL and returns error messages if validation fails + """ def sql_ast_from_migration_set(migrations) do case ast_from_ordered_migrations(migrations) do {ast, []} -> ast - {ast, errors} -> + {_ast, errors} -> {:error, errors} end end @@ -84,7 +87,7 @@ defmodule Electric.Migrations.Parse do foreign_keys_rows = get_rows_while(conn, foreign_statement, []) foreign_keys = - for [id, seq, table, from, to, on_update, on_delete, match] <- foreign_keys_rows do + for [_id, _seq, table, from, to, _on_update, _on_delete, _match] <- foreign_keys_rows do %{ :child_key => from, :parent_key => to, @@ -136,7 +139,7 @@ defmodule Electric.Migrations.Parse do end end - def check_sql(table_name, sql) do + defp check_sql(table_name, sql) do validation_fails = [] lower = String.downcase(sql) @@ -150,20 +153,17 @@ defmodule Electric.Migrations.Parse do validation_fails end - validation_fails = - if !String.contains?(lower, "without rowid") do - [ - "The table #{table_name} is not WITHOUT ROWID. Add the WITHOUT ROWID option at the end of the create table statement and make sure you also specify a primary key" - | validation_fails - ] - else - validation_fails - end + if !String.contains?(lower, "without rowid") do + [ + "The table #{table_name} is not WITHOUT ROWID. Add the WITHOUT ROWID option at the end of the create table statement and make sure you also specify a primary key" + | validation_fails + ] + else + validation_fails + end end def all_index_info(all_migrations) do - namespace = "main" - # get all the table names {:ok, conn} = Exqlite.Sqlite3.open(":memory:") for migration <- all_migrations do @@ -186,44 +186,43 @@ defmodule Electric.Migrations.Parse do :ok = Exqlite.Sqlite3.release(conn, statement) # for each table - infos = - for [type, name, tbl_name, rootpage, sql] <- info, into: %{} do - # column names - {:ok, info_statement} = Exqlite.Sqlite3.prepare(conn, "PRAGMA index_list(#{tbl_name});") - indexes = Enum.reverse(get_rows_while(conn, info_statement, [])) - - index_infos = - for [seq, name, unique, origin, partial] <- indexes, into: %{} do - {:ok, col_info_statement} = - Exqlite.Sqlite3.prepare(conn, "PRAGMA index_xinfo(#{name});") - - index_columns = Enum.reverse(get_rows_while(conn, col_info_statement, [])) - - index_column_infos = - for [seqno, cid, name, desc, coll, key] <- index_columns do - %{ - :seqno => seqno, - :cid => cid, - :name => name, - :desc => desc, - :coll => coll, - :key => key - } - end - - {seq, - %{ - :seq => seq, - :name => name, - :unique => unique, - :origin => origin, - :partial => partial, - :columns => index_column_infos - }} - end + for [_type, _name, tbl_name, _rootpage, _sql] <- info, into: %{} do + # column names + {:ok, info_statement} = Exqlite.Sqlite3.prepare(conn, "PRAGMA index_list(#{tbl_name});") + indexes = Enum.reverse(get_rows_while(conn, info_statement, [])) - {"main.#{tbl_name}", index_infos} - end + index_infos = + for [seq, name, unique, origin, partial] <- indexes, into: %{} do + {:ok, col_info_statement} = + Exqlite.Sqlite3.prepare(conn, "PRAGMA index_xinfo(#{name});") + + index_columns = Enum.reverse(get_rows_while(conn, col_info_statement, [])) + + index_column_infos = + for [seqno, cid, name, desc, coll, key] <- index_columns do + %{ + :seqno => seqno, + :cid => cid, + :name => name, + :desc => desc, + :coll => coll, + :key => key + } + end + + {seq, + %{ + :seq => seq, + :name => name, + :unique => unique, + :origin => origin, + :partial => partial, + :columns => index_column_infos + }} + end + + {"#{namespace}.#{tbl_name}", index_infos} + end end defp get_rows_while(conn, statement, rows) do @@ -243,7 +242,7 @@ defmodule Electric.Migrations.Parse do _ -> matching_unique_indexes = - for {seq, info} <- indexes do + for {_seq, info} <- indexes do case info.origin do "u" -> cols = @@ -269,7 +268,7 @@ defmodule Electric.Migrations.Parse do _ -> matching_desc_indexes = - for {seq, info} <- indexes do + for {_seq, info} <- indexes do case info.origin do "pk" -> cols = diff --git a/lib/electric/templates/bundle_js.eex b/lib/electric/migrations/templates/bundle_js.eex similarity index 100% rename from lib/electric/templates/bundle_js.eex rename to lib/electric/migrations/templates/bundle_js.eex diff --git a/lib/electric/templates/migration.eex b/lib/electric/migrations/templates/migration.eex similarity index 100% rename from lib/electric/templates/migration.eex rename to lib/electric/migrations/templates/migration.eex diff --git a/lib/electric/templates/triggers.eex b/lib/electric/migrations/templates/triggers.eex similarity index 100% rename from lib/electric/templates/triggers.eex rename to lib/electric/migrations/templates/triggers.eex diff --git a/lib/electric/migrations/triggers.ex b/lib/electric/migrations/triggers.ex index 76e5015..eb1d2bf 100644 --- a/lib/electric/migrations/triggers.ex +++ b/lib/electric/migrations/triggers.ex @@ -1,8 +1,12 @@ defmodule Electric.Migrations.Triggers do @moduledoc """ - + Adds triggers to the Satellite SQLite files with templates to allow integration with electric """ + @doc """ + Given an ordered set of Electric.Migration returns a templated version of the final migration + SQL with all the triggers needed by Satellite added. + """ def add_triggers_to_last_migration(migration_set, template) do case Electric.Migrations.Parse.sql_ast_from_migration_set(migration_set) do {:error, reasons} -> diff --git a/lib/electric/migrations/validation.ex b/lib/electric/migrations/validation.ex deleted file mode 100644 index 2165298..0000000 --- a/lib/electric/migrations/validation.ex +++ /dev/null @@ -1,24 +0,0 @@ -defmodule Electric.Migrations.Validation do - @moduledoc """ - - """ - - def validate(ordered_migrations) do - # ast = case sql_ast_from_ordered_migrations(ordered_migrations) do - # {:error, reason} - # end - end - - # def ensure_and_validate_original_sql(migration) do - # with_body = ensure_original_body(migration) - # {:ok, conn} = Exqlite.Sqlite3.open(":memory:") - # - # case Exqlite.Sqlite3.execute(conn, with_body.original_body) do - # :ok -> - # with_body - # - # {:error, reason} -> - # %{with_body | error: reason} - # end - # end -end diff --git a/test/migration_parse_test.exs b/test/migration_parse_test.exs index c6e2b01..ee65ff5 100644 --- a/test/migration_parse_test.exs +++ b/test/migration_parse_test.exs @@ -309,13 +309,6 @@ defmodule MigrationsParseTest do ) STRICT, WITHOUT ROWID; """ - table_info = - Electric.Migrations.Parse.sql_ast_from_migration_set([ - %Electric.Migration{name: "test1", original_body: sql_in} - ]) - - ## IO.inspect(table_info) - index_info = Electric.Migrations.Parse.all_index_info([sql_in]) assert index_info == %{ @@ -362,13 +355,6 @@ defmodule MigrationsParseTest do ); """ - table_info = - Electric.Migrations.Parse.sql_ast_from_migration_set([ - %Electric.Migration{name: "test1", original_body: sql_in} - ]) - - ## IO.inspect(table_info) - index_info = Electric.Migrations.Parse.all_index_info([sql_in]) assert index_info == %{ From 24b001fd1935378a76e21208a29a7344750eb565 Mon Sep 17 00:00:00 2001 From: James Arthur Date: Fri, 30 Sep 2022 17:58:05 +0200 Subject: [PATCH 05/18] hotfix: `check_migrations_folder` uses wrong option name. --- lib/electric/migrations/migrations.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/electric/migrations/migrations.ex b/lib/electric/migrations/migrations.ex index c99aa40..0717249 100644 --- a/lib/electric/migrations/migrations.ex +++ b/lib/electric/migrations/migrations.ex @@ -106,7 +106,7 @@ defmodule Electric.Migrations do end defp check_migrations_folder(options) do - migrations_folder = Map.get(options, :migrations, "migrations") + migrations_folder = Map.get(options, :dir, "migrations") if !File.exists?(migrations_folder) do {:error, "Couldn't find the migrations folder at #{migrations_folder}"} From 7be134f6dc7a85be2508679d4028c47b38a755f3 Mon Sep 17 00:00:00 2001 From: James Arthur Date: Sun, 2 Oct 2022 14:28:48 +0200 Subject: [PATCH 06/18] migrations: avoid swallowing build errors. The previous control flow was breaking silently when validation failed in any except the first migration file. This commit also demonstrates slightly more standard Elixir patterns: - using `with` to code the happy path in `build_migrations` whilst explicitly handling the return value of all the operations - use reduce instead of try catch for control flow in `add_triggers_to_migrations` --- lib/electric/migrations/migrations.ex | 66 ++++++++++++--------------- test/migration_test.exs | 2 +- 2 files changed, 31 insertions(+), 37 deletions(-) diff --git a/lib/electric/migrations/migrations.ex b/lib/electric/migrations/migrations.ex index 0717249..d1bacc8 100644 --- a/lib/electric/migrations/migrations.ex +++ b/lib/electric/migrations/migrations.ex @@ -74,31 +74,28 @@ defmodule Electric.Migrations do - :bundle will also create a index.js file in the migrations folder which exports a js object containing all the migrations """ def build_migrations(flags, options) do - case check_migrations_folder(options) do - {:ok, migrations_folder} -> - template = Map.get(options, :template, @satellite_template) - migration_set = ordered_migrations(migrations_folder) - add_triggers_to_migrations(migration_set, template) - - if flags[:manifest] do - write_manifest(migrations_folder) - end - - if flags[:json] do - write_bundle(migrations_folder) - end - - if flags[:bundle] do - write_js_bundle(migrations_folder) - end - - {:success, "Migrations built"} - + template = Map.get(options, :template, @satellite_template) + + with {:ok, folder} <- check_migrations_folder(options), + :ok <- folder |> ordered_migrations() |> add_triggers_to_migrations(template), + :ok <- optionally_write(&write_js_bundle/1, folder, flags[:bundle]), + :ok <- optionally_write(&write_json_bundle/1, folder, flags[:json]), + :ok <- optionally_write(&write_manifest/1, folder, flags[:manifest]) do + {:success, "Migrations built"} + else {:error, msg} -> {:error, msg} end end + defp optionally_write(func, folder, flag) when flag !== true do + :ok + end + + defp optionally_write(func, folder, flag) do + func.(folder) + end + @doc """ Does nothing yet """ @@ -158,7 +155,7 @@ defmodule Electric.Migrations do end @doc false - def write_bundle(src_folder) do + def write_json_bundle(src_folder) do migrations = create_bundle(src_folder, true) bundle_path = Path.join(src_folder, @bundle_file_name) @@ -224,22 +221,19 @@ defmodule Electric.Migrations do if length(failed_validation) > 0 do {:error, failed_validation} else - try do - for {_migration, i} <- Enum.with_index(validated_migrations) do - subset_of_migrations = Enum.take(validated_migrations, i + 1) - - # this is using a migration file path and all the migrations up to, an including this migration - case add_triggers_to_migration( - subset_of_migrations, - template - ) do - :ok -> :ok - {:error, reason} -> throw({:error, reason}) - end + validated_migrations + |> Enum.with_index() + |> Enum.reduce(nil, fn {_migration, i}, acc -> + case acc do + alt when alt in [nil, :ok] -> + validated_migrations + |> Enum.take(i + 1) + |> add_triggers_to_migration(template) + + {:error, reason} -> + {:error, reason} end - catch - {:error, reason} -> {:error, reason} - end + end) end end diff --git a/test/migration_test.exs b/test/migration_test.exs index 3ff79b8..7135647 100644 --- a/test/migration_test.exs +++ b/test/migration_test.exs @@ -770,7 +770,7 @@ defmodule MigrationsFileTest do } ) - Electric.Migrations.write_bundle(migrations_folder) + Electric.Migrations.write_json_bundle(migrations_folder) bundle = Jason.decode!(File.read!(bundle_path)) # IO.inspect(bundle) From 4e82529251408089ca721bb244ab7b2d48bce09c Mon Sep 17 00:00:00 2001 From: James Arthur Date: Sun, 2 Oct 2022 14:32:09 +0200 Subject: [PATCH 07/18] migrations: add an `_electric_migrations` table. --- lib/electric/migrations/templates/triggers.eex | 10 +++++++++- test/migration_test.exs | 11 ++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/electric/migrations/templates/triggers.eex b/lib/electric/migrations/templates/triggers.eex index 6cc91c3..d3a9503 100644 --- a/lib/electric/migrations/templates/triggers.eex +++ b/lib/electric/migrations/templates/triggers.eex @@ -21,7 +21,15 @@ CREATE TABLE IF NOT EXISTS _electric_meta ( value TEXT ); ---initialisation of the metadata table +-- Somewhere to track migrations +CREATE TABLE IF NOT EXISTS _electric_migrations ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + name TEXT UNIQUE, + sha256 TEXT + applied_at TEXT +); + +-- Initialisation of the metadata table INSERT INTO _electric_meta(key,value) VALUES ('currRowId', '-1'), ('ackRowId','-1'), ('compensations', 0); <% end %> diff --git a/test/migration_test.exs b/test/migration_test.exs index 7135647..c093200 100644 --- a/test/migration_test.exs +++ b/test/migration_test.exs @@ -123,7 +123,15 @@ defmodule MigrationsTest do value TEXT ); - --initialisation of the metadata table + -- Somewhere to track migrations + CREATE TABLE IF NOT EXISTS _electric_migrations ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + name TEXT UNIQUE, + sha256 TEXT + applied_at TEXT + ); + + -- Initialisation of the metadata table INSERT INTO _electric_meta(key,value) VALUES ('currRowId', '-1'), ('ackRowId','-1'), ('compensations', 0); @@ -278,6 +286,7 @@ defmodule MigrationsTest do MapSet.new([ "_electric_oplog", "_electric_meta", + "_electric_migrations", "fish", "sqlite_sequence", "trigger_settings" From 68e9dc41fb7879762ac64d9a047113530290b6b2 Mon Sep 17 00:00:00 2001 From: James Arthur Date: Sun, 2 Oct 2022 14:33:35 +0200 Subject: [PATCH 08/18] tweak: underscore unused variables. --- lib/electric/migrations/migrations.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/electric/migrations/migrations.ex b/lib/electric/migrations/migrations.ex index d1bacc8..a809585 100644 --- a/lib/electric/migrations/migrations.ex +++ b/lib/electric/migrations/migrations.ex @@ -88,11 +88,11 @@ defmodule Electric.Migrations do end end - defp optionally_write(func, folder, flag) when flag !== true do + defp optionally_write(_func, _folder, flag) when flag !== true do :ok end - defp optionally_write(func, folder, flag) do + defp optionally_write(func, folder, _flag) do func.(folder) end From 2c14d315c1f4b8dc2f4e180baebd7d73aa013040 Mon Sep 17 00:00:00 2001 From: James Arthur Date: Sun, 2 Oct 2022 14:39:18 +0200 Subject: [PATCH 09/18] tests: fix tests to use `:dir` option. --- test/migration_test.exs | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/test/migration_test.exs b/test/migration_test.exs index c093200..1c01ee5 100644 --- a/test/migration_test.exs +++ b/test/migration_test.exs @@ -506,8 +506,7 @@ defmodule MigrationsFileTest do File.write!(my_new_migration, new_content, [:append]) - {:success, _msg} = - Electric.Migrations.build_migrations(%{}, %{:migrations => migrations_path}) + {:success, _msg} = Electric.Migrations.build_migrations(%{}, %{:dir => migrations_path}) assert File.exists?(Path.join([migration_folder, "satellite.sql"])) end @@ -527,8 +526,7 @@ defmodule MigrationsFileTest do File.write!(my_new_migration, new_content, [:append]) Process.sleep(1000) - {:success, _msg} = - Electric.Migrations.new_migration("another", %{:migrations => migrations_path}) + {:success, _msg} = Electric.Migrations.new_migration("another", %{:dir => migrations_path}) cats_content = """ CREATE TABLE IF NOT EXISTS cats ( @@ -550,8 +548,7 @@ defmodule MigrationsFileTest do init_and_add_migration(temp) second_migration_folder = Path.dirname(most_recent_migration_file(migrations_path)) - {:success, _msg} = - Electric.Migrations.build_migrations(%{}, %{:migrations => migrations_path}) + {:success, _msg} = Electric.Migrations.build_migrations(%{}, %{:dir => migrations_path}) assert File.exists?(Path.join([second_migration_folder, "satellite.sql"])) end @@ -563,7 +560,7 @@ defmodule MigrationsFileTest do {:success, _msg} = Electric.Migrations.build_migrations(%{:manifest => true}, %{ - :migrations => migrations_path + :dir => migrations_path }) assert File.exists?(Path.join([migrations_path, "manifest.json"])) @@ -575,7 +572,7 @@ defmodule MigrationsFileTest do init_and_add_migration(temp) {:success, _msg} = - Electric.Migrations.build_migrations(%{:bundle => true}, %{:migrations => migrations_path}) + Electric.Migrations.build_migrations(%{:bundle => true}, %{:dir => migrations_path}) assert File.exists?(Path.join([migrations_path, "index.js"])) end @@ -586,7 +583,7 @@ defmodule MigrationsFileTest do init_and_add_migration(temp) {:success, _msg} = - Electric.Migrations.build_migrations(%{:bundle => true}, %{:migrations => migrations_path}) + Electric.Migrations.build_migrations(%{:bundle => true}, %{:dir => migrations_path}) assert File.exists?(Path.join([migrations_path, "index.js"])) end @@ -596,8 +593,7 @@ defmodule MigrationsFileTest do migrations_path = Path.join([temp, "migrations"]) init_and_add_migration(temp) - {:success, _msg} = - Electric.Migrations.build_migrations(%{}, %{:migrations => migrations_path}) + {:success, _msg} = Electric.Migrations.build_migrations(%{}, %{:dir => migrations_path}) migration = most_recent_migration_file(migrations_path) @@ -609,8 +605,7 @@ defmodule MigrationsFileTest do File.write!(migration, dogs_content, [:append]) - {:success, _msg} = - Electric.Migrations.build_migrations(%{}, %{:migrations => migrations_path}) + {:success, _msg} = Electric.Migrations.build_migrations(%{}, %{:dir => migrations_path}) end end @@ -675,7 +670,7 @@ defmodule MigrationsFileTest do Electric.Migrations.build_migrations( %{}, %{ - :migrations => migrations_folder, + :dir => migrations_folder, :template => @trigger_template } ) @@ -721,7 +716,7 @@ defmodule MigrationsFileTest do Electric.Migrations.build_migrations( %{}, %{ - :migrations => migrations_folder + :dir => migrations_folder } ) @@ -774,7 +769,7 @@ defmodule MigrationsFileTest do Electric.Migrations.build_migrations( %{}, %{ - :migrations => migrations_folder, + :dir => migrations_folder, :template => @trigger_template } ) @@ -837,7 +832,7 @@ defmodule MigrationsFileTest do Electric.Migrations.build_migrations( %{}, %{ - :migrations => migrations_folder, + :dir => migrations_folder, :template => @trigger_template } ) From e7d3b6304e546b63ce26b8394abdd481f49789fc Mon Sep 17 00:00:00 2001 From: James Arthur Date: Sun, 2 Oct 2022 15:18:45 +0200 Subject: [PATCH 10/18] migrations: whoops forgot the non nulls. --- lib/electric/migrations/templates/triggers.eex | 6 +++--- test/migration_test.exs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/electric/migrations/templates/triggers.eex b/lib/electric/migrations/templates/triggers.eex index d3a9503..8f7c580 100644 --- a/lib/electric/migrations/templates/triggers.eex +++ b/lib/electric/migrations/templates/triggers.eex @@ -24,9 +24,9 @@ CREATE TABLE IF NOT EXISTS _electric_meta ( -- Somewhere to track migrations CREATE TABLE IF NOT EXISTS _electric_migrations ( id INTEGER PRIMARY KEY AUTOINCREMENT, - name TEXT UNIQUE, - sha256 TEXT - applied_at TEXT + name TEXT NOT NULL UNIQUE, + sha256 TEXT NOT NULL, + applied_at TEXT NOT NULL ); -- Initialisation of the metadata table diff --git a/test/migration_test.exs b/test/migration_test.exs index 1c01ee5..eea0e12 100644 --- a/test/migration_test.exs +++ b/test/migration_test.exs @@ -126,9 +126,9 @@ defmodule MigrationsTest do -- Somewhere to track migrations CREATE TABLE IF NOT EXISTS _electric_migrations ( id INTEGER PRIMARY KEY AUTOINCREMENT, - name TEXT UNIQUE, - sha256 TEXT - applied_at TEXT + name TEXT NOT NULL UNIQUE, + sha256 TEXT NOT NULL, + applied_at TEXT NOT NULL ); -- Initialisation of the metadata table From 8b073ea02582029dd3be5631f85641265f5e16e2 Mon Sep 17 00:00:00 2001 From: James Arthur Date: Sun, 2 Oct 2022 17:10:15 +0200 Subject: [PATCH 11/18] migrations: tweak naming in the bundle template. --- lib/electric/migrations/templates/bundle_js.eex | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/electric/migrations/templates/bundle_js.eex b/lib/electric/migrations/templates/bundle_js.eex index 5ca2832..f6cec9a 100644 --- a/lib/electric/migrations/templates/bundle_js.eex +++ b/lib/electric/migrations/templates/bundle_js.eex @@ -1,3 +1 @@ -const _migrations = <%= migrations %>; - -export const migrations = _migrations; +export const data = <%= migrations %> From 7a92b599c154540bdb7d67e57b4582e84f2b063a Mon Sep 17 00:00:00 2001 From: James Arthur Date: Sun, 2 Oct 2022 18:34:55 +0200 Subject: [PATCH 12/18] migrations: tweak the initial meta entries. --- lib/electric/migrations/templates/triggers.eex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/electric/migrations/templates/triggers.eex b/lib/electric/migrations/templates/triggers.eex index 8f7c580..ebb1a3d 100644 --- a/lib/electric/migrations/templates/triggers.eex +++ b/lib/electric/migrations/templates/triggers.eex @@ -30,7 +30,7 @@ CREATE TABLE IF NOT EXISTS _electric_migrations ( ); -- Initialisation of the metadata table -INSERT INTO _electric_meta(key,value) VALUES ('currRowId', '-1'), ('ackRowId','-1'), ('compensations', 0); +INSERT INTO _electric_meta (key, value) VALUES ('compensations', 0), ('lastAckdRowId','0'), ('lastSentRowId', '0'), ('lsn', '0'); <% end %> -- These are toggles for turning the triggers on and off From 1da0afc80eeedffd5c07b190248631cfd137cd00 Mon Sep 17 00:00:00 2001 From: James Arthur Date: Sun, 2 Oct 2022 18:36:06 +0200 Subject: [PATCH 13/18] migrations: consistently namespace the system tables. --- .../migrations/templates/triggers.eex | 16 +++++----- test/migration_test.exs | 30 +++++++++---------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/lib/electric/migrations/templates/triggers.eex b/lib/electric/migrations/templates/triggers.eex index ebb1a3d..3ad52c0 100644 --- a/lib/electric/migrations/templates/triggers.eex +++ b/lib/electric/migrations/templates/triggers.eex @@ -34,9 +34,9 @@ INSERT INTO _electric_meta (key, value) VALUES ('compensations', 0), ('lastAckdR <% end %> -- These are toggles for turning the triggers on and off -DROP TABLE IF EXISTS trigger_settings; -CREATE TABLE trigger_settings(tablename STRING PRIMARY KEY, flag INTEGER); -<%= for {table_full_name, _table} <- tables do %>INSERT INTO trigger_settings(tablename,flag) VALUES ('<%= table_full_name %>', 1); +DROP TABLE IF EXISTS _electric_trigger_settings; +CREATE TABLE _electric_trigger_settings(tablename STRING PRIMARY KEY, flag INTEGER); +<%= for {table_full_name, _table} <- tables do %>INSERT INTO _electric_trigger_settings(tablename,flag) VALUES ('<%= table_full_name %>', 1); <% end %> <%= for {table_full_name, table} <- tables do %> /* Triggers for table <%= table.table_name %> */ @@ -58,7 +58,7 @@ END; DROP TRIGGER IF EXISTS insert_<%= table.namespace %>_<%= table.table_name %>_into_oplog; CREATE TRIGGER insert_<%= table.namespace %>_<%= table.table_name %>_into_oplog AFTER INSERT ON <%= table_full_name %> - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == '<%= table_full_name %>') + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == '<%= table_full_name %>') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) VALUES ('<%= table.namespace %>', '<%= table.table_name %>', 'INSERT', json_object(<%= Enum.join(for col <- table.primary do "'#{col}', new.#{col}" end, ", ") %>), json_object(<%= Enum.join(for col <- table[:columns] do "'#{col}', new.#{col}" end, ", ") %>), NULL, NULL); @@ -67,7 +67,7 @@ END; DROP TRIGGER IF EXISTS update_<%= table.namespace %>_<%= table.table_name %>_into_oplog; CREATE TRIGGER update_<%= table.namespace %>_<%= table.table_name %>_into_oplog AFTER UPDATE ON <%= table_full_name %> - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == '<%= table_full_name %>') + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == '<%= table_full_name %>') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) VALUES ('<%= table.namespace %>', '<%= table.table_name %>', 'UPDATE', json_object(<%= Enum.join(for col <- table.primary do "'#{col}', new.#{col}" end, ", ") %>), json_object(<%= Enum.join(for col <- table.columns do "'#{col}', new.#{col}" end, ", ") %>), json_object(<%= Enum.join(for col <- table.columns do "'#{col}', old.#{col}" end, ", ") %>), NULL); @@ -76,7 +76,7 @@ END; DROP TRIGGER IF EXISTS delete_<%= table.namespace %>_<%= table.table_name %>_into_oplog; CREATE TRIGGER delete_<%= table.namespace %>_<%= table.table_name %>_into_oplog AFTER DELETE ON <%= table_full_name %> - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == '<%= table_full_name %>') + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == '<%= table_full_name %>') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) VALUES ('<%= table.namespace %>', '<%= table.table_name %>', 'DELETE', json_object(<%= Enum.join(for col <- table.primary do "'#{col}', old.#{col}" end, ", ") %>), NULL, json_object(<%= Enum.join(for col <- table.columns do "'#{col}', old.#{col}" end, ", ") %>), NULL); @@ -87,7 +87,7 @@ END; DROP TRIGGER IF EXISTS compensation_insert_<%= table.namespace %>_<%= table.table_name %>_<%= foreign_key.child_key %>_into_oplog; CREATE TRIGGER compensation_insert_<%= table.namespace %>_<%= table.table_name %>_<%= foreign_key.child_key %>_into_oplog AFTER INSERT ON <%= table_full_name %> - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == '<%= tables[foreign_key.table].namespace %>.<%= tables[foreign_key.table].table_name %>') AND + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == '<%= tables[foreign_key.table].namespace %>.<%= tables[foreign_key.table].table_name %>') AND 1 == (SELECT value from _electric_meta WHERE key == 'compensations') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) @@ -98,7 +98,7 @@ END; DROP TRIGGER IF EXISTS compensation_update_<%= table.namespace %>_<%= table.table_name %>_<%= foreign_key.child_key %>_into_oplog; CREATE TRIGGER compensation_update_<%= table.namespace %>_<%= table.table_name %>_<%= foreign_key.child_key %>_into_oplog AFTER UPDATE ON <%= table.namespace %>.<%= table.table_name %> - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == '<%= tables[foreign_key.table].namespace %>.<%= tables[foreign_key.table].table_name %>') AND + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == '<%= tables[foreign_key.table].namespace %>.<%= tables[foreign_key.table].table_name %>') AND 1 == (SELECT value from _electric_meta WHERE key == 'compensations') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) diff --git a/test/migration_test.exs b/test/migration_test.exs index eea0e12..9f4bad1 100644 --- a/test/migration_test.exs +++ b/test/migration_test.exs @@ -132,14 +132,14 @@ defmodule MigrationsTest do ); -- Initialisation of the metadata table - INSERT INTO _electric_meta(key,value) VALUES ('currRowId', '-1'), ('ackRowId','-1'), ('compensations', 0); + INSERT INTO _electric_meta (key, value) VALUES ('compensations', 0), ('lastAckdRowId','0'), ('lastSentRowId', '0'), ('lsn', '0'); -- These are toggles for turning the triggers on and off - DROP TABLE IF EXISTS trigger_settings; - CREATE TABLE trigger_settings(tablename STRING PRIMARY KEY, flag INTEGER); - INSERT INTO trigger_settings(tablename,flag) VALUES ('main.cats', 1); - INSERT INTO trigger_settings(tablename,flag) VALUES ('main.fish', 1); + DROP TABLE IF EXISTS _electric_trigger_settings; + CREATE TABLE _electric_trigger_settings(tablename STRING PRIMARY KEY, flag INTEGER); + INSERT INTO _electric_trigger_settings(tablename,flag) VALUES ('main.cats', 1); + INSERT INTO _electric_trigger_settings(tablename,flag) VALUES ('main.fish', 1); /* Triggers for table cats */ @@ -161,7 +161,7 @@ defmodule MigrationsTest do DROP TRIGGER IF EXISTS insert_main_cats_into_oplog; CREATE TRIGGER insert_main_cats_into_oplog AFTER INSERT ON main.cats - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == 'main.cats') + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == 'main.cats') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) VALUES ('main', 'cats', 'INSERT', json_object('id', new.id), json_object('id', new.id, 'name', new.name, 'favourite', new.favourite), NULL, NULL); @@ -170,7 +170,7 @@ defmodule MigrationsTest do DROP TRIGGER IF EXISTS update_main_cats_into_oplog; CREATE TRIGGER update_main_cats_into_oplog AFTER UPDATE ON main.cats - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == 'main.cats') + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == 'main.cats') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) VALUES ('main', 'cats', 'UPDATE', json_object('id', new.id), json_object('id', new.id, 'name', new.name, 'favourite', new.favourite), json_object('id', old.id, 'name', old.name, 'favourite', old.favourite), NULL); @@ -179,7 +179,7 @@ defmodule MigrationsTest do DROP TRIGGER IF EXISTS delete_main_cats_into_oplog; CREATE TRIGGER delete_main_cats_into_oplog AFTER DELETE ON main.cats - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == 'main.cats') + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == 'main.cats') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) VALUES ('main', 'cats', 'DELETE', json_object('id', old.id), NULL, json_object('id', old.id, 'name', old.name, 'favourite', old.favourite), NULL); @@ -190,7 +190,7 @@ defmodule MigrationsTest do DROP TRIGGER IF EXISTS compensation_insert_main_cats_favourite_into_oplog; CREATE TRIGGER compensation_insert_main_cats_favourite_into_oplog AFTER INSERT ON main.cats - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == 'main.fish') AND + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == 'main.fish') AND 1 == (SELECT value from _electric_meta WHERE key == 'compensations') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) @@ -201,7 +201,7 @@ defmodule MigrationsTest do DROP TRIGGER IF EXISTS compensation_update_main_cats_favourite_into_oplog; CREATE TRIGGER compensation_update_main_cats_favourite_into_oplog AFTER UPDATE ON main.cats - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == 'main.fish') AND + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == 'main.fish') AND 1 == (SELECT value from _electric_meta WHERE key == 'compensations') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) @@ -231,7 +231,7 @@ defmodule MigrationsTest do DROP TRIGGER IF EXISTS insert_main_fish_into_oplog; CREATE TRIGGER insert_main_fish_into_oplog AFTER INSERT ON main.fish - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == 'main.fish') + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == 'main.fish') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) VALUES ('main', 'fish', 'INSERT', json_object('id', new.id, 'colour', new.colour), json_object('id', new.id, 'colour', new.colour), NULL, NULL); @@ -240,7 +240,7 @@ defmodule MigrationsTest do DROP TRIGGER IF EXISTS update_main_fish_into_oplog; CREATE TRIGGER update_main_fish_into_oplog AFTER UPDATE ON main.fish - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == 'main.fish') + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == 'main.fish') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) VALUES ('main', 'fish', 'UPDATE', json_object('id', new.id, 'colour', new.colour), json_object('id', new.id, 'colour', new.colour), json_object('id', old.id, 'colour', old.colour), NULL); @@ -249,7 +249,7 @@ defmodule MigrationsTest do DROP TRIGGER IF EXISTS delete_main_fish_into_oplog; CREATE TRIGGER delete_main_fish_into_oplog AFTER DELETE ON main.fish - WHEN 1 == (SELECT flag from trigger_settings WHERE tablename == 'main.fish') + WHEN 1 == (SELECT flag from _electric_trigger_settings WHERE tablename == 'main.fish') BEGIN INSERT INTO _electric_oplog (namespace, tablename, optype, primaryKey, newRow, oldRow, timestamp) VALUES ('main', 'fish', 'DELETE', json_object('id', old.id, 'colour', old.colour), NULL, json_object('id', old.id, 'colour', old.colour), NULL); @@ -287,9 +287,9 @@ defmodule MigrationsTest do "_electric_oplog", "_electric_meta", "_electric_migrations", + "_electric_trigger_settings", "fish", - "sqlite_sequence", - "trigger_settings" + "sqlite_sequence" ]) end From 02904b7a6cebaf5196f249032a70e799a69c23f1 Mon Sep 17 00:00:00 2001 From: Paul Harter Date: Mon, 3 Oct 2022 15:53:35 +0100 Subject: [PATCH 14/18] formatting --- lib/electric/migrations/generation.ex | 255 ++++++++--------- lib/electric/migrations/migration.ex | 28 +- lib/electric/migrations/migrations.ex | 42 +-- lib/electric/migrations/parse.ex | 269 +++++++++--------- .../templates/{bundle_js.eex => index.js.eex} | 0 .../{migration.eex => migration.sql.eex} | 0 .../{triggers.eex => satellite.sql.eex} | 2 +- lib/electric/migrations/triggers.ex | 14 +- test/migration_generation_test.exs | 12 +- test/migration_parse_test.exs | 6 +- test/migration_test.exs | 2 +- 11 files changed, 294 insertions(+), 336 deletions(-) rename lib/electric/migrations/templates/{bundle_js.eex => index.js.eex} (100%) rename lib/electric/migrations/templates/{migration.eex => migration.sql.eex} (100%) rename lib/electric/migrations/templates/{triggers.eex => satellite.sql.eex} (99%) diff --git a/lib/electric/migrations/generation.ex b/lib/electric/migrations/generation.ex index 16b81fa..eddb07f 100644 --- a/lib/electric/migrations/generation.ex +++ b/lib/electric/migrations/generation.ex @@ -3,141 +3,151 @@ defmodule Electric.Migrations.Generation do Generates PostgreSQL text from SQLite text """ + @type flavour() :: :postgresql | :sqlite + alias Electric.Migrations.Parse, as: Parse + @doc """ Given an ordered list of Electric.Migration objects creates a PostgreSQL file for the last migration in the list """ def postgres_for_ordered_migrations(ordered_migrations) do - {before_ast, after_ast} = before_and_after_ast(ordered_migrations) - get_postgres_for_infos(before_ast, after_ast) + with {:ok, before_ast, after_ast} = before_and_after_ast(ordered_migrations) do + postgres_string = get_postgres_for_ast_changes(before_ast, after_ast) + {:ok, postgres_string} + else + {:error, reasons} -> {:error, reasons} + end end defp before_and_after_ast(migration_set) do - after_ast = Electric.Migrations.Parse.sql_ast_from_migration_set(migration_set) - all_but_last_migration_set = Enum.take(migration_set, Enum.count(migration_set) - 1) - before_ast = Electric.Migrations.Parse.sql_ast_from_migration_set(all_but_last_migration_set) - {before_ast, after_ast} + with {:ok, after_ast} = Parse.sql_ast_from_migration_set(migration_set) do + all_but_last_migration_set = Enum.drop(migration_set, -1) + + with {:ok, before_ast} = Parse.sql_ast_from_migration_set(all_but_last_migration_set) do + {:ok, before_ast, after_ast} + else + {:error, reasons} -> {:error, reasons} + end + else + {:error, reasons} -> {:error, reasons} + end end - defp get_postgres_for_infos(before_ast, after_ast) do - get_sql_for_infos(before_ast, after_ast, "postgres") + defp get_postgres_for_ast_changes(before_ast, after_ast) do + get_sql_for_ast_changes(before_ast, after_ast, :postgresql) end - defp get_sqlite_for_infos(before_ast, after_ast) do - get_sql_for_infos(before_ast, after_ast, "sqlite") + defp get_sqlite_for_ast_changes(before_ast, after_ast) do + get_sql_for_ast_changes(before_ast, after_ast, :sqlite) end - defp get_sql_for_infos(before_ast, after_ast, flavour) do - statements = - for change <- table_changes(before_ast, after_ast) do - case change do - {nil, table_after} -> - ## https://www.sqlite.org/syntax/column-constraint.html - ## %{cid: 0, dflt_value: nil, name: "id", notnull: 0, pk: 1, type: "INTEGER"} - - column_definitions = - for {_column_id, column_info} <- table_after.column_infos do - "\n " <> column_def_sql_from_info(column_info, flavour) - end - - foreign_key_clauses = - for foreign_key_info <- table_after.foreign_keys_info do - "\n " <> foreign_key_sql_from_info(foreign_key_info, flavour) - end - - columns_and_keys = column_definitions ++ foreign_key_clauses - - case flavour do - "postgres" -> - "\nCREATE TABLE #{table_after.namespace}.#{table_after.table_name} (#{Enum.join(columns_and_keys, ",")});\n" - - "sqlite" -> - "\nCREATE TABLE #{table_after.namespace}.#{table_after.table_name} (#{Enum.join(columns_and_keys, ",")})STRICT;\n" - end - - {table_before, nil} -> - "DROP TABLE #{table_before.namespace}.#{table_before.table_name};\n" - - {table_before, table_after} -> - ## add columns - added_colums_lines = - for {column_id, column_info} <- table_after.column_infos, - !Map.has_key?(table_before.column_infos, column_id) do - "ALTER TABLE #{table_after.namespace}.#{table_after.table_name} ADD COLUMN #{column_def_sql_from_info(column_info, flavour)};\n" - end - - ## delete columns - dropped_colums_lines = - for {column_id, column_info} <- table_before.column_infos, - !Map.has_key?(table_after.column_infos, column_id) do - "ALTER TABLE #{table_before.namespace}.#{table_before.table_name} DROP COLUMN #{column_info.name};\n" - end - - ## rename columns - rename_colums_lines = - for {column_id, column_info} <- table_after.column_infos, - Map.has_key?(table_before.column_infos, column_id) && - column_info.name != table_before.column_infos[column_id].name do - "ALTER TABLE #{table_after.namespace}.#{table_after.table_name} RENAME COLUMN #{table_before.column_infos[column_id].name} TO #{column_info.name};\n" - end - - all_change_lines = added_colums_lines ++ dropped_colums_lines ++ rename_colums_lines - Enum.join(all_change_lines, " ") - end + defp get_sql_for_ast_changes(before_ast, after_ast, flavour) do + for change <- table_changes(before_ast, after_ast), into: "" do + case change do + {nil, table_after} -> + build_sql_create_table(table_after, flavour) + + {table_before, nil} -> + build_sql_drop_table(table_before) + + {table_before, table_after} -> + build_sql_alter_table(table_before, table_after, flavour) end + end + end - Enum.join(statements, "") + defp table_full_name(table_info) do + "#{table_info.namespace}.#{table_info.table_name}" end - defp table_changes(before_ast, after_ast) do - if before_ast == nil do - for {table_name, table_info} <- after_ast do - {nil, table_info} + defp build_sql_create_table(table_info, flavour) do + ## https://www.sqlite.org/syntax/column-constraint.html + ## %{cid: 0, dflt_value: nil, name: "id", notnull: 0, pk: 1, type: "INTEGER"} + + column_definitions = + Enum.map(table_info.column_infos, fn {_, info} -> + "\n " <> column_def_sql_from_info(info, flavour) + end) + + foreign_key_clauses = + for foreign_key_info <- table_info.foreign_keys_info do + "\n " <> foreign_key_sql_from_info(foreign_key_info, flavour) end - else - new_and_changed = - for {table_name, table_info} <- after_ast, table_info != before_ast[table_name] do - {before_ast[table_name], table_info} - end - dropped = - for {table_name, table_info} <- before_ast, !Map.has_key?(after_ast, table_name) do - {table_info, nil} - end + columns_and_keys = Enum.join(column_definitions ++ foreign_key_clauses, ",") - new_and_changed ++ dropped + case flavour do + :postgresql -> + "\nCREATE TABLE #{table_full_name(table_info)} (#{columns_and_keys});\n" + + :sqlite -> + "\nCREATE TABLE #{table_full_name(table_info)} (#{columns_and_keys})STRICT;\n" end end - defp foreign_key_sql_from_info(key_info, flavour) do - # DEFERRABLE + defp build_sql_drop_table(table_info) do + "DROP TABLE #{table_full_name(table_info)};\n" + end - # %{from: "daddy", id: 0, match: "NONE", on_delete: "NO ACTION", on_update: "NO ACTION", seq: 0, table: "parent", to: "id"} + defp build_sql_alter_table(table_before, table_after, flavour) do + ## add columns + added_colums_lines = + for {column_id, column_info} <- table_after.column_infos, + not Map.has_key?(table_before.column_infos, column_id) do + "ALTER TABLE #{table_full_name(table_after)} ADD COLUMN #{column_def_sql_from_info(column_info, flavour)};\n" + end - elements = [] + ## delete columns + dropped_colums_lines = + for {column_id, column_info} <- table_before.column_infos, + not Map.has_key?(table_after.column_infos, column_id) do + "ALTER TABLE #{table_full_name(table_after)} DROP COLUMN #{column_info.name};\n" + end - # force postgres to MATCH SIMPLE as sqlite always does this - elements = - case flavour do - "postgres" -> - ["MATCH SIMPLE" | elements] + ## rename columns + rename_colums_lines = + for {column_id, column_info} <- table_after.column_infos, + Map.has_key?(table_before.column_infos, column_id) && + column_info.name != table_before.column_infos[column_id].name do + "ALTER TABLE #{table_full_name(table_after)} RENAME COLUMN #{table_before.column_infos[column_id].name} TO #{column_info.name};\n" + end - _ -> - elements + all_change_lines = added_colums_lines ++ dropped_colums_lines ++ rename_colums_lines + Enum.join(all_change_lines, " ") + end + + defp table_changes(nil, after_ast) do + for {table_name, table_info} <- after_ast do + {nil, table_info} + end + end + + defp table_changes(before_ast, after_ast) do + new_and_changed = + for {table_name, table_info} <- after_ast, table_info != before_ast[table_name] do + {before_ast[table_name], table_info} end - elements = - if key_info.on_delete != "NO ACTION" do - ["ON DELETE #{key_info.on_delete}" | elements] - else - elements + dropped = + for {table_name, table_info} <- before_ast, not Map.has_key?(after_ast, table_name) do + {table_info, nil} end + new_and_changed ++ dropped + end + + defp prepend_if(list, true, item) when is_list(list), do: [item | list] + defp prepend_if(list, false, _) when is_list(list), do: list + + defp foreign_key_sql_from_info(key_info, flavour) do + # TODO DEFERRABLE + # key_info looks like this %{from: "daddy", id: 0, match: "NONE", on_delete: "NO ACTION", on_update: "NO ACTION", seq: 0, table: "parent", to: "id"} + elements = - if key_info.on_update != "NO ACTION" do - ["ON UPDATE #{key_info.on_update}" | elements] - else - elements - end + [] + # force postgres to MATCH SIMPLE as sqlite always does this + |> prepend_if(flavour == :postgresql, "MATCH SIMPLE") + |> prepend_if(key_info.on_delete != "NO ACTION", "ON DELETE #{key_info.on_delete}") + |> prepend_if(key_info.on_update != "NO ACTION", "ON UPDATE #{key_info.on_update}") elements = [ "FOREIGN KEY(#{key_info.from}) REFERENCES #{key_info.table}(#{key_info.to})" | elements @@ -171,7 +181,7 @@ defmodule Electric.Migrations.Generation do type_lookup = case flavour do - "postgres" -> + :postgresql -> %{ "TEXT" => "text", "NUMERIC" => "numeric", @@ -190,39 +200,14 @@ defmodule Electric.Migrations.Generation do } end - elements = [] + sorting = if column_info.pk_desc, do: " DESC", else: "" elements = - if column_info.dflt_value != nil do - ["DEFAULT #{column_info.dflt_value}" | elements] - else - elements - end - - elements = - if column_info.unique do - ["UNIQUE" | elements] - else - elements - end - - elements = - if column_info.notnull != 0 && column_info.pk == 0 do - ["NOT NULL" | elements] - else - elements - end - - elements = - if column_info.pk != 0 do - ["PRIMARY KEY#{if column_info.pk_desc do - " DESC" - else - "" - end}" | elements] - else - elements - end + [] + |> prepend_if(column_info.dflt_value != nil, "DEFAULT #{column_info.dflt_value}") + |> prepend_if(column_info.unique, "UNIQUE") + |> prepend_if(column_info.notnull != 0 && column_info.pk == 0, "NOT NULL") + |> prepend_if(column_info.pk != 0, "PRIMARY KEY#{sorting}") elements = [type_lookup[column_info.type] | elements] elements = [column_info.name | elements] diff --git a/lib/electric/migrations/migration.ex b/lib/electric/migrations/migration.ex index b24063a..3077c75 100644 --- a/lib/electric/migrations/migration.ex +++ b/lib/electric/migrations/migration.ex @@ -33,25 +33,25 @@ defmodule Electric.Migration do @doc """ reads the original source from file """ - def ensure_original_body(migration) do - if migration.original_body == nil do - sql = File.read!(original_file_path(migration)) - %{migration | original_body: sql} - else - migration - end + def ensure_original_body(migration) when migration.original_body == nil do + sql = File.read!(original_file_path(migration)) + %{migration | original_body: sql} + end + + def ensure_original_body(migration) when migration.original_body != nil do + migration end @doc """ reads the satellite source from file """ - def ensure_satellite_body(migration) do - if migration.satellite_body == nil do - sql = File.read!(satellite_file_path(migration)) - %{migration | satellite_body: sql} - else - migration - end + def ensure_satellite_body(migration) when migration.satellite_body == nil do + sql = File.read!(satellite_file_path(migration)) + %{migration | satellite_body: sql} + end + + def ensure_satellite_body(migration) when migration.satellite_body != nil do + migration end @doc """ diff --git a/lib/electric/migrations/migrations.ex b/lib/electric/migrations/migrations.ex index a809585..ef1796a 100644 --- a/lib/electric/migrations/migrations.ex +++ b/lib/electric/migrations/migrations.ex @@ -8,9 +8,9 @@ defmodule Electric.Migrations do @manifest_file_name "manifest.json" @bundle_file_name "manifest.bundle.json" @js_bundle_file_name "index.js" - @migration_template EEx.compile_file("lib/electric/migrations/templates/migration.eex") - @satellite_template EEx.compile_file("lib/electric/migrations/templates/triggers.eex") - @bundle_template EEx.compile_file("lib/electric/migrations/templates/bundle_js.eex") + @migration_template EEx.compile_file("lib/electric/migrations/templates/migration.sql.eex") + @satellite_template EEx.compile_file("lib/electric/migrations/templates/satellite.sql.eex") + @bundle_template EEx.compile_file("lib/electric/migrations/templates/index.js.eex") @doc """ Creates the migrations folder and adds in initial migration to it. @@ -105,7 +105,7 @@ defmodule Electric.Migrations do defp check_migrations_folder(options) do migrations_folder = Map.get(options, :dir, "migrations") - if !File.exists?(migrations_folder) do + if not File.exists?(migrations_folder) do {:error, "Couldn't find the migrations folder at #{migrations_folder}"} else if Path.basename(migrations_folder) == "migrations" do @@ -210,31 +210,19 @@ defmodule Electric.Migrations do end defp add_triggers_to_migrations(ordered_migrations, template) do - validated_migrations = + read_migrations = for migration <- ordered_migrations do Electric.Migration.ensure_original_body(migration) end - failed_validation = - Enum.filter(validated_migrations, fn migration -> migration.error != nil end) - - if length(failed_validation) > 0 do - {:error, failed_validation} - else - validated_migrations - |> Enum.with_index() - |> Enum.reduce(nil, fn {_migration, i}, acc -> - case acc do - alt when alt in [nil, :ok] -> - validated_migrations - |> Enum.take(i + 1) - |> add_triggers_to_migration(template) - - {:error, reason} -> - {:error, reason} - end - end) - end + 1..length(read_migrations) + |> Enum.map(&Enum.take(read_migrations, &1)) + |> Enum.reduce_while(:ok, fn subset, _ -> + case add_triggers_to_migration(subset, template) do + :ok -> {:cont, :ok} + {:error, reason} -> {:halt, {:error, reason}} + end + end) end @doc false @@ -255,7 +243,7 @@ defmodule Electric.Migrations do migration.original_body end - postgres_version = + {:ok, postgres_version} = Electric.Migrations.Generation.postgres_for_ordered_migrations(migration_set) hash = calc_hash(migration_fingerprint) @@ -272,7 +260,7 @@ defmodule Electric.Migrations do end end - if !File.exists?(satellite_file_path) do + if not File.exists?(satellite_file_path) do header = case Electric.Migration.get_original_metadata(migration) do {:error, _reason} -> diff --git a/lib/electric/migrations/parse.ex b/lib/electric/migrations/parse.ex index 8f54e1b..0c6bcef 100644 --- a/lib/electric/migrations/parse.ex +++ b/lib/electric/migrations/parse.ex @@ -10,7 +10,7 @@ defmodule Electric.Migrations.Parse do def sql_ast_from_migration_set(migrations) do case ast_from_ordered_migrations(migrations) do {ast, []} -> - ast + {:ok, ast} {_ast, errors} -> {:error, errors} @@ -45,98 +45,107 @@ defmodule Electric.Migrations.Parse do info = get_rows_while(conn, statement, []) :ok = Exqlite.Sqlite3.release(conn, statement) - # for each table - infos = - for [type, name, tbl_name, rootpage, sql] <- info do - validation_fails = check_sql(tbl_name, sql) + ast = + info + |> generate_ast(namespace, index_info, conn) + |> Map.new(fn info -> {"#{namespace}.#{info.table_name}", info} end) - # column names - {:ok, info_statement} = Exqlite.Sqlite3.prepare(conn, "PRAGMA table_info(#{tbl_name});") - columns = Enum.reverse(get_rows_while(conn, info_statement, [])) + validation_fails = + for {table_name, info} <- ast, length(info.validation_fails) > 0 do + info.validation_fails + end - column_names = - for [_cid, name, _type, _notnull, _dflt_value, _pk] <- columns do - name - end + {ast, List.flatten(validation_fails)} + end + end - column_infos = - for [cid, name, type, notnull, dflt_value, pk] <- columns, into: %{} do - {cid, - %{ - :cid => cid, - :name => name, - :type => type, - :notnull => notnull, - :unique => is_unique(name, index_info["#{namespace}.#{tbl_name}"]), - :pk_desc => is_primary_desc(name, index_info["#{namespace}.#{tbl_name}"]), - :dflt_value => dflt_value, - :pk => pk - }} - end + defp generate_ast(all_table_infos, namespace, index_info, conn) do + for table_info <- all_table_infos do + generate_table_ast(table_info, namespace, index_info, conn) + end + end - # private keys columns - private_key_column_names = - for [_cid, name, _type, _notnull, _dflt_value, pk] when pk == 1 <- columns do - name - end + defp generate_table_ast(table_info, namespace, index_info, conn) do + [type, name, tbl_name, rootpage, sql] = table_info - # foreign keys - {:ok, foreign_statement} = - Exqlite.Sqlite3.prepare(conn, "PRAGMA foreign_key_list(#{tbl_name});") + validation_fails = check_sql(tbl_name, sql) - foreign_keys_rows = get_rows_while(conn, foreign_statement, []) + # column names + {:ok, info_statement} = Exqlite.Sqlite3.prepare(conn, "PRAGMA table_info(#{tbl_name});") + columns = Enum.reverse(get_rows_while(conn, info_statement, [])) - foreign_keys = - for [_id, _seq, table, from, to, _on_update, _on_delete, _match] <- foreign_keys_rows do - %{ - :child_key => from, - :parent_key => to, - :table => "#{namespace}.#{table}" - } - end + column_names = + for [_cid, name, _type, _notnull, _dflt_value, _pk] <- columns do + name + end - foreign_keys_info = - for [id, seq, table, from, to, on_update, on_delete, match] <- foreign_keys_rows do - %{ - :id => id, - :seq => seq, - :table => table, - :from => from, - :to => to, - :on_update => on_update, - :on_delete => on_delete, - :match => match - } - end + column_infos = + for [cid, name, type, notnull, dflt_value, pk] <- columns, into: %{} do + {cid, + %{ + cid: cid, + name: name, + type: type, + notnull: notnull, + unique: is_unique(name, index_info["#{namespace}.#{tbl_name}"]), + pk_desc: is_primary_desc(name, index_info["#{namespace}.#{tbl_name}"]), + dflt_value: dflt_value, + pk: pk + }} + end - %{ - :table_name => tbl_name, - :table_info => %{ - type: type, - name: name, - tbl_name: tbl_name, - rootpage: rootpage, - sql: sql - }, - :columns => column_names, - :namespace => namespace, - :primary => private_key_column_names, - :foreign_keys => foreign_keys, - :column_infos => column_infos, - :foreign_keys_info => foreign_keys_info, - :validation_fails => validation_fails - } - end + # private keys columns + private_key_column_names = + for [_cid, name, _type, _notnull, _dflt_value, pk] when pk == 1 <- columns do + name + end - ast = Enum.into(infos, %{}, fn info -> {"#{namespace}.#{info.table_name}", info} end) + # foreign keys + {:ok, foreign_statement} = + Exqlite.Sqlite3.prepare(conn, "PRAGMA foreign_key_list(#{tbl_name});") - validation_fails = - for info <- infos, length(info.validation_fails) > 0 do - info.validation_fails - end + foreign_keys_rows = get_rows_while(conn, foreign_statement, []) - {ast, List.flatten(validation_fails)} - end + foreign_keys = + for [_id, _seq, table, from, to, _on_update, _on_delete, _match] <- foreign_keys_rows do + %{ + child_key: from, + parent_key: to, + table: "#{namespace}.#{table}" + } + end + + foreign_keys_info = + for [id, seq, table, from, to, on_update, on_delete, match] <- foreign_keys_rows do + %{ + id: id, + seq: seq, + table: table, + from: from, + to: to, + on_update: on_update, + on_delete: on_delete, + match: match + } + end + + %{ + table_name: tbl_name, + table_info: %{ + type: type, + name: name, + tbl_name: tbl_name, + rootpage: rootpage, + sql: sql + }, + columns: column_names, + namespace: namespace, + primary: private_key_column_names, + foreign_keys: foreign_keys, + column_infos: column_infos, + foreign_keys_info: foreign_keys_info, + validation_fails: validation_fails + } end defp check_sql(table_name, sql) do @@ -144,7 +153,7 @@ defmodule Electric.Migrations.Parse do lower = String.downcase(sql) validation_fails = - if !String.contains?(lower, "strict") do + if not String.contains?(lower, "strict") do [ "The table #{table_name} is not STRICT. Add the STRICT option at the end of the create table statement" | validation_fails @@ -153,7 +162,7 @@ defmodule Electric.Migrations.Parse do validation_fails end - if !String.contains?(lower, "without rowid") do + if not String.contains?(lower, "without rowid") do [ "The table #{table_name} is not WITHOUT ROWID. Add the WITHOUT ROWID option at the end of the create table statement and make sure you also specify a primary key" | validation_fails @@ -201,23 +210,23 @@ defmodule Electric.Migrations.Parse do index_column_infos = for [seqno, cid, name, desc, coll, key] <- index_columns do %{ - :seqno => seqno, - :cid => cid, - :name => name, - :desc => desc, - :coll => coll, - :key => key + seqno: seqno, + cid: cid, + name: name, + desc: desc, + coll: coll, + key: key } end {seq, %{ - :seq => seq, - :name => name, - :unique => unique, - :origin => origin, - :partial => partial, - :columns => index_column_infos + seq: seq, + name: name, + unique: unique, + origin: origin, + partial: partial, + columns: index_column_infos }} end @@ -235,55 +244,33 @@ defmodule Electric.Migrations.Parse do end end - defp is_unique(column_name, indexes) do - case indexes do - nil -> - false - - _ -> - matching_unique_indexes = - for {_seq, info} <- indexes do - case info.origin do - "u" -> - cols = - for key_column <- info.columns do - key_column.key == 1 && key_column.name == column_name - end - - Enum.any?(cols) - - _ -> - false - end - end + defp is_unique(column_name, nil) do + false + end - Enum.any?(matching_unique_indexes) - end + defp is_unique(column_name, indexes) do + matching_unique_indexes = + for {_, info} <- indexes, + info.origin == "u", + key_column <- info.columns, + key_column.key == 1 && key_column.name == column_name, + do: true + + Enum.any?(matching_unique_indexes) end - defp is_primary_desc(column_name, indexes) do - case indexes do - nil -> - false - - _ -> - matching_desc_indexes = - for {_seq, info} <- indexes do - case info.origin do - "pk" -> - cols = - for key_column <- info.columns do - key_column.key == 1 && key_column.name == column_name && key_column.desc == 1 - end - - Enum.any?(cols) - - _ -> - false - end - end + defp is_primary_desc(column_name, indexes) when indexes != nil do + matching_desc_indexes = + for {_, info} <- indexes, + info.origin == "pk", + key_column <- info.columns, + key_column.key == 1 && key_column.name == column_name && key_column.desc == 1, + do: true - Enum.any?(matching_desc_indexes) - end + Enum.any?(matching_desc_indexes) + end + + defp is_primary_desc(column_name, indexes) when indexes == nil do + false end end diff --git a/lib/electric/migrations/templates/bundle_js.eex b/lib/electric/migrations/templates/index.js.eex similarity index 100% rename from lib/electric/migrations/templates/bundle_js.eex rename to lib/electric/migrations/templates/index.js.eex diff --git a/lib/electric/migrations/templates/migration.eex b/lib/electric/migrations/templates/migration.sql.eex similarity index 100% rename from lib/electric/migrations/templates/migration.eex rename to lib/electric/migrations/templates/migration.sql.eex diff --git a/lib/electric/migrations/templates/triggers.eex b/lib/electric/migrations/templates/satellite.sql.eex similarity index 99% rename from lib/electric/migrations/templates/triggers.eex rename to lib/electric/migrations/templates/satellite.sql.eex index 3ad52c0..e7c3ceb 100644 --- a/lib/electric/migrations/templates/triggers.eex +++ b/lib/electric/migrations/templates/satellite.sql.eex @@ -18,7 +18,7 @@ CREATE TABLE IF NOT EXISTS _electric_oplog ( -- Somewhere to keep our metadata CREATE TABLE IF NOT EXISTS _electric_meta ( key TEXT, - value TEXT + value BLOB ); -- Somewhere to track migrations diff --git a/lib/electric/migrations/triggers.ex b/lib/electric/migrations/triggers.ex index eb1d2bf..f17f3ed 100644 --- a/lib/electric/migrations/triggers.ex +++ b/lib/electric/migrations/triggers.ex @@ -8,14 +8,12 @@ defmodule Electric.Migrations.Triggers do SQL with all the triggers needed by Satellite added. """ def add_triggers_to_last_migration(migration_set, template) do - case Electric.Migrations.Parse.sql_ast_from_migration_set(migration_set) do - {:error, reasons} -> - {:error, reasons} - - ast -> - sql_in = List.last(migration_set).original_body - is_init = length(migration_set) == 1 - template_all_the_things(sql_in, ast, template, is_init) + with {:ok, ast} = Electric.Migrations.Parse.sql_ast_from_migration_set(migration_set) do + sql_in = List.last(migration_set).original_body + is_init = length(migration_set) == 1 + template_all_the_things(sql_in, ast, template, is_init) + else + {:error, reasons} -> {:error, reasons} end end diff --git a/test/migration_generation_test.exs b/test/migration_generation_test.exs index 4c5b21e..8fc1c01 100644 --- a/test/migration_generation_test.exs +++ b/test/migration_generation_test.exs @@ -11,7 +11,7 @@ defmodule MigrationsPostgresTest do migration = %Electric.Migration{name: "test1", original_body: sql} - postgres_version = + {:ok, postgres_version} = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) expected = "\nCREATE TABLE main.fish (\n value text PRIMARY KEY);\n" @@ -35,7 +35,7 @@ defmodule MigrationsPostgresTest do migration_1 = %Electric.Migration{name: "test_1", original_body: sql1} migration_2 = %Electric.Migration{name: "test_2", original_body: sql2} - postgres_version = + {:ok, postgres_version} = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration_1, migration_2]) expected = "\nCREATE TABLE main.goat (\n name text PRIMARY KEY);\n" @@ -60,7 +60,7 @@ defmodule MigrationsPostgresTest do migration_1 = %Electric.Migration{name: "test_1", original_body: sql1} migration_2 = %Electric.Migration{name: "test_2", original_body: sql2} - postgres_version = + {:ok, postgres_version} = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration_1, migration_2]) expected = """ @@ -89,7 +89,7 @@ defmodule MigrationsPostgresTest do migration = %Electric.Migration{name: "test1", original_body: sql_in} - postgres_version = + {:ok, postgres_version} = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) expected = """ @@ -118,7 +118,7 @@ defmodule MigrationsPostgresTest do migration = %Electric.Migration{name: "test1", original_body: sql_in} - postgres_version = + {:ok, postgres_version} = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) expected = """ @@ -142,7 +142,7 @@ defmodule MigrationsPostgresTest do migration = %Electric.Migration{name: "test1", original_body: sql_in} - postgres_version = + {:ok, postgres_version} = Electric.Migrations.Generation.postgres_for_ordered_migrations([migration]) expected = """ diff --git a/test/migration_parse_test.exs b/test/migration_parse_test.exs index ee65ff5..9974fd0 100644 --- a/test/migration_parse_test.exs +++ b/test/migration_parse_test.exs @@ -11,7 +11,7 @@ defmodule MigrationsParseTest do """ migration = %Electric.Migration{name: "test1", original_body: sql_in} - info = Electric.Migrations.Parse.sql_ast_from_migration_set([migration]) + {:ok, info} = Electric.Migrations.Parse.sql_ast_from_migration_set([migration]) column_names = info["main.fish"][:columns] assert column_names == ["value", "colour"] @@ -67,7 +67,7 @@ defmodule MigrationsParseTest do ) STRICT, WITHOUT ROWID; """ - info = + {:ok, info} = Electric.Migrations.Parse.sql_ast_from_migration_set([ %Electric.Migration{name: "test1", original_body: sql_in} ]) @@ -184,7 +184,7 @@ defmodule MigrationsParseTest do ) STRICT, WITHOUT ROWID; """ - info = + {:ok, info} = Electric.Migrations.Parse.sql_ast_from_migration_set([ %Electric.Migration{name: "test1", original_body: sql_in} ]) diff --git a/test/migration_test.exs b/test/migration_test.exs index 9f4bad1..daaffe9 100644 --- a/test/migration_test.exs +++ b/test/migration_test.exs @@ -120,7 +120,7 @@ defmodule MigrationsTest do -- Somewhere to keep our metadata CREATE TABLE IF NOT EXISTS _electric_meta ( key TEXT, - value TEXT + value BLOB ); -- Somewhere to track migrations From 207f51bf6e163c7fbe629e4b21bb0a4f2a5d0b1f Mon Sep 17 00:00:00 2001 From: Paul Harter Date: Mon, 3 Oct 2022 16:11:47 +0100 Subject: [PATCH 15/18] reformatting --- lib/electric/migrations/generation.ex | 42 +++++++++++++++------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/lib/electric/migrations/generation.ex b/lib/electric/migrations/generation.ex index eddb07f..06a255a 100644 --- a/lib/electric/migrations/generation.ex +++ b/lib/electric/migrations/generation.ex @@ -10,25 +10,31 @@ defmodule Electric.Migrations.Generation do Given an ordered list of Electric.Migration objects creates a PostgreSQL file for the last migration in the list """ def postgres_for_ordered_migrations(ordered_migrations) do - with {:ok, before_ast, after_ast} = before_and_after_ast(ordered_migrations) do - postgres_string = get_postgres_for_ast_changes(before_ast, after_ast) - {:ok, postgres_string} - else - {:error, reasons} -> {:error, reasons} + case before_and_after_ast(ordered_migrations) do + {:ok, before_ast, after_ast} -> + postgres_string = get_postgres_for_ast_changes(before_ast, after_ast) + {:ok, postgres_string} + + {:error, reasons} -> + {:error, reasons} end end defp before_and_after_ast(migration_set) do - with {:ok, after_ast} = Parse.sql_ast_from_migration_set(migration_set) do - all_but_last_migration_set = Enum.drop(migration_set, -1) + case Parse.sql_ast_from_migration_set(migration_set) do + {:ok, after_ast} -> + all_but_last_migration_set = Enum.drop(migration_set, -1) - with {:ok, before_ast} = Parse.sql_ast_from_migration_set(all_but_last_migration_set) do - {:ok, before_ast, after_ast} - else - {:error, reasons} -> {:error, reasons} - end - else - {:error, reasons} -> {:error, reasons} + case Parse.sql_ast_from_migration_set(all_but_last_migration_set) do + {:ok, before_ast} -> + {:ok, before_ast, after_ast} + + {:error, reasons} -> + {:error, reasons} + end + + {:error, reasons} -> + {:error, reasons} end end @@ -36,9 +42,9 @@ defmodule Electric.Migrations.Generation do get_sql_for_ast_changes(before_ast, after_ast, :postgresql) end - defp get_sqlite_for_ast_changes(before_ast, after_ast) do - get_sql_for_ast_changes(before_ast, after_ast, :sqlite) - end + # defp _get_sqlite_for_ast_changes(before_ast, after_ast) do + # get_sql_for_ast_changes(before_ast, after_ast, :sqlite) + # end defp get_sql_for_ast_changes(before_ast, after_ast, flavour) do for change <- table_changes(before_ast, after_ast), into: "" do @@ -116,7 +122,7 @@ defmodule Electric.Migrations.Generation do end defp table_changes(nil, after_ast) do - for {table_name, table_info} <- after_ast do + for {_, table_info} <- after_ast do {nil, table_info} end end From d27f61d08d92c5301de200ff670f571eeee353aa Mon Sep 17 00:00:00 2001 From: Paul Harter Date: Mon, 3 Oct 2022 17:04:12 +0100 Subject: [PATCH 16/18] putting back with in before_and_after_ast --- lib/electric/migrations/generation.ex | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/lib/electric/migrations/generation.ex b/lib/electric/migrations/generation.ex index 06a255a..bbfec71 100644 --- a/lib/electric/migrations/generation.ex +++ b/lib/electric/migrations/generation.ex @@ -21,20 +21,10 @@ defmodule Electric.Migrations.Generation do end defp before_and_after_ast(migration_set) do - case Parse.sql_ast_from_migration_set(migration_set) do - {:ok, after_ast} -> - all_but_last_migration_set = Enum.drop(migration_set, -1) - - case Parse.sql_ast_from_migration_set(all_but_last_migration_set) do - {:ok, before_ast} -> - {:ok, before_ast, after_ast} - - {:error, reasons} -> - {:error, reasons} - end - - {:error, reasons} -> - {:error, reasons} + with {:ok, after_ast} <- Parse.sql_ast_from_migration_set(migration_set), + all_but_last_migration_set = Enum.drop(migration_set, -1), + {:ok, before_ast} <- Parse.sql_ast_from_migration_set(all_but_last_migration_set) do + {:ok, before_ast, after_ast} end end From d9679a2706df7c9a36851d50cab6c790bedb52ea Mon Sep 17 00:00:00 2001 From: Paul Harter Date: Mon, 3 Oct 2022 17:20:07 +0100 Subject: [PATCH 17/18] tidy up column SQL code generation --- lib/electric/migrations/generation.ex | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/electric/migrations/generation.ex b/lib/electric/migrations/generation.ex index bbfec71..68ef9e4 100644 --- a/lib/electric/migrations/generation.ex +++ b/lib/electric/migrations/generation.ex @@ -60,16 +60,13 @@ defmodule Electric.Migrations.Generation do ## %{cid: 0, dflt_value: nil, name: "id", notnull: 0, pk: 1, type: "INTEGER"} column_definitions = - Enum.map(table_info.column_infos, fn {_, info} -> - "\n " <> column_def_sql_from_info(info, flavour) - end) + table_info.column_infos + |> Enum.map(fn {_, info} -> info end) + |> Enum.map(&column_def_sql_from_info(&1, flavour)) foreign_key_clauses = - for foreign_key_info <- table_info.foreign_keys_info do - "\n " <> foreign_key_sql_from_info(foreign_key_info, flavour) - end - - columns_and_keys = Enum.join(column_definitions ++ foreign_key_clauses, ",") + Enum.map(table_info.foreign_keys_info, &foreign_key_sql_from_info(&1, flavour)) + columns_and_keys = "\n " <> Enum.join(column_definitions ++ foreign_key_clauses, ",\n ") case flavour do :postgresql -> From 73a35f1b98f6b5ba278a71015bd2fa3be24f5910 Mon Sep 17 00:00:00 2001 From: Paul Harter Date: Mon, 3 Oct 2022 18:59:54 +0100 Subject: [PATCH 18/18] fixing add_migration only returning success --- lib/electric/commands/migrations.ex | 15 ++++++++++++--- lib/electric/migrations/generation.ex | 1 + lib/electric/migrations/migrations.ex | 16 +++------------- lib/electric/migrations/parse.ex | 2 +- test/migration_parse_test.exs | 11 +++++------ test/migration_test.exs | 22 +++++++++++----------- 6 files changed, 33 insertions(+), 34 deletions(-) diff --git a/lib/electric/commands/migrations.ex b/lib/electric/commands/migrations.ex index d685bf9..8f3b40f 100644 --- a/lib/electric/commands/migrations.ex +++ b/lib/electric/commands/migrations.ex @@ -141,15 +141,24 @@ defmodule Electric.Commands.Migrations do end def init(%{args: _args, flags: _flags, options: options, unknown: _unknown}) do - Electric.Migrations.init_migrations(options) + case Electric.Migrations.init_migrations(options) do + {:ok, message} -> {:success, message} + {:error, message} -> {:error, message} + end end def new(%{args: args, flags: _flags, options: options, unknown: _unknown}) do - Electric.Migrations.new_migration(args.migration_name, options) + case Electric.Migrations.new_migration(args.migration_name, options) do + {:ok, message} -> {:success, message} + {:error, message} -> {:error, message} + end end def build(%{args: _args, flags: flags, options: options, unknown: _unknown}) do - Electric.Migrations.build_migrations(flags, options) + case Electric.Migrations.build_migrations(flags, options) do + {:ok, message} -> {:success, message} + {:error, message} -> {:error, message} + end end def sync(%{args: %{database_id: database_id}, options: %{dir: dir}}) do diff --git a/lib/electric/migrations/generation.ex b/lib/electric/migrations/generation.ex index 68ef9e4..f7daaee 100644 --- a/lib/electric/migrations/generation.ex +++ b/lib/electric/migrations/generation.ex @@ -66,6 +66,7 @@ defmodule Electric.Migrations.Generation do foreign_key_clauses = Enum.map(table_info.foreign_keys_info, &foreign_key_sql_from_info(&1, flavour)) + columns_and_keys = "\n " <> Enum.join(column_definitions ++ foreign_key_clauses, ",\n ") case flavour do diff --git a/lib/electric/migrations/migrations.ex b/lib/electric/migrations/migrations.ex index ef1796a..9864b7f 100644 --- a/lib/electric/migrations/migrations.ex +++ b/lib/electric/migrations/migrations.ex @@ -35,17 +35,7 @@ defmodule Electric.Migrations do {:error, "Migrations folder at #{migrations_folder} already exists."} else File.mkdir_p!(migrations_folder) - - case add_migration(migrations_folder, "init") do - {:ok, _} -> - {:success, "Your migrations folder with an initial migration has been created"} - - {:success, _} -> - {:success, "Your migrations folder with an initial migration has been created"} - - {:error, msg} -> - {:error, msg} - end + add_migration(migrations_folder, "init") end end @@ -81,7 +71,7 @@ defmodule Electric.Migrations do :ok <- optionally_write(&write_js_bundle/1, folder, flags[:bundle]), :ok <- optionally_write(&write_json_bundle/1, folder, flags[:json]), :ok <- optionally_write(&write_manifest/1, folder, flags[:manifest]) do - {:success, "Migrations built"} + {:ok, "Migrations built"} else {:error, msg} -> {:error, msg} @@ -135,7 +125,7 @@ defmodule Electric.Migrations do migration_file_path = Path.join([migration_folder, @migration_file_name]) File.write!(migration_file_path, body) - {:success, "Migration file created at #{migration_file_path}"} + {:ok, "Migration file created at #{migration_file_path}"} end def get_template() do diff --git a/lib/electric/migrations/parse.ex b/lib/electric/migrations/parse.ex index 0c6bcef..86da0a8 100644 --- a/lib/electric/migrations/parse.ex +++ b/lib/electric/migrations/parse.ex @@ -13,7 +13,7 @@ defmodule Electric.Migrations.Parse do {:ok, ast} {_ast, errors} -> - {:error, errors} + {:error, Enum.join(errors, "\n")} end end diff --git a/test/migration_parse_test.exs b/test/migration_parse_test.exs index 9974fd0..47f7ad2 100644 --- a/test/migration_parse_test.exs +++ b/test/migration_parse_test.exs @@ -25,7 +25,7 @@ defmodule MigrationsParseTest do SOME BOLLOCKS; """ - {:error, [reason]} = + {:error, reason} = Electric.Migrations.Parse.sql_ast_from_migration_set([ %Electric.Migration{name: "test1", original_body: sql_in} ]) @@ -41,16 +41,15 @@ defmodule MigrationsParseTest do ); """ - {:error, [reason1, reason2]} = + {:error, message} = Electric.Migrations.Parse.sql_ast_from_migration_set([ %Electric.Migration{name: "test1", original_body: sql_in} ]) - assert reason1 == - "The table fish is not WITHOUT ROWID. Add the WITHOUT ROWID option at the end of the create table statement and make sure you also specify a primary key" + expected = + "The table fish is not WITHOUT ROWID. Add the WITHOUT ROWID option at the end of the create table statement and make sure you also specify a primary key\nThe table fish is not STRICT. Add the STRICT option at the end of the create table statement" - assert reason2 == - "The table fish is not STRICT. Add the STRICT option at the end of the create table statement" + assert message == expected end test "tests getting SQL structure for templating" do diff --git a/test/migration_test.exs b/test/migration_test.exs index daaffe9..e721050 100644 --- a/test/migration_test.exs +++ b/test/migration_test.exs @@ -484,14 +484,14 @@ defmodule MigrationsFileTest do test "tests can init" do temp = temp_folder() migrations_path = Path.join([temp, "migrations"]) - {:success, _msg} = Electric.Migrations.init_migrations(%{:dir => temp}) + {:ok, _msg} = Electric.Migrations.init_migrations(%{:dir => temp}) assert File.exists?(migrations_path) end test "init and then modify and then build" do temp = temp_folder() migrations_path = Path.join([temp, "migrations"]) - {:success, _msg} = Electric.Migrations.init_migrations(%{:dir => temp}) + {:ok, _msg} = Electric.Migrations.init_migrations(%{:dir => temp}) assert File.exists?(migrations_path) sql_file_paths = Path.join([migrations_path, "*", "migration.sql"]) |> Path.wildcard() @@ -506,14 +506,14 @@ defmodule MigrationsFileTest do File.write!(my_new_migration, new_content, [:append]) - {:success, _msg} = Electric.Migrations.build_migrations(%{}, %{:dir => migrations_path}) + {:ok, _msg} = Electric.Migrations.build_migrations(%{}, %{:dir => migrations_path}) assert File.exists?(Path.join([migration_folder, "satellite.sql"])) end def init_and_add_migration(temp) do migrations_path = Path.join([temp, "migrations"]) - {:success, _msg} = Electric.Migrations.init_migrations(%{:dir => temp}) + {:ok, _msg} = Electric.Migrations.init_migrations(%{:dir => temp}) my_new_migration = most_recent_migration_file(migrations_path) @@ -526,7 +526,7 @@ defmodule MigrationsFileTest do File.write!(my_new_migration, new_content, [:append]) Process.sleep(1000) - {:success, _msg} = Electric.Migrations.new_migration("another", %{:dir => migrations_path}) + {:ok, _msg} = Electric.Migrations.new_migration("another", %{:dir => migrations_path}) cats_content = """ CREATE TABLE IF NOT EXISTS cats ( @@ -548,7 +548,7 @@ defmodule MigrationsFileTest do init_and_add_migration(temp) second_migration_folder = Path.dirname(most_recent_migration_file(migrations_path)) - {:success, _msg} = Electric.Migrations.build_migrations(%{}, %{:dir => migrations_path}) + {:ok, _msg} = Electric.Migrations.build_migrations(%{}, %{:dir => migrations_path}) assert File.exists?(Path.join([second_migration_folder, "satellite.sql"])) end @@ -558,7 +558,7 @@ defmodule MigrationsFileTest do migrations_path = Path.join([temp, "migrations"]) init_and_add_migration(temp) - {:success, _msg} = + {:ok, _msg} = Electric.Migrations.build_migrations(%{:manifest => true}, %{ :dir => migrations_path }) @@ -571,7 +571,7 @@ defmodule MigrationsFileTest do migrations_path = Path.join([temp, "migrations"]) init_and_add_migration(temp) - {:success, _msg} = + {:ok, _msg} = Electric.Migrations.build_migrations(%{:bundle => true}, %{:dir => migrations_path}) assert File.exists?(Path.join([migrations_path, "index.js"])) @@ -582,7 +582,7 @@ defmodule MigrationsFileTest do migrations_path = Path.join([temp, "migrations"]) init_and_add_migration(temp) - {:success, _msg} = + {:ok, _msg} = Electric.Migrations.build_migrations(%{:bundle => true}, %{:dir => migrations_path}) assert File.exists?(Path.join([migrations_path, "index.js"])) @@ -593,7 +593,7 @@ defmodule MigrationsFileTest do migrations_path = Path.join([temp, "migrations"]) init_and_add_migration(temp) - {:success, _msg} = Electric.Migrations.build_migrations(%{}, %{:dir => migrations_path}) + {:ok, _msg} = Electric.Migrations.build_migrations(%{}, %{:dir => migrations_path}) migration = most_recent_migration_file(migrations_path) @@ -605,7 +605,7 @@ defmodule MigrationsFileTest do File.write!(migration, dogs_content, [:append]) - {:success, _msg} = Electric.Migrations.build_migrations(%{}, %{:dir => migrations_path}) + {:ok, _msg} = Electric.Migrations.build_migrations(%{}, %{:dir => migrations_path}) end end