diff options
54 files changed, 477 insertions, 137 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index 62cab7261b..7617942267 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -63,6 +63,7 @@ PATH activesupport (= 6.0.0.beta3) activestorage (6.0.0.beta3) actionpack (= 6.0.0.beta3) + activejob (= 6.0.0.beta3) activerecord (= 6.0.0.beta3) marcel (~> 0.3.1) activesupport (6.0.0.beta3) diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index eb43ff9c63..dd69930e25 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -305,7 +305,7 @@ module ActionController logger.fatal do message = +"\n#{exception.class} (#{exception.message}):\n" - message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) + message << exception.annotated_source_code.to_s if exception.respond_to?(:annotated_source_code) message << " " << exception.backtrace.join("\n ") "#{message}\n\n" end diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 962d10d81b..88b3a93211 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -315,7 +315,7 @@ module Mime include Singleton def initialize - super "*/*", :all + super "*/*", nil end def all?; true; end diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index bb49bc4dda..59113e13f4 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -146,7 +146,7 @@ module ActionDispatch message = [] message << " " message << "#{exception.class} (#{exception.message}):" - message.concat(exception.annoted_source_code) if exception.respond_to?(:annoted_source_code) + message.concat(exception.annotated_source_code) if exception.respond_to?(:annotated_source_code) message << " " message.concat(trace) diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 21de05b323..2f8f191828 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -158,6 +158,12 @@ class RespondToController < ActionController::Base end end + def handle_any_with_template + respond_to do |type| + type.any { render "test/hello_world" } + end + end + def all_types_with_layout respond_to do |type| type.html @@ -572,6 +578,13 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "HTML", @response.body end + def test_handle_any_with_template + @request.accept = "*/*" + + get :handle_any_with_template + assert_equal "Hello world!", @response.body + end + def test_html_type_with_layout @request.accept = "text/html" get :all_types_with_layout diff --git a/actionview/lib/action_view/renderer/streaming_template_renderer.rb b/actionview/lib/action_view/renderer/streaming_template_renderer.rb index 279ef3c680..19fa399e2c 100644 --- a/actionview/lib/action_view/renderer/streaming_template_renderer.rb +++ b/actionview/lib/action_view/renderer/streaming_template_renderer.rb @@ -34,7 +34,7 @@ module ActionView return unless logger message = +"\n#{exception.class} (#{exception.message}):\n" - message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) + message << exception.annotated_source_code.to_s if exception.respond_to?(:annotated_source_code) message << " " << exception.backtrace.join("\n ") logger.fatal("#{message}\n\n") end diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 2230de2ca2..ebe52532d6 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -332,6 +332,7 @@ module ActionView # Make sure that the resulting String to be eval'd is in the # encoding of the code + original_source = source source = +<<-end_src def #{method_name}(local_assigns, output_buffer) @virtual_path = #{@virtual_path.inspect};#{locals_code};#{code} @@ -352,7 +353,14 @@ module ActionView raise WrongEncodingError.new(source, Encoding.default_internal) end - mod.module_eval(source, identifier, 0) + begin + mod.module_eval(source, identifier, 0) + rescue SyntaxError + # Account for when code in the template is not syntactically valid; e.g. if we're using + # ERB and the user writes <%= foo( %>, attempting to call a helper `foo` and interpolate + # the result into the template, but missing an end parenthesis. + raise SyntaxErrorInTemplate.new(self, original_source) + end end def handle_render_error(view, e) diff --git a/actionview/lib/action_view/template/error.rb b/actionview/lib/action_view/template/error.rb index e6d9e35815..aace0be04d 100644 --- a/actionview/lib/action_view/template/error.rb +++ b/actionview/lib/action_view/template/error.rb @@ -109,7 +109,7 @@ module ActionView end end - def annoted_source_code + def annotated_source_code source_extract(4) end @@ -138,4 +138,24 @@ module ActionView end TemplateError = Template::Error + + class SyntaxErrorInTemplate < TemplateError #:nodoc + def initialize(template, offending_code_string) + @offending_code_string = offending_code_string + super(template) + end + + def message + <<~MESSAGE + Encountered a syntax error while rendering template: check #{@offending_code_string} + MESSAGE + end + + def annotated_source_code + @offending_code_string.split("\n").map.with_index(1) { |line, index| + indentation = " " * 4 + "#{index}:#{indentation}#{line}" + } + end + end end diff --git a/actionview/test/fixtures/test/syntax_error.html.erb b/actionview/test/fixtures/test/syntax_error.html.erb new file mode 100644 index 0000000000..4004a2b187 --- /dev/null +++ b/actionview/test/fixtures/test/syntax_error.html.erb @@ -0,0 +1,4 @@ +<%= foo( + 1, + 2, +%> diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index 3cf3903a58..15c41051de 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -269,18 +269,24 @@ module RenderTestCases "and is followed by any combination of letters, numbers and underscores.", e.message end + def test_render_template_with_syntax_error + e = assert_raises(ActionView::Template::Error) { @view.render(template: "test/syntax_error") } + assert_match %r!syntax!, e.message + assert_equal "1: <%= foo(", e.annotated_source_code[0].strip + end + def test_render_partial_with_errors e = assert_raises(ActionView::Template::Error) { @view.render(partial: "test/raise") } assert_match %r!method.*doesnt_exist!, e.message assert_equal "", e.sub_template_message assert_equal "1", e.line_number - assert_equal "1: <%= doesnt_exist %>", e.annoted_source_code[0].strip + assert_equal "1: <%= doesnt_exist %>", e.annotated_source_code[0].strip assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name end def test_render_error_indentation e = assert_raises(ActionView::Template::Error) { @view.render(partial: "test/raise_indentation") } - error_lines = e.annoted_source_code + error_lines = e.annotated_source_code assert_match %r!error\shere!, e.message assert_equal "11", e.line_number assert_equal " 9: <p>Ninth paragraph</p>", error_lines.second @@ -300,7 +306,7 @@ module RenderTestCases assert_match %r!method.*doesnt_exist!, e.message assert_equal "", e.sub_template_message assert_equal "1", e.line_number - assert_equal "1: <%= doesnt_exist %>", e.annoted_source_code[0].strip + assert_equal "1: <%= doesnt_exist %>", e.annotated_source_code[0].strip assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name end diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 56fbfacbd7..74a0913b6f 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,9 @@ +* Type cast falsy boolean symbols on boolean attribute as false. + + Fixes #35676. + + *Ryuta Kamizono* + * Change how validation error translation strings are fetched: The new behavior will first try the more specific keys, including doing locale fallback, then try the less specific ones. @@ -130,7 +136,7 @@ *Unathi Chonco* -* Add `config.active_model.i18n_full_message` in order to control whether +* Add `config.active_model.i18n_customize_full_message` in order to control whether the `full_message` error format can be overridden at the attribute or model level in the locale files. This is `false` by default. diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index d7e682d406..3a692a3e64 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -63,9 +63,9 @@ module ActiveModel MESSAGE_OPTIONS = [:message] class << self - attr_accessor :i18n_full_message # :nodoc: + attr_accessor :i18n_customize_full_message # :nodoc: end - self.i18n_full_message = false + self.i18n_customize_full_message = false attr_reader :messages, :details @@ -413,7 +413,7 @@ module ActiveModel return message if attribute == :base attribute = attribute.to_s - if self.class.i18n_full_message && @base.class.respond_to?(:i18n_scope) + if self.class.i18n_customize_full_message && @base.class.respond_to?(:i18n_scope) attribute = attribute.remove(/\[\d\]/) parts = attribute.split(".") attribute_name = parts.pop diff --git a/activemodel/lib/active_model/railtie.rb b/activemodel/lib/active_model/railtie.rb index 0ed70bd473..eb7901c7e9 100644 --- a/activemodel/lib/active_model/railtie.rb +++ b/activemodel/lib/active_model/railtie.rb @@ -13,8 +13,8 @@ module ActiveModel ActiveModel::SecurePassword.min_cost = Rails.env.test? end - initializer "active_model.i18n_full_message" do - ActiveModel::Errors.i18n_full_message = config.active_model.delete(:i18n_full_message) || false + initializer "active_model.i18n_customize_full_message" do + ActiveModel::Errors.i18n_customize_full_message = config.active_model.delete(:i18n_customize_full_message) || false end end end diff --git a/activemodel/lib/active_model/type/boolean.rb b/activemodel/lib/active_model/type/boolean.rb index f6c6efbc87..e64d2c793c 100644 --- a/activemodel/lib/active_model/type/boolean.rb +++ b/activemodel/lib/active_model/type/boolean.rb @@ -14,7 +14,16 @@ module ActiveModel # - Empty strings are coerced to +nil+ # - All other values will be coerced to +true+ class Boolean < Value - FALSE_VALUES = [false, 0, "0", "f", "F", "false", "FALSE", "off", "OFF"].to_set + FALSE_VALUES = [ + false, 0, + "0", :"0", + "f", :f, + "F", :F, + "false", :false, + "FALSE", :FALSE, + "off", :off, + "OFF", :OFF, + ].to_set.freeze def type # :nodoc: :boolean diff --git a/activemodel/test/cases/railtie_test.rb b/activemodel/test/cases/railtie_test.rb index ab60285e2a..95ee7cace3 100644 --- a/activemodel/test/cases/railtie_test.rb +++ b/activemodel/test/cases/railtie_test.rb @@ -32,23 +32,23 @@ class RailtieTest < ActiveModel::TestCase assert_equal true, ActiveModel::SecurePassword.min_cost end - test "i18n full message defaults to false" do + test "i18n customize full message defaults to false" do @app.initialize! - assert_equal false, ActiveModel::Errors.i18n_full_message + assert_equal false, ActiveModel::Errors.i18n_customize_full_message end - test "i18n full message can be disabled" do - @app.config.active_model.i18n_full_message = false + test "i18n customize full message can be disabled" do + @app.config.active_model.i18n_customize_full_message = false @app.initialize! - assert_equal false, ActiveModel::Errors.i18n_full_message + assert_equal false, ActiveModel::Errors.i18n_customize_full_message end - test "i18n full message can be enabled" do - @app.config.active_model.i18n_full_message = true + test "i18n customize full message can be enabled" do + @app.config.active_model.i18n_customize_full_message = true @app.initialize! - assert_equal true, ActiveModel::Errors.i18n_full_message + assert_equal true, ActiveModel::Errors.i18n_customize_full_message end end diff --git a/activemodel/test/cases/type/boolean_test.rb b/activemodel/test/cases/type/boolean_test.rb index 2de0f53640..7f8490b2fe 100644 --- a/activemodel/test/cases/type/boolean_test.rb +++ b/activemodel/test/cases/type/boolean_test.rb @@ -23,6 +23,13 @@ module ActiveModel assert type.cast("\u3000\r\n") assert type.cast("\u0000") assert type.cast("SOMETHING RANDOM") + assert type.cast(:"1") + assert type.cast(:t) + assert type.cast(:T) + assert type.cast(:true) + assert type.cast(:TRUE) + assert type.cast(:on) + assert type.cast(:ON) # explicitly check for false vs nil assert_equal false, type.cast(false) @@ -34,6 +41,13 @@ module ActiveModel assert_equal false, type.cast("FALSE") assert_equal false, type.cast("off") assert_equal false, type.cast("OFF") + assert_equal false, type.cast(:"0") + assert_equal false, type.cast(:f) + assert_equal false, type.cast(:F) + assert_equal false, type.cast(:false) + assert_equal false, type.cast(:FALSE) + assert_equal false, type.cast(:off) + assert_equal false, type.cast(:OFF) end end end diff --git a/activemodel/test/cases/validations/i18n_validation_test.rb b/activemodel/test/cases/validations/i18n_validation_test.rb index ccb565c5bd..eb03e837f1 100644 --- a/activemodel/test/cases/validations/i18n_validation_test.rb +++ b/activemodel/test/cases/validations/i18n_validation_test.rb @@ -13,8 +13,8 @@ class I18nValidationTest < ActiveModel::TestCase I18n.backend = I18n::Backend::Simple.new I18n.backend.store_translations("en", errors: { messages: { custom: nil } }) - @original_i18n_full_message = ActiveModel::Errors.i18n_full_message - ActiveModel::Errors.i18n_full_message = true + @original_i18n_customize_full_message = ActiveModel::Errors.i18n_customize_full_message + ActiveModel::Errors.i18n_customize_full_message = true end def teardown @@ -22,7 +22,7 @@ class I18nValidationTest < ActiveModel::TestCase I18n.load_path.replace @old_load_path I18n.backend = @old_backend I18n.backend.reload! - ActiveModel::Errors.i18n_full_message = @original_i18n_full_message + ActiveModel::Errors.i18n_customize_full_message = @original_i18n_customize_full_message end def test_full_message_encoding @@ -47,7 +47,7 @@ class I18nValidationTest < ActiveModel::TestCase end def test_errors_full_messages_doesnt_use_attribute_format_without_config - ActiveModel::Errors.i18n_full_message = false + ActiveModel::Errors.i18n_customize_full_message = false I18n.backend.store_translations("en", activemodel: { errors: { models: { person: { attributes: { name: { format: "%{message}" } } } } } }) @@ -58,7 +58,7 @@ class I18nValidationTest < ActiveModel::TestCase end def test_errors_full_messages_uses_attribute_format - ActiveModel::Errors.i18n_full_message = true + ActiveModel::Errors.i18n_customize_full_message = true I18n.backend.store_translations("en", activemodel: { errors: { models: { person: { attributes: { name: { format: "%{message}" } } } } } }) @@ -69,7 +69,7 @@ class I18nValidationTest < ActiveModel::TestCase end def test_errors_full_messages_uses_model_format - ActiveModel::Errors.i18n_full_message = true + ActiveModel::Errors.i18n_customize_full_message = true I18n.backend.store_translations("en", activemodel: { errors: { models: { person: { format: "%{message}" } } } }) @@ -80,7 +80,7 @@ class I18nValidationTest < ActiveModel::TestCase end def test_errors_full_messages_uses_deeply_nested_model_attributes_format - ActiveModel::Errors.i18n_full_message = true + ActiveModel::Errors.i18n_customize_full_message = true I18n.backend.store_translations("en", activemodel: { errors: { models: { 'person/contacts/addresses': { attributes: { street: { format: "%{message}" } } } } } }) @@ -91,7 +91,7 @@ class I18nValidationTest < ActiveModel::TestCase end def test_errors_full_messages_uses_deeply_nested_model_model_format - ActiveModel::Errors.i18n_full_message = true + ActiveModel::Errors.i18n_customize_full_message = true I18n.backend.store_translations("en", activemodel: { errors: { models: { 'person/contacts/addresses': { format: "%{message}" } } } }) @@ -102,7 +102,7 @@ class I18nValidationTest < ActiveModel::TestCase end def test_errors_full_messages_with_indexed_deeply_nested_attributes_and_attributes_format - ActiveModel::Errors.i18n_full_message = true + ActiveModel::Errors.i18n_customize_full_message = true I18n.backend.store_translations("en", activemodel: { errors: { models: { 'person/contacts/addresses': { attributes: { street: { format: "%{message}" } } } } } }) @@ -113,7 +113,7 @@ class I18nValidationTest < ActiveModel::TestCase end def test_errors_full_messages_with_indexed_deeply_nested_attributes_and_model_format - ActiveModel::Errors.i18n_full_message = true + ActiveModel::Errors.i18n_customize_full_message = true I18n.backend.store_translations("en", activemodel: { errors: { models: { 'person/contacts/addresses': { format: "%{message}" } } } }) @@ -124,7 +124,7 @@ class I18nValidationTest < ActiveModel::TestCase end def test_errors_full_messages_with_indexed_deeply_nested_attributes_and_i18n_attribute_name - ActiveModel::Errors.i18n_full_message = true + ActiveModel::Errors.i18n_customize_full_message = true I18n.backend.store_translations("en", activemodel: { attributes: { 'person/contacts/addresses': { country: "Country" } } @@ -136,7 +136,7 @@ class I18nValidationTest < ActiveModel::TestCase end def test_errors_full_messages_with_indexed_deeply_nested_attributes_without_i18n_config - ActiveModel::Errors.i18n_full_message = false + ActiveModel::Errors.i18n_customize_full_message = false I18n.backend.store_translations("en", activemodel: { errors: { models: { 'person/contacts/addresses': { attributes: { street: { format: "%{message}" } } } } } }) @@ -147,7 +147,7 @@ class I18nValidationTest < ActiveModel::TestCase end def test_errors_full_messages_with_i18n_attribute_name_without_i18n_config - ActiveModel::Errors.i18n_full_message = false + ActiveModel::Errors.i18n_customize_full_message = false I18n.backend.store_translations("en", activemodel: { attributes: { 'person/contacts[0]/addresses[0]': { country: "Country" } } diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index df781b4f1e..690d487a09 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* Add `ActiveRecord::Relation#extract_associated` for extracting associated records from a relation. + + ``` + account.memberships.extract_associated(:user) + # => Returns collection of User records + ``` + + *DHH* + * Add `ActiveRecord::Relation#annotate` for adding SQL comments to its queries. For example: diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 81ab502824..ae1501f5a1 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -13,7 +13,7 @@ module ActiveRecord :destroy_all, :delete_all, :update_all, :destroy_by, :delete_by, :find_each, :find_in_batches, :in_batches, :select, :reselect, :order, :reorder, :group, :limit, :offset, :joins, :left_joins, :left_outer_joins, - :where, :rewhere, :preload, :eager_load, :includes, :from, :lock, :readonly, :extending, :or, + :where, :rewhere, :preload, :extract_associated, :eager_load, :includes, :from, :lock, :readonly, :extending, :or, :having, :create_with, :distinct, :references, :none, :unscope, :optimizer_hints, :merge, :except, :only, :count, :average, :minimum, :maximum, :sum, :calculate, :annotate, :pluck, :pick, :ids diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6f0f2125dc..c37855172b 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -154,6 +154,19 @@ module ActiveRecord self end + # Extracts a named +association+ from the relation. The named association is first preloaded, + # then the individual association records are collected from the relation. Like so: + # + # account.memberships.extract_associated(:user) + # # => Returns collection of User records + # + # This is short-hand for: + # + # account.memberships.preload(:user).collect(&:user) + def extract_associated(association) + preload(association).collect(&association) + end + # Use to indicate that the given +table_names+ are referenced by an SQL string, # and should therefore be JOINed in any query rather than loaded separately. # This method only works in conjunction with #includes. diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb.tt b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb.tt index 5f7201cfe1..562543f981 100644 --- a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb.tt +++ b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb.tt @@ -6,7 +6,7 @@ class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Mi t.string :password_digest<%= attribute.inject_options %> <% elsif attribute.token? -%> t.string :<%= attribute.name %><%= attribute.inject_options %> -<% else -%> +<% elsif !attribute.virtual? -%> t.<%= attribute.type %> :<%= attribute.name %><%= attribute.inject_options %> <% end -%> <% end -%> diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb.tt b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb.tt index 481c70201b..c07380bec9 100644 --- a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb.tt +++ b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb.tt @@ -7,7 +7,7 @@ class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Mi <%- elsif attribute.token? -%> add_column :<%= table_name %>, :<%= attribute.name %>, :string<%= attribute.inject_options %> add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %>, unique: true - <%- else -%> + <%- elsif !attribute.virtual? -%> add_column :<%= table_name %>, :<%= attribute.name %>, :<%= attribute.type %><%= attribute.inject_options %> <%- if attribute.has_index? -%> add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> @@ -21,7 +21,7 @@ class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Mi <%- attributes.each do |attribute| -%> <%- if attribute.reference? -%> t.references :<%= attribute.name %><%= attribute.inject_options %> - <%- else -%> + <%- elsif !attribute.virtual? -%> <%= '# ' unless attribute.has_index? -%>t.index <%= attribute.index_name %><%= attribute.inject_index_options %> <%- end -%> <%- end -%> @@ -37,7 +37,9 @@ class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Mi <%- if attribute.has_index? -%> remove_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> <%- end -%> + <%- if !attribute.virtual? %> remove_column :<%= table_name %>, :<%= attribute.name %>, :<%= attribute.type %><%= attribute.inject_options %> + <%- end -%> <%- end -%> <%- end -%> <%- end -%> diff --git a/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt b/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt index 55dc65c8ad..cdd029735a 100644 --- a/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt +++ b/activerecord/lib/rails/generators/active_record/model/templates/model.rb.tt @@ -3,6 +3,9 @@ class <%= class_name %> < <%= parent_class_name.classify %> <% attributes.select(&:reference?).each do |attribute| -%> belongs_to :<%= attribute.name %><%= ', polymorphic: true' if attribute.polymorphic? %><%= ', required: true' if attribute.required? %> <% end -%> +<% attributes.select(&:rich_text?).each do |attribute| -%> + has_rich_text :<%= attribute.name %> +<% end -%> <% attributes.select(&:token?).each do |attribute| -%> has_secure_token<% if attribute.name != "token" %> :<%= attribute.name %><% end %> <% end -%> diff --git a/activerecord/test/cases/boolean_test.rb b/activerecord/test/cases/boolean_test.rb index ab9f974e2c..18824004d2 100644 --- a/activerecord/test/cases/boolean_test.rb +++ b/activerecord/test/cases/boolean_test.rb @@ -40,4 +40,13 @@ class BooleanTest < ActiveRecord::TestCase assert_equal b_false, Boolean.find_by(value: "false") assert_equal b_true, Boolean.find_by(value: "true") end + + def test_find_by_falsy_boolean_symbol + ActiveModel::Type::Boolean::FALSE_VALUES.each do |value| + b_false = Boolean.create!(value: value) + + assert_not_predicate b_false, :value? + assert_equal b_false, Boolean.find_by(id: b_false.id, value: value.to_s.to_sym) + end + end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 36b4000018..131e034c66 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -602,6 +602,13 @@ class RelationTest < ActiveRecord::TestCase end end + def test_extracted_association + relation_authors = assert_queries(2) { Post.all.extract_associated(:author) } + root_authors = assert_queries(2) { Post.extract_associated(:author) } + assert_equal relation_authors, root_authors + assert_equal Post.all.collect(&:author), relation_authors + end + def test_find_with_included_associations assert_queries(2) do posts = Post.includes(:comments).order("posts.id") diff --git a/activestorage/activestorage.gemspec b/activestorage/activestorage.gemspec index 34029ac8ad..0ae2dcdd3e 100644 --- a/activestorage/activestorage.gemspec +++ b/activestorage/activestorage.gemspec @@ -28,7 +28,8 @@ Gem::Specification.new do |s| # NOTE: Please read our dependency guidelines before updating versions: # https://edgeguides.rubyonrails.org/security.html#dependency-management-and-cves - s.add_dependency "actionpack", version + s.add_dependency "actionpack", version + s.add_dependency "activejob", version s.add_dependency "activerecord", version s.add_dependency "marcel", "~> 0.3.1" diff --git a/activestorage/app/models/active_storage/attachment.rb b/activestorage/app/models/active_storage/attachment.rb index 13758d9179..874ba80ca8 100644 --- a/activestorage/app/models/active_storage/attachment.rb +++ b/activestorage/app/models/active_storage/attachment.rb @@ -46,3 +46,5 @@ class ActiveStorage::Attachment < ActiveRecord::Base record.attachment_reflections[name]&.options[:dependent] end end + +ActiveSupport.run_load_hooks :active_storage_attachment, ActiveStorage::Attachment diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 6ca7d49bc1..c9fbafad1f 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -193,17 +193,18 @@ class ActiveStorage::Blob < ActiveRecord::Base # # The tempfile's name is prefixed with +ActiveStorage-+ and the blob's ID. Its extension matches that of the blob. # - # By default, the tempfile is created in <tt>Dir.tmpdir</tt>. Pass +tempdir:+ to create it in a different directory: + # By default, the tempfile is created in <tt>Dir.tmpdir</tt>. Pass +tmpdir:+ to create it in a different directory: # - # blob.open(tempdir: "/path/to/tmp") do |file| + # blob.open(tmpdir: "/path/to/tmp") do |file| # # ... # end # # The tempfile is automatically closed and unlinked after the given block is executed. # # Raises ActiveStorage::IntegrityError if the downloaded data does not match the blob's checksum. - def open(tempdir: nil, &block) - ActiveStorage::Downloader.new(self, tempdir: tempdir).download_blob_to_tempfile(&block) + def open(tmpdir: nil, &block) + service.open key, checksum: checksum, + name: [ "ActiveStorage-#{id}-", filename.extension_with_delimiter ], tmpdir: tmpdir, &block end @@ -272,6 +273,6 @@ class ActiveStorage::Blob < ActiveRecord::Base { content_type: content_type } end end - - ActiveSupport.run_load_hooks(:active_storage_blob, self) end + +ActiveSupport.run_load_hooks :active_storage_blob, ActiveStorage::Blob diff --git a/activestorage/lib/active_storage/analyzer.rb b/activestorage/lib/active_storage/analyzer.rb index caa25418a5..26414ffbc2 100644 --- a/activestorage/lib/active_storage/analyzer.rb +++ b/activestorage/lib/active_storage/analyzer.rb @@ -24,14 +24,14 @@ module ActiveStorage private # Downloads the blob to a tempfile on disk. Yields the tempfile. def download_blob_to_tempfile(&block) #:doc: - blob.open tempdir: tempdir, &block + blob.open tmpdir: tmpdir, &block end def logger #:doc: ActiveStorage.logger end - def tempdir #:doc: + def tmpdir #:doc: Dir.tmpdir end end diff --git a/activestorage/lib/active_storage/downloader.rb b/activestorage/lib/active_storage/downloader.rb index 87be6efb05..4d7e832af5 100644 --- a/activestorage/lib/active_storage/downloader.rb +++ b/activestorage/lib/active_storage/downloader.rb @@ -2,24 +2,23 @@ module ActiveStorage class Downloader #:nodoc: - def initialize(blob, tempdir: nil) - @blob = blob - @tempdir = tempdir + attr_reader :service + + def initialize(service) + @service = service end - def download_blob_to_tempfile - open_tempfile do |file| - download_blob_to file - verify_integrity_of file + def open(key, checksum:, name: "ActiveStorage-", tmpdir: nil) + open_tempfile(name, tmpdir) do |file| + download key, file + verify_integrity_of file, checksum: checksum yield file end end private - attr_reader :blob, :tempdir - - def open_tempfile - file = Tempfile.open([ "ActiveStorage-#{blob.id}-", blob.filename.extension_with_delimiter ], tempdir) + def open_tempfile(name, tmpdir = nil) + file = Tempfile.open(name, tmpdir) begin yield file @@ -28,15 +27,15 @@ module ActiveStorage end end - def download_blob_to(file) + def download(key, file) file.binmode - blob.download { |chunk| file.write(chunk) } + service.download(key) { |chunk| file.write(chunk) } file.flush file.rewind end - def verify_integrity_of(file) - unless Digest::MD5.file(file).base64digest == blob.checksum + def verify_integrity_of(file, checksum:) + unless Digest::MD5.file(file).base64digest == checksum raise ActiveStorage::IntegrityError end end diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index 384e6ebfa6..fc75a8f816 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true require "rails" +require "action_controller/railtie" +require "active_job/railtie" +require "active_record/railtie" + require "active_storage" require "active_storage/previewer/poppler_pdf_previewer" diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb index 95a041fd16..af6bcadd4c 100644 --- a/activestorage/lib/active_storage/previewer.rb +++ b/activestorage/lib/active_storage/previewer.rb @@ -26,7 +26,7 @@ module ActiveStorage private # Downloads the blob to a tempfile on disk. Yields the tempfile. def download_blob_to_tempfile(&block) #:doc: - blob.open tempdir: tempdir, &block + blob.open tmpdir: tmpdir, &block end # Executes a system command, capturing its binary output in a tempfile. Yields the tempfile. @@ -42,7 +42,7 @@ module ActiveStorage # end # end # - # The output tempfile is opened in the directory returned by #tempdir. + # The output tempfile is opened in the directory returned by #tmpdir. def draw(*argv) #:doc: open_tempfile do |file| instrument :preview, key: blob.key do @@ -54,7 +54,7 @@ module ActiveStorage end def open_tempfile - tempfile = Tempfile.open("ActiveStorage-", tempdir) + tempfile = Tempfile.open("ActiveStorage-", tmpdir) begin yield tempfile @@ -77,7 +77,7 @@ module ActiveStorage ActiveStorage.logger end - def tempdir #:doc: + def tmpdir #:doc: Dir.tmpdir end end diff --git a/activestorage/lib/active_storage/service.rb b/activestorage/lib/active_storage/service.rb index c18fccbb1d..aac1e62e7f 100644 --- a/activestorage/lib/active_storage/service.rb +++ b/activestorage/lib/active_storage/service.rb @@ -82,6 +82,10 @@ module ActiveStorage raise NotImplementedError end + def open(*args, &block) + ActiveStorage::Downloader.new(self).open(*args, &block) + end + # Delete the file at the +key+. def delete(key) raise NotImplementedError diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index 54cf9e2b8a..b7b37bacf5 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -104,14 +104,12 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase end end - test "open in a custom tempdir" do - tempdir = Dir.mktmpdir - - create_file_blob(filename: "racecar.jpg").open(tempdir: tempdir) do |file| + test "open in a custom tmpdir" do + create_file_blob(filename: "racecar.jpg").open(tmpdir: tmpdir = Dir.mktmpdir) do |file| assert file.binmode? assert_equal 0, file.pos assert_match(/\.jpg\z/, file.path) - assert file.path.starts_with?(tempdir) + assert file.path.starts_with?(tmpdir) assert_equal file_fixture("racecar.jpg").binread, file.read, "Expected downloaded file to match fixture file" end end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 0551f0781f..de933ec122 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,26 @@ +* In `:zeitwerk` mode, eager load directories in engines and applications only + if present in their respective `config.eager_load_paths`. + + A common use case for this is adding `lib` to `config.autoload_paths`, but + not to `config.eager_load_paths`. In that configuration, for example, files + in the `lib` directory should not be eager loaded. + + *Xavier Noria* + +* Fix bug in Range comparisons when comparing to an excluded-end Range + + Before: + + (1..10).cover?(1...11) => false + + After: + + (1..10).cover?(1...11) => true + + With the same change for `Range#include?` and `Range#===`. + + *Owen Stephens* + * Use weak references in descendants tracker to allow anonymous subclasses to be garbage collected. diff --git a/activesupport/lib/active_support/core_ext/range/compare_range.rb b/activesupport/lib/active_support/core_ext/range/compare_range.rb index 6f6d2a27bb..ea1dc29a76 100644 --- a/activesupport/lib/active_support/core_ext/range/compare_range.rb +++ b/activesupport/lib/active_support/core_ext/range/compare_range.rb @@ -3,9 +3,10 @@ module ActiveSupport module CompareWithRange # Extends the default Range#=== to support range comparisons. - # (1..5) === (1..5) # => true - # (1..5) === (2..3) # => true - # (1..5) === (2..6) # => false + # (1..5) === (1..5) # => true + # (1..5) === (2..3) # => true + # (1..5) === (1...6) # => true + # (1..5) === (2..6) # => false # # The native Range#=== behavior is untouched. # ('a'..'f') === ('c') # => true @@ -13,17 +14,20 @@ module ActiveSupport def ===(value) if value.is_a?(::Range) # 1...10 includes 1..9 but it does not include 1..10. + # 1..10 includes 1...11 but it does not include 1...12. operator = exclude_end? && !value.exclude_end? ? :< : :<= - super(value.first) && value.last.send(operator, last) + value_max = !exclude_end? && value.exclude_end? ? value.max : value.last + super(value.first) && value_max.send(operator, last) else super end end # Extends the default Range#include? to support range comparisons. - # (1..5).include?(1..5) # => true - # (1..5).include?(2..3) # => true - # (1..5).include?(2..6) # => false + # (1..5).include?(1..5) # => true + # (1..5).include?(2..3) # => true + # (1..5).include?(1...6) # => true + # (1..5).include?(2..6) # => false # # The native Range#include? behavior is untouched. # ('a'..'f').include?('c') # => true @@ -31,17 +35,20 @@ module ActiveSupport def include?(value) if value.is_a?(::Range) # 1...10 includes 1..9 but it does not include 1..10. + # 1..10 includes 1...11 but it does not include 1...12. operator = exclude_end? && !value.exclude_end? ? :< : :<= - super(value.first) && value.last.send(operator, last) + value_max = !exclude_end? && value.exclude_end? ? value.max : value.last + super(value.first) && value_max.send(operator, last) else super end end # Extends the default Range#cover? to support range comparisons. - # (1..5).cover?(1..5) # => true - # (1..5).cover?(2..3) # => true - # (1..5).cover?(2..6) # => false + # (1..5).cover?(1..5) # => true + # (1..5).cover?(2..3) # => true + # (1..5).cover?(1...6) # => true + # (1..5).cover?(2..6) # => false # # The native Range#cover? behavior is untouched. # ('a'..'f').cover?('c') # => true @@ -49,8 +56,10 @@ module ActiveSupport def cover?(value) if value.is_a?(::Range) # 1...10 covers 1..9 but it does not cover 1..10. + # 1..10 covers 1...11 but it does not cover 1...12. operator = exclude_end? && !value.exclude_end? ? :< : :<= - super(value.first) && value.last.send(operator, last) + value_max = !exclude_end? && value.exclude_end? ? value.max : value.last + super(value.first) && value_max.send(operator, last) else super end diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb index 0b40c3d799..638152626b 100644 --- a/activesupport/lib/active_support/core_ext/string/output_safety.rb +++ b/activesupport/lib/active_support/core_ext/string/output_safety.rb @@ -135,10 +135,12 @@ module ActiveSupport #:nodoc: class SafeBuffer < String UNSAFE_STRING_METHODS = %w( capitalize chomp chop delete delete_prefix delete_suffix - downcase gsub lstrip next reverse rstrip slice squeeze strip - sub succ swapcase tr tr_s unicode_normalize upcase + downcase lstrip next reverse rstrip slice squeeze strip + succ swapcase tr tr_s unicode_normalize upcase ) + UNSAFE_STRING_METHODS_WITH_BACKREF = %w(gsub sub) + alias_method :original_concat, :concat private :original_concat @@ -253,11 +255,44 @@ module ActiveSupport #:nodoc: end end + UNSAFE_STRING_METHODS_WITH_BACKREF.each do |unsafe_method| + if unsafe_method.respond_to?(unsafe_method) + class_eval <<-EOT, __FILE__, __LINE__ + 1 + def #{unsafe_method}(*args, &block) # def gsub(*args, &block) + if block # if block + to_str.#{unsafe_method}(*args) { |*params| # to_str.gsub(*args) { |*params| + set_block_back_references(block, $~) # set_block_back_references(block, $~) + block.call(*params) # block.call(*params) + } # } + else # else + to_str.#{unsafe_method}(*args) # to_str.gsub(*args) + end # end + end # end + + def #{unsafe_method}!(*args, &block) # def gsub!(*args, &block) + @html_safe = false # @html_safe = false + if block # if block + super(*args) { |*params| # super(*args) { |*params| + set_block_back_references(block, $~) # set_block_back_references(block, $~) + block.call(*params) # block.call(*params) + } # } + else # else + super # super + end # end + end # end + EOT + end + end + private def html_escape_interpolated_argument(arg) (!html_safe? || arg.html_safe?) ? arg : CGI.escapeHTML(arg.to_s) end + + def set_block_back_references(block, match_data) + block.binding.eval("proc { |m| $~ = m }").call(match_data) + end end end diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index d5d00b5e6e..82f07c085e 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -70,6 +70,11 @@ module ActiveSupport #:nodoc: # only once. All directories in this set must also be present in +autoload_paths+. mattr_accessor :autoload_once_paths, default: [] + # This is a private set that collects all eager load paths during bootstrap. + # Useful for Zeitwerk integration. Its public interface is the config.* path + # accessors of each engine. + mattr_accessor :_eager_load_paths, default: Set.new + # An array of qualified constant names that have been loaded. Adding a name # to this array will cause it to be unloaded the next time Dependencies are # cleared. diff --git a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb index c6fdade006..a43d03cf09 100644 --- a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb +++ b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "set" require "active_support/core_ext/string/inflections" module ActiveSupport @@ -52,7 +53,7 @@ module ActiveSupport class << self def take_over setup_autoloaders - freeze_autoload_paths + freeze_paths decorate_dependencies end @@ -64,11 +65,11 @@ module ActiveSupport # prevent misconfigurations. next unless File.directory?(autoload_path) - if autoload_once?(autoload_path) - Rails.autoloaders.once.push_dir(autoload_path) - else - Rails.autoloaders.main.push_dir(autoload_path) - end + autoloader = \ + autoload_once?(autoload_path) ? Rails.autoloaders.once : Rails.autoloaders.main + + autoloader.push_dir(autoload_path) + autoloader.do_not_eager_load(autoload_path) unless eager_load?(autoload_path) end Rails.autoloaders.each(&:setup) @@ -78,9 +79,14 @@ module ActiveSupport Dependencies.autoload_once_paths.include?(autoload_path) end - def freeze_autoload_paths + def eager_load?(autoload_path) + Dependencies._eager_load_paths.member?(autoload_path) + end + + def freeze_paths Dependencies.autoload_paths.freeze Dependencies.autoload_once_paths.freeze + Dependencies._eager_load_paths.freeze end def decorate_dependencies diff --git a/activesupport/test/core_ext/range_ext_test.rb b/activesupport/test/core_ext/range_ext_test.rb index 4b8efb8a93..16d6a4c2f2 100644 --- a/activesupport/test/core_ext/range_ext_test.rb +++ b/activesupport/test/core_ext/range_ext_test.rb @@ -57,7 +57,7 @@ class RangeTest < ActiveSupport::TestCase end def test_should_include_other_with_exclusive_end - assert((1..10).include?(1...10)) + assert((1..10).include?(1...11)) end def test_should_compare_identical_inclusive @@ -69,7 +69,7 @@ class RangeTest < ActiveSupport::TestCase end def test_should_compare_other_with_exclusive_end - assert((1..10) === (1...10)) + assert((1..10) === (1...11)) end def test_exclusive_end_should_not_include_identical_with_inclusive_end @@ -93,6 +93,10 @@ class RangeTest < ActiveSupport::TestCase assert range.method(:include?) != range.method(:cover?) end + def test_should_cover_other_with_exclusive_end + assert((1..10).cover?(1...11)) + end + def test_overlaps_on_time time_range_1 = Time.utc(2005, 12, 10, 15, 30)..Time.utc(2005, 12, 10, 17, 30) time_range_2 = Time.utc(2005, 12, 10, 17, 00)..Time.utc(2005, 12, 10, 18, 00) diff --git a/activesupport/test/safe_buffer_test.rb b/activesupport/test/safe_buffer_test.rb index 32e1d2d5bf..b1a1c2d390 100644 --- a/activesupport/test/safe_buffer_test.rb +++ b/activesupport/test/safe_buffer_test.rb @@ -248,4 +248,22 @@ class SafeBufferTest < ActiveSupport::TestCase x = "Hello".html_safe assert_nil x[/a/, 1] end + + test "Should set back references" do + a = "foo123".html_safe + a2 = a.sub(/([a-z]+)([0-9]+)/) { $2 + $1 } + assert_equal "123foo", a2 + assert_not_predicate a2, :html_safe? + a.sub!(/([a-z]+)([0-9]+)/) { $2 + $1 } + assert_equal "123foo", a + assert_not_predicate a, :html_safe? + + b = "foo123 bar456".html_safe + b2 = b.gsub(/([a-z]+)([0-9]+)/) { $2 + $1 } + assert_equal "123foo 456bar", b2 + assert_not_predicate b2, :html_safe? + b.gsub!(/([a-z]+)([0-9]+)/) { $2 + $1 } + assert_equal "123foo 456bar", b + assert_not_predicate b, :html_safe? + end end diff --git a/guides/source/6_0_release_notes.md b/guides/source/6_0_release_notes.md index 04a02259e9..6c3091153f 100644 --- a/guides/source/6_0_release_notes.md +++ b/guides/source/6_0_release_notes.md @@ -237,6 +237,18 @@ Please refer to the [Changelog][active-model] for detailed changes. ### Notable changes +* Add a configuration option to customize format of the `ActiveModel::Errors#full_message`. + ([Pull Request](https://github.com/rails/rails/pull/32956)) + +* Add support for configuring attribute name for `has_secure_password`. + ([Pull Request](https://github.com/rails/rails/pull/26764)) + +* Add `#slice!` method to `ActiveModel::Errors`. + ([Pull Request](https://github.com/rails/rails/pull/34489)) + +* Add `ActiveModel::Errors#of_kind?` to check presence of a specific error. + ([Pull Request](https://github.com/rails/rails/pull/34866)) + Active Support -------------- diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md index 50d4a4c57d..270e4a3bf9 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -465,7 +465,6 @@ number of digits after the decimal point. * `default` Allows to set a default value on the column. Note that if you are using a dynamic value (such as a date), the default will only be calculated the first time (i.e. on the date the migration is applied). -* `index` Adds an index for the column. * `comment` Adds a comment for the column. Some adapters may support additional options; see the adapter specific API docs diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index cc6e08aaec..e40f16e62d 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -65,6 +65,7 @@ The methods are: * `distinct` * `eager_load` * `extending` +* `extract_associated` * `from` * `group` * `having` diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 04ad5a56a2..86e2dd284e 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -310,7 +310,7 @@ All these configuration options are delegated to the `I18n` library. ### Configuring Active Model -* `config.active_model.i18n_full_message` is a boolean value which controls whether the `full_message` error format can be overridden at the attribute or model level in the locale files. This is `false` by default. +* `config.active_model.i18n_customize_full_message` is a boolean value which controls whether the `full_message` error format can be overridden at the attribute or model level in the locale files. This is `false` by default. ### Configuring Active Record diff --git a/guides/source/engines.md b/guides/source/engines.md index a00311bffb..0b17137270 100644 --- a/guides/source/engines.md +++ b/guides/source/engines.md @@ -1518,6 +1518,7 @@ To hook into the initialization process of one of the following classes use the | `ActiveJob::Base` | `active_job` | | `ActiveJob::TestCase` | `active_job_test_case` | | `ActiveRecord::Base` | `active_record` | +| `ActiveStorage::Attachment` | `active_storage_attachment` | | `ActiveStorage::Blob` | `active_storage_blob` | | `ActiveSupport::TestCase` | `active_support_test_case` | | `i18n` | `i18n` | diff --git a/guides/source/testing.md b/guides/source/testing.md index 93372b5c26..1fad02812b 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -1404,7 +1404,7 @@ If you find your helpers are cluttering `test_helper.rb`, you can extract them i ```ruby # lib/test/multiple_assertions.rb module MultipleAssertions - def assert_multiple_of_fourty_two(number) + def assert_multiple_of_forty_two(number) assert (number % 42 == 0), 'expected #{number} to be a multiple of 42' end end @@ -1419,8 +1419,8 @@ require 'test/multiple_assertions' class NumberTest < ActiveSupport::TestCase include MultipleAssertions - test '420 is a multiple of fourty two' do - assert_multiple_of_fourty_two 420 + test '420 is a multiple of forty two' do + assert_multiple_of_forty_two 420 end end ``` diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 07bd56c978..9b3698aa5e 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -469,13 +469,16 @@ module Rails self end - # Eager load the application by loading all ruby - # files inside eager_load paths. def eager_load! - if Rails.autoloaders.zeitwerk_enabled? - eager_load_with_zeitwerk! - else - eager_load_with_dependencies! + # Already done by Zeitwerk::Loader.eager_load_all in the finisher. + return if Rails.autoloaders.zeitwerk_enabled? + + config.eager_load_paths.each do |load_path| + # Starts after load_path plus a slash, ends before ".rb". + relname_range = (load_path.to_s.length + 1)...-3 + Dir.glob("#{load_path}/**/*.rb").sort.each do |file| + require_dependency file[relname_range] + end end end @@ -567,12 +570,15 @@ module Rails ActiveSupport::Dependencies.autoload_paths.unshift(*_all_autoload_paths) ActiveSupport::Dependencies.autoload_once_paths.unshift(*_all_autoload_once_paths) - # Freeze so future modifications will fail rather than do nothing mysteriously config.autoload_paths.freeze - config.eager_load_paths.freeze config.autoload_once_paths.freeze end + initializer :set_eager_load_paths, before: :bootstrap_hook do + ActiveSupport::Dependencies._eager_load_paths.merge(config.eager_load_paths) + config.eager_load_paths.freeze + end + initializer :add_routing_paths do |app| routing_paths = paths["config/routes.rb"].existent @@ -651,22 +657,6 @@ module Rails private - def eager_load_with_zeitwerk! - (config.eager_load_paths - Zeitwerk::Loader.all_dirs).each do |path| - Dir.glob("#{path}/**/*.rb").sort.each { |file| require file } - end - end - - def eager_load_with_dependencies! - config.eager_load_paths.each do |load_path| - # Starts after load_path plus a slash, ends before ".rb". - relname_range = (load_path.to_s.length + 1)...-3 - Dir.glob("#{load_path}/**/*.rb").sort.each do |file| - require_dependency file[relname_range] - end - end - end - def load_config_initializer(initializer) # :doc: ActiveSupport::Notifications.instrument("load_config_initializer.railties", initializer: initializer) do load(initializer) diff --git a/railties/lib/rails/generators/generated_attribute.rb b/railties/lib/rails/generators/generated_attribute.rb index a8f7729fd3..e801ab0c90 100644 --- a/railties/lib/rails/generators/generated_attribute.rb +++ b/railties/lib/rails/generators/generated_attribute.rb @@ -74,6 +74,7 @@ module Rails when :datetime, :timestamp then :datetime_select when :date then :date_select when :text then :text_area + when :rich_text then :rich_text_area when :boolean then :check_box else :text_field @@ -82,15 +83,15 @@ module Rails def default @default ||= case type - when :integer then 1 - when :float then 1.5 - when :decimal then "9.99" - when :datetime, :timestamp, :time then Time.now.to_s(:db) - when :date then Date.today.to_s(:db) - when :string then name == "type" ? "" : "MyString" - when :text then "MyText" - when :boolean then false - when :references, :belongs_to then nil + when :integer then 1 + when :float then 1.5 + when :decimal then "9.99" + when :datetime, :timestamp, :time then Time.now.to_s(:db) + when :date then Date.today.to_s(:db) + when :string then name == "type" ? "" : "MyString" + when :text then "MyText" + when :boolean then false + when :references, :belongs_to, :rich_text then nil else "" end @@ -152,6 +153,14 @@ module Rails type == :token end + def rich_text? + type == :rich_text + end + + def virtual? + rich_text? + end + def inject_options (+"").tap { |s| options_for_migration.each { |k, v| s << ", #{k}: #{v.inspect}" } } end diff --git a/railties/lib/rails/generators/test_unit/model/templates/fixtures.yml.tt b/railties/lib/rails/generators/test_unit/model/templates/fixtures.yml.tt index ee4ae47727..0fd9f305d7 100644 --- a/railties/lib/rails/generators/test_unit/model/templates/fixtures.yml.tt +++ b/railties/lib/rails/generators/test_unit/model/templates/fixtures.yml.tt @@ -7,7 +7,7 @@ password_digest: <%%= BCrypt::Password.create('secret') %> <%- elsif attribute.reference? -%> <%= yaml_key_value(attribute.column_name.sub(/_id$/, ''), attribute.default || name) %> - <%- else -%> + <%- elsif !attribute.virtual? -%> <%= yaml_key_value(attribute.column_name, attribute.default) %> <%- end -%> <%- if attribute.polymorphic? -%> diff --git a/railties/test/application/zeitwerk_integration_test.rb b/railties/test/application/zeitwerk_integration_test.rb index c82b37d07d..5d2e34433a 100644 --- a/railties/test/application/zeitwerk_integration_test.rb +++ b/railties/test/application/zeitwerk_integration_test.rb @@ -124,12 +124,26 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase app_file "app/models/user.rb", "class User; end; $zeitwerk_integration_test_user = true" app_file "app/models/post.rb", "class Post; end; $zeitwerk_integration_test_post = true" + boot("production") assert $zeitwerk_integration_test_user assert $zeitwerk_integration_test_post end + test "eager loading loads code in engines" do + $test_blog_engine_eager_loaded = false + + engine("blog") do |bukkit| + bukkit.write("lib/blog.rb", "class BlogEngine < Rails::Engine; end") + bukkit.write("app/models/post.rb", "Post = $test_blog_engine_eager_loaded = true") + end + + boot("production") + + assert $test_blog_engine_eager_loaded + end + test "eager loading loads anything managed by Zeitwerk" do $zeitwerk_integration_test_user = false app_file "app/models/user.rb", "class User; end; $zeitwerk_integration_test_user = true" @@ -149,6 +163,34 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase assert $zeitwerk_integration_test_extras end + test "autoload directories not present in eager load paths are not eager loaded" do + $zeitwerk_integration_test_user = false + app_file "app/models/user.rb", "class User; end; $zeitwerk_integration_test_user = true" + + $zeitwerk_integration_test_lib = false + app_dir "lib" + app_file "lib/webhook_hacks.rb", "WebhookHacks = 1; $zeitwerk_integration_test_lib = true" + + $zeitwerk_integration_test_extras = false + app_dir "extras" + app_file "extras/websocket_hacks.rb", "WebsocketHacks = 1; $zeitwerk_integration_test_extras = true" + + add_to_config "config.autoload_paths << '#{app_path}/lib'" + add_to_config "config.autoload_once_paths << '#{app_path}/extras'" + + boot("production") + + assert $zeitwerk_integration_test_user + assert !$zeitwerk_integration_test_lib + assert !$zeitwerk_integration_test_extras + + assert WebhookHacks + assert WebsocketHacks + + assert $zeitwerk_integration_test_lib + assert $zeitwerk_integration_test_extras + end + test "autoload_paths are set as root dirs of main, and in the same order" do boot diff --git a/railties/test/generators/generated_attribute_test.rb b/railties/test/generators/generated_attribute_test.rb index 772b4f6f0d..7a1a2ee96b 100644 --- a/railties/test/generators/generated_attribute_test.rb +++ b/railties/test/generators/generated_attribute_test.rb @@ -38,6 +38,10 @@ class GeneratedAttributeTest < Rails::Generators::TestCase assert_field_type :boolean, :check_box end + def test_field_type_returns_rich_text_area + assert_field_type :rich_text, :rich_text_area + end + def test_field_type_with_unknown_type_returns_text_field %w(foo bar baz).each do |attribute_type| assert_field_type attribute_type, :text_field @@ -84,7 +88,7 @@ class GeneratedAttributeTest < Rails::Generators::TestCase end def test_default_value_is_nil - %w(references belongs_to).each do |attribute_type| + %w(references belongs_to rich_text).each do |attribute_type| assert_field_default_value attribute_type, nil end end diff --git a/railties/test/generators/migration_generator_test.rb b/railties/test/generators/migration_generator_test.rb index acc5fc3b25..8a66956290 100644 --- a/railties/test/generators/migration_generator_test.rb +++ b/railties/test/generators/migration_generator_test.rb @@ -366,6 +366,38 @@ class MigrationGeneratorTest < Rails::Generators::TestCase Rails.application.config.paths["db/migrate"] = old_paths end + def test_add_migration_ignores_virtual_attributes + migration = "add_rich_text_content_to_messages" + run_generator [migration, "content:rich_text"] + + assert_migration "db/migrate/#{migration}.rb" do |content| + assert_method :change, content do |change| + assert_no_match(/add_column :messages, :content, :rich_text/, change) + end + end + end + + def test_create_table_migration_ignores_virtual_attributes + run_generator ["create_messages", "content:rich_text"] + assert_migration "db/migrate/create_messages.rb" do |content| + assert_method :change, content do |change| + assert_match(/create_table :messages/, change) + assert_no_match(/ t\.rich_text :content/, change) + end + end + end + + def test_remove_migration_with_virtual_attributes + migration = "remove_content_from_messages" + run_generator [migration, "content:rich_text"] + + assert_migration "db/migrate/#{migration}.rb" do |content| + assert_method :change, content do |change| + assert_no_match(/remove_column :messages, :content, :rich_text/, change) + end + end + end + private def with_singular_table_name diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb index bdb430369e..0eb8e9d270 100644 --- a/railties/test/generators/model_generator_test.rb +++ b/railties/test/generators/model_generator_test.rb @@ -499,6 +499,23 @@ class ModelGeneratorTest < Rails::Generators::TestCase assert_file "app/models/user.rb", expected_file end + def test_model_with_rich_text_attribute_adds_has_rich_text + run_generator ["message", "content:rich_text"] + expected_file = <<~FILE + class Message < ApplicationRecord + has_rich_text :content + end + FILE + assert_file "app/models/message.rb", expected_file + end + + def test_skip_virtual_fields_in_fixtures + run_generator ["message", "content:rich_text"] + + assert_generated_fixture("test/fixtures/messages.yml", + "one" => nil, "two" => nil) + end + private def assert_generated_fixture(path, parsed_contents) fixture_file = File.new File.expand_path(path, destination_root) |