Skip to content

[core] Add x-is-free-form vendor extension #6849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ public class ModelUtils {
// A vendor extension to track the value of the 'disallowAdditionalPropertiesIfNotPresent' CLI
private static final String disallowAdditionalPropertiesIfNotPresent = "x-disallow-additional-properties-if-not-present";

private static final String freeFormExplicit = "x-is-free-form";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use FREE_FORM_EXPLICIT to match Java style for constants?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to do this whenever we get more caught up on pull requests, but matched other constant styles in the file to avoid potential merge conflicts elsewhere.

We do track Sonar and this is a rule that's evaluated.


private static ObjectMapper JSON_MAPPER, YAML_MAPPER;

static {
Expand Down Expand Up @@ -672,25 +674,23 @@ public static boolean isEmailSchema(Schema schema) {
}

/**
* Check to see if the schema is a model with at least one property.
* Check to see if the schema is a model
*
* @param schema potentially containing a '$ref'
* @return true if it's a model with at least one properties
*/
public static boolean isModel(Schema schema) {
if (schema == null) {
// TODO: Is this message necessary? A null schema is not a model, so the result is correct.
once(LOGGER).error("Schema cannot be null in isModel check");
return false;
}

// has at least one property
if (schema.getProperties() != null && !schema.getProperties().isEmpty()) {
// has properties
if (null != schema.getProperties()) {
return true;
}

// composed schema is a model
return schema instanceof ComposedSchema;
// composed schema is a model, consider very simple ObjectSchema a model
return schema instanceof ComposedSchema || schema instanceof ObjectSchema;
}

/**
Expand Down Expand Up @@ -745,6 +745,16 @@ public static boolean isFreeFormObject(OpenAPI openAPI, Schema schema) {
// no properties
if ((schema.getProperties() == null || schema.getProperties().isEmpty())) {
Schema addlProps = getAdditionalProperties(openAPI, schema);

if (schema.getExtensions() != null && schema.getExtensions().containsKey(freeFormExplicit)) {
// User has hard-coded vendor extension to handle free-form evaluation.
boolean isFreeFormExplicit = Boolean.parseBoolean(String.valueOf(schema.getExtensions().get(freeFormExplicit)));
if (!isFreeFormExplicit && addlProps != null && addlProps.getProperties() != null && !addlProps.getProperties().isEmpty()) {
once(LOGGER).error(String.format(Locale.ROOT, "Potentially confusing usage of %s within model which defines additional properties", freeFormExplicit));
}
return isFreeFormExplicit;
}

// additionalProperties not defined
if (addlProps == null) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ public void testAuthorizationsHasMoreWhenFiltered() {
assertEquals(getCodegenOperation.authMethods.size(), 2);
assertTrue(getCodegenOperation.authMethods.get(0).hasMore);
Assert.assertFalse(getCodegenOperation.authMethods.get(1).hasMore);
}
}

@Test
public void testFreeFormObjects() {
Expand Down Expand Up @@ -881,7 +881,7 @@ public void testRestTemplateFormMultipart() throws IOException {
//single file
"multipartSingleWithHttpInfo(File file)",
"formParams.add(\"file\", new FileSystemResource(file));"
);
);
}

/**
Expand Down Expand Up @@ -927,6 +927,35 @@ public void testWebClientFormMultipart() throws IOException {
);
}

@Test
public void testAllowModelWithNoProperties() throws Exception {
File output = Files.createTempDirectory("test").toFile();

final CodegenConfigurator configurator = new CodegenConfigurator()
.setGeneratorName("java")
.setLibrary(JavaClientCodegen.OKHTTP_GSON)
.setInputSpec("src/test/resources/2_0/emptyBaseModel.yaml")
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));

final ClientOptInput clientOptInput = configurator.toClientOptInput();
DefaultGenerator generator = new DefaultGenerator();
List<File> files = generator.opts(clientOptInput).generate();

Assert.assertEquals(files.size(), 47);
TestUtils.ensureContainsFile(files, output, "src/main/java/org/openapitools/client/model/RealCommand.java");
TestUtils.ensureContainsFile(files, output, "src/main/java/org/openapitools/client/model/Command.java");

validateJavaSourceFiles(files);

TestUtils.assertFileContains(Paths.get(output + "/src/main/java/org/openapitools/client/model/RealCommand.java"),
"class RealCommand extends Command");

TestUtils.assertFileContains(Paths.get(output + "/src/main/java/org/openapitools/client/model/Command.java"),
"class Command");

output.deleteOnExit();
}

/**
* See https://github.com/OpenAPITools/openapi-generator/issues/6715
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,15 @@ public void testGlobalProducesConsumes() {
Assert.assertEquals(unusedSchemas.size(), 0);
}

@Test
public void testIsModelAllowsEmptyBaseModel() {
final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/2_0/emptyBaseModel.yaml");
Schema commandSchema = ModelUtils.getSchema(openAPI, "Command");

Assert.assertTrue(ModelUtils.isModel(commandSchema));
Assert.assertFalse(ModelUtils.isFreeFormObject(openAPI, commandSchema));
}

@Test
public void testReferencedSchema() {
Schema otherObj = new ObjectSchema().addProperties("sprop", new StringSchema()).addProperties("iprop", new IntegerSchema());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
swagger: "2.0"
info:
title: Test Command model generation
description: Test Command model generation
version: 1.0.0
host: localhost:8080
schemes:
- https
definitions:
Command:
title: Command
description: The base object for all command objects.
type: object
# Explicitly avoid treating as a "free-form" or dynamic object, resulting in classical languages as a class with no properties.
x-is-free-form: false
RealCommand:
title: RealCommand
description: The real command.
allOf:
- $ref: '#/definitions/Command'
ApiError:
description: The base object for API errors.
type: object
required:
- code
- message
properties:
code:
description: The error code. Usually, it is the HTTP error code.
type: string
readOnly: true
message:
description: The error message.
type: string
readOnly: true
title: ApiError
parameters:
b_real_command:
name: real_command
in: body
description: A payload for executing a real command.
required: true
schema:
$ref: '#/definitions/RealCommand'
paths:
/execute:
post:
produces: []
operationId: executeRealCommand
parameters:
- name: real_command
in: body
description: A payload for executing a real command.
required: true
schema:
$ref: '#/definitions/RealCommand'
responses:
'204':
description: Successful request. No content returned.
'400':
description: Bad request.
schema:
$ref: '#/definitions/ApiError'
'404':
description: Not found.
schema:
$ref: '#/definitions/ApiError'
default:
description: Unknown error.
schema:
$ref: '#/definitions/ApiError'