Fix rule startlevel trigger executes during initialization (#3717)

* Fix rule startlevel trigger executes during initialization

Signed-off-by: Jan N. Klug <github@klug.nrw>
This commit is contained in:
J-N-K 2023-07-21 14:18:28 +02:00 committed by GitHub
parent 72ec0f3517
commit b8ae55c3f5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 74 additions and 56 deletions

View File

@ -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.
@ -254,7 +255,7 @@ public class RuleEngineImpl implements RuleManager, RegistryChangeListener<Modul
@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());
@ -265,6 +266,7 @@ public class RuleEngineImpl implements RuleManager, RegistryChangeListener<Modul
this.ruleRegistry = ruleRegistry;
this.readyService = readyService;
this.startLevelService = startLevelService;
listener = new RegistryChangeListener<>() {
@Override
@ -846,6 +848,17 @@ public class RuleEngineImpl implements RuleManager, RegistryChangeListener<Modul
// 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;
}

View File

@ -116,7 +116,7 @@ public class CoreModuleHandlerFactory extends BaseModuleHandlerFactory implement
} 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);

View File

@ -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);
@ -69,12 +67,6 @@ public class SystemTriggerHandler extends BaseTriggerModuleHandler implements Ev
@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

View File

@ -67,34 +67,17 @@ public class SystemTriggerHandlerTest {
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);
@ -107,8 +90,7 @@ public class SystemTriggerHandlerTest {
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);
@ -121,32 +103,11 @@ public class SystemTriggerHandlerTest {
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);

View File

