Finishing off the type/provider interface work, including adding package prefetch for all packages. The only not-done one is yum -- prefetch is set up for rpm, but not yum. We need to modify prefetching so that it also prefetches latest information, to avoid having to run yum so many times.

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

36 files changed, 244 insertions, 160 deletions

diff --git a/CHANGELOG b/CHANGELOG
index f053ce238..1408b4962 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,9 @@
+    Finishing up the type => provider interface work.  Basically, package
+    providers now return lists of provider instances.  In the proces,
+    I rewrote the interface between package types and providers, and also
+    enabled prefetching on all packages.  This should significantly speed
+    up most package operations.
+
     Hopefully fixing the file descriptor/open port problems, with patches
     from Valentin Vidic.
 
diff --git a/lib/puppet/metatype/instances.rb b/lib/puppet/metatype/instances.rb
index be7d1be1c..8a0a1d00c 100644
--- a/lib/puppet/metatype/instances.rb
+++ b/lib/puppet/metatype/instances.rb
@@ -255,15 +255,31 @@ class Puppet::Type
         end
 
         # Put the default provider first, then the rest of the suitable providers.
+        packages = {}
+        providers_by_source.collect do |provider|
+            provider.instances.collect do |instance|
+                if other = packages[instance.name]
+                    Puppet.warning "Package %s found in both %s and %s; skipping the %s version" %
+                        [instance.name, other.class.name, instance.class.name, instance.class.name]
+                    next
+                end
+                packages[instance.name] = instance
+
+                create(:name => instance.name, :provider => instance, :check => :all)
+            end
+        end.flatten.compact
+    end
+
+    # Return a list of one suitable provider per source, with the default provider first.
+    def self.providers_by_source
+        # Put the default provider first, then the rest of the suitable providers.
         sources = []
         [defaultprovider, suitableprovider].flatten.uniq.collect do |provider|
             next if sources.include?(provider.source)
 
             sources << provider.source
-            provider.instances.collect do |instance|
-                create(:name => instance.name, :provider => instance, :check => :all)
-            end
-        end.flatten.compact
+            provider
+        end.compact
     end
 
     # Create the path for logging and such.
diff --git a/lib/puppet/provider.rb b/lib/puppet/provider.rb
index a725611d0..3e23bf08a 100644
--- a/lib/puppet/provider.rb
+++ b/lib/puppet/provider.rb
@@ -329,6 +329,11 @@ class Puppet::Provider
         self.class.command(name)
     end
 
+    # Get a parameter value.
+    def get(param)
+        @property_hash[symbolize(param)] || :absent
+    end
+
     def initialize(resource = nil)
         if resource.is_a?(Hash)
             @property_hash = resource.dup
diff --git a/lib/puppet/provider/nameservice.rb b/lib/puppet/provider/nameservice.rb
index 8e01bfe21..bc205d43a 100644
--- a/lib/puppet/provider/nameservice.rb
+++ b/lib/puppet/provider/nameservice.rb
@@ -25,22 +25,10 @@ class Puppet::Provider::NameService < Puppet::Provider
             super
         end
 
-        def list
+        def instances
             objects = []
             listbyname do |name|
-                obj = nil
-                check = resource_type.validproperties
-                if obj = resource_type[name]
-                    obj[:check] = check
-                else
-                    # unless it exists, create it as an unmanaged object
-                    obj = resource_type.create(:name => name, :check => check)
-                end
-
-                next unless obj # In case there was an error somewhere
-                
-                objects << obj
-                yield obj if block_given?
+                objects << new(:name => name, :ensure => :present)
             end
 
             objects
diff --git a/lib/puppet/provider/package/apple.rb b/lib/puppet/provider/package/apple.rb
index e9fb00482..83606fc51 100755
--- a/lib/puppet/provider/package/apple.rb
+++ b/lib/puppet/provider/package/apple.rb
@@ -1,5 +1,7 @@
+require 'puppet/provider/package'
+
 # OS X Packaging sucks.  We can install packages, but that's about it.
-Puppet::Type.type(:package).provide :apple do
+Puppet::Type.type(:package).provide :apple, :parent => Puppet::Provider::Package do
     desc "Package management based on OS X's builtin packaging system.  This is
         essentially the simplest and least functional package system in existence --
         it only supports installation; no deletion or upgrades.  The provider will
diff --git a/lib/puppet/provider/package/apt.rb b/lib/puppet/provider/package/apt.rb
index 264e40b81..d39ce01f4 100755
--- a/lib/puppet/provider/package/apt.rb
+++ b/lib/puppet/provider/package/apt.rb
@@ -1,4 +1,4 @@
-Puppet::Type.type(:package).provide :apt, :parent => :dpkg do
+Puppet::Type.type(:package).provide :apt, :parent => :dpkg, :source => :dpkg do
     # Provide sorting functionality
     include Puppet::Util::Package
 
