Fix inclusion bug exceeding max value #642

mcmire · 2015-01-20T20:49:37Z

This is a fix for both #634 and #637.

In Rails 4.2, ActiveRecord was changed such that if you attempt to set
an attribute to a value and that value is outside the range of the
column, then it will raise a RangeError. For instance, an integer column
with a limit of 2 (i.e. a smallint) only accepts values between -32768
and +32767.

This means that if you try to do any of these three things, a RangeError
could be raised:

Use validate_numericality_of along with any of the comparison
submatchers and a value that sits on either side of the boundary.
Use allow_value with a value that sits outside the range.
Use validates_inclusion_of against an integer column. (Here we attempt
to set that column to a non-integer value to verify that the attribute
does not allow said value. That value is really a string version of a
large number, so if the column does not take large numbers then the
matcher could blow up.)

Ancillary changes in this commit:

Remove ValidationMessageFinder and ExceptionMessageFinder in favor of
Validator, StrictValidator, and ValidatorWithCapturedRangeError.
The allow_value matcher now uses an instance of Validator under the
hood. StrictValidator and/or ValidatorWithCapturedRangeError may be
mixed into the Validator object as needed.

houndci-bot · 2015-01-20T20:49:44Z

lib/shoulda/matchers/active_model/allow_value_matcher.rb

-        def message_finder
-          message_finder_factory.new(instance, attribute_to_check_message_against, context)
+        def validator
+          @_validator ||= Validator.new(attribute_to_check_message_against).tap do |v|


Line is too long. [86/80]

houndci-bot · 2015-01-21T23:58:19Z

lib/shoulda/matchers/active_model.rb

@@ -1,5 +1,6 @@
 require 'shoulda/matchers/active_model/helpers'
 require 'shoulda/matchers/active_model/validation_matcher'
+require 'shoulda/matchers/active_model/validator'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2015-01-23T02:30:17Z

lib/shoulda/matchers/active_model/allow_value_matcher.rb

-          :context, :value, :matched_error, :after_setting_value_callback
+        attr_reader :attribute_to_check_message_against
+        attr_accessor :values_to_match, :instance, :attribute_to_set, :value,
+          :matched_error, :after_setting_value_callback, :validator


Align the parameters of a method call if they span more than one line.

houndci-bot · 2015-01-23T02:33:08Z

lib/shoulda/matchers/active_model/validator.rb

+        protected
+
+        attr_reader :attribute, :context, :strict, :record,
+          :captured_range_error


Align the parameters of a method call if they span more than one line.

houndci-bot · 2015-01-23T02:40:48Z

lib/shoulda/matchers/active_model/strict_validator.rb

+          if expected_message
+            "exception to include #{expected_message.inspect}"
+          else
+            "an exception to have been raised"


Prefer single-quoted strings when you don't need string interpolation or special symbols.

houndci-bot · 2015-01-23T02:40:49Z

lib/shoulda/matchers/rails_shim.rb

+        simply_generate_validation_message(attribute, type, model_name, options)
+      end
+
+      def self.simply_generate_validation_message(attribute, type, model_name, options)


Line is too long. [87/80]

houndci-bot · 2015-01-23T02:47:22Z

lib/shoulda/matchers/rails_shim.rb

+          :"errors.attributes.#{attribute}.#{type}",
+          :"errors.messages.#{type}"
+        ]
+        primary_translation_key = :"activerecord.errors.models.#{model_name}.attributes.#{attribute}.#{type}"


Line is too long. [109/80]

houndci-bot · 2015-01-23T02:47:53Z

spec/unit/shoulda/matchers/active_model/allow_value_matcher_spec.rb

+            end
+            message = <<-MESSAGE.strip_heredoc.strip
+              Did not expect errors to include "some message" when attr is set to 100000,
+              got RangeError: "100000 is out of range for ActiveRecord::Type::Integer with limit 2"


Line is too long. [99/80]

...and also ComparisonMatcher

In Rails 4.2, ActiveRecord was changed such that if you attempt to set an attribute to a value and that value is outside the range of the column, then it will raise a RangeError. For instance, an integer column with a limit of 2 (i.e. a smallint) only accepts values between -32768 and +32767. This means that if you try to do any of these three things, a RangeError could be raised: * Use validate_numericality_of along with any of the comparison submatchers and a value that sits on either side of the boundary. * Use allow_value with a value that sits outside the range. * Use validates_inclusion_of against an integer column. (Here we attempt to set that column to a non-integer value to verify that the attribute does not allow said value. That value is really a string version of a large number, so if the column does not take large numbers then the matcher could blow up.) Ancillary changes in this commit: * Remove ValidationMessageFinder and ExceptionMessageFinder in favor of Validator, StrictValidator, and ValidatorWithCapturedRangeError. * The allow_value matcher now uses an instance of Validator under the hood. StrictValidator and/or ValidatorWithCapturedRangeError may be mixed into the Validator object as needed.

houndci-bot · 2015-01-23T04:05:13Z

spec/unit/shoulda/matchers/active_model/allow_value_matcher_spec.rb

+                strict
+            end
+            message = <<-MESSAGE.strip_heredoc.strip
+              Did not expect an exception to have been raised when attr is set to 100000,


Line is too long. [89/80]

