refactor: 两个 ZIP 打包 worker 合并为一个参数化引擎(604 行重复骨架 → 一份)

为什么:_batch_zip_worker(327 行 / 圈 78 / 嵌套 9)与 _podcast_zip_worker
(277 行 / 圈 84 / 嵌套 9)是两份平行实现——任务状态机、断线暂停、全局批量
名额、「逐条失败不计整批」、取消与异常的四路终态收敛、账本结算全部各写一遍,
连注释都逐字重复。上一轮修的 zip_valid 未绑定缺陷正是同型出现在这两处,
重复代码在持续制造同类 bug。

改法:新增 services/_zipcore.py 只保留共用骨架,差异收敛到 ZipJob 钩子:
prepare / download_one / write_entry / zip_filename 必实现,prefetch /
fail_label / done_msg / log_label / all_failed_msg / partial_msg / supplement /
on_delivered / cleanup 按需覆写。batch.py 与 podcast.py 只剩薄包装 + flavor 类。

线程模型(改动中最容易踩坏的一条,已写进模块 docstring):
ZipFile 不是线程安全的,且包内名去重是「读改写 used_arcnames」。因此把下载与
落包拆成两个钩子——只有 download_one 进线程池,zf.write 与去重一律在调用线程
串行。旧的音乐批量之所以安全,只是因为它的 zf.write 恰好写在主线程的
as_completed 循环里;照搬成「整个下载单元进池」会引入静默的 ZIP 损坏。

复杂度(同口径 AST 实测):
  两个 worker 合计   604 行 / 圈 162 / 嵌套 9,9
  → run_zip_job      167 行 / 圈  37 / 嵌套 4
    + _fill_zip       64 行 / 圈  13 / 嵌套 5
  全仓最大圈复杂度 84 → 37;>80 行函数 3 → 2(余下两个之一是本次未改动的
  _podcast_download_worker)。两文件改为三文件后总行数 930 → 1071,多出的
  主要是引擎 docstring——注释比代码贵,但它把「为什么这么拆」留给了下一个人。

逐条对齐的分叉(合并必须做的决定,此处全部列明):
1. 账本终态:播客全失败走自己的 except DownloadException → 落 'settled',音乐
   同语义事件落 'error'。统一为 'error'。全仓仅读取 'open'(对账/清理/计数),
   其余取值只作排障留痕 → 这是本次**唯一可观察的语义变更**,由新增用例固化。
2. 通用异常路径的「部分交付」:音乐侧会发布已打包的 ZIP 并挂 result,播客侧
   直接丢弃(却已按 delivered 结算 = 用户扣了费拿不到货)。统一采用音乐侧口径。
   该路径实际不可达(ZIP 写完后到发布前只有取消检查会抛错,已被取消分支接管)。
3. 全失败判定:音乐用 `ok == 0 and failed`、播客用早抛 DownloadException。
   统一为显式 `ok == 0`——好处是不再发布空 ZIP、不再把空批次标 done。
4. 早抛业务失败(无可下载条目 / 补扣失败):播客走 DownloadException 分支,
   音乐无此分支(落通用异常,用户看到的是「打包失败,请稍后重试」)。统一保留
   该分支,音乐侧同类失败从此也给出具体原因。
5. 错误终态写法:播客在手写任务字典(重复一遍「已终态不覆盖」,且漏掉 /F-E 的
   「worker 线程已死则按账本补结算」兜底),统一走 _mark_task_terminal。
6. 暂停检查点:统一采用音乐侧更严的一处——拿到 BATCH_ZIP_SEM 后再查一次
   pause/cancel。播客此前会多下完一集才停。
7. total 字段:音乐由提交侧设置、播客由 worker 设置,统一为 worker 设
   len(items)(对音乐是同一个值)。
8. 日志文案统一(tag / 名词提成类属性):'批量打包异常' 与 '批量ZIP任务异常'
   两种前缀收敛为一种;完成日志统一带上 task_id 与退回次数。

