From 720dda784e6bb478e0452efb499ce5bcf1e7d537 Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Thu, 12 Jan 2017 16:43:44 -0200
Subject: [PATCH] Remove error state and reset session on error instead
---
.../briar/sharing/BlogProtocolEngineImpl.java | 5 +-
.../sharing/ForumProtocolEngineImpl.java | 5 +-
.../briar/sharing/ProtocolEngineImpl.java | 115 +++++++++---------
.../org/briarproject/briar/sharing/State.java | 2 +-
.../sharing/ForumSharingIntegrationTest.java | 73 +++++++++++
.../briar/test/BriarIntegrationTest.java | 3 +
.../test/BriarIntegrationTestComponent.java | 3 -
7 files changed, 137 insertions(+), 69 deletions(-)
diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/BlogProtocolEngineImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/BlogProtocolEngineImpl.java
index aa279be427..3083387b6b 100644
--- a/briar-core/src/main/java/org/briarproject/briar/sharing/BlogProtocolEngineImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/sharing/BlogProtocolEngineImpl.java
@@ -14,7 +14,6 @@ import org.briarproject.bramble.api.system.Clock;
import org.briarproject.briar.api.blog.Blog;
import org.briarproject.briar.api.blog.BlogInvitationResponse;
import org.briarproject.briar.api.blog.BlogManager;
-import org.briarproject.briar.api.blog.BlogSharingManager;
import org.briarproject.briar.api.blog.event.BlogInvitationRequestReceivedEvent;
import org.briarproject.briar.api.blog.event.BlogInvitationResponseReceivedEvent;
import org.briarproject.briar.api.client.MessageTracker;
@@ -74,8 +73,8 @@ class BlogProtocolEngineImpl extends ProtocolEngineImpl<Blog> {
}
@Override
- protected ClientId getClientId() {
- return BlogSharingManager.CLIENT_ID;
+ protected ClientId getShareableClientId() {
+ return BlogManager.CLIENT_ID;
}
@Override
diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/ForumProtocolEngineImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/ForumProtocolEngineImpl.java
index 11530af843..9fb695e507 100644
--- a/briar-core/src/main/java/org/briarproject/briar/sharing/ForumProtocolEngineImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/sharing/ForumProtocolEngineImpl.java
@@ -15,7 +15,6 @@ import org.briarproject.briar.api.client.MessageTracker;
import org.briarproject.briar.api.forum.Forum;
import org.briarproject.briar.api.forum.ForumInvitationResponse;
import org.briarproject.briar.api.forum.ForumManager;
-import org.briarproject.briar.api.forum.ForumSharingManager;
import org.briarproject.briar.api.forum.event.ForumInvitationRequestReceivedEvent;
import org.briarproject.briar.api.forum.event.ForumInvitationResponseReceivedEvent;
import org.briarproject.briar.api.sharing.InvitationRequest;
@@ -74,8 +73,8 @@ class ForumProtocolEngineImpl extends ProtocolEngineImpl<Forum> {
}
@Override
- protected ClientId getClientId() {
- return ForumSharingManager.CLIENT_ID;
+ protected ClientId getShareableClientId() {
+ return ForumManager.CLIENT_ID;
}
@Override
diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java
index 4cc51ecb26..7064696e86 100644
--- a/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java
@@ -36,7 +36,6 @@ import static org.briarproject.briar.sharing.MessageType.DECLINE;
import static org.briarproject.briar.sharing.MessageType.INVITE;
import static org.briarproject.briar.sharing.MessageType.LEAVE;
import static org.briarproject.briar.sharing.SharingConstants.GROUP_KEY_CONTACT_ID;
-import static org.briarproject.briar.sharing.State.ERROR;
import static org.briarproject.briar.sharing.State.LOCAL_INVITED;
import static org.briarproject.briar.sharing.State.LOCAL_LEFT;
import static org.briarproject.briar.sharing.State.REMOTE_HANGING;
@@ -79,7 +78,6 @@ abstract class ProtocolEngineImpl<S extends Shareable>
case SHARING:
case LOCAL_LEFT:
case REMOTE_HANGING:
- case ERROR:
throw new ProtocolStateException(); // Invalid in these states
default:
throw new AssertionError();
@@ -132,7 +130,6 @@ abstract class ProtocolEngineImpl<S extends Shareable>
case SHARING:
case LOCAL_LEFT:
case REMOTE_HANGING:
- case ERROR:
throw new ProtocolStateException(); // Invalid in these states
default:
throw new AssertionError();
@@ -188,7 +185,6 @@ abstract class ProtocolEngineImpl<S extends Shareable>
case SHARING:
case LOCAL_LEFT:
case REMOTE_HANGING:
- case ERROR:
throw new ProtocolStateException(); // Invalid in these states
default:
throw new AssertionError();
@@ -232,7 +228,6 @@ abstract class ProtocolEngineImpl<S extends Shareable>
case LOCAL_INVITED:
case LOCAL_LEFT:
case REMOTE_HANGING:
- case ERROR:
return s; // Ignored in this state
default:
throw new AssertionError();
@@ -277,9 +272,7 @@ abstract class ProtocolEngineImpl<S extends Shareable>
return onRemoteInvite(txn, s, m, false, LOCAL_LEFT);
case LOCAL_INVITED:
case SHARING:
- return abort(txn, s); // Invalid in these states
- case ERROR:
- return s; // Ignored in this state
+ return abortWithMessage(txn, s); // Invalid in these states
default:
throw new AssertionError();
}
@@ -289,10 +282,11 @@ abstract class ProtocolEngineImpl<S extends Shareable>
InviteMessage<S> m, boolean available, State nextState)
throws DbException, FormatException {
// The timestamp must be higher than the last invite message, if any
- if (m.getTimestamp() <= s.getInviteTimestamp()) return abort(txn, s);
+ if (m.getTimestamp() <= s.getInviteTimestamp())
+ return abortWithMessage(txn, s);
// The dependency, if any, must be the last remote message
if (!isValidDependency(s, m.getPreviousMessageId()))
- return abort(txn, s);
+ return abortWithMessage(txn, s);
// Mark the invite message visible in the UI and (un)available to answer
markMessageVisibleInUi(txn, m.getId(), true);
markMessageAvailableToAnswer(txn, m.getId(), available);
@@ -312,10 +306,11 @@ abstract class ProtocolEngineImpl<S extends Shareable>
private Session onRemoteInviteWhenInvited(Transaction txn, Session s,
InviteMessage<S> m) throws DbException, FormatException {
// The timestamp must be higher than the last invite message, if any
- if (m.getTimestamp() <= s.getInviteTimestamp()) return abort(txn, s);
+ if (m.getTimestamp() <= s.getInviteTimestamp())
+ return abortWithMessage(txn, s);
// The dependency, if any, must be the last remote message
if (!isValidDependency(s, m.getPreviousMessageId()))
- return abort(txn, s);
+ return abortWithMessage(txn, s);
// Mark the invite message visible in the UI and unavailable to answer
markMessageVisibleInUi(txn, m.getId(), true);
markMessageAvailableToAnswer(txn, m.getId(), false);
@@ -349,9 +344,7 @@ abstract class ProtocolEngineImpl<S extends Shareable>
case LOCAL_INVITED:
case SHARING:
case LOCAL_LEFT:
- return abort(txn, s); // Invalid in these states
- case ERROR:
- return s; // Ignored in this state
+ return abortWithMessage(txn, s); // Invalid in these states
default:
throw new AssertionError();
}
@@ -360,10 +353,11 @@ abstract class ProtocolEngineImpl<S extends Shareable>
private Session onRemoteAccept(Transaction txn, Session s, AcceptMessage m,
State nextState) throws DbException, FormatException {
// The timestamp must be higher than the last invite message
- if (m.getTimestamp() <= s.getInviteTimestamp()) return abort(txn, s);
+ if (m.getTimestamp() <= s.getInviteTimestamp())
+ return abortWithMessage(txn, s);
// The dependency, if any, must be the last remote message
if (!isValidDependency(s, m.getPreviousMessageId()))
- return abort(txn, s);
+ return abortWithMessage(txn, s);
// Mark the response visible in the UI
markMessageVisibleInUi(txn, m.getId(), true);
// Track the message
@@ -382,8 +376,8 @@ abstract class ProtocolEngineImpl<S extends Shareable>
AcceptMessage m) throws DbException, FormatException {
// Perform normal remote accept validation and operation
Session session = onRemoteAccept(txn, s, m, SHARING);
- // Share the shareable with the contact
- if (session.getState() != ERROR)
+ // Share the shareable with the contact, if session was not reset
+ if (session.getState() != START)
setShareableVisibility(txn, s, SHARED);
return session;
}
@@ -402,9 +396,7 @@ abstract class ProtocolEngineImpl<S extends Shareable>
case LOCAL_INVITED:
case SHARING:
case LOCAL_LEFT:
- return abort(txn, s); // Invalid in these states
- case ERROR:
- return s; // Ignored in this state
+ return abortWithMessage(txn, s); // Invalid in these states
default:
throw new AssertionError();
}
@@ -413,10 +405,11 @@ abstract class ProtocolEngineImpl<S extends Shareable>
private Session onRemoteDecline(Transaction txn, Session s,
DeclineMessage m) throws DbException, FormatException {
// The timestamp must be higher than the last invite message
- if (m.getTimestamp() <= s.getInviteTimestamp()) return abort(txn, s);
+ if (m.getTimestamp() <= s.getInviteTimestamp())
+ return abortWithMessage(txn, s);
// The dependency, if any, must be the last remote message
if (!isValidDependency(s, m.getPreviousMessageId()))
- return abort(txn, s);
+ return abortWithMessage(txn, s);
// Mark the response visible in the UI
markMessageVisibleInUi(txn, m.getId(), true);
// Track the message
@@ -447,15 +440,13 @@ abstract class ProtocolEngineImpl<S extends Shareable>
case LOCAL_INVITED:
return onRemoteLeaveWhenInvited(txn, s, m);
case LOCAL_LEFT:
- return onRemoteLeave(txn, s, m);
+ return onRemoteLeaveWhenLocalLeft(txn, s, m);
case SHARING:
return onRemoteLeaveWhenSharing(txn, s, m);
case START:
case REMOTE_INVITED:
case REMOTE_HANGING:
- return abort(txn, s); // Invalid in these states
- case ERROR:
- return s; // Ignored in this state
+ return abortWithMessage(txn, s); // Invalid in these states
default:
throw new AssertionError();
}
@@ -463,28 +454,22 @@ abstract class ProtocolEngineImpl<S extends Shareable>
private Session onRemoteLeaveWhenInvited(Transaction txn, Session s,
LeaveMessage m) throws DbException, FormatException {
- // Carry out normal leave validation and operation
- Session session = onRemoteLeave(txn, s, m);
+ // The dependency, if any, must be the last remote message
+ if (!isValidDependency(s, m.getPreviousMessageId()))
+ return abortWithMessage(txn, s);
// Mark any invite messages in the session unavailable to answer
- if (session.getState() != ERROR)
- markInvitesUnavailableToAnswer(txn, s);
+ markInvitesUnavailableToAnswer(txn, s);
// Move to the next state
- return session;
+ return new Session(START, s.getContactGroupId(), s.getShareableId(),
+ s.getLastLocalMessageId(), m.getId(), s.getLocalTimestamp(),
+ s.getInviteTimestamp());
}
- private Session onRemoteLeave(Transaction txn, Session s, LeaveMessage m)
- throws DbException, FormatException {
+ private Session onRemoteLeaveWhenLocalLeft(Transaction txn, Session s,
+ LeaveMessage m) throws DbException, FormatException {
// The dependency, if any, must be the last remote message
if (!isValidDependency(s, m.getPreviousMessageId()))
- return abort(txn, s);
- if (s.getState() == SHARING) {
- // Broadcast event informing that contact left
- ContactId contactId = getContactId(txn, s.getContactGroupId());
- ContactLeftShareableEvent e =
- new ContactLeftShareableEvent(s.getShareableId(),
- contactId);
- txn.attach(e);
- }
+ return abortWithMessage(txn, s);
// Move to the next state
return new Session(START, s.getContactGroupId(), s.getShareableId(),
s.getLastLocalMessageId(), m.getId(), s.getLocalTimestamp(),
@@ -493,36 +478,48 @@ abstract class ProtocolEngineImpl<S extends Shareable>
private Session onRemoteLeaveWhenSharing(Transaction txn, Session s,
LeaveMessage m) throws DbException, FormatException {
- // Carry out normal leave validation and operation
- Session session = onRemoteLeave(txn, s, m);
+ // The dependency, if any, must be the last remote message
+ if (!isValidDependency(s, m.getPreviousMessageId()))
+ return abortWithMessage(txn, s);
+ // Broadcast event informing that contact left
+ ContactId contactId = getContactId(txn, s.getContactGroupId());
+ ContactLeftShareableEvent e =
+ new ContactLeftShareableEvent(s.getShareableId(),
+ contactId);
+ txn.attach(e);
// Stop sharing the shareable with the contact
- if (session.getState() != ERROR)
- setShareableVisibility(txn, s, INVISIBLE);
+ setShareableVisibility(txn, s, INVISIBLE);
// Move to the next state
- return session;
+ return new Session(START, s.getContactGroupId(), s.getShareableId(),
+ s.getLastLocalMessageId(), m.getId(), s.getLocalTimestamp(),
+ s.getInviteTimestamp());
}
@Override
public Session onAbortMessage(Transaction txn, Session s, AbortMessage m)
throws DbException, FormatException {
- return abort(txn, s);
+ abort(txn, s);
+ return new Session(START, s.getContactGroupId(), s.getShareableId(),
+ null, m.getId(), 0, 0);
}
- private Session abort(Transaction txn, Session s)
+ private void abort(Transaction txn, Session s)
throws DbException, FormatException {
- // If the session has already been aborted, do nothing
- if (s.getState() == ERROR) return s;
// Mark any invite messages in the session unavailable to answer
markInvitesUnavailableToAnswer(txn, s);
// If we subscribe, make the shareable invisible to the contact
if (isSubscribed(txn, s.getShareableId()))
setShareableVisibility(txn, s, INVISIBLE);
+ }
+
+ private Session abortWithMessage(Transaction txn, Session s)
+ throws DbException, FormatException {
+ abort(txn, s);
// Send an ABORT message
Message sent = sendAbortMessage(txn, s);
- // Move to the ERROR state
- return new Session(ERROR, s.getContactGroupId(), s.getShareableId(),
- sent.getId(), s.getLastRemoteMessageId(), sent.getTimestamp(),
- s.getInviteTimestamp());
+ // Reset the session back to initial state
+ return new Session(START, s.getContactGroupId(), s.getShareableId(),
+ sent.getId(), null, 0, 0);
}
private void markInvitesUnavailableToAnswer(Transaction txn, Session s)
@@ -541,10 +538,10 @@ abstract class ProtocolEngineImpl<S extends Shareable>
throws DbException {
if (!db.containsGroup(txn, g)) return false;
Group group = db.getGroup(txn, g);
- return group.getClientId().equals(getClientId());
+ return group.getClientId().equals(getShareableClientId());
}
- protected abstract ClientId getClientId();
+ protected abstract ClientId getShareableClientId();
private Message sendAbortMessage(Transaction txn, Session session)
throws DbException {
diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/State.java b/briar-core/src/main/java/org/briarproject/briar/sharing/State.java
index 21c26c5297..ade7d7a248 100644
--- a/briar-core/src/main/java/org/briarproject/briar/sharing/State.java
+++ b/briar-core/src/main/java/org/briarproject/briar/sharing/State.java
@@ -10,7 +10,7 @@ import javax.annotation.concurrent.Immutable;
enum State {
START(0), LOCAL_INVITED(1), REMOTE_INVITED(2), SHARING(3), LOCAL_LEFT(4),
- REMOTE_HANGING(5), ERROR(6);
+ REMOTE_HANGING(5);
private final int value;
diff --git a/briar-core/src/test/java/org/briarproject/briar/sharing/ForumSharingIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/sharing/ForumSharingIntegrationTest.java
index 6f351f7087..b33e2b6a7e 100644
--- a/briar-core/src/test/java/org/briarproject/briar/sharing/ForumSharingIntegrationTest.java
+++ b/briar-core/src/test/java/org/briarproject/briar/sharing/ForumSharingIntegrationTest.java
@@ -3,11 +3,14 @@ package org.briarproject.briar.sharing;
import net.jodah.concurrentunit.Waiter;
import org.briarproject.bramble.api.contact.Contact;
+import org.briarproject.bramble.api.data.BdfDictionary;
import org.briarproject.bramble.api.db.DbException;
import org.briarproject.bramble.api.db.Transaction;
import org.briarproject.bramble.api.event.Event;
import org.briarproject.bramble.api.event.EventListener;
import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.bramble.api.sync.Message;
+import org.briarproject.bramble.api.sync.MessageId;
import org.briarproject.bramble.test.TestDatabaseModule;
import org.briarproject.briar.api.forum.Forum;
import org.briarproject.briar.api.forum.ForumInvitationRequest;
@@ -42,6 +45,7 @@ public class ForumSharingIntegrationTest
extends BriarIntegrationTest<BriarIntegrationTestComponent> {
private ForumManager forumManager0, forumManager1;
+ private MessageEncoder messageEncoder;
private SharerListener listener0, listener2;
private InviteeListener listener1;
private Forum forum0;
@@ -67,6 +71,7 @@ public class ForumSharingIntegrationTest
forumSharingManager0 = c0.getForumSharingManager();
forumSharingManager1 = c1.getForumSharingManager();
forumSharingManager2 = c2.getForumSharingManager();
+ messageEncoder = new MessageEncoderImpl(clientHelper, messageFactory);
// initialize waiter fresh for each test
eventWaiter = new Waiter();
@@ -686,6 +691,74 @@ public class ForumSharingIntegrationTest
assertTrue(found);
}
+ @Test
+ public void testSessionResetAfterAbort() throws Exception {
+ // initialize and let invitee accept all requests
+ listenToEvents(true);
+
+ // send invitation
+ forumSharingManager0
+ .sendInvitation(forum0.getId(), contactId1From0, "Hi!",
+ clock.currentTimeMillis());
+
+ // sync first request message
+ sync0To1(1, true);
+ eventWaiter.await(TIMEOUT, 1);
+ assertTrue(listener1.requestReceived);
+
+ // get invitation MessageId for later
+ MessageId invitationId = null;
+ for (InvitationMessage m : forumSharingManager1
+ .getInvitationMessages(contactId0From1)) {
+ if (m instanceof ForumInvitationRequest) {
+ invitationId = m.getId();
+ }
+ }
+ assertNotNull(invitationId);
+
+ // sync response back
+ sync1To0(1, true);
+ eventWaiter.await(TIMEOUT, 1);
+ assertTrue(listener0.responseReceived);
+
+ // forum is shared mutually
+ assertTrue(forumSharingManager0.getSharedWith(forum0.getId())
+ .contains(contact1From0));
+ assertTrue(forumSharingManager1.getSharedWith(forum0.getId())
+ .contains(contact0From1));
+
+ // send an accept message for the same forum
+ Message m = messageEncoder.encodeAcceptMessage(
+ forumSharingManager0.getContactGroup(contact1From0).getId(),
+ forum0.getId(), clock.currentTimeMillis(), invitationId);
+ c0.getClientHelper().addLocalMessage(m, new BdfDictionary(), true);
+
+ // sync unexpected message and the expected abort message back
+ sync0To1(1, true);
+ sync1To0(1, true);
+
+ // forum is no longer shared mutually
+ assertFalse(forumSharingManager0.getSharedWith(forum0.getId())
+ .contains(contact1From0));
+ assertFalse(forumSharingManager1.getSharedWith(forum0.getId())
+ .contains(contact0From1));
+
+ // new invitation is possible now
+ forumSharingManager0
+ .sendInvitation(forum0.getId(), contactId1From0, null,
+ clock.currentTimeMillis());
+ sync0To1(1, true);
+
+ // and can be answered
+ sync1To0(1, true);
+
+ // forum is shared mutually again
+ assertTrue(forumSharingManager0.getSharedWith(forum0.getId())
+ .contains(contact1From0));
+ assertTrue(forumSharingManager1.getSharedWith(forum0.getId())
+ .contains(contact0From1));
+ }
+
@NotNullByDefault
private class SharerListener implements EventListener {
diff --git a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java
index 7d216adca0..1376500ddd 100644
--- a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java
+++ b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java
@@ -18,6 +18,7 @@ import org.briarproject.bramble.api.identity.LocalAuthor;
import org.briarproject.bramble.api.lifecycle.LifecycleManager;
import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
+import org.briarproject.bramble.api.sync.MessageFactory;
import org.briarproject.bramble.api.sync.SyncSession;
import org.briarproject.bramble.api.sync.SyncSessionFactory;
import org.briarproject.bramble.api.sync.event.MessageStateChangedEvent;
@@ -100,6 +101,8 @@ public abstract class BriarIntegrationTest<C extends BriarIntegrationTestCompone
@Inject
protected AuthorFactory authorFactory;
@Inject
+ protected MessageFactory messageFactory;
+ @Inject
protected ContactGroupFactory contactGroupFactory;
@Inject
protected PrivateGroupFactory privateGroupFactory;
diff --git a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java
index 5f036e6ee5..c4219c9e48 100644
--- a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java
+++ b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java
@@ -25,7 +25,6 @@ import org.briarproject.bramble.test.TestSeedProviderModule;
import org.briarproject.bramble.transport.TransportModule;
import org.briarproject.briar.api.blog.BlogManager;
import org.briarproject.briar.api.blog.BlogSharingManager;
-import org.briarproject.briar.api.client.MessageQueueManager;
import org.briarproject.briar.api.client.MessageTracker;
import org.briarproject.briar.api.forum.ForumManager;
import org.briarproject.briar.api.forum.ForumSharingManager;
@@ -133,8 +132,6 @@ public interface BriarIntegrationTestComponent {
MessageTracker getMessageTracker();
- MessageQueueManager getMessageQueueManager();
-
PrivateGroupManager getPrivateGroupManager();
TransportPropertyManager getTransportPropertyManager();
--
GitLab