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

test: re-add object store primary storage phpunit tests #48235

Merged
merged 5 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
121 changes: 121 additions & 0 deletions .github/workflows/phpunit-object-store-primary.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
# SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
# SPDX-License-Identifier: MIT
name: PHPUnit primary object store
Copy link
Contributor

Choose a reason for hiding this comment

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

This is what object-storage-s3.yml should do so I would not add a new one but just enable more tests there then.

Copy link
Member Author

Choose a reason for hiding this comment

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

The object-storage-* tests test the implementation of the object store. This tests how the/any object store integrates with the rest of the system

Copy link
Contributor

Choose a reason for hiding this comment

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

So this is more of an integration test?

But what I meant is why not just add this as a step to the other workflow. Then one would only need to configure all services once and less work if something changes.

on:
pull_request:
schedule:
- cron: "15 2 * * *"

concurrency:
group: phpunit-object-store-primary-${{ github.head_ref || github.run_id }}
cancel-in-progress: true

jobs:
changes:
runs-on: ubuntu-latest-low

outputs:
src: ${{ steps.changes.outputs.src}}

steps:
- uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
id: changes
continue-on-error: true
with:
filters: |
src:
- '.github/workflows/**'
- '3rdparty/**'
- '**/appinfo/**'
- '**/lib/**'
- '**/templates/**'
- '**/tests/**'
- 'vendor/**'
- 'vendor-bin/**'
- '.php-cs-fixer.dist.php'
- 'composer.json'
- 'composer.lock'
- '**.php'

object-store-primary-tests-minio:
runs-on: ubuntu-latest
needs: changes

if: ${{ github.repository_owner != 'nextcloud-gmbh' && needs.changes.outputs.src != 'false' }}

strategy:
# do not stop on another job's failure
fail-fast: false
matrix:
php-versions: ['8.1']
key: ['s3', 's3-multibucket']

name: php${{ matrix.php-versions }}-${{ matrix.key }}-minio

services:
cache:
image: ghcr.io/nextcloud/continuous-integration-redis:latest
ports:
- 6379:6379/tcp
options: --health-cmd="redis-cli ping" --health-interval=10s --health-timeout=5s --health-retries=3

minio:
image: bitnami/minio
env:
MINIO_ROOT_USER: nextcloud
MINIO_ROOT_PASSWORD: bWluaW8tc2VjcmV0LWtleS1uZXh0Y2xvdWQ=
MINIO_DEFAULT_BUCKETS: nextcloud
ports:
- "9000:9000"

steps:
- name: Checkout server
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
with:
submodules: true

- name: Set up php ${{ matrix.php-versions }}
uses: shivammathur/setup-php@c5fc0d8281aba02c7fda07d3a70cc5371548067d #v2.25.2
with:
php-version: ${{ matrix.php-versions }}
extensions: mbstring, fileinfo, intl, sqlite, pdo_sqlite, zip, gd
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Set up Nextcloud
env:
OBJECT_STORE: ${{ matrix.key }}
OBJECT_STORE_KEY: nextcloud
OBJECT_STORE_SECRET: bWluaW8tc2VjcmV0LWtleS1uZXh0Y2xvdWQ=
run: |
composer install
cp tests/redis.config.php config/
cp tests/preseed-config.php 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 tests/enable_all.php | grep -i -C9999 error && echo "Error during app setup" && exit 1 || exit 0

- name: Wait for S3
run: |
sleep 10
curl -f -m 1 --retry-connrefused --retry 10 --retry-delay 10 http://localhost:9000/minio/health/ready

- name: PHPUnit
run: composer run test:db

- name: S3 logs
if: always()
run: |
cat data/nextcloud.log
docker ps -a
docker ps -aq | while read container ; do IMAGE=$(docker inspect --format='{{.Config.Image}}' $container); echo $IMAGE; docker logs $container; echo "\n\n" ; done


object-store-primary-summary:
runs-on: ubuntu-latest-low
needs: [changes,object-store-primary-tests-minio]

if: always()

steps:
- name: Summary status
run: if ${{ needs.changes.outputs.src != 'false' && needs.object-store-primary-tests-minio.result != 'success' }}; then exit 1; fi
1 change: 1 addition & 0 deletions apps/files_sharing/tests/SharedStorageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ public function testMoveFromStorage(): void {

$sourceStorage = new \OC\Files\Storage\Temporary([]);
$sourceStorage->file_put_contents('foo.txt', 'asd');
$sourceStorage->getScanner()->scan('');

$sharedStorage->moveFromStorage($sourceStorage, 'foo.txt', 'bar.txt');
$this->assertTrue($sharedStorage->file_exists('bar.txt'));
Expand Down
4 changes: 0 additions & 4 deletions apps/files_trashbin/tests/StorageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

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 @@ -673,9 +672,6 @@ public function testTrashbinCollision(): void {
}

public function testMoveFromStoragePreserveFileId(): void {
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
86 changes: 66 additions & 20 deletions lib/private/Files/ObjectStore/ObjectStoreStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common implements IChunkedFil

private bool $handleCopiesAsOwned;
protected bool $validateWrites = true;
private bool $preserveCacheItemsOnDelete = false;

/**
* @param array $params
Expand Down Expand Up @@ -171,7 +172,9 @@ private function rmObjects(ICacheEntry $entry): bool {
}
}

$this->getCache()->remove($entry->getPath());
if (!$this->preserveCacheItemsOnDelete) {
$this->getCache()->remove($entry->getPath());
}

return true;
}
Expand Down Expand Up @@ -206,7 +209,9 @@ public function rmObject(ICacheEntry $entry): bool {
}
//removing from cache is ok as it does not exist in the objectstore anyway
}
$this->getCache()->remove($entry->getPath());
if (!$this->preserveCacheItemsOnDelete) {
$this->getCache()->remove($entry->getPath());
}
return true;
}

Expand Down Expand Up @@ -596,31 +601,68 @@ public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $t
if (!$sourceCacheEntry) {
$sourceCacheEntry = $sourceCache->get($sourceInternalPath);
}
if ($sourceCacheEntry->getMimeType() === FileInfo::MIMETYPE_FOLDER) {
$this->mkdir($targetInternalPath);
foreach ($sourceCache->getFolderContentsById($sourceCacheEntry->getId()) as $child) {
$this->moveFromStorage($sourceStorage, $child->getPath(), $targetInternalPath . '/' . $child->getName(), $child);
}

$this->copyObjects($sourceStorage, $sourceCache, $sourceCacheEntry);
if ($sourceStorage->instanceOfStorage(ObjectStoreStorage::class)) {
/** @var ObjectStoreStorage $sourceStorage */
$sourceStorage->setPreserveCacheOnDelete(true);
}
if ($sourceCacheEntry->getMimeType() === ICacheEntry::DIRECTORY_MIMETYPE) {
$sourceStorage->rmdir($sourceInternalPath);
} else {
$sourceStream = $sourceStorage->fopen($sourceInternalPath, 'r');
if (!$sourceStream) {
return false;
}
// move the cache entry before the contents so that we have the correct fileid/urn for the target
$this->getCache()->moveFromCache($sourceCache, $sourceInternalPath, $targetInternalPath);
try {
$this->writeStream($targetInternalPath, $sourceStream, $sourceCacheEntry->getSize());
} catch (\Exception $e) {
// restore the cache entry
$sourceCache->moveFromCache($this->getCache(), $targetInternalPath, $sourceInternalPath);
throw $e;
}
$sourceStorage->unlink($sourceInternalPath);
}
if ($sourceStorage->instanceOfStorage(ObjectStoreStorage::class)) {
/** @var ObjectStoreStorage $sourceStorage */
$sourceStorage->setPreserveCacheOnDelete(false);
}
$this->getCache()->moveFromCache($sourceCache, $sourceInternalPath, $targetInternalPath);

return true;
}

