Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Commit

Permalink
MqttService: Remove all deprecated methods, rename close to stop and …
Browse files Browse the repository at this point in the history 
…more.

* Remove name from MqttBrokerConnection. This is only necessary for the MqttService.
  Consumers of MqttBrokerConnection should not be bothered with that detail.
* Remove textConfigured flag. Again this is not interesting for anything outside MqttService.
* Don't parse configuration parameters ourself, but use a configuration values class instead.
* Use Nullable were it makes sense.

Signed-off-by: David Gräff <david.graeff@web.de>
  • Loading branch information
David Gräff committed Sep 1, 2017
1 parent 002c468 commit 370b6cf
Show file tree
Hide file tree
Showing 12 changed files with 240 additions and 554 deletions.
85 changes: 30 additions & 55 deletions ...sport/mqtt/MqttBrokerConnectionTests.java → ...nsport/mqtt/MqttBrokerConnectionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,31 +27,10 @@
*
* @author David Graeff - Initial contribution
*/
public class MqttBrokerConnectionTests {
@Test
public void testConstructor() throws ConfigurationException {
// Test tcp and ssl URLs
MqttBrokerConnection a = new MqttBrokerConnection("name", "tcp://123.123.123.123", false);
MqttBrokerConnection b = new MqttBrokerConnection("name", "ssl://123.123.123.123", true);
assertFalse(a.isTextualConfiguredBroker());
assertTrue(b.isTextualConfiguredBroker());
}

@Test(expected = ConfigurationException.class)
public void testConstructorInvalidProtocol() throws ConfigurationException {
new MqttBrokerConnection("name", "unsupported://123.123.123.123", false);
}

@Test(expected = ConfigurationException.class)
public void testConstructorInvalidName() throws ConfigurationException, MqttException {
new MqttBrokerConnection(" ", "tcp://123.123.123.123", false);
}

public class MqttBrokerConnectionTest {
@Test
public void messageConsumerTests() throws ConfigurationException, MqttException {
final String url = "tcp://123.123.123.123";
final String name = "TestName12@!";
MqttBrokerConnection a = new MqttBrokerConnection(name, url, false);
MqttBrokerConnection a = new MqttBrokerConnection("123.123.123.123", 0, false);
// Expect no consumers
assertFalse(a.hasConsumers());

Expand All @@ -69,9 +48,7 @@ public void messageConsumerTests() throws ConfigurationException, MqttException

@Test
public void reconnectPolicyDefaultTest() throws ConfigurationException, MqttException, InterruptedException {
final String url = "tcp://123.123.123.123";
final String name = "TestName12@!";
MqttBrokerConnection a = new MqttBrokerConnection(name, url, false);
MqttBrokerConnection a = new MqttBrokerConnection("123.123.123.123", 0, false);

// Check if the default policy is set and that the broker within the policy is set.
assertTrue(a.getReconnectStrategy() instanceof PeriodicReconnectStrategy);
Expand All @@ -81,26 +58,23 @@ public void reconnectPolicyDefaultTest() throws ConfigurationException, MqttExce

@Test
public void reconnectPolicyTests() throws ConfigurationException, MqttException, InterruptedException {
final String url = "tcp://123.123.123.123";
final String name = "TestName12@!";
MqttBrokerConnection a = spy(new MqttBrokerConnection(name, url, false));
MqttBrokerConnection a = spy(new MqttBrokerConnection("123.123.123.123", 0, false));

// Check setter
a.setReconnectStrategy(new PeriodicReconnectStrategy());
assertThat(a.getReconnectStrategy().getBrokerConnection(), is(a));

// Prepare a Mock to test if lostConnect is called and
// if the PeriodicReconnectPolicy indeed calls start()
PeriodicReconnectStrategy mockPolicy = spy(new PeriodicReconnectStrategy());
doReturn(a).when(mockPolicy).getBrokerConnection();
doReturn(0).when(mockPolicy).getFirstReconnectAfter();
doReturn(10000).when(mockPolicy).getReconnectFrequency();
PeriodicReconnectStrategy mockPolicy = spy(new PeriodicReconnectStrategy(10000, 0));
doNothing().when(a).start();
mockPolicy.start();
a.isConnecting = true;

// Fake a disconnect
a.setReconnectStrategy(mockPolicy);
IMqttActionListener l = a.createConnectionListener();
doReturn(false).when(a).isConnected();
doReturn(MqttConnectionState.DISCONNECTED).when(a).connectionState();
IMqttToken token = mock(IMqttToken.class);
when(token.getException()).thenReturn(new org.eclipse.paho.client.mqttv3.MqttException(1));
l.onFailure(token, null);
Expand All @@ -118,9 +92,7 @@ public void reconnectPolicyTests() throws ConfigurationException, MqttException,

@Test
public void connectionObserverTests() throws ConfigurationException, MqttException {
final String url = "tcp://123.123.123.123";
final String name = "TestName12@!";
MqttBrokerConnection a = spy(new MqttBrokerConnection(name, url, false));
MqttBrokerConnection a = spy(new MqttBrokerConnection("123.123.123.123", 0, false));

// Add an observer
assertFalse(a.hasConnectionObservers());
Expand All @@ -133,7 +105,7 @@ public void connectionObserverTests() throws ConfigurationException, MqttExcepti

// Cause a success callback
IMqttActionListener l = a.createConnectionListener();
doReturn(true).when(a).isConnected();
doReturn(MqttConnectionState.CONNECTED).when(a).connectionState();
l.onSuccess(null);
verify(connectionObserver, times(1)).connectionStateChanged(eq(MqttConnectionState.CONNECTED), anyObject());

Expand All @@ -143,7 +115,7 @@ public void connectionObserverTests() throws ConfigurationException, MqttExcepti
1);
when(token.getException()).thenReturn(testException);

doReturn(false).when(a).isConnected();
doReturn(MqttConnectionState.DISCONNECTED).when(a).connectionState();
l.onFailure(token, null);
verify(connectionObserver, times(1)).connectionStateChanged(eq(MqttConnectionState.DISCONNECTED),
eq(testException));
Expand All @@ -155,9 +127,7 @@ public void connectionObserverTests() throws ConfigurationException, MqttExcepti

@Test
public void lastWillAndTestamentTests() throws ConfigurationException {
final String url = "tcp://123.123.123.123";
final String name = "TestName12@!";
MqttBrokerConnection a = new MqttBrokerConnection(name, url, false);
MqttBrokerConnection a = new MqttBrokerConnection("123.123.123.123", 0, false);

assertNull(a.getLastWill());
assertNull(MqttWillAndTestament.fromString(""));
Expand All @@ -174,19 +144,28 @@ public void lastWillAndTestamentConstructorTests() {
new MqttWillAndTestament("", new byte[0], 0, false);
}

@Test(expected = IllegalArgumentException.class)
public void tooLongClientID() throws ConfigurationException {
MqttBrokerConnection a = new MqttBrokerConnection("123.123.123.123", 0, false);
// client ids longer than 23 characters should throw
a.setClientId("clientidclientidclientidclientid");
}

@Test(expected = IllegalArgumentException.class)
public void qosInvalid() throws ConfigurationException {
MqttBrokerConnection a = new MqttBrokerConnection("123.123.123.123", 0, false);
a.setQos(10);
}

@Test
public void setterGetterTests() throws ConfigurationException {
final String url = "tcp://123.123.123.123";
final String name = "TestName12@!";
MqttBrokerConnection a = new MqttBrokerConnection(name, url, false);
assertEquals("URL getter", a.getUrl(), url);
assertEquals("Name getter", a.getName(), name);
MqttBrokerConnection a = new MqttBrokerConnection("123.123.123.123", 0, false);
assertEquals("URL getter", a.getHost(), "123.123.123.123");
assertEquals("Name getter", a.getPort(), 0);
assertEquals("Secure getter", a.isSecure(), false);

a.setClientId("clientid");
assertEquals("ClientID getter/setter", "clientid", a.getClientId());
// client ids longer than 23 characters should be ignored
a.setClientId("clientidclientidclientidclientid");
assertEquals("ClientID too long check", "clientid", a.getClientId());

a.setCredentials("user@!", "password123@^");
assertEquals("User getter/setter", "user@!", a.getUser());
Expand All @@ -200,10 +179,6 @@ public void setterGetterTests() throws ConfigurationException {
a.setRetain(true);
assertTrue(a.isRetain());

assertEquals(MqttBrokerConnection.DEFAULT_QOS, a.getQos());
a.setQos(10);
assertEquals(MqttBrokerConnection.DEFAULT_QOS, a.getQos());
a.setQos(-10);
assertEquals(MqttBrokerConnection.DEFAULT_QOS, a.getQos());
a.setQos(2);
assertEquals(2, a.getQos());
Expand All @@ -214,6 +189,6 @@ public void setterGetterTests() throws ConfigurationException {
assertNotNull(a.getSSLContextProvider());
assertNotNull(a.getReconnectStrategy());

assertFalse(a.isConnected());
assertThat(a.connectionState(), is(MqttConnectionState.DISCONNECTED));
}
}
52 changes: 28 additions & 24 deletions ...e/io/transport/mqtt/MqttServiceTests.java → ...me/io/transport/mqtt/MqttServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,23 @@
import static org.junit.Assert.*;
import static org.mockito.Mockito.*;

import java.math.BigDecimal;
import java.util.Collections;
import java.util.Hashtable;
import java.util.Map;

import javax.naming.ConfigurationException;

import org.eclipse.smarthome.config.core.Configuration;
import org.eclipse.smarthome.io.transport.mqtt.internal.ConnectionConfiguration;
import org.junit.Test;

/**
* Tests the MqttService class
*
* @author David Graeff - Initial contribution
*/
public class MqttServiceTests {
public class MqttServiceTest {
// Tests addBrokersListener/removeBrokersListener
@Test
public void brokerConnectionListenerTests() throws ConfigurationException {
Expand All @@ -35,11 +38,11 @@ public void brokerConnectionListenerTests() throws ConfigurationException {
service.addBrokersListener(observer);
assertTrue(service.hasBrokerObservers());

MqttBrokerConnection connection = new MqttBrokerConnection("name", "tcp://123.123.123.123", false);
assertTrue(service.addBrokerConnection(connection));
MqttBrokerConnection connection = new MqttBrokerConnection("123.123.123.123", 0, false);
assertTrue(service.addBrokerConnection("name", connection));
verify(observer).brokerAdded(connection);

service.removeBrokerConnection(connection);
service.removeBrokerConnection("name");
verify(observer).brokerRemoved(connection);

service.removeBrokersListener(observer);
Expand All @@ -52,39 +55,40 @@ public void extractBrokerConfigurationsTests() throws ConfigurationException, Mq
MqttService service = new MqttService();

Map<String, Object> properties = new Hashtable<>();
properties.put("bam.name", "brokername");
properties.put("bam.url", "tcp://123.123.123.123");
Map<String, Map<String, String>> map = service.extractBrokerConfigurations(properties);
properties.put("bam.host", "123.123.123.123");
properties.put("bam.port", BigDecimal.valueOf(1234));
properties.put("bam.secure", false);
Map<String, Map<String, Object>> map = service.extractBrokerConfigurations(properties);
assertEquals(map.size(), 1);
Map<String, String> data = map.get("bam");
assertNotNull(data);
assertEquals("brokername", data.get("name"));
assertEquals("tcp://123.123.123.123", data.get("url"));
Map<String, Object> data = map.get("bam");
@SuppressWarnings("null")
ConnectionConfiguration c = new Configuration(data).as(ConnectionConfiguration.class);
assertEquals("123.123.123.123", c.host);
assertEquals(false, c.secure);
assertEquals(BigDecimal.valueOf(1234), c.port);
}

// Tests if updates to the textual configuration are processed correctly
// Tests if updates to the system broker connections via ConfigAdmin are processed correctly
@Test
public void textualConfigurationTests() throws ConfigurationException, MqttException {
public void updateViaConfigAdminTests() throws ConfigurationException, MqttException {
MqttService service = new MqttService();

Map<String, Object> properties = new Hashtable<>();
properties.put("bam.name", "brokername");
properties.put("bam.url", "tcp://123.123.123.123");
properties.put("bam.host", "123.123.123.123");

// Test activate
service.activate(properties);
assertThat(service.getAllBrokerConnections(), hasSize(1));
assertNotNull(service.getBrokerConnection("brokername"));
assertNotNull(service.getBrokerConnection("bam"));

Map<String, Object> properties2 = new Hashtable<>();
properties2.put("bam2.name", "brokername2");
properties2.put("bam2.url", "tcp://123.123.123.123");
properties2.put("bam2.url", "123.123.123.123");

// Test configuration change
service.modified(properties2);
assertThat(service.getAllBrokerConnections(), hasSize(1));
assertNull(service.getBrokerConnection("brokername"));
assertNotNull(service.getBrokerConnection("brokername2"));
assertNull(service.getBrokerConnection("bam"));
assertNotNull(service.getBrokerConnection("bam2"));

// Test if old broker connections are freed correctly
service.modified(Collections.emptyMap());
Expand All @@ -96,22 +100,22 @@ public void brokerConnectionAddRemoveEnumerateTests() {
MqttService service = new MqttService();
MqttBrokerConnection connection;
try {
connection = new MqttBrokerConnection("name", "tcp://123.123.123.123", false);
connection = new MqttBrokerConnection("123.123.123.123", 0, false);
} catch (ConfigurationException c) {
fail("Couldn't create a MqttBrokerConnection object");
return;
}
// Add
assertThat(service.getAllBrokerConnections(), hasSize(0));
assertTrue(service.addBrokerConnection(connection));
assertFalse(service.addBrokerConnection(connection));
assertTrue(service.addBrokerConnection("name", connection));
assertFalse(service.addBrokerConnection("name", connection));

// Get/Enumerate
assertNotNull(service.getBrokerConnection("name"));
assertThat(service.getAllBrokerConnections(), hasSize(1));

// Remove
service.removeBrokerConnection(connection);
service.removeBrokerConnection("name");
assertThat(service.getAllBrokerConnections(), hasSize(0));
}
}
1 change: 1 addition & 0 deletions bundles/io/org.eclipse.smarthome.io.transport.mqtt/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Import-Package: javax.naming,
org.eclipse.paho.client.mqttv3.logging,
org.eclipse.paho.client.mqttv3.persist,
org.eclipse.paho.client.mqttv3.util,
org.eclipse.smarthome.config.core,
org.eclipse.smarthome.core.events,
org.eclipse.smarthome.io.transport.mqtt,
org.osgi.service.component,
Expand Down
Loading

0 comments on commit 370b6cf

Please sign in to comment.