From 1f220e0e66cadda4c2ad79f543aea8b6f75d6e86 Mon Sep 17 00:00:00 2001
From: Philipp Etschel <philipp@comfylight.com>
Date: Wed, 6 Sep 2017 19:44:37 +0200
Subject: [PATCH] INT-4340: Disable XMPP Roster properly

JIRA: https://jira.spring.io/browse/INT-4340

* Roster is now disabled on login as well
* Fixed tagging and formatting issues
* Checkstyle and documented the changed behaviour as well

Polishing: rely on the `AbstractFactoryBean.getObject()`

**Cherry-pick to 4.3.x**
---
 .../config/XmppConnectionFactoryBean.java     | 33 +++++++++----
 .../XmppConnectionFactoryBeanTests.java       | 47 ++++++++++++++++++-
 src/reference/asciidoc/xmpp.adoc              |  8 ++--
 3 files changed, 73 insertions(+), 15 deletions(-)

diff --git a/spring-integration-xmpp/src/main/java/org/springframework/integration/xmpp/config/XmppConnectionFactoryBean.java b/spring-integration-xmpp/src/main/java/org/springframework/integration/xmpp/config/XmppConnectionFactoryBean.java
index 7c27126aa3..4e6c51c201 100644
--- a/spring-integration-xmpp/src/main/java/org/springframework/integration/xmpp/config/XmppConnectionFactoryBean.java
+++ b/spring-integration-xmpp/src/main/java/org/springframework/integration/xmpp/config/XmppConnectionFactoryBean.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -37,6 +37,7 @@ import org.springframework.util.StringUtils;
  * @author Oleg Zhurakousky
  * @author Florian Schmaus
  * @author Artem Bilan
+ * @author Philipp Etschel
  *
  * @see XMPPTCPConnection
  * @since 2.0
@@ -151,8 +152,16 @@ public class XmppConnectionFactoryBean extends AbstractFactoryBean<XMPPConnectio
 
 			connectionConfiguration = builder.build();
 		}
-		this.connection = new XMPPTCPConnection(connectionConfiguration);
-		return this.connection;
+		return new XMPPTCPConnection(connectionConfiguration);
+	}
+
+	protected XMPPTCPConnection getConnection() {
+		try {
+			return (XMPPTCPConnection) getObject();
+		}
+		catch (Exception e) {
+			throw new IllegalStateException("Cannot obtain connection instance", e);
+		}
 	}
 
 	@Override
@@ -161,19 +170,23 @@ public class XmppConnectionFactoryBean extends AbstractFactoryBean<XMPPConnectio
 			if (this.running) {
 				return;
 			}
+			XMPPTCPConnection connection = getConnection();
 			try {
-				this.connection.connect();
-				this.connection.addConnectionListener(new LoggingConnectionListener());
-				this.connection.login();
+				connection.connect();
+				connection.addConnectionListener(new LoggingConnectionListener());
+				Roster roster = Roster.getInstanceFor(connection);
 				if (this.subscriptionMode != null) {
-					Roster.getInstanceFor(this.connection)
-							.setSubscriptionMode(this.subscriptionMode);
+					roster.setSubscriptionMode(this.subscriptionMode);
 				}
+				else {
+					roster.setRosterLoadedAtLogin(false);
+				}
+				connection.login();
 				this.running = true;
 			}
 			catch (Exception e) {
 				throw new BeanInitializationException("failed to connect to XMPP service for "
-						+ this.connection.getServiceName(), e);
+						+ connection.getServiceName(), e);
 			}
 		}
 	}
