Merge pull request #17262 from nextcloud/objectstore-remove-cache-on-delete

dont delete cache entries if deleting an object from object store failed

3 files changed, 90 insertions, 3 deletions

diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php
index fbfbcfaa409..80f438d762c 100644
--- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php
+++ b/lib/private/Files/ObjectStore/ObjectStoreStorage.php
@@ -161,7 +161,9 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common {
 			return false;
 		}
 
-		$this->rmObjects($path);
+		if (!$this->rmObjects($path)) {
+			return false;
+		}
 
 		$this->getCache()->remove($path);
 
@@ -172,11 +174,17 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common {
 		$children = $this->getCache()->getFolderContents($path);
 		foreach ($children as $child) {
 			if ($child['mimetype'] === 'httpd/unix-directory') {
-				$this->rmObjects($child['path']);
+				if (!$this->rmObjects($child['path'])) {
+					return false;
+				}
 			} else {
-				$this->unlink($child['path']);
+				if(!$this->unlink($child['path'])) {
+					return false;
+				}
 			}
 		}
+
+		return true;
 	}
 
 	public function unlink($path) {
diff --git a/tests/lib/Files/ObjectStore/FailDeleteObjectStore.php b/tests/lib/Files/ObjectStore/FailDeleteObjectStore.php
new file mode 100644
index 00000000000..4ad3798a467
--- /dev/null
+++ b/tests/lib/Files/ObjectStore/FailDeleteObjectStore.php
@@ -0,0 +1,54 @@
+<?php declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2019 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 FailDeleteObjectStore 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) {
+		return $this->objectStore->writeObject($urn, $stream);
+	}
+
+	public function deleteObject($urn) {
+		throw new \Exception();
+	}
+
+	public function objectExists($urn) {
+		return $this->objectStore->objectExists($urn);
+	}
+}
+
diff --git a/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php b/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php
index 3b3827f460a..ce9c9d2cd69 100644
--- a/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php
+++ b/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php
@@ -181,4 +181,29 @@ class ObjectStoreStorageTest extends Storage {
 		}
 		$this->assertFalse($this->instance->file_exists('test.txt'));
 	}
+
+	public function testDeleteObjectFailureKeepCache() {
+		$objectStore = $this->instance->getObjectStore();
+		$this->instance->setObjectStore(new FailDeleteObjectStore($objectStore));
+		$cache = $this->instance->getCache();
+
+		$this->instance->file_put_contents('test.txt', 'foo');
+
+		$this->assertTrue($cache->inCache('test.txt'));
+
+		$this->assertFalse($this->instance->unlink('test.txt'));
+
+		$this->assertTrue($cache->inCache('test.txt'));
+
+		$this->instance->mkdir('foo');
+		$this->instance->file_put_contents('foo/test.txt', 'foo');
+
+		$this->assertTrue($cache->inCache('foo'));
+		$this->assertTrue($cache->inCache('foo/test.txt'));
+
+		$this->instance->rmdir('foo');
+
+		$this->assertTrue($cache->inCache('foo'));
+		$this->assertTrue($cache->inCache('foo/test.txt'));
+	}
 }