Netflix / dgs-codegen

Apache License 2.0
182 stars 99 forks source link

feature: add support for object input default values and add support for default inputs to kotlin2 #680

Closed dwilkolek closed 3 months ago

dwilkolek commented 5 months ago

closes #297

dwilkolek commented 5 months ago

Sure, I'll take a stab at it shortly.

dwilkolek commented 5 months ago

While peeking at java code path I've found a bug in kotlin path. When using typeMapping with Person to DifferentlyNamedClass mapping then it would fail.

Regarding Java path it will be slightly more difficult. In Kotlin path I used named parameters. In Java I see 4 paths:

  1. create json object and use ObjectMapper to create object in place
  2. use default no param constructor and call setters in initialiser block
  3. use constructor with arguments but it will require the same order and matching constructor for mapped types
  4. use builder - this would be great for generated classes, but will most likely fail for type mapped classes.
dwilkolek commented 5 months ago

I've implemented option 2. Lmk what do you think 🙏

dwilkolek commented 5 months ago

I realised that kotlin2 did not support default values at all. Now experience with default values should be the same across codegen configurations.

dwilkolek commented 4 months ago

@mbossenbroek This PR contains kotlin2 changes - added support for default values in input types 😉 @srinivasankavitha I've implemented java code path using initialiser block

If it make sense to you I can split changes into 2 PRs; one for object defaults, one adding kotlin2 default values

mbossenbroek commented 4 months ago

Looks good to me! 👍 I'll let @srinivasankavitha merge it cause there are other changes in there too

dwilkolek commented 3 months ago

@srinivasankavitha Are you waiting for any action from my side or is it ready to go? I'm not a huge fan of stale PRs hanging out in void 😅

srinivasankavitha commented 3 months ago

Sorry for the delay, and thanks for bringing this back to my attention. Just merged the PR and will go out in next week's release.