Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ActionService and ThingActions classes in Xtext cache #1714

Merged
merged 1 commit into from Oct 13, 2020

Conversation

wborn
Copy link
Member

@wborn wborn commented Oct 11, 2020

Xtext uses a cache for looking up classes when rules are run.
It also adds a null class value to this cache when a class is not found.

Once a value has entered the cache it will not be updated.
This causes the cache to return the wrong class (or the null value) when
calling static methods on ActionService and ThingActions classes that
were added/updated.

With the changes in this PR Xtext will be configured to use a custom cache
that updates the ActionService and ThingActions class references.

The PR also has a fix for the AnnotatedThingActionModuleTypeProvider not
properly sending ModuleType removed events when all ThingActions
registrations have been removed.

Fixes #1265
Fixes #1694

Xtext uses a cache for looking up classes when rules are run.
It also adds a null class value to this cache when a class is not found.

Once a value has entered the cache it will not be updated.
This causes the cache to return the wrong class (or the null value) when
calling static methods on ActionService and ThingActions classes that
were added/updated.

With the changes in this PR Xtext will be configured to use a custom cache
that updates the ActionService and ThingActions class references.

The PR also has a fix for the AnnotatedThingActionModuleTypeProvider not
properly sending ModuleType removed events when all ThingActions
registrations have been removed.

Fixes openhab#1265
Fixes openhab#1694

Signed-off-by: Wouter Born <github@maindrain.net>
@wborn wborn requested a review from a team October 11, 2020 18:19
wborn added a commit to wborn/openhab-addons that referenced this pull request Oct 11, 2020
* Remove proxy workarounds
* Move ThingActions and a few other classes into the internal package
* Use more consistent action labels/descriptions

Related to:

* openhab/openhab-core#1714
* openhab/openhab-core#1639

Signed-off-by: Wouter Born <github@maindrain.net>
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Wow, looks like pure magic and I do not claim to understand all details what you did there and even less how you have figure out to do it - but the result is fantastic 🎉 .

@kaikreuzer kaikreuzer merged commit 0d1a15e into openhab:master Oct 13, 2020
@wborn wborn deleted the fix-rules-class-caching branch October 13, 2020 21:34
J-N-K pushed a commit to openhab/openhab-addons that referenced this pull request Oct 14, 2020
* Remove proxy workarounds
* Move ThingActions and a few other classes into the internal package
* Use more consistent action labels/descriptions

Related to:

* openhab/openhab-core#1714
* openhab/openhab-core#1639

Signed-off-by: Wouter Born <github@maindrain.net>
@cweitkamp
Copy link
Contributor

@wborn Thanks again for this implementation. But I am afraid is is not the final solution. I still see an exception when updating a binding manually in the addons/ folder:

2020-10-30 12:04:52.830 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'b5c106f5b5' failed: class org.openhab.binding.pushover.internal.actions.PushoverActions cannot be cast to class org.openhab.binding.pushover.internal.actions.PushoverActions (org.openhab.binding.pushover.internal.actions.PushoverActions is in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @1979a84; org.openhab.binding.pushover.internal.actions.PushoverActions is in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @610f03)

@wborn
Copy link
Member Author

wborn commented Oct 30, 2020

I think there is still one other issue unrelated to this cache. I've also sometimes seen that if you would install a binding the ThingActions service registration is sometimes not created. As a result it would then also not be available when creating a rule using the UI.

So when that occurs the cache entry is probably also not updated. You can enable debug logging on org.openhab.core.model.rule.scoping.RulesClassCache to see if the ServiceRegistration for the binding ThingActions is created and causes the cache entry to be updated.

@cweitkamp
Copy link
Contributor

cweitkamp commented Oct 30, 2020

I removed the *.jar file and moved a new one into my addons/ folder. I got the updated message - but still the same error:

2020-10-30 14:20:58.280 [DEBUG] [e.model.rule.scoping.RulesClassCache] - Updated cache entry: org.openhab.binding.pushover.internal.actions.PushoverActions
...
2020-10-30 14:21:04.675 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'b5c106f5b5' failed: class org.openhab.binding.pushover.internal.actions.PushoverActions cannot be cast to class org.openhab.binding.pushover.internal.actions.PushoverActions (org.openhab.binding.pushover.internal.actions.PushoverActions is in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @b20289; org.openhab.binding.pushover.internal.actions.PushoverActions is in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @1666cf7)

@wborn
Copy link
Member Author

wborn commented Oct 30, 2020

Are you using DSL rules or some other scripting language?

@cweitkamp
Copy link
Contributor

These test were done with DSL rules. Created via UI.

@kaikreuzer kaikreuzer added this to the 3.0.0.M2 milestone Nov 2, 2020
@kaikreuzer kaikreuzer added the enhancement An enhancement or new feature of the Core label Nov 2, 2020
nowaterman pushed a commit to nowaterman/openhab-addons that referenced this pull request Jan 19, 2021
* Remove proxy workarounds
* Move ThingActions and a few other classes into the internal package
* Use more consistent action labels/descriptions

Related to:

* openhab/openhab-core#1714
* openhab/openhab-core#1639
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/tr-064-thing-action-phonebooklookup-not-working/113101/38

marcfischerboschio pushed a commit to marcfischerboschio/openHABaddon that referenced this pull request Apr 20, 2022
* Remove proxy workarounds
* Move ThingActions and a few other classes into the internal package
* Use more consistent action labels/descriptions

Related to:

* openhab/openhab-core#1714
* openhab/openhab-core#1639

Signed-off-by: Wouter Born <github@maindrain.net>
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 11, 2023
…#1714)

Xtext uses a cache for looking up classes when rules are run.
It also adds a null class value to this cache when a class is not found.

Once a value has entered the cache it will not be updated.
This causes the cache to return the wrong class (or the null value) when
calling static methods on ActionService and ThingActions classes that
were added/updated.

With the changes in this PR Xtext will be configured to use a custom cache
that updates the ActionService and ThingActions class references.

The PR also has a fix for the AnnotatedThingActionModuleTypeProvider not
properly sending ModuleType removed events when all ThingActions
registrations have been removed.

Fixes openhab#1265
Fixes openhab#1694

Signed-off-by: Wouter Born <github@maindrain.net>
GitOrigin-RevId: 0d1a15e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core
Projects
None yet
4 participants