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

Make it impossible to pass as input some type which was generated only to preserve forwards compatibility but was not meant to be used as input #5796

Open
StylianosGakis opened this issue Apr 10, 2024 · 2 comments

Comments

@StylianosGakis
Copy link
Contributor

Use case

For a schema which contains these

type Mutation {
    someMutation(input: SomeInput!): Irrelevant!
}

input SomeInput {
    otherField: ID!
    enumInput: SomeEnum
}

enum SomeEnum {
    FOO,
    BAR,
}

Apollo-kotlin generates this code for SomeEnum:

public enum class SomeEnum(
  public val rawValue: String,
) {
  FOO("FOO"),
  BAR("BAR"),
  /**
   * Auto generated constant for unknown enum values
   */
  UNKNOWN__("UNKNOWN__"),
  ;

  public companion object {
    public val type: EnumType = EnumType("SomeEnum", listOf("FOO", "BAR"))

    public fun safeValueOf(rawValue: String): ChatMessageContext =
        values().find { it.rawValue == rawValue } ?: UNKNOWN__

    /**
     * Returns all [SomeEnum] known at compile time
     */
    public fun knownValues(): Array<SomeEnum> = arrayOf(
      FOO,
      BAR,
    )
  }
}

This makes it possible in user code to try and pass to a mutation this:

apolloClient.mutation(SomeMutation(input = SomeInput(otherField = ..., enumInput = SomeEnum.UNKNOWN__))

This however will always fail on the backend validation since this entry is not part of the real schema, and there is nothing to map it to.

Describe the solution you'd like

The problem here is just that this is possible and the compiler is not warning us about trying to run something that would never work.

Optimally, this code should not compile in the first place.
If this turns out to be way too complicated, even a lint warning or something similar might be a good first step. But as of right now making this mistake is unfortunately something that can happen silently.

@martinbonnin
Copy link
Contributor

For the record, a possible solution is to use sealedClassesForEnumsMatching.set(listOf(".*")) (probably make it the default in v4) and have the UNKNOWN__(rawValue) constructor be annotated with an @OptIn annotation so that it's harder to call from user code.

@BoD BoD added ✔️ Fixed in SNAPSHOTs The fix has been merged and is available in SNAPSHOTs, and will be available in the next release ✅ Fixed in v4 and removed ✅ Fixed in v4 labels Apr 16, 2024
@BoD
Copy link
Contributor

BoD commented Apr 16, 2024

Heads up that we added the @OptIn annotation to the unknown constructor in #5813. This should be available in the v4 snapshots shortly!

For now we haven't decided on making sealed classes for enums the default in v4.

@BoD BoD added ✅ Fixed in v4 and removed ✔️ Fixed in SNAPSHOTs The fix has been merged and is available in SNAPSHOTs, and will be available in the next release labels Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants