feat: AI生成流程协议补充 --story=136729554 - #846
Conversation
There was a problem hiding this comment.
Code Review Summary
Issues Found
| # | File | 级别 | 问题 |
|---|---|---|---|
| 1 | node_builder.py:102 |
⚡ 性能 | build_subprocess 冗余查询 Template 且未过滤 space_id |
| 2 | converter.py:372 |
🚨 严重 | TemplateSnapshot.DoesNotExist 未捕获,可能 500 |
| 3 | variable_builder.py:26 |
re.match 未处理 value 非字符串及非法正则场景 |
|
| 4 | data_models.py:131 |
⚡ 性能 | validate_custom_type 每个变量执行一次 DB 查询(N+1) |
| 5 | converter.py:268 |
SubProcess 未加入 single-next 校验 tuple |
Details
1. node_builder.py:102 ⚡
build_subprocess 再次执行 Template.objects.get(id=template_id) 但未过滤 space_id。_validate 已经查过一次并做了 scope 校验,这里建议复用验证阶段的结果(如传入 template 对象),避免冗余查询且保持 space_id 约束一致。
2. converter.py:372 🚨
TemplateSnapshot.objects.get(id=template.snapshot_id) 可能抛出 TemplateSnapshot.DoesNotExist(数据不一致时),但外层 try 只捕获了 Template.DoesNotExist,会导致 500 错误。建议增加对 TemplateSnapshot.DoesNotExist 的捕获。
3. variable_builder.py:26
var.value 类型为 Any,非字符串时 re.match 会抛 TypeError;用户传入的畸形正则也可能导致 re.error。建议加 isinstance(var.value, str) 前置检查,并 try/except re.error 返回友好错误信息。
4. data_models.py:131 ⚡
validate_custom_type 在 Pydantic validator 里逐一执行 VariableModel.objects.get(code=v),多个变量时产生 N 次 DB 查询。建议将合法 custom_type 集合在 converter 层一次性预取后传入校验。
5. converter.py:268
SubProcess 的 next 在文档中标注为 string 类型,但此处校验 tuple (NodeType.ACTIVITY, NodeType.START_EVENT, NodeType.CONVERGE_GATEWAY) 未包含 NodeType.SUBPROCESS,用户传入 list 类型 next 时不会报错。建议补充。
整体看这个 PR 结构清晰,SubProcess 支持和 failure_strategy 校验逻辑完备,文档同步到位(含 apigw-docs.zip 更新)。上述问题以异常处理和冗余查询为主,建议优先修复 #2(可能造成线上 500)。
| """ | ||
| 构建 SubProcess 类型的 activity 节点(最小字段版本)。 | ||
| """ | ||
| template = Template.objects.get(id=template_id) |
There was a problem hiding this comment.
⚡ build_subprocess 再次执行 Template.objects.get(id=template_id) 但未过滤 space_id。_validate 阶段已经查过一次并做了 scope 校验,建议复用验证阶段的结果(如传入 template 对象),避免冗余查询且保持 space_id 约束一致。
| value=template_id, | ||
| ) | ||
| ) | ||
| template_snapshot = TemplateSnapshot.objects.get(id=template.snapshot_id) |
There was a problem hiding this comment.
🚨 此处 TemplateSnapshot.objects.get 可能抛出 TemplateSnapshot.DoesNotExist(数据不一致时),但外层 try 只捕获了 Template.DoesNotExist,会导致未处理异常(500)。建议增加 except TemplateSnapshot.DoesNotExist 分支。
|
|
||
| def build_constant(var: A2FlowVariable, index: int) -> dict: | ||
| if var.validation and var.value: | ||
| if not re.match(var.validation, var.value): |
There was a problem hiding this comment.
var.value 类型为 Any,非字符串时 re.match 抛 TypeError;畸形正则导致 re.error。建议加 isinstance(var.value, str) 前置检查并用 try/except re.error 返回友好错误。
| @validator("custom_type") | ||
| def validate_custom_type(cls, v): | ||
| try: | ||
| VariableModel.objects.get(code=v) |
There was a problem hiding this comment.
⚡ validate_custom_type 每个变量逐一执行 VariableModel.objects.get(code=v),多变量场景产生 N 次 DB 查询。建议在 converter 层一次性预取可用 code 集合后传入校验。
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #846 +/- ##
===========================================
- Coverage 83.60% 83.32% -0.29%
===========================================
Files 324 324
Lines 20722 20858 +136
===========================================
+ Hits 17324 17379 +55
- Misses 3398 3479 +81 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
No description provided.