From a1d5ddf99d85bc962ecef5d7dfc2eab8e958e2c0 Mon Sep 17 00:00:00 2001 From: Helio Costa e Silva Date: Thu, 19 Nov 2015 14:58:55 -0200 Subject: [PATCH 1/3] Added blank_value? to size predicate Now, the application will ignore blank values for `size` predicate as well occurs for other predicates i.e. presence. The only exceptional case down to Enumerable implementations which if defined will be treated as filled value, as in examples below: ```ruby class AwesomeValidator attribute :name, type: String, size: 8..10 attribute :tags, type: Array, size: 2..10 end AwesomeValidator.new().valid? # < true AwesomeValidator.new(name: '12345678', tags: '').valid? # < true AwesomeValidator.new(tags: ['a', 'b']).valid? # < true AwesomeValidator.new(name: nil).valid? # < true AwesomeValidator.new(name: 'ooo').valid? # < false AwesomeValidator.new(tags: []).valid? # < false AwesomeValidator.new(tags: []).valid? # < false AwesomeValidator.new(name: 'abc', tags: []).valid? # < false ``` @solnic [has argued about](https://github.com/lotus/validations/issues/80#issuecomment-158085889) Conditional Validations and 'magical' validations/coercions which may introduce more complexity into this kind of issue. Closes #81 --- lib/lotus/validations/attribute.rb | 4 ++++ lib/lotus/validations/blank_value_checker.rb | 10 ++++++++++ test/composed_validation_test.rb | 13 +++++++++++++ test/fixtures.rb | 16 ++++++++++++++-- test/size_validation_test.rb | 16 ++++++++++++++++ 5 files changed, 57 insertions(+), 2 deletions(-) diff --git a/lib/lotus/validations/attribute.rb b/lib/lotus/validations/attribute.rb index 3a96c61..83de62b 100644 --- a/lib/lotus/validations/attribute.rb +++ b/lib/lotus/validations/attribute.rb @@ -171,6 +171,8 @@ def confirmation # # If the quantity is a Range, the size of the value MUST be included. # + # If the attribute's value is blank, the size will not be considered + # # The value is an object which implements `#size`. # # @raise [ArgumentError] if the defined quantity isn't a Numeric or a @@ -181,6 +183,8 @@ def confirmation # @since 0.2.0 # @api private def size + return if blank_value? + _validate(__method__) do |validator| case validator when Numeric, ->(v) { v.respond_to?(:to_int) } diff --git a/lib/lotus/validations/blank_value_checker.rb b/lib/lotus/validations/blank_value_checker.rb index 3f9a04d..661873e 100644 --- a/lib/lotus/validations/blank_value_checker.rb +++ b/lib/lotus/validations/blank_value_checker.rb @@ -38,8 +38,18 @@ def _blank_string? # @since 0.2.2 # @api private def _empty_value? + return false if _enumerable? (@value.respond_to?(:empty?) and @value.empty?) end + + # Collectable classes should not be considered as blank value + # even if it's responds _true_ to its own `empty?` method. + # + # @since x.x.x + # @api private + def _enumerable? + @value.respond_to?(:each) + end end end end diff --git a/test/composed_validation_test.rb b/test/composed_validation_test.rb index 34c3b1d..97cd6b7 100644 --- a/test/composed_validation_test.rb +++ b/test/composed_validation_test.rb @@ -40,6 +40,19 @@ validator.valid?.must_equal true end + + # Bug https://github.com/lotus/validations/issues/81 + it 'is valid if included attributes are blank and does not define presence constraint' do + validator = ComposedValidationsWithoutPresenceTest.new(email: '', name: '') + + validator.valid?.must_equal true + end + + it 'is not valid if included attributes are invalid' do + validator = ComposedValidationsWithoutPresenceTest.new(email: 'fo', name: 'o') + + validator.valid?.must_equal false + end end describe 'nested composed validations' do diff --git a/test/fixtures.rb b/test/fixtures.rb index 620b754..6466222 100644 --- a/test/fixtures.rb +++ b/test/fixtures.rb @@ -106,8 +106,8 @@ class PresenceValidatorTest class FormatValidatorTest include Lotus::Validations - attribute :name, format: /\A[a-zA-Z]+\z/ - attribute :age, type: String, format: /\A[0-9]+\z/ + attribute :name, format: /\A[a-zA-Z]+\z/ + attribute :age, type: String, format: /\A[0-9]+\z/ end class InclusionValidatorTest @@ -185,6 +185,12 @@ module EmailValidations attribute :email, presence: true, format: /@/ end +module EmailValidationsWithoutPresence + include Lotus::Validations + + attribute :email, format: /@/ +end + module CommonValidations include EmailValidations end @@ -193,6 +199,12 @@ class ComposedValidationsTest include EmailValidations end +class ComposedValidationsWithoutPresenceTest + include EmailValidationsWithoutPresence + + attribute :name, size: 8..50 +end + module PasswordValidations include Lotus::Validations diff --git a/test/size_validation_test.rb b/test/size_validation_test.rb index f454d99..4df2ae6 100644 --- a/test/size_validation_test.rb +++ b/test/size_validation_test.rb @@ -103,6 +103,22 @@ end end + # Bug https://github.com/lotus/validations/issues/81 + it 'is valid if attribute is defined as blank' do + validator = SizeValidatorTest.new(password: 'foobarbazqux', ssn: '') + + validator.valid?.must_equal true + validator.errors.must_be_empty + end + + it 'is not valid if attribute is an empty collection' do + validator = SizeValidatorTest.new(password: 'quxbazbarfoo', ssn: []) + + validator.valid?.must_equal false + errors = validator.errors.for(:ssn) + errors.must_include Lotus::Validations::Error.new(:ssn, :size, 11, []) + end + it "raises an error when the validator can't be coerced into an integer" do -> { SizeValidatorErrorTest.new(password: 'secret').valid? }.must_raise ArgumentError end From 4569ae62ebbbac2e6fad1f71eb6c95687e2946ea Mon Sep 17 00:00:00 2001 From: Helio Costa e Silva Date: Thu, 19 Nov 2015 15:31:35 -0200 Subject: [PATCH 2/3] Updated README.md to explain blank values --- README.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/README.md b/README.md index 0ccb1e4..1b37269 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,35 @@ person.email # => "me@example.org" person.age # => raises NoMethodError because `:age` wasn't defined as attribute. ``` +#### Blank Values + +The framework will treat as valid any blank attributes, __without__ `presence`, for both `format` and `size` predicates. + +```ruby +require 'lotus/validations' + +class Person + include Lotus::Validations + + attribute :name, type: String, size: 5..45 + attribute :email, type: String, size: 20..80, format: /@/ + attribute :skills, type: Array, size: 1..3 + attribute :keys, type: Hash, size: 1..3 +end + +Person.new.valid? # < true +Person.new(name: '').valid? # < true +Person.new(email: '@').valid? # < true +Person.new(skills: '').valid? # < true +Person.new(skills: ['ruby', 'lotus']).valid? # < true + +Person.new(skills: []).valid? # < false +Person.new(keys: {}).valid? # < false +Person.new(keys: {a: :b}, skills: []).valid? # < false +``` + +If you want to _disable_ this behaviour, please, refer to [presence](https://github.com/lotus/validations#presence). + ### Validations If you prefer Lotus::Validations to **only define validations**, but **not attributes**, From c4bb8b7a391b1f36321b8c92e93922a49266f0f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9lio=20Costa=20e=20Silva?= Date: Sun, 22 Nov 2015 17:36:25 -0200 Subject: [PATCH 3/3] Fixed README.md --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 1b37269..145dcba 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,6 @@ end Person.new.valid? # < true Person.new(name: '').valid? # < true -Person.new(email: '@').valid? # < true Person.new(skills: '').valid? # < true Person.new(skills: ['ruby', 'lotus']).valid? # < true