Merge pull request #34 from danschultzer/refactor-tests-and-config

Refactor config and tests

config/test.exs

@@ -9,7 +9,7 @@ config :pow_assent, PowAssent.Test.Phoenix.Endpoint,
   secret_key_base: String.duplicate("abcdefghijklmnopqrstuvxyz0123456789", 2),
   render_errors: [view: PowAssent.Test.Phoenix.ErrorView, accepts: ~w(html json)]
 
-config :pow_assent, PowAssent.Test.Phoenix.MailerEndpoint,
+config :pow_assent, PowAssent.Test.Phoenix.EndpointConfirmEmail,
   secret_key_base: String.duplicate("abcdefghijklmnopqrstuvxyz0123456789", 2),
   render_errors: [view: PowAssent.Test.Phoenix.ErrorView, accepts: ~w(html json)]
 

lib/mix/tasks/pow_assent.install.ex

@@ -8,24 +8,25 @@ defmodule Mix.Tasks.PowAssent.Install do
   """
   use Mix.Task
 
-  alias Mix.Pow
+  alias Mix.Project
   alias Mix.Tasks.PowAssent.Ecto.Install
 
-  @switches []
-  @default_opts []
-
   @doc false
   def run(args) do
-    Pow.no_umbrella!("pow_assent.install")
+    no_umbrella!()
 
-    args
-    |> Pow.parse_options(@switches, @default_opts)
-    |> run_ecto_install(args)
+    run_ecto_install(args)
   end
 
-  defp run_ecto_install(config, args) do
+  defp run_ecto_install(args) do
     Install.run(args)
+  end
+
+  defp no_umbrella! do
+    if Project.umbrella?() do
+      Mix.raise("mix pow_assent.install can't be used in umbrella apps. Run mix pow_assent.ecto.install in your ecto app directory.")
+    end
 
-    config
+    :ok
   end
 end

lib/pow_assent.ex

@@ -42,6 +42,22 @@ defmodule PowAssent do
         error: :invalid_server_response
       }
     end
+
+    @spec unreachable(atom(), binary(), term()) :: %__MODULE__{}
+    def unreachable(adapter, url, error) do
+      %__MODULE__{
+        message:
+          """
+          Server was unreachable with #{inspect adapter}.
+
+          Failed with the following error:
+          #{inspect error}
+
+          URL: #{url}
+          """,
+        error: :unreachable
+      }
+    end
   end
 
   defmodule ConfigurationError do

lib/pow_assent/ecto/schema.ex

@@ -84,22 +84,15 @@ defmodule PowAssent.Ecto.Schema do
   end
 
   defp maybe_set_confirmed_at(changeset) do
-    case Map.has_key?(changeset.data, :email) do
-      true  -> set_confirmed_at(changeset)
+    case confirmable?(changeset) do
+      true  -> PowEmailConfirmation.Ecto.Schema.confirm_email_changeset(changeset)
       false -> changeset
     end
   end
 
-  defp set_confirmed_at(changeset) do
-    confirmable?  = Map.has_key?(changeset.data, :email_confirmed_at)
-
-    case confirmable? do
-      true  ->
-        now = Pow.Ecto.Schema.__timestamp_for__(changeset.data.__struct__, :email_confirmed_at)
-        Changeset.change(changeset, email_confirmed_at: now)
-
-      false ->
-        changeset
-    end
+  defp confirmable?(changeset) do
+    Map.has_key?(changeset.data, :email) and
+    Map.has_key?(changeset.data, :email_confirmed_at) and
+    !Map.get(changeset.data, :unconfirmed_email)
   end
 end

lib/pow_assent/ecto/user_identities/context.ex

@@ -18,7 +18,8 @@ defmodule PowAssent.Ecto.UserIdentities.Context do
         end
       end
 
-  Remember to update configuration with `users_context: MyApp.Users`.
+  Remember to update configuration with
+  `user_identities_context: MyApp.UserIdentities`.
 
   The following Pow methods can be accessed:
 
@@ -35,7 +36,8 @@ defmodule PowAssent.Ecto.UserIdentities.Context do
   """
   alias Ecto.Changeset
   alias PowAssent.Config
-  require Ecto.Query
+  # import Ecto.Query, only: [where: 3, join: 3, select: 3]
+  import Ecto.Query
 
   @type user :: map()
   @type user_identity :: map()
