Fixing #267. The problem was that the user provider was retrieving the @is value instead of the @should value, because it was using [] instead of the should method. I fixed the FakeModel to behave a bit more like real types, so that it keeps track of the is/should values, and also to keep track of which attributes are valid, since I immediately ran into another problem stemming from the use of the fakemodel.

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

7 files changed, 49 insertions, 15 deletions

diff --git a/lib/puppet/provider/group/groupadd.rb b/lib/puppet/provider/group/groupadd.rb
index 4c204ddf6..fe6150bd2 100644
--- a/lib/puppet/provider/group/groupadd.rb
+++ b/lib/puppet/provider/group/groupadd.rb
@@ -12,9 +12,9 @@ Puppet::Type.type(:group).provide :groupadd, :parent => Puppet::Provider::NameSe
 
     def addcmd
         cmd = [command(:add)]
-        if gid = @model[:gid]
+        if gid = @model.should(:gid)
             unless gid == :absent
-                cmd << flag(:gid) << "'%s'" % @model[:gid]
+                cmd << flag(:gid) << "'%s'" % gid
             end
         end
         if @model[:allowdupe] == :true
diff --git a/lib/puppet/provider/group/pw.rb b/lib/puppet/provider/group/pw.rb
index 48934a3c5..4cfcf997a 100644
--- a/lib/puppet/provider/group/pw.rb
+++ b/lib/puppet/provider/group/pw.rb
@@ -12,9 +12,9 @@ Puppet::Type.type(:group).provide :pw, :parent => Puppet::Provider::NameService:
 
     def addcmd
         cmd = [command(:pw), "groupadd", @model[:name]]
-        if gid = @model[:gid]
+        if gid = @model.should(:gid)
             unless gid == :absent
-                cmd << flag(:gid) << "'%s'" % @model[:gid]
+                cmd << flag(:gid) << "'%s'" % gid
             end
         end
 
diff --git a/lib/puppet/provider/nameservice/netinfo.rb b/lib/puppet/provider/nameservice/netinfo.rb
index be73f7cae..d7d6123e6 100644
--- a/lib/puppet/provider/nameservice/netinfo.rb
+++ b/lib/puppet/provider/nameservice/netinfo.rb
@@ -59,9 +59,6 @@ class NetInfo < Puppet::Provider::NameService
         # Because our stupid type can't create the whole thing at once,
         # we have to do this hackishness.  Yay.
         if arg == :present
-            if @model.class.name == :user
-                notice modifycmd(:groups, @model[:groups]).inspect
-            end
             # We need to generate the id if it's missing.
             @model.class.validstates.each do |name|
                 next if name == :ensure
diff --git a/lib/puppet/provider/user/pw.rb b/lib/puppet/provider/user/pw.rb
index 7342c42fe..393e2d8e0 100644
--- a/lib/puppet/provider/user/pw.rb
+++ b/lib/puppet/provider/user/pw.rb
@@ -22,11 +22,11 @@ Puppet::Type.type(:user).provide :pw, :parent => Puppet::Provider::NameService::
     def addcmd
         cmd = [command(:pw), "useradd", @model[:name]]
         @model.class.validstates.each do |state|
-            next if name == :ensure
+            next if state == :ensure
             # the value needs to be quoted, mostly because -c might
             # have spaces in it
-            if value = @model[state] and value != ""
-                cmd << flag(state) << "'%s'" % @model[state]
+            if value = @model.should(state) and value != ""
+                cmd << flag(state) << "'%s'" % value
             end
         end
 
diff --git a/lib/puppet/provider/user/useradd.rb b/lib/puppet/provider/user/useradd.rb
index edd5ec6e3..3b217e013 100644
--- a/lib/puppet/provider/user/useradd.rb
+++ b/lib/puppet/provider/user/useradd.rb
@@ -20,11 +20,11 @@ Puppet::Type.type(:user).provide :useradd, :parent => Puppet::Provider::NameServ
     def addcmd
         cmd = [command(:add)]
         @model.class.validstates.each do |state|
-            next if name == :ensure
+            next if state == :ensure
             # the value needs to be quoted, mostly because -c might
             # have spaces in it
-            if value = @model[state] and value != ""
-                cmd << flag(state) << "'%s'" % @model[state]
+            if value = @model.should(state) and value != ""
+                cmd << flag(state) << "'%s'" % value
             end
         end
         # stupid fedora
diff --git a/test/providers/user.rb b/test/providers/user.rb
index 15b453780..e400ff3ad 100644
--- a/test/providers/user.rb
+++ b/test/providers/user.rb
@@ -108,7 +108,6 @@ class TestUserProvider < Test::Unit::TestCase
 
     def eachstate
         Puppet::Type.type(:user).validstates.each do |state|
-            next if state == :ensure
             yield state
         end
     end
@@ -128,6 +127,7 @@ class TestUserProvider < Test::Unit::TestCase
     def fakedata(name, param)
         case param
         when :name: name
+        when :ensure: :present
         when :comment: "Puppet Testing User %s" % name
         when :gid: Process.gid()
         when :shell: findshell()
diff --git a/test/puppettest.rb b/test/puppettest.rb
index f67b2dc96..a2c3d186c 100644
--- a/test/puppettest.rb
+++ b/test/puppettest.rb
@@ -10,7 +10,7 @@ module TestPuppet
     include ObjectSpace
 
     # A baseclass for the faketypes.
-    class FakeModel < Hash
+    class FakeModel
         class << self
             attr_accessor :name
             @name = :fakemodel
@@ -24,7 +24,36 @@ module TestPuppet
             Puppet::Type.type(@name).validstate?(name)
         end
 
+        def self.to_s
+            "Fake%s" % @name.to_s.capitalize
+        end
+
+        def [](param)
+            if @realmodel.attrtype(param) == :state
+                @is[param]
+            else
+                @params[param]
+            end
+        end
+
+        def []=(param, value)
+            unless @realmodel.attrtype(param)
+                raise Puppet::DevError, "Invalid attribute %s for %s" %
+                    [param, @realmodel.name]
+            end
+            if @realmodel.attrtype(param) == :state
+                @should[param] = value
+            else
+                @params[param] = value
+            end
+        end
+
         def initialize(name)
+            @realmodel = Puppet::Type.type(self.class.name)
+            raise "Could not find type #{self.class.name}" unless @realmodel
+            @is = {}
+            @should = {}
+            @params = {}
             self[:name] = name
         end
 
@@ -32,6 +61,14 @@ module TestPuppet
             "%s(%s)" % [self.class.to_s.sub(/.+::/, ''), super()]
         end
 
+        def is(param)
+            @is[param]
+        end
+
+        def should(param)
+            @should[param]
+        end
+
         def name
             self[:name]
         end