Refactoring the Rails integration

This moves all code from the Parser class into
the ActiveRecord classes, and gets rid of
'ar_hash_merge'.

Signed-off-by: Luke Kanies <luke@madstop.com>

10 files changed, 318 insertions, 274 deletions

diff --git a/lib/puppet/indirector/catalog/active_record.rb b/lib/puppet/indirector/catalog/active_record.rb
index f57427e92..a76606535 100644
--- a/lib/puppet/indirector/catalog/active_record.rb
+++ b/lib/puppet/indirector/catalog/active_record.rb
@@ -26,7 +26,7 @@ class Puppet::Node::Catalog::ActiveRecord < Puppet::Indirector::ActiveRecord
 
         host = ar_model.find_by_name(catalog.name) || ar_model.create(:name => catalog.name)
 
-        host.setresources(catalog.vertices)
+        host.merge_resources(catalog.vertices)
         host.last_compile = Time.now
 
         host.save
diff --git a/lib/puppet/indirector/facts/active_record.rb b/lib/puppet/indirector/facts/active_record.rb
index 5fb2596d7..f16d81220 100644
--- a/lib/puppet/indirector/facts/active_record.rb
+++ b/lib/puppet/indirector/facts/active_record.rb
@@ -28,7 +28,7 @@ class Puppet::Node::Facts::ActiveRecord < Puppet::Indirector::ActiveRecord
 
         host = ar_model.find_by_name(facts.name) || ar_model.create(:name => facts.name)
         
-        host.setfacts(facts.values)
+        host.merge_facts(facts.values)
 
         host.save
     end
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 8d29ea346..23df1c624 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -58,6 +58,12 @@ class Puppet::Parser::Resource
         @ref.builtin = bool
     end
 
+    def eachparam
+        @params.each do |name, param|
+            yield param
+        end
+    end
+
     # Retrieve the associated definition and evaluate it.
     def evaluate
         if klass = @ref.definedtype
@@ -167,49 +173,6 @@ class Puppet::Parser::Resource
         end
     end
 
-    # Modify this resource in the Rails database.  Poor design, yo.
-    def modify_rails(db_resource)
-        args = rails_args
-        args.each do |param, value|
-            db_resource[param] = value unless db_resource[param] == value
-        end
-
-        # Handle file specially
-        if (self.file and  
-            (!db_resource.file or db_resource.file != self.file))
-            db_resource.file = self.file
-        end
-        
-        updated_params = @params.reject { |name, param| param.value == :undef }.inject({}) do |hash, ary|
-            hash[ary[0].to_s] = ary[1]
-            hash
-        end
-        
-        db_resource.ar_hash_merge(db_resource.get_params_hash(), updated_params,
-                                  :create => Proc.new { |name, parameter|
-                                      parameter.to_rails(db_resource)
-                                  }, :delete => Proc.new { |values|
-                                      values.each { |value| Puppet::Rails::ParamValue.delete(value['id']) }
-                                  }, :modify => Proc.new { |db, mem|
-                                      mem.modify_rails_values(db)
-                                  })
-        
-        updated_tags = tags.inject({}) { |hash, tag| 
-            hash[tag] = tag
-            hash
-        }
-
-        db_resource.ar_hash_merge(db_resource.get_tag_hash(), 
-                                  updated_tags,
-                                  :create => Proc.new { |name, tag|
-                                      db_resource.add_resource_tag(name)
-                                  }, :delete => Proc.new { |tag|
-                                      Puppet::Rails::ResourceTag.delete(tag['id'])
-                                  }, :modify => Proc.new { |db, mem|
-                                      # nothing here
-                                  })
-    end
-
     # Return the resource name, or the title if no name
     # was specified.
     def name
@@ -262,26 +225,6 @@ class Puppet::Parser::Resource
         end
     end
 
-    # Turn our parser resource into a Rails resource.  
-    def to_rails(host)
-        args = rails_args
-
-        db_resource = host.resources.build(args)
-
-        # Handle file specially
-        db_resource.file = self.file
-
-        db_resource.save
-
-        @params.each { |name, param|
-            next if param.value == :undef
-            param.to_rails(db_resource)
-        }
-        
-        tags.each { |tag| db_resource.add_resource_tag(tag) }
-
-        return db_resource
-    end
 
     # Create a Puppet::Resource instance from this parser resource.
     # We plan, at some point, on not needing to do this conversion, but
@@ -426,17 +369,6 @@ class Puppet::Parser::Resource
         end
     end
 
-    def rails_args
-        return [:type, :title, :line, :exported].inject({}) do |hash, param|
-            # 'type' isn't a valid column name, so we have to use another name.
-            to = (param == :type) ? :restype : param
-            if value = self.send(param)
-                hash[to] = value
-            end
-            hash
-        end
-    end
-
     # Make sure the resource's parameters are all valid for the type.
     def validate
         @params.each do |name, param|
diff --git a/lib/puppet/parser/resource/param.rb b/lib/puppet/parser/resource/param.rb
index 7ce58f4c4..6e22d3e17 100644
--- a/lib/puppet/parser/resource/param.rb
+++ b/lib/puppet/parser/resource/param.rb
@@ -22,80 +22,8 @@ class Puppet::Parser::Resource::Param
     def line_to_i
         return line ? Integer(line) : nil
     end
-
-    # Make sure an array (or possibly not an array) of values is correctly
-    # set up for Rails.  The main thing is that Resource::Reference objects
-    # should stay objects, so they just get serialized.
-    def munge_for_rails(values)
-        values = value.is_a?(Array) ? value : [value]
-        values.map do |v|
-            if v.is_a?(Puppet::Parser::Resource::Reference)
-                v
-            else
-                v.to_s
-            end
-        end
-    end
-    
-    # Store a new parameter in a Rails db.
-    def to_rails(db_resource)
-        values = munge_for_rails(value)
-
-        param_name = Puppet::Rails::ParamName.find_or_create_by_name(self.name.to_s)
-        line_number = line_to_i()
-        return values.collect do |v|
-            db_resource.param_values.create(:value => v,
-                                           :line => line_number,
-                                           :param_name => param_name)
-        end
-    end
-
-    def modify_rails_values(db_values)
-        #dev_warn if db_values.nil? || db_values.empty?
-
-        values_to_remove(db_values).each { |remove_me|
-            Puppet::Rails::ParamValue.delete(remove_me['id'])
-        }
-        line_number = line_to_i()
-        db_param_name = db_values[0]['param_name_id']
-        values_to_add(db_values).each { |add_me|
-            Puppet::Rails::ParamValue.create(:value => add_me,
-                                           :line => line_number,
-                                           :param_name_id => db_param_name,
-                                           :resource_id => db_values[0]['resource_id'] )
-        }
-    end
     
     def to_s
         "%s => %s" % [self.name, self.value]
     end
-
-    def compare(v,db_value)
-      if (v.is_a?(Puppet::Parser::Resource::Reference))
-        return v.to_s == db_value.to_s
-      else
-        return v == db_value
-      end
-    end
-    
-    def values_to_remove(db_values)
-        values = munge_for_rails(value)
-        line_number = line_to_i()
-        db_values.collect do |db|
-            db unless (db['line'] == line_number && 
-                       values.find { |v| 
-                         compare(v,db['value'])
-                       } )
-        end.compact
-    end
-
-    def values_to_add(db_values)
-        values = munge_for_rails(value)
-        line_number = line_to_i()
-        values.collect do |v|
-            v unless db_values.find { |db| (compare(v,db['value']) && 
-                                         line_number == db['line']) }
-        end.compact
-    end
 end
-
diff --git a/lib/puppet/rails/host.rb b/lib/puppet/rails/host.rb
index 23a22553d..b5831671b 100644
--- a/lib/puppet/rails/host.rb
+++ b/lib/puppet/rails/host.rb
@@ -56,16 +56,19 @@ class Puppet::Rails::Host < ActiveRecord::Base
             end
 
             # Store the facts into the database.
-            host.setfacts node.parameters
+            host.merge_facts(node.parameters)
 
             seconds = Benchmark.realtime {
-                host.setresources(resources)
+                host.merge_resources(resources)
             }
             Puppet.debug("Handled resources in %0.2f seconds" % seconds)
 
             host.last_compile = Time.now
 
