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 8e65b494da65a8111e130f4093732a9b04531dee..26ab19c330eaf756b87a1af7abf496efff2fc490 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 @@ -129,8 +129,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, public Map<TransportId, TransportProperties> getLocalProperties() throws DbException { Map<TransportId, TransportProperties> local; - // TODO: Transaction can be read-only when code is simplified - Transaction txn = db.startTransaction(false); + Transaction txn = db.startTransaction(true); try { local = getLocalProperties(txn); db.commitTransaction(txn); @@ -165,8 +164,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, throws DbException { try { TransportProperties p = null; - // TODO: Transaction can be read-only when code is simplified - Transaction txn = db.startTransaction(false); + Transaction txn = db.startTransaction(true); try { // Find the latest local update LatestUpdate latest = findLatest(txn, localGroup.getId(), t, @@ -192,8 +190,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, public Map<ContactId, TransportProperties> getRemoteProperties( TransportId t) throws DbException { Map<ContactId, TransportProperties> remote = new HashMap<>(); - // TODO: Transaction can be read-only when code is simplified - Transaction txn = db.startTransaction(false); + Transaction txn = db.startTransaction(true); try { for (Contact c : db.getContacts(txn)) remote.put(c.getId(), getRemoteProperties(txn, c, t)); @@ -227,8 +224,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, public TransportProperties getRemoteProperties(ContactId c, TransportId t) throws DbException { TransportProperties p; - // TODO: Transaction can be read-only when code is simplified - Transaction txn = db.startTransaction(false); + Transaction txn = db.startTransaction(true); try { p = getRemoteProperties(txn, db.getContact(txn, c), t); db.commitTransaction(txn); @@ -318,7 +314,6 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, private Map<TransportId, LatestUpdate> findLatestLocal(Transaction txn) throws DbException, FormatException { - // TODO: This can be simplified before 1.0 Map<TransportId, LatestUpdate> latestUpdates = new HashMap<>(); Map<MessageId, BdfDictionary> metadata = clientHelper .getMessageMetadataAsDictionary(txn, localGroup.getId()); @@ -326,17 +321,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, BdfDictionary meta = e.getValue(); TransportId t = new TransportId(meta.getString("transportId")); long version = meta.getLong("version"); - LatestUpdate latest = latestUpdates.get(t); - if (latest == null) { - latestUpdates.put(t, new LatestUpdate(e.getKey(), version)); - } else if (version > latest.version) { - // This update is newer - delete the previous one - db.removeMessage(txn, latest.messageId); - latestUpdates.put(t, new LatestUpdate(e.getKey(), version)); - } else { - // We've already found a newer update - delete this one - db.removeMessage(txn, e.getKey()); - } + latestUpdates.put(t, new LatestUpdate(e.getKey(), version)); } return latestUpdates; } @@ -344,38 +329,16 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, @Nullable private LatestUpdate findLatest(Transaction txn, GroupId g, TransportId t, boolean local) throws DbException, FormatException { - // TODO: This can be simplified before 1.0 - LatestUpdate latest = null; Map<MessageId, BdfDictionary> metadata = clientHelper.getMessageMetadataAsDictionary(txn, g); for (Entry<MessageId, BdfDictionary> e : metadata.entrySet()) { BdfDictionary meta = e.getValue(); if (meta.getString("transportId").equals(t.getString()) && meta.getBoolean("local") == local) { - long version = meta.getLong("version"); - if (latest == null) { - latest = new LatestUpdate(e.getKey(), version); - } else if (version > latest.version) { - // This update is newer - delete the previous one - if (local) { - db.removeMessage(txn, latest.messageId); - } else { - db.deleteMessage(txn, latest.messageId); - db.deleteMessageMetadata(txn, latest.messageId); - } - latest = new LatestUpdate(e.getKey(), version); - } else { - // We've already found a newer update - delete this one - if (local) { - db.removeMessage(txn, e.getKey()); - } else { - db.deleteMessage(txn, e.getKey()); - db.deleteMessageMetadata(txn, e.getKey()); - } - } + return new LatestUpdate(e.getKey(), meta.getLong("version")); } } - return latest; + return null; } private TransportProperties parseProperties(BdfList message) 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 ee3a2643bc987088b78784489a84afd6fc884be2..c2f8dc044ce634be1e760c7ece3f841a6e243717 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 @@ -215,6 +215,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { long timestamp = 123456789; Message message = getMessage(contactGroupId, timestamp); Metadata meta = new Metadata(); + // Version 4 is being delivered BdfDictionary metaDictionary = BdfDictionary.of( new BdfEntry("transportId", "foo"), new BdfEntry("version", 4), @@ -222,19 +223,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { ); Map<MessageId, BdfDictionary> messageMetadata = new LinkedHashMap<>(); - // Old remote updates for the same transport should be deleted - MessageId fooVersion2 = new MessageId(getRandomId()); - messageMetadata.put(fooVersion2, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 2), - new BdfEntry("local", false) - )); - MessageId fooVersion1 = new MessageId(getRandomId()); - messageMetadata.put(fooVersion1, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 1), - new BdfEntry("local", false) - )); + // 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"), @@ -248,11 +237,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroupId); will(returnValue(messageMetadata)); - // Versions 1-3 should be deleted - oneOf(db).deleteMessage(txn, fooVersion1); - oneOf(db).deleteMessageMetadata(txn, fooVersion1); - oneOf(db).deleteMessage(txn, fooVersion2); - oneOf(db).deleteMessageMetadata(txn, fooVersion2); + // The previous update (version 3) should be deleted oneOf(db).deleteMessage(txn, fooVersion3); oneOf(db).deleteMessageMetadata(txn, fooVersion3); }}); @@ -268,6 +253,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { long timestamp = 123456789; Message message = getMessage(contactGroupId, timestamp); Metadata meta = new Metadata(); + // Version 3 is being delivered BdfDictionary metaDictionary = BdfDictionary.of( new BdfEntry("transportId", "foo"), new BdfEntry("version", 3), @@ -275,19 +261,6 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { ); Map<MessageId, BdfDictionary> messageMetadata = new LinkedHashMap<>(); - // Old remote updates for the same transport should be deleted - MessageId fooVersion2 = new MessageId(getRandomId()); - messageMetadata.put(fooVersion2, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 2), - new BdfEntry("local", false) - )); - MessageId fooVersion1 = new MessageId(getRandomId()); - messageMetadata.put(fooVersion1, BdfDictionary.of( - new BdfEntry("transportId", "foo"), - new BdfEntry("version", 1), - new BdfEntry("local", false) - )); // A newer remote update for the same transport should not be deleted MessageId fooVersion4 = new MessageId(getRandomId()); messageMetadata.put(fooVersion4, BdfDictionary.of( @@ -302,11 +275,6 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroupId); will(returnValue(messageMetadata)); - // Versions 1 and 2 should be deleted, version 4 should not - oneOf(db).deleteMessage(txn, fooVersion1); - oneOf(db).deleteMessageMetadata(txn, fooVersion1); - oneOf(db).deleteMessage(txn, fooVersion2); - oneOf(db).deleteMessageMetadata(txn, fooVersion2); // The update being delivered (version 3) should be deleted oneOf(db).deleteMessage(txn, message.getId()); oneOf(db).deleteMessageMetadata(txn, message.getId()); @@ -343,7 +311,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { @Test public void testReturnsLatestLocalProperties() throws Exception { - Transaction txn = new Transaction(null, false); + Transaction txn = new Transaction(null, true); expectGetLocalProperties(txn); @@ -357,7 +325,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { @Test public void testReturnsEmptyPropertiesIfNoLocalPropertiesAreFound() throws Exception { - Transaction txn = new Transaction(null, false); + Transaction txn = new Transaction(null, true); Map<MessageId, BdfDictionary> messageMetadata = new LinkedHashMap<>(); // A local update for another transport should be ignored @@ -369,7 +337,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { )); context.checking(new Expectations() {{ - oneOf(db).startTransaction(false); + oneOf(db).startTransaction(true); will(returnValue(txn)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, localGroup.getId()); @@ -384,7 +352,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { @Test public void testReturnsLocalProperties() throws Exception { - Transaction txn = new Transaction(null, false); + Transaction txn = new Transaction(null, true); Map<MessageId, BdfDictionary> messageMetadata = new LinkedHashMap<>(); // A local update for another transport should be ignored @@ -404,7 +372,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { BdfList fooUpdate = BdfList.of("foo", 1, fooPropertiesDict); context.checking(new Expectations() {{ - oneOf(db).startTransaction(false); + oneOf(db).startTransaction(true); will(returnValue(txn)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, localGroup.getId()); @@ -423,7 +391,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { @Test public void testReturnsRemotePropertiesOrEmptyProperties() throws Exception { - Transaction txn = new Transaction(null, false); + Transaction txn = new Transaction(null, true); Contact contact1 = getContact(false); Contact contact2 = getContact(true); Contact contact3 = getContact(true); @@ -457,7 +425,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { BdfList fooUpdate = BdfList.of("foo", 1, fooPropertiesDict); context.checking(new Expectations() {{ - oneOf(db).startTransaction(false); + oneOf(db).startTransaction(true); will(returnValue(txn)); oneOf(db).getContacts(txn); will(returnValue(contacts)); @@ -638,28 +606,14 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { return new Message(messageId, g, timestamp, raw); } - private void expectGetLocalProperties(Transaction txn) - throws Exception { - Map<MessageId, BdfDictionary> messageMetadata = - new LinkedHashMap<>(); - // The only update for transport "foo" should be returned + private void expectGetLocalProperties(Transaction txn) throws Exception { + Map<MessageId, BdfDictionary> messageMetadata = new LinkedHashMap<>(); + // 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) )); - // An old update for transport "bar" should be deleted - MessageId barVersion2 = new MessageId(getRandomId()); - messageMetadata.put(barVersion2, BdfDictionary.of( - new BdfEntry("transportId", "bar"), - new BdfEntry("version", 2) - )); - // An even older update for transport "bar" should be deleted - MessageId barVersion1 = new MessageId(getRandomId()); - messageMetadata.put(barVersion1, BdfDictionary.of( - new BdfEntry("transportId", "bar"), - new BdfEntry("version", 1) - )); // The latest update for transport "bar" should be returned MessageId barVersion3 = new MessageId(getRandomId()); messageMetadata.put(barVersion3, BdfDictionary.of( @@ -674,8 +628,6 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { oneOf(clientHelper).getMessageMetadataAsDictionary(txn, localGroup.getId()); will(returnValue(messageMetadata)); - oneOf(db).removeMessage(txn, barVersion1); - oneOf(db).removeMessage(txn, barVersion2); // Retrieve and parse the latest local properties oneOf(clientHelper).getMessageAsList(txn, fooVersion999); will(returnValue(fooUpdate));