upload new files in objectstore to a .part path first

Signed-off-by: Robin Appelman <robin@icewind.nl>

lib/private/Files/ObjectStore/ObjectStoreStorage.php

@@ -413,18 +413,30 @@ 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
 			$this->objectStore->writeObject($this->getURN($fileId), fopen($tmpFile, 'r'));
 		} 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);
+			}
+		}
 	}
 
 	/**

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);
+	}
+}

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;
+	}
+}

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'));
+	}
 }