From f55514125cae291791365effc856d237008f7cd2 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Tue, 17 Mar 2009 18:04:22 -0700 Subject: [PATCH] Working toward getting a basic AbstractController framework --- actionpack/lib/action_controller/abstract.rb | 14 +- .../lib/action_controller/abstract/base.rb | 12 +- .../action_controller/abstract/exceptions.rb | 3 + actionpack/lib/action_controller/new_base.rb | 1 + .../lib/action_controller/new_base/base.rb | 37 +- .../new_base/hide_actions.rb | 6 + .../action_controller/new_base/renderer.rb | 11 + .../abstract_controller_test.rb | 70 ++++ actionpack/test/new_base/base_test.rb | 387 ++++++------------ 9 files changed, 269 insertions(+), 272 deletions(-) create mode 100644 actionpack/lib/action_controller/abstract/exceptions.rb create mode 100644 actionpack/lib/action_controller/new_base/renderer.rb diff --git a/actionpack/lib/action_controller/abstract.rb b/actionpack/lib/action_controller/abstract.rb index 85995189a3..3f5c4a185f 100644 --- a/actionpack/lib/action_controller/abstract.rb +++ b/actionpack/lib/action_controller/abstract.rb @@ -1,8 +1,10 @@ module AbstractController - autoload :Base, "action_controller/abstract/base" - autoload :Callbacks, "action_controller/abstract/callbacks" - autoload :Helpers, "action_controller/abstract/helpers" - autoload :Layouts, "action_controller/abstract/layouts" - autoload :Logger, "action_controller/abstract/logger" - autoload :Renderer, "action_controller/abstract/renderer" + autoload :Base, "action_controller/abstract/base" + autoload :Callbacks, "action_controller/abstract/callbacks" + autoload :Helpers, "action_controller/abstract/helpers" + autoload :Layouts, "action_controller/abstract/layouts" + autoload :Logger, "action_controller/abstract/logger" + autoload :Renderer, "action_controller/abstract/renderer" + # === Exceptions + autoload :ActionNotFound, "action_controller/abstract/exceptions" end \ No newline at end of file diff --git a/actionpack/lib/action_controller/abstract/base.rb b/actionpack/lib/action_controller/abstract/base.rb index fa86d68c04..ade7719cc0 100644 --- a/actionpack/lib/action_controller/abstract/base.rb +++ b/actionpack/lib/action_controller/abstract/base.rb @@ -17,14 +17,24 @@ module AbstractController end def process(action_name) + unless respond_to_action?(action_name) + raise ActionNotFound, "The action '#{action_name}' could not be found" + end + @_action_name = action_name process_action self.response_obj[:body] = self.response_body self end + private + def process_action - send(action_name) + respond_to?(action_name) ? send(action_name) : send(:action_missing, action_name) + end + + def respond_to_action?(action_name) + respond_to?(action_name) || respond_to?(:action_missing, true) end end diff --git a/actionpack/lib/action_controller/abstract/exceptions.rb b/actionpack/lib/action_controller/abstract/exceptions.rb new file mode 100644 index 0000000000..ec4680629b --- /dev/null +++ b/actionpack/lib/action_controller/abstract/exceptions.rb @@ -0,0 +1,3 @@ +module AbstractController + class ActionNotFound < StandardError ; end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/new_base.rb b/actionpack/lib/action_controller/new_base.rb index 2cef221de3..2870f71b7d 100644 --- a/actionpack/lib/action_controller/new_base.rb +++ b/actionpack/lib/action_controller/new_base.rb @@ -1,5 +1,6 @@ module ActionController autoload :AbstractBase, "action_controller/new_base/base" autoload :HideActions, "action_controller/new_base/hide_actions" + autoload :Renderer, "action_controller/new_base/renderer" autoload :UrlFor, "action_controller/new_base/url_for" end \ No newline at end of file diff --git a/actionpack/lib/action_controller/new_base/base.rb b/actionpack/lib/action_controller/new_base/base.rb index ebe7c8dda6..0400ddbf7a 100644 --- a/actionpack/lib/action_controller/new_base/base.rb +++ b/actionpack/lib/action_controller/new_base/base.rb @@ -1,26 +1,61 @@ module ActionController class AbstractBase < AbstractController::Base + + # :api: public attr_internal :request, :response, :params + # :api: public def self.controller_name @controller_name ||= controller_path.split("/").last end + # :api: public def controller_name() self.class.controller_name end - + + # :api: public def self.controller_path @controller_path ||= self.name.sub(/Controller$/, '').underscore end + # :api: public def controller_path() self.class.controller_path end + # :api: private def self.action_methods @action_names ||= Set.new(self.public_instance_methods - self::CORE_METHODS) end + # :api: private def self.action_names() action_methods end + # :api: private def action_methods() self.class.action_names end + + # :api: private def action_names() action_methods end + + # :api: plugin + def self.call(env) + controller = new + controller.call(env).to_rack + end + + # :api: plugin + def response_body=(body) + @_response["Content-Length"] = body.length + @_response.body = body + end + + # :api: private + def call(env) + @_request = ActionDispatch::Request.new(env) + @_response = ActionDispatch::Response.new + process(@_request.parameters[:action]) + end + + # :api: private + def to_rack + response.to_a + end end end \ No newline at end of file diff --git a/actionpack/lib/action_controller/new_base/hide_actions.rb b/actionpack/lib/action_controller/new_base/hide_actions.rb index 9847d7b086..b3777c3c1e 100644 --- a/actionpack/lib/action_controller/new_base/hide_actions.rb +++ b/actionpack/lib/action_controller/new_base/hide_actions.rb @@ -10,6 +10,12 @@ module ActionController def action_methods() self.class.action_names end def action_names() action_methods end + + private + + def respond_to_action?(action_name) + !hidden_actions.include?(action_name) && (super || respond_to?(:method_missing)) + end module ClassMethods def hide_action(*args) diff --git a/actionpack/lib/action_controller/new_base/renderer.rb b/actionpack/lib/action_controller/new_base/renderer.rb new file mode 100644 index 0000000000..503450c246 --- /dev/null +++ b/actionpack/lib/action_controller/new_base/renderer.rb @@ -0,0 +1,11 @@ +module ActionController + module Renderer + + def render(options) + if text = options[:text] + self.response_body = text + end + end + + end +end \ No newline at end of file diff --git a/actionpack/test/abstract_controller/abstract_controller_test.rb b/actionpack/test/abstract_controller/abstract_controller_test.rb index 7b0caa3837..22fc1a8c41 100644 --- a/actionpack/test/abstract_controller/abstract_controller_test.rb +++ b/actionpack/test/abstract_controller/abstract_controller_test.rb @@ -3,6 +3,10 @@ require File.join(File.expand_path(File.dirname(__FILE__)), "test_helper") module AbstractController module Testing + # Test basic dispatching. + # ==== + # * Call process + # * Test that the response_body is set correctly class SimpleController < AbstractController::Base end @@ -20,6 +24,8 @@ module AbstractController end end + # Test Render mixin + # ==== class RenderingController < AbstractController::Base include Renderer @@ -58,6 +64,9 @@ module AbstractController end end + # Test rendering with prefixes + # ==== + # * self._prefix is used when defined class PrefixedViews < RenderingController private def self.prefix @@ -92,6 +101,9 @@ module AbstractController end end + # Test rendering with layouts + # ==== + # self._layout is used when defined class WithLayouts < PrefixedViews include Layouts @@ -136,5 +148,63 @@ module AbstractController end end + # respond_to_action?(action_name) + # ==== + # * A method can be used as an action only if this method + # returns true when passed the method name as an argument + # * Defaults to true in AbstractController + class DefaultRespondToActionController < AbstractController::Base + def index() self.response_body = "success" end + end + + class ActionMissingRespondToActionController < AbstractController::Base + # No actions + private + def action_missing(action_name) + self.response_body = "success" + end + end + + class RespondToActionController < AbstractController::Base; + def index() self.response_body = "success" end + + def fail() self.response_body = "fail" end + + private + + def respond_to_action?(action_name) + action_name != :fail + end + + end + + class TestRespondToAction < ActiveSupport::TestCase + + def assert_dispatch(klass, body = "success", action = :index) + response = klass.process(action).response_obj[:body] + assert_equal body, response + end + + test "an arbitrary method is available as an action by default" do + assert_dispatch DefaultRespondToActionController, "success", :index + end + + test "raises ActionNotFound when method does not exist and action_missing is not defined" do + assert_raise(ActionNotFound) { DefaultRespondToActionController.process(:fail) } + end + + test "dispatches to action_missing when method does not exist and action_missing is defined" do + assert_dispatch ActionMissingRespondToActionController, "success", :ohai + end + + test "a method is available as an action if respond_to_action? returns true" do + assert_dispatch RespondToActionController, "success", :index + end + + test "raises ActionNotFound if method is defined but respond_to_action? returns false" do + assert_raise(ActionNotFound) { RespondToActionController.process(:fail) } + end + end + end end \ No newline at end of file diff --git a/actionpack/test/new_base/base_test.rb b/actionpack/test/new_base/base_test.rb index 7ac5eac3c5..9609c11753 100644 --- a/actionpack/test/new_base/base_test.rb +++ b/actionpack/test/new_base/base_test.rb @@ -23,38 +23,6 @@ require 'rubygems' require 'rack/test' module ActionController - module TestProcess - def process(action, parameters = nil, session = nil, flash = nil, http_method = 'GET') - # Sanity check for required instance variables so we can give an - # understandable error message. - %w(@controller @request @response).each do |iv_name| - if !(instance_variable_names.include?(iv_name) || instance_variable_names.include?(iv_name.to_sym)) || instance_variable_get(iv_name).nil? - raise "#{iv_name} is nil: make sure you set it in your test's setup method." - end - end - - @request.recycle! - @response.recycle! - - @html_document = nil - @request.env['REQUEST_METHOD'] = http_method - - @request.action = action.to_s - - parameters ||= {} - @request.assign_parameters(@controller.class.controller_path, action.to_s, parameters) - - @request.session = ActionController::TestSession.new(session) unless session.nil? - @request.session["flash"] = ActionController::Flash::FlashHash.new.update(flash) if flash - build_request_uri(action, parameters) - - # Base.class_eval { include ProcessWithTest } unless Base < ProcessWithTest - @controller.request = @request - @controller.response = @response - @controller.process(action) - end - end - class Base2 < AbstractBase include AbstractController::Callbacks include AbstractController::Renderer @@ -64,66 +32,134 @@ module ActionController include ActionController::HideActions include ActionController::UrlFor + include ActionController::Renderer CORE_METHODS = self.public_instance_methods end end -# Provide some controller to run the tests on. +# Temporary base class +class Rack::TestCase < ActiveSupport::TestCase + + include Rack::Test::Methods + + setup do + ActionController::Base.session_options[:key] = "abc" + ActionController::Base.session_options[:secret] = ("*" * 30) + ActionController::Routing.use_controllers! %w(happy_path/simple_dispatch) + end + + def self.get(url) + setup do |test| + test.get url + end + end + + def app + @app ||= ActionController::Dispatcher.new + end + + def assert_body(body) + assert_equal [body], last_response.body + end + + def assert_status(code) + assert_equal code, last_response.status + end + + def assert_content_type(type) + assert_equal type, last_response.headers["Content-Type"] + end + + def assert_header(name, value) + assert_equal value, last_response.headers[name] + end + +end + + +# Tests the controller dispatching happy path +module HappyPath + class SimpleDispatchController < ActionController::Base2 + def index + render :text => "success" + end + + def modify_response_body + self.response_body = "success" + end + + def modify_response_body_twice + ret = (self.response_body = "success") + self.response_body = "#{ret}!" + end + + def modify_response_headers + + end + end + + class SimpleRouteCase < Rack::TestCase + setup do + ActionController::Routing::Routes.draw do |map| + map.connect ':controller/:action' + end + end + end + + class TestSimpleDispatch < SimpleRouteCase + + get "/happy_path/simple_dispatch/index" + + test "sets the body" do + assert_body "success" + end + + test "sets the status code" do + assert_status 200 + end + + test "sets the content type" do + assert_content_type Mime::HTML + end + + test "sets the content length" do + assert_header "Content-Length", 7 + end + + end + + # :api: plugin + class TestDirectResponseMod < SimpleRouteCase + get "/happy_path/simple_dispatch/modify_response_body" + + test "sets the body" do + assert_body "success" + end + + test "setting the body manually sets the content length" do + assert_header "Content-Length", 7 + end + end + + # :api: plugin + class TestDirectResponseModTwice < SimpleRouteCase + get "/happy_path/simple_dispatch/modify_response_body_twice" + + test "self.response_body= returns the body being set" do + assert_body "success!" + end + + test "updating the response body updates the content length" do + assert_header "Content-Length", 8 + end + end +end + + +class EmptyController < ActionController::Base2 ; end module Submodule - class ContainedEmptyController < ActionController::Base2 - end - class ContainedNonEmptyController < ActionController::Base2 - def public_action - render :nothing => true - end - - hide_action :hidden_action - def hidden_action - raise "Noooo!" - end - - def another_hidden_action - end - hide_action :another_hidden_action - end - class SubclassedController < ContainedNonEmptyController - hide_action :public_action # Hiding it here should not affect the superclass. - end -end -class EmptyController < ActionController::Base2 -end -class NonEmptyController < ActionController::Base2 - def public_action - end - - hide_action :hidden_action - def hidden_action - end -end - -class MethodMissingController < ActionController::Base - - hide_action :shouldnt_be_called - def shouldnt_be_called - raise "NO WAY!" - end - -protected - - def method_missing(selector) - render :text => selector.to_s - end - -end - -class DefaultUrlOptionsController < ActionController::Base2 - def default_url_options_action - end - - def default_url_options(options = nil) - { :host => 'www.override.com', :action => 'new', :bacon => 'chunky' } - end + class ContainedEmptyController < ActionController::Base2 ; end end class ControllerClassTests < Test::Unit::TestCase @@ -137,181 +173,4 @@ class ControllerClassTests < Test::Unit::TestCase assert_equal 'empty', EmptyController.controller_name assert_equal 'contained_empty', Submodule::ContainedEmptyController.controller_name end -end - -class ControllerInstanceTests < Test::Unit::TestCase - def setup - @empty = EmptyController.new - @contained = Submodule::ContainedEmptyController.new - @empty_controllers = [@empty, @contained, Submodule::SubclassedController.new] - - @non_empty_controllers = [NonEmptyController.new, - Submodule::ContainedNonEmptyController.new] - end - - def test_action_methods - @empty_controllers.each do |c| - hide_mocha_methods_from_controller(c) - assert_equal Set.new, c.__send__(:action_methods), "#{c.controller_path} should be empty!" - end - @non_empty_controllers.each do |c| - hide_mocha_methods_from_controller(c) - assert_equal Set.new(%w(public_action)), c.__send__(:action_methods), "#{c.controller_path} should not be empty!" - end - end - - protected - # Mocha adds some public instance methods to Object that would be - # considered actions, so explicitly hide_action them. - def hide_mocha_methods_from_controller(controller) - mocha_methods = [ - :expects, :mocha, :mocha_inspect, :reset_mocha, :stubba_object, - :stubba_method, :stubs, :verify, :__metaclass__, :__is_a__, :to_matcher, - ] - controller.class.__send__(:hide_action, *mocha_methods) - end -end - - -class PerformActionTest < ActiveSupport::TestCase - class MockLogger - attr_reader :logged - - def initialize - @logged = [] - end - - def method_missing(method, *args) - @logged << args.first - end - end - - def use_controller(controller_class) - @controller = controller_class.new - - # enable a logger so that (e.g.) the benchmarking stuff runs, so we can get - # a more accurate simulation of what happens in "real life". - @controller.logger = Logger.new(nil) - - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new - - @request.host = "www.nxtangle.com" - - rescue_action_in_public! - end - - attr_accessor :app - include Rack::Test::Methods - - def with_routing - real_routes = ActionController::Routing::Routes - ActionController::Routing.module_eval { remove_const :Routes } - - temporary_routes = ActionController::Routing::RouteSet.new - ActionController::Routing.module_eval { const_set :Routes, temporary_routes } - - yield temporary_routes - ensure - if ActionController::Routing.const_defined? :Routes - ActionController::Routing.module_eval { remove_const :Routes } - end - ActionController::Routing.const_set(:Routes, real_routes) if real_routes - end - - def test_get_on_priv_should_show_selector - ActionController::Base.session_options[:key] = "abc" - ActionController::Base.session_options[:secret] = ("*" * 30) - - with_routing do |set| - set.draw do |map| - map.connect ':controller/:action' - end - - @app = ActionController::Dispatcher.new - - resp = get "/method_missing/shouldnt_be_called" - assert_equal 'shouldnt_be_called', resp.body - end - - # use_controller MethodMissingController - # get :shouldnt_be_called - # assert_response :success - # assert_equal 'shouldnt_be_called', @response.body - end - - def test_method_missing_is_not_an_action_name - use_controller MethodMissingController - assert ! @controller.__send__(:action_methods).include?('method_missing') - - get :method_missing - assert_response :success - assert_equal 'method_missing', @response.body - end - - def test_get_on_hidden_should_fail - use_controller NonEmptyController - get :hidden_action - assert_response 404 - - get :another_hidden_action - assert_response 404 - end - - def test_namespaced_action_should_log_module_name - use_controller Submodule::ContainedNonEmptyController - @controller.logger = MockLogger.new - get :public_action - assert_match /Processing\sSubmodule::ContainedNonEmptyController#public_action/, @controller.logger.logged[1] - end -end - -class DefaultUrlOptionsTest < ActionController::TestCase - tests DefaultUrlOptionsController - - def setup - @request.host = 'www.example.com' - rescue_action_in_public! - end - - def test_default_url_options_are_used_if_set - ActionController::Routing::Routes.draw do |map| - map.default_url_options 'default_url_options', :controller => 'default_url_options' - map.connect ':controller/:action/:id' - end - - get :default_url_options_action # Make a dummy request so that the controller is initialized properly. - - assert_equal 'http://www.override.com/default_url_options/new?bacon=chunky', @controller.url_for(:controller => 'default_url_options') - assert_equal 'http://www.override.com/default_url_options?bacon=chunky', @controller.send(:default_url_options_url) - ensure - ActionController::Routing::Routes.load! - end -end - -class EmptyUrlOptionsTest < ActionController::TestCase - tests NonEmptyController - - def setup - @request.host = 'www.example.com' - rescue_action_in_public! - end - - def test_ensure_url_for_works_as_expected_when_called_with_no_options_if_default_url_options_is_not_set - get :public_action - assert_equal "http://www.example.com/non_empty/public_action", @controller.url_for - end -end - -class EnsureNamedRoutesWorksTicket22BugTest < Test::Unit::TestCase - def test_named_routes_still_work - ActionController::Routing::Routes.draw do |map| - map.resources :things - end - EmptyController.send :include, ActionController::UrlWriter - - assert_equal '/things', EmptyController.new.send(:things_path) - ensure - ActionController::Routing::Routes.load! - end -end +end \ No newline at end of file