improvement: use new PendingCodegen error

Author: zachdaniel

.tool-versions

@@ -1,2 +1,2 @@
 erlang 27.0.1
-elixir 1.18.0-otp-27
+elixir 1.18.4-otp-27

lib/data_layer.ex

@@ -568,6 +568,7 @@ defmodule AshPostgres.DataLayer do
   end
 
   def codegen(args) do
+    Mix.Task.reenable("ash_postgres.generate_migrations")
     Mix.Task.run("ash_postgres.generate_migrations", args)
   end
 

lib/migration_generator/migration_generator.ex

@@ -3,8 +3,6 @@ defmodule AshPostgres.MigrationGenerator do
 
   require Logger
 
-  import Mix.Generator
-
   alias AshPostgres.MigrationGenerator.{Operation, Phase}
 
   defstruct snapshot_path: nil,
@@ -52,12 +50,46 @@ defmodule AshPostgres.MigrationGenerator do
       |> Enum.concat(find_repos())
       |> Enum.uniq()
 
-    Mix.shell().info("\nExtension Migrations: ")
-    create_extension_migrations(repos, opts)
-    Mix.shell().info("\nGenerating Tenant Migrations: ")
-    create_migrations(tenant_snapshots, opts, true, snapshots)
-    Mix.shell().info("\nGenerating Migrations:")
-    create_migrations(snapshots, opts, false)
+    extension_migration_files =
+      create_extension_migrations(repos, opts)
+
+    tenant_migration_files =
+      create_migrations(tenant_snapshots, opts, true, snapshots)
+
+    migration_files =
+      create_migrations(snapshots, opts, false)
+
+    case extension_migration_files ++ tenant_migration_files ++ migration_files do
+      [] ->
+        if !opts.check || opts.dry_run do
+          Mix.shell().info(
+            "No changes detected, so no migrations or snapshots have been created."
+          )
+        end
+
+        :ok
+
+      files ->
+        cond do
+          opts.check ->
+            raise Ash.Error.Framework.PendingCodegen,
+              diff: files
+
+          opts.dry_run ->
+            Mix.shell().info(
+              files
+              |> Enum.sort_by(&elem(&1, 0))
+              |> Enum.map_join("\n\n", fn {file, contents} ->
+                "#{file}\n#{contents}"
+              end)
+            )
+
+          true ->
+            Enum.each(files, fn {file, contents} ->
+              Mix.Generator.create_file(file, contents, force?: true)
+            end)
+        end
+    end
   end
 
   defp find_repos do
@@ -225,8 +257,7 @@ defmodule AshPostgres.MigrationGenerator do
         |> Enum.map(fn {_name, extension} -> extension end)
 
       if Enum.empty?(to_install) do
-        Mix.shell().info("No extensions to install")
-        :ok
+        []
       else
         dev = if opts.dev, do: "_dev"
 
@@ -378,40 +409,13 @@ defmodule AshPostgres.MigrationGenerator do
 
         contents = format(migration_file, contents, opts)
 
-        if opts.dry_run do
-          Mix.shell().info(snapshot_contents)
-          Mix.shell().info(contents)
-
-          if opts.check do
-            Mix.shell().error("""
-            Migrations would have been generated, but the --check flag was provided.
-
-            To see what migration would have been generated, run with the `--dry-run`
-            option instead. To generate those migrations, run without either flag.
-            """)
-
-            exit({:shutdown, 1})
-          end
-        else
-          if opts.check do
-            Mix.shell().error("""
-            Migrations would have been generated, but the --check flag was provided.
-
-            To see what migration would have been generated, run with the `--dry-run`
-            option instead. To generate those migrations, run without either flag.
-            """)
-
-            exit({:shutdown, 1})
-          end
-
-          create_file(snapshot_file, snapshot_contents, force: true)
-
-          if !opts.snapshots_only do
-            create_file(migration_file, contents)
-          end
-        end
+        [
+          {snapshot_file, snapshot_contents},
+          {migration_file, contents}
+        ]
       end
     end