diff --git a/lib/puppet/provider/package/aptitude.rb b/lib/puppet/provider/package/aptitude.rb
index 79e853c64..cad7449f4 100755
--- a/lib/puppet/provider/package/aptitude.rb
+++ b/lib/puppet/provider/package/aptitude.rb
@@ -1,4 +1,4 @@
-Puppet::Type.type(:package).provide :aptitude, :parent => :apt do
+Puppet::Type.type(:package).provide :aptitude, :parent => :apt, :source => :dpkg do
     desc "Package management via ``aptitude``."
 
     has_feature :versionable
diff --git a/lib/puppet/provider/package/aptrpm.rb b/lib/puppet/provider/package/aptrpm.rb
index 766f52525..00d60b236 100644
--- a/lib/puppet/provider/package/aptrpm.rb
+++ b/lib/puppet/provider/package/aptrpm.rb
@@ -1,4 +1,4 @@
-Puppet::Type.type(:package).provide :aptrpm, :parent => :rpm do
+Puppet::Type.type(:package).provide :aptrpm, :parent => :rpm, :source => :rpm do
     # Provide sorting functionality
     include Puppet::Util::Package
 
diff --git a/lib/puppet/provider/package/blastwave.rb b/lib/puppet/provider/package/blastwave.rb
index 07f904eef..83e9e60cf 100755
--- a/lib/puppet/provider/package/blastwave.rb
+++ b/lib/puppet/provider/package/blastwave.rb
@@ -1,5 +1,5 @@
 # Packaging using Blastwave's pkg-get program.
-Puppet::Type.type(:package).provide :blastwave, :parent => :sun do
+Puppet::Type.type(:package).provide :blastwave, :parent => :sun, :source => :sun do
     desc "Package management using Blastwave.org's ``pkg-get`` command on Solaris."
     pkgget = "pkg-get"
     if FileTest.executable?("/opt/csw/bin/pkg-get")
diff --git a/lib/puppet/provider/package/darwinport.rb b/lib/puppet/provider/package/darwinport.rb
index 8c0bbb891..1f317d31c 100755
--- a/lib/puppet/provider/package/darwinport.rb
+++ b/lib/puppet/provider/package/darwinport.rb
@@ -1,4 +1,6 @@
-Puppet::Type.type(:package).provide :darwinport do
+require 'puppet/provider/package'
+
+Puppet::Type.type(:package).provide :darwinport, :parent => Puppet::Provider::Package do
     desc "Package management using DarwinPorts on OS X."
 
     commands :port => "/opt/local/bin/port"
diff --git a/lib/puppet/provider/package/dpkg.rb b/lib/puppet/provider/package/dpkg.rb
index e09638669..6756bee1b 100755
--- a/lib/puppet/provider/package/dpkg.rb
+++ b/lib/puppet/provider/package/dpkg.rb
@@ -1,4 +1,6 @@
-Puppet::Type.type(:package).provide :dpkg do
+require 'puppet/provider/package'
+
+Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package do
     desc "Package management via ``dpkg``.  Because this only uses ``dpkg``
         and not ``apt``, you must specify the source of any packages you want
         to manage."
@@ -103,7 +105,7 @@ Puppet::Type.type(:package).provide :dpkg do
 
     def purge
         dpkg "--purge", @resource[:name]
-	 end
+    end
 end
 
 # $Id$
diff --git a/lib/puppet/provider/package/freebsd.rb b/lib/puppet/provider/package/freebsd.rb
index e9fbf6a4c..7e94bb795 100755
--- a/lib/puppet/provider/package/freebsd.rb
+++ b/lib/puppet/provider/package/freebsd.rb
@@ -26,13 +26,12 @@ Puppet::Type.type(:package).provide :freebsd, :parent => :openbsd do
     end
 
     def query
-        self.class.list
-
-        if @resource.is(:ensure)
-            return :listed
-        else
-            return nil
+        self.class.instances.each do |provider|
+            if provider.name == @resource.name
+                return provider.properties
+            end
         end
+        return nil
     end
 
     def uninstall
diff --git a/lib/puppet/provider/package/gem.rb b/lib/puppet/provider/package/gem.rb
index fb5e243e9..f160adeb7 100755
--- a/lib/puppet/provider/package/gem.rb
+++ b/lib/puppet/provider/package/gem.rb
@@ -1,5 +1,7 @@
+require 'puppet/provider/package'
+
 # Ruby gems support.
-Puppet::Type.type(:package).provide :gem do
+Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package do
     desc "Ruby Gem support.  By default uses remote gems, but you can specify
         the path to a local gem via ``source``."
 
diff --git a/lib/puppet/provider/package/openbsd.rb b/lib/puppet/provider/package/openbsd.rb
index 0e238f546..34739dc08 100755
--- a/lib/puppet/provider/package/openbsd.rb
+++ b/lib/puppet/provider/package/openbsd.rb
@@ -1,5 +1,7 @@
+require 'puppet/provider/package'
+
 # Packaging on OpenBSD.  Doesn't work anywhere else that I know of.
-Puppet::Type.type(:package).provide :openbsd do
+Puppet::Type.type(:package).provide :openbsd, :parent => Puppet::Provider::Package do
     desc "OpenBSD's form of ``pkg_add`` support."
 
     commands :pkginfo => "pkg_info", :pkgadd => "pkg_add", :pkgdelete => "pkg_delete"
diff --git a/lib/puppet/provider/package/pkgdmg.rb b/lib/puppet/provider/package/pkgdmg.rb
index e0864764a..b62772fdd 100644
--- a/lib/puppet/provider/package/pkgdmg.rb
+++ b/lib/puppet/provider/package/pkgdmg.rb
@@ -14,7 +14,9 @@
 # require 'ruby-debug'
 # Debugger.start
 
-Puppet::Type.type(:package).provide :pkgdmg do
+require 'puppet/provider/package'
+
+Puppet::Type.type(:package).provide :pkgdmg, :parent => Puppet::Provider::Package do
     desc "Package management based on Apple's Installer.app and DiskUtility.app"
 
     confine :exists => "/Library/Receipts"
diff --git a/lib/puppet/provider/package/portage.rb b/lib/puppet/provider/package/portage.rb
index 3b4c1a44f..2e06177cc 100644
--- a/lib/puppet/provider/package/portage.rb
+++ b/lib/puppet/provider/package/portage.rb
@@ -1,4 +1,6 @@
-Puppet::Type.type(:package).provide :portage do
+require 'puppet/provider/package'
+
+Puppet::Type.type(:package).provide :portage, :parent => Puppet::Provider::Package do
     desc "Provides packaging support for Gentoo's portage system."
 
     has_feature :versionable
diff --git a/lib/puppet/provider/package/ports.rb b/lib/puppet/provider/package/ports.rb
index 6c464cb66..1410de154 100755
--- a/lib/puppet/provider/package/ports.rb
+++ b/lib/puppet/provider/package/ports.rb
@@ -1,4 +1,4 @@
-Puppet::Type.type(:package).provide :ports, :parent => :freebsd do
+Puppet::Type.type(:package).provide :ports, :parent => :freebsd, :source => :freebsd do
     desc "Support for FreeBSD's ports.  Again, this still mixes packages
         and ports."
 
@@ -74,13 +74,13 @@ Puppet::Type.type(:package).provide :ports, :parent => :freebsd do
     end
 
     def query
-        self.class.list
-
-        if @resource.is(:ensure) and @resource.is(:ensure) != :absent
-            return :listed
-        else
-            return nil
+        self.class.instances.each do |instance|
+            if instance.name == self.name
+                return instance.properties
+            end
         end
+
+        return nil
     end
 
     def uninstall
diff --git a/lib/puppet/provider/package/rpm.rb b/lib/puppet/provider/package/rpm.rb
index 3b9ac8b15..de81bb12e 100755
--- a/lib/puppet/provider/package/rpm.rb
+++ b/lib/puppet/provider/package/rpm.rb
@@ -1,5 +1,6 @@
+require 'puppet/provider/package'
 # RPM packaging.  Should work anywhere that has rpm installed.
-Puppet::Type.type(:package).provide :rpm do
+Puppet::Type.type(:package).provide :rpm, :source => :rpm, :parent => Puppet::Provider::Package do
     desc "RPM packaging support; should work anywhere with a working ``rpm``
         binary."
 
@@ -43,16 +44,6 @@ Puppet::Type.type(:package).provide :rpm do
         return packages
     end
 
-    # Get rid of our cached values.
-    def flush
-        @current_values = {}
-    end
-
-    def initialize(*args)
-        super
-        @current_values = {}
-    end
-
     # Find the fully versioned package name and the version alone. Returns
     # a hash with entries :instance => fully versioned package name, and 
     # :ensure => version-release
@@ -80,7 +71,7 @@ Puppet::Type.type(:package).provide :rpm do
 
         @nvr = hash[:instance]
 
-        @current_values = hash
+        @property_hash = hash
 
         return hash
     end
