Hello all,
I''ve been working with the action_controller filter code lately and
I''ve
noticed a lot of code repetition and a lot of room for improvement without
breaking existing filter usage. I plan on submitting a patch that would
clean this all up and add a few additional features.
The main goal I am going for with the filter code is enabling "true"
around
filters: A filter method that may run whatever it wants before or after the
action, including running the action within a block. This becomes incredibly
useful when you talk about something like this:
def filter(controller)
if session_user?
Data.with_scope( :find => { :conditions => ["user_id = ?",
controller.session[:user_id] ] } ) do
yield # this runs the action (or subsequent filters) making sure that
all finds for Data require that the use is an owner.
end
With this sort of syntax for filters, you can completely drop the idea of
"before filters" and "after filters." With scoping this
easily lends itself
to the simplified CRUD controllers mentioned by DHH in his keynote speech at
RailsConf by keeping the controllers simple.
I think that filters and their supporting methods should be scoped out of
the controller. Adding a bunch of methods to the controller that
aren''t actions and don''t assist actions isn''t very
clean:
I picture one of thefollowing uses being the most useful.
add_filter :filter => ProjectScope.new, :except => [:list,:index]
class ProjectScope
def filter(controller)
Project.with_scope( ..etc.. )
yield
end
end
end
add_filter :filter => :project_scope, :except => [:list, :index]
def project_scope
yield
# this way drops the clean filter scope in favor of the simplicity of
using controller methods.
end
I would also like to enable an add_filter call, passing a block, but the
only way to allow that method to use yield is to instance_eval (any input on
that would be good)
What I want from this list, if I could, is some opinions on things you wish
filters did. Anything you think would simplify filters, etc.
Something that would be cool is to define the types of filter usage and just
cater to each type of usage. We already have ''verify'' which is
just a
before_filter that checks things like params and request.method. Maybe
we can think of a way to express any other usages that are comon,
providing a simple way to do the most comon filters.
Right now I can think of these cases. Please add any you can think of.
* validate or restrict access based on session
* scope or limit access based on params
* setup variables for usage in the actions or views.
* run methods that are called every time a action is called
* update record keeping objects
Sorry for the super long mail. I appreciate any input.
-Marty
_______________________________________________
Rails-core mailing list
Rails-core@lists.rubyonrails.org
http://lists.rubyonrails.org/mailman/listinfo/rails-core
I''ve definitely had a need for more improved around and after filters. If an action is cached, any after or around filters will not execute as processing is basically halted after the cached page is served. It would be nice to have filters that are guaranteed to run regardless of whatever happens during the execution of the action. Is this something that you would be addressing with your improvements? On 7/2/06, Martin Emde <martin.emde@gmail.com> wrote:> Hello all, > > I''ve been working with the action_controller filter > code lately and I''ve noticed a lot of code repetition and a > lot of room for improvement without breaking existing filter usage. I plan > on submitting a patch that would clean this all up and add a few additional > features. > > The main goal I am going for with the filter code is enabling "true" around > filters: A filter method that may run whatever it wants before or after the > action, including running the action within a block. This becomes incredibly > useful when you talk about something like this: > > def filter(controller) > if session_user? > Data.with_scope( :find => { :conditions => ["user_id = ?", > controller.session[:user_id] ] } ) do > yield # this runs the action (or subsequent filters) making sure that > all finds for Data require that the use is an owner. > end > > With this sort of syntax for filters, you can completely drop the idea of > "before filters" and "after filters." With scoping this easily lends itself > to the simplified CRUD controllers mentioned by DHH in his keynote speech at > RailsConf by keeping the controllers simple. > > I think that filters and their supporting methods should be scoped out of > the controller. Adding a bunch of methods to the > controller that aren''t actions and don''t > assist actions isn''t very clean: > > I picture one of thefollowing uses being the most useful. > > add_filter :filter => ProjectScope.new, :except => [:list,:index] > class ProjectScope > def filter(controller) > Project.with_scope( ..etc.. ) > yield > end > end > end > > add_filter :filter => :project_scope, :except => [:list, :index] > def project_scope > yield > # this way drops the clean filter scope in favor of > the simplicity of using controller methods. > end > > I would also like to enable an add_filter call, passing a block, but the > only way to allow that method to use yield is to instance_eval (any input on > that would be good) > > What I want from this list, if I could, is some opinions on things you wish > filters did. Anything you think would simplify filters, etc. > > Something that would be cool is to define the types of filter usage and just > cater to each type of usage. We already have ''verify'' which is just a > before_filter that checks things like params and request.method. Maybe we > can think of a way to express any other > usages that are comon, providing a simple > way to do the most comon filters. Right now I can think of > these cases. Please add any you can think of. > > * validate or restrict access based on session > * scope or limit access based on params > * setup variables for usage in the actions or views. > * run methods that are called every time a action is called > * update record keeping objects > > Sorry for the super long mail. I appreciate any input. > > -Marty > > _______________________________________________ > Rails-core mailing list > Rails-core@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails-core > > >
The way I want these filters to work is that every filter is a block that yields the next filter or action. That would mean that unless the action raises an exception, it should still go back thru all the filter methods after returning from each yield.. I''ll try to make sure this is going to work, I think it should be doable. On 7/2/06, Andrew Kaspick <akaspick@gmail.com> wrote:> > I''ve definitely had a need for more improved around and after filters. > If an action is cached, any after or around filters will not execute > as processing is basically halted after the cached page is served. > > It would be nice to have filters that are guaranteed to run regardless > of whatever happens during the execution of the action. > > Is this something that you would be addressing with your improvements? > > On 7/2/06, Martin Emde <martin.emde@gmail.com> wrote: > > Hello all, > > > > I''ve been working with the action_controller filter > > code lately and I''ve noticed a lot of code repetition and a > > lot of room for improvement without breaking existing filter usage. I > plan > > on submitting a patch that would clean this all up and add a few > additional > > features. > > > > The main goal I am going for with the filter code is enabling "true" > around > > filters: A filter method that may run whatever it wants before or after > the > > action, including running the action within a block. This becomes > incredibly > > useful when you talk about something like this: > > > > def filter(controller) > > if session_user? > > Data.with_scope( :find => { :conditions => ["user_id = ?", > > controller.session[:user_id] ] } ) do > > yield # this runs the action (or subsequent filters) making sure > that > > all finds for Data require that the use is an owner. > > end > > > > With this sort of syntax for filters, you can completely drop the idea > of > > "before filters" and "after filters." With scoping this easily lends > itself > > to the simplified CRUD controllers mentioned by DHH in his keynote > speech at > > RailsConf by keeping the controllers simple. > > > > I think that filters and their supporting methods should be scoped out > of > > the controller. Adding a bunch of methods to the > > controller that aren''t actions and don''t > > assist actions isn''t very clean: > > > > I picture one of thefollowing uses being the most useful. > > > > add_filter :filter => ProjectScope.new, :except => [:list,:index] > > class ProjectScope > > def filter(controller) > > Project.with_scope( ..etc.. ) > > yield > > end > > end > > end > > > > add_filter :filter => :project_scope, :except => [:list, :index] > > def project_scope > > yield > > # this way drops the clean filter scope in favor of > > the simplicity of using controller methods. > > end > > > > I would also like to enable an add_filter call, passing a block, but the > > only way to allow that method to use yield is to instance_eval (any > input on > > that would be good) > > > > What I want from this list, if I could, is some opinions on things you > wish > > filters did. Anything you think would simplify filters, etc. > > > > Something that would be cool is to define the types of filter usage and > just > > cater to each type of usage. We already have ''verify'' which is just a > > before_filter that checks things like params and request.method. Maybe > we > > can think of a way to express any other > > usages that are comon, providing a simple > > way to do the most comon filters. Right now I can think of > > these cases. Please add any you can think of. > > > > * validate or restrict access based on session > > * scope or limit access based on params > > * setup variables for usage in the actions or views. > > * run methods that are called every time a action is called > > * update record keeping objects > > > > Sorry for the super long mail. I appreciate any input. > > > > -Marty > > > > _______________________________________________ > > Rails-core mailing list > > Rails-core@lists.rubyonrails.org > > http://lists.rubyonrails.org/mailman/listinfo/rails-core > > > > > > > _______________________________________________ > Rails-core mailing list > Rails-core@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails-core >_______________________________________________ Rails-core mailing list Rails-core@lists.rubyonrails.org http://lists.rubyonrails.org/mailman/listinfo/rails-core
On 2-jul-2006, at 22:49, Andrew Kaspick wrote:> I''ve definitely had a need for more improved around and after filters. > If an action is cached, any after or around filters will not execute > as processing is basically halted after the cached page is served.The problem here is that there is no assurance that an after-filter will run. That way scoping sometimes propagates through all requests via the class variables. Basically it seems that it would be a nice idea to have around filters which are wrapped in begin/ensure so that the around filter is always fired for both before and after. I looked at how AP handles that and it might be somewhat cumbersome to implement. -- Julian ''Julik'' Tarkhanov please send all personal mail to me at julik.nl
Well, if you use the new around filter that I''m proposing you
won''t have a
"before action" and "after action" you''ll just have
one method that yields
the perform_action for the controller. This way it''s unavoidable
(without an
exception being raised) for the code not to be run.
For example you would make a filter that acts like a current "around"
filter. (I would call it a "before and after" filter)
def filter(controller)
if stuff_that_would_normally_be_a_before_action(args)
yield # runs the action (or the following filter)
stuff_that_would_normally_be_an_after_action
else
# cleanup if before_action stuff failed
end
end
This way when the action returns, it should run. If this is not the case,
please let me know a test case I could use to make sure this will work in
the new system.
-Martin
On 7/2/06, Julian ''Julik'' Tarkhanov <listbox@julik.nl>
wrote:>
>
> On 2-jul-2006, at 22:49, Andrew Kaspick wrote:
>
> > I''ve definitely had a need for more improved around and after
filters.
> > If an action is cached, any after or around filters will not execute
> > as processing is basically halted after the cached page is served.
>
> The problem here is that there is no assurance that an after-filter
> will run. That way scoping sometimes propagates
> through all requests via the class variables. Basically it seems that
> it would be a nice idea to have around filters which are wrapped
> in begin/ensure so that the around filter is always fired for both
> before and after.
>
> I looked at how AP handles that and it might be somewhat cumbersome
> to implement.
> --
> Julian ''Julik'' Tarkhanov
> please send all personal mail to
> me at julik.nl
>
>
> _______________________________________________
> Rails-core mailing list
> Rails-core@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>
_______________________________________________
Rails-core mailing list
Rails-core@lists.rubyonrails.org
http://lists.rubyonrails.org/mailman/listinfo/rails-core
Martin- Take a look at this plugin that just came out. I think it does what you are looking for as far as wrapping actions and yielding to them. Even allows for a chain of such filters. http://roman2k.free.fr/rails/meantime_filter/0.1.0/rdoc/ -Ezra On Jul 2, 2006, at 4:27 PM, Martin Emde wrote:> Well, if you use the new around filter that I''m proposing you won''t > have a "before action" and "after action" you''ll just have one > method that yields the perform_action for the controller. This way > it''s unavoidable (without an exception being raised) for the code > not to be run. > > For example you would make a filter that acts like a current > "around" filter. (I would call it a "before and after" filter) > > def filter(controller) > if stuff_that_would_normally_be_a_before_action(args) > yield # runs the action (or the following filter) > stuff_that_would_normally_be_an_after_action > else > # cleanup if before_action stuff failed > end > end > > This way when the action returns, it should run. If this is not the > case, please let me know a test case I could use to make sure this > will work in the new system. > > -Martin > > On 7/2/06, Julian ''Julik'' Tarkhanov <listbox@julik.nl> wrote: > > On 2-jul-2006, at 22:49, Andrew Kaspick wrote: > > > I''ve definitely had a need for more improved around and after > filters. > > If an action is cached, any after or around filters will not execute > > as processing is basically halted after the cached page is served. > > The problem here is that there is no assurance that an after-filter > will run. That way scoping sometimes propagates > through all requests via the class variables. Basically it seems that > it would be a nice idea to have around filters which are wrapped > in begin/ensure so that the around filter is always fired for both > before and after. > > I looked at how AP handles that and it might be somewhat cumbersome > to implement. > -- > Julian ''Julik'' Tarkhanov > please send all personal mail to > me at julik.nl > > > _______________________________________________ > Rails-core mailing list > Rails-core@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails-core > > _______________________________________________ > Rails-core mailing list > Rails-core@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails-core_______________________________________________ Rails-core mailing list Rails-core@lists.rubyonrails.org http://lists.rubyonrails.org/mailman/listinfo/rails-core
Great. Good to see people agree with the idea.
I still think there''s a lot of improvement to be made in the filter
code so
I will continue making this patch.
I also wanted to see what people''s preference was for yielding the
action
from a filter. The problem with yield is that yield from a block passed to
meantime_filter (or whatever you want to call it) will call the block passed
to the surrounding method at the time the Proc is created. This means the
filter has to use block.call when it''s passed as a Proc, but can call
yield
when it''s done as a method or an object responding to filter. It would
be
nice to eliminate this inconsitency.
An alternate option to yield would be
controller.filter_chain.next
to call the next filter/action.
Yield may make more sense in some cases, but when you''re forced to do a
proc
like { |controller, block| block.call } sometimes and def
filter(controller); yield; end; other times it may cause confusion.
What seems less confusing to you guys?
-Martin
On 7/2/06, Ezra Zygmuntowicz <ezmobius@gmail.com>
wrote:>
> Martin-
> Take a look at this plugin that just came out. I think it does what you
> are looking for as far as wrapping actions and yielding to them. Even
allows
> for a chain of such filters.
>
> http://roman2k.free.fr/rails/meantime_filter/0.1.0/rdoc/
>
> -Ezra
>
> On Jul 2, 2006, at 4:27 PM, Martin Emde wrote:
>
> Well, if you use the new around filter that I''m proposing you
won''t have a
> "before action" and "after action" you''ll just
have one method that yields
> the perform_action for the controller. This way it''s unavoidable
(without an
> exception being raised) for the code not to be run.
>
> For example you would make a filter that acts like a current
"around"
> filter. (I would call it a "before and after" filter)
>
> def filter(controller)
> if stuff_that_would_normally_be_a_before_action(args)
> yield # runs the action (or the following filter)
> stuff_that_would_normally_be_an_after_action
> else
> # cleanup if before_action stuff failed
> end
> end
>
> This way when the action returns, it should run. If this is not the case,
> please let me know a test case I could use to make sure this will work in
> the new system.
>
> -Martin
>
> On 7/2/06, Julian ''Julik'' Tarkhanov
<listbox@julik.nl> wrote:
> >
> >
> > On 2-jul-2006, at 22:49, Andrew Kaspick wrote:
> >
> > > I''ve definitely had a need for more improved around and
after filters.
> > > If an action is cached, any after or around filters will not
execute
> > > as processing is basically halted after the cached page is
served.
> >
> > The problem here is that there is no assurance that an after-filter
> > will run. That way scoping sometimes propagates
> > through all requests via the class variables. Basically it seems that
> > it would be a nice idea to have around filters which are wrapped
> > in begin/ensure so that the around filter is always fired for both
> > before and after.
> >
> > I looked at how AP handles that and it might be somewhat cumbersome
> > to implement.
> > --
> > Julian ''Julik'' Tarkhanov
> > please send all personal mail to
> > me at julik.nl
> >
> >
> > _______________________________________________
> > Rails-core mailing list
> > Rails-core@lists.rubyonrails.org
> > http://lists.rubyonrails.org/mailman/listinfo/rails-core
> >
>
> _______________________________________________
> Rails-core mailing list
> Rails-core@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>
>
>
> _______________________________________________
> Rails-core mailing list
> Rails-core@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>
>
>
_______________________________________________
Rails-core mailing list
Rails-core@lists.rubyonrails.org
http://lists.rubyonrails.org/mailman/listinfo/rails-core
I would tend to think { |controller, block| block.call } would more
clear, especially to people new to the framework and ruby itself.
On 7/5/06, Martin Emde <martin.emde@gmail.com>
wrote:> Great. Good to see people agree with the idea.
>
> I still think there''s a lot of improvement to be made in the
filter code so
> I will continue making this patch.
>
> I also wanted to see what people''s preference was for yielding the
action
> from a filter. The problem with yield is that yield from a block passed to
> meantime_filter (or whatever you want to call it) will call the block
passed
> to the surrounding method at the time the Proc is created. This means the
> filter has to use block.call when it''s passed as a Proc, but can
call yield
> when it''s done as a method or an object responding to filter. It
would be
> nice to eliminate this inconsitency.
>
> An alternate option to yield would be
> controller.filter_chain.next
> to call the next filter/action.
>
> Yield may make more sense in some cases, but when you''re forced to
do a proc
> like { |controller, block| block.call } sometimes and def
> filter(controller); yield; end; other times it may cause confusion.
>
> What seems less confusing to you guys?
>
> -Martin
>
>
> On 7/2/06, Ezra Zygmuntowicz <ezmobius@gmail.com > wrote:
> >
> >
> > Martin-
> >
> >
> > Take a look at this plugin that just came out. I think it does what
you
> are looking for as far as wrapping actions and yielding to them. Even
allows
> for a chain of such filters.
> >
> >
> > http://roman2k.free.fr/rails/meantime_filter/0.1.0/rdoc/
> >
> >
> >
> > -Ezra
> >
> >
> >
> >
> > On Jul 2, 2006, at 4:27 PM, Martin Emde wrote:
> >
> > Well, if you use the new around filter that I''m proposing you
won''t have a
> "before action" and "after action" you''ll just
have one method that yields
> the perform_action for the controller. This way it''s unavoidable
(without an
> exception being raised) for the code not to be run.
> >
> > For example you would make a filter that acts like a current
"around"
> filter. (I would call it a "before and after" filter)
> >
> > def filter(controller)
> > if stuff_that_would_normally_be_a_before_action(args)
> > yield # runs the action (or the following filter)
> > stuff_that_would_normally_be_an_after_action
> > else
> > # cleanup if before_action stuff failed
> > end
> > end
> >
> > This way when the action returns, it should run. If this is not the
case,
> please let me know a test case I could use to make sure this will work in
> the new system.
> >
> > -Martin
> >
> >
> > On 7/2/06, Julian ''Julik'' Tarkhanov <
listbox@julik.nl> wrote:
> > >
> > > On 2-jul-2006, at 22:49, Andrew Kaspick wrote:
> > >
> > > > I''ve definitely had a need for more improved around
and after filters.
> > > > If an action is cached, any after or around filters will not
execute
> > > > as processing is basically halted after the cached page is
served.
> > >
> > > The problem here is that there is no assurance that an
after-filter
> > > will run. That way scoping sometimes propagates
> > > through all requests via the class variables. Basically it seems
that
> > > it would be a nice idea to have around filters which are wrapped
> > > in begin/ensure so that the around filter is always fired for
both
> > > before and after.
> > >
> > > I looked at how AP handles that and it might be somewhat
cumbersome
> > > to implement.
> > > --
> > > Julian ''Julik'' Tarkhanov
> > > please send all personal mail to
> > > me at julik.nl
> > >
> > >
> > > _______________________________________________
> > > Rails-core mailing list
> > > Rails-core@lists.rubyonrails.org
> > >
> http://lists.rubyonrails.org/mailman/listinfo/rails-core
> > >
> >
> >
> > _______________________________________________
> > Rails-core mailing list
> > Rails-core@lists.rubyonrails.org
> > http://lists.rubyonrails.org/mailman/listinfo/rails-core
> >
> > _______________________________________________
> > Rails-core mailing list
> > Rails-core@lists.rubyonrails.org
> > http://lists.rubyonrails.org/mailman/listinfo/rails-core
> >
> >
> >
>
>
> _______________________________________________
> Rails-core mailing list
> Rails-core@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails-core
>
>
>