Merge pull request #6765 from chef/lcg/node-map-last-writer-wins

Convert node map to last-writer-wins for ties

7 files changed, 45 insertions, 62 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/integration/recipes/recipe_dsl_spec.rb b/spec/integration/recipes/recipe_dsl_spec.rb
index f9970afc92..0250786f0e 100644
--- a/spec/integration/recipes/recipe_dsl_spec.rb
+++ b/spec/integration/recipes/recipe_dsl_spec.rb
@@ -446,11 +446,11 @@ describe "Recipe DSL methods" do
 
           end
 
-          it "thingy3 works in a recipe and yields Thingy3 (the alphabetical one)" do
+          it "thingy3 works in a recipe and yields Thingy4 (the last one)" do
             recipe = converge do
               thingy3("blah") {}
             end
-            expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy3
+            expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy4
           end
 
           it "thingy4 does not work in a recipe" do
@@ -460,7 +460,7 @@ describe "Recipe DSL methods" do
           end
 
           it "resource_matching_short_name returns Thingy4" do
-            expect(Chef::Resource.resource_matching_short_name(:thingy3)).to eq RecipeDSLSpecNamespace::Thingy3
+            expect(Chef::Resource.resource_matching_short_name(:thingy3)).to eq RecipeDSLSpecNamespace::Thingy4
           end
         end
       end
@@ -501,14 +501,14 @@ describe "Recipe DSL methods" do
             expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy6
           end
 
-          it "thingy5 works in a recipe and yields Foo::Thingy5 (the alphabetical one)" do
+          it "thingy5 works in a recipe and yields Foo::Thingy6 (the last one)" do
             recipe = converge do
               thingy5("blah") {}
             end
-            expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy5
+            expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy6
           end
 
-          it "resource_matching_short_name returns Thingy5" do
+          it "resource_matching_short_name returns Thingy6" do
             expect(Chef::Resource.resource_matching_short_name(:thingy5)).to eq RecipeDSLSpecNamespace::Thingy5
           end
 
@@ -540,11 +540,11 @@ describe "Recipe DSL methods" do
 
             end
 
-            it "thingy5_2 works in a recipe and yields the RecipeDSLSpaceNamespace one (the alphabetical one)" do
+            it "thingy5_2 works in a recipe and yields the ZRecipeDSLSpaceNamespace one (the last one)" do
               recipe = converge do
                 thingy5_2("blah") {}
               end
-              expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy5
+              expect(BaseThingy.created_resource).to eq ZRecipeDSLSpecNamespace::Thingy5
             end
           end
 
@@ -593,11 +593,11 @@ describe "Recipe DSL methods" do
 
             end
 
-            it "thingy3 works in a recipe and yields Thingy3 (the alphabetical one)" do
+            it "thingy3 works in a recipe and yields Thingy4 (the last one)" do
               recipe = converge do
                 thingy3("blah") {}
               end
-              expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy3
+              expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy4
             end
 
             it "thingy4 does not work in a recipe" do
@@ -607,7 +607,7 @@ describe "Recipe DSL methods" do
             end
 
             it "resource_matching_short_name returns Thingy4" do
-              expect(Chef::Resource.resource_matching_short_name(:thingy3)).to eq RecipeDSLSpecNamespace::Thingy3
+              expect(Chef::Resource.resource_matching_short_name(:thingy3)).to eq RecipeDSLSpecNamespace::Thingy4
             end
           end
 
@@ -620,11 +620,11 @@ describe "Recipe DSL methods" do
 
             end
 
-            it "thingy3 works in a recipe and yields Thingy3 (the alphabetical one)" do
+            it "thingy3 works in a recipe and yields Thingy4 (the last one)" do
               recipe = converge do
                 thingy3("blah") {}
               end
-              expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy3
+              expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy4
             end
 
             it "thingy4 does not work in a recipe" do
@@ -634,7 +634,7 @@ describe "Recipe DSL methods" do
             end
 
             it "resource_matching_short_name returns Thingy4" do
-              expect(Chef::Resource.resource_matching_short_name(:thingy3)).to eq RecipeDSLSpecNamespace::Thingy3
+              expect(Chef::Resource.resource_matching_short_name(:thingy3)).to eq RecipeDSLSpecNamespace::Thingy4
             end
           end
         end
@@ -667,11 +667,11 @@ describe "Recipe DSL methods" do
             expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy7
           end
 
-          it "thingy8 works in a recipe and yields Thingy7 (alphabetical)" do
+          it "thingy8 works in a recipe and yields Thingy7 (last)" do
             recipe = converge do
               thingy8("blah") {}
             end
-            expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy7
+            expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy8
           end
 
           it "resource_matching_short_name returns Thingy8" do
@@ -853,17 +853,17 @@ describe "Recipe DSL methods" do
         end
         before { resource_class_z } # pull on it so it gets defined before the recipe runs
 
-        it "two_classes_one_dsl resolves to B (alphabetically earliest)" do
+        it "two_classes_one_dsl resolves to Z (last)" do
           temp_two_classes_one_dsl = two_classes_one_dsl
           recipe = converge do
             instance_eval("#{temp_two_classes_one_dsl} 'blah'")
           end
           expect(recipe.logged_warnings).to eq ""
-          expect(BaseThingy.created_resource).to eq resource_class
+          expect(BaseThingy.created_resource).to eq resource_class_z
         end
 
-        it "resource_matching_short_name returns B" do
-          expect(Chef::Resource.resource_matching_short_name(two_classes_one_dsl)).to eq resource_class
+        it "resource_matching_short_name returns Z" do
+          expect(Chef::Resource.resource_matching_short_name(two_classes_one_dsl)).to eq resource_class_z
         end
 
         context "and a priority array [ Z, B ]" do
@@ -880,8 +880,8 @@ describe "Recipe DSL methods" do
             expect(BaseThingy.created_resource).to eq resource_class_z
           end
 
-          it "resource_matching_short_name returns B" do
-            expect(Chef::Resource.resource_matching_short_name(two_classes_one_dsl)).to eq resource_class
+          it "resource_matching_short_name returns Z" do
+            expect(Chef::Resource.resource_matching_short_name(two_classes_one_dsl)).to eq resource_class_z
           end
 
           context "when Z provides(:two_classes_one_dsl) { false }" do
@@ -919,8 +919,8 @@ describe "Recipe DSL methods" do
             expect(BaseThingy.created_resource).to eq resource_class_z
           end
 
-          it "resource_matching_short_name returns B" do
-            expect(Chef::Resource.resource_matching_short_name(two_classes_one_dsl)).to eq resource_class
+          it "resource_matching_short_name returns Z" do
+            expect(Chef::Resource.resource_matching_short_name(two_classes_one_dsl)).to eq resource_class_z
           end
 
           context "when Z provides(:two_classes_one_dsl) { false }" do
@@ -1047,13 +1047,13 @@ describe "Recipe DSL methods" do
           context "which provides :two_classes_one_dsl" do
             before { provider_class_z.provides two_classes_one_dsl }
 
-            it "two_classes_one_dsl resolves to B (alphabetically earliest)" do
+            it "two_classes_one_dsl resolves to Z (last)" do
               temp_two_classes_one_dsl = two_classes_one_dsl
               recipe = converge do
                 instance_eval("#{temp_two_classes_one_dsl} 'blah'")
               end
               expect(recipe.logged_warnings).to eq ""
-              expect(BaseThingy.created_provider).to eq provider_class
+              expect(BaseThingy.created_provider).to eq provider_class_z
             end
 
             context "with a priority array [ Z, B ]" do
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"