refactor: let ResourceState own the trigger-on-all-events flag - #3549
refactor: let ResourceState own the trigger-on-all-events flag#3549csviri wants to merge 1 commit into
Conversation
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.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
There was a problem hiding this comment.
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
triggerOnAllEventsconstructor parameter toResourceStateManagerand propagate it when creatingResourceStateinstances. - Move the “all-events mode” gating logic into
ResourceStateby storing the flag as a final field and removing boolean parameters frommarkEventReceived/unMarkEventReceived. - Update
EventProcessorand 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. |
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
nextandtouches 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.