Convert node map to last-writer-wins for ties

We still bind preferentially by specificity, but for ties with
specificity we now prefer last-writer-wins instead of the alphabetic
by class name method we had before (that was never documented and I'm
pretty certain nobody understood).

So now `provides(:whatever) { true }` in a cookbook should always win
over core.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

6 files changed, 19 insertions, 36 deletions

diff --git a/lib/chef/node_map.rb b/lib/chef/node_map.rb
index dde93a437d..ecd5c9df8f 100644
--- a/lib/chef/node_map.rb
+++ b/lib/chef/node_map.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Lamont Granquist (<lamont@chef.io>)
-# Copyright:: Copyright 2014-2017, Chef Software Inc.
+# Copyright:: Copyright 2014-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -192,8 +192,10 @@ class Chef
       !!canonical == !!matcher[:canonical]
     end
 
-    # @api private
-    def dispatch_compare_matchers(key, new_matcher, matcher)
+    #
+    # "provides" lines with identical filters sort by class name (ascending).
+    #
+    def compare_matchers(key, new_matcher, matcher)
       cmp = compare_matcher_properties(new_matcher[:block], matcher[:block])
       return cmp if cmp != 0
       cmp = compare_matcher_properties(new_matcher[:platform_version], matcher[:platform_version])
@@ -210,26 +212,6 @@ class Chef
       0
     end
 
-    #
-    # "provides" lines with identical filters sort by class name (ascending).
-    #
-    def compare_matchers(key, new_matcher, matcher)
-      cmp = dispatch_compare_matchers(key, new_matcher, matcher)
-      if cmp == 0
-        # Sort by class name (ascending) as well, if all other properties
-        # are exactly equal
-        # XXX: remove this in Chef-14 and use last-writer-wins (prepend if they match)
-        if !new_matcher[:override]
-          # we only sort classes, which only sorts the handler array, this magically does not sort
-          # the priority array via the invisible else here.
-          if new_matcher[:klass].is_a?(Class)
-            cmp = compare_matcher_properties(new_matcher[:klass].name, matcher[:klass].name)
-          end
-        end
-      end
-      cmp
-    end
-
     def compare_matcher_properties(a, b)
       # falsity comparisons here handle both "nil" and "false"
       return 1 if !a && b
diff --git a/lib/chef/resource/file.rb b/lib/chef/resource/file.rb
index 43d9c57150..787992fc24 100644
--- a/lib/chef/resource/file.rb
+++ b/lib/chef/resource/file.rb
@@ -1,7 +1,7 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
 # Author:: Seth Chisamore (<schisamo@chef.io>)
-# Copyright:: Copyright 2008-2016 Chef Software, Inc.
+# Copyright:: Copyright 2008-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
diff --git a/lib/chef/resource/link.rb b/lib/chef/resource/link.rb
index 7a106b1703..d51d42b506 100644
--- a/lib/chef/resource/link.rb
+++ b/lib/chef/resource/link.rb
@@ -1,7 +1,7 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
 # Author:: Tyler Cloke (<tyler@chef.io>)
-# Copyright:: Copyright 2008-2016, Chef Software Inc.
+# Copyright:: Copyright 2008-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -34,6 +34,7 @@ class Chef
     # original file.
     class Link < Chef::Resource
       include Chef::Mixin::Securable
+      resource_name :link
 
       identity_attr :target_file
 
diff --git a/spec/unit/data_collector/resource_report_spec.rb b/spec/unit/data_collector/resource_report_spec.rb
index b3523622c4..278e94b53b 100644
--- a/spec/unit/data_collector/resource_report_spec.rb
+++ b/spec/unit/data_collector/resource_report_spec.rb
@@ -1,7 +1,7 @@
 #
 # Author:: Salim Afiune (<afiune@chef.io)
 #
-# Copyright:: Copyright 2012-2017, Chef Software Inc.
+# Copyright:: Copyright 2012-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -110,7 +110,7 @@ describe Chef::DataCollector::ResourceReport do
     context "for a lazy_resource that got skipped" do
       let(:resource) do
         klass = Class.new(Chef::Resource) do
-          resource_name "link"
+          resource_name "butters"
           property :sword, String, name_property: true, identity: true
         end
         resource = klass.new("hyrule")
@@ -129,7 +129,7 @@ describe Chef::DataCollector::ResourceReport do
           "name" => "hyrule",
           "result" => "create",
           "status" => "skipped",
-          "type" => :link,
+          "type" => :butters,
         }
       end
       let(:conditional) do
diff --git a/spec/unit/node_map_spec.rb b/spec/unit/node_map_spec.rb
index 7fa115b532..67bb741ec5 100644
--- a/spec/unit/node_map_spec.rb
+++ b/spec/unit/node_map_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Lamont Granquist (<lamont@chef.io>)
-# Copyright:: Copyright 2014-2017, Chef Software Inc.
+# Copyright:: Copyright 2014-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -122,16 +122,16 @@ describe Chef::NodeMap do
   describe "ordering classes" do
     class Foo; end
     class Bar; end
-    it "orders them alphabetically when they're set in the reverse order" do
+    it "last writer wins when its reverse alphabetic order" do
       node_map.set(:thing, Foo)
       node_map.set(:thing, Bar)
       expect(node_map.get(node, :thing)).to eql(Bar)
     end
 
-    it "orders them alphabetically when they're set in alphabetic order" do
+    it "last writer wins when its alphabetic order" do
       node_map.set(:thing, Bar)
       node_map.set(:thing, Foo)
-      expect(node_map.get(node, :thing)).to eql(Bar)
+      expect(node_map.get(node, :thing)).to eql(Foo)
     end
   end
 
diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb
index 7a538b721b..93d03756a7 100644
--- a/spec/unit/recipe_spec.rb
+++ b/spec/unit/recipe_spec.rb
@@ -3,7 +3,7 @@
 # Author:: Christopher Walters (<cw@chef.io>)
 # Author:: Tim Hinderliter (<tim@chef.io>)
 # Author:: Seth Chisamore (<schisamo@chef.io>)
-# Copyright:: Copyright 2008-2017, Chef Software Inc.
+# Copyright:: Copyright 2008-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -141,16 +141,16 @@ describe Chef::Recipe do
             Object.send(:remove_const, :TottenhamHotspur)
           end
 
-          it "selects the first one alphabetically" do
+          it "selects the last-writer wins" do
             Sounders.provides :football, platform: "nbc_sports"
             TottenhamHotspur.provides :football, platform: "nbc_sports"
 
             res1 = recipe.football "club world cup"
             expect(res1.name).to eql("club world cup")
-            expect(res1).to be_a_kind_of(Sounders)
+            expect(res1).to be_a_kind_of(TottenhamHotspur)
           end
 
-          it "selects the first one alphabetically even if the declaration order is reversed" do
+          it "selects the last-writer wins even if the declaration order is reversed" do
             TottenhamHotspur.provides :football2, platform: "nbc_sports"
             Sounders.provides :football2, platform: "nbc_sports"