Skip to content

Use constructor properties in Sharing #62335

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

Merged
provokateurin merged 2 commits into from
Jul 21, 2026
Merged

Use constructor properties in Sharing #62335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e3af67
refactor(core): Use constructor properties in Sharing
provokateurin Jul 20, 2026
513ecea
refactor(files): Use constructor properties in Sharing
provokateurin Jul 20, 2026
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
8 changes: 4 additions & 4 deletions apps/files/lib/AppInfo/Application.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
12 changes: 5 additions & 7 deletions apps/files/lib/Sharing/Permission/NodeCreateSharePermissionType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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;
}
}
12 changes: 5 additions & 7 deletions apps/files/lib/Sharing/Permission/NodeDeleteSharePermissionType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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;
}
}
12 changes: 5 additions & 7 deletions apps/files/lib/Sharing/Permission/NodeDownloadSharePermissionType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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');
}

Expand Down
12 changes: 5 additions & 7 deletions apps/files/lib/Sharing/Permission/NodeUpdateSharePermissionType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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;
}
}
27 changes: 8 additions & 19 deletions apps/files/lib/Sharing/Source/NodeShareSourceType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,41 +32,31 @@
/**
* @template-implements IEventListener<NodeDeletedEvent|MoveToTrashEvent>
*/
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');
}

#[\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;
}
Expand All @@ -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);
}
Expand Down
3 changes: 2 additions & 1 deletion apps/files/tests/Sharing/Source/NodeShareSourceTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]
Expand Down
10 changes: 5 additions & 5 deletions core/AppInfo/Application.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,13 @@ public function register(IRegistrationContext $context): void {

$registry = Server::get(ISharingRegistry::class);

@CarlSchwan CarlSchwan Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated nitpick: but I wonder if it would make sense to have IRegistrationContext::getSharingRegistry as an API


$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());
Comment thread
provokateurin marked this conversation as resolved.
$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);
Expand All @@ -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);
Expand Down
12 changes: 5 additions & 7 deletions core/Sharing/Permission/ReshareSharePermissionType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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;
}
}
29 changes: 12 additions & 17 deletions core/Sharing/Property/ExpirationDateSharePropertyType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}

Expand All @@ -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]
Expand All @@ -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) {
Expand Down
18 changes: 6 additions & 12 deletions core/Sharing/Property/PasswordSharePropertyType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,18 @@
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;
use OCP\Sharing\Share;
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]
Expand All @@ -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]
Expand All @@ -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;
Expand Down
Loading
Loading