fix fabric8io#4574 changing the behavior of get (fabric8io#4601)

Author: shawkins

CHANGELOG.md

@@ -11,6 +11,7 @@
 #### New Features
 
 #### _**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.
 
 ### 6.3.1 (2022-12-15)
 

extensions/camel-k/tests/src/test/java/io/fabric8/camelk/test/KameletBindingTest.java

@@ -39,7 +39,7 @@ void load() {
     // Given + When
     KameletBinding kameletBinding = client.v1alpha1().kameletBindings()
         .load(getClass().getResourceAsStream("/test-kameletbinding.yml"))
-        .get();
+        .item();
 
     // Then
     assertThat(kameletBinding)

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/FromServerGettable.java

@@ -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();
+
 }

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/dsl/Gettable.java

@@ -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
    */

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/dsl/NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable.java

@@ -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
@@ -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();
+
 }

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/dsl/Resource.java

@@ -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
@@ -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();
+
 }

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/extension/ResourceAdapter.java

@@ -326,6 +326,10 @@ public T serverSideApply() {
   }
 
   @Override
+  public T item() {
+    return resource.item();
+  }
+
   public DeletableWithOptions withTimeout(long timeout, TimeUnit unit) {
     return resource.withTimeout(timeout, unit);
   }

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java

@@ -105,7 +105,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;
 
@@ -119,7 +118,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();
@@ -141,9 +139,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;
@@ -155,23 +151,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);
@@ -282,7 +291,7 @@ public R load(String path) {
 
   @Override
   public BaseOperation<T, L, R> fromServer() {
-    return newInstance(context.withReloadingFromServer(true));
+    return this;
   }
 
   @Override
@@ -551,19 +560,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() {
@@ -748,12 +755,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;
@@ -763,10 +773,6 @@ public String getResourceVersion() {
     return resourceVersion;
   }
 
-  public Boolean isReloadingFromServer() {
-    return reloadingFromServer;
-  }
-
   public Long getGracePeriodSeconds() {
     return gracePeriodSeconds;
   }
@@ -860,7 +866,7 @@ public Readiness getReadiness() {
 
   @Override
   public final boolean isReady() {
-    T item = fromServer().get();
+    T item = get();
     if (item == null) {
       return false;
     }

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/HasMetadataOperation.java

@@ -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;
@@ -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);
   }
@@ -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);
@@ -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) {
@@ -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();