From b193c504d2a521f0b6404feb15b3313f1262c93c Mon Sep 17 00:00:00 2001 From: Ethan Uberseder Date: Fri, 22 Dec 2023 12:56:41 -0500 Subject: [PATCH 1/3] first attempt at generics --- .../crd/generator/AbstractJsonSchema.java | 76 +++++++++++++++---- 1 file changed, 60 insertions(+), 16 deletions(-) diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java index 7128f3d56e2..dc3fd03136a 100644 --- a/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java @@ -193,7 +193,7 @@ public boolean isPreserveUnknownFields() { */ protected T internalFrom(TypeDef definition, String... ignore) { List schemaSwaps = new ArrayList<>(); - T ret = internalFromImpl(definition, new HashSet<>(), schemaSwaps, ignore); + T ret = internalFromImpl(definition, new HashSet<>(), schemaSwaps, new ParameterMap(new HashMap<>()), ignore); validateRemainingSchemaSwaps("unmatched class", schemaSwaps); return ret; } @@ -277,7 +277,7 @@ private void validateRemainingSchemaSwaps(String error, List } } - private T internalFromImpl(TypeDef definition, Set visited, List schemaSwaps, String... ignore) { + private T internalFromImpl(TypeDef definition, Set visited, List schemaSwaps, ParameterMap parameterMap, String... ignore) { final B builder = newBuilder(); Set ignores = ignore.length > 0 ? new LinkedHashSet<>(Arrays.asList(ignore)) : Collections @@ -312,7 +312,7 @@ private T internalFromImpl(TypeDef definition, Set visited, List matchedSchemaSwaps = facade.getMatchedSchemaSwaps(); currentSchemaSwaps.removeAll(matchedSchemaSwaps); @@ -324,7 +324,7 @@ private T internalFromImpl(TypeDef definition, Set visited, List indexPotentialAccessors(TypeDef definition) { return accessors; } + private static class ParameterMap { + final Map mappings; + + ParameterMap(Map mappings) { + this.mappings = mappings; + } + + TypeRef exchange(TypeRef original){ + return exchange(original, true); + } + + TypeRef exchange(TypeRef original, boolean throwOnFailedLookup){ + if (original instanceof TypeParamRef) { + String name = ((TypeParamRef) original).getName(); + TypeRef ref = mappings.get(name); + if(ref != null) { + return ref; + } + if(throwOnFailedLookup) { + throw new RuntimeException(String.format("Could not find type mapping for parametrized type %s", name)); + } + } + return original; + } + + static ParameterMap from(ClassRef classRef, ParameterMap parentMappings) { + TypeDef def = Types.typeDefFrom(classRef); + + Map mappings = new HashMap<>(); + if(!def.getParameters().isEmpty()) { + for(int i=0; i annotations; @@ -515,6 +556,7 @@ private static class PropertyFacade { private final List propertyOrAccessors = new ArrayList<>(4); private final Set schemaSwaps; private final Set matchedSchemaSwaps; + private final ParameterMap parameterMap; private String renamedTo; private String description; private String defaultValue; @@ -530,9 +572,10 @@ private static class PropertyFacade { private String descriptionContributedBy; private TypeRef schemaFrom; - public PropertyFacade(Property property, Map potentialAccessors, Set schemaSwaps) { + public PropertyFacade(Property property, Map potentialAccessors, Set schemaSwaps, ParameterMap parameterMap) { original = property; this.schemaSwaps = schemaSwaps; + this.parameterMap = parameterMap; this.matchedSchemaSwaps = new HashSet<>(); final String capitalized = property.getNameCapitalized(); final String name = property.getName(); @@ -610,7 +653,7 @@ public Property process() { } }); - TypeRef typeRef = schemaFrom != null ? schemaFrom : original.getTypeRef(); + TypeRef typeRef = schemaFrom != null ? schemaFrom : parameterMap.exchange(original.getTypeRef()); String finalName = renamedTo != null ? renamedTo : original.getName(); return new Property(original.getAnnotations(), typeRef, finalName, @@ -685,16 +728,16 @@ private String extractUpdatedNameFromJacksonPropertyIfPresent(Property property) * @return the structural schema associated with the specified property */ public T internalFrom(String name, TypeRef typeRef) { - return internalFromImpl(name, typeRef, new HashSet<>(), new ArrayList<>()); + return internalFromImpl(name, typeRef, new HashSet<>(), new ArrayList<>(), new ParameterMap(new HashMap<>())); } - private T internalFromImpl(String name, TypeRef typeRef, Set visited, List schemaSwaps) { + private T internalFromImpl(String name, TypeRef typeRef, Set visited, List schemaSwaps, ParameterMap parameterMap) { // Note that ordering of the checks here is meaningful: we need to check for complex types last // in case some "complex" types are handled specifically if (typeRef.getDimensions() > 0 || io.sundr.model.utils.Collections.isCollection(typeRef)) { // Handle Collections & Arrays final TypeRef collectionType = TypeAs.combine(TypeAs.UNWRAP_ARRAY_OF, TypeAs.UNWRAP_COLLECTION_OF) .apply(typeRef); - final T schema = internalFromImpl(name, collectionType, visited, schemaSwaps); + final T schema = internalFromImpl(name, parameterMap.exchange(collectionType), visited, schemaSwaps, parameterMap); return arrayLikeProperty(schema); } else if (io.sundr.model.utils.Collections.IS_MAP.apply(typeRef)) { // Handle Maps final TypeRef keyType = TypeAs.UNWRAP_MAP_KEY_OF.apply(typeRef); @@ -704,15 +747,15 @@ private T internalFromImpl(String name, TypeRef typeRef, Set visited, Li } final TypeRef valueType = TypeAs.UNWRAP_MAP_VALUE_OF.apply(typeRef); - T schema = internalFromImpl(name, valueType, visited, schemaSwaps); + T schema = internalFromImpl(name, parameterMap.exchange(valueType, false), visited, schemaSwaps, parameterMap); if (schema == null) { LOGGER.warn("Property '{}' with '{}' value type is mapped to 'object' because its CRD representation cannot be extracted.", name, typeRef); - schema = internalFromImpl(name, OBJECT_REF, visited, schemaSwaps); + schema = internalFromImpl(name, OBJECT_REF, visited, schemaSwaps, parameterMap); } return mapLikeProperty(schema); } else if (io.sundr.model.utils.Optionals.isOptional(typeRef)) { // Handle Optionals - return internalFromImpl(name, TypeAs.UNWRAP_OPTIONAL_OF.apply(typeRef), visited, schemaSwaps); + return internalFromImpl(name, parameterMap.exchange(TypeAs.UNWRAP_OPTIONAL_OF.apply(typeRef)), visited, schemaSwaps, parameterMap); } else { final String typeName = COMMON_MAPPINGS.get(typeRef); if (typeName != null) { // we have a type that we handle specifically @@ -724,9 +767,9 @@ private T internalFromImpl(String name, TypeRef typeRef, Set visited, Li } else { if (typeRef instanceof ClassRef) { // Handle complex types ClassRef classRef = (ClassRef) typeRef; - TypeDef def = Types.typeDefFrom(classRef); // check if we're dealing with an enum + TypeDef def = Types.typeDefFrom(classRef); if (def.isEnum()) { final JsonNode[] enumValues = def.getProperties().stream() .map(this::extractUpdatedNameFromJacksonPropertyIfPresent) @@ -735,7 +778,7 @@ private T internalFromImpl(String name, TypeRef typeRef, Set visited, Li .toArray(JsonNode[]::new); return enumProperty(enumValues); } else { - return resolveNestedClass(name, def, visited, schemaSwaps); + return resolveNestedClass(name, classRef, visited, schemaSwaps, parameterMap); } } @@ -747,7 +790,8 @@ private T internalFromImpl(String name, TypeRef typeRef, Set visited, Li // Flag to detect cycles private boolean resolving = false; - private T resolveNestedClass(String name, TypeDef def, Set visited, List schemaSwaps) { + private T resolveNestedClass(String name, ClassRef classRef, Set visited, List schemaSwaps, ParameterMap parameterMap) { + TypeDef def = Types.typeDefFrom(classRef); if (!resolving) { visited.clear(); resolving = true; @@ -759,7 +803,7 @@ private T resolveNestedClass(String name, TypeDef def, Set visited, List visited.add(visitedName); } - T res = internalFromImpl(def, visited, schemaSwaps); + T res = internalFromImpl(def, visited, schemaSwaps, ParameterMap.from(classRef, parameterMap)); resolving = false; return res; } From 96bebe963f79b84f9a3c1b8815903e9ce9b1b1e1 Mon Sep 17 00:00:00 2001 From: Ethan Uberseder Date: Fri, 22 Dec 2023 15:15:35 -0500 Subject: [PATCH 2/3] add tests for generics --- .../fabric8/crd/example/generic/Generic.java | 5 ++ .../crd/example/generic/NestedGeneric.java | 8 ++++ .../example/generic/ResourceWithGeneric.java | 6 +++ .../generic/ResourceWithGenericSpec.java | 7 +++ .../crd/generator/v1/JsonSchemaTest.java | 47 +++++++++++++++++++ 5 files changed, 73 insertions(+) create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/generic/Generic.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/generic/NestedGeneric.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/generic/ResourceWithGeneric.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/generic/ResourceWithGenericSpec.java diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/generic/Generic.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/generic/Generic.java new file mode 100644 index 00000000000..49a9633d727 --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/generic/Generic.java @@ -0,0 +1,5 @@ +package io.fabric8.crd.example.generic; + +public class Generic { + T bar; +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/generic/NestedGeneric.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/generic/NestedGeneric.java new file mode 100644 index 00000000000..6dad0c9e742 --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/generic/NestedGeneric.java @@ -0,0 +1,8 @@ +package io.fabric8.crd.example.generic; + +import java.util.List; + +public class NestedGeneric

{ + Generic

quux; + List

corge; +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/generic/ResourceWithGeneric.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/generic/ResourceWithGeneric.java new file mode 100644 index 00000000000..7c08b5d5a80 --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/generic/ResourceWithGeneric.java @@ -0,0 +1,6 @@ +package io.fabric8.crd.example.generic; + +import io.fabric8.kubernetes.client.CustomResource; + +public class ResourceWithGeneric extends CustomResource { +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/generic/ResourceWithGenericSpec.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/generic/ResourceWithGenericSpec.java new file mode 100644 index 00000000000..5f93bfadcdb --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/generic/ResourceWithGenericSpec.java @@ -0,0 +1,7 @@ +package io.fabric8.crd.example.generic; + +public class ResourceWithGenericSpec { + Generic foo; + Generic baz; + NestedGeneric qux; +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/generator/v1/JsonSchemaTest.java b/crd-generator/api/src/test/java/io/fabric8/crd/generator/v1/JsonSchemaTest.java index 57f7c071990..8d487b77a1b 100644 --- a/crd-generator/api/src/test/java/io/fabric8/crd/generator/v1/JsonSchemaTest.java +++ b/crd-generator/api/src/test/java/io/fabric8/crd/generator/v1/JsonSchemaTest.java @@ -22,10 +22,12 @@ import io.fabric8.crd.example.extraction.Extraction; import io.fabric8.crd.example.extraction.IncorrectExtraction; import io.fabric8.crd.example.extraction.IncorrectExtraction2; +import io.fabric8.crd.example.generic.ResourceWithGeneric; import io.fabric8.crd.example.json.ContainingJson; import io.fabric8.crd.example.person.Person; import io.fabric8.crd.generator.utils.Types; import io.fabric8.kubernetes.api.model.apiextensions.v1.JSONSchemaProps; +import io.fabric8.kubernetes.api.model.apiextensions.v1.JSONSchemaPropsOrArray; import io.sundr.model.TypeDef; import org.junit.jupiter.api.Test; @@ -222,6 +224,51 @@ void shouldExtractPropertiesSchemaFromExtractValueAnnotation() { assertNull(barProps.get("baz")); } + @Test + void shouldProcessGenericClasses() { + TypeDef resourceWithGeneric = Types.typeDefFrom(ResourceWithGeneric.class); + JSONSchemaProps schema = JsonSchema.from(resourceWithGeneric); + assertNotNull(schema); + + Map properties = schema.getProperties(); + assertEquals(2, properties.size()); + + final JSONSchemaProps specSchema = properties.get("spec"); + Map spec = specSchema.getProperties(); + assertEquals(3, spec.size()); + + JSONSchemaProps foo = spec.get("foo"); + assertNotNull(foo); + Map fooProps = foo.getProperties(); + assertNotNull(fooProps); + assertEquals("string", fooProps.get("bar").getType()); + + JSONSchemaProps baz = spec.get("baz"); + assertNotNull(baz); + Map bazProps = baz.getProperties(); + assertNotNull(bazProps); + assertEquals("integer", bazProps.get("bar").getType()); + + JSONSchemaProps qux = spec.get("qux"); + assertNotNull(qux); + Map quxProps = qux.getProperties(); + assertEquals(2, quxProps.size()); + + JSONSchemaProps quux = quxProps.get("quux"); + assertNotNull(quux); + Map quuxProps = quux.getProperties(); + assertNotNull(quuxProps); + assertEquals("string", quuxProps.get("bar").getType()); + + JSONSchemaProps corge = quxProps.get("corge"); + assertNotNull(corge); + JSONSchemaPropsOrArray corgeItems = corge.getItems(); + assertNotNull(corgeItems); + JSONSchemaProps corgeItemsProps = corgeItems.getSchema(); + assertNotNull(corgeItemsProps); + assertEquals("string", corgeItemsProps.getType()); + } + @Test void shouldThrowIfSchemaSwapHasUnmatchedField() { TypeDef incorrectExtraction = Types.typeDefFrom(IncorrectExtraction.class); From 893f39ef4e5286016ea23b9768573df40adaaa46 Mon Sep 17 00:00:00 2001 From: Ethan Uberseder Date: Tue, 9 Jan 2024 17:52:21 -0500 Subject: [PATCH 3/3] remove unnecessary check --- .../io/fabric8/crd/generator/AbstractJsonSchema.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java index dc3fd03136a..92a61ec8d36 100644 --- a/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java @@ -392,13 +392,11 @@ static ParameterMap from(ClassRef classRef, ParameterMap parentMappings) { TypeDef def = Types.typeDefFrom(classRef); Map mappings = new HashMap<>(); - if(!def.getParameters().isEmpty()) { - for(int i=0; i