Skip to content

refactor: use DateTimeImmutable in unified sharing api instead of timestamp #62957

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.

Sign up for GitHub

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

Open
icewind1991 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

refactor: use DateTimeImmutable in unified sharing api instead of timestamp #62957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 5 additions & 4 deletions apps/files/tests/Sharing/Source/NodeShareSourceTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use NCU\Sharing\ShareAccessContext;
use NCU\Sharing\Source\ShareSource;
use OC\Files\Filesystem;
use OC\Sharing\SharingManager;
use OC\User\Database;
use OCA\Files\Sharing\Source\NodeShareSourceType;
use OCP\EventDispatcher\IEventDispatcher;
Expand Down Expand Up @@ -103,14 +104,14 @@ public function testDelete(): void {
$this->manager->addShareSource($accessContext, $id, new ShareSource($this->sourceType::class, (string)$this->node->getId()));
$this->dbConnection->commit();

$before = $this->manager->generateTimestamp();
$before = $this->manager->getTime();
$this->node->delete();
$after = $this->manager->generateTimestamp();
$after = $this->manager->getTime();

$this->dbConnection->beginTransaction();
$share = $this->manager->getShare($accessContext, $id);
$this->assertGreaterThanOrEqual($before, $share->lastUpdated);
$this->assertLessThanOrEqual($after, $share->lastUpdated);
$this->assertGreaterThanOrEqual($before, SharingManager::timeToMs($share->lastUpdated));
$this->assertLessThanOrEqual($after, SharingManager::timeToMs($share->lastUpdated));
$this->assertEquals([], $share->sources);

$this->manager->deleteShare($accessContext, $id);
Expand Down
24 changes: 19 additions & 5 deletions lib/private/Sharing/SharingBackend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ public function __construct(
}

#[\Override]
public function createShare(string $id, ShareUser $owner, int $lastUpdated): void {
public function createShare(string $id, ShareUser $owner, \DateTimeImmutable $lastUpdated): void {
$qb = $this->connection->getQueryBuilder();
$qb
->insert('sharing_share')
->values([
'id' => $qb->createNamedParameter($id),
'owner_user_id' => $qb->createNamedParameter($owner->userId),
'owner_instance' => $qb->createNamedParameter($owner->instance),
'last_updated' => $qb->createNamedParameter($lastUpdated),
'last_updated' => $qb->createNamedParameter(SharingManager::timeToMs($lastUpdated)),
'state' => $qb->createNamedParameter(ShareState::Draft->value),
])
->executeStatement();
Expand Down Expand Up @@ -533,13 +533,13 @@ public function getShareOwner(string $id): ShareUser {
* @param non-empty-list<string> $ids
*/
#[\Override]
public function setLastUpdated(array $ids, int $lastUpdated): void {
public function setLastUpdated(array $ids, \DateTimeImmutable $lastUpdated): void {
foreach (array_chunk($ids, 1000) as $chunk) {
$qb = $this->connection->getQueryBuilder();

$rowCount = $qb
->update('sharing_share')
->set('last_updated', $qb->createNamedParameter($lastUpdated, IQueryBuilder::PARAM_INT))
->set('last_updated', $qb->createNamedParameter(SharingManager::timeToMs($lastUpdated), IQueryBuilder::PARAM_INT))
->where($qb->expr()->in('id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY)))
->executeStatement();
if ($rowCount !== count($chunk)) {
Expand Down Expand Up @@ -954,7 +954,7 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID,
$shares = array_map(static fn (array $share): Share => new Share(
$share['id'],
$share['owner'],
$share['last_updated'],
self::parseTimestamp($share['last_updated']),
$share['state'],
$share['sources'],
$share['recipients'],
Expand Down Expand Up @@ -1005,4 +1005,18 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID,

return array_values($shares);
}

private static function parseTimestamp(int $timestampMs): \DateTimeImmutable {
if (method_exists(\DateTimeImmutable::class, 'createFromTimestamp')) {
// with php 8.3 the method doesn't exist and psalm doesn't know the return type
/** @psalm-suppress MixedReturnStatement */
return \DateTimeImmutable::createFromTimestamp((float)$timestampMs / 1000.0);
} else {
$time = \DateTimeImmutable::createFromFormat('U.u', (string)((float)$timestampMs / 1000.0));
if ($time === false) {
throw new \RuntimeException('Invalid timestamp for share');
}
return $time;
}
}
}
59 changes: 37 additions & 22 deletions lib/private/Sharing/SharingManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
use OCP\Security\ISecureRandom;
use OCP\Snowflake\ISnowflakeGenerator;
use OCP\User\Events\BeforeUserDeletedEvent;
use Psr\Clock\ClockInterface;
use Random\Randomizer;
use RuntimeException;

Expand Down Expand Up @@ -70,6 +71,7 @@ public function __construct(
private IDBConnection $dbConnection,
private ISharingRegistry $registry,
IAppConfig $appConfig,
private ClockInterface $clock,
) {
$this->randomizer = new Randomizer();
$this->l10n = $l10nFactory->get('sharing');
Expand Down Expand Up @@ -140,13 +142,8 @@ public function generateSecret(): string {
}

#[\Override]
public function generateTimestamp(): int {
$time = (int)(microtime(true) * 1000.0);
if ($time < 0) {
throw new RuntimeException('Have you invented time travel?');
}

return $time;
public function getTime(): \DateTimeImmutable {
return $this->clock->now();
}

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

$id = $this->snowflakeGenerator->nextId();
$lastUpdated = $this->generateTimestamp();
$lastUpdated = $this->getTime();
$this->backend->createShare($id, new ShareUser($currentUser->getUID(), null), $lastUpdated);

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

$this->backend->setLastUpdated([$id], $this->generateTimestamp());
$this->backend->setLastUpdated([$id], $this->getTime());

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

$this->backend->setLastUpdated([$id], $this->generateTimestamp());
$this->backend->setLastUpdated([$id], $this->getTime());

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

$this->backend->setLastUpdated([$id], $this->generateTimestamp());
$this->backend->setLastUpdated([$id], $this->getTime());

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

$this->assertInTransaction();

$timestamp = $this->generateTimestamp();
$timestamp = $this->getTime();

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

$this->assertInTransaction();

$this->backend->setLastUpdated([$id], $this->generateTimestamp());
$this->backend->setLastUpdated([$id], $this->getTime());

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

Expand Down Expand Up @@ -351,7 +348,7 @@ public function removeShareRecipient(ShareAccessContext $accessContext, string $

$this->assertInTransaction();

$this->backend->setLastUpdated([$id], $this->generateTimestamp());
$this->backend->setLastUpdated([$id], $this->getTime());

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

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

$this->assertInTransaction();

$timestamp = $this->generateTimestamp();
$timestamp = $this->getTime();

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

$this->assertInTransaction();

$timestamp = $this->generateTimestamp();
$timestamp = $this->getTime();

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

$this->backend->setLastUpdated([$id], $this->generateTimestamp());
$this->backend->setLastUpdated([$id], $this->getTime());

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

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

$timestamp = $this->generateTimestamp();
$timestamp = $this->getTime();
$this->backend->setLastUpdated([$share->id], $timestamp);

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

$this->backend->setLastUpdated([$id], $this->generateTimestamp());
$this->backend->setLastUpdated([$id], $this->getTime());

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

$timestamp = $this->generateTimestamp();
$timestamp = $this->getTime();
$this->backend->setLastUpdated([$share->id], $timestamp);

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

$this->backend->setLastUpdated([$id], $this->generateTimestamp());
$this->backend->setLastUpdated([$id], $this->getTime());

$owner = $this->backend->getShareOwner($id);
$this->validateShareOwnerOperation($accessContext, $owner);
Expand Down Expand Up @@ -574,7 +571,7 @@ public function updateSharePermission(ShareAccessContext $accessContext, string
public function selectSharePermissionPreset(ShareAccessContext $accessContext, string $id, string $permissionPresetClass): void {
$this->assertInTransaction();

$this->backend->setLastUpdated([$id], $this->generateTimestamp());
$this->backend->setLastUpdated([$id], $this->getTime());

$owner = $this->backend->getShareOwner($id);
$this->validateShareOwnerOperation($accessContext, $owner);
Expand Down Expand Up @@ -826,4 +823,22 @@ private function processShareUpdates(array $sharesOrIds): array {

return $shares;
}

/**
* @return non-negative-int
*/
public static function timeToMs(\DateTimeImmutable $time): int {
if (method_exists($time, 'getMicrosecond')) {
/** @var int $micros */
$micros = $time->getMicrosecond();
} else {
$micros = (int)$time->format('u');
}

$time = $time->getTimestamp() * 1000 + (int)floor($micros / 1000);
if ($time > 0) {
return $time;
}
throw new \RuntimeException('invalid date-time');
}
}
5 changes: 2 additions & 3 deletions lib/unstable/Sharing/ISharingBackend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ interface ISharingBackend {
*
* @experimental 35.0.0
*/
public function createShare(string $id, ShareUser $owner, int $lastUpdated): void;
public function createShare(string $id, ShareUser $owner, \DateTimeImmutable $lastUpdated): void;

/**
* Perform all updates when the owner was deleted.
Expand Down Expand Up @@ -199,9 +199,8 @@ public function getShareOwner(string $id): ShareUser;
* Set the last updated timestamp for multiple shares.
*
* @param non-empty-list<string> $ids
* @param non-negative-int $lastUpdated
* @throws ShareNotFoundException
* @experimental 35.0.0
*/
public function setLastUpdated(array $ids, int $lastUpdated): void;
public function setLastUpdated(array $ids, \DateTimeImmutable $lastUpdated): void;
}
5 changes: 2 additions & 3 deletions lib/unstable/Sharing/ISharingManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,11 @@ public function searchRecipients(ShareAccessContext $accessContext, ?array $filt
public function generateSecret(): string;

/**
* Generate a new timestamp in milliseconds since the UNIX epoch.
* Get the current time
*
* @return non-negative-int
* @experimental 35.0.0
*/
public function generateTimestamp(): int;
public function getTime(): \DateTimeImmutable;

/**
* Create a new share.
Expand Down
6 changes: 3 additions & 3 deletions lib/unstable/Sharing/Share.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use NCU\Sharing\Recipient\ShareRecipient;
use NCU\Sharing\Source\IShareSourceType;
use NCU\Sharing\Source\ShareSource;
use OC\Sharing\SharingManager;
use OCP\AppFramework\Attribute\Consumable;
use OCP\IURLGenerator;
use OCP\IUserManager;
Expand Down Expand Up @@ -154,8 +155,7 @@ public function __construct(
/** @var non-empty-string $id */
public readonly string $id,
public readonly ShareUser $owner,
/** @var non-negative-int $lastUpdated Unix time in milliseconds */
public readonly int $lastUpdated,
public readonly \DateTimeImmutable $lastUpdated,
public readonly ShareState $state,
/** @var list<ShareSource> $sources */
public readonly array $sources,
Expand Down Expand Up @@ -240,7 +240,7 @@ public function format(ISharingRegistry $registry, IFactory $l10nFactory, IURLGe
return [
'id' => $this->id,
'owner' => $this->owner->format($userManager),
'last_updated' => $this->lastUpdated,
'last_updated' => SharingManager::timeToMs($this->lastUpdated),
'state' => $this->state->value,
'sources' => ShareSource::formatMultiple($registry, $l10nFactory, $this->sources),
'recipients' => ShareRecipient::formatMultiple($registry, $l10nFactory, $urlGenerator, $userManager, $this->recipients),
Expand Down
8 changes: 4 additions & 4 deletions tests/Core/Sharing/Property/ExpirationDateSharePropertyTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ private function createDummyShare(ShareProperty $property): Share {
return new Share(
'123',
new ShareUser($this->user->getUID(), null),
0,
new DateTimeImmutable(),
ShareState::Active,
[],
[],
Expand All @@ -98,7 +98,7 @@ public function testGetRequired(string $defaultEnabledKey, string $defaultEnforc
$share = new Share(
'123',
new ShareUser('user', null),
0,
new DateTimeImmutable(),
ShareState::Active,
[],
[
Expand Down Expand Up @@ -133,7 +133,7 @@ public function testGetDefaultValue(string $defaultEnabledKey, string $defaultEn
$share = new Share(
'123',
new ShareUser('user', null),
0,
new DateTimeImmutable(),
ShareState::Active,
[],
[
Expand Down Expand Up @@ -167,7 +167,7 @@ public function testGetMinMaxDate(string $defaultEnabledKey, string $defaultEnfo
$share = new Share(
'123',
new ShareUser('user', null),
0,
new DateTimeImmutable(),
ShareState::Active,
[],
[
Expand Down
Loading
Loading