[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 <guus@goodbytes.nl>

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();
+     * Get a connected connection. Note that this method will return a connection constructed via the default connection
-        connection.connect().login();
+     * descriptor. It is primarily meant for integration tests to discover if the XMPP service supports a certain
-        return connection;
+     * 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.
-
+     *
-    protected AbstractXMPPConnection getUnconnectedConnection()
+     * <p> Please ensure that you invoke {@link #recycle(AbstractXMPPConnection connection)} once you are done with this connection.
-                    throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException {
+     *
-        return environment.connectionManager.constructConnection();
+     * @return a connected connection.
-    }
+     * @throws InterruptedException if the calling thread was interrupted.
-
+     * @throws SmackException if Smack detected an exceptional situation.
-    protected List<AbstractXMPPConnection> getUnconnectedConnections(int count)
+     * @throws IOException if an I/O error occurred.
-                    throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException {
+     * @throws XMPPException if an XMPP protocol error was received.
-        List<AbstractXMPPConnection> connections = new ArrayList<>(count);
+     */
-        for (int i = 0; i < count; i++) {
+    protected AbstractXMPPConnection getConnectedConnection()
-            AbstractXMPPConnection connection = getUnconnectedConnection();
+                    throws InterruptedException, SmackException, IOException, XMPPException {
-            connections.add(connection);
+        return environment.connectionManager.constructConnectedConnection();
-        }
-        return connections;
     }
 
     protected void recycle(AbstractXMPPConnection connection) {

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<? extends AbstractXMPPConnection> connectionClass) {
-        if (!testMethodParametersIsListOfConnections(method, connectionClass)
+        if (determineTestMethodParameterType(method, connectionClass) != null) {
-                        && !testMethodParametersVarargsConnections(method, connectionClass)) {
+            return;
-            throw new IllegalArgumentException(method + " is not a valid low level test method");
         }
+
+        throw new IllegalArgumentException(method + " is not a valid low level test method");
     }
 
     private List<ConcreteTest> 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;
+            switch (parameterType) {
-            if (parameterListOfConnections) {
+            case singleConnectedConnection:
-                connectionCount = smackIntegrationTestAnnotation.connectionCount();
+            case collectionOfConnections:
-                if (connectionCount < 1) {
+            case parameterListOfConnections:
-                    throw new IllegalArgumentException(testMethod + " is annotated to use less than one connection ('"
+                final boolean collectionOfConnections = parameterType == TestMethodParameterType.collectionOfConnections;
-                                    + connectionCount + ')');
+
+                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<? extends AbstractXMPPConnection> connections = connectionManager.constructConnectedConnections(
+                List<? extends AbstractXMPPConnection> connections = connectionManager.constructConnectedConnections(
-                            connectionDescriptor, connectionCount);
+                                connectionDescriptor, connectionCount);
 
-            if (parameterListOfConnections) {
+                if (collectionOfConnections) {
-                testMethod.invoke(test, connections);
+                    testMethod.invoke(test, connections);
-            } else {
+                } else {
-                Object[] connectionsArray = new Object[connectionCount];
+                    Object[] connectionsArray = new Object[connectionCount];
-                for (int i = 0; i < connectionsArray.length; i++) {
+                    for (int i = 0; i < connectionsArray.length; i++) {
-                    connectionsArray[i] = connections.remove(0);
+                        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) {
+    enum TestMethodParameterType {
-        return testMethodParametersIsListOfConnections(testMethod, AbstractXMPPConnection.class);
+        /**
+         * testMethod(Connection connection)
+         */
+        singleConnectedConnection,
+
+        /**
+         * testMethod(Collection&lt;Connection&gt;)
+         * <p> 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<? extends AbstractXMPPConnection> connectionClass) {
+    static TestMethodParameterType determineTestMethodParameterType(Method testMethod, Class<? extends AbstractXMPPConnection> 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<? extends AbstractXMPPConnection> connectionClass) {
         Class<?>[] parameterTypes = testMethod.getParameterTypes();
-        for (Class<?> parameterType : parameterTypes) {
+        if (parameterTypes.length == 0) {
-            if (!parameterType.isAssignableFrom(connectionClass)) {
+            return null;
-                return false;
-            }
         }
 
-        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;
     }
+
 }

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 extends AbstractXMPPConnection> C constructConnection(
                     XmppConnectionDescriptor<C, ? extends ConnectionConfiguration, ? extends ConnectionConfiguration.Builder<?, ?>> connectionDescriptor)
                     throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException {

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 {

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.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+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,
+        TestMethodParameterType determinedParameterType = SmackIntegrationTestFramework.determineTestMethodParameterType(testMethod, AbstractXMPPConnection.class);
-                        AbstractXMPPConnection.class));
+        assertEquals(TestMethodParameterType.collectionOfConnections, determinedParameterType);
     }
 
     @Test
     public void testInvalidLowLevelList() {
         Method testMethod = getTestMethod(InvalidLowLevelList.class);
-        assertFalse(SmackIntegrationTestFramework.testMethodParametersIsListOfConnections(testMethod,
+        TestMethodParameterType determinedParameterType = SmackIntegrationTestFramework.determineTestMethodParameterType(testMethod, AbstractXMPPConnection.class);
-                        AbstractXMPPConnection.class));
+        assertNull(determinedParameterType);
     }
 
     @Test
     public void testValidLowLevelVarargs() {
         Method testMethod = getTestMethod(ValidLowLevelVarargs.class);
-        assertTrue(SmackIntegrationTestFramework.testMethodParametersVarargsConnections(testMethod,
+        TestMethodParameterType determinedParameterType = SmackIntegrationTestFramework.determineTestMethodParameterType(testMethod, AbstractXMPPConnection.class);
-                        AbstractXMPPConnection.class));
+        assertEquals(TestMethodParameterType.parameterListOfConnections, determinedParameterType);
     }
 
     @Test
     public void testInvalidLowLevelVargs() {
         Method testMethod = getTestMethod(InvalidLowLevelVarargs.class);
-        assertFalse(SmackIntegrationTestFramework.testMethodParametersVarargsConnections(testMethod,
+        TestMethodParameterType determinedParameterType = SmackIntegrationTestFramework.determineTestMethodParameterType(testMethod, AbstractXMPPConnection.class);
-                        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);
+    }
+
 }