INT-4543: JacksonJsonObjectMapper: fix toJsonNode #2592
Merged
Conversation
This file contains hidden or 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
garyrussell
suggested changes
Oct 9, 2018
| @@ -137,7 +157,8 @@ protected JavaType extractJavaType(Map<String, Object> javaTypes) throws Excepti | |||
| JavaType contentClassType = this.createJavaType(javaTypes, JsonHeaders.CONTENT_TYPE_ID); | |||
| if (classType.getKeyType() == null) { | |||
| return this.objectMapper.getTypeFactory() | |||
| .constructCollectionType((Class<? extends Collection<?>>) classType.getRawClass(), contentClassType); | |||
| .constructCollectionType((Class<? extends Collection<?>>) classType | |||
| .getRawClass(), contentClassType); | |||
There was a problem hiding this comment.
More IDEA weirdness - did you upgrade again?
There was a problem hiding this comment.
Well, I get this option since I moved to new Mac Book.
So, it is independent of the upgrade.
I'll switch off an option to wrap if longer then 120.
There was a problem hiding this comment.
It's something else...
return this.objectMapper.getTypeFactory()
.constructCollectionType((Class<? extends Collection<?>>) classType.getRawClass(),
contentClassType);
is wrapped at 102 so I don't know why it split at 87; maybe wrapping at 90?
There was a problem hiding this comment.
Well, original line for me in IDEA looks like this:
So, it is longer than 120, therefore during reformatting IDEA decides to wrap it on the last . assuming it is a method call. That's what I see in other cases you pointed, too.
I don't think it's a big deal to have a discussion now. And unfortunately I don't see how to turn it off now 😢
garyrussell
reviewed
Oct 9, 2018
| public JsonNode toJsonNode(Object json) throws Exception { | ||
| if (json instanceof String) { | ||
| return this.objectMapper.readTree((String) json); | ||
| } |
There was a problem hiding this comment.
It's not as simple as this; if it's a simple String (rather than a String containing JSON), we need to encode foo as "foo".
org.springframework.integration.transformer.MessageTransformationException: failed to transform message; nested exception is com.fasterxml.jackson.core.JsonParseException: Unrecognized token 'foo': was expecting 'null', 'true', 'false' or NaN
at [Source: (String)"foo"; line: 1, column: 7], failedMessage=GenericMessage [payload=foo, headers={id=de53795e-2ab8-e5a0-032a-b20b38700a8e, timestamp=1539113912065}]
at org.springframework.integration.transformer.AbstractTransformer.transform(AbstractTransformer.java:44)
at org.springframework.integration.json.ObjectToJsonTransformerTests.testJsonStringAndJsonNode(ObjectToJsonTransformerTests.java:241)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
at org.junit.vintage.engine.execution.RunnerExecutor.execute(RunnerExecutor.java:39)
at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
at java.util.Iterator.forEachRemaining(Iterator.java:116)
at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
at org.junit.vintage.engine.VintageTestEngine.executeAllChildren(VintageTestEngine.java:79)
at org.junit.vintage.engine.VintageTestEngine.execute(VintageTestEngine.java:70)
at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:220)
at org.junit.platform.launcher.core.DefaultLauncher.lambda$execute$6(DefaultLauncher.java:188)
at org.junit.platform.launcher.core.DefaultLauncher.withInterceptedStreams(DefaultLauncher.java:202)
at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:181)
at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:128)
at org.eclipse.jdt.internal.junit5.runner.JUnit5TestReference.run(JUnit5TestReference.java:89)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:541)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:763)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:463)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:209)
Caused by: com.fasterxml.jackson.core.JsonParseException: Unrecognized token 'foo': was expecting 'null', 'true', 'false' or NaN
at [Source: (String)"foo"; line: 1, column: 7]
at com.fasterxml.jackson.core.JsonParser._constructError(JsonParser.java:1804)
at com.fasterxml.jackson.core.base.ParserMinimalBase._reportError(ParserMinimalBase.java:679)
at com.fasterxml.jackson.core.json.ReaderBasedJsonParser._reportInvalidToken(ReaderBasedJsonParser.java:2835)
at com.fasterxml.jackson.core.json.ReaderBasedJsonParser._reportInvalidToken(ReaderBasedJsonParser.java:2813)
at com.fasterxml.jackson.core.json.ReaderBasedJsonParser._matchToken2(ReaderBasedJsonParser.java:2620)
at com.fasterxml.jackson.core.json.ReaderBasedJsonParser._matchToken(ReaderBasedJsonParser.java:2598)
at com.fasterxml.jackson.core.json.ReaderBasedJsonParser._matchFalse(ReaderBasedJsonParser.java:2572)
at com.fasterxml.jackson.core.json.ReaderBasedJsonParser.nextToken(ReaderBasedJsonParser.java:719)
at com.fasterxml.jackson.databind.ObjectMapper._readTreeAndClose(ObjectMapper.java:4042)
at com.fasterxml.jackson.databind.ObjectMapper.readTree(ObjectMapper.java:2551)
at org.springframework.integration.support.json.Jackson2JsonObjectMapper.toJsonNode(Jackson2JsonObjectMapper.java:96)
at org.springframework.integration.support.json.Jackson2JsonObjectMapper.toJsonNode(Jackson2JsonObjectMapper.java:1)
at org.springframework.integration.json.ObjectToJsonTransformer.buildJsonPayload(ObjectToJsonTransformer.java:147)
at org.springframework.integration.json.ObjectToJsonTransformer.doTransform(ObjectToJsonTransformer.java:115)
at org.springframework.integration.transformer.AbstractTransformer.transform(AbstractTransformer.java:33)
... 44 more
There was a problem hiding this comment.
Somehow this was dropped from my review.
|
DO NOT MERGE YET. We shouldn't fallback to the |
artembilan
changed the title
JIRA: https://jira.spring.io/browse/INT-4543 When inbound payload is type of `String`, `byte[]`, `File`, `URL`, `InputStream` or `Reader`, we have to use an `ObjectMapper.readTree()` function. For all other types the `valueToTree()` should be used as a fallback **Cherry-pick to 5.0.x**
artembilan
changed the title
|
Ready for review. Thanks |
garyrussell
approved these changes
Oct 10, 2018
garyrussell
pushed a commit
that referenced
this pull request
Oct 10, 2018
* INT-4543: JacksonJsonObjectMapper: fix toJsonNode JIRA: https://jira.spring.io/browse/INT-4543 When inbound payload is type of `String`, `byte[]`, `File`, `URL`, `InputStream` or `Reader`, we have to use an `ObjectMapper.readTree()` function. For all other types the `valueToTree()` should be used as a fallback **Cherry-pick to 5.0.x** * * Fallback to the `valueToTree()` if not valid JSON * * Fallback to `valueToTree()` only for `String` and `byte[]`
|
Cherry-picked as 5874ae8 after resolving test conflicts. |
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.
JIRA: https://jira.spring.io/browse/INT-4543
When inbound payload is type of
String,byte[],File,URL,InputStreamorReader, we have to use anObjectMapper.readTree()function.
For all other types the
valueToTree()should be used as a fallbackCherry-pick to 5.0.x