@@ -182,7 +195,7 @@ public class XmppConnectionFactoryBean extends AbstractFactoryBean<XMPPConnectio
 	public void stop() {
 		synchronized (this.lifecycleMonitor) {
 			if (this.isRunning()) {
-				this.connection.disconnect();
+				getConnection().disconnect();
 				this.running = false;
 			}
 		}
diff --git a/spring-integration-xmpp/src/test/java/org/springframework/integration/xmpp/config/XmppConnectionFactoryBeanTests.java b/spring-integration-xmpp/src/test/java/org/springframework/integration/xmpp/config/XmppConnectionFactoryBeanTests.java
index 40d558a74d..b207b986fc 100644
--- a/spring-integration-xmpp/src/test/java/org/springframework/integration/xmpp/config/XmppConnectionFactoryBeanTests.java
+++ b/spring-integration-xmpp/src/test/java/org/springframework/integration/xmpp/config/XmppConnectionFactoryBeanTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,10 +16,14 @@
 
 package org.springframework.integration.xmpp.config;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 
 import org.jivesoftware.smack.XMPPConnection;
+import org.jivesoftware.smack.roster.Roster;
+import org.jivesoftware.smack.tcp.XMPPTCPConnection;
 import org.jivesoftware.smack.tcp.XMPPTCPConnectionConfiguration;
 import org.junit.Test;
 
@@ -29,6 +33,7 @@ import org.springframework.context.support.ClassPathXmlApplicationContext;
  * @author Oleg Zhurakousky
  * @author Gunnar Hillert
  * @author Artem Bilan
+ * @author Philipp Etschel
  */
 public class XmppConnectionFactoryBeanTests {
 
@@ -46,4 +51,44 @@ public class XmppConnectionFactoryBeanTests {
 		// the fact that no exception was thrown satisfies this test
 	}
 
+	@Test
+	public void testXmppConnectionFactoryBeanNoRoster() throws Exception {
+		XmppConnectionFactoryBean xmppConnectionFactoryBean =
+				new XmppConnectionFactoryBean() {
+
+					@Override
+					protected XMPPConnection createInstance() throws Exception {
+						return mock(XMPPTCPConnection.class);
+					}
+
+				};
+
+		xmppConnectionFactoryBean.setSubscriptionMode(null);
+		xmppConnectionFactoryBean.afterPropertiesSet();
+		xmppConnectionFactoryBean.start();
+		XMPPConnection connection = xmppConnectionFactoryBean.getObject();
+
+		assertFalse(Roster.getInstanceFor(connection).isRosterLoadedAtLogin());
+	}
+
+	@Test
+	public void testXmppConnectionFactoryBeanWithRoster() throws Exception {
+		XmppConnectionFactoryBean xmppConnectionFactoryBean =
+				new XmppConnectionFactoryBean() {
+
+					@Override
+					protected XMPPConnection createInstance() throws Exception {
+						return mock(XMPPTCPConnection.class);
+					}
+
+				};
+
+		xmppConnectionFactoryBean.setSubscriptionMode(Roster.SubscriptionMode.accept_all);
+		xmppConnectionFactoryBean.afterPropertiesSet();
+		xmppConnectionFactoryBean.start();
+		XMPPConnection connection = xmppConnectionFactoryBean.getObject();
+
+		assertTrue(Roster.getInstanceFor(connection).isRosterLoadedAtLogin());
+	}
+
 }
diff --git a/src/reference/asciidoc/xmpp.adoc b/src/reference/asciidoc/xmpp.adoc
index ffd9987678..74e8f77e38 100644
--- a/src/reference/asciidoc/xmpp.adoc
+++ b/src/reference/asciidoc/xmpp.adoc
@@ -54,10 +54,9 @@ We also register a `ConnectionListener` which will log connection events if the
 
 The `subscription-mode` initiates the Roster listener to deal with incoming subscriptions from other users.
 This functionality isn't always available for the target XMPP servers.
-For example GCM fully disables it.
-To switch off the Roster listener for subscriptions you should configure it with an empty string when using XML
-configuration: `subscription-mode=""`, or with `XmppConnectionFactoryBean.setSubscriptionMode(null)`
-when using Java Configuration.
+For example GCM/FCM fully disables it.
+To switch off the Roster listener for subscriptions you should configure it with an empty string when using XML configuration: `subscription-mode=""`, or with `XmppConnectionFactoryBean.setSubscriptionMode(null)` when using Java Configuration. Doing so will disable Roster at the login phase as well.
+See `Roster.setRosterLoadedAtLogin(Boolean)` for more information.
 
 [[xmpp-messages]]
 === XMPP Messages
@@ -240,6 +239,7 @@ Here's an example:
         </bean>
     </constructor-arg>
 </bean>
+
 <int:channel id="outboundEventChannel"/>
 
 <int-xmpp:outbound-channel-adapter id="outboundEventAdapter"