[xmpp] Improve reconnection logic (openhab#14397)

* Add null annotations
* Introduce re-connect logic

Signed-off-by: lsiepel <[email protected]>
Signed-off-by: Leo Siepel <[email protected]>

Author: lsiepel

bundles/org.openhab.binding.xmppclient/README.md

@@ -26,22 +26,23 @@ Bridge xmppclient:xmppBridge:xmpp "XMPP Client" [ host="xmpp.example.com", port=
 
 **xmppBridge** parameters:
 
-| Name     | Label              | Description                               |  Required | Default value         |
-|----------|--------------------|-------------------------------------------|-----------|-----------------------|
-| username | Username           | The XMPP username (left part of JID)      | true      | -                     |
-| domain   | Domain             | The XMPP domain name (right part of JID)  | true      | -                     |
-| password | Password           | The XMPP user password                    | true      | -                     |
-| host     | Server Hostname/IP | The IP/Hostname of the XMPP server        | false     | as "domain" parameter |
-| port     | XMPP server Port   | The typical port is 5222                  | false     | 5222                  |
+| Name         | Label              | Description                                                        | Required | Default value         |
+|--------------|--------------------|--------------------------------------------------------------------|----------|-----------------------|
+| username     | Username           | The XMPP username (left part of JID)                               | true     | -                     |
+| domain       | Domain             | The XMPP domain name (right part of JID)                           | true     | -                     |
+| password     | Password           | The XMPP user password                                             | true     | -                     |
+| host         | Server Hostname/IP | The IP/Hostname of the XMPP server                                 | false    | as "domain" parameter |
+| port         | XMPP server Port   | The typical port is 5222                                           | false    | 5222                  |
+| securityMode | Security Mode      | Sets the TLS security mode: `required`, `ifpossible` or `disabled` | false    | `required`            |
 
 ## Channels
 
 **publishTrigger** parameters:
 
-| Name      |      Label          |  Description                                                                                                                                                                                                                                       |  Required |
-|-----------|---------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------|
-| payload   | Payload condition   | An optional condition on the value                                                                                                                                                                                                                 | false     |
-| separator | Separator character | The trigger channel payload usually only contains the received text. If you define a separator character, for example '#', the sender UID and received text will be in the trigger channel payload. For example: [email protected]#My Message Text | false     |
+| Name      | Label               | Description                                                                                                                                                                                                                                        | Required |
+|-----------|---------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|
+| payload   | Payload condition   | An optional condition on the value                                                                                                                                                                                                                 | false    |
+| separator | Separator character | The trigger channel payload usually only contains the received text. If you define a separator character, for example '#', the sender UID and received text will be in the trigger channel payload. For example: [email protected]#My Message Text | false    |
 
 ## Example Rules
 

bundles/org.openhab.binding.xmppclient/src/main/java/org/openhab/binding/xmppclient/internal/XMPPClientBindingConstants.java

@@ -12,6 +12,7 @@
  */
 package org.openhab.binding.xmppclient.internal;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.openhab.core.thing.ThingTypeUID;
 
 /**
@@ -20,6 +21,7 @@
  *
  * @author Pavel Gololobov - Initial contribution
  */
+@NonNullByDefault
 public class XMPPClientBindingConstants {
     private static final String BINDING_ID = "xmppclient";
 

bundles/org.openhab.binding.xmppclient/src/main/java/org/openhab/binding/xmppclient/internal/XMPPClientHandlerFactory.java

@@ -14,6 +14,8 @@
 
 import java.util.Set;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.xmppclient.internal.handler.XMPPClientHandler;
 import org.openhab.core.thing.Bridge;
 import org.openhab.core.thing.Thing;
@@ -29,6 +31,7 @@
  *
  * @author Pavel Gololobov - Initial contribution
  */
+@NonNullByDefault
 @Component(configurationPid = "binding.xmppclient", service = ThingHandlerFactory.class)
 public class XMPPClientHandlerFactory extends BaseThingHandlerFactory {
     private static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Set
@@ -40,7 +43,7 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
     }
 
     @Override
-    protected ThingHandler createHandler(Thing thing) {
+    protected @Nullable ThingHandler createHandler(Thing thing) {
         ThingTypeUID thingTypeUID = thing.getThingTypeUID();
 
         if (thingTypeUID.equals(XMPPClientBindingConstants.BRIDGE_TYPE_XMPP)) {

bundles/org.openhab.binding.xmppclient/src/main/java/org/openhab/binding/xmppclient/internal/action/XMPPActions.java

@@ -14,7 +14,7 @@
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
-import org.openhab.binding.xmppclient.internal.XMPPClient;
+import org.openhab.binding.xmppclient.internal.client.XMPPClient;
 import org.openhab.binding.xmppclient.internal.handler.XMPPClientHandler;
 import org.openhab.core.automation.annotation.ActionInput;
 import org.openhab.core.automation.annotation.RuleAction;
@@ -35,7 +35,7 @@
 @ThingActionsScope(name = "xmppclient")
 @NonNullByDefault
 public class XMPPActions implements ThingActions {
-    private static final Logger logger = LoggerFactory.getLogger(XMPPActions.class);
+    private final Logger logger = LoggerFactory.getLogger(XMPPActions.class);
     private @Nullable XMPPClientHandler handler;
 
     @Override
@@ -58,12 +58,8 @@ public void publishXMPP(@ActionInput(name = "to", label = "To", description = "S
         }
 
         XMPPClient connection = clientHandler.getXMPPClient();
-        if (connection == null) {
-            logger.warn("XMPP ThingHandler connection is null");
-            return;
-        }
-        if ((to == null) || (text == null)) {
-            logger.info("Skipping XMPP messaging to {} value {}", to, text);
+        if (to == null || text == null) {
+            logger.warn("Skipping XMPP messaging to {} value {}", to, text);
             return;
         }
         connection.sendMessage(to, text);
@@ -80,11 +76,7 @@ public void publishXMPPImageByHTTP(
         }
 
         XMPPClient connection = clientHandler.getXMPPClient();
-        if (connection == null) {
-            logger.warn("XMPP ThingHandler connection is null");
-            return;
-        }
-        if ((to == null) || (filename == null)) {
+        if (to == null || filename == null) {
             logger.warn("Skipping XMPP messaging to {} value {}", to, filename);
             return;
         }

bundles/org.openhab.binding.xmppclient/src/main/java/org/openhab/binding/xmppclient/internal/XMPPClient.java bundles/org.openhab.binding.xmppclient/src/main/java/org/openhab/binding/xmppclient/internal/client/XMPPClient.java

@@ -10,15 +10,19 @@
  *
  * SPDX-License-Identifier: EPL-2.0
  */
-package org.openhab.binding.xmppclient.internal;
+package org.openhab.binding.xmppclient.internal.client;
 
 import java.io.File;
 import java.io.IOException;
 import java.net.URL;
 import java.util.HashSet;
+import java.util.Objects;
 import java.util.Set;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.jivesoftware.smack.AbstractXMPPConnection;
+import org.jivesoftware.smack.ConnectionConfiguration.SecurityMode;
 import org.jivesoftware.smack.ConnectionListener;
 import org.jivesoftware.smack.ReconnectionManager;
 import org.jivesoftware.smack.SmackException;
@@ -44,13 +48,20 @@
  * The {@link XMPPClient} is lib for handling XMPP connection and messaging
  *
  * @author Pavel Gololobov - Initial contribution
+ * @author Leo Siepel - Add reconnection logic
  */
+@NonNullByDefault
 public class XMPPClient implements IncomingChatMessageListener, ConnectionListener {
     private final Logger logger = LoggerFactory.getLogger(XMPPClient.class);
-    private AbstractXMPPConnection connection;
-    private ChatManager chatManager;
-    private HttpFileUploadManager httpFileUploadManager;
+    private @Nullable AbstractXMPPConnection connection;
+    private @Nullable ChatManager chatManager;
+    private @Nullable HttpFileUploadManager httpFileUploadManager;
     private Set<XMPPClientMessageSubscriber> subscribers = new HashSet<>();
+    private final XMPPClientEventlistener eventListener;
+
+    public XMPPClient(XMPPClientEventlistener eventListener) {
+        this.eventListener = eventListener;
+    }
 
     public void subscribe(XMPPClientMessageSubscriber channel) {
         logger.debug("Channel {} subscribed", channel.getName());
@@ -62,62 +73,75 @@ public void unsubscribe(XMPPClientMessageSubscriber channel) {
         subscribers.remove(channel);
     }
 
-    public void connect(String host, Integer port, String login, String domain, String password)
-            throws XMPPException, SmackException, IOException {
+    public void connect(String host, Integer port, String login, String domain, String password,
+            SecurityMode securityMode) throws XMPPClientConfigException, XMPPClientException {
         disconnect();
         String serverHost = domain;
-        if ((host != null) && !host.isEmpty()) {
+        if (!host.isBlank()) {
             serverHost = host;
         }
 
-        XMPPTCPConnectionConfiguration config = XMPPTCPConnectionConfiguration.builder() //
-                .setHost(serverHost) //
-                .setPort(port) //
-                .setUsernameAndPassword(login, password) //
-                .setXmppDomain(domain) //
-                .build();
+        XMPPTCPConnectionConfiguration config;
+        try {
+            config = XMPPTCPConnectionConfiguration.builder() //
+                    .setHost(serverHost) //
+                    .setPort(port) //
+                    .setUsernameAndPassword(login, password) //
+                    .setXmppDomain(domain) //
+                    .setSecurityMode(securityMode)//
+                    .build();
+        } catch (XmppStringprepException e) {
+            throw new XMPPClientConfigException(Objects.requireNonNullElse(e.getMessage(), "Unknown error message"));
+        }
 
-        connection = new XMPPTCPConnection(config);
-        connection.addConnectionListener(this);
+        AbstractXMPPConnection connectionLocal = new XMPPTCPConnection(config);
+        connection = connectionLocal;
+        connectionLocal.addConnectionListener(this);
 
-        ReconnectionManager reconnectionManager = ReconnectionManager.getInstanceFor(connection);
+        ReconnectionManager reconnectionManager = ReconnectionManager.getInstanceFor(connectionLocal);
         reconnectionManager.enableAutomaticReconnection();
 
         Identity identity = new Identity("client", "openHAB", "bot");
         ServiceDiscoveryManager sdm = ServiceDiscoveryManager.getInstanceFor(connection);
         sdm.setIdentity(identity);
 
         try {
-            connection.connect().login();
-        } catch (InterruptedException ex) {
+            connectionLocal.connect().login();
+        } catch (InterruptedException | XMPPException | SmackException | IOException e) {
+            throw new XMPPClientException(Objects.requireNonNullElse(e.getMessage(), "Unknown error message"),
+                    e.getCause());
         }
 
-        chatManager = ChatManager.getInstanceFor(connection);
+        ChatManager chatManager = ChatManager.getInstanceFor(connection);
         chatManager.addIncomingListener(this);
+        this.chatManager = chatManager;
         httpFileUploadManager = HttpFileUploadManager.getInstanceFor(connection);
     }
 
     public void disconnect() {
+        AbstractXMPPConnection connection = this.connection;
         if (connection != null) {
             connection.disconnect();
         }
     }
 
     public void sendMessage(String to, String message) {
         if (connection == null) {
-            logger.warn("XMPP connection is null");
+            eventListener.onErrorEvent("XMPP connection is null");
             return;
         }
+
+        ChatManager chatManager = this.chatManager;
         if (chatManager == null) {
-            logger.warn("XMPP chatManager is null");
+            eventListener.onErrorEvent("XMPP chatManager is null");
             return;
         }
         try {
             EntityBareJid jid = JidCreate.entityBareFrom(to);
             Chat chat = chatManager.chatWith(jid);
             chat.send(message);
         } catch (XmppStringprepException | SmackException.NotConnectedException | InterruptedException e) {
-            logger.info("XMPP message sending error", e);
+            logger.warn("XMPP message sending error", e);
         }
     }
 
@@ -126,12 +150,13 @@ public void sendImageByHTTP(String to, String filename) {
             logger.warn("XMPP connection is null");
             return;
         }
-        if (httpFileUploadManager == null) {
+        HttpFileUploadManager httpFileUploadManagerLocal = httpFileUploadManager;
+        if (httpFileUploadManagerLocal == null) {
             logger.warn("XMPP httpFileUploadManager is null");
             return;
         }
         try {
-            URL u = httpFileUploadManager.uploadFile(new File(filename));
+            URL u = httpFileUploadManagerLocal.uploadFile(new File(filename));
             // Use Stanza oob
             this.sendMessage(to, u.toString());
         } catch (XMPPException.XMPPErrorException | SmackException | InterruptedException | IOException e) {
@@ -140,7 +165,12 @@ public void sendImageByHTTP(String to, String filename) {
     }
 
     @Override
-    public void newIncomingMessage(EntityBareJid from, Message message, Chat chat) {
+    public void newIncomingMessage(@Nullable EntityBareJid from, @Nullable Message message, @Nullable Chat chat) {
+        if (from == null || message == null || chat == null) {
+            logger.debug("newIncomingMessage with atleast one null argument, should not happen");
+            return;
+        }
+
         logger.debug("XMPP {} says {}", from.asBareJid().toString(), message.getBody());
         for (XMPPClientMessageSubscriber subscriber : subscribers) {
             logger.debug("Push to subscriber {}", subscriber.getName());
@@ -149,30 +179,26 @@ public void newIncomingMessage(EntityBareJid from, Message message, Chat chat) {
     }
 
     @Override
-    public void connected(XMPPConnection connection) {
+    public void connected(@Nullable XMPPConnection connection) {
         logger.debug("Connected to XMPP server.");
+        eventListener.onAllOk();
     }
 
     @Override
-    public void authenticated(XMPPConnection connection, boolean resumed) {
+    public void authenticated(@Nullable XMPPConnection connection, boolean resumed) {
         logger.debug("Authenticated to XMPP server.");
+        eventListener.onAllOk();
     }
 
     @Override
     public void connectionClosed() {
         logger.debug("XMPP connection was closed.");
+        eventListener.onErrorEvent("XMPP connection was closed.");
     }
 
     @Override
-    public void connectionClosedOnError(Exception e) {
+    public void connectionClosedOnError(@Nullable Exception e) {
         logger.debug("Connection to XMPP server was lost.");
-        if (connection != null) {
-            connection.disconnect();
-            try {
-                connection.connect().login();
-            } catch (SmackException | IOException | XMPPException | InterruptedException ex) {
-                logger.info("XMPP connection error", ex);
-            }
-        }
+        eventListener.onErrorEvent("XMPP connection was closed.");
     }
 }

bundles/org.openhab.binding.xmppclient/src/main/java/org/openhab/binding/xmppclient/internal/client/XMPPClientConfigException.java

@@ -0,0 +1,40 @@
+/**
+ * Copyright (c) 2010-2024 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.binding.xmppclient.internal.client;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
+
+/**
+ * The {@link XMPPClientConfigException} represents a binding specific {@link Exception}.
+ *
+ * @author Leo Siepel - Initial contribution
+ */
+
+@NonNullByDefault
+public class XMPPClientConfigException extends Exception {
+
+    private static final long serialVersionUID = 1L;
+
+    public XMPPClientConfigException(String message) {
+        super(message);
+    }
+
+    public XMPPClientConfigException(String message, @Nullable Throwable cause) {
+        super(message, cause);
+    }
+
+    public XMPPClientConfigException(@Nullable Throwable cause) {
+        super(cause);
+    }
+}