improvement: better error messages from mix tasks

fix: various broken behavior from new aggregate work
improvement: validate that references refer to relationships

closes #99

lib/aggregate.ex

@@ -130,15 +130,13 @@ defmodule AshPostgres.Aggregate do
              aggregate,
              relationship_path,
              first_relationship
-           ),
-         {:ok, agg_query} <-
-           apply_agg_authorization_filter(
-             agg_query,
-             aggregate,
-             relationship_path,
-             first_relationship
            ) do
-      {:ok, agg_query}
+      apply_agg_authorization_filter(
+        agg_query,
+        aggregate,
+        relationship_path,
+        first_relationship
+      )
     end
   end
 
@@ -214,7 +212,7 @@ defmodule AshPostgres.Aggregate do
        ) do
     field = first_relationship.destination_attribute
 
-    subquery =
+    new_subquery =
       from(row in subquery,
         select_merge: map(row, ^[field]),
         group_by: field(row, ^first_relationship.destination_attribute),
@@ -226,7 +224,7 @@ defmodule AshPostgres.Aggregate do
         opts,
         source_binding,
         subquery.__ash_bindings__.current - 1,
-        subquery
+        new_subquery
       )
 
     subquery = AshPostgres.Join.set_join_prefix(subquery, query, first_relationship.destination)

lib/custom_aggregate.ex

@@ -1,4 +1,21 @@
 defmodule AshPostgres.CustomAggregate do
+  @moduledoc """
+  A custom aggregate implementation for ecto.
+  """
+
+  @doc """
+  The dynamic expression to create the aggregate.
+
+  The binding refers to the resource being aggregated,
+  use `as(^binding)` to reference it.
+
+  For example:
+
+      Ecto.Query.dynamic(
+        [],
+        fragment("string_agg(?, ?)", field(as(^binding), ^opts[:field]), ^opts[:delimiter])
+      )
+  """
   @callback dynamic(opts :: Keyword.t(), binding :: integer) :: Ecto.Query.dynamic()
 
   defmacro __using__(_) do

lib/data_layer.ex

@@ -415,6 +415,7 @@ defmodule AshPostgres.DataLayer do
   use Spark.Dsl.Extension,
     sections: @sections,
     transformers: [
+      AshPostgres.Transformers.ValidateReferences,
       AshPostgres.Transformers.VerifyRepo,
       AshPostgres.Transformers.EnsureTableOrPolymorphic,
       AshPostgres.Transformers.PreventMultidimensionalArrayAggregates
@@ -660,8 +661,6 @@ defmodule AshPostgres.DataLayer do
         destination_resource,
         path
       ) do
