-
-
Notifications
You must be signed in to change notification settings - Fork 114
When using GraphQL federation, external parties can extend types and … #866
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
Conversation
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.
Overall the idea and implementation look ok to me. Please address the requested changes and also check-code-style issues.
Thank you!
plugins/gradle/example-server/gradle/wrapper/gradle-wrapper.properties
Outdated
Show resolved
Hide resolved
src/main/java/com/kobylynskyi/graphql/codegen/mapper/InputDefinitionToDataModelMapper.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kobylynskyi/graphql/codegen/mapper/TypeDefinitionToDataModelMapper.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kobylynskyi/graphql/codegen/model/MappingConfig.java
Outdated
Show resolved
Hide resolved
…inputs from the initial schema. Consequently, the server might receive queries and mutations that contains more fields than the ones originally generated. This changes introduces two new options: | Option | Data Type | Default value | Description | | :---------------------------------------------------: | :-------------------------------------------------------------------: | :----------------------------------------------------: | ----------- | | `supportUnknownFields` | Boolean | false | Specifies whether api classes should support unknown fields during serialization or deserialization. If `true`, classes will include a property of type [`java.util.Map<String,Object>`](https://docs.oracle.com/javase/8/docs/api/index.html?java/util/Map.html) that will store unknown fields.| | `unknownFieldsPropertyName` | String | userDefinedFields | Specifies the name of the property to be included in api classes to support unknown fields during serialization or deserialization| When `supportUnknownFields=true` a new catch-all field annotated with the [com.fasterxml.jackson.annotation.JsonAnySetter](https://fasterxml.github.io/jackson-annotations/javadoc/2.12/com/fasterxml/jackson/annotation/JsonAnySetter.html) and [com.fasterxml.jackson.annotation.JsonAnyGetter](https://fasterxml.github.io/jackson-annotations/javadoc/2.12/com/fasterxml/jackson/annotation/JsonAnyGetter.html) is added to generated code to serialize and deserialize unknown fields using a map. Changes were made to: * Codegen library - Java * Codegen library - Kotlin * Codegen library - Scala * Maven plugin * Gradle plugin * SBT plugin Please let me know if you agree with the changes and/or if I missed anything. Thank you in advance
2. Removed unecessary import statements 3. Externalized the creation of ParameterDefinition to utility interface com.kobylynskyi.graphql.codegen.mapper.UnknownFieldsSupport to avoid duplicated code
2. Fixing code-style issues
Co-authored-by: Kyle <[email protected]>
…inputs from the initial schema. Consequently, the server might receive queries and mutations that contains more fields than the ones originally generated. This changes introduces two new options: | Option | Data Type | Default value | Description | | :---------------------------------------------------: | :-------------------------------------------------------------------: | :----------------------------------------------------: | ----------- | | `supportUnknownFields` | Boolean | false | Specifies whether api classes should support unknown fields during serialization or deserialization. If `true`, classes will include a property of type [`java.util.Map<String,Object>`](https://docs.oracle.com/javase/8/docs/api/index.html?java/util/Map.html) that will store unknown fields.| | `unknownFieldsPropertyName` | String | userDefinedFields | Specifies the name of the property to be included in api classes to support unknown fields during serialization or deserialization| When `supportUnknownFields=true` a new catch-all field annotated with the [com.fasterxml.jackson.annotation.JsonAnySetter](https://fasterxml.github.io/jackson-annotations/javadoc/2.12/com/fasterxml/jackson/annotation/JsonAnySetter.html) and [com.fasterxml.jackson.annotation.JsonAnyGetter](https://fasterxml.github.io/jackson-annotations/javadoc/2.12/com/fasterxml/jackson/annotation/JsonAnyGetter.html) is added to generated code to serialize and deserialize unknown fields using a map. Changes were made to: * Codegen library - Java * Codegen library - Kotlin * Codegen library - Scala * Maven plugin * Gradle plugin * SBT plugin Please let me know if you agree with the changes and/or if I missed anything. Thank you in advance
2. Removed unecessary import statements 3. Externalized the creation of ParameterDefinition to utility interface com.kobylynskyi.graphql.codegen.mapper.UnknownFieldsSupport to avoid duplicated code
2. Fixing code-style issues
src/main/java/com/kobylynskyi/graphql/codegen/mapper/TypeDefinitionToDataModelMapper.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kobylynskyi/graphql/codegen/mapper/UnknownFieldsSupport.java
Outdated
Show resolved
Hide resolved
This id done to avoid compilation errors for clients that do not use this feature and do not have jackson dependencies in their classpath
|
@aldib I've addressed some of the changes in your branch, please review my changes and let me know if you have any questions. |
|
@kobylynskyi Thank you for the changes. I fixed the unit tests that were failing after the rebase, and I'm happy to merge if you are. Thanks. |
|
@kobylynskyi I am not sure what to do about the failed sonar job. Could you please advise on how to solve the problem? Thanks |
Description
When using GraphQL federation, external parties can extend types and inputs from the initial schema. Consequently, the server might receive queries and mutations that contains more fields than the ones originally generated.
This changes introduces two new options:
supportUnknownFieldstrue, classes will include a property of typejava.util.Map<String,Object>that will store unknown fields.unknownFieldsPropertyNameWhen
supportUnknownFields=truea new catch-all field annotated with the com.fasterxml.jackson.annotation.JsonAnySetter and com.fasterxml.jackson.annotation.JsonAnyGetter is added to generated code to serialize and deserialize unknown fields using a map.Please let me know if you agree with the changes and/or if I missed anything.
Thank you in advance
Changes were made to: