diff options
35 files changed, 212 insertions, 162 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index a04de4b497..3b4dd79e81 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -26,6 +26,9 @@ Layout/CaseIndentation: Layout/CommentIndentation: Enabled: true +Layout/ElseAlignment: + Enabled: true + Layout/EmptyLineAfterMagicComment: Enabled: true @@ -140,6 +143,7 @@ Style/UnneededPercentQ: Lint/EndAlignment: Enabled: true EnforcedStyleAlignWith: variable + AutoCorrect: true # Use my_method(my_arg) not my_method( my_arg ) or my_method my_arg. Lint/RequireParentheses: diff --git a/Gemfile.lock b/Gemfile.lock index 441ff7417d..2d4c3e1383 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -37,7 +37,7 @@ PATH actioncable (5.2.0.beta2) actionpack (= 5.2.0.beta2) nio4r (~> 2.0) - websocket-driver (~> 0.6.1) + websocket-driver (>= 0.6.1) actionmailer (5.2.0.beta2) actionpack (= 5.2.0.beta2) actionview (= 5.2.0.beta2) diff --git a/actioncable/actioncable.gemspec b/actioncable/actioncable.gemspec index b5b98f1a6b..51db4dda3a 100644 --- a/actioncable/actioncable.gemspec +++ b/actioncable/actioncable.gemspec @@ -28,5 +28,5 @@ Gem::Specification.new do |s| s.add_dependency "actionpack", version s.add_dependency "nio4r", "~> 2.0" - s.add_dependency "websocket-driver", "~> 0.6.1" + s.add_dependency "websocket-driver", ">= 0.6.1" end diff --git a/actioncable/lib/action_cable/connection/client_socket.rb b/actioncable/lib/action_cable/connection/client_socket.rb index 10289ab55c..4b1964c4ae 100644 --- a/actioncable/lib/action_cable/connection/client_socket.rb +++ b/actioncable/lib/action_cable/connection/client_socket.rb @@ -83,7 +83,7 @@ module ActionCable when Numeric then @driver.text(message.to_s) when String then @driver.text(message) when Array then @driver.binary(message) - else false + else false end end diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index f0344fd927..35ba44005a 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -274,7 +274,7 @@ module ActionDispatch def standard_port case protocol when "https://" then 443 - else 80 + else 80 end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index d87a23a58c..31eb6104fe 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1573,7 +1573,7 @@ module ActionDispatch # Matches a URL pattern to one or more routes. # For more information, see match[rdoc-ref:Base#match]. # - # match 'path' => 'controller#action', via: patch + # match 'path' => 'controller#action', via: :patch # match 'path', to: 'controller#action', via: :post # match 'path', 'otherpath', on: :member, via: :get def match(path, *rest, &block) @@ -2082,9 +2082,9 @@ module ActionDispatch # [ :products, options.merge(params.permit(:page, :size).to_h.symbolize_keys) ] # end # - # In this instance the +params+ object comes from the context in which the the + # In this instance the +params+ object comes from the context in which the # block is executed, e.g. generating a URL inside a controller action or a view. - # If the block is executed where there isn't a params object such as this: + # If the block is executed where there isn't a +params+ object such as this: # # Rails.application.routes.url_helpers.browse_path # diff --git a/actionview/lib/action_view/helpers/date_helper.rb b/actionview/lib/action_view/helpers/date_helper.rb index 09040ccbc4..4c45f122fe 100644 --- a/actionview/lib/action_view/helpers/date_helper.rb +++ b/actionview/lib/action_view/helpers/date_helper.rb @@ -116,7 +116,7 @@ module ActionView when 10..19 then locale.t :less_than_x_seconds, count: 20 when 20..39 then locale.t :half_a_minute when 40..59 then locale.t :less_than_x_minutes, count: 1 - else locale.t :x_minutes, count: 1 + else locale.t :x_minutes, count: 1 end when 2...45 then locale.t :x_minutes, count: distance_in_minutes @@ -131,7 +131,7 @@ module ActionView when 43200...86400 then locale.t :about_x_months, count: (distance_in_minutes.to_f / 43200.0).round # 60 days up to 365 days when 86400...525600 then locale.t :x_months, count: (distance_in_minutes.to_f / 43200.0).round - else + else from_year = from_time.year from_year += 1 if from_time.month >= 3 to_year = to_time.year diff --git a/actionview/lib/action_view/helpers/text_helper.rb b/actionview/lib/action_view/helpers/text_helper.rb index 84d38aa416..34138de00e 100644 --- a/actionview/lib/action_view/helpers/text_helper.rb +++ b/actionview/lib/action_view/helpers/text_helper.rb @@ -13,9 +13,9 @@ module ActionView # # ==== Sanitization # - # Most text helpers by default sanitize the given content, but do not escape it. - # This means HTML tags will appear in the page but all malicious code will be removed. - # Let's look at some examples using the +simple_format+ method: + # Most text helpers that generate HTML output sanitize the given input by default, + # but do not escape it. This means HTML tags will appear in the page but all malicious + # code will be removed. Let's look at some examples using the +simple_format+ method: # # simple_format('<a href="http://example.com/">Example</a>') # # => "<p><a href=\"http://example.com/\">Example</a></p>" @@ -128,7 +128,7 @@ module ActionView # # => You searched for: <a href="search?q=rails">rails</a> # # highlight('<a href="javascript:alert(\'no!\')">ruby</a> on rails', 'rails', sanitize: false) - # # => "<a>ruby</a> on <mark>rails</mark>" + # # => <a href="javascript:alert('no!')">ruby</a> on <mark>rails</mark> def highlight(text, phrases, options = {}) text = sanitize(text) if options.fetch(:sanitize, true) diff --git a/activemodel/lib/active_model/attribute_mutation_tracker.rb b/activemodel/lib/active_model/attribute_mutation_tracker.rb index c67e1b809a..f55613ecd5 100644 --- a/activemodel/lib/active_model/attribute_mutation_tracker.rb +++ b/activemodel/lib/active_model/attribute_mutation_tracker.rb @@ -35,6 +35,10 @@ module ActiveModel end end + def changed_attribute_names + attr_names.select { |attr| changed?(attr) } + end + def any_changes? attr_names.any? { |attr| changed?(attr) } end @@ -109,8 +113,5 @@ module ActiveModel def original_value(*) end - - def force_change(*) - end end end diff --git a/activemodel/lib/active_model/attributes.rb b/activemodel/lib/active_model/attributes.rb index 28dd24b3cd..046ae67ad7 100644 --- a/activemodel/lib/active_model/attributes.rb +++ b/activemodel/lib/active_model/attributes.rb @@ -23,7 +23,7 @@ module ActiveModel end self.attribute_types = attribute_types.merge(name => type) define_default_attribute(name, options.fetch(:default, NO_DEFAULT_PROVIDED), type) - define_attribute_methods(name) + define_attribute_method(name) end private diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index d2ebd18107..0044fde6c5 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -3,6 +3,7 @@ require "active_support/hash_with_indifferent_access" require "active_support/core_ext/object/duplicable" require "active_model/attribute_mutation_tracker" +require "active_model/attribute_set" module ActiveModel # == Active \Model \Dirty @@ -142,9 +143,8 @@ module ActiveModel end def changes_applied # :nodoc: - @previously_changed = changes + _prepare_changes @mutations_before_last_save = mutations_from_database - @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new forget_attribute_assignments @mutations_from_database = nil end @@ -155,7 +155,7 @@ module ActiveModel # person.name = 'bob' # person.changed? # => true def changed? - changed_attributes.present? + mutations_from_database.any_changes? end # Returns an array with the name of the attributes with unsaved changes. @@ -164,24 +164,24 @@ module ActiveModel # person.name = 'bob' # person.changed # => ["name"] def changed - changed_attributes.keys + mutations_from_database.changed_attribute_names end # Handles <tt>*_changed?</tt> for +method_missing+. def attribute_changed?(attr, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN) # :nodoc: - !!changes_include?(attr) && + !!mutations_from_database.changed?(attr) && (to == OPTION_NOT_GIVEN || to == _read_attribute(attr)) && - (from == OPTION_NOT_GIVEN || from == changed_attributes[attr]) + (from == OPTION_NOT_GIVEN || from == attribute_was(attr)) end # Handles <tt>*_was</tt> for +method_missing+. def attribute_was(attr) # :nodoc: - attribute_changed?(attr) ? changed_attributes[attr] : _read_attribute(attr) + mutations_from_database.original_value(attr) end # Handles <tt>*_previously_changed?</tt> for +method_missing+. def attribute_previously_changed?(attr) #:nodoc: - previous_changes_include?(attr) + mutations_before_last_save.changed?(attr) end # Restore all previous data of the provided attributes. @@ -191,15 +191,12 @@ module ActiveModel # Clears all dirty data: current changes and previous changes. def clear_changes_information - @previously_changed = ActiveSupport::HashWithIndifferentAccess.new @mutations_before_last_save = nil - @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new forget_attribute_assignments @mutations_from_database = nil end def clear_attribute_changes(attr_names) - attributes_changed_by_setter.except!(*attr_names) attr_names.each do |attr_name| clear_attribute_change(attr_name) end @@ -212,13 +209,7 @@ module ActiveModel # person.name = 'robert' # person.changed_attributes # => {"name" => "bob"} def changed_attributes - # This should only be set by methods which will call changed_attributes - # multiple times when it is known that the computed value cannot change. - if defined?(@cached_changed_attributes) - @cached_changed_attributes - else - attributes_changed_by_setter.reverse_merge(mutations_from_database.changed_values).freeze - end + mutations_from_database.changed_values.freeze end # Returns a hash of changed attributes indicating their original @@ -228,9 +219,8 @@ module ActiveModel # person.name = 'bob' # person.changes # => { "name" => ["bill", "bob"] } def changes - cache_changed_attributes do - ActiveSupport::HashWithIndifferentAccess[changed.map { |attr| [attr, attribute_change(attr)] }] - end + _prepare_changes + mutations_from_database.changes end # Returns a hash of attributes that were changed before the model was saved. @@ -240,8 +230,7 @@ module ActiveModel # person.save # person.previous_changes # => {"name" => ["bob", "robert"]} def previous_changes - @previously_changed ||= ActiveSupport::HashWithIndifferentAccess.new - @previously_changed.merge(mutations_before_last_save.changes) + mutations_before_last_save.changes end def attribute_changed_in_place?(attr_name) # :nodoc: @@ -257,11 +246,17 @@ module ActiveModel unless defined?(@mutations_from_database) @mutations_from_database = nil end - @mutations_from_database ||= if defined?(@attributes) - ActiveModel::AttributeMutationTracker.new(@attributes) - else - NullMutationTracker.instance + + unless defined?(@attributes) + @_pseudo_attributes = true + @attributes = AttributeSet.new( + Hash.new { |h, attr| + h[attr] = Attribute.with_cast_value(attr, _clone_attribute(attr), Type.default_value) + } + ) end + + @mutations_from_database ||= ActiveModel::AttributeMutationTracker.new(@attributes) end def forget_attribute_assignments @@ -272,68 +267,45 @@ module ActiveModel @mutations_before_last_save ||= ActiveModel::NullMutationTracker.instance end - def cache_changed_attributes - @cached_changed_attributes = changed_attributes - yield - ensure - clear_changed_attributes_cache - end - - def clear_changed_attributes_cache - remove_instance_variable(:@cached_changed_attributes) if defined?(@cached_changed_attributes) - end - - # Returns +true+ if attr_name is changed, +false+ otherwise. - def changes_include?(attr_name) - attributes_changed_by_setter.include?(attr_name) || mutations_from_database.changed?(attr_name) - end - alias attribute_changed_by_setter? changes_include? - - # Returns +true+ if attr_name were changed before the model was saved, - # +false+ otherwise. - def previous_changes_include?(attr_name) - previous_changes.include?(attr_name) - end - # Handles <tt>*_change</tt> for +method_missing+. def attribute_change(attr) - [changed_attributes[attr], _read_attribute(attr)] if attribute_changed?(attr) + [attribute_was(attr), _read_attribute(attr)] if attribute_changed?(attr) end # Handles <tt>*_previous_change</tt> for +method_missing+. def attribute_previous_change(attr) - previous_changes[attr] if attribute_previously_changed?(attr) + mutations_before_last_save.change_to_attribute(attr) end # Handles <tt>*_will_change!</tt> for +method_missing+. def attribute_will_change!(attr) - unless attribute_changed?(attr) - begin - value = _read_attribute(attr) - value = value.duplicable? ? value.clone : value - rescue TypeError, NoMethodError - end - - set_attribute_was(attr, value) + attr = attr.to_s + mutations_from_database.force_change(attr).tap do + @attributes[attr] if defined?(@_pseudo_attributes) end - mutations_from_database.force_change(attr) end # Handles <tt>restore_*!</tt> for +method_missing+. def restore_attribute!(attr) if attribute_changed?(attr) - __send__("#{attr}=", changed_attributes[attr]) + __send__("#{attr}=", attribute_was(attr)) clear_attribute_changes([attr]) end end - def attributes_changed_by_setter - @attributes_changed_by_setter ||= ActiveSupport::HashWithIndifferentAccess.new + def _prepare_changes + if defined?(@_pseudo_attributes) + changed.each do |attr| + @attributes.write_from_user(attr, _read_attribute(attr)) + end + end end - # Force an attribute to have a particular "before" value - def set_attribute_was(attr, old_value) - attributes_changed_by_setter[attr] = old_value + def _clone_attribute(attr) + value = _read_attribute(attr) + value.duplicable? ? value.clone : value + rescue TypeError, NoMethodError + value end end end diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb index dfe041ff50..8c183ec516 100644 --- a/activemodel/test/cases/dirty_test.rb +++ b/activemodel/test/cases/dirty_test.rb @@ -5,12 +5,13 @@ require "cases/helper" class DirtyTest < ActiveModel::TestCase class DirtyModel include ActiveModel::Dirty - define_attribute_methods :name, :color, :size + define_attribute_methods :name, :color, :size, :status def initialize @name = nil @color = nil @size = nil + @status = "initialized" end def name @@ -40,6 +41,15 @@ class DirtyTest < ActiveModel::TestCase @size = val end + def status + @status + end + + def status=(val) + status_will_change! unless val == @status + @status = val + end + def save changes_applied end @@ -135,15 +145,20 @@ class DirtyTest < ActiveModel::TestCase test "saving should preserve previous changes" do @model.name = "Jericho Cane" + @model.status = "waiting" @model.save assert_equal [nil, "Jericho Cane"], @model.previous_changes["name"] + assert_equal ["initialized", "waiting"], @model.previous_changes["status"] end test "setting new attributes should not affect previous changes" do @model.name = "Jericho Cane" + @model.status = "waiting" @model.save @model.name = "DudeFella ManGuy" + @model.status = "finished" assert_equal [nil, "Jericho Cane"], @model.name_previous_change + assert_equal ["initialized", "waiting"], @model.previous_changes["status"] end test "saving should preserve model's previous changed status" do @@ -155,20 +170,26 @@ class DirtyTest < ActiveModel::TestCase test "previous value is preserved when changed after save" do assert_equal({}, @model.changed_attributes) @model.name = "Paul" - assert_equal({ "name" => nil }, @model.changed_attributes) + @model.status = "waiting" + assert_equal({ "name" => nil, "status" => "initialized" }, @model.changed_attributes) @model.save @model.name = "John" - assert_equal({ "name" => "Paul" }, @model.changed_attributes) + @model.status = "finished" + assert_equal({ "name" => "Paul", "status" => "waiting" }, @model.changed_attributes) end test "changing the same attribute multiple times retains the correct original value" do @model.name = "Otto" + @model.status = "waiting" @model.save @model.name = "DudeFella ManGuy" @model.name = "Mr. Manfredgensonton" + @model.status = "processing" + @model.status = "finished" assert_equal ["Otto", "Mr. Manfredgensonton"], @model.name_change + assert_equal ["waiting", "finished"], @model.status_change assert_equal @model.name_was, "Otto" end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 3de6fe566d..df4c79b0f6 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -32,9 +32,7 @@ module ActiveRecord # <tt>reload</tt> the record and clears changed attributes. def reload(*) super.tap do - @previously_changed = ActiveSupport::HashWithIndifferentAccess.new @mutations_before_last_save = nil - @attributes_changed_by_setter = ActiveSupport::HashWithIndifferentAccess.new @mutations_from_database = nil end end @@ -114,12 +112,12 @@ module ActiveRecord # Alias for +changed+ def changed_attribute_names_to_save - changes_to_save.keys + mutations_from_database.changed_attribute_names end # Alias for +changed_attributes+ def attributes_in_database - changes_to_save.transform_values(&:first) + mutations_from_database.changed_values end private diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index a8895f8606..c6e5122daf 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -38,7 +38,7 @@ module ActiveRecord " TABLESPACE = \"#{value}\"" when :connection_limit " CONNECTION LIMIT = #{value}" - else + else "" end end diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index dab70b835a..798328233f 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -1062,7 +1062,7 @@ module ActiveRecord end end - def current_version(connection = nil) + def current_version get_all_versions.max || 0 rescue ActiveRecord::NoDatabaseError end @@ -1169,8 +1169,8 @@ module ActiveRecord end # For cases where a table doesn't exist like loading from schema cache - def current_version(connection = nil) - MigrationContext.new(migrations_paths).current_version(connection) + def current_version + MigrationContext.new(migrations_paths).current_version end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index c1b1a5334a..cdd54cc502 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -362,7 +362,6 @@ module ActiveRecord became = klass.new became.instance_variable_set("@attributes", @attributes) became.instance_variable_set("@mutations_from_database", @mutations_from_database) if defined?(@mutations_from_database) - became.instance_variable_set("@changed_attributes", attributes_changed_by_setter) became.instance_variable_set("@new_record", new_record?) became.instance_variable_set("@destroyed", destroyed?) became.errors.copy!(errors) diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index d34fbfd4a9..ed589f9b43 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -85,10 +85,6 @@ module ActiveRecord @migrations_paths ||= Rails.application.paths["db/migrate"].to_a end - def migration_context - MigrationContext.new(migrations_paths) - end - def fixtures_path @fixtures_path ||= if ENV["FIXTURES_PATH"] File.join(root, ENV["FIXTURES_PATH"]) diff --git a/activerecord/test/cases/migrator_test.rb b/activerecord/test/cases/migrator_test.rb index d674e4506e..873455cf67 100644 --- a/activerecord/test/cases/migrator_test.rb +++ b/activerecord/test/cases/migrator_test.rb @@ -176,16 +176,20 @@ class MigratorTest < ActiveRecord::TestCase end def test_migrations_status_with_schema_define_in_subdirectories - _, migrator = migrator_class(3) - migrator = migrator.new("valid_with_subdirectories") + path = MIGRATIONS_ROOT + "/valid_with_subdirectories" + prev_paths = ActiveRecord::Migrator.migrations_paths + ActiveRecord::Migrator.migrations_paths = path - migrator.migrate + ActiveRecord::Schema.define(version: 3) do + end assert_equal [ - ["up", "001", "********** NO FILE **********"], - ["up", "002", "********** NO FILE **********"], - ["up", "003", "********** NO FILE **********"], - ], migrator.migrations_status + ["up", "001", "Valid people have last names"], + ["up", "002", "We need reminders"], + ["up", "003", "Innocent jointable"], + ], ActiveRecord::MigrationContext.new(path).migrations_status + ensure + ActiveRecord::Migrator.migrations_paths = prev_paths end def test_migrations_status_from_two_directories diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 5e01297fc1..061898d143 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,12 @@ +* Preserve display aspect ratio when extracting width and height from videos + with rectangular samples in `ActiveStorage::Analyzer::VideoAnalyzer`. + + When a video contains a display aspect ratio, emit it in metadata as + `:display_aspect_ratio` rather than the ambiguous `:aspect_ratio`. Compute + its height by scaling its encoded frame width according to the DAR. + + *George Claghorn* + * Use `after_destroy_commit` instead of `before_destroy` for purging attachments when a record is destroyed. diff --git a/activestorage/lib/active_storage/analyzer/video_analyzer.rb b/activestorage/lib/active_storage/analyzer/video_analyzer.rb index aa532da201..656e362187 100644 --- a/activestorage/lib/active_storage/analyzer/video_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/video_analyzer.rb @@ -9,12 +9,12 @@ module ActiveStorage # * Height (pixels) # * Duration (seconds) # * Angle (degrees) - # * Aspect ratio + # * Display aspect ratio # # Example: # # ActiveStorage::VideoAnalyzer.new(blob).metadata - # # => { width: 640, height: 480, duration: 5.0, angle: 0, aspect_ratio: [4, 3] } + # # => { 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. # @@ -25,24 +25,24 @@ module ActiveStorage end def metadata - { width: width, height: height, duration: duration, angle: angle, aspect_ratio: aspect_ratio }.compact + { width: width, height: height, duration: duration, angle: angle, display_aspect_ratio: display_aspect_ratio }.compact end private def width - rotated? ? raw_height : raw_width + if rotated? + computed_height || encoded_height + else + encoded_width + end end def height - rotated? ? raw_width : raw_height - end - - def raw_width - Integer(video_stream["width"]) if video_stream["width"] - end - - def raw_height - Integer(video_stream["height"]) if video_stream["height"] + if rotated? + encoded_width + else + computed_height || encoded_height + end end def duration @@ -53,16 +53,40 @@ module ActiveStorage Integer(tags["rotate"]) if tags["rotate"] end - def aspect_ratio + def display_aspect_ratio if descriptor = video_stream["display_aspect_ratio"] - descriptor.split(":", 2).collect(&:to_i) + if terms = descriptor.split(":", 2) + numerator = Integer(terms[0]) + denominator = Integer(terms[1]) + + [numerator, denominator] unless numerator == 0 + end end end + def rotated? angle == 90 || angle == 270 end + def computed_height + if encoded_width && display_height_scale + encoded_width * display_height_scale + end + end + + def encoded_width + @encoded_width ||= Float(video_stream["width"]) if video_stream["width"] + end + + def encoded_height + @encoded_height ||= Float(video_stream["height"]) if video_stream["height"] + end + + def display_height_scale + @display_height_scale ||= Float(display_aspect_ratio.last) / display_aspect_ratio.first if display_aspect_ratio + end + def tags @tags ||= video_stream["tags"] || {} diff --git a/activestorage/test/analyzer/video_analyzer_test.rb b/activestorage/test/analyzer/video_analyzer_test.rb index 7fc1cfce06..2612006551 100644 --- a/activestorage/test/analyzer/video_analyzer_test.rb +++ b/activestorage/test/analyzer/video_analyzer_test.rb @@ -8,28 +8,52 @@ require "active_storage/analyzer/video_analyzer" class ActiveStorage::Analyzer::VideoAnalyzerTest < ActiveSupport::TestCase test "analyzing a video" do blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4") - metadata = blob.tap(&:analyze).metadata + metadata = extract_metadata_from(blob) assert_equal 640, metadata[:width] assert_equal 480, metadata[:height] - assert_equal [4, 3], metadata[:aspect_ratio] + assert_equal [4, 3], metadata[:display_aspect_ratio] assert_equal 5.166648, metadata[:duration] assert_not_includes metadata, :angle end test "analyzing a rotated video" do blob = create_file_blob(filename: "rotated_video.mp4", content_type: "video/mp4") - metadata = blob.tap(&:analyze).metadata + metadata = extract_metadata_from(blob) assert_equal 480, metadata[:width] assert_equal 640, metadata[:height] - assert_equal [4, 3], metadata[:aspect_ratio] + assert_equal [4, 3], metadata[:display_aspect_ratio] assert_equal 5.227975, metadata[:duration] assert_equal 90, metadata[:angle] end + test "analyzing a video with rectangular samples" do + blob = create_file_blob(filename: "video_with_rectangular_samples.mp4", content_type: "video/mp4") + metadata = extract_metadata_from(blob) + + assert_equal 1280, metadata[:width] + assert_equal 720, metadata[:height] + assert_equal [16, 9], metadata[:display_aspect_ratio] + end + + test "analyzing a video with an undefined display aspect ratio" do + blob = create_file_blob(filename: "video_with_undefined_display_aspect_ratio.mp4", content_type: "video/mp4") + metadata = extract_metadata_from(blob) + + assert_equal 640, metadata[:width] + assert_equal 480, metadata[:height] + assert_nil metadata[:display_aspect_ratio] + end + test "analyzing a video without a video stream" do blob = create_file_blob(filename: "video_without_video_stream.mp4", content_type: "video/mp4") - assert_equal({ "analyzed" => true, "identified" => true }, blob.tap(&:analyze).metadata) + metadata = extract_metadata_from(blob) + assert_equal({ "analyzed" => true, "identified" => true }, metadata) end + + private + def extract_metadata_from(blob) + blob.tap(&:analyze).metadata + end end diff --git a/activestorage/test/fixtures/files/video_with_rectangular_samples.mp4 b/activestorage/test/fixtures/files/video_with_rectangular_samples.mp4 Binary files differnew file mode 100644 index 0000000000..12b04afc87 --- /dev/null +++ b/activestorage/test/fixtures/files/video_with_rectangular_samples.mp4 diff --git a/activestorage/test/fixtures/files/video_with_undefined_display_aspect_ratio.mp4 b/activestorage/test/fixtures/files/video_with_undefined_display_aspect_ratio.mp4 Binary files differnew file mode 100644 index 0000000000..eb354e756f --- /dev/null +++ b/activestorage/test/fixtures/files/video_with_undefined_display_aspect_ratio.mp4 diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb index 11d28d12a1..5b48254646 100644 --- a/activesupport/lib/active_support/core_ext/hash/conversions.rb +++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb @@ -165,7 +165,7 @@ module ActiveSupport Hash[params.map { |k, v| [k.to_s.tr("-", "_"), normalize_keys(v)] } ] when Array params.map { |v| normalize_keys(v) } - else + else params end end @@ -178,7 +178,7 @@ module ActiveSupport process_array(value) when String value - else + else raise "can't typecast #{value.class.name} - #{value.inspect}" end end diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 82c10b3079..abc648e0c6 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -670,7 +670,7 @@ module ActiveSupport #:nodoc: when Module desc.name || raise(ArgumentError, "Anonymous modules have no name to be referenced by") - else raise TypeError, "Not a valid constant descriptor: #{desc.inspect}" + else raise TypeError, "Not a valid constant descriptor: #{desc.inspect}" end end diff --git a/activesupport/lib/active_support/deprecation/reporting.rb b/activesupport/lib/active_support/deprecation/reporting.rb index 242e21b782..2c004f4c9e 100644 --- a/activesupport/lib/active_support/deprecation/reporting.rb +++ b/activesupport/lib/active_support/deprecation/reporting.rb @@ -61,7 +61,7 @@ module ActiveSupport case message when Symbol then "#{warning} (use #{message} instead)" when String then "#{warning} (#{message})" - else warning + else warning end end diff --git a/activesupport/lib/active_support/inflector/inflections.rb b/activesupport/lib/active_support/inflector/inflections.rb index 0450a4be4c..7e5dff1d6d 100644 --- a/activesupport/lib/active_support/inflector/inflections.rb +++ b/activesupport/lib/active_support/inflector/inflections.rb @@ -227,7 +227,7 @@ module ActiveSupport case scope when :all @plurals, @singulars, @uncountables, @humans = [], [], Uncountables.new, [] - else + else instance_variable_set "@#{scope}", [] end end diff --git a/activesupport/lib/active_support/inflector/methods.rb b/activesupport/lib/active_support/inflector/methods.rb index 60eeaa77cb..7e782e2a93 100644 --- a/activesupport/lib/active_support/inflector/methods.rb +++ b/activesupport/lib/active_support/inflector/methods.rb @@ -350,7 +350,7 @@ module ActiveSupport when 1; "st" when 2; "nd" when 3; "rd" - else "th" + else "th" end end end diff --git a/activesupport/lib/active_support/multibyte/unicode.rb b/activesupport/lib/active_support/multibyte/unicode.rb index a64223c0e0..f923061fae 100644 --- a/activesupport/lib/active_support/multibyte/unicode.rb +++ b/activesupport/lib/active_support/multibyte/unicode.rb @@ -276,7 +276,7 @@ module ActiveSupport reorder_characters(decompose(:compatibility, codepoints)) when :kc compose(reorder_characters(decompose(:compatibility, codepoints))) - else + else raise ArgumentError, "#{form} is not a valid normalization variant", caller end.pack("U*".freeze) end diff --git a/activesupport/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb index 4d81ac939e..b75f5733b5 100644 --- a/activesupport/lib/active_support/values/time_zone.rb +++ b/activesupport/lib/active_support/values/time_zone.rb @@ -238,7 +238,7 @@ module ActiveSupport when Numeric, ActiveSupport::Duration arg *= 3600 if arg.abs <= 13 all.find { |z| z.utc_offset == arg.to_i } - else + else raise ArgumentError, "invalid argument to TimeZone[]: #{arg.inspect}" end end diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb index ee79f954ec..3635703002 100644 --- a/activesupport/test/cache/stores/redis_cache_store_test.rb +++ b/activesupport/test/cache/stores/redis_cache_store_test.rb @@ -5,13 +5,9 @@ require "active_support/cache" require "active_support/cache/redis_cache_store" require_relative "../behaviors" -driver_name = %w[ ruby hiredis ].include?(ENV["REDIS_DRIVER"]) ? ENV["REDIS_DRIVER"] : "hiredis" -driver = Object.const_get("Redis::Connection::#{driver_name.camelize}") - -Redis::Connection.drivers.clear -Redis::Connection.drivers.append(driver) - module ActiveSupport::Cache::RedisCacheStoreTests + DRIVER = %w[ ruby hiredis ].include?(ENV["REDIS_DRIVER"]) ? ENV["REDIS_DRIVER"] : "hiredis" + class LookupTest < ActiveSupport::TestCase test "may be looked up as :redis_cache_store" do assert_kind_of ActiveSupport::Cache::RedisCacheStore, @@ -24,7 +20,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests assert_called_with Redis, :new, [ url: nil, connect_timeout: 20, read_timeout: 1, write_timeout: 1, - reconnect_attempts: 0, + reconnect_attempts: 0, driver: DRIVER ] do build end @@ -34,7 +30,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests assert_called_with Redis, :new, [ url: nil, connect_timeout: 20, read_timeout: 1, write_timeout: 1, - reconnect_attempts: 0, + reconnect_attempts: 0, driver: DRIVER ] do build url: [] end @@ -44,7 +40,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests assert_called_with Redis, :new, [ url: "redis://localhost:6379/0", connect_timeout: 20, read_timeout: 1, write_timeout: 1, - reconnect_attempts: 0, + reconnect_attempts: 0, driver: DRIVER ] do build url: "redis://localhost:6379/0" end @@ -54,7 +50,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests assert_called_with Redis, :new, [ url: "redis://localhost:6379/0", connect_timeout: 20, read_timeout: 1, write_timeout: 1, - reconnect_attempts: 0, + reconnect_attempts: 0, driver: DRIVER ] do build url: %w[ redis://localhost:6379/0 ] end @@ -64,10 +60,10 @@ module ActiveSupport::Cache::RedisCacheStoreTests assert_called_with Redis, :new, [ [ url: "redis://localhost:6379/0", connect_timeout: 20, read_timeout: 1, write_timeout: 1, - reconnect_attempts: 0 ], + reconnect_attempts: 0, driver: DRIVER ], [ url: "redis://localhost:6379/1", connect_timeout: 20, read_timeout: 1, write_timeout: 1, - reconnect_attempts: 0 ], + reconnect_attempts: 0, driver: DRIVER ], ], returns: Redis.new do @cache = build url: %w[ redis://localhost:6379/0 redis://localhost:6379/1 ] assert_kind_of ::Redis::Distributed, @cache.redis @@ -83,7 +79,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests private def build(**kwargs) - ActiveSupport::Cache::RedisCacheStore.new(**kwargs).tap do |cache| + ActiveSupport::Cache::RedisCacheStore.new(driver: DRIVER, **kwargs).tap do |cache| cache.redis end end @@ -93,11 +89,11 @@ module ActiveSupport::Cache::RedisCacheStoreTests setup do @namespace = "namespace" - @cache = ActiveSupport::Cache::RedisCacheStore.new(timeout: 0.1, namespace: @namespace, expires_in: 60) + @cache = ActiveSupport::Cache::RedisCacheStore.new(timeout: 0.1, namespace: @namespace, expires_in: 60, driver: DRIVER) # @cache.logger = Logger.new($stdout) # For test debugging # For LocalCacheBehavior tests - @peek = ActiveSupport::Cache::RedisCacheStore.new(timeout: 0.1, namespace: @namespace) + @peek = ActiveSupport::Cache::RedisCacheStore.new(timeout: 0.1, namespace: @namespace, driver: DRIVER) end teardown do @@ -120,7 +116,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests include EncodedKeyCacheBehavior setup do - @cache = ActiveSupport::Cache::RedisCacheStore.new(timeout: 0.1) + @cache = ActiveSupport::Cache::RedisCacheStore.new(timeout: 0.1, driver: DRIVER) @cache.logger = nil end end diff --git a/guides/source/association_basics.md b/guides/source/association_basics.md index 02d012d702..52c30f226f 100644 --- a/guides/source/association_basics.md +++ b/guides/source/association_basics.md @@ -795,7 +795,7 @@ The `belongs_to` association creates a one-to-one match with another model. In d #### Methods Added by `belongs_to` -When you declare a `belongs_to` association, the declaring class automatically gains five methods related to the association: +When you declare a `belongs_to` association, the declaring class automatically gains 6 methods related to the association: * `association` * `association=(associate)` @@ -1146,7 +1146,7 @@ The `has_one` association creates a one-to-one match with another model. In data #### Methods Added by `has_one` -When you declare a `has_one` association, the declaring class automatically gains five methods related to the association: +When you declare a `has_one` association, the declaring class automatically gains 6 methods related to the association: * `association` * `association=(associate)` @@ -1419,7 +1419,7 @@ The `has_many` association creates a one-to-many relationship with another model #### Methods Added by `has_many` -When you declare a `has_many` association, the declaring class automatically gains 16 methods related to the association: +When you declare a `has_many` association, the declaring class automatically gains 17 methods related to the association: * `collection` * `collection<<(object, ...)` @@ -1952,7 +1952,7 @@ The `has_and_belongs_to_many` association creates a many-to-many relationship wi #### Methods Added by `has_and_belongs_to_many` -When you declare a `has_and_belongs_to_many` association, the declaring class automatically gains 16 methods related to the association: +When you declare a `has_and_belongs_to_many` association, the declaring class automatically gains 17 methods related to the association: * `collection` * `collection<<(object, ...)` diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index e152a95fd3..863a914912 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -315,11 +315,13 @@ module Rails def convert_database_option_for_jruby if defined?(JRUBY_VERSION) - case options[:database] - when "postgresql" then options[:database].replace "jdbcpostgresql" - when "mysql" then options[:database].replace "jdbcmysql" - when "sqlite3" then options[:database].replace "jdbcsqlite3" + opt = options.dup + case opt[:database] + when "postgresql" then opt[:database] = "jdbcpostgresql" + when "mysql" then opt[:database] = "jdbcmysql" + when "sqlite3" then opt[:database] = "jdbcsqlite3" end + self.options = opt.freeze end end diff --git a/railties/lib/rails/generators/generated_attribute.rb b/railties/lib/rails/generators/generated_attribute.rb index 2728459968..f7fd30a5fb 100644 --- a/railties/lib/rails/generators/generated_attribute.rb +++ b/railties/lib/rails/generators/generated_attribute.rb @@ -75,7 +75,7 @@ module Rails when :date then :date_select when :text then :text_area when :boolean then :check_box - else + else :text_field end end @@ -91,7 +91,7 @@ module Rails when :text then "MyText" when :boolean then false when :references, :belongs_to then nil - else + else "" end end diff --git a/railties/lib/rails/generators/rails/app/templates/bin/yarn.tt b/railties/lib/rails/generators/rails/app/templates/bin/yarn.tt index b4e4d95286..3d5051ec4c 100644 --- a/railties/lib/rails/generators/rails/app/templates/bin/yarn.tt +++ b/railties/lib/rails/generators/rails/app/templates/bin/yarn.tt @@ -1,7 +1,7 @@ APP_ROOT = File.expand_path('..', __dir__) Dir.chdir(APP_ROOT) do begin - exec "yarnpkg #{ARGV.join(' ')}" + exec %w(yarnpkg) + ARGV rescue Errno::ENOENT $stderr.puts "Yarn executable was not detected in the system." $stderr.puts "Download Yarn at https://yarnpkg.com/en/docs/install" |