diff --git a/lib/private/Files/ObjectStore/Azure.php b/lib/private/Files/ObjectStore/Azure.php index 899c826ec1..e162c510b9 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 26db551a38..7ee1c8e205 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 280a8efa81..a1110d87c8 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 0d35ba2ed7..f9fc1b5a4a 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 e379e54d01..3d6bf9d69d 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 628fd5852d..83c4b1065d 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 0000000000..4310d8ba27 --- /dev/null +++ b/tests/lib/Files/ObjectStore/FailWriteObjectStore.php @@ -0,0 +1,53 @@ + + * + * @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\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 0000000000..5e8faed334 --- /dev/null +++ b/tests/lib/Files/ObjectStore/ObjectStoreStorageOverwrite.php @@ -0,0 +1,38 @@ + + * + * @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\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 c9d6c1bd92..3b3827f460 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 2116306053..1383c0149a 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')); + } }