diff options
31 files changed, 331 insertions, 250 deletions
diff --git a/.gitignore b/.gitignore index aa14cee911..52a431867c 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ # Check out http://help.github.com/ignore-files/ for how to set that up. debug.log +.Gemfile /.bundle /.rbenv-version /.rvmrc @@ -36,13 +36,11 @@ gem "memcache-client", ">= 1.8.5" platforms :mri_18 do gem "system_timer" - gem "ruby-debug", ">= 0.10.3" unless ENV['TRAVIS'] gem "json" end -platforms :mri_19 do - gem "ruby-debug19", :require => "ruby-debug" unless ENV['TRAVIS'] || RUBY_VERSION >= "2.0" -end +# Add your own local bundler stuff +instance_eval File.read ".Gemfile" if File.exists? ".Gemfile" platforms :mri do group :test do @@ -69,7 +67,6 @@ platforms :ruby do end platforms :jruby do - gem "ruby-debug", ">= 0.10.3" gem "json" gem "activerecord-jdbcsqlite3-adapter", ">= 1.2.0" diff --git a/RELEASING_RAILS.rdoc b/RELEASING_RAILS.rdoc index 1dfcfe2488..3f05e97b91 100644 --- a/RELEASING_RAILS.rdoc +++ b/RELEASING_RAILS.rdoc @@ -205,3 +205,34 @@ There are two simple steps for fixing the CI: 2. Fix it Repeat these steps until the CI is green. + +=== Manually trigger docs generation + +We have a post-receive hook in GitHub that calls the docs server on pushes. +Triggers generation and publication of edge docs, updates the contrib app, +and generates and publishes stable docs if a new stable tag is detected. + +The hook unfortunately is not invoked by tag pushing, so once the new stable +tag has been pushed to origin, please run + + curl -X POST -d '' http://rails-hooks.hashref.com/rails-master-hook + +You should see something like this: + + Rails master hook tasks scheduled: + + * updates the local checkout + * updates Rails Contributors + * generates and publishes edge docs + + If a new stable tag is detected it also + + * generates and publishes stable docs + + This needs typically a few minutes. + +Note you do not need to specify the tag, the docs server figures it out. + +Also, don't worry if you call that multiple times or the hook is triggered +again by some immediate regular push, if the scripts are running new calls +are just queued (in a queue of size 1). diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 4c265c41d8..dc52262503 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -62,8 +62,29 @@ persistent between requests so if you need to manipulate the environment for your test you need to do it before the cookie jar is created. +## Rails 3.1.3 (unreleased) ## + +* Fix using `tranlate` helper with a html translation which uses the `:count` option for + pluralization. + + *Jon Leighton* + ## Rails 3.1.2 (unreleased) ## +* Fix XSS security vulnerability in the `translate` helper method. When using interpolation + in combination with HTML-safe translations, the interpolated input would not get HTML + escaped. *GH 3664* + + Before: + + translate('foo_html', :something => '<script>') # => "...<script>..." + + After: + + translate('foo_html', :something => '<script>') # => "...<script>..." + + *Sergey Nartimov* + * Upgrade sprockets dependency to ~> 2.1.0 * Ensure that the format isn't applied twice to the cache key, else it becomes impossible diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 9da9143de3..41bada43ef 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -24,7 +24,7 @@ Gem::Specification.new do |s| s.add_dependency('rack', '~> 1.3.5') s.add_dependency('rack-test', '~> 0.6.1') s.add_dependency('journey', '~> 1.0.0') - s.add_dependency('sprockets', '~> 2.1.0') + s.add_dependency('sprockets', '~> 2.1.1') s.add_dependency('erubis', '~> 2.7.0') s.add_development_dependency('tzinfo', '~> 0.3.29') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 970236a05a..e3ad3f9ba7 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -374,10 +374,6 @@ module ActionDispatch # # Matches any request starting with 'path' # match 'path' => 'c#a', :anchor => false def match(path, options=nil) - mapping = Mapping.new(@set, @scope, path, options || {}) - app, conditions, requirements, defaults, as, anchor = mapping.to_route - @set.add_route(app, conditions, requirements, defaults, as, anchor) - self end # Mount a Rack-based application to be used within the application. @@ -607,9 +603,12 @@ module ActionDispatch options[:constraints] ||= {} unless options[:constraints].is_a?(Hash) - block, options[:constraints] = options[:constraints], {} + options[:blocks] = options[:constraints] + options[:constraints] = {} end + options[:options] = options + scope_options.each do |option| if value = options.delete(option) recover[option] = @scope[option] @@ -617,21 +616,12 @@ module ActionDispatch end end - recover[:block] = @scope[:blocks] - @scope[:blocks] = merge_blocks_scope(@scope[:blocks], block) - - recover[:options] = @scope[:options] - @scope[:options] = merge_options_scope(@scope[:options], options) - yield self ensure scope_options.each do |option| @scope[option] = recover[option] if recover.has_key?(option) end - - @scope[:options] = recover[:options] - @scope[:blocks] = recover[:block] end # Scopes routes to a specific controller @@ -867,8 +857,6 @@ module ActionDispatch CANONICAL_ACTIONS = %w(index create new show update destroy) class Resource #:nodoc: - DEFAULT_ACTIONS = [:index, :create, :new, :show, :update, :destroy, :edit] - attr_reader :controller, :path, :options def initialize(entities, options = {}) @@ -880,7 +868,7 @@ module ActionDispatch end def default_actions - self.class::DEFAULT_ACTIONS + [:index, :create, :new, :show, :update, :destroy, :edit] end def actions @@ -934,16 +922,17 @@ module ActionDispatch end class SingletonResource < Resource #:nodoc: - DEFAULT_ACTIONS = [:show, :create, :update, :destroy, :new, :edit] - def initialize(entities, options) super - @as = nil @controller = (options[:controller] || plural).to_s @as = options[:as] end + def default_actions + [:show, :create, :update, :destroy, :new, :edit] + end + def plural @plural ||= name.to_s.pluralize end @@ -991,7 +980,7 @@ module ActionDispatch return self end - resource_scope(SingletonResource.new(resources.pop, options)) do + resource_scope(:resource, SingletonResource.new(resources.pop, options)) do yield if block_given? collection do @@ -1122,7 +1111,7 @@ module ActionDispatch return self end - resource_scope(Resource.new(resources.pop, options)) do + resource_scope(:resources, Resource.new(resources.pop, options)) do yield if block_given? collection do @@ -1245,32 +1234,44 @@ module ActionDispatch parent_resource.instance_of?(Resource) && @scope[:shallow] end - def match(*args) - options = args.extract_options!.dup - options[:anchor] = true unless options.key?(:anchor) - - if args.length > 1 - args.each { |path| match(path, options.dup) } - return self + def match(path, *rest) + if rest.empty? && Hash === path + options = path + path, to = options.find { |name, value| name.is_a?(String) } + options[:to] = to + options.delete(path) + paths = [path] + else + options = rest.pop || {} + paths = [path] + rest end - on = options.delete(:on) - if VALID_ON_OPTIONS.include?(on) - args.push(options) - return send(on){ match(*args) } - elsif on + options[:anchor] = true unless options.key?(:anchor) + + if options[:on] && !VALID_ON_OPTIONS.include?(options[:on]) raise ArgumentError, "Unknown scope #{on.inspect} given to :on" end - if @scope[:scope_level] == :resources - args.push(options) - return nested { match(*args) } - elsif @scope[:scope_level] == :resource - args.push(options) - return member { match(*args) } + paths.each { |_path| decomposed_match(_path, options.dup) } + self + end + + def decomposed_match(path, options) # :nodoc: + if on = options.delete(:on) + send(on) { decomposed_match(path, options) } + else + case @scope[:scope_level] + when :resources + nested { decomposed_match(path, options) } + when :resource + member { decomposed_match(path, options) } + else + add_route(path, options) + end end + end - action = args.first + def add_route(action, options) # :nodoc: path = path_for_action(action, options.delete(:path)) if action.to_s =~ /^[\w\/]+$/ @@ -1279,13 +1280,15 @@ module ActionDispatch action = nil end - if options.key?(:as) && !options[:as] + if !options.fetch(:as) { true } options.delete(:as) else options[:as] = name_for_action(options[:as], action) end - super(path, options) + mapping = Mapping.new(@set, @scope, path, options) + app, conditions, requirements, defaults, as, anchor = mapping.to_route + @set.add_route(app, conditions, requirements, defaults, as, anchor) end def root(options={}) @@ -1341,7 +1344,7 @@ module ActionDispatch end def scope_action_options? #:nodoc: - @scope[:options].is_a?(Hash) && (@scope[:options][:only] || @scope[:options][:except]) + @scope[:options] && (@scope[:options][:only] || @scope[:options][:except]) end def scope_action_options #:nodoc: @@ -1349,11 +1352,11 @@ module ActionDispatch end def resource_scope? #:nodoc: - @scope[:scope_level].in?([:resource, :resources]) + [:resource, :resources].include? @scope[:scope_level] end def resource_method_scope? #:nodoc: - @scope[:scope_level].in?([:collection, :member, :new]) + [:collection, :member, :new].include? @scope[:scope_level] end def with_exclusive_scope @@ -1378,8 +1381,8 @@ module ActionDispatch @scope[:scope_level_resource] = old_resource end - def resource_scope(resource) #:nodoc: - with_scope_level(resource.is_a?(SingletonResource) ? :resource : :resources, resource) do + def resource_scope(kind, resource) #:nodoc: + with_scope_level(kind, resource) do scope(parent_resource.resource_scope) do yield end @@ -1387,10 +1390,12 @@ module ActionDispatch end def nested_options #:nodoc: - {}.tap do |options| - options[:as] = parent_resource.member_name - options[:constraints] = { "#{parent_resource.singular}_id".to_sym => id_constraint } if id_constraint? - end + options = { :as => parent_resource.member_name } + options[:constraints] = { + :"#{parent_resource.singular}_id" => id_constraint + } if id_constraint? + + options end def id_constraint? #:nodoc: @@ -1466,19 +1471,6 @@ module ActionDispatch end end - module Shorthand #:nodoc: - def match(path, *rest) - if rest.empty? && Hash === path - options = path - path, to = options.find { |name, value| name.is_a?(String) } - options.merge!(:to => to).delete(path) - super(path, options) - else - super - end - end - end - def initialize(set) #:nodoc: @set = set @scope = { :path_names => @set.resources_path_names } @@ -1489,7 +1481,6 @@ module ActionDispatch include Redirection include Scoping include Resources - include Shorthand end end end diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index 804991ad5f..330400e139 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -2,6 +2,54 @@ require 'action_dispatch/http/request' module ActionDispatch module Routing + class Redirect # :nodoc: + attr_reader :status, :block + + def initialize(status, block) + @status = status + @block = block + end + + def call(env) + req = Request.new(env) + + uri = URI.parse(path(req.symbolized_path_parameters, req)) + uri.scheme ||= req.scheme + uri.host ||= req.host + uri.port ||= req.port unless req.standard_port? + + body = %(<html><body>You are being <a href="#{ERB::Util.h(uri.to_s)}">redirected</a>.</body></html>) + + headers = { + 'Location' => uri.to_s, + 'Content-Type' => 'text/html', + 'Content-Length' => body.length.to_s + } + + [ status, headers, [body] ] + end + + def path(params, request) + block.call params, request + end + end + + class OptionRedirect < Redirect # :nodoc: + alias :options :block + + def path(params, request) + url_options = { + :protocol => request.protocol, + :host => request.host, + :port => request.optional_port, + :path => request.path, + :params => request.query_parameters + }.merge options + + ActionDispatch::Http::URL.url_for url_options + end + end + module Redirection # Redirect any path to another path: @@ -40,71 +88,27 @@ module ActionDispatch options = args.last.is_a?(Hash) ? args.pop : {} status = options.delete(:status) || 301 + return OptionRedirect.new(status, options) if options.any? + path = args.shift - path_proc = if path.is_a?(String) - proc { |params| (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) } - elsif options.any? - options_proc(options) - elsif path.respond_to?(:call) - proc { |params, request| path.call(params, request) } - elsif block - block - else - raise ArgumentError, "redirection argument not supported" - end + block = lambda { |params, request| + (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) + } if String === path - redirection_proc(status, path_proc) - end + block = path if path.respond_to? :call - private - - def options_proc(options) - proc do |params, request| - path = if options[:path].nil? - request.path - elsif params.empty? || !options[:path].match(/%\{\w*\}/) - options.delete(:path) - else - (options.delete(:path) % params) - end - - default_options = { - :protocol => request.protocol, - :host => request.host, - :port => request.optional_port, - :path => path, - :params => request.query_parameters - } - - ActionDispatch::Http::URL.url_for(options.reverse_merge(default_options)) - end + # :FIXME: remove in Rails 4.0 + if block && block.respond_to?(:arity) && block.arity < 2 + msg = "redirect blocks with arity of #{block.arity} are deprecated. Your block must take 2 parameters: the environment, and a request object" + ActiveSupport::Deprecation.warn msg + block = lambda { |params, _| block.call(params) } end - def redirection_proc(status, path_proc) - lambda do |env| - req = Request.new(env) - - params = [req.symbolized_path_parameters] - params << req if path_proc.arity > 1 - - uri = URI.parse(path_proc.call(*params)) - uri.scheme ||= req.scheme - uri.host ||= req.host - uri.port ||= req.port unless req.standard_port? - - body = %(<html><body>You are being <a href="#{ERB::Util.h(uri.to_s)}">redirected</a>.</body></html>) - - headers = { - 'Location' => uri.to_s, - 'Content-Type' => 'text/html', - 'Content-Length' => body.length.to_s - } - - [ status, headers, [body] ] - end - end + raise ArgumentError, "redirection argument not supported" unless block + Redirect.new status, block + end end end -end
\ No newline at end of file +end diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb index e8ad043e20..b37e837a23 100644 --- a/actionpack/lib/action_view/helpers/number_helper.rb +++ b/actionpack/lib/action_view/helpers/number_helper.rb @@ -193,7 +193,8 @@ module ActionView # number_with_delimiter(12345678) # => 12,345,678 # number_with_delimiter(12345678.05) # => 12,345,678.05 # number_with_delimiter(12345678, :delimiter => ".") # => 12.345.678 - # number_with_delimiter(12345678, :separator => ",") # => 12,345,678 + # number_with_delimiter(12345678, :delimiter => ",") # => 12,345,678 + # number_with_delimiter(12345678.05, :separator => " ") # => 12,345,678 05 # number_with_delimiter(12345678.05, :locale => :fr) # => 12 345 678,05 # number_with_delimiter(98765432.98, :delimiter => " ", :separator => ",") # # => 98 765 432,98 diff --git a/actionpack/lib/action_view/helpers/translation_helper.rb b/actionpack/lib/action_view/helpers/translation_helper.rb index be64dc823e..cc74eff53a 100644 --- a/actionpack/lib/action_view/helpers/translation_helper.rb +++ b/actionpack/lib/action_view/helpers/translation_helper.rb @@ -45,11 +45,18 @@ module ActionView # you know what kind of output to expect when you call translate in a template. def translate(key, options = {}) options.merge!(:rescue_format => :html) unless options.key?(:rescue_format) - translation = I18n.translate(scope_key_by_partial(key), options) - if html_safe_translation_key?(key) && translation.respond_to?(:html_safe) - translation.html_safe + if html_safe_translation_key?(key) + html_safe_options = options.dup + options.except(*I18n::RESERVED_KEYS).each do |name, value| + unless name == :count && value.is_a?(Numeric) + html_safe_options[name] = ERB::Util.html_escape(value.to_s) + end + end + translation = I18n.translate(scope_key_by_partial(key), html_safe_options) + + translation.respond_to?(:html_safe) ? translation.html_safe : translation else - translation + I18n.translate(scope_key_by_partial(key), options) end end alias :t :translate diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb index 15cb9d0f76..54a0ba96ff 100644 --- a/actionpack/lib/action_view/renderer/partial_renderer.rb +++ b/actionpack/lib/action_view/renderer/partial_renderer.rb @@ -366,8 +366,13 @@ module ActionView path = if object.respond_to?(:to_partial_path) object.to_partial_path else - ActiveSupport::Deprecation.warn "ActiveModel-compatible objects whose classes return a #model_name that responds to #partial_path are deprecated. Please respond to #to_partial_path directly instead." - object.class.model_name.partial_path + klass = object.class + if klass.respond_to?(:model_name) + ActiveSupport::Deprecation.warn "ActiveModel-compatible objects whose classes return a #model_name that responds to #partial_path are deprecated. Please respond to #to_partial_path directly instead." + klass.model_name.partial_path + else + raise ArgumentError.new("'#{object.inspect}' is not an ActiveModel-compatible object that returns a valid partial path.") + end end @partial_names[path] ||= path.dup.tap do |object_path| diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index cf22731823..19eee379fd 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -62,13 +62,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest match 'secure', :to => redirect("/secure/login") match 'mobile', :to => redirect(:subdomain => 'mobile') - match 'documentation', :to => redirect(:domain => 'example-documentation.com', :path => '') - match 'new_documentation', :to => redirect(:path => '/documentation/new') match 'super_new_documentation', :to => redirect(:host => 'super-docs.com') - match 'stores/:name', :to => redirect(:subdomain => 'stores', :path => '/%{name}') - match 'stores/:name(*rest)', :to => redirect(:subdomain => 'stores', :path => '/%{name}%{rest}') - match 'youtube_favorites/:youtube_id/:name', :to => redirect(YoutubeFavoritesRedirector) constraints(lambda { |req| true }) do @@ -79,7 +74,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest match 'sign_in' => "sessions#new" match 'account/modulo/:name', :to => redirect("/%{name}s") - match 'account/proc/:name', :to => redirect {|params| "/#{params[:name].pluralize}" } + match 'account/proc/:name', :to => redirect {|params, req| "/#{params[:name].pluralize}" } match 'account/proc_req' => redirect {|params, req| "/#{req.method}" } match 'account/google' => redirect('http://www.google.com/', :status => 302) @@ -711,20 +706,6 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - def test_redirect_hash_with_domain_and_path - with_test_routes do - get '/documentation' - verify_redirect 'http://www.example-documentation.com' - end - end - - def test_redirect_hash_with_path - with_test_routes do - get '/new_documentation' - verify_redirect 'http://www.example.com/documentation/new' - end - end - def test_redirect_hash_with_host with_test_routes do get '/super_new_documentation?section=top' @@ -732,20 +713,6 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - def test_redirect_hash_path_substitution - with_test_routes do - get '/stores/iernest' - verify_redirect 'http://stores.example.com/iernest' - end - end - - def test_redirect_hash_path_substitution_with_catch_all - with_test_routes do - get '/stores/iernest/products' - verify_redirect 'http://stores.example.com/iernest/products' - end - end - def test_redirect_class with_test_routes do get '/youtube_favorites/oHg5SJYRHA0/rick-rolld' @@ -2332,6 +2299,11 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal "/forced_collision", routes_forced_collision_path end + def test_redirect_argument_error + routes = Class.new { include ActionDispatch::Routing::Redirection }.new + assert_raises(ArgumentError) { routes.redirect Object.new } + end + def test_explicitly_avoiding_the_named_route assert !respond_to?(:routes_no_collision_path) end diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 77659918f7..c29519276d 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -142,6 +142,13 @@ module RenderTestCases "and is followed by any combinations of letters, numbers, or underscores.", e.message end + def test_render_partial_with_incompatible_object + @view.render(:partial => nil) + flunk "Render did not raise ArgumentError" + rescue ArgumentError => e + assert_equal "'#{nil.inspect}' is not an ActiveModel-compatible object that returns a valid partial path.", e.message + end + def test_render_partial_with_errors @view.render(:partial => "test/raise") flunk "Render did not raise Template::Error" diff --git a/actionpack/test/template/translation_helper_test.rb b/actionpack/test/template/translation_helper_test.rb index cd9f54e04c..397de9c2ce 100644 --- a/actionpack/test/template/translation_helper_test.rb +++ b/actionpack/test/template/translation_helper_test.rb @@ -17,8 +17,13 @@ class TranslationHelperTest < ActiveSupport::TestCase :hello => '<a>Hello World</a>', :html => '<a>Hello World</a>', :hello_html => '<a>Hello World</a>', + :interpolated_html => '<a>Hello %{word}</a>', :array_html => %w(foo bar), - :array => %w(foo bar) + :array => %w(foo bar), + :count_html => { + :one => '<a>One %{count}</a>', + :other => '<a>Other %{count}</a>' + } } ) @view = ::ActionView::Base.new(ActionController::Base.view_paths, {}) @@ -83,6 +88,17 @@ class TranslationHelperTest < ActiveSupport::TestCase assert translate(:'translations.hello_html').html_safe? end + def test_translate_escapes_interpolations_in_translations_with_a_html_suffix + assert_equal '<a>Hello <World></a>', translate(:'translations.interpolated_html', :word => '<World>') + assert_equal '<a>Hello <World></a>', translate(:'translations.interpolated_html', :word => stub(:to_s => "<World>")) + end + + def test_translate_with_html_count + assert_equal '<a>One 1</a>', translate(:'translations.count_html', :count => 1) + assert_equal '<a>Other 2</a>', translate(:'translations.count_html', :count => 2) + assert_equal '<a>Other <One></a>', translate(:'translations.count_html', :count => '<One>') + end + def test_translation_returning_an_array_ignores_html_suffix assert_equal ["foo", "bar"], translate(:'translations.array_html') end diff --git a/activemodel/test/cases/validations/exclusion_validation_test.rb b/activemodel/test/cases/validations/exclusion_validation_test.rb index 72a383f128..adab8ccb2b 100644 --- a/activemodel/test/cases/validations/exclusion_validation_test.rb +++ b/activemodel/test/cases/validations/exclusion_validation_test.rb @@ -46,12 +46,12 @@ class ExclusionValidationTest < ActiveModel::TestCase def test_validates_exclusion_of_with_lambda Topic.validates_exclusion_of :title, :in => lambda{ |topic| topic.author_name == "sikachu" ? %w( monkey elephant ) : %w( abe wasabi ) } - p = Topic.new - p.title = "elephant" - p.author_name = "sikachu" - assert p.invalid? + t = Topic.new + t.title = "elephant" + t.author_name = "sikachu" + assert t.invalid? - p.title = "wasabi" - assert p.valid? + t.title = "wasabi" + assert t.valid? end end diff --git a/activemodel/test/cases/validations/format_validation_test.rb b/activemodel/test/cases/validations/format_validation_test.rb index 2ce714fef0..41a1131bcb 100644 --- a/activemodel/test/cases/validations/format_validation_test.rb +++ b/activemodel/test/cases/validations/format_validation_test.rb @@ -101,25 +101,25 @@ class PresenceValidationTest < ActiveModel::TestCase def test_validates_format_of_with_lambda Topic.validates_format_of :content, :with => lambda{ |topic| topic.title == "digit" ? /\A\d+\Z/ : /\A\S+\Z/ } - p = Topic.new - p.title = "digit" - p.content = "Pixies" - assert p.invalid? + t = Topic.new + t.title = "digit" + t.content = "Pixies" + assert t.invalid? - p.content = "1234" - assert p.valid? + t.content = "1234" + assert t.valid? end def test_validates_format_of_without_lambda Topic.validates_format_of :content, :without => lambda{ |topic| topic.title == "characters" ? /\A\d+\Z/ : /\A\S+\Z/ } - p = Topic.new - p.title = "characters" - p.content = "1234" - assert p.invalid? + t = Topic.new + t.title = "characters" + t.content = "1234" + assert t.invalid? - p.content = "Pixies" - assert p.valid? + t.content = "Pixies" + assert t.valid? end def test_validates_format_of_for_ruby_class diff --git a/activemodel/test/cases/validations/inclusion_validation_test.rb b/activemodel/test/cases/validations/inclusion_validation_test.rb index 413da92de4..851d345eab 100644 --- a/activemodel/test/cases/validations/inclusion_validation_test.rb +++ b/activemodel/test/cases/validations/inclusion_validation_test.rb @@ -78,12 +78,12 @@ class InclusionValidationTest < ActiveModel::TestCase def test_validates_inclusion_of_with_lambda Topic.validates_inclusion_of :title, :in => lambda{ |topic| topic.author_name == "sikachu" ? %w( monkey elephant ) : %w( abe wasabi ) } - p = Topic.new - p.title = "wasabi" - p.author_name = "sikachu" - assert p.invalid? + t = Topic.new + t.title = "wasabi" + t.author_name = "sikachu" + assert t.invalid? - p.title = "elephant" - assert p.valid? + t.title = "elephant" + assert t.valid? end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 65578c1dc9..c50229e779 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -65,6 +65,19 @@ *Aaron Christy* +## Rails 3.1.3 (unreleased) ## + +* Perf fix: If we're deleting all records in an association, don't add a IN(..) clause + to the query. *GH 3672* + + *Jon Leighton* + +* Fix bug with referencing other mysql databases in set_table_name. *GH 3690* + +* Fix performance bug with mysql databases on a server with lots of other databses. *GH 3678* + + *Christos Zisopoulos and Kenny J* + ## Rails 3.1.2 (unreleased) ## * Fix bug with PostgreSQLAdapter#indexes. When the search path has multiple schemas, spaces diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 50ee60284c..3353cdf1ef 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -89,8 +89,12 @@ module ActiveRecord records.each { |r| r.destroy } update_counter(-records.length) unless inverse_updates_counter_cache? else - keys = records.map { |r| r[reflection.association_primary_key] } - scope = scoped.where(reflection.association_primary_key => keys) + scope = scoped + + unless records == load_target + keys = records.map { |r| r[reflection.association_primary_key] } + scope = scoped.where(reflection.association_primary_key => keys) + end if method == :delete_all update_counter(-scope.delete_all) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index e26b10fa97..f72d2974c8 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -576,17 +576,8 @@ module ActiveRecord # Returns a table's primary key and belonging sequence. def pk_and_sequence_for(table) - sql = <<-SQL - SELECT t.constraint_type, k.column_name - FROM information_schema.table_constraints t - JOIN information_schema.key_column_usage k - USING (constraint_name, table_schema, table_name) - WHERE t.table_schema = DATABASE() - AND t.table_name = '#{table}' - SQL - - execute_and_free(sql, 'SCHEMA') do |result| - keys = each_hash(result).select { |row| row[:constraint_type] == 'PRIMARY KEY' }.map { |row| row[:column_name] } + execute_and_free("SHOW INDEX FROM #{quote_table_name(table)} WHERE Key_name = 'PRIMARY'", 'SCHEMA') do |result| + keys = each_hash(result).map { |row| row[:Column_name] } keys.length == 1 ? [keys.first, nil] : nil end end diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 4ffcf6dbc1..abd71793fd 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -295,10 +295,10 @@ db_namespace = namespace :db do end desc 'Create the database, load the schema, and initialize with the seed data (use db:reset to also drop the db first)' - task :setup => :seed + task :setup => ['db:schema:load_if_ruby', 'db:structure:load_if_sql', :seed] desc 'Load the seed data from db/seeds.rb' - task :seed => ['db:schema:load_if_ruby', 'db:structure:load_if_sql'] do + task :seed do db_namespace['abort_if_pending_migrations'].invoke Rails.application.load_seed end @@ -377,10 +377,7 @@ db_namespace = namespace :db do ActiveRecord::Base.establish_connection(abcs[Rails.env]) File.open("#{Rails.root}/db/structure.sql", "w:utf-8") { |f| f << ActiveRecord::Base.connection.structure_dump } when /postgresql/ - ENV['PGHOST'] = abcs[Rails.env]['host'] if abcs[Rails.env]['host'] - ENV['PGPORT'] = abcs[Rails.env]['port'].to_s if abcs[Rails.env]['port'] - ENV['PGPASSWORD'] = abcs[Rails.env]['password'].to_s if abcs[Rails.env]['password'] - ENV['PGUSER'] = abcs[Rails.env]['username'].to_s if abcs[Rails.env]['username'] + set_psql_env(abcs[Rails.env]) search_path = abcs[Rails.env]['schema_search_path'] unless search_path.blank? search_path = search_path.split(",").map{|search_path_part| "--schema=#{search_path_part.strip}" }.join(" ") @@ -418,10 +415,7 @@ db_namespace = namespace :db do ActiveRecord::Base.connection.execute(table) end when /postgresql/ - ENV['PGHOST'] = abcs[env]['host'] if abcs[env]['host'] - ENV['PGPORT'] = abcs[env]['port'].to_s if abcs[env]['port'] - ENV['PGPASSWORD'] = abcs[env]['password'].to_s if abcs[env]['password'] - ENV['PGUSER'] = abcs[env]['username'].to_s if abcs[env]['username'] + set_psql_env(abcs[env]) `psql -f "#{Rails.root}/db/structure.sql" #{abcs[env]['database']} #{abcs[env]['template']}` when /sqlite/ dbfile = abcs[env]['database'] @@ -599,3 +593,10 @@ end def firebird_db_string(config) FireRuby::Database.db_string_for(config.symbolize_keys) end + +def set_psql_env(config) + ENV['PGHOST'] = config['host'] if config['host'] + ENV['PGPORT'] = config['port'].to_s if config['port'] + ENV['PGPASSWORD'] = config['password'].to_s if config['password'] + ENV['PGUSER'] = config['username'].to_s if config['username'] +end diff --git a/activerecord/test/cases/adapters/mysql/schema_test.rb b/activerecord/test/cases/adapters/mysql/schema_test.rb index a2155d1dd1..1aa034ed53 100644 --- a/activerecord/test/cases/adapters/mysql/schema_test.rb +++ b/activerecord/test/cases/adapters/mysql/schema_test.rb @@ -13,7 +13,7 @@ module ActiveRecord table = Post.table_name @db_name = db - @omgpost = Class.new(Post) do + @omgpost = Class.new(ActiveRecord::Base) do set_table_name "#{db}.#{table}" def self.name; 'Post'; end end @@ -23,6 +23,10 @@ module ActiveRecord assert @omgpost.find(:first) end + def test_primary_key + assert_equal 'id', @omgpost.primary_key + end + def test_table_exists? name = @omgpost.table_name assert @connection.table_exists?(name), "#{name} table should exist" diff --git a/activerecord/test/cases/adapters/mysql2/schema_test.rb b/activerecord/test/cases/adapters/mysql2/schema_test.rb index 858d1da2dd..49514e1539 100644 --- a/activerecord/test/cases/adapters/mysql2/schema_test.rb +++ b/activerecord/test/cases/adapters/mysql2/schema_test.rb @@ -13,7 +13,7 @@ module ActiveRecord table = Post.table_name @db_name = db - @omgpost = Class.new(Post) do + @omgpost = Class.new(ActiveRecord::Base) do set_table_name "#{db}.#{table}" def self.name; 'Post'; end end @@ -23,6 +23,10 @@ module ActiveRecord assert @omgpost.find(:first) end + def test_primary_key + assert_equal 'id', @omgpost.primary_key + end + def test_table_exists? name = @omgpost.table_name assert @connection.table_exists?(name), "#{name} table should exist" diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index b3ac271778..db90de4682 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -428,7 +428,7 @@ module ActiveSupport #:nodoc: end # Attempt to autoload the provided module name by searching for a directory - # matching the expect path suffix. If found, the module is created and assigned + # matching the expected path suffix. If found, the module is created and assigned # to +into+'s constants with the name +const_name+. Provided that the directory # was loaded from a reloadable base path, it is added to the set of constants # that are to be unloaded. diff --git a/railties/guides/source/action_mailer_basics.textile b/railties/guides/source/action_mailer_basics.textile index ad5b848d2c..26c95be031 100644 --- a/railties/guides/source/action_mailer_basics.textile +++ b/railties/guides/source/action_mailer_basics.textile @@ -362,21 +362,14 @@ When using named routes you only need to supply the +:host+: Email clients have no web context and so paths have no base URL to form complete web addresses. Thus, when using named routes only the "_url" variant makes sense. -It is also possible to set a default host that will be used in all mailers by setting the +:host+ option in the +ActionMailer::Base.default_url_options+ hash as follows: +It is also possible to set a default host that will be used in all mailers by setting the <tt>:host</tt> option as a configuration option in <tt>config/application.rb</tt>: <ruby> -class UserMailer < ActionMailer::Base - default_url_options[:host] = "example.com" - - def welcome_email(user) - @user = user - @url = user_url(@user) - mail(:to => user.email, - :subject => "Welcome to My Awesome Site") - end -end +config.action_mailer.default_url_options = { :host => "example.com" } </ruby> +If you use this setting, you should pass the <tt>:only_path => false</tt> option when using +url_for+. This will ensure that absolute URLs are generated because the +url_for+ view helper will, by default, generate relative URLs when a <tt>:host</tt> option isn't explicitly provided. + h4. Sending Multipart Emails Action Mailer will automatically send multipart emails if you have different templates for the same action. So, for our UserMailer example, if you have +welcome_email.text.erb+ and +welcome_email.html.erb+ in +app/views/user_mailer+, Action Mailer will automatically send a multipart email with the HTML and text versions setup as different parts. diff --git a/railties/guides/source/asset_pipeline.textile b/railties/guides/source/asset_pipeline.textile index 6ff5e87b6d..3681501293 100644 --- a/railties/guides/source/asset_pipeline.textile +++ b/railties/guides/source/asset_pipeline.textile @@ -438,7 +438,7 @@ location ~ ^/assets/ { } </plain> -When files are precompiled, Sprockets also creates a "gzipped":http://en.wikipedia.org/wiki/Gzip (.gz) version of your assets. Web servers are typically configured to use a moderate compression ratio as a compromise, but since precompilation happens once Sprockets uses the maximum compression ratio, thus reducing the size of the data transfer to the minimum. On the other hand, web servers can be configured to serve compressed content directly from disk, rather than deflating non-compressed files themselves. +When files are precompiled, Sprockets also creates a "gzipped":http://en.wikipedia.org/wiki/Gzip (.gz) version of your assets. Web servers are typically configured to use a moderate compression ratio as a compromise, but since precompilation happens once, Sprockets uses the maximum compression ratio, thus reducing the size of the data transfer to the minimum. On the other hand, web servers can be configured to serve compressed content directly from disk, rather than deflating non-compressed files themselves. Nginx is able to do this automatically enabling +gzip_static+: diff --git a/railties/guides/source/contributing_to_ruby_on_rails.textile b/railties/guides/source/contributing_to_ruby_on_rails.textile index 80c3cf6e1a..37ead2bff2 100644 --- a/railties/guides/source/contributing_to_ruby_on_rails.textile +++ b/railties/guides/source/contributing_to_ruby_on_rails.textile @@ -215,7 +215,7 @@ TIP: You may want to "put your git branch name in your shell prompt":http://qugs h3. Helping to Resolve Existing Issues -As a next step beyond reporting issues, you can help the core team resolve existing issues. If you check the "Everyone's Issues":https://github.com/rails/rails/issues?sort=created&direction=desc&state=open&page=1 list in GitHub Issues, you'll find lots of issues already requiring attention. What can you do for these? Quite a bit, actually: +As a next step beyond reporting issues, you can help the core team resolve existing issues. If you check the "Everyone's Issues":https://github.com/rails/rails/issues list in GitHub Issues, you'll find lots of issues already requiring attention. What can you do for these? Quite a bit, actually: h4. Verifying Bug Reports diff --git a/railties/guides/source/getting_started.textile b/railties/guides/source/getting_started.textile index 8a0a70efad..fde83ae730 100644 --- a/railties/guides/source/getting_started.textile +++ b/railties/guides/source/getting_started.textile @@ -308,6 +308,14 @@ manually with the application. * The +test+ environment is used when running automated tests. * The +production+ environment is used when you deploy your application for the world to use. +TIP: You don't have to update the database configurations manually. If you look at the +options of the application generator, you will see that one of the options +is named <tt>--database</tt>. This option allows you to choose an adapter from a +list of the most used relational databases. You can even run the generator +repeatedly: <tt>cd .. && rails new blog --database=mysql</tt>. When you confirm the overwriting + of the +config/database.yml+ file, your application will be configured for MySQL +instead of SQLite. Detailed examples of the common database connections are below. + h5. Configuring an SQLite3 Database Rails comes with built-in support for "SQLite3":http://www.sqlite.org, which is @@ -411,14 +419,6 @@ development: Change the username and password in the +development+ section as appropriate. -TIP: You don't have to update the database configurations manually. If you look at the -options of the application generator, you will see that one of the options -is named <tt>--database</tt>. This option allows you to choose an adapter from a -list of the most used relational databases. You can even run the generator -repeatedly: <tt>cd .. && rails new blog --database=mysql</tt>. When you confirm the overwriting - of the +config/database.yml+ file, your application will be configured for MySQL -instead of SQLite. - h4. Creating the Database Now that you have your database configured, it's time to have Rails create an diff --git a/railties/guides/source/i18n.textile b/railties/guides/source/i18n.textile index 2d4cc13571..e9477e84cf 100644 --- a/railties/guides/source/i18n.textile +++ b/railties/guides/source/i18n.textile @@ -231,7 +231,7 @@ end Now, when you call the +books_path+ method you should get +"/en/books"+ (for the default locale). An URL like +http://localhost:3001/nl/books+ should load the Netherlands locale, then, and following calls to +books_path+ should return +"/nl/books"+ (because the locale changed). -If you don't want to force the use of a locale in your routes you can use an optional path scope (donated by the use brackets) like so: +If you don't want to force the use of a locale in your routes you can use an optional path scope (denoted by the parentheses) like so: <ruby> # config/routes.rb diff --git a/railties/guides/source/testing.textile b/railties/guides/source/testing.textile index 2341a3522c..5dbbe2c0f0 100644 --- a/railties/guides/source/testing.textile +++ b/railties/guides/source/testing.textile @@ -927,7 +927,7 @@ class UserControllerTest < ActionController::TestCase assert_difference 'ActionMailer::Base.deliveries.size', +1 do post :invite_friend, :email => 'friend@example.com' end - invite_email = ActionMailer::Base.deliveries.first + invite_email = ActionMailer::Base.deliveries.last assert_equal "You have been invited by me@example.com", invite_email.subject assert_equal 'friend@example.com', invite_email.to[0] diff --git a/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb b/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb index f5c8ccf940..cd7d51e628 100644 --- a/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb +++ b/railties/lib/rails/generators/rails/plugin_new/plugin_new_generator.rb @@ -246,8 +246,20 @@ task :default => :test "rails plugin new #{self.arguments.map(&:usage).join(' ')} [options]" end + def original_name + @original_name ||= File.basename(destination_root) + end + def name - @name ||= File.basename(destination_root) + @name ||= begin + # same as ActiveSupport::Inflector#underscore except not replacing '-' + underscored = original_name.dup + underscored.gsub!(/([A-Z]+)([A-Z][a-z])/,'\1_\2') + underscored.gsub!(/([a-z\d])([A-Z])/,'\1_\2') + underscored.downcase! + + underscored + end end def camelized @@ -256,11 +268,11 @@ task :default => :test def valid_const? if camelized =~ /^\d/ - raise Error, "Invalid plugin name #{name}. Please give a name which does not start with numbers." + raise Error, "Invalid plugin name #{original_name}. Please give a name which does not start with numbers." elsif RESERVED_NAMES.include?(name) - raise Error, "Invalid plugin name #{name}. Please give a name which does not match one of the reserved rails words." + raise Error, "Invalid plugin name #{original_name}. Please give a name which does not match one of the reserved rails words." elsif Object.const_defined?(camelized) - raise Error, "Invalid plugin name #{name}, constant #{camelized} is already in use. Please choose another plugin name." + raise Error, "Invalid plugin name #{original_name}, constant #{camelized} is already in use. Please choose another plugin name." end end diff --git a/railties/test/generators/plugin_new_generator_test.rb b/railties/test/generators/plugin_new_generator_test.rb index b70f9f4d9c..826eac904e 100644 --- a/railties/test/generators/plugin_new_generator_test.rb +++ b/railties/test/generators/plugin_new_generator_test.rb @@ -37,6 +37,12 @@ class PluginNewGeneratorTest < Rails::Generators::TestCase assert_file "things-43/lib/things-43.rb", /module Things43/ end + def test_camelcase_plugin_name_underscores_filenames + run_generator [File.join(destination_root, "CamelCasedName")] + assert_no_file "CamelCasedName/lib/CamelCasedName.rb" + assert_file "CamelCasedName/lib/camel_cased_name.rb", /module CamelCasedName/ + end + def test_generating_without_options run_generator assert_file "README.rdoc", /Bukkits/ |