Merge branch '1091-reduce-polling-queries' into 'master'

Reduce number of DB queries used when polling for connections

Closes #1091

See merge request !604

bramble-android/src/main/java/org/briarproject/bramble/plugin/droidtooth/DroidtoothPlugin.java

@@ -363,8 +363,7 @@ class DroidtoothPlugin implements DuplexPlugin, EventListener {
 	@Override
 	public DuplexTransportConnection createConnection(ContactId c) {
 		if (!isRunning()) return null;
-		TransportProperties p = callback.getRemoteProperties().get(c);
-		if (p == null) 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-android/src/main/java/org/briarproject/bramble/plugin/tor/TorPlugin.java

@@ -55,6 +55,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Scanner;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executor;
@@ -538,16 +539,21 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 	public void poll(Collection<ContactId> connected) {
 		if (!isRunning()) return;
 		backoff.increment();
-		// TODO: Pass properties to connectAndCallBack()
-		for (ContactId c : callback.getRemoteProperties().keySet())
-			if (!connected.contains(c)) connectAndCallBack(c);
+		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());
+		}
 	}
 
-	private void connectAndCallBack(final ContactId c) {
+	private void connectAndCallBack(final ContactId c,
+			final TransportProperties p) {
 		ioExecutor.execute(new Runnable() {
 			@Override
 			public void run() {
-				DuplexTransportConnection d = createConnection(c);
+				if (!isRunning()) return;
+				DuplexTransportConnection d = createConnection(p);
 				if (d != null) {
 					backoff.reset();
 					callback.outgoingConnectionCreated(c, d);
@@ -559,8 +565,11 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 	@Override
 	public DuplexTransportConnection createConnection(ContactId c) {
 		if (!isRunning()) return null;
-		TransportProperties p = callback.getRemoteProperties().get(c);
-		if (p == null) return null;
+		return createConnection(callback.getRemoteProperties(c));
+	}
+
+	@Nullable
+	private DuplexTransportConnection createConnection(TransportProperties p) {
 		String onion = p.get(PROP_ONION);
 		if (StringUtils.isNullOrEmpty(onion)) return null;
 		if (!ONION.matcher(onion).matches()) {

bramble-api/src/main/java/org/briarproject/bramble/api/plugin/PluginCallback.java

@@ -29,6 +29,11 @@ public interface PluginCallback {
 	 */
 	Map<ContactId, TransportProperties> getRemoteProperties();
 
+	/**
+	 * Returns the plugin's remote transport properties for the given contact.
+	 */
+	TransportProperties getRemoteProperties(ContactId c);
+
 	/**
 	 * Merges the given settings with the namespaced settings
 	 */

bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyManager.java

@@ -49,6 +49,13 @@ public interface TransportPropertyManager {
 	Map<ContactId, TransportProperties> getRemoteProperties(TransportId t)
 			throws DbException;
 
+	/**
+	 * Returns the remote transport properties for the given contact and
+	 * transport.
+	 */
+	TransportProperties getRemoteProperties(ContactId c, TransportId t)
+			throws DbException;
+
 	/**
 	 * Merges the given properties with the existing local properties for the
 	 * given transport.

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

@@ -283,6 +283,16 @@ class PluginManagerImpl implements PluginManager, Service {
 			}
 		}
 
+		@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 {

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

 package org.briarproject.bramble.plugin.tcp;
 
 import org.briarproject.bramble.api.FormatException;
-import org.briarproject.bramble.api.contact.ContactId;
 import org.briarproject.bramble.api.data.BdfList;
 import org.briarproject.bramble.api.keyagreement.KeyAgreementConnection;
 import org.briarproject.bramble.api.keyagreement.KeyAgreementListener;
@@ -126,9 +125,8 @@ class LanTcpPlugin extends TcpPlugin {
 	}
 
 	@Override
-	protected List<InetSocketAddress> getRemoteSocketAddresses(ContactId c) {
-		TransportProperties p = callback.getRemoteProperties().get(c);
-		if (p == null) return Collections.emptyList();
+	protected List<InetSocketAddress> getRemoteSocketAddresses(
+			TransportProperties p) {
 		return parseSocketAddresses(p.get(PROP_IP_PORTS));
 	}
 

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

@@ -9,6 +9,7 @@ import org.briarproject.bramble.api.plugin.Backoff;
 import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin;
 import org.briarproject.bramble.api.plugin.duplex.DuplexPluginCallback;
 import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection;
+import org.briarproject.bramble.api.properties.TransportProperties;
 import org.briarproject.bramble.util.StringUtils;
 
 import java.io.IOException;
@@ -24,6 +25,8 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
 import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.logging.Logger;
@@ -66,11 +69,11 @@ abstract class TcpPlugin implements DuplexPlugin {
 	protected abstract void setLocalSocketAddress(InetSocketAddress a);
 
 	/**
-	 * Returns zero or more socket addresses for connecting to the given
-	 * contact.
+	 * Returns zero or more socket addresses for connecting to a contact with
+	 * the given transport properties.
 	 */
 	protected abstract List<InetSocketAddress> getRemoteSocketAddresses(
-			ContactId c);
+			TransportProperties p);
 
 	/**
 	 * Returns true if connections to the given address can be attempted.
@@ -207,16 +210,21 @@ abstract class TcpPlugin implements DuplexPlugin {
 	public void poll(Collection<ContactId> connected) {
 		if (!isRunning()) return;
 		backoff.increment();
-		// TODO: Pass properties to connectAndCallBack()
-		for (ContactId c : callback.getRemoteProperties().keySet())
-			if (!connected.contains(c)) connectAndCallBack(c);
+		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());
+		}
 	}
 
-	private void connectAndCallBack(final ContactId c) {
+	private void connectAndCallBack(final ContactId c,
+			final TransportProperties p) {
 		ioExecutor.execute(new Runnable() {
 			@Override
 			public void run() {
-				DuplexTransportConnection d = createConnection(c);
+				if (!isRunning()) return;
+				DuplexTransportConnection d = createConnection(p);
 				if (d != null) {
 					backoff.reset();
 					callback.outgoingConnectionCreated(c, d);
@@ -228,7 +236,12 @@ abstract class TcpPlugin implements DuplexPlugin {
 	@Override
 	public DuplexTransportConnection createConnection(ContactId c) {
 		if (!isRunning()) return null;
-		for (InetSocketAddress remote : getRemoteSocketAddresses(c)) {
+		return createConnection(callback.getRemoteProperties(c));
+	}
+
+	@Nullable
+	private DuplexTransportConnection createConnection(TransportProperties p) {
+		for (InetSocketAddress remote : getRemoteSocketAddresses(p)) {
 			if (!isConnectable(remote)) {
 				if (LOG.isLoggable(INFO)) {
 					SocketAddress local = socket.getLocalSocketAddress();

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

 package org.briarproject.bramble.plugin.tcp;
 
-import org.briarproject.bramble.api.contact.ContactId;
 import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
 import org.briarproject.bramble.api.plugin.Backoff;
@@ -78,9 +77,8 @@ class WanTcpPlugin extends TcpPlugin {
 	}
 
 	@Override
-	protected List<InetSocketAddress> getRemoteSocketAddresses(ContactId c) {
-		TransportProperties p = callback.getRemoteProperties().get(c);
-		if (p == null) return Collections.emptyList();
+	protected List<InetSocketAddress> getRemoteSocketAddresses(
+			TransportProperties p) {
 		InetSocketAddress parsed = parseSocketAddress(p.get(PROP_IP_PORT));
 		if (parsed == null) return Collections.emptyList();
 		return Collections.singletonList(parsed);

bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java

@@ -160,35 +160,52 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 	@Override
 	public Map<ContactId, TransportProperties> getRemoteProperties(
 			TransportId t) throws DbException {
+		Map<ContactId, TransportProperties> remote =
+				new HashMap<ContactId, TransportProperties>();
+		Transaction txn = db.startTransaction(true);
 		try {
-			Map<ContactId, TransportProperties> remote =
-					new HashMap<ContactId, TransportProperties>();
-			Transaction txn = db.startTransaction(true);
-			try {
-				for (Contact c : db.getContacts(txn)) {
-					// Don't return properties for inactive contacts
-					if (!c.isActive()) continue;
-					Group g = getContactGroup(c);
-					// Find the latest remote update
-					LatestUpdate latest = findLatest(txn, g.getId(), t, false);
-					if (latest != null) {
-						// Retrieve and parse the latest remote properties
-						BdfList message = clientHelper.getMessageAsList(txn,
-								latest.messageId);
-						if (message == null) throw new DbException();
-						remote.put(c.getId(), parseProperties(message));
-					}
-				}
-				db.commitTransaction(txn);
-			} finally {
-				db.endTransaction(txn);
-			}
-			return remote;
+			for (Contact c : db.getContacts(txn))
+				remote.put(c.getId(), getRemoteProperties(txn, c, t));
+			db.commitTransaction(txn);
+		} finally {
+			db.endTransaction(txn);
+		}
+		return remote;
+	}
+
+	private TransportProperties getRemoteProperties(Transaction txn, Contact c,
+			TransportId t) throws DbException {
+		// Don't return properties for inactive contacts
+		if (!c.isActive()) return new TransportProperties();
+		Group g = getContactGroup(c);
+		try {
+			// Find the latest remote update
+			LatestUpdate latest = findLatest(txn, g.getId(), t, false);
+			if (latest == null) return new TransportProperties();
+			// Retrieve and parse the latest remote properties
+			BdfList message =
+					clientHelper.getMessageAsList(txn, latest.messageId);
+			if (message == null) throw new DbException();
+			return parseProperties(message);
 		} catch (FormatException e) {
 			throw new DbException(e);
 		}
 	}
 
+	@Override
+	public TransportProperties getRemoteProperties(ContactId c, TransportId t)
+			throws DbException {
+		TransportProperties p;
+		Transaction txn = db.startTransaction(true);
+		try {
+			p = getRemoteProperties(txn, db.getContact(txn, c), t);
+			db.commitTransaction(txn);
+		} finally {
+			db.endTransaction(txn);
+		}
+		return p;
+	}
+
 	@Override
 	public void mergeLocalProperties(TransportId t, TransportProperties p)
 			throws DbException {

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

@@ -311,6 +311,11 @@ public class LanTcpPluginTest extends BrambleTestCase {
 			return remote;
 		}
 
+		@Override
+		public TransportProperties getRemoteProperties(ContactId c) {
+			return remote.get(c);
+		}
+
 		@Override
 		public void mergeSettings(Settings s) {
 		}

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

@@ -249,8 +249,7 @@ class BluetoothPlugin implements DuplexPlugin {
 	@Override
 	public DuplexTransportConnection createConnection(ContactId c) {
 		if (!running) return null;
-		TransportProperties p = callback.getRemoteProperties().get(c);
-		if (p == null) 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-j2se/src/main/java/org/briarproject/bramble/plugin/modem/ModemPlugin.java

@@ -145,8 +145,7 @@ class ModemPlugin implements DuplexPlugin, Modem.Callback {
 		String fromIso = callback.getLocalProperties().get("iso3166");
 		if (StringUtils.isNullOrEmpty(fromIso)) return null;
 		// Get the ISO 3166 code for the callee's country
-		TransportProperties properties = callback.getRemoteProperties().get(c);
-		if (properties == null) return null;
+		TransportProperties properties = callback.getRemoteProperties(c);
 		String toIso = properties.get("iso3166");
 		if (StringUtils.isNullOrEmpty(toIso)) return null;
 		// Get the callee's phone number

bramble-j2se/src/test/java/org/briarproject/bramble/plugin/modem/ModemPluginTest.java

@@ -9,8 +9,6 @@ import org.jmock.Mockery;
 import org.junit.Test;
 
 import java.io.IOException;
-import java.util.Collections;
-import java.util.Map;
 
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -65,12 +63,10 @@ public class ModemPluginTest extends BrambleTestCase {
 		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", NUMBER);
-		ContactId contactId = new ContactId(234);
-		final Map<ContactId, TransportProperties> remote =
-				Collections.singletonMap(contactId, p);
+		final TransportProperties remote = new TransportProperties();
+		remote.put("iso3166", ISO_1336);
+		remote.put("number", NUMBER);
+		final ContactId contactId = new ContactId(234);
 		context.checking(new Expectations() {{
 			// start()
 			oneOf(serialPortList).getPortNames();
@@ -82,7 +78,7 @@ public class ModemPluginTest extends BrambleTestCase {
 			// createConnection()
 			oneOf(callback).getLocalProperties();
 			will(returnValue(local));
-			oneOf(callback).getRemoteProperties();
+			oneOf(callback).getRemoteProperties(contactId);
 			will(returnValue(remote));
 			oneOf(modem).dial(NUMBER);
 			will(returnValue(true));
@@ -106,12 +102,10 @@ public class ModemPluginTest extends BrambleTestCase {
 		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", NUMBER);
-		ContactId contactId = new ContactId(234);
-		final Map<ContactId, TransportProperties> remote =
-				Collections.singletonMap(contactId, p);
+		final TransportProperties remote = new TransportProperties();
+		remote.put("iso3166", ISO_1336);
+		remote.put("number", NUMBER);
+		final ContactId contactId = new ContactId(234);
 		context.checking(new Expectations() {{
 			// start()
 			oneOf(serialPortList).getPortNames();
@@ -123,7 +117,7 @@ public class ModemPluginTest extends BrambleTestCase {
 			// createConnection()
 			oneOf(callback).getLocalProperties();
 			will(returnValue(local));
-			oneOf(callback).getRemoteProperties();
+			oneOf(callback).getRemoteProperties(contactId);
 			will(returnValue(remote));
 			oneOf(modem).dial(NUMBER);
 			will(returnValue(false));
@@ -147,12 +141,10 @@ public class ModemPluginTest extends BrambleTestCase {
 		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", NUMBER);
-		ContactId contactId = new ContactId(234);
-		final Map<ContactId, TransportProperties> remote =
-				Collections.singletonMap(contactId, p);
+		final TransportProperties remote = new TransportProperties();
+		remote.put("iso3166", ISO_1336);
+		remote.put("number", NUMBER);
+		final ContactId contactId = new ContactId(234);
 		context.checking(new Expectations() {{
 			// start()
 			oneOf(serialPortList).getPortNames();
@@ -164,7 +156,7 @@ public class ModemPluginTest extends BrambleTestCase {
 			// createConnection()
 			oneOf(callback).getLocalProperties();
 			will(returnValue(local));
-			oneOf(callback).getRemoteProperties();
+			oneOf(callback).getRemoteProperties(contactId);
 			will(returnValue(remote));
 			oneOf(modem).dial(NUMBER);
 			will(throwException(new IOException()));