Skip to content
GitLab
    • Explore Projects Groups Snippets
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • O openapi-generator
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 3,476
    • Issues 3,476
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 402
    • Merge requests 402
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • OpenAPI Tools
  • openapi-generator
  • Merge requests
  • !9834

Opt-in solution to remove unnecessary model for inline schema composition

  • Review changes

  • Download
  • Email patches
  • Plain diff
Open Administrator requested to merge github/fork/leo-sale/fix-unnecessary-models into master 4 years ago
  • Overview 0
  • Commits 1
  • Pipelines 1
  • Changes 33

Created by: leo-sale

fix #3100 fix #5171

Description

When there is a ComposedSchema with inline schemas, the children are flatten. What is does is it's creating a new schema, by taking the parent's name and adding _allOf, _anyOf or _oneOf, or taking the title value and appending all the properties defined in the inline schema. Then the inline schema becomes just a reference to the newly created schema which is added to the imports of the parent. (example in this issue)

This is alright when we have a discriminator in our inline schema and what we wish for is inheritance but when all we want is composition it's just creating unused schemas.

A possible quick fix to prevent these unnecessary schemas is to ignore them in the .openapi-generator-ignore but it does not remove the schemas from the allModels instance meaning that you can have reference to them in your generated files (like some imports or exports, hence possibly breaking the build).

Solution

The proposed solution is to generate the new models only when it's for inheritance and just adding the properties of the inline schema to the parent (overriding the parent's properties when there is a name's conflict) without generating a new model. This solution comes with an opt-in CLI parameter : --allow-inline-schemas, to ease the merge of this fix.

java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate \
   -i https://raw.githubusercontent.com/openapitools/openapi-generator/master/modules/openapi-generator/src/test/resources/3_0/petstore.yaml \
   -g php \
   -o /var/tmp/php_api_client
   --allow-inline-schemas
Compare
  • version 1
    8cf451af
    2 years ago

  • master (base)

and
  • latest version
    8cf451af
    1 commit, 2 years ago

  • version 1
    8cf451af
    1 commit, 2 years ago

33 files
+ 285
- 32

    Preferences

    File browser
    Compare changes
mod‎ules‎
openapi-gene‎rator-cli/src‎
main/java/org/opena‎pitools/codegen/cmd‎
Genera‎te.java‎ +7 -0
test/java/org/opena‎pitools/codegen/cmd‎
Generate‎Test.java‎ +6 -0
openapi-generator-‎core/…/…/…/…/…/…/…‎
WorkflowSe‎ttings.java‎ +15 -0
openapi-ge‎nerator/src‎
main/java/org/ope‎napitools/codegen‎
con‎fig‎
CodegenConfi‎gurator.java‎ +6 -0
CodegenCo‎nfig.java‎ +4 -0
CodegenCon‎stants.java‎ +5 -0
DefaultCo‎degen.java‎ +28 -0
DefaultGen‎erator.java‎ +1 -1
InlineModelR‎esolver.java‎ +52 -29
te‎st‎
java/org/opena‎pitools/codegen‎
opt‎ions‎
BashClientOptio‎nsProvider.java‎ +1 -0
DartClientOptio‎nsProvider.java‎ +1 -0
DartDioClientOpt‎ionsProvider.java‎ +1 -0
DartDioNextClientO‎ptionsProvider.java‎ +1 -0
ElixirClientOpti‎onsProvider.java‎ +1 -0
HaskellServantOpt‎ionsProvider.java‎ +1 -0
PhpClientOptio‎nsProvider.java‎ +1 -0
PhpLumenServerOpt‎ionsProvider.java‎ +1 -0
PhpSilexServerOpt‎ionsProvider.java‎ +1 -0
PhpSlim4ServerOpt‎ionsProvider.java‎ +1 -0
PhpSlimServerOpt‎ionsProvider.java‎ +1 -0
RubyClientOptio‎nsProvider.java‎ +1 -0
ScalaAkkaClientOp‎tionsProvider.java‎ +1 -0
ScalaHttpClientOp‎tionsProvider.java‎ +2 -1
Swift4Options‎Provider.java‎ +1 -0
Swift5Options‎Provider.java‎ +1 -0
TypeScriptAngularClie‎ntOptionsProvider.java‎ +1 -0
TypeScriptAngularJsCli‎entOptionsProvider.java‎ +1 -0
TypeScriptAureliaClie‎ntOptionsProvider.java‎ +1 -0
TypeScriptFetchClien‎tOptionsProvider.java‎ +1 -0
TypeScriptNestjsClien‎tOptionsProvider.java‎ +1 -0
TypeScriptNodeClient‎OptionsProvider.java‎ +1 -0
InlineModelRes‎olverTest.java‎ +38 -1
resour‎ces/3_0‎
issue_3‎100.yaml‎ +100 -0
modules/openapi-generator/src/main/java/org/openapitools/codegen/config/CodegenConfigurator.java
+ 6
- 0
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


