From 5be0e928c4918d4ab9a62686409137c5f4ec94c7 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 24 May 2019 10:50:03 +0100 Subject: [PATCH] Replace events with return value and exceptions. --- .../api/contact/ContactExchangeManager.java | 12 +- .../event/ContactExchangeFailedEvent.java | 32 ----- .../event/ContactExchangeSucceededEvent.java | 20 --- .../contact/ContactExchangeManagerImpl.java | 115 +++++------------- .../ContactExchangeViewModel.java | 61 ++++------ 5 files changed, 66 insertions(+), 174 deletions(-) delete mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeFailedEvent.java delete mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeSucceededEvent.java diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java index 67e72bab6..ef7442014 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java @@ -1,16 +1,24 @@ package org.briarproject.bramble.api.contact; import org.briarproject.bramble.api.crypto.SecretKey; +import org.briarproject.bramble.api.db.ContactExistsException; +import org.briarproject.bramble.api.db.DbException; +import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; +import java.io.IOException; + @NotNullByDefault public interface ContactExchangeManager { /** * Exchanges contact information with a remote peer. + * + * @return The contact's pseudonym + * @throws ContactExistsException If the contact already exists */ - void exchangeContacts(TransportId t, DuplexTransportConnection conn, - SecretKey masterKey, boolean alice); + Author exchangeContacts(TransportId t, DuplexTransportConnection conn, + SecretKey masterKey, boolean alice) throws IOException, DbException; } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeFailedEvent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeFailedEvent.java deleted file mode 100644 index 0d300f72f..000000000 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeFailedEvent.java +++ /dev/null @@ -1,32 +0,0 @@ -package org.briarproject.bramble.api.contact.event; - -import org.briarproject.bramble.api.event.Event; -import org.briarproject.bramble.api.identity.Author; -import org.briarproject.bramble.api.nullsafety.NotNullByDefault; - -import javax.annotation.Nullable; - -@NotNullByDefault -public class ContactExchangeFailedEvent extends Event { - - @Nullable - private final Author duplicateRemoteAuthor; - - public ContactExchangeFailedEvent(@Nullable Author duplicateRemoteAuthor) { - this.duplicateRemoteAuthor = duplicateRemoteAuthor; - } - - public ContactExchangeFailedEvent() { - this(null); - } - - @Nullable - public Author getDuplicateRemoteAuthor() { - return duplicateRemoteAuthor; - } - - public boolean wasDuplicateContact() { - return duplicateRemoteAuthor != null; - } - -} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeSucceededEvent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeSucceededEvent.java deleted file mode 100644 index 941a212af..000000000 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeSucceededEvent.java +++ /dev/null @@ -1,20 +0,0 @@ -package org.briarproject.bramble.api.contact.event; - -import org.briarproject.bramble.api.event.Event; -import org.briarproject.bramble.api.identity.Author; -import org.briarproject.bramble.api.nullsafety.NotNullByDefault; - -@NotNullByDefault -public class ContactExchangeSucceededEvent extends Event { - - private final Author remoteAuthor; - - public ContactExchangeSucceededEvent(Author remoteAuthor) { - this.remoteAuthor = remoteAuthor; - } - - public Author getRemoteAuthor() { - return remoteAuthor; - } - -} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java index bebe3c407..7c6ba360f 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java @@ -6,16 +6,12 @@ import org.briarproject.bramble.api.client.ClientHelper; import org.briarproject.bramble.api.contact.ContactExchangeManager; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.ContactManager; -import org.briarproject.bramble.api.contact.event.ContactExchangeFailedEvent; -import org.briarproject.bramble.api.contact.event.ContactExchangeSucceededEvent; import org.briarproject.bramble.api.crypto.CryptoComponent; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.data.BdfDictionary; import org.briarproject.bramble.api.data.BdfList; -import org.briarproject.bramble.api.db.ContactExistsException; import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.DbException; -import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.identity.LocalAuthor; @@ -46,7 +42,6 @@ import java.util.logging.Logger; import javax.inject.Inject; -import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.api.contact.RecordTypes.CONTACT_INFO; import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_SIGNATURE_LENGTH; @@ -56,7 +51,6 @@ import static org.briarproject.bramble.contact.ContactExchangeConstants.BOB_KEY_ import static org.briarproject.bramble.contact.ContactExchangeConstants.BOB_NONCE_LABEL; import static org.briarproject.bramble.contact.ContactExchangeConstants.PROTOCOL_VERSION; import static org.briarproject.bramble.contact.ContactExchangeConstants.SIGNING_LABEL; -import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.ValidationUtils.checkLength; import static org.briarproject.bramble.util.ValidationUtils.checkSize; @@ -85,7 +79,6 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { private final ClientHelper clientHelper; private final RecordReaderFactory recordReaderFactory; private final RecordWriterFactory recordWriterFactory; - private final EventBus eventBus; private final Clock clock; private final ConnectionManager connectionManager; private final ContactManager contactManager; @@ -98,7 +91,7 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { @Inject ContactExchangeManagerImpl(DatabaseComponent db, ClientHelper clientHelper, RecordReaderFactory recordReaderFactory, - RecordWriterFactory recordWriterFactory, EventBus eventBus, + RecordWriterFactory recordWriterFactory, Clock clock, ConnectionManager connectionManager, ContactManager contactManager, IdentityManager identityManager, TransportPropertyManager transportPropertyManager, @@ -108,7 +101,6 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { this.clientHelper = clientHelper; this.recordReaderFactory = recordReaderFactory; this.recordWriterFactory = recordWriterFactory; - this.eventBus = eventBus; this.clock = clock; this.connectionManager = connectionManager; this.contactManager = contactManager; @@ -120,33 +112,17 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { } @Override - public void exchangeContacts(TransportId t, DuplexTransportConnection conn, - SecretKey masterKey, boolean alice) { + public Author exchangeContacts(TransportId t, + DuplexTransportConnection conn, SecretKey masterKey, boolean alice) + throws IOException, DbException { // Get the transport connection's input and output streams - InputStream in; - OutputStream out; - try { - in = conn.getReader().getInputStream(); - out = conn.getWriter().getOutputStream(); - } catch (IOException e) { - logException(LOG, WARNING, e); - tryToClose(conn); - eventBus.broadcast(new ContactExchangeFailedEvent()); - return; - } + InputStream in = conn.getReader().getInputStream(); + OutputStream out = conn.getWriter().getOutputStream(); // Get the local author and transport properties - LocalAuthor localAuthor; - Map localProperties; - try { - localAuthor = identityManager.getLocalAuthor(); - localProperties = transportPropertyManager.getLocalProperties(); - } catch (DbException e) { - logException(LOG, WARNING, e); - eventBus.broadcast(new ContactExchangeFailedEvent()); - tryToClose(conn); - return; - } + LocalAuthor localAuthor = identityManager.getLocalAuthor(); + Map localProperties = + transportPropertyManager.getLocalProperties(); // Derive the header keys for the transport streams SecretKey aliceHeaderKey = crypto.deriveKey(ALICE_KEY_LABEL, masterKey, @@ -183,58 +159,37 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { // Exchange contact info long localTimestamp = clock.currentTimeMillis(); ContactInfo remoteInfo; - try { - if (alice) { - sendContactInfo(recordWriter, localAuthor, localProperties, - localSignature, localTimestamp); - remoteInfo = receiveContactInfo(recordReader); - } else { - remoteInfo = receiveContactInfo(recordReader); - sendContactInfo(recordWriter, localAuthor, localProperties, - localSignature, localTimestamp); - } - // Send EOF on the outgoing stream - streamWriter.sendEndOfStream(); - // Skip any remaining records from the incoming stream - recordReader.readRecord(r -> false, IGNORE); - } catch (IOException e) { - logException(LOG, WARNING, e); - eventBus.broadcast(new ContactExchangeFailedEvent()); - tryToClose(conn); - return; + if (alice) { + sendContactInfo(recordWriter, localAuthor, localProperties, + localSignature, localTimestamp); + remoteInfo = receiveContactInfo(recordReader); + } else { + remoteInfo = receiveContactInfo(recordReader); + sendContactInfo(recordWriter, localAuthor, localProperties, + localSignature, localTimestamp); } + // Send EOF on the outgoing stream + streamWriter.sendEndOfStream(); + // Skip any remaining records from the incoming stream + recordReader.readRecord(r -> false, IGNORE); // Verify the contact's signature if (!verify(remoteInfo.author, remoteNonce, remoteInfo.signature)) { LOG.warning("Invalid signature"); - eventBus.broadcast(new ContactExchangeFailedEvent()); - tryToClose(conn); - return; + throw new FormatException(); } // The agreed timestamp is the minimum of the peers' timestamps long timestamp = Math.min(localTimestamp, remoteInfo.timestamp); - try { - // Add the contact - ContactId contactId = addContact(remoteInfo.author, localAuthor, - masterKey, timestamp, alice, remoteInfo.properties); - // Reuse the connection as a transport connection - connectionManager.manageOutgoingConnection(contactId, t, conn); - // Pseudonym exchange succeeded - LOG.info("Pseudonym exchange succeeded"); - eventBus.broadcast( - new ContactExchangeSucceededEvent(remoteInfo.author)); - } catch (ContactExistsException e) { - logException(LOG, WARNING, e); - tryToClose(conn); - eventBus.broadcast( - new ContactExchangeFailedEvent(e.getRemoteAuthor())); - } catch (DbException e) { - logException(LOG, WARNING, e); - tryToClose(conn); - eventBus.broadcast(new ContactExchangeFailedEvent()); - } + // Add the contact + ContactId contactId = addContact(remoteInfo.author, localAuthor, + masterKey, timestamp, alice, remoteInfo.properties); + // Reuse the connection as a transport connection + connectionManager.manageOutgoingConnection(contactId, t, conn); + // Pseudonym exchange succeeded + LOG.info("Pseudonym exchange succeeded"); + return remoteInfo.author; } private byte[] sign(LocalAuthor author, byte[] nonce) { @@ -298,16 +253,6 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { }); } - private void tryToClose(DuplexTransportConnection conn) { - try { - LOG.info("Closing connection"); - conn.getReader().dispose(true, true); - conn.getWriter().dispose(true); - } catch (IOException e) { - logException(LOG, WARNING, e); - } - } - private static class ContactInfo { private final Author author; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java index 5b990f2d8..149f92b8d 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java @@ -7,58 +7,63 @@ import android.arch.lifecycle.MutableLiveData; import android.support.annotation.UiThread; import org.briarproject.bramble.api.contact.ContactExchangeManager; -import org.briarproject.bramble.api.contact.event.ContactExchangeFailedEvent; -import org.briarproject.bramble.api.contact.event.ContactExchangeSucceededEvent; import org.briarproject.bramble.api.crypto.SecretKey; -import org.briarproject.bramble.api.event.Event; -import org.briarproject.bramble.api.event.EventBus; -import org.briarproject.bramble.api.event.EventListener; +import org.briarproject.bramble.api.db.ContactExistsException; +import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; +import java.io.IOException; import java.util.concurrent.Executor; +import java.util.logging.Logger; import javax.annotation.Nullable; import javax.inject.Inject; -import static java.util.Objects.requireNonNull; +import static java.util.logging.Level.WARNING; +import static java.util.logging.Logger.getLogger; +import static org.briarproject.bramble.util.LogUtils.logException; @NotNullByDefault -class ContactExchangeViewModel extends AndroidViewModel - implements EventListener { +class ContactExchangeViewModel extends AndroidViewModel { + + private static final Logger LOG = + getLogger(ContactExchangeViewModel.class.getName()); private final Executor ioExecutor; private final ContactExchangeManager contactExchangeManager; - private final EventBus eventBus; private final MutableLiveData succeeded = new MutableLiveData<>(); @Nullable - private Author remoteAuthor, duplicateAuthor; + private volatile Author remoteAuthor, duplicateAuthor; @Inject ContactExchangeViewModel(Application app, @IoExecutor Executor ioExecutor, - ContactExchangeManager contactExchangeManager, EventBus eventBus) { + ContactExchangeManager contactExchangeManager) { super(app); this.ioExecutor = ioExecutor; this.contactExchangeManager = contactExchangeManager; - this.eventBus = eventBus; - eventBus.addListener(this); - } - - @Override - protected void onCleared() { - super.onCleared(); - eventBus.removeListener(this); } @UiThread void startContactExchange(TransportId t, DuplexTransportConnection conn, SecretKey masterKey, boolean alice) { - ioExecutor.execute(() -> contactExchangeManager.exchangeContacts(t, - conn, masterKey, alice)); + ioExecutor.execute(() -> { + try { + remoteAuthor = contactExchangeManager.exchangeContacts(t, conn, + masterKey, alice); + succeeded.postValue(true); + } catch (ContactExistsException e) { + duplicateAuthor = e.getRemoteAuthor(); + succeeded.postValue(false); + } catch (DbException | IOException e) { + logException(LOG, WARNING, e); + succeeded.postValue(false); + } + }); } @UiThread @@ -76,18 +81,4 @@ class ContactExchangeViewModel extends AndroidViewModel LiveData getSucceeded() { return succeeded; } - - @Override - public void eventOccurred(Event e) { - if (e instanceof ContactExchangeSucceededEvent) { - ContactExchangeSucceededEvent c = (ContactExchangeSucceededEvent) e; - remoteAuthor = c.getRemoteAuthor(); - succeeded.setValue(true); - } else if (e instanceof ContactExchangeFailedEvent) { - ContactExchangeFailedEvent c = (ContactExchangeFailedEvent) e; - if (c.wasDuplicateContact()) - duplicateAuthor = requireNonNull(c.getDuplicateRemoteAuthor()); - succeeded.setValue(false); - } - } }