-            host.save
+            seconds = Benchmark.realtime {
+                host.save
+            }
+            Puppet.debug("Saved host in %0.2f seconds" % seconds)
         end
 
         return host
@@ -94,52 +97,60 @@ class Puppet::Rails::Host < ActiveRecord::Base
     end
     
 
-    def setfacts(facts)
-        facts = facts.dup
-        
-        ar_hash_merge(get_facts_hash(), facts, 
-                      :create => Proc.new { |name, values|
-                          fact_name = Puppet::Rails::FactName.find_or_create_by_name(name)
-                          values = [values] unless values.is_a?(Array)
-                          values.each do |value|
-                              fact_values.build(:value => value,
-                                                :fact_name => fact_name)
-                          end
-                      }, :delete => Proc.new { |values|
-                          values.each { |value| self.fact_values.delete(value) }
-                      }, :modify => Proc.new { |db, mem|
-                          mem = [mem].flatten
-                          fact_name = db[0].fact_name
-                          db_values = db.collect { |fact_value| fact_value.value }
-                          (db_values - (db_values & mem)).each do |value|
-                              db.find_all { |fact_value| 
-                                  fact_value.value == value 
-                              }.each { |fact_value|
-                                  fact_values.delete(fact_value)
-                              }
-                          end
-                          (mem - (db_values & mem)).each do |value|
-                              fact_values.build(:value => value, 
-                                                :fact_name => fact_name)
-                          end
-                      })
+    # This is *very* similar to the merge_parameters method
+    # of Puppet::Rails::Resource.
+    def merge_facts(facts)
+        db_facts = {}
+
+        deletions = []
+        self.fact_values.find(:all, :include => :fact_name).each do |value|
+            deletions << value['id'] and next unless facts.include?(value['name'])
+            # Now store them for later testing.
+            db_facts[value['name']] ||= []
+            db_facts[value['name']] << value
+        end
+
+        # Now get rid of any parameters whose value list is different.
+        # This might be extra work in cases where an array has added or lost
+        # a single value, but in the most common case (a single value has changed)
+        # this makes sense.
+        db_facts.each do |name, value_hashes|
+            values = value_hashes.collect { |v| v['value'] }
+
+            unless values == facts[name]
+                value_hashes.each { |v| deletions << v['id'] }
+            end
+        end
+
+        # Perform our deletions.
+        Puppet::Rails::FactValue.delete(deletions) unless deletions.empty?
+
+        # Lastly, add any new parameters.
+        facts.each do |name, value|
+            next if db_facts.include?(name)
+            values = value.is_a?(Array) ? value : [value]
+
+            values.each do |v|
+                fact_values.build(:value => v, :fact_name => Puppet::Rails::FactName.find_or_create_by_name(name))
+            end
+        end
     end
 
     # Set our resources.
-    def setresources(list)
-        resource_by_id = nil
+    def merge_resources(list)
+        resources_by_id = nil
         seconds = Benchmark.realtime {
-            resource_by_id = find_resources()
+            resources_by_id = find_resources()
         }
         Puppet.debug("Searched for resources in %0.2f seconds" % seconds)
 
         seconds = Benchmark.realtime {
-            find_resources_parameters_tags(resource_by_id)
+            find_resources_parameters_tags(resources_by_id)
         } if id
         Puppet.debug("Searched for resource params and tags in %0.2f seconds" % seconds)
 
         seconds = Benchmark.realtime {
-            compare_to_catalog(resource_by_id, list)
+            compare_to_catalog(resources_by_id, list)
         }
         Puppet.debug("Resource comparison took %0.2f seconds" % seconds)
     end
@@ -161,37 +172,115 @@ class Puppet::Rails::Host < ActiveRecord::Base
         find_resources_tags(resources)
     end
 
-    # it seems that it can happen (see bug #2010) some resources are duplicated in the
-    # database (ie logically corrupted database), in which case we remove the extraneous
-    # entries.
     def compare_to_catalog(existing, list)
-        extra_db_resources = []
-        resources = existing.inject({}) do |hash, res|
-            resource = res[1]
-            if hash.include?(resource.ref)
-                extra_db_resources << hash[resource.ref]
-            end
+        compiled = list.inject({}) do |hash, resource|
             hash[resource.ref] = resource
             hash
         end
 
