<fix>[rest]: port Go SDK generator bug fixes from ZSV-11399#3616
Open
MatheMatrix wants to merge 2 commits into5.5.12from
Open
<fix>[rest]: port Go SDK generator bug fixes from ZSV-11399#3616MatheMatrix wants to merge 2 commits into5.5.12from
MatheMatrix wants to merge 2 commits into5.5.12from
Conversation
Port critical Go SDK generation fixes from MR !9249 (ZSV-11399): GoApiTemplate.groovy: - Add context.Context parameter to all generated Go method signatures - Add allTo response key extraction for PostWithRespKey - Add valid flag + isValid() for safe template initialization - Add reset() to prevent stale state across repeated SDK generation - Add getApiOptPath() for optional path handling - Fix pluralization: only replace y→ies after consonants - Pass allTo to GetWithSpec for correct response unwrapping GoInventory.groovy: - Add reset() + call GoApiTemplate.reset() at start of generation - Use template.isValid() instead of template.at != null - Fix time.Now → time.Now() in generated Go code (4 occurrences) - Remove deprecated generateClientFile() (~400 lines of dead code) base_param_types.go.template: - Fix time.Now → time.Now() (missing parentheses) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough更新增强了 Go API 模板生成器的状态管理与重置能力，在生成的 Go 客户端方法中统一加入 Changes
Sequence Diagram(s)（无序列图：更改以生成器/模板内部为主，未引入新的多组件运行时控制流） Estimated code review effort🎯 4 (复杂) | ⏱️ ~45 分钟 诗
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
rest/src/main/resources/scripts/GoApiTemplate.groovy (2)
555-588:⚠️ Potential issue | 🔴 Critical先把生成的
contextimport 补上。这里开始给所有 client 方法生成
ctx context.Context，但rest/src/main/resources/scripts/GoInventory.groovy里generateOtherActionsFile()和generateActionFiles()生成的 import block 还没有context。这样产出的other_actions.go/*_actions.go会直接报undefined: context。#!/bin/bash set -e echo "== Methods now using context.Context ==" sed -n '549,623p' rest/src/main/resources/scripts/GoApiTemplate.groovy | rg -n 'context\.Context|func \(cli \*ZSClient\)' echo echo "== other_actions.go import template ==" sed -n '1343,1353p' rest/src/main/resources/scripts/GoInventory.groovy echo echo "== grouped *_actions.go import template ==" sed -n '1663,1674p' rest/src/main/resources/scripts/GoInventory.groovyAlso applies to: 598-623
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoApiTemplate.groovy` around lines 555 - 588, The generated client methods in GoApiTemplate.groovy now add a ctx parameter of type context.Context, but the import blocks produced by generateOtherActionsFile() and generateActionFiles() in GoInventory.groovy don’t include the context import, causing undefined: context errors; update the import templates inside generateOtherActionsFile() and generateActionFiles() to include "context" (add it alongside existing imports) so that other_actions.go and grouped *_actions.go include import "context" whenever generated methods accept ctx context.Context.
1337-1344:⚠️ Potential issue | 🟠 Major异步方法没有向 PostWithAsync 传递 ctx 参数。
函数签名已包含
ctx context.Context参数，但在调用cli.PostWithAsync(resource, responseKey, params, retVal, true)时并未传递它。同步方法生成模板正确地将 ctx 传递给cli.Post(ctx, ...)、cli.Delete(ctx, ...)等调用。需要修正模板，将 ctx 作为参数传递给 PostWithAsync 调用，以确保正确的上下文传播和超时/取消语义。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoApiTemplate.groovy` around lines 1337 - 1344, The async method template in GoApiTemplate.groovy builds a ZSClient method (function name ${asyncMethodName}) that accepts ctx context.Context but calls cli.PostWithAsync without passing ctx; update the template so the PostWithAsync invocation on cli includes ctx as the first argument (matching how sync methods call cli.Post/cli.Delete) to propagate the context and preserve cancellation/timeouts when calling PostWithAsync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rest/src/main/resources/scripts/GoApiTemplate.groovy`:
- Around line 685-698: generateGetMethod currently ignores the parsed allTo
value and hardcodes response keys (e.g. "inventory" or "") in the unwrap=true
branches; update generateGetMethod to accept an allTo parameter (the value
parsed from `@RestResponse`) and replace the hardcoded responseKey arguments in
the GetWithRespKey and GetWithSpec calls inside the unwrap branches (references:
generateGetMethod, GetWithRespKey, GetWithSpec, variables clzName, cleanPath,
responseStructName, fieldName) so the generated methods use "${allTo}" instead
of "inventory" or "" across all branches.
In `@rest/src/main/resources/scripts/GoInventory.groovy`:
- Around line 109-115: The reset() method only clears longJobMappings but leaves
instance caches that leak between generate() runs; update reset() to also clear
allApiTemplates, generatedViewStructs, generatedParamStructs,
generatedClientMethods, additionalClasses (and any similar instance collections
used by generate()) so the GoInventory instance is fully reinitialized before a
new generation; locate the static void reset() method and add clear() calls for
each of those collections (and verify any other fields referenced by generate()
are reset as well) and keep the existing logger.warn call.
---
Outside diff comments:
In `@rest/src/main/resources/scripts/GoApiTemplate.groovy`:
- Around line 555-588: The generated client methods in GoApiTemplate.groovy now
add a ctx parameter of type context.Context, but the import blocks produced by
generateOtherActionsFile() and generateActionFiles() in GoInventory.groovy don’t
include the context import, causing undefined: context errors; update the import
templates inside generateOtherActionsFile() and generateActionFiles() to include
"context" (add it alongside existing imports) so that other_actions.go and
grouped *_actions.go include import "context" whenever generated methods accept
ctx context.Context.
- Around line 1337-1344: The async method template in GoApiTemplate.groovy
builds a ZSClient method (function name ${asyncMethodName}) that accepts ctx
context.Context but calls cli.PostWithAsync without passing ctx; update the
template so the PostWithAsync invocation on cli includes ctx as the first
argument (matching how sync methods call cli.Post/cli.Delete) to propagate the
context and preserve cancellation/timeouts when calling PostWithAsync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: 132f1f34-cafb-40da-9dd2-f10ccb8aeb3d
📒 Files selected for processing (3)
rest/src/main/resources/scripts/GoApiTemplate.groovyrest/src/main/resources/scripts/GoInventory.groovyrest/src/main/resources/scripts/templates/base_param_types.go.template
Comment on lines
+685
to
698
| return """func (cli *ZSClient) ${clzName}(ctx context.Context) (*view.${viewStructName}, error) { | ||
| \tvar resp view.${responseStructName} | ||
| \tif err := cli.GetWithRespKey("${cleanPath}", "", "inventory", nil, &resp); err != nil { | ||
| \tif err := cli.GetWithRespKey(ctx, "${cleanPath}", "", "inventory", nil, &resp); err != nil { | ||
| \t\treturn nil, err | ||
| \t} | ||
| \treturn &resp.${fieldName}, nil | ||
| } | ||
| """ | ||
| } else { | ||
| // Single placeholder: use GetWithRespKey with uuid to extract inventory | ||
| return """func (cli *ZSClient) ${clzName}(uuid string) (*view.${viewStructName}, error) { | ||
| return """func (cli *ZSClient) ${clzName}(ctx context.Context, uuid string) (*view.${viewStructName}, error) { | ||
| \tvar resp view.${responseStructName} | ||
| \tif err := cli.GetWithRespKey("${cleanPath}", uuid, "inventory", nil, &resp); err != nil { | ||
| \tif err := cli.GetWithRespKey(ctx, "${cleanPath}", uuid, "inventory", nil, &resp); err != nil { | ||
| \t\treturn nil, err |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
echo "== Current GET branches =="
sed -n '671,720p' rest/src/main/resources/scripts/GoApiTemplate.groovy
echo
echo "== RestResponse(allTo=...) usages =="
rg -nP '@RestResponse\s*\(\s*allTo\s*=\s*"[^"]+"' --glob '*.java'Repository: MatheMatrix/zstack
Length of output: 50374
🏁 Script executed:
# First, let's see the full generateGetMethod function and its context
sed -n '665,750p' rest/src/main/resources/scripts/GoApiTemplate.groovyRepository: MatheMatrix/zstack
Length of output: 3625
🏁 Script executed:
# Find where generateGetMethod is called and check for allTo availability
grep -n "generateGetMethod" rest/src/main/resources/scripts/GoApiTemplate.groovy
# Also check if allTo is parsed somewhere
grep -n "allTo" rest/src/main/resources/scripts/GoApiTemplate.groovy | head -20Repository: MatheMatrix/zstack
Length of output: 844
🏁 Script executed:
# Check lines 712-715 mentioned in "Also applies to"
sed -n '710,720p' rest/src/main/resources/scripts/GoApiTemplate.groovyRepository: MatheMatrix/zstack
Length of output: 459
GET 方法的 unwrap 分支没有使用解析出的 allTo 值。
generateGetMethod 虽然可以访问类成员变量 allTo（在第 97 行从 @RestResponse 注解解析），但该方法的 unwrap=true 分支中：
- 第 685-687 行：hardcode
"inventory"作为 responseKey - 第 698-700 行：hardcode
"inventory"作为 responseKey - 第 713 行：hardcode
""传给 GetWithSpec
这与代码中其他地方的做法不一致（如第 650、1084 行正确使用 "${allTo}"）。当 GET API 标注了 @RestResponse(allTo = "inventories") 或其他自定义值时，生成的 Go 代码仍会按错误的 key（或空字符串）解包响应，导致数据提取失败。
应该将 allTo 作为参数传入 generateGetMethod，并在所有分支中使用该参数而非 hardcode 值。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/GoApiTemplate.groovy` around lines 685 - 698,
generateGetMethod currently ignores the parsed allTo value and hardcodes
response keys (e.g. "inventory" or "") in the unwrap=true branches; update
generateGetMethod to accept an allTo parameter (the value parsed from
`@RestResponse`) and replace the hardcoded responseKey arguments in the
GetWithRespKey and GetWithSpec calls inside the unwrap branches (references:
generateGetMethod, GetWithRespKey, GetWithSpec, variables clzName, cleanPath,
responseStructName, fieldName) so the generated methods use "${allTo}" instead
of "inventory" or "" across all branches.
The previous reset() only cleared longJobMappings (static), leaving instance-level caches (allApiTemplates, generatedViewStructs, generatedParamStructs, generatedClientMethods, additionalClasses, etc.) intact across repeated generate() calls on the same instance. This caused skipped or duplicate output when TestGenerateGoSDK ran more than once in the same JVM. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
2 participants
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
从 MR !9249 (ZSV-11399) 平移 Go SDK 生成器的 bug fix 到 5.5.12 分支。
GoApiTemplate.groovy
ctx context.Context参数（关键修复，符合 Go 惯例）allTo响应键提取，支持PostWithRespKey正确解包响应valid标志 +isValid()方法，安全校验模板初始化reset()静态方法，防止重复 SDK 生成时状态污染getApiOptPath()处理可选路径GetWithSpec传入allTo参数用于正确响应解包GoInventory.groovy
reset()方法 + 在generate()开始时调用GoApiTemplate.reset()template.isValid()替代template.at != nulltime.Now→time.Now()（4 处缺少括号）generateClientFile()方法（~400 行死代码）base_param_types.go.template
time.Now→time.Now()（缺少括号导致编译警告）Origin
Ported from: http://gitlab.zstack.io/zstackio/zstack/-/merge_requests/9249
sync from gitlab !9477