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>

3 files changed, 306 insertions, 83 deletions

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.