<feature>[rest]: add go-sdk generator for zsv#3401
Open
ZStack-Robot wants to merge 1 commit intozsv_5.0.0from
Open
<feature>[rest]: add go-sdk generator for zsv#3401ZStack-Robot wants to merge 1 commit intozsv_5.0.0from
ZStack-Robot wants to merge 1 commit intozsv_5.0.0from
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough将 Go SDK 生成器从简陋模板扩展为完整的生成子系统：新增并扩展 GoApiTemplate、GoInventory，实现多阶段视图/参数/操作文件生成、异步与路径占位处理；引入测试生成器、数据结构生成重构、多个 Go 模板文件及详细中文开发规范与示例。 Changes
Sequence Diagram(s)sequenceDiagram
participant RestServer
participant GoInventory
participant GoApiTemplate
participant TemplateEngine
participant Filesystem
RestServer->>GoInventory: 触发生成流程 (discover APIs, inventories)
GoInventory->>GoApiTemplate: prepareApiTemplates() (创建/缓存模板)
GoApiTemplate->>GoApiTemplate: 解析 RestRequest, 解析 action/resource, 找 inventory
GoApiTemplate->>TemplateEngine: render 方法/视图/参数 模板
TemplateEngine->>Filesystem: 写出 SdkFile（views, params, actions, tests）
GoInventory->>Filesystem: 写出合并/附加视图和基类文件
GoInventory->>GoTestTemplate: 触发测试文件生成（按资源分组）
GoTestTemplate->>Filesystem: 写出测试文件
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 分钟 诗歌
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (5)
rest/src/main/resources/scripts/ZStack SDK Go 开发规范与标准.md-533-533 (1)
533-533:⚠️ Potential issue | 🟡 Minor测试目录命名不一致
此处提到测试文件放在
pkg/test1/目录，但在项目结构说明（Line 144）中使用的是pkg/integration-test/。建议统一目录命名。🔧 建议的修复
-测试文件放在 `pkg/test1/` 目录下，命名格式：`{resource}_test.go` +测试文件放在 `pkg/integration-test/` 目录下，命名格式：`{resource}_test.go`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md at line 533, The documentation currently references two different test directories ("pkg/test1" in the snippet and "pkg/integration-test" earlier) which is inconsistent; pick the canonical name (preferably "pkg/integration-test" per the project structure) and update all occurrences in the document so the test file guidance (naming pattern `{resource}_test.go`) and the project structure section use the same directory name; search for and replace "pkg/test1" with "pkg/integration-test" (or vice versa if you prefer the other name) to ensure consistency across the file.rest/src/main/resources/scripts/ZStack SDK Go 开发规范与标准.md-29-42 (1)
29-42:⚠️ Potential issue | 🟡 Minor变量重复声明会导致编译错误
在同一个
main()函数中，两个初始化示例都使用了变量名cli，这会导致 Go 编译错误（变量重复声明）。建议使用注释说明这是两种不同的认证方式，用户应选择其中一种。📝 建议的修复
func main() { // 方式一：使用 AccessKey 认证（推荐） cli := client.NewZSClient( client.DefaultZSConfig("192.168.1.100", "/zstack"). AccessKey("your-access-key-id", "your-access-key-secret"). Debug(true), ) + + // ... 使用 cli 进行操作 ... +} - // 方式二：使用账号密码认证（明文密码,客户端自动 SHA512 加密） +// 或者使用账号密码认证 +func mainAlternative() { + // 方式二：使用账号密码认证（明文密码，客户端自动 SHA512 加密） cli := client.NewZSClient( client.DefaultZSConfig("192.168.1.100", "/zstack"). Login("admin", "password"). // ✅ 传入明文密码即可 Debug(true), ) + + // ... 使用 cli 进行操作 ... }或者简化为注释说明：
func main() { // 方式一：使用 AccessKey 认证（推荐） cli := client.NewZSClient( client.DefaultZSConfig("192.168.1.100", "/zstack"). AccessKey("your-access-key-id", "your-access-key-secret"). Debug(true), ) - // 方式二：使用账号密码认证（明文密码，客户端自动 SHA512 加密） - cli := client.NewZSClient( - client.DefaultZSConfig("192.168.1.100", "/zstack"). - Login("admin", "password"). // ✅ 传入明文密码即可 - Debug(true), - ) + // 或者方式二：使用账号密码认证（明文密码，客户端自动 SHA512 加密） + // cli := client.NewZSClient( + // client.DefaultZSConfig("192.168.1.100", "/zstack"). + // Login("admin", "password"). // ✅ 传入明文密码即可 + // Debug(true), + // ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md around lines 29 - 42, The examples declare the same variable name cli twice in the same main() which causes a Go compile error; update the snippet so the two authentication examples do not collide by either renaming the second variable (e.g., cliAccessKey vs cliLogin), separating them into distinct commented examples, or showing only one example and turning the other into explanatory comments; locate usages of client.NewZSClient, client.DefaultZSConfig, AccessKey and Login in the snippet and ensure only one active cli declaration remains in the same scope.rest/src/main/resources/scripts/SDK生成器开发指南.md-133-133 (1)
133-133:⚠️ Potential issue | 🟡 Minor代码块缺少语言标记，建议补全
这会触发 markdownlint
MD040，也会降低代码高亮可读性。Also applies to: 359-359
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SDK生成器开发指南.md` at line 133, One or more fenced code blocks in SDK生成器开发指南.md are missing language identifiers (plain ```), which triggers markdownlint MD040 and reduces syntax highlighting; find the bare ``` fences (the ones at the two reported locations) and replace them with the appropriate language-marked fences (for example ```bash, ```json, ```java, etc., matching the snippet content) so each code block includes a language tag.rest/src/main/resources/scripts/templates/client.go.template-206-211 (1)
206-211:⚠️ Potential issue | 🟡 Minor
retVal == nil时返回值不一致当
retVal == nil时，函数返回nil, nil，但函数签名声明返回(http.Header, error)。调用者可能期望即使不需要解析响应体，也能获得响应头。这种不一致可能导致调用者丢失响应头信息。if retVal == nil { - return nil, nil + return respHeader, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/client.go.template` around lines 206 - 211, When retVal == nil the function currently returns nil, nil which conflicts with the declared return types (http.Header, error) and drops the response headers; change the branch that checks retVal to return respHeader, nil instead so callers still receive respHeader even when there's no body to unmarshal (update the block that references retVal, respHeader and resp.Unmarshal).rest/src/main/resources/scripts/templates/client.go.template-222-227 (1)
222-227:⚠️ Potential issue | 🟡 Minor
Zql方法存在相同的返回值不一致问题与
hql方法相同的问题，当retVal == nil时应返回respHeader而不是nil。if retVal == nil { - return nil, nil + return respHeader, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/client.go.template` around lines 222 - 227, In the Zql method, the early-return for retVal == nil returns nil, nil which is inconsistent with hql; change the branch to return respHeader, nil instead. Locate the Zql function where it checks "if retVal == nil" and replace the nil response with respHeader so the function returns (respHeader, nil) and otherwise continues to "return respHeader, resp.Unmarshal(retVal, unMarshalKeys...)".
🧹 Nitpick comments (9)
rest/src/main/resources/scripts/SdkApiTemplate.groovy (3)
96-114: 递归resolveAllClasses方法存在潜在的栈溢出风险
resolveAllClasses方法递归调用自身来处理新发现的类。如果存在循环依赖或非常深的类层次结构，可能导致StackOverflowError。建议使用迭代方式替代递归。建议使用迭代方式
def resolveAllClasses() { - if (laterResolvedClasses.isEmpty()) { - return - } - - Set<Class> toResolve = [] - toResolve.addAll(laterResolvedClasses) - - toResolve.each { Class clz -> - try { - resolveClass(clz) - laterResolvedClasses.remove(clz) - } catch (Throwable t) { - throw new CloudRuntimeException("failed to generate SDK for the class[${clz.getName()}]", t) + while (!laterResolvedClasses.isEmpty()) { + Set<Class> toResolve = new HashSet<>(laterResolvedClasses) + toResolve.each { Class clz -> + try { + resolveClass(clz) + laterResolvedClasses.remove(clz) + } catch (Throwable t) { + throw new CloudRuntimeException("failed to generate SDK for the class[${clz.getName()}]", t) + } } } - - resolveAllClasses() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` around lines 96 - 114, The recursive resolveAllClasses method can cause StackOverflowError for deep or cyclical class dependencies; replace the recursion with an iterative loop: repeatedly process a snapshot of laterResolvedClasses (e.g., while it is not empty, copy to a local Set/Class list, iterate calling resolveClass(clz) and remove processed entries from laterResolvedClasses), catching Throwable and wrapping it in CloudRuntimeException with the same message, and continue until no new classes remain; ensure you reference and update laterResolvedClasses and call resolveClass(Class) rather than recursively calling resolveAllClasses().
177-183: 生成的extends子句可能产生无效语法当父类与子类在同一包中时，生成的代码会包含完整包名，如
extends org.zstack.sdk.SomeClass。这虽然技术上正确，但如果父类的sdkClassName与simpleName不同，可能会产生不一致的引用。建议确保
extends子句使用与导入语句一致的类名。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` around lines 177 - 183, The generated extends clause may not match the import style; change the extends generation in SdkApiTemplate (where getTargetClassName(clz) and clz.superclass are used) so it uses the same identifier as produced by the imports list: if the parent's fully-qualified name appears in the imports collection, emit the parent's simpleName (or sdkClassName if you already use that mapping) for the extends clause; otherwise emit the fully-qualified name. Ensure you reference clz.superclass.simpleName (or the existing sdkClassName mapping) the same way imports.collect uses it so extends and import remain consistent.
79-91: 生成的SourceClassMap.java使用原始类型HashMap生成的代码使用了原始类型
HashMap而不是带泛型的HashMap<String, String>。虽然代码可以编译，但这会产生编译警告，且不符合现代 Java 的最佳实践。建议的修改
- public final static HashMap<String, String> srcToDstMapping = new HashMap() { + public final static HashMap<String, String> srcToDstMapping = new HashMap<String, String>() { { ${srcToDst.join("\n")} } }; - public final static HashMap<String, String> dstToSrcMapping = new HashMap() { + public final static HashMap<String, String> dstToSrcMapping = new HashMap<String, String>() { { ${dstToSrc.join("\n")} } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` around lines 79 - 91, The generated SourceClassMap class uses raw HashMap types for srcToDstMapping and dstToSrcMapping which emits warnings; update the anonymous-map instantiations to use generics (e.g., new HashMap<String, String>() or the diamond operator new HashMap<>()) so both fields and their initializers are typed as HashMap<String, String>, keeping the existing instance initializer blocks intact for srcToDstMapping and dstToSrcMapping.rest/src/main/resources/scripts/GoTestTemplate.groovy (3)
31-32: 注释编号错误第31行的注释编号为 "2." 但应该是 "3."，因为之前已经有 "1." 和 "2." 了。
- // 2. Per-resource test files + // 3. Per-resource test files🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoTestTemplate.groovy` around lines 31 - 32, 注释编号错误：在 GoTestTemplate.groovy 中对资源测试文件的注释行（位于使用 groupApisByResource() 并赋值给 resourceMap 的那一行）将注释前缀 "2." 更正为 "3."，确保序号连续（之前已有 "1." 和 "2."），只需修改该注释文本即可。
148-151:timePtr函数静默忽略解析错误可能隐藏测试问题当时间字符串格式不正确时，
time.Parse的错误被忽略，函数会返回零值时间的指针。这可能导致测试在应该失败时仍然通过。// timePtr parses a time string and returns a pointer. func timePtr(s string) *time.Time { \tt, _ := time.Parse(time.RFC3339, s) +\tif t.IsZero() { +\t\treturn nil +\t} \treturn &t }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoTestTemplate.groovy` around lines 148 - 151, The timePtr helper currently swallows parse errors (timePtr) which can hide failing tests; change its signature to return (*time.Time, error) instead of *time.Time, call time.Parse and if err != nil return (nil, err), otherwise return &t, nil, and update any callers in tests to handle the error (fail the test or propagate) so invalid RFC3339 inputs no longer produce a zero-value time silently.
21-57: 日志级别使用不当
logger.warn用于记录正常操作日志（如开始生成、完成生成等）不太合适。warn级别应该用于警告性质的消息，而正常操作流程应该使用logger.info或logger.debug。建议的修改
List<SdkFile> generate() { def files = [] - logger.warn("[GoSDK-Test] Starting test generation...") + logger.info("[GoSDK-Test] Starting test generation...") ... - logger.warn("[GoSDK-Test] Grouped ${resourceMap.size()} resources for test generation") + logger.info("[GoSDK-Test] Grouped ${resourceMap.size()} resources for test generation") ... - logger.warn("[GoSDK-Test] Test generation complete. Generated ${files.size()} test files") + logger.info("[GoSDK-Test] Test generation complete. Generated ${files.size()} test files")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoTestTemplate.groovy` around lines 21 - 57, The method generate() is using logger.warn for normal operational messages; replace those calls with logger.info (or logger.debug if you prefer verbose output) so normal start/complete/resource-count logs are not treated as warnings. Update the three logger.warn calls in generate()—the "[GoSDK-Test] Starting test generation..." log, the resource grouping log that includes "${resourceMap.size()}", and the completion log "[GoSDK-Test] Test generation complete. Generated ${files.size()} test files"—to logger.info and keep the message text unchanged.rest/src/main/resources/scripts/GoApiTemplate.groovy (2)
42-56: 静态集合的线程安全问题
generatedParamFiles、generatedActionFiles、generatedViewFiles、groupedApiNames和skippedApis都是静态集合，在多线程环境下可能存在并发修改问题。虽然 SDK 生成通常是单线程的，但使用线程安全的集合会更稳健。建议使用线程安全集合
- private static Set<String> generatedParamFiles = new HashSet<>() - private static Set<String> generatedActionFiles = new HashSet<>() - private static Set<String> generatedViewFiles = new HashSet<>() + private static Set<String> generatedParamFiles = Collections.synchronizedSet(new HashSet<>()) + private static Set<String> generatedActionFiles = Collections.synchronizedSet(new HashSet<>()) + private static Set<String> generatedViewFiles = Collections.synchronizedSet(new HashSet<>()) ... - static Set<String> groupedApiNames = new HashSet<>() + static Set<String> groupedApiNames = Collections.synchronizedSet(new HashSet<>())🤖 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 42 - 56, The static collections generatedParamFiles, generatedActionFiles, generatedViewFiles, groupedApiNames and skippedApis and the map longJobMappings are not thread-safe; replace their initializations with thread-safe variants (e.g., use ConcurrentHashMap.newKeySet() or Collections.newSetFromMap(new ConcurrentHashMap<>()) for sets and ConcurrentHashMap<Class,Class> for longJobMappings) so concurrent access during generation won't race; update the declarations for generatedParamFiles, generatedActionFiles, generatedViewFiles, groupedApiNames, skippedApis and longJobMappings to use those concurrent types (keep the same variable names and types where possible) and ensure imports/initializers reflect the concurrent implementations.
584-589: 简单的复数化逻辑可能产生不正确的变量名当前的复数化逻辑只处理了以 "y" 结尾的单词（改为 "ies"）和一般情况（加 "s"）。但这无法正确处理许多英语单词，例如：
- "Status" → "Statuss"（应该是 "Statuses"）
- "Index" → "Indexs"（应该是 "Indices" 或 "Indexes"）
- "Policy" → "Policies"（正确）
建议添加更多的复数化规则或使用现有的复数化库。
🤖 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 584 - 589, The current pluralization in GoApiTemplate.groovy incorrectly forms some plurals (e.g., resourceName -> varName logic: checks endsWith("y") and else add "s") causing "Status" -> "Statuss" and "Index" -> "Indexs"; update the pluralization used to produce varName by replacing the ad-hoc rules with either a small extended rule set (handle endings like "us"->"es"/"ses", "s"/"x"/"z"/"ch"/"sh" -> add "es", "is"->"es"/irregulars like "index"->"indices" or "indexes", preserve already plural forms) or integrate a well-tested pluralization library and call it where varName is computed (the code around the varName = resourceName... block), ensuring varName is correctly lowercased and pluralized for common irregulars.rest/src/main/resources/scripts/GoInventory.groovy (1)
42-42: 静态longJobMappings字段的线程安全问题
longJobMappings被声明为静态字段，但在scanLongJobMappings()方法中被修改。如果多个线程同时调用生成器，可能会导致数据竞争。考虑使用ConcurrentHashMap或确保单线程访问。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoInventory.groovy` at line 42, The static field longJobMappings is not thread-safe because it's a HashMap and is mutated by scanLongJobMappings(); change its declaration to use a thread-safe map (e.g., new ConcurrentHashMap<>()) or protect access in scanLongJobMappings() (and any other readers/writers) with synchronization; update the declaration of longJobMappings and ensure scanLongJobMappings() performs only thread-safe put/get operations (or use synchronized blocks) so concurrent generator invocations cannot race.
🤖 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 379-389: The getApiOptPath method is returning "path" instead of
the optional path variable "opt"; update the function (getApiOptPath) to use the
first optional path (opt) returned by at.optionalPaths() instead of "path" so
the method truly returns the optional route; specifically, change both return
branches to concatenate "v1" with opt (use "v1" + opt when opt.startsWith("/")
and "v1/" + opt otherwise) while keeping the existing empty-array guard.
- Around line 91-92: The code assumes RestResponse annotation exists and
dereferences restResponse without checking for null; update the logic around
responseClass.getAnnotation(RestResponse) to handle a null return (e.g., check
if restResponse == null) before calling restResponse.allTo(), and set allTo to a
safe default or compute it differently when the annotation is missing; reference
responseClass, getAnnotation(RestResponse), restResponse, and allTo when making
the change.
In `@rest/src/main/resources/scripts/GoInventory.groovy`:
- Line 299: The generated Go snippet incorrectly references the function
identifier time.Now without calling it, causing a compile error; update the
string passed to content.append (the line that currently contains "var _ =
time.Now // avoid unused import") to call the function (use time.Now()) so the
import is used and the code compiles; locate the content.append invocation that
adds the "var _ = time.Now" stub and change it to include parentheses.
- Around line 1740-1741: The current placeholder uses struct literal syntax for
the map type alias MapView (defined in generateBaseViewFile), which is invalid
in Go; change the unused-import placeholder for MapView in GoInventory.groovy to
initialize it as a map (e.g., use nil or make) instead of using `{}`—for example
replace the `view.MapView{}` placeholder with a nil-typed MapView or with a
make(view.MapView) style initialization so the import remains referenced without
causing a compile error.
- Around line 2093-2094: 在 generateParamFiles 方法中修复与前面相同的“time.Now”语法错误：定位
generateParamFiles 中构建脚本时插入的行（查找 content.append(...) 包含 "import \"time\""
和随后的占位符行），将占位用法改为对 time.Now 的调用或删除不必要的导入；具体来说，把类似的占位表达（例如 var _ =
time.Now）改为正确的调用形式（var _ = time.Now()）或直接移除该占位以避免未使用导入警告。
- Around line 1765-1766: other_views.go has the same syntax bug as
GoInventory.groovy: the unused-import workaround uses "time.Now" without calling
the function; locate the occurrence of the unused import workaround (the
statement using the blank identifier, e.g. var _ = time.Now) in other_views.go
and fix it by calling the function (use time.Now()) or, if time is truly unused,
remove the import and the workaround line entirely; ensure the symbol to edit is
the var _ = time.Now entry in other_views.go.
- Around line 1917-1918: The generateViewFiles method also appends the invalid
Go token "var _ = time.Now" causing the same syntax error; update the
content.append call in generateViewFiles that produces "var _ = time.Now //
avoid unused import" to use the function call "var _ = time.Now()" (i.e., add
the parentheses) or remove the unused-import workaround entirely, ensuring the
import "time" is either used properly or omitted; look for the
content.append(...) lines that build the import and the var _ = time.Now string
inside generateViewFiles and correct them accordingly.
- Around line 2062-2063: generateActionFiles() currently uses a composite
literal "var _ = view.MapView{}" which is invalid for interfaces/abstract types;
replace that line with a typed-nil or concrete-implementation sentinel to avoid
the unused-import trick, e.g. declare "var _ view.MapView =
(*view.SomeConcreteMapView)(nil)" (or if MapView is a struct use "var _ =
(*view.MapView)(nil)"), choosing the actual concrete type used in your codebase
so the import is referenced without constructing an invalid literal.
In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy`:
- Line 2103: 当前使用 responseClass.getDeclaredField(at.allTo())
只在当前类查找字段，会在字段位于父类时失败；请改为使用 FieldUtils.getField(responseClass, at.allTo(),
true)（或等效支持继承层级的工具方法）来查找字段并在结果为 null 时妥善处理，修改涉及的标识符包括 fsToAdd、at.allTo() 与
responseClass.getDeclaredField(...)，确保与同文件其它分支一致的字段查找行为。
- Around line 1931-1933: 在 RestDocumentationGenerator.groovy 的 generate() 实现中当前的
catch (Exception e) 仅调用 logger.warn(e.message, e) 导致异常被吞掉并可能让 generate() 返回
null；请不要吞掉文档生成异常：在捕获到异常时要在记录日志后重新抛出（或将其包装成一个合适的运行时异常并抛出），或者直接删除该 try/catch
让异常向上冒泡；定位到包含 logger.warn(e.message, e) 的 catch 块并修改为记录详细错误后抛出异常，以保证调用方能感知并处理失败。
In `@rest/src/main/resources/scripts/templates/base_params.go.template`:
- Around line 87-97: ConvertStruct2UrlValues currently calls
reflect.TypeOf(param).Kind() before checking param == nil which panics for nil;
move the nil check first and then validate that param is a non-nil pointer to a
struct. Specifically: in ConvertStruct2UrlValues, first if param == nil return
error; then get t := reflect.TypeOf(param) and verify t.Kind() == reflect.Ptr
and t.Elem().Kind() == reflect.Struct; also use v := reflect.ValueOf(param) and
check v.IsNil() only after confirming it's a pointer; after those checks proceed
to call structs.New(param) and set s.TagName = "json".
In `@rest/src/main/resources/scripts/templates/example_main.go`:
- Around line 28-43: After a successful cli.Login(ctx) the session is not
guaranteed to be released because multiple early returns exist; immediately
after checking err == nil assign the returned session and register a deferred
logout (e.g., defer cli.Logout(ctx, sessionView) or defer cli.Logout(ctx,
sessionView.Uuid) depending on the Logout signature) so the session is always
cleaned up on function exit; add this same defer placement for the other login
occurrences referenced (the block around QueryVmInstance and the later block at
the other occurrence) and ensure Logout is safe to call with the session value
used.
In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md:
- Around line 641-668: The API method templates for Create{Resource},
Query{Resource}, Get{Resource} and Destroy{Resource} are calling the HTTP
helpers without the context parameter; update each call to pass the incoming ctx
as the first argument (e.g., change cli.Post("v1/{resources}", ...) to
cli.Post(ctx, "v1/{resources}", ...), cli.List(...) to cli.List(ctx, ...),
cli.Get(...) to cli.Get(ctx, ...), and cli.Delete(...) to cli.Delete(ctx, ...))
so the method signatures match the HTTP helper usage in the example (see
Create{Resource}, Query{Resource}, Get{Resource}, Destroy{Resource} and the
corresponding cli.Post/cli.List/cli.Get/cli.Delete symbols).
- Around line 280-304: The doc shows a custom ZStackTime type and BaseTimeView
struct but later says they should be removed; update this section to eliminate
ambiguity by either (A) adding a clear deprecation/status note above the
BaseTimeView example stating that ZStackTime and BaseTimeView are deprecated and
the recommended pattern is to use native time.Time, or (B) replace the example
so VmInstanceInventoryView (and any examples) embed time.Time fields directly
(e.g., CreateDate time.Time, LastOpDate time.Time) and remove references to
ZStackTime; reference the symbols ZStackTime, BaseTimeView and
VmInstanceInventoryView when making the change so readers see the deprecation
and the replacement pattern.
- Around line 1-1090:
将文档中“现行标准”与“待实施改进（第11节）”明确区分：在文档开头增加版本/状态声明并标注“第1-10节为当前标准、11节为改进计划”，将
DefaultZSConfig/生成器相关示例保持在各自区块；统一测试目录表述，将所有出现的 pkg/test1/ 和
pkg/integration-test/ 规范化为同一目录名（例如
pkg/integration-test/），并修改示例和“测试文件位置”条目中的相应引用；修正 BaseInfoView 与生成器规则中关于 name
字段必填性的冲突，确保 BaseInfoView 定义（BaseInfoView、VmInstanceInventoryView 示例和“判断规则”）一致地将
name 标记为可选或必填（并同步更新“指针类型优化”与“嵌入规则”段落中对 name 的说明）。
- Around line 827-833: BaseInfoView currently declares Name as a pointer with
`omitempty` (optional) while the docs state "uuid and name are always required";
either make Name a non-pointer string and remove `omitempty` in the BaseInfoView
struct to enforce requiredness, or—if some resources truly allow missing
names—update the documentation/comment that claims "uuid and name 始终必填" to
reflect that name can be optional; adjust the BaseInfoView struct and the
corresponding comment text so they are consistent (symbols: BaseInfoView, Name,
UUID).
---
Minor comments:
In `@rest/src/main/resources/scripts/SDK生成器开发指南.md`:
- Line 133: One or more fenced code blocks in SDK生成器开发指南.md are missing language
identifiers (plain ```), which triggers markdownlint MD040 and reduces syntax
highlighting; find the bare ``` fences (the ones at the two reported locations)
and replace them with the appropriate language-marked fences (for example
```bash, ```json, ```java, etc., matching the snippet content) so each code
block includes a language tag.
In `@rest/src/main/resources/scripts/templates/client.go.template`:
- Around line 206-211: When retVal == nil the function currently returns nil,
nil which conflicts with the declared return types (http.Header, error) and
drops the response headers; change the branch that checks retVal to return
respHeader, nil instead so callers still receive respHeader even when there's no
body to unmarshal (update the block that references retVal, respHeader and
resp.Unmarshal).
- Around line 222-227: In the Zql method, the early-return for retVal == nil
returns nil, nil which is inconsistent with hql; change the branch to return
respHeader, nil instead. Locate the Zql function where it checks "if retVal ==
nil" and replace the nil response with respHeader so the function returns
(respHeader, nil) and otherwise continues to "return respHeader,
resp.Unmarshal(retVal, unMarshalKeys...)".
In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md:
- Line 533: The documentation currently references two different test
directories ("pkg/test1" in the snippet and "pkg/integration-test" earlier)
which is inconsistent; pick the canonical name (preferably
"pkg/integration-test" per the project structure) and update all occurrences in
the document so the test file guidance (naming pattern `{resource}_test.go`) and
the project structure section use the same directory name; search for and
replace "pkg/test1" with "pkg/integration-test" (or vice versa if you prefer the
other name) to ensure consistency across the file.
- Around line 29-42: The examples declare the same variable name cli twice in
the same main() which causes a Go compile error; update the snippet so the two
authentication examples do not collide by either renaming the second variable
(e.g., cliAccessKey vs cliLogin), separating them into distinct commented
examples, or showing only one example and turning the other into explanatory
comments; locate usages of client.NewZSClient, client.DefaultZSConfig, AccessKey
and Login in the snippet and ensure only one active cli declaration remains in
the same scope.
---
Nitpick comments:
In `@rest/src/main/resources/scripts/GoApiTemplate.groovy`:
- Around line 42-56: The static collections generatedParamFiles,
generatedActionFiles, generatedViewFiles, groupedApiNames and skippedApis and
the map longJobMappings are not thread-safe; replace their initializations with
thread-safe variants (e.g., use ConcurrentHashMap.newKeySet() or
Collections.newSetFromMap(new ConcurrentHashMap<>()) for sets and
ConcurrentHashMap<Class,Class> for longJobMappings) so concurrent access during
generation won't race; update the declarations for generatedParamFiles,
generatedActionFiles, generatedViewFiles, groupedApiNames, skippedApis and
longJobMappings to use those concurrent types (keep the same variable names and
types where possible) and ensure imports/initializers reflect the concurrent
implementations.
- Around line 584-589: The current pluralization in GoApiTemplate.groovy
incorrectly forms some plurals (e.g., resourceName -> varName logic: checks
endsWith("y") and else add "s") causing "Status" -> "Statuss" and "Index" ->
"Indexs"; update the pluralization used to produce varName by replacing the
ad-hoc rules with either a small extended rule set (handle endings like
"us"->"es"/"ses", "s"/"x"/"z"/"ch"/"sh" -> add "es", "is"->"es"/irregulars like
"index"->"indices" or "indexes", preserve already plural forms) or integrate a
well-tested pluralization library and call it where varName is computed (the
code around the varName = resourceName... block), ensuring varName is correctly
lowercased and pluralized for common irregulars.
In `@rest/src/main/resources/scripts/GoInventory.groovy`:
- Line 42: The static field longJobMappings is not thread-safe because it's a
HashMap and is mutated by scanLongJobMappings(); change its declaration to use a
thread-safe map (e.g., new ConcurrentHashMap<>()) or protect access in
scanLongJobMappings() (and any other readers/writers) with synchronization;
update the declaration of longJobMappings and ensure scanLongJobMappings()
performs only thread-safe put/get operations (or use synchronized blocks) so
concurrent generator invocations cannot race.
In `@rest/src/main/resources/scripts/GoTestTemplate.groovy`:
- Around line 31-32: 注释编号错误：在 GoTestTemplate.groovy 中对资源测试文件的注释行（位于使用
groupApisByResource() 并赋值给 resourceMap 的那一行）将注释前缀 "2." 更正为 "3."，确保序号连续（之前已有 "1."
和 "2."），只需修改该注释文本即可。
- Around line 148-151: The timePtr helper currently swallows parse errors
(timePtr) which can hide failing tests; change its signature to return
(*time.Time, error) instead of *time.Time, call time.Parse and if err != nil
return (nil, err), otherwise return &t, nil, and update any callers in tests to
handle the error (fail the test or propagate) so invalid RFC3339 inputs no
longer produce a zero-value time silently.
- Around line 21-57: The method generate() is using logger.warn for normal
operational messages; replace those calls with logger.info (or logger.debug if
you prefer verbose output) so normal start/complete/resource-count logs are not
treated as warnings. Update the three logger.warn calls in generate()—the
"[GoSDK-Test] Starting test generation..." log, the resource grouping log that
includes "${resourceMap.size()}", and the completion log "[GoSDK-Test] Test
generation complete. Generated ${files.size()} test files"—to logger.info and
keep the message text unchanged.
In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy`:
- Around line 96-114: The recursive resolveAllClasses method can cause
StackOverflowError for deep or cyclical class dependencies; replace the
recursion with an iterative loop: repeatedly process a snapshot of
laterResolvedClasses (e.g., while it is not empty, copy to a local Set/Class
list, iterate calling resolveClass(clz) and remove processed entries from
laterResolvedClasses), catching Throwable and wrapping it in
CloudRuntimeException with the same message, and continue until no new classes
remain; ensure you reference and update laterResolvedClasses and call
resolveClass(Class) rather than recursively calling resolveAllClasses().
- Around line 177-183: The generated extends clause may not match the import
style; change the extends generation in SdkApiTemplate (where
getTargetClassName(clz) and clz.superclass are used) so it uses the same
identifier as produced by the imports list: if the parent's fully-qualified name
appears in the imports collection, emit the parent's simpleName (or sdkClassName
if you already use that mapping) for the extends clause; otherwise emit the
fully-qualified name. Ensure you reference clz.superclass.simpleName (or the
existing sdkClassName mapping) the same way imports.collect uses it so extends
and import remain consistent.
- Around line 79-91: The generated SourceClassMap class uses raw HashMap types
for srcToDstMapping and dstToSrcMapping which emits warnings; update the
anonymous-map instantiations to use generics (e.g., new HashMap<String,
String>() or the diamond operator new HashMap<>()) so both fields and their
initializers are typed as HashMap<String, String>, keeping the existing instance
initializer blocks intact for srcToDstMapping and dstToSrcMapping.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
rest/src/main/resources/scripts/GoApiTemplate.groovyrest/src/main/resources/scripts/GoInventory.groovyrest/src/main/resources/scripts/GoTestTemplate.groovyrest/src/main/resources/scripts/RestDocumentationGenerator.groovyrest/src/main/resources/scripts/SDK生成器开发指南.mdrest/src/main/resources/scripts/SdkApiTemplate.groovyrest/src/main/resources/scripts/ZStack SDK Go 开发规范与标准.mdrest/src/main/resources/scripts/templates/base_param_types.go.templaterest/src/main/resources/scripts/templates/base_params.go.templaterest/src/main/resources/scripts/templates/client.go.templaterest/src/main/resources/scripts/templates/example_main.gorest/src/main/resources/scripts/templates/login_params.go.templaterest/src/main/resources/scripts/templates/session_additional_views.go.templaterest/src/main/resources/scripts/templates/session_views.go.template
Comment on lines
+28
to
+43
| sessionView, err := cli.Login(ctx) | ||
| if err != nil { | ||
| fmt.Printf("登录失败: %v\n", err) | ||
| return | ||
| } | ||
| fmt.Printf("登录成功！Session UUID: %s\n", sessionView.Uuid) | ||
|
|
||
| // 查询虚拟机列表 | ||
| fmt.Println("\n开始查询虚拟机...") | ||
| queryParams := param.NewQueryParam() | ||
| queryParams.Limit(10) | ||
| vms, err := cli.QueryVmInstance(ctx, &queryParams) | ||
| if err != nil { | ||
| fmt.Printf("查询失败: %v\n", err) | ||
| return | ||
| } |
There was a problem hiding this comment.
登录成功后应立即注册 defer Logout
当前多个错误分支会提前 return，导致已登录会话不一定被释放。
建议修复
sessionView, err := cli.Login(ctx)
if err != nil {
fmt.Printf("登录失败: %v\n", err)
return
}
fmt.Printf("登录成功！Session UUID: %s\n", sessionView.Uuid)
+ defer func() {
+ if err := cli.Logout(ctx); err != nil {
+ fmt.Printf("登出失败: %v\n", err)
+ }
+ }()
@@
- // 登出
- fmt.Println("\n正在登出...")
- if err := cli.Logout(ctx); err != nil {
- fmt.Printf("登出失败: %v\n", err)
- } else {
- fmt.Println("登出成功！")
- }
+ fmt.Println("\n流程结束")Also applies to: 85-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/templates/example_main.go` around lines 28 -
43, After a successful cli.Login(ctx) the session is not guaranteed to be
released because multiple early returns exist; immediately after checking err ==
nil assign the returned session and register a deferred logout (e.g., defer
cli.Logout(ctx, sessionView) or defer cli.Logout(ctx, sessionView.Uuid)
depending on the Logout signature) so the session is always cleaned up on
function exit; add this same defer placement for the other login occurrences
referenced (the block around QueryVmInstance and the later block at the other
occurrence) and ensure Logout is safe to call with the session value used.
MatheMatrix
force-pushed
the
sync/zstackio/ZSV-11399
branch
from
1ae17f6 to
37f9d81
Compare
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (8)
rest/src/main/resources/scripts/SDK生成器开发指南.md (1)
19-19: 建议为所有 fenced code block 显式标注语言，避免 Markdown Lint 告警。这几处代码块当前使用了无语言标记的 ```，会触发 MD040。建议分别改为
bash/`text`/`groovy`/`go` 等明确语言，提升可读性与渲染一致性。Also applies to: 46-46, 162-162, 573-573, 857-857, 878-878
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SDK生成器开发指南.md` at line 19, Several fenced code blocks are missing language identifiers (they use bare ```), which triggers MD040; replace each unlabelled ``` token with an explicit language-tagged fence (e.g., ```bash, ```text, ```groovy, ```go) for the corresponding examples to satisfy Markdown linting and improve rendering—locate the unlabelled fences (the raw ``` occurrences) and update them to the appropriate language label.rest/src/main/resources/scripts/ZStack SDK Go 开发规范与标准.md (1)
133-148: 代码块缺少语言标识符项目结构的代码块应该添加语言标识符以获得正确的语法高亮。
建议修复
-``` +```text zsphere-sdk-go-v2/ ├── pkg/ │ ├── client/ # API 客户端和操作方法🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md around lines 133 - 148, The fenced code block showing the project directory (starting with "zsphere-sdk-go-v2/") lacks a language identifier; update that Markdown code fence in "ZStack SDK Go 开发规范与标准.md" to include a language specifier (e.g., text) so the block is rendered with proper highlighting—locate the triple-backtick fence around the directory tree and add the language token immediately after the opening backticks.rest/src/main/resources/scripts/SdkApiTemplate.groovy (1)
197-209: 对非 ZStack/非 Java 类抛出异常可能过于严格当遇到既不是 Java 标准库也不是 ZStack 命名空间的类时，直接抛出
CloudRuntimeException可能会导致生成过程中断。考虑某些第三方依赖类型可能出现在字段中的情况。建议改进
} else if (clz.getCanonicalName().startsWith("org.zstack")) { return true } else { - throw new CloudRuntimeException("${clz.getName()} is neither JRE class nor ZStack class") + // 第三方类按非 ZStack 类处理 + logger.warn("${clz.getName()} is neither JRE class nor ZStack class, treating as non-ZStack type") + return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` around lines 197 - 209, The isZStackClass(Class clz) method currently throws CloudRuntimeException when a class is neither a JRE class nor in the org.zstack namespace; change this to handle third-party/unknown classes more gracefully (e.g., return false and optionally log a warning) so generation does not abort on external types. Update the branches in isZStackClass to remove the throw, return false for non-java/non-org.zstack classes, and add a concise log message using the existing logging mechanism if available to aid debugging without failing the process.rest/src/main/resources/scripts/GoTestTemplate.groovy (2)
21-56: 日志级别使用不当第 23、33、55 行使用
logger.warn记录普通信息日志，建议改用logger.info，将warn保留给真正的警告信息（如第 509 行的重复文件跳过警告）。建议修复
- logger.warn("[GoSDK-Test] Starting test generation...") + logger.info("[GoSDK-Test] Starting test generation...") ... - logger.warn("[GoSDK-Test] Grouped ${resourceMap.size()} resources for test generation") + logger.info("[GoSDK-Test] Grouped ${resourceMap.size()} resources for test generation") ... - logger.warn("[GoSDK-Test] Test generation complete. Generated ${files.size()} test files") + logger.info("[GoSDK-Test] Test generation complete. Generated ${files.size()} test files")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoTestTemplate.groovy` around lines 21 - 56, In generate(), change the three informational logger.warn calls (the ones logging "[GoSDK-Test] Starting test generation...", "[GoSDK-Test] Grouped ${resourceMap.size()} resources for test generation", and "[GoSDK-Test] Test generation complete. Generated ${files.size()} test files") to logger.info so normal progress messages use info level; keep logger.warn for true warnings elsewhere (e.g., the duplicate-file skip warning). Locate these calls in the generate() method where files, resourceMap and related variables are used and update the logging level accordingly.
64-65: 正则表达式可能不符合预期
'Inventory\$'中的\$在 Groovy 单引号字符串中会被解释为字面量$，而不是正则表达式的行尾锚点。如果意图是匹配以 "Inventory" 结尾的类名，应该使用'Inventory$'（无转义）。建议修复
- String prefix = inventoryClass.simpleName.replaceAll('Inventory\$', '') + String prefix = inventoryClass.simpleName.replaceAll('Inventory$', '')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoTestTemplate.groovy` around lines 64 - 65, The replaceAll call inside inventories.each (where prefix is computed from inventoryClass.simpleName) is using the string literal 'Inventory\$' so the backslash is treated as a literal and not as a regex end-of-line anchor; change the call to use a proper regex that matches class names ending with "Inventory" (e.g., use a regex form with an unescaped $ anchor or a regex literal) so replaceAll removes the trailing "Inventory" as intended when computing prefix.rest/src/main/resources/scripts/GoApiTemplate.groovy (2)
391-393:generate()方法返回空列表
generate()方法直接返回空列表，实际的代码生成逻辑委托给了generateMethodCode()等方法。这符合当前架构设计（由GoInventory协调生成），但建议添加注释说明这一设计意图。List<SdkFile> generate() { + // Note: Actual code generation is delegated to generateMethodCode() and other helpers, + // coordinated by GoInventory. This method returns empty as GoApiTemplate does not + // produce standalone files. return [] }🤖 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 391 - 393, The generate() method currently returns an empty list but the actual generation is delegated elsewhere; add a clear explanatory comment or JavaDoc to the generate() method indicating that code generation is handled by generateMethodCode() (and coordinated by GoInventory) and that returning an empty List<SdkFile> is intentional to avoid confusion for future readers; reference the generateMethodCode() helper and the GoInventory coordinator in the comment so maintainers understand the delegation.
1259-1263: 生成代码缩进不一致此处使用了实际的制表符而非
\t转义序列，与其他生成方法的风格不一致：return """func (cli *ZSClient) ${clzName}(ctx context.Context, ${params}, deleteMode param.DeleteMode) error { return cli.DeleteWithSpec(ctx, "${cleanPath}", ${firstParam}, ${spec}, ${paramsStr}, nil) } """建议统一使用
\t以保持代码风格一致性：-return """func (cli *ZSClient) ${clzName}(ctx context.Context, ${params}, deleteMode param.DeleteMode) error { - return cli.DeleteWithSpec(ctx, "${cleanPath}", ${firstParam}, ${spec}, ${paramsStr}, nil) -} -""" +return """func (cli *ZSClient) ${clzName}(ctx context.Context, ${params}, deleteMode param.DeleteMode) error { +\treturn cli.DeleteWithSpec(ctx, "${cleanPath}", ${firstParam}, ${spec}, ${paramsStr}, nil) +} +"""🤖 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 1259 - 1263, The generated Go method string uses a literal tab character instead of the escaped sequence, causing inconsistent indentation; in GoApiTemplate.groovy update the return string for the function that emits "func (cli *ZSClient) ${clzName}(...)" so its inner indentation uses "\t" escapes (e.g. "\treturn cli.DeleteWithSpec(...)") instead of actual tab bytes, matching the other generators; adjust the multiline string around the ${clzName} / DeleteWithSpec emission to replace literal tabs with "\t" and ensure the closing brace line indentation matches the same escaped style.rest/src/main/resources/scripts/GoInventory.groovy (1)
317-712: 大量已弃用代码保留在代码库中
generateClientFile()方法被标记为@Deprecated，并且注释说明client.go是手动维护的。然而，这个方法包含约 400 行代码，全部是死代码。建议考虑删除此方法以减少代码维护负担，或者如果需要保留作为参考，可以将其移到单独的文档或注释块中。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoInventory.groovy` around lines 317 - 712, The generateClientFile() method is deprecated and contains ~400 lines of dead code; remove it from the file (or move its content to a separate docs/resource file) to avoid keeping large deprecated code in the codebase. Specifically, delete the private method generateClientFile() (and the local SdkFile construction inside it) or extract its string content into a plain reference file under docs/ or resources/; before deleting, search for any callers of generateClientFile and remove or refactor those call sites, and if you move the content ensure any build-time generators referencing this method are updated to read the new doc/resource instead.
🤖 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 1306-1327: The resource assignment in generateAsyncMethod is a
no-op because resource is set via apiPath.replaceAll(/^v1/, "v1"); update
generateAsyncMethod to either (a) use apiPath directly by assigning resource =
apiPath, or (b) strip the leading "v1/" prefix if that was intended by replacing
the leading prefix with an empty string (i.e., remove the ^v1/ prefix) — change
the assignment of the resource variable in generateAsyncMethod accordingly so it
reflects the intended URL path.
In `@rest/src/main/resources/scripts/GoTestTemplate.groovy`:
- Around line 328-351: The generated unit test omits the required ctx argument
when calling SDK methods; update generateQueryClientTest so it declares a
context (e.g., ctx := context.Background()) before calling the client and pass
ctx as the first parameter to cli.${methodName} (i.e., cli.${methodName}(ctx,
&queryParam)), and apply the same change to generateGetClientTest,
generateCreateClientTest, generateUpdateClientTest and generateDeleteClientTest
so all generated tests create and pass a context.Context as the first argument
to SDK method calls.
In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy`:
- Around line 1715-1720: The call to getApiExampleOfTheClass from curlExample
can return null and must be null-checked to avoid an NPE later in substituteUrl;
update curlExample() to check the returned value from
getApiExampleOfTheClass(clz) and either throw a CloudRuntimeException (same
style as the check in getApiExampleOfTheClass usage around the apiFields
handling) or return/skip building the curl example when null, ensuring
substituteUrl is only called with a non-null example map.
In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy`:
- Around line 264-265: The code currently calls
responseClass.getDeclaredField(src) which only finds fields declared on the
class itself; replace this call with the existing utility
getFieldRecursively(responseClass, src) so inherited fields are found, then pass
the returned Field to addToFields(dst, f) as before; update any null handling
around the result of getFieldRecursively to match how addToFields expects a
missing field.
- Around line 155-156: The code calls the static method
SdkApiTemplate.getPackageName(...) in SdkApiTemplate.groovy and
SdkDataStructureGenerator.groovy but that static method does not exist, causing
MissingMethodException; add a static getPackageName(String fullType) method to
the SdkApiTemplate class (or alternatively change callers to use an existing
utility) that returns the package portion for a given type string (handling
primitives, boxed types and nested/inner class names), then update all callers
in SdkApiTemplate and SdkDataStructureGenerator to use this static method;
ensure the method signature exactly matches SdkApiTemplate.getPackageName(...)
so existing calls at lines referencing that symbol resolve.
In `@rest/src/main/resources/scripts/SDK生成器开发指南.md`:
- Line 309:
在文档中将两处表述替换为更自然/专业的中文表达：把第309处短语“使用单独的集合追踪发现阶段处理的类”修改为“使用独立集合来追踪发现阶段处理的类”或“使用单独地集合追踪发现阶段处理的类”（优先推荐第一种更通顺的写法），并把第541处短语“从单参数改为三参数”修改为“从单个参数改为三个参数”；在相应函数/段落（包含上述原始短语的段落）直接替换文本并保持周边标点与格式一致。
In `@rest/src/main/resources/scripts/templates/client.go.template`:
- Around line 206-212: The hql method currently returns nil, nil when retVal ==
nil, losing the http.Header; change the retVal == nil branch in function hql to
return respHeader, nil so callers receive the response headers; apply the same
fix to the Zql method (the retVal == nil branch around lines referenced in the
diff) so both hql and Zql return (respHeader, nil) instead of (nil, nil).
In `@rest/src/main/resources/scripts/templates/example_main.go`:
- Around line 18-21: The sample calls DefaultZSConfig with only a hostname which
conflicts with the spec requiring an explicit context path; update the example
in example_main.go so DefaultZSConfig is invoked with both the API endpoint and
the context path (e.g., include "/zstack") or otherwise provide the full base
URL that contains the context path, and keep the rest of the chain
(LoginAccount, Debug) unchanged so DefaultZSConfig(...) supplies the correct
context path as per the documentation.
In `@rest/src/main/resources/scripts/templates/login_params.go.template`:
- Around line 5-22: Rename the struct LoginByAccountParam to LogInByAccountParam
to match the existing LogIn naming convention used by LogInByUserParam and
backend Java classes; update all references/usages (constructors,
marshaling/unmarshaling, variable declarations, imports, tests, and any template
placeholders) that reference LoginByAccountParam to the new LogInByAccountParam
identifier, and ensure the JSON field tag and inner type
LoginByAccountDetailParam remain unchanged so serialization stays consistent.
In
`@rest/src/main/resources/scripts/templates/session_additional_views.go.template`:
- Around line 1-26: Duplicate unused template session_views.go.template
duplicates SessionInventoryView and WebUISessionView (same as
session_additional_views.go.template) and isn't referenced by the generator;
delete the redundant session_views.go.template file, leaving
session_additional_views.go.template (which defines SessionInventoryView and
WebUISessionView) as the single source, and verify GoInventory.groovy still
imports session_additional_views.go.template and the generator/build passes.
In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md:
- Around line 955-993: The document has duplicate section numbers: "### 11.8
客户端代码生成简化" and "### 11.8 可选字段指针类型支持（2026-01 新增）"; rename the second heading to
"### 11.9 可选字段指针类型支持（2026-01 新增）" and update any downstream section numbers or
table-of-contents entries that reference these headings so numbering remains
consistent across the document; locate the headings by the exact heading text
"### 11.8 客户端代码生成简化" and "### 11.8 可选字段指针类型支持（2026-01 新增）" to make the change.
---
Nitpick comments:
In `@rest/src/main/resources/scripts/GoApiTemplate.groovy`:
- Around line 391-393: The generate() method currently returns an empty list but
the actual generation is delegated elsewhere; add a clear explanatory comment or
JavaDoc to the generate() method indicating that code generation is handled by
generateMethodCode() (and coordinated by GoInventory) and that returning an
empty List<SdkFile> is intentional to avoid confusion for future readers;
reference the generateMethodCode() helper and the GoInventory coordinator in the
comment so maintainers understand the delegation.
- Around line 1259-1263: The generated Go method string uses a literal tab
character instead of the escaped sequence, causing inconsistent indentation; in
GoApiTemplate.groovy update the return string for the function that emits "func
(cli *ZSClient) ${clzName}(...)" so its inner indentation uses "\t" escapes
(e.g. "\treturn cli.DeleteWithSpec(...)") instead of actual tab bytes, matching
the other generators; adjust the multiline string around the ${clzName} /
DeleteWithSpec emission to replace literal tabs with "\t" and ensure the closing
brace line indentation matches the same escaped style.
In `@rest/src/main/resources/scripts/GoInventory.groovy`:
- Around line 317-712: The generateClientFile() method is deprecated and
contains ~400 lines of dead code; remove it from the file (or move its content
to a separate docs/resource file) to avoid keeping large deprecated code in the
codebase. Specifically, delete the private method generateClientFile() (and the
local SdkFile construction inside it) or extract its string content into a plain
reference file under docs/ or resources/; before deleting, search for any
callers of generateClientFile and remove or refactor those call sites, and if
you move the content ensure any build-time generators referencing this method
are updated to read the new doc/resource instead.
In `@rest/src/main/resources/scripts/GoTestTemplate.groovy`:
- Around line 21-56: In generate(), change the three informational logger.warn
calls (the ones logging "[GoSDK-Test] Starting test generation...",
"[GoSDK-Test] Grouped ${resourceMap.size()} resources for test generation", and
"[GoSDK-Test] Test generation complete. Generated ${files.size()} test files")
to logger.info so normal progress messages use info level; keep logger.warn for
true warnings elsewhere (e.g., the duplicate-file skip warning). Locate these
calls in the generate() method where files, resourceMap and related variables
are used and update the logging level accordingly.
- Around line 64-65: The replaceAll call inside inventories.each (where prefix
is computed from inventoryClass.simpleName) is using the string literal
'Inventory\$' so the backslash is treated as a literal and not as a regex
end-of-line anchor; change the call to use a proper regex that matches class
names ending with "Inventory" (e.g., use a regex form with an unescaped $ anchor
or a regex literal) so replaceAll removes the trailing "Inventory" as intended
when computing prefix.
In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy`:
- Around line 197-209: The isZStackClass(Class clz) method currently throws
CloudRuntimeException when a class is neither a JRE class nor in the org.zstack
namespace; change this to handle third-party/unknown classes more gracefully
(e.g., return false and optionally log a warning) so generation does not abort
on external types. Update the branches in isZStackClass to remove the throw,
return false for non-java/non-org.zstack classes, and add a concise log message
using the existing logging mechanism if available to aid debugging without
failing the process.
In `@rest/src/main/resources/scripts/SDK生成器开发指南.md`:
- Line 19: Several fenced code blocks are missing language identifiers (they use
bare ```), which triggers MD040; replace each unlabelled ``` token with an
explicit language-tagged fence (e.g., ```bash, ```text, ```groovy, ```go) for
the corresponding examples to satisfy Markdown linting and improve
rendering—locate the unlabelled fences (the raw ``` occurrences) and update them
to the appropriate language label.
In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md:
- Around line 133-148: The fenced code block showing the project directory
(starting with "zsphere-sdk-go-v2/") lacks a language identifier; update that
Markdown code fence in "ZStack SDK Go 开发规范与标准.md" to include a language
specifier (e.g., text) so the block is rendered with proper highlighting—locate
the triple-backtick fence around the directory tree and add the language token
immediately after the opening backticks.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
rest/src/main/resources/scripts/GoApiTemplate.groovyrest/src/main/resources/scripts/GoInventory.groovyrest/src/main/resources/scripts/GoTestTemplate.groovyrest/src/main/resources/scripts/RestDocumentationGenerator.groovyrest/src/main/resources/scripts/SDK生成器开发指南.mdrest/src/main/resources/scripts/SdkApiTemplate.groovyrest/src/main/resources/scripts/ZStack SDK Go 开发规范与标准.mdrest/src/main/resources/scripts/templates/base_param_types.go.templaterest/src/main/resources/scripts/templates/base_params.go.templaterest/src/main/resources/scripts/templates/client.go.templaterest/src/main/resources/scripts/templates/example_main.gorest/src/main/resources/scripts/templates/login_params.go.templaterest/src/main/resources/scripts/templates/session_additional_views.go.templaterest/src/main/resources/scripts/templates/session_views.go.template
🚧 Files skipped from review as they are similar to previous changes (2)
- rest/src/main/resources/scripts/templates/base_param_types.go.template
- rest/src/main/resources/scripts/templates/base_params.go.template
Comment on lines
+155
to
156
| imports.add("${SdkApiTemplate.getPackageName(f.type)}.${simpleName}") | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for SdkApiTemplate class and getPackageName method
rg -n "class SdkApiTemplate|def getPackageName|static.*getPackageName" --type groovy rest/src/main/resources/scripts/Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# Search for SdkDataStructureGenerator class
rg -n "class SdkDataStructureGenerator" --type groovy rest/src/main/resources/scripts/Repository: MatheMatrix/zstack
Length of output: 301
🏁 Script executed:
# Check all references to SdkApiTemplate.getPackageName() in the entire codebase
rg -n "SdkApiTemplate\.getPackageName" rest/src/main/resources/scripts/Repository: MatheMatrix/zstack
Length of output: 1685
🏁 Script executed:
# Search for the definition of getPackageName method
rg -n "def getPackageName" rest/src/main/resources/scripts/Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# Check the content of SdkApiTemplate.groovy around line 155 and look for method definitions
cat rest/src/main/resources/scripts/SdkApiTemplate.groovy | head -200 | tail -50Repository: MatheMatrix/zstack
Length of output: 1737
🏁 Script executed:
# Get list of all groovy files in the scripts directory
ls -la rest/src/main/resources/scripts/ | grep ".groovy"Repository: MatheMatrix/zstack
Length of output: 512
🏁 Script executed:
# Check the entire SdkApiTemplate.groovy file to find getPackageName definition
cat rest/src/main/resources/scripts/SdkApiTemplate.groovyRepository: MatheMatrix/zstack
Length of output: 13038
🏁 Script executed:
# Also check SdkDataStructureGenerator.groovy to see if it has getPackageName
cat rest/src/main/resources/scripts/SdkDataStructureGenerator.groovyRepository: MatheMatrix/zstack
Length of output: 13296
🏁 Script executed:
# Search for getPackageName method definition across all groovy files
rg -n "def getPackageName|static.*def getPackageName" rest/src/main/resources/scripts/ --type groovyRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# Search in parent class or imported locations
rg -n "getPackageName" rest/src/main/resources/scripts/ --type groovy -B 2 -A 2 | grep -E "(class|interface|import|def getPackageName)"Repository: MatheMatrix/zstack
Length of output: 2091
🏁 Script executed:
# Check if there's a SdkTemplate interface that might define this method
fd "SdkTemplate" rest/Repository: MatheMatrix/zstack
Length of output: 117
🏁 Script executed:
# Check the SdkTemplate interface
cat rest/src/main/java/org/zstack/rest/sdk/SdkTemplate.javaRepository: MatheMatrix/zstack
Length of output: 220
🏁 Script executed:
# Also search for getPackageName in the entire rest directory
rg -n "static.*getPackageName|def getPackageName" rest/ --type groovy --type javaRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# Search broadly for getPackageName in all files
rg -n "getPackageName" rest/ --type groovy --type javaRepository: MatheMatrix/zstack
Length of output: 1685
🏁 Script executed:
# Look for any class named SdkApiTemplate
rg -n "class SdkApiTemplate" --type groovy --type javaRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# Search for getPackageName more broadly across the entire repository
rg -n "getPackageName" --type groovy --type java | head -50Repository: MatheMatrix/zstack
Length of output: 2882
🏁 Script executed:
# Check if there are other SdkApiTemplate files anywhere
find . -name "*SdkApiTemplate*" -type fRepository: MatheMatrix/zstack
Length of output: 117
🏁 Script executed:
# Search for any method definition in SdkApiTemplate.groovy that could provide getPackageName
grep -n "def " rest/src/main/resources/scripts/SdkApiTemplate.groovyRepository: MatheMatrix/zstack
Length of output: 996
🏁 Script executed:
# Check if getPackageName might be defined as a static method with different syntax
grep -n "getPackageName" rest/src/main/resources/scripts/SdkApiTemplate.groovy -A 3 -B 3Repository: MatheMatrix/zstack
Length of output: 1769
🏁 Script executed:
# Look at the entire SdkApiTemplate.groovy file structure
wc -l rest/src/main/resources/scripts/SdkApiTemplate.groovyRepository: MatheMatrix/zstack
Length of output: 119
🏁 Script executed:
# Read the end of the SdkApiTemplate.groovy file where getPackageName might be defined
tail -100 rest/src/main/resources/scripts/SdkApiTemplate.groovyRepository: MatheMatrix/zstack
Length of output: 3262
🏁 Script executed:
# Search specifically for static method definitions or properties in SdkApiTemplate
grep -n "static\|@" rest/src/main/resources/scripts/SdkApiTemplate.groovy | tail -30Repository: MatheMatrix/zstack
Length of output: 363
缺少关键的静态方法 getPackageName()
在 SdkApiTemplate.groovy 和 SdkDataStructureGenerator.groovy 中，代码多处调用 SdkApiTemplate.getPackageName()（第 155、169、179、281、297 行），但该静态方法在任何文件中都未定义。这将导致运行时错误（MissingMethodException）。必须实现这个方法或更正调用方式。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` around lines 155 -
156, The code calls the static method SdkApiTemplate.getPackageName(...) in
SdkApiTemplate.groovy and SdkDataStructureGenerator.groovy but that static
method does not exist, causing MissingMethodException; add a static
getPackageName(String fullType) method to the SdkApiTemplate class (or
alternatively change callers to use an existing utility) that returns the
package portion for a given type string (handling primitives, boxed types and
nested/inner class names), then update all callers in SdkApiTemplate and
SdkDataStructureGenerator to use this static method; ensure the method signature
exactly matches SdkApiTemplate.getPackageName(...) so existing calls at lines
referencing that symbol resolve.
Comment on lines
+264
to
+265
| Field f = responseClass.getDeclaredField(src) | ||
| addToFields(dst, f) |
There was a problem hiding this comment.
getDeclaredField 无法查找继承字段
第 264 行使用 responseClass.getDeclaredField(src) 只能查找当前类声明的字段，无法找到从父类继承的字段。建议使用同文件中已定义的 getFieldRecursively 方法。
建议修复
- Field f = responseClass.getDeclaredField(src)
+ Field f = getFieldRecursively(responseClass, src)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Field f = responseClass.getDeclaredField(src) | |
| addToFields(dst, f) | |
| Field f = getFieldRecursively(responseClass, src) | |
| addToFields(dst, f) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` around lines 264 -
265, The code currently calls responseClass.getDeclaredField(src) which only
finds fields declared on the class itself; replace this call with the existing
utility getFieldRecursively(responseClass, src) so inherited fields are found,
then pass the returned Field to addToFields(dst, f) as before; update any null
handling around the result of getFieldRecursively to match how addToFields
expects a missing field.
Comment on lines
+206
to
+212
|
|
||
| if retVal == nil { | ||
| return nil, nil | ||
| } | ||
|
|
||
| return respHeader, resp.Unmarshal(retVal, unMarshalKeys...) | ||
| } |
There was a problem hiding this comment.
hql 方法在 retVal == nil 时丢失响应头
当 retVal == nil 时，方法返回 nil, nil，但函数签名表明它应该返回 (http.Header, error)。这会导致调用者丢失可能有用的响应头信息。
if retVal == nil {
- return nil, nil
+ return respHeader, nil
}同样的问题也存在于第 223-225 行的 Zql 方法中。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if retVal == nil { | |
| return nil, nil | |
| } | |
| return respHeader, resp.Unmarshal(retVal, unMarshalKeys...) | |
| } | |
| if retVal == nil { | |
| return respHeader, nil | |
| } | |
| return respHeader, resp.Unmarshal(retVal, unMarshalKeys...) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/templates/client.go.template` around lines
206 - 212, The hql method currently returns nil, nil when retVal == nil, losing
the http.Header; change the retVal == nil branch in function hql to return
respHeader, nil so callers receive the response headers; apply the same fix to
the Zql method (the retVal == nil branch around lines referenced in the diff) so
both hql and Zql return (respHeader, nil) instead of (nil, nil).
Comment on lines
+18
to
+21
| // 方式2: 使用默认配置（推荐） | ||
| config := client.DefaultZSConfig("YOUR_ZSTACK_API_ENDPOINT"). | ||
| LoginAccount("admin", "password"). | ||
| Debug(true) |
There was a problem hiding this comment.
DefaultZSConfig 参数与文档不一致
根据规范文档（第 17 行），DefaultZSConfig 需要显式传入 context path（如 "/zstack"），但此示例只传入了 hostname。
建议修复
- config := client.DefaultZSConfig("YOUR_ZSTACK_API_ENDPOINT").
+ config := client.DefaultZSConfig("YOUR_ZSTACK_API_ENDPOINT", "/zstack").
LoginAccount("admin", "password").
Debug(true)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // 方式2: 使用默认配置（推荐） | |
| config := client.DefaultZSConfig("YOUR_ZSTACK_API_ENDPOINT"). | |
| LoginAccount("admin", "password"). | |
| Debug(true) | |
| // 方式2: 使用默认配置（推荐） | |
| config := client.DefaultZSConfig("YOUR_ZSTACK_API_ENDPOINT", "/zstack"). | |
| LoginAccount("admin", "password"). | |
| Debug(true) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/templates/example_main.go` around lines 18 -
21, The sample calls DefaultZSConfig with only a hostname which conflicts with
the spec requiring an explicit context path; update the example in
example_main.go so DefaultZSConfig is invoked with both the API endpoint and the
context path (e.g., include "/zstack") or otherwise provide the full base URL
that contains the context path, and keep the rest of the chain (LoginAccount,
Debug) unchanged so DefaultZSConfig(...) supplies the correct context path as
per the documentation.
Comment on lines
+5
to
+22
| type LoginByAccountParam struct { | ||
| BaseParam | ||
| LoginByAccount LoginByAccountDetailParam `json:"logInByAccount"` | ||
| } | ||
|
|
||
| type LoginByAccountDetailParam struct { | ||
| AccountName string `json:"accountName"` // Account name | ||
| Password string `json:"password"` // Password | ||
| AccountType string `json:"accountType"` // Account type | ||
| CaptchaUuid string `json:"captchaUuid"` // Captcha UUID | ||
| VerifyCode string `json:"verifyCode"` // Verification code | ||
| ClientInfo map[string]interface{} `json:"clientInfo"` // Client information | ||
| } | ||
|
|
||
| type LogInByUserParam struct { | ||
| BaseParam | ||
| LogInByUser LogInByUserDetailParam `json:"logInByUser"` | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the login_params.go.template file
fd -n "login_params.go.template" -x cat -n {}Repository: MatheMatrix/zstack
Length of output: 292
🏁 Script executed:
# Search for backend API classes related to login in Java files
rg -n "class.*Login|class.*LogIn" --type java -A 2Repository: MatheMatrix/zstack
Length of output: 13190
🏁 Script executed:
# Check if there are other similar parameter struct definitions to understand the naming pattern
rg -n "type.*Param struct" rest/src/main/resources/scripts/templates/ -A 2Repository: MatheMatrix/zstack
Length of output: 4256
结构体命名不一致：LoginByAccountParam 应改为 LogInByAccountParam
结构体名称命名不一致：LoginByAccountParam 使用 "Login"，而 LogInByUserParam 使用 "LogIn"。检查后端 Java API 发现，登录相关的 Action 和 Message 类名均统一使用 "LogIn"（如 LogInAction、LogInByAccountAction、APILogInMsg、APILogInByAccountMsg）。建议将 LoginByAccountParam 改为 LogInByAccountParam 以与后端 API 命名约定保持一致。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/templates/login_params.go.template` around
lines 5 - 22, Rename the struct LoginByAccountParam to LogInByAccountParam to
match the existing LogIn naming convention used by LogInByUserParam and backend
Java classes; update all references/usages (constructors,
marshaling/unmarshaling, variable declarations, imports, tests, and any template
placeholders) that reference LoginByAccountParam to the new LogInByAccountParam
identifier, and ensure the JSON field tag and inner type
LoginByAccountDetailParam remain unchanged so serialization stays consistent.
Comment on lines
+1
to
+26
| // Copyright (c) ZStack.io, Inc. | ||
|
|
||
| package view | ||
|
|
||
| import "time" | ||
|
|
||
| // SessionInventoryView Session | ||
| type SessionInventoryView struct { | ||
| Uuid string `json:"uuid"` | ||
| AccountUuid string `json:"accountUuid,omitempty"` | ||
| UserUuid string `json:"userUuid,omitempty"` | ||
| UserType string `json:"userType,omitempty"` | ||
| ExpiredDate time.Time `json:"expiredDate,omitempty"` | ||
| CreateDate time.Time `json:"createDate,omitempty"` | ||
| } | ||
|
|
||
| // WebUISessionView Web UI Session | ||
| type WebUISessionView struct { | ||
| SessionId string `json:"sessionId"` // Session ID | ||
| AccountUuid string `json:"accountUuid"` // Account UUID | ||
| UserUuid string `json:"userUuid"` // User UUID | ||
| UserName string `json:"username"` // Username | ||
| LoginType string `json:"loginType"` // Login type | ||
| CurrentIdentity string `json:"currentIdentity"` // Current identity | ||
| ZSVersion string `json:"zsVersion"` // ZStack Cloud version | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare the two template files
diff rest/src/main/resources/scripts/templates/session_views.go.template \
rest/src/main/resources/scripts/templates/session_additional_views.go.templateRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# List all template files to understand the naming patterns
find rest/src/main/resources/scripts/templates -name "*.template" -type f | head -20Repository: MatheMatrix/zstack
Length of output: 473
🏁 Script executed:
#!/bin/bash
# Read session_views.go.template
echo "=== session_views.go.template ==="
cat rest/src/main/resources/scripts/templates/session_views.go.template
echo ""
echo "=== session_additional_views.go.template ==="
cat rest/src/main/resources/scripts/templates/session_additional_views.go.templateRepository: MatheMatrix/zstack
Length of output: 2106
🏁 Script executed:
#!/bin/bash
# Check git history to understand when these files were added
cd rest/src/main/resources/scripts/templates
git log --oneline --follow session_views.go.template | head -5
git log --oneline --follow session_additional_views.go.template | head -5Repository: MatheMatrix/zstack
Length of output: 173
🏁 Script executed:
#!/bin/bash
# Search for references to these template files to understand their usage
rg "session_additional_views|session_views" rest/src/main/resources/scripts/templates --type-list
grep -r "session_additional_views\|session_views" rest/src --include="*.java" --include="*.gradle" | head -20Repository: MatheMatrix/zstack
Length of output: 451
🏁 Script executed:
#!/bin/bash
# Search for references to these template files
grep -r "session_additional_views\|session_views" rest/src --include="*.java" --include="*.gradle" --include="*.xml" | grep -v ".class" | head -30Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search in the entire rest directory for any build or generation logic
find rest/src -name "*.java" -type f -exec grep -l "session.*view" {} \; | head -10Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search for files that might process/use these templates
find rest/src -name "*enerator*" -o -name "*emplate*" | head -20Repository: MatheMatrix/zstack
Length of output: 925
🏁 Script executed:
#!/bin/bash
# Check for pom.xml or build configuration in rest module
cat rest/pom.xml | head -100Repository: MatheMatrix/zstack
Length of output: 3956
🏁 Script executed:
#!/bin/bash
# Search for any references to the templates directory
grep -r "templates" rest/src --include="*.java" | head -20Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search for references in generator scripts
grep -r "session_additional_views\|session_views" rest/src/main/resources/scripts/ --include="*.groovy"Repository: MatheMatrix/zstack
Length of output: 1045
🏁 Script executed:
#!/bin/bash
# Check SdkDataStructureGenerator.groovy for template usage
grep -A 5 -B 5 "\.template\|session" rest/src/main/resources/scripts/SdkDataStructureGenerator.groovy | head -50Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Look for file reading patterns that might load these templates
grep -r "session_additional_views\|session_views\|getResource\|loadResource" rest/src --include="*.java" --include="*.groovy" | head -20Repository: MatheMatrix/zstack
Length of output: 2200
🏁 Script executed:
#!/bin/bash
# Double-check: search everywhere for session_views (without additional) to confirm it's not used
grep -r "session_views" rest/src --include="*.java" --include="*.groovy" --include="*.gradle" --include="*.xml" | grep -v "session_additional_views"Repository: MatheMatrix/zstack
Length of output: 44
删除未使用的重复模板文件 session_views.go.template
session_views.go.template 与 session_additional_views.go.template 内容完全相同，但在整个代码库中没有被使用。只有 session_additional_views.go.template 被 GoInventory.groovy 引入，应删除 session_views.go.template 以消除冗余。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@rest/src/main/resources/scripts/templates/session_additional_views.go.template`
around lines 1 - 26, Duplicate unused template session_views.go.template
duplicates SessionInventoryView and WebUISessionView (same as
session_additional_views.go.template) and isn't referenced by the generator;
delete the redundant session_views.go.template file, leaving
session_additional_views.go.template (which defines SessionInventoryView and
WebUISessionView) as the single source, and verify GoInventory.groovy still
imports session_additional_views.go.template and the generator/build passes.
Comment on lines
+955
to
+993
| ### 11.8 客户端代码生成简化 | ||
|
|
||
| **规则**: `client.go` 不再由生成器生成，使用固定模板文件 | ||
|
|
||
| **原因**: | ||
|
|
||
| - `client.go` 是基础设施代码，逻辑稳定 | ||
| - 避免每次生成都覆盖手动优化的实现 | ||
| - 简化生成器逻辑 | ||
|
|
||
| **实现方式**: | ||
|
|
||
| 1. 创建 `client.go.template` 固定模板文件 | ||
| 2. 生成器启动时直接复制模板到输出目录 | ||
| 3. 移除 `GoInventory.generateClientFile()` 方法 | ||
|
|
||
| **模板文件位置**: | ||
|
|
||
| ``` | ||
| rest/src/main/resources/scripts/templates/ | ||
| └── client.go.template # 固定的 client.go 实现 | ||
| ``` | ||
|
|
||
| **生成器调用**: | ||
|
|
||
| ```groovy | ||
| private SdkFile copyClientTemplate() { | ||
| def template = new File("templates/client.go.template") | ||
| def sdkFile = new SdkFile() | ||
| sdkFile.subPath = "/pkg/client/" | ||
| sdkFile.fileName = "client.go" | ||
| sdkFile.content = template.text | ||
| return sdkFile | ||
| } | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ### 11.8 可选字段指针类型支持（2026-01 新增） |
There was a problem hiding this comment.
章节编号重复：11.8 出现两次
第 955 行和第 993 行都标记为 "11.8"，这会导致文档结构混乱。建议将第二个 11.8（可选字段指针类型支持）重新编号为 11.9。
建议修复
-### 11.8 可选字段指针类型支持（2026-01 新增）
+### 11.9 可选字段指针类型支持（2026-01 新增）🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 973-973: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md around lines 955 -
993, The document has duplicate section numbers: "### 11.8 客户端代码生成简化" and "###
11.8 可选字段指针类型支持（2026-01 新增）"; rename the second heading to "### 11.9
可选字段指针类型支持（2026-01 新增）" and update any downstream section numbers or
table-of-contents entries that reference these headings so numbering remains
consistent across the document; locate the headings by the exact heading text
"### 11.8 客户端代码生成简化" and "### 11.8 可选字段指针类型支持（2026-01 新增）" to make the change.
37f9d81 to
4c28489
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (9)
rest/src/main/resources/scripts/SdkApiTemplate.groovy (1)
270-273:⚠️ Potential issue | 🟡 Minor
getDeclaredField用于查找 "total" 字段可能失败第 272 行假设
total字段直接声明在responseClass.superclass中。如果该字段在更上层的祖先类中定义（如APIQueryReply的父类），此调用将抛出NoSuchFieldException。建议使用getFieldRecursively进行查找。🐛 建议修复
// hack if (APIQueryReply.class.isAssignableFrom(responseClass)) { - addToFields("total", responseClass.superclass.getDeclaredField("total")) + addToFields("total", getFieldRecursively(responseClass, "total")) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` around lines 270 - 273, The current hack uses responseClass.superclass.getDeclaredField("total") which will throw NoSuchFieldException if "total" lives further up the ancestor chain; change this to use the existing getFieldRecursively utility (or implement one) to locate "total" on the superclass chain before calling addToFields in the APIQueryReply branch: call getFieldRecursively(responseClass.superclass, "total") (or equivalent) and pass the returned Field to addToFields, and handle the null/exception case gracefully (e.g., skip adding or log) instead of calling getDeclaredField directly.rest/src/main/resources/scripts/RestDocumentationGenerator.groovy (3)
1629-1637:⚠️ Potential issue | 🟡 Minor
curlExample()中getApiExampleOfTheClass返回 null 时仍可能导致 NPE虽然添加了 try-catch 包裹
substituteUrl调用，但当getApiExampleOfTheClass(clz)返回null时（类没有__example__方法），allFields为null，后续substituteUrl会抛出异常。建议在调用前添加 null 检查，与getRequestBody()方法（lines 1715-1717）保持一致。建议修复
Map allFields = getApiExampleOfTheClass(clz) + if (allFields == null) { + logger.warn("No __example__ method found for class ${clz.name}, skipping curl example") + return "" + } String urlPath try { urlPath = substituteUrl("${RestConstants.API_VERSION}${it}", allFields)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy` around lines 1629 - 1637, In curlExample(), avoid NPE by checking the result of getApiExampleOfTheClass(clz) before calling substituteUrl: if getApiExampleOfTheClass(clz) returns null, handle it the same way getRequestBody() does (e.g., skip substitution or set allFields to an empty map) so substituteUrl(allFields) is not invoked with null; update the flow around the variables allFields and urlPath and keep the existing try-catch for substituteUrl to preserve error logging.
1931-1933:⚠️ Potential issue | 🟠 Major不要吞掉文档生成异常
当前
generate()方法的 catch 块仅记录警告日志但不抛出异常，可能导致返回null并将根本原因隐藏到后续流程中。建议修复
} catch (Exception e) { logger.warn(e.message, e) + throw new CloudRuntimeException("failed to generate markdown", e) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy` around lines 1931 - 1933, The catch in the generate() method currently only logs and swallows exceptions (catch (Exception e) { logger.warn(e.message, e) }), which can cause generate() to return null and hide the root cause; update that catch to rethrow the error (or wrap it in a RuntimeException/IOException with the original exception as the cause) after logging so callers receive the failure (i.e., log the exception via logger.warn(...) and then throw e or throw new RuntimeException("Failed to generate documentation", e) from the generate() method).
2103-2103:⚠️ Potential issue | 🟠 Major
allTo字段查找应支持继承层级此处使用
getDeclaredField()只会在当前类中查找字段；若字段定义在父类中，将直接失败。同文件其他位置（如 line 2115）已使用FieldUtils.getField(...)处理该问题。建议修复
- fsToAdd[at.allTo()] = responseClass.getDeclaredField(at.allTo()) + fsToAdd[at.allTo()] = FieldUtils.getField(at.allTo(), responseClass)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy` at line 2103, The code uses responseClass.getDeclaredField(at.allTo()) which only searches the current class and will fail if the field is declared in a superclass; change this to use FieldUtils.getField(responseClass, at.allTo(), true) (matching the usage elsewhere) so inherited fields are found, and handle the potential null return (e.g., throw or log a clear error) before assigning into fsToAdd[at.allTo()]; update the reference at the same location where fsToAdd and at.allTo() are used.rest/src/main/resources/scripts/templates/login_params.go.template (1)
5-22:⚠️ Potential issue | 🟡 Minor结构体命名不一致：
LoginByAccountParam应改为LogInByAccountParam结构体命名不一致：
LoginByAccountParam（Line 5）使用 "Login"，而LogInByUserParam（Line 19）使用 "LogIn"。后端 Java API 类名统一使用 "LogIn"（如APILogInByAccountMsg）。建议将LoginByAccountParam改为LogInByAccountParam以保持一致性。注意：JSON 标签
json:"logInByAccount"已正确使用 "logIn"，此修改仅涉及 Go 结构体名称。建议修复
-type LoginByAccountParam struct { +type LogInByAccountParam struct { BaseParam - LoginByAccount LoginByAccountDetailParam `json:"logInByAccount"` + LogInByAccount LogInByAccountDetailParam `json:"logInByAccount"` } -type LoginByAccountDetailParam struct { +type LogInByAccountDetailParam struct { AccountName string `json:"accountName"` // Account name🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/login_params.go.template` around lines 5 - 22, Rename the top-level struct LoginByAccountParam to LogInByAccountParam to match the existing LogInByUserParam naming convention and backend Java API (e.g., APILogInByAccountMsg); leave the JSON tag `json:"logInByAccount"` and the nested type LoginByAccountDetailParam unchanged, and update any references/usages of LoginByAccountParam throughout the codebase (handlers, constructors, type assertions, imports) to the new symbol LogInByAccountParam so compilation and consistency are preserved.rest/src/main/resources/scripts/templates/example_main.go (2)
18-21:⚠️ Potential issue | 🟡 Minor
DefaultZSConfig参数与文档不一致根据 SDK 开发指南（第 779 行），
ZSConfig需要显式设置ContextPath（如"/zstack"）。建议示例与文档保持一致，明确展示如何配置 context path。建议修复
- // 方式2: 使用默认配置（推荐） - config := client.DefaultZSConfig("YOUR_ZSTACK_API_ENDPOINT"). + // 方式2: 使用默认配置（推荐），需确保 DefaultZSConfig 内部设置了正确的 context path + config := client.DefaultZSConfig("YOUR_ZSTACK_API_ENDPOINT", "/zstack"). LoginAccount("admin", "password"). Debug(true)或者保持原样，但在注释中说明
DefaultZSConfig会自动设置默认的 context path。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/example_main.go` around lines 18 - 21, The example uses DefaultZSConfig but doesn't set the required ContextPath per the SDK guide; update the example to explicitly set the context path on the ZSConfig (e.g., call the ContextPath setter on the config returned by DefaultZSConfig, alongside LoginAccount and Debug) so it shows ContextPath("/zstack") as required by the docs (or alternatively, add a comment clarifying that DefaultZSConfig will set a default context path); refer to DefaultZSConfig and the ZSConfig/ContextPath setter to locate where to change the sample.
28-43:⚠️ Potential issue | 🟠 Major登录成功后应立即注册
defer Logout当前多个错误分支会提前
return（如 lines 31, 42），导致已登录会话不一定被释放。建议在登录成功后立即使用defer注册登出操作。建议修复
sessionView, err := cli.Login(ctx) if err != nil { fmt.Printf("登录失败: %v\n", err) return } fmt.Printf("登录成功！Session UUID: %s\n", sessionView.Uuid) + defer func() { + fmt.Println("\n正在登出...") + if err := cli.Logout(ctx); err != nil { + fmt.Printf("登出失败: %v\n", err) + } else { + fmt.Println("登出成功！") + } + }()然后删除 lines 85-91 处的手动登出代码。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/example_main.go` around lines 28 - 43, After a successful cli.Login (where sessionView is returned) register a deferred logout immediately — call cli.Logout with the same ctx and session identifier (sessionView or its Uuid) using defer so any early returns still release the session; then remove the later manual logout block that explicitly calls Logout (the separate "manual登出" code) to avoid double-logout. Target the Login/Logout calls and sessionView usage in example_main.go (e.g., the cli.Login and cli.Logout invocation sites) when applying this change.rest/src/main/resources/scripts/GoTestTemplate.groovy (1)
328-431:⚠️ Potential issue | 🟠 Major生成的单元测试缺少
ctx参数所有客户端单元测试生成方法（
generateQueryClientTest、generateGetClientTest、generateCreateClientTest、generateUpdateClientTest、generateDeleteClientTest）调用 SDK 方法时没有传递context参数。根据 SDK 设计规范和集成测试模板（第 531 行），所有 SDK 方法都应接收ctx context.Context作为第一个参数。建议修复 - 以 generateQueryClientTest 为例
func Test${methodName}_Client(t *testing.T) { +\tctx := context.Background() \tcli, cleanup := newMockClient(func(w http.ResponseWriter, r *http.Request) { ... \tqueryParam := param.NewQueryParam() -\tresult, err := cli.${methodName}(&queryParam) +\tresult, err := cli.${methodName}(ctx, &queryParam)同样的修改需要应用到其他四个方法：
generateGetClientTest:cli.${methodName}(ctx, "test-uuid-001")generateCreateClientTest:cli.${methodName}(ctx, param.${paramStruct}{})generateUpdateClientTest:cli.${methodName}(ctx, "test-uuid-001", param.${paramStruct}{})generateDeleteClientTest:cli.${methodName}(ctx, "test-uuid-001", param.DeleteModePermissive)同时需要在导入中添加
"context"包。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoTestTemplate.groovy` around lines 328 - 431, Tests generated by generateQueryClientTest, generateGetClientTest, generateCreateClientTest, generateUpdateClientTest, and generateDeleteClientTest call SDK methods without the required ctx context.Context first parameter; update each generated call to pass ctx as the first argument (e.g., in generateQueryClientTest call cli.${methodName}(ctx, &queryParam), in generateGetClientTest call cli.${methodName}(ctx, "test-uuid-001"), in generateCreateClientTest call cli.${methodName}(ctx, param.${paramStruct}{}), in generateUpdateClientTest call cli.${methodName}(ctx, "test-uuid-001", param.${paramStruct}{}), and in generateDeleteClientTest call cli.${methodName}(ctx, "test-uuid-001", param.DeleteModePermissive)); also add the "context" import to the test file imports so ctx can be created/passed.rest/src/main/resources/scripts/templates/client.go.template (1)
207-209:⚠️ Potential issue | 🟡 Minor
hql/Zql在retVal == nil分支丢失响应头Line 207 和 Line 223 当前都返回
nil, nil，会让调用方拿不到http.Header。这里应返回respHeader, nil。✅ 建议修复
if retVal == nil { - return nil, nil + return respHeader, nil }if retVal == nil { - return nil, nil + return respHeader, nil }Also applies to: 223-225
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/client.go.template` around lines 207 - 209, The nil-check branches currently return nil, nil which discards response headers; update both places that check retVal (the branch using retVal == nil around lines where retVal and respHeader are in scope) to return respHeader, nil instead of nil, nil so callers receive the http.Header; ensure both occurrences (the retVal == nil branch and the similar branch lower down) are changed to return respHeader, nil.
🧹 Nitpick comments (4)
rest/src/main/resources/scripts/SdkApiTemplate.groovy (2)
34-40: 实例字段reflections被构造函数中的局部变量遮蔽且从未使用第 34 行声明了实例字段
reflections，但第 37 行在构造函数中声明了同名局部变量将其遮蔽。实例字段在整个类中从未被引用（第 221 行直接使用Platform.reflections），属于死代码。建议移除未使用的实例字段，或在构造函数中直接赋值给实例字段：
♻️ 建议修复
- Reflections reflections = Platform.reflections + Reflections reflections SdkDataStructureGenerator() { - Reflections reflections = Platform.getReflections() + reflections = Platform.getReflections() responseClasses = reflections.getTypesAnnotatedWith(RestResponse.class)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` around lines 34 - 40, The instance field "reflections" is shadowed by a local variable in the constructor SdkDataStructureGenerator() and is never used; either remove the unused instance field "reflections" or set that field inside the constructor instead of declaring a new local Reflections variable (replace "Reflections reflections = Platform.getReflections()" with "this.reflections = Platform.getReflections()"), and ensure other code (e.g., sites using Platform.reflections or the constructor logic that populates responseClasses and laterResolvedClasses) consistently uses the instance field if you choose to keep it.
79-90: 生成的 Java 代码使用原始类型 HashMap生成的
SourceClassMap.java中 HashMap 未指定泛型类型参数，这会在编译生成的 SDK 代码时产生 unchecked 警告。♻️ 建议修复
- public final static HashMap<String, String> srcToDstMapping = new HashMap() { + public final static HashMap<String, String> srcToDstMapping = new HashMap<String, String>() { { ${"$"}{srcToDst.join("\\n")} } }; - public final static HashMap<String, String> dstToSrcMapping = new HashMap() { + public final static HashMap<String, String> dstToSrcMapping = new HashMap<String, String>() { {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` around lines 79 - 90, The generated SourceClassMap uses raw HashMap instantiations which cause unchecked warnings; change the constructors for srcToDstMapping and dstToSrcMapping from new HashMap() to include generics (e.g. new HashMap<String, String>() or new HashMap<>()) so the declarations public final static HashMap<String, String> srcToDstMapping and dstToSrcMapping match their instantiations and eliminate raw-type usage in the SourceClassMap class.rest/src/main/resources/scripts/templates/base_param_types.go.template (1)
5-7: 建议：如果time包确实未被使用，可以移除当前使用
var _ = time.Now来抑制未使用导入警告。如果此模板中确实没有使用time.Time类型，建议直接移除import "time"和相关的 dummy 变量，而不是使用此模式来绕过编译器检查。如果这是为了预留给可能使用时间类型的生成代码，则保持现状是可以接受的。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/base_param_types.go.template` around lines 5 - 7, The template currently imports the time package and suppresses unused-import errors with the dummy symbol `var _ = time.Now`; if generated code does not actually use time types, remove the `import "time"` and the `var _ = time.Now` placeholder to avoid dead imports; if the import is intentionally reserved for potential generated fields of type `time.Time`, leave them as-is—search for the `import "time"` line and the `var _ = time.Now` declaration in the template to make the change.rest/src/main/resources/scripts/templates/login_params.go.template (1)
32-51: IAM2 相关结构体命名建议保持一致
LoginIAM2VirtualIDWithLdapParam和LoginIAM2PlatformParam使用 "Login" 前缀。如果统一采用 "LogIn" 命名规范，这些结构体也应相应更新为LogInIAM2VirtualIDWithLdapParam和LogInIAM2PlatformParam。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/login_params.go.template` around lines 32 - 51, The struct names use "Login" but you want "LogIn" for consistency; rename types LoginIAM2VirtualIDWithLdapParam -> LogInIAM2VirtualIDWithLdapParam, LoginIAM2VirtualIDWithLdapDetailParam -> LogInIAM2VirtualIDWithLdapDetailParam, LoginIAM2PlatformParam -> LogInIAM2PlatformParam and LoginIAM2PlatformDetailParam -> LogInIAM2PlatformDetailParam, update their type references (e.g., the LoginIAM2VirtualIDWithLdap field types and LoginIAM2Platform fields) and any JSON tags or external references if necessary, and then search/replace across the codebase to update all usages/imports to the new identifiers to avoid compilation errors.
🤖 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 1306-1319: The generated async method always calls
cli.PostWithAsync in generateAsyncMethod (using asyncMethodName and resource)
which ignores the httpMethod and will mis-call PUT/DELETE endpoints; change
generateAsyncMethod to branch on the httpMethod parameter: for "POST" call
cli.PostWithAsync(...), for "PUT" call cli.PutWithAsync(...), for "DELETE" call
cli.DeleteWithAsync(...), and for other methods call a generic
cli.RequestWithAsync(ctx, "<METHOD>", resource, responseKey, params, retVal,
true) or equivalent; replace the fixed cli.PostWithAsync(...) invocation with
this conditional dispatch so the generated async method uses the correct HTTP
verb.
In `@rest/src/main/resources/scripts/GoInventory.groovy`:
- Around line 160-176: The generate() method reuses mutable module-level
collections causing duplicate/contaminated output on repeated calls; at the
start of generate() clear or reinitialize shared state variables such as
allApiTemplates, generatedViewStructs, generatedParamStructs,
generatedClientMethods, longJobMappings (and any cached inventories if
applicable) so each invocation starts from a clean state; locate the generate()
method and add resets for those symbols (allApiTemplates, generatedViewStructs,
generatedParamStructs, generatedClientMethods, longJobMappings,
inventories/getInventories cache) before calling scanLongJobMappings() and
prepareApiTemplates().
In `@rest/src/main/resources/scripts/templates/session_views.go.template`:
- Around line 1-26: The file session_views.go.template duplicates the types
SessionInventoryView and WebUISessionView already present in
session_additional_views.go.template and is unused; remove the duplicate
template file session_views.go.template, verify there are no references to
SessionInventoryView or WebUISessionView pointing to that file (ensure code
generation uses session_additional_views.go.template), and run the repository's
build/generation step to confirm no missing-template errors.
---
Duplicate comments:
In `@rest/src/main/resources/scripts/GoTestTemplate.groovy`:
- Around line 328-431: Tests generated by generateQueryClientTest,
generateGetClientTest, generateCreateClientTest, generateUpdateClientTest, and
generateDeleteClientTest call SDK methods without the required ctx
context.Context first parameter; update each generated call to pass ctx as the
first argument (e.g., in generateQueryClientTest call cli.${methodName}(ctx,
&queryParam), in generateGetClientTest call cli.${methodName}(ctx,
"test-uuid-001"), in generateCreateClientTest call cli.${methodName}(ctx,
param.${paramStruct}{}), in generateUpdateClientTest call cli.${methodName}(ctx,
"test-uuid-001", param.${paramStruct}{}), and in generateDeleteClientTest call
cli.${methodName}(ctx, "test-uuid-001", param.DeleteModePermissive)); also add
the "context" import to the test file imports so ctx can be created/passed.
In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy`:
- Around line 1629-1637: In curlExample(), avoid NPE by checking the result of
getApiExampleOfTheClass(clz) before calling substituteUrl: if
getApiExampleOfTheClass(clz) returns null, handle it the same way
getRequestBody() does (e.g., skip substitution or set allFields to an empty map)
so substituteUrl(allFields) is not invoked with null; update the flow around the
variables allFields and urlPath and keep the existing try-catch for
substituteUrl to preserve error logging.
- Around line 1931-1933: The catch in the generate() method currently only logs
and swallows exceptions (catch (Exception e) { logger.warn(e.message, e) }),
which can cause generate() to return null and hide the root cause; update that
catch to rethrow the error (or wrap it in a RuntimeException/IOException with
the original exception as the cause) after logging so callers receive the
failure (i.e., log the exception via logger.warn(...) and then throw e or throw
new RuntimeException("Failed to generate documentation", e) from the generate()
method).
- Line 2103: The code uses responseClass.getDeclaredField(at.allTo()) which only
searches the current class and will fail if the field is declared in a
superclass; change this to use FieldUtils.getField(responseClass, at.allTo(),
true) (matching the usage elsewhere) so inherited fields are found, and handle
the potential null return (e.g., throw or log a clear error) before assigning
into fsToAdd[at.allTo()]; update the reference at the same location where
fsToAdd and at.allTo() are used.
In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy`:
- Around line 270-273: The current hack uses
responseClass.superclass.getDeclaredField("total") which will throw
NoSuchFieldException if "total" lives further up the ancestor chain; change this
to use the existing getFieldRecursively utility (or implement one) to locate
"total" on the superclass chain before calling addToFields in the APIQueryReply
branch: call getFieldRecursively(responseClass.superclass, "total") (or
equivalent) and pass the returned Field to addToFields, and handle the
null/exception case gracefully (e.g., skip adding or log) instead of calling
getDeclaredField directly.
In `@rest/src/main/resources/scripts/templates/client.go.template`:
- Around line 207-209: The nil-check branches currently return nil, nil which
discards response headers; update both places that check retVal (the branch
using retVal == nil around lines where retVal and respHeader are in scope) to
return respHeader, nil instead of nil, nil so callers receive the http.Header;
ensure both occurrences (the retVal == nil branch and the similar branch lower
down) are changed to return respHeader, nil.
In `@rest/src/main/resources/scripts/templates/example_main.go`:
- Around line 18-21: The example uses DefaultZSConfig but doesn't set the
required ContextPath per the SDK guide; update the example to explicitly set the
context path on the ZSConfig (e.g., call the ContextPath setter on the config
returned by DefaultZSConfig, alongside LoginAccount and Debug) so it shows
ContextPath("/zstack") as required by the docs (or alternatively, add a comment
clarifying that DefaultZSConfig will set a default context path); refer to
DefaultZSConfig and the ZSConfig/ContextPath setter to locate where to change
the sample.
- Around line 28-43: After a successful cli.Login (where sessionView is
returned) register a deferred logout immediately — call cli.Logout with the same
ctx and session identifier (sessionView or its Uuid) using defer so any early
returns still release the session; then remove the later manual logout block
that explicitly calls Logout (the separate "manual登出" code) to avoid
double-logout. Target the Login/Logout calls and sessionView usage in
example_main.go (e.g., the cli.Login and cli.Logout invocation sites) when
applying this change.
In `@rest/src/main/resources/scripts/templates/login_params.go.template`:
- Around line 5-22: Rename the top-level struct LoginByAccountParam to
LogInByAccountParam to match the existing LogInByUserParam naming convention and
backend Java API (e.g., APILogInByAccountMsg); leave the JSON tag
`json:"logInByAccount"` and the nested type LoginByAccountDetailParam unchanged,
and update any references/usages of LoginByAccountParam throughout the codebase
(handlers, constructors, type assertions, imports) to the new symbol
LogInByAccountParam so compilation and consistency are preserved.
---
Nitpick comments:
In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy`:
- Around line 34-40: The instance field "reflections" is shadowed by a local
variable in the constructor SdkDataStructureGenerator() and is never used;
either remove the unused instance field "reflections" or set that field inside
the constructor instead of declaring a new local Reflections variable (replace
"Reflections reflections = Platform.getReflections()" with "this.reflections =
Platform.getReflections()"), and ensure other code (e.g., sites using
Platform.reflections or the constructor logic that populates responseClasses and
laterResolvedClasses) consistently uses the instance field if you choose to keep
it.
- Around line 79-90: The generated SourceClassMap uses raw HashMap
instantiations which cause unchecked warnings; change the constructors for
srcToDstMapping and dstToSrcMapping from new HashMap() to include generics (e.g.
new HashMap<String, String>() or new HashMap<>()) so the declarations public
final static HashMap<String, String> srcToDstMapping and dstToSrcMapping match
their instantiations and eliminate raw-type usage in the SourceClassMap class.
In `@rest/src/main/resources/scripts/templates/base_param_types.go.template`:
- Around line 5-7: The template currently imports the time package and
suppresses unused-import errors with the dummy symbol `var _ = time.Now`; if
generated code does not actually use time types, remove the `import "time"` and
the `var _ = time.Now` placeholder to avoid dead imports; if the import is
intentionally reserved for potential generated fields of type `time.Time`, leave
them as-is—search for the `import "time"` line and the `var _ = time.Now`
declaration in the template to make the change.
In `@rest/src/main/resources/scripts/templates/login_params.go.template`:
- Around line 32-51: The struct names use "Login" but you want "LogIn" for
consistency; rename types LoginIAM2VirtualIDWithLdapParam ->
LogInIAM2VirtualIDWithLdapParam, LoginIAM2VirtualIDWithLdapDetailParam ->
LogInIAM2VirtualIDWithLdapDetailParam, LoginIAM2PlatformParam ->
LogInIAM2PlatformParam and LoginIAM2PlatformDetailParam ->
LogInIAM2PlatformDetailParam, update their type references (e.g., the
LoginIAM2VirtualIDWithLdap field types and LoginIAM2Platform fields) and any
JSON tags or external references if necessary, and then search/replace across
the codebase to update all usages/imports to the new identifiers to avoid
compilation errors.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
rest/src/main/resources/scripts/GoApiTemplate.groovyrest/src/main/resources/scripts/GoInventory.groovyrest/src/main/resources/scripts/GoTestTemplate.groovyrest/src/main/resources/scripts/RestDocumentationGenerator.groovyrest/src/main/resources/scripts/SDK生成器开发指南.mdrest/src/main/resources/scripts/SdkApiTemplate.groovyrest/src/main/resources/scripts/ZStack SDK Go 开发规范与标准.mdrest/src/main/resources/scripts/templates/base_param_types.go.templaterest/src/main/resources/scripts/templates/base_params.go.templaterest/src/main/resources/scripts/templates/client.go.templaterest/src/main/resources/scripts/templates/example_main.gorest/src/main/resources/scripts/templates/login_params.go.templaterest/src/main/resources/scripts/templates/session_additional_views.go.templaterest/src/main/resources/scripts/templates/session_views.go.template
🚧 Files skipped from review as they are similar to previous changes (2)
- rest/src/main/resources/scripts/templates/base_params.go.template
- rest/src/main/resources/scripts/ZStack SDK Go 开发规范与标准.md
Comment on lines
1
to
26
| // Copyright (c) ZStack.io, Inc. | ||
|
|
||
| package view | ||
|
|
||
| import "time" | ||
|
|
||
| // SessionInventoryView Session | ||
| type SessionInventoryView struct { | ||
| Uuid string `json:"uuid"` | ||
| AccountUuid string `json:"accountUuid,omitempty"` | ||
| UserUuid string `json:"userUuid,omitempty"` | ||
| UserType string `json:"userType,omitempty"` | ||
| ExpiredDate time.Time `json:"expiredDate,omitempty"` | ||
| CreateDate time.Time `json:"createDate,omitempty"` | ||
| } | ||
|
|
||
| // WebUISessionView Web UI Session | ||
| type WebUISessionView struct { | ||
| SessionId string `json:"sessionId"` // Session ID | ||
| AccountUuid string `json:"accountUuid"` // Account UUID | ||
| UserUuid string `json:"userUuid"` // User UUID | ||
| UserName string `json:"username"` // Username | ||
| LoginType string `json:"loginType"` // Login type | ||
| CurrentIdentity string `json:"currentIdentity"` // Current identity | ||
| ZSVersion string `json:"zsVersion"` // ZStack Cloud version | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the template files exist and compare them
echo "=== Checking if files exist ==="
ls -la rest/src/main/resources/scripts/templates/session_*.go.template 2>&1
echo ""
echo "=== Comparing session template files ==="
if [ -f rest/src/main/resources/scripts/templates/session_views.go.template ] && [ -f rest/src/main/resources/scripts/templates/session_additional_views.go.template ]; then
diff rest/src/main/resources/scripts/templates/session_views.go.template \
rest/src/main/resources/scripts/templates/session_additional_views.go.template && echo "Files are identical" || echo "Files are different"
else
echo "One or both files do not exist"
fi
echo ""
echo "=== Searching for references to session_views.go.template ==="
rg "session_views\.go\.template" rest/src --include="*.java" --include="*.groovy" -i 2>&1 || echo "No references found"
echo ""
echo "=== Searching for references to session_additional_views.go.template ==="
rg "session_additional_views\.go\.template" rest/src --include="*.java" --include="*.groovy" -i 2>&1 || echo "No references found"Repository: MatheMatrix/zstack
Length of output: 1459
🏁 Script executed:
#!/bin/bash
# Search for references using correct rg syntax
echo "=== Searching for references to session_views.go.template ==="
rg "session_views\.go\.template" rest/src -t java -t groovy 2>&1 || echo "No references found"
echo ""
echo "=== Searching for references to session_additional_views.go.template ==="
rg "session_additional_views\.go\.template" rest/src -t java -t groovy 2>&1 || echo "No references found"
echo ""
echo "=== Search for session_views in all context ==="
rg "session_views" rest/src -t java -t groovy -A 2 -B 2 2>&1 | head -50
echo ""
echo "=== Search for session_additional_views in all context ==="
rg "session_additional_views" rest/src -t java -t groovy -A 2 -B 2 2>&1 | head -50
echo ""
echo "=== Search for GoInventory ==="
rg "GoInventory" rest/src -t java -t groovy -A 2 -B 2 2>&1 | head -100Repository: MatheMatrix/zstack
Length of output: 7527
删除未使用的重复模板文件 session_views.go.template
此文件与 session_additional_views.go.template 内容完全相同。搜索结果表明，仅 session_additional_views.go.template 被 GoInventory.groovy 引用和使用，而 session_views.go.template 在整个代码库中没有任何引用。建议删除此重复未使用的模板文件以减少维护负担。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/templates/session_views.go.template` around
lines 1 - 26, The file session_views.go.template duplicates the types
SessionInventoryView and WebUISessionView already present in
session_additional_views.go.template and is unused; remove the duplicate
template file session_views.go.template, verify there are no references to
SessionInventoryView or WebUISessionView pointing to that file (ensure code
generation uses session_additional_views.go.template), and run the repository's
build/generation step to confirm no missing-template errors.
4c28489 to
d89f518
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (14)
rest/src/main/resources/scripts/templates/example_main.go (2)
28-43:⚠️ Potential issue | 🟠 Major登录成功后应立即注册
defer Logout当前多个错误分支（第 31、42 行）会提前
return，导致已登录会话不会被释放，造成会话泄漏。建议修复
sessionView, err := cli.Login(ctx) if err != nil { fmt.Printf("登录失败: %v\n", err) return } fmt.Printf("登录成功！Session UUID: %s\n", sessionView.Uuid) + defer func() { + if err := cli.Logout(ctx); err != nil { + fmt.Printf("登出失败: %v\n", err) + } + }()同时删除第 85-91 行的手动登出代码。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/example_main.go` around lines 28 - 43, After successful login via cli.Login(ctx) (the call that returns sessionView and err), immediately register a deferred logout: call cli.Logout(ctx, sessionView.Uuid) with defer so any early returns (e.g., after QueryVmInstance) always release the session; remove the later manual logout block (the explicit Logout calls around the functions in the example_main.go template) to avoid double-logout. Ensure you reference the returned sessionView.Uuid when calling cli.Logout in the defer and handle/log any error inside the deferred call if needed.
18-21:⚠️ Potential issue | 🟡 Minor
DefaultZSConfig参数与文档不一致根据规范文档，
DefaultZSConfig需要显式传入 context path（如"/zstack"），但此示例只传入了 hostname。建议修复
- config := client.DefaultZSConfig("YOUR_ZSTACK_API_ENDPOINT"). + config := client.DefaultZSConfig("YOUR_ZSTACK_API_ENDPOINT", "/zstack"). LoginAccount("admin", "password"). Debug(true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/example_main.go` around lines 18 - 21, The call to DefaultZSConfig currently only passes the hostname but the spec requires an explicit context path; update the example in example_main.go so DefaultZSConfig is invoked with both the API endpoint and the context path (e.g., "/zstack"), then keep the fluent calls LoginAccount("admin","password") and Debug(true) unchanged; locate the DefaultZSConfig invocation and add the second argument for the context path accordingly.rest/src/main/resources/scripts/templates/login_params.go.template (1)
5-8:⚠️ Potential issue | 🟡 Minor结构体命名不一致：
LoginByAccountParam应改为LogInByAccountParam
LoginByAccountParam使用 "Login"，而LogInByUserParam使用 "LogIn"。后端 Java API 登录相关类统一使用 "LogIn" 命名约定（如LogInAction、APILogInByAccountMsg）。建议保持一致性。建议修复
-type LoginByAccountParam struct { +type LogInByAccountParam struct { BaseParam - LoginByAccount LoginByAccountDetailParam `json:"logInByAccount"` + LogInByAccount LogInByAccountDetailParam `json:"logInByAccount"` } -type LoginByAccountDetailParam struct { +type LogInByAccountDetailParam struct {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/login_params.go.template` around lines 5 - 8, Rename the struct type LoginByAccountParam to LogInByAccountParam to match the project's "LogIn" naming convention and update all internal references/usages; keep the nested type LoginByAccountDetailParam (or rename similarly if needed) and preserve the json tag `logInByAccount` unless API contract requires change. Ensure constructors, serializers, deserializers, and any template consumers that reference LoginByAccountParam are updated to the new LogInByAccountParam identifier so builds and codegen remain consistent.rest/src/main/resources/scripts/ZStack SDK Go 开发规范与标准.md (2)
953-991:⚠️ Potential issue | 🟡 Minor章节编号重复：11.8 出现两次
第 953 行 "11.8 客户端代码生成简化" 和第 991 行 "11.8 可选字段指针类型支持" 编号重复，导致文档结构混乱。
建议修复
-### 11.8 可选字段指针类型支持（2026-01 新增） +### 11.9 可选字段指针类型支持（2026-01 新增）🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md around lines 953 - 991, The document has duplicate section numbers: the heading "11.8 客户端代码生成简化" and the later heading "11.8 可选字段指针类型支持" should not share the same number; update the later heading to "11.9 可选字段指针类型支持" (and increment any subsequent section numbers accordingly), and also update any table-of-contents entries or cross-references that mention "11.8" so they point to the new "11.9" heading; locate the headings by their literal texts "11.8 客户端代码生成简化" and "11.8 可选字段指针类型支持" and adjust numbering consistently throughout the document.
825-831:⚠️ Potential issue | 🟡 MinorBaseInfoView 中 Name 字段的必填性定义不一致
此处将
Name定义为可选字段（使用omitempty），但在第 1005-1008 行的注释中明确说明 "uuid 和 name 始终必填"。建议统一规范。建议修复
如果 name 确实是必填字段：
type BaseInfoView struct { UUID string `json:"uuid"` // 资源唯一标识 - Name string `json:"name,omitempty"` // 资源名称 + Name string `json:"name"` // 资源名称（必填） }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md around lines 825 - 831, BaseInfoView's Name field has an inconsistent required/optional definition: update the code so the struct tag and the documentation match. Either (A) make Name truly required by removing `omitempty` from BaseInfoView.Name and keep the comment "uuid and name 始终必填", or (B) keep `omitempty` and change the comment at the earlier doc block (the one that currently says "uuid 和 name 始终必填") to reflect that Name is optional; modify the BaseInfoView struct or the comment accordingly to ensure they are consistent.rest/src/main/resources/scripts/SDK生成器开发指南.md (1)
309-309:⚠️ Potential issue | 🟡 Minor中文表述可微调
"使用单独的集合追踪发现阶段处理的类" 建议改为 "使用独立集合来追踪发现阶段处理的类"，表达更自然流畅。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SDK生成器开发指南.md` at line 309, Update the sentence "使用单独的集合追踪发现阶段处理的类" to the more natural wording "使用独立集合来追踪发现阶段处理的类" in the SDK生成器开发指南; locate the exact phrase in the document (the line containing 使用单独的集合追踪发现阶段处理的类) and replace it with the suggested phrasing, preserving surrounding punctuation and formatting.rest/src/main/resources/scripts/GoTestTemplate.groovy (1)
353-431:⚠️ Potential issue | 🟠 Major其他客户端测试方法缺少
ctx参数
generateGetClientTest、generateCreateClientTest、generateUpdateClientTest和generateDeleteClientTest生成的测试代码仍未传递ctx参数，与 SDK 设计规范不一致。建议修复 generateGetClientTest
\tdefer cleanup() +\tctx := context.Background() -\tresult, err := cli.${methodName}("test-uuid-001") +\tresult, err := cli.${methodName}(ctx, "test-uuid-001")类似地修复
generateCreateClientTest、generateUpdateClientTest和generateDeleteClientTest。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoTestTemplate.groovy` around lines 353 - 431, The generated client tests (generateGetClientTest, generateCreateClientTest, generateUpdateClientTest, generateDeleteClientTest) are missing the ctx parameter when calling the client methods; update each template so the call passes a context (e.g. context.Background()) as the first argument: for generateGetClientTest call cli.${methodName}(context.Background(), "test-uuid-001"), for generateCreateClientTest call cli.${methodName}(context.Background(), param.${paramStruct}{}), for generateUpdateClientTest call cli.${methodName}(context.Background(), "test-uuid-001", param.${paramStruct}{}), and for generateDeleteClientTest call cli.${methodName}(context.Background(), "test-uuid-001", param.DeleteModePermissive); also ensure the test file imports the "context" package if not already present.rest/src/main/resources/scripts/templates/base_params.go.template (1)
87-93:⚠️ Potential issue | 🔴 Critical
ConvertStruct2UrlValues在param == nil时会 panic第 87 行在判空之前调用
reflect.TypeOf(param).Kind()会导致运行时 panic。当param == nil时，reflect.TypeOf(nil)返回 nil，对 nil 调用.Kind()会触发 panic。需要调整检查顺序：先判空再进行反射操作。建议修复
func ConvertStruct2UrlValues(param interface{}) (url.Values, error) { - if reflect.Ptr != reflect.TypeOf(param).Kind() { - return nil, errors.New("model should be pointer kind") - } - result := url.Values{} - if param == nil || reflect.ValueOf(param).IsNil() { + if param == nil { return nil, errors.New("param is nil") } + t := reflect.TypeOf(param) + v := reflect.ValueOf(param) + if t.Kind() != reflect.Ptr || v.IsNil() { + return nil, errors.New("param should be a non-nil pointer") + } + if t.Elem().Kind() != reflect.Struct { + return nil, errors.New("param should point to a struct") + } + result := url.Values{} s := structs.New(param)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/base_params.go.template` around lines 87 - 93, ConvertStruct2UrlValues currently calls reflect.TypeOf(param).Kind() before checking param == nil which panics when param is nil; move the nil-check ahead of any reflect.TypeOf/reflect.ValueOf usage: first validate param != nil (and only after that call reflect.TypeOf(param) to get its Kind and compare to reflect.Ptr), then use reflect.ValueOf(param).IsNil() safely; reference the ConvertStruct2UrlValues function and the param variable when making this reorder.rest/src/main/resources/scripts/GoInventory.groovy (1)
160-176:⚠️ Potential issue | 🟠 Major
generate()入口缺少状态重置，重复执行会污染结果。Line 160 起直接复用了
allApiTemplates、generatedViewStructs、generatedParamStructs、generatedClientMethods、longJobMappings等状态；同 JVM 重复调用会出现重复生成或漏生成。建议修复
List<SdkFile> generate() { def files = [] + // reset mutable state for idempotent generation in same JVM + inventories.clear() + markedInventories.clear() + viewStructNameMap.clear() + additionalClasses.clear() + generatedViewStructs.clear() + generatedViewFiles.clear() + longJobMappings.clear() + paramNestedTypes.clear() + generatedParamStructs.clear() + generatedClientMethods.clear() + currentGeneratingClass = null + allApiTemplates.clear() + GoApiTemplate.groupedApiNames.clear() logger.warn("[GoSDK] ===== GoInventory.generate() START =====")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoInventory.groovy` around lines 160 - 176, The generate() method reuses mutable class-level state across invocations causing duplicate or missing outputs; at the start of generate() explicitly reset/initialize the collections and maps used by the method (e.g. allApiTemplates, generatedViewStructs, generatedParamStructs, generatedClientMethods, longJobMappings, inventories and any other cached lists) so each call starts from a clean state before calling scanLongJobMappings(), getInventories(), or prepareApiTemplates(); locate these fields and assign empty lists/maps (or recreate them) at the top of the generate() function to prevent cross-call contamination.rest/src/main/resources/scripts/RestDocumentationGenerator.groovy (2)
2107-2107:⚠️ Potential issue | 🟠 Major
allTo字段查找仍未覆盖继承层级。Line 2107 使用
getDeclaredField()只查当前类；当字段定义在父类时会直接失败。建议修复
- fsToAdd[at.allTo()] = responseClass.getDeclaredField(at.allTo()) + Field target = FieldUtils.getField(at.allTo(), responseClass) + if (target == null) { + throw new CloudRuntimeException("cannot find field[${at.allTo()}] on class[${responseClass.name}] or its super classes") + } + fsToAdd[at.allTo()] = target🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy` at line 2107, The code uses responseClass.getDeclaredField(at.allTo()) which only checks the current class and fails if the field is declared in a superclass; update the logic that populates fsToAdd (where fsToAdd[at.allTo()] = responseClass.getDeclaredField(...)) to search the class hierarchy instead: implement or call a utility (e.g., a new findFieldInHierarchy(responseClass, at.allTo()) helper) that walks responseClass and its superclasses calling getDeclaredField until found (or returns null/throws a clear exception), and use that result when assigning fsToAdd[at.allTo()]; ensure you reference the existing symbols responseClass, at.allTo(), and fsToAdd in the fix.
1935-1937:⚠️ Potential issue | 🟠 Major不要吞掉
MarkDown.generate()的异常。Line 1935-1937 当前只打日志后返回
null，会把真实失败点延后成下游 NPE，排障成本很高。建议修复
} catch (Exception e) { logger.warn(e.message, e) + throw new CloudRuntimeException("failed to generate markdown for ${doc?._rest?._request?._clz?.name}", e) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy` around lines 1935 - 1937, The current catch block around MarkDown.generate() only logs the exception (logger.warn(e.message, e)) and returns null, which hides the root cause and leads to downstream NPEs; change the handler so after logging you rethrow the original exception (or wrap and throw a RuntimeException/IllegalStateException with e as the cause) instead of returning null, ensuring the failure from MarkDown.generate() surfaces immediately; locate the catch handling the call to MarkDown.generate() and replace the silent swallow with logger.warn(...) followed by throw e (or throw new RuntimeException("Failed to generate markdown", e)).rest/src/main/resources/scripts/SdkApiTemplate.groovy (2)
155-155:⚠️ Potential issue | 🔴 Critical
SdkApiTemplate.getPackageName(...)仍在被调用，符号解析风险很高。当前类已是
SdkDataStructureGenerator，但这些位置仍依赖SdkApiTemplate.getPackageName(...)。如果仓库中没有同名静态方法，会在生成阶段直接失败。#!/bin/bash # 1) 查找 SdkApiTemplate / SdkDataStructureGenerator 的类定义与 getPackageName 定义 rg -n "class\\s+SdkApiTemplate|class\\s+SdkDataStructureGenerator|getPackageName\\s*\\(" --type groovy rest/src/main/resources/scripts # 2) 查找所有对 SdkApiTemplate.getPackageName 的调用点 rg -n "SdkApiTemplate\\.getPackageName\\s*\\(" --type groovy rest/src/main/resources/scriptsAlso applies to: 169-169, 179-179, 281-281, 297-297
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` at line 155, The template still calls the external static SdkApiTemplate.getPackageName(...) which risks unresolved-symbol failures during generation; update those calls to use the current generator's method (replace SdkApiTemplate.getPackageName(...) with SdkDataStructureGenerator.getPackageName(...) or the local equivalent) in all locations shown (including the import-add line and the other occurrences around lines noted), or alternatively add a local static getPackageName method to SdkDataStructureGenerator and invoke that; ensure only the class-local symbol (SdkDataStructureGenerator.getPackageName) is referenced so generation won't depend on SdkApiTemplate.
264-265:⚠️ Potential issue | 🟠 Major
fieldsTo分支仍只查当前类字段。Line 264-265 使用
responseClass.getDeclaredField(src)，父类字段会漏掉；本文件已有getFieldRecursively(...)，建议统一使用。建议修复
- Field f = responseClass.getDeclaredField(src) + Field f = getFieldRecursively(responseClass, src) addToFields(dst, f)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` around lines 264 - 265, In the fieldsTo branch the code uses responseClass.getDeclaredField(src) which misses inherited fields; replace that call with the existing getFieldRecursively(responseClass, src) and pass its returned Field into addToFields(dst, f) so inherited fields are discovered; update the occurrence of responseClass.getDeclaredField(src) in the fieldsTo handling to call getFieldRecursively(...) instead and keep using addToFields(dst, f).rest/src/main/resources/scripts/templates/client.go.template (1)
207-208:⚠️ Potential issue | 🟡 Minor
retVal == nil时不应丢弃响应头。
hql()和Zql()当前在retVal == nil分支返回(nil, nil)，会导致调用方拿不到respHeader。建议修复
if retVal == nil { - return nil, nil + return respHeader, nil } @@ if retVal == nil { - return nil, nil + return respHeader, nil }Also applies to: 223-224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/client.go.template` around lines 207 - 208, When retVal == nil the code currently returns nil, nil and discards respHeader; update the retVal==nil branch in both hql() and Zql() to return the response header instead of nil so callers receive respHeader (i.e. return nil, respHeader[, nil] matching the function's return signature), ensuring you preserve the existing return-value order and include any trailing error return if present.
🧹 Nitpick comments (3)
rest/src/main/resources/scripts/SDK生成器开发指南.md (1)
132-135: 代码块缺少语言标识符第 133 行和第 359 行的代码块缺少语言标识符，建议添加以支持语法高亮。
建议修复
-``` +```text 处理 Java 类型 fieldType:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SDK生成器开发指南.md` around lines 132 - 135, The markdown has fenced code blocks without language identifiers (notably around the examples used in the context for mapJavaTypeToGoType()); update those fenced blocks by adding an appropriate language tag (e.g., ```text or ```java) to enable syntax highlighting and clarity—search for the code blocks that show "处理 Java 类型 fieldType:" and other example snippets near the mapJavaTypeToGoType() discussion and add the language identifier to each opening ``` fence.rest/src/main/resources/scripts/templates/base_param_types.go.template (1)
5-7: 可移除未使用的time导入当前文件中没有任何字段使用
time.Time类型，time包仅通过var _ = time.Now抑制未使用警告。如果确实不需要，建议直接移除该导入。建议修复
-import "time" - -var _ = time.Now // avoid unused import🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/base_param_types.go.template` around lines 5 - 7, Remove the unused time import and its suppression placeholder: delete the import "time" statement and the line "var _ = time.Now" from the template (base_param_types.go.template) so there is no unused package import; if any fields later require time.Time, re-add the import where actually used.rest/src/main/resources/scripts/GoApiTemplate.groovy (1)
42-56: 静态集合的线程安全性这些静态集合（
generatedParamFiles、generatedActionFiles等）使用的是非线程安全的HashSet和HashMap。如果生成过程未来变为并发执行，可能会导致数据竞争。当前单线程执行场景下不会有问题，但建议考虑使用
ConcurrentHashMap.newKeySet()或Collections.synchronizedSet()以增强代码的健壮性。🤖 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 42 - 56, The static collections (generatedParamFiles, generatedActionFiles, generatedViewFiles, groupedApiNames, skippedApis) and the longJobMappings Map are created with non-thread-safe HashSet/HashMap; change them to thread-safe implementations (e.g., use ConcurrentHashMap.newKeySet() for sets or Collections.synchronizedSet(...) and use ConcurrentHashMap for longJobMappings) by replacing the existing initializers for generatedParamFiles, generatedActionFiles, generatedViewFiles, groupedApiNames, skippedApis and longJobMappings in GoApiTemplate.groovy so the collections remain safe if generation becomes concurrent.
🤖 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 584-589: The pluralization logic for varName incorrectly converts
any word ending with "y" to "ies"; update the logic that sets varName (derived
from resourceName) to only replace a trailing "y" with "ies" when the preceding
letter is a consonant (i.e., not a, e, i, o, u); otherwise, for vowel+y keep the
"y" and append "s". Locate the block that computes varName from resourceName and
add a check of the character before the final 'y' (handle single-letter names
safely) using a vowel set before performing the y→ies substitution, leaving the
existing fallback that appends "s" when not ending with "s".
In `@rest/src/main/resources/scripts/GoTestTemplate.groovy`:
- Around line 328-351: The generated test in generateQueryClientTest has a
malformed ctx declaration (missing tab indentation before "ctx :=
context.Background()") and the test file's imports generated by
generateClientTestFile are missing the "context" package; fix
generateQueryClientTest to prepend a "\t" to the "ctx := context.Background()"
line so the generated code aligns with other indented lines, and update
generateClientTestFile's import list to include "context" so generated tests
compile and use context correctly.
In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy`:
- Around line 2324-2325: DELETE parameters are being marked as body while the
curl generator treats DELETE as queryString; update the location assignment so
DELETE is marked as query like GET: modify the conditional using at.method() to
include HttpMethod.DELETE (so it sets location = LOCATION_QUERY for DELETE
requests) and verify the curl construction logic that treats DELETE as
queryString remains consistent with this change (ensure the curl generator's
DELETE handling and the LOCATION_QUERY enum/constant are used consistently).
---
Duplicate comments:
In `@rest/src/main/resources/scripts/GoInventory.groovy`:
- Around line 160-176: The generate() method reuses mutable class-level state
across invocations causing duplicate or missing outputs; at the start of
generate() explicitly reset/initialize the collections and maps used by the
method (e.g. allApiTemplates, generatedViewStructs, generatedParamStructs,
generatedClientMethods, longJobMappings, inventories and any other cached lists)
so each call starts from a clean state before calling scanLongJobMappings(),
getInventories(), or prepareApiTemplates(); locate these fields and assign empty
lists/maps (or recreate them) at the top of the generate() function to prevent
cross-call contamination.
In `@rest/src/main/resources/scripts/GoTestTemplate.groovy`:
- Around line 353-431: The generated client tests (generateGetClientTest,
generateCreateClientTest, generateUpdateClientTest, generateDeleteClientTest)
are missing the ctx parameter when calling the client methods; update each
template so the call passes a context (e.g. context.Background()) as the first
argument: for generateGetClientTest call cli.${methodName}(context.Background(),
"test-uuid-001"), for generateCreateClientTest call
cli.${methodName}(context.Background(), param.${paramStruct}{}), for
generateUpdateClientTest call cli.${methodName}(context.Background(),
"test-uuid-001", param.${paramStruct}{}), and for generateDeleteClientTest call
cli.${methodName}(context.Background(), "test-uuid-001",
param.DeleteModePermissive); also ensure the test file imports the "context"
package if not already present.
In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy`:
- Line 2107: The code uses responseClass.getDeclaredField(at.allTo()) which only
checks the current class and fails if the field is declared in a superclass;
update the logic that populates fsToAdd (where fsToAdd[at.allTo()] =
responseClass.getDeclaredField(...)) to search the class hierarchy instead:
implement or call a utility (e.g., a new findFieldInHierarchy(responseClass,
at.allTo()) helper) that walks responseClass and its superclasses calling
getDeclaredField until found (or returns null/throws a clear exception), and use
that result when assigning fsToAdd[at.allTo()]; ensure you reference the
existing symbols responseClass, at.allTo(), and fsToAdd in the fix.
- Around line 1935-1937: The current catch block around MarkDown.generate() only
logs the exception (logger.warn(e.message, e)) and returns null, which hides the
root cause and leads to downstream NPEs; change the handler so after logging you
rethrow the original exception (or wrap and throw a
RuntimeException/IllegalStateException with e as the cause) instead of returning
null, ensuring the failure from MarkDown.generate() surfaces immediately; locate
the catch handling the call to MarkDown.generate() and replace the silent
swallow with logger.warn(...) followed by throw e (or throw new
RuntimeException("Failed to generate markdown", e)).
In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy`:
- Line 155: The template still calls the external static
SdkApiTemplate.getPackageName(...) which risks unresolved-symbol failures during
generation; update those calls to use the current generator's method (replace
SdkApiTemplate.getPackageName(...) with
SdkDataStructureGenerator.getPackageName(...) or the local equivalent) in all
locations shown (including the import-add line and the other occurrences around
lines noted), or alternatively add a local static getPackageName method to
SdkDataStructureGenerator and invoke that; ensure only the class-local symbol
(SdkDataStructureGenerator.getPackageName) is referenced so generation won't
depend on SdkApiTemplate.
- Around line 264-265: In the fieldsTo branch the code uses
responseClass.getDeclaredField(src) which misses inherited fields; replace that
call with the existing getFieldRecursively(responseClass, src) and pass its
returned Field into addToFields(dst, f) so inherited fields are discovered;
update the occurrence of responseClass.getDeclaredField(src) in the fieldsTo
handling to call getFieldRecursively(...) instead and keep using
addToFields(dst, f).
In `@rest/src/main/resources/scripts/SDK生成器开发指南.md`:
- Line 309: Update the sentence "使用单独的集合追踪发现阶段处理的类" to the more natural wording
"使用独立集合来追踪发现阶段处理的类" in the SDK生成器开发指南; locate the exact phrase in the document
(the line containing 使用单独的集合追踪发现阶段处理的类) and replace it with the suggested
phrasing, preserving surrounding punctuation and formatting.
In `@rest/src/main/resources/scripts/templates/base_params.go.template`:
- Around line 87-93: ConvertStruct2UrlValues currently calls
reflect.TypeOf(param).Kind() before checking param == nil which panics when
param is nil; move the nil-check ahead of any reflect.TypeOf/reflect.ValueOf
usage: first validate param != nil (and only after that call
reflect.TypeOf(param) to get its Kind and compare to reflect.Ptr), then use
reflect.ValueOf(param).IsNil() safely; reference the ConvertStruct2UrlValues
function and the param variable when making this reorder.
In `@rest/src/main/resources/scripts/templates/client.go.template`:
- Around line 207-208: When retVal == nil the code currently returns nil, nil
and discards respHeader; update the retVal==nil branch in both hql() and Zql()
to return the response header instead of nil so callers receive respHeader (i.e.
return nil, respHeader[, nil] matching the function's return signature),
ensuring you preserve the existing return-value order and include any trailing
error return if present.
In `@rest/src/main/resources/scripts/templates/example_main.go`:
- Around line 28-43: After successful login via cli.Login(ctx) (the call that
returns sessionView and err), immediately register a deferred logout: call
cli.Logout(ctx, sessionView.Uuid) with defer so any early returns (e.g., after
QueryVmInstance) always release the session; remove the later manual logout
block (the explicit Logout calls around the functions in the example_main.go
template) to avoid double-logout. Ensure you reference the returned
sessionView.Uuid when calling cli.Logout in the defer and handle/log any error
inside the deferred call if needed.
- Around line 18-21: The call to DefaultZSConfig currently only passes the
hostname but the spec requires an explicit context path; update the example in
example_main.go so DefaultZSConfig is invoked with both the API endpoint and the
context path (e.g., "/zstack"), then keep the fluent calls
LoginAccount("admin","password") and Debug(true) unchanged; locate the
DefaultZSConfig invocation and add the second argument for the context path
accordingly.
In `@rest/src/main/resources/scripts/templates/login_params.go.template`:
- Around line 5-8: Rename the struct type LoginByAccountParam to
LogInByAccountParam to match the project's "LogIn" naming convention and update
all internal references/usages; keep the nested type LoginByAccountDetailParam
(or rename similarly if needed) and preserve the json tag `logInByAccount`
unless API contract requires change. Ensure constructors, serializers,
deserializers, and any template consumers that reference LoginByAccountParam are
updated to the new LogInByAccountParam identifier so builds and codegen remain
consistent.
In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md:
- Around line 953-991: The document has duplicate section numbers: the heading
"11.8 客户端代码生成简化" and the later heading "11.8 可选字段指针类型支持" should not share the
same number; update the later heading to "11.9 可选字段指针类型支持" (and increment any
subsequent section numbers accordingly), and also update any table-of-contents
entries or cross-references that mention "11.8" so they point to the new "11.9"
heading; locate the headings by their literal texts "11.8 客户端代码生成简化" and "11.8
可选字段指针类型支持" and adjust numbering consistently throughout the document.
- Around line 825-831: BaseInfoView's Name field has an inconsistent
required/optional definition: update the code so the struct tag and the
documentation match. Either (A) make Name truly required by removing `omitempty`
from BaseInfoView.Name and keep the comment "uuid and name 始终必填", or (B) keep
`omitempty` and change the comment at the earlier doc block (the one that
currently says "uuid 和 name 始终必填") to reflect that Name is optional; modify the
BaseInfoView struct or the comment accordingly to ensure they are consistent.
---
Nitpick comments:
In `@rest/src/main/resources/scripts/GoApiTemplate.groovy`:
- Around line 42-56: The static collections (generatedParamFiles,
generatedActionFiles, generatedViewFiles, groupedApiNames, skippedApis) and the
longJobMappings Map are created with non-thread-safe HashSet/HashMap; change
them to thread-safe implementations (e.g., use ConcurrentHashMap.newKeySet() for
sets or Collections.synchronizedSet(...) and use ConcurrentHashMap for
longJobMappings) by replacing the existing initializers for generatedParamFiles,
generatedActionFiles, generatedViewFiles, groupedApiNames, skippedApis and
longJobMappings in GoApiTemplate.groovy so the collections remain safe if
generation becomes concurrent.
In `@rest/src/main/resources/scripts/SDK生成器开发指南.md`:
- Around line 132-135: The markdown has fenced code blocks without language
identifiers (notably around the examples used in the context for
mapJavaTypeToGoType()); update those fenced blocks by adding an appropriate
language tag (e.g., ```text or ```java) to enable syntax highlighting and
clarity—search for the code blocks that show "处理 Java 类型 fieldType:" and other
example snippets near the mapJavaTypeToGoType() discussion and add the language
identifier to each opening ``` fence.
In `@rest/src/main/resources/scripts/templates/base_param_types.go.template`:
- Around line 5-7: Remove the unused time import and its suppression
placeholder: delete the import "time" statement and the line "var _ = time.Now"
from the template (base_param_types.go.template) so there is no unused package
import; if any fields later require time.Time, re-add the import where actually
used.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
rest/src/main/resources/scripts/GoApiTemplate.groovyrest/src/main/resources/scripts/GoInventory.groovyrest/src/main/resources/scripts/GoTestTemplate.groovyrest/src/main/resources/scripts/RestDocumentationGenerator.groovyrest/src/main/resources/scripts/SDK生成器开发指南.mdrest/src/main/resources/scripts/SdkApiTemplate.groovyrest/src/main/resources/scripts/ZStack SDK Go 开发规范与标准.mdrest/src/main/resources/scripts/templates/base_param_types.go.templaterest/src/main/resources/scripts/templates/base_params.go.templaterest/src/main/resources/scripts/templates/client.go.templaterest/src/main/resources/scripts/templates/example_main.gorest/src/main/resources/scripts/templates/login_params.go.templaterest/src/main/resources/scripts/templates/session_additional_views.go.template
Comment on lines
+328
to
+351
| private String generateQueryClientTest(String prefix, String methodName, String snakeName) { | ||
| return """\ | ||
| func Test${methodName}_Client(t *testing.T) { | ||
| \tcli, cleanup := newMockClient(func(w http.ResponseWriter, r *http.Request) { | ||
| \t\tassertEqual(t, http.MethodGet, r.Method) | ||
| \t\tw.WriteHeader(http.StatusOK) | ||
| \t\tw.Write(mockInventoriesResponse(map[string]interface{}{ | ||
| \t\t\t"uuid": "test-uuid-001", | ||
| \t\t\t"name": "test-${snakeName}", | ||
| \t\t})) | ||
| \t}) | ||
| \tdefer cleanup() | ||
| ctx := context.Background() | ||
| \tqueryParam := param.NewQueryParam() | ||
| \tresult, err := cli.${methodName}(ctx, &queryParam) | ||
| \tassertNoError(t, err) | ||
| \tif len(result) == 0 { | ||
| \t\tt.Fatal("expected at least one result") | ||
| \t} | ||
| \tassertEqual(t, "test-uuid-001", result[0].UUID) | ||
| } | ||
|
|
||
| """ | ||
| } |
There was a problem hiding this comment.
生成的单元测试 ctx 参数修复不完整
generateQueryClientTest 中添加了 ctx 声明（第 340 行），但存在以下问题：
- 第 340 行缩进不正确（缺少
\t前缀） - 需要在导入中添加
"context"包（第 293-297 行）
建议修复
\tdefer cleanup()
-ctx := context.Background()
+\tctx := context.Background()
\tqueryParam := param.NewQueryParam()同时在 generateClientTestFile 的导入中添加 context：
content.append("import (\n")
+content.append("\t\"context\"\n")
content.append("\t\"net/http\"\n")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/GoTestTemplate.groovy` around lines 328 -
351, The generated test in generateQueryClientTest has a malformed ctx
declaration (missing tab indentation before "ctx := context.Background()") and
the test file's imports generated by generateClientTestFile are missing the
"context" package; fix generateQueryClientTest to prepend a "\t" to the "ctx :=
context.Background()" line so the generated code aligns with other indented
lines, and update generateClientTestFile's import list to include "context" so
generated tests compile and use context correctly.
Comment on lines
2324
to
2325
| } else if (at.method() == HttpMethod.GET) { | ||
| location = LOCATION_QUERY |
There was a problem hiding this comment.
DELETE 参数被标成 body，和 curl 生成逻辑冲突。
Line 2324 仅将 GET 标记为 query，但 Line 1625 在 curl 构造中把 DELETE 也当 queryString。会导致 DELETE API 的参数文档与示例不一致。
建议修复
- } else if (at.method() == HttpMethod.GET) {
+ } else if (at.method() == HttpMethod.GET || at.method() == HttpMethod.DELETE) {
location = LOCATION_QUERY
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } else if (at.method() == HttpMethod.GET) { | |
| location = LOCATION_QUERY | |
| } else if (at.method() == HttpMethod.GET || at.method() == HttpMethod.DELETE) { | |
| location = LOCATION_QUERY |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy` around
lines 2324 - 2325, DELETE parameters are being marked as body while the curl
generator treats DELETE as queryString; update the location assignment so DELETE
is marked as query like GET: modify the conditional using at.method() to include
HttpMethod.DELETE (so it sets location = LOCATION_QUERY for DELETE requests) and
verify the curl construction logic that treats DELETE as queryString remains
consistent with this change (ensure the curl generator's DELETE handling and the
LOCATION_QUERY enum/constant are used consistently).
d89f518 to
b07698e
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (8)
rest/src/main/resources/scripts/ZStack SDK Go 开发规范与标准.md (3)
825-830:⚠️ Potential issue | 🟠 Major
BaseInfoView.Name的必填语义与标签不一致这里使用了
json:"name,omitempty"，与文档里“uuid/name 必填”的规则冲突。请统一为必填表达或同步调整规则描述。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md around lines 825 - 830, The struct tag for BaseInfoView.Name currently uses `json:"name,omitempty"` which makes Name optional at JSON marshal time while docs require "uuid/name" to be required; update the tag to remove `omitempty` (i.e., `json:"name"`) to make Name required, or if you choose to keep it optional, update the documentation to reflect that change—modify the `BaseInfoView` definition (field Name) to align code and docs consistently.
991-991:⚠️ Potential issue | 🟡 Minor章节编号重复（11.8 出现两次）
建议将第二个“11.8”顺延为“11.9”，并同步目录/引用。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md at line 991, The second heading "### 11.8 可选字段指针类型支持（2026-01 新增）" is a duplicate number; update that heading to "### 11.9 可选字段指针类型支持（2026-01 新增）" and also update any corresponding table-of-contents entries and internal references that point to "11.8" so they point to "11.9" (search for the exact heading text and "11.8" occurrences in the document to locate and change them).
279-304:⚠️ Potential issue | 🟠 Major时间类型规范前后冲突，当前标准不明确
前文仍在示例中强调
ZStackTime/BaseTimeView，但后文 11.1 明确要求移除并统一time.Time。建议在本节明确“历史实现”或直接改成当前推荐写法。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md around lines 279 - 304, The document shows conflicting time-type guidance: remove the legacy ZStackTime type and BaseTimeView usage and update examples to use standard time.Time consistently (e.g., change CreateDate/LastOpDate to time.Time in BaseTimeView or embed time.Time directly in VmInstanceInventoryView), or explicitly mark the ZStackTime/BaseTimeView block as historical/deprecated; update the example structs (ZStackTime, BaseTimeView, VmInstanceInventoryView) and explanatory text to reflect the single recommended approach (use time.Time) so the spec no longer contradicts section 11.1.rest/src/main/resources/scripts/RestDocumentationGenerator.groovy (2)
2107-2107:⚠️ Potential issue | 🟠 Major
allTo字段查找应支持继承层级
getDeclaredField()只查当前类；字段在父类时会直接失败。建议修复
- fsToAdd[at.allTo()] = responseClass.getDeclaredField(at.allTo()) + Field f = FieldUtils.getField(at.allTo(), responseClass) + if (f == null) { + throw new CloudRuntimeException("cannot find field[${at.allTo()}] from class[${responseClass.name}]") + } + fsToAdd[at.allTo()] = f🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy` at line 2107, The lookup using responseClass.getDeclaredField(at.allTo()) fails for inherited fields; change the logic that populates fsToAdd (where fsToAdd[at.allTo()] = responseClass.getDeclaredField(at.allTo())) to walk the inheritance chain: start from responseClass and loop through superclasses trying getDeclaredField(at.allTo()) until found (or throw/handle NoSuchFieldException), then assign that Field into fsToAdd[at.allTo()]; ensure you reference at.allTo() and preserve existing exception handling.
1935-1937:⚠️ Potential issue | 🟠 Major不要吞掉 markdown 生成异常
这里只记录日志不抛出，会把真实失败延后成隐式问题（例如返回
null）。建议修复
} catch (Exception e) { logger.warn(e.message, e) + throw new CloudRuntimeException("failed to generate markdown", e) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy` around lines 1935 - 1937, The catch block in RestDocumentationGenerator.groovy currently swallows exceptions (catch (Exception e) { logger.warn(e.message, e) }) which hides markdown-generation failures and can lead to null/implicit errors; change this to log and rethrow (or throw a new unchecked exception with the original as the cause) from the same location so callers of the markdown-generation method get a failure instead of a silent null—update the catch in the method that performs markdown generation to either rethrow e or throw new RuntimeException("Markdown generation failed", e) after logging, and ensure the callers handle or propagate the exception appropriately.rest/src/main/resources/scripts/templates/base_params.go.template (1)
87-93:⚠️ Potential issue | 🔴 Critical先判空再反射，否则
nil入参会直接 panic当前顺序在
param == nil时会在反射阶段崩溃；并且还缺少“必须是指向 struct 的指针”校验。建议修复
func ConvertStruct2UrlValues(param interface{}) (url.Values, error) { - if reflect.Ptr != reflect.TypeOf(param).Kind() { - return nil, errors.New("model should be pointer kind") - } - result := url.Values{} - if param == nil || reflect.ValueOf(param).IsNil() { + if param == nil { return nil, errors.New("param is nil") } + t := reflect.TypeOf(param) + v := reflect.ValueOf(param) + if t.Kind() != reflect.Ptr || v.IsNil() { + return nil, errors.New("param should be a non-nil pointer") + } + if t.Elem().Kind() != reflect.Struct { + return nil, errors.New("param should point to a struct") + } + result := url.Values{} s := structs.New(param)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/templates/base_params.go.template` around lines 87 - 93, Move the nil check before any reflection and add a strict pointer-to-struct validation: first return an error if param == nil, then get t := reflect.TypeOf(param) and ensure t.Kind() == reflect.Ptr, then ensure t.Elem().Kind() == reflect.Struct; only after those checks use reflect.ValueOf(param) and IsNil() (or dereference) as needed; update the error messages to indicate "param is nil" and "param must be a pointer to a struct" to reference the failing condition for symbols shown (param, reflect.TypeOf, reflect.Ptr, reflect.ValueOf).rest/src/main/resources/scripts/SdkApiTemplate.groovy (2)
155-155:⚠️ Potential issue | 🔴 Critical
SdkApiTemplate.getPackageName(...)依赖未定义，生成流程会中断当前类已是
SdkDataStructureGenerator，但仍硬编码调用SdkApiTemplate.getPackageName(...)。若仓库无该静态方法，会直接失败。#!/bin/bash set -e # 检查类定义与 getPackageName 定义/调用关系 rg -nP 'class\s+SdkApiTemplate|class\s+SdkDataStructureGenerator|getPackageName\s*\(' rest/src/main/resources/scripts --type groovy建议修复（示例）
- imports.add("${SdkApiTemplate.getPackageName(f.type)}.${simpleName}") + imports.add("${getPackageName(f.type)}.${simpleName}") ... - String packageName = SdkApiTemplate.getPackageName(clz) + String packageName = getPackageName(clz) ... - String packageName = SdkApiTemplate.getPackageName(responseClass) + String packageName = getPackageName(responseClass)static String getPackageName(Class clz) { return clz?.package?.name }Also applies to: 169-170, 179-179, 281-281, 297-297
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` at line 155, 当前代码在 SdkDataStructureGenerator 中硬编码调用 SdkApiTemplate.getPackageName(...) 导致若该静态方法不存在生成流程会中断；在 SdkDataStructureGenerator 中新增一个静态方法 getPackageName(Class clz)（返回 clz?.package?.name 或等价安全取值）并将现有调用（例如 imports.add("${SdkApiTemplate.getPackageName(f.type)}.${simpleName}") 以及其在同文件的其他出现）改为调用本类的静态方法（或去掉类限定符直接调用 getPackageName），保证包名解析不依赖外部类。
264-265:⚠️ Potential issue | 🟠 Major字段映射仍使用
getDeclaredField，父类字段会漏解析这里应与同文件递归查找策略保持一致，否则
RestResponse.fieldsTo指向继承字段时会失败。建议修复
- Field f = responseClass.getDeclaredField(src) + Field f = getFieldRecursively(responseClass, src) addToFields(dst, f)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy` around lines 264 - 265, The code uses responseClass.getDeclaredField(src) which misses inherited fields; change it to search the class hierarchy (like the other recursive lookup in this template) so inherited fields are found before failing: implement or call a helper (e.g., getFieldFromHierarchy/lookupFieldInSuperclasses) that walks responseClass and its superclasses to locate src, then pass the found Field to addToFields(dst, field); ensure the behavior matches the existing recursive strategy used elsewhere in this file.
🧹 Nitpick comments (3)
rest/src/main/resources/scripts/GoApiTemplate.groovy (2)
42-44: 未使用的静态字段
generatedParamFiles、generatedActionFiles和generatedViewFiles这三个静态集合在当前代码中没有被使用。GoInventory类有自己的实例级跟踪集合（如generatedParamStructs、generatedViewStructs）来完成相同的功能。如果这些字段是为将来预留的，建议添加注释说明；否则可以移除以减少维护负担。
🤖 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 42 - 44, The three unused static fields generatedParamFiles, generatedActionFiles, and generatedViewFiles in GoApiTemplate.groovy are redundant because GoInventory already tracks generatedParamStructs and generatedViewStructs at instance level; remove these unused static Set<String> fields from the file (or, if intended for future use, add a clarifying comment above each declaration) to avoid dead code and reduce maintenance burden; search for generatedParamFiles, generatedActionFiles, and generatedViewFiles to ensure no references remain before committing the deletion.
1261-1264: 制表符格式不一致此处使用实际的 Tab 字符进行缩进，而代码其他位置（如 lines 969-971）使用
\t转义序列。虽然两者在 Groovy 多行字符串中都有效，但混合使用可能导致生成的 Go 代码格式不一致。return """func (cli *ZSClient) ${clzName}(ctx context.Context, ${params}, deleteMode param.DeleteMode) error { - return cli.DeleteWithSpec(ctx, "${cleanPath}", ${firstParam}, ${spec}, ${paramsStr}, nil) +\treturn cli.DeleteWithSpec(ctx, "${cleanPath}", ${firstParam}, ${spec}, ${paramsStr}, nil) } """🤖 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 1261 - 1264, The multi-line string for the generated Go method uses actual Tab characters for indentation causing inconsistent formatting vs. other template lines that use the "\t" escape; update the returned string in GoApiTemplate.groovy (the block building the "func (cli *ZSClient) ${clzName}(...)" template) to replace literal tab characters with "\t" escape sequences so the generated Go code matches the existing "\t"-based indentation style (ensure all leading tabs inside the triple-quoted string use "\t" consistently).rest/src/main/resources/scripts/GoInventory.groovy (1)
309-313: 外层NoSuchFieldExceptioncatch 块不可达在
findInventoryClass方法中，内层的两个 try 块（lines 260-287 和 290-308）已经分别捕获了NoSuchFieldException。因此，外层 catch 块（line 309-310）永远不会被执行，属于死代码。建议移除不可达代码
} catch (NoSuchFieldException e) { // ignore } - } catch (NoSuchFieldException e) { - logger.debug("[GoSDK] No 'inventories' field in " + responseClass?.simpleName) } catch (Exception e) { logger.warn("[GoSDK] Error finding inventory class for " + clzName + ": " + e.message) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rest/src/main/resources/scripts/GoInventory.groovy` around lines 309 - 313, The outer NoSuchFieldException catch in findInventoryClass is unreachable because the two inner try blocks already catch NoSuchFieldException; remove the outer catch block (or merge logic) so there is no dead code—ensure any necessary error handling is moved into the inner try/catch blocks or let the exception propagate from findInventoryClass as intended; update findInventoryClass to keep only the required catches for the inner try blocks and remove the outer NoSuchFieldException handler.
🤖 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/GoTestTemplate.groovy`:
- Around line 475-477: The template calls DefaultZSConfig with only hostname but
the SDK signature is DefaultZSConfig(hostname, contextPath string) — update the
GoTestTemplate.groovy generator to pass a contextPath argument (e.g., the
configured contextPath variable or "/" as appropriate) when constructing config;
also modify TestMain so it does not rely on defer for logout: replace the
pattern that calls defer testCli.Logout(ctx) followed immediately by
os.Exit(m.Run()) with capturing exitCode := m.Run(), explicitly call
testCli.Logout(ctx) (or handle its error) and then call os.Exit(exitCode) to
ensure logout runs; references: DefaultZSConfig, TestMain, testCli.Logout,
m.Run.
- Around line 485-487: 不要在调用 os.Exit() 之前依赖 defer 来执行清理；将 m.Run() 的返回值保存为一个变量（调用
m.Run()），然后在显式调用 testCli.Logout(ctx) 执行清理，最后根据保存的返回值调用 os.Exit() 退出。也就是说，确保先执行
testCli.Logout(ctx)（而不是依赖 defer）再调用 os.Exit(returnCode)，并在说明中引用 m.Run(),
testCli.Logout 和 os.Exit 三个标识符以便定位修改位置。
In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy`:
- Around line 1324-1330: The generated Python example incorrectly includes a
Java-style declaration string "${actionName}.Result res = action.call()"; update
the code that builds the example (the cols.add call that uses
"${actionName}.Result res = action.call()") to produce a valid Python call
without type syntax, e.g. "res = action.call()"; also locate and update the
other place in this generator that emits the same Java-style string so both
Python SDK samples use "res = action.call()".
- Around line 1564-1566: 在 catch (InvocationTargetException e) 的处理里不要用
e.getTargetException().printStackTrace()（它返回 void 并副作用打印到 stderr），而应把目标异常作为
cause 传入并在消息中引用其具体信息：构造 CloudRuntimeException 时使用新消息字符串包含 clz.name 和
e.getTargetException().getMessage()（或 toString()）并将 e.getTargetException() 作为异常的
cause（例如 new CloudRuntimeException("cannot generate...: " + target.getMessage(),
target)），以保留原始异常链并避免副作用；相关符号：InvocationTargetException, e.getTargetException(),
CloudRuntimeException, clz.name, __example__().
In `@rest/src/main/resources/scripts/SDK生成器开发指南.md`:
- Line 133: Two fenced code blocks in the SDK生成器开发指南.md are missing language
identifiers (triggering markdownlint MD040); update those two code blocks by
adding appropriate language tags — use ```text for the plain/text snippet and
```groovy for the Groovy snippet — so each fenced block declares its language
and resolves the lint warnings.
---
Duplicate comments:
In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy`:
- Line 2107: The lookup using responseClass.getDeclaredField(at.allTo()) fails
for inherited fields; change the logic that populates fsToAdd (where
fsToAdd[at.allTo()] = responseClass.getDeclaredField(at.allTo())) to walk the
inheritance chain: start from responseClass and loop through superclasses trying
getDeclaredField(at.allTo()) until found (or throw/handle NoSuchFieldException),
then assign that Field into fsToAdd[at.allTo()]; ensure you reference at.allTo()
and preserve existing exception handling.
- Around line 1935-1937: The catch block in RestDocumentationGenerator.groovy
currently swallows exceptions (catch (Exception e) { logger.warn(e.message, e)
}) which hides markdown-generation failures and can lead to null/implicit
errors; change this to log and rethrow (or throw a new unchecked exception with
the original as the cause) from the same location so callers of the
markdown-generation method get a failure instead of a silent null—update the
catch in the method that performs markdown generation to either rethrow e or
throw new RuntimeException("Markdown generation failed", e) after logging, and
ensure the callers handle or propagate the exception appropriately.
In `@rest/src/main/resources/scripts/SdkApiTemplate.groovy`:
- Line 155: 当前代码在 SdkDataStructureGenerator 中硬编码调用
SdkApiTemplate.getPackageName(...) 导致若该静态方法不存在生成流程会中断；在
SdkDataStructureGenerator 中新增一个静态方法 getPackageName(Class clz)（返回
clz?.package?.name 或等价安全取值）并将现有调用（例如
imports.add("${SdkApiTemplate.getPackageName(f.type)}.${simpleName}")
以及其在同文件的其他出现）改为调用本类的静态方法（或去掉类限定符直接调用 getPackageName），保证包名解析不依赖外部类。
- Around line 264-265: The code uses responseClass.getDeclaredField(src) which
misses inherited fields; change it to search the class hierarchy (like the other
recursive lookup in this template) so inherited fields are found before failing:
implement or call a helper (e.g.,
getFieldFromHierarchy/lookupFieldInSuperclasses) that walks responseClass and
its superclasses to locate src, then pass the found Field to addToFields(dst,
field); ensure the behavior matches the existing recursive strategy used
elsewhere in this file.
In `@rest/src/main/resources/scripts/templates/base_params.go.template`:
- Around line 87-93: Move the nil check before any reflection and add a strict
pointer-to-struct validation: first return an error if param == nil, then get t
:= reflect.TypeOf(param) and ensure t.Kind() == reflect.Ptr, then ensure
t.Elem().Kind() == reflect.Struct; only after those checks use
reflect.ValueOf(param) and IsNil() (or dereference) as needed; update the error
messages to indicate "param is nil" and "param must be a pointer to a struct" to
reference the failing condition for symbols shown (param, reflect.TypeOf,
reflect.Ptr, reflect.ValueOf).
In `@rest/src/main/resources/scripts/ZStack` SDK Go 开发规范与标准.md:
- Around line 825-830: The struct tag for BaseInfoView.Name currently uses
`json:"name,omitempty"` which makes Name optional at JSON marshal time while
docs require "uuid/name" to be required; update the tag to remove `omitempty`
(i.e., `json:"name"`) to make Name required, or if you choose to keep it
optional, update the documentation to reflect that change—modify the
`BaseInfoView` definition (field Name) to align code and docs consistently.
- Line 991: The second heading "### 11.8 可选字段指针类型支持（2026-01 新增）" is a duplicate
number; update that heading to "### 11.9 可选字段指针类型支持（2026-01 新增）" and also update
any corresponding table-of-contents entries and internal references that point
to "11.8" so they point to "11.9" (search for the exact heading text and "11.8"
occurrences in the document to locate and change them).
- Around line 279-304: The document shows conflicting time-type guidance: remove
the legacy ZStackTime type and BaseTimeView usage and update examples to use
standard time.Time consistently (e.g., change CreateDate/LastOpDate to time.Time
in BaseTimeView or embed time.Time directly in VmInstanceInventoryView), or
explicitly mark the ZStackTime/BaseTimeView block as historical/deprecated;
update the example structs (ZStackTime, BaseTimeView, VmInstanceInventoryView)
and explanatory text to reflect the single recommended approach (use time.Time)
so the spec no longer contradicts section 11.1.
---
Nitpick comments:
In `@rest/src/main/resources/scripts/GoApiTemplate.groovy`:
- Around line 42-44: The three unused static fields generatedParamFiles,
generatedActionFiles, and generatedViewFiles in GoApiTemplate.groovy are
redundant because GoInventory already tracks generatedParamStructs and
generatedViewStructs at instance level; remove these unused static Set<String>
fields from the file (or, if intended for future use, add a clarifying comment
above each declaration) to avoid dead code and reduce maintenance burden; search
for generatedParamFiles, generatedActionFiles, and generatedViewFiles to ensure
no references remain before committing the deletion.
- Around line 1261-1264: The multi-line string for the generated Go method uses
actual Tab characters for indentation causing inconsistent formatting vs. other
template lines that use the "\t" escape; update the returned string in
GoApiTemplate.groovy (the block building the "func (cli *ZSClient)
${clzName}(...)" template) to replace literal tab characters with "\t" escape
sequences so the generated Go code matches the existing "\t"-based indentation
style (ensure all leading tabs inside the triple-quoted string use "\t"
consistently).
In `@rest/src/main/resources/scripts/GoInventory.groovy`:
- Around line 309-313: The outer NoSuchFieldException catch in
findInventoryClass is unreachable because the two inner try blocks already catch
NoSuchFieldException; remove the outer catch block (or merge logic) so there is
no dead code—ensure any necessary error handling is moved into the inner
try/catch blocks or let the exception propagate from findInventoryClass as
intended; update findInventoryClass to keep only the required catches for the
inner try blocks and remove the outer NoSuchFieldException handler.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
rest/src/main/resources/scripts/GoApiTemplate.groovyrest/src/main/resources/scripts/GoInventory.groovyrest/src/main/resources/scripts/GoTestTemplate.groovyrest/src/main/resources/scripts/RestDocumentationGenerator.groovyrest/src/main/resources/scripts/SDK生成器开发指南.mdrest/src/main/resources/scripts/SdkApiTemplate.groovyrest/src/main/resources/scripts/ZStack SDK Go 开发规范与标准.mdrest/src/main/resources/scripts/templates/base_param_types.go.templaterest/src/main/resources/scripts/templates/base_params.go.templaterest/src/main/resources/scripts/templates/client.go.templaterest/src/main/resources/scripts/templates/example_main.gorest/src/main/resources/scripts/templates/login_params.go.templaterest/src/main/resources/scripts/templates/session_additional_views.go.template
🚧 Files skipped from review as they are similar to previous changes (4)
- rest/src/main/resources/scripts/templates/example_main.go
- rest/src/main/resources/scripts/templates/session_additional_views.go.template
- rest/src/main/resources/scripts/templates/login_params.go.template
- rest/src/main/resources/scripts/templates/client.go.template
Comment on lines
+475
to
+477
| \tconfig := client.DefaultZSConfig(hostname). | ||
| \t\tLoginAccount(account, password). | ||
| \t\tDebug(true) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "*.groovy" -o -name "*.go" | head -20Repository: MatheMatrix/zstack
Length of output: 1720
🏁 Script executed:
fd -e groovy -e go | grep -E "(GoTestTemplate|DefaultZSConfig)" | head -20Repository: MatheMatrix/zstack
Length of output: 115
🏁 Script executed:
rg -n "DefaultZSConfig" --type groovy --type go -B 2 -A 5Repository: MatheMatrix/zstack
Length of output: 2455
🏁 Script executed:
find . -name "GoTestTemplate.groovy" -exec wc -l {} \;Repository: MatheMatrix/zstack
Length of output: 121
🏁 Script executed:
find . -name "GoTestTemplate.groovy" -exec cat -n {} \; | sed -n '470,490p'Repository: MatheMatrix/zstack
Length of output: 631
DefaultZSConfig 调用缺少必需参数，生成的测试代码无法编译
当前生成的代码仅传递 hostname 参数，但 SDK 中 DefaultZSConfig 的实际签名要求两个参数：DefaultZSConfig(hostname, contextPath string)。生成的集成测试会在编译时直接失败。需要在生成逻辑中补充 contextPath 参数。
此外，TestMain 中存在另一个关键问题：
- 行 485：
defer testCli.Logout(ctx) - 行 487：
os.Exit(m.Run())
在 Go 中，os.Exit() 会立即终止程序，绕过所有 defer 语句执行，导致登出清理逻辑永远不会执行。应改用 exitCode := m.Run() 后再调用 testCli.Logout(ctx)，最后才 os.Exit(exitCode)。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/GoTestTemplate.groovy` around lines 475 -
477, The template calls DefaultZSConfig with only hostname but the SDK signature
is DefaultZSConfig(hostname, contextPath string) — update the
GoTestTemplate.groovy generator to pass a contextPath argument (e.g., the
configured contextPath variable or "/" as appropriate) when constructing config;
also modify TestMain so it does not rely on defer for logout: replace the
pattern that calls defer testCli.Logout(ctx) followed immediately by
os.Exit(m.Run()) with capturing exitCode := m.Run(), explicitly call
testCli.Logout(ctx) (or handle its error) and then call os.Exit(exitCode) to
ensure logout runs; references: DefaultZSConfig, TestMain, testCli.Logout,
m.Run.
Comment on lines
+485
to
+487
| \tdefer testCli.Logout(ctx) | ||
|
|
||
| \tos.Exit(m.Run()) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read the relevant section
fd GoTestTemplate.groovy --type f | head -5Repository: MatheMatrix/zstack
Length of output: 115
🏁 Script executed:
# Count lines to understand file size
wc -l rest/src/main/resources/scripts/GoTestTemplate.groovy 2>/dev/null || echo "File not found in expected location"Repository: MatheMatrix/zstack
Length of output: 119
🏁 Script executed:
# Search for the file across the repository
find . -name "GoTestTemplate.groovy" -type f 2>/dev/nullRepository: MatheMatrix/zstack
Length of output: 117
🏁 Script executed:
sed -n '480,495p' rest/src/main/resources/scripts/GoTestTemplate.groovyRepository: MatheMatrix/zstack
Length of output: 441
os.Exit() 会跳过 defer 语句，导致 Logout 清理逻辑失效
在 Go 中，os.Exit() 直接终止进程，不会执行已注册的 defer 语句。此代码中 testCli.Logout(ctx) 永远无法执行，会导致资源泄漏或连接未正确关闭。
修复方案
-\tdefer testCli.Logout(ctx)
-
-\tos.Exit(m.Run())
+\tcode := m.Run()
+\t_ = testCli.Logout(ctx)
+\tos.Exit(code)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/GoTestTemplate.groovy` around lines 485 -
487, 不要在调用 os.Exit() 之前依赖 defer 来执行清理；将 m.Run() 的返回值保存为一个变量（调用 m.Run()），然后在显式调用
testCli.Logout(ctx) 执行清理，最后根据保存的返回值调用 os.Exit() 退出。也就是说，确保先执行
testCli.Logout(ctx)（而不是依赖 defer）再调用 os.Exit(returnCode)，并在说明中引用 m.Run(),
testCli.Logout 和 os.Exit 三个标识符以便定位修改位置。
Comment on lines
1324
to
1330
| def cols = ["${actionName} action = ${actionName}()"] | ||
| List<String> conds = getQueryConditionExampleOfTheClass(clz) | ||
| conds = conds.collect { return "\"" + it + "\""} | ||
| cols.add("action.conditions = [${conds.join(",")}]".toString()) | ||
| cols.add("action.sessionId = \"b86c9016b4f24953a9edefb53ca0678c\"") | ||
| cols.add("res = action.call()") | ||
| cols.add("action.conditions = [${conds.join(",")}]") | ||
| cols.add("""action.sessionId = "b86c9016b4f24953a9edefb53ca0678c\"""") | ||
| cols.add("${actionName}.Result res = action.call()") | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the RestDocumentationGenerator.groovy file
find . -name "RestDocumentationGenerator.groovy" -type fRepository: MatheMatrix/zstack
Length of output: 129
🏁 Script executed:
# Read the file to understand the context around lines 1324-1330
cat -n rest/src/main/resources/scripts/RestDocumentationGenerator.groovy | sed -n '1320,1340p'Repository: MatheMatrix/zstack
Length of output: 899
🏁 Script executed:
# Also check around lines 1823-1837
cat -n rest/src/main/resources/scripts/RestDocumentationGenerator.groovy | sed -n '1820,1845p'Repository: MatheMatrix/zstack
Length of output: 961
🏁 Script executed:
# Let's search for other occurrences of similar patterns with ".Result res"
rg "\.Result res" rest/src/main/resources/scripts/RestDocumentationGenerator.groovy -nRepository: MatheMatrix/zstack
Length of output: 343
🏁 Script executed:
# Let's understand the broader context - search for Python SDK related code
rg -i "python" rest/src/main/resources/scripts/RestDocumentationGenerator.groovy -n -B 3 -A 3 | head -80Repository: MatheMatrix/zstack
Length of output: 952
Python SDK 示例包含 Java 风格类型声明，生成的代码无法在 Python 中执行
行 1329 和 1836 中的 "${actionName}.Result res = action.call()" 使用了 Java 风格的类型声明，这在 Python 中无效。Python 不支持这种显式类型声明语法。应该改为：
- cols.add("${actionName}.Result res = action.call()")
+ cols.add("res = action.call()")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def cols = ["${actionName} action = ${actionName}()"] | |
| List<String> conds = getQueryConditionExampleOfTheClass(clz) | |
| conds = conds.collect { return "\"" + it + "\""} | |
| cols.add("action.conditions = [${conds.join(",")}]".toString()) | |
| cols.add("action.sessionId = \"b86c9016b4f24953a9edefb53ca0678c\"") | |
| cols.add("res = action.call()") | |
| cols.add("action.conditions = [${conds.join(",")}]") | |
| cols.add("""action.sessionId = "b86c9016b4f24953a9edefb53ca0678c\"""") | |
| cols.add("${actionName}.Result res = action.call()") | |
| def cols = ["${actionName} action = ${actionName}()"] | |
| List<String> conds = getQueryConditionExampleOfTheClass(clz) | |
| conds = conds.collect { return "\"" + it + "\""} | |
| cols.add("action.conditions = [${conds.join(",")}]") | |
| cols.add("""action.sessionId = "b86c9016b4f24953a9edefb53ca0678c\"""") | |
| cols.add("res = action.call()") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy` around
lines 1324 - 1330, The generated Python example incorrectly includes a
Java-style declaration string "${actionName}.Result res = action.call()"; update
the code that builds the example (the cols.add call that uses
"${actionName}.Result res = action.call()") to produce a valid Python call
without type syntax, e.g. "res = action.call()"; also locate and update the
other place in this generator that emits the same Java-style string so both
Python SDK samples use "res = action.call()".
Comment on lines
1564
to
1566
| } catch (InvocationTargetException e) { | ||
| throw new CloudRuntimeException("cannot generate the markdown document for the class[${clz.name}], the __example__() method has an error: ${e.getTargetException().printStackTrace()}") | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "RestDocumentationGenerator.groovy"Repository: MatheMatrix/zstack
Length of output: 127
🏁 Script executed:
sed -n '1560,1570p' rest/src/main/resources/scripts/RestDocumentationGenerator.groovyRepository: MatheMatrix/zstack
Length of output: 500
🏁 Script executed:
fd "CloudRuntimeException\\.java" --type fRepository: MatheMatrix/zstack
Length of output: 137
🏁 Script executed:
cat -n header/src/main/java/org/zstack/header/exception/CloudRuntimeException.javaRepository: MatheMatrix/zstack
Length of output: 746
InvocationTargetException 重新包装时丢失原始 cause
printStackTrace() 在字符串插值中返回 void，导致消息内容无效且产生副作用（打印到标准错误），同时未保留原始异常链。
建议修复
try {
example = m.invoke(null)
} catch (InvocationTargetException e) {
- throw new CloudRuntimeException("cannot generate the markdown document for the class[${clz.name}], the __example__() method has an error: ${e.getTargetException().printStackTrace()}")
+ Throwable target = e.getTargetException()
+ throw new CloudRuntimeException(
+ "cannot generate the markdown document for the class[${clz.name}], the __example__() method has an error: ${target?.message}",
+ target
+ )
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/RestDocumentationGenerator.groovy` around
lines 1564 - 1566, 在 catch (InvocationTargetException e) 的处理里不要用
e.getTargetException().printStackTrace()（它返回 void 并副作用打印到 stderr），而应把目标异常作为
cause 传入并在消息中引用其具体信息：构造 CloudRuntimeException 时使用新消息字符串包含 clz.name 和
e.getTargetException().getMessage()（或 toString()）并将 e.getTargetException() 作为异常的
cause（例如 new CloudRuntimeException("cannot generate...: " + target.getMessage(),
target)），以保留原始异常链并避免副作用；相关符号：InvocationTargetException, e.getTargetException(),
CloudRuntimeException, clz.name, __example__().
| | 无法识别的类型 | `interface{}` | `interface{}` | | ||
|
|
||
| ### 3.2 上下文感知的类型映射 | ||
|
|
There was a problem hiding this comment.
补全 fenced code block 的语言标识
这两处代码块未声明语言，会持续触发 markdownlint 的 MD040。建议按内容分别补成 text/groovy（或实际语言）。
Also applies to: 359-359
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 133-133: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rest/src/main/resources/scripts/SDK生成器开发指南.md` at line 133, Two fenced code
blocks in the SDK生成器开发指南.md are missing language identifiers (triggering
markdownlint MD040); update those two code blocks by adding appropriate language
tags — use ```text for the plain/text snippet and ```groovy for the Groovy
snippet — so each fenced block declares its language and resolves the lint
warnings.
implement the go-sdk generator for zsv. how to use it: 1. cd premium/test-premium 2. mvn test -Djacoco.skip=true -Dtest=TestGenerateGoSDK Resolves: ZSV-11399 Change-Id: I6e6f62696a65696a65677a75786d79616163697a
MatheMatrix
force-pushed
the
sync/zstackio/ZSV-11399
branch
from
b07698e to
94efd8e
Compare
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.
implement the go-sdk generator for zsv.
how to use it:
Resolves: ZSV-11399
Change-Id: I6e6f62696a65696a65677a75786d79616163697a
sync from gitlab !9249