diff --git a/bundles/org.openhab.core.io.transport.modbus/src/test/java/org/openhab/core/io/transport/modbus/test/IntegrationTestSupport.java b/bundles/org.openhab.core.io.transport.modbus/src/test/java/org/openhab/core/io/transport/modbus/test/IntegrationTestSupport.java index cee8c920e..e671c47e8 100644 --- a/bundles/org.openhab.core.io.transport.modbus/src/test/java/org/openhab/core/io/transport/modbus/test/IntegrationTestSupport.java +++ b/bundles/org.openhab.core.io.transport.modbus/src/test/java/org/openhab/core/io/transport/modbus/test/IntegrationTestSupport.java @@ -26,6 +26,8 @@ import java.net.Socket; import java.net.SocketException; import java.net.UnknownHostException; import java.util.HashMap; +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.function.LongSupplier; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -309,10 +311,21 @@ public class IntegrationTestSupport extends JavaTest { public class TCPSlaveConnectionFactoryImpl implements TCPSlaveConnectionFactory { + private final Queue acceptedSockets = new ConcurrentLinkedQueue<>(); + @Override public TCPSlaveConnection create(@NonNullByDefault({}) Socket socket) { + acceptedSockets.add(socket); return new TCPSlaveConnection(socket, new SpyingModbusTCPTransportFactory()); } + + public long countOpenSockets() { + return acceptedSockets.stream().filter(s -> !s.isClosed()).count(); + } + + public void clearAcceptedSockets() { + acceptedSockets.clear(); + } } public class UDPSlaveTerminalFactoryImpl implements UDPSlaveTerminalFactory { diff --git a/bundles/org.openhab.core.io.transport.modbus/src/test/java/org/openhab/core/io/transport/modbus/test/SmokeTest.java b/bundles/org.openhab.core.io.transport.modbus/src/test/java/org/openhab/core/io/transport/modbus/test/SmokeTest.java index 16348d2ea..cdc902e7f 100644 --- a/bundles/org.openhab.core.io.transport.modbus/src/test/java/org/openhab/core/io/transport/modbus/test/SmokeTest.java +++ b/bundles/org.openhab.core.io.transport.modbus/src/test/java/org/openhab/core/io/transport/modbus/test/SmokeTest.java @@ -17,21 +17,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assumptions.*; -import java.io.IOException; -import java.lang.reflect.Constructor; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.net.InetAddress; -import java.net.Socket; -import java.net.SocketImpl; -import java.net.SocketImplFactory; -import java.net.SocketOption; -import java.net.StandardSocketOptions; import java.util.BitSet; -import java.util.Objects; import java.util.Optional; -import java.util.Queue; -import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -74,14 +61,6 @@ public class SmokeTest extends IntegrationTestSupport { private static final int DISCRETE_EVERY_N_TRUE = 3; private static final int HOLDING_REGISTER_MULTIPLIER = 1; private static final int INPUT_REGISTER_MULTIPLIER = 10; - private static final SpyingSocketFactory SOCKET_SPY = new SpyingSocketFactory(); - static { - try { - Socket.setSocketImplFactory(SOCKET_SPY); - } catch (IOException e) { - fail("Could not install socket spy in SmokeTest"); - } - } /** * Whether tests are run in Continuous Integration environment, i.e. Jenkins or Travis CI @@ -138,8 +117,8 @@ public class SmokeTest extends IntegrationTestSupport { } @BeforeEach - public void setUpSocketSpy() throws IOException { - SOCKET_SPY.sockets.clear(); + public void setUpSocketSpy() { + ((TCPSlaveConnectionFactoryImpl) tcpConnectionFactory).clearAcceptedSockets(); } /** @@ -844,7 +823,7 @@ public class SmokeTest extends IntegrationTestSupport { config.setReconnectAfterMillis(9_000_000); // 1. capture open connections at this point - long openSocketsBefore = getNumberOfOpenClients(SOCKET_SPY); + long openSocketsBefore = getNumberOfOpenClients(); assertThat(openSocketsBefore, is(equalTo(0L))); // 2. make poll, binding opens the tcp connection @@ -861,7 +840,7 @@ public class SmokeTest extends IntegrationTestSupport { } waitForAssert(() -> { // 3. ensure one open connection - long openSocketsAfter = getNumberOfOpenClients(SOCKET_SPY); + long openSocketsAfter = getNumberOfOpenClients(); assertThat(openSocketsAfter, is(equalTo(1L))); }); try (ModbusCommunicationInterface ignored = modbusManager.newModbusCommunicationInterface(endpoint, @@ -876,20 +855,20 @@ public class SmokeTest extends IntegrationTestSupport { }); assertTrue(latch.await(60, TimeUnit.SECONDS)); } - assertThat(getNumberOfOpenClients(SOCKET_SPY), is(equalTo(1L))); + assertThat(getNumberOfOpenClients(), is(equalTo(1L))); // wait for moment (to check that no connections are closed) Thread.sleep(1000); // no more than 1 connection, even though requests are going through - assertThat(getNumberOfOpenClients(SOCKET_SPY), is(equalTo(1L))); + assertThat(getNumberOfOpenClients(), is(equalTo(1L))); } Thread.sleep(1000); // Still one connection open even after closing second connection - assertThat(getNumberOfOpenClients(SOCKET_SPY), is(equalTo(1L))); + assertThat(getNumberOfOpenClients(), is(equalTo(1L))); } // 4. close (the last) comms // ensure that open connections are closed // (despite huge "reconnect after millis") waitForAssert(() -> { - long openSocketsAfterClose = getNumberOfOpenClients(SOCKET_SPY); + long openSocketsAfterClose = getNumberOfOpenClients(); assertThat(openSocketsAfterClose, is(equalTo(0L))); }); } @@ -908,7 +887,7 @@ public class SmokeTest extends IntegrationTestSupport { config.setReconnectAfterMillis(2_000); // 1. capture open connections at this point - long openSocketsBefore = getNumberOfOpenClients(SOCKET_SPY); + long openSocketsBefore = getNumberOfOpenClients(); assertThat(openSocketsBefore, is(equalTo(0L))); // 2. make poll, binding opens the tcp connection @@ -926,119 +905,20 @@ public class SmokeTest extends IntegrationTestSupport { // Right after the poll we should have one connection open waitForAssert(() -> { // 3. ensure one open connection - long openSocketsAfter = getNumberOfOpenClients(SOCKET_SPY); + long openSocketsAfter = getNumberOfOpenClients(); assertThat(openSocketsAfter, is(equalTo(1L))); }); // 4. Connection should close itself by the commons pool eviction policy (checking for old idle connection // every now and then) waitForAssert(() -> { // 3. ensure one open connection - long openSocketsAfter = getNumberOfOpenClients(SOCKET_SPY); + long openSocketsAfter = getNumberOfOpenClients(); assertThat(openSocketsAfter, is(equalTo(0L))); }, 60_000, 50); } } - private long getNumberOfOpenClients(SpyingSocketFactory socketSpy) { - localAddress(); - return socketSpy.sockets.stream().filter(this::isConnectedToTestServer).count(); - } - - /** - * Spy all sockets that are created - * - * @author Sami Salonen - * - */ - private static class SpyingSocketFactory implements SocketImplFactory { - - Queue sockets = new ConcurrentLinkedQueue<>(); - - @Override - public SocketImpl createSocketImpl() { - SocketImpl socket = newSocksSocketImpl(); - sockets.add(socket); - return socket; - } - } - - private static SocketImpl newSocksSocketImpl() { - try { - Class socksSocketImplClass = Class.forName("java.net.SocksSocketImpl"); - Class socketImplClass = SocketImpl.class; - - // // For Debugging - // for (Method method : socketImplClass.getDeclaredMethods()) { - // LoggerFactory.getLogger("foobar") - // .error("SocketImpl." + method.getName() + Arrays.toString(method.getParameters())); - // } - // for (Constructor constructor : socketImplClass.getDeclaredConstructors()) { - // LoggerFactory.getLogger("foobar") - // .error("SocketImpl." + constructor.getName() + Arrays.toString(constructor.getParameters())); - // } - // for (Method method : socksSocketImplClass.getDeclaredMethods()) { - // LoggerFactory.getLogger("foobar") - // .error("SocksSocketImpl." + method.getName() + Arrays.toString(method.getParameters())); - // } - // for (Constructor constructor : socksSocketImplClass.getDeclaredConstructors()) { - // LoggerFactory.getLogger("foobar").error( - // "SocksSocketImpl." + constructor.getName() + Arrays.toString(constructor.getParameters())); - // } - - try { - Constructor constructor = socksSocketImplClass.getDeclaredConstructor(); - constructor.setAccessible(true); - return (SocketImpl) Objects.requireNonNull(constructor.newInstance()); - } catch (NoSuchMethodException e) { - // Newer Javas (Java 14->) do not have default constructor 'SocksSocketImpl()' - // Instead we use "static SocketImpl.createPlatformSocketImpl" and "SocksSocketImpl(SocketImpl) - Method socketImplCreateMethod = socketImplClass.getDeclaredMethod("createPlatformSocketImpl", - boolean.class); - socketImplCreateMethod.setAccessible(true); - Object socketImpl = socketImplCreateMethod - .invoke(/* null since we deal with static method */ giveNull(), /* server */false); - - Constructor socksSocketImplConstructor = socksSocketImplClass - .getDeclaredConstructor(socketImplClass); - socksSocketImplConstructor.setAccessible(true); - return (SocketImpl) Objects.requireNonNull(socksSocketImplConstructor.newInstance(socketImpl)); - } - } catch (Exception e) { - throw new IllegalStateException(e); - } - } - - private boolean isConnectedToTestServer(SocketImpl impl) { - final InetAddress testServerAddress = localAddress(); - - final int port; - boolean connected = true; - final InetAddress address; - try { - Method getPort = SocketImpl.class.getDeclaredMethod("getPort"); - getPort.setAccessible(true); - port = (int) getPort.invoke(impl); - - Method getInetAddressMethod = SocketImpl.class.getDeclaredMethod("getInetAddress"); - getInetAddressMethod.setAccessible(true); - address = (InetAddress) getInetAddressMethod.invoke(impl); - - // hacky (but java8-14 compatible) way to know if socket is open - // SocketImpl.getOption throws IOException when socket is closed - Method getOption = SocketImpl.class.getDeclaredMethod("getOption", SocketOption.class); - getOption.setAccessible(true); - try { - getOption.invoke(impl, StandardSocketOptions.SO_KEEPALIVE); - } catch (InvocationTargetException e) { - if (e.getTargetException() instanceof IOException) { - connected = false; - } else { - throw e; - } - } - } catch (Exception e) { - throw new IllegalStateException(e); - } - return port == tcpModbusPort && connected && address.equals(testServerAddress); + private long getNumberOfOpenClients() { + return ((TCPSlaveConnectionFactoryImpl) tcpConnectionFactory).countOpenSockets(); } } diff --git a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/YamlModelRepositoryImpl.java b/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/YamlModelRepositoryImpl.java index 9150eea5e..99c5b30dd 100644 --- a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/YamlModelRepositoryImpl.java +++ b/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/YamlModelRepositoryImpl.java @@ -143,7 +143,7 @@ public class YamlModelRepositoryImpl implements WatchService.WatchEventListener, objectMapper.findAndRegisterModules(); objectMapper.setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE); objectMapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY); - objectMapper.setSerializationInclusion(Include.NON_NULL); + objectMapper.setDefaultPropertyInclusion(Include.NON_NULL); objectMapper.enable(JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN); this.watchService = watchService; diff --git a/bundles/org.openhab.core/src/test/java/org/openhab/core/cache/lru/LRUMediaCacheEntryTest.java b/bundles/org.openhab.core/src/test/java/org/openhab/core/cache/lru/LRUMediaCacheEntryTest.java index c5340320f..5fe055a18 100644 --- a/bundles/org.openhab.core/src/test/java/org/openhab/core/cache/lru/LRUMediaCacheEntryTest.java +++ b/bundles/org.openhab.core/src/test/java/org/openhab/core/cache/lru/LRUMediaCacheEntryTest.java @@ -201,7 +201,7 @@ public class LRUMediaCacheEntryTest { } }).toList(); - IOException possibleException = exceptionCatched.getValue(); + IOException possibleException = exceptionCatched.get(); if (possibleException != null) { throw possibleException; } diff --git a/tools/upgradetool/src/main/java/org/openhab/core/tools/UpgradeTool.java b/tools/upgradetool/src/main/java/org/openhab/core/tools/UpgradeTool.java index 199dd9a12..380147e28 100644 --- a/tools/upgradetool/src/main/java/org/openhab/core/tools/UpgradeTool.java +++ b/tools/upgradetool/src/main/java/org/openhab/core/tools/UpgradeTool.java @@ -12,6 +12,7 @@ */ package org.openhab.core.tools; +import java.io.IOException; import java.io.PrintStream; import java.nio.file.Path; import java.time.ZonedDateTime; @@ -20,10 +21,10 @@ import java.util.Set; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.DefaultParser; -import org.apache.commons.cli.HelpFormatter; import org.apache.commons.cli.Option; import org.apache.commons.cli.Options; import org.apache.commons.cli.ParseException; +import org.apache.commons.cli.help.HelpFormatter; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.storage.json.internal.JsonStorage; @@ -69,15 +70,15 @@ public class UpgradeTool { options.addOption(Option.builder().longOpt(OPT_USERDATA_DIR).desc( "USERDATA directory to process. Enclose it in double quotes to ensure that any backslashes are not ignored by your command shell.") - .numberOfArgs(1).build()); + .numberOfArgs(1).get()); options.addOption(Option.builder().longOpt(OPT_CONF_DIR).desc( "CONF directory to process. Enclose it in double quotes to ensure that any backslashes are not ignored by your command shell.") - .numberOfArgs(1).build()); + .numberOfArgs(1).get()); options.addOption(Option.builder().longOpt(OPT_COMMAND).numberOfArgs(1) - .desc("command to execute (executes all if omitted)").build()); - options.addOption(Option.builder().longOpt(OPT_LIST_COMMANDS).desc("list available commands").build()); - options.addOption(Option.builder().longOpt(OPT_LOG).numberOfArgs(1).desc("log verbosity").build()); - options.addOption(Option.builder().longOpt(OPT_FORCE).desc("force execution (even if already done)").build()); + .desc("command to execute (executes all if omitted)").get()); + options.addOption(Option.builder().longOpt(OPT_LIST_COMMANDS).desc("list available commands").get()); + options.addOption(Option.builder().longOpt(OPT_LOG).numberOfArgs(1).desc("log verbosity").get()); + options.addOption(Option.builder().longOpt(OPT_FORCE).desc("force execution (even if already done)").get()); return options; } @@ -143,8 +144,12 @@ public class UpgradeTool { } }); } catch (ParseException e) { - HelpFormatter formatter = new HelpFormatter(); - formatter.printHelp("upgradetool", "", options, "", true); + HelpFormatter formatter = HelpFormatter.builder().get(); + try { + formatter.printHelp("upgradetool", "", options, "", true); + } catch (IOException ioException) { + LOGGER.error("Error printing help: {}", ioException.getMessage()); + } } System.exit(0); diff --git a/tools/upgradetool/src/main/java/org/openhab/core/tools/internal/YamlConfigurationV1TagsUpgrader.java b/tools/upgradetool/src/main/java/org/openhab/core/tools/internal/YamlConfigurationV1TagsUpgrader.java index 6c09f7fb6..bb3afe204 100644 --- a/tools/upgradetool/src/main/java/org/openhab/core/tools/internal/YamlConfigurationV1TagsUpgrader.java +++ b/tools/upgradetool/src/main/java/org/openhab/core/tools/internal/YamlConfigurationV1TagsUpgrader.java @@ -82,7 +82,7 @@ public class YamlConfigurationV1TagsUpgrader implements Upgrader { objectMapper.findAndRegisterModules(); objectMapper.setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE); objectMapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY); - objectMapper.setSerializationInclusion(Include.NON_NULL); + objectMapper.setDefaultPropertyInclusion(Include.NON_NULL); objectMapper.enable(JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN); }