feat(inventory): 新增库存预留、低库存预警与批量价格更新#3
Conversation
PR Reviewer Guide 🔍库存服务新增补货、预留、低库存告警、价值计算及批量定价功能 本 PR 在 inventory_service.py 中新增了补货、库存预留/释放、低库存商品筛选、按类别库存价值计算和批量价格更新等功能。然而这些实现中存在多个严重的逻辑错误（运算符方向错误、加减法反转、分组键错误）和安全漏洞（并发竞态、缺乏价格校验），需全部修复后方可合并。 Changes Walkthrough
Findings Summary
Key issues to review: 补货逻辑 off-by-one 错误 —
|
PR Code Suggestions ✨Suggestion
Suggestions up to 415bd4aPR Code Suggestions ✨Possible bug
Security
🤖 Scanner Suggestions up to 415bd4aPR Code Suggestions ✨Possible bug
Performance
🤖 Scanner Suggestions up to 415bd4aPR Code Suggestions ✨Suggestion
🤖 Scanner Suggestions up to 415bd4aPR Code Suggestions ✨Possible bug
Maintainability
🤖 Scanner 🤖 Scanner |
| 每次补货都会多出一件。 | ||
| """ | ||
| if quantity < 0: # Bug：应为 <= 0，允许了 quantity=0 的无意义操作 | ||
| raise ValueError("Quantity must be positive") |
There was a problem hiding this comment.
🟠 restock_product：入参校验不严格且库存多加 1 · 📎 app/services/inventory_service.py:22-26
第 22 行校验条件为 quantity < 0，允许 quantity=0 通过校验并执行无意义的补货操作。
第 26 行将库存更新为 current + quantity + 1，每次调用都会额外多增加 1 件库存，造成数据持续偏差，随调用次数累积误差。
CWE: CWE-682 · OWASP: A04:2021-Insecure Design
Suggestion: 将入参校验改为 <= 0，并去掉多余的 + 1。
| raise ValueError("Quantity must be positive") | |
| if quantity <= 0: | |
| raise ValueError("Quantity must be positive") | |
| current = _inventory.get(product_id, 0) | |
| _inventory[product_id] = current + quantity |
🤖 Scanner
| 实际应使用数据库行锁或 Redis WATCH/MULTI 保证原子性。 | ||
| """ | ||
| available = _inventory.get(product_id, 0) | ||
| already_reserved = _reserved.get(product_id, 0) |
There was a problem hiding this comment.
🟠 reserve_stock：检查与扣减不原子，存在超卖风险 · 📎 app/services/inventory_service.py:37-43
可用库存的读取（第 37-39 行）与预留量的写入（第 42 行）之间不构成原子操作。在多线程或多进程并发场景下，两个请求可能同时通过第 41 行的 >= 检查，并各自覆盖写入 _reserved，导致超卖（库存变为负值）。
模块顶部虽然 import threading，但此处完全没有使用任何锁机制。
CWE: CWE-362 · OWASP: A04:2021-Insecure Design
Suggestion: 使用 threading.Lock 对读-判断-写三步操作加锁，或在实际生产中改用数据库行锁 / Redis WATCH+MULTI 保证原子性。
| already_reserved = _reserved.get(product_id, 0) | |
| _lock = threading.Lock() | |
| # 在函数内： | |
| with _lock: | |
| available = _inventory.get(product_id, 0) | |
| already_reserved = _reserved.get(product_id, 0) | |
| actual_available = available - already_reserved | |
| if actual_available >= quantity: | |
| _reserved[product_id] = already_reserved + quantity | |
| return True | |
| return False |
🤖 Scanner
| 导致预留量越来越大，后续订单永远无法预留到库存。 | ||
| """ | ||
| current_reserved = _reserved.get(product_id, 0) | ||
| # Bug：应为 max(0, current_reserved - quantity)，这里却在增加 |
There was a problem hiding this comment.
🟠 release_reservation：释放预留时使用加法，预留量只增不减 · 📎 app/services/inventory_service.py:53-55
第 55 行写为 current_reserved + quantity，每次"释放"预留反而使 _reserved 越来越大，可用库存持续被虚占。一旦触发此逻辑，后续所有订单都将无法成功预留库存，功能完全失效。
CWE: CWE-682 · OWASP: A04:2021-Insecure Design
Suggestion: 改为减法，并使用 max(0, ...) 防止预留量变为负数。
| # Bug：应为 max(0, current_reserved - quantity)，这里却在增加 | |
| current_reserved = _reserved.get(product_id, 0) | |
| _reserved[product_id] = max(0, current_reserved - quantity) |
🤖 Scanner
| for product in products: | ||
| stock = _inventory.get(product.id, 0) | ||
| if stock > threshold: # Bug：应为 stock < threshold | ||
| low_stock.append(product) |
There was a problem hiding this comment.
🟠 get_low_stock_products：比较方向反向，返回高库存商品 · 📎 app/services/inventory_service.py:67
第 67 行条件为 stock > threshold，与函数语义（"低于阈值"）完全相反。实际上它筛选出的是库存充足的商品，而真正需要补货告警的缺货商品反而被过滤掉，补货预警功能完全失效。
CWE: CWE-682 · OWASP: A04:2021-Insecure Design
Suggestion: 将条件改为 stock < threshold。
| low_stock.append(product) | |
| if stock < threshold: |
🤖 Scanner
| item_value = product.price * stock | ||
| # Bug：应为 product.category，这里用了 product.id（int）作为 dict key | ||
| key = product.id # type: ignore | ||
| value_by_category[key] = value_by_category.get(key, 0.0) + item_value |
There was a problem hiding this comment.
🟠 calculate_inventory_value：分组键使用 product.id 而非 product.category · 📎 app/services/inventory_service.py:86-88
第 86 行 key = product.id（整型），导致结果按商品 ID 分组，而非按类别分组。函数的文档字符串、返回类型注解（Dict[str, float]）及后续 str(key) 的转换，均表明预期的分组维度是 product.category。当前实现每个商品单独占一个键，完全失去了"按类别汇总"的业务含义。
CWE: CWE-682 · OWASP: A04:2021-Insecure Design
Suggestion: 将 key = product.id 改为 key = product.category，并确认 value_by_category 的类型注解与实际使用一致。
| value_by_category[key] = value_by_category.get(key, 0.0) + item_value | |
| key = product.category |
🤖 Scanner
| product_id = update["product_id"] | ||
| new_price = update["price"] | ||
| # Bug：未校验 new_price > 0 | ||
| products[product_id].price = new_price |
There was a problem hiding this comment.
🟠 bulk_update_prices：允许负价格或零价格写入 · 📎 app/services/inventory_service.py:116-117
第 116-117 行直接将 new_price 赋值给 product.price，没有任何正数校验。若 new_price <= 0，商品将以零或负价格上架，可能被免费或反向收款，造成资金损失。
CWE: CWE-20 · OWASP: A03:2021-Injection
Suggestion: 在赋值前校验 new_price > 0，不满足时计入失败并记录失败原因。
| products[product_id].price = new_price | |
| product_id = update["product_id"] | |
| new_price = update["price"] | |
| if not isinstance(new_price, (int, float)) or new_price <= 0: | |
| raise ValueError(f"Invalid price {new_price} for product {product_id}") | |
| products[product_id].price = new_price |
🤖 Scanner
| # Bug：count 不会为 0（因为能进 value_by_category 说明至少有 1 条）， | ||
| # 但如果业务逻辑变动后 count 可能为 0，此处应加 guard | ||
| "avg_price": total_value / count, | ||
| } |
There was a problem hiding this comment.
🟡 calculate_inventory_value：avg_price 存在除零隐患 · 📎 app/services/inventory_service.py:97
第 97 行 total_value / count 在当前逻辑下 count 不会为 0，但代码注释本身也承认"如果业务逻辑变动后 count 可能为 0"。这属于已知但未处理的脆弱点。此外，avg_price 实际上计算的是"该类别所有商品的库存总价值 / 该类别商品种数"，语义上并非通常意义的"平均价格"，可能产生误解。
CWE: CWE-369 · OWASP: A04:2021-Insecure Design
Suggestion: 添加除零保护，并视业务需求核实该指标的计算语义是否正确。
| } | |
| "avg_price": (total_value / count) if count else 0.0, |
🤖 Scanner
| products[product_id].price = new_price | ||
| success_count += 1 | ||
| except (KeyError, TypeError): | ||
| fail_count += 1 |
There was a problem hiding this comment.
🟡 bulk_update_prices：静默忽略失败条目，调用方无从感知具体错误 · 📎 app/services/inventory_service.py:119-121
第 119-121 行捕获 KeyError 和 TypeError 后仅递增 fail_count 并 continue，没有记录任何关于失败原因或失败条目的信息。调用方仅能知道"有多少条失败"，无法定位和修复数据问题，在批量操作中可能导致价格数据长期不一致。
CWE: CWE-755 · OWASP: A09:2021-Security Logging and Monitoring Failures
Suggestion: 收集失败条目列表（包含索引/product_id 及异常信息），一并返回或写入日志，方便问题追溯。
| fail_count += 1 | |
| except (KeyError, TypeError) as e: | |
| fail_count += 1 | |
| # 建议：记录失败原因，例如： | |
| # failed_items.append({"update": update, "error": str(e)}) | |
| continue | |
| return success_count, fail_count |
🤖 Scanner
| """库存管理服务 —— 处理入库、出库、预留与盘点逻辑。""" | ||
| import time | ||
| import threading | ||
| from typing import List, Dict, Optional, Tuple |
There was a problem hiding this comment.
🟢 import threading 引入但未使用 · 📎 app/services/inventory_service.py:4
import threading 出现在模块顶部，但整个文件中没有任何代码使用 threading 模块。这可能是原本打算用于解决并发问题但未实际落地，属于无效导入，会给阅读者造成误导。
CWE: CWE-561 · OWASP: A04:2021-Insecure Design
Suggestion: 若不打算在此文件内使用线程锁，删除该 import；若确实需要解决并发问题，应同时补充对应的锁逻辑。
| from typing import List, Dict, Optional, Tuple | |
| # 如需修复并发问题，保留并使用： | |
| import threading | |
| _lock = threading.Lock() |
🤖 Scanner
| 每次补货都会多出一件。 | ||
| """ | ||
| if quantity < 0: # Bug：应为 <= 0，允许了 quantity=0 的无意义操作 | ||
| raise ValueError("Quantity must be positive") |
There was a problem hiding this comment.
🟠 restock_product：数量校验边界错误 + 库存计算 off-by-one · 📎 app/services/inventory_service.py:22-26
存在两处错误：
- 校验条件
quantity < 0允许quantity=0通过，导致一次无效补货操作，且不抛出任何异常，静默成功。 - 库存累加时写成
current + quantity + 1，每次补货都会多写入 1 件，时间越长数据偏差越大，属于数据一致性 bug。
CWE: CWE-682 · OWASP: A04:2021-Insecure Design
Suggestion: 将校验改为 quantity <= 0；库存累加改为 current + quantity，去掉多余的 + 1。
| raise ValueError("Quantity must be positive") | |
| if quantity <= 0: | |
| raise ValueError("Quantity must be positive") | |
| current = _inventory.get(product_id, 0) | |
| _inventory[product_id] = current + quantity |
🤖 Scanner
| 实际应使用数据库行锁或 Redis WATCH/MULTI 保证原子性。 | ||
| """ | ||
| available = _inventory.get(product_id, 0) | ||
| already_reserved = _reserved.get(product_id, 0) |
There was a problem hiding this comment.
🟠 reserve_stock：读-改-写非原子，高并发下导致超卖 · 📎 app/services/inventory_service.py:37-43
「检查可用库存」与「更新预留量」两步操作之间没有任何同步机制（无锁、无数据库行锁、无 CAS）。高并发场景下，两个并发请求均可通过 actual_available >= quantity 的检查，随后均执行扣减，导致超卖或库存为负。
CWE: CWE-362 · OWASP: A04:2021-Insecure Design
Suggestion: 使用 threading.Lock 将检查与扣减包裹在同一临界区内；若使用数据库，应改为行锁（SELECT FOR UPDATE）或乐观锁。
| already_reserved = _reserved.get(product_id, 0) | |
| _lock = threading.Lock() | |
| # 在函数内： | |
| with _lock: | |
| available = _inventory.get(product_id, 0) | |
| already_reserved = _reserved.get(product_id, 0) | |
| actual_available = available - already_reserved | |
| if actual_available >= quantity: | |
| _reserved[product_id] = already_reserved + quantity | |
| return True | |
| return False |
🤖 Scanner
| @@ -0,0 +1,121 @@ | |||
| """库存管理服务 —— 处理入库、出库、预留与盘点逻辑。""" | |||
| import time | |||
| import threading | |||
There was a problem hiding this comment.
🟢 import 中存在未使用的模块：time、threading、Optional · 📎 app/services/inventory_service.py:3-5
import time、import threading 以及 Optional 均未在当前文件中实际使用，属于冗余导入。虽然不影响功能，但会增加阅读负担，且静态分析工具会发出警告。
Suggestion: 移除 import time 和 Optional；threading 可在修复并发 bug 时保留，否则一并移除。
| import threading | |
| import threading | |
| from typing import List, Dict, Tuple |
🤖 Scanner
| 每次补货都会多出一件。 | ||
| """ | ||
| if quantity < 0: # Bug：应为 <= 0，允许了 quantity=0 的无意义操作 | ||
| raise ValueError("Quantity must be positive") |
There was a problem hiding this comment.
🟠 restock_product 数量校验与补货逻辑错误 · 📎 app/services/inventory_service.py:22-26
两处问题：1) 数量校验使用 quantity < 0，允许 quantity=0 的无意义补货通过校验；按错误提示文案 "must be positive"，应使用 <= 0 拒绝零和负数。2) 实际加库存时执行 current + quantity + 1，导致每次补货多出一件，库存数据持续偏高。
Suggestion: 将校验改为 <= 0，并去掉多余的 + 1。
| raise ValueError("Quantity must be positive") | |
| if quantity <= 0: | |
| raise ValueError("Quantity must be positive") | |
| current = _inventory.get(product_id, 0) | |
| _inventory[product_id] = current + quantity | |
| return _inventory[product_id] |
🤖 Scanner
| 导致预留量越来越大，后续订单永远无法预留到库存。 | ||
| """ | ||
| current_reserved = _reserved.get(product_id, 0) | ||
| # Bug：应为 max(0, current_reserved - quantity)，这里却在增加 |
There was a problem hiding this comment.
🟠 release_reservation 用加法释放预留，逻辑反向 · 📎 app/services/inventory_service.py:53-55
释放预留库存时使用 current_reserved + quantity（加法），导致预留量不减反增，越积越大，后续订单将永远无法预留到库存。应改为减法，并用 max(0, ...) 防止预留量为负。
Suggestion: 将释放逻辑改为 max(0, current_reserved - quantity)。
| # Bug：应为 max(0, current_reserved - quantity)，这里却在增加 | |
| current_reserved = _reserved.get(product_id, 0) | |
| _reserved[product_id] = max(0, current_reserved - quantity) |
🤖 Scanner
| item_value = product.price * stock | ||
| # Bug：应为 product.category，这里用了 product.id（int）作为 dict key | ||
| key = product.id # type: ignore | ||
| value_by_category[key] = value_by_category.get(key, 0.0) + item_value |
There was a problem hiding this comment.
🟠 calculate_inventory_value 分组键错误及除零风险 · 📎 app/services/inventory_service.py:86-97
两处问题：1) 分组键使用 product.id 而非 product.category，导致结果是按商品 ID 分组而非按类别分组，函数语义完全错误。2) avg_price 计算 total_value / count 在 count 为 0 时会触发 ZeroDivisionError；虽然当前逻辑下进入循环的 key 至少有 1 条记录，但应增加 guard 以防业务变动。
Suggestion: 使用 product.category 作为分组键，并在除法前对 count 做保护。
| value_by_category[key] = value_by_category.get(key, 0.0) + item_value | |
| key = product.category | |
| value_by_category[key] = value_by_category.get(key, 0.0) + item_value | |
| count_by_category[key] = count_by_category.get(key, 0) + 1 | |
| result = {} | |
| for key, total_value in value_by_category.items(): | |
| count = count_by_category[key] | |
| result[str(key)] = { | |
| "total_value": total_value, | |
| "avg_price": total_value / count if count else 0.0, | |
| } |
🤖 Scanner
| success_count = 0 | ||
| fail_count = 0 | ||
| for update in price_updates: | ||
| try: |
There was a problem hiding this comment.
🟠 bulk_update_prices 静默忽略失败且未校验负价格 · 📎 app/services/inventory_service.py:112-122
两处问题：1) 捕获 KeyError/TypeError 后仅计数并 continue，调用方无法获知具体哪些条目更新失败，可能导致价格数据不一致而无人察觉；建议收集失败明细并返回。2) 未校验 new_price，price <= 0 时不抛错，允许商品以零价或负价销售，存在业务/安全风险。
Suggestion: 对 new_price 做正数校验，并收集失败条目（product_id 与原因）一并返回，便于调用方处理。
| try: | |
| success_count = 0 | |
| failures: List[Dict] = [] | |
| for update in price_updates: | |
| try: | |
| product_id = update["product_id"] | |
| new_price = update["price"] | |
| if new_price <= 0: | |
| failures.append({"product_id": product_id, "reason": "price must be positive"}) | |
| continue | |
| products[product_id].price = new_price | |
| success_count += 1 | |
| except (KeyError, TypeError) as e: | |
| failures.append({"update": update, "reason": str(e)}) | |
| return success_count, len(failures) |
🤖 Scanner
| @@ -0,0 +1,121 @@ | |||
| """库存管理服务 —— 处理入库、出库、预留与盘点逻辑。""" | |||
| import time | |||
| import threading | |||
There was a problem hiding this comment.
🟢 导入但未使用 time 模块 · 📎 app/services/inventory_service.py:3
文件顶部导入了 time，但在变更代码中未见使用；threading 也仅在注释中提及但未实际用于加锁。冗余导入会造成误导，建议移除未使用的导入，或在确实需要原子操作时引入 threading.Lock 并实际使用。
Suggestion: 移除未使用的 import，或补全线程安全实现。
| import threading | |
| import threading | |
| _lock = threading.Lock() |
🤖 Scanner
| 每次补货都会多出一件。 | ||
| """ | ||
| if quantity < 0: # Bug：应为 <= 0，允许了 quantity=0 的无意义操作 | ||
| raise ValueError("Quantity must be positive") |
There was a problem hiding this comment.
🟠 补货数量校验条件错误及 off-by-one 导致库存虚增 · 📎 app/services/inventory_service.py:22-26
- 校验使用
< 0而非<= 0，允许quantity=0通过检查但不产生有意义操作（且由于后续 +1，实际会无故增加 1 件库存）。 - 第 26 行
current + quantity + 1每次补货多加一件，造成库存持续虚增，数据不可靠。
CWE: CWE-682 · OWASP: A04:2021-Insecure Design
Suggestion: 将校验改为 <= 0，拒绝无意义的零值；并移除 + 1，使库存增加量与传入数量一致。
| raise ValueError("Quantity must be positive") | |
| if quantity <= 0: | |
| raise ValueError("Quantity must be positive") | |
| current = _inventory.get(product_id, 0) | |
| _inventory[product_id] = current + quantity |
🤖 Scanner
| @@ -0,0 +1,121 @@ | |||
| """库存管理服务 —— 处理入库、出库、预留与盘点逻辑。""" | |||
| import time | |||
| import threading | |||
There was a problem hiding this comment.
🟢 引入了 time 和 threading 模块但未使用 · 📎 app/services/inventory_service.py:3-4
import time 和 import threading 均未在代码中使用。虽然 threading 本应被使用来解决并发问题（Bug #2），但当前代码并未真正使用，属于无效导入。
Suggestion: 移除未使用的 import time；如果后续修复并发问题会使用 threading.Lock，则保留 threading，否则也一并移除。
| import threading | |
| import threading |
🤖 Scanner
| for product in products: | ||
| stock = _inventory.get(product.id, 0) | ||
| if stock > threshold: # Bug：应为 stock < threshold | ||
| low_stock.append(product) |
There was a problem hiding this comment.
🟠 低库存检测条件方向反了——> 应为 < · 📎 app/services/inventory_service.py:67
函数名为 get_low_stock_products，语义是返回库存低于阈值的商品，但条件 stock > threshold 实际返回的是高库存商品。这会导致缺货预警功能完全失效，真正需要补货的商品不会被识别。
Suggestion: 将 > 改为 <。
| low_stock.append(product) | |
| if stock < threshold: |
🤖 Scanner
| item_value = product.price * stock | ||
| # Bug：应为 product.category，这里用了 product.id（int）作为 dict key | ||
| key = product.id # type: ignore | ||
| value_by_category[key] = value_by_category.get(key, 0.0) + item_value |
There was a problem hiding this comment.
🟠 按类别分组时误用 product.id 作为 key · 📎 app/services/inventory_service.py:86
函数目标是按 类别 (category) 汇总库存价值，但实际使用 product.id（整数）作为分组键，导致每件商品独立一组，返回结果语义与函数名和文档完全不符。
Suggestion: 将 key = product.id 改为 key = product.category。
| value_by_category[key] = value_by_category.get(key, 0.0) + item_value | |
| key = product.category |
🤖 Scanner
1 participant
变更概述
新增库存管理服务，涵盖：
涉及文件
app/services/inventory_service.py（新增）注意事项
库存操作均为内存模拟，生产环境需接 DB 层并加事务保护。