read-cache-after-write aware list and byIndex

[csviri]

Now list() and byIndex() of ManagedInformerEventSource (so also subsclasses) are aware of cached resources for read-cache-after-write consistency.

Adds ghost resource handling for keys() and missing javadocs.

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

[Copilot]

Pull request overview

This PR extends the informer-backed caches to provide read-cache-after-write consistency for list(...) and indexed lookups by merging results with the temporary resource cache (including “ghost” resources), and adds a streaming variant for index-based queries.

Changes:

• Add byIndexStream to the indexed cache API and implement it across informer wrappers/managers.
• Make ManagedInformerEventSource.list(...) / byIndex(...) / byIndexStream(...) merge results with TemporaryResourceCache for read-cache-after-write consistency.
• Add tests covering replacement/skipping/ghost-resource behavior for list and index streams.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java	Adds tests validating list/byIndex merge behavior with temporary cache and ghost resources.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java	Exposes temp-cache snapshot/emptiness helpers used for merging logic.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java	Implements read-cache-after-write consistent list/byIndex by merging informer results with temporary cache.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java	Adds byIndexStream implementation delegating to existing byIndex.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java	Adds byIndexStream aggregator and reuses it for byIndex.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/Cache.java	Adds Javadoc to clarify cache/listing semantics.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java	Clarifies Javadoc to explicitly describe read-cache-after-write consistency.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceCache.java	Adds Javadoc for namespace-scoped list operations.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/IndexedResourceCache.java	Extends API with byIndexStream.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 7 comments.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

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

• TemporaryResourceCache.getResources() already returns a defensive copy, so wrapping it again in a new HashMap here creates an unnecessary extra copy. Consider using the returned map directly.

    if (!comparableResourceVersions || temporaryResourceCache.isEmpty()) {
      return stream;
    }
    var tempResources = new HashMap<>(temporaryResourceCache.getResources());
    if (tempResources.isEmpty()) {
      return stream;

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (2)

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java:574

• Same issue as above: this stubs list(String, Predicate) but the code under test calls the 1-arg list(String) overload. Update the stub so it matches the invoked overload (or stub both) to avoid the mock returning null and breaking the test.

    var mim = mock(InformerManager.class);
    when(mim.list(nullable(String.class), any())).thenReturn(Stream.of(original));
    doReturn(mim).when(informerEventSource).manager();

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

• Same redundant copy here: getResources() already returns a defensive copy, so new HashMap<>(temporaryResourceCache.getResources()) makes an extra copy/allocation without adding safety.

    if (!comparableResourceVersions || temporaryResourceCache.isEmpty()) {
      return stream;
    }
    var tempResources = new HashMap<>(temporaryResourceCache.getResources());
    if (tempResources.isEmpty()) {
      return stream;

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[metacosm]

Shouldn't we return an immutable version here instead of a mutable copy?

[csviri]

This is intentional since later we remove elements from that map.

[metacosm]

I think it'd be better to let the client code make that copy, then so that it's explicit that it's working out of a copy, without having to look at the getResources code. Otherwise, we might think that we're directly modifying the cache itself.

[csviri]

In that case I would return it as unmodifiable map, and wrap it on reader, this would be a nicer API for sure. I just did not bother since it is an internal API (and unittested). But will do it, if this caught your eye.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.