-        compiled = list.inject({}) do |hash, resource|
-            hash[resource.ref] = resource
-            hash
+        resources = nil
+        seconds = Benchmark.realtime {
+            resources = remove_unneeded_resources(compiled, existing)
+        }
+        Puppet.debug("Resource removal took %0.2f seconds" % seconds)
+
+        # Now for all resources in the catalog but not in the db, we're pretty easy.
+        additions = nil
+        seconds = Benchmark.realtime {
+            additions = perform_resource_merger(compiled, resources)
+        }
+        Puppet.debug("Resource merger took %0.2f seconds" % seconds)
+
+        seconds = Benchmark.realtime {
+            additions.each do |resource|
+                build_rails_resource_from_parser_resource(resource)
+            end
+        }
+        Puppet.debug("Resource addition took %0.2f seconds" % seconds)
+    end
+
+    def add_new_resources(additions)
+        additions.each do |resource|
+            Puppet::Rails::Resource.from_parser_resource(self, resource)
+        end
+    end
+
+    # Turn a parser resource into a Rails resource.  
+    def build_rails_resource_from_parser_resource(resource)
+        args = Puppet::Rails::Resource.rails_resource_initial_args(resource)
+
+        db_resource = self.resources.build(args)
+
+        # Our file= method does the name to id conversion.
+        db_resource.file = resource.file
+
+        resource.eachparam do |param|
+            Puppet::Rails::ParamValue.from_parser_param(param).each do |value_hash|
+                db_resource.param_values.build(value_hash)
+            end
         end
 
-        ar_hash_merge(resources, compiled,
-                      :create => Proc.new { |ref, resource|
-                          resource.to_rails(self)
-                      }, :delete => Proc.new { |resource|
-                          self.resources.delete(resource)
-                      }, :modify => Proc.new { |db, mem|
-                          mem.modify_rails(db)
-                      })
+        resource.tags.each { |tag| db_resource.add_resource_tag(tag) }
+
+        return db_resource
+    end
+
 
-        # fix-up extraneous resources
-        extra_db_resources.each do |resource|
-            self.resources.delete(resource)
+    def perform_resource_merger(compiled, resources)
+        return compiled.values if resources.empty?
+
+        # Now for all resources in the catalog but not in the db, we're pretty easy.
+        times = Hash.new(0)
+        additions = []
+        compiled.each do |ref, resource|
+            if db_resource = resources[ref]
+                db_resource.merge_parser_resource(resource).each do |name, time|
+                    times[name] += time
+                end
+            else
+                additions << resource
+            end
+        end
+        times.each do |name, time|
+            Puppet.debug("Resource merger(%s) took %0.2f seconds" % [name, time])
+        end
+
+        return additions
+    end
+
+    def remove_unneeded_resources(compiled, existing)
+        deletions = []
+        resources = {}
+        existing.each do |id, resource|
+            # it seems that it can happen (see bug #2010) some resources are duplicated in the
+            # database (ie logically corrupted database), in which case we remove the extraneous
+            # entries.
+            if resources.include?(resource.ref)
+                deletions << id
+                next
+            end
+
+            # If the resource is in the db but not in the catalog, mark it
+            # for removal.
+            unless compiled.include?(resource.ref)
+                deletions << id
+                next
+            end
+
+            resources[resource.ref] = resource
+        end
+
+        # We need to use 'destroy' here, not 'delete', so that all
+        # dependent objects get removed, too.
+        Puppet::Rails::Resource.destroy(*deletions) unless deletions.empty?
+
+        # Now for all resources in the catalog but not in the db, we're pretty easy.
+        compiled.each do |ref, resource|
+            if db_resource = resources[ref]
+                db_resource.merge_parser_resource(resource)
+            else
+                self.resources << Puppet::Rails::Resource.from_parser_resource(resource)
+            end
         end
     end
 
