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.

Sign up for GitHub

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

Jump to bottom

Fix rule startlevel trigger executes during initialization #3717

Merged
merged 2 commits into from
Jul 21, 2023
Merged

Fix rule startlevel trigger executes during initialization #3717

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f22c90d
Fix rule startlevel trigger executes during initialization
J-N-K Jul 20, 2023
750bdb6
fix itests
J-N-K Jul 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion ...ab.core.automation/src/main/java/org/openhab/core/automation/internal/RuleEngineImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ public class RuleEngineImpl implements RuleManager, RegistryChangeListener<Modul
* The storage for the disable information
*/
private final Storage<Boolean> disabledRulesStorage;
private final StartLevelService startLevelService;

/**
* Locker which does not permit rule initialization when the rule engine is stopping.
Expand Down Expand Up @@ -254,7 +255,7 @@ public void runNow(String uid, boolean considerConditions, @Nullable Map<String,
@Activate
public RuleEngineImpl(final @Reference ModuleTypeRegistry moduleTypeRegistry,
final @Reference RuleRegistry ruleRegistry, final @Reference StorageService storageService,
final @Reference ReadyService readyService) {
final @Reference ReadyService readyService, final @Reference StartLevelService startLevelService) {
this.disabledRulesStorage = storageService.<Boolean> getStorage(DISABLED_RULE_STORAGE,
this.getClass().getClassLoader());

Expand All @@ -265,6 +266,7 @@ public RuleEngineImpl(final @Reference ModuleTypeRegistry moduleTypeRegistry,

this.ruleRegistry = ruleRegistry;
this.readyService = readyService;
this.startLevelService = startLevelService;

listener = new RegistryChangeListener<>() {
@Override
Expand Down Expand Up @@ -846,6 +848,17 @@ private boolean activateRule(final WrappedRule rule) {
// Register the rule and set idle status.
register(rule);
setStatus(ruleUID, new RuleStatusInfo(RuleStatus.IDLE));

// check if we have to trigger because of the startlevel
List<Trigger> slTriggers = rule.getTriggers().stream().map(WrappedTrigger::unwrap)
.filter(t -> SystemTriggerHandler.STARTLEVEL_MODULE_TYPE_ID.equals(t.getTypeUID())).toList();
if (slTriggers.stream()
.anyMatch(t -> ((BigDecimal) t.getConfiguration().get(SystemTriggerHandler.CFG_STARTLEVEL))
.intValue() <= startLevelService.getStartLevel())) {
runNow(rule.getUID(), true,
Map.of(SystemTriggerHandler.OUT_STARTLEVEL, StartLevelService.STARTLEVEL_RULES));
}

return true;
}

Expand Down
2 changes: 1 addition & 1 deletion ...in/java/org/openhab/core/automation/internal/module/factory/CoreModuleHandlerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public Collection<String> getTypes() {
} else if (ItemCommandTriggerHandler.MODULE_TYPE_ID.equals(moduleTypeUID)) {
return new ItemCommandTriggerHandler(trigger, ruleUID, bundleContext, itemRegistry);
} else if (SystemTriggerHandler.STARTLEVEL_MODULE_TYPE_ID.equals(moduleTypeUID)) {
return new SystemTriggerHandler(trigger, bundleContext, startLevelService);
return new SystemTriggerHandler(trigger, bundleContext);
} else if (ThingStatusTriggerHandler.CHANGE_MODULE_TYPE_ID.equals(moduleTypeUID)
|| ThingStatusTriggerHandler.UPDATE_MODULE_TYPE_ID.equals(moduleTypeUID)) {
return new ThingStatusTriggerHandler(trigger, bundleContext);
Expand Down
10 changes: 1 addition & 9 deletions ...c/main/java/org/openhab/core/automation/internal/module/handler/SystemTriggerHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,13 @@ public class SystemTriggerHandler extends BaseTriggerModuleHandler implements Ev

private final Integer startlevel;
private final Set<String> types;
private final StartLevelService startLevelService;

private boolean triggered = false;

private final ServiceRegistration<?> eventSubscriberRegistration;

public SystemTriggerHandler(Trigger module, BundleContext bundleContext, StartLevelService startLevelService) {
public SystemTriggerHandler(Trigger module, BundleContext bundleContext) {
super(module);
this.startLevelService = startLevelService;
this.startlevel = ((BigDecimal) module.getConfiguration().get(CFG_STARTLEVEL)).intValue();
if (STARTLEVEL_MODULE_TYPE_ID.equals(module.getTypeUID())) {
this.types = Set.of(StartlevelEvent.TYPE);
Expand All @@ -69,12 +67,6 @@ public SystemTriggerHandler(Trigger module, BundleContext bundleContext, StartLe
@Override
public void setCallback(ModuleHandlerCallback callback) {
super.setCallback(callback);

// trigger immediately when start level is already reached
int currentStartLevel = startLevelService.getStartLevel();
if (currentStartLevel > StartLevelService.STARTLEVEL_RULEENGINE && currentStartLevel >= startlevel) {
trigger();
}
}

@Override
Expand Down
47 changes: 4 additions & 43 deletions ...st/java/org/openhab/core/automation/internal/module/handler/SystemTriggerHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,34 +67,17 @@ public void setup() {
public void testDoesNotTriggerIfStartLevelTooLow() {
when(startLevelServiceMock.getStartLevel()).thenReturn(0);

SystemTriggerHandler triggerHandler = new SystemTriggerHandler(triggerMock, bundleContextMock,
startLevelServiceMock);
SystemTriggerHandler triggerHandler = new SystemTriggerHandler(triggerMock, bundleContextMock);
triggerHandler.setCallback(callbackMock);

verifyNoInteractions(callbackMock);
}

@Test
public void testDoesTriggerImmediatelyIfStartLevelHigherOnInit() {
when(startLevelServiceMock.getStartLevel()).thenReturn(100);

SystemTriggerHandler triggerHandler = new SystemTriggerHandler(triggerMock, bundleContextMock,
startLevelServiceMock);
triggerHandler.setCallback(callbackMock);

ArgumentCaptor<Map> captor = ArgumentCaptor.forClass(Map.class);
verify(callbackMock).triggered(eq(triggerMock), captor.capture());

Map<String, Object> configuration = (Map<String, Object>) captor.getValue();
assertThat(configuration.get(SystemTriggerHandler.OUT_STARTLEVEL), is(CFG_STARTLEVEL));
}

@Test
public void testDoesNotTriggerIfStartLevelEventLower() {
when(startLevelServiceMock.getStartLevel()).thenReturn(0);

SystemTriggerHandler triggerHandler = new SystemTriggerHandler(triggerMock, bundleContextMock,
startLevelServiceMock);
SystemTriggerHandler triggerHandler = new SystemTriggerHandler(triggerMock, bundleContextMock);
triggerHandler.setCallback(callbackMock);

Event event = SystemEventFactory.createStartlevelEvent(70);
Expand All @@ -107,8 +90,7 @@ public void testDoesNotTriggerIfStartLevelEventLower() {
public void testDoesTriggerIfStartLevelEventHigher() {
when(startLevelServiceMock.getStartLevel()).thenReturn(0);

SystemTriggerHandler triggerHandler = new SystemTriggerHandler(triggerMock, bundleContextMock,
startLevelServiceMock);
SystemTriggerHandler triggerHandler = new SystemTriggerHandler(triggerMock, bundleContextMock);
triggerHandler.setCallback(callbackMock);

Event event = SystemEventFactory.createStartlevelEvent(100);
Expand All @@ -121,32 +103,11 @@ public void testDoesTriggerIfStartLevelEventHigher() {
assertThat(configuration.get(SystemTriggerHandler.OUT_STARTLEVEL), is(CFG_STARTLEVEL));
}

@Test
public void testDoesNotTriggerAfterInitialTrigger() {
when(startLevelServiceMock.getStartLevel()).thenReturn(100);

SystemTriggerHandler triggerHandler = new SystemTriggerHandler(triggerMock, bundleContextMock,
startLevelServiceMock);
triggerHandler.setCallback(callbackMock);

ArgumentCaptor<Map> captor = ArgumentCaptor.forClass(Map.class);
verify(callbackMock).triggered(eq(triggerMock), captor.capture());

Map<String, Object> configuration = (Map<String, Object>) captor.getValue();
assertThat(configuration.get(SystemTriggerHandler.OUT_STARTLEVEL), is(CFG_STARTLEVEL));

Event event = SystemEventFactory.createStartlevelEvent(100);
triggerHandler.receive(event);

verifyNoMoreInteractions(callbackMock);
}

@Test
public void testDoesNotTriggerAfterEventTrigger() {
when(startLevelServiceMock.getStartLevel()).thenReturn(0);

SystemTriggerHandler triggerHandler = new SystemTriggerHandler(triggerMock, bundleContextMock,
startLevelServiceMock);
SystemTriggerHandler triggerHandler = new SystemTriggerHandler(triggerMock, bundleContextMock);
triggerHandler.setCallback(callbackMock);

Event event = SystemEventFactory.createStartlevelEvent(100);
Expand Down