Allow merge when unique subfolder already exists

gl00ten · gl00ten · commit 8964c2b0bd11 · 2026-08-06T00:30:46.000+01:00
On convert, if the destination unique folder is already present, ask for
confirmation then proceed with normal per-file renames (matching paths
may be overwritten; unrelated files stay). Do not delete the destination
directory. Layout is still set only after all renames succeed.
diff --git a/src/base/bittorrent/toplevelpayload.cpp b/src/base/bittorrent/toplevelpayload.cpp
@@ -30,6 +30,8 @@
 
 #include <algorithm>
 
+#include <QCoreApplication>
+
 #include "base/global.h"
 #include "base/utils/fs.h"
 
@@ -141,3 +143,46 @@ PathList BitTorrent::applyUniqueSubfolderLayout(PathList filePaths, const Torren
     Path::addRootFolder(filePaths, Path(folderName));
     return filePaths;
 }
+
+BitTorrent::UniqueSubfolderMigrationPlan BitTorrent::makeUniqueSubfolderMigrationPlan(
+        const PathList &currentPaths, const TorrentID &id, const QString &torrentName
+        , const Path &storageRoot)
+{
+    UniqueSubfolderMigrationPlan plan;
+    if (currentPaths.isEmpty())
+        return plan;
+
+    const PathList targetPaths = applyUniqueSubfolderLayout(currentPaths, id, torrentName);
+    if (targetPaths == currentPaths)
+        return plan;
+
+    if (storageRoot.isEmpty())
+    {
+        plan.blocked = true;
+        plan.blockReason = QCoreApplication::translate("BitTorrent", "Storage location is unknown.");
+        return plan;
+    }
+
+    const Path uniqueDir = Path::findRootFolder(targetPaths);
+    if (uniqueDir.isEmpty())
+    {
+        plan.blocked = true;
+        plan.blockReason = QCoreApplication::translate("BitTorrent", "Target unique subfolder is invalid.");
+        return plan;
+    }
+
+    for (int i = 0; i < targetPaths.size(); ++i)
+    {
+        if (targetPaths.at(i) != currentPaths.at(i))
+            plan.renames.append({.fileIndex = i, .to = targetPaths.at(i)});
+    }
+    if (plan.renames.isEmpty())
+        return plan;
+
+    // Existing destination: do not block. UI may confirm a merge (overwrite matching paths only).
+    const Path destAbs = storageRoot / uniqueDir;
+    if (destAbs.exists())
+        plan.existingUniqueFolder = destAbs;
+
+    return plan;
+}
diff --git a/src/base/bittorrent/toplevelpayload.h b/src/base/bittorrent/toplevelpayload.h
@@ -46,16 +46,32 @@ namespace BitTorrent
         Path to;
     };
 
-    // Preflight only. If blocked, show blockReason and do nothing.
+    // Preflight only — no disk changes.
+    // When existingUniqueFolder is set, the UI should confirm a merge before start.
     struct UniqueSubfolderMigrationPlan
     {
         QList<UniqueSubfolderRename> renames;
+
+        // Absolute path of an existing unique destination (empty if none).
+        Path existingUniqueFolder;
+
         bool blocked = false;
         QString blockReason;
 
         bool isEmpty() const
         {
             return renames.isEmpty() && !blocked;
         }
+
+        bool needsConfirmation() const
+        {
+            return !existingUniqueFolder.isEmpty();
+        }
     };
+
+    // Pure preflight used by TorrentImpl and unit tests.
+    // storageRoot: absolute save/download location for this torrent.
+    UniqueSubfolderMigrationPlan makeUniqueSubfolderMigrationPlan(
+            const PathList &currentPaths, const TorrentID &id, const QString &torrentName
+            , const Path &storageRoot);
 }
diff --git a/src/base/bittorrent/torrentimpl.cpp b/src/base/bittorrent/torrentimpl.cpp
@@ -2897,45 +2897,10 @@ nonstd::expected<void, QString> TorrentImpl::exportToFile(const Path &path) cons
 
 UniqueSubfolderMigrationPlan TorrentImpl::planUniqueSubfolderMigration() const
 {
-    UniqueSubfolderMigrationPlan plan;
     if (!hasMetadata())
-        return plan;
-
-    const PathList currentPaths = filePaths();
-    const PathList targetPaths = applyUniqueSubfolderLayout(currentPaths, id(), info().name());
-    if (targetPaths == currentPaths)
-        return plan;
-
-    const Path storageRoot = actualStorageLocation();
-    if (storageRoot.isEmpty())
-    {
-        plan.blocked = true;
-        plan.blockReason = tr("Storage location is unknown.");
-        return plan;
-    }
-
-    const Path uniqueDir = Path::findRootFolder(targetPaths);
-    if (uniqueDir.isEmpty())
-    {
-        plan.blocked = true;
-        plan.blockReason = tr("Target unique subfolder is invalid.");
-        return plan;
-    }
-
-    // Bare minimum: refuse if the unique folder already exists. No wipe / use-existing.
-    if ((storageRoot / uniqueDir).exists())
-    {
-        plan.blocked = true;
-        plan.blockReason = tr("Unique subfolder already exists: \"%1\".").arg(uniqueDir.toString());
-        return plan;
-    }
+        return {};
 
-    for (int i = 0; i < targetPaths.size(); ++i)
-    {
-        if (targetPaths.at(i) != currentPaths.at(i))
-            plan.renames.append({.fileIndex = i, .to = targetPaths.at(i)});
-    }
-    return plan;
+    return makeUniqueSubfolderMigrationPlan(filePaths(), id(), info().name(), actualStorageLocation());
 }
 
 void TorrentImpl::startUniqueSubfolderMigration(const UniqueSubfolderMigrationPlan &plan)
@@ -3000,7 +2965,15 @@ void TorrentImpl::finishUniqueSubfolderMigration()
     }
 
     forceRecheck();
-    emit uniqueSubfolderMigrationFinished(false, tr("Rename failed. Layout was not updated."));
+
+    QStringList failed;
+    failed.reserve(job.failedFileIndexes.size());
+    for (const int index : asConst(job.failedFileIndexes))
+        failed.append(QString::number(index));
+
+    emit uniqueSubfolderMigrationFinished(false
+            , tr("Rename failed for file index(es): %1. Layout was not updated. A recheck was started.")
+            .arg(failed.join(u", "_s)));
 }
 
 QFuture<QList<PeerInfo>> TorrentImpl::fetchPeerInfo() const
diff --git a/src/gui/optionsdialog.cpp b/src/gui/optionsdialog.cpp
@@ -587,7 +587,8 @@ void OptionsDialog::loadDownloadsTabOptions()
             + u"</p><p><b>" + tr("Create unique subfolder") + u"</b> - "
             + tr("Put content in a unique folder with a short hash suffix (e.g. Show a19f83c275d1). "
                  "Avoids name collisions; breaks path-based cross-seeding. "
-                 "Existing torrents stay as-is until converted via right-click.")
+                 "Existing torrents stay as-is until converted via right-click "
+                 "(if the folder already exists, you can confirm a merge that replaces matching files).")
             + u"</p><p><b>" + tr("Don't create subfolder") + u"</b> - "
             + tr("Put files directly in the save path.")
             + u"</p></body></html>");
diff --git a/src/gui/transferlistwidget.cpp b/src/gui/transferlistwidget.cpp
@@ -982,6 +982,24 @@ void TransferListWidget::createUniqueSubfolderForSelectedTorrents()
         if (plan.isEmpty())
             return;
 
