Merge pull request #1244 from alexjfisher/ensure_packages

Convert `ensure_packages` to new API and refactor

Author: ekohl

REFERENCE.md

@@ -60,6 +60,7 @@ or the default value if nothing was found.
 * [`empty`](#empty): **Deprecated:** Returns true if the variable is empty.
 * [`enclose_ipv6`](#enclose_ipv6): Takes an array of ip addresses and encloses the ipv6 addresses with square brackets.
 * [`ensure_packages`](#ensure_packages): Takes a list of packages and only installs them if they don't already exist.
+* [`ensure_packages`](#ensure_packages): Deprecated 3x version of the `ensure_packages` function
 * [`ensure_resource`](#ensure_resource): Takes a resource type, title, and a list of attributes that describe a
 resource.
 * [`ensure_resources`](#ensure_resources): Takes a resource type, title (only hash), and a list of attributes that describe a
@@ -1966,17 +1967,41 @@ Returns: `Any` encloses the ipv6 addresses with square brackets.
 
 ### <a name="ensure_packages"></a>`ensure_packages`
 
-Type: Ruby 3.x API
+Type: Ruby 4.x API
 
 It optionally takes a hash as a second parameter that will be passed as the
 third argument to the ensure_resource() function.
 
-#### `ensure_packages()`
+#### `ensure_packages(Variant[String[1], Array[String[1]], Hash[String[1], Any]] $packages, Optional[Hash] $default_attributes)`
 
 It optionally takes a hash as a second parameter that will be passed as the
 third argument to the ensure_resource() function.
 
-Returns: `Any` install the passed packages
+Returns: `Undef` Returns nothing.
+
+##### `packages`
+
+Data type: `Variant[String[1], Array[String[1]], Hash[String[1], Any]]`
+
+The packages to ensure are installed. If it's a Hash it will be passed to `ensure_resource`
+
+##### `default_attributes`
+
+Data type: `Optional[Hash]`
+
+Default attributes to be passed to the `ensure_resource()` function
+
+### <a name="ensure_packages"></a>`ensure_packages`
+
+Type: Ruby 3.x API
+
+Deprecated 3x version of the `ensure_packages` function
+
+#### `ensure_packages()`
+
+The ensure_packages function.
+
+Returns: `Any`
 
 ### <a name="ensure_resource"></a>`ensure_resource`
 

lib/puppet/functions/ensure_packages.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+# @summary Takes a list of packages and only installs them if they don't already exist.
+#
+# It optionally takes a hash as a second parameter that will be passed as the
+# third argument to the ensure_resource() function.
+Puppet::Functions.create_function(:ensure_packages, Puppet::Functions::InternalFunction) do
+  # @param packages
+  #   The packages to ensure are installed. If it's a Hash it will be passed to `ensure_resource`
+  # @param default_attributes
+  #   Default attributes to be passed to the `ensure_resource()` function
+  # @return [Undef] Returns nothing.
+  dispatch :ensure_packages do
+    scope_param
+    param 'Variant[String[1], Array[String[1]], Hash[String[1], Any]]', :packages
+    optional_param 'Hash', :default_attributes
+    return_type 'Undef'
+  end
+
+  def ensure_packages(scope, packages, default_attributes = nil)
+    if default_attributes
+      defaults = { 'ensure' => 'installed' }.merge(default_attributes)
+      if defaults['ensure'] == 'present'
+        defaults['ensure'] = 'installed'
+      end
+    else
+      defaults = { 'ensure' => 'installed' }
+    end
+
+    if packages.is_a?(Hash)
+      scope.call_function('ensure_resources', ['package', packages.dup, defaults])
+    else
+      Array(packages).each do |package_name|
+        scope.call_function('ensure_resource', ['package', package_name, defaults])
+      end
+    end
+    nil
+  end
+end

lib/puppet/parser/functions/ensure_packages.rb

@@ -1,54 +1,9 @@
 # frozen_string_literal: true
 
-#
-# ensure_packages.rb
-#
 module Puppet::Parser::Functions
-  newfunction(:ensure_packages, type: :statement, doc: <<-DOC
-    @summary
-      Takes a list of packages and only installs them if they don't already exist.
-
-    It optionally takes a hash as a second parameter that will be passed as the
-    third argument to the ensure_resource() function.
-
-    @return
-      install the passed packages
-  DOC
-  ) do |arguments|
-    raise(Puppet::ParseError, "ensure_packages(): Wrong number of arguments given (#{arguments.size} for 1 or 2)") if arguments.size > 2 || arguments.empty?
-    raise(Puppet::ParseError, 'ensure_packages(): Requires second argument to be a Hash') if arguments.size == 2 && !arguments[1].is_a?(Hash)
-
-    if arguments[0].is_a?(Hash)
-      if arguments[1]
-        defaults = { 'ensure' => 'installed' }.merge(arguments[1])
-        if defaults['ensure'] == 'present'
-          defaults['ensure'] = 'installed'
-        end
-      else
-        defaults = { 'ensure' => 'installed' }
-      end
-
-      Puppet::Parser::Functions.function(:ensure_resources)
-      function_ensure_resources(['package', arguments[0].dup, defaults])
-    else
-      packages = Array(arguments[0])
-
-      if arguments[1]
-        defaults = { 'ensure' => 'installed' }.merge(arguments[1])
-        if defaults['ensure'] == 'present'
-          defaults['ensure'] = 'installed'
-        end
-      else
-        defaults = { 'ensure' => 'installed' }
-      end
-
-      Puppet::Parser::Functions.function(:ensure_resource)
-      packages.each do |package_name|
-        raise(Puppet::ParseError, 'ensure_packages(): Empty String provided for package name') if package_name.empty?
-        function_ensure_resource(['package', package_name, defaults])
-      end
-    end
+  newfunction(:ensure_packages, type: :statement, doc: '@summary Deprecated 3x version of the `ensure_packages` function') do |arguments|
+    # Call the 4.x version of this function in case 3.x ruby code uses this function
+    Puppet.warn_once('deprecations', '3xfunction#ensure_packages', 'Calling function_ensure_packages via the Scope class is deprecated. Use Scope#call_function instead')
+    call_function('ensure_packages', arguments)
   end
 end
-
-# vim: set ts=2 sw=2 et :

spec/functions/ensure_packages_spec.rb

@@ -4,15 +4,6 @@
 
 describe 'ensure_packages' do
   it { is_expected.not_to eq(nil) }
-  it { is_expected.to run.with_params.and_raise_error(Puppet::ParseError) }
-  it {
-    pending('should not accept numbers as arguments')
-    is_expected.to run.with_params(1).and_raise_error(Puppet::ParseError)
-  }
-  it {
-    pending('should not accept numbers as arguments')
-    is_expected.to run.with_params(['packagename', 1]).and_raise_error(Puppet::ParseError)
-  }
   it { is_expected.to run.with_params('packagename') }
   it { is_expected.to run.with_params(['packagename1', 'packagename2']) }
 
@@ -36,14 +27,6 @@
     end
   end
 
-  context 'when given an empty packages array' do
-    let(:pre_condition) { 'notify { "hi": } -> Package <| |>; $somearray = ["vim",""]; ensure_packages($somearray)' }
-
-    describe 'after running ensure_package(["vim", ""])' do
-      it { expect { catalogue }.to raise_error(Puppet::ParseError, %r{Empty String provided}) }
-    end
-  end
-
   context 'when given hash of packages' do
     before(:each) do
       subject.execute({ 'foo' => { 'provider' => 'rpm' }, 'bar' => { 'provider' => 'gem' } }, 'ensure' => 'present')