Skip to content
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

[BUG] Properties marked nullable in swagger file are not nullable in generated output #506

Closed
point-source opened this issue Nov 28, 2022 · 10 comments · Fixed by #520
Closed
Assignees
Labels
bug Something isn't working

Comments

@point-source
Copy link

point-source commented Nov 28, 2022

Describe the bug
I have a property on line 18748 of this swagger file like so:

"limit": {
    "type": "number",
    "format": "double",
    "description": "For `credit`-type accounts, this represents the credit limit.\n\nFor `depository`-type accounts, this represents the pre-arranged overdraft limit, which is common for current (checking) accounts in Europe.\n\nIn North America, this field is typically only available for `credit`-type accounts.",
    "nullable": true
},

The generated output for this property is:

limit: (json['limit'] as num).toDouble(),

For reference, this is part of the AccountBalance model:

AccountBalance _$AccountBalanceFromJson(Map<String, dynamic> json) =>
    AccountBalance(
      available: (json['available'] as num).toDouble(),
      current: (json['current'] as num).toDouble(),
      limit: (json['limit'] as num).toDouble(),
      isoCurrencyCode: json['iso_currency_code'] as String,
      unofficialCurrencyCode: json['unofficial_currency_code'] as String,
      lastUpdatedDatetime: json['last_updated_datetime'] == null
          ? null
          : DateTime.parse(json['last_updated_datetime'] as String),
    );

When I try to deserialize the server response, it fails because Null is not a subtype of num.

Expected behavior
The generated output should allow the limit property (and other properties marked null) to be null.

Swagger specification link
Link to swagger/OpenApi documentation

Library version used:
master branch on commit b9ecb61 (v2.8.7)

Additional Info:
I suspect the issue might be on this line:
..isNullable = (json[kIsNullable] ?? json[kIsNullable] ?? false) as bool;

It appears to me like the intention was to handle both "x-nullable" and "nullable" cases but accidentally is checking "x-nullable" twice.

Patching this results in this output:

AccountBalance _$AccountBalanceFromJson(Map<String, dynamic> json) =>
    AccountBalance(
      available: (json['available'] as num?)?.toDouble(),
      current: (json['current'] as num?)?.toDouble(),
      limit: (json['limit'] as num?)?.toDouble(),
      isoCurrencyCode: json['iso_currency_code'] as String?,
      unofficialCurrencyCode: json['unofficial_currency_code'] as String?,
      lastUpdatedDatetime: json['last_updated_datetime'] == null
          ? null
          : DateTime.parse(json['last_updated_datetime'] as String),
    );

While this fixes the case that started this ticket, it did not fix a similar issue with the component on line 18570 as references from line 43202:

            "PlaidError": {
                "description": "We use standard HTTP response codes for success and failure notifications, and our errors are further classified by `error_type`. In general, 200 HTTP codes correspond to success, 40X codes are for developer- or user-related failures, and 50X codes are for Plaid-related issues. An Item with a non-`null` error object will only be part of an API response when calling `/item/get` to view Item status. Otherwise, error fields will be `null` if no error has occurred; if an error has occurred, an error code will be returned instead.",
                "type": "object",
                "additionalProperties": true,
                "title": "Error",
                "nullable": true,
                "properties": {
                    "error_type": {
                        "$ref": "#/components/schemas/PlaidErrorType"
                    },
...

Reference to the component:

                    "error": {
                        "$ref": "#/components/schemas/PlaidError"
                    },
@aledavidgueva
Copy link

Hi! I have the same problem starting from version 2.8.4... By the moment, i rollback to version 2.8.3

@Vovanella95
Copy link
Collaborator

@point-source Could you please share swagger of AccountBalance?

@Vovanella95
Copy link
Collaborator

@aledavidgueva could you please share Your swagger file?

@point-source
Copy link
Author

point-source commented Nov 29, 2022

@Vovanella95 I did. It's right there where it says "Link to Swagger/OpenApi documentation".

But here's another link just in case: https://raw.githubusercontent.com/plaid/plaid-openapi/master/2020-09-14.yml

I just converted it to json with a yaml to json converter online.

@Vovanella95
Copy link
Collaborator

@point-source cool, will take a look!

@point-source
Copy link
Author

point-source commented Nov 29, 2022

I fixed the first part of this bug here: point-source@cee0a10

As best I can tell, the second example in my post is failing because referenced models which have a nullable flag are not marked nullable. Line 684 of swagger_models_generator.dart should be handling this but the prop.isNullable flag is not properly set at the time that line is executed.

@fryette
Copy link
Contributor

fryette commented Dec 2, 2022

@point-source will you be able to provide a pull request with a fix?

@point-source
Copy link
Author

Sadly not. I was only able to fix the first issue but not the second. I don't understand this library well enough to close the entire ticket. I can still open a PR though if you want to pull in the bit that I did do. Perhaps with some guidance on where to look I can help with the rest?

@point-source
Copy link
Author

Is there any other way I can help with this?

@Vovanella95
Copy link
Collaborator

@point-source sorry no updates for now. Will take a look into it this week. Thanks for patience!

Vovanella95 added a commit that referenced this issue Dec 16, 2022
* Fixed baseUrl type (Issue #515)

* Fixed generation of nullable fields (Issue #506)

Co-authored-by: Uladzimir Paliukhovich <uladzimir_paliukhovich@epam.com>
Vovanella95 added a commit that referenced this issue Dec 16, 2022
* Fixed baseUrl type (Issue #515)

* Fixed generation of nullable fields (Issue #506)

* Fixed generation of @OptionalBody()

* Updated pubspec and changelog

Co-authored-by: Uladzimir Paliukhovich <uladzimir_paliukhovich@epam.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants