-
-
Notifications
You must be signed in to change notification settings - Fork 133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#105 Allow Procs for dynamic value usage #135
Changes from all commits
a4503c1
5b2cc9d
0cbf13c
d818ad4
88f4497
b3bcce0
aab5904
29b0e3f
d651d9c
b6a3f8a
162bbae
4dfa0e4
a8347c8
48ea8f5
7e84d89
611fc78
7a3dcab
0c9616f
804d036
ffc7297
a9047b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,3 +10,4 @@ test/dummy/storage/ | |
test/dummy/tmp/ | ||
test/dummy/.byebug_history | ||
*.gem | ||
/.idea |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,27 +2,33 @@ | |
|
||
module ActiveStorageValidations | ||
class ContentTypeValidator < ActiveModel::EachValidator # :nodoc: | ||
include OptionProcUnfolding | ||
|
||
AVAILABLE_CHECKS = %i[with in].freeze | ||
|
||
def validate_each(record, attribute, _value) | ||
return true if !record.send(attribute).attached? || types.empty? | ||
return true unless record.send(attribute).attached? | ||
|
||
types = authorized_types(record) | ||
return true if types.empty? | ||
|
||
files = Array.wrap(record.send(attribute)) | ||
|
||
errors_options = { authorized_types: types_to_human_format } | ||
errors_options = { authorized_types: types_to_human_format(types) } | ||
errors_options[:message] = options[:message] if options[:message].present? | ||
|
||
files.each do |file| | ||
next if is_valid?(file) | ||
next if is_valid?(file, types) | ||
|
||
errors_options[:content_type] = content_type(file) | ||
record.errors.add(attribute, :content_type_invalid, **errors_options) | ||
break | ||
end | ||
end | ||
|
||
def types | ||
return @types if defined? @types | ||
|
||
@types = (Array.wrap(options[:with]) + Array.wrap(options[:in])).compact.map do |type| | ||
def authorized_types(record) | ||
flat_options = unfold_procs(record, self.options, AVAILABLE_CHECKS) | ||
(Array.wrap(flat_options[:with]) + Array.wrap(flat_options[:in])).compact.map do |type| | ||
if type.is_a?(Regexp) | ||
type | ||
else | ||
|
@@ -31,7 +37,7 @@ def types | |
end | ||
end | ||
|
||
def types_to_human_format | ||
def types_to_human_format(types) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @codegeek319 probably naming should be changed, or code refactored because we have method "types", and we have parameter types and local variables. this is confusing. Do we need to pass types everywhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @igorkasyanchuk sorry for the late answer and tackling of this issue! I renamed the |
||
types | ||
.map { |type| type.to_s.split('/').last.upcase } | ||
.join(', ') | ||
|
@@ -41,7 +47,7 @@ def content_type(file) | |
file.blob.present? && file.blob.content_type | ||
end | ||
|
||
def is_valid?(file) | ||
def is_valid?(file, types) | ||
file_type = content_type(file) | ||
types.any? do |type| | ||
type == file_type || (type.is_a?(Regexp) && type.match?(file_type.to_s)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,25 +4,30 @@ | |
|
||
module ActiveStorageValidations | ||
class DimensionValidator < ActiveModel::EachValidator # :nodoc | ||
include OptionProcUnfolding | ||
|
||
AVAILABLE_CHECKS = %i[width height min max].freeze | ||
|
||
def initialize(options) | ||
def process_options(record) | ||
flat_options = unfold_procs(record, self.options, AVAILABLE_CHECKS) | ||
|
||
[:width, :height].each do |length| | ||
if options[length] and options[length].is_a?(Hash) | ||
if range = options[length][:in] | ||
if flat_options[length] and flat_options[length].is_a?(Hash) | ||
if (range = flat_options[length][:in]) | ||
raise ArgumentError, ":in must be a Range" unless range.is_a?(Range) | ||
options[length][:min], options[length][:max] = range.min, range.max | ||
flat_options[length][:min], flat_options[length][:max] = range.min, range.max | ||
end | ||
end | ||
end | ||
[:min, :max].each do |dim| | ||
if range = options[dim] | ||
if (range = flat_options[dim]) | ||
raise ArgumentError, ":#{dim} must be a Range (width..height)" unless range.is_a?(Range) | ||
options[:width] = { dim => range.first } | ||
options[:height] = { dim => range.last } | ||
flat_options[:width] = { dim => range.first } | ||
flat_options[:height] = { dim => range.last } | ||
end | ||
end | ||
super | ||
|
||
flat_options | ||
end | ||
|
||
|
||
|
@@ -64,51 +69,52 @@ def validate_each(record, attribute, _value) | |
|
||
|
||
def is_valid?(record, attribute, file_metadata) | ||
flat_options = process_options(record) | ||
# Validation fails unless file metadata contains valid width and height. | ||
if file_metadata[:width].to_i <= 0 || file_metadata[:height].to_i <= 0 | ||
add_error(record, attribute, options[:message].presence || :image_metadata_missing) | ||
add_error(record, attribute, :image_metadata_missing) | ||
return false | ||
end | ||
|
||
# Validation based on checks :min and :max (:min, :max has higher priority to :width, :height). | ||
if options[:min] || options[:max] | ||
if options[:min] && ( | ||
(options[:width][:min] && file_metadata[:width] < options[:width][:min]) || | ||
(options[:height][:min] && file_metadata[:height] < options[:height][:min]) | ||
if flat_options[:min] || flat_options[:max] | ||
if flat_options[:min] && ( | ||
(flat_options[:width][:min] && file_metadata[:width] < flat_options[:width][:min]) || | ||
(flat_options[:height][:min] && file_metadata[:height] < flat_options[:height][:min]) | ||
) | ||
add_error(record, attribute, options[:message].presence || :"dimension_min_inclusion", width: options[:width][:min], height: options[:height][:min]) | ||
add_error(record, attribute, :dimension_min_inclusion, width: flat_options[:width][:min], height: flat_options[:height][:min]) | ||
return false | ||
end | ||
if options[:max] && ( | ||
(options[:width][:max] && file_metadata[:width] > options[:width][:max]) || | ||
(options[:height][:max] && file_metadata[:height] > options[:height][:max]) | ||
if flat_options[:max] && ( | ||
(flat_options[:width][:max] && file_metadata[:width] > flat_options[:width][:max]) || | ||
(flat_options[:height][:max] && file_metadata[:height] > flat_options[:height][:max]) | ||
) | ||
add_error(record, attribute, options[:message].presence || :"dimension_max_inclusion", width: options[:width][:max], height: options[:height][:max]) | ||
add_error(record, attribute, :dimension_max_inclusion, width: flat_options[:width][:max], height: flat_options[:height][:max]) | ||
return false | ||
end | ||
|
||
# Validation based on checks :width and :height. | ||
else | ||
width_or_height_invalid = false | ||
[:width, :height].each do |length| | ||
next unless options[length] | ||
if options[length].is_a?(Hash) | ||
if options[length][:in] && (file_metadata[length] < options[length][:min] || file_metadata[length] > options[length][:max]) | ||
add_error(record, attribute, options[:message].presence || :"dimension_#{length}_inclusion", min: options[length][:min], max: options[length][:max]) | ||
next unless flat_options[length] | ||
if flat_options[length].is_a?(Hash) | ||
if flat_options[length][:in] && (file_metadata[length] < flat_options[length][:min] || file_metadata[length] > flat_options[length][:max]) | ||
add_error(record, attribute, :"dimension_#{length}_inclusion", min: flat_options[length][:min], max: flat_options[length][:max]) | ||
width_or_height_invalid = true | ||
else | ||
if options[length][:min] && file_metadata[length] < options[length][:min] | ||
add_error(record, attribute, options[:message].presence || :"dimension_#{length}_greater_than_or_equal_to", length: options[length][:min]) | ||
if flat_options[length][:min] && file_metadata[length] < flat_options[length][:min] | ||
add_error(record, attribute, :"dimension_#{length}_greater_than_or_equal_to", length: flat_options[length][:min]) | ||
width_or_height_invalid = true | ||
end | ||
if options[length][:max] && file_metadata[length] > options[length][:max] | ||
add_error(record, attribute, options[:message].presence || :"dimension_#{length}_less_than_or_equal_to", length: options[length][:max]) | ||
if flat_options[length][:max] && file_metadata[length] > flat_options[length][:max] | ||
add_error(record, attribute, :"dimension_#{length}_less_than_or_equal_to", length: flat_options[length][:max]) | ||
width_or_height_invalid = true | ||
end | ||
end | ||
else | ||
if file_metadata[length] != options[length] | ||
add_error(record, attribute, options[:message].presence || :"dimension_#{length}_equal_to", length: options[length]) | ||
if file_metadata[length] != flat_options[length] | ||
add_error(record, attribute, :"dimension_#{length}_equal_to", length: flat_options[length]) | ||
width_or_height_invalid = true | ||
end | ||
end | ||
|
@@ -120,10 +126,10 @@ def is_valid?(record, attribute, file_metadata) | |
true # valid file | ||
end | ||
|
||
def add_error(record, attribute, type, **attrs) | ||
key = options[:message].presence || type | ||
return if record.errors.added?(attribute, key) | ||
record.errors.add(attribute, key, **attrs) | ||
def add_error(record, attribute, default_message, **attrs) | ||
message = options[:message].presence || default_message | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe an interesting possibility is to allow a proc for the message as well. But lets do that in a separate PR if people would like that idea. |
||
return if record.errors.added?(attribute, message) | ||
record.errors.add(attribute, message, **attrs) | ||
end | ||
|
||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
module ActiveStorageValidations | ||
module OptionProcUnfolding | ||
|
||
def unfold_procs(record, object, only_keys = nil) | ||
case object | ||
when Hash | ||
object.merge(object) { |key, value| only_keys&.exclude?(key) ? unfold_procs(record, value, []) : unfold_procs(record, value) } | ||
when Array | ||
object.map { |o| unfold_procs(record, o, only_keys) } | ||
else | ||
object.is_a?(Proc) ? object.call(record) : object | ||
end | ||
end | ||
|
||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we reuse the code and logic from active model? Since we depend on it anyway, why not use that a bit more. For example the https://github.com/rails/rails/blob/d695972025146713fae9a089dcaf2239ede354d4/activemodel/lib/active_model/validations/resolve_value.rb#L6 method might be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That method doesn't descend into Hashes and Arrays and tries to call Symbols as methods. That's not what we need for our "options-processor". (Also it isn't yet part of any rails stable version.)
If it wasn't for Rails 5.2 compatibility I would have used
deep_transform_values
(https://apidock.com/rails/v6.0.0/Hash/deep_transform_values), but that was added in Rails 6. :(