spring-data-redis icon indicating copy to clipboard operation
spring-data-redis copied to clipboard

feat: add custom NullValueSerializer initializer

Open AnneMayor opened this issue 1 year ago β€’ 7 comments

  • [x] You have read the Spring Data contribution guidelines.
  • [x] You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • [x] You submit test cases (unit or integration tests) that back your changes.
  • [ ] You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

resolved #2878

AnneMayor avatar May 11 '24 13:05 AnneMayor

@AnneMayor Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

pivotal-cla avatar May 11 '24 13:05 pivotal-cla

@AnneMayor Thank you for signing the Contributor License Agreement!

pivotal-cla avatar May 11 '24 14:05 pivotal-cla

I'm looking for another great solution and trying to apply it.

AnneMayor avatar May 12 '24 03:05 AnneMayor

Please, review it. Thanks :)

AnneMayor avatar May 12 '24 11:05 AnneMayor

Thanks for looking into this enhancement. Taking a step back from the actual thing we want to solve, we have a growing number of constructors (6 constructors already). Adding another one is weakening the design. I would suggest creating a builder that accepts ObjectMapper, JacksonObjectReader, JacksonObjectWriter, and classPropertyTypeName for typing information. Also, we should add registerNullValueSerializer (boolean) for more flexibility.

That way, we can hide customization in the builder and allow further configuration options without stretching the number of constructors.

Let me know whether this makes sense.

mp911de avatar May 13 '24 12:05 mp911de

@mp911de Thank you for the detailed review! I think that's a great idea too. I am gonna create a new Builder class as you suggested within this weekend:) Thanks πŸ‘

AnneMayor avatar May 13 '24 14:05 AnneMayor

@mp911de I created Builder class. Please, review it :D

AnneMayor avatar May 19 '24 11:05 AnneMayor

Thank you for your contribution. That's merged, polished, and backported now. You might be interested in the polishing commit 8357c7d88b4e7ee6d8bb73f1a38e602c11ff758c that refines the builder API to accept parameter-by-parameter instead of requiring all elements upfront to ease with usage experience. Let me know if you have any questions.

mp911de avatar May 21 '24 13:05 mp911de

Polishing commit https://github.com/spring-projects/spring-data-redis/commit/8357c7d88b4e7ee6d8bb73f1a38e602c11ff758c is really awsome πŸ‘

Thank you for kind&rapid review! (I'm friend of @AnneMayor and helping her to contribute on open source πŸ˜ƒ)

injae-kim avatar May 21 '24 13:05 injae-kim

@mp911de , Thank you so much for your kind and rapid review 😊 I am very happy to contribute this open source. I am going to look into your polishing commit and learn from it. Again, thanks a lot πŸ‘ @injae-kim he is a great supporter to help me to contribute. Without him, I might be unable to merge the PR so fast. Thank you for your kind support, @injae-kim πŸ‘ You're the best!

AnneMayor avatar May 21 '24 13:05 AnneMayor