Release v1.5.0#88
Merged
Merged
Conversation
- AdminHandler: GET /admin/resources, /:rid, /:rid/versions - ResourceLogic: ListResources, GetByID, CountVersions, ListByResource - Resource/Version repo: ListResources, CountResources, GetResourceByID, ListVersionsByResource, CountVersionsByResource - Wire: inject AdminHandler into HandlerSet - Model: ListResourcesRequest, ListVersionsRequest, ResourceItem, VersionItem, ResourceDetailData, PageData response types
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体性的反馈：
- 避免在
admin.go中使用点导入（针对logic/misc和model），改为显式导入，以保持符号来源清晰并防止命名冲突。 - 如果其他 handler 使用（或将会使用）类似的分页和映射逻辑，可以考虑把
normalizePage和toResourceItem抽取到一个共享的辅助函数中，以避免重复并保持行为一致。
给 AI Agents 的提示
请根据这次代码评审中的评论进行修改：
## 总体评论
- 避免在 `admin.go` 中使用点导入（针对 `logic/misc` 和 `model`），改为显式导入，以保持符号来源清晰并防止命名冲突。
- 如果其他 handler 使用（或将会使用）类似的分页和映射逻辑，可以考虑把 `normalizePage` 和 `toResourceItem` 抽取到一个共享的辅助函数中，以避免重复并保持行为一致。
## 单独评论
### 评论 1
<location path="internal/interfaces/rest/handler/admin.go" line_range="52-61" />
<code_context>
+ g.Get("/:rid/versions", h.ListVersions)
+}
+
+func normalizePage(page, size int) (int, int) {
+ if page <= 0 {
+ page = defaultPage
+ }
+ if size <= 0 {
+ size = defaultPageSize
+ }
+ if size > maxPageSize {
+ size = maxPageSize
+ }
+ return page, size
+}
+
</code_context>
<issue_to_address>
**建议（性能）：** 考虑对 page 进行封顶，避免因极大的偏移量导致非常重的查询。
当前 `normalizePage` 只对 `pageSize` 做了边界限制，所以客户端仍然可以发送数百万级的 `page` 值，在大表上会产生非常大的偏移量（`(page-1)*size`），并且当 `page` 接近 `math.MaxInt/size` 时可能导致整数溢出。建议对 `page`（或最大偏移量）增加上限，以防御异常请求。
建议实现：
```golang
maxPageSize = 100
maxPage = 1_000_000
)
```
```golang
func (h *AdminHandler) Register(r fiber.Router) {
g := r.Group("/admin/resources")
g.Get("/", h.ListResources)
g.Get("/:rid", h.GetResource)
g.Get("/:rid/versions", h.ListVersions)
}
func normalizePage(page, size int) (int, int) {
if page <= 0 {
page = defaultPage
}
if page > maxPage {
page = maxPage
}
if size <= 0 {
size = defaultPageSize
}
if size > maxPageSize {
size = maxPageSize
}
return page, size
}
```
</issue_to_address>帮我变得更有用！请对每条评论点击 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Avoid using dot-imports in
admin.go(forlogic/miscandmodel) and instead import explicitly to keep symbol origins clear and prevent name collisions. - Consider extracting
normalizePageandtoResourceIteminto a shared helper if other handlers use (or will use) similar pagination and mapping logic, to avoid duplication and keep behavior consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Avoid using dot-imports in `admin.go` (for `logic/misc` and `model`) and instead import explicitly to keep symbol origins clear and prevent name collisions.
- Consider extracting `normalizePage` and `toResourceItem` into a shared helper if other handlers use (or will use) similar pagination and mapping logic, to avoid duplication and keep behavior consistent.
## Individual Comments
### Comment 1
<location path="internal/interfaces/rest/handler/admin.go" line_range="52-61" />
<code_context>
+ g.Get("/:rid/versions", h.ListVersions)
+}
+
+func normalizePage(page, size int) (int, int) {
+ if page <= 0 {
+ page = defaultPage
+ }
+ if size <= 0 {
+ size = defaultPageSize
+ }
+ if size > maxPageSize {
+ size = maxPageSize
+ }
+ return page, size
+}
+
</code_context>
<issue_to_address>
**suggestion (performance):** Consider capping page to avoid extremely large offsets causing heavy queries.
`normalizePage` currently only bounds `pageSize`, so a client can still send `page` values in the millions, resulting in huge offsets (`(page-1)*size`) on large tables and potential integer overflow when `page` approaches `math.MaxInt/size`. Consider adding an upper limit on `page` (or a maximum offset) to guard against pathological requests.
Suggested implementation:
```golang
maxPageSize = 100
maxPage = 1_000_000
)
```
```golang
func (h *AdminHandler) Register(r fiber.Router) {
g := r.Group("/admin/resources")
g.Get("/", h.ListResources)
g.Get("/:rid", h.GetResource)
g.Get("/:rid/versions", h.ListVersions)
}
func normalizePage(page, size int) (int, int) {
if page <= 0 {
page = defaultPage
}
if page > maxPage {
page = maxPage
}
if size <= 0 {
size = defaultPageSize
}
if size > maxPageSize {
size = maxPageSize
}
return page, size
}
```
</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
+52
to
+61
| func normalizePage(page, size int) (int, int) { | ||
| if page <= 0 { | ||
| page = defaultPage | ||
| } | ||
| if size <= 0 { | ||
| size = defaultPageSize | ||
| } | ||
| if size > maxPageSize { | ||
| size = maxPageSize | ||
| } |
There was a problem hiding this comment.
建议（性能）： 考虑对 page 进行封顶，避免因极大的偏移量导致非常重的查询。
当前 normalizePage 只对 pageSize 做了边界限制，所以客户端仍然可以发送数百万级的 page 值，在大表上会产生非常大的偏移量（(page-1)*size），并且当 page 接近 math.MaxInt/size 时可能导致整数溢出。建议对 page（或最大偏移量）增加上限，以防御异常请求。
建议实现：
maxPageSize = 100
maxPage = 1_000_000
)func (h *AdminHandler) Register(r fiber.Router) {
g := r.Group("/admin/resources")
g.Get("/", h.ListResources)
g.Get("/:rid", h.GetResource)
g.Get("/:rid/versions", h.ListVersions)
}
func normalizePage(page, size int) (int, int) {
if page <= 0 {
page = defaultPage
}
if page > maxPage {
page = maxPage
}
if size <= 0 {
size = defaultPageSize
}
if size > maxPageSize {
size = maxPageSize
}
return page, size
}Original comment in English
suggestion (performance): Consider capping page to avoid extremely large offsets causing heavy queries.
normalizePage currently only bounds pageSize, so a client can still send page values in the millions, resulting in huge offsets ((page-1)*size) on large tables and potential integer overflow when page approaches math.MaxInt/size. Consider adding an upper limit on page (or a maximum offset) to guard against pathological requests.
Suggested implementation:
maxPageSize = 100
maxPage = 1_000_000
)func (h *AdminHandler) Register(r fiber.Router) {
g := r.Group("/admin/resources")
g.Get("/", h.ListResources)
g.Get("/:rid", h.GetResource)
g.Get("/:rid/versions", h.ListVersions)
}
func normalizePage(page, size int) (int, int) {
if page <= 0 {
page = defaultPage
}
if page > maxPage {
page = maxPage
}
if size <= 0 {
size = defaultPageSize
}
if size > maxPageSize {
size = maxPageSize
}
return page, size
}
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by Sourcery
添加只读的管理员资源管理 API 以及配套的数据模型、组装逻辑和仓储/业务层，并进行少量依赖更新。
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
Add read-only admin resource management APIs and supporting data models, wiring, and repository/logic layers, along with minor dependency updates.
New Features:
Enhancements: