TypeAs.UNWRAP_MAP_VALUE_OF and UNWRAP_MAP_KEY_OF return wrong results for multimaps

[xRodney]

Hello again,

as a follow up to #333, I would like to address the issue that generic arguments on non-trivial Maps and Collections are not resolved correctly.

I come across this issue while working on kubernetes-client/crd-generator, using keycloak-operator as a sample operator.

In Keycloak, they have a class similar to the following:

public class MultivaluedHashMap<K, V> extends HashMap<K, List<V>> {...}

(ref https://github.com/keycloak/keycloak/blob/main/common/src/main/java/org/keycloak/common/util/MultivaluedHashMap.java)

The class is then used like this:

public class RealmRepresentation {
    private MultivaluedHashMap<String, ComponentExportRepresentation> components;
	// ...
}

(ref https://github.com/keycloak/keycloak/blob/main/core/src/main/java/org/keycloak/representations/idm/RealmRepresentation.java#L189)

The reference MultivaluedHashMap<String, ComponentExportRepresentation> is correctly recognized as a Map by sundrio, but then it is investivated for its key and value parameters.
For this, two routines from io.sundr.builder.internal.functions.TypeAs are used: UNWRAP_MAP_KEY_OF and UNWRAP_MAP_VALUE_OF.

The key is correctly recognized as String, but the value is ComponentExportRepresentation. In other words, the reference is treated as Map<String, ComponentExportRepresentation>. But the correct type should be Map<String, List<ComponentExportRepresentation>>.

If we look closer into the routines in TypeAs, we can see that they both delegate to a common implementation:

  private static Optional<TypeRef> extractArgument(ClassRef classRef, Function<TypeRef, Boolean> typeCheckFn,
      int argumentIndex) {
    if (typeCheckFn.apply(classRef)) {
      if (classRef.getArguments().size() > argumentIndex) {
        return Optional.of(classRef.getArguments().get(argumentIndex));
      } else {
        // Fallback
        TypeDef typeDef = GetDefinition.of(classRef);
        return Stream
            .concat(typeDef.getExtendsList().stream(), typeDef.getImplementsList().stream())
            .map((ref) -> extractArgument(ref, typeCheckFn, argumentIndex))
            .reduce(Optional.empty(), (o, n) -> o.isPresent() ? o : n);
      }
    } else {
      return Optional.empty();
    }
  }

(ref https://github.com/sundrio/sundrio/blob/main/annotations/builder/src/main/java/io/sundr/builder/internal/functions/TypeAs.java#L225)

We can see that this implementation is really basic (although, I believe, it is correct in 99% real world scenarios :D ).

The typeCheckFn returns true if the type id or subclasses Map. So for MultivaluedHashMap it returns true.

It then assumes that either we are investigating the map directly (or a subclass that has exactly the same type arguments), or we reference a subclass that has all arguments specified (like class StringMap implements Map<String,String>).
Our MultivaluedHashMap falls into the first category, as it has enough arguments. The actual relation of these arguments to the Map interface is never investigated.

Other examples that for which the implementation would also yield bad results:

• interface StringKeyedMap<V> extends Map<String,V>
• interface SwappedArguments<V,K> extends Map<K,V>
• interface MoreArguments<X,K,V> extends Map<K,V>
• interface OptionalMap<K,V> extends Map<K, Optional<V>>
• ... and many others

Yes, most of these are corner cases, which should never appear in a production code, and righfully so.
But as I wrote earlier, I stubled across that 1% of legitimate cases for which the algorithm should work.

This has been tested on the latest main, which is 0.93-SNAPSHOT.

I believe I may have a fix for that, I will be opening a PR shortly.