Replace REQUEST_TIME in classes with direct container access

All the changes in this patch look good. Just one remark:

+++ b/core/modules/system/system.install
@@ -2579,3 +2579,7 @@ function system_update_8901() {
+
+function system_update_8902() {
+  // Nop for testing purposes to force a container rebuild.
+}

Why is this change necessary for this patch?

I have added a patch but not sure if it is enough or not.

Here I have removed the method system_update_8902() because I think it doesn't do anything.

Here again, we need to review the new patch @daffie.

All the code changes look good.
For me it is RTBC.

+++ b/core/modules/aggregator/src/Controller/AggregatorController.php
@@ -21,14 +22,24 @@ class AggregatorController extends ControllerBase {
+    $this->time = $time ?: \Drupal::service('datetime.time');

Do we need to trigger deprecation notices where constructor arguments have been added?

This needs to be done in D9 first. The patch doesn't apply there.

And the answer to #19 is yes which makes this issue 9.1.x really. Especially as REQUEST_TIME is hanging about till D10.

Here I have added patches for Drupal 8.9.x and Drupal 9.0.x

I have added drupal's version in patch name.

@ravi.shankar: Could you post an interdiff.txt file for the changes you have made (for the 9.0 branch as 8.9 branch will never be committed). Thank you for working on this issue.

See possibly related issues noted here:

https://www.drupal.org/project/drupal/issues/3112283#comment-13605217

+++ b/core/modules/aggregator/src/Controller/AggregatorController.php
@@ -21,14 +22,24 @@ class AggregatorController extends ControllerBase {
+    $this->time = $time ?: \Drupal::service('datetime.time');

this kind of changes require deprecation error about passing null and to be cleared in next major

I think part of this changes could be reverted to use request time from current request

+++ b/core/modules/aggregator/src/Controller/AggregatorController.php
@@ -21,14 +22,24 @@ class AggregatorController extends ControllerBase {
+    $this->time = $time ?: \Drupal::service('datetime.time');

+++ b/core/modules/content_translation/src/ContentTranslationHandler.php
@@ -127,6 +137,7 @@ public function __construct(EntityTypeInterface $entity_type, LanguageManagerInt
+    $this->time = $time ?: \Drupal::service('datetime.time');

+++ b/core/modules/system/src/DateFormatListBuilder.php
@@ -32,11 +40,14 @@ class DateFormatListBuilder extends ConfigEntityListBuilder {
+    $this->time = $time ?: \Drupal::service('datetime.time');

+++ b/core/modules/user/src/Controller/UserController.php
@@ -70,13 +78,16 @@ class UserController extends ControllerBase {
+    $this->time = $time ?: \Drupal::service('datetime.time');

+++ b/core/modules/views/src/Plugin/views/argument/Date.php
@@ -70,12 +78,15 @@ class Date extends Formula implements ContainerFactoryPluginInterface {
+    $this->time = $time ?: \Drupal::service('datetime.time');

+++ b/core/modules/views/src/Plugin/views/cache/Time.php
@@ -53,10 +61,13 @@ class Time extends CachePluginBase {
+    $this->time = $time ?: \Drupal::service('datetime.time');

+++ b/core/modules/views/src/Plugin/views/field/Date.php
@@ -44,12 +52,15 @@ class Date extends FieldPluginBase {
+    $this->time = $time ?: \Drupal::service('datetime.time');

should use injection deprecation to explain that service will be required for injection

Rerolled patch against 9.5

Rerolled patch against 9.5

I'd suggest to target D10 first since this impacts the PHPStan baseline, and then backport.

Uploading a rerolled patch to 10.0.x version, thanks!

The testbot is not happy. See: https://www.drupal.org/pift-ci-job/2393088

Confirmed with @catch what branch this should be against: https://drupal.slack.com/archives/C014CT1CN1M/p1659679942164229

10.1.x it is.

How painful, it’s way more deprecation tests than actual changes… some comments in MR

some digging in the MR

One consideration on whether we can update the property definition with readonly and remove the @var tag - that repeats several times. I see there are alredy some cases where this was committed but would not like to get into coding standards discussions.

Three points I think need more work.

- Updated IS
- Updated CR
- Addressed threads from review by @mondrake (thanks!)
- Rebased MR on 11.x

All good for me, thanks. There are also deprecation tests for missing constructor arguments, which I think are no longer required, but it's a plus.

I'm triaging RTBC issues. I read the IS and the comments. I didn't find any unanswered questions.

I skimmed the patch and found quite a few out of scope formatting changes which I think these are limited to changing the _construct methods to be multi line. And this change is inconsistent. I then looked at the size of the change and determined it is 65K and, 29 files + 681 − 116 lines. That is well above the recommended patch size of 10-40K. This likely needs to be split into two issues.

I am setting to NW and assigning to myself to remove the out of scope changes to see how much that helps. I will get back to this after a short break.

Oh, I also updated credit.

This needed a rebase which I did. I then had to restore a .js file, core/modules/ckeditor5/js/build/drupalMedia.js from 11.x, which I somehow got wrong in the rebase.

Removing the out of scope changes is not sufficient. I am going to look at splitting this into two issues while tests run for the rebase.

It is still rather large but I hope this is a bit easier to review now without the plugins. I have updated the title and the CR.

Do we need a followup for the plugins?

Also would it be worth it to create a trait for time? If not changes in scope seem good.

Discussed all the new tests added here with @catch. There's no need to add the constructor tests here. They don't really test anything.

I've updated the MR to deprecate for 10.3.0 and to use autowiring instead of create where this issue was adding a create.

Committed a9b4000 and pushed to 11.x. Thanks!

After discussion with @catch I decided to delete the change record because it's not adding anything beyond the deprecation message and the constructor documentation.