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.

Sign up for GitHub

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

Jump to bottom

Introduce specific annotations for the generators #4348

Merged
merged 2 commits into from Sep 5, 2022
Merged

Introduce specific annotations for the generators #4348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2cfb20
Introduce specific annotations for the generators
andreaTP Aug 24, 2022
80d38b5
add javadocs
andreaTP Aug 26, 2022
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
5 changes: 5 additions & 0 deletions crd-generator/api/pom.xml
View file
Open in desktop
Expand Up @@ -36,6 +36,11 @@
<scope>compile</scope>
</dependency>

<dependency>
<groupId>io.fabric8</groupId>
<artifactId>generator-annotations</artifactId>
</dependency>

<dependency>
<groupId>io.fabric8</groupId>
<artifactId>kubernetes-model-common</artifactId>
Expand Down
135 changes: 129 additions & 6 deletions crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java
View file
Open in desktop
Expand Up @@ -17,7 +17,6 @@

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
import io.fabric8.crd.generator.annotation.SchemaSwap;
import io.fabric8.crd.generator.utils.Types;
import io.fabric8.kubernetes.api.model.Duration;
import io.fabric8.kubernetes.api.model.IntOrString;
Expand Down Expand Up @@ -60,6 +59,8 @@ public abstract class AbstractJsonSchema<T, B> {
protected static final TypeDef DATE = TypeDef.forName(Date.class.getName());
protected static final TypeRef DATE_REF = DATE.toReference();

private static final String VALUE = "value";

private static final String INT_OR_STRING_MARKER = "int_or_string";
private static final String STRING_MARKER = "string";
private static final String INTEGER_MARKER = "integer";
Expand All @@ -78,6 +79,11 @@ public abstract class AbstractJsonSchema<T, B> {
public static final String ANNOTATION_JSON_IGNORE = "com.fasterxml.jackson.annotation.JsonIgnore";
public static final String ANNOTATION_JSON_ANY_GETTER = "com.fasterxml.jackson.annotation.JsonAnyGetter";
public static final String ANNOTATION_JSON_ANY_SETTER = "com.fasterxml.jackson.annotation.JsonAnySetter";
public static final String ANNOTATION_MIN = "io.fabric8.generator.annotation.Min";
Copy link
Collaborator

Choose a reason for hiding this comment

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

All annotations should be put in the same package, imo.

Copy link
Member Author

@andreaTP andreaTP Aug 26, 2022 •
edited

Choose a reason for hiding this comment

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

At this point we have annotations in 2 places:

  • io.fabric8.generator.annotation -> generic modifiers to represent CRD properties in Java code
  • io.fabric8.crd.generator.annotation -> specific annotations to tweak specifically the crd-generator

Do you see any other invariant @metacosm ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

As soon as the annotations are used in different contexts, it would be better for users to help discoverability, in particular, for the annotations to be in the same location even though they initially were developed for different purposes… Obviously, changing annotations location at this point wouldn't be backwards compatible so maybe just a matter of documentation? It might make sense to explain why they live in different packages so that users (and future maintainers, down the line 😸) don't think it's an arbitrary decision.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a brief JavaDoc on each annotation, let me know if you want other/more changes for this 🙂

public static final String ANNOTATION_MAX = "io.fabric8.generator.annotation.Max";
public static final String ANNOTATION_PATTERN = "io.fabric8.generator.annotation.Pattern";
public static final String ANNOTATION_NULLABLE = "io.fabric8.generator.annotation.Nullable";
public static final String ANNOTATION_REQUIRED = "io.fabric8.generator.annotation.Required";
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";
Expand Down Expand Up @@ -110,6 +116,51 @@ public static String getSchemaTypeFor(TypeRef typeRef) {
return type;
}

protected static class SchemaPropsOptions {
final Optional<Double> min;
final Optional<Double> max;
final Optional<String> pattern;
final boolean nullable;
final boolean required;

SchemaPropsOptions() {
min = Optional.empty();
max = Optional.empty();
pattern = Optional.empty();
nullable = false;
required = false;
}

public SchemaPropsOptions(Optional<Double> min, Optional<Double> max, Optional<String> pattern,
boolean nullable, boolean required) {
this.min = min;
this.max = max;
this.pattern = pattern;
this.nullable = nullable;
this.required = required;
}

public Optional<Double> getMin() {
return min;
}

public Optional<Double> getMax() {
return max;
}

public Optional<String> getPattern() {
return pattern;
}

public boolean isNullable() {
return nullable;
}

public boolean getRequired() {
return nullable;
}
}

/**
* Creates the JSON schema for the particular {@link TypeDef}. This is template method where
* sub-classes are supposed to provide specific implementations of abstract methods.
Expand Down Expand Up @@ -264,7 +315,15 @@ private T internalFromImpl(TypeDef definition, Set<String> visited, List<Interna
} else {
possiblyUpdatedSchema = addDescription(schema, description);
}
addProperty(possiblyRenamedProperty, builder, possiblyUpdatedSchema);

SchemaPropsOptions options = new SchemaPropsOptions(
facade.min,
facade.max,
facade.pattern,
facade.nullable,
facade.required);

addProperty(possiblyRenamedProperty, builder, possiblyUpdatedSchema, options);
}

validateRemainingSchemaSwaps("unmatched field", currentSchemaSwaps.stream().collect(Collectors.toList()));
Expand All @@ -286,6 +345,10 @@ private static class PropertyOrAccessor {
private final String propertyName;
private final String type;
private String renamedTo;
private Optional<Double> min;
private Optional<Double> max;
private Optional<String> pattern;
private boolean nullable;
private boolean required;
private boolean ignored;
private boolean preserveUnknownFields;
Expand All @@ -297,6 +360,10 @@ private PropertyOrAccessor(Collection<AnnotationRef> annotations, String name, S
this.name = name;
this.propertyName = propertyName;
type = isMethod ? "accessor" : "field";

min = Optional.empty();
max = Optional.empty();
pattern = Optional.empty();
}

static PropertyOrAccessor fromProperty(Property property) {
Expand All @@ -310,17 +377,34 @@ static PropertyOrAccessor fromMethod(Method method, String propertyName) {
public void process() {
annotations.forEach(a -> {
switch (a.getClassRef().getFullyQualifiedName()) {
case ANNOTATION_NULLABLE:
nullable = true;
break;
case ANNOTATION_MAX:
max = Optional.of((Double) a.getParameters().get(VALUE));
break;
case ANNOTATION_MIN:
min = Optional.of((Double) a.getParameters().get(VALUE));
break;
case ANNOTATION_PATTERN:
pattern = Optional.of((String) a.getParameters().get(VALUE));
break;
case ANNOTATION_NOT_NULL:
LOGGER.warn("Annotation: {} on property: {} is deprecated. Please use: {} instead", ANNOTATION_NOT_NULL, name,
ANNOTATION_REQUIRED);
required = true;
break;
case ANNOTATION_REQUIRED:
required = true;
break;
case ANNOTATION_JSON_PROPERTY:
final String nameFromAnnotation = (String) a.getParameters().get("value");
final String nameFromAnnotation = (String) a.getParameters().get(VALUE);
if (!Strings.isNullOrEmpty(nameFromAnnotation) && !propertyName.equals(nameFromAnnotation)) {
renamedTo = nameFromAnnotation;
}
break;
case ANNOTATION_JSON_PROPERTY_DESCRIPTION:
final String descriptionFromAnnotation = (String) a.getParameters().get("value");
final String descriptionFromAnnotation = (String) a.getParameters().get(VALUE);
if (!Strings.isNullOrEmpty(descriptionFromAnnotation)) {
description = descriptionFromAnnotation;
}
Expand All @@ -343,6 +427,22 @@ public String getRenamedTo() {
return renamedTo;
}

public boolean isNullable() {
return nullable;
}

public Optional<Double> getMax() {
return max;
}

public Optional<Double> getMin() {
return min;
}

public Optional<String> getPattern() {
return pattern;
}

public boolean isRequired() {
return required;
}
Expand Down Expand Up @@ -387,6 +487,10 @@ private static class PropertyFacade {
private final Set<InternalSchemaSwap> matchedSchemaSwaps;
private String renamedTo;
private String description;
private Optional<Double> min;
private Optional<Double> max;
private Optional<String> pattern;
private boolean nullable;
private boolean required;
private boolean ignored;
private boolean preserveUnknownFields;
Expand Down Expand Up @@ -414,6 +518,9 @@ public PropertyFacade(Property property, Map<String, Method> potentialAccessors,
if (method != null) {
propertyOrAccessors.add(PropertyOrAccessor.fromMethod(method, name));
}
min = Optional.empty();
max = Optional.empty();
pattern = Optional.empty();
}

public Property process() {
Expand Down Expand Up @@ -450,6 +557,22 @@ public Property process() {
}
}

if (p.getMin().isPresent()) {
min = p.getMin();
}

if (p.getMax().isPresent()) {
max = p.getMax();
}

if (p.getPattern().isPresent()) {
pattern = p.getPattern();
}

if (p.isNullable()) {
nullable = true;
}

if (p.isRequired()) {
required = true;
} else if (p.isIgnored()) {
Expand Down Expand Up @@ -502,7 +625,7 @@ private String extractUpdatedNameFromJacksonPropertyIfPresent(Property property)
.findAny()
// if we found an annotated accessor, override the property's name if needed
.map(a -> {
final String fromAnnotation = (String) a.getParameters().get("value");
final String fromAnnotation = (String) a.getParameters().get(VALUE);
if (!Strings.isNullOrEmpty(fromAnnotation) && !name.equals(fromAnnotation)) {
return fromAnnotation;
} else {
Expand All @@ -527,7 +650,7 @@ private String extractUpdatedNameFromJacksonPropertyIfPresent(Property property)
* @param builder the builder representing the schema being built
* @param schema the built schema for the property being added
*/
public abstract void addProperty(Property property, B builder, T schema);
public abstract void addProperty(Property property, B builder, T schema, SchemaPropsOptions options);

/**
* Finishes up the process by actually building the final JSON schema based on the provided
Expand Down
5 changes: 4 additions & 1 deletion crd-generator/api/src/main/java/io/fabric8/crd/generator/annotation/SchemaFrom.java
View file
Open in desktop
Expand Up @@ -17,7 +17,10 @@

import java.lang.annotation.*;

@Target({ElementType.ANNOTATION_TYPE, ElementType.FIELD})
/*
* Used to tweak the behavior of the crd-generator
*/
@Target({ ElementType.ANNOTATION_TYPE, ElementType.FIELD })
@Retention(RetentionPolicy.RUNTIME)
public @interface SchemaFrom {
Class<?> type() default void.class;
Expand Down
7 changes: 6 additions & 1 deletion crd-generator/api/src/main/java/io/fabric8/crd/generator/annotation/SchemaSwap.java
View file
Open in desktop
Expand Up @@ -17,10 +17,15 @@

import java.lang.annotation.*;

@Target({ElementType.ANNOTATION_TYPE, ElementType.TYPE_USE, ElementType.TYPE})
/*
* Used to tweak the behavior of the crd-generator
*/
@Target({ ElementType.ANNOTATION_TYPE, ElementType.TYPE_USE, ElementType.TYPE })
@Retention(RetentionPolicy.RUNTIME)
public @interface SchemaSwap {
Class<?> originalType();

String fieldName();

Class<?> targetType() default void.class;
}
45 changes: 27 additions & 18 deletions crd-generator/api/src/main/java/io/fabric8/crd/generator/v1/JsonSchema.java
View file
Open in desktop
Expand Up @@ -22,18 +22,19 @@
import io.sundr.model.Property;
import io.sundr.model.TypeDef;
import io.sundr.model.TypeRef;

import java.util.List;

public class JsonSchema extends AbstractJsonSchema<JSONSchemaProps, JSONSchemaPropsBuilder> {

private static final JsonSchema instance = new JsonSchema();

private static final JSONSchemaProps JSON_SCHEMA_INT_OR_STRING = new JSONSchemaPropsBuilder()
.withXKubernetesIntOrString(true)
.withAnyOf(
new JSONSchemaPropsBuilder().withType("integer").build(),
new JSONSchemaPropsBuilder().withType("string").build())
.build();
.withXKubernetesIntOrString(true)
.withAnyOf(
new JSONSchemaPropsBuilder().withType("integer").build(),
new JSONSchemaPropsBuilder().withType("string").build())
.build();

/**
* Creates the JSON schema for the particular {@link TypeDef}.
Expand All @@ -55,8 +56,16 @@ public JSONSchemaPropsBuilder newBuilder() {

@Override
public void addProperty(Property property, JSONSchemaPropsBuilder builder,
JSONSchemaProps schema) {
JSONSchemaProps schema, SchemaPropsOptions options) {
if (schema != null) {
options.getMin().ifPresent(schema::setMinimum);
options.getMax().ifPresent(schema::setMaximum);
options.getPattern().ifPresent(schema::setPattern);

if (options.isNullable()) {
schema.setNullable(true);
}

builder.addToProperties(property.getName(), schema);
}
}
Expand All @@ -73,21 +82,21 @@ public JSONSchemaProps build(JSONSchemaPropsBuilder builder, List<String> requir
@Override
protected JSONSchemaProps arrayLikeProperty(JSONSchemaProps schema) {
return new JSONSchemaPropsBuilder()
.withType("array")
.withNewItems()
.withSchema(schema)
.and()
.build();
.withType("array")
.withNewItems()
.withSchema(schema)
.and()
.build();
}

@Override
protected JSONSchemaProps mapLikeProperty(JSONSchemaProps schema) {
return new JSONSchemaPropsBuilder()
.withType("object")
.withNewAdditionalProperties()
.withSchema(schema)
.endAdditionalProperties()
.build();
.withType("object")
.withNewAdditionalProperties()
.withSchema(schema)
.endAdditionalProperties()
.build();
}

@Override
Expand All @@ -108,7 +117,7 @@ protected JSONSchemaProps enumProperty(JsonNode... enumValues) {
@Override
protected JSONSchemaProps addDescription(JSONSchemaProps schema, String description) {
return new JSONSchemaPropsBuilder(schema)
.withDescription(description)
.build();
.withDescription(description)
.build();
}
}