删除的不可达分支(可证明):
  两个 worker 在 zip_valid = True 之后各留了一段 `if not zip_valid:` 失败收敛
  (音乐 16 行 / 播客 18 行,含「ZIP 打包失败(可能磁盘空间不足)…」文案)。
  赋值紧跟其后、中间无任何语句可改写 zip_valid,故恒不可达——是原注释所称
  「复审 修正」留下的护栏。zip_valid 在异常路径仍有意义(决定 delivered 是否为 0)。

等价性证据(三道,互相独立):
1. AST 字面量多重集比对 + 逐条位置定位:数值 16 种一个不少。少掉的 8 处 `0`
   全部来自上面删的不可达分支与重复字面量合并(`delivered = … else 0` 5 处→3 处)。
2. 反向注入验证:把 git HEAD 的旧 worker 注入成 services.batch / services.podcast,
   用同一套 tests/test_zip_workers.py 去跑 → 第 1 批 9 例行为契约**全过**,证明
   这套测试忠实编码了旧行为、因而对新实现构成等价性验证;新增 4 例中只有账本
   终态那 1 例失败('settled' == 'error'),恰好把上面第 1 条对齐实证出来。该用例
   里「全部单集下载失败: …(已退回 2 次下载次数)」的文案断言在旧实现上也通过
   → 用户可见文案逐字保留。
3. 新增 4 例固化测试:同名歌声的音频/歌词/封面三条目共享序号后缀(F-C)、
   两边的全失败文案与账本终态、单集失败后任务目录不留中间文件与 .part 残片。

顺带修掉一个自己引入的缺陷:引擎化后播客侧的单条失败文案本会退化成整个条目
字典,字面量审计里 '(' ')' 各少 1 处把它逼了出来 → 补 ZipJob.fail_label 覆盖。

验证:ruff 全过 · pyright 0/0 · pytest 213 passed(209→213)· 生产 data/ 未被
触碰(全部用例走 tmp_path 沙箱)。
This commit is contained in:
snowy-yang committed 2026-09-26 19:25:06 +08:00
1 parent 65a81af94a
commit 5c8f941d19
4 files changed
+848 -579

No files matched your search