+    |> List.flatten()
   end
 
   defp set_ash_functions(snapshot, installed_extensions) do
@@ -429,7 +433,7 @@ defmodule AshPostgres.MigrationGenerator do
   defp create_migrations(snapshots, opts, tenant?, non_tenant_snapshots \\ []) do
     snapshots
     |> Enum.group_by(& &1.repo)
-    |> Enum.each(fn {repo, snapshots} ->
+    |> Enum.flat_map(fn {repo, snapshots} ->
       deduped = deduplicate_snapshots(snapshots, opts, non_tenant_snapshots)
 
       snapshots_with_operations =
@@ -444,18 +448,7 @@ defmodule AshPostgres.MigrationGenerator do
       |> Enum.uniq()
       |> case do
         [] ->
-          tenant_str =
-            if tenant? do
-              "tenant "
-            else
-              ""
-            end
-
-          Mix.shell().info(
-            "No #{tenant_str}changes detected, so no migrations or snapshots have been created."
-          )
-
-          :ok
+          []
 
         operations ->
           dev_migrations = get_dev_migrations(opts, tenant?, repo)
@@ -475,21 +468,12 @@ defmodule AshPostgres.MigrationGenerator do
             end
           end
 
-          if opts.check do
-            Mix.shell().error("""
-            Migrations would have been generated, but the --check flag was provided.
-
-            To see what migration would have been generated, run with the `--dry-run`
-            option instead. To generate those migrations, run without either flag.
-            """)
-
-            exit({:shutdown, 1})
-          end
-
-          if !opts.snapshots_only do
+          if opts.snapshots_only do
+            []
+          else
             operations
             |> split_into_migrations()
-            |> Enum.each(fn operations ->
+            |> Enum.map(fn operations ->
               run_without_transaction? =
                 Enum.any?(operations, fn
                   %Operation.AddCustomIndex{index: %{concurrently: true}} ->
@@ -505,11 +489,10 @@ defmodule AshPostgres.MigrationGenerator do
               operations
               |> organize_operations
               |> build_up_and_down()
-              |> write_migration!(repo, opts, tenant?, run_without_transaction?)
+              |> migration(repo, opts, tenant?, run_without_transaction?)
             end)
           end
-
-          create_new_snapshot(snapshots, repo_name(repo), opts, tenant?)
+          |> Enum.concat(create_new_snapshot(snapshots, repo_name(repo), opts, tenant?))
       end
     end)
   end
@@ -1110,7 +1093,7 @@ defmodule AshPostgres.MigrationGenerator do
     repo |> Module.split() |> List.last() |> Macro.underscore()
   end
 
-  defp write_migration!({up, down}, repo, opts, tenant?, run_without_transaction?) do
+  defp migration({up, down}, repo, opts, tenant?, run_without_transaction?) do
     migration_path = migration_path(opts, repo, tenant?)
 
     require_name!(opts)
@@ -1185,13 +1168,7 @@ defmodule AshPostgres.MigrationGenerator do
     """
 
     try do
-      contents = format(migration_file, contents, opts)
-
-      if opts.dry_run do
-        Mix.shell().info(contents)
-      else
-        create_file(migration_file, contents)
-      end
+      {migration_file, format(migration_file, contents, opts)}
     rescue
       exception ->
         reraise(
@@ -1223,45 +1200,44 @@ defmodule AshPostgres.MigrationGenerator do
   end
 
   defp create_new_snapshot(snapshots, repo_name, opts, tenant?) do
-    if !opts.dry_run do
-      Enum.each(snapshots, fn snapshot ->
-        snapshot_binary = snapshot_to_binary(snapshot)
+    Enum.map(snapshots, fn snapshot ->
+      snapshot_binary = snapshot_to_binary(snapshot)
 
-        snapshot_folder =
-          if tenant? do
-            opts
-            |> snapshot_path(snapshot.repo)
-            |> Path.join(repo_name)
-            |> Path.join("tenants")
-          else
-            opts
-            |> snapshot_path(snapshot.repo)
-            |> Path.join(repo_name)
-          end
+      snapshot_folder =
+        if tenant? do
+          opts
+          |> snapshot_path(snapshot.repo)
+          |> Path.join(repo_name)
+          |> Path.join("tenants")
+        else
+          opts
+          |> snapshot_path(snapshot.repo)
+          |> Path.join(repo_name)
+        end
 
-        dev = if opts.dev, do: "_dev"
+      dev = if opts.dev, do: "_dev"
 
-        snapshot_file =
-          if snapshot.schema do
-            Path.join(
-              snapshot_folder,
-              "#{snapshot.schema}.#{snapshot.table}/#{timestamp()}#{dev}.json"
-            )
-          else
-            Path.join(snapshot_folder, "#{snapshot.table}/#{timestamp()}#{dev}.json")
-          end
+      snapshot_file =
+        if snapshot.schema do
+          Path.join(
+            snapshot_folder,
+            "#{snapshot.schema}.#{snapshot.table}/#{timestamp()}#{dev}.json"
+          )
+        else
+          Path.join(snapshot_folder, "#{snapshot.table}/#{timestamp()}#{dev}.json")
+        end
 
-        File.mkdir_p(Path.dirname(snapshot_file))
-        create_file(snapshot_file, snapshot_binary, force: true)
+      File.mkdir_p(Path.dirname(snapshot_file))
 
-        old_snapshot_folder = Path.join(snapshot_folder, "#{snapshot.table}#{dev}.json")
+      old_snapshot_folder = Path.join(snapshot_folder, "#{snapshot.table}#{dev}.json")
 
-        if File.exists?(old_snapshot_folder) do
-          new_snapshot_folder = Path.join(snapshot_folder, "#{snapshot.table}/initial#{dev}.json")
-          File.rename(old_snapshot_folder, new_snapshot_folder)
-        end
-      end)
-    end
+      if File.exists?(old_snapshot_folder) do
+        new_snapshot_folder = Path.join(snapshot_folder, "#{snapshot.table}/initial#{dev}.json")
+        File.rename(old_snapshot_folder, new_snapshot_folder)
+      end
+
+      {snapshot_file, snapshot_binary}
+    end)
   end
 
   @doc false
@@ -2961,13 +2937,13 @@ defmodule AshPostgres.MigrationGenerator do
 
   defp renaming?(table, removing, opts) do
     if opts.dev do
+      false
+    else
       if opts.no_shell? do
         raise "Unimplemented: cannot determine: Are you renaming #{table}.#{removing.source}? without shell input"
       else
         yes?(opts, "Are you renaming #{table}.#{removing.source}?")
       end
-    else
-      false
     end
   end
 

mix.exs

@@ -166,7 +166,8 @@ defmodule AshPostgres.MixProject do
   # Run "mix help deps" to learn about dependencies.
   defp deps do
     [
-      {:ash, ash_version("~> 3.4 and >= 3.4.69")},
+      # {:ash, ash_version("~> 3.4 and >= 3.4.69")},
+      {:ash, github: "ash-project/ash", override: true},
       {:ash_sql, ash_sql_version("~> 0.2 and >= 0.2.72")},
       {:igniter, "~> 0.6", optional: true},
       {:ecto_sql, "~> 3.12"},
@@ -189,24 +190,24 @@ defmodule AshPostgres.MixProject do
     ]
   end
 
-  defp ash_version(default_version) do
-    case System.get_env("ASH_VERSION") do
-      nil ->
-        default_version
+  # defp ash_version(default_version) do
+  #   case System.get_env("ASH_VERSION") do
+  #     nil ->
+  #       default_version
 
-      "local" ->
-        [path: "../ash", override: true]
+  #     "local" ->
+  #       [path: "../ash", override: true]
 
-      "main" ->
-        [git: "https://github.com/ash-project/ash.git", override: true]
+  #     "main" ->
+  #       [git: "https://github.com/ash-project/ash.git", override: true]
 
-      version when is_binary(version) ->
-        "~> #{version}"
+  #     version when is_binary(version) ->
+  #       "~> #{version}"
 
-      version ->
-        version
-    end
-  end
+  #     version ->
+  #       version
+  #   end
+  # end
 
   defp ash_sql_version(default_version) do
     case System.get_env("ASH_SQL_VERSION") do

mix.lock

@@ -1,5 +1,5 @@
 %{
-  "ash": {:hex, :ash, "3.5.12", "435a6916d47e4ed6eabce886de2443d17af9dd9ca9765a29c73d9e02507b86b3", [:mix], [{:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8", [hex: :ets, repo: "hexpm", optional: false]}, {:igniter, "~> 0.6", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:owl, "~> 0.11", [hex: :owl, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: true]}, {:plug, ">= 0.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:reactor, "~> 0.11", [hex: :reactor, repo: "hexpm", optional: false]}, {:simple_sat, ">= 0.1.1 and < 1.0.0-0", [hex: :simple_sat, repo: "hexpm", optional: true]}, {:spark, ">= 2.2.60 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, ">= 0.2.6 and < 1.0.0-0", [hex: :splode, repo: "hexpm", optional: false]}, {:stream_data, "~> 1.0", [hex: :stream_data, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.1", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "503492989c56e33c300d731b3717d1df9eeebba2b9018e5dd9f330db727edb57"},
+  "ash": {:git, "https://github.com/ash-project/ash.git", "e7b1a738644e1092ed350115ebebf5da8ac2aec1", []},
   "ash_sql": {:hex, :ash_sql, "0.2.76", "4afac3284194f3d7820b7edc1263ac1eb232a25734406ad7b2284683b9384205", [:mix], [{:ash, "~> 3.5", [hex: :ash, repo: "hexpm", optional: false]}, {:ecto, "~> 3.9", [hex: :ecto, repo: "hexpm", optional: false]}, {:ecto_sql, "~> 3.9", [hex: :ecto_sql, repo: "hexpm", optional: false]}], "hexpm", "f6bc02d8c4cba3f8f9a532e6ff73eaf8a4f7685257646a58fe7a320cfaf10c39"},
   "benchee": {:hex, :benchee, "1.4.0", "9f1f96a30ac80bab94faad644b39a9031d5632e517416a8ab0a6b0ac4df124ce", [:mix], [{:deep_merge, "~> 1.0", [hex: :deep_merge, repo: "hexpm", optional: false]}, {:statistex, "~> 1.0", [hex: :statistex, repo: "hexpm", optional: false]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "299cd10dd8ce51c9ea3ddb74bb150f93d25e968f93e4c1fa31698a8e4fa5d715"},
   "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"},

test/migration_generator_test.exs

@@ -1420,15 +1420,15 @@ defmodule AshPostgres.MigrationGeneratorTest do
       [domain: Domain]
     end
 
-    test "returns code(1) if snapshots and resources don't fit", %{domain: domain} do
-      assert catch_exit(
-               AshPostgres.MigrationGenerator.generate(domain,
-                 snapshot_path: "test_snapshots_path",
-                 migration_path: "test_migration_path",
-                 check: true,
-                 auto_name: true
-               )
-             ) == {:shutdown, 1}
+    test "raises an error on pending codegen", %{domain: domain} do
+      assert_raise Ash.Error.Framework.PendingCodegen, fn ->
+        AshPostgres.MigrationGenerator.generate(domain,
+          snapshot_path: "test_snapshots_path",
+          migration_path: "test_migration_path",
+          check: true,
+          auto_name: true
+        )
+      end
 
       refute File.exists?(Path.wildcard("test_migration_path2/**/*_migrate_resources*.exs"))
       refute File.exists?(Path.wildcard("test_snapshots_path2/test_repo/posts/*.json"))