diff --git a/lib/active_record_where_assoc/core_logic.rb b/lib/active_record_where_assoc/core_logic.rb index fb83d78..aeadab0 100644 --- a/lib/active_record_where_assoc/core_logic.rb +++ b/lib/active_record_where_assoc/core_logic.rb @@ -364,6 +364,9 @@ def self.process_association_step_limits(current_scope, reflection, relation_kla if reflection.klass.table_name.include?(".") || option_value(options, :never_alias_limit) # We use unscoped to avoid duplicating the conditions in the query, which is noise. (unless it # could helps the query planner of the DB, if someone can show it to be worth it, then this can be changed.) + if reflection.klass.primary_key.is_a?(Array) + raise NeverAliasLimitDoesntWorkWithCompositePrimaryKeysError, "Sorry, it just doesn't work..." + end reflection.klass.unscoped.where(reflection.klass.primary_key.to_sym => current_scope) else @@ -396,14 +399,12 @@ def self.build_alias_scope_for_recursive_association(reflection, poly_belongs_to alias_scope = foreign_klass.base_class.unscoped alias_scope = alias_scope.from("#{table.name} #{ALIAS_TABLE.name}") - primary_key_constraints = - Array.wrap(primary_key).map do |a_primary_key| + primary_key_constraints = + Array(primary_key).map do |a_primary_key| table[a_primary_key].eq(ALIAS_TABLE[a_primary_key]) - end + end - primary_key_constraints.any? ? - alias_scope.where(primary_key_constraints.inject(&:and)) : - alias_scope + alias_scope.where(primary_key_constraints.inject(&:and)) end def self.wrapper_and_join_constraints(record_class, reflection, options = {}) @@ -435,12 +436,12 @@ def self.wrapper_and_join_constraints(record_class, reflection, options = {}) constraint_foreign_keys = Array.wrap(foreign_key) constraint_key_map = constraint_keys.zip(constraint_foreign_keys) - primary_foreign_key_constraints = + primary_foreign_key_constraints = constraint_key_map.map do |primary_and_foreign_keys| - a_primary_key, a_foreign_key = primary_and_foreign_keys + a_primary_key, a_foreign_key = primary_and_foreign_keys table[a_primary_key].eq(foreign_table[a_foreign_key]) - end + end constraints = primary_foreign_key_constraints.inject(&:and) diff --git a/lib/active_record_where_assoc/exceptions.rb b/lib/active_record_where_assoc/exceptions.rb index 7c26998..3e3a844 100644 --- a/lib/active_record_where_assoc/exceptions.rb +++ b/lib/active_record_where_assoc/exceptions.rb @@ -6,4 +6,7 @@ class MySQLDoesntSupportSubLimitError < StandardError class PolymorphicBelongsToWithoutClasses < StandardError end + + class NeverAliasLimitDoesntWorkWithCompositePrimaryKeysError < StandardError + end end diff --git a/test/support/base_test_model.rb b/test/support/base_test_model.rb index 70464be..51a9151 100644 --- a/test/support/base_test_model.rb +++ b/test/support/base_test_model.rb @@ -86,10 +86,10 @@ def self.testable_has_and_belongs_to_many(association_name, given_scope = nil, * testable_association(:has_and_belongs_to_many, association_name, given_scope, **options) end - def self.create_default!(*source_associations) + def self.create_default!(*source_associations, **attributes) condition_value = TestHelpers.condition_value_result_for(*source_associations) || 1 condition_value *= need_test_condition_value_for(:default_scope) - create!(test_condition_column => condition_value) + create!(test_condition_column => condition_value, **attributes) end def create_has_one!(association_name, attrs = {}) @@ -99,9 +99,18 @@ def create_has_one!(association_name, attrs = {}) target_model = reflection.klass - old_matched_ids = target_model.where(reflection.foreign_key => self.id).unscope(:offset, :limit).pluck(:id).to_a + old_matched_ids = target_model.where(reflection.foreign_key => [self.id]).unscope(:offset, :limit).pluck(*target_model.primary_key).to_a record = send("create_#{association_name}!", attrs) - target_model.where(id: old_matched_ids).unscope(:offset, :limit).update_all(reflection.foreign_key => self.id) + + if old_matched_ids.present? + if reflection.foreign_key.is_a?(Array) + to_update = reflection.foreign_key.zip(self.id).to_h + else + to_update = {reflection.foreign_key => self.id} + end + target_model.where(target_model.primary_key => old_matched_ids).unscope(:offset, :limit).update_all(to_update) + end + record end @@ -109,7 +118,7 @@ def create_has_one!(association_name, attrs = {}) # the matching association of each passed source_associations def create_assoc!(association_name, *source_associations) options = source_associations.extract_options! - options = options.reverse_merge(allow_no_source: false, adhoc_value: nil, skip_default: false, use_bad_type: false) + options = options.reverse_merge(allow_no_source: false, adhoc_value: nil, skip_default: false, use_bad_type: false, attributes: {}) raise "Must be a direct association, not #{association_name.inspect}" unless association_name =~ /^[mobz]p?l?\d+$/ raise "Need at least one source model or a nil instead" if !options[:allow_no_source] && source_associations.empty? @@ -123,9 +132,7 @@ def create_assoc!(association_name, *source_associations) target_model = options[:target_model] || reflection.klass - if options[:skip_attributes] - attributes = {} - else + if !options[:skip_attributes] condition_value = target_model.test_condition_value_for(:default_scope) unless options[:skip_default] if source_associations.present? @@ -133,9 +140,10 @@ def create_assoc!(association_name, *source_associations) condition_value *= TestHelpers.condition_value_result_for(*source_associations) end - attributes = { target_model.test_condition_column => condition_value, - target_model.adhoc_column_name => options[:adhoc_value], - } + attributes = options[:attributes].merge( + target_model.test_condition_column => condition_value, + target_model.adhoc_column_name => options[:adhoc_value], + ) end case association_macro diff --git a/test/support/custom_asserts.rb b/test/support/custom_asserts.rb index d77696c..99d4527 100644 --- a/test/support/custom_asserts.rb +++ b/test/support/custom_asserts.rb @@ -54,7 +54,7 @@ def assert_exists_with_matching_from(start_from, association_name, *args, &block msgs << "Expected query from where_assoc_exists to include the SQL from assoc_exists_sql" end if !exists_relation.exists? - msgs << "Expected a match but got none for S0.where_assoc_exists(#{association_name.inspect}, ...)" + msgs << "Expected a match but got none for where_assoc_exists(#{association_name.inspect}, ...)" end not_exists_relation = start_from.where_assoc_not_exists(association_name, *args, &block) @@ -62,7 +62,7 @@ def assert_exists_with_matching_from(start_from, association_name, *args, &block msgs << "Expected query from where_assoc_not_exists to include the SQL from assoc_not_exists_sql" end if not_exists_relation.exists? - msgs << "Expected no matches but got one for S0.where_assoc_not_exists(#{association_name.inspect}, ...)" + msgs << "Expected no matches but got one for where_assoc_not_exists(#{association_name.inspect}, ...)" end assert msgs.empty?, msgs.map { |s| " #{s}" }.join("\n") end @@ -74,7 +74,7 @@ def assert_exists_without_matching_from(start_from, association_name, *args, &bl msgs << "Expected query from where_assoc_exists to include the SQL from assoc_exists_sql" end if exists_relation.exists? - msgs << "Expected no matches but got one for S0.where_assoc_exists(#{association_name.inspect}, ...)" + msgs << "Expected no matches but got one for where_assoc_exists(#{association_name.inspect}, ...)" end not_exists_relation = start_from.where_assoc_not_exists(association_name, *args, &block) @@ -82,7 +82,7 @@ def assert_exists_without_matching_from(start_from, association_name, *args, &bl msgs << "Expected query from where_assoc_not_exists to include the SQL from assoc_not_exists_sql" end if !not_exists_relation.exists? - msgs << "Expected a match but got none for S0.where_assoc_not_exists(#{association_name.inspect}, ...)" + msgs << "Expected a match but got none for where_assoc_not_exists(#{association_name.inspect}, ...)" end assert msgs.empty?, msgs.map { |s| " #{s}" }.join("\n") end @@ -163,10 +163,13 @@ def manual_testing_results_from(start_from, matching_nb, operator, association_n record_sets = record_sets.map do |records| next records if records.blank? - scope = records.first.class.base_class.unscoped.where(id: records.map(&:id)) + + record_klass = records.first.class.base_class + + scope = record_klass.unscoped.where(record_klass.primary_key => records.map(&:id)) scope = scope.where(conditions) scope = ActiveRecordWhereAssoc::CoreLogic.apply_proc_scope(scope, block) if block - scope.pluck(:id) + scope.pluck(*record_klass.primary_key) end correct_result = record_sets.any? do |records| diff --git a/test/support/models.rb b/test/support/models.rb index 7949c22..e16ca0e 100644 --- a/test/support/models.rb +++ b/test/support/models.rb @@ -286,4 +286,30 @@ class UnabstractModel < AbstractModel end class NeverAbstractedModel < BaseTestModel +end + + +class Ck0 < BaseTestModel + self.primary_key = [:an_id0, :a_text0] + setup_test_default_scope + + testable_has_many :m1, class_name: "Ck1", foreign_key: [:an_id0, :a_text0] + testable_has_one :o1, -> { order("ck1s.an_id1 DESC, ck1s.a_text1 DESC") }, class_name: "Ck1", foreign_key: [:an_id0, :a_text0] + # I don't think has_and_belongs_to_many supports composite keys? + #testable_has_and_belongs_to_many :z1, class_name: "Ck1" + + testable_has_many :m2m1, class_name: "Ck2", through: :m1, source: :m2, foreign_key: [:an_id0, :a_text0] +end + +class Ck1 < BaseTestModel + self.primary_key = [:an_id1, :a_text1] + setup_test_default_scope + + testable_belongs_to :b0, class_name: "Ck0", foreign_key: [:an_id0, :a_text0] + testable_has_many :m2, class_name: "Ck2", foreign_key: [:an_id1, :a_text1] +end + +class Ck2 < BaseTestModel + self.primary_key = [:an_id2, :a_text2] + setup_test_default_scope end \ No newline at end of file diff --git a/test/support/schema.rb b/test/support/schema.rb index 0a63781..2b975de 100644 --- a/test/support/schema.rb +++ b/test/support/schema.rb @@ -162,4 +162,37 @@ t.integer :never_abstracted_models_column, limit: 8 t.integer :never_abstracted_models_adhoc_column, limit: 8 end + + create_table :ck0s, primary_key: [:an_id0, :a_text0] do |t| + t.integer :an_id0 + t.text :a_text0 + + t.integer :ck0s_column + t.integer :ck0s_adhoc_column + end + + create_table :ck1s, primary_key: [:an_id1, :a_text1] do |t| + t.integer :an_id1 + t.text :a_text1 + + t.integer :an_id0 + t.text :a_text0 + + t.integer :ck1s_column + t.integer :ck1s_adhoc_column + end + + create_join_table "ck0s", "ck1s" + + + create_table :ck2s, primary_key: [:an_id2, :a_text2] do |t| + t.integer :an_id2 + t.text :a_text2 + + t.integer :an_id1 + t.text :a_text1 + + t.integer :ck2s_column + t.integer :ck2s_adhoc_column + end end diff --git a/test/tests/wa_composite_keys_test.rb b/test/tests/wa_composite_keys_test.rb new file mode 100644 index 0000000..44420a1 --- /dev/null +++ b/test/tests/wa_composite_keys_test.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require_relative "../test_helper" + +describe "wa" do + it "belongs_to with composite_key works" do + ck1 = Ck1.create_default!(an_id1: 0, a_text1: "hi") + ck1.create_assoc!(:b0, :Ck1_b0, attributes: {an_id0: 1, a_text0: "bar"}) + ck0_spam = ck1.create_assoc!(:b0, :Ck1_b0, attributes: {an_id0: 1, a_text0: "spam"}) + ck1.save! # Save the updates ids + + assert_wa_from(Ck1, 1, :b0) + + ck1_1 = Ck1.create_default!(an_id1: 1, a_text1: "foo") + ck1_2 = Ck1.create_default!(an_id1: 12, a_text1: "foo") + + assert_equal [ck1], Ck1.where_assoc_count(1, :==, :b0).to_a.sort_by(&:an_id1) + assert_equal [ck1_1, ck1_2], Ck1.where_assoc_count(0, :==, :b0).to_a.sort_by(&:an_id1) + end + + it "has_many with composite_key works" do + ck0 = Ck0.create_default!(an_id0: 1, a_text0: "foo") + ck0.create_assoc!(:m1, :Ck0_m1, attributes: {an_id1: 1, a_text1: "bar"}) + ck0.create_assoc!(:m1, :Ck0_m1, attributes: {an_id1: 1, a_text1: "spam"}) + ck0.create_assoc!(:m1, :Ck0_m1, attributes: {an_id1: 2, a_text1: "bar"}) + + Ck1.create_default!(an_id1: 42, a_text1: "foo") + + assert_wa_from(Ck0, 3, :m1) + end + + it "has_one with composite_key works" do + ck0 = Ck0.create_default!(an_id0: 1, a_text0: "foo") + ck0.create_assoc!(:o1, :Ck0_o1, attributes: {an_id1: 1, a_text1: "bar"}) + ck0.create_assoc!(:o1, :Ck0_o1, attributes: {an_id1: 1, a_text1: "spam"}) + ck0.create_assoc!(:o1, :Ck0_o1, attributes: {an_id1: 2, a_text1: "bar"}) + + assert_wa_from(Ck0, 1, :o1) + end + + # I don't think has_and_belongs_to_many supports composite keys? + # it "has_and_belongs_to_many with composite_key works" do + # ck0 = Ck0.create_default!(an_id0: 1, a_text0: "foo") + # ck0.create_assoc!(:z1, :Ck0_z1, attributes: {an_id1: 1, a_text1: "bar"}) + # ck0.create_assoc!(:z1, :Ck0_z1, attributes: {an_id1: 1, a_text1: "spam"}) + # ck0.create_assoc!(:z1, :Ck0_z1, attributes: {an_id1: 2, a_text1: "bar"}) + + # assert_wa_from(Ck0, 3, :z1) + # end + + + it "has_many through has_many with composite_key works" do + ck0 = Ck0.create_default!(an_id0: 1, a_text0: "foo") + ck0.create_assoc!(:m1, :Ck0_m1, attributes: {an_id1: 1, a_text1: "bar"}) + ck1_2 = ck0.create_assoc!(:m1, :Ck0_m1, attributes: {an_id1: 1, a_text1: "spam"}) + ck0.create_assoc!(:m1, :Ck0_m1, attributes: {an_id1: 2, a_text1: "bar"}) + + assert_wa_from(Ck0, 0, :m2m1) + + ck1_2.create_assoc!(:m2, :Ck1_m2, :Ck0_m2m1, attributes: {an_id2: 130, a_text2: "hello"}) + assert_wa_from(Ck0, 1, :m2m1) + end + + it "raise on composite_key with never_alias_limit" do + sql = Ck0.where_assoc_exists(:o1) { from("hello") }.to_sql + assert !sql.include?("an_int0") + + assert_raises(ActiveRecordWhereAssoc::NeverAliasLimitDoesntWorkWithCompositePrimaryKeysError) { + Ck0.where_assoc_exists(:o1, nil, never_alias_limit: true) { from("hello") }.to_sql + } + end +end