summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMorris Jobke <hey@morrisjobke.de>2019-01-14 11:26:29 +0100
committerGitHub <noreply@github.com>2019-01-14 11:26:29 +0100
commit5a27e54f4b6eb50b24ca8a6e4f77878b04e1c3fd (patch)
treec413f25054124afebea96ede701ffea688d04b6c
parent8113dfd4e7587d829631f7656005b0d18897a3c6 (diff)
parentd6bf5d43841f8d80ed54137db75c8ca6d2396cf4 (diff)
downloadnextcloud-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.php13
-rw-r--r--lib/private/Files/ObjectStore/ObjectStoreStorage.php22
-rw-r--r--lib/private/Files/ObjectStore/S3ObjectTrait.php4
-rw-r--r--lib/private/Files/ObjectStore/StorageObjectStore.php3
-rw-r--r--lib/private/Files/ObjectStore/Swift.php3
-rw-r--r--lib/public/Files/ObjectStore/IObjectStore.php9
-rw-r--r--tests/lib/Files/ObjectStore/FailWriteObjectStore.php53
-rw-r--r--tests/lib/Files/ObjectStore/ObjectStoreStorageOverwrite.php38
-rw-r--r--tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php17
-rw-r--r--tests/lib/Files/ObjectStore/ObjectStoreTest.php15
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'));
+ }
}