PolymorphicJsonAdapterFactory withDefaultSubtype and withFallbackSubtype support

[DeweyReed]

Issue #756(How to handle new polymorphic label in backward compatible way?) is folded into #739(PolymorphicJsonAdapterFactory could have a fallback class like with enum) which is closed by the PR #778.

The newly added method withDefaultValue falls back to default value(like an error message) but can't fall back to a default type(or label).

Code example:

now we can do this

PolymorphicJsonAdapterFactory.of(Weather.class, "type")
        .withSubtype(Sunny.class, "sunny")
        .withSubtype(Rain.class, "rain")
        .withDefaultValue(Sunny(...))

we can't do this without breaking backward compatility

PolymorphicJsonAdapterFactory.of(Weather.class, "type")
        .withSubtype(Sunny.class, "sunny")
        .withSubtype(Rain.class, "rain")
        .withDefaultSubtype(Rain.class)
        // or this
        //.withDefaultLabel("sunny")

[NightlyNexus]

withDefaultSubtype sounds good to me. open to a PR, unless anyone has other concerns.

[ZacSweers]

I worry this would be kind of a footguns since it assumes they all have compatible data fields for the default subtype. I've always just seen unknown fallbacks used with basically like a singleton "unknown" instance to signal it was unrecognized, otherwise you could have problems trying to deserialized unknown data and hoping it works

[NightlyNexus]

abstract class PromoBanner {
  String title;
  int background_color;

  static final class ItemPromoBanner {
    String message;
  }

  static final class CollectionPromoBanner {
    int count;
  }

  static final class DefaultUnknownPromoBanner {
  }
}

I imagine a case like this, where I can still use the some of the dynamic data that I know about (title, background_color) from decoding the unknown type DefaultUnknownPromoBanner.

[alex-tavella]

Hey guys, do you have any plan on having this? It would be really useful for a scenario kinda similar to the one @NightlyNexus pointed out. .withDefaultValue(value) won't be suffice to us since we want to send analytics events on failing types (e.g. every occurrence of DefaultUnknownPromoBanner with its labelkey).

[ZacSweers]

For the example given, you can just make a singleton instance of DefaultUnknownPromoBanner and provide that. If it's not decoding anything, there's not much different between having an instance vs asking moshi to reflectively deserialize nothing. The API overhead of different fallback strategies is a bit gnarly if we just add custom types, so if we add more in this space I think it's far more likely that we'd add some sort of flexible callback like so:

.withUnknownHandler { label: String, reader: JsonReader, moshi: Moshi -> 
 // Do whatever you want here, return a subtype of the base type though 
}

[alex-tavella]

Thanks for the fast reply, let me clarify the scenario a bit more.
In our case, we have a setup that looks like this:

sealed class Card(
  open val cardType: String
)

data class BannerCard(
  val banner: BannerData
) : Card("banner")

data class SmallBannerCard(
  val smallBanner: SmallBannerData
) : Card("smallbanner")

data class SimpleHeaderCard(
  val title: String,
  val subtitle: String
) : Card("simpleheader")

object CardAdapterFactory {
  fun get() = 
       PolymorphicJsonAdapterFactory.of(Card::class.java, "cardType")
            .withSubtype(BannerCard::class.java, "banner")
            .withSubtype(SmallBannerCard::class.java, "smallbanner")
            .withSubtype(SimpleHeaderCard::class.java, "simpleheader")
}

In addition we want to handle the fallback scenario by logging the data we don't "understand". So, ideally we want to have an UnknownCard similar to:

data class UnknownCard(
  override val cardType: String
) : Card(cardType)

Having it as an object wouldn't suffice for us since we wouldn't have the label key cardType deserialized so current api doesn't work for us.

Having said all that, I agree with you that the api could get gnarly with different fallback strategies, although something like .withUnknownHandler { labelKey -> would suffice (maybe moshi and reader instances are not even needed).

Edit: disconsider what I said on .withUnknownHandler api, moshi and reader are useful to delegate parsing:

.withUnknownHandler { _, moshi, reader -> moshi.adapter(UnknownCard::class.java).fromJson(reader) }

[macisamuele]

I'm wondering if it would be "just" simpler to provide an entry point that allow to pass a JsonAdapter in the flow encoding/decoding flow.
The passed adapter will be used only in case the label is unknown or the class is not managed by
PolymorphicJsonAdapterFactory.

This should provide enough flexibility to developers as well as letting developers working with moshi building-blocks already familiar to them.

The interface that I have in mind is something like

object CardAdapterFactory {
  fun get() = 
       PolymorphicJsonAdapterFactory.of(Card::class.java, "cardType")
            .withSubtype(BannerCard::class.java, "banner")
            .withSubtype(SmallBannerCard::class.java, "smallbanner")
            .withSubtype(SimpleHeaderCard::class.java, "simpleheader")
            .withFallbackJsonAdapter(object: JsonAdapter<Card> { ... })
}

---

I ended up here as I was looking for a workaround to the JsonDataException exception during decoding for Yelp/swagger-gradle-codegen#103

[ZacSweers]

Looks like this can be closed now 👍