Merge branch '474-manual-decline' into 'master'

Don't automatically respond to declined introductions

See merge request akwizgran/briar!777

briar-android/src/main/java/org/briarproject/briar/android/contact/ConversationActivity.java

@@ -575,10 +575,6 @@ public class ConversationActivity extends BriarActivity
 			@Override
 			public void onSuccess(String contactName) {
 				runOnUiThreadUnlessDestroyed(() -> {
-					// If the other introducee declined, we can no longer
-					// respond to the request
-					if (!m.isIntroducer() && !m.wasAccepted())
-						markRequestAnswered(m.getSessionId());
 					ConversationItem item = ConversationItem
 							.from(ConversationActivity.this, contactName, m);
 					addConversationItem(item);
@@ -631,26 +627,6 @@ public class ConversationActivity extends BriarActivity
 		});
 	}
 
-	private void markRequestAnswered(SessionId sessionId) {
-		int size = adapter.getItemCount();
-		for (int i = 0; i < size; i++) {
-			ConversationItem item = adapter.getItemAt(i);
-			if (item instanceof ConversationRequestItem) {
-				ConversationRequestItem req = (ConversationRequestItem) item;
-				if (req.getSessionId().equals(sessionId)
-						&& !req.wasAnswered()) {
-					LOG.info("Marking request answered");
-					req.setAnswered(true);
-					int position = adapter.findItemPosition(req);
-					if (position != INVALID_POSITION)
-						adapter.notifyItemChanged(position, req);
-					// There shouldn't be more than one unanswered request
-					return;
-				}
-			}
-		}
-	}
-
 	private void markMessages(Collection<MessageId> messageIds,
 			boolean sent, boolean seen) {
 		runOnUiThreadUnlessDestroyed(() -> {

briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java

@@ -46,6 +46,7 @@ import static org.briarproject.briar.introduction.IntroduceeState.AWAIT_RESPONSE
 import static org.briarproject.briar.introduction.IntroduceeState.LOCAL_ACCEPTED;
 import static org.briarproject.briar.introduction.IntroduceeState.LOCAL_DECLINED;
 import static org.briarproject.briar.introduction.IntroduceeState.REMOTE_ACCEPTED;
+import static org.briarproject.briar.introduction.IntroduceeState.REMOTE_DECLINED;
 import static org.briarproject.briar.introduction.IntroduceeState.START;
 
 @Immutable
@@ -94,6 +95,7 @@ class IntroduceeProtocolEngine
 			IntroduceeSession session, long timestamp) throws DbException {
 		switch (session.getState()) {
 			case AWAIT_RESPONSES:
+			case REMOTE_DECLINED:
 			case REMOTE_ACCEPTED:
 				return onLocalAccept(txn, session, timestamp);
 			case START:
@@ -112,6 +114,7 @@ class IntroduceeProtocolEngine
 			IntroduceeSession session, long timestamp) throws DbException {
 		switch (session.getState()) {
 			case AWAIT_RESPONSES:
+			case REMOTE_DECLINED:
 			case REMOTE_ACCEPTED:
 				return onLocalDecline(txn, session, timestamp);
 			case START:
@@ -133,6 +136,7 @@ class IntroduceeProtocolEngine
 				return onRemoteRequest(txn, session, m);
 			case AWAIT_RESPONSES:
 			case LOCAL_DECLINED:
+			case REMOTE_DECLINED:
 			case LOCAL_ACCEPTED:
 			case REMOTE_ACCEPTED:
 			case AWAIT_AUTH:
@@ -153,6 +157,7 @@ class IntroduceeProtocolEngine
 			case LOCAL_ACCEPTED:
 				return onRemoteAccept(txn, session, m);
 			case START:
+			case REMOTE_DECLINED:
 			case REMOTE_ACCEPTED:
 			case AWAIT_AUTH:
 			case AWAIT_ACTIVATE:
@@ -172,6 +177,7 @@ class IntroduceeProtocolEngine
 			case LOCAL_ACCEPTED:
 				return onRemoteDecline(txn, session, m);
 			case START:
+			case REMOTE_DECLINED:
 			case REMOTE_ACCEPTED:
 			case AWAIT_AUTH:
 			case AWAIT_ACTIVATE:
@@ -190,6 +196,7 @@ class IntroduceeProtocolEngine
 			case START:
 			case AWAIT_RESPONSES:
 			case LOCAL_DECLINED:
+			case REMOTE_DECLINED:
 			case LOCAL_ACCEPTED:
 			case REMOTE_ACCEPTED:
 			case AWAIT_ACTIVATE:
@@ -208,6 +215,7 @@ class IntroduceeProtocolEngine
 			case START:
 			case AWAIT_RESPONSES:
 			case LOCAL_DECLINED:
+			case REMOTE_DECLINED:
 			case LOCAL_ACCEPTED:
 			case REMOTE_ACCEPTED:
 			case AWAIT_AUTH:
@@ -272,26 +280,28 @@ class IntroduceeProtocolEngine
 				transportPropertyManager.getLocalProperties(txn);
 
 		// Send a ACCEPT message
-		long localTimestamp =
-				Math.max(timestamp + 1, getLocalTimestamp(s));
+		long localTimestamp = Math.max(timestamp + 1, getLocalTimestamp(s));
 		Message sent = sendAcceptMessage(txn, s, localTimestamp, publicKey,
 				localTimestamp, transportProperties, true);
 		// Track the message
 		messageTracker.trackOutgoingMessage(txn, sent);
 
 		// Determine the next state
-		IntroduceeState state =
-				s.getState() == AWAIT_RESPONSES ? LOCAL_ACCEPTED : AWAIT_AUTH;
-		IntroduceeSession sNew = IntroduceeSession
-				.addLocalAccept(s, state, sent, publicKey, privateKey,
-						localTimestamp, transportProperties);
-
-		if (state == AWAIT_AUTH) {
-			// Move to the AWAIT_AUTH state
-			return onLocalAuth(txn, sNew);
+		switch (s.getState()) {
+			case AWAIT_RESPONSES:
+				return IntroduceeSession.addLocalAccept(s, LOCAL_ACCEPTED, sent,
+						publicKey, privateKey, localTimestamp,
+						transportProperties);
+			case REMOTE_DECLINED:
+				return IntroduceeSession.clear(s, START, sent.getId(),
+						localTimestamp, s.getLastRemoteMessageId());
+			case REMOTE_ACCEPTED:
+				return onLocalAuth(txn, IntroduceeSession.addLocalAccept(s,
+						AWAIT_AUTH, sent, publicKey, privateKey, localTimestamp,
+						transportProperties));
+			default:
+				throw new AssertionError();
 		}
-		// Move to the LOCAL_ACCEPTED state
-		return sNew;
 	}
 
 	private IntroduceeSession onLocalDecline(Transaction txn,
@@ -308,10 +318,9 @@ class IntroduceeProtocolEngine
 
 		// Move to the START or LOCAL_DECLINED state, if still awaiting response
 		IntroduceeState state =
-				s.getState() == REMOTE_ACCEPTED ? START : LOCAL_DECLINED;
-		return IntroduceeSession
-				.clear(s, state, sent.getId(), sent.getTimestamp(),
-						s.getLastRemoteMessageId());
+				s.getState() == AWAIT_RESPONSES ? LOCAL_DECLINED : START;
+		return IntroduceeSession.clear(s, state, sent.getId(),
+				sent.getTimestamp(), s.getLastRemoteMessageId());
 	}
 
 	private IntroduceeSession onRemoteAccept(Transaction txn,
@@ -357,22 +366,10 @@ class IntroduceeProtocolEngine
 		broadcastIntroductionResponseReceivedEvent(txn, s,
 				s.getIntroducer().getId(), s.getRemote().author, m);
 
-		if (s.getState() == AWAIT_RESPONSES) {
-			// Mark the request message unavailable to answer
-			markRequestsUnavailableToAnswer(txn, s);
-
-			// Send a DECLINE message
-			Message sent =
-					sendDeclineMessage(txn, s, getLocalTimestamp(s), false);
-
-			// Move back to START state
-			return IntroduceeSession.clear(s, START, sent.getId(),
-					sent.getTimestamp(), m.getMessageId());
-		} else {
-			// Move back to START state
-			return IntroduceeSession.clear(s, START, s.getLastLocalMessageId(),
-					s.getLocalTimestamp(), m.getMessageId());
-		}
+		// Move to REMOTE_DECLINED state
+		return IntroduceeSession.clear(s, REMOTE_DECLINED,
+				s.getLastLocalMessageId(), s.getLocalTimestamp(),
+				m.getMessageId());
 	}
 
 	private IntroduceeSession onRemoteResponseWhenDeclined(Transaction txn,
@@ -385,6 +382,17 @@ class IntroduceeProtocolEngine
 		if (isInvalidDependency(s, m.getPreviousMessageId()))
 			return abort(txn, s);
 
+		// Mark the response visible in the UI
+		markMessageVisibleInUi(txn, m.getMessageId());
+
+		// Track the incoming message
+		messageTracker
+				.trackMessage(txn, m.getGroupId(), m.getTimestamp(), false);
+
+		// Broadcast IntroductionResponseReceivedEvent
+		broadcastIntroductionResponseReceivedEvent(txn, s,
+				s.getIntroducer().getId(), s.getRemote().author, m);
+
 		// Move to START state
 		return IntroduceeSession.clear(s, START, s.getLastLocalMessageId(),
 				s.getLocalTimestamp(), m.getMessageId());
@@ -525,9 +533,8 @@ class IntroduceeProtocolEngine
 		txn.attach(new IntroductionAbortedEvent(s.getSessionId()));
 
 		// Reset the session back to initial state
-		return IntroduceeSession
-				.clear(s, START, sent.getId(), sent.getTimestamp(),
-						s.getLastRemoteMessageId());
+		return IntroduceeSession.clear(s, START, sent.getId(),
+				sent.getTimestamp(), s.getLastRemoteMessageId());
 	}
 
 	private boolean isInvalidDependency(IntroduceeSession s,

briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeState.java

@@ -12,10 +12,11 @@ enum IntroduceeState implements State {
 	START(0),
 	AWAIT_RESPONSES(1),
 	LOCAL_DECLINED(2),
-	LOCAL_ACCEPTED(3),
-	REMOTE_ACCEPTED(4),
-	AWAIT_AUTH(5),
-	AWAIT_ACTIVATE(6);
+	REMOTE_DECLINED(3),
+	LOCAL_ACCEPTED(4),
+	REMOTE_ACCEPTED(5),
+	AWAIT_AUTH(6),
+	AWAIT_ACTIVATE(7);
 
 	private final int value;
 

briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java

@@ -17,7 +17,6 @@ import org.briarproject.bramble.api.identity.Author;
 import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
-import org.briarproject.bramble.api.properties.TransportProperties;
 import org.briarproject.bramble.api.properties.TransportPropertyManager;
 import org.briarproject.bramble.api.sync.Group;
 import org.briarproject.bramble.api.sync.Message;
@@ -39,15 +38,12 @@ import org.junit.Test;
 
 import java.io.IOException;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Map;
 import java.util.concurrent.TimeoutException;
-import java.util.logging.Logger;
 
 import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH;
 import static org.briarproject.bramble.test.TestPluginConfigModule.TRANSPORT_ID;
 import static org.briarproject.bramble.test.TestUtils.getRandomBytes;
-import static org.briarproject.bramble.test.TestUtils.getTransportId;
 import static org.briarproject.bramble.test.TestUtils.getTransportProperties;
 import static org.briarproject.bramble.test.TestUtils.getTransportPropertiesMap;
 import static org.briarproject.briar.api.introduction.IntroductionManager.CLIENT_ID;
@@ -86,9 +82,6 @@ public class IntroductionIntegrationTest
 	private IntroduceeListener listener1;
 	private IntroduceeListener listener2;
 
-	private static final Logger LOG =
-			Logger.getLogger(IntroductionIntegrationTest.class.getName());
-
 	interface StateVisitor {
 		AcceptMessage visit(AcceptMessage response);
 	}
@@ -374,11 +367,10 @@ public class IntroductionIntegrationTest
 		assertEquals(2,
 				introductionManager0.getIntroductionMessages(contactId2From0)
 						.size());
-		// introducee1 also sees the decline response from introducee2
 		assertEquals(3,
 				introductionManager1.getIntroductionMessages(contactId0From1)
 						.size());
-		assertEquals(2,
+		assertEquals(3,
 				introductionManager2.getIntroductionMessages(contactId0From2)
 						.size());
 		assertFalse(listener0.aborted);
@@ -386,58 +378,6 @@ public class IntroductionIntegrationTest
 		assertFalse(listener2.aborted);
 	}
 
-	@Test
-	public void testIntroductionSessionDelayedFirstDecline() throws Exception {
-		addListeners(false, false);
-
-		// make introduction
-		long time = clock.currentTimeMillis();
-		introductionManager0
-				.makeIntroduction(contact1From0, contact2From0, null, time);
-
-		// sync request messages
-		sync0To1(1, true);
-		sync0To2(1, true);
-
-		// wait for requests to arrive
-		eventWaiter.await(TIMEOUT, 2);
-		assertTrue(listener1.requestReceived);
-		assertTrue(listener2.requestReceived);
-
-		// sync first response
-		sync1To0(1, true);
-		eventWaiter.await(TIMEOUT, 1);
-		assertTrue(listener0.response1Received);
-
-		// sync fake transport properties back to 1, so Message ACK can arrive
-		// and the assertDefaultUiMessages() check at the end will not fail
-		TransportProperties tp = new TransportProperties(
-				Collections.singletonMap("key", "value"));
-		c0.getTransportPropertyManager()
-				.mergeLocalProperties(getTransportId(), tp);
-		sync0To1(1, true);
-
-		// sync second response
-		sync2To0(1, true);
-		eventWaiter.await(TIMEOUT, 1);
-		assertTrue(listener0.response2Received);
-
-		// sync first forwarded response
-		sync0To2(1, true);
-
-		// note how the second response will not be forwarded anymore
-
-		assertFalse(contactManager1
-				.contactExists(author2.getId(), author1.getId()));
-		assertFalse(contactManager2
-				.contactExists(author1.getId(), author2.getId()));
-
-		assertDefaultUiMessages();
-		assertFalse(listener0.aborted);
-		assertFalse(listener1.aborted);
-		assertFalse(listener2.aborted);
-	}
-
 	@Test
 	public void testResponseAndAuthInOneSync() throws Exception {
 		addListeners(true, true);
@@ -467,9 +407,8 @@ public class IntroductionIntegrationTest
 		assertTrue(listener2.requestReceived);
 
 		// answer request manually
-		introductionManager2
-				.respondToIntroduction(contactId0From2, listener2.sessionId, time,
-						true);
+		introductionManager2.respondToIntroduction(contactId0From2,
+				listener2.sessionId, time, true);
 
 		// sync second response and AUTH
 		sync2To0(2, true);
@@ -479,7 +418,7 @@ public class IntroductionIntegrationTest
 		// Forward AUTH
 		sync0To1(1, true);
 
-		// Second AUTH and ACTIATE and forward them
+		// Second AUTH and ACTIVATE and forward them
 		sync1To0(2, true);
 		sync0To2(2, true);
 
@@ -495,13 +434,12 @@ public class IntroductionIntegrationTest
 	}
 
 	/**
-	 * When an introducee declines an introduction,
-	 * the other introducee needs to respond before returning to START state,
-	 * otherwise a subsequent attempt at introducing the same contacts will fail
+	 * When an introducee declines an introduction, the other introducee needs
+	 * to respond before returning to the START state, otherwise a subsequent
+	 * attempt at introducing the same contacts will fail.
 	 */
 	@Test
-	public void testAutomaticSecondDecline() throws Exception {
-		// introducee1 declines automatically and introducee2 doesn't answer
+	public void testIntroductionSessionManualDecline() throws Exception {
 		addListeners(false, true);
 		listener2.answerRequests = false;
 
@@ -547,9 +485,18 @@ public class IntroductionIntegrationTest
 
 		// assert that introducee2 is in correct state
 		introduceeSession = getIntroduceeSession(c2);
+		assertEquals(IntroduceeState.REMOTE_DECLINED,
+				introduceeSession.getState());
+
+		// answer request manually
+		introductionManager2.respondToIntroduction(contactId0From2,
+				listener2.sessionId, time, false);
+
+		// now introducee2 should have returned to the START state
+		introduceeSession = getIntroduceeSession(c2);
 		assertEquals(IntroduceeState.START, introduceeSession.getState());
 
-		// second response should be an immediate automatic DECLINE
+		// sync second response
 		sync2To0(1, true);
 		eventWaiter.await(TIMEOUT, 1);
 		assertTrue(listener0.response2Received);
@@ -562,7 +509,7 @@ public class IntroductionIntegrationTest
 		introduceeSession = getIntroduceeSession(c1);
 		assertEquals(LOCAL_DECLINED, introduceeSession.getState());
 
-		// forward automatic decline
+		// forward second response
 		sync0To1(1, true);
 
 		// introducee1 can finally move to the START
@@ -579,16 +526,14 @@ public class IntroductionIntegrationTest
 				introductionManager0.getIntroductionMessages(contactId2From0)
 						.size());
 		assertGroupCount(messageTracker0, g2.getId(), 2, 1);
-		assertEquals(2,
+		assertEquals(3,
 				introductionManager1.getIntroductionMessages(contactId0From1)
 						.size());
-		assertGroupCount(messageTracker1, g1.getId(), 2, 1);
-		// the automatic DECLINE is invisible in the UI
-		// so there's just the remote REQUEST and remote DECLINE
-		assertEquals(2,
+		assertGroupCount(messageTracker1, g1.getId(), 3, 2);
+		assertEquals(3,
 				introductionManager2.getIntroductionMessages(contactId0From2)
 						.size());
-		assertGroupCount(messageTracker2, g2.getId(), 2, 2);
+		assertGroupCount(messageTracker2, g2.getId(), 3, 2);
 
 		assertFalse(listener0.aborted);
 		assertFalse(listener1.aborted);