Merge branch '2228-more-mailbox-client-manager-preliminaries' into 'master'

More mailbox client manager preliminaries

See merge request briar/briar!1694

bramble-android/proguard-rules.txt

@@ -18,3 +18,7 @@
 -dontnote com.google.common.**
 
 -dontwarn com.fasterxml.jackson.databind.ext.Java7SupportImpl
+
+# Keep all Jackson-serialisable classes and their members
+-keep interface com.fasterxml.jackson.databind.annotation.JsonSerialize
+-keep @com.fasterxml.jackson.databind.annotation.JsonSerialize class * { *; }

bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java

@@ -349,13 +349,13 @@ public interface DatabaseComponent extends TransactionManager {
 			Metadata query) throws DbException;
 
 	/**
-	 * Returns the IDs of some messages received from the given contact that
+	 * Returns the IDs of all messages received from the given contact that
-	 * need to be acknowledged, up to the given number of messages.
+	 * need to be acknowledged.
 	 * <p/>
 	 * Read-only.
 	 */
-	Collection<MessageId> getMessagesToAck(Transaction txn, ContactId c,
+	Collection<MessageId> getMessagesToAck(Transaction txn, ContactId c)
-			int maxMessages) throws DbException;
+			throws DbException;
 
 	/**
 	 * Returns the IDs of some messages that are eligible to be sent to the
@@ -492,6 +492,8 @@ public interface DatabaseComponent extends TransactionManager {
 	 * Returns the message with the given ID for transmission to the given
 	 * contact over a transport with the given maximum latency. Returns null
 	 * if the message is no longer visible to the contact.
+	 * <p/>
+	 * Read-only if {@code markAsSent} is false.
 	 *
 	 * @param markAsSent True if the message should be marked as sent.
 	 * If false it can be marked as sent by calling

bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxProperties.java

@@ -85,7 +85,8 @@ public class MailboxProperties {
 					onion.equals(m.onion) &&
 					authToken.equals(m.authToken) &&
 					NullSafety.equals(inboxId, m.inboxId) &&
-					NullSafety.equals(outboxId, m.outboxId);
+					NullSafety.equals(outboxId, m.outboxId) &&
+					serverSupports.equals(m.serverSupports);
 		}
 		return false;
 	}

bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxStatus.java

@@ -67,6 +67,13 @@ public class MailboxStatus {
 		return attemptsSinceSuccess;
 	}
 
+	/**
+	 * Returns the mailbox's supported API versions.
+	 */
+	public List<MailboxVersion> getServerSupports() {
+		return serverSupports;
+	}
+
 	/**
 	 * @return true if this status indicates a problem with the mailbox.
 	 */

bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/event/MailboxUpdateSentEvent.java

@@ -0,0 +1,39 @@
+package org.briarproject.bramble.api.mailbox.event;
+
+import org.briarproject.bramble.api.contact.ContactId;
+import org.briarproject.bramble.api.event.Event;
+import org.briarproject.bramble.api.mailbox.MailboxUpdate;
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+
+import javax.annotation.concurrent.Immutable;
+
+/**
+ * An event that is broadcast when a mailbox update is sent to a contact.
+ * <p>
+ * Note that this event is not broadcast when a mailbox is paired or
+ * unpaired, although updates are sent to all contacts in those situations.
+ *
+ * @see MailboxPairedEvent
+ * @see MailboxUnpairedEvent
+ */
+@Immutable
+@NotNullByDefault
+public class MailboxUpdateSentEvent extends Event {
+
+	private final ContactId contactId;
+	private final MailboxUpdate mailboxUpdate;
+
+	public MailboxUpdateSentEvent(ContactId contactId,
+			MailboxUpdate mailboxUpdate) {
+		this.contactId = contactId;
+		this.mailboxUpdate = mailboxUpdate;
+	}
+
+	public ContactId getContactId() {
+		return contactId;
+	}
+
+	public MailboxUpdate getMailboxUpdate() {
+		return mailboxUpdate;
+	}
+}

bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java

@@ -52,6 +52,7 @@ import java.util.Map.Entry;
 import javax.annotation.concurrent.Immutable;
 import javax.inject.Inject;
 
+import static java.util.Collections.sort;
 import static org.briarproject.bramble.api.client.ContactGroupConstants.GROUP_KEY_CONTACT_ID;
 import static org.briarproject.bramble.api.identity.Author.FORMAT_VERSION;
 import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH;
@@ -474,6 +475,8 @@ class ClientHelperImpl implements ClientHelper {
 			list.add(new MailboxVersion(element.getLong(0).intValue(),
 					element.getLong(1).intValue()));
 		}
+		// Sort the list of versions for easier comparison
+		sort(list);
 		return list;
 	}
 

bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java

@@ -620,11 +620,11 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 
 	@Override
 	public Collection<MessageId> getMessagesToAck(Transaction transaction,
-			ContactId c, int maxMessages) throws DbException {
+			ContactId c) throws DbException {
 		T txn = unbox(transaction);
 		if (!db.containsContact(txn, c))
 			throw new NoSuchContactException();
-		return db.getMessagesToAck(txn, c, maxMessages);
+		return db.getMessagesToAck(txn, c, Integer.MAX_VALUE);
 	}
 
 	@Override
