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

[dart-dio] correctly handle Map<String, Object>, List<Object> using JsonObject #4401

Merged
merged 2 commits into from
Nov 17, 2019

Conversation

amondnet
Copy link
Contributor

@amondnet amondnet commented Nov 7, 2019

google/built_value.dart#134

dart-dio generator does not handle Map<String, dynamic> properly. This PR fixes this issue using JsonObject.

  • Map<String, Object> should be BuiltMap<String,JsonObject>
  • List<Object> should be BuiltList<JsonObject>
  Test:
    properties:
      objectMap:
        type: object
        additionalProperties:
          type: object
      objectList:
        type: array
        items:
          type: object
  @nullable
  @BuiltValueField(wireName: 'objectMap')
  BuiltMap<String, JsonObject> get objectMap;

  @nullable
  @BuiltValueField(wireName: 'objectList')
  BuiltList<JsonObject> get objectList;

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

CC: @ircecho @swipesight @jaumard @nickmeinhold @athornz

@nickmeinhold
Copy link
Contributor

Sounds great to me (though I would defer to @athornz if he has any issue).

I'm fairly new to the project but trying to be helpful where I can. One thing I'd love to do is encourage adding tests to PRs, especially where a fix addresses a specific issue and can be checked with some straightforward tests.

Any interest in adding a test or two? The dart generator has tests that are run in CI - currently it's just an old set of e2e tests with stubbed out network calls, and needs lots of work but it's a working example. We could copy the tests over to dart-dio and add running them to the CI script if you want.

And if no time or interest, no worries!

@wing328 wing328 added this to the 4.2.1 milestone Nov 12, 2019
@wing328 wing328 modified the milestones: 4.2.1, 4.2.2 Nov 15, 2019
@wing328 wing328 merged commit 78d7ffb into OpenAPITools:master Nov 17, 2019
@wing328 wing328 changed the title feat(dart-dio): correctly handle Map<String, Object>, List<Object> using JsonObject [dart-dio] correctly handle Map<String, Object>, List<Object> using JsonObject Nov 17, 2019
jimschubert added a commit to jimschubert/openapi-generator that referenced this pull request Nov 17, 2019
…ulti-level-model-hierarchy

* origin/master:
  minor fix to CI failure
  feat(dart-dio): correctly handle Map<String, Object>, List<Object> using JsonObject (OpenAPITools#4401)
  [OCAML] Fixes cloud.drone.io ocaml-test (OpenAPITools#4501)
  [elm] Add support for oneOf (OpenAPITools#4434)
  [BUG] [Java] Client resttemplate and webclient. Form Params are badly added when they are lists  (OpenAPITools#4461)
  fix: prevent ClassCastException when handling options of (issue OpenAPITools#4468) (OpenAPITools#4495)
  Fixes Python client Configuration class so each instance uses its own dicts (OpenAPITools#4485)
@wing328
Copy link
Member

wing328 commented Dec 2, 2019

@amondnet thanks for the PR, which has been included in the v4.2.2 release: https://twitter.com/oas_generator/status/1201432648544972800

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

Successfully merging this pull request may close these issues.

3 participants