Got some crashes while trying to build a non-trivi...

# apollo-kotlin

[Stylianos Gakis]

Got some crashes while trying to build a non-trivial model using test-builders. More in thread 🧵

[Stylianos Gakis]

Maybe relevant information: I am using response-based code-gen and flattenModels.set(true).

The test I added looks something like this. Seems normal enough, but crashes, and running it with --debug shows an java.lang.ArithmeticException: / by zero from this line. It’s non obvious to me what I could’ve potentially done wrong. The last line that comes from the generated code looks something like:

public class DataBuilder1 : MapBuilder() {
  public var location: String by StubbedProperty(__map, "location")

  public override fun build(): Map<String, Any?> = mapOf(
    "location" to resolve("location", EmbarkExternalRedirectLocation.type.notNull()),
  )
}

where EmbarkExternalRedirectLocation is:

public enum class EmbarkExternalRedirectLocation(
  public val rawValue: String
) {
  MailingList("MailingList"),
  Offer("Offer"),
  Close("Close"),
  Chat("Chat"),
  /**
   * Auto generated constant for unknown enum values
   */
  UNKNOWN__("UNKNOWN__"),
  ;

  public companion object {
    public val type: EnumType = EnumType("EmbarkExternalRedirectLocation")

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

[Stylianos Gakis]

Not sure either how to debug this nor how to give more information about what is happening. If you know what I can do please tell me and I will help as much as I can. The repository is open sourced, and the “giraffe” service we use can be introspected as well so I guess cloning it could be one thing but if I can make it easier for you I’d be happy to!

[Stylianos Gakis]

On a side note: Interestingly, when actually going ahead and trying to add the value manually, I also see that even though the location in my schema is defined as the aforementioned Enum, I get to populate it as a normal string? Is that expected? It’d be nice to be able to have a type in there instead right? Changing it to a sealed class by adding it to sealedClassesForEnumsMatching doesn’t seem to change that either

[Stylianos Gakis]

And then got a __typename missing for the default case of a union type, fixed it here.
But after this commit, again, getting another division by 0 on resolving another enum. This feels like either I am messing up a lot in a lot of places, or I could try and file a bug after we discuss this here and we decide that something is indeed off.
Mainly for the division by 0 exceptions when dealing with enums that aren’t explicitly specified in my builder, but also potentially with the missing __typename that I don’t quite understand how is happening or if it should be happening.

[mbonnin]

Having to specify strings instead of enums and custom scalars feels weird indeed. That's because this is what the parsers do expect and we reuse the parsers to build the models

[mbonnin]

Not sure about the division by zero

[mbonnin]

Is there a commit I can checkout to reproduce the issue?

[Stylianos Gakis]

This should work

[Stylianos Gakis]

You may need to:
• Add a local.properties file with something similar to sdk.dir=/Users/stylianosgakis/Library/Android/sdk if opening from IntelliJ instead of AS
• Go inside the .graphqls file that’s automatically generated and search for the text “When we’ve deleted an item” and remove the extra string ticks \""" above and below that comment. It’s a known problem we have in our schema which is just very annoying and I don’t know how to fix xD

[Stylianos Gakis]

Then running ./gradlew :shared:cleanTestDebugUnitTest :shared:testDebugUnitTest --tests "com.hedvig.embarkx.embark.story.GetEmbarkStoryUseCaseTest.successfulRequestReturnsTheContainingData" --debug is the one that brings up the exception

[mbonnin]

(it's building)

[Stylianos Gakis]

For reference, this is a glance of what I am looking at from running this

[mbonnin]

Alright, this is a side effect of https://github.com/apollographql/apollo-kotlin/commit/a1536a269ffe5896f1b5290e55bd361e2d398861 that the test builders haven't been moved to using the generated scalar types (but still rely on CompiledStringType, etc...) Nope, it's just that default enums are not handled

[mbonnin]

It's a bit late here to dive into this but will look more tomorrow

[mbonnin]

Many thanks for sharing your use case, that helps a ton 🙏

[Stylianos Gakis]

Amazing, thanks a lot for taking the time to clone and everything!
For now I assume I'll just avoid using the test builder as I don't see a workaround to just make it compile without maybe specifying the entire object manually. If I can add anything tomorrow please feel free to ping me or whatever, I'd be happy to help if I can!

Super random side note: While we're at it and you saw that escaped comment bug we have in our graphqls schema, do you have any ideas how to fix that? Maybe like making a Gradle plugin which will intercept the file generation task and very specifically target that line of code? Anything easier?

[mbonnin]

I think (but I'll need to double check that) that the parser shouldn't choke on this (cf https://spec.graphql.org/draft/#BlockStringCharacter)

[mbonnin]

Easy fix is register a custom task to do the replacement

[mbonnin]

Something like

tasks.register("updateSchema") {
    dependsOn("downloadGiraffeSchemaFromIntrospection")
    inputs.file(downloadedSchema)
    outputs.file(patchedSchema)
    
    doLast { 
        patchedSchema.writeText(downloadedSchema.readText().replace("\\\"\"\"", ""))
    }
}

[Stylianos Gakis]

Yeah but as you probably saw when you tried to generate the models from the schema before changing that, it does actually choke unfortunately 😅
And wow I was about to say I've never really done anything in Gradle myself, and it sounds daunting, but this doesn't look that bad! I'll check it out first thing in the morning and report back on results 😅
I've stated this before but I just can't thank you enough for all the help you've been providing to me and others on this channel! Always a very pleasant experience being in this channel!

[mbonnin]

Sure thing, thanks a lot for all the feedback 🙂 ! It's always great to have real life use case, make sure all the unit tests match what's being used in real life 🙂

[mbonnin]

Calling it a day, will dig more tomorrow and update this thread

[mbonnin]

Fix there: https://github.com/apollographql/apollo-kotlin/pull/3894

[annsofi]

👋 I'm running into the same thing on 3.1.0, and being new to test builders I'm not sure if I've missed a step somewhere that I should do to handle this?
My case is similar except that I have a list of enums instead of setting it directly. Probably easier to see what I mean with some code (the problem is with the resolutions field):

public class ConsumptionAnalysisBuilder : MapBuilder() {
  public var valuesFrom: String? by StubbedProperty(__map, "valuesFrom")

  public var valuesTo: String? by StubbedProperty(__map, "valuesTo")

  public var resolutions: List<String> by StubbedProperty(__map, "resolutions")

  public override fun build(): Map<String, Any?> = mapOf(
    "valuesFrom" to resolve("valuesFrom", GraphQLString.type),
    "valuesTo" to resolve("valuesTo", GraphQLString.type),
    "resolutions" to resolve("resolutions", Resolution.type.notNull().list().notNull()),
  )
}

[mbonnin]

@annsofi Most likely the same issue, can you try with the SNAPSHOTs?

[annsofi]

Yep!

[annsofi]

@mbonnin that seems to work, thanks! I wasn't sure what version the fix would be in and forgot that there's snapshots to test.

[mbonnin]

Sorry about the inconvenience. We'll do a new version this week

[annsofi]

No worries, it is mentioned that they're experimental so some trial and error is probably expected 🙂