@@ -103,13 +94,13 @@ Puppet::Type.type(:package).provide :rpm do
         end
         # RPM gets pissy if you try to install an already 
         # installed package
-        if @resource.should(:ensure) == @current_values[:ensure] or
-            @resource.should(:ensure) == :latest && @current_values[:ensure] == latest
+        if @resource.should(:ensure) == @property_hash[:ensure] or
+            @resource.should(:ensure) == :latest && @property_hash[:ensure] == latest
             return
         end
 
         flag = "-i"
-        if @current_values[:ensure] and @current_values[:ensure] != :absent
+        if @property_hash[:ensure] and @property_hash[:ensure] != :absent
             flag = "-U"
         end
 
diff --git a/lib/puppet/provider/package/sun.rb b/lib/puppet/provider/package/sun.rb
index c5b954881..4275c69fa 100755
--- a/lib/puppet/provider/package/sun.rb
+++ b/lib/puppet/provider/package/sun.rb
@@ -1,6 +1,8 @@
-# Sun packaging.  No one else uses these package tools, AFAIK.
+# Sun packaging.
 
-Puppet::Type.type(:package).provide :sun do
+require 'puppet/provider/package'
+
+Puppet::Type.type(:package).provide :sun, :parent => Puppet::Provider::Package do
     desc "Sun's packaging system.  Requires that you specify the source for
         the packages you're managing."
     commands :pkginfo => "/usr/bin/pkginfo",
diff --git a/lib/puppet/provider/package/sunfreeware.rb b/lib/puppet/provider/package/sunfreeware.rb
index 7933eb42f..6c5e1d702 100755
--- a/lib/puppet/provider/package/sunfreeware.rb
+++ b/lib/puppet/provider/package/sunfreeware.rb
@@ -1,5 +1,5 @@
 # At this point, it's an exact copy of the Blastwave stuff.
-Puppet::Type.type(:package).provide :sunfreeware, :parent => :blastwave do
+Puppet::Type.type(:package).provide :sunfreeware, :parent => :blastwave, :source => :sun do
     desc "Package management using sunfreeware.com's ``pkg-get`` command on Solaris.
         At this point, support is exactly the same as ``blastwave`` support and
         has not actually been tested."
diff --git a/lib/puppet/provider/package/up2date.rb b/lib/puppet/provider/package/up2date.rb
index 5dc32e34a..84545888a 100644
--- a/lib/puppet/provider/package/up2date.rb
+++ b/lib/puppet/provider/package/up2date.rb
@@ -1,4 +1,4 @@
-Puppet.type(:package).provide :up2date, :parent => :rpm do
+Puppet.type(:package).provide :up2date, :parent => :rpm, :source => :rpm do
     desc "Support for Red Hat's proprietary ``up2date`` package update
         mechanism."
 
diff --git a/lib/puppet/provider/package/yum.rb b/lib/puppet/provider/package/yum.rb
index 34b9b7f10..b9212ef37 100755
--- a/lib/puppet/provider/package/yum.rb
+++ b/lib/puppet/provider/package/yum.rb
@@ -1,4 +1,4 @@
-Puppet::Type.type(:package).provide :yum, :parent => :rpm do
+Puppet::Type.type(:package).provide :yum, :parent => :rpm, :source => :rpm do
     desc "Support via ``yum``."
 
     has_feature :versionable
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index d27f4038a..bb756740d 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -483,6 +483,7 @@ class Transaction
 
         # Now call prefetch, passing in the resources so that the provider instances can be replaced.
         prefetchers.each do |provider, resources|
+            Puppet.debug "Prefetching %s resources for %s" % [provider.name, provider.resource_type.name]
             begin
                 provider.prefetch(resources)
             rescue => detail
diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb
index d116238a1..38b5b1443 100644
--- a/lib/puppet/type/package.rb
+++ b/lib/puppet/type/package.rb
@@ -383,7 +383,7 @@ module Puppet
         # The 'query' method returns a hash of info if the package
         # exists and returns nil if it does not.
         def exists?
-            @provider.query
+            @provider.get(:ensure) != :absent
         end
 
         # okay, there are two ways that a package could be created...
@@ -413,49 +413,14 @@ module Puppet
         end
 
         def retrieve
-            # If the package is installed, then retrieve all of the information
-            # about it and set it appropriately.
-            if hash = @provider.query
-                if hash == :listed # Mmmm, hackalicious
-                    return {}
+            @provider.properties.inject({}) do |props, ary|
+                name, value = ary
+                if prop = @parameters[name]
+                    props[prop] = value
                 end
-                hash.each { |param, value|
-                    unless self.class.validattr?(param)
-                        hash.delete(param)
-                    end
-                }
-
-                setparams(hash)
-                
-                return properties().inject({}) { |prop_hash, property|
-                           prop_hash[property] = hash[property.name] if hash.has_key?(property.name)
-                           prop_hash
-                       }
-            else
-                # Else just mark all of the properties absent.
-                # FIXARB: Not sure why this is using validproperties instead of
-                #         properties()?
-                return self.class.validproperties.inject({}) { |h, name|
-                          h[@parameters[name]] = :absent
-                          h
-                       }
+                props
             end
         end
-
-        # Set all of the params' "is" value.  Most are parameters, but some
-        # are properties.
-        def setparams(hash)
-            # Everything on packages is a parameter except :ensure
-            hash.each { |param, value| 
-                next if param == :ensure
-                if self.class.attrtype(param) == :property
-                    add_property_parameter(param)
-                    self.provider.send("%s=" % param, value)
-                else
-                    self[param] = value
-                end
-            }
-        end
     end # Puppet.type(:package)
 end
 
diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb
index 2df311633..89b64cc97 100644
--- a/lib/puppet/type/service.rb
+++ b/lib/puppet/type/service.rb
@@ -134,8 +134,8 @@ module Puppet
 #                end
 
                 if property = @resource.property(:enable)
-                    property.retrieve
-                    unless property.insync?
+                    val = property.retrieve
+                    unless property.insync?(val)
                         property.sync
                     end
                 end
diff --git a/test/language/interpreter.rb b/test/language/interpreter.rb
index 7fe4eacac..fd1847df9 100755
--- a/test/language/interpreter.rb
+++ b/test/language/interpreter.rb
@@ -193,7 +193,7 @@ class TestInterpreter < Test::Unit::TestCase
         assert(culain, "Did not find culain in ldap")
 
         assert_nothing_raised do
