Fixing most of the rails stuff. I think everything basically works now, and now I am just going through and making sure things get deleted when they are supposed (i.e., you remove a resource and it gets deleted from the host's config).

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

3 files changed, 110 insertions, 90 deletions

diff --git a/lib/puppet/parser/collector.rb b/lib/puppet/parser/collector.rb
index ea54c2dcf..841d8585f 100644
--- a/lib/puppet/parser/collector.rb
+++ b/lib/puppet/parser/collector.rb
@@ -6,13 +6,16 @@ class Puppet::Parser::Collector
     # Collect exported objects.
     def collect_exported
         require 'puppet/rails'
-        Puppet.info "collecting %s" % @type
         # First get everything from the export table.  Just reuse our
         # collect_virtual method but tell it to use 'exported? for the test.
         resources = collect_virtual(true)
 
         count = 0
 
+        unless @scope.host
+            raise Puppet::DevError, "Cannot collect resources for a nil host"
+        end
+
         # We're going to collect objects from rails, but we don't want any
         # objects from this host.
         host = Puppet::Rails::Host.find_by_name(@scope.host)
@@ -23,53 +26,23 @@ class Puppet::Parser::Collector
         else
             #Puppet.info "Host %s is uninitialized" % @scope.host
         end
-        Puppet.info "collecting %s" % @type
 
         # Now look them up in the rails db.  When we support attribute comparison
         # and such, we'll need to vary the conditions, but this works with no
         # attributes, anyway.
         time = Puppet::Util.thinmark do
-            Puppet::Rails::Resource.find_all_by_type_and_exported(@type, true,
+            Puppet::Rails::Resource.find_all_by_restype_and_exported(@type, true,
                 args
             ).each do |obj|
-                if existing = @scope.findresource(obj.type, obj.title)
-                    # See if we exported it; if so, just move on
-                    if @scope.host == obj.host.name
-                        next
-                    else
-                        # Next see if we've already collected this resource
-                        if existing.rails_id == obj.id
-                            # This is the one we've already collected
-                            next
-                        else
-                            raise Puppet::ParseError,
-                                "Exported resource %s[%s] cannot override local resource" %
-                                [obj.type, obj.title]
-                        end
-                    end
+                if resource = export_resource(obj)
+                    count += 1
+                    resources << resource
                 end
-                count += 1
-                begin
-                resource = obj.to_resource(self.scope)
-                
-                # XXX Because the scopes don't expect objects to return values,
-                # we have to manually add our objects to the scope.  This is
-                # uber-lame.
-                    scope.setresource(resource)
-                rescue => detail
-                    if Puppet[:trace]
-                        puts detail.backtrace
-                    end
-                    raise
-                end
-                resource.exported = false
-
-                resources << resource
             end
         end
 
-        scope.debug("Collected %s %s resources in %.2f seconds" %
-            [count, @type, time])
+        scope.debug("Collected %s %s resource%s in %.2f seconds" %
+            [count, @type, count == 1 ? "" : "s", time])
 
         return resources
     end
@@ -148,6 +121,42 @@ class Puppet::Parser::Collector
             return true
         end
     end
+
+    def export_resource(obj)
+        if existing = @scope.findresource(obj.restype, obj.title)
+            # See if we exported it; if so, just move on
+            if @scope.host == obj.host.name
+                return nil
+            else
+                # Next see if we've already collected this resource
+                if existing.rails_id == obj.id
+                    # This is the one we've already collected
+                    return nil
+                else
+                    raise Puppet::ParseError,
+                        "Exported resource %s cannot override local resource" %
+                        [obj.ref]
+                end
+            end
+        end
+
+        begin
+            resource = obj.to_resource(self.scope)
+            
+            # XXX Because the scopes don't expect objects to return values,
+            # we have to manually add our objects to the scope.  This is
+            # �ber-lame.
+            scope.setresource(resource)
+        rescue => detail
+            if Puppet[:trace]
+                puts detail.backtrace
+            end
+            raise
+        end
+        resource.exported = false
+
+        return resource
+    end
 end
 
 # $Id$
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 7ea85dded..857b5fa39 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -101,27 +101,6 @@ class Puppet::Parser::Resource
         else
             self.fail "Cannot find definition %s" % self.type
         end
-
-
-#        if builtin?
-#            devfail "Cannot evaluate a builtin type"
-#        end
-#
-#        unless klass = scope.finddefine(self.type)
-#            self.fail "Cannot find definition %s" % self.type
-#        end
-#
-#        finish()
-#
-#        scope.deleteresource(self)
-#
-#        return klass.evaluate(:scope => scope,
-#                              :type => self.type,
-#                              :name => self.title,
-#                              :arguments => self.to_hash,
-#                              :scope => self.scope,
-#                              :exported => self.exported
-#        )
     ensure
         @evaluated = true
     end
@@ -252,9 +231,24 @@ class Puppet::Parser::Resource
         end
     end
 
-    # Store our object as a Rails object.  We need the host object we're storing it
-    # with.
-    def store(host)
+    #def tags
+    #    unless defined? @tags
+    #        @tags = scope.tags
+    #        @tags << self.type
+    #    end
+    #    @tags
+    #end
+
+    def to_hash
+        @params.inject({}) do |hash, ary|
+            param = ary[1]
+            hash[param.name] = param.value
+            hash
+        end
+    end
+
+    # Turn our parser resource into a Rails resource.
+    def to_rails(host)
         args = {}
         #FIXME: support files/lines, etc.
         #%w{type title tags file line exported}.each do |param|
@@ -280,28 +274,26 @@ class Puppet::Parser::Resource
             obj = host.resources.build(args)
         end
 
+        if l = self.line
+            obj.line = l
+        end
+
         # Either way, now add our parameters
+        exists = {}
+        obj.param_names.each do |pn| exists[pn.name] = pn end
         @params.each do |name, param|
-            param.store(obj)
+            param.to_rails(obj)
+            exists.delete(name.to_s) if exists.include?(name.to_s)
         end
 
-        return obj
-    end
-
-    #def tags
-    #    unless defined? @tags
-    #        @tags = scope.tags
-    #        @tags << self.type
-    #    end
-    #    @tags
-    #end
-
-    def to_hash
-        @params.inject({}) do |hash, ary|
-            param = ary[1]
-            hash[param.name] = param.value
-            hash
+        unless exists.empty?
+            obj.save
+            exists.each do |name, param|
+                obj.param_names.delete(param)
+            end
         end
+
+        return obj
     end
 
     def to_s
diff --git a/lib/puppet/parser/resource/param.rb b/lib/puppet/parser/resource/param.rb
index 2e5d78034..6f24f1486 100644
--- a/lib/puppet/parser/resource/param.rb
+++ b/lib/puppet/parser/resource/param.rb
@@ -16,22 +16,41 @@ class Puppet::Parser::Resource::Param
     end
 
     # Store this parameter in a Rails db.
-    def store(resource)
-        args = {}
-        #[:name, :value, :line, :file].each do |var|
-        [:name, :value].each do |var|
-            args[var] = self.send(var)
+    def to_rails(res)
+        values = value.is_a?(Array) ? value : [value]
+
+        unless pn = res.param_names.find_by_name(self.name.to_s)
+            # We're creating it anew.
+            pn = res.param_names.build(:name => self.name.to_s)
+        end
+
+        if l = self.line
+            pn.line = Integer(l)
+        end
+
+        exists = {}
+        pn.param_values.each { |pv| exists[pv.value] = pv }
+        values.each do |value|
+            unless pn.param_values.find_by_value(value)
+                pn.param_values.build(:value => value)
+            end
+            # Mark that this is still valid.
+            if exists.include?(value)
+                exists.delete(value)
+            end
         end
-        args[:name] = args[:name].to_s
-        args[:name].each do |name|
-            pn = resource.param_names.find_or_create_by_name(name)
-            args[:value].each do |value|
-                pv = pn.param_values.find_or_create_by_value(value)
+
+        # And remove any existing values that are not in the current value list.
+        unless exists.empty?
+            # We have to save the current state else the deletion somehow deletes
+            # our new values.
+            pn.save
+            exists.each do |value, obj|
+                pn.param_values.delete(obj)
             end
         end
-        obj = resource.param_names.find_by_name(args[:name], :include => :param_values)
 
-        return obj
+        return pn
     end
 
     def to_s