Removed polling from ModemPlugin.

briar-desktop/src/org/briarproject/plugins/modem/ModemFactoryImpl.java

@@ -8,19 +8,19 @@ import org.briarproject.system.SystemClock;
 
 class ModemFactoryImpl implements ModemFactory {
 
-	private final Executor executor;
+	private final Executor ioExecutor;
 	private final ReliabilityLayerFactory reliabilityFactory;
 	private final Clock clock;
 
-	ModemFactoryImpl(Executor executor,
+	ModemFactoryImpl(Executor ioExecutor,
 			ReliabilityLayerFactory reliabilityFactory) {
-		this.executor = executor;
+		this.ioExecutor = ioExecutor;
 		this.reliabilityFactory = reliabilityFactory;
 		clock = new SystemClock();
 	}
 
 	public Modem createModem(Modem.Callback callback, String portName) {
-		return new ModemImpl(executor, reliabilityFactory, clock, callback,
+		return new ModemImpl(ioExecutor, reliabilityFactory, clock, callback,
 				new SerialPortImpl(portName));
 	}
 }

briar-desktop/src/org/briarproject/plugins/modem/ModemImpl.java

@@ -32,7 +32,7 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener {
 	private static final int CONNECT_TIMEOUT = 2 * 60 * 1000; // Milliseconds
 	private static final int ESCAPE_SEQUENCE_GUARD_TIME = 1000; // Milliseconds
 
-	private final Executor executor;
+	private final Executor ioExecutor;
 	private final ReliabilityLayerFactory reliabilityFactory;
 	private final Clock clock;
 	private final Callback callback;
@@ -45,9 +45,9 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener {
 	private ReliabilityLayer reliability = null; // Locking: this
 	private boolean initialised = false, connected = false; // Locking: this
 
-	ModemImpl(Executor executor, ReliabilityLayerFactory reliabilityFactory,
+	ModemImpl(Executor ioExecutor, ReliabilityLayerFactory reliabilityFactory,
 			Clock clock, Callback callback, SerialPort port) {
-		this.executor = executor;
+		this.ioExecutor = ioExecutor;
 		this.reliabilityFactory = reliabilityFactory;
 		this.clock = clock;
 		this.callback = callback;
@@ -333,7 +333,7 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener {
 						notifyAll();
 					}
 				} else if(s.equals("RING")) {
-					executor.execute(new Runnable() {
+					ioExecutor.execute(new Runnable() {
 						public void run() {
 							try {
 								answer();

briar-desktop/src/org/briarproject/plugins/modem/ModemPlugin.java

@@ -6,14 +6,8 @@ import static java.util.logging.Level.WARNING;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
 import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.logging.Logger;
 
@@ -35,29 +29,24 @@ class ModemPlugin implements DuplexPlugin, Modem.Callback {
 	private static final Logger LOG =
 			Logger.getLogger(ModemPlugin.class.getName());
 
-	private final Executor ioExecutor;
 	private final ModemFactory modemFactory;
 	private final SerialPortList serialPortList;
 	private final DuplexPluginCallback callback;
 	private final int maxFrameLength;
 	private final long maxLatency, pollingInterval;
-	private final boolean shuffle; // Used to disable shuffling for testing
 
 	private volatile boolean running = false;
 	private volatile Modem modem = null;
 
-	ModemPlugin(Executor ioExecutor, ModemFactory modemFactory,
+	ModemPlugin(ModemFactory modemFactory, SerialPortList serialPortList,
-			SerialPortList serialPortList, DuplexPluginCallback callback,
+			DuplexPluginCallback callback, int maxFrameLength, long maxLatency,
-			int maxFrameLength, long maxLatency, long pollingInterval,
+			long pollingInterval) {
-			boolean shuffle) {
-		this.ioExecutor = ioExecutor;
 		this.modemFactory = modemFactory;
 		this.serialPortList = serialPortList;
 		this.callback = callback;
 		this.maxFrameLength = maxFrameLength;
 		this.maxLatency = maxLatency;
 		this.pollingInterval = pollingInterval;
-		this.shuffle = shuffle;
 	}
 
 	public TransportId getId() {
@@ -105,9 +94,8 @@ class ModemPlugin implements DuplexPlugin, Modem.Callback {
 		return running;
 	}
 
-	// FIXME: Don't poll this plugin
 	public boolean shouldPoll() {
-		return true;
+		return false;
 	}
 
 	public long getPollingInterval() {
@@ -115,57 +103,7 @@ class ModemPlugin implements DuplexPlugin, Modem.Callback {
 	}
 
 	public void poll(Collection<ContactId> connected) {
-		if(!connected.isEmpty()) return; // One at a time please
+		throw new UnsupportedOperationException();
-		ioExecutor.execute(new Runnable() {
-			public void run() {
-				poll();
-			}
-		});
-	}
-
-	private void poll() {
-		if(!running) return;
-		// Get the ISO 3166 code for the caller's country
-		String callerIso = callback.getLocalProperties().get("iso3166");
-		if(StringUtils.isNullOrEmpty(callerIso)) return;
-		// Call contacts one at a time in a random order
-		Map<ContactId, TransportProperties> remote =
-				callback.getRemoteProperties();
-		List<ContactId> contacts = new ArrayList<ContactId>(remote.keySet());
-		if(shuffle) Collections.shuffle(contacts);
-		Iterator<ContactId> it = contacts.iterator();
-		while(it.hasNext() && running) {
-			ContactId c = it.next();
-			// Get the ISO 3166 code for the callee's country
-			TransportProperties properties = remote.get(c);
-			if(properties == null) continue;
-			String calleeIso = properties.get("iso3166");
-			if(StringUtils.isNullOrEmpty(calleeIso)) continue;
-			// Get the callee's phone number
-			String number = properties.get("number");
-			if(StringUtils.isNullOrEmpty(number)) continue;
-			// Convert the number into direct dialling form
-			number = CountryCodes.translate(number, callerIso, calleeIso);
-			if(number == null) continue;
-			// Dial the number
-			try {
-				if(!modem.dial(number)) continue;
-			} catch(IOException e) {
-				if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
-				if(resetModem()) continue;
-				break;
-			}
-			LOG.info("Outgoing call connected");
-			ModemTransportConnection conn = new ModemTransportConnection();
-			callback.outgoingConnectionCreated(c, conn);
-			try {
-				conn.waitForDisposal();
-			} catch(InterruptedException e) {
-				LOG.warning("Interrupted while polling");
-				Thread.currentThread().interrupt();
-				break;
-			}
-		}
 	}
 
 	boolean resetModem() {
@@ -257,10 +195,6 @@ class ModemPlugin implements DuplexPlugin, Modem.Callback {
 			disposalFinished.countDown();
 		}
 
-		private void waitForDisposal() throws InterruptedException {
-			disposalFinished.await();
-		}
-
 		private class Reader implements TransportConnectionReader {
 
 			public int getMaxFrameLength() {

briar-desktop/src/org/briarproject/plugins/modem/ModemPluginFactory.java

@@ -15,13 +15,11 @@ public class ModemPluginFactory implements DuplexPluginFactory {
 	private static final long MAX_LATENCY = 60 * 1000; // 1 minute
 	private static final long POLLING_INTERVAL = 60 * 60 * 1000; // 1 hour
 
-	private final Executor ioExecutor;
 	private final ModemFactory modemFactory;
 	private final SerialPortList serialPortList;
 
 	public ModemPluginFactory(Executor ioExecutor,
 			ReliabilityLayerFactory reliabilityFactory) {
-		this.ioExecutor = ioExecutor;
 		modemFactory = new ModemFactoryImpl(ioExecutor, reliabilityFactory);
 		serialPortList = new SerialPortListImpl();
 	}
@@ -34,8 +32,7 @@ public class ModemPluginFactory implements DuplexPluginFactory {
 		// This plugin is not enabled by default
 		String enabled = callback.getConfig().get("enabled");
 		if(StringUtils.isNullOrEmpty(enabled)) return null;
-		return new ModemPlugin(ioExecutor, modemFactory, serialPortList,
+		return new ModemPlugin(modemFactory, serialPortList, callback,
-				callback, MAX_FRAME_LENGTH, MAX_LATENCY, POLLING_INTERVAL,
+				MAX_FRAME_LENGTH, MAX_LATENCY, POLLING_INTERVAL);
-				true);
 	}
 }

briar-tests/src/org/briarproject/plugins/modem/ModemPluginTest.java

@@ -1,33 +1,21 @@
 package org.briarproject.plugins.modem;
 
-import static java.util.concurrent.TimeUnit.SECONDS;
-
 import java.io.IOException;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 
 import org.briarproject.BriarTestCase;
 import org.briarproject.api.ContactId;
 import org.briarproject.api.TransportProperties;
 import org.briarproject.api.plugins.duplex.DuplexPluginCallback;
-import org.briarproject.api.plugins.duplex.DuplexTransportConnection;
-import org.hamcrest.Description;
 import org.jmock.Expectations;
 import org.jmock.Mockery;
-import org.jmock.api.Action;
-import org.jmock.api.Invocation;
 import org.junit.Test;
 
 public class ModemPluginTest extends BriarTestCase {
 
 	private static final String ISO_1336 = "GB";
-	private static final String NUMBER1 = "0123";
+	private static final String NUMBER = "0123456789";
-	private static final String NUMBER2 = "0234";
-	private static final String NUMBER3 = "0345";
 
 	@Test
 	public void testModemCreation() throws Exception {
@@ -35,8 +23,8 @@ public class ModemPluginTest extends BriarTestCase {
 		final ModemFactory modemFactory = context.mock(ModemFactory.class);
 		final SerialPortList serialPortList =
 				context.mock(SerialPortList.class);
-		final ModemPlugin plugin = new ModemPlugin(null, modemFactory,
+		final ModemPlugin plugin = new ModemPlugin(modemFactory,
-				serialPortList, null, 0, 0, 0, true);
+				serialPortList, null, 0, 0, 0);
 		final Modem modem = context.mock(Modem.class);
 		context.checking(new Expectations() {{
 			oneOf(serialPortList).getPortNames();
@@ -69,14 +57,14 @@ public class ModemPluginTest extends BriarTestCase {
 				context.mock(SerialPortList.class);
 		final DuplexPluginCallback callback =
 				context.mock(DuplexPluginCallback.class);
-		final ModemPlugin plugin = new ModemPlugin(null, modemFactory,
+		final ModemPlugin plugin = new ModemPlugin(modemFactory,
-				serialPortList, callback, 0, 0, 0, true);
+				serialPortList, callback, 0, 0, 0);
 		final Modem modem = context.mock(Modem.class);
 		final TransportProperties local = new TransportProperties();
 		local.put("iso3166", ISO_1336);
 		TransportProperties p = new TransportProperties();
 		p.put("iso3166", ISO_1336);
-		p.put("number", NUMBER1);
+		p.put("number", NUMBER);
 		ContactId contactId = new ContactId(234);
 		final Map<ContactId, TransportProperties> remote =
 				Collections.singletonMap(contactId, p);
@@ -93,7 +81,7 @@ public class ModemPluginTest extends BriarTestCase {
 			will(returnValue(local));
 			oneOf(callback).getRemoteProperties();
 			will(returnValue(remote));
-			oneOf(modem).dial(NUMBER1);
+			oneOf(modem).dial(NUMBER);
 			will(returnValue(true));
 		}});
 		assertTrue(plugin.start());
@@ -110,14 +98,14 @@ public class ModemPluginTest extends BriarTestCase {
 				context.mock(SerialPortList.class);
 		final DuplexPluginCallback callback =
 				context.mock(DuplexPluginCallback.class);
-		final ModemPlugin plugin = new ModemPlugin(null, modemFactory,
+		final ModemPlugin plugin = new ModemPlugin(modemFactory,
-				serialPortList, callback, 0, 0, 0, true);
+				serialPortList, callback, 0, 0, 0);
 		final Modem modem = context.mock(Modem.class);
 		final TransportProperties local = new TransportProperties();
 		local.put("iso3166", ISO_1336);
 		TransportProperties p = new TransportProperties();
 		p.put("iso3166", ISO_1336);
-		p.put("number", NUMBER1);
+		p.put("number", NUMBER);
 		ContactId contactId = new ContactId(234);
 		final Map<ContactId, TransportProperties> remote =
 				Collections.singletonMap(contactId, p);
@@ -134,7 +122,7 @@ public class ModemPluginTest extends BriarTestCase {
 			will(returnValue(local));
 			oneOf(callback).getRemoteProperties();
 			will(returnValue(remote));
-			oneOf(modem).dial(NUMBER1);
+			oneOf(modem).dial(NUMBER);
 			will(returnValue(false));
 		}});
 		assertTrue(plugin.start());
@@ -151,14 +139,14 @@ public class ModemPluginTest extends BriarTestCase {
 				context.mock(SerialPortList.class);
 		final DuplexPluginCallback callback =
 				context.mock(DuplexPluginCallback.class);
-		final ModemPlugin plugin = new ModemPlugin(null, modemFactory,
+		final ModemPlugin plugin = new ModemPlugin(modemFactory,
-				serialPortList, callback, 0, 0, 0, true);
+				serialPortList, callback, 0, 0, 0);
 		final Modem modem = context.mock(Modem.class);
 		final TransportProperties local = new TransportProperties();
 		local.put("iso3166", ISO_1336);
 		TransportProperties p = new TransportProperties();
 		p.put("iso3166", ISO_1336);
-		p.put("number", NUMBER1);
+		p.put("number", NUMBER);
 		ContactId contactId = new ContactId(234);
 		final Map<ContactId, TransportProperties> remote =
 				Collections.singletonMap(contactId, p);
@@ -175,7 +163,7 @@ public class ModemPluginTest extends BriarTestCase {
 			will(returnValue(local));
 			oneOf(callback).getRemoteProperties();
 			will(returnValue(remote));
-			oneOf(modem).dial(NUMBER1);
+			oneOf(modem).dial(NUMBER);
 			will(throwException(new IOException()));
 			// resetModem()
 			oneOf(serialPortList).getPortNames();
@@ -190,98 +178,4 @@ public class ModemPluginTest extends BriarTestCase {
 		assertNull(plugin.createConnection(contactId));
 		context.assertIsSatisfied();
 	}
-
-	@Test
-	public void testPolling() throws Exception {
-		final ExecutorService ioExecutor = Executors.newSingleThreadExecutor();
-		Mockery context = new Mockery();
-		final ModemFactory modemFactory = context.mock(ModemFactory.class);
-		final SerialPortList serialPortList =
-				context.mock(SerialPortList.class);
-		final DuplexPluginCallback callback =
-				context.mock(DuplexPluginCallback.class);
-		// Disable shuffling for this test, it confuses jMock
-		final ModemPlugin plugin = new ModemPlugin(ioExecutor, modemFactory,
-				serialPortList, callback, 0, 0, 0, false);
-		final Modem modem = context.mock(Modem.class);
-		final TransportProperties local = new TransportProperties();
-		local.put("iso3166", ISO_1336);
-		TransportProperties p1 = new TransportProperties();
-		p1.put("iso3166", ISO_1336);
-		p1.put("number", NUMBER1);
-		TransportProperties p2 = new TransportProperties();
-		p2.put("iso3166", ISO_1336);
-		p2.put("number", NUMBER2);
-		TransportProperties p3 = new TransportProperties();
-		p3.put("iso3166", ISO_1336);
-		p3.put("number", NUMBER3);
-		ContactId contactId1 = new ContactId(234);
-		ContactId contactId2 = new ContactId(345);
-		ContactId contactId3 = new ContactId(456);
-		final Map<ContactId, TransportProperties> remote =
-				new HashMap<ContactId, TransportProperties>();
-		remote.put(contactId1, p1);
-		remote.put(contactId2, p2);
-		remote.put(contactId3, p3);
-		final DisposeAction disposeAction = new DisposeAction();
-		context.checking(new Expectations() {{
-			// start()
-			oneOf(serialPortList).getPortNames();
-			will(returnValue(new String[] { "foo" }));
-			oneOf(modemFactory).createModem(plugin, "foo");
-			will(returnValue(modem));
-			oneOf(modem).start();
-			will(returnValue(true));
-			// poll()
-			oneOf(callback).getLocalProperties();
-			will(returnValue(local));
-			oneOf(callback).getRemoteProperties();
-			will(returnValue(remote));
-			// First call to dial() throws an exception
-			oneOf(modem).dial(NUMBER1);
-			will(throwException(new IOException()));
-			// resetModem()
-			oneOf(serialPortList).getPortNames();
-			will(returnValue(new String[] { "foo" }));
-			oneOf(modemFactory).createModem(plugin, "foo");
-			will(returnValue(modem));
-			oneOf(modem).start();
-			will(returnValue(true));
-			// Second call to dial() returns true
-			oneOf(modem).dial(NUMBER2);
-			will(returnValue(true));
-			// A connection is passed to the callback - dispose of it
-			oneOf(callback).outgoingConnectionCreated(
-					with(any(ContactId.class)),
-					with(any(DuplexTransportConnection.class)));
-			will(disposeAction);
-			oneOf(modem).hangUp();
-			// Third call to dial() returns false
-			oneOf(modem).dial(NUMBER3);
-			will(returnValue(false));
-		}});
-		assertTrue(plugin.start());
-		plugin.poll(Collections.<ContactId>emptyList());
-		assertTrue(disposeAction.invoked.await(5, SECONDS));
-		ioExecutor.shutdown();
-		context.assertIsSatisfied();
-	}
-
-	private static class DisposeAction implements Action {
-
-		private final CountDownLatch invoked = new CountDownLatch(1);
-
-		public void describeTo(Description description) {
-			description.appendText("Disposes of a transport connection");
-		}
-
-		public Object invoke(Invocation invocation) throws Throwable {
-			DuplexTransportConnection conn =
-					(DuplexTransportConnection) invocation.getParameter(1);
-			conn.getReader().dispose(false, true);
-			conn.getWriter().dispose(false);
-			invoked.countDown();
-			return null;
-		}
-	}
 }