Add Jsonb support #9648

eddumelendez · 2017-07-01T01:14:32Z

Spring Framework 5 will support Jsonb as a HttpMessageConverter, this
commit adds auto-configuration support. Also, support for Jsonb in the
@jsontest has been added.

This implementation is running against Yasson (RI for Jsonb).

snicoll

Wow, thanks for the pr @eddumelendez! Looks good and quite complete. I've made some comments.

In particular, I am not sure that the conditions part is mature. If you want us to brainstorm on this let me know!

snicoll · 2017-07-27T17:00:48Z

...boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json

@@ -572,12 +572,16 @@
  },
  {
    "name": "spring.http.converters.preferred-json-mapper",
+    "defaultValue" : "jackson",


This area is the hints section, we can't see the default value there.

snicoll · 2017-07-27T17:01:18Z

spring-boot-autoconfigure/pom.xml

@@ -724,6 +734,12 @@
 			<scope>test</scope>
 		</dependency>
 		<dependency>
+			<groupId>org.eclipse</groupId>
+			<artifactId>yasson</artifactId>
+			<version>${javax-jsonb.version}</version>


That version shouldn't be there.

snicoll · 2017-07-27T17:03:11Z

...java/org/springframework/boot/autoconfigure/http/GsonHttpMessageConvertersConfiguration.java

@@ -41,7 +43,7 @@

 	@Configuration
 	@ConditionalOnBean(Gson.class)
-	@Conditional(PreferGsonOrMissingJacksonCondition.class)
+	@Conditional(PreferGsonOrMissingJacksonAndJsonbCondition.class)


What is the rationale behind this? Gson is to be used if both Jackson and JSonB are missing? With three choices, the combinations are becoming increasingly complex so I wonder if we shouldn't revisit this differently.

regarding to the conditional thing. Should I rewrite conditions with an SpringBootCondition implementation? Just to make sure, I would like to know that the current conditions are ok.

snicoll · 2017-07-27T17:03:27Z

...java/org/springframework/boot/autoconfigure/http/GsonHttpMessageConvertersConfiguration.java

 		static class JacksonMissing {

 		}

+		@ConditionalOnProperty(name = HttpMessageConvertersAutoConfiguration.PREFERRED_MAPPER_PROPERTY, havingValue = "jsonb")


That particular one looks like a smell to me. What were you trying to express?

snicoll · 2017-07-27T17:04:20Z

...ava/org/springframework/boot/autoconfigure/http/JsonbHttpMessageConvertersConfiguration.java

+
+	}
+
+	private static class JacksonAndGsonMissing extends NoneNestedConditions {


Ditto. We should probably have a specific condition implementation for this

snicoll · 2017-07-27T17:05:30Z

spring-boot-test-autoconfigure/pom.xml

@@ -209,6 +219,12 @@
 			<scope>test</scope>
 		</dependency>
 		<dependency>
+			<groupId>org.eclipse</groupId>
+			<artifactId>yasson</artifactId>
+			<version>${javax-jsonb.version}</version>


Ditto. This version shouldn't be there.

snicoll · 2017-07-27T17:06:32Z

spring-boot-test/pom.xml

@@ -138,6 +148,12 @@
 			<scope>test</scope>
 		</dependency>
 		<dependency>
+			<groupId>org.eclipse</groupId>
+			<artifactId>yasson</artifactId>
+			<version>${javax-jsonb.version}</version>


Ditto for the version.

jhoeller · 2017-07-27T18:12:11Z

Note that Yasson itself is not a complete provider: that is, "org.eclipse:yasson:1.0" requires a manual extra declaration of "org.glassfish:javax.json:1.1" as the underlying JSON Processing provider. That's probably because Yasson is meant to be added to a Java EE server setup which comes with a JSON Processing provider out of the box.

By contrast, Apache Johnzon's "org.apache.johnzon:johnzon-jsonb:1.1.1" is complete on its own, which is why we are using it for our JSON Binding API tests in the Spring Framework test suite. I have no experience with it otherwise though, so I can't make any judgement about its production capabilities or its benefits over Yasson in general.

eddumelendez · 2017-07-28T16:43:47Z

Thanks for feedback @snicoll and @jhoeller . I am working in small changes and I'll be back to discuss about condition topic.

I just add org.apache.johnzon:johnzon-jsonb:1.1.1 dependency and the following test fails AbstractJsonMarshalTesterTests#parseMapShouldReturnContent

javax.json.bind.JsonbException: ObjectConverters are only supported for Classes not Types

	at org.apache.johnzon.jsonb.JohnsonJsonb.fromJson(JohnsonJsonb.java:160)
	at org.springframework.boot.test.json.JsonbTester.readObject(JsonbTester.java:89)
	at org.springframework.boot.test.json.AbstractJsonMarshalTester.read(AbstractJsonMarshalTester.java:298)
	at org.springframework.boot.test.json.AbstractJsonMarshalTester.parse(AbstractJsonMarshalTester.java:178)
	at org.springframework.boot.test.json.AbstractJsonMarshalTesterTests.parseMapShouldReturnContent(AbstractJsonMarshalTesterTests.java:177)
	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.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
	at org.junit.rules.ExpectedException$ExpectedExceptionStatement.evaluate(ExpectedException.java:239)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	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 com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Caused by: org.apache.johnzon.mapper.MapperException: ObjectConverters are only supported for Classes not Types
	at org.apache.johnzon.mapper.MappingParserImpl.buildObject(MappingParserImpl.java:203)
	at org.apache.johnzon.mapper.MappingParserImpl.readObject(MappingParserImpl.java:135)
	at org.apache.johnzon.mapper.MappingParserImpl.readObject(MappingParserImpl.java:127)
	at org.apache.johnzon.mapper.MappingParserImpl.readObject(MappingParserImpl.java:117)
	at org.apache.johnzon.mapper.Mapper.mapObject(Mapper.java:236)
	at org.apache.johnzon.mapper.Mapper.readObject(Mapper.java:187)
	at org.apache.johnzon.jsonb.JohnsonJsonb.fromJson(JohnsonJsonb.java:154)
	... 29 more

eddumelendez · 2017-07-30T02:51:37Z

In regards to the previous comment, issue JOHNZON-132 was created and fix has been applied and it is scheduled for org.apache.johnzon:johnzon-jsonb:1.1.2. I will move from yasson to johnzon once the release is done.

@jsontest

Spring Framework 5 will support Jsonb as a HttpMessageConverter, this commit adds auto-configuration support. Also, support for Jsonb in the @jsontest has been added. This implementation is running against Yasson (RI for Jsonb).

snicoll · 2017-09-21T13:13:36Z

spring-boot-test-autoconfigure/src/main/resources/META-INF/spring.factories

@@ -123,6 +126,7 @@ org.springframework.boot.autoconfigure.gson.GsonAutoConfiguration,\
 org.springframework.boot.autoconfigure.hateoas.HypermediaAutoConfiguration,\
 org.springframework.boot.autoconfigure.http.HttpMessageConvertersAutoConfiguration,\
 org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration,\
+org.springframework.boot.autoconfigure.jsonb.JsonbAutoConfiguration.\


@jsontest

Spring Framework 5 will support Jsonb as a HttpMessageConverter, this commit adds auto-configuration support. Also, support for Jsonb in the @jsontest has been added. This implementation is running against Apache Johnzon See gh-9648

* pr/9648: Polish "Add Jsonb support" Add Jsonb support

snicoll · 2017-09-21T13:17:43Z

Thanks @eddumelendez! Merged with some polish in 8d63b74

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 1, 2017

eddumelendez force-pushed the jsonb branch from e9d25bc to ff69dd5 Compare July 3, 2017 04:57

philwebb added this to the 2.0.0.M4 milestone Jul 5, 2017

philwebb added priority: normal type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 5, 2017

snicoll requested changes Jul 27, 2017

View reviewed changes

wilkinsona modified the milestones: 2.0.0.M4, 2.0.0.M5 Jul 28, 2017

eddumelendez added 6 commits August 6, 2017 11:32

Add Jsonb support

bf4e710

Spring Framework 5 will support Jsonb as a HttpMessageConverter, this commit adds auto-configuration support. Also, support for Jsonb in the @jsontest has been added. This implementation is running against Yasson (RI for Jsonb).

Move yasson version to spring-boot-dependencies/pom.xml

17dcc17

Remove defaultValue

8446f48

Fix typo

bb36711

Polish

de63cc0

Move from yasson to johnzon

091bad7

eddumelendez force-pushed the jsonb branch from ff69dd5 to 091bad7 Compare August 6, 2017 18:28

Add javax.json:javax.json-api

c66456f

snicoll reviewed Sep 21, 2017

View reviewed changes

snicoll self-assigned this Sep 21, 2017

snicoll closed this in 8d63b74 Sep 21, 2017

snicoll added a commit that referenced this pull request Sep 21, 2017

Merge pull request #9648 from eddumelendez:jsonb

2e01d90

* pr/9648: Polish "Add Jsonb support" Add Jsonb support

eddumelendez deleted the jsonb branch January 17, 2018 13:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add Jsonb support #9648

Add Jsonb support #9648

Uh oh!

eddumelendez commented Jul 1, 2017

Uh oh!

snicoll left a comment

Uh oh!

snicoll Jul 27, 2017

Uh oh!

snicoll Jul 27, 2017

Uh oh!

snicoll Jul 27, 2017 •

edited

Loading

Uh oh!

eddumelendez Aug 3, 2017

Uh oh!

snicoll Jul 27, 2017

Uh oh!

snicoll Jul 27, 2017

Uh oh!

snicoll Jul 27, 2017

Uh oh!

snicoll Jul 27, 2017

Uh oh!

jhoeller commented Jul 27, 2017

Uh oh!

eddumelendez commented Jul 28, 2017

Uh oh!

eddumelendez commented Jul 30, 2017 •

edited

Loading

Uh oh!

snicoll Sep 21, 2017

Uh oh!

snicoll commented Sep 21, 2017

Uh oh!

Uh oh!


		}

		private static class JacksonAndGsonMissing extends NoneNestedConditions {

Add Jsonb support #9648

Add Jsonb support #9648

Uh oh!

Conversation

eddumelendez commented Jul 1, 2017

Uh oh!

snicoll left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snicoll Jul 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhoeller commented Jul 27, 2017

Uh oh!

eddumelendez commented Jul 28, 2017

Uh oh!

eddumelendez commented Jul 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snicoll commented Sep 21, 2017

Uh oh!

Uh oh!

snicoll Jul 27, 2017 •

edited

Loading

eddumelendez commented Jul 30, 2017 •

edited

Loading