Merge pull request #376 from danschultzer/ecto-after-compile

Add after compile validation check to ensure all fields and assocs have been defined

Author: danschultzer

CHANGELOG.md

@@ -13,6 +13,7 @@
 * [`Pow.Ecto.Schema.Changeset`] `Pow.Ecto.Schema.Changeset.confirm_password_changeset/3` now uses the `Ecto.Changeset.validate_confirmation/3` for validation and expects `:password_confirmation` instead of `:confirm_password` in params
 * [`Pow.Ecto.Schema.Changeset`] `Pow.Ecto.Schema.Changeset.new_password_changeset/3` now only requires the `:password_hash` if there have been no previous errors set in the changeset
 * [`Pow.Ecto.Schema`] No longer adds `:confirm_password` virtual field
+* [`Pow.Ecto.Schema`] Now has an `@after_compile` callback that ensures all required fields has been defined
 * [`PowInvitation.Phoenix.InvitationView`] Now renders `:password_confirmation` field instead of `:confirm_password`
 * [`PowResetPassword.Phoenix.ResetPasswordView`] Now renders `:password_confirmation` field instead of `:confirm_password`
 * [`Pow.Phoenix.RegistrationView`] Now renders `:password_confirmation` field instead of `:confirm_password`

lib/pow/ecto/schema.ex

@@ -94,6 +94,25 @@ defmodule Pow.Ecto.Schema do
         end
       end
 
+  An `@after_compile` callback will raise an error if there are missing fields
+  or associations, so you can forego the `pow_user_fields/0` call, and write
+  out the whole schema instead:
+
+      defmodule MyApp.Users.User do
+        use Ecto.Schema
+        use Pow.Ecto.Schema, user_id_field: :email
+
+        schema "users" do
+          field :email,            :string, null: false
+          field :password_hash,    :string
+          field :current_password, :string, virtual: true
+          field :password,         :string, virtual: true
+          field :confirm_password, :string, virtual: true
+
+          timestamps()
+        end
+      end
+
   ## Customize Pow changeset
 
   You can extract individual changeset methods to modify the changeset flow
@@ -124,6 +143,11 @@ defmodule Pow.Ecto.Schema do
   alias Ecto.Changeset
   alias Pow.Config
 
+  defmodule SchemaError do
+    @moduledoc false
+    defexception [:message]
+  end
+
   @callback changeset(Ecto.Schema.t() | Changeset.t(), map()) :: Changeset.t()
   @callback verify_password(Ecto.Schema.t(), binary()) :: boolean()
 
@@ -145,6 +169,7 @@ defmodule Pow.Ecto.Schema do
       unquote(__MODULE__).__register_fields__()
       unquote(__MODULE__).__register_assocs__()
       unquote(__MODULE__).__register_user_id_field__()
+      unquote(__MODULE__).__register_after_compile_validation__()
     end
   end
 
@@ -294,6 +319,87 @@ defmodule Pow.Ecto.Schema do
   def user_id_field(config) when is_list(config), do: Config.get(config, :user_id_field, @default_user_id_field)
   def user_id_field(_any), do: @default_user_id_field
 
