Skip to content

Conversation

alexcheng1982
Copy link
Contributor

FunctionCallbackWrapper has a Builder object to build FunctionCallbackWrappers. The Builder has a method withObjectMapper to pass in custom ObjectMappers. However, this custom ObjectMapper is not used in the constructor of FunctionCallbackWrapper, so FunctionCallbackWrapper always uses the default ObjectMapper.

new ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)

This causes problems when using Kotlin data classes as the function types. For serialization/deserialization with Kotlin data classes, the KotlinModule should be registered to the ObjectMapper, which requires a custom ObjectMapper. The workaround is using records.

In this fix, I added the ObjectMapper to the constructor of FunctionCallbackWrapper. No null-check is required as this constructor is private and Builder is the only way to create new instances of FunctionCallbackWrapper. Null-check is already done in the build method of Builder. The default value of ObjectMapper is the same as before, which is also already defined in Builder.

This allows custom ObjectMappers to be used when creating FunctionCallbackWrappers
@tzolov tzolov self-assigned this Apr 8, 2024
@tzolov tzolov added this to the 1.0.0-M1 milestone Apr 8, 2024
@tzolov
Copy link
Contributor

tzolov commented Apr 8, 2024

Thanks for the improvement @alexcheng1982

@tzolov
Copy link
Contributor

tzolov commented Apr 8, 2024

Rebased, squashed and merged at 0a3a202

in addition to the original commit make the default object mapper match the config of the ModelOptionsUtil.

@tzolov tzolov closed this Apr 8, 2024
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.

2 participants