Skip to content

refactor: let ResourceState own the trigger-on-all-events flag - #3549

Open
csviri wants to merge 1 commit into
operator-framework:nextfrom
csviri:resource-state-trigger-flag
Open

refactor: let ResourceState own the trigger-on-all-events flag#3549
csviri wants to merge 1 commit into
operator-framework:nextfrom
csviri:resource-state-trigger-flag

Conversation

@csviri

@csviri csviri commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

markEventReceived and unMarkEventReceived took a boolean that only ever selects
which IllegalStateException guards apply, and every EventProcessor call site
supplied it by re-reading the same controller configuration value. It can never
differ between calls for a given processor, but nothing enforced that: a call
site passing the wrong value would silently change which state transitions are
legal, and the state machine could not be read without also reading its callers.

Decide it once at the edge: ResourceStateManager takes the flag at construction
(EventProcessor already knows it there) and passes it to each ResourceState,
which keeps it as a final field. Both types are package-private, so this is
self-contained.


Quality-only change: no intended behavior difference. Cut from next and
touches a disjoint set of files from the sibling cleanup PRs, so it can be merged
independently and in any order.

Verified on this branch alone: mvn -o -pl operator-framework-core,operator-framework-junit -am test
(693 core + 6 junit tests, no failures) and mvn spotless:check.

markEventReceived and unMarkEventReceived took a boolean that only ever selects
which IllegalStateException guards apply, and every EventProcessor call site
supplied it by re-reading the same controller configuration value. It can never
differ between calls for a given processor, but nothing enforced that: a call
site passing the wrong value would silently change which state transitions are
legal, and the state machine could not be read without also reading its callers.

Decide it once at the edge: ResourceStateManager takes the flag at construction
(EventProcessor already knows it there) and passes it to each ResourceState,
which keeps it as a final field. Both types are package-private, so this is
self-contained.
Copilot AI lite review requested due to automatic review settings August 7, 2026 11:55
@openshift-ci
openshift-ci Bot requested review from metacosm and xstefank August 7, 2026 11:55
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c0f8babc-8b2c-42d7-86ba-3b5d96e12687

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors event-state handling so the triggerReconcilerOnAllEvents flag is decided once at EventProcessor construction time, stored in ResourceStateManager, and then owned by each ResourceState instance (as an immutable field). This removes repeated flag plumbing across call sites and makes the ResourceState transition rules self-contained.

Changes:

  • Add a triggerOnAllEvents constructor parameter to ResourceStateManager and propagate it when creating ResourceState instances.
  • Move the “all-events mode” gating logic into ResourceState by storing the flag as a final field and removing boolean parameters from markEventReceived / unMarkEventReceived.
  • Update EventProcessor and unit tests to use the new constructors and method signatures.

Reviewed changes

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

File Description
operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ResourceStateManagerTest.java Updates tests to construct ResourceStateManager with the mode flag and use the new no-arg markEventReceived / unMarkEventReceived APIs.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ResourceStateManager.java Stores triggerOnAllEvents once and passes it to ResourceState creation paths.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ResourceState.java Makes the “trigger on all events” mode an intrinsic part of the state machine instance; removes per-call boolean parameters.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java Constructs ResourceStateManager with controller configuration and removes redundant flag passing to ResourceState methods.

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.

2 participants