An intermediate commit. All of the classes that use parsedfile are assuredly broken, since I have basically completely rewritten it. These classes have been a thorn in my side almost since I created them, yet they have been significantly less functional that I wanted. So, I decided to do the rewrite I have been putting off, just to spend all of the maintenance time now so I do not spend 3 days on them every release.

Tomorrow I will be porting all of the existing types (including cron, hopefully) over to this new base.  This will also make it possible to add other types of providers to these classes; we should be able to reuse the netinfo provider for os x types, and we should be able to create a cron provider that writes to /etc/crontab instead of user crontabs.

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

9 files changed, 257 insertions, 150 deletions

diff --git a/lib/puppet/metatype/evaluation.rb b/lib/puppet/metatype/evaluation.rb
index dde86a28d..30a3e7a80 100644
--- a/lib/puppet/metatype/evaluation.rb
+++ b/lib/puppet/metatype/evaluation.rb
@@ -1,51 +1,4 @@
 class Puppet::Type
-    # retrieve the current value of all contained states
-    def retrieve
-        # it's important to use the method here, as it follows the order
-        # in which they're defined in the object
-        states().each { |state|
-            state.retrieve
-        }
-    end
-
-    # Retrieve the changes associated with all of the states.
-    def statechanges
-        # If we are changing the existence of the object, then none of
-        # the other states matter.
-        changes = []
-        if @states.include?(:ensure) and ! @states[:ensure].insync?
-            #self.info "ensuring %s from %s" %
-            #    [@states[:ensure].should, @states[:ensure].is]
-            changes = [Puppet::StateChange.new(@states[:ensure])]
-        # Else, if the 'ensure' state is correctly absent, then do
-        # nothing
-        elsif @states.include?(:ensure) and @states[:ensure].is == :absent
-            #self.info "Object is correctly absent"
-            return []
-        else
-            #if @states.include?(:ensure)
-            #    self.info "ensure: Is: %s, Should: %s" %
-            #        [@states[:ensure].is, @states[:ensure].should]
-            #else
-            #    self.info "no ensure state"
-            #end
-            changes = states().find_all { |state|
-                ! state.insync?
-            }.collect { |state|
-                Puppet::StateChange.new(state)
-            }
-        end
-
-        if Puppet[:debug] and changes.length > 0
-            self.debug("Changing " + changes.collect { |ch|
-                    ch.state.name
-                }.join(",")
-            )
-        end
-
-        changes
-    end
-
     # this method is responsible for collecting state changes
     # we always descend into the children before we evaluate our current
     # states
@@ -94,6 +47,13 @@ class Puppet::Type
         return changes.flatten
     end
 
+    # By default, try flushing the provider.
+    def flush
+        if self.provider and self.provider.respond_to?(:flush)
+            self.provider.flush
+        end
+    end
+
     # if all contained objects are in sync, then we're in sync
     # FIXME I don't think this is used on the type instances any more,
     # it's really only used for testing
@@ -119,6 +79,53 @@ class Puppet::Type
         #self.debug("%s sync status is %s" % [self,insync])
         return insync
     end
+
+    # retrieve the current value of all contained states
+    def retrieve
+        # it's important to use the method here, as it follows the order
+        # in which they're defined in the object
+        states().each { |state|
+            state.retrieve
+        }
+    end
+
+    # Retrieve the changes associated with all of the states.
+    def statechanges
+        # If we are changing the existence of the object, then none of
+        # the other states matter.
+        changes = []
+        if @states.include?(:ensure) and ! @states[:ensure].insync?
+            #self.info "ensuring %s from %s" %
+            #    [@states[:ensure].should, @states[:ensure].is]
+            changes = [Puppet::StateChange.new(@states[:ensure])]
+        # Else, if the 'ensure' state is correctly absent, then do
+        # nothing
+        elsif @states.include?(:ensure) and @states[:ensure].is == :absent
+            #self.info "Object is correctly absent"
+            return []
+        else
+            #if @states.include?(:ensure)
+            #    self.info "ensure: Is: %s, Should: %s" %
+            #        [@states[:ensure].is, @states[:ensure].should]
+            #else
+            #    self.info "no ensure state"
+            #end
+            changes = states().find_all { |state|
+                ! state.insync?
+            }.collect { |state|
+                Puppet::StateChange.new(state)
+            }
+        end
+
+        if Puppet[:debug] and changes.length > 0
+            self.debug("Changing " + changes.collect { |ch|
+                    ch.state.name
+                }.join(",")
+            )
+        end
+
+        changes
+    end
 end
 
 # $Id$
diff --git a/lib/puppet/metatype/providers.rb b/lib/puppet/metatype/providers.rb
index c59b7f84a..b1ce820be 100644
--- a/lib/puppet/metatype/providers.rb
+++ b/lib/puppet/metatype/providers.rb
@@ -154,10 +154,13 @@ class Puppet::Type
 
     def self.unprovide(name)
         if @providers.has_key? name
+            rmclass(name,
+                :hash => @providers,
+                :prefix => "Provider"
+            )
             if @defaultprovider and @defaultprovider.name == name
                 @defaultprovider = nil
             end
-            @providers.delete(name)
         end
     end
 
diff --git a/lib/puppet/provider/mount/parsed.rb b/lib/puppet/provider/mount/parsed.rb
index 3db4d7ac2..086cd5cd9 100755
--- a/lib/puppet/provider/mount/parsed.rb
+++ b/lib/puppet/provider/mount/parsed.rb
@@ -156,6 +156,7 @@ Puppet::Type.type(:mount).provide :parsed, :parent => Puppet::Provider::ParsedFi
         when "Solaris": df = "#{command(:df)} -k"
         end
         %x{#{df}}.split("\n").find do |line|
+            p line
             fs = line.split(/\s+/)[-1]
             if platform == "Darwin"
                 fs == "/private/var/automount" + @model[:path] or
diff --git a/lib/puppet/provider/parsedfile.rb b/lib/puppet/provider/parsedfile.rb
index 35267c673..f9e98d96a 100755
--- a/lib/puppet/provider/parsedfile.rb
+++ b/lib/puppet/provider/parsedfile.rb
@@ -1,44 +1,58 @@
 require 'puppet'
-
+require 'puppet/util/fileparsing'
+
+# This provider can be used as the parent class for a provider that
+# parses and generates files.  Its content must be loaded via the
+# 'prefetch' method, and the file will be written when 'flush' is called
+# on the provider instance.  At this point, the file is written once
+# for every provider instance.
+#
+# Once the provider prefetches the data, it's the model's job to copy
+# that data over to the @is variables.
 class Puppet::Provider::ParsedFile < Puppet::Provider
+    extend Puppet::Util::FileParsing
+
     class << self
-        attr_accessor :filetype, :fields
-        attr_reader :path
-        attr_writer :fileobj
+        attr_reader :filetype
+        attr_accessor :default_target
     end
 
-    # Override 'newstate' so that all states default to having the
-    # correct parent type
-    def self.newstate(name, options = {}, &block)
-        options[:parent] ||= Puppet::State::ParsedParam
-        super(name, options, &block)
-    end
+    attr_accessor :state_hash
 
-    # Add another type var.
-    def self.initvars
-        @instances = []
-        super
+    def self.clear
+        @target_objects.clear
+        @records.clear
     end
 
-    # Add a non-object comment or whatever to our list of instances
-    def self.comment(line)
-        @instances << line
+    def self.filetype=(type)
+        if type.is_a?(Class)
+            @filetype = type
+        elsif klass = Puppet::FileType.filetype(type)
+            @filetype = klass
+        else
+            raise ArgumentError, "Invalid filetype %s" % type
+        end
     end
 
-    # Override the default Puppet::Type method, because instances
-    # also need to be deleted from the @instances hash
-    def self.delete(child)
-        if @instances.include?(child)
-            @instances.delete(child)
+    # Flush all of the targets for which there are modified records.
+    def self.flush(record)
+        return unless defined?(@modified) and ! @modified.empty?
+
+        # Make sure this record is on the list to be flushed.
+        unless record[:on_disk]
+            record[:on_disk] = true
+            @records << record
         end
-        super
-    end
 
-    # Initialize the object if necessary.
-    def self.fileobj
-        @fileobj ||= @filetype.new(@path)
+        @modified.sort { |a,b| a.to_s <=> b.to_s }.uniq.each do |target|
+            Puppet.debug "Flushing %s provider target %s" % [@model.name, target]
+            flush_target(target)
+        end
+    end
 
-        @fileobj
+    # Flush all of the records relating to a specific target.
+    def self.flush_target(target)
+        target_object(target).write(to_file(target_records(target)))
     end
 
     # Return the header placed at the top of each generated file, warning
@@ -49,24 +63,93 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
 # HEADER: is definitely not recommended.\n}
     end
 
-    # Parse a file
-    #
-    # Subclasses must override this method.
-    def self.parse(text)
-        raise Puppet::DevError, "Parse was not overridden in %s" %
-            self.name
+    # Add another type var.
+    def self.initvars
+        @records = []
+
+        # Default to flat files
+        @filetype = Puppet::FileType.filetype(:flat)
+        super
+    end
+
+    # Create attribute methods for each of the model's non-metaparam attributes.
+    def self.model=(model)
+        [model.validstates, model.parameters].flatten.each do |attr|
+            define_method(attr) do @state_hash[attr] end
+
+            define_method(attr.to_s + "=") do |val|
+                # Mark that this target was modified.
+                modeltarget = @model[:target]
+
+                # If they're the same, then just mark that one as modified
+                if @state_hash[:target] and @state_hash[:target] == modeltarget
+                    self.class.modified(modeltarget)
+                else
+                    # Always mark the modeltarget as modified, and if there's
+                    # and old state_hash target, mark it as modified and replace
+                    # it.
+                    self.class.modified(modeltarget)
+                    if @state_hash[:target]
+                        self.class.modified(@state_hash[:target])
+                    end
+                    @state_hash[:target] = modeltarget
+                end
+                @state_hash[attr] = val.to_s
+            end
+        end
+        @model = model
+    end
+
+    # Mark a target as modified so we know to flush it.  This only gets
+    # used within the attr= methods.
+    def self.modified(target)
+        @modified ||= []
+        @modified << target
+    end
+
+    # Retrieve all of the data from disk.  There are three ways to know
+    # while files to retrieve:  We might have a list of file objects already
+    # set up, there might be instances of our associated model and they
+    # will have a path parameter set, and we will have a default path
+    # set.  We need to turn those three locations into a list of files,
+    # prefetch each one, and make sure they're associated with each appropriate
+    # model instance.
+    def self.prefetch
+        # Reset the record list.
+        @records = []
+        targets().each do |target|
+            prefetch_target(target)
+        end
     end
 
-    # If they change the path, we need to get rid of our cache object
-    def self.path=(path)
-        @fileobj = nil
-        @path = path
+    # Prefetch an individual target.
+    def self.prefetch_target(target)
+        @records += retrieve(target).each do |r|
+            r[:on_disk] = true
+            r[:target] = target
+        end
+
+        # Retrieve the current state of this target.
+        target_records(target).find_all { |i| i.is_a?(Hash) }.each do |record|
+            # Find any model instances whose names match our instances.
+            if instance = self.model[record[:name]]
+                instance.provider.state_hash = record
+            elsif self.respond_to?(:match)
+                if instance = self.match(record)
+                    record[:name] = instance[:name]
+                    instance.provider.state_hash = record
+                end
+            end
+            # Any unmatched records are assumed to be unmanaged, so
+            # we just leave them alone.
+        end
     end
 
     # Retrieve the text for the file. Returns nil in the unlikely
     # event that it doesn't exist.
-    def self.retrieve
-        text = fileobj.read
+    def self.retrieve(path)
+        # XXX We need to be doing something special here in case of failure.
+        text = target_object(path).read
         if text.nil? or text == ""
             # there is no file
             return []
@@ -76,73 +159,68 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
     end
 
     # Write out the file.
-    def self.store(instances)
-        if instances.empty?
-            Puppet.notice "No %s instances for %s" % [self.name, @path]
+    def self.store(records)
+        if records.empty?
+            Puppet.notice "No %s records for %s" % [self.name, @path]
         else
-            fileobj.write(self.to_file(instances))
+            target_object.write(self.to_file(instances))
         end
     end
 
-    # Collect all Host instances convert them into literal text.
-    def self.to_file(instances)
-        str = self.header()
-        unless instances.empty?
-            # Reject empty hashes and those with :ensure == :absent
-            str += instances.reject { |obj|
-                obj.is_a? Hash and (obj.empty? or obj[:ensure] == :absent)
-            }.collect { |obj|
-                # If it's a hash, convert it, otherwise just write it out
-                if obj.is_a?(Hash)
-                    to_record(obj)
-                else
-                    obj.to_s
-                end
-            }.join("\n") + "\n"
+    # Initialize the object if necessary.
+    def self.target_object(target)
+        @target_objects ||= {}
+        @target_objects[target] ||= @filetype.new(target)
 
-            return str
-        else
-            Puppet.notice "No %s instances" % self.name
-            return ""
-        end
+        @target_objects[target]
     end
 
-    # A Simple wrapper method that subclasses can override, so there's more control
-    # over how instances are retrieved.
-    def allinstances
-        self.class.retrieve
+    # Find all of the records for a given target
+    def self.target_records(target)
+        @records.find_all { |r| r[:target] == target }
     end
 
-    def clear
-        super
-        @instances = nil
-    end
+    # Find a list of all of the targets that we should be reading.  This is
+    # used to figure out what targets we need to prefetch.
+    def self.targets
+        targets = []
+        # First get the default target
+        unless self.default_target
+            raise Puppet::DevError, "Parsed Providers must define a default target"
+        end
+        targets << self.default_target
 
-    # Return a hash that maps to our info, if possible.
-    def hash
-        @instances = allinstances()
+        # Then get each of the file objects
+        targets += @target_objects.keys
 
-        namevar = @model.class.namevar
-        if @instances and h = @instances.find do |o|
-            o.is_a? Hash and o[namevar] == @model[namevar]
-        end
-            @me = h
-        else
-            @me = {}
-            if @instances.empty?
-                @instances = [@me]
-            else
-                @instances << @me
-            end
+        # Lastly, check the file from any model instances
+        self.model.each do |model|
+            targets << model[:target]
         end
 
-        return @me
+        targets.uniq
+    end
+
+    # Write our data to disk.
+    def flush
+        # Make sure we've got a target and name set.
+        @state_hash[:target] ||= @model[:target]
+        @state_hash[:name] ||= @model[:name]
+
+        self.class.flush(@state_hash)
     end
 
     def initialize(model)
         super
 
-        @instances = nil
+        # Initialize our data hash with the record type.  The record type
+        # in the subclass has to create a record matching its name.
+        @state_hash = {:record_type => self.class.name}
+    end
+
+    # Have we been modified since the last flush?
+    def modified?
+        @state[:flush_needed]
     end
 
     def store(hash = nil)
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 05fbe6914..a7ea48a19 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -262,6 +262,7 @@ class Transaction
                 nil
             end
         }.reject { |o| o.nil? }.uniq.each do |klass|
