From f8eb32144f5d5007de780fe6b19a46e1b150e369 Mon Sep 17 00:00:00 2001 From: xiaoxia Date: Mon, 24 Aug 2026 10:18:13 +0800 Subject: [PATCH 1/6] =?UTF-8?q?fix:=203=E4=B8=AAbug=E4=BF=AE=E5=A4=8D=20-?= =?UTF-8?q?=20flush/append=5Flog/plan=5Fid=E5=85=9C=E5=BA=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug 1 (P0): edit_plan_service.replace_all_clips_transactional - db.add(model)后加db.flush()，让新建clip写入事务 - 后续查询status=='pending'才能找到新记录并标记ready - 根因：未flush的记录对SQLAlchemy query不可见 Bug 2 (P1): generation.py 失败日志append_log参数重复 - 原代码: append_log('任务失败', str(error), stage='render') - '任务失败'作为第一个位置参数已赋给stage，又传stage='render' → TypeError - 修复: append_log('render', str(error), level='ERROR', error_type=...) - 排查其他调用点无同样问题 Bug 3 (P0): 正式生成API缺少plan_id兜底关联 - 预览API有兜底：前端未传source_edit_plan_id时通过template_id+user_id查找plan - 正式生成API没有此逻辑，导致Worker拿不到plan_id无法走DB渲染路径 - 修复：任务创建并入队后，若source_edit_plan_id为空且template_id存在， 通过SQLAlchemyEditPlanRepository.list_by_template查找该用户最新的plan关联 新增7个回归测试，127个相关测试全过 --- apps/api/app/api/routes/generation_tasks.py | 27 +++ apps/api/app/services/edit_plan_service.py | 3 + apps/worker/worker_app/tasks/generation.py | 5 +- tests/unit/test_three_bugs_fix.py | 183 ++++++++++++++++++++ 4 files changed, 215 insertions(+), 3 deletions(-) create mode 100644 tests/unit/test_three_bugs_fix.py diff --git a/apps/api/app/api/routes/generation_tasks.py b/apps/api/app/api/routes/generation_tasks.py index 450d43f2f..7880f44c7 100755 --- a/apps/api/app/api/routes/generation_tasks.py +++ b/apps/api/app/api/routes/generation_tasks.py @@ -377,6 +377,33 @@ def create_generation_task( title_config=request.title_config, db=db, ) + + # 兜底关联编辑计划：前端未传 source_edit_plan_id 时， + # 通过 template_id + user_id 查找该用户的 plan + if not task.source_edit_plan_id and request.template_id: + try: + from packages.adapters.sqlalchemy_impl.edit_plan_repository import ( + SQLAlchemyEditPlanRepository, + ) + + _plan_repo = SQLAlchemyEditPlanRepository(db) + _plans = _plan_repo.list_by_template(request.template_id, limit=20) + for _p in _plans: + if (_p.created_by_user_id or "") == user_id: + task.source_edit_plan_id = _p.id + generation_task_repository.update(task) + logger.info( + "[生成任务] 自动关联编辑计划: task_id=%s plan_id=%s", + task.id, + _p.id, + ) + break + except Exception: + logger.warning( + "[生成任务] 查找关联编辑计划失败（不影响主流程）: task_id=%s", + task.id, + exc_info=True, + ) else: failed_tasks.append(task) except UserPendingLimitExceeded as _e: diff --git a/apps/api/app/services/edit_plan_service.py b/apps/api/app/services/edit_plan_service.py index 41f0afff6..926a7e47f 100755 --- a/apps/api/app/services/edit_plan_service.py +++ b/apps/api/app/services/edit_plan_service.py @@ -423,6 +423,9 @@ class EditPlanService: ) db.add(model) + # flush 让新建 clip 写入当前事务（未 commit），后续查询才能找到它们 + db.flush() + # 3. 标记有 asset_id 的 clips 为 ready（不 commit） pending_with_asset = ( db.query(EditPlanClipModel) diff --git a/apps/worker/worker_app/tasks/generation.py b/apps/worker/worker_app/tasks/generation.py index a089726af..1ca355a1f 100644 --- a/apps/worker/worker_app/tasks/generation.py +++ b/apps/worker/worker_app/tasks/generation.py @@ -1919,12 +1919,11 @@ def generate_video(self, task_id: str) -> dict: _repo = SQLAlchemyGenerationTaskRepository(_session) gen_task = _repo.get(task_id) if gen_task: - gen_task.append_log( # type: ignore[misc] - "任务失败", + gen_task.append_log( + "render", str(error), level="ERROR", error_type=type(error).__name__, - stage="render", ) _flush_logs(task_id, gen_task) finally: diff --git a/tests/unit/test_three_bugs_fix.py b/tests/unit/test_three_bugs_fix.py new file mode 100644 index 000000000..698d92a4b --- /dev/null +++ b/tests/unit/test_three_bugs_fix.py @@ -0,0 +1,183 @@ +"""Regression tests for 3 bug fixes: flush, append_log, plan_id fallback.""" + +from __future__ import annotations + +import json +import os +from unittest.mock import MagicMock, patch + +import pytest + +os.environ.setdefault("JWT_SECRET_KEY", "unit-test-secret-key-for-testing") + + +# ── Bug 1: db.flush() before pending query ───────────────────────────────── + +class TestReplaceAllClipsFlush: + """replace_all_clips_transactional must flush before querying pending clips.""" + + def _make_svc_and_db(self, pending_results): + """Helper: create service + db mock. pending_results = list returned by pending query.""" + from app.services.edit_plan_service import EditPlanService + + db = MagicMock() + # Delete query + delete_query = MagicMock() + delete_query.filter.return_value.delete.return_value = 0 + # Pending query: single .filter() with multiple conditions + ready_query = MagicMock() + ready_query.filter.return_value.all.return_value = pending_results + db.query.side_effect = [delete_query, ready_query] + return db, EditPlanService + + def _setup_clip_mocks(self, mock_clip_cls, mock_model_cls, asset_id="asset-1"): + mock_entity = MagicMock() + mock_entity.id = "clip-1" + mock_entity.plan_id = "plan-1" + mock_entity.clip_type = "main" + mock_entity.order = 0 + mock_entity.asset_id = asset_id + mock_entity.text_content = "" + mock_entity.start_time = 0.0 + mock_entity.duration = 3.0 + mock_entity.transition_effect = "cut" + mock_entity.transition_duration = 0.0 + mock_entity.playback_speed = 1.0 + mock_entity.status.value = "pending" + mock_entity.config = {} + mock_clip_cls.create.return_value = mock_entity + mock_model_cls.return_value = MagicMock() + return mock_entity + + @patch("packages.adapters.sqlalchemy_impl.models.EditPlanClipModel") + @patch("app.services.edit_plan_service.EditPlanClip") + def test_flush_called_between_add_and_query(self, mock_clip_cls, mock_model_cls): + """db.flush() must be called after db.add() and before the pending query.""" + self._setup_clip_mocks(mock_clip_cls, mock_model_cls) + db, SvcClass = self._make_svc_and_db([]) + + clip_repo = MagicMock() + clip_repo.session = db + svc = SvcClass.__new__(SvcClass) + svc._clip_repo = clip_repo + + svc.replace_all_clips_transactional( + "plan-1", + [{"asset_id": "asset-1", "start_time": 0.0, "duration": 3.0, "order": 0}], + ) + + db.flush.assert_called_once() + # Verify ordering: add → flush → query → commit + method_names = [c[0] for c in db.method_calls] + add_idx = method_names.index("add") + flush_idx = method_names.index("flush") + commit_idx = method_names.index("commit") + assert add_idx < flush_idx < commit_idx + + @patch("packages.adapters.sqlalchemy_impl.models.EditPlanClipModel") + @patch("app.services.edit_plan_service.EditPlanClip") + def test_flush_marks_new_clips_ready(self, mock_clip_cls, mock_model_cls): + """After flush, new clips with asset_id are found and marked ready.""" + self._setup_clip_mocks(mock_clip_cls, mock_model_cls) + + # Use a plain object so we can verify attribute mutation + class FakeClip: + status = "pending" + + pending_clip = FakeClip() + db, SvcClass = self._make_svc_and_db([pending_clip]) + + clip_repo = MagicMock() + clip_repo.session = db + svc = SvcClass.__new__(SvcClass) + svc._clip_repo = clip_repo + + svc.replace_all_clips_transactional( + "plan-1", + [{"asset_id": "asset-1", "start_time": 0.0, "duration": 3.0, "order": 0}], + ) + + assert pending_clip.status == "ready" + + +# ── Bug 2: append_log no TypeError ───────────────────────────────────────── + +class TestAppendLogNoConflict: + """append_log must not receive duplicate 'stage' parameter.""" + + def _make_task(self): + from packages.domain.generation_task import GenerationTask + return GenerationTask.create( + project_id="proj-1", + asset_library_id="lib-1", + strategy_id="one_take", + template_id="tpl-1", + asset_ids=["a1"], + created_by_user_id="user-1", + ) + + def test_append_log_with_stage_as_first_positional(self): + """append_log(stage, message, ...) works correctly.""" + task = self._make_task() + task.append_log("render", "some error", level="ERROR", error_type="RuntimeError") + + entries = json.loads(task.logs) + assert len(entries) == 1 + assert entries[0]["stage"] == "render" + assert entries[0]["message"] == "some error" + assert entries[0]["level"] == "ERROR" + assert entries[0]["error_type"] == "RuntimeError" + + def test_duplicate_stage_raises_type_error(self): + """Sanity check: passing stage both positionally and as kwarg raises TypeError.""" + task = self._make_task() + with pytest.raises(TypeError): + task.append_log( + "任务失败", # positional → stage + "some error", + level="ERROR", + stage="render", # duplicate → TypeError + ) + + +# ── Bug 3: plan_id fallback in create_generation_task ────────────────────── + +class TestPlanIdFallback: + """Formal generation API should fallback to find plan by template_id + user_id.""" + + def test_fallback_code_present_in_source(self): + """Verify the fallback logic is present in the generation_tasks module.""" + import inspect + from apps.api.app.api.routes import generation_tasks + + source = inspect.getsource(generation_tasks.create_generation_task) + assert "SQLAlchemyEditPlanRepository" in source + assert "list_by_template" in source + assert "兜底关联编辑计划" in source + assert "自动关联编辑计划" in source + + def test_fallback_only_runs_when_source_edit_plan_id_empty(self): + """Verify the condition checks for empty source_edit_plan_id.""" + import inspect + from apps.api.app.api.routes import generation_tasks + + source = inspect.getsource(generation_tasks.create_generation_task) + assert "not task.source_edit_plan_id and request.template_id" in source + + def test_list_by_template_method_exists(self): + """Verify SQLAlchemyEditPlanRepository.list_by_template is callable.""" + from packages.adapters.sqlalchemy_impl.edit_plan_repository import ( + SQLAlchemyEditPlanRepository, + ) + + mock_db = MagicMock() + mock_session = MagicMock() + mock_db.query.return_value = mock_session + mock_session.filter.return_value = mock_session + mock_session.order_by.return_value = mock_session + mock_session.offset.return_value = mock_session + mock_session.limit.return_value.all.return_value = [] + + repo = SQLAlchemyEditPlanRepository(mock_db) + result = repo.list_by_template("tpl-1", limit=20) + assert result == [] -- 2.54.0 From b348507d2513714ec17c5bbac23e961b27e17e33 Mon Sep 17 00:00:00 2001 From: CI Bot Date: Mon, 24 Aug 2026 02:25:53 +0000 Subject: [PATCH 2/6] style: auto-format with black + isort + prettier [skip ci-format-check] --- tests/unit/test_three_bugs_fix.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/unit/test_three_bugs_fix.py b/tests/unit/test_three_bugs_fix.py index 698d92a4b..f74846ae2 100644 --- a/tests/unit/test_three_bugs_fix.py +++ b/tests/unit/test_three_bugs_fix.py @@ -13,6 +13,7 @@ os.environ.setdefault("JWT_SECRET_KEY", "unit-test-secret-key-for-testing") # ── Bug 1: db.flush() before pending query ───────────────────────────────── + class TestReplaceAllClipsFlush: """replace_all_clips_transactional must flush before querying pending clips.""" @@ -102,11 +103,13 @@ class TestReplaceAllClipsFlush: # ── Bug 2: append_log no TypeError ───────────────────────────────────────── + class TestAppendLogNoConflict: """append_log must not receive duplicate 'stage' parameter.""" def _make_task(self): from packages.domain.generation_task import GenerationTask + return GenerationTask.create( project_id="proj-1", asset_library_id="lib-1", @@ -142,12 +145,14 @@ class TestAppendLogNoConflict: # ── Bug 3: plan_id fallback in create_generation_task ────────────────────── + class TestPlanIdFallback: """Formal generation API should fallback to find plan by template_id + user_id.""" def test_fallback_code_present_in_source(self): """Verify the fallback logic is present in the generation_tasks module.""" import inspect + from apps.api.app.api.routes import generation_tasks source = inspect.getsource(generation_tasks.create_generation_task) @@ -159,6 +164,7 @@ class TestPlanIdFallback: def test_fallback_only_runs_when_source_edit_plan_id_empty(self): """Verify the condition checks for empty source_edit_plan_id.""" import inspect + from apps.api.app.api.routes import generation_tasks source = inspect.getsource(generation_tasks.create_generation_task) -- 2.54.0 From 9bfe0b996377edebb03de1298943ea70eee59182 Mon Sep 17 00:00:00 2001 From: xiaoxia Date: Mon, 24 Aug 2026 10:56:52 +0800 Subject: [PATCH 3/6] =?UTF-8?q?test:=20=E8=A1=A5=E5=85=85plan=5Fid?= =?UTF-8?q?=E5=85=9C=E5=BA=95=E9=80=BB=E8=BE=91=E6=89=A7=E8=A1=8C=E6=B5=8B?= =?UTF-8?q?=E8=AF=95=EF=BC=8Cdiff=E8=A6=86=E7=9B=96=E7=8E=87100%?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 新增3个端到端测试覆盖generation_tasks.py的兜底关联逻辑： - test_fallback_sets_plan_id_when_empty: 验证空plan_id时自动查找并关联 - test_no_fallback_when_plan_id_already_set: 验证已有plan_id时不触发兜底 - test_fallback_handles_exception_gracefully: 验证查找异常不阻塞主流程 总计10个回归测试，diff覆盖率从7%提升到100%（14/14行全覆盖） --- coverage.xml | 19875 ++++++++++++++++++++++++++++ tests/unit/test_three_bugs_fix.py | 226 + 2 files changed, 20101 insertions(+) create mode 100644 coverage.xml diff --git a/coverage.xml b/coverage.xml new file mode 100644 index 000000000..6c17b9ce8 --- /dev/null +++ b/coverage.xml @@ -0,0 +1,19875 @@ + + + + + + /opt/xiaoxia-saas/apps/api/app + /opt/xiaoxia-saas/packages + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/unit/test_three_bugs_fix.py b/tests/unit/test_three_bugs_fix.py index f74846ae2..19e9531ac 100644 --- a/tests/unit/test_three_bugs_fix.py +++ b/tests/unit/test_three_bugs_fix.py @@ -187,3 +187,229 @@ class TestPlanIdFallback: repo = SQLAlchemyEditPlanRepository(mock_db) result = repo.list_by_template("tpl-1", limit=20) assert result == [] + + +class TestPlanIdFallbackExecution: + """Test that the fallback logic actually executes when source_edit_plan_id is empty.""" + + @staticmethod + def _make_mock_task(source_edit_plan_id=""): + t = MagicMock() + t.id = "task-1" + t.project_id = "proj-1" + t.asset_library_id = "" + t.strategy_id = "one_take" + t.voice_library_id = "" + t.template_id = "tpl-1" + t.asset_ids = [] + t.title_ids = [] + t.voice_ids = [] + t.source_edit_plan_id = source_edit_plan_id + t.asset_select_mode = "manual" + t.batch_id = "" + t.video_title = "" + t.resolution = "" + t.bgm_config = None + t.is_preview = False + t.source_task_id = "" + t.output_width = 1280 + t.output_height = 720 + t.cover_url = "" + t.custom_title = "" + t.title_config = {} + t.logs = "[]" + t.status = "pending" + t.progress = 0.0 + t.error_message = "" + t.error_info = None + t.created_at = "2026-01-01T00:00:00Z" + t.updated_at = "2026-01-01T00:00:00Z" + t.started_at = None + t.completed_at = None + t.created_by_user_id = "user-1" + t.auto_retry_enabled = False + t.auto_retry_max = 0 + t.auto_retry_count = 0 + return t + + @staticmethod + def _make_request(source_edit_plan_id="", template_id="tpl-1"): + req = MagicMock() + req.template_id = template_id + req.source_edit_plan_id = source_edit_plan_id + req.asset_ids = [] + req.asset_select_mode = "manual" + req.asset_select_count = 0 + req.voice_library_id = "" + req.title_ids = [] + req.voice_ids = [] + req.strategy_id = "one_take" + req.count = 1 + req.video_title = "" + req.resolution = "" + req.bgm_config = None + req.auto_retry_enabled = False + req.auto_retry_max = 0 + req.is_preview = False + req.source_task_id = "" + req.output_width = 0 + req.output_height = 0 + req.cover_url = "" + req.custom_title = "" + req.title_config = {} + req.project_id = None + req.asset_library_id = None + return req + + @patch("apps.api.app.api.routes.generation_tasks._to_generation_task_response") + @patch("apps.api.app.api.routes.generation_tasks.safe_enqueue_generation_task") + @patch("apps.api.app.api.routes.generation_tasks.CreateGenerationTaskUseCase") + @patch("apps.api.app.api.routes.generation_tasks._resolve_project_and_library") + def test_fallback_sets_plan_id_when_empty(self, mock_resolve, mock_uc_cls, mock_enqueue, mock_resp_fn): + """When source_edit_plan_id is empty, fallback finds plan and sets it.""" + from apps.api.app.api.routes.generation_tasks import create_generation_task + + mock_resolve.return_value = ("proj-1", None) + mock_task = self._make_mock_task(source_edit_plan_id="") + mock_uc_cls.return_value.execute.return_value = mock_task + mock_enqueue.return_value = True + from app.schemas.generation_task import GenerationTaskResponse + mock_resp_fn.return_value = GenerationTaskResponse( + id="task-1", project_id="proj-1", asset_library_id="", + strategy_id="one_take", voice_library_id="", template_id="tpl-1", + asset_ids=[], title_ids=[], voice_ids=[], source_edit_plan_id="", + asset_select_mode="", batch_id="", video_title="", resolution="", + bgm_config={}, is_preview=False, source_task_id="", + output_width=1280, output_height=720, cover_url="", custom_title="", + title_config={}, logs="[]", status="pending", progress=0.0, + error_message="", created_at="2026-01-01T00:00:00Z", + updated_at="2026-01-01T00:00:00Z", started_at=None, completed_at=None, + created_by_user_id="user-1", auto_retry_enabled=False, + auto_retry_max=0, auto_retry_count=0, result_count=0, + ) + + mock_plan = MagicMock() + mock_plan.id = "plan-found-123" + mock_plan.created_by_user_id = "user-1" + + mock_plan_repo_cls = MagicMock() + mock_plan_repo_instance = MagicMock() + mock_plan_repo_instance.list_by_template.return_value = [mock_plan] + mock_plan_repo_cls.return_value = mock_plan_repo_instance + + mock_gen_repo = MagicMock() + mock_gen_repo.count_pending_by_user.return_value = 0 + mock_gen_repo.count_pending_total.return_value = 0 + + with patch( + "packages.adapters.sqlalchemy_impl.edit_plan_repository.SQLAlchemyEditPlanRepository", + mock_plan_repo_cls, + ): + create_generation_task( + request=self._make_request(source_edit_plan_id="", template_id="tpl-1"), + authenticated_user=MagicMock(user=MagicMock(id="user-1")), + generation_task_repository=mock_gen_repo, + project_repository=MagicMock(), + asset_library_repository=MagicMock(), + asset_repository=MagicMock(), + db=MagicMock(), + ) + + assert mock_task.source_edit_plan_id == "plan-found-123" + mock_gen_repo.update.assert_called_once_with(mock_task) + + @patch("apps.api.app.api.routes.generation_tasks._to_generation_task_response") + @patch("apps.api.app.api.routes.generation_tasks.safe_enqueue_generation_task") + @patch("apps.api.app.api.routes.generation_tasks.CreateGenerationTaskUseCase") + @patch("apps.api.app.api.routes.generation_tasks._resolve_project_and_library") + def test_no_fallback_when_plan_id_already_set(self, mock_resolve, mock_uc_cls, mock_enqueue, mock_resp_fn): + """When source_edit_plan_id is already set, fallback should NOT run.""" + from apps.api.app.api.routes.generation_tasks import create_generation_task + + mock_resolve.return_value = ("proj-1", None) + mock_task = self._make_mock_task(source_edit_plan_id="plan-already-set") + mock_uc_cls.return_value.execute.return_value = mock_task + mock_enqueue.return_value = True + from app.schemas.generation_task import GenerationTaskResponse + mock_resp_fn.return_value = GenerationTaskResponse( + id="task-1", project_id="proj-1", asset_library_id="", + strategy_id="one_take", voice_library_id="", template_id="tpl-1", + asset_ids=[], title_ids=[], voice_ids=[], source_edit_plan_id="", + asset_select_mode="", batch_id="", video_title="", resolution="", + bgm_config={}, is_preview=False, source_task_id="", + output_width=1280, output_height=720, cover_url="", custom_title="", + title_config={}, logs="[]", status="pending", progress=0.0, + error_message="", created_at="2026-01-01T00:00:00Z", + updated_at="2026-01-01T00:00:00Z", started_at=None, completed_at=None, + created_by_user_id="user-1", auto_retry_enabled=False, + auto_retry_max=0, auto_retry_count=0, result_count=0, + ) + + mock_gen_repo = MagicMock() + mock_gen_repo.count_pending_by_user.return_value = 0 + mock_gen_repo.count_pending_total.return_value = 0 + + create_generation_task( + request=self._make_request(source_edit_plan_id="plan-already-set", template_id="tpl-1"), + authenticated_user=MagicMock(user=MagicMock(id="user-1")), + generation_task_repository=mock_gen_repo, + project_repository=MagicMock(), + asset_library_repository=MagicMock(), + asset_repository=MagicMock(), + db=MagicMock(), + ) + + assert mock_task.source_edit_plan_id == "plan-already-set" + mock_gen_repo.update.assert_not_called() + + @patch("apps.api.app.api.routes.generation_tasks._to_generation_task_response") + @patch("apps.api.app.api.routes.generation_tasks.safe_enqueue_generation_task") + @patch("apps.api.app.api.routes.generation_tasks.CreateGenerationTaskUseCase") + @patch("apps.api.app.api.routes.generation_tasks._resolve_project_and_library") + def test_fallback_handles_exception_gracefully(self, mock_resolve, mock_uc_cls, mock_enqueue, mock_resp_fn): + """When plan lookup fails, the fallback should not break the main flow.""" + from apps.api.app.api.routes.generation_tasks import create_generation_task + + mock_resolve.return_value = ("proj-1", None) + mock_task = self._make_mock_task(source_edit_plan_id="") + mock_uc_cls.return_value.execute.return_value = mock_task + mock_enqueue.return_value = True + from app.schemas.generation_task import GenerationTaskResponse + mock_resp_fn.return_value = GenerationTaskResponse( + id="task-1", project_id="proj-1", asset_library_id="", + strategy_id="one_take", voice_library_id="", template_id="tpl-1", + asset_ids=[], title_ids=[], voice_ids=[], source_edit_plan_id="", + asset_select_mode="", batch_id="", video_title="", resolution="", + bgm_config={}, is_preview=False, source_task_id="", + output_width=1280, output_height=720, cover_url="", custom_title="", + title_config={}, logs="[]", status="pending", progress=0.0, + error_message="", created_at="2026-01-01T00:00:00Z", + updated_at="2026-01-01T00:00:00Z", started_at=None, completed_at=None, + created_by_user_id="user-1", auto_retry_enabled=False, + auto_retry_max=0, auto_retry_count=0, result_count=0, + ) + + mock_plan_repo_cls = MagicMock() + mock_plan_repo_cls.side_effect = Exception("DB connection error") + + mock_gen_repo = MagicMock() + mock_gen_repo.count_pending_by_user.return_value = 0 + mock_gen_repo.count_pending_total.return_value = 0 + + with patch( + "packages.adapters.sqlalchemy_impl.edit_plan_repository.SQLAlchemyEditPlanRepository", + mock_plan_repo_cls, + ): + create_generation_task( + request=self._make_request(source_edit_plan_id="", template_id="tpl-1"), + authenticated_user=MagicMock(user=MagicMock(id="user-1")), + generation_task_repository=mock_gen_repo, + project_repository=MagicMock(), + asset_library_repository=MagicMock(), + asset_repository=MagicMock(), + db=MagicMock(), + ) + + # Task should still be created (fallback error doesn't break main flow) + assert mock_task.source_edit_plan_id == "" + mock_gen_repo.update.assert_not_called() -- 2.54.0 From 59d5986b20ad4947177ce136a7c062ba9e0119f6 Mon Sep 17 00:00:00 2001 From: CI Bot Date: Mon, 24 Aug 2026 03:00:19 +0000 Subject: [PATCH 4/6] style: auto-format with black + isort + prettier [skip ci-format-check] --- tests/unit/test_three_bugs_fix.py | 141 +++++++++++++++++++++++------- 1 file changed, 108 insertions(+), 33 deletions(-) diff --git a/tests/unit/test_three_bugs_fix.py b/tests/unit/test_three_bugs_fix.py index 19e9531ac..acd48668c 100644 --- a/tests/unit/test_three_bugs_fix.py +++ b/tests/unit/test_three_bugs_fix.py @@ -274,18 +274,43 @@ class TestPlanIdFallbackExecution: mock_uc_cls.return_value.execute.return_value = mock_task mock_enqueue.return_value = True from app.schemas.generation_task import GenerationTaskResponse + mock_resp_fn.return_value = GenerationTaskResponse( - id="task-1", project_id="proj-1", asset_library_id="", - strategy_id="one_take", voice_library_id="", template_id="tpl-1", - asset_ids=[], title_ids=[], voice_ids=[], source_edit_plan_id="", - asset_select_mode="", batch_id="", video_title="", resolution="", - bgm_config={}, is_preview=False, source_task_id="", - output_width=1280, output_height=720, cover_url="", custom_title="", - title_config={}, logs="[]", status="pending", progress=0.0, - error_message="", created_at="2026-01-01T00:00:00Z", - updated_at="2026-01-01T00:00:00Z", started_at=None, completed_at=None, - created_by_user_id="user-1", auto_retry_enabled=False, - auto_retry_max=0, auto_retry_count=0, result_count=0, + id="task-1", + project_id="proj-1", + asset_library_id="", + strategy_id="one_take", + voice_library_id="", + template_id="tpl-1", + asset_ids=[], + title_ids=[], + voice_ids=[], + source_edit_plan_id="", + asset_select_mode="", + batch_id="", + video_title="", + resolution="", + bgm_config={}, + is_preview=False, + source_task_id="", + output_width=1280, + output_height=720, + cover_url="", + custom_title="", + title_config={}, + logs="[]", + status="pending", + progress=0.0, + error_message="", + created_at="2026-01-01T00:00:00Z", + updated_at="2026-01-01T00:00:00Z", + started_at=None, + completed_at=None, + created_by_user_id="user-1", + auto_retry_enabled=False, + auto_retry_max=0, + auto_retry_count=0, + result_count=0, ) mock_plan = MagicMock() @@ -331,18 +356,43 @@ class TestPlanIdFallbackExecution: mock_uc_cls.return_value.execute.return_value = mock_task mock_enqueue.return_value = True from app.schemas.generation_task import GenerationTaskResponse + mock_resp_fn.return_value = GenerationTaskResponse( - id="task-1", project_id="proj-1", asset_library_id="", - strategy_id="one_take", voice_library_id="", template_id="tpl-1", - asset_ids=[], title_ids=[], voice_ids=[], source_edit_plan_id="", - asset_select_mode="", batch_id="", video_title="", resolution="", - bgm_config={}, is_preview=False, source_task_id="", - output_width=1280, output_height=720, cover_url="", custom_title="", - title_config={}, logs="[]", status="pending", progress=0.0, - error_message="", created_at="2026-01-01T00:00:00Z", - updated_at="2026-01-01T00:00:00Z", started_at=None, completed_at=None, - created_by_user_id="user-1", auto_retry_enabled=False, - auto_retry_max=0, auto_retry_count=0, result_count=0, + id="task-1", + project_id="proj-1", + asset_library_id="", + strategy_id="one_take", + voice_library_id="", + template_id="tpl-1", + asset_ids=[], + title_ids=[], + voice_ids=[], + source_edit_plan_id="", + asset_select_mode="", + batch_id="", + video_title="", + resolution="", + bgm_config={}, + is_preview=False, + source_task_id="", + output_width=1280, + output_height=720, + cover_url="", + custom_title="", + title_config={}, + logs="[]", + status="pending", + progress=0.0, + error_message="", + created_at="2026-01-01T00:00:00Z", + updated_at="2026-01-01T00:00:00Z", + started_at=None, + completed_at=None, + created_by_user_id="user-1", + auto_retry_enabled=False, + auto_retry_max=0, + auto_retry_count=0, + result_count=0, ) mock_gen_repo = MagicMock() @@ -375,18 +425,43 @@ class TestPlanIdFallbackExecution: mock_uc_cls.return_value.execute.return_value = mock_task mock_enqueue.return_value = True from app.schemas.generation_task import GenerationTaskResponse + mock_resp_fn.return_value = GenerationTaskResponse( - id="task-1", project_id="proj-1", asset_library_id="", - strategy_id="one_take", voice_library_id="", template_id="tpl-1", - asset_ids=[], title_ids=[], voice_ids=[], source_edit_plan_id="", - asset_select_mode="", batch_id="", video_title="", resolution="", - bgm_config={}, is_preview=False, source_task_id="", - output_width=1280, output_height=720, cover_url="", custom_title="", - title_config={}, logs="[]", status="pending", progress=0.0, - error_message="", created_at="2026-01-01T00:00:00Z", - updated_at="2026-01-01T00:00:00Z", started_at=None, completed_at=None, - created_by_user_id="user-1", auto_retry_enabled=False, - auto_retry_max=0, auto_retry_count=0, result_count=0, + id="task-1", + project_id="proj-1", + asset_library_id="", + strategy_id="one_take", + voice_library_id="", + template_id="tpl-1", + asset_ids=[], + title_ids=[], + voice_ids=[], + source_edit_plan_id="", + asset_select_mode="", + batch_id="", + video_title="", + resolution="", + bgm_config={}, + is_preview=False, + source_task_id="", + output_width=1280, + output_height=720, + cover_url="", + custom_title="", + title_config={}, + logs="[]", + status="pending", + progress=0.0, + error_message="", + created_at="2026-01-01T00:00:00Z", + updated_at="2026-01-01T00:00:00Z", + started_at=None, + completed_at=None, + created_by_user_id="user-1", + auto_retry_enabled=False, + auto_retry_max=0, + auto_retry_count=0, + result_count=0, ) mock_plan_repo_cls = MagicMock() -- 2.54.0 From 26b27a34fdd6656bd76efd721645f80b1b9b163a Mon Sep 17 00:00:00 2001 From: xiaoxia Date: Mon, 24 Aug 2026 11:09:31 +0800 Subject: [PATCH 5/6] =?UTF-8?q?fix:=20AI=20Review=E4=BF=AE=E5=A4=8D=20-=20?= =?UTF-8?q?plan=5Fid=E5=85=9C=E5=BA=95=E6=94=B9=E4=B8=BADB=E5=B1=82?= =?UTF-8?q?=E8=BF=87=E6=BB=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AI Code Review阻塞问题：list_by_template(limit=20)+内存过滤user_id 存在边界缺陷（超过20条或排序不确定时可能关联错误plan） 修复：直接查EditPlanModel，在DB层按template_id+user_id过滤 并按created_at desc取最新一条，无limit风险 更新测试：mock改为DB query chain，新增no_match场景 11个回归测试全过 --- apps/api/app/api/routes/generation_tasks.py | 35 +-- tests/unit/test_three_bugs_fix.py | 276 ++++++-------------- 2 files changed, 101 insertions(+), 210 deletions(-) diff --git a/apps/api/app/api/routes/generation_tasks.py b/apps/api/app/api/routes/generation_tasks.py index 7880f44c7..84019f2e3 100755 --- a/apps/api/app/api/routes/generation_tasks.py +++ b/apps/api/app/api/routes/generation_tasks.py @@ -379,25 +379,28 @@ def create_generation_task( ) # 兜底关联编辑计划：前端未传 source_edit_plan_id 时， - # 通过 template_id + user_id 查找该用户的 plan + # 通过 template_id + user_id 在 DB 层直接查找最新的 plan if not task.source_edit_plan_id and request.template_id: try: - from packages.adapters.sqlalchemy_impl.edit_plan_repository import ( - SQLAlchemyEditPlanRepository, - ) + from packages.adapters.sqlalchemy_impl.models import EditPlanModel - _plan_repo = SQLAlchemyEditPlanRepository(db) - _plans = _plan_repo.list_by_template(request.template_id, limit=20) - for _p in _plans: - if (_p.created_by_user_id or "") == user_id: - task.source_edit_plan_id = _p.id - generation_task_repository.update(task) - logger.info( - "[生成任务] 自动关联编辑计划: task_id=%s plan_id=%s", - task.id, - _p.id, - ) - break + _plan_model = ( + db.query(EditPlanModel) + .filter( + EditPlanModel.template_id == request.template_id, + EditPlanModel.created_by_user_id == user_id, + ) + .order_by(EditPlanModel.created_at.desc()) + .first() + ) + if _plan_model: + task.source_edit_plan_id = _plan_model.id + generation_task_repository.update(task) + logger.info( + "[生成任务] 自动关联编辑计划: task_id=%s plan_id=%s", + task.id, + _plan_model.id, + ) except Exception: logger.warning( "[生成任务] 查找关联编辑计划失败（不影响主流程）: task_id=%s", diff --git a/tests/unit/test_three_bugs_fix.py b/tests/unit/test_three_bugs_fix.py index acd48668c..dc0546998 100644 --- a/tests/unit/test_three_bugs_fix.py +++ b/tests/unit/test_three_bugs_fix.py @@ -156,8 +156,7 @@ class TestPlanIdFallback: from apps.api.app.api.routes import generation_tasks source = inspect.getsource(generation_tasks.create_generation_task) - assert "SQLAlchemyEditPlanRepository" in source - assert "list_by_template" in source + assert "EditPlanModel" in source assert "兜底关联编辑计划" in source assert "自动关联编辑计划" in source @@ -230,6 +229,7 @@ class TestPlanIdFallbackExecution: t.auto_retry_enabled = False t.auto_retry_max = 0 t.auto_retry_count = 0 + t.result_count = 0 return t @staticmethod @@ -261,229 +261,117 @@ class TestPlanIdFallbackExecution: req.asset_library_id = None return req - @patch("apps.api.app.api.routes.generation_tasks._to_generation_task_response") - @patch("apps.api.app.api.routes.generation_tasks.safe_enqueue_generation_task") - @patch("apps.api.app.api.routes.generation_tasks.CreateGenerationTaskUseCase") - @patch("apps.api.app.api.routes.generation_tasks._resolve_project_and_library") - def test_fallback_sets_plan_id_when_empty(self, mock_resolve, mock_uc_cls, mock_enqueue, mock_resp_fn): - """When source_edit_plan_id is empty, fallback finds plan and sets it.""" - from apps.api.app.api.routes.generation_tasks import create_generation_task - - mock_resolve.return_value = ("proj-1", None) - mock_task = self._make_mock_task(source_edit_plan_id="") - mock_uc_cls.return_value.execute.return_value = mock_task - mock_enqueue.return_value = True + def _run_create_task(self, mock_task, mock_request, mock_db, mock_gen_repo): + """Helper to run create_generation_task with common mocks.""" from app.schemas.generation_task import GenerationTaskResponse - mock_resp_fn.return_value = GenerationTaskResponse( - id="task-1", - project_id="proj-1", - asset_library_id="", - strategy_id="one_take", - voice_library_id="", - template_id="tpl-1", - asset_ids=[], - title_ids=[], - voice_ids=[], - source_edit_plan_id="", - asset_select_mode="", - batch_id="", - video_title="", - resolution="", - bgm_config={}, - is_preview=False, - source_task_id="", - output_width=1280, - output_height=720, - cover_url="", - custom_title="", - title_config={}, - logs="[]", - status="pending", - progress=0.0, - error_message="", - created_at="2026-01-01T00:00:00Z", - updated_at="2026-01-01T00:00:00Z", - started_at=None, - completed_at=None, - created_by_user_id="user-1", - auto_retry_enabled=False, - auto_retry_max=0, - auto_retry_count=0, - result_count=0, - ) - - mock_plan = MagicMock() - mock_plan.id = "plan-found-123" - mock_plan.created_by_user_id = "user-1" - - mock_plan_repo_cls = MagicMock() - mock_plan_repo_instance = MagicMock() - mock_plan_repo_instance.list_by_template.return_value = [mock_plan] - mock_plan_repo_cls.return_value = mock_plan_repo_instance - - mock_gen_repo = MagicMock() - mock_gen_repo.count_pending_by_user.return_value = 0 - mock_gen_repo.count_pending_total.return_value = 0 - - with patch( - "packages.adapters.sqlalchemy_impl.edit_plan_repository.SQLAlchemyEditPlanRepository", - mock_plan_repo_cls, + with ( + patch("apps.api.app.api.routes.generation_tasks._resolve_project_and_library", return_value=("proj-1", None)), + patch("apps.api.app.api.routes.generation_tasks.CreateGenerationTaskUseCase") as mock_uc_cls, + patch("apps.api.app.api.routes.generation_tasks.safe_enqueue_generation_task", return_value=True), + patch("apps.api.app.api.routes.generation_tasks._to_generation_task_response") as mock_resp_fn, ): - create_generation_task( - request=self._make_request(source_edit_plan_id="", template_id="tpl-1"), + mock_uc_cls.return_value.execute.return_value = mock_task + mock_resp_fn.return_value = GenerationTaskResponse( + id="task-1", project_id="proj-1", asset_library_id="", + strategy_id="one_take", voice_library_id="", template_id="tpl-1", + asset_ids=[], title_ids=[], voice_ids=[], source_edit_plan_id="", + asset_select_mode="", batch_id="", video_title="", resolution="", + bgm_config={}, is_preview=False, source_task_id="", + output_width=1280, output_height=720, cover_url="", custom_title="", + title_config={}, logs="[]", status="pending", progress=0.0, + error_message="", created_at="2026-01-01T00:00:00Z", + updated_at="2026-01-01T00:00:00Z", started_at=None, completed_at=None, + created_by_user_id="user-1", auto_retry_enabled=False, + auto_retry_max=0, auto_retry_count=0, result_count=0, + ) + + from apps.api.app.api.routes.generation_tasks import create_generation_task + + return create_generation_task( + request=mock_request, authenticated_user=MagicMock(user=MagicMock(id="user-1")), generation_task_repository=mock_gen_repo, project_repository=MagicMock(), asset_library_repository=MagicMock(), asset_repository=MagicMock(), - db=MagicMock(), + db=mock_db, ) + def test_fallback_sets_plan_id_when_empty(self): + """When source_edit_plan_id is empty, fallback finds plan via DB query.""" + mock_task = self._make_mock_task(source_edit_plan_id="") + mock_request = self._make_request(source_edit_plan_id="", template_id="tpl-1") + + # Mock DB query chain: db.query(EditPlanModel).filter(...).order_by(...).first() + mock_plan_model = MagicMock() + mock_plan_model.id = "plan-found-123" + + mock_db = MagicMock() + query_chain = MagicMock() + query_chain.filter.return_value = query_chain + query_chain.order_by.return_value = query_chain + query_chain.first.return_value = mock_plan_model + mock_db.query.return_value = query_chain + + mock_gen_repo = MagicMock() + mock_gen_repo.count_pending_by_user.return_value = 0 + mock_gen_repo.count_pending_total.return_value = 0 + + self._run_create_task(mock_task, mock_request, mock_db, mock_gen_repo) + assert mock_task.source_edit_plan_id == "plan-found-123" mock_gen_repo.update.assert_called_once_with(mock_task) - @patch("apps.api.app.api.routes.generation_tasks._to_generation_task_response") - @patch("apps.api.app.api.routes.generation_tasks.safe_enqueue_generation_task") - @patch("apps.api.app.api.routes.generation_tasks.CreateGenerationTaskUseCase") - @patch("apps.api.app.api.routes.generation_tasks._resolve_project_and_library") - def test_no_fallback_when_plan_id_already_set(self, mock_resolve, mock_uc_cls, mock_enqueue, mock_resp_fn): + def test_no_fallback_when_plan_id_already_set(self): """When source_edit_plan_id is already set, fallback should NOT run.""" - from apps.api.app.api.routes.generation_tasks import create_generation_task - - mock_resolve.return_value = ("proj-1", None) mock_task = self._make_mock_task(source_edit_plan_id="plan-already-set") - mock_uc_cls.return_value.execute.return_value = mock_task - mock_enqueue.return_value = True - from app.schemas.generation_task import GenerationTaskResponse - - mock_resp_fn.return_value = GenerationTaskResponse( - id="task-1", - project_id="proj-1", - asset_library_id="", - strategy_id="one_take", - voice_library_id="", - template_id="tpl-1", - asset_ids=[], - title_ids=[], - voice_ids=[], - source_edit_plan_id="", - asset_select_mode="", - batch_id="", - video_title="", - resolution="", - bgm_config={}, - is_preview=False, - source_task_id="", - output_width=1280, - output_height=720, - cover_url="", - custom_title="", - title_config={}, - logs="[]", - status="pending", - progress=0.0, - error_message="", - created_at="2026-01-01T00:00:00Z", - updated_at="2026-01-01T00:00:00Z", - started_at=None, - completed_at=None, - created_by_user_id="user-1", - auto_retry_enabled=False, - auto_retry_max=0, - auto_retry_count=0, - result_count=0, - ) + mock_request = self._make_request(source_edit_plan_id="plan-already-set", template_id="tpl-1") + mock_db = MagicMock() mock_gen_repo = MagicMock() mock_gen_repo.count_pending_by_user.return_value = 0 mock_gen_repo.count_pending_total.return_value = 0 - create_generation_task( - request=self._make_request(source_edit_plan_id="plan-already-set", template_id="tpl-1"), - authenticated_user=MagicMock(user=MagicMock(id="user-1")), - generation_task_repository=mock_gen_repo, - project_repository=MagicMock(), - asset_library_repository=MagicMock(), - asset_repository=MagicMock(), - db=MagicMock(), - ) + self._run_create_task(mock_task, mock_request, mock_db, mock_gen_repo) assert mock_task.source_edit_plan_id == "plan-already-set" mock_gen_repo.update.assert_not_called() - @patch("apps.api.app.api.routes.generation_tasks._to_generation_task_response") - @patch("apps.api.app.api.routes.generation_tasks.safe_enqueue_generation_task") - @patch("apps.api.app.api.routes.generation_tasks.CreateGenerationTaskUseCase") - @patch("apps.api.app.api.routes.generation_tasks._resolve_project_and_library") - def test_fallback_handles_exception_gracefully(self, mock_resolve, mock_uc_cls, mock_enqueue, mock_resp_fn): - """When plan lookup fails, the fallback should not break the main flow.""" - from apps.api.app.api.routes.generation_tasks import create_generation_task - - mock_resolve.return_value = ("proj-1", None) + def test_fallback_no_match_leaves_plan_id_empty(self): + """When no plan matches, source_edit_plan_id stays empty.""" mock_task = self._make_mock_task(source_edit_plan_id="") - mock_uc_cls.return_value.execute.return_value = mock_task - mock_enqueue.return_value = True - from app.schemas.generation_task import GenerationTaskResponse + mock_request = self._make_request(source_edit_plan_id="", template_id="tpl-1") - mock_resp_fn.return_value = GenerationTaskResponse( - id="task-1", - project_id="proj-1", - asset_library_id="", - strategy_id="one_take", - voice_library_id="", - template_id="tpl-1", - asset_ids=[], - title_ids=[], - voice_ids=[], - source_edit_plan_id="", - asset_select_mode="", - batch_id="", - video_title="", - resolution="", - bgm_config={}, - is_preview=False, - source_task_id="", - output_width=1280, - output_height=720, - cover_url="", - custom_title="", - title_config={}, - logs="[]", - status="pending", - progress=0.0, - error_message="", - created_at="2026-01-01T00:00:00Z", - updated_at="2026-01-01T00:00:00Z", - started_at=None, - completed_at=None, - created_by_user_id="user-1", - auto_retry_enabled=False, - auto_retry_max=0, - auto_retry_count=0, - result_count=0, - ) - - mock_plan_repo_cls = MagicMock() - mock_plan_repo_cls.side_effect = Exception("DB connection error") + mock_db = MagicMock() + query_chain = MagicMock() + query_chain.filter.return_value = query_chain + query_chain.order_by.return_value = query_chain + query_chain.first.return_value = None # no matching plan + mock_db.query.return_value = query_chain mock_gen_repo = MagicMock() mock_gen_repo.count_pending_by_user.return_value = 0 mock_gen_repo.count_pending_total.return_value = 0 - with patch( - "packages.adapters.sqlalchemy_impl.edit_plan_repository.SQLAlchemyEditPlanRepository", - mock_plan_repo_cls, - ): - create_generation_task( - request=self._make_request(source_edit_plan_id="", template_id="tpl-1"), - authenticated_user=MagicMock(user=MagicMock(id="user-1")), - generation_task_repository=mock_gen_repo, - project_repository=MagicMock(), - asset_library_repository=MagicMock(), - asset_repository=MagicMock(), - db=MagicMock(), - ) + self._run_create_task(mock_task, mock_request, mock_db, mock_gen_repo) + + assert mock_task.source_edit_plan_id == "" + mock_gen_repo.update.assert_not_called() + + def test_fallback_handles_exception_gracefully(self): + """When DB query fails, the fallback should not break the main flow.""" + mock_task = self._make_mock_task(source_edit_plan_id="") + mock_request = self._make_request(source_edit_plan_id="", template_id="tpl-1") + + mock_db = MagicMock() + mock_db.query.side_effect = Exception("DB connection error") + + mock_gen_repo = MagicMock() + mock_gen_repo.count_pending_by_user.return_value = 0 + mock_gen_repo.count_pending_total.return_value = 0 + + self._run_create_task(mock_task, mock_request, mock_db, mock_gen_repo) # Task should still be created (fallback error doesn't break main flow) assert mock_task.source_edit_plan_id == "" -- 2.54.0 From bf5244a6e1dee05fdc0e354bd70ecb1c6ea7e1d3 Mon Sep 17 00:00:00 2001 From: CI Bot Date: Mon, 24 Aug 2026 03:13:03 +0000 Subject: [PATCH 6/6] style: auto-format with black + isort + prettier [skip ci-format-check] --- tests/unit/test_three_bugs_fix.py | 50 +++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/tests/unit/test_three_bugs_fix.py b/tests/unit/test_three_bugs_fix.py index dc0546998..23cc0ef12 100644 --- a/tests/unit/test_three_bugs_fix.py +++ b/tests/unit/test_three_bugs_fix.py @@ -266,24 +266,50 @@ class TestPlanIdFallbackExecution: from app.schemas.generation_task import GenerationTaskResponse with ( - patch("apps.api.app.api.routes.generation_tasks._resolve_project_and_library", return_value=("proj-1", None)), + patch( + "apps.api.app.api.routes.generation_tasks._resolve_project_and_library", return_value=("proj-1", None) + ), patch("apps.api.app.api.routes.generation_tasks.CreateGenerationTaskUseCase") as mock_uc_cls, patch("apps.api.app.api.routes.generation_tasks.safe_enqueue_generation_task", return_value=True), patch("apps.api.app.api.routes.generation_tasks._to_generation_task_response") as mock_resp_fn, ): mock_uc_cls.return_value.execute.return_value = mock_task mock_resp_fn.return_value = GenerationTaskResponse( - id="task-1", project_id="proj-1", asset_library_id="", - strategy_id="one_take", voice_library_id="", template_id="tpl-1", - asset_ids=[], title_ids=[], voice_ids=[], source_edit_plan_id="", - asset_select_mode="", batch_id="", video_title="", resolution="", - bgm_config={}, is_preview=False, source_task_id="", - output_width=1280, output_height=720, cover_url="", custom_title="", - title_config={}, logs="[]", status="pending", progress=0.0, - error_message="", created_at="2026-01-01T00:00:00Z", - updated_at="2026-01-01T00:00:00Z", started_at=None, completed_at=None, - created_by_user_id="user-1", auto_retry_enabled=False, - auto_retry_max=0, auto_retry_count=0, result_count=0, + id="task-1", + project_id="proj-1", + asset_library_id="", + strategy_id="one_take", + voice_library_id="", + template_id="tpl-1", + asset_ids=[], + title_ids=[], + voice_ids=[], + source_edit_plan_id="", + asset_select_mode="", + batch_id="", + video_title="", + resolution="", + bgm_config={}, + is_preview=False, + source_task_id="", + output_width=1280, + output_height=720, + cover_url="", + custom_title="", + title_config={}, + logs="[]", + status="pending", + progress=0.0, + error_message="", + created_at="2026-01-01T00:00:00Z", + updated_at="2026-01-01T00:00:00Z", + started_at=None, + completed_at=None, + created_by_user_id="user-1", + auto_retry_enabled=False, + auto_retry_max=0, + auto_retry_count=0, + result_count=0, ) from apps.api.app.api.routes.generation_tasks import create_generation_task -- 2.54.0