According to the change record change record
usage of global constant REQUEST_TIME and usage of $time = time(); should be replaced by using the service \Drupal::time()->getRequestTime(); added for the time.

$request_time = $_SERVER['REQUEST_TIME'];
$request_time = REQUEST_TIME;
$request_time = $requestStack->getCurrentRequest()->server->get('REQUEST_TIME');
$request_time = $request->server->get('REQUEST_TIME');

and most instances of
$time = time();
can be replaced with
$request_time = \Drupal::time()->getRequestTime();
or the equivalent using the injected service.

$request_time_float = $_SERVER['REQUEST_TIME_FLOAT'];
$request_time_float = $requestStack->getCurrentRequest()->server->get('REQUEST_TIME_FLOAT');
$request_time_float = $request->server->get('REQUEST_TIME_FLOAT');

and many instances of

$microtime = microtime();
$microtime = microtime(TRUE);

can be replaced with
$request_time = \Drupal::time()->getRequestMicroTime();
or the equivalent using the injected service.
$time = time();
can be replaced with
$request_time = \Drupal::time()->getCurrentTime();

$microtime = microtime();
$microtime = microtime(TRUE);

can be replaced with
$request_time = \Drupal::time()->getCurrentMicroTime();
or the equivalent using the injected service.

CommentFileSizeAuthor
#23 2820345-23.patch5.74 KBjofitz
#23 interdiff-21-23.txt3.77 KBjofitz
#21 2820345-21.patch2.49 KBjofitz
#15 test_for_time_service_injection_2820345-2.patch2.36 KBlomasr
#13 test_for_time_service_injection_2820345.patch2.36 KBlomasr
#5 2820345_4.patch725 byteslomasr

Comments

Prashant.c created an issue. See original summary.

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented
Issue summary: View changes
prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented
catch’s picture

catch
he/him
Primary language English
commented
Category: Feature request » Task
Priority: Critical » Normal
Issue tags: -core

We should do this, but it's a normal task rather than critical feature request :)

lomasr’s picture

lomasr commented
StatusFileSize
new 2820345_4.patch725 bytes

Added a patch for core migrate module. Replaced time() with \Drupal::time()->getRequestTime()

lomasr’s picture

lomasr commented
Status: Active » Needs review
lomasr’s picture

lomasr commented

Replacing all of them can take some time, so I have created a sub issue

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@lomasr before splitting this in many separate issues we need to consider what is the best scope. "Scope" means the set of changes included in an issue. See https://www.drupal.org/core/scope for guidelines and examples for Drupal core issue scope.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Since this can be done with a find and replace to start it probably does not make sense to splitting into sub-issues. Yes, sometimes the service will need injecting but I still think that a single should suffice.

lomasr’s picture

lomasr commented

@alexpott Thanks I will go through the link that you have suggested before adding new issues.

lomasr’s picture

lomasr commented

You are right splitting this into child issues does not make sense. Lets add patch to this issue only.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

One scope suggestion is to deal with time() and REQUEST_TIME in different issues. Also though we need to be careful when we use the service. In a test there is no point using the time service to get the time... ie. consider the following code:

    // Visit User Info page before updating translation strings. Change the
    // created time to ensure that the we're dealing in seconds and it can't be
    // exactly 1 minute.
    $this->adminUser->set('created', time() - 1)->save();

This is from LocalePluralFormatTest and it should not be changed.

lomasr’s picture

lomasr commented
StatusFileSize
new test_for_time_service_injection_2820345.patch2.36 KB

This is my first time to try to inject a service in Drupal 8 . Adding a test patch in order to continue injecting time services correctly in drupal core. Please review. Thanks.

Status: Needs review » Needs work

The last submitted patch, 13: test_for_time_service_injection_2820345.patch, failed testing.

lomasr’s picture

lomasr commented
StatusFileSize
new test_for_time_service_injection_2820345-2.patch2.36 KB

Adding the patch to test the injection of time service.

lomasr’s picture

lomasr commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 15: test_for_time_service_injection_2820345-2.patch, failed testing.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joachim’s picture

joachim commented
Title: Replace usage of REQUEST_TIME by using Time Service » Replace usage of REQUEST_TIME by using Time Service in Migrate
Component: other » migration system
Parent issue: » #2902895: [meta][no patch] Replace uses of REQUEST_TIME and time() with time service
jofitz’s picture

jofitz
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new 2820345-21.patch2.49 KB

Re-roll (hence no interdiff) and corrections to proof-of-concept dependency injection.

Status: Needs review » Needs work

The last submitted patch, 21: 2820345-21.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jofitz’s picture

jofitz
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-21-23.txt3.77 KB
new 2820345-23.patch5.74 KB

Address coding standard errors and test failures from #21.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Closed (won't fix)

This fix should get picked up by a much more broad, replace all deprecated REQUEST_TIME issue.