making "Type.new" private, and switching to "Type.create", so that i can merge new objects with existing objects and such; converted all files, and tested them

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

10 files changed, 222 insertions, 55 deletions

diff --git a/lib/puppet/client.rb b/lib/puppet/client.rb
index 679db2c51..cbb7dbedc 100644
--- a/lib/puppet/client.rb
+++ b/lib/puppet/client.rb
@@ -245,6 +245,7 @@ module Puppet
                     end
                 end
 
+                # FIXME this should be in getconfig, not apply
                 container = @objects.to_type
                 #if @local
                 #    container = @objects.to_type
diff --git a/lib/puppet/parser/ast.rb b/lib/puppet/parser/ast.rb
index 1918f60e9..a4fcb301a 100644
--- a/lib/puppet/parser/ast.rb
+++ b/lib/puppet/parser/ast.rb
@@ -646,7 +646,7 @@ module Puppet
                     # should we implicitly iterate here?
                     # yes, i believe that we essentially have to...
                     objnames.collect { |objname|
-                        if object.is_a?(Component)
+                        if object.is_a?(AST::Component)
                             objname = "%s[%s]" % [objtype,objname]
                             objtype = "component"
                         end
@@ -990,7 +990,7 @@ module Puppet
 
                     begin
                         scope.settype(name,
-                            Component.new(
+                            AST::Component.new(
                                 :name => name,
                                 :args => args,
                                 :code => @code
diff --git a/lib/puppet/server/fileserver.rb b/lib/puppet/server/fileserver.rb
index 04449c025..a666b4c7a 100755
--- a/lib/puppet/server/fileserver.rb
+++ b/lib/puppet/server/fileserver.rb
@@ -27,7 +27,7 @@ class Server
 
             obj = nil
             unless obj = Puppet::Type::PFile[dir]
-                obj = Puppet::Type::PFile.new(
+                obj = Puppet::Type::PFile.create(
                     :name => dir,
                     :check => CHECKPARAMS
                 )
diff --git a/lib/puppet/sslcertificates.rb b/lib/puppet/sslcertificates.rb
index da6853a8c..bc3382ab6 100755
--- a/lib/puppet/sslcertificates.rb
+++ b/lib/puppet/sslcertificates.rb
@@ -14,7 +14,7 @@ module SSLCertificates
     def self.mkdir(dir)
         # this is all a bunch of stupid hackery
         unless FileTest.exists?(dir)
-            comp = Puppet::Type::Component.new(
+            comp = Puppet::Type::Component.create(
                 :name => "certdir creation"
             )
             path = ['']
@@ -26,7 +26,7 @@ module SSLCertificates
                         raise "%s exists but is not a directory" % File.join(path)
                     end
                 else
-                    obj = Puppet::Type.type(:file).new(
+                    obj = Puppet::Type.type(:file).create(
                         :name => File.join(path),
                         :mode => "750",
                         :create => "directory"
diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb
index 046522ca2..57e5f8377 100644
--- a/lib/puppet/transportable.rb
+++ b/lib/puppet/transportable.rb
@@ -53,7 +53,7 @@ module Puppet
             retobj = nil
             if type = Puppet::Type.type(self.type)
                 begin
-                    retobj = type.new(self)
+                    retobj = type.create(self)
                 rescue => detail
                     # FIXME TransObject should be handling what happens when there's an error
                     if Puppet[:debug]
@@ -123,7 +123,7 @@ module Puppet
             else
                 Puppet.debug "%s has no parameters" % @name
             end
-            container = Puppet::Type::Component.new(hash)
+            container = Puppet::Type::Component.create(hash)
             nametable = {}
 
             self.each { |child|
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 27f5c95b4..39050ca55 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -681,6 +681,32 @@ class Type < Puppet::Element
     public
 
     #---------------------------------------------------------------
+    # force users to call this, so that we can merge objects if
+    # necessary
+    def self.create(hash)
+        if name = self["name"] || self[:name]
+            # if the object already exists
+            if retobj = self[name]
+                retobj.merge(self)
+
+                return retobj
+            else
+                return new(hash)
+            end
+        else
+            return new(hash)
+            #raise Puppet::Error, "You must specify a name for objects of type %s" %
+            #    self.to_s
+        end
+    end
+    #---------------------------------------------------------------
+
+    # and then make 'new' private
+    class << self
+        private :new
+    end
+
+    #---------------------------------------------------------------
     # initialize the type instance
     def initialize(hash)
         @children = []
@@ -769,6 +795,31 @@ class Type < Puppet::Element
     #---------------------------------------------------------------
 
     #---------------------------------------------------------------
+    # merge new information with an existing object, checking for conflicts
+    # and such
+    def merge(hash)
+        hash.each { |param, value|
+            if param.is_a?(String)
+                hash[param.intern] = value
+                hash.delete(param)
+            end
+        }
+
+        hash.each { |param, value|
+            # FIXME we should really allow equal values, but for now, don't allow
+            # any values
+
+            if oldval = self[param]
+                if self.parent.class == self.class # they're a result of recursion
+                end
+                unless oldval == value
+                end
+            end
+        }
+    end
+    #---------------------------------------------------------------
+
+    #---------------------------------------------------------------
     # derive the instance name based on class.namevar
     def name
         unless defined? @name and @name
diff --git a/lib/puppet/type/cron.rb b/lib/puppet/type/cron.rb
index 8694fcd07..7e0ae9fc4 100755
--- a/lib/puppet/type/cron.rb
+++ b/lib/puppet/type/cron.rb
@@ -8,51 +8,73 @@ require 'puppet/type/state'
 module Puppet
     module CronType
         module Default
-            def self.fields
-                return [:minute, :hour, :weekday, :monthday, :command]
+            def self.read(user)
+                %x{crontab -u #{user} -l}
             end
 
-            def self.retrieve(user)
-                %x{crontab -u #{user} -l 2>/dev/null}.split("\n").each { |line|
-                    hash = {}
-                    ary = line.split(" ")
-                    fields().each { |param|
-                        hash[param] = ary.shift
-                    }
-
-                    if ary.length > 0
-                        hash[:command] += " " + ary.join(" ")
-                    end
-                    cron = nil
-                    unless cron = Puppet::Type::Cron[hash[:command]]
-                        cron = Puppet::Type::Cron.new
-                    end
-
-                    hash.each { |param, value|
-                        cron.is = [param, value]
-                    }
+            def self.write(user, text)
+                IO.popen("crontab -u #{user} -") { |p|
+                    p.print text
                 }
             end
+        end
 
-            def self.sync(user)
+        module SunOS
+            def self.read(user)
+                %x{crontab -l #{user}}
+            end
+
+            def self.write(user, text)
+                # FIXME this should use our user object, since it already does
+                # this for us
+                require 'etc'
+
+                begin
+                    obj = Etc.getpwnam(user)
+                rescue ArgumentError
+                    raise Puppet::Error, "User %s not found"
+                end
+
+                uid = obj.uid
+
+                olduid = nil
+                if Process.uid == uid
+                    olduid = Process.uid
+                    Process.euid = uid
+                end
+
+                IO.popen("crontab -") { |p|
+                    p.print text
+                }
+
+                if olduid
+                    Process.euid = olduid
+                end
             end
         end
     end
 
     class State
         class CronCommand < Puppet::State
+            @name = :command
             @doc = "The command to execute in the cron job.  The environment
                 provided to the command varies by local system rules, and it is
                 best to always provide a fully qualified command.  The user's
                 profile is not sourced when the command is run, so if the
                 user's environment is desired it should be sourced manually."
-            @name = :command
 
             def retrieve
                 # nothing...
             end
 
             def sync
+                @parent.store
+
+                if @is == :notfound
+                    return :cron_created
+                else
+                    return :cron_changed
+                end
             end
         end
     end
@@ -65,28 +87,30 @@ module Puppet
 
             @parameters = [
                 :name,
-                :command,
                 :user,
                 :minute,
                 :hour,
                 :weekday,
+                :month,
                 :monthday
             ]
 
             @paramdoc[:name] = "The symbolic name of the cron job.  This name
-                is used for human reference only and is optional."
-            @paramdoc[:user] = "The user to run the command as.  This user
-                must be allowed to run cron jobs, which is not currently checked
-                by Puppet."
+                is used for human reference only."
+            @paramdoc[:user] = "The user to run the command as.  This user must
+                be allowed to run cron jobs, which is not currently checked by
+                Puppet."
             @paramdoc[:minute] = "The minute at which to run the cron job.
                 Optional; if specified, must be between 0 and 59, inclusive."
             @paramdoc[:hour] = "The hour at which to run the cron job. Optional;
                 if specified, must be between 0 and 23, inclusive."
             @paramdoc[:weekday] = "The weekday on which to run the command.
                 Optional; if specified, must be between 0 and 6, inclusive, with
-                0 being Sunday."
+                0 being Sunday, or must be the name of the day (e.g., Tuesday)."
+            @paramdoc[:month] = "The month of the year.  Optional; if specified
+                must be between 1 and 12 or the month name (e.g., December)."
             @paramdoc[:monthday] = "The day of the month on which to run the
-                command.  Optional; if specified, must be between 0 and 31."
+                command.  Optional; if specified, must be between 1 and 31."
 
             @doc = "Installs cron jobs.  All fields except the command 
                 and the user are optional, although specifying no periodic
@@ -99,12 +123,76 @@ module Puppet
 
             @synced = {}
 
+            @instances = {}
+
             case Facter["operatingsystem"].value
-            when "Stub":
-                # nothing
+            when "SunOS":
+                @crontype = Puppet::CronType::SunOS
             else
-                Puppet.err "including default"
-                include Puppet::CronType::Default
+                @crontype = Puppet::CronType::Default
+            end
+
+            # FIXME so the fundamental problem is, what if the object
+            # already exists?
+
+            def self.fields
+                return [:minute, :hour, :monthday, :month, :weekday, :command]
+            end
+
+            def self.instance(obj)
+                @instances << obj
+            end
+
+            def self.retrieve(user)
+                Puppet.err "Retrieving"
+                crons = []
+                hash = {}
+                #%x{crontab -u #{user} -l 2>/dev/null}.split("\n").each { |line|
+                @crontype.read(user).split("\n").each { |line|
+                    case line
+                    when /^# Puppet Name: (\w+)$/: hash[:name] = $1
+                    when /^#/: # add other comments to the list as they are
+                        crons << line 
+                    else
+                        ary = line.split(" ")
+                        fields().each { |param|
+                            hash[param] = ary.shift
+                        }
+
+                        if ary.length > 0
+                            hash[:command] += " " + ary.join(" ")
+                        end
+                        cron = nil
+                        unless hash.include?(:name)
+                            Puppet.info "Autogenerating name for %s" % hash[:command]
+                            hash[:name] = "cron-%s" % hash.object_id
+                        end
+
+                        unless cron = Puppet::Type::Cron[hash[:command]]
+                            cron = Puppet::Type::Cron.create
+                        end
+
+                        hash.each { |param, value|
+                            cron.is = [param, value]
+                        }
+                        crons << cron
+                        hash.clear
+                    end
+                }
+
+                @instances[user] = crons
+                @loaded[user] = Time.now
+            end
+
+            def self.store(user)
+                if @instances.include?(user)
+                    @crontype.write(user,
+                        @instances[user].join("\n")
+                    )
+                    @synced[user] = Time.now
+                else
+                    Puppet.notice "No cron instances for %s" % user
+                end
             end
 
             def self.loaded?(user)
@@ -116,9 +204,29 @@ module Puppet
             end
 
             def self.sync(user)
+                Puppet.err ary.length
+                str = ary.collect { |obj|
+                    Puppet.err obj.name
+                    self.to_cron(obj)
+                }.join("\n")
+
+                puts str
+            end
+
+            def self.to_cron(obj)
+                hash = {:command => obj.should(:command)}
+                self.fields().reject { |f| f == :command }.each { |param|
+                    hash[param] = obj[param] || "*"
+                }
+
+                self.fields.collect { |f|
+                    hash[f]
+                }.join(" ")
             end
 
             def initialize(hash)
+                self.class.instance(self)
+                super
             end
 
             def is=(ary)
@@ -127,20 +235,27 @@ module Puppet
                     param = param.intern
                 end
                 unless @states.include?(param)
-                    if stateklass = self.class.validstate?(name) 
-                        begin
-                        @states[param] = stateklass.new(
-                            :parent => self
-                        )
-                        rescue => detail
-                        end
-                    else
-                        raise Puppet::Error, "Invalid parameter %s" % [name]
-                    end
-
+                    self.newstate(param)
                 end
                 @states[param].is = value
             end
+
+            def paramuser=(user)
+                @parameters[:user] = user
+            end
+
+            def retrieve
+                unless @parameters.include?(:user)
+                    raise Puppet::Error, "You must specify the cron user"
+                end
+
+                # look for the existing instance...
+                # and then set @is = :notfound
+            end
+
+            def store
+                self.class.store(@parameters[:user])
+            end
         end
     end
 end
diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb
index 7155db3f1..a04346201 100644
--- a/lib/puppet/type/package.rb
+++ b/lib/puppet/type/package.rb
@@ -515,7 +515,7 @@ module Puppet
                     }
                     return object
                 else # just create it
-                    obj = self.new(:name => name)
+                    obj = self.create(:name => name)
                     hash.each { |var,value|
                         obj.addis(var,value)
                     }
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index df22c59b2..f8f897017 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -1102,7 +1102,7 @@ module Puppet
                 else # create it anew
                     #notice "Creating new file with args %s" % args.inspect
                     begin
-                        child = klass.new(args)
+                        child = klass.create(args)
                         child.parent = self
                         @children << child
                     rescue Puppet::Error => detail
diff --git a/lib/puppet/type/symlink.rb b/lib/puppet/type/symlink.rb
index 0dcbc244a..050904297 100755
--- a/lib/puppet/type/symlink.rb
+++ b/lib/puppet/type/symlink.rb
@@ -167,7 +167,7 @@ module Puppet
                     :source => @target
                 }
 
-                dir = Puppet::Type::PFile.new(args)
+                dir = Puppet::Type::PFile.create(args)
                 dir.parent = self
                 Puppet.debug "Got dir %s" % dir.name
                 self.push dir