Skip to content

Commit 2ec357f

Browse files
icewind1991icewind1991
committed
refactor: use DateTimeImmutable in unified sharing api instead of timestamp
Signed-off-by: Robin Appelman <robin@icewind.nl>
1 parent 18cc06c commit 2ec357f

11 files changed

Lines changed: 152 additions & 145 deletions

File tree

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use NCU\Sharing\ShareAccessContext;
1313
use NCU\Sharing\Source\ShareSource;
1414
use OC\Files\Filesystem;
15+
use OC\Sharing\SharingManager;
1516
use OC\User\Database;
1617
use OCA\Files\Sharing\Source\NodeShareSourceType;
1718
use OCP\EventDispatcher\IEventDispatcher;
@@ -103,14 +104,14 @@ public function testDelete(): void {
103104
$this->manager->addShareSource($accessContext, $id, new ShareSource($this->sourceType::class, (string)$this->node->getId()));
104105
$this->dbConnection->commit();
105106

106-
$before = $this->manager->generateTimestamp();
107+
$before = $this->manager->getTime();
107108
$this->node->delete();
108-
$after = $this->manager->generateTimestamp();
109+
$after = $this->manager->getTime();
109110

110111
$this->dbConnection->beginTransaction();
111112
$share = $this->manager->getShare($accessContext, $id);
112-
$this->assertGreaterThanOrEqual($before, $share->lastUpdated);
113-
$this->assertLessThanOrEqual($after, $share->lastUpdated);
113+
$this->assertGreaterThanOrEqual($before, SharingManager::timeToMs($share->lastUpdated));
114+
$this->assertLessThanOrEqual($after, SharingManager::timeToMs($share->lastUpdated));
114115
$this->assertEquals([], $share->sources);
115116

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

‎apps/files_sharing/lib/Sharing/LegacyBackend.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -391,8 +391,7 @@ public function getShare(string $id): Share {
391391
return new Share(
392392
$id,
393393
$owner,
394-
// TODO
395-
0,
394+
\DateTimeImmutable::createFromMutable($legacyShares[0]->getShareTime()),
396395
// TODO
397396
ShareState::Active,
398397
array_values($sources),

‎lib/private/Sharing/SharingBackend.php‎

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

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

540540
$rowCount = $qb
541541
->update('sharing_share')
542-
->set('last_updated', $qb->createNamedParameter($lastUpdated, IQueryBuilder::PARAM_INT))
542+
->set('last_updated', $qb->createNamedParameter(SharingManager::timeToMs($lastUpdated), IQueryBuilder::PARAM_INT))
543543
->where($qb->expr()->in('id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)))
544544
->executeStatement();
545545
if ($rowCount !== count($chunk)) {
@@ -954,7 +954,7 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID,
954954
$shares = array_map(static fn (array $share): Share => new Share(
955955
$share['id'],
956956
$share['owner'],
957-
$share['last_updated'],
957+
new \DateTimeImmutable('@' . $share['last_updated'] / 1000), // timestamp is stored in milliseconds
958958
$share['state'],
959959
$share['sources'],
960960
$share['recipients'],

‎lib/private/Sharing/SharingManager.php‎

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
use OCP\Share\IManager;
5151
use OCP\Snowflake\ISnowflakeGenerator;
5252
use OCP\User\Events\BeforeUserDeletedEvent;
53+
use Psr\Clock\ClockInterface;
5354
use Random\Randomizer;
5455
use RuntimeException;
5556

@@ -81,6 +82,7 @@ public function __construct(
8182
private readonly IDBConnection $dbConnection,
8283
private readonly ISharingRegistry $registry,
8384
private readonly IManager $legacyManager,
85+
private readonly ClockInterface $clock,
8486
IAppConfig $appConfig,
8587
) {
8688
$this->randomizer = new Randomizer();
@@ -160,13 +162,8 @@ public function generateSecret(): string {
160162
}
161163

162164
#[\Override]
163-
public function generateTimestamp(): int {
164-
$time = (int)(microtime(true) * 1000.0);
165-
if ($time < 0) {
166-
throw new RuntimeException('Have you invented time travel?');
167-
}
168-
169-
return $time;
165+
public function getTime(): \DateTimeImmutable {
166+
return $this->clock->now();
170167
}
171168

172169
#[\Override]
@@ -178,7 +175,7 @@ public function createShare(ShareAccessContext $accessContext): string {
178175
$this->assertInTransaction();
179176

180177
$id = $this->snowflakeGenerator->nextId();
181-
$lastUpdated = $this->generateTimestamp();
178+
$lastUpdated = $this->getTime();
182179
$this->backend->createShare($id, new ShareUser($currentUser->getUID(), null), $lastUpdated);
183180

184181
$this->processShareUpdates([$id]);
@@ -210,7 +207,7 @@ public function onOwnerDeleted(ShareAccessContext $accessContext, ShareUser $own
210207
public function updateShareState(ShareAccessContext $accessContext, string $id, ShareState $state): void {
211208
$this->assertInTransaction();
212209

213-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
210+
$this->backend->setLastUpdated([$id], $this->getTime());
214211

215212
$owner = $this->backend->getShareOwner($id);
216213
$this->validateShareOwnerOperation($accessContext, $owner);
@@ -229,7 +226,7 @@ public function updateShareState(ShareAccessContext $accessContext, string $id,
229226
public function addShareSource(ShareAccessContext $accessContext, string $id, ShareSource $source): void {
230227
$this->assertInTransaction();
231228

232-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
229+
$this->backend->setLastUpdated([$id], $this->getTime());
233230

234231
$owner = $this->backend->getShareOwner($id);
235232
$this->validateShareOwnerOperation($accessContext, $owner);
@@ -270,7 +267,7 @@ public function addShareSource(ShareAccessContext $accessContext, string $id, Sh
270267
public function removeShareSource(ShareAccessContext $accessContext, string $id, ShareSource $source): void {
271268
$this->assertInTransaction();
272269

273-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
270+
$this->backend->setLastUpdated([$id], $this->getTime());
274271

275272
$owner = $this->backend->getShareOwner($id);
276273
$this->validateShareOwnerOperation($accessContext, $owner);
@@ -288,7 +285,7 @@ public function onSourceDeleted(ShareAccessContext $accessContext, ShareSource $
288285

289286
$this->assertInTransaction();
290287

291-
$timestamp = $this->generateTimestamp();
288+
$timestamp = $this->getTime();
292289

293290
$updatedIds = $this->backend->onSourceDeleted($source);
294291
if ($updatedIds === []) {
@@ -308,7 +305,7 @@ public function addShareRecipient(ShareAccessContext $accessContext, string $id,
308305

309306
$this->assertInTransaction();
310307

311-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
308+
$this->backend->setLastUpdated([$id], $this->getTime());
312309

313310
$owner = $this->backend->getShareOwner($id);
314311

@@ -371,7 +368,7 @@ public function removeShareRecipient(ShareAccessContext $accessContext, string $
371368

372369
$this->assertInTransaction();
373370

374-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
371+
$this->backend->setLastUpdated([$id], $this->getTime());
375372

376373
$owner = $this->backend->getShareOwner($id);
377374

@@ -396,7 +393,7 @@ public function onRecipientDeleted(ShareAccessContext $accessContext, ShareRecip
396393

397394
$this->assertInTransaction();
398395

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

401398
$updatedIds = $this->backend->onRecipientDeleted($recipient);
402399
if ($updatedIds === []) {
@@ -416,7 +413,7 @@ public function onInitiatorDeleted(ShareAccessContext $accessContext, ShareUser
416413

417414
$this->assertInTransaction();
418415

419-
$timestamp = $this->generateTimestamp();
416+
$timestamp = $this->getTime();
420417

421418
$updatedIds = $this->backend->onInitiatorDeleted($initiator);
422419
if ($updatedIds === []) {
@@ -432,7 +429,7 @@ public function onInitiatorDeleted(ShareAccessContext $accessContext, ShareUser
432429
public function updateShareRecipientSecret(ShareAccessContext $accessContext, string $id, ShareRecipient $recipient, string $secret): void {
433430
$this->assertInTransaction();
434431

435-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
432+
$this->backend->setLastUpdated([$id], $this->getTime());
436433

437434
$owner = $this->backend->getShareOwner($id);
438435

@@ -464,7 +461,7 @@ public function updateShareRecipientSecret(ShareAccessContext $accessContext, st
464461
public function createSharePropertyDefaultValue(Share $share, string $propertyTypeClass): Share {
465462
$this->assertInTransaction();
466463

467-
$timestamp = $this->generateTimestamp();
464+
$timestamp = $this->getTime();
468465
$this->backend->setLastUpdated([$share->id], $timestamp);
469466

470467
if (($propertyType = $this->registry->getPropertyTypes()[$propertyTypeClass] ?? null) === null) {
@@ -498,7 +495,7 @@ public function createSharePropertyDefaultValue(Share $share, string $propertyTy
498495
public function updateShareProperty(ShareAccessContext $accessContext, string $id, ShareProperty $property): void {
499496
$this->assertInTransaction();
500497

501-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
498+
$this->backend->setLastUpdated([$id], $this->getTime());
502499

503500
$owner = $this->backend->getShareOwner($id);
504501
$this->validateShareOwnerOperation($accessContext, $owner);
@@ -523,8 +520,8 @@ public function updateShareProperty(ShareAccessContext $accessContext, string $i
523520
public function createSharePermissionDefaultValue(Share $share, string $permissionTypeClass): Share {
524521
$this->assertInTransaction();
525522

526-
$timestamp = $this->generateTimestamp();
527-
$this->backend->setLastUpdated([$share->id], $timestamp);
523+
$time = $this->getTime();
524+
$this->backend->setLastUpdated([$share->id], $time);
528525

529526
if (($permissionType = $this->registry->getPermissionTypes()[$permissionTypeClass] ?? null) === null) {
530527
throw new RuntimeException('The permission is not registered: ' . $permissionTypeClass);
@@ -540,7 +537,7 @@ public function createSharePermissionDefaultValue(Share $share, string $permissi
540537
$share = new Share(
541538
$share->id,
542539
$share->owner,
543-
$timestamp,
540+
$time,
544541
$share->state,
545542
$share->sources,
546543
$share->recipients,
@@ -557,7 +554,7 @@ public function createSharePermissionDefaultValue(Share $share, string $permissi
557554
public function updateSharePermission(ShareAccessContext $accessContext, string $id, SharePermission $permission): void {
558555
$this->assertInTransaction();
559556

560-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
557+
$this->backend->setLastUpdated([$id], $this->getTime());
561558

562559
$owner = $this->backend->getShareOwner($id);
563560
$this->validateShareOwnerOperation($accessContext, $owner);
@@ -594,7 +591,7 @@ public function updateSharePermission(ShareAccessContext $accessContext, string
594591
public function selectSharePermissionPreset(ShareAccessContext $accessContext, string $id, string $permissionPresetClass): void {
595592
$this->assertInTransaction();
596593

597-
$this->backend->setLastUpdated([$id], $this->generateTimestamp());
594+
$this->backend->setLastUpdated([$id], $this->getTime());
598595

599596
$owner = $this->backend->getShareOwner($id);
600597
$this->validateShareOwnerOperation($accessContext, $owner);
@@ -980,4 +977,13 @@ private function updateShare(Share $share): void {
980977
$this->backend->createSharePermission($share->id, $permission);
981978
}
982979
}
980+
981+
public static function timeToMs(\DateTimeImmutable $time): int {
982+
if (method_exists($time, 'getMicrosecond')) {
983+
$micros = $time->getMicrosecond();
984+
} else {
985+
$micros = (int)$time->format('u');
986+
}
987+
return $time->getTimestamp() * 1000 + (int)floor($micros / 1000);
988+
}
983989
}

‎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.
@@ -199,9 +199,8 @@ public function getShareOwner(string $id): ShareUser;
199199
* Set the last updated timestamp for multiple shares.
200200
*
201201
* @param non-empty-list<string> $ids
202-
* @param non-negative-int $lastUpdated
203202
* @throws ShareNotFoundException
204203
* @experimental 35.0.0
205204
*/
206-
public function setLastUpdated(array $ids, int $lastUpdated): void;
205+
public function setLastUpdated(array $ids, \DateTimeImmutable $lastUpdated): void;
207206
}

‎lib/unstable/Sharing/ISharingManager.php‎

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

5353
/**
54-
* Generate a new timestamp in milliseconds since the UNIX epoch.
55-
*
56-
* @return non-negative-int
54+
* Get the current time
55+
*
5756
* @experimental 35.0.0
5857
*/
59-
public function generateTimestamp(): int;
58+
public function getTime(): \DateTimeImmutable;
6059

6160
/**
6261
* Create a new share.

‎lib/unstable/Sharing/Share.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,7 @@ public function __construct(
154154
/** @var non-empty-string $id */
155155
public readonly string $id,
156156
public readonly ShareUser $owner,
157-
/** @var non-negative-int $lastUpdated Unix time in milliseconds */
158-
public readonly int $lastUpdated,
157+
public readonly \DateTimeImmutable $lastUpdated,
159158
public readonly ShareState $state,
160159
/** @var list<ShareSource> $sources */
161160
public readonly array $sources,
@@ -237,10 +236,12 @@ public function format(ISharingRegistry $registry, IFactory $l10nFactory, IURLGe
237236
// First sort by priority and then sort by class name to get a stable order regardless of the DB order
238237
usort($permissions, static fn (array $a, array $b): int => 2 * ($b['priority'] <=> $a['priority']) + ($a['class'] <=> $b['class']));
239238

239+
/** @var non-negative-int $timestamp */
240+
$timestamp = $this->lastUpdated->getTimestamp();
240241
return [
241242
'id' => $this->id,
242243
'owner' => $this->owner->format($userManager),
243-
'last_updated' => $this->lastUpdated,
244+
'last_updated' => $timestamp,
244245
'state' => $this->state->value,
245246
'sources' => ShareSource::formatMultiple($registry, $l10nFactory, $this->sources),
246247
'recipients' => ShareRecipient::formatMultiple($registry, $l10nFactory, $urlGenerator, $userManager, $this->recipients),

‎tests/Core/Sharing/Recipient/GroupShareRecipientTypeTest.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use NCU\Sharing\ShareAccessContext;
1616
use OC\Core\Sharing\Recipient\GroupShareRecipientType;
1717
use OC\Group\Database;
18+
use OC\Sharing\SharingManager;
1819
use OCP\EventDispatcher\IEventDispatcher;
1920
use OCP\IDBConnection;
2021
use OCP\IGroup;
@@ -140,14 +141,14 @@ public function testDelete(): void {
140141
$this->manager->addShareRecipient($accessContext, $id, new ShareRecipient($this->recipientType::class, $this->group1->getGID(), null));
141142
$this->dbConnection->commit();
142143

143-
$before = $this->manager->generateTimestamp();
144+
$before = $this->manager->getTime();
144145
$this->group1->delete();
145-
$after = $this->manager->generateTimestamp();
146+
$after = $this->manager->getTime();
146147

147148
$this->dbConnection->beginTransaction();
148149
$share = $this->manager->getShare($accessContext, $id);
149-
$this->assertGreaterThanOrEqual($before, $share->lastUpdated);
150-
$this->assertLessThanOrEqual($after, $share->lastUpdated);
150+
$this->assertGreaterThanOrEqual($before, SharingManager::timeToMs($share->lastUpdated));
151+
$this->assertLessThanOrEqual($after, SharingManager::timeToMs($share->lastUpdated));
151152
$this->assertEquals([], $share->recipients);
152153

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

‎tests/Core/Sharing/Recipient/TeamShareRecipientTypeTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,14 +171,14 @@ public function testDelete(): void {
171171
$circlesManager = Server::get(CirclesManager::class);
172172
$circlesManager->startSession($circlesManager->getLocalFederatedUser($this->user1->getUID()));
173173

174-
$before = $this->manager->generateTimestamp();
174+
$before = $this->manager->getTime();
175175
$circlesManager->destroyCircle($this->team1->getId());
176-
$after = $this->manager->generateTimestamp();
176+
$after = $this->manager->getTime();
177177

178178
$this->dbConnection->beginTransaction();
179179
$share = $this->manager->getShare($accessContext, $id);
180-
$this->assertGreaterThanOrEqual($before, $share->lastUpdated);
181-
$this->assertLessThanOrEqual($after, $share->lastUpdated);
180+
$this->assertGreaterThanOrEqual($before, $share->lastUpdated->getTimestamp());
181+
$this->assertLessThanOrEqual($after, $share->lastUpdated->getTimestamp());
182182
$this->assertEquals([], $share->recipients);
183183

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

0 commit comments

Comments
 (0)