From 346bec94e88c8320d45e6c5407f6c4cc38704412 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 28 Apr 2020 17:45:17 +0100 Subject: [PATCH 1/4] Discover contacts' BT addresses from incoming connections. --- .../AndroidBluetoothTransportConnection.java | 3 + .../AbstractDuplexTransportConnection.java | 8 ++ .../duplex/DuplexTransportConnection.java | 7 ++ .../properties/TransportPropertyManager.java | 8 ++ .../bramble/plugin/ConnectionManagerImpl.java | 14 +++- .../TransportPropertyManagerImpl.java | 45 +++++++++-- .../TransportPropertyManagerImplTest.java | 79 ++++++++++++++++--- .../test/TestDuplexTransportConnection.java | 6 ++ 8 files changed, 154 insertions(+), 16 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java index 989c80d47..aedb15f67 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java @@ -10,6 +10,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import static org.briarproject.bramble.api.plugin.BluetoothConstants.PROP_ADDRESS; + @NotNullByDefault class AndroidBluetoothTransportConnection extends AbstractDuplexTransportConnection { @@ -23,6 +25,7 @@ class AndroidBluetoothTransportConnection super(plugin); this.connectionManager = connectionManager; this.socket = socket; + remote.put(PROP_ADDRESS, socket.getRemoteDevice().getAddress()); } @Override diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/AbstractDuplexTransportConnection.java b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/AbstractDuplexTransportConnection.java index ba84134e9..27aad596a 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/AbstractDuplexTransportConnection.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/AbstractDuplexTransportConnection.java @@ -4,6 +4,7 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.Plugin; import org.briarproject.bramble.api.plugin.TransportConnectionReader; import org.briarproject.bramble.api.plugin.TransportConnectionWriter; +import org.briarproject.bramble.api.properties.TransportProperties; import java.io.IOException; import java.io.InputStream; @@ -14,6 +15,8 @@ import java.util.concurrent.atomic.AtomicBoolean; public abstract class AbstractDuplexTransportConnection implements DuplexTransportConnection { + protected final TransportProperties remote = new TransportProperties(); + private final Plugin plugin; private final Reader reader; private final Writer writer; @@ -44,6 +47,11 @@ public abstract class AbstractDuplexTransportConnection return writer; } + @Override + public TransportProperties getRemoteProperties() { + return remote; + } + private class Reader implements TransportConnectionReader { @Override diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/DuplexTransportConnection.java b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/DuplexTransportConnection.java index f235e240b..d0e98bfba 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/DuplexTransportConnection.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/DuplexTransportConnection.java @@ -3,6 +3,7 @@ package org.briarproject.bramble.api.plugin.duplex; 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.properties.TransportProperties; /** * An interface for reading and writing data over a duplex transport. The @@ -23,4 +24,10 @@ public interface DuplexTransportConnection { * for writing to the connection. */ TransportConnectionWriter getWriter(); + + /** + * Returns a possibly empty set of {@link TransportProperties} describing + * the remote peer. + */ + TransportProperties getRemoteProperties(); } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyManager.java index a634bfaa7..de10f2aca 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyManager.java @@ -34,6 +34,14 @@ public interface TransportPropertyManager { void addRemoteProperties(Transaction txn, ContactId c, Map props) throws DbException; + /** + * Stores the given properties discovered from an incoming transport + * connection. They will be overridden by any properties received while + * adding the contact or synced from the contact. + */ + void addRemotePropertiesFromConnection(ContactId c, TransportId t, + TransportProperties props) throws DbException; + /** * Returns the local transport properties for all transports. */ diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionManagerImpl.java index d3fec490b..979e8ea01 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionManagerImpl.java @@ -15,6 +15,8 @@ import org.briarproject.bramble.api.plugin.TransportConnectionReader; import org.briarproject.bramble.api.plugin.TransportConnectionWriter; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; +import org.briarproject.bramble.api.properties.TransportProperties; +import org.briarproject.bramble.api.properties.TransportPropertyManager; import org.briarproject.bramble.api.sync.SyncSession; import org.briarproject.bramble.api.sync.SyncSessionFactory; import org.briarproject.bramble.api.transport.KeyManager; @@ -52,6 +54,7 @@ class ConnectionManagerImpl implements ConnectionManager { private final HandshakeManager handshakeManager; private final ContactExchangeManager contactExchangeManager; private final ConnectionRegistry connectionRegistry; + private final TransportPropertyManager transportPropertyManager; @Inject ConnectionManagerImpl(@IoExecutor Executor ioExecutor, @@ -60,7 +63,8 @@ class ConnectionManagerImpl implements ConnectionManager { SyncSessionFactory syncSessionFactory, HandshakeManager handshakeManager, ContactExchangeManager contactExchangeManager, - ConnectionRegistry connectionRegistry) { + ConnectionRegistry connectionRegistry, + TransportPropertyManager transportPropertyManager) { this.ioExecutor = ioExecutor; this.keyManager = keyManager; this.streamReaderFactory = streamReaderFactory; @@ -69,6 +73,7 @@ class ConnectionManagerImpl implements ConnectionManager { this.handshakeManager = handshakeManager; this.contactExchangeManager = contactExchangeManager; this.connectionRegistry = connectionRegistry; + this.transportPropertyManager = transportPropertyManager; } @Override @@ -269,6 +274,7 @@ class ConnectionManagerImpl implements ConnectionManager { private final TransportId transportId; private final TransportConnectionReader reader; private final TransportConnectionWriter writer; + private final TransportProperties remote; @Nullable private volatile SyncSession outgoingSession = null; @@ -278,6 +284,7 @@ class ConnectionManagerImpl implements ConnectionManager { this.transportId = transportId; reader = connection.getReader(); writer = connection.getWriter(); + remote = connection.getRemoteProperties(); } @Override @@ -313,13 +320,16 @@ class ConnectionManagerImpl implements ConnectionManager { // Start the outgoing session on another thread ioExecutor.execute(() -> runOutgoingSession(contactId)); try { + // Store any transport properties discovered from the connection + transportPropertyManager.addRemotePropertiesFromConnection( + contactId, transportId, remote); // Create and run the incoming session createIncomingSession(ctx, reader).run(); reader.dispose(false, true); // Interrupt the outgoing session so it finishes cleanly SyncSession out = outgoingSession; if (out != null) out.interrupt(); - } catch (IOException e) { + } catch (DbException | IOException e) { logException(LOG, WARNING, e); onReadError(true); } finally { diff --git a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java index 9e79bfd66..c12b5ddf6 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java @@ -140,6 +140,27 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, } } + @Override + public void addRemotePropertiesFromConnection(ContactId c, TransportId t, + TransportProperties props) throws DbException { + if (props.isEmpty()) return; + try { + db.transaction(false, txn -> { + Group g = getContactGroup(db.getContact(txn, c)); + BdfDictionary meta = clientHelper.getGroupMetadataAsDictionary( + txn, g.getId()); + BdfDictionary discovered = + meta.getOptionalDictionary("discovered"); + if (discovered == null) discovered = new BdfDictionary(); + discovered.putAll(props); + meta.put("discovered", discovered); + clientHelper.mergeGroupMetadata(txn, g.getId(), meta); + }); + } catch (FormatException e) { + throw new DbException(e); + } + } + @Override public Map getLocalProperties() throws DbException { @@ -203,12 +224,26 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, Group g = getContactGroup(c); try { // Find the latest remote update + TransportProperties remote; 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); - return parseProperties(message); + if (latest == null) { + remote = new TransportProperties(); + } else { + // Retrieve and parse the latest remote properties + BdfList message = + clientHelper.getMessageAsList(txn, latest.messageId); + remote = parseProperties(message); + } + // Merge in any discovered properties + BdfDictionary meta = + clientHelper.getGroupMetadataAsDictionary(txn, g.getId()); + BdfDictionary d = meta.getOptionalDictionary("discovered"); + if (d == null) return remote; + TransportProperties merged = + clientHelper.parseAndValidateTransportProperties(d); + // Received properties override discovered properties + merged.putAll(remote); + return merged; } catch (FormatException e) { throw new DbException(e); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java index 007d264b7..e042a255c 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java @@ -24,12 +24,12 @@ import org.briarproject.bramble.test.DbExpectations; import org.jmock.Expectations; import org.junit.Test; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import static java.util.Arrays.asList; +import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.briarproject.bramble.api.properties.TransportPropertyManager.CLIENT_ID; @@ -405,25 +405,25 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { List contacts = asList(contact1, contact2); Group contactGroup1 = getGroup(CLIENT_ID, MAJOR_VERSION); Group contactGroup2 = getGroup(CLIENT_ID, MAJOR_VERSION); - Map messageMetadata2 = + Map messageMetadata = new LinkedHashMap<>(); // A remote update for another transport should be ignored MessageId barUpdateId = new MessageId(getRandomId()); - messageMetadata2.put(barUpdateId, BdfDictionary.of( + messageMetadata.put(barUpdateId, BdfDictionary.of( new BdfEntry("transportId", "bar"), new BdfEntry("version", 1), new BdfEntry("local", false) )); // A local update for the right transport should be ignored MessageId localUpdateId = new MessageId(getRandomId()); - messageMetadata2.put(localUpdateId, BdfDictionary.of( + messageMetadata.put(localUpdateId, BdfDictionary.of( new BdfEntry("transportId", "foo"), new BdfEntry("version", 1), new BdfEntry("local", true) )); // A remote update for the right transport should be returned MessageId fooUpdateId = new MessageId(getRandomId()); - messageMetadata2.put(fooUpdateId, BdfDictionary.of( + messageMetadata.put(fooUpdateId, BdfDictionary.of( new BdfEntry("transportId", "foo"), new BdfEntry("version", 1), new BdfEntry("local", false) @@ -440,19 +440,25 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { will(returnValue(contactGroup1)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup1.getId()); - will(returnValue(Collections.emptyMap())); + will(returnValue(emptyMap())); + oneOf(clientHelper).getGroupMetadataAsDictionary(txn, + contactGroup1.getId()); + will(returnValue(new BdfDictionary())); // Second contact: returns an update oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, MAJOR_VERSION, contact2); will(returnValue(contactGroup2)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup2.getId()); - will(returnValue(messageMetadata2)); + will(returnValue(messageMetadata)); oneOf(clientHelper).getMessageAsList(txn, fooUpdateId); will(returnValue(fooUpdate)); oneOf(clientHelper).parseAndValidateTransportProperties( fooPropertiesDict); will(returnValue(fooProperties)); + oneOf(clientHelper).getGroupMetadataAsDictionary(txn, + contactGroup2.getId()); + will(returnValue(new BdfDictionary())); }}); TransportPropertyManagerImpl t = createInstance(); @@ -463,6 +469,61 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { assertEquals(fooProperties, properties.get(contact2.getId())); } + @Test + public void testReceivePropertiesOverrideDiscoveredProperties() + throws Exception { + Transaction txn = new Transaction(null, true); + Contact contact = getContact(); + List contacts = singletonList(contact); + Group contactGroup = getGroup(CLIENT_ID, MAJOR_VERSION); + MessageId updateId = new MessageId(getRandomId()); + Map messageMetadata = singletonMap(updateId, + BdfDictionary.of( + new BdfEntry("transportId", "foo"), + new BdfEntry("version", 1), + new BdfEntry("local", false) + )); + BdfList update = BdfList.of("foo", 1, fooPropertiesDict); + TransportProperties discovered = new TransportProperties(); + discovered.put("fooKey1", "overridden"); + discovered.put("fooKey3", "fooValue3"); + BdfDictionary discoveredDict = new BdfDictionary(discovered); + BdfDictionary groupMeta = + BdfDictionary.of(new BdfEntry("discovered", discoveredDict)); + TransportProperties merged = new TransportProperties(); + merged.putAll(fooProperties); + merged.put("fooKey3", "fooValue3"); + + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + oneOf(db).getContacts(txn); + will(returnValue(contacts)); + // One update + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(clientHelper).getMessageMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(messageMetadata)); + oneOf(clientHelper).getMessageAsList(txn, updateId); + will(returnValue(update)); + oneOf(clientHelper).parseAndValidateTransportProperties( + fooPropertiesDict); + will(returnValue(fooProperties)); + oneOf(clientHelper).getGroupMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(groupMeta)); + oneOf(clientHelper).parseAndValidateTransportProperties( + discoveredDict); + will(returnValue(discovered)); + }}); + + TransportPropertyManagerImpl t = createInstance(); + Map properties = + t.getRemoteProperties(new TransportId("foo")); + assertEquals(merged, properties.get(contact.getId())); + } + @Test public void testMergingUnchangedPropertiesDoesNotCreateUpdate() throws Exception { @@ -505,7 +566,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { // There are no existing properties to merge with oneOf(clientHelper).getMessageMetadataAsDictionary(txn, localGroup.getId()); - will(returnValue(Collections.emptyMap())); + will(returnValue(emptyMap())); // Store the new properties in the local group, version 1 expectStoreMessage(txn, localGroup.getId(), "foo", fooPropertiesDict, 1, true, false); @@ -517,7 +578,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { will(returnValue(contactGroup)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup.getId()); - will(returnValue(Collections.emptyMap())); + will(returnValue(emptyMap())); expectStoreMessage(txn, contactGroup.getId(), "foo", fooPropertiesDict, 1, true, true); }}); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/TestDuplexTransportConnection.java b/bramble-core/src/test/java/org/briarproject/bramble/test/TestDuplexTransportConnection.java index 007ec5f9d..0ed6f9045 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/test/TestDuplexTransportConnection.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/TestDuplexTransportConnection.java @@ -4,6 +4,7 @@ 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.duplex.DuplexTransportConnection; +import org.briarproject.bramble.api.properties.TransportProperties; import java.io.IOException; import java.io.InputStream; @@ -37,6 +38,11 @@ public class TestDuplexTransportConnection return writer; } + @Override + public TransportProperties getRemoteProperties() { + return new TransportProperties(); + } + /** * Creates and returns a pair of TestDuplexTransportConnections that are * connected to each other. From 7320099494ac372e0bea63cf3b67b21bdc5c6d09 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 28 Apr 2020 17:56:01 +0100 Subject: [PATCH 2/4] Also store properties discovered from outgoing connections. This is useful when adding a Bluetooth address is discovered while adding a contact. --- .../briarproject/bramble/plugin/ConnectionManagerImpl.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionManagerImpl.java index 979e8ea01..ba45b6e1a 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionManagerImpl.java @@ -385,6 +385,7 @@ class ConnectionManagerImpl implements ConnectionManager { private final TransportId transportId; private final TransportConnectionReader reader; private final TransportConnectionWriter writer; + private final TransportProperties remote; @Nullable private volatile SyncSession outgoingSession = null; @@ -395,6 +396,7 @@ class ConnectionManagerImpl implements ConnectionManager { this.transportId = transportId; reader = connection.getReader(); writer = connection.getWriter(); + remote = connection.getRemoteProperties(); } @Override @@ -471,13 +473,16 @@ class ConnectionManagerImpl implements ConnectionManager { connectionRegistry.registerConnection(contactId, transportId, false); try { + // Store any transport properties discovered from the connection + transportPropertyManager.addRemotePropertiesFromConnection( + contactId, transportId, remote); // Create and run the incoming session createIncomingSession(ctx, reader).run(); reader.dispose(false, true); // Interrupt the outgoing session so it finishes cleanly SyncSession out = outgoingSession; if (out != null) out.interrupt(); - } catch (IOException e) { + } catch (DbException | IOException e) { logException(LOG, WARNING, e); onReadError(); } finally { From b5fe55faf3fba29358c0011c67a37c2b8d2689b3 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 29 Apr 2020 15:28:27 +0100 Subject: [PATCH 3/4] Validate remote address. --- .../plugin/bluetooth/AndroidBluetoothTransportConnection.java | 4 +++- .../main/java/org/briarproject/bramble/util/AndroidUtils.java | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java index aedb15f67..226ad0330 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java @@ -11,6 +11,7 @@ import java.io.InputStream; import java.io.OutputStream; import static org.briarproject.bramble.api.plugin.BluetoothConstants.PROP_ADDRESS; +import static org.briarproject.bramble.util.AndroidUtils.isValidBluetoothAddress; @NotNullByDefault class AndroidBluetoothTransportConnection @@ -25,7 +26,8 @@ class AndroidBluetoothTransportConnection super(plugin); this.connectionManager = connectionManager; this.socket = socket; - remote.put(PROP_ADDRESS, socket.getRemoteDevice().getAddress()); + String address = socket.getRemoteDevice().getAddress(); + if (isValidBluetoothAddress(address)) remote.put(PROP_ADDRESS, address); } @Override diff --git a/bramble-android/src/main/java/org/briarproject/bramble/util/AndroidUtils.java b/bramble-android/src/main/java/org/briarproject/bramble/util/AndroidUtils.java index 6182bed9b..4d5c8ea13 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/util/AndroidUtils.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/util/AndroidUtils.java @@ -71,7 +71,7 @@ public class AndroidUtils { return new Pair<>("", ""); } - private static boolean isValidBluetoothAddress(@Nullable String address) { + public static boolean isValidBluetoothAddress(@Nullable String address) { return !StringUtils.isNullOrEmpty(address) && BluetoothAdapter.checkBluetoothAddress(address) && !address.equals(FAKE_BLUETOOTH_ADDRESS); From fb918457d49ba7d7eaae0a7d779044be98753d68 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 29 Apr 2020 15:37:21 +0100 Subject: [PATCH 4/4] Use constants for metadata keys. --- .../TransportPropertyConstants.java | 24 ++++ .../TransportPropertyManagerImpl.java | 33 +++-- .../TransportPropertyManagerImplTest.java | 119 +++++++++--------- 3 files changed, 106 insertions(+), 70 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyConstants.java b/bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyConstants.java index 9a46624e5..99f2fa798 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyConstants.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyConstants.java @@ -11,4 +11,28 @@ public interface TransportPropertyConstants { * The maximum length of a property's key or value in UTF-8 bytes. */ int MAX_PROPERTY_LENGTH = 100; + + /** + * Message metadata key for the transport ID of a local or remote update, + * as a BDF string. + */ + String MSG_KEY_TRANSPORT_ID = "transportId"; + + /** + * Message metadata key for the version number of a local or remote update, + * as a BDF long. + */ + String MSG_KEY_VERSION = "version"; + + /** + * Message metadata key for whether an update is local or remote, as a BDF + * boolean. + */ + String MSG_KEY_LOCAL = "local"; + + /** + * Group metadata key for any discovered transport properties of the + * contact, as a BDF dictionary. + */ + String GROUP_KEY_DISCOVERED = "discovered"; } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java index c12b5ddf6..f0bcde0e6 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java @@ -37,6 +37,11 @@ import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import javax.inject.Inject; +import static org.briarproject.bramble.api.properties.TransportPropertyConstants.GROUP_KEY_DISCOVERED; +import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MSG_KEY_LOCAL; +import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MSG_KEY_TRANSPORT_ID; +import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MSG_KEY_VERSION; + @Immutable @NotNullByDefault class TransportPropertyManagerImpl implements TransportPropertyManager, @@ -111,10 +116,10 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, try { // Find the latest update for this transport, if any BdfDictionary d = metadataParser.parse(meta); - TransportId t = new TransportId(d.getString("transportId")); + TransportId t = new TransportId(d.getString(MSG_KEY_TRANSPORT_ID)); LatestUpdate latest = findLatest(txn, m.getGroupId(), t, false); if (latest != null) { - if (d.getLong("version") > latest.version) { + if (d.getLong(MSG_KEY_VERSION) > latest.version) { // This update is newer - delete the previous update db.deleteMessage(txn, latest.messageId); db.deleteMessageMetadata(txn, latest.messageId); @@ -150,10 +155,10 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, BdfDictionary meta = clientHelper.getGroupMetadataAsDictionary( txn, g.getId()); BdfDictionary discovered = - meta.getOptionalDictionary("discovered"); + meta.getOptionalDictionary(GROUP_KEY_DISCOVERED); if (discovered == null) discovered = new BdfDictionary(); discovered.putAll(props); - meta.put("discovered", discovered); + meta.put(GROUP_KEY_DISCOVERED, discovered); clientHelper.mergeGroupMetadata(txn, g.getId(), meta); }); } catch (FormatException e) { @@ -237,7 +242,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, // Merge in any discovered properties BdfDictionary meta = clientHelper.getGroupMetadataAsDictionary(txn, g.getId()); - BdfDictionary d = meta.getOptionalDictionary("discovered"); + BdfDictionary d = meta.getOptionalDictionary(GROUP_KEY_DISCOVERED); if (d == null) return remote; TransportProperties merged = clientHelper.parseAndValidateTransportProperties(d); @@ -316,9 +321,9 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, long now = clock.currentTimeMillis(); Message m = clientHelper.createMessage(g, now, body); BdfDictionary meta = new BdfDictionary(); - meta.put("transportId", t.getString()); - meta.put("version", version); - meta.put("local", local); + meta.put(MSG_KEY_TRANSPORT_ID, t.getString()); + meta.put(MSG_KEY_VERSION, version); + meta.put(MSG_KEY_LOCAL, local); clientHelper.addLocalMessage(txn, m, meta, shared, false); } catch (FormatException e) { throw new RuntimeException(e); @@ -337,8 +342,9 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, .getMessageMetadataAsDictionary(txn, localGroup.getId()); for (Entry e : metadata.entrySet()) { BdfDictionary meta = e.getValue(); - TransportId t = new TransportId(meta.getString("transportId")); - long version = meta.getLong("version"); + TransportId t = + new TransportId(meta.getString(MSG_KEY_TRANSPORT_ID)); + long version = meta.getLong(MSG_KEY_VERSION); latestUpdates.put(t, new LatestUpdate(e.getKey(), version)); } return latestUpdates; @@ -351,9 +357,10 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, clientHelper.getMessageMetadataAsDictionary(txn, g); for (Entry e : metadata.entrySet()) { BdfDictionary meta = e.getValue(); - if (meta.getString("transportId").equals(t.getString()) - && meta.getBoolean("local") == local) { - return new LatestUpdate(e.getKey(), meta.getLong("version")); + if (meta.getString(MSG_KEY_TRANSPORT_ID).equals(t.getString()) + && meta.getBoolean(MSG_KEY_LOCAL) == local) { + return new LatestUpdate(e.getKey(), + meta.getLong(MSG_KEY_VERSION)); } } return null; diff --git a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java index e042a255c..6b11df71e 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java @@ -32,6 +32,10 @@ import static java.util.Arrays.asList; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; +import static org.briarproject.bramble.api.properties.TransportPropertyConstants.GROUP_KEY_DISCOVERED; +import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MSG_KEY_LOCAL; +import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MSG_KEY_TRANSPORT_ID; +import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MSG_KEY_VERSION; import static org.briarproject.bramble.api.properties.TransportPropertyManager.CLIENT_ID; import static org.briarproject.bramble.api.properties.TransportPropertyManager.MAJOR_VERSION; import static org.briarproject.bramble.api.sync.Group.Visibility.SHARED; @@ -186,25 +190,25 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { Message message = getMessage(contactGroupId); Metadata meta = new Metadata(); BdfDictionary metaDictionary = BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 2), - new BdfEntry("local", false) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 2), + new BdfEntry(MSG_KEY_LOCAL, false) ); Map messageMetadata = new LinkedHashMap<>(); // A remote update for another transport should be ignored MessageId barUpdateId = new MessageId(getRandomId()); messageMetadata.put(barUpdateId, BdfDictionary.of( - new BdfEntry("transportId", "bar"), - new BdfEntry("version", 1), - new BdfEntry("local", false) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "bar"), + new BdfEntry(MSG_KEY_VERSION, 1), + new BdfEntry(MSG_KEY_LOCAL, false) )); // A local update for the same transport should be ignored MessageId localUpdateId = new MessageId(getRandomId()); messageMetadata.put(localUpdateId, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 1), - new BdfEntry("local", true) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 1), + new BdfEntry(MSG_KEY_LOCAL, true) )); context.checking(new Expectations() {{ @@ -228,18 +232,18 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { Metadata meta = new Metadata(); // Version 4 is being delivered BdfDictionary metaDictionary = BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 4), - new BdfEntry("local", false) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 4), + new BdfEntry(MSG_KEY_LOCAL, false) ); Map messageMetadata = new LinkedHashMap<>(); // An older remote update for the same transport should be deleted MessageId fooVersion3 = new MessageId(getRandomId()); messageMetadata.put(fooVersion3, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 3), - new BdfEntry("local", false) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 3), + new BdfEntry(MSG_KEY_LOCAL, false) )); context.checking(new Expectations() {{ @@ -265,18 +269,18 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { Metadata meta = new Metadata(); // Version 3 is being delivered BdfDictionary metaDictionary = BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 3), - new BdfEntry("local", false) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 3), + new BdfEntry(MSG_KEY_LOCAL, false) ); Map messageMetadata = new LinkedHashMap<>(); // A newer remote update for the same transport should not be deleted MessageId fooVersion4 = new MessageId(getRandomId()); messageMetadata.put(fooVersion4, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 4), - new BdfEntry("local", false) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 4), + new BdfEntry(MSG_KEY_LOCAL, false) )); context.checking(new Expectations() {{ @@ -342,9 +346,9 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { // A local update for another transport should be ignored MessageId barUpdateId = new MessageId(getRandomId()); messageMetadata.put(barUpdateId, BdfDictionary.of( - new BdfEntry("transportId", "bar"), - new BdfEntry("version", 1), - new BdfEntry("local", true) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "bar"), + new BdfEntry(MSG_KEY_VERSION, 1), + new BdfEntry(MSG_KEY_LOCAL, true) )); context.checking(new DbExpectations() {{ @@ -366,16 +370,16 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { // A local update for another transport should be ignored MessageId barUpdateId = new MessageId(getRandomId()); messageMetadata.put(barUpdateId, BdfDictionary.of( - new BdfEntry("transportId", "bar"), - new BdfEntry("version", 1), - new BdfEntry("local", true) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "bar"), + new BdfEntry(MSG_KEY_VERSION, 1), + new BdfEntry(MSG_KEY_LOCAL, true) )); // A local update for the right transport should be returned MessageId fooUpdateId = new MessageId(getRandomId()); messageMetadata.put(fooUpdateId, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 1), - new BdfEntry("local", true) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 1), + new BdfEntry(MSG_KEY_LOCAL, true) )); BdfList fooUpdate = BdfList.of("foo", 1, fooPropertiesDict); @@ -410,23 +414,23 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { // A remote update for another transport should be ignored MessageId barUpdateId = new MessageId(getRandomId()); messageMetadata.put(barUpdateId, BdfDictionary.of( - new BdfEntry("transportId", "bar"), - new BdfEntry("version", 1), - new BdfEntry("local", false) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "bar"), + new BdfEntry(MSG_KEY_VERSION, 1), + new BdfEntry(MSG_KEY_LOCAL, false) )); // A local update for the right transport should be ignored MessageId localUpdateId = new MessageId(getRandomId()); messageMetadata.put(localUpdateId, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 1), - new BdfEntry("local", true) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 1), + new BdfEntry(MSG_KEY_LOCAL, true) )); // A remote update for the right transport should be returned MessageId fooUpdateId = new MessageId(getRandomId()); messageMetadata.put(fooUpdateId, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 1), - new BdfEntry("local", false) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 1), + new BdfEntry(MSG_KEY_LOCAL, false) )); BdfList fooUpdate = BdfList.of("foo", 1, fooPropertiesDict); @@ -479,17 +483,18 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { MessageId updateId = new MessageId(getRandomId()); Map messageMetadata = singletonMap(updateId, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 1), - new BdfEntry("local", false) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 1), + new BdfEntry(MSG_KEY_LOCAL, false) )); BdfList update = BdfList.of("foo", 1, fooPropertiesDict); TransportProperties discovered = new TransportProperties(); discovered.put("fooKey1", "overridden"); discovered.put("fooKey3", "fooValue3"); BdfDictionary discoveredDict = new BdfDictionary(discovered); - BdfDictionary groupMeta = - BdfDictionary.of(new BdfEntry("discovered", discoveredDict)); + BdfDictionary groupMeta = BdfDictionary.of( + new BdfEntry(GROUP_KEY_DISCOVERED, discoveredDict) + ); TransportProperties merged = new TransportProperties(); merged.putAll(fooProperties); merged.put("fooKey3", "fooValue3"); @@ -531,9 +536,9 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { MessageId updateId = new MessageId(getRandomId()); Map messageMetadata = singletonMap(updateId, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 1), - new BdfEntry("local", true) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 1), + new BdfEntry(MSG_KEY_LOCAL, true) )); BdfList update = BdfList.of("foo", 1, fooPropertiesDict); @@ -593,9 +598,9 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { Contact contact = getContact(); Group contactGroup = getGroup(CLIENT_ID, MAJOR_VERSION); BdfDictionary oldMetadata = BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 1), - new BdfEntry("local", true) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 1), + new BdfEntry(MSG_KEY_LOCAL, true) ); MessageId localGroupUpdateId = new MessageId(getRandomId()); Map localGroupMessageMetadata = @@ -650,14 +655,14 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { // The latest update for transport "foo" should be returned MessageId fooVersion999 = new MessageId(getRandomId()); messageMetadata.put(fooVersion999, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 999) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "foo"), + new BdfEntry(MSG_KEY_VERSION, 999) )); // The latest update for transport "bar" should be returned MessageId barVersion3 = new MessageId(getRandomId()); messageMetadata.put(barVersion3, BdfDictionary.of( - new BdfEntry("transportId", "bar"), - new BdfEntry("version", 3) + new BdfEntry(MSG_KEY_TRANSPORT_ID, "bar"), + new BdfEntry(MSG_KEY_VERSION, 3) )); BdfList fooUpdate = BdfList.of("foo", 999, fooPropertiesDict); BdfList barUpdate = BdfList.of("bar", 3, barPropertiesDict); @@ -688,9 +693,9 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { Message message = getMessage(g); long timestamp = message.getTimestamp(); BdfDictionary meta = BdfDictionary.of( - new BdfEntry("transportId", transportId), - new BdfEntry("version", version), - new BdfEntry("local", local) + new BdfEntry(MSG_KEY_TRANSPORT_ID, transportId), + new BdfEntry(MSG_KEY_VERSION, version), + new BdfEntry(MSG_KEY_LOCAL, local) ); context.checking(new Expectations() {{