+            # XXX We need to do something special here in case of failure.
             if klass.respond_to?(:prefetch)
                 klass.prefetch
             end
diff --git a/lib/puppet/type/mount.rb b/lib/puppet/type/mount.rb
index b92ab735b..259d2f894 100755
--- a/lib/puppet/type/mount.rb
+++ b/lib/puppet/type/mount.rb
@@ -13,6 +13,9 @@ module Puppet
                   is entered into the mount table and mounted."
 
             newvalue(:present, :event => :mount_created) do
+                # The parsedtype nature of the provider automatically
+                # creates the mount in the file, and we're not mounting,
+                # so we don't do anything here.
             end
 
             newvalue(:absent, :event => :mount_deleted) do
@@ -37,12 +40,15 @@ module Puppet
             end
 
             def retrieve
+                fail "called retrieve"
+                Puppet.warning @is.inspect
                 if provider.mounted?
-                    return :mounted
+                    @is = :mounted
                 else
                     val = super()
-                    return val
+                    @is = val
                 end
+
             end
         end
 
diff --git a/lib/puppet/type/parsedtype.rb b/lib/puppet/type/parsedtype.rb
index 4297946a9..607134c46 100755
--- a/lib/puppet/type/parsedtype.rb
+++ b/lib/puppet/type/parsedtype.rb
@@ -120,6 +120,8 @@ module Puppet
                     obj.is = [param, value]
                 }
             end
