Skip to content

Commit 0883085

Browse files
halfcrazyhalfcrazy
authored
fix(go-server): use operationId consistently to fix uncompilable output for duplicate operationIds (#24169)
When two paths share the same auto-generated operationId (e.g. /foo and /foo/), go-server and go-gin-server produced uncompilable code: the route map and interfaces used {{operationId}} (deduplicated to FooGet_0 by addOperationToGroup) while the controller handler and service stub used {{nickname}} (deduplicated to FooGet_1 by a separate pass with a different counter convention), so routes referenced handlers that didn't exist. Switch the go-server/go-gin-server templates from {{nickname}} to {{operationId}} so routes, interfaces, handlers and service stubs all reference the same deduplicated name. This is a no-op for non-duplicate operationIds and leaves DefaultGenerator untouched, so no other generator is affected. Add regression tests for both generators covering the /foo + /foo/ case.
1 parent ddeb30a commit 0883085

8 files changed

Lines changed: 143 additions & 10 deletions

File tree

‎modules/openapi-generator/src/main/resources/go-gin-server/controller-api.mustache‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ type {{classname}} struct {
1515
{{#isDeprecated}}
1616
// Deprecated
1717
{{/isDeprecated}}
18-
func (api *{{classname}}) {{nickname}}(c *gin.Context) {
18+
func (api *{{classname}}) {{operationId}}(c *gin.Context) {
1919
// Your handler implementation
2020
c.JSON(200, gin.H{"status": "OK"})
2121
}

‎modules/openapi-generator/src/main/resources/go-gin-server/interface-api.mustache‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@ type {{classname}} interface {
1010
1111
1212
{{#operation}}
13-
// {{nickname}} {{httpMethod}} {{{basePathWithoutHost}}}{{{path}}}{{#summary}}
13+
// {{operationId}} {{httpMethod}} {{{basePathWithoutHost}}}{{{path}}}{{#summary}}
1414
// {{{.}}} {{/summary}}
1515
{{#isDeprecated}}
1616
// Deprecated
1717
{{/isDeprecated}}
18-
{{nickname}}(c *gin.Context)
18+
{{operationId}}(c *gin.Context)
1919

2020
{{/operation}}
2121
{{/operations}}

‎modules/openapi-generator/src/main/resources/go-server/controller-api.mustache‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,11 @@ func (c *{{classname}}Controller) OrderedRoutes() []Route {
9292

9393
{{#operations}}{{#operation}}
9494

95-
// {{nickname}} - {{{summary}}}
95+
// {{operationId}} - {{{summary}}}
9696
{{#isDeprecated}}
9797
// Deprecated
9898
{{/isDeprecated}}
99-
func (c *{{classname}}Controller) {{nickname}}(w http.ResponseWriter, r *http.Request) {
99+
func (c *{{classname}}Controller) {{operationId}}(w http.ResponseWriter, r *http.Request) {
100100
{{#hasFormParams}}
101101
{{#isMultipart}}
102102
if err := r.ParseMultipartForm(32 << 20); err != nil {
@@ -641,7 +641,7 @@ func (c *{{classname}}Controller) {{nickname}}(w http.ResponseWriter, r *http.Re
641641
{{/isArray}}
642642
{{/isBodyParam}}
643643
{{/allParams}}
644-
result, err := c.service.{{nickname}}(r.Context(){{#allParams}}, {{#isNullable}}{{#isBodyParam}}&{{/isBodyParam}}{{/isNullable}}{{paramName}}Param{{/allParams}})
644+
result, err := c.service.{{operationId}}(r.Context(){{#allParams}}, {{#isNullable}}{{#isBodyParam}}&{{/isBodyParam}}{{/isNullable}}{{paramName}}Param{{/allParams}})
645645
// If an error occurred, encode the error with the status code
646646
if err != nil {
647647
c.errorHandler(w, r, err, &result)

‎modules/openapi-generator/src/main/resources/go-server/service.mustache‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@ func New{{classname}}Service() *{{classname}}Service {
1919
return &{{classname}}Service{}
2020
}{{#operations}}{{#operation}}
2121

22-
// {{nickname}} - {{summary}}
22+
// {{operationId}} - {{summary}}
2323
{{#isDeprecated}}
2424
// Deprecated
2525
{{/isDeprecated}}
26-
func (s *{{classname}}Service) {{nickname}}(ctx context.Context{{#allParams}}, {{paramName}} {{#isNullable}}*{{/isNullable}}{{dataType}}{{/allParams}}) (ImplResponse, error) {
27-
// TODO - update {{nickname}} with the required logic for this service method.
26+
func (s *{{classname}}Service) {{operationId}}(ctx context.Context{{#allParams}}, {{paramName}} {{#isNullable}}*{{/isNullable}}{{dataType}}{{/allParams}}) (ImplResponse, error) {
27+
// TODO - update {{operationId}} with the required logic for this service method.
2828
// Add {{classFilename}}_service.go to the .openapi-generator-ignore to avoid overwriting this service implementation when updating open api generation.
2929

3030
{{#responses}}
@@ -39,5 +39,5 @@ func (s *{{classname}}Service) {{nickname}}(ctx context.Context{{#allParams}}, {
3939

4040
{{/dataType}}
4141
{{/responses}}
42-
return Response(http.StatusNotImplemented, nil), errors.New("{{nickname}} method not implemented")
42+
return Response(http.StatusNotImplemented, nil), errors.New("{{operationId}} method not implemented")
4343
}{{/operation}}{{/operations}}

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/goginserver/GoGinServerCodegenTest.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@
2020
import org.openapitools.codegen.DefaultGenerator;
2121
import org.openapitools.codegen.TestUtils;
2222
import org.openapitools.codegen.config.CodegenConfigurator;
23+
import org.testng.Assert;
2324
import org.testng.annotations.Test;
2425

2526
import java.io.File;
2627
import java.io.IOException;
28+
import java.nio.charset.StandardCharsets;
2729
import java.nio.file.Files;
2830
import java.nio.file.Paths;
2931
import java.util.List;
@@ -85,6 +87,42 @@ public void verifyInterfaceOnly() throws IOException {
8587
"type PetAPI interface");
8688
}
8789

90+
@Test
91+
public void verifyDuplicateOperationIdsAreConsistent() throws IOException {
92+
// Regression test for duplicate auto-generated operationIds (e.g. /foo and /foo/).
93+
// go-gin-server's routers.mustache references handleFunctions.<Class>.{{operationId}}
94+
// while interface-api.mustache/controller-api.mustache used to declare the method
95+
// with {{nickname}}. nickname and operationId were deduplicated by two independent
96+
// passes with inconsistent counter conventions (_1 vs _0), so routers referenced
97+
// FooGet_0 while the handler/interface declared FooGet_1, producing uncompilable
98+
// code. The fix makes those templates use {{operationId}} consistently.
99+
File output = Files.createTempDirectory("test").toFile();
100+
output.deleteOnExit();
101+
102+
final CodegenConfigurator configurator = createDefaultCodegenConfigurator(output)
103+
.setInputSpec("src/test/resources/3_0/go-gin-server/duplicate-operation-ids.yaml");
104+
105+
DefaultGenerator generator = new DefaultGenerator();
106+
List<File> files = generator.opts(configurator.toClientOptInput()).generate();
107+
files.forEach(File::deleteOnExit);
108+
109+
// routers.go uses {{operationId}} to reference the handler method on the generated struct.
110+
TestUtils.assertFileContains(Paths.get(output + "/go/routers.go"),
111+
"handleFunctions.DefaultAPI.FooGet_0");
112+
// controller-api.mustache now uses {{operationId}} for the handler method, matching
113+
// the {{operationId}} reference in routers.go.
114+
TestUtils.assertFileContains(Paths.get(output + "/go/api_default.go"),
115+
"func (api *DefaultAPI) FooGet_0(");
116+
117+
// Negative assertion: the divergent _1 name must not appear anywhere.
118+
String routers = new String(Files.readAllBytes(Paths.get(output + "/go/routers.go")), StandardCharsets.UTF_8);
119+
String apiDefault = new String(Files.readAllBytes(Paths.get(output + "/go/api_default.go")), StandardCharsets.UTF_8);
120+
Assert.assertFalse(routers.contains("FooGet_1"),
121+
"nickname diverged from operationId (FooGet_1 present in routers.go)");
122+
Assert.assertFalse(apiDefault.contains("FooGet_1"),
123+
"nickname diverged from operationId (FooGet_1 present in api_default.go)");
124+
}
125+
88126
private static CodegenConfigurator createDefaultCodegenConfigurator(File output) {
89127
return new CodegenConfigurator()
90128
.setGeneratorName("go-gin-server")

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/goserver/GoServerCodegenTest.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import java.io.File;
2727
import java.io.IOException;
28+
import java.nio.charset.StandardCharsets;
2829
import java.nio.file.Files;
2930
import java.nio.file.Paths;
3031
import java.util.List;
@@ -74,6 +75,58 @@ public void verifyOrder() throws IOException {
7475

7576
}
7677

78+
@Test
79+
public void verifyDuplicateOperationIdsAreConsistent() throws IOException {
80+
// Regression test for duplicate auto-generated operationIds (e.g. /foo and /foo/).
81+
// Previously nickname and operationId were deduplicated by two independent passes with
82+
// inconsistent counter conventions (_1 vs _0), so the route map / interfaces used
83+
// {{operationId}} (e.g. FooGet_0) while the controller handler / service stub used
84+
// {{nickname}} (e.g. FooGet_1), producing uncompilable code.
85+
File output = Files.createTempDirectory("test").toFile();
86+
output.deleteOnExit();
87+
88+
final CodegenConfigurator configurator = createDefaultCodegenConfigurator(output)
89+
.setInputSpec("src/test/resources/3_0/go-server/duplicate-operation-ids.yaml");
90+
91+
DefaultGenerator generator = new DefaultGenerator();
92+
List<File> files = generator.opts(configurator.toClientOptInput()).generate();
93+
files.forEach(File::deleteOnExit);
94+
95+
// The deduplicated operationId pair is FooGet and FooGet_0; routes, handlers and
96+
// service stubs must all reference the same name (templates use {{operationId}}).
97+
TestUtils.assertFileContains(Paths.get(output + "/go/api_default.go"),
98+
"\"FooGet\": Route{");
99+
TestUtils.assertFileContains(Paths.get(output + "/go/api_default.go"),
100+
"\"FooGet_0\": Route{");
101+
102+
// Route handler reference (uses {{operationId}}) and handler method definition
103+
// (uses {{nickname}}) must reference the same name.
104+
TestUtils.assertFileContains(Paths.get(output + "/go/api_default.go"),
105+
"c.FooGet_0,");
106+
TestUtils.assertFileContains(Paths.get(output + "/go/api_default.go"),
107+
"func (c *DefaultAPIController) FooGet_0(");
108+
// Service call inside the handler must also use the deduplicated operationId.
109+
TestUtils.assertFileContains(Paths.get(output + "/go/api_default.go"),
110+
"c.service.FooGet_0(");
111+
112+
// The interface (api.go) and the service stub must declare FooGet_0 too.
113+
TestUtils.assertFileContains(Paths.get(output + "/go/api.go"),
114+
"FooGet_0(");
115+
TestUtils.assertFileContains(Paths.get(output + "/go/api_default_service.go"),
116+
"func (s *DefaultAPIService) FooGet_0(");
117+
118+
// Negative assertions: the divergent _1 name must not appear anywhere.
119+
String apiDefault = new String(Files.readAllBytes(Paths.get(output + "/go/api_default.go")), StandardCharsets.UTF_8);
120+
String api = new String(Files.readAllBytes(Paths.get(output + "/go/api.go")), StandardCharsets.UTF_8);
121+
String apiService = new String(Files.readAllBytes(Paths.get(output + "/go/api_default_service.go")), StandardCharsets.UTF_8);
122+
Assert.assertFalse(apiDefault.contains("FooGet_1"),
123+
"nickname diverged from operationId (FooGet_1 present in api_default.go)");
124+
Assert.assertFalse(api.contains("FooGet_1"),
125+
"nickname diverged from operationId (FooGet_1 present in api.go)");
126+
Assert.assertFalse(apiService.contains("FooGet_1"),
127+
"nickname diverged from operationId (FooGet_1 present in api_default_service.go)");
128+
}
129+
77130
private static CodegenConfigurator createDefaultCodegenConfigurator(File output) {
78131
return new CodegenConfigurator()
79132
.setGeneratorName("go-server")

‎modules/openapi-generator/src/test/resources/3_0/go-gin-server/duplicate-operation-ids.yaml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
openapi: 3.0.1
2+
3+
info:
4+
version: 1.0.0
5+
title: Duplicate auto-generated operationIds
6+
7+
paths:
8+
/foo:
9+
get:
10+
tags:
11+
- default
12+
responses:
13+
'200':
14+
description: ok
15+
/foo/:
16+
get:
17+
tags:
18+
- default
19+
responses:
20+
'200':
21+
description: ok

‎modules/openapi-generator/src/test/resources/3_0/go-server/duplicate-operation-ids.yaml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
openapi: 3.0.1
2+
3+
info:
4+
version: 1.0.0
5+
title: Duplicate auto-generated operationIds
6+
7+
paths:
8+
/foo:
9+
get:
10+
tags:
11+
- default
12+
responses:
13+
'200':
14+
description: ok
15+
/foo/:
16+
get:
17+
tags:
18+
- default
19+
responses:
20+
'200':
21+
description: ok

0 commit comments

Comments
 (0)