From 1642a34e8a489090820a417af6c394491b426739 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 1 Oct 2024 16:14:13 +0200 Subject: [PATCH] refactor(files_sharing): Add Storage parameter strong types Signed-off-by: provokateurin --- apps/files_sharing/lib/External/Storage.php | 19 +++---- apps/files_sharing/lib/SharedStorage.php | 50 ++++++++----------- .../tests/ExternalStorageTest.php | 2 +- 3 files changed, 33 insertions(+), 38 deletions(-) diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 718afbf7499..39e907cffff 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -24,6 +24,7 @@ use OCP\Files\Cache\IWatcher; use OCP\Files\NotFoundException; use OCP\Files\Storage\IDisableEncryptionStorage; use OCP\Files\Storage\IReliableEtagStorage; +use OCP\Files\Storage\IStorage; use OCP\Files\StorageInvalidException; use OCP\Files\StorageNotAvailableException; use OCP\Http\Client\IClientService; @@ -94,7 +95,7 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, ); } - public function getWatcher($path = '', $storage = null): IWatcher { + public function getWatcher(string $path = '', ?IStorage $storage = null): IWatcher { if (!$storage) { $storage = $this; } @@ -129,14 +130,14 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, return 'shared::' . md5($this->token . '@' . $this->getRemote()); } - public function getCache($path = '', $storage = null): ICache { + public function getCache(string $path = '', ?IStorage $storage = null): ICache { if (is_null($this->cache)) { $this->cache = new Cache($this, $this->cloudId); } return $this->cache; } - public function getScanner($path = '', $storage = null): IScanner { + public function getScanner(string $path = '', ?IStorage $storage = null): IScanner { if (!$storage) { $storage = $this; } @@ -147,7 +148,7 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, return $this->scanner; } - public function hasUpdated($path, $time): bool { + public function hasUpdated(string $path, int $time): bool { // since for owncloud webdav servers we can rely on etag propagation we only need to check the root of the storage // because of that we only do one check for the entire storage per request if ($this->updateChecked) { @@ -217,7 +218,7 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, } } - public function file_exists($path): bool { + public function file_exists(string $path): bool { if ($path === '') { return true; } else { @@ -322,18 +323,18 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, return json_decode($response->getBody(), true); } - public function getOwner($path): string|false { + public function getOwner(string $path): string|false { return $this->cloudId->getDisplayId(); } - public function isSharable($path): bool { + public function isSharable(string $path): bool { if (\OCP\Util::isSharingDisabledForUser() || !\OC\Share\Share::isResharingAllowed()) { return false; } return (bool)($this->getPermissions($path) & Constants::PERMISSION_SHARE); } - public function getPermissions($path): int { + public function getPermissions(string $path): int { $response = $this->propfind($path); if ($response === false) { return 0; @@ -408,7 +409,7 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, return $permissions; } - public function free_space($path): int|float|false { + public function free_space(string $path): int|float|false { return parent::free_space(''); } diff --git a/apps/files_sharing/lib/SharedStorage.php b/apps/files_sharing/lib/SharedStorage.php index 7addc9f0a91..63c5fa9017d 100644 --- a/apps/files_sharing/lib/SharedStorage.php +++ b/apps/files_sharing/lib/SharedStorage.php @@ -202,7 +202,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha self::$initDepth--; } - public function instanceOfStorage($class): bool { + public function instanceOfStorage(string $class): bool { if ($class === '\OC\Files\Storage\Common' || $class == Common::class) { return true; } @@ -234,7 +234,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha return 'shared::' . $this->getMountPoint(); } - public function getPermissions($path = ''): int { + public function getPermissions(string $path = ''): int { if (!$this->isValid()) { return 0; } @@ -252,11 +252,11 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha return $permissions; } - public function isCreatable($path): bool { + public function isCreatable(string $path): bool { return (bool)($this->getPermissions($path) & \OCP\Constants::PERMISSION_CREATE); } - public function isReadable($path): bool { + public function isReadable(string $path): bool { if (!$this->isValid()) { return false; } @@ -269,22 +269,22 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha return $storage->isReadable($internalPath); } - public function isUpdatable($path): bool { + public function isUpdatable(string $path): bool { return (bool)($this->getPermissions($path) & \OCP\Constants::PERMISSION_UPDATE); } - public function isDeletable($path): bool { + public function isDeletable(string $path): bool { return (bool)($this->getPermissions($path) & \OCP\Constants::PERMISSION_DELETE); } - public function isSharable($path): bool { + public function isSharable(string $path): bool { if (\OCP\Util::isSharingDisabledForUser() || !\OC\Share\Share::isResharingAllowed()) { return false; } return (bool)($this->getPermissions($path) & \OCP\Constants::PERMISSION_SHARE); } - public function fopen($path, $mode) { + public function fopen(string $path, string $mode) { $source = $this->getUnjailedPath($path); switch ($mode) { case 'r+': @@ -336,7 +336,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha return $this->nonMaskedStorage->fopen($this->getUnjailedPath($path), $mode); } - public function rename($source, $target): bool { + public function rename(string $source, string $target): bool { $this->init(); $isPartFile = pathinfo($source, PATHINFO_EXTENSION) === 'part'; $targetExists = $this->file_exists($target); @@ -364,10 +364,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha return $this->superShare->getTarget(); } - /** - * @param string $path - */ - public function setMountPoint($path): void { + public function setMountPoint(string $path): void { $this->superShare->setTarget($path); foreach ($this->groupedShares as $share) { @@ -397,7 +394,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha return $this->superShare->getNodeType(); } - public function getCache($path = '', $storage = null): ICache { + public function getCache(string $path = '', ?IStorage $storage = null): ICache { if ($this->cache) { return $this->cache; } @@ -418,18 +415,18 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha return $this->cache; } - public function getScanner($path = '', $storage = null): IScanner { + public function getScanner(string $path = '', ?IStorage $storage = null): IScanner { if (!$storage) { $storage = $this; } return new \OCA\Files_Sharing\Scanner($storage); } - public function getOwner($path): string|false { + public function getOwner(string $path): string|false { return $this->superShare->getShareOwner(); } - public function getWatcher($path = '', $storage = null): IWatcher { + public function getWatcher(string $path = '', ?IStorage $storage = null): IWatcher { if ($this->watcher) { return $this->watcher; } @@ -467,7 +464,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha return true; } - public function acquireLock($path, $type, ILockingProvider $provider): void { + public function acquireLock(string $path, int $type, ILockingProvider $provider): void { /** @var ILockingStorage $targetStorage */ [$targetStorage, $targetInternalPath] = $this->resolvePath($path); $targetStorage->acquireLock($targetInternalPath, $type, $provider); @@ -478,7 +475,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha } } - public function releaseLock($path, $type, ILockingProvider $provider): void { + public function releaseLock(string $path, int $type, ILockingProvider $provider): void { /** @var ILockingStorage $targetStorage */ [$targetStorage, $targetInternalPath] = $this->resolvePath($path); $targetStorage->releaseLock($targetInternalPath, $type, $provider); @@ -489,7 +486,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha } } - public function changeLock($path, $type, ILockingProvider $provider): void { + public function changeLock(string $path, int $type, ILockingProvider $provider): void { /** @var ILockingStorage $targetStorage */ [$targetStorage, $targetInternalPath] = $this->resolvePath($path); $targetStorage->changeLock($targetInternalPath, $type, $provider); @@ -503,7 +500,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha ]; } - public function setAvailability($isAvailable): void { + public function setAvailability(bool $isAvailable): void { // shares do not participate in availability logic } @@ -527,7 +524,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha return $this->storage; } - public function file_get_contents($path): string|false { + public function file_get_contents(string $path): string|false { $info = [ 'target' => $this->getMountPoint() . '/' . $path, 'source' => $this->getUnjailedPath($path), @@ -536,7 +533,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha return parent::file_get_contents($path); } - public function file_put_contents($path, $data): int|float|false { + public function file_put_contents(string $path, mixed $data): int|float|false { $info = [ 'target' => $this->getMountPoint() . '/' . $path, 'source' => $this->getUnjailedPath($path), @@ -545,15 +542,12 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements LegacyISha return parent::file_put_contents($path, $data); } - /** - * @return void - */ - public function setMountOptions(array $options) { + public function setMountOptions(array $options): void { /* Note: This value is never read */ $this->mountOptions = $options; } - public function getUnjailedPath($path): string { + public function getUnjailedPath(string $path): string { $this->init(); return parent::getUnjailedPath($path); } diff --git a/apps/files_sharing/tests/ExternalStorageTest.php b/apps/files_sharing/tests/ExternalStorageTest.php index 90aad4dca6e..7abb7265153 100644 --- a/apps/files_sharing/tests/ExternalStorageTest.php +++ b/apps/files_sharing/tests/ExternalStorageTest.php @@ -109,7 +109,7 @@ class TestSharingExternalStorage extends \OCA\Files_Sharing\External\Storage { return $this->createBaseUri(); } - public function stat($path): array|false { + public function stat(string $path): array|false { if ($path === '') { return ['key' => 'value']; } -- 2.39.5