possible refinements

Author: shawkins

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java

@@ -17,48 +17,36 @@
 
 import java.util.Optional;
 
+import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEvent;
 
 class EventFilterDetails {
 
   private int activeUpdates = 0;
   private ResourceEvent lastEvent;
-  private int lastUpdatedResourceVersion = -1;
-
-  public int getActiveUpdates() {
-    return activeUpdates;
-  }
 
   public void increaseActiveUpdates() {
     activeUpdates = activeUpdates + 1;
   }
 
-  public void decreaseActiveUpdates() {
+  public boolean decreaseActiveUpdates() {
     activeUpdates = activeUpdates - 1;
+    return activeUpdates == 0;
   }
 
   public void setLastEvent(ResourceEvent event) {
     lastEvent = event;
   }
 
-  public void setLastUpdatedResourceVersion(String version) {
-    var parsed = Integer.parseInt(version);
-    if (parsed > lastUpdatedResourceVersion) {
-      lastUpdatedResourceVersion = parsed;
-    }
-  }
-
-  public Optional<ResourceEvent> getLatestEventAfterLastUpdateEvent() {
-    if (lastEvent == null) return Optional.empty();
-    if (Integer.parseInt(lastEvent.getResource().orElseThrow().getMetadata().getResourceVersion())
-        > lastUpdatedResourceVersion) {
+  public Optional<ResourceEvent> getLatestEventAfterLastUpdateEvent(String updatedResourceVersion) {
+    if (lastEvent != null
+        && (updatedResourceVersion == null
+            || ReconcileUtils.validateAndCompareResourceVersions(
+                    lastEvent.getResource().orElseThrow().getMetadata().getResourceVersion(),
+                    updatedResourceVersion)
+                > 0)) {
       return Optional.of(lastEvent);
-    } else {
-      return Optional.empty();
     }
-  }
-
-  public boolean isFilteringDone() {
-    return activeUpdates == 0;
+    return Optional.empty();
   }
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java

@@ -79,16 +79,10 @@ public synchronized Optional<ResourceEvent> doneEventFilterModify(
       return Optional.empty();
     }
     var ed = activeUpdates.get(resourceID);
-    ed.decreaseActiveUpdates();
-    if (updatedResourceVersion != null) {
-      ed.setLastUpdatedResourceVersion(updatedResourceVersion);
-    }
-    if (ed.getActiveUpdates() == 0) {
-      var latestEventAfterUpdate = ed.getLatestEventAfterLastUpdateEvent();
-      if (latestEventAfterUpdate.isPresent()) {
-        activeUpdates.remove(resourceID);
-      }
-      return latestEventAfterUpdate;
+    // TODO: null check
+    if (ed.decreaseActiveUpdates()) {
+      activeUpdates.remove(resourceID);
+      return ed.getLatestEventAfterLastUpdateEvent(updatedResourceVersion);
     } else {
       return Optional.empty();
     }
@@ -125,7 +119,7 @@ private synchronized boolean onEvent(T resource, boolean unknownState, boolean d
     int comp = 0;
     if (cached != null) {
       comp = ReconcileUtils.validateAndCompareResourceVersions(resource, cached);
-      if (comp >= 0 || unknownState) {
+      if (comp > 0 || unknownState) {
         cache.remove(resourceId);
         // we propagate event only for our update or newer other can be discarded since we know we
         // will receive
@@ -142,12 +136,7 @@ private synchronized boolean onEvent(T resource, boolean unknownState, boolean d
               ? new ResourceDeleteEvent(ResourceAction.DELETED, resourceId, resource, unknownState)
               : new ResourceEvent(
                   ResourceAction.UPDATED, resourceId, resource)); // todo true action
-      if (ed.isFilteringDone() && ed.getLatestEventAfterLastUpdateEvent().isPresent()) {
-        activeUpdates.remove(resourceId);
-        return false;
-      } else {
-        return true;
-      }
+      return true;
     } else {
       return filterEvent;
     }

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java

@@ -151,9 +151,9 @@ void putBeforeEvent() {
     nextResource.getMetadata().setResourceVersion("3");
     temporaryResourceCache.putResource(nextResource);
 
-    // the result is false since the put was not part of event filtering update
+    // now expect an event with the matching resourceVersion to be known after the put
     result = temporaryResourceCache.onAddOrUpdateEvent(nextResource);
-    assertThat(result).isFalse();
+    assertThat(result).isTrue();
   }
 
   @Test