Merge branch 'reflected-transport-properties' into 'master'

Reflect discovered transport properties back to the remote peer

See merge request briar/briar!1264

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

@@ -12,6 +12,11 @@ public interface TransportPropertyConstants {
 	 */
 	int MAX_PROPERTY_LENGTH = 100;
 
+	/**
+	 * Prefix for keys that represent reflected properties.
+	 */
+	String REFLECTED_PROPERTY_PREFIX = "u:";
+
 	/**
 	 * Message metadata key for the transport ID of a local or remote update,
 	 * as a BDF string.

bramble-api/src/main/java/org/briarproject/bramble/api/properties/event/RemoteTransportPropertiesUpdatedEvent.java

@@ -0,0 +1,27 @@
+package org.briarproject.bramble.api.properties.event;
+
+import org.briarproject.bramble.api.event.Event;
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.bramble.api.plugin.TransportId;
+import org.briarproject.bramble.api.properties.TransportProperties;
+
+import javax.annotation.concurrent.Immutable;
+
+/**
+ * An event that is broadcast when {@link TransportProperties} are received
+ * from a contact.
+ */
+@Immutable
+@NotNullByDefault
+public class RemoteTransportPropertiesUpdatedEvent extends Event {
+
+	private final TransportId transportId;
+
+	public RemoteTransportPropertiesUpdatedEvent(TransportId transportId) {
+		this.transportId = transportId;
+	}
+
+	public TransportId getTransportId() {
+		return transportId;
+	}
+}

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

@@ -18,6 +18,7 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.plugin.TransportId;
 import org.briarproject.bramble.api.properties.TransportProperties;
 import org.briarproject.bramble.api.properties.TransportPropertyManager;
+import org.briarproject.bramble.api.properties.event.RemoteTransportPropertiesUpdatedEvent;
 import org.briarproject.bramble.api.sync.Group;
 import org.briarproject.bramble.api.sync.Group.Visibility;
 import org.briarproject.bramble.api.sync.GroupId;
@@ -42,6 +43,7 @@ import static org.briarproject.bramble.api.properties.TransportPropertyConstants
 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.TransportPropertyConstants.REFLECTED_PROPERTY_PREFIX;
 import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty;
 
 @Immutable
@@ -129,8 +131,10 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 					// We've already received a newer update - delete this one
 					db.deleteMessage(txn, m.getId());
 					db.deleteMessageMetadata(txn, m.getId());
+					return false;
 				}
 			}
+			txn.attach(new RemoteTransportPropertiesUpdatedEvent(t));
 		} catch (FormatException e) {
 			throw new InvalidMessageException(e);
 		}
@@ -153,15 +157,27 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 		if (props.isEmpty()) return;
 		try {
 			db.transaction(false, txn -> {
-				Group g = getContactGroup(db.getContact(txn, c));
+				Contact contact = db.getContact(txn, c);
+				Group g = getContactGroup(contact);
 				BdfDictionary meta = clientHelper.getGroupMetadataAsDictionary(
 						txn, g.getId());
 				BdfDictionary discovered =
 						meta.getOptionalDictionary(GROUP_KEY_DISCOVERED);
-				if (discovered == null) discovered = new BdfDictionary();
-				discovered.putAll(props);
-				meta.put(GROUP_KEY_DISCOVERED, discovered);
-				clientHelper.mergeGroupMetadata(txn, g.getId(), meta);
+				BdfDictionary merged;
+				boolean changed;
+				if (discovered == null) {
+					merged = new BdfDictionary(props);
+					changed = true;
+				} else {
+					merged = new BdfDictionary(discovered);
+					merged.putAll(props);
+					changed = !merged.equals(discovered);
+				}
+				if (changed) {
+					meta.put(GROUP_KEY_DISCOVERED, merged);
+					clientHelper.mergeGroupMetadata(txn, g.getId(), meta);
+					updateLocalProperties(txn, contact, t);
+				}
 			});
 		} catch (FormatException e) {
 			throw new DbException(e);
@@ -226,6 +242,24 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 		});
 	}
 