@@ -47,7 +49,7 @@ defmodule PowAssent.Ecto.UserIdentities.Context do
               | {:error, Changeset.t()}
   @callback create_user(binary(), binary(), map(), map()) ::
               {:ok, map()}
-              | {:error, {:bound_to_different_user | :missing_user_id_field, Changeset.t()}}
+              | {:error, {:bound_to_different_user | :invalid_user_id_field, Changeset.t()}}
               | {:error, Changeset.t()}
   @callback delete(user(), binary()) ::
               {:ok, {number(), nil}} | {:error, {:no_password, Changeset.t()}}
@@ -99,16 +101,12 @@ defmodule PowAssent.Ecto.UserIdentities.Context do
   """
   @spec get_user_by_provider_uid(binary(), binary(), Config.t()) :: user() | nil
   def get_user_by_provider_uid(provider, uid, config) do
-    repo   = repo(config)
-    struct = user_identity_struct(config)
-
-    struct
-    |> repo.get_by(provider: provider, uid: uid)
-    |> repo.preload(:user)
-    |> case do
-      nil      -> nil
-      identity -> identity.user
-    end
+    config
+    |> user_identity_schema_mod()
+    |> where([i], i.provider == ^provider and i.uid == ^uid)
+    |> join(:left, [i], i in assoc(i, :user))
+    |> select([_, u], u)
+    |> repo(config).one()
   end
 
   @doc """
@@ -118,12 +116,11 @@ defmodule PowAssent.Ecto.UserIdentities.Context do
   """
   @spec create(user(), binary(), binary(), Config.t()) :: {:ok, user_identity()} | {:error, {:bound_to_different_user, map()}} | {:error, Changeset.t()}
   def create(user, provider, uid, config) do
-    repo          = repo(config)
     user_identity = Ecto.build_assoc(user, :user_identities)
 
     user_identity
     |> user_identity.__struct__.changeset(%{provider: provider, uid: uid})
-    |> repo.insert()
+    |> repo(config).insert()
     |> case do
       {:error, %{errors: [uid_provider: _]} = changeset} ->
         {:error, {:bound_to_different_user, changeset}}
@@ -138,23 +135,22 @@ defmodule PowAssent.Ecto.UserIdentities.Context do
 
   User schema module and repo module will be fetched from config.
   """
-  @spec create_user(binary(), binary(), map(), map(), Config.t()) :: {:ok, map()} | {:error, {:bound_to_different_user | :missing_user_id_field, Changeset.t()}} | {:error, Changeset.t()}
+  @spec create_user(binary(), binary(), map(), map(), Config.t()) :: {:ok, map()} | {:error, {:bound_to_different_user | :invalid_user_id_field, Changeset.t()}} | {:error, Changeset.t()}
   def create_user(provider, uid, params, user_id_params, config) do
-    repo          = repo(config)
-    user_struct   = user_struct(config)
+    user_mod      = user_schema_mod(config)
     user_identity = %{provider: provider, uid: uid}
-    user          = struct(user_struct)
-    user_id_field = user_struct.pow_user_id_field()
+    user_id_field = user_mod.pow_user_id_field()
 
-    user
-    |> user_struct.user_identity_changeset(user_identity, params, user_id_params)
-    |> repo.insert()
+    user_mod
+    |> struct()
+    |> user_mod.user_identity_changeset(user_identity, params, user_id_params)
+    |> repo(config).insert()
     |> case do
       {:error, %{changes: %{user_identities: [%{errors: [uid_provider: _]}]}} = changeset} ->
         {:error, {:bound_to_different_user, changeset}}
 
       {:error, %{errors: [{^user_id_field, _}]} = changeset} ->
-        {:error, {:missing_user_id_field, changeset}}
+        {:error, {:invalid_user_id_field, changeset}}
 
       {:error, changeset} ->
         {:error, changeset}
@@ -177,19 +173,19 @@ defmodule PowAssent.Ecto.UserIdentities.Context do
 
     user.user_identities
     |> Enum.split_with(&(&1.provider == provider))
-    |> maybe_delete(user, repo, config)
+    |> maybe_delete(user, repo)
   end
 
-  defp maybe_delete({user_identities, rest}, %{password_hash: password_hash}, repo, config) when length(rest) > 0 or not is_nil(password_hash) do
-    user_identity = user_identity_struct(config)
-    results       =
-      user_identity
-      |> Ecto.Query.where([u], u.id in ^Enum.map(user_identities, &(&1.id)))
+  defp maybe_delete({user_identities, rest}, %{password_hash: password_hash} = user, repo) when length(rest) > 0 or not is_nil(password_hash) do
+    results =
+      user
+      |> Ecto.assoc(:user_identities)
+      |> where([i], i.id in ^Enum.map(user_identities, &(&1.id)))
       |> repo.delete_all()
 
     {:ok, results}
   end
