Merge branch '1565-duplicate-remote-contacts' into 'master'

UX for handling duplicate handshake links

Closes #1565

See merge request briar/briar!1173

bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java

@@ -4,8 +4,10 @@ import org.briarproject.bramble.api.FormatException;
 import org.briarproject.bramble.api.Pair;
 import org.briarproject.bramble.api.UnsupportedVersionException;
 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.db.NoSuchContactException;
+import org.briarproject.bramble.api.db.PendingContactExistsException;
 import org.briarproject.bramble.api.db.Transaction;
 import org.briarproject.bramble.api.identity.Author;
 import org.briarproject.bramble.api.identity.AuthorId;
@@ -117,9 +119,14 @@ public interface ContactManager {
 	 * @throws FormatException If the link is invalid
 	 * @throws GeneralSecurityException If the pending contact's handshake
 	 * public key is invalid
+	 * @throws ContactExistsException If a contact with the same handshake
+	 * public key already exists
+	 * @throws PendingContactExistsException If a pending contact with the same
+	 * handshake public key already exists
 	 */
 	PendingContact addPendingContact(String link, String alias)
-			throws DbException, FormatException, GeneralSecurityException;
+			throws DbException, FormatException, GeneralSecurityException,
+			ContactExistsException, PendingContactExistsException;
 
 	/**
 	 * Returns the pending contact with the given ID.

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

@@ -83,7 +83,7 @@ public interface DatabaseComponent extends TransactionManager {
 	/**
 	 * Stores a pending contact.
 	 */
-	void addPendingContact(Transaction txn, PendingContact p)
+	void addPendingContact(Transaction txn, PendingContact p, AuthorId local)
 			throws DbException;
 
 	/**

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

@@ -1,9 +1,21 @@
 package org.briarproject.bramble.api.db;
 
+import org.briarproject.bramble.api.contact.PendingContact;
+
 /**
  * Thrown when a duplicate pending contact is added to the database. This
  * exception may occur due to concurrent updates and does not indicate a
  * database error.
  */
 public class PendingContactExistsException extends DbException {
+
+	private final PendingContact pendingContact;
+
+	public PendingContactExistsException(PendingContact pendingContact) {
+		this.pendingContact = pendingContact;
+	}
+
+	public PendingContact getPendingContact() {
+		return pendingContact;
+	}
 }

bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java

@@ -139,7 +139,8 @@ class ContactManagerImpl implements ContactManager, EventListener {
 				pendingContactFactory.createPendingContact(link, alias);
 		Transaction txn = db.startTransaction(false);
 		try {
-			db.addPendingContact(txn, p);
+			AuthorId local = identityManager.getLocalAuthor(txn).getId();
+			db.addPendingContact(txn, p, local);
 			KeyPair ourKeyPair = identityManager.getHandshakeKeys(txn);
 			keyManager.addPendingContact(txn, p.getId(), p.getPublicKey(),
 					ourKeyPair);

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

@@ -267,6 +267,16 @@ interface Database<T> {
 	 */
 	Collection<ContactId> getContacts(T txn, AuthorId local) throws DbException;
 
+	/**
+	 * Returns the contact with the given {@code handshakePublicKey}
+	 * for the given local pseudonym or {@code null} if none exists.
+	 * <p/>
+	 * Read-only.
+	 */
+	@Nullable
+	Contact getContact(T txn, PublicKey handshakePublicKey, AuthorId local)
+			throws DbException;
+
 	/**
 	 * Returns the group with the given ID.
 	 * <p/>

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

@@ -291,12 +291,17 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 	}
 
 	@Override
-	public void addPendingContact(Transaction transaction, PendingContact p)
-			throws DbException {
+	public void addPendingContact(Transaction transaction, PendingContact p,
+			AuthorId local) throws DbException {
 		if (transaction.isReadOnly()) throw new IllegalArgumentException();
 		T txn = unbox(transaction);
-		if (db.containsPendingContact(txn, p.getId()))
-			throw new PendingContactExistsException();
+		Contact contact = db.getContact(txn, p.getPublicKey(), local);
+		if (contact != null)
+			throw new ContactExistsException(local, contact.getAuthor());
+		if (db.containsPendingContact(txn, p.getId())) {
+			PendingContact existing = db.getPendingContact(txn, p.getId());
+			throw new PendingContactExistsException(existing);
+		}
 		db.addPendingContact(txn, p);
 		transaction.attach(new PendingContactAddedEvent(p));
 	}

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

@@ -1465,6 +1465,47 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
+	@Nullable
+	@Override
+	public Contact getContact(Connection txn, PublicKey handshakePublicKey,
+			AuthorId localAuthorId) throws DbException {
+		PreparedStatement ps = null;
+		ResultSet rs = null;
+		try {
+			String sql = "SELECT contactId, authorId, formatVersion, name,"
+					+ " alias, publicKey, verified"
+					+ " FROM contacts"
+					+ " WHERE handshakePublicKey = ? AND localAuthorId = ?";
+			ps = txn.prepareStatement(sql);
+			ps.setBytes(1, handshakePublicKey.getEncoded());
+			ps.setBytes(2, localAuthorId.getBytes());
+			rs = ps.executeQuery();
+			if (!rs.next()) {
+				rs.close();
+				ps.close();
+				return null;
+			}
+			ContactId contactId = new ContactId(rs.getInt(1));
+			AuthorId authorId = new AuthorId(rs.getBytes(2));
+			int formatVersion = rs.getInt(3);
+			String name = rs.getString(4);
+			String alias = rs.getString(5);
+			PublicKey publicKey = new SignaturePublicKey(rs.getBytes(6));
+			boolean verified = rs.getBoolean(7);
+			if (rs.next()) throw new DbStateException();
+			rs.close();
+			ps.close();
+			Author author =
+					new Author(authorId, formatVersion, name, publicKey);
+			return new Contact(contactId, author, localAuthorId, alias,
+					handshakePublicKey, verified);
+		} catch (SQLException e) {
+			tryToClose(rs, LOG, WARNING);
+			tryToClose(ps, LOG, WARNING);
+			throw new DbException(e);
+		}
+	}
+
 	@Override
 	public Group getGroup(Connection txn, GroupId g) throws DbException {
 		PreparedStatement ps = null;

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

@@ -34,6 +34,7 @@ import org.briarproject.bramble.test.BrambleTestCase;
 import org.briarproject.bramble.test.SettableClock;
 import org.briarproject.bramble.test.TestDatabaseConfig;
 import org.briarproject.bramble.test.TestMessageFactory;
+import org.briarproject.bramble.test.TestUtils;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -1149,6 +1150,43 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase {
 		db.close();
 	}
 
+	@Test
+	public void testGetContactsByHandshakePublicKey() throws Exception {
+		Database<Connection> db = open(false);
+		Connection txn = db.startTransaction();
+
+		// Add an identity for a local author - no contacts should be
+		// associated
+		db.addIdentity(txn, identity);
+		PublicKey handshakePublicKey = TestUtils.getSignaturePublicKey();
+		Contact contact =
+				db.getContact(txn, handshakePublicKey, localAuthor.getId());
+		assertNull(contact);
+
+		// Add a contact associated with the local author
+		assertEquals(contactId, db.addContact(txn, author, localAuthor.getId(),
+				handshakePublicKey, true));
+		contact = db.getContact(txn, handshakePublicKey, localAuthor.getId());
+		assertNotNull(contact);
+		assertEquals(contactId, contact.getId());
+		assertEquals(author, contact.getAuthor());
+		assertNull(contact.getAlias());
+		assertEquals(handshakePublicKey, contact.getHandshakePublicKey());
+		assertTrue(contact.isVerified());
+		assertEquals(author.getName(), contact.getAuthor().getName());
+		assertEquals(author.getPublicKey(), contact.getAuthor().getPublicKey());
+		assertEquals(author.getFormatVersion(),
+				contact.getAuthor().getFormatVersion());
+
+		// Ensure no contacts are returned after contact was deleted
+		db.removeContact(txn, contactId);
+		contact = db.getContact(txn, handshakePublicKey, localAuthor.getId());
+		assertNull(contact);
+
+		db.commitTransaction(txn);
+		db.close();
+	}
+
 	@Test
 	public void testOfferedMessages() throws Exception {
 		Database<Connection> db = open(false);

briar-android/src/main/java/org/briarproject/briar/android/contact/add/remote/AddContactViewModel.java

@@ -9,8 +9,10 @@ import android.support.annotation.Nullable;
 import org.briarproject.bramble.api.FormatException;
 import org.briarproject.bramble.api.UnsupportedVersionException;
 import org.briarproject.bramble.api.contact.ContactManager;
+import org.briarproject.bramble.api.contact.PendingContact;
 import org.briarproject.bramble.api.db.DatabaseExecutor;
 import org.briarproject.bramble.api.db.DbException;
+import org.briarproject.bramble.api.db.NoSuchPendingContactException;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.briar.android.viewmodel.LiveEvent;
 import org.briarproject.briar.android.viewmodel.LiveResult;
@@ -118,4 +120,19 @@ public class AddContactViewModel extends AndroidViewModel {
 		return addContactResult;
 	}
 
+	public void updatePendingContact(String name, PendingContact p) {
+		dbExecutor.execute(() -> {
+			try {
+				contactManager.removePendingContact(p.getId());
+				addContact(name);
+			} catch(NoSuchPendingContactException e) {
+				logException(LOG, WARNING, e);
+				// no error in UI as pending contact was converted into contact
+			} catch (DbException e) {
+				logException(LOG, WARNING, e);
+				addContactResult.postValue(new LiveResult<>(e));
+			}
+		});
+	}
+
 }

briar-android/src/main/java/org/briarproject/briar/android/contact/add/remote/NicknameFragment.java

@@ -2,10 +2,15 @@ package org.briarproject.briar.android.contact.add.remote;
 
 import android.arch.lifecycle.ViewModelProvider;
 import android.arch.lifecycle.ViewModelProviders;
+import android.content.Context;
+import android.content.DialogInterface.OnClickListener;
 import android.content.Intent;
+import android.graphics.drawable.Drawable;
 import android.os.Bundle;
+import android.support.annotation.StringRes;
 import android.support.design.widget.TextInputEditText;
 import android.support.design.widget.TextInputLayout;
+import android.support.v7.app.AlertDialog.Builder;
 import android.text.Editable;
 import android.view.LayoutInflater;
 import android.view.View;
@@ -15,6 +20,10 @@ import android.widget.ProgressBar;
 import android.widget.Toast;
 
 import org.briarproject.bramble.api.UnsupportedVersionException;
+import org.briarproject.bramble.api.contact.PendingContact;
+import org.briarproject.bramble.api.db.ContactExistsException;
+import org.briarproject.bramble.api.db.PendingContactExistsException;
+import org.briarproject.bramble.api.identity.Author;
 import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
 import org.briarproject.briar.R;
@@ -24,9 +33,13 @@ import org.briarproject.briar.android.fragment.BaseFragment;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
 
+import static android.support.v4.content.ContextCompat.getColor;
+import static android.support.v4.content.ContextCompat.getDrawable;
+import static android.support.v4.graphics.drawable.DrawableCompat.setTint;
 import static android.view.View.INVISIBLE;
 import static android.view.View.VISIBLE;
 import static android.widget.Toast.LENGTH_LONG;
+import static java.util.Objects.requireNonNull;
 import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH;
 import static org.briarproject.bramble.util.StringUtils.utf8IsTooLong;
 
@@ -107,23 +120,95 @@ public class NicknameFragment extends BaseFragment {
 
 		viewModel.getAddContactResult().observe(this, result -> {
 			if (result == null) return;
-			if (result.hasError()) {
-				int stringRes;
-				if (result
-						.getException() instanceof UnsupportedVersionException) {
-					stringRes = R.string.unsupported_link;
-				} else {
-					stringRes = R.string.adding_contact_error;
-				}
-				Toast.makeText(getContext(), stringRes, LENGTH_LONG).show();
-			} else {
-				Intent intent = new Intent(getActivity(),
-						PendingContactListActivity.class);
-				startActivity(intent);
-			}
-			finish();
+			if (result.hasError())
+				handleException(name, requireNonNull(result.getException()));
+			else
+				showPendingContactListActivity();
 		});
 		viewModel.addContact(name);
 	}
 
+	private void showPendingContactListActivity() {
+		Intent intent = new Intent(getActivity(),
+				PendingContactListActivity.class);
+		startActivity(intent);
+		finish();
+	}
+
+	private void handleException(String name, Exception e) {
+		if (e instanceof ContactExistsException) {
+			ContactExistsException ce = (ContactExistsException) e;
+			handleExistingContact(name, ce.getRemoteAuthor());
+		} else if (e instanceof PendingContactExistsException) {
+			PendingContactExistsException pe =
+					(PendingContactExistsException) e;
+			handleExistingPendingContact(name, pe.getPendingContact());
+		} else if (e instanceof UnsupportedVersionException) {
+			int stringRes = R.string.unsupported_link;
+			Toast.makeText(getContext(), stringRes, LENGTH_LONG).show();
+			finish();
+		} else {
+			int stringRes = R.string.adding_contact_error;
+			Toast.makeText(getContext(), stringRes, LENGTH_LONG).show();
+			finish();
+		}
+	}
+
+	private void handleExistingContact(String name, Author existing) {
+		OnClickListener listener = (d, w) -> {
+			d.dismiss();
+			String str = getString(R.string.contact_already_exists, name);
+			Toast.makeText(getContext(), str, LENGTH_LONG).show();
+			finish();
+		};
+		showSameLinkDialog(existing.getName(), name,
+				R.string.duplicate_link_dialog_text_1_contact, listener);
+	}
+
+	private void handleExistingPendingContact(String name, PendingContact p) {
+		OnClickListener listener = (d, w) -> {
+			viewModel.updatePendingContact(name, p);
+			Toast.makeText(getContext(), R.string.pending_contact_updated_toast,
+					LENGTH_LONG).show();
+			d.dismiss();
+			showPendingContactListActivity();
+		};
+		showSameLinkDialog(p.getAlias(), name,
+				R.string.duplicate_link_dialog_text_1, listener);
+	}
+
+	private void showSameLinkDialog(String name1, String name2,
+			@StringRes int existsRes, OnClickListener samePersonListener) {
+		Context ctx = requireContext();
+		Builder b = new Builder(ctx, R.style.BriarDialogTheme_Neutral);
+		b.setTitle(getString(R.string.duplicate_link_dialog_title));
+		String msg = getString(existsRes, name1) + "\n\n" +
+				getString(R.string.duplicate_link_dialog_text_2, name2, name1);
+		b.setMessage(msg);
+		b.setPositiveButton(R.string.same_person_button, samePersonListener);
+		b.setNegativeButton(R.string.different_person_button, (d, w) -> {
+			d.dismiss();
+			showWarningDialog(name1, name2);
+		});
+		b.setCancelable(false);
+		b.show();
+	}
+
+	private void showWarningDialog(String name1, String name2) {
+		Context ctx = requireContext();
+		Builder b = new Builder(ctx, R.style.BriarDialogTheme);
+		Drawable icon = getDrawable(ctx, R.drawable.alerts_and_states_error);
+		setTint(requireNonNull(icon), getColor(ctx, R.color.color_primary));
+		b.setIcon(icon);
+		b.setTitle(getString(R.string.duplicate_link_dialog_title));
+		b.setMessage(
+				getString(R.string.duplicate_link_dialog_text_3, name1, name2));
+		b.setPositiveButton(R.string.ok, (d, w) -> {
+			d.dismiss();
+			finish();
+		});
+		b.setCancelable(false);
+		b.show();
+	}
+
 }

briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java

@@ -62,8 +62,7 @@ public class ContactExchangeActivity extends KeyAgreementActivity {
 	@UiThread
 	private void contactExchangeSucceeded(Author remoteAuthor) {
 		String contactName = remoteAuthor.getName();
-		String format = getString(R.string.contact_added_toast);
-		String text = String.format(format, contactName);
+		String text = getString(R.string.contact_added_toast, contactName);
 		Toast.makeText(this, text, LENGTH_LONG).show();
 		supportFinishAfterTransition();
 	}

briar-android/src/main/res/values-zh-rCN/strings.xml

@@ -199,7 +199,7 @@
   <string name="adding_contact_slow_text">添加此联系人比通常花费了更多时间。\n\n请检查您的联系人是否收到您的链接并已添加您：</string>
   <string name="offline_state">无网络连接</string>
   <string name="duplicate_link_dialog_title">重复的链接</string>
-  <string name="duplicate_link_dialog_text_1">您已经有此链接的待处理联系人：</string>
+  <string name="duplicate_link_dialog_text_1">您已经有此链接的待处理联系人：%s</string>
   <!--This is a question asking whether two nicknames refer to the same person-->
   <string name="duplicate_link_dialog_text_2">%s 和 %s 是同一个人吗？</string>
   <!--This is a button for answering that two nicknames do indeed refer to the same person. This

briar-android/src/main/res/values/strings.xml

@@ -227,6 +227,7 @@
 	<string name="offline_state">No Internet connection</string>
 	<string name="duplicate_link_dialog_title">Duplicate Link</string>
 	<string name="duplicate_link_dialog_text_1">You already have a pending contact with this link: %s</string>
+	<string name="duplicate_link_dialog_text_1_contact">You already have a contact with this link: %s</string>
 	<!-- This is a question asking whether two nicknames refer to the same person -->
 	<string name="duplicate_link_dialog_text_2">Are %s and %s the same person?</string>
 	<!-- This is a button for answering that two nicknames do indeed refer to the same person. This