Skip to content

Commit 9e96391

Browse files
icewind1991icewind1991
committed
refactor: use DateTimeImmutable in unified sharing api instead of timestamp
Signed-off-by: Robin Appelman <robin@icewind.nl> # Conflicts: # apps/files/tests/Sharing/Source/NodeShareSourceTypeTest.php
1 parent f8f6e27 commit 9e96391

17 files changed

Lines changed: 251 additions & 249 deletions

‎apps/files/tests/Sharing/Source/NodeShareSourceTypeTest.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use NCU\Sharing\ShareAccessContext;
1414
use NCU\Sharing\Source\ShareSource;
1515
use OC\Files\Filesystem;
16+
use OC\Sharing\SharingManager;
1617
use OCA\Files\Sharing\Source\NodeShareSourceType;
1718
use OCP\EventDispatcher\IEventDispatcher;
1819
use OCP\Files\Cache\IFileAccess;
@@ -111,14 +112,14 @@ public function testDelete(): void {
111112
$this->manager->addShareSource($accessContext, $id, new ShareSource($this->sourceType::class, (string)$this->node->getId()));
112113
$this->dbConnection->commit();
113114

114-
$before = $this->manager->generateTimestamp();
115+
$before = $this->manager->getTime();
115116
$this->node->delete();
116-
$after = $this->manager->generateTimestamp();
117+
$after = $this->manager->getTime();
117118

118119
$this->dbConnection->beginTransaction();
119120
$share = $this->manager->getShare($accessContext, $id);
120-
$this->assertGreaterThanOrEqual($before, $share->lastUpdated);
121-
$this->assertLessThanOrEqual($after, $share->lastUpdated);
121+
$this->assertGreaterThanOrEqual(SharingManager::timeToMs($before), SharingManager::timeToMs($share->lastUpdated));
122+
$this->assertLessThanOrEqual(SharingManager::timeToMs($after), SharingManager::timeToMs($share->lastUpdated));
122123
$this->assertEquals([], $share->sources);
123124

124125
$this->manager->deleteShare($accessContext, $id);

‎lib/private/Sharing/SharingBackend.php‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,15 @@ public function __construct(
5959
}
6060

6161
#[\Override]
62-
public function createShare(string $id, ShareUser $owner, int $lastUpdated): void {
62+
public function createShare(string $id, ShareUser $owner, \DateTimeImmutable $lastUpdated): void {
6363
$qb = $this->connection->getQueryBuilder();
6464
$qb
6565
->insert('sharing_share')
6666
->values([
6767
'id' => $qb->createNamedParameter($id),
6868
'owner_user_id' => $qb->createNamedParameter($owner->userId),
6969
'owner_instance' => $qb->createNamedParameter($owner->instance),
70-
'last_updated' => $qb->createNamedParameter($lastUpdated),
70+
'last_updated' => $qb->createNamedParameter(SharingManager::timeToMs($lastUpdated)),
7171
'state' => $qb->createNamedParameter(ShareState::Draft->value),
7272
])
7373
->executeStatement();
@@ -534,13 +534,13 @@ public function getShareOwner(string $id): ShareUser {
534534
* @param non-empty-list<string> $ids
535535
*/
536536
#[\Override]
537-
public function setLastUpdated(array $ids, int $lastUpdated): void {
537+
public function setLastUpdated(array $ids, \DateTimeImmutable $lastUpdated): void {
538538
foreach (array_chunk($ids, 1000) as $chunk) {
539539
$qb = $this->connection->getQueryBuilder();
540540

541541
$rowCount = $qb
542542
->update('sharing_share')
543-
->set('last_updated', $qb->createNamedParameter($lastUpdated, IQueryBuilder::PARAM_INT))
543+
->set('last_updated', $qb->createNamedParameter(SharingManager::timeToMs($lastUpdated), IQueryBuilder::PARAM_INT))
544544
->where($qb->expr()->in('id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)))
545545
->executeStatement();
546546
if ($rowCount !== count($chunk)) {
@@ -979,7 +979,7 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID,
979979
$shares = array_map(static fn (array $share): Share => new Share(
980980
$share['id'],
981981
$share['owner'],
982-
$share['last_updated'],
982+
self::parseTimestamp($share['last_updated']),
983983
$share['state'],
984984
$share['sources'],
985985
$share['recipients'],
@@ -1031,4 +1031,18 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID,
10311031

10321032
return array_values($shares);
10331033
}
1034+
1035+
private static function parseTimestamp(int $timestampMs): \DateTimeImmutable {
1036+
if (method_exists(\DateTimeImmutable::class, 'createFromTimestamp')) {
1037+
// with php 8.3 the method doesn't exist and psalm doesn't know the return type
1038+
/** @psalm-suppress MixedReturnStatement */
1039+
return \DateTimeImmutable::createFromTimestamp((float)$timestampMs / 1000.0);
1040+
}
1041+
$time = \DateTimeImmutable::createFromFormat('U.u', (string)((float)$timestampMs / 1000.0));
1042+
if ($time === false) {
1043+
throw new \RuntimeException('Invalid timestamp for share');
1044+
}
1045+
1046+
return $time;
1047+
}
10341048
}

‎lib/private/Sharing/SharingManager.php‎

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
use OCP\Security\ISecureRandom;
4343
use OCP\Snowflake\ISnowflakeGenerator;
4444
use OCP\User\Events\BeforeUserDeletedEvent;
45+
use Psr\Clock\ClockInterface;
4546
use Random\Randomizer;
4647
use RuntimeException;
4748

@@ -70,6 +71,7 @@ public function __construct(
7071
private IDBConnection $dbConnection,
7172
private ISharingRegistry $registry,
7273
IAppConfig $appConfig,
74+
private ClockInterface $clock,
7375
) {
7476
$this->randomizer = new Randomizer();
7577
$this->l10n = $l10nFactory->get('sharing');
@@ -140,13 +142,8 @@ public function generateSecret(): string {
140142
}
141143

142144
#[\Override]
143-
public function generateTimestamp(): int {
144-
$time = (int)(microtime(true) * 1000.0);
145-
if ($time < 0) {
146-
throw new RuntimeException('Have you invented time travel?');
147-
}
148-
149-
return $time;
145+
public function getTime(): \DateTimeImmutable {
146+
return $this->clock->now();
150147
}
151148

152149
#[\Override]
@@ -158,7 +155,7 @@ public function createShare(ShareAccessContext $accessContext): string {
158155
$this->assertInTransaction();
159156

160157
$id = $this->snowflakeGenerator->nextId();
161-
$lastUpdated = $this->generateTimestamp();
158+
$lastUpdated = $this->getTime();
162159
$this->backend->createShare($id, new ShareUser($currentUser->getUID(), null), $lastUpdated);
163160

164161
$this->processShareUpdates([$id]);
@@ -190,7 +187,7 @@ public function onOwnerDeleted(ShareAccessContext $accessContext, ShareUser $own
190187
public function updateShareState(ShareAccessContext $accessContext, string $id, ShareState $state): void {
191188
$this->assertInTransaction();
192189

193-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
190+
$this->backend->setLastUpdated([$id], $this->getTime());
194191

195192
$owner = $this->backend->getShareOwner($id);
196193
$this->validateShareOwnerOperation($accessContext, $owner);
@@ -209,7 +206,7 @@ public function updateShareState(ShareAccessContext $accessContext, string $id,
209206
public function addShareSource(ShareAccessContext $accessContext, string $id, ShareSource $source): void {
210207
$this->assertInTransaction();
211208

212-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
209+
$this->backend->setLastUpdated([$id], $this->getTime());
213210

214211
$owner = $this->backend->getShareOwner($id);
215212
$this->validateShareOwnerOperation($accessContext, $owner);
@@ -250,7 +247,7 @@ public function addShareSource(ShareAccessContext $accessContext, string $id, Sh
250247
public function removeShareSource(ShareAccessContext $accessContext, string $id, ShareSource $source): void {
251248
$this->assertInTransaction();
252249

253-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
250+
$this->backend->setLastUpdated([$id], $this->getTime());
254251

255252
$owner = $this->backend->getShareOwner($id);
256253
$this->validateShareOwnerOperation($accessContext, $owner);
@@ -268,7 +265,7 @@ public function onSourceDeleted(ShareAccessContext $accessContext, ShareSource $
268265

269266
$this->assertInTransaction();
270267

271-
$timestamp = $this->generateTimestamp();
268+
$timestamp = $this->getTime();
272269

273270
$updatedIds = $this->backend->onSourceDeleted($source);
274271
if ($updatedIds === []) {
@@ -288,7 +285,7 @@ public function addShareRecipient(ShareAccessContext $accessContext, string $id,
288285

289286
$this->assertInTransaction();
290287

291-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
288+
$this->backend->setLastUpdated([$id], $this->getTime());
292289

293290
$owner = $this->backend->getShareOwner($id);
294291

@@ -351,7 +348,7 @@ public function removeShareRecipient(ShareAccessContext $accessContext, string $
351348

352349
$this->assertInTransaction();
353350

354-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
351+
$this->backend->setLastUpdated([$id], $this->getTime());
355352

356353
$owner = $this->backend->getShareOwner($id);
357354

@@ -376,7 +373,7 @@ public function onRecipientDeleted(ShareAccessContext $accessContext, ShareRecip
376373

377374
$this->assertInTransaction();
378375

379-
$timestamp = $this->generateTimestamp();
376+
$timestamp = $this->getTime();
380377

381378
$updatedIds = $this->backend->onRecipientDeleted($recipient);
382379
if ($updatedIds === []) {
@@ -396,7 +393,7 @@ public function onInitiatorDeleted(ShareAccessContext $accessContext, ShareUser
396393

397394
$this->assertInTransaction();
398395

399-
$timestamp = $this->generateTimestamp();
396+
$timestamp = $this->getTime();
400397

401398
$updatedIds = $this->backend->onInitiatorDeleted($initiator);
402399
if ($updatedIds === []) {
@@ -412,7 +409,7 @@ public function onInitiatorDeleted(ShareAccessContext $accessContext, ShareUser
412409
public function updateShareRecipientSecret(ShareAccessContext $accessContext, string $id, ShareRecipient $recipient, string $secret): void {
413410
$this->assertInTransaction();
414411

415-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
412+
$this->backend->setLastUpdated([$id], $this->getTime());
416413

417414
$owner = $this->backend->getShareOwner($id);
418415

@@ -444,7 +441,7 @@ public function updateShareRecipientSecret(ShareAccessContext $accessContext, st
444441
public function createSharePropertyDefaultValue(Share $share, string $propertyTypeClass): Share {
445442
$this->assertInTransaction();
446443

447-
$timestamp = $this->generateTimestamp();
444+
$timestamp = $this->getTime();
448445
$this->backend->setLastUpdated([$share->id], $timestamp);
449446

450447
if (($propertyType = $this->registry->getPropertyTypes()[$propertyTypeClass] ?? null) === null) {
@@ -478,7 +475,7 @@ public function createSharePropertyDefaultValue(Share $share, string $propertyTy
478475
public function updateShareProperty(ShareAccessContext $accessContext, string $id, ShareProperty $property): void {
479476
$this->assertInTransaction();
480477

481-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
478+
$this->backend->setLastUpdated([$id], $this->getTime());
482479

483480
$owner = $this->backend->getShareOwner($id);
484481
$this->validateShareOwnerOperation($accessContext, $owner);
@@ -503,7 +500,7 @@ public function updateShareProperty(ShareAccessContext $accessContext, string $i
503500
public function createSharePermissionDefaultValue(Share $share, string $permissionTypeClass): Share {
504501
$this->assertInTransaction();
505502

506-
$timestamp = $this->generateTimestamp();
503+
$timestamp = $this->getTime();
507504
$this->backend->setLastUpdated([$share->id], $timestamp);
508505

509506
if (($permissionType = $this->registry->getPermissionTypes()[$permissionTypeClass] ?? null) === null) {
@@ -537,7 +534,7 @@ public function createSharePermissionDefaultValue(Share $share, string $permissi
537534
public function updateSharePermission(ShareAccessContext $accessContext, string $id, SharePermission $permission): void {
538535
$this->assertInTransaction();
539536

540-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
537+
$this->backend->setLastUpdated([$id], $this->getTime());
541538

542539
$owner = $this->backend->getShareOwner($id);
543540
$this->validateShareOwnerOperation($accessContext, $owner);
@@ -574,7 +571,7 @@ public function updateSharePermission(ShareAccessContext $accessContext, string
574571
public function selectSharePermissionPreset(ShareAccessContext $accessContext, string $id, string $permissionPresetClass): void {
575572
$this->assertInTransaction();
576573

577-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
574+
$this->backend->setLastUpdated([$id], $this->getTime());
578575

579576
$owner = $this->backend->getShareOwner($id);
580577
$this->validateShareOwnerOperation($accessContext, $owner);
@@ -826,4 +823,23 @@ private function processShareUpdates(array $sharesOrIds): array {
826823

827824
return $shares;
828825
}
826+
827+
/**
828+
* @return non-negative-int
829+
*/
830+
public static function timeToMs(\DateTimeImmutable $time): int {
831+
if (method_exists($time, 'getMicrosecond')) {
832+
/** @var int $micros */
833+
$micros = $time->getMicrosecond();
834+
} else {
835+
$micros = (int)$time->format('u');
836+
}
837+
838+
$time = $time->getTimestamp() * 1000 + (int)floor($micros / 1000);
839+
if ($time > 0) {
840+
return $time;
841+
}
842+
843+
throw new \RuntimeException('invalid date-time');
844+
}
829845
}

‎lib/unstable/Sharing/ISharingBackend.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ interface ISharingBackend {
2929
*
3030
* @experimental 35.0.0
3131
*/
32-
public function createShare(string $id, ShareUser $owner, int $lastUpdated): void;
32+
public function createShare(string $id, ShareUser $owner, \DateTimeImmutable $lastUpdated): void;
3333

3434
/**
3535
* Perform all updates when the owner was deleted.
@@ -200,9 +200,8 @@ public function getShareOwner(string $id): ShareUser;
200200
* Set the last updated timestamp for multiple shares.
201201
*
202202
* @param non-empty-list<string> $ids
203-
* @param non-negative-int $lastUpdated
204203
* @throws ShareNotFoundException
205204
* @experimental 35.0.0
206205
*/
207-
public function setLastUpdated(array $ids, int $lastUpdated): void;
206+
public function setLastUpdated(array $ids, \DateTimeImmutable $lastUpdated): void;
208207
}

‎lib/unstable/Sharing/ISharingManager.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,11 @@ public function searchRecipients(ShareAccessContext $accessContext, ?array $filt
5050
public function generateSecret(): string;
5151

5252
/**
53-
* Generate a new timestamp in milliseconds since the UNIX epoch.
53+
* Get the current time
5454
*
55-
* @return non-negative-int
5655
* @experimental 35.0.0
5756
*/
58-
public function generateTimestamp(): int;
57+
public function getTime(): \DateTimeImmutable;
5958

6059
/**
6160
* Create a new share.

‎lib/unstable/Sharing/Share.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use NCU\Sharing\Recipient\ShareRecipient;
1919
use NCU\Sharing\Source\IShareSourceType;
2020
use NCU\Sharing\Source\ShareSource;
21+
use OC\Sharing\SharingManager;
2122
use OCP\AppFramework\Attribute\Consumable;
2223
use OCP\IURLGenerator;
2324
use OCP\IUserManager;
@@ -154,8 +155,7 @@ public function __construct(
154155
/** @var non-empty-string $id */
155156
public readonly string $id,
156157
public readonly ShareUser $owner,
157-
/** @var non-negative-int $lastUpdated Unix time in milliseconds */
158-
public readonly int $lastUpdated,
158+
public readonly \DateTimeImmutable $lastUpdated,
159159
public readonly ShareState $state,
160160
/** @var list<ShareSource> $sources */
161161
public readonly array $sources,
@@ -240,7 +240,7 @@ public function format(ISharingRegistry $registry, IFactory $l10nFactory, IURLGe
240240
return [
241241
'id' => $this->id,
242242
'owner' => $this->owner->format($userManager),
243-
'last_updated' => $this->lastUpdated,
243+
'last_updated' => SharingManager::timeToMs($this->lastUpdated),
244244
'state' => $this->state->value,
245245
'sources' => ShareSource::formatMultiple($registry, $l10nFactory, $this->sources),
246246
'recipients' => ShareRecipient::formatMultiple($registry, $l10nFactory, $urlGenerator, $userManager, $this->recipients),

‎tests/Core/Sharing/Property/ExpirationDateSharePropertyTypeTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ private function createDummyShare(ShareProperty $property): Share {
7272
return new Share(
7373
'123',
7474
new ShareUser($this->user->getUID(), null),
75-
0,
75+
new DateTimeImmutable(),
7676
ShareState::Active,
7777
[],
7878
[],
@@ -98,7 +98,7 @@ public function testGetRequired(string $defaultEnabledKey, string $defaultEnforc
9898
$share = new Share(
9999
'123',
100100
new ShareUser('user', null),
101-
0,
101+
new DateTimeImmutable(),
102102
ShareState::Active,
103103
[],
104104
[
@@ -133,7 +133,7 @@ public function testGetDefaultValue(string $defaultEnabledKey, string $defaultEn
133133
$share = new Share(
134134
'123',
135135
new ShareUser('user', null),
136-
0,
136+
new DateTimeImmutable(),
137137
ShareState::Active,
138138
[],
139139
[
@@ -167,7 +167,7 @@ public function testGetMinMaxDate(string $defaultEnabledKey, string $defaultEnfo
167167
$share = new Share(
168168
'123',
169169
new ShareUser('user', null),
170-
0,
170+
new DateTimeImmutable(),
171171
ShareState::Active,
172172
[],
173173
[

0 commit comments

Comments
 (0)