Skip to content

Commit

Permalink
fix fabric8io#3625: updating the default logic for maps and objectmet…
Browse files Browse the repository at this point in the history
…adata

fixing format
  • Loading branch information
shawkins committed May 18, 2022
1 parent 51cc99a commit c896128
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 44 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#### Improvements
* Remove `setIntVal`, `setStrVal`, `setKind` setters from `IntOrString` class to avoid invalid combinations
* Fix #3625: ObjectMetadata and its annotations and labels are non-null by default.
* Fix #3852: Deserializing kamelets fails with UnrecognizedPropertyException
* Fix #3889 : remove piped stream for file download
* Fix #1285: removed references to manually calling registerCustomKind
Expand Down
10 changes: 10 additions & 0 deletions doc/MIGRATION-v6.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Migration from 5.x to 6.x

## Contents:
- [ObjectMetadata](#objectmetadata)
- [Backwards Compatibility Interceptor](#backwards-compatibility-interceptor)
- [Namespace Changes](#namespace-changes)
- [API/Impl split](#api-impl-split)
Expand All @@ -19,6 +20,15 @@
- [Delete Behavior](#delete-behavior)
- [Stream Changes](#stream-changes)

## ObjectMetadata

ObjectMetadata will default to a non-null value. All Maps in the generated object models, including ObjectMetadata.annotations and labels will also be non-null by default. A small behavioral difference is that the default value will be omitted from serialization, previously if you did something like
```
Pod p = new Pod();
p.setMetadata(new ObjectMetadata());
```
p would serialize with "metadata: {}" - that will now be omitted.

## Backwards Compatibility Interceptor

kubernetes.backwardsCompatibilityInterceptor.disable now defaults to true, rather than false. If you need backwards compatibility support, please set this property to false.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,36 +46,36 @@
*
* Properties are set up automatically as follows:
* <ul>
* <li>group is set using {@link io.fabric8.kubernetes.api.model.HasMetadata#getGroup(Class)}</li>
* <li>version is set using {@link io.fabric8.kubernetes.api.model.HasMetadata#getVersion(Class)}</li>
* <li>singular is set using {@link io.fabric8.kubernetes.api.model.HasMetadata#getSingular(Class)}</li>
* <li>plural is set using {@link io.fabric8.kubernetes.api.model.HasMetadata#getPlural(Class)}</li>
* <li>computed CRD name using {@link CustomResource#getCRDName(Class)}</li>
* <li>group is set using {@link io.fabric8.kubernetes.api.model.HasMetadata#getGroup(Class)}</li>
* <li>version is set using {@link io.fabric8.kubernetes.api.model.HasMetadata#getVersion(Class)}</li>
* <li>singular is set using {@link io.fabric8.kubernetes.api.model.HasMetadata#getSingular(Class)}</li>
* <li>plural is set using {@link io.fabric8.kubernetes.api.model.HasMetadata#getPlural(Class)}</li>
* <li>computed CRD name using {@link CustomResource#getCRDName(Class)}</li>
* </ul>
*
* In addition, {@link CustomResource#setApiVersion(String)} and {@link CustomResource#setKind(String)} are overridden to not do anything since these values
* In addition, {@link CustomResource#setApiVersion(String)} and {@link CustomResource#setKind(String)} are overridden to not do
* anything since these values
* are set.
*
* @param <S> the class providing the {@code Spec} part of this CustomResource
* @param <T> the class providing the {@code Status} part of this CustomResource
*/
@JsonDeserialize(
using = JsonDeserializer.None.class
)
@JsonDeserialize(using = JsonDeserializer.None.class)
@JsonPropertyOrder({
"apiVersion",
"kind",
"metadata",
"spec",
"status"
"apiVersion",
"kind",
"metadata",
"spec",
"status"
})
@JsonInclude(Include.NON_NULL)
@Buildable(editableEnabled = false, validationEnabled = false, generateBuilderPackage = false, lazyCollectionInitEnabled = false, builderPackage = "io.fabric8.kubernetes.api.builder", refs = {
@BuildableReference(io.fabric8.kubernetes.api.model.ObjectMeta.class),
@BuildableReference(io.fabric8.kubernetes.api.model.ObjectMeta.class),
})
public abstract class CustomResource<S, T> implements HasMetadata {
private static final Logger LOG = LoggerFactory.getLogger(CustomResource.class);

@JsonInclude(value = Include.CUSTOM, valueFilter = ObjectMeta.class)
private ObjectMeta metadata = new ObjectMeta();

@JsonProperty("spec")
Expand All @@ -100,7 +100,7 @@ public CustomResource() {
final Class<? extends CustomResource> clazz = getClass();
if (isNullOrEmpty(version)) {
throw new IllegalArgumentException(clazz.getName() + " CustomResource must provide an API version using @"
+ Group.class.getName() + " and @" + Version.class.getName() + " annotations");
+ Group.class.getName() + " and @" + Version.class.getName() + " annotations");
}
this.apiVersion = version;
this.kind = HasMetadata.super.getKind();
Expand All @@ -126,15 +126,19 @@ public static boolean getStorage(Class<? extends CustomResource> clazz) {

/**
* Override to provide your own Spec instance
* @return a new Spec instance or {@code null} if the responsibility of instantiating the Spec is left to users of this CustomResource
*
* @return a new Spec instance or {@code null} if the responsibility of instantiating the Spec is left to users of this
* CustomResource
*/
protected S initSpec() {
return null;
}

/**
* Override to provide your own Status instance
* @return a new Status instance or {@code null} if the responsibility of instantiating the Status is left to users of this CustomResource
*
* @return a new Status instance or {@code null} if the responsibility of instantiating the Status is left to users of this
* CustomResource
*/
protected T initStatus() {
return null;
Expand All @@ -143,12 +147,12 @@ protected T initStatus() {
@Override
public String toString() {
return "CustomResource{" +
"kind='" + getKind() + '\'' +
", apiVersion='" + getApiVersion() + '\'' +
", metadata=" + metadata +
", spec=" + spec +
", status=" + status +
'}';
"kind='" + getKind() + '\'' +
", apiVersion='" + getApiVersion() + '\'' +
", metadata=" + metadata +
", spec=" + spec +
", status=" + status +
'}';
}

@Override
Expand All @@ -159,7 +163,8 @@ public String getApiVersion() {
@Override
public void setApiVersion(String version) {
// already set in constructor
LOG.debug("Calling CustomResource#setApiVersion doesn't do anything because the API version is computed and shouldn't be changed");
LOG.debug(
"Calling CustomResource#setApiVersion doesn't do anything because the API version is computed and shouldn't be changed");
}

@Override
Expand Down Expand Up @@ -190,6 +195,7 @@ public static String getPlural(Class<?> clazz) {
return HasMetadata.getPlural(clazz);
}

@Override
@JsonIgnore
public String getPlural() {
return plural;
Expand All @@ -203,6 +209,7 @@ public static String getSingular(Class<?> clazz) {
return HasMetadata.getSingular(clazz);
}

@Override
@JsonIgnore
public String getSingular() {
return singular;
Expand Down Expand Up @@ -232,8 +239,8 @@ public String getCRDName() {
*/
public static String[] getShortNames(Class<? extends CustomResource> clazz) {
return Optional.ofNullable(clazz.getAnnotation(ShortNames.class))
.map(ShortNames::value)
.orElse(new String[]{});
.map(ShortNames::value)
.orElse(new String[] {});
}

/**
Expand Down Expand Up @@ -284,21 +291,33 @@ public void setStatus(T status) {

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof CustomResource)) return false;
if (this == o)
return true;
if (!(o instanceof CustomResource))
return false;

CustomResource<?, ?> that = (CustomResource<?, ?>) o;

if (served != that.served) return false;
if (storage != that.storage) return false;
if (!metadata.equals(that.metadata)) return false;
if (!Objects.equals(spec, that.spec)) return false;
if (!Objects.equals(status, that.status)) return false;
if (!singular.equals(that.singular)) return false;
if (!crdName.equals(that.crdName)) return false;
if (!kind.equals(that.kind)) return false;
if (!apiVersion.equals(that.apiVersion)) return false;
if (!scope.equals(that.scope)) return false;
if (served != that.served)
return false;
if (storage != that.storage)
return false;
if (!metadata.equals(that.metadata))
return false;
if (!Objects.equals(spec, that.spec))
return false;
if (!Objects.equals(status, that.status))
return false;
if (!singular.equals(that.singular))
return false;
if (!crdName.equals(that.crdName))
return false;
if (!kind.equals(that.kind))
return false;
if (!apiVersion.equals(that.apiVersion))
return false;
if (!scope.equals(that.scope))
return false;
return plural.equals(that.plural);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.fasterxml.jackson.annotation.JsonAnySetter;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonInclude.Include;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import com.fasterxml.jackson.databind.JsonNode;
Expand Down Expand Up @@ -56,7 +57,8 @@ public class GenericKubernetesResource implements HasMetadata {
@JsonProperty("kind")
private String kind;
@JsonProperty("metadata")
private ObjectMeta metadata;
@JsonInclude(value = Include.CUSTOM, valueFilter = ObjectMeta.class)
private ObjectMeta metadata = new ObjectMeta();
@JsonIgnore
private Map<String, Object> additionalProperties = new LinkedHashMap<>();

Expand Down Expand Up @@ -130,7 +132,7 @@ public <T> T get(Object... path) {

/**
* The same as {@link #get(Object...)}, but starting at any root raw object
*
*
* @param <T> type of the returned object (Map, Collection, or value).
* @param root starting object
* @param path of the field to retrieve.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/**
* 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.kubernetes.jsonschema2pojo;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.databind.JsonNode;
import com.sun.codemodel.JClass;
import com.sun.codemodel.JExpr;
import com.sun.codemodel.JFieldVar;
import com.sun.codemodel.JType;
import org.jsonschema2pojo.Schema;
import org.jsonschema2pojo.rules.DefaultRule;
import org.jsonschema2pojo.rules.RuleFactory;

import java.util.LinkedHashMap;
import java.util.Map;

/**
* Adds default map and objectmeta handling
*/
public class Fabric8DefaultRule extends DefaultRule {

public static final String OBJECTMETA = "io.fabric8.kubernetes.api.model.ObjectMeta";
public static final String ANNOTATION_VALUE_FILTER = "valueFilter";

private final RuleFactory ruleFactory;

public Fabric8DefaultRule(RuleFactory ruleFactory) {
super(ruleFactory);
this.ruleFactory = ruleFactory;
}

@Override
public JFieldVar apply(String nodeName, JsonNode node, JsonNode parent, JFieldVar field, Schema currentSchema) {
JType fieldType = field.type();
String fieldTypeName = fieldType.fullName();

if (node == null || node.asText() == null || node.asText().isEmpty()) {
if (ruleFactory.getGenerationConfig().isInitializeCollections() && fieldTypeName.startsWith(Map.class.getName())) {
JClass keyGenericType = ((JClass) fieldType).getTypeParameters().get(0);
JClass valueGenericType = ((JClass) fieldType).getTypeParameters().get(1);

JClass mapImplClass = fieldType.owner().ref(LinkedHashMap.class);
mapImplClass = mapImplClass.narrow(keyGenericType, valueGenericType);
// maps are not marked as omitJavaEmpty - it's simplest to just add the annotation here, rather than updating the generator
field.annotate(JsonInclude.class).param(KubernetesCoreTypeAnnotator.ANNOTATION_VALUE, JsonInclude.Include.NON_EMPTY);
field.init(JExpr._new(mapImplClass));
}

if (fieldTypeName.equals(OBJECTMETA)) {
JClass implClass = fieldType.owner().ref(fieldTypeName);
// annotate to omit the default
field.annotate(JsonInclude.class).param(KubernetesCoreTypeAnnotator.ANNOTATION_VALUE, JsonInclude.Include.CUSTOM)
.param(ANNOTATION_VALUE_FILTER, implClass);
field.init(JExpr._new(implClass));
}

}

return super.apply(nodeName, node, parent, field, currentSchema);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/
package io.fabric8.kubernetes.jsonschema2pojo;

import com.sun.codemodel.JFieldVar;
import com.sun.codemodel.JPackage;
import com.sun.codemodel.JType;
import org.jsonschema2pojo.DefaultGenerationConfig;
import org.jsonschema2pojo.Jackson2Annotator;
import org.jsonschema2pojo.SchemaStore;
Expand All @@ -23,9 +26,6 @@
import org.jsonschema2pojo.util.NameHelper;
import org.jsonschema2pojo.util.ParcelableHelper;

import com.sun.codemodel.JPackage;
import com.sun.codemodel.JType;

public class Fabric8RuleFactory extends RuleFactory {

private final Fabric8NameHelper nameHelper;
Expand All @@ -44,4 +44,9 @@ public NameHelper getNameHelper() {
public Rule<JPackage, JType> getObjectRule() {
return new Fabric8ObjectRule(this, new ParcelableHelper(), getReflectionHelper());
}

@Override
public Rule<JFieldVar, JFieldVar> getDefaultRule() {
return new Fabric8DefaultRule(this);
}
}

0 comments on commit c896128

Please sign in to comment.