diff options
113 files changed, 1071 insertions, 338 deletions
diff --git a/.travis.yml b/.travis.yml index d29ef1702a..d465378c72 100644 --- a/.travis.yml +++ b/.travis.yml @@ -59,14 +59,14 @@ matrix: - "GEM=ar:mysql2" addons: mariadb: 10.0 - - rvm: jruby-9.0.5.0 + - rvm: jruby-9.1.5.0 jdk: oraclejdk8 env: - "JRUBY_OPTS='--dev -J-Xmx1024M'" - "GEM='ap'" allow_failures: - rvm: ruby-head - - rvm: jruby-9.0.5.0 + - rvm: jruby-9.1.5.0 fast_finish: true notifications: diff --git a/Gemfile.lock b/Gemfile.lock index 4257685c46..1a39484c2b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -322,9 +322,9 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) - sqlite3 (1.3.11) - sqlite3 (1.3.11-x64-mingw32) - sqlite3 (1.3.11-x86-mingw32) + sqlite3 (1.3.12) + sqlite3 (1.3.12-x64-mingw32) + sqlite3 (1.3.12-x86-mingw32) stackprof (0.2.10) sucker_punch (2.0.2) concurrent-ruby (~> 1.0.0) diff --git a/actioncable/CHANGELOG.md b/actioncable/CHANGELOG.md index d70d32ce07..2c84d3158f 100644 --- a/actioncable/CHANGELOG.md +++ b/actioncable/CHANGELOG.md @@ -1,7 +1,7 @@ * Permit same-origin connections by default. - New option `config.action_cable.allow_same_origin_as_host = false` - to disable. + Added new option `config.action_cable.allow_same_origin_as_host = false` + to disable this behaviour. *Dávid Halász*, *Matthew Draper* @@ -13,7 +13,7 @@ *Vladimir Dementyev* -* Buffer writes to websocket connections, to avoid blocking threads +* Buffer now writes to websocket connections, to avoid blocking threads that could be doing more useful things. *Matthew Draper*, *Tinco Andringa* @@ -23,7 +23,7 @@ *Tinco Andringa* -* Add ActiveSupport::Notifications hook to Broadcaster#broadcast. +* Add `ActiveSupport::Notifications` hook to `Broadcaster#broadcast`. *Matthew Wear* diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index c9c347ea26..37ccef2a78 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,4 +1,16 @@ -* Allow keys not found in RACK_KEY_TRANSLATION for setting the environment when rendering +* Add support for arbitrary hashes in strong parameters: + + ```ruby + params.permit(preferences: {}) + ``` + + *Xavier Noria* + +* Add `ActionController::Parameters#merge!`, which behaves the same as `Hash#merge!`. + + *Yuji Yaginuma* + +* Allow keys not found in `RACK_KEY_TRANSLATION` for setting the environment when rendering arbitrary templates. *Sammy Larbi* diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index 8e588812f8..603c2e9ea7 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -215,7 +215,7 @@ module AbstractController # ==== Returns # * <tt>string</tt> - The name of the method that handles the action # * false - No valid method name could be found. - # Raise AbstractController::ActionNotFound. + # Raise +AbstractController::ActionNotFound+. def _find_action_name(action_name) _valid_action_name?(action_name) && method_for_action(action_name) end @@ -231,11 +231,11 @@ module AbstractController # with a template matching the action name is considered to exist. # # If you override this method to handle additional cases, you may - # also provide a method (like _handle_method_missing) to handle + # also provide a method (like +_handle_method_missing+) to handle # the case. # - # If none of these conditions are true, and method_for_action - # returns nil, an AbstractController::ActionNotFound exception will be raised. + # If none of these conditions are true, and +method_for_action+ + # returns +nil+, an +AbstractController::ActionNotFound+ exception will be raised. # # ==== Parameters # * <tt>action_name</tt> - An action name to find a method name for diff --git a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb index 6c103bb042..798564db96 100644 --- a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb +++ b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb @@ -40,7 +40,7 @@ module ActionController end # Pick the template digest to include in the ETag. If the +:template+ option - # is present, use the named template. If +:template+ is nil or absent, use + # is present, use the named template. If +:template+ is +nil+ or absent, use # the default controller/action template. If +:template+ is false, omit the # template digest from the ETag. def pick_template_for_etag(options) diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 9c82abb640..e14da22e01 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -334,6 +334,15 @@ module ActionController # params = ActionController::Parameters.new(tags: ['rails', 'parameters']) # params.permit(tags: []) # + # Sometimes it is not possible or convenient to declare the valid keys of + # a hash parameter or its internal structure. Just map to an empty hash: + # + # params.permit(preferences: {}) + # + # but be careful because this opens the door to arbitrary input. In this + # case, +permit+ ensures values in the returned structure are permitted + # scalars and filters out anything else. + # # You can also use +permit+ on nested parameters, like: # # params = ActionController::Parameters.new({ @@ -382,14 +391,15 @@ module ActionController case filter when Symbol, String permitted_scalar_filter(params, filter) - when Hash then + when Hash hash_filter(params, filter) end end unpermitted_parameters!(params) if self.class.action_on_unpermitted_parameters - params.permit! + params.permitted = true + params end # Returns a parameter for the given +key+. If not found, @@ -539,7 +549,7 @@ module ActionController new_instance_with_inherited_permitted_status(@parameters.select(&block)) end - # Equivalent to Hash#keep_if, but returns nil if no changes were made. + # Equivalent to Hash#keep_if, but returns +nil+ if no changes were made. def select!(&block) @parameters.select!(&block) self @@ -573,6 +583,13 @@ module ActionController ) end + # Returns current <tt>ActionController::Parameters</tt> instance which + # +other_hash+ merges into current hash. + def merge!(other_hash) + @parameters.merge!(other_hash.to_h) + self + end + # This is required by ActiveModel attribute assignment, so that user can # pass +Parameters+ to a mass assignment methods in a model. It should not # matter as we are using +HashWithIndifferentAccess+ internally. @@ -759,6 +776,7 @@ module ActionController end EMPTY_ARRAY = [] + EMPTY_HASH = {} def hash_filter(params, filter) filter = filter.with_indifferent_access @@ -772,6 +790,11 @@ module ActionController array_of_permitted_scalars?(self[key]) do |val| params[key] = val end + elsif filter[key] == EMPTY_HASH + # Declaration { preferences: {} }. + if value.is_a?(Parameters) + params[key] = permit_any_in_parameters(value) + end elsif non_scalar?(value) # Declaration { user: :name } or { user: [:name, :age, { address: ... }] }. params[key] = each_element(value) do |element| @@ -781,6 +804,39 @@ module ActionController end end + def permit_any_in_parameters(params) + self.class.new.tap do |sanitized| + params.each do |key, value| + case value + when ->(v) { permitted_scalar?(v) } + sanitized[key] = value + when Array + sanitized[key] = permit_any_in_array(value) + when Parameters + sanitized[key] = permit_any_in_parameters(value) + else + # Filter this one out. + end + end + sanitized.permitted = true + end + end + + def permit_any_in_array(array) + [].tap do |sanitized| + array.each do |element| + case element + when ->(e) { permitted_scalar?(e) } + sanitized << element + when Parameters + sanitized << permit_any_in_parameters(element) + else + # Filter this one out. + end + end + end + end + def initialize_copy(source) super @parameters = @parameters.dup diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 803ac52605..357ca56036 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -249,7 +249,7 @@ module ActionDispatch # :nodoc: end end - # Sets the HTTP character set. In case of nil parameter + # Sets the HTTP character set. In case of +nil+ parameter # it sets the charset to utf-8. # # response.charset = 'utf-16' # => 'utf-16' diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 4fc4df4463..956c53e813 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -179,7 +179,7 @@ module ActionDispatch # Returns a jar that'll automatically generate a signed representation of cookie value and verify it when reading from # the cookie again. This is useful for creating cookies with values that the user is not supposed to change. If a signed - # cookie was tampered with by the user (or a 3rd party), nil will be returned. + # cookie was tampered with by the user (or a 3rd party), +nil+ will be returned. # # If +secrets.secret_key_base+ and +secrets.secret_token+ (deprecated) are both set, # legacy cookies signed with the old key generator will be transparently upgraded. @@ -202,7 +202,7 @@ module ActionDispatch end # Returns a jar that'll automatically encrypt cookie values before sending them to the client and will decrypt them for read. - # If the cookie was tampered with by the user (or a 3rd party), nil will be returned. + # If the cookie was tampered with by the user (or a 3rd party), +nil+ will be returned. # # If +secrets.secret_key_base+ and +secrets.secret_token+ (deprecated) are both set, # legacy cookies signed with the old key generator will be transparently upgraded. diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 6900934712..6dddcc6ee1 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -129,7 +129,7 @@ module ActionDispatch end # Builds a hash containing the flashes to keep for the next request. - # If there are none to keep, returns nil. + # If there are none to keep, returns +nil+. def to_session_value #:nodoc: flashes_to_keep = @flashes.except(*@discard) return nil if flashes_to_keep.empty? diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index 8b98009efc..a2a80f39fc 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -85,7 +85,7 @@ module ActionDispatch end # Returns value of the key stored in the session or - # nil if the given key is not found in the session. + # +nil+ if the given key is not found in the session. def [](key) load_for_read! @delegate[key.to_s] diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 1cb7173aa0..60bfb66f2f 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -168,6 +168,44 @@ class ParametersPermitTest < ActiveSupport::TestCase end end + test "key to empty hash: arbitrary hashes are permitted" do + params = ActionController::Parameters.new( + username: "fxn", + preferences: { + scheme: "Marazul", + font: { + name: "Source Code Pro", + size: 12 + }, + tabstops: [4, 8, 12, 16], + suspicious: [true, Object.new, false, /yo!/], + dubious: [{a: :a, b: /wtf!/}, {c: :c}], + injected: Object.new + }, + hacked: 1 # not a hash + ) + + permitted = params.permit(:username, preferences: {}, hacked: {}) + + assert permitted.permitted? + assert permitted[:preferences].permitted? + assert permitted[:preferences][:font].permitted? + assert permitted[:preferences][:dubious].all?(&:permitted?) + + assert_equal "fxn", permitted[:username] + assert_equal "Marazul", permitted[:preferences][:scheme] + assert_equal "Source Code Pro", permitted[:preferences][:font][:name] + assert_equal 12, permitted[:preferences][:font][:size] + assert_equal [4, 8, 12, 16], permitted[:preferences][:tabstops] + assert_equal [true, false], permitted[:preferences][:suspicious] + assert_equal :a, permitted[:preferences][:dubious][0][:a] + assert_equal :c, permitted[:preferences][:dubious][1][:c] + + assert_filtered_out permitted[:preferences][:dubious][0], :b + assert_filtered_out permitted[:preferences], :injected + assert_filtered_out permitted, :hacked + end + test "fetch raises ParameterMissing exception" do e = assert_raises(ActionController::ParameterMissing) do @params.fetch :foo @@ -244,6 +282,23 @@ class ParametersPermitTest < ActiveSupport::TestCase assert merged_params[:id] end + test "not permitted is sticky beyond merge!" do + assert_not @params.merge!(a: "b").permitted? + end + + test "permitted is sticky beyond merge!" do + @params.permit! + assert @params.merge!(a: "b").permitted? + end + + test "merge! with parameters" do + other_params = ActionController::Parameters.new(id: "1234").permit! + @params.merge!(other_params) + + assert_equal "1234", @params[:id] + assert_equal "32", @params[:person][:age] + end + test "modifying the parameters" do @params[:person][:hometown] = "Chicago" @params[:person][:family] = { brother: "Jonas" } diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 55ceced30c..05aa4ff6ad 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -385,7 +385,7 @@ XML $stderr = STDERR end - assert err.empty? + assert err.empty?, err.inspect end def test_assert_generates diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 67bd9b5c8f..6e6ce64e72 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -2,12 +2,12 @@ *Rafael Mendonça França* -* Render now accepts any keys for locals, including reserved words +* Render now accepts any keys for locals, including reserved keywords. Only locals with valid variable names get set directly. Others - will still be available in local_assigns. + will still be available in `local_assigns`. - Example of render with reserved words: + Example of render with reserved keywords: ```erb <%= render "example", class: "text-center", message: "Hello world!" %> diff --git a/actionview/lib/action_view/helpers/asset_url_helper.rb b/actionview/lib/action_view/helpers/asset_url_helper.rb index fab49e402b..a089502dd9 100644 --- a/actionview/lib/action_view/helpers/asset_url_helper.rb +++ b/actionview/lib/action_view/helpers/asset_url_helper.rb @@ -97,7 +97,7 @@ module ActionView # to avoid warnings in the client about mixed media. # Note that the request parameter might not be supplied, e.g. when the assets # are precompiled via a Rake task. Make sure to use a Proc instead of a lambda, - # since a Proc allows missing parameters and sets them to nil. + # since a +Proc+ allows missing parameters and sets them to +nil+. # # config.action_controller.asset_host = Proc.new { |source, request| # if request && request.ssl? @@ -232,7 +232,7 @@ module ActionView stylesheet: ".css" } - # Compute extname to append to asset path. Returns nil if + # Compute extname to append to asset path. Returns +nil+ if # nothing should be added. def compute_asset_extname(source, options = {}) return if options[:extname] == false diff --git a/actionview/lib/action_view/helpers/atom_feed_helper.rb b/actionview/lib/action_view/helpers/atom_feed_helper.rb index cef8098f67..3538515aee 100644 --- a/actionview/lib/action_view/helpers/atom_feed_helper.rb +++ b/actionview/lib/action_view/helpers/atom_feed_helper.rb @@ -163,7 +163,7 @@ module ActionView @xml, @view, @feed_options = xml, view, feed_options end - # Accepts a Date or Time object and inserts it in the proper format. If nil is passed, current time in UTC is used. + # Accepts a Date or Time object and inserts it in the proper format. If +nil+ is passed, current time in UTC is used. def updated(date_or_time = nil) @xml.updated((date_or_time || Time.now.utc).xmlschema) end @@ -174,7 +174,7 @@ module ActionView # # * <tt>:published</tt>: Time first published. Defaults to the created_at attribute on the record if one such exists. # * <tt>:updated</tt>: Time of update. Defaults to the updated_at attribute on the record if one such exists. - # * <tt>:url</tt>: The URL for this entry or false or nil for not having a link tag. Defaults to the polymorphic_url for the record. + # * <tt>:url</tt>: The URL for this entry or +false+ or +nil+ for not having a link tag. Defaults to the +polymorphic_url+ for the record. # * <tt>:id</tt>: The ID for this entry. Defaults to "tag:#{@view.request.host},#{@feed_options[:schema_date]}:#{record.class}/#{record.id}" # * <tt>:type</tt>: The TYPE for this entry. Defaults to "text/html". def entry(record, options = {}) diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb index 87d341d862..bf1c8ceaed 100644 --- a/actionview/lib/action_view/helpers/cache_helper.rb +++ b/actionview/lib/action_view/helpers/cache_helper.rb @@ -88,7 +88,7 @@ module ActionView # # === Explicit dependencies # - # Some times you'll have template dependencies that can't be derived at all. This is typically + # Sometimes you'll have template dependencies that can't be derived at all. This is typically # the case when you have template rendering that happens in helpers. Here's an example: # # <%= render_sortable_todolists @project.todolists %> diff --git a/actionview/lib/action_view/helpers/date_helper.rb b/actionview/lib/action_view/helpers/date_helper.rb index 8ec351b360..09dc6ef6bd 100644 --- a/actionview/lib/action_view/helpers/date_helper.rb +++ b/actionview/lib/action_view/helpers/date_helper.rb @@ -220,7 +220,7 @@ module ActionView # the respective locale (e.g. [:year, :month, :day] in the en locale that ships with Rails). # * <tt>:include_blank</tt> - Include a blank option in every select field so it's possible to set empty # dates. - # * <tt>:default</tt> - Set a default date if the affected date isn't set or is nil. + # * <tt>:default</tt> - Set a default date if the affected date isn't set or is +nil+. # * <tt>:selected</tt> - Set a date that overrides the actual value. # * <tt>:disabled</tt> - Set to true if you want show the select fields as disabled. # * <tt>:prompt</tt> - Set to true (for a generic prompt), a prompt string or a hash of prompt strings @@ -866,7 +866,7 @@ module ActionView end # Returns translated month names, but also ensures that a custom month - # name array has a leading nil element. + # name array has a leading +nil+ element. def month_names @month_names ||= begin month_names = @options[:use_month_names] || translated_month_names diff --git a/actionview/lib/action_view/helpers/text_helper.rb b/actionview/lib/action_view/helpers/text_helper.rb index bd3371ccc8..07dccf5b41 100644 --- a/actionview/lib/action_view/helpers/text_helper.rb +++ b/actionview/lib/action_view/helpers/text_helper.rb @@ -151,7 +151,7 @@ module ActionView # defined in <tt>:radius</tt> (which defaults to 100). If the excerpt radius overflows the beginning or end of the +text+, # then the <tt>:omission</tt> option (which defaults to "...") will be prepended/appended accordingly. Use the # <tt>:separator</tt> option to choose the delimitation. The resulting string will be stripped in any case. If the +phrase+ - # isn't found, nil is returned. + # isn't found, +nil+ is returned. # # excerpt('This is an example', 'an', radius: 5) # # => ...s is an exam... diff --git a/actionview/lib/action_view/layouts.rb b/actionview/lib/action_view/layouts.rb index e8abfeac52..bb1d3032ad 100644 --- a/actionview/lib/action_view/layouts.rb +++ b/actionview/lib/action_view/layouts.rb @@ -91,16 +91,16 @@ module ActionView # layout false # # In these examples, we have three implicit lookup scenarios: - # * The BankController uses the "bank" layout. - # * The ExchangeController uses the "exchange" layout. - # * The CurrencyController inherits the layout from BankController. + # * The +BankController+ uses the "bank" layout. + # * The +ExchangeController+ uses the "exchange" layout. + # * The +CurrencyController+ inherits the layout from BankController. # # However, when a layout is explicitly set, the explicitly set layout wins: - # * The InformationController uses the "information" layout, explicitly set. - # * The TellerController also uses the "information" layout, because the parent explicitly set it. - # * The EmployeeController uses the "employee" layout, because it set the layout to nil, resetting the parent configuration. - # * The VaultController chooses a layout dynamically by calling the <tt>access_level_layout</tt> method. - # * The TillController does not use a layout at all. + # * The +InformationController+ uses the "information" layout, explicitly set. + # * The +TellerController+ also uses the "information" layout, because the parent explicitly set it. + # * The +EmployeeController+ uses the "employee" layout, because it set the layout to +nil+, resetting the parent configuration. + # * The +VaultController+ chooses a layout dynamically by calling the <tt>access_level_layout</tt> method. + # * The +TillController+ does not use a layout at all. # # == Types of layouts # @@ -148,8 +148,8 @@ module ActionView # The template will be looked always in <tt>app/views/layouts/</tt> folder. But you can point # <tt>layouts</tt> folder direct also. <tt>layout "layouts/demo"</tt> is the same as <tt>layout "demo"</tt>. # - # Setting the layout to nil forces it to be looked up in the filesystem and fallbacks to the parent behavior if none exists. - # Setting it to nil is useful to re-enable template lookup overriding a previous configuration set in the parent: + # Setting the layout to +nil+ forces it to be looked up in the filesystem and fallbacks to the parent behavior if none exists. + # Setting it to +nil+ is useful to re-enable template lookup overriding a previous configuration set in the parent: # # class ApplicationController < ActionController::Base # layout "application" @@ -254,7 +254,7 @@ module ActionView # true:: raise an ArgumentError # nil:: Force default layout behavior with inheritance # - # Return value of Proc & Symbol arguments should be String, false, true or nil + # Return value of +Proc & Symbol+ arguments should be +String+, +false+, +true+ or +nil+ # with the same meaning as described above. # ==== Parameters # * <tt>layout</tt> - The layout to use. @@ -404,11 +404,11 @@ module ActionView # # ==== Parameters # * <tt>formats</tt> - The formats accepted to this layout - # * <tt>require_layout</tt> - If set to true and layout is not found, - # an +ArgumentError+ exception is raised (defaults to false) + # * <tt>require_layout</tt> - If set to +true+ and layout is not found, + # an +ArgumentError+ exception is raised (defaults to +false+) # # ==== Returns - # * <tt>template</tt> - The template object for the default layout (or nil) + # * <tt>template</tt> - The template object for the default layout (or +nil+) def _default_layout(formats, require_layout = false) begin value = _layout(formats) if action_has_layout? diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index bf338ec910..b8a79da97f 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -98,7 +98,7 @@ module ActionView # # <%= render partial: "ad", collection: @advertisements, spacer_template: "ad_divider" %> # - # If the given <tt>:collection</tt> is nil or empty, <tt>render</tt> will return nil. This will allow you + # If the given <tt>:collection</tt> is +nil+ or empty, <tt>render</tt> will return nil. This will allow you # to specify a text which will displayed instead by using this form: # # <%= render(partial: "ad", collection: @advertisements) || "There's no ad to be displayed" %> diff --git a/actionview/lib/action_view/view_paths.rb b/actionview/lib/action_view/view_paths.rb index b5cde5b43f..a9638d1e6d 100644 --- a/actionview/lib/action_view/view_paths.rb +++ b/actionview/lib/action_view/view_paths.rb @@ -46,10 +46,22 @@ module ActionView {} end + # Append a path to the list of view paths for the current <tt>LookupContext</tt>. + # + # ==== Parameters + # * <tt>path</tt> - If a String is provided, it gets converted into + # the default view path. You may also provide a custom view path + # (see ActionView::PathSet for more information) def append_view_path(path) lookup_context.view_paths.push(*path) end + # Prepend a path to the list of view paths for the current <tt>LookupContext</tt>. + # + # ==== Parameters + # * <tt>path</tt> - If a String is provided, it gets converted into + # the default view path. You may also provide a custom view path + # (see ActionView::PathSet for more information) def prepend_view_path(path) lookup_context.view_paths.unshift(*path) end diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index 8b815155b1..a1f7c971db 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -68,7 +68,7 @@ module ActiveModel # # Options: # * <tt>:on</tt> - Specifies the contexts where this validation is active. - # Runs in all validation contexts by default (nil). You can pass a symbol + # Runs in all validation contexts by default +nil+. You can pass a symbol # or an array of symbols. (e.g. <tt>on: :create</tt> or # <tt>on: :custom_validation_context</tt> or # <tt>on: [:create, :custom_validation_context]</tt>) @@ -134,7 +134,7 @@ module ActiveModel # # Options: # * <tt>:on</tt> - Specifies the contexts where this validation is active. - # Runs in all validation contexts by default (nil). You can pass a symbol + # Runs in all validation contexts by default +nil+. You can pass a symbol # or an array of symbols. (e.g. <tt>on: :create</tt> or # <tt>on: :custom_validation_context</tt> or # <tt>on: [:create, :custom_validation_context]</tt>) diff --git a/activemodel/lib/active_model/validations/validates.rb b/activemodel/lib/active_model/validations/validates.rb index c3cbb6e291..f95f44de61 100644 --- a/activemodel/lib/active_model/validations/validates.rb +++ b/activemodel/lib/active_model/validations/validates.rb @@ -72,7 +72,7 @@ module ActiveModel # There is also a list of options that could be used along with validators: # # * <tt>:on</tt> - Specifies the contexts where this validation is active. - # Runs in all validation contexts by default (nil). You can pass a symbol + # Runs in all validation contexts by default +nil+. You can pass a symbol # or an array of symbols. (e.g. <tt>on: :create</tt> or # <tt>on: :custom_validation_context</tt> or # <tt>on: [:create, :custom_validation_context]</tt>) diff --git a/activemodel/lib/active_model/validations/with.rb b/activemodel/lib/active_model/validations/with.rb index 6ae9630d82..e3f7a9bcb2 100644 --- a/activemodel/lib/active_model/validations/with.rb +++ b/activemodel/lib/active_model/validations/with.rb @@ -45,7 +45,7 @@ module ActiveModel # # Configuration options: # * <tt>:on</tt> - Specifies the contexts where this validation is active. - # Runs in all validation contexts by default (nil). You can pass a symbol + # Runs in all validation contexts by default +nil+. You can pass a symbol # or an array of symbols. (e.g. <tt>on: :create</tt> or # <tt>on: :custom_validation_context</tt> or # <tt>on: [:create, :custom_validation_context]</tt>) diff --git a/activemodel/test/cases/type/big_integer_test.rb b/activemodel/test/cases/type/big_integer_test.rb index db1d215083..56002b7cc6 100644 --- a/activemodel/test/cases/type/big_integer_test.rb +++ b/activemodel/test/cases/type/big_integer_test.rb @@ -12,7 +12,7 @@ module ActiveModel def test_small_values type = Type::BigInteger.new - assert_equal -9999999999999999999999999999999, type.serialize(-9999999999999999999999999999999) + assert_equal(-9999999999999999999999999999999, type.serialize(-9999999999999999999999999999999)) end def test_large_values diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c2f26bce70..8efefe9b2e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,30 @@ +* Added `stat` method to `ActiveRecord::ConnectionAdapters::ConnectionPool` + + Example: + + ActiveRecord::Base.connection_pool.stat # => + { size: 15, connections: 1, busy: 1, dead: 0, idle: 0, waiting: 0, checkout_timeout: 5 } + + *Pavel Evstigneev* + +* Avoid `unscope(:order)` when `limit_value` is presented for `count`. + + If `limit_value` is presented, records fetching order is very important + for performance. We should not unscope the order in the case. + + *Ryuta Kamizono* + +* Fix an Active Record `DateTime` field `NoMethodError` caused by incomplete + datetime. + + Fixes #24195. + + *Sen Zhang* + +* Allow `slice` to take an array of methods(without the need for splatting). + + *Cohen Carlisle* + * Improved partial writes with HABTM and has many through associations to fire database query only if relation has been changed. @@ -10,12 +37,12 @@ *Prathamesh Sonpatki* -* Optimistic locking: Added ability update locking_column value. - Ignore optimistic locking if update with new locking_column value. +* Optimistic locking: Added ability to update `locking_column` value. + Ignore optimistic locking if trying to update with new `locking_column` value. *bogdanvlviv* -* Fixed: Optimistic locking does not work well with null in the database. +* Fixed: Optimistic locking does not work well with `null` in the database. Fixes #26024 diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb index 5ca8fe576e..08dfc3a64f 100644 --- a/activerecord/lib/active_record/aggregations.rb +++ b/activerecord/lib/active_record/aggregations.rb @@ -206,7 +206,7 @@ module ActiveRecord # or a Proc that is called when a new value is assigned to the value object. The converter is # passed the single value that is used in the assignment and is only called if the new value is # not an instance of <tt>:class_name</tt>. If <tt>:allow_nil</tt> is set to true, the converter - # can return nil to skip the assignment. + # can return +nil+ to skip the assignment. # # Option examples: # composed_of :temperature, mapping: %w(reading celsius) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index b5f1f1980a..3c94c4bd7f 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -264,7 +264,7 @@ module ActiveRecord super end - # Returns the specified association instance if it exists, nil otherwise. + # Returns the specified association instance if it exists, +nil+ otherwise. def association_instance_get(name) @association_cache[name] end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index f506614591..84d0493a60 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -112,6 +112,15 @@ module ActiveRecord record end + # Remove the inverse association, if possible + def remove_inverse_instance(record) + if invertible_for?(record) + inverse = record.association(inverse_reflection_for(record).name) + inverse.target = nil + inverse.inversed = false + end + end + # Returns the class of the target. belongs_to polymorphic overrides this to look at the # polymorphic_type field on the owner. def klass @@ -166,7 +175,7 @@ module ActiveRecord def initialize_attributes(record, except_from_scope_attributes = nil) #:nodoc: except_from_scope_attributes ||= {} skip_assign = [reflection.foreign_key, reflection.type].compact - assigned_keys = record.changed + assigned_keys = record.changed_attribute_names_to_save assigned_keys += except_from_scope_attributes.keys.map(&:to_s) attributes = create_scope.except(*(assigned_keys - skip_assign)) record.assign_attributes(attributes) @@ -254,7 +263,7 @@ module ActiveRecord # so that when stale_state is different from the value stored on the last find_target, # the target is stale. # - # This is only relevant to certain associations, which is why it returns nil by default. + # This is only relevant to certain associations, which is why it returns +nil+ by default. def stale_state end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 3121e70a04..a1609ab0fb 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -35,17 +35,17 @@ module ActiveRecord::Associations::Builder # :nodoc: @_after_create_counter_called = false elsif (@_after_replace_counter_called ||= false) @_after_replace_counter_called = false - elsif attribute_changed?(foreign_key) && !new_record? + elsif saved_change_to_attribute?(foreign_key) && !new_record? if reflection.polymorphic? - model = attribute(reflection.foreign_type).try(:constantize) - model_was = attribute_was(reflection.foreign_type).try(:constantize) + model = attribute_in_database(reflection.foreign_type).try(:constantize) + model_was = attribute_before_last_save(reflection.foreign_type).try(:constantize) else model = reflection.klass model_was = reflection.klass end - foreign_key_was = attribute_was foreign_key - foreign_key = attribute foreign_key + foreign_key_was = attribute_before_last_save foreign_key + foreign_key = attribute_in_database foreign_key if foreign_key && model.respond_to?(:increment_counter) model.increment_counter(cache_column, foreign_key) @@ -70,14 +70,16 @@ module ActiveRecord::Associations::Builder # :nodoc: klass.attr_readonly cache_column if klass && klass.respond_to?(:attr_readonly) end - def self.touch_record(o, foreign_key, name, touch, touch_method) # :nodoc: - old_foreign_id = o.changed_attributes[foreign_key] + def self.touch_record(o, changes, foreign_key, name, touch, touch_method) # :nodoc: + old_foreign_id = changes[foreign_key] && changes[foreign_key].first if old_foreign_id association = o.association(name) reflection = association.reflection if reflection.polymorphic? - klass = o.public_send("#{reflection.foreign_type}_was").constantize + foreign_type = reflection.foreign_type + klass = changes[foreign_type] && changes[foreign_type].first || o.public_send(foreign_type) + klass = klass.constantize else klass = association.klass end @@ -107,13 +109,13 @@ module ActiveRecord::Associations::Builder # :nodoc: n = reflection.name touch = reflection.options[:touch] - callback = lambda { |record| - BelongsTo.touch_record(record, foreign_key, n, touch, belongs_to_touch_method) - } + callback = lambda { |changes_method| lambda { |record| + BelongsTo.touch_record(record, record.send(changes_method), foreign_key, n, touch, belongs_to_touch_method) + }} - model.after_save callback, if: :changed? - model.after_touch callback - model.after_destroy callback + model.after_save callback.(:saved_changes), if: :saved_changes? + model.after_touch callback.(:changes_to_save) + model.after_destroy callback.(:changes_to_save) end def self.add_destroy_callbacks(model, reflection) diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 278c95e27b..5a323c62e6 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -192,11 +192,8 @@ module ActiveRecord # +delete_records+. They are in any case removed from the collection. def delete(*records) return if records.empty? - _options = records.extract_options! - dependent = _options[:dependent] || options[:dependent] - records = find(records) if records.any? { |record| record.kind_of?(Integer) || record.kind_of?(String) } - delete_or_destroy(records, dependent) + delete_or_destroy(records, options[:dependent]) end # Deletes the +records+ and removes them from this association calling @@ -222,11 +219,7 @@ module ActiveRecord # +count_records+, which is a method descendants have to provide. def size if !find_target? || loaded? - if association_scope.distinct_value - target.uniq.size - else - target.size - end + target.size elsif !association_scope.group_values.empty? load_target.size elsif !association_scope.distinct_value && target.is_a?(Array) @@ -375,7 +368,7 @@ module ActiveRecord persisted.map! do |record| if mem_record = memory.delete(record) - ((record.attribute_names & mem_record.attribute_names) - mem_record.changes.keys).each do |name| + ((record.attribute_names & mem_record.attribute_names) - mem_record.changed_attribute_names_to_save).each do |name| mem_record[name] = record[name] end @@ -435,8 +428,9 @@ module ActiveRecord records.each { |record| callback(:after_remove, record) } end - # Delete the given records from the association, using one of the methods :destroy, - # :delete_all or :nullify (or nil, in which case a default is used). + # Delete the given records from the association, + # using one of the methods +:destroy+, +:delete_all+ + # or +:nullify+ (or +nil+, in which case a default is used). def delete_records(records, method) raise NotImplementedError end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index d1d0cc4c49..742cd25509 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -72,7 +72,7 @@ module ActiveRecord # the loaded flag is set to true as well. def count_records count = if reflection.has_cached_counter? - owner._read_attribute reflection.counter_cache_column + owner._read_attribute(reflection.counter_cache_column).to_i else scope.count end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 5ea9577301..21bd668dff 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -35,7 +35,7 @@ module ActiveRecord return target unless target || record assigning_another_record = target != record - if assigning_another_record || record.changed? + if assigning_another_record || record.has_changes_to_save? save &&= owner.persisted? transaction_if(save) do @@ -86,8 +86,9 @@ module ActiveRecord target.delete when :destroy target.destroy - else + else nullify_owner_attributes(target) + remove_inverse_instance(target) if target.persisted? && owner.persisted? && !target.save set_owner_attributes(target) diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index c9638bf70b..b22190455a 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true require "active_support/core_ext/module/attribute_accessors" require "active_record/attribute_mutation_tracker" @@ -15,6 +16,18 @@ module ActiveRecord class_attribute :partial_writes, instance_writer: false self.partial_writes = true + + after_create { changes_internally_applied } + after_update { changes_internally_applied } + + # Attribute methods for "changed in last call to save?" + attribute_method_affix(prefix: "saved_change_to_", suffix: "?") + attribute_method_prefix("saved_change_to_") + attribute_method_suffix("_before_last_save") + + # Attribute methods for "will change if I call save?" + attribute_method_affix(prefix: "will_save_change_to_", suffix: "?") + attribute_method_suffix("_change_to_be_saved", "_in_database") end # Attempts to +save+ the record and clears changed attributes if successful. @@ -35,8 +48,8 @@ module ActiveRecord # <tt>reload</tt> the record and clears changed attributes. def reload(*) super.tap do - @mutation_tracker = nil @previous_mutation_tracker = nil + clear_mutation_trackers @changed_attributes = HashWithIndifferentAccess.new end end @@ -46,19 +59,26 @@ module ActiveRecord @attributes = self.class._default_attributes.map do |attr| attr.with_value_from_user(@attributes.fetch_value(attr.name)) end - @mutation_tracker = nil + clear_mutation_trackers + end + + def changes_internally_applied # :nodoc: + @mutations_before_last_save = mutation_tracker + forget_attribute_assignments + @mutations_from_database = AttributeMutationTracker.new(@attributes) end def changes_applied @previous_mutation_tracker = mutation_tracker @changed_attributes = HashWithIndifferentAccess.new - store_original_attributes + clear_mutation_trackers end def clear_changes_information @previous_mutation_tracker = nil @changed_attributes = HashWithIndifferentAccess.new - store_original_attributes + forget_attribute_assignments + clear_mutation_trackers end def raw_write_attribute(attr_name, *) @@ -80,17 +100,27 @@ module ActiveRecord if defined?(@cached_changed_attributes) @cached_changed_attributes else + emit_warning_if_needed("changed_attributes", "attributes_in_database") super.reverse_merge(mutation_tracker.changed_values).freeze end end def changes cache_changed_attributes do + emit_warning_if_needed("changes", "changes_to_save") super end end def previous_changes + unless previous_mutation_tracker.equal?(mutations_before_last_save) + ActiveSupport::Deprecation.warn(<<-EOW.strip_heredoc) + The behavior of `previous_changes` inside of after callbacks is + deprecated without replacement. In the next release of Rails, + this method inside of `after_save` will return the changes that + were just saved. + EOW + end previous_mutation_tracker.changes end @@ -98,6 +128,109 @@ module ActiveRecord mutation_tracker.changed_in_place?(attr_name) end + # Did this attribute change when we last saved? This method can be invoked + # as `saved_change_to_name?` instead of `saved_change_to_attribute?("name")`. + # Behaves similarly to +attribute_changed?+. This method is useful in + # after callbacks to determine if the call to save changed a certain + # attribute. + # + # ==== Options + # + # +from+ When passed, this method will return false unless the original + # value is equal to the given option + # + # +to+ When passed, this method will return false unless the value was + # changed to the given value + def saved_change_to_attribute?(attr_name, **options) + mutations_before_last_save.changed?(attr_name, **options) + end + + # Returns the change to an attribute during the last save. If the + # attribute was changed, the result will be an array containing the + # original value and the saved value. + # + # Behaves similarly to +attribute_change+. This method is useful in after + # callbacks, to see the change in an attribute that just occurred + # + # This method can be invoked as `saved_change_to_name` in instead of + # `saved_change_to_attribute("name")` + def saved_change_to_attribute(attr_name) + mutations_before_last_save.change_to_attribute(attr_name) + end + + # Returns the original value of an attribute before the last save. + # Behaves similarly to +attribute_was+. This method is useful in after + # callbacks to get the original value of an attribute before the save that + # just occurred + def attribute_before_last_save(attr_name) + mutations_before_last_save.original_value(attr_name) + end + + # Did the last call to `save` have any changes to change? + def saved_changes? + mutations_before_last_save.any_changes? + end + + # Returns a hash containing all the changes that were just saved. + def saved_changes + mutations_before_last_save.changes + end + + # Alias for `attribute_changed?` + def will_save_change_to_attribute?(attr_name, **options) + mutations_from_database.changed?(attr_name, **options) + end + + # Alias for `attribute_change` + def attribute_change_to_be_saved(attr_name) + mutations_from_database.change_to_attribute(attr_name) + end + + # Alias for `attribute_was` + def attribute_in_database(attr_name) + mutations_from_database.original_value(attr_name) + end + + # Alias for `changed?` + def has_changes_to_save? + mutations_from_database.any_changes? + end + + # Alias for `changes` + def changes_to_save + mutations_from_database.changes + end + + # Alias for `changed` + def changed_attribute_names_to_save + changes_to_save.keys + end + + # Alias for `changed_attributes` + def attributes_in_database + changes_to_save.transform_values(&:first) + end + + def attribute_was(*) + emit_warning_if_needed("attribute_was", "attribute_in_database") + super + end + + def attribute_change(*) + emit_warning_if_needed("attribute_change", "attribute_change_to_be_saved") + super + end + + def attribute_changed?(*) + emit_warning_if_needed("attribute_changed?", "will_save_change_to_attribute?") + super + end + + def changed(*) + emit_warning_if_needed("changed", "changed_attribute_names_to_save") + super + end + private def mutation_tracker @@ -107,12 +240,37 @@ module ActiveRecord @mutation_tracker ||= AttributeMutationTracker.new(@attributes) end + def emit_warning_if_needed(method_name, new_method_name) + unless mutation_tracker.equal?(mutations_from_database) + ActiveSupport::Deprecation.warn(<<-EOW.squish) + The behavior of `#{method_name}` inside of after callbacks will + be changing in the next version of Rails. The new return value will reflect the + behavior of calling the method after `save` returned (e.g. the opposite of what + it returns now). To maintain the current behavior, use `#{new_method_name}` + instead. + EOW + end + end + + def mutations_from_database + unless defined?(@mutations_from_database) + @mutations_from_database = nil + end + @mutations_from_database ||= mutation_tracker + end + def changes_include?(attr_name) super || mutation_tracker.changed?(attr_name) end def clear_attribute_change(attr_name) mutation_tracker.forget_change(attr_name) + mutations_from_database.forget_change(attr_name) + end + + def attribute_will_change!(attr_name) + super + mutations_from_database.force_change(attr_name) end def _update_record(*) @@ -124,18 +282,27 @@ module ActiveRecord end def keys_for_partial_write - changed & self.class.column_names + changed_attribute_names_to_save & self.class.column_names end - def store_original_attributes + def forget_attribute_assignments @attributes = @attributes.map(&:forgetting_assignment) + end + + def clear_mutation_trackers @mutation_tracker = nil + @mutations_from_database = nil + @mutations_before_last_save = nil end def previous_mutation_tracker @previous_mutation_tracker ||= NullMutationTracker.instance end + def mutations_before_last_save + @mutations_before_last_save ||= previous_mutation_tracker + end + def cache_changed_attributes @cached_changed_attributes = changed_attributes yield diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index 6243398a52..287367f92a 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -45,6 +45,11 @@ module ActiveRecord attribute_was(self.class.primary_key) end + def id_in_database + sync_with_transaction_state + attribute_in_database(self.class.primary_key) + end + protected def attribute_method?(attr_name) @@ -60,7 +65,7 @@ module ActiveRecord end end - ID_ATTRIBUTE_METHODS = %w(id id= id? id_before_type_cast id_was).to_set + ID_ATTRIBUTE_METHODS = %w(id id= id? id_before_type_cast id_was id_in_database).to_set def dangerous_attribute_method?(method_name) super && !ID_ATTRIBUTE_METHODS.include?(method_name) diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index ce9985e2e1..500d903857 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -39,7 +39,7 @@ module ActiveRecord end def set_time_zone_without_conversion(value) - ::Time.zone.local_to_utc(value).in_time_zone if value + ::Time.zone.local_to_utc(value).try(:in_time_zone) if value end def map_avoiding_infinite_recursion(value) diff --git a/activerecord/lib/active_record/attribute_mutation_tracker.rb b/activerecord/lib/active_record/attribute_mutation_tracker.rb index c257aef52f..db86b2b294 100644 --- a/activerecord/lib/active_record/attribute_mutation_tracker.rb +++ b/activerecord/lib/active_record/attribute_mutation_tracker.rb @@ -1,7 +1,10 @@ module ActiveRecord class AttributeMutationTracker # :nodoc: + OPTION_NOT_GIVEN = Object.new + def initialize(attributes) @attributes = attributes + @forced_changes = Set.new end def changed_values @@ -14,15 +17,29 @@ module ActiveRecord def changes attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result| - if changed?(attr_name) - result[attr_name] = [attributes[attr_name].original_value, attributes.fetch_value(attr_name)] + change = change_to_attribute(attr_name) + if change + result[attr_name] = change end end end - def changed?(attr_name) + def change_to_attribute(attr_name) + if changed?(attr_name) + [attributes[attr_name].original_value, attributes.fetch_value(attr_name)] + end + end + + def any_changes? + attr_names.any? { |attr| changed?(attr) } + end + + def changed?(attr_name, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN) attr_name = attr_name.to_s - attributes[attr_name].changed? + forced_changes.include?(attr_name) || + attributes[attr_name].changed? && + (OPTION_NOT_GIVEN == from || attributes[attr_name].original_value == from) && + (OPTION_NOT_GIVEN == to || attributes[attr_name].value == to) end def changed_in_place?(attr_name) @@ -32,11 +49,20 @@ module ActiveRecord def forget_change(attr_name) attr_name = attr_name.to_s attributes[attr_name] = attributes[attr_name].forgetting_assignment + forced_changes.delete(attr_name) + end + + def original_value(attr_name) + attributes[attr_name].original_value + end + + def force_change(attr_name) + forced_changes << attr_name.to_s end protected - attr_reader :attributes + attr_reader :attributes, :forced_changes private @@ -48,14 +74,21 @@ module ActiveRecord class NullMutationTracker # :nodoc: include Singleton - def changed_values + def changed_values(*) {} end - def changes + def changes(*) {} end + def change_to_attribute(attr_name) + end + + def any_changes?(*) + false + end + def changed?(*) false end @@ -66,5 +99,8 @@ module ActiveRecord def forget_change(*) end + + def original_value(*) + end end end diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index d3901f6efe..b343332bae 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -267,7 +267,7 @@ module ActiveRecord # Returns whether or not this record has been changed in any way (including whether # any of its nested autosave associations are likewise changed) def changed_for_autosave? - new_record? || changed? || marked_for_destruction? || nested_records_changed_for_autosave? + new_record? || has_changes_to_save? || marked_for_destruction? || nested_records_changed_for_autosave? end private @@ -451,7 +451,7 @@ module ActiveRecord def record_changed?(reflection, record, key) record.new_record? || (record.has_attribute?(reflection.foreign_key) && record[reflection.foreign_key] != key) || - record.attribute_changed?(reflection.foreign_key) + record.will_save_change_to_attribute?(reflection.foreign_key) end # Saves the associated record if it's new or <tt>:autosave</tt> is enabled. diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 1e7e939097..ac1aa2df45 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -14,6 +14,7 @@ require "active_support/core_ext/module/introspection" require "active_support/core_ext/object/duplicable" require "active_support/core_ext/class/subclasses" require "active_record/attribute_decorators" +require "active_record/define_callbacks" require "active_record/errors" require "active_record/log_subscriber" require "active_record/explain_subscriber" @@ -303,6 +304,7 @@ module ActiveRecord #:nodoc: include AttributeDecorators include Locking::Optimistic include Locking::Pessimistic + include DefineCallbacks include AttributeMethods include Callbacks include Timestamp diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index c616733aa4..f2e3912c6e 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -265,17 +265,6 @@ module ActiveRecord :before_destroy, :around_destroy, :after_destroy, :after_commit, :after_rollback ] - module ClassMethods # :nodoc: - include ActiveModel::Callbacks - end - - included do - include ActiveModel::Validations::Callbacks - - define_model_callbacks :initialize, :find, :touch, only: :after - define_model_callbacks :save, :create, :update, :destroy - end - def destroy #:nodoc: @_destroy_callback_already_called ||= false return if @_destroy_callback_already_called diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index e9ecb78e27..e6b6b60c1b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -69,7 +69,7 @@ module ActiveRecord # threads, which can occur if a programmer forgets to close a # connection at the end of a thread or a thread dies unexpectedly. # Regardless of this setting, the Reaper will be invoked before every - # blocking wait. (Default nil, which means don't schedule the Reaper). + # blocking wait. (Default +nil+, which means don't schedule the Reaper). # #-- # Synchronization policy: @@ -116,7 +116,7 @@ module ActiveRecord end end - # If +element+ is in the queue, remove and return it, or nil. + # If +element+ is in the queue, remove and return it, or +nil+. def delete(element) synchronize do @queue.delete(element) @@ -135,7 +135,7 @@ module ActiveRecord # If +timeout+ is not given, remove and return the head the # queue if the number of available elements is strictly # greater than the number of threads currently waiting (that - # is, don't jump ahead in line). Otherwise, return nil. + # is, don't jump ahead in line). Otherwise, return +nil+. # # If +timeout+ is given, block if there is no element # available, waiting up to +timeout+ seconds for an element to @@ -171,14 +171,14 @@ module ActiveRecord @queue.size > @num_waiting end - # Removes and returns the head of the queue if possible, or nil. + # Removes and returns the head of the queue if possible, or +nil+. def remove @queue.shift end # Remove and return the head the queue if the number of # available elements is strictly greater than the number of - # threads currently waiting. Otherwise, return nil. + # threads currently waiting. Otherwise, return +nil+. def no_wait_poll remove if can_remove_no_wait? end @@ -282,7 +282,7 @@ module ActiveRecord end # Every +frequency+ seconds, the reaper will call +reap+ on +pool+. - # A reaper instantiated with a nil frequency will never reap the + # A reaper instantiated with a +nil+ frequency will never reap the # connection pool. # # Configure the frequency by setting "reaping_frequency" in your @@ -307,6 +307,7 @@ module ActiveRecord end include MonitorMixin + include QueryCache::ConnectionPoolConfiguration attr_accessor :automatic_reconnect, :checkout_timeout, :schema_cache attr_reader :spec, :connections, :size, :reaper @@ -581,6 +582,24 @@ module ActiveRecord @available.num_waiting end + # Return connection pool's usage statistic + # Example: + # + # ActiveRecord::Base.connection_pool.stat # => { size: 15, connections: 1, busy: 1, dead: 0, idle: 0, waiting: 0, checkout_timeout: 5 } + def stat + synchronize do + { + size: size, + connections: @connections.size, + busy: @connections.count { |c| c.in_use? && c.owner.alive? }, + dead: @connections.count { |c| c.in_use? && !c.owner.alive? }, + idle: @connections.count { |c| !c.in_use? }, + waiting: num_waiting_in_queue, + checkout_timeout: checkout_timeout + } + end + end + private #-- # this is unfortunately not concurrent diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb index 95c72f1e20..407e019326 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb @@ -46,7 +46,7 @@ module ActiveRecord end # Returns the maximum number of elements in an IN (x,y,z) clause. - # nil means no limit. + # +nil+ means no limit. def in_clause_length nil end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index aa2dfdd573..faccd1d641 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -115,7 +115,7 @@ module ActiveRecord # Executes an INSERT query and returns the new record's ID # - # +id_value+ will be returned unless the value is nil, in + # +id_value+ will be returned unless the value is +nil+, in # which case the database will attempt to calculate the last inserted # id and return that value. # diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 376fbca74f..7eab7de5d3 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -4,6 +4,9 @@ module ActiveRecord class << self def included(base) #:nodoc: dirties_query_cache base, :insert, :update, :delete, :rollback_to_savepoint, :rollback_db_transaction + + base.set_callback :checkout, :after, :configure_query_cache! + base.set_callback :checkin, :after, :disable_query_cache! end def dirties_query_cache(base, *method_names) @@ -18,6 +21,27 @@ module ActiveRecord end end + module ConnectionPoolConfiguration + def initialize(*) + super + @query_cache_enabled = Concurrent::Map.new { false } + end + + def enable_query_cache! + @query_cache_enabled[connection_cache_key(Thread.current)] = true + connection.enable_query_cache! if active_connection? + end + + def disable_query_cache! + @query_cache_enabled.delete connection_cache_key(Thread.current) + connection.disable_query_cache! if active_connection? + end + + def query_cache_enabled + @query_cache_enabled[connection_cache_key(Thread.current)] + end + end + attr_reader :query_cache, :query_cache_enabled def initialize(*) @@ -41,6 +65,7 @@ module ActiveRecord def disable_query_cache! @query_cache_enabled = false + clear_query_cache end # Disable the query cache within the block. @@ -96,6 +121,10 @@ module ActiveRecord def locked?(arel) arel.respond_to?(:locked) && arel.locked end + + def configure_query_cache! + enable_query_cache! if pool.query_cache_enabled + 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 135421819c..237367c8b3 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -62,17 +62,17 @@ module ActiveRecord # notably, the instance methods provided by SchemaStatements are very useful. class AbstractAdapter ADAPTER_NAME = "Abstract".freeze + include ActiveSupport::Callbacks + define_callbacks :checkout, :checkin + include Quoting, DatabaseStatements, SchemaStatements include DatabaseLimits include QueryCache - include ActiveSupport::Callbacks include ColumnDumper include Savepoints SIMPLE_INT = /\A\d+\z/ - define_callbacks :checkout, :checkin - attr_accessor :visitor, :pool attr_reader :schema_cache, :owner, :logger alias :in_use? :owner @@ -161,6 +161,14 @@ module ActiveRecord SchemaCreation.new self end + # Returns an array of +Column+ objects for the table specified by +table_name+. + def columns(table_name) # :nodoc: + table_name = table_name.to_s + column_definitions(table_name).map do |field| + new_column_from_field(table_name, field) + end + end + # this method must only be called while holding connection pool's mutex def lease if in_use? 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 6f334f5c8d..cbbba5b1a5 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -398,18 +398,14 @@ module ActiveRecord indexes end - # Returns an array of +Column+ objects for the table specified by +table_name+. - def columns(table_name) # :nodoc: - table_name = table_name.to_s - column_definitions(table_name).map do |field| - type_metadata = fetch_type_metadata(field[:Type], field[:Extra]) - if type_metadata.type == :datetime && field[:Default] == "CURRENT_TIMESTAMP" - default, default_function = nil, field[:Default] - else - default, default_function = field[:Default], nil - end - new_column(field[:Field], default, type_metadata, field[:Null] == "YES", table_name, default_function, field[:Collation], comment: field[:Comment].presence) + def new_column_from_field(table_name, field) # :nodoc: + type_metadata = fetch_type_metadata(field[:Type], field[:Extra]) + if type_metadata.type == :datetime && field[:Default] == "CURRENT_TIMESTAMP" + default, default_function = nil, field[:Default] + else + default, default_function = field[:Default], nil end + new_column(field[:Field], default, type_metadata, field[:Null] == "YES", table_name, default_function, field[:Collation], comment: field[:Comment].presence) end def table_comment(table_name) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index a3e2c913c5..45e400b75b 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -14,12 +14,10 @@ module ActiveRecord config[:username] = "root" if config[:username].nil? config[:flags] ||= 0 - if Mysql2::Client.const_defined? :FOUND_ROWS - if config[:flags].kind_of? Array - config[:flags].push "FOUND_ROWS".freeze - else - config[:flags] |= Mysql2::Client::FOUND_ROWS - end + if config[:flags].kind_of? Array + config[:flags].push "FOUND_ROWS".freeze + else + config[:flags] |= Mysql2::Client::FOUND_ROWS end client = Mysql2::Client.new(config) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 69f797da3a..9e7487b27f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -221,21 +221,23 @@ module ActiveRecord end.compact end - # Returns the list of all column definitions for a table. - def columns(table_name) # :nodoc: - table_name = table_name.to_s - column_definitions(table_name).map do |column_name, type, default, notnull, oid, fmod, collation, comment| - oid = oid.to_i - fmod = fmod.to_i - type_metadata = fetch_type_metadata(column_name, type, oid, fmod) - default_value = extract_value_from_default(default) - default_function = extract_default_function(default_value, default) - new_column(column_name, default_value, type_metadata, !notnull, table_name, default_function, collation, comment: comment.presence) - end - end - - def new_column(*args) # :nodoc: - PostgreSQLColumn.new(*args) + def new_column_from_field(table_name, field) # :nondoc: + column_name, type, default, notnull, oid, fmod, collation, comment = field + oid = oid.to_i + fmod = fmod.to_i + type_metadata = fetch_type_metadata(column_name, type, oid, fmod) + default_value = extract_value_from_default(default) + default_function = extract_default_function(default_value, default) + PostgreSQLColumn.new( + column_name, + default_value, + type_metadata, + !notnull, + table_name, + default_function, + collation, + comment: comment.presence + ) end def table_options(table_name) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/utils.rb b/activerecord/lib/active_record/connection_adapters/postgresql/utils.rb index 9a0b80d7d3..1412928ca5 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/utils.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/utils.rb @@ -53,7 +53,7 @@ module ActiveRecord # Returns an instance of <tt>ActiveRecord::ConnectionAdapters::PostgreSQL::Name</tt> # extracted from +string+. - # +schema+ is nil if not specified in +string+. + # +schema+ is +nil+ if not specified in +string+. # +schema+ and +identifier+ exclude surrounding quotes (regardless of whether provided in +string+) # +string+ supports the range of schema/table references understood by PostgreSQL, for example: # diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 2c50321048..0493ab4e4b 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -304,24 +304,20 @@ module ActiveRecord select_values(sql, "SCHEMA").any? end - # Returns an array of +Column+ objects for the table specified by +table_name+. - def columns(table_name) # :nodoc: - table_name = table_name.to_s - table_structure(table_name).map do |field| - case field["dflt_value"] - when /^null$/i - field["dflt_value"] = nil - when /^'(.*)'$/m - field["dflt_value"] = $1.gsub("''", "'") - when /^"(.*)"$/m - field["dflt_value"] = $1.gsub('""', '"') - end - - collation = field["collation"] - sql_type = field["type"] - type_metadata = fetch_type_metadata(sql_type) - new_column(field["name"], field["dflt_value"], type_metadata, field["notnull"].to_i == 0, table_name, nil, collation) + def new_column_from_field(table_name, field) # :nondoc: + case field["dflt_value"] + when /^null$/i + field["dflt_value"] = nil + when /^'(.*)'$/m + field["dflt_value"] = $1.gsub("''", "'") + when /^"(.*)"$/m + field["dflt_value"] = $1.gsub('""', '"') end + + collation = field["collation"] + sql_type = field["type"] + type_metadata = fetch_type_metadata(sql_type) + new_column(field["name"], field["dflt_value"], type_metadata, field["notnull"].to_i == 0, table_name, nil, collation) end # Returns an array of indexes for the given table. @@ -430,11 +426,12 @@ module ActiveRecord protected - def table_structure(table_name) + def table_structure(table_name) # :nodoc: structure = exec_query("PRAGMA table_info(#{quote_table_name(table_name)})", "SCHEMA") raise(ActiveRecord::StatementInvalid, "Could not find table '#{table_name}'") if structure.empty? table_structure_with_collation(table_name, structure) end + alias column_definitions table_structure def alter_table(table_name, options = {}) #:nodoc: altered_table_name = "a#{table_name}" diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index d24bd4efa8..1fbe374ade 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -330,8 +330,8 @@ module ActiveRecord # # Instantiates a single new object # User.new(first_name: 'Jamie') def initialize(attributes = nil) - @attributes = self.class._default_attributes.deep_dup self.class.define_attribute_methods + @attributes = self.class._default_attributes.deep_dup init_internals initialize_internals_callback @@ -538,7 +538,7 @@ module ActiveRecord # Returns a hash of the given methods with their names as keys and returned values as values. def slice(*methods) - Hash[methods.map! { |method| [method, public_send(method)] }].with_indifferent_access + Hash[methods.flatten.map! { |method| [method, public_send(method)] }].with_indifferent_access end private diff --git a/activerecord/lib/active_record/define_callbacks.rb b/activerecord/lib/active_record/define_callbacks.rb new file mode 100644 index 0000000000..47d3762245 --- /dev/null +++ b/activerecord/lib/active_record/define_callbacks.rb @@ -0,0 +1,20 @@ +module ActiveRecord + # This module exists because `ActiveRecord::AttributeMethods::Dirty` needs to + # define callbacks, but continue to have its version of `save` be the super + # method of `ActiveRecord::Callbacks`. This will be removed when the removal + # of deprecated code removes this need. + module DefineCallbacks + extend ActiveSupport::Concern + + module ClassMethods # :nodoc: + include ActiveModel::Callbacks + end + + included do + include ActiveModel::Validations::Callbacks + + define_model_callbacks :initialize, :find, :touch, :only => :after + define_model_callbacks :save, :create, :update, :destroy + end + end +end diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 8fbe43e3ec..6464d40c94 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -43,7 +43,7 @@ module ActiveRecord # Raised when connection to the database could not been established (for example when # {ActiveRecord::Base.connection=}[rdoc-ref:ConnectionHandling#connection] - # is given a nil object). + # is given a +nil+ object). class ConnectionNotEstablished < ActiveRecordError end diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 0eaee05056..21c5e5b5bb 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -88,7 +88,7 @@ module ActiveRecord # assert_equal "Ruby on Rails", @rubyonrails.name # end # - # In order to use these methods to access fixtured data within your testcases, you must specify one of the + # In order to use these methods to access fixtured data within your test cases, you must specify one of the # following in your ActiveSupport::TestCase-derived class: # # - to fully enable instantiated fixtures (enable alternate methods #1 and #2 above) @@ -103,7 +103,7 @@ module ActiveRecord # # = Dynamic fixtures with ERB # - # Some times you don't care about the content of the fixtures as much as you care about the volume. + # Sometimes you don't care about the content of the fixtures as much as you care about the volume. # In these cases, you can mix ERB in with your YAML fixtures to create a bunch of fixtures for load # testing, like: # diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb index 3c54c6048d..8e71b60b29 100644 --- a/activerecord/lib/active_record/integration.rb +++ b/activerecord/lib/active_record/integration.rb @@ -15,9 +15,9 @@ module ActiveRecord self.cache_timestamp_format = :usec end - # Returns a String, which Action Pack uses for constructing a URL to this - # object. The default implementation returns this record's id as a String, - # or nil if this record's unsaved. + # Returns a +String+, which Action Pack uses for constructing a URL to this + # object. The default implementation returns this record's id as a +String+, + # or +nil+ if this record's unsaved. # # For example, suppose that you have a User model, and that you have a # <tt>resources :users</tt> route. Normally, +user_path+ will diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 525f7444a5..2a28c6bf6d 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -292,7 +292,7 @@ module ActiveRecord end # Sets the name of the sequence to use when generating ids to the given - # value, or (if the value is nil or false) to the value returned by the + # value, or (if the value is +nil+ or +false+) to the value returned by the # given block. This is required for Oracle and is useful for any # database which relies on sequences for primary key generation. # diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 6933f3f9b8..8e13ee3564 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -253,7 +253,11 @@ module ActiveRecord verify_readonly_attribute(name) public_send("#{name}=", value) - changed? ? save(validate: false) : true + if has_changes_to_save? + save(validate: false) + else + true + end end # Updates the attributes of the model from the passed-in hash and saves the @@ -336,7 +340,7 @@ module ActiveRecord # record could be saved. def increment!(attribute, by = 1) increment(attribute, by) - change = public_send(attribute) - (attribute_was(attribute.to_s) || 0) + change = public_send(attribute) - (attribute_in_database(attribute.to_s) || 0) self.class.update_counters(id, attribute => change) clear_attribute_change(attribute) # eww self @@ -548,7 +552,7 @@ module ActiveRecord if attributes_values.empty? 0 else - self.class.unscoped._update_record attributes_values, id, id_was + self.class.unscoped._update_record attributes_values, id, id_in_database end end diff --git a/activerecord/lib/active_record/query_cache.rb b/activerecord/lib/active_record/query_cache.rb index c42c22ab09..ec246e97bc 100644 --- a/activerecord/lib/active_record/query_cache.rb +++ b/activerecord/lib/active_record/query_cache.rb @@ -24,19 +24,19 @@ module ActiveRecord end def self.run - connection = ActiveRecord::Base.connection - enabled = connection.query_cache_enabled - connection.enable_query_cache! + caching_pool = ActiveRecord::Base.connection_pool + caching_was_enabled = caching_pool.query_cache_enabled - [connection, enabled] + caching_pool.enable_query_cache! + + [caching_pool, caching_was_enabled] end - def self.complete((connection, enabled)) - connection.clear_query_cache - connection.disable_query_cache! unless enabled + def self.complete((caching_pool, caching_was_enabled)) + caching_pool.disable_query_cache! unless caching_was_enabled - unless ActiveRecord::Base.connected? && ActiveRecord::Base.connection.transaction_open? - ActiveRecord::Base.clear_active_connections! + ActiveRecord::Base.connection_handler.connection_pool_list.each do |pool| + pool.release_connection if pool.active_connection? && !pool.connection.transaction_open? end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index e912f97d63..ef3c3bfae8 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -282,11 +282,6 @@ module ActiveRecord end def autosave=(autosave) - # autosave and inverse_of do not get along together nowadays. They may - # for example cause double saves. Thus, we disable this flag. If in the - # future those two flags are known to work well together, this could be - # removed. - @automatic_inverse_of = false @options[:autosave] = autosave parent_reflection = self.parent_reflection if parent_reflection @@ -541,14 +536,10 @@ module ActiveRecord # Attempts to find the inverse association name automatically. # If it cannot find a suitable inverse association name, it returns - # nil. + # +nil+. def inverse_name options.fetch(:inverse_of) do - if @automatic_inverse_of == false - nil - else - @automatic_inverse_of ||= automatic_inverse_of - end + @automatic_inverse_of ||= automatic_inverse_of end end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index e4676f79a5..827688a663 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -223,17 +223,17 @@ module ActiveRecord end def execute_simple_calculation(operation, column_name, distinct) #:nodoc: - # PostgreSQL doesn't like ORDER BY when there are no GROUP BY - relation = unscope(:order) - column_alias = column_name - if operation == "count" && (relation.limit_value || relation.offset_value) + if operation == "count" && (limit_value || offset_value) # Shortcut when limit is zero. - return 0 if relation.limit_value == 0 + return 0 if limit_value == 0 - query_builder = build_count_subquery(relation, column_name, distinct) + query_builder = build_count_subquery(spawn, column_name, distinct) else + # PostgreSQL doesn't like ORDER BY when there are no GROUP BY + relation = unscope(:order) + column = aggregate_column(column_name) select_value = operation_over_aggregate_column(column, operation, distinct) diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index 094c0e9c6f..6af84c1266 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -42,7 +42,7 @@ module ActiveRecord # Adds a class method for retrieving and querying objects. # The method is intended to return an ActiveRecord::Relation # object, which is composable with other scopes. - # If it returns nil or false, an + # If it returns +nil+ or +false+, an # {all}[rdoc-ref:Scoping::Named::ClassMethods#all] scope is returned instead. # # A \scope represents a narrowing of a database query, such as diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb index 6641ab5df1..63100e38a1 100644 --- a/activerecord/lib/active_record/timestamp.rb +++ b/activerecord/lib/active_record/timestamp.rb @@ -74,7 +74,7 @@ module ActiveRecord timestamp_attributes_for_update_in_model.each do |column| column = column.to_s - next if attribute_changed?(column) + next if will_save_change_to_attribute?(column) write_attribute(column, current_time) end end @@ -82,7 +82,7 @@ module ActiveRecord end def should_record_timestamps? - record_timestamps && (!partial_writes? || changed?) + record_timestamps && (!partial_writes? || has_changes_to_save?) end def timestamp_attributes_for_create_in_model diff --git a/activerecord/lib/active_record/touch_later.rb b/activerecord/lib/active_record/touch_later.rb index c337a7532f..cacde9c881 100644 --- a/activerecord/lib/active_record/touch_later.rb +++ b/activerecord/lib/active_record/touch_later.rb @@ -25,7 +25,7 @@ module ActiveRecord # touch the parents as we are not calling the after_save callbacks self.class.reflect_on_all_associations(:belongs_to).each do |r| if touch = r.options[:touch] - ActiveRecord::Associations::Builder::BelongsTo.touch_record(self, r.foreign_key, r.name, touch, :touch_later) + ActiveRecord::Associations::Builder::BelongsTo.touch_record(self, changes_to_save, r.foreign_key, r.name, touch, :touch_later) end end end diff --git a/activerecord/lib/active_record/validations/associated.rb b/activerecord/lib/active_record/validations/associated.rb index b14db85167..c695965d7b 100644 --- a/activerecord/lib/active_record/validations/associated.rb +++ b/activerecord/lib/active_record/validations/associated.rb @@ -37,7 +37,7 @@ module ActiveRecord # # * <tt>:message</tt> - A custom error message (default is: "is invalid"). # * <tt>:on</tt> - Specifies the contexts where this validation is active. - # Runs in all validation contexts by default (nil). You can pass a symbol + # Runs in all validation contexts by default +nil+. You can pass a symbol # or an array of symbols. (e.g. <tt>on: :create</tt> or # <tt>on: :custom_validation_context</tt> or # <tt>on: [:create, :custom_validation_context]</tt>) diff --git a/activerecord/lib/active_record/validations/presence.rb b/activerecord/lib/active_record/validations/presence.rb index ad82ea66c4..ca5eda2f84 100644 --- a/activerecord/lib/active_record/validations/presence.rb +++ b/activerecord/lib/active_record/validations/presence.rb @@ -44,7 +44,7 @@ module ActiveRecord # Configuration options: # * <tt>:message</tt> - A custom error message (default is: "can't be blank"). # * <tt>:on</tt> - Specifies the contexts where this validation is active. - # Runs in all validation contexts by default (nil). You can pass a symbol + # Runs in all validation contexts by default +nil+. You can pass a symbol # or an array of symbols. (e.g. <tt>on: :create</tt> or # <tt>on: :custom_validation_context</tt> or # <tt>on: [:create, :custom_validation_context]</tt>) diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 8c4930a81d..bed93bfc26 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -17,7 +17,7 @@ module ActiveRecord relation = build_relation(finder_class, attribute, value) if record.persisted? if finder_class.primary_key - relation = relation.where.not(finder_class.primary_key => record.id_was || record.id) + relation = relation.where.not(finder_class.primary_key => record.id_in_database || record.id) else raise UnknownPrimaryKey.new(finder_class, "Can not validate uniqueness for persisted record without primary key.") end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 7eb04b1ffd..3afd062950 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1742,6 +1742,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert !company.clients.loaded? end + def test_counter_cache_on_unloaded_association + car = Car.create(name: "My AppliCar") + assert_equal car.engines.size, 0 + end + def test_get_ids_ignores_include_option assert_equal [readers(:michael_welcome).id], posts(:welcome).readers_with_person_ids end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 483d1162bc..862f33a1a0 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -601,7 +601,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase new_ship = Ship.create(name: "new name") assert_queries(2) do - # One query for updating name and second query for updating pirate_id + # One query to nullify the old ship, one query to update the new ship pirate.ship = new_ship end diff --git a/activerecord/test/cases/attribute_methods/read_test.rb b/activerecord/test/cases/attribute_methods/read_test.rb index a8592bd179..978dd93fa4 100644 --- a/activerecord/test/cases/attribute_methods/read_test.rb +++ b/activerecord/test/cases/attribute_methods/read_test.rb @@ -14,6 +14,7 @@ module ActiveRecord def self.decorate_matching_attribute_types(*); end def self.initialize_generated_modules; end + include ActiveRecord::DefineCallbacks include ActiveRecord::AttributeMethods def self.attribute_names diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 80e76caaaa..a3f82ed49d 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -792,6 +792,7 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase end @ship.pirate.catchphrase = "Changed Catchphrase" + @ship.name_will_change! assert_raise(RuntimeError) { assert !@pirate.save } assert_not_nil @pirate.reload.ship diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 4dc6059d96..4e9d78de5d 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1428,6 +1428,16 @@ class BasicsTest < ActiveRecord::TestCase assert_nil hash["firm_name"] end + def test_slice_accepts_array_argument + attrs = { + title: "slice", + author_name: "@Cohen-Carlisle", + content: "accept arrays so I don't have to splat" + }.with_indifferent_access + topic = Topic.new(attrs) + assert_equal attrs, topic.slice(attrs.keys) + end + def test_default_values_are_deeply_dupped company = Company.new company.description << "foo" diff --git a/activerecord/test/cases/connection_adapters/quoting_test.rb b/activerecord/test/cases/connection_adapters/quoting_test.rb deleted file mode 100644 index 59dcb96ebc..0000000000 --- a/activerecord/test/cases/connection_adapters/quoting_test.rb +++ /dev/null @@ -1,13 +0,0 @@ -require "cases/helper" - -module ActiveRecord - module ConnectionAdapters - module Quoting - class QuotingTest < ActiveRecord::TestCase - def test_quoting_classes - assert_equal "'Object'", AbstractAdapter.new(nil).quote(Object) - end - end - end - end -end diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index d7ff9d6880..b08e4f603c 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -184,14 +184,14 @@ module ActiveRecord def test_checkout_behaviour pool = ConnectionPool.new ActiveRecord::Base.connection_pool.spec - connection = pool.connection - assert_not_nil connection + main_connection = pool.connection + assert_not_nil main_connection threads = [] 4.times do |i| threads << Thread.new(i) do - connection = pool.connection - assert_not_nil connection - connection.close + thread_connection = pool.connection + assert_not_nil thread_connection + thread_connection.close end end @@ -526,6 +526,26 @@ module ActiveRecord end end + def test_connection_pool_stat + with_single_connection_pool do |pool| + pool.with_connection do |connection| + stats = pool.stat + assert_equal({ size: 1, connections: 1, busy: 1, dead: 0, idle: 0, waiting: 0, checkout_timeout: 5 }, stats) + end + + stats = pool.stat + assert_equal({ size: 1, connections: 1, busy: 0, dead: 0, idle: 1, waiting: 0, checkout_timeout: 5 }, stats) + + Thread.new do + pool.checkout + Thread.current.kill + end.join + + stats = pool.stat + assert_equal({ size: 1, connections: 1, busy: 0, dead: 1, idle: 0, waiting: 0, checkout_timeout: 5 }, stats) + end + end + private def with_single_connection_pool one_conn_spec = ActiveRecord::Base.connection_pool.spec.dup diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 3bd8475bb0..0e58e65a07 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -726,6 +726,89 @@ class DirtyTest < ActiveRecord::TestCase assert person.changed? end + test "saved_change_to_attribute? returns whether a change occurred in the last save" do + person = Person.create!(first_name: "Sean") + + assert person.saved_change_to_first_name? + refute person.saved_change_to_gender? + assert person.saved_change_to_first_name?(from: nil, to: "Sean") + assert person.saved_change_to_first_name?(from: nil) + assert person.saved_change_to_first_name?(to: "Sean") + refute person.saved_change_to_first_name?(from: "Jim", to: "Sean") + refute person.saved_change_to_first_name?(from: "Jim") + refute person.saved_change_to_first_name?(to: "Jim") + end + + test "saved_change_to_attribute returns the change that occurred in the last save" do + person = Person.create!(first_name: "Sean", gender: "M") + + assert_equal [nil, "Sean"], person.saved_change_to_first_name + assert_equal [nil, "M"], person.saved_change_to_gender + + person.update(first_name: "Jim") + + assert_equal ["Sean", "Jim"], person.saved_change_to_first_name + assert_nil person.saved_change_to_gender + end + + test "attribute_before_last_save returns the original value before saving" do + person = Person.create!(first_name: "Sean", gender: "M") + + assert_nil person.first_name_before_last_save + assert_nil person.gender_before_last_save + + person.first_name = "Jim" + + assert_nil person.first_name_before_last_save + assert_nil person.gender_before_last_save + + person.save + + assert_equal "Sean", person.first_name_before_last_save + assert_equal "M", person.gender_before_last_save + end + + test "saved_changes? returns whether the last call to save changed anything" do + person = Person.create!(first_name: "Sean") + + assert person.saved_changes? + + person.save + + refute person.saved_changes? + end + + test "saved_changes returns a hash of all the changes that occurred" do + person = Person.create!(first_name: "Sean", gender: "M") + + assert_equal [nil, "Sean"], person.saved_changes[:first_name] + assert_equal [nil, "M"], person.saved_changes[:gender] + assert_equal %w(id first_name gender created_at updated_at).sort, person.saved_changes.keys.sort + + travel(1.second) do + person.update(first_name: "Jim") + end + + assert_equal ["Sean", "Jim"], person.saved_changes[:first_name] + assert_equal %w(first_name lock_version updated_at).sort, person.saved_changes.keys.sort + end + + test "changed? in after callbacks returns true but is deprecated" do + klass = Class.new(ActiveRecord::Base) do + self.table_name = "people" + + after_save do + ActiveSupport::Deprecation.silence do + raise "changed? should be true" unless changed? + end + raise "has_changes_to_save? should be false" if has_changes_to_save? + end + end + + person = klass.create!(first_name: "Sean") + refute person.changed? + end + private def with_partial_writes(klass, on = true) old = klass.partial_writes? diff --git a/activerecord/test/cases/multiparameter_attributes_test.rb b/activerecord/test/cases/multiparameter_attributes_test.rb index 18ecc67f0b..ceb5724377 100644 --- a/activerecord/test/cases/multiparameter_attributes_test.rb +++ b/activerecord/test/cases/multiparameter_attributes_test.rb @@ -260,6 +260,13 @@ class MultiParameterAttributeTest < ActiveRecord::TestCase topic.attributes = attributes assert_equal Time.zone.local(2000, 1, 1, 16, 24, 0), topic.bonus_time assert_not topic.bonus_time.utc? + + attributes = { + "written_on(1i)" => "2000", "written_on(2i)" => "", "written_on(3i)" => "", + "written_on(4i)" => "", "written_on(5i)" => "" + } + topic.attributes = attributes + assert_nil topic.written_on end ensure Topic.reset_column_information diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 29b2deea26..90054ce83d 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -37,7 +37,7 @@ class QueryCacheTest < ActiveRecord::TestCase end def test_exceptional_middleware_clears_and_disables_cache_on_error - assert !ActiveRecord::Base.connection.query_cache_enabled, "cache off" + assert_cache :off mw = middleware { |env| Task.find 1 @@ -47,42 +47,66 @@ class QueryCacheTest < ActiveRecord::TestCase } assert_raises(RuntimeError) { mw.call({}) } - assert_equal 0, ActiveRecord::Base.connection.query_cache.length - assert !ActiveRecord::Base.connection.query_cache_enabled, "cache off" + assert_cache :off end - def test_exceptional_middleware_cleans_up_correct_cache - connection = ActiveRecord::Base.connection - called = false + def test_query_cache_across_threads + ActiveRecord::Base.connection_pool.connections.each do |conn| + assert_cache :off, conn + end + + assert !ActiveRecord::Base.connection.nil? + assert_cache :off + + middleware { + assert_cache :clean - mw = middleware { |env| - Task.find 1 Task.find 1 - assert_equal 1, connection.query_cache.length + assert_cache :dirty - # Checkin connection early + thread_1_connection = ActiveRecord::Base.connection ActiveRecord::Base.clear_active_connections! - # Make sure ActiveRecord::Base.connection doesn't checkout the same connection - ActiveRecord::Base.connection_pool.remove(connection) + assert_cache :off, thread_1_connection - called = true - } - mw.call({}) + started = Concurrent::Event.new + checked = Concurrent::Event.new - assert called - assert_equal 0, connection.query_cache.length - assert !connection.query_cache_enabled, "cache off" - end + thread_2_connection = nil + thread = Thread.new { + thread_2_connection = ActiveRecord::Base.connection - def test_exceptional_middleware_leaves_enabled_cache_alone - ActiveRecord::Base.connection.enable_query_cache! + assert_equal thread_2_connection, thread_1_connection + assert_cache :off - mw = middleware { |env| - raise "lol borked" - } - assert_raises(RuntimeError) { mw.call({}) } + middleware { + assert_cache :clean + + Task.find 1 + assert_cache :dirty + + started.set + checked.wait + + ActiveRecord::Base.clear_active_connections! + }.call({}) + } + + started.wait - assert ActiveRecord::Base.connection.query_cache_enabled, "cache on" + thread_1_connection = ActiveRecord::Base.connection + assert_not_equal thread_1_connection, thread_2_connection + assert_cache :dirty, thread_2_connection + checked.set + thread.join + + assert_cache :off, thread_2_connection + }.call({}) + + ActiveRecord::Base.connection_pool.connections.each do |conn| + assert_cache :off, conn + end + ensure + ActiveRecord::Base.clear_all_connections! end def test_middleware_delegates @@ -106,10 +130,10 @@ class QueryCacheTest < ActiveRecord::TestCase end def test_cache_enabled_during_call - assert !ActiveRecord::Base.connection.query_cache_enabled, "cache off" + assert_cache :off mw = middleware { |env| - assert ActiveRecord::Base.connection.query_cache_enabled, "cache on" + assert_cache :clean [200, {}, nil] } mw.call({}) @@ -289,12 +313,62 @@ class QueryCacheTest < ActiveRecord::TestCase end end + def test_query_cache_does_not_establish_connection_if_unconnected + ActiveRecord::Base.clear_active_connections! + refute ActiveRecord::Base.connection_handler.active_connections? # sanity check + + middleware { + refute ActiveRecord::Base.connection_handler.active_connections?, "QueryCache forced ActiveRecord::Base to establish a connection in setup" + }.call({}) + + refute ActiveRecord::Base.connection_handler.active_connections?, "QueryCache forced ActiveRecord::Base to establish a connection in cleanup" + end + + def test_query_cache_is_enabled_on_connections_established_after_middleware_runs + ActiveRecord::Base.clear_active_connections! + refute ActiveRecord::Base.connection_handler.active_connections? # sanity check + + middleware { + assert ActiveRecord::Base.connection.query_cache_enabled, "QueryCache did not get lazily enabled" + }.call({}) + end + + def test_query_caching_is_local_to_the_current_thread + ActiveRecord::Base.clear_active_connections! + + middleware { + assert ActiveRecord::Base.connection_pool.query_cache_enabled + assert ActiveRecord::Base.connection.query_cache_enabled + + Thread.new { + refute ActiveRecord::Base.connection_pool.query_cache_enabled + refute ActiveRecord::Base.connection.query_cache_enabled + }.join + }.call({}) + end + private def middleware(&app) executor = Class.new(ActiveSupport::Executor) ActiveRecord::QueryCache.install_executor_hooks executor lambda { |env| executor.wrap { app.call(env) } } end + + def assert_cache(state, connection = ActiveRecord::Base.connection) + case state + when :off + assert !connection.query_cache_enabled, "cache should be off" + assert connection.query_cache.empty?, "cache should be empty" + when :clean + assert connection.query_cache_enabled, "cache should be on" + assert connection.query_cache.empty?, "cache should be empty" + when :dirty + assert connection.query_cache_enabled, "cache should be on" + assert !connection.query_cache.empty?, "cache should be dirty" + else + raise "unknown state" + end + end end class QueryCacheExpiryTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb index 296dafacc2..05b71638c1 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -124,6 +124,10 @@ module ActiveRecord assert_equal "'lol'", @quoter.quote(DateTime.now, nil) end + def test_quoting_classes + assert_equal "'Object'", @quoter.quote(Object) + end + def test_crazy_object crazy = Object.new e = assert_raises(TypeError) do diff --git a/activerecord/test/models/eye.rb b/activerecord/test/models/eye.rb index ab3b3eacf3..f53c34e4b1 100644 --- a/activerecord/test/models/eye.rb +++ b/activerecord/test/models/eye.rb @@ -22,12 +22,12 @@ class Eye < ActiveRecord::Base alias trace_after_create2 trace_after_create def trace_after_update - (@after_update_callbacks_stack ||= []) << iris.changed? + (@after_update_callbacks_stack ||= []) << iris.has_changes_to_save? end alias trace_after_update2 trace_after_update def trace_after_save - (@after_save_callbacks_stack ||= []) << iris.changed? + (@after_save_callbacks_stack ||= []) << iris.has_changes_to_save? end alias trace_after_save2 trace_after_save end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 983ac076a9..d889f46031 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -905,7 +905,6 @@ ActiveRecord::Schema.define do create_table(t, force: true) {} end - # NOTE - the following 4 tables are used by models that have :inverse_of options on the associations create_table :men, force: true do |t| t.string :name end @@ -929,14 +928,14 @@ ActiveRecord::Schema.define do t.integer :zine_id end - create_table :wheels, force: true do |t| - t.references :wheelable, polymorphic: true - end - create_table :zines, force: true do |t| t.string :title end + create_table :wheels, force: true do |t| + t.references :wheelable, polymorphic: true + end + create_table :countries, force: true, id: false, primary_key: "country_id" do |t| t.string :country_id t.string :name diff --git a/activerecord/test/support/config.rb b/activerecord/test/support/config.rb index d0717f7b34..5817e427e3 100644 --- a/activerecord/test/support/config.rb +++ b/activerecord/test/support/config.rb @@ -1,5 +1,5 @@ require "yaml" -require "erubis" +require "erb" require "fileutils" require "pathname" @@ -20,7 +20,7 @@ module ARTest FileUtils.cp TEST_ROOT + "/config.example.yml", config_file end - erb = Erubis::Eruby.new(config_file.read) + erb = ERB.new(config_file.read) expand_config(YAML.parse(erb.result(binding)).transform) end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 83ff80e31a..fac91c31da 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,23 @@ +* Ensure duration parsing is consistent across DST changes + + Previously `ActiveSupport::Duration.parse` used `Time.current` and + `Time#advance` to calculate the number of seconds in the duration + from an arbitrary collection of parts. However as `advance` tries to + be consistent across DST boundaries this meant that either the + duration was shorter or longer depending on the time of year. + + This was fixed by using an absolute reference point in UTC which + isn't subject to DST transitions. An arbitrary date of Jan 1st, 2000 + was chosen for no other reason that it seemed appropriate. + + Additionally, duration parsing should now be marginally faster as we + are no longer creating instances of `ActiveSupport::TimeWithZone` + every time we parse a duration string. + + Fixes #26941. + + *Andrew White* + * Use `Hash#compact` and `Hash#compact!` from Ruby 2.4. Old Ruby versions will continue to get these methods from Active Support as before. diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index cfd5e39bc4..00f4480308 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -110,7 +110,7 @@ module ActiveSupport # operator and can only be used on values written with the :raw option. # Calling it on a value not stored with :raw will initialize that value # to zero. - def increment(name, amount = 1, options = nil) # :nodoc: + def increment(name, amount = 1, options = nil) options = merged_options(options) instrument(:increment, name, amount: amount) do rescue_error_with nil do @@ -123,7 +123,7 @@ module ActiveSupport # operator and can only be used on values written with the :raw option. # Calling it on a value not stored with :raw will initialize that value # to zero. - def decrement(name, amount = 1, options = nil) # :nodoc: + def decrement(name, amount = 1, options = nil) options = merged_options(options) instrument(:decrement, name, amount: amount) do rescue_error_with nil do diff --git a/activesupport/lib/active_support/core_ext/hash/compact.rb b/activesupport/lib/active_support/core_ext/hash/compact.rb index 5cae495bda..e357284be0 100644 --- a/activesupport/lib/active_support/core_ext/hash/compact.rb +++ b/activesupport/lib/active_support/core_ext/hash/compact.rb @@ -14,7 +14,7 @@ class Hash unless Hash.instance_methods(false).include?(:compact!) # Replaces current hash with non +nil+ values. - # Returns nil if no changes were made, otherwise returns the hash. + # Returns +nil+ if no changes were made, otherwise returns the hash. # # hash = { a: true, b: false, c: nil } # hash.compact! # => { a: true, b: false } diff --git a/activesupport/lib/active_support/core_ext/kernel/reporting.rb b/activesupport/lib/active_support/core_ext/kernel/reporting.rb index d0197af95f..c02618d5f3 100644 --- a/activesupport/lib/active_support/core_ext/kernel/reporting.rb +++ b/activesupport/lib/active_support/core_ext/kernel/reporting.rb @@ -1,7 +1,7 @@ module Kernel module_function - # Sets $VERBOSE to nil for the duration of the block and back to its original + # Sets $VERBOSE to +nil+ for the duration of the block and back to its original # value afterwards. # # silence_warnings do diff --git a/activesupport/lib/active_support/core_ext/securerandom.rb b/activesupport/lib/active_support/core_ext/securerandom.rb index b2e4fff79a..a57685bea1 100644 --- a/activesupport/lib/active_support/core_ext/securerandom.rb +++ b/activesupport/lib/active_support/core_ext/securerandom.rb @@ -6,7 +6,7 @@ module SecureRandom # # The argument _n_ specifies the length, of the random string to be generated. # - # If _n_ is not specified or is nil, 16 is assumed. It may be larger in the future. + # If _n_ is not specified or is +nil+, 16 is assumed. It may be larger in the future. # # The result may contain alphanumeric characters except 0, O, I and l # diff --git a/activesupport/lib/active_support/core_ext/string/access.rb b/activesupport/lib/active_support/core_ext/string/access.rb index caa48e34c5..6133826f37 100644 --- a/activesupport/lib/active_support/core_ext/string/access.rb +++ b/activesupport/lib/active_support/core_ext/string/access.rb @@ -3,7 +3,7 @@ class String # position. The first character of the string is at position 0, the next at # position 1, and so on. If a range is supplied, a substring containing # characters at offsets given by the range is returned. In both cases, if an - # offset is negative, it is counted from the end of the string. Returns nil + # offset is negative, it is counted from the end of the string. Returns +nil+ # if the initial offset falls outside the string. Returns an empty string if # the beginning of the range is greater than the end of the string. # @@ -17,7 +17,7 @@ class String # # If a Regexp is given, the matching portion of the string is returned. # If a String is given, that given string is returned if it occurs in - # the string. In both cases, nil is returned if there is no match. + # the string. In both cases, +nil+ is returned if there is no match. # # str = "hello" # str.at(/lo/) # => "lo" diff --git a/activesupport/lib/active_support/core_ext/string/inflections.rb b/activesupport/lib/active_support/core_ext/string/inflections.rb index 7e12700c8c..765c0919bb 100644 --- a/activesupport/lib/active_support/core_ext/string/inflections.rb +++ b/activesupport/lib/active_support/core_ext/string/inflections.rb @@ -67,7 +67,7 @@ class String end # +safe_constantize+ tries to find a declared constant with the name specified - # in the string. It returns nil when the name is not in CamelCase + # in the string. It returns +nil+ when the name is not in CamelCase # or is not initialized. See ActiveSupport::Inflector.safe_constantize # # 'Module'.safe_constantize # => Module diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb index 862dabb1e8..82322291d0 100644 --- a/activesupport/lib/active_support/duration.rb +++ b/activesupport/lib/active_support/duration.rb @@ -7,6 +7,8 @@ module ActiveSupport # # 1.month.ago # equivalent to Time.now.advance(months: -1) class Duration + EPOCH = ::Time.utc(2000) + attr_accessor :value, :parts autoload :ISO8601Parser, "active_support/duration/iso8601_parser" @@ -140,8 +142,7 @@ module ActiveSupport # If invalid string is provided, it will raise +ActiveSupport::Duration::ISO8601Parser::ParsingError+. def self.parse(iso8601duration) parts = ISO8601Parser.new(iso8601duration).parse! - time = ::Time.current - new(time.advance(parts) - time, parts) + new(EPOCH.advance(parts) - EPOCH, parts) end # Build ISO 8601 Duration string for this duration. diff --git a/activesupport/lib/active_support/duration/iso8601_serializer.rb b/activesupport/lib/active_support/duration/iso8601_serializer.rb index 39b1b40e72..51d53e2f8d 100644 --- a/activesupport/lib/active_support/duration/iso8601_serializer.rb +++ b/activesupport/lib/active_support/duration/iso8601_serializer.rb @@ -26,7 +26,7 @@ module ActiveSupport if parts.key?(:seconds) time << "#{sprintf(@precision ? "%0.0#{@precision}f" : '%g', parts[:seconds])}S" end - output << "T#{time}" if time.present? + output << "T#{time}" unless time.empty? "#{sign}#{output}" end diff --git a/activesupport/lib/active_support/json/encoding.rb b/activesupport/lib/active_support/json/encoding.rb index 6eb73723ad..defaf3f395 100644 --- a/activesupport/lib/active_support/json/encoding.rb +++ b/activesupport/lib/active_support/json/encoding.rb @@ -68,7 +68,8 @@ module ActiveSupport :ESCAPE_REGEX_WITHOUT_HTML_ENTITIES, :EscapedString # Convert an object into a "JSON-ready" representation composed of - # primitives like Hash, Array, String, Numeric, and true/false/nil. + # primitives like Hash, Array, String, Numeric, + # and +true+/+false+/+nil+. # Recursively calls #as_json to the object to recursively build a # fully JSON-ready object. # diff --git a/activesupport/lib/active_support/multibyte/chars.rb b/activesupport/lib/active_support/multibyte/chars.rb index 938e4ebb72..262f25b874 100644 --- a/activesupport/lib/active_support/multibyte/chars.rb +++ b/activesupport/lib/active_support/multibyte/chars.rb @@ -87,7 +87,7 @@ module ActiveSupport #:nodoc: end # Works like <tt>String#slice!</tt>, but returns an instance of - # Chars, or nil if the string was not modified. The string will not be + # Chars, or +nil+ if the string was not modified. The string will not be # modified if the range given is out of bounds # # string = 'Welcome' diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb index 7a49bbb960..6000ea44be 100644 --- a/activesupport/lib/active_support/number_helper.rb +++ b/activesupport/lib/active_support/number_helper.rb @@ -109,7 +109,7 @@ module ActiveSupport # * <tt>:locale</tt> - Sets the locale to be used for formatting # (defaults to current locale). # * <tt>:precision</tt> - Sets the precision of the number - # (defaults to 3). Keeps the number's precision if nil. + # (defaults to 3). Keeps the number's precision if +nil+. # * <tt>:significant</tt> - If +true+, precision will be the number # of significant_digits. If +false+, the number of fractional # digits (defaults to +false+). @@ -183,7 +183,7 @@ module ActiveSupport # * <tt>:locale</tt> - Sets the locale to be used for formatting # (defaults to current locale). # * <tt>:precision</tt> - Sets the precision of the number - # (defaults to 3). Keeps the number's precision if nil. + # (defaults to 3). Keeps the number's precision if +nil+. # * <tt>:significant</tt> - If +true+, precision will be the number # of significant_digits. If +false+, the number of fractional # digits (defaults to +false+). diff --git a/activesupport/lib/active_support/rescuable.rb b/activesupport/lib/active_support/rescuable.rb index dc3f27a16d..135690cc42 100644 --- a/activesupport/lib/active_support/rescuable.rb +++ b/activesupport/lib/active_support/rescuable.rb @@ -74,7 +74,7 @@ module ActiveSupport # # If no handler matches the exception, check for a handler matching the # (optional) exception.cause. If no handler matches the exception or its - # cause, this returns nil so you can deal with unhandled exceptions. + # cause, this returns +nil+, so you can deal with unhandled exceptions. # Be sure to re-raise unhandled exceptions if this is what you expect. # # begin @@ -83,7 +83,7 @@ module ActiveSupport # rescue_with_handler(exception) || raise # end # - # Returns the exception if it was handled and nil if it was not. + # Returns the exception if it was handled and +nil+ if it was not. def rescue_with_handler(exception, object: self) if handler = handler_for_rescue(exception, object: object) handler.call exception diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 53575272e3..b7b4a9dd00 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -292,11 +292,11 @@ class DurationTest < ActiveSupport::TestCase def test_iso8601_output_precision expectations = [ - [nil, "P1Y1MT5.55S", 1.year + 1.month + (5.55).seconds ], - [0, "P1Y1MT6S", 1.year + 1.month + (5.55).seconds ], - [1, "P1Y1MT5.5S", 1.year + 1.month + (5.55).seconds ], - [2, "P1Y1MT5.55S", 1.year + 1.month + (5.55).seconds ], - [3, "P1Y1MT5.550S", 1.year + 1.month + (5.55).seconds ], + [nil, "P1Y1MT8.55S", 1.year + 1.month + (8.55).seconds ], + [0, "P1Y1MT9S", 1.year + 1.month + (8.55).seconds ], + [1, "P1Y1MT8.6S", 1.year + 1.month + (8.55).seconds ], + [2, "P1Y1MT8.55S", 1.year + 1.month + (8.55).seconds ], + [3, "P1Y1MT8.550S", 1.year + 1.month + (8.55).seconds ], [nil, "PT1S", 1.second ], [2, "PT1.00S", 1.second ], [nil, "PT1.4S", (1.4).seconds ], @@ -322,4 +322,35 @@ class DurationTest < ActiveSupport::TestCase assert_equal time + duration, time + ActiveSupport::Duration.parse(duration.iso8601), pattern.inspect end end + + def test_iso8601_parsing_across_spring_dst_boundary + with_env_tz eastern_time_zone do + with_tz_default "Eastern Time (US & Canada)" do + travel_to Time.utc(2016, 3, 11) do + assert_equal 604800, ActiveSupport::Duration.parse("P7D").to_i + assert_equal 604800, ActiveSupport::Duration.parse("P1W").to_i + end + end + end + end + + def test_iso8601_parsing_across_autumn_dst_boundary + with_env_tz eastern_time_zone do + with_tz_default "Eastern Time (US & Canada)" do + travel_to Time.utc(2016, 11, 4) do + assert_equal 604800, ActiveSupport::Duration.parse("P7D").to_i + assert_equal 604800, ActiveSupport::Duration.parse("P1W").to_i + end + end + end + end + + private + def eastern_time_zone + if Gem.win_platform? + "EST5EDT" + else + "America/New_York" + end + end end diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index 7b1138c7d4..40eb838d32 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -258,6 +258,17 @@ scalar values, map the key to an empty array: params.permit(id: []) ``` +Sometimes it is not possible or convenient to declare the valid keys of +a hash parameter or its internal structure. Just map to an empty hash: + +```ruby +params.permit(preferences: {}) +``` + +but be careful because this opens the door to arbitrary input. In this +case, `permit` ensures values in the returned structure are permitted +scalars and filters out anything else. + To whitelist an entire hash of parameters, the `permit!` method can be used: @@ -265,9 +276,10 @@ used: params.require(:log_entry).permit! ``` -This will mark the `:log_entry` parameters hash and any sub-hash of it as -permitted. Extreme care should be taken when using `permit!`, as it -will allow all current and future model attributes to be mass-assigned. +This marks the `:log_entry` parameters hash and any sub-hash of it as +permitted and does not check for permitted scalars, anything is accepted. +Extreme care should be taken when using `permit!`, as it will allow all current +and future model attributes to be mass-assigned. #### Nested Parameters diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md index d91c9bd606..6e7e29ed60 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -229,7 +229,7 @@ As always, what has been generated for you is just a starting point. You can add 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 +Also, the generator accepts column type as `references` (also available as `belongs_to`). For instance: ```bash @@ -958,10 +958,10 @@ ActiveRecord::Schema.define(version: 20080906171750) do create_table "products", force: true do |t| t.string "name" - t.text "description" + t.text "description" t.datetime "created_at" t.datetime "updated_at" - t.string "part_number" + t.string "part_number" end end ``` diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 38b1ffc4c8..31220f9be2 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -953,6 +953,9 @@ class Client < ApplicationRecord end ``` +NOTE: Please note that the optimistic locking will be ignored if you update the +locking column's value. + ### Pessimistic Locking Pessimistic locking uses a locking mechanism provided by the underlying database. Using `lock` when building a relation obtains an exclusive lock on the selected rows. Relations using `lock` are usually wrapped inside a transaction for preventing deadlock conditions. diff --git a/guides/source/active_support_instrumentation.md b/guides/source/active_support_instrumentation.md index 3fc9d9bfa9..03c9183eb3 100644 --- a/guides/source/active_support_instrumentation.md +++ b/guides/source/active_support_instrumentation.md @@ -226,6 +226,24 @@ Action View } ``` +### render_collection.action_view + +| Key | Value | +| ------------- | ------------------------------------- | +| `:identifier` | Full path to template | +| `:count` | Size of collection | +| `:cache_hits` | Number of partials fetched from cache | + +`:cache_hits` is only included if the collection is rendered with `cached: true`. + +```ruby +{ + identifier: "/Users/adam/projects/notifications/app/views/posts/_post.html.erb", + count: 3, + cache_hits: 0 +} +``` + Active Record ------------ diff --git a/guides/source/configuring.md b/guides/source/configuring.md index c625cf67f6..b0334bfe4a 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -175,11 +175,11 @@ pipeline is enabled. It is set to `true` by default. * `config.assets.manifest` defines the full path to be used for the asset precompiler's manifest file. Defaults to a file named `manifest-<random>.json` in the `config.assets.prefix` directory within the public folder. -* `config.assets.digest` enables the use of MD5 fingerprints in asset names. Set to `true` by default. +* `config.assets.digest` enables the use of SHA256 fingerprints in asset names. Set to `true` by default. * `config.assets.debug` disables the concatenation and compression of assets. Set to `true` by default in `development.rb`. -* `config.assets.version` is an option string that is used in MD5 hash generation. This can be changed to force all files to be recompiled. +* `config.assets.version` is an option string that is used in SHA256 hash generation. This can be changed to force all files to be recompiled. * `config.assets.compile` is a boolean that can be used to turn on live Sprockets compilation in production. diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index 31d5c4f71d..c04d42d743 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -1655,8 +1655,8 @@ This creates five files and one empty directory: | app/views/comments/ | Views of the controller are stored here | | test/controllers/comments_controller_test.rb | The test for the controller | | app/helpers/comments_helper.rb | A view helper file | -| app/assets/javascripts/comment.coffee | CoffeeScript for the controller | -| app/assets/stylesheets/comment.scss | Cascading style sheet for the controller | +| app/assets/javascripts/comments.coffee | CoffeeScript for the controller | +| app/assets/stylesheets/comments.scss | Cascading style sheet for the controller | Like with any blog, our readers will create their comments directly after reading the article, and once they have added their comment, will be sent back diff --git a/guides/source/i18n.md b/guides/source/i18n.md index 887774961a..fd54bca4ff 100644 --- a/guides/source/i18n.md +++ b/guides/source/i18n.md @@ -166,7 +166,7 @@ def set_locale I18n.locale = extract_locale_from_tld || I18n.default_locale end -# Get locale from top-level domain or return nil if such locale is not available +# Get locale from top-level domain or return +nil+ if such locale is not available # You have to put something like: # 127.0.0.1 application.com # 127.0.0.1 application.it diff --git a/guides/source/layouts_and_rendering.md b/guides/source/layouts_and_rendering.md index 7e4ec5ba7e..0c24ec884c 100644 --- a/guides/source/layouts_and_rendering.md +++ b/guides/source/layouts_and_rendering.md @@ -1082,7 +1082,7 @@ definitions for several similar resources: * `shared/_search_filters.html.erb` ```html+erb - <%= form_for(@q) do |f| %> + <%= form_for(search) do |f| %> <h1>Search form:</h1> <fieldset> <%= yield f %> diff --git a/guides/source/rails_on_rack.md b/guides/source/rails_on_rack.md index ed935e1008..340933c7ee 100644 --- a/guides/source/rails_on_rack.md +++ b/guides/source/rails_on_rack.md @@ -181,7 +181,6 @@ $ bin/rails middleware (in /Users/lifo/Rails/blog) use ActionDispatch::Static use #<ActiveSupport::Cache::Strategy::LocalCache::Middleware:0x00000001c304c8> -use Rack::Runtime ... run Rails.application.routes ``` diff --git a/guides/source/testing.md b/guides/source/testing.md index 0ac5121b12..bc1f78fb2a 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -414,7 +414,7 @@ You can also run an entire directory of tests by providing the path to the direc $ bin/rails test test/controllers # run all tests from specific directory ``` -The test runner provides lot of other features too like failing fast, deferring test output +The test runner also provides a lot of other features like failing fast, deferring test output at the end of test run and so on. Check the documentation of the test runner as follows: ```bash @@ -859,7 +859,7 @@ You also have access to three instance variables in your functional tests, after class ArticlesControllerTest < ActionDispatch::IntegrationTest test "should get index" do get articles_url - + assert_equal "index", @controller.action_name assert_equal "application/x-www-form-urlencoded", @request.media_type assert_match "Articles", @response.body diff --git a/railties/lib/rails.rb b/railties/lib/rails.rb index 5d862e3fec..ee48043a50 100644 --- a/railties/lib/rails.rb +++ b/railties/lib/rails.rb @@ -53,7 +53,7 @@ module Rails end # Returns a Pathname object of the current Rails project, - # otherwise it returns nil if there is no project: + # otherwise it returns +nil+ if there is no project: # # Rails.root # # => #<Pathname:/Users/someuser/some/path/project> @@ -100,7 +100,7 @@ module Rails end # Returns a Pathname object of the public folder of the current - # Rails project, otherwise it returns nil if there is no project: + # Rails project, otherwise it returns +nil+ if there is no project: # # Rails.public_path # # => #<Pathname:/Users/someuser/some/path/project/public> diff --git a/railties/lib/rails/commands/dbconsole/dbconsole_command.rb b/railties/lib/rails/commands/dbconsole/dbconsole_command.rb index 54457cf78b..35e8673215 100644 --- a/railties/lib/rails/commands/dbconsole/dbconsole_command.rb +++ b/railties/lib/rails/commands/dbconsole/dbconsole_command.rb @@ -1,6 +1,3 @@ -require "erb" -require "yaml" - require "rails/command/environment_argument" module Rails diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 91342c592c..83e9c30548 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -67,6 +67,9 @@ module Rails class_option :skip_listen, type: :boolean, default: false, desc: "Don't generate configuration that depends on the listen gem" + class_option :skip_coffee, type: :boolean, default: false, + desc: "Don't use CoffeeScript" + class_option :skip_javascript, type: :boolean, aliases: "-J", default: false, desc: "Skip JavaScript files" @@ -322,7 +325,9 @@ module Rails if options[:skip_javascript] || options[:skip_sprockets] [] else - gems = [coffee_gemfile_entry, javascript_runtime_gemfile_entry] + gems = [javascript_runtime_gemfile_entry] + gems << coffee_gemfile_entry unless options[:skip_coffee] + gems << GemfileEntry.version("#{options[:javascript]}-rails", nil, "Use #{options[:javascript]} as the JavaScript library") diff --git a/railties/lib/rails/generators/rails/app/templates/bin/setup b/railties/lib/rails/generators/rails/app/templates/bin/setup.tt index acae810c1a..8635e97b76 100644 --- a/railties/lib/rails/generators/rails/app/templates/bin/setup +++ b/railties/lib/rails/generators/rails/app/templates/bin/setup.tt @@ -16,6 +16,7 @@ chdir APP_ROOT do puts '== Installing dependencies ==' system! 'gem install bundler --conservative' system('bundle check') || system!('bundle install') +<% unless options.skip_active_record -%> # puts "\n== Copying sample files ==" # unless File.exist?('config/database.yml') @@ -24,6 +25,7 @@ chdir APP_ROOT do puts "\n== Preparing database ==" system! 'bin/rails db:setup' +<% end -%> puts "\n== Removing old logs and tempfiles ==" system! 'bin/rails log:clear tmp:clear' diff --git a/railties/lib/rails/generators/rails/app/templates/bin/update b/railties/lib/rails/generators/rails/app/templates/bin/update.tt index 770a605fed..d385b363c6 100644 --- a/railties/lib/rails/generators/rails/app/templates/bin/update +++ b/railties/lib/rails/generators/rails/app/templates/bin/update.tt @@ -16,9 +16,11 @@ chdir APP_ROOT do puts '== Installing dependencies ==' system! 'gem install bundler --conservative' system('bundle check') || system!('bundle install') +<% unless options.skip_active_record -%> puts "\n== Updating database ==" system! 'bin/rails db:migrate' +<% end -%> puts "\n== Removing old logs and tempfiles ==" system! 'bin/rails log:clear tmp:clear' diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index 80afdcc726..1d968ec8f0 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -283,7 +283,7 @@ task default: :test end def namespaced_name - @namespaced_name ||= name.gsub("-", "/") + @namespaced_name ||= name.tr("-", "/") end protected diff --git a/railties/lib/rails/railtie.rb b/railties/lib/rails/railtie.rb index eb3f5d4ee9..696db61f01 100644 --- a/railties/lib/rails/railtie.rb +++ b/railties/lib/rails/railtie.rb @@ -132,27 +132,19 @@ module Rails end def rake_tasks(&blk) - @rake_tasks ||= [] - @rake_tasks << blk if blk - @rake_tasks + register_block_for(:rake_tasks, &blk) end def console(&blk) - @load_console ||= [] - @load_console << blk if blk - @load_console + register_block_for(:load_console, &blk) end def runner(&blk) - @load_runner ||= [] - @load_runner << blk if blk - @load_runner + register_block_for(:runner, &blk) end def generators(&blk) - @generators ||= [] - @generators << blk if blk - @generators + register_block_for(:generators, &blk) end def abstract_railtie? @@ -195,6 +187,17 @@ module Rails super end end + + private + # receives an instance variable identifier, set the variable value if is + # blank and append given block to value, which will be used later in + # `#each_registered_block(type, &block)` + def register_block_for(type, &blk) + var_name = "@#{type}" + blocks = instance_variable_defined?(var_name) ? instance_variable_get(var_name) : instance_variable_set(var_name, []) + blocks << blk if blk + blocks + end end delegate :railtie_name, to: :class @@ -241,6 +244,7 @@ module Rails private + # run `&block` in every registered block in `#register_block_for` def each_registered_block(type, &block) klass = self.class while klass.respond_to?(type) diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 9299b9ebea..3ec99193e3 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -363,6 +363,12 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_file "test/test_helper.rb" do |helper_content| assert_no_match(/fixtures :all/, helper_content) end + assert_file "bin/setup" do |setup_content| + assert_no_match(/db:setup/, setup_content) + end + assert_file "bin/update" do |update_content| + assert_no_match(/db:migrate/, update_content) + end assert_file "config/initializers/new_framework_defaults.rb" do |initializer_content| assert_no_match(/belongs_to_required_by_default/, initializer_content) @@ -482,6 +488,16 @@ class AppGeneratorTest < Rails::Generators::TestCase end end + def test_coffeescript_is_skipped_if_required + run_generator [destination_root, "--skip-coffee"] + + assert_file "Gemfile" do |content| + assert_no_match(/coffee-rails/, content) + assert_match(/jquery-rails/, content) + assert_match(/uglifier/, content) + end + end + def test_inclusion_of_jbuilder run_generator assert_gem "jbuilder" |