add nillable apt_repository and nillable properties

in Chef-13 nillable will become 'true' by default and we'll have
to deprecate and remove all nillable properties, but for now
this lets us opt-in, which was can't currently do in Chef-12.

6 files changed, 50 insertions, 16 deletions

diff --git a/lib/chef/property.rb b/lib/chef/property.rb
index c36daa821a..45ab4dd522 100644
--- a/lib/chef/property.rb
+++ b/lib/chef/property.rb
@@ -72,6 +72,9 @@ class Chef
     #     property defaults to the same value as `name`. Equivalent to
     #     `default: lazy { name }`, except that #property_is_set? will
     #     return `true` if the property is set *or* if `name` is set.
+    #   @option options [Boolean] :nillable `true` opt-in to Chef-13 style behavior where
+    #     attempting to set a nil value will really set a nil value instead of issuing
+    #     a warning and operating like a getter
     #   @option options [Object] :default The value this property
     #     will return if the user does not set one. If this is `lazy`, it will
     #     be run in the context of the instance (and able to access other
@@ -233,7 +236,7 @@ class Chef
     #
     def validation_options
       @validation_options ||= options.reject { |k, v|
-        [:declared_in, :name, :instance_variable_name, :desired_state, :identity, :default, :name_property, :coerce, :required].include?(k)
+        [:declared_in, :name, :instance_variable_name, :desired_state, :identity, :default, :name_property, :coerce, :required, :nillable].include?(k)
       }
     end
 
@@ -262,7 +265,7 @@ class Chef
         return get(resource)
       end
 
-      if value.nil?
+      if value.nil? && !nillable?
         # In Chef 12, value(nil) does a *get* instead of a set, so we
         # warn if the value would have been changed. In Chef 13, it will be
         # equivalent to value = nil.
@@ -670,5 +673,9 @@ class Chef
 
       result
     end
+
+    def nillable?
+      !!options[:nillable]
+    end
   end
 end
diff --git a/lib/chef/provider/apt_repository.rb b/lib/chef/provider/apt_repository.rb
index 8880a059ac..1e7db80620 100644
--- a/lib/chef/provider/apt_repository.rb
+++ b/lib/chef/provider/apt_repository.rb
@@ -239,13 +239,15 @@ class Chef
 
         uri = '"' + uri + '"' unless uri.start_with?("'", '"')
         components = Array(components).join(" ")
-        options = ""
-        options << "arch=#{arch} " if arch
+        options = []
+        options << "arch=#{arch}" if arch
         options << "trusted=yes" if trusted
-        options = "[#{options}]" unless options.empty?
-        info = "#{options} #{uri} #{distribution} #{components}\n".lstrip
-        repo =  "deb      #{info}"
-        repo << "deb-src  #{info}" if add_src
+        optstr = unless options.empty?
+                   "[" + options.join(" ") + "]"
+                 end
+        info = [ optstr, uri, distribution, components ].compact.join(" ")
+        repo =  "deb      #{info}\n"
+        repo << "deb-src  #{info}\n" if add_src
         repo
       end
     end
diff --git a/lib/chef/resource/apt_repository.rb b/lib/chef/resource/apt_repository.rb
index e1ea665858..bba753638c 100644
--- a/lib/chef/resource/apt_repository.rb
+++ b/lib/chef/resource/apt_repository.rb
@@ -26,7 +26,7 @@ class Chef
 
       property :repo_name, String, name_property: true
       property :uri, String
-      property :distribution, String, default: lazy { node["lsb"]["codename"] }
+      property :distribution, [ String, nil ], default: lazy { node["lsb"]["codename"] }, nillable: true
       property :components, Array, default: []
       property :arch, [String, nil], default: nil
       property :trusted, [TrueClass, FalseClass], default: false
diff --git a/spec/unit/property/validation_spec.rb b/spec/unit/property/validation_spec.rb
index db3c9fe4f6..4e1b252863 100644
--- a/spec/unit/property/validation_spec.rb
+++ b/spec/unit/property/validation_spec.rb
@@ -99,13 +99,15 @@ describe "Chef::Resource.property validation" do
         expect(resource.x nil).to be_nil
         expect(resource.x).to be_nil
       end
