Merge branch 'tickets/0.24.x/1622' into 0.24.x

Conflicts:

	CHANGELOG

6 files changed, 67 insertions, 107 deletions

diff --git a/lib/puppet/provider/nameservice/directoryservice.rb b/lib/puppet/provider/nameservice/directoryservice.rb
index e2e68b2ca..fcc44f9e3 100644
--- a/lib/puppet/provider/nameservice/directoryservice.rb
+++ b/lib/puppet/provider/nameservice/directoryservice.rb
@@ -206,9 +206,18 @@ class DirectoryService < Puppet::Provider::NameService
         if ensure_value == :present
             @resource.class.validproperties.each do |name|
                 next if name == :ensure
-                next unless val = @resource.should(name) || autogen(name)
-                # JJM: This calls the method.
-                self.send(name.to_s + "=", val)
+
+                # LAK: We use property.sync here rather than directly calling
+                # the settor method because the properties might do some kind
+                # of conversion.  In particular, the user gid property might
+                # have a string and need to convert it to a number
+                if @resource.should(name)
+                    @resource.property(name).sync
+                elsif value = autogen(name)
+                    self.send(name.to_s + "=", value)
+                else
+                    next
+                end
             end
         end 
     end
diff --git a/lib/puppet/provider/nameservice/netinfo.rb b/lib/puppet/provider/nameservice/netinfo.rb
index 29600052b..ac7bc94b1 100644
--- a/lib/puppet/provider/nameservice/netinfo.rb
+++ b/lib/puppet/provider/nameservice/netinfo.rb
@@ -138,8 +138,18 @@ class NetInfo < Puppet::Provider::NameService
         if arg == :present
             @resource.class.validproperties.each do |name|
                 next if name == :ensure
-                next unless val = @resource.should(name) || autogen(name)
-                self.send(name.to_s + "=", val)
+
+                # LAK: We use property.sync here rather than directly calling
+                # the settor method because the properties might do some kind
+                # of conversion.  In particular, the user gid property might
+                # have a string and need to convert it to a number
+                if @resource.should(name)
+                    @resource.property(name).sync
+                elsif value = autogen(name)
+                    self.send(name.to_s + "=", value)
+                else
+                    next
+                end
             end
         end
     end
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index c7a866e2c..e377a3547 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -901,19 +901,31 @@ class Type
         
     # retrieve the current value of all contained properties
     def retrieve
-         return currentpropvalues
+        return currentpropvalues
     end
     
-    # get a hash of the current properties.  
-    def currentpropvalues(override_value = nil)
-        # it's important to use the method here, as it follows the order
-        # in which they're defined in the object
-        return properties().inject({}) { | prophash, property|
-                   prophash[property] = override_value.nil? ? 
-                                          property.retrieve : 
-                                             override_value
-                   prophash
-               }
+    # Get a hash of the current properties.  Returns a hash with
+    # the actual property instance as the key and the current value
+    # as the, um, value.
+    def currentpropvalues
+        # It's important to use the 'properties' method here, as it follows the order
+        # in which they're defined in the class.  It also guarantees that 'ensure'
+        # is the first property, which is important for skipping 'retrieve' on
+        # all the properties if the resource is absent.
+        ensure_state = false
+        return properties().inject({}) do | prophash, property|
+            if property.name == :ensure
+                ensure_state = property.retrieve
+                prophash[property] = ensure_state
+            else
+                if ensure_state == :absent
+                    prophash[property] = :absent
+                else
+                    prophash[property] = property.retrieve
+                end
+            end
+            prophash
+        end
     end
 
     # Are we running in noop mode?
diff --git a/lib/puppet/type/group.rb b/lib/puppet/type/group.rb
index 2a5ac30da..cb11a60a4 100755
--- a/lib/puppet/type/group.rb
+++ b/lib/puppet/type/group.rb
@@ -118,14 +118,6 @@ module Puppet
 
             defaultto false
         end
-
-        def retrieve
-            if self.provider and @provider.exists?
-                return super
-            else
-               return currentpropvalues(:absent) 
-            end
-        end
     end
 end
 
diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb
index 039bcb7cb..bb0a86fd0 100755
--- a/lib/puppet/type/user.rb
+++ b/lib/puppet/type/user.rb
@@ -49,25 +49,6 @@ module Puppet
                     return :absent
                 end
             end
-
-            # The default 'sync' method only selects among a list of registered
-            # values.
-            def sync
-#                if self.insync?
-#                    self.info "already in sync"
-#                    return nil
-                #else
-                    #self.info "%s vs %s" % [self.is.inspect, self.should.inspect]
-#               end
-                unless self.class.values
-                    self.devfail "No values defined for %s" %
-                        self.class.name
-                end
-
-                # Set ourselves to whatever our should value is.
-                self.set(self.should)
-            end
-
         end
 
         newproperty(:uid) do
@@ -95,50 +76,26 @@ module Puppet
         newproperty(:gid) do
             desc "The user's primary group.  Can be specified numerically or
                 by name."
-            
-            def found?
-                defined? @found and @found
-            end
 
-            munge do |gid|
-                method = :getgrgid
-                case gid
-                when String
-                    if gid =~ /^[-0-9]+$/
-                        gid = Integer(gid)
-                    else
-                        method = :getgrnam
-                    end
-                when Symbol
-                    unless gid == :auto or gid == :absent
-                        self.devfail "Invalid GID %s" % gid
-                    end
-                    # these are treated specially by sync()
-                    return gid
-                end
-
-                if group = Puppet::Util.gid(gid)
-                    @found = true
-                    return group
+            munge do |value|
+                if value.is_a?(String) and value =~ /^[-0-9]+$/
+                    Integer(value)
                 else
-                    @found = false
-                    return gid
+                    value
                 end
             end
 
-            # *shudder*  Make sure that we've looked up the group and gotten
-            # an ID for it.  Yuck-o.
-            def should
-                unless defined? @should
-                    return super
-                end
-                unless found?
-                    @should = @should.each { |val|
-                        next unless val
-                        Puppet::Util.gid(val)
-                    }
+            def sync
+                found = false
+                @should.each do |value|
+                    if number = Puppet::Util.gid(value)
+                        provider.gid = number
+                        found = true
+                        break
+                    end
                 end
-                super
+
+                fail "Could not find group(s) %s" % @should.join(",") unless found
             end
         end
 
@@ -234,29 +191,6 @@ module Puppet
             end
         end
 
-        # these three properties are all implemented differently on each platform,
-        # so i'm disabling them for now
-
-        # FIXME Puppet::Property::UserLocked is currently non-functional
-        #newproperty(:locked) do
-        #    desc "The expected return code.  An error will be returned if the
-        #        executed command returns something else."
-        #end
-
-        # FIXME Puppet::Property::UserExpire is currently non-functional
-        #newproperty(:expire) do
-        #    desc "The expected return code.  An error will be returned if the
-        #        executed command returns something else."
-        #    @objectaddflag = "-e"
-        #end
-
-        # FIXME Puppet::Property::UserInactive is currently non-functional
-        #newproperty(:inactive) do
-        #    desc "The expected return code.  An error will be returned if the
-        #        executed command returns something else."
-        #    @objectaddflag = "-f"
-        #end
-
         newparam(:name) do
             desc "User name.  While limitations are determined for
                 each operating system, it is generally a good idea to keep to
@@ -338,7 +272,7 @@ module Puppet
                 current_value = :absent
 
                 if absent
-                   prophash[property] = :absent
+                    prophash[property] = :absent
                 else
                     current_value = property.retrieve
                     prophash[property] = current_value
@@ -346,7 +280,6 @@ module Puppet
 
                 if property.name == :ensure and current_value == :absent
                     absent = true
-#                    next
                 end
                 prophash
             }
diff --git a/lib/puppet/type/zone.rb b/lib/puppet/type/zone.rb
index 4fd92672c..7601ec47b 100644
--- a/lib/puppet/type/zone.rb
+++ b/lib/puppet/type/zone.rb
@@ -377,7 +377,11 @@ end
             result = setstatus(hash)
             result
         else
-            return currentpropvalues(:absent)
+            # Return all properties as absent.
+            return properties().inject({}) do | prophash, property|
+                prophash[property] = :absent
+                prophash
+            end
         end
     end