remove duping for load_current_value and code duplicationlcg/after-resource

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

2 files changed, 24 insertions, 52 deletions

diff --git a/lib/chef/resource/action_class.rb b/lib/chef/resource/action_class.rb
index 23418df4ed..eefa968197 100644
--- a/lib/chef/resource/action_class.rb
+++ b/lib/chef/resource/action_class.rb
@@ -29,71 +29,42 @@ class Chef
         "#{new_resource || "<no resource>"} action #{action ? action.inspect : "<no action>"}"
       end
 
-      #
-      # If load_current_value! is defined on the resource, use that.
-      #
-      def load_current_resource
+      def return_load_current_value
+        resource = nil
         if new_resource.respond_to?(:load_current_value!)
-          # dup the resource and then reset desired-state properties.
-          current_resource = new_resource.dup
+          resource = new_resource.class.new(new_resource.name, new_resource.run_context)
 
-          # We clear desired state in the copy, because it is supposed to be actual state.
-          # We keep identity properties and non-desired-state, which are assumed to be
-          # "control" values like `recurse: true`
-          current_resource.class.properties.each_value do |property|
-            if property.desired_state? && !property.identity? && !property.name_property?
-              property.reset(current_resource)
+          # copy the non-desired state, the identity properties and name property to the new resource
+          # (the desired state values must be loaded by load_current_value)
+          resource.class.properties.each_value do |property|
+            if !property.desired_state? || property.identity? || property.name_property?
+              property.set(resource, new_resource.send(property.name)) if new_resource.class.properties[property.name].is_set?(new_resource)
             end
           end
 
-          # Call the actual load_current_value! method. If it raises
-          # CurrentValueDoesNotExist, set current_resource to `nil`.
+          # we support optionally passing the new_resource as an arg to load_current_value and
+          # load_current_value can raise in order to clear the current_resource to nil
           begin
-            # If the user specifies load_current_value do |desired_resource|, we
-            # pass in the desired resource as well as the current one.
-            if current_resource.method(:load_current_value!).arity > 0
-              current_resource.load_current_value!(new_resource)
+            if resource.method(:load_current_value!).arity > 0
+              resource.load_current_value!(new_resource)
             else
-              current_resource.load_current_value!
+              resource.load_current_value!
             end
           rescue Chef::Exceptions::CurrentValueDoesNotExist
-            current_resource = nil
+            resource = nil
           end
         end
+        resource
+      end
 
-        @current_resource = current_resource
+      # build the before state (current_resource)
+      def load_current_resource
+        @current_resource = return_load_current_value
       end
 
+      # build the after state (after_resource)
       def load_after_resource
-        if new_resource.respond_to?(:load_current_value!)
-          # dup the resource and then reset desired-state properties.
-          after_resource = new_resource.dup
-
-          # We clear desired state in the copy, because it is supposed to be actual state.
-          # We keep identity properties and non-desired-state, which are assumed to be
-          # "control" values like `recurse: true`
-          after_resource.class.properties.each_value do |property|
-            if property.desired_state? && !property.identity? && !property.name_property?
-              property.reset(after_resource)
-            end
-          end
-
-          # Call the actual load_current_value! method. If it raises
-          # CurrentValueDoesNotExist, set after_resource to `nil`.
-          begin
-            # If the user specifies load_current_value do |desired_resource|, we
-            # pass in the desired resource as well as the current one.
-            if after_resource.method(:load_current_value!).arity > 0
-              after_resource.load_current_value!(new_resource)
-            else
-              after_resource.load_current_value!
-            end
-          rescue Chef::Exceptions::CurrentValueDoesNotExist
-            after_resource = nil
-          end
-        end
-
-        @after_resource = after_resource
+        @after_resource = return_load_current_value
       end
 
       def self.include_resource_dsl?
diff --git a/spec/unit/resource/chocolatey_source_spec.rb b/spec/unit/resource/chocolatey_source_spec.rb
index 60bd773594..aad9c4c283 100644
--- a/spec/unit/resource/chocolatey_source_spec.rb
+++ b/spec/unit/resource/chocolatey_source_spec.rb
@@ -1,5 +1,5 @@
 #
-# Copyright:: Copyright 2018, Chef Software, Inc.
+# Copyright:: Copyright 2018-2020, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -49,9 +49,10 @@ describe Chef::Resource::ChocolateySource do
   before(:each) do
     disable_provider # vivify before mocking
     enable_provider
+    current_resource
     allow(resource).to receive(:provider_for_action).and_return(disable_provider)
     allow(resource).to receive(:provider_for_action).and_return(enable_provider)
-    allow(resource).to receive(:dup).and_return(current_resource)
+    allow(resource.class).to receive(:new).and_return(current_resource)
     @original_env = ENV.to_hash
     ENV["ALLUSERSPROFILE"] = 'C:\ProgramData'
   end