Update gem_package with version constraint on :upgrade action

Add Package#version_requirement_satisfied? to separate fuzzy-match (:install) and exact-match (:upgrade) version checks when supported by the package subclass.
Wrap Package#candidate_version in a lazy-initialization object to minimize unnecessary API queries when candidate_version is not needed.
(fixes #3867)

2 files changed, 45 insertions, 9 deletions

diff --git a/spec/unit/provider/package/rubygems_spec.rb b/spec/unit/provider/package/rubygems_spec.rb
index 6e77e7c112..29de4b4d11 100644
--- a/spec/unit/provider/package/rubygems_spec.rb
+++ b/spec/unit/provider/package/rubygems_spec.rb
@@ -496,12 +496,35 @@ describe Chef::Provider::Package::Rubygems do
         provider.load_current_resource
       end
 
-      it "does not query for available versions when the current version is the target version" do
-        expect(provider.candidate_version).to be_nil
+      context "when the current version is the target version" do
+        it "does not query for available versions" do
+          expect(provider.gem_env).not_to receive(:candidate_version_from_remote)
+          expect(provider.gem_env).not_to receive(:install)
+          provider.run_action(:upgrade)
+          expect(new_resource).not_to be_updated_by_last_action
+        end
       end
 
-      context "when the current version is not the target version" do
-        let(:target_version) { "9000.0.2" }
+      context "when the current version satisfies the target version requirement" do
+        let(:target_version) { ">= 0" }
+
+        it "does not query for available versions on install" do
+          expect(provider.gem_env).not_to receive(:candidate_version_from_remote)
+          expect(provider.gem_env).not_to receive(:install)
+          provider.run_action(:install)
+          expect(new_resource).not_to be_updated_by_last_action
+        end
+
+        it "queries for available versions on upgrade" do
+          expect(provider.gem_env).to receive(:candidate_version_from_remote).
+            and_return(Gem::Version.new("9000.0.2"))
+          expect(provider.gem_env).to receive(:install)
+          provider.run_action(:upgrade)
+          expect(new_resource).to be_updated_by_last_action
+        end
+      end
+
+      context "when the requested source is a remote server" do
         let(:source) { "http://mygems.example.com" }
 
         it "determines the candidate version by querying the remote gem servers" do
@@ -669,6 +692,11 @@ describe Chef::Provider::Package::Rubygems do
               expect(provider.gem_env).not_to receive(:install)
               provider.run_action(:install)
             end
+
+            it "performs the upgrade" do
+              expect(provider.gem_env).to receive(:install)
+              provider.run_action(:upgrade)
+            end
           end
 
           context "if the fuzzy operator is used" do
@@ -679,6 +707,11 @@ describe Chef::Provider::Package::Rubygems do
               expect(provider.gem_env).not_to receive(:install)
               provider.run_action(:install)
             end
+
+            it "it upgrades an existing gem" do
+              expect(provider.gem_env).to receive(:install)
+              provider.run_action(:upgrade)
+            end
           end
         end
       end
diff --git a/spec/unit/provider/package_spec.rb b/spec/unit/provider/package_spec.rb
index abf0322868..e4354d0db4 100644
--- a/spec/unit/provider/package_spec.rb
+++ b/spec/unit/provider/package_spec.rb
@@ -566,8 +566,11 @@ describe "Chef::Provider::Package - Multi" do
   let(:new_resource) { Chef::Resource::Package.new(%w{emacs vi}) }
   let(:current_resource) { Chef::Resource::Package.new(%w{emacs vi}) }
   let(:candidate_version) { [ "1.0", "6.2" ] }
+  class MyPackageProvider < Chef::Provider::Package
+    use_multipackage_api
+  end
   let(:provider) do
-    provider = Chef::Provider::Package.new(new_resource, run_context)
+    provider = MyPackageProvider.new(new_resource, run_context)
     provider.current_resource = current_resource
     provider.candidate_version = candidate_version
     provider
@@ -731,7 +734,7 @@ describe "Chef::Provider::Package - Multi" do
 
     it "should remove the packages if all are installed" do
       expect(provider).to be_removing_package
-      expect(provider).to receive(:remove_package).with(%w{emacs vi}, nil)
+      expect(provider).to receive(:remove_package).with(%w{emacs vi}, [nil])
       provider.run_action(:remove)
       expect(new_resource).to be_updated
       expect(new_resource).to be_updated_by_last_action
@@ -740,7 +743,7 @@ describe "Chef::Provider::Package - Multi" do
     it "should remove the packages if some are installed" do
       current_resource.version ["1.0", nil]
       expect(provider).to be_removing_package
-      expect(provider).to receive(:remove_package).with(%w{emacs vi}, nil)
+      expect(provider).to receive(:remove_package).with(%w{emacs vi}, [nil])
       provider.run_action(:remove)
       expect(new_resource).to be_updated
       expect(new_resource).to be_updated_by_last_action
@@ -787,7 +790,7 @@ describe "Chef::Provider::Package - Multi" do
 
     it "should purge the packages if all are installed" do
       expect(provider).to be_removing_package
-      expect(provider).to receive(:purge_package).with(%w{emacs vi}, nil)
+      expect(provider).to receive(:purge_package).with(%w{emacs vi}, [nil])
       provider.run_action(:purge)
       expect(new_resource).to be_updated
       expect(new_resource).to be_updated_by_last_action
@@ -796,7 +799,7 @@ describe "Chef::Provider::Package - Multi" do
     it "should purge the packages if some are installed" do
       current_resource.version ["1.0", nil]
       expect(provider).to be_removing_package
-      expect(provider).to receive(:purge_package).with(%w{emacs vi}, nil)
+      expect(provider).to receive(:purge_package).with(%w{emacs vi}, [nil])
       provider.run_action(:purge)
       expect(new_resource).to be_updated
       expect(new_resource).to be_updated_by_last_action