From fb918457d49ba7d7eaae0a7d779044be98753d68 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 29 Apr 2020 15:37:21 +0100 Subject: [PATCH] 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() {{