diff options
171 files changed, 2022 insertions, 1040 deletions
diff --git a/.travis.yml b/.travis.yml index 7f5b2095e3..f0e9d570f5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,7 @@ before_install: rvm: - 1.9.3 - 2.0.0 - - jruby-19mode + - jruby-head - rbx-19mode env: - "GEM=railties" @@ -15,7 +15,7 @@ env: - "GEM=ar:postgresql" matrix: allow_failures: - - rvm: jruby-19mode + - rvm: jruby-head - rvm: rbx-19mode notifications: email: false @@ -8,7 +8,7 @@ gemspec gem 'mocha', '~> 0.14', require: false gem 'rack-cache', '~> 1.2' -gem 'bcrypt-ruby', '~> 3.1.0' +gem 'bcrypt-ruby', '~> 3.1.2' gem 'jquery-rails', '~> 2.2.0' gem 'turbolinks' gem 'coffee-rails', '~> 4.0.0' diff --git a/actionmailer/README.rdoc b/actionmailer/README.rdoc index 178572706c..96dd0b1a2e 100644 --- a/actionmailer/README.rdoc +++ b/actionmailer/README.rdoc @@ -61,9 +61,7 @@ generated would look like this: Thank you for signing up! -In previous versions of Rails you would call <tt>create_method_name</tt> and -<tt>deliver_method_name</tt>. Rails 3.0 has a much simpler interface - you -simply call the method and optionally call +deliver+ on the return value. +In order to send mails, you simply call the method and then call +deliver+ on the return value. Calling the method returns a Mail Message object: diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 63a208dbcf..7af9165605 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,13 +1,45 @@ +* Fixing repond_with working directly on the options hash + This fixes an issue where the respond_with worked directly with the given + options hash, so that if a user relied on it after calling respond_with, + the hash wouldn't be the same. + + Fixes #12029. + + *bluehotdog* + +* Fix `ActionDispatch::RemoteIp::GetIp#calculate_ip` to only check for spoofing + attacks if both `HTTP_CLIENT_IP` and `HTTP_X_FORWARDED_FOR` are set. + + Fixes #10844. + + *Tamir Duberstein* + +* Strong parameters should permit nested number as key. + + Fixes #12293. + + *kennyj* + +* Fix regex used to detect URI schemes in `redirect_to` to be consistent with + RFC 3986. + + *Derek Prior* + +* Fix incorrect `assert_redirected_to` failure message for protocol-relative + URLs. + + *Derek Prior* + * Fix an issue where router can't recognize downcased url encoding path. - Fixes #12269 + Fixes #12269. *kennyj* * Fix custom flash type definition. Misusage of the `_flash_types` class variable caused an error when reloading controllers with custom flash types. - Fixes #12057 + Fixes #12057. *Ricardo de Cillo* @@ -28,21 +60,21 @@ * Fix an issue where :if and :unless controller action procs were being run before checking for the correct action in the :only and :unless options. - Fixes #11799 + Fixes #11799. *Nicholas Jakobsen* * Fix an issue where `assert_dom_equal` and `assert_dom_not_equal` were ignoring the passed failure message argument. - Fixes #11751 + Fixes #11751. *Ryan McGeary* * Allow REMOTE_ADDR, HTTP_HOST and HTTP_USER_AGENT to be overridden from the environment passed into `ActionDispatch::TestRequest.new`. - Fixes #11590 + Fixes #11590. *Andrew White* @@ -57,7 +89,7 @@ * Skip routes pointing to a redirect or mounted application when generating urls using an options hash as they aren't relevant and generate incorrect urls. - Fixes #8018 + Fixes #8018. *Andrew White* @@ -75,7 +107,7 @@ * Fix `ActionDispatch::ParamsParser#parse_formatted_parameters` to rewind body input stream on parsing json params. - Fixes #11345 + Fixes #11345. *Yuri Bol*, *Paul Nikitochkin* @@ -108,7 +140,7 @@ was setting `request.formats` with an array containing a `nil` value, which raised an error when setting the controller formats. - Fixes #10965 + Fixes #10965. *Becker* @@ -117,7 +149,7 @@ no `:to` present in the options hash so should only affect routes using the shorthand syntax (i.e. endpoint is inferred from the path). - Fixes #9856 + Fixes #9856. *Yves Senn*, *Andrew White* diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index f93f0d4404..db7b56f47e 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -73,7 +73,7 @@ module ActionController # <input type="text" name="post[address]" value="hyacintvej"> # # A request stemming from a form holding these inputs will include <tt>{ "post" => { "name" => "david", "address" => "hyacintvej" } }</tt>. - # If the address input had been named "post[address][street]", the params would have included + # If the address input had been named \"post[address][street]", the params would have included # <tt>{ "post" => { "address" => { "street" => "hyacintvej" } } }</tt>. There's no limit to the depth of the nesting. # # == Sessions diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 66dabd821f..a072fce1a1 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -326,6 +326,7 @@ module ActionController #:nodoc: if collector = retrieve_collector_from_mimes(&block) options = resources.size == 1 ? {} : resources.extract_options! + options = options.clone options[:default_response] = collector.response (options.delete(:responder) || self.class.responder).call(self, resources, options) end diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index e9031f3fac..ab14a61b97 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -71,6 +71,26 @@ module ActionController self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.h(location)}\">redirected</a>.</body></html>" end + def _compute_redirect_to_location(options) #:nodoc: + case options + # The scheme name consist of a letter followed by any combination of + # letters, digits, and the plus ("+"), period ("."), or hyphen ("-") + # characters; and is terminated by a colon (":"). + # See http://tools.ietf.org/html/rfc3986#section-3.1 + # The protocol relative scheme starts with a double slash "//". + when /\A([a-z][a-z\d\-+\.]*:|\/\/).*/i + options + when String + request.protocol + request.host_with_port + options + when :back + request.headers["Referer"] or raise RedirectBackError + when Proc + _compute_redirect_to_location options.call + else + url_for(options) + end.delete("\0\r\n") + end + private def _extract_redirect_to_status(options, response_status) if options.is_a?(Hash) && options.key?(:status) @@ -81,24 +101,5 @@ module ActionController 302 end end - - def _compute_redirect_to_location(options) - case options - # The scheme name consist of a letter followed by any combination of - # letters, digits, and the plus ("+"), period ("."), or hyphen ("-") - # characters; and is terminated by a colon (":"). - # The protocol relative scheme starts with a double slash "//" - when %r{\A(\w[\w+.-]*:|//).*} - options - when String - request.protocol + request.host_with_port + options - when :back - request.headers["Referer"] or raise RedirectBackError - when Proc - _compute_redirect_to_location options.call - else - url_for(options) - end.delete("\0\r\n") - end end end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index b495ab3f0f..66403d533c 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -334,7 +334,7 @@ module ActionController def each_element(object) if object.is_a?(Array) object.map { |el| yield el }.compact - elsif object.is_a?(Hash) && object.keys.all? { |k| k =~ /\A-?\d+\z/ } + elsif fields_for_style?(object) hash = object.class.new object.each { |k,v| hash[k] = yield v } hash @@ -343,6 +343,10 @@ module ActionController end end + def fields_for_style?(object) + object.is_a?(Hash) && object.all? { |k, v| k =~ /\A-?\d+\z/ && v.is_a?(Hash) } + end + def unpermitted_parameters!(params) unpermitted_keys = unpermitted_keys(params) if unpermitted_keys.any? diff --git a/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb b/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb index da0cddd93c..971cb3447f 100644 --- a/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb +++ b/actionpack/lib/action_dispatch/journey/gtg/transition_table.rb @@ -9,8 +9,8 @@ module ActionDispatch attr_reader :memos def initialize - @regexp_states = Hash.new { |h,k| h[k] = {} } - @string_states = Hash.new { |h,k| h[k] = {} } + @regexp_states = {} + @string_states = {} @accepting = {} @memos = Hash.new { |h,k| h[k] = [] } end @@ -111,14 +111,8 @@ module ActionDispatch end def []=(from, to, sym) - case sym - when String - @string_states[from][sym] = to - when Regexp - @regexp_states[from][sym] = to - else - raise ArgumentError, 'unknown symbol: %s' % sym.class - end + to_mappings = states_hash_for(sym)[from] ||= {} + to_mappings[sym] = to end def states @@ -137,18 +131,35 @@ module ActionDispatch private + def states_hash_for(sym) + case sym + when String + @string_states + when Regexp + @regexp_states + else + raise ArgumentError, 'unknown symbol: %s' % sym.class + end + end + def move_regexp(t, a) return [] if t.empty? t.map { |s| - @regexp_states[s].map { |re, v| re === a ? v : nil } + if states = @regexp_states[s] + states.map { |re, v| re === a ? v : nil } + end }.flatten.compact.uniq end def move_string(t, a) return [] if t.empty? - t.map { |s| @string_states[s][a] }.compact + t.map do |s| + if states = @string_states[s] + states[a] + end + end.compact end end end diff --git a/actionpack/lib/action_dispatch/journey/router/utils.rb b/actionpack/lib/action_dispatch/journey/router/utils.rb index 80011597aa..1edf86cd88 100644 --- a/actionpack/lib/action_dispatch/journey/router/utils.rb +++ b/actionpack/lib/action_dispatch/journey/router/utils.rb @@ -7,11 +7,13 @@ module ActionDispatch # Normalizes URI path. # # Strips off trailing slash and ensures there is a leading slash. + # Also converts downcase url encoded string to uppercase. # # normalize_path("/foo") # => "/foo" # normalize_path("/foo/") # => "/foo" # normalize_path("foo") # => "/foo" # normalize_path("") # => "/" + # normalize_path("/%ab") # => "/%AB" def self.normalize_path(path) path = "/#{path}" path.squeeze!('/') @@ -36,7 +38,7 @@ module ActionDispatch UNSAFE_FRAGMENT = Regexp.new("[^#{safe_fragment}]", false).freeze end - Parser = URI.const_defined?(:Parser) ? URI::Parser.new : URI + Parser = URI::Parser.new def self.escape_path(path) Parser.escape(path.to_s, UriEscape::UNSAFE_SEGMENT) diff --git a/actionpack/lib/action_dispatch/journey/visitors.rb b/actionpack/lib/action_dispatch/journey/visitors.rb index 0a8cb1b4d4..9e66cab052 100644 --- a/actionpack/lib/action_dispatch/journey/visitors.rb +++ b/actionpack/lib/action_dispatch/journey/visitors.rb @@ -1,9 +1,12 @@ # encoding: utf-8 + +require 'thread_safe' + module ActionDispatch module Journey # :nodoc: module Visitors # :nodoc: class Visitor # :nodoc: - DISPATCH_CACHE = Hash.new { |h,k| + DISPATCH_CACHE = ThreadSafe::Cache.new { |h,k| h[k] = :"visit_#{k}" } @@ -84,44 +87,43 @@ module ActionDispatch # Used for formatting urls (url_for) class Formatter < Visitor # :nodoc: - attr_reader :options, :consumed + attr_reader :options def initialize(options) @options = options - @consumed = {} end private - def visit_GROUP(node) - if consumed == options - nil - else - route = visit(node.left) - route.include?("\0") ? nil : route + def visit(node, optional = false) + case node.type + when :LITERAL, :SLASH, :DOT + node.left + when :STAR + visit(node.left) + when :GROUP + visit(node.left, true) + when :CAT + visit_CAT(node, optional) + when :SYMBOL + visit_SYMBOL(node) end end - def terminal(node) - node.left - end - - def binary(node) - [visit(node.left), visit(node.right)].join - end + def visit_CAT(node, optional) + left = visit(node.left, optional) + right = visit(node.right, optional) - def nary(node) - node.children.map { |c| visit(c) }.join + if optional && !(right && left) + "" + else + [left, right].join + end end def visit_SYMBOL(node) - key = node.to_sym - - if value = options[key] - consumed[key] = value + if value = options[node.to_sym] Router::Utils.escape_path(value) - else - "\0" end end end diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 8879291dbd..57bc6d5cd0 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -143,7 +143,7 @@ module ActionDispatch # proxies with incompatible IP header conventions, and there is no way # for us to determine which header is the right one after the fact. # Since we have no idea, we give up and explode. - should_check_ip = @check_ip && client_ips.last + should_check_ip = @check_ip && client_ips.last && forwarded_ips.last if should_check_ip && !forwarded_ips.include?(client_ips.last) # We don't know which came from the proxy, and which from the user raise IpSpoofAttackError, "IP spoofing attack?! " + diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index 6d3f8da932..2fb03f2712 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -74,6 +74,19 @@ module ActionDispatch # * <tt>:routing_type</tt> - Allowed values are <tt>:path</tt> or <tt>:url</tt>. # Default is <tt>:url</tt>. # + # Also includes all the options from <tt>url_for</tt>. These include such + # things as <tt>:anchor</tt> or <tt>:trailing_slash</tt>. Example usage + # is given below: + # + # polymorphic_url([blog, post], anchor: 'my_anchor') + # # => "http://example.com/blogs/1/posts/1#my_anchor" + # polymorphic_url([blog, post], anchor: 'my_anchor', script_name: "/my_app") + # # => "http://example.com/my_app/blogs/1/posts/1#my_anchor" + # + # For all of these options, see the documentation for <tt>url_for</tt>. + # + # ==== Functionality + # # # an Article record # polymorphic_url(record) # same as article_url(record) # diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 8e19025722..bcebe532bf 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -20,7 +20,7 @@ module ActionDispatch # # <%= link_to('Click here', controller: 'users', # action: 'new', message: 'Welcome!') %> - # # => "/users/new?message=Welcome%21" + # # => <a href="/users/new?message=Welcome%21">Click here</a> # # link_to, and all other functions that require URL generation functionality, # actually use ActionController::UrlFor under the hood. And in particular, diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 44ed0ac1f3..93f9fab9c2 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -67,21 +67,11 @@ module ActionDispatch end def normalize_argument_to_redirection(fragment) - normalized = case fragment - when Regexp - fragment - when %r{^\w[A-Za-z\d+.-]*:.*} - fragment - when String - @request.protocol + @request.host_with_port + fragment - when :back - raise RedirectBackError unless refer = @request.headers["Referer"] - refer - else - @controller.url_for(fragment) - end - - normalized.respond_to?(:delete) ? normalized.delete("\0\r\n") : normalized + if Regexp === fragment + fragment + else + @controller._compute_redirect_to_location(fragment) + end end end end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 22a410db94..ba4cffcd3e 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -39,6 +39,8 @@ class ActionPackAssertionsController < ActionController::Base def redirect_external() redirect_to "http://www.rubyonrails.org"; end + def redirect_external_protocol_relative() redirect_to "//www.rubyonrails.org"; end + def response404() head '404 AWOL' end def response500() head '500 Sorry' end @@ -258,6 +260,19 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase end end + def test_assert_redirect_failure_message_with_protocol_relative_url + begin + process :redirect_external_protocol_relative + assert_redirected_to "/foo" + rescue ActiveSupport::TestCase::Assertion => ex + assert_no_match( + /#{request.protocol}#{request.host}\/\/www.rubyonrails.org/, + ex.message, + 'protocol relative url was incorrectly normalized' + ) + end + end + def test_template_objects_exist process :assign_this assert !@controller.instance_variable_defined?(:"@hi") @@ -309,6 +324,9 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase process :redirect_external assert_equal 'http://www.rubyonrails.org', @response.redirect_url + + process :redirect_external_protocol_relative + assert_equal '//www.rubyonrails.org', @response.redirect_url end def test_no_redirect_url diff --git a/actionpack/test/controller/mime/respond_with_test.rb b/actionpack/test/controller/mime/respond_with_test.rb index 76af9e3414..a70592fa1b 100644 --- a/actionpack/test/controller/mime/respond_with_test.rb +++ b/actionpack/test/controller/mime/respond_with_test.rb @@ -65,7 +65,17 @@ class RespondWithController < ActionController::Base respond_with(resource, :responder => responder) end + def respond_with_additional_params + @params = RespondWithController.params + respond_with({:result => resource}, @params) + end + protected + def self.params + { + :foo => 'bar' + } + end def resource Customer.new("david", request.delete? ? nil : 13) @@ -145,6 +155,11 @@ class RespondWithControllerTest < ActionController::TestCase Mime::Type.unregister(:mobile) end + def test_respond_with_shouldnt_modify_original_hash + get :respond_with_additional_params + assert_equal RespondWithController.params, assigns(:params) + end + def test_using_resource @request.accept = "application/xml" get :using_resource diff --git a/actionpack/test/controller/parameters/nested_parameters_test.rb b/actionpack/test/controller/parameters/nested_parameters_test.rb index 91df527dec..3b1257e8d5 100644 --- a/actionpack/test/controller/parameters/nested_parameters_test.rb +++ b/actionpack/test/controller/parameters/nested_parameters_test.rb @@ -169,4 +169,19 @@ class NestedParametersTest < ActiveSupport::TestCase assert_filtered_out permitted[:book][:authors_attributes]['-1'], :age_of_death end + + test "nested number as key" do + params = ActionController::Parameters.new({ + product: { + properties: { + '0' => "prop0", + '1' => "prop1" + } + } + }) + params = params.require(:product).permit(:properties => ["0"]) + assert_not_nil params[:properties]["0"] + assert_nil params[:properties]["1"] + assert_equal "prop0", params[:properties]["0"] + end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index e4c3ddd3f9..3e9e90a950 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -1102,6 +1102,19 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal 'projects#index', @response.body end + def test_scoped_root_as_name + draw do + scope '(:locale)', :locale => /en|pl/ do + root :to => 'projects#index', :as => 'projects' + end + end + + assert_equal '/en', projects_path(:locale => 'en') + assert_equal '/', projects_path + get '/en' + assert_equal 'projects#index', @response.body + end + def test_scope_with_format_option draw do get "direct/index", as: :no_format_direct, format: false diff --git a/actionpack/test/fixtures/helpers/helpery_test_helper.rb b/actionpack/test/fixtures/helpers/helpery_test_helper.rb deleted file mode 100644 index a4f2951efa..0000000000 --- a/actionpack/test/fixtures/helpers/helpery_test_helper.rb +++ /dev/null @@ -1,5 +0,0 @@ -module HelperyTestHelper - def helpery_test - "Default" - end -end diff --git a/actionpack/test/fixtures/respond_with/respond_with_additional_params.html.erb b/actionpack/test/fixtures/respond_with/respond_with_additional_params.html.erb new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/actionpack/test/fixtures/respond_with/respond_with_additional_params.html.erb diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 53142a835e..a09650c559 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,26 @@ +* Fix `collection_check_boxes` generated hidden input to use the name attribute provided + in the options hash. + + *Angel N. Sciortino* + +* Fix some edge cases for AV `select` helper with `:selected` option + + *Bogdan Gusiev* + +* Ability to pass block to `select` helper + + <%= select(report, "campaign_ids") do %> + <% available_campaigns.each do |c| -%> + <%= content_tag(:option, c.name, value: c.id, data: { tags: c.tags.to_json }) %> + <% end -%> + <% end -%> + + *Bogdan Gusiev* + +* Handle `:namespace` form option in collection labels + + *Vasiliy Ermolovich* + * Fix `form_for` when both `namespace` and `as` options are present `as` option no longer overwrites `namespace` option when generating @@ -13,6 +36,12 @@ *Josh Lauer*, *Justin Ridgewell* +* Fixed a bug where the lookup details were not being taken into account + when caching the digest of a template - changes to the details now + cause a different cache key to be used. + + *Daniel Schierbeck* + * Added an `extname` hash option for `javascript_include_tag` method. Before: diff --git a/actionview/RUNNING_UNIT_TESTS.rdoc b/actionview/RUNNING_UNIT_TESTS.rdoc index a0c35e1810..104b3e288d 100644 --- a/actionview/RUNNING_UNIT_TESTS.rdoc +++ b/actionview/RUNNING_UNIT_TESTS.rdoc @@ -18,7 +18,7 @@ which can be further narrowed down to one test: == Dependency on Active Record and database setup -Test cases in the test/active_record/ directory depend on having +Test cases in the test/activerecord/ directory depend on having activerecord and sqlite installed. If Active Record is not in actionview/../activerecord directory, or the sqlite rubygem is not installed, these tests are skipped. diff --git a/actionview/Rakefile b/actionview/Rakefile index e1cb4b5be0..d56fe9ea76 100644 --- a/actionview/Rakefile +++ b/actionview/Rakefile @@ -11,7 +11,7 @@ task :test => ["test:template", "test:integration:action_pack", "test:integratio namespace :test do task :isolated do - Dir.glob("test/{active_record,template}/**/*_test.rb").all? do |file| + Dir.glob("test/{actionpack,activerecord,template}/**/*_test.rb").all? do |file| sh(Gem.ruby, '-w', '-Ilib:test', file) end or raise "Failures" end diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 95c3200c81..af158a630b 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -10,7 +10,10 @@ module ActionView class << self def digest(name, format, finder, options = {}) - cache_key = ([name, format] + Array.wrap(options[:dependencies])).join('.') + details_key = finder.details_key.hash + dependencies = Array.wrap(options[:dependencies]) + cache_key = ([name, details_key, format] + dependencies).join('.') + # this is a correctly done double-checked locking idiom # (ThreadSafe::Cache's lookups have volatile semantics) @@cache[cache_key] || @@digest_monitor.synchronize do diff --git a/actionview/lib/action_view/helpers/asset_tag_helper.rb b/actionview/lib/action_view/helpers/asset_tag_helper.rb index a13d0021ea..e2430140c2 100644 --- a/actionview/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionview/lib/action_view/helpers/asset_tag_helper.rb @@ -224,14 +224,14 @@ module ActionView # # ==== Examples # - # image_tag('rails.png') - # # => <img alt="Rails" src="/assets/rails.png" /> + # image_alt('rails.png') + # # => Rails # - # image_tag('hyphenated-file-name.png') - # # => <img alt="Hyphenated file name" src="/assets/hyphenated-file-name.png" /> + # image_alt('hyphenated-file-name.png') + # # => Hyphenated file name # - # image_tag('underscored_file_name.png') - # # => <img alt="Underscored file name" src="/assets/underscored_file_name.png" /> + # image_alt('underscored_file_name.png') + # # => Underscored file name def image_alt(src) File.basename(src, '.*').sub(/-[[:xdigit:]]{32}\z/, '').tr('-_', ' ').capitalize end diff --git a/actionview/lib/action_view/helpers/form_options_helper.rb b/actionview/lib/action_view/helpers/form_options_helper.rb index fcd151ac32..4347983bad 100644 --- a/actionview/lib/action_view/helpers/form_options_helper.rb +++ b/actionview/lib/action_view/helpers/form_options_helper.rb @@ -128,6 +128,15 @@ module ActionView # or <tt>selected: nil</tt> to leave all options unselected. Similarly, you can specify values to be disabled in the option # tags by specifying the <tt>:disabled</tt> option. This can either be a single value or an array of values to be disabled. # + # A block can be passed to +select+ to customize how the options tags will be rendered. This + # is useful when the options tag has complex attributes. + # + # select(report, "campaign_ids") do + # available_campaigns.each do |c| + # content_tag(:option, c.name, value: c.id, data: { tags: c.tags.to_json }) + # end + # end + # # ==== Gotcha # # The HTML specification says when +multiple+ parameter passed to select and all options got deselected @@ -152,8 +161,8 @@ module ActionView # In case if you don't want the helper to generate this hidden field you can specify # <tt>include_hidden: false</tt> option. # - def select(object, method, choices, options = {}, html_options = {}) - Tags::Select.new(object, method, self, choices, options, html_options).render + def select(object, method, choices = nil, options = {}, html_options = {}, &block) + Tags::Select.new(object, method, self, choices, options, html_options, &block).render end # Returns <tt><select></tt> and <tt><option></tt> tags for the collection of existing return values of @@ -766,8 +775,8 @@ module ActionView # <% end %> # # Please refer to the documentation of the base helper for details. - def select(method, choices, options = {}, html_options = {}) - @template.select(@object_name, method, choices, objectify_options(options), @default_options.merge(html_options)) + def select(method, choices = nil, options = {}, html_options = {}, &block) + @template.select(@object_name, method, choices, objectify_options(options), @default_options.merge(html_options), &block) end # Wraps ActionView::Helpers::FormOptionsHelper#collection_select for form builders: diff --git a/actionview/lib/action_view/helpers/tags/base.rb b/actionview/lib/action_view/helpers/tags/base.rb index 3fe3f4e9df..8607da301c 100644 --- a/actionview/lib/action_view/helpers/tags/base.rb +++ b/actionview/lib/action_view/helpers/tags/base.rb @@ -119,7 +119,8 @@ module ActionView html_options = html_options.stringify_keys add_default_name_and_id(html_options) options[:include_blank] ||= true unless options[:prompt] || select_not_required?(html_options) - select = content_tag("select", add_options(option_tags, options, value(object)), html_options) + value = options.fetch(:selected) { value(object) } + select = content_tag("select", add_options(option_tags, options, value), html_options) if html_options["multiple"] && options.fetch(:include_hidden, true) tag("input", :disabled => html_options["disabled"], :name => html_options["name"], :type => "hidden", :value => "") + select diff --git a/actionview/lib/action_view/helpers/tags/collection_check_boxes.rb b/actionview/lib/action_view/helpers/tags/collection_check_boxes.rb index 52006d856b..9b77ebeb1b 100644 --- a/actionview/lib/action_view/helpers/tags/collection_check_boxes.rb +++ b/actionview/lib/action_view/helpers/tags/collection_check_boxes.rb @@ -27,7 +27,8 @@ module ActionView # Append a hidden field to make sure something will be sent back to the # server if all check boxes are unchecked. - hidden = @template_object.hidden_field_tag("#{tag_name}[]", "", :id => nil) + hidden_name = @html_options[:name] || "#{tag_name}[]" + hidden = @template_object.hidden_field_tag(hidden_name, "", :id => nil) rendered_collection + hidden end diff --git a/actionview/lib/action_view/helpers/tags/collection_helpers.rb b/actionview/lib/action_view/helpers/tags/collection_helpers.rb index 388dcf1f13..787039c82e 100644 --- a/actionview/lib/action_view/helpers/tags/collection_helpers.rb +++ b/actionview/lib/action_view/helpers/tags/collection_helpers.rb @@ -18,7 +18,8 @@ module ActionView end def label(label_html_options={}, &block) - @template_object.label(@object_name, @sanitized_attribute_name, @text, label_html_options, &block) + html_options = label_html_options.merge(@input_html_options) + @template_object.label(@object_name, @sanitized_attribute_name, @text, html_options, &block) end end diff --git a/actionview/lib/action_view/helpers/tags/label.rb b/actionview/lib/action_view/helpers/tags/label.rb index 35d3ba8434..180aa9ac27 100644 --- a/actionview/lib/action_view/helpers/tags/label.rb +++ b/actionview/lib/action_view/helpers/tags/label.rb @@ -30,6 +30,7 @@ module ActionView add_default_name_and_id_for_value(tag_value, name_and_id) options.delete("index") options.delete("namespace") + options.delete("multiple") options["for"] = name_and_id["id"] unless options.key?("for") if block_given? diff --git a/actionview/lib/action_view/helpers/tags/select.rb b/actionview/lib/action_view/helpers/tags/select.rb index d64e2f68ef..00881d9978 100644 --- a/actionview/lib/action_view/helpers/tags/select.rb +++ b/actionview/lib/action_view/helpers/tags/select.rb @@ -3,8 +3,9 @@ module ActionView module Tags # :nodoc: class Select < Base # :nodoc: def initialize(object_name, method_name, template_object, choices, options, html_options) - @choices = choices + @choices = block_given? ? template_object.capture { yield } : choices @choices = @choices.to_a if @choices.is_a?(Range) + @html_options = html_options super(object_name, method_name, template_object, options) diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index 1920a94567..8a4918a8c0 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -92,8 +92,9 @@ module ActionView # ==== Data attributes # # * <tt>confirm: 'question?'</tt> - This will allow the unobtrusive JavaScript - # driver to prompt with the question specified. If the user accepts, the link is - # processed normally, otherwise no action is taken. + # driver to prompt with the question specified (in this case, the + # resulting text would be <tt>question?</tt>. If the user accepts, the + # link is processed normally, otherwise no action is taken. # * <tt>:disable_with</tt> - Value of this parameter will be # used as the value for a disabled version of the submit # button when the form is submitted. This feature is provided diff --git a/actionview/lib/action_view/railtie.rb b/actionview/lib/action_view/railtie.rb index 4113448af0..c2783f6377 100644 --- a/actionview/lib/action_view/railtie.rb +++ b/actionview/lib/action_view/railtie.rb @@ -48,5 +48,9 @@ module ActionView ActionMailer::Base.send(:include, ActionView::Layouts) end end + + rake_tasks do + load "action_view/tasks/dependencies.rake" + end end end diff --git a/actionview/lib/action_view/tasks/dependencies.rake b/actionview/lib/action_view/tasks/dependencies.rake new file mode 100644 index 0000000000..1b9426c0e5 --- /dev/null +++ b/actionview/lib/action_view/tasks/dependencies.rake @@ -0,0 +1,17 @@ +namespace :cache_digests do + desc 'Lookup nested dependencies for TEMPLATE (like messages/show or comments/_comment.html)' + task :nested_dependencies => :environment do + abort 'You must provide TEMPLATE for the task to run' unless ENV['TEMPLATE'].present? + template, format = ENV['TEMPLATE'].split(".") + format ||= :html + puts JSON.pretty_generate ActionView::Digestor.new(template, format, ApplicationController.new.lookup_context).nested_dependencies + end + + desc 'Lookup first-level dependencies for TEMPLATE (like messages/show or comments/_comment.html)' + task :dependencies => :environment do + abort 'You must provide TEMPLATE for the task to run' unless ENV['TEMPLATE'].present? + template, format = ENV['TEMPLATE'].split(".") + format ||= :html + puts JSON.pretty_generate ActionView::Digestor.new(template, format, ApplicationController.new.lookup_context).dependencies + end +end diff --git a/actionview/test/fixtures/helpers/fun/games_helper.rb b/actionview/test/fixtures/helpers/fun/games_helper.rb deleted file mode 100644 index 3b7adce086..0000000000 --- a/actionview/test/fixtures/helpers/fun/games_helper.rb +++ /dev/null @@ -1,5 +0,0 @@ -module Fun - module GamesHelper - def stratego() "Iz guuut!" end - end -end
\ No newline at end of file diff --git a/actionview/test/fixtures/helpers/fun/pdf_helper.rb b/actionview/test/fixtures/helpers/fun/pdf_helper.rb deleted file mode 100644 index 0171be8500..0000000000 --- a/actionview/test/fixtures/helpers/fun/pdf_helper.rb +++ /dev/null @@ -1,5 +0,0 @@ -module Fun - module PdfHelper - def foobar() 'baz' end - end -end diff --git a/actionview/test/fixtures/helpers/just_me_helper.rb b/actionview/test/fixtures/helpers/just_me_helper.rb deleted file mode 100644 index b140a7b9b4..0000000000 --- a/actionview/test/fixtures/helpers/just_me_helper.rb +++ /dev/null @@ -1,3 +0,0 @@ -module JustMeHelper - def me() "mine!" end -end
\ No newline at end of file diff --git a/actionview/test/fixtures/helpers/me_too_helper.rb b/actionview/test/fixtures/helpers/me_too_helper.rb deleted file mode 100644 index ce56042143..0000000000 --- a/actionview/test/fixtures/helpers/me_too_helper.rb +++ /dev/null @@ -1,3 +0,0 @@ -module MeTooHelper - def me() "me too!" end -end
\ No newline at end of file diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index c6608e214a..0f6b14a57d 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -15,6 +15,16 @@ end class FixtureFinder FIXTURES_DIR = "#{File.dirname(__FILE__)}/../fixtures/digestor" + attr_reader :details + + def initialize + @details = {} + end + + def details_key + details.hash + end + def find(logical_name, keys, partial, options) FixtureTemplate.new("digestor/#{partial ? logical_name.gsub(%r|/([^/]+)$|, '/_\1') : logical_name}.#{options[:formats].first}.erb") end @@ -140,6 +150,20 @@ class TemplateDigestorTest < ActionView::TestCase end end + def test_details_are_included_in_cache_key + # Cache the template digest. + old_digest = digest("events/_event") + + # Change the template; the cached digest remains unchanged. + change_template("events/_event") + + # The details are changed, so a new cache key is generated. + finder.details[:foo] = "bar" + + # The cache is busted. + assert_not_equal old_digest, digest("events/_event") + end + def test_extra_whitespace_in_render_partial assert_digest_difference("messages/edit") do change_template("messages/_form") @@ -220,7 +244,11 @@ class TemplateDigestorTest < ActionView::TestCase end def digest(template_name, options={}) - ActionView::Digestor.digest(template_name, :html, FixtureFinder.new, options) + ActionView::Digestor.digest(template_name, :html, finder, options) + end + + def finder + @finder ||= FixtureFinder.new end def change_template(template_name) diff --git a/actionview/test/template/form_collections_helper_test.rb b/actionview/test/template/form_collections_helper_test.rb index b56847396d..d28e4aeb48 100644 --- a/actionview/test/template/form_collections_helper_test.rb +++ b/actionview/test/template/form_collections_helper_test.rb @@ -179,6 +179,13 @@ class FormCollectionsHelperTest < ActionView::TestCase assert_select "input[type=hidden][name='user[category_ids][]'][value=]", :count => 1 end + test 'collection check boxes generates a hidden field using the given :name in :html_options' do + collection = [Category.new(1, 'Category 1'), Category.new(2, 'Category 2')] + with_collection_check_boxes :user, :category_ids, collection, :id, :name, {}, {name: "user[other_category_ids][]"} + + assert_select "input[type=hidden][name='user[other_category_ids][]'][value=]", :count => 1 + end + test 'collection check boxes accepts a collection and generate a serie of checkboxes with labels for label method' do collection = [Category.new(1, 'Category 1'), Category.new(2, 'Category 2')] with_collection_check_boxes :user, :category_ids, collection, :id, :name diff --git a/actionview/test/template/form_helper_test.rb b/actionview/test/template/form_helper_test.rb index 944884c9dd..3e8a2468ed 100644 --- a/actionview/test/template/form_helper_test.rb +++ b/actionview/test/template/form_helper_test.rb @@ -1282,6 +1282,24 @@ class FormHelperTest < ActionView::TestCase assert_dom_equal expected, output_buffer end + def test_form_with_namespace_and_with_collection_radio_buttons + post = Post.new + def post.active; false; end + + form_for(post, namespace: 'foo') do |f| + concat f.collection_radio_buttons(:active, [true, false], :to_s, :to_s) + end + + expected = whole_form("/posts", "foo_new_post", "new_post") do + "<input id='foo_post_active_true' name='post[active]' type='radio' value='true' />" + + "<label for='foo_post_active_true'>true</label>" + + "<input checked='checked' id='foo_post_active_false' name='post[active]' type='radio' value='false' />" + + "<label for='foo_post_active_false'>false</label>" + end + + assert_dom_equal expected, output_buffer + end + def test_form_for_with_collection_check_boxes post = Post.new def post.tag_ids; [1, 3]; end @@ -1361,6 +1379,24 @@ class FormHelperTest < ActionView::TestCase assert_dom_equal expected, output_buffer end + def test_form_with_namespace_and_with_collection_check_boxes + post = Post.new + def post.tag_ids; [1]; end + collection = [[1, "Tag 1"]] + + form_for(post, namespace: 'foo') do |f| + concat f.collection_check_boxes(:tag_ids, collection, :first, :last) + end + + expected = whole_form("/posts", "foo_new_post", "new_post") do + "<input checked='checked' id='foo_post_tag_ids_1' name='post[tag_ids][]' type='checkbox' value='1' />" + + "<label for='foo_post_tag_ids_1'>Tag 1</label>" + + "<input name='post[tag_ids][]' type='hidden' value='' />" + end + + assert_dom_equal expected, output_buffer + end + def test_form_for_with_file_field_generate_multipart Post.send :attr_accessor, :file diff --git a/actionview/test/template/form_options_helper_test.rb b/actionview/test/template/form_options_helper_test.rb index a6977766d1..50e9d132a7 100644 --- a/actionview/test/template/form_options_helper_test.rb +++ b/actionview/test/template/form_options_helper_test.rb @@ -556,6 +556,21 @@ class FormOptionsHelperTest < ActionView::TestCase ) end + def test_select_under_fields_for_with_block + @post = Post.new + + output_buffer = fields_for :post, @post do |f| + concat(f.select(:category) do + concat content_tag(:option, "hello world") + end) + end + + assert_dom_equal( + "<select id=\"post_category\" name=\"post[category]\"><option>hello world</option></select>", + output_buffer + ) + end + def test_select_with_multiple_to_add_hidden_input output_buffer = select(:post, :category, "", {}, :multiple => true) assert_dom_equal( @@ -783,6 +798,22 @@ class FormOptionsHelperTest < ActionView::TestCase ) end + def test_select_not_existing_method_with_selected_value + @post = Post.new + assert_dom_equal( + "<select id=\"post_locale\" name=\"post[locale]\"><option value=\"en\">en</option>\n<option value=\"ru\" selected=\"selected\">ru</option></select>", + select("post", "locale", %w( en ru ), :selected => 'ru') + ) + end + + def test_select_with_prompt_and_selected_value + @post = Post.new + assert_dom_equal( + "<select id=\"post_category\" name=\"post[category]\"><option value=\"one\">one</option>\n<option selected=\"selected\" value=\"two\">two</option></select>", + select("post", "category", %w( one two ), :selected => 'two', :prompt => true) + ) + end + def test_select_with_disabled_array @post = Post.new @post.category = "<mus>" diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 3d3c61ed1c..eb21b69163 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,14 @@ +* Updated the `ActiveModel::Dirty#changed_attributes` method to be indifferent between using + symbols and strings as keys. + + *William Myers* + +* Added new API methods `reset_changes` and `changed_applied` to `ActiveModel::Dirty` + that control changes state. Previsously you needed to update internal + instance variables, but now API methods are available. + + *Bogdan Gusiev* + * Fix has_secure_password. `password_confirmation` validations are triggered even if no `password_confirmation` is set. diff --git a/activemodel/examples/validations.rb b/activemodel/examples/validations.rb index c94cd17e18..b8e74acd5e 100644 --- a/activemodel/examples/validations.rb +++ b/activemodel/examples/validations.rb @@ -1,3 +1,4 @@ +require File.expand_path('../../../load_paths', __FILE__) require 'active_model' class Person diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index ea5ddf71de..c5f1b3f11a 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -14,13 +14,9 @@ module ActiveModel # track. # * Call <tt>attr_name_will_change!</tt> before each change to the tracked # attribute. - # - # If you wish to also track previous changes on save or update, you need to - # add: - # - # @previously_changed = changes - # - # inside of your save or update method. + # * Call <tt>changes_applied</tt> after the changes are persisted. + # * Call <tt>reset_changes</tt> when you want to reset the changes + # information. # # A minimal implementation could be: # @@ -39,8 +35,12 @@ module ActiveModel # end # # def save - # @previously_changed = changes - # @changed_attributes.clear + # # do persistence work + # changes_applied + # end + # + # def reload! + # reset_changes # end # end # @@ -65,6 +65,12 @@ module ActiveModel # person.changed? # => false # person.name_changed? # => false # + # Reset the changes: + # + # person.previous_changes # => {"name" => ["Uncle Bob", "Bill"]} + # person.reload! + # person.previous_changes # => {} + # # Assigning the same value leaves the attribute unchanged: # # person.name = 'Bill' @@ -129,7 +135,7 @@ module ActiveModel # person.save # person.previous_changes # => {"name" => ["bob", "robert"]} def previous_changes - @previously_changed + @previously_changed ||= {} end # Returns a hash of the attributes with unsaved changes indicating their original @@ -139,7 +145,7 @@ module ActiveModel # person.name = 'robert' # person.changed_attributes # => {"name" => "bob"} def changed_attributes - @changed_attributes ||= {} + @changed_attributes ||= ActiveSupport::HashWithIndifferentAccess.new end # Handle <tt>*_changed?</tt> for +method_missing+. @@ -154,6 +160,18 @@ module ActiveModel private + # Removes current changes and makes them accessible through +previous_changes+. + def changes_applied + @previously_changed = changes + @changed_attributes = {} + end + + # Removes all dirty data: current changes and previous changes + def reset_changes + @previously_changed = {} + @changed_attributes = {} + end + # Handle <tt>*_change</tt> for +method_missing+. def attribute_change(attr) [changed_attributes[attr], __send__(attr)] if attribute_changed?(attr) diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 8b9ac97bbb..17fafe4be9 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -20,9 +20,9 @@ module ActiveModel # value to the password_confirmation attribute and the validation # will not be triggered. # - # You need to add bcrypt-ruby (~> 3.1.0) to Gemfile to use #has_secure_password: + # You need to add bcrypt-ruby (~> 3.1.2) to Gemfile to use #has_secure_password: # - # gem 'bcrypt-ruby', '~> 3.1.0' + # gem 'bcrypt-ruby', '~> 3.1.2' # # Example using Active Record (which automatically includes ActiveModel::SecurePassword): # @@ -46,7 +46,7 @@ module ActiveModel # This is to avoid ActiveModel (and by extension the entire framework) # being dependent on a binary library. begin - gem 'bcrypt-ruby', '~> 3.1.0' + gem 'bcrypt-ruby', '~> 3.1.2' require 'bcrypt' rescue LoadError $stderr.puts "You don't have bcrypt-ruby installed in your application. Please add it to your Gemfile and run bundle install" diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb index ba45089cca..a90d0b1299 100644 --- a/activemodel/test/cases/dirty_test.rb +++ b/activemodel/test/cases/dirty_test.rb @@ -3,11 +3,12 @@ require "cases/helper" class DirtyTest < ActiveModel::TestCase class DirtyModel include ActiveModel::Dirty - define_attribute_methods :name, :color + define_attribute_methods :name, :color, :size def initialize @name = nil @color = nil + @size = nil end def name @@ -28,9 +29,17 @@ class DirtyTest < ActiveModel::TestCase @color = val end + def size + @size + end + + def size=(val) + attribute_will_change!(:size) unless val == @size + @size = val + end + def save - @previously_changed = changes - @changed_attributes.clear + changes_applied end end @@ -125,4 +134,9 @@ class DirtyTest < ActiveModel::TestCase assert_equal ["Otto", "Mr. Manfredgensonton"], @model.name_change assert_equal @model.name_was, "Otto" end + + test "using attribute_will_change! with a symbol" do + @model.size = 1 + assert @model.size_changed? + end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 474b11f537..83c94caa53 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,66 @@ +* `has_and_belongs_to_many` is now transparently implemented in terms of + `has_many :through`. Behavior should remain the same, if not, it is a bug. + +* `create_savepoint`, `rollback_to_savepoint` and `release_savepoint` accept + a savepoint name. + + *Yves Senn* + +* Make `next_migration_number` accessible for third party generators. + + *Yves Senn* + +* Objects instantiated using a null relationship will now retain the + attributes of the where clause. + + Fixes #11676, #11675, #11376. + + *Paul Nikitochkin*, *Peter Brown*, *Nthalk* + +* Fixed `ActiveRecord::Associations::CollectionAssociation#find` + when using `has_many` association with `:inverse_of` and finding an array of one element, + it should return an array of one element too. + + *arthurnn* + +* Callbacks on has_many should access the in memory parent if a inverse_of is set. + + *arthurnn* + +* `ActiveRecord::ConnectionAdapters.string_to_time` respects + string with timezone (e.g. Wed, 04 Sep 2013 20:30:00 JST). + + Fixes #12278. + + *kennyj* + +* Calling `update_attributes` will now throw an `ArgumentError` whenever it + gets a `nil` argument. More specifically, it will throw an error if the + argument that it gets passed does not respond to to `stringify_keys`. + + Example: + + @my_comment.update_attributes(nil) # => raises ArgumentError + + *John Wang* + +* Deprecate `quoted_locking_column` method, which isn't used anywhere. + + *kennyj* + +* Migration dump UUID default functions to schema.rb. + + Fixes #10751. + + *kennyj* + +* Fixed a bug in `ActiveRecord::Associations::CollectionAssociation#find_by_scan` + when using `has_many` association with `:inverse_of` option and UUID primary key. + + Fixes #10450. + + *kennyj* + * ActiveRecord::Base#<=> has been removed. Primary keys may not be in order, or even be numbers, so sorting by id doesn't make sense. Please use `sort_by` and specify the attribute you wish to sort with. For example, change: @@ -8,11 +71,13 @@ Post.all.to_a.sort_by(&:id) + *Aaron Patterson* + * Fix: joins association, with defined in the scope block constraints by using several where constraints and at least of them is not `Arel::Nodes::Equality`, generates invalid SQL expression. - Fixes: #11963 + Fixes #11963. *Paul Nikitochkin* @@ -185,13 +250,13 @@ to allow the connection adapter to properly determine how to quote the value. This was affecting certain databases that use specific column types. - Fixes: #6763 + Fixes #6763. *Alfred Wong* * rescue from all exceptions in `ConnectionManagement#call` - Fixes #11497 + Fixes #11497. As `ActiveRecord::ConnectionAdapters::ConnectionManagement` middleware does not rescue from Exception (but only from StandardError), the Connection @@ -213,7 +278,7 @@ * Remove extra decrement of transaction deep level. - Fixes: #4566 + Fixes #4566. *Paul Nikitochkin* @@ -233,7 +298,7 @@ * Remove extra select and update queries on save/touch/destroy ActiveRecord model with belongs to reflection with option `touch: true`. - Fixes: #11288 + Fixes #11288. *Paul Nikitochkin* diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 33cbafc6aa..74e2774626 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -73,12 +73,6 @@ module ActiveRecord end end - class HasAndBelongsToManyAssociationForeignKeyNeeded < ActiveRecordError #:nodoc: - def initialize(reflection) - super("Cannot create self referential has_and_belongs_to_many association on '#{reflection.class_name rescue nil}##{reflection.name rescue nil}'. :association_foreign_key cannot be the same as the :foreign_key.") - end - end - class EagerLoadPolymorphicError < ActiveRecordError #:nodoc: def initialize(reflection) super("Can not eagerly load the polymorphic association #{reflection.name.inspect}") @@ -114,7 +108,6 @@ module ActiveRecord autoload :BelongsToAssociation, 'active_record/associations/belongs_to_association' autoload :BelongsToPolymorphicAssociation, 'active_record/associations/belongs_to_polymorphic_association' - autoload :HasAndBelongsToManyAssociation, 'active_record/associations/has_and_belongs_to_many_association' autoload :HasManyAssociation, 'active_record/associations/has_many_association' autoload :HasManyThroughAssociation, 'active_record/associations/has_many_through_association' autoload :HasOneAssociation, 'active_record/associations/has_one_association' @@ -1560,8 +1553,39 @@ module ActiveRecord # has_and_belongs_to_many :categories, join_table: "prods_cats" # has_and_belongs_to_many :categories, -> { readonly } def has_and_belongs_to_many(name, scope = nil, options = {}, &extension) - reflection = Builder::HasAndBelongsToMany.build(self, name, scope, options, &extension) - Reflection.add_reflection self, name, reflection + if scope.is_a?(Hash) + options = scope + scope = nil + end + + builder = Builder::HasAndBelongsToMany.new name, self, options + + join_model = builder.through_model + + middle_reflection = builder.middle_reflection join_model + + Builder::HasMany.define_callbacks self, middle_reflection + Reflection.add_reflection self, middle_reflection.name, middle_reflection + + include Module.new { + class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def destroy_associations + association(:#{middle_reflection.name}).delete_all(:delete_all) + association(:#{name}).reset + super + end + RUBY + } + + hm_options = {} + hm_options[:through] = middle_reflection.name + hm_options[:source] = join_model.right_reflection.name + + [:before_add, :after_add, :before_remove, :after_remove].each do |k| + hm_options[k] = options[k] if options.key? k + end + + has_many name, scope, hm_options, &extension end end end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 67d24b35d1..04c36d5740 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -13,7 +13,6 @@ module ActiveRecord # BelongsToAssociation # BelongsToPolymorphicAssociation # CollectionAssociation - # HasAndBelongsToManyAssociation # HasManyAssociation # HasManyThroughAssociation + ThroughAssociation class Association #:nodoc: diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 8027acfb83..d862a5f29d 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -44,18 +44,6 @@ module ActiveRecord chain.each_with_index do |reflection, i| table, foreign_table = tables.shift, tables.first - if reflection.source_macro == :has_and_belongs_to_many - join_table = tables.shift - - scope = scope.joins(join( - join_table, - table[reflection.association_primary_key]. - eq(join_table[reflection.association_foreign_key]) - )) - - table, foreign_table = join_table, tables.first - end - if reflection.source_macro == :belongs_to if reflection.options[:polymorphic] key = reflection.association_primary_key(self.klass) diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 1059fc032d..fe8274e1d8 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -8,7 +8,6 @@ # - HasOneAssociation # - CollectionAssociation # - HasManyAssociation -# - HasAndBelongsToManyAssociation module ActiveRecord::Associations::Builder class Association #:nodoc: @@ -22,6 +21,15 @@ module ActiveRecord::Associations::Builder attr_reader :name, :scope, :options def self.build(model, name, scope, options, &block) + builder = create_builder model, name, scope, options, &block + reflection = builder.build(model) + define_accessors model, reflection + define_callbacks model, reflection + builder.define_extensions model + reflection + end + + def self.create_builder(model, name, scope, options, &block) raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol) if scope.is_a?(Hash) @@ -29,12 +37,7 @@ module ActiveRecord::Associations::Builder scope = nil end - builder = new(name, scope, options, &block) - reflection = builder.build(model) - builder.define_accessors model - builder.define_callbacks model, reflection - builder.define_extensions model - reflection + new(name, scope, options, &block) end def initialize(name, scope, options) @@ -68,8 +71,8 @@ module ActiveRecord::Associations::Builder def define_extensions(model) end - def define_callbacks(model, reflection) - add_before_destroy_callbacks(model, name) if options[:dependent] + def self.define_callbacks(model, reflection) + add_before_destroy_callbacks(model, reflection) if reflection.options[:dependent] Association.extensions.each do |extension| extension.build model, reflection end @@ -81,14 +84,14 @@ module ActiveRecord::Associations::Builder # end # # Post.first.comments and Post.first.comments= methods are defined by this method... - - def define_accessors(model) + def self.define_accessors(model, reflection) mixin = model.generated_feature_methods - define_readers(mixin) - define_writers(mixin) + name = reflection.name + define_readers(mixin, name) + define_writers(mixin, name) end - def define_readers(mixin) + def self.define_readers(mixin, name) mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 def #{name}(*args) association(:#{name}).reader(*args) @@ -96,7 +99,7 @@ module ActiveRecord::Associations::Builder CODE end - def define_writers(mixin) + def self.define_writers(mixin, name) mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 def #{name}=(value) association(:#{name}).writer(value) @@ -104,17 +107,18 @@ module ActiveRecord::Associations::Builder CODE end - def valid_dependent_options + def self.valid_dependent_options raise NotImplementedError end private - def add_before_destroy_callbacks(model, name) - unless valid_dependent_options.include? options[:dependent] - raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{options[:dependent]}" + def self.add_before_destroy_callbacks(model, reflection) + unless valid_dependent_options.include? reflection.options[:dependent] + raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{reflection.options[:dependent]}" end + name = reflection.name model.before_destroy lambda { |o| o.association(name).handle_dependency } end end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 4e88b50ec5..c0b1847f33 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -8,43 +8,39 @@ module ActiveRecord::Associations::Builder super + [:foreign_type, :polymorphic, :touch] end - def constructable? - !options[:polymorphic] - end - - def valid_dependent_options + def self.valid_dependent_options [:destroy, :delete] end - def define_callbacks(model, reflection) + def self.define_callbacks(model, reflection) super - add_counter_cache_callbacks(model, reflection) if options[:counter_cache] - add_touch_callbacks(model, reflection) if options[:touch] + add_counter_cache_callbacks(model, reflection) if reflection.options[:counter_cache] + add_touch_callbacks(model, reflection) if reflection.options[:touch] end - def define_accessors(mixin) + def self.define_accessors(mixin, reflection) super add_counter_cache_methods mixin end private - def add_counter_cache_methods(mixin) + def self.add_counter_cache_methods(mixin) return if mixin.method_defined? :belongs_to_counter_cache_after_create mixin.class_eval do - def belongs_to_counter_cache_after_create(association, reflection) - if record = send(association.name) + def belongs_to_counter_cache_after_create(reflection) + if record = send(reflection.name) cache_column = reflection.counter_cache_column record.class.increment_counter(cache_column, record.id) @_after_create_counter_called = true end end - def belongs_to_counter_cache_before_destroy(association, reflection) + def belongs_to_counter_cache_before_destroy(reflection) foreign_key = reflection.foreign_key.to_sym unless destroyed_by_association && destroyed_by_association.foreign_key.to_sym == foreign_key - record = send association.name + record = send reflection.name if record && !self.destroyed? cache_column = reflection.counter_cache_column record.class.decrement_counter(cache_column, record.id) @@ -52,13 +48,13 @@ module ActiveRecord::Associations::Builder end end - def belongs_to_counter_cache_after_update(association, reflection) + def belongs_to_counter_cache_after_update(reflection) foreign_key = reflection.foreign_key cache_column = reflection.counter_cache_column if (@_after_create_counter_called ||= false) @_after_create_counter_called = false - elsif attribute_changed?(foreign_key) && !new_record? && association.constructable? + elsif attribute_changed?(foreign_key) && !new_record? && reflection.constructable? model = reflection.klass foreign_key_was = attribute_was foreign_key foreign_key = attribute foreign_key @@ -74,20 +70,19 @@ module ActiveRecord::Associations::Builder end end - def add_counter_cache_callbacks(model, reflection) + def self.add_counter_cache_callbacks(model, reflection) cache_column = reflection.counter_cache_column - association = self model.after_create lambda { |record| - record.belongs_to_counter_cache_after_create(association, reflection) + record.belongs_to_counter_cache_after_create(reflection) } model.before_destroy lambda { |record| - record.belongs_to_counter_cache_before_destroy(association, reflection) + record.belongs_to_counter_cache_before_destroy(reflection) } model.after_update lambda { |record| - record.belongs_to_counter_cache_after_update(association, reflection) + record.belongs_to_counter_cache_after_update(reflection) } klass = reflection.class_name.safe_constantize @@ -120,10 +115,10 @@ module ActiveRecord::Associations::Builder end end - def add_touch_callbacks(model, reflection) + def self.add_touch_callbacks(model, reflection) foreign_key = reflection.foreign_key - n = name - touch = options[:touch] + n = reflection.name + touch = reflection.options[:touch] callback = lambda { |record| BelongsTo.touch_record(record, foreign_key, n, touch) diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index 7bd0687c0b..d68f5d6792 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -1,4 +1,4 @@ -# This class is inherited by the has_many and has_many_and_belongs_to_many association classes +# This class is inherited by the has_many and has_many_and_belongs_to_many association classes require 'active_record/associations' @@ -23,9 +23,13 @@ module ActiveRecord::Associations::Builder end end - def define_callbacks(model, reflection) + def self.define_callbacks(model, reflection) super - CALLBACKS.each { |callback_name| define_callback(model, callback_name) } + name = reflection.name + options = reflection.options + CALLBACKS.each { |callback_name| + define_callback(model, callback_name, name, options) + } end def define_extensions(model) @@ -35,7 +39,7 @@ module ActiveRecord::Associations::Builder end end - def define_callback(model, callback_name) + def self.define_callback(model, callback_name, name, options) full_callback_name = "#{callback_name}_for_#{name}" # TODO : why do i need method_defined? I think its because of the inheritance chain @@ -54,8 +58,7 @@ module ActiveRecord::Associations::Builder end # Defines the setter and getter methods for the collection_singular_ids. - - def define_readers(mixin) + def self.define_readers(mixin, name) super mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 @@ -65,7 +68,7 @@ module ActiveRecord::Associations::Builder CODE end - def define_writers(mixin) + def self.define_writers(mixin, name) super mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 55ec3bf4f1..af596a3a64 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -1,24 +1,121 @@ module ActiveRecord::Associations::Builder - class HasAndBelongsToMany < CollectionAssociation #:nodoc: - def macro - :has_and_belongs_to_many + class HasAndBelongsToMany # :nodoc: + class JoinTableResolver + KnownTable = Struct.new :join_table + + class KnownClass + def initialize(lhs_class, rhs_class_name) + @lhs_class = lhs_class + @rhs_class_name = rhs_class_name + @join_table = nil + end + + def join_table + @join_table ||= [@lhs_class.table_name, klass.table_name].sort.join("\0").gsub(/^(.*_)(.+)\0\1(.+)/, '\1\2_\3').gsub("\0", "_") + end + + private + def klass; @rhs_class_name.constantize; end + end + + def self.build(lhs_class, name, options) + if options[:join_table] + KnownTable.new options[:join_table] + else + class_name = options.fetch(:class_name) { + name.to_s.camelize.singularize + } + KnownClass.new lhs_class, class_name + end + end end - def valid_options - super + [:join_table, :association_foreign_key] + attr_reader :lhs_model, :association_name, :options + + def initialize(association_name, lhs_model, options) + @association_name = association_name + @lhs_model = lhs_model + @options = options end - def define_callbacks(model, reflection) - super - name = self.name - model.send(:include, Module.new { - class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def destroy_associations - association(:#{name}).delete_all - super - end - RUBY - }) + def through_model + habtm = JoinTableResolver.build lhs_model, association_name, options + + join_model = Class.new(ActiveRecord::Base) { + class << self; + attr_accessor :class_resolver + attr_accessor :name + attr_accessor :table_name_resolver + attr_accessor :left_reflection + attr_accessor :right_reflection + end + + def self.table_name + table_name_resolver.join_table + end + + def self.compute_type(class_name) + class_resolver.compute_type class_name + end + + def self.add_left_association(name, options) + belongs_to name, options + self.left_reflection = reflect_on_association(name) + end + + def self.add_right_association(name, options) + rhs_name = name.to_s.singularize.to_sym + belongs_to rhs_name, options + self.right_reflection = reflect_on_association(rhs_name) + end + + } + + join_model.name = "HABTM_#{association_name.to_s.camelize}" + join_model.table_name_resolver = habtm + join_model.class_resolver = lhs_model + + join_model.add_left_association :left_side, class: lhs_model + join_model.add_right_association association_name, belongs_to_options(options) + join_model + end + + def middle_reflection(join_model) + middle_name = [lhs_model.name.downcase.pluralize, + association_name].join('_').gsub(/::/, '_').to_sym + middle_options = middle_options join_model + hm_builder = HasMany.create_builder(lhs_model, + middle_name, + nil, + middle_options) + hm_builder.build lhs_model + end + + private + + def middle_options(join_model) + middle_options = {} + middle_options[:class] = join_model + middle_options[:source] = join_model.left_reflection.name + if options.key? :foreign_key + middle_options[:foreign_key] = options[:foreign_key] + end + middle_options + end + + def belongs_to_options(options) + rhs_options = {} + + if options.key? :class_name + rhs_options[:foreign_key] = options[:class_name].foreign_key + rhs_options[:class_name] = options[:class_name] + end + + if options.key? :association_foreign_key + rhs_options[:foreign_key] = options[:association_foreign_key] + end + + rhs_options end end end diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index a60cb4769a..7909b93622 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -8,7 +8,7 @@ module ActiveRecord::Associations::Builder super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :counter_cache] end - def valid_dependent_options + def self.valid_dependent_options [:destroy, :delete_all, :nullify, :restrict_with_error, :restrict_with_exception] end end diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index 62d454ce55..f359efd496 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -10,18 +10,14 @@ module ActiveRecord::Associations::Builder valid end - def constructable? - !options[:through] - end - - def valid_dependent_options + def self.valid_dependent_options [:destroy, :delete, :nullify, :restrict_with_error, :restrict_with_exception] end private - def add_before_destroy_callbacks(model, name) - super unless options[:through] + def self.add_before_destroy_callbacks(model, reflection) + super unless reflection.options[:through] end end end diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index d97c0e9afd..9a25980be8 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -1,4 +1,4 @@ -# This class is inherited by the has_one and belongs_to association classes +# This class is inherited by the has_one and belongs_to association classes module ActiveRecord::Associations::Builder class SingularAssociation < Association #:nodoc: @@ -6,18 +6,13 @@ module ActiveRecord::Associations::Builder super + [:remote, :dependent, :counter_cache, :primary_key, :inverse_of] end - def constructable? - true - end - - def define_accessors(model) + def self.define_accessors(model, reflection) super - define_constructors(model.generated_feature_methods) if constructable? + define_constructors(model.generated_feature_methods, reflection.name) if reflection.constructable? end # Defines the (build|create)_association methods for belongs_to or has_one association - - def define_constructors(mixin) + def self.define_constructors(mixin, name) mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 def build_#{name}(*args, &block) association(:#{name}).build(*args, &block) diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index e32c0b0377..75f8990cf0 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -7,7 +7,6 @@ module ActiveRecord # collections. See the class hierarchy in AssociationProxy. # # CollectionAssociation: - # HasAndBelongsToManyAssociation => has_and_belongs_to_many # HasManyAssociation => has_many # HasManyThroughAssociation + ThroughAssociation => has_many :through # @@ -80,14 +79,13 @@ module ActiveRecord load_target.find(*args) { |*block_args| yield(*block_args) } else if options[:inverse_of] && loaded? - args = args.flatten - raise RecordNotFound, "Couldn't find #{scope.klass.name} without an ID" if args.blank? - + args_flatten = args.flatten + raise RecordNotFound, "Couldn't find #{scope.klass.name} without an ID" if args_flatten.blank? result = find_by_scan(*args) result_size = Array(result).size - if !result || result_size != args.size - scope.raise_record_not_found_exception!(args, result_size, args.size) + if !result || result_size != args_flatten.size + scope.raise_record_not_found_exception!(args_flatten, result_size, args_flatten.size) else result end @@ -554,14 +552,14 @@ module ActiveRecord # specified, then #find scans the entire collection. def find_by_scan(*args) expects_array = args.first.kind_of?(Array) - ids = args.flatten.compact.map{ |arg| arg.to_i }.uniq + ids = args.flatten.compact.map{ |arg| arg.to_s }.uniq if ids.size == 1 id = ids.first - record = load_target.detect { |r| id == r.id } + record = load_target.detect { |r| id == r.id.to_s } expects_array ? [ record ] : record else - load_target.select { |r| ids.include?(r.id) } + load_target.select { |r| ids.include?(r.id.to_s) } end end diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb deleted file mode 100644 index b2e6c708bf..0000000000 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ /dev/null @@ -1,56 +0,0 @@ -module ActiveRecord - # = Active Record Has And Belongs To Many Association - module Associations - class HasAndBelongsToManyAssociation < CollectionAssociation #:nodoc: - attr_reader :join_table - - def initialize(owner, reflection) - @join_table = Arel::Table.new(reflection.join_table) - super - end - - def insert_record(record, validate = true, raise = false) - if record.new_record? - if raise - record.save!(:validate => validate) - else - return unless record.save(:validate => validate) - end - end - - stmt = join_table.compile_insert( - join_table[reflection.foreign_key] => owner.id, - join_table[reflection.association_foreign_key] => record.id - ) - - owner.class.connection.insert stmt - - record - end - - private - - def count_records - load_target.size - end - - def delete_records(records, method) - relation = join_table - condition = relation[reflection.foreign_key].eq(owner.id) - - unless records == :all - condition = condition.and( - relation[reflection.association_foreign_key] - .in(records.map { |x| x.id }.compact) - ) - end - - owner.class.connection.delete(relation.where(condition).compile_delete) - end - - def invertible_for?(record) - false - end - end - end -end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index a3fcca8a27..0a23109b9b 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -32,6 +32,7 @@ module ActiveRecord def insert_record(record, validate = true, raise = false) set_owner_attributes(record) + set_inverse_instance(record) if raise record.save!(:validate => validate) diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 5aa17e5fbb..f12d5c49b5 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -1,11 +1,34 @@ module ActiveRecord module Associations class JoinDependency # :nodoc: - autoload :JoinPart, 'active_record/associations/join_dependency/join_part' autoload :JoinBase, 'active_record/associations/join_dependency/join_base' autoload :JoinAssociation, 'active_record/associations/join_dependency/join_association' - attr_reader :join_parts, :reflections, :alias_tracker, :base_klass + attr_reader :join_parts, :alias_tracker, :base_klass + + def self.make_tree(associations) + hash = {} + walk_tree associations, hash + hash + end + + def self.walk_tree(associations, hash) + case associations + when Symbol, String + hash[associations.to_sym] ||= {} + when Array + associations.each do |assoc| + walk_tree assoc, hash + end + when Hash + associations.each do |k,v| + cache = hash[k] ||= {} + walk_tree v, cache + end + else + raise ConfigurationError, associations.inspect + end + end # base is the base class on which operation is taking place. # associations is the list of associations which are joined using hash, symbol or array. @@ -32,18 +55,23 @@ module ActiveRecord @base_klass = base @table_joins = joins @join_parts = [JoinBase.new(base)] - @associations = {} - @reflections = [] @alias_tracker = AliasTracker.new(base.connection, joins) @alias_tracker.aliased_name_for(base.table_name) # Updates the count for base.table_name to 1 - build(associations) + tree = self.class.make_tree associations + build tree, join_parts.last, Arel::InnerJoin end def graft(*associations) - associations.each do |association| - join_associations.detect {|a| association == a} || - build(association.reflection.name, association.find_parent_in(self) || join_base, association.join_type) - end + join_assocs = join_associations + base = join_base + + associations.reject { |association| + join_assocs.detect { |a| association == a } + }.each { |association| + join_part = find_parent_part(association.parent) || base + type = association.join_type + find_or_build_scalar association.reflection, join_part, type + } self end @@ -51,8 +79,8 @@ module ActiveRecord join_parts.drop 1 end - def join_base - join_parts.first + def reflections + join_associations.map(&:reflection) end def join_relation(relation) @@ -70,101 +98,107 @@ module ActiveRecord }.flatten end - def instantiate(rows) + def instantiate(result_set) primary_key = join_base.aliased_primary_key parents = {} - records = rows.map { |model| - primary_id = model[primary_key] - parent = parents[primary_id] ||= join_base.instantiate(model) - construct(parent, @associations, join_associations, model) + type_caster = result_set.column_type primary_key + assoc = associations + + records = result_set.map { |row_hash| + primary_id = type_caster.type_cast row_hash[primary_key] + parent = parents[primary_id] ||= join_base.instantiate(row_hash) + construct(parent, assoc, join_associations, row_hash, result_set) parent }.uniq - remove_duplicate_results!(base_klass, records, @associations) + remove_duplicate_results!(base_klass, records, assoc) records end - protected + private + + def associations + join_associations.each_with_object({}) do |assoc, tree| + cache_joined_association assoc, tree + end + end + + def find_parent_part(parent) + join_parts.detect do |join_part| + case parent + when JoinBase + parent.base_klass == join_part.base_klass + else + parent == join_part + end + end + end + + def join_base + join_parts.first + end def remove_duplicate_results!(base, records, associations) - case associations - when Symbol, String - reflection = base.reflections[associations] + associations.each_key do |name| + reflection = base.reflect_on_association(name) remove_uniq_by_reflection(reflection, records) - when Array - associations.each do |association| - remove_duplicate_results!(base, records, association) - end - when Hash - associations.each_key do |name| - reflection = base.reflections[name] - remove_uniq_by_reflection(reflection, records) - - parent_records = [] - records.each do |record| - if descendant = record.send(reflection.name) - if reflection.collection? - parent_records.concat descendant.target.uniq - else - parent_records << descendant - end + + parent_records = [] + records.each do |record| + if descendant = record.send(reflection.name) + if reflection.collection? + parent_records.concat descendant.target.uniq + else + parent_records << descendant end end + end - remove_duplicate_results!(reflection.klass, parent_records, associations[name]) unless parent_records.empty? + unless parent_records.empty? + remove_duplicate_results!(reflection.klass, parent_records, associations[name]) end end end - def cache_joined_association(association) + def cache_joined_association(association, tree) associations = [] parent = association.parent while parent != join_base associations.unshift(parent.reflection.name) parent = parent.parent end - ref = @associations - associations.each do |key| - ref = ref[key] + ref = associations.inject(tree) do |cache,key| + cache[key] end ref[association.reflection.name] ||= {} end - def build(associations, parent = join_parts.last, join_type = Arel::InnerJoin) - case associations - when Symbol, String - reflection = parent.reflections[associations.intern] or - raise ConfigurationError, "Association named '#{ associations }' was not found on #{ parent.base_klass.name }; perhaps you misspelled it?" - unless join_association = find_join_association(reflection, parent) - @reflections << reflection - join_association = build_join_association(reflection, parent) - join_association.join_type = join_type - @join_parts << join_association - cache_joined_association(join_association) - end - join_association - when Array - associations.each do |association| - build(association, parent, join_type) - end - when Hash - associations.keys.sort_by { |a| a.to_s }.each do |name| - join_association = build(name, parent, join_type) - build(associations[name], join_association, join_type) - end - else - raise ConfigurationError, associations.inspect + def find_reflection(klass, name) + klass.reflect_on_association(name.intern) or + raise ConfigurationError, "Association named '#{ name }' was not found on #{ klass.name }; perhaps you misspelled it?" + end + + def build(associations, parent, join_type) + associations.each do |name, right| + reflection = find_reflection parent.base_klass, name + join_association = build_join_association reflection, parent, join_type + @join_parts << join_association + build right, join_association, join_type end end - def find_join_association(name_or_reflection, parent) - if String === name_or_reflection - name_or_reflection = name_or_reflection.to_sym + def find_or_build_scalar(reflection, parent, join_type) + unless join_association = find_join_association(reflection, parent) + join_association = build_join_association(reflection, parent, join_type) + @join_parts << join_association end + join_association + end + def find_join_association(reflection, parent) join_associations.detect { |j| - j.reflection == name_or_reflection && j.parent == parent + j.reflection == reflection && j.parent == parent } end @@ -174,39 +208,42 @@ module ActiveRecord end end - def build_join_association(reflection, parent) - JoinAssociation.new(reflection, self, parent) + def build_join_association(reflection, parent, join_type) + reflection.check_validity! + + if reflection.options[:polymorphic] + raise EagerLoadPolymorphicError.new(reflection) + end + + JoinAssociation.new(reflection, join_parts.length, parent, join_type, alias_tracker) end - def construct(parent, associations, join_parts, row) - case associations - when Symbol, String - name = associations.to_s + def construct(parent, associations, join_parts, row, rs) + associations.sort_by { |k,_| k.to_s }.each do |association_name, assoc| + association = construct_scalar(parent, association_name, join_parts, row, rs) + construct(association, assoc, join_parts, row, rs) if association + end + end - join_part = join_parts.detect { |j| - j.reflection.name.to_s == name && - j.parent_table_name == parent.class.table_name } + def construct_scalar(parent, associations, join_parts, row, rs) + name = associations.to_s - raise(ConfigurationError, "No such association") unless join_part + join_part = join_parts.detect { |j| + j.reflection.name.to_s == name && + j.parent_table_name == parent.class.table_name + } - join_parts.delete(join_part) - construct_association(parent, join_part, row) - when Array - associations.each do |association| - construct(parent, association, join_parts, row) - end - when Hash - associations.sort_by { |k,_| k.to_s }.each do |association_name, assoc| - association = construct(parent, association_name, join_parts, row) - construct(association, assoc, join_parts, row) if association - end - else - raise ConfigurationError, associations.inspect - end + raise(ConfigurationError, "No such association") unless join_part + + join_parts.delete(join_part) + construct_association(parent, join_part, row, rs) end - def construct_association(record, join_part, row) - return if record.id.to_s != join_part.parent.record_id(row).to_s + def construct_association(record, join_part, row, rs) + caster = rs.column_type(join_part.parent.aliased_primary_key) + row_id = caster.type_cast row[join_part.parent.aliased_primary_key] + + return if record.id != row_id macro = join_part.reflection.macro if macro == :has_one @@ -216,7 +253,7 @@ module ActiveRecord else association = join_part.instantiate(row) unless row[join_part.aliased_primary_key].nil? case macro - when :has_many, :has_and_belongs_to_many + when :has_many other = record.association(join_part.reflection.name) other.loaded! other.target.push(association) if association diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 58fc00d811..f6a04ef9f1 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -1,3 +1,5 @@ +require 'active_record/associations/join_dependency/join_part' + module ActiveRecord module Associations class JoinDependency # :nodoc: @@ -7,11 +9,6 @@ module ActiveRecord # The reflection of the association represented attr_reader :reflection - # The JoinDependency object which this JoinAssociation exists within. This is mainly - # relevant for generating aliases which do not conflict with other joins which are - # part of the query. - attr_reader :join_dependency - # A JoinBase instance representing the active record we are joining onto. # (So in Author.has_many :posts, the Author would be that base record.) attr_reader :parent @@ -23,24 +20,18 @@ module ActiveRecord attr_reader :aliased_prefix attr_reader :tables + attr_reader :alias_tracker delegate :options, :through_reflection, :source_reflection, :chain, :to => :reflection - delegate :alias_tracker, :to => :join_dependency - - def initialize(reflection, join_dependency, parent = nil) - reflection.check_validity! - - if reflection.options[:polymorphic] - raise EagerLoadPolymorphicError.new(reflection) - end + def initialize(reflection, index, parent, join_type, alias_tracker) super(reflection.klass) @reflection = reflection - @join_dependency = join_dependency + @alias_tracker = alias_tracker @parent = parent - @join_type = Arel::InnerJoin - @aliased_prefix = "t#{ join_dependency.join_parts.size }" + @join_type = join_type + @aliased_prefix = "t#{ index }" @tables = construct_tables.reverse end @@ -53,17 +44,6 @@ module ActiveRecord other.parent == parent end - def find_parent_in(other_join_dependency) - other_join_dependency.join_parts.detect do |join_part| - case parent - when JoinBase - parent.base_klass == join_part.base_klass - else - parent == join_part - end - end - end - def join_constraints joins = [] tables = @tables.dup @@ -83,17 +63,6 @@ module ActiveRecord when :belongs_to key = reflection.association_primary_key foreign_key = reflection.foreign_key - when :has_and_belongs_to_many - # Join the join table first... - joins << join( - table, - table[reflection.foreign_key]. - eq(foreign_table[reflection.active_record_primary_key])) - - foreign_table, table = table, tables.shift - - key = reflection.association_primary_key - foreign_key = reflection.association_foreign_key else key = reflection.foreign_key foreign_key = reflection.active_record_primary_key diff --git a/activerecord/lib/active_record/associations/join_dependency/join_base.rb b/activerecord/lib/active_record/associations/join_dependency/join_base.rb index a7dacdbfd6..c689d06594 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_base.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_base.rb @@ -1,3 +1,5 @@ +require 'active_record/associations/join_dependency/join_part' + module ActiveRecord module Associations class JoinDependency # :nodoc: diff --git a/activerecord/lib/active_record/associations/join_dependency/join_part.rb b/activerecord/lib/active_record/associations/join_dependency/join_part.rb index 8024105472..2b6034ca9d 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_part.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_part.rb @@ -13,7 +13,7 @@ module ActiveRecord # association. attr_reader :base_klass - delegate :table_name, :column_names, :primary_key, :reflections, :arel_engine, :to => :base_klass + delegate :table_name, :column_names, :primary_key, :arel_engine, :to => :base_klass def initialize(base_klass) @base_klass = base_klass diff --git a/activerecord/lib/active_record/associations/join_helper.rb b/activerecord/lib/active_record/associations/join_helper.rb index 27b70edf1a..f345d16841 100644 --- a/activerecord/lib/active_record/associations/join_helper.rb +++ b/activerecord/lib/active_record/associations/join_helper.rb @@ -10,21 +10,12 @@ module ActiveRecord private def construct_tables - tables = [] - chain.each do |reflection| - tables << alias_tracker.aliased_table_for( + chain.map do |reflection| + alias_tracker.aliased_table_for( table_name_for(reflection), table_alias_for(reflection, reflection != self.reflection) ) - - if reflection.source_macro == :has_and_belongs_to_many - tables << alias_tracker.aliased_table_for( - reflection.source_reflection.join_table, - table_alias_for(reflection, true) - ) - end end - tables end def table_name_for(reflection) diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 6cc9d6c079..713ff80d47 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -42,12 +42,9 @@ module ActiveRecord autoload :HasManyThrough, 'active_record/associations/preloader/has_many_through' autoload :HasOne, 'active_record/associations/preloader/has_one' autoload :HasOneThrough, 'active_record/associations/preloader/has_one_through' - autoload :HasAndBelongsToMany, 'active_record/associations/preloader/has_and_belongs_to_many' autoload :BelongsTo, 'active_record/associations/preloader/belongs_to' end - attr_reader :records, :associations, :preload_scope, :model - # Eager loads the named associations for the given Active Record record(s). # # In this description, 'association name' shall refer to the name passed @@ -82,40 +79,47 @@ module ActiveRecord # [ :books, :author ] # { author: :avatar } # [ :books, { author: :avatar } ] - def initialize(records, associations, preload_scope = nil) - @records = Array.wrap(records).compact.uniq - @associations = Array.wrap(associations) - @preload_scope = preload_scope || NULL_RELATION - end NULL_RELATION = Struct.new(:values).new({}) - def run - unless records.empty? - associations.each { |association| preload(association) } + def preload(records, associations, preload_scope = nil) + records = Array.wrap(records).compact.uniq + associations = Array.wrap(associations) + preload_scope = preload_scope || NULL_RELATION + + if records.empty? + [] + else + associations.flat_map { |association| + preloaders_on association, records, preload_scope + } end end private - def preload(association) + def preloaders_on(association, records, scope) case association when Hash - preload_hash(association) + preloaders_for_hash(association, records, scope) when Symbol - preload_one(association) + preloaders_for_one(association, records, scope) when String - preload_one(association.to_sym) + preloaders_for_one(association.to_sym, records, scope) else raise ArgumentError, "#{association.inspect} was not recognised for preload" end end - def preload_hash(association) - association.each do |parent, child| - Preloader.new(records, parent, preload_scope).run - Preloader.new(records.map { |record| record.send(parent) }.flatten, child).run - end + def preloaders_for_hash(association, records, scope) + parent, child = association.to_a.first # hash should only be of length 1 + + loaders = preloaders_for_one parent, records, scope + + recs = loaders.flat_map(&:preloaded_records).uniq + loaders.concat Array.wrap(child).flat_map { |assoc| + preloaders_on assoc, recs, scope + } end # Not all records have the same class, so group then preload group on the reflection @@ -125,52 +129,81 @@ module ActiveRecord # Additionally, polymorphic belongs_to associations can have multiple associated # classes, depending on the polymorphic_type field. So we group by the classes as # well. - def preload_one(association) - grouped_records(association).each do |reflection, klasses| - klasses.each do |klass, records| - preloader_for(reflection).new(klass, records, reflection, preload_scope).run + def preloaders_for_one(association, records, scope) + grouped_records(association, records).flat_map do |reflection, klasses| + klasses.map do |rhs_klass, rs| + loader = preloader_for(reflection, rs, rhs_klass).new(rhs_klass, rs, reflection, scope) + loader.run self + loader end end end - def grouped_records(association) - Hash[ - records_by_reflection(association).map do |reflection, records| - [reflection, records.group_by { |record| association_klass(reflection, record) }] - end - ] + def grouped_records(association, records) + reflection_records = records_by_reflection(association, records) + + reflection_records.each_with_object({}) do |(reflection, r_records),h| + h[reflection] = r_records.group_by { |record| + association_klass(reflection, record) + } + end end - def records_by_reflection(association) + def records_by_reflection(association, records) records.group_by do |record| - reflection = record.class.reflections[association] + reflection = record.class.reflect_on_association(association) - unless reflection - raise ActiveRecord::ConfigurationError, "Association named '#{association}' was not found; " \ - "perhaps you misspelled it?" - end - - reflection + reflection || raise_config_error(association) end end + def raise_config_error(association) + raise ActiveRecord::ConfigurationError, + "Association named '#{association}' was not found; " \ + "perhaps you misspelled it?" + end + def association_klass(reflection, record) if reflection.macro == :belongs_to && reflection.options[:polymorphic] - klass = record.send(reflection.foreign_type) + klass = record.read_attribute(reflection.foreign_type.to_s) klass && klass.constantize else reflection.klass end end - def preloader_for(reflection) + class AlreadyLoaded + attr_reader :owners, :reflection + + def initialize(klass, owners, reflection, preload_scope) + @owners = owners + @reflection = reflection + end + + def run(preloader); end + + def preloaded_records + owners.flat_map { |owner| owner.read_attribute reflection.name } + end + end + + class NullPreloader + def self.new(klass, owners, reflection, preload_scope); self; end + def self.run(preloader); end + end + + def preloader_for(reflection, owners, rhs_klass) + return NullPreloader unless rhs_klass + + if owners.first.association(reflection.name).loaded? + return AlreadyLoaded + end + case reflection.macro when :has_many reflection.options[:through] ? HasManyThrough : HasMany when :has_one reflection.options[:through] ? HasOneThrough : HasOne - when :has_and_belongs_to_many - HasAndBelongsToMany when :belongs_to BelongsTo end diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 928da71eed..69b65982b3 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -3,6 +3,7 @@ module ActiveRecord class Preloader class Association #:nodoc: attr_reader :owners, :reflection, :preload_scope, :model, :klass + attr_reader :preloaded_records def initialize(klass, owners, reflection, preload_scope) @klass = klass @@ -12,15 +13,14 @@ module ActiveRecord @model = owners.first && owners.first.class @scope = nil @owners_by_key = nil + @preloaded_records = [] end - def run - unless owners.first.association(reflection.name).loaded? - preload - end + def run(preloader) + preload(preloader) end - def preload + def preload(preloader) raise NotImplementedError end @@ -68,36 +68,39 @@ module ActiveRecord private - def associated_records_by_owner + def associated_records_by_owner(preloader) owners_map = owners_by_key owner_keys = owners_map.keys.compact # Each record may have multiple owners, and vice-versa - records_by_owner = Hash[owners.map { |owner| [owner, []] }] + records_by_owner = owners.each_with_object({}) do |owner,h| + h[owner] = [] + end - if klass && owner_keys.any? + if owner_keys.any? # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) # Make several smaller queries if necessary or make one query if the adapter supports it sliced = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) - sliced.each { |slice| - records = records_for(slice) - caster = type_caster(records, association_key_name) - records.each do |record| - owner_key = caster.call record[association_key_name] - - owners_map[owner_key].each do |owner| - records_by_owner[owner] << record - end + + records = load_slices sliced + records.each do |record, owner_key| + owners_map[owner_key].each do |owner| + records_by_owner[owner] << record end - } + end end records_by_owner end - IDENTITY = lambda { |value| value } - def type_caster(results, name) - IDENTITY + def load_slices(slices) + @preloaded_records = slices.flat_map { |slice| + records_for(slice) + } + + @preloaded_records.map { |record| + [record, record[association_key_name]] + } end def reflection_scope @@ -116,6 +119,14 @@ module ActiveRecord scope.select! preload_values[:select] || values[:select] || table[Arel.star] scope.includes! preload_values[:includes] || values[:includes] + if preload_values.key? :order + scope.order! preload_values[:order] + else + if values.key? :order + scope.order! values[:order] + end + end + if options[:as] scope.where!(klass.table_name => { reflection.type => model.base_class.sti_name }) end diff --git a/activerecord/lib/active_record/associations/preloader/collection_association.rb b/activerecord/lib/active_record/associations/preloader/collection_association.rb index e6cd35e7a1..5adffcd831 100644 --- a/activerecord/lib/active_record/associations/preloader/collection_association.rb +++ b/activerecord/lib/active_record/associations/preloader/collection_association.rb @@ -9,8 +9,8 @@ module ActiveRecord super.order(preload_scope.values[:order] || reflection_scope.values[:order]) end - def preload - associated_records_by_owner.each do |owner, records| + def preload(preloader) + associated_records_by_owner(preloader).each do |owner, records| association = owner.association(reflection.name) association.loaded! association.target.concat(records) diff --git a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb deleted file mode 100644 index c042a44b21..0000000000 --- a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb +++ /dev/null @@ -1,65 +0,0 @@ -module ActiveRecord - module Associations - class Preloader - class HasAndBelongsToMany < CollectionAssociation #:nodoc: - attr_reader :join_table - - def initialize(klass, records, reflection, preload_options) - super - @join_table = Arel::Table.new(reflection.join_table).alias('t0') - end - - # Unlike the other associations, we want to get a raw array of rows so that we can - # access the aliased column on the join table - def records_for(ids) - scope = query_scope ids - klass.connection.select_all(scope.arel, 'SQL', scope.bind_values) - end - - def owner_key_name - reflection.active_record_primary_key - end - - def association_key_name - 'ar_association_key_name' - end - - def association_key - join_table[reflection.foreign_key] - end - - private - - # Once we have used the join table column (in super), we manually instantiate the - # actual records, ensuring that we don't create more than one instances of the same - # record - def associated_records_by_owner - records = {} - super.each_value do |rows| - rows.map! { |row| records[row[klass.primary_key]] ||= klass.instantiate(row) } - end - end - - def type_caster(results, name) - caster = results.column_types.fetch(name, results.identity_type) - lambda { |value| caster.type_cast value } - end - - def build_scope - super.joins(join).select(join_select) - end - - def join_select - association_key.as(Arel.sql(association_key_name)) - end - - def join - condition = table[reflection.association_primary_key].eq( - join_table[reflection.association_foreign_key]) - - table.create_join(join_table, table.create_on(condition)) - end - end - end - end -end diff --git a/activerecord/lib/active_record/associations/preloader/has_many_through.rb b/activerecord/lib/active_record/associations/preloader/has_many_through.rb index 157b627ad5..7b37b5942d 100644 --- a/activerecord/lib/active_record/associations/preloader/has_many_through.rb +++ b/activerecord/lib/active_record/associations/preloader/has_many_through.rb @@ -4,7 +4,7 @@ module ActiveRecord class HasManyThrough < CollectionAssociation #:nodoc: include ThroughAssociation - def associated_records_by_owner + def associated_records_by_owner(preloader) records_by_owner = super if reflection_scope.distinct_value diff --git a/activerecord/lib/active_record/associations/preloader/singular_association.rb b/activerecord/lib/active_record/associations/preloader/singular_association.rb index 44e804d785..2b5cfda8ce 100644 --- a/activerecord/lib/active_record/associations/preloader/singular_association.rb +++ b/activerecord/lib/active_record/associations/preloader/singular_association.rb @@ -5,8 +5,8 @@ module ActiveRecord private - def preload - associated_records_by_owner.each do |owner, associated_records| + def preload(preloader) + associated_records_by_owner(preloader).each do |owner, associated_records| record = associated_records.first association = owner.association(reflection.name) diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 2c625cec04..ea21836c65 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -2,7 +2,6 @@ module ActiveRecord module Associations class Preloader module ThroughAssociation #:nodoc: - def through_reflection reflection.through_reflection end @@ -11,34 +10,67 @@ module ActiveRecord reflection.source_reflection end - def associated_records_by_owner - through_records = through_records_by_owner + def associated_records_by_owner(preloader) + preloader.preload(owners, + through_reflection.name, + through_scope) + + through_records = owners.map do |owner, h| + association = owner.association through_reflection.name + + [owner, Array(association.reader)] + end + + reset_association owners, through_reflection.name - Preloader.new(through_records.values.flatten, source_reflection.name, reflection_scope).run + middle_records = through_records.map { |(_,rec)| rec }.flatten - through_records.each do |owner, records| - records.map! { |r| r.send(source_reflection.name) }.flatten! - records.compact! + preloaders = preloader.preload(middle_records, + source_reflection.name, + reflection_scope) + + middle_to_pl = preloaders.each_with_object({}) do |pl,h| + pl.owners.each { |middle| + h[middle] = pl + } end + + through_records.each_with_object({}) { |(lhs,center),records_by_owner| + pl_to_middle = center.group_by { |record| middle_to_pl[record] } + + records_by_owner[lhs] = pl_to_middle.flat_map do |pl, middles| + rhs_records = middles.flat_map { |r| + r.send(source_reflection.name) + }.compact + + loaded_records = pl.preloaded_records + i = 0 + record_index = loaded_records.each_with_object({}) { |r,indexes| + indexes[r] = i + i += 1 + } + records = rhs_records.sort_by { |rhs| record_index[rhs] } + @preloaded_records.concat rhs_records + records + end + } end private - def through_records_by_owner - Preloader.new(owners, through_reflection.name, through_scope).run - + def reset_association(owners, association_name) should_reset = (through_scope != through_reflection.klass.unscoped) || (reflection.options[:source_type] && through_reflection.collection?) - owners.each_with_object({}) do |owner, h| - association = owner.association through_reflection.name - h[owner] = Array(association.reader) - - # Dont cache the association - we would only be caching a subset - association.reset if should_reset + # Dont cache the association - we would only be caching a subset + if should_reset + owners.each { |owner| + owner.association(association_name).reset + } end end + def through_scope scope = through_reflection.klass.unscoped diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index 4f06955406..30fa2c8ba5 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -12,6 +12,9 @@ module ActiveRecord # of this method is +false+ an <tt>ActiveModel::ForbiddenAttributesError</tt> # exception is raised. def assign_attributes(new_attributes) + if !new_attributes.respond_to?(:stringify_keys) + raise ArgumentError, "When assigning attributes, you must pass a hash as an argument." + end return if new_attributes.blank? attributes = new_attributes.stringify_keys diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 208da2cb77..bf270c1829 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/enumerable' require 'mutex_m' +require 'thread_safe' module ActiveRecord # = Active Record Attribute Methods @@ -29,23 +30,18 @@ module ActiveRecord } class AttributeMethodCache - include Mutex_m - def initialize - super @module = Module.new - @method_cache = {} + @method_cache = ThreadSafe::Cache.new end def [](name) - synchronize do - @method_cache.fetch(name) { - safe_name = name.unpack('h*').first - temp_method = "__temp__#{safe_name}" - ActiveRecord::AttributeMethods::AttrNames.set_name_cache safe_name, name - @module.module_eval method_body(temp_method, safe_name), __FILE__, __LINE__ - @method_cache[name] = @module.instance_method temp_method - } + @method_cache.compute_if_absent(name) do + safe_name = name.unpack('h*').first + temp_method = "__temp__#{safe_name}" + ActiveRecord::AttributeMethods::AttrNames.set_name_cache safe_name, name + @module.module_eval method_body(temp_method, safe_name), __FILE__, __LINE__ + @module.instance_method temp_method end end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index dc2399643c..19e81abba5 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -19,8 +19,7 @@ module ActiveRecord # Attempts to +save+ the record and clears changed attributes if successful. def save(*) if status = super - @previously_changed = changes - @changed_attributes.clear + changes_applied end status end @@ -28,16 +27,14 @@ module ActiveRecord # Attempts to <tt>save!</tt> the record and clears changed attributes if successful. def save!(*) super.tap do - @previously_changed = changes - @changed_attributes.clear + changes_applied end end # <tt>reload</tt> the record and clears changed attributes. def reload(*) super.tap do - @previously_changed.clear - @changed_attributes.clear + reset_changes end end @@ -48,11 +45,11 @@ module ActiveRecord # The attribute already has an unsaved change. if attribute_changed?(attr) - old = @changed_attributes[attr] - @changed_attributes.delete(attr) unless _field_changed?(attr, old, value) + old = changed_attributes[attr] + changed_attributes.delete(attr) unless _field_changed?(attr, old, value) else old = clone_attribute_value(:read_attribute, attr) - @changed_attributes[attr] = old if _field_changed?(attr, old, value) + changed_attributes[attr] = old if _field_changed?(attr, old, value) end # Carry on. diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index e4c484d64b..128a9377c1 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -23,11 +23,14 @@ module ActiveRecord # Check out <tt>ActiveRecord::Transactions</tt> for more details about <tt>after_commit</tt> and # <tt>after_rollback</tt>. # + # Additionally, an <tt>after_touch</tt> callback is triggered whenever an + # object is touched. + # # Lastly an <tt>after_find</tt> and <tt>after_initialize</tt> callback is triggered for each object that # is found and instantiated by a finder, with <tt>after_initialize</tt> being triggered after new objects # are instantiated as well. # - # That's a total of twelve callbacks, which gives you immense power to react and prepare for each state in the + # There are nineteen callbacks in total, which give you immense power to react and prepare for each state in the # Active Record life cycle. The sequence for calling <tt>Base#save</tt> for an existing record is similar, # except that each <tt>_create</tt> callback is replaced by the corresponding <tt>_update</tt> callback. # diff --git a/activerecord/lib/active_record/connection_adapters/abstract/savepoints.rb b/activerecord/lib/active_record/connection_adapters/abstract/savepoints.rb new file mode 100644 index 0000000000..25c17ce971 --- /dev/null +++ b/activerecord/lib/active_record/connection_adapters/abstract/savepoints.rb @@ -0,0 +1,21 @@ +module ActiveRecord + module ConnectionAdapters + module Savepoints #:nodoc: + def supports_savepoints? + true + end + + def create_savepoint(name = current_savepoint_name) + execute("SAVEPOINT #{name}") + end + + def rollback_to_savepoint(name = current_savepoint_name) + execute("ROLLBACK TO SAVEPOINT #{name}") + end + + def release_savepoint(name = current_savepoint_name) + execute("RELEASE SAVEPOINT #{name}") + end + end + end +end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index dde45b0ef3..cbe563676b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -33,6 +33,7 @@ module ActiveRecord autoload :Quoting autoload :ConnectionPool autoload :QueryCache + autoload :Savepoints end autoload_at 'active_record/connection_adapters/abstract/transaction' do @@ -395,13 +396,13 @@ module ActiveRecord @transaction.number end - def create_savepoint + def create_savepoint(name = nil) end - def rollback_to_savepoint + def rollback_to_savepoint(name = nil) end - def release_savepoint + def release_savepoint(name = nil) end def case_sensitive_modifier(node) @@ -423,13 +424,14 @@ module ActiveRecord protected - def log(sql, name = "SQL", binds = []) + def log(sql, name = "SQL", binds = [], statement_name = nil) @instrumenter.instrument( "sql.active_record", - :sql => sql, - :name => name, - :connection_id => object_id, - :binds => binds) { yield } + :sql => sql, + :name => name, + :connection_id => object_id, + :statement_name => statement_name, + :binds => binds) { yield } rescue => e message = "#{e.class.name}: #{e.message}: #{sql}" @logger.error message if @logger 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 d502daf230..138ab811dc 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -3,6 +3,8 @@ require 'arel/visitors/bind_visitor' module ActiveRecord module ConnectionAdapters class AbstractMysqlAdapter < AbstractAdapter + include Savepoints + class SchemaCreation < AbstractAdapter::SchemaCreation def visit_AddColumn(o) @@ -194,11 +196,6 @@ module ActiveRecord true end - # Returns true, since this connection adapter supports savepoints. - def supports_savepoints? - true - end - def supports_bulk_alter? #:nodoc: true end @@ -340,18 +337,6 @@ module ActiveRecord # Transactions aren't supported end - def create_savepoint - execute("SAVEPOINT #{current_savepoint_name}") - end - - def rollback_to_savepoint - execute("ROLLBACK TO SAVEPOINT #{current_savepoint_name}") - end - - def release_savepoint - execute("RELEASE SAVEPOINT #{current_savepoint_name}") - end - # In the simple case, MySQL allows us to place JOINs directly into the UPDATE # query. However, this does not allow for LIMIT, OFFSET and ORDER. To support # these, we must use a subquery. diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index fb53090edc..2596c221bc 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -203,11 +203,19 @@ module ActiveRecord end end - def new_time(year, mon, mday, hour, min, sec, microsec) + def new_time(year, mon, mday, hour, min, sec, microsec, offset = nil) # Treat 0000-00-00 00:00:00 as nil. return nil if year.nil? || (year == 0 && mon == 0 && mday == 0) - Time.send(Base.default_timezone, year, mon, mday, hour, min, sec, microsec) rescue nil + if offset + time = Time.utc(year, mon, mday, hour, min, sec, microsec) rescue nil + return nil unless time + + time -= offset + Base.default_timezone == :utc ? time : time.getlocal + else + Time.public_send(Base.default_timezone, year, mon, mday, hour, min, sec, microsec) rescue nil + end end def fast_string_to_date(string) @@ -232,7 +240,7 @@ module ActiveRecord time_hash = Date._parse(string) time_hash[:sec_fraction] = microseconds(time_hash) - new_time(*time_hash.values_at(:year, :mon, :mday, :hour, :min, :sec, :sec_fraction)) + new_time(*time_hash.values_at(:year, :mon, :mday, :hour, :min, :sec, :sec_fraction, :offset)) end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index 86b96a77fb..fa173d13a2 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -134,35 +134,31 @@ module ActiveRecord end def exec_query(sql, name = 'SQL', binds = []) - log(sql, name, binds) do - result = without_prepared_statement?(binds) ? exec_no_cache(sql, binds) : - exec_cache(sql, binds) - - types = {} - fields = result.fields - fields.each_with_index do |fname, i| - ftype = result.ftype i - fmod = result.fmod i - types[fname] = OID::TYPE_MAP.fetch(ftype, fmod) { |oid, mod| - warn "unknown OID: #{fname}(#{oid}) (#{sql})" - OID::Identity.new - } - end - - ret = ActiveRecord::Result.new(fields, result.values, types) - result.clear - return ret + result = without_prepared_statement?(binds) ? exec_no_cache(sql, name, binds) : + exec_cache(sql, name, binds) + + types = {} + fields = result.fields + fields.each_with_index do |fname, i| + ftype = result.ftype i + fmod = result.fmod i + types[fname] = OID::TYPE_MAP.fetch(ftype, fmod) { |oid, mod| + warn "unknown OID: #{fname}(#{oid}) (#{sql})" + OID::Identity.new + } end + + ret = ActiveRecord::Result.new(fields, result.values, types) + result.clear + return ret end def exec_delete(sql, name = 'SQL', binds = []) - log(sql, name, binds) do - result = binds.empty? ? exec_no_cache(sql, binds) : - exec_cache(sql, binds) - affected = result.cmd_tuples - result.clear - affected - end + result = without_prepared_statement?(binds) ? exec_no_cache(sql, name, binds) : + exec_cache(sql, name, binds) + affected = result.cmd_tuples + result.clear + affected end alias :exec_update :exec_delete @@ -218,18 +214,6 @@ module ActiveRecord def rollback_db_transaction execute "ROLLBACK" end - - def create_savepoint - execute("SAVEPOINT #{current_savepoint_name}") - end - - def rollback_to_savepoint - execute("ROLLBACK TO SAVEPOINT #{current_savepoint_name}") - end - - def release_savepoint - execute("RELEASE SAVEPOINT #{current_savepoint_name}") - end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 13978fd113..0c4d005e63 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -45,16 +45,18 @@ module ActiveRecord module ConnectionAdapters # PostgreSQL-specific extensions to column definitions in a table. class PostgreSQLColumn < Column #:nodoc: - attr_accessor :array + attr_accessor :array, :default_function # Instantiates a new PostgreSQL column definition in a table. def initialize(name, default, oid_type, sql_type = nil, null = true) @oid_type = oid_type + default_value = self.class.extract_value_from_default(default) + @default_function = default if !default_value && default && default =~ /.+\(.*\)/ if sql_type =~ /\[\]$/ @array = true - super(name, self.class.extract_value_from_default(default), sql_type[0..sql_type.length - 3], null) + super(name, default_value, sql_type[0..sql_type.length - 3], null) else @array = false - super(name, self.class.extract_value_from_default(default), sql_type, null) + super(name, default_value, sql_type, null) end end @@ -428,6 +430,7 @@ module ActiveRecord include ReferentialIntegrity include SchemaStatements include DatabaseStatements + include Savepoints # Returns 'PostgreSQL' as adapter name for identification purposes. def adapter_name @@ -439,6 +442,7 @@ module ActiveRecord def prepare_column_options(column, types) spec = super spec[:array] = 'true' if column.respond_to?(:array) && column.array + spec[:default] = "\"#{column.default_function}\"" if column.respond_to?(:default_function) && column.default_function spec end @@ -617,11 +621,6 @@ module ActiveRecord true end - # Returns true, since this connection adapter supports savepoints. - def supports_savepoints? - true - end - # Returns true. def supports_explain? true @@ -768,27 +767,29 @@ module ActiveRecord FEATURE_NOT_SUPPORTED = "0A000" # :nodoc: - def exec_no_cache(sql, binds) - @connection.async_exec(sql) + def exec_no_cache(sql, name, binds) + log(sql, name, binds) { @connection.async_exec(sql) } end - def exec_cache(sql, binds) + def exec_cache(sql, name, binds) stmt_key = prepare_statement(sql) - # Clear the queue - @connection.get_last_result - @connection.send_query_prepared(stmt_key, binds.map { |col, val| - type_cast(val, col) - }) - @connection.block - @connection.get_last_result - rescue PGError => e + log(sql, name, binds, stmt_key) do + @connection.send_query_prepared(stmt_key, binds.map { |col, val| + type_cast(val, col) + }) + @connection.block + @connection.get_last_result + end + rescue ActiveRecord::StatementInvalid => e + pgerror = e.original_exception + # Get the PG code for the failure. Annoyingly, the code for # prepared statements whose return value may have changed is # FEATURE_NOT_SUPPORTED. Check here for more details: # http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/utils/cache/plancache.c#l573 begin - code = e.result.result_error_field(PGresult::PG_DIAG_SQLSTATE) + code = pgerror.result.result_error_field(PGresult::PG_DIAG_SQLSTATE) rescue raise e end @@ -813,6 +814,8 @@ module ActiveRecord unless @statements.key? sql_key nextkey = @statements.next_key @connection.prepare nextkey, sql + # Clear the queue + @connection.get_last_result @statements[sql_key] = nextkey end @statements[sql_key] diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 136094dcc9..997384daea 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -53,6 +53,8 @@ module ActiveRecord # # * <tt>:database</tt> - Path to the database file. class SQLite3Adapter < AbstractAdapter + include Savepoints + class Version include Comparable @@ -351,18 +353,6 @@ module ActiveRecord exec_query(sql, name).rows end - def create_savepoint - execute("SAVEPOINT #{current_savepoint_name}") - end - - def rollback_to_savepoint - execute("ROLLBACK TO SAVEPOINT #{current_savepoint_name}") - end - - def release_savepoint - execute("RELEASE SAVEPOINT #{current_savepoint_name}") - end - def begin_db_transaction #:nodoc: log('begin transaction',nil) { @connection.transaction } end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 5e6fee52f7..366ebde418 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -163,7 +163,7 @@ module ActiveRecord # ==== Example: # # Instantiates a single new object # User.new(first_name: 'Jamie') - def initialize(attributes = nil) + def initialize(attributes = nil, options = {}) defaults = self.class.column_defaults.dup defaults.each { |k, v| defaults[k] = v.dup if v.duplicable? } @@ -176,7 +176,9 @@ module ActiveRecord ensure_proper_type populate_with_current_scope_attributes - assign_attributes(attributes) if attributes + # +options+ argument is only needed to make protected_attributes gem easier to hook. + # Remove it when we drop support to this gem. + init_attributes(attributes, options) if attributes yield self if block_given? run_callbacks :initialize unless _initialize_callbacks.empty? @@ -411,8 +413,6 @@ module ActiveRecord @aggregation_cache = {} @association_cache = {} @attributes_cache = {} - @previously_changed = {} - @changed_attributes = {} @readonly = false @destroyed = false @marked_for_destruction = false @@ -429,8 +429,14 @@ module ActiveRecord # optimistic locking) won't get written unless they get marked as changed self.class.columns.each do |c| attr, orig_value = c.name, c.default - @changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @attributes[attr]) + changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @attributes[attr]) end end + + # This method is needed to make protected_attributes gem easier to hook. + # Remove it when we drop support to this gem. + def init_attributes(attributes, options) + assign_attributes(attributes) + end end end diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 9a26e5df3f..3bb3131bd1 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -639,8 +639,6 @@ module ActiveRecord if association.options[:through] add_join_records(rows, row, HasManyThroughProxy.new(association)) end - when :has_and_belongs_to_many - add_join_records(rows, row, HABTMProxy.new(association)) end end end @@ -674,16 +672,6 @@ module ActiveRecord end end - class HABTMProxy < ReflectionProxy # :nodoc: - def rhs_key - @association.association_foreign_key - end - - def lhs_key - @association.foreign_key - end - end - private def primary_key_name @primary_key_name ||= model_class && model_class.primary_key diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 626fe40103..55776a91c0 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -150,6 +150,7 @@ module ActiveRecord # Quote the column name used for optimistic locking. def quoted_locking_column + ActiveSupport::Deprecation.warn "ActiveRecord::Base.quoted_locking_column is deprecated and will be removed in Rails 4.2 or later." connection.quote_column_name(locking_column) end diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index 0358a36b14..927fbab8f0 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -17,7 +17,7 @@ module ActiveRecord def initialize super - @odd_or_even = false + @odd = false end def render_bind(column, value) @@ -60,17 +60,8 @@ module ActiveRecord debug " #{name} #{sql}#{binds}" end - def identity(event) - return unless logger.debug? - - name = color(event.payload[:name], odd? ? CYAN : MAGENTA, true) - line = odd? ? color(event.payload[:line], nil, true) : event.payload[:line] - - debug " #{name} #{line}" - end - def odd? - @odd_or_even = !@odd_or_even + @odd = !@odd end def logger diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index d166f0dd66..716020e7e7 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -42,10 +42,6 @@ module ActiveRecord "" end - def where_values_hash - {} - end - def count(*) 0 end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index d630f31f5f..a73a140ef1 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -335,8 +335,18 @@ module ActiveRecord # Reloads the record from the database. # - # This method modifies the receiver in-place. Attributes are updated, and - # caches busted, in particular the associations cache. + # This method finds record by its primary key (which could be assigned manually) and + # modifies the receiver in-place: + # + # account = Account.new + # # => #<Account id: nil, email: nil> + # account.id = 1 + # account.reload + # # Account Load (1.2ms) SELECT "accounts".* FROM "accounts" WHERE "accounts"."id" = $1 LIMIT 1 [["id", 1]] + # # => #<Account id: 1, email: 'account@example.com'> + # + # Attributes are reloaded from the database, and caches busted, in + # particular the associations cache. # # If the record no longer exists in the database <tt>ActiveRecord::RecordNotFound</tt> # is raised. Otherwise, in addition to the in-place modification the method @@ -391,7 +401,8 @@ module ActiveRecord end # Saves the record with the updated_at/on attributes set to the current time. - # Please note that no validation is performed and no callbacks are executed. + # Please note that no validation is performed and only the +after_touch+ + # callback is executed. # If an attribute name is passed, that attribute is updated along with # updated_at/on attributes. # @@ -434,7 +445,7 @@ module ActiveRecord changes[self.class.locking_column] = increment_lock if locking_enabled? - @changed_attributes.except!(*changes.keys) + changed_attributes.except!(*changes.keys) primary_key = self.class.primary_key self.class.unscoped.where(primary_key => self[primary_key]).update_all(changes) == 1 end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index f47282b7fd..e88c5d17cb 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -12,8 +12,6 @@ module ActiveRecord def self.create(macro, name, scope, options, ar) case macro - when :has_and_belongs_to_many - klass = AssociationReflection when :has_many, :belongs_to, :has_one klass = options[:through] ? ThroughReflection : AssociationReflection when :composed_of @@ -196,10 +194,11 @@ module ActiveRecord def initialize(macro, name, scope, options, active_record) super - @collection = [:has_many, :has_and_belongs_to_many].include?(macro) + @collection = :has_many == macro @automatic_inverse_of = nil @type = options[:as] && "#{options[:as]}_type" @foreign_type = options[:foreign_type] || "#{name}_type" + @constructable = calculate_constructable(macro, options) end # Returns a new, unsaved instance of the associated class. +attributes+ will @@ -208,6 +207,10 @@ module ActiveRecord klass.new(attributes, &block) end + def constructable? # :nodoc: + @constructable + end + def table_name klass.table_name end @@ -251,10 +254,6 @@ module ActiveRecord def check_validity! check_validity_of_inverse! - - if has_and_belongs_to_many? && association_foreign_key == foreign_key - raise HasAndBelongsToManyAssociationForeignKeyNeeded.new(self) - end end def check_validity_of_inverse! @@ -336,10 +335,6 @@ module ActiveRecord macro == :belongs_to end - def has_and_belongs_to_many? - macro == :has_and_belongs_to_many - end - def association_class case macro when :belongs_to @@ -348,8 +343,6 @@ module ActiveRecord else Associations::BelongsToAssociation end - when :has_and_belongs_to_many - Associations::HasAndBelongsToManyAssociation when :has_many if options[:through] Associations::HasManyThroughAssociation @@ -374,11 +367,23 @@ module ActiveRecord protected - def actual_source_reflection # FIXME: this is a horrible name - self - end + def actual_source_reflection # FIXME: this is a horrible name + self + end private + + def calculate_constructable(macro, options) + case macro + when :belongs_to + !options[:polymorphic] + when :has_one + !options[:through] + else + true + end + end + # Attempts to find the inverse association name automatically. # If it cannot find a suitable inverse association name, it returns # nil. @@ -588,7 +593,7 @@ module ActiveRecord # A through association is nested if there would be more than one join table def nested? - chain.length > 2 || through_reflection.has_and_belongs_to_many? + chain.length > 2 end # We want to use the klass from this reflection, rather than just delegate straight to diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 4e86e905ed..cfaf566ec4 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -599,8 +599,9 @@ module ActiveRecord preload = preload_values preload += includes_values unless eager_loading? + preloader = ActiveRecord::Associations::Preloader.new preload.each do |associations| - ActiveRecord::Associations::Preloader.new(@records, associations).run + preloader.preload @records, associations end @records.each { |record| record.readonly! } if readonly_value diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index fd8496442e..49b01909c6 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -34,7 +34,7 @@ module ActiveRecord # between id 0 and 10,000 and worker 2 handle from 10,000 and beyond # (by setting the +:start+ option on that worker). # - # # Let's process for a batch of 2000 records, skiping the first 2000 rows + # # Let's process for a batch of 2000 records, skipping the first 2000 rows # Person.find_each(start: 2000, batch_size: 2000) do |person| # person.party_all_night! # end diff --git a/activerecord/lib/active_record/result.rb b/activerecord/lib/active_record/result.rb index d0f1cb5b75..1dc3bf3f12 100644 --- a/activerecord/lib/active_record/result.rb +++ b/activerecord/lib/active_record/result.rb @@ -46,6 +46,10 @@ module ActiveRecord IDENTITY_TYPE end + def column_type(name) + @column_types[name] || identity_type + end + def each if block_given? hash_rows.each { |row| yield row } diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 1181cc739e..8986d255cd 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -17,9 +17,19 @@ module ActiveRecord cattr_accessor :ignore_tables @@ignore_tables = [] - def self.dump(connection=ActiveRecord::Base.connection, stream=STDOUT) - new(connection).dump(stream) - stream + class << self + def dump(connection=ActiveRecord::Base.connection, stream=STDOUT, config = ActiveRecord::Base) + new(connection, generate_options(config)).dump(stream) + stream + end + + private + def generate_options(config) + { + table_name_prefix: config.table_name_prefix, + table_name_suffix: config.table_name_suffix + } + end end def dump(stream) @@ -32,10 +42,11 @@ module ActiveRecord private - def initialize(connection) + def initialize(connection, options = {}) @connection = connection @types = @connection.native_database_types @version = Migrator::current_version rescue nil + @options = options end def header(stream) @@ -201,7 +212,7 @@ HEADER end def remove_prefix_and_suffix(table) - table.gsub(/^(#{ActiveRecord::Base.table_name_prefix})(.+)(#{ActiveRecord::Base.table_name_suffix})$/, "\\2") + table.gsub(/^(#{@options[:table_name_prefix]})(.+)(#{@options[:table_name_suffix]})$/, "\\2") end end end diff --git a/activerecord/lib/rails/generators/active_record.rb b/activerecord/lib/rails/generators/active_record.rb index c8aa37f275..dc29213235 100644 --- a/activerecord/lib/rails/generators/active_record.rb +++ b/activerecord/lib/rails/generators/active_record.rb @@ -1,23 +1,17 @@ require 'rails/generators/named_base' -require 'rails/generators/migration' require 'rails/generators/active_model' +require 'rails/generators/active_record/migration' require 'active_record' module ActiveRecord module Generators # :nodoc: class Base < Rails::Generators::NamedBase # :nodoc: - include Rails::Generators::Migration + include ActiveRecord::Generators::Migration # Set the current directory as base for the inherited generators. def self.base_root File.dirname(__FILE__) end - - # Implement the required interface for Rails::Generators::Migration. - def self.next_migration_number(dirname) - next_migration_number = current_migration_number(dirname) + 1 - ActiveRecord::Migration.next_migration_number(next_migration_number) - end end end end diff --git a/activerecord/lib/rails/generators/active_record/migration.rb b/activerecord/lib/rails/generators/active_record/migration.rb new file mode 100644 index 0000000000..b7418cf42f --- /dev/null +++ b/activerecord/lib/rails/generators/active_record/migration.rb @@ -0,0 +1,18 @@ +require 'rails/generators/migration' + +module ActiveRecord + module Generators # :nodoc: + module Migration + extend ActiveSupport::Concern + include Rails::Generators::Migration + + module ClassMethods + # Implement the required interface for Rails::Generators::Migration. + def next_migration_number(dirname) + next_migration_number = current_migration_number(dirname) + 1 + ActiveRecord::Migration.next_migration_number(next_migration_number) + end + end + end + end +end diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb index fedd9f603c..679c515e8c 100644 --- a/activerecord/test/cases/adapters/mysql2/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb @@ -3,14 +3,14 @@ require "cases/helper" class MysqlConnectionTest < ActiveRecord::TestCase def setup super + @subscriber = SQLSubscriber.new + ActiveSupport::Notifications.subscribe('sql.active_record', @subscriber) @connection = ActiveRecord::Base.connection - @connection.extend(LogIntercepter) - @connection.intercepted = true end def teardown - @connection.intercepted = false - @connection.logged = [] + ActiveSupport::Notifications.unsubscribe(@subscriber) + super end def test_no_automatic_reconnection_after_timeout @@ -72,14 +72,14 @@ class MysqlConnectionTest < ActiveRecord::TestCase def test_logs_name_show_variable @connection.show_variable 'foo' - assert_equal "SCHEMA", @connection.logged[0][1] + assert_equal "SCHEMA", @subscriber.logged[0][1] end def test_logs_name_rename_column_sql @connection.execute "CREATE TABLE `bar_baz` (`foo` varchar(255))" - @connection.logged = [] + @subscriber.logged.clear @connection.send(:rename_column_sql, 'bar_baz', 'foo', 'foo2') - assert_equal "SCHEMA", @connection.logged[0][1] + assert_equal "SCHEMA", @subscriber.logged[0][1] ensure @connection.execute "DROP TABLE `bar_baz`" end diff --git a/activerecord/test/cases/adapters/postgresql/connection_test.rb b/activerecord/test/cases/adapters/postgresql/connection_test.rb index 6b726ce875..81aa977c59 100644 --- a/activerecord/test/cases/adapters/postgresql/connection_test.rb +++ b/activerecord/test/cases/adapters/postgresql/connection_test.rb @@ -7,14 +7,14 @@ module ActiveRecord def setup super + @subscriber = SQLSubscriber.new + ActiveSupport::Notifications.subscribe('sql.active_record', @subscriber) @connection = ActiveRecord::Base.connection - @connection.extend(LogIntercepter) - @connection.intercepted = true end def teardown - @connection.intercepted = false - @connection.logged = [] + ActiveSupport::Notifications.unsubscribe(@subscriber) + super end def test_encoding @@ -47,38 +47,48 @@ module ActiveRecord def test_tables_logs_name @connection.tables('hello') - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_indexes_logs_name @connection.indexes('items', 'hello') - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_table_exists_logs_name @connection.table_exists?('items') - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_table_alias_length_logs_name @connection.instance_variable_set("@table_alias_length", nil) @connection.table_alias_length - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_current_database_logs_name @connection.current_database - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_encoding_logs_name @connection.encoding - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_schema_names_logs_name @connection.schema_names - assert_equal 'SCHEMA', @connection.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] + end + + def test_statement_key_is_logged + bindval = 1 + @connection.exec_query('SELECT $1::integer', 'SQL', [[nil, bindval]]) + name = @subscriber.payloads.last[:statement_name] + assert name + res = @connection.exec_query("EXPLAIN (FORMAT JSON) EXECUTE #{name}(#{bindval})") + plan = res.column_types['QUERY PLAN'].type_cast res.rows.first.first + assert_operator plan.length, :>, 0 end # Must have with_manual_interventions set to true for this diff --git a/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb b/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb index f1c4b85126..c5fd40accc 100644 --- a/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb +++ b/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb @@ -1,38 +1,40 @@ require 'cases/helper' -module ActiveRecord::ConnectionAdapters - class PostgreSQLAdapter < AbstractAdapter - class InactivePGconn - def query(*args) - raise PGError - end +module ActiveRecord + module ConnectionAdapters + class PostgreSQLAdapter < AbstractAdapter + class InactivePGconn + def query(*args) + raise PGError + end - def status - PGconn::CONNECTION_BAD + def status + PGconn::CONNECTION_BAD + end end - end - class StatementPoolTest < ActiveRecord::TestCase - def test_cache_is_per_pid - return skip('must support fork') unless Process.respond_to?(:fork) + class StatementPoolTest < ActiveRecord::TestCase + def test_cache_is_per_pid + return skip('must support fork') unless Process.respond_to?(:fork) - cache = StatementPool.new nil, 10 - cache['foo'] = 'bar' - assert_equal 'bar', cache['foo'] + cache = StatementPool.new nil, 10 + cache['foo'] = 'bar' + assert_equal 'bar', cache['foo'] - pid = fork { - lookup = cache['foo']; - exit!(!lookup) - } + pid = fork { + lookup = cache['foo']; + exit!(!lookup) + } - Process.waitpid pid - assert $?.success?, 'process should exit successfully' - end + Process.waitpid pid + assert $?.success?, 'process should exit successfully' + end - def test_dealloc_does_not_raise_on_inactive_connection - cache = StatementPool.new InactivePGconn.new, 10 - cache['foo'] = 'bar' - assert_nothing_raised { cache.clear } + def test_dealloc_does_not_raise_on_inactive_connection + cache = StatementPool.new InactivePGconn.new, 10 + cache['foo'] = 'bar' + assert_nothing_raised { cache.clear } + end end end end diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb index b573d48807..0cd5b420fc 100644 --- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb +++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb @@ -61,6 +61,7 @@ class PostgresqlUUIDTest < ActiveRecord::TestCase schema = StringIO.new ActiveRecord::SchemaDumper.dump(@connection, schema) assert_match(/\bcreate_table "pg_uuids", id: :uuid\b/, schema.string) + assert_match(/t\.uuid "other_uuid", default: "uuid_generate_v4\(\)"/, schema.string) end end @@ -93,3 +94,43 @@ class PostgresqlUUIDTestNilDefault < ActiveRecord::TestCase assert_nil col_desc["default"] end end + +class PostgresqlUUIDTestInverseOf < ActiveRecord::TestCase + class UuidPost < ActiveRecord::Base + self.table_name = 'pg_uuid_posts' + has_many :uuid_comments, inverse_of: :uuid_post + end + + class UuidComment < ActiveRecord::Base + self.table_name = 'pg_uuid_comments' + belongs_to :uuid_post + end + + def setup + @connection = ActiveRecord::Base.connection + @connection.reconnect! + + @connection.transaction do + @connection.create_table('pg_uuid_posts', id: :uuid) do |t| + t.string 'title' + end + @connection.create_table('pg_uuid_comments', id: :uuid) do |t| + t.uuid :uuid_post_id, default: 'uuid_generate_v4()' + t.string 'content' + end + end + end + + def teardown + @connection.transaction do + @connection.execute 'drop table if exists pg_uuid_comments' + @connection.execute 'drop table if exists pg_uuid_posts' + end + end + + def test_collection_association_with_uuid + post = UuidPost.create! + comment = post.uuid_comments.create! + assert post.uuid_comments.find(comment.id) + end +end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 6ba6518eaa..ce7c869eec 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -21,8 +21,8 @@ module ActiveRecord ) eosql - @conn.extend(LogIntercepter) - @conn.intercepted = true + @subscriber = SQLSubscriber.new + ActiveSupport::Notifications.subscribe('sql.active_record', @subscriber) end def test_valid_column @@ -31,16 +31,16 @@ module ActiveRecord end # sqlite databases should be able to support any type and not - # just the ones mentioned in the native_database_types. - # Therefore test_invalid column should always return true + # just the ones mentioned in the native_database_types. + # Therefore test_invalid column should always return true # even if the type is not valid. def test_invalid_column assert @conn.valid_type?(:foobar) end def teardown - @conn.intercepted = false - @conn.logged = [] + ActiveSupport::Notifications.unsubscribe(@subscriber) + super end def test_column_types @@ -256,7 +256,7 @@ module ActiveRecord def test_tables_logs_name assert_logged [['SCHEMA', []]] do @conn.tables('hello') - assert_not_nil @conn.logged.first.shift + assert_not_nil @subscriber.logged.first.shift end end @@ -268,7 +268,7 @@ module ActiveRecord def test_table_exists_logs_name assert @conn.table_exists?('items') - assert_equal 'SCHEMA', @conn.logged[0][1] + assert_equal 'SCHEMA', @subscriber.logged[0][1] end def test_columns @@ -306,10 +306,10 @@ module ActiveRecord end def test_indexes_logs - assert_difference('@conn.logged.length') do + assert_difference('@subscriber.logged.length') do @conn.indexes('items') end - assert_match(/items/, @conn.logged.last.first) + assert_match(/items/, @subscriber.logged.last.first) end def test_no_indexes @@ -370,7 +370,7 @@ module ActiveRecord def assert_logged logs yield - assert_equal logs, @conn.logged + assert_equal logs, @subscriber.logged end end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 8d3d6962fc..874ae77ff7 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -747,6 +747,8 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_with_default_scope_as_block + # warm up the habtm cache + EagerDeveloperWithBlockDefaultScope.where(:name => 'David').first.projects developer = EagerDeveloperWithBlockDefaultScope.where(:name => 'David').first projects = Project.order(:id).to_a assert_no_queries do @@ -1136,6 +1138,10 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_deep_including_through_habtm + # warm up habtm cache + posts = Post.all.merge!(:includes => {:categories => :categorizations}, :order => "posts.id").to_a + posts[0].categories[0].categorizations.length + posts = Post.all.merge!(:includes => {:categories => :categorizations}, :order => "posts.id").to_a assert_no_queries { assert_equal 2, posts[0].categories[0].categorizations.length } assert_no_queries { assert_equal 1, posts[0].categories[1].categorizations.length } diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index f77066e6ab..be928ec8ee 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -613,7 +613,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase 3, Developer.references(:developers_projects_join).merge( :includes => {:projects => :developers}, - :where => 'developers_projects_join.joined_on IS NOT NULL' + :where => 'projects_developers_projects_join.joined_on IS NOT NULL' ).to_a.size ) end @@ -632,7 +632,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal( 3, Developer.references(:developers_projects_join).merge( - :includes => {:projects => :developers}, :where => 'developers_projects_join.joined_on IS NOT NULL', + :includes => {:projects => :developers}, :where => 'projects_developers_projects_join.joined_on IS NOT NULL', :group => group.join(",") ).to_a.size ) @@ -646,8 +646,8 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase end def test_find_scoped_grouped - assert_equal 5, categories(:general).posts_grouped_by_title.size - assert_equal 1, categories(:technology).posts_grouped_by_title.size + assert_equal 5, categories(:general).posts_grouped_by_title.to_a.size + assert_equal 1, categories(:technology).posts_grouped_by_title.to_a.size end def test_find_scoped_grouped_having @@ -718,12 +718,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal project, developer.projects.first end - def test_self_referential_habtm_without_foreign_key_set_should_raise_exception - assert_raise(ActiveRecord::HasAndBelongsToManyAssociationForeignKeyNeeded) { - SelfMember.new.friends - } - end - def test_dynamic_find_should_respect_association_include # SQL error in sort clause if :include is not included # due to Unknown column 'authors.id' diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index ebeead0dc2..caa916346a 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -80,6 +80,13 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 'exotic', bulb.name end + def test_build_from_association_should_respect_scope + author = Author.new + + post = author.thinking_posts.build + assert_equal 'So I was thinking', post.title + end + def test_create_from_association_with_nil_values_should_work car = Car.create(:name => 'honda') @@ -337,6 +344,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_raise(ActiveRecord::RecordNotFound) { firm.clients.find(2, 99) } end + def test_find_ids_and_inverse_of + force_signal37_to_load_all_clients_of_firm + + firm = companies(:first_firm) + client = firm.clients_of_firm.find(3) + assert_kind_of Client, client + + client_ary = firm.clients_of_firm.find([3]) + assert_kind_of Array, client_ary + assert_equal client, client_ary.first + end + def test_find_all firm = Firm.all.merge!(:order => "id").first assert_equal 2, firm.clients.where("#{QUOTED_TYPE} = 'Client'").to_a.length @@ -518,6 +537,13 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end end + def test_inverse_on_before_validate + firm = companies(:first_firm) + assert_queries(1) do + firm.clients_of_firm << Client.new("name" => "Natural Company") + end + end + def test_new_aliased_to_build company = companies(:first_firm) new_client = assert_no_queries { company.clients_of_firm.new("name" => "Another Client") } @@ -1610,6 +1636,22 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal car.id, bulb.attributes_after_initialize['car_id'] end + def test_attributes_are_set_when_initialized_from_has_many_null_relationship + car = Car.new name: 'honda' + bulb = car.bulbs.where(name: 'headlight').first_or_initialize + assert_equal 'headlight', bulb.name + end + + def test_attributes_are_set_when_initialized_from_polymorphic_has_many_null_relationship + post = Post.new title: 'title', body: 'bar' + tag = Tag.create!(name: 'foo') + + tagging = post.taggings.where(tag: tag).first_or_initialize + + assert_equal tag.id, tagging.tag_id + assert_equal 'Post', tagging.taggable_type + end + def test_replace car = Car.create(:name => 'honda') bulb1 = car.bulbs.create diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index dd8b426b25..5299e4e17e 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -29,7 +29,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase fixtures :posts, :readers, :people, :comments, :authors, :categories, :taggings, :tags, :owners, :pets, :toys, :jobs, :references, :companies, :members, :author_addresses, :subscribers, :books, :subscriptions, :developers, :categorizations, :essays, - :categories_posts + :categories_posts, :clubs, :memberships # Dummies to force column loads so query counts are clean. def setup @@ -37,10 +37,45 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase Reader.create :person_id => 0, :post_id => 0 end + def test_preload_sti_rhs_class + developers = Developer.includes(:firms).all.to_a + assert_no_queries do + developers.each { |d| d.firms } + end + end + + def test_preload_sti_middle_relation + club = Club.create!(name: 'Aaron cool banana club') + member1 = Member.create!(name: 'Aaron') + member2 = Member.create!(name: 'Cat') + + SuperMembership.create! club: club, member: member1 + CurrentMembership.create! club: club, member: member2 + + club1 = Club.includes(:members).find_by_id club.id + assert_equal [member1, member2].sort_by(&:id), + club1.members.sort_by(&:id) + end + def make_model(name) Class.new(ActiveRecord::Base) { define_singleton_method(:name) { name } } end + def test_ordered_habtm + person_prime = Class.new(ActiveRecord::Base) do + def self.name; 'Person'; end + + has_many :readers + has_many :posts, -> { order('posts.id DESC') }, :through => :readers + end + posts = person_prime.includes(:posts).first.posts + + assert_operator posts.length, :>, 1 + posts.each_cons(2) do |left,right| + assert_operator left.id, :>, right.id + end + end + def test_singleton_has_many_through book = make_model "Book" subscription = make_model "Subscription" diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 2477e60e51..8c81e00865 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -446,6 +446,19 @@ class InverseHasManyTests < ActiveRecord::TestCase def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.first.secret_interests } end + + def test_child_instance_should_point_to_parent_without_saving + man = Man.new + i = Interest.create(:topic => 'Industrial Revolution Re-enactment') + + man.interests << i + assert_not_nil i.man + + i.man.name = "Charles" + assert_equal i.man.name, man.name + + assert !man.persisted? + end end class InverseBelongsToTests < ActiveRecord::TestCase diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb index cf3c07845c..8ef351cda8 100644 --- a/activerecord/test/cases/associations/nested_through_associations_test.rb +++ b/activerecord/test/cases/associations/nested_through_associations_test.rb @@ -214,7 +214,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase end def test_has_many_through_has_many_with_has_and_belongs_to_many_source_reflection_preload - authors = assert_queries(3) { Author.includes(:post_categories).to_a.sort_by(&:id) } + authors = assert_queries(4) { Author.includes(:post_categories).to_a.sort_by(&:id) } general, cooking = categories(:general), categories(:cooking) assert_no_queries do @@ -242,7 +242,8 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase end def test_has_many_through_has_and_belongs_to_many_with_has_many_source_reflection_preload - categories = assert_queries(3) { Category.includes(:post_comments).to_a.sort_by(&:id) } + Category.includes(:post_comments).to_a # preheat cache + categories = assert_queries(4) { Category.includes(:post_comments).to_a.sort_by(&:id) } greetings, more = comments(:greetings), comments(:more_greetings) assert_no_queries do @@ -270,7 +271,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase end def test_has_many_through_has_many_with_has_many_through_habtm_source_reflection_preload - authors = assert_queries(5) { Author.includes(:category_post_comments).to_a.sort_by(&:id) } + authors = assert_queries(6) { Author.includes(:category_post_comments).to_a.sort_by(&:id) } greetings, more = comments(:greetings), comments(:more_greetings) assert_no_queries do diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index baba55ea43..9c66ed354e 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -92,7 +92,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase def test_set_attributes_without_hash topic = Topic.new - assert_nothing_raised { topic.attributes = '' } + assert_raise(ArgumentError) { topic.attributes = '' } end def test_integers_as_nil diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 635278abc1..517d2674a7 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -1440,10 +1440,6 @@ class TestAutosaveAssociationValidationMethodsGeneration < ActiveRecord::TestCas test "should generate validation methods for HABTM associations with :validate => true" do assert_respond_to @pirate, :validate_associated_records_for_parrots end - - test "should not generate validation methods for HABTM associations without :validate => true" do - assert !@pirate.respond_to?(:validate_associated_records_for_non_validated_parrots) - end end class TestAutosaveAssociationWithTouch < ActiveRecord::TestCase diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 5812e20a1b..d4433ef889 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -566,6 +566,15 @@ class BasicsTest < ActiveRecord::TestCase assert_equal topic, Topic.find(topic.id) end + def test_destroy_without_prepared_statement + topic = Topic.create(title: 'foo') + Topic.connection.unprepared_statement do + Topic.find(topic.id).destroy + end + + assert_equal nil, Topic.find_by_id(topic.id) + end + def test_blank_ids one = Subscriber.new(:id => '') two = Subscriber.new(:id => '') @@ -1339,6 +1348,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_marshal_between_processes + skip "can't marshal between processes when using an in-memory db" if in_memory_db? skip "fork isn't supported" unless Process.respond_to?(:fork) # Define a new model to ensure there are no caches diff --git a/activerecord/test/cases/column_test.rb b/activerecord/test/cases/column_test.rb index 3a4f414ae8..5ab2f18e9d 100644 --- a/activerecord/test/cases/column_test.rb +++ b/activerecord/test/cases/column_test.rb @@ -110,6 +110,16 @@ module ActiveRecord assert_equal 1800, column.type_cast(30.minutes) assert_equal 7200, column.type_cast(2.hours) end + + def test_string_to_time_with_timezone + old = ActiveRecord::Base.default_timezone + [:utc, :local].each do |zone| + ActiveRecord::Base.default_timezone = zone + assert_equal Time.utc(2013, 9, 4, 0, 0, 0), Column.string_to_time("Wed, 04 Sep 2013 03:00:00 EAT") + end + rescue + ActiveRecord::Base.default_timezone = old + end end end end diff --git a/activerecord/test/cases/forbidden_attributes_protection_test.rb b/activerecord/test/cases/forbidden_attributes_protection_test.rb index 490b599fb6..981a75faf6 100644 --- a/activerecord/test/cases/forbidden_attributes_protection_test.rb +++ b/activerecord/test/cases/forbidden_attributes_protection_test.rb @@ -61,4 +61,9 @@ class ForbiddenAttributesProtectionTest < ActiveRecord::TestCase assert_equal 'Guille', person.first_name assert_equal 'm', person.gender end + + def test_blank_attributes_should_not_raise + person = Person.new + assert_nil person.assign_attributes(ProtectedParams.new({})) + end end diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index f96978aff8..739e2b2f19 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -119,21 +119,24 @@ class << Time end end -module LogIntercepter - attr_accessor :logged, :intercepted - def self.extended(base) - base.logged = [] +class SQLSubscriber + attr_reader :logged + attr_reader :payloads + + def initialize + @logged = [] + @payloads = [] end - def log(sql, name = 'SQL', binds = [], &block) - if @intercepted - @logged << [sql, name, binds] - yield - else - super(sql, name,binds, &block) - end + + def start(name, id, payload) + @payloads << payload + @logged << [payload[:sql], payload[:name], payload[:binds]] end + + def finish(name, id, payload); end end + module InTimeZone private diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index a9be132801..73cf99a5d7 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -313,8 +313,12 @@ class InheritanceTest < ActiveRecord::TestCase assert_kind_of SpecialSubscriber, SpecialSubscriber.find("webster132") assert_nothing_raised { s = SpecialSubscriber.new("name" => "And breaaaaathe!"); s.id = 'roger'; s.save } end -end + def test_scope_inherited_properly + assert_nothing_raised { Company.of_first_firm } + assert_nothing_raised { Client.of_first_firm } + end +end class InheritanceComputeTypeTest < ActiveRecord::TestCase fixtures :companies diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index dfa12cb97c..a16ed963fe 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -272,6 +272,10 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert p.treasures.empty? assert RichPerson.connection.select_all("SELECT * FROM peoples_treasures WHERE rich_person_id = 1").empty? end + + def test_quoted_locking_column_is_deprecated + assert_deprecated { ActiveRecord::Base.quoted_locking_column } + end end class OptimisticLockingWithSchemaChangeTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/modules_test.rb b/activerecord/test/cases/modules_test.rb index 08b3408665..9124105e6d 100644 --- a/activerecord/test/cases/modules_test.rb +++ b/activerecord/test/cases/modules_test.rb @@ -1,6 +1,7 @@ require "cases/helper" require 'models/company_in_module' require 'models/shop' +require 'models/developer' class ModulesTest < ActiveRecord::TestCase fixtures :accounts, :companies, :projects, :developers, :collections, :products, :variants diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 30dc2a34c6..6cd3e2154e 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -419,10 +419,6 @@ class PersistenceTest < ActiveRecord::TestCase assert !Topic.find(1).approved? end - def test_update_attribute_does_not_choke_on_nil - assert Topic.find(1).update(nil) - end - def test_update_attribute_for_readonly_attribute minivan = Minivan.find('m1') assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') } @@ -701,6 +697,17 @@ class PersistenceTest < ActiveRecord::TestCase assert_equal topic.title, Topic.find(1234).title end + def test_update_attributes_parameters + topic = Topic.find(1) + assert_nothing_raised do + topic.update_attributes({}) + end + + assert_raises(ArgumentError) do + topic.update_attributes(nil) + end + end + def test_update! Reply.validates_presence_of(:title) reply = Reply.find(2) diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 88a12c61df..f814947ab2 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -42,6 +42,11 @@ class RelationTest < ActiveRecord::TestCase end def test_two_scopes_with_includes_should_not_drop_any_include + # heat habtm cache + car = Car.incl_engines.incl_tyres.first + car.tyres.length + car.engines.length + car = Car.incl_engines.incl_tyres.first assert_no_queries { car.tyres.length } assert_no_queries { car.engines.length } @@ -300,6 +305,10 @@ class RelationTest < ActiveRecord::TestCase assert_equal({}, Developer.none.where_values_hash) end + def test_null_relation_where_values_hash + assert_equal({ 'salary' => 100_000 }, Developer.none.where(salary: 100_000).where_values_hash) + end + def test_joins_with_nil_argument assert_nothing_raised { DependentFirm.joins(nil).first } end diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index a48ae1036f..32f86f9c88 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -299,7 +299,7 @@ class SchemaDumperTest < ActiveRecord::TestCase def test_schema_dump_includes_uuid_shorthand_definition output = standard_dump - if %r{create_table "poistgresql_uuids"} =~ output + if %r{create_table "postgresql_uuids"} =~ output assert_match %r{t.uuid "guid"}, output end end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index a5cb22aaf6..17206ffe99 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -375,6 +375,36 @@ class TransactionTest < ActiveRecord::TestCase assert_equal "Three", @three end if Topic.connection.supports_savepoints? + def test_using_named_savepoints + Topic.transaction do + @first.approved = true + @first.save! + Topic.connection.create_savepoint("first") + + @first.approved = false + @first.save! + Topic.connection.rollback_to_savepoint("first") + assert @first.reload.approved? + + @first.approved = false + @first.save! + Topic.connection.release_savepoint("first") + assert_not @first.reload.approved? + end + end if Topic.connection.supports_savepoints? + + def test_releasing_named_savepoints + Topic.transaction do + Topic.connection.create_savepoint("another") + Topic.connection.release_savepoint("another") + + # The savepoint is now gone and we can't remove it again. + assert_raises(ActiveRecord::StatementInvalid) do + Topic.connection.release_savepoint("another") + end + end + end + def test_rollback_when_commit_raises Topic.connection.expects(:begin_db_transaction) Topic.connection.expects(:commit_db_transaction).raises('OH NOES') diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 8104c607b5..0b0b304121 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -11,6 +11,11 @@ class Company < AbstractCompany has_many :contracts has_many :developers, :through => :contracts + scope :of_first_firm, lambda { + joins(:account => :firm). + where('firms.id' => 1) + } + def arbitrary_method "I am Jack's profound disappointment" end @@ -39,7 +44,7 @@ class Firm < Company has_many :unsorted_clients, :class_name => "Client" has_many :unsorted_clients_with_symbol, :class_name => :Client has_many :clients_sorted_desc, -> { order "id DESC" }, :class_name => "Client" - has_many :clients_of_firm, -> { order "id" }, :foreign_key => "client_of", :class_name => "Client" + has_many :clients_of_firm, -> { order "id" }, :foreign_key => "client_of", :class_name => "Client", :inverse_of => :firm has_many :clients_ordered_by_name, -> { order "name" }, :class_name => "Client" has_many :unvalidated_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :validate => false has_many :dependent_clients_of_firm, -> { order "id" }, :foreign_key => "client_of", :class_name => "Client", :dependent => :destroy @@ -117,6 +122,10 @@ class Client < Company has_many :accounts, :through => :firm, :source => :accounts belongs_to :account + validate do + firm + end + class RaisedOnSave < RuntimeError; end attr_accessor :raise_on_save before_save do diff --git a/activerecord/test/models/contract.rb b/activerecord/test/models/contract.rb index 2cf5aa7a85..cdf7b267b5 100644 --- a/activerecord/test/models/contract.rb +++ b/activerecord/test/models/contract.rb @@ -1,6 +1,7 @@ class Contract < ActiveRecord::Base belongs_to :company belongs_to :developer + belongs_to :firm, :foreign_key => 'company_id' before_save :hi after_save :bye diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index c8e2be580e..a26de55758 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -38,6 +38,8 @@ class Developer < ActiveRecord::Base has_and_belongs_to_many :special_projects, :join_table => 'developers_projects', :association_foreign_key => 'project_id' has_many :audit_logs + has_many :contracts + has_many :firms, :through => :contracts, :source => :firm scope :jamises, -> { where(:name => 'Jamis') } diff --git a/activerecord/test/models/membership.rb b/activerecord/test/models/membership.rb index bcbb7e42c5..df7167ee93 100644 --- a/activerecord/test/models/membership.rb +++ b/activerecord/test/models/membership.rb @@ -8,6 +8,11 @@ class CurrentMembership < Membership belongs_to :club end +class SuperMembership < Membership + belongs_to :member, -> { order('members.id DESC') } + belongs_to :club +end + class SelectedMembership < Membership def self.default_scope select("'1' as foo") diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 9ab81c73c8..f1dd7c312d 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,40 @@ +* `require_dependency` accepts objects that respond to `to_path`, in + particular `Pathname` instances. + + *Benjamin Fleischer* + +* Disable the ability to iterate over Range of AS::TimeWithZone + due to significant performance issues. + + *Bogdan Gusiev* + +* Allow attaching event subscribers to ActiveSupport::Notifications namespaces + before they're defined. Essentially, this means instead of this: + + class JokeSubscriber < ActiveSupport::Subscriber + def sql(event) + puts "A rabbi and a priest walk into a bar..." + end + + # This call needs to happen *after* defining the methods. + attach_to "active_record" + end + + You can do this: + + class JokeSubscriber < ActiveSupport::Subscriber + # This is much easier to read! + attach_to "active_record" + + def sql(event) + puts "A rabbi and a priest walk into a bar..." + end + end + + This should make it easier to read and understand these subscribers. + + *Daniel Schierbeck* + * Add `Date#middle_of_day`, `DateTime#middle_of_day` and `Time#middle_of_day` methods. Also added `midday`, `noon`, `at_midday`, `at_noon` and `at_middle_of_day` as aliases. diff --git a/activesupport/bin/generate_tables b/activesupport/bin/generate_tables index 5fefa429df..f39e89b7d0 100755 --- a/activesupport/bin/generate_tables +++ b/activesupport/bin/generate_tables @@ -28,12 +28,6 @@ module ActiveSupport def initialize @ucd = Unicode::UnicodeDatabase.new - - default = Codepoint.new - default.combining_class = 0 - default.uppercase_mapping = 0 - default.lowercase_mapping = 0 - @ucd.codepoints = Hash.new(default) end def parse_codepoints(line) diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 308a1b2cd9..c5633d902f 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -234,7 +234,7 @@ module ActiveSupport # bump the cache expiration time by the value set in <tt>:race_condition_ttl</tt>. # Yes, this process is extending the time for a stale value by another few # seconds. Because of extended life of the previous cache, other processes - # will continue to use slightly stale data for a just a big longer. In the + # will continue to use slightly stale data for a just a bit longer. In the # meantime that first process will go ahead and will write into cache the # new value. After that all the processes will start getting new value. # The key is to keep <tt>:race_condition_ttl</tt> small. diff --git a/activesupport/lib/active_support/core_ext/range.rb b/activesupport/lib/active_support/core_ext/range.rb index 1d8b1ede5a..9368e81235 100644 --- a/activesupport/lib/active_support/core_ext/range.rb +++ b/activesupport/lib/active_support/core_ext/range.rb @@ -1,3 +1,4 @@ require 'active_support/core_ext/range/conversions' require 'active_support/core_ext/range/include_range' require 'active_support/core_ext/range/overlaps' +require 'active_support/core_ext/range/each' diff --git a/activesupport/lib/active_support/core_ext/range/each.rb b/activesupport/lib/active_support/core_ext/range/each.rb new file mode 100644 index 0000000000..d51ea2e944 --- /dev/null +++ b/activesupport/lib/active_support/core_ext/range/each.rb @@ -0,0 +1,24 @@ +require 'active_support/core_ext/module/aliasing' +require 'active_support/core_ext/object/acts_like' + +class Range #:nodoc: + + def each_with_time_with_zone(&block) + ensure_iteration_allowed + each_without_time_with_zone(&block) + end + alias_method_chain :each, :time_with_zone + + def step_with_time_with_zone(n = 1, &block) + ensure_iteration_allowed + step_without_time_with_zone(n, &block) + end + alias_method_chain :step, :time_with_zone + + private + def ensure_iteration_allowed + if first.acts_like?(:time) + raise TypeError, "can't iterate from #{first.class}" + end + end +end diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index db9f5d4baa..df13fef0a4 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -198,9 +198,19 @@ module ActiveSupport #:nodoc: Dependencies.require_or_load(file_name) end + # Interprets a file using <tt>mechanism</tt> and marks its defined + # constants as autoloaded. <tt>file_name</tt> can be either a string or + # respond to <tt>to_path</tt>. + # + # Use this method in code that absolutely needs a certain constant to be + # defined at that point. A typical use case is to make constant name + # resolution deterministic for constants with the same relative name in + # different namespaces whose evaluation would depend on load order + # otherwise. def require_dependency(file_name, message = "No such file to load -- %s") + file_name = file_name.to_path if file_name.respond_to?(:to_path) unless file_name.is_a?(String) - raise ArgumentError, "the file name must be a String -- you passed #{file_name.inspect}" + raise ArgumentError, "the file name must either be a String or implement #to_path -- you passed #{file_name.inspect}" end Dependencies.depend_on(file_name, message) diff --git a/activesupport/lib/active_support/multibyte/chars.rb b/activesupport/lib/active_support/multibyte/chars.rb index a42e7f6542..3c0cf9f137 100644 --- a/activesupport/lib/active_support/multibyte/chars.rb +++ b/activesupport/lib/active_support/multibyte/chars.rb @@ -56,11 +56,10 @@ module ActiveSupport #:nodoc: # Forward all undefined methods to the wrapped string. def method_missing(method, *args, &block) + result = @wrapped_string.__send__(method, *args, &block) if method.to_s =~ /!$/ - result = @wrapped_string.__send__(method, *args, &block) self if result else - result = @wrapped_string.__send__(method, *args, &block) result.kind_of?(String) ? chars(result) : result end end diff --git a/activesupport/lib/active_support/multibyte/unicode.rb b/activesupport/lib/active_support/multibyte/unicode.rb index 04e6b71580..1845c6ae38 100644 --- a/activesupport/lib/active_support/multibyte/unicode.rb +++ b/activesupport/lib/active_support/multibyte/unicode.rb @@ -287,6 +287,13 @@ module ActiveSupport class Codepoint attr_accessor :code, :combining_class, :decomp_type, :decomp_mapping, :uppercase_mapping, :lowercase_mapping + # Initializing Codepoint object with default values + def initialize + @combining_class = 0 + @uppercase_mapping = 0 + @lowercase_mapping = 0 + end + def swapcase_mapping uppercase_mapping > 0 ? uppercase_mapping : lowercase_mapping end diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb index 54cff6d394..c9c0eff2bf 100644 --- a/activesupport/lib/active_support/number_helper.rb +++ b/activesupport/lib/active_support/number_helper.rb @@ -460,7 +460,7 @@ module ActiveSupport # See <tt>number_to_human_size</tt> if you want to print a file # size. # - # You can also define you own unit-quantifier names if you want + # You can also define your own unit-quantifier names if you want # to use other decimal units (eg.: 1500 becomes "1.5 # kilometers", 0.150 becomes "150 milliliters", etc). You may # define a wide range of unit quantifiers, even fractional ones diff --git a/activesupport/lib/active_support/subscriber.rb b/activesupport/lib/active_support/subscriber.rb index 34c6f900c1..f2966f23fc 100644 --- a/activesupport/lib/active_support/subscriber.rb +++ b/activesupport/lib/active_support/subscriber.rb @@ -31,18 +31,41 @@ module ActiveSupport # Attach the subscriber to a namespace. def attach_to(namespace, subscriber=new, notifier=ActiveSupport::Notifications) + @namespace = namespace + @subscriber = subscriber + @notifier = notifier + subscribers << subscriber + # Add event subscribers for all existing methods on the class. subscriber.public_methods(false).each do |event| - next if %w{ start finish }.include?(event.to_s) + add_event_subscriber(event) + end + end - notifier.subscribe("#{event}.#{namespace}", subscriber) + # Adds event subscribers for all new methods added to the class. + def method_added(event) + # Only public methods are added as subscribers, and only if a notifier + # has been set up. This means that subscribers will only be set up for + # classes that call #attach_to. + if public_method_defined?(event) && notifier + add_event_subscriber(event) end end def subscribers @@subscribers ||= [] end + + protected + + attr_reader :subscriber, :notifier, :namespace + + def add_event_subscriber(event) + return if %w{ start finish }.include?(event.to_s) + + notifier.subscribe("#{event}.#{namespace}", subscriber) + end end def initialize diff --git a/activesupport/test/core_ext/range_ext_test.rb b/activesupport/test/core_ext/range_ext_test.rb index 2f8723a074..854b0a38bd 100644 --- a/activesupport/test/core_ext/range_ext_test.rb +++ b/activesupport/test/core_ext/range_ext_test.rb @@ -90,4 +90,26 @@ class RangeTest < ActiveSupport::TestCase time_range_2 = Time.utc(2005, 12, 10, 17, 31)..Time.utc(2005, 12, 10, 18, 00) assert !time_range_1.overlaps?(time_range_2) end + + def test_each_on_time_with_zone + twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone['Eastern Time (US & Canada)'] , Time.utc(2006,11,28,10,30)) + assert_raises TypeError do + ((twz - 1.hour)..twz).each {} + end + end + + def test_step_on_time_with_zone + twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone['Eastern Time (US & Canada)'] , Time.utc(2006,11,28,10,30)) + assert_raises TypeError do + ((twz - 1.hour)..twz).step(1) {} + end + end + + def test_include_on_time_with_zone + twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone['Eastern Time (US & Canada)'] , Time.utc(2006,11,28,10,30)) + assert_raises TypeError do + ((twz - 1.hour)..twz).include?(twz) + end + end + end diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index 9b84e75902..2392b71960 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -35,6 +35,17 @@ class DependenciesTest < ActiveSupport::TestCase assert_equal expected.path, e.path end + def test_require_dependency_accepts_an_object_which_implements_to_path + o = Object.new + def o.to_path; 'dependencies/service_one'; end + assert_nothing_raised { + require_dependency o + } + assert defined?(ServiceOne) + ensure + remove_constants(:ServiceOne) + end + def test_tracking_loaded_files require_dependency 'dependencies/service_one' require_dependency 'dependencies/service_two' diff --git a/activesupport/test/subscriber_test.rb b/activesupport/test/subscriber_test.rb new file mode 100644 index 0000000000..253411aa3d --- /dev/null +++ b/activesupport/test/subscriber_test.rb @@ -0,0 +1,40 @@ +require 'abstract_unit' +require 'active_support/subscriber' + +class TestSubscriber < ActiveSupport::Subscriber + attach_to :doodle + + cattr_reader :event + + def self.clear + @@event = nil + end + + def open_party(event) + @@event = event + end + + private + + def private_party(event) + @@event = event + end +end + +class SubscriberTest < ActiveSupport::TestCase + def setup + TestSubscriber.clear + end + + def test_attaches_subscribers + ActiveSupport::Notifications.instrument("open_party.doodle") + + assert_equal "open_party.doodle", TestSubscriber.event.name + end + + def test_does_not_attach_private_methods + ActiveSupport::Notifications.instrument("private_party.doodle") + + assert_nil TestSubscriber.event + end +end diff --git a/guides/CHANGELOG.md b/guides/CHANGELOG.md index afa695d445..38e407b198 100644 --- a/guides/CHANGELOG.md +++ b/guides/CHANGELOG.md @@ -2,4 +2,8 @@ *Sıtkı Bağdat* +* Added the Rails maintenance policy to the guides. + + *Matias Korhonen* + Please check [4-0-stable](https://github.com/rails/rails/blob/4-0-stable/guides/CHANGELOG.md) for previous changes. diff --git a/guides/code/getting_started/Gemfile b/guides/code/getting_started/Gemfile index 3d57012901..eef6180804 100644 --- a/guides/code/getting_started/Gemfile +++ b/guides/code/getting_started/Gemfile @@ -31,7 +31,7 @@ end gem 'jbuilder', '~> 1.2' # To use ActiveModel has_secure_password -# gem 'bcrypt-ruby', '~> 3.1.0' +# gem 'bcrypt-ruby', '~> 3.1.2' # Use unicorn as the app server # gem 'unicorn' diff --git a/guides/source/3_0_release_notes.md b/guides/source/3_0_release_notes.md index d398cd680c..cf9d694de7 100644 --- a/guides/source/3_0_release_notes.md +++ b/guides/source/3_0_release_notes.md @@ -73,8 +73,6 @@ You can see an example of how that works at [Rails Upgrade is now an Official Pl Aside from Rails Upgrade tool, if you need more help, there are people on IRC and [rubyonrails-talk](http://groups.google.com/group/rubyonrails-talk) that are probably doing the same thing, possibly hitting the same issues. Be sure to blog your own experiences when upgrading so others can benefit from your knowledge! -More information - [The Path to Rails 3: Approaching the upgrade](http://omgbloglol.com/post/353978923/the-path-to-rails-3-approaching-the-upgrade) - Creating a Rails 3.0 application -------------------------------- diff --git a/guides/source/active_model_basics.md b/guides/source/active_model_basics.md index 1d87646e49..0019d08328 100644 --- a/guides/source/active_model_basics.md +++ b/guides/source/active_model_basics.md @@ -120,8 +120,8 @@ class Person end def save - @previously_changed = changes # do save work... + changes_applied end end ``` diff --git a/guides/source/active_record_basics.md b/guides/source/active_record_basics.md index ad08cb01f7..34baae509b 100644 --- a/guides/source/active_record_basics.md +++ b/guides/source/active_record_basics.md @@ -48,10 +48,10 @@ overall database access code. Active Record gives us several mechanisms, the most important being the ability to: -* Represent models and their data -* Represent associations between these models -* Represent inheritance hierarchies through related models -* Validate models before they get persisted to the database +* Represent models and their data. +* Represent associations between these models. +* Represent inheritance hierarchies through related models. +* Validate models before they get persisted to the database. * Perform database operations in an object-oriented fashion. Convention over Configuration in Active Record @@ -78,9 +78,9 @@ of two or more words, the model class name should follow the Ruby conventions, using the CamelCase form, while the table name must contain the words separated by underscores. Examples: -* Database Table - Plural with underscores separating words (e.g., `book_clubs`) +* Database Table - Plural with underscores separating words (e.g., `book_clubs`). * Model Class - Singular with the first letter of each word capitalized (e.g., -`BookClub`) +`BookClub`). | Model / Class | Table / Schema | | ------------- | -------------- | @@ -101,7 +101,7 @@ depending on the purpose of these columns. fields that Active Record will look for when you create associations between your models. * **Primary keys** - By default, Active Record will use an integer column named - `id` as the table's primary key. When using [Rails + `id` as the table's primary key. When using [Active Record Migrations](migrations.html) to create your tables, this column will be automatically created. @@ -116,7 +116,7 @@ to Active Record instances: locking](http://api.rubyonrails.org/classes/ActiveRecord/Locking.html) to a model. * `type` - Specifies that the model uses [Single Table - Inheritance](http://api.rubyonrails.org/classes/ActiveRecord/Base.html) + Inheritance](http://api.rubyonrails.org/classes/ActiveRecord/Base.html). * `(association_name)_type` - Stores the type for [polymorphic associations](association_basics.html#polymorphic-associations). * `(table_name)_count` - Used to cache the number of belonging objects on @@ -368,6 +368,6 @@ Rails keeps track of which files have been committed to the database and provides rollback features. To actually create the table, you'd run `rake db:migrate` and to roll it back, `rake db:rollback`. -Note that the above code is database-agnostic: it will run in MySQL, postgresql, -Oracle and others. You can learn more about migrations in the [Active Record -Migrations guide](migrations.html) +Note that the above code is database-agnostic: it will run in MySQL, +PostgreSQL, Oracle and others. You can learn more about migrations in the +[Active Record Migrations guide](migrations.html). diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 28013beeae..57e8e080f4 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -1189,7 +1189,7 @@ class Post < ActiveRecord::Base end ``` -This may then be called using this: +Call the scope as if it were a class method: ```ruby Post.created_before(Time.zone.now) @@ -1358,7 +1358,7 @@ COMMIT The new record might not be saved to the database; that depends on whether validations passed or not (just like `create`). -Suppose we want to set the 'locked' attribute to true if we're +Suppose we want to set the 'locked' attribute to `false` if we're creating a new record, but we don't want to include it in the query. So we want to find the client named "Andy", or if that client doesn't exist, create a client named "Andy" which is not locked. diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index e6b849e4c9..d3f49b19fa 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -96,12 +96,13 @@ INFO: The predicate for strings uses the Unicode-aware character class `[:space: WARNING: Note that numbers are not mentioned. In particular, 0 and 0.0 are **not** blank. -For example, this method from `ActionDispatch::Session::AbstractStore` uses `blank?` for checking whether a session key is present: +For example, this method from `ActionController::HttpAuthentication::Token::ControllerMethods` uses `blank?` for checking whether a token is present: ```ruby -def ensure_session_key! - if @key.blank? - raise ArgumentError, 'A key is required...' +def authenticate(controller, &login_procedure) + token, options = token_and_options(controller.request) + unless token.blank? + login_procedure.call(token, options) end end ``` @@ -1999,7 +2000,7 @@ Produce a string representation of a number in human-readable words: 1234567890123456.to_s(:human) # => "1.23 Quadrillion" ``` -NOTE: Defined in `active_support/core_ext/numeric/formatting.rb`. +NOTE: Defined in `active_support/core_ext/numeric/conversions.rb`. Extensions to `Integer` ----------------------- @@ -2444,7 +2445,7 @@ dup[1][2] = 4 array[1][2] == nil # => true ``` -NOTE: Defined in `active_support/core_ext/array/deep_dup.rb`. +NOTE: Defined in `active_support/core_ext/object/deep_dup.rb`. ### Grouping @@ -2670,45 +2671,7 @@ hash[:b][:e] == nil # => true hash[:b][:d] == [3, 4] # => true ``` -NOTE: Defined in `active_support/core_ext/hash/deep_dup.rb`. - -### Diffing - -The method `diff` returns a hash that represents a diff of the receiver and the argument with the following logic: - -* Pairs `key`, `value` that exist in both hashes do not belong to the diff hash. - -* If both hashes have `key`, but with different values, the pair in the receiver wins. - -* The rest is just merged. - -```ruby -{a: 1}.diff(a: 1) -# => {}, first rule - -{a: 1}.diff(a: 2) -# => {:a=>1}, second rule - -{a: 1}.diff(b: 2) -# => {:a=>1, :b=>2}, third rule - -{a: 1, b: 2, c: 3}.diff(b: 1, c: 3, d: 4) -# => {:a=>1, :b=>2, :d=>4}, all rules - -{}.diff({}) # => {} -{a: 1}.diff({}) # => {:a=>1} -{}.diff(a: 1) # => {:a=>1} -``` - -An important property of this diff hash is that you can retrieve the original hash by applying `diff` twice: - -```ruby -hash.diff(hash2).diff(hash2) == hash -``` - -Diffing hashes may be useful for error messages related to expected option hashes for example. - -NOTE: Defined in `active_support/core_ext/hash/diff.rb`. +NOTE: Defined in `active_support/core_ext/object/deep_dup.rb`. ### Working with Keys @@ -3843,13 +3806,13 @@ def default_helper_module! module_path = module_name.underscore helper module_path rescue MissingSourceFile => e - raise e unless e.is_missing? "#{module_path}_helper" + raise e unless e.is_missing? "helpers/#{module_path}_helper" rescue NameError => e raise e unless e.missing_name? "#{module_name}Helper" end ``` -NOTE: Defined in `active_support/core_ext/name_error.rb`. +NOTE: Defined in `actionpack/lib/abstract_controller/helpers.rb`. Extensions to `LoadError` ------------------------- @@ -3872,4 +3835,4 @@ rescue NameError => e end ``` -NOTE: Defined in `active_support/core_ext/load_error.rb`. +NOTE: Defined in `actionpack/lib/abstract_controller/helpers.rb`. diff --git a/guides/source/association_basics.md b/guides/source/association_basics.md index c58dd2e90a..91b268d766 100644 --- a/guides/source/association_basics.md +++ b/guides/source/association_basics.md @@ -261,7 +261,10 @@ With `through: :sections` specified, Rails will now understand: ### The `has_one :through` Association -A `has_one :through` association sets up a one-to-one connection with another model. This association indicates that the declaring model can be matched with one instance of another model by proceeding _through_ a third model. For example, if each supplier has one account, and each account is associated with one account history, then the customer model could look like this: +A `has_one :through` association sets up a one-to-one connection with another model. This association indicates +that the declaring model can be matched with one instance of another model by proceeding _through_ a third model. +For example, if each supplier has one account, and each account is associated with one account history, then the +supplier model could look like this: ```ruby class Supplier < ActiveRecord::Base @@ -337,7 +340,7 @@ class CreateAssembliesAndParts < ActiveRecord::Migration t.timestamps end - create_table :assemblies_parts do |t| + create_table :assemblies_parts, id: false do |t| t.belongs_to :assembly t.belongs_to :part end diff --git a/guides/source/command_line.md b/guides/source/command_line.md index 6f792be8ae..51ff921d7b 100644 --- a/guides/source/command_line.md +++ b/guides/source/command_line.md @@ -1,8 +1,6 @@ The Rails Command Line ====================== -Rails comes with every command line tool you'll need to - After reading this guide, you will know: * How to create a Rails application. @@ -387,7 +385,7 @@ Active Record version 4.0.0 Action Pack version 4.0.0 Action Mailer version 4.0.0 Active Support version 4.0.0 -Middleware Rack::Sendfile, ActionDispatch::Static, Rack::Lock, #<ActiveSupport::Cache::Strategy::LocalCache::Middleware:0x007ffd131a7c88>, Rack::Runtime, Rack::MethodOverride, ActionDispatch::RequestId, Rails::Rack::Logger, ActionDispatch::ShowExceptions, ActionDispatch::DebugExceptions, ActionDispatch::RemoteIp, ActionDispatch::Reloader, ActionDispatch::Callbacks, ActiveRecord::Migration::CheckPending, ActiveRecord::ConnectionAdapters::ConnectionManagement, ActiveRecord::QueryCache, ActionDispatch::Cookies, ActionDispatch::Session::EncryptedCookieStore, ActionDispatch::Flash, ActionDispatch::ParamsParser, Rack::Head, Rack::ConditionalGet, Rack::ETag +Middleware Rack::Sendfile, ActionDispatch::Static, Rack::Lock, #<ActiveSupport::Cache::Strategy::LocalCache::Middleware:0x007ffd131a7c88>, Rack::Runtime, Rack::MethodOverride, ActionDispatch::RequestId, Rails::Rack::Logger, ActionDispatch::ShowExceptions, ActionDispatch::DebugExceptions, ActionDispatch::RemoteIp, ActionDispatch::Reloader, ActionDispatch::Callbacks, ActiveRecord::Migration::CheckPending, ActiveRecord::ConnectionAdapters::ConnectionManagement, ActiveRecord::QueryCache, ActionDispatch::Cookies, ActionDispatch::Session::CookieStore, ActionDispatch::Flash, ActionDispatch::ParamsParser, Rack::Head, Rack::ConditionalGet, Rack::ETag Application root /home/foobar/commandsapp Environment development Database adapter sqlite3 diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 5f170474ee..b14f8b6e7f 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -273,6 +273,12 @@ config.middleware.delete "Rack::MethodOverride" * `config.active_record.cache_timestamp_format` controls the format of the timestamp value in the cache key. Default is `:number`. +* `config.active_record.record_timestamps` is a boolean value which controls whether or not timestamping of `create` and `update` operations on a model occur. The default value is `true`. + +* `config.active_record.partial_writes` is a boolean value and controls whether or not partial writes are used (i.e. whether updates only set attributes that are dirty). Note that when using partial writes, you should also use optimistic locking `config.active_record.lock_optimistically` since concurrent updates may write attributes based on a possibly stale read state. The default value is `true`. + +* `config.active_record.attribute_types_cached_by_default` sets the attribute types that `ActiveRecord::AttributeMethods` will cache by default on reads. The default is `[:datetime, :timestamp, :time, :date]`. + The MySQL adapter adds one additional configuration option: * `ActiveRecord::ConnectionAdapters::MysqlAdapter.emulate_booleans` controls whether Active Record will consider all `tinyint(1)` columns in a MySQL database to be booleans and is true by default. @@ -303,7 +309,7 @@ The schema dumper adds one additional configuration option: * `config.action_controller.permit_all_parameters` sets all the parameters for mass assignment to be permitted by default. The default value is `false`. -* `config.action_controller.action_on_unpermitted_params` enables logging or raising an exception if parameters that are not explicitly permitted are found. Set to `:log` or `:raise` to enable. The default value is `:log` in development and test environments, and `false` in all other environments. +* `config.action_controller.action_on_unpermitted_parameters` enables logging or raising an exception if parameters that are not explicitly permitted are found. Set to `:log` or `:raise` to enable. The default value is `:log` in development and test environments, and `false` in all other environments. ### Configuring Action Dispatch @@ -528,7 +534,7 @@ Change the username and password in the `development` section as appropriate. By default Rails ships with three environments: "development", "test", and "production". While these are sufficient for most use cases, there are circumstances when you want more environments. -Imagine you have a server which mirrors the production environment but is only used for testing. Such a server is commonly called a "staging server". To define an environment called "staging" for this server just by create a file called `config/environments/staging.rb`. Please use the contents of any existing file in `config/environments` as a starting point and make the necessary changes from there. +Imagine you have a server which mirrors the production environment but is only used for testing. Such a server is commonly called a "staging server". To define an environment called "staging" for this server, just create a file called `config/environments/staging.rb`. Please use the contents of any existing file in `config/environments` as a starting point and make the necessary changes from there. That environment is no different than the default ones, start a server with `rails server -e staging`, a console with `rails console staging`, `Rails.env.staging?` works, etc. @@ -604,7 +610,7 @@ Rails has 5 initialization events which can be hooked into (listed in the order * `before_eager_load`: This is run directly before eager loading occurs, which is the default behavior for the `production` environment and not for the `development` environment. -* `after_initialize`: Run directly after the initialization of the application, but before the application initializers are run. +* `after_initialize`: Run directly after the initialization of the application, after the application initializers in `config/initializers` are run. To define an event for these hooks, use the block syntax within a `Rails::Application`, `Rails::Railtie` or `Rails::Engine` subclass: @@ -759,4 +765,15 @@ Since the connection pooling is handled inside of Active Record by default, all Any one request will check out a connection the first time it requires access to the database, after which it will check the connection back in, at the end of the request, meaning that the additional connection slot will be available again for the next request in the queue. +If you try to use more connections than are available, Active Record will block +and wait for a connection from the pool. When it cannot get connection, a timeout +error similar to given below will be thrown. + +```ruby +ActiveRecord::ConnectionTimeoutError - could not obtain a database connection within 5 seconds. The max pool size is currently 5; consider increasing it: +``` + +If you get the above error, you might want to increase the size of connection +pool by incrementing the `pool` option in `database.yml` + NOTE. If you have enabled `Rails.threadsafe!` mode then there could be a chance that several threads may be accessing multiple connections simultaneously. So depending on your current request load, you could very well have multiple threads contending for a limited amount of connections. diff --git a/guides/source/documents.yaml b/guides/source/documents.yaml index 1b16f4e516..1bf9ff95e1 100644 --- a/guides/source/documents.yaml +++ b/guides/source/documents.yaml @@ -150,6 +150,13 @@ url: ruby_on_rails_guides_guidelines.html description: This guide documents the Ruby on Rails guides guidelines. - + name: Maintenance Policy + documents: + - + name: Maintenance Policy + url: maintenance_policy.html + description: What versions of Ruby on Rails are currently supported, and when to expect new versions. +- name: Release Notes documents: - diff --git a/guides/source/engines.md b/guides/source/engines.md index ec51fb9234..c71b728ef7 100644 --- a/guides/source/engines.md +++ b/guides/source/engines.md @@ -307,7 +307,11 @@ create test/models/blorgh/comment_test.rb create test/fixtures/blorgh/comments.yml ``` -This generator call will generate just the necessary model files it needs, namespacing the files under a `blorgh` directory and creating a model class called `Blorgh::Comment`. +This generator call will generate just the necessary model files it needs, namespacing the files under a `blorgh` directory and creating a model class called `Blorgh::Comment`. Now run the migration to create our blorgh_comments table: + +```bash +$ rake db:migrate +``` To show the comments on a post, edit `app/views/blorgh/posts/show.html.erb` and add this line before the "Edit" link: @@ -950,7 +954,7 @@ INFO. Remember that in order to use languages like Sass or CoffeeScript, you sho There are some situations where your engine's assets are not required by the host application. For example, say that you've created an admin functionality that only exists for your engine. In this case, the host application doesn't need to require `admin.css` -or `admin.js`. Only the gem's admin layout needs these assets. It doesn't make sense for the host app to include `"blorg/admin.css"` in it's stylesheets. In this situation, you should explicitly define these assets for precompilation. +or `admin.js`. Only the gem's admin layout needs these assets. It doesn't make sense for the host app to include `"blorgh/admin.css"` in it's stylesheets. In this situation, you should explicitly define these assets for precompilation. This tells sprockets to add your engine assets when `rake assets:precompile` is ran. You can define assets for precompilation in `engine.rb` diff --git a/guides/source/generators.md b/guides/source/generators.md index a8a34d0ac4..e06b13deba 100644 --- a/guides/source/generators.md +++ b/guides/source/generators.md @@ -35,7 +35,7 @@ $ rails generate helper --help Creating Your First Generator ----------------------------- -Since Rails 3.0, generators are built on top of [Thor](https://github.com/wycats/thor). Thor provides powerful options parsing and a great API for manipulating files. For instance, let's build a generator that creates an initializer file named `initializer.rb` inside `config/initializers`. +Since Rails 3.0, generators are built on top of [Thor](https://github.com/erikhuda/thor). Thor provides powerful options parsing and a great API for manipulating files. For instance, let's build a generator that creates an initializer file named `initializer.rb` inside `config/initializers`. The first step is to create a file at `lib/generators/initializer_generator.rb` with the following content: @@ -47,7 +47,7 @@ class InitializerGenerator < Rails::Generators::Base end ``` -NOTE: `create_file` is a method provided by `Thor::Actions`. Documentation for `create_file` and other Thor methods can be found in [Thor's documentation](http://rdoc.info/github/wycats/thor/master/Thor/Actions.html) +NOTE: `create_file` is a method provided by `Thor::Actions`. Documentation for `create_file` and other Thor methods can be found in [Thor's documentation](http://rdoc.info/github/erikhuda/thor/master/Thor/Actions.html) Our new generator is quite simple: it inherits from `Rails::Generators::Base` and has one method definition. When a generator is invoked, each public method in the generator is executed sequentially in the order that it is defined. Finally, we invoke the `create_file` method that will create a file at the given destination with the given content. If you are familiar with the Rails Application Templates API, you'll feel right at home with the new generators API. @@ -171,7 +171,7 @@ Before we customize our workflow, let's first see what our scaffold looks like: ```bash $ rails generate scaffold User name:string invoke active_record - create db/migrate/20091120125558_create_users.rb + create db/migrate/20130924151154_create_users.rb create app/models/user.rb invoke test_unit create test/models/user_test.rb @@ -193,6 +193,9 @@ $ rails generate scaffold User name:string create app/helpers/users_helper.rb invoke test_unit create test/helpers/users_helper_test.rb + invoke jbuilder + create app/views/users/index.json.jbuilder + create app/views/users/show.json.jbuilder invoke assets invoke coffee create app/assets/javascripts/users.js.coffee @@ -221,11 +224,18 @@ To demonstrate this, we are going to create a new helper generator that simply a ```bash $ rails generate generator rails/my_helper + create lib/generators/rails/my_helper + create lib/generators/rails/my_helper/my_helper_generator.rb + create lib/generators/rails/my_helper/USAGE + create lib/generators/rails/my_helper/templates ``` -After that, we can delete both the `templates` directory and the `source_root` class method from our new generators, because we are not going to need them. So our new generator looks like the following: +After that, we can delete both the `templates` directory and the `source_root` +class method call from our new generator, because we are not going to need them. +Add the method below, so our generator looks like the following: ```ruby +# lib/generators/rails/my_helper/my_helper_generator.rb class Rails::MyHelperGenerator < Rails::Generators::NamedBase def create_helper_file create_file "app/helpers/#{file_name}_helper.rb", <<-FILE @@ -241,6 +251,7 @@ We can try out our new generator by creating a helper for users: ```bash $ rails generate my_helper products + create app/helpers/products_helper.rb ``` And it will generate the following helper file in `app/helpers`: @@ -279,6 +290,7 @@ Since Rails 3.0, this is easy to do due to the hooks concept. Our new helper doe To do that, we can change the generator this way: ```ruby +# lib/generators/rails/my_helper/my_helper_generator.rb class Rails::MyHelperGenerator < Rails::Generators::NamedBase def create_helper_file create_file "app/helpers/#{file_name}_helper.rb", <<-FILE @@ -351,7 +363,7 @@ Now, if you create a Comment scaffold, you will see that the shoulda generators ```bash $ rails generate scaffold Comment body:text invoke active_record - create db/migrate/20091120151323_create_comments.rb + create db/migrate/20130924143118_create_comments.rb create app/models/comment.rb invoke shoulda create test/models/comment_test.rb @@ -373,6 +385,9 @@ $ rails generate scaffold Comment body:text create app/helpers/comments_helper.rb invoke shoulda create test/helpers/comments_helper_test.rb + invoke jbuilder + create app/views/comments/index.json.jbuilder + create app/views/comments/show.json.jbuilder invoke assets invoke coffee create app/assets/javascripts/comments.js.coffee @@ -422,7 +437,7 @@ Generator methods The following are methods available for both generators and templates for Rails. -NOTE: Methods provided by Thor are not covered this guide and can be found in [Thor's documentation](http://rdoc.info/github/wycats/thor/master/Thor/Actions.html) +NOTE: Methods provided by Thor are not covered this guide and can be found in [Thor's documentation](http://rdoc.info/github/erikhuda/thor/master/Thor/Actions.html) ### `gem` diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index e5bc5ef038..bb2e8e906f 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -22,8 +22,8 @@ with Rails. However, to get the most out of it, you need to have some prerequisites installed: * The [Ruby](http://www.ruby-lang.org/en/downloads) language version 1.9.3 or newer -* The [RubyGems](http://rubygems.org/) packaging system - * To learn more about RubyGems, please read the [RubyGems User Guide](http://docs.rubygems.org/read/book/1) +* The [RubyGems](http://rubygems.org) packaging system + * To learn more about RubyGems, please read the [RubyGems Guides](http://guides.rubygems.org) * A working installation of the [SQLite3 Database](http://www.sqlite.org) Rails is a web application framework running on the Ruby programming language. @@ -262,7 +262,7 @@ of code: ### Setting the Application Home Page Now that we have made the controller and view, we need to tell Rails when we -want Hello Rails! to show up. In our case, we want it to show up when we +want `Hello, Rails!` to show up. In our case, we want it to show up when we navigate to the root URL of our site, <http://localhost:3000>. At the moment, "Welcome Aboard" is occupying that spot. @@ -522,6 +522,7 @@ Edit the `form_for` line inside `app/views/posts/new.html.erb` to look like this In this example, the `posts_path` helper is passed to the `:url` option. To see what Rails will do with this, we look back at the output of `rake routes`: + ```bash $ rake routes Prefix Verb URI Pattern Controller#Action @@ -535,6 +536,7 @@ edit_post GET /posts/:id/edit(.:format) posts#edit DELETE /posts/:id(.:format) posts#destroy root / welcome#index ``` + The `posts_path` helper tells Rails to point the form to the URI Pattern associated with the `posts` prefix; and the form will (by default) send a `POST` request @@ -687,7 +689,7 @@ invoking the command: `rake db:migrate RAILS_ENV=production`. ### Saving data in the controller -Back in `posts_controller`, we need to change the `create` action +Back in `PostsController`, we need to change the `create` action to use the new `Post` model to save the data in the database. Open `app/controllers/posts_controller.rb` and change the `create` action to look like this: @@ -806,7 +808,7 @@ The route for this as per output of `rake routes` is: posts GET /posts(.:format) posts#index ``` -And an action for that route inside the `PostsController` in the `app/controllers/posts_controller.rb` file: +Add the corresponding `index` action for that route inside the `PostsController` in the `app/controllers/posts_controller.rb` file: ```ruby def index @@ -846,7 +848,7 @@ Open `app/views/welcome/index.html.erb` and modify it as follows: ```html+erb <h1>Hello, Rails!</h1> -<%= link_to "My Blog", controller: "posts" %> +<%= link_to 'My Blog', controller: 'posts' %> ``` The `link_to` method is one of Rails' built-in view helpers. It creates a @@ -1013,7 +1015,7 @@ errors with `@post.errors.full_messages`. arguments. If the number is greater than one, the string will be automatically pluralized. -The reason why we added `@post = Post.new` in `posts_controller` is that +The reason why we added `@post = Post.new` in the `PostsController` is that otherwise `@post` would be `nil` in our view, and calling `@post.errors.any?` would throw an error. @@ -1031,7 +1033,7 @@ attempt to do just that on the new post form [(http://localhost:3000/posts/new)] We've covered the "CR" part of CRUD. Now let's focus on the "U" part, updating posts. -The first step we'll take is adding an `edit` action to `posts_controller`. +The first step we'll take is adding an `edit` action to the `PostsController`. ```ruby def edit @@ -1132,7 +1134,7 @@ appear next to the "Show" link: <tr> <td><%= post.title %></td> <td><%= post.text %></td> - <td><%= link_to 'Show', post %></td> + <td><%= link_to 'Show', post_path(post) %></td> <td><%= link_to 'Edit', edit_post_path(post) %></td> </tr> <% end %> @@ -1338,7 +1340,7 @@ class Comment < ActiveRecord::Base end ``` -This is very similar to the `post.rb` model that you saw earlier. The difference +This is very similar to the `Post` model that you saw earlier. The difference is the line `belongs_to :post`, which sets up an Active Record _association_. You'll learn a little about associations in the next section of this guide. @@ -1816,6 +1818,7 @@ class CommentsController < ApplicationController @post = Post.find(params[:post_id]) ... end + # snipped for brevity ``` diff --git a/guides/source/i18n.md b/guides/source/i18n.md index 948b6f167c..33daa79133 100644 --- a/guides/source/i18n.md +++ b/guides/source/i18n.md @@ -843,6 +843,7 @@ So, for example, instead of the default error message `"can not be blank"` you c | numericality | :equal_to | :equal_to | count | | numericality | :less_than | :less_than | count | | numericality | :less_than_or_equal_to | :less_than_or_equal_to | count | +| numericality | :only_integer | :not_an_integer | - | | numericality | :odd | :odd | - | | numericality | :even | :even | - | diff --git a/guides/source/maintenance_policy.md b/guides/source/maintenance_policy.md new file mode 100644 index 0000000000..93729c6f72 --- /dev/null +++ b/guides/source/maintenance_policy.md @@ -0,0 +1,56 @@ +Maintenance Policy for Ruby on Rails +==================================== + +Support of the Rails framework is divided into four groups: New features, bug +fixes, security issues, and severe security issues. They are handled as +follows, all versions in x.y.z format + +-------------------------------------------------------------------------------- + +New Features +------------ + +New features are only added to the master branch and will not be made available +in point releases. + +Bug Fixes +--------- + +Only the latest release series will receive bug fixes. When enough bugs are +fixed and its deemed worthy to release a new gem, this is the branch it happens +from. + +**Currently included series:** 4.0.z + +Security Issues +--------------- + +The current release series and the next most recent one will receive patches +and new versions in case of a security issue. + +These releases are created by taking the last released version, applying the +security patches, and releasing. Those patches are then applied to the end of +the x-y-stable branch. For example, a theoretical 1.2.3 security release would +be built from 1.2.2, and then added to the end of 1-2-stable. This means that +security releases are easy to upgrade to if you're running the latest version +of Rails. + +**Currently included series:** 4.0.z, 3.2.z + +Severe Security Issues +---------------------- + +For severe security issues we will provide new versions as above, and also the +last major release series will receive patches and new versions. The +classification of the security issue is judged by the core team. + +**Currently included series:** 4.0.z, 3.2.z + +Unsupported Release Series +-------------------------- + +When a release series is no longer supported, it's your own responsibility to +deal with bugs and security issues. We may provide backports of the fixes and +publish them to git, however there will be no new versions released. If you are +not comfortable maintaining your own versions, you should upgrade to a +supported version. diff --git a/guides/source/migrations.md b/guides/source/migrations.md index 3c512e0390..b7283d16cc 100644 --- a/guides/source/migrations.md +++ b/guides/source/migrations.md @@ -184,7 +184,7 @@ class RemovePartNumberFromProducts < ActiveRecord::Migration end ``` -You are not limited to one magically generated column. For example +You are not limited to one magically generated column. For example: ```bash $ rails generate migration AddDetailsToProducts part_number:string price:decimal @@ -227,7 +227,7 @@ or remove from it as you see fit by editing the `db/migrate/YYYYMMDDHHMMSS_add_details_to_products.rb` file. Also, the generator accepts column type as `references`(also available as -`belongs_to`). For instance +`belongs_to`). For instance: ```bash $ rails generate migration AddUserRefToProducts user:references @@ -269,7 +269,7 @@ end The model and scaffold generators will create migrations appropriate for adding a new model. This migration will already contain instructions for creating the relevant table. If you tell Rails what columns you want, then statements for -adding these columns will also be created. For example, running +adding these columns will also be created. For example, running: ```bash $ rails generate model Product name:string description:text @@ -301,8 +301,9 @@ braces. You can use the following modifiers: * `precision` Defines the precision for the `decimal` fields * `scale` Defines the scale for the `decimal` fields * `polymorphic` Adds a `type` column for `belongs_to` associations +* `null` Allows or disallows `NULL` values in the column. -For instance, running +For instance, running: ```bash $ rails generate migration AddDetailsToProducts 'price:decimal{5,2}' supplier:references{polymorphic} @@ -313,7 +314,7 @@ will produce a migration that looks like this ```ruby class AddDetailsToProducts < ActiveRecord::Migration def change - add_column :products, :price, precision: 5, scale: 2 + add_column :products, :price, :decimal, precision: 5, scale: 2 add_reference :products, :supplier, polymorphic: true, index: true end end @@ -344,7 +345,7 @@ By default, `create_table` will create a primary key called `id`. You can change the name of the primary key with the `:primary_key` option (don't forget to update the corresponding model) or, if you don't want a primary key at all, you can pass the option `id: false`. If you need to pass database specific options -you can place an SQL fragment in the `:options` option. For example, +you can place an SQL fragment in the `:options` option. For example: ```ruby create_table :products, options: "ENGINE=BLACKHOLE" do |t| @@ -358,7 +359,7 @@ will append `ENGINE=BLACKHOLE` to the SQL statement used to create the table ### Creating a Join Table Migration method `create_join_table` creates a HABTM join table. A typical use -would be +would be: ```ruby create_join_table :products, :categories @@ -377,7 +378,7 @@ will create the `product_id` and `category_id` with the `:null` option as `true`. You can pass the option `:table_name` when you want to customize the table -name. For example, +name. For example: ```ruby create_join_table :products, :categories, table_name: :categorization @@ -399,7 +400,7 @@ end A close cousin of `create_table` is `change_table`, used for changing existing tables. It is used in a similar fashion to `create_table` but the object -yielded to the block knows more tricks. For example +yielded to the block knows more tricks. For example: ```ruby change_table :products do |t| @@ -463,7 +464,7 @@ or write the `up` and `down` methods instead of using the `change` method. Complex migrations may require processing that Active Record doesn't know how to reverse. You can use `reversible` to specify what to do when running a -migration what else to do when reverting it. For example, +migration what else to do when reverting it. For example: ```ruby class ExampleMigration < ActiveRecord::Migration @@ -647,7 +648,7 @@ will update your `db/schema.rb` file to match the structure of your database. If you specify a target version, Active Record will run the required migrations (change, up, down) until it has reached the specified version. The version is the numerical prefix on the migration's filename. For example, to migrate -to version 20080906120000 run +to version 20080906120000 run: ```bash $ rake db:migrate VERSION=20080906120000 @@ -664,7 +665,7 @@ down to, but not including, 20080906120000. A common task is to rollback the last migration. For example, if you made a mistake in it and wish to correct it. Rather than tracking down the version -number associated with the previous migration you can run +number associated with the previous migration you can run: ```bash $ rake db:rollback @@ -682,7 +683,7 @@ will revert the last 3 migrations. The `db:migrate:redo` task is a shortcut for doing a rollback and then migrating back up again. As with the `db:rollback` task, you can use the `STEP` parameter -if you need to go more than one version back, for example +if you need to go more than one version back, for example: ```bash $ rake db:migrate:redo STEP=3 @@ -712,7 +713,7 @@ contents of the current `schema.rb` file. If a migration can't be rolled back, If you need to run a specific migration up or down, the `db:migrate:up` and `db:migrate:down` tasks will do that. Just specify the appropriate version and the corresponding migration will have its `change`, `up` or `down` method -invoked, for example, +invoked, for example: ```bash $ rake db:migrate:up VERSION=20080906120000 @@ -754,7 +755,7 @@ Several methods are provided in migrations that allow you to control all this: | say | Takes a message argument and outputs it as is. A second boolean argument can be passed to specify whether to indent or not. | say_with_time | Outputs text along with how long it took to run its block. If the block returns an integer it assumes it is the number of rows affected. -For example, this migration +For example, this migration: ```ruby class CreateProducts < ActiveRecord::Migration @@ -1039,8 +1040,8 @@ this, then you should set the schema format to `:sql`. Instead of using Active Record's schema dumper, the database's structure will be dumped using a tool specific to the database (via the `db:structure:dump` Rake task) into `db/structure.sql`. For example, for PostgreSQL, the `pg_dump` -utility is used. For MySQL, this file will contain the output of `SHOW CREATE -TABLE` for the various tables. +utility is used. For MySQL, this file will contain the output of +`SHOW CREATE TABLE` for the various tables. Loading these schemas is simply a question of executing the SQL statements they contain. By definition, this will create a perfect copy of the database's diff --git a/guides/source/plugins.md b/guides/source/plugins.md index ca55ee0df2..d0aa2e55a2 100644 --- a/guides/source/plugins.md +++ b/guides/source/plugins.md @@ -34,9 +34,15 @@ different rails applications using RubyGems and Bundler if desired. Rails ships with a `rails plugin new` command which creates a - skeleton for developing any kind of Rails extension with the ability - to run integration tests using a dummy Rails application. See usage - and options by asking for help: +skeleton for developing any kind of Rails extension with the ability +to run integration tests using a dummy Rails application. Create your +plugin with the command: + +```bash +$ rails plugin new yaffle +``` + +See usage and options by asking for help: ```bash $ rails plugin --help diff --git a/guides/source/security.md b/guides/source/security.md index 4aba39f55a..d7a41497f8 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -290,7 +290,7 @@ NOTE: _Make sure file uploads don't overwrite important files, and process media Many web applications allow users to upload files. _File names, which the user may choose (partly), should always be filtered_ as an attacker could use a malicious file name to overwrite any file on the server. If you store file uploads at /var/www/uploads, and the user enters a file name like "../../../etc/passwd", it may overwrite an important file. Of course, the Ruby interpreter would need the appropriate permissions to do so - one more reason to run web servers, database servers and other programs as a less privileged Unix user. -When filtering user input file names, _don't try to remove malicious parts_. Think of a situation where the web application removes all "../" in a file name and an attacker uses a string such as "....//" - the result will be "../". It is best to use a whitelist approach, which _checks for the validity of a file name with a set of accepted characters_. This is opposed to a blacklist approach which attempts to remove not allowed characters. In case it isn't a valid file name, reject it (or replace not accepted characters), but don't remove them. Here is the file name sanitizer from the [attachment_fu plugin](https://github.com/technoweenie/attachment_fu/tree/master:) +When filtering user input file names, _don't try to remove malicious parts_. Think of a situation where the web application removes all "../" in a file name and an attacker uses a string such as "....//" - the result will be "../". It is best to use a whitelist approach, which _checks for the validity of a file name with a set of accepted characters_. This is opposed to a blacklist approach which attempts to remove not allowed characters. In case it isn't a valid file name, reject it (or replace not accepted characters), but don't remove them. Here is the file name sanitizer from the [attachment_fu plugin](https://github.com/technoweenie/attachment_fu/tree/master): ```ruby def sanitize_filename(filename) @@ -447,7 +447,7 @@ Here are some ideas how to hide honeypot fields by JavaScript and/or CSS: The most simple negative CAPTCHA is one hidden honeypot field. On the server side, you will check the value of the field: If it contains any text, it must be a bot. Then, you can either ignore the post or return a positive result, but not saving the post to the database. This way the bot will be satisfied and moves on. You can do this with annoying users, too. -You can find more sophisticated negative CAPTCHAs in Ned Batchelder's [blog post](http://nedbatchelder.com/text/stopbots.html:) +You can find more sophisticated negative CAPTCHAs in Ned Batchelder's [blog post](http://nedbatchelder.com/text/stopbots.html): * Include a field with the current UTC time-stamp in it and check it on the server. If it is too far in the past, or if it is in the future, the form is invalid. * Randomize the field names @@ -760,7 +760,7 @@ The following is an excerpt from the [Js.Yamanner@m](http://www.symantec.com/sec The worms exploits a hole in Yahoo's HTML/JavaScript filter, which usually filters all target and onload attributes from tags (because there can be JavaScript). The filter is applied only once, however, so the onload attribute with the worm code stays in place. This is a good example why blacklist filters are never complete and why it is hard to allow HTML/JavaScript in a web application. -Another proof-of-concept webmail worm is Nduja, a cross-domain worm for four Italian webmail services. Find more details on [Rosario Valotta's paper](http://www.xssed.com/article/9/Paper_A_PoC_of_a_cross_webmail_worm_XWW_called_Njuda_connection/). Both webmail worms have the goal to harvest email addresses, something a criminal hacker could make money with. +Another proof-of-concept webmail worm is Nduja, a cross-domain worm for four Italian webmail services. Find more details on [Rosario Valotta's paper](http://www.xssed.com/news/37/Nduja_Connection_A_cross_webmail_worm_XWW/). Both webmail worms have the goal to harvest email addresses, something a criminal hacker could make money with. In December 2006, 34,000 actual user names and passwords were stolen in a [MySpace phishing attack](http://news.netcraft.com/archives/2006/10/27/myspace_accounts_compromised_by_phishers.html). The idea of the attack was to create a profile page named "login_home_index_html", so the URL looked very convincing. Specially-crafted HTML and CSS was used to hide the genuine MySpace content from the page and instead display its own login form. diff --git a/guides/source/testing.md b/guides/source/testing.md index 5b8829b89a..edf4813d74 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -438,10 +438,12 @@ Now that we have used Rails scaffold generator for our `Post` resource, it has a Let me take you through one such test, `test_should_get_index` from the file `posts_controller_test.rb`. ```ruby -test "should get index" do - get :index - assert_response :success - assert_not_nil assigns(:posts) +class PostsControllerTest < ActionController::TestCase + test "should get index" do + get :index + assert_response :success + assert_not_nil assigns(:posts) + end end ``` @@ -532,7 +534,7 @@ instance variable: ```ruby # setting a HTTP Header -@request.headers["Accepts"] = "text/plain, text/html" +@request.headers["Accept"] = "text/plain, text/html" get :index # simulate the request with custom header # setting a CGI variable diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index 3daa10ea07..224213268e 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -339,7 +339,7 @@ config.assets.js_compressor = :uglifier ### sass-rails -* `asset_url` with two arguments is deprecated. For example: `asset-url("rails.png", image)` becomes `asset-url("rails.png")` +* `asset-url` with two arguments is deprecated. For example: `asset-url("rails.png", image)` becomes `asset-url("rails.png")` Upgrading from Rails 3.1 to Rails 3.2 ------------------------------------- diff --git a/rails.gemspec b/rails.gemspec index b426faf0e8..d1c199a97a 100644 --- a/rails.gemspec +++ b/rails.gemspec @@ -21,6 +21,7 @@ Gem::Specification.new do |s| s.add_dependency 'activesupport', version s.add_dependency 'actionpack', version s.add_dependency 'actionview', version + s.add_dependency 'activemodel', version s.add_dependency 'activerecord', version s.add_dependency 'actionmailer', version s.add_dependency 'railties', version diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 797cffc884..4e6bf899af 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,16 @@ +* Expose MiddlewareStack#unshift to environment configuration. + + *Ben Pickles* + +* Include `web-console` into newly generated applications' Gemfile. + + *Genadi Samokovarov* + +* `rails server` will only extend the logger to output to STDOUT + in development environment. + + *Richard Schneeman* + * Don't require passing path to app before options in `rails new` and `rails plugin new` diff --git a/railties/lib/rails/commands/server.rb b/railties/lib/rails/commands/server.rb index 87d6505ed5..485bd1eb09 100644 --- a/railties/lib/rails/commands/server.rb +++ b/railties/lib/rails/commands/server.rb @@ -32,7 +32,8 @@ module Rails opt_parser.parse! args - options[:server] = args.shift + options[:log_stdout] = options[:daemonize].blank? && options[:environment] == "development" + options[:server] = args.shift options end end @@ -74,7 +75,7 @@ module Rails FileUtils.mkdir_p(File.join(Rails.root, 'tmp', dir_to_make)) end - unless options[:daemonize] + if options[:log_stdout] wrapped_app # touch the app so the logger is set up console = ActiveSupport::Logger.new($stdout) diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index c694513960..f5d7dede66 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -59,6 +59,10 @@ module Rails @operations << [__method__, args, block] end + def unshift(*args, &block) + @operations << [__method__, args, block] + end + def merge_into(other) #:nodoc: @operations.each do |operation, args, block| other.send(operation, *args, &block) diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index cce11ba8bc..92c876c835 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -335,7 +335,7 @@ module Rails def handle_rails_rc! unless argv.delete("--no-rc") - insert_railsrc(railsrc) + insert_railsrc_into_argv!(railsrc) end end @@ -347,7 +347,7 @@ module Rails end end - def insert_railsrc(railsrc) + def insert_railsrc_into_argv!(railsrc) if File.exist?(railsrc) extra_args_string = File.read(railsrc) extra_args = extra_args_string.split(/\n+/).map {|l| l.split}.flatten diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index adc83353b4..edc76e6c34 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -10,13 +10,16 @@ source 'https://rubygems.org' # Build JSON APIs with ease. Read more: https://github.com/rails/jbuilder gem 'jbuilder', '~> 1.2' +# Run `rails console` in the browser. Read more: https://github.com/rails/web-console +gem 'web-console', group: :development + group :doc do # bundle exec rake doc:rails generates the API under doc/api. gem 'sdoc', require: false end # Use ActiveModel has_secure_password -# gem 'bcrypt-ruby', '~> 3.1.0' +# gem 'bcrypt-ruby', '~> 3.1.2' # Use unicorn as the app server # gem 'unicorn' diff --git a/railties/lib/rails/generators/rails/app/templates/config/databases/postgresql.yml b/railties/lib/rails/generators/rails/app/templates/config/databases/postgresql.yml index eb569b7dab..0194dce6f3 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/databases/postgresql.yml +++ b/railties/lib/rails/generators/rails/app/templates/config/databases/postgresql.yml @@ -55,6 +55,8 @@ production: adapter: postgresql encoding: unicode database: <%= app_name %>_production + # For details on connection pooling, see rails configration guide + # http://guides.rubyonrails.org/configuring.html#database-pooling pool: 5 username: <%= app_name %> password: diff --git a/railties/lib/rails/rack/logger.rb b/railties/lib/rails/rack/logger.rb index ef4cdcb080..3b35798679 100644 --- a/railties/lib/rails/rack/logger.rb +++ b/railties/lib/rails/rack/logger.rb @@ -11,7 +11,6 @@ module Rails def initialize(app, taggers = nil) @app = app @taggers = taggers || [] - @instrumenter = ActiveSupport::Notifications.instrumenter end def call(env) @@ -33,7 +32,8 @@ module Rails logger.debug '' end - @instrumenter.start 'request.action_dispatch', request: request + instrumenter = ActiveSupport::Notifications.instrumenter + instrumenter.start 'request.action_dispatch', request: request logger.info started_request_message(request) resp = @app.call(env) resp[2] = ::Rack::BodyProxy.new(resp[2]) { finish(request) } @@ -70,7 +70,8 @@ module Rails private def finish(request) - @instrumenter.finish 'request.action_dispatch', request: request + instrumenter = ActiveSupport::Notifications.instrumenter + instrumenter.finish 'request.action_dispatch', request: request end def development? diff --git a/railties/test/application/middleware/remote_ip_test.rb b/railties/test/application/middleware/remote_ip_test.rb index 91c5807379..946b82eeb3 100644 --- a/railties/test/application/middleware/remote_ip_test.rb +++ b/railties/test/application/middleware/remote_ip_test.rb @@ -33,6 +33,16 @@ module ApplicationTests end end + test "works with both headers individually" do + make_basic_app + assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do + assert_equal "1.1.1.1", remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1") + end + assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do + assert_equal "1.1.1.2", remote_ip("HTTP_CLIENT_IP" => "1.1.1.2") + end + end + test "can disable IP spoofing check" do make_basic_app do |app| app.config.action_dispatch.ip_spoofing_check = false diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 31a35a09bb..20d1d76d78 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -144,6 +144,12 @@ module ApplicationTests assert_equal "Rack::Config", middleware.second end + test 'unshift middleware' do + add_to_config 'config.middleware.unshift Rack::Config' + boot! + assert_equal 'Rack::Config', middleware.first + end + test "Rails.cache does not respond to middleware" do add_to_config "config.cache_store = :memory_store" boot! diff --git a/railties/test/commands/server_test.rb b/railties/test/commands/server_test.rb index cb57b3c0cd..20afca2618 100644 --- a/railties/test/commands/server_test.rb +++ b/railties/test/commands/server_test.rb @@ -39,4 +39,14 @@ class Rails::ServerTest < ActiveSupport::TestCase assert_equal 'production', server.options[:environment] end end + + def test_log_stdout + args = ["-e", "development"] + options = Rails::Server::Options.new.parse!(args) + assert_equal true, options[:log_stdout] + + args = ["-e", "production"] + options = Rails::Server::Options.new.parse!(args) + assert_equal false, options[:log_stdout] + end end |