From 8b7c8447a0e1eebc512cb58ea734542e03f29374 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 10 Feb 2022 15:37:57 +0100 Subject: [PATCH 1/8] move root mount setup to mountproviders Signed-off-by: Robin Appelman --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Files/Mount/RootMountProvider.php | 103 +++++++++++++ lib/private/Server.php | 2 + lib/private/legacy/OC_Util.php | 104 ------------- .../lib/Files/Mount/RootMountProviderTest.php | 141 ++++++++++++++++++ 6 files changed, 248 insertions(+), 104 deletions(-) create mode 100644 lib/private/Files/Mount/RootMountProvider.php create mode 100644 tests/lib/Files/Mount/RootMountProviderTest.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 00fefc81bc5fc..6fd3d9207db39 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1129,6 +1129,7 @@ 'OC\\Files\\Mount\\MoveableMount' => $baseDir . '/lib/private/Files/Mount/MoveableMount.php', 'OC\\Files\\Mount\\ObjectHomeMountProvider' => $baseDir . '/lib/private/Files/Mount/ObjectHomeMountProvider.php', 'OC\\Files\\Mount\\ObjectStorePreviewCacheMountProvider' => $baseDir . '/lib/private/Files/Mount/ObjectStorePreviewCacheMountProvider.php', + 'OC\\Files\\Mount\\RootMountProvider' => $baseDir . '/lib/private/Files/Mount/RootMountProvider.php', 'OC\\Files\\Node\\File' => $baseDir . '/lib/private/Files/Node/File.php', 'OC\\Files\\Node\\Folder' => $baseDir . '/lib/private/Files/Node/Folder.php', 'OC\\Files\\Node\\HookConnector' => $baseDir . '/lib/private/Files/Node/HookConnector.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 1526714be3a81..c8c0e7df381eb 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1158,6 +1158,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Files\\Mount\\MoveableMount' => __DIR__ . '/../../..' . '/lib/private/Files/Mount/MoveableMount.php', 'OC\\Files\\Mount\\ObjectHomeMountProvider' => __DIR__ . '/../../..' . '/lib/private/Files/Mount/ObjectHomeMountProvider.php', 'OC\\Files\\Mount\\ObjectStorePreviewCacheMountProvider' => __DIR__ . '/../../..' . '/lib/private/Files/Mount/ObjectStorePreviewCacheMountProvider.php', + 'OC\\Files\\Mount\\RootMountProvider' => __DIR__ . '/../../..' . '/lib/private/Files/Mount/RootMountProvider.php', 'OC\\Files\\Node\\File' => __DIR__ . '/../../..' . '/lib/private/Files/Node/File.php', 'OC\\Files\\Node\\Folder' => __DIR__ . '/../../..' . '/lib/private/Files/Node/Folder.php', 'OC\\Files\\Node\\HookConnector' => __DIR__ . '/../../..' . '/lib/private/Files/Node/HookConnector.php', diff --git a/lib/private/Files/Mount/RootMountProvider.php b/lib/private/Files/Mount/RootMountProvider.php new file mode 100644 index 0000000000000..b301fc6dd149d --- /dev/null +++ b/lib/private/Files/Mount/RootMountProvider.php @@ -0,0 +1,103 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Files\Mount; + +use OC; +use OC\Files\ObjectStore\ObjectStoreStorage; +use OC\Files\Storage\LocalRootStorage; +use OC_App; +use OCP\Files\Config\IRootMountProvider; +use OCP\Files\Storage\IStorageFactory; +use OCP\IConfig; +use Psr\Log\LoggerInterface; + +class RootMountProvider implements IRootMountProvider { + private IConfig $config; + private LoggerInterface $logger; + + public function __construct(IConfig $config, LoggerInterface $logger) { + $this->config = $config; + $this->logger = $logger; + } + + public function getRootMounts(IStorageFactory $loader): array { + $objectStore = $this->config->getSystemValue('objectstore', null); + $objectStoreMultiBucket = $this->config->getSystemValue('objectstore_multibucket', null); + + if ($objectStoreMultiBucket) { + return [$this->getMultiBucketStoreRootMount($loader, $objectStoreMultiBucket)]; + } elseif ($objectStore) { + return [$this->getObjectStoreRootMount($loader, $objectStore)]; + } else { + return [$this->getLocalRootMount($loader)]; + } + } + + private function validateObjectStoreConfig(array &$config) { + if (empty($config['class'])) { + $this->logger->error('No class given for objectstore', ['app' => 'files']); + } + if (!isset($config['arguments'])) { + $config['arguments'] = []; + } + + // instantiate object store implementation + $name = $config['class']; + if (strpos($name, 'OCA\\') === 0 && substr_count($name, '\\') >= 2) { + $segments = explode('\\', $name); + OC_App::loadApp(strtolower($segments[1])); + } + } + + private function getLocalRootMount(IStorageFactory $loader): MountPoint { + $configDataDirectory = $this->config->getSystemValue("datadirectory", OC::$SERVERROOT . "/data"); + return new MountPoint(LocalRootStorage::class, '/', ['datadir' => $configDataDirectory], $loader, null, null, self::class); + } + + private function getObjectStoreRootMount(IStorageFactory $loader, array $config): MountPoint { + $this->validateObjectStoreConfig($config); + + $config['arguments']['objectstore'] = new $config['class']($config['arguments']); + // mount with plain / root object store implementation + $config['class'] = ObjectStoreStorage::class; + + return new MountPoint($config['class'], '/', $config['arguments'], $loader, null, null, self::class); + } + + private function getMultiBucketStoreRootMount(IStorageFactory $loader, array $config): MountPoint { + $this->validateObjectStoreConfig($config); + + if (!isset($config['arguments']['bucket'])) { + $config['arguments']['bucket'] = ''; + } + // put the root FS always in first bucket for multibucket configuration + $config['arguments']['bucket'] .= '0'; + + $config['arguments']['objectstore'] = new $config['class']($config['arguments']); + // mount with plain / root object store implementation + $config['class'] = ObjectStoreStorage::class; + + return new MountPoint($config['class'], '/', $config['arguments'], $loader, null, null, self::class); + } +} diff --git a/lib/private/Server.php b/lib/private/Server.php index 13bbf972abb0a..5098e90ab84b4 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -91,6 +91,7 @@ use OC\Files\Mount\LocalHomeMountProvider; use OC\Files\Mount\ObjectHomeMountProvider; use OC\Files\Mount\ObjectStorePreviewCacheMountProvider; +use OC\Files\Mount\RootMountProvider; use OC\Files\Node\HookConnector; use OC\Files\Node\LazyRoot; use OC\Files\Node\Root; @@ -952,6 +953,7 @@ public function __construct($webRoot, \OC\Config $config) { $manager->registerProvider(new CacheMountProvider($config)); $manager->registerHomeProvider(new LocalHomeMountProvider()); $manager->registerHomeProvider(new ObjectHomeMountProvider($config)); + $manager->registerRootProvider(new RootMountProvider($config, $c->get(LoggerInterface::class))); $manager->registerRootProvider(new ObjectStorePreviewCacheMountProvider($logger, $config)); return $manager; diff --git a/lib/private/legacy/OC_Util.php b/lib/private/legacy/OC_Util.php index 9110678537f07..40dcbb13b9395 100644 --- a/lib/private/legacy/OC_Util.php +++ b/lib/private/legacy/OC_Util.php @@ -66,11 +66,9 @@ use bantu\IniGetWrapper\IniGetWrapper; use OC\AppFramework\Http\Request; -use OC\Files\Storage\LocalRootStorage; use OCP\Files\Template\ITemplateManager; use OCP\IConfig; use OCP\IGroupManager; -use OCP\ILogger; use OCP\IURLGenerator; use OCP\IUser; use OCP\Share\IManager; @@ -80,7 +78,6 @@ class OC_Util { public static $scripts = []; public static $styles = []; public static $headers = []; - private static $rootMounted = false; private static $rootFsSetup = false; private static $fsSetup = false; @@ -91,93 +88,6 @@ protected static function getAppManager() { return \OC::$server->getAppManager(); } - private static function initLocalStorageRootFS() { - // mount local file backend as root - $configDataDirectory = \OC::$server->getSystemConfig()->getValue("datadirectory", OC::$SERVERROOT . "/data"); - //first set up the local "root" storage - \OC\Files\Filesystem::initMountManager(); - if (!self::$rootMounted) { - \OC\Files\Filesystem::mount(LocalRootStorage::class, ['datadir' => $configDataDirectory], '/'); - self::$rootMounted = true; - } - } - - /** - * mounting an object storage as the root fs will in essence remove the - * necessity of a data folder being present. - * TODO make home storage aware of this and use the object storage instead of local disk access - * - * @param array $config containing 'class' and optional 'arguments' - * @suppress PhanDeprecatedFunction - */ - private static function initObjectStoreRootFS($config) { - // check misconfiguration - if (empty($config['class'])) { - \OCP\Util::writeLog('files', 'No class given for objectstore', ILogger::ERROR); - } - if (!isset($config['arguments'])) { - $config['arguments'] = []; - } - - // instantiate object store implementation - $name = $config['class']; - if (strpos($name, 'OCA\\') === 0 && substr_count($name, '\\') >= 2) { - $segments = explode('\\', $name); - OC_App::loadApp(strtolower($segments[1])); - } - $config['arguments']['objectstore'] = new $config['class']($config['arguments']); - // mount with plain / root object store implementation - $config['class'] = '\OC\Files\ObjectStore\ObjectStoreStorage'; - - // mount object storage as root - \OC\Files\Filesystem::initMountManager(); - if (!self::$rootMounted) { - \OC\Files\Filesystem::mount($config['class'], $config['arguments'], '/'); - self::$rootMounted = true; - } - } - - /** - * mounting an object storage as the root fs will in essence remove the - * necessity of a data folder being present. - * - * @param array $config containing 'class' and optional 'arguments' - * @suppress PhanDeprecatedFunction - */ - private static function initObjectStoreMultibucketRootFS($config) { - // check misconfiguration - if (empty($config['class'])) { - \OCP\Util::writeLog('files', 'No class given for objectstore', ILogger::ERROR); - } - if (!isset($config['arguments'])) { - $config['arguments'] = []; - } - - // instantiate object store implementation - $name = $config['class']; - if (strpos($name, 'OCA\\') === 0 && substr_count($name, '\\') >= 2) { - $segments = explode('\\', $name); - OC_App::loadApp(strtolower($segments[1])); - } - - if (!isset($config['arguments']['bucket'])) { - $config['arguments']['bucket'] = ''; - } - // put the root FS always in first bucket for multibucket configuration - $config['arguments']['bucket'] .= '0'; - - $config['arguments']['objectstore'] = new $config['class']($config['arguments']); - // mount with plain / root object store implementation - $config['class'] = '\OC\Files\ObjectStore\ObjectStoreStorage'; - - // mount object storage as root - \OC\Files\Filesystem::initMountManager(); - if (!self::$rootMounted) { - \OC\Files\Filesystem::mount($config['class'], $config['arguments'], '/'); - self::$rootMounted = true; - } - } - /** * Can be set up * @@ -279,19 +189,6 @@ public static function setupRootFS(string $user = '') { \OC\Files\Filesystem::logWarningWhenAddingStorageWrapper($prevLogging); - //check if we are using an object storage - $objectStore = \OC::$server->getSystemConfig()->getValue('objectstore', null); - $objectStoreMultibucket = \OC::$server->getSystemConfig()->getValue('objectstore_multibucket', null); - - // use the same order as in ObjectHomeMountProvider - if (isset($objectStoreMultibucket)) { - self::initObjectStoreMultibucketRootFS($objectStoreMultibucket); - } elseif (isset($objectStore)) { - self::initObjectStoreRootFS($objectStore); - } else { - self::initLocalStorageRootFS(); - } - /** @var \OCP\Files\Config\IMountProviderCollection $mountProviderCollection */ $mountProviderCollection = \OC::$server->query(\OCP\Files\Config\IMountProviderCollection::class); $rootMountProviders = $mountProviderCollection->getRootMounts(); @@ -501,7 +398,6 @@ public static function tearDownFS() { \OC::$server->getRootFolder()->clearCache(); self::$fsSetup = false; self::$rootFsSetup = false; - self::$rootMounted = false; } /** diff --git a/tests/lib/Files/Mount/RootMountProviderTest.php b/tests/lib/Files/Mount/RootMountProviderTest.php new file mode 100644 index 0000000000000..e5eaabf93bee4 --- /dev/null +++ b/tests/lib/Files/Mount/RootMountProviderTest.php @@ -0,0 +1,141 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace Test\Files\Mount; + +use OC\Files\Mount\RootMountProvider; +use OC\Files\ObjectStore\ObjectStoreStorage; +use OC\Files\ObjectStore\S3; +use OC\Files\Storage\LocalRootStorage; +use OC\Files\Storage\StorageFactory; +use OCP\IConfig; +use Psr\Log\LoggerInterface; +use Test\TestCase; + +/** + * @group DB + */ +class RootMountProviderTest extends TestCase { + private StorageFactory $loader; + + protected function setUp(): void { + parent::setUp(); + + $this->loader = new StorageFactory(); + } + + private function getConfig(array $systemConfig): IConfig { + $config = $this->createMock(IConfig::class); + $config->method('getSystemValue') + ->willReturnCallback(function (string $key, $default) use ($systemConfig) { + return $systemConfig[$key] ?? $default; + }); + return $config; + } + + private function getProvider(array $systemConfig): RootMountProvider { + $config = $this->getConfig($systemConfig); + $provider = new RootMountProvider($config, $this->createMock(LoggerInterface::class)); + return $provider; + } + + public function testLocal() { + $provider = $this->getProvider([ + 'datadirectory' => '/data', + ]); + $mounts = $provider->getRootMounts($this->loader); + $this->assertCount(1, $mounts); + $mount = $mounts[0]; + $this->assertEquals('/', $mount->getMountPoint()); + /** @var LocalRootStorage $storage */ + $storage = $mount->getStorage(); + $this->assertInstanceOf(LocalRootStorage::class, $storage); + $this->assertEquals('/data/', $storage->getSourcePath('')); + } + + public function testObjectStore() { + $provider = $this->getProvider([ + 'objectstore' => [ + "class" => "OC\Files\ObjectStore\S3", + "arguments" => [ + "bucket" => "nextcloud", + "autocreate" => true, + "key" => "minio", + "secret" => "minio123", + "hostname" => "localhost", + "port" => 9000, + "use_ssl" => false, + "use_path_style" => true, + "uploadPartSize" => 52428800, + ], + ], + ]); + $mounts = $provider->getRootMounts($this->loader); + $this->assertCount(1, $mounts); + $mount = $mounts[0]; + $this->assertEquals('/', $mount->getMountPoint()); + /** @var ObjectStoreStorage $storage */ + $storage = $mount->getStorage(); + $this->assertInstanceOf(ObjectStoreStorage::class, $storage); + + $class = new \ReflectionClass($storage); + $prop = $class->getProperty('objectStore'); + $prop->setAccessible(true); + /** @var S3 $objectStore */ + $objectStore = $prop->getValue($storage); + $this->assertEquals('nextcloud', $objectStore->getBucket()); + } + + public function testObjectStoreMultiBucket() { + $provider = $this->getProvider([ + 'objectstore_multibucket' => [ + "class" => "OC\Files\ObjectStore\S3", + "arguments" => [ + "bucket" => "nextcloud", + "autocreate" => true, + "key" => "minio", + "secret" => "minio123", + "hostname" => "localhost", + "port" => 9000, + "use_ssl" => false, + "use_path_style" => true, + "uploadPartSize" => 52428800, + ], + ], + ]); + $mounts = $provider->getRootMounts($this->loader); + $this->assertCount(1, $mounts); + $mount = $mounts[0]; + $this->assertEquals('/', $mount->getMountPoint()); + /** @var ObjectStoreStorage $storage */ + $storage = $mount->getStorage(); + $this->assertInstanceOf(ObjectStoreStorage::class, $storage); + + $class = new \ReflectionClass($storage); + $prop = $class->getProperty('objectStore'); + $prop->setAccessible(true); + /** @var S3 $objectStore */ + $objectStore = $prop->getValue($storage); + $this->assertEquals('nextcloud0', $objectStore->getBucket()); + } +} From 215fa17d04da01173fa54b5580b919f3a9bb9a1c Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 25 Feb 2022 15:14:32 +0100 Subject: [PATCH 2/8] catch storage not available when deleting avatar Signed-off-by: Robin Appelman --- lib/private/Avatar/AvatarManager.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/private/Avatar/AvatarManager.php b/lib/private/Avatar/AvatarManager.php index c3afd8094c741..77138085dc915 100644 --- a/lib/private/Avatar/AvatarManager.php +++ b/lib/private/Avatar/AvatarManager.php @@ -43,6 +43,7 @@ use OCP\Files\IAppData; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; +use OCP\Files\StorageNotAvailableException; use OCP\IAvatar; use OCP\IAvatarManager; use OCP\IConfig; @@ -173,7 +174,7 @@ public function deleteUserAvatar(string $userId): void { $folder->delete(); } catch (NotFoundException $e) { $this->logger->debug("No cache for the user $userId. Ignoring avatar deletion"); - } catch (NotPermittedException $e) { + } catch (NotPermittedException | StorageNotAvailableException $e) { $this->logger->error("Unable to delete user avatars for $userId. gnoring avatar deletion"); } catch (NoUserException $e) { $this->logger->debug("User $userId not found. gnoring avatar deletion"); From 72f9fd951d8f969c5553061dbe919fcef1cd7ecb Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 25 Feb 2022 15:15:21 +0100 Subject: [PATCH 3/8] run ci with primary s3 setup Signed-off-by: Robin Appelman --- .github/workflows/s3-primary.yml | 70 ++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 .github/workflows/s3-primary.yml diff --git a/.github/workflows/s3-primary.yml b/.github/workflows/s3-primary.yml new file mode 100644 index 0000000000000..3ea21b768ec9f --- /dev/null +++ b/.github/workflows/s3-primary.yml @@ -0,0 +1,70 @@ +name: S3 primary storage +on: + pull_request: + push: + branches: + - master + - stable* + + +jobs: + s3-primary-tests-minio: + runs-on: ubuntu-latest + + strategy: + # do not stop on another job's failure + fail-fast: false + matrix: + php-versions: ['8.0'] + key: ['objectstore', 'objectstore_multibucket'] + + name: php${{ matrix.php-versions }}-${{ matrix.key }}-minio + + services: + minio: + env: + MINIO_ACCESS_KEY: minio + MINIO_SECRET_KEY: minio123 + image: bitnami/minio:2021.10.6 + ports: + - "9000:9000" + + steps: + - name: Checkout server + uses: actions/checkout@v2 + with: + submodules: true + + - name: Set up php ${{ matrix.php-versions }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php-versions }} + tools: phpunit + extensions: mbstring, fileinfo, intl, sqlite, pdo_sqlite, zip, gd + + - name: Set up Nextcloud + run: | + mkdir data + echo ' ["class" => "OC\Files\ObjectStore\S3", "arguments" => ["bucket" => "nextcloud", "autocreate" => true, "key" => "minio", "secret" => "minio123", "hostname" => "localhost", "port" => 9000, "use_ssl" => false, "use_path_style" => true, "uploadPartSize" => 52428800]]];' > config/config.php + ./occ maintenance:install --verbose --database=sqlite --database-name=nextcloud --database-host=127.0.0.1 --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password + php -f index.php + + - name: PHPUnit + working-directory: tests + run: phpunit --configuration phpunit-autotest.xml --group DB,SLOWDB + - name: S3 logs + if: always() + run: | + docker ps -a + docker logs $(docker ps -aq) + + + s3-primary-summary: + runs-on: ubuntu-latest + needs: [s3-primary-tests-minio] + + if: always() + + steps: + - name: Summary status + run: if ${{ needs.s3-primary-tests-minio.result != 'success' }}; then exit 1; fi From b0ce876f41f6593373505a67a2fe5737c21fa6b6 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 25 Feb 2022 15:25:13 +0100 Subject: [PATCH 4/8] return dummy availability if storage is not found in cache Signed-off-by: Robin Appelman --- lib/private/Files/Cache/Storage.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/Cache/Storage.php b/lib/private/Files/Cache/Storage.php index 33785607ef767..2de2c2f84d711 100644 --- a/lib/private/Files/Cache/Storage.php +++ b/lib/private/Files/Cache/Storage.php @@ -158,7 +158,7 @@ public static function getNumericStorageId($storageId) { } /** - * @return array|null [ available, last_checked ] + * @return array [ available, last_checked ] */ public function getAvailability() { if ($row = self::getStorageById($this->storageId)) { @@ -167,7 +167,10 @@ public function getAvailability() { 'last_checked' => $row['last_checked'] ]; } else { - return null; + return [ + 'available' => true, + 'last_checked' => time(), + ]; } } From 0235764f7039a80b94363c7a8f4c7ff89e347389 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 25 Feb 2022 15:39:20 +0100 Subject: [PATCH 5/8] skip localstorage dependend trashbin test when not using local user storage Signed-off-by: Robin Appelman --- apps/files_trashbin/tests/StorageTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/files_trashbin/tests/StorageTest.php b/apps/files_trashbin/tests/StorageTest.php index 448c3a6adb1d6..0a7a129ca28d2 100644 --- a/apps/files_trashbin/tests/StorageTest.php +++ b/apps/files_trashbin/tests/StorageTest.php @@ -33,6 +33,7 @@ use OC\Files\Filesystem; use OC\Files\Storage\Common; +use OC\Files\Storage\Local; use OC\Files\Storage\Temporary; use OCA\Files_Trashbin\AppInfo\Application; use OCA\Files_Trashbin\Events\MoveToTrashEvent; @@ -661,6 +662,9 @@ public function testTrashbinCollision() { } public function testMoveFromStoragePreserveFileId() { + if (!$this->userView->getMount('')->getStorage()->instanceOfStorage(Local::class)) { + $this->markTestSkipped("Skipping on non-local users storage"); + } $this->userView->file_put_contents('test.txt', 'foo'); $fileId = $this->userView->getFileInfo('test.txt')->getId(); From ad10a3331693c90c47d04c7ac6883aa5ca1ed5c6 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 25 Feb 2022 15:49:30 +0100 Subject: [PATCH 6/8] make share cache test resilient against skeleton files not being there Signed-off-by: Robin Appelman --- apps/files_sharing/tests/CacheTest.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/apps/files_sharing/tests/CacheTest.php b/apps/files_sharing/tests/CacheTest.php index 2623b4c34b594..f4f64bc6a328e 100644 --- a/apps/files_sharing/tests/CacheTest.php +++ b/apps/files_sharing/tests/CacheTest.php @@ -243,17 +243,15 @@ public function testSearchByMime() { public function testGetFolderContentsInRoot() { $results = $this->user2View->getDirectoryContent('/'); + $results = (array_filter($results, function($file) { + return $file->getName() !== 'welcome.txt'; + })); // we should get the shared items "shareddir" and "shared single file.txt" // additional root will always contain the example file "welcome.txt", // so this will be part of the result $this->verifyFiles( [ - [ - 'name' => 'welcome.txt', - 'path' => 'files/welcome.txt', - 'mimetype' => 'text/plain', - ], [ 'name' => 'shareddir', 'path' => 'files/shareddir', From d08d0ebdda82bdbc8c5cc5f7432e98a913fa2321 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 25 Feb 2022 16:01:16 +0100 Subject: [PATCH 7/8] skip localstorage dependend user test when not using local user storage Signed-off-by: Robin Appelman --- tests/lib/User/UserTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index aa56959d3cbcf..7fab7ececcaf5 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -10,9 +10,11 @@ namespace Test\User; use OC\AllConfig; +use OC\Files\Mount\ObjectHomeMountProvider; use OC\Hooks\PublicEmitter; use OC\User\User; use OCP\Comments\ICommentsManager; +use OCP\Files\Storage\IStorageFactory; use OCP\IConfig; use OCP\IUser; use OCP\Notification\IManager as INotificationManager; @@ -214,6 +216,16 @@ public function testDelete() { } public function testDeleteWithDifferentHome() { + + /** @var ObjectHomeMountProvider $homeProvider */ + $homeProvider = \OC::$server->get(ObjectHomeMountProvider::class); + $user = $this->createMock(IUser::class); + $user->method('getUID') + ->willReturn('foo'); + if ($homeProvider->getHomeMountForUser($user, $this->createMock(IStorageFactory::class)) !== null) { + $this->markTestSkipped("Skipping test for non local home storage"); + } + /** * @var Backend | \PHPUnit\Framework\MockObject\MockObject $backend */ From ec15020777c9f0de248ced8b83fe48767c2919b6 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 25 Feb 2022 16:11:38 +0100 Subject: [PATCH 8/8] also handle expired pre-write shared lock on dav upload when not using part files Signed-off-by: Robin Appelman --- apps/dav/lib/Connector/Sabre/File.php | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 28817b93b760d..fa8a46e053082 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -173,7 +173,23 @@ public function put($data) { [$storage, $internalPath] = $this->fileView->resolvePath($this->path); try { if (!$needsPartFile) { - $this->changeLock(ILockingProvider::LOCK_EXCLUSIVE); + try { + $this->changeLock(ILockingProvider::LOCK_EXCLUSIVE); + } catch (LockedException $e) { + // during very large uploads, the shared lock we got at the start might have been expired + // meaning that the above lock can fail not just only because somebody else got a shared lock + // or because there is no existing shared lock to make exclusive + // + // Thus we try to get a new exclusive lock, if the original lock failed because of a different shared + // lock this will still fail, if our original shared lock expired the new lock will be successful and + // the entire operation will be safe + + try { + $this->acquireLock(ILockingProvider::LOCK_EXCLUSIVE); + } catch (LockedException $ex) { + throw new FileLocked($e->getMessage(), $e->getCode(), $e); + } + } } if (!is_resource($data)) {