+135 -7
View File
@@ -1,13 +1,17 @@
# -*- coding: utf-8 -*-
"""两个 ZIP 打包 worker 的行为契约测试(沙箱,不触碰生产 data/)。
为什么:``_batch_zip_worker`` / ``_podcast_zip_worker`` 合计约 642 行此前**零测试覆盖**,
而两者高度重复——连 ``_wait_if_paused`` 那 16 行都逐字相同(上一次修的
``zip_valid`` 可能未绑定缺陷就是同型出现在这两处,重复代码在持续制造同类 bug)。
为什么:``_batch_zip_worker`` / ``_podcast_zip_worker`` 合并前合计约 642 行、
**零测试覆盖**,且两者高度重复——连 ``_wait_if_paused`` 那 16 行都逐字相同
(``zip_valid`` 未绑定那类缺陷就是同型出现在这两处)。抽取公共实现
(``services/_zipcore.py``)之前,先用**行为级**测试钉住可观察契约:任务终态、
ZIP 内容、账本守恒、暂停/恢复/取消语义。断言只依赖「调用 worker 后世界里发生了
什么」,不引用任何内部函数名——所以合并实现时本文件一行未改,它同时就是等价性
验证器。
抽公共实现之前,先用**行为级**测试钉住可观察契约:任务终态、ZIP 内容、
账本守恒、暂停/恢复/取消语义。断言只依赖「调用 worker 后世界里发生了什么」,
不引用任何内部函数名——这样重构时测试不必跟着改,才具备等价性验证能力。
末尾「合并后的契约固化」一节是**新增**的钉子:合并时逐条对齐了几处分叉
(详见各用例 docstring),这些用例在合并前必然失败——留着它们,那次对齐才不会
被后来者当成「本来就如此」而回退。
账本口径:一律用 ``reserve_and_open`` 真预扣(``ledger_open`` 只记账不扣款,
会让「退款」凭空给余额加次数,守恒断言就失去意义)。
@@ -15,6 +19,7 @@
import threading
import time
import zipfile
from dataclasses import replace
from pathlib import Path
from typing import Any
@@ -23,7 +28,7 @@ import pytest
import auth_system as auth
import core.db as core_db
from core.state import DOWNLOAD_TASKS, DOWNLOAD_TASKS_LOCK
from music_downloader import DownloadCanceled, DownloadResult, MusicInfo
from music_downloader import DownloadCanceled, DownloadException, DownloadResult, MusicInfo
from services import batch as batch_svc
from services import podcast as podcast_svc
from services.api_service import api_service
@@ -276,6 +281,7 @@ def test_batch_zip_pause_then_cancel(sandbox, monkeypatch):
assert _status(tid) == 'canceled', f"终态={_status(tid)} msg={task.get('msg')}"
assert task['result'] is None, '未交付不应发布 ZIP'
assert task['msg'] == '任务已取消(已退回 3 次下载次数)', task['msg']
assert _balance() == before, '取消后应全额退回预扣'
row = auth.ledger_get(tid)
assert row is not None and row['delivered'] == 0 and row['status'] != 'open'
@@ -454,3 +460,125 @@ def test_podcast_zip_all_fail_refunds(podcast_stub, monkeypatch):
assert _balance() == before, '全失败应全额退回'
row = auth.ledger_get(tid)
assert row is not None and row['delivered'] == 0
# ============ 合并后的契约固化(分叉点对齐 / 拆分风险点) ============
#
# 以下用例在「两个 worker 各写一份」的旧实现上**必然失败**——它们钉的是合并时
# 逐条对齐的结果,以及「下载」与「落包」拆成两个钩子后新增的边界。
class _SameNameDownloader(_FakeDownloader):
"""所有歌曲同名同歌手、且都带封面——用于验证包内三条目共享序号后缀。"""
def get_music_info(self, mid: Any, quality: str, cookies: Any) -> MusicInfo:
return replace(super().get_music_info(mid, quality, cookies),
name='同名歌', artists='同歌手',
pic_url='http://example.invalid/cover.jpg')
def test_batch_zip_dedups_three_arcnames_with_shared_suffix(sandbox, monkeypatch):
"""同名同歌手两首:音频/歌词/封面三条目**共享同一序号后缀**。
F-C 去重口径:同名同歌手但音源扩展不同(flac vs mp3)时,歌词/封面条目也会
互相覆盖。合并把「落包」拆成独立钩子后,去重读取的 ``used_arcnames`` 仍是
串行读改写,且三条目必须同步跳号(只给音频加序号就等于没修 F-C)。
"""
uid = _uid()
monkeypatch.setattr(api_service, 'downloader', _SameNameDownloader(sandbox['cache']))
monkeypatch.setattr(batch_svc, 'fetch_cover_bytes', lambda _u: b'\xff\xd8\xff\xe0')
tid, task = _register_batch(sandbox, [701, 702], reserved=2, user_id=uid)
th = _run(batch_svc._batch_zip_worker, tid)
th.join(timeout=30)
assert _status(tid) == 'done', f"终态={_status(tid)} msg={task.get('msg')}"
base = '同歌手 - 同名歌'
assert sorted(_zip_names(task['result']['zip_path'])) == sorted([
f'{base}.mp3', f'{base}.lrc', f'{base}.jpg',
f'{base} (2).mp3', f'{base} (2).lrc', f'{base} (2).jpg',
])
def test_batch_all_fail_message_and_ledger_status(sandbox, monkeypatch):
"""全失败:用户可见文案含条目数与退费说明;账本终态 error。
纯固化钉——合并前后的文案与账本终态在此路径上本就一致(对齐时以音乐侧为准)。
"""
uid = _uid()
monkeypatch.setattr(api_service, 'downloader',
_FakeDownloader(sandbox['cache'], fail_ids=(801, 802)))
tid, task = _register_batch(sandbox, [801, 802], reserved=2, user_id=uid)
th = _run(batch_svc._batch_zip_worker, tid)
th.join(timeout=30)
assert task['msg'] == '全部失败(2 首),已退回 2 次', task['msg']
row = auth.ledger_get(tid)
assert row is not None and row['status'] == 'error'
def test_podcast_all_fail_message_and_ledger_status(podcast_stub, monkeypatch):
"""全失败:文案含前 3 条失败明细与退费说明;**账本终态统一为 error**。
合并前这条路径走播客自己的 ``except DownloadException`` 处理器,账本落
``'settled'``,与音乐侧同一语义事件落的 ``'error'`` 不一致。整个代码库里
只有 ``'open'`` 被读取(对账、清理、计数),其余取值仅作排障留痕,故合并时
统一为语义更准确的 ``'error'``——本用例就是那次对齐的印记。
"""
uid = _uid()
before = _balance()
def _boom(prog, quality, cookies):
raise RuntimeError('上游不可用')
monkeypatch.setattr(podcast_svc, '_podcast_resolve_audio', _boom)
tid, task = _register_podcast(podcast_stub, [31, 32], reserved=2, user_id=uid)
th = _run(podcast_svc._podcast_zip_worker, tid)
th.join(timeout=30)
# 失败文案一律「单集名(异常)」——引擎的 fail_label 钩子,漏覆盖会退化成整个字典
assert task['msg'] == (
'全部单集下载失败: 单集31(上游不可用); 单集32(上游不可用)(已退回 2 次下载次数)'
), task['msg']
assert task['result'] is None, '不发布空包'
assert _balance() == before, '全失败应全额退回'
row = auth.ledger_get(tid)
assert row is not None and row['status'] == 'error' and row['delivered'] == 0
def test_podcast_cleans_temp_files_on_episode_failure(podcast_stub, monkeypatch):
"""单集下载中途失败:任务目录不留中间音频与 .part 残片。
合并把「下载」与「落包」拆成两个钩子后,临时文件生命周期跨越两者:失败路径
由 download_one 自己清,成功路径由 write_entry 的 finally 清(原实现在同一段
try/finally 里,拆开后若只保留一处就会漏清)。
"""
uid = _uid()
before = _balance()
def _flaky(url: str, target: Path, total_size: int, cancel_ev: Any,
task_id: str = '', progress_label: str = '') -> int:
if '/102.' in url:
target.write_bytes(b'\x00' * 64) # 正文残留
target.with_name(target.name + '.part').write_bytes(b'\x00' * 32) # 残片
raise DownloadException('上游断流')
target.write_bytes(b'\xff\xfb\x90\x00' + b'\x00' * 128)
return target.stat().st_size
monkeypatch.setattr(podcast_svc, '_podcast_stream_download', _flaky)
tid, task = _register_podcast(podcast_stub, [101, 102], reserved=2, user_id=uid)
th = _run(podcast_svc._podcast_zip_worker, tid)
th.join(timeout=30)
assert _status(tid) == 'done', f"终态={_status(tid)} msg={task.get('msg')}"
# done 是「已完成条目数」(含失败),不是成功数——与旧实现同口径
assert (task['done'], task['failed']) == (2, 1)
assert task['msg'] == '成功1 失败1 退回1次', task['msg']
assert sum(1 for n in _zip_names(task['result']['zip_path']) if n.endswith('.mp3')) == 1
assert _balance() == before - 1, '只对已入包的 1 集扣费'
work_dir = podcast_stub['downloads'] / 'tasks' / tid
leftover = sorted(p.name for p in work_dir.glob('podcast_zip_tmp_*'))
assert leftover == [], f'中间文件未清理: {leftover}'