Skip to content

Commit

Permalink
fix fabric8io#4574 changing the behavior of get
Browse files Browse the repository at this point in the history
  • Loading branch information
shawkins committed Nov 24, 2022
1 parent 66ca558 commit d75301e
Show file tree
Hide file tree
Showing 90 changed files with 276 additions and 244 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* Fix #4582: updated [client.secrets] createOrReplace document

#### _**Note**_: Breaking changes
* Fix #4574: fromServer has been deprecated - it no longer needs to be called. All get() operations will fetch the resource(s) from the api server. If you need the context item that was passed in from a resource, load, or resourceList methods, use the item or items method.
* Fix #4515: files located at the root of jars named model.properties, e.g. core.properties, have been removed
* Fix #3923: removed KubernetesResourceMappingProvider - a META-INF/services/io.fabric8.kubernetes.api.model.KubernetesResource list of resources is used instead.
* Fix #4597: remove the deprecated support for `javax.validation.constraints.NotNull` in the `crd-generator`, to mark a property as `required` it needs to be annotated with `io.fabric8.generator.annotation.Required`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ void load() {
// Given + When
KameletBinding kameletBinding = client.v1alpha1().kameletBindings()
.load(getClass().getResourceAsStream("/test-kameletbinding.yml"))
.get();
.item();

// Then
assertThat(kameletBinding)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,11 @@

public interface FromServerGettable<T> extends Gettable<T> {

/**
* @deprecated {@link #get()} will always return the latest resource from the server, there is no
* need to call fromServer.
*/
@Deprecated
Gettable<T> fromServer();

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
public interface Gettable<T> {

/**
* Get the current state from the api server.
*
* See also {@link Resource#item()}
*
* @return the item or null if the item doesn't exist.
* @throws io.fabric8.kubernetes.client.KubernetesClientException if an error occurs
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

package io.fabric8.kubernetes.client.dsl;

import io.fabric8.kubernetes.api.model.HasMetadata;

import java.util.List;
import java.util.stream.Stream;

public interface NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable<T> extends
Expand All @@ -26,4 +29,9 @@ public interface NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable<T>
@Override
Stream<NamespaceableResource<T>> resources();

/**
* Return the items used to create this list context - these values are not from the server.
*/
List<HasMetadata> items();

}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ default GracePeriodConfigurable<? extends Deletable> cascading(boolean enabled)
boolean isReady();

/**
* Perform a {@link Gettable#get()}, but throws an exception if the server resource does not exist.
*
* @return the item or throws an exception if the item doesn't exist.
* @throws io.fabric8.kubernetes.client.KubernetesClientException if an error occurs
* @throws io.fabric8.kubernetes.client.ResourceNotFoundException if resource is absent
Expand All @@ -68,4 +70,12 @@ default GracePeriodConfigurable<? extends Deletable> cascading(boolean enabled)

ReplaceDeletable<T> lockResourceVersion(String resourceVersion);

/**
* Get the item used to create the current operation context if available.
*
* @return the current item if provided via the load, resource, or resourceList method, or null if this resource was created
* just from a class.
*/
T item();

}
Original file line number Diff line number Diff line change
Expand Up @@ -324,4 +324,9 @@ public T serverSideApply() {
return resource.serverSideApply();
}

