From 6aeccd3c8ea84e0d5a11a255a79243a3a206c50b Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 10 Feb 2015 10:31:17 -0700 Subject: [PATCH] Uniqueness: Support PG array columns for scopes Please note that array columns do not work properly between Rails 4.0 and 4.2. * If you set an array-of-date or array-of-time attribute to an array, it will fail to hold that value. This is an issue with timestamp-aware attributes and is fixed in Rails 4.2. * Even if this worked, the uniqueness validator cannot cope with array columns, producing an invalid query. This will be fixed in 4.1.10. --- NEWS.md | 13 ++- .../validate_uniqueness_of_matcher.rb | 31 +++++-- .../unit/helpers/active_record_versions.rb | 4 + spec/support/unit/helpers/database_helpers.rb | 2 + .../validate_uniqueness_of_matcher_spec.rb | 86 +++++++++++++++++-- 5 files changed, 122 insertions(+), 14 deletions(-) diff --git a/NEWS.md b/NEWS.md index c2c4e977..f20c702f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -29,7 +29,18 @@ * Fix `validate_uniqueness_of` + `scoped_to` so that when one of the scope attributes is a UUID column that ends in an "f", the matcher is able to - generate a proper "next" value without erroring. + generate a proper "next" value without erroring. ([#402], [#587], [#662]) + + * Fix `validate_uniqueness_of` so that it works with scope attributes that are + PostgreSQL array columns. Please note that this is only supported for + Rails 4.2 and greater, as versions before this cannot handle array columns + correctly, particularly in conjunction with the uniqueness validator. + ([#554]) + +[#402]: https://github.com/thoughtbot/shoulda-matchers/pull/402 +[#587]: https://github.com/thoughtbot/shoulda-matchers/pull/587 +[#662]: https://github.com/thoughtbot/shoulda-matchers/pull/662 +[#554]: https://github.com/thoughtbot/shoulda-matchers/pull/554 * Fix `define_enum_for` so that it actually tests that the attribute is present in the list of defined enums, as you could fool it by merely defining a class diff --git a/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb b/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb index f403c1db..164920ba 100644 --- a/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb +++ b/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb @@ -304,6 +304,7 @@ module Shoulda instance.__send__("#{@attribute}=", value_for_new_record(options)) ensure_secure_password_set(instance) instance.save(validate: false) + pp all_records: @subject.class.all @created_record = instance end end @@ -371,10 +372,12 @@ module Shoulda @options[:scopes].all? do |scope| previous_value = all_records.map(&scope).max - # Assume the scope is a foreign key if the field is nil - previous_value ||= correct_type_for_column(@subject.class.columns_hash[scope.to_s]) - - next_value = next_value_for(scope, previous_value) + next_value = + if previous_value.blank? + dummy_value_for(scope) + else + next_value_for(scope, previous_value) + end @subject.__send__("#{scope}=", next_value) @@ -392,9 +395,17 @@ module Shoulda end end - def correct_type_for_column(column) + def dummy_value_for(scope) column = column_for(scope) + if column.array + [ dummy_scalar_value_for(column) ] + else + dummy_scalar_value_for(column) + end + end + + def dummy_scalar_value_for(column) case column.type when :string '0' @@ -410,6 +421,16 @@ module Shoulda def next_value_for(scope, previous_value) column = column_for(scope) + if previous_value.is_a?(Array) + [ next_scalar_value_for(scope, previous_value[0]) ] + else + next_scalar_value_for(scope, previous_value) + end + end + + def next_scalar_value_for(scope, previous_value) + column = column_for(scope) + if column.type == :uuid SecureRandom.uuid elsif defined_as_enum?(scope) diff --git a/spec/support/unit/helpers/active_record_versions.rb b/spec/support/unit/helpers/active_record_versions.rb index afc7b445..55184348 100644 --- a/spec/support/unit/helpers/active_record_versions.rb +++ b/spec/support/unit/helpers/active_record_versions.rb @@ -20,5 +20,9 @@ module UnitTests def active_record_supports_has_secure_password? active_record_version >= 3.1 end + + def active_record_supports_array_columns? + active_record_version > 4.2 + end end end diff --git a/spec/support/unit/helpers/database_helpers.rb b/spec/support/unit/helpers/database_helpers.rb index e3ab073e..93598e54 100644 --- a/spec/support/unit/helpers/database_helpers.rb +++ b/spec/support/unit/helpers/database_helpers.rb @@ -12,5 +12,7 @@ module UnitTests def database_supports_uuid_columns? database_adapter == :postgresql end + alias_method :database_supports_array_columns?, + :database_supports_uuid_columns? end end diff --git a/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb index 006972be..f2b649d4 100644 --- a/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb @@ -4,6 +4,7 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo shared_context 'it supports scoped attributes of a certain type' do |options = {}| column_type = options.fetch(:column_type) value_type = options.fetch(:value_type, column_type) + array = options.fetch(:array, false) context 'when the correct scope is specified' do context 'when the subject is a new record' do @@ -34,11 +35,11 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo context "when more than one record exists that has the next version of the attribute's value" do it 'accepts' do - value1 = dummy_value_for(value_type) + value1 = dummy_value_for(value_type, array: array) value2 = next_version_of(value1, value_type) value3 = next_version_of(value2, value_type) model = define_model_validating_uniqueness( - scopes: [ build_attribute(name: :scope) ], + scopes: [ build_attribute(name: :scope) ] ) create_record_from(model, scope: value2) create_record_from(model, scope: value3) @@ -95,6 +96,13 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo ) expect(record).not_to validate_uniqueness end + + it 'accepts if the scope is unset beforehand' do + record = build_record_validating_uniqueness( + scopes: [ build_attribute(name: :scope, value: nil) ] + ) + expect(record).to validate_uniqueness + end end context 'when a non-existent attribute is specified as a scope' do @@ -107,7 +115,11 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo end define_method(:build_attribute) do |options| - options.merge(column_type: column_type, value_type: value_type) + options.merge( + column_type: column_type, + value_type: value_type, + array: array + ) end end @@ -323,6 +335,45 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo column_type: :uuid end end + + if database_supports_array_columns? && active_record_supports_array_columns? + context 'when one of the scoped attributes is a array-of-string column' do + include_examples 'it supports scoped attributes of a certain type', + column_type: :string, + array: true + end + + context 'when one of the scoped attributes is an array-of-integer column' do + include_examples 'it supports scoped attributes of a certain type', + column_type: :integer, + array: true + end + + context 'when one of the scoped attributes is an array-of-date column' do + include_examples 'it supports scoped attributes of a certain type', + column_type: :date, + array: true + end + + context 'when one of the scoped attributes is an array-of-datetime column (using DateTime)' do + include_examples 'it supports scoped attributes of a certain type', + column_type: :datetime, + array: true + end + + context 'when one of the scoped attributes is an array-of-datetime column (using Time)' do + include_examples 'it supports scoped attributes of a certain type', + column_type: :datetime, + value_type: :time, + array: true + end + + context 'when one of the scoped attributes is an array-of-text column' do + include_examples 'it supports scoped attributes of a certain type', + column_type: :text, + array: true + end + end end context 'when the model has a case-sensitive validation on a string attribute' do @@ -579,6 +630,7 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo { value_type: :string, column_type: :string, + array: false } end @@ -603,10 +655,15 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo def column_options_from(attributes) attributes.inject({}) do |options, attribute| - options[attribute[:name]] = { + options_for_attribute = options[attribute[:name]] = { type: attribute[:column_type], options: attribute.fetch(:options, {}) } + + if attribute[:array] + options_for_attribute[:options][:array] = attribute[:array] + end + options end end @@ -614,12 +671,23 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo def attributes_with_values_for(model) model_attributes[model].each_with_object({}) do |attribute, attrs| attrs[attribute[:name]] = attribute.fetch(:value) do - dummy_value_for(attribute[:value_type]) + dummy_value_for( + attribute[:value_type], + array: attribute[:array] + ) end end end - def dummy_value_for(attribute_type) + def dummy_value_for(attribute_type, array: false) + if array + [ dummy_scalar_value_for(attribute_type) ] + else + dummy_scalar_value_for(attribute_type) + end + end + + def dummy_scalar_value_for(attribute_type) case attribute_type when :string, :text 'dummy value' @@ -639,9 +707,11 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo end def next_version_of(value, value_type) - if value_type == :uuid + if value.is_a?(Array) + [ next_version_of(value[0], value_type) ] + elsif value_type == :uuid SecureRandom.uuid - elsif value_type == :time + elsif value.is_a?(Time) value + 1 elsif value.respond_to?(:next) value.next