-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
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 <coding@schilljs.com> | ||
* @copyright Copyright (c) 2016, ownCloud, Inc. | ||
* | ||
* @author Bernhard Posselt <dev@bernhard-posselt.com> | ||
|
@@ -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 | ||
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? ;) 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? 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 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 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 :( 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. 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 <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()); | ||
} | ||
} |
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.
: 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