-    raise "what"
-
     case lateral_join_query(
            query,
            root_data,

lib/migration_generator/migration_generator.ex

@@ -518,9 +518,6 @@ defmodule AshPostgres.MigrationGenerator do
       [] ->
         nil
 
-      [reference] ->
-        reference
-
       references ->
         %{
           destination_attribute: merge_uniq!(references, table, :destination_attribute, name),

lib/mix/helpers.ex

@@ -1,6 +1,6 @@
 defmodule AshPostgres.MixHelpers do
   @moduledoc false
-  def apis(opts, args) do
+  def apis!(opts, args) do
     apps =
       if apps_paths = Mix.Project.apps_paths() do
         apps_paths |> Map.keys() |> Enum.sort()
@@ -26,16 +26,54 @@ defmodule AshPostgres.MixHelpers do
         configured_apis
       end
 
-    Enum.map(apis, &ensure_compiled(&1, args))
+    apis
+    |> Enum.map(&ensure_compiled(&1, args))
+    |> case do
+      [] ->
+        raise "must supply the --apis argument, or set `config :my_app, ash_apis: [...]` in config"
+
+      apis ->
+        apis
+    end
   end
 
-  def repos(opts, args) do
-    opts
-    |> apis(args)
-    |> Enum.flat_map(&Ash.Api.Info.resources/1)
-    |> Enum.filter(&(Ash.DataLayer.data_layer(&1) == AshPostgres.DataLayer))
+  def repos!(opts, args) do
+    apis = apis!(opts, args)
+
+    resources =
+      apis
+      |> Enum.flat_map(&Ash.Api.Info.resources/1)
+      |> Enum.filter(&(Ash.DataLayer.data_layer(&1) == AshPostgres.DataLayer))
+      |> case do
+        [] ->
+          raise """
+          No resources with `data_layer: AshPostgres.DataLayer` found in the apis #{Enum.map_join(apis, ",", &inspect/1)}.
+
+          Must be able to find at least one resource with `data_layer: AshPostgres.DataLayer`.
+          """
+
+        resources ->
+          resources
+      end
+
+    resources
     |> Enum.map(&AshPostgres.DataLayer.Info.repo(&1))
     |> Enum.uniq()
+    |> case do
+      [] ->
+        raise """
+        No repos could be found configured on the resources in the apis: #{Enum.map_join(apis, ",", &inspect/1)}
+
+        At least one resource must have a repo configured.
+
+        The following resources were found with `data_layer: AshPostgres.DataLayer`:
+
+        #{Enum.map_join(resources, "\n", &"* #{inspect(&1)}")}
+        """
+
+      repos ->
+        repos
+    end
   end
 
   def delete_flag(args, arg) do

lib/mix/tasks/ash_postgres.create.ex

@@ -34,11 +34,7 @@ defmodule Mix.Tasks.AshPostgres.Create do
   def run(args) do
     {opts, _} = OptionParser.parse!(args, strict: @switches, aliases: @aliases)
 
-    repos = AshPostgres.MixHelpers.repos(opts, args)
-
-    if repos == [] do
-      raise "must supply the --apis argument, or set `config :my_app, ash_apis: [...]` in config"
-    end
+    repos = AshPostgres.MixHelpers.repos!(opts, args)
 
     repo_args =
       Enum.flat_map(repos, fn repo ->

lib/mix/tasks/ash_postgres.drop.ex

@@ -44,11 +44,7 @@ defmodule Mix.Tasks.AshPostgres.Drop do
     {opts, _} = OptionParser.parse!(args, strict: @switches, aliases: @aliases)
     opts = Keyword.merge(@default_opts, opts)
 
-    repos = AshPostgres.MixHelpers.repos(opts, args)
-
-    if repos == [] do
-      raise "must supply the --apis argument, or set `config :my_app, ash_apis: [...]` in config"
-    end
+    repos = AshPostgres.MixHelpers.repos!(opts, args)
 
     repo_args =
       Enum.flat_map(repos, fn repo ->

lib/mix/tasks/ash_postgres.generate_migrations.ex

@@ -88,11 +88,7 @@ defmodule Mix.Tasks.AshPostgres.GenerateMigrations do
         ]
       )
 
-    apis = AshPostgres.MixHelpers.apis(opts, args)
-
-    if apis == [] do
-      raise "must supply the --apis argument, or set `config :my_app, ash_apis: [...]` in config"
-    end
+    apis = AshPostgres.MixHelpers.apis!(opts, args)
 
     opts =
       opts

lib/mix/tasks/ash_postgres.migrate.ex

@@ -103,11 +103,7 @@ defmodule Mix.Tasks.AshPostgres.Migrate do
   def run(args) do
     {opts, _} = OptionParser.parse!(args, strict: @switches, aliases: @aliases)
 
-    repos = AshPostgres.MixHelpers.repos(opts, args)
-
-    if repos == [] do
-      raise "must supply the --apis argument, or set `config :my_app, ash_apis: [...]` in config"
-    end
+    repos = AshPostgres.MixHelpers.repos!(opts, args)
 
     repo_args =
       Enum.flat_map(repos, fn repo ->

lib/mix/tasks/ash_postgres.rollback.ex

@@ -62,11 +62,7 @@ defmodule Mix.Tasks.AshPostgres.Rollback do
         aliases: [n: :step, v: :to]
       )
 
-    repos = AshPostgres.MixHelpers.repos(opts, args)
-
-    if repos == [] do
-      raise "must supply the --apis argument, or set `config :my_app, ash_apis: [...]` in config"
-    end
+    repos = AshPostgres.MixHelpers.repos!(opts, args)
 
     repo_args =
       Enum.flat_map(repos, fn repo ->

lib/transformers/prevent_multidimensional_array_aggregates.ex

@@ -1,5 +1,5 @@
 defmodule AshPostgres.Transformers.PreventMultidimensionalArrayAggregates do
-  @moduledoc "Prevents at compile time certain aggregates that are unsupported by `AshPostgres`"
+  @moduledoc "Prevents at compile time certain aggregates that are unsupported by AshPostgres"
   use Spark.Dsl.Transformer
   alias Spark.Dsl.Transformer
 

lib/transformers/validate_references.ex

@@ -0,0 +1,23 @@
+defmodule AshPostgres.Transformers.ValidateReferences do
+  @moduledoc "Ensures that all `references` on a resource refer to a real relationship"
+  use Spark.Dsl.Transformer
+  alias Spark.Dsl.Transformer
+
+  def after_compile?, do: true
+
+  def transform(dsl) do
+    dsl
+    |> AshPostgres.DataLayer.Info.references()
+    |> Enum.each(fn reference ->
+      unless Ash.Resource.Info.relationship(dsl, reference.relationship) do
+        raise Spark.Error.DslError,
+          path: [:postgres, :references, reference.relationship],
+          module: Transformer.get_persisted(dsl, :module),
+          message:
+            "Found reference configuration for relationship `#{reference.relationship}`, but no such relationship exists"
+      end
+    end)
+
+    {:ok, dsl}
+  end
+end

mix.exs

@@ -142,6 +142,7 @@ defmodule AshPostgres.MixProject do
       {:postgrex, ">= 0.0.0"},
       {:ash, ash_version("~> 2.4 and >= 2.4.19")},
       {:git_ops, "~> 2.5.1", only: :dev},
+      {:nimble_options, "~> 0.5"},
       {:ex_doc, "~> 0.22", only: :dev, runtime: false},
       {:ex_check, "~> 0.14", only: :dev},
       {:credo, ">= 0.0.0", only: :dev, runtime: false},

mix.lock

@@ -1,5 +1,5 @@
 %{
-  "ash": {:hex, :ash, "2.4.19", "ba11a4fcf474220dcd4f3bdae708454d090f5b178f22067d22c05d30ba0e67b0", [:mix], [{:comparable, "~> 1.0", [hex: :comparable, repo: "hexpm", optional: false]}, {:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:earmark, "~> 1.4", [hex: :earmark, repo: "hexpm", optional: true]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8.0", [hex: :ets, repo: "hexpm", optional: false]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.4.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: false]}, {:plug, ">= 0.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:spark, ">= 0.2.0", [hex: :spark, repo: "hexpm", optional: false]}, {:stream_data, "~> 0.5.0", [hex: :stream_data, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.1", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "664bc547c55784122b884912126a2a9ba6710851fafc6d76571e8151c83e1b88"},
+  "ash": {:hex, :ash, "2.4.24", "fb74aaf9ee8d9c8397c1c57d2d2ebf48cc0b3a933736eab66b25cea72826ac9f", [:mix], [{:comparable, "~> 1.0", [hex: :comparable, repo: "hexpm", optional: false]}, {:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:earmark, "~> 1.4", [hex: :earmark, repo: "hexpm", optional: true]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8.0", [hex: :ets, repo: "hexpm", optional: false]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: false]}, {:plug, ">= 0.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:spark, "~> 0.2.18", [hex: :spark, repo: "hexpm", optional: false]}, {:stream_data, "~> 0.5.0", [hex: :stream_data, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.1", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "e1b7ac0cf41af75f954bfb9500d1aeb54e4b3b34ec41dbe71c685b92ef8304ae"},
   "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm", "7af5c7e09fe1d40f76c8e4f9dd2be7cebd83909f31fee7cd0e9eadc567da8353"},
   "certifi": {:hex, :certifi, "2.9.0", "6f2a475689dd47f19fb74334859d460a2dc4e3252a3324bd2111b8f0429e7e21", [:rebar3], [], "hexpm", "266da46bdb06d6c6d35fde799bcb28d36d985d424ad7c08b5bb48f5b5cdd4641"},
   "comparable": {:hex, :comparable, "1.0.0", "bb669e91cedd14ae9937053e5bcbc3c52bb2f22422611f43b6e38367d94a495f", [:mix], [{:typable, "~> 0.1", [hex: :typable, repo: "hexpm", optional: false]}], "hexpm", "277c11eeb1cd726e7cd41c6c199e7e52fa16ee6830b45ad4cdc62e51f62eb60c"},

test/support/string_agg.ex

@@ -1,4 +1,5 @@
 defmodule AshPostgres.Test.StringAgg do
+  @moduledoc false
   use Ash.Resource.Aggregate.CustomAggregate
   use AshPostgres.CustomAggregate