From f252a59be029927be8b4d61f006d08371dfc7e23 Mon Sep 17 00:00:00 2001 From: Mathieu EUSTACHY Date: Wed, 1 Jan 2025 15:12:48 +0100 Subject: [PATCH] [Validator] Update base iterable method for attachables --- .../content_type_validator.rb | 14 ++++------ .../duration_validator.rb | 2 +- .../shared/asv_attachable.rb | 28 ++++++++++++------- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/lib/active_storage_validations/content_type_validator.rb b/lib/active_storage_validations/content_type_validator.rb index 078022a5..23b65eb3 100644 --- a/lib/active_storage_validations/content_type_validator.rb +++ b/lib/active_storage_validations/content_type_validator.rb @@ -34,11 +34,9 @@ def validate_each(record, attribute, _value) @authorized_content_types = authorized_content_types_from_options(record) return if @authorized_content_types.empty? - checked_files = disable_spoofing_protection? ? attached_files(record, attribute) : attachables_from_changes(record, attribute) - - checked_files.each do |file| - set_attachable_cached_values(file) - is_valid?(record, attribute, file) + attachables_and_blobs(record, attribute).each do |attachable, blob| + set_attachable_cached_values(blob) + is_valid?(record, attribute, attachable) end end @@ -55,9 +53,9 @@ def authorized_content_types_from_options(record) end end - def set_attachable_cached_values(attachable) - @attachable_content_type = disable_spoofing_protection? ? attachable.blob.content_type : attachable_content_type_rails_like(attachable) - @attachable_filename = disable_spoofing_protection? ? attachable.blob.filename.to_s : attachable_filename(attachable).to_s + def set_attachable_cached_values(blob) + @attachable_content_type = blob.content_type + @attachable_filename = blob.filename.to_s end # Check if the provided content_type is authorized and not spoofed against diff --git a/lib/active_storage_validations/duration_validator.rb b/lib/active_storage_validations/duration_validator.rb index 6c7f5b6f..91676726 100644 --- a/lib/active_storage_validations/duration_validator.rb +++ b/lib/active_storage_validations/duration_validator.rb @@ -20,7 +20,7 @@ def validate_each(record, attribute, _value) flat_options = set_flat_options(record) - attachables_from_changes(record, attribute).each do |attachable| + attachables_and_blobs(record, attribute).each do |attachable, blob| duration = metadata_for(attachable)[:duration] if duration.to_i <= 0 diff --git a/lib/active_storage_validations/shared/asv_attachable.rb b/lib/active_storage_validations/shared/asv_attachable.rb index e5570632..70a68885 100644 --- a/lib/active_storage_validations/shared/asv_attachable.rb +++ b/lib/active_storage_validations/shared/asv_attachable.rb @@ -17,21 +17,29 @@ module ASVAttachable # attachables is the only way to get the attached file io that is necessary # to perform file analyses. def validate_changed_files_from_metadata(record, attribute) - attachables_from_changes(record, attribute).each do |attachable| + attachables_and_blobs(record, attribute).each do |attachable, blob| is_valid?(record, attribute, attachable, metadata_for(attachable)) end end - # Retrieve an array of newly submitted attachables. Some file could be passed - # several times, we just need to perform the analysis once on the file, - # therefore the use of #uniq. - def attachables_from_changes(record, attribute) + # Retrieve an array-like of attachables and blobs. Unlike its name suggests, + # getting attachables from attachment_changes is not getting the changed + # attachables but all attachables from the has_many_attached relation. + # See #attach at: https://github.com/rails/rails/blob/main/activestorage/lib/active_storage/attached/many.rb + # + # Some file could be passed several times, we just need to perform the + # analysis once on the file, therefore the use of #uniq. + def attachables_and_blobs(record, attribute) changes = record.attachment_changes[attribute.to_s] - return [] if changes.blank? + return to_enum(:attachables_and_blobs, record, attribute) if changes.blank? || !block_given? - Array.wrap( - changes.is_a?(ActiveStorage::Attached::Changes::CreateMany) ? changes.attachables : changes.attachable - ).uniq + if changes.is_a?(ActiveStorage::Attached::Changes::CreateMany) + changes.attachables.uniq.zip(changes.blobs.uniq).each do |attachable, blob| + yield attachable, blob + end + else + yield changes.attachable, changes.blob + end end # Retrieve the full declared content_type from attachable. @@ -62,7 +70,7 @@ def full_attachable_content_type(attachable) def attachable_content_type(attachable) full_attachable_content_type(attachable) && full_attachable_content_type(attachable).downcase.split(/[;,\s]/, 2).first end - + # Retrieve the content_type from attachable using the same logic as Rails # ActiveStorage::Blob::Identifiable#identify_content_type def attachable_content_type_rails_like(attachable)