Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(appframework): ⌚ Make ITimeFactory extend \PSR\Clock\ClockInterface #35872

Merged
merged 1 commit into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion lib/private/AppFramework/Utility/TimeFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
declare(strict_types=1);

/**
* @copyright Copyright (c) 2022, Joas Schilling <coding@schilljs.com>
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
* @author Bernhard Posselt <dev@bernhard-posselt.com>
Expand Down Expand Up @@ -30,11 +31,23 @@
use OCP\AppFramework\Utility\ITimeFactory;

/**
* Needed to mock calls to time()
* Use this to get a timestamp or DateTime object in code to remain testable
*
* @since 8.0.0
* @since 26.0.0 Extends the \Psr\Clock\ClockInterface interface
* @ref https://www.php-fig.org/psr/psr-20/#21-clockinterface
*/
class TimeFactory implements ITimeFactory {
protected \DateTimeZone $timezone;

public function __construct() {
$this->timezone = new \DateTimeZone('UTC');
}

/**
* @return int the result of a call to time()
* @since 8.0.0
* @deprecated 26.0.0 {@see ITimeFactory::now()}
*/
public function getTime(): int {
return time();
Expand All @@ -45,8 +58,19 @@ public function getTime(): int {
* @param \DateTimeZone $timezone
* @return \DateTime
* @since 15.0.0
* @deprecated 26.0.0 {@see ITimeFactory::now()}
*/
public function getDateTime(string $time = 'now', \DateTimeZone $timezone = null): \DateTime {
return new \DateTime($time, $timezone);
}

public function now(): \DateTimeImmutable {
return new \DateTimeImmutable('now', $this->timezone);
}
public function withTimeZone(\DateTimeZone $timezone): static {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

: static is 8.0+ only, but currently linting is still run against 7.4 while we plan to drop it.
Could remove the typing for now, or we keep it and make sure the 7.4 lint job is removed already

$clone = clone $this;
$clone->timezone = $timezone;

return $clone;
}
}
1 change: 1 addition & 0 deletions lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,7 @@ public function __construct($webRoot, \OC\Config $config) {
$this->registerDeprecatedAlias('ControllerMethodReflector', \OCP\AppFramework\Utility\IControllerMethodReflector::class);

$this->registerAlias(\OCP\AppFramework\Utility\ITimeFactory::class, \OC\AppFramework\Utility\TimeFactory::class);
$this->registerAlias(\Psr\Clock\ClockInterface::class, \OCP\AppFramework\Utility\ITimeFactory::class);
/** @deprecated 19.0.0 */
$this->registerDeprecatedAlias('TimeFactory', \OCP\AppFramework\Utility\ITimeFactory::class);

Expand Down
22 changes: 19 additions & 3 deletions lib/public/AppFramework/Utility/ITimeFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
declare(strict_types=1);

/**
* @copyright Copyright (c) 2022, Joas Schilling <coding@schilljs.com>
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
* @author Bernhard Posselt <dev@bernhard-posselt.com>
Expand All @@ -26,22 +27,37 @@
*/
namespace OCP\AppFramework\Utility;

use Psr\Clock\ClockInterface;

/**
* Needed to mock calls to time()
* Use this to get a timestamp or DateTime object in code to remain testable
*
* @since 8.0.0
* @since 26.0.0 Extends the \Psr\Clock\ClockInterface interface
Copy link
Member

@ChristophWurst ChristophWurst Oct 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure about that? ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YOu mean extends vs implements?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure it's correct here, but yeah on the actual implementation the comment could be adjusted

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The since 26. I just had Mail integration tests fail because I used now for 26. It's only there for 27+ :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah see comment above:

Dang, this slipped through. To preserve stability in OCP, I'm moving it to 27

Sorry about that :(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix in #40865

* @ref https://www.php-fig.org/psr/psr-20/#21-clockinterface
*/
interface ITimeFactory {

interface ITimeFactory extends ClockInterface {
/**
* @return int the result of a call to time()
* @since 8.0.0
* @deprecated 26.0.0 {@see ITimeFactory::now()}
*/
public function getTime(): int;

/**
* @param string $time
* @param \DateTimeZone $timezone
* @param \DateTimeZone|null $timezone
* @return \DateTime
* @since 15.0.0
* @deprecated 26.0.0 {@see ITimeFactory::now()}
*/
public function getDateTime(string $time = 'now', \DateTimeZone $timezone = null): \DateTime;

/**
* @param \DateTimeZone $timezone
* @return static
* @since 26.0.0
*/
public function withTimeZone(\DateTimeZone $timezone): static;
}
49 changes: 49 additions & 0 deletions tests/lib/AppFramework/Utility/TimeFactoryTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2022, Joas Schilling <coding@schilljs.com>
*
* @author Joas Schilling <coding@schilljs.com>
*
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

namespace Test\AppFramework\Utility;

use OC\AppFramework\Utility\TimeFactory;

class TimeFactoryTest extends \Test\TestCase {
protected TimeFactory $timeFactory;

protected function setUp(): void {
$this->timeFactory = new TimeFactory();
}

public function testNow(): void {
$now = $this->timeFactory->now();
self::assertSame('UTC', $now->getTimezone()->getName());
}

public function testNowWithTimeZone(): void {
$timezone = new \DateTimeZone('Europe/Berlin');
$withTimeZone = $this->timeFactory->withTimeZone($timezone);

$now = $withTimeZone->now();
self::assertSame('Europe/Berlin', $now->getTimezone()->getName());
}
}