From c1aa5b0e14cd4bd27a5d8bd85cf7c36fa5911830 Mon Sep 17 00:00:00 2001 From: Yehuda Katz and Carl Lerche Date: Tue, 7 Apr 2009 14:57:18 -0700 Subject: [PATCH] Add depends_on, use, and setup to abstract up ideas about module inheritance. --- .../action_controller/abstract/callbacks.rb | 9 +++---- .../lib/action_controller/abstract/helpers.rb | 23 ++++++++-------- .../lib/action_controller/abstract/layouts.rb | 17 ++++++------ .../lib/action_controller/abstract/logger.rb | 4 +-- .../action_controller/abstract/renderer.rb | 20 ++++++-------- .../new_base/hide_actions.rb | 9 +++---- .../lib/action_controller/new_base/layouts.rb | 3 +++ .../action_controller/new_base/renderer.rb | 11 +------- .../abstract_controller_test.rb | 4 +-- .../abstract_controller/callbacks_test.rb | 2 +- .../test/abstract_controller/helper_test.rb | 4 +-- .../test/abstract_controller/layouts_test.rb | 4 +-- actionpack/test/new_base/test_helper.rb | 18 ++++++------- .../lib/active_support/core_ext/module.rb | 1 + .../active_support/core_ext/module/setup.rb | 26 +++++++++++++++++++ 15 files changed, 82 insertions(+), 73 deletions(-) create mode 100644 activesupport/lib/active_support/core_ext/module/setup.rb diff --git a/actionpack/lib/action_controller/abstract/callbacks.rb b/actionpack/lib/action_controller/abstract/callbacks.rb index a88d4c15675e1..c8b509081c840 100644 --- a/actionpack/lib/action_controller/abstract/callbacks.rb +++ b/actionpack/lib/action_controller/abstract/callbacks.rb @@ -1,11 +1,8 @@ module AbstractController module Callbacks - def self.included(klass) - klass.class_eval do - include ActiveSupport::NewCallbacks - define_callbacks :process_action - extend ClassMethods - end + setup do + include ActiveSupport::NewCallbacks + define_callbacks :process_action end def process_action diff --git a/actionpack/lib/action_controller/abstract/helpers.rb b/actionpack/lib/action_controller/abstract/helpers.rb index 1947c360b84d8..1f0b38417b7d5 100644 --- a/actionpack/lib/action_controller/abstract/helpers.rb +++ b/actionpack/lib/action_controller/abstract/helpers.rb @@ -1,17 +1,18 @@ module AbstractController module Helpers - - def self.included(klass) - klass.class_eval do - extend ClassMethods - unless self < ::AbstractController::Renderer - raise "You need to include AbstractController::Renderer before including " \ - "AbstractController::Helpers" - end - extlib_inheritable_accessor :master_helper_module - self.master_helper_module = Module.new - end + depends_on Renderer + + setup do + extlib_inheritable_accessor :master_helper_module + self.master_helper_module = Module.new end + + # def self.included(klass) + # klass.class_eval do + # extlib_inheritable_accessor :master_helper_module + # self.master_helper_module = Module.new + # end + # end def _action_view @_action_view ||= begin diff --git a/actionpack/lib/action_controller/abstract/layouts.rb b/actionpack/lib/action_controller/abstract/layouts.rb index 20c9abb9e500c..268028315124e 100644 --- a/actionpack/lib/action_controller/abstract/layouts.rb +++ b/actionpack/lib/action_controller/abstract/layouts.rb @@ -1,25 +1,24 @@ module AbstractController module Layouts - def self.included(base) - base.extend ClassMethods - end - + depends_on Renderer + module ClassMethods def layout(layout) unless [String, Symbol, FalseClass, NilClass].include?(layout.class) raise ArgumentError, "Layouts must be specified as a String, Symbol, false, or nil" end - @layout = layout || false # Converts nil to false + @_layout = layout || false # Converts nil to false + _write_layout_method end def _write_layout_method - case @layout + case @_layout when String - self.class_eval %{def _layout() #{@layout.inspect} end} + self.class_eval %{def _layout() #{@_layout.inspect} end} when Symbol - self.class_eval %{def _layout() #{@layout} end} + self.class_eval %{def _layout() #{@_layout} end} when false self.class_eval %{def _layout() end} else @@ -43,7 +42,7 @@ def _render_template(template, options) private def _layout() end # This will be overwritten - + def _layout_for_option(name) case name when String then _layout_for_name(name) diff --git a/actionpack/lib/action_controller/abstract/logger.rb b/actionpack/lib/action_controller/abstract/logger.rb index 846d8ad040f15..4117369bd46ed 100644 --- a/actionpack/lib/action_controller/abstract/logger.rb +++ b/actionpack/lib/action_controller/abstract/logger.rb @@ -1,7 +1,7 @@ module AbstractController module Logger - def self.included(klass) - klass.cattr_accessor :logger + setup do + cattr_accessor :logger end end end \ No newline at end of file diff --git a/actionpack/lib/action_controller/abstract/renderer.rb b/actionpack/lib/action_controller/abstract/renderer.rb index 5daade61092cd..68c3b07b84738 100644 --- a/actionpack/lib/action_controller/abstract/renderer.rb +++ b/actionpack/lib/action_controller/abstract/renderer.rb @@ -2,20 +2,16 @@ module AbstractController module Renderer + depends_on AbstractController::Logger - def self.included(klass) - klass.class_eval do - extend ClassMethods - - attr_internal :formats - - extlib_inheritable_accessor :_view_paths - - self._view_paths ||= ActionView::PathSet.new - include AbstractController::Logger - end + setup do + attr_internal :formats + + extlib_inheritable_accessor :_view_paths + + self._view_paths ||= ActionView::PathSet.new end - + def _action_view @_action_view ||= ActionView::Base.new(self.class.view_paths, {}, self) end diff --git a/actionpack/lib/action_controller/new_base/hide_actions.rb b/actionpack/lib/action_controller/new_base/hide_actions.rb index b3777c3c1e008..473a8ea72b39a 100644 --- a/actionpack/lib/action_controller/new_base/hide_actions.rb +++ b/actionpack/lib/action_controller/new_base/hide_actions.rb @@ -1,11 +1,8 @@ module ActionController module HideActions - def self.included(klass) - klass.class_eval do - extend ClassMethods - extlib_inheritable_accessor :hidden_actions - self.hidden_actions ||= Set.new - end + setup do + extlib_inheritable_accessor :hidden_actions + self.hidden_actions ||= Set.new end def action_methods() self.class.action_names end diff --git a/actionpack/lib/action_controller/new_base/layouts.rb b/actionpack/lib/action_controller/new_base/layouts.rb index c0efb669b2d37..2351472dad2a3 100644 --- a/actionpack/lib/action_controller/new_base/layouts.rb +++ b/actionpack/lib/action_controller/new_base/layouts.rb @@ -1,5 +1,8 @@ module ActionController module Layouts + depends_on ActionController::Renderer + depends_on AbstractController::Layouts + def render_to_string(options) if !options.key?(:text) || options.key?(:layout) options[:_layout] = options.key?(:layout) ? _layout_for_option(options[:layout]) : _default_layout diff --git a/actionpack/lib/action_controller/new_base/renderer.rb b/actionpack/lib/action_controller/new_base/renderer.rb index 24ca9be077c60..044c15f409e6e 100644 --- a/actionpack/lib/action_controller/new_base/renderer.rb +++ b/actionpack/lib/action_controller/new_base/renderer.rb @@ -1,15 +1,6 @@ module ActionController module Renderer - - # def self.included(klass) - # klass.extend ClassMethods - # end - # - # module ClassMethods - # def prefix - # @prefix ||= name.underscore - # end - # end + depends_on AbstractController::Renderer def initialize(*) self.formats = [:html] diff --git a/actionpack/test/abstract_controller/abstract_controller_test.rb b/actionpack/test/abstract_controller/abstract_controller_test.rb index 96193fd24ca7e..6d338888211c9 100644 --- a/actionpack/test/abstract_controller/abstract_controller_test.rb +++ b/actionpack/test/abstract_controller/abstract_controller_test.rb @@ -27,7 +27,7 @@ class TestBasic < ActiveSupport::TestCase # Test Render mixin # ==== class RenderingController < AbstractController::Base - include Renderer + use Renderer def _prefix() end @@ -116,7 +116,7 @@ class TestPrefixedViews < ActiveSupport::TestCase # ==== # self._layout is used when defined class WithLayouts < PrefixedViews - include Layouts + use Layouts private def self.layout(formats) diff --git a/actionpack/test/abstract_controller/callbacks_test.rb b/actionpack/test/abstract_controller/callbacks_test.rb index 89243b631e427..5fce30f4780b8 100644 --- a/actionpack/test/abstract_controller/callbacks_test.rb +++ b/actionpack/test/abstract_controller/callbacks_test.rb @@ -4,7 +4,7 @@ module AbstractController module Testing class ControllerWithCallbacks < AbstractController::Base - include AbstractController::Callbacks + use AbstractController::Callbacks end class Callback1 < ControllerWithCallbacks diff --git a/actionpack/test/abstract_controller/helper_test.rb b/actionpack/test/abstract_controller/helper_test.rb index e1b2141331fa4..6284fa4f704bf 100644 --- a/actionpack/test/abstract_controller/helper_test.rb +++ b/actionpack/test/abstract_controller/helper_test.rb @@ -4,8 +4,8 @@ module AbstractController module Testing class ControllerWithHelpers < AbstractController::Base - include Renderer - include Helpers + use Renderer + use Helpers def render(string) super(:_template_name => string) diff --git a/actionpack/test/abstract_controller/layouts_test.rb b/actionpack/test/abstract_controller/layouts_test.rb index 838a44be12fa2..ec8faffc51be1 100644 --- a/actionpack/test/abstract_controller/layouts_test.rb +++ b/actionpack/test/abstract_controller/layouts_test.rb @@ -5,8 +5,8 @@ module Layouts # Base controller for these tests class Base < AbstractController::Base - include AbstractController::Renderer - include AbstractController::Layouts + use AbstractController::Renderer + use AbstractController::Layouts self.view_paths = [ActionView::FixtureTemplate::FixturePath.new( "layouts/hello.erb" => "With String <%= yield %>", diff --git a/actionpack/test/new_base/test_helper.rb b/actionpack/test/new_base/test_helper.rb index 420abe58f7907..90580600592dc 100644 --- a/actionpack/test/new_base/test_helper.rb +++ b/actionpack/test/new_base/test_helper.rb @@ -26,16 +26,14 @@ module ActionController class Base2 < AbstractBase - include AbstractController::Callbacks - include AbstractController::Renderer - include AbstractController::Helpers - include AbstractController::Layouts - include AbstractController::Logger - - include ActionController::HideActions - include ActionController::UrlFor - include ActionController::Layouts - include ActionController::Renderer + use AbstractController::Callbacks + use AbstractController::Helpers + use AbstractController::Logger + + use ActionController::HideActions + use ActionController::UrlFor + use ActionController::Renderer + use ActionController::Layouts def self.inherited(klass) ::ActionController::Base2.subclasses << klass.to_s diff --git a/activesupport/lib/active_support/core_ext/module.rb b/activesupport/lib/active_support/core_ext/module.rb index da8d28ec13570..cb314370944ef 100644 --- a/activesupport/lib/active_support/core_ext/module.rb +++ b/activesupport/lib/active_support/core_ext/module.rb @@ -8,6 +8,7 @@ require 'active_support/core_ext/module/aliasing' require 'active_support/core_ext/module/model_naming' require 'active_support/core_ext/module/synchronization' +require 'active_support/core_ext/module/setup' module ActiveSupport module CoreExtensions diff --git a/activesupport/lib/active_support/core_ext/module/setup.rb b/activesupport/lib/active_support/core_ext/module/setup.rb new file mode 100644 index 0000000000000..e6dfd0cf56757 --- /dev/null +++ b/activesupport/lib/active_support/core_ext/module/setup.rb @@ -0,0 +1,26 @@ +class Module + attr_accessor :_setup_block + attr_accessor :_dependencies + + def setup(&blk) + @_setup_block = blk + end + + def use(mod) + return if self < mod + + (mod._dependencies || []).each do |dep| + use dep + end + # raise "Circular dependencies" if self < mod + include mod + extend mod.const_get("ClassMethods") if mod.const_defined?("ClassMethods") + class_eval(&mod._setup_block) if mod._setup_block + end + + def depends_on(mod) + return if self < mod + @_dependencies ||= [] + @_dependencies << mod + end +end \ No newline at end of file