Fix more SAT findings and add a few suppressions (#2335)

* Fix more SAT findings and add a few suppressions

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

bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/ScriptFileWatcher.java

@@ -298,6 +298,7 @@ public class ScriptFileWatcher extends AbstractWatchService
     }
 
     @Override
+    @SuppressWarnings("PMD.EmptyWhileStmt")
     public synchronized void onReadyMarkerRemoved(ReadyMarker readyMarker) {
         int newLevel = Integer.parseInt(readyMarker.getIdentifier());
 

bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/ScriptFileWatcherTest.java

@@ -12,8 +12,7 @@
  */
 package org.openhab.core.automation.module.script.rulesupport.internal.loader;
 
-import static java.nio.file.StandardWatchEventKinds.ENTRY_CREATE;
+import static java.nio.file.StandardWatchEventKinds.*;
-import static java.nio.file.StandardWatchEventKinds.ENTRY_MODIFY;
 import static org.junit.jupiter.api.Assertions.fail;
 import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.*;
@@ -26,6 +25,7 @@ import java.util.concurrent.ScheduledExecutorService;
 
 import javax.script.ScriptEngine;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -45,15 +45,15 @@ import org.opentest4j.AssertionFailedError;
  *
  * @author Jonathan Gilbert - initial contribution
  */
+@NonNullByDefault
 class ScriptFileWatcherTest {
 
-    private ScriptFileWatcher scriptFileWatcher;
+    private @NonNullByDefault({}) ScriptFileWatcher scriptFileWatcher;
-    private ScriptEngineManager scriptEngineManager;
+    private @NonNullByDefault({}) ScriptEngineManager scriptEngineManager;
-    private DependencyTracker dependencyTracker;
+    private @NonNullByDefault({}) DependencyTracker dependencyTracker;
-    private ReadyService readyService;
+    private @NonNullByDefault({}) ReadyService readyService;
 
-    @TempDir
+    protected @NonNullByDefault({}) @TempDir Path tempScriptDir;
-    Path tempScriptDir;
 
     @BeforeEach
     public void setUp() {
@@ -324,8 +324,11 @@ class ScriptFileWatcherTest {
         verify(dependencyTracker).addLibForScript(p.toFile().toURI().toString(), "test");
     }
 
+    /**
+     * @see https://github.com/openhab/openhab-core/issues/2246
+     */
     @Test
-    public void testRemoveBeforeReAdd_bug2246() {
+    public void testRemoveBeforeReAdd() {
         when(scriptEngineManager.isSupported("js")).thenReturn(true);
         ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class);
         when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class));

bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/provider/AutomationResourceBundlesEventQueue.java

@@ -125,8 +125,10 @@ public class AutomationResourceBundlesEventQueue<@NonNull E> implements Runnable
                             return;
                         }
                     }