@ -16,6 +16,7 @@ import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.util.Collection;
import java.util.Optional;
@ -80,6 +81,7 @@ public class AutomationIntegrationJsonTest extends JavaOSGiTest {
private final Logger logger = LoggerFactory.getLogger(AutomationIntegrationJsonTest.class);
private @NonNullByDefault({}) EventPublisher eventPublisher;
private @NonNullByDefault({}) ItemRegistry itemRegistry;
private @NonNullByDefault({}) StartLevelService startLevelService;
private @NonNullByDefault({}) RuleRegistry ruleRegistry;
private @NonNullByDefault({}) RuleManager ruleManager;
private @NonNullByDefault({}) ManagedRuleProvider managedRuleProvider;
@ -96,8 +98,12 @@ public class AutomationIntegrationJsonTest extends JavaOSGiTest {
eventPublisher = getService(EventPublisher.class);
itemRegistry = getService(ItemRegistry.class);
startLevelService = mock(StartLevelService.class);
when(startLevelService.getStartLevel()).thenReturn(100);
registerService(startLevelService, StartLevelService.class.getName());
CoreModuleHandlerFactory coreModuleHandlerFactory = new CoreModuleHandlerFactory(getBundleContext(),
eventPublisher, itemRegistry, mock(TimeZoneProvider.class), mock(StartLevelService.class));
eventPublisher, itemRegistry, mock(TimeZoneProvider.class), startLevelService);
mock(CoreModuleHandlerFactory.class);
registerService(coreModuleHandlerFactory);

View File

@ -17,6 +17,7 @@ import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.util.Collection;
import java.util.Collections;
@ -98,6 +99,7 @@ public class AutomationIntegrationTest extends JavaOSGiTest {
private final Logger logger = LoggerFactory.getLogger(AutomationIntegrationTest.class);
private @Nullable EventPublisher eventPublisher;
private @Nullable ItemRegistry itemRegistry;
private @NonNullByDefault({}) StartLevelService startLevelService;
private @Nullable RuleRegistry ruleRegistry;
private @Nullable RuleManager ruleEngine;
private @Nullable ManagedRuleProvider managedRuleProvider;
@ -113,9 +115,13 @@ public class AutomationIntegrationTest extends JavaOSGiTest {
eventPublisher = getService(EventPublisher.class);
itemRegistry = getService(ItemRegistry.class);
startLevelService = mock(StartLevelService.class);
when(startLevelService.getStartLevel()).thenReturn(100);
registerService(startLevelService, StartLevelService.class.getName());
CoreModuleHandlerFactory coreModuleHandlerFactory = new CoreModuleHandlerFactory(getBundleContext(),
Objects.requireNonNull(eventPublisher), Objects.requireNonNull(itemRegistry),
mock(TimeZoneProvider.class), mock(StartLevelService.class));
mock(TimeZoneProvider.class), startLevelService);
mock(CoreModuleHandlerFactory.class);
registerService(coreModuleHandlerFactory);

View File

@ -15,6 +15,8 @@ package org.openhab.core.automation.integration.test;
import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.time.ZoneId;
import java.time.ZonedDateTime;
@ -54,6 +56,7 @@ import org.openhab.core.automation.util.RuleBuilder;
import org.openhab.core.common.registry.ProviderChangeListener;
import org.openhab.core.config.core.Configuration;
import org.openhab.core.service.ReadyMarker;
import org.openhab.core.service.StartLevelService;
import org.openhab.core.storage.StorageService;
import org.openhab.core.test.java.JavaOSGiTest;
import org.slf4j.Logger;
@ -71,11 +74,16 @@ public class RuleSimulationTest extends JavaOSGiTest {
private @Nullable RuleRegistry ruleRegistry;
private @Nullable RuleManager ruleEngine;
private @NonNullByDefault({}) StartLevelService startLevelService;
@BeforeEach
public void before() {
registerVolatileStorageService();
startLevelService = mock(StartLevelService.class);
when(startLevelService.getStartLevel()).thenReturn(100);
registerService(startLevelService, StartLevelService.class.getName());
StorageService storageService = getService(StorageService.class);
ruleRegistry = getService(RuleRegistry.class);

View File

@ -15,6 +15,7 @@ package org.openhab.core.automation.internal.module;
import static java.util.Map.entry;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.util.ArrayList;
import java.util.Collection;
@ -69,9 +70,13 @@ public class RunRuleModuleTest extends JavaOSGiTest {
private final Logger logger = LoggerFactory.getLogger(RunRuleModuleTest.class);
private final VolatileStorageService volatileStorageService = new VolatileStorageService();
private @NonNullByDefault({}) StartLevelService startLevelService;
@BeforeEach
public void before() {
startLevelService = mock(StartLevelService.class);
when(startLevelService.getStartLevel()).thenReturn(100);
registerService(startLevelService, StartLevelService.class.getName());
EventPublisher eventPublisher = getService(EventPublisher.class);
ItemRegistry itemRegistry = getService(ItemRegistry.class);
CoreModuleHandlerFactory coreModuleHandlerFactory = new CoreModuleHandlerFactory(getBundleContext(),

View File

@ -15,6 +15,7 @@ package org.openhab.core.automation.internal.module;
import static java.util.Map.entry;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.util.Collection;
import java.util.Collections;
@ -79,9 +80,13 @@ public class RuntimeRuleTest extends JavaOSGiTest {
private final Logger logger = LoggerFactory.getLogger(RuntimeRuleTest.class);
private final VolatileStorageService volatileStorageService = new VolatileStorageService();
private @NonNullByDefault({}) StartLevelService startLevelService;
@BeforeEach
public void before() {
startLevelService = mock(StartLevelService.class);
when(startLevelService.getStartLevel()).thenReturn(100);
registerService(startLevelService, StartLevelService.class.getName());
EventPublisher eventPublisher = getService(EventPublisher.class);
ItemRegistry itemRegistry = getService(ItemRegistry.class);
CoreModuleHandlerFactory coreModuleHandlerFactory = new CoreModuleHandlerFactory(getBundleContext(),

View File

@ -16,6 +16,7 @@ import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.util.ArrayList;
import java.util.Collection;
@ -78,6 +79,7 @@ public abstract class BasicConditionHandlerTest extends JavaOSGiTest {
private @NonNullByDefault({}) RuleRegistry ruleRegistry;
private @NonNullByDefault({}) RuleManager ruleEngine;
private @Nullable Event itemEvent;
private @NonNullByDefault({}) StartLevelService startLevelService;
/**
* This executes before every test and before the
@ -86,6 +88,9 @@ public abstract class BasicConditionHandlerTest extends JavaOSGiTest {
*/
@BeforeEach
public void beforeBase() {
startLevelService = mock(StartLevelService.class);
when(startLevelService.getStartLevel()).thenReturn(100);
registerService(startLevelService, StartLevelService.class.getName());
EventPublisher eventPublisher = getService(EventPublisher.class);
ItemRegistry itemRegistry = getService(ItemRegistry.class);
CoreModuleHandlerFactory coreModuleHandlerFactory = new CoreModuleHandlerFactory(getBundleContext(),

View File

@ -16,6 +16,7 @@ import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.util.Collection;
import java.util.HashMap;
@ -75,9 +76,13 @@ public class RuntimeRuleTest extends JavaOSGiTest {
private VolatileStorageService volatileStorageService = new VolatileStorageService();
private @NonNullByDefault({}) RuleRegistry ruleRegistry;
private @NonNullByDefault({}) RuleManager ruleEngine;
private @NonNullByDefault({}) StartLevelService startLevelService;
@BeforeEach
public void before() {
startLevelService = mock(StartLevelService.class);
when(startLevelService.getStartLevel()).thenReturn(100);
registerService(startLevelService, StartLevelService.class.getName());
EventPublisher eventPublisher = getService(EventPublisher.class);
ItemRegistry itemRegistry = getService(ItemRegistry.class);
CoreModuleHandlerFactory coreModuleHandlerFactory = new CoreModuleHandlerFactory(getBundleContext(),

View File

@ -17,6 +17,7 @@ import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.util.ArrayList;
import java.util.Collection;
@ -78,12 +79,16 @@ public class RuleEventTest extends JavaOSGiTest {
private @Nullable Event itemEvent = null;
private @Nullable Event ruleRemovedEvent = null;
private @NonNullByDefault({}) StartLevelService startLevelService;
public RuleEventTest() {
}
@BeforeEach
public void before() {
startLevelService = mock(StartLevelService.class);
when(startLevelService.getStartLevel()).thenReturn(100);
registerService(startLevelService, StartLevelService.class.getName());
EventPublisher eventPublisher = getService(EventPublisher.class);
ItemRegistry itemRegistry = getService(ItemRegistry.class);
CoreModuleHandlerFactory coreModuleHandlerFactory = new CoreModuleHandlerFactory(getBundleContext(),

View File

@ -13,6 +13,8 @@
package org.openhab.core.automation.internal;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.math.BigDecimal;
import java.util.ArrayList;
@ -41,6 +43,7 @@ import org.openhab.core.config.core.ConfigDescriptionParameterBuilder;
import org.openhab.core.config.core.Configuration;
import org.openhab.core.config.core.FilterCriteria;
import org.openhab.core.config.core.ParameterOption;
import org.openhab.core.service.StartLevelService;
import org.openhab.core.test.java.JavaOSGiTest;
/**
@ -54,10 +57,14 @@ public class RuleEngineTest extends JavaOSGiTest {
private @NonNullByDefault({}) RuleEngineImpl ruleEngine;
private @NonNullByDefault({}) RuleRegistry ruleRegistry;
private @NonNullByDefault({}) StartLevelService startLevelService;
@BeforeEach
public void setup() {
registerVolatileStorageService();
startLevelService = mock(StartLevelService.class);
when(startLevelService.getStartLevel()).thenReturn(100);
registerService(startLevelService, StartLevelService.class.getName());
ruleEngine = (RuleEngineImpl) getService(RuleManager.class);
ruleRegistry = getService(RuleRegistry.class);
registerService(new TestModuleTypeProvider(), ModuleTypeProvider.class.getName());