+        // Merge if the unique folder already exists (overwrite matching paths only).
+        if (plan.needsConfirmation())
+        {
+            const QString text = tr(
+                    "Torrent: \"%1\"\n\n"
+                    "The unique subfolder already exists. Continuing will merge the torrent into it "
+                    "and replace files with matching paths. Other files will not be removed.")
+                    .arg(torrent->name());
+
+            const QMessageBox::StandardButton answer = QMessageBox::warning(this
+                    , tr("Create unique subfolder")
+                    , text
+                    , (QMessageBox::Ok | QMessageBox::Cancel)
+                    , QMessageBox::Cancel);
+            if (answer != QMessageBox::Ok)
+                return;
+        }
+
         connect(torrent, &BitTorrent::Torrent::uniqueSubfolderMigrationFinished, this
                 , [this](const bool success, const QString &message)
         {
@@ -1076,7 +1094,8 @@ void TransferListWidget::displayListMenu()
     auto *actionCreateUniqueSubfolder = new QAction(UIThemeManager::instance()->getIcon(u"edit-rename"_s)
             , tr("Create &unique subfolder"), listMenu);
     actionCreateUniqueSubfolder->setToolTip(tr(
-            "Move selected torrents into a unique folder (e.g. Show a19f83c275d1)."));
+            "Move selected torrents into a unique folder (e.g. Show a19f83c275d1). "
+            "If that folder already exists, matching files may be overwritten after confirmation."));
     connect(actionCreateUniqueSubfolder, &QAction::triggered, this, &TransferListWidget::createUniqueSubfolderForSelectedTorrents);
     auto *actionSequentialDownload = new TriStateAction(tr("Download in sequential order"), listMenu);
     connect(actionSequentialDownload, &QAction::triggered, this, &TransferListWidget::setSelectedTorrentsSequentialDownload);
diff --git a/test/testtoplevelpayload.cpp b/test/testtoplevelpayload.cpp
@@ -26,7 +26,10 @@
  * exception statement from your version.
  */
 
+#include <QDir>
+#include <QFile>
 #include <QObject>
+#include <QTemporaryDir>
 #include <QTest>
 
 #include "base/bittorrent/toplevelpayload.h"
@@ -150,6 +153,127 @@ private slots:
         const PathList out = applyUniqueSubfolderLayout(partial, sampleId, u"Show"_s);
         QCOMPARE(out, partial);
     }
