Use object ID when detected unprocessed Resources (#5604)

* 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>

1 files changed, 56 insertions, 0 deletions

diff --git a/spec/unit/data_collector_spec.rb b/spec/unit/data_collector_spec.rb
index 4ae227ba3c..f3f7ffb30f 100644
--- a/spec/unit/data_collector_spec.rb
+++ b/spec/unit/data_collector_spec.rb
@@ -682,4 +682,60 @@ describe Chef::DataCollector::Reporter do
       end
     end
   end
+
+  describe "#detect_unprocessed_resources" do
+    context "when resources do not override core methods" 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 resources, 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
+
+    context "when a resource overrides a core method, such as #hash" do
+      it "does not raise an exception" do
+        resource_a  = Chef::Resource::Service.new("processed service")
+        resource_b  = Chef::Resource::Service.new("unprocessed service")
+
+        resource_a.action = :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)
+
+        # override the #hash method on resource_a to return a String instead of
+        # a Fixnum. Without the fix in chef/chef#5604, this would raise an
+        # exception when getting added to the Set/Hash.
+        resource_a.define_singleton_method(:hash) { "a string" }
+
+        # process the actions for resource_a, but not resource_b
+        reporter.resource_up_to_date(resource_a, :start)
+        reporter.resource_completed(resource_a)
+
+        expect { reporter.send(:detect_unprocessed_resources) }.not_to raise_error
+      end
+    end
+  end
 end