From 50b8dbeef7066cf02cfeb17bf7fd637331b6356d Mon Sep 17 00:00:00 2001 From: John Keiser Date: Mon, 14 Dec 2015 16:55:26 -0800 Subject: Only warn about potentially duplicate properties during the resource initializer --- lib/chef/property.rb | 9 ++++--- lib/chef/resource.rb | 15 +++++++----- lib/chef/resource_builder.rb | 11 +++++++-- spec/integration/recipes/resource_action_spec.rb | 30 +++++++++++++++++++++++- 4 files changed, 53 insertions(+), 12 deletions(-) diff --git a/lib/chef/property.rb b/lib/chef/property.rb index 742eba42fd..89e4ffe0e6 100644 --- a/lib/chef/property.rb +++ b/lib/chef/property.rb @@ -18,6 +18,8 @@ require 'chef/exceptions' require 'chef/delayed_evaluator' +require 'chef/chef_class' +require 'chef/log' class Chef # @@ -314,9 +316,10 @@ class Chef # # It won't do what they expect. This checks whether you try to *read* # `content` while we are compiling the resource. - if resource.respond_to?(:enclosing_provider) && resource.enclosing_provider && - !resource.currently_running_action && - !name_property? && + if resource.respond_to?(:resource_initializing) && + resource.resource_initializing && + resource.respond_to?(:enclosing_provider) && + resource.enclosing_provider && resource.enclosing_provider.respond_to?(name) Chef::Log.warn("#{Chef::Log.caller_location}: property #{name} is declared in both #{resource} and #{resource.enclosing_provider}. Use new_resource.#{name} instead. At #{Chef::Log.caller_location}") end diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb index f969ccd84c..67cf4cd711 100644 --- a/lib/chef/resource.rb +++ b/lib/chef/resource.rb @@ -569,8 +569,6 @@ class Chef # def run_action(action, notification_type=nil, notifying_resource=nil) # reset state in case of multiple actions on the same resource. - old_currently_running_action = @currently_running_action - @currently_running_action = action @elapsed_time = 0 start_time = Time.now events.resource_action_start(self, action, notification_type, notifying_resource) @@ -612,7 +610,6 @@ class Chef end end ensure - @currently_running_action = old_currently_running_action @elapsed_time = Time.now - start_time # Reporting endpoint doesn't accept a negative resource duration so set it to 0. # A negative value can occur when a resource changes the system time backwards @@ -621,12 +618,18 @@ class Chef end # - # If we are currently running an action, this shows the action we are running. - # If the resource is running multiple actions at once, this will show the most recent. + # If we are currently initializing the resource, this will be true. # # Do NOT use this. It may be removed. It is for internal purposes only. # @api private - attr_reader :currently_running_action + attr_reader :resource_initializing + def resource_initializing=(value) + if value + @resource_initializing = true + else + remove_instance_variable(:@resource_initializing) + end + end # # Generic Ruby and Data Structure Stuff (for user) diff --git a/lib/chef/resource_builder.rb b/lib/chef/resource_builder.rb index 72b3788cf0..5b2ed4bb3f 100644 --- a/lib/chef/resource_builder.rb +++ b/lib/chef/resource_builder.rb @@ -70,7 +70,14 @@ class Chef resource.params = params # Evaluate resource attribute DSL - resource.instance_eval(&block) if block_given? + if block_given? + resource.resource_initializing = true + begin + resource.instance_eval(&block) + ensure + resource.resource_initializing = false + end + end # emit a cloned resource warning if it is warranted if prior_resource @@ -130,7 +137,7 @@ class Chef @prior_resource ||= begin key = "#{type}[#{name}]" - prior_resource = run_context.resource_collection.lookup(key) + run_context.resource_collection.lookup(key) rescue Chef::Exceptions::ResourceNotFound nil end diff --git a/spec/integration/recipes/resource_action_spec.rb b/spec/integration/recipes/resource_action_spec.rb index fe6b4083f1..a0e0dbb708 100644 --- a/spec/integration/recipes/resource_action_spec.rb +++ b/spec/integration/recipes/resource_action_spec.rb @@ -377,6 +377,15 @@ describe "Resource.action" do x x end end + def self.x_warning_line + __LINE__-4 + end + action :set_x_to_x_in_non_initializer do + r = resource_action_spec_with_x 'hi' do + x 10 + end + x_times_2 = r.x*2 + end action :set_x_to_10 do resource_action_spec_with_x 'hi' do x 10 @@ -384,6 +393,8 @@ describe "Resource.action" do end end + attr_reader :x_warning_line + it "Using the enclosing resource to set x to x emits a warning that you're using the wrong x" do recipe = converge { resource_action_spec_also_with_x 'hi' do @@ -393,7 +404,16 @@ describe "Resource.action" do } warnings = recipe.logs.lines.select { |l| l =~ /warn/i } expect(warnings.size).to eq 1 - expect(warnings[0]).to match(/property x is declared in both resource_action_spec_with_x\[hi\] and resource_action_spec_also_with_x\[hi\] action :set_x_to_x. Use new_resource.x instead. At #{__FILE__}:#{__LINE__-19}/) + expect(warnings[0]).to match(/property x is declared in both resource_action_spec_with_x\[hi\] and resource_action_spec_also_with_x\[hi\] action :set_x_to_x. Use new_resource.x instead. At #{__FILE__}:#{ResourceActionSpecAlsoWithX.x_warning_line}/) + end + + it "Using the enclosing resource to set x to x outside the initializer emits no warning" do + expect_recipe { + resource_action_spec_also_with_x 'hi' do + x 1 + action :set_x_to_x_in_non_initializer + end + }.to emit_no_warnings_or_errors end it "Using the enclosing resource to set x to 10 emits no warning" do @@ -404,6 +424,14 @@ describe "Resource.action" do end }.to emit_no_warnings_or_errors end + + it "Using the enclosing resource to set x to 10 emits no warning" do + expect_recipe { + r = resource_action_spec_also_with_x 'hi' + r.x 1 + r.action :set_x_to_10 + }.to emit_no_warnings_or_errors + end end end -- cgit v1.2.1