fix fabric8io#4081 fabric8io#4082: correcting/removing versionable

also improving informOnCondition

CHANGELOG.md

@@ -4,13 +4,15 @@
 #### Bugs
 * Fix #3832 #1883: simplifying the isHttpsAvailable check
 * Fix #3745: the client will throw better exceptions when a namespace is not discernible for an operation
+* Fix #4081: moving Versionable.withResourceVersion to a method on WatchAndWaitable and removing Waitable from the return type
 
 #### Improvements
 * Remove `setIntVal`, `setStrVal`, `setKind` setters from `IntOrString` class to avoid invalid combinations
 * Fix #3889 : remove piped stream for file download
 * Fix #1285: removed references to manually calling registerCustomKind
 * Fix #3334: adding basic support for server side apply.  Use patch(PatchContext.of(PatchType.SERVER_SIDE_APPLY), service), or new PatchContext.Builder().withPatchType(PatchType.SERVER_SIDE_APPLY).withForce(true).build() to override conflicts.
 * Fix #3969: relist will not trigger sync events
+* Fix #4082: improving informOnCondition to test the initial list instead of individual add events
 * Fix #3968: SharedIndexInformer.initialState can be used to set the store state before the informer starts. 
 SharedIndexInformer allows for the addition and removal of indexes even after starting, and you can remove the default namespace index if you wish.
 And Store.getKey can be used rather than directly referencing static Cache functions.

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

@@ -15,12 +15,11 @@
  */
 package io.fabric8.kubernetes.client.dsl;
 
-import io.fabric8.kubernetes.client.Watcher;
-
 import java.util.stream.Stream;
 
 public interface FilterWatchListDeletable<T, L, R>
