Skip to content

Commit ac013ff

Browse files
fixfonfixfon
authored
[Java][Spring] Do not generate required args constructor that duplicates @lombok.AllArgsConstructor (#25396)
* [Java][Spring] Do not generate required args constructor that duplicates @lombok.AllArgsConstructor When every property of a model is required, the generated required args constructor has the same signature as the one Lombok generates for @lombok.AllArgsConstructor, so the generated class does not compile. Skip the generated constructor in that case and keep it when the signatures differ (nullable properties with openApiNullable, parent properties). Fixes #25395 * [Java][Spring] Keep required args constructor when additionalProperties adds a Lombok parameter The generated additionalProperties map is a field too, so @lombok.AllArgsConstructor takes it as an extra parameter and no longer collides with the required args constructor.
1 parent 7bb0bf1 commit ac013ff

5 files changed

Lines changed: 184 additions & 0 deletions

File tree

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1758,6 +1758,15 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
17581758
&& (cm.hasRequired || cm.vendorExtensions.containsKey("x-java-all-args-constructor"))) {
17591759
cm.vendorExtensions.put("x-java-no-args-constructor", true);
17601760
}
1761+
// the required args constructor only has lombok's signature when every own field is a plain required
1762+
// argument: requiredVars also holds inherited properties, a JsonNullable field takes a plain value
1763+
// and the additionalProperties map is a field too
1764+
if (lombokAnnotations != null && lombokAnnotations.containsKey("AllArgsConstructor")
1765+
&& cm.additionalPropertiesType == null
1766+
&& cm.requiredVars.size() == cm.vars.size()
1767+
&& cm.vars.stream().allMatch(p -> p.required && !(openApiNullable && p.isNullable))) {
1768+
cm.vendorExtensions.put("x-java-lombok-duplicates-required-args-constructor", true);
1769+
}
17611770
}
17621771