+                } catch (IllegalStateException e) {
+                    continue;
                 } catch (Throwable t) {
-                    if (!closed && !(t instanceof IllegalStateException)) {
+                    if (!closed) {
                         logger.warn("Processing bundle event {}, for automation resource bundle '{}' failed",
                                 event.getType(), event.getBundle().getSymbolicName(), t);
                     }

bundles/org.openhab.core.io.rest.auth/src/main/java/org/openhab/core/io/rest/auth/internal/AuthFilter.java

@@ -91,7 +91,7 @@ public class AuthFilter implements ContainerRequestFilter {
     private ExpiringUserSecurityContextCache authCache = new ExpiringUserSecurityContextCache(
             Duration.ofHours(cacheExpiration).toMillis());
 
-    private final byte[] RANDOM_BYTES = new byte[32];
+    private static final byte[] RANDOM_BYTES = new byte[32];
 
     private final JwtHelper jwtHelper;
     private final UserRegistry userRegistry;

bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/PlainMessageBodyReader.java

@@ -57,7 +57,7 @@ public class PlainMessageBodyReader<T> implements MessageBodyReader<T> {
         } else if (type.equals(Byte[].class) || genericType.equals(Byte[].class)) {
             final Byte[] dataB = new Byte[data.length];
             for (int i = 0; i < data.length; ++i) {
-                dataB[i] = data[i];
+                dataB[i] = Byte.valueOf(data[i]);
             }
             return (T) dataB;
         } else {

bundles/org.openhab.core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/link/EnrichedItemChannelLinkDTO.java

@@ -20,7 +20,7 @@ import org.openhab.core.thing.link.dto.ItemChannelLinkDTO;
  * This is an enriched data transfer object that is used to serialize items channel links with dynamic data like the
  * editable flag.
  *
- * @author Christoph Weitkamp- Initial contribution
+ * @author Christoph Weitkamp - Initial contribution
  */
 public class EnrichedItemChannelLinkDTO extends ItemChannelLinkDTO {
 

bundles/org.openhab.core.io.transport.modbus/src/test/java/org/openhab/core/io/transport/modbus/test/IntegrationTestSupport.java

@@ -38,6 +38,8 @@ import org.openhab.core.io.transport.modbus.endpoint.ModbusSlaveEndpoint;
 import org.openhab.core.io.transport.modbus.endpoint.ModbusTCPSlaveEndpoint;
 import org.openhab.core.io.transport.modbus.internal.ModbusManagerImpl;
 import org.openhab.core.test.java.JavaTest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import gnu.io.SerialPort;
 import net.wimpi.modbus.Modbus;
@@ -68,6 +70,8 @@ import net.wimpi.modbus.util.SerialParameters;
 @MockitoSettings(strictness = Strictness.WARN)
 public class IntegrationTestSupport extends JavaTest {
 
+    private final Logger logger = LoggerFactory.getLogger(IntegrationTestSupport.class);
+
     public enum ServerType {
         TCP,
         UDP,
@@ -175,11 +179,9 @@ public class IntegrationTestSupport extends JavaTest {
             if (ServerType.TCP.equals(serverType)) {
                 verify(tcpConnectionFactory, times(expectedConnections)).create(any(Socket.class));
             } else if (ServerType.UDP.equals(serverType)) {
-                // No-op
+                logger.debug("No-op, UDP server type");
-                // verify(udpTerminalFactory, times(expectedConnections)).create(any(InetAddress.class),
-                // any(Integer.class));
             } else if (ServerType.SERIAL.equals(serverType)) {
-                // No-op
+                logger.debug("No-op, SERIAL server type");
             } else {
                 throw new UnsupportedOperationException();
             }
@@ -206,14 +208,15 @@ public class IntegrationTestSupport extends JavaTest {
     private void stopServer() {
         if (ServerType.TCP.equals(serverType)) {
             tcpListener.stop();
+            logger.debug("Stopped TCP listener, tcpModbusPort={}", tcpModbusPort);
         } else if (ServerType.UDP.equals(serverType)) {
             udpListener.stop();
-            System.err.println(udpModbusPort);
+            logger.debug("Stopped UDP listener, udpModbusPort={}", udpModbusPort);
         } else if (ServerType.SERIAL.equals(serverType)) {
             try {
                 serialServerThread.join(100);
             } catch (InterruptedException e) {
-                System.err.println("Serial server thread .join() interrupted! Will interrupt it now.");
+                logger.debug("Serial server thread .join() interrupted! Will interrupt it now.");
             }
             serialServerThread.interrupt();
         } else {

bundles/org.openhab.core.io.transport.modbus/src/test/java/org/openhab/core/io/transport/modbus/test/SmokeTest.java

@@ -677,7 +677,6 @@ public class SmokeTest extends IntegrationTestSupport {
                             } catch (AssertionError e) {
                                 unexpectedCount.incrementAndGet();
                             }
-
                         } else {
                             unexpectedCount.incrementAndGet();
                         }
@@ -885,7 +884,6 @@ public class SmokeTest extends IntegrationTestSupport {
             Thread.sleep(1000);
             // Still one connection open even after closing second connection
             assertThat(getNumberOfOpenClients(SOCKET_SPY), is(equalTo(1L)));
-
         } // 4. close (the last) comms
           // ensure that open connections are closed
           // (despite huge "reconnect after millis")
@@ -937,7 +935,6 @@ public class SmokeTest extends IntegrationTestSupport {
                 long openSocketsAfter = getNumberOfOpenClients(SOCKET_SPY);
                 assertThat(openSocketsAfter, is(equalTo(0L)));
             }, 60_000, 50);
-
         }
     }
 

bundles/org.openhab.core.persistence/src/test/java/org/openhab/core/persistence/extensions/TestPersistenceService.java

@@ -83,8 +83,7 @@ public class TestPersistenceService implements QueryablePersistenceService {
             for (int i = 0; i <= 15; i++) {
                 final int hours = i;
                 final ZonedDateTime theDate = nowMinusFifteenHours.plusHours(hours);
-                if (theDate.isBefore(beginDate) || theDate.isAfter(endDate)) {
+                if (!theDate.isBefore(beginDate) && !theDate.isAfter(endDate)) {
-                } else {
                     results.add(new HistoricItem() {
                         @Override
                         public ZonedDateTime getTimestamp() {

bundles/org.openhab.core.test/src/main/java/org/openhab/core/test/java/JavaTest.java

@@ -180,6 +180,7 @@ public class JavaTest {
      * @param sleepTime interval for checking the condition
      * @return the return value of the supplied assertion object's function on success
      */
+    @SuppressWarnings("PMD.AvoidCatchingNPE")
     private <T> T waitForAssert(Supplier<T> assertion, @Nullable Runnable beforeLastCall, long timeout,
             long sleepTime) {
         final long timeoutNs = TimeUnit.MILLISECONDS.toNanos(timeout);

bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ChannelCommandDescriptionProvider.java

@@ -83,6 +83,7 @@ public class ChannelCommandDescriptionProvider implements CommandDescriptionProv
         return null;
     }
 
+    @SuppressWarnings("PMD.CompareObjectsWithEquals")
     private @Nullable CommandDescription getDynamicCommandDescription(Channel channel,
             @Nullable CommandDescription originalCommandDescription, @Nullable Locale locale) {
         for (DynamicCommandDescriptionProvider dynamicCommandDescriptionProvider : dynamicCommandDescriptionProviders) {
@@ -90,6 +91,7 @@ public class ChannelCommandDescriptionProvider implements CommandDescriptionProv
                 CommandDescription dynamicCommandDescription = dynamicCommandDescriptionProvider
                         .getCommandDescription(channel, originalCommandDescription, locale);
                 if (dynamicCommandDescription != null) {
+                    // Compare by reference to make sure a new command description is returned
                     if (dynamicCommandDescription == originalCommandDescription) {
                         logger.error(
                                 "Dynamic command description matches original command description. DynamicCommandDescriptionProvider implementations must never return the original command description. {} has to be fixed.",

bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ChannelStateDescriptionProvider.java

@@ -128,12 +128,14 @@ public class ChannelStateDescriptionProvider implements StateDescriptionFragment
         return null;
     }
 
+    @SuppressWarnings("PMD.CompareObjectsWithEquals")
     private @Nullable StateDescription getDynamicStateDescription(Channel channel,
             @Nullable StateDescription originalStateDescription, @Nullable Locale locale) {
         for (DynamicStateDescriptionProvider provider : dynamicStateDescriptionProviders) {
             StateDescription dynamicStateDescription = provider.getStateDescription(channel, originalStateDescription,
                     locale);
             if (dynamicStateDescription != null) {
+                // Compare by reference to make sure a new state description is returned
                 if (dynamicStateDescription == originalStateDescription) {
                     logger.error(
                             "Dynamic state description matches original state description. DynamicStateDescriptionProvider implementations must never return the original state description. {} has to be fixed.",

bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingManagerImpl.java

@@ -604,6 +604,7 @@ public class ThingManagerImpl
         }
     }
 
+    @SuppressWarnings("PMD.CompareObjectsWithEquals")
     private @Nullable ThingHandler replaceThing(@Nullable Thing oldThing, Thing newThing) {
         final ThingHandler thingHandler = thingHandlers.get(newThing.getUID());
         if (oldThing != newThing) {

bundles/org.openhab.core.ui.icon/src/main/java/org/openhab/core/ui/icon/internal/IconServlet.java

@@ -126,7 +126,7 @@ public class IconServlet extends OpenHABServlet {
                 if (provider == null) {
                     provider = provider2;
                     format = otherFormat;
-                } else if (provider2 != provider) {
+                } else if (!provider2.equals(provider)) {
                     Integer prio = provider.hasIcon(category, iconSetId, format);
                     Integer prio2 = provider2.hasIcon(category, iconSetId, otherFormat);
                     if ((prio != null && prio2 != null && prio < prio2) || (prio == null && prio2 != null)) {

bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/proxy/BlockingProxyServlet.java

@@ -100,14 +100,13 @@ public class BlockingProxyServlet extends HttpServlet {
                     HttpField header = iterator.next();
                     response.setHeader(header.getName(), header.getValue());
                 }
+            } catch (TimeoutException e) {
+                logger.warn("Proxy servlet failed to stream content due to a timeout");
+                response.sendError(HttpServletResponse.SC_GATEWAY_TIMEOUT);
+                return;
             } catch (Exception e) {
-                if (e instanceof TimeoutException) {
+                logger.warn("Proxy servlet failed to stream content: {}", e.getMessage());
-                    logger.warn("Proxy servlet failed to stream content due to a timeout");
+                response.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
-                    response.sendError(HttpServletResponse.SC_GATEWAY_TIMEOUT);
-                } else {
-                    logger.warn("Proxy servlet failed to stream content: {}", e.getMessage());
-                    response.sendError(HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
-                }
                 return;
             }
             // now copy/stream the body content

bundles/org.openhab.core/src/main/java/org/openhab/core/internal/events/ThreadedEventHandler.java

@@ -61,7 +61,7 @@ public class ThreadedEventHandler implements Closeable {
                         logger.trace("inspect event: {}", event);
                         if (event == null) {
                             logger.debug("Hey, you have really very few events.");
-                        } else if (event == notifyEvent) {
+                        } else if (event.equals(notifyEvent)) {
                             // received an internal notification
                         } else {
                             worker.handleEvent(event);

bundles/org.openhab.core/src/test/java/org/openhab/core/internal/items/ItemStateConverterImplTest.java

@@ -20,6 +20,7 @@ import static org.mockito.Mockito.*;
 import javax.measure.quantity.Length;
 import javax.measure.quantity.Temperature;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.openhab.core.i18n.UnitProvider;
@@ -38,9 +39,10 @@ import org.openhab.core.types.UnDefType;
  *
  * @author Henning Treu - Initial contribution
  */
+@NonNullByDefault
 public class ItemStateConverterImplTest {
 
-    private ItemStateConverterImpl itemStateConverter;
+    private @NonNullByDefault({}) ItemStateConverterImpl itemStateConverter;
 
     @BeforeEach
     public void setup() {
@@ -57,6 +59,7 @@ public class ItemStateConverterImplTest {
     }
 
     @Test
+    @SuppressWarnings("PMD.CompareObjectsWithEquals")
     public void testNoConversion() {
         Item item = new NumberItem("number");
         State originalState = new DecimalType(12.34);

itests/org.openhab.core.automation.integration.tests/src/main/java/org/openhab/core/automation/integration/test/AutomationIntegrationTest.java

@@ -565,7 +565,6 @@ public class AutomationIntegrationTest extends JavaOSGiTest {
     @Test
     public void testChainOfCompositeModules() throws ItemNotFoundException {
         Configuration triggerConfig = new Configuration(Map.of("itemName", "myMotionItem4"));
-        Map<String, Object> eventInputs = Map.of("event", "ItemStateChangeTrigger4.event");
         Map<String, Object> params = new HashMap<>();
         params.put("itemName", "myLampItem4");
         params.put("command", "ON");
@@ -818,7 +817,7 @@ public class AutomationIntegrationTest extends JavaOSGiTest {
         RuleTemplateProvider templateProvider = new RuleTemplateProvider() {
             @Override
             public @Nullable RuleTemplate getTemplate(String UID, @Nullable Locale locale) {
-                if (UID == templateUID) {
+                if (UID.equals(templateUID)) {
                     return template;
                 } else {
                     return null;
@@ -860,9 +859,9 @@ public class AutomationIntegrationTest extends JavaOSGiTest {
 
             @Override
             public <T extends ModuleType> @Nullable T getModuleType(String UID, @Nullable Locale locale) {
-                if (UID == triggerTypeUID) {
+                if (UID.equals(triggerTypeUID)) {
                     return (T) triggerType;
-                } else if (UID == actionTypeUID) {
+                } else if (UID.equals(actionTypeUID)) {
                     return (T) actionType;
                 } else {
                     return null;

itests/org.openhab.core.tests/src/main/java/org/openhab/core/internal/common/SafeCallerImplTest.java

@@ -23,6 +23,8 @@ import java.lang.management.ManagementFactory;
 import java.lang.management.ThreadInfo;
 import java.lang.management.ThreadMXBean;
 import java.text.MessageFormat;
+import java.time.Duration;
+import java.time.Instant;
 import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
@@ -31,11 +33,12 @@ import java.util.Random;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.function.Function;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -53,6 +56,7 @@ import org.slf4j.LoggerFactory;
  * @author Simon Kaufmann - Initial contribution and API.
  */
 @ExtendWith(MockitoExtension.class)
+@NonNullByDefault
 public class SafeCallerImplTest extends JavaTest {
 
     private static final int THREAD_POOL_SIZE = 3;
@@ -68,13 +72,14 @@ public class SafeCallerImplTest extends JavaTest {
 
     private final Logger logger = LoggerFactory.getLogger(SafeCallerImplTest.class);
 
-    private SafeCallerImpl safeCaller;
-    private QueueingThreadPoolExecutor scheduler;
-    private TestInfo testInfo;
     private final List<AssertingThread> threads = new LinkedList<>();
 
-    private @Mock Runnable mockTimeoutHandler;
+    private @NonNullByDefault({}) SafeCallerImpl safeCaller;
-    private @Mock Consumer<Throwable> mockErrorHandler;
+    private @NonNullByDefault({}) QueueingThreadPoolExecutor scheduler;
+    private @NonNullByDefault({}) TestInfo testInfo;
+
+    private @NonNullByDefault({}) @Mock Runnable timeoutHandlerMock;
+    private @NonNullByDefault({}) @Mock Consumer<Throwable> errorHandlerMock;
 
     public static interface ITarget {
         public String method();
@@ -135,9 +140,9 @@ public class SafeCallerImplTest extends JavaTest {
         Runnable mock = mock(Runnable.class);
         doThrow(RuntimeException.class).when(mock).run();
 
-        safeCaller.create(mock, Runnable.class).onException(mockErrorHandler).build().run();
+        safeCaller.create(mock, Runnable.class).onException(errorHandlerMock).build().run();
         waitForAssert(() -> {
-            verify(mockErrorHandler).accept(isA(Throwable.class));
+            verify(errorHandlerMock).accept(isA(Throwable.class));
         });
     }
 
@@ -146,9 +151,9 @@ public class SafeCallerImplTest extends JavaTest {
         Runnable mock = mock(Runnable.class);
         doAnswer(a -> sleep(BLOCK)).when(mock).run();
 
-        safeCaller.create(mock, Runnable.class).withTimeout(TIMEOUT).onTimeout(mockTimeoutHandler).build().run();
+        safeCaller.create(mock, Runnable.class).withTimeout(TIMEOUT).onTimeout(timeoutHandlerMock).build().run();
         waitForAssert(() -> {
-            verify(mockTimeoutHandler).run();
+            verify(timeoutHandlerMock).run();
         });
     }
 
@@ -294,10 +299,10 @@ public class SafeCallerImplTest extends JavaTest {
         doAnswer(a -> sleep(BLOCK)).when(mock).run();
 
         assertDurationAbove(BLOCK - GRACE, () -> {
-            safeCaller.create(mock, Runnable.class).withTimeout(BLOCK + GRACE * 2).onTimeout(mockTimeoutHandler).build()
+            safeCaller.create(mock, Runnable.class).withTimeout(BLOCK + GRACE * 2).onTimeout(timeoutHandlerMock).build()
                     .run();
         });
-        verifyNoMoreInteractions(mockTimeoutHandler);
+        verifyNoMoreInteractions(timeoutHandlerMock);
     }
 
     @Test
@@ -388,11 +393,11 @@ public class SafeCallerImplTest extends JavaTest {
 
         assertDurationBelow(GRACE, () -> {
             safeCaller.create(mock1, Runnable.class).withTimeout(TIMEOUT).withAsync().withIdentifier(identifier)
-                    .onTimeout(mockTimeoutHandler).onException(mockErrorHandler).build().run();
+                    .onTimeout(timeoutHandlerMock).onException(errorHandlerMock).build().run();
         });
         waitForAssert(() -> verify(mock1, times(1)).run());
-        waitForAssert(() -> verify(mockTimeoutHandler, times(1)).run());
+        waitForAssert(() -> verify(timeoutHandlerMock, times(1)).run());
-        verifyNoMoreInteractions(mockErrorHandler);
+        verifyNoMoreInteractions(errorHandlerMock);
     }
 
     @Test
@@ -403,12 +408,12 @@ public class SafeCallerImplTest extends JavaTest {
 
         assertDurationBelow(GRACE, () -> {
             safeCaller.create(mock1, Runnable.class).withTimeout(TIMEOUT).withAsync().withIdentifier(identifier)
-                    .onTimeout(mockTimeoutHandler).onException(mockErrorHandler).build().run();
+                    .onTimeout(timeoutHandlerMock).onException(errorHandlerMock).build().run();
         });
         waitForAssert(() -> verify(mock1, times(1)).run());
-        waitForAssert(() -> verify(mockErrorHandler, times(1)).accept(isA(Exception.class)));
+        waitForAssert(() -> verify(errorHandlerMock, times(1)).accept(isA(Exception.class)));
-        verifyNoMoreInteractions(mockErrorHandler);
+        verifyNoMoreInteractions(errorHandlerMock);
-        verifyNoMoreInteractions(mockTimeoutHandler);
+        verifyNoMoreInteractions(timeoutHandlerMock);
     }
 
     @Test
@@ -538,24 +543,28 @@ public class SafeCallerImplTest extends JavaTest {
     }
 
     private void assertDurationBetween(long low, long high, Runnable runnable) {
-        long startNanos = System.nanoTime();
+        Instant start = Instant.now();
         try {
             runnable.run();
         } finally {
-            long durationMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNanos);
+            Duration duration = Duration.between(start, Instant.now());
-            try {
+            assertDurationBetween(low, high, duration.toMillis());
-                if (low > -1) {
+        }
-                    assertTrue(durationMillis >= low, MessageFormat
+    }
-                            .format("Duration should have been above {0} but was {1}", low, durationMillis));
+
-                }
+    private void assertDurationBetween(long low, long high, long durationMillis) throws AssertionError {
-                if (high > -1) {
+        try {
-                    assertTrue(durationMillis < high, MessageFormat
+            if (low > -1) {
-                            .format("Duration should have been below {0} but was {1}", high, durationMillis));
+                assertTrue(durationMillis >= low,
-                }
+                        MessageFormat.format("Duration should have been above {0} but was {1}", low, durationMillis));
-            } catch (AssertionError e) {
-                logger.debug("{}", createThreadDump(testInfo.getTestMethod().get().getName()));
-                throw e;
             }
+            if (high > -1) {
+                assertTrue(durationMillis < high,
+                        MessageFormat.format("Duration should have been below {0} but was {1}", high, durationMillis));
+            }
+        } catch (AssertionError e) {
+            logger.debug("{}", createThreadDump(testInfo.getTestMethod().get().getName()));
+            throw e;
         }
     }
 
@@ -580,7 +589,7 @@ public class SafeCallerImplTest extends JavaTest {
         return sb.toString();
     }
 
-    private static Object sleep(int duration) {
+    private static @Nullable Object sleep(int duration) {
         try {
             Thread.sleep(duration);
         } catch (InterruptedException e) {
@@ -632,8 +641,8 @@ public class SafeCallerImplTest extends JavaTest {
     }
 
     private static class AssertingThread extends Thread {
-        private AssertionError assertionError;
+        private @Nullable AssertionError assertionError;
-        private RuntimeException runtimeException;
+        private @Nullable RuntimeException runtimeException;
 
         public AssertingThread(Runnable runnable) {
             super(runnable);

tools/static-code-analysis/checkstyle/suppressions.xml

@@ -1,27 +1,26 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<!DOCTYPE suppressions PUBLIC
+<!DOCTYPE suppressions PUBLIC "-//Puppy Crawl//DTD Suppressions 1.1//EN" "http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">
-    "-//Puppy Crawl//DTD Suppressions 1.1//EN"
-"http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">
 
 <suppressions>
-    <!-- These suppressions define which files to be suppressed for which checks. -->
+	<!-- These suppressions define which files to be suppressed for which checks. -->
-    <suppress files=".+[\\/]internal[\\/].+\.java" checks="JavadocType|JavadocVariable|JavadocMethod|MissingJavadocFilterCheck"/>
+	<suppress files=".+[\\/]internal[\\/].+\.java" checks="JavadocType|JavadocVariable|JavadocMethod|MissingJavadocFilterCheck"/>
-    <suppress files=".+DTO\.java" checks="JavadocType|JavadocVariable|JavadocMethod|MissingJavadocFilterCheck" />
+	<suppress files=".+DTO\.java" checks="JavadocType|JavadocVariable|JavadocMethod|MissingJavadocFilterCheck|NullAnnotationsCheck" />
-    <suppress files=".+Impl\.java" checks="JavadocType|JavadocVariable|JavadocMethod|MissingJavadocFilterCheck"/>
+	<suppress files=".+Impl\.java" checks="JavadocType|JavadocVariable|JavadocMethod|MissingJavadocFilterCheck"/>
-    <suppress files=".+[\\/]pom\.xml" checks="OnlyTabIndentationCheck"/>
+	<suppress files=".+[\\/]pom\.xml" checks="OnlyTabIndentationCheck"/>
-    <suppress files=".+[\\/]OH-INF[\\/].+\.xml" checks="OhInfXmlLabelCheck"/>
+	<suppress files=".+[\\/]OH-INF[\\/].+\.xml" checks="OhInfXmlLabelCheck"/>
 
-    <!-- All generated files will skip the author tag check -->
+	<!-- All generated files will skip the author tag check -->
-    <suppress files=".+[\\/]gen[\\/].+\.java" checks="AuthorTagCheck"/>
+	<suppress files=".+[\\/]gen[\\/].+\.java" checks="AuthorTagCheck"/>
-    <!-- Some checks will be supressed for test bundles -->
+	<!-- Some checks will be supressed for test bundles -->
-    <suppress files=".+.test[\\/].+" checks="RequireBundleCheck|OutsideOfLibExternalLibrariesCheck|ManifestExternalLibrariesCheck|BuildPropertiesExternalLibrariesCheck"/>
+	<suppress files=".+.test[\\/].+" checks="RequireBundleCheck|OutsideOfLibExternalLibrariesCheck|ManifestExternalLibrariesCheck|BuildPropertiesExternalLibrariesCheck"/>
 
-    <!-- openHAB Core specific suppressions-->
+	<!-- openHAB Core specific suppressions-->
 
-    <!-- These bundles are generated trough XText -->
+	<!-- These bundles are generated trough XText -->
-    <suppress files=".+org.openhab.core.model.+" checks="RequireBundleCheck|ExportInternalPackageCheck|ManifestPackageVersionCheck|ImportExportedPackagesCheck|PackageExportsNameCheck|PomXmlCheck"/>
+	<suppress files=".+org.openhab.core.model.+" checks="RequireBundleCheck|ExportInternalPackageCheck|ManifestPackageVersionCheck|ImportExportedPackagesCheck|PackageExportsNameCheck|PomXmlCheck"/>
 
-    <suppress files=".+org.openhab.core.config.discovery.mdns.internal.MDNSDiscoveryService.java|.+org.openhab.core.config.discovery.upnp.internal.UpnpDiscoveryService.java|.+org.openhab.core.io.console.eclipse.internal.ConsoleSupportEclipse.java|.+org.openhab.core.io.console.rfc147.internal.CommandWrapper.java|.+org.openhab.core.library.unit.MetricPrefix.java" checks="MethodNameCheck"/>
+	<suppress files=".+org.openhab.core.config.core.ConfigurableService" checks="ConstantNameCheck"/>
+	<suppress files=".+org.openhab.core.config.discovery.mdns.internal.MDNSDiscoveryService.java|.+org.openhab.core.config.discovery.upnp.internal.UpnpDiscoveryService.java|.+org.openhab.core.io.console.eclipse.internal.ConsoleSupportEclipse.java|.+org.openhab.core.io.console.rfc147.internal.CommandWrapper.java|.+org.openhab.core.library.unit.BinaryPrefix.java|.+org.openhab.core.library.unit.MetricPrefix.java" checks="MethodNameCheck"/>
 
 	<!-- Add suppression as discussed in https://github.com/openhab/static-code-analysis/issues/265 -->
 	<suppress files=".+org.openhab.core.common.registry.AbstractRegistry.java" checks="DeclarativeServicesDependencyInjectionCheck"/>