Keep last sent clientSupports on record, sending update only if changed

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<MailboxVersion> parseMailboxVersionList(BdfList bdfList)
+			throws FormatException;
+
 	/**
 	 * Retrieves the contact ID from the group metadata of the given contact
 	 * group.

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;
 

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<MailboxVersion> clientSupportsList =
-				getMailboxVersionList(clientSupports);
+				parseMailboxVersionList(clientSupports);
 		List<MailboxVersion> 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<MailboxVersion> getMailboxVersionList(BdfList bdfList)
+	@Override
+	public List<MailboxVersion> parseMailboxVersionList(BdfList bdfList)
 			throws FormatException {
 		List<MailboxVersion> list = new ArrayList<>();
 		for (int i = 0; i < bdfList.size(); i++) {

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();
 		}
 	}
 

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<MessageId, BdfDictionary> 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<MessageId, BdfDictionary> 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<Contact> contacts = singletonList(contact);
 		Group contactGroup = getGroup(CLIENT_ID, MAJOR_VERSION);
 
 		Map<MessageId, BdfDictionary> 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<MessageId, BdfDictionary> 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);
 	}