Very nice ! just sent you a couple of PR for your new project.
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.
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.
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.
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.
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.
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
Let me get back to you in a day or two.
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).
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?
Create a function which does your thing and use it.
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.
@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(®ister_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?