Skip to content

Commit

Permalink
typescript: Fix Union Types Import Issue (#6789)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
FrankEssenberger committed Aug 20, 2020
1 parent 71321bd commit d3017ff
Show file tree
Hide file tree
Showing 6 changed files with 251 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ public interface CodegenConfig {

String toModelImport(String name);

Map<String,String> toModelImportMap(String name);

String toApiImport(String name);

void addOperationToGroup(String tag, String resourcePath, Operation operation, CodegenOperation co, Map<String, List<CodegenOperation>> operations);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.github.benmanes.caffeine.cache.Ticker;
import com.google.common.base.CaseFormat;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.samskivert.mustache.Mustache;
import com.samskivert.mustache.Mustache.Compiler;
import com.samskivert.mustache.Mustache.Lambda;
Expand Down Expand Up @@ -1367,6 +1368,16 @@ public String toModelImport(String name) {
}
}

/**
* Returns the same content as [[toModelImport]] with key the fully-qualified Model name and value the initial input.
* In case of union types this method has a key for each separate model and import.
* @param name the name of the "Model"
* @return Map of fully-qualified models.
*/
public Map<String,String> toModelImportMap(String name){
return Collections.singletonMap(this.toModelImport(name),name);
}

/**
* Return the fully-qualified "Api" name for import
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.nio.file.Path;
import java.time.ZonedDateTime;
import java.util.*;
import java.util.stream.Collectors;

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

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

List<Map<String, String>> imports = new ArrayList<>();
Set<String> mappingSet = new TreeSet<>();
for (String nextImport : allImports) {
Map<String, String> im = new LinkedHashMap<>();
String mapping = config.importMapping().get(nextImport);
if (mapping == null) {
mapping = config.toModelImport(nextImport);
}
Map<String,String> mappings = getAllImportsMappings(allImports);
Set<Map<String, String>> imports = toImportsObjects(mappings);

if (mapping != null && !mappingSet.contains(mapping)) { // ensure import (mapping) is unique
mappingSet.add(mapping);
im.put("import", mapping);
im.put("classname", nextImport);
if (!imports.contains(im)) { // avoid duplicates
imports.add(im);
}
}
}

operations.put("imports", imports);
//Some codegen implementations rely on a list interface for the imports
operations.put("imports", imports.stream().collect(Collectors.toList()));

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

/**
* Transforms a set of imports to a map with key config.toModelImport(import) and value the import string.
* @param allImports - Set of imports
* @return Map of fully qualified import path and initial import.
*/
private Map<String,String> getAllImportsMappings(Set<String> allImports){
Map<String,String> result = new HashMap<>();
allImports.forEach(nextImport->{
String mapping = config.importMapping().get(nextImport);
if(mapping!= null){
result.put(mapping,nextImport);
}else{
result.putAll(config.toModelImportMap(nextImport));
}
});
return result;
}

/**
* Using an import map created via {@link #getAllImportsMappings(Set)} to build a list import objects.
* The import objects have two keys: import and classname which hold the key and value of the initial map entry.
*
* @param mappedImports Map of fully qualified import and import
* @return The set of unique imports
*/
private Set<Map<String,String>> toImportsObjects(Map<String,String> mappedImports){
Set<Map<String, String>> result = new TreeSet<Map<String,String>>(
(Comparator<Map<String, String>>) (o1, o2) -> {
String s1 = o1.get("classname");
String s2 = o2.get("classname");
return s1.compareTo(s2);
}
);

mappedImports.entrySet().forEach(mapping->{
Map<String, String> im = new LinkedHashMap<>();
im.put("import", mapping.getKey());
im.put("classname", mapping.getValue());
result.add(im);
});
return result;
}

private Map<String, Object> processModels(CodegenConfig config, Map<String, Schema> definitions) {
Map<String, Object> objs = new HashMap<>();
objs.put("package", config.modelPackage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

package org.openapitools.codegen.languages;

import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import io.swagger.v3.oas.models.OpenAPI;
import io.swagger.v3.oas.models.media.ArraySchema;
import io.swagger.v3.oas.models.media.ComposedSchema;
Expand All @@ -35,6 +37,7 @@
import java.io.File;
import java.text.SimpleDateFormat;
import java.util.*;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -224,6 +227,47 @@ public void processOpts() {
}
}

@Override
public String toModelImport( String name){
if(isUnionType(name)){
LOGGER.warn("The import is a union type. Consider using the toModelImportMap method.");
return toModelImportMap(name).values().stream().collect(Collectors.joining("|"));
}
return super.toModelImport(name);
}

/**
* Maps the fully qualified model import to the initial given name. In case of union types the map will have multiple entries.
* For example for "classA | classB" the map will two entries have ["model.classA","classA"] and ["model.classB","classB"].
*
* @param name the name of the "Model"
* @return Map between the fully qualified model import and the initial given name.
*/
@Override
public Map<String,String> toModelImportMap( String name){
if(isUnionType(name)){
String[] names = splitUnionType(name);
return toImportMap(names);
}
return toImportMap(name);
}

private boolean isUnionType(String name){
return name.contains("|");
}

private String[] splitUnionType(String name){
return name.replace(" ","").split("\\|");
}

private Map<String,String> toImportMap(String... names){
Map<String,String> result = Maps.newHashMap();
for(String name: names){
result.put(toModelImport(name),name);
}
return result;
}

@Override
public void preprocessOpenAPI(OpenAPI openAPI) {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package org.openapitools.codegen.typescript;

import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.StringUtils;
import org.openapitools.codegen.DefaultGenerator;
import org.openapitools.codegen.Generator;
import org.openapitools.codegen.config.CodegenConfigurator;
import org.openapitools.codegen.languages.TypeScriptAxiosClientCodegen;
import org.testng.Assert;
import org.testng.annotations.Test;

import java.io.File;
import java.io.IOException;
import java.util.List;

public class SharedTypeScriptTest {
@Test
public void typesInImportsAreSplittedTest() throws IOException {
CodegenConfigurator config =
new CodegenConfigurator()
.setInputSpec("src/test/resources/split-import.json")
.setModelPackage("model")
.setApiPackage("api")
.setOutputDir("src/test/resources/typesInImportsAreSplittedTest")
.addAdditionalProperty(
TypeScriptAxiosClientCodegen.SEPARATE_MODELS_AND_API, true);

config.setGeneratorName("typescript-axios");
checkAPIFile(getGenerator(config).generate(), "default-api.ts");

config.setGeneratorName("typescript-node");
checkAPIFile(getGenerator(config).generate(), "defaultApi.ts");

config.setGeneratorName("typescript-angular");
checkAPIFile(getGenerator(config).generate(), "default.service.ts");

FileUtils.deleteDirectory(new File("src/test/resources/typesInImportsAreSplittedTest"));
}

private Generator getGenerator(CodegenConfigurator config) {
return new DefaultGenerator().opts(config.toClientOptInput());
}

private void checkAPIFile(List<File> files, String apiFileName) throws IOException {
File apiFile = files.stream().filter(file->file.getName().contains(apiFileName)).findFirst().get();
String apiFileContent = FileUtils.readFileToString(apiFile);
Assert.assertTrue(!apiFileContent.contains("import { OrganizationWrapper | PersonWrapper }"));
Assert.assertEquals(StringUtils.countMatches(apiFileContent,"import { PersonWrapper }"),1);
Assert.assertEquals(StringUtils.countMatches(apiFileContent,"import { OrganizationWrapper }"),1);
}

@Test
public void oldImportsStillPresentTest() throws IOException {
CodegenConfigurator config =
new CodegenConfigurator()
.setInputSpec("petstore.json")
.setModelPackage("model")
.setApiPackage("api")
.setOutputDir("src/test/resources/oldImportsStillPresentTest/")
.addAdditionalProperty(
TypeScriptAxiosClientCodegen.SEPARATE_MODELS_AND_API, true);

config.setGeneratorName("typescript-angular");
final List<File> files = getGenerator(config).generate();
File pets = files.stream().filter(file->file.getName().contains("pet.ts")).findFirst().get();
String apiFileContent = FileUtils.readFileToString(pets);
Assert.assertTrue(apiFileContent.contains("import { Category }"));
Assert.assertTrue(apiFileContent.contains("import { Tag }"));

FileUtils.deleteDirectory(new File("src/test/resources/oldImportsStillPresentTest/"));
}
}
74 changes: 74 additions & 0 deletions modules/openapi-generator/src/test/resources/split-import.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
{
"openapi": "3.0.2",
"info": {
"title": "SAP Graph - Customers API",
"version": "0.1.0"
},
"paths": {
"/Customer": {
"get": {
"operationId": "getCustomer",
"responses": {
"200": {
"$ref": "#/components/responses/CustomerResponse"
}
}
}
},
"/Person": {
"get": {
"operationId": "getPerson",
"responses": {
"200": {
"$ref": "#/components/responses/PersonResponse"
}
}
}
}
},
"components": {
"schemas": {
"OrganizationWrapper": {
"type": "object",
"properties": {
"id": {"type": "string"}
}
},
"PersonWrapper": {
"type": "object",
"properties": {
"id": {"type": "string"}
}
}
},
"responses": {
"CustomerResponse": {
"description": "Get Customer",
"content": {
"application/json": {
"schema": {
"oneOf": [
{
"$ref": "#/components/schemas/OrganizationWrapper"
},
{
"$ref": "#/components/schemas/PersonWrapper"
}
]
}
}
}
},
"PersonResponse": {
"description": "Get Person",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/PersonWrapper"
}
}
}
}
}
}
}

0 comments on commit d3017ff

Please sign in to comment.