+
+    void testPlanDestAbsent() const
+    {
+        QTemporaryDir tmp;
+        QVERIFY(tmp.isValid());
+        const Path storageRoot {tmp.path()};
+
+        const PathList current {Path(u"Show/ep.mkv"_s)};
+        const UniqueSubfolderMigrationPlan plan = makeUniqueSubfolderMigrationPlan(
+                current, sampleId, u"Show"_s, storageRoot);
+
+        QVERIFY(!plan.blocked);
+        QVERIFY(!plan.needsConfirmation());
+        QCOMPARE(plan.renames.size(), 1);
+        QCOMPARE(plan.renames.at(0).to, Path(u"Show a19f83c275d1/ep.mkv"_s));
+    }
+
+    void testPlanDestExistsNeedsConfirmation() const
+    {
+        QTemporaryDir tmp;
+        QVERIFY(tmp.isValid());
+        const Path storageRoot {tmp.path()};
+        const Path dest = storageRoot / Path(u"Show a19f83c275d1"_s);
+        QVERIFY(Utils::Fs::mkpath(dest));
+
+        const PathList current {Path(u"Show/ep.mkv"_s)};
+        const UniqueSubfolderMigrationPlan plan = makeUniqueSubfolderMigrationPlan(
+                current, sampleId, u"Show"_s, storageRoot);
+
+        QVERIFY(!plan.blocked);
+        QVERIFY(plan.needsConfirmation());
+        QCOMPARE(plan.existingUniqueFolder, dest);
+        QCOMPARE(plan.renames.size(), 1);
+        QCOMPARE(plan.renames.at(0).to, Path(u"Show a19f83c275d1/ep.mkv"_s));
+    }
+
+    void testPlanConflictingTorrentPathsStillRenamed() const
+    {
+        QTemporaryDir tmp;
+        QVERIFY(tmp.isValid());
+        const Path storageRoot {tmp.path()};
+        const Path dest = storageRoot / Path(u"Show a19f83c275d1"_s);
+        QVERIFY(Utils::Fs::mkpath(dest));
+        // Existing file that conflicts with a torrent path — plan still renames onto it (overwrite).
+        QFile conflict {(dest / Path(u"ep.mkv"_s)).data()};
+        QVERIFY(conflict.open(QIODevice::WriteOnly));
+        conflict.write("old");
+        conflict.close();
+
+        const PathList current {Path(u"Show/ep.mkv"_s), Path(u"Show/extra.mkv"_s)};
+        const UniqueSubfolderMigrationPlan plan = makeUniqueSubfolderMigrationPlan(
+                current, sampleId, u"Show"_s, storageRoot);
+
+        QVERIFY(plan.needsConfirmation());
+        QCOMPARE(plan.renames.size(), 2);
+        QCOMPARE(plan.renames.at(0).to, Path(u"Show a19f83c275d1/ep.mkv"_s));
+        QCOMPARE(plan.renames.at(1).to, Path(u"Show a19f83c275d1/extra.mkv"_s));
+    }
+
+    void testPlanUnrelatedDestFilesNotInRenameList() const
+    {
+        QTemporaryDir tmp;
+        QVERIFY(tmp.isValid());
+        const Path storageRoot {tmp.path()};
+        const Path dest = storageRoot / Path(u"Show a19f83c275d1"_s);
+        QVERIFY(Utils::Fs::mkpath(dest));
+        QFile unrelated {(dest / Path(u"notes.txt"_s)).data()};
+        QVERIFY(unrelated.open(QIODevice::WriteOnly));
+        unrelated.write("keep me");
+        unrelated.close();
+
+        const PathList current {Path(u"Show/ep.mkv"_s)};
+        const UniqueSubfolderMigrationPlan plan = makeUniqueSubfolderMigrationPlan(
+                current, sampleId, u"Show"_s, storageRoot);
+
+        QVERIFY(plan.needsConfirmation());
+        // Only torrent file paths are planned — unrelated notes.txt is never a rename target.
+        QCOMPARE(plan.renames.size(), 1);
+        QCOMPARE(plan.renames.at(0).to, Path(u"Show a19f83c275d1/ep.mkv"_s));
+        QVERIFY(QFile::exists((dest / Path(u"notes.txt"_s)).data()));
+    }
+
+    void testPlanEmptyWhenAlreadyUnique() const
+    {
+        QTemporaryDir tmp;
+        QVERIFY(tmp.isValid());
+        const Path storageRoot {tmp.path()};
+
+        const PathList current {Path(u"Show a19f83c275d1/ep.mkv"_s)};
+        const UniqueSubfolderMigrationPlan plan = makeUniqueSubfolderMigrationPlan(
+                current, sampleId, u"Show"_s, storageRoot);
+
+        QVERIFY(plan.isEmpty());
+        QVERIFY(!plan.needsConfirmation());
+    }
+
+    void testPlanBlockedWhenStorageUnknown() const
+    {
+        const PathList current {Path(u"Show/ep.mkv"_s)};
+        const UniqueSubfolderMigrationPlan plan = makeUniqueSubfolderMigrationPlan(
+                current, sampleId, u"Show"_s, {});
+
+        QVERIFY(plan.blocked);
+        QVERIFY(!plan.blockReason.isEmpty());
+        QVERIFY(plan.renames.isEmpty());
+    }
+
+    // Layout flag is only set by finishUniqueSubfolderMigration when all renames succeed
+    // (TorrentImpl). Plan itself never changes content layout — it only lists renames.
+    void testPlanDoesNotImplyLayoutChange() const
+    {
+        QTemporaryDir tmp;
+        QVERIFY(tmp.isValid());
+        const PathList current {Path(u"Show/ep.mkv"_s)};
+        const UniqueSubfolderMigrationPlan plan = makeUniqueSubfolderMigrationPlan(
+                current, sampleId, u"Show"_s, Path(tmp.path()));
+
+        QVERIFY(!plan.renames.isEmpty());
+        // No layout field on the plan — conversion is deferred until renames complete.
+        QVERIFY(!plan.blocked);
+    }
 };
 
 QTEST_APPLESS_MAIN(TestTopLevelPayload)
