[1009] 修复单个模板点击取消后重复点击会额外触发下载框的问题 (#3370)

Author: Yuki-Nagori

devel/1009.md

@@ -0,0 +1,160 @@
+# [1009] 修复单个模板点击取消后重复点击会额外触发下载框的问题
+
+## 1 相关文档
+- [dddd.md](dddd.md) - 任务文档模板
+
+## 2 任务相关的代码文件
+- `src/Mogan/TemplateCenter/template_api.hpp`
+- `src/Mogan/TemplateCenter/template_api.cpp`
+
+## 3 如何测试
+
+### 确定性测试（单元测试）
+```bash
+./bin/test_only template_api_test
+```
+
+### 非确定性测试（功能验证）
+
+1. 启动 Mogan，进入 Template Center
+2. 点击一个**未缓存**的模板卡片 → 弹出下载进度对话框
+3. 点击进度对话框的 **Cancel** 按钮 → 对话框应关闭
+4. **立即再次点击同一个模板** → 应重新弹出下载进度对话框
+5. 等待下载完成 → 模板应正常打开，**不应出现多个下载框**
+6. 重复步骤 2-5 多次（取消 3~5 次后再下载）→ 下载完成时应**只打开一次文档**
+
+### 3.2 边界场景
+
+- 取消下载后快速切换到其他模板再切回 → 不应有残留下载影响新模板
+- 取消下载后断网再点击 → 应正常提示下载失败，不出现残留状态
+
+## 4 如何提交
+
+提交前执行以下最少步骤：
+
+```bash
+# 编译验证
+xmake build
+```
+
+## 5 What
+
+修复模板下载过程中点击取消后，重新下载同一个模板会导致下载框和文档打开次数"每次递增"的 bug。
+
+1. **`TemplateAPI` 新增 `abortDownload()` 私有方法**
+   - 静默终止指定模板的网络请求，清理 `downloadReplies_`
+   - **不**发射任何信号，仅用于内部清理
+
+2. **`downloadTemplate()` 改调 `abortDownload()` 清理旧请求**
+   - 启动新下载前，用 `abortDownload` 终止同一模板的旧请求
+   - 避免旧请求的信号干扰新下载流程
+
+3. **`cancelDownload()` 清理后 emit `downloadFailed`**
+   - 用户点击 Cancel 时，终止请求并发射 `downloadFailed(templateId, "Download cancelled")`
+   - 确保 `TemplateManager::downloadTemplateSync()` 中的 `QEventLoop` 立即退出
+
+4. **`onDownloadFinished()` 补充 HTTP status code 检查**
+   - 原实现仅检查 `reply->error()`，当服务器返回 HTTP 404 但网络传输成功时，`QNetworkReply::error()` 可能仍为 `NoError`
+   - 增加对 `HttpStatusCodeAttribute` 的检查，将 `>= 400` 的 HTTP 响应统一视为下载失败
+
+## 6 Why
+
+### 6.1 现象
+
+用户在下载模板时点击取消，再次点击同一模板重新下载，下载完成后：
+- 弹出多个（随取消次数递增）下载完成提示
+- 同一文档被多次打开
+
+### 6.2 根因
+
+`TemplateAPI::cancelDownload()` 的实现顺序有问题：
+
+```cpp
+void TemplateAPI::cancelDownload(const QString& templateId) {
+  auto it = downloadReplies_.find(templateId);
+  if (it != downloadReplies_.end() && it.value()) {
+    disconnect(it.value(), nullptr, this, nullptr);  // 先断开信号
+    it.value()->abort();                                // 再 abort
+    it.value()->deleteLater();
+    downloadReplies_.erase(it);
+  }
+}
+```
+
+- `disconnect` 后调用 `abort()`，`QNetworkReply::finished` 信号发出时，`onDownloadFinished` 已收不到
+- `downloadFailed` 信号**永远不会被 emit**
+- `TemplateManager::downloadTemplateSync()` 中的 `QEventLoop` 收不到 `downloadCompleted` 或 `downloadFailed`，只能傻等 30 秒超时
+
+在这段超时期间：
+- 旧的调用 `downloadTemplateSync()` 的 UI 层对象仍然卡在 `loop.exec()` 中
+- 旧对象仍然连接着 `TemplateManager` 的 `downloadCompleted` / `downloadFailed` 信号
+- 用户点击"重新下载"后，新的下载完成时会 emit `downloadCompleted`
+- **新旧所有卡住的对象都会收到该信号**，导致模板被多次打开
+
+取消 N 次就会累积 N 个"幽灵对象"，下次成功下载时同时触发 N+1 次完成处理。
+
+## 7 How
+
+### 7.1 信号缺失问题
+
+`cancelDownload` 需要 emit `downloadFailed`，让 `downloadTemplateSync` 的 `QEventLoop` 立即退出。但 `downloadTemplate()` 内部在启动新下载前也会调用 `cancelDownload` 清理旧请求，此时 emit `downloadFailed` 会误伤新启动的下载。
+
+### 7.2 拆分为两个方法
+
+| 方法 | 触发场景 | 是否 emit 信号 |
+|------|----------|----------------|
+| `abortDownload` | `downloadTemplate` 内部启动新下载前 | 否 |
+| `cancelDownload` | 用户点击 Cancel 按钮 | 是，emit `downloadFailed` |
+
+```cpp
+void TemplateAPI::abortDownload(const QString& templateId) {
+  auto it = downloadReplies_.find(templateId);
+  if (it != downloadReplies_.end() && it.value()) {
+    disconnect(it.value(), nullptr, this, nullptr);
+    it.value()->abort();
+    it.value()->deleteLater();
+    downloadReplies_.erase(it);
+  }
+}
+
+void TemplateAPI::cancelDownload(const QString& templateId) {
+  auto it = downloadReplies_.find(templateId);
+  if (it != downloadReplies_.end() && it.value()) {
+    disconnect(it.value(), nullptr, this, nullptr);
+    it.value()->abort();
+    it.value()->deleteLater();
+    downloadReplies_.erase(it);
+    emit downloadFailed(templateId, tr("Download cancelled"));
+  }
+}
+```
+
+### 7.3 调用链路
+
+**用户取消下载：**
+> 注：`QTMTemplateOpener` 是本文档中的概念化表述，对应实际代码中调用
+> `TemplateManager::downloadTemplateSync()` 的 UI 层对象（如启动页的模板卡片打开逻辑）。
+
+```
+QProgressDialog::canceled
+  → UI 层对象::cancelDownload (templateId)
+    → TemplateManager::cancelDownload (templateId)
+      → TemplateAPI::cancelDownload (templateId)  [emit downloadFailed]
+        → TemplateManager::downloadFailed
+          → downloadTemplateSync::QEventLoop::quit [立即退出]
+```
+
+**重新下载（启动新请求前清理旧请求）：**
+```
+UI 层对象::openTemplate
+  → TemplateManager::downloadTemplateSync
+    → TemplateManager::downloadTemplate
+      → TemplateAPI::downloadTemplate
+        → TemplateAPI::abortDownload (templateId)  [静默清理，不 emit]
+```
+
+### 7.4 修复后行为
+
+- 用户点击取消 → `cancelDownload` emit `downloadFailed` → `QEventLoop` 立刻退出 → 调用 `downloadTemplateSync()` 的 UI 层对象立即析构，不残留
+- 重新下载同一个模板时，旧的对象已彻底销毁，不会"幽灵"接收新下载的完成信号
+- `downloadTemplate` 内部调用 `abortDownload` 清理旧请求，不会误 emit `downloadFailed` 导致新下载被中断

src/Mogan/TemplateCenter/template_api.cpp

@@ -35,14 +35,10 @@ TemplateAPI::setMetadataEtag (const QString& etag) {
 }
 
 TemplateAPI::~TemplateAPI () {
-  // Cancel all active downloads
-  for (auto reply : downloadReplies_) {
-    if (!reply) continue;
-    disconnect (reply, nullptr, this, nullptr);
-    reply->abort ();
-    reply->deleteLater ();
+  // Abort all active downloads (reuses abortDownload for consistency)
+  while (!downloadReplies_.isEmpty ()) {
+    abortDownload (downloadReplies_.begin ().key ());
   }
-  downloadReplies_.clear ();
 
   if (metadataReply_) {
     disconnect (metadataReply_, nullptr, this, nullptr);
@@ -95,8 +91,8 @@ TemplateAPI::downloadTemplate (const QString& templateId,
     return;
   }
 
-  // Cancel any existing download for this template
-  cancelDownload (templateId);
+  // Abort any existing download for this template (internal cleanup, no signal)
+  abortDownload (templateId);
 
   QNetworkRequest request{QUrl (downloadUrl)};
   setupRequestHeaders (request);
@@ -114,14 +110,28 @@ TemplateAPI::downloadTemplate (const QString& templateId,
            &TemplateAPI::onDownloadProgress);
 }
 
-void
-TemplateAPI::cancelDownload (const QString& templateId) {
+bool
+TemplateAPI::abortAndRemoveReply (const QString& templateId) {
   auto it= downloadReplies_.find (templateId);
   if (it != downloadReplies_.end () && it.value ()) {
     disconnect (it.value (), nullptr, this, nullptr);
     it.value ()->abort ();
     it.value ()->deleteLater ();
     downloadReplies_.erase (it);
+    return true;
+  }
+  return false;
+}
+
+void
+TemplateAPI::abortDownload (const QString& templateId) {
+  abortAndRemoveReply (templateId);
+}
+
+void
+TemplateAPI::cancelDownload (const QString& templateId) {
+  if (abortAndRemoveReply (templateId)) {
+    emit downloadFailed (templateId, tr ("Download cancelled"));
   }
 }
 
@@ -205,6 +215,16 @@ TemplateAPI::onDownloadFinished () {
     return;
   }
 
+  // Check HTTP status code (e.g., 404 may not trigger QNetworkReply error)
+  int httpStatus=
+      reply->attribute (QNetworkRequest::HttpStatusCodeAttribute).toInt ();
+  if (httpStatus >= 400) {
+    emit downloadFailed (templateId,
+                         tr ("Download failed: HTTP %1").arg (httpStatus));
+    reply->deleteLater ();
+    return;
+  }
+
   // Ensure target directory exists
   QDir dir (QFileInfo (targetPath).path ());
   if (!dir.exists ()) {

src/Mogan/TemplateCenter/template_api.hpp

@@ -49,6 +49,14 @@ class TemplateAPI : public QObject {
   void fetchMetadata ();
   void downloadTemplate (const QString& templateId, const QString& downloadUrl,
                          const QString& targetPath);
+
+  /**
+   * @brief 终止下载并发射 downloadFailed（用户点击取消）。
+   *
+   * 用户在进度对话框中点击 Cancel 时调用。
+   * 发射 downloadFailed(templateId, "Download cancelled")，
+   * 使 TemplateManager::downloadTemplateSync() 中的 QEventLoop 立即退出。
+   */
   void cancelDownload (const QString& templateId);
 
   // Metadata ETag for conditional requests
@@ -99,6 +107,18 @@ private slots:
   // Request management
   void setupRequestHeaders (QNetworkRequest& request);
 
+  /**
+   * @brief 静默终止下载，不发射任何信号（内部使用）。
+   *
+   * 在 downloadTemplate() 为同一 templateId 启动新请求前调用。
+   * 安静地终止旧的 QNetworkReply 并清理 downloadReplies_，
+   * 但不发射 downloadFailed，避免打断新下载流程。
+   */
+  void abortDownload (const QString& templateId);
+
+private:
+  bool abortAndRemoveReply (const QString& templateId);
+
 private:
   // API configuration
   QString apiBaseUrl_;

src/Mogan/TemplateCenter/template_manager.cpp

@@ -390,8 +390,8 @@ TemplateManager::downloadTemplateSync (const QString& templateId, int timeoutMs,
   connect (&timer, &QTimer::timeout, [&] () {
     if (finished) return;
     errorStr= tr ("Download timed out");
-    cancelDownload (templateId);
     finished= true;
+    cancelDownload (templateId);
     loop.quit ();
   });
   timer.start (timeoutMs);