+  @doc false
+  defmacro __register_after_compile_validation__ do
+    quote do
+      def pow_validate_after_compilation!(env, _bytecode) do
+        unquote(__MODULE__).__require_assocs__(__MODULE__)
+        unquote(__MODULE__).__require_fields__(__MODULE__)
+      end
+
+      @after_compile {__MODULE__, :pow_validate_after_compilation!}
+    end
+  end
+
+  @doc false
+  def __require_assocs__(module) do
+    ecto_assocs = Module.get_attribute(module, :ecto_assocs)
+
+    module
+    |> Module.get_attribute(:pow_assocs)
+    |> Enum.reverse()
+    |> Enum.filter(fn assoc ->
+      not Enum.any?(ecto_assocs, &assocs_match?(elem(assoc, 0), elem(assoc, 1), &1))
+    end)
+    |> Enum.map(fn
+      {type, name, queryable}           -> "#{type} #{inspect name}, #{inspect queryable}"
+      {type, name, queryable, defaults} -> "#{type} #{inspect name}, #{inspect queryable}, #{inspect defaults}"
+    end)
+    |> case do
+      []         -> :ok
+      assoc_defs -> raise_missing_assocs_error(module, assoc_defs)
+    end
+  end
+
+  defp raise_missing_assocs_error(module, assoc_defs) do
+    raise SchemaError, message:
+      """
+      Please define the following association(s) in the schema for #{inspect module}:
+
+      #{Enum.join(assoc_defs, "\n")}
+      """
+  end
+
+  @doc false
+  def __require_fields__(module) do
+    ecto_fields      = Module.get_attribute(module, :ecto_fields)
+    changeset_fields = Module.get_attribute(module, :changeset_fields)
+
+    module
+    |> Module.get_attribute(:pow_fields)
+    |> Enum.reverse()
+    |> Enum.filter(&missing_field?(&1, ecto_fields, changeset_fields))
+    |> Enum.map(fn
+      {name, type}           -> "field #{inspect name}, #{inspect type}"
+      {name, type, defaults} -> "field #{inspect name}, #{inspect type}, #{inspect defaults}"
+    end)
+    |> case do
+      []         -> :ok
+      field_defs -> raise_missing_fields_error(module, field_defs)
+    end
+  end
+
+  defp missing_field?({name, type, defaults}, ecto_fields, changeset_fields) do
+    case defaults[:virtual] do
+      true -> missing_field?(name, type, changeset_fields)
+      _any -> missing_field?(name, type, ecto_fields)
+    end
+  end
+  defp missing_field?({name, type}, ecto_fields, _changeset_fields),
+    do: missing_field?(name, type, ecto_fields)
+  defp missing_field?(name, type, existing_fields) do
+    not Enum.member?(existing_fields, {name, type})
+  end
+
+  defp raise_missing_fields_error(module, field_defs) do
+    raise SchemaError, message:
+      """
+      Please define the following field(s) in the schema for #{inspect module}:
+
+      #{Enum.join(field_defs, "\n")}
+      """
+  end
+
   @doc """
   Normalizes the user id field.
 

lib/pow/extension/ecto/schema.ex

@@ -99,11 +99,11 @@ defmodule Pow.Extension.Ecto.Schema do
   @doc false
   defmacro __register_after_compile_validation__ do
     quote do
-      def validate_after_compilation!(env, _bytecode) do
+      def pow_extension_validate_after_compilation!(env, _bytecode) do
         unquote(__MODULE__).validate!(@pow_extension_config, __MODULE__)
       end
 
-      @after_compile {__MODULE__, :validate_after_compilation!}
+      @after_compile {__MODULE__, :pow_extension_validate_after_compilation!}
     end
   end
 

test/pow/ecto/schema_test.exs

@@ -68,4 +68,57 @@ defmodule Pow.Ecto.SchemaTest do
     assert %{on_replace: :mark_as_invalid} = OverrideAssocUser.__schema__(:association, :parent)
     assert %{on_delete: :delete_all} = OverrideAssocUser.__schema__(:association, :children)
   end
+
+  module_raised_with =
+    try do
+      defmodule MissingAssocsUser do
+        use Ecto.Schema
+        use Pow.Ecto.Schema
+
+        @pow_assocs {:belongs_to, :invited_by, __MODULE__, foreign_key: :user_id}
+        @pow_assocs {:has_many, :invited, __MODULE__}
+
+        schema "users" do
+          timestamps()
+        end
+      end
+    rescue
+      e in Pow.Ecto.Schema.SchemaError -> e.message
+    end
+
+  test "requires assocs defined" do
+    assert unquote(module_raised_with) ==
+      """
+      Please define the following association(s) in the schema for Pow.Ecto.SchemaTest.MissingAssocsUser:
+
+      belongs_to :invited_by, Pow.Ecto.SchemaTest.MissingAssocsUser, [foreign_key: :user_id]
+      has_many :invited, Pow.Ecto.SchemaTest.MissingAssocsUser
+      """
+  end
+
+  module_raised_with =
+    try do
+      defmodule MissingFieldsUser do
+        use Ecto.Schema
+        use Pow.Ecto.Schema
+
+        schema "users" do
+          timestamps()
+        end
+      end
+    rescue
+      e in Pow.Ecto.Schema.SchemaError -> e.message
+    end
+
+  test "requires fields defined" do
+    assert unquote(module_raised_with) ==
+      """
+      Please define the following field(s) in the schema for Pow.Ecto.SchemaTest.MissingFieldsUser:
+
+      field :email, :string, [null: false]
+      field :password_hash, :string
+      field :current_password, :string, [virtual: true]
+      field :password, :string, [virtual: true]
+      """
+  end
 end