diff --git a/apps/files/composer/composer/autoload_classmap.php b/apps/files/composer/composer/autoload_classmap.php index 80e87ce2a96fc..b834c18d05155 100644 --- a/apps/files/composer/composer/autoload_classmap.php +++ b/apps/files/composer/composer/autoload_classmap.php @@ -104,5 +104,6 @@ 'OCA\\Files\\Sharing\\Permission\\NodeReadSharePermissionType' => $baseDir . '/../lib/Sharing/Permission/NodeReadSharePermissionType.php', 'OCA\\Files\\Sharing\\Permission\\NodeUpdateSharePermissionType' => $baseDir . '/../lib/Sharing/Permission/NodeUpdateSharePermissionType.php', 'OCA\\Files\\Sharing\\Property\\NodeGridViewSharePropertyType' => $baseDir . '/../lib/Sharing/Property/NodeGridViewSharePropertyType.php', + 'OCA\\Files\\Sharing\\Source\\NodeShareSourceMetadata' => $baseDir . '/../lib/Sharing/Source/NodeShareSourceMetadata.php', 'OCA\\Files\\Sharing\\Source\\NodeShareSourceType' => $baseDir . '/../lib/Sharing/Source/NodeShareSourceType.php', ); diff --git a/apps/files/composer/composer/autoload_static.php b/apps/files/composer/composer/autoload_static.php index 4dfc62aedb094..7a627de1a5f6a 100644 --- a/apps/files/composer/composer/autoload_static.php +++ b/apps/files/composer/composer/autoload_static.php @@ -119,6 +119,7 @@ class ComposerStaticInitFiles 'OCA\\Files\\Sharing\\Permission\\NodeReadSharePermissionType' => __DIR__ . '/..' . '/../lib/Sharing/Permission/NodeReadSharePermissionType.php', 'OCA\\Files\\Sharing\\Permission\\NodeUpdateSharePermissionType' => __DIR__ . '/..' . '/../lib/Sharing/Permission/NodeUpdateSharePermissionType.php', 'OCA\\Files\\Sharing\\Property\\NodeGridViewSharePropertyType' => __DIR__ . '/..' . '/../lib/Sharing/Property/NodeGridViewSharePropertyType.php', + 'OCA\\Files\\Sharing\\Source\\NodeShareSourceMetadata' => __DIR__ . '/..' . '/../lib/Sharing/Source/NodeShareSourceMetadata.php', 'OCA\\Files\\Sharing\\Source\\NodeShareSourceType' => __DIR__ . '/..' . '/../lib/Sharing/Source/NodeShareSourceType.php', ); diff --git a/apps/files/lib/Sharing/Source/NodeShareSourceMetadata.php b/apps/files/lib/Sharing/Source/NodeShareSourceMetadata.php new file mode 100644 index 0000000000000..b4ee4744648b4 --- /dev/null +++ b/apps/files/lib/Sharing/Source/NodeShareSourceMetadata.php @@ -0,0 +1,36 @@ +cacheEntry->getName(); + return $name !== '' ? $name: (string)$this->cacheEntry->getId(); + } + + #[\Override] + public function getIcon(): ShareIconURL { + $url = $this->urlGenerator->linkToRouteAbsolute('core.Preview.getPreviewByFileId', ['fileId' => $this->cacheEntry->getId(), 'x' => 64, 'y' => 64]); + + return new ShareIconURL($url, $url); + } +} diff --git a/apps/files/lib/Sharing/Source/NodeShareSourceType.php b/apps/files/lib/Sharing/Source/NodeShareSourceType.php index ea1a435e0223f..e0839c636360f 100644 --- a/apps/files/lib/Sharing/Source/NodeShareSourceType.php +++ b/apps/files/lib/Sharing/Source/NodeShareSourceType.php @@ -10,9 +10,9 @@ namespace OCA\Files\Sharing\Source; use Exception; -use NCU\Sharing\Icon\ShareIconURL; use NCU\Sharing\ISharingManager; use NCU\Sharing\ShareAccessContext; +use NCU\Sharing\Source\IShareSourceMetadata; use NCU\Sharing\Source\IShareSourceType; use NCU\Sharing\Source\ShareSource; use OCA\Files\AppInfo\Application; @@ -20,6 +20,8 @@ use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\EventDispatcher\IEventListener; +use OCP\Files\Cache\ICacheEntry; +use OCP\Files\Cache\IFileAccess; use OCP\Files\Events\Node\NodeDeletedEvent; use OCP\Files\IRootFolder; use OCP\Files\Node; @@ -39,6 +41,7 @@ public function __construct( private IRootFolder $rootFolder, private IURLGenerator $urlGenerator, private ISharingManager $manager, + private IFileAccess $fileAccess, ) { $eventDispatcher->addServiceListener(NodeDeletedEvent::class, self::class); $eventDispatcher->addServiceListener(MoveToTrashEvent::class, self::class); @@ -55,20 +58,24 @@ public function validateSource(string $source): bool { } #[\Override] - public function getSourceDisplayName(string $source): ?string { - $displayName = $this->rootFolder->getFirstNodeById((int)$source)?->getName(); - if ($displayName === '') { - return null; + public function getSourceMetadata(string $source): ?IShareSourceMetadata { + $cacheEntry = $this->fileAccess->getByFileId((int)$source); + if ($cacheEntry instanceof ICacheEntry) { + return new NodeShareSourceMetadata($this->urlGenerator, $cacheEntry); } - return $displayName; + return null; } #[\Override] - public function getSourceIcon(string $source): ShareIconURL { - $url = $this->urlGenerator->linkToRouteAbsolute('core.Preview.getPreviewByFileId', ['fileId' => $source, 'x' => 64, 'y' => 64]); - - return new ShareIconURL($url, $url); + public function getSourcesMetadata(array $sources): array { + $sources = array_map(intval(...), $sources); + $cacheEntries = $this->fileAccess->getByFileIds($sources); + // we actually have an `array` instead of an `array` here, + // but since numeric string array keys are automatically casted to ints anyway they are functionally equivalent + /** @var array $metadata */ + $metadata = array_map(fn (ICacheEntry $cacheEntry): NodeShareSourceMetadata => new NodeShareSourceMetadata($this->urlGenerator, $cacheEntry), $cacheEntries); + return $metadata; } #[\Override] diff --git a/apps/files/tests/Sharing/Source/NodeShareSourceTypeTest.php b/apps/files/tests/Sharing/Source/NodeShareSourceTypeTest.php index 31d699361cfd0..2f984e9dd8437 100644 --- a/apps/files/tests/Sharing/Source/NodeShareSourceTypeTest.php +++ b/apps/files/tests/Sharing/Source/NodeShareSourceTypeTest.php @@ -7,26 +7,29 @@ declare(strict_types=1); +use NCU\Sharing\Icon\ShareIconURL; use NCU\Sharing\ISharingManager; use NCU\Sharing\ISharingRegistry; use NCU\Sharing\ShareAccessContext; use NCU\Sharing\Source\ShareSource; use OC\Files\Filesystem; -use OC\User\Database; use OCA\Files\Sharing\Source\NodeShareSourceType; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Files\Cache\IFileAccess; use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\IDBConnection; use OCP\IURLGenerator; use OCP\IUser; -use OCP\IUserManager; use OCP\Server; use PHPUnit\Framework\Attributes\Group; use Test\TestCase; +use Test\Traits\UserTrait; #[Group(name: 'DB')] final class NodeShareSourceTypeTest extends TestCase { + use UserTrait; + private IDBConnection $dbConnection; private ISharingManager $manager; @@ -45,18 +48,20 @@ public function setUp(): void { $this->manager = Server::get(ISharingManager::class); - $userManager = Server::get(IUserManager::class); - $userManager->clearBackends(); - $userManager->registerBackend(new Database()); - - $user1 = $userManager->createUser('user1', 'password'); - $this->assertNotFalse($user1); + $user1 = $this->createUser('user1', 'password'); $this->user1 = $user1; $userFolder = Server::get(IRootFolder::class)->getUserFolder($this->user1->getUID()); $this->node = $userFolder->newFile('foo.txt', 'bar'); - $this->sourceType = new NodeShareSourceType(Server::get(IEventDispatcher::class), $this->dbConnection, Server::get(IRootFolder::class), Server::get(IURLGenerator::class), $this->manager); + $this->sourceType = new NodeShareSourceType( + Server::get(IEventDispatcher::class), + $this->dbConnection, + Server::get(IRootFolder::class), + Server::get(IURLGenerator::class), + $this->manager, + Server::get(IFileAccess::class), + ); } #[\Override] @@ -74,13 +79,16 @@ public function testValidateSource(): void { } public function testGetSourceDisplayName(): void { - $this->assertEquals('foo.txt', $this->sourceType->getSourceDisplayName((string)$this->node->getId())); + $this->assertEquals('foo.txt', $this->sourceType->getSourceMetadata((string)$this->node->getId())?->getDisplayName()); } public function testGetSourceIcon(): void { $source = (string)$this->node->getId(); - $icon = $this->sourceType->getSourceIcon($source); + $icon = $this->sourceType->getSourceMetadata($source)?->getIcon(); + if (!$icon instanceof ShareIconURL) { + $this->fail('Unexpected share icon for ' . $source); + } foreach ([$icon->light, $icon->dark] as $url) { $this->assertStringStartsWith('http://localhost/index.php/core/preview?', $url); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 3b9131c36d69c..02f1089e2d1a3 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -67,8 +67,10 @@ 'NCU\\Sharing\\ShareAccessContext' => $baseDir . '/lib/unstable/Sharing/ShareAccessContext.php', 'NCU\\Sharing\\ShareState' => $baseDir . '/lib/unstable/Sharing/ShareState.php', 'NCU\\Sharing\\ShareUser' => $baseDir . '/lib/unstable/Sharing/ShareUser.php', + 'NCU\\Sharing\\Source\\IShareSourceMetadata' => $baseDir . '/lib/unstable/Sharing/Source/IShareSourceMetadata.php', 'NCU\\Sharing\\Source\\IShareSourceType' => $baseDir . '/lib/unstable/Sharing/Source/IShareSourceType.php', 'NCU\\Sharing\\Source\\ShareSource' => $baseDir . '/lib/unstable/Sharing/Source/ShareSource.php', + 'NCU\\Sharing\\Source\\ShareSourceMetadata' => $baseDir . '/lib/unstable/Sharing/Source/ShareSourceMetadata.php', 'NCU\\WorkflowEngine\\Events\\RegisterRuntimeOperationsEvent' => $baseDir . '/lib/unstable/WorkflowEngine/Events/RegisterRuntimeOperationsEvent.php', 'NCU\\WorkflowEngine\\RuntimeOperation' => $baseDir . '/lib/unstable/WorkflowEngine/RuntimeOperation.php', 'NCU\\WorkflowEngine\\RuntimeScope' => $baseDir . '/lib/unstable/WorkflowEngine/RuntimeScope.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index be73562e801fb..8f9599c042d02 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -108,8 +108,10 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'NCU\\Sharing\\ShareAccessContext' => __DIR__ . '/../../..' . '/lib/unstable/Sharing/ShareAccessContext.php', 'NCU\\Sharing\\ShareState' => __DIR__ . '/../../..' . '/lib/unstable/Sharing/ShareState.php', 'NCU\\Sharing\\ShareUser' => __DIR__ . '/../../..' . '/lib/unstable/Sharing/ShareUser.php', + 'NCU\\Sharing\\Source\\IShareSourceMetadata' => __DIR__ . '/../../..' . '/lib/unstable/Sharing/Source/IShareSourceMetadata.php', 'NCU\\Sharing\\Source\\IShareSourceType' => __DIR__ . '/../../..' . '/lib/unstable/Sharing/Source/IShareSourceType.php', 'NCU\\Sharing\\Source\\ShareSource' => __DIR__ . '/../../..' . '/lib/unstable/Sharing/Source/ShareSource.php', + 'NCU\\Sharing\\Source\\ShareSourceMetadata' => __DIR__ . '/../../..' . '/lib/unstable/Sharing/Source/ShareSourceMetadata.php', 'NCU\\WorkflowEngine\\Events\\RegisterRuntimeOperationsEvent' => __DIR__ . '/../../..' . '/lib/unstable/WorkflowEngine/Events/RegisterRuntimeOperationsEvent.php', 'NCU\\WorkflowEngine\\RuntimeOperation' => __DIR__ . '/../../..' . '/lib/unstable/WorkflowEngine/RuntimeOperation.php', 'NCU\\WorkflowEngine\\RuntimeScope' => __DIR__ . '/../../..' . '/lib/unstable/WorkflowEngine/RuntimeScope.php', diff --git a/lib/private/Sharing/SharingBackend.php b/lib/private/Sharing/SharingBackend.php index 5d6cb5d4017d2..fa8fa9e25c1f2 100644 --- a/lib/private/Sharing/SharingBackend.php +++ b/lib/private/Sharing/SharingBackend.php @@ -27,6 +27,7 @@ use NCU\Sharing\ShareAccessContext; use NCU\Sharing\ShareState; use NCU\Sharing\ShareUser; +use NCU\Sharing\Source\IShareSourceMetadata; use NCU\Sharing\Source\IShareSourceType; use NCU\Sharing\Source\ShareSource; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -697,9 +698,14 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID, $chunks = array_chunk(array_keys($shares), 1000); $registrySourceTypes = $this->registry->getSourceTypes(); - /** @var array, bool>> $shareSourceTypeClasses */ + /** @var array, bool>> $shareSourceTypeClasses */ $shareSourceTypeClasses = []; foreach ($chunks as $chunk) { + /** @var array, non-empty-string[]> $shareSourceValues */ + $shareSourceValues = []; + /** @var array, array> $shareSourceMetas */ + $shareSourceMetas = []; + $qb = $this->connection->getQueryBuilder(); $qb ->select( @@ -711,21 +717,38 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID, ->where($qb->expr()->in('ss.share_id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); $result = $qb->executeQuery(); - foreach ($result->fetchAll() as $row) { - /** @var class-string $typeClass */ + /** @var array{source_class: class-string, source_value: non-empty-string, share_id: int}[] $rows */ + $rows = $result->fetchAll(); + + foreach ($rows as $row) { + $typeClass = $row['source_class']; + $value = $row['source_value']; + + $shareSourceValues[$typeClass] ??= []; + $shareSourceValues[$typeClass][] = $value; + } + + foreach ($shareSourceValues as $typeClass => $values) { + if (($sourceType = ($this->registry->getSourceTypes()[$typeClass] ?? null)) === null) { + throw new RuntimeException('The source type is not registered: ' . $typeClass); + } + + $shareSourceMetas[$typeClass] = $sourceType->getSourcesMetadata($values); + } + + foreach ($rows as $row) { $typeClass = $row['source_class']; if (!isset($registrySourceTypes[$typeClass])) { // Skip sources that are currently not compatible, but don't remove them. continue; } - /** @var non-empty-string $value */ $value = $row['source_value']; - /** @var non-empty-string $id */ $id = (string)$row['share_id']; $shares[$id]['sources'][] = new ShareSource( $typeClass, $value, + $shareSourceMetas[$typeClass][$value] ?? null, ); $shareSourceTypeClasses[$id] ??= []; @@ -907,6 +930,7 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID, /** @var array, bool>> $shareCompatiblePermissionTypeClasses */ $shareCompatiblePermissionTypeClasses = []; foreach (array_keys($shares) as $id) { + $id = (string)$id; $shareCompatiblePermissionTypeClasses[$id] = []; foreach ($registryGenericPermissionTypeClasses as $permissionTypeClass) { $shareCompatiblePermissionTypeClasses[$id][$permissionTypeClass] = true; @@ -982,6 +1006,7 @@ private function list(ShareAccessContext $accessContext, ?string $filterShareID, } foreach (array_keys($shares) as $id) { + $id = (string)$id; foreach (array_keys($registryPropertyTypes) as $propertyTypeClass) { $share = $shares[$id]; if ( diff --git a/lib/private/Sharing/SharingManager.php b/lib/private/Sharing/SharingManager.php index 76593a8f4bc0b..dc96ad31a85ff 100644 --- a/lib/private/Sharing/SharingManager.php +++ b/lib/private/Sharing/SharingManager.php @@ -698,7 +698,7 @@ private function validateInteraction(ShareAccessContext $accessContext, Share $s $action = new ShareAction(null, array_values(array_map(static fn (SharePermission $permission): string => $permission->class, $share->getEnabledPermissions()))); $usersToCheck = []; - if ($share->owner->instance === null && ($ownerUser = $this->userManager->get($share->owner->userId)) !== null) { + if ($share->owner->instance === null && ($ownerUser = $this->userManager->get($share->owner->userId)) instanceof IUser) { $usersToCheck[] = $ownerUser; } diff --git a/lib/unstable/Sharing/Source/IShareSourceMetadata.php b/lib/unstable/Sharing/Source/IShareSourceMetadata.php new file mode 100644 index 0000000000000..676b54c21608e --- /dev/null +++ b/lib/unstable/Sharing/Source/IShareSourceMetadata.php @@ -0,0 +1,35 @@ + * @experimental 35.0.0 */ - public function getSourceIcon(string $source): null|ShareIconSVG|ShareIconURL; + public function getSourcesMetadata(array $sources): array; /** * @param non-empty-string $userId diff --git a/lib/unstable/Sharing/Source/ShareSource.php b/lib/unstable/Sharing/Source/ShareSource.php index 00b519dfc858d..59e6cb9f77351 100644 --- a/lib/unstable/Sharing/Source/ShareSource.php +++ b/lib/unstable/Sharing/Source/ShareSource.php @@ -20,18 +20,31 @@ * @experimental 35.0.0 */ #[Consumable(since: '35.0.0')] -final readonly class ShareSource { +final class ShareSource { /** * @experimental 35.0.0 */ public function __construct( /** @var class-string $class */ - public string $class, + public readonly string $class, /** @var non-empty-string $value */ - public string $value, + public readonly string $value, + private ?IShareSourceMetadata $metadata = null, ) { } + private function getMetadata(ISharingRegistry $registry): IShareSourceMetadata { + if (($sourceType = ($registry->getSourceTypes()[$this->class] ?? null)) === null) { + throw new RuntimeException('The source type is not registered: ' . $this->class); + } + + if (!$this->metadata instanceof \NCU\Sharing\Source\IShareSourceMetadata) { + $this->metadata = $sourceType->getSourceMetadata($this->value) ?? new ShareSourceMetadata($this->value, null); + } + + return $this->metadata; + } + /** * @return SharingSource * @experimental 35.0.0 @@ -41,7 +54,8 @@ public function format(ISharingRegistry $registry, IFactory $l10nFactory, bool $ throw new RuntimeException('The source type is not registered: ' . $this->class); } - $displayName = $sourceType->getSourceDisplayName($this->value) ?? $this->value; + $metadata = $this->getMetadata($registry); + $displayName = $metadata->getDisplayName(); if (!$isUnique) { $displayName .= ' (' . $sourceType->getDisplayName($l10nFactory) . ': ' . $this->value . ')'; } @@ -50,7 +64,7 @@ public function format(ISharingRegistry $registry, IFactory $l10nFactory, bool $ 'class' => $this->class, 'value' => $this->value, 'display_name' => $displayName, - 'icon' => $sourceType->getSourceIcon($this->value)?->format(), + 'icon' => $metadata->getIcon()?->format(), ]; } @@ -60,15 +74,13 @@ public function format(ISharingRegistry $registry, IFactory $l10nFactory, bool $ * @experimental 35.0.0 */ public static function formatMultiple(ISharingRegistry $registry, IFactory $l10nFactory, array $sources): array { - $sourceTypes = $registry->getSourceTypes(); - $sourceDisplayNames = []; foreach ($sources as $source) { - $displayName = $sourceTypes[$source->class]?->getSourceDisplayName($source->value) ?? $source->value; + $displayName = $source->getMetadata($registry)->getDisplayName(); $sourceDisplayNames[$displayName] ??= 0; ++$sourceDisplayNames[$displayName]; } - return array_map(static fn (ShareSource $source): array => $source->format($registry, $l10nFactory, $sourceDisplayNames[$sourceTypes[$source->class]?->getSourceDisplayName($source->value) ?? $source->value] === 1), $sources); + return array_map(static fn (ShareSource $source): array => $source->format($registry, $l10nFactory, $sourceDisplayNames[$source->getMetadata($registry)->getDisplayName()] === 1), $sources); } } diff --git a/lib/unstable/Sharing/Source/ShareSourceMetadata.php b/lib/unstable/Sharing/Source/ShareSourceMetadata.php new file mode 100644 index 0000000000000..4030454f608bb --- /dev/null +++ b/lib/unstable/Sharing/Source/ShareSourceMetadata.php @@ -0,0 +1,50 @@ +displayName; + } + + /** + * Get the icon for the share source + * + * @experimental 35.0.0 + */ + #[\Override] + public function getIcon(): null|ShareIconSVG|ShareIconURL { + return $this->icon; + } +} diff --git a/tests/lib/Sharing/TestShareSourceType1.php b/tests/lib/Sharing/TestShareSourceType1.php index e61bd869db945..79d3de636ec24 100644 --- a/tests/lib/Sharing/TestShareSourceType1.php +++ b/tests/lib/Sharing/TestShareSourceType1.php @@ -10,8 +10,9 @@ namespace Test\Sharing; use NCU\Sharing\Icon\ShareIconSVG; -use NCU\Sharing\Icon\ShareIconURL; +use NCU\Sharing\Source\IShareSourceMetadata; use NCU\Sharing\Source\IShareSourceType; +use NCU\Sharing\Source\ShareSourceMetadata; use OCP\Interaction\InteractionResource; use OCP\L10N\IFactory; @@ -35,13 +36,22 @@ public function validateSource(string $source): bool { } #[\Override] - public function getSourceDisplayName(string $source): ?string { - return $this->validSources[$source]; + public function getSourceMetadata(string $source): ?IShareSourceMetadata { + if (isset($this->validSources[$source])) { + return new ShareSourceMetadata( + $this->validSources[$source], + new ShareIconSVG(''), + ); + } + + return null; } #[\Override] - public function getSourceIcon(string $source): null|ShareIconSVG|ShareIconURL { - return new ShareIconSVG(''); + public function getSourcesMetadata(array $sources): array { + $metas = array_map($this->getSourceMetadata(...), $sources); + $metas = array_combine($sources, $metas); + return array_filter($metas); } #[\Override] diff --git a/tests/lib/Traits/UserTrait.php b/tests/lib/Traits/UserTrait.php index 43b1219677bb0..73c43053ea2f1 100644 --- a/tests/lib/Traits/UserTrait.php +++ b/tests/lib/Traits/UserTrait.php @@ -34,17 +34,17 @@ public function getUID(): string { trait UserTrait { protected Dummy $userBackend; - protected function createUser($name, $password): IUser { + protected function createUser(string $name, string $password): IUser { $this->userBackend->createUser($name, $password); return new DummyUser($name); } - protected function setUpUserTrait() { + protected function setUpUserTrait(): void { $this->userBackend = new Dummy(); Server::get(IUserManager::class)->registerBackend($this->userBackend); } - protected function tearDownUserTrait() { + protected function tearDownUserTrait(): void { Server::get(IUserManager::class)->removeBackend($this->userBackend); } }