From 8595d4139aa800f2cf712d3ba1e786d2de421e23 Mon Sep 17 00:00:00 2001 From: Dusan Jakub Date: Thu, 25 Aug 2022 17:39:39 +0200 Subject: [PATCH] fix: @SchemaSwap can only be used once @SchemaSwap is now repeatable, so multiple modifications can be applied from the same resource root. Fixed a bug when the swap was "used up" after just one usage, meaning that if the referenced type was used multiple times in object hierarchy, only the first one was swapped. Fixes fabric8io/kubernetes-client#4350 --- CHANGELOG.md | 3 + .../crd/generator/AbstractJsonSchema.java | 152 ++++++------------ .../crd/generator/InternalSchemaSwaps.java | 138 ++++++++++++++++ .../crd/generator/annotation/SchemaSwap.java | 31 +++- .../crd/generator/annotation/SchemaSwaps.java | 30 ++++ .../extraction/DeeplyNestedSchemaSwaps.java | 49 ++++++ .../extraction/MultipleSchemaSwaps.java | 26 +++ .../example/extraction/SchemaSwapSpec.java | 35 ++++ .../crd/generator/v1/JsonSchemaTest.java | 116 ++++++++++--- .../schemaswaps/MultipleSchemaSwaps.java | 30 ++++ .../schemaswaps/SchemaSwapCRDTest.java | 75 +++++++++ .../generator/schemaswaps/SchemaSwapSpec.java | 35 ++++ doc/CRD-generator.md | 2 + 13 files changed, 594 insertions(+), 128 deletions(-) create mode 100644 crd-generator/api/src/main/java/io/fabric8/crd/generator/InternalSchemaSwaps.java create mode 100644 crd-generator/api/src/main/java/io/fabric8/crd/generator/annotation/SchemaSwaps.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/extraction/DeeplyNestedSchemaSwaps.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/extraction/MultipleSchemaSwaps.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/extraction/SchemaSwapSpec.java create mode 100644 crd-generator/test/src/test/java/io/fabric8/crd/generator/schemaswaps/MultipleSchemaSwaps.java create mode 100644 crd-generator/test/src/test/java/io/fabric8/crd/generator/schemaswaps/SchemaSwapCRDTest.java create mode 100644 crd-generator/test/src/test/java/io/fabric8/crd/generator/schemaswaps/SchemaSwapSpec.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 977be5f7e7b..2d8a860e233 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### 6.2-SNAPSHOT #### Bugs +* Fix #4350: SchemaSwap annotation is now repeatable and is applied multiple times if classes are used more than once in the class hierarchy. #### Improvements @@ -11,6 +12,7 @@ #### New Features #### _**Note**_: Breaking changes in the API +* Fix #4350: SchemaSwap's fieldName parameter now expects a field name only, not a method or a constructor. ### 6.1.1 (2022-09-01) @@ -28,6 +30,7 @@ fix #4373: NO_PROXY should allow URIs with hyphens ("circleci-internal-outer-bui * Fix #4320: corrected leader transitions field on leader election leases * Fix #4360: JUnit dependencies aren't leaked in child modules + #### Improvements * Fix #887: added KubernetesClient.visitResources to search and perform other operations across all resources. * Fix #3960: adding a KubernetesMockServer.expectCustomResource helper method and additional mock crd support 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 59fbfd70d6b..3f2cd7585f2 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 @@ -29,13 +29,13 @@ import org.slf4j.LoggerFactory; import java.util.*; -import java.util.stream.Collectors; import static io.sundr.model.utils.Types.BOOLEAN_REF; import static io.sundr.model.utils.Types.DOUBLE_REF; import static io.sundr.model.utils.Types.INT_REF; import static io.sundr.model.utils.Types.LONG_REF; import static io.sundr.model.utils.Types.STRING_REF; +import static io.sundr.model.utils.Types.VOID; /** * Encapsulates the common logic supporting OpenAPI schema generation for CRD generation. @@ -81,6 +81,7 @@ public abstract class AbstractJsonSchema { public static final String ANNOTATION_NOT_NULL = "javax.validation.constraints.NotNull"; public static final String ANNOTATION_SCHEMA_FROM = "io.fabric8.crd.generator.annotation.SchemaFrom"; public static final String ANNOTATION_SCHEMA_SWAP = "io.fabric8.crd.generator.annotation.SchemaSwap"; + public static final String ANNOTATION_SCHEMA_SWAPS = "io.fabric8.crd.generator.annotation.SchemaSwaps"; public static final String JSON_NODE_TYPE = "com.fasterxml.jackson.databind.JsonNode"; @@ -119,60 +120,12 @@ public static String getSchemaTypeFor(TypeRef typeRef) { * @return The schema. */ protected T internalFrom(TypeDef definition, String... ignore) { - List schemaSwaps = new ArrayList<>(); + InternalSchemaSwaps schemaSwaps = new InternalSchemaSwaps(); T ret = internalFromImpl(definition, new HashSet<>(), schemaSwaps, ignore); - validateRemainingSchemaSwaps("unmatched class", schemaSwaps); + schemaSwaps.throwIfUnmatchedSwaps(); return ret; } - private static class InternalSchemaSwap { - final ClassRef targetType; - final ClassRef originalType; - final String fieldName; - - public InternalSchemaSwap(ClassRef originalType, String fieldName, ClassRef targetType) { - this.originalType = originalType; - this.fieldName = fieldName; - this.targetType = targetType; - } - - public ClassRef getTargetType() { - return targetType; - } - - public ClassRef getOriginalType() { - return originalType; - } - - public String getFieldName() { - return fieldName; - } - - @Override - public String toString() { - return "{" + - "targetType=" + targetType + - ", originalType=" + originalType + - ", fieldName='" + fieldName + '\'' + - '}'; - } - - @Override - public boolean equals(Object o) { - if (this == o) - return true; - if (o == null || getClass() != o.getClass()) - return false; - InternalSchemaSwap that = (InternalSchemaSwap) o; - return targetType.equals(that.targetType) && originalType.equals(that.originalType) && fieldName.equals(that.fieldName); - } - - @Override - public int hashCode() { - return Objects.hash(targetType, originalType, fieldName); - } - } - private static ClassRef extractClassRef(Object type) { if (type != null) { if (type instanceof ClassRef) { @@ -187,25 +140,44 @@ private static ClassRef extractClassRef(Object type) { } } - private InternalSchemaSwap extractSchemaSwap(AnnotationRef annotation) { - Map params = annotation.getParameters(); - return new InternalSchemaSwap( - extractClassRef(params.get("originalType")), - (String) params.get("fieldName"), - extractClassRef(params.get("targetType"))); + private void extractSchemaSwaps(ClassRef definitionType, AnnotationRef annotation, InternalSchemaSwaps schemaSwaps) { + String fullyQualifiedName = annotation.getClassRef().getFullyQualifiedName(); + switch (fullyQualifiedName) { + case ANNOTATION_SCHEMA_SWAP: + extractSchemaSwap(definitionType, annotation, schemaSwaps); + break; + case ANNOTATION_SCHEMA_SWAPS: + Map params = annotation.getParameters(); + Object[] values = (Object[]) params.get("value"); + for (Object value : values) { + extractSchemaSwap(definitionType, value, schemaSwaps); + } + break; + } } - private void validateRemainingSchemaSwaps(String error, List schemaSwaps) { - if (!schemaSwaps.isEmpty()) { - String umatchedSchemaSwaps = schemaSwaps - .stream() - .map(InternalSchemaSwap::toString) - .collect(Collectors.joining(",", "[", "]")); - throw new IllegalArgumentException("SchemaSwap annotation error " + error + ": " + umatchedSchemaSwaps); + private void extractSchemaSwap(ClassRef definitionType, Object annotation, InternalSchemaSwaps schemaSwaps) { + if (annotation instanceof SchemaSwap) { + SchemaSwap schemaSwap = (SchemaSwap) annotation; + schemaSwaps.registerSwap(definitionType, + extractClassRef(schemaSwap.originalType()), + schemaSwap.fieldName(), + extractClassRef(schemaSwap.targetType())); + + } else if (annotation instanceof AnnotationRef + && ((AnnotationRef) annotation).getClassRef().getFullyQualifiedName().equals(ANNOTATION_SCHEMA_SWAP)) { + Map params = ((AnnotationRef) annotation).getParameters(); + schemaSwaps.registerSwap(definitionType, + extractClassRef(params.get("originalType")), + (String) params.get("fieldName"), + extractClassRef(params.getOrDefault("targetType", void.class))); + + } else { + throw new IllegalArgumentException("Unmanaged annotation type passed to the SchemaSwaps: " + annotation); } } - private T internalFromImpl(TypeDef definition, Set visited, List schemaSwaps, String... ignore) { + private T internalFromImpl(TypeDef definition, Set visited, InternalSchemaSwaps schemaSwaps, String... ignore) { final B builder = newBuilder(); Set ignores = ignore.length > 0 ? new LinkedHashSet<>(Arrays.asList(ignore)) : Collections @@ -215,19 +187,7 @@ private T internalFromImpl(TypeDef definition, Set visited, List newSchemaSwaps = definition - .getAnnotations() - .stream() - .filter(a -> a.getClassRef().getFullyQualifiedName().equals(ANNOTATION_SCHEMA_SWAP)) - .map(this::extractSchemaSwap) - .collect(Collectors.toList()); - - schemaSwaps.addAll(newSchemaSwaps); - - final Set currentSchemaSwaps = schemaSwaps - .stream() - .filter(iss -> iss.getOriginalType().getFullyQualifiedName().equals(definition.getFullyQualifiedName())) - .collect(Collectors.toSet()); + definition.getAnnotations().forEach(annotation -> extractSchemaSwaps(definition.toReference(), annotation, schemaSwaps)); // index potential accessors by name for faster lookup final Map accessors = indexPotentialAccessors(definition); @@ -239,11 +199,9 @@ private T internalFromImpl(TypeDef definition, Set visited, List matchedSchemaSwaps = facade.getMatchedSchemaSwaps(); - currentSchemaSwaps.removeAll(matchedSchemaSwaps); - schemaSwaps.removeAll(matchedSchemaSwaps); name = possiblyRenamedProperty.getName(); if (facade.required) { @@ -267,7 +225,6 @@ private T internalFromImpl(TypeDef definition, Set visited, List propertyOrAccessors = new ArrayList<>(4); - private final Set schemaSwaps; - private final Set matchedSchemaSwaps; private String renamedTo; private String description; private boolean required; @@ -395,10 +350,8 @@ private static class PropertyFacade { private String descriptionContributedBy; private TypeRef schemaFrom; - public PropertyFacade(Property property, Map potentialAccessors, Set schemaSwaps) { + public PropertyFacade(Property property, Map potentialAccessors, ClassRef schemaSwap) { original = property; - this.schemaSwaps = schemaSwaps; - this.matchedSchemaSwaps = new HashSet<>(); final String capitalized = property.getNameCapitalized(); final String name = property.getName(); propertyOrAccessors.add(PropertyOrAccessor.fromProperty(property)); @@ -414,21 +367,12 @@ public PropertyFacade(Property property, Map potentialAccessors, if (method != null) { propertyOrAccessors.add(PropertyOrAccessor.fromMethod(method, name)); } + schemaFrom = schemaSwap; } public Property process() { final String name = original.getName(); - Optional currentSchemaSwap = schemaSwaps - .stream() - .filter(iss -> iss.getFieldName().equals(name)) - .findFirst(); - - currentSchemaSwap.ifPresent(iss -> { - schemaFrom = iss.targetType; - matchedSchemaSwaps.add(iss); - }); - propertyOrAccessors.forEach(p -> { p.process(); final String contributorName = p.toString(); @@ -471,10 +415,6 @@ public Property process() { return new Property(original.getAnnotations(), typeRef, finalName, original.getComments(), original.getModifiers(), original.getAttributes()); } - - public Set getMatchedSchemaSwaps() { - return this.matchedSchemaSwaps; - } } private boolean isPotentialAccessor(Method method) { @@ -547,10 +487,10 @@ 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 InternalSchemaSwaps()); } - private T internalFromImpl(String name, TypeRef typeRef, Set visited, List schemaSwaps) { + private T internalFromImpl(String name, TypeRef typeRef, Set visited, InternalSchemaSwaps schemaSwaps) { // 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 @@ -598,7 +538,7 @@ private T internalFromImpl(String name, TypeRef typeRef, Set visited, Li .map(JsonNodeFactory.instance::textNode) .toArray(JsonNode[]::new); return enumProperty(enumValues); - } else { + } else if (!classRef.getFullyQualifiedName().equals(VOID.getName())) { return resolveNestedClass(name, def, visited, schemaSwaps); } @@ -611,7 +551,7 @@ 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, TypeDef def, Set visited, InternalSchemaSwaps schemaSwaps) { if (!resolving) { visited.clear(); resolving = true; diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/InternalSchemaSwaps.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/InternalSchemaSwaps.java new file mode 100644 index 00000000000..2c1a030aef4 --- /dev/null +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/InternalSchemaSwaps.java @@ -0,0 +1,138 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.generator; + +import io.sundr.model.ClassRef; + +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.StringJoiner; +import java.util.stream.Collectors; + +public class InternalSchemaSwaps { + private final Map swaps = new HashMap<>(); + + public void registerSwap(ClassRef definitionType, ClassRef originalType, String fieldName, ClassRef targetType) { + Value value = new Value(definitionType, originalType, fieldName, targetType); + swaps.put(new Key(originalType, fieldName), value); + } + + public Optional lookupAndMark(ClassRef originalType, String name) { + Value value = swaps.get(new Key(originalType, name)); + if (value != null) { + value.markUsed(); + return Optional.of(value.getTargetType()); + } else { + return Optional.empty(); + } + } + + public void throwIfUnmatchedSwaps() { + String unmatchedSchemaSwaps = swaps.values().stream().filter(value -> !value.used) + .map(Object::toString) + .collect(Collectors.joining(", ")); + if (!unmatchedSchemaSwaps.isEmpty()) { + throw new IllegalArgumentException("Unmatched SchemaSwaps: " + unmatchedSchemaSwaps); + } + } + + private static final class Key { + private final ClassRef originalType; + private final String fieldName; + + public Key(ClassRef originalType, String fieldName) { + this.originalType = originalType; + this.fieldName = fieldName; + } + + public ClassRef getOriginalType() { + return originalType; + } + + public String getFieldName() { + return fieldName; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Key key = (Key) o; + return Objects.equals(originalType, key.originalType) && Objects.equals(fieldName, key.fieldName); + } + + @Override + public int hashCode() { + return Objects.hash(originalType, fieldName); + } + + @Override + public String toString() { + return new StringJoiner(", ", Key.class.getSimpleName() + "[", "]") + .add("originalType=" + originalType) + .add("fieldName='" + fieldName + "'") + .toString(); + } + } + + private static class Value { + private final ClassRef originalType; + private final String fieldName; + private final ClassRef targetType; + private boolean used; + private final ClassRef definitionType; + + public Value(ClassRef definitionType, ClassRef originalType, String fieldName, ClassRef targetType) { + this.definitionType = definitionType; + this.originalType = originalType; + this.fieldName = fieldName; + this.targetType = targetType; + this.used = false; + } + + private void markUsed() { + this.used = true; + } + + public ClassRef getOriginalType() { + return originalType; + } + + public String getFieldName() { + return fieldName; + } + + public ClassRef getTargetType() { + return targetType; + } + + public boolean isUsed() { + return used; + } + + @Override + public String toString() { + return "@SchemaSwap(originalType=" + originalType + ", fieldName=\"" + fieldName + "\", targetType=" + targetType + + ") on " + definitionType; + } + } +} diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/annotation/SchemaSwap.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/annotation/SchemaSwap.java index 3c01f566c8b..4903d4e31c4 100644 --- a/crd-generator/api/src/main/java/io/fabric8/crd/generator/annotation/SchemaSwap.java +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/annotation/SchemaSwap.java @@ -17,10 +17,39 @@ import java.lang.annotation.*; -@Target({ElementType.ANNOTATION_TYPE, ElementType.TYPE_USE, ElementType.TYPE}) +/** + * Annotation that allows replacing a nested schema with one from another class. + * + * This is an alternative to {@link SchemaFrom} for cases when the classes + * are coming from an external source and fields cannot be annotated directly. + * + * @see SchemaFrom + */ +@Target({ ElementType.ANNOTATION_TYPE, ElementType.TYPE_USE, ElementType.TYPE }) @Retention(RetentionPolicy.RUNTIME) +@Repeatable(SchemaSwaps.class) public @interface SchemaSwap { + /** + * The owning class of the field whose type is to be replaced. + *