@@ -746,7 +746,9 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 	public Message getMessageToSend(Transaction transaction, ContactId c,
 			MessageId m, long maxLatency, boolean markAsSent)
 			throws DbException {
-		if (transaction.isReadOnly()) throw new IllegalArgumentException();
+		if (markAsSent && transaction.isReadOnly()) {
+			throw new IllegalArgumentException();
+		}
 		T txn = unbox(transaction);
 		if (!db.containsContact(txn, c))
 			throw new NoSuchContactException();
@@ -1097,11 +1099,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 		T txn = unbox(transaction);
 		if (!db.containsContact(txn, c))
 			throw new NoSuchContactException();
-		List<MessageId> visible = new ArrayList<>(acked.size());
+		db.lowerAckFlag(txn, c, acked);
-		for (MessageId m : acked) {
-			if (db.containsVisibleMessage(txn, c, m)) visible.add(m);
-		}
-		db.lowerAckFlag(txn, c, visible);
 	}
 
 	@Override

bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxConnectivityChecker.java

@@ -5,13 +5,20 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.system.Clock;
 import org.briarproject.bramble.mailbox.MailboxApi.ApiException;
 
+import java.util.logging.Logger;
+
 import javax.annotation.concurrent.ThreadSafe;
 import javax.inject.Inject;
 
+import static java.util.logging.Logger.getLogger;
+
 @ThreadSafe
 @NotNullByDefault
 class ContactMailboxConnectivityChecker extends ConnectivityCheckerImpl {
 
+	private static final Logger LOG =
+			getLogger(ContactMailboxConnectivityChecker.class.getName());
+
 	private final MailboxApi mailboxApi;
 
 	@Inject
@@ -25,7 +32,9 @@ class ContactMailboxConnectivityChecker extends ConnectivityCheckerImpl {
 	ApiCall createConnectivityCheckTask(MailboxProperties properties) {
 		if (properties.isOwner()) throw new IllegalArgumentException();
 		return new SimpleApiCall(() -> {
+			LOG.info("Checking connectivity of contact's mailbox");
 			if (!mailboxApi.checkStatus(properties)) throw new ApiException();
+			LOG.info("Contact's mailbox is reachable");
 			// Call the observers and cache the result
 			onConnectivityCheckSucceeded(clock.currentTimeMillis());
 		});

bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java

@@ -22,7 +22,6 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 
 import javax.inject.Inject;
@@ -35,6 +34,7 @@ import okhttp3.Response;
 import okhttp3.ResponseBody;
 
 import static com.fasterxml.jackson.databind.MapperFeature.BLOCK_UNSAFE_POLYMORPHIC_BASE_TYPES;
+import static java.util.Collections.sort;
 import static java.util.Objects.requireNonNull;
 import static okhttp3.internal.Util.EMPTY_REQUEST;
 import static org.briarproject.bramble.util.IoUtils.copyAndClose;
@@ -125,6 +125,8 @@ class MailboxApiImpl implements MailboxApi {
 			if (major < 0 || minor < 0) throw new ApiException();
 			serverSupports.add(new MailboxVersion(major, minor));
 		}
+		// Sort the list of versions for easier comparison
+		sort(serverSupports);
 		return serverSupports;
 	}
 
@@ -245,7 +247,7 @@ class MailboxApiImpl implements MailboxApi {
 				if (time < 1) throw new ApiException();
 				list.add(new MailboxFile(MailboxFileId.fromString(name), time));
 			}
-			Collections.sort(list);
+			sort(list);
 			return list;
 		} catch (JacksonException | InvalidMailboxIdException e) {
 			throw new ApiException();

bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImpl.java

@@ -27,6 +27,7 @@ import org.briarproject.bramble.api.mailbox.MailboxUpdateWithMailbox;
 import org.briarproject.bramble.api.mailbox.MailboxVersion;
 import org.briarproject.bramble.api.mailbox.event.MailboxPairedEvent;
 import org.briarproject.bramble.api.mailbox.event.MailboxUnpairedEvent;
+import org.briarproject.bramble.api.mailbox.event.MailboxUpdateSentEvent;
 import org.briarproject.bramble.api.mailbox.event.RemoteMailboxUpdateEvent;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.sync.Group;
@@ -114,6 +115,7 @@ class MailboxUpdateManagerImpl implements MailboxUpdateManager,
 				}
 				Group g = getContactGroup(c);
 				storeMessageReplaceLatest(txn, g.getId(), updated);
+				txn.attach(new MailboxUpdateSentEvent(c.getId(), updated));
 			}
 		} else {
 			db.addGroup(txn, localGroup);
@@ -146,14 +148,16 @@ class MailboxUpdateManagerImpl implements MailboxUpdateManager,
 		clientHelper.setContactId(txn, g.getId(), c.getId());
 		MailboxProperties ownProps =
 				mailboxSettingsManager.getOwnMailboxProperties(txn);
+		MailboxUpdate u;
 		if (ownProps != null) {
 			// We are paired, create and send props to the newly added contact
-			createAndSendUpdateWithMailbox(txn, c, ownProps.getServerSupports(),
+			u = createAndSendUpdateWithMailbox(txn, c,
-					ownProps.getOnion());
+					ownProps.getServerSupports(), ownProps.getOnion());
 		} else {
 			// Not paired, but we still want to get our clientSupports sent
-			sendUpdateNoMailbox(txn, c);
+			u = sendUpdateNoMailbox(txn, c);
 		}
+		txn.attach(new MailboxUpdateSentEvent(c.getId(), u));
 	}
 
 	@Override

bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUploadWorker.java

@@ -328,13 +328,13 @@ class MailboxUploadWorker implements MailboxWorker, ConnectivityObserver,
 		LOG.info("Uploading file");
 		mailboxApi.addFile(mailboxProperties, folderId, file);
 		markMessagesSentOrAcked(sessionRecord);
+		delete(file);
 		synchronized (lock) {
 			if (state != State.WRITING_UPLOADING) return;
 			state = State.CHECKING_FOR_DATA;
 			apiCall = null;
 			this.file = null;
 		}
-		delete(file);
 		checkForDataToSend();
 	}
 

bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxWorkerFactoryImpl.java

@@ -88,8 +88,10 @@ class MailboxWorkerFactoryImpl implements MailboxWorkerFactory {
 	public MailboxWorker createContactListWorkerForOwnMailbox(
 			ConnectivityChecker connectivityChecker,
 			MailboxProperties properties) {
-		return new OwnMailboxContactListWorker(ioExecutor, db, eventBus,
+		OwnMailboxContactListWorker worker = new OwnMailboxContactListWorker(
-				connectivityChecker, mailboxApiCaller, mailboxApi,
+				ioExecutor, db, eventBus, connectivityChecker, mailboxApiCaller,
-				mailboxUpdateManager, properties);
+				mailboxApi, mailboxUpdateManager, properties);
+		eventBus.addListener(worker);
+		return worker;
 	}
 }

bramble-core/src/main/java/org/briarproject/bramble/mailbox/OwnMailboxConnectivityChecker.java

