Changeset 5268

Timestamp:

10/09/06 06:11:40 (2 years ago)

Author:

bitsweat

Message:

r5540@ks: jeremy | 2006-10-08 23:05:30 -0700
#5949
r5541@ks: jeremy | 2006-10-08 23:07:08 -0700
Fix filter skipping in controller subclasses.
r5557@ks: jeremy | 2006-10-08 23:11:24 -0700
Update changelog. Closes #5949, references #6297, references #6299.

Files:

• trunk/actionpack/CHANGELOG (modified) (1 diff)
• trunk/actionpack/lib/action_controller/filters.rb (modified) (8 diffs)
• trunk/actionpack/test/controller/filters_test.rb (modified) (3 diffs)

trunk/actionpack/CHANGELOG

@@ -1 +1 @@
-*SVN*
+
+* Fix filter skipping in controller subclasses.  #5949, #6297, #6299 [Martin Emde]
-
-* render_text may optionally append to the response body. render_javascript appends by default. This allows you to chain multiple render :update calls by setting @performed_render = false between them (awaiting a better public API).  [Jeremy Kemper]

trunk/actionpack/lib/action_controller/filters.rb

@@ -363 +363 @@
-      # Returns a mapping between filters and the actions that may run them.
-      def included_actions #:nodoc:
-filter_chain.inject({}) { |h,f| h[f.filter] = f.included_actions; h 
+read_inheritable_attribute("included_actions") || {
-      end
-
-      # Returns a mapping between filters and actions that may not run them.
-      def excluded_actions #:nodoc:
-filter_chain.inject({}) { |h,f| h[f.filter] = f.excluded_actions; h 
+read_inheritable_attribute("excluded_actions") || {
-      end
-
@@ -378 +378 @@
-      def find_filter(filter, &block) #:nodoc:
-        filter_chain.select { |f| f.filter == filter && (!block_given? || yield(f)) }.first
+      end
+
+      # Returns true if the filter is excluded from the given action
+      def filter_excluded_from_action?(filter,action) #:nodoc:
+        if (ia = included_actions[filter]) && !ia.empty?
+          !ia.include?(action)
+        else
+          (excluded_actions[filter] || []).include?(action)
+        end
-      end
-
@@ -385 +394 @@
-        attr_reader :filter, :included_actions, :excluded_actions
-
-, options={}
+
-          @filter = filter
-          @position = options[:position]
-          update_conditions(options)
-        end
-
-        def update_conditions(conditions)
-          if conditions[:only]
-            @included_actions = [conditions[:only]].flatten.map(&:to_s)
-          else
-            @excluded_actions = [conditions[:except]].flatten.compact.map(&:to_s)
-          end
-          build_excluded_actions_hash
-        end
-
-        def remove_actions_from_included_actions!(*actions)
-          if @included_actions
-            actions.flatten.map(&:to_s).each { |action| @included_actions.delete(action) }
-            build_excluded_actions_hash
-          end
-        end
-
@@ -415 +406 @@
-        end
-
-        def excluded_from?(action)
-          @excluded_actions_hash[action]
-        end
-
-        def call(controller, &block)
-          raise(ActionControllerError, 'No filter type: Nothing to do here.')
-        end
-
-        private
-        def build_excluded_actions_hash
-          @excluded_actions_hash =
-            if @included_actions
-              @included_actions.inject(Hash.new(true)){|h, a| h[a] = false; h}
-            else
-              @excluded_actions.inject(Hash.new(false)){|h, a| h[a] = true; h}
-            end
-        end
-      end
@@ -512 +489 @@
-        def create_filters(filters, position, &block) #:nodoc:
-          filters, conditions = extract_conditions(filters, &block)
-
-
-
-
-
-
+
+
-            # change all the filters into instances of Filter derived classes
-
-
+
+
+
+
-            # apply proxy to filter if necessary
-            case position
-            when :before
-, proxy_conditions
+
-            when :after
-, proxy_conditions
+
-            else
-              filter
-            end
-          end
+
+          update_conditions(filters, conditions)
+
+          filters
-        end
-
@@ -554 +533 @@
-        end
-
+        def extract_conditions(*filters, &block) #:nodoc:
+          filters.flatten!
+          conditions = filters.last.is_a?(Hash) ? filters.pop : {}
+          filters << block if block_given?
+          return filters, conditions
+        end
+
+        def update_conditions(filters, conditions)
+          return if conditions.empty?
+          if conditions[:only]
+            write_inheritable_hash('included_actions', condition_hash(filters, conditions[:only]))
+          else
+            write_inheritable_hash('excluded_actions', condition_hash(filters, conditions[:except])) if conditions[:except]
+          end
+        end
+
+        def condition_hash(filters, *actions)
+          actions = actions.flatten.map(&:to_s)
+          filters.inject({}) { |h,f| h.update( f => (actions.blank? ? nil : actions)) }
+        end
+
+        def skip_filter_in_chain(*filters, &test) #:nodoc:
+          filters, conditions = extract_conditions(filters)
+          filters.map! { |f| block_given? ? find_filter(f, &test) : find_filter(f) }
+          filters.compact!
+
+          if conditions.empty?
+            delete_filters_in_chain(filters)
+          else
+            remove_actions_from_included_actions!(filters,conditions[:only] || [])
+            conditions[:only], conditions[:except] = conditions[:except], conditions[:only]
+            update_conditions(filters,conditions)
+          end
+        end
+
+        def remove_actions_from_included_actions!(filters,*actions)
+          actions = actions.flatten.map(&:to_s)
+          updated_hash = filters.inject(included_actions) do |hash,filter|
+            ia = (hash[filter] || []) - actions
+            ia.blank? ? hash.delete(filter) : hash[filter] = ia
+            hash
+          end
+          write_inheritable_attribute('included_actions', updated_hash)
+        end
+
+        def delete_filters_in_chain(filters) #:nodoc:
+          write_inheritable_attribute('filter_chain', filter_chain.reject { |f| filters.include?(f) })
+        end
+
-        def filter_responds_to_before_and_after(filter) #:nodoc:
-          filter.respond_to?(:before) && filter.respond_to?(:after)
@@ -570 +598 @@
-          end
-        end
-
-        def extract_conditions(*filters, &block) #:nodoc:
-          filters.flatten!
-          conditions = filters.last.is_a?(Hash) ? filters.pop : {}
-          filters << block if block_given?
-          return filters.flatten, conditions
-        end
-
-        def skip_filter_in_chain(*filters, &test) #:nodoc:
-          filters, conditions = extract_conditions(filters)
-          finder_proc = block_given? ?
-            proc { |f| find_filter(f, &test) } :
-            proc { |f| find_filter(f) }
-
-          filters.map(&finder_proc).reject(&:nil?).each do |filter|
-            if conditions.empty?
-              delete_filter_in_chain(filter)
-            else
-              filter.remove_actions_from_included_actions!(conditions[:only] || [])
-              conditions[:only], conditions[:except] = conditions[:except], conditions[:only]
-              filter.update_conditions(conditions)
-            end
-          end
-        end
-
-        def delete_filter_in_chain(filter) #:nodoc:
-          write_inheritable_attribute('filter_chain', filter_chain.reject { |f| f == filter })
-        end
-    end
-
@@ -628 +628 @@
-        return (performed? || perform_action_without_filters) if index >= chain.size
-        filter = chain[index]
-filter.excluded_from?(
+self.class.filter_excluded_from_action?(filter,
-
-        halted = false

trunk/actionpack/test/controller/filters_test.rb

@@ -155 +155 @@
-      end
-  end
-
+
-  class ConditionalParentOfConditionalSkippingController < ConditionalFilterController
-    before_filter :conditional_in_parent, :only => [:show, :another_action]
@@ -171 +171 @@
-    skip_before_filter :conditional_in_parent, :only => :another_action
-    skip_after_filter  :conditional_in_parent, :only => :another_action
+  end
+
+  class AnotherChildOfConditionalParentController < ConditionalParentOfConditionalSkippingController
+    skip_before_filter :conditional_in_parent, :only => :show
-  end
-
@@ -414 +418 @@
-  end
-
+  def test_condition_skipping_of_filters_when_siblings_also_have_conditions
+    assert_equal %w( conditional_in_parent conditional_in_parent ), test_process(ChildOfConditionalParentController).template.assigns['ran_filter'], "1"
+    assert_equal nil, test_process(AnotherChildOfConditionalParentController).template.assigns['ran_filter']
+    assert_equal %w( conditional_in_parent conditional_in_parent ), test_process(ChildOfConditionalParentController).template.assigns['ran_filter']
+  end
+
-  private
-    def test_process(controller, action = "show")