fix: assorted robustness and clarity fixes (#3533)

* fix: assorted robustness and clarity fixes

Three unrelated small items found while auditing.

ManagedInformerEventSource.get could throw instead of falling back to the
informer cache. It compared a temporary-cache hit against
`manager().lastSyncResourceVersion(namespace)` without guarding either
failure mode:

* `lastSyncResourceVersion` is null until an informer has completed its
  initial list, and `compareResourceVersions` dereferences it, so this
  threw a NullPointerException.
* `InformerManager.lastSyncResourceVersion` does `getSource(ns)
  .orElseThrow()`, which throws `NoSuchElementException` for a namespace
  that is no longer watched after a dynamic namespace change - a window
  that only closes on the next re-list, when `checkGhostResources` prunes
  the entry.

`TemporaryResourceCache.putResource` already guards the null case; the
comparison is now extracted into a helper that guards both and falls back
to the informer cache, which is the safe answer in either case.

ExpectationResult.name() threw a bare NullPointerException when no
expectation was registered, which is a normal result state produced by
`ExpectationManager.checkExpectation`. It now throws
`IllegalStateException` pointing at `isExpectationPresent()`, and says so
in the javadoc.

ResourceOperations cleanups: drops the `options` parameter of
`desiredForJsonPatch`, which was never read at any of its six call sites,
and rewrites the comment on `create` - it claimed the operation "check if
the resource already exists", which it does not; the actual reason
filtering is safe is that the API server rejects a duplicate create.

* Apply suggestions from code review

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>

---------

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Author: csviri

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java

@@ -406,7 +406,9 @@ public <R extends HasMetadata> R update(
    * @return the created resource as returned by the API server
    */
   public <R extends HasMetadata> R create(R resource) {
-    // it is safe to do event filtering for create since check if the resource already exists.
+    // filtering the own event is safe here without optimistic locking or a matcher: a create either
+    // succeeds, in which case the event we filter is unambiguously ours, or the API server rejects
+    // it because the resource already exists.
     return create(resource, Options.forceFilterEvents());
   }
 
@@ -445,7 +447,7 @@ public <R extends HasMetadata> R create(
     if (informerEventSource == null) {
       return create(resource);
     }
-    // it is safe to do event filtering for create since check if the resource already exists.
+    // see the note on create(R) about why filtering the own event is safe here
     return resourcePatch(
         resource, r -> context.getClient().resource(r).create(), informerEventSource, options);
   }
@@ -556,7 +558,7 @@ public <R extends HasMetadata> R jsonPatch(R actualResource, UnaryOperator<R> un
    */
   public <R extends HasMetadata> R jsonPatch(
       R actualResource, UnaryOperator<R> unaryOperator, Options options) {
-    R desired = desiredForJsonPatch(actualResource, unaryOperator, options);
+    R desired = desiredForJsonPatch(actualResource, unaryOperator);
     return resourcePatch(
         desired,
         actualResource,
@@ -580,7 +582,7 @@ public <R extends HasMetadata> R jsonPatch(
       UnaryOperator<R> unaryOperator,
       InformerEventSource<R, P> informerEventSource,
       Options options) {
-    R desired = desiredForJsonPatch(actualResource, unaryOperator, options);
+    R desired = desiredForJsonPatch(actualResource, unaryOperator);
     return resourcePatch(
         desired,
         actualResource,
@@ -620,7 +622,7 @@ public <R extends HasMetadata> R jsonPatchStatus(
    */
   public <R extends HasMetadata> R jsonPatchStatus(
       R actualResource, UnaryOperator<R> unaryOperator, Options options) {
-    R desired = desiredForJsonPatch(actualResource, unaryOperator, options);
+    R desired = desiredForJsonPatch(actualResource, unaryOperator);
     return resourcePatch(
         desired,
         actualResource,
@@ -645,7 +647,7 @@ public <R extends HasMetadata> R jsonPatchStatus(
       UnaryOperator<R> unaryOperator,
       InformerEventSource<R, P> informerEventSource,
       Options options) {
-    R desired = desiredForJsonPatch(actualResource, unaryOperator, options);
+    R desired = desiredForJsonPatch(actualResource, unaryOperator);
     return resourcePatch(
         desired,
         actualResource,
@@ -680,7 +682,7 @@ public P jsonPatchPrimary(P actualResource, UnaryOperator<P> unaryOperator) {
    * @return the patched resource as returned by the API server
    */
   public P jsonPatchPrimary(P actualResource, UnaryOperator<P> unaryOperator, Options options) {
-    P desired = desiredForJsonPatch(actualResource, unaryOperator, options);
+    P desired = desiredForJsonPatch(actualResource, unaryOperator);
     return resourcePatch(
         desired,
         actualResource,
@@ -717,7 +719,7 @@ public P jsonPatchPrimaryStatus(P actualResource, UnaryOperator<P> unaryOperator
    */
   public P jsonPatchPrimaryStatus(
       P actualResource, UnaryOperator<P> unaryOperator, Options options) {
-    P desired = desiredForJsonPatch(actualResource, unaryOperator, options);
+    P desired = desiredForJsonPatch(actualResource, unaryOperator);
     return resourcePatch(
         desired,
         actualResource,
@@ -1447,7 +1449,7 @@ public enum Mode {
   }
 
   private <T extends HasMetadata> T desiredForJsonPatch(
-      T actualResource, UnaryOperator<T> unaryOperator, Options options) {
+      T actualResource, UnaryOperator<T> unaryOperator) {
     var cloned =
         context
             .getControllerConfiguration()

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

@@ -224,10 +224,7 @@ public Optional<R> get(ResourceID resourceID) {
     Optional<R> resource = temporaryResourceCache.getResourceFromCache(resourceID);
     if (comparableResourceVersions
         && resource.isPresent()
-        && ReconcilerUtilsInternal.compareResourceVersions(
-                resource.get().getMetadata().getResourceVersion(),
-                manager().lastSyncResourceVersion(resource.get().getMetadata().getNamespace()))
-            > 0) {
+        && isLaterThanLastSyncResourceVersion(resource.orElseThrow())) {
       log.debug("Latest resource found in temporary cache for Resource ID: {}", resourceID);
       return resource;
     } else {
@@ -242,6 +239,31 @@ public Optional<R> get(ResourceID resourceID) {
     }
   }
 
+  /**
+   * A resource from the temporary cache is only preferred over the informer cache if we can tell
+   * that it is newer. The last sync resource version is not available before an informer has
+   * completed its initial list, and the namespace might not be watched anymore after a dynamic
+   * namespace change, in both of which cases the informer cache is used instead.
+   */
+  private boolean isLaterThanLastSyncResourceVersion(R resource) {
+    var namespace = resource.getMetadata().getNamespace();
+    if (!manager().isWatchingNamespace(namespace)) {
+      return false;
+    }
+    final String lastSyncResourceVersion;
+    try {
+      lastSyncResourceVersion = manager().lastSyncResourceVersion(namespace);
+    } catch (java.util.NoSuchElementException e) {
+      return false;
+    }
+    if (lastSyncResourceVersion == null) {
+      return false;
+    }
+    return ReconcilerUtilsInternal.compareResourceVersions(
+            resource.getMetadata().getResourceVersion(), lastSyncResourceVersion)
+        > 0;
+  }
+
   /**
    * @deprecated Use {@link #get(ResourceID)} instead.
    */

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/expectation/ExpectationResult.java

@@ -36,7 +36,16 @@ public boolean isNotPresentOrFulfilled() {
     return !isExpectationPresent() || isFulfilled();
   }
 
+  /**
+   * @return the name of the expectation this result refers to
+   * @throws IllegalStateException if there is no expectation, check {@link #isExpectationPresent()}
+   *     first
+   */
   public String name() {
+    if (expectation == null) {
+      throw new IllegalStateException(
+          "No expectation present for this result, check isExpectationPresent() first.");
+    }
     return expectation.name();
   }
 }