@@ -59,6 +59,7 @@ class OwnMailboxConnectivityChecker extends ConnectivityCheckerImpl {
 	private boolean checkConnectivityAndStoreResult(
 			MailboxProperties properties) throws DbException {
 		try {
+			LOG.info("Checking whether own mailbox is reachable");
 			List<MailboxVersion> serverSupports =
 					mailboxApi.getServerSupports(properties);
 			LOG.info("Own mailbox is reachable");

bramble-core/src/main/java/org/briarproject/bramble/sync/MailboxOutgoingSession.java

@@ -14,7 +14,9 @@ import org.briarproject.bramble.api.sync.SyncRecordWriter;
 import org.briarproject.bramble.api.transport.StreamWriter;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 import java.util.logging.Logger;
 
 import javax.annotation.concurrent.ThreadSafe;
@@ -61,26 +63,32 @@ class MailboxOutgoingSession extends SimplexOutgoingSession {
 
 	@Override
 	void sendAcks() throws DbException, IOException {
-		while (!isInterrupted()) {
+		List<MessageId> idsToAck = loadMessageIdsToAck();
-			Collection<MessageId> idsToAck = loadMessageIdsToAck();
+		int idsSent = 0;
-			if (idsToAck.isEmpty()) break;
+		while (idsSent < idsToAck.size() && !isInterrupted()) {
-			recordWriter.writeAck(new Ack(idsToAck));
+			int idsRemaining = idsToAck.size() - idsSent;
-			sessionRecord.onAckSent(idsToAck);
+			long capacity = getRemainingCapacity();
+			long idCapacity =
+					(capacity - RECORD_HEADER_BYTES) / MessageId.LENGTH;
+			if (idCapacity == 0) break; // Out of capacity
+			int idsInRecord = (int) min(idCapacity, MAX_MESSAGE_IDS);
+			int idsToSend = min(idsRemaining, idsInRecord);
+			List<MessageId> acked =
+					idsToAck.subList(idsSent, idsSent + idsToSend);
+			recordWriter.writeAck(new Ack(acked));
+			sessionRecord.onAckSent(acked);
 			LOG.info("Sent ack");
+			idsSent += idsToSend;
 		}
 	}
 
-	private Collection<MessageId> loadMessageIdsToAck() throws DbException {
+	private List<MessageId> loadMessageIdsToAck() throws DbException {
-		long idCapacity = (getRemainingCapacity() - RECORD_HEADER_BYTES)
-				/ MessageId.LENGTH;
-		if (idCapacity <= 0) return emptyList(); // Out of capacity
-		int maxMessageIds = (int) min(idCapacity, MAX_MESSAGE_IDS);
 		Collection<MessageId> ids = db.transactionWithResult(true, txn ->
-				db.getMessagesToAck(txn, contactId, maxMessageIds));
+				db.getMessagesToAck(txn, contactId));
 		if (LOG.isLoggable(INFO)) {
 			LOG.info(ids.size() + " messages to ack");
 		}
-		return ids;
+		return new ArrayList<>(ids);
 	}
 
 	private long getRemainingCapacity() {

bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java

@@ -389,7 +389,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase {
 
 		try {
 			db.transaction(true, transaction ->
-					db.getMessagesToAck(transaction, contactId, 123));
+					db.getMessagesToAck(transaction, contactId));
 			fail();
 		} catch (NoSuchContactException expected) {
 			// Expected
@@ -1396,14 +1396,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase {
 			will(returnValue(txn));
 			oneOf(database).containsContact(txn, contactId);
 			will(returnValue(true));
-			// First message is still visible to the contact - flag lowered
+			oneOf(database).lowerAckFlag(txn, contactId, acked);
-			oneOf(database).containsVisibleMessage(txn, contactId, messageId);
-			will(returnValue(true));
-			// Second message is no longer visible - flag not lowered
-			oneOf(database).containsVisibleMessage(txn, contactId, messageId1);
-			will(returnValue(false));
-			oneOf(database)
-					.lowerAckFlag(txn, contactId, singletonList(messageId));
 			oneOf(database).commitTransaction(txn);
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, eventBus,

bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxUpdateManagerImplTest.java

@@ -19,6 +19,7 @@ import org.briarproject.bramble.api.mailbox.MailboxUpdateWithMailbox;
 import org.briarproject.bramble.api.mailbox.MailboxVersion;
 import org.briarproject.bramble.api.mailbox.event.MailboxPairedEvent;
 import org.briarproject.bramble.api.mailbox.event.MailboxUnpairedEvent;
+import org.briarproject.bramble.api.mailbox.event.MailboxUpdateSentEvent;
 import org.briarproject.bramble.api.mailbox.event.RemoteMailboxUpdateEvent;
 import org.briarproject.bramble.api.sync.Group;
 import org.briarproject.bramble.api.sync.GroupId;
@@ -185,6 +186,9 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase {
 
 		MailboxUpdateManagerImpl t = createInstance(someClientSupportsList);
 		t.onDatabaseOpened(txn);
+
+		MailboxUpdateSentEvent e = getEvent(txn, MailboxUpdateSentEvent.class);
+		assertNoMailboxPropertiesSent(e, someClientSupportsList);
 	}
 
 	@Test
@@ -238,11 +242,15 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase {
 
 		MailboxUpdateManagerImpl t = createInstance(someClientSupportsList);
 		t.onDatabaseOpened(txn);
+
+		MailboxUpdateSentEvent e = getEvent(txn, MailboxUpdateSentEvent.class);
+		assertMailboxPropertiesSent(e, someClientSupportsList);
 	}
 
 	@Test
 	public void testUnchangedClientSupportsOnSecondStartup() throws Exception {
-		Transaction txn = new Transaction(null, false);
+		Transaction txn1 = new Transaction(null, false);
+		Transaction txn2 = new Transaction(null, false);
 
 		Map<MessageId, BdfDictionary> emptyMessageMetadata =
 				new LinkedHashMap<>();
@@ -251,46 +259,49 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase {
 				someClientSupports));
 
 		context.checking(new Expectations() {{
-			oneOf(db).containsGroup(txn, localGroup.getId());
+			oneOf(db).containsGroup(txn1, localGroup.getId());
 			will(returnValue(false));
-			oneOf(db).addGroup(txn, localGroup);
+			oneOf(db).addGroup(txn1, localGroup);
-			oneOf(db).getContacts(txn);
+			oneOf(db).getContacts(txn1);
 			will(returnValue(singletonList(contact)));
 
 			// addingContact()
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					MAJOR_VERSION, contact);
 			will(returnValue(contactGroup));
-			oneOf(db).addGroup(txn, contactGroup);
+			oneOf(db).addGroup(txn1, contactGroup);
-			oneOf(clientVersioningManager).getClientVisibility(txn,
+			oneOf(clientVersioningManager).getClientVisibility(txn1,
 					contact.getId(), CLIENT_ID, MAJOR_VERSION);
 			will(returnValue(SHARED));
-			oneOf(db).setGroupVisibility(txn, contact.getId(),
+			oneOf(db).setGroupVisibility(txn1, contact.getId(),
 					contactGroupId, SHARED);
-			oneOf(clientHelper).setContactId(txn, contactGroupId,
+			oneOf(clientHelper).setContactId(txn1, contactGroupId,
 					contact.getId());
-			oneOf(mailboxSettingsManager).getOwnMailboxProperties(txn);
+			oneOf(mailboxSettingsManager).getOwnMailboxProperties(txn1);
 			will(returnValue(null));
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					MAJOR_VERSION, contact);
 			will(returnValue(contactGroup));
-			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
+			oneOf(clientHelper).getMessageMetadataAsDictionary(txn1,
 					contactGroupId);
 			will(returnValue(emptyMessageMetadata));
-			expectStoreMessage(txn, contactGroupId, 1, someClientSupports,
+			expectStoreMessage(txn1, contactGroupId, 1, someClientSupports,
 					emptyServerSupports, emptyPropsDict);
 
-			oneOf(clientHelper).mergeGroupMetadata(txn, localGroup.getId(),
+			oneOf(clientHelper).mergeGroupMetadata(txn1, localGroup.getId(),
 					sentDict);
 		}});
 
 		MailboxUpdateManagerImpl t = createInstance(someClientSupportsList);
-		t.onDatabaseOpened(txn);
+		t.onDatabaseOpened(txn1);
+
+		MailboxUpdateSentEvent e = getEvent(txn1, MailboxUpdateSentEvent.class);
+		assertNoMailboxPropertiesSent(e, someClientSupportsList);
 
 		context.checking(new Expectations() {{
-			oneOf(db).containsGroup(txn, localGroup.getId());
+			oneOf(db).containsGroup(txn2, localGroup.getId());
 			will(returnValue(true));
-			oneOf(clientHelper).getGroupMetadataAsDictionary(txn,
+			oneOf(clientHelper).getGroupMetadataAsDictionary(txn2,
 					localGroup.getId());
 			will(returnValue(sentDict));
 			oneOf(clientHelper).parseMailboxVersionList(someClientSupports);
@@ -298,13 +309,16 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase {
 		}});
 
 		t = createInstance(someClientSupportsList);
-		t.onDatabaseOpened(txn);
+		t.onDatabaseOpened(txn2);
+
+		assertFalse(hasEvent(txn2, MailboxUpdateSentEvent.class));
 	}
 
 	@Test
 	public void testSendsUpdateWhenClientSupportsChangedOnSecondStartup()
 			throws Exception {
-		Transaction txn = new Transaction(null, false);
+		Transaction txn1 = new Transaction(null, false);
+		Transaction txn2 = new Transaction(null, false);
 
 		Map<MessageId, BdfDictionary> emptyMessageMetadata =
 				new LinkedHashMap<>();
@@ -313,41 +327,45 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase {
 				someClientSupports));
 
 		context.checking(new Expectations() {{
-			oneOf(db).containsGroup(txn, localGroup.getId());
+			oneOf(db).containsGroup(txn1, localGroup.getId());
 			will(returnValue(false));
-			oneOf(db).addGroup(txn, localGroup);
+			oneOf(db).addGroup(txn1, localGroup);
-			oneOf(db).getContacts(txn);
+			oneOf(db).getContacts(txn1);
 			will(returnValue(singletonList(contact)));
 
 			// addingContact()
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					MAJOR_VERSION, contact);
 			will(returnValue(contactGroup));
-			oneOf(db).addGroup(txn, contactGroup);
+			oneOf(db).addGroup(txn1, contactGroup);
-			oneOf(clientVersioningManager).getClientVisibility(txn,
+			oneOf(clientVersioningManager).getClientVisibility(txn1,
 					contact.getId(), CLIENT_ID, MAJOR_VERSION);
 			will(returnValue(SHARED));
-			oneOf(db).setGroupVisibility(txn, contact.getId(),
+			oneOf(db).setGroupVisibility(txn1, contact.getId(),
 					contactGroupId, SHARED);
-			oneOf(clientHelper).setContactId(txn, contactGroupId,
+			oneOf(clientHelper).setContactId(txn1, contactGroupId,
 					contact.getId());
-			oneOf(mailboxSettingsManager).getOwnMailboxProperties(txn);
+			oneOf(mailboxSettingsManager).getOwnMailboxProperties(txn1);
 			will(returnValue(null));
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					MAJOR_VERSION, contact);
 			will(returnValue(contactGroup));
-			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
+			oneOf(clientHelper).getMessageMetadataAsDictionary(txn1,
 					contactGroupId);
 			will(returnValue(emptyMessageMetadata));
-			expectStoreMessage(txn, contactGroupId, 1, someClientSupports,
+			expectStoreMessage(txn1, contactGroupId, 1, someClientSupports,
 					emptyServerSupports, emptyPropsDict);
 
-			oneOf(clientHelper).mergeGroupMetadata(txn, localGroup.getId(),
+			oneOf(clientHelper).mergeGroupMetadata(txn1, localGroup.getId(),
 					sentDict);
 		}});
 
 		MailboxUpdateManagerImpl t = createInstance(someClientSupportsList);
-		t.onDatabaseOpened(txn);
+		t.onDatabaseOpened(txn1);
+
+		MailboxUpdateSentEvent e1 =
+				getEvent(txn1, MailboxUpdateSentEvent.class);
+		assertNoMailboxPropertiesSent(e1, someClientSupportsList);
 
 		BdfDictionary metaDictionary = BdfDictionary.of(
 				new BdfEntry(MSG_KEY_VERSION, 1),
@@ -356,58 +374,62 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase {
 		Map<MessageId, BdfDictionary> messageMetadata = new LinkedHashMap<>();
 		MessageId messageId = new MessageId(getRandomId());
 		messageMetadata.put(messageId, metaDictionary);
-		BdfList body = BdfList.of(1, someClientSupports, someServerSupports,
+		BdfList oldBody = BdfList.of(1, someClientSupports, emptyServerSupports,
-				propsDict);
+				emptyPropsDict);
 		BdfDictionary newerSentDict = BdfDictionary.of(new BdfEntry(
 				GROUP_KEY_SENT_CLIENT_SUPPORTS,
 				newerClientSupports));
 
 		context.checking(new Expectations() {{
-			oneOf(db).containsGroup(txn, localGroup.getId());
+			oneOf(db).containsGroup(txn2, localGroup.getId());
 			will(returnValue(true));
 
 			// Find out that we are now on newerClientSupportsList
-			oneOf(clientHelper).getGroupMetadataAsDictionary(txn,
+			oneOf(clientHelper).getGroupMetadataAsDictionary(txn2,
 					localGroup.getId());
 			will(returnValue(sentDict));
 			oneOf(clientHelper).parseMailboxVersionList(someClientSupports);
 			will(returnValue(someClientSupportsList));
 
-			oneOf(db).getContacts(txn);
+			oneOf(db).getContacts(txn2);
 			will(returnValue(singletonList(contact)));
-			oneOf(db).getContact(txn, contact.getId());
+			oneOf(db).getContact(txn2, contact.getId());
 			will(returnValue(contact));
 
 			// getLocalUpdate()
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					MAJOR_VERSION, contact);
 			will(returnValue(contactGroup));
-			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
+			oneOf(clientHelper).getMessageMetadataAsDictionary(txn2,
 					contactGroupId);
 			will(returnValue(messageMetadata));
-			oneOf(clientHelper).getMessageAsList(txn, messageId);
+			oneOf(clientHelper).getMessageAsList(txn2, messageId);
-			will(returnValue(body));
+			will(returnValue(oldBody));
 			oneOf(clientHelper).parseAndValidateMailboxUpdate(
-					someClientSupports, someServerSupports, propsDict);
+					someClientSupports, emptyServerSupports, emptyPropsDict);
-			will(returnValue(updateWithMailbox));
+			will(returnValue(updateNoMailbox));
 			oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
 					MAJOR_VERSION, contact);
 			will(returnValue(contactGroup));
 
 			// storeMessageReplaceLatest()
-			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
+			oneOf(clientHelper).getMessageMetadataAsDictionary(txn2,
 					contactGroupId);
 			will(returnValue(messageMetadata));
-			expectStoreMessage(txn, contactGroupId, 2,
+			expectStoreMessage(txn2, contactGroupId, 2,
-					newerClientSupports, someServerSupports, propsDict);
+					newerClientSupports, emptyServerSupports, emptyPropsDict);
-			oneOf(db).removeMessage(txn, messageId);
+			oneOf(db).removeMessage(txn2, messageId);
 
-			oneOf(clientHelper).mergeGroupMetadata(txn, localGroup.getId(),
+			oneOf(clientHelper).mergeGroupMetadata(txn2, localGroup.getId(),
 					newerSentDict);
 		}});
 
 		t = createInstance(newerClientSupportsList);
-		t.onDatabaseOpened(txn);
+		t.onDatabaseOpened(txn2);
+
+		MailboxUpdateSentEvent e2 =
+				getEvent(txn2, MailboxUpdateSentEvent.class);
+		assertNoMailboxPropertiesSent(e2, newerClientSupportsList);
 	}
 
 	@Test
@@ -443,6 +465,9 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase {
 
 		MailboxUpdateManagerImpl t = createInstance(someClientSupportsList);
 		t.addingContact(txn, contact);
+
+		MailboxUpdateSentEvent e = getEvent(txn, MailboxUpdateSentEvent.class);
+		assertNoMailboxPropertiesSent(e, someClientSupportsList);
 	}
 
 	@Test
@@ -484,6 +509,9 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase {
 
 		MailboxUpdateManagerImpl t = createInstance(someClientSupportsList);
 		t.addingContact(txn, contact);
+
+		MailboxUpdateSentEvent e = getEvent(txn, MailboxUpdateSentEvent.class);
+		assertMailboxPropertiesSent(e, someClientSupportsList);
 	}
 
 	@Test
@@ -693,6 +721,8 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase {
 				u.getClientSupports());
 		assertEquals(updateWithMailbox.getMailboxProperties(),
 				u.getMailboxProperties());
+
+		assertFalse(hasEvent(txn, MailboxUpdateSentEvent.class));
 	}
 
 	@Test
@@ -734,6 +764,8 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase {
 		assertEquals(singleton(contact.getId()), localUpdates.keySet());
 		MailboxUpdate u = localUpdates.get(contact.getId());
 		assertFalse(u.hasMailbox());
+
+		assertFalse(hasEvent(txn, MailboxUpdateSentEvent.class));
 	}
 
 	@Test
@@ -915,4 +947,22 @@ public class MailboxUpdateManagerImplTest extends BrambleMockTestCase {
 					false);
 		}});
 	}
+
+	private void assertNoMailboxPropertiesSent(MailboxUpdateSentEvent e,
+			List<MailboxVersion> clientSupports) {
+		assertEquals(contact.getId(), e.getContactId());
+		MailboxUpdate u = e.getMailboxUpdate();
+		assertEquals(clientSupports, u.getClientSupports());
+		assertFalse(u.hasMailbox());
+	}
+
+	private void assertMailboxPropertiesSent(MailboxUpdateSentEvent e,
+			List<MailboxVersion> clientSupports) {
+		assertEquals(contact.getId(), e.getContactId());
+		MailboxUpdate u = e.getMailboxUpdate();
+		assertEquals(clientSupports, u.getClientSupports());
+		assertTrue(u.hasMailbox());
+		MailboxUpdateWithMailbox uMailbox = (MailboxUpdateWithMailbox) u;
+		assertEquals(updateProps, uMailbox.getMailboxProperties());
+	}
 }

bramble-core/src/test/java/org/briarproject/bramble/sync/MailboxOutgoingSessionTest.java

@@ -14,11 +14,13 @@ import org.briarproject.bramble.api.sync.SyncRecordWriter;
 import org.briarproject.bramble.api.sync.Versions;
 import org.briarproject.bramble.api.transport.StreamWriter;
 import org.briarproject.bramble.test.BrambleMockTestCase;
+import org.briarproject.bramble.test.CaptureArgumentAction;
 import org.briarproject.bramble.test.DbExpectations;
 import org.junit.Test;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
 
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
@@ -68,13 +70,10 @@ public class MailboxOutgoingSessionTest extends BrambleMockTestCase {
 			oneOf(eventBus).addListener(session);
 			// Send the protocol versions
 			oneOf(recordWriter).writeVersions(with(any(Versions.class)));
-			// Calculate capacity for acks
-			oneOf(recordWriter).getBytesWritten();
-			will(returnValue((long) versionRecordBytes));
 			// No messages to ack
 			oneOf(db).transactionWithResult(with(true),
 					withDbCallable(noAckIdTxn));
-			oneOf(db).getMessagesToAck(noAckIdTxn, contactId, MAX_MESSAGE_IDS);
+			oneOf(db).getMessagesToAck(noAckIdTxn, contactId);
 			will(returnValue(emptyList()));
 			// Calculate capacity for messages
 			oneOf(recordWriter).getBytesWritten();
@@ -106,7 +105,6 @@ public class MailboxOutgoingSessionTest extends BrambleMockTestCase {
 				MAX_FILE_PAYLOAD_BYTES);
 
 		Transaction ackIdTxn = new Transaction(null, true);
-		Transaction noAckIdTxn = new Transaction(null, true);
 		Transaction msgIdTxn = new Transaction(null, true);
 		Transaction msgTxn = new Transaction(null, true);
 
@@ -114,28 +112,24 @@ public class MailboxOutgoingSessionTest extends BrambleMockTestCase {
 		long capacityForMessages =
 				MAX_FILE_PAYLOAD_BYTES - versionRecordBytes - ackRecordBytes;
 
+		AtomicReference<Ack> ack = new AtomicReference<>();
+
 		context.checking(new DbExpectations() {{
 			// Add listener
 			oneOf(eventBus).addListener(session);
 			// Send the protocol versions
 			oneOf(recordWriter).writeVersions(with(any(Versions.class)));
+			// Load the IDs to ack
+			oneOf(db).transactionWithResult(with(true),
+					withDbCallable(ackIdTxn));
+			oneOf(db).getMessagesToAck(ackIdTxn, contactId);
+			will(returnValue(singletonList(message.getId())));
 			// Calculate capacity for acks
 			oneOf(recordWriter).getBytesWritten();
 			will(returnValue((long) versionRecordBytes));
-			// One message to ack
-			oneOf(db).transactionWithResult(with(true),
-					withDbCallable(ackIdTxn));
-			oneOf(db).getMessagesToAck(ackIdTxn, contactId, MAX_MESSAGE_IDS);
-			will(returnValue(singletonList(message.getId())));
 			// Send the ack
-			oneOf(recordWriter).getBytesWritten();
-			will(returnValue((long) versionRecordBytes));
 			oneOf(recordWriter).writeAck(with(any(Ack.class)));
-			// No more messages to ack
+			will(new CaptureArgumentAction<>(ack, Ack.class, 0));
-			oneOf(db).transactionWithResult(with(true),
-					withDbCallable(noAckIdTxn));
-			oneOf(db).getMessagesToAck(noAckIdTxn, contactId, MAX_MESSAGE_IDS);
-			will(returnValue(emptyList()));
 			// Calculate capacity for messages
 			oneOf(recordWriter).getBytesWritten();
 			will(returnValue((long) versionRecordBytes + ackRecordBytes));
@@ -162,6 +156,7 @@ public class MailboxOutgoingSessionTest extends BrambleMockTestCase {
 
 		assertEquals(singletonList(message.getId()),
 				sessionRecord.getAckedIds());
+		assertEquals(singletonList(message.getId()), ack.get().getMessageIds());
 		assertEquals(singletonList(message1.getId()),
 				sessionRecord.getSentIds());
 	}
@@ -178,48 +173,50 @@ public class MailboxOutgoingSessionTest extends BrambleMockTestCase {
 				eventBus, contactId, transportId, MAX_LATENCY,
 				streamWriter, recordWriter, sessionRecord, capacity);
 
-		Transaction ackIdTxn1 = new Transaction(null, true);
+		Transaction ackIdTxn = new Transaction(null, true);
-		Transaction ackIdTxn2 = new Transaction(null, true);
 
 		int firstAckRecordBytes =
 				RECORD_HEADER_BYTES + MessageId.LENGTH * MAX_MESSAGE_IDS;
 		int secondAckRecordBytes = RECORD_HEADER_BYTES + MessageId.LENGTH;
 
-		List<MessageId> idsInFirstAck = new ArrayList<>(MAX_MESSAGE_IDS);
+		// There are MAX_MESSAGE_IDS + 2 messages that need to be acked, but
-		for (int i = 0; i < MAX_MESSAGE_IDS; i++) {
+		// only enough capacity to ack MAX_MESSAGE_IDS + 1 messages
-			idsInFirstAck.add(new MessageId(getRandomId()));
+		List<MessageId> idsToAck = new ArrayList<>(MAX_MESSAGE_IDS + 2);
+		for (int i = 0; i < MAX_MESSAGE_IDS + 2; i++) {
+			idsToAck.add(new MessageId(getRandomId()));
 		}
+		// The first ack contains MAX_MESSAGE_IDS IDs
+		List<MessageId> idsInFirstAck = idsToAck.subList(0, MAX_MESSAGE_IDS);
+		// The second ack contains one ID
 		List<MessageId> idsInSecondAck =
-				singletonList(new MessageId(getRandomId()));
+				idsToAck.subList(MAX_MESSAGE_IDS, MAX_MESSAGE_IDS + 1);
-		List<MessageId> allIds = new ArrayList<>(MAX_MESSAGE_IDS + 1);
+		List<MessageId> idsAcked = idsToAck.subList(0, MAX_MESSAGE_IDS + 1);
-		allIds.addAll(idsInFirstAck);
+
-		allIds.addAll(idsInSecondAck);
+		AtomicReference<Ack> firstAck = new AtomicReference<>();
+		AtomicReference<Ack> secondAck = new AtomicReference<>();
 
 		context.checking(new DbExpectations() {{
 			// Add listener
 			oneOf(eventBus).addListener(session);
 			// Send the protocol versions
 			oneOf(recordWriter).writeVersions(with(any(Versions.class)));
+			// Load the IDs to ack
+			oneOf(db).transactionWithResult(with(true),
+					withDbCallable(ackIdTxn));
+			oneOf(db).getMessagesToAck(ackIdTxn, contactId);
+			will(returnValue(idsToAck));
 			// Calculate capacity for acks
 			oneOf(recordWriter).getBytesWritten();
 			will(returnValue((long) versionRecordBytes));
-			// Load the IDs for the first ack record
-			oneOf(db).transactionWithResult(with(true),
-					withDbCallable(ackIdTxn1));
-			oneOf(db).getMessagesToAck(ackIdTxn1, contactId, MAX_MESSAGE_IDS);
-			will(returnValue(idsInFirstAck));
 			// Send the first ack record
 			oneOf(recordWriter).writeAck(with(any(Ack.class)));
+			will(new CaptureArgumentAction<>(firstAck, Ack.class, 0));
 			// Calculate remaining capacity for acks
 			oneOf(recordWriter).getBytesWritten();
 			will(returnValue((long) versionRecordBytes + firstAckRecordBytes));
-			// Load the IDs for the second ack record
-			oneOf(db).transactionWithResult(with(true),
-					withDbCallable(ackIdTxn2));
-			oneOf(db).getMessagesToAck(ackIdTxn2, contactId, 1);
-			will(returnValue(idsInSecondAck));
 			// Send the second ack record
 			oneOf(recordWriter).writeAck(with(any(Ack.class)));
+			will(new CaptureArgumentAction<>(secondAck, Ack.class, 0));
 			// Not enough capacity left for another ack
 			oneOf(recordWriter).getBytesWritten();
 			will(returnValue((long) versionRecordBytes + firstAckRecordBytes
@@ -236,7 +233,9 @@ public class MailboxOutgoingSessionTest extends BrambleMockTestCase {
 
 		session.run();
 
-		assertEquals(allIds, sessionRecord.getAckedIds());
+		assertEquals(idsAcked, sessionRecord.getAckedIds());
+		assertEquals(idsInFirstAck, firstAck.get().getMessageIds());
+		assertEquals(idsInSecondAck, secondAck.get().getMessageIds());
 		assertEquals(emptyList(), sessionRecord.getSentIds());
 	}
 }