-
Notifications
You must be signed in to change notification settings - Fork 381
BREAKING CHANGE: Upgrade to Jackson 3 #2162
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
Changes from all commits
4a04c41
33c5e30
a65ea98
b4589f0
d39b2d1
ce2b7d1
531c96a
71534c0
00946d1
d0d0882
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,24 +17,24 @@ | |
| package com.expediagroup.graphql.client.jackson.serializers | ||
|
|
||
| import com.expediagroup.graphql.client.jackson.types.OptionalInput | ||
| import com.fasterxml.jackson.core.JsonGenerator | ||
| import com.fasterxml.jackson.databind.JsonSerializer | ||
| import com.fasterxml.jackson.databind.SerializerProvider | ||
| import tools.jackson.core.JsonGenerator | ||
| import tools.jackson.databind.SerializationContext | ||
| import tools.jackson.databind.ValueSerializer | ||
|
|
||
| class OptionalInputSerializer : JsonSerializer<OptionalInput<*>>() { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| class OptionalInputSerializer : ValueSerializer<OptionalInput<*>>() { | ||
|
|
||
| override fun isEmpty(provider: SerializerProvider, value: OptionalInput<*>?): Boolean { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed, and the default name of the parameter is |
||
| override fun isEmpty(ctxt: SerializationContext, value: OptionalInput<*>?): Boolean { | ||
| return value == OptionalInput.Undefined | ||
| } | ||
|
|
||
| override fun serialize(value: OptionalInput<*>, gen: JsonGenerator, serializers: SerializerProvider) { | ||
| override fun serialize(value: OptionalInput<*>, gen: JsonGenerator, ctxt: SerializationContext) { | ||
| when (value) { | ||
| is OptionalInput.Undefined -> return | ||
| is OptionalInput.Defined -> { | ||
| if (value.value == null) { | ||
| serializers.defaultNullValueSerializer.serialize(value.value, gen, serializers) | ||
| ctxt.defaultNullValueSerializer.serialize(value.value, gen, ctxt) | ||
| } else { | ||
| serializers.findValueSerializer(value.value::class.java).serialize(value.value, gen, serializers) | ||
| ctxt.findValueSerializer(value.value::class.java).serialize(value.value, gen, ctxt) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,18 +32,22 @@ import com.expediagroup.graphql.client.jackson.types.JacksonGraphQLError | |
| import com.expediagroup.graphql.client.jackson.types.JacksonGraphQLResponse | ||
| import com.expediagroup.graphql.client.jackson.types.JacksonGraphQLSourceLocation | ||
| import com.expediagroup.graphql.client.jackson.types.OptionalInput | ||
| import com.fasterxml.jackson.databind.SerializationFeature | ||
| import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper | ||
| import org.junit.jupiter.api.Test | ||
| import tools.jackson.databind.SerializationFeature | ||
| import tools.jackson.module.kotlin.jacksonMapperBuilder | ||
| import java.util.UUID | ||
| import kotlin.test.assertEquals | ||
|
|
||
| class GraphQLClientJacksonSerializerTest { | ||
|
|
||
| private val testMapper = jacksonObjectMapper() | ||
| .enable(SerializationFeature.INDENT_OUTPUT) | ||
| private val testMapper = jacksonMapperBuilder().enable(SerializationFeature.INDENT_OUTPUT).build() | ||
| private val serializer = GraphQLClientJacksonSerializer(testMapper) | ||
|
|
||
| private fun assertSerializedJsonEquals(expected: String, actual: String) { | ||
| // Check the contents rather than the string order | ||
| assertEquals(testMapper.readTree(expected), testMapper.readTree(actual)) | ||
| } | ||
|
|
||
|
Comment on lines
+46
to
+50
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default ordering of the generated serialized objects changed https://github.com/FasterXML/jackson/blob/main/jackson3/MIGRATING_TO_JACKSON_3.md#changes-mapperfeature This test previously checked for exact equivalence, including ordering. It's sufficient to just check for functional equivalence. |
||
| @Test | ||
| fun `verify we can serialize GraphQLClientRequest`() { | ||
| val testQuery = FirstQuery(FirstQuery.Variables(input = 1.0f)) | ||
|
|
@@ -58,7 +62,7 @@ class GraphQLClientJacksonSerializerTest { | |
| """.trimMargin() | ||
|
|
||
| val serialized = serializer.serialize(testQuery) | ||
| assertEquals(expected, serialized) | ||
| assertSerializedJsonEquals(expected, serialized) | ||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -78,7 +82,7 @@ class GraphQLClientJacksonSerializerTest { | |
| """.trimMargin() | ||
|
|
||
| val serialized = serializer.serialize(queries) | ||
| assertEquals(expected, serialized) | ||
| assertSerializedJsonEquals(expected, serialized) | ||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -193,7 +197,7 @@ class GraphQLClientJacksonSerializerTest { | |
| """.trimMargin() | ||
|
|
||
| val serialized = serializer.serialize(scalarQuery) | ||
| assertEquals(expected, serialized) | ||
| assertSerializedJsonEquals(expected, serialized) | ||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -239,7 +243,7 @@ class GraphQLClientJacksonSerializerTest { | |
| """.trimMargin() | ||
|
|
||
| val serialized = serializer.serialize(query) | ||
| assertEquals(expected, serialized) | ||
| assertSerializedJsonEquals(expected, serialized) | ||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -276,7 +280,7 @@ class GraphQLClientJacksonSerializerTest { | |
| """.trimMargin() | ||
|
|
||
| val serialized = serializer.serialize(query) | ||
| assertEquals(expected, serialized) | ||
| assertSerializedJsonEquals(expected, serialized) | ||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -309,7 +313,7 @@ class GraphQLClientJacksonSerializerTest { | |
| |} | ||
| """.trimMargin() | ||
| val serialized = serializer.serialize(query) | ||
| assertEquals(expected, serialized) | ||
| assertSerializedJsonEquals(expected, serialized) | ||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -325,7 +329,7 @@ class GraphQLClientJacksonSerializerTest { | |
| |} | ||
| """.trimMargin() | ||
| val serialized = serializer.serialize(query) | ||
| assertEquals(expected, serialized) | ||
| assertSerializedJsonEquals(expected, serialized) | ||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -345,6 +349,6 @@ class GraphQLClientJacksonSerializerTest { | |
| """.trimMargin() | ||
|
|
||
| val serialized = serializer.serialize(entitiesQuery) | ||
| assertEquals(expected, serialized) | ||
| assertSerializedJsonEquals(expected, serialized) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,8 @@ | |
| package com.expediagroup.graphql.client.jackson.data.scalars | ||
|
|
||
| import com.expediagroup.graphql.client.converter.ScalarConverter | ||
| import com.fasterxml.jackson.databind.util.StdConverter | ||
| import com.fasterxml.jackson.annotation.JsonProperty | ||
| import tools.jackson.databind.util.StdConverter | ||
| import kotlin.Any | ||
|
|
||
| class AnyToAnyConverter : StdConverter<Any, Any>() { | ||
|
|
@@ -34,5 +35,6 @@ class AnyScalarConverter : ScalarConverter<Any> { | |
|
|
||
| // representation would not be part of the generated sources | ||
| data class ProductEntityRepresentation(val id: String) { | ||
| @get:JsonProperty("__typename") | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As part of the upgrade, this explicit annotation is now required. Previously it would be fine for it to be implicit. |
||
| val __typename: String = "Product" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,16 +18,15 @@ package com.expediagroup.graphql.client.jackson.serializers | |
|
|
||
| import com.expediagroup.graphql.client.jackson.types.OptionalInput | ||
| import com.fasterxml.jackson.annotation.JsonInclude | ||
| import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper | ||
| import org.junit.jupiter.api.Test | ||
| import tools.jackson.module.kotlin.jacksonMapperBuilder | ||
| import kotlin.test.assertEquals | ||
|
|
||
| class OptionalInputSerializerTest { | ||
|
|
||
| private val mapper = jacksonObjectMapper() | ||
| init { | ||
| mapper.setSerializationInclusion(JsonInclude.Include.NON_EMPTY) | ||
| } | ||
| private val mapper = jacksonMapperBuilder() | ||
| .changeDefaultPropertyInclusion { incl -> incl.withValueInclusion(JsonInclude.Include.NON_EMPTY) } | ||
| .build() | ||
|
Comment on lines
+27
to
+29
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This demonstrates the new builder pattern |
||
|
|
||
| @Test | ||
| fun `verify undefined value is serialized to empty JSON`() { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ObjectMapper objects are now immutable, so we need to rebuild it to apply the settings that we need.