Issue refactoring plugs example from Programming Phoenix

Phoenix Forum Questions / Help
,
1

I am currently in the “Writing an Authentication Plug” section of chapter 5 and I stumbled upon something that I did not fully understand (might be more Elixir related to a certain extent…). Anyway the controller level plug implementation given is the following:

defp authenticate(conn, _opts) do
  if conn.assigns.current_user do
    conn
  else
    conn
    |> put_flash(:error, "You must be logged in to access that page")
    |> redirect(to: page_path(conn, :index))
    |> halt()
  end
end

Now I thought to myself that if/else or cond can (should, depending on who you talk to…) be changed to leverage pattern matching. So I did a quick refactor as follow:

defp authenticate(%Plug.Conn{assigns: %{current_user: nil}} = conn, _opts) do
  conn
  |> put_flash(:error, "You must be logged in to access that page")
  |> redirect(to: page_path(conn, :index))
  |> halt()
end

defp authenticate(conn, _opts) do
  conn
end

And that seems to be working but I thought it was a bit heavy on the eye with the nested pattern matching inside the function declaration so I thought I would refactor it again to use guard clause instead:

defp authenticate(conn, _opts) when is_nil(conn.assigns.current_user) do
  conn
  |> put_flash(:error, "You must be logged in to access that page")
  |> redirect(to: page_path(conn, :index))
  |> halt()
end

defp authenticate(conn, _opts) do
  conn
end

Unfortunately, with this approach I keep receiving the following compilation error:

== Compilation error on file web/controllers/user_controller.ex ==
** (CompileError) web/controllers/user_controller.ex:34: cannot invoke remote function conn.assigns().current_user/0 inside guard
    (stdlib) lists.erl:1354: :lists.mapfoldl/3
    web/controllers/user_controller.ex:34: (module)
    (stdlib) erl_eval.erl:670: :erl_eval.do_apply/6

Now I did check the Plug.Conn implementation and it clearly states that assigns is of type %{atom => any}, so I guess I am missing something here.

Would anyone be willing to help me understand what is happening here?

Thanks,

Fred

1 Like
2

It’s relatively simple, you can’t use maps that way in guard clauses. It’s simply a limitation of how guards work.

3

Thanks for the quick reply, I mean there might be more complex reasons under the hood for not supporting this but I am wondering if this should be reported as a bug or proposed as a new feature as I believe it makes the code above much more readable, elegant and explicit, due to the test being clearly defined in guard clause, than the deeply nested pattern matching embedded within the argument list.

I mean the Elixir guide does not mention this limitation in the guard/clause section and, in that same guide, mentions this access pattern as an “interesting” property of maps.

Thoughts?

ping @josevalim

4

My thought:

What is happening is that the match will decompose the data so you can pull it apart, whereas guards instead perform logic. The proper way to do the example would indeed by the first example, you could do the second by assigning it to something such as:

defp authenticate(%Plug.Conn{assigns: %{current_user: user}} = conn, _opts) when is_nil(user) do

Although that would be needlessly verbose.

Technically conn.assigns.current_user is performing non O(1) function calls (a map looking is O(log(n)) as I recall, and guards not only require that their calls are O(1) but are white-listed as well. Matches are a bit more freeform in how they can match.

5

The issue isn’t with the O of the functionality. For example length is a valid guard clause which is O(n). Rather it’s just a matter of what guard functions are provided to us by BEAM. Why isn’t there a guard clause for getting a value out of a map? I’m not 100% sure, but to be clear the changes required to support this functionality would have to come from the BEAM itself, it isn’t something elixir can just add.

Stepping back a bit, I actually disagree that this is an improvement over the pattern match.

1 Like
6

It is simply not supported by the VM. I have been lobbying so we get such a feature though.

1 Like
7

Understood, I did not know it was dependent on the underlying Erlang VM.

Thanks for taking the time to clarify this.

Cheers!