Skip to content
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

enable the use of a custom ObjectMapper #735

Closed

Conversation

jitokim
Copy link
Contributor

@jitokim jitokim commented Jul 7, 2024

fix: #723 using builder of JsonPulsarHeaderMapper.

AS-IS
JacksonUtils.enhancedObjectMapper() was always used.
JsonPulsarHeaderMapper.builder().build()

TO-BE
Inject ObjectMapper to builder in the constructor of AbstractPulsarMessageToSpringMessageAdapter
JsonPulsarHeaderMapper.builder().objectMapper(objectMapper).build()

By adding ObjectMapper to the constructor of AbstractPulsarMessageToSpringMessageAdapter, we enable the use of a custom ObjectMapper.

@jitokim jitokim changed the title add objectmapper to the constructor of MessageAdapter enable the use of a custom ObjectMapper Jul 7, 2024
@onobc
Copy link
Collaborator

onobc commented Jul 8, 2024

Thanks for the contribution @jitokim . The proposed changes would allow a user-provided object mapper only for Pulsar message headers JSON de/serialization. To allow a user-provided object mapper when de/serializing the message payload is a bit more complicated. I have a PR in progress to do this. What I will do is combine that PR w/ part of this PR. I will try to get something for review in the next few days. I will ping you when I do.

Thank you.

@jitokim
Copy link
Contributor Author

jitokim commented Jul 8, 2024

I agree that allowing a user-provided object mapper for the message payload is complicated
Thank you for the review 😃

@onobc
Copy link
Collaborator

onobc commented Jul 30, 2024

Hi @jitokim
I have used your initial code proposal as a starting point for #762.

  1. Thank you for the reminder to implement this for the JSON headers.
  2. Thank you for the great starting point.

You can see I tweaked it quite a bit, namely:

  • now looking up of the ObjectMapper bean later than afterSingletonInstantiated as that is a bit too early to get access to the mapper
  • lookup the mapper by special name pulsarHeaderObjectMapper
  • added tests for @PulsarReader @PulsarListener, @ReactivePulsarListener
  • added docs

I had intentions of keeping your original commit in my PR but it diverged enough that I did not go that route.
I would however like to give you the credit you deserve via a co-author on the new PR. If you agree can you reply here w/ the following info:

Closing in favor of #762

@onobc onobc closed this Jul 30, 2024
@jitokim
Copy link
Contributor Author

jitokim commented Jul 30, 2024

Hi, @onobc

Thank you so much for considering me and for the thoughtful acknowledgment.

For the information, I would like to use the following:
First name: Jihoon
Last name: Kim
Email address: [email protected]

I really appreciate the great open-source project you're creating.

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.

Add ability to use custom ObjectMapper for JSON schema
2 participants