/**
* Copy the object(s) of a file or folder into this storage, without touching the cache
*/
private function copyObjects(IStorage $sourceStorage, ICache $sourceCache, ICacheEntry $sourceCacheEntry) {
$copiedFiles = [];
try {
foreach ($this->getAllChildObjects($sourceCache, $sourceCacheEntry) as $file) {
$sourceStream = $sourceStorage->fopen($file->getPath(), 'r');
if (!$sourceStream) {
throw new \Exception("Failed to open source file {$file->getPath()} ({$file->getId()})");
}
$this->objectStore->writeObject($this->getURN($file->getId()), $sourceStream, $file->getMimeType());
if (is_resource($sourceStream)) {
fclose($sourceStream);
}
$copiedFiles[] = $file->getId();
}
} catch (\Exception $e) {
foreach ($copiedFiles as $fileId) {
try {
$this->objectStore->deleteObject($this->getURN($fileId));
} catch (\Exception $e) {
// ignore
}
}
throw $e;
}
}

/**
* @return \Iterator<ICacheEntry>
*/
private function getAllChildObjects(ICache $cache, ICacheEntry $entry): \Iterator {
if ($entry->getMimeType() === FileInfo::MIMETYPE_FOLDER) {
foreach ($cache->getFolderContentsById($entry->getId()) as $child) {
yield from $this->getAllChildObjects($cache, $child);
}
} else {
yield $entry;
}
}

public function copy($source, $target): bool {
$source = $this->normalizePath($source);
$target = $this->normalizePath($target);
Expand Down Expand Up @@ -754,4 +796,8 @@ public function cancelChunkedWrite(string $targetPath, string $writeToken): void
$urn = $this->getURN($cacheEntry->getId());
$this->objectStore->abortMultipartUpload($urn, $writeToken);
}

public function setPreserveCacheOnDelete(bool $preserve) {
$this->preserveCacheItemsOnDelete = $preserve;
}
}
20 changes: 16 additions & 4 deletions lib/private/Files/Storage/Common.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use OC\Files\Cache\Watcher;
use OC\Files\FilenameValidator;
use OC\Files\Filesystem;
use OC\Files\ObjectStore\ObjectStoreStorage;
use OC\Files\Storage\Wrapper\Jail;
use OC\Files\Storage\Wrapper\Wrapper;
use OCP\Files\Cache\ICache;
Expand Down Expand Up @@ -586,10 +587,21 @@ public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $t

$result = $this->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath, true);
if ($result) {
if ($sourceStorage->is_dir($sourceInternalPath)) {
$result = $sourceStorage->rmdir($sourceInternalPath);
} else {
$result = $sourceStorage->unlink($sourceInternalPath);
if ($sourceStorage->instanceOfStorage(ObjectStoreStorage::class)) {
/** @var ObjectStoreStorage $sourceStorage */
$sourceStorage->setPreserveCacheOnDelete(true);
}
try {
if ($sourceStorage->is_dir($sourceInternalPath)) {
$result = $sourceStorage->rmdir($sourceInternalPath);
} else {
$result = $sourceStorage->unlink($sourceInternalPath);
}
} finally {
if ($sourceStorage->instanceOfStorage(ObjectStoreStorage::class)) {
/** @var ObjectStoreStorage $sourceStorage */
$sourceStorage->setPreserveCacheOnDelete(false);
}
}
}
return $result;
Expand Down
15 changes: 15 additions & 0 deletions tests/preseed-config.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@
'use_path_style' => true
]
];
} elseif (getenv('OBJECT_STORE') === 's3-multibucket') {
$CONFIG['objectstore_multibucket'] = [
'class' => 'OC\\Files\\ObjectStore\\S3',
'arguments' => [
'bucket' => 'nextcloud',
'autocreate' => true,
'key' => getenv('OBJECT_STORE_KEY') ?: 'nextcloud',
'secret' => getenv('OBJECT_STORE_SECRET') ?: 'nextcloud',
'hostname' => getenv('OBJECT_STORE_HOST') ?: 'localhost',
'port' => 9000,
'use_ssl' => false,
// required for some non amazon s3 implementations
'use_path_style' => true
]
];
} elseif (getenv('OBJECT_STORE') === 'azure') {
$CONFIG['objectstore'] = [
'class' => 'OC\\Files\\ObjectStore\\Azure',
Expand Down
Loading