christiandeange / ozone

Kotlin Multiplatform bindings for Bluesky
MIT License
36 stars 6 forks source link

Generate sealed unions with an "unknown" type instead of enums, for string definitions with 'known values' #10

Open Emplexx opened 4 months ago

Emplexx commented 4 months ago

For example, the following definition:

"labelValue": {
      "type": "string",
      "knownValues": [
        "!hide",
        "!no-promote",
        "!warn",
        "!no-unauthenticated",
        "dmca-violation",
        "doxxing",
        "porn",
        "sexual",
        "nudity",
        "nsfl",
        "gore"
      ]
    }

currently produces this code:

public enum class LabelValue {
  @SerialName("!hide")
  HIDE,
  @SerialName("!no-promote")
  NO_PROMOTE,
  @SerialName("!warn")
  WARN,
  @SerialName("!no-unauthenticated")
  NO_UNAUTHENTICATED,
  @SerialName("dmca-violation")
  DMCA_VIOLATION,
  @SerialName("doxxing")
  DOXXING,
  @SerialName("porn")
  PORN,
  @SerialName("sexual")
  SEXUAL,
  @SerialName("nudity")
  NUDITY,
  @SerialName("nsfl")
  NSFL,
  @SerialName("gore")
  GORE,
}

Instead, in my opinion, it would be better for it to produce something like the following:

sealed class LabelValue(open val raw: String) {
    data object HIDE : LabelValue("!hide")
    data object NO_PROMOTE : LabelValue("!no-promote")
    data object WARN : LabelValue("!warn")
    // ...more known values here

    data class __UNKNOWN(override val raw: String) : LabelValue(raw)
}

Aside from containing all the known values, it would also contain an "unknown" type, which would be used as a fallback when deserializing responses in case an unknown value is detected, and as a way to retrieve and use such value later in code.

From the ATProto lexicon docs:

  • knownValues (array of strings, options: a set of suggested or common values for this field. Values are not limited to this set (aka, not a closed enum).
  • enum (array of strings, optional): a closed set of allowed values

which suggests that it is an error to make definitions with knownValues into closed enums.

This becomes an issue, for example, when sending a request to app.bsky.labeler.getServices (for details about labellers), since it will result in a SerializationException if a labeller defines a custom label value: com.atproto.label.LabelValue does not contain element with name 'sexual-figurative' at path $.views[0].policies.labelValues[5]

Emplexx commented 2 days ago

653074225e2ad142ba8a128549be989313c2bc9d fixes a SerializationException being thrown since there is now a fallback value, however another related issue (which I only now realise I should've explicitly mentioned in the issue description, my apologies) is that there still wouldn't be a way to get the underlying "unknown" string value, which can be important;

For example, the request app.bsky.labeler.getServices mentioned before returns a list of LabelValue enums, the name of which you need to get the definition of the label (also returned by that request). This is impossible to do if the value falls back to "unknown" instead of preserving the actual value.

I went ahead and tried to implement this myself (https://github.com/Emplexx/ozone/commit/2f28d2bfd404f4b275701ea5e540ad0118342e57), I tested it with a real response and it seems to work both ways, serializing and deserializing. Should I open a pull request?