From 20e3da3a680bd768abd5c85ca02cee4eeb7bb084 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 25 Jul 2019 22:31:21 -0700 Subject: [PATCH] Fix presence matcher w/ custom serializer When using the presence matcher on an attribute that is defined with `serialize` and using a custom serializer instead of a Rails built-in, the matcher failed spectacularly. This commit fixes that. --- .../validate_presence_of_matcher.rb | 15 ++--- .../active_record/serialize_matcher.rb | 8 +-- lib/shoulda/matchers/rails_shim.rb | 67 +++++++++++-------- .../validate_presence_of_matcher_spec.rb | 39 +++++++++-- 4 files changed, 80 insertions(+), 49 deletions(-) diff --git a/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb index 3efc7c21..de770b07 100644 --- a/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb @@ -314,10 +314,11 @@ validation for you? Instead of using `validate_presence_of`, try def attribute_accepts_string_values? if association? false - elsif attribute_serializer - attribute_serializer.object_class == String + elsif attribute_serialization_coder.respond_to?(:object_class) + attribute_serialization_coder.object_class == String else - attribute_type.try(:type) == :string + RailsShim.supports_full_attributes_api?(model) && + attribute_type.try(:type) == :string end end @@ -355,12 +356,8 @@ validation for you? Instead of using `validate_presence_of`, try end end - def attribute_serializer - if attribute_type.respond_to?(:coder) - attribute_type.coder - else - nil - end + def attribute_serialization_coder + RailsShim.attribute_serialization_coder_for(model, @attribute) end def attribute_type diff --git a/lib/shoulda/matchers/active_record/serialize_matcher.rb b/lib/shoulda/matchers/active_record/serialize_matcher.rb index 83c240b9..ca8965b0 100644 --- a/lib/shoulda/matchers/active_record/serialize_matcher.rb +++ b/lib/shoulda/matchers/active_record/serialize_matcher.rb @@ -183,15 +183,11 @@ module Shoulda end def attribute_is_serialized? - serialized_attributes.include?(@name) + !!serialization_coder end def serialization_coder - serialized_attributes[@name] - end - - def serialized_attributes - Shoulda::Matchers::RailsShim.serialized_attributes_for(model) + RailsShim.attribute_serialization_coder_for(model, @name) end def model diff --git a/lib/shoulda/matchers/rails_shim.rb b/lib/shoulda/matchers/rails_shim.rb index 3d6df307..929b6485 100644 --- a/lib/shoulda/matchers/rails_shim.rb +++ b/lib/shoulda/matchers/rails_shim.rb @@ -69,21 +69,20 @@ module Shoulda end def serialized_attributes_for(model) - if defined?(::ActiveRecord::Type::Serialized) - # Rails 5+ - serialized_columns = model.columns.select do |column| - model.type_for_attribute(column.name).is_a?( - ::ActiveRecord::Type::Serialized, - ) + attribute_types_for(model). + inject({}) do |hash, (attribute_name, attribute_type)| + if attribute_type.is_a?(::ActiveRecord::Type::Serialized) + hash.merge(attribute_name => attribute_type.coder) + else + hash + end end + rescue NotImplementedError + {} + end - serialized_columns.inject({}) do |hash, column| - hash[column.name.to_s] = model.type_for_attribute(column.name).coder - hash - end - else - model.serialized_attributes - end + def attribute_serialization_coder_for(model, attribute_name) + serialized_attributes_for(model)[attribute_name.to_s] end def type_cast_default_for(model, column) @@ -156,14 +155,35 @@ module Shoulda nil end - def attribute_type_for(model, attribute_name) - if supports_full_attributes_api?(model) - model.attribute_types[attribute_name.to_s] + def attribute_types_for(model) + if model.respond_to?(:attribute_types) + model.attribute_types + elsif model.respond_to?(:type_for_attribute) + model.columns.inject({}) do |hash, column| + key = column.name.to_s + value = model.type_for_attribute(column.name) + hash.merge(key => value) + end else - LegacyAttributeType.new(model, attribute_name) + raise NotImplementedError end end + def attribute_type_for(model, attribute_name) + attribute_types_for(model)[attribute_name.to_s] + rescue NotImplementedError + if model.respond_to?(:type_for_attribute) + model.type_for_attribute(attribute_name) + else + FakeAttributeType.new(model, attribute_name) + end + end + + def supports_full_attributes_api?(model) + defined?(::ActiveModel::Attributes) && + model.respond_to?(:attribute_types) + end + private def simply_generate_validation_message( @@ -188,23 +208,14 @@ module Shoulda I18n.translate(primary_translation_key, translate_options) end - def supports_full_attributes_api?(model) - defined?(::ActiveModel::Attributes) && - model.respond_to?(:attribute_types) - end - - class LegacyAttributeType + class FakeAttributeType def initialize(model, attribute_name) @model = model @attribute_name = attribute_name end def coder - if model.respond_to?(:serialized_attributes) - ActiveSupport::Deprecation.silence do - model.serialized_attributes[attribute_name.to_s] - end - end + nil end private diff --git a/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb index a7ccf1fe..49c12e1f 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb @@ -57,23 +57,50 @@ this could not be proved. end context 'when the attribute is decorated with serialize' do - context 'and the type is a string' do + context 'and the serializer is a built-in Ruby type' do + context 'and the type is a string' do + it 'still works' do + record = record_validating_presence_of(:traits) do + serialize :traits, String + end + + expect(record).to validate_presence_of(:traits) + end + end + + context 'and the type is not a string' do + it 'still works' do + record = record_validating_presence_of(:traits) do + serialize :traits, Array + end + + expect(record).to validate_presence_of(:traits) + end + end + end + + context 'and the serializer is JSON' do it 'still works' do record = record_validating_presence_of(:traits) do - serialize :traits, String + serialize :traits, JSON end expect(record).to validate_presence_of(:traits) end end - context 'and the type is not a string' do + context 'and the serializer is something custom' do it 'still works' do - record = record_validating_presence_of(:traits) do - serialize :traits, Array + serializer = Class.new do + define_singleton_method(:dump) { |value| value } + define_singleton_method(:load) { |value| value } end - expect(record).to validate_presence_of(:traits) + record = record_validating_presence_of(:data) do + serialize :data, serializer + end + + expect(record).to validate_presence_of(:data) end end end