-      it "changing x to nil warns that the get will change to a set in Chef 13 and does not change the value" do
-        resource.instance_eval { @x = "default" }
-        expect { resource.x nil }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
-          /An attempt was made to change x from "default" to nil by calling x\(nil\). In Chef 12, this does a get rather than a set. In Chef 13, this will change to set the value to nil./
-        Chef::Config[:treat_deprecation_warnings_as_errors] = false
-        expect(resource.x nil).to eq "default"
-        expect(resource.x).to eq "default"
+      unless tags.include?(:nillable)
+        it "changing x to nil warns that the get will change to a set in Chef 13 and does not change the value" do
+          resource.instance_eval { @x = "default" }
+          expect { resource.x nil }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
+            /An attempt was made to change x from "default" to nil by calling x\(nil\). In Chef 12, this does a get rather than a set. In Chef 13, this will change to set the value to nil./
+          Chef::Config[:treat_deprecation_warnings_as_errors] = false
+          expect(resource.x nil).to eq "default"
+          expect(resource.x).to eq "default"
+        end
       end
     end
     if tags.include?(:nil_is_valid)
@@ -123,6 +125,15 @@ describe "Chef::Resource.property validation" do
           expect(resource.x).to eq "default"
         end
       end
+    elsif tags.include?(:nillable)
+      with_property ":x, #{validation}, nillable: true" do
+        it "changing x to nil with nillable true overwrites defaults and just works" do
+          resource.instance_eval { @x = "default" }
+          expect { resource.x nil }.not_to raise_error
+          expect(resource.x nil).to eq nil
+          expect(resource.x).to eq nil
+        end
+      end
     else
       it "property :x, #{validation}, default: nil warns that the default is invalid" do
         expect { resource_class.class_eval("property :x, #{validation}, default: nil", __FILE__, __LINE__) }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
@@ -268,6 +279,11 @@ describe "Chef::Resource.property validation" do
     validation_test "[]",
       [],
       [ :a ]
+
+    validation_test "[ String, nil ], nillable: true",
+      [ nil, "thing" ],
+      [ :nope, false ],
+      :nillable
   end
 
   # is
diff --git a/spec/unit/provider/apt_repository_spec.rb b/spec/unit/provider/apt_repository_spec.rb
index 543b65cc90..d8f2c85cb7 100644
--- a/spec/unit/provider/apt_repository_spec.rb
+++ b/spec/unit/provider/apt_repository_spec.rb
@@ -158,6 +158,11 @@ C5986B4F1257FFA86632CBA746181433FBB75451
       expect(provider.build_repo("http://test/uri", "unstable", "main", false, nil)).to eql(target)
     end
 
+    it "should create a repository string with no distribution" do
+      target = %Q{deb      "http://test/uri" main\n}
+      expect(provider.build_repo("http://test/uri", nil, "main", false, nil)).to eql(target)
+    end
+
     it "should create a repository string with source" do
       target = %Q{deb      "http://test/uri" unstable main\ndeb-src  "http://test/uri" unstable main\n}
       expect(provider.build_repo("http://test/uri", "unstable", "main", false, nil, true)).to eql(target)
diff --git a/spec/unit/resource/apt_repository_spec.rb b/spec/unit/resource/apt_repository_spec.rb
index 88d9ca2508..cd55ce66cc 100644
--- a/spec/unit/resource/apt_repository_spec.rb
+++ b/spec/unit/resource/apt_repository_spec.rb
@@ -31,4 +31,8 @@ describe Chef::Resource::AptRepository do
     expect(resource.keyserver).to eql("keyserver.ubuntu.com")
   end
 
+  it "the default distribution should be nillable" do
+    expect(resource.distribution(nil)).to eql(nil)
+    expect(resource.distribution).to eql(nil)
+  end
 end