Fix "Multiple Parents Found" issue when moving a relationship.

Originally, `.parent=` was written in terms of `add_children`.  However,
`add_children` supports duplicating nodes in the tree.  This is reasonable
because you may have a tree where multiple entries make sense, and for
those things that have multiple entries, you wouldn't call `.parent`, you
would probably call `.parents` and it is expected that calling `.parent`
would blow up with the "Multiple Parents Found" error.  However, the
expectation of calling `.parent=` is that it will become the new, sole,
parent, and so implementing it via add_children is wrong because it
creates an oddity in that you can call `.parent=` and then not be able to
call `.parent` afterwards.

https://bugzilla.redhat.com/show_bug.cgi?id=1379464
https://bugzilla.redhat.com/show_bug.cgi?id=1391850
https://bugzilla.redhat.com/show_bug.cgi?id=1406431

app/models/account.rb

@@ -104,7 +104,7 @@ def users
   end
 
   def add_user(owns)
-    with_valid_account_type('group') { set_child(owns) }
+    with_valid_account_type('group') { add_child(owns) }
   end
 
   def remove_user(owns)
@@ -120,7 +120,7 @@ def groups
   end
 
   def add_group(owner)
-    with_valid_account_type('user') { set_parent(owner) }
+    with_valid_account_type('user') { add_parent(owner) }
   end
 
   def remove_group(owner)

app/models/mixins/relationship_mixin.rb

@@ -522,7 +522,7 @@ def child_types(*args)
     Relationship.resource_types(child_rels(*args))
   end
 
-  def parent=(parent)
+  def add_parent(parent)
     parent.with_relationship_type(relationship_type) { parent.add_child(self) }
   end
 
@@ -560,25 +560,28 @@ def add_children(*child_objs)
   end
   alias_method :add_child, :add_children
 
-  #
-  # Backward compatibility methods
-  #
-
-  alias_method :set_parent, :parent=
-  alias_method :set_child,  :add_children
-
-  def replace_parent(parent)
+  def parent=(parent)
     if parent.nil?
       remove_all_parents
     else
       parent.with_relationship_type(relationship_type) do
         parent_rel = parent.init_relationship
         init_relationship(parent_rel)  # TODO: Deal with any multi-instances
+
+        parent.clear_relationships_cache
       end
     end
 
     clear_relationships_cache
   end
+  alias_method :replace_parent, :parent=
+
+  #
+  # Backward compatibility methods
+  #
+
+  alias_method :set_parent, :parent=
+  alias_method :set_child,  :add_children
 
   def replace_children(*child_objs)
     child_objs = child_objs.flatten

lib/extensions/ar_miq_set.rb

@@ -33,8 +33,7 @@ def sets
   end # module SingletonMethods
 
   def make_memberof(set)
-    raise "object of type #{self.class} may not be a member of a set of type #{set.class}" unless self.kind_of?(set.class.model_class)
-    with_relationship_type("membership") { self.parent = set }
+    set.add_member(self)
   end
 end # module ActsAsMiqSetMember
 

spec/models/mixins/relationship_mixin_spec.rb

@@ -196,6 +196,62 @@
       end
     end
 
+    describe "#add_parent" do
+      let(:folder1) { FactoryGirl.create(:ems_folder) }
+      let(:folder2) { FactoryGirl.create(:ems_folder) }
+      let(:vm)      { FactoryGirl.create(:vm) }
+
+      it "puts an object under another object" do
+        vm.with_relationship_type(test_rel_type) do
+          vm.add_parent(folder1)
+
+          expect(vm.parents).to eq([folder1])
+          expect(vm.parent).to eq(folder1)
+        end
+
+        expect(folder1.with_relationship_type(test_rel_type) { folder1.children }).to eq([vm])
+      end
+
+      it "allows an object to be placed under multiple parents" do
+        vm.with_relationship_type(test_rel_type) do
+          vm.add_parent(folder1)
+          vm.add_parent(folder2)
+
+          expect(vm.parents).to match_array([folder1, folder2])
+          expect { vm.parent }.to raise_error(RuntimeError, "Multiple parents found.")
+        end
+
+        expect(folder1.with_relationship_type(test_rel_type) { folder1.children }).to eq([vm])
+        expect(folder2.with_relationship_type(test_rel_type) { folder2.children }).to eq([vm])
+      end
+    end
+
+    describe "#parent=" do
+      let(:folder) { FactoryGirl.create(:ems_folder) }
+      let(:vm)     { FactoryGirl.create(:vm) }
+
+      it "puts an object under another object" do
+        vm.with_relationship_type(test_rel_type) do
+          vm.parent = folder
+          expect(vm.parent).to eq(folder)
+        end
+
+        expect(folder.with_relationship_type(test_rel_type) { folder.children }).to eq([vm])
+      end
+
+      it "moves an object that already has a parent under an another object" do
+        vm.with_relationship_type(test_rel_type) { vm.parent = FactoryGirl.create(:ems_folder) }
+        vm.reload
+
+        vm.with_relationship_type(test_rel_type) do
+          vm.parent = folder
+          expect(vm.parent).to eq(folder)
+        end
+
+        expect(folder.with_relationship_type(test_rel_type) { folder.children }).to eq([vm])
+      end
+    end
+
     it "#replace_children" do
       new_vms = (0...2).collect { FactoryGirl.create(:vm_vmware) }
       vms[0].with_relationship_type(test_rel_type) do |v|