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

fix #4590: only using a builder if there are visitors #4593

Merged
merged 1 commit into from
Nov 22, 2022
Merged

fix #4590: only using a builder if there are visitors #4593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop

This file was deleted.

11 changes: 7 additions & 4 deletions kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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
3 changes: 1 addition & 2 deletions ...src/main/java/io/fabric8/openshift/client/dsl/internal/project/ProjectOperationsImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import io.fabric8.kubernetes.client.dsl.internal.HasMetadataOperation;
import io.fabric8.kubernetes.client.dsl.internal.HasMetadataOperationsImpl;
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.openshift.api.model.Project;
import io.fabric8.openshift.api.model.ProjectBuilder;
Expand Down Expand Up @@ -74,7 +73,7 @@ public List<HasMetadata> createProjectAndRoleBindings(String name, String descri

// Create Role Bindings
NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl listOp = new NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(
context.withItem(projectRoleBindings), new NamespaceVisitOperationContext());
context.withItem(projectRoleBindings));
result.addAll(listOp.createOrReplace());

return result;
Expand Down
12 changes: 4 additions & 8 deletions ...ternal/OpenShiftNamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -102,9 +100,7 @@ private static List<HasMetadata> 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);
}
}