[core] Introduce Builder.failOnUnknownStates() and unit tests

The previous approach of emitting a severe log message when a
state (descriptor) was unknown was misleading. There are valid cases
where some states are not known, if, for example, a module was
explicitly disabled.

Using Builder.failOnUnknownStates() in unit tests is far cleaner, as
the existence of unknown states is tested in a controlled environment:
one where are states are supposed to be known.

smack-core/src/main/java/org/jivesoftware/smack/c2s/ModularXmppClientToServerConnectionConfiguration.java

@@ -54,7 +54,7 @@ public final class ModularXmppClientToServerConnectionConfiguration extends Conn
         }
 
         try {
-            initialStateDescriptorVertex = StateDescriptorGraph.constructStateDescriptorGraph(backwardEdgeStateDescriptors);
+            initialStateDescriptorVertex = StateDescriptorGraph.constructStateDescriptorGraph(backwardEdgeStateDescriptors, builder.failOnUnknownStates);
         }
         catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException
                         | NoSuchMethodException | SecurityException e) {
@@ -91,6 +91,8 @@ public final class ModularXmppClientToServerConnectionConfiguration extends Conn
 
         private final Map<Class<? extends ModularXmppClientToServerConnectionModuleDescriptor>, ModularXmppClientToServerConnectionModuleDescriptor> modulesDescriptors = new HashMap<>();
 
+        private boolean failOnUnknownStates;
+
         private Builder() {
             SmackConfiguration.addAllKnownModulesTo(this);
         }
@@ -163,6 +165,17 @@ public final class ModularXmppClientToServerConnectionConfiguration extends Conn
             return getThis();
         }
 
