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

Allow for customisation of ObjectMapper in Jackson2ExecutionContextStringSerializer [BATCH-2828] #786

Closed
spring-issuemaster opened this issue Jul 9, 2019 · 6 comments

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Jul 9, 2019

Matt Byrne opened BATCH-2828 and commented

Link from StackOverflow post: https://stackoverflow.com/a/56949526/945789

 

There is currently no way to use the current configuration of ObjectMapper in Jackson2ExecutionContextStringSerializer and also provide customisations without copying and pasting code. There was a fix for https://jira.spring.io/browse/BATCH-2680 which resulted in a private jackson module being created: JobParametersModule and being included in the default ObjectMapper setup, however we cannot get access to that if we would like to customise ObjectMapper features.

I'd like to be able to register additional modules, namely KotlinModule so I can use data class for objects stored in execution context without having to mark everything as an optional field. I have seen other posts where people would also like to customise objectMapper.

Ideally it would be nice to declaratively register additional ObjectMapper features for boot, or allow us to pass in a base ObjectMapper (the default configured one in boot for example) that can then be copied internally with customisations. If that's not so easy then making the objectMapper property protected to allow subclasses to reference would be nice, or even increasing the visibility of JobParametersModule.

Any help and suggestions greatly appreciated!

 

 

 

 

 


Affects: 4.1.2

Issue Links:

  • BATCH-2838 Jackson2ExecutionContextStringSerializer fails when deserializing java.time.LocalDate
    ("is duplicated by")

Backported to: 4.2.0.RC1

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 9, 2019

Mahmoud Ben Hassine commented

Matt Byrne That's a valid point. Thank you for opening this issue.

The public setter for ObjectMapper can be updated to register the JobParametersModule like this:

public void setObjectMapper(ObjectMapper objectMapper) {
    Assert.notNull(objectMapper, "ObjectMapper must not be null");
    this.objectMapper = objectMapper; // custom options and modules registered by the user
    this.objectMapper.registerModule(new JobParametersModule());
}

This would allow the user to provide a custom ObjectMapper without the need to make JobParametersModule public (As a user, I should not be aware of this internal module and even if it was public, it is not my job to register it manually to make things work).

Wdyt?

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 9, 2019

Matt Byrne commented

Hey Mahmoud Ben Hassine - yes that would be a valid solution. This was what I was saying as one of the options where we pass a base objectMapper "copy it and customise" using objectMapper.copy(). The only danger with doing what you are doing on what appears to be a standard settter is that people may end up passing the default objectMapper bean that spring boot creates and having unexpected side-effects in getting their objectMapper changed. I think there are plenty of other spring projects where you can pass that objectMapper around safely. Just a thought ... maybe a method name could indicate what's happening? Not sure.

 

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 9, 2019

Matt Byrne commented

Also it would be nice if the autoconfiguration of all of this could use the default spring boot objectmapper somehow and just do what we're talking about (with option to override). That also seems pretty consistent with what other spring auto configs do. This is a nice to have though ... maybe a separate issue ;)

 

Thanks for speedy reply btw

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 9, 2019

Matt Byrne commented

Ignore my request for the autoconfigure, but would still be nice if the setter does a copy before customising.

 

I now realise why you don't inject the default objectmapper by default. Because batch serialises and deserialises to db you need to maintain consistency in objectmapper to avoid issues between versions. Using Spring Boot's objectmapper by default would potentially bring unexpected changes and cause deserialisation issues

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 27, 2019

Mahmoud Ben Hassine commented

Exactly, you nailed it! I agree on doing a objectMapper.copy() in the setter instead of this.objectMapper = objectMapper.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Sep 4, 2019

Mahmoud Ben Hassine commented

Fixed in a15a6bf

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

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.