From fae2717072785dec2b6edec35ab50ccc8da5e1e1 Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Fri, 18 Nov 2022 15:12:02 -0500 Subject: [PATCH] fix #4590: only using a builder if there are visitors --- CHANGELOG.md | 1 + ...hDeleteRecreateWaitApplicableListImpl.java | 71 +++++++------------ .../NamespaceVisitOperationContext.java | 36 ---------- .../client/mock/ResourceListTest.java | 11 +-- ...hDeleteRecreateWaitApplicableListImpl.java | 12 ++-- 5 files changed, 36 insertions(+), 95 deletions(-) delete mode 100644 kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitOperationContext.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 176aef9d522..50146950f50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### 6.3-SNAPSHOT #### Bugs +* Fix #4590: only using a builder if there are visitors * Fix #4159: ensure the token refresh obeys how the Config was created * Fix #4491: added a more explicit shutdown exception for okhttp * Fix #4510: Fix StackOverflow on cyclic references involving collections. diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java index 827921752d9..1e8303a013a 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java @@ -16,14 +16,12 @@ package io.fabric8.kubernetes.client.dsl.internal; import com.fasterxml.jackson.databind.ObjectMapper; -import io.fabric8.kubernetes.api.builder.TypedVisitor; import io.fabric8.kubernetes.api.builder.VisitableBuilder; import io.fabric8.kubernetes.api.builder.Visitor; import io.fabric8.kubernetes.api.model.DeletionPropagation; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.KubernetesList; import io.fabric8.kubernetes.api.model.KubernetesResourceList; -import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.fabric8.kubernetes.api.model.StatusDetails; import io.fabric8.kubernetes.client.Client; import io.fabric8.kubernetes.client.KubernetesClient; @@ -63,36 +61,19 @@ public class NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImp implements ParameterNamespaceListVisitFromServerGetDeleteRecreateWaitApplicable, Waitable, HasMetadata> { - static class ChangeNamespace extends TypedVisitor { - - private final String explicitNamespace; - - ChangeNamespace(String explicitNamespace) { - this.explicitNamespace = explicitNamespace; - } - - @Override - public void visit(ObjectMetaBuilder builder) { - builder.withNamespace(explicitNamespace); - } - } - private static final Logger LOGGER = LoggerFactory .getLogger(NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.class); protected static final String EXPRESSION = "expression"; protected static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); - private final NamespaceVisitOperationContext namespaceVisitOperationContext; private OperationContext context; - public NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(OperationContext context, - NamespaceVisitOperationContext namespaceVisitOperationContext) { - this.namespaceVisitOperationContext = namespaceVisitOperationContext; + public NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(OperationContext context) { this.context = context; } public NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(Client client, Object item) { - this(HasMetadataOperationsImpl.defaultContext(client).withItem(item), new NamespaceVisitOperationContext()); + this(HasMetadataOperationsImpl.defaultContext(client).withItem(item)); } @Override @@ -110,7 +91,7 @@ List getItems() { return asHasMetadata(item).stream() .map(meta -> acceptVisitors(meta, - Collections.emptyList(), namespaceVisitOperationContext.getExplicitNamespace(), this.context)) + Collections.emptyList(), this.context)) .collect(Collectors.toList()); } @@ -196,17 +177,17 @@ private static void logAsNotReady(Throwable t, HasMetadata meta) { public NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable withParameters( Map parameters) { Object item = Serialization.unmarshal((InputStream) context.getItem(), parameters); - return newInstance(context.withItem(item), namespaceVisitOperationContext); + return newInstance(context.withItem(item)); } @Override public ListVisitFromServerWritable dryRun(boolean isDryRun) { - return newInstance(this.context.withDryRun(isDryRun), namespaceVisitOperationContext); + return newInstance(this.context.withDryRun(isDryRun)); } @Override public ListVisitFromServerWritable fieldValidation(Validation fieldValidation) { - return newInstance(this.context.withFieldValidation(fieldValidation), namespaceVisitOperationContext); + return newInstance(this.context.withFieldValidation(fieldValidation)); } @Override @@ -234,31 +215,30 @@ public ListVisitFromServerGetDeleteRecreateWaitApplicable inNamespa if (explicitNamespace == null) { throw new KubernetesClientException("namespace cannot be null"); } - return newInstance(context.withNamespace(explicitNamespace), - namespaceVisitOperationContext.withExplicitNamespace(explicitNamespace)); + return newInstance(context.withNamespace(explicitNamespace)); } @Override public Gettable> fromServer() { - return newInstance(context.withReloadingFromServer(true), namespaceVisitOperationContext); + return newInstance(context.withReloadingFromServer(true)); } @Override public ListVisitFromServerGetDeleteRecreateWaitApplicable accept(Visitor... visitors) { return newInstance(context.withItem(getItems().stream() - .map(i -> acceptVisitors(i, Arrays.asList(visitors), namespaceVisitOperationContext.getExplicitNamespace(), context)) - .collect(Collectors.toList())), namespaceVisitOperationContext); + .map(i -> acceptVisitors(i, Arrays.asList(visitors), context)) + .collect(Collectors.toList()))); } @Override public ListVisitFromServerGetDeleteRecreateWaitApplicable withGracePeriod(long gracePeriodSeconds) { - return newInstance(context.withGracePeriodSeconds(gracePeriodSeconds), namespaceVisitOperationContext); + return newInstance(context.withGracePeriodSeconds(gracePeriodSeconds)); } @Override public ListVisitFromServerGetDeleteRecreateWaitApplicable withPropagationPolicy( DeletionPropagation propagationPolicy) { - return newInstance(context.withPropagationPolicy(propagationPolicy), namespaceVisitOperationContext); + return newInstance(context.withPropagationPolicy(propagationPolicy)); } protected Readiness getReadiness() { @@ -287,23 +267,21 @@ protected List asHasMetadata(Object item) { return result; } - public NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl newInstance(OperationContext context, - NamespaceVisitOperationContext namespaceVisitOperationContext) { - return new NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(context, namespaceVisitOperationContext); + public NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl newInstance(OperationContext context) { + return new NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(context); } - static HasMetadata acceptVisitors(HasMetadata item, List visitors, String explicitNamespace, - OperationContext context) { - VisitableBuilder builder = context.getHandler(item).edit(item); + static HasMetadata acceptVisitors(HasMetadata item, List visitors, OperationContext context) { + if (!visitors.isEmpty()) { + VisitableBuilder builder = context.getHandler(item).edit(item); - //Let's apply any visitor that might have been specified. - for (Visitor v : visitors) { - builder.accept(v); - } - if (explicitNamespace != null) { - builder.accept(new ChangeNamespace(explicitNamespace)); + //Let's apply any visitor that might have been specified. + for (Visitor v : visitors) { + builder.accept(v); + } + item = builder.build(); } - return builder.build(); + return item; } @Override @@ -318,8 +296,7 @@ public ListVisitFromServerWritable dryRun() { @Override public ListVisitFromServerGetDeleteRecreateWaitApplicable inAnyNamespace() { - return newInstance(context.withNamespace(null), - namespaceVisitOperationContext.withExplicitNamespace(null)); + return newInstance(context.withNamespace(null)); } @Override diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitOperationContext.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitOperationContext.java deleted file mode 100644 index 494bd2b4cad..00000000000 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitOperationContext.java +++ /dev/null @@ -1,36 +0,0 @@ -/** - * 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.dsl.internal; - -public class NamespaceVisitOperationContext { - private String explicitNamespace; - - public NamespaceVisitOperationContext(String explicitNamespace) { - this.explicitNamespace = explicitNamespace; - } - - public NamespaceVisitOperationContext() { - } - - public String getExplicitNamespace() { - return explicitNamespace; - } - - public NamespaceVisitOperationContext withExplicitNamespace(String explicitNamespace) { - return new NamespaceVisitOperationContext(explicitNamespace); - } - -} \ No newline at end of file diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java index 1809292addb..3c11775920f 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java @@ -37,6 +37,7 @@ import io.fabric8.kubernetes.client.dsl.NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable; import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient; import io.fabric8.kubernetes.client.server.mock.KubernetesMockServer; +import io.fabric8.kubernetes.client.utils.Serialization; import okhttp3.mockwebserver.RecordedRequest; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -105,14 +106,16 @@ void testCreateOrReplaceFailedCreate() { } @Test - void testCreateWithExplicitNamespace() { + void testCreateWithExplicitNamespace() throws InterruptedException { Pod pod1 = new PodBuilder().withNewMetadata().withName("pod1").withNamespace("test").and().build(); server.expect().post().withPath("/api/v1/namespaces/ns1/pods").andReturn(HTTP_CREATED, pod1).once(); - List response = client.resourceList(new PodListBuilder().addToItems(pod1).build()).inNamespace("ns1") - .createOrReplace(); - assertTrue(response.contains(pod1)); + client.resourceList(new PodListBuilder().addToItems(pod1).build()).inNamespace("ns1").createOrReplace(); + + Pod sent = Serialization.unmarshal(server.getLastRequest().getBody().inputStream()); + // ensure the namespace was modified + assertEquals("ns1", sent.getMetadata().getNamespace()); } @Test diff --git a/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftNamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java b/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftNamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java index e6c78314a5e..ca1f1b76ed4 100644 --- a/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftNamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java +++ b/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftNamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java @@ -22,7 +22,6 @@ import io.fabric8.kubernetes.client.Client; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.dsl.internal.NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl; -import io.fabric8.kubernetes.client.dsl.internal.NamespaceVisitOperationContext; import io.fabric8.kubernetes.client.dsl.internal.OperationContext; import io.fabric8.kubernetes.client.utils.Utils; import io.fabric8.openshift.api.model.Parameter; @@ -40,9 +39,8 @@ public OpenShiftNamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableList super(client, item); } - public OpenShiftNamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(OperationContext context, - NamespaceVisitOperationContext namespaceVisitOperationContext) { - super(context, namespaceVisitOperationContext); + public OpenShiftNamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(OperationContext context) { + super(context); } @Override @@ -102,9 +100,7 @@ private static List processTemplateList(Template item, Boolean enab } @Override - public NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl newInstance(OperationContext context, - NamespaceVisitOperationContext namespaceVisitOperationContext) { - return new OpenShiftNamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(context, - namespaceVisitOperationContext); + public NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl newInstance(OperationContext context) { + return new OpenShiftNamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(context); } }