diff --git a/NEWS.md b/NEWS.md index 454ce66a..d79feaef 100644 --- a/NEWS.md +++ b/NEWS.md @@ -12,6 +12,9 @@ * The `ensure_inclusion_of` matcher now works with a decimal column. +* Fix association matchers on Rails 3 so they work when used in conjunction with + a submatcher such as #order and an association that has :include on it. + # v 2.4.0 * Fix a bug with the `validate_numericality_of` matcher that would not allow the diff --git a/lib/shoulda/matchers/active_record.rb b/lib/shoulda/matchers/active_record.rb index 51fb003f..6cd15d28 100644 --- a/lib/shoulda/matchers/active_record.rb +++ b/lib/shoulda/matchers/active_record.rb @@ -4,6 +4,7 @@ require 'shoulda/matchers/active_record/association_matchers/order_matcher' require 'shoulda/matchers/active_record/association_matchers/through_matcher' require 'shoulda/matchers/active_record/association_matchers/dependent_matcher' require 'shoulda/matchers/active_record/association_matchers/model_reflector' +require 'shoulda/matchers/active_record/association_matchers/model_reflection' require 'shoulda/matchers/active_record/association_matchers/option_verifier' require 'shoulda/matchers/active_record/have_db_column_matcher' require 'shoulda/matchers/active_record/have_db_index_matcher' diff --git a/lib/shoulda/matchers/active_record/association_matchers/model_reflection.rb b/lib/shoulda/matchers/active_record/association_matchers/model_reflection.rb new file mode 100644 index 00000000..b85b51b2 --- /dev/null +++ b/lib/shoulda/matchers/active_record/association_matchers/model_reflection.rb @@ -0,0 +1,80 @@ +require 'delegate' + +module Shoulda + module Matchers + module ActiveRecord + module AssociationMatchers + class ModelReflection < SimpleDelegator + def initialize(reflection) + super(reflection) + @reflection = reflection + @subject = reflection.active_record + end + + def associated_class + reflection.klass + end + + def through? + reflection.options[:through] + end + + def join_table + join_table = + if reflection.respond_to?(:join_table) + reflection.join_table + else + reflection.options[:join_table] + end + + join_table.to_s + end + + def association_relation + if reflection.respond_to?(:scope) + convert_scope_to_relation(reflection.scope) + else + convert_options_to_relation(reflection.options) + end + end + + private + + attr_reader :reflection, :subject + + def convert_scope_to_relation(scope) + relation = associated_class.all + + if scope + # Source: AR::Associations::AssociationScope#eval_scope + relation.instance_exec(subject, &scope) + else + relation + end + end + + def convert_options_to_relation(options) + relation = associated_class.scoped + relation = extend_relation_with(relation, :where, options[:conditions]) + relation = extend_relation_with(relation, :includes, options[:include]) + relation = extend_relation_with(relation, :order, options[:order]) + relation = extend_relation_with(relation, :group, options[:group]) + relation = extend_relation_with(relation, :having, options[:having]) + relation = extend_relation_with(relation, :limit, options[:limit]) + relation = extend_relation_with(relation, :offset, options[:offset]) + relation = extend_relation_with(relation, :select, options[:select]) + relation + end + + def extend_relation_with(relation, method_name, value) + if value + relation.__send__(method_name, value) + else + relation + end + end + end + end + end + end +end diff --git a/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb b/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb index b426b4e1..a8a9cac0 100644 --- a/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb +++ b/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb @@ -3,6 +3,9 @@ module Shoulda # :nodoc: module ActiveRecord # :nodoc: module AssociationMatchers class ModelReflector + delegate :associated_class, :through?, :join_table, + :association_relation, to: :reflection + def initialize(subject, name) @subject = subject @name = name @@ -13,57 +16,17 @@ module Shoulda # :nodoc: end def reflect_on_association(name) - model_class.reflect_on_association(name) + reflection = model_class.reflect_on_association(name) + + if reflection + ModelReflection.new(reflection) + end end def model_class subject.class end - def associated_class - reflection.klass - end - - def through? - reflection.options[:through] - end - - def join_table - if reflection.respond_to? :join_table - reflection.join_table.to_s - else - reflection.options[:join_table].to_s - end - end - - def association_relation - if reflection.respond_to?(:scope) && reflection.scope - relation_from_scope(reflection.scope) - else - options = reflection.options - relation = RailsShim.clean_scope(reflection.klass) - if options[:conditions] - relation = relation.where(options[:conditions]) - end - if options[:include] - relation = relation.include(options[:include]) - end - if options[:order] - relation = relation.order(options[:order]) - end - if options[:group] - relation = relation.group(options[:group]) - end - if options[:having] - relation = relation.having(options[:having]) - end - if options[:limit] - relation = relation.limit(options[:limit]) - end - relation - end - end - def build_relation_with_clause(name, value) case name when :conditions then associated_class.where(value) @@ -82,15 +45,6 @@ module Shoulda # :nodoc: private - def relation_from_scope(scope) - # Source: AR::Associations::AssociationScope#eval_scope - if scope.is_a?(::Proc) - associated_class.all.instance_exec(subject, &scope) - else - scope - end - end - attr_reader :subject, :name end end diff --git a/spec/shoulda/matchers/active_record/association_matchers/model_reflection_spec.rb b/spec/shoulda/matchers/active_record/association_matchers/model_reflection_spec.rb new file mode 100644 index 00000000..c8dfba80 --- /dev/null +++ b/spec/shoulda/matchers/active_record/association_matchers/model_reflection_spec.rb @@ -0,0 +1,247 @@ +require 'spec_helper' + +describe Shoulda::Matchers::ActiveRecord::AssociationMatchers::ModelReflection do + it 'delegates other methods to the given Reflection object' do + define_model(:country) + person_model = define_model(:person, country_id: :integer) do + belongs_to :country + end + delegate_reflection = person_model.reflect_on_association(:country) + delegate_reflection.stubs(foo: 'bar') + reflection = described_class.new(delegate_reflection) + + expect(reflection.foo).to eq 'bar' + end + + describe '#associated_class' do + it 'returns the model that the association refers to' do + define_model(:country) + person_model = define_model(:person, country_id: :integer) do + belongs_to :country + end + delegate_reflection = person_model.reflect_on_association(:country) + reflection = described_class.new(delegate_reflection) + + expect(reflection.associated_class).to be Country + end + end + + describe '#through?' do + it 'returns true if the reflection is for a has_many :through association' do + define_model(:city, person_id: :integer) + define_model(:person, country_id: :integer) do + has_many :cities + end + country_model = define_model(:country) do + has_many :people + has_many :cities, through: :people + end + delegate_reflection = country_model.reflect_on_association(:cities) + reflection = described_class.new(delegate_reflection) + + expect(reflection).to be_through + end + + it 'returns false if not' do + define_model(:person, country_id: :integer) + country_model = define_model(:country) do + has_many :people + end + delegate_reflection = country_model.reflect_on_association(:people) + reflection = described_class.new(delegate_reflection) + + expect(reflection).not_to be_through + end + end + + describe '#join_table' do + context 'when the association was defined with a :join_table option' do + it 'returns the value of the option' do + define_model(:person, country_id: :integer) + country_model = define_model(:country) do + has_and_belongs_to_many :people, join_table: 'foos' + end + delegate_reflection = country_model.reflect_on_association(:people) + reflection = described_class.new(delegate_reflection) + + expect(reflection.join_table).to eq 'foos' + end + end + + context 'when the association was not defined with :join_table' do + it 'returns the default join_table that ActiveRecord generates' do + define_model(:person, country_id: :integer) + country_model = define_model(:country) do + has_and_belongs_to_many :people + end + delegate_reflection = country_model.reflect_on_association(:people) + reflection = described_class.new(delegate_reflection) + + expect(reflection.join_table).to eq 'countries_people' + end + end + end + + describe '#association_relation' do + if rails_4_x? + context 'when the reflection object has a #scope method' do + context 'when the scope is a block' do + it 'executes the block in the context of an empty scope' do + define_model(:country, mood: :string) + person_model = define_model(:person, country_id: :integer) do + belongs_to :country, -> { where(mood: 'nice') } + end + delegate_reflection = person_model.reflect_on_association(:country) + reflection = described_class.new(delegate_reflection) + + actual_sql = reflection.association_relation.to_sql + expected_sql = Country.where(mood: 'nice').to_sql + expect(actual_sql).to eq expected_sql + end + end + + context 'when the scope is nil' do + it 'returns an empty scope' do + define_model(:country) + person_model = define_model(:person, country_id: :integer) do + belongs_to :country + end + delegate_reflection = person_model.reflect_on_association(:country) + reflection = described_class.new(delegate_reflection) + + actual_sql = reflection.association_relation.to_sql + expected_sql = Country.all.to_sql + expect(actual_sql).to eq expected_sql + end + end + end + end + + if rails_3_x? + context 'when the reflection object does not have a #scope method' do + context 'when the reflection options contain :conditions' do + it 'creates an ActiveRecord::Relation from the conditions' do + define_model(:country, mood: :string) + person_model = define_model(:person, country_id: :integer) do + belongs_to :country, conditions: { mood: 'nice' } + end + delegate_reflection = person_model.reflect_on_association(:country) + reflection = described_class.new(delegate_reflection) + + actual_sql = reflection.association_relation.to_sql + expected_sql = Country.where(mood: 'nice').to_sql + expect(actual_sql).to eq expected_sql + end + end + + context 'when the reflection options contain :order' do + it 'creates an ActiveRecord::Relation from the order' do + define_model(:person, country_id: :integer, age: :integer) + country_model = define_model(:country) do + has_many :people, order: 'age' + end + delegate_reflection = country_model.reflect_on_association(:people) + reflection = described_class.new(delegate_reflection) + + actual_sql = reflection.association_relation.to_sql + expected_sql = Person.order('age').to_sql + expect(actual_sql).to eq expected_sql + end + end + + context 'when the reflection options contain :include' do + it 'creates an ActiveRecord::Relation from the include' do + define_model(:city, country_id: :integer) + define_model(:country) do + has_many :cities + end + person_model = define_model(:person, country_id: :integer) do + belongs_to :country, include: :cities + end + delegate_reflection = person_model.reflect_on_association(:country) + reflection = described_class.new(delegate_reflection) + + actual_includes = reflection.association_relation.includes_values + expected_includes = Country.includes(:cities).includes_values + expect(actual_includes).to eq expected_includes + end + end + + context 'when the reflection options contain :group' do + it 'creates an ActiveRecord::Relation from the group' do + country_model = define_model(:country, mood: :string) do + has_many :people, group: 'age' + end + define_model(:person, country_id: :integer, age: :integer) + delegate_reflection = country_model.reflect_on_association(:people) + reflection = described_class.new(delegate_reflection) + + actual_sql = reflection.association_relation.to_sql + expected_sql = Person.group('age').to_sql + expect(actual_sql).to eq expected_sql + end + end + + context 'when the reflection options contain :having' do + it 'creates an ActiveRecord::Relation from the having' do + country_model = define_model(:country) do + has_many :people, having: 'country_id > 1' + end + define_model(:person, country_id: :integer) + delegate_reflection = country_model.reflect_on_association(:people) + reflection = described_class.new(delegate_reflection) + + actual_sql = reflection.association_relation.to_sql + expected_sql = Person.having('country_id > 1').to_sql + expect(actual_sql).to eq expected_sql + end + end + + context 'when the reflection options contain :limit' do + it 'creates an ActiveRecord::Relation from the limit' do + country_model = define_model(:country) do + has_many :people, limit: 10 + end + define_model(:person, country_id: :integer) + delegate_reflection = country_model.reflect_on_association(:people) + reflection = described_class.new(delegate_reflection) + + actual_sql = reflection.association_relation.to_sql + expected_sql = Person.limit(10).to_sql + expect(actual_sql).to eq expected_sql + end + end + + context 'when the reflection options contain :offset' do + it 'creates an ActiveRecord::Relation from the offset' do + country_model = define_model(:country) do + has_many :people, offset: 5 + end + define_model(:person, country_id: :integer) + delegate_reflection = country_model.reflect_on_association(:people) + reflection = described_class.new(delegate_reflection) + + actual_sql = reflection.association_relation.to_sql + expected_sql = Person.offset(5).to_sql + expect(actual_sql).to eq expected_sql + end + end + + context 'when the reflection options contain :select' do + it 'creates an ActiveRecord::Relation from the select' do + country_model = define_model(:country) do + has_many :people, select: 'age' + end + define_model(:person, country_id: :integer, age: :integer) + delegate_reflection = country_model.reflect_on_association(:people) + reflection = described_class.new(delegate_reflection) + + actual_sql = reflection.association_relation.to_sql + expected_sql = Person.select('age').to_sql + expect(actual_sql).to eq expected_sql + end + end + end + end + end +end diff --git a/spec/support/rails_versions.rb b/spec/support/rails_versions.rb new file mode 100644 index 00000000..459ef598 --- /dev/null +++ b/spec/support/rails_versions.rb @@ -0,0 +1,18 @@ +module RailsVersions + def rails_version + Gem::Version.new(Rails::VERSION::STRING) + end + + def rails_3_x? + Gem::Requirement.new('~> 3.0').satisfied_by?(rails_version) + end + + def rails_4_x? + Gem::Requirement.new('~> 4.0').satisfied_by?(rails_version) + end +end + +RSpec.configure do |config| + config.include(RailsVersions) + config.extend(RailsVersions) +end