From 7736a3b6fcb34586e1a47644a62ffb68a57fbfe9 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 26 Jun 2020 09:59:03 +0100 Subject: [PATCH] Use separate methods for registering incoming and outgoing connections. --- .../api/connection/ConnectionRegistry.java | 38 ++++++++++++-- .../connection/ConnectionRegistryImpl.java | 20 +++++++- .../IncomingDuplexSyncConnection.java | 4 +- .../OutgoingDuplexSyncConnection.java | 5 +- .../ConnectionRegistryImplTest.java | 50 ++++++++----------- 5 files changed, 76 insertions(+), 41 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/connection/ConnectionRegistry.java b/bramble-api/src/main/java/org/briarproject/bramble/api/connection/ConnectionRegistry.java index 4ff3eb5ef..d23b0144a 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/connection/ConnectionRegistry.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/connection/ConnectionRegistry.java @@ -22,14 +22,42 @@ import java.util.Collection; public interface ConnectionRegistry { /** - * Registers a connection with the given contact over the given transport. + * Registers an incoming connection from the given contact over the given + * transport. The connection's {@link Priority priority} can be set later + * via {@link #setPriority(ContactId, TransportId, InterruptibleConnection, + * Priority)} if a priority record is received from the contact. *

* Broadcasts {@link ConnectionOpenedEvent}. Also broadcasts * {@link ContactConnectedEvent} if this is the only connection with the * contact. */ - void registerConnection(ContactId c, TransportId t, - InterruptibleConnection conn, boolean incoming); + void registerIncomingConnection(ContactId c, TransportId t, + InterruptibleConnection conn); + + /** + * Registers an outgoing connection to the given contact over the given + * transport. + *

+ * Broadcasts {@link ConnectionOpenedEvent}. Also broadcasts + * {@link ContactConnectedEvent} if this is the only connection with the + * contact. + *

+ * If the registry has any "better" connections with the given contact, the + * given connection will be interrupted. If the registry has any "worse" + * connections with the given contact, those connections will be + * interrupted. + *

+ * Connection A is considered "better" than connection B if both + * connections have had their priorities set, and either A's transport is + * {@link PluginConfig#getTransportPreferences() preferred} to B's, or + * they use the same transport and A has higher {@link Priority priority} + * than B. + *

+ * For backward compatibility, connections without priorities are not + * considered better or worse than other connections. + */ + void registerOutgoingConnection(ContactId c, TransportId t, + InterruptibleConnection conn, Priority priority); /** * Unregisters a connection with the given contact over the given transport. @@ -43,8 +71,8 @@ public interface ConnectionRegistry { /** * Sets the {@link Priority priority} of a connection that was previously - * registered via {@link #registerConnection(ContactId, TransportId, - * InterruptibleConnection, boolean)}. + * registered via {@link #registerIncomingConnection(ContactId, TransportId, + * InterruptibleConnection)}. *

* If the registry has any "better" connections with the given contact, the * given connection will be interrupted. If the registry has any "worse" diff --git a/bramble-core/src/main/java/org/briarproject/bramble/connection/ConnectionRegistryImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/connection/ConnectionRegistryImpl.java index 897ef0880..bf33c7387 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/connection/ConnectionRegistryImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/connection/ConnectionRegistryImpl.java @@ -61,7 +61,25 @@ class ConnectionRegistryImpl implements ConnectionRegistry { } @Override - public void registerConnection(ContactId c, TransportId t, + public void registerIncomingConnection(ContactId c, TransportId t, + InterruptibleConnection conn) { + if (LOG.isLoggable(INFO)) { + LOG.info("Incoming connection registered: " + t); + } + registerConnection(c, t, conn, true); + } + + @Override + public void registerOutgoingConnection(ContactId c, TransportId t, + InterruptibleConnection conn, Priority priority) { + if (LOG.isLoggable(INFO)) { + LOG.info("Outgoing connection registered: " + t); + } + registerConnection(c, t, conn, false); + setPriority(c, t, conn, priority); + } + + private void registerConnection(ContactId c, TransportId t, InterruptibleConnection conn, boolean incoming) { if (LOG.isLoggable(INFO)) { if (incoming) LOG.info("Incoming connection registered: " + t); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/connection/IncomingDuplexSyncConnection.java b/bramble-core/src/main/java/org/briarproject/bramble/connection/IncomingDuplexSyncConnection.java index f1fa9fe2c..abf34ca0f 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/connection/IncomingDuplexSyncConnection.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/connection/IncomingDuplexSyncConnection.java @@ -59,8 +59,8 @@ class IncomingDuplexSyncConnection extends DuplexSyncConnection onReadError(true); return; } - connectionRegistry.registerConnection(contactId, transportId, - this, true); + connectionRegistry.registerIncomingConnection(contactId, transportId, + this); // Start the outgoing session on another thread ioExecutor.execute(() -> runOutgoingSession(contactId)); try { diff --git a/bramble-core/src/main/java/org/briarproject/bramble/connection/OutgoingDuplexSyncConnection.java b/bramble-core/src/main/java/org/briarproject/bramble/connection/OutgoingDuplexSyncConnection.java index 958f651dc..d7f777b7b 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/connection/OutgoingDuplexSyncConnection.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/connection/OutgoingDuplexSyncConnection.java @@ -104,9 +104,8 @@ class OutgoingDuplexSyncConnection extends DuplexSyncConnection onReadError(); return; } - connectionRegistry.registerConnection(contactId, transportId, - this, false); - connectionRegistry.setPriority(contactId, transportId, this, priority); + connectionRegistry.registerOutgoingConnection(contactId, transportId, + this, priority); try { // Store any transport properties discovered from the connection transportPropertyManager.addRemotePropertiesFromConnection( diff --git a/bramble-core/src/test/java/org/briarproject/bramble/connection/ConnectionRegistryImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/connection/ConnectionRegistryImplTest.java index 6a2821219..c8b6de86d 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/connection/ConnectionRegistryImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/connection/ConnectionRegistryImplTest.java @@ -85,7 +85,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); oneOf(eventBus).broadcast(with(any(ContactConnectedEvent.class))); }}); - c.registerConnection(contactId1, transportId1, conn1, true); + c.registerIncomingConnection(contactId1, transportId1, conn1); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -101,7 +101,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { context.checking(new Expectations() {{ oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); }}); - c.registerConnection(contactId1, transportId1, conn2, true); + c.registerIncomingConnection(contactId1, transportId1, conn2); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -170,9 +170,9 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { exactly(2).of(eventBus).broadcast(with(any( ContactConnectedEvent.class))); }}); - c.registerConnection(contactId1, transportId1, conn1, true); - c.registerConnection(contactId2, transportId1, conn2, true); - c.registerConnection(contactId2, transportId2, conn3, true); + c.registerIncomingConnection(contactId1, transportId1, conn1); + c.registerIncomingConnection(contactId2, transportId1, conn2); + c.registerIncomingConnection(contactId2, transportId2, conn3); context.assertIsSatisfied(); assertTrue(c.isConnected(contactId1)); @@ -212,12 +212,12 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { ConnectionRegistry c = new ConnectionRegistryImpl(eventBus, pluginConfig); - // Connect via transport 1 (worse than 2) and set priority to low + // Connect via transport 1 (worse than 2) with no priority set context.checking(new Expectations() {{ oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); oneOf(eventBus).broadcast(with(any(ContactConnectedEvent.class))); }}); - c.registerConnection(contactId1, transportId1, conn1, true); + c.registerIncomingConnection(contactId1, transportId1, conn1); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -234,8 +234,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { context.checking(new Expectations() {{ oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); }}); - c.registerConnection(contactId1, transportId2, conn2, true); - c.setPriority(contactId1, transportId2, conn2, high); + c.registerOutgoingConnection(contactId1, transportId2, conn2, high); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -253,8 +252,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { context.checking(new Expectations() {{ oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); }}); - c.registerConnection(contactId1, transportId3, conn3, true); - c.setPriority(contactId1, transportId3, conn3, high); + c.registerOutgoingConnection(contactId1, transportId3, conn3, high); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -290,8 +288,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); oneOf(eventBus).broadcast(with(any(ContactConnectedEvent.class))); }}); - c.registerConnection(contactId1, transportId1, conn1, true); - c.setPriority(contactId1, transportId1, conn1, low); + c.registerOutgoingConnection(contactId1, transportId1, conn1, low); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -312,8 +309,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { oneOf(conn2).interruptOutgoingSession(); oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); }}); - c.registerConnection(contactId1, transportId2, conn2, true); - c.setPriority(contactId1, transportId2, conn2, high); + c.registerOutgoingConnection(contactId1, transportId2, conn2, high); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -331,8 +327,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { context.checking(new Expectations() {{ oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); }}); - c.registerConnection(contactId1, transportId3, conn3, true); - c.setPriority(contactId1, transportId3, conn3, low); + c.registerOutgoingConnection(contactId1, transportId3, conn3, low); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -391,8 +386,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); oneOf(eventBus).broadcast(with(any(ContactConnectedEvent.class))); }}); - c.registerConnection(contactId1, transportId1, conn1, true); - c.setPriority(contactId1, transportId1, conn1, high); + c.registerOutgoingConnection(contactId1, transportId1, conn1, high); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -410,8 +404,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { oneOf(conn1).interruptOutgoingSession(); oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); }}); - c.registerConnection(contactId1, transportId2, conn2, true); - c.setPriority(contactId1, transportId2, conn2, low); + c.registerOutgoingConnection(contactId1, transportId2, conn2, low); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -429,7 +422,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { context.checking(new Expectations() {{ oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); }}); - c.registerConnection(contactId1, transportId3, conn3, true); + c.registerOutgoingConnection(contactId1, transportId3, conn3, high); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -486,8 +479,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); oneOf(eventBus).broadcast(with(any(ContactConnectedEvent.class))); }}); - c.registerConnection(contactId1, transportId1, conn1, true); - c.setPriority(contactId1, transportId1, conn1, high); + c.registerOutgoingConnection(contactId1, transportId1, conn1, high); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -499,7 +491,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { context.checking(new Expectations() {{ oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); }}); - c.registerConnection(contactId1, transportId1, conn2, true); + c.registerIncomingConnection(contactId1, transportId1, conn2); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -526,8 +518,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { oneOf(conn3).interruptOutgoingSession(); oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); }}); - c.registerConnection(contactId1, transportId1, conn3, true); - c.setPriority(contactId1, transportId1, conn3, low); + c.registerOutgoingConnection(contactId1, transportId1, conn3, low); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -551,8 +542,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); oneOf(eventBus).broadcast(with(any(ContactConnectedEvent.class))); }}); - c.registerConnection(contactId1, transportId1, conn1, true); - c.setPriority(contactId1, transportId1, conn1, low); + c.registerOutgoingConnection(contactId1, transportId1, conn1, low); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -564,7 +554,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { context.checking(new Expectations() {{ oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); }}); - c.registerConnection(contactId1, transportId1, conn2, true); + c.registerIncomingConnection(contactId1, transportId1, conn2); context.assertIsSatisfied(); assertEquals(singletonList(contactId1),