Only stick non-frozen values to the resource

2 files changed, 57 insertions, 6 deletions

diff --git a/lib/chef/property.rb b/lib/chef/property.rb
index 17554c5661..2eeec87f75 100644
--- a/lib/chef/property.rb
+++ b/lib/chef/property.rb
@@ -279,12 +279,20 @@ class Chef
           value = default
           if value.is_a?(DelayedEvaluator)
             value = exec_in_resource(resource, value)
-            value = coerce(resource, value)
-            # We don't validate defaults
+          end
+
+          value = coerce(resource, value)
+
+          # We don't validate defaults
+
+          # If the value is mutable (non-frozen), we set it on the instance
+          # so that people can mutate it.  (All constant default values are
+          # frozen.)
+          if !value.frozen?
             set_value(resource, value)
-          else
-            value = coerce(resource, value)
           end
+
+          value
         end
       end
     end
diff --git a/spec/unit/property_spec.rb b/spec/unit/property_spec.rb
index c613739522..23f6f51d2c 100644
--- a/spec/unit/property_spec.rb
+++ b/spec/unit/property_spec.rb
@@ -397,9 +397,9 @@ describe "Chef::Resource.property" do
         resource.x = 10
         expect(resource.property_is_set?(:x)).to be_truthy
       end
-      it "when x is retrieved, property_is_set?(:x) is true" do
+      it "when x is retrieved, property_is_set?(:x) is false" do
         resource.x
-        expect(resource.property_is_set?(:x)).to be_truthy
+        expect(resource.property_is_set?(:x)).to be_falsey
       end
     end
   end
@@ -595,6 +595,7 @@ describe "Chef::Resource.property" do
     end
 
     context "coercion of defaults" do
+      # Frozen default, non-frozen coerce
       with_property ':x, coerce: proc { |v| "#{v}#{next_index}" }, default: 10' do
         it "when the resource is created, the proc is not yet run" do
           resource
@@ -605,6 +606,24 @@ describe "Chef::Resource.property" do
           expect(resource.x).to eq 'hi1'
           expect(Namer.current_index).to eq 1
         end
+        it "when x is retrieved, coercion is run exactly once" do
+          expect(resource.x).to eq '101'
+          expect(resource.x).to eq '101'
+          expect(Namer.current_index).to eq 1
+        end
+      end
+
+      # Frozen default, frozen coerce
+      with_property ':x, coerce: proc { |v| "#{v}#{next_index}".freeze }, default: 10' do
+        it "when the resource is created, the proc is not yet run" do
+          resource
+          expect(Namer.current_index).to eq 0
+        end
+        it "when x is set, coercion is run" do
+          expect(resource.x 'hi').to eq 'hi1'
+          expect(resource.x).to eq 'hi1'
+          expect(Namer.current_index).to eq 1
+        end
         it "when x is retrieved, coercion is run each time" do
           expect(resource.x).to eq '101'
           expect(resource.x).to eq '102'
@@ -612,6 +631,7 @@ describe "Chef::Resource.property" do
         end
       end
 
+      # Frozen lazy default, non-frozen coerce
       with_property ':x, coerce: proc { |v| "#{v}#{next_index}" }, default: lazy { 10 }' do
         it "when the resource is created, the proc is not yet run" do
           resource
@@ -622,6 +642,29 @@ describe "Chef::Resource.property" do
           expect(resource.x).to eq 'hi1'
           expect(Namer.current_index).to eq 1
         end
+        it "when x is retrieved, coercion is run exactly once" do
+          expect(resource.x).to eq '101'
+          expect(resource.x).to eq '101'
+          expect(Namer.current_index).to eq 1
+        end
+      end
+
+      # Non-frozen lazy default, frozen coerce
+      with_property ':x, coerce: proc { |v| "#{v}#{next_index}".freeze }, default: lazy { "10" }' do
+        it "when the resource is created, the proc is not yet run" do
+          resource
+          expect(Namer.current_index).to eq 0
+        end
+        it "when x is set, coercion is run" do
+          expect(resource.x 'hi').to eq 'hi1'
+          expect(resource.x).to eq 'hi1'
+          expect(Namer.current_index).to eq 1
+        end
+        it "when x is retrieved, coercion is run each time" do
+          expect(resource.x).to eq '101'
+          expect(resource.x).to eq '102'
+          expect(Namer.current_index).to eq 2
+        end
       end
 
       with_property ':x, proc { |v| Namer.next_index; true }, coerce: proc { |v| "#{v}#{next_index}" }, default: lazy { 10 }' do