Support customization of the MapperFactory #228
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixed a deprecation warning from an old method provided by Jackson.
Previously, the fields of the MapperFactory were static. This effectively allowed only a single instance of the object, and also made it impossible to override the createObjectMapper() method with personal customizations like adding serializers/deserializers. The SCIM SDK already treated MapperFactory instances as if they were not static, so we will now permit flexible customization. The documentation makes it clear that the superclass's object mapper must be fetched first for optimal behavior. Reviewer: vyhhuang Reviewer: dougbulkley JiraIssue: DS-49067
kqarryzada
commented
Jul 19, 2024
| var objectMapperBuilder = JsonMapper.builder(new ScimJsonFactory()); | ||
|
|
||
| // Do not care about case when de-serializing POJOs. | ||
| objectMapperBuilder.enable(ACCEPT_CASE_INSENSITIVE_PROPERTIES); |
There was a problem hiding this comment.
The original mapper.configure() call for this property (which I've now deleted) was marked as deprecated by Jackson, so I've updated the logic to instead set the property on the builder class instead. I'm not sure why other setting types (e.g., deserialization features) are okay with setting this directly on an object mapper, but it seems to be what the library encourages.
| @@ -68,7 +69,8 @@ protected ScimJsonFactory(@NotNull final ScimJsonFactory sourceFactory, | |||
| JsonParser createScimFilterParser(@NotNull final Reader r) | |||
| throws IOException | |||
| { | |||
| IOContext ctxt = _createContext(r, false); | |||
| ContentReference reference = ContentReference.construct(true, r); | |||
| IOContext ctxt = _createContext(reference, false); | |||
There was a problem hiding this comment.
This is an unrelated change that resolves another deprecation warning. I've kept this in a separate commit.
dougbulkley
approved these changes
Jul 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the fields of the MapperFactory were static. This
effectively allowed only a single instance of the object, and also made
it impossible to override the createObjectMapper() method with personal
customizations like adding serializers/deserializers. The SCIM SDK
already treated MapperFactory instances as if they were not static, so
we will now permit flexible customization. The documentation makes it
clear that the superclass's object mapper must be fetched first for
optimal behavior.
Reviewer: vyhhuang
Reviewer: dougbulkley
JiraIssue: DS-49067
Resolves #147