+        /**
+         * Fail if there are unknown states in Smack's state descriptor graph. This method is used mostly for testing
+         * the internals of Smack. Users can safely ignore it.
+         *
+         * @return a reference to this builder.
+         */
+        public Builder failOnUnknownStates() {
+            failOnUnknownStates = true;
+            return getThis();
+        }
+
         @Override
         protected Builder getThis() {
             return this;

smack-core/src/main/java/org/jivesoftware/smack/fsm/StateDescriptorGraph.java

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright 2018 Florian Schmaus
+ * Copyright 2018-2021 Florian Schmaus
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -28,7 +28,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.logging.Logger;
 
 import org.jivesoftware.smack.c2s.ModularXmppClientToServerConnection.DisconnectedStateDescriptor;
 import org.jivesoftware.smack.c2s.internal.ModularXmppClientToServerConnectionInternal;
@@ -47,8 +46,6 @@ import org.jivesoftware.smack.util.MultiMap;
  */
 public class StateDescriptorGraph {
 
-    private static final Logger LOGGER = Logger.getLogger(StateDescriptorGraph.class.getName());
-
     private static GraphVertex<StateDescriptor> addNewStateDescriptorGraphVertex(
                     Class<? extends StateDescriptor> stateDescriptorClass,
                     Map<Class<? extends StateDescriptor>, GraphVertex<StateDescriptor>> graphVertexes)
@@ -102,7 +99,8 @@ public class StateDescriptorGraph {
     }
 
     private static void handleStateDescriptorGraphVertex(GraphVertex<StateDescriptor> node,
-                    HandleStateDescriptorGraphVertexContext context)
+                    HandleStateDescriptorGraphVertexContext context,
+                    boolean failOnUnknownStates)
                     throws InstantiationException, IllegalAccessException, IllegalArgumentException, InvocationTargetException, NoSuchMethodException, SecurityException {
         Class<? extends StateDescriptor> stateDescriptorClass = node.element.getClass();
         boolean alreadyHandled = context.recurseInto(stateDescriptorClass);
@@ -126,7 +124,7 @@ public class StateDescriptorGraph {
         case 1:
             GraphVertex<StateDescriptor> soleSuccessorNode = successorStateDescriptors.values().iterator().next();
             node.addOutgoingEdge(soleSuccessorNode);
-            handleStateDescriptorGraphVertex(soleSuccessorNode, context);
+            handleStateDescriptorGraphVertex(soleSuccessorNode, context, failOnUnknownStates);
             return;
         }
 
@@ -144,9 +142,8 @@ public class StateDescriptorGraph {
             StateDescriptor successorStateDescriptor = successorStateDescriptorGraphNode.element;
             Class<? extends StateDescriptor> successorStateDescriptorClass = successorStateDescriptor.getClass();
             for (Class<? extends StateDescriptor> subordinateClass : successorStateDescriptor.getSubordinates()) {
-                if (!successorClasses.contains(subordinateClass)) {
-                    LOGGER.severe(successorStateDescriptor + " points to a subordinate '" + subordinateClass + "' which is not part of the successor set");
-                    continue;
+                if (failOnUnknownStates && !successorClasses.contains(subordinateClass)) {
+                    throw new IllegalStateException(successorStateDescriptor + " points to a subordinate '" + subordinateClass + "' which is not part of the successor set");
                 }
 
                 GraphVertex<Class<? extends StateDescriptor>> superiorClassNode = lookupAndCreateIfRequired(
@@ -157,10 +154,9 @@ public class StateDescriptorGraph {
                 superiorClassNode.addOutgoingEdge(subordinateClassNode);
             }
             for (Class<? extends StateDescriptor> superiorClass : successorStateDescriptor.getSuperiors()) {
-                if (!successorClasses.contains(superiorClass)) {
-                    LOGGER.severe(successorStateDescriptor + " points to a superior '" + superiorClass
+                if (failOnUnknownStates && !successorClasses.contains(superiorClass)) {
+                    throw new IllegalStateException(successorStateDescriptor + " points to a superior '" + superiorClass
                                     + "' which is not part of the successor set");
-                    continue;
                 }
 
                 GraphVertex<Class<? extends StateDescriptor>> subordinateClassNode = lookupAndCreateIfRequired(
@@ -193,11 +189,13 @@ public class StateDescriptorGraph {
             node.addOutgoingEdge(successorVertex);
 
             // Recurse further.
-            handleStateDescriptorGraphVertex(successorVertex, context);
+            handleStateDescriptorGraphVertex(successorVertex, context, failOnUnknownStates);
         }
     }
 
-    public static GraphVertex<StateDescriptor> constructStateDescriptorGraph(Set<Class<? extends StateDescriptor>> backwardEdgeStateDescriptors)
+    public static GraphVertex<StateDescriptor> constructStateDescriptorGraph(
+                    Set<Class<? extends StateDescriptor>> backwardEdgeStateDescriptors,
+                    boolean failOnUnknownStates)
                     throws InstantiationException, IllegalAccessException, IllegalArgumentException,
                     InvocationTargetException, NoSuchMethodException, SecurityException {
         Map<Class<? extends StateDescriptor>, GraphVertex<StateDescriptor>> graphVertexes = new HashMap<>();
@@ -219,7 +217,7 @@ public class StateDescriptorGraph {
         }
 
         HandleStateDescriptorGraphVertexContext context = new HandleStateDescriptorGraphVertexContext(graphVertexes, inferredForwardEdges);
-        handleStateDescriptorGraphVertex(initialNode, context);
+        handleStateDescriptorGraphVertex(initialNode, context, failOnUnknownStates);
 
         return initialNode;
     }

smack-java8-full/src/test/java/org/jivesoftware/smack/full/ModularXmppClientToServerConnectionStateGraphTest.java

@@ -1,6 +1,6 @@
 /**
  *
- * Copyright 2020 Florian Schmaus
+ * Copyright 2020-2021 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,6 +17,7 @@
 package org.jivesoftware.smack.full;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.io.IOException;
 import java.io.PrintWriter;
@@ -25,6 +26,8 @@ import java.io.StringWriter;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
 
+import org.jivesoftware.smack.c2s.ModularXmppClientToServerConnectionConfiguration;
+import org.jivesoftware.smack.compression.CompressionModuleDescriptor;
 import org.jivesoftware.smack.util.EqualsUtil;
 import org.jivesoftware.smack.util.HashCode;
 
@@ -34,6 +37,7 @@ import org.jgrapht.graph.DirectedPseudograph;
 import org.jgrapht.io.DOTImporter;
 import org.jgrapht.io.ImportException;
 import org.junit.jupiter.api.Test;
+import org.jxmpp.stringprep.XmppStringprepException;
 
 public class ModularXmppClientToServerConnectionStateGraphTest {
 
@@ -80,4 +84,24 @@ public class ModularXmppClientToServerConnectionStateGraphTest {
         assertEquals(expectedStateGraph, currentStateGraph);
     }
 
+    @Test
+    public void testNoUnknownStates() throws XmppStringprepException {
+        ModularXmppClientToServerConnectionConfiguration.builder()
+            .setUsernameAndPassword("user", "password")
+            .setXmppDomain("example.org")
+            .failOnUnknownStates() // This is the actual option that enableds this test.
+            .build();
+    }
+
+    @Test
+    public void throwsOnUnknownStates() throws XmppStringprepException {
+        assertThrows(IllegalStateException.class, () ->
+            ModularXmppClientToServerConnectionConfiguration.builder()
+                .setUsernameAndPassword("user", "password")
+                .setXmppDomain("example.org")
+                .removeModule(CompressionModuleDescriptor.class)
+                .failOnUnknownStates() // This is the actual option that enableds this test.
+                .build()
+        );
+    }
 }