diff options
70 files changed, 862 insertions, 337 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' 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/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/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index c3bb0a49fc..61fe11cf45 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -58,22 +58,32 @@ module ActionView alias :eql? :equal? @details_keys = Concurrent::Map.new + @digest_cache = Concurrent::Map.new - def self.get(details) + def self.digest_cache(details) + @digest_cache[details_cache_key(details)] ||= Concurrent::Map.new + end + + def self.details_cache_key(details) if details[:formats] details = details.dup details[:formats] &= Template::Types.symbols end - @details_keys[details] ||= Concurrent::Map.new + @details_keys[details] ||= Object.new end def self.clear + ActionView::ViewPaths.all_view_paths.each do |path_set| + path_set.each(&:clear_cache) + end + ActionView::LookupContext.fallbacks.each(&:clear_cache) @view_context_class = nil @details_keys.clear + @digest_cache.clear end def self.digest_caches - @details_keys.values + @digest_cache.values end def self.view_context_class(klass) @@ -88,7 +98,7 @@ module ActionView # Calculate the details key. Remove the handlers from calculation to improve performance # since the user cannot modify it explicitly. def details_key #:nodoc: - @details_key ||= DetailsKey.get(@details) if @cache + @details_key ||= DetailsKey.details_cache_key(@details) if @cache end # Temporary skip passing the details_key forward. @@ -102,7 +112,8 @@ module ActionView private def _set_detail(key, value) # :doc: - @details = @details.dup if @details_key + @details = @details.dup if @digest_cache || @details_key + @digest_cache = nil @details_key = nil @details[key] = value end @@ -178,7 +189,7 @@ module ActionView user_details = @details.merge(options) if @cache - details_key = DetailsKey.get(user_details) + details_key = DetailsKey.details_cache_key(user_details) else details_key = nil end @@ -205,7 +216,7 @@ module ActionView end if @cache - [details, DetailsKey.get(details)] + [details, DetailsKey.details_cache_key(details)] else [details, nil] end @@ -236,6 +247,7 @@ module ActionView def initialize(view_paths, details = {}, prefixes = []) @details_key = nil + @digest_cache = nil @cache = true @prefixes = prefixes @rendered_format = nil @@ -245,7 +257,7 @@ module ActionView end def digest_cache - details_key + @digest_cache ||= DetailsKey.digest_cache(@details) end def initialize_details(target, details) diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index cf7d1105e0..da92ce1f5e 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -87,6 +87,7 @@ module ActionView # Returns an object that is able to render templates. def view_renderer # :nodoc: + # Lifespan: Per controller @_view_renderer ||= ActionView::Renderer.new(lookup_context) end diff --git a/actionview/lib/action_view/view_paths.rb b/actionview/lib/action_view/view_paths.rb index d5694d77f4..3ca5aedc14 100644 --- a/actionview/lib/action_view/view_paths.rb +++ b/actionview/lib/action_view/view_paths.rb @@ -5,13 +5,21 @@ module ActionView extend ActiveSupport::Concern included do - class_attribute :_view_paths, default: ActionView::PathSet.new.freeze + ViewPaths.set_view_paths(self, ActionView::PathSet.new.freeze) end delegate :template_exists?, :any_templates?, :view_paths, :formats, :formats=, :locale, :locale=, to: :lookup_context module ClassMethods + def _view_paths + ViewPaths.get_view_paths(self) + end + + def _view_paths=(paths) + ViewPaths.set_view_paths(self, paths) + end + def _prefixes # :nodoc: @_prefixes ||= begin return local_prefixes if superclass.abstract? @@ -29,6 +37,22 @@ module ActionView end end + # :stopdoc: + @all_view_paths = {} + + def self.get_view_paths(klass) + @all_view_paths[klass] || get_view_paths(klass.superclass) + end + + def self.set_view_paths(klass, paths) + @all_view_paths[klass] = paths + end + + def self.all_view_paths + @all_view_paths.values.uniq + end + # :startdoc: + # The prefixes used in render "foo" shortcuts. def _prefixes # :nodoc: self.class._prefixes diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb index 4574b798d9..83bb651ea3 100644 --- a/actionview/test/template/log_subscriber_test.rb +++ b/actionview/test/template/log_subscriber_test.rb @@ -11,9 +11,9 @@ class AVLogSubscriberTest < ActiveSupport::TestCase def setup super + ActionView::LookupContext::DetailsKey.clear + view_paths = ActionController::Base.view_paths - view_paths.each(&:clear_cache) - ActionView::LookupContext.fallbacks.each(&:clear_cache) lookup_context = ActionView::LookupContext.new(view_paths, {}, ["test"]) renderer = ActionView::Renderer.new(lookup_context) diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index b8d8717db4..3f298d81f3 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -631,9 +631,8 @@ class CachedViewRenderTest < ActiveSupport::TestCase # Ensure view path cache is primed def setup + ActionView::LookupContext::DetailsKey.clear view_paths = ActionController::Base.view_paths - view_paths.each(&:clear_cache) - ActionView::LookupContext.fallbacks.each(&:clear_cache) assert_equal ActionView::OptimizedFileSystemResolver, view_paths.first.class setup_view(view_paths) end @@ -650,9 +649,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase # Test the same thing as above, but make sure the view path # is not eager loaded def setup - view_paths = ActionController::Base.view_paths - view_paths.each(&:clear_cache) - ActionView::LookupContext.fallbacks.each(&:clear_cache) + ActionView::LookupContext::DetailsKey.clear path = ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH) view_paths = ActionView::PathSet.new([path]) assert_equal ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH), view_paths.first @@ -710,10 +707,10 @@ class CachedCollectionViewRenderTest < ActiveSupport::TestCase # Ensure view path cache is primed setup do + ActionView::LookupContext::DetailsKey.clear + view_paths = ActionController::Base.view_paths assert_equal ActionView::OptimizedFileSystemResolver, view_paths.first.class - view_paths.each(&:clear_cache) - ActionView::LookupContext.fallbacks.each(&:clear_cache) ActionView::PartialRenderer.collection_cache = ActiveSupport::Cache::MemoryStore.new diff --git a/actionview/test/template/resolver_cache_test.rb b/actionview/test/template/resolver_cache_test.rb index 8a5db1346a..90b61a2aa1 100644 --- a/actionview/test/template/resolver_cache_test.rb +++ b/actionview/test/template/resolver_cache_test.rb @@ -4,6 +4,7 @@ require "abstract_unit" class ResolverCacheTest < ActiveSupport::TestCase def test_inspect_shields_cache_internals + ActionView::LookupContext::DetailsKey.clear assert_match %r(#<ActionView::Resolver:0x[0-9a-f]+ @cache=#<ActionView::Resolver::Cache:0x[0-9a-f]+ keys=0 queries=0>>), ActionView::Resolver.new.inspect end end diff --git a/actionview/test/template/streaming_render_test.rb b/actionview/test/template/streaming_render_test.rb index 4567ee31b4..a5b59a700e 100644 --- a/actionview/test/template/streaming_render_test.rb +++ b/actionview/test/template/streaming_render_test.rb @@ -7,9 +7,9 @@ end class SetupFiberedBase < ActiveSupport::TestCase def setup + ActionView::LookupContext::DetailsKey.clear + view_paths = ActionController::Base.view_paths - view_paths.each(&:clear_cache) - ActionView::LookupContext.fallbacks.each(&:clear_cache) @assigns = { secret: "in the sauce", name: nil } @view = ActionView::Base.with_empty_template_cache.with_view_paths(view_paths, @assigns) diff --git a/actionview/test/template/test_case_test.rb b/actionview/test/template/test_case_test.rb index ab3ababba4..c89aff9c9d 100644 --- a/actionview/test/template/test_case_test.rb +++ b/actionview/test/template/test_case_test.rb @@ -24,6 +24,11 @@ module ActionView DeveloperStruct = Struct.new(:name) module SharedTests + def setup + ActionView::LookupContext::DetailsKey.clear + super + end + def self.included(test_case) test_case.class_eval do test "helpers defined on ActionView::TestCase are available" do 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/CHANGELOG.md b/activerecord/CHANGELOG.md index e957361ec3..f16a746b91 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -29,7 +29,7 @@ ``` config.active_record.database_selector = { delay: 2.seconds } config.active_record.database_resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver - config.active_record.database_operations = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session + config.active_record.database_resolver_context = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session ``` To change the database selection strategy, pass a custom class to the @@ -38,7 +38,7 @@ ``` config.active_record.database_selector = { delay: 10.seconds } config.active_record.database_resolver = MyResolver - config.active_record.database_operations = MyResolver::MyCookies + config.active_record.database_resolver_context = MyResolver::MyCookies ``` *Eileen M. Uchitelle* 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 68f53d5c1c..b0c0beac0e 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -347,7 +347,6 @@ module ActiveRecord add_to_target(record) do result = insert_record(record, true, raise) { @_was_loaded = loaded? - @association_ids = nil } end raise ActiveRecord::Rollback unless result @@ -384,6 +383,7 @@ module ActiveRecord delete_records(existing_records, method) if existing_records.any? @target -= records + @association_ids = nil records.each { |record| callback(:after_remove, record) } end @@ -424,7 +424,6 @@ module ActiveRecord unless owner.new_record? result &&= insert_record(record, true, raise) { @_was_loaded = loaded? - @association_ids = nil } end end @@ -447,6 +446,7 @@ module ActiveRecord if index target[index] = record elsif @_was_loaded || !loaded? + @association_ids = nil target << record end 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/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index d6f7359055..041e62077c 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -42,11 +42,10 @@ module ActiveRecord def associate_records_to_owner(owner, records) association = owner.association(reflection.name) - association.loaded! if reflection.collection? - association.target.concat(records) + association.target = records else - association.target = records.first unless records.empty? + association.target = records.first end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index c8d5f679a8..1c8df3c08a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -185,7 +185,7 @@ module ActiveRecord def wait_poll(timeout) @num_waiting += 1 - t0 = Time.now + t0 = Concurrent.monotonic_time elapsed = 0 loop do ActiveSupport::Dependencies.interlock.permit_concurrent_loads do @@ -194,7 +194,7 @@ module ActiveRecord return remove if any? - elapsed = Time.now - t0 + elapsed = Concurrent.monotonic_time - t0 if elapsed >= timeout msg = "could not obtain a connection from the pool within %0.3f seconds (waited %0.3f seconds); all pooled connections were in use" % [timeout, elapsed] @@ -686,13 +686,13 @@ module ActiveRecord end newly_checked_out = [] - timeout_time = Time.now + (@checkout_timeout * 2) + timeout_time = Concurrent.monotonic_time + (@checkout_timeout * 2) @available.with_a_bias_for(Thread.current) do loop do synchronize do return if collected_conns.size == @connections.size && @now_connecting == 0 - remaining_timeout = timeout_time - Time.now + remaining_timeout = timeout_time - Concurrent.monotonic_time remaining_timeout = 0 if remaining_timeout < 0 conn = checkout_for_exclusive_access(remaining_timeout) collected_conns << conn 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 b2a6109548..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) @@ -198,41 +198,44 @@ module ActiveRecord end module ColumnMethods + extend ActiveSupport::Concern + # Appends a primary key definition to the table definition. # Can be called multiple times, but this is probably not a good idea. def primary_key(name, type = :primary_key, **options) column(name, type, options.merge(primary_key: true)) end + ## + # :method: column + # :call-seq: column(name, type, options = {}) + # # Appends a column or columns of a specified type. # # t.string(:goat) # t.string(:goat, :sheep) # # See TableDefinition#column - [ - :bigint, - :binary, - :boolean, - :date, - :datetime, - :decimal, - :float, - :integer, - :json, - :string, - :text, - :time, - :timestamp, - :virtual, - ].each do |column_type| - module_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{column_type}(*args, **options) - args.each { |name| column(name, :#{column_type}, options) } + + included do + define_column_methods :bigint, :binary, :boolean, :date, :datetime, :decimal, + :float, :integer, :json, :string, :text, :time, :timestamp, :virtual + + alias :numeric :decimal + end + + class_methods do + private def define_column_methods(*column_types) # :nodoc: + column_types.each do |column_type| + module_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{column_type}(*names, **options) + raise ArgumentError, "Missing column name(s) for #{column_type}" if names.empty? + names.each { |name| column(name, :#{column_type}, options) } + end + RUBY end - CODE + end end - alias_method :numeric, :decimal end # Represents the schema of an SQL table in an abstract way. This class @@ -395,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 @@ -519,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 @@ -692,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/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 7b69a63f6e..569c146f92 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -166,9 +166,9 @@ module ActiveRecord def explain(arel, binds = []) sql = "EXPLAIN #{to_sql(arel, binds)}" - start = Time.now + start = Concurrent.monotonic_time result = exec_query(sql, "EXPLAIN", binds) - elapsed = Time.now - start + elapsed = Concurrent.monotonic_time - start MySQL::ExplainPrettyPrinter.new.pp(result, elapsed) end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb index 90bcdf3297..8cb6b64fec 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb @@ -4,48 +4,56 @@ module ActiveRecord module ConnectionAdapters module MySQL module ColumnMethods - def blob(*args, **options) - args.each { |name| column(name, :blob, options) } - end + extend ActiveSupport::Concern - def tinyblob(*args, **options) - args.each { |name| column(name, :tinyblob, options) } - end + ## + # :method: blob + # :call-seq: blob(*names, **options) - def mediumblob(*args, **options) - args.each { |name| column(name, :mediumblob, options) } - end + ## + # :method: tinyblob + # :call-seq: tinyblob(*names, **options) - def longblob(*args, **options) - args.each { |name| column(name, :longblob, options) } - end + ## + # :method: mediumblob + # :call-seq: mediumblob(*names, **options) - def tinytext(*args, **options) - args.each { |name| column(name, :tinytext, options) } - end + ## + # :method: longblob + # :call-seq: longblob(*names, **options) - def mediumtext(*args, **options) - args.each { |name| column(name, :mediumtext, options) } - end + ## + # :method: tinytext + # :call-seq: tinytext(*names, **options) - def longtext(*args, **options) - args.each { |name| column(name, :longtext, options) } - end + ## + # :method: mediumtext + # :call-seq: mediumtext(*names, **options) - def unsigned_integer(*args, **options) - args.each { |name| column(name, :unsigned_integer, options) } - end + ## + # :method: longtext + # :call-seq: longtext(*names, **options) - def unsigned_bigint(*args, **options) - args.each { |name| column(name, :unsigned_bigint, options) } - end + ## + # :method: unsigned_integer + # :call-seq: unsigned_integer(*names, **options) - def unsigned_float(*args, **options) - args.each { |name| column(name, :unsigned_float, options) } - end + ## + # :method: unsigned_bigint + # :call-seq: unsigned_bigint(*names, **options) + + ## + # :method: unsigned_float + # :call-seq: unsigned_float(*names, **options) + + ## + # :method: unsigned_decimal + # :call-seq: unsigned_decimal(*names, **options) - def unsigned_decimal(*args, **options) - args.each { |name| column(name, :unsigned_decimal, options) } + included do + define_column_methods :blob, :tinyblob, :mediumblob, :longblob, + :tinytext, :mediumtext, :longtext, :unsigned_integer, :unsigned_bigint, + :unsigned_float, :unsigned_decimal end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb index dc4a0bb26e..3bb7c52899 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb @@ -4,6 +4,8 @@ module ActiveRecord module ConnectionAdapters module PostgreSQL module ColumnMethods + extend ActiveSupport::Concern + # Defines the primary key field. # Use of the native PostgreSQL UUID type is supported, and can be used # by defining your tables as such: @@ -51,124 +53,131 @@ module ActiveRecord super end - def bigserial(*args, **options) - args.each { |name| column(name, :bigserial, options) } - end + ## + # :method: bigserial + # :call-seq: bigserial(*names, **options) - def bit(*args, **options) - args.each { |name| column(name, :bit, options) } - end + ## + # :method: bit + # :call-seq: bit(*names, **options) - def bit_varying(*args, **options) - args.each { |name| column(name, :bit_varying, options) } - end + ## + # :method: bit_varying + # :call-seq: bit_varying(*names, **options) - def cidr(*args, **options) - args.each { |name| column(name, :cidr, options) } - end + ## + # :method: cidr + # :call-seq: cidr(*names, **options) - def citext(*args, **options) - args.each { |name| column(name, :citext, options) } - end + ## + # :method: citext + # :call-seq: citext(*names, **options) - def daterange(*args, **options) - args.each { |name| column(name, :daterange, options) } - end + ## + # :method: daterange + # :call-seq: daterange(*names, **options) - def hstore(*args, **options) - args.each { |name| column(name, :hstore, options) } - end + ## + # :method: hstore + # :call-seq: hstore(*names, **options) - def inet(*args, **options) - args.each { |name| column(name, :inet, options) } - end + ## + # :method: inet + # :call-seq: inet(*names, **options) - def interval(*args, **options) - args.each { |name| column(name, :interval, options) } - end + ## + # :method: interval + # :call-seq: interval(*names, **options) - def int4range(*args, **options) - args.each { |name| column(name, :int4range, options) } - end + ## + # :method: int4range + # :call-seq: int4range(*names, **options) - def int8range(*args, **options) - args.each { |name| column(name, :int8range, options) } - end + ## + # :method: int8range + # :call-seq: int8range(*names, **options) - def jsonb(*args, **options) - args.each { |name| column(name, :jsonb, options) } - end + ## + # :method: jsonb + # :call-seq: jsonb(*names, **options) - def ltree(*args, **options) - args.each { |name| column(name, :ltree, options) } - end + ## + # :method: ltree + # :call-seq: ltree(*names, **options) - def macaddr(*args, **options) - args.each { |name| column(name, :macaddr, options) } - end + ## + # :method: macaddr + # :call-seq: macaddr(*names, **options) - def money(*args, **options) - args.each { |name| column(name, :money, options) } - end + ## + # :method: money + # :call-seq: money(*names, **options) - def numrange(*args, **options) - args.each { |name| column(name, :numrange, options) } - end + ## + # :method: numrange + # :call-seq: numrange(*names, **options) - def oid(*args, **options) - args.each { |name| column(name, :oid, options) } - end + ## + # :method: oid + # :call-seq: oid(*names, **options) - def point(*args, **options) - args.each { |name| column(name, :point, options) } - end + ## + # :method: point + # :call-seq: point(*names, **options) - def line(*args, **options) - args.each { |name| column(name, :line, options) } - end + ## + # :method: line + # :call-seq: line(*names, **options) - def lseg(*args, **options) - args.each { |name| column(name, :lseg, options) } - end + ## + # :method: lseg + # :call-seq: lseg(*names, **options) - def box(*args, **options) - args.each { |name| column(name, :box, options) } - end + ## + # :method: box + # :call-seq: box(*names, **options) - def path(*args, **options) - args.each { |name| column(name, :path, options) } - end + ## + # :method: path + # :call-seq: path(*names, **options) - def polygon(*args, **options) - args.each { |name| column(name, :polygon, options) } - end + ## + # :method: polygon + # :call-seq: polygon(*names, **options) - def circle(*args, **options) - args.each { |name| column(name, :circle, options) } - end + ## + # :method: circle + # :call-seq: circle(*names, **options) - def serial(*args, **options) - args.each { |name| column(name, :serial, options) } - end + ## + # :method: serial + # :call-seq: serial(*names, **options) - def tsrange(*args, **options) - args.each { |name| column(name, :tsrange, options) } - end + ## + # :method: tsrange + # :call-seq: tsrange(*names, **options) - def tstzrange(*args, **options) - args.each { |name| column(name, :tstzrange, options) } - end + ## + # :method: tstzrange + # :call-seq: tstzrange(*names, **options) - def tsvector(*args, **options) - args.each { |name| column(name, :tsvector, options) } - end + ## + # :method: tsvector + # :call-seq: tsvector(*names, **options) - def uuid(*args, **options) - args.each { |name| column(name, :uuid, options) } - end + ## + # :method: uuid + # :call-seq: uuid(*names, **options) + + ## + # :method: xml + # :call-seq: xml(*names, **options) - def xml(*args, **options) - args.each { |name| column(name, :xml, options) } + included do + define_column_methods :bigserial, :bit, :bit_varying, :cidr, :citext, :daterange, + :hstore, :inet, :interval, :int4range, :int8range, :jsonb, :ltree, :macaddr, + :money, :numrange, :oid, :point, :line, :lseg, :box, :path, :polygon, :circle, + :serial, :tsrange, :tstzrange, :tsvector, :uuid, :xml end end 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/middleware/database_selector.rb b/activerecord/lib/active_record/middleware/database_selector.rb index 3ab50f5f6b..b5b5df074c 100644 --- a/activerecord/lib/active_record/middleware/database_selector.rb +++ b/activerecord/lib/active_record/middleware/database_selector.rb @@ -11,9 +11,9 @@ 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 - # operations class that sets a value that helps the resolver class decide - # when to switch. + # 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. # # Rails default middleware uses the request's session to set a timestamp # that informs the application when to read from a primary or read from a @@ -24,7 +24,7 @@ module ActiveRecord # # config.active_record.database_selector = { delay: 2.seconds } # config.active_record.database_resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver - # config.active_record.database_operations = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session + # config.active_record.database_resolver_context = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session # # New applications will include these lines commented out in the production.rb. # @@ -33,16 +33,16 @@ module ActiveRecord # # config.active_record.database_selector = { delay: 2.seconds } # config.active_record.database_resolver = MyResolver - # config.active_record.database_operations = MyResolver::MySession + # config.active_record.database_resolver_context = MyResolver::MySession class DatabaseSelector - def initialize(app, resolver_klass = Resolver, operations_klass = Resolver::Session, options = {}) + def initialize(app, resolver_klass = Resolver, context_klass = Resolver::Session, options = {}) @app = app @resolver_klass = resolver_klass - @operations_klass = operations_klass + @context_klass = context_klass @options = options end - attr_reader :resolver_klass, :operations_klass, :options + attr_reader :resolver_klass, :context_klass, :options # Middleware that determines which database connection to use in a multiple # database application. @@ -57,13 +57,13 @@ module ActiveRecord private def select_database(request, &blk) - operations = operations_klass.build(request) - database_resolver = resolver_klass.call(operations, options) + context = context_klass.call(request) + resolver = resolver_klass.call(context, options) if reading_request?(request) - database_resolver.read(&blk) + resolver.read(&blk) else - database_resolver.write(&blk) + resolver.write(&blk) end end diff --git a/activerecord/lib/active_record/middleware/database_selector/resolver.rb b/activerecord/lib/active_record/middleware/database_selector/resolver.rb index a84c292714..80b8cd7cae 100644 --- a/activerecord/lib/active_record/middleware/database_selector/resolver.rb +++ b/activerecord/lib/active_record/middleware/database_selector/resolver.rb @@ -18,18 +18,18 @@ module ActiveRecord class Resolver # :nodoc: SEND_TO_REPLICA_DELAY = 2.seconds - def self.call(resolver, options = {}) - new(resolver, options) + def self.call(context, options = {}) + new(context, options) end - def initialize(resolver, options = {}) - @resolver = resolver + def initialize(context, options = {}) + @context = context @options = options @delay = @options && @options[:delay] ? @options[:delay] : SEND_TO_REPLICA_DELAY @instrumenter = ActiveSupport::Notifications.instrumenter end - attr_reader :resolver, :delay, :instrumenter + attr_reader :context, :delay, :instrumenter def read(&blk) if read_from_primary? @@ -68,7 +68,7 @@ module ActiveRecord instrumenter.instrument("database_selector.active_record.wrote_to_primary") do yield ensure - resolver.update_last_write_timestamp + context.update_last_write_timestamp end end end @@ -82,7 +82,7 @@ module ActiveRecord end def time_since_last_write_ok? - Time.now - resolver.last_write_timestamp >= send_to_replica_delay + Time.now - context.last_write_timestamp >= send_to_replica_delay end end end diff --git a/activerecord/lib/active_record/middleware/database_selector/resolver/session.rb b/activerecord/lib/active_record/middleware/database_selector/resolver/session.rb index 33e0af5ee4..df7af054b7 100644 --- a/activerecord/lib/active_record/middleware/database_selector/resolver/session.rb +++ b/activerecord/lib/active_record/middleware/database_selector/resolver/session.rb @@ -10,7 +10,7 @@ module ActiveRecord # The last_write is used to determine whether it's safe to read # from the replica or the request needs to be sent to the primary. class Session # :nodoc: - def self.build(request) + def self.call(request) new(request.session) end 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/railtie.rb b/activerecord/lib/active_record/railtie.rb index aac49a92b4..a1d7c893bf 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -91,7 +91,7 @@ module ActiveRecord initializer "active_record.database_selector" do if options = config.active_record.delete(:database_selector) resolver = config.active_record.delete(:database_resolver) - operations = config.active_record.delete(:database_operations) + operations = config.active_record.delete(:database_resolver_context) config.app_middleware.use ActiveRecord::Middleware::DatabaseSelector, resolver, operations, options end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 32f0609798..347d745d19 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -67,11 +67,7 @@ module ActiveRecord # user = users.new { |user| user.name = 'Oscar' } # user.name # => Oscar def new(attributes = nil, &block) - current_scope = klass.current_scope(true) - block = -> record do - klass.current_scope = current_scope - yield record if block_given? - end + block = klass.current_scope_restoring_block(&block) scoping { klass.new(attributes, &block) } end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 8f1065c1e7..91cfc4e849 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -307,8 +307,6 @@ module ActiveRecord return false if !conditions || limit_value == 0 - conditions = sanitize_forbidden_attributes(conditions) - if eager_loading? relation = apply_join_dependency(eager_loading: false) return relation.exists?(conditions) @@ -316,7 +314,7 @@ module ActiveRecord relation = construct_relation_for_exists(conditions) - skip_query_cache_if_necessary { connection.select_value(relation.arel, "#{name} Exists") } ? true : false + skip_query_cache_if_necessary { connection.select_one(relation.arel, "#{name} Exists") } ? true : false end # This method is called whenever no records are found with either a single @@ -354,7 +352,13 @@ module ActiveRecord end def construct_relation_for_exists(conditions) - relation = except(:select, :distinct, :order)._select!(ONE_AS_ONE).limit!(1) + conditions = sanitize_forbidden_attributes(conditions) + + if distinct_value && offset_value + relation = limit(1) + else + relation = except(:select, :distinct, :order)._select!(ONE_AS_ONE).limit!(1) + end case conditions when Array, Hash diff --git a/activerecord/lib/active_record/scoping.rb b/activerecord/lib/active_record/scoping.rb index 35e9dcbffc..1142a87d25 100644 --- a/activerecord/lib/active_record/scoping.rb +++ b/activerecord/lib/active_record/scoping.rb @@ -30,6 +30,14 @@ module ActiveRecord def current_scope=(scope) ScopeRegistry.set_value_for(:current_scope, self, scope) end + + def current_scope_restoring_block(&block) + current_scope = self.current_scope(true) + -> *args do + self.current_scope = current_scope + yield(*args) if block_given? + end + end end def populate_with_current_scope_attributes # :nodoc: 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 a61569420e..c01138c059 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -63,7 +63,8 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase ActiveRecord::SQLCounter.clear_log Client.find(3).firm ensure - assert ActiveRecord::SQLCounter.log_all.all? { |sql| /order by/i !~ sql }, "ORDER BY was used in the query" + sql_log = ActiveRecord::SQLCounter.log + assert sql_log.all? { |sql| /order by/i !~ sql }, "ORDER BY was used in the query: #{sql_log}" end def test_belongs_to_with_primary_key diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index a9e22c7643..b9e16cab21 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -18,7 +18,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase :categorizations, :people, :categories, :edges, :vertices def test_eager_association_loading_with_cascaded_two_levels - authors = Author.all.merge!(includes: { posts: :comments }, order: "authors.id").to_a + authors = Author.includes(posts: :comments).order(:id).to_a assert_equal 3, authors.size assert_equal 5, authors[0].posts.size assert_equal 3, authors[1].posts.size @@ -26,7 +26,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_with_cascaded_two_levels_and_one_level - authors = Author.all.merge!(includes: [{ posts: :comments }, :categorizations], order: "authors.id").to_a + authors = Author.includes({ posts: :comments }, :categorizations).order(:id).to_a assert_equal 3, authors.size assert_equal 5, authors[0].posts.size assert_equal 3, authors[1].posts.size @@ -36,7 +36,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_with_hmt_does_not_table_name_collide_when_joining_associations - authors = Author.joins(:posts).eager_load(:comments).where(posts: { tags_count: 1 }).to_a + authors = Author.joins(:posts).eager_load(:comments).where(posts: { tags_count: 1 }).order(:id).to_a assert_equal 3, assert_no_queries { authors.size } assert_equal 10, assert_no_queries { authors[0].comments.size } end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 5fdc5a92fc..bf44be1811 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2004,6 +2004,21 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_not_predicate company.clients, :loaded? 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 + firm.clients.build + assert_equal 1, firm.clients.size + end + + def test_ids_reader_cache_should_be_cleared_when_collection_is_deleted + firm = companies(:first_firm) + assert_equal [2, 3, 11], firm.client_ids + client = firm.clients.first + firm.clients.delete(client) + 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 diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 3e5b5c1275..7bb629466d 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -37,8 +37,8 @@ class HasOneAssociationsTest < ActiveRecord::TestCase ActiveRecord::SQLCounter.clear_log companies(:first_firm).account ensure - log_all = ActiveRecord::SQLCounter.log_all - assert log_all.all? { |sql| /order by/i !~ sql }, "ORDER BY was used in the query: #{log_all}" + sql_log = ActiveRecord::SQLCounter.log + assert sql_log.all? { |sql| /order by/i !~ sql }, "ORDER BY was used in the query: #{sql_log}" end def test_has_one_cache_nils diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 6af2a43c7f..4040682280 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -271,6 +271,16 @@ class FinderTest < ActiveRecord::TestCase assert_equal true, Topic.exists?({}) end + def test_exists_with_distinct_and_offset_and_joins + assert Post.left_joins(:comments).distinct.offset(10).exists? + assert_not Post.left_joins(:comments).distinct.offset(11).exists? + end + + def test_exists_with_distinct_and_offset_and_select + assert Post.select(:body).distinct.offset(3).exists? + assert_not Post.select(:body).distinct.offset(4).exists? + end + # Ensure +exists?+ runs without an error by excluding distinct value. # See https://github.com/rails/rails/pull/26981. def test_exists_with_order_and_distinct 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/columns_test.rb b/activerecord/test/cases/migration/columns_test.rb index cedd9c44e3..1c66c8186f 100644 --- a/activerecord/test/cases/migration/columns_test.rb +++ b/activerecord/test/cases/migration/columns_test.rb @@ -318,6 +318,17 @@ module ActiveRecord ensure connection.drop_table(:my_table) rescue nil end + + def test_add_column_without_column_name + e = assert_raise ArgumentError do + connection.create_table "my_table", force: true do |t| + t.timestamp + end + end + assert_equal "Missing column name(s) for timestamp", e.message + ensure + connection.drop_table :my_table, if_exists: true + end end end end 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/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index d77fa3c864..8bb531f1b8 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -16,6 +16,10 @@ *Stefan Schüßler* +* Add `Hash#deep_transform_values`, and `Hash#deep_transform_values!`. + + *Guillermo Iguaran* + ## Rails 6.0.0.beta1 (January 18, 2019) ## * Remove deprecated `Module#reachable?` method. diff --git a/activesupport/lib/active_support/cache/memory_store.rb b/activesupport/lib/active_support/cache/memory_store.rb index 106b616529..629eb2dd70 100644 --- a/activesupport/lib/active_support/cache/memory_store.rb +++ b/activesupport/lib/active_support/cache/memory_store.rb @@ -62,13 +62,13 @@ module ActiveSupport return if pruning? @pruning = true begin - start_time = Time.now + start_time = Concurrent.monotonic_time cleanup instrument(:prune, target_size, from: @cache_size) do keys = synchronize { @key_access.keys.sort { |a, b| @key_access[a].to_f <=> @key_access[b].to_f } } keys.each do |key| delete_entry(key, options) - return if @cache_size <= target_size || (max_time && Time.now - start_time > max_time) + return if @cache_size <= target_size || (max_time && Concurrent.monotonic_time - start_time > max_time) end end ensure diff --git a/activesupport/lib/active_support/core_ext/hash.rb b/activesupport/lib/active_support/core_ext/hash.rb index c4b9e5f1a0..2f0901d853 100644 --- a/activesupport/lib/active_support/core_ext/hash.rb +++ b/activesupport/lib/active_support/core_ext/hash.rb @@ -2,6 +2,7 @@ require "active_support/core_ext/hash/conversions" require "active_support/core_ext/hash/deep_merge" +require "active_support/core_ext/hash/deep_transform_values" require "active_support/core_ext/hash/except" require "active_support/core_ext/hash/indifferent_access" require "active_support/core_ext/hash/keys" diff --git a/activesupport/lib/active_support/core_ext/hash/deep_transform_values.rb b/activesupport/lib/active_support/core_ext/hash/deep_transform_values.rb new file mode 100644 index 0000000000..720a1f67c8 --- /dev/null +++ b/activesupport/lib/active_support/core_ext/hash/deep_transform_values.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +class Hash + # Returns a new hash with all keys converted by the block operation. + # This includes the keys from the root hash and from all + # nested hashes and arrays. + # + # hash = { person: { name: 'Rob', age: '28' } } + # + # hash.deep_transform_values{ |value| value.to_s.upcase } + # # => {person: {name: "ROB", age: "28"}} + def deep_transform_values(&block) + _deep_transform_values_in_object(self, &block) + end + + # Destructively converts all values by using the block operation. + # This includes the values from the root hash and from all + # nested hashes and arrays. + def deep_transform_values!(&block) + _deep_transform_values_in_object!(self, &block) + end + + private + # support methods for deep transforming nested hashes and arrays + def _deep_transform_values_in_object(object, &block) + case object + when Hash + object.transform_values { |value| _deep_transform_values_in_object(value, &block) } + when Array + object.map { |e| _deep_transform_values_in_object(e, &block) } + else + yield(object) + end + end + + def _deep_transform_values_in_object!(object, &block) + case object + when Hash + object.transform_values! { |value| _deep_transform_values_in_object!(value, &block) } + when Array + object.map! { |e| _deep_transform_values_in_object!(e, &block) } + else + yield(object) + end + end +end diff --git a/activesupport/lib/active_support/notifications/instrumenter.rb b/activesupport/lib/active_support/notifications/instrumenter.rb index 125c06f37a..00a57c38c9 100644 --- a/activesupport/lib/active_support/notifications/instrumenter.rb +++ b/activesupport/lib/active_support/notifications/instrumenter.rb @@ -137,7 +137,7 @@ module ActiveSupport private def now - Process.clock_gettime(Process::CLOCK_MONOTONIC) + Concurrent.monotonic_time end if clock_gettime_supported? diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index e8e0a1ae72..8572d56722 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -33,6 +33,8 @@ class HashExtTest < ActiveSupport::TestCase h = {} assert_respond_to h, :deep_transform_keys assert_respond_to h, :deep_transform_keys! + assert_respond_to h, :deep_transform_values + assert_respond_to h, :deep_transform_values! assert_respond_to h, :symbolize_keys assert_respond_to h, :symbolize_keys! assert_respond_to h, :deep_symbolize_keys @@ -78,6 +80,31 @@ class HashExtTest < ActiveSupport::TestCase assert_equal({ "a" => { b: { "c" => 3 } } }, @nested_mixed) end + def test_deep_transform_values + assert_equal({ "a" => "1", "b" => "2" }, @strings.deep_transform_values { |value| value.to_s }) + assert_equal({ "a" => { "b" => { "c" => "3" } } }, @nested_strings.deep_transform_values { |value| value.to_s }) + assert_equal({ "a" => [ { "b" => "2" }, { "c" => "3" }, "4" ] }, @string_array_of_hashes.deep_transform_values { |value| value.to_s }) + end + + def test_deep_transform_values_not_mutates + transformed_hash = @nested_mixed.deep_dup + transformed_hash.deep_transform_values { |value| value.to_s } + assert_equal @nested_mixed, transformed_hash + end + + def test_deep_transform_values! + assert_equal({ "a" => "1", "b" => "2" }, @strings.deep_transform_values! { |value| value.to_s }) + assert_equal({ "a" => { "b" => { "c" => "3" } } }, @nested_strings.deep_transform_values! { |value| value.to_s }) + assert_equal({ "a" => [ { "b" => "2" }, { "c" => "3" }, "4" ] }, @string_array_of_hashes.deep_transform_values! { |value| value.to_s }) + end + + def test_deep_transform_values_with_bang_mutates + transformed_hash = @nested_mixed.deep_dup + transformed_hash.deep_transform_values! { |value| value.to_s } + assert_equal({ "a" => { b: { "c" => "3" } } }, transformed_hash) + assert_equal({ "a" => { b: { "c" => 3 } } }, @nested_mixed) + end + def test_symbolize_keys assert_equal @symbols, @symbols.symbolize_keys assert_equal @symbols, @strings.symbolize_keys 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/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/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/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/generators/rails/app/templates/config/environments/production.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt index 94f7dd0c79..ed1cf09eeb 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt @@ -107,9 +107,10 @@ Rails.application.configure do # The `database_resolver` class is used by the middleware to determine which # database is appropriate to use based on the time delay. # - # The `database_operations` class is used by the middleware to set timestamps - # for the last write to the primary. The resolver uses the operations class - # timestamps to determine how long to wait before reading from the replica. + # The `database_resolver_context` class is used by the middleware to set + # timestamps for the last write to the primary. The resolver uses the context + # class timestamps to determine how long to wait before reading from the + # replica. # # By default Rails will store a last write timestamp in the session. The # DatabaseSelector middleware is designed as such you can define your own @@ -117,5 +118,5 @@ Rails.application.configure do # these configuration options. # config.active_record.database_selector = { delay: 2.seconds } # config.active_record.database_resolver = ActiveRecord::Middleware::DatabaseSelector::Resolver - # config.active_record.database_operations = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session + # config.active_record.database_resolver_context = ActiveRecord::Middleware::DatabaseSelector::Resolver::Session end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 37fba72ee3..9ea4d6dc5f 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1714,7 +1714,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 +1727,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 +1747,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 +1782,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/server_test.rb b/railties/test/application/server_test.rb index 5b8aab08d5..9df36b3444 100644 --- a/railties/test/application/server_test.rb +++ b/railties/test/application/server_test.rb @@ -30,17 +30,15 @@ module ApplicationTests pid = nil Bundler.with_original_env do - begin - pid = Process.spawn("bin/rails server -P tmp/dummy.pid", chdir: app_path, in: replica, out: replica, err: replica) - assert_output("Listening", primary) + pid = Process.spawn("bin/rails server -P tmp/dummy.pid", chdir: app_path, in: replica, out: replica, err: replica) + assert_output("Listening", primary) - rails("restart") + rails("restart") - assert_output("Restarting", primary) - assert_output("Inherited", primary) - ensure - kill(pid) if pid - end + assert_output("Restarting", primary) + assert_output("Inherited", primary) + ensure + kill(pid) if pid end end diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index ca7601f6fe..0e8e0e86ee 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -472,21 +472,17 @@ 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` } + 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. 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..106b1029f0 --- /dev/null +++ b/railties/test/isolation/assets/package.json @@ -0,0 +1,9 @@ +{ + "name": "dummy", + "private": true, + "dependencies": { + "@rails/ujs": "file:../../../../actionview", + "@rails/webpacker": "https://github.com/rails/webpacker.git", + "turbolinks": "^5.2.0" + } +} |