Deprecate time function

`time` was both an old API ruby function, and not needed since Puppet 4. In this commit, the `parser` function is removed and replaced by a non-namespaced APIv4 function. This is done to preserve compatibility before the function is removed completely in a later release. The ruby function calls a Puppet language function which implements the existing behaviour using core Puppet functionality. The original function did something weird with an optional `timezone` parameter. Nobody really remembers why. It appears that very early versions of Ruby may have been buggy and needed this as a workaround. To be clear, seconds since the epoch are the same _everywhere_. Timezone is not relevant.
puppetlabs · Feb 7, 2024 · fea0881 · fea0881
1 parent 7e7ded4
commit fea0881
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 109 deletions.
diff --git a/REFERENCE.md b/REFERENCE.md
@@ -42,7 +42,6 @@ from an array or key from a hash.
 * [`delete_undef_values`](#delete_undef_values): Returns a copy of input hash or array with all undefs deleted.
 * [`delete_values`](#delete_values): Deletes all instances of a given value from a hash.
 * [`deprecation`](#deprecation): Function to print deprecation warnings, Logs a warning once for a given key.
-* [`deprecation`](#deprecation): Function to print deprecation warnings (this is the 3.X version of it).
 * [`difference`](#difference): This function returns the difference between two arrays.
 * [`dirname`](#dirname): Returns the dirname of a path.
 * [`dos2unix`](#dos2unix): Returns the Unix version of the given string.
@@ -142,6 +141,7 @@ Puppet structure
 * [`stdlib::shell_escape`](#stdlib--shell_escape): Escapes a string so that it can be safely used in a Bourne shell command line.
 * [`stdlib::start_with`](#stdlib--start_with): Returns true if str starts with one of the prefixes given. Each of the prefixes should be a String.
 * [`stdlib::str2resource`](#stdlib--str2resource): This converts a string to a puppet resource.
+* [`stdlib::time`](#stdlib--time): This function is deprecated. It implements the functionality of the original non-namespaced stdlib `time` function.
 * [`stdlib::to_json`](#stdlib--to_json): Convert a data structure and output to JSON
 * [`stdlib::to_json_pretty`](#stdlib--to_json_pretty): Convert data structure and output to pretty JSON
 * [`stdlib::to_python`](#stdlib--to_python): Convert an object into a String containing its Python representation
@@ -161,7 +161,7 @@ OS X versions >= 10.7).
 * [`suffix`](#suffix): This function applies a suffix to all elements in an array, or to the keys
 in a hash.
 * [`swapcase`](#swapcase): This function will swap the existing case of a string.
-* [`time`](#time): This function will return the current time since epoch as an integer.
+* [`time`](#time): DEPRECATED.  Use the native Puppet fuctionality instead of this function. eg `Integer(Timestamp().strftime('%s'))`
 * [`to_bytes`](#to_bytes): Converts the argument into bytes, for example 4 kB becomes 4096.
 * [`to_json`](#to_json): DEPRECATED.  Use the namespaced function [`stdlib::to_json`](#stdlibto_json) instead.
 * [`to_json_pretty`](#to_json_pretty): DEPRECATED.  Use the namespaced function [`stdlib::to_json_pretty`](#stdlibto_json_pretty) instead.
@@ -1480,20 +1480,6 @@ When `true`, (the default), the function is affected by the puppet setting 'stri
 (outputs as an error message), :off (no message / error is displayed) and :warning
 (default, outputs a warning).
 
-### <a name="deprecation"></a>`deprecation`
-
-Type: Ruby 3.x API
-
-The uniqueness key - can appear once. The msg is the message text including any positional
-information that is formatted by the user/caller of the method.).
-
-#### `deprecation()`
-
-The uniqueness key - can appear once. The msg is the message text including any positional
-information that is formatted by the user/caller of the method.).
-
-Returns: `String` return deprecation warnings
-
 ### <a name="difference"></a>`difference`
 
 Type: Ruby 3.x API
@@ -4111,6 +4097,24 @@ Data type: `String`
 
 The string to lookup as a resource
 
+### <a name="stdlib--time"></a>`stdlib::time`
+
+Type: Puppet Language
+
+It is provided for compatability, but users should use the native time related functions directly.
+
+#### `stdlib::time(Optional[String] $_timezone = undef)`
+
+It is provided for compatability, but users should use the native time related functions directly.
+
+Returns: `Integer`
+
+##### `_timezone`
+
+Data type: `Optional[String]`
+
+This parameter doesn't do anything, but exists for compatability reasons
+
 ### <a name="stdlib--to_json"></a>`stdlib::to_json`
 
 Type: Ruby 4.x API
@@ -4876,43 +4880,21 @@ Would result in: "AbCd"
 
 ### <a name="time"></a>`time`
 
-Type: Ruby 3.x API
-
-> *Note:* that since Puppet 4.8.0 the Puppet language has the data types Timestamp (a point in time) and
-Timespan (a duration). The following example is equivalent to calling time() without
-any arguments:
-
-```Timestamp()```
-
-#### Examples
-
-##### **Usage**
-
-```puppet
-
-time()
-Will return something like: 1311972653
-```
+Type: Ruby 4.x API
 
-#### `time()`
+DEPRECATED.  Use the native Puppet fuctionality instead of this function. eg `Integer(Timestamp().strftime('%s'))`
 
-> *Note:* that since Puppet 4.8.0 the Puppet language has the data types Timestamp (a point in time) and
-Timespan (a duration). The following example is equivalent to calling time() without
-any arguments:
+#### `time(Any *$args)`
 
-```Timestamp()```
+The time function.
 
-Returns: `Any` the current time since epoch as an integer.
+Returns: `Any`
 
-##### Examples
+##### `*args`
 
-###### **Usage**
+Data type: `Any`
 
-```puppet
 
-time()
-Will return something like: 1311972653
-```
 
 ### <a name="to_bytes"></a>`to_bytes`
 

diff --git a/functions/time.pp b/functions/time.pp
@@ -0,0 +1,11 @@
+# @summary This function is deprecated. It implements the functionality of the original non-namespaced stdlib `time` function.
+#
+# It is provided for compatability, but users should use the native time related functions directly.
+#
+# @param _timezone
+#   This parameter doesn't do anything, but exists for compatability reasons
+function stdlib::time(Optional[String] $_timezone = undef) >> Integer {
+  # Note the `timezone` parameter doesn't do anything and didn't in the ruby implementation for _years_ (pre 1.8.7 perhaps ???)
+  deprecation('time', 'The stdlib `time` function is deprecated. Please direcly use native Puppet functionality instead. eg. `Integer(Timestamp().strftime(\'%s\'))`', false)
+  Integer(Timestamp().strftime('%s'))
+}
diff --git a/lib/puppet/functions/time.rb b/lib/puppet/functions/time.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+# @summary DEPRECATED.  Use the native Puppet fuctionality instead of this function. eg `Integer(Timestamp().strftime('%s'))`
+Puppet::Functions.create_function(:time) do
+  dispatch :call_puppet_function do
+    repeated_param 'Any', :args
+  end
+  def call_puppet_function(*args)
+    # Note, `stdlib::time` calls `deprecation`, so we don't also do that here.
+    call_function('stdlib::time', *args)
+  end
+end
diff --git a/lib/puppet/parser/functions/time.rb b/lib/puppet/parser/functions/time.rb
diff --git a/spec/functions/time_spec.rb b/spec/functions/time_spec.rb
@@ -4,21 +4,20 @@
 
 describe 'time' do
   it { is_expected.not_to be_nil }
-  it { is_expected.to run.with_params('a', '').and_raise_error(Puppet::ParseError, %r{wrong number of arguments}i) }
 
   context 'when running at a specific time' do
     before(:each) do
       # get a value before stubbing the function
       test_time = Time.utc(2006, 10, 13, 8, 15, 11)
-      allow(Time).to receive(:new).with(no_args).and_return(test_time).once
+      allow(Time).to receive(:now).and_return(test_time)
     end
 
     it { is_expected.to run.with_params.and_return(1_160_727_311) }
     it { is_expected.to run.with_params('').and_return(1_160_727_311) }
-    it { is_expected.to run.with_params([]).and_return(1_160_727_311) }
-    it { is_expected.to run.with_params({}).and_return(1_160_727_311) }
-    it { is_expected.to run.with_params('foo').and_return(1_160_727_311) }
-    it { is_expected.to run.with_params('UTC').and_return(1_160_727_311) }
-    it { is_expected.to run.with_params('America/New_York').and_return(1_160_727_311) }
+
+    describe('Timezone is irrelevant') do
+      it { is_expected.to run.with_params('UTC').and_return(1_160_727_311) }
+      it { is_expected.to run.with_params('America/New_York').and_return(1_160_727_311) }
+    end
   end
 end