Merge pull request #31453 from nextcloud/s3-primary-ci-23

[stable23] Run tests with primary object storage in CI + large upload fixes

.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 '<?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

apps/dav/lib/Connector/Sabre/File.php

@@ -184,7 +184,23 @@ class File extends Node implements IFile {
 		[$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)) {

apps/files_sharing/tests/CacheTest.php

@@ -243,17 +243,15 @@ class CacheTest extends TestCase {
 
 	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',

apps/files_trashbin/tests/StorageTest.php

@@ -33,6 +33,7 @@ namespace OCA\Files_Trashbin\Tests;
 
 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 @@ class StorageTest extends \Test\TestCase {
 	}
 
 	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();
 

lib/private/Files/Cache/Storage.php

@@ -158,7 +158,7 @@ class Storage {
 	}
 
 	/**
-	 * @return array|null [ available, last_checked ]
+	 * @return array [ available, last_checked ]
 	 */
 	public function getAvailability() {
 		if ($row = self::getStorageById($this->storageId)) {
@@ -167,7 +167,10 @@ class Storage {
 				'last_checked' => $row['last_checked']
 			];
 		} else {
-			return null;
+			return [
+				'available' => true,
+				'last_checked' => time(),
+			];
 		}
 	}
 

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 @@ class UserTest extends TestCase {
 	}
 
 	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
 		 */