@@ -477,6 +477,11 @@ public class CodegenConfigurator {
return this;
}
public CodegenConfigurator setAllowInlineSchemas(boolean allowInlineSchemas) {
workflowSettingsBuilder.withAllowInlineSchemas(allowInlineSchemas);
return this;
}
@SuppressWarnings("WeakerAccess")
public Context<?> toContext() {
Validate.notEmpty(generatorName, "generator name must be specified");
@@ -592,6 +597,7 @@ public class CodegenConfigurator {
config.setEnablePostProcessFile(workflowSettings.isEnablePostProcessFile());
config.setEnableMinimalUpdate(workflowSettings.isEnableMinimalUpdate());
config.setStrictSpecBehavior(workflowSettings.isStrictSpecBehavior());
config.setAllowInlineSchemas(workflowSettings.isAllowInlineSchemas());
TemplatingEngineAdapter templatingEngine = TemplatingEngineLoader.byIdentifier(workflowSettings.getTemplatingEngineName());
config.setTemplatingEngine(templatingEngine);
modules/openapi-generator/src/main/java/org/openapitools/codegen/config/CodegenConfigurator.java
+ 6
- 0
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


@@ -477,6 +477,11 @@ public class CodegenConfigurator {
return this;
}
public CodegenConfigurator setAllowInlineSchemas(boolean allowInlineSchemas) {
workflowSettingsBuilder.withAllowInlineSchemas(allowInlineSchemas);
return this;
}
@SuppressWarnings("WeakerAccess")
public Context<?> toContext() {
Validate.notEmpty(generatorName, "generator name must be specified");
@@ -592,6 +597,7 @@ public class CodegenConfigurator {
config.setEnablePostProcessFile(workflowSettings.isEnablePostProcessFile());
config.setEnableMinimalUpdate(workflowSettings.isEnableMinimalUpdate());
config.setStrictSpecBehavior(workflowSettings.isStrictSpecBehavior());
config.setAllowInlineSchemas(workflowSettings.isAllowInlineSchemas());
TemplatingEngineAdapter templatingEngine = TemplatingEngineLoader.byIdentifier(workflowSettings.getTemplatingEngineName());
config.setTemplatingEngine(templatingEngine);
modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConfig.java
+ 4
- 0
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


Conflict: This file was modified in both the source and target branches. Ask someone with write access to resolve it.
@@ -299,5 +299,9 @@ public interface CodegenConfig {
void setRemoveEnumValuePrefix(boolean removeEnumValuePrefix);
boolean isAllowInlineSchemas();
void setAllowInlineSchemas(boolean allowInlineSchemas);
Schema unaliasSchema(Schema schema, Map<String, String> usedImportMappings);
}
modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConstants.java
+ 5
- 0
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


Conflict: This file was modified in both the source and target branches. Ask someone with write access to resolve it.
@@ -385,4 +385,9 @@ public class CodegenConstants {
"If true (default), keep the old (incorrect) behaviour that 'additionalProperties' is set to false by default.";
public static final String USE_ONEOF_DISCRIMINATOR_LOOKUP = "useOneOfDiscriminatorLookup";
public static final String USE_ONEOF_DISCRIMINATOR_LOOKUP_DESC = "Use the discriminator's mapping in oneOf to speed up the model lookup. IMPORTANT: Validation (e.g. one and only one match in oneOf's schemas) will be skipped.";
public static final String ALLOW_INLINE_SCHEMAS = "allowInlineSchemas";
public static final String ALLOW_INLINE_SCHEMAS_DESC =
"If false (default), generate a new model for each inline schema, independently of a discriminator's presence in a composed model (containing allOf, anyOf or oneOf). " +
"If true, compose the inline schema if there is no discriminator in it else generate a new model and use inheritance";
}
modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
+ 28
- 0
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


Conflict: This file was modified in both the source and target branches. Ask someone with write access to resolve it.
@@ -250,6 +250,9 @@ public class DefaultCodegen implements CodegenConfig {
// See CodegenConstants.java for more details.
protected boolean disallowAdditionalPropertiesIfNotPresent = true;
// flag to indicate whether or not a new model is generated for an inline schema without discriminator
protected boolean allowInlineSchemas = false;
// make openapi available to all methods
protected OpenAPI openAPI;
@@ -368,6 +371,10 @@ public class DefaultCodegen implements CodegenConfig {
this.setDisallowAdditionalPropertiesIfNotPresent(Boolean.parseBoolean(additionalProperties
.get(CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT).toString()));
}
if (additionalProperties.containsKey(CodegenConstants.ALLOW_INLINE_SCHEMAS)) {
this.setAllowInlineSchemas(Boolean.parseBoolean(additionalProperties
.get(CodegenConstants.ALLOW_INLINE_SCHEMAS).toString()));
}
}
/***
@@ -1229,6 +1236,10 @@ public class DefaultCodegen implements CodegenConfig {
this.useOneOfInterfaces = useOneOfInterfaces;
}
public Boolean getAllowInlineSchemas() { return allowInlineSchemas; }
public void setAllowInlineSchemas(Boolean allowInlineSchemas) { this.allowInlineSchemas = allowInlineSchemas; }
/**
* Return the regular expression/JSON schema pattern (http://json-schema.org/latest/json-schema-validation.html#anchor33)
*
@@ -1537,6 +1548,8 @@ public class DefaultCodegen implements CodegenConfig {
cliOptions.add(disallowAdditionalPropertiesIfNotPresentOpt);
this.setDisallowAdditionalPropertiesIfNotPresent(true);
cliOptions.add(CliOption.newBoolean(CodegenConstants.ALLOW_INLINE_SCHEMAS,
CodegenConstants.ALLOW_INLINE_SCHEMAS_DESC).defaultValue(Boolean.TRUE.toString()));
// initialize special character mapping
initializeSpecialCharacterMapping();
@@ -6505,6 +6518,21 @@ public class DefaultCodegen implements CodegenConfig {
this.removeEnumValuePrefix = removeEnumValuePrefix;
}
/**
* Get the boolean value indicating whether to allow inline schemas
*/
@Override
public boolean isAllowInlineSchemas() { return this.allowInlineSchemas; }
/**
* Set the boolean value indicating whether to allow inline schemas
*
* @param allowInlineSchemas true to allow inline schemas
*/
@Override
public void setAllowInlineSchemas(boolean allowInlineSchemas) {
this.allowInlineSchemas = allowInlineSchemas;
}
//// Following methods are related to the "useOneOfInterfaces" feature
/**
modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java
+ 1
- 1
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


Conflict: This file was modified in both the source and target branches. Ask someone with write access to resolve it.
@@ -859,7 +859,7 @@ public class DefaultGenerator implements Generator {
// resolve inline models
InlineModelResolver inlineModelResolver = new InlineModelResolver();
inlineModelResolver.flatten(openAPI);
inlineModelResolver.flatten(openAPI, config);
configureGeneratorProperties();
configureOpenAPIInfo();
modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java
+ 52
- 29
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


Conflict: This file was modified in both the source and target branches. Ask someone with write access to resolve it.
@@ -53,7 +53,11 @@ public class InlineModelResolver {
final Logger LOGGER = LoggerFactory.getLogger(InlineModelResolver.class);
void flatten(OpenAPI openapi) {
void flatten(OpenAPI openAPI) {
flatten(openAPI, null);
}
void flatten(OpenAPI openapi, CodegenConfig config) {
this.openapi = openapi;
if (openapi.getComponents() == null) {
@@ -65,7 +69,7 @@ public class InlineModelResolver {
}
flattenPaths(openapi);
flattenComponents(openapi);
flattenComponents(openapi, config);
}
/**
@@ -370,7 +374,7 @@ public class InlineModelResolver {
* @param key a unique name ofr the composed schema.
* @param children the list of nested schemas within a composed schema (allOf, anyOf, oneOf).
*/
private void flattenComposedChildren(OpenAPI openAPI, String key, List<Schema> children) {
private void flattenComposedChildren(OpenAPI openAPI, CodegenConfig config, String key, List<Schema> children) {
if (children == null || children.isEmpty()) {
return;
}
@@ -381,29 +385,48 @@ public class InlineModelResolver {
(component.get$ref() == null) &&
((component.getProperties() != null && !component.getProperties().isEmpty()) ||
(component.getEnum() != null && !component.getEnum().isEmpty()))) {
// If a `title` attribute is defined in the inline schema, codegen uses it to name the
// inline schema. Otherwise, we'll use the default naming such as InlineObject1, etc.
// We know that this is not the best way to name the model.
//
// Such naming strategy may result in issues. If the value of the 'title' attribute
// happens to match a schema defined elsewhere in the specification, 'innerModelName'
// will be the same as that other schema.
//
// To have complete control of the model naming, one can define the model separately
// instead of inline.
String innerModelName = resolveModelName(component.getTitle(), key);
Schema innerModel = modelFromProperty(openAPI, component, innerModelName);
String existing = matchGenerated(innerModel);
if (existing == null) {
openAPI.getComponents().addSchemas(innerModelName, innerModel);
addGenerated(innerModelName, innerModel);
Schema schema = new Schema().$ref(innerModelName);
schema.setRequired(component.getRequired());
listIterator.set(schema);
if ((config != null && config.isAllowInlineSchemas()) && component.getDiscriminator() == null) {
String schemaName = key.substring(0, key.length() - 6);
Schema schema = openAPI.getComponents().getSchemas().get(schemaName);
flattenProperties(openAPI, component.getProperties(), schemaName);
schema.setType("object");
if (schema.getProperties() != null) {
schema.getProperties().putAll(component.getProperties());
} else {
schema.setProperties(component.getProperties());
}
if (schema.getRequired() != null) {
schema.getRequired().addAll(component.getRequired());
} else {
schema.setRequired(component.getRequired());
}
} else {
Schema schema = new Schema().$ref(existing);
schema.setRequired(component.getRequired());
listIterator.set(schema);
// If a `title` attribute is defined in the inline schema, codegen uses it to name the
// inline schema. Otherwise, we'll use the default naming such as InlineObject1, etc.
// We know that this is not the best way to name the model.
//
// Such naming strategy may result in issues. If the value of the 'title' attribute
// happens to match a schema defined elsewhere in the specification, 'innerModelName'
// will be the same as that other schema.
//
// To have complete control of the model naming, one can define the model separately
// instead of inline.
String innerModelName = resolveModelName(component.getTitle(), key);
Schema innerModel = modelFromProperty(openAPI, component, innerModelName);
String existing = matchGenerated(innerModel);
if (existing == null) {
openAPI.getComponents().addSchemas(innerModelName, innerModel);
addGenerated(innerModelName, innerModel);
Schema schema = new Schema().$ref(innerModelName);
schema.setRequired(component.getRequired());
listIterator.set(schema);
} else {
Schema schema = new Schema().$ref(existing);
schema.setRequired(component.getRequired());
listIterator.set(schema);
}
}
}
}
@@ -414,7 +437,7 @@ public class InlineModelResolver {
*
* @param openAPI target spec
*/
private void flattenComponents(OpenAPI openAPI) {
private void flattenComponents(OpenAPI openAPI, CodegenConfig config) {
Map<String, Schema> models = openAPI.getComponents().getSchemas();
if (models == null) {
return;
@@ -426,9 +449,9 @@ public class InlineModelResolver {
if (ModelUtils.isComposedSchema(model)) {
ComposedSchema m = (ComposedSchema) model;
// inline child schemas
flattenComposedChildren(openAPI, modelName + "_allOf", m.getAllOf());
flattenComposedChildren(openAPI, modelName + "_anyOf", m.getAnyOf());
flattenComposedChildren(openAPI, modelName + "_oneOf", m.getOneOf());
flattenComposedChildren(openAPI, config, modelName + "_allOf", m.getAllOf());
flattenComposedChildren(openAPI, null, modelName + "_anyOf", m.getAnyOf());
flattenComposedChildren(openAPI, null, modelName + "_oneOf", m.getOneOf());
} else if (model instanceof Schema) {
Schema m = model;
Map<String, Schema> properties = m.getProperties();
modules/openapi-generator/src/test/java/org/openapitools/codegen/options/BashClientOptionsProvider.java
+ 1
- 0
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


Conflict: This file was modified in both the source and target branches. Ask someone with write access to resolve it.
@@ -72,6 +72,7 @@ public class BashClientOptionsProvider implements OptionsProvider {
.put(CodegenConstants.PREPEND_FORM_OR_BODY_PARAMETERS, PREPEND_FORM_OR_BODY_PARAMETERS_VALUE)
.put(CodegenConstants.LEGACY_DISCRIMINATOR_BEHAVIOR, "true")
.put(CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT, "true")
.put(CodegenConstants.ALLOW_INLINE_SCHEMAS, "false")
.build();
}
modules/openapi-generator/src/test/java/org/openapitools/codegen/options/DartClientOptionsProvider.java
+ 1
- 0
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


@@ -64,6 +64,7 @@ public class DartClientOptionsProvider implements OptionsProvider {
.put(CodegenConstants.PREPEND_FORM_OR_BODY_PARAMETERS, PREPEND_FORM_OR_BODY_PARAMETERS_VALUE)
.put(CodegenConstants.LEGACY_DISCRIMINATOR_BEHAVIOR, "true")
.put(CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT, "true")
.put(CodegenConstants.ALLOW_INLINE_SCHEMAS, "false")
.put("serializationLibrary", "custom")
.build();
}
modules/openapi-generator/src/test/java/org/openapitools/codegen/options/DartDioClientOptionsProvider.java
+ 1
- 0
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


Conflict: This file was modified in both the source and target branches. Ask someone with write access to resolve it.
@@ -68,6 +68,7 @@ public class DartDioClientOptionsProvider implements OptionsProvider {
.put(DartDioClientCodegen.NULLABLE_FIELDS, NULLABLE_FIELDS)
.put(CodegenConstants.LEGACY_DISCRIMINATOR_BEHAVIOR, "true")
.put(CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT, "true")
.put(CodegenConstants.ALLOW_INLINE_SCHEMAS, "false")
.build();
}
modules/openapi-generator/src/test/java/org/openapitools/codegen/options/DartDioNextClientOptionsProvider.java
+ 1
- 0
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


Conflict: This file was modified in the source branch, but removed in the target branch. Ask someone with write access to resolve it.
@@ -64,6 +64,7 @@ public class DartDioNextClientOptionsProvider implements OptionsProvider {
.put(CodegenConstants.PREPEND_FORM_OR_BODY_PARAMETERS, PREPEND_FORM_OR_BODY_PARAMETERS_VALUE)
.put(CodegenConstants.LEGACY_DISCRIMINATOR_BEHAVIOR, "true")
.put(CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT, "true")
.put(CodegenConstants.ALLOW_INLINE_SCHEMAS, "false")
.build();
}
modules/openapi-generator/src/test/java/org/openapitools/codegen/options/ElixirClientOptionsProvider.java
+ 1
- 0
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


Conflict: This file was modified in both the source and target branches. Ask someone with write access to resolve it.
@@ -45,6 +45,7 @@ public class ElixirClientOptionsProvider implements OptionsProvider {
.put(CodegenConstants.PREPEND_FORM_OR_BODY_PARAMETERS, PREPEND_FORM_OR_BODY_PARAMETERS_VALUE)
.put(CodegenConstants.LEGACY_DISCRIMINATOR_BEHAVIOR, "true")
.put(CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT, "true")
.put(CodegenConstants.ALLOW_INLINE_SCHEMAS, "false")
.build();
}
modules/openapi-generator/src/test/java/org/openapitools/codegen/options/HaskellServantOptionsProvider.java
+ 1
- 0
  • View file @ 8cf451af

  • Edit in single-file editor

  • Open in Web IDE


Conflict: This file was modified in both the source and target branches. Ask someone with write access to resolve it.
@@ -50,6 +50,7 @@ public class HaskellServantOptionsProvider implements OptionsProvider {
.put(HaskellServantCodegen.PROP_SERVE_STATIC, HaskellServantCodegen.PROP_SERVE_STATIC_DEFAULT.toString())
.put(CodegenConstants.LEGACY_DISCRIMINATOR_BEHAVIOR, "true")
.put(CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT, "true")
.put(CodegenConstants.ALLOW_INLINE_SCHEMAS, "false")
.build();
}
0 Assignees
None
Assign to
0 Reviewers
None
Request review from
Labels
0
None
0
None
    Assign labels
  • Manage project labels

Milestone
No milestone
None
None
Time tracking
No estimate or time spent
Lock merge request
Unlocked
2
2 participants
Administrator
Jim Schubert
Reference: OpenAPITools/openapi-generator!9834
Source branch: github/fork/leo-sale/fix-unnecessary-models

Menu

Explore Projects Groups Snippets