[katello-devel] before_filter behavior and notices

Brad Buckingham bbuckingham at redhat.com
Wed Jul 20 13:09:41 UTC 2011


Team,

While working on activation keys within a remote branch, I decided to do 
some 'copy/paste' reuse
of a before_filter.  In doing so, I observed a couple of issues with the 
filter that I chose to 'reuse'.  After looking further, I found that the 
same issues existed in several controllers.  This email is to share the 
finding, in case others may benefit.

Observation/Issue 1 - several of the before_filters are used to fetch 
ActiveRecord data (e.g. find); however, they are not catching the 
exception if the ActiveRecord cannot be found.  As a result, the error 
logic contained in the filter is being skipped.
     Fix - wrap the find in a begin/rescue block, catching the AR 
exceptions.

Observation/Issue 2 - when the exception is caught, the error notice 
(i.e. errors) is not being displayed to the user.  The reason being, the 
error handling logic in these filters generally performs a render or 
redirect, which will halt the filter chain causing the flash_to_headers 
after_filter not to be executed.
     Fix - added a new method to the ApplicationController called 
'execute_after_filters' and call this method from within the 
before_filter's error leg.  This method currently only calls 
'flash_to_headers'; however, if other after_filters are introduced in 
the ApplicationController in the future, they can also be added here.

I have gone through the code to address these observations in the UI 
controllers, where appropriate.  For details on the commit, it is:

http://git.fedorahosted.org/git/?p=katello.git;a=commit;h=5165c9b2cc463a80df0ed73b22d006b8ff65e6e3

For an example of the typical changes made to the filters, see:

http://git.fedorahosted.org/git/?p=katello.git;a=blobdiff;f=src/app/controllers/sync_plans_controller.rb;h=4e3fe1834d841fdfbe6c7b267574ca55b22bc146;hp=89b94e9e3957e351c1535c5fe98be8770fd71208;hb=5165c9b2cc463a80df0ed73b22d006b8ff65e6e3;hpb=b7cea35221795fa7a779e04fc524cd74b554596e

thanks,
Brad




More information about the katello-devel mailing list