Skip to content

Commit d3017ff

Browse files
FrankEssenbergerFrankEssenberger
authored
typescript: Fix Union Types Import Issue (#6789)
* First approach for discussion * typo * add addiotional method * polish a bit * remove call of super method * fix javadoc error * com.google.common.collect. * merge master regenerate samples * sort imports alphabetically * sort imports alphabetically with right key * typo * add type previous imports are still there. * add type previous imports are still there. * remove new test to see if they are the problem. * merge master add tests back in * align changes which should not lead to failing test but you never know. * remove formatting changes * dummy change * revert spaces * revert spaces * revert functional changes * comment out test * remove model * remove interface method * remove test class completely * put test class back - test body commented out * rename test methods * put back logic and tests * remove generated APIs * remark amakhrov * check in one generated file to test * adjust call super * add comment use set.
1 parent 71321bd commit d3017ff

6 files changed

Lines changed: 251 additions & 19 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,8 @@ public interface CodegenConfig {
174174

175175
String toModelImport(String name);
176176

177+
Map<String,String> toModelImportMap(String name);
178+
177179
String toApiImport(String name);
178180

179181
void addOperationToGroup(String tag, String resourcePath, Operation operation, CodegenOperation co, Map<String, List<CodegenOperation>> operations);

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.github.benmanes.caffeine.cache.Ticker;
2323
import com.google.common.base.CaseFormat;
2424
import com.google.common.collect.ImmutableMap;
25+
import com.google.common.collect.Maps;
2526
import com.samskivert.mustache.Mustache;
2627
import com.samskivert.mustache.Mustache.Compiler;
2728
import com.samskivert.mustache.Mustache.Lambda;
@@ -1367,6 +1368,16 @@ public String toModelImport(String name) {
13671368
}
13681369
}
13691370

1371+
/**
1372+
* Returns the same content as [[toModelImport]] with key the fully-qualified Model name and value the initial input.
1373+
* In case of union types this method has a key for each separate model and import.
1374+
* @param name the name of the "Model"
1375+
* @return Map of fully-qualified models.
1376+
*/
1377+
public Map<String,String> toModelImportMap(String name){
1378+
return Collections.singletonMap(this.toModelImport(name),name);
1379+
}
1380+
13701381
/**
13711382
* Return the fully-qualified "Api" name for import
13721383
*

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

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import java.nio.file.Path;
5959
import java.time.ZonedDateTime;
6060
import java.util.*;
61+
import java.util.stream.Collectors;
6162

6263
import static org.openapitools.codegen.utils.OnceLogger.once;
6364

@@ -1077,26 +1078,11 @@ private Map<String, Object> processOperations(CodegenConfig config, String tag,
10771078
allImports.addAll(op.imports);
10781079
}
10791080

1080-
List<Map<String, String>> imports = new ArrayList<>();
1081-
Set<String> mappingSet = new TreeSet<>();
1082-
for (String nextImport : allImports) {
1083-
Map<String, String> im = new LinkedHashMap<>();
1084-
String mapping = config.importMapping().get(nextImport);
1085-
if (mapping == null) {
1086-
mapping = config.toModelImport(nextImport);
1087-
}
1081+
Map<String,String> mappings = getAllImportsMappings(allImports);
1082+
Set<Map<String, String>> imports = toImportsObjects(mappings);
10881083

1089-
if (mapping != null && !mappingSet.contains(mapping)) { // ensure import (mapping) is unique
1090-
mappingSet.add(mapping);
1091-
im.put("import", mapping);
1092-
im.put("classname", nextImport);
1093-
if (!imports.contains(im)) { // avoid duplicates
1094-
imports.add(im);
1095-
}
1096-
}
1097-
}
1098-
1099-
operations.put("imports", imports);
1084+
//Some codegen implementations rely on a list interface for the imports
1085+
operations.put("imports", imports.stream().collect(Collectors.toList()));
11001086

11011087
// add a flag to indicate whether there's any {{import}}
11021088
if (imports.size() > 0) {
@@ -1115,6 +1101,49 @@ private Map<String, Object> processOperations(CodegenConfig config, String tag,
11151101
return operations;
11161102
}
11171103

1104+
/**
1105+
* Transforms a set of imports to a map with key config.toModelImport(import) and value the import string.
1106+
* @param allImports - Set of imports
1107+
* @return Map of fully qualified import path and initial import.
1108+
*/
1109+
private Map<String,String> getAllImportsMappings(Set<String> allImports){
1110+
Map<String,String> result = new HashMap<>();
1111+
allImports.forEach(nextImport->{
1112+
String mapping = config.importMapping().get(nextImport);
1113+
if(mapping!= null){
1114+
result.put(mapping,nextImport);
1115+
}else{
1116+
result.putAll(config.toModelImportMap(nextImport));
1117+
}
1118+
});
1119+
return result;
1120+
}
1121+
1122+
/**
1123+
* Using an import map created via {@link #getAllImportsMappings(Set)} to build a list import objects.
1124+
* The import objects have two keys: import and classname which hold the key and value of the initial map entry.
1125+
*
1126+
* @param mappedImports Map of fully qualified import and import
1127+
* @return The set of unique imports
1128+
*/
1129+
private Set<Map<String,String>> toImportsObjects(Map<String,String> mappedImports){
1130+
Set<Map<String, String>> result = new TreeSet<Map<String,String>>(
1131+
(Comparator<Map<String, String>>) (o1, o2) -> {
1132+
String s1 = o1.get("classname");
1133+
String s2 = o2.get("classname");
1134+
return s1.compareTo(s2);
1135+
}
1136+
);
1137+
1138+
mappedImports.entrySet().forEach(mapping->{
1139+
Map<String, String> im = new LinkedHashMap<>();
1140+
im.put("import", mapping.getKey());
1141+
im.put("classname", mapping.getValue());
1142+
result.add(im);
1143+
});
1144+
return result;
1145+
}
1146+
11181147
private Map<String, Object> processModels(CodegenConfig config, Map<String, Schema> definitions) {
11191148
Map<String, Object> objs = new HashMap<>();
11201149
objs.put("package", config.modelPackage());

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
package org.openapitools.codegen.languages;
1919

20+
import com.google.common.collect.Lists;
21+
import com.google.common.collect.Maps;
2022
import io.swagger.v3.oas.models.OpenAPI;
2123
import io.swagger.v3.oas.models.media.ArraySchema;
2224
import io.swagger.v3.oas.models.media.ComposedSchema;
@@ -35,6 +37,7 @@
3537
import java.io.File;
3638
import java.text.SimpleDateFormat;
3739
import java.util.*;
40+
import java.util.function.Function;
3841
import java.util.stream.Collectors;
3942
import java.util.stream.Stream;
4043

@@ -224,6 +227,47 @@ public void processOpts() {
224227
}
225228
}
226229

230+
@Override
231+
public String toModelImport( String name){
232+
if(isUnionType(name)){
233+
LOGGER.warn("The import is a union type. Consider using the toModelImportMap method.");
234+
return toModelImportMap(name).values().stream().collect(Collectors.joining("|"));
235+
}
236+
return super.toModelImport(name);
237+
}
238+
239+
/**
240+
* Maps the fully qualified model import to the initial given name. In case of union types the map will have multiple entries.
241+
* For example for "classA | classB" the map will two entries have ["model.classA","classA"] and ["model.classB","classB"].
242+
*
243+
* @param name the name of the "Model"
244+
* @return Map between the fully qualified model import and the initial given name.
245+
*/
246+
@Override
247+
public Map<String,String> toModelImportMap( String name){
248+
if(isUnionType(name)){
249+
String[] names = splitUnionType(name);
250+
return toImportMap(names);
251+
}
252+
return toImportMap(name);
253+
}
254+
255+
private boolean isUnionType(String name){
256+
return name.contains("|");
257+
}
258+
259+
private String[] splitUnionType(String name){
260+
return name.replace(" ","").split("\\|");
261+
}
262+
263+
private Map<String,String> toImportMap(String... names){
264+
Map<String,String> result = Maps.newHashMap();
265+
for(String name: names){
266+
result.put(toModelImport(name),name);
267+
}
268+
return result;
269+
}
270+
227271
@Override
228272
public void preprocessOpenAPI(OpenAPI openAPI) {
229273

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/typescript/SharedTypeScriptTest.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package org.openapitools.codegen.typescript;
2+
3+
import org.apache.commons.io.FileUtils;
4+
import org.apache.commons.lang3.StringUtils;
5+
import org.openapitools.codegen.DefaultGenerator;
6+
import org.openapitools.codegen.Generator;
7+
import org.openapitools.codegen.config.CodegenConfigurator;
8+
import org.openapitools.codegen.languages.TypeScriptAxiosClientCodegen;
9+
import org.testng.Assert;
10+
import org.testng.annotations.Test;
11+
12+
import java.io.File;
13+
import java.io.IOException;
14+
import java.util.List;
15+
16+
public class SharedTypeScriptTest {
17+
@Test
18+
public void typesInImportsAreSplittedTest() throws IOException {
19+
CodegenConfigurator config =
20+
new CodegenConfigurator()
21+
.setInputSpec("src/test/resources/split-import.json")
22+
.setModelPackage("model")
23+
.setApiPackage("api")
24+
.setOutputDir("src/test/resources/typesInImportsAreSplittedTest")
25+
.addAdditionalProperty(
26+
TypeScriptAxiosClientCodegen.SEPARATE_MODELS_AND_API, true);
27+
28+
config.setGeneratorName("typescript-axios");
29+
checkAPIFile(getGenerator(config).generate(), "default-api.ts");
30+
31+
config.setGeneratorName("typescript-node");
32+
checkAPIFile(getGenerator(config).generate(), "defaultApi.ts");
33+
34+
config.setGeneratorName("typescript-angular");
35+
checkAPIFile(getGenerator(config).generate(), "default.service.ts");
36+
37+
FileUtils.deleteDirectory(new File("src/test/resources/typesInImportsAreSplittedTest"));
38+
}
39+
40+
private Generator getGenerator(CodegenConfigurator config) {
41+
return new DefaultGenerator().opts(config.toClientOptInput());
42+
}
43+
44+
private void checkAPIFile(List<File> files, String apiFileName) throws IOException {
45+
File apiFile = files.stream().filter(file->file.getName().contains(apiFileName)).findFirst().get();
46+
String apiFileContent = FileUtils.readFileToString(apiFile);
47+
Assert.assertTrue(!apiFileContent.contains("import { OrganizationWrapper | PersonWrapper }"));
48+
Assert.assertEquals(StringUtils.countMatches(apiFileContent,"import { PersonWrapper }"),1);
49+
Assert.assertEquals(StringUtils.countMatches(apiFileContent,"import { OrganizationWrapper }"),1);
50+
}
51+
52+
@Test
53+
public void oldImportsStillPresentTest() throws IOException {
54+
CodegenConfigurator config =
55+
new CodegenConfigurator()
56+
.setInputSpec("petstore.json")
57+
.setModelPackage("model")
58+
.setApiPackage("api")
59+
.setOutputDir("src/test/resources/oldImportsStillPresentTest/")
60+
.addAdditionalProperty(
61+
TypeScriptAxiosClientCodegen.SEPARATE_MODELS_AND_API, true);
62+
63+
config.setGeneratorName("typescript-angular");
64+
final List<File> files = getGenerator(config).generate();
65+
File pets = files.stream().filter(file->file.getName().contains("pet.ts")).findFirst().get();
66+
String apiFileContent = FileUtils.readFileToString(pets);
67+
Assert.assertTrue(apiFileContent.contains("import { Category }"));
68+
Assert.assertTrue(apiFileContent.contains("import { Tag }"));
69+
70+
FileUtils.deleteDirectory(new File("src/test/resources/oldImportsStillPresentTest/"));
71+
}
72+
}

‎modules/openapi-generator/src/test/resources/split-import.json‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
{
2+
"openapi": "3.0.2",
3+
"info": {
4+
"title": "SAP Graph - Customers API",
5+
"version": "0.1.0"
6+
},
7+
"paths": {
8+
"/Customer": {
9+
"get": {
10+
"operationId": "getCustomer",
11+
"responses": {
12+
"200": {
13+
"$ref": "#/components/responses/CustomerResponse"
14+
}
15+
}
16+
}
17+
},
18+
"/Person": {
19+
"get": {
20+
"operationId": "getPerson",
21+
"responses": {
22+
"200": {
23+
"$ref": "#/components/responses/PersonResponse"
24+
}
25+
}
26+
}
27+
}
28+
},
29+
"components": {
30+
"schemas": {
31+
"OrganizationWrapper": {
32+
"type": "object",
33+
"properties": {
34+
"id": {"type": "string"}
35+
}
36+
},
37+
"PersonWrapper": {
38+
"type": "object",
39+
"properties": {
40+
"id": {"type": "string"}
41+
}
42+
}
43+
},
44+
"responses": {
45+
"CustomerResponse": {
46+
"description": "Get Customer",
47+
"content": {
48+
"application/json": {
49+
"schema": {
50+
"oneOf": [
51+
{
52+
"$ref": "#/components/schemas/OrganizationWrapper"
53+
},
54+
{
55+
"$ref": "#/components/schemas/PersonWrapper"
56+
}
57+
]
58+
}
59+
}
60+
}
61+
},
62+
"PersonResponse": {
63+
"description": "Get Person",
64+
"content": {
65+
"application/json": {
66+
"schema": {
67+
"$ref": "#/components/schemas/PersonWrapper"
68+
}
69+
}
70+
}
71+
}
72+
}
73+
}
74+
}

0 commit comments

Comments
 (0)