diff --git a/lib/puppet/rails/param_value.rb b/lib/puppet/rails/param_value.rb
index 483e4d2e8..a5dbbaed4 100644
--- a/lib/puppet/rails/param_value.rb
+++ b/lib/puppet/rails/param_value.rb
@@ -7,6 +7,32 @@ class Puppet::Rails::ParamValue < ActiveRecord::Base
     belongs_to :param_name
     belongs_to :resource
 
+    # Store a new parameter in a Rails db.
+    def self.from_parser_param(param)
+        values = munge_parser_values(param.value)
+
+        param_name = Puppet::Rails::ParamName.find_or_create_by_name(param.name.to_s)
+        line_number = param.line_to_i()
+        return values.collect do |v|
+            {:value => v, :line => line_number, :param_name => param_name}
+        end
+    end
+
+    # Make sure an array (or possibly not an array) of values is correctly
+    # set up for Rails.  The main thing is that Resource::Reference objects
+    # should stay objects, so they just get serialized.
+    def self.munge_parser_values(value)
+        values = value.is_a?(Array) ? value : [value]
+        values.map do |v|
+            if v.is_a?(Puppet::Parser::Resource::Reference)
+                v
+            else
+                v.to_s
+            end
+        end
+    end
+
+
     def value
         unserialize_value(self[:value])
     end
@@ -18,7 +44,7 @@ class Puppet::Rails::ParamValue < ActiveRecord::Base
     end
 
     def to_label
-      "#{self.param_name.name}"
+        "#{self.param_name.name}"
     end
 
     # returns an array of hash containing all the parameters of a given resource
@@ -42,6 +68,8 @@ class Puppet::Rails::ParamValue < ActiveRecord::Base
         end
         params
     end
-
+    
+    def to_s
+        "%s => %s" % [self.name, self.value]
+    end
 end
-
diff --git a/lib/puppet/rails/resource.rb b/lib/puppet/rails/resource.rb
index c3d287af2..27bf96783 100644
--- a/lib/puppet/rails/resource.rb
+++ b/lib/puppet/rails/resource.rb
@@ -17,9 +17,41 @@ class Puppet::Rails::Resource < ActiveRecord::Base
     belongs_to :source_file
     belongs_to :host
 
+    # Turn a parser resource into a Rails resource.  
+    def self.from_parser_resource(resource)
+        args = rails_resource_initial_args(resource)
+
+        db_resource = create(args)
+
+        # Our file= method does the name to id conversion.
+        db_resource.file = resource.file
+
+        resource.eachparam do |param|
+            Puppet::Rails::ParamValue.from_parser_param(param).each do |value_hash|
+                db_resource.param_values.build(value_hash)
+            end
+        end
+
+        resource.tags.each { |tag| db_resource.add_resource_tag(tag) }
+
+        return db_resource
+    end
+
+    # Determine the basic details on the resource.
+    def self.rails_resource_initial_args(resource)
+        return [:type, :title, :line, :exported].inject({}) do |hash, param|
+            # 'type' isn't a valid column name, so we have to use another name.
+            to = (param == :type) ? :restype : param
+            if value = resource.send(param)
+                hash[to] = value
+            end
+            hash
+        end
+    end
+
     def add_resource_tag(tag)
-        pt = Puppet::Rails::PuppetTag.find_or_create_by_name(tag, :include => :puppet_tag)
-        resource_tags.create(:puppet_tag => pt)
+        pt = Puppet::Rails::PuppetTag.find_or_create_by_name(tag)
+        resource_tags.build(:puppet_tag => pt)
     end
 
     def file
@@ -56,30 +88,95 @@ class Puppet::Rails::Resource < ActiveRecord::Base
         @tags_hash = hash
     end
 
-    # returns a hash of param_names.name => [param_values]
-    def get_params_hash(values = nil)
-        values ||= @params_hash || Puppet::Rails::ParamValue.find_all_params_from_resource(self)
-        if values.size == 0
-            return {}
+    def [](param)
+        return super || parameter(param)
+    end
+
+    # Make sure this resource is equivalent to the provided Parser resource.
+    def merge_parser_resource(resource)
+        merge_attributes(resource)
+        merge_parameters(resource)
+        merge_tags(resource)
+    end
+
+    def merge_attributes(resource)
+        args = self.class.rails_resource_initial_args(resource)
+        args.each do |param, value|
+            self[param] = value unless resource[param] == value
         end
