diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/ConnectionRegistry.java b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/ConnectionRegistry.java index 776e0507429350c84635aaf0077211a84678f17b..7f0adec367fbd0f72a3c00cf6320565bd427b5ab 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/ConnectionRegistry.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/ConnectionRegistry.java @@ -1,7 +1,14 @@ package org.briarproject.bramble.api.plugin; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.plugin.event.ConnectionClosedEvent; +import org.briarproject.bramble.api.plugin.event.ConnectionOpenedEvent; +import org.briarproject.bramble.api.plugin.event.ContactConnectedEvent; +import org.briarproject.bramble.api.plugin.event.ContactDisconnectedEvent; +import org.briarproject.bramble.api.rendezvous.event.RendezvousConnectionClosedEvent; +import org.briarproject.bramble.api.rendezvous.event.RendezvousConnectionOpenedEvent; import java.util.Collection; @@ -11,13 +18,50 @@ import java.util.Collection; @NotNullByDefault public interface ConnectionRegistry { + /** + * Registers a connection with the given contact over the given transport. + * Broadcasts {@link ConnectionOpenedEvent}. Also broadcasts + * {@link ContactConnectedEvent} if this is the only connection with the + * contact. + */ void registerConnection(ContactId c, TransportId t, boolean incoming); + /** + * Unregisters a connection with the given contact over the given transport. + * Broadcasts {@link ConnectionClosedEvent}. Also broadcasts + * {@link ContactDisconnectedEvent} if this is the only connection with + * the contact. + */ void unregisterConnection(ContactId c, TransportId t, boolean incoming); + /** + * Returns any contacts that are connected via the given transport. + */ Collection<ContactId> getConnectedContacts(TransportId t); + /** + * Returns true if the given contact is connected via the given transport. + */ boolean isConnected(ContactId c, TransportId t); + /** + * Returns true if the given contact is connected via any transport. + */ boolean isConnected(ContactId c); + + /** + * Registers a connection with the given pending contact. Broadcasts + * {@link RendezvousConnectionOpenedEvent} if this is the only connection + * with the pending contact. + * + * @return True if this is the only connection with the pending contact, + * false if it is redundant and should be closed + */ + boolean registerConnection(PendingContactId p); + + /** + * Unregisters a connection with the given pending contact. Broadcasts + * {@link RendezvousConnectionClosedEvent}. + */ + void unregisterConnection(PendingContactId p, boolean success); } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/rendezvous/event/RendezvousConnectionClosedEvent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/rendezvous/event/RendezvousConnectionClosedEvent.java new file mode 100644 index 0000000000000000000000000000000000000000..c9ec9ac768ae6214264ed5b8c82db3ad929a35ff --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/rendezvous/event/RendezvousConnectionClosedEvent.java @@ -0,0 +1,32 @@ +package org.briarproject.bramble.api.rendezvous.event; + +import org.briarproject.bramble.api.contact.PendingContactId; +import org.briarproject.bramble.api.event.Event; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import javax.annotation.concurrent.Immutable; + +/** + * An event that is broadcast when a rendezvous connection is closed. + */ +@Immutable +@NotNullByDefault +public class RendezvousConnectionClosedEvent extends Event { + + private final PendingContactId pendingContactId; + private final boolean success; + + public RendezvousConnectionClosedEvent(PendingContactId pendingContactId, + boolean success) { + this.pendingContactId = pendingContactId; + this.success = success; + } + + public PendingContactId getPendingContactId() { + return pendingContactId; + } + + public boolean isSuccess() { + return success; + } +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/rendezvous/event/RendezvousConnectionOpenedEvent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/rendezvous/event/RendezvousConnectionOpenedEvent.java new file mode 100644 index 0000000000000000000000000000000000000000..5324c081d408971a9d861196f016dbcb11dc2ad2 --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/rendezvous/event/RendezvousConnectionOpenedEvent.java @@ -0,0 +1,25 @@ +package org.briarproject.bramble.api.rendezvous.event; + +import org.briarproject.bramble.api.contact.PendingContactId; +import org.briarproject.bramble.api.event.Event; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import javax.annotation.concurrent.Immutable; + +/** + * An event that is broadcast when a rendezvous connection is opened. + */ +@Immutable +@NotNullByDefault +public class RendezvousConnectionOpenedEvent extends Event { + + private final PendingContactId pendingContactId; + + public RendezvousConnectionOpenedEvent(PendingContactId pendingContactId) { + this.pendingContactId = pendingContactId; + } + + public PendingContactId getPendingContactId() { + return pendingContactId; + } +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionRegistryImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionRegistryImpl.java index 2e4571040107af06e18a965cd5f3108977b73207..06056f96041986ae6d14ba690d138ec159ba4e73 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionRegistryImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionRegistryImpl.java @@ -2,6 +2,7 @@ package org.briarproject.bramble.plugin; import org.briarproject.bramble.api.Multiset; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.ConnectionRegistry; @@ -10,41 +11,49 @@ import org.briarproject.bramble.api.plugin.event.ConnectionClosedEvent; import org.briarproject.bramble.api.plugin.event.ConnectionOpenedEvent; import org.briarproject.bramble.api.plugin.event.ContactConnectedEvent; import org.briarproject.bramble.api.plugin.event.ContactDisconnectedEvent; +import org.briarproject.bramble.api.rendezvous.event.RendezvousConnectionClosedEvent; +import org.briarproject.bramble.api.rendezvous.event.RendezvousConnectionOpenedEvent; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; +import java.util.Set; import java.util.logging.Logger; +import javax.annotation.concurrent.GuardedBy; import javax.annotation.concurrent.ThreadSafe; import javax.inject.Inject; import static java.util.logging.Level.INFO; +import static java.util.logging.Logger.getLogger; @ThreadSafe @NotNullByDefault class ConnectionRegistryImpl implements ConnectionRegistry { private static final Logger LOG = - Logger.getLogger(ConnectionRegistryImpl.class.getName()); + getLogger(ConnectionRegistryImpl.class.getName()); private final EventBus eventBus; - private final Lock lock = new ReentrantLock(); - // The following are locking: lock - private final Map<TransportId, Multiset<ContactId>> connections; + private final Object lock = new Object(); + @GuardedBy("lock") + private final Map<TransportId, Multiset<ContactId>> contactConnections; + @GuardedBy("lock") private final Multiset<ContactId> contactCounts; + @GuardedBy("lock") + private final Set<PendingContactId> connectedPendingContacts; @Inject ConnectionRegistryImpl(EventBus eventBus) { this.eventBus = eventBus; - connections = new HashMap<>(); + contactConnections = new HashMap<>(); contactCounts = new Multiset<>(); + connectedPendingContacts = new HashSet<>(); } @Override @@ -55,17 +64,14 @@ class ConnectionRegistryImpl implements ConnectionRegistry { else LOG.info("Outgoing connection registered: " + t); } boolean firstConnection = false; - lock.lock(); - try { - Multiset<ContactId> m = connections.get(t); + synchronized (lock) { + Multiset<ContactId> m = contactConnections.get(t); if (m == null) { m = new Multiset<>(); - connections.put(t, m); + contactConnections.put(t, m); } m.add(c); if (contactCounts.add(c) == 1) firstConnection = true; - } finally { - lock.unlock(); } eventBus.broadcast(new ConnectionOpenedEvent(c, t, incoming)); if (firstConnection) { @@ -82,14 +88,12 @@ class ConnectionRegistryImpl implements ConnectionRegistry { else LOG.info("Outgoing connection unregistered: " + t); } boolean lastConnection = false; - lock.lock(); - try { - Multiset<ContactId> m = connections.get(t); - if (m == null) throw new IllegalArgumentException(); + synchronized (lock) { + Multiset<ContactId> m = contactConnections.get(t); + if (m == null || !m.contains(c)) + throw new IllegalArgumentException(); m.remove(c); if (contactCounts.remove(c) == 0) lastConnection = true; - } finally { - lock.unlock(); } eventBus.broadcast(new ConnectionClosedEvent(c, t, incoming)); if (lastConnection) { @@ -100,37 +104,47 @@ class ConnectionRegistryImpl implements ConnectionRegistry { @Override public Collection<ContactId> getConnectedContacts(TransportId t) { - lock.lock(); - try { - Multiset<ContactId> m = connections.get(t); + synchronized (lock) { + Multiset<ContactId> m = contactConnections.get(t); if (m == null) return Collections.emptyList(); List<ContactId> ids = new ArrayList<>(m.keySet()); if (LOG.isLoggable(INFO)) LOG.info(ids.size() + " contacts connected: " + t); return ids; - } finally { - lock.unlock(); } } @Override public boolean isConnected(ContactId c, TransportId t) { - lock.lock(); - try { - Multiset<ContactId> m = connections.get(t); + synchronized (lock) { + Multiset<ContactId> m = contactConnections.get(t); return m != null && m.contains(c); - } finally { - lock.unlock(); } } @Override public boolean isConnected(ContactId c) { - lock.lock(); - try { + synchronized (lock) { return contactCounts.contains(c); - } finally { - lock.unlock(); } } + + @Override + public boolean registerConnection(PendingContactId p) { + boolean added; + synchronized (lock) { + added = connectedPendingContacts.add(p); + } + if (added) eventBus.broadcast(new RendezvousConnectionOpenedEvent(p)); + return added; + } + + @Override + public void unregisterConnection(PendingContactId p, boolean success) { + synchronized (lock) { + if (!connectedPendingContacts.remove(p)) + throw new IllegalArgumentException(); + } + eventBus.broadcast(new RendezvousConnectionClosedEvent(p, success)); + } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/plugin/ConnectionRegistryImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/plugin/ConnectionRegistryImplTest.java index 526a39215495817194eea65978ac2c00e8c901cf..13c0466e8f6d2613386890855d03136f60a3077f 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/plugin/ConnectionRegistryImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/plugin/ConnectionRegistryImplTest.java @@ -1,6 +1,7 @@ package org.briarproject.bramble.plugin; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.plugin.ConnectionRegistry; import org.briarproject.bramble.api.plugin.TransportId; @@ -8,93 +9,106 @@ import org.briarproject.bramble.api.plugin.event.ConnectionClosedEvent; import org.briarproject.bramble.api.plugin.event.ConnectionOpenedEvent; import org.briarproject.bramble.api.plugin.event.ContactConnectedEvent; import org.briarproject.bramble.api.plugin.event.ContactDisconnectedEvent; -import org.briarproject.bramble.test.BrambleTestCase; +import org.briarproject.bramble.api.rendezvous.event.RendezvousConnectionClosedEvent; +import org.briarproject.bramble.api.rendezvous.event.RendezvousConnectionOpenedEvent; +import org.briarproject.bramble.test.BrambleMockTestCase; import org.jmock.Expectations; -import org.jmock.Mockery; import org.junit.Test; import java.util.Collection; -import java.util.Collections; -import java.util.NoSuchElementException; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static org.briarproject.bramble.test.TestUtils.getContactId; +import static org.briarproject.bramble.test.TestUtils.getRandomId; import static org.briarproject.bramble.test.TestUtils.getTransportId; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -public class ConnectionRegistryImplTest extends BrambleTestCase { +public class ConnectionRegistryImplTest extends BrambleMockTestCase { - private final ContactId contactId, contactId1; - private final TransportId transportId, transportId1; + private final EventBus eventBus = context.mock(EventBus.class); - public ConnectionRegistryImplTest() { - contactId = getContactId(); - contactId1 = getContactId(); - transportId = getTransportId(); - transportId1 = getTransportId(); - } + private final ContactId contactId = getContactId(); + private final ContactId contactId1 = getContactId(); + private final TransportId transportId = getTransportId(); + private final TransportId transportId1 = getTransportId(); + private final PendingContactId pendingContactId = + new PendingContactId(getRandomId()); @Test public void testRegisterAndUnregister() { - Mockery context = new Mockery(); - EventBus eventBus = context.mock(EventBus.class); - context.checking(new Expectations() {{ - exactly(5).of(eventBus).broadcast(with(any( - ConnectionOpenedEvent.class))); - exactly(2).of(eventBus).broadcast(with(any( - ConnectionClosedEvent.class))); - exactly(3).of(eventBus).broadcast(with(any( - ContactConnectedEvent.class))); - oneOf(eventBus).broadcast(with(any( - ContactDisconnectedEvent.class))); - }}); - ConnectionRegistry c = new ConnectionRegistryImpl(eventBus); // The registry should be empty - assertEquals(Collections.emptyList(), - c.getConnectedContacts(transportId)); - assertEquals(Collections.emptyList(), - c.getConnectedContacts(transportId1)); + assertEquals(emptyList(), c.getConnectedContacts(transportId)); + assertEquals(emptyList(), c.getConnectedContacts(transportId1)); + // Check that a registered connection shows up - this should // broadcast a ConnectionOpenedEvent and a ContactConnectedEvent + context.checking(new Expectations() {{ + oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); + oneOf(eventBus).broadcast(with(any(ContactConnectedEvent.class))); + }}); c.registerConnection(contactId, transportId, true); - assertEquals(Collections.singletonList(contactId), + assertEquals(singletonList(contactId), c.getConnectedContacts(transportId)); - assertEquals(Collections.emptyList(), - c.getConnectedContacts(transportId1)); + assertEquals(emptyList(), c.getConnectedContacts(transportId1)); + context.assertIsSatisfied(); + // Register an identical connection - this should broadcast a // ConnectionOpenedEvent and lookup should be unaffected + context.checking(new Expectations() {{ + oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class))); + }}); c.registerConnection(contactId, transportId, true); - assertEquals(Collections.singletonList(contactId), + assertEquals(singletonList(contactId), c.getConnectedContacts(transportId)); - assertEquals(Collections.emptyList(), - c.getConnectedContacts(transportId1)); + assertEquals(emptyList(), c.getConnectedContacts(transportId1)); + context.assertIsSatisfied(); + // Unregister one of the connections - this should broadcast a // ConnectionClosedEvent and lookup should be unaffected + context.checking(new Expectations() {{ + oneOf(eventBus).broadcast(with(any(ConnectionClosedEvent.class))); + }}); c.unregisterConnection(contactId, transportId, true); - assertEquals(Collections.singletonList(contactId), + assertEquals(singletonList(contactId), c.getConnectedContacts(transportId)); - assertEquals(Collections.emptyList(), - c.getConnectedContacts(transportId1)); + assertEquals(emptyList(), c.getConnectedContacts(transportId1)); + context.assertIsSatisfied(); + // Unregister the other connection - this should broadcast a // ConnectionClosedEvent and a ContactDisconnectedEvent + context.checking(new Expectations() {{ + oneOf(eventBus).broadcast(with(any(ConnectionClosedEvent.class))); + oneOf(eventBus).broadcast(with(any( + ContactDisconnectedEvent.class))); + }}); c.unregisterConnection(contactId, transportId, true); - assertEquals(Collections.emptyList(), - c.getConnectedContacts(transportId)); - assertEquals(Collections.emptyList(), - c.getConnectedContacts(transportId1)); + assertEquals(emptyList(), c.getConnectedContacts(transportId)); + assertEquals(emptyList(), c.getConnectedContacts(transportId1)); + context.assertIsSatisfied(); + // Try to unregister the connection again - exception should be thrown try { c.unregisterConnection(contactId, transportId, true); fail(); - } catch (NoSuchElementException expected) { + } catch (IllegalArgumentException expected) { // Expected } + // Register both contacts with one transport, one contact with both - // this should broadcast three ConnectionOpenedEvents and two // ContactConnectedEvents + context.checking(new Expectations() {{ + exactly(3).of(eventBus).broadcast(with(any( + ConnectionOpenedEvent.class))); + exactly(2).of(eventBus).broadcast(with(any( + ContactConnectedEvent.class))); + }}); c.registerConnection(contactId, transportId, true); c.registerConnection(contactId1, transportId, true); c.registerConnection(contactId1, transportId1, true); @@ -102,8 +116,34 @@ public class ConnectionRegistryImplTest extends BrambleTestCase { assertEquals(2, connected.size()); assertTrue(connected.contains(contactId)); assertTrue(connected.contains(contactId1)); - assertEquals(Collections.singletonList(contactId1), + assertEquals(singletonList(contactId1), c.getConnectedContacts(transportId1)); + } + + @Test + public void testRegisterAndUnregisterPendingContacts() { + ConnectionRegistry c = new ConnectionRegistryImpl(eventBus); + + context.checking(new Expectations() {{ + oneOf(eventBus).broadcast(with(any( + RendezvousConnectionOpenedEvent.class))); + }}); + assertTrue(c.registerConnection(pendingContactId)); + assertFalse(c.registerConnection(pendingContactId)); // Redundant context.assertIsSatisfied(); + + context.checking(new Expectations() {{ + oneOf(eventBus).broadcast(with(any( + RendezvousConnectionClosedEvent.class))); + }}); + c.unregisterConnection(pendingContactId, true); + context.assertIsSatisfied(); + + try { + c.unregisterConnection(pendingContactId, true); + fail(); + } catch (IllegalArgumentException expected) { + // Expected + } } }