openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

Feat: Add freezed serialization to dart-dio generator

Open bahag-chandrana opened this issue 2 years ago • 44 comments

This PR is aimed at adding the freezed serialization to the dart-dio generator. The changes were actually copied from changes made on old branch which was tested extensively in personal projects. Since the branches diverged I have migrated the changes to the freshly cloned branch.

The new changes could be tested by simply changing the serialization library to freezed.

PR checklist

  • [x] Read the contribution guidelines.
  • [x] 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.
  • [x] 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.
  • [x] File the PR against the correct branch: master (6.1.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
  • [x] If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request. @jaumard (2018/09) @josh-burton (2019/12) @amondnet (2019/12) @sbu-WBT (2020/12) @kuhnroyal (2020/12) @agilob (2020/12) @ahmednfwela (2021/08)

TODO's:

  • [x] Add support of oneOf/anyOf/allOf
  • [x] Union based ResponseType
  • [ ] Required tests to validate changes

bahag-chandrana avatar Jul 31 '22 13:07 bahag-chandrana

can you base your PR on https://github.com/OpenAPITools/openapi-generator/pull/12295 ? It will help with oneOf/anyOf integration

ahmednfwela avatar Aug 01 '22 04:08 ahmednfwela

can you base your PR on #12295 ? It will help with oneOf/anyOf integration

I thought it is BuiltValue specific changes..! Also haven't quite understood yet what is happening in the adaptToDartInheritance method. If it's a general change, I will look into little more in detail and see if I can use it as well.

bahag-chandrana avatar Aug 01 '22 06:08 bahag-chandrana

my PR affects only built_value , but since you are modifying java files, it's best to rebase it on mine

ahmednfwela avatar Aug 01 '22 07:08 ahmednfwela

my PR affects only built_value , but since you are modifying java files, it's best to rebase it on mine

Wouldn't it make sense for me to rebase it from main once your PR is merged to main. I was thinking more in that line.

bahag-chandrana avatar Aug 01 '22 08:08 bahag-chandrana

if you are willing to wait until mine is merged, then yeah sure I just wanted to help add oneOf support

ahmednfwela avatar Aug 01 '22 09:08 ahmednfwela

I think we could rebase this now and get freezed finally going.

kuhnroyal avatar Sep 06 '22 22:09 kuhnroyal

I will work on it this week to rebase and see if could also include oneOf/anyOf support in freezed.

bahag-chandrana avatar Sep 12 '22 15:09 bahag-chandrana

Looks like this approval failed because of a test failure? When looking at the logs, it seems to be a temporary Maven failure. Can someone re-run the test again?

oravecz avatar Nov 14 '22 21:11 oravecz

@ahmednfwela Could you explain me how the new vendor extensions are to be used for the oneOf and anyOf types. I assume allOf all ready works as it should. In this freezed version I deserialize the models based on the descriminator property. This should be sufficient for both anyOf and oneOf as I understood.

bahag-chandrana avatar Nov 26 '22 10:11 bahag-chandrana

Also anybody know how to map response code to response models. Which mustache template variables can I use to create a union typed response for api endpoints. Meaning if response code is 200 map to model A and response code is 400 map to error model B.

bahag-chandrana avatar Nov 26 '22 10:11 bahag-chandrana

@bahag-chandrana the vendor extensions only describe parent/child relations, and they are:

"x-is-parent": false,
"x-has-self-and-ancestor-only-props": false,
"x-is-child": false,
"x-is-pure": false,
"x-self-only-props": [],
"x-has-ancestor-only-props": false,
"x-has-self-only-props": false,
"x-self-and-ancestor-only-props": [],
"x-ancestor-only-props": []

a pure model is a model with no allOf, oneOf nor anyOf


oneOf and anyOf describe a different problem however.

  • How do you represent a property that can be either String or num ?

This is why I made the one_of package, so you can do OneOf2<String, num>, and worry about serialization/deserialization later (e.g. in one_of_serializer)

but since Freezed supports unions, you can just do

@freezed
class MyModelUnion with _$MyModelUnion {
  const factory MyModelUnion.asNum(num value) = AsNum;
  const factory MyModelUnion.asString(String value) = AsString;
}

ahmednfwela avatar Nov 26 '22 10:11 ahmednfwela

Also anybody know how to map response code to response models.

they exist in the responses object for each operation e.g.

"responses": [
  {
    "headers": [],
    "responseHeaders": [],
    "code": "200",
    "is1xx": false,
    "is2xx": true,
    "is3xx": false,
    "is4xx": false,
    "is5xx": false,
    "message": "Bar created",
    "dataType": "Bar",
    "baseType": "Bar",
    "hasHeaders": false,
    "isString": false,
    "isNumeric": false,
    "isInteger": false,
    "isShort": false,
    "isLong": false,
    "isUnboundedInteger": false,
    "isNumber": false,
    "isFloat": false,
    "isDouble": false,
    "isDecimal": false,
    "isByteArray": false,
    "isBoolean": false,
    "isDate": false,
    "isDateTime": false,
    "isUuid": false,
    "isEmail": false,
    "isModel": true,
    "isFreeFormObject": false,
    "isAnyType": false,
    "isDefault": false,
    "simpleType": false,
    "primitiveType": false,
    "isMap": false,
    "isArray": false,
    "isBinary": false,
    "isFile": false,
    "isNull": false,
    "schema": {
      "$ref": "#/components/schemas/Bar"
    },
    "jsonSchema": "{\n  \"description\" : \"Bar created\",\n  \"content\" : {\n    \"application/json\" : {\n      \"schema\" : {\n        \"$ref\" : \"#/components/schemas/Bar\"\n      }\n    }\n  }\n}",
    "vendorExtensions": {},
    "uniqueItems": false,
    "exclusiveMinimum": false,
    "exclusiveMaximum": false,
    "vars": [],
    "requiredVars": [],
    "hasValidation": false,
    "additionalPropertiesIsAnyType": false,
    "hasVars": false,
    "hasRequired": false,
    "hasDiscriminatorWithNonEmptyMapping": false,
    "hasMultipleTypes": false,
    "content": {
      "application/json": {
        "schema": {
          "openApiType": "Bar",
          "baseName": "SchemaFor200ResponseBodyApplicationJson",
          "complexType": "Bar",
          "getter": "getSchemaFor200ResponseBodyApplicationJson",
          "setter": "setSchemaFor200ResponseBodyApplicationJson",
          "dataType": "Bar",
          "datatypeWithEnum": "Bar",
          "name": "schemaFor200ResponseBodyApplicationJson",
          "defaultValueWithParam": " = data.SchemaFor200ResponseBodyApplicationJson;",
          "baseType": "Bar",
          "example": "null",
          "jsonSchema": "{\n  \"$ref\" : \"#/components/schemas/Bar\"\n}",
          "exclusiveMinimum": false,
          "exclusiveMaximum": false,
          "required": false,
          "deprecated": false,
          "hasMoreNonReadOnly": false,
          "isPrimitiveType": false,
          "isModel": true,
          "isContainer": false,
          "isString": false,
          "isNumeric": false,
          "isInteger": false,
          "isShort": false,
          "isLong": false,
          "isUnboundedInteger": false,
          "isNumber": false,
          "isFloat": false,
          "isDouble": false,
          "isDecimal": false,
          "isByteArray": false,
          "isBinary": false,
          "isFile": false,
          "isBoolean": false,
          "isDate": false,
          "isDateTime": false,
          "isUuid": false,
          "isUri": false,
          "isEmail": false,
          "isNull": false,
          "isFreeFormObject": false,
          "isAnyType": false,
          "isArray": false,
          "isMap": false,
          "isEnum": false,
          "isInnerEnum": false,
          "isReadOnly": false,
          "isWriteOnly": false,
          "isNullable": false,
          "isSelfReference": false,
          "isCircularReference": false,
          "isDiscriminator": false,
          "vars": [],
          "requiredVars": [],
          "vendorExtensions": {},
          "hasValidation": false,
          "isInherited": false,
          "nameInCamelCase": "SchemaFor200ResponseBodyApplicationJson",
          "nameInSnakeCase": "SCHEMA_FOR200_RESPONSE_BODY_APPLICATION_JSON",
          "uniqueItems": false,
          "isXmlAttribute": false,
          "isXmlWrapped": false,
          "additionalPropertiesIsAnyType": false,
          "hasVars": false,
          "hasRequired": false,
          "hasDiscriminatorWithNonEmptyMapping": false,
          "hasMultipleTypes": false,
          "ref": "#/components/schemas/Bar",
          "schemaIsFromAdditionalProperties": false,
          "datatype": "Bar",
          "iexclusiveMaximum": false,
          "hasItems": false
        },
        "testCases": {}
      }
    },
    "schemaIsFromAdditionalProperties": false,
    "range": false,
    "isPrimitiveType": false,
    "wildcard": false,
    "complexType": "Bar"
  }
]

ahmednfwela avatar Nov 26 '22 11:11 ahmednfwela

I kinda avoided the usual approach in order to utilize the existing models. In freezed we can't use existing data models to build a union. So as an alternative I map oneOf/anyOf as following.

@freezed
class MyModelUnion with _$MyModelUnion {
  const factory MyModelUnion.asObj1({
    required Obj1 obj1;
  }) = AsObj1;
  const factory MyModelUnion.asObj2({
    required Obj2 obj2;
  }) = AsObj2;
}

Then in the fromJson i use the discriminator to map it to the correct model. We use this approach in our project a lot and it works pretty good.

bahag-chandrana avatar Nov 26 '22 11:11 bahag-chandrana

Also anybody know how to map response code to response models.

they exist in the responses object for each operation e.g.

Cool, lemme check this model.

bahag-chandrana avatar Nov 26 '22 11:11 bahag-chandrana

well, I am not an expert in freezed, but If your approach works for all cases, it should be fine I guess

ahmednfwela avatar Nov 26 '22 11:11 ahmednfwela

It seems like the templates in this PR do not use formParams to generate a form body. The body object passed is null.

  /myapi/signin:
    post:
      summary: Generate a token on behalf of a user (without credentials)
      requestBody:
        required: true
        content:
          application/x-www-form-urlencoded:
            schema:
              type: object
              additionalProperties: false
              properties:
                actRole:
                  type: string
                actFor:
                  type: string
              required:
                - actRole
                - actFor

Notice the FormParams show up in the API signature (api.mustache)

  Future<Response<Signin>> signin({ 
    String role = 'SUPPORT',
    required String actRole,
    required String actFor,
    CancelToken? cancelToken,
    Map<String, dynamic>? headers,
    Map<String, dynamic>? extra,
    ValidateStatus? validateStatus,
    ProgressCallback? onSendProgress,
    ProgressCallback? onReceiveProgress,
  }) async {

However, the template for serializing parameters (freezed/api/serialize.mustache) does not deal with form params.

{{#bodyParam}}_bodyData=jsonEncode({{{paramName}}});{{/bodyParam}}

Resulting in a null _bodyData object sent in the request.

    dynamic _bodyData;

    try {

    } catch(error, stackTrace) {
      throw DioError(
         requestOptions: _options.compose(
          _dio.options,
          _path,
        ),
        type: DioErrorType.other,
        error: error,
      )..stackTrace = stackTrace;
    }

oravecz avatar Nov 29 '22 13:11 oravecz

I will have a look into this.

bahag-chandrana avatar Nov 30 '22 14:11 bahag-chandrana

I was able to handle form data using this as the serialize.mustache template. Unfortunately, I am new to Dart and Dio and not sure how to handle multipart form data (and file uploads).

{{#hasFormParams}}
  {{#isMultipart}}
    // Warning: multipart form data not really implemented in this template
  {{/isMultipart}}
  {{^isMultipart}}
    _bodyData = {
    {{#formParams}}
      "{{{paramName}}}": {{{paramName}}},
    {{/formParams}}
    };
  {{/isMultipart}}
{{/hasFormParams}}
{{#bodyParam}}
  {{#bodyParam}}_bodyData=jsonEncode({{{paramName}}});{{/bodyParam}}
{{/bodyParam}}

oravecz avatar Nov 30 '22 15:11 oravecz

@ahmednfwela @kuhnroyal I am trying access the mustache variable {{#operations}} in a file added via supportingFiles.add. Do you have any hints how I could access the operation in these mustache files. I see this work out of the box in api.mustache. Not sure what I am missing in the configuaration.

bahag-chandrana avatar Nov 30 '22 16:11 bahag-chandrana

try adding your file to apiTemplateFiles instead, e.g. https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractDartCodegen.java#L102

ahmednfwela avatar Dec 02 '22 04:12 ahmednfwela

try adding your file to apiTemplateFiles instead, e.g. https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractDartCodegen.java#L102

Thank you 👍. I did try it and it didn't work. I will give it a try again this weekend.

bahag-chandrana avatar Dec 02 '22 09:12 bahag-chandrana

try adding your file to apiTemplateFiles instead, e.g. https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractDartCodegen.java#L102

Thank you 👍. I did try it and it didn't work. I will give it a try again this weekend.

I was able to add a file to the apiTemplateFiles but this overwrites existing api client files. I am sure I am missing a configuration. Not sure what I am missing though 🥲 .

bahag-chandrana avatar Dec 05 '22 08:12 bahag-chandrana

Any update?

ykaito21 avatar Feb 19 '23 16:02 ykaito21

@ahmednfwela @kuhnroyal I have managed to generate freezed based models for all the oneOf/anyOf examples provided by @ahmednfwela. I have placed them in the following repository and it uses melos scripts to run the generator on each package using the jar built with my changes.

https://github.com/bahag-chandrana/test-json-serialize

Would appreciate if you could take a look into and provide some feedback.

I am currently looking into handling multipart requests which is available in built value but not handled in freezed properly as @oravecz pointed out. I hope I can come up with some basic fixes soon.

I am planning to post pone the implementation of union based response types to a different PR.

bahag-chandrana avatar Feb 26 '23 10:02 bahag-chandrana

@oravecz i have provided a basic fix for handling form data as well. Could you double check from your side if it looks fine. thanks in advance.

bahag-chandrana avatar Feb 27 '23 07:02 bahag-chandrana

Thanks for your great effort! I will take a look when I have the time

ahmednfwela avatar Feb 27 '23 09:02 ahmednfwela

Hi All,

I have added a response code based union response which is now behind a flag freezedUnionResponse. This only works when serialization library is freezed. The samples are now available in this repository under here.

The main purpose of this feature is to enable developers to handle all the response code defined in the spec in an exclusive manner.

https://github.com/bahag-chandrana/test-json-serialize/tree/main/packages/union_response_clients

A sample for pet store response is as shown below.

Sample defitnition

  "/pet/{petId}":
    get:
      tags:
        - pet
      summary: Find pet by ID
      description: Returns a single pet
      operationId: getPetById
      parameters:
        - name: petId
          in: path
          description: ID of pet to return
          required: true
          schema:
            type: integer
            format: int64
      responses:
        "200":
          description: successful operation
          content:
            application/xml:
              schema:
                $ref: "#/components/schemas/Pet"
            application/json:
              schema:
                $ref: "#/components/schemas/Pet"
        "400":
          description: Invalid ID supplied
        "404":
          description: Pet not found
Sample response model
@freezed
class GetPetByIdData with _$GetPetByIdData {
  const GetPetByIdData._();
  const factory GetPetByIdData.as200({
    required Pet responseData,
  }) = GetPetByIdDataAs200;
  const factory GetPetByIdData.as400({
    required Object? responseData,
  }) = GetPetByIdDataAs400;
  const factory GetPetByIdData.as404({
    required Object? responseData,
  }) = GetPetByIdDataAs404;
  const factory GetPetByIdData.unknown({
    int? statusCode,
    required Object? responseData,
  }) = GetPetByIdResponseUnknown;

  /// Converts the incoming response into the correct response code based freezed union case.
  static GetPetByIdData toUnionData(int? statusCode, Object? responseData) {
    switch (statusCode) {
      case 200:
        Pet _responseData;
        _responseData = Pet.fromJson(responseData as Map<String, dynamic>);

        return GetPetByIdData.as200(responseData: _responseData);
      case 400:
        Object? _responseData;
        _responseData = responseData;

        return GetPetByIdData.as400(responseData: _responseData);
      case 404:
        Object? _responseData;
        _responseData = responseData;

        return GetPetByIdData.as404(responseData: _responseData);
      default:
        return GetPetByIdData.unknown(
          responseData: responseData,
        );
    }
  }
}
Sample api client method
Future<Response<GetPetByIdData>> getPetById({
    required int petId,
    CancelToken? cancelToken,
    Map<String, dynamic>? headers,
    Map<String, dynamic>? extra,
    ValidateStatus? validateStatus,
    ProgressCallback? onSendProgress,
    ProgressCallback? onReceiveProgress,
  }) async {
    final _path =
        r'/pet/{petId}'.replaceAll('{' r'petId' '}', petId.toString());
    final _options = Options(
      method: r'GET',
      headers: <String, dynamic>{
        ...?headers,
      },
      extra: <String, dynamic>{
        'secure': <Map<String, String>>[
          {
            'type': 'apiKey',
            'name': 'api_key',
            'keyName': 'api_key',
            'where': 'header',
          },
        ],
        ...?extra,
      },
      validateStatus: validateStatus,
    );

    final _response = await _dio.request<Object>(
      _path,
      options: _options,
      cancelToken: cancelToken,
      onSendProgress: onSendProgress,
      onReceiveProgress: onReceiveProgress,
    );

    GetPetByIdData _responseData;

    try {
      _responseData = GetPetByIdData.toUnionData(
        _response.statusCode,
        _response.data,
      );
    } catch (error, stackTrace) {
      throw DioError(
        requestOptions: _response.requestOptions,
        response: _response,
        type: DioErrorType.unknown,
        error: error,
        stackTrace: stackTrace,
      );
    }

    return Response<GetPetByIdData>(
      data: _responseData,
      headers: _response.headers,
      isRedirect: _response.isRedirect,
      requestOptions: _response.requestOptions,
      redirects: _response.redirects,
      statusCode: _response.statusCode,
      statusMessage: _response.statusMessage,
      extra: _response.extra,
    );
  }

bahag-chandrana avatar Mar 19 '23 10:03 bahag-chandrana

@kuhnroyal @ahmednfwela can you help me progress with this PR.

bahag-chandrana avatar May 03 '23 07:05 bahag-chandrana

sorry I have been very busy lately ,will review it today

ahmednfwela avatar May 03 '23 08:05 ahmednfwela

I am making a PR to unify the tests for all dart generators, since the current schema we use doesn't support one_of/any_of etc... and we have to copy/paste the test configs for every new serializer/generator this new PR will also prepare for a rework of https://github.com/OpenAPITools/openapi-generator/pull/14346 to support both dio and http library

ahmednfwela avatar May 05 '23 20:05 ahmednfwela