pkgsrc change: allow build on Ruby 2.2. # 2.8.0 ### Deprecations * `ensure_length_of` has been renamed to `validate_length_of`. `ensure_length_of` is deprecated and will be removed in 3.0.0. * `set_the_flash` has been renamed to `set_flash`. `set_the_flash` is deprecated and will be removed in 3.0.0. * `set_session(:foo)` is deprecated in favor of `set_session[:foo]`. `set_session(:foo)` will be invalid syntax in 3.0.0. * Using `should set_session[:key].to(nil)` to assert that that a value has not been set is deprecated. Please use `should_not set_session[:key]` instead. In 3.0.0, `should set_session[:key].to(nil)` will only pass if the value is truly nil. ### Bug fixes * Fix `delegate_method` so that it works again with shoulda-context. ([#591]) * Fix `validate_uniqueness_of` when used with `scoped_to` so that when one of the scope attributes is a polymorphic `*_type` attribute and the model has another validation on the same attribute, the matcher does not fail with an error. ([#592]) * Fix `has_many` used with `through` so that when the association does not exist, and the matcher fails, it does not raise an error when producing the failure message. ([#588]) * Fix `have_and_belong_to_many` used with `join_table` so that it does not fail when `foreign_key` and/or `association_foreign_key` was specified on the association as a symbol instead of a string. ([#584]) * Fix `allow_value` when an i18n translation key is passed to `with_message` and the `:against` option is used to specify an alternate attribute. A bug here also happened to affect `validate_confirmation_of` when an i18n translation key is passed to `with_message`. ([#593]) * Fix `class_name` qualifier for association matchers so that if the model being referenced is namespaced, the matcher will correctly resolve the class before checking it against the association's `class_name`. ([#537]) * Fix `validate_inclusion_of` used with `with_message` so that it fails if given a message that does not match the message on the validation. ([#598]) * Fix `route` matcher so that when controller and action are specified in hash notation (e.g. `posts#show`), route parameters such as `id` do not need to be specified as a string but may be specified as a number as well. ([#602]) * Fix `allow_value`, `validate_numericality_of` and `validate_inclusion_of` so that they handle RangeErrors emitted from ActiveRecord 4.2. These exceptions arise whenever we attempt to set an attribute using a value that lies outside the range of the column (assuming the column is an integer). RangeError is now treated specially, failing the test instead of bubbling up as an error. ([#634], [#637], [#642]) ### Features * Add ability to test `:primary_key` option on associations. ([#597]) * Add `allow_blank` qualifier to `validate_uniqueness_of` to complement the `allow_blank` option. ([#543]) * Change `set_session` so that #[] and #to qualifiers are optional, similar to `set_flash`. That is, you can now say `should set_session` to assert that any flash value has been set, or `should set_session.to('value')` to assert that any value in the session is 'value'. * Change `set_session` so that its #to qualifier supports regexps, similar to `set_flash`. * Add `with_prefix` qualifier to `delegate_method` to correspond to the `prefix` option for Rails's `delegate` macro. ([#622]) * Add support for Rails 4.2, especially fixing `serialize` matcher to remove warning about `serialized_attributes` being deprecated. ([#627]) * Update `dependent` qualifier on association matchers to support `:destroy`, `:delete`, `:nullify`, `:restrict`, `:restrict_with_exception`, and `:restrict_with_error`. You can also pass `true` or `false` to assert that the association has (or has not) been declared with *any* dependent option. ([#631]) ### Improvements * Tweak `allow_value` failure message so that it reads a bit nicer when listing existing errors. [#591]: thoughtbot/shoulda-matchers#591 [#592]: thoughtbot/shoulda-matchers#592 [#588]: thoughtbot/shoulda-matchers#588 [#584]: thoughtbot/shoulda-matchers#584 [#593]: thoughtbot/shoulda-matchers#593 [#597]: thoughtbot/shoulda-matchers#597 [#537]: thoughtbot/shoulda-matchers#537 [#598]: thoughtbot/shoulda-matchers#598 [#602]: thoughtbot/shoulda-matchers#602 [#543]: thoughtbot/shoulda-matchers#543 [#622]: thoughtbot/shoulda-matchers#622 [#627]: thoughtbot/shoulda-matchers#627 [#631]: thoughtbot/shoulda-matchers#631 [#634]: thoughtbot/shoulda-matchers#634 [#637]: thoughtbot/shoulda-matchers#637 [#642]: thoughtbot/shoulda-matchers#642 # 2.7.0 ### Deprecations * `ensure_inclusion_of` has been renamed to `validate_inclusion_of`. `ensure_inclusion_of` is deprecated and will be removed in 3.0.0. * `ensure_exclusion_of` has been renamed to `validate_exclusion_of`. `ensure_exclusion_of` is deprecated and will be removed in 3.0.0. ### Bug fixes * Fix `delegate_method` so that it does not raise an error if the method that returns the delegate object is private. * Warn when `ensure_inclusion_of` is chained with `.in_array([false, true])` as well as with `.in_array([true, false])`. * Fix `set_session` so that the `to` qualifier if given nil checks that the session variable in question was set to nil (previously this actually did nothing). * Fix `filter_param` so that it works when `config.filter_parameters` contains regexes. * Fix `delegate_method` so that it can be required independent of Active Support. * Fix `validate_uniqueness_of`. When used against an unpersisted record whose model contained a non-nullable column other than the one being validated, the matcher would break. Even if the test set that column to a value beforehand, the record had to be persisted in order for the matcher to work. Now this is no longer the case and the record can remain unpersisted. * Fix `validate_absence_of`: it required that a string be passed as the attribute name rather than a symbol (which is the usual and documented usage). ### Improvements * `have_and_belongs_to_many` now checks to make sure that the join table contains the correct columns for the left- and right-hand side of the association. * Reword failure message for `delegate_method` so that it's a little more helpful. ### Features * Add new matcher `define_enum_for` to test usage of the `enum` macro introduced in Rails 4.1.

houndci-bot reviewed Jan 20, 2015
View reviewed changes

mcmire added this to the 2.8.0 milestone Jan 20, 2015

mcmire force-pushed the ew-fix-inclusion-bug branch from f5089a4 to e78aebe Compare January 21, 2015 23:58

houndci-bot reviewed Jan 21, 2015
View reviewed changes

mcmire mentioned this pull request Jan 23, 2015

Handle RangeError in Rails 4.2 for allow_value / validate_numericality_of #637

Closed

Use pry-nav for more debugging awesomeness

03977d1

mcmire force-pushed the ew-fix-inclusion-bug branch from d45dfdb to 0b77d1d Compare January 23, 2015 02:30