search

spring-cloud / spring-cloud-function

Apache License 2.0
1.04k stars 618 forks source link

feat: allow configuring a custom JsonMapper #1160

Closed sonallux closed 3 months ago

sonallux commented 4 months ago

This PR adjust the definition of the JsonMapper bean in the ContextFunctionCatalogAutoConfiguration to be conditional on missing bean, so a custom JsonMapper can be registered and used with spring-cloud-functions.

This PR fixes #1159 and fixes #1059.

olegz commented 3 months ago

I am trying to understand the purpose for this. JsonMapper is just a wrapper class over ObjectMapper or Gson, so don't you think it would be better to address it by injecting an instance of one of those? I know it can't be done now, but thinking that we could. Can you please elaborate about the problem you are trying to solve?

sonallux commented 3 months ago

I am trying to understand the purpose for this. JsonMapper is just a wrapper class over ObjectMapper or Gson, so don't you think it would be better to address it by injecting an instance of one of those? I know it can't be done now, but thinking that we could. Can you please elaborate about the problem you are trying to solve?

@olegz I just wanted to fix issue #1159 and it seemed reasonable to allow providing a custom JsonMapper bean. Yes allowing to inject the json implementation would be better. The other PR #1162 is implementing this behaviour.

olegz commented 3 months ago

The #1162 has been merged and I honestly prefer this approach, so please let me know if that's ok with you so I can close this

sonallux commented 3 months ago

The #1162 has been merged and I honestly prefer this approach, so please let me know if that's ok with you so I can close this

I am perfectly fine with this, so I am closing my PR.

Xyaren commented 2 months ago

I request this to be expedited. This issue is currently breaking non-default json serialization.