From da54712ae14b96dc94d294949e0ce3eba88e74b0 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 9 May 2019 17:03:50 +0100 Subject: [PATCH] Refactor ContactExchangeTask into reusable manager. --- .../api/contact/ContactExchangeManager.java | 16 ++++ .../contact/ContactExchangeConstants.java | 21 +---- ...l.java => ContactExchangeManagerImpl.java} | 31 +++++--- .../bramble/contact/ContactModule.java | 10 +-- .../briar/android/AndroidComponent.java | 4 +- .../keyagreement/ContactExchangeActivity.java | 78 +++++++------------ 6 files changed, 72 insertions(+), 88 deletions(-) create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java rename bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeTask.java => bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeConstants.java (53%) rename bramble-core/src/main/java/org/briarproject/bramble/contact/{ContactExchangeTaskImpl.java => ContactExchangeManagerImpl.java} (89%) 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 new file mode 100644 index 000000000..67e72bab6 --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java @@ -0,0 +1,16 @@ +package org.briarproject.bramble.api.contact; + +import org.briarproject.bramble.api.crypto.SecretKey; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.plugin.TransportId; +import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; + +@NotNullByDefault +public interface ContactExchangeManager { + + /** + * Exchanges contact information with a remote peer. + */ + void exchangeContacts(TransportId t, DuplexTransportConnection conn, + SecretKey masterKey, boolean alice); +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeTask.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeConstants.java similarity index 53% rename from bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeTask.java rename to bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeConstants.java index fd8dd20eb..9f39d3470 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeTask.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeConstants.java @@ -1,16 +1,6 @@ -package org.briarproject.bramble.api.contact; +package org.briarproject.bramble.contact; -import org.briarproject.bramble.api.crypto.SecretKey; -import org.briarproject.bramble.api.identity.LocalAuthor; -import org.briarproject.bramble.api.nullsafety.NotNullByDefault; -import org.briarproject.bramble.api.plugin.TransportId; -import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; - -/** - * A task for conducting a contact information exchange with a remote peer. - */ -@NotNullByDefault -public interface ContactExchangeTask { +interface ContactExchangeConstants { /** * The current version of the contact exchange protocol. @@ -42,11 +32,4 @@ public interface ContactExchangeTask { * Label for signing key binding nonces. */ String SIGNING_LABEL = "org.briarproject.briar.contact/EXCHANGE"; - - /** - * Exchanges contact information with a remote peer. - */ - void exchangeContacts(LocalAuthor localAuthor, SecretKey masterKey, - DuplexTransportConnection conn, TransportId transportId, - boolean alice); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeTaskImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java similarity index 89% rename from bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeTaskImpl.java rename to bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java index 3b2668a99..f68ab538b 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeTaskImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java @@ -3,7 +3,7 @@ package org.briarproject.bramble.contact; import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.Predicate; import org.briarproject.bramble.api.client.ClientHelper; -import org.briarproject.bramble.api.contact.ContactExchangeTask; +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; @@ -17,6 +17,7 @@ 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; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; @@ -49,16 +50,22 @@ 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; +import static org.briarproject.bramble.contact.ContactExchangeConstants.ALICE_KEY_LABEL; +import static org.briarproject.bramble.contact.ContactExchangeConstants.ALICE_NONCE_LABEL; +import static org.briarproject.bramble.contact.ContactExchangeConstants.BOB_KEY_LABEL; +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; @MethodsNotNullByDefault @ParametersNotNullByDefault -class ContactExchangeTaskImpl implements ContactExchangeTask { +class ContactExchangeManagerImpl implements ContactExchangeManager { private static final Logger LOG = - getLogger(ContactExchangeTaskImpl.class.getName()); + getLogger(ContactExchangeManagerImpl.class.getName()); // Accept records with current protocol version, known record type private static final Predicate ACCEPT = r -> @@ -82,17 +89,18 @@ class ContactExchangeTaskImpl implements ContactExchangeTask { private final Clock clock; private final ConnectionManager connectionManager; private final ContactManager contactManager; + private final IdentityManager identityManager; private final TransportPropertyManager transportPropertyManager; private final CryptoComponent crypto; private final StreamReaderFactory streamReaderFactory; private final StreamWriterFactory streamWriterFactory; @Inject - ContactExchangeTaskImpl(DatabaseComponent db, ClientHelper clientHelper, + ContactExchangeManagerImpl(DatabaseComponent db, ClientHelper clientHelper, RecordReaderFactory recordReaderFactory, RecordWriterFactory recordWriterFactory, EventBus eventBus, Clock clock, ConnectionManager connectionManager, - ContactManager contactManager, + ContactManager contactManager, IdentityManager identityManager, TransportPropertyManager transportPropertyManager, CryptoComponent crypto, StreamReaderFactory streamReaderFactory, StreamWriterFactory streamWriterFactory) { @@ -104,6 +112,7 @@ class ContactExchangeTaskImpl implements ContactExchangeTask { this.clock = clock; this.connectionManager = connectionManager; this.contactManager = contactManager; + this.identityManager = identityManager; this.transportPropertyManager = transportPropertyManager; this.crypto = crypto; this.streamReaderFactory = streamReaderFactory; @@ -111,9 +120,8 @@ class ContactExchangeTaskImpl implements ContactExchangeTask { } @Override - public void exchangeContacts(LocalAuthor localAuthor, - SecretKey masterKey, DuplexTransportConnection conn, - TransportId transportId, boolean alice) { + public void exchangeContacts(TransportId t, DuplexTransportConnection conn, + SecretKey masterKey, boolean alice) { // Get the transport connection's input and output streams InputStream in; OutputStream out; @@ -127,9 +135,11 @@ class ContactExchangeTaskImpl implements ContactExchangeTask { return; } - // Get the local transport properties + // 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); @@ -210,8 +220,7 @@ class ContactExchangeTaskImpl implements ContactExchangeTask { ContactId contactId = addContact(remoteInfo.author, localAuthor, masterKey, timestamp, alice, remoteInfo.properties); // Reuse the connection as a transport connection - connectionManager.manageOutgoingConnection(contactId, transportId, - conn); + connectionManager.manageOutgoingConnection(contactId, t, conn); // Pseudonym exchange succeeded LOG.info("Pseudonym exchange succeeded"); eventBus.broadcast( diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactModule.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactModule.java index f31ce108a..614edd4ea 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactModule.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactModule.java @@ -1,6 +1,6 @@ package org.briarproject.bramble.contact; -import org.briarproject.bramble.api.contact.ContactExchangeTask; +import org.briarproject.bramble.api.contact.ContactExchangeManager; import org.briarproject.bramble.api.contact.ContactManager; import javax.inject.Inject; @@ -19,14 +19,14 @@ public class ContactModule { @Provides @Singleton - ContactManager getContactManager(ContactManagerImpl contactManager) { + ContactManager provideContactManager(ContactManagerImpl contactManager) { return contactManager; } @Provides - ContactExchangeTask provideContactExchangeTask( - ContactExchangeTaskImpl contactExchangeTask) { - return contactExchangeTask; + ContactExchangeManager provideContactExchangeManager( + ContactExchangeManagerImpl contactExchangeManager) { + return contactExchangeManager; } @Provides diff --git a/briar-android/src/main/java/org/briarproject/briar/android/AndroidComponent.java b/briar-android/src/main/java/org/briarproject/briar/android/AndroidComponent.java index 90a76e6da..e2473e846 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/AndroidComponent.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/AndroidComponent.java @@ -8,7 +8,7 @@ import org.briarproject.bramble.BrambleCoreEagerSingletons; import org.briarproject.bramble.BrambleCoreModule; import org.briarproject.bramble.account.BriarAccountModule; import org.briarproject.bramble.api.account.AccountManager; -import org.briarproject.bramble.api.contact.ContactExchangeTask; +import org.briarproject.bramble.api.contact.ContactExchangeManager; import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.crypto.CryptoExecutor; import org.briarproject.bramble.api.crypto.PasswordStrengthEstimator; @@ -128,7 +128,7 @@ public interface AndroidComponent SettingsManager settingsManager(); - ContactExchangeTask contactExchangeTask(); + ContactExchangeManager contactExchangeManager(); KeyAgreementTask keyAgreementTask(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java index a99ce9aaa..9d8f661e2 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java @@ -4,15 +4,12 @@ import android.os.Bundle; import android.support.annotation.UiThread; import android.widget.Toast; -import org.briarproject.bramble.api.contact.ContactExchangeTask; +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.db.DbException; import org.briarproject.bramble.api.event.Event; import org.briarproject.bramble.api.event.EventListener; import org.briarproject.bramble.api.identity.Author; -import org.briarproject.bramble.api.identity.IdentityManager; -import org.briarproject.bramble.api.identity.LocalAuthor; import org.briarproject.bramble.api.keyagreement.KeyAgreementResult; import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; @@ -21,32 +18,25 @@ import org.briarproject.briar.R; import org.briarproject.briar.android.activity.ActivityComponent; import java.util.concurrent.Executor; -import java.util.logging.Logger; import javax.annotation.Nullable; import javax.inject.Inject; import static android.widget.Toast.LENGTH_LONG; -import static java.util.logging.Level.WARNING; -import static org.briarproject.bramble.util.LogUtils.logException; +import static java.util.Objects.requireNonNull; @MethodsNotNullByDefault @ParametersNotNullByDefault public class ContactExchangeActivity extends KeyAgreementActivity implements EventListener { - private static final Logger LOG = - Logger.getLogger(ContactExchangeActivity.class.getName()); - @Inject @IoExecutor Executor ioExecutor; // Fields that are accessed from background threads must be volatile @Inject - volatile ContactExchangeTask contactExchangeTask; - @Inject - volatile IdentityManager identityManager; + volatile ContactExchangeManager contactExchangeManager; @Override public void injectActivity(ActivityComponent component) { @@ -62,76 +52,62 @@ public class ContactExchangeActivity extends KeyAgreementActivity implements @Override public void onStart() { super.onStart(); - // Listen to updates from contactExchangeTask + // Listen to updates from ContactExchangeManager eventBus.addListener(this); } @Override protected void onStop() { super.onStop(); - // Stop listening to updates from contactExchangeTask + // Stop listening to updates from ContactExchangeManager eventBus.addListener(this); } private void startContactExchange(KeyAgreementResult result) { ioExecutor.execute(() -> { - LocalAuthor localAuthor; - // Load the local pseudonym - try { - localAuthor = identityManager.getLocalAuthor(); - } catch (DbException e) { - logException(LOG, WARNING, e); - contactExchangeFailed(); - return; - } // Exchange contact details - contactExchangeTask.exchangeContacts(localAuthor, - result.getMasterKey(), result.getConnection(), - result.getTransportId(), result.wasAlice()); + contactExchangeManager.exchangeContacts(result.getTransportId(), + result.getConnection(), result.getMasterKey(), + result.wasAlice()); }); } @Override public void eventOccurred(Event e) { if (e instanceof ContactExchangeSucceededEvent) { - contactExchangeSucceeded( - ((ContactExchangeSucceededEvent) e).getRemoteAuthor()); + ContactExchangeSucceededEvent c = (ContactExchangeSucceededEvent) e; + contactExchangeSucceeded(c.getRemoteAuthor()); } else if (e instanceof ContactExchangeFailedEvent) { - ContactExchangeFailedEvent fe = (ContactExchangeFailedEvent) e; - if (fe.wasDuplicateContact()) { - duplicateContact(fe.getDuplicateRemoteAuthor()); + ContactExchangeFailedEvent c = (ContactExchangeFailedEvent) e; + if (c.wasDuplicateContact()) { + duplicateContact(requireNonNull(c.getDuplicateRemoteAuthor())); } else { contactExchangeFailed(); } } } + @UiThread private void contactExchangeSucceeded(Author remoteAuthor) { - runOnUiThreadUnlessDestroyed(() -> { - String contactName = remoteAuthor.getName(); - String format = getString(R.string.contact_added_toast); - String text = String.format(format, contactName); - Toast.makeText(ContactExchangeActivity.this, text, LENGTH_LONG) - .show(); - supportFinishAfterTransition(); - }); + String contactName = remoteAuthor.getName(); + String format = getString(R.string.contact_added_toast); + String text = String.format(format, contactName); + Toast.makeText(this, text, LENGTH_LONG).show(); + supportFinishAfterTransition(); } + @UiThread private void duplicateContact(Author remoteAuthor) { - runOnUiThreadUnlessDestroyed(() -> { - String contactName = remoteAuthor.getName(); - String format = getString(R.string.contact_already_exists); - String text = String.format(format, contactName); - Toast.makeText(ContactExchangeActivity.this, text, LENGTH_LONG) - .show(); - finish(); - }); + String contactName = remoteAuthor.getName(); + String format = getString(R.string.contact_already_exists); + String text = String.format(format, contactName); + Toast.makeText(this, text, LENGTH_LONG).show(); + finish(); } + @UiThread private void contactExchangeFailed() { - runOnUiThreadUnlessDestroyed(() -> { - showErrorFragment(R.string.connection_error_explanation); - }); + showErrorFragment(R.string.connection_error_explanation); } @UiThread