-  defp maybe_delete(_any, user, _repo, _config) do
+  defp maybe_delete(_any, user, _repo) do
     changeset =
       user
       |> Changeset.change()
@@ -205,24 +201,27 @@ defmodule PowAssent.Ecto.UserIdentities.Context do
   """
   @spec all(user(), Config.t()) :: [map()]
   def all(user, config) do
-    repo = repo(config)
-
     user
     |> Ecto.assoc(:user_identities)
-    |> repo.all()
+    |> repo(config).all()
   end
 
-  defp user_identity_struct(config) do
-    association = user_struct(config).__schema__(:association, :user_identities) || raise_no_user_identity_error()
+  defp user_identity_schema_mod(config) when is_list(config) do
+    config
+    |> user_schema_mod()
+    |> user_identity_schema_mod()
+  end
+  defp user_identity_schema_mod(user_mod) when is_atom(user_mod) do
+    association = user_mod.__schema__(:association, :user_identities) || raise_no_user_identity_error()
 
     association.queryable
   end
 
   @spec raise_no_user_identity_error :: no_return
   defp raise_no_user_identity_error do
-    Config.raise_error("The `:user` configuration option doesnt' have a `:user_identities` association.")
+    Config.raise_error("The `:user` configuration option doesn't have a `:user_identities` association.")
   end
 
-  def user_struct(config), do: Pow.Ecto.Context.user_schema_mod(config)
-  def repo(config), do: Pow.Ecto.Context.repo(config)
+  defdelegate user_schema_mod(config), to: Pow.Ecto.Context
+  defdelegate repo(config), to: Pow.Ecto.Context
 end

lib/pow_assent/ecto/user_identities/schema.ex

@@ -40,21 +40,16 @@ defmodule PowAssent.Ecto.UserIdentities.Schema do
 
     quote do
       @behaviour unquote(__MODULE__)
-      import unquote(__MODULE__), only: [pow_assent_user_identity_fields: 0]
-
       @pow_user_mod unquote(user_mod)
       @pow_assent_config unquote(config)
 
       @spec changeset(Ecto.Schema.t() | Changeset.t(), map()) :: Changeset.t()
-      def changeset(user_identity_or_changeset, attrs),
-        do: pow_assent_changeset(user_identity_or_changeset, attrs)
-
-      @spec pow_assent_changeset(Ecto.Schema.t() | Changeset.t(), map()) :: Changeset.t()
-      def pow_assent_changeset(user_identity_or_changeset, attrs) do
-        unquote(__MODULE__).changeset(user_identity_or_changeset, attrs, unquote(config))
-      end
+      def changeset(user_identity_or_changeset, attrs), do: pow_assent_changeset(user_identity_or_changeset, attrs)
 
       defoverridable unquote(__MODULE__)
+
+      unquote(__MODULE__).__pow_assent_methods__()
+      unquote(__MODULE__).__register_fields__()
     end
   end
 
@@ -66,12 +61,31 @@ defmodule PowAssent.Ecto.UserIdentities.Schema do
     quote do
       belongs_to :user, @pow_user_mod
 
-      for {name, type, opts} <- unquote(__MODULE__).Fields.attrs(@pow_assent_config) do
-        field(name, type, opts)
+      for {name, type, defaults} <- @pow_assent_fields do
+        field(name, type, defaults)
+      end
+    end
+  end
+
+  @doc false
+  defmacro __pow_assent_methods__ do
+    quote do
+      import unquote(__MODULE__), only: [pow_assent_user_identity_fields: 0]
+
+      @spec pow_assent_changeset(Ecto.Schema.t() | Changeset.t(), map()) :: Changeset.t()
+      def pow_assent_changeset(user_identity_or_changeset, attrs) do
+        unquote(__MODULE__).changeset(user_identity_or_changeset, attrs, @pow_assent_config)
       end
     end
   end
 
+  @doc false
+  defmacro __register_fields__ do
+    quote do
+      @pow_assent_fields unquote(__MODULE__).Fields.attrs(@pow_assent_config)
+    end
+  end
+
   @doc """
   Validates a user identity.
   """

lib/pow_assent/ecto/user_identities/schema/fields.ex

@@ -4,6 +4,11 @@ defmodule PowAssent.Ecto.UserIdentities.Schema.Fields do
   """
   alias PowAssent.Config
 
