From eb1c36404da38801ca755deccfd754512dd74782 Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Thu, 8 Jun 2017 13:31:40 -0700 Subject: Deprecate property namespace magic This forces everyone to starting using new_resource.property_name instead of just property_name. Signed-off-by: Lamont Granquist --- lib/chef/deprecated.rb | 10 +++++++++ lib/chef/dsl/declare_resource.rb | 10 ++++++++- lib/chef/provider.rb | 2 ++ spec/integration/recipes/accumulator_spec.rb | 12 +++++------ spec/integration/recipes/resource_action_spec.rb | 25 ++++++++++++++++------- spec/integration/recipes/resource_load_spec.rb | 26 +++++------------------- 6 files changed, 50 insertions(+), 35 deletions(-) diff --git a/lib/chef/deprecated.rb b/lib/chef/deprecated.rb index 372609c10a..2cf0f3d1bc 100644 --- a/lib/chef/deprecated.rb +++ b/lib/chef/deprecated.rb @@ -248,6 +248,16 @@ class Chef end end + class NamespaceCollisions < Base + def id + 19 + end + + def target + "namespace_collisions.html" + end + end + # id 3694 was deleted class Generic < Base diff --git a/lib/chef/dsl/declare_resource.rb b/lib/chef/dsl/declare_resource.rb index 6869e77eca..0d727b4861 100644 --- a/lib/chef/dsl/declare_resource.rb +++ b/lib/chef/dsl/declare_resource.rb @@ -155,7 +155,15 @@ class Chef def edit_resource(type, name, created_at: nil, run_context: self.run_context, &resource_attrs_block) edit_resource!(type, name, created_at: created_at, run_context: run_context, &resource_attrs_block) rescue Chef::Exceptions::ResourceNotFound - declare_resource(type, name, created_at: created_at, run_context: run_context, &resource_attrs_block) + resource = declare_resource(type, name, created_at: created_at, run_context: run_context) + if resource_attrs_block + if defined?(new_resource) + resource.instance_exec(new_resource, &resource_attrs_block) + else + resource.instance_exec(&resource_attrs_block) + end + end + resource end # Lookup a resource in the resource collection by name. If the resource is not diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb index c7048d50e5..71d3b12b34 100644 --- a/lib/chef/provider.rb +++ b/lib/chef/provider.rb @@ -350,9 +350,11 @@ class Chef # if args.empty? && !block if !new_resource.property_is_set?(__method__) && current_resource + Chef.deprecated(:namespace_collisions, "rename #{name} to current_resource.#{name}") return current_resource.public_send(__method__) end end + Chef.deprecated(:namespace_collisions, "rename #{name} to new_resource.#{name}") new_resource.public_send(__method__, *args, &block) end EOM diff --git a/spec/integration/recipes/accumulator_spec.rb b/spec/integration/recipes/accumulator_spec.rb index e6afe09b8c..4a193bd7f0 100644 --- a/spec/integration/recipes/accumulator_spec.rb +++ b/spec/integration/recipes/accumulator_spec.rb @@ -62,7 +62,7 @@ describe "Accumulators" do default_action :create action :create do - email_alias address do + email_alias new_resource.address do recipients new_resource.recipients end end @@ -78,7 +78,7 @@ describe "Accumulators" do default_action :create action :create do - nested address do + nested new_resource.address do recipients new_resource.recipients end end @@ -149,8 +149,8 @@ describe "Accumulators" do delayed_action :create end end - r.variables[:aliases][address] ||= [] - r.variables[:aliases][address] += new_resource.recipients + r.variables[:aliases][new_resource.address] ||= [] + r.variables[:aliases][new_resource.address] += new_resource.recipients end EOM @@ -164,7 +164,7 @@ describe "Accumulators" do default_action :create action :create do - email_alias address do + email_alias new_resource.address do recipients new_resource.recipients end end @@ -180,7 +180,7 @@ describe "Accumulators" do default_action :create action :create do - nested address do + nested new_resource.address do recipients new_resource.recipients end end diff --git a/spec/integration/recipes/resource_action_spec.rb b/spec/integration/recipes/resource_action_spec.rb index 03331c8625..f5f2d4a764 100644 --- a/spec/integration/recipes/resource_action_spec.rb +++ b/spec/integration/recipes/resource_action_spec.rb @@ -416,6 +416,7 @@ module ResourceActionSpec 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 + Chef::Config[:treat_deprecation_warnings_as_errors] = false recipe = converge do resource_action_spec_also_with_x "hi" do x 1 @@ -423,34 +424,43 @@ module ResourceActionSpec end end warnings = recipe.logs.lines.select { |l| l =~ /warn/i } - expect(warnings.size).to eq 1 + expect(warnings.size).to eq 2 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 do + Chef::Config[:treat_deprecation_warnings_as_errors] = false + recipe = converge do resource_action_spec_also_with_x "hi" do x 1 action :set_x_to_x_in_non_initializer end - end.to emit_no_warnings_or_errors + end + warnings = recipe.logs.lines.select { |l| l =~ /warn/i } + expect(warnings.size).to eq 1 # the deprecation warning, not the property masking one end it "Using the enclosing resource to set x to 10 emits no warning" do - expect_recipe do + Chef::Config[:treat_deprecation_warnings_as_errors] = false + recipe = converge do resource_action_spec_also_with_x "hi" do x 1 action :set_x_to_10 end - end.to emit_no_warnings_or_errors + end + warnings = recipe.logs.lines.select { |l| l =~ /warn/i } + expect(warnings.size).to eq 1 # the deprecation warning, not the property masking one end it "Using the enclosing resource to set x to 10 emits no warning" do - expect_recipe do + Chef::Config[:treat_deprecation_warnings_as_errors] = false + recipe = converge do r = resource_action_spec_also_with_x "hi" r.x 1 r.action :set_x_to_10 - end.to emit_no_warnings_or_errors + end + warnings = recipe.logs.lines.select { |l| l =~ /warn/i } + expect(warnings.size).to eq 1 # the deprecation warning, not the property masking one end end @@ -496,6 +506,7 @@ module ResourceActionSpec end it "Raises an error when attempting to use a template in the action" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false expect_converge do has_property_named_template "hi" end.to raise_error(/Property `template` of `has_property_named_template\[hi\]` was incorrectly passed a block. Possible property-resource collision. To call a resource named `template` either rename the property or else use `declare_resource\(:template, ...\)`/) diff --git a/spec/integration/recipes/resource_load_spec.rb b/spec/integration/recipes/resource_load_spec.rb index 791b83c23a..4fc14c0687 100644 --- a/spec/integration/recipes/resource_load_spec.rb +++ b/spec/integration/recipes/resource_load_spec.rb @@ -32,7 +32,7 @@ describe "Resource.load_current_value" do @created end action :create do - new_resource.class.created_x = x + new_resource.class.created_x = new_resource.x end end result.resource_name resource_name @@ -45,10 +45,10 @@ describe "Resource.load_current_value" do context "with a resource with load_current_value" do before :each do resource_class.load_current_value do - x "loaded #{Namer.incrementing_value} (#{self.class.properties.sort_by { |name, p| name }. - select { |name, p| p.is_set?(self) }. - map { |name, p| "#{name}=#{p.get(self)}" }. - join(", ")})" + x "loaded #{Namer.incrementing_value} (#{self.class.properties.sort_by { |name, p| name } + .select { |name, p| p.is_set?(self) } + .map { |name, p| "#{name}=#{p.get(self)}" } + .join(", ")})" end end @@ -119,22 +119,6 @@ describe "Resource.load_current_value" do end end - context "and a resource with no values set" do - let(:resource) do - e = self - r = nil - converge do - r = public_send(e.resource_name, "blah") do - end - end - r - end - - it "the provider accesses values from load_current_value" do - expect(resource.class.created_x).to eq "loaded 1 (name=blah)" - end - end - let (:subresource_name) do :"load_current_value_subresource_dsl#{Namer.current_index}" end -- cgit v1.2.1