From 90e91221d98ae60132602fe81959a675b47e53dd Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 16 Jul 2020 14:25:43 +0100 Subject: [PATCH] Reflect discovered properties back to the remote peer. --- .../TransportPropertyConstants.java | 5 + .../TransportPropertyManagerImpl.java | 81 ++++++++-- .../TransportPropertyManagerImplTest.java | 149 ++++++++++++++++-- 3 files changed, 207 insertions(+), 28 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 3e6fe3865..8fc77fae0 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-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java index 3ef9236fa..44f48911a 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 @@ -44,6 +44,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; @Immutable @NotNullByDefault @@ -162,15 +163,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); @@ -235,6 +248,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 void storeContactIdsForContactGroups(Transaction txn) throws DbException { try { @@ -333,18 +364,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); } } }); @@ -353,6 +376,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 4d3831fdc..3a3e1c420 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 @@ -61,23 +61,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() { @@ -657,6 +662,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); }}); @@ -665,6 +673,49 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { t.mergeLocalProperties(new TransportId("foo"), fooProperties); } + @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); @@ -713,6 +764,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 @@ -723,6 +777,75 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { t.mergeLocalProperties(new TransportId("foo"), fooProperties); } + @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