Improves calculateTypes: adds returnTypes, recurses into map value ty… #3286
Conversation
…pes and generate Kotlin test classes through Wire.
| @@ -50,6 +54,10 @@ internal class MiskWebFormBuilderTest { | |||
| ) | |||
| ) | |||
|
|
|||
| // Check map value types | |||
There was a problem hiding this comment.
this isnt really testing the map type right? SHould we assert on the pretty print Map<K,V> .... like the enum example
There was a problem hiding this comment.
So we are calculating the types from Shipment (L27), this assertion tests that we recursed into Warehouse, and then into the map value type (Robot from map<int32, Robot> robots = 7;), correctly.
There was a problem hiding this comment.
If your point is we didn't check annotations/type on this robots field itself (i.e., as a type inside Warehouse), that is correct. I will add it now for thoroughness.
|
This PR is temporarily blocked (due to item 2 in description) by the unexpected Wire behavior described in this Slack thread. |
| @@ -50,6 +54,13 @@ internal class MiskWebFormBuilderTest { | |||
| ) | |||
| ) | |||
|
|
|||
| // Check map value types are included | |||
There was a problem hiding this comment.
Could you add assertions that the key type (in this case int32) is also present in the calculated types?
There was a problem hiding this comment.
Do to this I think we would need to change handleField a bit, so I will merge as is and we can work on it in a separate PR.
|
I wrote a |
* refs/heads/master: Move `develocity` block after `plugins` (#3321) chore: upgrade dependencies to close security vulnerabilities (#3323) Deprecate Config tab in admin dashboard menu (#3322) Add description to Metadata class, Add AlertMessage, CodeBlock, ToggleContainer components (#3316) Fix Metadata tab overflow scrolling (#3315) Add contract to requireRequest (#3318) Delete misk-events-testing which has been replaced by misk-events test fixtures (#3317) Add id to Metadata tab h1 (#3314) Allow dashboard menu links with query parameters (#3313) Refactor JvmMetadataAction into Metadata plugin (#3305) Fix ConfigMetadata prettyPrint to match Config tab (#3311) Make metadata classes internal (#3310) Add metadata for cron jobs (#3309) Add metadata example to exemplar, make backwards compat (#3306) Make AllMetadataAction filterable (#3303) # Conflicts: # misk-admin/src/test/kotlin/misk/web/metadata/all/AllMetadataActionTest.kt
|
Tested with Wire-5.0.0-alpha03 locally and working as expected. Merging. |
…pes and generate Kotlin test classes through Wire.
This PR
returnTypes(similar toresponseTypes) inWebActionMetadata. This is useful when the return of a web action is not wrapped (e.g., in aResponse).misk-adminand instead generates them with Gradle as needed. To do this, we duplicate the .proto definitions using different package names so that Wire can process both with different outputs.calculateTypessimilarly to how we parse Lists.