From 4467f9e260548cf05bd0f398907fcca9f76ab6da Mon Sep 17 00:00:00 2001 From: Daniel Lublin Date: Wed, 18 May 2022 14:13:01 +0200 Subject: [PATCH] Keep last sent clientSupports on record, sending update only if changed --- .../bramble/api/client/ClientHelper.java | 5 ++ .../api/mailbox/MailboxUpdateManager.java | 6 ++ .../bramble/client/ClientHelperImpl.java | 7 +- .../mailbox/MailboxUpdateManagerImpl.java | 53 ++++++++++----- .../mailbox/MailboxUpdateManagerImplTest.java | 66 +++++++++++-------- 5 files changed, 90 insertions(+), 47 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java b/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java index 940ee2f13..da9ee3163 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java @@ -10,6 +10,7 @@ import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.mailbox.MailboxUpdate; +import org.briarproject.bramble.api.mailbox.MailboxVersion; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.properties.TransportProperties; @@ -19,6 +20,7 @@ import org.briarproject.bramble.api.sync.MessageId; import java.security.GeneralSecurityException; import java.util.Collection; +import java.util.List; import java.util.Map; @NotNullByDefault @@ -134,6 +136,9 @@ public interface ClientHelper { BdfList serverSupports, BdfDictionary properties) throws FormatException; + List parseMailboxVersionList(BdfList bdfList) + throws FormatException; + /** * Retrieves the contact ID from the group metadata of the given contact * group. diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxUpdateManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxUpdateManager.java index 92c0ebc78..0e101a408 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxUpdateManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxUpdateManager.java @@ -57,6 +57,12 @@ public interface MailboxUpdateManager { */ String MSG_KEY_LOCAL = "local"; + /** + * Key in the client's local group for storing the clientSupports list that + * was last sent out. + */ + String GROUP_KEY_SENT_CLIENT_SUPPORTS = "sentClientSupports"; + MailboxUpdate getLocalUpdate(Transaction txn, ContactId c) throws DbException; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java index 7baf56f56..584986c0f 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java @@ -419,9 +419,9 @@ class ClientHelperImpl implements ClientHelper { BdfList serverSupports, BdfDictionary properties) throws FormatException { List clientSupportsList = - getMailboxVersionList(clientSupports); + parseMailboxVersionList(clientSupports); List serverSupportsList = - getMailboxVersionList(serverSupports); + parseMailboxVersionList(serverSupports); // We must always learn what Mailbox API version(s) the client supports if (clientSupports.isEmpty()) { @@ -460,7 +460,8 @@ class ClientHelperImpl implements ClientHelper { new MailboxFolderId(inboxId), new MailboxFolderId(outboxId)); } - private List getMailboxVersionList(BdfList bdfList) + @Override + public List parseMailboxVersionList(BdfList bdfList) throws FormatException { List list = new ArrayList<>(); for (int i = 0; i < bdfList.size(); i++) { diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImpl.java index 7511e5598..5627bdd44 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImpl.java @@ -8,6 +8,7 @@ import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.ContactManager.ContactHook; import org.briarproject.bramble.api.crypto.CryptoComponent; 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.DatabaseComponent; @@ -84,27 +85,47 @@ class MailboxUpdateManagerImpl implements MailboxUpdateManager, @Override public void onDatabaseOpened(Transaction txn) throws DbException { if (db.containsGroup(txn, localGroup.getId())) { + try { + BdfDictionary meta = clientHelper.getGroupMetadataAsDictionary( + txn, localGroup.getId()); + BdfList sent = meta.getList(GROUP_KEY_SENT_CLIENT_SUPPORTS); + if (clientHelper.parseMailboxVersionList(sent) + .equals(CLIENT_SUPPORTS)) { + return; + } + } catch (FormatException e) { + throw new DbException(); + } + // Our current clientSupports list has changed compared to what we + // last sent out. for (Contact c : db.getContacts(txn)) { MailboxUpdate latest = getLocalUpdate(txn, c.getId()); - if (!latest.getClientSupports().equals(CLIENT_SUPPORTS)) { - MailboxUpdate updated; - if (latest.hasMailbox()) { - updated = new MailboxUpdateWithMailbox( - (MailboxUpdateWithMailbox) latest, - CLIENT_SUPPORTS); - } else { - updated = new MailboxUpdate(CLIENT_SUPPORTS); - } - Group g = getContactGroup(c); - storeMessageReplaceLatest(txn, g.getId(), updated); + MailboxUpdate updated; + if (latest.hasMailbox()) { + updated = new MailboxUpdateWithMailbox( + (MailboxUpdateWithMailbox) latest, + CLIENT_SUPPORTS); + } else { + updated = new MailboxUpdate(CLIENT_SUPPORTS); } + Group g = getContactGroup(c); + storeMessageReplaceLatest(txn, g.getId(), updated); + } + } else { + db.addGroup(txn, localGroup); + // Set things up for any pre-existing contacts + for (Contact c : db.getContacts(txn)) { + addingContact(txn, c); } - return; } - db.addGroup(txn, localGroup); - // Set things up for any pre-existing contacts - for (Contact c : db.getContacts(txn)) { - addingContact(txn, c); + + try { + BdfDictionary meta = BdfDictionary.of(new BdfEntry( + GROUP_KEY_SENT_CLIENT_SUPPORTS, + encodeSupportsList(CLIENT_SUPPORTS))); + clientHelper.mergeGroupMetadata(txn, localGroup.getId(), meta); + } catch (FormatException e) { + throw new DbException(); } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImplTest.java index d203f83b1..17e991ee7 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImplTest.java @@ -35,6 +35,7 @@ import java.util.Map; import static java.util.Collections.singletonList; import static org.briarproject.bramble.api.mailbox.MailboxUpdateManager.CLIENT_ID; +import static org.briarproject.bramble.api.mailbox.MailboxUpdateManager.GROUP_KEY_SENT_CLIENT_SUPPORTS; import static org.briarproject.bramble.api.mailbox.MailboxUpdateManager.MAJOR_VERSION; import static org.briarproject.bramble.api.mailbox.MailboxUpdateManager.MSG_KEY_LOCAL; import static org.briarproject.bramble.api.mailbox.MailboxUpdateManager.MSG_KEY_VERSION; @@ -132,6 +133,9 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { Contact contact = getContact(); Group contactGroup = getGroup(CLIENT_ID, MAJOR_VERSION); Map messageMetadata = new LinkedHashMap<>(); + BdfDictionary sentDict = BdfDictionary.of(new BdfEntry( + GROUP_KEY_SENT_CLIENT_SUPPORTS, + realClientSupports)); context.checking(new Expectations() {{ oneOf(db).containsGroup(txn, localGroup.getId()); @@ -139,6 +143,8 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { oneOf(db).addGroup(txn, localGroup); oneOf(db).getContacts(txn); will(returnValue(singletonList(contact))); + + // addingContact() oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, MAJOR_VERSION, contact); will(returnValue(contactGroup)); @@ -160,6 +166,9 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { will(returnValue(messageMetadata)); expectStoreMessage(txn, contactGroup.getId(), 1, realClientSupports, emptyServerSupports, emptyPropsDict, true); + + oneOf(clientHelper).mergeGroupMetadata(txn, localGroup.getId(), + sentDict); }}); MailboxUpdateManagerImpl t = createInstance(); @@ -173,6 +182,9 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { Contact contact = getContact(); Group contactGroup = getGroup(CLIENT_ID, MAJOR_VERSION); Map messageMetadata = new LinkedHashMap<>(); + BdfDictionary sentDict = BdfDictionary.of(new BdfEntry( + GROUP_KEY_SENT_CLIENT_SUPPORTS, + realClientSupports)); context.checking(new Expectations() {{ oneOf(db).containsGroup(txn, localGroup.getId()); @@ -180,6 +192,8 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { oneOf(db).addGroup(txn, localGroup); oneOf(db).getContacts(txn); will(returnValue(singletonList(contact))); + + // addingContact() oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, MAJOR_VERSION, contact); will(returnValue(contactGroup)); @@ -207,6 +221,9 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { will(returnValue(messageMetadata)); expectStoreMessage(txn, contactGroup.getId(), 1, realClientSupports, someServerSupports, propsDict, true); + + oneOf(clientHelper).mergeGroupMetadata(txn, localGroup.getId(), + sentDict); }}); MailboxUpdateManagerImpl t = createInstance(); @@ -214,18 +231,27 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { } @Test - public void testChecksForCurrentClientSupportsInLatestUpdateOnSecondStartup() + public void testChecksLatestSentClientSupportsOnSecondStartup() throws Exception { Transaction txn = new Transaction(null, false); Contact contact = getContact(); - List contacts = singletonList(contact); Group contactGroup = getGroup(CLIENT_ID, MAJOR_VERSION); Map emptyMessageMetadata = new LinkedHashMap<>(); + BdfDictionary sentDict = BdfDictionary.of(new BdfEntry( + GROUP_KEY_SENT_CLIENT_SUPPORTS, + realClientSupports)); context.checking(new Expectations() {{ + oneOf(db).containsGroup(txn, localGroup.getId()); + will(returnValue(false)); + oneOf(db).addGroup(txn, localGroup); + oneOf(db).getContacts(txn); + will(returnValue(singletonList(contact))); + + // addingContact() oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, MAJOR_VERSION, contact); will(returnValue(contactGroup)); @@ -247,41 +273,25 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase { will(returnValue(emptyMessageMetadata)); expectStoreMessage(txn, contactGroup.getId(), 1, realClientSupports, emptyServerSupports, emptyPropsDict, true); + + oneOf(clientHelper).mergeGroupMetadata(txn, localGroup.getId(), + sentDict); }}); MailboxUpdateManagerImpl t = createInstance(); - t.addingContact(txn, contact); - - Message message = getMessage(contactGroup.getId()); - BdfList body = BdfList.of(1, realClientSupports, emptyServerSupports, - emptyPropsDict); - BdfDictionary metaDictionary = BdfDictionary.of( - new BdfEntry(MSG_KEY_VERSION, 1), - new BdfEntry(MSG_KEY_LOCAL, true) - ); - Map messageMetadata = new LinkedHashMap<>(); - messageMetadata.put(message.getId(), metaDictionary); + t.onDatabaseOpened(txn); context.checking(new Expectations() {{ oneOf(db).containsGroup(txn, localGroup.getId()); will(returnValue(true)); - oneOf(db).getContacts(txn); - will(returnValue(contacts)); - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory) - .createContactGroup(CLIENT_ID, MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(clientHelper).getMessageMetadataAsDictionary(txn, - contactGroup.getId()); - will(returnValue(messageMetadata)); - oneOf(clientHelper).getMessageAsList(txn, message.getId()); - will(returnValue(body)); - oneOf(clientHelper).parseAndValidateMailboxUpdate( - realClientSupports, emptyServerSupports, emptyPropsDict); - will(returnValue(updateNoMailbox)); + oneOf(clientHelper).getGroupMetadataAsDictionary(txn, + localGroup.getId()); + will(returnValue(sentDict)); + oneOf(clientHelper).parseMailboxVersionList(realClientSupports); + will(returnValue(CLIENT_SUPPORTS)); }}); + t = createInstance(); t.onDatabaseOpened(txn); }