diff options
60 files changed, 456 insertions, 335 deletions
@@ -80,8 +80,7 @@ group :cable do gem "hiredis", require: false gem "redis", "~> 4.0", require: false - # For Redis 4.0 support. Unreleased 9cb81bf. - gem "redis-namespace", github: "resque/redis-namespace" + gem "redis-namespace" gem "websocket-client-simple", github: "matthewd/websocket-client-simple", branch: "close-race", require: false diff --git a/Gemfile.lock b/Gemfile.lock index cc3b0508c5..d0bfe589be 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -53,13 +53,6 @@ GIT tilt (>= 1.1, < 3) GIT - remote: https://github.com/resque/redis-namespace.git - revision: 1403f511f6ae1ec9a8f330298a4cacf73fb10afc - specs: - redis-namespace (1.5.3) - redis (>= 3.0.4) - -GIT remote: https://github.com/robin850/sdoc.git revision: 0e340352f3ab2f196c8a8743f83c2ee286e4f71c branch: upgrade @@ -409,6 +402,8 @@ GEM rdoc (5.1.0) redcarpet (3.2.3) redis (4.0.1) + redis-namespace (1.6.0) + redis (>= 3.0.4) representable (3.0.4) declarative (< 0.1.0) declarative-option (< 0.2.0) @@ -563,7 +558,7 @@ DEPENDENCIES rb-inotify! redcarpet (~> 3.2.3) redis (~> 4.0) - redis-namespace! + redis-namespace resque resque-scheduler! rubocop (>= 0.47) diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index 41a47f2c82..ec86b8bc47 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -9,7 +9,7 @@ module ActionDispatch # sub-hashes of the params hash to filter. Filtering only certain sub-keys # from a hash is possible by using the dot notation: 'credit_card.number'. # If a block is given, each key and value of the params hash and all - # sub-hashes is passed to it, the value or key can be replaced using + # sub-hashes is passed to it, where the value or the key can be replaced using # String#replace or similar method. # # env["action_dispatch.parameter_filter"] = [:password] @@ -48,7 +48,7 @@ module ActionDispatch @filtered_env ||= env_filter.filter(@env) end - # Reconstructed a path with all sensitive GET parameters replaced. + # Reconstructs a path with all sensitive GET parameters replaced. def filtered_path @filtered_path ||= query_string.empty? ? path : "#{path}?#{filtered_query_string}" end diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index f0f106c8ba..be455642de 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -98,7 +98,7 @@ class ACLogSubscriberTest < ActionController::TestCase @old_logger = ActionController::Base.logger - @cache_path = File.join Dir.tmpdir, Dir::Tmpname.make_tmpname("tmp", "cache") + @cache_path = Dir.mktmpdir(%w[tmp cache]) @controller.cache_store = :file_store, @cache_path ActionController::LogSubscriber.attach_to :action_controller end diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index e3227103d6..c700cb72ec 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,9 @@ +* Fix issues with `field_error_proc` wrapping `optgroup` and select divider `option`. + + Fixes #31088 + + *Matthias Neumayr* + * Remove deprecated Erubis ERB handler. *Rafael Mendonça França* diff --git a/actionview/Rakefile b/actionview/Rakefile index 9e5ef35334..8650f541b0 100644 --- a/actionview/Rakefile +++ b/actionview/Rakefile @@ -32,7 +32,7 @@ namespace :test do task :ujs do begin Dir.mkdir("log") - pid = spawn("bundle exec rackup test/ujs/config.ru -p 4567 -s puma > log/test.log 2>&1") + pid = spawn("bundle exec rackup test/ujs/config.ru -p 4567 -s puma > log/test.log 2>&1", pgroup: true) start_time = Time.now @@ -48,7 +48,7 @@ namespace :test do system("npm run lint && bundle exec ruby ../ci/qunit-selenium-runner.rb http://localhost:4567/") status = $?.exitstatus ensure - Process.kill("KILL", pid) if pid + Process.kill("KILL", -pid) if pid FileUtils.rm_rf("log") end diff --git a/actionview/lib/action_view/helpers/active_model_helper.rb b/actionview/lib/action_view/helpers/active_model_helper.rb index f1ef715710..e41a95d2ce 100644 --- a/actionview/lib/action_view/helpers/active_model_helper.rb +++ b/actionview/lib/action_view/helpers/active_model_helper.rb @@ -17,8 +17,8 @@ module ActionView end end - def content_tag(*) - error_wrapping(super) + def content_tag(type, options, *) + select_markup_helper?(type) ? super : error_wrapping(super) end def tag(type, options, *) @@ -43,6 +43,10 @@ module ActionView object.respond_to?(:errors) && object.errors.respond_to?(:[]) && error_message.present? end + def select_markup_helper?(type) + ["optgroup", "option"].include?(type) + end + def tag_generate_errors?(options) options["type"] != "hidden" end diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb index 80d53c2c6e..e86e18dd78 100644 --- a/actionview/lib/action_view/helpers/form_tag_helper.rb +++ b/actionview/lib/action_view/helpers/form_tag_helper.rb @@ -115,7 +115,7 @@ module ActionView # # <option>Write</option></select> # # select_tag "people", options_from_collection_for_select(@people, "id", "name"), include_blank: true - # # => <select id="people" name="people"><option value=""></option><option value="1">David</option></select> + # # => <select id="people" name="people"><option value="" label=" "></option><option value="1">David</option></select> # # select_tag "people", options_from_collection_for_select(@people, "id", "name"), include_blank: "All" # # => <select id="people" name="people"><option value="">All</option><option value="1">David</option></select> diff --git a/actionview/test/template/active_model_helper_test.rb b/actionview/test/template/active_model_helper_test.rb index a929d8dc3d..36afed6dd6 100644 --- a/actionview/test/template/active_model_helper_test.rb +++ b/actionview/test/template/active_model_helper_test.rb @@ -6,7 +6,7 @@ class ActiveModelHelperTest < ActionView::TestCase tests ActionView::Helpers::ActiveModelHelper silence_warnings do - Post = Struct.new(:author_name, :body, :updated_at) do + Post = Struct.new(:author_name, :body, :category, :published, :updated_at) do include ActiveModel::Conversion include ActiveModel::Validations @@ -22,10 +22,14 @@ class ActiveModelHelperTest < ActionView::TestCase @post = Post.new @post.errors[:author_name] << "can't be empty" @post.errors[:body] << "foo" + @post.errors[:category] << "must exist" + @post.errors[:published] << "must be accepted" @post.errors[:updated_at] << "bar" @post.author_name = "" @post.body = "Back to the hill and over it again!" + @post.category = "rails" + @post.published = false @post.updated_at = Date.new(2004, 6, 15) end @@ -56,6 +60,25 @@ class ActiveModelHelperTest < ActionView::TestCase assert_dom_equal(expected_dom, select("post", "author_name", [:a, :b], prompt: "Choose one...")) end + def test_select_grouped_options_with_errors + grouped_options = [ + ["A", [["A1"], ["A2"]]], + ["B", [["B1"], ["B2"]]], + ] + + assert_dom_equal( + %(<div class="field_with_errors"><select name="post[category]" id="post_category"><optgroup label="A"><option value="A1">A1</option>\n<option value="A2">A2</option></optgroup><optgroup label="B"><option value="B1">B1</option>\n<option value="B2">B2</option></optgroup></select></div>), + select("post", "category", grouped_options) + ) + end + + def test_collection_select_with_errors + assert_dom_equal( + %(<div class="field_with_errors"><select name="post[author_name]" id="post_author_name"><option value="a">a</option>\n<option value="b">b</option></select></div>), + collection_select("post", "author_name", [:a, :b], :to_s, :to_s) + ) + end + def test_date_select_with_errors assert_dom_equal( %(<div class="field_with_errors"><select id="post_updated_at_1i" name="post[updated_at(1i)]">\n<option selected="selected" value="2004">2004</option>\n<option value="2005">2005</option>\n</select>\n<input id="post_updated_at_2i" name="post[updated_at(2i)]" type="hidden" value="6" />\n<input id="post_updated_at_3i" name="post[updated_at(3i)]" type="hidden" value="1" />\n</div>), @@ -77,6 +100,55 @@ class ActiveModelHelperTest < ActionView::TestCase ) end + def test_label_with_errors + assert_dom_equal( + %(<div class="field_with_errors"><label for="post_body">Body</label></div>), + label("post", "body") + ) + end + + def test_check_box_with_errors + assert_dom_equal( + %(<input name="post[published]" type="hidden" value="0" /><div class="field_with_errors"><input type="checkbox" value="1" name="post[published]" id="post_published" /></div>), + check_box("post", "published") + ) + end + + def test_check_boxes_with_errors + assert_dom_equal( + %(<input name="post[published]" type="hidden" value="0" /><div class="field_with_errors"><input type="checkbox" value="1" name="post[published]" id="post_published" /></div><input name="post[published]" type="hidden" value="0" /><div class="field_with_errors"><input type="checkbox" value="1" name="post[published]" id="post_published" /></div>), + check_box("post", "published") + check_box("post", "published") + ) + end + + def test_radio_button_with_errors + assert_dom_equal( + %(<div class="field_with_errors"><input type="radio" value="rails" checked="checked" name="post[category]" id="post_category_rails" /></div>), + radio_button("post", "category", "rails") + ) + end + + def test_radio_buttons_with_errors + assert_dom_equal( + %(<div class="field_with_errors"><input type="radio" value="rails" checked="checked" name="post[category]" id="post_category_rails" /></div><div class="field_with_errors"><input type="radio" value="java" name="post[category]" id="post_category_java" /></div>), + radio_button("post", "category", "rails") + radio_button("post", "category", "java") + ) + end + + def test_collection_check_boxes_with_errors + assert_dom_equal( + %(<input type="hidden" name="post[category][]" value="" /><div class="field_with_errors"><input type="checkbox" value="ruby" name="post[category][]" id="post_category_ruby" /></div><label for="post_category_ruby">ruby</label><div class="field_with_errors"><input type="checkbox" value="java" name="post[category][]" id="post_category_java" /></div><label for="post_category_java">java</label>), + collection_check_boxes("post", "category", [:ruby, :java], :to_s, :to_s) + ) + end + + def test_collection_radio_buttons_with_errors + assert_dom_equal( + %(<input type="hidden" name="post[category]" value="" /><div class="field_with_errors"><input type="radio" value="ruby" name="post[category]" id="post_category_ruby" /></div><label for="post_category_ruby">ruby</label><div class="field_with_errors"><input type="radio" value="java" name="post[category]" id="post_category_java" /></div><label for="post_category_java">java</label>), + collection_radio_buttons("post", "category", [:ruby, :java], :to_s, :to_s) + ) + end + def test_hidden_field_does_not_render_errors assert_dom_equal( %(<input id="post_author_name" name="post[author_name]" type="hidden" value="" />), diff --git a/actionview/test/template/form_options_helper_test.rb b/actionview/test/template/form_options_helper_test.rb index a66db2f3dc..f0eed1e290 100644 --- a/actionview/test/template/form_options_helper_test.rb +++ b/actionview/test/template/form_options_helper_test.rb @@ -1251,6 +1251,25 @@ class FormOptionsHelperTest < ActionView::TestCase html end + def test_time_zone_select_with_priority_zones_and_errors + @firm = Firm.new("D") + @firm.extend ActiveModel::Validations + @firm.errors[:time_zone] << "invalid" + zones = [ ActiveSupport::TimeZone.new("A"), ActiveSupport::TimeZone.new("D") ] + html = time_zone_select("firm", "time_zone", zones) + assert_dom_equal "<div class=\"field_with_errors\">" \ + "<select id=\"firm_time_zone\" name=\"firm[time_zone]\">" \ + "<option value=\"A\">A</option>\n" \ + "<option value=\"D\" selected=\"selected\">D</option>" \ + "<option value=\"\" disabled=\"disabled\">-------------</option>\n" \ + "<option value=\"B\">B</option>\n" \ + "<option value=\"C\">C</option>\n" \ + "<option value=\"E\">E</option>" \ + "</select>" \ + "</div>", + html + end + def test_time_zone_select_with_default_time_zone_and_nil_value @firm = Firm.new() @firm.time_zone = nil diff --git a/activemodel/lib/active_model.rb b/activemodel/lib/active_model.rb index a97d7989be..faecd6b96f 100644 --- a/activemodel/lib/active_model.rb +++ b/activemodel/lib/active_model.rb @@ -30,6 +30,8 @@ require "active_model/version" module ActiveModel extend ActiveSupport::Autoload + autoload :Attribute + autoload :Attributes autoload :AttributeAssignment autoload :AttributeMethods autoload :BlockValidator, "active_model/validator" diff --git a/activerecord/lib/active_record/attribute.rb b/activemodel/lib/active_model/attribute.rb index fc474edc15..43130c37c5 100644 --- a/activerecord/lib/active_record/attribute.rb +++ b/activemodel/lib/active_model/attribute.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module ActiveRecord +module ActiveModel class Attribute # :nodoc: class << self def from_database(name, value, type) @@ -130,8 +130,6 @@ module ActiveRecord coder["value"] = value if defined?(@value) end - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. protected attr_reader :original_attribute @@ -237,6 +235,7 @@ module ActiveRecord self.class.new(name, type) end end + private_constant :FromDatabase, :FromUser, :Null, :Uninitialized, :WithCastValue end end diff --git a/activerecord/lib/active_record/attribute/user_provided_default.rb b/activemodel/lib/active_model/attribute/user_provided_default.rb index f746960fac..f274b687d4 100644 --- a/activerecord/lib/active_record/attribute/user_provided_default.rb +++ b/activemodel/lib/active_model/attribute/user_provided_default.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require "active_record/attribute" +require "active_model/attribute" -module ActiveRecord +module ActiveModel class Attribute # :nodoc: class UserProvidedDefault < FromUser # :nodoc: def initialize(name, value, type, database_default) @@ -22,8 +22,6 @@ module ActiveRecord self.class.new(name, user_provided_value, type, original_attribute) end - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. protected attr_reader :user_provided_value diff --git a/activerecord/lib/active_record/attribute_mutation_tracker.rb b/activemodel/lib/active_model/attribute_mutation_tracker.rb index 94bf641a5d..9072460124 100644 --- a/activerecord/lib/active_record/attribute_mutation_tracker.rb +++ b/activemodel/lib/active_model/attribute_mutation_tracker.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module ActiveRecord +module ActiveModel class AttributeMutationTracker # :nodoc: OPTION_NOT_GIVEN = Object.new @@ -107,5 +107,8 @@ module ActiveRecord def original_value(*) end + + def force_change(*) + end end end diff --git a/activerecord/lib/active_record/attribute_set.rb b/activemodel/lib/active_model/attribute_set.rb index 9785666e77..a892accbc6 100644 --- a/activerecord/lib/active_record/attribute_set.rb +++ b/activemodel/lib/active_model/attribute_set.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true -require "active_record/attribute_set/builder" -require "active_record/attribute_set/yaml_encoder" +require "active_model/attribute_set/builder" +require "active_model/attribute_set/yaml_encoder" -module ActiveRecord +module ActiveModel class AttributeSet # :nodoc: delegate :each_value, :fetch, to: :attributes diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activemodel/lib/active_model/attribute_set/builder.rb index 349cc7e403..f94f47370f 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activemodel/lib/active_model/attribute_set/builder.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require "active_record/attribute" +require "active_model/attribute" -module ActiveRecord +module ActiveModel class AttributeSet # :nodoc: class Builder # :nodoc: attr_reader :types, :always_initialized, :default @@ -92,8 +92,6 @@ module ActiveRecord @materialized = true end - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. protected attr_reader :types, :values, :additional_types, :delegate_hash, :default diff --git a/activerecord/lib/active_record/attribute_set/yaml_encoder.rb b/activemodel/lib/active_model/attribute_set/yaml_encoder.rb index 9254ce16ab..4ea945b956 100644 --- a/activerecord/lib/active_record/attribute_set/yaml_encoder.rb +++ b/activemodel/lib/active_model/attribute_set/yaml_encoder.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true -module ActiveRecord +module ActiveModel class AttributeSet # Attempts to do more intelligent YAML dumping of an - # ActiveRecord::AttributeSet to reduce the size of the resulting string + # ActiveModel::AttributeSet to reduce the size of the resulting string class YAMLEncoder # :nodoc: def initialize(default_types) @default_types = default_types @@ -33,8 +33,6 @@ module ActiveRecord end end - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. protected attr_reader :default_types diff --git a/activemodel/lib/active_model/attributes.rb b/activemodel/lib/active_model/attributes.rb index 3e34d3b83a..13cad87875 100644 --- a/activemodel/lib/active_model/attributes.rb +++ b/activemodel/lib/active_model/attributes.rb @@ -1,24 +1,30 @@ # frozen_string_literal: true +require "active_support/core_ext/object/deep_dup" require "active_model/type" +require "active_model/attribute_set" +require "active_model/attribute/user_provided_default" module ActiveModel module Attributes #:nodoc: extend ActiveSupport::Concern include ActiveModel::AttributeMethods - include ActiveModel::Dirty included do attribute_method_suffix "=" class_attribute :attribute_types, :_default_attributes, instance_accessor: false - self.attribute_types = {} - self._default_attributes = {} + self.attribute_types = Hash.new(Type.default_value) + self._default_attributes = AttributeSet.new({}) end module ClassMethods - def attribute(name, cast_type = Type::Value.new, **options) - self.attribute_types = attribute_types.merge(name.to_s => cast_type) - self._default_attributes = _default_attributes.merge(name.to_s => options[:default]) + def attribute(name, type = Type::Value.new, **options) + name = name.to_s + if type.is_a?(Symbol) + type = ActiveModel::Type.lookup(type, **options.except(:default)) + end + self.attribute_types = attribute_types.merge(name => type) + define_default_attribute(name, options.fetch(:default, NO_DEFAULT_PROVIDED), type) define_attribute_methods(name) end @@ -37,11 +43,29 @@ module ActiveModel undef_method :__temp__#{safe_name}= STR end + + NO_DEFAULT_PROVIDED = Object.new # :nodoc: + private_constant :NO_DEFAULT_PROVIDED + + def define_default_attribute(name, value, type) + self._default_attributes = _default_attributes.deep_dup + if value == NO_DEFAULT_PROVIDED + default_attribute = _default_attributes[name].with_type(type) + else + default_attribute = Attribute::UserProvidedDefault.new( + name, + value, + type, + _default_attributes.fetch(name.to_s) { nil }, + ) + end + _default_attributes[name] = default_attribute + end end def initialize(*) + @attributes = self.class._default_attributes.deep_dup super - clear_changes_information end private @@ -53,21 +77,17 @@ module ActiveModel attr_name.to_s end - cast_type = self.class.attribute_types[name] - - deserialized_value = ActiveModel::Type.lookup(cast_type).cast(value) - attribute_will_change!(name) unless deserialized_value == attribute(name) - instance_variable_set("@#{name}", deserialized_value) - deserialized_value + @attributes.write_from_user(attr_name.to_s, value) + value end - def attribute(name) - if instance_variable_defined?("@#{name}") - instance_variable_get("@#{name}") + def attribute(attr_name) + name = if self.class.attribute_alias?(attr_name) + self.class.attribute_alias(attr_name).to_s else - default = self.class._default_attributes[name] - default.respond_to?(:call) ? default.call : default + attr_name.to_s end + @attributes.fetch_value(name) end # Handle *= for method_missing. diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index 943db0ab52..ddd93e34a6 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -2,6 +2,7 @@ require "active_support/hash_with_indifferent_access" require "active_support/core_ext/object/duplicable" +require "active_model/attribute_mutation_tracker" module ActiveModel # == Active \Model \Dirty @@ -130,6 +131,24 @@ module ActiveModel attribute_method_affix prefix: "restore_", suffix: "!" end + def initialize_dup(other) # :nodoc: + super + if self.class.respond_to?(:_default_attributes) + @attributes = self.class._default_attributes.map do |attr| + attr.with_value_from_user(@attributes.fetch_value(attr.name)) + end + end + @mutations_from_database = nil + end + + def changes_applied # :nodoc: + @previously_changed = changes + @mutations_before_last_save = mutations_from_database + @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new + forget_attribute_assignments + @mutations_from_database = nil + end + # Returns +true+ if any of the attributes have unsaved changes, +false+ otherwise. # # person.changed? # => false @@ -148,6 +167,60 @@ module ActiveModel changed_attributes.keys end + # Handles <tt>*_changed?</tt> for +method_missing+. + def attribute_changed?(attr, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN) # :nodoc: + !!changes_include?(attr) && + (to == OPTION_NOT_GIVEN || to == _read_attribute(attr)) && + (from == OPTION_NOT_GIVEN || from == changed_attributes[attr]) + end + + # Handles <tt>*_was</tt> for +method_missing+. + def attribute_was(attr) # :nodoc: + attribute_changed?(attr) ? changed_attributes[attr] : _read_attribute(attr) + end + + # Handles <tt>*_previously_changed?</tt> for +method_missing+. + def attribute_previously_changed?(attr) #:nodoc: + previous_changes_include?(attr) + end + + # Restore all previous data of the provided attributes. + def restore_attributes(attributes = changed) + attributes.each { |attr| restore_attribute! attr } + end + + # Clears all dirty data: current changes and previous changes. + def clear_changes_information + @previously_changed = ActiveSupport::HashWithIndifferentAccess.new + @mutations_before_last_save = nil + @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new + forget_attribute_assignments + @mutations_from_database = nil + end + + def clear_attribute_changes(attr_names) + attributes_changed_by_setter.except!(*attr_names) + attr_names.each do |attr_name| + clear_attribute_change(attr_name) + end + end + + # Returns a hash of the attributes with unsaved changes indicating their original + # values like <tt>attr => original value</tt>. + # + # person.name # => "bob" + # person.name = 'robert' + # person.changed_attributes # => {"name" => "bob"} + def changed_attributes + # This should only be set by methods which will call changed_attributes + # multiple times when it is known that the computed value cannot change. + if defined?(@cached_changed_attributes) + @cached_changed_attributes + else + attributes_changed_by_setter.reverse_merge(mutations_from_database.changed_values).freeze + end + end + # Returns a hash of changed attributes indicating their original # and new values like <tt>attr => [original value, new value]</tt>. # @@ -155,7 +228,9 @@ module ActiveModel # person.name = 'bob' # person.changes # => { "name" => ["bill", "bob"] } def changes - ActiveSupport::HashWithIndifferentAccess[changed.map { |attr| [attr, attribute_change(attr)] }] + cache_changed_attributes do + ActiveSupport::HashWithIndifferentAccess[changed.map { |attr| [attr, attribute_change(attr)] }] + end end # Returns a hash of attributes that were changed before the model was saved. @@ -166,45 +241,51 @@ module ActiveModel # person.previous_changes # => {"name" => ["bob", "robert"]} def previous_changes @previously_changed ||= ActiveSupport::HashWithIndifferentAccess.new + @previously_changed.merge(mutations_before_last_save.changes) end - # Returns a hash of the attributes with unsaved changes indicating their original - # values like <tt>attr => original value</tt>. - # - # person.name # => "bob" - # person.name = 'robert' - # person.changed_attributes # => {"name" => "bob"} - def changed_attributes - @changed_attributes ||= ActiveSupport::HashWithIndifferentAccess.new + def attribute_changed_in_place?(attr_name) # :nodoc: + mutations_from_database.changed_in_place?(attr_name) end - # Handles <tt>*_changed?</tt> for +method_missing+. - def attribute_changed?(attr, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN) # :nodoc: - !!changes_include?(attr) && - (to == OPTION_NOT_GIVEN || to == _read_attribute(attr)) && - (from == OPTION_NOT_GIVEN || from == changed_attributes[attr]) - end + private + def clear_attribute_change(attr_name) + mutations_from_database.forget_change(attr_name) + end - # Handles <tt>*_was</tt> for +method_missing+. - def attribute_was(attr) # :nodoc: - attribute_changed?(attr) ? changed_attributes[attr] : _read_attribute(attr) - end + def mutations_from_database + unless defined?(@mutations_from_database) + @mutations_from_database = nil + end + @mutations_from_database ||= if @attributes + ActiveModel::AttributeMutationTracker.new(@attributes) + else + NullMutationTracker.instance + end + end - # Handles <tt>*_previously_changed?</tt> for +method_missing+. - def attribute_previously_changed?(attr) #:nodoc: - previous_changes_include?(attr) - end + def forget_attribute_assignments + @attributes = @attributes.map(&:forgetting_assignment) if @attributes + end - # Restore all previous data of the provided attributes. - def restore_attributes(attributes = changed) - attributes.each { |attr| restore_attribute! attr } - end + def mutations_before_last_save + @mutations_before_last_save ||= ActiveModel::NullMutationTracker.instance + end - private + def cache_changed_attributes + @cached_changed_attributes = changed_attributes + yield + ensure + clear_changed_attributes_cache + end + + def clear_changed_attributes_cache + remove_instance_variable(:@cached_changed_attributes) if defined?(@cached_changed_attributes) + end # Returns +true+ if attr_name is changed, +false+ otherwise. def changes_include?(attr_name) - attributes_changed_by_setter.include?(attr_name) + attributes_changed_by_setter.include?(attr_name) || mutations_from_database.changed?(attr_name) end alias attribute_changed_by_setter? changes_include? @@ -214,18 +295,6 @@ module ActiveModel previous_changes.include?(attr_name) end - # Removes current changes and makes them accessible through +previous_changes+. - def changes_applied # :doc: - @previously_changed = changes - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new - end - - # Clears all dirty data: current changes and previous changes. - def clear_changes_information # :doc: - @previously_changed = ActiveSupport::HashWithIndifferentAccess.new - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new - end - # Handles <tt>*_change</tt> for +method_missing+. def attribute_change(attr) [changed_attributes[attr], _read_attribute(attr)] if attribute_changed?(attr) @@ -238,15 +307,16 @@ module ActiveModel # Handles <tt>*_will_change!</tt> for +method_missing+. def attribute_will_change!(attr) - return if attribute_changed?(attr) + unless attribute_changed?(attr) + begin + value = _read_attribute(attr) + value = value.duplicable? ? value.clone : value + rescue TypeError, NoMethodError + end - begin - value = _read_attribute(attr) - value = value.duplicable? ? value.clone : value - rescue TypeError, NoMethodError + set_attribute_was(attr, value) end - - set_attribute_was(attr, value) + mutations_from_database.force_change(attr) end # Handles <tt>restore_*!</tt> for +method_missing+. @@ -257,18 +327,13 @@ module ActiveModel end end - # This is necessary because `changed_attributes` might be overridden in - # other implementations (e.g. in `ActiveRecord`) - alias_method :attributes_changed_by_setter, :changed_attributes # :nodoc: + def attributes_changed_by_setter + @attributes_changed_by_setter ||= ActiveSupport::HashWithIndifferentAccess.new + end # Force an attribute to have a particular "before" value def set_attribute_was(attr, old_value) attributes_changed_by_setter[attr] = old_value end - - # Remove changes information for the provided attributes. - def clear_attribute_changes(attributes) # :doc: - attributes_changed_by_setter.except!(*attributes) - end end end diff --git a/activemodel/lib/active_model/type.rb b/activemodel/lib/active_model/type.rb index b0ed67f28e..39324999c9 100644 --- a/activemodel/lib/active_model/type.rb +++ b/activemodel/lib/active_model/type.rb @@ -32,6 +32,10 @@ module ActiveModel def lookup(*args, **kwargs) # :nodoc: registry.lookup(*args, **kwargs) end + + def default_value # :nodoc: + @default_value ||= Value.new + end end register(:big_integer, Type::BigInteger) diff --git a/activerecord/test/cases/attribute_set_test.rb b/activemodel/test/cases/attribute_set_test.rb index 8be77ed88f..d50e6cfa7a 100644 --- a/activerecord/test/cases/attribute_set_test.rb +++ b/activemodel/test/cases/attribute_set_test.rb @@ -2,8 +2,8 @@ require "cases/helper" -module ActiveRecord - class AttributeSetTest < ActiveRecord::TestCase +module ActiveModel + class AttributeSetTest < ActiveModel::TestCase test "building a new set from raw attributes" do builder = AttributeSet::Builder.new(foo: Type::Integer.new, bar: Type::Float.new) attributes = builder.build_from_database(foo: "1.1", bar: "2.2") diff --git a/activerecord/test/cases/attribute_test.rb b/activemodel/test/cases/attribute_test.rb index 1731e7926e..14d86cef97 100644 --- a/activerecord/test/cases/attribute_test.rb +++ b/activemodel/test/cases/attribute_test.rb @@ -2,8 +2,8 @@ require "cases/helper" -module ActiveRecord - class AttributeTest < ActiveRecord::TestCase +module ActiveModel + class AttributeTest < ActiveModel::TestCase setup do @type = Minitest::Mock.new end diff --git a/activemodel/test/cases/attributes_dirty_test.rb b/activemodel/test/cases/attributes_dirty_test.rb index 26b0e85db3..83a86371e0 100644 --- a/activemodel/test/cases/attributes_dirty_test.rb +++ b/activemodel/test/cases/attributes_dirty_test.rb @@ -1,12 +1,12 @@ # frozen_string_literal: true require "cases/helper" -require "active_model/attributes" class AttributesDirtyTest < ActiveModel::TestCase class DirtyModel include ActiveModel::Model include ActiveModel::Attributes + include ActiveModel::Dirty attribute :name, :string attribute :color, :string attribute :size, :integer @@ -69,12 +69,10 @@ class AttributesDirtyTest < ActiveModel::TestCase end test "attribute mutation" do - @model.instance_variable_set("@name", "Yam".dup) + @model.name = "Yam" + @model.save assert !@model.name_changed? @model.name.replace("Hadad") - assert !@model.name_changed? - @model.name_will_change! - @model.name.replace("Baal") assert @model.name_changed? end @@ -190,4 +188,18 @@ class AttributesDirtyTest < ActiveModel::TestCase assert_equal "Dmitry", @model.name assert_equal "White", @model.color end + + test "changing the attribute reports a change only when the cast value changes" do + @model.size = "2.3" + @model.save + @model.size = "2.1" + + assert_equal false, @model.changed? + + @model.size = "5.1" + + assert_equal true, @model.changed? + assert_equal true, @model.size_changed? + assert_equal({ "size" => [2, 5] }, @model.changes) + end end diff --git a/activemodel/test/cases/attributes_test.rb b/activemodel/test/cases/attributes_test.rb index 064cba40e3..914aee1ac0 100644 --- a/activemodel/test/cases/attributes_test.rb +++ b/activemodel/test/cases/attributes_test.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "cases/helper" -require "active_model/attributes" module ActiveModel class AttributesTest < ActiveModel::TestCase @@ -13,7 +12,7 @@ module ActiveModel attribute :string_field, :string attribute :decimal_field, :decimal attribute :string_with_default, :string, default: "default string" - attribute :date_field, :string, default: -> { Date.new(2016, 1, 1) } + attribute :date_field, :date, default: -> { Date.new(2016, 1, 1) } attribute :boolean_field, :boolean end @@ -48,31 +47,6 @@ module ActiveModel assert_equal true, data.boolean_field end - test "dirty" do - data = ModelForAttributesTest.new( - integer_field: "2.3", - string_field: "Rails FTW", - decimal_field: "12.3", - boolean_field: "0" - ) - - assert_equal false, data.changed? - - data.integer_field = "2.1" - - assert_equal false, data.changed? - - data.string_with_default = "default string" - - assert_equal false, data.changed? - - data.integer_field = "5.1" - - assert_equal true, data.changed? - assert_equal true, data.integer_field_changed? - assert_equal({ "integer_field" => [2, 5] }, data.changes) - end - test "nonexistent attribute" do assert_raise ActiveModel::UnknownAttributeError do ModelForAttributesTest.new(nonexistent: "nonexistent") diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb index 2cd9e185e6..dfe041ff50 100644 --- a/activemodel/test/cases/dirty_test.rb +++ b/activemodel/test/cases/dirty_test.rb @@ -219,4 +219,8 @@ class DirtyTest < ActiveModel::TestCase assert_equal "Dmitry", @model.name assert_equal "White", @model.color end + + test "model can be dup-ed without Attributes" do + assert @model.dup + end end diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index bf6dfd46e1..0e036f05f5 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -27,14 +27,14 @@ require "active_support" require "active_support/rails" require "active_model" require "arel" +require "yaml" require "active_record/version" -require "active_record/attribute_set" +require "active_model/attribute_set" module ActiveRecord extend ActiveSupport::Autoload - autoload :Attribute autoload :Base autoload :Callbacks autoload :Core @@ -181,3 +181,7 @@ end ActiveSupport.on_load(:i18n) do I18n.load_path << File.expand_path("active_record/locale/en.yml", __dir__) end + +YAML.load_tags["!ruby/object:ActiveRecord::AttributeSet"] = "ActiveModel::AttributeSet" +YAML.load_tags["!ruby/object:ActiveRecord::Attribute::FromDatabase"] = "ActiveModel::Attribute::FromDatabase" +YAML.load_tags["!ruby/object:ActiveRecord::LazyAttributeHash"] = "ActiveModel::LazyAttributeHash" diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index e1754d4a19..5a93a89d0a 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -166,8 +166,6 @@ module ActiveRecord end class AlreadyLoaded # :nodoc: - attr_reader :owners, :reflection - def initialize(klass, owners, reflection, preload_scope) @owners = owners @reflection = reflection @@ -178,6 +176,9 @@ module ActiveRecord def preloaded_records owners.flat_map { |owner| owner.association(reflection.name).target } end + + protected + attr_reader :owners, :reflection end # Returns a class containing the logic needed to load preload the data diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index e77761692d..19c337dc39 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -4,7 +4,6 @@ module ActiveRecord module Associations class Preloader class Association #:nodoc: - attr_reader :owners, :reflection, :preload_scope, :model, :klass attr_reader :preloaded_records def initialize(klass, owners, reflection, preload_scope) @@ -28,6 +27,9 @@ module ActiveRecord end end + protected + attr_reader :owners, :reflection, :preload_scope, :model, :klass + private # The name of the key on the associated records def association_key_name diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index b1813ba66b..762275fbad 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -4,14 +4,6 @@ module ActiveRecord module Associations class Preloader module ThroughAssociation #:nodoc: - def through_reflection - reflection.through_reflection - end - - def source_reflection - reflection.source_reflection - end - def run(preloader) already_loaded = owners.first.association(through_reflection.name).loaded? through_scope = through_scope() @@ -48,6 +40,13 @@ module ActiveRecord end private + def through_reflection + reflection.through_reflection + end + + def source_reflection + reflection.source_reflection + end def preload_index @preload_index ||= @preloaded_records.each_with_object({}).with_index do |(id, result), index| diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 79110d04f4..3de6fe566d 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "active_support/core_ext/module/attribute_accessors" -require "active_record/attribute_mutation_tracker" module ActiveRecord module AttributeMethods @@ -33,65 +32,13 @@ module ActiveRecord # <tt>reload</tt> the record and clears changed attributes. def reload(*) super.tap do + @previously_changed = ActiveSupport::HashWithIndifferentAccess.new @mutations_before_last_save = nil + @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new @mutations_from_database = nil - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new end end - def initialize_dup(other) # :nodoc: - super - @attributes = self.class._default_attributes.map do |attr| - attr.with_value_from_user(@attributes.fetch_value(attr.name)) - end - @mutations_from_database = nil - end - - def changes_applied # :nodoc: - @mutations_before_last_save = mutations_from_database - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new - forget_attribute_assignments - @mutations_from_database = nil - end - - def clear_changes_information # :nodoc: - @mutations_before_last_save = nil - @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new - forget_attribute_assignments - @mutations_from_database = nil - end - - def clear_attribute_changes(attr_names) # :nodoc: - super - attr_names.each do |attr_name| - clear_attribute_change(attr_name) - end - end - - def changed_attributes # :nodoc: - # This should only be set by methods which will call changed_attributes - # multiple times when it is known that the computed value cannot change. - if defined?(@cached_changed_attributes) - @cached_changed_attributes - else - super.reverse_merge(mutations_from_database.changed_values).freeze - end - end - - def changes # :nodoc: - cache_changed_attributes do - super - end - end - - def previous_changes # :nodoc: - mutations_before_last_save.changes - end - - def attribute_changed_in_place?(attr_name) # :nodoc: - mutations_from_database.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 <tt>saved_change_to_attribute?("name")</tt>. # Behaves similarly to +attribute_changed?+. This method is useful in @@ -182,26 +129,6 @@ module ActiveRecord result end - def mutations_from_database - unless defined?(@mutations_from_database) - @mutations_from_database = nil - end - @mutations_from_database ||= AttributeMutationTracker.new(@attributes) - end - - def changes_include?(attr_name) - super || mutations_from_database.changed?(attr_name) - end - - def clear_attribute_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(*) partial_writes? ? super(keys_for_partial_write) : super end @@ -213,25 +140,6 @@ module ActiveRecord def keys_for_partial_write changed_attribute_names_to_save & self.class.column_names end - - def forget_attribute_assignments - @attributes = @attributes.map(&:forgetting_assignment) - end - - def mutations_before_last_save - @mutations_before_last_save ||= NullMutationTracker.instance - end - - def cache_changed_attributes - @cached_changed_attributes = changed_attributes - yield - ensure - clear_changed_attributes_cache - end - - def clear_changed_attributes_cache - remove_instance_variable(:@cached_changed_attributes) if defined?(@cached_changed_attributes) - end end end end diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb index 9224d58928..0b7c9398a8 100644 --- a/activerecord/lib/active_record/attributes.rb +++ b/activerecord/lib/active_record/attributes.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "active_record/attribute/user_provided_default" +require "active_model/attribute/user_provided_default" module ActiveRecord # See ActiveRecord::Attributes::ClassMethods for documentation @@ -250,14 +250,14 @@ module ActiveRecord if value == NO_DEFAULT_PROVIDED default_attribute = _default_attributes[name].with_type(type) elsif from_user - default_attribute = Attribute::UserProvidedDefault.new( + default_attribute = ActiveModel::Attribute::UserProvidedDefault.new( name, value, type, _default_attributes.fetch(name.to_s) { nil }, ) else - default_attribute = Attribute.from_database(name, value, type) + default_attribute = ActiveModel::Attribute.from_database(name, value, type) end _default_attributes[name] = default_attribute end diff --git a/activerecord/lib/active_record/legacy_yaml_adapter.rb b/activerecord/lib/active_record/legacy_yaml_adapter.rb index 23644aab8f..ffa095dd94 100644 --- a/activerecord/lib/active_record/legacy_yaml_adapter.rb +++ b/activerecord/lib/active_record/legacy_yaml_adapter.rb @@ -8,7 +8,7 @@ module ActiveRecord case coder["active_record_yaml_version"] when 1, 2 then coder else - if coder["attributes"].is_a?(AttributeSet) + if coder["attributes"].is_a?(ActiveModel::AttributeSet) Rails420.convert(klass, coder) else Rails41.convert(klass, coder) diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index bed9400f51..12ee4a4137 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -323,7 +323,7 @@ module ActiveRecord end def attributes_builder # :nodoc: - @attributes_builder ||= AttributeSet::Builder.new(attribute_types, primary_key) do |name| + @attributes_builder ||= ActiveModel::AttributeSet::Builder.new(attribute_types, primary_key) do |name| unless columns_hash.key?(name) _default_attributes[name].dup end @@ -346,7 +346,7 @@ module ActiveRecord end def yaml_encoder # :nodoc: - @yaml_encoder ||= AttributeSet::YAMLEncoder.new(attribute_types) + @yaml_encoder ||= ActiveModel::AttributeSet::YAMLEncoder.new(attribute_types) end # Returns the type of the attribute with the given name, after applying @@ -376,7 +376,7 @@ module ActiveRecord end def _default_attributes # :nodoc: - @default_attributes ||= AttributeSet.new({}) + @default_attributes ||= ActiveModel::AttributeSet.new({}) end # Returns an array of column names as strings. diff --git a/activerecord/lib/active_record/relation/query_attribute.rb b/activerecord/lib/active_record/relation/query_attribute.rb index fad08e2613..3532f28858 100644 --- a/activerecord/lib/active_record/relation/query_attribute.rb +++ b/activerecord/lib/active_record/relation/query_attribute.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true -require "active_record/attribute" +require "active_model/attribute" module ActiveRecord class Relation - class QueryAttribute < Attribute # :nodoc: + class QueryAttribute < ActiveModel::Attribute # :nodoc: def type_cast(value) value end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 897ff5c8af..9c5be4ad9b 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -930,7 +930,7 @@ module ActiveRecord arel.where(where_clause.ast) unless where_clause.empty? arel.having(having_clause.ast) unless having_clause.empty? if limit_value - limit_attribute = Attribute.with_cast_value( + limit_attribute = ActiveModel::Attribute.with_cast_value( "LIMIT".freeze, connection.sanitize_limit(limit_value), Type.default_value, @@ -938,7 +938,7 @@ module ActiveRecord arel.take(Arel::Nodes::BindParam.new(limit_attribute)) end if offset_value - offset_attribute = Attribute.with_cast_value( + offset_attribute = ActiveModel::Attribute.with_cast_value( "OFFSET".freeze, offset_value.to_i, Type.default_value, diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 6791d50940..e857180bd1 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -611,14 +611,12 @@ unless in_memory_db? end end - if current_adapter?(:PostgreSQLAdapter, :OracleAdapter) - def test_no_locks_no_wait - first, second = duel { Person.find 1 } - assert first.end > second.end - end - - private + def test_no_locks_no_wait + first, second = duel { Person.find 1 } + assert first.end > second.end + end + private def duel(zzz = 5) t0, t1, t2, t3 = nil, nil, nil, nil @@ -646,6 +644,5 @@ unless in_memory_db? assert t3 > t2 [t0.to_f..t1.to_f, t2.to_f..t3.to_f] end - end end end diff --git a/activerecord/test/cases/transaction_isolation_test.rb b/activerecord/test/cases/transaction_isolation_test.rb index b1ebccdcc3..eaafd13360 100644 --- a/activerecord/test/cases/transaction_isolation_test.rb +++ b/activerecord/test/cases/transaction_isolation_test.rb @@ -15,9 +15,7 @@ unless ActiveRecord::Base.connection.supports_transaction_isolation? end end end -end - -if ActiveRecord::Base.connection.supports_transaction_isolation? +else class TransactionIsolationTest < ActiveRecord::TestCase self.use_transactional_tests = false diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 7fd125ab74..5c8ae4d3cb 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -954,27 +954,25 @@ class TransactionsWithTransactionalFixturesTest < ActiveRecord::TestCase end end if Topic.connection.supports_savepoints? -if current_adapter?(:PostgreSQLAdapter) +if ActiveRecord::Base.connection.supports_transaction_isolation? class ConcurrentTransactionTest < TransactionTest # This will cause transactions to overlap and fail unless they are performed on # separate database connections. - unless in_memory_db? - def test_transaction_per_thread - threads = 3.times.map do - Thread.new do - Topic.transaction do - topic = Topic.find(1) - topic.approved = !topic.approved? - assert topic.save! - topic.approved = !topic.approved? - assert topic.save! - end - Topic.connection.close + def test_transaction_per_thread + threads = 3.times.map do + Thread.new do + Topic.transaction do + topic = Topic.find(1) + topic.approved = !topic.approved? + assert topic.save! + topic.approved = !topic.approved? + assert topic.save! end + Topic.connection.close end - - threads.each(&:join) end + + threads.each(&:join) end # Test for dirty reads among simultaneous transactions. diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index 958b172efb..6957119780 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -85,7 +85,7 @@ module ActiveStorage end # Reads the object for the given key in chunks, yielding each to the block. - def stream(key, &block) + def stream(key) object = object_for(key) chunk_size = 5.megabytes diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 9c27c68919..780c887b49 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,16 @@ +* Allow `Range#include?` on TWZ ranges + + In #11474 we prevented TWZ ranges being iterated over which matched + Ruby's handling of Time ranges and as a consequence `include?` + stopped working with both Time ranges and TWZ ranges. However in + ruby/ruby@b061634 support was added for `include?` to use `cover?` + for 'linear' objects. Since we have no way of making Ruby consider + TWZ instances as 'linear' we have to override `Range#include?`. + + Fixes #30799. + + *Andrew White* + * Fix acronym support in `humanize` Acronym inflections are stored with lowercase keys in the hash but diff --git a/activesupport/lib/active_support/core_ext/range.rb b/activesupport/lib/active_support/core_ext/range.rb index 51ae0ddd21..4074e91d17 100644 --- a/activesupport/lib/active_support/core_ext/range.rb +++ b/activesupport/lib/active_support/core_ext/range.rb @@ -2,5 +2,6 @@ require "active_support/core_ext/range/conversions" require "active_support/core_ext/range/include_range" +require "active_support/core_ext/range/include_time_with_zone" require "active_support/core_ext/range/overlaps" require "active_support/core_ext/range/each" diff --git a/activesupport/lib/active_support/core_ext/range/each.rb b/activesupport/lib/active_support/core_ext/range/each.rb index cdff6393d7..2f22cd0e92 100644 --- a/activesupport/lib/active_support/core_ext/range/each.rb +++ b/activesupport/lib/active_support/core_ext/range/each.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/time_with_zone" + module ActiveSupport module EachTimeWithZone #:nodoc: def each(&block) @@ -15,7 +17,7 @@ module ActiveSupport private def ensure_iteration_allowed - raise TypeError, "can't iterate from #{first.class}" if first.is_a?(Time) + raise TypeError, "can't iterate from #{first.class}" if first.is_a?(TimeWithZone) end end end diff --git a/activesupport/lib/active_support/core_ext/range/include_time_with_zone.rb b/activesupport/lib/active_support/core_ext/range/include_time_with_zone.rb new file mode 100644 index 0000000000..5f80acf68e --- /dev/null +++ b/activesupport/lib/active_support/core_ext/range/include_time_with_zone.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require "active_support/time_with_zone" + +module ActiveSupport + module IncludeTimeWithZone #:nodoc: + # Extends the default Range#include? to support ActiveSupport::TimeWithZone. + # + # (1.hour.ago..1.hour.from_now).include?(Time.current) # => true + # + def include?(value) + if first.is_a?(TimeWithZone) + cover?(value) + elsif last.is_a?(TimeWithZone) + cover?(value) + else + super + end + end + end +end + +Range.prepend(ActiveSupport::IncludeTimeWithZone) diff --git a/activesupport/lib/active_support/testing/assertions.rb b/activesupport/lib/active_support/testing/assertions.rb index 46eed73d24..b24aa36ede 100644 --- a/activesupport/lib/active_support/testing/assertions.rb +++ b/activesupport/lib/active_support/testing/assertions.rb @@ -159,7 +159,7 @@ module ActiveSupport if to == UNTRACKED error = "#{expression.inspect} didn't change" error = "#{message}.\n#{error}" if message - assert_not_equal before, after, error + assert before != after, error else error = "#{expression.inspect} didn't change to #{to}" error = "#{message}.\n#{error}" if message @@ -190,12 +190,7 @@ module ActiveSupport error = "#{expression.inspect} did change to #{after}" error = "#{message}.\n#{error}" if message - - if before.nil? - assert_nil after, error - else - assert_equal before, after, error - end + assert before == after, error retval end diff --git a/activesupport/test/core_ext/range_ext_test.rb b/activesupport/test/core_ext/range_ext_test.rb index 0467123e55..049fac8fd4 100644 --- a/activesupport/test/core_ext/range_ext_test.rb +++ b/activesupport/test/core_ext/range_ext_test.rb @@ -121,9 +121,12 @@ class RangeTest < ActiveSupport::TestCase def test_include_on_time_with_zone twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone["Eastern Time (US & Canada)"] , Time.utc(2006, 11, 28, 10, 30)) - assert_raises TypeError do - ((twz - 1.hour)..twz).include?(twz) - end + assert ((twz - 1.hour)..twz).include?(twz) + end + + def test_case_equals_on_time_with_zone + twz = ActiveSupport::TimeWithZone.new(nil, ActiveSupport::TimeZone["Eastern Time (US & Canada)"] , Time.utc(2006, 11, 28, 10, 30)) + assert ((twz - 1.hour)..twz) === twz end def test_date_time_with_each diff --git a/activesupport/test/test_case_test.rb b/activesupport/test/test_case_test.rb index 9bc9183668..84e4953fe2 100644 --- a/activesupport/test/test_case_test.rb +++ b/activesupport/test/test_case_test.rb @@ -179,6 +179,7 @@ class AssertDifferenceTest < ActiveSupport::TestCase end def test_assert_changes_works_with_any_object + # Silences: instance variable @new_object not initialized. retval = silence_warnings do assert_changes :@new_object, from: nil, to: 42 do @new_object = 42 @@ -201,7 +202,7 @@ class AssertDifferenceTest < ActiveSupport::TestCase def test_assert_changes_with_to_and_case_operator token = nil - assert_changes -> { token }, to: /\w{32}/ do + assert_changes -> { token }, to: /\w{32}/ do token = SecureRandom.hex end end @@ -236,7 +237,7 @@ class AssertDifferenceTest < ActiveSupport::TestCase end end - assert_equal "@object.num should not change.\n\"@object.num\" did change to 1.\nExpected: 0\n Actual: 1", error.message + assert_equal "@object.num should not change.\n\"@object.num\" did change to 1", error.message end end diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index 2d265818f7..5592e8d78e 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -218,6 +218,7 @@ module Rails rails.delete("app") rails.delete("plugin") rails.delete("encrypted_secrets") + rails.delete("credentials") hidden_namespaces.each { |n| groups.delete(n.to_s) } diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 59ca4c849f..bdeddff645 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -461,7 +461,11 @@ module Rails def run_active_storage unless skip_active_storage? - rails_command "active_storage:install", capture: options[:quiet] + if bundle_install? + rails_command "active_storage:install", capture: options[:quiet] + else + log("Active Storage installation was skipped. Please run `bin/rails active_storage:install` to install Active Storage files.") + end end end diff --git a/railties/test/application/bin_setup_test.rb b/railties/test/application/bin_setup_test.rb index 9fac0435e4..54934dbe24 100644 --- a/railties/test/application/bin_setup_test.rb +++ b/railties/test/application/bin_setup_test.rb @@ -16,8 +16,6 @@ module ApplicationTests def test_bin_setup Dir.chdir(app_path) do - FileUtils.rm_rf("db/migrate") - app_file "db/schema.rb", <<-RUBY ActiveRecord::Schema.define(version: 20140423102712) do create_table(:articles) {} @@ -39,8 +37,6 @@ module ApplicationTests def test_bin_setup_output Dir.chdir(app_path) do - FileUtils.rm_rf("db/migrate") - app_file "db/schema.rb", "" output = `bin/setup 2>&1` diff --git a/railties/test/application/generators_test.rb b/railties/test/application/generators_test.rb index 47c815d221..e5e557d204 100644 --- a/railties/test/application/generators_test.rb +++ b/railties/test/application/generators_test.rb @@ -188,10 +188,11 @@ module ApplicationTests Rails::Command.send(:remove_const, "APP_PATH") end - test "help does not show hidden namespaces" do + test "help does not show hidden namespaces and hidden commands" do FileUtils.cd(rails_root) do output = rails("generate", "--help") assert_no_match "active_record:migration", output + assert_no_match "credentials", output output = rails("destroy", "--help") assert_no_match "active_record:migration", output diff --git a/railties/test/application/rackup_test.rb b/railties/test/application/rackup_test.rb index 1dcc2826f0..383f18a7da 100644 --- a/railties/test/application/rackup_test.rb +++ b/railties/test/application/rackup_test.rb @@ -26,7 +26,6 @@ module ApplicationTests test "config.ru can be racked up" do Dir.chdir app_path do - FileUtils.rm_rf("db/migrate") @app = rackup assert_welcome get("/") end diff --git a/railties/test/application/rake/dbs_test.rb b/railties/test/application/rake/dbs_test.rb index 009f2887c9..fd22477539 100644 --- a/railties/test/application/rake/dbs_test.rb +++ b/railties/test/application/rake/dbs_test.rb @@ -287,8 +287,6 @@ module ApplicationTests ENV.delete "RAILS_ENV" ENV.delete "RACK_ENV" - Dir.chdir(app_path) { FileUtils.rm_rf("db/migrate") } - app_file "db/schema.rb", <<-RUBY ActiveRecord::Schema.define(version: "1") do create_table :users do |t| @@ -310,8 +308,6 @@ module ApplicationTests end test "db:setup sets ar_internal_metadata" do - Dir.chdir(app_path) { FileUtils.rm_rf("db/migrate") } - app_file "db/schema.rb", "" rails "db:setup" diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index 30bd283b48..e92a0466dd 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -11,7 +11,6 @@ module ApplicationTests def setup build_app create_schema - remove_migrations end def teardown @@ -728,10 +727,6 @@ module ApplicationTests app_file "db/schema.rb", "" end - def remove_migrations - Dir.chdir(app_path) { FileUtils.rm_rf("db/migrate") } - end - def create_test_file(path = :unit, name = "test", pass: true) app_file "test/#{path}/#{name}_test.rb", <<-RUBY require 'test_helper' diff --git a/railties/test/application/test_test.rb b/railties/test/application/test_test.rb index 50238ed682..0a51e98656 100644 --- a/railties/test/application/test_test.rb +++ b/railties/test/application/test_test.rb @@ -8,7 +8,6 @@ module ApplicationTests def setup build_app - remove_migrations end def teardown @@ -321,10 +320,6 @@ Expected: ["id", "name"] end private - def remove_migrations - Dir.chdir(app_path) { FileUtils.rm_rf("db/migrate") } - end - def assert_unsuccessful_run(name, message) result = run_test_file(name) assert_not_equal 0, $?.to_i diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index c5a59edab2..78962ee30b 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -68,7 +68,6 @@ DEFAULT_APP_FILES = %w( config/spring.rb config/storage.yml db - db/migrate db/seeds.rb lib lib/tasks @@ -304,6 +303,21 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_file "Gemfile", /^# gem 'mini_magick'/ end + def test_active_storage_install + command_check = -> command, _ do + @binstub_called ||= 0 + case command + when "active_storage:install" + @binstub_called += 1 + assert_equal 1, @binstub_called, "active_storage:install expected to be called once, but was called #{@install_called} times." + end + end + + generator.stub :rails_command, command_check do + quietly { generator.invoke_all } + end + end + def test_app_update_does_not_generate_active_storage_contents_when_skip_active_storage_is_given app_root = File.join(destination_root, "myapp") run_generator [app_root, "--skip-active-storage"] @@ -409,7 +423,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_config_jdbcmysql_database - run_generator([destination_root, "-d", "jdbcmysql", "--skip-active-storage"]) + run_generator([destination_root, "-d", "jdbcmysql"]) assert_file "config/database.yml", /mysql/ assert_gem "activerecord-jdbcmysql-adapter" end @@ -427,7 +441,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_config_jdbc_database - run_generator([destination_root, "-d", "jdbc", "--skip-active-storage"]) + run_generator([destination_root, "-d", "jdbc"]) assert_file "config/database.yml", /jdbc/ assert_file "config/database.yml", /mssql/ assert_gem "activerecord-jdbc-adapter" diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 06d223e26d..49256883d6 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -230,7 +230,7 @@ class PluginGeneratorTest < Rails::Generators::TestCase end def test_ensure_that_tests_work - run_generator [destination_root, "--skip-active-storage"] + run_generator FileUtils.cd destination_root quietly { system "bundle install" } assert_match(/1 runs, 1 assertions, 0 failures, 0 errors/, `bin/test 2>&1`) diff --git a/railties/test/generators/plugin_test_runner_test.rb b/railties/test/generators/plugin_test_runner_test.rb index 48a3bcadcd..89c3f1e496 100644 --- a/railties/test/generators/plugin_test_runner_test.rb +++ b/railties/test/generators/plugin_test_runner_test.rb @@ -7,7 +7,7 @@ class PluginTestRunnerTest < ActiveSupport::TestCase def setup @destination_root = Dir.mktmpdir("bukkits") - Dir.chdir(@destination_root) { `bundle exec rails plugin new bukkits --skip-bundle --skip-active-storage` } + Dir.chdir(@destination_root) { `bundle exec rails plugin new bukkits --skip-bundle` } plugin_file "test/dummy/db/schema.rb", "" end diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index ed09847443..6b746f3fed 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -63,7 +63,7 @@ module SharedGeneratorTests end def test_shebang_is_added_to_rails_file - run_generator [destination_root, "--ruby", "foo/bar/baz", "--full", "--skip-active-storage"] + run_generator [destination_root, "--ruby", "foo/bar/baz", "--full"] assert_file "bin/rails", /#!foo\/bar\/baz/ end @@ -222,7 +222,6 @@ module SharedGeneratorTests end assert_file "#{application_path}/config/storage.yml" - assert_directory "#{application_path}/db/migrate" assert_directory "#{application_path}/storage" assert_directory "#{application_path}/tmp/storage" diff --git a/railties/test/generators/test_runner_in_engine_test.rb b/railties/test/generators/test_runner_in_engine_test.rb index 2acd96ecd4..0e15b5e388 100644 --- a/railties/test/generators/test_runner_in_engine_test.rb +++ b/railties/test/generators/test_runner_in_engine_test.rb @@ -7,7 +7,7 @@ class TestRunnerInEngineTest < ActiveSupport::TestCase def setup @destination_root = Dir.mktmpdir("bukkits") - Dir.chdir(@destination_root) { `bundle exec rails plugin new bukkits --full --skip-bundle --skip-active-storage` } + Dir.chdir(@destination_root) { `bundle exec rails plugin new bukkits --full --skip-bundle` } plugin_file "test/dummy/db/schema.rb", "" end diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index 132b3b3a6e..fc710feb63 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -90,6 +90,8 @@ module RailtiesTest boot_rails Dir.chdir(app_path) do + # Install Active Storage migration file first so as not to affect test. + `bundle exec rake active_storage:install` output = `bundle exec rake bukkits:install:migrations` ["CreateUsers", "AddLastNameToUsers", "CreateSessions"].each do |migration_name| @@ -175,6 +177,8 @@ module RailtiesTest boot_rails Dir.chdir(app_path) do + # Install Active Storage migration file first so as not to affect test. + `bundle exec rake active_storage:install` output = `bundle exec rake railties:install:migrations`.split("\n") assert_match(/Copied migration \d+_create_users\.core_engine\.rb from core_engine/, output.first) |