fix: provider source id contains slash will lead to 405 #4162
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了两个问题，并提供了一些整体性的反馈：
- 这些 API 现在在删除请求的 payload 中使用
id，而在查询模型时使用source_id；建议在这些端点之间统一字段命名，以避免调用方和后续维护者产生混淆。 - 删除端点在 payload 为空时会返回通用的“缺少配置数据”错误，即使它实际上只需要一个 ID；你可以通过直接校验 ID 字段并在缺失时返回更具体的错误信息来简化逻辑。
提供给 AI 代理的提示词
Please address the comments from this code review:
## Overall Comments
- The APIs now use `id` in the delete payload and `source_id` in the models query; consider standardizing on a single field name across these endpoints to avoid confusion for callers and future maintainers.
- The delete endpoint currently returns a generic '缺少配置数据' error when the payload is empty even though it only needs an ID; you might simplify this by directly validating the ID field and returning a more specific message when it is missing.
## Individual Comments
### Comment 1
<location> `astrbot/dashboard/routes/config.py:205-211` </location>
<code_context>
self.register_routes()
- async def delete_provider_source(self, provider_source_id: str):
+ async def delete_provider_source(self):
"""删除 provider_source，并更新关联的 providers"""
+ post_data = await request.json
+ if not post_data:
+ return Response().error("缺少配置数据").__dict__
+
+ provider_source_id = post_data.get("id")
+ if not provider_source_id:
+ return Response().error("缺少 provider_source_id").__dict__
</code_context>
<issue_to_address>
**suggestion:** Consider aligning identifier field naming with other provider source endpoints.
This handler reads the identifier from `id` in the JSON body, while `get_provider_source_models` now uses `source_id` as a query parameter. Using different names for the same concept can cause confusion and subtle bugs as more callers are added. Consider standardizing on one field name (e.g., `source_id`) across all provider source endpoints.
```suggestion
post_data = await request.json
if not post_data:
return Response().error("缺少配置数据").__dict__
# 与其他 provider_source 接口保持字段命名一致，优先使用 source_id
provider_source_id = post_data.get("source_id") or post_data.get("id")
if not provider_source_id:
return Response().error("缺少 source_id").__dict__
```
</issue_to_address>
### Comment 2
<location> `astrbot/dashboard/routes/config.py:695-702` </location>
<code_context>
return Response().error(f"获取嵌入维度失败: {e!s}").__dict__
- async def get_provider_source_models(self, provider_source_id: str):
+ async def get_provider_source_models(self):
"""获取指定 provider_source 支持的模型列表
本质上会临时初始化一个 Provider 实例，调用 get_models() 获取模型列表，然后销毁实例
"""
+ provider_source_id = request.args.get("source_id")
+ if not provider_source_id:
+ return Response().error("缺少参数 source_id").__dict__
</code_context>
<issue_to_address>
**suggestion:** Clarify the choice of using query param vs body for source id across endpoints.
This endpoint reads `source_id` from the query string, while `delete_provider_source` and `update_provider_source` expect the same identifier in the JSON body. Using different locations for the same resource id across related endpoints makes the API less predictable. Please consider aligning these endpoints on a single convention for where the provider source id is supplied.
```suggestion
async def get_provider_source_models(self):
"""获取指定 provider_source 支持的模型列表
本质上会临时初始化一个 Provider 实例，调用 get_models() 获取模型列表，然后销毁实例
为了与 delete_provider_source / update_provider_source 等接口保持一致，
此处从 JSON Body 中读取 source_id，而不是使用查询参数。
"""
post_data = request.get_json(silent=True) or {}
provider_source_id = post_data.get("source_id")
if not provider_source_id:
return Response().error("缺少参数 source_id").__dict__
```
</issue_to_address>帮我变得更有用！请在每一条评论上点选 👍 或 👎，我会根据你的反馈改进之后的评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The APIs now use
idin the delete payload andsource_idin the models query; consider standardizing on a single field name across these endpoints to avoid confusion for callers and future maintainers. - The delete endpoint currently returns a generic '缺少配置数据' error when the payload is empty even though it only needs an ID; you might simplify this by directly validating the ID field and returning a more specific message when it is missing.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The APIs now use `id` in the delete payload and `source_id` in the models query; consider standardizing on a single field name across these endpoints to avoid confusion for callers and future maintainers.
- The delete endpoint currently returns a generic '缺少配置数据' error when the payload is empty even though it only needs an ID; you might simplify this by directly validating the ID field and returning a more specific message when it is missing.
## Individual Comments
### Comment 1
<location> `astrbot/dashboard/routes/config.py:205-211` </location>
<code_context>
self.register_routes()
- async def delete_provider_source(self, provider_source_id: str):
+ async def delete_provider_source(self):
"""删除 provider_source，并更新关联的 providers"""
+ post_data = await request.json
+ if not post_data:
+ return Response().error("缺少配置数据").__dict__
+
+ provider_source_id = post_data.get("id")
+ if not provider_source_id:
+ return Response().error("缺少 provider_source_id").__dict__
</code_context>
<issue_to_address>
**suggestion:** Consider aligning identifier field naming with other provider source endpoints.
This handler reads the identifier from `id` in the JSON body, while `get_provider_source_models` now uses `source_id` as a query parameter. Using different names for the same concept can cause confusion and subtle bugs as more callers are added. Consider standardizing on one field name (e.g., `source_id`) across all provider source endpoints.
```suggestion
post_data = await request.json
if not post_data:
return Response().error("缺少配置数据").__dict__
# 与其他 provider_source 接口保持字段命名一致，优先使用 source_id
provider_source_id = post_data.get("source_id") or post_data.get("id")
if not provider_source_id:
return Response().error("缺少 source_id").__dict__
```
</issue_to_address>
### Comment 2
<location> `astrbot/dashboard/routes/config.py:695-702` </location>
<code_context>
return Response().error(f"获取嵌入维度失败: {e!s}").__dict__
- async def get_provider_source_models(self, provider_source_id: str):
+ async def get_provider_source_models(self):
"""获取指定 provider_source 支持的模型列表
本质上会临时初始化一个 Provider 实例，调用 get_models() 获取模型列表，然后销毁实例
"""
+ provider_source_id = request.args.get("source_id")
+ if not provider_source_id:
+ return Response().error("缺少参数 source_id").__dict__
</code_context>
<issue_to_address>
**suggestion:** Clarify the choice of using query param vs body for source id across endpoints.
This endpoint reads `source_id` from the query string, while `delete_provider_source` and `update_provider_source` expect the same identifier in the JSON body. Using different locations for the same resource id across related endpoints makes the API less predictable. Please consider aligning these endpoints on a single convention for where the provider source id is supplied.
```suggestion
async def get_provider_source_models(self):
"""获取指定 provider_source 支持的模型列表
本质上会临时初始化一个 Provider 实例，调用 get_models() 获取模型列表，然后销毁实例
为了与 delete_provider_source / update_provider_source 等接口保持一致，
此处从 JSON Body 中读取 source_id，而不是使用查询参数。
"""
post_data = request.get_json(silent=True) or {}
provider_source_id = post_data.get("source_id")
if not provider_source_id:
return Response().error("缺少参数 source_id").__dict__
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+205
to
+211
| post_data = await request.json | ||
| if not post_data: | ||
| return Response().error("缺少配置数据").__dict__ | ||
|
|
||
| provider_source_id = post_data.get("id") | ||
| if not provider_source_id: | ||
| return Response().error("缺少 provider_source_id").__dict__ |
Contributor
There was a problem hiding this comment.
suggestion: 建议将标识符字段命名与其他 provider source 相关端点保持一致。
当前处理函数从 JSON body 中的 id 读取标识符，而 get_provider_source_models 现在使用查询参数中的 source_id。对同一个概念使用不同的字段名，随着调用方增多，容易造成困惑并引入隐蔽问题。建议在所有 provider source 相关端点中统一字段名（例如统一使用 source_id）。
Suggested change
| post_data = await request.json | |
| if not post_data: | |
| return Response().error("缺少配置数据").__dict__ | |
| provider_source_id = post_data.get("id") | |
| if not provider_source_id: | |
| return Response().error("缺少 provider_source_id").__dict__ | |
| post_data = await request.json | |
| if not post_data: | |
| return Response().error("缺少配置数据").__dict__ | |
| # 与其他 provider_source 接口保持字段命名一致，优先使用 source_id | |
| provider_source_id = post_data.get("source_id") or post_data.get("id") | |
| if not provider_source_id: | |
| return Response().error("缺少 source_id").__dict__ |
Original comment in English
suggestion: Consider aligning identifier field naming with other provider source endpoints.
This handler reads the identifier from id in the JSON body, while get_provider_source_models now uses source_id as a query parameter. Using different names for the same concept can cause confusion and subtle bugs as more callers are added. Consider standardizing on one field name (e.g., source_id) across all provider source endpoints.
Suggested change
| post_data = await request.json | |
| if not post_data: | |
| return Response().error("缺少配置数据").__dict__ | |
| provider_source_id = post_data.get("id") | |
| if not provider_source_id: | |
| return Response().error("缺少 provider_source_id").__dict__ | |
| post_data = await request.json | |
| if not post_data: | |
| return Response().error("缺少配置数据").__dict__ | |
| # 与其他 provider_source 接口保持字段命名一致，优先使用 source_id | |
| provider_source_id = post_data.get("source_id") or post_data.get("id") | |
| if not provider_source_id: | |
| return Response().error("缺少 source_id").__dict__ |
Comment on lines
+695
to
+702
| async def get_provider_source_models(self): | ||
| """获取指定 provider_source 支持的模型列表 | ||
| 本质上会临时初始化一个 Provider 实例，调用 get_models() 获取模型列表，然后销毁实例 | ||
| """ | ||
| provider_source_id = request.args.get("source_id") | ||
| if not provider_source_id: | ||
| return Response().error("缺少参数 source_id").__dict__ |
Contributor
There was a problem hiding this comment.
suggestion: 请在各个端点之间明确统一是通过查询参数还是通过请求体传递 source id。
这个端点从查询字符串中读取 source_id，而 delete_provider_source 和 update_provider_source 则期望在 JSON body 中获取相同的标识符。在相关端点之间对同一个资源 ID 使用不同的传递方式，会降低 API 的可预测性。建议在这些端点之间统一约定 provider source id 的传递方式。
Suggested change
| async def get_provider_source_models(self): | |
| """获取指定 provider_source 支持的模型列表 | |
| 本质上会临时初始化一个 Provider 实例，调用 get_models() 获取模型列表，然后销毁实例 | |
| """ | |
| provider_source_id = request.args.get("source_id") | |
| if not provider_source_id: | |
| return Response().error("缺少参数 source_id").__dict__ | |
| async def get_provider_source_models(self): | |
| """获取指定 provider_source 支持的模型列表 | |
| 本质上会临时初始化一个 Provider 实例，调用 get_models() 获取模型列表，然后销毁实例 | |
| 为了与 delete_provider_source / update_provider_source 等接口保持一致， | |
| 此处从 JSON Body 中读取 source_id，而不是使用查询参数。 | |
| """ | |
| post_data = request.get_json(silent=True) or {} | |
| provider_source_id = post_data.get("source_id") | |
| if not provider_source_id: | |
| return Response().error("缺少参数 source_id").__dict__ |
Original comment in English
suggestion: Clarify the choice of using query param vs body for source id across endpoints.
This endpoint reads source_id from the query string, while delete_provider_source and update_provider_source expect the same identifier in the JSON body. Using different locations for the same resource id across related endpoints makes the API less predictable. Please consider aligning these endpoints on a single convention for where the provider source id is supplied.
Suggested change
| async def get_provider_source_models(self): | |
| """获取指定 provider_source 支持的模型列表 | |
| 本质上会临时初始化一个 Provider 实例，调用 get_models() 获取模型列表，然后销毁实例 | |
| """ | |
| provider_source_id = request.args.get("source_id") | |
| if not provider_source_id: | |
| return Response().error("缺少参数 source_id").__dict__ | |
| async def get_provider_source_models(self): | |
| """获取指定 provider_source 支持的模型列表 | |
| 本质上会临时初始化一个 Provider 实例，调用 get_models() 获取模型列表，然后销毁实例 | |
| 为了与 delete_provider_source / update_provider_source 等接口保持一致， | |
| 此处从 JSON Body 中读取 source_id，而不是使用查询参数。 | |
| """ | |
| post_data = request.get_json(silent=True) or {} | |
| provider_source_id = post_data.get("source_id") | |
| if not provider_source_id: | |
| return Response().error("缺少参数 source_id").__dict__ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
更新 provider source 配置 API，以避免路径参数问题，改为依赖请求 body/查询参数。
Bug Fixes（错误修复）:
Enhancements（功能增强）:
Original summary in English
Summary by Sourcery
Update provider source configuration APIs to avoid path-parameter issues and rely on request body/query parameters instead.
Bug Fixes:
Enhancements:
Original summary in English
Summary by Sourcery
更新 provider source 配置 API，以避免路径参数问题，改为依赖请求 body/查询参数。
Bug Fixes（错误修复）:
Enhancements（功能增强）:
Original summary in English
Summary by Sourcery
Update provider source configuration APIs to avoid path-parameter issues and rely on request body/query parameters instead.
Bug Fixes:
Enhancements: