Fix versions restoring with S3

Signed-off-by: Louis Chemineau <[email protected]>
nextcloud · Jan 26, 2023 · 88abb2d · 88abb2d
1 parent 3da63f4
commit 88abb2d
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 5 deletions.
diff --git a/apps/files_versions/lib/Listener/FileEventsListener.php b/apps/files_versions/lib/Listener/FileEventsListener.php
@@ -30,13 +30,16 @@
  */
 namespace OCA\Files_Versions\Listener;
 
+use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
+use OC\DB\Exceptions\DbalException;
 use OC\Files\Filesystem;
 use OC\Files\Mount\MoveableMount;
 use OC\Files\Node\NonExistingFile;
 use OC\Files\View;
 use OCA\Files_Versions\Db\VersionEntity;
 use OCA\Files_Versions\Db\VersionsMapper;
 use OCA\Files_Versions\Storage;
+use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\EventDispatcher\Event;
 use OCP\EventDispatcher\IEventListener;
 use OCP\Files\Events\Node\BeforeNodeCopiedEvent;
@@ -151,14 +154,30 @@ public function touch_hook(Node $node): void {
 
 		unset($this->nodesTouched[$node->getId()]);
 
-		// We update the timestamp of the version entity associated with the previousNode.
-		$versionEntity = $this->versionsMapper->findVersionForFileId($previousNode->getId(), $previousNode->getMTime());
-		// Create a version in the DB for the current content.
-		$versionEntity->setTimestamp($node->getMTime());
-		$this->versionsMapper->update($versionEntity);
+		try {
+			// We update the timestamp of the version entity associated with the previousNode.
+			$versionEntity = $this->versionsMapper->findVersionForFileId($previousNode->getId(), $previousNode->getMTime());
+			// Create a version in the DB for the current content.
+			$versionEntity->setTimestamp($node->getMTime());
+			$this->versionsMapper->update($versionEntity);
+		} catch (DbalException $ex) {
+			// Ignore UniqueConstraintViolationException, as we are probably in the middle of a rollback
+			// Where the previous node would temporary have the mtime of the old version, so the rollback touches it to fix it.
+			if (!($ex->getPrevious() instanceof UniqueConstraintViolationException)) {
+				throw $ex;
+			}
+		} catch (DoesNotExistException $ex) {
+			// Ignore DoesNotExistException, as we are probably in the middle of a rollback
+			// Where the previous node would temporary have a wrong mtime, so the rollback touches it to fix it.
+		}
 	}
 
 	public function created(Node $node): void {
+		// Do not handle folders.
+		if ($node instanceof Folder) {
+			return;
+		}
+
 		$versionEntity = new VersionEntity();
 		$versionEntity->setFileId($node->getId());
 		$versionEntity->setTimestamp($node->getMTime());

diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php
@@ -35,6 +35,9 @@
 namespace OCA\Files_Versions\Tests;
 
 use OC\Files\Storage\Temporary;
+use OCA\Files_Versions\Db\VersionEntity;
+use OCA\Files_Versions\Db\VersionsMapper;
+use OCP\Files\IMimeTypeLoader;
 use OCP\IConfig;
 use OCP\IUser;
 use OCP\Share\IShare;
@@ -54,6 +57,14 @@ class VersioningTest extends \Test\TestCase {
 	 * @var \OC\Files\View
 	 */
 	private $rootView;
+	/**
+	 * @var VersionsMapper
+	 */
+	private $versionsMapper;
+	/**
+	 * @var IMimeTypeLoader
+	 */
+	private $mimeTypeLoader;
 	private $user1;
 	private $user2;
 
@@ -108,6 +119,9 @@ protected function setUp(): void {
 			$this->rootView->mkdir(self::USERS_VERSIONS_ROOT);
 		}
 
+		$this->versionsMapper = \OCP\Server::get(VersionsMapper::class);
+		$this->mimeTypeLoader = \OCP\Server::get(IMimeTypeLoader::class);
+
 		$this->user1 = $this->createMock(IUser::class);
 		$this->user1->method('getUID')
 			->willReturn(self::TEST_VERSIONS_USER);
@@ -762,6 +776,7 @@ private function doTestRestore() {
 		$filePath = self::TEST_VERSIONS_USER . '/files/sub/test.txt';
 		$this->rootView->file_put_contents($filePath, 'test file');
 
+		$fileInfo = $this->rootView->getFileInfo($filePath);
 		$t0 = $this->rootView->filemtime($filePath);
 
 		// not exactly the same timestamp as the file
@@ -774,8 +789,26 @@ private function doTestRestore() {
 		$v2 = self::USERS_VERSIONS_ROOT . '/sub/test.txt.v' . $t2;
 
 		$this->rootView->mkdir(self::USERS_VERSIONS_ROOT . '/sub');
+
 		$this->rootView->file_put_contents($v1, 'version1');
+		$fileInfoV1 = $this->rootView->getFileInfo($v1);
+		$versionEntity = new VersionEntity();
+		$versionEntity->setFileId($fileInfo->getId());
+		$versionEntity->setTimestamp($t1);
+		$versionEntity->setSize($fileInfoV1->getSize());
+		$versionEntity->setMimetype($this->mimeTypeLoader->getId($fileInfoV1->getMimetype()));
+		$versionEntity->setMetadata([]);
+		$this->versionsMapper->insert($versionEntity);
+
 		$this->rootView->file_put_contents($v2, 'version2');
+		$fileInfoV2 = $this->rootView->getFileInfo($v2);
+		$versionEntity = new VersionEntity();
+		$versionEntity->setFileId($fileInfo->getId());
+		$versionEntity->setTimestamp($t2);
+		$versionEntity->setSize($fileInfoV2->getSize());
+		$versionEntity->setMimetype($this->mimeTypeLoader->getId($fileInfoV2->getMimetype()));
+		$versionEntity->setMetadata([]);
+		$this->versionsMapper->insert($versionEntity);
 
 		$oldVersions = \OCA\Files_Versions\Storage::getVersions(
 			self::TEST_VERSIONS_USER, '/sub/test.txt'