From c5bb15c631d38ba3632c6d70c447de6065974db0 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Thu, 9 Feb 2023 20:36:25 +0100 Subject: [PATCH] [sinttest] Add UnconnectedConnectionSource for low-level tests Previously low-level tests where run, potentially multiple times, with the default connection descriptor. Reported-by: Guus der Kinderen --- .../AbstractSmackLowLevelIntegrationTest.java | 47 +++-- .../SmackIntegrationTestFramework.java | 183 ++++++++++++------ .../smack/inttest/XmppConnectionManager.java | 7 +- .../smack/LoginIntegrationTest.java | 7 +- .../SmackIntegrationTestFrameWorkTest.java | 38 ++-- 5 files changed, 180 insertions(+), 102 deletions(-) diff --git a/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/AbstractSmackLowLevelIntegrationTest.java b/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/AbstractSmackLowLevelIntegrationTest.java index 25435627f..d517494db 100644 --- a/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/AbstractSmackLowLevelIntegrationTest.java +++ b/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/AbstractSmackLowLevelIntegrationTest.java @@ -1,6 +1,6 @@ /** * - * Copyright 2015-2020 Florian Schmaus + * Copyright 2015-2023 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,20 +17,19 @@ package org.igniterealtime.smack.inttest; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; import org.jivesoftware.smack.AbstractXMPPConnection; import org.jivesoftware.smack.SmackException; -import org.jivesoftware.smack.SmackException.NoResponseException; -import org.jivesoftware.smack.SmackException.NotConnectedException; import org.jivesoftware.smack.XMPPException; -import org.jivesoftware.smack.XMPPException.XMPPErrorException; import org.jxmpp.jid.DomainBareJid; public abstract class AbstractSmackLowLevelIntegrationTest extends AbstractSmackIntTest { + public interface UnconnectedConnectionSource { + AbstractXMPPConnection getUnconnectedConnection(); + } + private final SmackIntegrationTestEnvironment environment; /** @@ -47,25 +46,23 @@ public abstract class AbstractSmackLowLevelIntegrationTest extends AbstractSmack this.service = configuration.service; } - protected AbstractXMPPConnection getConnectedConnection() throws InterruptedException, XMPPException, SmackException, IOException { - AbstractXMPPConnection connection = getUnconnectedConnection(); - connection.connect().login(); - return connection; - } - - protected AbstractXMPPConnection getUnconnectedConnection() - throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { - return environment.connectionManager.constructConnection(); - } - - protected List getUnconnectedConnections(int count) - throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { - List connections = new ArrayList<>(count); - for (int i = 0; i < count; i++) { - AbstractXMPPConnection connection = getUnconnectedConnection(); - connections.add(connection); - } - return connections; + /** + * Get a connected connection. Note that this method will return a connection constructed via the default connection + * descriptor. It is primarily meant for integration tests to discover if the XMPP service supports a certain + * feature, that the integration test requires to run. This feature discovery is typically done in the constructor of the + * integration tests. And if the discovery fails a {@link TestNotPossibleException} should be thrown by he constructor. + * + *

Please ensure that you invoke {@link #recycle(AbstractXMPPConnection connection)} once you are done with this connection. + * + * @return a connected connection. + * @throws InterruptedException if the calling thread was interrupted. + * @throws SmackException if Smack detected an exceptional situation. + * @throws IOException if an I/O error occurred. + * @throws XMPPException if an XMPP protocol error was received. + */ + protected AbstractXMPPConnection getConnectedConnection() + throws InterruptedException, SmackException, IOException, XMPPException { + return environment.connectionManager.constructConnectedConnection(); } protected void recycle(AbstractXMPPConnection connection) { diff --git a/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/SmackIntegrationTestFramework.java b/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/SmackIntegrationTestFramework.java index 433386600..0caf8cd96 100644 --- a/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/SmackIntegrationTestFramework.java +++ b/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/SmackIntegrationTestFramework.java @@ -1,6 +1,6 @@ /** * - * Copyright 2015-2021 Florian Schmaus + * Copyright 2015-2023 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -54,7 +54,9 @@ import org.jivesoftware.smack.Smack; import org.jivesoftware.smack.SmackConfiguration; import org.jivesoftware.smack.SmackException; import org.jivesoftware.smack.SmackException.NoResponseException; +import org.jivesoftware.smack.SmackException.NotConnectedException; import org.jivesoftware.smack.XMPPException; +import org.jivesoftware.smack.XMPPException.XMPPErrorException; import org.jivesoftware.smack.tcp.XMPPTCPConnectionConfiguration; import org.jivesoftware.smack.util.StringUtils; import org.jivesoftware.smack.util.TLSUtils; @@ -510,10 +512,11 @@ public class SmackIntegrationTestFramework { private static void verifyLowLevelTestMethod(Method method, Class connectionClass) { - if (!testMethodParametersIsListOfConnections(method, connectionClass) - && !testMethodParametersVarargsConnections(method, connectionClass)) { - throw new IllegalArgumentException(method + " is not a valid low level test method"); + if (determineTestMethodParameterType(method, connectionClass) != null) { + return; } + + throw new IllegalArgumentException(method + " is not a valid low level test method"); } private List invokeLowLevel(LowLevelTestMethod lowLevelTestMethod, AbstractSmackLowLevelIntegrationTest test) { @@ -824,47 +827,69 @@ public class SmackIntegrationTestFramework { } private final class LowLevelTestMethod { + private final Method testMethod; private final SmackIntegrationTest smackIntegrationTestAnnotation; - private final boolean parameterListOfConnections; + private final TestMethodParameterType parameterType; private LowLevelTestMethod(Method testMethod) { this.testMethod = testMethod; smackIntegrationTestAnnotation = testMethod.getAnnotation(SmackIntegrationTest.class); assert smackIntegrationTestAnnotation != null; - parameterListOfConnections = testMethodParametersIsListOfConnections(testMethod); + parameterType = determineTestMethodParameterType(testMethod); } private void invoke(AbstractSmackLowLevelIntegrationTest test, XmppConnectionDescriptor connectionDescriptor) throws IllegalAccessException, IllegalArgumentException, InvocationTargetException, InterruptedException, SmackException, IOException, XMPPException { - final int connectionCount; - if (parameterListOfConnections) { - connectionCount = smackIntegrationTestAnnotation.connectionCount(); - if (connectionCount < 1) { - throw new IllegalArgumentException(testMethod + " is annotated to use less than one connection ('" - + connectionCount + ')'); + switch (parameterType) { + case singleConnectedConnection: + case collectionOfConnections: + case parameterListOfConnections: + final boolean collectionOfConnections = parameterType == TestMethodParameterType.collectionOfConnections; + + final int connectionCount; + if (collectionOfConnections) { + connectionCount = smackIntegrationTestAnnotation.connectionCount(); + if (connectionCount < 1) { + throw new IllegalArgumentException(testMethod + " is annotated to use less than one connection ('" + + connectionCount + ')'); + } + } else { + connectionCount = testMethod.getParameterCount(); } - } else { - connectionCount = testMethod.getParameterCount(); - } - List connections = connectionManager.constructConnectedConnections( - connectionDescriptor, connectionCount); + List connections = connectionManager.constructConnectedConnections( + connectionDescriptor, connectionCount); - if (parameterListOfConnections) { - testMethod.invoke(test, connections); - } else { - Object[] connectionsArray = new Object[connectionCount]; - for (int i = 0; i < connectionsArray.length; i++) { - connectionsArray[i] = connections.remove(0); + if (collectionOfConnections) { + testMethod.invoke(test, connections); + } else { + Object[] connectionsArray = new Object[connectionCount]; + for (int i = 0; i < connectionsArray.length; i++) { + connectionsArray[i] = connections.remove(0); + } + testMethod.invoke(test, connectionsArray); } - testMethod.invoke(test, connectionsArray); - } - connectionManager.recycle(connections); + connectionManager.recycle(connections); + break; + case unconnectedConnectionSource: + AbstractSmackLowLevelIntegrationTest.UnconnectedConnectionSource source = () -> { + try { + return environment.connectionManager.constructConnection(connectionDescriptor); + } catch (NoResponseException | XMPPErrorException | NotConnectedException + | InterruptedException e) { + // TODO: Ideally we would wrap the exceptions in an unchecked exceptions, catch those unchecked + // exceptions below and throw the wrapped checked exception. + throw new RuntimeException(e); + } + }; + testMethod.invoke(test, source); + break; + } } @Override @@ -873,46 +898,80 @@ public class SmackIntegrationTestFramework { } } - private static boolean testMethodParametersIsListOfConnections(Method testMethod) { - return testMethodParametersIsListOfConnections(testMethod, AbstractXMPPConnection.class); + enum TestMethodParameterType { + /** + * testMethod(Connection connection) + */ + singleConnectedConnection, + + /** + * testMethod(Collection<Connection>) + *

It can also be a subclass of collection like List. In fact, the type of the parameter being List is expected to be the common case. + */ + collectionOfConnections, + + /** + * testMethod(Connection a, Connection b, Connection c) + */ + parameterListOfConnections, + + /** + * testMethod(UnconnectedConnectionSource unconnectedConnectionSource) + */ + unconnectedConnectionSource, + }; + + static TestMethodParameterType determineTestMethodParameterType(Method testMethod) { + return determineTestMethodParameterType(testMethod, AbstractXMPPConnection.class); } - static boolean testMethodParametersIsListOfConnections(Method testMethod, Class connectionClass) { - Type[] parameterTypes = testMethod.getGenericParameterTypes(); - if (parameterTypes.length != 1) { - return false; - } - Class soleParameter = testMethod.getParameterTypes()[0]; - if (!Collection.class.isAssignableFrom(soleParameter)) { - return false; - } - - ParameterizedType soleParameterizedType = (ParameterizedType) parameterTypes[0]; - Type[] actualTypeArguments = soleParameterizedType.getActualTypeArguments(); - if (actualTypeArguments.length != 1) { - return false; - } - - Type soleActualTypeArgument = actualTypeArguments[0]; - if (!(soleActualTypeArgument instanceof Class)) { - return false; - } - Class soleActualTypeArgumentAsClass = (Class) soleActualTypeArgument; - if (!connectionClass.isAssignableFrom(soleActualTypeArgumentAsClass)) { - return false; - } - - return true; - } - - static boolean testMethodParametersVarargsConnections(Method testMethod, Class connectionClass) { + static TestMethodParameterType determineTestMethodParameterType(Method testMethod, Class connectionClass) { Class[] parameterTypes = testMethod.getParameterTypes(); - for (Class parameterType : parameterTypes) { - if (!parameterType.isAssignableFrom(connectionClass)) { - return false; - } + if (parameterTypes.length == 0) { + return null; } - return true; + if (parameterTypes.length > 1) { + // If there are more parameters, then all must be assignable from the connection class. + for (Class parameterType : parameterTypes) { + if (!connectionClass.isAssignableFrom(parameterType)) { + return null; + } + } + + return TestMethodParameterType.parameterListOfConnections; + } + + // This method has exactly a single parameter. + Class soleParameter = parameterTypes[0]; + + if (Collection.class.isAssignableFrom(soleParameter)) { + // The sole parameter is assignable from collection, which means that it is a parameterized generic type. + ParameterizedType soleParameterizedType = (ParameterizedType) testMethod.getGenericParameterTypes()[0]; + Type[] actualTypeArguments = soleParameterizedType.getActualTypeArguments(); + if (actualTypeArguments.length != 1) { + // The parameter list of the Collection has more than one type. + return null; + } + + Type soleActualTypeArgument = actualTypeArguments[0]; + if (!(soleActualTypeArgument instanceof Class)) { + return null; + } + + Class soleActualTypeArgumentAsClass = (Class) soleActualTypeArgument; + if (!connectionClass.isAssignableFrom(soleActualTypeArgumentAsClass)) { + return null; + } + + return TestMethodParameterType.collectionOfConnections; + } else if (connectionClass.isAssignableFrom(soleParameter)) { + return TestMethodParameterType.singleConnectedConnection; + } else if (AbstractSmackLowLevelIntegrationTest.UnconnectedConnectionSource.class.isAssignableFrom(soleParameter)) { + return TestMethodParameterType.unconnectedConnectionSource; + } + + return null; } + } diff --git a/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/XmppConnectionManager.java b/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/XmppConnectionManager.java index caac9b0cd..e74bced82 100644 --- a/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/XmppConnectionManager.java +++ b/smack-integration-test/src/main/java/org/igniterealtime/smack/inttest/XmppConnectionManager.java @@ -1,6 +1,6 @@ /** * - * Copyright 2018-2021 Florian Schmaus + * Copyright 2018-2023 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -421,6 +421,11 @@ public class XmppConnectionManager { return constructConnection(defaultConnectionDescriptor); } + AbstractXMPPConnection constructConnectedConnection() + throws InterruptedException, SmackException, IOException, XMPPException { + return constructConnectedConnection(defaultConnectionDescriptor); + } + C constructConnection( XmppConnectionDescriptor> connectionDescriptor) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { diff --git a/smack-integration-test/src/main/java/org/jivesoftware/smack/LoginIntegrationTest.java b/smack-integration-test/src/main/java/org/jivesoftware/smack/LoginIntegrationTest.java index 7534c4449..2c9dbb9ec 100644 --- a/smack-integration-test/src/main/java/org/jivesoftware/smack/LoginIntegrationTest.java +++ b/smack-integration-test/src/main/java/org/jivesoftware/smack/LoginIntegrationTest.java @@ -1,6 +1,6 @@ /** * - * Copyright 2015-2020 Florian Schmaus + * Copyright 2015-2023 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -42,6 +42,7 @@ public class LoginIntegrationTest extends AbstractSmackLowLevelIntegrationTest { * Check that the server is returning the correct error when trying to login using an invalid * (i.e. non-existent) user. * + * @param unconnectedConnectionSource the unconnected connections source that is used. * @throws InterruptedException if the calling thread was interrupted. * @throws XMPPException if an XMPP protocol error was received. * @throws IOException if an I/O error occurred. @@ -50,12 +51,12 @@ public class LoginIntegrationTest extends AbstractSmackLowLevelIntegrationTest { * @throws KeyManagementException if there was a key mangement error. */ @SmackIntegrationTest - public void testInvalidLogin() throws SmackException, IOException, XMPPException, + public void testInvalidLogin(UnconnectedConnectionSource unconnectedConnectionSource) throws SmackException, IOException, XMPPException, InterruptedException, KeyManagementException, NoSuchAlgorithmException { final String nonExistentUserString = StringUtils.insecureRandomString(24); final String invalidPassword = "invalidPassword"; - AbstractXMPPConnection connection = getUnconnectedConnection(); + AbstractXMPPConnection connection = unconnectedConnectionSource.getUnconnectedConnection(); connection.connect(); try { diff --git a/smack-integration-test/src/test/java/org/igniterealtime/smack/inttest/SmackIntegrationTestFrameWorkTest.java b/smack-integration-test/src/test/java/org/igniterealtime/smack/inttest/SmackIntegrationTestFrameWorkTest.java index 94e7ad3c8..48b827770 100644 --- a/smack-integration-test/src/test/java/org/igniterealtime/smack/inttest/SmackIntegrationTestFrameWorkTest.java +++ b/smack-integration-test/src/test/java/org/igniterealtime/smack/inttest/SmackIntegrationTestFrameWorkTest.java @@ -1,6 +1,6 @@ /** * - * Copyright 2019-2020 Florian Schmaus + * Copyright 2019-2023 Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,14 +16,16 @@ */ package org.igniterealtime.smack.inttest; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import java.lang.reflect.Method; import java.util.List; import org.jivesoftware.smack.AbstractXMPPConnection; +import org.igniterealtime.smack.inttest.SmackIntegrationTestFramework.TestMethodParameterType; + import org.junit.jupiter.api.Test; public class SmackIntegrationTestFrameWorkTest { @@ -69,28 +71,42 @@ public class SmackIntegrationTestFrameWorkTest { @Test public void testValidLowLevelList() { Method testMethod = getTestMethod(ValidLowLevelList.class); - assertTrue(SmackIntegrationTestFramework.testMethodParametersIsListOfConnections(testMethod, - AbstractXMPPConnection.class)); + TestMethodParameterType determinedParameterType = SmackIntegrationTestFramework.determineTestMethodParameterType(testMethod, AbstractXMPPConnection.class); + assertEquals(TestMethodParameterType.collectionOfConnections, determinedParameterType); } @Test public void testInvalidLowLevelList() { Method testMethod = getTestMethod(InvalidLowLevelList.class); - assertFalse(SmackIntegrationTestFramework.testMethodParametersIsListOfConnections(testMethod, - AbstractXMPPConnection.class)); + TestMethodParameterType determinedParameterType = SmackIntegrationTestFramework.determineTestMethodParameterType(testMethod, AbstractXMPPConnection.class); + assertNull(determinedParameterType); } @Test public void testValidLowLevelVarargs() { Method testMethod = getTestMethod(ValidLowLevelVarargs.class); - assertTrue(SmackIntegrationTestFramework.testMethodParametersVarargsConnections(testMethod, - AbstractXMPPConnection.class)); + TestMethodParameterType determinedParameterType = SmackIntegrationTestFramework.determineTestMethodParameterType(testMethod, AbstractXMPPConnection.class); + assertEquals(TestMethodParameterType.parameterListOfConnections, determinedParameterType); } @Test public void testInvalidLowLevelVargs() { Method testMethod = getTestMethod(InvalidLowLevelVarargs.class); - assertFalse(SmackIntegrationTestFramework.testMethodParametersVarargsConnections(testMethod, - AbstractXMPPConnection.class)); + TestMethodParameterType determinedParameterType = SmackIntegrationTestFramework.determineTestMethodParameterType(testMethod, AbstractXMPPConnection.class); + assertNull(determinedParameterType); } + + private static class ValidUnconnectedConnectionSource { + @SuppressWarnings("unused") + public void test(AbstractSmackLowLevelIntegrationTest.UnconnectedConnectionSource source) { + } + } + + @Test + public void testValidUnconnectedConnectionSource() { + Method testMethod = getTestMethod(ValidUnconnectedConnectionSource.class); + TestMethodParameterType determinedParameterType = SmackIntegrationTestFramework.determineTestMethodParameterType(testMethod, AbstractXMPPConnection.class); + assertEquals(TestMethodParameterType.unconnectedConnectionSource, determinedParameterType); + } + }