Skip to content

Commit

Permalink
fix fabric8io#4590: only using a builder if there are visitors
Browse files Browse the repository at this point in the history
  • Loading branch information
@shawkins
shawkins committed Nov 19, 2022
1 parent f58b1dd commit 89deeda
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 87 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -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.
Expand Down
71 changes: 24 additions & 47 deletions ...nt/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -63,36 +61,19 @@ public class NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImp
implements ParameterNamespaceListVisitFromServerGetDeleteRecreateWaitApplicable<HasMetadata>,
Waitable<List<HasMetadata>, HasMetadata> {

static class ChangeNamespace extends TypedVisitor<ObjectMetaBuilder> {

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
Expand All @@ -110,7 +91,7 @@ List<HasMetadata> getItems() {

return asHasMetadata(item).stream()
.map(meta -> acceptVisitors(meta,
Collections.emptyList(), namespaceVisitOperationContext.getExplicitNamespace(), this.context))
Collections.emptyList(), this.context))
.collect(Collectors.toList());
}

Expand Down Expand Up @@ -196,17 +177,17 @@ private static void logAsNotReady(Throwable t, HasMetadata meta) {
public NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable<HasMetadata> withParameters(
Map<String, String> parameters) {
Object item = Serialization.unmarshal((InputStream) context.getItem(), parameters);
return newInstance(context.withItem(item), namespaceVisitOperationContext);
return newInstance(context.withItem(item));
}

@Override
public ListVisitFromServerWritable<HasMetadata> dryRun(boolean isDryRun) {
return newInstance(this.context.withDryRun(isDryRun), namespaceVisitOperationContext);
return newInstance(this.context.withDryRun(isDryRun));
}

@Override
public ListVisitFromServerWritable<HasMetadata> fieldValidation(Validation fieldValidation) {
return newInstance(this.context.withFieldValidation(fieldValidation), namespaceVisitOperationContext);
return newInstance(this.context.withFieldValidation(fieldValidation));
}

@Override
Expand Down Expand Up @@ -234,31 +215,30 @@ public ListVisitFromServerGetDeleteRecreateWaitApplicable<HasMetadata> 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<List<HasMetadata>> fromServer() {
return newInstance(context.withReloadingFromServer(true), namespaceVisitOperationContext);
return newInstance(context.withReloadingFromServer(true));
}

@Override
public ListVisitFromServerGetDeleteRecreateWaitApplicable<HasMetadata> 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<HasMetadata> withGracePeriod(long gracePeriodSeconds) {
return newInstance(context.withGracePeriodSeconds(gracePeriodSeconds), namespaceVisitOperationContext);
return newInstance(context.withGracePeriodSeconds(gracePeriodSeconds));
}

@Override
public ListVisitFromServerGetDeleteRecreateWaitApplicable<HasMetadata> withPropagationPolicy(
DeletionPropagation propagationPolicy) {
return newInstance(context.withPropagationPolicy(propagationPolicy), namespaceVisitOperationContext);
return newInstance(context.withPropagationPolicy(propagationPolicy));
}

protected Readiness getReadiness() {
Expand Down Expand Up @@ -287,23 +267,21 @@ protected List<HasMetadata> 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<Visitor> visitors, String explicitNamespace,
OperationContext context) {
VisitableBuilder<HasMetadata, ?> builder = context.getHandler(item).edit(item);
static HasMetadata acceptVisitors(HasMetadata item, List<Visitor> visitors, OperationContext context) {
if (!visitors.isEmpty()) {
VisitableBuilder<HasMetadata, ?> 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
Expand All @@ -318,8 +296,7 @@ public ListVisitFromServerWritable<HasMetadata> dryRun() {

@Override
public ListVisitFromServerGetDeleteRecreateWaitApplicable<HasMetadata> inAnyNamespace() {
return newInstance(context.withNamespace(null),
namespaceVisitOperationContext.withExplicitNamespace(null));
return newInstance(context.withNamespace(null));
}

@Override
Expand Down
36 changes: 0 additions & 36 deletions ...c/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitOperationContext.java
View file

This file was deleted.

11 changes: 7 additions & 4 deletions kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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<HasMetadata> 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
Expand Down

0 comments on commit 89deeda

Please sign in to comment.