Refactoring some of the rails code. The speed is now pretty good, but the tagging stuff does not seem to be working and is certainly working very ineffficiently. Blake says he is going to take a look at that.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2350 980ebf18-57e1-0310-9a29-db15c13687c0

5 files changed, 75 insertions, 90 deletions

diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 8ef382204..9e14b83ee 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -258,7 +258,7 @@ class Puppet::Parser::Resource
     # Turn our parser resource into a Rails resource.
     def to_rails(host, resource = nil)
         args = {}
-        [:type, :title, :tags, :file, :line, :exported].each do |param|
+        [:type, :title, :line, :exported].each do |param|
             # 'type' isn't a valid column name, so we have to use something else.
             if param == :type
                 to = :restype
@@ -274,7 +274,8 @@ class Puppet::Parser::Resource
         if resource
             # We exist
             args.each do |param, value|
-                unless resource[param] == value
+                v = resource[param]
+                unless v == value
                     resource[param] = value
                 end
             end
@@ -283,30 +284,17 @@ class Puppet::Parser::Resource
             resource = host.resources.build(args)
         end
 
-        # Either way, now add our parameters
-        newparams = @params.dup
-        remove = []
-        resource.param_names.each do |pn|
-            name = pn.name.intern
-            if param = newparams[name]
-                # Mark that we found this in the db
-                newparams.delete(name)
-                param.to_rails(resource, pn)
-            else
-                remove << pn
+        # Handle file and tags specially
+        [:file, :tags].each do |param|
+            if self.send(param) and (!resource.send(param) or resource.send(param) != self.send(param))
+                resource.send(param.to_s + "=", self.send(param))
             end
         end
 
-        newparams.each do |name, param|
-            param.to_rails(resource)
-        end
-
-        remove.each do |param|
-            resource.param_names.delete(param)
-        end
-        #obj.collection_merge(:param_names, @params) do |name, param|
-        #    param.to_rails(obj)
-        #end
+        # Either way, now add our parameters
+        newparams = {}
+        @params.each { |name, p| newparams[name.to_s] = p }
+        resource.collection_merge :param_names, :existing => resource.param_names, :updates => newparams
 
         return resource
     end
diff --git a/lib/puppet/parser/resource/param.rb b/lib/puppet/parser/resource/param.rb
index 56a50de1c..bc33ba8f9 100644
--- a/lib/puppet/parser/resource/param.rb
+++ b/lib/puppet/parser/resource/param.rb
@@ -46,16 +46,8 @@ class Puppet::Parser::Resource::Param
                 pn.param_values.build(:value => v.to_s)
             end
             pn.param_values = objects
-            #pn.save
         end
 
-#        pn.collection_merge(:param_values, values) do |value|
-#            unless pv = pn.param_values.find_by_value(value)
-#                pv = pn.param_values.build(:value => value)
-#            end
-#            pv
-#        end
-
         return pn
     end
 
diff --git a/lib/puppet/rails/host.rb b/lib/puppet/rails/host.rb
index ac547e013..fbf51c838 100644
--- a/lib/puppet/rails/host.rb
+++ b/lib/puppet/rails/host.rb
@@ -3,6 +3,8 @@ require 'puppet/rails/fact_name'
 require 'puppet/rails/source_file'
 require 'puppet/util/rails/collection_merger'
 
+# Puppet::TIME_DEBUG = true
+
 class Puppet::Rails::Host < ActiveRecord::Base
     include Puppet::Util
     include Puppet::Util::CollectionMerger
@@ -40,7 +42,8 @@ class Puppet::Rails::Host < ActiveRecord::Base
             #unless host = find_by_name(name)
             seconds = Benchmark.realtime {
                 #unless host = find_by_name(name, :include => {:resources => {:param_names => :param_values}, :fact_names => :fact_values})
-                unless host = find_by_name(name)
+                unless host = find_by_name(name, :include => {:fact_names => :fact_values})
+                #unless host = find_by_name(name)
                     host = new(:name => name)
                 end
             }
@@ -56,7 +59,11 @@ class Puppet::Rails::Host < ActiveRecord::Base
                 raise ArgumentError, "You must pass resources"
             end
 