+ * It is an error if the type is not used in the same schema hierarchy where the {@link SchemaSwap} is used. + */ Class originalType(); + + /** + * Name of the field whose type is to be replaced. + *

+ * The name should be specified exactly as defined in the Java class, before any renames + * and transformations ({@code @JsonProperty} and similar) take place. + *

+ * It is an error if the field does not exist on {@link #originalType()} + */ String fieldName(); + + /** + * The replacement schema that will be used for the {@link #fieldName()} instead of its specified type + *

+ * The default value of {@code void.class} causes the field to be skipped + */ Class targetType() default void.class; } diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/annotation/SchemaSwaps.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/annotation/SchemaSwaps.java new file mode 100644 index 00000000000..fb99288d4e3 --- /dev/null +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/annotation/SchemaSwaps.java @@ -0,0 +1,30 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.generator.annotation; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * A container for multiple {@link SchemaSwap}s + */ +@Target({ ElementType.ANNOTATION_TYPE, ElementType.TYPE_USE, ElementType.TYPE }) +@Retention(RetentionPolicy.RUNTIME) +public @interface SchemaSwaps { + SchemaSwap[] value(); +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/extraction/DeeplyNestedSchemaSwaps.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/extraction/DeeplyNestedSchemaSwaps.java new file mode 100644 index 00000000000..aa5136f77ef --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/extraction/DeeplyNestedSchemaSwaps.java @@ -0,0 +1,49 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.extraction; + +import io.fabric8.crd.generator.annotation.SchemaSwap; +import io.fabric8.kubernetes.client.CustomResource; + +@SchemaSwap(originalType = DeeplyNestedSchemaSwaps.MyObject.class, fieldName = "shouldBeString", targetType = String.class) +public class DeeplyNestedSchemaSwaps extends CustomResource { + + public static class Spec { + private MyObject myObject; + private Level1 level1; + } + + private static class Level1 { + private Level2 level2a; + private MyObject myObject; + private Level2 level2b; + } + + private static class Level2 { + private MyObject myObject1; + private Level3 level3; + private MyObject myObject2; + } + + private static class Level3 { + private MyObject myObject1; + private MyObject myObject2; + } + + public static class MyObject { + private int shouldBeString; + } +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/extraction/MultipleSchemaSwaps.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/extraction/MultipleSchemaSwaps.java new file mode 100644 index 00000000000..6f8c9febc5f --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/extraction/MultipleSchemaSwaps.java @@ -0,0 +1,26 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.extraction; + +import io.fabric8.crd.generator.annotation.SchemaSwap; +import io.fabric8.kubernetes.client.CustomResource; + +@SchemaSwap(originalType = SchemaSwapSpec.SomeObject.class, fieldName = "shouldBeString", targetType = String.class) +@SchemaSwap(originalType = SchemaSwapSpec.AnotherObject.class, fieldName = "shouldBeInt", targetType = Integer.class) +@SchemaSwap(originalType = SchemaSwapSpec.YetAnotherObject.class, fieldName = "shouldBeSkipped") +public class MultipleSchemaSwaps extends CustomResource { + +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/extraction/SchemaSwapSpec.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/extraction/SchemaSwapSpec.java new file mode 100644 index 00000000000..530aa131adb --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/extraction/SchemaSwapSpec.java @@ -0,0 +1,35 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.extraction; + +public class SchemaSwapSpec { + private SomeObject first; + private SomeObject second; + private AnotherObject third; + private YetAnotherObject fourth; + + static class SomeObject { + private int shouldBeString; + } + + static class AnotherObject { + private String shouldBeInt; + } + + static class YetAnotherObject { + private String shouldBeSkipped; + } +} 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 a0715d3cd44..909f445ea9d 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 @@ -18,16 +18,19 @@ import com.fasterxml.jackson.databind.JsonNode; import io.fabric8.crd.example.annotated.Annotated; import io.fabric8.crd.example.basic.Basic; +import io.fabric8.crd.example.extraction.DeeplyNestedSchemaSwaps; +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.extraction.MultipleSchemaSwaps; import io.fabric8.crd.example.json.ContainingJson; -import io.fabric8.crd.example.extraction.Extraction; 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.sundr.model.TypeDef; import org.junit.jupiter.api.Test; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -41,19 +44,18 @@ void shouldCreateJsonSchemaFromClass() { TypeDef person = Types.typeDefFrom(Person.class); JSONSchemaProps schema = JsonSchema.from(person); assertNotNull(schema); - Map properties = schema.getProperties(); - assertEquals(7, properties.size()); + Map properties = assertSchemaHasNumberOfProperties(schema, 7); final List personTypes = properties.get("type").getEnum().stream().map(JsonNode::asText) - .collect(Collectors.toList()); + .collect(Collectors.toList()); assertEquals(2, personTypes.size()); assertTrue(personTypes.contains("crazy")); assertTrue(personTypes.contains("crazier")); final Map addressProperties = properties.get("addresses").getItems() - .getSchema().getProperties(); + .getSchema().getProperties(); assertEquals(5, addressProperties.size()); final List addressTypes = addressProperties.get("type").getEnum().stream() - .map(JsonNode::asText) - .collect(Collectors.toList()); + .map(JsonNode::asText) + .collect(Collectors.toList()); assertEquals(2, addressTypes.size()); assertTrue(addressTypes.contains("home")); assertTrue(addressTypes.contains("work")); @@ -76,11 +78,9 @@ void shouldAugmentPropertiesSchemaFromAnnotations() { TypeDef annotated = Types.typeDefFrom(Annotated.class); JSONSchemaProps schema = JsonSchema.from(annotated); assertNotNull(schema); - Map properties = schema.getProperties(); - assertEquals(2, properties.size()); + Map properties = assertSchemaHasNumberOfProperties(schema, 2); final JSONSchemaProps specSchema = properties.get("spec"); - Map spec = specSchema.getProperties(); - assertEquals(6, spec.size()); + Map spec = assertSchemaHasNumberOfProperties(specSchema, 6); // check descriptions are present assertTrue(spec.containsKey("from-field")); @@ -119,11 +119,9 @@ void shouldProduceKubernetesPreserveFields() { TypeDef containingJson = Types.typeDefFrom(ContainingJson.class); JSONSchemaProps schema = JsonSchema.from(containingJson); assertNotNull(schema); - Map properties = schema.getProperties(); - assertEquals(2, properties.size()); + Map properties = assertSchemaHasNumberOfProperties(schema, 2); final JSONSchemaProps specSchema = properties.get("spec"); - Map spec = specSchema.getProperties(); - assertEquals(3, spec.size()); + Map spec = assertSchemaHasNumberOfProperties(specSchema, 3); // check preserve unknown fields is present assertTrue(spec.containsKey("free")); @@ -147,11 +145,9 @@ void shouldExtractPropertiesSchemaFromExtractValueAnnotation() { TypeDef extraction = Types.typeDefFrom(Extraction.class); JSONSchemaProps schema = JsonSchema.from(extraction); assertNotNull(schema); - Map properties = schema.getProperties(); - assertEquals(2, properties.size()); + Map properties = assertSchemaHasNumberOfProperties(schema, 2); final JSONSchemaProps specSchema = properties.get("spec"); - Map spec = specSchema.getProperties(); - assertEquals(2, spec.size()); + Map spec = assertSchemaHasNumberOfProperties(specSchema, 2); // check typed SchemaFrom JSONSchemaProps foo = spec.get("foo"); @@ -178,15 +174,93 @@ void shouldExtractPropertiesSchemaFromExtractValueAnnotation() { assertNull(barProps.get("baz")); } + @Test + void shouldExtractPropertiesSchemaFromSchemaSwapAnnotations() { + TypeDef extraction = Types.typeDefFrom(MultipleSchemaSwaps.class); + JSONSchemaProps schema = JsonSchema.from(extraction); + assertNotNull(schema); + Map properties = assertSchemaHasNumberOfProperties(schema, 2); + final JSONSchemaProps specSchema = properties.get("spec"); + Map spec = assertSchemaHasNumberOfProperties(specSchema, 4); + + // 'first' is replaced by SchemaSwap from int to string + JSONSchemaProps first = spec.get("first"); + assertPropertyHasType(first, "shouldBeString", "string"); + + // 'second' is replaced by the same SchemaSwap that is applied multiple times + JSONSchemaProps second = spec.get("second"); + assertPropertyHasType(second, "shouldBeString", "string"); + + // 'third' is replaced by another SchemaSwap + JSONSchemaProps third = spec.get("third"); + assertPropertyHasType(third, "shouldBeInt", "integer"); + + // 'fourth' is replaced by another SchemaSwap and its property deleted + JSONSchemaProps fourth = spec.get("fourth"); + Map properties4 = fourth.getProperties(); + assertNotNull(properties); + assertTrue(properties4.isEmpty()); + } + + @Test + void shouldApplySchemaSwapsMultipleTimesInDeepClassHierarchy() { + TypeDef extraction = Types.typeDefFrom(DeeplyNestedSchemaSwaps.class); + JSONSchemaProps schema = JsonSchema.from(extraction); + assertNotNull(schema); + Map properties = assertSchemaHasNumberOfProperties(schema, 2); + Map spec = assertSchemaHasNumberOfProperties(properties.get("spec"), 2); + + assertPropertyHasType(spec.get("myObject"), "shouldBeString", "string"); + Map level1 = assertSchemaHasNumberOfProperties(spec.get("level1"), 3); + + assertPropertyHasType(level1.get("myObject"), "shouldBeString", "string"); + List> levels2 = new ArrayList<>(); + levels2.add(assertSchemaHasNumberOfProperties(level1.get("level2a"), 3)); + levels2.add(assertSchemaHasNumberOfProperties(level1.get("level2b"), 3)); + + for (Map level2 : levels2) { + assertPropertyHasType(level2.get("myObject1"), "shouldBeString", "string"); + assertPropertyHasType(level2.get("myObject2"), "shouldBeString", "string"); + + Map level3 = assertSchemaHasNumberOfProperties(level2.get("level3"), 2); + assertPropertyHasType(level3.get("myObject1"), "shouldBeString", "string"); + assertPropertyHasType(level3.get("myObject2"), "shouldBeString", "string"); + } + } + @Test void shouldThrowIfSchemaSwapHasUnmatchedField() { TypeDef incorrectExtraction = Types.typeDefFrom(IncorrectExtraction.class); - assertThrows(IllegalArgumentException.class, () -> JsonSchema.from(incorrectExtraction)); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> JsonSchema.from(incorrectExtraction)); + assertEquals( + "Unmatched SchemaSwaps: @SchemaSwap(originalType=io.fabric8.crd.example.extraction.ExtractionSpec, fieldName=\"FOO\", targetType=io" + + ".fabric8.crd.example.extraction.FooExtractor) on io.fabric8.crd.example.extraction.IncorrectExtraction", + exception.getMessage()); } @Test void shouldThrowIfSchemaSwapHasUnmatchedClass() { TypeDef incorrectExtraction2 = Types.typeDefFrom(IncorrectExtraction2.class); - assertThrows(IllegalArgumentException.class, () -> JsonSchema.from(incorrectExtraction2)); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> JsonSchema.from(incorrectExtraction2)); + assertEquals( + "Unmatched SchemaSwaps: @SchemaSwap(originalType=io.fabric8.crd.example.basic.BasicSpec, fieldName=\"bar\", targetType=io.fabric8.crd" + + ".example.extraction.FooExtractor) on io.fabric8.crd.example.extraction.IncorrectExtraction2", + exception.getMessage()); + } + + private static Map assertSchemaHasNumberOfProperties(JSONSchemaProps specSchema, int expected) { + Map spec = specSchema.getProperties(); + assertEquals(expected, spec.size()); + return spec; + } + + private static void assertPropertyHasType(JSONSchemaProps spec, String name, String expectedType) { + Map properties = spec.getProperties(); + assertNotNull(properties); + JSONSchemaProps property = properties.get(name); + assertNotNull(property, "Property " + name + " should exist"); + assertEquals(expectedType, property.getType(), "Property " + name + " should have expected type"); } } diff --git a/crd-generator/test/src/test/java/io/fabric8/crd/generator/schemaswaps/MultipleSchemaSwaps.java b/crd-generator/test/src/test/java/io/fabric8/crd/generator/schemaswaps/MultipleSchemaSwaps.java new file mode 100644 index 00000000000..8d841c8b977 --- /dev/null +++ b/crd-generator/test/src/test/java/io/fabric8/crd/generator/schemaswaps/MultipleSchemaSwaps.java @@ -0,0 +1,30 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.generator.schemaswaps; + +import io.fabric8.crd.generator.annotation.SchemaSwap; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.Version; + +@Version("v1") +@Group("acme.com") +@SchemaSwap(originalType = SchemaSwapSpec.SomeObject.class, fieldName = "shouldBeString", targetType = String.class) +@SchemaSwap(originalType = SchemaSwapSpec.AnotherObject.class, fieldName = "shouldBeInt", targetType = Integer.class) +@SchemaSwap(originalType = SchemaSwapSpec.YetAnotherObject.class, fieldName = "shouldBeSkipped") +public class MultipleSchemaSwaps extends CustomResource { + +} diff --git a/crd-generator/test/src/test/java/io/fabric8/crd/generator/schemaswaps/SchemaSwapCRDTest.java b/crd-generator/test/src/test/java/io/fabric8/crd/generator/schemaswaps/SchemaSwapCRDTest.java new file mode 100644 index 00000000000..11db80a98b9 --- /dev/null +++ b/crd-generator/test/src/test/java/io/fabric8/crd/generator/schemaswaps/SchemaSwapCRDTest.java @@ -0,0 +1,75 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.generator.schemaswaps; + +import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceDefinition; +import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceDefinitionVersion; +import io.fabric8.kubernetes.api.model.apiextensions.v1.JSONSchemaProps; +import io.fabric8.kubernetes.client.utils.Serialization; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class SchemaSwapCRDTest { + + @Test + void testCrd() { + CustomResourceDefinition d = Serialization.unmarshal(getClass().getClassLoader() + .getResourceAsStream("META-INF/fabric8/multipleschemaswaps.acme.com-v1.yml"), + CustomResourceDefinition.class); + assertNotNull(d); + + CustomResourceDefinitionVersion v1 = d.getSpec().getVersions().get(0); + assertNotNull(v1); + assertEquals("v1", v1.getName()); + Map spec = v1.getSchema().getOpenAPIV3Schema().getProperties().get("spec").getProperties(); + assertNotNull(spec); + + // 'first' is replaced by SchemaSwap from int to string + JSONSchemaProps first = spec.get("first"); + Map firstProps = first.getProperties(); + assertNotNull(firstProps); + JSONSchemaProps firstProperty = firstProps.get("shouldBeString"); + assertEquals("string", firstProperty.getType()); + + // 'second' is replaced by the same SchemaSwap that is applied multiple times + JSONSchemaProps second = spec.get("second"); + Map secondProps = second.getProperties(); + assertNotNull(secondProps); + JSONSchemaProps secondProperty = secondProps.get("shouldBeString"); + assertEquals("string", secondProperty.getType()); + + // 'third' is replaced by another SchemaSwap + JSONSchemaProps third = spec.get("third"); + Map thirdProps = third.getProperties(); + assertNotNull(thirdProps); + JSONSchemaProps thirdProperty = thirdProps.get("shouldBeInt"); + assertEquals("integer", thirdProperty.getType()); + + // 'fourth' is replaced by another SchemaSwap and its property deleted + JSONSchemaProps fourth = spec.get("fourth"); + Map fourthProps = fourth.getProperties(); + assertNotNull(fourthProps); + assertTrue(fourthProps.isEmpty()); + } + +} diff --git a/crd-generator/test/src/test/java/io/fabric8/crd/generator/schemaswaps/SchemaSwapSpec.java b/crd-generator/test/src/test/java/io/fabric8/crd/generator/schemaswaps/SchemaSwapSpec.java new file mode 100644 index 00000000000..ac20d16131d --- /dev/null +++ b/crd-generator/test/src/test/java/io/fabric8/crd/generator/schemaswaps/SchemaSwapSpec.java @@ -0,0 +1,35 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.generator.schemaswaps; + +public class SchemaSwapSpec { + private SomeObject first; + private SomeObject second; + private AnotherObject third; + private YetAnotherObject fourth; + + static class SomeObject { + private int shouldBeString; + } + + static class AnotherObject { + private String shouldBeInt; + } + + static class YetAnotherObject { + private String shouldBeSkipped; + } +} diff --git a/doc/CRD-generator.md b/doc/CRD-generator.md index 3a69a2fdcb7..3499eff7966 100644 --- a/doc/CRD-generator.md +++ b/doc/CRD-generator.md @@ -225,6 +225,8 @@ The CRD generator will perform the same substitution as a `SchemaFrom` annotatio type: object ``` +The name of the field is restricted to the original `fieldName` and should be backed by a matching concrete field of the matching class. Getters, setters, and constructors are not taken into consideration. + ### Generating `x-kubernetes-preserve-unknown-fields: true` If a field or one of its accessors is annotated with