+	private void updateLocalProperties(Transaction txn, Contact c,
+			TransportId t) throws DbException {
+		try {
+			TransportProperties local;
+			LatestUpdate latest = findLatest(txn, localGroup.getId(), t, true);
+			if (latest == null) {
+				local = new TransportProperties();
+			} else {
+				BdfList message = clientHelper.getMessageAsList(txn,
+						latest.messageId);
+				local = parseProperties(message);
+			}
+			storeLocalProperties(txn, c, t, local);
+		} catch (FormatException e) {
+			throw new DbException(e);
+		}
+	}
+
 	private TransportProperties getRemoteProperties(Transaction txn, Contact c,
 			TransportId t) throws DbException {
 		Group g = getContactGroup(c);
@@ -298,18 +332,10 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 					storeMessage(txn, localGroup.getId(), t, merged, version,
 							true, false);
 					// Delete the previous update, if any
-					if (latest != null)
-						db.removeMessage(txn, latest.messageId);
+					if (latest != null) db.removeMessage(txn, latest.messageId);
 					// Store the merged properties in each contact's group
 					for (Contact c : db.getContacts(txn)) {
-						Group g = getContactGroup(c);
-						latest = findLatest(txn, g.getId(), t, true);
-						version = latest == null ? 1 : latest.version + 1;
-						storeMessage(txn, g.getId(), t, merged, version,
-								true, true);
-						// Delete the previous update, if any
-						if (latest != null)
-							db.removeMessage(txn, latest.messageId);
+						storeLocalProperties(txn, c, t, merged);
 					}
 				}
 			});
@@ -318,6 +344,34 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 		}
 	}
 
+	private void storeLocalProperties(Transaction txn, Contact c,
+			TransportId t, TransportProperties p)
+			throws DbException, FormatException {
+		Group g = getContactGroup(c);
+		LatestUpdate latest = findLatest(txn, g.getId(), t, true);
+		long version = latest == null ? 1 : latest.version + 1;
+		// Reflect any remote properties we've discovered
+		BdfDictionary meta = clientHelper.getGroupMetadataAsDictionary(txn,
+				g.getId());
+		BdfDictionary discovered =
+				meta.getOptionalDictionary(GROUP_KEY_DISCOVERED);
+		TransportProperties combined;
+		if (discovered == null) {
+			combined = p;
+		} else {
+			combined = new TransportProperties(p);
+			TransportProperties d = clientHelper
+					.parseAndValidateTransportProperties(discovered);
+			for (Entry<String, String> e : d.entrySet()) {
+				String key = REFLECTED_PROPERTY_PREFIX + e.getKey();
+				combined.put(key, e.getValue());
+			}
+		}
+		storeMessage(txn, g.getId(), t, combined, version, true, true);
+		// Delete the previous update, if any
+		if (latest != null) db.removeMessage(txn, latest.messageId);
+	}
+
 	private Group getContactGroup(Contact c) {
 		return contactGroupFactory.createContactGroup(CLIENT_ID,
 				MAJOR_VERSION, c);

bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java

@@ -8,11 +8,15 @@ import org.briarproject.bramble.api.data.BdfDictionary;
 import org.briarproject.bramble.api.data.BdfEntry;
 import org.briarproject.bramble.api.data.BdfList;
 import org.briarproject.bramble.api.data.MetadataParser;
+import org.briarproject.bramble.api.db.CommitAction;
 import org.briarproject.bramble.api.db.DatabaseComponent;
+import org.briarproject.bramble.api.db.EventAction;
 import org.briarproject.bramble.api.db.Metadata;
 import org.briarproject.bramble.api.db.Transaction;
+import org.briarproject.bramble.api.event.Event;
 import org.briarproject.bramble.api.plugin.TransportId;
 import org.briarproject.bramble.api.properties.TransportProperties;
+import org.briarproject.bramble.api.properties.event.RemoteTransportPropertiesUpdatedEvent;
 import org.briarproject.bramble.api.sync.Group;
 import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.Message;
@@ -45,6 +49,7 @@ import static org.briarproject.bramble.test.TestUtils.getMessage;
 import static org.briarproject.bramble.test.TestUtils.getRandomId;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 
@@ -59,23 +64,28 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 	private final Clock clock = context.mock(Clock.class);
 
 	private final Group localGroup = getGroup(CLIENT_ID, MAJOR_VERSION);
-	private final BdfDictionary fooPropertiesDict = BdfDictionary.of(
-			new BdfEntry("fooKey1", "fooValue1"),
-			new BdfEntry("fooKey2", "fooValue2")
-	);
-	private final BdfDictionary barPropertiesDict = BdfDictionary.of(
-			new BdfEntry("barKey1", "barValue1"),
-			new BdfEntry("barKey2", "barValue2")
-	);
+	private final BdfDictionary fooPropertiesDict, barPropertiesDict;
+	private final BdfDictionary discoveredPropertiesDict, mergedPropertiesDict;
 	private final TransportProperties fooProperties, barProperties;
+	private final TransportProperties discoveredProperties;
 
-	public TransportPropertyManagerImplTest() throws Exception {
+	public TransportPropertyManagerImplTest() {
 		fooProperties = new TransportProperties();
-		for (String key : fooPropertiesDict.keySet())
-			fooProperties.put(key, fooPropertiesDict.getString(key));
+		fooProperties.put("fooKey1", "fooValue1");
+		fooProperties.put("fooKey2", "fooValue2");
+		fooPropertiesDict = new BdfDictionary(fooProperties);
+
 		barProperties = new TransportProperties();
-		for (String key : barPropertiesDict.keySet())
-			barProperties.put(key, barPropertiesDict.getString(key));
+		barProperties.put("barKey1", "barValue1");
+		barProperties.put("barKey2", "barValue2");
+		barPropertiesDict = new BdfDictionary(barProperties);
+
+		discoveredProperties = new TransportProperties();
+		discoveredProperties.put("fooKey3", "fooValue3");
+		discoveredPropertiesDict = new BdfDictionary(discoveredProperties);
+
+		mergedPropertiesDict = new BdfDictionary(fooProperties);
+		mergedPropertiesDict.put("u:fooKey3", "fooValue3");
 	}
 
 	private TransportPropertyManagerImpl createInstance() {
@@ -221,6 +231,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 
 		TransportPropertyManagerImpl t = createInstance();
 		assertFalse(t.incomingMessage(txn, message, meta));
+		assertTrue(hasEvent(txn, RemoteTransportPropertiesUpdatedEvent.class));
 	}
 
 	@Test
@@ -259,6 +270,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 
 		TransportPropertyManagerImpl t = createInstance();
 		assertFalse(t.incomingMessage(txn, message, meta));
+		assertTrue(hasEvent(txn, RemoteTransportPropertiesUpdatedEvent.class));
 	}
 
 	@Test
@@ -292,10 +304,12 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			// The update being delivered (version 3) should be deleted
 			oneOf(db).deleteMessage(txn, message.getId());
 			oneOf(db).deleteMessageMetadata(txn, message.getId());
+			// No event should be broadcast
 		}});
 
 		TransportPropertyManagerImpl t = createInstance();
 		assertFalse(t.incomingMessage(txn, message, meta));
+		assertFalse(hasEvent(txn, RemoteTransportPropertiesUpdatedEvent.class));
 	}
 
 	@Test
@@ -588,6 +602,9 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
 					contactGroup.getId());
 			will(returnValue(emptyMap()));
+			oneOf(clientHelper).getGroupMetadataAsDictionary(txn,
+					contactGroup.getId());
+			will(returnValue(new BdfDictionary()));
 			expectStoreMessage(txn, contactGroup.getId(), "foo",
 					fooPropertiesDict, 1, true, true);
 		}});
@@ -596,6 +613,49 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 		t.mergeLocalProperties(new TransportId("foo"), properties);
 	}
 
+	@Test
+	public void testMergingNewPropertiesCreatesUpdateWithReflectedProperties()
+			throws Exception {
+		Transaction txn = new Transaction(null, false);
+		Contact contact = getContact();
+		Group contactGroup = getGroup(CLIENT_ID, MAJOR_VERSION);
+		BdfDictionary contactGroupMeta = BdfDictionary.of(
+				new BdfEntry(GROUP_KEY_DISCOVERED, discoveredPropertiesDict)
+		);
+
+		context.checking(new DbExpectations() {{
+			oneOf(db).transaction(with(false), withDbRunnable(txn));
+			// There are no existing properties to merge with
+			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
+					localGroup.getId());
+			will(returnValue(emptyMap()));
+			// Store the new properties in the local group, version 1
+			expectStoreMessage(txn, localGroup.getId(), "foo",
+					fooPropertiesDict, 1, true, false);
+			// Store the new properties in each contact's group, version 1
+			oneOf(db).getContacts(txn);
+			will(returnValue(singletonList(contact)));
+			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
+					MAJOR_VERSION, contact);
+			will(returnValue(contactGroup));
+			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
+					contactGroup.getId());
+			will(returnValue(emptyMap()));
+			oneOf(clientHelper).getGroupMetadataAsDictionary(txn,
+					contactGroup.getId());
+			will(returnValue(contactGroupMeta));
+			// Reflect discovered properties
+			oneOf(clientHelper).parseAndValidateTransportProperties(
+					discoveredPropertiesDict);
+			will(returnValue(discoveredProperties));
+			expectStoreMessage(txn, contactGroup.getId(), "foo",
+					mergedPropertiesDict, 1, true, true);
+		}});
+
+		TransportPropertyManagerImpl t = createInstance();
+		t.mergeLocalProperties(new TransportId("foo"), fooProperties);
+	}
+
 	@Test
 	public void testMergingUpdatedPropertiesCreatesUpdate() throws Exception {
 		Transaction txn = new Transaction(null, false);
@@ -652,6 +712,9 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
 					contactGroup.getId());
 			will(returnValue(contactGroupMessageMetadata));
+			oneOf(clientHelper).getGroupMetadataAsDictionary(txn,
+					contactGroup.getId());
+			will(returnValue(new BdfDictionary()));
 			expectStoreMessage(txn, contactGroup.getId(), "foo",
 					fooPropertiesDict, 2, true, true);
 			// Delete the previous update
@@ -662,6 +725,75 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 		t.mergeLocalProperties(new TransportId("foo"), properties);
 	}
 
+	@Test
+	public void testMergingUpdatedPropertiesCreatesUpdateWithReflectedProperties()
+			throws Exception {
+		Transaction txn = new Transaction(null, false);
+		Contact contact = getContact();
+		Group contactGroup = getGroup(CLIENT_ID, MAJOR_VERSION);
+		BdfDictionary contactGroupMeta = BdfDictionary.of(
+				new BdfEntry(GROUP_KEY_DISCOVERED, discoveredPropertiesDict)
+		);
+		BdfDictionary oldMetadata = BdfDictionary.of(
+				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<MessageId, BdfDictionary> localGroupMessageMetadata =
+				singletonMap(localGroupUpdateId, oldMetadata);
+		MessageId contactGroupUpdateId = new MessageId(getRandomId());
+		Map<MessageId, BdfDictionary> contactGroupMessageMetadata =
+				singletonMap(contactGroupUpdateId, oldMetadata);
+		TransportProperties oldProperties = new TransportProperties();
+		oldProperties.put("fooKey1", "oldFooValue1");
+		BdfDictionary oldPropertiesDict = BdfDictionary.of(
+				new BdfEntry("fooKey1", "oldFooValue1")
+		);
+		BdfList oldUpdate = BdfList.of("foo", 1, oldPropertiesDict);
+
+		context.checking(new DbExpectations() {{
+			oneOf(db).transaction(with(false), withDbRunnable(txn));
+			// Merge the new properties with the existing properties
+			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
+					localGroup.getId());
+			will(returnValue(localGroupMessageMetadata));
+			oneOf(clientHelper).getMessageAsList(txn, localGroupUpdateId);
+			will(returnValue(oldUpdate));
+			oneOf(clientHelper).parseAndValidateTransportProperties(
+					oldPropertiesDict);
+			will(returnValue(oldProperties));
+			// Store the merged properties in the local group, version 2
+			expectStoreMessage(txn, localGroup.getId(), "foo",
+					fooPropertiesDict, 2, true, false);
+			// Delete the previous update
+			oneOf(db).removeMessage(txn, localGroupUpdateId);
+			// Store the merged properties in each contact's group, version 2
+			oneOf(db).getContacts(txn);
+			will(returnValue(singletonList(contact)));
+			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
+					MAJOR_VERSION, contact);
+			will(returnValue(contactGroup));
+			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
+					contactGroup.getId());
+			will(returnValue(contactGroupMessageMetadata));
+			oneOf(clientHelper).getGroupMetadataAsDictionary(txn,
+					contactGroup.getId());
+			will(returnValue(contactGroupMeta));
+			// Reflect discovered properties
+			oneOf(clientHelper).parseAndValidateTransportProperties(
+					discoveredPropertiesDict);
+			will(returnValue(discoveredProperties));
+			expectStoreMessage(txn, contactGroup.getId(), "foo",
+					mergedPropertiesDict, 2, true, true);
+			// Delete the previous update
+			oneOf(db).removeMessage(txn, contactGroupUpdateId);
+		}});
+
+		TransportPropertyManagerImpl t = createInstance();
+		t.mergeLocalProperties(new TransportId("foo"), fooProperties);
+	}
+
 	private void expectGetLocalProperties(Transaction txn) throws Exception {
 		Map<MessageId, BdfDictionary> messageMetadata = new LinkedHashMap<>();
 		// The latest update for transport "foo" should be returned
@@ -719,4 +851,15 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 					false);
 		}});
 	}
+
+	private boolean hasEvent(Transaction txn,
+			Class<? extends Event> eventClass) {
+		for (CommitAction action : txn.getActions()) {
+			if (action instanceof EventAction) {
+				Event event = ((EventAction) action).getEvent();
+				if (eventClass.isInstance(event)) return true;
+			}
+		}
+		return false;
+	}
 }