Dart codegen bug fixes: honor prefix/suffix, handle operation return types, replaces Object with Map #12574

jeffmikels · 2022-06-11T21:26:34Z

This pull requests modifies the Dart codegen and templates to accomplish the following:

Dart codegen now respects the modelNamePrefix, modelNameSuffix, apiNamePrefix, and apiNameSuffix settings
Dart codegen now properly handles the case when a query parameter is named _ (internally using the variable name q)
Dart codegen now supports an additional property named apiClientName to override the default ApiClient
CodegenOperation.java was modified to expose a returnInnerType field for those operations whose return value is an array or mapping type value.
Dart codegen makes use of the new returnInnerType value to properly handle the return type of operations.
Dart codegen now uses Map as the default type for an otherwise unspecified api object.

@jaumard (2018/09)
@josh-burton (2019/12)
@amondnet (2019/12)
@sbu-WBT (2020/12)
@kuhnroyal (2020/12)
@agilob (2020/12)
@ahmednfwela (2021/08)

PR checklist

Read the contribution guidelines.
Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
Run the following to build the project and update samples:
```
./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
```
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
For Windows users, please run the script in Git BASH.
File the PR against the correct branch: master (6.0.1) (patch release), 6.1.x (breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

…ize modelNamePrefix and Suffix, added apiClientName option to dart

ahmednfwela · 2022-06-12T09:09:02Z

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

@@ -31,7 +31,7 @@ public class CodegenOperation {
            isRestfulIndex, isRestfulShow, isRestfulCreate, isRestfulUpdate, isRestfulDestroy,
            isRestful, isDeprecated, isCallbackRequest, uniqueItems, hasDefaultResponse = false,
            hasErrorResponseObject; // if 4xx, 5xx responses have at least one error object defined
-    public String path, operationId, returnType, returnFormat, httpMethod, returnBaseType,
+    public String path, operationId, returnType, returnFormat, httpMethod, returnBaseType, returnInnerType,


what extra information does returnInnerType add ?

As an example, when the returnType is List<Map<String, String>> then returnBaseType will be List but the dart code, to be type safe, will want to call .cast<Map<String, String>>(). Therefore, we need to expose Map<String, String> to the template somehow.

In this case, returnInnerType will contain the string 'Map<String, String>'

why use this instead of just items property?

The generated code in the earlier version was simply wrong. Dart would fail to compile it under certain cases.

To see where it matters, look at the template in dart2 named api.mustache on line 180. In this PR, you'll see this:

{{#isArray}} final responseBody = await _decodeBodyBytes(response); return (await apiClient.deserializeAsync(responseBody, '{{{returnType}}}') as List) .cast<{{{returnInnerType}}}>() .{{#uniqueItems}}toSet(){{/uniqueItems}}{{^uniqueItems}}toList(){{/uniqueItems}}; {{/isArray}}

Previously, it said this:

{{#isArray}} final responseBody = await _decodeBodyBytes(response); return (await apiClient.deserializeAsync(responseBody, '{{{returnType}}}') as List) .cast<{{{returnBaseType}}}>() .{{#uniqueItems}}toSet(){{/uniqueItems}}{{^uniqueItems}}toList(){{/uniqueItems}}; {{/isArray}}

In most cases, that is fine, but consider the following example from the openApi document from the Mattermost project. There is this api endpoint:

"/teams/{team_id}/groups_by_channels": { "get": { "tags": [ "groups" ], "summary": "Get team groups by channels", "description": "Retrieve the set of groups associated with the channels in the given team grouped by channel.\n\n##### Permissions\nMust have `manage_system` permission or can access only for current user\n\n__Minimum server version__: 5.11\n", "operationId": "GetGroupsAssociatedToChannelsByTeam", "parameters": [ { "name": "team_id", "in": "path", "description": "Team GUID", "required": true, "schema": { "type": "string" } }, { "name": "page", "in": "query", "description": "The page to select.", "schema": { "type": "integer", "default": 0 } }, { "name": "per_page", "in": "query", "description": "The number of groups per page.", "schema": { "type": "integer", "default": 60 } }, { "name": "filter_allow_reference", "in": "query", "description": "Boolean which filters in the group entries with the `allow_reference` attribute set.", "schema": { "type": "boolean", "default": false } }, { "name": "paginate", "in": "query", "description": "Boolean to determine whether the pagination should be applied or not", "schema": { "type": "boolean", "default": false } } ], "responses": { "200": { "description": "Group list retrieval successful", "content": { "application/json": { "schema": { "type": "array", "items": { "$ref": "#/components/schemas/GroupsAssociatedToChannels" } } } } }, "400": { "$ref": "#/components/responses/BadRequest" }, "401": { "$ref": "#/components/responses/Unauthorized" }, "403": { "$ref": "#/components/responses/Forbidden" }, "500": { "$ref": "#/components/responses/InternalServerError" }, "501": { "$ref": "#/components/responses/NotImplemented" } } } },

In this case, the returnType will be List<Map<String, List<GroupWithSchemeAdmin>>> but the returnBaseType will be Map which will result in the following Dart code:

return (await apiClient.deserializeAsync(responseBody, 'List<Map<String, List<GroupWithSchemeAdmin>>>') as List) .cast<Map>() .toList();

However, this code will not compile because the .cast operation coerces all list objects to Map which is the same as Map<dynamic, dynamic> and is LESS SPECIFIC than Map<String, List<GroupWithSchemeAdmin>>. The compiler can't guarantee that this cast will be valid and so the code won't compile.

By adding the returnInnerType variable to the java class and by changing the mustache template, the new code will look like this:

return (await apiClient.deserializeAsync(responseBody, 'List<Map<String, List<GroupWithSchemeAdmin>>>') as List) .cast<Map<String, List<GroupWithSchemeAdmin>>>() .toList();

This code will compile.

Why not use the items property?

It's true that items.dataType contains the exact same data, but that value is not available to api.mustache in the context where it is needed.

This value must be exposed in the same context as returnType.

ahmednfwela · 2022-06-12T09:10:44Z

...tstore/dart-dio/petstore_client_lib_fake-json_serializable/lib/src/model/nullable_class.dart

@@ -122,7 +122,7 @@ class NullableClass {
  )


-  final List<Object>? arrayNullableProp;


Object doesn't have to be a Map, it can also be String,num,List ,....

I believe this came from my changes to AbstractDartCodegen.java line 152 where I specify that openApi generic object types should map to dart Map types.

This change is needed because the "Object" type in Dart does not equate to the "object" type in the openApi spec. Rather, openApi generic objects are equivalent to Dart "Map" types and should be treated as such.

then it would be list<map<string, object?>>

In most cases Dart deserializes a JSON object to Map<String, dynamic> so it could be List<Map<String, dynamic>> to be specific, but List<Map> should be fine. Remember, an object of type List<Map> can contain items of Map<String, dynamic> but an object of type List<Map<String, dynamic>> cannot contain items of type Map. Also remember that Map is the same as Map<dynamic, dynamic>.

ahmednfwela · 2022-06-12T09:11:49Z

openapi-generator.code-workspace

 		"maven.pomfile.autoUpdateEffectivePOM": true,
+		"workspace.isHidden": false,


why are these change necessary ?

For some reason, vscode was reformatting the java code to be different from what was already checked into the repo. I didn't want the diffs to look terrible, so I disabled format on save in my workspace, and vscode saved the changes to the committed workspace file.

jeffmikels · 2022-06-16T06:28:35Z

Are there any other questions?

jeffmikels · 2022-06-16T19:21:19Z

...i-generator/src/test/java/org/openapitools/codegen/options/DartDioClientOptionsProvider.java

@@ -37,6 +37,7 @@ public class DartDioClientOptionsProvider implements OptionsProvider {
    public static final String PUB_AUTHOR_VALUE = "Author";
    public static final String PUB_AUTHOR_EMAIL_VALUE = "author@homepage";
    public static final String PUB_HOMEPAGE_VALUE = "Homepage";
+    public static final String API_CLIENT_NAME_VALUE = "Homepage";


This change allows apiClient to be used in additionalProperties which will override the default name for the top level ApiClient class.

… class

wing328 · 2022-08-04T06:37:49Z

@jeffmikels thanks for the PR.

I would suggest breaking down the PR into smaller one for easier review and merge.

What about filing a separate for the following change to start with?

Dart codegen now respects the modelNamePrefix, modelNameSuffix, apiNamePrefix, and apiNameSuffix settings

jeffmikels · 2022-10-10T16:25:21Z

@jeffmikels thanks for the PR.

I would suggest breaking down the PR into smaller one for easier review and merge.

What about filing a separate for the following change to start with?

Dart codegen now respects the modelNamePrefix, modelNameSuffix, apiNamePrefix, and apiNameSuffix settings

The code changes were extensive because they required a number of changes to be implemented simultaneously. The respecting of modelNamePrefix, modelNameSuffix, apiNamePrefix, and apiNameSuffix settings was a nearly trivial change. The other changes were more important and the main reason for the PR.

kuhnroyal · 2023-05-02T14:23:52Z

@jeffmikels Do you have plans to break this in smaller pieces? Otherwise please be so kind and close the PR.

Nexushunter · 2023-10-15T22:14:01Z

Is there an ETA on this? I'm running into the issue of the Object.listFromJson here. If needed I can pick up the object => map conversion.

valpackett · 2024-06-05T07:22:28Z

I'm hitting the Object.listFromJson issue right now..

Just doing the typeMapping.put("object", "Map"); change didn't help, now it's Error: Member not found: 'Map.listFromJson'. in some places instead of Object

jeffmikels and others added 5 commits June 11, 2022 16:54

added returnInnerType for container types, made Dart generator recogn…

e8d7107

…ize modelNamePrefix and Suffix, added apiClientName option to dart

Merge branch 'OpenAPITools:master' into master

ad6a3fc

added returnInnerType for container types, made Dart generator recogn…

f1babda

…ize modelNamePrefix and Suffix, added apiClientName option to dart

added returnInnerType for container types, made Dart generator recogn…

7fd619d

…ize modelNamePrefix and Suffix, added apiClientName option to dart

added returnInnerType for container types, made Dart generator recogn…

57a5a27

…ize modelNamePrefix and Suffix, added apiClientName option to dart

jeffmikels changed the title ~~This pull request implements three specific changes for Dart~~ Dart codegen bug fixes: honor prefix/suffix, handle operation return types, replaces Object with Map Jun 11, 2022

ahmednfwela suggested changes Jun 12, 2022

View reviewed changes

making sure modelNamePrefix is honored throughout the library

3afd82a

jeffmikels commented Jun 16, 2022

View reviewed changes

fixed the default option for API_CLIENT_NAME when testing the DartDio…

fc9037e

… class

wing328 added Client: Dart Issue: Bug labels Jul 6, 2022

jeffmikels mentioned this pull request Oct 10, 2022

[BUG] [Dart] additional properties: fromJson with field of type List<Map<String, Object>> does not generate correctly #12914

Open

6 tasks

vcampitelli mentioned this pull request May 2, 2023

Dart client is not working FusionAuth/fusionauth-issues#2240

Closed

		@@ -122,7 +122,7 @@ class NullableClass {
		)


		final List<Object>? arrayNullableProp;

		"maven.pomfile.autoUpdateEffectivePOM": true,
		"workspace.isHidden": false,

Uh oh!

Dart codegen bug fixes: honor prefix/suffix, handle operation return types, replaces Object with Map #12574

Are you sure you want to change the base?

Dart codegen bug fixes: honor prefix/suffix, handle operation return types, replaces Object with Map #12574

Uh oh!

Conversation

jeffmikels commented Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffmikels Jun 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffmikels Jun 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffmikels Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffmikels Jun 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffmikels commented Jun 16, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wing328 commented Aug 4, 2022

Uh oh!

jeffmikels commented Oct 10, 2022

Uh oh!

kuhnroyal commented May 2, 2023

Uh oh!

Nexushunter commented Oct 15, 2023

Uh oh!

valpackett commented Jun 5, 2024

Uh oh!

Uh oh!

jeffmikels commented Jun 11, 2022 •

edited

Loading

jeffmikels Jun 13, 2022 •

edited

Loading

jeffmikels Jun 13, 2022 •

edited

Loading

jeffmikels Jun 16, 2022 •

edited

Loading

jeffmikels Jun 13, 2022 •

edited

Loading