diff --git a/bramble-android/proguard-rules.txt b/bramble-android/proguard-rules.txt index fee1df635..c34540941 100644 --- a/bramble-android/proguard-rules.txt +++ b/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 * { *; } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java index c28c07fe9..ff43c7272 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java +++ b/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 - * need to be acknowledged, up to the given number of messages. + * Returns the IDs of all messages received from the given contact that + * need to be acknowledged. *

* Read-only. */ - Collection getMessagesToAck(Transaction txn, ContactId c, - int maxMessages) throws DbException; + Collection getMessagesToAck(Transaction txn, ContactId c) + 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. + *

+ * 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 diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxProperties.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxProperties.java index 4175a5752..d16cba6b8 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxProperties.java +++ b/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; } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxStatus.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxStatus.java index 3ba6bb92f..5b729a7b3 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxStatus.java +++ b/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 getServerSupports() { + return serverSupports; + } + /** * @return true if this status indicates a problem with the mailbox. */ diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/event/MailboxUpdateSentEvent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/event/MailboxUpdateSentEvent.java new file mode 100644 index 000000000..4017780bd --- /dev/null +++ b/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. + *

+ * 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; + } +} 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 c6cbe9efc..35a8ddb73 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 @@ -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; } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java index cfc20ecce..23c538c26 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java @@ -620,11 +620,11 @@ class DatabaseComponentImpl implements DatabaseComponent { @Override public Collection 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 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 implements DatabaseComponent { T txn = unbox(transaction); if (!db.containsContact(txn, c)) throw new NoSuchContactException(); - List visible = new ArrayList<>(acked.size()); - for (MessageId m : acked) { - if (db.containsVisibleMessage(txn, c, m)) visible.add(m); - } - db.lowerAckFlag(txn, c, visible); + db.lowerAckFlag(txn, c, acked); } @Override diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxConnectivityChecker.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxConnectivityChecker.java index 240f2c718..e29eb5e5c 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/ContactMailboxConnectivityChecker.java +++ b/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()); }); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java index 7bf057904..8ae436a61 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiImpl.java +++ b/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(); 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 17857551b..4a6556c52 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 @@ -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(), - ownProps.getOnion()); + u = createAndSendUpdateWithMailbox(txn, c, + 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 diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUploadWorker.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUploadWorker.java index 5c6462c6d..b40376014 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxUploadWorker.java +++ b/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(); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxWorkerFactoryImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxWorkerFactoryImpl.java index 34a3cab74..3031741ee 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxWorkerFactoryImpl.java +++ b/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, - connectivityChecker, mailboxApiCaller, mailboxApi, - mailboxUpdateManager, properties); + OwnMailboxContactListWorker worker = new OwnMailboxContactListWorker( + ioExecutor, db, eventBus, connectivityChecker, mailboxApiCaller, + mailboxApi, mailboxUpdateManager, properties); + eventBus.addListener(worker); + return worker; } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/OwnMailboxConnectivityChecker.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/OwnMailboxConnectivityChecker.java index ddf73b1b5..1882cc5f4 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/OwnMailboxConnectivityChecker.java +++ b/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 serverSupports = mailboxApi.getServerSupports(properties); LOG.info("Own mailbox is reachable"); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/sync/MailboxOutgoingSession.java b/bramble-core/src/main/java/org/briarproject/bramble/sync/MailboxOutgoingSession.java index 3dcf95a3a..afe7a72a8 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/sync/MailboxOutgoingSession.java +++ b/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()) { - Collection idsToAck = loadMessageIdsToAck(); - if (idsToAck.isEmpty()) break; - recordWriter.writeAck(new Ack(idsToAck)); - sessionRecord.onAckSent(idsToAck); + List idsToAck = loadMessageIdsToAck(); + int idsSent = 0; + while (idsSent < idsToAck.size() && !isInterrupted()) { + int idsRemaining = idsToAck.size() - idsSent; + 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 acked = + idsToAck.subList(idsSent, idsSent + idsToSend); + recordWriter.writeAck(new Ack(acked)); + sessionRecord.onAckSent(acked); LOG.info("Sent ack"); + idsSent += idsToSend; } } - private Collection 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); + private List loadMessageIdsToAck() throws DbException { Collection 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() { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java index 2e342186a..9daec1220 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java +++ b/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).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).lowerAckFlag(txn, contactId, acked); oneOf(database).commitTransaction(txn); }}); DatabaseComponent db = createDatabaseComponent(database, eventBus, 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 d149c0be8..f176dc89b 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 @@ -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 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).getContacts(txn); + oneOf(db).addGroup(txn1, localGroup); + 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(clientVersioningManager).getClientVisibility(txn, + oneOf(db).addGroup(txn1, contactGroup); + 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 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).getContacts(txn); + oneOf(db).addGroup(txn1, localGroup); + 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(clientVersioningManager).getClientVisibility(txn, + oneOf(db).addGroup(txn1, contactGroup); + 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 messageMetadata = new LinkedHashMap<>(); MessageId messageId = new MessageId(getRandomId()); messageMetadata.put(messageId, metaDictionary); - BdfList body = BdfList.of(1, someClientSupports, someServerSupports, - propsDict); + BdfList oldBody = BdfList.of(1, someClientSupports, emptyServerSupports, + 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); - will(returnValue(body)); + oneOf(clientHelper).getMessageAsList(txn2, messageId); + will(returnValue(oldBody)); oneOf(clientHelper).parseAndValidateMailboxUpdate( - someClientSupports, someServerSupports, propsDict); - will(returnValue(updateWithMailbox)); + someClientSupports, emptyServerSupports, emptyPropsDict); + 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, - newerClientSupports, someServerSupports, propsDict); - oneOf(db).removeMessage(txn, messageId); + expectStoreMessage(txn2, contactGroupId, 2, + newerClientSupports, emptyServerSupports, emptyPropsDict); + 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 clientSupports) { + assertEquals(contact.getId(), e.getContactId()); + MailboxUpdate u = e.getMailboxUpdate(); + assertEquals(clientSupports, u.getClientSupports()); + assertFalse(u.hasMailbox()); + } + + private void assertMailboxPropertiesSent(MailboxUpdateSentEvent e, + List 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()); + } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/sync/MailboxOutgoingSessionTest.java b/bramble-core/src/test/java/org/briarproject/bramble/sync/MailboxOutgoingSessionTest.java index 814599358..a623cc8af 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/sync/MailboxOutgoingSessionTest.java +++ b/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 = 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 - oneOf(db).transactionWithResult(with(true), - withDbCallable(noAckIdTxn)); - oneOf(db).getMessagesToAck(noAckIdTxn, contactId, MAX_MESSAGE_IDS); - will(returnValue(emptyList())); + will(new CaptureArgumentAction<>(ack, Ack.class, 0)); // 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 ackIdTxn2 = new Transaction(null, true); + Transaction ackIdTxn = new Transaction(null, true); int firstAckRecordBytes = RECORD_HEADER_BYTES + MessageId.LENGTH * MAX_MESSAGE_IDS; int secondAckRecordBytes = RECORD_HEADER_BYTES + MessageId.LENGTH; - List idsInFirstAck = new ArrayList<>(MAX_MESSAGE_IDS); - for (int i = 0; i < MAX_MESSAGE_IDS; i++) { - idsInFirstAck.add(new MessageId(getRandomId())); + // There are MAX_MESSAGE_IDS + 2 messages that need to be acked, but + // only enough capacity to ack MAX_MESSAGE_IDS + 1 messages + List 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 idsInFirstAck = idsToAck.subList(0, MAX_MESSAGE_IDS); + // The second ack contains one ID List idsInSecondAck = - singletonList(new MessageId(getRandomId())); - List allIds = new ArrayList<>(MAX_MESSAGE_IDS + 1); - allIds.addAll(idsInFirstAck); - allIds.addAll(idsInSecondAck); + idsToAck.subList(MAX_MESSAGE_IDS, MAX_MESSAGE_IDS + 1); + List idsAcked = idsToAck.subList(0, MAX_MESSAGE_IDS + 1); + + AtomicReference firstAck = new AtomicReference<>(); + AtomicReference 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()); } }