Phoenix: Add ability to conditionally plug based on action name

[chrismccord]

[chrismccord]

Alright folks, feedback please: I'm spitballing some ideas on conditionally plugging plugs based on the current controller action. We have a couple options. First, we can use a "Pure Plug" approach. Alternatively we can introduce a new macro or override the plug macro. I'd like to explore the pure plug approach before going the macro route.

Below is a braindump of ways to conditionally scope plugs while staying with the "just a plug" approach
(ie no new macro or plug macro override):

@jeregrine @nurugger07 @scrogson @josevalim

# Plugs without scoped actions
defmodule MyApp.SomeController do
  use Phoenix.Controller

  plug AuthenticateAdmin
  plug :assign_layout, "print"
  plug :action
  plug :render

  def index(conn, _params) do
    conn
  end
end

# Idea 1 - Only / Except Plugs
defmodule MyApp.SomeController do
  use Phoenix.Controller

  plug AuthenticateAdmin
  plug Only,   {:assign_layout, "print", [:index, :show]}
  plug :action
  plug Except, {:render, [:update]}

  def index(conn, _params) do
    conn
  end
end

# Idea 2 - Actions Plug
defmodule MyApp.SomeController do
  use Phoenix.Controller

  plug AuthenticateAdmin
  plug Actions, {:assign_layout, "print", only: [:index, :show]}
  plug :action
  plug Actions, {:render, except: [:update]}

  def index(conn, _params) do
    conn
  end
end

[josevalim]

The biggest problem with the pure plug approach (not going into how it
looks like/read) is that you cannot use it for private functions, and that
is a deal breaker to me, because otherwise they can be used.

_José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Lead Developer_

[chrismccord]

The biggest problem with the pure plug approach (not going into how it
looks like/read) is that you cannot use it for private functions, and that
is a deal breaker to me, because otherwise they can be used.

Ah right. You mentioned this issue before, but I had forgotten about it. Not being able to use private functions is already a caveat of "function plugs", so perhaps this is ok? I'm not opposed to a macro on top, but since we're using Plugs internally, it would be generating public functions to call the private ones, which defeats the purpose of priv functions, right?

[chrismccord]

I should mention my goals right now are to keep the plug chain execution explicit (i.e., explicit top-bottom flow of execution that plugs gives us). So if we ship a new macro(s), I would still like it to preserve the execution flow of the conn.

[josevalim]

Function plugs should work with private functions just fine, if not, it is
a bug.

_José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Lead Developer_

[chrismccord]

Function plugs should work with private functions just fine, if not, it is
a bug.

Oh, right. I see the issue. Outside plugs wouldn't be able to invoke. @josevalim What if we imported the plugs from Phoenix.Controller? Like any good metaprogrammer, I'm trying to exhaust other options before code generation. This doesn't read quite as nicely as a before_action or similar:

# Idea 1 - Only / Except Plugs
defmodule MyApp.SomeController do
  use Phoenix.Controller

  plug AuthenticateAdmin
  plug :only,   {:assign_layout, "print", [:index, :show]}
  plug :action
  plug :except, {:render, [:update]}

  def index(conn, _params) do
    conn
  end
end

Any thoughts on something that's readable _and_ maintains explicit conn flow? I'm wracking my brain, but I haven't yet produced anything I'm happy with, api wise.

[chrismccord]

I take that back, the previous solution would require a touch of code generation, i.e.:
(We would also need to accommodate Module plugs)

quote do
  def only(conn, {plug, opts, actions}) do
    if action_name(conn) in actions do
      apply(__MODULE__, plug, conn, opts)
    else
      conn
    end
  end

  def except(conn, {plug, opts, actions}) do
    if not(action_name(conn) in actions) do
      apply(__MODULE__, plug, conn, opts)
    else
      conn
    end
  end
end

Is this preferable to a macro layer on top of Plug, for the Controllers?

[chrismccord]

I spiked the "pure plug" idea out in code to get a better idea. This works for private functions:
https://github.com/phoenixframework/phoenix/commit/3f9d79877c95547842723c4823d64faf6ff466c8

[chrismccord]

Ok, idea 3, and one I actually like! I wasn't thrilled with the only/except plugs clarity and how it read. Here's a twist on the naming with a single :scoped plug. I think it works really well. Thoughts?

defmodule MyController do
  use Phoenix.Controller

  plug :assign_layout, "print"
  plug :scoped, {:authenticate, only: [:create, :update]}
  plug :action
  plug :scoped, {:render, except: [:edit]}

  ...
end

https://github.com/phoenixframework/phoenix/blob/cm/conditional-plugs/lib/phoenix/plugs/builder.ex

[josevalim]

Nice! The current implementation still has the downside private functions can't be called, right?

[chrismccord]

The private functions can be called just fine since we inject the scoped plug as a local function :)

[josevalim]

I am looking at the code and it doesn't look like it would work. And the test is not really using a public function, while it should:

https://github.com/phoenixframework/phoenix/compare/cm/conditional-plugs#diff-007da05ef4617f61ab734c81a8cd2476R35

:)

[chrismccord]

@josevalim This next revision works, I promise :)

What do you think?
https://github.com/phoenixframework/phoenix/blob/cm/conditional-plugs/lib/phoenix/plugs/builder.ex

[josevalim]

Yeah, the only I could think it would work would be through macros like that. As we discussed, whatever we decide here affects the router and which kind of macros we want to provide there too. So it is hard to decide until we put all the pieces together.

[chrismccord]

After some guidance from @josevalim and Bruce Tate at the conf, we'll be going the macro layer on top of plug for the controllers :) We'll still abide by the Plug contract, so plugs should "just work" from non-phoenix middlewares when slotted into the phoenix stack. I'll put some ideas here soon. Thanks for the feedback!

[johnsinco]

Did this ever get integrated into Phoenix? We were looking for something along these lines to handle situations like

plug Authenticate, only: :create

[Gazler]

@johnsinco You can do this in a controller:

plug Authenticate when action in [:create]

[taylorbrooks]

@Gazler Where is the documentation for using when action in [:create]?

I'd love to read more around this.

[Gazler]

@taylorbrooks http://hexdocs.pm/phoenix/Phoenix.Controller.Pipeline.html#summary

[chintan-mishra]

The above link is not working. Use the one below

https://hexdocs.pm/phoenix/Phoenix.Controller.html#module-guards