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

fix for #4082 #4083

Merged
merged 1 commit into from
Mar 26, 2019
Merged

fix for #4082 #4083

merged 1 commit into from
Mar 26, 2019

Conversation

senivam
Copy link
Contributor

@senivam senivam commented Mar 21, 2019

Fixes reported issue #4082

Signed-off-by: Maxim Nesen [email protected]

@senivam senivam requested a review from jansupol March 21, 2019 14:26
Copy link
Contributor

@jansupol jansupol left a comment

Choose a reason for hiding this comment

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

Please add a test that demonstrates the need for this change

@senivam
Copy link
Contributor Author

senivam commented Mar 21, 2019

I was thinking about that and I'm not sure that change is really needed. However it seems to be really weird behavior when framework modifies input from client which is not expected to be modified. And the only reason Jersey does this which I've found is that it tries to avoid NPE when cloning NULL map from input. As a result I've handled possible NULL map in some different way.

I've added tests which demonstrates 3 use-cases which can hapend - irrelevant input map (checked and map is still the same). Irrelevant immutable Map (same as previous but the map is explicitly immutable so if there is an attempt to modify it an exception is thrown), NULL input map - works as before (original functionality is kept).

Signed-off-by: Maxim Nesen <[email protected]>
Copy link
Member

@lukasj lukasj left a comment

Choose a reason for hiding this comment

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

LGTM

@senivam senivam merged commit 682f66c into eclipse-ee4j:master Mar 26, 2019
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.

None yet

3 participants