From 0827b067eca09f0f0f413cfc6efee7119b05a859 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 6 Apr 2017 15:42:12 -0300 Subject: [PATCH 1/2] Harmonize position of boolean message variables --- .../briar/api/client/BaseMessageHeader.java | 14 +++++++------- .../api/introduction/IntroductionMessage.java | 2 +- .../briar/api/messaging/PrivateMessageHeader.java | 2 +- .../briar/api/sharing/InvitationMessage.java | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/briar-api/src/main/java/org/briarproject/briar/api/client/BaseMessageHeader.java b/briar-api/src/main/java/org/briarproject/briar/api/client/BaseMessageHeader.java index 4707ba8ca..8cbbceb32 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/client/BaseMessageHeader.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/client/BaseMessageHeader.java @@ -13,18 +13,18 @@ public abstract class BaseMessageHeader { private final MessageId id; private final GroupId groupId; private final long timestamp; - private final boolean local, read, sent, seen; + private final boolean local, sent, seen, read; public BaseMessageHeader(MessageId id, GroupId groupId, long timestamp, - boolean local, boolean read, boolean sent, boolean seen) { + boolean local, boolean sent, boolean seen, boolean read) { this.id = id; this.groupId = groupId; this.timestamp = timestamp; this.local = local; - this.read = read; this.sent = sent; this.seen = seen; + this.read = read; } public MessageId getId() { @@ -43,10 +43,6 @@ public abstract class BaseMessageHeader { return local; } - public boolean isRead() { - return read; - } - public boolean isSent() { return sent; } @@ -55,4 +51,8 @@ public abstract class BaseMessageHeader { return seen; } + public boolean isRead() { + return read; + } + } diff --git a/briar-api/src/main/java/org/briarproject/briar/api/introduction/IntroductionMessage.java b/briar-api/src/main/java/org/briarproject/briar/api/introduction/IntroductionMessage.java index 680a955ff..861469a3a 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/introduction/IntroductionMessage.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/introduction/IntroductionMessage.java @@ -22,7 +22,7 @@ public class IntroductionMessage extends BaseMessageHeader { GroupId groupId, int role, long time, boolean local, boolean sent, boolean seen, boolean read) { - super(messageId, groupId, time, local, read, sent, seen); + super(messageId, groupId, time, local, sent, seen, read); this.sessionId = sessionId; this.messageId = messageId; this.role = role; diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/PrivateMessageHeader.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/PrivateMessageHeader.java index bc0003177..9caf3bd56 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/messaging/PrivateMessageHeader.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/PrivateMessageHeader.java @@ -14,7 +14,7 @@ public class PrivateMessageHeader extends BaseMessageHeader { public PrivateMessageHeader(MessageId id, GroupId groupId, long timestamp, boolean local, boolean read, boolean sent, boolean seen) { - super(id, groupId, timestamp, local, read, sent, seen); + super(id, groupId, timestamp, local, sent, seen, read); } } diff --git a/briar-api/src/main/java/org/briarproject/briar/api/sharing/InvitationMessage.java b/briar-api/src/main/java/org/briarproject/briar/api/sharing/InvitationMessage.java index 4bdc39455..df93ebdfa 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/sharing/InvitationMessage.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/sharing/InvitationMessage.java @@ -20,7 +20,7 @@ public class InvitationMessage extends BaseMessageHeader { boolean local, boolean sent, boolean seen, boolean read, SessionId sessionId, ContactId contactId) { - super(id, groupId, time, local, read, sent, seen); + super(id, groupId, time, local, sent, seen, read); this.sessionId = sessionId; this.contactId = contactId; } From 85c17b4cb090a58057290c1fb438d3abadd058ae Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 7 Apr 2017 09:45:35 -0300 Subject: [PATCH 2/2] Fix MessageId calculation for deprecated MessageQueue This was preventing introduction messages from getting ACKed. The introduction tests were modified to check for this. --- .../bramble/db/DatabaseComponentImpl.java | 4 +- .../briar/client/QueueMessageFactoryImpl.java | 8 +++- .../IntroductionIntegrationTest.java | 43 +++++++++++++++---- 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java index dd8dea976..9e8d31a61 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java @@ -668,7 +668,9 @@ class DatabaseComponentImpl implements DatabaseComponent { acked.add(m); } } - transaction.attach(new MessagesAckedEvent(c, acked)); + if (acked.size() > 0) { + transaction.attach(new MessagesAckedEvent(c, acked)); + } } @Override diff --git a/briar-core/src/main/java/org/briarproject/briar/client/QueueMessageFactoryImpl.java b/briar-core/src/main/java/org/briarproject/briar/client/QueueMessageFactoryImpl.java index 03722734b..4dcc471ad 100644 --- a/briar-core/src/main/java/org/briarproject/briar/client/QueueMessageFactoryImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/client/QueueMessageFactoryImpl.java @@ -14,6 +14,7 @@ import javax.inject.Inject; import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_LENGTH; import static org.briarproject.bramble.api.sync.SyncConstants.MESSAGE_HEADER_LENGTH; +import static org.briarproject.bramble.util.ByteUtils.INT_64_BYTES; import static org.briarproject.briar.api.client.QueueMessage.MAX_QUEUE_MESSAGE_BODY_LENGTH; import static org.briarproject.briar.api.client.QueueMessage.QUEUE_MESSAGE_HEADER_LENGTH; @@ -39,11 +40,14 @@ class QueueMessageFactoryImpl implements QueueMessageFactory { ByteUtils.writeUint64(queuePosition, raw, MESSAGE_HEADER_LENGTH); System.arraycopy(body, 0, raw, QUEUE_MESSAGE_HEADER_LENGTH, body.length); - byte[] timeBytes = new byte[ByteUtils.INT_64_BYTES]; + byte[] timeBytes = new byte[INT_64_BYTES]; ByteUtils.writeUint64(timestamp, timeBytes, 0); + byte[] bodyBytes = new byte[body.length + INT_64_BYTES]; + System.arraycopy(raw, MESSAGE_HEADER_LENGTH, bodyBytes, 0, + body.length + INT_64_BYTES); MessageId id = new MessageId( crypto.hash(MessageId.LABEL, groupId.getBytes(), timeBytes, - body)); + bodyBytes)); return new QueueMessage(id, groupId, timestamp, queuePosition, raw); } diff --git a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java index 06ff73e68..7a4f263f8 100644 --- a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java @@ -19,6 +19,7 @@ import org.briarproject.bramble.api.event.EventListener; 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.plugin.TransportId; import org.briarproject.bramble.api.properties.TransportProperties; import org.briarproject.bramble.api.properties.TransportPropertyManager; import org.briarproject.bramble.api.sync.Group; @@ -360,6 +361,14 @@ public class IntroductionIntegrationTest 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(new TransportId("fake"), tp); + sync0To1(1, true); + // sync second response sync2To0(1, true); eventWaiter.await(TIMEOUT, 1); @@ -836,14 +845,32 @@ public class IntroductionIntegrationTest } private void assertDefaultUiMessages() throws DbException { - assertEquals(2, introductionManager0.getIntroductionMessages( - contactId1From0).size()); - assertEquals(2, introductionManager0.getIntroductionMessages( - contactId2From0).size()); - assertEquals(2, introductionManager1.getIntroductionMessages( - contactId0From1).size()); - assertEquals(2, introductionManager2.getIntroductionMessages( - contactId0From2).size()); + Collection messages = + introductionManager0.getIntroductionMessages(contactId1From0); + assertEquals(2, messages.size()); + assertMessagesAreAcked(messages); + + messages = introductionManager0.getIntroductionMessages( + contactId2From0); + assertEquals(2, messages.size()); + assertMessagesAreAcked(messages); + + messages = introductionManager1.getIntroductionMessages( + contactId0From1); + assertEquals(2, messages.size()); + assertMessagesAreAcked(messages); + + messages = introductionManager2.getIntroductionMessages( + contactId0From2); + assertEquals(2, messages.size()); + assertMessagesAreAcked(messages); + } + + private void assertMessagesAreAcked( + Collection messages) { + for (IntroductionMessage msg : messages) { + if (msg.isLocal()) assertTrue(msg.isSeen()); + } } private void addListeners(boolean accept1, boolean accept2) {