Issue #3371840 by mondrake, Spokje: Time::getRequestTime is not immutable when there is no request

core/lib/Drupal/Component/Datetime/Time.php

@@ -6,23 +6,32 @@
 
 /**
  * Provides a class for obtaining system time.
+ *
+ * While the normal use case of this class expects that a Request object is
+ * available from the RequestStack, it is still possible to use it without, for
+ * example for early bootstrap containers or for unit tests. In those cases,
+ * the class will access global variables or set a proxy request time in order
+ * to return the request time.
  */
 class Time implements TimeInterface {
 
   /**
    * The request stack.
-   *
-   * @var \Symfony\Component\HttpFoundation\RequestStack
    */
-  protected $requestStack;
+  protected ?RequestStack $requestStack;
+
+  /**
+   * A proxied request time if the request time is not available.
+   */
+  protected float $proxyRequestTime;
 
   /**
    * Constructs a Time object.
    *
-   * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack
-   *   The request stack.
+   * @param \Symfony\Component\HttpFoundation\RequestStack|null $request_stack
+   *   (Optional) The request stack.
    */
-  public function __construct(RequestStack $request_stack) {
+  public function __construct(RequestStack $request_stack = NULL) {
     $this->requestStack = $request_stack;
   }
 
@@ -30,26 +39,26 @@ public function __construct(RequestStack $request_stack) {
    * {@inheritdoc}
    */
   public function getRequestTime() {
-    $request = $this->requestStack->getCurrentRequest();
+    $request = $this->requestStack ? $this->requestStack->getCurrentRequest() : NULL;
     if ($request) {
       return $request->server->get('REQUEST_TIME');
     }
     // If this is called prior to the request being pushed to the stack fallback
     // to built-in globals (if available) or the system time.
-    return $_SERVER['REQUEST_TIME'] ?? $this->getCurrentTime();
+    return $_SERVER['REQUEST_TIME'] ?? $this->getProxyRequestTime();
   }
 
   /**
    * {@inheritdoc}
    */
   public function getRequestMicroTime() {
-    $request = $this->requestStack->getCurrentRequest();
+    $request = $this->requestStack ? $this->requestStack->getCurrentRequest() : NULL;
     if ($request) {
       return $request->server->get('REQUEST_TIME_FLOAT');
     }
     // If this is called prior to the request being pushed to the stack fallback
     // to built-in globals (if available) or the system time.
-    return $_SERVER['REQUEST_TIME_FLOAT'] ?? $this->getCurrentMicroTime();
+    return $_SERVER['REQUEST_TIME_FLOAT'] ?? $this->getProxyRequestMicroTime();
   }
 
   /**
@@ -66,4 +75,30 @@ public function getCurrentMicroTime() {
     return microtime(TRUE);
   }
 
+  /**
+   * Returns a mimic of the timestamp of the current request.
+   *
+   * @return int
+   *   A value returned by time().
+   */
+  protected function getProxyRequestTime(): int {
+    if (!isset($this->proxyRequestTime)) {
+      $this->proxyRequestTime = $this->getCurrentMicroTime();
+    }
+    return (int) $this->proxyRequestTime;
+  }
+
+  /**
+   * Returns a mimic of the timestamp of the current request.
+   *
+   * @return float
+   *   A value returned by microtime().
+   */
+  protected function getProxyRequestMicroTime(): float {
+    if (!isset($this->proxyRequestTime)) {
+      $this->proxyRequestTime = $this->getCurrentMicroTime();
+    }
+    return $this->proxyRequestTime;
+  }
+
 }

core/tests/Drupal/Tests/Component/Datetime/TimeTest.php

@@ -83,7 +83,9 @@ public function testGetRequestMicroTime() {
    * @covers ::getRequestTime
    */
   public function testGetRequestTimeNoRequest() {
-    $expected = 12345678;
+    // With no request, and no global variable, we expect to get the int part
+    // of the microtime.
+    $expected = 1234567;
     unset($_SERVER['REQUEST_TIME']);
     $this->assertEquals($expected, $this->time->getRequestTime());
     $_SERVER['REQUEST_TIME'] = 23456789;

core/tests/Drupal/Tests/Component/Datetime/TimeWithNoRequestTest.php

+<?php
+
+namespace Drupal\Tests\Component\Datetime;
+
+use Drupal\Component\Datetime\Time;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Tests that getRequest(Micro)Time works when no underlying request exists.
+ *
+ * @coversDefaultClass \Drupal\Component\Datetime\Time
+ * @group Datetime
+ * @runTestsInSeparateProcesses
+ * @preserveGlobalState disabled
+ */
+class TimeWithNoRequestTest extends TestCase {
+
+  /**
+   * The time class for testing.
+   */
+  protected Time $time;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    parent::setUp();
+
+    // We need to explicitly unset the $_SERVER variables, so that Time is
+    // forced to look for current time.
+    unset($_SERVER['REQUEST_TIME']);
+    unset($_SERVER['REQUEST_TIME_FLOAT']);
+
+    $this->time = new Time();
+  }
+
+  /**
+   * Tests the getRequestTime method.
+   *
+   * @covers ::getRequestTime
+   */
+  public function testGetRequestTimeImmutable(): void {
+    $requestTime = $this->time->getRequestTime();
+    sleep(2);
+    $this->assertSame($requestTime, $this->time->getRequestTime());
+  }
+
+  /**
+   * Tests the getRequestMicroTime method.
+   *
+   * @covers ::getRequestMicroTime
+   */
+  public function testGetRequestMicroTimeImmutable() {
+    $requestTime = $this->time->getRequestMicroTime();
+    usleep(20000);
+    $this->assertSame($requestTime, $this->time->getRequestMicroTime());
+  }
+
+}