go-gin-server: minor updates - #24658
Conversation
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/GoGinServerCodegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/GoGinServerCodegen.java:139">
P3: The regex 'fix' doesn't change behavior. In Java, only the opening brace `{` is a regex metacharacter that must be escaped; a standalone closing `}` is treated as a literal (you only escape it when it closes a `{n,m}` quantifier). So `\{(.*?)}` and the previous `\{(.*?)\}` match exactly the same strings, and the route transformation (/v1/pets/{petId} -> /v1/pets/:petId) is identical in both. Since the PR describes this as preventing malformed routes, the change is a no-op rather than an actual fix. Consider either reverting it or clarifying the real root cause of any malformed-route issue (e.g. the non-greedy group matching across multiple params like `{a}x{b}`), so the fix actually addresses a case the old pattern got wrong.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| for (CodegenOperation op : operationList) { | ||
| if (op.path != null) { | ||
| op.path = op.path.replaceAll("\\{(.*?)\\}", ":$1"); | ||
| op.path = op.path.replaceAll("\\{(.*?)}", ":$1"); |
There was a problem hiding this comment.
P3: The regex 'fix' doesn't change behavior. In Java, only the opening brace { is a regex metacharacter that must be escaped; a standalone closing } is treated as a literal (you only escape it when it closes a {n,m} quantifier). So \{(.*?)} and the previous \{(.*?)\} match exactly the same strings, and the route transformation (/v1/pets/{petId} -> /v1/pets/:petId) is identical in both. Since the PR describes this as preventing malformed routes, the change is a no-op rather than an actual fix. Consider either reverting it or clarifying the real root cause of any malformed-route issue (e.g. the non-greedy group matching across multiple params like {a}x{b}), so the fix actually addresses a case the old pattern got wrong.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/GoGinServerCodegen.java, line 139:
<comment>The regex 'fix' doesn't change behavior. In Java, only the opening brace `{` is a regex metacharacter that must be escaped; a standalone closing `}` is treated as a literal (you only escape it when it closes a `{n,m}` quantifier). So `\{(.*?)}` and the previous `\{(.*?)\}` match exactly the same strings, and the route transformation (/v1/pets/{petId} -> /v1/pets/:petId) is identical in both. Since the PR describes this as preventing malformed routes, the change is a no-op rather than an actual fix. Consider either reverting it or clarifying the real root cause of any malformed-route issue (e.g. the non-greedy group matching across multiple params like `{a}x{b}`), so the fix actually addresses a case the old pattern got wrong.</comment>
<file context>
@@ -137,7 +136,8 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
for (CodegenOperation op : operationList) {
if (op.path != null) {
- op.path = op.path.replaceAll("\\{(.*?)\\}", ":$1");
+ op.path = op.path.replaceAll("\\{(.*?)}", ":$1");
+ LOGGER.debug("Updated operation path: {}", op.path);
}
</file context>
FYI @antihax (2017/11) @grokify (2018/07) @kemokemo (2018/09) @jirikuncar (2021/01) @ph4r5h4d (2021/04) @lwj5 (2023/04)
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
Summary by cubic
Fixes route path conversion in the Go Gin server generator so OpenAPI params like
{id}become Gin:id. Adds debug logging for the transformed paths and renames configs for consistency.Bug Fixes
{param}with:param, preventing malformed routes.Refactors
bin/configs/go-gin-server.yamlandbin/configs/go-gin-server-interface-only.yaml(no content changes).Written for commit 6f0fde8. Summary will update on new commits.