Nicest way to emulate function decorators?

Fixed! :stuck_out_tongue:

2 Likes

Very nice ! just sent you a couple of PR for your new project. :slight_smile:

Thanks!

I will second @benwilson512 here - Iā€™m not sure decorator pattern really fits in Elixir.

As to instrumentation - we have the awesome tracing abilities that allow us to introspect function calls without adding any line of code - even in production.

There are native things in :dbg and :trace, but I prefer :recon and recently started using https://github.com/tatsuya6502/recon_ex

# Prints next 3 calls to SomeModule.function with return value
ReconTrace.calls({SomeModule, :function, fn _ -> :return end}, 3)
# Prints next 3 calls to SomeModule.function with return value in process pid
ReconTrace.calls({SomeModule, :function, fn _ -> :return end}, 3, pid: pid)

Itā€™s much more powerful, than this. Those should be good examples of things I use the most.

2 Likes

Just discovered your decorator library as I was searching for exactly this behaviour. Wrapping method calls with timing traces. Perfect solution for me; thanks for making it available.

1 Like

This may be too late but you really really really shouldnā€™t copy Elixirā€™s private source. When you rely on Elixir internal details and then your code breaks when users of your code update their Elixir version, it gives a sense of instability to the ecosystem while we are working very hard to keep Elixir backwards compatible.

One suggestion is, when you are traversing the AST, you should see if thatā€™s a decorated attribute and, if not, you should regenerate it as Kernel.@(ast) or something similar.

However, you should not need to traverse the AST to implement decorators. It should be possible to implement this by using @on_definition. Since @on_definition is going to be invoked before every function clause, you can collect information such as function body, arguments, etc and then use @before_compile to traverse all of the collected functions, calling the decorator, and emitting new clauses. You can also use defoverridable and tell decorators to simply invoke super when they need to call the parent function.

Here is an example implementation:

defmodule Decorator do
  defmacro __using__(_) do
    quote do
      @on_definition Decorator
      @before_compile Decorator
      @decorators %{}
      @decorator nil
    end
  end

  def __on_definition__(env, kind, fun, args, _guards, _body) do
    if decorator = Module.get_attribute(env.module, :decorator) do
      decorators = Module.get_attribute(env.module, :decorators)
      decorators = Map.put(decorators, {kind, fun, length(args)}, decorator)
      Module.put_attribute(env.module, :decorators, decorators)
      Module.put_attribute(env.module, :decorator, nil)
    end
    :ok
  end

  defmacro __before_compile__(env) do
    decorators = Module.get_attribute(env.module, :decorators)
    for {{kind, fun, arity}, decorator} <- decorators do
      args = generate_args(arity)
      body = decorator.decorate(kind, fun, args)
      quote do
        defoverridable [{unquote(fun), unquote(arity)}]

        Kernel.unquote(kind)(unquote(fun)(unquote_splicing(args))) do
          unquote(body)
        end
      end
    end
  end

  defp generate_args(0), do: []
  defp generate_args(n), do: for(i <- 1..n, do: Macro.var(:"var#{i}", __MODULE__))
end

defmodule InspectDecorator do
  def decorate(_kind, _name, args) do
    quote do
      IO.inspect super(unquote_splicing(args))
    end
  end
end

defmodule Sample do
  use Decorator

  @decorator InspectDecorator
  def add(a, b) do
    a + b
  end
end

# Should print 3
Sample.add(1, 2)

I donā€™t quite agree with the idea but I believe the implementation above would be better than relying on Elixir internals. The sketch above is also not complete, for example, we donā€™t support multiple decorators, but you should be able to change it by changing the on_definition implementation.

18 Likes

Thanks JosĆ© for your reply. At the time, overriding Kernel.@ felt like the only solution, but Iā€™ll revisit this based on your example. Your solution looks elegant ā€“ I never knew you could override a function from within the same module.

Iā€™m still wondering how to do multiple decorators without inspecting the moduleā€™s AST or doing something with Kernel.@ though, because the putting 2 @decorate module attributes below each other just causes the last one to overwrite he previous value iirc.

They wont if you define the attribute like Module.register_attribute __MODULE__, :decorate, accumulate: true, then it will be a list (latest attr value would be first), and you can just get this list __on_definition__ and reset it to nil for next functions.

3 Likes

Yeah, didt think of that option, should be doable!

Hereā€™s the usage of the decorator I implemented.

defmodule ProgressDecorator do
  use Decorator.Define, [progress: 0]

  def progress(body, context) do
    quote do
      reply = unquote(body)

      case reply do
        list when is_list(list) -> Progress.incr(unquote(context.name), length(list))
        _ -> Progress.incr(unquote(context.name), 1)
      end

      reply
    end
  end
end

Itā€™s used to track progress of the stages in a Flow pipeline to generate a visualisation.

2 Likes

Iā€™m getting there but thereā€™s one issue with this approach, and thatā€™s when decorating a private function, Elixir raises a warning that defoverridable is deprecated for defp's. Any suggestion on how to address this?

I guess the easiest way is to stop supporting decorators for defp altogether :slight_smile:

Let me get back to you in a day or two.

1 Like

Only supporting decorators for public functions would be an acceptable restriction.

We have removed the warning on the v1.4 branch. On the upcoming v1.4.1, private functions will be once again overridable without warnings.

On master we have cleaned up the implementation and we also allow private macros to be overridable (something that was never supported).

3 Likes

Reviving this one because Iā€™m really excited about @arjanā€™s decorator lib, and yet @josevalim @OvermindDL1 and @benwilson512 all expressing discontent over this pattern.

We want to use decorators to perform validation on controller actions and channel functions; we are using elixir/phoenix strictly as a realtime data api that has a mixed surface of both REST and websockets (no forms, js, templates, assets, etc). We need to validate passed-in params for data integrity, example: an AuthController has register action that passes in name, username, password. We need to make sure all are non-null strings, username is unique, and password is 8 chars or more.

As recommended in Whatā€™s New in Ecto 2.0, a good way to validate incoming data is with Ecto.Schema with embedded_schema. Weā€™re using @vicā€™s excellent vic/params to reduce boilerplate, but there is still the matter of invoking the changeset, checking if it is valid, then returning error responses that are properly structured per our requirements. Why make this call in every action? Isnā€™t it cleaner to have a decorator:

defparams register_params %{ name!: :string, username!: :string, password!: :string }

@validate register_params
def register(conn, payload) do
...

Our first inclination was do something like plug :validate which is invoked before every action, but itā€™s harder (and less explicit) to associate the right schema+changeset with corresponding action.

Like others, the consensus of negativity from the core team tends to preclude others from using this pattern, even though otherwise it seems like a great fit. So I ask, what exactly are you recommending for something like our scenario, if not decorators?

1 Like

Create a function which does your thing and use it.

5 Likes

I actually agree with @NobbZ here, in my opinion the use of function decorators should be limited to things that are not critical to the function that is being decorated.

The reason why I wrote the function decorator lib is for the AppSignal Elixir integration, to easily annotate a function that should be measured. So in my opinion decorators should be used mostly for logging, instrumentation and debugging, and not for performing ā€œbusiness-criticalā€ things like branching etc.

2 Likes

@NobbZ @arjan this is really confusing to me, for a couple of reasons. The first is that many other languages & frameworks handle validation with decorators (Scala with PlayFramework, C# with asp.net mvc, and even node with express to name a few). Why would elixir be so different that a technique like this would be advised against so strongly?

Secondly, the essence of Phoenix middleware with Plug architecture is doing nearly the same thing, itā€™s just a bit more cumbersome. Are you also recommending against the Plug approach too?

I know itā€™s really easy to just say ā€œCreate a function ā€¦ and use itā€, but saying something like that is not really welcoming to new members of the community that are trying to figure out best practices, especially coming from other popular languages.

I just read this article by on medium about elixir validation with ecto and plug: https://medium.com/@feymartynov/validating-controller-params-with-ecto-and-plug-in-phoenix-5fe2cf77a224

The tl;dr of the approach is to use plug to tie an ecto changeset to a single action using the filtering syntax:

plug MyApp.Params.Pagination, :pagination when action in [:index]

Then later on thereā€™s some macro magic to perform the validation logic, detect issues, and set the appropriate api response.

This is very very similar to what we are doing with @decorators, with ours reading a bit more explicitly:

defparams register_params %{ name!: :string, username!: :string, avatar: [field: :string, default: ""]}
@decorate validate(&register_params/1)
def register(conn, params) do
...

The argument in the article is the same that I have:

Params validation and error response logic is abstracted out from controller actions so they donā€™t have redundant code and are more readable.

My questions still remain ā€“ why is the @decorator pattern not recommended for validation? And does the same go for plug ... when action in [...] to achieve similar functionality?

@arjan: How about add also ā€™@decorate_allā€™ and ā€™@decorate_multiā€™?

1 Like