Skip to content

[Inline model resolver] various improvements #12293

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.

Sign up for GitHub

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
wing328 merged 14 commits into from
May 10, 2022
Merged

[Inline model resolver] various improvements #12293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
772461a
better handling of requestbody in the inline resolver
wing328 May 3, 2022
eb6057c
remove commented code
wing328 May 3, 2022
8cca3ad
better request body naming
wing328 May 3, 2022
8ea4676
fix unique naming
wing328 May 3, 2022
654d6fc
minor code format change
wing328 May 4, 2022
ecaf120
removed additional underscore from names, fix test
wing328 May 5, 2022
55bec72
more fixes, update tests
wing328 May 6, 2022
9bf3d81
fix all tests
wing328 May 7, 2022
3fdf290
undo changes to default codegen
wing328 May 7, 2022
81a898d
Merge remote-tracking branch 'origin/master' into inline-request-body
wing328 May 7, 2022
dc29dc8
update samples
wing328 May 7, 2022
b863332
update python tests
wing328 May 7, 2022
ead4a7e
add new files
wing328 May 7, 2022
ce41a83
update samples
wing328 May 7, 2022
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
195 changes: 82 additions & 113 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,7 @@ public static Callback getCallback(OpenAPI openAPI, String name) {
* Return the first defined Schema for a RequestBody
*
* @param requestBody request body of the operation
* @return firstSchema
* @return first schema
*/
public static Schema getSchemaFromRequestBody(RequestBody requestBody) {
return getSchemaFromContent(requestBody.getContent());
Expand Down
31 changes: 16 additions & 15 deletions modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,8 @@ public void testFormParameterHasDefaultValue() {
final DefaultCodegen codegen = new DefaultCodegen();
codegen.setOpenAPI(openAPI);

Schema requestBodySchema = ModelUtils.getSchemaFromRequestBody(openAPI.getPaths().get("/fake").getGet().getRequestBody());
Schema requestBodySchema = ModelUtils.getReferencedSchema(openAPI,
ModelUtils.getSchemaFromRequestBody(openAPI.getPaths().get("/fake").getGet().getRequestBody()));
CodegenParameter codegenParameter = codegen.fromFormProperty("enum_form_string", (Schema) requestBodySchema.getProperties().get("enum_form_string"), new HashSet<String>());

Assert.assertEquals(codegenParameter.defaultValue, "-efg");
Expand All @@ -245,6 +246,8 @@ public void testDateTimeFormParameterHasDefaultValue() {
codegen.setOpenAPI(openAPI);

Schema requestBodySchema = ModelUtils.getSchemaFromRequestBody(openAPI.getPaths().get("/thingy/{date}").getPost().getRequestBody());
// dereference
requestBodySchema = ModelUtils.getReferencedSchema(openAPI, requestBodySchema);
CodegenParameter codegenParameter = codegen.fromFormProperty("visitDate", (Schema) requestBodySchema.getProperties().get("visitDate"),
new HashSet<>());

Expand Down Expand Up @@ -613,7 +616,8 @@ public void testGetSchemaTypeWithComposedSchemaWithOneOf() {
final DefaultCodegen codegen = new DefaultCodegen();

Operation operation = openAPI.getPaths().get("/state").getPost();
Schema schema = ModelUtils.getSchemaFromRequestBody(operation.getRequestBody());
Schema schema = ModelUtils.getReferencedSchema(openAPI,
ModelUtils.getSchemaFromRequestBody(operation.getRequestBody()));
String type = codegen.getSchemaType(schema);

Assert.assertNotNull(type);
Expand Down Expand Up @@ -2344,18 +2348,14 @@ public void testUseOneOfInterfaces() {
cg.preprocessOpenAPI(openAPI);

// assert names of the response/request schema oneOf interfaces are as expected
Assert.assertEquals(
openAPI.getPaths()
.get("/state")
.getPost()
.getRequestBody()
.getContent()
.get("application/json")
.getSchema()
.getExtensions()
.get("x-one-of-name"),
"CreateState"
);
Schema s = ModelUtils.getReferencedSchema(openAPI, openAPI.getPaths()
.get("/state")
.getPost()
.getRequestBody()
.getContent()
.get("application/json")
.getSchema());
Assert.assertEquals(s.getExtensions().get("x-one-of-name"), "CreateStateRequest");
Assert.assertEquals(
openAPI.getPaths()
.get("/state")
Expand All @@ -2372,7 +2372,8 @@ public void testUseOneOfInterfaces() {
// for the array schema, assert that a oneOf interface was added to schema map
Schema items = ((ArraySchema) openAPI.getComponents().getSchemas().get("CustomOneOfArraySchema")).getItems();
Assert.assertEquals(items.get$ref(), "#/components/schemas/CustomOneOfArraySchema_inner");
Schema innerItem = openAPI.getComponents().getSchemas().get("CustomOneOfArraySchema_inner");
//Assert.assertEquals(items.get$ref(), "#/components/schemas/createState_request");
Schema innerItem = ModelUtils.getReferencedSchema(openAPI, openAPI.getComponents().getSchemas().get("CustomOneOfArraySchema_inner"));
Assert.assertEquals(innerItem.getExtensions().get("x-one-of-name"), "CustomOneOfArraySchemaInner");
}

Expand Down
54 changes: 31 additions & 23 deletions ...les/openapi-generator/src/test/java/org/openapitools/codegen/InlineModelResolverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public void resolveInlineModel2EqualInnerModels() {
}

@Test
public void resolveInlineModel2DifferentInnerModelsWIthSameTitle() {
public void resolveInlineModel2DifferentInnerModelsWithSameTitle() {
OpenAPI openapi = new OpenAPI();
openapi.setComponents(new Components());
openapi.getComponents().addSchemas("User", new ObjectSchema()
Expand Down Expand Up @@ -329,7 +329,9 @@ public void resolveInlineRequestBodyWhenNoComponents() {
new InlineModelResolver().flatten(openAPI);

assertNotNull(openAPI.getComponents());
assertNotNull(openAPI.getComponents().getRequestBodies());
// no longer create inline requestBodies as references in the refactored inline model resolver (6.x)
assertNull(openAPI.getComponents().getRequestBodies());
assertNotNull(openAPI.getComponents().getSchemas().get("test1_request"));
}

@Test
Expand Down Expand Up @@ -359,7 +361,8 @@ public void resolveInlineRequestBody() {
.get("/resolve_inline_request_body")
.getPost()
.getRequestBody();
assertNotNull(requestBodyReference.get$ref());
assertEquals("#/components/schemas/resolveInlineRequestBody_request",
requestBodyReference.getContent().get("application/json").getSchema().get$ref());

RequestBody requestBody = ModelUtils.getReferencedRequestBody(openAPI, requestBodyReference);
MediaType mediaType = requestBody.getContent().get("application/json");
Expand Down Expand Up @@ -391,7 +394,8 @@ public void resolveInlineRequestBodyWithTitle() {
new InlineModelResolver().flatten(openAPI);

RequestBody requestBodyReference = openAPI.getPaths().get("/resolve_inline_request_body_with_title").getPost().getRequestBody();
assertEquals("#/components/requestBodies/resolve_inline_request_body_with_title", requestBodyReference.get$ref());
assertEquals("#/components/schemas/resolve_inline_request_body_with_title",
requestBodyReference.getContent().get("application/json").getSchema().get$ref());
}

@Test
Expand Down Expand Up @@ -429,7 +433,7 @@ public void resolveInlineArrayRequestBody() {

ArraySchema requestBody = (ArraySchema) mediaType.getSchema();
assertNotNull(requestBody.getItems().get$ref());
assertEquals("#/components/schemas/inline_object_2", requestBody.getItems().get$ref());
assertEquals("#/components/schemas/resolveInlineArrayRequestBody_request_inner", requestBody.getItems().get$ref());

Schema items = ModelUtils.getReferencedSchema(openAPI, ((ArraySchema) mediaType.getSchema()).getItems());
assertTrue(items.getProperties().get("street") instanceof StringSchema);
Expand Down Expand Up @@ -603,10 +607,11 @@ public void arbitraryObjectRequestBodyProperty() {
.getContent()
.get("application/json");

assertTrue(mediaType.getSchema() instanceof ObjectSchema);

ObjectSchema requestBodySchema = (ObjectSchema) mediaType.getSchema();
assertTrue(requestBodySchema.getProperties().get("arbitrary_object_request_body_property") instanceof ObjectSchema);
assertEquals("#/components/schemas/arbitraryObjectRequestBodyProperty_request", mediaType.getSchema().get$ref());
Schema requestBodySchema = ModelUtils.getReferencedSchema(openAPI, mediaType.getSchema());
assertNotNull(requestBodySchema);
assertEquals(1, requestBodySchema.getProperties().size(), 1);
assertTrue(requestBodySchema.getProperties().get("arbitrary_object_request_body_property") instanceof ObjectSchema);
}

@Test
Expand Down Expand Up @@ -903,9 +908,9 @@ public void arbitraryObjectModelWithArrayInlineWithTitle() {

ObjectSchema items = (ObjectSchema) openAPI.getComponents().getSchemas().get("ArbitraryObjectModelWithArrayInlineWithTitleInner");
assertEquals("ArbitraryObjectModelWithArrayInlineWithTitleInner", items.getTitle());
assertTrue(items.getProperties().get("arbitrary_object_model_with_array_inline_with_title") instanceof ObjectSchema);
assertTrue(items.getProperties().get("arbitrary_object_model_with_array_inline_with_title_property") instanceof ObjectSchema);

ObjectSchema itemsProperty = (ObjectSchema) items.getProperties().get("arbitrary_object_model_with_array_inline_with_title");
ObjectSchema itemsProperty = (ObjectSchema) items.getProperties().get("arbitrary_object_model_with_array_inline_with_title_property");
assertNull(itemsProperty.getProperties());
}

Expand Down Expand Up @@ -941,19 +946,21 @@ public void nullable() {
Schema nullablePropertySchema = ModelUtils.getReferencedSchema(openAPI, nullablePropertyReference);
assertTrue(nullablePropertySchema.getNullable());


Schema nullableRequestBodyReference = (Schema) openAPI
.getPaths()
.get("/nullable_properties")
.getPost()
.getRequestBody()
.getContent()
.get("application/json")
.getSchema()
.getProperties()
.get("nullable_request_body_property");
.getSchema();
//.getProperties()
//.get("nullable_request_body_property");
Schema nullableRequestBodySchema = ModelUtils.getReferencedSchema(openAPI, nullableRequestBodyReference);
assertTrue(nullableRequestBodySchema.getNullable());
//assertEquals(nullableRequestBodySchema, "");
Schema nullableSchema = ModelUtils.getReferencedSchema(openAPI,
((Schema)nullableRequestBodySchema.getProperties().get("nullable_request_body_property")));
assertTrue(nullableSchema.getNullable());
}

@Test
Expand All @@ -970,14 +977,15 @@ public void callbacks() {
.get("{$request.body#/callbackUri}")
.getPost()
.getRequestBody();
assertNotNull(callbackRequestBodyReference.get$ref());
assertNotNull(callbackRequestBodyReference.getContent().get("application/json").getSchema().get$ref());
assertEquals("#/components/schemas/webhookNotify_request", callbackRequestBodyReference.getContent().get("application/json").getSchema().get$ref());

RequestBody resolvedCallbackRequestBody = openAPI
/*RequestBody resolvedCallbackRequestBody = openAPI
.getComponents()
.getRequestBodies()
.get(ModelUtils.getSimpleRef(callbackRequestBodyReference.get$ref()));
.getSchemas()
.get(ModelUtils.getSimpleRef(callbackRequestBodyReference.getContent().get("application/json").getSchema().get$ref()));*/

Schema callbackRequestSchemaReference = resolvedCallbackRequestBody
Schema callbackRequestSchemaReference = callbackRequestBodyReference
.getContent()
.get("application/json")
.getSchema();
Expand All @@ -999,8 +1007,8 @@ public void testInlineSchemaNameMapping() {
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/inline_model_resolver.yaml");
InlineModelResolver resolver = new InlineModelResolver();
Map<String, String> inlineSchemaNames = new HashMap<>();
inlineSchemaNames.put("inline_object_2", "SomethingMapped");
inlineSchemaNames.put("inline_object_4", "nothing_new");
inlineSchemaNames.put("resolveInlineArrayRequestBody_request_inner", "SomethingMapped");
inlineSchemaNames.put("arbitraryRequestBodyArrayProperty_request_inner", "nothing_new");
resolver.setInlineSchemaNameMapping(inlineSchemaNames);
resolver.flatten(openAPI);

Expand Down
8 changes: 4 additions & 4 deletions ...enapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,8 +517,8 @@ public void testDoGenerateRequestBodyRequiredAttribute_3134_Regression() throws
generator.opts(input).generate();

JavaFileAssert.assertThat(Paths.get(outputPath + "/src/main/java/org/openapitools/api/ExampleApi.java"))
.assertMethod("exampleApiPost", "InlineObject")
.hasParameter("inlineObject")
.assertMethod("exampleApiPost", "InlineRequest")
.hasParameter("inlineRequest")
.assertParameterAnnotations()
.containsWithNameAndAttributes("RequestBody", ImmutableMap.of("required", "false"));

Expand Down Expand Up @@ -591,7 +591,7 @@ public void testMultipartBoot() throws IOException {

// Check that api validates mixed multipart request
JavaFileAssert.assertThat(files.get("MultipartMixedApi.java"))
.assertMethod("multipartMixed", "MultipartMixedStatus", "MultipartFile", "MultipartMixedMarker")
.assertMethod("multipartMixed", "MultipartMixedStatus", "MultipartFile", "MultipartMixedRequestMarker")
.hasParameter("status").withType("MultipartMixedStatus")
.assertParameterAnnotations()
.containsWithName("Valid")
Expand All @@ -602,7 +602,7 @@ public void testMultipartBoot() throws IOException {
.assertParameterAnnotations()
.containsWithNameAndAttributes("RequestPart", ImmutableMap.of("value", "\"file\"", "required", "true"))
.toParameter().toMethod()
.hasParameter("marker").withType("MultipartMixedMarker")
.hasParameter("marker").withType("MultipartMixedRequestMarker")
.assertParameterAnnotations()
.containsWithNameAndAttributes("RequestParam", ImmutableMap.of("value", "\"marker\"", "required", "false"));
}
Expand Down
6 changes: 5 additions & 1 deletion ...erator/src/test/java/org/openapitools/codegen/typescript/TypeScriptClientCodegenTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ public void testComposedSchemasImportTypesIndividually() {
codegen.setOpenAPI(openApi);
PathItem path = openApi.getPaths().get("/pets");
CodegenOperation operation = codegen.fromOperation("/pets", "patch", path.getPatch(), path.getServers());
Assert.assertEquals(operation.imports, Sets.newHashSet("Cat", "Dog"));
// TODO revise the commented test below as oneOf is no longer defined inline
//but instead defined using $ref with the new inline model resolver in 6.x
//Assert.assertEquals(operation.imports, Sets.newHashSet("Cat", "Dog"));
Assert.assertEquals(operation.imports, Sets.newHashSet("InlineRequest"));
Copy link
Member Author

@wing328 wing328 May 9, 2022

Choose a reason for hiding this comment

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

@TiFu FYI. We've updated the inlined resolver to create a separate model/schema for all inline schemas(including oneOf/allOf) in the request bodies.


}

@Test
Expand Down
3 changes: 2 additions & 1 deletion modules/openapi-generator/src/test/java/org/openapitools/codegen/utils/ModelUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public void testGetAllUsedSchemas() {
"SomeObj18",
"Common18",
"SomeObj18_allOf",
"inline_request",
"Obj19ByAge",
"Obj19ByType",
"SomeObj20",
Expand All @@ -78,7 +79,7 @@ public void testGetAllUsedSchemas() {
"AChild30",
"BChild30"
);
Assert.assertEquals(allUsedSchemas.size(), expectedAllUsedSchemas.size());
Assert.assertEquals(allUsedSchemas, expectedAllUsedSchemas);
Assert.assertTrue(allUsedSchemas.containsAll(expectedAllUsedSchemas));
}

Expand Down
11 changes: 10 additions & 1 deletion modules/openapi-generator/src/test/resources/3_0/composed-oneof.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,13 @@ components:
oneOf:
- $ref: '#/components/schemas/ObjA'
- $ref: '#/components/schemas/ObjB'
- $ref: '#/components/schemas/ObjC'
discriminator:
propertyName: realtype
mapping:
a-type: '#/components/schemas/ObjA'
b-type: '#/components/schemas/ObjB'
c-type: '#/components/schemas/ObjC'
ObjA:
type: object
properties:
Expand All @@ -78,4 +80,11 @@ components:
type: string
code:
type: integer
format: int32
format: int32
ObjC:
type: object
properties:
realtype:
type: string
state:
type: string
2 changes: 1 addition & 1 deletion modules/openapi-generator/src/test/resources/3_0/inline_model_resolver.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ components:
title: ArbitraryObjectModelWithArrayInlineWithTitleInner
type: object
properties:
arbitrary_object_model_with_array_inline_with_title:
arbitrary_object_model_with_array_inline_with_title_property:
type: object
EmptyExampleOnStringTypeModels:
type: string
Expand Down
16 changes: 8 additions & 8 deletions samples/client/others/csharp-netcore-complex-files/.openapi-generator/FILES
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
Org.OpenAPITools.sln
README.md
appveyor.yml
docs/InlineObject.md
docs/InlineObject1.md
docs/InlineObject2.md
docs/MultipartApi.md
docs/MultipartMixedMarker.md
docs/MultipartArrayRequest.md
docs/MultipartMixedRequest.md
docs/MultipartMixedRequestMarker.md
docs/MultipartMixedStatus.md
docs/MultipartSingleRequest.md
git_push.sh
src/Org.OpenAPITools.Test/Org.OpenAPITools.Test.csproj
src/Org.OpenAPITools/Api/MultipartApi.cs
Expand All @@ -28,9 +28,9 @@ src/Org.OpenAPITools/Client/OpenAPIDateConverter.cs
src/Org.OpenAPITools/Client/RequestOptions.cs
src/Org.OpenAPITools/Client/RetryConfiguration.cs
src/Org.OpenAPITools/Model/AbstractOpenAPISchema.cs
src/Org.OpenAPITools/Model/InlineObject.cs
src/Org.OpenAPITools/Model/InlineObject1.cs
src/Org.OpenAPITools/Model/InlineObject2.cs
src/Org.OpenAPITools/Model/MultipartMixedMarker.cs
src/Org.OpenAPITools/Model/MultipartArrayRequest.cs
src/Org.OpenAPITools/Model/MultipartMixedRequest.cs
src/Org.OpenAPITools/Model/MultipartMixedRequestMarker.cs
src/Org.OpenAPITools/Model/MultipartMixedStatus.cs
src/Org.OpenAPITools/Model/MultipartSingleRequest.cs
src/Org.OpenAPITools/Org.OpenAPITools.csproj
8 changes: 4 additions & 4 deletions samples/client/others/csharp-netcore-complex-files/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,11 @@ Class | Method | HTTP request | Description
<a name="documentation-for-models"></a>
## Documentation for Models

- [Model.InlineObject](docs/InlineObject.md)
- [Model.InlineObject1](docs/InlineObject1.md)
- [Model.InlineObject2](docs/InlineObject2.md)
- [Model.MultipartMixedMarker](docs/MultipartMixedMarker.md)
- [Model.MultipartArrayRequest](docs/MultipartArrayRequest.md)
- [Model.MultipartMixedRequest](docs/MultipartMixedRequest.md)
- [Model.MultipartMixedRequestMarker](docs/MultipartMixedRequestMarker.md)
- [Model.MultipartMixedStatus](docs/MultipartMixedStatus.md)
- [Model.MultipartSingleRequest](docs/MultipartSingleRequest.md)


<a name="documentation-for-authorization"></a>
Expand Down
6 changes: 3 additions & 3 deletions samples/client/others/csharp-netcore-complex-files/docs/MultipartApi.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ No authorization required

<a name="multipartmixed"></a>
# **MultipartMixed**
> void MultipartMixed (MultipartMixedStatus status, System.IO.Stream file, MultipartMixedMarker marker = null)
> void MultipartMixed (MultipartMixedStatus status, System.IO.Stream file, MultipartMixedRequestMarker marker = null)



Expand All @@ -105,7 +105,7 @@ namespace Example
var apiInstance = new MultipartApi(config);
var status = (MultipartMixedStatus) "ALLOWED"; // MultipartMixedStatus |
var file = new System.IO.MemoryStream(System.IO.File.ReadAllBytes("/path/to/file.txt")); // System.IO.Stream | a file
var marker = new MultipartMixedMarker(); // MultipartMixedMarker | (optional)
var marker = new MultipartMixedRequestMarker(); // MultipartMixedRequestMarker | (optional)

try
{
Expand All @@ -128,7 +128,7 @@ Name | Type | Description | Notes
------------- | ------------- | ------------- | -------------
**status** | **MultipartMixedStatus**| |
**file** | **System.IO.Stream****System.IO.Stream**| a file |
**marker** | [**MultipartMixedMarker**](MultipartMixedMarker.md)| | [optional]
**marker** | [**MultipartMixedRequestMarker**](MultipartMixedRequestMarker.md)| | [optional]

### Return type

Expand Down
10 changes: 10 additions & 0 deletions samples/client/others/csharp-netcore-complex-files/docs/MultipartArrayRequest.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Org.OpenAPITools.Model.MultipartArrayRequest

## Properties

Name | Type | Description | Notes
------------ | ------------- | ------------- | -------------
**Files** | **List&lt;System.IO.Stream&gt;** | Many files | [optional]

[[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md)

Loading