Closing #585 -- providers can now have optional commands, which only differ from normal commands in that they do not affect a provider's suitability

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

CHANGELOG

@@ -1,3 +1,8 @@
+    Providers can now specify that some commands are optional (#585).
+    Also, the 'command' method returns nil on missing commands,
+    rather than throwing an error, so the presence of commands
+    be tested.
+
     The 'useradd' provider for Users can now manage passwords.
     No other providers can, at this point.
 

lib/puppet/provider.rb

@@ -28,48 +28,17 @@ class Puppet::Provider
                 [name, self.name]
         end
 
-        unless command =~ /^\//
-            raise Puppet::Error, "Command #{command} could not be found"
+        if command == :missing
+            return nil
         end
 
         command
     end
 
-    # Define one or more binaries we'll be using
+    # Define commands that are not optional.
     def self.commands(hash)
-        hash.each do |name, path|
-            name = symbolize(name)
-            @origcommands[name] = path
-            # Keep the short name if we couldn't find it.
-            unless path =~ /^\//
-                if tmp = binary(path)
-                    path = tmp
-                end
-            end
-            @commands[name] = path
+        optional_commands(hash) do |name, path|
             confine :exists => path
-
-            # Now define a method for that command
-            unless metaclass.method_defined? name
-                meta_def(name) do |*args|
-                    if args.empty?
-                        cmd = [command(name)]
-                    else
-                        cmd = [command(name)] + args
-                    end
-                    # This might throw an ExecutionFailure, but the system above
-                    # will catch it, if so.
-                    return execute(cmd)
-                end
-                
-                # And then define an instance method that just calls the class method.
-                # We need both, so both instances and classes can easily run the commands.
-                unless method_defined? name
-                    define_method(name) do |*args|
-                        self.class.send(name, *args)
-                    end
-                end
-            end
         end
     end
 
@@ -128,6 +97,34 @@ class Puppet::Provider
         end
     end
 
+    # Create the methods for a given command.
+    def self.make_command_methods(name)
+        # Now define a method for that command
+        unless metaclass.method_defined? name
+            meta_def(name) do |*args|
+                unless command(name)
+                    raise Puppet::Error, "Command %s is missing" % name
+                end
+                if args.empty?
+                    cmd = [command(name)]
+                else
+                    cmd = [command(name)] + args
+                end
+                # This might throw an ExecutionFailure, but the system above
+                # will catch it, if so.
+                return execute(cmd)
+            end
+            
+            # And then define an instance method that just calls the class method.
+            # We need both, so both instances and classes can easily run the commands.
+            unless method_defined? name
+                define_method(name) do |*args|
+                    self.class.send(name, *args)
+                end
+            end
+        end
+    end
+
     # Create getter/setter methods for each property our model supports.
     # They all get stored in @property_hash.  This method is useful
     # for those providers that use prefetch and flush.
@@ -156,6 +153,30 @@ class Puppet::Provider
 
     self.initvars
 
+    # Define one or more binaries we'll be using.  If a block is passed, yield the name
+    # and path to the block (really only used by 'commands').
+    def self.optional_commands(hash)
+        hash.each do |name, path|
+            name = symbolize(name)
+            @origcommands[name] = path
+
+            # Try to find the full path (or verify already-full paths); otherwise
+            # store that the command is missing so we know it's defined but absent.
+            if tmp = binary(path)
+                @commands[name] = path
+            else
+                @commands[name] = :missing
+            end
+
+            if block_given?
+                yield(name, path)
+            end
+
+            # Now define the class and instance methods.
+            make_command_methods(name)
+        end
+    end
+
     # Check whether this implementation is suitable for our platform.
     def self.suitable?
         # A single false result is sufficient to turn the whole thing down.

test/ral/manager/provider.rb

@@ -15,6 +15,8 @@ class TestTypeProviders < Test::Unit::TestCase
             newparam(:name) do end
         end
 
+        cleanup { Puppet::Type.rmtype(:defaultprovidertest) }
+
         basic = type.provide(:basic) do
             defaultfor :operatingsystem => :somethingelse,
                 :operatingsystemrelease => :yayness
@@ -47,6 +49,33 @@ class TestTypeProviders < Test::Unit::TestCase
         assert_equal(should, type.allattrs.reject { |p| ! should.include?(p) },
             "Providify did not reorder parameters")
     end
+
+    # Make sure optional commands get looked up but don't affect suitability.
+    def test_optional_commands
+        type = Puppet::Type.newtype(:optional_commands) {}
+
+        cleanup { Puppet::Type.rmtype(:optional_commands) }
+
+        # Define a provider with mandatory commands
+        required = type.provide(:required) {
+            commands :missing => "/no/such/binary/definitely"
+        }
+
+        # And another with optional commands
+        optional = type.provide(:optional) {
+            optional_commands :missing => "/no/such/binary/definitely"
+        }
+
+        assert(! required.suitable?, "Provider with missing commands considered suitable")
+        assert_nil(required.command(:missing), "Provider returned non-nil from missing command")
+
+        assert(optional.suitable?, "Provider with optional commands considered unsuitable")
+        assert_nil(optional.command(:missing), "Provider returned non-nil from missing command")
+
+        assert_raise(Puppet::Error, "Provider did not fail when missing command was called") do
+            optional.missing
+        end
+    end
 end
 
 class TestProviderFeatures < Test::Unit::TestCase