Skip to content

feat: allow configuring a custom JsonMapper #1160

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

Closed

Conversation

sonallux
Copy link

@sonallux sonallux commented Jul 11, 2024

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
Copy link
Contributor

olegz commented Aug 8, 2024

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
Copy link
Author

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
Copy link
Contributor

olegz commented Aug 15, 2024

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
Copy link
Author

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.

@sonallux sonallux closed this Aug 16, 2024
@Xyaren
Copy link

Xyaren commented Sep 3, 2024

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

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

Successfully merging this pull request may close these issues.

Custom ObjectMapper is not used JsonMapper bean is not used
3 participants