Skip to content

Commit 8e382a3

Browse files
Sekator778Sekator778
authored
[Java][Spring] useRecords: keep a model that a default constructs as a class (#25252)
1 parent 4cb4be2 commit 8e382a3

4 files changed

Lines changed: 327 additions & 2 deletions

File tree

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

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import java.net.URL;
6363
import java.util.*;
6464
import java.util.regex.Matcher;
65+
import java.util.regex.Pattern;
6566
import java.util.stream.Collectors;
6667

6768
import static org.apache.commons.lang3.StringUtils.isNotEmpty;
@@ -1860,6 +1861,9 @@ private void validateUseRecords() {
18601861
private static final Set<String> RECORD_FORBIDDEN_COMPONENT_NAMES = new HashSet<>(Arrays.asList(
18611862
"clone", "finalize", "getClass", "hashCode", "notify", "notifyAll", "toString", "wait"));
18621863

1864+
/** A constructor call in a default value, as {@code toDefaultValue} writes it: {@code new Sub()}, {@code new Sub().x("a")}. */
1865+
private static final Pattern RECORD_DEFAULT_CONSTRUCTOR_CALL = Pattern.compile("(?<![\\w.])new (\\w+)\\(");
1866+
18631867
/**
18641868
* Decides for every model whether it is generated as a record and stores the decision in the vendor
18651869
* extension {@code x-is-record}. A model that stays a class is logged with the reason.
@@ -1873,14 +1877,15 @@ private void markRecordModels(Map<String, CodegenModel> allModels) {
18731877
}
18741878

18751879
Set<String> parameterSchemas = schemasBoundFromParameters();
1880+
Map<String, String> constructedByDefault = modelsConstructedByDefault(allModels);
18761881

18771882
for (CodegenModel cm : allModels.values()) {
18781883
// enums and oneOf interfaces are untouched; aliases (a schema that is only a $ref or a primitive) are
18791884
// never written as a model file (DefaultGenerator skips CodegenModel.isAlias), so they get no decision
18801885
if (cm.isEnum || cm.isAlias || Boolean.TRUE.equals(cm.vendorExtensions.get(X_IS_ONE_OF_INTERFACE))) {
18811886
continue;
18821887
}
1883-
String reason = reasonForClass(cm, oneOfMembers, parameterSchemas);
1888+
String reason = reasonForClass(cm, oneOfMembers, parameterSchemas, constructedByDefault);
18841889
if (reason != null) {
18851890
LOGGER.info("{}: model '{}' is generated as a class: {}", USE_RECORDS, cm.classname, reason);
18861891
continue;
@@ -1899,6 +1904,54 @@ private void markRecordModels(Map<String, CodegenModel> allModels) {
18991904
}
19001905
}
19011906

1907+
/**
1908+
* Classnames of the models that the default value of a model-typed property constructs, each with the first
1909+
* property that does it. A record has only its canonical constructor, so {@code new Sub()} and the fluent
1910+
* {@code new Sub().x("a")} that {@code toDefaultValue} writes do not compile for a record; such a model stays a class.
1911+
* Only the default of a property whose type, or whose innermost items type, is a model is read, never the text
1912+
* of a string default. A model without properties is left out: its record has a no-argument constructor, and so
1913+
* is a model that gets no file.
1914+
*/
1915+
private Map<String, String> modelsConstructedByDefault(Map<String, CodegenModel> allModels) {
1916+
Set<String> modelNames = new HashSet<>();
1917+
Set<String> modelsWithProperties = new HashSet<>();
1918+
for (CodegenModel cm : allModels.values()) {
1919+
modelNames.add(cm.classname);
1920+
if (!cm.vars.isEmpty() && !hasNoModelFile(cm)) {
1921+
modelsWithProperties.add(cm.classname);
1922+
}
1923+
}
1924+
Map<String, String> constructed = new HashMap<>();
1925+
for (CodegenModel cm : new TreeMap<>(allModels).values()) {
1926+
for (CodegenProperty cp : cm.vars) {
1927+
CodegenProperty inner = cp.mostInnerItems != null ? cp.mostInnerItems : cp;
1928+
// complexType is null when the model is named like a Java type, as a model named Integer is
1929+
String type = inner.complexType != null ? inner.complexType : inner.isModel ? inner.dataType : null;
1930+
// the initial value that the templates print, which is not the default for a nullable property
1931+
String initialValue = recordInitialValue(cp);
1932+
if (type == null || !modelNames.contains(type) || initialValue == null) {
1933+
continue;
1934+
}
1935+
Matcher matcher = RECORD_DEFAULT_CONSTRUCTOR_CALL.matcher(initialValue);
1936+
while (matcher.find()) {
1937+
if (modelsWithProperties.contains(matcher.group(1))) {
1938+
constructed.putIfAbsent(matcher.group(1), "property '" + cp.name + "' in model '" + cm.classname + "'");
1939+
}
1940+
}
1941+
}
1942+
}
1943+
return constructed;
1944+
}
1945+
1946+
/**
1947+
* Whether {@code DefaultGenerator} writes no file for this model: its name is type-mapped to a type that has an
1948+
* import mapping, as {@code URI} and {@code UUID} are (the property then has the JDK class as its type).
1949+
*/
1950+
private boolean hasNoModelFile(CodegenModel cm) {
1951+
String mappedType = typeMapping.get(cm.schemaName);
1952+
return mappedType != null && importMapping.containsKey(mappedType);
1953+
}
1954+
19021955
/**
19031956
* Names of the schemas that a request binds from parameters (query, header, cookie, path), not from a body:
19041957
* every schema a non-body parameter refers to, directly, in {@code content}, through {@code allOf}, array
@@ -1975,7 +2028,8 @@ private void collectReferencedSchemas(Schema<?> schema, Set<String> names) {
19752028
}
19762029
}
19772030

1978-
private String reasonForClass(CodegenModel cm, Set<String> oneOfMembers, Set<String> parameterSchemas) {
2031+
private String reasonForClass(CodegenModel cm, Set<String> oneOfMembers, Set<String> parameterSchemas,
2032+
Map<String, String> constructedByDefault) {
19792033
if (cm.parent != null) {
19802034
return "it has a parent (" + cm.parent + ")";
19812035
}
@@ -2019,6 +2073,9 @@ private String reasonForClass(CodegenModel cm, Set<String> oneOfMembers, Set<Str
20192073
return "property '" + cp.name + "' uses the FAIL null policy";
20202074
}
20212075
}
2076+
if (constructedByDefault.containsKey(cm.classname)) {
2077+
return "the default of " + constructedByDefault.get(cm.classname) + " constructs it with new " + cm.classname + "()";
2078+
}
20222079
return null;
20232080
}
20242081

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

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10936,4 +10936,138 @@ public void javaCamelStillGeneratesClasses() throws IOException {
1093610936

1093710937
JavaFileAssert.assertThat(files.get("Pet.java")).isNormalClass().fileDoesNotContain("record");
1093810938
}
10939+
10940+
private static final String RECORDS_MODEL_DEFAULTS_SPEC = "src/test/resources/3_0/spring/records-model-defaults.yaml";
10941+
10942+
@Test
10943+
public void useRecordsKeepsModelConstructedByDefaultAsClass() throws IOException {
10944+
Map<String, File> files = generateRecords(RECORDS_MODEL_DEFAULTS_SPEC, Map.of());
10945+
10946+
assertSource(files, "Sub.java").contains("public class Sub {").doesNotContain("public record Sub(");
10947+
assertSource(files, "Holder.java").contains("public record Holder(", " inner = new Sub();");
10948+
}
10949+
10950+
@Test
10951+
public void useRecordsLogsWhyModelConstructedByDefaultIsAClass() {
10952+
List<String> messages = captureLogMessages(SpringCodegen.class, () -> {
10953+
try {
10954+
generateRecords(RECORDS_MODEL_DEFAULTS_SPEC, Map.of());
10955+
} catch (IOException e) {
10956+
throw new RuntimeException(e);
10957+
}
10958+
});
10959+
10960+
assertThat(messages).contains(
10961+
"useRecords: model 'Sub' is generated as a class: the default of property 'inner' in model 'Holder' constructs it with new Sub()");
10962+
}
10963+
10964+
@Test
10965+
public void useRecordsKeepsModelWithNonEmptyDefaultAsClass() throws IOException {
10966+
Map<String, File> files = generateRecords(RECORDS_MODEL_DEFAULTS_SPEC, Map.of());
10967+
10968+
assertSource(files, "Named.java").contains("public class Named {");
10969+
assertSource(files, "Holder.java").contains(" named = new Named().x(\"a\");");
10970+
}
10971+
10972+
@Test
10973+
public void useRecordsKeepsEveryModelOfNestedDefaultAsClass() throws IOException {
10974+
Map<String, File> files = generateRecords(RECORDS_MODEL_DEFAULTS_SPEC, Map.of());
10975+
10976+
assertSource(files, "Outer.java").contains("public class Outer {");
10977+
assertSource(files, "Leaf.java").contains("public class Leaf {");
10978+
assertSource(files, "Holder.java").contains("new Outer().inner(new Leaf().v(\"r\"))");
10979+
}
10980+
10981+
@Test
10982+
public void useRecordsKeepsModelInListAndSetDefaultAsClass() throws IOException {
10983+
Map<String, File> files = generateRecords(RECORDS_MODEL_DEFAULTS_SPEC, Map.of());
10984+
10985+
assertSource(files, "ListItem.java").contains("public class ListItem {");
10986+
assertSource(files, "SetItem.java").contains("public class SetItem {");
10987+
assertSource(files, "Holder.java").contains(
10988+
" items = new ArrayList<>(Arrays.asList(new ListItem()));",
10989+
" uniqueItems = new LinkedHashSet<>(Arrays.asList(new SetItem()));");
10990+
}
10991+
10992+
@Test
10993+
public void useRecordsKeepsModelWithoutPropertiesAsRecord() throws IOException {
10994+
Map<String, File> files = generateRecords(RECORDS_MODEL_DEFAULTS_SPEC, Map.of());
10995+
10996+
assertSource(files, "Hollow.java").contains("public record Hollow(");
10997+
assertSource(files, "Holder.java").contains(" hollow = new Hollow();");
10998+
}
10999+
11000+
@Test
11001+
public void useRecordsKeepsOriginalReasonOfModelThatAlreadyStaysClass() {
11002+
List<String> messages = captureLogMessages(SpringCodegen.class, () -> {
11003+
try {
11004+
generateRecords(RECORDS_MODEL_DEFAULTS_SPEC, Map.of());
11005+
} catch (IOException e) {
11006+
throw new RuntimeException(e);
11007+
}
11008+
});
11009+
11010+
assertThat(messages).contains("useRecords: model 'LooseSub' is generated as a class: it has additionalProperties");
11011+
assertThat(messages).noneMatch(m -> m.contains("model 'LooseSub'") && m.contains("constructs it"));
11012+
}
11013+
11014+
@Test
11015+
public void useRecordsIgnoresStringDefaultThatLooksLikeConstructor() throws IOException {
11016+
Map<String, File> files = generateRecords(RECORDS_MODEL_DEFAULTS_SPEC, Map.of());
11017+
11018+
assertSource(files, "Untouched.java").contains("public record Untouched(");
11019+
assertSource(files, "Holder.java").contains(" text = \"new Untouched()\";");
11020+
}
11021+
11022+
@Test
11023+
public void useRecordsOffKeepsModelDefaultsAsClasses() throws IOException {
11024+
Map<String, File> files = generateFromContract(RECORDS_MODEL_DEFAULTS_SPEC, SPRING_BOOT);
11025+
11026+
JavaFileAssert.assertThat(files.get("Holder.java"))
11027+
.isNormalClass()
11028+
.fileContains("private Sub inner = new Sub();", "private Named named = new Named().x(\"a\");");
11029+
JavaFileAssert.assertThat(files.get("Untouched.java")).isNormalClass();
11030+
}
11031+
11032+
@Test
11033+
public void useRecordsKeepsNullableModelWithDefaultAsRecord() throws IOException {
11034+
List<String> messages = captureLogMessages(SpringCodegen.class, () -> {
11035+
try {
11036+
Map<String, File> files = generateRecords(RECORDS_MODEL_DEFAULTS_SPEC, Map.of());
11037+
assertSource(files, "Quiet.java").contains("public record Quiet(");
11038+
assertSource(files, "QuietItem.java").contains("public record QuietItem(");
11039+
assertSource(files, "Holder.java").contains(
11040+
" quiet = JsonNullable.<Quiet>undefined();",
11041+
" quietItems = JsonNullable.<List<@Valid QuietItem>>undefined();");
11042+
} catch (IOException e) {
11043+
throw new RuntimeException(e);
11044+
}
11045+
});
11046+
11047+
assertThat(messages).noneMatch(m -> m.contains("model 'Quiet'") || m.contains("model 'QuietItem'"));
11048+
}
11049+
11050+
@Test
11051+
public void useRecordsKeepsModelInNullableContainerDefaultAsClassWithoutOpenApiNullable() throws IOException {
11052+
Map<String, File> files = generateRecords(RECORDS_MODEL_DEFAULTS_SPEC, Map.of(CodegenConstants.OPENAPI_NULLABLE, "false"));
11053+
11054+
// a nullable property that is not a container is initialised with null, a nullable container with its default
11055+
assertSource(files, "Quiet.java").contains("public record Quiet(");
11056+
assertSource(files, "QuietItem.java").contains("public class QuietItem {");
11057+
assertSource(files, "Holder.java").contains(" quietItems = new ArrayList<>(Arrays.asList(new QuietItem()));");
11058+
}
11059+
11060+
@Test
11061+
public void useRecordsLogsNoModelThatHasNoFile() throws IOException {
11062+
List<String> messages = captureLogMessages(SpringCodegen.class, () -> {
11063+
try {
11064+
Map<String, File> files = generateRecords("src/test/resources/3_0/spring/records-model-defaults-jdk-names.yaml", Map.of());
11065+
assertThat(files).doesNotContainKey("URI.java");
11066+
} catch (IOException e) {
11067+
throw new RuntimeException(e);
11068+
}
11069+
});
11070+
11071+
assertThat(messages).noneMatch(m -> m.contains("model 'URI'"));
11072+
}
1093911073
}

‎modules/openapi-generator/src/test/resources/3_0/spring/records-model-defaults-jdk-names.yaml‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
openapi: 3.0.3
2+
info:
3+
title: Schemas named like JDK types
4+
version: "1"
5+
paths: {}
6+
components:
7+
schemas:
8+
URI:
9+
type: object
10+
properties:
11+
x:
12+
type: string
13+
Holder:
14+
type: object
15+
properties:
16+
inner:
17+
allOf:
18+
- $ref: '#/components/schemas/URI'
19+
default: {}

‎modules/openapi-generator/src/test/resources/3_0/spring/records-model-defaults.yaml‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
openapi: 3.0.3
2+
info:
3+
title: Models constructed by a default
4+
version: "1"
5+
paths: {}
6+
components:
7+
schemas:
8+
Sub:
9+
type: object
10+
properties:
11+
x:
12+
type: string
13+
n:
14+
type: integer
15+
default: 3
16+
Named:
17+
type: object
18+
properties:
19+
x:
20+
type: string
21+
Outer:
22+
type: object
23+
properties:
24+
inner:
25+
$ref: '#/components/schemas/Leaf'
26+
Leaf:
27+
type: object
28+
properties:
29+
v:
30+
type: string
31+
ListItem:
32+
type: object
33+
properties:
34+
x:
35+
type: string
36+
SetItem:
37+
type: object
38+
properties:
39+
x:
40+
type: string
41+
Hollow:
42+
allOf:
43+
- type: object
44+
LooseSub:
45+
type: object
46+
additionalProperties: true
47+
properties:
48+
x:
49+
type: string
50+
Quiet:
51+
type: object
52+
properties:
53+
x:
54+
type: string
55+
QuietItem:
56+
type: object
57+
properties:
58+
x:
59+
type: string
60+
Untouched:
61+
type: object
62+
properties:
63+
x:
64+
type: string
65+
Holder:
66+
type: object
67+
properties:
68+
inner:
69+
allOf:
70+
- $ref: '#/components/schemas/Sub'
71+
default: {}
72+
named:
73+
allOf:
74+
- $ref: '#/components/schemas/Named'
75+
default:
76+
x: a
77+
chain:
78+
allOf:
79+
- $ref: '#/components/schemas/Outer'
80+
default:
81+
inner:
82+
v: r
83+
items:
84+
type: array
85+
items:
86+
$ref: '#/components/schemas/ListItem'
87+
default: [{}]
88+
uniqueItems:
89+
type: array
90+
uniqueItems: true
91+
items:
92+
$ref: '#/components/schemas/SetItem'
93+
default: [{}]
94+
hollow:
95+
allOf:
96+
- $ref: '#/components/schemas/Hollow'
97+
default: {}
98+
loose:
99+
allOf:
100+
- $ref: '#/components/schemas/LooseSub'
101+
default: {}
102+
text:
103+
type: string
104+
default: new Untouched()
105+
quiet:
106+
allOf:
107+
- $ref: '#/components/schemas/Quiet'
108+
nullable: true
109+
default: {}
110+
quietItems:
111+
type: array
112+
nullable: true
113+
items:
114+
$ref: '#/components/schemas/QuietItem'
115+
default: [{}]

0 commit comments

Comments
 (0)