+  @attrs [
+    {:provider, :string, null: false},
+    {:uid, :string, null: false}
+  ]
+
   @doc """
   List of attributes for the ecto schema.
 
@@ -15,11 +20,7 @@ defmodule PowAssent.Ecto.UserIdentities.Schema.Fields do
   def attrs(config) do
     users_table = Config.get(config, :users_table, "users")
 
-    [
-      {:user_id, {:references, users_table}},
-      {:provider, :string, null: false},
-      {:uid, :string, null: false}
-    ]
+    [{:user_id, {:references, users_table}}] ++ @attrs
   end
 
   @doc """

lib/pow_assent/ecto/user_identities/schema/module.ex

@@ -11,9 +11,7 @@ defmodule PowAssent.Ecto.UserIdentities.Schema.Module do
   @template """
   defmodule <%= inspect schema.module %> do
     use Ecto.Schema
-    use PowAssent.Ecto.UserIdentities.Schema,
-      user: <%= inspect schema.user_module %>
-
+    use PowAssent.Ecto.UserIdentities.Schema, user: <%= inspect(schema.user_module) %>
   <%= if schema.binary_id do %>
     @primary_key {:id, :binary_id, autogenerate: true}
     @foreign_key_type :binary_id<% end %>

lib/pow_assent/http_adapter/httpc.ex

@@ -51,8 +51,7 @@ defmodule PowAssent.HTTPAdapter.Httpc do
 
     {:ok, %HTTPResponse{status: status, headers: headers, body: body}}
   end
-  defp format_response({:error, {:failed_connect, _}}), do: {:error, :econnrefused}
-  defp format_response(response), do: response
+  defp format_response({:error, error}), do: {:error, error}
 
   defp parse_httpc_opts(nil, url), do: default_httpc_opts(url)
   defp parse_httpc_opts(opts, _url), do: opts

lib/pow_assent/http_adapter/mint.ex

@@ -58,8 +58,7 @@ defmodule PowAssent.HTTPAdapter.Mint do
 
     {:ok, %HTTPResponse{status: status, headers: headers, body: body}}
   end
-  defp format_response({:error, {:tls_alert, _tls_error}}), do: {:error, :econnrefused}
-  defp format_response(response), do: response
+  defp format_response({:error, response}), do: {:error, response}
 
   defp merge_body([{:data, _request, new_body} | rest], body), do: merge_body(rest, body <> new_body)
   defp merge_body(_rest, body), do: body

lib/pow_assent/operations.ex

@@ -42,7 +42,7 @@ defmodule PowAssent.Operations do
   This calls `Pow.Ecto.UserIdentities.Context.create_user/5` or
   `create_user/4` on a custom context module.
   """
-  @spec create_user(binary(), binary(), map(), map(), Config.t()) :: {:ok, map()} | {:error, {:bound_to_different_user | :missing_user_id_field, map()}} | {:error, map()} | no_return
+  @spec create_user(binary(), binary(), map(), map(), Config.t()) :: {:ok, map()} | {:error, {:bound_to_different_user | :invalid_user_id_field, map()}} | {:error, map()} | no_return
   def create_user(provider, uid, params, user_id_params, config) do
     case context_module(config) do
       Context -> Context.create_user(provider, uid, params, user_id_params, config)

lib/pow_assent/phoenix/controllers/authorization_controller.ex

@@ -64,7 +64,7 @@ defmodule PowAssent.Phoenix.AuthorizationController do
     |> put_flash(:error, messages(conn).account_already_bound_to_other_user(conn))
     |> redirect(to: routes(conn).registration_path(conn, :new))
   end
-  def respond_callback({:error, {:missing_user_id_field, _changeset}, conn}) do
+  def respond_callback({:error, {:invalid_user_id_field, _changeset}, conn}) do
     conn
     |> put_session("pow_assent_params", conn.private[:pow_assent_params])
     |> redirect(to: routes(conn).path_for(conn, RegistrationController, :add_user_id, [conn.params["provider"]]))
@@ -113,6 +113,5 @@ defmodule PowAssent.Phoenix.AuthorizationController do
     assign(conn, :callback_url, url)
   end
 
-  defp handle_strategy_error(:econnrefused), do: raise "Connection refused"
   defp handle_strategy_error(error), do: raise error
 end