Remove RemovableDrivePlugin, refactor plugin interface.

bramble-core/src/main/java/org/briarproject/bramble/plugin/PluginManagerImpl.java

@@ -32,12 +32,10 @@ import org.briarproject.bramble.api.settings.Settings;
 import org.briarproject.bramble.api.settings.SettingsManager;
 import org.briarproject.bramble.api.system.Clock;
 import org.briarproject.bramble.api.system.Scheduler;
-import org.briarproject.bramble.api.ui.UiCallback;
 
 import java.security.SecureRandom;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -71,7 +69,6 @@ class PluginManagerImpl implements PluginManager, Service {
 	private final TransportPropertyManager transportPropertyManager;
 	private final SecureRandom random;
 	private final Clock clock;
-	private final UiCallback uiCallback;
 	private final Map<TransportId, Plugin> plugins;
 	private final List<SimplexPlugin> simplexPlugins;
 	private final List<DuplexPlugin> duplexPlugins;
@@ -85,8 +82,7 @@ class PluginManagerImpl implements PluginManager, Service {
 			ConnectionRegistry connectionRegistry,
 			SettingsManager settingsManager,
 			TransportPropertyManager transportPropertyManager,
-			SecureRandom random, Clock clock,
-			UiCallback uiCallback) {
+			SecureRandom random, Clock clock) {
 		this.ioExecutor = ioExecutor;
 		this.scheduler = scheduler;
 		this.eventBus = eventBus;
@@ -97,7 +93,6 @@ class PluginManagerImpl implements PluginManager, Service {
 		this.transportPropertyManager = transportPropertyManager;
 		this.random = random;
 		this.clock = clock;
-		this.uiCallback = uiCallback;
 		plugins = new ConcurrentHashMap<>();
 		simplexPlugins = new CopyOnWriteArrayList<>();
 		duplexPlugins = new CopyOnWriteArrayList<>();
@@ -106,13 +101,14 @@ class PluginManagerImpl implements PluginManager, Service {
 	}
 
 	@Override
-	public void startService() throws ServiceException {
+	public void startService() {
 		if (used.getAndSet(true)) throw new IllegalStateException();
 		// Instantiate the poller
 		if (pluginConfig.shouldPoll()) {
 			LOG.info("Starting poller");
 			Poller poller = new Poller(ioExecutor, scheduler, connectionManager,
-					connectionRegistry, this, random, clock);
+					connectionRegistry, this, transportPropertyManager, random,
+					clock);
 			eventBus.addListener(poller);
 		}
 		// Instantiate the simplex plugins and start them asynchronously
@@ -297,26 +293,6 @@ class PluginManagerImpl implements PluginManager, Service {
 			}
 		}
 
-		@Override
-		public Map<ContactId, TransportProperties> getRemoteProperties() {
-			try {
-				return transportPropertyManager.getRemoteProperties(id);
-			} catch (DbException e) {
-				if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
-				return Collections.emptyMap();
-			}
-		}
-
-		@Override
-		public TransportProperties getRemoteProperties(ContactId c) {
-			try {
-				return transportPropertyManager.getRemoteProperties(c, id);
-			} catch (DbException e) {
-				if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
-				return new TransportProperties();
-			}
-		}
-
 		@Override
 		public void mergeSettings(Settings s) {
 			try {
@@ -335,21 +311,6 @@ class PluginManagerImpl implements PluginManager, Service {
 			}
 		}
 
-		@Override
-		public int showChoice(String[] options, String... message) {
-			return uiCallback.showChoice(options, message);
-		}
-
-		@Override
-		public boolean showConfirmationMessage(String... message) {
-			return uiCallback.showConfirmationMessage(message);
-		}
-
-		@Override
-		public void showMessage(String... message) {
-			uiCallback.showMessage(message);
-		}
-
 		@Override
 		public void transportEnabled() {
 			eventBus.broadcast(new TransportEnabledEvent(id));

bramble-core/src/main/java/org/briarproject/bramble/plugin/PluginModule.java

@@ -1,18 +1,10 @@
 package org.briarproject.bramble.plugin;
 
-import org.briarproject.bramble.api.event.EventBus;
-import org.briarproject.bramble.api.lifecycle.IoExecutor;
 import org.briarproject.bramble.api.lifecycle.LifecycleManager;
 import org.briarproject.bramble.api.plugin.BackoffFactory;
 import org.briarproject.bramble.api.plugin.ConnectionManager;
 import org.briarproject.bramble.api.plugin.ConnectionRegistry;
 import org.briarproject.bramble.api.plugin.PluginManager;
-import org.briarproject.bramble.api.system.Clock;
-import org.briarproject.bramble.api.system.Scheduler;
-
-import java.security.SecureRandom;
-import java.util.concurrent.Executor;
-import java.util.concurrent.ScheduledExecutorService;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;

bramble-core/src/main/java/org/briarproject/bramble/plugin/Poller.java

@@ -2,6 +2,7 @@ package org.briarproject.bramble.plugin;
 
 import org.briarproject.bramble.api.contact.ContactId;
 import org.briarproject.bramble.api.contact.event.ContactStatusChangedEvent;
+import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.event.Event;
 import org.briarproject.bramble.api.event.EventListener;
 import org.briarproject.bramble.api.lifecycle.IoExecutor;
@@ -19,10 +20,13 @@ import org.briarproject.bramble.api.plugin.event.ConnectionOpenedEvent;
 import org.briarproject.bramble.api.plugin.event.TransportDisabledEvent;
 import org.briarproject.bramble.api.plugin.event.TransportEnabledEvent;
 import org.briarproject.bramble.api.plugin.simplex.SimplexPlugin;
+import org.briarproject.bramble.api.properties.TransportProperties;
+import org.briarproject.bramble.api.properties.TransportPropertyManager;
 import org.briarproject.bramble.api.system.Clock;
 import org.briarproject.bramble.api.system.Scheduler;
 
 import java.security.SecureRandom;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.Executor;
@@ -33,10 +37,10 @@ import java.util.concurrent.locks.ReentrantLock;
 import java.util.logging.Logger;
 
 import javax.annotation.concurrent.ThreadSafe;
-import javax.inject.Inject;
 
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.logging.Level.INFO;
+import static java.util.logging.Level.WARNING;
 
 @ThreadSafe
 @NotNullByDefault
@@ -49,6 +53,7 @@ class Poller implements EventListener {
 	private final ConnectionManager connectionManager;
 	private final ConnectionRegistry connectionRegistry;
 	private final PluginManager pluginManager;
+	private final TransportPropertyManager transportPropertyManager;
 	private final SecureRandom random;
 	private final Clock clock;
 	private final Lock lock;
@@ -58,12 +63,14 @@ class Poller implements EventListener {
 			@Scheduler ScheduledExecutorService scheduler,
 			ConnectionManager connectionManager,
 			ConnectionRegistry connectionRegistry, PluginManager pluginManager,
+			TransportPropertyManager transportPropertyManager,
 			SecureRandom random, Clock clock) {
 		this.ioExecutor = ioExecutor;
 		this.scheduler = scheduler;
 		this.connectionManager = connectionManager;
 		this.connectionRegistry = connectionRegistry;
 		this.pluginManager = pluginManager;
+		this.transportPropertyManager = transportPropertyManager;
 		this.random = random;
 		this.clock = clock;
 		lock = new ReentrantLock();
@@ -119,10 +126,15 @@ class Poller implements EventListener {
 	private void connectToContact(ContactId c, SimplexPlugin p) {
 		ioExecutor.execute(() -> {
 			TransportId t = p.getId();
-			if (!connectionRegistry.isConnected(c, t)) {
-				TransportConnectionWriter w = p.createWriter(c);
+			if (connectionRegistry.isConnected(c, t)) return;
+			try {
+				TransportProperties props =
+						transportPropertyManager.getRemoteProperties(c, t);
+				TransportConnectionWriter w = p.createWriter(props);
 				if (w != null)
 					connectionManager.manageOutgoingConnection(c, t, w);
+			} catch (DbException e) {
+				if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 			}
 		});
 	}
@@ -130,10 +142,15 @@ class Poller implements EventListener {
 	private void connectToContact(ContactId c, DuplexPlugin p) {
 		ioExecutor.execute(() -> {
 			TransportId t = p.getId();
-			if (!connectionRegistry.isConnected(c, t)) {
-				DuplexTransportConnection d = p.createConnection(c);
+			if (connectionRegistry.isConnected(c, t)) return;
+			try {
+				TransportProperties props =
+						transportPropertyManager.getRemoteProperties(c, t);
+				DuplexTransportConnection d = p.createConnection(props);
 				if (d != null)
 					connectionManager.manageOutgoingConnection(c, t, d);
+			} catch (DbException e) {
+				if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 			}
 		});
 	}
@@ -185,7 +202,17 @@ class Poller implements EventListener {
 	private void poll(Plugin p) {
 		TransportId t = p.getId();
 		if (LOG.isLoggable(INFO)) LOG.info("Polling plugin " + t);
-		p.poll(connectionRegistry.getConnectedContacts(t));
+		try {
+			Map<ContactId, TransportProperties> remote =
+					transportPropertyManager.getRemoteProperties(t);
+			Collection<ContactId> connected =
+					connectionRegistry.getConnectedContacts(t);
+			remote = new HashMap<>(remote);
+			remote.keySet().removeAll(connected);
+			if (!remote.isEmpty()) p.poll(remote);
+		} catch (DbException e) {
+			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+		}
 	}
 
 	private class ScheduledPollTask {

bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java

@@ -26,7 +26,6 @@ import org.briarproject.bramble.util.StringUtils;
 
 import java.io.IOException;
 import java.security.SecureRandom;
-import java.util.Collection;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.UUID;
@@ -250,19 +249,16 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 	}
 
 	@Override
-	public void poll(Collection<ContactId> connected) {
+	public void poll(Map<ContactId, TransportProperties> contacts) {
 		if (!isRunning() || !shouldAllowContactConnections()) return;
 		backoff.increment();
 		// Try to connect to known devices in parallel
-		Map<ContactId, TransportProperties> remote =
-				callback.getRemoteProperties();
-		for (Entry<ContactId, TransportProperties> e : remote.entrySet()) {
-			ContactId c = e.getKey();
-			if (connected.contains(c)) continue;
+		for (Entry<ContactId, TransportProperties> e : contacts.entrySet()) {
 			String address = e.getValue().get(PROP_ADDRESS);
 			if (StringUtils.isNullOrEmpty(address)) continue;
 			String uuid = e.getValue().get(PROP_UUID);
 			if (StringUtils.isNullOrEmpty(uuid)) continue;
+			ContactId c = e.getKey();
 			ioExecutor.execute(() -> {
 				if (!isRunning() || !shouldAllowContactConnections()) return;
 				if (!connectionLimiter.canOpenContactConnection()) return;
@@ -308,10 +304,9 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 	}
 
 	@Override
-	public DuplexTransportConnection createConnection(ContactId c) {
+	public DuplexTransportConnection createConnection(TransportProperties p) {
 		if (!isRunning() || !shouldAllowContactConnections()) return null;
 		if (!connectionLimiter.canOpenContactConnection()) return null;
-		TransportProperties p = callback.getRemoteProperties(c);
 		String address = p.get(PROP_ADDRESS);
 		if (StringUtils.isNullOrEmpty(address)) return null;
 		String uuid = p.get(PROP_UUID);

bramble-core/src/main/java/org/briarproject/bramble/plugin/file/FilePlugin.java

@@ -1,27 +1,21 @@
 package org.briarproject.bramble.plugin.file;
 
-import org.briarproject.bramble.api.contact.ContactId;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.plugin.TransportConnectionReader;
 import org.briarproject.bramble.api.plugin.TransportConnectionWriter;
 import org.briarproject.bramble.api.plugin.simplex.SimplexPlugin;
 import org.briarproject.bramble.api.plugin.simplex.SimplexPluginCallback;
+import org.briarproject.bramble.api.properties.TransportProperties;
 
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
-import java.io.OutputStream;
-import java.util.Collection;
-import java.util.Locale;
-import java.util.concurrent.Executor;
-import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.logging.Logger;
 
-import javax.annotation.Nullable;
-
 import static java.util.logging.Level.WARNING;
-import static org.briarproject.bramble.api.transport.TransportConstants.MIN_STREAM_LENGTH;
+import static org.briarproject.bramble.api.plugin.FileConstants.PROP_PATH;
+import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty;
 
 @NotNullByDefault
 abstract class FilePlugin implements SimplexPlugin {
@@ -29,25 +23,15 @@ abstract class FilePlugin implements SimplexPlugin {
 	private static final Logger LOG =
 			Logger.getLogger(FilePlugin.class.getName());
 
-	protected final Executor ioExecutor;
 	protected final SimplexPluginCallback callback;
 	protected final int maxLatency;
-	protected final AtomicBoolean used = new AtomicBoolean(false);
 
-	protected volatile boolean running = false;
+	protected abstract void writerFinished(File f, boolean exception);
 
-	@Nullable
-	protected abstract File chooseOutputDirectory();
+	protected abstract void readerFinished(File f, boolean exception,
+			boolean recognised);
 
-	protected abstract Collection<File> findFilesByName(String filename);
-
-	protected abstract void writerFinished(File f);
-
-	protected abstract void readerFinished(File f);
-
-	protected FilePlugin(Executor ioExecutor, SimplexPluginCallback callback,
-			int maxLatency) {
-		this.ioExecutor = ioExecutor;
+	FilePlugin(SimplexPluginCallback callback, int maxLatency) {
 		this.callback = callback;
 		this.maxLatency = maxLatency;
 	}
@@ -58,81 +42,36 @@ abstract class FilePlugin implements SimplexPlugin {
 	}
 
 	@Override
-	public int getMaxIdleTime() {
-		return Integer.MAX_VALUE; // We don't need keepalives
-	}
-
-	@Override
-	public boolean isRunning() {
-		return running;
-	}
-
-	@Override
-	public TransportConnectionReader createReader(ContactId c) {
-		return null;
-	}
-
-	@Override
-	public TransportConnectionWriter createWriter(ContactId c) {
-		if (!running) return null;
-		return createWriter(createConnectionFilename());
-	}
-
-	private String createConnectionFilename() {
-		StringBuilder s = new StringBuilder(12);
-		for (int i = 0; i < 8; i++) s.append((char) ('a' + Math.random() * 26));
-		s.append(".dat");
-		return s.toString();
-	}
-
-	// Package access for testing
-	boolean isPossibleConnectionFilename(String filename) {
-		return filename.toLowerCase(Locale.US).matches("[a-z]{8}\\.dat");
-	}
-
-	@Nullable
-	private TransportConnectionWriter createWriter(String filename) {
-		if (!running) return null;
-		File dir = chooseOutputDirectory();
-		if (dir == null || !dir.exists() || !dir.isDirectory()) return null;
-		File f = new File(dir, filename);
+	public TransportConnectionReader createReader(TransportProperties p) {
+		if (!isRunning()) return null;
+		String path = p.get(PROP_PATH);
+		if (isNullOrEmpty(path)) return null;
 		try {
-			long capacity = dir.getFreeSpace();
-			if (capacity < MIN_STREAM_LENGTH) return null;
-			OutputStream out = new FileOutputStream(f);
-			return new FileTransportWriter(f, out, capacity, this);
+			File file = new File(path);
+			FileInputStream in = new FileInputStream(file);
+			return new FileTransportReader(file, in, this);
 		} catch (IOException e) {
 			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
-			f.delete();
 			return null;
 		}
 	}
 
-	protected void createReaderFromFile(File f) {
-		if (!running) return;
-		ioExecutor.execute(new ReaderCreator(f));
-	}
-
-	private class ReaderCreator implements Runnable {
-
-		private final File file;
-
-		private ReaderCreator(File file) {
-			this.file = file;
-		}
-
-		@Override
-		public void run() {
-			if (isPossibleConnectionFilename(file.getName())) {
-				try {
-					FileInputStream in = new FileInputStream(file);
-					callback.readerCreated(new FileTransportReader(file, in,
-							FilePlugin.this));
-				} catch (IOException e) {
-					if (LOG.isLoggable(WARNING))
-						LOG.log(WARNING, e.toString(), e);
-				}
+	@Override
+	public TransportConnectionWriter createWriter(TransportProperties p) {
+		if (!isRunning()) return null;
+		String path = p.get(PROP_PATH);
+		if (isNullOrEmpty(path)) return null;
+		try {
+			File file = new File(path);
+			if (!file.exists() && !file.createNewFile()) {
+				LOG.info("Failed to create file");
+				return null;
 			}
+			FileOutputStream out = new FileOutputStream(file);
+			return new FileTransportWriter(file, out, this);
+		} catch (IOException e) {
+			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+			return null;
 		}
 	}
 }

bramble-core/src/main/java/org/briarproject/bramble/plugin/file/FileTransportReader.java

@@ -38,9 +38,6 @@ class FileTransportReader implements TransportConnectionReader {
 		} catch (IOException e) {
 			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 		}
-		if (recognised) {
-			file.delete();
-			plugin.readerFinished(file);
-		}
+		plugin.readerFinished(file, exception, recognised);
 	}
 }

bramble-core/src/main/java/org/briarproject/bramble/plugin/file/FileTransportWriter.java

@@ -18,14 +18,11 @@ class FileTransportWriter implements TransportConnectionWriter {
 
 	private final File file;
 	private final OutputStream out;
-	private final long capacity;
 	private final FilePlugin plugin;
 
-	FileTransportWriter(File file, OutputStream out, long capacity,
-			FilePlugin plugin) {
+	FileTransportWriter(File file, OutputStream out, FilePlugin plugin) {
 		this.file = file;
 		this.out = out;
-		this.capacity = capacity;
 		this.plugin = plugin;
 	}
 
@@ -39,11 +36,6 @@ class FileTransportWriter implements TransportConnectionWriter {
 		return plugin.getMaxIdleTime();
 	}
 
-	@Override
-	public long getCapacity() {
-		return capacity;
-	}
-
 	@Override
 	public OutputStream getOutputStream() {
 		return out;
@@ -56,7 +48,6 @@ class FileTransportWriter implements TransportConnectionWriter {
 		} catch (IOException e) {
 			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 		}
-		if (exception) file.delete();
-		else plugin.writerFinished(file);
+		plugin.writerFinished(file, exception);
 	}
 }

bramble-core/src/main/java/org/briarproject/bramble/plugin/tcp/TcpPlugin.java

@@ -207,20 +207,16 @@ abstract class TcpPlugin implements DuplexPlugin {
 	}
 
 	@Override
-	public void poll(Collection<ContactId> connected) {
+	public void poll(Map<ContactId, TransportProperties> contacts) {
 		if (!isRunning()) return;
 		backoff.increment();
-		Map<ContactId, TransportProperties> remote =
-				callback.getRemoteProperties();
-		for (Entry<ContactId, TransportProperties> e : remote.entrySet()) {
-			ContactId c = e.getKey();
-			if (!connected.contains(c)) connectAndCallBack(c, e.getValue());
+		for (Entry<ContactId, TransportProperties> e : contacts.entrySet()) {
+			connectAndCallBack(e.getKey(), e.getValue());
 		}
 	}
 
 	private void connectAndCallBack(ContactId c, TransportProperties p) {
 		ioExecutor.execute(() -> {
-			if (!isRunning()) return;
 			DuplexTransportConnection d = createConnection(p);
 			if (d != null) {
 				backoff.reset();
@@ -230,13 +226,8 @@ abstract class TcpPlugin implements DuplexPlugin {
 	}
 
 	@Override
-	public DuplexTransportConnection createConnection(ContactId c) {
+	public DuplexTransportConnection createConnection(TransportProperties p) {
 		if (!isRunning()) return null;
-		return createConnection(callback.getRemoteProperties(c));
-	}
-
-	@Nullable
-	private DuplexTransportConnection createConnection(TransportProperties p) {
 		for (InetSocketAddress remote : getRemoteSocketAddresses(p)) {
 			if (!isConnectable(remote)) {
 				if (LOG.isLoggable(INFO)) {

bramble-core/src/test/java/org/briarproject/bramble/plugin/PluginManagerImplTest.java

@@ -15,7 +15,6 @@ import org.briarproject.bramble.api.plugin.simplex.SimplexPluginFactory;
 import org.briarproject.bramble.api.properties.TransportPropertyManager;
 import org.briarproject.bramble.api.settings.SettingsManager;
 import org.briarproject.bramble.api.system.Clock;
-import org.briarproject.bramble.api.ui.UiCallback;
 import org.briarproject.bramble.test.BrambleTestCase;
 import org.jmock.Expectations;
 import org.jmock.Mockery;
@@ -26,9 +25,7 @@ import java.security.SecureRandom;
 import java.util.Arrays;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
-import java.util.concurrent.RejectedExecutionHandler;
 import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ScheduledThreadPoolExecutor;
 
 import static org.briarproject.bramble.test.TestUtils.getTransportId;
 
@@ -40,19 +37,20 @@ public class PluginManagerImplTest extends BrambleTestCase {
 			setThreadingPolicy(new Synchroniser());
 		}};
 		Executor ioExecutor = Executors.newSingleThreadExecutor();
-		ScheduledExecutorService scheduler = context.mock(ScheduledExecutorService.class);
+		ScheduledExecutorService scheduler =
+				context.mock(ScheduledExecutorService.class);
 		SecureRandom random = new SecureRandom();
 		Clock clock = context.mock(Clock.class);
 		EventBus eventBus = context.mock(EventBus.class);
 		PluginConfig pluginConfig = context.mock(PluginConfig.class);
 		ConnectionManager connectionManager =
 				context.mock(ConnectionManager.class);
-		ConnectionRegistry connectionRegistry = context.mock(ConnectionRegistry.class);
+		ConnectionRegistry connectionRegistry =
+				context.mock(ConnectionRegistry.class);
 		SettingsManager settingsManager =
 				context.mock(SettingsManager.class);
 		TransportPropertyManager transportPropertyManager =
 				context.mock(TransportPropertyManager.class);
-		UiCallback uiCallback = context.mock(UiCallback.class);
 
 		// Two simplex plugin factories: both create plugins, one fails to start
 		SimplexPluginFactory simplexFactory =
@@ -124,9 +122,9 @@ public class PluginManagerImplTest extends BrambleTestCase {
 			oneOf(duplexPlugin).stop();
 		}});
 
-		PluginManagerImpl p = new PluginManagerImpl(ioExecutor, scheduler, eventBus,
-				pluginConfig, connectionManager, connectionRegistry, settingsManager,
-				transportPropertyManager, random, clock, uiCallback);
+		PluginManagerImpl p = new PluginManagerImpl(ioExecutor, scheduler,
+				eventBus, pluginConfig, connectionManager, connectionRegistry,
+				settingsManager, transportPropertyManager, random, clock);
 
 		// Two plugins should be started and stopped
 		p.startService();

bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerTest.java

@@ -15,6 +15,8 @@ import org.briarproject.bramble.api.plugin.event.ConnectionOpenedEvent;
 import org.briarproject.bramble.api.plugin.event.TransportDisabledEvent;
 import org.briarproject.bramble.api.plugin.event.TransportEnabledEvent;
 import org.briarproject.bramble.api.plugin.simplex.SimplexPlugin;
+import org.briarproject.bramble.api.properties.TransportProperties;
+import org.briarproject.bramble.api.properties.TransportPropertyManager;
 import org.briarproject.bramble.api.system.Clock;
 import org.briarproject.bramble.test.BrambleMockTestCase;
 import org.briarproject.bramble.test.ImmediateExecutor;
@@ -24,13 +26,15 @@ import org.jmock.lib.legacy.ClassImposteriser;
 import org.junit.Test;
 
 import java.security.SecureRandom;
-import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 
+import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
+import static java.util.Collections.singletonMap;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.briarproject.bramble.test.TestUtils.getTransportId;
 
@@ -44,6 +48,8 @@ public class PollerTest extends BrambleMockTestCase {
 			context.mock(ConnectionRegistry.class);
 	private final PluginManager pluginManager =
 			context.mock(PluginManager.class);
+	private final TransportPropertyManager transportPropertyManager =
+			context.mock(TransportPropertyManager.class);
 	private final Clock clock = context.mock(Clock.class);
 	private final ScheduledFuture future = context.mock(ScheduledFuture.class);
 	private final SecureRandom random;
@@ -51,6 +57,7 @@ public class PollerTest extends BrambleMockTestCase {
 	private final Executor ioExecutor = new ImmediateExecutor();
 	private final TransportId transportId = getTransportId();
 	private final ContactId contactId = new ContactId(234);
+	private final TransportProperties properties = new TransportProperties();
 	private final int pollingInterval = 60 * 1000;
 	private final long now = System.currentTimeMillis();
 
@@ -66,8 +73,8 @@ public class PollerTest extends BrambleMockTestCase {
 		SimplexPlugin simplexPlugin1 =
 				context.mock(SimplexPlugin.class, "simplexPlugin1");
 		TransportId simplexId1 = getTransportId();
-		List<SimplexPlugin> simplexPlugins = Arrays.asList(simplexPlugin,
-				simplexPlugin1);
+		List<SimplexPlugin> simplexPlugins =
+				asList(simplexPlugin, simplexPlugin1);
 		TransportConnectionWriter simplexWriter =
 				context.mock(TransportConnectionWriter.class);
 
@@ -76,8 +83,8 @@ public class PollerTest extends BrambleMockTestCase {
 		TransportId duplexId = getTransportId();
 		DuplexPlugin duplexPlugin1 =
 				context.mock(DuplexPlugin.class, "duplexPlugin1");
-		List<DuplexPlugin> duplexPlugins = Arrays.asList(duplexPlugin,
-				duplexPlugin1);
+		List<DuplexPlugin> duplexPlugins =
+				asList(duplexPlugin, duplexPlugin1);
 		DuplexTransportConnection duplexConnection =
 				context.mock(DuplexTransportConnection.class);
 
@@ -96,8 +103,12 @@ public class PollerTest extends BrambleMockTestCase {
 			will(returnValue(simplexId1));
 			oneOf(connectionRegistry).isConnected(contactId, simplexId1);
 			will(returnValue(false));
+			// Get the transport properties
+			oneOf(transportPropertyManager).getRemoteProperties(contactId,
+					simplexId1);
+			will(returnValue(properties));
 			// Connect to the contact
-			oneOf(simplexPlugin1).createWriter(contactId);
+			oneOf(simplexPlugin1).createWriter(properties);
 			will(returnValue(simplexWriter));
 			// Pass the connection to the connection manager
 			oneOf(connectionManager).manageOutgoingConnection(contactId,
@@ -105,7 +116,7 @@ public class PollerTest extends BrambleMockTestCase {
 			// Get the duplex plugins
 			oneOf(pluginManager).getDuplexPlugins();
 			will(returnValue(duplexPlugins));
-			// The first plugin supports polling
+			// The duplex plugin supports polling
 			oneOf(duplexPlugin).shouldPoll();
 			will(returnValue(true));
 			// Check whether the contact is already connected
@@ -113,8 +124,12 @@ public class PollerTest extends BrambleMockTestCase {
 			will(returnValue(duplexId));
 			oneOf(connectionRegistry).isConnected(contactId, duplexId);
 			will(returnValue(false));
+			// Get the transport properties
+			oneOf(transportPropertyManager).getRemoteProperties(contactId,
+					duplexId);
+			will(returnValue(properties));
 			// Connect to the contact
-			oneOf(duplexPlugin).createConnection(contactId);
+			oneOf(duplexPlugin).createConnection(properties);
 			will(returnValue(duplexConnection));
 			// Pass the connection to the connection manager
 			oneOf(connectionManager).manageOutgoingConnection(contactId,
@@ -125,7 +140,8 @@ public class PollerTest extends BrambleMockTestCase {
 		}});
 
 		Poller p = new Poller(ioExecutor, scheduler, connectionManager,
-				connectionRegistry, pluginManager, random, clock);
+				connectionRegistry, pluginManager, transportPropertyManager,
+				random, clock);
 
 		p.eventOccurred(new ContactStatusChangedEvent(contactId, true));
 	}
@@ -165,8 +181,12 @@ public class PollerTest extends BrambleMockTestCase {
 			// Check whether the contact is already connected
 			oneOf(connectionRegistry).isConnected(contactId, transportId);
 			will(returnValue(false));
+			// Get the transport properties
+			oneOf(transportPropertyManager).getRemoteProperties(contactId,
+					transportId);
+			will(returnValue(properties));
 			// Connect to the contact
-			oneOf(plugin).createConnection(contactId);
+			oneOf(plugin).createConnection(properties);
 			will(returnValue(duplexConnection));
 			// Pass the connection to the connection manager
 			oneOf(connectionManager).manageOutgoingConnection(contactId,
@@ -174,15 +194,15 @@ public class PollerTest extends BrambleMockTestCase {
 		}});
 
 		Poller p = new Poller(ioExecutor, scheduler, connectionManager,
-				connectionRegistry, pluginManager, random, clock);
+				connectionRegistry, pluginManager, transportPropertyManager,
+				random, clock);
 
 		p.eventOccurred(new ConnectionClosedEvent(contactId, transportId,
 				false));
 	}
 
-
 	@Test
-	public void testRescheduleOnConnectionOpened() throws Exception {
+	public void testRescheduleOnConnectionOpened() {
 		Plugin plugin = context.mock(Plugin.class);
 
 		context.checking(new Expectations() {{
@@ -205,14 +225,15 @@ public class PollerTest extends BrambleMockTestCase {
 		}});
 
 		Poller p = new Poller(ioExecutor, scheduler, connectionManager,
-				connectionRegistry, pluginManager, random, clock);
+				connectionRegistry, pluginManager, transportPropertyManager,
+				random, clock);
 
 		p.eventOccurred(new ConnectionOpenedEvent(contactId, transportId,
 				false));
 	}
 
 	@Test
-	public void testRescheduleDoesNotReplaceEarlierTask() throws Exception {
+	public void testRescheduleDoesNotReplaceEarlierTask() {
 		Plugin plugin = context.mock(Plugin.class);
 
 		context.checking(new Expectations() {{
@@ -248,7 +269,8 @@ public class PollerTest extends BrambleMockTestCase {
 		}});
 
 		Poller p = new Poller(ioExecutor, scheduler, connectionManager,
-				connectionRegistry, pluginManager, random, clock);
+				connectionRegistry, pluginManager, transportPropertyManager,
+				random, clock);
 
 		p.eventOccurred(new ConnectionOpenedEvent(contactId, transportId,
 				false));
@@ -257,7 +279,7 @@ public class PollerTest extends BrambleMockTestCase {
 	}
 
 	@Test
-	public void testRescheduleReplacesLaterTask() throws Exception {
+	public void testRescheduleReplacesLaterTask() {
 		Plugin plugin = context.mock(Plugin.class);
 
 		context.checking(new Expectations() {{
@@ -296,7 +318,8 @@ public class PollerTest extends BrambleMockTestCase {
 		}});
 
 		Poller p = new Poller(ioExecutor, scheduler, connectionManager,
-				connectionRegistry, pluginManager, random, clock);
+				connectionRegistry, pluginManager, transportPropertyManager,
+				random, clock);
 
 		p.eventOccurred(new ConnectionOpenedEvent(contactId, transportId,
 				false));
@@ -306,8 +329,7 @@ public class PollerTest extends BrambleMockTestCase {
 
 	@Test
 	public void testPollsOnTransportEnabled() throws Exception {
-		Plugin plugin = context.mock(Plugin.class);
-		List<ContactId> connected = Collections.singletonList(contactId);
+		DuplexPlugin plugin = context.mock(DuplexPlugin.class);
 
 		context.checking(new Expectations() {{
 			allowing(plugin).getId();
@@ -335,20 +357,69 @@ public class PollerTest extends BrambleMockTestCase {
 			oneOf(scheduler).schedule(with(any(Runnable.class)),
 					with((long) (pollingInterval * 0.5)), with(MILLISECONDS));
 			will(returnValue(future));
-			// Poll the plugin
+			// Get the transport properties and connected contacts
+			oneOf(transportPropertyManager).getRemoteProperties(transportId);
+			will(returnValue(singletonMap(contactId, properties)));
 			oneOf(connectionRegistry).getConnectedContacts(transportId);
-			will(returnValue(connected));
-			oneOf(plugin).poll(connected);
+			will(returnValue(emptyList()));
+			// Poll the plugin
+			oneOf(plugin).poll(singletonMap(contactId, properties));
 		}});
 
 		Poller p = new Poller(ioExecutor, scheduler, connectionManager,
-				connectionRegistry, pluginManager, random, clock);
+				connectionRegistry, pluginManager, transportPropertyManager,
+				random, clock);
 
 		p.eventOccurred(new TransportEnabledEvent(transportId));
 	}
 
 	@Test
-	public void testCancelsPollingOnTransportDisabled() throws Exception {
+	public void testDoesNotPollIfAllContactsAreConnected() throws Exception {
+		DuplexPlugin plugin = context.mock(DuplexPlugin.class);
+
+		context.checking(new Expectations() {{
+			allowing(plugin).getId();
+			will(returnValue(transportId));
+			// Get the plugin
+			oneOf(pluginManager).getPlugin(transportId);
+			will(returnValue(plugin));
+			// The plugin supports polling
+			oneOf(plugin).shouldPoll();
+			will(returnValue(true));
+			// Schedule a polling task immediately
+			oneOf(clock).currentTimeMillis();
+			will(returnValue(now));
+			oneOf(scheduler).schedule(with(any(Runnable.class)), with(0L),
+					with(MILLISECONDS));
+			will(returnValue(future));
+			will(new RunAction());
+			// Running the polling task schedules the next polling task
+			oneOf(plugin).getPollingInterval();
+			will(returnValue(pollingInterval));
+			oneOf(random).nextDouble();
+			will(returnValue(0.5));
+			oneOf(clock).currentTimeMillis();
+			will(returnValue(now));
+			oneOf(scheduler).schedule(with(any(Runnable.class)),
+					with((long) (pollingInterval * 0.5)), with(MILLISECONDS));
+			will(returnValue(future));
+			// Get the transport properties and connected contacts
+			oneOf(transportPropertyManager).getRemoteProperties(transportId);
+			will(returnValue(singletonMap(contactId, properties)));
+			oneOf(connectionRegistry).getConnectedContacts(transportId);
+			will(returnValue(singletonList(contactId)));
+			// All contacts are connected, so don't poll the plugin
+		}});
+
+		Poller p = new Poller(ioExecutor, scheduler, connectionManager,
+				connectionRegistry, pluginManager, transportPropertyManager,
+				random, clock);
+
+		p.eventOccurred(new TransportEnabledEvent(transportId));
+	}
+
+	@Test
+	public void testCancelsPollingOnTransportDisabled() {
 		Plugin plugin = context.mock(Plugin.class);
 
 		context.checking(new Expectations() {{
@@ -371,7 +442,8 @@ public class PollerTest extends BrambleMockTestCase {
 		}});
 
 		Poller p = new Poller(ioExecutor, scheduler, connectionManager,
-				connectionRegistry, pluginManager, random, clock);
+				connectionRegistry, pluginManager, transportPropertyManager,
+				random, clock);
 
 		p.eventOccurred(new TransportEnabledEvent(transportId));
 		p.eventOccurred(new TransportDisabledEvent(transportId));

bramble-core/src/test/java/org/briarproject/bramble/plugin/tcp/LanTcpPluginTest.java

@@ -23,8 +23,6 @@ import java.net.ServerSocket;
 import java.net.Socket;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.Hashtable;
-import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
@@ -40,7 +38,6 @@ import static org.junit.Assert.assertTrue;
 
 public class LanTcpPluginTest extends BrambleTestCase {
 
-	private final ContactId contactId = new ContactId(234);
 	private final Backoff backoff = new TestBackoff();
 
 	@Test
@@ -160,12 +157,10 @@ public class LanTcpPluginTest extends BrambleTestCase {
 				error.set(true);
 			}
 		}).start();
-		// Tell the plugin about the port
+		// Connect to the port
 		TransportProperties p = new TransportProperties();
 		p.put("ipPorts", addrString + ":" + port);
-		callback.remote.put(contactId, p);
-		// Connect to the port
-		DuplexTransportConnection d = plugin.createConnection(contactId);
+		DuplexTransportConnection d = plugin.createConnection(p);
 		assertNotNull(d);
 		// Check that the connection was accepted
 		assertTrue(latch.await(5, SECONDS));
@@ -281,7 +276,7 @@ public class LanTcpPluginTest extends BrambleTestCase {
 	}
 
 	@Test
-	public void testComparatorPrefersNonZeroPorts() throws Exception {
+	public void testComparatorPrefersNonZeroPorts() {
 		Comparator<InetSocketAddress> comparator = new LanAddressComparator();
 		InetSocketAddress nonZero = new InetSocketAddress("1.2.3.4", 1234);
 		InetSocketAddress zero = new InetSocketAddress("1.2.3.4", 0);
@@ -294,7 +289,7 @@ public class LanTcpPluginTest extends BrambleTestCase {
 	}
 
 	@Test
-	public void testComparatorPrefersLongerPrefixes() throws Exception {
+	public void testComparatorPrefersLongerPrefixes() {
 		Comparator<InetSocketAddress> comparator = new LanAddressComparator();
 		InetSocketAddress prefix192 = new InetSocketAddress("192.168.0.1", 0);
 		InetSocketAddress prefix172 = new InetSocketAddress("172.16.0.1", 0);
@@ -314,7 +309,7 @@ public class LanTcpPluginTest extends BrambleTestCase {
 	}
 
 	@Test
-	public void testComparatorPrefersSiteLocalToLinkLocal() throws Exception {
+	public void testComparatorPrefersSiteLocalToLinkLocal() {
 		Comparator<InetSocketAddress> comparator = new LanAddressComparator();
 		InetSocketAddress prefix192 = new InetSocketAddress("192.168.0.1", 0);
 		InetSocketAddress prefix172 = new InetSocketAddress("172.16.0.1", 0);
@@ -345,8 +340,6 @@ public class LanTcpPluginTest extends BrambleTestCase {
 	@NotNullByDefault
 	private static class Callback implements DuplexPluginCallback {
 
-		private final Map<ContactId, TransportProperties> remote =
-				new Hashtable<>();
 		private final CountDownLatch propertiesLatch = new CountDownLatch(1);
 		private final CountDownLatch connectionsLatch = new CountDownLatch(1);
 		private final TransportProperties local = new TransportProperties();
@@ -361,16 +354,6 @@ public class LanTcpPluginTest extends BrambleTestCase {
 			return local;
 		}
 
-		@Override
-		public Map<ContactId, TransportProperties> getRemoteProperties() {
-			return remote;
-		}
-
-		@Override
-		public TransportProperties getRemoteProperties(ContactId c) {
-			return remote.get(c);
-		}
-
 		@Override
 		public void mergeSettings(Settings s) {
 		}
@@ -381,20 +364,6 @@ public class LanTcpPluginTest extends BrambleTestCase {
 			propertiesLatch.countDown();
 		}
 
-		@Override
-		public int showChoice(String[] options, String... message) {
-			return -1;
-		}
-
-		@Override
-		public boolean showConfirmationMessage(String... message) {
-			return false;
-		}
-
-		@Override
-		public void showMessage(String... message) {
-		}
-
 		@Override
 		public void incomingConnectionCreated(DuplexTransportConnection d) {
 			connectionsLatch.countDown();

bramble-core/src/test/java/org/briarproject/bramble/test/CaptureArgumentAction.java

@@ -20,7 +20,7 @@ public class CaptureArgumentAction<T> implements Action {
 	}
 
 	@Override
-	public Object invoke(Invocation invocation) throws Throwable {
+	public Object invoke(Invocation invocation) {
 		captured.set(capturedClass.cast(invocation.getParameter(index)));
 		return null;
 	}

bramble-core/src/test/java/org/briarproject/bramble/test/TestPluginConfigModule.java

@@ -9,13 +9,14 @@ import org.briarproject.bramble.api.plugin.simplex.SimplexPluginCallback;
 import org.briarproject.bramble.api.plugin.simplex.SimplexPluginFactory;
 
 import java.util.Collection;
-import java.util.Collections;
 
 import javax.annotation.Nullable;
 
 import dagger.Module;
 import dagger.Provides;
 
+import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
 import static org.briarproject.bramble.test.TestUtils.getTransportId;
 
 @Module
@@ -51,12 +52,12 @@ public class TestPluginConfigModule {
 
 			@Override
 			public Collection<DuplexPluginFactory> getDuplexFactories() {
-				return Collections.emptyList();
+				return emptyList();
 			}
 
 			@Override
 			public Collection<SimplexPluginFactory> getSimplexFactories() {
-				return Collections.singletonList(simplex);
+				return singletonList(simplex);
 			}
 
 			@Override