-            host.setresources(hash[:resources])
+
+            seconds = Benchmark.realtime {
+                host.setresources(hash[:resources])
+            }
+            Puppet.notice("Handled resources in %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG)
 
             host.last_compile = Time.now
 
@@ -85,35 +92,17 @@ class Puppet::Rails::Host < ActiveRecord::Base
         facts = facts.dup
         remove = []
 
-        existing = nil
-        seconds = Benchmark.realtime {
-            existing = fact_names.find(:all, :include => :fact_values)
-        }
-        Puppet.debug("Searched for facts in %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG)
-
-        existing.each do |fn|
-            if value = facts[fn.name]
-                facts.delete(fn.name)
-                fn.fact_values.each do |fv|
-                    unless value == fv.value
-                        fv.value = value
-                    end
+        collection_merge :fact_names, :updates => facts, :modify => Proc.new { |fn, name, value|
+            fn.fact_values.each do |fv|
+                unless value == fv.value
+                    fv.value = value
                 end
-            else
-                remove << fn
+                break
             end
-        end
-
-        # Make a new fact for the rest of them
-        facts.each do |fact, value|
-            fn = fact_names.build(:name => fact)
+        }, :create => Proc.new { |name, value|
+            fn = fact_names.build(:name => name)
             fn.fact_values = [fn.fact_values.build(:value => value)]
-        end
-
-        # Now remove anything necessary.
-        remove.each do |fn|
-            fact_names.delete(fn)
-        end
+        }
     end
 
     # Set our resources.
@@ -122,28 +111,16 @@ class Puppet::Rails::Host < ActiveRecord::Base
         remove = []
         existing = nil
         seconds = Benchmark.realtime {
+            #existing = resources.find(:all)
             existing = resources.find(:all, :include => {:param_names => :param_values})
+            #existing = resources
         }
         Puppet.notice("Searched for resources in %0.2f seconds" % seconds) if defined?(Puppet::TIME_DEBUG)
         list.each do |resource|
             compiled[resource.ref] = resource
         end
-        existing.each do |resource|
-            if comp = compiled[resource.ref]
-                compiled.delete(comp.ref)
-                comp.to_rails(self, resource)
-            else
-                remove << resource
-            end
-        end
-
-        compiled.each do |name, resource|
-            resource.to_rails(self)
-        end
 
-        remove.each do |resource|
-            resources.delete(resource)
-        end
+        collection_merge :resources, :existing => existing, :updates => compiled
     end
 
     def update_connect_time
diff --git a/lib/puppet/rails/resource.rb b/lib/puppet/rails/resource.rb
index c3bf8c38a..4f26af6d9 100644
--- a/lib/puppet/rails/resource.rb
+++ b/lib/puppet/rails/resource.rb
@@ -14,19 +14,30 @@ class Puppet::Rails::Resource < ActiveRecord::Base
     acts_as_taggable
     
     def tags=(tags)
-        tags.each do |tag|   
-            self.tag_with tag
+        #puts "setting tags %s" % tags.inspect
+        self.tag_with(tags.join(","))
+    end
+
+    def file
+        if f = self.source_file
+            return f.filename
+        else
+            return nil
         end
     end
 
     def file=(file)
-       self.source_file = Puppet::Rails::SourceFile.new(:filename => file)
+        self.source_file = Puppet::Rails::SourceFile.new(:filename => file)
     end
 
     def [](param)
         return super || parameter(param)
     end
 
+    def name
+        ref()
+    end
+
     def parameter(param)
         if pn = param_names.find_by_name(param)
             if pv = pn.param_values.find(:first)
diff --git a/lib/puppet/util/rails/collection_merger.rb b/lib/puppet/util/rails/collection_merger.rb
index 7afd76f9c..69e0309c9 100644
--- a/lib/puppet/util/rails/collection_merger.rb
+++ b/lib/puppet/util/rails/collection_merger.rb
@@ -1,24 +1,41 @@
 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, list)
-        remove = send(collection).dup
-
-        list.each do |value|
-            object = yield(value)
-            if remove.include?(object)
-                remove.delete(object)
+    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
 
-        unless remove.empty?
-            # We have to save the current state else the deletion somehow deletes
-            # our new values.
-            save
-            remove.each do |r|
-                send(collection).delete(r)
+        # 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
 end