#6226 before_filters overwriting each other - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#6226 new

before_filters overwriting each other

Reported by Alexey Ilyichev | December 27th, 2010 @ 12:28 PM

Hi!

Consider this code:

class UsersController < ApplicationController
  before_filter :log_filter_call, :only => :index
  before_filter :log_filter_call, :only => :show
  # ...
  # standard scaffold methods
private
  def log_filter_call
   logger.debug 'Before filter called during ' + params[:controller] + '#' + params[:action]
  end
end

In rails 2.3.10 log_filter_call not called for #index action, and called twice for #show action
In rails 3.0.3 log_filter_call not called for #index action, and called once for #show action

I expected it being called once for both #index and #show.

Comments and changes to this ticket

You flagged this item as spam.
Alexey Ilyichev December 27th, 2010 @ 03:02 PM
It seems to me that this code from active_support/callbacks.rb (set_callback method) is what causes that behaviour. I wonder why it was made this way.
```
filters.each do |filter|
  chain.delete_if {|c| c.matches?(type, filter) }
end
```

Oriol Gual December 27th, 2010 @ 03:32 PM

Why don't you just use this:

class UsersController < ApplicationController
  before_filter :log_filter_call, :only => [:index, :show]
  # ...
end

You flagged this item as spam.
Alexey Ilyichev December 27th, 2010 @ 03:36 PM
Originally we wanted to split some of our controllers into mixins, containing sets of actions, because in our app we have many namespaces that share alot of common actions, but not totally same. And some actions that should belong to different mixins might have the same before_filters, so it would be nice if we could declare the same filter in these mixins and not get a conflict like this.
You flagged this item as spam.
2kan December 28th, 2010 @ 10:45 AM
+1 I expected it being called once for both #index and #show too.
You flagged this item as spam.
tikh December 29th, 2010 @ 09:52 AM
- Tag set to “action_controller, before_filter”
+1
You flagged this item as spam.
Alexey Ilyichev December 30th, 2010 @ 09:43 AM
Seems it was intended this way. See commit ef692162.
Neeraj Singh January 22nd, 2011 @ 02:47 AM
- Importance changed from “” to “Low”
This is how callbacks are working right now.
```
class A < ApplicationController
 before_filter :ensure_logged_in, :only => [:index] 
 def index
  end
end

class B < A
 before_filter :ensure_logged_in, :except => [:index] 
 # index will be called without calling ensure_logged_in
 def index
 end
end
```
If the behavior of the callback is changed to not to remove duplicate callbacks then a subclass always has to worry about unwanted changes propagating to it. That would mean any subclass will have to guard itself by using skip_before_filter then use its own filter.

On the flip side I do think inheritance means subclass probably wants the behavior given to parent class.

This change would potentially break existing apps if they are relying in the feature I illustrated above.