-        values.inject({}) do |hash, value|
-            hash[value['name']] ||= []
-            hash[value['name']] << value
-            hash
+
+        # Handle file specially
+        if (resource.file and  (!resource.file or self.file != resource.file))
+            self.file = resource.file
         end
     end
 
-    def get_tag_hash(tags = nil)
-        tags ||= @tags_hash || Puppet::Rails::ResourceTag.find_all_tags_from_resource(self)
-        return tags.inject({}) do |hash, tag|
-            # We have to store the tag object, not just the tag name.
-            hash[tag['name']] = tag
-            hash
+    def merge_parameters(resource)
+        catalog_params = {}
+        resource.eachparam do |param|
+            catalog_params[param.name.to_s] = param
+        end
+
+        db_params = {}
+
+        deletions = []
+        #Puppet::Rails::ParamValue.find_all_params_from_resource(self).each do |value|
+        @params_hash.each do |value|
+            # First remove any parameters our catalog resource doesn't have at all.
+            deletions << value['id'] and next unless catalog_params.include?(value['name'])
+
+            # Now store them for later testing.
+            db_params[value['name']] ||= []
+            db_params[value['name']] << value
+        end
+
+        # Now get rid of any parameters whose value list is different.
+        # This might be extra work in cases where an array has added or lost
+        # a single value, but in the most common case (a single value has changed)
+        # this makes sense.
+        db_params.each do |name, value_hashes|
+            values = value_hashes.collect { |v| v['value'] }
+
+            unless value_compare(catalog_params[name].value, values)
+                value_hashes.each { |v| deletions << v['id'] }
+            end
+        end
+
+        # Perform our deletions.
+        Puppet::Rails::ParamValue.delete(deletions) unless deletions.empty?
+
+        # Lastly, add any new parameters.
+        catalog_params.each do |name, param|
+            next if db_params.include?(name)
+            values = param.value.is_a?(Array) ? param.value : [param.value]
+
+            values.each do |v|
+                param_values.build(:value => serialize_value(v), :line => param.line, :param_name => Puppet::Rails::ParamName.find_or_create_by_name(name))
+            end
         end
     end
+        
+    # Make sure the tag list is correct.
+    def merge_tags(resource)
+        in_db = []
+        deletions = []
+        resource_tags = resource.tags
+        #Puppet::Rails::ResourceTag.find_all_tags_from_resource(self).each do |tag|
+        @tags_hash.each do |tag|
+            deletions << tag['id'] and next unless resource_tags.include?(tag['name'])
+            in_db << tag['name']
+        end
+        Puppet::Rails::ResourceTag.delete(deletions) unless deletions.empty?
 
-    def [](param)
-        return super || parameter(param)
+        (resource_tags - in_db).each do |tag|
+            add_resource_tag(tag)
+        end
+    end
+
+    def value_compare(v,db_value)
+        v = [v] unless v.is_a?(Array)
+
+        v == db_value
     end
 
     def name
@@ -88,7 +185,7 @@ class Puppet::Rails::Resource < ActiveRecord::Base
 
     def parameter(param)
         if pn = param_names.find_by_name(param)
-            if pv = param_values.find(:first, :conditions => [ 'param_name_id = ?', pn]                                                            )
+            if pv = param_values.find(:first, :conditions => [ 'param_name_id = ?', pn])
                 return pv.value
             else
                 return nil
@@ -112,6 +209,15 @@ class Puppet::Rails::Resource < ActiveRecord::Base
         "%s[%s]" % [self[:restype].split("::").collect { |s| s.capitalize }.join("::"), self.title.to_s]
     end
 
+    # Returns a hash of parameter names and values, no ActiveRecord instances.
+    def to_hash
+        Puppet::Rails::ParamValue.find_all_params_from_resource(self).inject({}) do |hash, value|
+            hash[value['name']] ||= []
+            hash[value['name']] << value.value
+            hash
+        end
+    end
+
     # Convert our object to a resource.  Do not retain whether the object
     # is exported, though, since that would cause it to get stripped
     # from the configuration.
diff --git a/lib/puppet/util/rails/collection_merger.rb b/lib/puppet/util/rails/collection_merger.rb
index 4a941b881..3a28bb304 100644
--- a/lib/puppet/util/rails/collection_merger.rb
+++ b/lib/puppet/util/rails/collection_merger.rb
@@ -1,43 +1,4 @@
 module Puppet::Util::CollectionMerger
-    # Merge new values with the old list.  This is only necessary
-    # because deletion seems to mess things up on unsaved objects.
-    def collection_merge(collection, args)
-        remove = []
-        list = args[:existing] || send(collection)
-        hash = args[:updates]
-        list.each do |object|
-            name = object.name
-            if existing = hash[name]
-                hash.delete(name)
-                if existing.respond_to?(:to_rails)
-                    existing.to_rails(self, object)
-                elsif args.include?(:modify)
-                    args[:modify].call(object, name, existing)
-                else
-                    raise ArgumentError, "Must pass :modify or the new objects must respond to :to_rails"
-                end
-            else
-                remove << object
-            end
-        end
-
-        # Make a new rails object for the rest of them
-        hash.each do |name, object|
-            if object.respond_to?(:to_rails)
-                object.to_rails(self)
-            elsif args.include?(:create)
-                args[:create].call(name, object)
-            else
-                raise ArgumentError, "Must pass :create or the new objects must respond to :to_rails"
-            end
-        end
-
-        # Now remove anything necessary.
-        remove.each do |object|
-            send(collection).delete(object)
-        end
-    end
-    
     def ar_hash_merge(db_hash, mem_hash, args)
         (db_hash.keys | mem_hash.keys).each do |key|
             if (db_hash[key] && mem_hash[key])
diff --git a/spec/unit/indirector/catalog/active_record.rb b/spec/unit/indirector/catalog/active_record.rb
index b8571e5b8..948b811d3 100755
--- a/spec/unit/indirector/catalog/active_record.rb
+++ b/spec/unit/indirector/catalog/active_record.rb
@@ -75,7 +75,7 @@ describe Puppet::Node::Catalog::ActiveRecord do
 
     describe "when saving an instance" do
         before do
-            @host = stub 'host', :name => "foo", :save => nil, :setresources => nil, :last_compile= => nil
+            @host = stub 'host', :name => "foo", :save => nil, :merge_resources => nil, :last_compile= => nil
             Puppet::Rails::Host.stubs(:find_by_name).returns @host
             @catalog = Puppet::Node::Catalog.new("foo")
             @request = stub 'request', :key => "foo", :instance => @catalog
@@ -97,7 +97,7 @@ describe Puppet::Node::Catalog::ActiveRecord do
 
         it "should set the catalog vertices as resources on the Rails host instance" do
             @catalog.expects(:vertices).returns "foo"
-            @host.expects(:setresources).with("foo")
+            @host.expects(:merge_resources).with("foo")
 
             @terminus.save(@request)
         end
diff --git a/spec/unit/indirector/facts/active_record.rb b/spec/unit/indirector/facts/active_record.rb
index 340f2cf4c..fc35f1a45 100755
--- a/spec/unit/indirector/facts/active_record.rb
+++ b/spec/unit/indirector/facts/active_record.rb
@@ -67,7 +67,7 @@ describe Puppet::Node::Facts::ActiveRecord do
 
     describe "when saving an instance" do
         before do
-            @host = stub 'host', :name => "foo", :save => nil, :setfacts => nil
+            @host = stub 'host', :name => "foo", :save => nil, :merge_facts => nil
             Puppet::Rails::Host.stubs(:find_by_name).returns @host
             @facts = Puppet::Node::Facts.new("foo", "one" => "two", "three" => "four")
             @request = stub 'request', :key => "foo", :instance => @facts
@@ -89,7 +89,7 @@ describe Puppet::Node::Facts::ActiveRecord do
 
         it "should set the facts as facts on the Rails host instance" do
             # There is other stuff added to the hash.
-            @host.expects(:setfacts).with { |args| args["one"] == "two" and args["three"] == "four" }
+            @host.expects(:merge_facts).with { |args| args["one"] == "two" and args["three"] == "four" }
 
             @terminus.save(@request)
         end