-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat(appframework): ⌚ Make ITimeFactory extend \PSR\Clock\ClockInterface #35872
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| declare(strict_types=1); | ||
|
|
||
| /** | ||
| * @copyright Copyright (c) 2022, Joas Schilling <[email protected]> | ||
| * @copyright Copyright (c) 2016, ownCloud, Inc. | ||
| * | ||
| * @author Bernhard Posselt <[email protected]> | ||
|
|
@@ -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(); | ||
|
|
@@ -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 { | ||
| $clone = clone $this; | ||
| $clone->timezone = $timezone; | ||
|
|
||
| return $clone; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| declare(strict_types=1); | ||
|
|
||
| /** | ||
| * @copyright Copyright (c) 2022, Joas Schilling <[email protected]> | ||
| * @copyright Copyright (c) 2016, ownCloud, Inc. | ||
| * | ||
| * @author Bernhard Posselt <[email protected]> | ||
|
|
@@ -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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are you sure about that? ;)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. YOu mean extends vs implements?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yeah see comment above:
Sorry about that :(
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
| } | ||
| 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 <[email protected]> | ||
| * | ||
| * @author Joas Schilling <[email protected]> | ||
| * | ||
| * @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()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
: staticis 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