diff --git a/shierfeng-fastapi-backend/module_business/service/shipment_service.py b/shierfeng-fastapi-backend/module_business/service/shipment_service.py index fb6b2c3..b6d4e50 100644 --- a/shierfeng-fastapi-backend/module_business/service/shipment_service.py +++ b/shierfeng-fastapi-backend/module_business/service/shipment_service.py @@ -663,16 +663,15 @@ class ShipmentService: hidden_site_ids: set[int], require_active: bool, ) -> None: + warehouses: dict[int, Any] = {} + warehouse_permission_errors: dict[int, PermissionException] = {} for warehouse_id in sorted(warehouse_ids): warehouse = await MasterDao.get_warehouse( query_db, warehouse_id, for_update=True, ) - if warehouse is None: - raise ServiceException(message='来源仓库不存在') - if require_active and warehouse.status != '0': - raise ServiceException(message='来源仓库已停用') + warehouses[warehouse_id] = warehouse try: await AccessService.assert_warehouse_access( query_db, @@ -680,21 +679,18 @@ class ShipmentService: warehouse_id, for_update=True, ) - except PermissionException: - if warehouse_id in hidden_warehouse_ids: - cls._raise_shipment_not_accessible(current_user) - raise + except PermissionException as exc: + warehouse_permission_errors[warehouse_id] = exc + sites: dict[int, Any] = {} + site_permission_errors: dict[int, PermissionException] = {} for site_id in sorted(site_ids): site = await MasterDao.get_site( query_db, site_id, for_update=True, ) - if site is None: - raise ServiceException(message='目标现场不存在') - if require_active and site.status != '0': - raise ServiceException(message='目标现场已停用') + sites[site_id] = site try: await AccessService.assert_site_access( query_db, @@ -702,10 +698,33 @@ class ShipmentService: site_id, for_update=True, ) - except PermissionException: - if site_id in hidden_site_ids: - cls._raise_shipment_not_accessible(current_user) - raise + except PermissionException as exc: + site_permission_errors[site_id] = exc + + if hidden_warehouse_ids & warehouse_permission_errors.keys() or ( + hidden_site_ids & site_permission_errors.keys() + ): + cls._raise_shipment_not_accessible(current_user) + + explicit_permission_errors = [ + warehouse_permission_errors[warehouse_id] for warehouse_id in sorted(warehouse_permission_errors) + ] + explicit_permission_errors.extend(site_permission_errors[site_id] for site_id in sorted(site_permission_errors)) + if explicit_permission_errors: + raise explicit_permission_errors[0] + + for warehouse_id in sorted(warehouses): + warehouse = warehouses[warehouse_id] + if warehouse is None: + raise ServiceException(message='来源仓库不存在') + if require_active and warehouse.status != '0': + raise ServiceException(message='来源仓库已停用') + for site_id in sorted(sites): + site = sites[site_id] + if site is None: + raise ServiceException(message='目标现场不存在') + if require_active and site.status != '0': + raise ServiceException(message='目标现场已停用') @classmethod async def _lock_and_reauthorize_route( diff --git a/shierfeng-fastapi-backend/tests/module_business/test_shipment_service.py b/shierfeng-fastapi-backend/tests/module_business/test_shipment_service.py index 6beda9e..183a8bd 100644 --- a/shierfeng-fastapi-backend/tests/module_business/test_shipment_service.py +++ b/shierfeng-fastapi-backend/tests/module_business/test_shipment_service.py @@ -1072,6 +1072,138 @@ class ShipmentDraftServiceTest(unittest.IsolatedAsyncioTestCase): db.commit.assert_not_awaited() db.rollback.assert_awaited_once() + async def test_edit_hidden_current_route_denial_wins_after_all_stable_route_locks(self) -> None: + scenarios = ( + ('both-warehouses-revoked', {7, 8}, set(), set(), set()), + ('new-warehouse-and-current-site-revoked', {7}, {10}, set(), set()), + ('current-warehouse-disabled-and-revoked', {8}, set(), {8}, set()), + ('current-site-disabled-and-revoked', set(), {10}, set(), {10}), + ) + expected_locks = [ + 'warehouse-master-7', + 'warehouse-mapping-7', + 'warehouse-master-8', + 'warehouse-mapping-8', + 'site-master-9', + 'site-mapping-9', + 'site-master-10', + 'site-mapping-10', + ] + for ( + scenario, + denied_warehouse_ids, + denied_site_ids, + disabled_warehouse_ids, + disabled_site_ids, + ) in scenarios: + with self.subTest(scenario=scenario): + db = MagicMock(spec=AsyncSession) + shipment = make_shipment( + source_warehouse_id=8, + target_site_id=10, + pallet_ids=(2,), + ) + page = ShipmentUpdateModel( + shipmentId=12, + sourceWarehouseId=7, + targetSiteId=9, + palletIds=[2], + ) + locks: list[str] = [] + + async def warehouse_access( + _db: AsyncSession, + _user: CurrentUserModel, + warehouse_id: int, + _denied_ids: set[int] = denied_warehouse_ids, + _locks: list[str] = locks, + **kwargs: Any, + ) -> None: + if not kwargs.get('for_update'): + return + _locks.append(f'warehouse-mapping-{warehouse_id}') + if warehouse_id in _denied_ids: + raise PermissionException(data='', message='无权访问该仓库') + + async def site_access( + _db: AsyncSession, + _user: CurrentUserModel, + site_id: int, + _denied_ids: set[int] = denied_site_ids, + _locks: list[str] = locks, + **kwargs: Any, + ) -> None: + if not kwargs.get('for_update'): + return + _locks.append(f'site-mapping-{site_id}') + if site_id in _denied_ids: + raise PermissionException(data='', message='无权访问该客户现场') + + async def get_warehouse( + _db: AsyncSession, + warehouse_id: int, + _disabled_ids: set[int] = disabled_warehouse_ids, + _locks: list[str] = locks, + **kwargs: Any, + ) -> BizWarehouse: + if kwargs.get('for_update'): + _locks.append(f'warehouse-master-{warehouse_id}') + return BizWarehouse( + warehouse_id=warehouse_id, + warehouse_code=f'W-{warehouse_id}', + warehouse_name=f'{warehouse_id}号仓', + status='1' if kwargs.get('for_update') and warehouse_id in _disabled_ids else '0', + ) + + async def get_site( + _db: AsyncSession, + site_id: int, + _disabled_ids: set[int] = disabled_site_ids, + _locks: list[str] = locks, + **kwargs: Any, + ) -> BizCustomerSite: + if kwargs.get('for_update'): + _locks.append(f'site-master-{site_id}') + return BizCustomerSite( + site_id=site_id, + customer_id=1, + site_code=f'S-{site_id}', + site_name=f'{site_id}号现场', + status='1' if kwargs.get('for_update') and site_id in _disabled_ids else '0', + ) + + update = AsyncMock() + replace_items = AsyncMock() + with ( + patch.object(AccessDao, 'list_user_warehouse_ids', new=AsyncMock(return_value=[7, 8])), + patch.object(AccessDao, 'list_user_site_ids', new=AsyncMock(return_value=[9, 10])), + patch.object(OperationDao, 'get_shipment_for_update', new=AsyncMock(return_value=shipment)), + patch.object( + AccessService, + 'assert_warehouse_access', + new=AsyncMock(side_effect=warehouse_access), + ), + patch.object(AccessService, 'assert_site_access', new=AsyncMock(side_effect=site_access)), + patch.object(MasterDao, 'get_warehouse', new=AsyncMock(side_effect=get_warehouse)), + patch.object(MasterDao, 'get_site', new=AsyncMock(side_effect=get_site)), + patch.object( + ShipmentService, + '_lock_and_validate_pallets', + new=AsyncMock(return_value=([], [], [])), + ), + patch.object(OperationDao, 'update_shipment', new=update), + patch.object(OperationDao, 'replace_shipment_items', new=replace_items), + self.assertRaises(PermissionException) as raised, + ): + await ShipmentService.edit_shipment_services(db, page, self.user) + + self.assertEqual('无权访问该发货单', raised.exception.message) + self.assertEqual(expected_locks, locks) + update.assert_not_awaited() + replace_items.assert_not_awaited() + db.commit.assert_not_awaited() + db.rollback.assert_awaited_once() + async def test_edit_locks_old_and_new_routes_in_stable_grouped_order(self) -> None: db = MagicMock(spec=AsyncSession) shipment = make_shipment( @@ -1242,6 +1374,19 @@ class ShipmentDraftServiceTest(unittest.IsolatedAsyncioTestCase): status='0', ) + async def get_site( + _db: AsyncSession, + site_id: int, + **_kwargs: Any, + ) -> BizCustomerSite: + return BizCustomerSite( + site_id=site_id, + customer_id=1, + site_code=f'S-{site_id}', + site_name=f'{site_id}号现场', + status='0', + ) + async def warehouse_access( _db: AsyncSession, _user: CurrentUserModel, @@ -1253,12 +1398,16 @@ class ShipmentDraftServiceTest(unittest.IsolatedAsyncioTestCase): delete = AsyncMock() warehouse_mapping = AsyncMock(side_effect=warehouse_access) + site_lock = AsyncMock(side_effect=get_site) + site_mapping = AsyncMock() with ( patch.object(AccessDao, 'list_user_warehouse_ids', new=AsyncMock(return_value=[7, 8])), patch.object(AccessDao, 'list_user_site_ids', new=AsyncMock(return_value=[9, 10])), patch.object(OperationDao, 'get_shipment_for_update', new=AsyncMock(side_effect=lock_order)), patch.object(MasterDao, 'get_warehouse', new=AsyncMock(side_effect=get_warehouse)), patch.object(AccessService, 'assert_warehouse_access', new=warehouse_mapping), + patch.object(MasterDao, 'get_site', new=site_lock), + patch.object(AccessService, 'assert_site_access', new=site_mapping), patch.object(OperationDao, 'delete_shipment', new=delete), self.assertRaises(PermissionException) as raised, ): @@ -1268,6 +1417,9 @@ class ShipmentDraftServiceTest(unittest.IsolatedAsyncioTestCase): self.assertEqual([11, 12], parent_lock_order) self.assertEqual([7, 8], [call.args[2] for call in warehouse_mapping.await_args_list]) self.assertTrue(all(call.kwargs.get('for_update') for call in warehouse_mapping.await_args_list)) + self.assertEqual([9, 10], [call.args[1] for call in site_lock.await_args_list]) + self.assertEqual([9, 10], [call.args[2] for call in site_mapping.await_args_list]) + self.assertTrue(all(call.kwargs.get('for_update') for call in site_mapping.await_args_list)) delete.assert_not_awaited() self.db.commit.assert_not_awaited() self.db.rollback.assert_awaited_once() @@ -1762,7 +1914,16 @@ class ShipmentConfirmServiceTest(unittest.IsolatedAsyncioTestCase): inventory_lock = AsyncMock() update = AsyncMock() add_event = AsyncMock() - site_lock = AsyncMock() + site_lock = AsyncMock( + return_value=BizCustomerSite( + site_id=9, + customer_id=1, + site_code='S-9', + site_name='九号现场', + status='0', + ) + ) + site_mapping = AsyncMock() warehouse_mapping = AsyncMock(side_effect=PermissionException(data='', message='无权访问该仓库')) with ( patch.object(AccessDao, 'list_user_warehouse_ids', new=AsyncMock(return_value=[7])), @@ -1789,6 +1950,7 @@ class ShipmentConfirmServiceTest(unittest.IsolatedAsyncioTestCase): ) as warehouse_lock, patch.object(AccessService, 'assert_warehouse_access', new=warehouse_mapping), patch.object(MasterDao, 'get_site', new=site_lock), + patch.object(AccessService, 'assert_site_access', new=site_mapping), patch.object(OperationDao, 'update_shipment', new=update), patch.object(InventoryDao, 'add_event', new=add_event), self.assertRaises(PermissionException) as raised, @@ -1803,7 +1965,8 @@ class ShipmentConfirmServiceTest(unittest.IsolatedAsyncioTestCase): self.assertEqual('无权访问该发货单', raised.exception.message) warehouse_lock.assert_awaited_once_with(self.db, 7, for_update=True) warehouse_mapping.assert_awaited_once_with(self.db, self.user, 7, for_update=True) - site_lock.assert_not_awaited() + site_lock.assert_awaited_once_with(self.db, 9, for_update=True) + site_mapping.assert_awaited_once_with(self.db, self.user, 9, for_update=True) inventory_lock.assert_not_awaited() update.assert_not_awaited() add_event.assert_not_awaited() @@ -1997,7 +2160,7 @@ class ShipmentConfirmServiceTest(unittest.IsolatedAsyncioTestCase): ) warehouse_lock.assert_awaited_once_with(self.db, 7, for_update=True) warehouse_mapping.assert_awaited_once_with(self.db, self.user, 7, for_update=True) - site_lock.assert_not_awaited() + site_lock.assert_awaited_once_with(self.db, 9, for_update=True) self.db.commit.assert_not_awaited() self.assertEqual(2, self.db.rollback.await_count) @@ -2007,6 +2170,16 @@ class ShipmentConfirmServiceTest(unittest.IsolatedAsyncioTestCase): update = AsyncMock() add_event = AsyncMock() mapping = AsyncMock() + site_lock = AsyncMock( + return_value=BizCustomerSite( + site_id=9, + customer_id=1, + site_code='S-9', + site_name='九号现场', + status='0', + ) + ) + site_mapping = AsyncMock() with ( patch.object(AccessDao, 'list_user_warehouse_ids', new=AsyncMock(return_value=[7])), patch.object(AccessDao, 'list_user_site_ids', new=AsyncMock(return_value=[9])), @@ -2035,6 +2208,8 @@ class ShipmentConfirmServiceTest(unittest.IsolatedAsyncioTestCase): ), ), patch.object(AccessService, 'assert_warehouse_access', new=mapping), + patch.object(MasterDao, 'get_site', new=site_lock), + patch.object(AccessService, 'assert_site_access', new=site_mapping), patch.object(OperationDao, 'update_shipment', new=update), patch.object(InventoryDao, 'add_event', new=add_event), self.assertRaises(ServiceException) as raised, @@ -2045,7 +2220,9 @@ class ShipmentConfirmServiceTest(unittest.IsolatedAsyncioTestCase): self.assertEqual('DRAFT', shipment.status) self.assertEqual('FILLED_IN_WAREHOUSE', inventory.flow_state) self.assertEqual(4, inventory.version) - mapping.assert_not_awaited() + mapping.assert_awaited_once_with(self.db, self.user, 7, for_update=True) + site_lock.assert_awaited_once_with(self.db, 9, for_update=True) + site_mapping.assert_awaited_once_with(self.db, self.user, 9, for_update=True) update.assert_not_awaited() add_event.assert_not_awaited() self.db.rollback.assert_awaited_once() @@ -2121,7 +2298,16 @@ class ShipmentConfirmServiceTest(unittest.IsolatedAsyncioTestCase): update = AsyncMock() add_event = AsyncMock() transition = MagicMock() - site_lock = AsyncMock() + site_lock = AsyncMock( + return_value=BizCustomerSite( + site_id=9, + customer_id=1, + site_code='S-9', + site_name='九号现场', + status='0', + ) + ) + site_mapping = AsyncMock() with ( patch.object(AccessDao, 'list_user_warehouse_ids', new=AsyncMock(return_value=[7])), patch.object(AccessDao, 'list_user_site_ids', new=AsyncMock(return_value=[9])), @@ -2157,6 +2343,7 @@ class ShipmentConfirmServiceTest(unittest.IsolatedAsyncioTestCase): new=AsyncMock(side_effect=PermissionException(data='', message='无权访问该仓库')), ), patch.object(MasterDao, 'get_site', new=site_lock), + patch.object(AccessService, 'assert_site_access', new=site_mapping), patch.object(PalletStateMachine, 'ensure_transition', new=transition), patch.object(OperationDao, 'update_shipment', new=update), patch.object(InventoryDao, 'add_event', new=add_event), @@ -2170,7 +2357,8 @@ class ShipmentConfirmServiceTest(unittest.IsolatedAsyncioTestCase): self.assertEqual(7, inventory.warehouse_id) self.assertIsNone(inventory.site_id) self.assertEqual(4, inventory.version) - site_lock.assert_not_awaited() + site_lock.assert_awaited_once_with(self.db, 9, for_update=True) + site_mapping.assert_awaited_once_with(self.db, self.user, 9, for_update=True) transition.assert_not_called() update.assert_not_awaited() add_event.assert_not_awaited()