@Override
public T item() {
return resource.item();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ public class BaseOperation<T extends HasMetadata, L extends KubernetesResourceLi
private final T item;

private final String resourceVersion;
private final boolean reloadingFromServer;
private final long gracePeriodSeconds;
private final DeletionPropagation propagationPolicy;

Expand All @@ -115,7 +114,6 @@ public class BaseOperation<T extends HasMetadata, L extends KubernetesResourceLi
protected BaseOperation(OperationContext ctx) {
super(ctx);
this.item = (T) ctx.getItem();
this.reloadingFromServer = ctx.isReloadingFromServer();
this.resourceVersion = ctx.getResourceVersion();
this.gracePeriodSeconds = ctx.getGracePeriodSeconds();
this.propagationPolicy = ctx.getPropagationPolicy();
Expand All @@ -137,9 +135,7 @@ protected URL fetchListUrl(URL url, ListOptions listOptions) {
@Override
public T get() {
try {
final T answer = getMandatory();
updateApiVersion(answer);
return answer;
return requireFromServer();
} catch (KubernetesClientException e) {
if (e.getCode() != HttpURLConnection.HTTP_NOT_FOUND) {
throw e;
Expand All @@ -151,23 +147,36 @@ public T get() {
@Override
public T require() {
try {
T answer = getMandatory();
if (answer == null) {
throw new ResourceNotFoundException("The resource you request doesn't exist or couldn't be fetched.");
}
return answer;
return requireFromServer();
} catch (KubernetesClientException e) {
if (e.getCode() != HttpURLConnection.HTTP_NOT_FOUND) {
throw e;
}
throw new ResourceNotFoundException("Resource not found : " + e.getMessage(), e);
throw new ResourceNotFoundException("Resource couldn't be fetched : " + e.getMessage(), e);
}
}

public T getMandatory() {
if (item != null && !reloadingFromServer) {
/**
* Return the context item or retrieves the remote item
*
* @return
*/
public T getItemOrRequireFromServer() {
if (item != null) {
return Serialization.clone(item);
}
return requireFromServer();
}

/**
* Get the current item from the server
* <br>
* Will always return non-null or throw an exception.
* <br>
* Differs from {@link #require()} in that it does not throw a {@link ResourceNotFoundException} exception
* which for some reason is not a {@link KubernetesClientException}
*/
protected T requireFromServer() {
if (Utils.isNullOrEmpty(getName())) {
throw new KubernetesClientException("name not specified for an operation requiring one.");
}
try {
URL requestUrl = getCompleteResourceUrl();
return handleGet(requestUrl);
Expand Down Expand Up @@ -278,7 +287,7 @@ public R load(String path) {

@Override
public BaseOperation<T, L, R> fromServer() {
return newInstance(context.withReloadingFromServer(true));
return this;
}

@Override
Expand Down Expand Up @@ -523,19 +532,17 @@ public T patchStatus(T item) {

@Override
public T patchStatus() {
// fromServer shouldn't be necessary here as we're using a merge patch, but
// just in case that changes we want consistency with the other patch methods
return this.fromServer().patchStatus(getNonNullItem());
throw new KubernetesClientException(READ_ONLY_UPDATE_EXCEPTION_MESSAGE);
}

@Override
public T patch() {
return this.fromServer().patch(getNonNullItem());
throw new KubernetesClientException(READ_ONLY_UPDATE_EXCEPTION_MESSAGE);
}

@Override
public T patch(PatchContext patchContext) {
return this.fromServer().patch(patchContext, getNonNullItem());
throw new KubernetesClientException(READ_ONLY_UPDATE_EXCEPTION_MESSAGE);
}

protected T getNonNullItem() {
Expand Down Expand Up @@ -720,12 +727,15 @@ private URL getCompleteResourceUrl() throws MalformedURLException {
URL requestUrl = getNamespacedUrl(checkNamespace(item));
if (name != null) {
requestUrl = new URL(URLUtils.join(requestUrl.toString(), name));
} else if (item != null && reloadingFromServer) {
requestUrl = new URL(URLUtils.join(requestUrl.toString(), checkName(item)));
}
return requestUrl;
}

@Override
public T item() {
return getItem();
}

@Override
public final T getItem() {
return item;
Expand All @@ -735,10 +745,6 @@ public String getResourceVersion() {
return resourceVersion;
}

public Boolean isReloadingFromServer() {
return reloadingFromServer;
}

public Long getGracePeriodSeconds() {
return gracePeriodSeconds;
}
Expand Down Expand Up @@ -832,7 +838,7 @@ public Readiness getReadiness() {

@Override
public final boolean isReady() {
T item = fromServer().get();
T item = get();
if (item == null) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,11 @@
import io.fabric8.kubernetes.api.model.KubernetesResourceList;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.ResourceNotFoundException;
import io.fabric8.kubernetes.client.dsl.Resource;
import io.fabric8.kubernetes.client.dsl.base.PatchContext;
import io.fabric8.kubernetes.client.dsl.base.PatchType;
import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil;
import io.fabric8.kubernetes.client.utils.Serialization;
import io.fabric8.kubernetes.client.utils.Utils;

import java.io.IOException;
import java.util.concurrent.TimeUnit;
Expand All @@ -53,7 +51,7 @@ public HasMetadataOperation(OperationContext ctx, Class<T> type, Class<L> listTy

@Override
public T edit(UnaryOperator<T> function) {
T item = getMandatory();
T item = getItemOrRequireFromServer();
T clone = clone(item);
return patch(null, clone, function.apply(item), false);
}
Expand All @@ -64,50 +62,26 @@ private T clone(T item) {

@Override
public T editStatus(UnaryOperator<T> function) {
T item = getMandatory();
T item = getItemOrRequireFromServer();
T clone = clone(item);
return patch(null, clone, function.apply(item), true);
}

@Override
public T accept(Consumer<T> consumer) {
T item = getMandatory();
T item = getItemOrRequireFromServer();
T clone = clone(item);
consumer.accept(item);
return patch(null, clone, item, false);
}

@Override
public T edit(Visitor... visitors) {
T item = getMandatory();
T item = getItemOrRequireFromServer();
T clone = clone(item);
return patch(null, clone, context.getHandler(item).edit(item).accept(visitors).build(), false);
}

/**
* Get the current item from the server
* <br>
* Will always return non-null or throw an exception.
*/
protected T requireFromServer() {
try {
if (Utils.isNotNullOrEmpty(getName())) {
return newInstance(context.withItem(null)).require();
}
if (getItem() != null) {
String name = KubernetesResourceUtil.getName(getItem());
if (Utils.isNotNullOrEmpty(name)) {
return newInstance(context.withItem(null)).withName(name).require();
}
}
} catch (ResourceNotFoundException e) {
if (e.getCause() instanceof KubernetesClientException) {
throw (KubernetesClientException) e.getCause();
}
}
throw new KubernetesClientException("name not specified for an operation requiring one.");
}

@Override
public T replace() {
return replace(getItem(), false);
Expand Down Expand Up @@ -192,9 +166,15 @@ protected T replace(T item, boolean status) {
throw KubernetesClientException.launderThrowable(forOperationType(REPLACE_OPERATION), caught);
}

/**
* Perform a patch. If the base is not provided and one is required, it will
* be fetched from the server.
*/
protected T patch(PatchContext context, T base, T item, boolean status) {
if (base == null && context != null && context.getPatchType() == PatchType.JSON) {
base = getMandatory();
if (context == null || context.getPatchType() == PatchType.JSON) {
if (base == null) {
base = requireFromServer();
}
if (base.getMetadata() != null) {
// prevent the resourceVersion from being modified in the patch
if (item.getMetadata() == null) {
Expand All @@ -220,20 +200,35 @@ protected T patch(PatchContext context, T base, T item, boolean status) {
return visitor.apply(item);
}

@Override
public T patchStatus() {
return patch(PatchContext.of(PatchType.JSON_MERGE), null, getNonNullItem(), true);
}

@Override
public T patch() {
return patch(null, null, getNonNullItem(), false);
}

@Override
public T patch(PatchContext patchContext) {
return patch(patchContext, null, getNonNullItem(), false);
}

@Override
public T patchStatus(T item) {
return patch(PatchContext.of(PatchType.JSON_MERGE), null, clone(item), true);
return patch(PatchContext.of(PatchType.JSON_MERGE), getItem(), clone(item), true);
}

@Override
public T patch(PatchContext patchContext, T item) {
return patch(patchContext, null, clone(item), false);
return patch(patchContext, getItem(), clone(item), false);
}

@Override
public T patch(PatchContext patchContext, String patch) {
try {
final T got = getMandatory();
final T got = getItemOrRequireFromServer();
return handlePatch(patchContext, got, convertToJson(patch), getType(), false);
} catch (InterruptedException interruptedException) {
Thread.currentThread().interrupt();
Expand Down
Loading

0 comments on commit d75301e

Please sign in to comment.