From 06ea5716ee0f8f921b203ac113b8a78053662149 Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <opensource@fthiessen.de>
Date: Wed, 2 Sep 2026 16:39:14 +0200
Subject: [PATCH] fix(trashbin): properly unpause trashbin on error

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
---
 .../files_trashbin/lib/Trash/TrashManager.php | 16 ++--
 .../tests/Trash/TrashManagerTest.php          | 86 +++++++++++++++++++
 2 files changed, 97 insertions(+), 5 deletions(-)
 create mode 100644 apps/files_trashbin/tests/Trash/TrashManagerTest.php

diff --git a/apps/files_trashbin/lib/Trash/TrashManager.php b/apps/files_trashbin/lib/Trash/TrashManager.php
index cf0f34cff6d69..1a5831c68fd93 100644
--- a/apps/files_trashbin/lib/Trash/TrashManager.php
+++ b/apps/files_trashbin/lib/Trash/TrashManager.php
@@ -109,13 +109,19 @@ public function moveToTrash(IStorage $storage, string $internalPath): bool {
 		}
 		try {
 			$backend = $this->getBackendForStorage($storage);
-			$this->trashPaused = true;
-			$result = $backend->moveToTrash($storage, $internalPath);
-			$this->trashPaused = false;
-			return $result;
-		} catch (BackendNotFoundException $e) {
+		} catch (BackendNotFoundException) {
 			return false;
 		}
+
+		// pausing prevents the backend from recursing into the trash logic again,
+		// it has to be released even when the move fails or the trash bin would
+		// stay disabled for the rest of the request
+		$this->trashPaused = true;
+		try {
+			return $backend->moveToTrash($storage, $internalPath);
+		} finally {
+			$this->trashPaused = false;
+		}
 	}
 
 	#[\Override]
diff --git a/apps/files_trashbin/tests/Trash/TrashManagerTest.php b/apps/files_trashbin/tests/Trash/TrashManagerTest.php
new file mode 100644
index 0000000000000..2d9c0141e13e9
--- /dev/null
+++ b/apps/files_trashbin/tests/Trash/TrashManagerTest.php
@@ -0,0 +1,86 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Files_Trashbin\Tests\Trash;
+
+use OCA\Files_Trashbin\Trash\ITrashBackend;
+use OCA\Files_Trashbin\Trash\TrashManager;
+use OCP\Files\NotFoundException;
+use OCP\Files\Storage\IStorage;
+use PHPUnit\Framework\MockObject\MockObject;
+use Test\TestCase;
+
+class TrashManagerTest extends TestCase {
+
+	private TrashManager $manager;
+	private IStorage&MockObject $storage;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->manager = new TrashManager();
+		$this->storage = $this->createMock(IStorage::class);
+		$this->storage->method('instanceOfStorage')
+			->with(IStorage::class)
+			->willReturn(true);
+	}
+
+	private function registerBackend(ITrashBackend $backend): void {
+		$this->manager->registerBackend(IStorage::class, $backend);
+	}
+
+	public function testMoveToTrashWithoutBackend(): void {
+		$this->assertFalse($this->manager->moveToTrash($this->storage, 'files/test.txt'));
+	}
+
+	public function testMoveToTrashUsesBackend(): void {
+		$backend = $this->createMock(ITrashBackend::class);
+		$backend->expects($this->once())
+			->method('moveToTrash')
+			->with($this->storage, 'files/test.txt')
+			->willReturn(true);
+		$this->registerBackend($backend);
+
+		$this->assertTrue($this->manager->moveToTrash($this->storage, 'files/test.txt'));
+	}
+
+	/**
+	 * The backend must not see the delete it performs itself, but the pause has
+	 * to be released again afterwards so following deletes still get trashed.
+	 */
+	public function testMoveToTrashPausesOnlyDuringTheMove(): void {
+		$backend = $this->createMock(ITrashBackend::class);
+		$backend->expects($this->exactly(2))
+			->method('moveToTrash')
+			->willReturnCallback(function (): bool {
+				$this->assertFalse(
+					$this->manager->moveToTrash($this->storage, 'files/nested.txt'),
+					'Trash has to be paused while the backend moves a file',
+				);
+				return true;
+			});
+		$this->registerBackend($backend);
+
+		$this->assertTrue($this->manager->moveToTrash($this->storage, 'files/first.txt'));
+		$this->assertTrue($this->manager->moveToTrash($this->storage, 'files/second.txt'));
+	}
+
+	public function testMoveToTrashReleasesPauseOnException(): void {
+		$backend = $this->createMock(ITrashBackend::class);
+		$backend->method('moveToTrash')
+			->willThrowException(new NotFoundException('test not found while trying to get owner'));
+		$this->registerBackend($backend);
+
+		$this->expectException(NotFoundException::class);
+		try {
+			$this->manager->moveToTrash($this->storage, 'files/test.txt');
+		} finally {
+			$this->assertFalse($this->invokePrivate($this->manager, 'trashPaused'));
+		}
+	}
+}