From 7e3af671a3f6f5cb86dabc7029ee02f4057c2a10 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Mon, 20 Jul 2026 12:02:39 +0200 Subject: [PATCH 1/2] refactor(core): Use constructor properties in Sharing Signed-off-by: provokateurin --- core/AppInfo/Application.php | 10 +++---- .../Permission/ReshareSharePermissionType.php | 12 ++++---- .../ExpirationDateSharePropertyType.php | 29 ++++++++----------- .../Property/PasswordSharePropertyType.php | 18 ++++-------- .../Recipient/EmailShareRecipientType.php | 10 +++---- .../Recipient/GroupShareRecipientType.php | 14 +++------ .../Recipient/TeamShareRecipientType.php | 14 +++------ .../Recipient/TokenShareRecipientType.php | 12 ++++---- .../Recipient/UserShareRecipientType.php | 15 ++++------ .../ExpirationDateSharePropertyTypeTest.php | 2 +- .../PasswordSharePropertyTypeTest.php | 2 +- .../Recipient/EmailShareRecipientTypeTest.php | 2 +- .../Recipient/GroupShareRecipientTypeTest.php | 2 +- .../Recipient/TeamShareRecipientTypeTest.php | 2 +- .../Recipient/TokenShareRecipientTypeTest.php | 3 +- .../Recipient/UserShareRecipientTypeTest.php | 2 +- .../Sharing/AbstractSharingManagerTests.php | 4 +-- 17 files changed, 60 insertions(+), 93 deletions(-) diff --git a/core/AppInfo/Application.php b/core/AppInfo/Application.php index c2c5c20b0a1e2..fd396fa7f08dc 100644 --- a/core/AppInfo/Application.php +++ b/core/AppInfo/Application.php @@ -118,13 +118,13 @@ public function register(IRegistrationContext $context): void { $registry = Server::get(ISharingRegistry::class); - $registry->registerRecipientType(new EmailShareRecipientType()); + $registry->registerRecipientType(Server::get(EmailShareRecipientType::class)); $registry->registerRecipientType(Server::get(GroupShareRecipientType::class)); $registry->registerRecipientType(Server::get(TeamShareRecipientType::class)); - $registry->registerRecipientType(new TokenShareRecipientType()); + $registry->registerRecipientType(Server::get(TokenShareRecipientType::class)); $registry->registerRecipientType(Server::get(UserShareRecipientType::class)); - $registry->registerPropertyType(new ExpirationDateSharePropertyType()); + $registry->registerPropertyType(Server::get(ExpirationDateSharePropertyType::class)); $registry->markPropertyTypeCompatibleWithRecipientType(ExpirationDateSharePropertyType::class, EmailShareRecipientType::class); $registry->markPropertyTypeCompatibleWithRecipientType(ExpirationDateSharePropertyType::class, GroupShareRecipientType::class); $registry->markPropertyTypeCompatibleWithRecipientType(ExpirationDateSharePropertyType::class, TeamShareRecipientType::class); @@ -141,14 +141,14 @@ public function register(IRegistrationContext $context): void { $registry->markPropertyTypeCompatibleWithRecipientType(NoteSharePropertyType::class, TokenShareRecipientType::class); $registry->markPropertyTypeCompatibleWithRecipientType(NoteSharePropertyType::class, UserShareRecipientType::class); - $registry->registerPropertyType(new PasswordSharePropertyType()); + $registry->registerPropertyType(Server::get(PasswordSharePropertyType::class)); $registry->markPropertyTypeCompatibleWithRecipientType(PasswordSharePropertyType::class, EmailShareRecipientType::class); $registry->markPropertyTypeCompatibleWithRecipientType(PasswordSharePropertyType::class, TokenShareRecipientType::class); $registry->registerPermissionPreset(new ViewSharePermissionPreset()); $registry->registerPermissionPreset(new EditSharePermissionPreset()); - $registry->registerPermissionType(null, new ReshareSharePermissionType()); + $registry->registerPermissionType(null, Server::get(ReshareSharePermissionType::class)); // Cannot use the APP_ID from files_sharing Application and EXCLUDE_RESHARE_FROM_EDIT from files_sharing ConfigLexicon, because the classes are not registered yet. if (!Server::get(IAppConfig::class)->getValueBool('files_sharing', 'shareapi_exclude_reshare_from_edit')) { $registry->markPermissionTypeCompatibleWithPermissionPreset(ReshareSharePermissionType::class, EditSharePermissionPreset::class); diff --git a/core/Sharing/Permission/ReshareSharePermissionType.php b/core/Sharing/Permission/ReshareSharePermissionType.php index 930e8d413dcae..1100571169c3b 100644 --- a/core/Sharing/Permission/ReshareSharePermissionType.php +++ b/core/Sharing/Permission/ReshareSharePermissionType.php @@ -13,14 +13,12 @@ use OCP\Constants; use OCP\IAppConfig; use OCP\L10N\IFactory; -use OCP\Server; use OCP\Sharing\Permission\ISharePermissionType; -final class ReshareSharePermissionType implements ISharePermissionType { - private ?IAppConfig $appConfig = null; - - private function getAppConfig(): IAppConfig { - return $this->appConfig ??= Server::get(IAppConfig::class); +final readonly class ReshareSharePermissionType implements ISharePermissionType { + public function __construct( + private IAppConfig $appConfig, + ) { } #[\Override] @@ -40,6 +38,6 @@ public function getPriority(): int { #[\Override] public function isEnabledByDefault(): bool { - return ($this->getAppConfig()->getValueInt(Application::APP_ID, 'shareapi_default_permissions') & Constants::PERMISSION_SHARE) === Constants::PERMISSION_SHARE; + return ($this->appConfig->getValueInt(Application::APP_ID, 'shareapi_default_permissions') & Constants::PERMISSION_SHARE) === Constants::PERMISSION_SHARE; } } diff --git a/core/Sharing/Property/ExpirationDateSharePropertyType.php b/core/Sharing/Property/ExpirationDateSharePropertyType.php index 525333c7ba040..fac8c55a280e2 100644 --- a/core/Sharing/Property/ExpirationDateSharePropertyType.php +++ b/core/Sharing/Property/ExpirationDateSharePropertyType.php @@ -14,7 +14,6 @@ use DateTimeInterface; use OC\Core\AppInfo\Application; use OCP\L10N\IFactory; -use OCP\Server; use OCP\Share\IManager; use OCP\Sharing\Property\ADateSharePropertyType; use OCP\Sharing\Property\ISharePropertyTypeFilter; @@ -26,13 +25,9 @@ final class ExpirationDateSharePropertyType extends ADateSharePropertyType implements ISharePropertyTypeFilter { private readonly DateTimeImmutable $now; - private ?IManager $legacyManager = null; - - private function getLegacyManager(): IManager { - return $this->legacyManager ??= Server::get(IManager::class); - } - - public function __construct() { + public function __construct( + private readonly IManager $legacyManager, + ) { $this->now = new DateTimeImmutable(); } @@ -58,15 +53,15 @@ public function isAdvanced(): bool { #[\Override] public function isRequired(): bool { - if ($this->getLegacyManager()->shareApiLinkDefaultExpireDateEnforced()) { + if ($this->legacyManager->shareApiLinkDefaultExpireDateEnforced()) { return true; } - if ($this->getLegacyManager()->shareApiRemoteDefaultExpireDateEnforced()) { + if ($this->legacyManager->shareApiRemoteDefaultExpireDateEnforced()) { return true; } - return $this->getLegacyManager()->shareApiInternalDefaultExpireDateEnforced(); + return $this->legacyManager->shareApiInternalDefaultExpireDateEnforced(); } #[\Override] @@ -93,16 +88,16 @@ public function getMaxDate(): ?DateTimeImmutable { private function getMaxExpirationDate(): ?DateTimeImmutable { // We do not have any distinction between link/remote/internal, so we just apply the lowest expiration days count to be safe. $days = INF; - if ($this->getLegacyManager()->shareApiLinkDefaultExpireDate()) { - $days = min($days, $this->getLegacyManager()->shareApiLinkDefaultExpireDays()); + if ($this->legacyManager->shareApiLinkDefaultExpireDate()) { + $days = min($days, $this->legacyManager->shareApiLinkDefaultExpireDays()); } - if ($this->getLegacyManager()->shareApiRemoteDefaultExpireDate()) { - $days = min($days, $this->getLegacyManager()->shareApiRemoteDefaultExpireDays()); + if ($this->legacyManager->shareApiRemoteDefaultExpireDate()) { + $days = min($days, $this->legacyManager->shareApiRemoteDefaultExpireDays()); } - if ($this->getLegacyManager()->shareApiInternalDefaultExpireDate()) { - $days = min($days, $this->getLegacyManager()->shareApiInternalDefaultExpireDays()); + if ($this->legacyManager->shareApiInternalDefaultExpireDate()) { + $days = min($days, $this->legacyManager->shareApiInternalDefaultExpireDays()); } if ($days !== INF) { diff --git a/core/Sharing/Property/PasswordSharePropertyType.php b/core/Sharing/Property/PasswordSharePropertyType.php index 6932e7f9a0953..6b491ac2d9704 100644 --- a/core/Sharing/Property/PasswordSharePropertyType.php +++ b/core/Sharing/Property/PasswordSharePropertyType.php @@ -12,7 +12,6 @@ use OC\Core\AppInfo\Application; use OCP\L10N\IFactory; use OCP\Security\IHasher; -use OCP\Server; use OCP\Share\IManager; use OCP\Sharing\Property\APasswordSharePropertyType; use OCP\Sharing\Property\ISharePropertyTypeFilter; @@ -20,16 +19,11 @@ use OCP\Sharing\ShareAccessContext; final class PasswordSharePropertyType extends APasswordSharePropertyType implements ISharePropertyTypeFilter { - private ?IManager $legacyManager = null; - private ?IHasher $hasher = null; - - private function getLegacyManager(): IManager { - return $this->legacyManager ??= Server::get(IManager::class); - } - - private function getHasher(): IHasher { - return $this->hasher ??= Server::get(IHasher::class); + public function __construct( + private readonly IManager $legacyManager, + private readonly IHasher $hasher, + ) { } #[\Override] @@ -55,7 +49,7 @@ public function isAdvanced(): bool { #[\Override] public function isRequired(): bool { // TODO: Enable group memberships check based on the owner. - return $this->getLegacyManager()->shareApiLinkEnforcePassword(false); + return $this->legacyManager->shareApiLinkEnforcePassword(false); } #[\Override] @@ -72,7 +66,7 @@ public function isFiltered(ShareAccessContext $accessContext, Share $share): boo if (($property = $share->properties[self::class] ?? null) !== null && $property->value !== null) { // TODO: Check if the hash has to be updated and save it. - return !$this->getHasher()->verify($argument, $property->value); + return !$this->hasher->verify($argument, $property->value); } return false; diff --git a/core/Sharing/Recipient/EmailShareRecipientType.php b/core/Sharing/Recipient/EmailShareRecipientType.php index 42f1c5e97edbf..82d1a77cc739e 100644 --- a/core/Sharing/Recipient/EmailShareRecipientType.php +++ b/core/Sharing/Recipient/EmailShareRecipientType.php @@ -15,7 +15,6 @@ use OCP\IUser; use OCP\L10N\IFactory; use OCP\Mail\IEmailValidator; -use OCP\Server; use OCP\Share\IShare; use OCP\Sharing\Icon\ShareIconSVG; use OCP\Sharing\Icon\ShareIconURL; @@ -24,10 +23,9 @@ // TODO: Add logic to send emails when share state is updated to active final class EmailShareRecipientType extends AShareRecipientTypeSearchCollaborator { - private ?IEmailValidator $emailValidator = null; - - private function getEmailValidator(): IEmailValidator { - return $this->emailValidator ??= Server::get(IEmailValidator::class); + public function __construct( + private readonly IEmailValidator $emailValidator, + ) { } #[\Override] @@ -37,7 +35,7 @@ public function getDisplayName(IFactory $l10nFactory): string { #[\Override] public function validateRecipient(string $recipient): bool { - return $this->getEmailValidator()->isValid($recipient); + return $this->emailValidator->isValid($recipient); } #[\Override] diff --git a/core/Sharing/Recipient/GroupShareRecipientType.php b/core/Sharing/Recipient/GroupShareRecipientType.php index 5e46745cced98..c1c3970dd4695 100644 --- a/core/Sharing/Recipient/GroupShareRecipientType.php +++ b/core/Sharing/Recipient/GroupShareRecipientType.php @@ -21,7 +21,6 @@ use OCP\Interaction\Receivers\GroupReceiver; use OCP\IUser; use OCP\L10N\IFactory; -use OCP\Server; use OCP\Share\IShare; use OCP\Sharing\Icon\ShareIconSVG; use OCP\Sharing\Icon\ShareIconURL; @@ -34,20 +33,15 @@ * @template-implements IEventListener */ final class GroupShareRecipientType extends AShareRecipientTypeSearchCollaborator implements IEventListener { - private ?IGroupManager $groupManager = null; - public function __construct( IEventDispatcher $eventDispatcher, private readonly IDBConnection $dbConnection, + private readonly IGroupManager $groupManager, private readonly ISharingManager $manager, ) { $eventDispatcher->addServiceListener(GroupDeletedEvent::class, self::class); } - private function getGroupManager(): IGroupManager { - return $this->groupManager ??= Server::get(IGroupManager::class); - } - #[\Override] public function getDisplayName(IFactory $l10nFactory): string { return $l10nFactory->get(Application::APP_ID)->t('Group'); @@ -55,7 +49,7 @@ public function getDisplayName(IFactory $l10nFactory): string { #[\Override] public function validateRecipient(string $recipient): bool { - return $this->getGroupManager()->groupExists($recipient); + return $this->groupManager->groupExists($recipient); } #[\Override] @@ -64,12 +58,12 @@ public function getRecipients(?IUser $currentUser, mixed $arguments): array { return []; } - return $this->getGroupManager()->getUserGroupIds($currentUser); + return $this->groupManager->getUserGroupIds($currentUser); } #[\Override] public function getRecipientDisplayName(string $recipient): ?string { - $displayName = $this->getGroupManager()->getDisplayName($recipient); + $displayName = $this->groupManager->getDisplayName($recipient); if ($displayName === '') { return null; } diff --git a/core/Sharing/Recipient/TeamShareRecipientType.php b/core/Sharing/Recipient/TeamShareRecipientType.php index 229c2a6f2a6e7..e5e455ea8c73d 100644 --- a/core/Sharing/Recipient/TeamShareRecipientType.php +++ b/core/Sharing/Recipient/TeamShareRecipientType.php @@ -20,7 +20,6 @@ use OCP\Interaction\Receivers\CircleReceiver; use OCP\IUser; use OCP\L10N\IFactory; -use OCP\Server; use OCP\Share\IShare; use OCP\Sharing\Icon\ShareIconSVG; use OCP\Sharing\Icon\ShareIconURL; @@ -35,20 +34,15 @@ * @template-implements IEventListener */ final class TeamShareRecipientType extends AShareRecipientTypeSearchCollaborator implements IEventListener { - private ?ITeamManager $teamManager = null; - public function __construct( IEventDispatcher $eventDispatcher, private readonly IDBConnection $dbConnection, + private readonly ITeamManager $teamManager, private readonly ISharingManager $manager, ) { $eventDispatcher->addServiceListener(DestroyingCircleEvent::class, self::class); } - private function getTeamManager(): ITeamManager { - return $this->teamManager ??= Server::get(ITeamManager::class); - } - #[\Override] public function getDisplayName(IFactory $l10nFactory): string { return $l10nFactory->get(Application::APP_ID)->t('Team'); @@ -56,7 +50,7 @@ public function getDisplayName(IFactory $l10nFactory): string { #[\Override] public function validateRecipient(string $recipient): bool { - return $this->getTeamManager()->getTeam($recipient) instanceof Team; + return $this->teamManager->getTeam($recipient) instanceof Team; } #[\Override] @@ -65,12 +59,12 @@ public function getRecipients(?IUser $currentUser, mixed $arguments): array { return []; } - return array_map(static fn (Team $team): string => $team->getId(), $this->getTeamManager()->getTeamsForUser($currentUser->getUID())); + return array_map(static fn (Team $team): string => $team->getId(), $this->teamManager->getTeamsForUser($currentUser->getUID())); } #[\Override] public function getRecipientDisplayName(string $recipient): ?string { - return $this->getTeamManager()->getTeam($recipient)?->getDisplayName(); + return $this->teamManager->getTeam($recipient)?->getDisplayName(); } #[\Override] diff --git a/core/Sharing/Recipient/TokenShareRecipientType.php b/core/Sharing/Recipient/TokenShareRecipientType.php index 1a669e1318f0f..1ddebc7d25e5b 100644 --- a/core/Sharing/Recipient/TokenShareRecipientType.php +++ b/core/Sharing/Recipient/TokenShareRecipientType.php @@ -14,7 +14,6 @@ use OCP\Interaction\Receivers\LinkReceiver; use OCP\IUser; use OCP\L10N\IFactory; -use OCP\Server; use OCP\Share\IManager; use OCP\Sharing\Icon\ShareIconSVG; use OCP\Sharing\Icon\ShareIconURL; @@ -22,11 +21,10 @@ use OCP\Sharing\Recipient\IShareRecipientTypePublicSecret; // TODO: Rename to Link? -final class TokenShareRecipientType implements IShareRecipientType, IShareRecipientTypePublicSecret { - private ?IManager $legacyManager = null; - - private function getLegacyManager(): IManager { - return $this->legacyManager ??= Server::get(IManager::class); +final readonly class TokenShareRecipientType implements IShareRecipientType, IShareRecipientTypePublicSecret { + public function __construct( + private IManager $legacyManager, + ) { } #[\Override] @@ -66,6 +64,6 @@ public function isSecretPublic(string $recipient): bool { #[\Override] public function isSecretUpdatable(string $recipient): bool { - return $this->getLegacyManager()->allowCustomTokens(); + return $this->legacyManager->allowCustomTokens(); } } diff --git a/core/Sharing/Recipient/UserShareRecipientType.php b/core/Sharing/Recipient/UserShareRecipientType.php index abeff761a9906..29b8410af4245 100644 --- a/core/Sharing/Recipient/UserShareRecipientType.php +++ b/core/Sharing/Recipient/UserShareRecipientType.php @@ -20,7 +20,6 @@ use OCP\IUser; use OCP\IUserManager; use OCP\L10N\IFactory; -use OCP\Server; use OCP\Share\IShare; use OCP\Sharing\Icon\ShareIconURL; use OCP\Sharing\ISharingManager; @@ -33,20 +32,16 @@ * @template-implements IEventListener */ final class UserShareRecipientType extends AShareRecipientTypeSearchCollaborator implements IEventListener { - private ?IUserManager $userManager = null; public function __construct( IEventDispatcher $eventDispatcher, private readonly IDBConnection $dbConnection, + private readonly IUserManager $userManager, private readonly ISharingManager $manager, ) { $eventDispatcher->addServiceListener(UserDeletedEvent::class, self::class); } - private function getUserManager(): IUserManager { - return $this->userManager ??= Server::get(IUserManager::class); - } - #[\Override] public function getDisplayName(IFactory $l10nFactory): string { return $l10nFactory->get(Application::APP_ID)->t('User'); @@ -54,7 +49,7 @@ public function getDisplayName(IFactory $l10nFactory): string { #[\Override] public function validateRecipient(string $recipient): bool { - return $this->getUserManager()->userExists($recipient); + return $this->userManager->userExists($recipient); } #[\Override] @@ -68,14 +63,14 @@ public function getRecipients(?IUser $currentUser, mixed $arguments): array { #[\Override] public function getRecipientDisplayName(string $recipient): ?string { - return $this->getUserManager()->getDisplayName($recipient); + return $this->userManager->getDisplayName($recipient); } #[\Override] public function getRecipientIcon(string $recipient): ShareIconURL { return new ShareIconURL( - $this->getUserManager()->getAvatarUrlLight($recipient, 64), - $this->getUserManager()->getAvatarUrlDark($recipient, 64), + $this->userManager->getAvatarUrlLight($recipient, 64), + $this->userManager->getAvatarUrlDark($recipient, 64), ); } diff --git a/tests/Core/Sharing/Property/ExpirationDateSharePropertyTypeTest.php b/tests/Core/Sharing/Property/ExpirationDateSharePropertyTypeTest.php index 0de4bc1c87c1a..f4c5fb1f854fd 100644 --- a/tests/Core/Sharing/Property/ExpirationDateSharePropertyTypeTest.php +++ b/tests/Core/Sharing/Property/ExpirationDateSharePropertyTypeTest.php @@ -41,7 +41,7 @@ public function setUp(): void { $this->assertNotFalse($user); $this->user = $user; - $this->propertyType = new ExpirationDateSharePropertyType(); + $this->propertyType = Server::get(ExpirationDateSharePropertyType::class); } #[\Override] diff --git a/tests/Core/Sharing/Property/PasswordSharePropertyTypeTest.php b/tests/Core/Sharing/Property/PasswordSharePropertyTypeTest.php index 1bbf3c4b374a5..febd440646931 100644 --- a/tests/Core/Sharing/Property/PasswordSharePropertyTypeTest.php +++ b/tests/Core/Sharing/Property/PasswordSharePropertyTypeTest.php @@ -36,7 +36,7 @@ public function setUp(): void { $this->assertNotFalse($user); $this->user = $user; - $this->propertyType = new PasswordSharePropertyType(); + $this->propertyType = Server::get(PasswordSharePropertyType::class); } #[\Override] diff --git a/tests/Core/Sharing/Recipient/EmailShareRecipientTypeTest.php b/tests/Core/Sharing/Recipient/EmailShareRecipientTypeTest.php index c4cab57f93152..8e04826a5c146 100644 --- a/tests/Core/Sharing/Recipient/EmailShareRecipientTypeTest.php +++ b/tests/Core/Sharing/Recipient/EmailShareRecipientTypeTest.php @@ -46,7 +46,7 @@ public function setUp(): void { self::loginAsUser($this->user1->getUID()); - $this->recipientType = new EmailShareRecipientType(); + $this->recipientType = Server::get(EmailShareRecipientType::class); } #[\Override] diff --git a/tests/Core/Sharing/Recipient/GroupShareRecipientTypeTest.php b/tests/Core/Sharing/Recipient/GroupShareRecipientTypeTest.php index d9caa4658c6ae..e9af4005348b2 100644 --- a/tests/Core/Sharing/Recipient/GroupShareRecipientTypeTest.php +++ b/tests/Core/Sharing/Recipient/GroupShareRecipientTypeTest.php @@ -82,7 +82,7 @@ public function setUp(): void { $this->group1->addUser($this->user1); - $this->recipientType = new GroupShareRecipientType(Server::get(IEventDispatcher::class), $this->dbConnection, $this->manager); + $this->recipientType = new GroupShareRecipientType(Server::get(IEventDispatcher::class), $this->dbConnection, $groupManager, $this->manager); } #[\Override] diff --git a/tests/Core/Sharing/Recipient/TeamShareRecipientTypeTest.php b/tests/Core/Sharing/Recipient/TeamShareRecipientTypeTest.php index a1c9c38a4018b..82c9af1231c6f 100644 --- a/tests/Core/Sharing/Recipient/TeamShareRecipientTypeTest.php +++ b/tests/Core/Sharing/Recipient/TeamShareRecipientTypeTest.php @@ -98,7 +98,7 @@ public function setUp(): void { $this->team2 = $this->createTeam($teamManager, 'team2'); $this->team3 = $this->createTeam($teamManager, 'team3'); - $this->recipientType = new TeamShareRecipientType(Server::get(IEventDispatcher::class), $this->dbConnection, $this->manager); + $this->recipientType = new TeamShareRecipientType(Server::get(IEventDispatcher::class), $this->dbConnection, $teamManager, $this->manager); } #[\Override] diff --git a/tests/Core/Sharing/Recipient/TokenShareRecipientTypeTest.php b/tests/Core/Sharing/Recipient/TokenShareRecipientTypeTest.php index 388c3c6b5fa72..e26d80b98abed 100644 --- a/tests/Core/Sharing/Recipient/TokenShareRecipientTypeTest.php +++ b/tests/Core/Sharing/Recipient/TokenShareRecipientTypeTest.php @@ -10,6 +10,7 @@ namespace Tests\Core\Sharing\Recipient; use OC\Core\Sharing\Recipient\TokenShareRecipientType; +use OCP\Server; use PHPUnit\Framework\Attributes\Group; use Test\TestCase; @@ -21,7 +22,7 @@ final class TokenShareRecipientTypeTest extends TestCase { public function setUp(): void { parent::setUp(); - $this->recipientType = new TokenShareRecipientType(); + $this->recipientType = Server::get(TokenShareRecipientType::class); } public function testValidateRecipient(): void { diff --git a/tests/Core/Sharing/Recipient/UserShareRecipientTypeTest.php b/tests/Core/Sharing/Recipient/UserShareRecipientTypeTest.php index 86aca7e66342c..88b25f8e1bbb7 100644 --- a/tests/Core/Sharing/Recipient/UserShareRecipientTypeTest.php +++ b/tests/Core/Sharing/Recipient/UserShareRecipientTypeTest.php @@ -72,7 +72,7 @@ public function setUp(): void { $this->user3 = $this->createUser($userManager, 'user3', 'password'); $this->user4 = $this->createUser($userManager, 'user4', 'password'); - $this->recipientType = new UserShareRecipientType(Server::get(IEventDispatcher::class), $this->dbConnection, $this->manager); + $this->recipientType = new UserShareRecipientType(Server::get(IEventDispatcher::class), $this->dbConnection, $userManager, $this->manager); } #[\Override] diff --git a/tests/lib/Sharing/AbstractSharingManagerTests.php b/tests/lib/Sharing/AbstractSharingManagerTests.php index 27e32eaadb675..76bf834abc2e4 100644 --- a/tests/lib/Sharing/AbstractSharingManagerTests.php +++ b/tests/lib/Sharing/AbstractSharingManagerTests.php @@ -138,7 +138,7 @@ public function setUp(): void { $this->registry->markPermissionTypeCompatibleWithPermissionPreset(TestSharePermissionType1::class, TestSharePermissionPreset2::class); $this->registry->registerPermissionType(TestShareSourceType2::class, new TestSharePermissionType2()); $this->registry->markPermissionTypeCompatibleWithPermissionPreset(TestSharePermissionType2::class, TestSharePermissionPreset2::class); - $this->registry->registerPermissionType(null, new ReshareSharePermissionType()); + $this->registry->registerPermissionType(null, Server::get(ReshareSharePermissionType::class)); } #[\Override] @@ -2953,7 +2953,7 @@ public function testGetShareWithSecret(): void { true, false, )); - $this->registry->registerPermissionType(null, new ReshareSharePermissionType()); + $this->registry->registerPermissionType(null, Server::get(ReshareSharePermissionType::class)); $accessContext = new ShareAccessContext($this->owner); From 513ecea5a594890829792ccb82018b3b311a0257 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Mon, 20 Jul 2026 12:02:46 +0200 Subject: [PATCH 2/2] refactor(files): Use constructor properties in Sharing Signed-off-by: provokateurin --- apps/files/lib/AppInfo/Application.php | 8 +++--- .../NodeCreateSharePermissionType.php | 12 ++++----- .../NodeDeleteSharePermissionType.php | 12 ++++----- .../NodeDownloadSharePermissionType.php | 12 ++++----- .../NodeUpdateSharePermissionType.php | 12 ++++----- .../Sharing/Source/NodeShareSourceType.php | 27 ++++++------------- .../Source/NodeShareSourceTypeTest.php | 3 ++- 7 files changed, 34 insertions(+), 52 deletions(-) diff --git a/apps/files/lib/AppInfo/Application.php b/apps/files/lib/AppInfo/Application.php index ad8f886665368..a75858884c4b6 100644 --- a/apps/files/lib/AppInfo/Application.php +++ b/apps/files/lib/AppInfo/Application.php @@ -116,20 +116,20 @@ public function register(IRegistrationContext $context): void { $registry->markPropertyTypeCompatibleWithSourceType(NodeGridViewSharePropertyType::class, NodeShareSourceType::class); $registry->markPropertyTypeCompatibleWithRecipientType(NodeGridViewSharePropertyType::class, TokenShareRecipientType::class); - $registry->registerPermissionType(NodeShareSourceType::class, new NodeCreateSharePermissionType()); + $registry->registerPermissionType(NodeShareSourceType::class, Server::get(NodeCreateSharePermissionType::class)); $registry->markPermissionTypeCompatibleWithPermissionPreset(NodeCreateSharePermissionType::class, EditSharePermissionPreset::class); $registry->registerPermissionType(NodeShareSourceType::class, new NodeReadSharePermissionType()); $registry->markPermissionTypeCompatibleWithPermissionPreset(NodeReadSharePermissionType::class, ViewSharePermissionPreset::class); $registry->markPermissionTypeCompatibleWithPermissionPreset(NodeReadSharePermissionType::class, EditSharePermissionPreset::class); - $registry->registerPermissionType(NodeShareSourceType::class, new NodeUpdateSharePermissionType()); + $registry->registerPermissionType(NodeShareSourceType::class, Server::get(NodeUpdateSharePermissionType::class)); $registry->markPermissionTypeCompatibleWithPermissionPreset(NodeUpdateSharePermissionType::class, EditSharePermissionPreset::class); - $registry->registerPermissionType(NodeShareSourceType::class, new NodeDeleteSharePermissionType()); + $registry->registerPermissionType(NodeShareSourceType::class, Server::get(NodeDeleteSharePermissionType::class)); $registry->markPermissionTypeCompatibleWithPermissionPreset(NodeDeleteSharePermissionType::class, EditSharePermissionPreset::class); - $registry->registerPermissionType(NodeShareSourceType::class, new NodeDownloadSharePermissionType()); + $registry->registerPermissionType(NodeShareSourceType::class, Server::get(NodeDownloadSharePermissionType::class)); $registry->markPermissionTypeCompatibleWithPermissionPreset(NodeDownloadSharePermissionType::class, ViewSharePermissionPreset::class); $registry->markPermissionTypeCompatibleWithPermissionPreset(NodeDownloadSharePermissionType::class, EditSharePermissionPreset::class); } diff --git a/apps/files/lib/Sharing/Permission/NodeCreateSharePermissionType.php b/apps/files/lib/Sharing/Permission/NodeCreateSharePermissionType.php index 476d491441346..a34369ab4b269 100644 --- a/apps/files/lib/Sharing/Permission/NodeCreateSharePermissionType.php +++ b/apps/files/lib/Sharing/Permission/NodeCreateSharePermissionType.php @@ -13,14 +13,12 @@ use OCP\Constants; use OCP\IAppConfig; use OCP\L10N\IFactory; -use OCP\Server; use OCP\Sharing\Permission\ISharePermissionType; -final class NodeCreateSharePermissionType implements ISharePermissionType { - private ?IAppConfig $appConfig = null; - - private function getAppConfig(): IAppConfig { - return $this->appConfig ??= Server::get(IAppConfig::class); +final readonly class NodeCreateSharePermissionType implements ISharePermissionType { + public function __construct( + private IAppConfig $appConfig, + ) { } #[\Override] @@ -40,6 +38,6 @@ public function getPriority(): int { #[\Override] public function isEnabledByDefault(): bool { - return ($this->getAppConfig()->getValueInt(\OC\Core\AppInfo\Application::APP_ID, 'shareapi_default_permissions') & Constants::PERMISSION_CREATE) === Constants::PERMISSION_CREATE; + return ($this->appConfig->getValueInt(\OC\Core\AppInfo\Application::APP_ID, 'shareapi_default_permissions') & Constants::PERMISSION_CREATE) === Constants::PERMISSION_CREATE; } } diff --git a/apps/files/lib/Sharing/Permission/NodeDeleteSharePermissionType.php b/apps/files/lib/Sharing/Permission/NodeDeleteSharePermissionType.php index bcaf4825435f8..60468009d31d2 100644 --- a/apps/files/lib/Sharing/Permission/NodeDeleteSharePermissionType.php +++ b/apps/files/lib/Sharing/Permission/NodeDeleteSharePermissionType.php @@ -13,14 +13,12 @@ use OCP\Constants; use OCP\IAppConfig; use OCP\L10N\IFactory; -use OCP\Server; use OCP\Sharing\Permission\ISharePermissionType; -final class NodeDeleteSharePermissionType implements ISharePermissionType { - private ?IAppConfig $appConfig = null; - - private function getAppConfig(): IAppConfig { - return $this->appConfig ??= Server::get(IAppConfig::class); +final readonly class NodeDeleteSharePermissionType implements ISharePermissionType { + public function __construct( + private IAppConfig $appConfig, + ) { } #[\Override] @@ -40,6 +38,6 @@ public function getPriority(): int { #[\Override] public function isEnabledByDefault(): bool { - return ($this->getAppConfig()->getValueInt(\OC\Core\AppInfo\Application::APP_ID, 'shareapi_default_permissions') & Constants::PERMISSION_DELETE) === Constants::PERMISSION_DELETE; + return ($this->appConfig->getValueInt(\OC\Core\AppInfo\Application::APP_ID, 'shareapi_default_permissions') & Constants::PERMISSION_DELETE) === Constants::PERMISSION_DELETE; } } diff --git a/apps/files/lib/Sharing/Permission/NodeDownloadSharePermissionType.php b/apps/files/lib/Sharing/Permission/NodeDownloadSharePermissionType.php index f513199517763..8798e7cfd93c5 100644 --- a/apps/files/lib/Sharing/Permission/NodeDownloadSharePermissionType.php +++ b/apps/files/lib/Sharing/Permission/NodeDownloadSharePermissionType.php @@ -11,15 +11,13 @@ use OCA\Files\AppInfo\Application; use OCP\L10N\IFactory; -use OCP\Server; use OCP\Share\IManager; use OCP\Sharing\Permission\ISharePermissionType; -final class NodeDownloadSharePermissionType implements ISharePermissionType { - private ?IManager $legacyManager = null; - - private function getLegacyManager(): IManager { - return $this->legacyManager ??= Server::get(IManager::class); +final readonly class NodeDownloadSharePermissionType implements ISharePermissionType { + public function __construct( + private IManager $legacyManager, + ) { } #[\Override] @@ -30,7 +28,7 @@ public function getDisplayName(IFactory $l10nFactory): string { #[\Override] public function getHint(IFactory $l10nFactory): ?string { // If previews are still allowed, the download option is only hidden, because on a technical level it is still possible to download. - if ($this->getLegacyManager()->allowViewWithoutDownload()) { + if ($this->legacyManager->allowViewWithoutDownload()) { return $l10nFactory->get(Application::APP_ID)->t('When disabled, the option to download will be hidden'); } diff --git a/apps/files/lib/Sharing/Permission/NodeUpdateSharePermissionType.php b/apps/files/lib/Sharing/Permission/NodeUpdateSharePermissionType.php index 87d046eb2af40..9645705916764 100644 --- a/apps/files/lib/Sharing/Permission/NodeUpdateSharePermissionType.php +++ b/apps/files/lib/Sharing/Permission/NodeUpdateSharePermissionType.php @@ -13,14 +13,12 @@ use OCP\Constants; use OCP\IAppConfig; use OCP\L10N\IFactory; -use OCP\Server; use OCP\Sharing\Permission\ISharePermissionType; -final class NodeUpdateSharePermissionType implements ISharePermissionType { - private ?IAppConfig $appConfig = null; - - private function getAppConfig(): IAppConfig { - return $this->appConfig ??= Server::get(IAppConfig::class); +final readonly class NodeUpdateSharePermissionType implements ISharePermissionType { + public function __construct( + private IAppConfig $appConfig, + ) { } #[\Override] @@ -40,6 +38,6 @@ public function getPriority(): int { #[\Override] public function isEnabledByDefault(): bool { - return ($this->getAppConfig()->getValueInt(\OC\Core\AppInfo\Application::APP_ID, 'shareapi_default_permissions') & Constants::PERMISSION_UPDATE) === Constants::PERMISSION_UPDATE; + return ($this->appConfig->getValueInt(\OC\Core\AppInfo\Application::APP_ID, 'shareapi_default_permissions') & Constants::PERMISSION_UPDATE) === Constants::PERMISSION_UPDATE; } } diff --git a/apps/files/lib/Sharing/Source/NodeShareSourceType.php b/apps/files/lib/Sharing/Source/NodeShareSourceType.php index df99e5aa48743..af972af00bf20 100644 --- a/apps/files/lib/Sharing/Source/NodeShareSourceType.php +++ b/apps/files/lib/Sharing/Source/NodeShareSourceType.php @@ -23,7 +23,6 @@ use OCP\Interaction\Resources\NodeResource; use OCP\IURLGenerator; use OCP\L10N\IFactory; -use OCP\Server; use OCP\Sharing\Icon\ShareIconURL; use OCP\Sharing\ISharingManager; use OCP\Sharing\ShareAccessContext; @@ -33,28 +32,18 @@ /** * @template-implements IEventListener */ -final class NodeShareSourceType implements IShareSourceType, IEventListener { - private ?IRootFolder $rootFolder = null; - - private ?IURLGenerator $urlGenerator = null; - +final readonly class NodeShareSourceType implements IShareSourceType, IEventListener { public function __construct( IEventDispatcher $eventDispatcher, - private readonly IDBConnection $dbConnection, - private readonly ISharingManager $manager, + private IDBConnection $dbConnection, + private IRootFolder $rootFolder, + private IURLGenerator $urlGenerator, + private ISharingManager $manager, ) { $eventDispatcher->addServiceListener(NodeDeletedEvent::class, self::class); $eventDispatcher->addServiceListener(MoveToTrashEvent::class, self::class); } - private function getRootFolder(): IRootFolder { - return $this->rootFolder ??= Server::get(IRootFolder::class); - } - - private function getUrlGenerator(): IURLGenerator { - return $this->urlGenerator ??= Server::get(IURLGenerator::class); - } - #[\Override] public function getDisplayName(IFactory $l10nFactory): string { return $l10nFactory->get(Application::APP_ID)->t('File'); @@ -62,12 +51,12 @@ public function getDisplayName(IFactory $l10nFactory): string { #[\Override] public function validateSource(string $source): bool { - return $this->getRootFolder()->getFirstNodeById((int)$source) instanceof Node; + return $this->rootFolder->getFirstNodeById((int)$source) instanceof Node; } #[\Override] public function getSourceDisplayName(string $source): ?string { - $displayName = $this->getRootFolder()->getFirstNodeById((int)$source)?->getName(); + $displayName = $this->rootFolder->getFirstNodeById((int)$source)?->getName(); if ($displayName === '') { return null; } @@ -77,7 +66,7 @@ public function getSourceDisplayName(string $source): ?string { #[\Override] public function getSourceIcon(string $source): ShareIconURL { - $url = $this->getUrlGenerator()->linkToRouteAbsolute('core.Preview.getPreviewByFileId', ['fileId' => $source, 'x' => 64, 'y' => 64]); + $url = $this->urlGenerator->linkToRouteAbsolute('core.Preview.getPreviewByFileId', ['fileId' => $source, 'x' => 64, 'y' => 64]); return new ShareIconURL($url, $url); } diff --git a/apps/files/tests/Sharing/Source/NodeShareSourceTypeTest.php b/apps/files/tests/Sharing/Source/NodeShareSourceTypeTest.php index 66066b80b2165..7eeeaa4363366 100644 --- a/apps/files/tests/Sharing/Source/NodeShareSourceTypeTest.php +++ b/apps/files/tests/Sharing/Source/NodeShareSourceTypeTest.php @@ -15,6 +15,7 @@ use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\IDBConnection; +use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\Server; @@ -56,7 +57,7 @@ public function setUp(): void { $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, $this->manager); + $this->sourceType = new NodeShareSourceType(Server::get(IEventDispatcher::class), $this->dbConnection, Server::get(IRootFolder::class), Server::get(IURLGenerator::class), $this->manager); } #[\Override]