Add a null-check to java enum safeValueOf

[BoD]

Fix for #5901

• Also make the constructor of the class and of UNKNOWN__ nullable
• Also call super() in equals() and hashCode()

[netlify]

✅ Deploy Preview for apollo-android-docs canceled.

Name	Link
🔨 Latest commit	7a3c146
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-android-docs/deploys/6655ef0ff619ec0008bf35c5

[martinbonnin]

A few comments. Also, can we do the same for Kotlin codegen?

[martinbonnin]

What is the rationale for this?

[BoD]

I noticed that without this, 2 unknown enums with a different rawValue would still be equal, since that field is not included in the comparison. The super version does include the field in the comparison.

[martinbonnin]

Shouldn't it compare rawValues instead? I would expect Object.equals() to do referential equality only?

[BoD]

You're totally right! I assumed equals was implemented in the base class but it's not, so calling super doesn't work. In the Kotlin version the equals/hashCode is ad-hoc, I'll do the same in Java.

[martinbonnin]

Same here. Do we really need that change?

[martinbonnin]

TIL about Objects.requireNonNull. Should we use that instead of

apollo-kotlin/libraries/apollo-api-java/src/main/java/com/apollographql/apollo3/api/java/Assertions.java

Line 4 in 39df250

public static <T> T checkNotNull(T value, String errorMessage) {

?

[BoD]

I was almost sure we had something like that but couldn't find it anymore 😅. Ours is a bit nicer since you can pass a message - so I'd keep it.

[martinbonnin]

Can you add a comment on ours?

// A version of Objects.requireNonNull that allows a customized message

[BoD]

can we do the same for Kotlin codegen?

You're right! We can make the class constructor private. I'd keep the Kotlin UNKNOWN__ constructor public though as it's a nice way to signal a warning when using it, thanks to the opt-in annotation. On the other hand, it's still possible to call safeValueOf without any warning... Maybe safeValueOf should be annotated too?

[martinbonnin]

I'd keep the Kotlin UNKNOWN__ constructor public though as it's a nice way to signal a warning when using it, thanks to the opt-in annotation

I'd try to keep the symmetry with Java as much as possible.

On the other hand, it's still possible to call safeValueOf without any warning... Maybe safeValueOf should be annotated too?

I'd go for that. And rename the @ApolloEnumConstructor to @ApolloUnknownEnum or something like that.

[BoD]

Turns out, actually in Kotlin we can't make the UNKOWN__ constructor private, because unlike Java, outer classes can't access private constructors of inner classes. I kept the optin annotation there, and also applied it to safeValueOf.