17631772
if (substituteGenericPagedModel && !pagedModelRegistry.isEmpty()) {

‎modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ public {{>sealed}}class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}
106106
{{/vendorExtensions.x-java-no-args-constructor}}
107107
{{^lombok.Data}}
108108
{{^lombok.RequiredArgsConstructor}}
109+
{{^vendorExtensions.x-java-lombok-duplicates-required-args-constructor}}
109110
{{#generatedConstructorWithRequiredArgs}}
110111
{{#hasRequired}}
111112

@@ -129,6 +130,7 @@ public {{>sealed}}class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}
129130
}
130131
{{/hasRequired}}
131132
{{/generatedConstructorWithRequiredArgs}}
133+
{{/vendorExtensions.x-java-lombok-duplicates-required-args-constructor}}
132134
{{/lombok.RequiredArgsConstructor}}
133135
{{#vendorExtensions.x-java-all-args-constructor}}
134136

‎modules/openapi-generator/src/main/resources/java-camel-server/pojo.mustache‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ public class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}{{^parent}}
117117
{{/vendorExtensions.x-java-no-args-constructor}}
118118
{{^lombok.Data}}
119119
{{^lombok.RequiredArgsConstructor}}
120+
{{^vendorExtensions.x-java-lombok-duplicates-required-args-constructor}}
120121
{{#generatedConstructorWithRequiredArgs}}
121122
{{#hasRequired}}
122123

@@ -140,6 +141,7 @@ public class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}{{^parent}}
140141
}
141142
{{/hasRequired}}
142143
{{/generatedConstructorWithRequiredArgs}}
144+
{{/vendorExtensions.x-java-lombok-duplicates-required-args-constructor}}
143145
{{/lombok.RequiredArgsConstructor}}
144146
{{#vendorExtensions.x-java-all-args-constructor}}
145147

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5827,6 +5827,78 @@ public void generateAllArgsConstructor() throws IOException {
58275827
.hasParameter("count").toConstructor();
58285828
}
58295829

5830+
@Test
5831+
public void lombokAllArgsConstructorReplacesRequiredArgsConstructor_issue25395() throws IOException {
5832+
Map<String, File> output = generateFromContract(
5833+
"src/test/resources/3_0/spring/issue_25395.yaml",
5834+
SPRING_BOOT,
5835+
Map.of(AbstractJavaCodegen.ADDITIONAL_MODEL_TYPE_ANNOTATIONS, "@lombok.AllArgsConstructor",
5836+
CodegenConstants.OPENAPI_NULLABLE, "false",
5837+
INTERFACE_ONLY, "true")
5838+
);
5839+
5840+
// lombok generates Pet(Long, String), a generated one would be a duplicate
5841+
JavaFileAssert.assertThat(output.get("Pet.java"))
5842+
.assertConstructor().toFileAssert()
5843+
.hasNoConstructor("Long", "String");
5844+
JavaFileAssert.assertThat(output.get("NicknamedPet.java"))
5845+
.assertConstructor().toFileAssert()
5846+
.hasNoConstructor("Long", "String");
5847+
JavaFileAssert.assertThat(output.get("Tag.java"))
5848+
.assertConstructor("Long");
5849+
}
5850+
5851+
@Test
5852+
public void lombokAllArgsConstructorKeepsRequiredArgsConstructorOfNullableProperty_issue25395() throws IOException {
5853+
Map<String, File> output = generateFromContract(
5854+
"src/test/resources/3_0/spring/issue_25395.yaml",
5855+
SPRING_BOOT,
5856+
Map.of(AbstractJavaCodegen.ADDITIONAL_MODEL_TYPE_ANNOTATIONS, "@lombok.AllArgsConstructor",
5857+
CodegenConstants.OPENAPI_NULLABLE, "true",
5858+
INTERFACE_ONLY, "true")
5859+
);
5860+
5861+
// the field is JsonNullable<String> but the generated parameter is String, so lombok's constructor differs
5862+
JavaFileAssert.assertThat(output.get("NicknamedPet.java"))
5863+
.assertConstructor("Long", "String");
5864+
}
5865+
5866+
@Test
5867+
public void lombokAllArgsConstructorWithInheritedProperties_issue25395() throws IOException {
5868+
Map<String, File> output = generateFromContract(
5869+
"src/test/resources/3_0/spring/issue_25395.yaml",
5870+
SPRING_BOOT,
5871+
Map.of(AbstractJavaCodegen.ADDITIONAL_MODEL_TYPE_ANNOTATIONS, "@lombok.AllArgsConstructor",
5872+
CodegenConstants.OPENAPI_NULLABLE, "false",
5873+
INTERFACE_ONLY, "true")
5874+
);
5875+
5876+
// lombok only covers the own field, the generated constructor also takes the inherited one
5877+
JavaFileAssert.assertThat(output.get("Dog.java"))
5878+
.assertConstructor("String", "String")
5879+
.bodyContainsLines("super(kind)", "this.breed = breed");
5880+
// the parent has no required property, so the signature is the same as lombok's
5881+
JavaFileAssert.assertThat(output.get("Hound.java"))
5882+
.assertConstructor().toFileAssert()
5883+
.hasNoConstructor("String");
5884+
}
5885+
5886+
@Test
5887+
public void lombokAllArgsConstructorKeepsRequiredArgsConstructorWithAdditionalProperties_issue25395() throws IOException {
5888+
Map<String, File> output = generateFromContract(
5889+
"src/test/resources/3_0/spring/issue_25395.yaml",
5890+
SPRING_BOOT,
5891+
Map.of(AbstractJavaCodegen.ADDITIONAL_MODEL_TYPE_ANNOTATIONS, "@lombok.AllArgsConstructor",
5892+
CodegenConstants.OPENAPI_NULLABLE, "false",
5893+
INTERFACE_ONLY, "true")
5894+
);
5895+
5896+
// the additionalProperties map is a field too, so lombok's constructor takes it as a third parameter
5897+
JavaFileAssert.assertThat(output.get("Bag.java"))
5898+
.assertConstructor().toFileAssert()
5899+
.assertConstructor("Long", "String");
5900+
}
5901+
58305902
@Test
58315903
public void allOfDuplicatedProperties() throws IOException {
58325904
Map<String, File> output = generateFromContract(

‎modules/openapi-generator/src/test/resources/3_0/spring/issue_25395.yaml‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
openapi: 3.0.3
2+
info:
3+
title: Test Issue #25395
4+
version: 1.0.0
5+
paths:
6+
/pets:
7+
get:
8+
operationId: listPets
9+
responses:
10+
'200':
11+
description: default response
12+
content:
13+
application/json:
14+
schema:
15+
type: array
16+
items:
17+
$ref: '#/components/schemas/Pet'
18+
components:
19+
schemas:
20+
Pet:
21+
type: object
22+
required:
23+
- id
24+
- name
25+
properties:
26+
id:
27+
type: integer
28+
format: int64
29+
name:
30+
type: string
31+
Tag:
32+
type: object
33+
required:
34+
- id
35+
properties:
36+
id:
37+
type: integer
38+
format: int64
39+
label:
40+
type: string
41+
NicknamedPet:
42+
type: object
43+
required:
44+
- id
45+
- nickname
46+
properties:
47+
id:
48+
type: integer
49+
format: int64
50+
nickname:
51+
type: string
52+
nullable: true
53+
Animal:
54+
type: object
55+
required:
56+
- kind
57+
properties:
58+
kind:
59+
type: string
60+
discriminator:
61+
propertyName: kind
62+
Dog:
63+
allOf:
64+
- $ref: '#/components/schemas/Animal'
65+
- type: object
66+
required:
67+
- breed
68+
properties:
69+
breed:
70+
type: string
71+
Base:
72+
type: object
73+
properties:
74+
kind:
75+
type: string
76+
discriminator:
77+
propertyName: kind
78+
Hound:
79+
allOf:
80+
- $ref: '#/components/schemas/Base'
81+
- type: object
82+
required:
83+
- scent
84+
properties:
85+
scent:
86+
type: string
87+
Bag:
88+
type: object
89+
required:
90+
- id
91+
- name
92+
properties:
93+
id:
94+
type: integer
95+
format: int64
96+
name:
97+
type: string
98+
additionalProperties:
99+
type: string

0 commit comments

Comments
 (0)