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 99f2fa798..0d46964d6 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 @@ -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. diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/properties/event/RemoteTransportPropertiesUpdatedEvent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/properties/event/RemoteTransportPropertiesUpdatedEvent.java new file mode 100644 index 000000000..89b7ec131 --- /dev/null +++ b/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; + } +} 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 b1557e526..d5f2d4212 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 @@ -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 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); 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 b0602b5ad..9073917a2 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 @@ -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 localGroupMessageMetadata = + singletonMap(localGroupUpdateId, oldMetadata); + MessageId contactGroupUpdateId = new MessageId(getRandomId()); + Map 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 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 eventClass) { + for (CommitAction action : txn.getActions()) { + if (action instanceof EventAction) { + Event event = ((EventAction) action).getEvent(); + if (eventClass.isInstance(event)) return true; + } + } + return false; + } }