From 11e6d64e4dc66d96e83758b8cc451ab28542011f Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 20 Apr 2016 13:55:59 -0300 Subject: [PATCH] Show relevant decline responses in the conversation * If the user has already declined, we don't show that the other introducee has declined as well. The backend doesn't have that information, so this is compatible with the principle of showing what we know. * If the user has already accepted or hasn't yet responded, we show the decline response in the private conversation with the introducer. If the user hasn't yet responded, we hide the accept/decline buttons in the introduction request message. Messages an introducee receives in a `FINISHED` state are now being ignored and deleted. Closes #295 --- .../IntroductionIntegrationTest.java | 126 ++++++++++++++++-- briar-android/res/values/strings.xml | 1 + .../android/contact/ConversationItem.java | 12 +- .../org/briarproject/api/ProtocolEngine.java | 17 ++- .../api/introduction/IntroductionMessage.java | 15 ++- .../api/introduction/IntroductionRequest.java | 11 +- .../introduction/IntroductionResponse.java | 10 +- .../introduction/IntroduceeEngine.java | 16 +-- .../introduction/IntroduceeManager.java | 25 ++-- .../introduction/IntroducerEngine.java | 6 +- .../introduction/IntroductionManagerImpl.java | 34 +++-- 11 files changed, 219 insertions(+), 54 deletions(-) diff --git a/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTest.java b/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTest.java index 7f2f02be2..ca1026084 100644 --- a/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTest.java +++ b/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTest.java @@ -50,6 +50,7 @@ import javax.inject.Inject; import static org.briarproject.TestPluginsModule.MAX_LATENCY; import static org.briarproject.TestPluginsModule.TRANSPORT_ID; import static org.briarproject.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -293,7 +294,19 @@ public class IntroductionIntegrationTest extends BriarTestCase { assertFalse(contactManager2 .contactExists(author1.getId(), author2.getId())); - assertDefaultUiMessages(); + assertEquals(2, + introductionManager0.getIntroductionMessages(contactId1) + .size()); + assertEquals(2, + introductionManager0.getIntroductionMessages(contactId2) + .size()); + assertEquals(2, + introductionManager1.getIntroductionMessages(contactId0) + .size()); + // introducee2 should also have the decline response of introducee1 + assertEquals(3, + introductionManager2.getIntroductionMessages(contactId0) + .size()); } finally { stopLifecycles(); } @@ -369,6 +382,97 @@ public class IntroductionIntegrationTest extends BriarTestCase { assertFalse(contactManager2 .contactExists(author1.getId(), author2.getId())); + assertEquals(2, + introductionManager0.getIntroductionMessages(contactId1) + .size()); + assertEquals(2, + introductionManager0.getIntroductionMessages(contactId2) + .size()); + // introducee1 also sees the decline response from introducee2 + assertEquals(3, + introductionManager1.getIntroductionMessages(contactId0) + .size()); + assertEquals(2, + introductionManager2.getIntroductionMessages(contactId0) + .size()); + } finally { + stopLifecycles(); + } + } + + @Test + public void testIntroductionSessionDelayedFirstDecline() throws Exception { + startLifecycles(); + try { + // Add Identities + addDefaultIdentities(); + + // Add Transport Properties + addTransportProperties(); + + // Add introducees as contacts + contactId1 = contactManager0.addContact(author1, author0.getId(), + master, clock.currentTimeMillis(), true, true + ); + contactId2 = contactManager0.addContact(author2, author0.getId(), + master, clock.currentTimeMillis(), true, true + ); + // Add introducer back + contactId0 = contactManager1.addContact(author0, author1.getId(), + master, clock.currentTimeMillis(), false, true + ); + ContactId contactId02 = contactManager2.addContact(author0, + author2.getId(), master, clock.currentTimeMillis(), false, + true + ); + assertTrue(contactId0.equals(contactId02)); + + // listen to events + IntroducerListener listener0 = new IntroducerListener(); + t0.getEventBus().addListener(listener0); + IntroduceeListener listener1 = new IntroduceeListener(1, false); + t1.getEventBus().addListener(listener1); + IntroduceeListener listener2 = new IntroduceeListener(2, false); + t2.getEventBus().addListener(listener2); + + // make introduction + long time = clock.currentTimeMillis(); + Contact introducee1 = contactManager0.getContact(contactId1); + Contact introducee2 = contactManager0.getContact(contactId2); + introductionManager0 + .makeIntroduction(introducee1, introducee2, null, time); + + // sync request messages + deliverMessage(sync0, contactId0, sync1, contactId1); + deliverMessage(sync0, contactId0, sync2, contactId2); + + // wait for requests to arrive + eventWaiter.await(TIMEOUT, 2); + assertTrue(listener1.requestReceived); + assertTrue(listener2.requestReceived); + + // sync first response + deliverMessage(sync1, contactId1, sync0, contactId0, "1 to 0"); + eventWaiter.await(TIMEOUT, 1); + assertTrue(listener0.response1Received); + + // sync second response + deliverMessage(sync2, contactId2, sync0, contactId0, "2 to 0"); + eventWaiter.await(TIMEOUT, 1); + assertTrue(listener0.response2Received); + + // sync first forwarded response + deliverMessage(sync0, contactId0, sync2, contactId2); + + // note how the second response will not be forwarded anymore + + assertFalse(contactManager1 + .contactExists(author2.getId(), author1.getId())); + assertFalse(contactManager2 + .contactExists(author1.getId(), author2.getId())); + + // since introducee2 was already in FINISHED state when + // introducee1's response arrived, she ignores and deletes it assertDefaultUiMessages(); } finally { stopLifecycles(); @@ -614,14 +718,18 @@ public class IntroductionIntegrationTest extends BriarTestCase { } private void assertDefaultUiMessages() throws DbException { - assertTrue(introductionManager0.getIntroductionMessages(contactId1) - .size() == 2); - assertTrue(introductionManager0.getIntroductionMessages(contactId2) - .size() == 2); - assertTrue(introductionManager1.getIntroductionMessages(contactId0) - .size() == 2); - assertTrue(introductionManager2.getIntroductionMessages(contactId0) - .size() == 2); + assertEquals(2, + introductionManager0.getIntroductionMessages(contactId1) + .size()); + assertEquals(2, + introductionManager0.getIntroductionMessages(contactId2) + .size()); + assertEquals(2, + introductionManager1.getIntroductionMessages(contactId0) + .size()); + assertEquals(2, + introductionManager2.getIntroductionMessages(contactId0) + .size()); } private class IntroduceeListener implements EventListener { diff --git a/briar-android/res/values/strings.xml b/briar-android/res/values/strings.xml index ca1e962f4..e4452b1bc 100644 --- a/briar-android/res/values/strings.xml +++ b/briar-android/res/values/strings.xml @@ -161,6 +161,7 @@ You declined the introduction to %1$s. %1$s accepted to be introduced to %2$s. %1$s declined to be introduced to %2$s. + %1$s has informed us that %2$s has declined the introduction. Introduced contact was added You have been introduced to %1$s. diff --git a/briar-android/src/org/briarproject/android/contact/ConversationItem.java b/briar-android/src/org/briarproject/android/contact/ConversationItem.java index 2c1492a8c..76eb803a3 100644 --- a/briar-android/src/org/briarproject/android/contact/ConversationItem.java +++ b/briar-android/src/org/briarproject/android/contact/ConversationItem.java @@ -77,9 +77,15 @@ public abstract class ConversationItem { R.string.introduction_response_accepted_received, contactName, ir.getName()); } else { - text = ctx.getString( - R.string.introduction_response_declined_received, - contactName, ir.getName()); + if (ir.isIntroducer()) { + text = ctx.getString( + R.string.introduction_response_declined_received, + contactName, ir.getName()); + } else { + text = ctx.getString( + R.string.introduction_response_declined_received_by_introducee, + contactName, ir.getName()); + } } return new ConversationNoticeInItem(ir.getMessageId(), text, ir.getTime(), ir.isRead()); diff --git a/briar-api/src/org/briarproject/api/ProtocolEngine.java b/briar-api/src/org/briarproject/api/ProtocolEngine.java index 65d9fe52e..61833f657 100644 --- a/briar-api/src/org/briarproject/api/ProtocolEngine.java +++ b/briar-api/src/org/briarproject/api/ProtocolEngine.java @@ -12,16 +12,27 @@ public interface ProtocolEngine { StateUpdate onMessageDelivered(S localState, M delivered); class StateUpdate { - public final boolean deleteMessages; + public final boolean deleteMessage; public final boolean deleteState; public final S localState; public final List toSend; public final List toBroadcast; - public StateUpdate(boolean deleteMessages, boolean deleteState, + /** + * This class represents an update of the local protocol state. + * It only shows how the state should be updated, + * but does not carry out the updates on its own. + * + * @param deleteMessage whether to delete the message that triggered the state update. This will be ignored for {@link ProtocolEngine#onLocalAction}. + * @param deleteState whether to delete the localState {@link S} + * @param localState the new local state + * @param toSend a list of messages to be sent as part of the state update + * @param toBroadcast a list of events to broadcast as result of the state update + */ + public StateUpdate(boolean deleteMessage, boolean deleteState, S localState, List toSend, List toBroadcast) { - this.deleteMessages = deleteMessages; + this.deleteMessage = deleteMessage; this.deleteState = deleteState; this.localState = localState; this.toSend = toSend; diff --git a/briar-api/src/org/briarproject/api/introduction/IntroductionMessage.java b/briar-api/src/org/briarproject/api/introduction/IntroductionMessage.java index 6ac98793f..8582f36c6 100644 --- a/briar-api/src/org/briarproject/api/introduction/IntroductionMessage.java +++ b/briar-api/src/org/briarproject/api/introduction/IntroductionMessage.java @@ -2,19 +2,24 @@ package org.briarproject.api.introduction; import org.briarproject.api.sync.MessageId; +import static org.briarproject.api.introduction.IntroductionConstants.ROLE_INTRODUCEE; +import static org.briarproject.api.introduction.IntroductionConstants.ROLE_INTRODUCER; + abstract public class IntroductionMessage { private final SessionId sessionId; private final MessageId messageId; + private final int role; private final long time; private final boolean local, sent, seen, read; public IntroductionMessage(SessionId sessionId, MessageId messageId, - long time, boolean local, boolean sent, boolean seen, + int role, long time, boolean local, boolean sent, boolean seen, boolean read) { this.sessionId = sessionId; this.messageId = messageId; + this.role = role; this.time = time; this.local = local; this.sent = sent; @@ -50,5 +55,13 @@ abstract public class IntroductionMessage { return read; } + public boolean isIntroducer() { + return role == ROLE_INTRODUCER; + } + + public boolean isIntroducee() { + return role == ROLE_INTRODUCEE; + } + } diff --git a/briar-api/src/org/briarproject/api/introduction/IntroductionRequest.java b/briar-api/src/org/briarproject/api/introduction/IntroductionRequest.java index 163eb57be..dc674f77b 100644 --- a/briar-api/src/org/briarproject/api/introduction/IntroductionRequest.java +++ b/briar-api/src/org/briarproject/api/introduction/IntroductionRequest.java @@ -9,12 +9,13 @@ public class IntroductionRequest extends IntroductionResponse { private final boolean answered, exists, introducesOtherIdentity; public IntroductionRequest(SessionId sessionId, MessageId messageId, - long time, boolean local, boolean sent, boolean seen, boolean read, - AuthorId authorId, String name, boolean accepted, String message, - boolean answered, boolean exists, boolean introducesOtherIdentity) { + int role, long time, boolean local, boolean sent, boolean seen, + boolean read, AuthorId authorId, String name, boolean accepted, + String message, boolean answered, boolean exists, + boolean introducesOtherIdentity) { - super(sessionId, messageId, time, local, sent, seen, read, authorId, - name, accepted); + super(sessionId, messageId, role, time, local, sent, seen, read, + authorId, name, accepted); this.message = message; this.answered = answered; diff --git a/briar-api/src/org/briarproject/api/introduction/IntroductionResponse.java b/briar-api/src/org/briarproject/api/introduction/IntroductionResponse.java index e73065bcc..956131ccb 100644 --- a/briar-api/src/org/briarproject/api/introduction/IntroductionResponse.java +++ b/briar-api/src/org/briarproject/api/introduction/IntroductionResponse.java @@ -10,10 +10,11 @@ public class IntroductionResponse extends IntroductionMessage { private final boolean accepted; public IntroductionResponse(SessionId sessionId, MessageId messageId, - long time, boolean local, boolean sent, boolean seen, boolean read, - AuthorId remoteAuthorId, String name, boolean accepted) { + int role, long time, boolean local, boolean sent, boolean seen, + boolean read, AuthorId remoteAuthorId, String name, + boolean accepted) { - super(sessionId, messageId, time, local, sent, seen, read); + super(sessionId, messageId, role, time, local, sent, seen, read); this.remoteAuthorId = remoteAuthorId; this.name = name; @@ -28,4 +29,7 @@ public class IntroductionResponse extends IntroductionMessage { return accepted; } + public AuthorId getRemoteAuthorId() { + return remoteAuthorId; + } } diff --git a/briar-core/src/org/briarproject/introduction/IntroduceeEngine.java b/briar-core/src/org/briarproject/introduction/IntroduceeEngine.java index ad6e005f6..cb1ee7e02 100644 --- a/briar-core/src/org/briarproject/introduction/IntroduceeEngine.java +++ b/briar-core/src/org/briarproject/introduction/IntroduceeEngine.java @@ -51,6 +51,7 @@ import static org.briarproject.api.introduction.IntroductionConstants.OUR_TIME; import static org.briarproject.api.introduction.IntroductionConstants.PUBLIC_KEY; import static org.briarproject.api.introduction.IntroductionConstants.REMOTE_AUTHOR_ID; import static org.briarproject.api.introduction.IntroductionConstants.REMOTE_AUTHOR_IS_US; +import static org.briarproject.api.introduction.IntroductionConstants.ROLE_INTRODUCEE; import static org.briarproject.api.introduction.IntroductionConstants.SESSION_ID; import static org.briarproject.api.introduction.IntroductionConstants.STATE; import static org.briarproject.api.introduction.IntroductionConstants.TASK; @@ -195,14 +196,11 @@ public class IntroduceeEngine messages = Collections.emptyList(); events = Collections.emptyList(); } - // we are done (probably declined response) and ignore this message + // we are done (probably declined response), ignore & delete message else if (currentState == FINISHED) { - if(action == REMOTE_DECLINE || action == REMOTE_ACCEPT) { - // record response data, - // so we later know which response was ours - addResponseData(localState, msg); - } - return noUpdate(localState); + return new StateUpdate(true, + false, localState, new ArrayList(0), + new ArrayList(0)); } // this should not happen else { @@ -341,8 +339,8 @@ public class IntroduceeEngine localState.getBoolean(REMOTE_AUTHOR_IS_US); IntroductionRequest ir = new IntroductionRequest(sessionId, messageId, - time, false, false, false, false, authorId, name, false, - message, false, exists, introducesOtherIdentity); + ROLE_INTRODUCEE, time, false, false, false, false, authorId, + name, false, message, false, exists, introducesOtherIdentity); return new IntroductionRequestReceivedEvent(contactId, ir); } diff --git a/briar-core/src/org/briarproject/introduction/IntroduceeManager.java b/briar-core/src/org/briarproject/introduction/IntroduceeManager.java index b329a3abf..c0f9506f5 100644 --- a/briar-core/src/org/briarproject/introduction/IntroduceeManager.java +++ b/briar-core/src/org/briarproject/introduction/IntroduceeManager.java @@ -1,6 +1,5 @@ package org.briarproject.introduction; - import org.briarproject.api.Bytes; import org.briarproject.api.FormatException; import org.briarproject.api.TransportId; @@ -39,6 +38,7 @@ import java.util.logging.Logger; import javax.inject.Inject; +import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; import static org.briarproject.api.introduction.IntroduceeProtocolState.AWAIT_REQUEST; import static org.briarproject.api.introduction.IntroductionConstants.ACCEPT; @@ -51,6 +51,7 @@ import static org.briarproject.api.introduction.IntroductionConstants.E_PUBLIC_K import static org.briarproject.api.introduction.IntroductionConstants.GROUP_ID; import static org.briarproject.api.introduction.IntroductionConstants.INTRODUCER; import static org.briarproject.api.introduction.IntroductionConstants.LOCAL_AUTHOR_ID; +import static org.briarproject.api.introduction.IntroductionConstants.MESSAGE_ID; import static org.briarproject.api.introduction.IntroductionConstants.MESSAGE_TIME; import static org.briarproject.api.introduction.IntroductionConstants.NAME; import static org.briarproject.api.introduction.IntroductionConstants.NOT_OUR_RESPONSE; @@ -170,7 +171,7 @@ class IntroduceeManager { BdfDictionary message) throws DbException, FormatException { IntroduceeEngine engine = new IntroduceeEngine(); - processStateUpdate(txn, engine.onMessageReceived(state, message)); + processStateUpdate(txn, message, engine.onMessageReceived(state, message)); } public void acceptIntroduction(Transaction txn, BdfDictionary state, @@ -200,7 +201,7 @@ class IntroduceeManager { // start engine and process its state update IntroduceeEngine engine = new IntroduceeEngine(); - processStateUpdate(txn, engine.onLocalAction(state, localAction)); + processStateUpdate(txn, null, engine.onLocalAction(state, localAction)); } public void declineIntroduction(Transaction txn, BdfDictionary state, @@ -217,11 +218,11 @@ class IntroduceeManager { // start engine and process its state update IntroduceeEngine engine = new IntroduceeEngine(); - processStateUpdate(txn, + processStateUpdate(txn, null, engine.onLocalAction(state, localAction)); } - private void processStateUpdate(Transaction txn, + private void processStateUpdate(Transaction txn, BdfDictionary msg, IntroduceeEngine.StateUpdate result) throws DbException, FormatException { @@ -242,6 +243,16 @@ class IntroduceeManager { for (Event event : result.toBroadcast) { txn.attach(event); } + + // delete message + if (result.deleteMessage && msg != null) { + MessageId messageId = new MessageId(msg.getRaw(MESSAGE_ID)); + if (LOG.isLoggable(INFO)) { + LOG.info("Deleting message with id " + messageId.hashCode()); + } + db.deleteMessage(txn, messageId); + db.deleteMessageMetadata(txn, messageId); + } } private void performTasks(Transaction txn, BdfDictionary localState) @@ -253,8 +264,6 @@ class IntroduceeManager { long task = localState.getLong(TASK); localState.put(TASK, BdfDictionary.NULL_VALUE); - - if (task == TASK_ADD_CONTACT) { if (localState.getBoolean(EXISTS)) { // we have this contact already, so do not perform actions @@ -374,7 +383,7 @@ class IntroduceeManager { BdfDictionary localAction = new BdfDictionary(); localAction.put(TYPE, TYPE_ABORT); try { - processStateUpdate(txn, + processStateUpdate(txn, null, engine.onLocalAction(state, localAction)); } catch (DbException e) { if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); diff --git a/briar-core/src/org/briarproject/introduction/IntroducerEngine.java b/briar-core/src/org/briarproject/introduction/IntroducerEngine.java index 6beacb934..6af05d14b 100644 --- a/briar-core/src/org/briarproject/introduction/IntroducerEngine.java +++ b/briar-core/src/org/briarproject/introduction/IntroducerEngine.java @@ -55,6 +55,7 @@ import static org.briarproject.api.introduction.IntroductionConstants.PUBLIC_KEY import static org.briarproject.api.introduction.IntroductionConstants.PUBLIC_KEY2; import static org.briarproject.api.introduction.IntroductionConstants.RESPONSE_1; import static org.briarproject.api.introduction.IntroductionConstants.RESPONSE_2; +import static org.briarproject.api.introduction.IntroductionConstants.ROLE_INTRODUCER; import static org.briarproject.api.introduction.IntroductionConstants.SESSION_ID; import static org.briarproject.api.introduction.IntroductionConstants.STATE; import static org.briarproject.api.introduction.IntroductionConstants.TYPE; @@ -302,8 +303,9 @@ public class IntroducerEngine boolean accept = msg.getBoolean(ACCEPT); IntroductionResponse ir = - new IntroductionResponse(sessionId, messageId, time, false, - false, false, false, authorId, name, accept); + new IntroductionResponse(sessionId, messageId, ROLE_INTRODUCER, + time, false, false, false, false, authorId, name, + accept); return new IntroductionResponseReceivedEvent(contactId, ir); } diff --git a/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java b/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java index 24c89324e..0d0c3102b 100644 --- a/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java +++ b/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java @@ -41,6 +41,7 @@ import java.util.logging.Logger; import javax.inject.Inject; import static java.util.logging.Level.WARNING; +import static org.briarproject.api.introduction.IntroduceeProtocolState.FINISHED; import static org.briarproject.api.introduction.IntroductionConstants.ACCEPT; import static org.briarproject.api.introduction.IntroductionConstants.ANSWERED; import static org.briarproject.api.introduction.IntroductionConstants.AUTHOR_ID_1; @@ -331,6 +332,7 @@ class IntroductionManagerImpl extends BdfIncomingMessageHook BdfDictionary state = getSessionState(txn, g, sessionId.getBytes()); + int role = state.getLong(ROLE).intValue(); boolean local; long time = msg.getLong(MESSAGE_TIME); boolean accepted = msg.getBoolean(ACCEPT, false); @@ -338,7 +340,7 @@ class IntroductionManagerImpl extends BdfIncomingMessageHook AuthorId authorId; String name; if (type == TYPE_RESPONSE) { - if (state.getLong(ROLE) == ROLE_INTRODUCER) { + if (role == ROLE_INTRODUCER) { if (!concernsThisContact(contactId, messageId, state)) { // this response is not from contactId continue; @@ -350,22 +352,30 @@ class IntroductionManagerImpl extends BdfIncomingMessageHook } else { if (Arrays.equals(state.getRaw(NOT_OUR_RESPONSE), messageId.getBytes())) { - // this response is not ours, don't include it - continue; + // this response is not ours, + // check if it was a decline + if (!accepted) { + local = false; + } else { + // don't include positive responses + continue; + } + } else { + local = true; } - local = true; authorId = new AuthorId( state.getRaw(REMOTE_AUTHOR_ID)); name = state.getString(NAME); } IntroductionResponse ir = new IntroductionResponse( - sessionId, messageId, time, local, s.isSent(), - s.isSeen(), read, authorId, name, accepted); + sessionId, messageId, role, time, local, + s.isSent(), s.isSeen(), read, authorId, name, + accepted); list.add(ir); } else if (type == TYPE_REQUEST) { String message; boolean answered, exists, introducesOtherIdentity; - if (state.getLong(ROLE) == ROLE_INTRODUCER) { + if (role == ROLE_INTRODUCER) { local = true; authorId = getAuthorIdForIntroducer(contactId, state); @@ -380,15 +390,17 @@ class IntroductionManagerImpl extends BdfIncomingMessageHook state.getRaw(REMOTE_AUTHOR_ID)); name = state.getString(NAME); message = state.getOptionalString(MSG); - answered = state.getBoolean(ANSWERED); + boolean finished = state.getLong(STATE) == + FINISHED.getValue(); + answered = finished || state.getBoolean(ANSWERED); exists = state.getBoolean(EXISTS); introducesOtherIdentity = state.getBoolean(REMOTE_AUTHOR_IS_US); } IntroductionRequest ir = new IntroductionRequest( - sessionId, messageId, time, local, s.isSent(), - s.isSeen(), read, authorId, name, accepted, - message, answered, exists, + sessionId, messageId, role, time, local, + s.isSent(), s.isSeen(), read, authorId, name, + accepted, message, answered, exists, introducesOtherIdentity); list.add(ir); }