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

fix: Stack overflow on multi maps (and other non-trivial generic classes) #4413

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
#### Bugs
* Fix #4369: Informers will retry with a backoff on list/watch failure as they did in 5.12 and prior.
* Fix #4350: SchemaSwap annotation is now repeatable and is applied multiple times if classes are used more than once in the class hierarchy.
* Fix #4413: Stack overflow on multi maps (and other non-trivial generic classes)

#### Improvements
* Fix #4348: Introduce specific annotations for the generators
Expand All @@ -15,6 +16,7 @@
* Fix #4243: Update Tekton pipeline model to v0.39.0
* Fix #4243: Update Tekton triggers model to v0.20.2
* Fix #4383: bump snakeyaml from 1.30 to 1.31
* Fix #4413: update sundrio to 0.94

#### New Features

Expand Down
Expand Up @@ -25,6 +25,10 @@
import io.sundr.model.*;
import io.sundr.model.functions.GetDefinition;
import io.sundr.model.repo.DefinitionRepository;
import io.sundr.model.visitors.ApplyTypeParamMappingToMethod;
import io.sundr.model.visitors.ApplyTypeParamMappingToProperty;
import io.sundr.model.visitors.ApplyTypeParamMappingToTypeArguments;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -80,6 +84,10 @@ private static TypeDef projectDefinition(ClassRef ref) {
}

List<TypeParamDef> parameters = definition.getParameters();
if (parameters.size() != arguments.size()) {
throw new IllegalStateException("Incompatible reference " + ref + " to " + definition);
}

Map<String, TypeRef> mappings = new HashMap<>();
for (int i = 0; i < arguments.size(); i++) {
String name = parameters.get(i).getName();
Expand All @@ -88,62 +96,12 @@ private static TypeDef projectDefinition(ClassRef ref) {
}

return new TypeDefBuilder(definition)
.accept(mapClassRefArguments(mappings), mapGenericProperties(mappings))
.accept(new ApplyTypeParamMappingToTypeArguments(mappings)) // existing type arguments must be handled before methods and properties
.accept(new ApplyTypeParamMappingToProperty(mappings),
new ApplyTypeParamMappingToMethod(mappings))
.build();
}

/**
* Map generic properties to known {@link TypeRef} based on the specified mappings.
* Example: Given a property {@code T size} and a map containing {@code T -> Integer} the final
* property will be: {@code Integer type}.
* @param mappings A map that maps class arguments names to types.
* @return a visitors that performs the actual mapping.
*/
private static TypedVisitor<PropertyBuilder> mapGenericProperties(Map<String, TypeRef> mappings) {
return new TypedVisitor<PropertyBuilder>() {
@Override
public void visit(PropertyBuilder property) {
TypeRef typeRef = property.buildTypeRef();
if (typeRef instanceof TypeParamRef) {
TypeParamRef typeParamRef = (TypeParamRef) typeRef;
String key = typeParamRef.getName();
TypeRef paramRef = mappings.get(key);
if (paramRef != null) {
property.withTypeRef(paramRef);
}
}
}
};
}

/**
* Map arguments, of {@link ClassRef} instances to known {@link TypeRef} based on the specified mappings.
* Example: Given a class reference to {@code Shape<T>} and a map containing {@code T -> Integer}
* the final reference will be: {@code Shape<Integer> type}.
* @param mappings A map that maps class arguments names to types.
* @return a visitors that performs the actual mapping.
*/
private static TypedVisitor<ClassRefBuilder> mapClassRefArguments(Map<String, TypeRef> mappings) {
return new TypedVisitor<ClassRefBuilder>() {
@Override
public void visit(ClassRefBuilder c) {
List<TypeRef> arguments = new ArrayList<>();
for (TypeRef arg : c.buildArguments()) {
TypeRef mappedRef = arg;
if (arg instanceof TypeParamRef) {
TypeParamRef typeParamRef = (TypeParamRef) arg;
TypeRef mapping = mappings.get(typeParamRef.getName());
if (mapping != null) {
mappedRef = mapping;
}
}
arguments.add(mappedRef);
}
c.withArguments(arguments);
}
};
}

private static Set<ClassRef> projectSuperClasses(TypeDef definition) {
List<ClassRef> extendsList = definition.getExtendsList();
return extendsList.stream()
Expand Down
Expand Up @@ -15,6 +15,7 @@
*/
package io.fabric8.crd.example.map;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

Expand All @@ -32,4 +33,7 @@ public Map<String, Map<String, List<Boolean>>> getTest2() {
return test2;
}

private MyMultiMap<String, String> test3;

class MyMultiMap<K, V> extends HashMap<K, List<V>> {}
}
Expand Up @@ -275,7 +275,7 @@ void mapPropertyShouldHaveCorrectValueType() {
final Map<String, JSONSchemaProps> specProps = version.getSchema().getOpenAPIV3Schema()
.getProperties().get("spec").getProperties();

assertEquals(2, specProps.size());
assertEquals(3, specProps.size());

checkMapProp(specProps, "test", "array");
String arrayType = specProps.get("test").getAdditionalProperties().getSchema().getItems().getSchema().getType();
Expand All @@ -286,6 +286,12 @@ void mapPropertyShouldHaveCorrectValueType() {
String valueType = valueSchema.getType();
assertEquals("array", valueType);

// this check is currently failing, because multimaps are incorrectly processed as if they were normal maps
// (class MultiMap<K,V> implements Map<K,List<V>> is treated like just Map<K,V>)
// checkMapProp(specProps, "test3", "object");
final JSONSchemaProps props = specProps.get("test3");
assertEquals("object", props.getType());

assertEquals("boolean", valueSchema.getItems().getSchema().getType());
});
}
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -80,7 +80,7 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>

<!-- Core versions -->
<sundrio.version>0.93.0</sundrio.version>
<sundrio.version>0.94-bugfix-multi-maps3-SNAPSHOT</sundrio.version>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: 0.94

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xRodney / @iocanel please ping me as soon this is ready to review 🙏

<okhttp.version>3.12.12</okhttp.version>
<okhttp.bundle.version>3.12.1_1</okhttp.bundle.version>
<okio.version>1.15.0</okio.version>
Expand Down