Skip to content

Fix customization of JsonReadFeatures.#304

Merged
kqarryzada merged 1 commit into
masterfrom
DS-51680-fix-JsonReadFeature
Jul 10, 2026
Merged

Fix customization of JsonReadFeatures.#304
kqarryzada merged 1 commit into
masterfrom
DS-51680-fix-JsonReadFeature

Conversation

@kqarryzada

Copy link
Copy Markdown
Collaborator

This commit fixes an issue where setting a custom Jackson JsonReadFeature property was not reflected by the SCIM SDK's JsonMapper.

Reviewer: dougbulkley
Reviewer: vyhhuang

JiraIssue: DS-51680

This commit fixes an issue where setting a custom Jackson
JsonReadFeature property was not reflected by the SCIM SDK's
JsonMapper.

Reviewer: dougbulkley
Reviewer: vyhhuang

JiraIssue: DS-51680
@kqarryzada kqarryzada self-assigned this Jul 10, 2026

@vyhhuang vyhhuang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look good to me.

@dougbulkley dougbulkley left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any experience with JsonReadFeature class. Wondering why you only test the ALLOW_SINGLE_QUOTES constant and not the other ones?

@kqarryzada

Copy link
Copy Markdown
Collaborator Author

@dougbulkley, this is really because most features aren't as relevant for filter processing. For example, JsonReadFeature.ALLOW_TRAILING_COMMA doesn't work since we explicitly don't allow that in a filter expression anyway. I suppose the numeric handling ones do fit, though it arguably doesn't add substantial coverage and could be harder to maintain if such parameters are deprecated by Jackson in the future.

@kqarryzada
kqarryzada merged commit b41bedd into master Jul 10, 2026
6 checks passed
@kqarryzada
kqarryzada deleted the DS-51680-fix-JsonReadFeature branch July 10, 2026 20:29
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.

3 participants