+
+            return obj
         end
 
         # Override 'newstate' so that all states default to having the
@@ -130,14 +132,15 @@ module Puppet
         end
 
         def self.list
-            suitableprovider.collect do |provider|
-                puts provider.name
-                provider.retrieve.collect { |i| p i; i.is_a? Hash }.collect { |i| hash2obj(i) }
+            ret = suitableprovider.collect do |provider|
+                provider.retrieve.find_all { |i| i.is_a? Hash }.collect { |i| hash2obj(i) }
             end.flatten
         end
 
         def self.listbyname
-            retrieve.collect { |i| i.is_a? Hash }.collect { |i| i[:name] }
+            suitableprovider.collect do |provider|
+                provider.retrieve.find_all { |i| i.is_a? Hash }.collect { |i| i[:name] }
+            end.flatten
         end
 
         # Make sure they've got an explicit :ensure class.
diff --git a/lib/puppet/util/classgen.rb b/lib/puppet/util/classgen.rb
index 8fa2a34cf..0c774eecd 100644
--- a/lib/puppet/util/classgen.rb
+++ b/lib/puppet/util/classgen.rb
@@ -50,7 +50,7 @@ module Puppet::Util::ClassGen
     # Remove an existing class
     def rmclass(name, options)
         options = symbolize_options(options)
