Add ability to validate already saved attachment (needs remote download)

[gaelchriste]

Hello, in DimensionValidator is there a reason to validate image dimensions only when uploading attachment and not all the time ?

active_storage_validations/lib/active_storage_validations/dimension_validator.rb

Lines 41 to 53 in b970f95

	def validate_each(record, attribute, _value)
	return true unless record.send(attribute).attached?
	changes = record.attachment_changes[attribute.to_s]
	return true if changes.blank?
	files = Array.wrap(changes.is_a?(ActiveStorage::Attached::Changes::CreateMany) ? changes.attachables : changes.attachable)
	files.each do |file|
	metadata = Metadata.new(file).metadata
	next if is_valid?(record, attribute, metadata)
	break
	end
	end

Example:

user.avatar.attach(...) # image with wrong dimensions
user.valid? # => false
user.save(validate: false)
user.valid? # => true

Thank you!

[Mth0158]

@igorkasyanchuk any thoughts on this one?
I guess the validator should add an error when using .valid? as stated in Rails documentation

[igorkasyanchuk]

yeah, perhaps, but can create a bigger issue. If a file is stored in S3 for example, it means that to validate it we need to download it back and measure the dimensions. Probably we can mark this issue as "known issue". It can be handled somehow inside app logic.

wdyt?

[ckornaros]

@igorkasyanchuk I agree with @Mth0158 .valid? should raise an error in creation or update. I understand your point of view that it can be a issue in some case so maybe we can add an option to choose if we want to validate attachment only on creation ?

[Mth0158]

Hi @ckornaros,
Having done some work on the gem recently, I think the best approach here would be to add an option on validator needing file ios to run (basically all the are using the Attachable concern).
IMO it has to be an option because the files are stored remotely, to analyze and validate these files we have to download them.

[Mth0158]

Hi @ckornaros, @gaelchriste,

Long time no see. This issue should be solved with the v2 coming this month! ✅
This issue only affects has_one_attached relation. The has_many_attached would have validated all files again (and again and again and costing a lot of memory usage for nothing, therefore the use of blob metadata).

FYI, starting from v2 we will store the file metadata (width, height and so on) inside ActiveStorage::Blob#metadata method. Therefore, when doing the below code:

user.avatar.attach(...) # image with wrong dimensions
user.valid? # => false
user.save(validate: false)
user.valid? # => true

It should return false for the last line since the file metadata saved in the first user.valid? will be used for validation.