Skip to content

feat(core): add by-name secondary resource lookup on Context#3373

Open
afalhambra-hivemq wants to merge 1 commit into
operator-framework:nextfrom
afalhambra-hivemq:feature/context-secondary-resource-by-name
Open

feat(core): add by-name secondary resource lookup on Context#3373
afalhambra-hivemq wants to merge 1 commit into
operator-framework:nextfrom
afalhambra-hivemq:feature/context-secondary-resource-by-name

Conversation

@afalhambra-hivemq
Copy link
Copy Markdown
Contributor

@afalhambra-hivemq afalhambra-hivemq commented May 22, 2026

Adds three typed by-name secondary-resource accessors to Context<P>:

  • <R extends HasMetadata> Optional<R> getSecondaryResource(Class<R>, String eventSourceName, ResourceID) (abstract)
  • <R extends HasMetadata> Optional<R> getSecondaryResource(Class<R>, String eventSourceName, String name) (default; uses the primary's namespace)
  • <R extends HasMetadata> Stream<R> getSecondaryResourcesAsStream(Class<R>, String eventSourceName) (abstract)

Closes #3372

Copilot AI review requested due to automatic review settings May 22, 2026 07:56
@openshift-ci openshift-ci Bot requested review from csviri and xstefank May 22, 2026 07:56
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds new Context APIs for retrieving secondary resources by ResourceID and streaming secondary resources from a specific named event source, with cache-backed fast paths and tests to validate behavior.

Changes:

  • Added Context#getSecondaryResource(..., ResourceID) and a convenience overload that infers namespace from the primary resource.
  • Added Context#getSecondaryResourcesAsStream(..., eventSourceName) with a ResourceCache fast path for read-cache-after-write consistency.
  • Expanded DefaultContextTest to cover cache hit/miss, fallback behavior, cluster-scoped vs namespaced primaries, and workflow-managed missing event sources.

Reviewed changes

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

File Description
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java Introduces new public API methods and Javadoc for ResourceID lookups and named event-source streaming.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java Implements the new APIs, adds cache/resource-cache fast paths, and refactors missing-event-source handling into a helper.
operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContextTest.java Adds unit tests validating the new APIs and fast-path/fallback behaviors.

@afalhambra-hivemq afalhambra-hivemq force-pushed the feature/context-secondary-resource-by-name branch 4 times, most recently from 495d579 to c8b2feb Compare May 22, 2026 09:02
Copy link
Copy Markdown
Collaborator

@metacosm metacosm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@afalhambra-hivemq afalhambra-hivemq force-pushed the feature/context-secondary-resource-by-name branch from c8b2feb to 1012314 Compare May 22, 2026 11:23
Copy link
Copy Markdown
Collaborator

@csviri csviri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR @afalhambra-hivemq !

Could you pls target 'next' branch so this ends up in next minor release rather than in next patch release.

I was also thinking if we should use ResourceID in the method or simply name and namespace directly it might be less user boilerplate code at the end,or? What do you think?

@afalhambra-hivemq afalhambra-hivemq force-pushed the feature/context-secondary-resource-by-name branch from 1012314 to 83dbac3 Compare May 25, 2026 06:10
Copy link
Copy Markdown
Collaborator

@xstefank xstefank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just please change the target branch and the @since tags. Thanks!

@afalhambra-hivemq afalhambra-hivemq force-pushed the feature/context-secondary-resource-by-name branch from 83dbac3 to 1b3bfe4 Compare May 25, 2026 06:43
@afalhambra-hivemq afalhambra-hivemq changed the base branch from main to next May 25, 2026 06:43
@afalhambra-hivemq
Copy link
Copy Markdown
Contributor Author

afalhambra-hivemq commented May 25, 2026

Thank you for the PR @afalhambra-hivemq !

Could you pls target 'next' branch so this ends up in next minor release rather than in next patch release.

I was also thinking if we should use ResourceID in the method or simply name and namespace directly it might be less user boilerplate code at the end,or? What do you think?

Agree, cleaner and less boilerplate for customers.

Also targeting next branch, and changed @since to 5.4.0 as suggested.

@afalhambra-hivemq afalhambra-hivemq force-pushed the feature/context-secondary-resource-by-name branch from 1b3bfe4 to 4b4da08 Compare May 25, 2026 06:47
@afalhambra-hivemq afalhambra-hivemq requested a review from csviri May 25, 2026 07:38
Signed-off-by: Antonio Fernandez Alhambra <antonio.alhambra@hivemq.com>
@afalhambra-hivemq afalhambra-hivemq force-pushed the feature/context-secondary-resource-by-name branch from 4b4da08 to 51b5e6f Compare May 25, 2026 10:59
@afalhambra-hivemq afalhambra-hivemq requested a review from csviri May 25, 2026 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Typed by-name secondary resource accessors on Context

5 participants