-        const = name2const(name)
+        const = genconst_string(name, options)
         retval = false
         if const_defined? const
             remove_const(const)
@@ -68,6 +68,16 @@ module Puppet::Util::ClassGen
 
     private
 
+    # Generate the constant to create or remove.
+    def genconst_string(name, options)
+        unless const = options[:constant]
+            prefix = options[:prefix] || ""
+            const = prefix + name2const(name)
+        end
+
+        return const
+    end
+
     # This does the actual work of creating our class or module.  It's just a
     # slightly abstract version of genclass.
     def genthing(name, type, options, block)
@@ -120,10 +130,7 @@ module Puppet::Util::ClassGen
 
     # Handle the setting and/or removing of the associated constant.
     def handleclassconst(klass, name, options)
-        unless const = options[:constant]
-            prefix = options[:prefix] || ""
-            const = prefix + name2const(name)
-        end
+        const = genconst_string(name, options)
 
         if const_defined? const
             if options[:overwrite]
diff --git a/lib/puppet/util/fileparsing.rb b/lib/puppet/util/fileparsing.rb
index e4998cf7e..705192b11 100644
--- a/lib/puppet/util/fileparsing.rb
+++ b/lib/puppet/util/fileparsing.rb
@@ -108,10 +108,11 @@ module Puppet::Util::FileParsing
             raise ArgumentError, "Must include a list of fields"
         end
 
+        invalidfields = [:record_type, :target, :on_disk]
         options[:fields] = options[:fields].collect do |field|
             r = symbolize(field)
-            if r == :record_type
-                raise ArgumentError.new("Cannot have fields named record_type")
+            if invalidfields.include?(r)
+                raise ArgumentError.new("Cannot have fields named %s" % r)
             end
             r
         end