Skip to content

<fix>[rest]: improve markdown validation error reporting #3669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zstack-robot-1 wants to merge 1 commit into
base: feature-5.5.12-zns
Choose a base branch
from
+97 −78
Open

<fix>[rest]: improve markdown validation error reporting #3669

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
175 changes: 97 additions & 78 deletions rest/src/main/resources/scripts/RestDocumentationGenerator.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -787,81 +787,91 @@ class RestDocumentationGenerator implements DocumentGenerator {
return globalConfigMarkDown
}

Boolean isConsistent(GlobalConfigMarkDown md, GlobalConfig globalConfig) {
if (md == null || globalConfig == null) {
return false
}
List<String> isConsistent(GlobalConfigMarkDown md, GlobalConfig globalConfig) {
if (md == null) {
return ["GlobalConfigMarkDown is null"]
}
if (globalConfig == null) {
return ["GlobalConfig is null"]
}
String mdPath =
PathUtil.join(PathUtil.join(Paths.get("../doc").toAbsolutePath().normalize().toString(),
"globalconfig"), md.globalConfig.category, md.globalConfig.name) + ".md"
Comment on lines +790 to 799
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

不要用 Markdown 内容回推报错路径。

这里重新用 md.globalConfig.category/name 组装 mdPath，但这两个字段本身就是后面要校验的内容。只要 Name 或 Category 过期，最终报错就会指向旧路径，而不是当前实际校验的文件，新的聚合报错会误导排查。

💡 建议修改 
-    List<String> isConsistent(GlobalConfigMarkDown md, GlobalConfig globalConfig) {
+    List<String> isConsistent(GlobalConfigMarkDown md, GlobalConfig globalConfig, String mdPath) {
         if (md == null) {
             return ["GlobalConfigMarkDown is null"]
         }
         if (globalConfig == null) {
             return ["GlobalConfig is null"]
         }
-        String mdPath =
-                PathUtil.join(PathUtil.join(Paths.get("../doc").toAbsolutePath().normalize().toString(),
-                        "globalconfig"), md.globalConfig.category, md.globalConfig.name) + ".md"
-        List<String> inconsistencies = isConsistent(markDown, globalConfig)
+        List<String> inconsistencies = isConsistent(markDown, globalConfig, mdPath)

Also applies to: 862-862

🤖 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 790 - 799, The code in isConsistent currently builds mdPath from
md.globalConfig.category and md.globalConfig.name which can be stale; change the
path construction to use the authoritative globalConfig values (e.g.,
globalConfig.category and globalConfig.name) instead of md.globalConfig.*, and
guard nulls (md or globalConfig) already checked; update the mdPath assignment
(the PathUtil.join call that sets mdPath) to reference globalConfig.category and
globalConfig.name so reported paths reflect the actual files being validated.

List<String> classes = new ArrayList<>()
initializer.bindResources.get(globalConfig.getIdentity()).each { classes.add(it.getName()) }
List<String> newClasses = classes.sort()
String validatorString = initializer.validatorMap.get(globalConfig.getIdentity())
Boolean flag = true
if (md.globalConfig.name != globalConfig.name) {
logger.info("name of ${mdPath} is not latest")
flag = false
}
if (md.globalConfig.defaultValue != globalConfig.defaultValue) {
logger.info("defaultValue of ${mdPath} is not latest")
flag = false
}
if (StringUtils.trimToEmpty(md.globalConfig.description) != StringUtils.trimToEmpty(globalConfig.description)) {
logger.info("desc of ${mdPath} is not latest")
flag = false
}
if (md.globalConfig.type != globalConfig.type) {
if (globalConfig.type != null) {
logger.info("type of ${mdPath} is not latest")
flag = false
}
}
if (md.globalConfig.category != globalConfig.category) {
logger.info("category of ${mdPath} is not latest")
flag = false
}
List<String> oldClasses = md.globalConfig.resources.sort()
if (oldClasses != newClasses) {
logger.info("classes of ${mdPath} is not latest")
flag = false
}
List<String> mismatches = []
if (md.globalConfig.name != globalConfig.name) {
logger.info("name of ${mdPath} is not latest")
mismatches.add("name mismatch in ${mdPath}: expected='${md.globalConfig.name}', actual='${globalConfig.name}'")
}
if (md.globalConfig.defaultValue != globalConfig.defaultValue) {
logger.info("defaultValue of ${mdPath} is not latest")
mismatches.add("defaultValue mismatch in ${mdPath}: expected='${md.globalConfig.defaultValue}', actual='${globalConfig.defaultValue}'")
}
if (StringUtils.trimToEmpty(md.globalConfig.description) != StringUtils.trimToEmpty(globalConfig.description)) {
logger.info("desc of ${mdPath} is not latest")
mismatches.add("description mismatch in ${mdPath}: expected='${StringUtils.trimToEmpty(md.globalConfig.description)}', actual='${StringUtils.trimToEmpty(globalConfig.description)}'")
}
if (md.globalConfig.type != globalConfig.type) {
if (globalConfig.type != null) {
logger.info("type of ${mdPath} is not latest")
mismatches.add("type mismatch in ${mdPath}: expected='${md.globalConfig.type}', actual='${globalConfig.type}'")
}
}
if (md.globalConfig.category != globalConfig.category) {
logger.info("category of ${mdPath} is not latest")
mismatches.add("category mismatch in ${mdPath}: expected='${md.globalConfig.category}', actual='${globalConfig.category}'")
}
List<String> oldClasses = md.globalConfig.resources.sort()
if (oldClasses != newClasses) {
logger.info("classes of ${mdPath} is not latest")
mismatches.add("resources mismatch in ${mdPath}: expected='${oldClasses}', actual='${newClasses}'")
}

if (md.globalConfig.valueRange != (validatorString)) {
boolean useBooleanValidator = (globalConfig.type == "java.lang.Boolean"
&& md.globalConfig.valueRange == "{true, false}")
if (validatorString != null || !useBooleanValidator) {
logger.info("valueRange of ${mdPath} is not latest")
logger.info("valueRange = ${md.globalConfig.valueRange} validatorString = ${validatorString}")
flag = false
}
}
return flag
}
if (validatorString != null || !useBooleanValidator) {
logger.info("valueRange of ${mdPath} is not latest")
logger.info("valueRange = ${md.globalConfig.valueRange} validatorString = ${validatorString}")
mismatches.add("valueRange mismatch in ${mdPath}: expected='${md.globalConfig.valueRange}', actual='${validatorString}'")
Comment on lines +805 to +839
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

expected / actual 的方向写反了。

现在的 mismatch 文案把 Markdown 里的旧值写成 expected，把代码里的最新值写成 actual。这会让人按反方向修文档。建议至少对调两者，或者直接改成 markdown= / current= 这类不含歧义的键。

💡 示例 
-            mismatches.add("name mismatch in ${mdPath}: expected='${md.globalConfig.name}', actual='${globalConfig.name}'")
+            mismatches.add("name mismatch in ${mdPath}: markdown='${md.globalConfig.name}', current='${globalConfig.name}'")

其余 defaultValue / description / type / category / resources / valueRange 的报错也需要同样调整。

🤖 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 805 - 839, The mismatch messages currently label the markdown
(md.globalConfig.*) values as "expected" and the code/current (globalConfig /
validatorString / newClasses) values as "actual", which is reversed; update each
mismatches.add call (for name, defaultValue, description, type, category,
resources/classes, valueRange) to either swap the two sides so expected= current
code value and actual= markdown value or — clearer — replace the labels with
unambiguous keys like "markdown=" and "current=" (modify the mismatches.add
string constructions that reference md.globalConfig.name, globalConfig.name,
md.globalConfig.defaultValue, globalConfig.defaultValue,
StringUtils.trimToEmpty(md.globalConfig.description),
StringUtils.trimToEmpty(globalConfig.description), md.globalConfig.type,
globalConfig.type, md.globalConfig.category, globalConfig.category,
oldClasses/newClasses, and md.globalConfig.valueRange/validatorString while
keeping the mdPath context intact).

}
}
return mismatches
}

void checkMD(String mdPath, GlobalConfig globalConfig) {
String result = ShellUtils.runAndReturn(
"grep '${PLACEHOLDER}' ${mdPath}").stdout.replaceAll("\n", "")
if (!result.empty) {
throw new CloudRuntimeException("Placeholders are detected in ${mdPath}, please replace them by content.")
}
GlobalConfigMarkDown markDown = getExistGlobalConfigMarkDown(mdPath)
if (markDown.desc_CN.isEmpty()
|| markDown.name_CN.isEmpty()
|| markDown.valueRangeRemark.isEmpty()
|| markDown.defaultValueRemark.isEmpty()
|| markDown.resourcesGranularitiesRemark.isEmpty()
|| markDown.additionalRemark.isEmpty()
|| markDown.backgroundInformation.isEmpty()
|| markDown.isUIExposed.isEmpty()
|| markDown.isCLIExposed.isEmpty()
) {
throw new CloudRuntimeException("The necessary information of ${mdPath} is missing, please complete the information before submission.")
}
if (!isConsistent(markDown, globalConfig)) {
throw new CloudRuntimeException("${mdPath} is not match with its definition, please use Repair mode to correct it.")
}
}
if (!result.empty) {
throw new CloudRuntimeException("Placeholders are detected in ${mdPath}, please replace them by content.")
}
GlobalConfigMarkDown markDown = getExistGlobalConfigMarkDown(mdPath)
List<String> missingFields = []
if (markDown.desc_CN.isEmpty()) missingFields.add("desc_CN")
if (markDown.name_CN.isEmpty()) missingFields.add("name_CN")
if (markDown.valueRangeRemark.isEmpty()) missingFields.add("valueRangeRemark")
if (markDown.defaultValueRemark.isEmpty()) missingFields.add("defaultValueRemark")
if (markDown.resourcesGranularitiesRemark.isEmpty()) missingFields.add("resourcesGranularitiesRemark")
if (markDown.additionalRemark.isEmpty()) missingFields.add("additionalRemark")
if (markDown.backgroundInformation.isEmpty()) missingFields.add("backgroundInformation")
if (markDown.isUIExposed.isEmpty()) missingFields.add("isUIExposed")
if (markDown.isCLIExposed.isEmpty()) missingFields.add("isCLIExposed")
List<String> inconsistencies = isConsistent(markDown, globalConfig)
if (!missingFields.isEmpty() || !inconsistencies.isEmpty()) {
StringBuilder sb = new StringBuilder("Validation failed for ${mdPath}:\n")
if (!missingFields.isEmpty()) {
sb.append("Missing required fields: ${missingFields}\n")
}
if (!inconsistencies.isEmpty()) {
sb.append("Inconsistent fields:\n")
inconsistencies.each { sb.append("- ${it}\n") }
}
throw new CloudRuntimeException(sb.toString())
}
}

class ElaborationMarkDown {
private def table = ["|编号|描述|原因|操作建议|更多|"]
Expand Down Expand Up @@ -2815,23 +2825,32 @@ ${additionalRemark}
return System.getProperty("ignoreError") != null
}

void testGlobalConfigTemplateAndMarkDown() {
Map<String, GlobalConfig> allConfigs = initializer.configs
allConfigs.each {
String newPath =
PathUtil.join(PathUtil.join(Paths.get("../doc").toAbsolutePath().normalize().toString(),
"globalconfig"), it.value.category, it.value.name) + DEPRECATED + ".md"
if (new File(newPath).exists()) {
void testGlobalConfigTemplateAndMarkDown() {
Map<String, GlobalConfig> allConfigs = initializer.configs
List<String> allErrors = []
allConfigs.each {
String newPath =
PathUtil.join(PathUtil.join(Paths.get("../doc").toAbsolutePath().normalize().toString(),
"globalconfig"), it.value.category, it.value.name) + DEPRECATED + ".md"
if (new File(newPath).exists()) {
return
}
String mdPath =
PathUtil.join(PathUtil.join(Paths.get("../doc").toAbsolutePath().normalize().toString(),
"globalconfig"), it.value.category, it.value.name) + ".md"
File mdFile = new File(mdPath)
if (!mdFile.exists()) {
throw new CloudRuntimeException("Not found the document markdown of the global config ${it.value.name} , please generate it first.")
}
checkMD(mdPath, it.value)
}
}
}
String mdPath =
PathUtil.join(PathUtil.join(Paths.get("../doc").toAbsolutePath().normalize().toString(),
"globalconfig"), it.value.category, it.value.name) + ".md"
File mdFile = new File(mdPath)
if (!mdFile.exists()) {
allErrors.add("Not found the document markdown of the global config ${it.value.name} , please generate it first.")
return
Comment on lines +2841 to +2844
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

缺失 Markdown 的报错需要带上真实路径。

这里只输出了 name，同名配置分布在不同 category 时会变得不可区分；而且这句英文也不够自然。聚合报错里最好直接输出 mdPath，例如 Global config markdown not found: ${mdPath}。

💡 建议修改 
-                allErrors.add("Not found the document markdown of the global config ${it.value.name} , please generate it first.")
+                allErrors.add("Global config markdown not found: ${mdPath}. Please generate it first.")

As per coding guidelines, "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写".

🤖 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 2841 - 2844, The error message when a markdown file is missing currently
only logs it.value.name; update the message to include the actual file path
mdPath and use clear English. Locate the block that constructs mdFile and pushes
to allErrors (variables mdFile, mdPath, allErrors, it.value.name) and replace
the string with a descriptive message such as "Global config markdown not found:
${mdPath}" so each aggregated error shows the real path and is unambiguous.

}
try {
checkMD(mdPath, it.value)
} catch (CloudRuntimeException e) {
allErrors.add(e.message)
}
}
if (!allErrors.isEmpty()) {
throw new CloudRuntimeException(allErrors.join("\n\n"))
}
}
}