-            assert_equal(basenode.classname.to_s, culain.parentclass.classname.to_s,
+            assert_equal(basenode.classname.to_s, culain.parentobj.classname.to_s,
                 "Did not get parent class")
         end
     end
diff --git a/test/lib/puppettest/fakes.rb b/test/lib/puppettest/fakes.rb
index 3c19d6c43..df1c3732e 100644
--- a/test/lib/puppettest/fakes.rb
+++ b/test/lib/puppettest/fakes.rb
@@ -114,6 +114,10 @@ module PuppetTest
             end
         end
 
+        def self.source
+            self.name
+        end
+
         def self.supports_parameter?(param)
             true
         end
@@ -129,6 +133,13 @@ module PuppetTest
         def initialize(resource)
             @resource = resource
         end
+
+        def properties
+            self.class.resource_type.validproperties.inject({}) do |props, name|
+                props[name] = self.send(name) || :absent
+                props
+            end
+        end
     end
 
     class FakeParsedProvider < FakeProvider
diff --git a/test/ral/providers/mount/parsed.rb b/test/ral/providers/mount/parsed.rb
index 37aa6f157..38d4a6675 100755
--- a/test/ral/providers/mount/parsed.rb
+++ b/test/ral/providers/mount/parsed.rb
@@ -133,12 +133,13 @@ class TestParsedMounts < Test::Unit::TestCase
 
             assert(FileTest.exists?(@provider.default_target),
                 "FSTab %s does not exist" % @provider.default_target)
+
             assert_nothing_raised do
-                @provider.prefetch
+                @provider.prefetch("/" => type)
             end
 
-            assert_equal(:present, provider.property_hash[:ensure],
-                "Could not find root fs with provider %s" % provider.name)
+            assert_equal(:present, type.provider.property_hash[:ensure],
+                "Could not find root fs with provider %s" % provider.class.name)
 
             assert_nothing_raised {
                 assert(provider.mounted?, "Root is considered not mounted")
diff --git a/test/ral/providers/package/apt.rb b/test/ral/providers/package/apt.rb
index 5197880ff..a56233c0b 100755
--- a/test/ral/providers/package/apt.rb
+++ b/test/ral/providers/package/apt.rb
@@ -28,7 +28,7 @@ class AptPackageProviderTest < PuppetTest::TestCase
 							  'faff'
 					  ).returns(
 					        "deinstall ok config-files faff 1.2.3-1\n"
-					  ).times(2)
+					  ).times(1)
 
 		pkg.provider.expects(
 		                 :aptget
@@ -56,7 +56,7 @@ class AptPackageProviderTest < PuppetTest::TestCase
 					        'faff'
 					  ).returns(
 					        "install ok installed faff 1.2.3-1\n"
-					  ).times(2)
+					  ).times(1)
 		pkg.provider.expects(
 		                 :aptget
 					  ).with(
@@ -124,7 +124,7 @@ class AptPackageProviderTest < PuppetTest::TestCase
 					        'faff'
 					  ).returns(
 					        "deinstall ok config-files faff 1.2.3-1\n"
-					  ).times(2)
+					  ).times(1)
 		pkg.provider.expects(
 		                 :aptget
 					  ).with(
diff --git a/test/ral/providers/package/aptitude.rb b/test/ral/providers/package/aptitude.rb
index 891603f64..f8edcebde 100755
--- a/test/ral/providers/package/aptitude.rb
+++ b/test/ral/providers/package/aptitude.rb
@@ -28,7 +28,7 @@ class AptitudePackageProviderTest < PuppetTest::TestCase
  							  'faff'
  					  ).returns(
  					        "deinstall ok config-files faff 1.2.3-1\n"
- 					  ).times(2)
+ 					  ).times(1)
 
 		pkg.provider.expects(
 		                 :aptitude
@@ -58,7 +58,7 @@ class AptitudePackageProviderTest < PuppetTest::TestCase
 					        'faff'
 					  ).returns(
 					        "install ok installed faff 1.2.3-1\n"
-					  ).times(2)
+					  ).times(1)
 		pkg.provider.expects(
 		                 :aptitude
 					  ).with(
diff --git a/test/ral/providers/package/aptrpm.rb b/test/ral/providers/package/aptrpm.rb
index 0388135df..a3257bcce 100755
--- a/test/ral/providers/package/aptrpm.rb
+++ b/test/ral/providers/package/aptrpm.rb
@@ -28,7 +28,7 @@ class AptrpmPackageProviderTest < PuppetTest::TestCase
                             '--nodigest',
                             '--qf',
                             "%{NAME}-%{VERSION}-%{RELEASE} %{VERSION}-%{RELEASE}\n"
-                      ).raises(Puppet::ExecutionFailure, "couldn't find rpm").times(2)
+                      ).raises(Puppet::ExecutionFailure, "couldn't find rpm").times(1)
 
         pkg.provider.expects(
                          :aptget
@@ -56,7 +56,7 @@ class AptrpmPackageProviderTest < PuppetTest::TestCase
                         "%{NAME}-%{VERSION}-%{RELEASE} %{VERSION}-%{RELEASE}\n"
                     ).returns(
                         "faff-1.2.3-1 1.2.3-1\n"
-                    ).times(2)
+                    ).times(1)
         pkg.provider.expects(
                         :aptget
                     ).with(
diff --git a/test/ral/providers/package/dpkg.rb b/test/ral/providers/package/dpkg.rb
index 69b6a3c10..7e4427584 100755
--- a/test/ral/providers/package/dpkg.rb
+++ b/test/ral/providers/package/dpkg.rb
@@ -27,7 +27,7 @@ class DpkgPackageProviderTest < PuppetTest::TestCase
 							  'faff'
 					  ).returns(
 					        "deinstall ok config-files faff 1.2.3-1\n"
-					  ).times(2)
+					  ).times(1)
 
 		pkg.provider.expects(
 		                 :dpkg
@@ -51,7 +51,7 @@ class DpkgPackageProviderTest < PuppetTest::TestCase
 					        'faff'
 					  ).returns(
 					        "install ok installed faff 1.2.3-1\n"
-					  ).times(2)
+					  ).times(1)
 		pkg.provider.expects(
 		                 :dpkg
 					  ).with(
diff --git a/test/ral/providers/service.rb b/test/ral/providers/service.rb
index 3e1541be1..d21298162 100755
--- a/test/ral/providers/service.rb
+++ b/test/ral/providers/service.rb
@@ -55,7 +55,7 @@ class TestLocalService < Test::Unit::TestCase
         }
 
         comp = newcomp("servicetst", service)
-        service[:ensure] = true
+        service[:ensure] = :running
 
         Puppet.info "Starting %s" % service.name
         assert_apply(service)
@@ -63,10 +63,15 @@ class TestLocalService < Test::Unit::TestCase
         # Some package systems background the work, so we need to give them
         # time to do their work.
         sleep(1.5)
+        props = nil
         assert_nothing_raised() {
-            service.retrieve
+            props = service.retrieve
         }
