diff options
85 files changed, 1164 insertions, 293 deletions
diff --git a/.travis.yml b/.travis.yml index 74f8d4795f..ba1263ad2e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -47,6 +47,8 @@ before_script: # Decodes to e.g. `export VARIABLE=VALUE` - $(base64 --decode <<< "ZXhwb3J0IFNBVUNFX0FDQ0VTU19LRVk9YTAzNTM0M2YtZTkyMi00MGIzLWFhM2MtMDZiM2VhNjM1YzQ4") - $(base64 --decode <<< "ZXhwb3J0IFNBVUNFX1VTRVJOQU1FPXJ1YnlvbnJhaWxz") + - "[[ $GEM != 'railties' ]] || (cd actionview && yarn build)" + - "[[ $GEM != 'railties' ]] || (cd railties/test/isolation/assets && yarn install)" script: 'ci/travis.rb' @@ -44,7 +44,9 @@ gem "libxml-ruby", platforms: :ruby gem "connection_pool", require: false # for railties app_generator_test -gem "bootsnap", ">= 1.1.0", require: false +gem "bootsnap", ">= 1.4.0", require: false + +gem "zeitwerk", ">= 1.0.0" if RUBY_ENGINE == "ruby" # Active Job group :job do diff --git a/Gemfile.lock b/Gemfile.lock index b05ae7915e..fa8094bf23 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -166,9 +166,9 @@ GEM childprocess faraday selenium-webdriver - bootsnap (1.3.2) + bootsnap (1.4.0) msgpack (~> 1.0) - bootsnap (1.3.2-java) + bootsnap (1.4.0-java) msgpack (~> 1.0) builder (3.2.3) bunny (2.13.0) @@ -324,10 +324,10 @@ GEM minitest-server (1.0.5) minitest (~> 5.0) mono_logger (1.1.0) - msgpack (1.2.4) - msgpack (1.2.4-java) - msgpack (1.2.4-x64-mingw32) - msgpack (1.2.4-x86-mingw32) + msgpack (1.2.6) + msgpack (1.2.6-java) + msgpack (1.2.6-x64-mingw32) + msgpack (1.2.6-x86-mingw32) multi_json (1.13.1) multipart-post (2.0.0) mustache (1.1.0) @@ -516,6 +516,7 @@ GEM websocket-extensions (0.1.3) xpath (3.2.0) nokogiri (~> 1.8) + zeitwerk (1.0.0) PLATFORMS java @@ -535,7 +536,7 @@ DEPENDENCIES benchmark-ips blade blade-sauce_labs_plugin - bootsnap (>= 1.1.0) + bootsnap (>= 1.4.0) byebug capybara (>= 2.15) chromedriver-helper @@ -588,6 +589,7 @@ DEPENDENCIES webmock webpacker (>= 4.0.0.rc.3) websocket-client-simple! + zeitwerk (>= 1.0.0) BUNDLED WITH - 1.17.2 + 1.17.3 diff --git a/actioncable/CHANGELOG.md b/actioncable/CHANGELOG.md index 42edbd6954..3ce35edbb8 100644 --- a/actioncable/CHANGELOG.md +++ b/actioncable/CHANGELOG.md @@ -1,3 +1,26 @@ +* Allow passing custom configuration to `ActionCable::Server::Base`. + + You can now create a standalone Action Cable server with a custom configuration + (e.g. to run it in isolation from the default one): + + ```ruby + config = ActionCable::Server::Configuration.new + config.cable = { adapter: "redis", channel_prefix: "custom_" } + + CUSTOM_CABLE = ActionCable::Server::Base.new(config: config) + ``` + + Then you can mount it in the `routes.rb` file: + + ```ruby + Rails.application.routes.draw do + mount CUSTOM_CABLE => "/custom_cable" + # ... + end + ``` + + *Vladimir Dementyev* + * Add `:action_cable_connection` and `:action_cable_channel` load hooks. You can use them to extend `ActionCable::Connection::Base` and `ActionCable::Channel::Base` diff --git a/actioncable/lib/action_cable/server/base.rb b/actioncable/lib/action_cable/server/base.rb index 2b9e1cba3b..98b3743175 100644 --- a/actioncable/lib/action_cable/server/base.rb +++ b/actioncable/lib/action_cable/server/base.rb @@ -12,14 +12,17 @@ module ActionCable include ActionCable::Server::Broadcasting include ActionCable::Server::Connections - cattr_accessor :config, instance_accessor: true, default: ActionCable::Server::Configuration.new + cattr_accessor :config, instance_accessor: false, default: ActionCable::Server::Configuration.new + + attr_reader :config def self.logger; config.logger; end delegate :logger, to: :config attr_reader :mutex - def initialize + def initialize(config: self.class.config) + @config = config @mutex = Monitor.new @remote_connections = @event_loop = @worker_pool = @pubsub = nil end diff --git a/actioncable/test/subscription_adapter/channel_prefix.rb b/actioncable/test/subscription_adapter/channel_prefix.rb index 3071facd9d..475e6cfd3a 100644 --- a/actioncable/test/subscription_adapter/channel_prefix.rb +++ b/actioncable/test/subscription_adapter/channel_prefix.rb @@ -2,17 +2,9 @@ require "test_helper" -class ActionCable::Server::WithIndependentConfig < ActionCable::Server::Base - # ActionCable::Server::Base defines config as a class variable. - # Need config to be an instance variable here as we're testing 2 separate configs - def config - @config ||= ActionCable::Server::Configuration.new - end -end - module ChannelPrefixTest def test_channel_prefix - server2 = ActionCable::Server::WithIndependentConfig.new + server2 = ActionCable::Server::Base.new(config: ActionCable::Server::Configuration.new) server2.config.cable = alt_cable_config server2.config.logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } diff --git a/actionmailbox/test/dummy/db/schema.rb b/actionmailbox/test/dummy/db/schema.rb index 10d4111a89..76c979bcec 100644 --- a/actionmailbox/test/dummy/db/schema.rb +++ b/actionmailbox/test/dummy/db/schema.rb @@ -42,4 +42,5 @@ ActiveRecord::Schema.define(version: 2018_02_12_164506) do t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true end + add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" end diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 5610212fad..033a7f5b9e 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -593,6 +593,7 @@ module ActionMailer private def set_payload_for_mail(payload, mail) + payload[:mail] = mail.encoded payload[:mailer] = name payload[:message_id] = mail.message_id payload[:subject] = mail.subject @@ -601,7 +602,6 @@ module ActionMailer payload[:bcc] = mail.bcc if mail.bcc.present? payload[:cc] = mail.cc if mail.cc.present? payload[:date] = mail.date - payload[:mail] = mail.encoded payload[:perform_deliveries] = mail.perform_deliveries end diff --git a/actionmailer/lib/action_mailer/log_subscriber.rb b/actionmailer/lib/action_mailer/log_subscriber.rb index 25c99342c2..26910f20f0 100644 --- a/actionmailer/lib/action_mailer/log_subscriber.rb +++ b/actionmailer/lib/action_mailer/log_subscriber.rb @@ -10,11 +10,10 @@ module ActionMailer def deliver(event) info do perform_deliveries = event.payload[:perform_deliveries] - recipients = Array(event.payload[:to]).join(", ") if perform_deliveries - "Sent mail to #{recipients} (#{event.duration.round(1)}ms)" + "Delivered mail #{event.payload[:message_id]} (#{event.duration.round(1)}ms)" else - "Skipped sending mail to #{recipients} as `perform_deliveries` is false" + "Skipped delivery of mail #{event.payload[:message_id]} as `perform_deliveries` is false" end end diff --git a/actionmailer/lib/action_mailer/railtie.rb b/actionmailer/lib/action_mailer/railtie.rb index 23488db790..893a4a25b1 100644 --- a/actionmailer/lib/action_mailer/railtie.rb +++ b/actionmailer/lib/action_mailer/railtie.rb @@ -59,6 +59,14 @@ module ActionMailer end end + initializer "action_mailer.set_autoload_paths" do |app| + options = app.config.action_mailer + + if options.show_previews && options.preview_path + ActiveSupport::Dependencies.autoload_paths << options.preview_path + end + end + initializer "action_mailer.compile_config_methods" do ActiveSupport.on_load(:action_mailer) do config.compile_methods! if config.respond_to?(:compile_methods!) @@ -76,12 +84,8 @@ module ActionMailer if options.show_previews app.routes.prepend do - get "/rails/mailers" => "rails/mailers#index", internal: true - get "/rails/mailers/*path" => "rails/mailers#preview", internal: true - end - - if options.preview_path - ActiveSupport::Dependencies.autoload_paths << options.preview_path + get "/rails/mailers" => "rails/mailers#index", internal: true + get "/rails/mailers/*path" => "rails/mailers#preview", internal: true end end end diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index d0c4f189fd..15613d4dce 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -913,6 +913,23 @@ class BaseTest < ActiveSupport::TestCase ActiveSupport::Notifications.unsubscribe "process.action_mailer" end + test "notification for deliver" do + begin + events = [] + ActiveSupport::Notifications.subscribe("deliver.action_mailer") do |*args| + events << ActiveSupport::Notifications::Event.new(*args) + end + + BaseMailer.welcome(body: "Hello there").deliver_now + + assert_equal 1, events.length + assert_equal "deliver.action_mailer", events[0].name + assert_not_nil events[0].payload[:message_id] + ensure + ActiveSupport::Notifications.unsubscribe "deliver.action_mailer" + end + end + private # Execute the block setting the given values and restoring old values after diff --git a/actionmailer/test/log_subscriber_test.rb b/actionmailer/test/log_subscriber_test.rb index fb569ce45f..f09f1997c0 100644 --- a/actionmailer/test/log_subscriber_test.rb +++ b/actionmailer/test/log_subscriber_test.rb @@ -24,11 +24,11 @@ class AMLogSubscriberTest < ActionMailer::TestCase end def test_deliver_is_notified - BaseMailer.welcome.deliver_now + BaseMailer.welcome(message_id: "123@abc").deliver_now wait assert_equal(1, @logger.logged(:info).size) - assert_match(/Sent mail to system@test\.lindsaar\.net/, @logger.logged(:info).first) + assert_match(/Delivered mail 123@abc/, @logger.logged(:info).first) assert_equal(2, @logger.logged(:debug).size) assert_match(/BaseMailer#welcome: processed outbound mail in [\d.]+ms/, @logger.logged(:debug).first) @@ -38,11 +38,11 @@ class AMLogSubscriberTest < ActionMailer::TestCase end def test_deliver_message_when_perform_deliveries_is_false - BaseMailer.welcome_without_deliveries.deliver_now + BaseMailer.welcome_without_deliveries(message_id: "123@abc").deliver_now wait assert_equal(1, @logger.logged(:info).size) - assert_match("Skipped sending mail to system@test.lindsaar.net as `perform_deliveries` is false", @logger.logged(:info).first) + assert_match("Skipped delivery of mail 123@abc as `perform_deliveries` is false", @logger.logged(:info).first) assert_equal(2, @logger.logged(:debug).size) assert_match(/BaseMailer#welcome_without_deliveries: processed outbound mail in [\d.]+ms/, @logger.logged(:debug).first) diff --git a/actionmailer/test/mailers/base_mailer.rb b/actionmailer/test/mailers/base_mailer.rb index c1bb48cc96..dbe1c4f0e6 100644 --- a/actionmailer/test/mailers/base_mailer.rb +++ b/actionmailer/test/mailers/base_mailer.rb @@ -21,8 +21,8 @@ class BaseMailer < ActionMailer::Base mail(template_name: "welcome", template_path: path) end - def welcome_without_deliveries - mail(template_name: "welcome") + def welcome_without_deliveries(hash = {}) + mail({ template_name: "welcome" }.merge!(hash)) mail.perform_deliveries = false end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 45439a3bb1..10d85037ae 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -335,7 +335,7 @@ module ActionDispatch klass = APP_SESSIONS[app] ||= Class.new(Integration::Session) { # If the app is a Rails app, make url_helpers available on the session. # This makes app.url_for and app.foo_path available in the console. - if app.respond_to?(:routes) + if app.respond_to?(:routes) && app.routes.is_a?(ActionDispatch::Routing::RouteSet) include app.routes.url_helpers include app.routes.mounted_helpers end diff --git a/actionpack/test/dispatch/routing/non_dispatch_routed_app_test.rb b/actionpack/test/dispatch/routing/non_dispatch_routed_app_test.rb new file mode 100644 index 0000000000..676a8c38d4 --- /dev/null +++ b/actionpack/test/dispatch/routing/non_dispatch_routed_app_test.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require "abstract_unit" + +module ActionDispatch + module Routing + class NonDispatchRoutedAppTest < ActionDispatch::IntegrationTest + # For example, Grape::API + class SimpleApp + def self.call(env) + [ 200, { "Content-Type" => "text/plain" }, [] ] + end + + def self.routes + [] + end + end + + setup { @app = SimpleApp } + + test "does not except" do + get "/foo" + assert_response :success + end + end + end +end diff --git a/actiontext/test/dummy/db/schema.rb b/actiontext/test/dummy/db/schema.rb index 5216c5fd33..2388986835 100644 --- a/actiontext/test/dummy/db/schema.rb +++ b/actiontext/test/dummy/db/schema.rb @@ -55,4 +55,5 @@ ActiveRecord::Schema.define(version: 2018_10_03_185713) do t.datetime "updated_at", precision: 6, null: false end + add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" end diff --git a/activejob/lib/active_job/queue_name.rb b/activejob/lib/active_job/queue_name.rb index 7bb1e35181..de259261de 100644 --- a/activejob/lib/active_job/queue_name.rb +++ b/activejob/lib/active_job/queue_name.rb @@ -18,6 +18,26 @@ module ActiveJob # post.to_feed! # end # end + # + # Can be given a block that will evaluate in the context of the job + # allowing +self.arguments+ to be accessed so that a dynamic queue name + # can be applied: + # + # class PublishToFeedJob < ApplicationJob + # queue_as do + # post = self.arguments.first + # + # if post.paid? + # :paid_feeds + # else + # :feeds + # end + # end + # + # def perform(post) + # post.to_feed! + # end + # end def queue_as(part_name = nil, &block) if block_given? self.queue_name = block diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 9824787658..90921dec8b 100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -65,11 +65,42 @@ end task adapter => "#{adapter}:env" do adapter_short = adapter == "db2" ? adapter : adapter[/^[a-z0-9]+/] puts [adapter, adapter_short].inspect - (Dir["test/cases/**/*_test.rb"].reject { + + failing_files = [] + + test_files = (Dir["test/cases/**/*_test.rb"].reject { |x| x.include?("/adapters/") - } + Dir["test/cases/adapters/#{adapter_short}/**/*_test.rb"]).all? do |file| - sh(Gem.ruby, "-w", "-Itest", file) - end || raise("Failures") + } + Dir["test/cases/adapters/#{adapter_short}/**/*_test.rb"]).sort + + if ENV["BUILDKITE_PARALLEL_JOB_COUNT"] + n = ENV["BUILDKITE_PARALLEL_JOB"].to_i + m = ENV["BUILDKITE_PARALLEL_JOB_COUNT"].to_i + + test_files = test_files.each_slice(m).map { |slice| slice[n] }.compact + end + + test_files.each do |file| + puts "--- #{file}" + success = sh(Gem.ruby, "-w", "-Itest", file) + unless success + failing_files << file + puts "^^^ +++" + end + puts + end + + puts "--- All tests completed" + unless failing_files.empty? + puts "^^^ +++" + puts + puts "Failed in:" + failing_files.each do |file| + puts " #{file}" + end + puts + + exit 1 + end end end end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 5d0927f17d..0bb63b97ae 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -17,6 +17,23 @@ module ActiveRecord # CollectionAssociation # HasManyAssociation + ForeignAssociation # HasManyThroughAssociation + ThroughAssociation + # + # Associations in Active Record are middlemen between the object that + # holds the association, known as the <tt>owner</tt>, and the associated + # result set, known as the <tt>target</tt>. Association metadata is available in + # <tt>reflection</tt>, which is an instance of <tt>ActiveRecord::Reflection::AssociationReflection</tt>. + # + # For example, given + # + # class Blog < ActiveRecord::Base + # has_many :posts + # end + # + # blog = Blog.first + # + # The association of <tt>blog.posts</tt> has the object +blog+ as its + # <tt>owner</tt>, the collection of its posts as <tt>target</tt>, and + # the <tt>reflection</tt> object represents a <tt>:has_many</tt> macro. class Association #:nodoc: attr_reader :owner, :target, :reflection diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index b0c0beac0e..c3d4eab562 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -209,8 +209,7 @@ module ActiveRecord # This method is abstract in the sense that it relies on # +count_records+, which is a method descendants have to provide. def size - if !find_target? - loaded! unless loaded? + if !find_target? || loaded? target.size elsif @association_ids @association_ids.size diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 78993cee8a..edcb44f0fc 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -2,11 +2,8 @@ module ActiveRecord module Associations - # Association proxies in Active Record are middlemen between the object that - # holds the association, known as the <tt>@owner</tt>, and the actual associated - # object, known as the <tt>@target</tt>. The kind of association any proxy is - # about is available in <tt>@reflection</tt>. That's an instance of the class - # ActiveRecord::Reflection::AssociationReflection. + # Collection proxies in Active Record are middlemen between an + # <tt>association</tt>, and its <tt>target</tt> result set. # # For example, given # @@ -16,14 +13,14 @@ module ActiveRecord # # blog = Blog.first # - # the association proxy in <tt>blog.posts</tt> has the object in +blog+ as - # <tt>@owner</tt>, the collection of its posts as <tt>@target</tt>, and - # the <tt>@reflection</tt> object represents a <tt>:has_many</tt> macro. + # The collection proxy returned by <tt>blog.posts</tt> is built from a + # <tt>:has_many</tt> <tt>association</tt>, and delegates to a collection + # of posts as the <tt>target</tt>. # - # This class delegates unknown methods to <tt>@target</tt> via - # <tt>method_missing</tt>. + # This class delegates unknown methods to the <tt>association</tt>'s + # relation class via a delegate cache. # - # The <tt>@target</tt> object is not \loaded until needed. For example, + # The <tt>target</tt> result set is not loaded until needed. For example, # # blog.posts.count # diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 6f67934a79..5972846940 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -52,7 +52,11 @@ module ActiveRecord # If the collection is empty the target is set to an empty array and # the loaded flag is set to true as well. def count_records - count = counter_cache_value || scope.count(:all) + count = if reflection.has_cached_counter? + owner._read_attribute(reflection.counter_cache_column).to_i + else + scope.count(:all) + end # If there's nothing in the database and @target has no new records # we are certain the current target is an empty array. This is a @@ -62,14 +66,6 @@ module ActiveRecord [association_scope.limit_value, count].compact.min end - def counter_cache_value - reflection.has_cached_counter? ? owner._read_attribute(reflection.counter_cache_column).to_i : nil - end - - def find_target? - super && !counter_cache_value&.zero? - end - def update_counter(difference, reflection = reflection()) if reflection.has_cached_counter? owner.increment!(reflection.counter_cache_column, difference) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb index 2cb0a2a4df..7d20825a75 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb @@ -15,7 +15,7 @@ module ActiveRecord end delegate :quote_column_name, :quote_table_name, :quote_default_expression, :type_to_sql, - :options_include_default?, :supports_indexes_in_create?, :supports_foreign_keys_in_create?, :foreign_key_options, + :options_include_default?, :supports_indexes_in_create?, :supports_foreign_keys?, :foreign_key_options, to: :@conn, private: true private @@ -50,7 +50,7 @@ module ActiveRecord statements.concat(o.indexes.map { |column_name, options| index_in_create(o.name, column_name, options) }) end - if supports_foreign_keys_in_create? + if supports_foreign_keys? statements.concat(o.foreign_keys.map { |to_table, options| foreign_key_in_create(o.name, to_table, options) }) end @@ -127,6 +127,9 @@ module ActiveRecord end def foreign_key_in_create(from_table, to_table, options) + prefix = ActiveRecord::Base.table_name_prefix + suffix = ActiveRecord::Base.table_name_suffix + to_table = "#{prefix}#{to_table}#{suffix}" options = foreign_key_options(from_table, to_table, options) accept ForeignKeyDefinition.new(from_table, to_table, options) end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 198898b764..a27751fd70 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -102,7 +102,7 @@ module ActiveRecord alias validated? validate? def export_name_on_schema_dump? - name !~ ActiveRecord::SchemaDumper.fk_ignore_pattern + !ActiveRecord::SchemaDumper.fk_ignore_pattern.match?(name) if name end def defined_for?(to_table_ord = nil, to_table: nil, **options) @@ -398,10 +398,7 @@ module ActiveRecord end def foreign_key(table_name, options = {}) # :nodoc: - table_name_prefix = ActiveRecord::Base.table_name_prefix - table_name_suffix = ActiveRecord::Base.table_name_suffix - table_name = "#{table_name_prefix}#{table_name}#{table_name_suffix}" - foreign_keys.push([table_name, options]) + foreign_keys << [table_name, options] end # Appends <tt>:datetime</tt> columns <tt>:created_at</tt> and @@ -522,6 +519,7 @@ module ActiveRecord # t.json # t.virtual # t.remove + # t.remove_foreign_key # t.remove_references # t.remove_belongs_to # t.remove_index @@ -695,6 +693,16 @@ module ActiveRecord @base.add_foreign_key(name, *args) end + # Removes the given foreign key from the table. + # + # t.remove_foreign_key(:authors) + # t.remove_foreign_key(column: :author_id) + # + # See {connection.remove_foreign_key}[rdoc-ref:SchemaStatements#remove_foreign_key] + def remove_foreign_key(*args) + @base.remove_foreign_key(name, *args) + end + # Checks to see if a foreign key exists. # # t.foreign_key(:authors) unless t.foreign_key_exists?(:authors) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index d88e75d692..78e153bcc9 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1029,9 +1029,7 @@ module ActiveRecord end def foreign_key_column_for(table_name) # :nodoc: - prefix = Base.table_name_prefix - suffix = Base.table_name_suffix - name = table_name.to_s =~ /#{prefix}(.+)#{suffix}/ ? $1 : table_name.to_s + name = strip_table_name_prefix_and_suffix(table_name) "#{name.singularize}_id" end @@ -1328,6 +1326,12 @@ module ActiveRecord { column: column_names } end + def strip_table_name_prefix_and_suffix(table_name) + prefix = Base.table_name_prefix + suffix = Base.table_name_suffix + table_name.to_s =~ /#{prefix}(.+)#{suffix}/ ? $1 : table_name.to_s + end + def foreign_key_name(table_name, options) options.fetch(:name) do identifier = "#{table_name}_#{options.fetch(:column)}_fk" diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 9a7d7285f2..3c9510e469 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -335,6 +335,7 @@ module ActiveRecord def supports_foreign_keys_in_create? supports_foreign_keys? end + deprecate :supports_foreign_keys_in_create? # Does this adapter support views? def supports_views? diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb index 2394982a7d..0a637d81fa 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb @@ -52,6 +52,34 @@ module ActiveRecord end.compact end + def add_foreign_key(from_table, to_table, **options) + alter_table(from_table) do |definition| + to_table = strip_table_name_prefix_and_suffix(to_table) + definition.foreign_key(to_table, options) + end + end + + def remove_foreign_key(from_table, to_table = nil, **options) + to_table ||= options[:to_table] + options = options.except(:name, :to_table) + foreign_keys = foreign_keys(from_table) + + fkey = foreign_keys.detect do |fk| + table = to_table || begin + table = options[:column].to_s.delete_suffix("_id") + Base.pluralize_table_names ? table.pluralize : table + end + table = strip_table_name_prefix_and_suffix(table) + fk_to_table = strip_table_name_prefix_and_suffix(fk.to_table) + fk_to_table == table && options.all? { |k, v| fk.options[k].to_s == v.to_s } + end || raise(ArgumentError, "Table '#{from_table}' has no foreign key for #{to_table}") + + alter_table(from_table, foreign_keys) do |definition| + fk_to_table = strip_table_name_prefix_and_suffix(fkey.to_table) + definition.foreign_keys.delete([fk_to_table, fkey.options]) + end + end + def create_schema_dumper(options) SQLite3::SchemaDumper.create(self, options) end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 7b3630662b..9f28fdf749 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -121,7 +121,7 @@ module ActiveRecord true end - def supports_foreign_keys_in_create? + def supports_foreign_keys? true end @@ -320,6 +320,9 @@ module ActiveRecord def remove_column(table_name, column_name, type = nil, options = {}) #:nodoc: alter_table(table_name) do |definition| definition.remove_column column_name + definition.foreign_keys.delete_if do |_, fk_options| + fk_options[:column] == column_name.to_s + end end end @@ -421,9 +424,8 @@ module ActiveRecord type.to_sym == :primary_key || options[:primary_key] end - def alter_table(table_name, options = {}) + def alter_table(table_name, foreign_keys = foreign_keys(table_name), **options) altered_table_name = "a#{table_name}" - foreign_keys = foreign_keys(table_name) caller = lambda do |definition| rename = options[:rename] || {} @@ -431,7 +433,8 @@ module ActiveRecord if column = rename[fk.options[:column]] fk.options[:column] = column end - definition.foreign_key(fk.to_table, fk.options) + to_table = strip_table_name_prefix_and_suffix(fk.to_table) + definition.foreign_key(to_table, fk.options) end yield definition if block_given? diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb index 73adf66684..654a01f23b 100644 --- a/activerecord/lib/active_record/database_configurations.rb +++ b/activerecord/lib/active_record/database_configurations.rb @@ -157,7 +157,7 @@ module ActiveRecord configs else configs.map do |config| - ActiveRecord::DatabaseConfigurations::UrlConfig.new(env, config.spec_name, url, config.config) + ActiveRecord::DatabaseConfigurations::UrlConfig.new(config.env_name, config.spec_name, url, config.config) end end else diff --git a/activerecord/lib/active_record/middleware/database_selector.rb b/activerecord/lib/active_record/middleware/database_selector.rb index aad263695b..b5b5df074c 100644 --- a/activerecord/lib/active_record/middleware/database_selector.rb +++ b/activerecord/lib/active_record/middleware/database_selector.rb @@ -11,7 +11,7 @@ module ActiveRecord # behavior. # # The resolver class defines when the application should switch (i.e. read - # from the primary if a write occurred less than 2 seconds ago) and an + # from the primary if a write occurred less than 2 seconds ago) and a # resolver context class that sets a value that helps the resolver class # decide when to switch. # diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index 94906a2943..abc939826b 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -77,18 +77,18 @@ module ActiveRecord class V5_1 < V5_2 def change_column(table_name, column_name, type, options = {}) - if adapter_name == "PostgreSQL" + if connection.adapter_name == "PostgreSQL" super(table_name, column_name, type, options.except(:default, :null, :comment)) - change_column_default(table_name, column_name, options[:default]) if options.key?(:default) - change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) - change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) + connection.change_column_default(table_name, column_name, options[:default]) if options.key?(:default) + connection.change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) + connection.change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) else super end end def create_table(table_name, options = {}) - if adapter_name == "Mysql2" + if connection.adapter_name == "Mysql2" super(table_name, options: "ENGINE=InnoDB", **options) else super @@ -110,13 +110,13 @@ module ActiveRecord end def create_table(table_name, options = {}) - if adapter_name == "PostgreSQL" + if connection.adapter_name == "PostgreSQL" if options[:id] == :uuid && !options.key?(:default) options[:default] = "uuid_generate_v4()" end end - unless adapter_name == "Mysql2" && options[:id] == :bigint + unless connection.adapter_name == "Mysql2" && options[:id] == :bigint if [:integer, :bigint].include?(options[:id]) && !options.key?(:default) options[:default] = nil end @@ -190,7 +190,7 @@ module ActiveRecord if options[:name].present? options[:name].to_s else - index_name(table_name, column: column_names) + connection.index_name(table_name, column: column_names) end super end @@ -210,15 +210,17 @@ module ActiveRecord end def index_name_for_remove(table_name, options = {}) - index_name = index_name(table_name, options) + index_name = connection.index_name(table_name, options) - unless index_name_exists?(table_name, index_name) + unless connection.index_name_exists?(table_name, index_name) if options.is_a?(Hash) && options.has_key?(:name) options_without_column = options.dup options_without_column.delete :column - index_name_without_column = index_name(table_name, options_without_column) + index_name_without_column = connection.index_name(table_name, options_without_column) - return index_name_without_column if index_name_exists?(table_name, index_name_without_column) + if connection.index_name_exists?(table_name, index_name_without_column) + return index_name_without_column + end end raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' does not exist" diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 2213fbefb4..510a275b4e 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -142,7 +142,7 @@ module ActiveRecord end end - # Deletes the row with a primary key matching the +id+ argument, using a + # Deletes the row with a primary key matching the +id+ argument, using an # SQL +DELETE+ statement, and returns the number of rows deleted. Active # Record objects are not instantiated, so the object's callbacks are not # executed, including any <tt>:dependent</tt> association options. diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 8c1b2e2be1..2a27f53f06 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -40,8 +40,7 @@ module ActiveRecord def find_by_sql(sql, binds = [], preparable: nil, &block) result_set = connection.select_all(sanitize_sql(sql), "#{name} Load", binds, preparable: preparable) column_types = result_set.column_types.dup - cached_columns_hash = connection.schema_cache.columns_hash(table_name) - cached_columns_hash.each_key { |k| column_types.delete k } + attribute_types.each_key { |k| column_types.delete k } message_bus = ActiveSupport::Notifications.instrumenter payload = { diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index cef31bea94..bdd3c540bb 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -186,11 +186,9 @@ module ActiveRecord relation = apply_join_dependency relation.pluck(*column_names) else - disallow_raw_sql!(column_names) + klass.disallow_raw_sql!(column_names) relation = spawn - relation.select_values = column_names.map { |cn| - @klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn - } + relation.select_values = column_names result = skip_query_cache_if_necessary { klass.connection.select_all(relation.arel, nil) } result.cast_values(klass.attribute_types) end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index eb80aab701..75976aa8fc 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1052,11 +1052,13 @@ module ActiveRecord def arel_columns(columns) columns.flat_map do |field| - if (Symbol === field || String === field) && (klass.has_attribute?(field) || klass.attribute_alias?(field)) && !from_clause.value - arel_attribute(field) - elsif Symbol === field - connection.quote_table_name(field.to_s) - elsif Proc === field + case field + when Symbol + field = field.to_s + arel_column(field) { connection.quote_table_name(field) } + when String + arel_column(field) { field } + when Proc field.call else field @@ -1064,6 +1066,16 @@ module ActiveRecord end end + def arel_column(field) + field = klass.attribute_alias(field) if klass.attribute_alias?(field) + + if klass.columns_hash.key?(field) && !from_clause.value + arel_attribute(field) + else + yield + end + end + def reverse_sql_order(order_query) if order_query.empty? return [arel_attribute(primary_key).desc] if primary_key diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 05d8aa59c4..2baf3db49a 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -348,6 +348,10 @@ module ActiveRecord assert_equal "special_db_type", @connection.type_to_sql(:special_db_type) end + def test_supports_foreign_keys_in_create_is_deprecated + assert_deprecated { @connection.supports_foreign_keys_in_create? } + end + def test_supports_multi_insert_is_deprecated assert_deprecated { @connection.supports_multi_insert? } end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index c01138c059..3525fa2ab8 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -448,8 +448,13 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end def test_with_select - assert_equal 1, Company.find(2).firm_with_select.attributes.size - assert_equal 1, Company.all.merge!(includes: :firm_with_select).find(2).firm_with_select.attributes.size + assert_equal 1, Post.find(2).author_with_select.attributes.size + assert_equal 1, Post.includes(:author_with_select).find(2).author_with_select.attributes.size + end + + def test_custom_attribute_with_select + assert_equal 2, Company.find(2).firm_with_select.attributes.size + assert_equal 2, Company.includes(:firm_with_select).find(2).firm_with_select.attributes.size end def test_belongs_to_without_counter_cache_option diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index bf44be1811..7d669198ca 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1230,10 +1230,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_not_predicate Ship.reflect_on_association(:treasures), :has_cached_counter? # Count should come from sql count() of treasures rather than treasures_count attribute - assert_queries(1) do - assert_equal ship.treasures.size, 0 - assert_predicate ship.treasures, :loaded? - end + assert_equal ship.treasures.size, 0 assert_no_difference lambda { ship.reload.treasures_count }, "treasures_count should not be changed" do ship.treasures.create(name: "Gold") @@ -1354,20 +1351,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase post = posts(:welcome) assert_no_queries do assert_not_empty post.comments - assert_equal 2, post.comments.size - assert_not_predicate post.comments, :loaded? - end - post = posts(:misc_by_bob) - assert_no_queries do - assert_empty post.comments - assert_predicate post.comments, :loaded? - end - end - - def test_empty_association_loading_with_counter_cache - post = posts(:misc_by_bob) - assert_no_queries do - assert_empty post.comments.to_a end end @@ -1755,6 +1738,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_nil posts.first end + def test_destroy_all_on_desynced_counter_cache_association + category = categories(:general) + assert_operator category.categorizations.count, :>, 0 + + category.categorizations.destroy_all + assert_equal 0, category.categorizations.count + end + def test_destroy_on_association_clears_scope author = Author.create!(name: "Gannon") posts = author.posts @@ -2004,6 +1995,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_not_predicate company.clients, :loaded? end + def test_counter_cache_on_unloaded_association + car = Car.create(name: "My AppliCar") + assert_equal 0, car.engines.size + end + def test_ids_reader_cache_not_used_for_size_when_association_is_dirty firm = Firm.create!(name: "Startup") assert_equal 0, firm.client_ids.size @@ -2019,24 +2015,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal [3, 11], firm.client_ids end - def test_zero_counter_cache_usage_on_unloaded_association - car = Car.create!(name: "My AppliCar") - assert_no_queries do - assert_equal car.engines.size, 0 - assert_predicate car.engines, :loaded? - end - end - - def test_counter_cache_on_new_record_unloaded_association - car = Car.new(name: "My AppliCar") - # Ensure no schema queries inside assertion - Engine.primary_key - assert_no_queries do - assert_equal car.engines.size, 0 - assert_predicate car.engines, :loaded? - end - end - def test_get_ids_ignores_include_option assert_equal [readers(:michael_welcome).id], posts(:welcome).readers_with_person_ids end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 363593ca19..1b8f748bad 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1226,14 +1226,15 @@ class BasicsTest < ActiveRecord::TestCase end def test_attribute_names - assert_equal ["id", "type", "firm_id", "firm_name", "name", "client_of", "rating", "account_id", "description"], - Company.attribute_names + expected = ["id", "type", "firm_id", "firm_name", "name", "client_of", "rating", "account_id", "description", "metadata"] + assert_equal expected, Company.attribute_names end def test_has_attribute assert Company.has_attribute?("id") assert Company.has_attribute?("type") assert Company.has_attribute?("name") + assert Company.has_attribute?("metadata") assert_not Company.has_attribute?("lastname") assert_not Company.has_attribute?("age") end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 850bc49676..b001667ac9 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -690,8 +690,9 @@ class CalculationsTest < ActiveRecord::TestCase end def test_pluck_not_auto_table_name_prefix_if_column_joined - Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)]) - assert_equal [7], Company.joins(:contracts).pluck(:developer_id) + company = Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)]) + metadata = company.contracts.first.metadata + assert_equal [metadata], Company.joins(:contracts).pluck(:metadata) end def test_pluck_with_selection_clause diff --git a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb index 225cccc62c..515bf5df06 100644 --- a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb +++ b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb @@ -72,6 +72,16 @@ module ActiveRecord assert_equal expected, actual end + def test_resolver_with_database_uri_and_multiple_envs + ENV["DATABASE_URL"] = "postgres://localhost" + ENV["RAILS_ENV"] = "test" + + config = { "production" => { "adapter" => "postgresql", "database" => "foo_prod" }, "test" => { "adapter" => "postgresql", "database" => "foo_test" } } + actual = resolve_spec(:test, config) + expected = { "adapter" => "postgresql", "database" => "foo_test", "host" => "localhost", "name" => "test" } + assert_equal expected, actual + end + def test_resolver_with_database_uri_and_unknown_symbol_key ENV["DATABASE_URL"] = "postgres://localhost/foo" config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } diff --git a/activerecord/test/cases/database_selector_test.rb b/activerecord/test/cases/database_selector_test.rb index 4106a6ec46..fd02d2acb4 100644 --- a/activerecord/test/cases/database_selector_test.rb +++ b/activerecord/test/cases/database_selector_test.rb @@ -11,6 +11,10 @@ module ActiveRecord @session = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session.new(@session_store) end + teardown do + ActiveRecord::Base.connection_handlers = { writing: ActiveRecord::Base.default_connection_handler } + end + def test_empty_session assert_equal Time.at(0), @session.last_write_timestamp end diff --git a/activerecord/test/cases/migration/change_schema_test.rb b/activerecord/test/cases/migration/change_schema_test.rb index 7777508349..cc0587fa50 100644 --- a/activerecord/test/cases/migration/change_schema_test.rb +++ b/activerecord/test/cases/migration/change_schema_test.rb @@ -462,7 +462,11 @@ module ActiveRecord end def test_create_table_with_force_cascade_drops_dependent_objects - skip "MySQL > 5.5 does not drop dependent objects with DROP TABLE CASCADE" if current_adapter?(:Mysql2Adapter) + if current_adapter?(:Mysql2Adapter) + skip "MySQL > 5.5 does not drop dependent objects with DROP TABLE CASCADE" + elsif current_adapter?(:SQLite3Adapter) + skip "SQLite3 does not support DROP TABLE CASCADE syntax" + end # can't re-create table referenced by foreign key assert_raises(ActiveRecord::StatementInvalid) do @connection.create_table :trains, force: true diff --git a/activerecord/test/cases/migration/foreign_key_test.rb b/activerecord/test/cases/migration/foreign_key_test.rb index bb233fbf74..32586e525b 100644 --- a/activerecord/test/cases/migration/foreign_key_test.rb +++ b/activerecord/test/cases/migration/foreign_key_test.rb @@ -3,7 +3,7 @@ require "cases/helper" require "support/schema_dumping_helper" -if ActiveRecord::Base.connection.supports_foreign_keys_in_create? +if ActiveRecord::Base.connection.supports_foreign_keys? module ActiveRecord class Migration class ForeignKeyInCreateTest < ActiveRecord::TestCase @@ -20,9 +20,7 @@ if ActiveRecord::Base.connection.supports_foreign_keys_in_create? end end - class ForeignKeyChangeColumnTest < ActiveRecord::TestCase - self.use_transactional_tests = false - + module ForeignKeyChangeColumnSharedTest class Rocket < ActiveRecord::Base has_many :astronauts end @@ -31,24 +29,39 @@ if ActiveRecord::Base.connection.supports_foreign_keys_in_create? belongs_to :rocket end - setup do - @connection = ActiveRecord::Base.connection - @connection.create_table "rockets", force: true do |t| - t.string :name + class CreateRocketsMigration < ActiveRecord::Migration::Current + def up + create_table :rockets do |t| + t.string :name + end + + create_table :astronauts do |t| + t.string :name + t.references :rocket, foreign_key: true + end end - @connection.create_table "astronauts", force: true do |t| - t.string :name - t.references :rocket, foreign_key: true + def down + drop_table :astronauts, if_exists: true + drop_table :rockets, if_exists: true end + end + + def setup + @connection = ActiveRecord::Base.connection + @migration = CreateRocketsMigration.new + silence_stream($stdout) { @migration.migrate(:up) } + Rocket.reset_table_name Rocket.reset_column_information + Astronaut.reset_table_name Astronaut.reset_column_information end - teardown do - @connection.drop_table "astronauts", if_exists: true - @connection.drop_table "rockets", if_exists: true + def teardown + silence_stream($stdout) { @migration.migrate(:down) } + Rocket.reset_table_name Rocket.reset_column_information + Astronaut.reset_table_name Astronaut.reset_column_information end @@ -56,53 +69,114 @@ if ActiveRecord::Base.connection.supports_foreign_keys_in_create? rocket = Rocket.create!(name: "myrocket") rocket.astronauts << Astronaut.create! - @connection.change_column_null :rockets, :name, false + @connection.change_column_null Rocket.table_name, :name, false - foreign_keys = @connection.foreign_keys("astronauts") + foreign_keys = @connection.foreign_keys(Astronaut.table_name) assert_equal 1, foreign_keys.size fk = foreign_keys.first assert_equal "myrocket", Rocket.first.name - assert_equal "astronauts", fk.from_table - assert_equal "rockets", fk.to_table + assert_equal Astronaut.table_name, fk.from_table + assert_equal Rocket.table_name, fk.to_table end def test_rename_column_of_child_table rocket = Rocket.create!(name: "myrocket") rocket.astronauts << Astronaut.create! - @connection.rename_column :astronauts, :name, :astronaut_name + @connection.rename_column Astronaut.table_name, :name, :astronaut_name - foreign_keys = @connection.foreign_keys("astronauts") + foreign_keys = @connection.foreign_keys(Astronaut.table_name) assert_equal 1, foreign_keys.size fk = foreign_keys.first assert_equal "myrocket", Rocket.first.name - assert_equal "astronauts", fk.from_table - assert_equal "rockets", fk.to_table + assert_equal Astronaut.table_name, fk.from_table + assert_equal Rocket.table_name, fk.to_table end def test_rename_reference_column_of_child_table rocket = Rocket.create!(name: "myrocket") rocket.astronauts << Astronaut.create! - @connection.rename_column :astronauts, :rocket_id, :new_rocket_id + @connection.rename_column Astronaut.table_name, :rocket_id, :new_rocket_id - foreign_keys = @connection.foreign_keys("astronauts") + foreign_keys = @connection.foreign_keys(Astronaut.table_name) assert_equal 1, foreign_keys.size fk = foreign_keys.first assert_equal "myrocket", Rocket.first.name - assert_equal "astronauts", fk.from_table - assert_equal "rockets", fk.to_table + assert_equal Astronaut.table_name, fk.from_table + assert_equal Rocket.table_name, fk.to_table assert_equal "new_rocket_id", fk.options[:column] end + + def test_remove_reference_column_of_child_table + rocket = Rocket.create!(name: "myrocket") + rocket.astronauts << Astronaut.create! + + @connection.remove_column Astronaut.table_name, :rocket_id + + assert_empty @connection.foreign_keys(Astronaut.table_name) + end + + def test_remove_foreign_key_by_column + rocket = Rocket.create!(name: "myrocket") + rocket.astronauts << Astronaut.create! + + @connection.remove_foreign_key Astronaut.table_name, column: :rocket_id + + assert_empty @connection.foreign_keys(Astronaut.table_name) + end + + def test_remove_foreign_key_by_column_in_change_table + rocket = Rocket.create!(name: "myrocket") + rocket.astronauts << Astronaut.create! + + @connection.change_table Astronaut.table_name do |t| + t.remove_foreign_key column: :rocket_id + end + + assert_empty @connection.foreign_keys(Astronaut.table_name) + end + end + + class ForeignKeyChangeColumnTest < ActiveRecord::TestCase + include ForeignKeyChangeColumnSharedTest + + self.use_transactional_tests = false + end + + class ForeignKeyChangeColumnWithPrefixTest < ActiveRecord::TestCase + include ForeignKeyChangeColumnSharedTest + + self.use_transactional_tests = false + + setup do + ActiveRecord::Base.table_name_prefix = "p_" + end + + teardown do + ActiveRecord::Base.table_name_prefix = nil + end + end + + class ForeignKeyChangeColumnWithSuffixTest < ActiveRecord::TestCase + include ForeignKeyChangeColumnSharedTest + + self.use_transactional_tests = false + + setup do + ActiveRecord::Base.table_name_suffix = "_p" + end + + teardown do + ActiveRecord::Base.table_name_suffix = nil + end end end end -end -if ActiveRecord::Base.connection.supports_foreign_keys? module ActiveRecord class Migration class ForeignKeyTest < ActiveRecord::TestCase @@ -141,7 +215,7 @@ if ActiveRecord::Base.connection.supports_foreign_keys? assert_equal "fk_test_has_pk", fk.to_table assert_equal "fk_id", fk.column assert_equal "pk_id", fk.primary_key - assert_equal "fk_name", fk.name + assert_equal "fk_name", fk.name unless current_adapter?(:SQLite3Adapter) end def test_add_foreign_key_inferes_column @@ -155,7 +229,7 @@ if ActiveRecord::Base.connection.supports_foreign_keys? assert_equal "rockets", fk.to_table assert_equal "rocket_id", fk.column assert_equal "id", fk.primary_key - assert_equal("fk_rails_78146ddd2e", fk.name) + assert_equal "fk_rails_78146ddd2e", fk.name unless current_adapter?(:SQLite3Adapter) end def test_add_foreign_key_with_column @@ -169,7 +243,7 @@ if ActiveRecord::Base.connection.supports_foreign_keys? assert_equal "rockets", fk.to_table assert_equal "rocket_id", fk.column assert_equal "id", fk.primary_key - assert_equal("fk_rails_78146ddd2e", fk.name) + assert_equal "fk_rails_78146ddd2e", fk.name unless current_adapter?(:SQLite3Adapter) end def test_add_foreign_key_with_non_standard_primary_key @@ -188,7 +262,7 @@ if ActiveRecord::Base.connection.supports_foreign_keys? assert_equal "space_shuttles", fk.to_table assert_equal "pk", fk.primary_key ensure - @connection.remove_foreign_key :astronauts, name: "custom_pk" + @connection.remove_foreign_key :astronauts, name: "custom_pk", to_table: "space_shuttles" @connection.drop_table :space_shuttles end @@ -262,6 +336,8 @@ if ActiveRecord::Base.connection.supports_foreign_keys? end def test_foreign_key_exists_by_name + skip if current_adapter?(:SQLite3Adapter) + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", name: "fancy_named_fk" assert @connection.foreign_key_exists?(:astronauts, name: "fancy_named_fk") @@ -293,6 +369,8 @@ if ActiveRecord::Base.connection.supports_foreign_keys? end def test_remove_foreign_key_by_name + skip if current_adapter?(:SQLite3Adapter) + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", name: "fancy_named_fk" assert_equal 1, @connection.foreign_keys("astronauts").size @@ -301,9 +379,10 @@ if ActiveRecord::Base.connection.supports_foreign_keys? end def test_remove_foreign_non_existing_foreign_key_raises - assert_raises ArgumentError do + e = assert_raises ArgumentError do @connection.remove_foreign_key :astronauts, :rockets end + assert_equal "Table 'astronauts' has no foreign key for rockets", e.message end if ActiveRecord::Base.connection.supports_validate_constraints? @@ -382,7 +461,11 @@ if ActiveRecord::Base.connection.supports_foreign_keys? def test_schema_dumping_with_options output = dump_table_schema "fk_test_has_fk" - assert_match %r{\s+add_foreign_key "fk_test_has_fk", "fk_test_has_pk", column: "fk_id", primary_key: "pk_id", name: "fk_name"$}, output + if current_adapter?(:SQLite3Adapter) + assert_match %r{\s+add_foreign_key "fk_test_has_fk", "fk_test_has_pk", column: "fk_id", primary_key: "pk_id"$}, output + else + assert_match %r{\s+add_foreign_key "fk_test_has_fk", "fk_test_has_pk", column: "fk_id", primary_key: "pk_id", name: "fk_name"$}, output + end end def test_schema_dumping_with_custom_fk_ignore_pattern @@ -436,7 +519,7 @@ if ActiveRecord::Base.connection.supports_foreign_keys? end class CreateSchoolsAndClassesMigration < ActiveRecord::Migration::Current - def change + def up create_table(:schools) create_table(:classes) do |t| @@ -444,6 +527,11 @@ if ActiveRecord::Base.connection.supports_foreign_keys? end add_foreign_key :classes, :schools end + + def down + drop_table :classes, if_exists: true + drop_table :schools, if_exists: true + end end def test_add_foreign_key_with_prefix diff --git a/activerecord/test/cases/migration/references_foreign_key_test.rb b/activerecord/test/cases/migration/references_foreign_key_test.rb index 620e9ab6ca..0d9adcc145 100644 --- a/activerecord/test/cases/migration/references_foreign_key_test.rb +++ b/activerecord/test/cases/migration/references_foreign_key_test.rb @@ -2,7 +2,7 @@ require "cases/helper" -if ActiveRecord::Base.connection.supports_foreign_keys_in_create? +if ActiveRecord::Base.connection.supports_foreign_keys? module ActiveRecord class Migration class ReferencesForeignKeyInCreateTest < ActiveRecord::TestCase @@ -65,9 +65,7 @@ if ActiveRecord::Base.connection.supports_foreign_keys_in_create? end end end -end -if ActiveRecord::Base.connection.supports_foreign_keys? module ActiveRecord class Migration class ReferencesForeignKeyTest < ActiveRecord::TestCase @@ -172,13 +170,18 @@ if ActiveRecord::Base.connection.supports_foreign_keys? end class CreateDogsMigration < ActiveRecord::Migration::Current - def change + def up create_table :dog_owners create_table :dogs do |t| t.references :dog_owner, foreign_key: true end end + + def down + drop_table :dogs, if_exists: true + drop_table :dog_owners, if_exists: true + end end def test_references_foreign_key_with_prefix diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 0ab0459c38..857d743605 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -14,6 +14,7 @@ require "models/person" require "models/computer" require "models/reply" require "models/company" +require "models/contract" require "models/bird" require "models/car" require "models/engine" @@ -1815,6 +1816,12 @@ class RelationTest < ActiveRecord::TestCase assert_equal [1, 1, 1], posts.map(&:author_address_id) end + test "joins with select custom attribute" do + contract = Company.create!(name: "test").contracts.create! + company = Company.joins(:contracts).select(:id, :metadata).find(contract.company_id) + assert_equal contract.metadata, company.metadata + end + test "delegations do not leak to other classes" do Topic.all.by_lifo assert Topic.all.class.method_defined?(:by_lifo) diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 838f515aad..a0f48d23f1 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -13,6 +13,8 @@ class Company < AbstractCompany has_many :contracts has_many :developers, through: :contracts + attribute :metadata, :json + scope :of_first_firm, lambda { joins(account: :firm). where("firms.id" => 1) diff --git a/activerecord/test/models/contract.rb b/activerecord/test/models/contract.rb index 3f663375c4..89719775c4 100644 --- a/activerecord/test/models/contract.rb +++ b/activerecord/test/models/contract.rb @@ -5,7 +5,9 @@ class Contract < ActiveRecord::Base belongs_to :developer, primary_key: :id belongs_to :firm, foreign_key: "company_id" - before_save :hi + attribute :metadata, :json + + before_save :hi, :update_metadata after_save :bye attr_accessor :hi_count, :bye_count @@ -19,6 +21,10 @@ class Contract < ActiveRecord::Base @bye_count ||= 0 @bye_count += 1 end + + def update_metadata + self.metadata = { company_id: company_id, developer_id: developer_id } + end end class NewContract < Contract diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index e32cc59399..65d1fce66d 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -31,6 +31,7 @@ class Post < ActiveRecord::Base belongs_to :author_with_posts, -> { includes(:posts) }, class_name: "Author", foreign_key: :author_id belongs_to :author_with_address, -> { includes(:author_address) }, class_name: "Author", foreign_key: :author_id + belongs_to :author_with_select, -> { select(:id) }, class_name: "Author", foreign_key: :author_id def first_comment super.body diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 86d5a67a13..349b8afc48 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -247,6 +247,7 @@ ActiveRecord::Schema.define do create_table :contracts, force: true do |t| t.references :developer, index: false t.references :company, index: false + t.string :metadata end create_table :customers, force: true do |t| diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 7002474b8d..8bb531f1b8 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,7 @@ +* Revise `ActiveSupport::Notifications.unsubscribe` to correctly handle Regex or other multiple-pattern subscribers. + + *Zach Kemp* + * Add `before_reset` callback to `CurrentAttributes` and define `after_reset` as an alias of `resets` for symmetry. *Rosa Gutierrez* diff --git a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb new file mode 100644 index 0000000000..6892b31a5d --- /dev/null +++ b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module ActiveSupport + module Dependencies + module ZeitwerkIntegration # :nodoc: all + module Decorations + def clear + Dependencies.unload_interlock do + Rails.autoloader.reload + end + end + + def constantize(cpath) + Inflector.constantize(cpath) + end + + def safe_constantize(cpath) + Inflector.safe_constantize(cpath) + end + + def autoloaded_constants + Rails.autoloaders.flat_map do |autoloader| + autoloader.loaded.to_a + end + end + + def autoloaded?(object) + cpath = object.is_a?(Module) ? object.name : object.to_s + Rails.autoloaders.any? { |autoloader| autoloader.loaded?(cpath) } + end + end + + class << self + def take_over + setup_autoloaders + freeze_autoload_paths + decorate_dependencies + end + + private + + def setup_autoloaders + Dependencies.autoload_paths.each do |autoload_path| + if File.directory?(autoload_path) + if autoload_once?(autoload_path) + Rails.once_autoloader.push_dir(autoload_path) + else + Rails.autoloader.push_dir(autoload_path) + end + end + end + + Rails.autoloaders.each(&:setup) + end + + def autoload_once?(autoload_path) + Dependencies.autoload_once_paths.include?(autoload_path) || + Gem.path.any? { |gem_path| autoload_path.to_s.start_with?(gem_path) } + end + + def freeze_autoload_paths + Dependencies.autoload_paths.freeze + Dependencies.autoload_once_paths.freeze + end + + def decorate_dependencies + Dependencies.singleton_class.prepend(Decorations) + Object.class_eval { alias_method :require_dependency, :require } + end + end + end + end +end diff --git a/activesupport/lib/active_support/notifications.rb b/activesupport/lib/active_support/notifications.rb index 7ccc333463..d9e93b530c 100644 --- a/activesupport/lib/active_support/notifications.rb +++ b/activesupport/lib/active_support/notifications.rb @@ -153,6 +153,15 @@ module ActiveSupport # # ActiveSupport::Notifications.unsubscribe("render") # + # Subscribers using a regexp or other pattern-matching object will remain subscribed + # to all events that match their original pattern, unless those events match a string + # passed to `unsubscribe`: + # + # subscriber = ActiveSupport::Notifications.subscribe(/render/) { } + # ActiveSupport::Notifications.unsubscribe('render_template.action_view') + # subscriber.matches?('render_template.action_view') # => false + # subscriber.matches?('render_partial.action_view') # => true + # # == Default Queue # # Notifications ships with a queue implementation that consumes and publishes events diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index 11721db103..f06504cf2c 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -2,6 +2,7 @@ require "mutex_m" require "concurrent/map" +require "set" module ActiveSupport module Notifications @@ -39,6 +40,7 @@ module ActiveSupport when String @string_subscribers[subscriber_or_name].clear @listeners_for.delete(subscriber_or_name) + @other_subscribers.each { |sub| sub.unsubscribe!(subscriber_or_name) } else pattern = subscriber_or_name.try(:pattern) if String === pattern @@ -113,11 +115,33 @@ module ActiveSupport end end + class Matcher #:nodoc: + attr_reader :pattern, :exclusions + + def self.wrap(pattern) + return pattern if String === pattern + new(pattern) + end + + def initialize(pattern) + @pattern = pattern + @exclusions = Set.new + end + + def unsubscribe!(name) + exclusions << -name if pattern === name + end + + def ===(name) + pattern === name && !exclusions.include?(name) + end + end + class Evented #:nodoc: attr_reader :pattern def initialize(pattern, delegate) - @pattern = pattern + @pattern = Matcher.wrap(pattern) @delegate = delegate @can_publish = delegate.respond_to?(:publish) end @@ -137,11 +161,15 @@ module ActiveSupport end def subscribed_to?(name) - @pattern === name + pattern === name end def matches?(name) - @pattern && @pattern === name + pattern && pattern === name + end + + def unsubscribe!(name) + pattern.unsubscribe!(name) end end @@ -204,6 +232,10 @@ module ActiveSupport true end + def unsubscribe!(*) + false + end + alias :matches? :=== end end diff --git a/activesupport/test/notifications/evented_notification_test.rb b/activesupport/test/notifications/evented_notification_test.rb index 4beb8194b9..ab2a9b8659 100644 --- a/activesupport/test/notifications/evented_notification_test.rb +++ b/activesupport/test/notifications/evented_notification_test.rb @@ -84,6 +84,39 @@ module ActiveSupport [:finish, "hi", 1, {}] ], listener.events end + + def test_listen_to_regexp + notifier = Fanout.new + listener = Listener.new + notifier.subscribe(/[a-z]*.world/, listener) + notifier.start("hi.world", 1, {}) + notifier.finish("hi.world", 2, {}) + notifier.start("hello.world", 1, {}) + notifier.finish("hello.world", 2, {}) + + assert_equal [ + [:start, "hi.world", 1, {}], + [:finish, "hi.world", 2, {}], + [:start, "hello.world", 1, {}], + [:finish, "hello.world", 2, {}] + ], listener.events + end + + def test_listen_to_regexp_with_exclusions + notifier = Fanout.new + listener = Listener.new + notifier.subscribe(/[a-z]*.world/, listener) + notifier.unsubscribe("hi.world") + notifier.start("hi.world", 1, {}) + notifier.finish("hi.world", 2, {}) + notifier.start("hello.world", 1, {}) + notifier.finish("hello.world", 2, {}) + + assert_equal [ + [:start, "hello.world", 1, {}], + [:finish, "hello.world", 2, {}] + ], listener.events + end end end end diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index b5d72d1a42..bb20d26a25 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -128,6 +128,25 @@ module Notifications assert_equal [["named.subscription", :foo], ["named.subscription", :foo]], @events end + def test_unsubscribing_by_name_leaves_regexp_matched_subscriptions + @matched_events = [] + @notifier.subscribe(/subscription/) { |*args| @matched_events << event(*args) } + @notifier.publish("named.subscription", :before) + @notifier.wait + [@events, @named_events, @matched_events].each do |collector| + assert_includes(collector, ["named.subscription", :before]) + end + @notifier.unsubscribe("named.subscription") + @notifier.publish("named.subscription", :after) + @notifier.publish("other.subscription", :after) + @notifier.wait + assert_includes(@events, ["named.subscription", :after]) + assert_includes(@events, ["other.subscription", :after]) + assert_includes(@matched_events, ["other.subscription", :after]) + assert_not_includes(@matched_events, ["named.subscription", :after]) + assert_not_includes(@named_events, ["named.subscription", :after]) + end + private def event(*args) args diff --git a/guides/source/action_cable_overview.md b/guides/source/action_cable_overview.md index 8f5c44849a..7a9ff96c44 100644 --- a/guides/source/action_cable_overview.md +++ b/guides/source/action_cable_overview.md @@ -671,6 +671,21 @@ To configure the URL, add a call to `action_cable_meta_tag` in your HTML layout HEAD. This uses a URL or path typically set via `config.action_cable.url` in the environment configuration files. +### Worker Pool Configuration + +The worker pool is used to run connection callbacks and channel actions in +isolation from the server's main thread. Action Cable allows the application +to configure the number of simultaneously processed threads in the worker pool. + +```ruby +config.action_cable.worker_pool_size = 4 +``` + +Also, note that your server must provide at least the same number of database +connections as you have workers. The default worker pool size is set to 4, so +that means you have to make at least 4 database connections available. + You can change that in `config/database.yml` through the `pool` attribute. + ### Other Configurations The other common option to configure is the log tags applied to the @@ -688,11 +703,6 @@ config.action_cable.log_tags = [ For a full list of all configuration options, see the `ActionCable::Server::Configuration` class. -Also, note that your server must provide at least the same number of database -connections as you have workers. The default worker pool size is set to 4, so -that means you have to make at least that available. You can change that in -`config/database.yml` through the `pool` attribute. - ## Running Standalone Cable Servers ### In App diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index aa746e4731..d0d84251e4 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -212,7 +212,7 @@ class PeopleController < ActionController::Base end # This will pass with flying colors as long as there's a person key - # in the parameters, otherwise it'll raise a + # in the parameters, otherwise it'll raise an # ActionController::ParameterMissing exception, which will get # caught by ActionController::Base and turned into a 400 Bad # Request error. diff --git a/guides/source/active_record_migrations.md b/guides/source/active_record_migrations.md index 905c76e5c1..2c1796c464 100644 --- a/guides/source/active_record_migrations.md +++ b/guides/source/active_record_migrations.md @@ -496,9 +496,6 @@ NOTE: Active Record only supports single column foreign keys. `execute` and `structure.sql` are required to use composite foreign keys. See [Schema Dumping and You](#schema-dumping-and-you). -NOTE: The SQLite3 adapter doesn't support `add_foreign_key` since SQLite supports -only [a limited subset of ALTER TABLE](https://www.sqlite.org/lang_altertable.html). - Removing a foreign key is easy as well: ```ruby diff --git a/guides/source/asset_pipeline.md b/guides/source/asset_pipeline.md index e7faa5c330..9f8edea598 100644 --- a/guides/source/asset_pipeline.md +++ b/guides/source/asset_pipeline.md @@ -745,7 +745,7 @@ mapping requests back to Sprockets. A typical manifest file looks like: The default location for the manifest is the root of the location specified in `config.assets.prefix` ('/assets' by default). -NOTE: If there are missing precompiled files in production you will get an +NOTE: If there are missing precompiled files in production you will get a `Sprockets::Helpers::RailsHelper::AssetPaths::AssetNotPrecompiledError` exception indicating the name of the missing file(s). diff --git a/guides/source/configuring.md b/guides/source/configuring.md index e7129dd274..ad82614a0d 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -791,6 +791,9 @@ main application. You can set this as nil to not mount Action Cable as part of your normal Rails server. +You can find more detailed configuration options in the +[Action Cable Overview](action_cable_overview.html#configuration). + ### Configuring Active Storage diff --git a/package.json b/package.json index 43055fa6bc..e5c71777ba 100644 --- a/package.json +++ b/package.json @@ -4,10 +4,7 @@ "actioncable", "actiontext", "activestorage", - "actionview", - "tmp/templates/app_template", - "railties/test/fixtures/tmp/bukkits/**/test/dummy", - "railties/test/fixtures/tmp/bukkits/**/spec/dummy" + "actionview" ], "dependencies": { "webpack": "^4.17.1" diff --git a/railties/.gitignore b/railties/.gitignore index c08562e016..17a49da08c 100644 --- a/railties/.gitignore +++ b/railties/.gitignore @@ -2,4 +2,5 @@ /test/500.html /test/fixtures/tmp/ /test/initializer/root/log/ +/test/isolation/assets/yarn.lock /tmp/ diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 19f4de8a1d..d66add2ca0 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,8 @@ +* Fix non-symbol access to nested hashes returned from `Rails::Application.config_for` + being broken by allowing non-symbol access with a deprecation notice. + + *Ufuk Kayserilioglu* + * Fix deeply nested namespace command printing. *Gannon McGibbon* diff --git a/railties/Rakefile b/railties/Rakefile index 1c4725e2d6..4ae546b202 100644 --- a/railties/Rakefile +++ b/railties/Rakefile @@ -38,7 +38,7 @@ namespace :test do test_patterns = dirs.map { |dir| "test/#{dir}/*_test.rb" } test_files = Dir[*test_patterns].select do |file| - !file.start_with?("test/fixtures/") + !file.start_with?("test/fixtures/") && !file.start_with?("test/isolation/assets/") end.sort if ENV["BUILDKITE_PARALLEL_JOB_COUNT"] diff --git a/railties/lib/rails.rb b/railties/lib/rails.rb index 092105d502..bca2cf34e1 100644 --- a/railties/lib/rails.rb +++ b/railties/lib/rails.rb @@ -110,5 +110,25 @@ module Rails def public_path application && Pathname.new(application.paths["public"].first) end + + def autoloader + if configuration.autoloader == :zeitwerk + @autoloader ||= Zeitwerk::Loader.new + end + end + + def once_autoloader + if configuration.autoloader == :zeitwerk + @once_autoloader ||= Zeitwerk::Loader.new + end + end + + def autoloaders + if configuration.autoloader == :zeitwerk + [autoloader, once_autoloader] + else + [] + end + end end end diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 5a924ab8e6..d0417f8a49 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -7,6 +7,7 @@ require "active_support/key_generator" require "active_support/message_verifier" require "active_support/encrypted_configuration" require "active_support/deprecation" +require "active_support/hash_with_indifferent_access" require "rails/engine" require "rails/secrets" @@ -230,8 +231,8 @@ module Rails config = YAML.load(ERB.new(yaml.read).result) || {} config = (config["shared"] || {}).merge(config[env] || {}) - ActiveSupport::OrderedOptions.new.tap do |config_as_ordered_options| - config_as_ordered_options.update(config.deep_symbolize_keys) + ActiveSupport::OrderedOptions.new.tap do |options| + options.update(NonSymbolAccessDeprecatedHash.new(config)) end else raise "Could not load configuration. No such file - #{yaml}" @@ -590,5 +591,38 @@ module Rails def build_middleware config.app_middleware + super end + + class NonSymbolAccessDeprecatedHash < HashWithIndifferentAccess # :nodoc: + def initialize(value = nil) + if value.is_a?(Hash) + value.each_pair { |k, v| self[k] = v } + else + super + end + end + + def []=(key, value) + if value.is_a?(Hash) + value = self.class.new(value) + end + super(key.to_sym, value) + end + + private + + def convert_key(key) + unless key.kind_of?(Symbol) + ActiveSupport::Deprecation.warn(<<~MESSAGE.squish) + Accessing hashes returned from config_for by non-symbol keys + is deprecated and will be removed in Rails 6.1. + Use symbols for access instead. + MESSAGE + + key = key.to_sym + end + + key + end + end end end diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index b7838f7e32..16fbc99e7a 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -20,7 +20,7 @@ module Rails :read_encrypted_secrets, :log_level, :content_security_policy_report_only, :content_security_policy_nonce_generator, :require_master_key, :credentials - attr_reader :encoding, :api_only, :loaded_config_version + attr_reader :encoding, :api_only, :loaded_config_version, :autoloader def initialize(*) super @@ -64,6 +64,7 @@ module Rails @credentials = ActiveSupport::OrderedOptions.new @credentials.content_path = default_credentials_content_path @credentials.key_path = default_credentials_key_path + @autoloader = :classic end def load_defaults(target_version) @@ -117,6 +118,8 @@ module Rails when "6.0" load_defaults "5.2" + self.autoloader = :zeitwerk if RUBY_ENGINE == "ruby" + if respond_to?(:action_view) action_view.default_enforce_utf8 = false end @@ -267,6 +270,14 @@ module Rails end end + def autoloader=(autoloader) + if %i(classic zeitwerk).include?(autoloader) + @autoloader = autoloader + else + raise ArgumentError, "config.autoloader may be :classic or :zeitwerk, got #{autoloader.inspect} instead" + end + end + class Custom #:nodoc: def initialize @configurations = Hash.new diff --git a/railties/lib/rails/application/finisher.rb b/railties/lib/rails/application/finisher.rb index 04aaf6dd9a..39e8ef6631 100644 --- a/railties/lib/rails/application/finisher.rb +++ b/railties/lib/rails/application/finisher.rb @@ -21,6 +21,13 @@ module Rails end end + initializer :let_zeitwerk_take_over do + if config.autoloader == :zeitwerk + require "active_support/dependencies/zeitwerk_integration" + ActiveSupport::Dependencies::ZeitwerkIntegration.take_over + end + end + initializer :add_builtin_route do |app| if Rails.env.development? app.routes.prepend do @@ -66,6 +73,7 @@ module Rails initializer :eager_load! do if config.eager_load ActiveSupport.run_load_hooks(:before_eager_load, self) + Zeitwerk::Loader.eager_load_all if defined?(Zeitwerk) config.eager_load_namespaces.each(&:eager_load!) end end diff --git a/railties/lib/rails/command/behavior.rb b/railties/lib/rails/command/behavior.rb index 7f32b04cf1..7fb2a99e67 100644 --- a/railties/lib/rails/command/behavior.rb +++ b/railties/lib/rails/command/behavior.rb @@ -71,8 +71,9 @@ module Rails paths = [] namespaces.each do |namespace| pieces = namespace.split(":") - paths << pieces.dup.push(pieces.last).join("/") - paths << pieces.join("/") + path = pieces.join("/") + paths << "#{path}/#{pieces.last}" + paths << path end paths.uniq! paths diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index d6c329b581..2485158a7b 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -472,12 +472,10 @@ module Rails # Eager load the application by loading all ruby # files inside eager_load paths. def eager_load! - 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 + if Rails.autoloader + eager_load_with_zeitwerk! + else + eager_load_with_dependencies! end end @@ -653,6 +651,22 @@ 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/rails/app/templates/Gemfile.tt b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt index d39b5d311f..a1f1224a45 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt @@ -28,7 +28,7 @@ ruby <%= "'#{RUBY_VERSION}'" -%> <% if depend_on_bootsnap? -%> # Reduces boot times through caching; required in config/boot.rb -gem 'bootsnap', '>= 1.1.0', require: false +gem 'bootsnap', '>= 1.4.0', require: false <%- end -%> <%- if options.api? -%> @@ -36,7 +36,9 @@ gem 'bootsnap', '>= 1.1.0', require: false # gem 'rack-cors' <%- end -%> -<% if RUBY_ENGINE == 'ruby' -%> +<% if RUBY_ENGINE == "ruby" -%> +gem "zeitwerk", ">= 1.0.0" + group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console gem 'byebug', platforms: [:mri, :mingw, :x64_mingw] diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 37fba72ee3..960f708bdf 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1157,6 +1157,27 @@ module ApplicationTests end end + test "autoloader & autoloader=" do + app "development" + + config = Rails.application.config + assert_instance_of Zeitwerk::Loader, Rails.autoloader + assert_instance_of Zeitwerk::Loader, Rails.once_autoloader + assert_equal [Rails.autoloader, Rails.once_autoloader], Rails.autoloaders + + config.autoloader = :classic + assert_nil Rails.autoloader + assert_nil Rails.once_autoloader + assert_empty Rails.autoloaders + + config.autoloader = :zeitwerk + assert_instance_of Zeitwerk::Loader, Rails.autoloader + assert_instance_of Zeitwerk::Loader, Rails.once_autoloader + assert_equal [Rails.autoloader, Rails.once_autoloader], Rails.autoloaders + + assert_raises(ArgumentError) { config.autoloader = :unknown } + end + test "config.action_view.cache_template_loading with cache_classes default" do add_to_config "config.cache_classes = true" @@ -1714,7 +1735,7 @@ module ApplicationTests assert_equal true, Rails.application.config.action_mailer.show_previews end - test "config_for loads custom configuration from yaml accessible as symbol" do + test "config_for loads custom configuration from yaml accessible as symbol or string" do app_file "config/custom.yml", <<-RUBY development: foo: 'bar' @@ -1727,6 +1748,7 @@ module ApplicationTests app "development" assert_equal "bar", Rails.application.config.my_custom_config[:foo] + assert_equal "bar", Rails.application.config.my_custom_config["foo"] end test "config_for loads nested custom configuration from yaml as symbol keys" do @@ -1746,6 +1768,25 @@ module ApplicationTests assert_equal 1, Rails.application.config.my_custom_config[:foo][:bar][:baz] end + test "config_for loads nested custom configuration from yaml with deprecated non-symbol access" do + app_file "config/custom.yml", <<-RUBY + development: + foo: + bar: + baz: 1 + RUBY + + add_to_config <<-RUBY + config.my_custom_config = config_for('custom') + RUBY + + app "development" + + assert_deprecated do + assert_equal 1, Rails.application.config.my_custom_config["foo"]["bar"]["baz"] + end + end + test "config_for makes all hash methods available" do app_file "config/custom.yml", <<-RUBY development: @@ -1762,12 +1803,93 @@ module ApplicationTests actual = Rails.application.config.my_custom_config - assert_equal actual, foo: 0, bar: { baz: 1 } - assert_equal actual.keys, [ :foo, :bar ] - assert_equal actual.values, [ 0, baz: 1] - assert_equal actual.to_h, foo: 0, bar: { baz: 1 } - assert_equal actual[:foo], 0 - assert_equal actual[:bar], baz: 1 + assert_equal({ foo: 0, bar: { baz: 1 } }, actual) + assert_equal([ :foo, :bar ], actual.keys) + assert_equal([ 0, baz: 1], actual.values) + assert_equal({ foo: 0, bar: { baz: 1 } }, actual.to_h) + assert_equal(0, actual[:foo]) + assert_equal({ baz: 1 }, actual[:bar]) + end + + test "config_for generates deprecation notice when nested hash methods are called with non-symbols" do + app_file "config/custom.yml", <<-RUBY + development: + foo: + bar: 1 + baz: 2 + qux: + boo: 3 + RUBY + + app "development" + + actual = Rails.application.config_for("custom")[:foo] + + # slice + assert_deprecated do + assert_equal({ bar: 1, baz: 2 }, actual.slice("bar", "baz")) + end + + # except + assert_deprecated do + assert_equal({ qux: { boo: 3 } }, actual.except("bar", "baz")) + end + + # dig + assert_deprecated do + assert_equal(3, actual.dig("qux", "boo")) + end + + # fetch - hit + assert_deprecated do + assert_equal(1, actual.fetch("bar", 0)) + end + + # fetch - miss + assert_deprecated do + assert_equal(0, actual.fetch("does-not-exist", 0)) + end + + # fetch_values + assert_deprecated do + assert_equal([1, 2], actual.fetch_values("bar", "baz")) + end + + # key? - hit + assert_deprecated do + assert(actual.key?("bar")) + end + + # key? - miss + assert_deprecated do + assert_not(actual.key?("does-not-exist")) + end + + # slice! + actual = Rails.application.config_for("custom")[:foo] + + assert_deprecated do + slice = actual.slice!("bar", "baz") + assert_equal({ bar: 1, baz: 2 }, actual) + assert_equal({ qux: { boo: 3 } }, slice) + end + + # extract! + actual = Rails.application.config_for("custom")[:foo] + + assert_deprecated do + extracted = actual.extract!("bar", "baz") + assert_equal({ bar: 1, baz: 2 }, extracted) + assert_equal({ qux: { boo: 3 } }, actual) + end + + # except! + actual = Rails.application.config_for("custom")[:foo] + + assert_deprecated do + actual.except!("bar", "baz") + assert_equal({ qux: { boo: 3 } }, actual) + end end test "config_for uses the Pathname object if it is provided" do diff --git a/railties/test/application/mailer_previews_test.rb b/railties/test/application/mailer_previews_test.rb index ba186bda44..fb84276b8a 100644 --- a/railties/test/application/mailer_previews_test.rb +++ b/railties/test/application/mailer_previews_test.rb @@ -85,6 +85,7 @@ module ApplicationTests end test "mailer previews are loaded from a custom preview_path" do + app_dir "lib/mailer_previews" add_to_config "config.action_mailer.preview_path = '#{app_path}/lib/mailer_previews'" mailer "notifier", <<-RUBY @@ -254,6 +255,7 @@ module ApplicationTests end test "mailer previews are reloaded from a custom preview_path" do + app_dir "lib/mailer_previews" add_to_config "config.action_mailer.preview_path = '#{app_path}/lib/mailer_previews'" app("development") @@ -818,6 +820,7 @@ module ApplicationTests def build_app super app_file "config/routes.rb", "Rails.application.routes.draw do; end" + app_dir "test/mailers/previews" end def mailer(name, contents) diff --git a/railties/test/application/multiple_applications_test.rb b/railties/test/application/multiple_applications_test.rb index 432344bccc..f0f1112f6b 100644 --- a/railties/test/application/multiple_applications_test.rb +++ b/railties/test/application/multiple_applications_test.rb @@ -100,30 +100,6 @@ module ApplicationTests assert_nothing_raised { AppTemplate::Application.new } end - def test_initializers_run_on_different_applications_go_to_the_same_class - application1 = AppTemplate::Application.new - run_count = 0 - - AppTemplate::Application.initializer :init0 do - run_count += 1 - end - - application1.initializer :init1 do - run_count += 1 - end - - AppTemplate::Application.new.initializer :init2 do - run_count += 1 - end - - assert_equal 0, run_count, "Without loading the initializers, the count should be 0" - - # Set config.eager_load to false so that an eager_load warning doesn't pop up - AppTemplate::Application.create { config.eager_load = false }.initialize! - - assert_equal 3, run_count, "There should have been three initializers that incremented the count" - end - def test_consoles_run_on_different_applications_go_to_the_same_class run_count = 0 AppTemplate::Application.console { run_count += 1 } diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index 44e3b0f66b..fe56e3d076 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -145,8 +145,8 @@ module ApplicationTests # loading a specific fixture rails "db:fixtures:load", "FIXTURES=products" - assert_equal 2, ::AppTemplate::Application::Product.count - assert_equal 0, ::AppTemplate::Application::User.count + assert_equal 2, Product.count + assert_equal 0, User.count end def test_loading_only_yml_fixtures diff --git a/railties/test/application/zeitwerk_integration_test.rb b/railties/test/application/zeitwerk_integration_test.rb new file mode 100644 index 0000000000..628a85acd8 --- /dev/null +++ b/railties/test/application/zeitwerk_integration_test.rb @@ -0,0 +1,164 @@ +# frozen_string_literal: true + +require "isolation/abstract_unit" +require "active_support/dependencies/zeitwerk_integration" + +class ZeitwerkIntegrationTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + + def setup + build_app + end + + def boot(env = "development") + app(env) + end + + def teardown + teardown_app + end + + def deps + ActiveSupport::Dependencies + end + + def decorated? + deps.singleton_class < deps::ZeitwerkIntegration::Decorations + end + + test "ActiveSupport::Dependencies is decorated by default" do + boot + + assert decorated? + assert_instance_of Zeitwerk::Loader, Rails.autoloader + assert_instance_of Zeitwerk::Loader, Rails.once_autoloader + assert_equal [Rails.autoloader, Rails.once_autoloader], Rails.autoloaders + end + + test "ActiveSupport::Dependencies is not decorated in classic mode" do + add_to_config "config.autoloader = :classic" + boot + + assert_not decorated? + assert_nil Rails.autoloader + assert_nil Rails.once_autoloader + assert_empty Rails.autoloaders + end + + test "constantize returns the value stored in the constant" do + app_file "app/models/admin/user.rb", "class Admin::User; end" + boot + + assert_same Admin::User, deps.constantize("Admin::User") + end + + test "constantize raises if the constant is unknown" do + boot + + assert_raises(NameError) { deps.constantize("Admin") } + end + + test "safe_constantize returns the value stored in the constant" do + app_file "app/models/admin/user.rb", "class Admin::User; end" + boot + + assert_same Admin::User, deps.safe_constantize("Admin::User") + end + + test "safe_constantize returns nil for unknown constants" do + boot + + assert_nil deps.safe_constantize("Admin") + end + + test "autoloaded_constants returns autoloaded constant paths" do + app_file "app/models/admin/user.rb", "class Admin::User; end" + app_file "app/models/post.rb", "class Post; end" + boot + + assert Admin::User + assert_equal ["Admin", "Admin::User"], deps.autoloaded_constants + end + + test "autoloaded? says if a constant has been autoloaded" do + app_file "app/models/user.rb", "class User; end" + app_file "app/models/post.rb", "class Post; end" + boot + + assert Post + assert deps.autoloaded?("Post") + assert deps.autoloaded?(Post) + assert_not deps.autoloaded?("User") + end + + test "eager loading loads the application code" do + $zeitwerk_integration_test_user = false + $zeitwerk_integration_test_post = false + + 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 anything managed by Zeitwerk" do + $zeitwerk_integration_test_user = false + app_file "app/models/user.rb", "class User; end; $zeitwerk_integration_test_user = true" + + $zeitwerk_integration_test_extras = false + app_dir "extras" + app_file "extras/webhook_hacks.rb", "WebhookHacks = 1; $zeitwerk_integration_test_extras = true" + + require "zeitwerk" + autoloader = Zeitwerk::Loader.new + autoloader.push_dir("#{app_path}/extras") + autoloader.setup + + boot("production") + + assert $zeitwerk_integration_test_user + assert $zeitwerk_integration_test_extras + end + + test "autoload paths that are below Gem.path go to the once autoloader" do + app_dir "extras" + add_to_config 'config.autoload_paths << "#{Rails.root}/extras"' + + # Mocks Gem.path to include the extras directory. + Gem.singleton_class.prepend( + Module.new do + def path + super + ["#{Rails.root}/extras"] + end + end + ) + boot + + assert_not_includes Rails.autoloader.dirs, "#{app_path}/extras" + assert_includes Rails.once_autoloader.dirs, "#{app_path}/extras" + end + + test "clear reloads the main autoloader, and does not reload the once one" do + boot + + $zeitwerk_integration_reload_test = [] + + autoloader = Rails.autoloader + def autoloader.reload + $zeitwerk_integration_reload_test << :autoloader + super + end + + once_autoloader = Rails.once_autoloader + def once_autoloader.reload + $zeitwerk_integration_reload_test << :once_autoloader + super + end + + ActiveSupport::Dependencies.clear + + assert_equal %i(autoloader), $zeitwerk_integration_reload_test + end +end diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 1ee9e43e89..937b8eb427 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -660,6 +660,15 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_gem "jbuilder" end + def test_inclusion_of_zeitwerk + run_generator + if RUBY_ENGINE == "ruby" + assert_gem "zeitwerk" + else + assert_no_gem "zeitwerk" + end + end + def test_inclusion_of_a_debugger run_generator if defined?(JRUBY_VERSION) || RUBY_ENGINE == "rbx" diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 0ff4c03f4a..47d42645c6 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -421,6 +421,10 @@ module TestHelpers file_name end + def app_dir(path) + FileUtils.mkdir_p("#{app_path}/#{path}") + end + def remove_file(path) FileUtils.rm_rf "#{app_path}/#{path}" end @@ -472,42 +476,26 @@ Module.new do FileUtils.rm_rf(app_template_path) FileUtils.mkdir_p(app_template_path) - Dir.chdir "#{RAILS_FRAMEWORK_ROOT}/actionview" do - `yarn build` - end - - `#{Gem.ruby} #{RAILS_FRAMEWORK_ROOT}/railties/exe/rails new #{app_template_path} --skip-bundle --skip-listen --no-rc` + `#{Gem.ruby} #{RAILS_FRAMEWORK_ROOT}/railties/exe/rails new #{app_template_path} --skip-bundle --skip-listen --no-rc --skip-webpack-install` File.open("#{app_template_path}/config/boot.rb", "w") do |f| f.puts "require 'rails/all'" end - Dir.chdir(app_template_path) { `yarn add https://github.com/rails/webpacker.git` } # Use the latest version. - - # Manually install `webpack` as bin symlinks are not created for sub dependencies - # in workspaces. See https://github.com/yarnpkg/yarn/issues/4964 - Dir.chdir(app_template_path) { `yarn add webpack@4.17.1 --tilde` } - Dir.chdir(app_template_path) { `yarn add webpack-cli` } - - # FIXME: Temporary fix for webpack + ajv@6.9.0 compatible issue. - # See https://github.com/epoberezkin/ajv/issues/941 - Dir.chdir(app_template_path) do - package = File.read("package.json") - resolutions = <<~EOS - , - "resolutions": { - "ajv": "6.8.1" - } - } - EOS - if package =~ /\n}\n\z/ - File.open("package.json", "w") { |f| f.puts $` + resolutions + $' } - end - end + assets_path = "#{RAILS_FRAMEWORK_ROOT}/railties/test/isolation/assets" + FileUtils.cp("#{assets_path}/package.json", "#{app_template_path}/package.json") + FileUtils.cp("#{assets_path}/config/webpacker.yml", "#{app_template_path}/config/webpacker.yml") + FileUtils.cp_r("#{assets_path}/config/webpack", "#{app_template_path}/config/webpack") + FileUtils.ln_s("#{assets_path}/node_modules", "#{app_template_path}/node_modules") + FileUtils.chdir(app_template_path) { `bin/rails webpacker:binstubs` } # Fake 'Bundler.require' -- we run using the repo's Gemfile, not an # app-specific one: we don't want to require every gem that lists. contents = File.read("#{app_template_path}/config/application.rb") - contents.sub!(/^Bundler\.require.*/, "%w(turbolinks webpacker).each { |r| require r }") + if RUBY_ENGINE == "ruby" + contents.sub!(/^Bundler\.require.*/, "%w(turbolinks webpacker zeitwerk).each { |r| require r }") + else + contents.sub!(/^Bundler\.require.*/, "%w(turbolinks webpacker).each { |r| require r }") + end File.write("#{app_template_path}/config/application.rb", contents) require "rails" diff --git a/railties/test/isolation/assets/config/webpack/development.js b/railties/test/isolation/assets/config/webpack/development.js new file mode 100644 index 0000000000..395290f431 --- /dev/null +++ b/railties/test/isolation/assets/config/webpack/development.js @@ -0,0 +1,3 @@ +process.env.NODE_ENV = process.env.NODE_ENV || 'development' +const { environment } = require('@rails/webpacker') +module.exports = environment.toWebpackConfig() diff --git a/railties/test/isolation/assets/config/webpack/production.js b/railties/test/isolation/assets/config/webpack/production.js new file mode 100644 index 0000000000..d064a6a7fb --- /dev/null +++ b/railties/test/isolation/assets/config/webpack/production.js @@ -0,0 +1,3 @@ +process.env.NODE_ENV = process.env.NODE_ENV || 'production' +const { environment } = require('@rails/webpacker') +module.exports = environment.toWebpackConfig() diff --git a/railties/test/isolation/assets/config/webpack/test.js b/railties/test/isolation/assets/config/webpack/test.js new file mode 100644 index 0000000000..395290f431 --- /dev/null +++ b/railties/test/isolation/assets/config/webpack/test.js @@ -0,0 +1,3 @@ +process.env.NODE_ENV = process.env.NODE_ENV || 'development' +const { environment } = require('@rails/webpacker') +module.exports = environment.toWebpackConfig() diff --git a/railties/test/isolation/assets/config/webpacker.yml b/railties/test/isolation/assets/config/webpacker.yml new file mode 100644 index 0000000000..0b1f43a407 --- /dev/null +++ b/railties/test/isolation/assets/config/webpacker.yml @@ -0,0 +1,8 @@ +default: &default + check_yarn_integrity: false +development: + <<: *default +test: + <<: *default +production: + <<: *default diff --git a/railties/test/isolation/assets/package.json b/railties/test/isolation/assets/package.json new file mode 100644 index 0000000000..7c34450fe0 --- /dev/null +++ b/railties/test/isolation/assets/package.json @@ -0,0 +1,11 @@ +{ + "name": "dummy", + "private": true, + "dependencies": { + "@rails/actioncable": "file:../../../../actioncable", + "@rails/activestorage": "file:../../../../activestorage", + "@rails/ujs": "file:../../../../actionview", + "@rails/webpacker": "https://github.com/rails/webpacker.git", + "turbolinks": "^5.2.0" + } +} diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index 851407dede..69f6e34d58 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -704,25 +704,27 @@ YAML RUBY @plugin.write "app/controllers/bukkits/foo_controller.rb", <<-RUBY - class Bukkits::FooController < ActionController::Base - def index - render inline: "<%= help_the_engine %>" - end + module Bukkits + class FooController < ActionController::Base + def index + render inline: "<%= help_the_engine %>" + end - def show - render plain: foo_path - end + def show + render plain: foo_path + end - def from_app - render inline: "<%= (self.respond_to?(:bar_path) || self.respond_to?(:something)) %>" - end + def from_app + render inline: "<%= (self.respond_to?(:bar_path) || self.respond_to?(:something)) %>" + end - def routes_helpers_in_view - render inline: "<%= foo_path %>, <%= main_app.bar_path %>" - end + def routes_helpers_in_view + render inline: "<%= foo_path %>, <%= main_app.bar_path %>" + end - def polymorphic_path_without_namespace - render plain: polymorphic_path(Post.new) + def polymorphic_path_without_namespace + render plain: polymorphic_path(Post.new) + end end end RUBY |