diff options
author | Adam Leff <adam@leff.co> | 2016-12-01 11:14:38 -0500 |
---|---|---|
committer | Adam Leff <adam@leff.co> | 2016-12-05 17:38:21 -0500 |
commit | 31ded71e8c00e2a26cf0cdfee42f56998f03dcbc (patch) | |
tree | ce9ec3fc0b7ae86857872932a56a92baaa4a8c1c | |
parent | 0b5c6a2ec8db1f8ab368c16a066b9a5b17a657cc (diff) | |
download | chef-31ded71e8c00e2a26cf0cdfee42f56998f03dcbc.tar.gz |
Use object ID when detected unprocessed Resources
In the Data Collector, when detecting unprocessed resources, a Set
was built up using the resource object itself as the Set element.
Internally, Set adds this to a Hash.
We allow users to create custom resources that could contain a property
named "hash" which in turn wires up a `#hash` instance method on the
Resource. Ruby expects object's `#hash` method to return a Fixnum that
it uses internally. So if a resource had a "hash" property that returned
a String, bad things happened.
With this change, we make our own Hash and use the resource's object ID
in the key so we don't have to worry about the resource's `#hash` method
getting called and throwing an exception. I will send up a separate
change that warns users when they choose a property name that is already
an existing method name.
Fixes #5565.
Signed-off-by: Adam Leff <adam@leff.co>
-rw-r--r-- | lib/chef/data_collector.rb | 25 | ||||
-rw-r--r-- | spec/unit/data_collector_spec.rb | 27 |
2 files changed, 43 insertions, 9 deletions
diff --git a/lib/chef/data_collector.rb b/lib/chef/data_collector.rb index 0e676145eb..d9f6038b2b 100644 --- a/lib/chef/data_collector.rb +++ b/lib/chef/data_collector.rb @@ -430,26 +430,33 @@ class Chef end def detect_unprocessed_resources - # create a Set containing all resource+action combinations from - # the Resource Collection - collection_resources = Set.new + # create a Hash (for performance reasons, rather than an Array) containing all + # resource+action combinations from the Resource Collection + # + # We use the object ID instead of the resource itself in the Hash key because + # we currently allow users to create a property called "hash" which creates + # a #hash instance method on the resource. Ruby expects that to be a Fixnum, + # so bad things happen when adding an object to an Array or a Hash if it's not. + collection_resources = {} run_context.resource_collection.all_resources.each do |resource| Array(resource.action).each do |action| - collection_resources.add([resource, action]) + collection_resources[[resource.__id__, action]] = resource end end - # Delete from the Set any resource+action combination we have + # Delete from the Hash any resource+action combination we have # already processed. all_resource_reports.each do |report| - collection_resources.delete([report.resource, report.action]) + collection_resources.delete([report.resource.__id__, report.action]) end - # The items remaining in the Set are unprocessed resource+actions, + # The items remaining in the Hash are unprocessed resource+actions, # so we'll create new resource reports for them which default to # a state of "unprocessed". - collection_resources.each do |resource, action| - add_resource_report(create_resource_report(resource, action)) + collection_resources.each do |key, resource| + # The Hash key is an array of the Resource's object ID and the action. + # We need to pluck out the action. + add_resource_report(create_resource_report(resource, key[1])) end end diff --git a/spec/unit/data_collector_spec.rb b/spec/unit/data_collector_spec.rb index 4ae227ba3c..18a778b714 100644 --- a/spec/unit/data_collector_spec.rb +++ b/spec/unit/data_collector_spec.rb @@ -682,4 +682,31 @@ describe Chef::DataCollector::Reporter do end end end + + describe "#detect_unprocessed_resources" do + it "adds resource reports for any resources that have not yet been processed" do + resource_a = Chef::Resource::Service.new("processed service") + resource_b = Chef::Resource::Service.new("unprocessed service") + + resource_a.action = [ :enable, :start ] + resource_b.action = :start + + run_context = Chef::RunContext.new(Chef::Node.new, Chef::CookbookCollection.new, nil) + run_context.resource_collection.insert(resource_a) + run_context.resource_collection.insert(resource_b) + + allow(reporter).to receive(:run_context).and_return(run_context) + + # process the actions for resource_a, but not resource_b + reporter.resource_up_to_date(resource_a, :enable) + reporter.resource_completed(resource_a) + reporter.resource_up_to_date(resource_a, :start) + reporter.resource_completed(resource_a) + expect(reporter.all_resource_reports.size).to eq(2) + + # detect unprocessed resourced, which should find that resource_b has not yet been processed + reporter.send(:detect_unprocessed_resources) + expect(reporter.all_resource_reports.size).to eq(3) + end + end end |