From 84dbf247e60b7f3a595ada051461a7da35e67547 Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Wed, 29 Mar 2023 10:53:59 -0400 Subject: [PATCH] fix #5009: ensuring underlying serialization is used --- CHANGELOG.md | 1 + .../SerializationWrappedPolymorphicTest.java | 119 ++++++++++++++++++ .../jackson/BeanPropertyWriterDelegate.java | 14 +-- .../jackson/SettableBeanPropertyDelegate.java | 3 - .../jackson/UnmatchedFieldTypeModule.java | 6 + 5 files changed, 131 insertions(+), 12 deletions(-) create mode 100644 kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/SerializationWrappedPolymorphicTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 03f502257be..db7e45e20e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * Fix #4963: Openshift Client return 403 when use websocket * Fix #4985: triggering the immediate cleanup of the okhttp idle task * fix #5002: Jetty response completion accounts for header processing +* Fix #5009: addressing issue with serialization of wrapped polymophic types #### Improvements * Fix #4434: Update CronJobIT to use `batch/v1` CronJob instead diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/SerializationWrappedPolymorphicTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/SerializationWrappedPolymorphicTest.java new file mode 100644 index 00000000000..3ce9fe44619 --- /dev/null +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/SerializationWrappedPolymorphicTest.java @@ -0,0 +1,119 @@ +/** + * 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.client.utils; + +import com.fasterxml.jackson.annotation.JsonAnyGetter; +import com.fasterxml.jackson.annotation.JsonAnySetter; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.annotation.JsonTypeName; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import io.fabric8.kubernetes.api.model.KubernetesResource; +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.Version; +import lombok.Data; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; + +class SerializationWrappedPolymorphicTest { + + @Group("example.com") + @Version("v1alpha1") + static class TestCR extends CustomResource implements Namespaced { + + @JsonDeserialize(using = JsonDeserializer.None.class) + static class TestCRSpec implements KubernetesResource { + + @JsonIgnore + private Map additionalProperties = new HashMap(); + + private List children = new ArrayList<>(); + + public List getChildren() { + return children; + } + + public void setChildren(List children) { + this.children = children; + } + + @JsonAnyGetter + public Map getAdditionalProperties() { + return this.additionalProperties; + } + + @JsonAnySetter + public void setAdditionalProperty(String name, Object value) { + this.additionalProperties.put(name, value); + } + } + + @Override + protected TestCRSpec initSpec() { + return new TestCRSpec(); + } + } + + @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.WRAPPER_OBJECT) + @JsonSubTypes({ // + @JsonSubTypes.Type(FooChild.class), // + @JsonSubTypes.Type(BarChild.class), // + }) + @JsonDeserialize(using = JsonDeserializer.None.class) + interface Child extends KubernetesResource { + } + + @JsonTypeName("foo") + @Data + static class FooChild implements Child { + private String name; + } + + @JsonTypeName("bar") + @Data + static class BarChild implements Child { + private String file; + } + + @Test + void testPolyRoundTrip() { + TestCR original = new TestCR(); + FooChild foo = new FooChild(); + foo.setName("alice"); + BarChild bar = new BarChild(); + bar.setFile("bob"); + original.getSpec().setChildren(Arrays.asList(foo, bar)); + String json = Serialization.asJson(original); + assertThat(json).isEqualTo( + "{\"apiVersion\":\"example.com/v1alpha1\",\"kind\":\"TestCR\",\"metadata\":{},\"spec\":{\"children\":[{\"foo\":{\"name\":\"alice\"}},{\"bar\":{\"file\":\"bob\"}}]}}"); + TestCR deserialized = Serialization.unmarshal(json, TestCR.class); + assertEquals(deserialized.getSpec().getChildren(), original.getSpec().getChildren()); + } + +} diff --git a/kubernetes-model-generator/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/jackson/BeanPropertyWriterDelegate.java b/kubernetes-model-generator/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/jackson/BeanPropertyWriterDelegate.java index 7f73c2a76bc..84333e9af75 100644 --- a/kubernetes-model-generator/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/jackson/BeanPropertyWriterDelegate.java +++ b/kubernetes-model-generator/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/jackson/BeanPropertyWriterDelegate.java @@ -42,13 +42,11 @@ public class BeanPropertyWriterDelegate extends BeanPropertyWriter { private static final Logger logger = LoggerFactory.getLogger(BeanPropertyWriterDelegate.class); - private final BeanPropertyWriter delegate; private final AnnotatedMember anyGetter; private final transient Supplier logDuplicateWarning; BeanPropertyWriterDelegate(BeanPropertyWriter delegate, AnnotatedMember anyGetter, Supplier logDuplicateWarning) { super(delegate); - this.delegate = delegate; this.anyGetter = anyGetter; this.logDuplicateWarning = logDuplicateWarning; } @@ -56,17 +54,15 @@ public class BeanPropertyWriterDelegate extends BeanPropertyWriter { @Override public void serializeAsField(Object bean, JsonGenerator gen, SerializerProvider prov) throws Exception { Object valueInAnyGetter = null; - if (anyGetter != null) { - Object anyGetterValue = anyGetter.getValue(bean); - if (anyGetterValue != null) { - valueInAnyGetter = ((Map) anyGetterValue).get(delegate.getName()); - } + Object anyGetterValue = anyGetter.getValue(bean); + if (anyGetterValue != null) { + valueInAnyGetter = ((Map) anyGetterValue).get(getName()); } if (valueInAnyGetter == null) { - delegate.serializeAsField(bean, gen, prov); + super.serializeAsField(bean, gen, prov); } else if (Boolean.TRUE.equals(logDuplicateWarning.get())) { logger.warn("Value in field '{}' ignored in favor of value in additionalProperties ({}) for {}", - delegate.getName(), valueInAnyGetter, bean.getClass().getName()); + getName(), valueInAnyGetter, bean.getClass().getName()); } } } diff --git a/kubernetes-model-generator/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/jackson/SettableBeanPropertyDelegate.java b/kubernetes-model-generator/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/jackson/SettableBeanPropertyDelegate.java index d93881617ab..2dabccd9cd2 100644 --- a/kubernetes-model-generator/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/jackson/SettableBeanPropertyDelegate.java +++ b/kubernetes-model-generator/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/jackson/SettableBeanPropertyDelegate.java @@ -171,9 +171,6 @@ public Object setAndReturn(Object instance, Object value) throws IOException { } private boolean shouldUseAnySetter() { - if (anySetter == null) { - return false; - } return useAnySetter.getAsBoolean(); } } diff --git a/kubernetes-model-generator/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/jackson/UnmatchedFieldTypeModule.java b/kubernetes-model-generator/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/jackson/UnmatchedFieldTypeModule.java index 83d28f72d91..84b4e48bce5 100644 --- a/kubernetes-model-generator/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/jackson/UnmatchedFieldTypeModule.java +++ b/kubernetes-model-generator/kubernetes-model-common/src/main/java/io/fabric8/kubernetes/model/jackson/UnmatchedFieldTypeModule.java @@ -54,6 +54,9 @@ public UnmatchedFieldTypeModule(boolean logWarnings, boolean restrictToTemplates @Override public BeanDeserializerBuilder updateBuilder(DeserializationConfig config, BeanDescription beanDesc, BeanDeserializerBuilder builder) { + if (builder.getAnySetter() == null) { + return builder; + } builder.getProperties().forEachRemaining(p -> builder.addOrReplaceProperty( new SettableBeanPropertyDelegate(p, builder.getAnySetter(), UnmatchedFieldTypeModule.this::useAnySetter) { }, true)); @@ -64,6 +67,9 @@ public BeanDeserializerBuilder updateBuilder(DeserializationConfig config, BeanD @Override public BeanSerializerBuilder updateBuilder(SerializationConfig config, BeanDescription beanDesc, BeanSerializerBuilder builder) { + if (builder.getBeanDescription().findAnyGetter() == null) { + return builder; + } builder.setProperties(builder.getProperties().stream() .map(p -> new BeanPropertyWriterDelegate(p, builder.getBeanDescription().findAnyGetter(), UnmatchedFieldTypeModule.this::isLogWarnings))