<fix>[gosdk]: port generator fixes to feature-5.5.22-zcf-temporary#3908
Open
ZStack-Robot wants to merge 4 commits intofeature-5.5.22-zcf-temporaryfrom
Open
<fix>[gosdk]: port generator fixes to feature-5.5.22-zcf-temporary#3908ZStack-Robot wants to merge 4 commits intofeature-5.5.22-zcf-temporaryfrom
ZStack-Robot wants to merge 4 commits intofeature-5.5.22-zcf-temporaryfrom
Conversation
1. Delete-via-POST: detect actionType=="Delete" with httpMethod==POST and generate PostWithRespKey with empty responseKey instead of Post with hardcoded "inventory" key. Handles URL placeholders via buildFullPath. 2. Get-with-inventories: when reply has "inventories" (List) instead of "inventory" (single), use the response struct directly with GetWithRespKey and empty responseKey to unmarshal the full response. Resolves: ZCF-0 Change-Id: I708245d6bd49172fd27488a506dec57d2bfd73ee
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>
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>
Walkthrough生成器添加模板有效性标志与重置，读取并传递 allTo；将所有生成的客户端方法改为接受 context.Context 并调用上下文感知的 cli 辅助函数；为 POST-delete 添加专门分支；修复 time 未使用导入占位写法。 变更Go SDK 生成器上下文与验证重构
Sequence Diagram(s)sequenceDiagram
participant Client
participant GeneratedMethod
participant CLI
Client->>GeneratedMethod: 调用 Method(ctx, params)
GeneratedMethod->>CLI: cli.Post/Get/Put/Delete(ctx, path, body)
CLI-->>GeneratedMethod: 返回 response
GeneratedMethod-->>Client: 返回结果
预估代码审查工作量🎯 4 (复杂) | ⏱️ ~60 分钟 诗
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rest/src/main/resources/scripts/GoApiTemplate.groovy`:
- Around line 992-1053: generateDeleteViaPostMethod currently emits Go methods
without a ctx context.Context parameter and does not pass ctx into
PostWithRespKey; update the generator so every produced function signature
includes ctx context.Context (e.g. func (cli *ZSClient) ${clzName}(ctx
context.Context, ...)) whether or not there are URL placeholders or params,
adjust placeholder param ordering (ctx first) and change every PostWithRespKey
call to pass ctx as the first argument (PostWithRespKey(ctx, ...)); ensure you
update all branches (placeholders vs no placeholders, with and without
param.${clzName}Param) so signatures and calls are consistent with other
generators like generateCreateMethod/generateQueryMethod.
In `@rest/src/main/resources/scripts/GoInventory.groovy`:
- Around line 109-128: The reset() method currently clears many caches but omits
the viewStructNameMap map, so stale Class->String mappings persist across
generations; update GoInventory.reset() to also clear viewStructNameMap (the
Map<Class, String> defined earlier) by invoking its clear() alongside the other
cache clears so the view struct name cache is reset before each generate() run.
🪄 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: b4293a39-916d-49c6-8934-a1490f13b214
📒 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
+109
to
+128
| /** | ||
| * Reset all static and instance state for clean re-generation. | ||
| * Must be called before each generate() to avoid stale caches | ||
| * causing skipped or duplicate output across repeated runs. | ||
| */ | ||
| void reset() { | ||
| longJobMappings.clear() | ||
| allApiTemplates.clear() | ||
| inventories.clear() | ||
| markedInventories.clear() | ||
| additionalClasses.clear() | ||
| generatedViewStructs.clear() | ||
| generatedViewFiles.clear() | ||
| paramNestedTypes.clear() | ||
| generatedParamStructs.clear() | ||
| generatedClientMethods.clear() | ||
| generatingForParam = false | ||
| currentGeneratingClass = null | ||
| logger.warn("[GoSDK] Reset GoInventory state (static + instance)") | ||
| } |
There was a problem hiding this comment.
reset() 方法遗漏清理 viewStructNameMap
reset() 方法未清理 viewStructNameMap（第 32 行定义的 Map<Class, String>）。此缓存存储类到视图结构体名称的映射，若不清理可能导致跨生成运行时残留过期映射。
🐛 建议修复
void reset() {
longJobMappings.clear()
allApiTemplates.clear()
inventories.clear()
markedInventories.clear()
+ viewStructNameMap.clear()
additionalClasses.clear()
generatedViewStructs.clear()
generatedViewFiles.clear()
paramNestedTypes.clear()
generatedParamStructs.clear()
generatedClientMethods.clear()
generatingForParam = false
currentGeneratingClass = null
logger.warn("[GoSDK] Reset GoInventory state (static + instance)")
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rest/src/main/resources/scripts/GoInventory.groovy` around lines 109 - 128,
The reset() method currently clears many caches but omits the viewStructNameMap
map, so stale Class->String mappings persist across generations; update
GoInventory.reset() to also clear viewStructNameMap (the Map<Class, String>
defined earlier) by invoking its clear() alongside the other cache clears so the
view struct name cache is reset before each generate() run.
Generate POST APIs with URL placeholders as explicit Go method
parameters and build the request path with fmt.Sprintf. Also use
PostWithRespKey for POST responses so APIs returning plain Event/Reply
without inventory are unmarshaled from the full response body.
This fixes AddIAM2VirtualIDsToGroup and other POST APIs whose REST path
contains placeholders such as {uuid}, {groupUuid}, or multiple path args.
Resolves: ZCF-0
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rest/src/main/resources/scripts/GoApiTemplate.groovy (1)
1377-1383:⚠️ Potential issue | 🟠 Major | ⚡ Quick win异步生成方法中
ctx参数未传递给cli.PostWithAsync()。方法签名已接受
ctx context.Context参数（第1377行），但后续对cli.PostWithAsync(...)的调用（第1383行）并未将其传递。若 PostWithAsync 已更新为 context-aware 签名，生成的 Go 代码将无法编译；即使 helper 尚未更新，取消和超时功能也无法正常工作。建议修改：
最小修复
- builder.append("\tapiId, err := cli.PostWithAsync(resource, responseKey, params, retVal, true)\n") + builder.append("\tapiId, err := cli.PostWithAsync(ctx, resource, responseKey, params, retVal, true)\n")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rest/src/main/resources/scripts/GoApiTemplate.groovy` around lines 1377 - 1383, The generated async method for ZSClient (func (cli *ZSClient) ${asyncMethodName}(ctx context.Context, ...)) accepts a ctx but does not forward it to cli.PostWithAsync; update the call to cli.PostWithAsync to pass the context (e.g., include ctx as the first argument) so the generated method forwards cancellation/timeouts and matches a context-aware PostWithAsync signature; modify the template around the PostWithAsync invocation in GoApiTemplate.groovy to include ctx when building the call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rest/src/main/resources/scripts/GoApiTemplate.groovy`:
- Around line 1032-1036: 生成器在生成 DELETE 方法（当 useSpec 为 false）时始终在签名中加入 uuid
string 并调用 cli.Delete，导致无占位符路径（例如 /delete/sso/client）会多拼一个 path
segment；在模板里的分支需依照占位符数量拆成两条：当没有占位符时，生成 func (cli *ZSClient) ${clzName}(ctx
context.Context, deleteMode param.DeleteMode) error 并调用 cli.Delete(ctx,
"${cleanPath}", "", string(deleteMode)) 或提供不传 uuid 的重载；当只有一个占位符时，保持现有签名 func
(cli *ZSClient) ${clzName}(ctx context.Context, uuid string, deleteMode
param.DeleteMode) error 并调用 cli.Delete(ctx, "${cleanPath}", uuid,
string(deleteMode))；修改模板中 useSpec 分支逻辑和生成的 ZSClient.${clzName}
方法以根据占位符数选择正确的签名和传参。
---
Outside diff comments:
In `@rest/src/main/resources/scripts/GoApiTemplate.groovy`:
- Around line 1377-1383: The generated async method for ZSClient (func (cli
*ZSClient) ${asyncMethodName}(ctx context.Context, ...)) accepts a ctx but does
not forward it to cli.PostWithAsync; update the call to cli.PostWithAsync to
pass the context (e.g., include ctx as the first argument) so the generated
method forwards cancellation/timeouts and matches a context-aware PostWithAsync
signature; modify the template around the PostWithAsync invocation in
GoApiTemplate.groovy to include ctx when building the call.
🪄 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: 08c0c647-4713-4b45-91e7-8ec78e681b35
📒 Files selected for processing (1)
rest/src/main/resources/scripts/GoApiTemplate.groovy
Comment on lines
1032
to
1036
| if (!useSpec) { | ||
| // Single or no placeholder: use the standard Delete method | ||
| return """func (cli *ZSClient) ${clzName}(uuid string, deleteMode param.DeleteMode) error { | ||
| \treturn cli.Delete("${cleanPath}", uuid, string(deleteMode)) | ||
| return """func (cli *ZSClient) ${clzName}(ctx context.Context, uuid string, deleteMode param.DeleteMode) error { | ||
| \treturn cli.Delete(ctx, "${cleanPath}", uuid, string(deleteMode)) | ||
| } |
There was a problem hiding this comment.
DELETE 无占位符时不应继续生成 uuid 参数。
这里的注释写的是 “Single or no placeholder”，但生成的方法签名始终包含 uuid string，并把它传给 cli.Delete。对 /delete/sso/client 这类无占位符路径，SDK 会凭空多拼一个 path segment，最终请求 URL 会错误。
🔧 建议拆分 0/1 个占位符分支
- if (!useSpec) {
- // Single or no placeholder: use the standard Delete method
- return """func (cli *ZSClient) ${clzName}(ctx context.Context, uuid string, deleteMode param.DeleteMode) error {
-\treturn cli.Delete(ctx, "${cleanPath}", uuid, string(deleteMode))
-}
-"""
+ if (!useSpec) {
+ if (placeholders.isEmpty()) {
+ return """func (cli *ZSClient) ${clzName}(ctx context.Context, deleteMode param.DeleteMode) error {
+\treturn cli.Delete(ctx, "${cleanPath}", "", string(deleteMode))
+}
+"""
+ }
+ return """func (cli *ZSClient) ${clzName}(ctx context.Context, uuid string, deleteMode param.DeleteMode) error {
+\treturn cli.Delete(ctx, "${cleanPath}", uuid, string(deleteMode))
+}
+"""
} else {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rest/src/main/resources/scripts/GoApiTemplate.groovy` around lines 1032 -
1036, 生成器在生成 DELETE 方法（当 useSpec 为 false）时始终在签名中加入 uuid string 并调用
cli.Delete，导致无占位符路径（例如 /delete/sso/client）会多拼一个 path
segment；在模板里的分支需依照占位符数量拆成两条：当没有占位符时，生成 func (cli *ZSClient) ${clzName}(ctx
context.Context, deleteMode param.DeleteMode) error 并调用 cli.Delete(ctx,
"${cleanPath}", "", string(deleteMode)) 或提供不传 uuid 的重载；当只有一个占位符时，保持现有签名 func
(cli *ZSClient) ${clzName}(ctx context.Context, uuid string, deleteMode
param.DeleteMode) error 并调用 cli.Delete(ctx, "${cleanPath}", uuid,
string(deleteMode))；修改模板中 useSpec 分支逻辑和生成的 ZSClient.${clzName}
方法以根据占位符数选择正确的签名和传参。
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
3 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.
Cherry-pick Go SDK generator fixes from feature-zcf-v0.1 to feature-5.5.22-zcf-temporary:
Purpose:
Note:
sync from gitlab !9792