* fix has_many association is broken [#35]

  Patch by culturespy. Thanks!!!


git-svn-id: http://ruby-activeldap.googlecode.com/svn/trunk@1107 fd4f1387-ac19-0410-9179-05984c98faae

Author: koutou

README

@@ -114,7 +114,9 @@ list, please point out.
 * Lennon Day-Reynolds: Bug reports.
 * Tilo: A bug report.
 * Matt Mencel: A bug report.
-* CultureSpy: A bug report.
+* CultureSpy:
+  * Bug reports.
+  * Bug fixes.
 * gwarf12: A bug report.
 * Baptiste Grenier: API improvement ideas.
 * Richard 3 Nicholas: API improvement ideas.

lib/active_ldap/association/has_many.rb

@@ -8,23 +8,23 @@ class HasMany < Collection
 
       private
       def insert_entry(entry)
-        entry[@options[:foreign_key_name]] = @owner[primary_key]
+        entry[foreign_key] = @owner[primary_key]
         entry.save
       end
 
       def find_target
-        collect_targets(:primary_key_name)
+        collect_targets(primary_key)
       end
 
       def delete_entries(entries)
-        key = @options[:foreign_key_name]
+        _foreign_key = foreign_key
         components = @owner[primary_key, true].reject do |value|
           value.nil?
         end
         filter = [:and,
-                  [:and, {key => components}],
+                  [:and, {_foreign_key => components}],
                   [:or, {foreign_class.dn_attribute => entries.collect(&:id)}]]
-        foreign_class.update_all({key => []}, filter)
+        foreign_class.update_all({_foreign_key => []}, filter)
       end
     end
   end

lib/active_ldap/association/has_many_utils.rb

@@ -3,15 +3,14 @@ module Association
     module HasManyUtils
       private
       def collect_targets(requested_target_key, need_requested_targets=false)
-        foreign_base_key = primary_key
-        return [] if foreign_base_key.nil?
-
-        requested_targets = @owner[@options[requested_target_key], true]
+        _foreign_key = foreign_key
+        return [] if _foreign_key.nil?
 
+        requested_targets = @owner[requested_target_key, true]
         requested_targets = requested_targets.reject(&:nil?)
         if requested_targets.empty?
           targets = []
-        elsif foreign_base_key == "dn"
+        elsif _foreign_key == "dn"
           requested_targets = requested_targets.collect do |target|
             if target.is_a?(DN)
               target
@@ -28,7 +27,7 @@ def collect_targets(requested_target_key, need_requested_targets=false)
           end
         else
           components = requested_targets.collect do |value|
-            [foreign_base_key, value]
+            [_foreign_key, value]
           end
           options = find_options(:filter => [:or, *components])
           targets = foreign_class.find(:all, options)

lib/active_ldap/association/has_many_wrap.rb

@@ -9,11 +9,11 @@ class HasManyWrap < Collection
       private
       def insert_entry(entry)
         old_value = @owner[@options[:wrap], true]
-        _primary_key = primary_key
-        if _primary_key == "dn"
+        _foreign_key = foreign_key
+        if _foreign_key == "dn"
           old_value = dn_values_to_string_values(old_value)
         end
-        current_value = entry[_primary_key, true]
+        current_value = entry[_foreign_key, true]
         current_value = dn_values_to_string_values(current_value)
         new_value = (old_value + current_value).uniq.sort
         if old_value != new_value
@@ -24,11 +24,11 @@ def insert_entry(entry)
 
       def delete_entries(entries)
         old_value = @owner[@options[:wrap], true]
-        _primary_key = primary_key
-        if _primary_key == "dn"
+        _foreign_key = foreign_key
+        if _foreign_key == "dn"
           old_value = dn_values_to_string_values(old_value)
         end
-        current_value = entries.collect {|entry| entry[_primary_key]}
+        current_value = entries.collect {|entry| entry[_foreign_key]}
         current_value = dn_values_to_string_values(current_value)
         new_value = old_value - current_value
         new_value = new_value.uniq.sort
@@ -39,20 +39,24 @@ def delete_entries(entries)
       end
 
       def find_target
-        targets, requested_targets = collect_targets(:wrap, true)
+        targets, requested_targets = collect_targets(@options[:wrap], true)
         return [] if targets.nil?
 
         found_targets = {}
-        foreign_base_key = primary_key
+        _foreign_key = foreign_key
         targets.each do |target|
-          found_targets[target[foreign_base_key]] ||= target
+          found_targets[target[_foreign_key]] ||= target
         end
 
         klass = foreign_class
         requested_targets.collect do |name|
           found_targets[name] || klass.new(name)
         end
       end
+
+      def foreign_key
+        @options[:primary_key_name] || foreign_class.dn_attribute
+      end
     end
   end
 end

lib/active_ldap/association/proxy.rb

@@ -70,7 +70,11 @@ def have_foreign_key?
       end
 
       def primary_key
-        @options[:primary_key_name] || foreign_class.dn_attribute
+        @options[:primary_key_name] || @owner.dn_attribute
+      end
+
+      def foreign_key
+        @options[:foreign_key_name] || foreign_class.dn_attribute
       end
 
       def load_target

lib/active_ldap/associations.rb

@@ -125,13 +125,13 @@ def has_many(association_id, options = {})
           :extend => options[:extend],
         }
         if opts[:wrap]
-          opts[:foreign_key_name] ||= "#{association_id}_id"
           association_class = Association::HasManyWrap
         else
           association_class = Association::HasMany
           primary_key_name = opts[:primary_key_name]
           foreign_key_name = opts[:foreign_key_name]
           if primary_key_name != foreign_key_name and
+              primary_key_name != "dn" and
               !new.have_attribute?(primary_key_name)
             message = _(":primary_key and :foreign_key has_many options are " \
                         "inverted their mean since 1.1.0. Please invert them.")

test/al-test-utils.rb

@@ -289,6 +289,7 @@ def make_temporary_user(config={})
         uid_number = config[:uid_number] || default_uid
         gid_number = config[:gid_number] || default_gid
         home_directory = config[:home_directory] || "/nonexistent"
+        see_also = config[:see_also]
         _wrap_assertion do
           assert(!@user_class.exists?(uid))
           assert_raise(ActiveLdap::EntryNotFound) do
@@ -302,6 +303,7 @@ def make_temporary_user(config={})
           user.gid_number = gid_number
           user.home_directory = home_directory
           user.user_password = ActiveLdap::UserPassword.ssha(password)
+          user.see_also = see_also
           unless config[:simple]
             user.add_class('shadowAccount', 'inetOrgPerson',
                            'organizationalPerson')

test/test_associations.rb

@@ -4,6 +4,31 @@ class TestAssociations < Test::Unit::TestCase
   include AlTestUtils
 
   priority :must
+  def test_has_many_of_self
+    @user_class.has_many(:references,
+                         :class_name => "User",
+                         :primary_key => "dn",
+                         :foreign_key => "seeAlso")
+    @user_class.set_associated_class(:references, @user_class)
+    make_temporary_user do |user1, password1|
+      make_temporary_user(:see_also => user1.dn.to_s) do |user2, password2|
+        make_temporary_user(:see_also => user2.dn.to_s) do |user3, password3|
+          make_temporary_user(:see_also => user2.dn.to_s) do |user4, password4|
+            make_temporary_user(:see_also => user1.dn.to_s) do |user5, password5|
+              user1_references = user1.references.collect {|r| r.dn.to_s}
+              user2_references = user2.references.collect {|r| r.dn.to_s}
+              user1_expected_references = [user2, user5].collect {|r| r.dn.to_s}
+              user2_expected_references = [user3, user4].collect {|r| r.dn.to_s}
+              assert_equal(user1_expected_references, user1_references)
+              assert_equal(user2_expected_references, user2_references)
+            end
+          end
+        end
+      end
+    end
+  end
+
+  priority :normal
   def test_belongs_to_add_with_string
     make_temporary_user do |user,|
       make_temporary_group do |group1|
@@ -25,7 +50,6 @@ def test_belongs_to_add_with_string
     end
   end
 
-  priority :normal
   def test_has_many_delete_required_attribute
     make_temporary_group do |group|
       make_temporary_user do |user,|