fix: keep trashbin cache and db in sync

Signed-off-by: Hoang Pham <hoangmaths96@gmail.com>

Author: hweihwang

apps/files_trashbin/lib/Trashbin.php

@@ -309,6 +309,8 @@ public static function move2trash($file_path, $ownerOnly = false) {
 			$trashStorage->moveFromStorage($sourceStorage, $sourceInternalPath, $trashInternalPath);
 			if ($inCache) {
 				$trashStorage->getUpdater()->renameFromStorage($sourceStorage, $sourceInternalPath, $trashInternalPath);
+			} else {
+				$trashStorage->getUpdater()->update($trashInternalPath);
 			}
 		} catch (CopyRecursiveException $e) {
 			$moveSuccessful = false;
@@ -345,18 +347,51 @@ public static function move2trash($file_path, $ownerOnly = false) {
 				->setValue('location', $query->createNamedParameter($location))
 				->setValue('user', $query->createNamedParameter($owner))
 				->setValue('deleted_by', $query->createNamedParameter($deletedBy));
-			$result = $query->executeStatement();
-			if (!$result) {
-				Server::get(LoggerInterface::class)->error('trash bin database couldn\'t be updated', ['app' => 'files_trashbin']);
+			$inserted = false;
+			try {
+				$inserted = ($query->executeStatement() === 1);
+			} catch (\Throwable $e) {
+				Server::get(LoggerInterface::class)->error(
+					'trash bin database insert failed',
+					[
+						'app' => 'files_trashbin',
+						'exception' => $e,
+						'user' => $owner,
+						'filename' => $filename,
+						'timestamp' => $timestamp,
+					]
+				);
+			}
+			if (!$inserted) {
+				Server::get(LoggerInterface::class)->error(
+					'trash bin database couldn\'t be updated, removing trash payload',
+					[
+						'app' => 'files_trashbin',
+						'user' => $owner,
+						'filename' => $filename,
+						'timestamp' => $timestamp,
+					]
+				);
+				if ($trashStorage->file_exists($trashInternalPath)) {
+					if ($trashStorage->is_dir($trashInternalPath)) {
+						$trashStorage->rmdir($trashInternalPath);
+					} else {
+						$trashStorage->unlink($trashInternalPath);
+					}
+				}
+				$trashStorage->getUpdater()->remove($trashInternalPath);
+				$moveSuccessful = false;
 			}
-			Util::emitHook('\OCA\Files_Trashbin\Trashbin', 'post_moveToTrash', ['filePath' => Filesystem::normalizePath($file_path),
-				'trashPath' => Filesystem::normalizePath(static::getTrashFilename($filename, $timestamp))]);
+			if ($inserted) {
+				Util::emitHook('\OCA\Files_Trashbin\Trashbin', 'post_moveToTrash', ['filePath' => Filesystem::normalizePath($file_path),
+					'trashPath' => Filesystem::normalizePath(static::getTrashFilename($filename, $timestamp))]);
 
-			self::retainVersions($filename, $owner, $ownerPath, $timestamp);
+				self::retainVersions($filename, $owner, $ownerPath, $timestamp);
 
-			// if owner !== user we need to also add a copy to the users trash
-			if ($user !== $owner && $ownerOnly === false) {
-				self::copyFilesToUser($ownerPath, $owner, $file_path, $user, $timestamp);
+				// if owner !== user we need to also add a copy to the users trash
+				if ($user !== $owner && $ownerOnly === false) {
+					self::copyFilesToUser($ownerPath, $owner, $file_path, $user, $timestamp);
+				}
 			}
 		}
 
@@ -689,13 +724,6 @@ public static function delete($filename, $user, $timestamp = null) {
 		$size = 0;
 
 		if ($timestamp) {
-			$query = Server::get(IDBConnection::class)->getQueryBuilder();
-			$query->delete('files_trash')
-				->where($query->expr()->eq('user', $query->createNamedParameter($user)))
-				->andWhere($query->expr()->eq('id', $query->createNamedParameter($filename)))
-				->andWhere($query->expr()->eq('timestamp', $query->createNamedParameter($timestamp)));
-			$query->executeStatement();
-
 			$file = static::getTrashFilename($filename, $timestamp);
 		} else {
 			$file = $filename;
@@ -706,6 +734,14 @@ public static function delete($filename, $user, $timestamp = null) {
 		try {
 			$node = $userRoot->get('/files_trashbin/files/' . $file);
 		} catch (NotFoundException $e) {
+			if ($timestamp) {
+				$query = Server::get(IDBConnection::class)->getQueryBuilder();
+				$query->delete('files_trash')
+					->where($query->expr()->eq('user', $query->createNamedParameter($user)))
+					->andWhere($query->expr()->eq('id', $query->createNamedParameter($filename)))
+					->andWhere($query->expr()->eq('timestamp', $query->createNamedParameter($timestamp)));
+				$query->executeStatement();
+			}
 			return $size;
 		}
 
@@ -719,6 +755,15 @@ public static function delete($filename, $user, $timestamp = null) {
 		$node->delete();
 		self::emitTrashbinPostDelete('/files_trashbin/files/' . $file);
 
+		if ($timestamp) {
+			$query = Server::get(IDBConnection::class)->getQueryBuilder();
+			$query->delete('files_trash')
+				->where($query->expr()->eq('user', $query->createNamedParameter($user)))
+				->andWhere($query->expr()->eq('id', $query->createNamedParameter($filename)))
+				->andWhere($query->expr()->eq('timestamp', $query->createNamedParameter($timestamp)));
+			$query->executeStatement();
+		}
+
 		return $size;
 	}
 

apps/files_trashbin/tests/StorageTest.php

@@ -118,6 +118,25 @@ public function testSingleStorageDeleteFile(): void {
 		$this->assertEquals('test.txt', substr($name, 0, strrpos($name, '.')));
 	}
 
+	public function testTrashEntryCreatedWhenSourceNotInCache(): void {
+		$this->userView->file_put_contents('uncached.txt', 'foo');
+
+		[$storage, $internalPath] = $this->userView->resolvePath('uncached.txt');
+		$cache = $storage->getCache();
+		$cache->remove($internalPath);
+		$this->assertFalse($cache->inCache($internalPath));
+
+		$this->userView->unlink('uncached.txt');
+
+		$results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/');
+		$this->assertCount(1, $results);
+		$name = $results[0]->getName();
+		$this->assertEquals('uncached.txt', substr($name, 0, strrpos($name, '.')));
+
+		[$trashStorage, $trashInternalPath] = $this->rootView->resolvePath('/' . $this->user . '/files_trashbin/files/' . $name);
+		$this->assertTrue($trashStorage->getCache()->inCache($trashInternalPath));
+	}
+
 	/**
 	 * Test that deleting a folder puts it into the trashbin.
 	 */