Skip to content
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.

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

move root mount setup to mountproviders #31266

Merged
merged 8 commits into from
Mar 4, 2022
70 changes: 70 additions & 0 deletions .github/workflows/s3-primary.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
name: S3 primary storage
Copy link
Member

Choose a reason for hiding this comment

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

hurrah for more tests!

Copy link
Member

Choose a reason for hiding this comment

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

is this backportable separately?

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 '<?php $CONFIG=["${{ matrix.key }}" => ["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
18 changes: 17 additions & 1 deletion apps/dav/lib/Connector/Sabre/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

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

is this directly related to this PR or is this a fix that we could use separately ?

} catch (LockedException $ex) {
throw new FileLocked($e->getMessage(), $e->getCode(), $e);
}
}
}

if (!is_resource($data)) {
Expand Down
8 changes: 3 additions & 5 deletions apps/files_sharing/tests/CacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
4 changes: 4 additions & 0 deletions apps/files_trashbin/tests/StorageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
3 changes: 2 additions & 1 deletion lib/private/Avatar/AvatarManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
7 changes: 5 additions & 2 deletions lib/private/Files/Cache/Storage.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -167,7 +167,10 @@ public function getAvailability() {
'last_checked' => $row['last_checked']
];
} else {
return null;
return [
'available' => true,
'last_checked' => time(),
];
}
}

Expand Down
103 changes: 103 additions & 0 deletions lib/private/Files/Mount/RootMountProvider.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
<?php

declare(strict_types=1);
/**
* @copyright Copyright (c) 2022 Robin Appelman <robin@icewind.nl>
*
* @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 <http://www.gnu.org/licenses/>.
*
*/

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);
}
}
2 changes: 2 additions & 0 deletions lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Loading