diff options
22 files changed, 491 insertions, 155 deletions
diff --git a/actionmailer/README.rdoc b/actionmailer/README.rdoc index 59c33b7940..9feb2add5b 100644 --- a/actionmailer/README.rdoc +++ b/actionmailer/README.rdoc @@ -98,7 +98,7 @@ Example: class Mailman < ActionMailer::Base def receive(email) - page = Page.find_by_address(email.to.first) + page = Page.find_by(address: email.to.first) page.emails.create( subject: email.subject, body: email.body ) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index a8325b86fc..4b15c5cb41 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,6 +1,42 @@ ## Rails 4.0.0 (unreleased) ## -* Allow value to be set on date_select tag helper. +* Change the behavior of route defaults so that explicit defaults are no longer + required where the key is not part of the path. For example: + + resources :posts, bucket_type: 'posts' + + will be required whenever constructing the url from a hash such as a functional + test or using url_for directly. However using the explicit form alters the + behavior so it's not required: + + resources :projects, defaults: { bucket_type: 'projects' } + + This changes existing behavior slightly in that any routes which only differ + in their defaults will match the first route rather than the closest match. + + *Andrew White* + +* Add support for routing constraints other than Regexp and String. + For example this now allows the use of arrays like this: + + get '/foo/:action', to: 'foo', constraints: { subdomain: %w[www admin] } + + or constraints where the request method returns an Fixnum like this: + + get '/foo', to: 'foo#index', constraints: { port: 8080 } + + Note that this only applies to constraints on the request - path constraints + still need to be specified as Regexps as the various constraints are compiled + into a single Regexp. + + *Andrew White* + +* Fix a bug in integration tests where setting the port via a url passed to + the process method was ignored when constructing the request environment. + + *Andrew White* + +* Allow `:selected` to be set on `date_select` tag helper. *Colin Burn-Murdoch* diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 13ec55fe92..59b91a240e 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -65,7 +65,6 @@ module ActionController def redirect_to(options = {}, response_status = {}) #:doc: raise ActionControllerError.new("Cannot redirect to nil!") unless options raise AbstractController::DoubleRenderError if response_body - logger.debug { "Redirected by #{caller(1).first rescue "unknown"}" } if logger self.status = _extract_redirect_to_status(options, response_status) self.location = _compute_redirect_to_location(options) diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index cf755bfbeb..82c55660ea 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -1,3 +1,5 @@ +require 'action_controller/metal/exceptions' + module ActionDispatch module Journey # The Formatter class is used for formatting URLs. For example, parameters @@ -27,7 +29,10 @@ module ActionDispatch return [route.format(parameterized_parts), params] end - raise Router::RoutingError.new "missing required keys: #{missing_keys}" + message = "No route matches #{constraints.inspect}" + message << " missing required keys: #{missing_keys.inspect}" if name + + raise ActionController::UrlGenerationError, message end def clear diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index d3988cf31e..063302e0f2 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -1,7 +1,7 @@ module ActionDispatch module Journey # :nodoc: class Route # :nodoc: - attr_reader :app, :path, :verb, :defaults, :ip, :name + attr_reader :app, :path, :defaults, :name attr_reader :constraints alias :conditions :constraints @@ -12,15 +12,11 @@ module ActionDispatch # +path+ is a path constraint. # +constraints+ is a hash of constraints to be applied to this route. def initialize(name, app, path, constraints, defaults = {}) - constraints = constraints.dup @name = name @app = app @path = path - @verb = constraints[:request_method] || // - @ip = constraints.delete(:ip) || // @constraints = constraints - @constraints.keep_if { |_,v| Regexp === v || String === v } @defaults = defaults @required_defaults = nil @required_parts = nil @@ -49,7 +45,7 @@ module ActionDispatch end def required_keys - path.required_names.map { |x| x.to_sym } + required_defaults.keys + required_parts + required_defaults.keys end def score(constraints) @@ -83,12 +79,38 @@ module ActionDispatch @required_parts ||= path.required_names.map { |n| n.to_sym } end + def required_default?(key) + (constraints[:required_defaults] || []).include?(key) + end + def required_defaults - @required_defaults ||= begin - matches = parts - @defaults.dup.delete_if { |k,_| matches.include?(k) } + @required_defaults ||= @defaults.dup.delete_if do |k,_| + parts.include?(k) || !required_default?(k) + end + end + + def matches?(request) + constraints.all? do |method, value| + next true unless request.respond_to?(method) + + case value + when Regexp, String + value === request.send(method).to_s + when Array + value.include?(request.send(method)) + else + value === request.send(method) + end end end + + def ip + constraints[:ip] || // + end + + def verb + constraints[:request_method] || // + end end end end diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index 1fc45a2109..31868b1814 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -131,11 +131,7 @@ module ActionDispatch } routes.concat get_routes_as_head(routes) - routes.sort_by!(&:precedence).select! { |r| - r.constraints.all? { |k, v| v === req.send(k) } && - r.verb === req.request_method - } - routes.reject! { |r| req.ip && !(r.ip === req.ip) } + routes.sort_by!(&:precedence).select! { |r| r.matches?(req) } routes.map! { |r| match_data = r.path.match(req.path_info) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 1046a7d75a..6d93f609a6 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -8,6 +8,8 @@ require 'action_dispatch/routing/redirection' module ActionDispatch module Routing class Mapper + URL_OPTIONS = [:protocol, :subdomain, :domain, :host, :port] + class Constraints #:nodoc: def self.new(app, constraints, request = Rack::Request) if constraints.any? @@ -45,37 +47,68 @@ module ActionDispatch end class Mapping #:nodoc: - IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix] + IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix, :format] ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z} SHORTHAND_REGEX = %r{/[\w/]+$} WILDCARD_PATH = %r{\*([^/\)]+)\)?$} - def initialize(set, scope, path, options) - @set, @scope = set, scope - @segment_keys = nil - @options = (@scope[:options] || {}).merge(options) - @path = normalize_path(path) - normalize_options! + attr_reader :scope, :path, :options, :requirements, :conditions, :defaults - via_all = @options.delete(:via) if @options[:via] == :all + def initialize(set, scope, path, options) + @set, @scope, @path, @options = set, scope, path, options + @requirements, @conditions, @defaults = {}, {}, {} - if !via_all && request_method_condition.empty? - msg = "You should not use the `match` method in your router without specifying an HTTP method.\n" \ - "If you want to expose your action to GET, use `get` in the router:\n\n" \ - " Instead of: match \"controller#action\"\n" \ - " Do: get \"controller#action\"" - raise msg - end + normalize_path! + normalize_options! + normalize_requirements! + normalize_conditions! + normalize_defaults! end def to_route - [ app, conditions, requirements, defaults, @options[:as], @options[:anchor] ] + [ app, conditions, requirements, defaults, options[:as], options[:anchor] ] end private + def normalize_path! + raise ArgumentError, "path is required" if @path.blank? + @path = Mapper.normalize_path(@path) + + if required_format? + @path = "#{@path}.:format" + elsif optional_format? + @path = "#{@path}(.:format)" + end + end + + def required_format? + options[:format] == true + end + + def optional_format? + options[:format] != false && !path.include?(':format') && !path.end_with?('/') + end + def normalize_options! - path_without_format = @path.sub(/\(\.:format\)$/, '') + @options.reverse_merge!(scope[:options]) if scope[:options] + path_without_format = path.sub(/\(\.:format\)$/, '') + + # Add a constraint for wildcard route to make it non-greedy and match the + # optional format part of the route by default + if path_without_format.match(WILDCARD_PATH) && @options[:format] != false + @options[$1.to_sym] ||= /.+?/ + end + + if path_without_format.match(':controller') + raise ArgumentError, ":controller segment is not allowed within a namespace block" if scope[:module] + + # Add a default constraint for :controller path segments that matches namespaced + # controllers with default routes like :controller/:action/:id(.:format), e.g: + # GET /admin/products/show/1 + # => { controller: 'admin/products', action: 'show', id: '1' } + @options[:controller] ||= /.+?/ + end if using_match_shorthand?(path_without_format, @options) to_shorthand = @options[:to].blank? @@ -83,85 +116,101 @@ module ActionDispatch end @options.merge!(default_controller_and_action(to_shorthand)) + end - requirements.each do |name, requirement| - # segment_keys.include?(k.to_s) || k == :controller - next unless Regexp === requirement && !constraints[name] + # match "account/overview" + def using_match_shorthand?(path, options) + path && (options[:to] || options[:action]).nil? && path =~ SHORTHAND_REGEX + end + + def normalize_format! + if options[:format] == true + options[:format] = /.+/ + elsif options[:format] == false + options.delete(:format) + end + end + + def normalize_requirements! + constraints.each do |key, requirement| + next unless segment_keys.include?(key) || key == :controller if requirement.source =~ ANCHOR_CHARACTERS_REGEX raise ArgumentError, "Regexp anchor characters are not allowed in routing requirements: #{requirement.inspect}" end + if requirement.multiline? - raise ArgumentError, "Regexp multiline option not allowed in routing requirements: #{requirement.inspect}" + raise ArgumentError, "Regexp multiline option is not allowed in routing requirements: #{requirement.inspect}" end + + @requirements[key] = requirement end - if @options[:constraints].is_a?(Hash) - (@options[:defaults] ||= {}).reverse_merge!(defaults_from_constraints(@options[:constraints])) + if options[:format] == true + @requirements[:format] = /.+/ + elsif Regexp === options[:format] + @requirements[:format] = options[:format] + elsif String === options[:format] + @requirements[:format] = Regexp.compile(options[:format]) end end - # match "account/overview" - def using_match_shorthand?(path, options) - path && (options[:to] || options[:action]).nil? && path =~ SHORTHAND_REGEX - end + def normalize_defaults! + @defaults.merge!(scope[:defaults]) if scope[:defaults] + @defaults.merge!(options[:defaults]) if options[:defaults] - def normalize_path(path) - raise ArgumentError, "path is required" if path.blank? - path = Mapper.normalize_path(path) + options.each do |key, default| + next if Regexp === default || IGNORE_OPTIONS.include?(key) + @defaults[key] = default + end - if path.match(':controller') - raise ArgumentError, ":controller segment is not allowed within a namespace block" if @scope[:module] + if options[:constraints].is_a?(Hash) + options[:constraints].each do |key, default| + next unless URL_OPTIONS.include?(key) && (String === default || Fixnum === default) + @defaults[key] ||= default + end + end - # Add a default constraint for :controller path segments that matches namespaced - # controllers with default routes like :controller/:action/:id(.:format), e.g: - # GET /admin/products/show/1 - # => { controller: 'admin/products', action: 'show', id: '1' } - @options[:controller] ||= /.+?/ + if Regexp === options[:format] + @defaults[:format] = nil + elsif String === options[:format] + @defaults[:format] = options[:format] end + end - # Add a constraint for wildcard route to make it non-greedy and match the - # optional format part of the route by default - if path.match(WILDCARD_PATH) && @options[:format] != false - @options[$1.to_sym] ||= /.+?/ + def normalize_conditions! + @conditions.merge!(:path_info => path) + + constraints.each do |key, condition| + next if segment_keys.include?(key) || key == :controller + @conditions[key] = condition end - if @options[:format] == false - @options.delete(:format) - path - elsif path.include?(":format") || path.end_with?('/') - path - elsif @options[:format] == true - "#{path}.:format" - else - "#{path}(.:format)" + @conditions[:required_defaults] = [] + options.each do |key, required_default| + next if segment_keys.include?(key) || IGNORE_OPTIONS.include?(key) + next if Regexp === required_default + @conditions[:required_defaults] << key end - end - def app - Constraints.new( - to.respond_to?(:call) ? to : Routing::RouteSet::Dispatcher.new(:defaults => defaults), - blocks, - @set.request_class - ) - end + via_all = options.delete(:via) if options[:via] == :all - def conditions - { :path_info => @path }.merge!(constraints).merge!(request_method_condition) - end + if !via_all && options[:via].blank? + msg = "You should not use the `match` method in your router without specifying an HTTP method.\n" \ + "If you want to expose your action to GET, use `get` in the router:\n\n" \ + " Instead of: match \"controller#action\"\n" \ + " Do: get \"controller#action\"" + raise msg + end - def requirements - @requirements ||= (@options[:constraints].is_a?(Hash) ? @options[:constraints] : {}).tap do |requirements| - requirements.reverse_merge!(@scope[:constraints]) if @scope[:constraints] - @options.each { |k, v| requirements[k] ||= v if v.is_a?(Regexp) } + if via = options[:via] + list = Array(via).map { |m| m.to_s.dasherize.upcase } + @conditions.merge!(:request_method => list) end end - def defaults - @defaults ||= (@options[:defaults] || {}).tap do |defaults| - defaults.reverse_merge!(@scope[:defaults]) if @scope[:defaults] - @options.each { |k, v| defaults[k] = v unless v.is_a?(Regexp) || IGNORE_OPTIONS.include?(k.to_sym) } - end + def app + Constraints.new(endpoint, blocks, @set.request_class) end def default_controller_and_action(to_shorthand=nil) @@ -188,11 +237,11 @@ module ActionDispatch controller = controller.to_s unless controller.is_a?(Regexp) action = action.to_s unless action.is_a?(Regexp) - if controller.blank? && segment_keys.exclude?("controller") + if controller.blank? && segment_keys.exclude?(:controller) raise ArgumentError, "missing :controller" end - if action.blank? && segment_keys.exclude?("action") + if action.blank? && segment_keys.exclude?(:action) raise ArgumentError, "missing :action" end @@ -204,50 +253,55 @@ module ActionDispatch end def blocks - constraints = @options[:constraints] - if constraints.present? && !constraints.is_a?(Hash) - [constraints] + if options[:constraints].present? && !options[:constraints].is_a?(Hash) + [options[:constraints]] else - @scope[:blocks] || [] + scope[:blocks] || [] end end def constraints - @constraints ||= requirements.reject { |k, v| segment_keys.include?(k.to_s) || k == :controller } - end + @constraints ||= {}.tap do |constraints| + constraints.merge!(scope[:constraints]) if scope[:constraints] - def request_method_condition - if via = @options[:via] - list = Array(via).map { |m| m.to_s.dasherize.upcase } - { :request_method => list } - else - { } + options.except(*IGNORE_OPTIONS).each do |key, option| + constraints[key] = option if Regexp === option + end + + constraints.merge!(options[:constraints]) if options[:constraints].is_a?(Hash) end end def segment_keys - return @segment_keys if @segment_keys + @segment_keys ||= path_pattern.names.map{ |s| s.to_sym } + end + + def path_pattern + Journey::Path::Pattern.new(strexp) + end - @segment_keys = Journey::Path::Pattern.new( - Journey::Router::Strexp.compile(@path, requirements, SEPARATORS) - ).names + def strexp + Journey::Router::Strexp.compile(path, requirements, SEPARATORS) + end + + def endpoint + to.respond_to?(:call) ? to : dispatcher + end + + def dispatcher + Routing::RouteSet::Dispatcher.new(:defaults => defaults) end def to - @options[:to] + options[:to] end def default_controller - @options[:controller] || @scope[:controller] + options[:controller] || scope[:controller] end def default_action - @options[:action] || @scope[:action] - end - - def defaults_from_constraints(constraints) - url_keys = [:protocol, :subdomain, :domain, :host, :port] - constraints.select { |k, v| url_keys.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) } + options[:action] || scope[:action] end end @@ -641,7 +695,11 @@ module ActionDispatch options[:constraints] ||= {} if options[:constraints].is_a?(Hash) - (options[:defaults] ||= {}).reverse_merge!(defaults_from_constraints(options[:constraints])) + defaults = options[:constraints].select do + |k, v| URL_OPTIONS.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) + end + + (options[:defaults] ||= {}).reverse_merge!(defaults) else block, options[:constraints] = options[:constraints], {} end @@ -846,11 +904,6 @@ module ActionDispatch def override_keys(child) #:nodoc: child.key?(:only) || child.key?(:except) ? [:only, :except] : [] end - - def defaults_from_constraints(constraints) - url_keys = [:protocol, :subdomain, :domain, :host, :port] - constraints.select { |k, v| url_keys.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) } - end end # Resource routing allows you to quickly declare all of the common routes diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index b1959e388c..c72310cca3 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -421,7 +421,7 @@ module ActionDispatch end conditions.keep_if do |k, _| - k == :action || k == :controller || + k == :action || k == :controller || k == :required_defaults || @request_class.public_method_defined?(k) || path_values.include?(k) end end @@ -527,12 +527,10 @@ module ActionDispatch recall[:action] = options.delete(:action) if options[:action] == 'index' end - # Generates a path from routes, returns [path, params] - # if no path is returned the formatter will raise Journey::Router::RoutingError + # Generates a path from routes, returns [path, params]. + # If no route is generated the formatter will raise ActionController::UrlGenerationError def generate @set.formatter.generate(:path_info, named_route, options, recall, PARAMETERIZE) - rescue Journey::Router::RoutingError => e - raise ActionController::UrlGenerationError, "No route matches #{options.inspect} #{e.message}" end def different_controller? diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 1fc5933e98..ed4e88aab6 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -273,7 +273,7 @@ module ActionDispatch if path =~ %r{://} location = URI.parse(path) https! URI::HTTPS === location if location.scheme - host! location.host if location.host + host! "#{location.host}:#{location.port}" if location.host path = location.query ? "#{location.path}?#{location.query}" : location.path end diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index 1fbf61a5a9..cf978d8e83 100644 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -193,6 +193,7 @@ module ActionView # * <tt>:include_blank</tt> - Include a blank option in every select field so it's possible to set empty # dates. # * <tt>:default</tt> - Set a default date if the affected date isn't set or is nil. + # * <tt>:selected</tt> - Set a date that overrides the actual value. # * <tt>:disabled</tt> - Set to true if you want show the select fields as disabled. # * <tt>:prompt</tt> - Set to true (for a generic prompt), a prompt string or a hash of prompt strings # for <tt>:year</tt>, <tt>:month</tt>, <tt>:day</tt>, <tt>:hour</tt>, <tt>:minute</tt> and <tt>:second</tt>. @@ -234,6 +235,10 @@ module ActionView # # which is initially set to the date 3 days from the current date # date_select("article", "written_on", default: 3.days.from_now) # + # # Generates a date select that when POSTed is stored in the article variable, in the written_on attribute + # # which is set in the form with todays date, regardless of the value in the Active Record object. + # date_select("article", "written_on", selected: Date.today) + # # # Generates a date select that when POSTed is stored in the credit_card variable, in the bill_due attribute # # that will have a default day of 20. # date_select("credit_card", "bill_due", default: { day: 20 }) diff --git a/actionpack/lib/action_view/helpers/tags/date_select.rb b/actionpack/lib/action_view/helpers/tags/date_select.rb index f073c63f73..380d6d3686 100644 --- a/actionpack/lib/action_view/helpers/tags/date_select.rb +++ b/actionpack/lib/action_view/helpers/tags/date_select.rb @@ -27,7 +27,7 @@ module ActionView end def datetime_selector(options, html_options) - datetime = options[:value] || value(object) || default_datetime(options) + datetime = options[:selected] || value(object) || default_datetime(options) @auto_index ||= nil options = options.dup diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index cf561d913a..e2239c05c7 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -466,6 +466,58 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest assert_equal 'http://www.example.com/foo', url_for(:controller => "foo") end + def test_port_via_host! + with_test_route_set do + host! 'www.example.com:8080' + get '/get' + assert_equal 8080, request.port + end + end + + def test_port_via_process + with_test_route_set do + get 'http://www.example.com:8080/get' + assert_equal 8080, request.port + end + end + + def test_https_and_port_via_host_and_https! + with_test_route_set do + host! 'www.example.com' + https! true + + get '/get' + assert_equal 443, request.port + assert_equal true, request.ssl? + + host! 'www.example.com:443' + https! true + + get '/get' + assert_equal 443, request.port + assert_equal true, request.ssl? + + host! 'www.example.com:8443' + https! true + + get '/get' + assert_equal 8443, request.port + assert_equal true, request.ssl? + end + end + + def test_https_and_port_via_process + with_test_route_set do + get 'https://www.example.com/get' + assert_equal 443, request.port + assert_equal true, request.ssl? + + get 'https://www.example.com:8443/get' + assert_equal 8443, request.port + assert_equal true, request.ssl? + end + end + private def with_test_route_set with_routing do |set| diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index e4d78d58b9..df31338f09 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -931,3 +931,34 @@ class AnonymousControllerTest < ActionController::TestCase assert_equal 'anonymous', @response.body end end + +class RoutingDefaultsTest < ActionController::TestCase + def setup + @controller = Class.new(ActionController::Base) do + def post + render :text => request.fullpath + end + + def project + render :text => request.fullpath + end + end.new + + @routes = ActionDispatch::Routing::RouteSet.new.tap do |r| + r.draw do + get '/posts/:id', :to => 'anonymous#post', :bucket_type => 'post' + get '/projects/:id', :to => 'anonymous#project', :defaults => { :bucket_type => 'project' } + end + end + end + + def test_route_option_can_be_passed_via_process + get :post, :id => 1, :bucket_type => 'post' + assert_equal '/posts/1', @response.body + end + + def test_route_default_is_not_required_for_building_request_uri + get :project, :id => 2 + assert_equal '/projects/2', @response.body + end +end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index cb5299e8d3..da7474e73c 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3252,3 +3252,79 @@ class TestOptionalRootSegments < ActionDispatch::IntegrationTest assert_equal '/page/1', root_path(:page => '1') end end + +class TestPortConstraints < ActionDispatch::IntegrationTest + Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| + app.draw do + ok = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, []] } + + get '/integer', to: ok, constraints: { :port => 8080 } + get '/string', to: ok, constraints: { :port => '8080' } + get '/array', to: ok, constraints: { :port => [8080] } + get '/regexp', to: ok, constraints: { :port => /8080/ } + end + end + + include Routes.url_helpers + def app; Routes end + + def test_integer_port_constraints + get 'http://www.example.com/integer' + assert_response :not_found + + get 'http://www.example.com:8080/integer' + assert_response :success + end + + def test_string_port_constraints + get 'http://www.example.com/string' + assert_response :not_found + + get 'http://www.example.com:8080/string' + assert_response :success + end + + def test_array_port_constraints + get 'http://www.example.com/array' + assert_response :not_found + + get 'http://www.example.com:8080/array' + assert_response :success + end + + def test_regexp_port_constraints + get 'http://www.example.com/regexp' + assert_response :not_found + + get 'http://www.example.com:8080/regexp' + assert_response :success + end +end + +class TestRouteDefaults < ActionDispatch::IntegrationTest + stub_controllers do |routes| + Routes = routes + Routes.draw do + resources :posts, bucket_type: 'post' + resources :projects, defaults: { bucket_type: 'project' } + end + end + + def app + Routes + end + + include Routes.url_helpers + + def test_route_options_are_required_for_url_for + assert_raises(ActionController::UrlGenerationError) do + assert_equal '/posts/1', url_for(controller: 'posts', action: 'show', id: 1, only_path: true) + end + + assert_equal '/posts/1', url_for(controller: 'posts', action: 'show', id: 1, bucket_type: 'post', only_path: true) + end + + def test_route_defaults_are_not_required_for_url_for + assert_equal '/projects/1', url_for(controller: 'projects', action: 'show', id: 1, only_path: true) + end +end diff --git a/actionpack/test/journey/route_test.rb b/actionpack/test/journey/route_test.rb index 78608a5c6b..cbe6284714 100644 --- a/actionpack/test/journey/route_test.rb +++ b/actionpack/test/journey/route_test.rb @@ -6,18 +6,18 @@ module ActionDispatch def test_initialize app = Object.new path = Path::Pattern.new '/:controller(/:action(/:id(.:format)))' - defaults = Object.new + defaults = {} route = Route.new("name", app, path, {}, defaults) assert_equal app, route.app assert_equal path, route.path - assert_equal defaults, route.defaults + assert_same defaults, route.defaults end def test_route_adds_itself_as_memo app = Object.new path = Path::Pattern.new '/:controller(/:action(/:id(.:format)))' - defaults = Object.new + defaults = {} route = Route.new("name", app, path, {}, defaults) route.ast.grep(Nodes::Terminal).each do |node| @@ -82,11 +82,14 @@ module ActionDispatch end def test_score + constraints = {:required_defaults => [:controller, :action]} + defaults = {:controller=>"pages", :action=>"show"} + path = Path::Pattern.new "/page/:id(/:action)(.:format)" - specific = Route.new "name", nil, path, {}, {:controller=>"pages", :action=>"show"} + specific = Route.new "name", nil, path, constraints, defaults path = Path::Pattern.new "/:controller(/:action(/:id))(.:format)" - generic = Route.new "name", nil, path, {} + generic = Route.new "name", nil, path, constraints knowledge = {:id=>20, :controller=>"pages", :action=>"show"} diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 27bdb0108a..3d52b2e9ee 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -155,7 +155,7 @@ module ActionDispatch Router::Strexp.new("/foo/:id", { :id => /\d/ }, ['/', '.', '?'], false) ] - assert_raises(Router::RoutingError) do + assert_raises(ActionController::UrlGenerationError) do @formatter.generate(:path_info, nil, { :id => '10' }, { }) end end @@ -168,7 +168,7 @@ module ActionDispatch path, _ = @formatter.generate(:path_info, nil, { :id => '10' }, { }) assert_equal '/foo/10', path - assert_raises(Router::RoutingError) do + assert_raises(ActionController::UrlGenerationError) do @formatter.generate(:path_info, nil, { :id => 'aa' }, { }) end end @@ -194,11 +194,11 @@ module ActionDispatch path = Path::Pattern.new pattern @router.routes.add_route nil, path, {}, {}, route_name - error = assert_raises(Router::RoutingError) do + error = assert_raises(ActionController::UrlGenerationError) do @formatter.generate(:path_info, route_name, { }, { }) end - assert_match(/required keys: \[:id\]/, error.message) + assert_match(/missing required keys: \[:id\]/, error.message) end def test_X_Cascade diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb index 0ab5c15131..d622392caf 100644 --- a/actionpack/test/template/date_helper_test.rb +++ b/actionpack/test/template/date_helper_test.rb @@ -1511,7 +1511,7 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, date_select("post", "written_on") end - def test_date_select_with_value + def test_date_select_with_selected @post = Post.new @post.written_on = Date.new(2004, 6, 15) @@ -1528,7 +1528,7 @@ class DateHelperTest < ActionView::TestCase expected << "</select>\n" - assert_dom_equal expected, date_select("post", "written_on", :value => '2004-07-10'.to_date) + assert_dom_equal expected, date_select("post", "written_on", :selected => Date.new(2004, 07, 10)) end @@ -1989,6 +1989,25 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, time_select("post", "written_on") end + def test_time_select_with_selected + @post = Post.new + @post.written_on = Time.local(2004, 6, 15, 15, 16, 35) + + expected = %{<input type="hidden" id="post_written_on_1i" name="post[written_on(1i)]" value="2004" />\n} + expected << %{<input type="hidden" id="post_written_on_2i" name="post[written_on(2i)]" value="6" />\n} + expected << %{<input type="hidden" id="post_written_on_3i" name="post[written_on(3i)]" value="15" />\n} + + expected << %(<select id="post_written_on_4i" name="post[written_on(4i)]">\n) + 0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 12}>#{sprintf("%02d", i)}</option>\n) } + expected << "</select>\n" + expected << " : " + expected << %(<select id="post_written_on_5i" name="post[written_on(5i)]">\n) + 0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 20}>#{sprintf("%02d", i)}</option>\n) } + expected << "</select>\n" + + assert_dom_equal expected, time_select("post", "written_on", :selected => Time.local(2004, 6, 15, 12, 20, 30)) + end + def test_time_select_without_date_hidden_fields @post = Post.new @post.written_on = Time.local(2004, 6, 15, 15, 16, 35) @@ -2186,6 +2205,35 @@ class DateHelperTest < ActionView::TestCase assert_dom_equal expected, datetime_select("post", "updated_at") end + def test_datetime_select_with_selected + @post = Post.new + @post.updated_at = Time.local(2004, 6, 15, 16, 35) + + expected = %{<select id="post_updated_at_1i" name="post[updated_at(1i)]">\n} + expected << %{<option value="1999">1999</option>\n<option value="2000">2000</option>\n<option value="2001">2001</option>\n<option value="2002">2002</option>\n<option value="2003">2003</option>\n<option value="2004" selected="selected">2004</option>\n<option value="2005">2005</option>\n<option value="2006">2006</option>\n<option value="2007">2007</option>\n<option value="2008">2008</option>\n<option value="2009">2009</option>\n} + expected << "</select>\n" + + expected << %{<select id="post_updated_at_2i" name="post[updated_at(2i)]">\n} + expected << %{<option value="1">January</option>\n<option value="2">February</option>\n<option value="3" selected="selected">March</option>\n<option value="4">April</option>\n<option value="5">May</option>\n<option value="6">June</option>\n<option value="7">July</option>\n<option value="8">August</option>\n<option value="9">September</option>\n<option value="10">October</option>\n<option value="11">November</option>\n<option value="12">December</option>\n} + expected << "</select>\n" + + expected << %{<select id="post_updated_at_3i" name="post[updated_at(3i)]">\n} + expected << %{<option value="1">1</option>\n<option value="2">2</option>\n<option value="3">3</option>\n<option value="4">4</option>\n<option value="5">5</option>\n<option value="6">6</option>\n<option value="7">7</option>\n<option value="8">8</option>\n<option value="9">9</option>\n<option value="10" selected="selected">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15">15</option>\n<option value="16">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n<option value="24">24</option>\n<option value="25">25</option>\n<option value="26">26</option>\n<option value="27">27</option>\n<option value="28">28</option>\n<option value="29">29</option>\n<option value="30">30</option>\n<option value="31">31</option>\n} + expected << "</select>\n" + + expected << " — " + + expected << %{<select id="post_updated_at_4i" name="post[updated_at(4i)]">\n} + expected << %{<option value="00">00</option>\n<option value="01">01</option>\n<option value="02">02</option>\n<option value="03">03</option>\n<option value="04">04</option>\n<option value="05">05</option>\n<option value="06">06</option>\n<option value="07">07</option>\n<option value="08">08</option>\n<option value="09">09</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12" selected="selected">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15">15</option>\n<option value="16">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n} + expected << "</select>\n" + expected << " : " + expected << %{<select id="post_updated_at_5i" name="post[updated_at(5i)]">\n} + expected << %{<option value="00">00</option>\n<option value="01">01</option>\n<option value="02">02</option>\n<option value="03">03</option>\n<option value="04">04</option>\n<option value="05">05</option>\n<option value="06">06</option>\n<option value="07">07</option>\n<option value="08">08</option>\n<option value="09">09</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15">15</option>\n<option value="16">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n<option value="24">24</option>\n<option value="25">25</option>\n<option value="26">26</option>\n<option value="27">27</option>\n<option value="28">28</option>\n<option value="29">29</option>\n<option value="30" selected="selected">30</option>\n<option value="31">31</option>\n<option value="32">32</option>\n<option value="33">33</option>\n<option value="34">34</option>\n<option value="35">35</option>\n<option value="36">36</option>\n<option value="37">37</option>\n<option value="38">38</option>\n<option value="39">39</option>\n<option value="40">40</option>\n<option value="41">41</option>\n<option value="42">42</option>\n<option value="43">43</option>\n<option value="44">44</option>\n<option value="45">45</option>\n<option value="46">46</option>\n<option value="47">47</option>\n<option value="48">48</option>\n<option value="49">49</option>\n<option value="50">50</option>\n<option value="51">51</option>\n<option value="52">52</option>\n<option value="53">53</option>\n<option value="54">54</option>\n<option value="55">55</option>\n<option value="56">56</option>\n<option value="57">57</option>\n<option value="58">58</option>\n<option value="59">59</option>\n} + expected << "</select>\n" + + assert_dom_equal expected, datetime_select("post", "updated_at", :selected => Time.local(2004, 3, 10, 12, 30)) + end + def test_datetime_select_defaults_to_time_zone_now_when_config_time_zone_is_set # The love zone is UTC+0 mytz = Class.new(ActiveSupport::TimeZone) { diff --git a/activemodel/README.rdoc b/activemodel/README.rdoc index 3cc2d9ad8e..1b1fe2fa2b 100644 --- a/activemodel/README.rdoc +++ b/activemodel/README.rdoc @@ -1,7 +1,7 @@ = Active Model -- model interfaces for Rails Active Model provides a known set of interfaces for usage in model classes. -They allow for Action Pack helpers to interact with non-ActiveRecord models, +They allow for Action Pack helpers to interact with non-Active Record models, for example. Active Model also helps building custom ORMs for use outside of the Rails framework. @@ -11,7 +11,7 @@ code from Rails, or monkey patch entire helpers to make them handle objects that did not exactly conform to the Active Record interface. This would result in code duplication and fragile applications that broke on upgrades. Active Model solves this by defining an explicit API. You can read more about the -API in ActiveModel::Lint::Tests. +API in <tt>ActiveModel::Lint::Tests</tt>. Active Model provides a default module that implements the basic API required to integrate with Action Pack out of the box: <tt>ActiveModel::Model</tt>. diff --git a/activemodel/test/cases/conversion_test.rb b/activemodel/test/cases/conversion_test.rb index d679ad41aa..a037666cbc 100644 --- a/activemodel/test/cases/conversion_test.rb +++ b/activemodel/test/cases/conversion_test.rb @@ -29,4 +29,8 @@ class ConversionTest < ActiveModel::TestCase assert_equal "helicopters/helicopter", Helicopter.new.to_partial_path, "ActiveModel::Conversion#to_partial_path caching should be class-specific" end + + test "to_partial_path handles namespaced models" do + assert_equal "helicopter/comanches/comanche", Helicopter::Comanche.new.to_partial_path + end end diff --git a/activemodel/test/models/helicopter.rb b/activemodel/test/models/helicopter.rb index a52b6fb4dd..933f3c463a 100644 --- a/activemodel/test/models/helicopter.rb +++ b/activemodel/test/models/helicopter.rb @@ -1,3 +1,7 @@ class Helicopter include ActiveModel::Conversion end + +class Helicopter::Comanche + include ActiveModel::Conversion +end diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index 1b4ed1cac4..17378969a5 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -15,18 +15,20 @@ module ActiveRecord establish_connection configuration_without_database connection.create_database configuration['database'], creation_options establish_connection configuration - rescue error_class => error - raise error unless error.errno == ACCESS_DENIED_ERROR - - $stdout.print error.error - establish_connection root_configuration_without_database - connection.create_database configuration['database'], creation_options - connection.execute grant_statement.gsub(/\s+/, ' ').strip - establish_connection configuration - rescue ActiveRecord::StatementInvalid, error_class => error + rescue ActiveRecord::StatementInvalid => error if /database exists/ === error.message raise DatabaseAlreadyExists else + raise + end + rescue error_class => error + if error.respond_to?(:errno) && error.errno == ACCESS_DENIED_ERROR + $stdout.print error.error + establish_connection root_configuration_without_database + connection.create_database configuration['database'], creation_options + connection.execute grant_statement.gsub(/\s+/, ' ').strip + establish_connection configuration + else $stderr.puts "Couldn't create database for #{configuration.inspect}, #{creation_options.inspect}" $stderr.puts "(If you set the charset manually, make sure you have a matching collation)" if configuration['encoding'] end @@ -96,6 +98,8 @@ module ActiveRecord Mysql2::Error elsif defined?(Mysql) Mysql::Error + else + StandardError end end diff --git a/railties/test/application/middleware/cache_test.rb b/railties/test/application/middleware/cache_test.rb index c99666d7e4..b8e0c9be60 100644 --- a/railties/test/application/middleware/cache_test.rb +++ b/railties/test/application/middleware/cache_test.rb @@ -81,8 +81,8 @@ module ApplicationTests add_to_config "config.action_dispatch.rack_cache = true" get "/expires/expires_header" - assert_equal "miss, store", last_response.headers["X-Rack-Cache"] - assert_equal "max-age=10, public", last_response.headers["Cache-Control"] + assert_equal "miss, ignore, store", last_response.headers["X-Rack-Cache"] + assert_equal "max-age=10, public", last_response.headers["Cache-Control"] body = last_response.body @@ -115,8 +115,8 @@ module ApplicationTests add_to_config "config.action_dispatch.rack_cache = true" get "/expires/expires_etag" - assert_equal "miss, store", last_response.headers["X-Rack-Cache"] - assert_equal "public", last_response.headers["Cache-Control"] + assert_equal "miss, ignore, store", last_response.headers["X-Rack-Cache"] + assert_equal "public", last_response.headers["Cache-Control"] body = last_response.body etag = last_response.headers["ETag"] @@ -149,8 +149,8 @@ module ApplicationTests add_to_config "config.action_dispatch.rack_cache = true" get "/expires/expires_last_modified" - assert_equal "miss, store", last_response.headers["X-Rack-Cache"] - assert_equal "public", last_response.headers["Cache-Control"] + assert_equal "miss, ignore, store", last_response.headers["X-Rack-Cache"] + assert_equal "public", last_response.headers["Cache-Control"] body = last_response.body last = last_response.headers["Last-Modified"] |