fix: handle duplicate key on trash unique constraint during concurrent deletes

The unique constraint on oc_group_folders_trash (folder_id, name,
deleted_time) uses second-granularity timestamps. When multiple files
with the same base name inside the same group folder are deleted within
the same second (e.g. by the desktop client issuing bulk DELETEs), the
INSERT in TrashManager::addTrashItem() fails with a unique constraint
violation. Because the file has already been physically moved to trash
storage before the INSERT, the failed DB record leaves the file
orphaned — resulting in data loss.

Insert the trash record before moving the file, using a for-loop that
retries with an incremented deleted_time on constraint collision. This
way the on-disk trash name is constructed from the confirmed timestamp
and no renames are needed. If the subsequent file move fails, the DB
record is cleaned up.

This follows the existing pattern in VersionsBackend.php for handling
the same class of constraint violation.

Fixes: #4014

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Iva Horn <iva.horn@nextcloud.com>

Author: i2h3

lib/Trash/TrashBackend.php

@@ -260,21 +260,39 @@ public function moveToTrash(IStorage $storage, string $internalPath): bool {
 
 			$trashFolder = $this->setupTrashFolder($folder, $storage->getUser());
 			$trashStorage = $trashFolder->getStorage();
+
+			$originalLocation = $internalPath;
+			if ($storage->instanceOfStorage(ISharedStorage::class)) {
+				/** @var Jail $jail */
+				$jail = $storage->getWrapperStorage();
+				$originalLocation = $jail->getUnjailedPath($originalLocation);
+			}
+
+			$deletedBy = $this->userSession->getUser();
+
+			// Insert the trash record before moving the file so that the
+			// on-disk name already matches the confirmed deleted_time.
+			// The unique constraint on (folder_id, name, deleted_time) uses
+			// second granularity, so concurrent deletes of same-named files
+			// can collide.  Retry with an incremented timestamp on conflict.
 			$time = time();
+			for ($retry = 0; $retry < 5; $retry++) {
+				try {
+					$this->trashManager->addTrashItem($folderId, $name, $time, $originalLocation, $fileEntry->getId(), $deletedBy?->getUID() ?? '');
+					break;
+				} catch (\OCP\DB\Exception $e) {
+					if ($e->getReason() === \OCP\DB\Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
+						$time++;
+					} else {
+						throw $e;
+					}
+				}
+			}
+
 			$trashName = $name . '.d' . $time;
 			$targetInternalPath = $trashFolder->getInternalPath() . '/' . $trashName;
 			$result = $trashStorage->moveFromStorage($storage, $internalPath, $targetInternalPath);
 			if ($result) {
-				$originalLocation = $internalPath;
-				if ($storage->instanceOfStorage(ISharedStorage::class)) {
-					/** @var Jail $jail */
-					$jail = $storage->getWrapperStorage();
-					$originalLocation = $jail->getUnjailedPath($originalLocation);
-				}
-
-				$deletedBy = $this->userSession->getUser();
-				$this->trashManager->addTrashItem($folderId, $name, $time, $originalLocation, $fileEntry->getId(), $deletedBy?->getUID() ?? '');
-
 				// some storage backends (object/encryption) can either already move the cache item or cause the target to be scanned
 				// so we only conditionally do the cache move here
 				if (!$trashStorage->getCache()->inCache($targetInternalPath)) {
@@ -285,6 +303,8 @@ public function moveToTrash(IStorage $storage, string $internalPath): bool {
 					$storage->getCache()->remove($internalPath);
 				}
 			} else {
+				// Move failed — clean up the DB record to avoid an orphaned trash entry
+				$this->trashManager->removeItem($folderId, $name, $time);
 				throw new \Exception('Failed to move Team folder item to trash');
 			}
 

tests/Trash/TrashBackendTest.php

@@ -18,6 +18,7 @@
 use OCA\GroupFolders\Folder\FolderManager;
 use OCA\GroupFolders\Mount\GroupFolderStorage;
 use OCA\GroupFolders\Trash\TrashBackend;
+use OCA\GroupFolders\Trash\TrashManager;
 use OCP\Constants;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
@@ -36,6 +37,7 @@ class TrashBackendTest extends TestCase {
 	private string $folderName;
 	private TrashBackend $trashBackend;
 	private FolderManager $folderManager;
+	private TrashManager $trashManager;
 	private RuleManager $ruleManager;
 	private int $folderId;
 	private Folder $managerUserFolder;
@@ -59,6 +61,7 @@ public function setUp(): void {
 		$groupBackend->addToGroup('normal', 'gf_normal');
 
 		$this->trashBackend = \OCP\Server::get(TrashBackend::class);
+		$this->trashManager = \OCP\Server::get(TrashManager::class);
 		$this->folderManager = \OCP\Server::get(FolderManager::class);
 		$this->ruleManager = \OCP\Server::get(RuleManager::class);
 
@@ -284,4 +287,49 @@ public function testWrongOriginalLocation(): void {
 		$this->trashBackend->restoreItem($trashedOfUserA[0]);
 		$this->assertTrue($userAFolder->nodeExists('D/E/F/G'));
 	}
+
+	public function testMoveToTrashSameNameDifferentSubfolders(): void {
+		$this->loginAsUser('manager');
+
+		// Create two subfolders each containing a file with the same name
+		$folderA = $this->managerUserFolder->newFolder("{$this->folderName}/SubA");
+		$folderB = $this->managerUserFolder->newFolder("{$this->folderName}/SubB");
+		$fileA = $folderA->newFile('Readme.md', 'content A');
+		$fileB = $folderB->newFile('Readme.md', 'content B');
+
+		$this->assertTrue($this->managerUserFolder->nodeExists("{$this->folderName}/SubA/Readme.md"));
+		$this->assertTrue($this->managerUserFolder->nodeExists("{$this->folderName}/SubB/Readme.md"));
+
+		// Delete both files — they share the same base name within the same
+		// group folder which previously triggered a unique constraint violation
+		// on (folder_id, name, deleted_time) when both deletes hit the same second.
+		$this->trashBackend->moveToTrash($fileA->getStorage(), $fileA->getInternalPath());
+		$this->trashBackend->moveToTrash($fileB->getStorage(), $fileB->getInternalPath());
+
+		$this->assertFalse($this->managerUserFolder->nodeExists("{$this->folderName}/SubA/Readme.md"));
+		$this->assertFalse($this->managerUserFolder->nodeExists("{$this->folderName}/SubB/Readme.md"));
+
+		// Both files must appear in the trash
+		$trashItems = $this->trashBackend->listTrashRoot($this->managerUser);
+		$readmeItems = array_values(array_filter($trashItems, fn ($item) => $item->getName() === 'Readme.md'));
+		$this->assertCount(2, $readmeItems, 'Both Readme.md files should be in the trash');
+
+		// The two trash entries must have distinct deleted_time values
+		$times = array_map(fn ($item) => $item->getDeletedTime(), $readmeItems);
+		$this->assertCount(2, array_unique($times), 'Trash entries should have distinct deleted_time values');
+
+		// Verify the DB records exist
+		$dbRows = $this->trashManager->listTrashForFolders([$this->folderId]);
+		$dbReadmeRows = array_values(array_filter($dbRows, fn ($row) => $row['name'] === 'Readme.md'));
+		$this->assertCount(2, $dbReadmeRows, 'Both Readme.md entries should exist in oc_group_folders_trash');
+
+		// Restore both items and verify files come back
+		foreach ($readmeItems as $item) {
+			$this->trashBackend->restoreItem($item);
+		}
+		$this->assertTrue($this->managerUserFolder->nodeExists("{$this->folderName}/SubA/Readme.md"));
+		$this->assertTrue($this->managerUserFolder->nodeExists("{$this->folderName}/SubB/Readme.md"));
+
+		$this->logout();
+	}
 }