-        assert(service.insync?, "Service %s is not running" % service.name)
+        props.each do |prop, value|
+            if prop.name == :ensure
+                assert_equal(:running, value, "Service %s is not running" % service.name)
+            end
+        end
 
         # test refreshing it
         assert_nothing_raised() {
@@ -77,17 +82,22 @@ class TestLocalService < Test::Unit::TestCase
         assert_nothing_raised() {
             service[:ensure] = :stopped
         }
-        assert_nothing_raised() {
-            service.retrieve
-        }
-        assert(!service.insync?(), "Service %s is not running" % service.name)
+        props.each do |prop, value|
+            if prop.name == :ensure
+                assert_equal(:running, value, "Service %s is not running" % service.name)
+            end
+        end
         Puppet.info "stopping %s" % service.name
         assert_events([:service_stopped], comp)
         sleep(1.5)
         assert_nothing_raised() {
-            service.retrieve
+            props = service.retrieve
         }
-        assert(service.insync?, "Service %s has not stopped" % service.name)
+        props.each do |prop, value|
+            if prop.name == :ensure
+                assert_equal(:stopped, value, "Service %s is not running" % service.name)
+            end
+        end
     end
 
     def cycleenable(service)
@@ -104,26 +114,35 @@ class TestLocalService < Test::Unit::TestCase
         # Some package systems background the work, so we need to give them
         # time to do their work.
         sleep(1.5)
+        props = nil
         assert_nothing_raised() {
-            service.retrieve
+            props = service.retrieve
         }
-        assert(service.insync?, "Service %s is not enabled" % service.name)
+        props.each do |prop, value|
+            if prop.name == :enable
+                assert_equal(value, :true, "Service %s is not enabled" % service.name)
+            end
+        end
 
-        # now stop it
+        # now disable it
         assert_nothing_raised() {
             service[:enable] = false
         }
         assert_nothing_raised() {
-            service.retrieve
+            props = service.retrieve
         }
-        assert(!service.insync?(), "Service %s is not enabled" % service.name)
+        props.each do |prop, value|
+            assert_equal(value, :true, "Service %s is already disabled" % service.name)
+        end
         Puppet.info "disabling %s" % service.name
         assert_events([:service_disabled], comp)
         sleep(1.5)
         assert_nothing_raised() {
-            service.retrieve
+            props = service.retrieve
         }
-        assert(service.insync?, "Service %s has not been disabled" % service.name)
+        props.each do |prop, value|
+            assert_equal(value, :false, "Service %s is still enabled" % service.name)
+        end
     end
 
     def test_status
@@ -174,30 +193,37 @@ class TestLocalService < Test::Unit::TestCase
                 startenable = nil
                 startensure = nil
                 svc[:check] = [:ensure, :enable]
-                svc.retrieve
+                properties = nil
                 assert_nothing_raised("Could not get status") {
-                    startenable = svc.property(:enable).is
-                    startensure = svc.property(:ensure).is
+                    properties = svc.retrieve
                 }
+                initial = properties.dup
 
                 svc[:enable] = false
                 svc[:ensure] = :stopped
                 assert_apply(svc)
 
                 sleep 1
-                svc.retrieve
-                assert(svc.insync?, "Service did not sync both properties")
+                assert_nothing_raised("Could not get status") {
+                    properties = svc.retrieve
+                }
+                properties.each do |prop, value|
+                    assert(prop.insync?(value), "Service did not sync %s property" % prop.name)
+                end
 
                 svc[:enable] = true
                 svc[:ensure] = :running
                 assert_apply(svc)
 
                 sleep 1
-                svc.retrieve
-                assert(svc.insync?, "Service did not sync both properties")
+                assert_nothing_raised("Could not get status") {
+                    properties = svc.retrieve
+                }
+                assert(svc.insync?(properties), "Service did not sync both properties")
 
-                svc[:enable] = startenable
-                svc[:ensure] = startensure
+                initial.each do |prop, value|
+                    svc[prop.name] = value
+                end
                 assert_apply(svc)
                 Puppet.type(:component).clear
             end
diff --git a/test/ral/providers/user.rb b/test/ral/providers/user.rb
index dfc9b6136..7e5290549 100755
--- a/test/ral/providers/user.rb
+++ b/test/ral/providers/user.rb
@@ -164,13 +164,12 @@ class TestUserProvider < Test::Unit::TestCase
 
         # Now try it by object
         assert_nothing_raised {
-            names = @provider.list
+            names = @provider.instances
         }
         assert(names.length > 0, "Listed no users as objects")
 
         names.each do |obj|
-            assert_instance_of(Puppet::Type.type(:user), obj)
-            assert(obj[:provider], "Provider was not set")
+            assert_instance_of(@provider, obj)
         end
     end
 
@@ -186,7 +185,7 @@ class TestUserProvider < Test::Unit::TestCase
         Puppet::Type.type(:user).validproperties.each do |property|
             next if property == :ensure
             # This is mostly in place for the 'password' stuff.
-            next unless user.class.supports_parameter?(property)
+            next unless user.class.supports_parameter?(property) and Puppet.features.root?
             val = nil
             assert_nothing_raised {
                 val = user.send(property)
diff --git a/test/ral/types/mount.rb b/test/ral/types/mount.rb
index 4b76b3ec2..24c353692 100755
--- a/test/ral/types/mount.rb
+++ b/test/ral/types/mount.rb
@@ -18,10 +18,14 @@ class TestMounts < Test::Unit::TestCase
             :yayness
         end
 
+        def self.instances
+            []
+        end
+
         def create
             @ensure = :present
-            @model.class.validproperties.each do |property|
-                if value = @model.should(property)
+            @resource.class.validproperties.each do |property|
+                if value = @resource.should(property)
                     self.send(property.to_s + "=", value)
                 end
             end
@@ -188,19 +192,19 @@ class TestMounts < Test::Unit::TestCase
     
     # Darwin doesn't put its mount table into netinfo
     unless Facter.value(:operatingsystem) == "Darwin"
-    def test_list
-        list = nil
-        assert(@mount.respond_to?(:list),
-            "No list method defined for mount")
+    def test_instances
+        instances = nil
+        assert(@mount.respond_to?(:instances),
+            "No instances method defined for mount")
 
         assert_nothing_raised do
-            list = Puppet::Type.type(:mount).list
+            instances = Puppet::Type.type(:mount).instances
         end
         
-        assert(list.length > 0, "Did not return any mounts")
+        assert(instances.length > 0, "Did not return any mounts")
 
-        root = list.find { |o| o[:name] == "/" }
-        assert(root, "Could not find root root filesystem in list results")
+        root = instances.find { |o| o[:name] == "/" }
+        assert(root, "Could not find root root filesystem in instances results")
   
         current_values = nil
         assert_nothing_raised do
@@ -245,8 +249,8 @@ class TestMounts < Test::Unit::TestCase
         assert_events([], mount)
 
         # Now try listing and making sure the object is actually gone.
-        list = mount.provider.class.list
-        assert(! list.find { |r| r[:name] == mount[:name] },
+        instances = mount.provider.class.instances
+        assert(! instances.find { |r| r[:name] == mount[:name] },
             "Mount was not actually removed")
     end
     end
diff --git a/test/ral/types/package.rb b/test/ral/types/package.rb
index e37cdfe06..6e0ddc035 100755
--- a/test/ral/types/package.rb
+++ b/test/ral/types/package.rb
@@ -4,6 +4,7 @@ $:.unshift("../../lib") if __FILE__ =~ /\.rb$/
 
 require 'puppettest'
 require 'facter'
+require 'mocha'
 
 $platform = Facter["operatingsystem"].value
 
@@ -81,6 +82,61 @@ class TestPackages < Test::Unit::TestCase
                 "Incorrect default package format")
         end
     end
+
+    # Make sure we can prefetch and retrieve packages
+    def test_package_instances
+        providers = []
+        @type.instances.each do |resource|
+            # Just do one of each type
+            next if providers.include?(resource.provider.class)
+            providers << resource.provider.class
+
+            # We should have data on the resource
+            assert(resource.exists?, "Listed resource thinks it's absent")
+
+            # Now flush the resource and make sure it clears the property_hash
+            assert_nothing_raised("Could not flush package") do
+                resource.flush
+            end
+
+            assert_equal(:absent, resource.provider.get(:ensure), "Flushing did not empty property hash")
+
+            # And query anew
+            props = nil
+            assert_nothing_raised("Could not retrieve package again") do
+                props = resource.retrieve
+            end
+            provider_props = resource.provider.send(:instance_variable_get, "@property_hash")
+            props.each do |prop, value|
+                assert_equal(value, provider_props[prop.name], "Query did not return same result as the property_hash for %s" % prop.name)
+            end
+        end
+    end
+
+    # Make sure we can prefetch package information, rather than getting it one package at a time.
+    def test_prefetch
+        @type.providers_by_source.each do |provider|
+            # First get a list of packages
+            list = provider.instances
+
+            packages = {}
+            list.each do |package|
+                packages[package.name] = @type.create(:name => package.name, :ensure => :installed)
+                break if packages.length > 4
+            end
+
+            # Now prefetch using that list of packages
+            assert_nothing_raised("Could not prefetch with %s" % provider.name) do
+                provider.prefetch(packages)
+            end
+
+            # And make sure each package is marked as existing, without calling query
+            packages.each do |name, package|
+                assert(package.exists?, "Package of type %s not marked present" % provider.name)
+                package.provider.expects(:query).never
+            end
+        end
+    end
 end
 
 # $Id$