Archived
refactor: 抽两个 zip worker 的重复实现,并补齐暂停/恢复路径的测试
为什么先测后改:_batch_zip_worker(347 行) 与 _podcast_zip_worker(295 行) 近乎 复制——连 _wait_if_paused 那 16 行都逐字相同(注释只差一行)。重复的代价是实打实 的:上一次修的 zip_valid「变量可能未绑定」缺陷就是**同型出现在这两处**。 但暂停/恢复是一条并发状态机,此前**零测试覆盖**,直接改不符合先测量后改动。 所以本 commit 分两步落地。 第一步(新增 tests/test_zip_workers.py,9 个用例)——行为级契约 断言只用「调用 worker 后世界里发生了什么」:任务终态、ZIP 内条目、账本守恒 (delivered / refunded)、暂停/恢复/取消后的余额。不引用任何内部函数名, 这样第二步重构时测试不必跟着改,才具备等价性验证能力。 - batch:正常完成 / 暂停→恢复 / 暂停→取消 / 全失败退费 / 循环中途取消 / 发布前一刻取消 - podcast:正常完成 / 暂停→恢复 / 全失败退费 - 脚手架一律用 reserve_and_open 真预扣(ledger_open 只记账不扣款, 会让「退款」凭空给余额加次数,守恒断言就失去意义) 写测试过程中暴露两处代码与注释不符,如实钉住而非粉饰: 1. **循环内取消并不会「交付已打包部分」**。zip_valid = True 位于 with zipfile 块**之后**,而 while 循环内的任何取消都以异常**穿过**该块 (zipfile.__exit__ 仍写出中央目录,但那行赋值不执行)→ zip_valid 恒为 False → delivered = ok_count if zip_valid else 0 恒为 0。源码注释所称的 「部分成功后取消照常交付(成果守恒)」只在发布前最后一个检查点可达, 已分别用两个用例把两条路径都钉住。 2. podcast 的 _wait_if_paused 与 batch 版本除 docstring 外完全一致。 第二步——抽取(只抽逐字相同、零语义差异的部分) - _taskcore._wait_if_paused(task, pause_ev, cancel_ev, done):done 写成参数 而非捕获闭包,使函数脱离两个 worker 各自的局部状态 - _taskcore._task_work_dir(task_id):任务专属目录约定(ZIP 与中间文件都落 tasks/<task_id>/,对根目录的缓存清扫免疫) - 两个 worker 各自的 16 行副本与 2 行 mkdir 删除,调用点改为显式传参 复杂度实测量(同口径,git HEAD 对比): 两个 worker 合计 行数 642 → 604(-38) 决策点 152 → 140(-12) 新增共享实现 _wait_if_paused 27 行/7 决策点 · _task_work_dir 11 行/1 **净行数基本持平(总量 -38 +38)**——重复代码抽取的收益从来不是行数变少, 而是**唯一真源**:以后改暂停语义只需改一处,不会再出现「同型缺陷出现在两地」。 验证:pytest 184 passed(175 → 184,+9)· ruff 0 · pyright 0 errors/0 warnings
This commit is contained in:
1 parent
7a70eef636
commit
1d86ec87ec
4 files changed
+511
-43
No files matched your search
@@ -5,6 +5,7 @@
|
||||
import threading
|
||||
import time
|
||||
import traceback
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import auth_system
|
||||
@@ -12,6 +13,9 @@ from core.state import (
|
||||
DOWNLOAD_TASKS,
|
||||
DOWNLOAD_TASKS_LOCK,
|
||||
)
|
||||
from music_downloader import (
|
||||
DownloadCanceled,
|
||||
)
|
||||
|
||||
from .api_service import (
|
||||
api_service,
|
||||
@@ -108,3 +112,45 @@ def _settle_batch_billing(task_id: str, ok_count: int, final_status: str = 'sett
|
||||
except Exception:
|
||||
pass
|
||||
return refund
|
||||
|
||||
|
||||
def _task_work_dir(task_id: str) -> Path:
|
||||
"""任务专属工作目录(交付 ZIP 与中间文件都落这里)。
|
||||
|
||||
原实现把临时包建在 downloads 根目录,会被缓存清扫线程按 mtime 回收
|
||||
(Linux 上已打开文件同样能被 unlink),排队超时的任务于是出现
|
||||
「扣费后领取 404」的悬空交付;任务目录从此对根目录清扫免疫。
|
||||
音乐歌单打包与播客整台打包共用本目录约定。
|
||||
"""
|
||||
path = api_service.downloads_path / 'tasks' / task_id
|
||||
path.mkdir(parents=True, exist_ok=True)
|
||||
return path
|
||||
|
||||
|
||||
def _wait_if_paused(task: dict[str, Any], pause_ev: threading.Event,
|
||||
cancel_ev: threading.Event, done: int) -> None:
|
||||
"""断线暂停闸门:置 paused 态并等待恢复或取消,恢复后回到 running。
|
||||
|
||||
两个打包 worker(音乐歌单 / 播客整台)共用同一套暂停语义——此前各自持有
|
||||
一份逐字相同的副本(连注释都只差一行),是「同型缺陷同时出现在两处」的温床。
|
||||
|
||||
调用点位于窗口循环顶部,等待期间**不持有** BATCH_ZIP_SEM:名额交还队列
|
||||
给下一位,这正是「断线不占坑」的实现方式。
|
||||
|
||||
``done`` 由调用方按当前进度传入(ok_count + len(failed))——写成参数而非
|
||||
捕获闭包,是为了让本函数脱离两个 worker 各自的局部状态。
|
||||
"""
|
||||
if not (pause_ev.is_set() or task.get('status') == 'paused'):
|
||||
return
|
||||
with DOWNLOAD_TASKS_LOCK:
|
||||
task['status'] = 'paused'
|
||||
task['done'] = done
|
||||
task['msg'] = '已暂停(重新打开页面自动恢复)'
|
||||
while not cancel_ev.is_set() and (pause_ev.is_set() or task.get('status') == 'paused'):
|
||||
time.sleep(1)
|
||||
if cancel_ev.is_set():
|
||||
raise DownloadCanceled("任务已取消")
|
||||
pause_ev.clear()
|
||||
with DOWNLOAD_TASKS_LOCK:
|
||||
task['status'] = 'running'
|
||||
task['msg'] = '继续下载...'
|
||||
+5
-23
@@ -27,6 +27,8 @@ from ._shared import (
|
||||
from ._taskcore import (
|
||||
_mark_task_terminal,
|
||||
_settle_batch_billing,
|
||||
_task_work_dir,
|
||||
_wait_if_paused,
|
||||
)
|
||||
from .api_service import (
|
||||
api_service,
|
||||
@@ -110,30 +112,10 @@ def _batch_zip_worker(task_id: str) -> None:
|
||||
DOWNLOAD_TASKS[task_id]['done'] = ok_count + len(failed)
|
||||
DOWNLOAD_TASKS[task_id]['failed'] = len(failed)
|
||||
|
||||
def _wait_if_paused() -> None:
|
||||
"""暂停时等待恢复或取消(不持有 BATCH_ZIP_SEM)"""
|
||||
if not (pause_ev.is_set() or task['status'] == 'paused'):
|
||||
return
|
||||
with DOWNLOAD_TASKS_LOCK:
|
||||
task['status'] = 'paused'
|
||||
task['done'] = ok_count + len(failed)
|
||||
task['msg'] = '已暂停(重新打开页面自动恢复)'
|
||||
while not cancel_ev.is_set() and (pause_ev.is_set() or task['status'] == 'paused'):
|
||||
time.sleep(1)
|
||||
if cancel_ev.is_set():
|
||||
raise DownloadCanceled("任务已取消")
|
||||
pause_ev.clear()
|
||||
with DOWNLOAD_TASKS_LOCK:
|
||||
task['status'] = 'running'
|
||||
task['msg'] = '继续下载...'
|
||||
|
||||
# ZIP 构建放 downloads 卷(原实现落系统 %TEMP%——500 首无损可达 GB 级,
|
||||
# 小系统盘一次大单即塞爆;单曲/播客早已指定 dir,唯批量漏了)
|
||||
# F-A:临时包建在任务专属目录 tasks/<task_id>/——原放根目录会被
|
||||
# 缓存清扫线程按 mtime 回收(Linux 上已打开文件也能被 unlink,Docker 部署
|
||||
# 排队超 35 分钟即出现"扣费后领取 404"的悬空交付),根目录从此碰不到交付物
|
||||
_task_dir_for_zip = api_service.downloads_path / 'tasks' / task_id
|
||||
_task_dir_for_zip.mkdir(parents=True, exist_ok=True)
|
||||
# 且建在任务专属目录(见 _task_work_dir):根目录会被缓存清扫按 mtime 回收
|
||||
_task_dir_for_zip = _task_work_dir(task_id)
|
||||
# 中央目录完整写出后(下方)才置 True,否则一律按失败全额退费
|
||||
with _tempfile.NamedTemporaryFile(prefix='batch_zip_', suffix='.zip',
|
||||
dir=str(_task_dir_for_zip),
|
||||
@@ -149,7 +131,7 @@ def _batch_zip_worker(task_id: str) -> None:
|
||||
if cancel_ev.is_set():
|
||||
raise DownloadCanceled("任务已取消")
|
||||
# 断线暂停:释放名额等待恢复/取消,恢复后重新进入排队
|
||||
_wait_if_paused()
|
||||
_wait_if_paused(task, pause_ev, cancel_ev, ok_count + len(failed))
|
||||
# 每个窗口持有 BATCH_ZIP_SEM(同一时刻全局最多 3 个批量任务在下载)
|
||||
# 等待名额期间可被取消,不再无限阻塞线程与退款
|
||||
if not BATCH_ZIP_SEM.acquire_cancellable(cancel_ev):
|
||||
|
||||
+4
-20
@@ -33,6 +33,8 @@ from ._shared import (
|
||||
from ._taskcore import (
|
||||
_mark_task_terminal,
|
||||
_settle_batch_billing,
|
||||
_task_work_dir,
|
||||
_wait_if_paused,
|
||||
)
|
||||
from .api_service import (
|
||||
api_service,
|
||||
@@ -359,23 +361,6 @@ def _podcast_zip_worker(task_id: str) -> None:
|
||||
except Exception as e:
|
||||
raise DownloadException(f"补扣下载次数异常: {e}") from e
|
||||
|
||||
def _wait_if_paused() -> None:
|
||||
"""暂停时等待恢复或取消"""
|
||||
if not (pause_ev.is_set() or task['status'] == 'paused'):
|
||||
return
|
||||
with DOWNLOAD_TASKS_LOCK:
|
||||
task['status'] = 'paused'
|
||||
task['done'] = ok_count + len(failed)
|
||||
task['msg'] = '已暂停(重新打开页面自动恢复)'
|
||||
while not cancel_ev.is_set() and (pause_ev.is_set() or task['status'] == 'paused'):
|
||||
time.sleep(1)
|
||||
if cancel_ev.is_set():
|
||||
raise DownloadCanceled("任务已取消")
|
||||
pause_ev.clear()
|
||||
with DOWNLOAD_TASKS_LOCK:
|
||||
task['status'] = 'running'
|
||||
task['msg'] = '继续下载...'
|
||||
|
||||
def _update_progress(msg: str = '') -> None:
|
||||
with DOWNLOAD_TASKS_LOCK:
|
||||
t = DOWNLOAD_TASKS[task_id]
|
||||
@@ -389,8 +374,7 @@ def _podcast_zip_worker(task_id: str) -> None:
|
||||
# 未 valid 一律不发布、不按 ok_count 结算
|
||||
# 同音乐批量——ZIP 构建放 downloads 卷,不落系统 %TEMP%
|
||||
# 与音乐批量打包同口径:建在任务专属目录 tasks/<task_id>/,根目录缓存清扫不再触及交付物
|
||||
_podcast_task_dir = api_service.downloads_path / 'tasks' / task_id
|
||||
_podcast_task_dir.mkdir(parents=True, exist_ok=True)
|
||||
_podcast_task_dir = _task_work_dir(task_id)
|
||||
with _tempfile.NamedTemporaryFile(prefix='podcast_batch_', suffix='.zip',
|
||||
dir=str(_podcast_task_dir),
|
||||
delete=False) as zip_tmp:
|
||||
@@ -400,7 +384,7 @@ def _podcast_zip_worker(task_id: str) -> None:
|
||||
for idx, ep in enumerate(episodes, 1):
|
||||
if cancel_ev.is_set():
|
||||
raise DownloadCanceled("任务已取消")
|
||||
_wait_if_paused()
|
||||
_wait_if_paused(task, pause_ev, cancel_ev, ok_count + len(failed))
|
||||
# 并发对齐音乐批量:播客打包同样占用全局批量名额(BATCH_ZIP_SEM,全局 3),
|
||||
# 逐集 acquire/release——暂停等待/取消时不长期占名额;音频本身仍受
|
||||
# 全局下载信号量约束。原实现不受批量并发控制,多用户同时打包会无限并发
|
||||
|
||||
Reference in new issue
Block a user