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

[FEATURE] New factory API to control the type labelKey serialization. #1595

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hossain-khan
Copy link

@hossain-khan hossain-khan commented Dec 21, 2022

Summary

New API autoSerializeLabel(boolean) added to allow the factory can be configured with boolean flag to disable automatic serialization of the type label, and it's respective value. Here is how the configuration may look like in the factory builder.

PolymorphicJsonAdapterFactory.of(Message.class, "type")
    .withSubtype(Success.class, "success")
    .withSubtype(MessageWithType.class, "customType")
    .autoSerializeLabel(false)) // ← New API!

This is done for use cases when all known subtypes has serializable type property in the class definition, then it generates double type properties when serialized to JSON.

Background of the change

There are two key reason for requesting this feature (new API).

  1. In projects, sub-type classes can have type property which is serializable. Adding @Json(ignore = true) is not an option. That results in double type value in the serialized JSON object.
  2. There are some use cases where serializing object via parent type is not always applicable (eg Message in this example). When sub-type has type class property, it can be serialized directly without using polymorphic adapter (eg. MessageWithType in this example). Having this API would allow to have type property in class and allow it to play nicely with polymorphic adapter when deserializing as well.

✅ Signed "Square Inc. Individual Contributor License Agreement" for my GitHub username.

New API `autoSerializeLabel(boolean)` added to allow the factory can be configured with boolean flag to disable automatic serialization of the type label, and it's respective value. Here is how the configuration may look like in the factory builder.

```java
PolymorphicJsonAdapterFactory.of(Message.class, "type")
    .withSubtype(Success.class, "success")
    .withSubtype(MessageWithType.class, "customType")
    .autoSerializeLabel(false)) // ← New API!
```

This is done for use cases when all known subtypes has serializable type property in the class definition, then it generates double `type` properties when serialized to JSON.
@hossain-khan
Copy link
Author

🎄 Happy Holidays, and happy to answer any questions related to this feature request. 🙏🏽

@hossain-khan
Copy link
Author

I don't think the CI / Test Mode KSP failure is related to my code changes

org.gradle.api.ProjectConfigurationException: A problem occurred configuring project ':moshi-kotlin-codegen'.
Caused by: org.gradle.api.internal.artifacts.ivyservice.DefaultLenientConfiguration$ArtifactResolveException: Could not resolve all files for configuration ':moshi-kotlin-codegen:classpath'.
Caused by: org.gradle.api.resources.ResourceException: Could not get resource 'https://plugins.gradle.org/m2/gradle/plugin/com/github/jengelman/gradle/plugins/shadow/7.0.0/shadow-7.0.0.jar'.
Caused by: org.gradle.internal.resource.transport.http.HttpRequestException: Could not GET 'https://plugins-artifacts.gradle.org/gradle.plugin.com.github.jengelman.gradle.plugins/shadow/7.0.0/8e7b12a0a2c2d170884a860679a019bab60b63bc99ebfa0db0897d249d86f345/shadow-7.0.0.jar'.
Caused by: javax.net.ssl.SSLHandshakeException: Received fatal alert: handshake_failure

Could some please re-start the CI task when possible! Thanks

…onal-label

# Conflicts:
#	moshi-adapters/src/main/java/com/squareup/moshi/adapters/PolymorphicJsonAdapterFactory.kt
@waqasmoazzam
Copy link

Hey folks, I am having the same issue at our organization where it is showing duplicate keys in the serialized JSON object. I cannot change the domain models. Having this config would help avoid the duplication issue.

@JakeWharton is this something that can be considered for future release?

@hossain-khan
Copy link
Author

I think a good way to summarize this PR requests is mentioned here #181 (comment) in the video 🎥

You can watch Jesse and I talk about this phenomenon here: https://youtu.be/PCxz2LEmuL4?t=1643. We don't build kitchen sink libraries.

Not sure how to solve it in extensible way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants