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

JacksonTester handles some characters asymmetrically #16629

Closed

Conversation

Projects
None yet
5 participants
@dberrueta-atlassian
Copy link

commented Apr 24, 2019

This fixes #15727 by customising the json-path configuration for each {Jackson|Gson|BasicJson}Tester. It also adds integration tests.

This PR replaces #15735 . It is essentially a squashed version of that one.

/**
* Integration tests for {@link GsonTester}. Shows typical usage.
*
* @author Andy Wilkinson

This comment has been minimized.

Copy link
@dberrueta-atlassian

dberrueta-atlassian Apr 24, 2019

Author

@wilkinsona I hope you don't mind I attributed this file to you. You provided part of the code in #15735 (comment)

@@ -81,6 +83,28 @@ public void typicalMapTest() throws Exception {
.isEqualTo(1);
}

@Test
public void stringLiteral() throws Exception {

This comment has been minimized.

Copy link
@dberrueta-atlassian

dberrueta-atlassian Apr 24, 2019

Author

Same test than for Gson, just to confirm that there isn't a similar regression when using Jackson.

@dberrueta-atlassian

This comment has been minimized.

Copy link
Author

commented Apr 24, 2019

I took @wilkinsona 's suggestion from #15735 (comment) and in this new PR I limit the change just to JacksonTester, which was the only tester for which I had observed an asymmetric behaviour.

@wilkinsona wilkinsona changed the title Configure coordinated JSON provider in json-path. Fixes gh-15727 JacksonTester handles some characters asymmetrically Apr 24, 2019

@wilkinsona

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

Given that this is a bug and the fix appears to be reasonably low-risk, I'm tempted to fix this in 2.1.x. Flagging for team attention to see what everyone else thinks.

@wilkinsona wilkinsona modified the milestones: 2.2.x, 2.1.x Apr 24, 2019

@wilkinsona wilkinsona self-assigned this Apr 24, 2019

philwebb added a commit that referenced this pull request Apr 25, 2019

Use Jackson configuration with JsonPath
Update `JacksonTester` so that the JsonPath instance is explicitly configured
with both a `JacksonJsonProvider` and a `JacksonMappingProvider`.

Prior to this commit, the handling of special characters was not symmetrical
between the serialization (handled via the JacksonTester) and the parsing (handled
via JsonPath) due to the fact that JsonPath used `SimpleJson` as its parser.

See gh-16629

philwebb added a commit that referenced this pull request Apr 25, 2019

Polish "Use Jackson configuration with JsonPath"
Polish contribution to use a factory method in `AbstractJsonMarshalTester`
rather than additional constructor arguments.

Also change the `JsonContent` tests so that the `Configuration` constructor
is package private. This keeps JsonPath classes out of our public API, at
the expense of limiting custom JsonPath configurations to just our code.

See gh-16629

@philwebb philwebb closed this in 7302974 Apr 25, 2019

@philwebb philwebb modified the milestones: 2.1.x, 2.1.5 Apr 25, 2019

@philwebb

This comment has been minimized.

Copy link
Member

commented Apr 25, 2019

I forgot to comment here yesterday. This is now merged into 2.1.x and master along with a polish commit. Thanks very much @dberrueta-atlassian for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.