diff options
author | Morris Jobke <hey@morrisjobke.de> | 2019-01-14 11:26:29 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-01-14 11:26:29 +0100 |
commit | 5a27e54f4b6eb50b24ca8a6e4f77878b04e1c3fd (patch) | |
tree | c413f25054124afebea96ede701ffea688d04b6c | |
parent | 8113dfd4e7587d829631f7656005b0d18897a3c6 (diff) | |
parent | d6bf5d43841f8d80ed54137db75c8ca6d2396cf4 (diff) | |
download | nextcloud-server-5a27e54f4b6eb50b24ca8a6e4f77878b04e1c3fd.tar.gz nextcloud-server-5a27e54f4b6eb50b24ca8a6e4f77878b04e1c3fd.zip |
Merge pull request #13032 from nextcloud/objectstore-write-exists
upload new files in objectstore to a .part path first
-rw-r--r-- | lib/private/Files/ObjectStore/Azure.php | 13 | ||||
-rw-r--r-- | lib/private/Files/ObjectStore/ObjectStoreStorage.php | 22 | ||||
-rw-r--r-- | lib/private/Files/ObjectStore/S3ObjectTrait.php | 4 | ||||
-rw-r--r-- | lib/private/Files/ObjectStore/StorageObjectStore.php | 3 | ||||
-rw-r--r-- | lib/private/Files/ObjectStore/Swift.php | 3 | ||||
-rw-r--r-- | lib/public/Files/ObjectStore/IObjectStore.php | 9 | ||||
-rw-r--r-- | tests/lib/Files/ObjectStore/FailWriteObjectStore.php | 53 | ||||
-rw-r--r-- | tests/lib/Files/ObjectStore/ObjectStoreStorageOverwrite.php | 38 | ||||
-rw-r--r-- | tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php | 17 | ||||
-rw-r--r-- | tests/lib/Files/ObjectStore/ObjectStoreTest.php | 15 |
10 files changed, 171 insertions, 6 deletions
diff --git a/lib/private/Files/ObjectStore/Azure.php b/lib/private/Files/ObjectStore/Azure.php index 899c826ec19..e162c510b98 100644 --- a/lib/private/Files/ObjectStore/Azure.php +++ b/lib/private/Files/ObjectStore/Azure.php @@ -115,4 +115,17 @@ class Azure implements IObjectStore { public function deleteObject($urn) { $this->getBlobClient()->deleteBlob($this->containerName, $urn); } + + public function objectExists($urn) { + try { + $this->getBlobClient()->getBlobMetadata($this->containerName, $urn); + return true; + } catch (ServiceException $e) { + if ($e->getCode() === 404) { + return false; + } else { + throw $e; + } + } + } } diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php index 26db551a384..7ee1c8e2055 100644 --- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php +++ b/lib/private/Files/ObjectStore/ObjectStoreStorage.php @@ -436,7 +436,10 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common { $stat['mimetype'] = $mimetype; $stat['etag'] = $this->getETag($path); - $fileId = $this->getCache()->put($path, $stat); + $exists = $this->getCache()->inCache($path); + $uploadPath = $exists ? $path : $path . '.part'; + $fileId = $this->getCache()->put($uploadPath, $stat); + $urn = $this->getURN($fileId); try { //upload to object storage if ($size === null) { @@ -446,22 +449,31 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common { ]); $size = $writtenSize; }); - $this->objectStore->writeObject($this->getURN($fileId), $countStream); + $this->objectStore->writeObject($urn, $countStream); if (is_resource($countStream)) { fclose($countStream); } } else { - $this->objectStore->writeObject($this->getURN($fileId), $stream); + $this->objectStore->writeObject($urn, $stream); } } catch (\Exception $ex) { - $this->getCache()->remove($path); + $this->getCache()->remove($uploadPath); $this->logger->logException($ex, [ 'app' => 'objectstore', - 'message' => 'Could not create object ' . $this->getURN($fileId) . ' for ' . $path, + 'message' => 'Could not create object ' . $urn . ' for ' . $path, ]); throw $ex; // make this bubble up } + if (!$exists) { + if ($this->objectStore->objectExists($urn)) { + $this->getCache()->move($uploadPath, $path); + } else { + $this->getCache()->remove($uploadPath); + throw new \Exception("Object not found after writing (urn: $urn, path: $path)", 404); + } + } + return $size; } } diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index 280a8efa81c..a1110d87c8f 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -90,4 +90,8 @@ trait S3ObjectTrait { 'Key' => $urn ]); } + + public function objectExists($urn) { + return $this->getConnection()->doesObjectExist($this->bucket, $urn); + } } diff --git a/lib/private/Files/ObjectStore/StorageObjectStore.php b/lib/private/Files/ObjectStore/StorageObjectStore.php index 0d35ba2ed7a..f9fc1b5a4aa 100644 --- a/lib/private/Files/ObjectStore/StorageObjectStore.php +++ b/lib/private/Files/ObjectStore/StorageObjectStore.php @@ -89,4 +89,7 @@ class StorageObjectStore implements IObjectStore { $this->storage->unlink($urn); } + public function objectExists($urn) { + return $this->storage->file_exists($urn); + } } diff --git a/lib/private/Files/ObjectStore/Swift.php b/lib/private/Files/ObjectStore/Swift.php index e379e54d018..3d6bf9d69da 100644 --- a/lib/private/Files/ObjectStore/Swift.php +++ b/lib/private/Files/ObjectStore/Swift.php @@ -128,4 +128,7 @@ class Swift implements IObjectStore { $this->getContainer()->delete(); } + public function objectExists($urn) { + return $this->getContainer()->objectExists($urn); + } } diff --git a/lib/public/Files/ObjectStore/IObjectStore.php b/lib/public/Files/ObjectStore/IObjectStore.php index 628fd5852da..83c4b1065d6 100644 --- a/lib/public/Files/ObjectStore/IObjectStore.php +++ b/lib/public/Files/ObjectStore/IObjectStore.php @@ -63,4 +63,13 @@ interface IObjectStore { * @since 7.0.0 */ public function deleteObject($urn); + + /** + * Check if an object exists in the object store + * + * @param string $urn + * @return bool + * @since 16.0.0 + */ + public function objectExists($urn); } diff --git a/tests/lib/Files/ObjectStore/FailWriteObjectStore.php b/tests/lib/Files/ObjectStore/FailWriteObjectStore.php new file mode 100644 index 00000000000..4310d8ba27c --- /dev/null +++ b/tests/lib/Files/ObjectStore/FailWriteObjectStore.php @@ -0,0 +1,53 @@ +<?php declare(strict_types=1); +/** + * @copyright Copyright (c) 2018 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 Test\Files\ObjectStore; + +use OCP\Files\ObjectStore\IObjectStore; + +class FailWriteObjectStore implements IObjectStore { + private $objectStore; + + public function __construct(IObjectStore $objectStore) { + $this->objectStore = $objectStore; + } + + public function getStorageId() { + return $this->objectStore->getStorageId(); + } + + public function readObject($urn) { + return $this->objectStore->readObject($urn); + } + + public function writeObject($urn, $stream) { + // emulate a failed write that didn't throw an error + return true; + } + + public function deleteObject($urn) { + $this->objectStore->deleteObject($urn); + } + + public function objectExists($urn) { + return $this->objectStore->objectExists($urn); + } +} diff --git a/tests/lib/Files/ObjectStore/ObjectStoreStorageOverwrite.php b/tests/lib/Files/ObjectStore/ObjectStoreStorageOverwrite.php new file mode 100644 index 00000000000..5e8faed3347 --- /dev/null +++ b/tests/lib/Files/ObjectStore/ObjectStoreStorageOverwrite.php @@ -0,0 +1,38 @@ +<?php declare(strict_types=1); +/** + * @copyright Copyright (c) 2018 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 Test\Files\ObjectStore; + +use OC\Files\ObjectStore\ObjectStoreStorage; +use OCP\Files\ObjectStore\IObjectStore; + +/** + * Allow overwriting the object store instance for test purposes + */ +class ObjectStoreStorageOverwrite extends ObjectStoreStorage { + public function setObjectStore(IObjectStore $objectStore) { + $this->objectStore = $objectStore; + } + + public function getObjectStore(): IObjectStore { + return $this->objectStore; + } +} diff --git a/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php b/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php index c9d6c1bd922..3b3827f460a 100644 --- a/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php +++ b/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php @@ -30,6 +30,8 @@ use Test\Files\Storage\Storage; * @group DB */ class ObjectStoreStorageTest extends Storage { + /** @var ObjectStoreStorageOverwrite */ + protected $instance; /** * @var IObjectStore @@ -42,7 +44,7 @@ class ObjectStoreStorageTest extends Storage { $baseStorage = new Temporary(); $this->objectStorage = new StorageObjectStore($baseStorage); $config['objectstore'] = $this->objectStorage; - $this->instance = new ObjectStoreStorage($config); + $this->instance = new ObjectStoreStorageOverwrite($config); } protected function tearDown() { @@ -166,4 +168,17 @@ class ObjectStoreStorageTest extends Storage { $targetId = $this->instance->getCache()->getId('target'); $this->assertSame($sourceId, $targetId, 'fileid must be stable on move or shares will break'); } + + public function testWriteObjectSilentFailure() { + $objectStore = $this->instance->getObjectStore(); + $this->instance->setObjectStore(new FailWriteObjectStore($objectStore)); + + try { + $this->instance->file_put_contents('test.txt', 'foo'); + $this->fail('expected exception'); + } catch (\Exception $e) { + $this->assertStringStartsWith('Object not found after writing', $e->getMessage()); + } + $this->assertFalse($this->instance->file_exists('test.txt')); + } } diff --git a/tests/lib/Files/ObjectStore/ObjectStoreTest.php b/tests/lib/Files/ObjectStore/ObjectStoreTest.php index 2116306053e..1383c0149a2 100644 --- a/tests/lib/Files/ObjectStore/ObjectStoreTest.php +++ b/tests/lib/Files/ObjectStore/ObjectStoreTest.php @@ -94,4 +94,19 @@ abstract class ObjectStoreTest extends TestCase { $this->assertEquals(1, 1); } } + + public function testExists() { + $stream = $this->stringToStream('foobar'); + + $instance = $this->getInstance(); + $this->assertFalse($instance->objectExists('2')); + + $instance->writeObject('2', $stream); + + $this->assertTrue($instance->objectExists('2')); + + $instance->deleteObject('2'); + + $this->assertFalse($instance->objectExists('2')); + } } |