From 935c46351f6f6730569c50fd0c59e8157486b827 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Wed, 15 Jul 2009 16:03:44 -0700
Subject: Fixing #2403 - provider specificity is richer and better

We have extended the concept of provider specificity so it
now includes both specified defaults and class depth, so:

* We are much more likely to choose the correct provider;
  e.g., 'init' will be chosen over 'base'

* We're much less likely to print this warning, because it's
  only printed when provider specificities are equal which
  is much rarer

lib/puppet/provider.rb   |    4 ++--
lib/puppet/type.rb       |    4 ++--
spec/unit/type.rb        |   17 +++++++++++++++++
test/ral/manager/type.rb |   22 ----------------------
4 files changed, 21 insertions(+), 26 deletions(-)

Signed-off-by: Luke Kanies <luke@madstop.com>
---
 lib/puppet/provider.rb   |  4 ++--
 lib/puppet/type.rb       |  4 ++--
 spec/unit/provider.rb    | 31 +++++++++++++++++++++++++++++++
 spec/unit/type.rb        | 17 +++++++++++++++++
 test/ral/manager/type.rb | 22 ----------------------
 5 files changed, 52 insertions(+), 26 deletions(-)
 create mode 100755 spec/unit/provider.rb

diff --git a/lib/puppet/provider.rb b/lib/puppet/provider.rb
index 1f13538af..3cef0dd71 100644
--- a/lib/puppet/provider.rb
+++ b/lib/puppet/provider.rb
@@ -91,8 +91,8 @@ class Puppet::Provider
         end
     end
 
-    def self.defaultnum
-        @defaults.length
+    def self.specificity
+        (@defaults.length * 100) + ancestors.select { |a| a.is_a? Class }.length
     end
 
     def self.initvars
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 91f991814..098d83254 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -1465,8 +1465,8 @@ class Type
 
             # If we don't have any default we use suitable providers
             defaults = suitable if defaults.empty?
-            max = defaults.collect { |provider| provider.defaultnum }.max
-            defaults = defaults.find_all { |provider| provider.defaultnum == max }
+            max = defaults.collect { |provider| provider.specificity }.max
+            defaults = defaults.find_all { |provider| provider.specificity == max }
 
             retval = nil
             if defaults.length > 1
diff --git a/spec/unit/provider.rb b/spec/unit/provider.rb
new file mode 100755
index 000000000..9d781540b
--- /dev/null
+++ b/spec/unit/provider.rb
@@ -0,0 +1,31 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../spec_helper'
+
+describe Puppet::Provider do
+    it "should have a specifity class method" do
+        Puppet::Provider.should respond_to(:specificity)
+    end
+
+    it "should consider two defaults to be higher specificity than one default" do
+        one = Class.new(Puppet::Provider)
+        one.initvars
+        one.defaultfor :operatingsystem => "solaris"
+
+        two = Class.new(Puppet::Provider)
+        two.initvars
+        two.defaultfor :operatingsystem => "solaris", :operatingsystemrelease => "5.10"
+
+        two.specificity.should > one.specificity
+    end
+
+    it "should consider a subclass more specific than its parent class" do
+        one = Class.new(Puppet::Provider)
+        one.initvars
+
+        two = Class.new(one)
+        two.initvars
+
+        two.specificity.should > one.specificity
+    end
+end
diff --git a/spec/unit/type.rb b/spec/unit/type.rb
index a55009dfc..2bcac7aec 100755
--- a/spec/unit/type.rb
+++ b/spec/unit/type.rb
@@ -72,6 +72,23 @@ describe Puppet::Type do
         Puppet::Type.type(:mount).new(:name => "foo").should respond_to(:exported?)
     end
 
+    describe "when choosing a default provider" do
+        it "should choose the provider with the highest specificity" do
+            # Make a fake type
+            type = Puppet::Type.newtype(:defaultprovidertest) do
+                newparam(:name) do end
+            end
+
+            basic = type.provide(:basic) {}
+            greater = type.provide(:greater) {}
+
+            basic.stubs(:specificity).returns 1
+            greater.stubs(:specificity).returns 2
+
+            type.defaultprovider.should equal(greater)
+        end
+    end
+
     describe "when initializing" do
         describe "and passed a TransObject" do
             it "should fail" do
diff --git a/test/ral/manager/type.rb b/test/ral/manager/type.rb
index 2624e26ea..30da6d114 100755
--- a/test/ral/manager/type.rb
+++ b/test/ral/manager/type.rb
@@ -307,28 +307,6 @@ class TestType < Test::Unit::TestCase
         assert_equal(path, file[:name], "Did not get correct name")
     end
 
-    # Make sure default providers behave correctly
-    def test_defaultproviders
-        # Make a fake type
-        type = Puppet::Type.newtype(:defaultprovidertest) do
-            newparam(:name) do end
-        end
-
-        basic = type.provide(:basic) do
-            defaultfor :operatingsystem => :somethingelse,
-                :operatingsystemrelease => :yayness
-        end
-
-        assert_equal(basic, type.defaultprovider)
-        type.defaultprovider = nil
-
-        greater = type.provide(:greater) do
-            defaultfor :operatingsystem => Facter.value("operatingsystem")
-        end
-
-        assert_equal(greater, type.defaultprovider)
-    end
-
     # Make sure that we can have multiple non-isomorphic objects with the same name,
     # but not with isomorphic objects.
     def test_isomorphic_names
-- 
cgit