diff options
25 files changed, 204 insertions, 109 deletions
diff --git a/actionmailbox/db/migrate/20180917164000_create_action_mailbox_tables.rb b/actionmailbox/db/migrate/20180917164000_create_action_mailbox_tables.rb index 89ab66c1a9..2bf4335808 100644 --- a/actionmailbox/db/migrate/20180917164000_create_action_mailbox_tables.rb +++ b/actionmailbox/db/migrate/20180917164000_create_action_mailbox_tables.rb @@ -5,11 +5,7 @@ class CreateActionMailboxTables < ActiveRecord::Migration[6.0] t.string :message_id, null: false t.string :message_checksum, null: false - if supports_datetime_with_precision? - t.timestamps precision: 6 - else - t.timestamps - end + t.timestamps t.index [ :message_id, :message_checksum ], name: "index_action_mailbox_inbound_emails_uniqueness", unique: true end diff --git a/actiontext/test/dummy/db/migrate/2018052816_create_action_text_tables.rb b/actiontext/db/migrate/20180528164100_create_action_text_tables.rb index 6e7177620f..e7c66ea6ae 100644 --- a/actiontext/test/dummy/db/migrate/2018052816_create_action_text_tables.rb +++ b/actiontext/db/migrate/20180528164100_create_action_text_tables.rb @@ -2,7 +2,7 @@ class CreateActionTextTables < ActiveRecord::Migration[6.0] def change create_table :action_text_rich_texts do |t| t.string :name, null: false - t.text :body, limit: 16777215 + t.text :body, size: :long t.references :record, null: false, polymorphic: true, index: false t.timestamps diff --git a/actiontext/lib/templates/installer.rb b/actiontext/lib/templates/installer.rb index 990e41ca00..a8000eb9fc 100644 --- a/actiontext/lib/templates/installer.rb +++ b/actiontext/lib/templates/installer.rb @@ -26,7 +26,7 @@ if APPLICATION_PACK_PATH.exist? line = %[require("#{name}")] unless APPLICATION_PACK_PATH.read.include? line say "Adding #{name} to #{APPLICATION_PACK_PATH}" - append_to_file APPLICATION_PACK_PATH, "#{line}\n" + append_to_file APPLICATION_PACK_PATH, "\n#{line}" end end end diff --git a/actiontext/db/migrate/201805281641_create_action_text_tables.rb b/actiontext/test/dummy/db/migrate/20180528164100_create_action_text_tables.rb index 74c7a0ecb9..e7c66ea6ae 100644 --- a/actiontext/db/migrate/201805281641_create_action_text_tables.rb +++ b/actiontext/test/dummy/db/migrate/20180528164100_create_action_text_tables.rb @@ -2,11 +2,10 @@ class CreateActionTextTables < ActiveRecord::Migration[6.0] def change create_table :action_text_rich_texts do |t| t.string :name, null: false - t.text :body, limit: 16777215 + t.text :body, size: :long t.references :record, null: false, polymorphic: true, index: false - t.datetime :created_at, null: false - t.datetime :updated_at, null: false + t.timestamps t.index [ :record_type, :record_id, :name ], name: "index_action_text_rich_texts_uniqueness", unique: true end diff --git a/actiontext/test/dummy/db/schema.rb b/actiontext/test/dummy/db/schema.rb index 71080a1c69..5216c5fd33 100644 --- a/actiontext/test/dummy/db/schema.rb +++ b/actiontext/test/dummy/db/schema.rb @@ -14,7 +14,7 @@ ActiveRecord::Schema.define(version: 2018_10_03_185713) do create_table "action_text_rich_texts", force: :cascade do |t| t.string "name", null: false - t.text "body", limit: 16777215 + t.text "body" t.string "record_type", null: false t.integer "record_id", null: false t.datetime "created_at", precision: 6, null: false diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index 554d223c0e..c2f6439e26 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -3,6 +3,7 @@ require "concurrent/map" require "active_support/core_ext/module/remove_method" require "active_support/core_ext/module/attribute_accessors" +require "active_support/deprecation" require "action_view/template/resolver" module ActionView @@ -106,12 +107,6 @@ module ActionView module ViewPaths attr_reader :view_paths, :html_fallback_for_js - # Whenever setting view paths, makes a copy so that we can manipulate them in - # instance objects as we wish. - def view_paths=(paths) - @view_paths = ActionView::PathSet.new(Array(paths)) - end - def find(name, prefixes = [], partial = false, keys = [], options = {}) @view_paths.find(*args_for_lookup(name, prefixes, partial, keys, options)) end @@ -138,19 +133,34 @@ module ActionView # Adds fallbacks to the view paths. Useful in cases when you are rendering # a :file. def with_fallbacks - added_resolvers = 0 - self.class.fallbacks.each do |resolver| - next if view_paths.include?(resolver) - view_paths.push(resolver) - added_resolvers += 1 + view_paths = build_view_paths((@view_paths.paths + self.class.fallbacks).uniq) + + if block_given? + ActiveSupport::Deprecation.warn <<~eowarn + Calling `with_fallbacks` with a block is deprecated. Call methods on + the lookup context returned by `with_fallbacks` instead. + eowarn + + begin + _view_paths = @view_paths + @view_paths = view_paths + yield + ensure + @view_paths = _view_paths + end + else + ActionView::LookupContext.new(view_paths, @details, @prefixes) end - yield - ensure - added_resolvers.times { view_paths.pop } end private + # Whenever setting view paths, makes a copy so that we can manipulate them in + # instance objects as we wish. + def build_view_paths(paths) + ActionView::PathSet.new(Array(paths)) + end + def args_for_lookup(name, prefixes, partial, keys, details_options) name, prefixes = normalize_name(name, prefixes) details, details_key = detail_args_for(details_options) @@ -226,7 +236,7 @@ module ActionView @rendered_format = nil @details = initialize_details({}, details) - self.view_paths = view_paths + @view_paths = build_view_paths(view_paths) end def digest_cache diff --git a/actionview/lib/action_view/renderer/abstract_renderer.rb b/actionview/lib/action_view/renderer/abstract_renderer.rb index 20b2523cac..ae366ce54a 100644 --- a/actionview/lib/action_view/renderer/abstract_renderer.rb +++ b/actionview/lib/action_view/renderer/abstract_renderer.rb @@ -17,7 +17,7 @@ module ActionView # that new object is called in turn. This abstracts the setup and rendering # into a separate classes for partials and templates. class AbstractRenderer #:nodoc: - delegate :find_template, :find_file, :template_exists?, :any_templates?, :with_fallbacks, :with_layout_format, :formats, to: :@lookup_context + delegate :template_exists?, :any_templates?, :formats, to: :@lookup_context def initialize(lookup_context) @lookup_context = lookup_context @@ -38,8 +38,6 @@ module ActionView end def instrument(name, **options) # :doc: - options[:identifier] ||= (@template && @template.identifier) || @path - ActiveSupport::Notifications.instrument("render_#{name}.action_view", options) do |payload| yield payload end diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index f175c30aa1..478400e016 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -296,42 +296,43 @@ module ActionView def render(context, options, block) setup(context, options, block) - @template = find_partial + template = find_partial @lookup_context.rendered_format ||= begin - if @template && @template.formats.present? - @template.formats.first + if template && template.formats.first + template.formats.first else formats.first end end if @collection - render_collection + render_collection(context, template) else - render_partial + render_partial(context, template) end end private - def render_collection - instrument(:collection, count: @collection.size) do |payload| + def render_collection(view, template) + identifier = (template && template.identifier) || @path + instrument(:collection, identifier: identifier, count: @collection.size) do |payload| return nil if @collection.blank? if @options.key?(:spacer_template) - spacer = find_template(@options[:spacer_template], @locals.keys).render(@view, @locals) + spacer = find_template(@options[:spacer_template], @locals.keys).render(view, @locals) end - cache_collection_render(payload) do - @template ? collection_with_template : collection_without_template + cache_collection_render(payload, view, template) do + template ? collection_with_template(view, template) : collection_without_template(view) end.join(spacer).html_safe end end - def render_partial - instrument(:partial) do |payload| - view, locals, block = @view, @locals, @block + def render_partial(view, template) + instrument(:partial, identifier: template.identifier) do |payload| + locals, block = @locals, @block object, as = @object, @variable if !block && (layout = @options[:layout]) @@ -341,12 +342,12 @@ module ActionView object = locals[as] if object.nil? # Respect object when object is false locals[as] = object if @has_object - content = @template.render(view, locals) do |*name| + content = template.render(view, locals) do |*name| view._layout_for(*name, &block) end content = layout.render(view, locals) { content } if layout - payload[:cache_hit] = view.view_renderer.cache_hits[@template.virtual_path] + payload[:cache_hit] = view.view_renderer.cache_hits[template.virtual_path] content end end @@ -359,7 +360,6 @@ module ActionView # set to that string. Otherwise, the +options[:partial]+ object must # respond to +to_partial_path+ in order to setup the path. def setup(context, options, block) - @view = context @options = options @block = block @@ -381,10 +381,10 @@ module ActionView @collection = collection_from_object || collection_from_options if @collection - paths = @collection_data = @collection.map { |o| partial_path(o) } + paths = @collection_data = @collection.map { |o| partial_path(o, context) } @path = paths.uniq.one? ? paths.first : nil else - @path = partial_path + @path = partial_path(@object, context) end end @@ -423,8 +423,8 @@ module ActionView @lookup_context.find_template(path, prefixes, true, locals, @details) end - def collection_with_template - view, locals, template = @view, @locals, @template + def collection_with_template(view, template) + locals = @locals as, counter, iteration = @variable, @variable_counter, @variable_iteration if layout = @options[:layout] @@ -445,8 +445,8 @@ module ActionView end end - def collection_without_template - view, locals, collection_data = @view, @locals, @collection_data + def collection_without_template(view) + locals, collection_data = @locals, @collection_data cache = {} keys = @locals.keys @@ -474,7 +474,7 @@ module ActionView # # If +prefix_partial_path_with_controller_namespace+ is true, then this # method will prefix the partial paths with a namespace. - def partial_path(object = @object) + def partial_path(object, view) object = object.to_model if object.respond_to?(:to_model) path = if object.respond_to?(:to_partial_path) @@ -483,7 +483,7 @@ module ActionView raise ArgumentError.new("'#{object.inspect}' is not an ActiveModel-compatible object. It must implement :to_partial_path.") end - if @view.prefix_partial_path_with_controller_namespace + if view.prefix_partial_path_with_controller_namespace prefixed_partial_names[path] ||= merge_prefix_into_object_path(@context_prefix, path.dup) else path diff --git a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb index 5aa6f77902..388f9e5e56 100644 --- a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb +++ b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb @@ -11,13 +11,13 @@ module ActionView end private - def cache_collection_render(instrumentation_payload) + def cache_collection_render(instrumentation_payload, view, template) return yield unless @options[:cached] # Result is a hash with the key represents the # key used for cache lookup and the value is the item # on which the partial is being rendered - keyed_collection = collection_by_cache_keys + keyed_collection = collection_by_cache_keys(view, template) # Pull all partials from cache # Result is a hash, key matches the entry in @@ -51,23 +51,21 @@ module ActionView @options[:cached].respond_to?(:call) end - def collection_by_cache_keys + def collection_by_cache_keys(view, template) seed = callable_cache_key? ? @options[:cached] : ->(i) { i } + digest_path = view.digest_path_from_virtual(template.virtual_path) + @collection.each_with_object({}) do |item, hash| - hash[expanded_cache_key(seed.call(item))] = item + hash[expanded_cache_key(seed.call(item), view, template, digest_path)] = item end end - def expanded_cache_key(key) - key = @view.combined_fragment_cache_key(@view.cache_fragment_name(key, virtual_path: @template.virtual_path, digest_path: digest_path)) + def expanded_cache_key(key, view, template, digest_path) + key = view.combined_fragment_cache_key(view.cache_fragment_name(key, virtual_path: template.virtual_path, digest_path: digest_path)) key.frozen? ? key.dup : key # #read_multi & #write may require mutability, Dalli 2.6.0. end - def digest_path - @digest_path ||= @view.digest_path_from_virtual(@template.virtual_path) - end - # `order_by` is an enumerable object containing keys of the cache, # all keys are passed in whether found already or not. # diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb index fb5539e0db..c36baeffcd 100644 --- a/actionview/lib/action_view/renderer/template_renderer.rb +++ b/actionview/lib/action_view/renderer/template_renderer.rb @@ -12,7 +12,7 @@ module ActionView @lookup_context.rendered_format ||= (template.formats.first || formats.first) - render_template(context, template, options[:layout], options[:locals]) + render_template(context, template, options[:layout], options[:locals] || {}) end private @@ -28,7 +28,7 @@ module ActionView elsif options.key?(:html) Template::HTML.new(options[:html], formats.first) elsif options.key?(:file) - with_fallbacks { find_file(options[:file], nil, false, keys, @details) } + @lookup_context.with_fallbacks.find_file(options[:file], nil, false, keys, @details) elsif options.key?(:inline) handler = Template.handler_for_extension(options[:type] || "erb") Template.new(options[:inline], "inline template", handler, locals: keys) @@ -36,7 +36,7 @@ module ActionView if options[:template].respond_to?(:render) options[:template] else - find_template(options[:template], options[:prefixes], false, keys, @details) + @lookup_context.find_template(options[:template], options[:prefixes], false, keys, @details) end else raise ArgumentError, "You invoked render but did not give any of :partial, :template, :inline, :file, :plain, :html or :body option." @@ -45,9 +45,7 @@ module ActionView # Renders the given template. A string representing the layout can be # supplied as well. - def render_template(view, template, layout_name = nil, locals = nil) - locals ||= {} - + def render_template(view, template, layout_name, locals) render_with_layout(view, layout_name, locals) do |layout| instrument(:template, identifier: template.identifier, layout: layout.try(:virtual_path)) do template.render(view, locals) { |*name| view._layout_for(*name) } @@ -82,9 +80,9 @@ module ActionView when String begin if layout.start_with?("/") - with_fallbacks { find_template(layout, nil, false, [], details) } + @lookup_context.with_fallbacks.find_template(layout, nil, false, [], details) else - find_template(layout, nil, false, [], details) + @lookup_context.find_template(layout, nil, false, [], details) end rescue ActionView::MissingTemplate all_details = @details.merge(formats: @lookup_context.default_formats) diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb index 1fad08a689..d6203b95c5 100644 --- a/actionview/lib/action_view/testing/resolvers.rb +++ b/actionview/lib/action_view/testing/resolvers.rb @@ -8,13 +8,15 @@ module ActionView #:nodoc: # useful for testing extensions that have no way of knowing what the file # system will look like at runtime. class FixtureResolver < PathResolver - attr_reader :hash - def initialize(hash = {}, pattern = nil) super(pattern) @hash = hash end + def data + @hash + end + def to_s @hash.keys.join(", ") end diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb index 68e151f154..290f832794 100644 --- a/actionview/test/template/lookup_context_test.rb +++ b/actionview/test/template/lookup_context_test.rb @@ -5,10 +5,14 @@ require "abstract_controller/rendering" class LookupContextTest < ActiveSupport::TestCase def setup - @lookup_context = ActionView::LookupContext.new(FIXTURE_LOAD_PATH, {}) + @lookup_context = build_lookup_context(FIXTURE_LOAD_PATH, {}) ActionView::LookupContext::DetailsKey.clear end + def build_lookup_context(paths, details) + ActionView::LookupContext.new(paths, details) + end + def teardown I18n.locale = :en end @@ -118,19 +122,32 @@ class LookupContextTest < ActiveSupport::TestCase test "adds fallbacks to view paths when required" do assert_equal 1, @lookup_context.view_paths.size - @lookup_context.with_fallbacks do - assert_equal 3, @lookup_context.view_paths.size - assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("") - assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("/") + assert_deprecated do + @lookup_context.with_fallbacks do + assert_equal 3, @lookup_context.view_paths.size + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("") + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("/") + end end + + @lookup_context = @lookup_context.with_fallbacks + + assert_equal 3, @lookup_context.view_paths.size + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("") + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("/") end test "add fallbacks just once in nested fallbacks calls" do - @lookup_context.with_fallbacks do + assert_deprecated do @lookup_context.with_fallbacks do - assert_equal 3, @lookup_context.view_paths.size + @lookup_context.with_fallbacks do + assert_equal 3, @lookup_context.view_paths.size + end end end + + @lookup_context = @lookup_context.with_fallbacks.with_fallbacks + assert_equal 3, @lookup_context.view_paths.size end test "generates a new details key for each details hash" do @@ -156,13 +173,13 @@ class LookupContextTest < ActiveSupport::TestCase end test "gives the key forward to the resolver, so it can be used as cache key" do - @lookup_context.view_paths = ActionView::FixtureResolver.new("test/_foo.erb" => "Foo") + @lookup_context = build_lookup_context(ActionView::FixtureResolver.new("test/_foo.erb" => "Foo"), {}) template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source # Now we are going to change the template, but it won't change the returned template # since we will hit the cache. - @lookup_context.view_paths.first.hash["test/_foo.erb"] = "Bar" + @lookup_context.view_paths.first.data["test/_foo.erb"] = "Bar" template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source @@ -185,7 +202,7 @@ class LookupContextTest < ActiveSupport::TestCase end test "can disable the cache on demand" do - @lookup_context.view_paths = ActionView::FixtureResolver.new("test/_foo.erb" => "Foo") + @lookup_context = build_lookup_context(ActionView::FixtureResolver.new("test/_foo.erb" => "Foo"), {}) old_template = @lookup_context.find("foo", %w(test), true) template = @lookup_context.find("foo", %w(test), true) @@ -221,12 +238,12 @@ class LookupContextWithFalseCaching < ActiveSupport::TestCase # Now we are going to change the template, but it won't change the returned template # since the timestamp is the same. - @resolver.hash["test/_foo.erb"][0] = "Bar" + @resolver.data["test/_foo.erb"][0] = "Bar" template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source # Now update the timestamp. - @resolver.hash["test/_foo.erb"][1] = Time.now.utc + @resolver.data["test/_foo.erb"][1] = Time.now.utc template = @lookup_context.find("foo", %w(test), true) assert_equal "Bar", template.source end @@ -237,7 +254,7 @@ class LookupContextWithFalseCaching < ActiveSupport::TestCase template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source - @resolver.hash.clear + @resolver.data.clear assert_raise ActionView::MissingTemplate do @lookup_context.find("foo", %w(test), true) end @@ -246,12 +263,12 @@ class LookupContextWithFalseCaching < ActiveSupport::TestCase test "if no template was cached in the first lookup, retrieval should work in the second call" do ActionView::Resolver.stub(:caching?, false) do - @resolver.hash.clear + @resolver.data.clear assert_raise ActionView::MissingTemplate do @lookup_context.find("foo", %w(test), true) end - @resolver.hash["test/_foo.erb"] = ["Foo", Time.utc(2000)] + @resolver.data["test/_foo.erb"] = ["Foo", Time.utc(2000)] template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source end diff --git a/activejob/lib/active_job/enqueuing.rb b/activejob/lib/active_job/enqueuing.rb index ce118c1e8a..5609d13f5f 100644 --- a/activejob/lib/active_job/enqueuing.rb +++ b/activejob/lib/active_job/enqueuing.rb @@ -67,7 +67,7 @@ module ActiveJob false else ActiveSupport::Deprecation.warn( - "Rails 6.0 will return false when the enqueing is aborted. Make sure your code doesn't depend on it" \ + "Rails 6.0 will return false when the enqueuing is aborted. Make sure your code doesn't depend on it" \ " returning the instance of the job and set `config.active_job.return_false_on_aborted_enqueue = true`" \ " to remove the deprecations." ) diff --git a/activejob/test/cases/test_helper_test.rb b/activejob/test/cases/test_helper_test.rb index 046033921d..4d934df31b 100644 --- a/activejob/test/cases/test_helper_test.rb +++ b/activejob/test/cases/test_helper_test.rb @@ -1688,7 +1688,7 @@ class PerformedJobsTest < ActiveJob::TestCase end end - def test_assert_performed_with_without_bllock_with_global_id_args + def test_assert_performed_with_without_block_with_global_id_args ricardo = Person.new(9) HelloJob.perform_later(ricardo) perform_enqueued_jobs diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 167ade30e3..67cf5881d3 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* MySQL: Support `:size` option to change text and blob size. + + *Ryuta Kamizono* + * Make `t.timestamps` with precision by default. *Ryuta Kamizono* 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 415a0576c4..7b69a63f6e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -29,7 +29,7 @@ module ActiveRecord NATIVE_DATABASE_TYPES = { primary_key: "bigint auto_increment PRIMARY KEY", string: { name: "varchar", limit: 255 }, - text: { name: "text", limit: 65535 }, + text: { name: "text" }, integer: { name: "int", limit: 4 }, float: { name: "float", limit: 24 }, decimal: { name: "decimal" }, @@ -37,7 +37,8 @@ module ActiveRecord timestamp: { name: "timestamp" }, time: { name: "time" }, date: { name: "date" }, - binary: { name: "blob", limit: 65535 }, + binary: { name: "blob" }, + blob: { name: "blob" }, boolean: { name: "tinyint", limit: 1 }, json: { name: "json" }, } 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 2ed4ad16ae..90bcdf3297 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb @@ -56,6 +56,16 @@ module ActiveRecord case type when :virtual type = options[:type] + when :text, :blob, :binary + case (size = options[:size])&.to_s + when "tiny", "medium", "long" + sql_type = @conn.native_database_types[type][:name] + type = "#{size}#{sql_type}" + else + raise ArgumentError, <<~MSG unless size.nil? + #{size.inspect} is invalid :size value. Only :tiny, :medium, and :long are allowed. + MSG + end when :primary_key type = :integer options[:limit] ||= 8 diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb index d23178e43c..57518b02fa 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb @@ -10,6 +10,10 @@ module ActiveRecord spec[:unsigned] = "true" if column.unsigned? spec[:auto_increment] = "true" if column.auto_increment? + if /\A(?<size>tiny|medium|long)(?:text|blob)/ =~ column.sql_type + spec = { size: size.to_sym.inspect }.merge!(spec) + end + if @connection.supports_virtual_columns? && column.virtual? spec[:as] = extract_expression_for_virtual_column(column) spec[:stored] = "true" if /\b(?:STORED|PERSISTENT)\b/.match?(column.extra) @@ -37,13 +41,15 @@ module ActiveRecord case column.sql_type when /\Atimestamp\b/ :timestamp - when "tinyblob" - :blob else super end end + def schema_limit(column) + super unless /\A(?:tiny|medium|long)?(?:text|blob)/.match?(column.sql_type) + end + def schema_precision(column) super unless /\A(?:date)?time(?:stamp)?\b/.match?(column.sql_type) && column.precision == 0 end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 381d5ab29b..95beeb4cae 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -236,6 +236,8 @@ module ActiveRecord # @local_tz is initialized as nil to avoid warnings when connect tries to use it @local_tz = nil + @default_timezone = nil + @timestamp_decoder = nil @max_identifier_length = nil configure_connection @@ -628,6 +630,10 @@ module ActiveRecord def exec_no_cache(sql, name, binds) materialize_transactions + # make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been + # made since we established the connection + update_typemap_for_default_timezone + type_casted_binds = type_casted_binds(binds) log(sql, name, binds, type_casted_binds) do ActiveSupport::Dependencies.interlock.permit_concurrent_loads do @@ -638,6 +644,7 @@ module ActiveRecord def exec_cache(sql, name, binds) materialize_transactions + update_typemap_for_default_timezone stmt_key = prepare_statement(sql, binds) type_casted_binds = type_casted_binds(binds) @@ -826,6 +833,18 @@ module ActiveRecord @connection.type_map_for_queries = map end + def update_typemap_for_default_timezone + if @default_timezone != ActiveRecord::Base.default_timezone && @timestamp_decoder + decoder_class = ActiveRecord::Base.default_timezone == :utc ? + PG::TextDecoder::TimestampUtc : + PG::TextDecoder::TimestampWithoutTimeZone + + @timestamp_decoder = decoder_class.new(@timestamp_decoder.to_h) + @connection.type_map_for_results.add_coder(@timestamp_decoder) + @default_timezone = ActiveRecord::Base.default_timezone + end + end + def add_pg_decoders coders_by_name = { "int2" => PG::TextDecoder::Integer, @@ -836,6 +855,13 @@ module ActiveRecord "float8" => PG::TextDecoder::Float, "bool" => PG::TextDecoder::Boolean, } + + if defined?(PG::TextDecoder::TimestampUtc) + # Use native PG encoders available since pg-1.1 + coders_by_name["timestamp"] = PG::TextDecoder::TimestampUtc + coders_by_name["timestamptz"] = PG::TextDecoder::TimestampWithTimeZone + end + known_coder_types = coders_by_name.keys.map { |n| quote(n) } query = <<~SQL % known_coder_types.join(", ") SELECT t.oid, t.typname @@ -851,6 +877,10 @@ module ActiveRecord map = PG::TypeMapByOid.new coders.each { |coder| map.add_coder(coder) } @connection.type_map_for_results = map + + # extract timestamp decoder for use in update_typemap_for_default_timezone + @timestamp_decoder = coders.find { |coder| coder.name == "timestamp" } + update_typemap_for_default_timezone end def construct_coder(row, coder_class) diff --git a/activerecord/lib/active_record/railties/collection_cache_association_loading.rb b/activerecord/lib/active_record/railties/collection_cache_association_loading.rb index b5129e4239..dfaac4eefb 100644 --- a/activerecord/lib/active_record/railties/collection_cache_association_loading.rb +++ b/activerecord/lib/active_record/railties/collection_cache_association_loading.rb @@ -20,12 +20,12 @@ module ActiveRecord end end - def collection_without_template + def collection_without_template(*) @relation.preload_associations(@collection) if @relation super end - def collection_with_template + def collection_with_template(*) @relation.preload_associations(@collection) if @relation super end diff --git a/activerecord/test/cases/cache_key_test.rb b/activerecord/test/cases/cache_key_test.rb index 3a06b1c795..c27eb8a65d 100644 --- a/activerecord/test/cases/cache_key_test.rb +++ b/activerecord/test/cases/cache_key_test.rb @@ -51,7 +51,7 @@ module ActiveRecord end test "cache_version is the same when it comes from the DB or from the user" do - skip("Mysql2 does not return a string value for updated_at") if current_adapter?(:Mysql2Adapter) + skip("Mysql2 and PostgreSQL don't return a string value for updated_at") if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter) record = CacheMeWithVersion.create record_from_db = CacheMeWithVersion.find(record.id) @@ -63,7 +63,7 @@ module ActiveRecord end test "cache_version does not truncate zeros when timestamp ends in zeros" do - skip("Mysql2 does not return a string value for updated_at") if current_adapter?(:Mysql2Adapter) + skip("Mysql2 and PostgreSQL don't return a string value for updated_at") if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter) travel_to Time.now.beginning_of_day do record = CacheMeWithVersion.create @@ -84,7 +84,7 @@ module ActiveRecord end test "cache_version does NOT call updated_at when value is from the database" do - skip("Mysql2 does not return a string value for updated_at") if current_adapter?(:Mysql2Adapter) + skip("Mysql2 and PostgreSQL don't return a string value for updated_at") if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter) record = CacheMeWithVersion.create record_from_db = CacheMeWithVersion.find(record.id) diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 46e2ff79d9..02031e51ef 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -626,6 +626,18 @@ class MigrationTest < ActiveRecord::TestCase ensure Person.connection.drop_table :test_text_limits, if_exists: true end + + def test_invalid_text_size_should_raise + e = assert_raise(ArgumentError) do + Person.connection.create_table :test_text_sizes, force: true do |t| + t.text :bigtext, size: 0xfffffffff + end + end + + assert_match(/#{0xfffffffff} is invalid :size value\. Only :tiny, :medium, and :long are allowed\./, e.message) + ensure + Person.connection.drop_table :test_text_sizes, if_exists: true + end end if ActiveRecord::Base.connection.supports_advisory_locks? diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index dda3efa47c..49e9be9565 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -245,25 +245,31 @@ class SchemaDumperTest < ActiveRecord::TestCase if current_adapter?(:Mysql2Adapter) def test_schema_dump_includes_length_for_mysql_binary_fields - output = standard_dump + output = dump_table_schema "binary_fields" assert_match %r{t\.binary\s+"var_binary",\s+limit: 255$}, output assert_match %r{t\.binary\s+"var_binary_large",\s+limit: 4095$}, output end def test_schema_dump_includes_length_for_mysql_blob_and_text_fields - output = standard_dump - assert_match %r{t\.blob\s+"tiny_blob",\s+limit: 255$}, output + output = dump_table_schema "binary_fields" + assert_match %r{t\.binary\s+"tiny_blob",\s+size: :tiny$}, output assert_match %r{t\.binary\s+"normal_blob"$}, output - assert_match %r{t\.binary\s+"medium_blob",\s+limit: 16777215$}, output - assert_match %r{t\.binary\s+"long_blob",\s+limit: 4294967295$}, output - assert_match %r{t\.text\s+"tiny_text",\s+limit: 255$}, output + assert_match %r{t\.binary\s+"medium_blob",\s+size: :medium$}, output + assert_match %r{t\.binary\s+"long_blob",\s+size: :long$}, output + assert_match %r{t\.text\s+"tiny_text",\s+size: :tiny$}, output assert_match %r{t\.text\s+"normal_text"$}, output - assert_match %r{t\.text\s+"medium_text",\s+limit: 16777215$}, output - assert_match %r{t\.text\s+"long_text",\s+limit: 4294967295$}, output + assert_match %r{t\.text\s+"medium_text",\s+size: :medium$}, output + assert_match %r{t\.text\s+"long_text",\s+size: :long$}, output + assert_match %r{t\.binary\s+"tiny_blob_2",\s+size: :tiny$}, output + assert_match %r{t\.binary\s+"medium_blob_2",\s+size: :medium$}, output + assert_match %r{t\.binary\s+"long_blob_2",\s+size: :long$}, output + assert_match %r{t\.text\s+"tiny_text_2",\s+size: :tiny$}, output + assert_match %r{t\.text\s+"medium_text_2",\s+size: :medium$}, output + assert_match %r{t\.text\s+"long_text_2",\s+size: :long$}, output end def test_schema_does_not_include_limit_for_emulated_mysql_boolean_fields - output = standard_dump + output = dump_table_schema "booleans" assert_no_match %r{t\.boolean\s+"has_fun",.+limit: 1}, output end diff --git a/activerecord/test/schema/mysql2_specific_schema.rb b/activerecord/test/schema/mysql2_specific_schema.rb index 61e9bc9af7..b143035213 100644 --- a/activerecord/test/schema/mysql2_specific_schema.rb +++ b/activerecord/test/schema/mysql2_specific_schema.rb @@ -27,6 +27,7 @@ ActiveRecord::Schema.define do create_table :binary_fields, force: true do |t| t.binary :var_binary, limit: 255 t.binary :var_binary_large, limit: 4095 + t.tinyblob :tiny_blob t.blob :normal_blob t.mediumblob :medium_blob @@ -36,6 +37,13 @@ ActiveRecord::Schema.define do t.mediumtext :medium_text t.longtext :long_text + t.binary :tiny_blob_2, size: :tiny + t.binary :medium_blob_2, size: :medium + t.binary :long_blob_2, size: :long + t.text :tiny_text_2, size: :tiny + t.text :medium_text_2, size: :medium + t.text :long_text_2, size: :long + t.index :var_binary end diff --git a/activestorage/lib/active_storage/analyzer/video_analyzer.rb b/activestorage/lib/active_storage/analyzer/video_analyzer.rb index 18d8ff8237..e56c97f4f5 100644 --- a/activestorage/lib/active_storage/analyzer/video_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/video_analyzer.rb @@ -11,7 +11,7 @@ module ActiveStorage # # Example: # - # ActiveStorage::VideoAnalyzer.new(blob).metadata + # ActiveStorage::Analyzer::VideoAnalyzer.new(blob).metadata # # => { width: 640.0, height: 480.0, duration: 5.0, angle: 0, display_aspect_ratio: [4, 3] } # # When a video's angle is 90 or 270 degrees, its width and height are automatically swapped for convenience. |