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

[REQ][dart] Dart full OAS3 support #8179

Open
3 of 9 tasks
kuhnroyal opened this issue Dec 14, 2020 · 18 comments
Open
3 of 9 tasks

[REQ][dart] Dart full OAS3 support #8179

kuhnroyal opened this issue Dec 14, 2020 · 18 comments

Comments

@kuhnroyal
Copy link
Contributor

kuhnroyal commented Dec 14, 2020

This is an epic to track remaining work for full dart/dart-dio OAS3 support.

General:

Dart:

  • There are problems regarding deserialization of maps, de-serialization code is trying to call Map.mapFromJson. There are open issues for this and the problem is now evident in the generated fake petstore.
    Related issues: [BUG][Dart] Map.mapFromJson is generated, but there's no such method on map. #8029
  • De-serialization of maps of maps e.g. Map<String, Map<String, String>>
  • De-serialization of maps of enums e.g. Map<String, Enum>
  • De-serialization of list of list of model e.g. List<List<ReadOnlyFirst>>
  • Support enums of type double, de-serialization via switch statement doesn't work here
  • De-serialization of List<Object> tries to call Object.mapFromJson
  • OAS3 fake petstore contains a model called Client, this conflict with http.Client (addressed by [dart][dart-dio] Prevent name clashes with existing dart types #8198)

Dart-Dio:

@auto-labeler
Copy link

auto-labeler bot commented Dec 14, 2020

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@kuhnroyal
Copy link
Contributor Author

@agilob FYI if you have time to tackle some of the Dart issues

@agilob
Copy link
Contributor

agilob commented Dec 14, 2020

Might have more time in 2 weeks during Xmas break, but can't promise it.

@kuhnroyal
Copy link
Contributor Author

Dart-dio now compiles with full OAS3 fake petstore.

@Grohden
Copy link
Contributor

Grohden commented Dec 18, 2020

I'm curious, is dart generator working for you all using the current templates?
I had to fix a lot of cast issues from json conversions for the dart generator, here's my files if it help...
https://gist.github.com/Grohden/1fd8d110c806d8b239e88d07847c1e5a

But still I'm not able to even compile the project, there's still some issues with enum names :/

Edit: I've updated my gist, dart analyzer is kinda happy now, but I haven't tested it on runtime yet.
Edit 2: too soon, dart analyzer lied to me.

@wing328
Copy link
Member

wing328 commented Dec 18, 2020

@Grohden are you using the latest master to generate the Dart API client? There's been many enhancements, bug fixes to the Dart generators

@Grohden
Copy link
Contributor

Grohden commented Dec 18, 2020

@wing328 I'm manually copying the current master dart templates.
But I'm still on the latest released beta (must be on the beta3). Is there any way to use the master 'release' without cloning and building it myself? (a wget url)

@wing328
Copy link
Member

wing328 commented Dec 18, 2020

You can find the snapshot version of the latest master in the project's README

@kuhnroyal
Copy link
Contributor Author

@Grohden you can not only copy the templates, that is not enough, there are many changes in the generators as well.

The problems regarding json serialization are listed as todos.

@kuhnroyal
Copy link
Contributor Author

@Grohden If you have specific solutions to the some of the json mapping problems, please don't hesitate to post them here. The dart generator is not my primary focus so any help is appreciated.

@kuhnroyal
Copy link
Contributor Author

I have updated the known compile errors at the top.

@Grohden
Copy link
Contributor

Grohden commented Dec 18, 2020

@kuhnroyal I've downloaded the latest master (this project could suggest a clone with depth=1 btw) and built the package.. it fixed some issues (inline enum probably), but still it had problems, so I fixed them for myself.

Here's my commit Grohden@72c8e15, maybe I will open a PR later... but all those fixes were for my project and I can't guarantee that all those casts will work (I've tested a single method in runtime and it worked)

In any case the project wasn't even compiling without them, so it's better than nothing.

Edit: had to ammend a change in the commit, I've update its URL

@kuhnroyal
Copy link
Contributor Author

What Dart version are you using? Curious why all the casts are necessary.

@Grohden
Copy link
Contributor

Grohden commented Dec 19, 2020

Using dart 2.11.0-213.1.beta,

Hmm, so you're not using dart analyzer? or is using it without implicit-casts:false?
https://dart.dev/guides/language/analysis-options#enabling-additional-type-checks

I always use dart analyzer with strong mode, and I've generated my api client 'inside' (as a package) my project, so it's probably inheriting my rules... I'm also not sure that if you're using strong mode it applies to all libs used.

if you generate the petstore example and enable the strong mode with a file in the root called analysis_options.yaml with this:

analyzer:
  strong-mode:
    implicit-casts: false

you probably gonna see the same compile errors as I.. but I'm not sure if that's desirable 🤔

@Grohden
Copy link
Contributor

Grohden commented Dec 19, 2020

I've tested a few things:

  • Running master

    • Generating a package OUTSIDE of my project folder and then importing it with path in yaml
      • Map method error, does not compile
    • Generating a package INSIDE of my project folder and then importing it with path in yaml
      • Map method error, does not compile
      • Not sure if the method error hides the cast issues in the generated package.. but probably not 🤔
  • Running my branch

    • Generating a package OUTSIDE of my project folder and then importing it with path in yaml
      • Compiles and work
    • Generating a package INSIDE of my project folder and then importing it with path in yaml
      • Compiles and work
    • Generating the package and copying lib content to some lib/folder of my project
      • Compiles and work
      • Since this will be considered as 'my project sources' it will generate some warnings based on my lint rules, that is expected.

My branch probably fixes these:

  • There are problems regarding deserialization of maps, de-serialization code is trying to call Map.mapFromJson. There are open issues for this and the problem is now evident in the generated fake petstore.
  • De-serialization of maps of maps e.g. Map<String, Map<String, String>>
  • [maybe?] De-serialization of maps of enums e.g. Map<String, Enum>
  • [maybe?] De-serialization of list of list of model e.g. List<List>
  • [maybe?] Support enums of type double, de-serialization via switch statement doesn't work here
  • [maybe?] De-serialization of List tries to call Object.mapFromJson
  • OAS3 fake petstore contains a model called Client, this conflict with http.Client

  • If all of those issues were found by trying to compile the fake petstore they're probably fixed in my brach as I've also tested it against the fake petstore api..

    In any case, as I've said, those fixes were for my project, so if I open a PR I cannot guarantee that it will work for all cases..

@kuhnroyal
Copy link
Contributor Author

I always use strict-mode but I just recently started helping out here. I opened #8231 yesterday but thats just for dart-dio.
Some of the casts you have in your changes don't really make sense to me on Dart 2.9 even with implicit-dynamics: false thats why I am wondering.

@Grohden
Copy link
Contributor

Grohden commented Dec 19, 2020

@kuhnroyal u mean dynamics to a "more specialized dynamic"? (like dynamic to Map<String, dynamic> or dynamic to String)
these are for the implicit cast:false, a implicit cast to a more specialized version is forbidden...
I'm gonna open a PR soon so we can discuss better my changes

@kuhnroyal
Copy link
Contributor Author

I know, I have a branch with this but the casts are not all needed with Dart 2.9

analyzer:
  language:
    strict-inference: true
    strict-raw-types: true
  strong-mode:
    implicit-dynamic: false

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants