Keep resolveConfig in sync - #5776
Conversation
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
|
fyi @wborn / @florian-h05 |
There was a problem hiding this comment.
Pull request overview
This PR addresses a correctness gap introduced by caching resolved Thing configuration in BaseThingHandler: replacing this.thing directly could leave getConfig() / getConfigAs() returning values resolved from a previous Thing. It extracts the Thing+cache replacement logic into a dedicated protected method so custom thingUpdated(Thing) implementations can keep the cache consistent without forcing a full dispose/reinitialize lifecycle.
Changes:
- Extracted Thing replacement + resolved-config refresh into a new protected helper (
setThing(Thing)), and updated the defaultthingUpdated(Thing)to use it. - Added a unit test that primes the config cache, replaces the Thing via the new helper, and asserts the resolved configuration is updated.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/binding/BaseThingHandler.java | Adds a protected helper to replace the handled Thing while keeping the resolved configuration cache in sync. |
| bundles/org.openhab.core.thing/src/test/java/org/openhab/core/thing/binding/BaseThingHandlerTest.java | Adds coverage to ensure setThing() updates the resolved configuration cache even after it was previously populated. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
florian-h05
left a comment
There was a problem hiding this comment.
LGTM, thanks for the fix!
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
As a follow up to #5568
That PR made the
BaseThingHandlercaches the resolved Thing configuration inresolvedConfig.The default
thingUpdated(Thing)implementation updates both the Thing and this cache. Custom implementations cannot do the same becauseresolvedConfigis private. Assigning a new value tothis.thingcan therefore leavegetConfig()andgetConfigAs()returning configuration resolved from the previous Thing.This change extracts the existing replacement logic into a protected final
setThing(Thing)method. The defaultthingUpdated(Thing)implementation uses this method, so its behavior remains unchanged.Custom implementations can now replace the Thing without using the default dispose/reinitialize lifecycle while keeping the resolved configuration cache in sync.
A unit test was added that populates the old cache, replaces the Thing using
setThing(), and verifies that the new resolved configuration is returned.