diff options
46 files changed, 268 insertions, 98 deletions
diff --git a/actionmailer/lib/action_mailer/test_case.rb b/actionmailer/lib/action_mailer/test_case.rb index d3eded547c..1d09a4ee96 100644 --- a/actionmailer/lib/action_mailer/test_case.rb +++ b/actionmailer/lib/action_mailer/test_case.rb @@ -41,6 +41,7 @@ module ActionMailer setup :initialize_test_deliveries setup :set_expected_mail teardown :restore_test_deliveries + ActiveSupport.run_load_hooks(:action_mailer_test_case, self) end module ClassMethods diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index a66a1e8af3..999ac82d42 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,10 +1,27 @@ +* Fix nested multiple roots + + The PR #20940 enabled the use of multiple roots with different constraints + at the top level but unfortunately didn't work when those roots were inside + a namespace and also broke the use of root inside a namespace after a top + level root was defined because the check for the existence of the named route + used the global :root name and not the namespaced name. + + This is fixed by using the name_for_action method to expand the :root name to + the full namespaced name. We can pass nil for the second argument as we're not + dealing with resource definitions so don't need to handle the cases for edit + and new routes. + + Fixes #26148. + + *Ryo Hashimoto*, *Andrew White* + * Include the content of the flash in the auto-generated etag. This solves the following problem: 1. POST /messages 2. redirect_to messages_url, notice: 'Message was created' 3. GET /messages/1 4. GET /messages - + Step 4 would before still include the flash message, even though it's no longer relevant, because the etag cache was recorded with the flash in place and didn't change when it was gone. diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index b598dd4d77..6a1acb64f7 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -70,6 +70,7 @@ module ActionController #:nodoc: send_file_headers! options self.status = options[:status] || 200 + self.content_type = options[:type] if options.key?(:type) self.content_type = options[:content_type] if options.key?(:content_type) response.send_file path end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 11a6cf8e15..d84320b713 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -627,6 +627,7 @@ module ActionController include ActionDispatch::Assertions class_attribute :_controller_class setup :setup_controller_request_and_response + ActiveSupport.run_load_hooks(:action_controller_test_case, self) end private diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index d5eef2987d..0528a6ad08 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -86,7 +86,7 @@ module ActionDispatch @req.fetch_header(env_name(key)) do return default unless default == DEFAULT return yield if block_given? - raise NameError, key + raise KeyError, key end end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 9788431943..e8173e2a99 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -224,8 +224,10 @@ module ActionDispatch # :nodoc: # Sets the HTTP content type. def content_type=(content_type) - header_info = parse_content_type - set_content_type content_type.to_s, header_info.charset || self.class.default_charset + return unless content_type + new_header_info = parse_content_type(content_type.to_s) + prev_header_info = parsed_content_type_header + set_content_type new_header_info.mime_type, new_header_info.charset || prev_header_info.charset || self.class.default_charset end # Sets the HTTP response's content MIME type. For example, in the controller @@ -238,7 +240,7 @@ module ActionDispatch # :nodoc: # information. def content_type - parse_content_type.mime_type + parsed_content_type_header.mime_type end def sending_file=(v) @@ -253,7 +255,7 @@ module ActionDispatch # :nodoc: # response.charset = 'utf-16' # => 'utf-16' # response.charset = nil # => 'utf-8' def charset=(charset) - header_info = parse_content_type + header_info = parsed_content_type_header if false == charset set_header CONTENT_TYPE, header_info.mime_type else @@ -265,7 +267,7 @@ module ActionDispatch # :nodoc: # The charset of the response. HTML wants to know the encoding of the # content you're giving them, so we need to send that along. def charset - header_info = parse_content_type + header_info = parsed_content_type_header header_info.charset || self.class.default_charset end @@ -403,8 +405,7 @@ module ActionDispatch # :nodoc: ContentTypeHeader = Struct.new :mime_type, :charset NullContentTypeHeader = ContentTypeHeader.new nil, nil - def parse_content_type - content_type = get_header CONTENT_TYPE + def parse_content_type(content_type) if content_type type, charset = content_type.split(/;\s*charset=/) type = nil if type.empty? @@ -414,6 +415,12 @@ module ActionDispatch # :nodoc: end end + # Small internal convenience method to get the parsed version of the current + # content type header. + def parsed_content_type_header + parse_content_type(get_header(CONTENT_TYPE)) + end + def set_content_type(content_type, charset) type = (content_type || "").dup type << "; charset=#{charset}" if charset @@ -450,7 +457,7 @@ module ActionDispatch # :nodoc: def assign_default_content_type_and_charset! return if content_type - ct = parse_content_type + ct = parsed_content_type_header set_content_type(ct.mime_type || Mime[:html].to_s, ct.charset || self.class.default_charset) end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index b3acac42fc..67b9463a6a 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1927,7 +1927,7 @@ to this: end def match_root_route(options) - name = has_named_route?(:root) ? nil : :root + name = has_named_route?(name_for_action(:root, nil)) ? nil : :root args = ["/", { as: name, via: :get }.merge!(options)] match(*args) diff --git a/actionpack/test/controller/force_ssl_test.rb b/actionpack/test/controller/force_ssl_test.rb index 8d5e3ce2fe..2b3859aa57 100644 --- a/actionpack/test/controller/force_ssl_test.rb +++ b/actionpack/test/controller/force_ssl_test.rb @@ -229,14 +229,12 @@ class ForceSSLFlashTest < ActionController::TestCase assert_response 302 assert_equal "http://test.host/force_ssl_flash/cheeseburger", redirect_to_url - # FIXME: AC::TestCase#build_request_uri doesn't build a new uri if PATH_INFO exists @request.env.delete("PATH_INFO") get :cheeseburger assert_response 301 assert_equal "https://test.host/force_ssl_flash/cheeseburger", redirect_to_url - # FIXME: AC::TestCase#build_request_uri doesn't build a new uri if PATH_INFO exists @request.env.delete("PATH_INFO") get :use_flash diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index 25420ead3b..8dc565ac8d 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -233,4 +233,18 @@ class SendFileTest < ActionController::TestCase response = process("file") assert_equal 200, response.status end + + def test_send_file_charset_with_type_options_key + @controller = SendFileWithActionControllerLive.new + @controller.options = { type: "text/calendar; charset=utf-8" } + response = process("file") + assert_equal "text/calendar; charset=utf-8", response.headers["Content-Type"] + end + + def test_send_file_charset_with_content_type_options_key + @controller = SendFileWithActionControllerLive.new + @controller.options = { content_type: "text/calendar" } + response = process("file") + assert_equal "text/calendar; charset=utf-8", response.headers["Content-Type"] + end end diff --git a/actionpack/test/dispatch/header_test.rb b/actionpack/test/dispatch/header_test.rb index 7a155d70cb..374e618b42 100644 --- a/actionpack/test/dispatch/header_test.rb +++ b/actionpack/test/dispatch/header_test.rb @@ -158,4 +158,10 @@ class HeaderTest < ActiveSupport::TestCase assert_equal({ "HTTP_REFERER"=>"http://example.com/", "CONTENT_TYPE"=>"text/plain" }, env) end + + test "fetch exception" do + assert_raises KeyError do + @headers.fetch(:some_key_that_doesnt_exist) + end + end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index ee5c30ef0e..0938460632 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3669,6 +3669,48 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_multiple_roots + draw do + namespace :foo do + root "pages#index", constraints: { host: 'www.example.com' } + root "admin/pages#index", constraints: { host: 'admin.example.com' } + end + + root "pages#index", constraints: { host: 'www.example.com' } + root "admin/pages#index", constraints: { host: 'admin.example.com' } + end + + get "http://www.example.com/foo" + assert_equal "foo/pages#index", @response.body + + get "http://admin.example.com/foo" + assert_equal "foo/admin/pages#index", @response.body + + get "http://www.example.com/" + assert_equal "pages#index", @response.body + + get "http://admin.example.com/" + assert_equal "admin/pages#index", @response.body + end + + def test_namespaced_roots + draw do + namespace :foo do + root "test#index" + end + + root "test#index" + + namespace :bar do + root "test#index" + end + end + + assert_equal "/foo", foo_root_path + assert_equal "/", root_path + assert_equal "/bar", bar_root_path + end + private def draw(&block) diff --git a/actionview/lib/action_view/test_case.rb b/actionview/lib/action_view/test_case.rb index 2805cfe612..3eb1ac0826 100644 --- a/actionview/lib/action_view/test_case.rb +++ b/actionview/lib/action_view/test_case.rb @@ -152,6 +152,7 @@ module ActionView included do setup :setup_with_controller + ActiveSupport.run_load_hooks(:action_view_test_case, self) end private diff --git a/activejob/lib/active_job/test_case.rb b/activejob/lib/active_job/test_case.rb index 0d97e00bfa..a5ec45e4a7 100644 --- a/activejob/lib/active_job/test_case.rb +++ b/activejob/lib/active_job/test_case.rb @@ -3,5 +3,7 @@ require "active_support/test_case" module ActiveJob class TestCase < ActiveSupport::TestCase include ActiveJob::TestHelper + + ActiveSupport.run_load_hooks(:active_job_test_case, self) end end diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 45ef14013a..191039f598 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -372,7 +372,7 @@ module ActiveModel To achieve the same use: - errors.add(attribute, :empty, options) if value.blank? + errors.add(attribute, :blank, options) if value.blank? MESSAGE Array(attributes).each do |attribute| diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index af27955554..48e0e5c9f6 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -26,8 +26,6 @@ module ActiveModel raw_value = value end - return if options[:allow_nil] && raw_value.nil? - unless is_number?(raw_value) record.errors.add(attr_name, :not_a_number, filtered_options(raw_value)) return diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index 84ed430d2a..fefab29f15 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -35,6 +35,13 @@ class NumericalityValidationTest < ActiveModel::TestCase valid!(NIL + FLOATS + INTEGERS + BIGDECIMAL + INFINITY) end + def test_validates_numericality_of_with_blank_allowed + Topic.validates_numericality_of :approved, allow_blank: true + + invalid!(JUNK) + valid!(NIL + BLANK + FLOATS + INTEGERS + BIGDECIMAL + INFINITY) + end + def test_validates_numericality_of_with_integer_only Topic.validates_numericality_of :approved, only_integer: true diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4aa1853bde..f62d29b5bf 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,15 @@ +* Inverse association instances will now be set before `after_find` or + `after_initialize` callbacks are run. + + Fixes #26320. + + *Sean Griffin* + +* Remove unnecessarily association load when a `belongs_to` association has already been + loaded then the foreign key is changed directly and the record saved. + + *James Coleman* + * Remove standardized column types/arguments spaces in schema dump. *Tim Petricola* diff --git a/activerecord/lib/active_record/association_relation.rb b/activerecord/lib/active_record/association_relation.rb index 2da2d968b9..de2d03cd0b 100644 --- a/activerecord/lib/active_record/association_relation.rb +++ b/activerecord/lib/active_record/association_relation.rb @@ -29,7 +29,10 @@ module ActiveRecord private def exec_queries - super.each { |r| @association.set_inverse_instance r } + super do |r| + @association.set_inverse_instance r + yield r if block_given? + end end end end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 0f51b35164..0c911a5396 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -390,9 +390,9 @@ module ActiveRecord end binds = AssociationScope.get_bind_values(owner, reflection.chain) - records = sc.execute(binds, klass, conn) - records.each { |record| set_inverse_instance(record) } - records + sc.execute(binds, klass, conn) do |record| + set_inverse_instance(record) + end end # We have some records loaded from the database (persisted) and some that are diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 62acad0eda..02f0721bed 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -286,17 +286,19 @@ module ActiveRecord end def construct_model(record, node, row, model_cache, id, aliases) - model = model_cache[node][id] ||= node.instantiate(row, - aliases.column_aliases(node)) other = record.association(node.reflection.name) + model = model_cache[node][id] ||= + node.instantiate(row, aliases.column_aliases(node)) do |m| + other.set_inverse_instance(m) + end + if node.reflection.collection? other.target.push(model) else other.target = model end - other.set_inverse_instance(model) model end end diff --git a/activerecord/lib/active_record/associations/join_dependency/join_part.rb b/activerecord/lib/active_record/associations/join_dependency/join_part.rb index 551087f822..61cec5403a 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_part.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_part.rb @@ -62,8 +62,8 @@ module ActiveRecord hash end - def instantiate(row, aliases) - base_klass.instantiate(extract_record(row, aliases)) + def instantiate(row, aliases, &block) + base_klass.instantiate(extract_record(row, aliases), &block) end end end diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 4bb627f399..07407700cd 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -62,7 +62,12 @@ module ActiveRecord private def associated_records_by_owner(preloader) - records = load_records + records = load_records do |record| + owner = owners_by_key[convert_key(record[association_key_name])] + association = owner.association(reflection.name) + association.set_inverse_instance(record) + end + owners.each_with_object({}) do |owner, result| result[owner] = records[convert_key(owner[owner_key_name])] || [] end @@ -79,6 +84,15 @@ module ActiveRecord @owner_keys end + def owners_by_key + unless defined?(@owners_by_key) + @owners_by_key = owners.each_with_object({}) do |owner, h| + h[convert_key(owner[owner_key_name])] = owner + end + end + @owners_by_key + end + def key_conversion_required? @key_conversion_required ||= association_key_type != owner_key_type end @@ -99,13 +113,13 @@ module ActiveRecord @model.type_for_attribute(owner_key_name.to_s).type end - def load_records + def load_records(&block) return {} if owner_keys.empty? # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) # Make several smaller queries if necessary or make one query if the adapter supports it slices = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) @preloaded_records = slices.flat_map do |slice| - records_for(slice) + records_for(slice).load(&block) end @preloaded_records.group_by do |record| convert_key(record[association_key_name]) diff --git a/activerecord/lib/active_record/associations/preloader/collection_association.rb b/activerecord/lib/active_record/associations/preloader/collection_association.rb index 24b8e01029..26690bf16d 100644 --- a/activerecord/lib/active_record/associations/preloader/collection_association.rb +++ b/activerecord/lib/active_record/associations/preloader/collection_association.rb @@ -9,7 +9,6 @@ module ActiveRecord association = owner.association(reflection.name) association.loaded! association.target.concat(records) - records.each { |record| association.set_inverse_instance(record) } end end end diff --git a/activerecord/lib/active_record/associations/preloader/singular_association.rb b/activerecord/lib/active_record/associations/preloader/singular_association.rb index 0888d383a6..5c5828262e 100644 --- a/activerecord/lib/active_record/associations/preloader/singular_association.rb +++ b/activerecord/lib/active_record/associations/preloader/singular_association.rb @@ -10,7 +10,6 @@ module ActiveRecord association = owner.association(reflection.name) association.target = record - association.set_inverse_instance(record) if record end end end diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 6e6620aad5..db84876b0a 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -457,7 +457,9 @@ module ActiveRecord # In addition, it will destroy the association if it was marked for destruction. def save_belongs_to_association(reflection) association = association_instance_get(reflection.name) - record = association && association.load_target + return unless association && association.loaded? && !association.stale_target? + + record = association.load_target if record && !record.destroyed? autosave = reflection.options[:autosave] diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index cb11cdefd9..2725c85446 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -366,6 +366,8 @@ module ActiveRecord self.class.define_attribute_methods + yield self if block_given? + _run_find_callbacks _run_initialize_callbacks @@ -541,15 +543,15 @@ module ActiveRecord private - # Under Ruby 1.9, Array#flatten will call #to_ary (recursively) on each of the elements - # of the array, and then rescues from the possible NoMethodError. If those elements are - # ActiveRecord::Base's, then this triggers the various method_missing's that we have, - # which significantly impacts upon performance. - # - # So we can avoid the method_missing hit by explicitly defining #to_ary as nil here. - # - # See also http://tenderlovemaking.com/2011/06/28/til-its-ok-to-return-nil-from-to_ary.html - def to_ary # :nodoc: + # +Array#flatten+ will call +#to_ary+ (recursively) on each of the elements of + # the array, and then rescues from the possible +NoMethodError+. If those elements are + # +ActiveRecord::Base+'s, then this triggers the various +method_missing+'s that we have, + # which significantly impacts upon performance. + # + # So we can avoid the +method_missing+ hit by explicitly defining +#to_ary+ as +nil+ here. + # + # See also http://tenderlovemaking.com/2011/06/28/til-its-ok-to-return-nil-from-to_ary.html + def to_ary nil end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index dc4af4f390..a04ef2e263 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -63,10 +63,10 @@ module ActiveRecord # # See <tt>ActiveRecord::Inheritance#discriminate_class_for_record</tt> to see # how this "single-table" inheritance mapping is implemented. - def instantiate(attributes, column_types = {}) + def instantiate(attributes, column_types = {}, &block) klass = discriminate_class_for_record(attributes) attributes = klass.attributes_builder.build_from_database(attributes, column_types) - klass.allocate.init_with("attributes" => attributes, "new_record" => false) + klass.allocate.init_with("attributes" => attributes, "new_record" => false, &block) end private @@ -178,7 +178,7 @@ module ActiveRecord # and #destroy returns +false+. # See ActiveRecord::Callbacks for further details. def destroy - raise ReadOnlyRecord, "#{self.class} is marked as readonly" if readonly? + _raise_readonly_record_error if readonly? destroy_associations self.class.connection.add_transaction_record(self) destroy_row if persisted? @@ -535,7 +535,7 @@ module ActiveRecord end def create_or_update(*args) - raise ReadOnlyRecord, "#{self.class} is marked as readonly" if readonly? + _raise_readonly_record_error if readonly? result = new_record? ? _create_record : _update_record(*args) result != false end @@ -577,5 +577,9 @@ module ActiveRecord def belongs_to_touch_method :touch end + + def _raise_readonly_record_error + raise ReadOnlyRecord, "#{self.class} is marked as readonly" + end end end diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index dd7d650207..36689f6559 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -35,7 +35,7 @@ module ActiveRecord # # Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date] # Post.find_by_sql ["SELECT body FROM comments WHERE author = :user_id OR approved_by = :user_id", { :user_id => user_id }] - def find_by_sql(sql, binds = [], preparable: nil) + def find_by_sql(sql, binds = [], preparable: nil, &block) result_set = connection.select_all(sanitize_sql(sql), "#{name} Load", binds, preparable: preparable) column_types = result_set.column_types.dup columns_hash.each_key { |k| column_types.delete k } @@ -47,7 +47,7 @@ module ActiveRecord } message_bus.instrument("instantiation.active_record", payload) do - result_set.map { |record| instantiate(record, column_types) } + result_set.map { |record| instantiate(record, column_types, &block) } end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 85b1ddf8db..6d571cf026 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -29,9 +29,7 @@ module ActiveRecord end def initialize_copy(other) - # This method is a hot spot, so for now, use Hash[] to dup the hash. - # https://bugs.ruby-lang.org/issues/7166 - @values = Hash[@values] + @values = @values.dup reset end @@ -564,8 +562,8 @@ module ActiveRecord # return value is the relation itself, not the records. # # Post.where(published: true).load # => #<ActiveRecord::Relation> - def load - exec_queries unless loaded? + def load(&block) + exec_queries(&block) unless loaded? self end @@ -661,7 +659,7 @@ module ActiveRecord end def values - Hash[@values] + @values.dup end def inspect @@ -680,8 +678,8 @@ module ActiveRecord private - def exec_queries - @records = eager_loading? ? find_with_associations.freeze : @klass.find_by_sql(arel, bound_attributes).freeze + def exec_queries(&block) + @records = eager_loading? ? find_with_associations.freeze : @klass.find_by_sql(arel, bound_attributes, &block).freeze preload = preload_values preload += includes_values unless eager_loading? diff --git a/activerecord/lib/active_record/statement_cache.rb b/activerecord/lib/active_record/statement_cache.rb index fd67032235..d19bb96ede 100644 --- a/activerecord/lib/active_record/statement_cache.rb +++ b/activerecord/lib/active_record/statement_cache.rb @@ -99,12 +99,12 @@ module ActiveRecord @bind_map = bind_map end - def execute(params, klass, connection) + def execute(params, klass, connection, &block) bind_values = bind_map.bind params sql = query_builder.sql_for bind_values, connection - klass.find_by_sql(sql, bind_values, preparable: true) + klass.find_by_sql(sql, bind_values, preparable: true, &block) end alias :call :execute end diff --git a/activerecord/lib/active_record/table_metadata.rb b/activerecord/lib/active_record/table_metadata.rb index a398e0cb7d..58184f3872 100644 --- a/activerecord/lib/active_record/table_metadata.rb +++ b/activerecord/lib/active_record/table_metadata.rb @@ -10,9 +10,7 @@ module ActiveRecord end def resolve_column_aliases(hash) - # This method is a hot spot, so for now, use Hash[] to dup the hash. - # https://bugs.ruby-lang.org/issues/7166 - new_hash = Hash[hash] + new_hash = hash.dup hash.each do |key, _| if (key.is_a?(Symbol)) && klass.attribute_alias?(key) new_hash[klass.attribute_alias(key)] = new_hash.delete(key) diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 3f42cb9b9d..2418346d1b 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -626,6 +626,12 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_queries(0) { tagging.super_tag } end + def test_dont_find_target_when_saving_foreign_key_after_stale_association_loaded + client = Client.create!(name: "Test client", firm_with_basic_id: Firm.find(1)) + client.firm_id = Firm.create!(name: "Test firm").id + assert_queries(1) { client.save! } + end + def test_field_name_same_as_foreign_key computer = Computer.find(1) assert_not_nil computer.developer, ":foreign key == attribute didn't lock up" # ' diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 0b23cea420..6fe6ee6783 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -494,6 +494,33 @@ class InverseHasManyTests < ActiveRecord::TestCase assert !man.persisted? end + + def test_inverse_instance_should_be_set_before_find_callbacks_are_run + reset_callbacks(Interest, :find) do + Interest.after_find { raise unless association(:man).loaded? && man.present? } + + assert Man.first.interests.reload.any? + assert Man.includes(:interests).first.interests.any? + assert Man.joins(:interests).includes(:interests).first.interests.any? + end + end + + def test_inverse_instance_should_be_set_before_initialize_callbacks_are_run + reset_callbacks(Interest, :initialize) do + Interest.after_initialize { raise unless association(:man).loaded? && man.present? } + + assert Man.first.interests.reload.any? + assert Man.includes(:interests).first.interests.any? + assert Man.joins(:interests).includes(:interests).first.interests.any? + end + end + + def reset_callbacks(target, type) + old_callbacks = target.send(:get_callbacks, type).deep_dup + yield + ensure + target.send(:set_callbacks, type, old_callbacks) if old_callbacks + end end class InverseBelongsToTests < ActiveRecord::TestCase diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 619a5f4f3f..7f7faca70d 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -215,7 +215,7 @@ class QueryCacheTest < ActiveRecord::TestCase def test_query_cached_even_when_types_are_reset Task.cache do # Warm the cache - task = Task.find(1) + Task.find(1) Task.connection.type_map.clear diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index f01ff23bcf..f5f4ba61b7 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -256,7 +256,7 @@ class Module # end # end # - # The target can be anything callable withing the object. E.g. instance + # The target can be anything callable within the object. E.g. instance # variables, methods, constants ant the likes. def delegate_missing_to(target) target = target.to_s diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index a8e98b704a..1c599b8851 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -65,16 +65,5 @@ module ActiveSupport alias :assert_not_predicate :refute_predicate alias :assert_not_respond_to :refute_respond_to alias :assert_not_same :refute_same - - # Assertion that the block should not raise an exception. - # - # Passes if evaluated code in the yielded block raises no exception. - # - # assert_nothing_raised do - # perform_service(param: 'no_exception') - # end - def assert_nothing_raised - yield - end end end diff --git a/activesupport/lib/active_support/testing/assertions.rb b/activesupport/lib/active_support/testing/assertions.rb index 7770aa8006..8c9ea2c0e8 100644 --- a/activesupport/lib/active_support/testing/assertions.rb +++ b/activesupport/lib/active_support/testing/assertions.rb @@ -19,6 +19,17 @@ module ActiveSupport assert !object, message end + # Assertion that the block should not raise an exception. + # + # Passes if evaluated code in the yielded block raises no exception. + # + # assert_nothing_raised do + # perform_service(param: 'no_exception') + # end + def assert_nothing_raised + yield + end + # Test numeric difference between the return value of an expression as a # result of what is evaluated in the yielded block. # @@ -136,7 +147,7 @@ module ActiveSupport retval = yield unless from == UNTRACKED - error = "#{expression.inspect} isn't #{from}" + error = "#{expression.inspect} isn't #{from.inspect}" error = "#{message}.\n#{error}" if message assert from === before, error end diff --git a/activesupport/lib/active_support/time_with_zone.rb b/activesupport/lib/active_support/time_with_zone.rb index 9d1147620c..74c4a39342 100644 --- a/activesupport/lib/active_support/time_with_zone.rb +++ b/activesupport/lib/active_support/time_with_zone.rb @@ -80,7 +80,7 @@ module ActiveSupport # Returns a <tt>Time</tt> instance of the simultaneous time in the system timezone. def localtime(utc_offset = nil) - utc.getlocal(utc_offset) + @localtime ||= utc.getlocal(utc_offset) end alias_method :getlocal, :localtime diff --git a/activesupport/test/test_case_test.rb b/activesupport/test/test_case_test.rb index d3d2ed8d7a..dfbb8aa41d 100644 --- a/activesupport/test/test_case_test.rb +++ b/activesupport/test/test_case_test.rb @@ -138,6 +138,15 @@ class AssertDifferenceTest < ActiveSupport::TestCase end end + def test_assert_changes_with_from_option_with_nil + error = assert_raises Minitest::Assertion do + assert_changes "@object.num", from: nil do + @object.increment + end + end + assert_equal "\"@object.num\" isn't nil", error.message + end + def test_assert_changes_with_to_option assert_changes "@object.num", to: 1 do @object.increment diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 7239105b29..c938edd8f7 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -1217,21 +1217,25 @@ NOTE. If you are running in a multi-threaded environment, there could be a chanc Custom configuration -------------------- -You can configure your own code through the Rails configuration object with custom configuration. It works like this: +You can configure your own code through the Rails configuration object with +custom configuration under either the `config.x` namespace, or `config` directly. +The key difference between these two is that you should be using `config.x` if you +are defining _nested_ configuration (ex: `config.x.nested.nested.hi`), and just +`config` for _single level_ configuration (ex: `config.hello`). ```ruby - config.payment_processing.schedule = :daily - config.payment_processing.retries = 3 + config.x.payment_processing.schedule = :daily + config.x.payment_processing.retries = 3 config.super_debugger = true ``` These configuration points are then available through the configuration object: ```ruby - Rails.configuration.payment_processing.schedule # => :daily - Rails.configuration.payment_processing.retries # => 3 - Rails.configuration.super_debugger # => true - Rails.configuration.super_debugger.not_set # => nil + Rails.configuration.x.payment_processing.schedule # => :daily + Rails.configuration.x.payment_processing.retries # => 3 + Rails.configuration.x.payment_processing.not_set # => nil + Rails.configuration.super_debugger # => true ``` You can also use `Rails::Application.config_for` to load whole configuration files: diff --git a/guides/source/contributing_to_ruby_on_rails.md b/guides/source/contributing_to_ruby_on_rails.md index ba8d085f79..332d8fc852 100644 --- a/guides/source/contributing_to_ruby_on_rails.md +++ b/guides/source/contributing_to_ruby_on_rails.md @@ -677,7 +677,7 @@ $ git format-patch master --stdout > ~/my_changes.patch Switch over to the target branch and apply your changes: ```bash -$ git checkout -b my_backport_branch 3-2-stable +$ git checkout -b my_backport_branch 4-2-stable $ git apply ~/my_changes.patch ``` diff --git a/guides/source/credits.html.erb b/guides/source/credits.html.erb index 511d76041b..fa81b27036 100644 --- a/guides/source/credits.html.erb +++ b/guides/source/credits.html.erb @@ -32,7 +32,7 @@ Ruby on Rails Guides: Credits <% end %> <%= author('Oscar Del Ben', 'oscardelben', 'oscardelben.jpg') do %> -Oscar Del Ben is a software engineer at <a href="http://www.wildfireapp.com/">Wildfire</a>. He's a regular open source contributor (<a href="https://github.com/oscardelben">GitHub account</a>) and tweets regularly at <a href="https://twitter.com/oscardelben">@oscardelben</a>. +Oscar Del Ben is a software engineer at <a href="http://www.businessinsider.com/google-buys-wildfire-2012-8">Wildfire</a>. He's a regular open source contributor (<a href="https://github.com/oscardelben">GitHub account</a>) and tweets regularly at <a href="https://twitter.com/oscardelben">@oscardelben</a>. <% end %> <%= author('Frederick Cheung', 'fcheung') do %> diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index 54421a328c..c4a8eacc57 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -182,6 +182,7 @@ of the files and folders that Rails created by default: |test/|Unit tests, fixtures, and other test apparatus. These are covered in [Testing Rails Applications](testing.html).| |tmp/|Temporary files (like cache and pid files).| |vendor/|A place for all third-party code. In a typical Rails application this includes vendored gems.| +|.gitignore|This file tells git which files (or patterns) it should ignore. See [Github - Ignoring files](https://help.github.com/articles/ignoring-files) for more info about ignoring files. Hello, Rails! ------------- diff --git a/guides/source/i18n.md b/guides/source/i18n.md index 1565008a69..887774961a 100644 --- a/guides/source/i18n.md +++ b/guides/source/i18n.md @@ -1123,7 +1123,7 @@ I18n support in Ruby on Rails was introduced in the release 2.2 and is still evo Thus we encourage everybody to experiment with new ideas and features in gems or other libraries and make them available to the community. (Don't forget to announce your work on our [mailing list](http://groups.google.com/group/rails-i18n)!) -If you find your own locale (language) missing from our [example translations data](https://github.com/svenfuchs/rails-i18n/tree/master/rails/locale) repository for Ruby on Rails, please [_fork_](https://github.com/guides/fork-a-project-and-submit-your-modifications) the repository, add your data and send a [pull request](https://github.com/guides/pull-requests). +If you find your own locale (language) missing from our [example translations data](https://github.com/svenfuchs/rails-i18n/tree/master/rails/locale) repository for Ruby on Rails, please [_fork_](https://github.com/guides/fork-a-project-and-submit-your-modifications) the repository, add your data and send a [pull request](https://help.github.com/articles/about-pull-requests/). Resources diff --git a/guides/source/security.md b/guides/source/security.md index 36eb61be8b..5c3d465220 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -287,7 +287,7 @@ Another class of security vulnerabilities surrounds the use of redirection and f WARNING: _Redirection in a web application is an underestimated cracker tool: Not only can the attacker forward the user to a trap web site, they may also create a self-contained attack._ -Whenever the user is allowed to pass (parts of) the URL for redirection, it is possibly vulnerable. The most obvious attack would be to redirect users to a fake web application which looks and feels exactly as the original one. This so-called phishing attack works by sending an unsuspicious link in an email to the users, injecting the link by XSS in the web application or putting the link into an external site. It is unsuspicious, because the link starts with the URL to the web application and the URL to the malicious site is hidden in the redirection parameter: http://www.example.com/site/redirect?to= www.attacker.com. Here is an example of a legacy action: +Whenever the user is allowed to pass (parts of) the URL for redirection, it is possibly vulnerable. The most obvious attack would be to redirect users to a fake web application which looks and feels exactly as the original one. This so-called phishing attack works by sending an unsuspicious link in an email to the users, injecting the link by XSS in the web application or putting the link into an external site. It is unsuspicious, because the link starts with the URL to the web application and the URL to the malicious site is hidden in the redirection parameter: http://www.example.com/site/redirect?to=www.attacker.com. Here is an example of a legacy action: ```ruby def legacy diff --git a/guides/source/testing.md b/guides/source/testing.md index 4ca3236ec1..8f9246dea2 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -239,8 +239,8 @@ Run options: --seed 1808 Error: ArticleTest#test_should_report_error: -NameError: undefined local variable or method `some_undefined_variable' for #<ArticleTest:0x007fee3aa71798> - test/models/article_test.rb:11:in `block in <class:ArticleTest>' +NameError: undefined local variable or method 'some_undefined_variable' for #<ArticleTest:0x007fee3aa71798> + test/models/article_test.rb:11:in 'block in <class:ArticleTest>' bin/rails test test/models/article_test.rb:9 @@ -312,7 +312,6 @@ specify to make your test failure messages clearer. | `assert_not_in_delta( expected, actual, [delta], [msg] )` | Ensures that the numbers `expected` and `actual` are not within `delta` of each other.| | `assert_throws( symbol, [msg] ) { block }` | Ensures that the given block throws the symbol.| | `assert_raises( exception1, exception2, ... ) { block }` | Ensures that the given block raises one of the given exceptions.| -| `assert_nothing_raised { block }` | Ensures that the given block doesn't raise any exceptions.| | `assert_instance_of( class, obj, [msg] )` | Ensures that `obj` is an instance of `class`.| | `assert_not_instance_of( class, obj, [msg] )` | Ensures that `obj` is not an instance of `class`.| | `assert_kind_of( class, obj, [msg] )` | Ensures that `obj` is an instance of `class` or is descending from it.| @@ -343,6 +342,7 @@ Rails adds some custom assertions of its own to the `minitest` framework: | --------------------------------------------------------------------------------- | ------- | | [`assert_difference(expressions, difference = 1, message = nil) {...}`](http://api.rubyonrails.org/classes/ActiveSupport/Testing/Assertions.html#method-i-assert_difference) | Test numeric difference between the return value of an expression as a result of what is evaluated in the yielded block.| | [`assert_no_difference(expressions, message = nil, &block)`](http://api.rubyonrails.org/classes/ActiveSupport/Testing/Assertions.html#method-i-assert_no_difference) | Asserts that the numeric result of evaluating an expression is not changed before and after invoking the passed in block.| +| [`assert_nothing_raised { block }`](http://api.rubyonrails.org/classes/ActiveSupport/TestCase.html#method-i-assert_nothing_raised) | Ensures that the given block doesn't raise any exceptions.| | [`assert_recognizes(expected_options, path, extras={}, message=nil)`](http://api.rubyonrails.org/classes/ActionDispatch/Assertions/RoutingAssertions.html#method-i-assert_recognizes) | Asserts that the routing of the given path was handled correctly and that the parsed options (given in the expected_options hash) match path. Basically, it asserts that Rails recognizes the route given by expected_options.| | [`assert_generates(expected_path, options, defaults={}, extras = {}, message=nil)`](http://api.rubyonrails.org/classes/ActionDispatch/Assertions/RoutingAssertions.html#method-i-assert_generates) | Asserts that the provided options can be used to generate the provided path. This is the inverse of assert_recognizes. The extras parameter is used to tell the request the names and values of additional request parameters that would be in a query string. The message parameter allows you to specify a custom error message for assertion failures.| | [`assert_response(type, message = nil)`](http://api.rubyonrails.org/classes/ActionDispatch/Assertions/ResponseAssertions.html#method-i-assert_response) | Asserts that the response comes with a specific status code. You can specify `:success` to indicate 200-299, `:redirect` to indicate 300-399, `:missing` to indicate 404, or `:error` to match the 500-599 range. You can also pass an explicit status number or its symbolic equivalent. For more information, see [full list of status codes](http://rubydoc.info/github/rack/rack/master/Rack/Utils#HTTP_STATUS_CODES-constant) and how their [mapping](http://rubydoc.info/github/rack/rack/master/Rack/Utils#SYMBOL_TO_STATUS_CODE-constant) works.| @@ -369,7 +369,7 @@ documentation](http://docs.seattlerb.org/minitest). We can run all of our tests at once by using the `bin/rails test` command. -Or we can run a single test by passing the `bin/rails test` command the filename containing the test cases. +Or we can run a single test file by passing the `bin/rails test` command the filename containing the test cases. ```bash $ bin/rails test test/models/article_test.rb @@ -763,16 +763,11 @@ The `get` method kicks off the web request and populates the results into the `@ * The action of the controller you are requesting. This can be in the form of a string or a route (i.e. `articles_url`). - * `params`: option with a hash of request parameters to pass into the action (e.g. query string parameters or article variables). - * `headers`: for setting the headers that will be passed with the request. - * `env`: for customizing the request environment as needed. - * `xhr`: whether the request is Ajax request or not. Can be set to true for marking the request as Ajax. - * `as`: for encoding the request with different content type. Supports `:json` by default. All of these keyword arguments are optional. @@ -865,8 +860,8 @@ 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_equal "index", @controller.action_name + assert_equal "application/x-www-form-urlencoded", @request.media_type assert_match "Articles", @response.body end end @@ -1056,7 +1051,7 @@ To avoid code duplication, you can add your own test helpers. Sign in helper can be a good example: ```ruby -#test/test_helper.rb +# test/test_helper.rb module SignInHelper def sign_in_as(user) @@ -1362,7 +1357,7 @@ Here is an example using the [`travel_to`](http://api.rubyonrails.org/classes/Ac user = User.create(name: 'Gaurish', activation_date: Date.new(2004, 10, 24)) assert_not user.applicable_for_gifting? travel_to Date.new(2004, 11, 24) do - assert_equal Date.new(2004, 10, 24), user.activation_date # inside the travel_to block `Date.current` is mocked + assert_equal Date.new(2004, 10, 24), user.activation_date # inside the `travel_to` block `Date.current` is mocked assert user.applicable_for_gifting? end assert_equal Date.new(2004, 10, 24), user.activation_date # The change was visible only inside the `travel_to` block. |