diff options
78 files changed, 795 insertions, 361 deletions
diff --git a/actionmailbox/CHANGELOG.md b/actionmailbox/CHANGELOG.md index d92367dc17..3e92283c12 100644 --- a/actionmailbox/CHANGELOG.md +++ b/actionmailbox/CHANGELOG.md @@ -1,3 +1,9 @@ +* Allow skipping incineration of processed emails. + + This can be done by setting `config.action_mailbox.incinerate` to `false`. + + *Pratik Naik* + ## Rails 6.0.0.beta1 (January 18, 2019) ## * Added to Rails. diff --git a/actionmailbox/app/jobs/action_mailbox/incineration_job.rb b/actionmailbox/app/jobs/action_mailbox/incineration_job.rb index 1579a3c7c8..351bd67c69 100644 --- a/actionmailbox/app/jobs/action_mailbox/incineration_job.rb +++ b/actionmailbox/app/jobs/action_mailbox/incineration_job.rb @@ -6,6 +6,9 @@ module ActionMailbox # # Since this incineration is set for the future, it'll automatically ignore any <tt>InboundEmail</tt>s # that have already been deleted and discard itself if so. + # + # You can disable incinerating processed emails by setting +config.action_mailbox.incinerate+ or + # +ActionMailbox.incinerate+ to +false+. class IncinerationJob < ActiveJob::Base queue_as { ActionMailbox.queues[:incineration] } diff --git a/actionmailbox/app/models/action_mailbox/inbound_email/incineratable.rb b/actionmailbox/app/models/action_mailbox/inbound_email/incineratable.rb index 697331ede4..e7c8782f33 100644 --- a/actionmailbox/app/models/action_mailbox/inbound_email/incineratable.rb +++ b/actionmailbox/app/models/action_mailbox/inbound_email/incineratable.rb @@ -7,7 +7,7 @@ module ActionMailbox::InboundEmail::Incineratable extend ActiveSupport::Concern included do - after_update_commit :incinerate_later, if: -> { status_previously_changed? && processed? } + after_update_commit :incinerate_later, if: -> { ActionMailbox.incinerate && status_previously_changed? && processed? } end def incinerate_later diff --git a/actionmailbox/lib/action_mailbox.rb b/actionmailbox/lib/action_mailbox.rb index b4ff25a9ab..772dbd6529 100644 --- a/actionmailbox/lib/action_mailbox.rb +++ b/actionmailbox/lib/action_mailbox.rb @@ -11,6 +11,7 @@ module ActionMailbox mattr_accessor :ingress mattr_accessor :logger + mattr_accessor :incinerate, default: true mattr_accessor :incinerate_after, default: 30.days mattr_accessor :queues, default: {} end diff --git a/actionmailbox/lib/action_mailbox/engine.rb b/actionmailbox/lib/action_mailbox/engine.rb index d5a07a7dce..27334c037e 100644 --- a/actionmailbox/lib/action_mailbox/engine.rb +++ b/actionmailbox/lib/action_mailbox/engine.rb @@ -14,6 +14,7 @@ module ActionMailbox config.eager_load_namespaces << ActionMailbox config.action_mailbox = ActiveSupport::OrderedOptions.new + config.action_mailbox.incinerate = true config.action_mailbox.incinerate_after = 30.days config.action_mailbox.queues = ActiveSupport::InheritableOptions.new \ @@ -22,6 +23,7 @@ module ActionMailbox initializer "action_mailbox.config" do config.after_initialize do |app| ActionMailbox.logger = app.config.action_mailbox.logger || Rails.logger + ActionMailbox.incinerate = app.config.action_mailbox.incinerate.nil? ? true : app.config.action_mailbox.incinerate ActionMailbox.incinerate_after = app.config.action_mailbox.incinerate_after || 30.days ActionMailbox.queues = app.config.action_mailbox.queues || {} end 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/actionmailbox/test/unit/inbound_email/incineration_test.rb b/actionmailbox/test/unit/inbound_email/incineration_test.rb index 21c01a9cea..54488349fd 100644 --- a/actionmailbox/test/unit/inbound_email/incineration_test.rb +++ b/actionmailbox/test/unit/inbound_email/incineration_test.rb @@ -44,4 +44,14 @@ class ActionMailbox::InboundEmail::IncinerationTest < ActiveSupport::TestCase perform_enqueued_jobs only: ActionMailbox::IncinerationJob end end + + test "skipping incineration" do + original, ActionMailbox.incinerate = ActionMailbox.incinerate, false + + assert_no_enqueued_jobs only: ActionMailbox::IncinerationJob do + create_inbound_email_from_fixture("welcome.eml").delivered! + end + ensure + ActionMailbox.incinerate = original + end end diff --git a/actionpack/lib/action_dispatch/middleware/debug_view.rb b/actionpack/lib/action_dispatch/middleware/debug_view.rb index 5a7010a1c2..f16484d1ea 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_view.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_view.rb @@ -15,6 +15,10 @@ module ActionDispatch super(renderer, assigns) end + def compiled_method_container + self.class + end + def debug_params(params) clean_params = params.clone clean_params.delete("action") 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/CHANGELOG.md b/actionview/CHANGELOG.md index 5e17e65bde..5e7b271fb9 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,13 @@ +* ActionView::Template.finalize_compiled_template_methods is deprecated with + no replacement. + + *tenderlove* + +* config.action_view.finalize_compiled_template_methods is deprecated with + no replacement. + + *tenderlove* + * Ensure unique DOM IDs for collection inputs with float values. Fixes #34974 diff --git a/actionview/lib/action_view.rb b/actionview/lib/action_view.rb index 6ff2d70e35..8cb4648a67 100644 --- a/actionview/lib/action_view.rb +++ b/actionview/lib/action_view.rb @@ -35,7 +35,6 @@ module ActionView eager_autoload do autoload :Base autoload :Context - autoload :CompiledTemplates, "action_view/context" autoload :Digestor autoload :Helpers autoload :LookupContext diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index b143e13c96..420136d6de 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -11,10 +11,6 @@ require "action_view/template" require "action_view/lookup_context" module ActionView #:nodoc: - module CompiledTemplates #:nodoc: - # holds compiled template code - end - # = Action View Base # # Action View templates can be written in several ways. @@ -146,8 +142,6 @@ module ActionView #:nodoc: class Base include Helpers, ::ERB::Util, Context - include CompiledTemplates - # Specify the proc used to decorate input tags that refer to attributes with errors. cattr_accessor :field_error_proc, default: Proc.new { |html_tag, instance| "<div class=\"field_with_errors\">#{html_tag}</div>".html_safe } @@ -186,6 +180,20 @@ module ActionView #:nodoc: def xss_safe? #:nodoc: true end + + def with_empty_template_cache # :nodoc: + subclass = Class.new(self) { + # We can't implement these as self.class because subclasses will + # share the same template cache as superclasses, so "changed?" won't work + # correctly. + define_method(:compiled_method_container) { subclass } + define_singleton_method(:compiled_method_container) { subclass } + } + end + + def changed?(other) # :nodoc: + compiled_method_container != other.compiled_method_container + end end attr_reader :view_renderer @@ -260,7 +268,11 @@ module ActionView #:nodoc: end def compiled_method_container - CompiledTemplates + raise NotImplementedError, <<~msg + Subclasses of ActionView::Base must implement `compiled_method_container` + or use the class method `with_empty_template_cache` for constructing + an ActionView::Base subclass that has an empty cache. + msg end ActiveSupport.run_load_hooks(:action_view, self) diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index c2f6439e26..61fe11cf45 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -58,21 +58,36 @@ 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) + @view_context_class ||= klass.with_empty_template_cache end end @@ -83,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. @@ -97,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 @@ -173,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 @@ -200,7 +216,7 @@ module ActionView end if @cache - [details, DetailsKey.get(details)] + [details, DetailsKey.details_cache_key(details)] else [details, nil] end @@ -231,6 +247,7 @@ module ActionView def initialize(view_paths, details = {}, prefixes = []) @details_key = nil + @digest_cache = nil @cache = true @prefixes = prefixes @rendered_format = nil @@ -240,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/railtie.rb b/actionview/lib/action_view/railtie.rb index 12d06bf376..a25e1d3d02 100644 --- a/actionview/lib/action_view/railtie.rb +++ b/actionview/lib/action_view/railtie.rb @@ -6,11 +6,13 @@ require "rails" module ActionView # = Action View Railtie class Railtie < Rails::Engine # :nodoc: + NULL_OPTION = Object.new + config.action_view = ActiveSupport::OrderedOptions.new config.action_view.embed_authenticity_token_in_remote_forms = nil config.action_view.debug_missing_translation = true config.action_view.default_enforce_utf8 = nil - config.action_view.finalize_compiled_template_methods = true + config.action_view.finalize_compiled_template_methods = NULL_OPTION config.eager_load_namespaces << ActionView @@ -48,8 +50,11 @@ module ActionView initializer "action_view.finalize_compiled_template_methods" do |app| ActiveSupport.on_load(:action_view) do - ActionView::Template.finalize_compiled_template_methods = - app.config.action_view.delete(:finalize_compiled_template_methods) + option = app.config.action_view.delete(:finalize_compiled_template_methods) + + if option != NULL_OPTION + ActiveSupport::Deprecation.warn "action_view.finalize_compiled_template_methods is deprecated and has no effect" + end end end diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index 205665a8c6..da92ce1f5e 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -35,24 +35,36 @@ module ActionView end module ClassMethods - def view_context_class - @view_context_class ||= begin - supports_path = supports_path? - routes = respond_to?(:_routes) && _routes - helpers = respond_to?(:_helpers) && _helpers - - Class.new(ActionView::Base) do - if routes - include routes.url_helpers(supports_path) - include routes.mounted_helpers - end - - if helpers - include helpers - end + def _routes + end + + def _helpers + end + + def build_view_context_class(klass, supports_path, routes, helpers) + Class.new(klass) do + if routes + include routes.url_helpers(supports_path) + include routes.mounted_helpers + end + + if helpers + include helpers end end end + + def view_context_class + klass = ActionView::LookupContext::DetailsKey.view_context_class(ActionView::Base) + + @view_context_class ||= build_view_context_class(klass, supports_path?, _routes, _helpers) + + if klass.changed?(@view_context_class) + @view_context_class = build_view_context_class(klass, supports_path?, _routes, _helpers) + end + + @view_context_class + end end def view_context_class @@ -75,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/template.rb b/actionview/lib/action_view/template.rb index 8a5407c622..37f476e554 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -2,6 +2,7 @@ require "active_support/core_ext/object/try" require "active_support/core_ext/kernel/singleton_class" +require "active_support/deprecation" require "thread" require "delegate" @@ -10,7 +11,13 @@ module ActionView class Template extend ActiveSupport::Autoload - mattr_accessor :finalize_compiled_template_methods, default: true + def self.finalize_compiled_template_methods + ActiveSupport::Deprecation.warn "ActionView::Template.finalize_compiled_template_methods is deprecated and has no effect" + end + + def self.finalize_compiled_template_methods=(_) + ActiveSupport::Deprecation.warn "ActionView::Template.finalize_compiled_template_methods= is deprecated and has no effect" + end # === Encodings in ActionView::Template # @@ -118,16 +125,6 @@ module ActionView attr_reader :source, :identifier, :handler, :original_encoding, :updated_at - # This finalizer is needed (and exactly with a proc inside another proc) - # otherwise templates leak in development. - Finalizer = proc do |method_name, mod| # :nodoc: - proc do - mod.module_eval do - remove_possible_method method_name - end - end - end - attr_reader :variable def initialize(source, identifier, handler, details) @@ -337,9 +334,6 @@ module ActionView end mod.module_eval(source, identifier, 0) - if finalize_compiled_template_methods - ObjectSpace.define_finalizer(self, Finalizer[method_name, mod]) - end end def handle_render_error(view, e) 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/abstract_unit.rb b/actionview/test/abstract_unit.rb index acc2ed029b..b649b3c9dd 100644 --- a/actionview/test/abstract_unit.rb +++ b/actionview/test/abstract_unit.rb @@ -48,7 +48,8 @@ module RenderERBUtils @view ||= begin path = ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH) view_paths = ActionView::PathSet.new([path]) - ActionView::Base.with_view_paths(view_paths) + view = ActionView::Base.with_empty_template_cache + view.with_view_paths(view_paths) end end @@ -61,7 +62,8 @@ module RenderERBUtils ActionView::Template.handler_for_extension(:erb), {}) - template.render(ActionView::Base.empty, {}).strip + view = ActionView::Base.with_empty_template_cache + template.render(view.empty, {}).strip end end diff --git a/actionview/test/activerecord/multifetch_cache_test.rb b/actionview/test/activerecord/multifetch_cache_test.rb index 229b4e56d0..f56168bda5 100644 --- a/actionview/test/activerecord/multifetch_cache_test.rb +++ b/actionview/test/activerecord/multifetch_cache_test.rb @@ -10,8 +10,10 @@ class MultifetchCacheTest < ActiveRecordTestCase def setup view_paths = ActionController::Base.view_paths + view_paths.each(&:clear_cache) + ActionView::LookupContext.fallbacks.each(&:clear_cache) - @view = Class.new(ActionView::Base) do + @view = Class.new(ActionView::Base.with_empty_template_cache) do def view_cache_dependencies [] end diff --git a/actionview/test/template/compiled_templates_test.rb b/actionview/test/template/compiled_templates_test.rb index ded4786e62..6375555bb4 100644 --- a/actionview/test/template/compiled_templates_test.rb +++ b/actionview/test/template/compiled_templates_test.rb @@ -3,7 +3,18 @@ require "abstract_unit" class CompiledTemplatesTest < ActiveSupport::TestCase - teardown do + attr_reader :view_class + + def setup + super + view_paths = ActionController::Base.view_paths + view_paths.each(&:clear_cache) + ActionView::LookupContext.fallbacks.each(&:clear_cache) + @view_class = ActionView::Base.with_empty_template_cache + end + + def teardown + super ActionView::LookupContext::DetailsKey.clear end @@ -72,13 +83,13 @@ class CompiledTemplatesTest < ActiveSupport::TestCase def render_with_cache(*args) view_paths = ActionController::Base.view_paths - ActionView::Base.with_view_paths(view_paths, {}).render(*args) + view_class.with_view_paths(view_paths, {}).render(*args) end def render_without_cache(*args) path = ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH) view_paths = ActionView::PathSet.new([path]) - ActionView::Base.with_view_paths(view_paths, {}).render(*args) + view_class.with_view_paths(view_paths, {}).render(*args) end def modify_template(template, content) diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb index 9fcf80bb24..83bb651ea3 100644 --- a/actionview/test/template/log_subscriber_test.rb +++ b/actionview/test/template/log_subscriber_test.rb @@ -11,10 +11,13 @@ class AVLogSubscriberTest < ActiveSupport::TestCase def setup super - view_paths = ActionController::Base.view_paths + ActionView::LookupContext::DetailsKey.clear + + view_paths = ActionController::Base.view_paths + lookup_context = ActionView::LookupContext.new(view_paths, {}, ["test"]) renderer = ActionView::Renderer.new(lookup_context) - @view = ActionView::Base.new(renderer, {}) + @view = ActionView::Base.with_empty_template_cache.new(renderer, {}) ActionView::LogSubscriber.attach_to :action_view diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index f89d087c34..3f298d81f3 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -9,7 +9,8 @@ end module RenderTestCases def setup_view(paths) @assigns = { secret: "in the sauce" } - @view = Class.new(ActionView::Base) do + + @view = Class.new(ActionView::Base.with_empty_template_cache) do def view_cache_dependencies; []; end def combined_fragment_cache_key(key) @@ -17,7 +18,9 @@ module RenderTestCases end end.with_view_paths(paths, @assigns) - @controller_view = TestController.new.view_context + controller = TestController.new + + @controller_view = controller.view_context_class.with_empty_template_cache.new(controller.view_renderer, controller.view_assigns, controller) # Reload and register danish language for testing I18n.backend.store_translations "da", {} @@ -628,6 +631,7 @@ class CachedViewRenderTest < ActiveSupport::TestCase # Ensure view path cache is primed def setup + ActionView::LookupContext::DetailsKey.clear view_paths = ActionController::Base.view_paths assert_equal ActionView::OptimizedFileSystemResolver, view_paths.first.class setup_view(view_paths) @@ -645,6 +649,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase # Test the same thing as above, but make sure the view path # is not eager loaded def setup + 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 @@ -702,6 +707,8 @@ 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 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 dda2095013..a5b59a700e 100644 --- a/actionview/test/template/streaming_render_test.rb +++ b/actionview/test/template/streaming_render_test.rb @@ -7,9 +7,12 @@ end class SetupFiberedBase < ActiveSupport::TestCase def setup + ActionView::LookupContext::DetailsKey.clear + view_paths = ActionController::Base.view_paths + @assigns = { secret: "in the sauce", name: nil } - @view = ActionView::Base.with_view_paths(view_paths, @assigns) + @view = ActionView::Base.with_empty_template_cache.with_view_paths(view_paths, @assigns) @controller_view = TestController.new.view_context end diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index 8257d97b7c..a069c8f2d0 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -19,7 +19,8 @@ class TestERBTemplate < ActiveSupport::TestCase end class Context < ActionView::Base - def initialize + def initialize(*) + super @output_buffer = "original" @virtual_path = nil end @@ -63,7 +64,8 @@ class TestERBTemplate < ActiveSupport::TestCase end def setup - @context = Context.new + @context = Context.with_empty_template_cache.empty + super end def test_basic_template 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/actionview/test/template/translation_helper_test.rb b/actionview/test/template/translation_helper_test.rb index d04e68e182..23fc9850c4 100644 --- a/actionview/test/template/translation_helper_test.rb +++ b/actionview/test/template/translation_helper_test.rb @@ -36,7 +36,10 @@ class TranslationHelperTest < ActiveSupport::TestCase } } ) - @view = ::ActionView::Base.with_view_paths(ActionController::Base.view_paths, {}) + view_paths = ActionController::Base.view_paths + view_paths.each(&:clear_cache) + ActionView::LookupContext.fallbacks.each(&:clear_cache) + @view = ::ActionView::Base.with_empty_template_cache.with_view_paths(view_paths, {}) end teardown do diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c412646cc9..f16a746b91 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Fix `relation.create` to avoid leaking scope to initialization block and callbacks. + + Fixes #9894, #17577. + + *Ryuta Kamizono* + * Chaining named scope is no longer leaking to class level querying methods. Fixes #14003. @@ -23,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 @@ -32,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/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/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..ec241fdbe9 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 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/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 bab00ef065..347d745d19 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -67,6 +67,7 @@ module ActiveRecord # user = users.new { |user| user.name = 'Oscar' } # user.name # => Oscar def new(attributes = nil, &block) + block = klass.current_scope_restoring_block(&block) scoping { klass.new(attributes, &block) } end @@ -92,7 +93,11 @@ module ActiveRecord # users.create(name: nil) # validation on name # # => #<User id: nil, name: nil, ...> def create(attributes = nil, &block) - scoping { klass.create(attributes, &block) } + if attributes.is_a?(Array) + attributes.collect { |attr| create(attr, &block) } + else + new(attributes, &block).tap(&:save) + end end # Similar to #create, but calls @@ -102,7 +107,11 @@ module ActiveRecord # Expects arguments in the same format as # {ActiveRecord::Base.create!}[rdoc-ref:Persistence::ClassMethods#create!]. def create!(attributes = nil, &block) - scoping { klass.create!(attributes, &block) } + if attributes.is_a?(Array) + attributes.collect { |attr| create!(attr, &block) } + else + new(attributes, &block).tap(&:save!) + end end def first_or_create(attributes = nil, &block) # :nodoc: @@ -312,12 +321,12 @@ module ActiveRecord # Please check unscoped if you want to remove all previous scopes (including # the default_scope) during the execution of a block. def scoping - @delegate_to_klass && klass.current_scope ? yield : klass._scoping(self) { yield } + @delegate_to_klass && klass.current_scope(true) ? yield : _scoping(self) { yield } end def _exec_scope(*args, &block) # :nodoc: @delegate_to_klass = true - instance_exec(*args, &block) || self + _scoping(nil) { instance_exec(*args, &block) || self } ensure @delegate_to_klass = false end @@ -632,6 +641,13 @@ module ActiveRecord end private + def _scoping(scope) + previous, klass.current_scope = klass.current_scope(true), scope + yield + ensure + klass.current_scope = previous + end + def _substitute_values(values) values.map do |name, value| attr = arel_attribute(name) 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/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index d758e289ca..8cf4fc469f 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -8,7 +8,7 @@ module ActiveRecord module SpawnMethods # This is overridden by Associations::CollectionProxy def spawn #:nodoc: - @delegate_to_klass && klass.current_scope ? klass.all : clone + @delegate_to_klass && klass.current_scope(true) ? klass.all : clone end # Merges in the conditions from <tt>other</tt>, if <tt>other</tt> is an ActiveRecord::Relation. diff --git a/activerecord/lib/active_record/scoping.rb b/activerecord/lib/active_record/scoping.rb index c3a56b2174..1142a87d25 100644 --- a/activerecord/lib/active_record/scoping.rb +++ b/activerecord/lib/active_record/scoping.rb @@ -27,10 +27,17 @@ module ActiveRecord ScopeRegistry.value_for(:current_scope, self, skip_inherited_scope) end - private - def current_scope=(scope) - ScopeRegistry.set_value_for(:current_scope, self, scope) + 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: @@ -84,7 +91,7 @@ module ActiveRecord base = model.base_class while klass <= base value = @registry[scope_type][klass.name] - return value || nil unless value.nil? + return value if value klass = klass.superclass end end diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb index 6caf9b3251..8c612df27a 100644 --- a/activerecord/lib/active_record/scoping/default.rb +++ b/activerecord/lib/active_record/scoping/default.rb @@ -31,14 +31,7 @@ module ActiveRecord # Post.limit(10) # Fires "SELECT * FROM posts LIMIT 10" # } def unscoped - block_given? ? _scoping(relation) { yield } : relation - end - - def _scoping(relation) # :nodoc: - previous, self.current_scope = current_scope(true), relation - yield - ensure - self.current_scope = previous + block_given? ? relation.scoping { yield } : relation end # Are there attributes associated with this scope? diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index 987e6bd63f..5278eb29a9 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -180,8 +180,7 @@ module ActiveRecord if body.respond_to?(:to_proc) singleton_class.define_method(name) do |*args| - scope = all - scope = _scoping(false) { scope._exec_scope(*args, &body) } + scope = all._exec_scope(*args, &body) scope = scope.extending(extension) if extension scope end 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/base_test.rb b/activerecord/test/cases/base_test.rb index 4938b6865f..363593ca19 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1535,7 +1535,7 @@ class BasicsTest < ActiveRecord::TestCase Bird.create!(name: "Bluejay") ActiveRecord::Base.connection.while_preventing_writes do - assert_queries(2) { Bird.where(name: "Bluejay").explain } + assert_nothing_raised { Bird.where(name: "Bluejay").explain } end end 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..6810c6c4ee 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,103 @@ 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 + 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 +204,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 +218,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 +232,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 +251,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 +325,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 +358,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 +368,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 +450,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 +508,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 +516,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 6b5b877260..0ab0459c38 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1167,7 +1167,12 @@ class RelationTest < ActiveRecord::TestCase end def test_first_or_create_with_block - parrot = Bird.where(color: "green").first_or_create { |bird| bird.name = "parrot" } + canary = Bird.create!(color: "yellow", name: "canary") + parrot = Bird.where(color: "green").first_or_create do |bird| + bird.name = "parrot" + assert_equal canary, Bird.find_by!(name: "canary") + end + assert_equal 1, parrot.total_count assert_kind_of Bird, parrot assert_predicate parrot, :persisted? assert_equal "green", parrot.color @@ -1209,7 +1214,12 @@ class RelationTest < ActiveRecord::TestCase end def test_first_or_create_bang_with_valid_block - parrot = Bird.where(color: "green").first_or_create! { |bird| bird.name = "parrot" } + canary = Bird.create!(color: "yellow", name: "canary") + parrot = Bird.where(color: "green").first_or_create! do |bird| + bird.name = "parrot" + assert_equal canary, Bird.find_by!(name: "canary") + end + assert_equal 1, parrot.total_count assert_kind_of Bird, parrot assert_predicate parrot, :persisted? assert_equal "green", parrot.color @@ -1259,7 +1269,12 @@ class RelationTest < ActiveRecord::TestCase end def test_first_or_initialize_with_block - parrot = Bird.where(color: "green").first_or_initialize { |bird| bird.name = "parrot" } + canary = Bird.create!(color: "yellow", name: "canary") + parrot = Bird.where(color: "green").first_or_initialize do |bird| + bird.name = "parrot" + assert_equal canary, Bird.find_by!(name: "canary") + end + assert_equal 1, parrot.total_count assert_kind_of Bird, parrot assert_not_predicate parrot, :persisted? assert_predicate parrot, :valid? diff --git a/activerecord/test/models/bird.rb b/activerecord/test/models/bird.rb index cfefa555b3..c9f6759c7d 100644 --- a/activerecord/test/models/bird.rb +++ b/activerecord/test/models/bird.rb @@ -16,4 +16,9 @@ class Bird < ActiveRecord::Base def cancel_save_callback_method throw(:abort) end + + attr_accessor :total_count + after_initialize do + self.total_count = Bird.count + end end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 2da774ca66..7002474b8d 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -12,6 +12,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/guides/source/configuring.md b/guides/source/configuring.md index 2911b1f7c3..e7129dd274 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -590,13 +590,6 @@ Defaults to `'signed cookie'`. * `config.action_view.default_enforce_utf8` determines whether forms are generated with a hidden tag that forces older versions of Internet Explorer to submit forms encoded in UTF-8. This defaults to `false`. -* `config.action_view.finalize_compiled_template_methods` determines - whether the methods on `ActionView::CompiledTemplates` that templates - compile themselves to are removed when template instances are - destroyed by the garbage collector. This helps prevent memory leaks in - development mode, but for large test suites, disabling this option in - the test environment can improve performance. This defaults to `true`. - ### Configuring Action Mailbox 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/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt index 223aa56187..63ed3fa952 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt @@ -48,7 +48,4 @@ Rails.application.configure do # Raises error for missing translations. # config.action_view.raise_on_missing_translations = true - - # Prevent expensive template finalization at end of test suite runs. - config.action_view.finalize_compiled_template_methods = false end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 7006b0855f..37fba72ee3 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -2094,7 +2094,9 @@ module ApplicationTests test "ActionView::Template.finalize_compiled_template_methods is true by default" do app "test" - assert_equal true, ActionView::Template.finalize_compiled_template_methods + assert_deprecated do + ActionView::Template.finalize_compiled_template_methods + end end test "ActionView::Template.finalize_compiled_template_methods can be configured via config.action_view.finalize_compiled_template_methods" do @@ -2106,7 +2108,9 @@ module ApplicationTests app "test" - assert_equal false, ActionView::Template.finalize_compiled_template_methods + assert_deprecated do + ActionView::Template.finalize_compiled_template_methods + end end test "ActiveJob::Base.return_false_on_aborted_enqueue is true by default" 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 |