From 9a08a2f09cea1338f5e84ea4f5ee7a9ad17d5f44 Mon Sep 17 00:00:00 2001 From: Rachael Wright-Munn Date: Sun, 2 Aug 2020 12:10:39 -0400 Subject: [PATCH] Add ComparisonValidator to Rails to support validations between two comparable values. We allow for compare validations in NumericalityValidator, but these only work on numbers. There are various comparisons people may want to validate, from dates to strings, to custom comparisons. ``` validates_comparison_of :end_date, greater_than: :start_date ``` Refactor NumericalityValidator to share module Comparison with ComparabilityValidator * Move creating the option_value into a reusable module * Separate COMPARE_CHECKS which support compare functions and accept values * Move odd/even checks to NUMBER_CHECKS as they can only be run on numbers --- .../active_model/validations/comparability.rb | 38 +++ .../active_model/validations/comparison.rb | 78 ++++++ .../active_model/validations/numericality.rb | 49 ++-- .../validations/comparison_validation_test.rb | 243 ++++++++++++++++++ guides/source/active_record_validations.md | 30 +++ 5 files changed, 413 insertions(+), 25 deletions(-) create mode 100644 activemodel/lib/active_model/validations/comparability.rb create mode 100644 activemodel/lib/active_model/validations/comparison.rb create mode 100644 activemodel/test/cases/validations/comparison_validation_test.rb diff --git a/activemodel/lib/active_model/validations/comparability.rb b/activemodel/lib/active_model/validations/comparability.rb new file mode 100644 index 0000000000..2e7243ce69 --- /dev/null +++ b/activemodel/lib/active_model/validations/comparability.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module ActiveModel + module Validations + module Comparability #:nodoc: + COMPARE_CHECKS = { greater_than: :>, greater_than_or_equal_to: :>=, + equal_to: :==, less_than: :<, less_than_or_equal_to: :<=, + other_than: :!= }.freeze + + def option_value(record, option_value) + case option_value + when Proc + option_value.call(record) + when Symbol + record.send(option_value) + else + option_value + end + end + + def error_options(value, option_value) + options.except(*COMPARE_CHECKS.keys).merge!( + count: option_value, + value: value + ) + end + + def error_value(record, option_value) + case option_value + when Proc + option_value(record, option_value) + else + option_value + end + end + end + end +end diff --git a/activemodel/lib/active_model/validations/comparison.rb b/activemodel/lib/active_model/validations/comparison.rb new file mode 100644 index 0000000000..eace67e243 --- /dev/null +++ b/activemodel/lib/active_model/validations/comparison.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +module ActiveModel + module Validations + class ComparisonValidator < EachValidator # :nodoc: + include Comparability + + def check_validity! + unless (options.keys & COMPARE_CHECKS.keys).any? + raise ArgumentError, "Expected one of :greater_than, :greater_than_or_equal_to, "\ + ":equal_to, :less_than, :less_than_or_equal_to, nor :other_than supplied." + end + end + + def validate_each(record, attr_name, value) + options.slice(*COMPARE_CHECKS.keys).each do |option, raw_option_value| + if value.nil? || value.blank? + return record.errors.add(attr_name, :blank, **error_options(value, error_value(record, raw_option_value))) + end + + unless value.send(COMPARE_CHECKS[option], option_value(record, raw_option_value)) + record.errors.add(attr_name, option, **error_options(value, error_value(record, raw_option_value))) + end + rescue ArgumentError => e + record.errors.add(attr_name, e.message) + end + end + end + + module HelperMethods + # Validates the value of a specified attribute fulfills all + # defined comparisons with another value, proc, or attribute. + # + # class Person < ActiveRecord::Base + # validates_comparison_of :value, greater_than: 'the sum of its parts' + # end + # + # Configuration options: + # * :message - A custom error message (default is: "failed comparison"). + # * :greater_than - Specifies the value must be greater than the + # supplied value. + # * :greater_than_or_equal_to - Specifies the value must be + # greater than or equal the supplied value. + # * :equal_to - Specifies the value must be equal to the supplied + # value. + # * :less_than - Specifies the value must be less than the + # supplied value. + # * :less_than_or_equal_to - Specifies the value must be less + # than or equal the supplied value. + # * :other_than - Specifies the value must not be equal to the + # supplied value. + # + # There is also a list of default options supported by every validator: + # +:if+, +:unless+, +:on+, +:allow_nil+, +:allow_blank+, and +:strict+ . + # See ActiveModel::Validations#validates for more information + # + # The validator requires at least one of the following checks be supplied. + # Each will accept a proc, value, or a symbol which corresponds to a method: + # + # * :greater_than + # * :greater_than_or_equal_to + # * :equal_to + # * :less_than + # * :less_than_or_equal_to + # * :other_than + # + # For example: + # + # class Person < ActiveRecord::Base + # validates_comparison_of :birth_date, less_than_or_equal_to: -> { Date.today } + # validates_comparison_of :preferred_name, other_than: :given_name, allow_nil: true + # end + def validates_comparison_of(*attr_names) + validates_with ComparisonValidator, _merge_attributes(attr_names) + end + end + end +end diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index f6c15a70aa..9207cb275a 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -5,24 +5,25 @@ require "bigdecimal/util" module ActiveModel module Validations class NumericalityValidator < EachValidator # :nodoc: - CHECKS = { greater_than: :>, greater_than_or_equal_to: :>=, - equal_to: :==, less_than: :<, less_than_or_equal_to: :<=, - odd: :odd?, even: :even?, other_than: :!=, in: :in? }.freeze + include Comparability - RESERVED_OPTIONS = CHECKS.keys + [:only_integer] + RANGE_CHECKS = { in: :in? } + NUMBER_CHECKS = { odd: :odd?, even: :even? } + + RESERVED_OPTIONS = COMPARE_CHECKS.keys + NUMBER_CHECKS.keys + RANGE_CHECKS.keys + [:only_integer] INTEGER_REGEX = /\A[+-]?\d+\z/ HEXADECIMAL_REGEX = /\A[+-]?0[xX]/ def check_validity! - keys = CHECKS.keys - [:odd, :even, :in] - options.slice(*keys).each do |option, value| + options.slice(*COMPARE_CHECKS.keys).each do |option, value| unless value.is_a?(Numeric) || value.is_a?(Proc) || value.is_a?(Symbol) raise ArgumentError, ":#{option} must be a number, a symbol or a proc" end end - options.slice(:in).each do |option, value| + + options.slice(*RANGE_CHECKS).each do |option, value| unless value.is_a?(Range) raise ArgumentError, ":#{option} must be a range" end @@ -42,23 +43,18 @@ module ActiveModel value = parse_as_number(value, precision, scale) - options.slice(*CHECKS.keys).each do |option, option_value| - case option - when :odd, :even - unless value.to_i.public_send(CHECKS[option]) + options.slice(*RESERVED_OPTIONS).each do |option, option_value| + if NUMBER_CHECKS.keys.include? option + unless value.to_i.send(NUMBER_CHECKS[option]) record.errors.add(attr_name, option, **filtered_options(value)) end - else - case option_value - when Proc - option_value = option_value.call(record) - when Symbol - option_value = record.send(option_value) + elsif RANGE_CHECKS.keys.include? option + unless value.send(RANGE_CHECKS[option], option_value) + record.errors.add(attr_name, option, **filtered_options(value).merge!(count: option_value)) end - - option_value = parse_as_number(option_value, precision, scale, option) - - unless value.public_send(CHECKS[option], option_value) + elsif COMPARE_CHECKS.keys.include? option + option_value = option_as_number(record, option_value, precision, scale) + unless value.send(COMPARE_CHECKS[option], option_value) record.errors.add(attr_name, option, **filtered_options(value).merge!(count: option_value)) end end @@ -66,10 +62,12 @@ module ActiveModel end private - def parse_as_number(raw_value, precision, scale, option = nil) - if option == :in - raw_value if raw_value.is_a?(Range) - elsif raw_value.is_a?(Float) + def option_as_number(record, option_value, precision, scale) + parse_as_number(option_value(record, option_value), precision, scale) + end + + def parse_as_number(raw_value, precision, scale) + if raw_value.is_a?(Float) parse_float(raw_value, precision, scale) elsif raw_value.is_a?(BigDecimal) round(raw_value, scale) @@ -180,6 +178,7 @@ module ActiveModel # supplied value. # * :odd - Specifies the value must be an odd number. # * :even - Specifies the value must be an even number. + # * :in - Check that the value is within a range. # # There is also a list of default options supported by every validator: # +:if+, +:unless+, +:on+, +:allow_nil+, +:allow_blank+, and +:strict+ . diff --git a/activemodel/test/cases/validations/comparison_validation_test.rb b/activemodel/test/cases/validations/comparison_validation_test.rb new file mode 100644 index 0000000000..f635d9b9aa --- /dev/null +++ b/activemodel/test/cases/validations/comparison_validation_test.rb @@ -0,0 +1,243 @@ +# frozen_string_literal: true + +require "cases/helper" + +require "models/topic" +require "models/person" + +class ComparisonValidationTest < ActiveModel::TestCase + def teardown + Topic.clear_validators! + end + + def test_validates_comparison_with_greater_than_using_numeric + Topic.validates_comparison_of :approved, greater_than: 10 + + invalid!([-12, 10], "must be greater than 10") + valid!([11]) + end + + def test_validates_comparison_with_greater_than_using_date + date_value = Date.parse("2020-08-02") + Topic.validates_comparison_of :approved, greater_than: date_value + + invalid!([ + Date.parse("2019-08-03"), + Date.parse("2020-07-03"), + Date.parse("2020-08-01"), + Date.parse("2020-08-02"), + DateTime.new(2020, 8, 1, 12, 34)], "must be greater than 2020-08-02") + valid!([Date.parse("2020-08-03"), DateTime.new(2020, 8, 2, 12, 34)]) + end + + def test_validates_comparison_with_greater_than_using_string + Topic.validates_comparison_of :approved, greater_than: "cat" + + invalid!(["ant", "cat"], "must be greater than cat") + valid!(["dog", "whale"]) + end + + def test_validates_comparison_with_greater_than_or_equal_to_using_numeric + Topic.validates_comparison_of :approved, greater_than_or_equal_to: 10 + + invalid!([-12, 5], "must be greater than or equal to 10") + valid!([11, 10]) + end + + def test_validates_comparison_with_greater_than_or_equal_to_using_string + Topic.validates_comparison_of :approved, greater_than_or_equal_to: "cat" + + invalid!(["ant"], "must be greater than or equal to cat") + valid!(["cat", "dog", "whale"]) + end + + def test_validates_comparison_with_greater_than_or_equal_to_using_date + date_value = Date.parse("2020-08-02") + Topic.validates_comparison_of :approved, greater_than_or_equal_to: date_value + + invalid!([ + Date.parse("2019-08-03"), + Date.parse("2020-07-03"), + Date.parse("2020-08-01"), + DateTime.new(2020, 8, 1, 12, 34)], "must be greater than or equal to 2020-08-02") + valid!([Date.parse("2020-08-03"), DateTime.new(2020, 8, 2, 12, 34), Date.parse("2020-08-02")]) + end + + def test_validates_comparison_with_equal_to_using_numeric + Topic.validates_comparison_of :approved, equal_to: 10 + + invalid!([-12, 5, 11], "must be equal to 10") + valid!([10]) + end + + def test_validates_comparison_with_equal_to_using_date + date_value = Date.parse("2020-08-02") + Topic.validates_comparison_of :approved, equal_to: date_value + + invalid!([ + Date.parse("2019-08-03"), + Date.parse("2020-07-03"), + Date.parse("2020-08-01"), + DateTime.new(2020, 8, 1, 12, 34), + Date.parse("2020-08-03"), + DateTime.new(2020, 8, 2, 12, 34)], "must be equal to 2020-08-02") + valid!([Date.parse("2020-08-02"), DateTime.new(2020, 8, 2, 0, 0)]) + end + + def test_validates_comparison_with_less_than_using_numeric + Topic.validates_comparison_of :approved, less_than: 10 + + invalid!([11, 10], "must be less than 10") + valid!([-12, -5, 5]) + end + + def test_validates_comparison_with_less_than_using_date + date_value = Date.parse("2020-08-02") + Topic.validates_comparison_of :approved, less_than: date_value + + invalid!([ + Date.parse("2020-08-02"), + Date.parse("2020-08-03"), + DateTime.new(2020, 8, 2, 12, 34)], "must be less than 2020-08-02") + valid!([Date.parse("2019-08-03"), + Date.parse("2020-07-03"), + Date.parse("2020-08-01"), + DateTime.new(2020, 8, 1, 12, 34)]) + end + + def test_validates_comparison_with_less_than_or_equal_to_using_numeric + Topic.validates_comparison_of :approved, less_than_or_equal_to: 10 + + invalid!([12], "must be less than or equal to 10") + valid!([-11, 5, 10]) + end + + def test_validates_comparison_with_less_than_or_equal_to_using_date + date_value = Date.parse("2020-08-02") + Topic.validates_comparison_of :approved, less_than_or_equal_to: date_value + + invalid!([ + Date.parse("2020-08-03"), + DateTime.new(2020, 8, 2, 12, 34)], "must be less than or equal to 2020-08-02") + valid!([Date.parse("2019-08-03"), + Date.parse("2020-07-03"), + Date.parse("2020-08-01"), + Date.parse("2020-08-02"), + DateTime.new(2020, 8, 1, 12, 34)]) + end + + def test_validates_comparison_with_other_than_using_numeric + Topic.validates_comparison_of :approved, other_than: 10 + + invalid!([10], "must be other than 10") + valid!([-12, 5, 11]) + end + + def test_validates_comparison_with_other_than_using_date + date_value = Date.parse("2020-08-02") + Topic.validates_comparison_of :approved, other_than: date_value + + invalid!([Date.parse("2020-08-02"), DateTime.new(2020, 8, 2, 0, 0)], "must be other than 2020-08-02") + valid!([ + Date.parse("2019-08-03"), + Date.parse("2020-07-03"), + Date.parse("2020-08-01"), + DateTime.new(2020, 8, 1, 12, 34), + Date.parse("2020-08-03"), + DateTime.new(2020, 8, 2, 12, 34)]) + end + + def test_validates_comparison_with_proc + Topic.define_method(:requested) { Date.new(2020, 8, 1) } + Topic.validates_comparison_of :approved, greater_than_or_equal_to: Proc.new(&:requested) + + invalid!([Date.new(2020, 7, 1), Date.new(2019, 7, 1), DateTime.new(2020, 7, 1, 22, 34)]) + valid!([Date.new(2020, 8, 2), DateTime.new(2021, 8, 1)]) + ensure + Topic.remove_method :requested + end + + def test_validates_comparison_with_method + Topic.define_method(:requested) { Date.new(2020, 8, 1) } + Topic.validates_comparison_of :approved, greater_than_or_equal_to: :requested + + invalid!([Date.new(2020, 7, 1), Date.new(2019, 7, 1), DateTime.new(2020, 7, 1, 22, 34)]) + valid!([Date.new(2020, 8, 2), DateTime.new(2021, 8, 1)]) + ensure + Topic.remove_method :requested + end + + def test_validates_comparison_with_custom_compare + custom = Struct.new(:amount) { + include Comparable + + def <=>(other) + amount % 100 <=> other.amount % 100 + end + } + Topic.validates_comparison_of :approved, greater_than_or_equal_to: custom.new(1150) + + invalid!([custom.new(530), custom.new(2325)]) + valid!([custom.new(575), custom.new(250), custom.new(1999)]) + end + + def test_validates_comparison_with_blank_allowed + Topic.validates_comparison_of :approved, greater_than: "cat", allow_blank: true + + invalid!(["ant"]) + valid!([nil, ""]) + end + + def test_validates_comparison_with_nil_allowed + Topic.validates_comparison_of :approved, less_than: 100, allow_nil: true + + invalid!([200]) + valid!([nil, 50]) + end + + def test_validates_comparison_of_incomparables + Topic.validates_comparison_of :approved, less_than: "cat" + + invalid!([12], "comparison of Integer with String failed") + invalid!([nil]) + valid!([]) + end + + def test_validates_comparison_of_multiple_values + Topic.validates_comparison_of :approved, other_than: 17, greater_than: 13 + + invalid!([12, nil, 17]) + valid!([15]) + end + + def test_validates_comparison_of_no_options + error = assert_raises(ArgumentError) do + Topic.validates_comparison_of(:approved) + end + assert_equal "Expected one of :greater_than, :greater_than_or_equal_to, :equal_to," \ + " :less_than, :less_than_or_equal_to, nor :other_than supplied.", error.message + end + + private + def invalid!(values, error = nil) + with_each_topic_approved_value(values) do |topic, value| + assert topic.invalid?, "#{value.inspect} failed comparison" + assert topic.errors[:approved].any?, "FAILED for #{value.inspect}" + assert_equal error, topic.errors[:approved].first if error + end + end + + def valid!(values) + with_each_topic_approved_value(values) do |topic, value| + assert topic.valid?, "#{value.inspect} failed comparison with validation error: #{topic.errors[:approved].first}" + end + end + + def with_each_topic_approved_value(values) + topic = Topic.new(title: "comparison test", content: "whatever") + values.each do |value| + topic.approved = value + yield topic, value + end + end +end diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md index 5fbe76c518..15a6213d6c 100644 --- a/guides/source/active_record_validations.md +++ b/guides/source/active_record_validations.md @@ -387,6 +387,36 @@ end The default error message for this helper is _"doesn't match confirmation"_. +### `comparison` + +This check will validate a comparison between any two comparable values. +The validator requires a compare option be supplied. Each option accepts a +value, proc, or symbol. Any class that includes Comparable can be compared. + +```ruby +class Promotion < ApplicationRecord + validates :start_date, comparison: { greater_than: :end_date } +end +``` + +These options are all supported: + +* `:greater_than` - Specifies the value must be greater than the supplied + value. The default error message for this option is _"must be greater than + %{count}"_. +* `:greater_than_or_equal_to` - Specifies the value must be greater than or + equal to the supplied value. The default error message for this option is + _"must be greater than or equal to %{count}"_. +* `:equal_to` - Specifies the value must be equal to the supplied value. The + default error message for this option is _"must be equal to %{count}"_. +* `:less_than` - Specifies the value must be less than the supplied value. The + default error message for this option is _"must be less than %{count}"_. +* `:less_than_or_equal_to` - Specifies the value must be less than or equal to + the supplied value. The default error message for this option is _"must be + less than or equal to %{count}"_. +* `:other_than` - Specifies the value must be other than the supplied value. + The default error message for this option is _"must be other than %{count}"_. + ### `exclusion` This helper validates that the attributes' values are not included in a given