-    extends Filterable<FilterWatchListDeletable<T, L, R>>, Watchable<Watcher<T>>, Versionable<WatchAndWaitable<T>>, Listable<L>,
+    extends Filterable<FilterWatchListDeletable<T, L, R>>, Listable<L>,
+    WatchAndWaitable<T>,
     DeletableWithOptions,
     Informable<T> {
 

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

@@ -29,7 +29,7 @@
 public interface Resource<T> extends
     FromServerGettable<T>,
     Lockable<ReplaceDeletable<T>>,
-    WatchAndWaitable<T>, Versionable<WatchAndWaitable<T>>,
+    WatchAndWaitable<T>,
     WritableOperation<T>,
     DryRunable<WritableOperation<T>>,
     Requirable<T>, Readiable, Informable<T> {

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

@@ -18,4 +18,7 @@
 import io.fabric8.kubernetes.client.Watcher;
 
 public interface WatchAndWaitable<T> extends Watchable<Watcher<T>>, Waitable<T, T> {
+
+  Watchable<Watcher<T>> withResourceVersion(String resourceVersion);
+
 }

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

@@ -69,7 +69,6 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -789,7 +788,7 @@ public T waitUntilCondition(Predicate<T> condition, long amount, TimeUnit timeUn
       if (l.isEmpty()) {
         return condition.test(null);
       }
-      return condition.test(l.get(0));
+      return l.stream().allMatch(condition);
     });
 
     if (!Utils.waitUntilReady(futureCondition, amount, timeUnit)) {
@@ -806,7 +805,6 @@ public T waitUntilCondition(Predicate<T> condition, long amount, TimeUnit timeUn
   @Override
   public CompletableFuture<List<T>> informOnCondition(Predicate<List<T>> condition) {
     CompletableFuture<List<T>> future = new CompletableFuture<>();
-    AtomicReference<Runnable> tester = new AtomicReference<>();
 
     // create an informer that supplies the tester with events and empty list handling
     SharedIndexInformer<T> informer = this.createInformer(0);
@@ -815,41 +813,46 @@ public CompletableFuture<List<T>> informOnCondition(Predicate<List<T>> condition
     future.whenComplete((r, t) -> informer.stop());
 
     // use the cache to evaluate the list predicate, trapping any exceptions
-    Runnable test = () -> {
+    Consumer<List<T>> test = list -> {
       try {
         // could skip if lastResourceVersion has not changed
-        List<T> list = informer.getStore().list();
         if (condition.test(list)) {
           future.complete(list);
         }
       } catch (Exception e) {
         future.completeExceptionally(e);
       }
     };
-    tester.set(test);
+
+    // run the test initially against the full list
+    // TODO: we'll duplicate the listing once the informer starts
+    List<T> items = this.list().getItems();
+    test.accept(items);
+    if (future.isDone()) {
+      return future;
+    }
 
     informer.addEventHandler(new ResourceEventHandler<T>() {
       @Override
       public void onAdd(T obj) {
-        test.run();
+        test.accept(informer.getStore().list());
       }
 
       @Override
       public void onDelete(T obj, boolean deletedFinalStateUnknown) {
-        test.run();
+        test.accept(informer.getStore().list());
       }
 
       @Override
       public void onUpdate(T oldObj, T newObj) {
-        test.run();
+        test.accept(informer.getStore().list());
       }
 
       @Override
       public void onNothing() {
-        test.run();
+        test.accept(informer.getStore().list());
       }
-    });
-    informer.run();
+    }).initialState(items.stream()).run();
     return future;
   }
 

kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ConfigMapCrudTest.java

@@ -27,6 +27,9 @@
 import org.junit.jupiter.api.Test;
 
 import java.util.Collections;
+import java.util.Set;
+import java.util.concurrent.ConcurrentSkipListSet;
+import java.util.concurrent.TimeUnit;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -41,22 +44,22 @@ class ConfigMapCrudTest {
   void testCrud() {
 
     ConfigMap configmap1 = new ConfigMapBuilder().withNewMetadata().withName("configmap1").endMetadata()
-      .addToData("one", "1")
-      .build();
+        .addToData("one", "1")
+        .build();
     ConfigMap configmap2 = new ConfigMapBuilder()
-      .withNewMetadata()
-      .addToLabels("foo", "bar")
-      .withName("configmap2")
-      .endMetadata()
-      .addToData("two", "2")
-      .build();
+        .withNewMetadata()
+        .addToLabels("foo", "bar")
+        .withName("configmap2")
+        .endMetadata()
+        .addToData("two", "2")
+        .build();
     ConfigMap configmap3 = new ConfigMapBuilder()
-      .withNewMetadata()
-      .addToLabels("foo", "bar")
-      .withName("configmap2")
-      .endMetadata()
-      .addToData("three", "3")
-      .build();
+        .withNewMetadata()
+        .addToLabels("foo", "bar")
+        .withName("configmap2")
+        .endMetadata()
+        .addToData("three", "3")
+        .build();
 
     client.configMaps().inNamespace("ns1").create(configmap1);
     client.configMaps().inNamespace("ns1").create(configmap2);
@@ -70,6 +73,24 @@ void testCrud() {
     assertNotNull(aConfigMapList);
     assertEquals(3, aConfigMapList.getItems().size());
 
+    Set<String> uids = new ConcurrentSkipListSet<>();
+
+    // make sure waitUntilCondition is called on each item
+    client.configMaps().inAnyNamespace().waitUntilCondition(c -> {
+      uids.add(c.getMetadata().getUid());
+      return true;
+    }, 1, TimeUnit.SECONDS);
+
+    assertEquals(3, uids.size());
+
+    // make sure that the initial list is used, not the individual add events
+    client.configMaps().inAnyNamespace().informOnCondition(l -> {
+      if (l.size() != 3) {
+        throw new AssertionError();
+      }
+      return true;
+    }).join();
+
     aConfigMapList = client.configMaps().inAnyNamespace().withLabels(Collections.singletonMap("foo", "bar")).list();
     assertNotNull(aConfigMapList);
     assertEquals(2, aConfigMapList.getItems().size());
@@ -83,11 +104,13 @@ void testCrud() {
     assertNotNull(aConfigMapList);
     assertEquals(1, aConfigMapList.getItems().size());
 
-    configmap2 = client.configMaps().inNamespace("ns1").withName("configmap2").edit(c -> new ConfigMapBuilder(c).addToData("II", "TWO").build());
+    configmap2 = client.configMaps().inNamespace("ns1").withName("configmap2")
+        .edit(c -> new ConfigMapBuilder(c).addToData("II", "TWO").build());
     assertNotNull(configmap2);
     assertEquals("TWO", configmap2.getData().get("II"));
 
-    configmap2 = client.configMaps().inNamespace("ns1").withName("configmap2").patch(PatchContext.of(PatchType.JSON_MERGE), new ConfigMapBuilder(configmap2).addToData("III", "THREE").build());
+    configmap2 = client.configMaps().inNamespace("ns1").withName("configmap2").patch(PatchContext.of(PatchType.JSON_MERGE),
+        new ConfigMapBuilder(configmap2).addToData("III", "THREE").build());
     assertNotNull(configmap2);
     assertEquals("THREE", configmap2.getData().get("III"));
   }
@@ -97,15 +120,16 @@ void testCrud() {
   void edit() {
     // Given
     final ConfigMap cm = new ConfigMapBuilder()
-      .withNewMetadata().withName("config-map").endMetadata()
-      .addToData("one", "1")
-      .build();
+        .withNewMetadata().withName("config-map").endMetadata()
+        .addToData("one", "1")
+        .build();
     client.configMaps().create(cm);
     // When
-    client.configMaps().withName("config-map").edit(c -> new ConfigMapBuilder(c).removeFromData("one").addToData("two", "2").build());
+    client.configMaps().withName("config-map")
+        .edit(c -> new ConfigMapBuilder(c).removeFromData("one").addToData("two", "2").build());
     // Then
     assertThat(client.configMaps().withName("config-map").get().getData())
-      .hasSize(1)
-      .containsExactlyInAnyOrderEntriesOf(Collections.singletonMap("two", "2"));
+        .hasSize(1)
+        .containsExactlyInAnyOrderEntriesOf(Collections.singletonMap("two", "2"));
   }
 }