From 8a3e5bfb501fc79b3b9d3e65fa6be8887f8d3fa8 Mon Sep 17 00:00:00 2001 From: akwizgran <akwizgran@users.sourceforge.net> Date: Thu, 8 Sep 2016 14:57:41 +0100 Subject: [PATCH] Refactor ValidationManager and fix some bugs. #619 --- .../org/briarproject/BlogManagerTest.java | 5 +- .../BlogSharingIntegrationTest.java | 29 +- .../org/briarproject/ForumManagerTest.java | 7 +- .../ForumSharingIntegrationTest.java | 27 +- .../IntroductionIntegrationTest.java | 16 +- .../api/clients/BdfMessageContext.java | 10 +- .../api/clients/ClientHelper.java | 10 +- .../api/db/DatabaseComponent.java | 43 +- .../api/event/MessageStateChangedEvent.java | 23 +- .../api/sync/BaseMessageContext.java | 4 +- .../briarproject/api/sync/MessageContext.java | 10 +- .../api/sync/ValidationManager.java | 2 +- .../briarproject/blogs/BlogManagerImpl.java | 9 +- .../briarproject/blogs/BlogPostValidator.java | 6 +- .../clients/ClientHelperImpl.java | 9 +- .../clients/MessageQueueManagerImpl.java | 4 +- .../src/org/briarproject/db/Database.java | 47 +- .../db/DatabaseComponentImpl.java | 40 +- .../src/org/briarproject/db/JdbcDatabase.java | 92 +-- .../briarproject/forum/ForumManagerImpl.java | 6 +- .../forum/ForumPostValidator.java | 2 +- .../introduction/IntroduceeManager.java | 3 +- .../introduction/IntroducerManager.java | 4 +- .../messaging/MessagingManagerImpl.java | 2 +- .../TransportPropertyManagerImpl.java | 2 +- .../sharing/SharingManagerImpl.java | 4 +- .../sync/ValidationManagerImpl.java | 580 ++++++++--------- .../blogs/BlogManagerImplTest.java | 2 +- .../clients/MessageQueueManagerImplTest.java | 6 +- .../db/DatabaseComponentImplTest.java | 36 +- .../org/briarproject/db/H2DatabaseTest.java | 222 ++++--- .../introduction/IntroduceeManagerTest.java | 3 +- .../introduction/IntroducerManagerTest.java | 3 +- .../sync/ValidationManagerImplTest.java | 594 +++++++++++------- 34 files changed, 959 insertions(+), 903 deletions(-) diff --git a/briar-android-tests/src/test/java/org/briarproject/BlogManagerTest.java b/briar-android-tests/src/test/java/org/briarproject/BlogManagerTest.java index 3bfaae9a2a..d0f1d6c126 100644 --- a/briar-android-tests/src/test/java/org/briarproject/BlogManagerTest.java +++ b/briar-android-tests/src/test/java/org/briarproject/BlogManagerTest.java @@ -58,7 +58,6 @@ import static org.briarproject.api.blogs.MessageType.WRAPPED_POST; import static org.briarproject.api.sync.ValidationManager.State.DELIVERED; import static org.briarproject.api.sync.ValidationManager.State.INVALID; import static org.briarproject.api.sync.ValidationManager.State.PENDING; -import static org.briarproject.api.sync.ValidationManager.State.VALID; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -536,14 +535,14 @@ public class BlogManagerTest { } private class Listener implements EventListener { + @Override public void eventOccurred(Event e) { if (e instanceof MessageStateChangedEvent) { MessageStateChangedEvent event = (MessageStateChangedEvent) e; if (!event.isLocal()) { if (event.getState() == DELIVERED) { deliveryWaiter.resume(); - } else if (event.getState() == VALID || - event.getState() == INVALID || + } else if (event.getState() == INVALID || event.getState() == PENDING) { validationWaiter.resume(); } diff --git a/briar-android-tests/src/test/java/org/briarproject/BlogSharingIntegrationTest.java b/briar-android-tests/src/test/java/org/briarproject/BlogSharingIntegrationTest.java index 0c62fd4e45..70afe99efd 100644 --- a/briar-android-tests/src/test/java/org/briarproject/BlogSharingIntegrationTest.java +++ b/briar-android-tests/src/test/java/org/briarproject/BlogSharingIntegrationTest.java @@ -25,7 +25,6 @@ import org.briarproject.api.identity.IdentityManager; import org.briarproject.api.identity.LocalAuthor; import org.briarproject.api.lifecycle.LifecycleManager; import org.briarproject.api.sharing.InvitationMessage; -import org.briarproject.api.sync.ClientId; import org.briarproject.api.sync.SyncSession; import org.briarproject.api.sync.SyncSessionFactory; import org.briarproject.api.sync.ValidationManager.State; @@ -424,7 +423,7 @@ public class BlogSharingIntegrationTest extends BriarTestCase { eventWaiter.await(TIMEOUT, 1); assertTrue(listener0.responseReceived); - // blog was added successfully and is shard both ways + // blog was added successfully and is shared both ways assertEquals(3, blogManager1.getBlogs().size()); Collection<Contact> sharedWith = blogSharingManager0.getSharedWith(blog2.getId()); @@ -514,20 +513,13 @@ public class BlogSharingIntegrationTest extends BriarTestCase { volatile boolean requestReceived = false; volatile boolean responseReceived = false; + @Override public void eventOccurred(Event e) { if (e instanceof MessageStateChangedEvent) { MessageStateChangedEvent event = (MessageStateChangedEvent) e; State s = event.getState(); - ClientId c = event.getClientId(); - if ((s == DELIVERED || s == INVALID) && - c.equals(blogSharingManager0.getClientId()) && - !event.isLocal()) { - LOG.info("TEST: Sharer received message in group " + - event.getMessage().getGroupId().hashCode()); - msgWaiter.resume(); - } else if (s == DELIVERED && !event.isLocal() && - c.equals(blogManager0.getClientId())) { - LOG.info("TEST: Sharer received blog post"); + if ((s == DELIVERED || s == INVALID) && !event.isLocal()) { + LOG.info("TEST: Sharer received message"); msgWaiter.resume(); } } else if (e instanceof BlogInvitationResponseReceivedEvent) { @@ -572,20 +564,13 @@ public class BlogSharingIntegrationTest extends BriarTestCase { this(accept, true); } + @Override public void eventOccurred(Event e) { if (e instanceof MessageStateChangedEvent) { MessageStateChangedEvent event = (MessageStateChangedEvent) e; State s = event.getState(); - ClientId c = event.getClientId(); - if ((s == DELIVERED || s == INVALID) && - c.equals(blogSharingManager0.getClientId()) && - !event.isLocal()) { - LOG.info("TEST: Invitee received message in group " + - event.getMessage().getGroupId().hashCode()); - msgWaiter.resume(); - } else if (s == DELIVERED && !event.isLocal() && - c.equals(blogManager0.getClientId())) { - LOG.info("TEST: Invitee received blog post"); + if ((s == DELIVERED || s == INVALID) && !event.isLocal()) { + LOG.info("TEST: Invitee received message"); msgWaiter.resume(); } } else if (e instanceof BlogInvitationReceivedEvent) { diff --git a/briar-android-tests/src/test/java/org/briarproject/ForumManagerTest.java b/briar-android-tests/src/test/java/org/briarproject/ForumManagerTest.java index c9e6c9aea9..0fa8e80c1e 100644 --- a/briar-android-tests/src/test/java/org/briarproject/ForumManagerTest.java +++ b/briar-android-tests/src/test/java/org/briarproject/ForumManagerTest.java @@ -57,7 +57,6 @@ import static org.briarproject.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGT import static org.briarproject.api.sync.ValidationManager.State.DELIVERED; import static org.briarproject.api.sync.ValidationManager.State.INVALID; import static org.briarproject.api.sync.ValidationManager.State.PENDING; -import static org.briarproject.api.sync.ValidationManager.State.VALID; import static org.junit.Assert.assertTrue; public class ForumManagerTest { @@ -298,7 +297,7 @@ public class ForumManagerTest { assertEquals(1, forumManager0.getPostHeaders(g).size()); assertEquals(0, forumManager1.getPostHeaders(g).size()); - // send posts to 1 + // send the child post to 1 sync0To1(); validationWaiter.await(TIMEOUT, 1); assertEquals(1, forumManager0.getPostHeaders(g).size()); @@ -327,14 +326,14 @@ public class ForumManagerTest { } private class Listener implements EventListener { + @Override public void eventOccurred(Event e) { if (e instanceof MessageStateChangedEvent) { MessageStateChangedEvent event = (MessageStateChangedEvent) e; if (!event.isLocal()) { if (event.getState() == DELIVERED) { deliveryWaiter.resume(); - } else if (event.getState() == VALID || - event.getState() == INVALID || + } else if (event.getState() == INVALID || event.getState() == PENDING) { validationWaiter.resume(); } diff --git a/briar-android-tests/src/test/java/org/briarproject/ForumSharingIntegrationTest.java b/briar-android-tests/src/test/java/org/briarproject/ForumSharingIntegrationTest.java index 3f45a5536a..8206489592 100644 --- a/briar-android-tests/src/test/java/org/briarproject/ForumSharingIntegrationTest.java +++ b/briar-android-tests/src/test/java/org/briarproject/ForumSharingIntegrationTest.java @@ -38,7 +38,6 @@ import org.briarproject.api.identity.LocalAuthor; import org.briarproject.api.lifecycle.LifecycleManager; import org.briarproject.api.sharing.InvitationItem; import org.briarproject.api.sharing.InvitationMessage; -import org.briarproject.api.sync.ClientId; import org.briarproject.api.sync.Group; import org.briarproject.api.sync.SyncSession; import org.briarproject.api.sync.SyncSessionFactory; @@ -928,20 +927,13 @@ public class ForumSharingIntegrationTest extends BriarTestCase { volatile boolean requestReceived = false; volatile boolean responseReceived = false; + @Override public void eventOccurred(Event e) { if (e instanceof MessageStateChangedEvent) { MessageStateChangedEvent event = (MessageStateChangedEvent) e; State s = event.getState(); - ClientId c = event.getClientId(); - if ((s == DELIVERED || s == INVALID) && - c.equals(forumSharingManager0.getClientId()) && - !event.isLocal()) { - LOG.info("TEST: Sharer received message in group " + - event.getMessage().getGroupId().hashCode()); - msgWaiter.resume(); - } else if (s == DELIVERED && !event.isLocal() && - c.equals(forumManager0.getClientId())) { - LOG.info("TEST: Sharer received forum post"); + if ((s == DELIVERED || s == INVALID) && !event.isLocal()) { + LOG.info("TEST: Sharer received message"); msgWaiter.resume(); } } else if (e instanceof ForumInvitationResponseReceivedEvent) { @@ -986,20 +978,13 @@ public class ForumSharingIntegrationTest extends BriarTestCase { this(accept, true); } + @Override public void eventOccurred(Event e) { if (e instanceof MessageStateChangedEvent) { MessageStateChangedEvent event = (MessageStateChangedEvent) e; State s = event.getState(); - ClientId c = event.getClientId(); - if ((s == DELIVERED || s == INVALID) && - c.equals(forumSharingManager0.getClientId()) && - !event.isLocal()) { - LOG.info("TEST: Invitee received message in group " + - event.getMessage().getGroupId().hashCode()); - msgWaiter.resume(); - } else if (s == DELIVERED && !event.isLocal() && - c.equals(forumManager0.getClientId())) { - LOG.info("TEST: Invitee received forum post"); + if ((s == DELIVERED || s == INVALID) && !event.isLocal()) { + LOG.info("TEST: Invitee received message"); msgWaiter.resume(); } } else if (e instanceof ForumInvitationReceivedEvent) { 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 564db75c6f..c92d588f0e 100644 --- a/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTest.java +++ b/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTest.java @@ -36,7 +36,6 @@ import org.briarproject.api.introduction.IntroductionRequest; import org.briarproject.api.lifecycle.LifecycleManager; import org.briarproject.api.properties.TransportProperties; import org.briarproject.api.properties.TransportPropertyManager; -import org.briarproject.api.sync.ClientId; import org.briarproject.api.sync.Group; import org.briarproject.api.sync.GroupId; import org.briarproject.api.sync.MessageId; @@ -1107,13 +1106,9 @@ public class IntroductionIntegrationTest extends BriarTestCase { if (e instanceof MessageStateChangedEvent) { MessageStateChangedEvent event = (MessageStateChangedEvent) e; State s = event.getState(); - ClientId c = event.getClientId(); - if ((s == DELIVERED || s == INVALID) && - c.equals(introductionManager0.getClientId()) && - !event.isLocal()) { + if ((s == DELIVERED || s == INVALID) && !event.isLocal()) { LOG.info("TEST: Introducee" + introducee + - " received message in group " + - event.getMessage().getGroupId().hashCode()); + " received message"); msgWaiter.resume(); } } else if (e instanceof IntroductionRequestReceivedEvent) { @@ -1174,11 +1169,8 @@ public class IntroductionIntegrationTest extends BriarTestCase { public void eventOccurred(Event e) { if (e instanceof MessageStateChangedEvent) { MessageStateChangedEvent event = (MessageStateChangedEvent) e; - if (event.getState() == DELIVERED && event.getClientId() - .equals(introductionManager0.getClientId()) && - !event.isLocal()) { - LOG.info("TEST: Introducer received message in group " + - event.getMessage().getGroupId().hashCode()); + if (event.getState() == DELIVERED && !event.isLocal()) { + LOG.info("TEST: Introducer received message"); msgWaiter.resume(); } } else if (e instanceof IntroductionResponseReceivedEvent) { diff --git a/briar-api/src/org/briarproject/api/clients/BdfMessageContext.java b/briar-api/src/org/briarproject/api/clients/BdfMessageContext.java index d856c973c8..70f410c039 100644 --- a/briar-api/src/org/briarproject/api/clients/BdfMessageContext.java +++ b/briar-api/src/org/briarproject/api/clients/BdfMessageContext.java @@ -3,22 +3,24 @@ package org.briarproject.api.clients; import org.briarproject.api.data.BdfDictionary; import org.briarproject.api.sync.BaseMessageContext; import org.briarproject.api.sync.MessageId; +import org.jetbrains.annotations.NotNull; import java.util.Collection; +import java.util.Collections; public class BdfMessageContext extends BaseMessageContext { private final BdfDictionary dictionary; - public BdfMessageContext(BdfDictionary dictionary, - Collection<MessageId> dependencies) { + public BdfMessageContext(@NotNull BdfDictionary dictionary, + @NotNull Collection<MessageId> dependencies) { super(dependencies); this.dictionary = dictionary; } - public BdfMessageContext(BdfDictionary dictionary) { - this(dictionary, null); + public BdfMessageContext(@NotNull BdfDictionary dictionary) { + this(dictionary, Collections.<MessageId>emptyList()); } public BdfDictionary getDictionary() { diff --git a/briar-api/src/org/briarproject/api/clients/ClientHelper.java b/briar-api/src/org/briarproject/api/clients/ClientHelper.java index c6553c91bb..c2d3877597 100644 --- a/briar-api/src/org/briarproject/api/clients/ClientHelper.java +++ b/briar-api/src/org/briarproject/api/clients/ClientHelper.java @@ -5,7 +5,6 @@ import org.briarproject.api.data.BdfDictionary; import org.briarproject.api.data.BdfList; import org.briarproject.api.db.DbException; import org.briarproject.api.db.Transaction; -import org.briarproject.api.sync.ClientId; import org.briarproject.api.sync.GroupId; import org.briarproject.api.sync.Message; import org.briarproject.api.sync.MessageId; @@ -15,12 +14,11 @@ import java.util.Map; public interface ClientHelper { - void addLocalMessage(Message m, ClientId c, BdfDictionary metadata, - boolean shared) throws DbException, FormatException; + void addLocalMessage(Message m, BdfDictionary metadata, boolean shared) + throws DbException, FormatException; - void addLocalMessage(Transaction txn, Message m, ClientId c, - BdfDictionary metadata, boolean shared) throws DbException, - FormatException; + void addLocalMessage(Transaction txn, Message m, BdfDictionary metadata, + boolean shared) throws DbException, FormatException; Message createMessage(GroupId g, long timestamp, BdfDictionary body) throws FormatException; diff --git a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java index 328b978697..a30ffabaa8 100644 --- a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java +++ b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java @@ -18,6 +18,7 @@ import org.briarproject.api.sync.MessageStatus; import org.briarproject.api.sync.Offer; import org.briarproject.api.sync.Request; import org.briarproject.api.transport.TransportKeys; +import org.jetbrains.annotations.Nullable; import java.util.Collection; import java.util.Map; @@ -77,7 +78,7 @@ public interface DatabaseComponent { /** * Stores a local message. */ - void addLocalMessage(Transaction txn, Message m, ClientId c, Metadata meta, + void addLocalMessage(Transaction txn, Message m, Metadata meta, boolean shared) throws DbException; /** @@ -125,6 +126,7 @@ public interface DatabaseComponent { * Returns an acknowledgement for the given contact, or null if there are * no messages to acknowledge. */ + @Nullable Ack generateAck(Transaction txn, ContactId c, int maxMessages) throws DbException; @@ -134,6 +136,7 @@ public interface DatabaseComponent { * transport with the given maximum latency. Returns null if there are no * sendable messages that fit in the given length. */ + @Nullable Collection<byte[]> generateBatch(Transaction txn, ContactId c, int maxLength, int maxLatency) throws DbException; @@ -142,6 +145,7 @@ public interface DatabaseComponent { * transport with the given maximum latency, or null if there are no * messages to offer. */ + @Nullable Offer generateOffer(Transaction txn, ContactId c, int maxMessages, int maxLatency) throws DbException; @@ -149,6 +153,7 @@ public interface DatabaseComponent { * Returns a request for the given contact, or null if there are no * messages to request. */ + @Nullable Request generateRequest(Transaction txn, ContactId c, int maxMessages) throws DbException; @@ -159,6 +164,7 @@ public interface DatabaseComponent { * requested by the contact are returned. Returns null if there are no * sendable messages that fit in the given length. */ + @Nullable Collection<byte[]> generateRequestedBatch(Transaction txn, ContactId c, int maxLength, int maxLatency) throws DbException; @@ -244,17 +250,8 @@ public interface DatabaseComponent { throws DbException; /** - * Returns the IDs of any messages that need to be delivered to the given - * client. - * <p/> - * Read-only. - */ - Collection<MessageId> getMessagesToDeliver(Transaction txn, ClientId c) - throws DbException; - - /** - * Returns the IDs of any messages that are still pending due to - * dependencies to other messages for the given client. + * Returns the IDs of any messages that are valid but pending delivery due + * to dependencies on other messages for the given client. * <p/> * Read-only. */ @@ -267,6 +264,7 @@ public interface DatabaseComponent { * <p/> * Read-only. */ + @Nullable byte[] getRawMessage(Transaction txn, MessageId m) throws DbException; /** @@ -314,7 +312,13 @@ public interface DatabaseComponent { GroupId g) throws DbException; /** - * Returns the dependencies of the given message. + * Returns the IDs and states of all dependencies of the given message. + * Missing dependencies have the state {@link + * org.briarproject.api.sync.ValidationManager.State UNKNOWN}. + * Dependencies in other groups have the state {@link + * org.briarproject.api.sync.ValidationManager.State INVALID}. + * Note that these states are not set on the dependencies themselves; the + * returned states should only be taken in the context of the given message. * <p/> * Read-only. */ @@ -323,12 +327,21 @@ public interface DatabaseComponent { /** * Returns all IDs of messages that depend on the given message. + * Messages in other groups that declare a dependency on the given message + * will be returned even though such dependencies are invalid. * <p/> * Read-only. */ Map<MessageId, State> getMessageDependents(Transaction txn, MessageId m) throws DbException; + /** + * Gets the validation and delivery state of the given message. + * <p/> + * Read-only. + */ + State getMessageState(Transaction txn, MessageId m) throws DbException; + /** * Returns the status of the given message with respect to the given * contact. @@ -449,9 +462,9 @@ public interface DatabaseComponent { throws DbException; /** - * Sets the state of the message with respect to validation and delivery. + * Sets the validation and delivery state of the given message. */ - void setMessageState(Transaction txn, Message m, ClientId c, State valid) + void setMessageState(Transaction txn, MessageId m, State state) throws DbException; /** diff --git a/briar-api/src/org/briarproject/api/event/MessageStateChangedEvent.java b/briar-api/src/org/briarproject/api/event/MessageStateChangedEvent.java index c2aafa0347..4ad29e0102 100644 --- a/briar-api/src/org/briarproject/api/event/MessageStateChangedEvent.java +++ b/briar-api/src/org/briarproject/api/event/MessageStateChangedEvent.java @@ -1,8 +1,7 @@ package org.briarproject.api.event; -import org.briarproject.api.sync.ClientId; -import org.briarproject.api.sync.Message; -import org.briarproject.api.sync.ValidationManager; +import org.briarproject.api.sync.MessageId; + import static org.briarproject.api.sync.ValidationManager.State; /** @@ -10,25 +9,19 @@ import static org.briarproject.api.sync.ValidationManager.State; */ public class MessageStateChangedEvent extends Event { - private final Message message; - private final ClientId clientId; + private final MessageId messageId; private final boolean local; private final State state; - public MessageStateChangedEvent(Message message, ClientId clientId, - boolean local, State state) { - this.message = message; - this.clientId = clientId; + public MessageStateChangedEvent(MessageId messageId, boolean local, + State state) { + this.messageId = messageId; this.local = local; this.state = state; } - public Message getMessage() { - return message; - } - - public ClientId getClientId() { - return clientId; + public MessageId getMessageId() { + return messageId; } public boolean isLocal() { diff --git a/briar-api/src/org/briarproject/api/sync/BaseMessageContext.java b/briar-api/src/org/briarproject/api/sync/BaseMessageContext.java index de574b7c16..7eb411a6a0 100644 --- a/briar-api/src/org/briarproject/api/sync/BaseMessageContext.java +++ b/briar-api/src/org/briarproject/api/sync/BaseMessageContext.java @@ -1,12 +1,14 @@ package org.briarproject.api.sync; +import org.jetbrains.annotations.NotNull; + import java.util.Collection; public abstract class BaseMessageContext { private final Collection<MessageId> dependencies; - public BaseMessageContext(Collection<MessageId> dependencies) { + public BaseMessageContext(@NotNull Collection<MessageId> dependencies) { this.dependencies = dependencies; } diff --git a/briar-api/src/org/briarproject/api/sync/MessageContext.java b/briar-api/src/org/briarproject/api/sync/MessageContext.java index 35f45e76ca..d0bef01179 100644 --- a/briar-api/src/org/briarproject/api/sync/MessageContext.java +++ b/briar-api/src/org/briarproject/api/sync/MessageContext.java @@ -1,22 +1,24 @@ package org.briarproject.api.sync; import org.briarproject.api.db.Metadata; +import org.jetbrains.annotations.NotNull; import java.util.Collection; +import java.util.Collections; public class MessageContext extends BaseMessageContext { private final Metadata metadata; - public MessageContext(Metadata metadata, - Collection<MessageId> dependencies) { + public MessageContext(@NotNull Metadata metadata, + @NotNull Collection<MessageId> dependencies) { super(dependencies); this.metadata = metadata; } - public MessageContext(Metadata metadata) { - this(metadata, null); + public MessageContext(@NotNull Metadata metadata) { + this(metadata, Collections.<MessageId>emptyList()); } public Metadata getMetadata() { diff --git a/briar-api/src/org/briarproject/api/sync/ValidationManager.java b/briar-api/src/org/briarproject/api/sync/ValidationManager.java index e92a6a6827..ce6a11b234 100644 --- a/briar-api/src/org/briarproject/api/sync/ValidationManager.java +++ b/briar-api/src/org/briarproject/api/sync/ValidationManager.java @@ -12,7 +12,7 @@ public interface ValidationManager { enum State { - UNKNOWN(0), INVALID(1), PENDING(2), VALID(3), DELIVERED(4); + UNKNOWN(0), INVALID(1), PENDING(2), DELIVERED(3); private final int value; diff --git a/briar-core/src/org/briarproject/blogs/BlogManagerImpl.java b/briar-core/src/org/briarproject/blogs/BlogManagerImpl.java index 92f7e9c8a0..072268f34e 100644 --- a/briar-core/src/org/briarproject/blogs/BlogManagerImpl.java +++ b/briar-core/src/org/briarproject/blogs/BlogManagerImpl.java @@ -59,12 +59,12 @@ import static org.briarproject.api.blogs.BlogConstants.KEY_COMMENT; import static org.briarproject.api.blogs.BlogConstants.KEY_DESCRIPTION; import static org.briarproject.api.blogs.BlogConstants.KEY_ORIGINAL_MSG_ID; import static org.briarproject.api.blogs.BlogConstants.KEY_ORIGINAL_PARENT_MSG_ID; +import static org.briarproject.api.blogs.BlogConstants.KEY_PARENT_MSG_ID; import static org.briarproject.api.blogs.BlogConstants.KEY_PUBLIC_KEY; import static org.briarproject.api.blogs.BlogConstants.KEY_READ; import static org.briarproject.api.blogs.BlogConstants.KEY_TIMESTAMP; import static org.briarproject.api.blogs.BlogConstants.KEY_TIME_RECEIVED; import static org.briarproject.api.blogs.BlogConstants.KEY_TYPE; -import static org.briarproject.api.blogs.BlogConstants.KEY_PARENT_MSG_ID; import static org.briarproject.api.blogs.MessageType.COMMENT; import static org.briarproject.api.blogs.MessageType.POST; import static org.briarproject.api.blogs.MessageType.WRAPPED_COMMENT; @@ -298,8 +298,7 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager, meta.put(KEY_TIMESTAMP, p.getMessage().getTimestamp()); meta.put(KEY_AUTHOR, authorToBdfDictionary(p.getAuthor())); meta.put(KEY_READ, true); - clientHelper.addLocalMessage(txn, p.getMessage(), CLIENT_ID, meta, - true); + clientHelper.addLocalMessage(txn, p.getMessage(), meta, true); // broadcast event about new post GroupId groupId = p.getMessage().getGroupId(); @@ -346,7 +345,7 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager, meta.put(KEY_AUTHOR, authorToBdfDictionary(author)); // Send comment - clientHelper.addLocalMessage(txn, message, CLIENT_ID, meta, true); + clientHelper.addLocalMessage(txn, message, meta, true); // broadcast event BlogPostHeader h = @@ -429,7 +428,7 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager, meta.put(KEY_TIME_RECEIVED, pOriginalHeader.getTimeReceived()); // Send wrapped message and store metadata - clientHelper.addLocalMessage(txn, wMessage, CLIENT_ID, meta, true); + clientHelper.addLocalMessage(txn, wMessage, meta, true); return wMessage.getId(); } diff --git a/briar-core/src/org/briarproject/blogs/BlogPostValidator.java b/briar-core/src/org/briarproject/blogs/BlogPostValidator.java index eaab2313fe..328153ad7f 100644 --- a/briar-core/src/org/briarproject/blogs/BlogPostValidator.java +++ b/briar-core/src/org/briarproject/blogs/BlogPostValidator.java @@ -32,9 +32,9 @@ import static org.briarproject.api.blogs.BlogConstants.KEY_AUTHOR; import static org.briarproject.api.blogs.BlogConstants.KEY_AUTHOR_ID; import static org.briarproject.api.blogs.BlogConstants.KEY_AUTHOR_NAME; import static org.briarproject.api.blogs.BlogConstants.KEY_COMMENT; +import static org.briarproject.api.blogs.BlogConstants.KEY_ORIGINAL_MSG_ID; import static org.briarproject.api.blogs.BlogConstants.KEY_ORIGINAL_PARENT_MSG_ID; import static org.briarproject.api.blogs.BlogConstants.KEY_PARENT_MSG_ID; -import static org.briarproject.api.blogs.BlogConstants.KEY_ORIGINAL_MSG_ID; import static org.briarproject.api.blogs.BlogConstants.KEY_PUBLIC_KEY; import static org.briarproject.api.blogs.BlogConstants.KEY_READ; import static org.briarproject.api.blogs.BlogConstants.KEY_TIMESTAMP; @@ -114,7 +114,7 @@ class BlogPostValidator extends BdfMessageValidator { BdfDictionary meta = new BdfDictionary(); meta.put(KEY_ORIGINAL_MSG_ID, m.getId()); meta.put(KEY_AUTHOR, authorToBdfDictionary(a)); - return new BdfMessageContext(meta, null); + return new BdfMessageContext(meta); } private BdfMessageContext validateComment(Message m, Group g, BdfList body) @@ -197,7 +197,7 @@ class BlogPostValidator extends BdfMessageValidator { meta.put(KEY_ORIGINAL_MSG_ID, wMessage.getId()); meta.put(KEY_TIMESTAMP, wTimestamp); meta.put(KEY_AUTHOR, c.getDictionary().getDictionary(KEY_AUTHOR)); - return new BdfMessageContext(meta, null); + return new BdfMessageContext(meta); } private BdfMessageContext validateWrappedComment(Message m, Group g, diff --git a/briar-core/src/org/briarproject/clients/ClientHelperImpl.java b/briar-core/src/org/briarproject/clients/ClientHelperImpl.java index a8af6137ab..890dc5146d 100644 --- a/briar-core/src/org/briarproject/clients/ClientHelperImpl.java +++ b/briar-core/src/org/briarproject/clients/ClientHelperImpl.java @@ -18,7 +18,6 @@ import org.briarproject.api.db.DatabaseComponent; import org.briarproject.api.db.DbException; import org.briarproject.api.db.Metadata; import org.briarproject.api.db.Transaction; -import org.briarproject.api.sync.ClientId; import org.briarproject.api.sync.GroupId; import org.briarproject.api.sync.Message; import org.briarproject.api.sync.MessageFactory; @@ -62,11 +61,11 @@ class ClientHelperImpl implements ClientHelper { } @Override - public void addLocalMessage(Message m, ClientId c, BdfDictionary metadata, + public void addLocalMessage(Message m, BdfDictionary metadata, boolean shared) throws DbException, FormatException { Transaction txn = db.startTransaction(false); try { - addLocalMessage(txn, m, c, metadata, shared); + addLocalMessage(txn, m, metadata, shared); txn.setComplete(); } finally { db.endTransaction(txn); @@ -74,10 +73,10 @@ class ClientHelperImpl implements ClientHelper { } @Override - public void addLocalMessage(Transaction txn, Message m, ClientId c, + public void addLocalMessage(Transaction txn, Message m, BdfDictionary metadata, boolean shared) throws DbException, FormatException { - db.addLocalMessage(txn, m, c, metadataEncoder.encode(metadata), shared); + db.addLocalMessage(txn, m, metadataEncoder.encode(metadata), shared); } @Override diff --git a/briar-core/src/org/briarproject/clients/MessageQueueManagerImpl.java b/briar-core/src/org/briarproject/clients/MessageQueueManagerImpl.java index 83ede64e67..9869707d58 100644 --- a/briar-core/src/org/briarproject/clients/MessageQueueManagerImpl.java +++ b/briar-core/src/org/briarproject/clients/MessageQueueManagerImpl.java @@ -16,10 +16,10 @@ import org.briarproject.api.sync.Group; import org.briarproject.api.sync.GroupId; import org.briarproject.api.sync.InvalidMessageException; import org.briarproject.api.sync.Message; +import org.briarproject.api.sync.MessageContext; import org.briarproject.api.sync.MessageId; import org.briarproject.api.sync.ValidationManager; import org.briarproject.api.sync.ValidationManager.IncomingMessageHook; -import org.briarproject.api.sync.MessageContext; import org.briarproject.util.ByteUtils; import java.util.ArrayList; @@ -70,7 +70,7 @@ class MessageQueueManagerImpl implements MessageQueueManager { saveQueueState(txn, queue.getId(), queueState); QueueMessage q = queueMessageFactory.createMessage(queue.getId(), timestamp, queuePosition, body); - db.addLocalMessage(txn, q, queue.getClientId(), meta, true); + db.addLocalMessage(txn, q, meta, true); return q; } diff --git a/briar-core/src/org/briarproject/db/Database.java b/briar-core/src/org/briarproject/db/Database.java index 0eb51b3173..5a07c0f06d 100644 --- a/briar-core/src/org/briarproject/db/Database.java +++ b/briar-core/src/org/briarproject/db/Database.java @@ -18,6 +18,7 @@ import org.briarproject.api.sync.MessageId; import org.briarproject.api.sync.MessageStatus; import org.briarproject.api.sync.ValidationManager.State; import org.briarproject.api.transport.TransportKeys; +import org.jetbrains.annotations.Nullable; import java.util.Collection; import java.util.Map; @@ -83,10 +84,10 @@ interface Database<T> { throws DbException; /** - * Adds a dependency between two MessageIds + * Adds a dependency between two messages in the given group. */ - void addMessageDependency(T txn, MessageId dependentId, - MessageId dependencyId) throws DbException; + void addMessageDependency(T txn, GroupId g, MessageId dependent, + MessageId dependency) throws DbException; /** * Records that a message has been offered by the given contact. @@ -281,11 +282,13 @@ interface Database<T> { Collection<LocalAuthor> getLocalAuthors(T txn) throws DbException; /** - * Returns the dependencies of the given message. - * This method makes sure that dependencies in different groups - * are returned as {@link ValidationManager.State.INVALID}. Note that this - * is not set on the dependencies themselves; the returned states should - * only be taken in the context of the given message. + * Returns the IDs and states of all dependencies of the given message. + * Missing dependencies have the state {@link + * org.briarproject.api.sync.ValidationManager.State UNKNOWN}. + * Dependencies in other groups have the state {@link + * org.briarproject.api.sync.ValidationManager.State INVALID}. + * Note that these states are not set on the dependencies themselves; the + * returned states should only be taken in the context of the given message. * <p/> * Read-only. */ @@ -293,7 +296,9 @@ interface Database<T> { throws DbException; /** - * Returns all IDs of messages that depend on the given message. + * Returns all IDs and states of all dependents of the given message. + * Messages in other groups that declare a dependency on the given message + * will be returned even though such dependencies are invalid. * <p/> * Read-only. */ @@ -350,6 +355,13 @@ interface Database<T> { */ Metadata getMessageMetadata(T txn, MessageId m) throws DbException; + /** + * Returns the validation and delivery state of the given message. + * <p/> + * Read-only. + */ + State getMessageState(T txn, MessageId m) throws DbException; + /** * Returns the status of all messages in the given group with respect * to the given contact. @@ -377,15 +389,6 @@ interface Database<T> { Collection<MessageId> getMessagesToAck(T txn, ContactId c, int maxMessages) throws DbException; - /** - * Returns the IDs of any messages that need to be delivered to the given - * client. - * <p/> - * Read-only. - */ - Collection<MessageId> getMessagesToDeliver(T txn, ClientId c) - throws DbException; - /** * Returns the IDs of some messages that are eligible to be offered to the * given contact, up to the given number of messages. @@ -437,6 +440,7 @@ interface Database<T> { * <p/> * Read-only. */ + @Nullable byte[] getRawMessage(T txn, MessageId m) throws DbException; /** @@ -592,7 +596,7 @@ interface Database<T> { * Marks the given contact as active or inactive. */ void setContactActive(T txn, ContactId c, boolean active) - throws DbException; + throws DbException; /** * Marks the given message as shared or unshared. @@ -601,10 +605,9 @@ interface Database<T> { throws DbException; /** - * Marks the given message as valid or invalid. + * Sets the validation and delivery state of the given message. */ - void setMessageState(T txn, MessageId m, State state) - throws DbException; + void setMessageState(T txn, MessageId m, State state) throws DbException; /** * Sets the reordering window for the given contact and transport in the diff --git a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java index 6e7d75aa44..ab4bf27644 100644 --- a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java +++ b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java @@ -28,10 +28,10 @@ import org.briarproject.api.event.LocalAuthorRemovedEvent; import org.briarproject.api.event.MessageAddedEvent; import org.briarproject.api.event.MessageRequestedEvent; import org.briarproject.api.event.MessageSharedEvent; +import org.briarproject.api.event.MessageStateChangedEvent; import org.briarproject.api.event.MessageToAckEvent; import org.briarproject.api.event.MessageToRequestEvent; import org.briarproject.api.event.MessagesAckedEvent; -import org.briarproject.api.event.MessageStateChangedEvent; import org.briarproject.api.event.MessagesSentEvent; import org.briarproject.api.event.SettingsUpdatedEvent; import org.briarproject.api.identity.Author; @@ -50,6 +50,7 @@ import org.briarproject.api.sync.Offer; import org.briarproject.api.sync.Request; import org.briarproject.api.sync.ValidationManager.State; import org.briarproject.api.transport.TransportKeys; +import org.jetbrains.annotations.Nullable; import java.util.ArrayList; import java.util.Collection; @@ -188,7 +189,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { } } - public void addLocalMessage(Transaction transaction, Message m, ClientId c, + public void addLocalMessage(Transaction transaction, Message m, Metadata meta, boolean shared) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); T txn = unbox(transaction); @@ -197,7 +198,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { if (!db.containsMessage(txn, m.getId())) { addMessage(txn, m, DELIVERED, shared); transaction.attach(new MessageAddedEvent(m, null)); - transaction.attach(new MessageStateChangedEvent(m, c, true, + transaction.attach(new MessageStateChangedEvent(m.getId(), true, DELIVERED)); if (shared) transaction.attach(new MessageSharedEvent(m.getId())); } @@ -271,6 +272,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { db.deleteMessageMetadata(txn, m); } + @Nullable public Ack generateAck(Transaction transaction, ContactId c, int maxMessages) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); @@ -283,6 +285,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { return new Ack(ids); } + @Nullable public Collection<byte[]> generateBatch(Transaction transaction, ContactId c, int maxLength, int maxLatency) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); @@ -301,6 +304,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { return Collections.unmodifiableList(messages); } + @Nullable public Offer generateOffer(Transaction transaction, ContactId c, int maxMessages, int maxLatency) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); @@ -313,6 +317,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { return new Offer(ids); } + @Nullable public Request generateRequest(Transaction transaction, ContactId c, int maxMessages) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); @@ -326,6 +331,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { return new Request(ids); } + @Nullable public Collection<byte[]> generateRequestedBatch(Transaction transaction, ContactId c, int maxLength, int maxLatency) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); @@ -420,18 +426,13 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { return db.getMessagesToValidate(txn, c); } - public Collection<MessageId> getMessagesToDeliver(Transaction transaction, - ClientId c) throws DbException { - T txn = unbox(transaction); - return db.getMessagesToDeliver(txn, c); - } - public Collection<MessageId> getPendingMessages(Transaction transaction, ClientId c) throws DbException { T txn = unbox(transaction); return db.getPendingMessages(txn, c); } + @Nullable public byte[] getRawMessage(Transaction transaction, MessageId m) throws DbException { T txn = unbox(transaction); @@ -473,6 +474,14 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { return db.getMessageMetadataForValidator(txn, m); } + public State getMessageState(Transaction transaction, MessageId m) + throws DbException { + T txn = unbox(transaction); + if (!db.containsMessage(txn, m)) + throw new NoSuchMessageException(); + return db.getMessageState(txn, m); + } + public Collection<MessageStatus> getMessageStatus(Transaction transaction, ContactId c, GroupId g) throws DbException { T txn = unbox(transaction); @@ -720,14 +729,14 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { if (shared) transaction.attach(new MessageSharedEvent(m)); } - public void setMessageState(Transaction transaction, Message m, ClientId c, + public void setMessageState(Transaction transaction, MessageId m, State state) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); T txn = unbox(transaction); - if (!db.containsMessage(txn, m.getId())) + if (!db.containsMessage(txn, m)) throw new NoSuchMessageException(); - db.setMessageState(txn, m.getId(), state); - transaction.attach(new MessageStateChangedEvent(m, c, false, state)); + db.setMessageState(txn, m, state); + transaction.attach(new MessageStateChangedEvent(m, false, state)); } public void addMessageDependencies(Transaction transaction, @@ -737,8 +746,9 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { T txn = unbox(transaction); if (!db.containsMessage(txn, dependent.getId())) throw new NoSuchMessageException(); - for (MessageId dependencyId : dependencies) { - db.addMessageDependency(txn, dependent.getId(), dependencyId); + for (MessageId dependency : dependencies) { + db.addMessageDependency(txn, dependent.getGroupId(), + dependent.getId(), dependency); } } diff --git a/briar-core/src/org/briarproject/db/JdbcDatabase.java b/briar-core/src/org/briarproject/db/JdbcDatabase.java index 24d07373f8..9015a3e473 100644 --- a/briar-core/src/org/briarproject/db/JdbcDatabase.java +++ b/briar-core/src/org/briarproject/db/JdbcDatabase.java @@ -25,6 +25,7 @@ import org.briarproject.api.transport.IncomingKeys; import org.briarproject.api.transport.OutgoingKeys; import org.briarproject.api.transport.TransportKeys; import org.briarproject.util.StringUtils; +import org.jetbrains.annotations.Nullable; import java.security.SecureRandom; import java.sql.Connection; @@ -53,7 +54,6 @@ import static org.briarproject.api.sync.ValidationManager.State.DELIVERED; import static org.briarproject.api.sync.ValidationManager.State.INVALID; import static org.briarproject.api.sync.ValidationManager.State.PENDING; import static org.briarproject.api.sync.ValidationManager.State.UNKNOWN; -import static org.briarproject.api.sync.ValidationManager.State.VALID; import static org.briarproject.db.DatabaseConstants.DB_SETTINGS_NAMESPACE; import static org.briarproject.db.DatabaseConstants.DEVICE_ID_KEY; import static org.briarproject.db.DatabaseConstants.DEVICE_SETTINGS_NAMESPACE; @@ -67,8 +67,8 @@ import static org.briarproject.db.ExponentialBackoff.calculateExpiry; */ abstract class JdbcDatabase implements Database<Connection> { - private static final int SCHEMA_VERSION = 26; - private static final int MIN_SCHEMA_VERSION = 26; + private static final int SCHEMA_VERSION = 27; + private static final int MIN_SCHEMA_VERSION = 27; private static final String CREATE_SETTINGS = "CREATE TABLE settings" @@ -155,8 +155,12 @@ abstract class JdbcDatabase implements Database<Connection> { private static final String CREATE_MESSAGE_DEPENDENCIES = "CREATE TABLE messageDependencies" - + " (messageId HASH NOT NULL," + + " (groupId HASH NOT NULL," + + " messageId HASH NOT NULL," + " dependencyId HASH NOT NULL," // Not a foreign key + + " FOREIGN KEY (groupId)" + + " REFERENCES groups (groupId)" + + " ON DELETE CASCADE," + " FOREIGN KEY (messageId)" + " REFERENCES messages (messageId)" + " ON DELETE CASCADE)"; @@ -609,16 +613,17 @@ abstract class JdbcDatabase implements Database<Connection> { } } - public void addMessageDependency(Connection txn, MessageId dependentId, - MessageId dependencyId) throws DbException { + public void addMessageDependency(Connection txn, GroupId g, + MessageId dependent, MessageId dependency) throws DbException { PreparedStatement ps = null; try { - String sql = - "INSERT INTO messageDependencies (messageId, dependencyId)" - + " VALUES (?, ?)"; + String sql = "INSERT INTO messageDependencies" + + " (groupId, messageId, dependencyId)" + + " VALUES (?, ?, ?)"; ps = txn.prepareStatement(sql); - ps.setBytes(1, dependentId.getBytes()); - ps.setBytes(2, dependencyId.getBytes()); + ps.setBytes(1, g.getBytes()); + ps.setBytes(2, dependent.getBytes()); + ps.setBytes(3, dependency.getBytes()); int affected = ps.executeUpdate(); if (affected != 1) throw new DbStateException(); ps.close(); @@ -1453,7 +1458,7 @@ abstract class JdbcDatabase implements Database<Connection> { PreparedStatement ps = null; ResultSet rs = null; try { - String sql = "SELECT d.dependencyId, m.state, m.groupId" + String sql = "SELECT d.dependencyId, m.state, d.groupId, m.groupId" + " FROM messageDependencies AS d" + " LEFT OUTER JOIN messages AS m" + " ON d.dependencyId = m.messageId" @@ -1463,13 +1468,17 @@ abstract class JdbcDatabase implements Database<Connection> { rs = ps.executeQuery(); Map<MessageId, State> dependencies = new HashMap<MessageId, State>(); while (rs.next()) { - MessageId messageId = new MessageId(rs.getBytes(1)); + MessageId dependency = new MessageId(rs.getBytes(1)); State state = State.fromValue(rs.getInt(2)); - if (state != UNKNOWN) { - // set dependency invalid if it is in a different group - if (!hasGroupId(txn, m, rs.getBytes(3))) state = INVALID; + if (rs.wasNull()) { + state = UNKNOWN; // Missing dependency + } else { + GroupId dependentGroupId = new GroupId(rs.getBytes(3)); + GroupId dependencyGroupId = new GroupId(rs.getBytes(4)); + if (!dependentGroupId.equals(dependencyGroupId)) + state = INVALID; // Dependency in another group } - dependencies.put(messageId, state); + dependencies.put(dependency, state); } rs.close(); ps.close(); @@ -1481,22 +1490,28 @@ abstract class JdbcDatabase implements Database<Connection> { } } - private boolean hasGroupId(Connection txn, MessageId m, byte[] g) - throws DbException { + public Map<MessageId, State> getMessageDependents(Connection txn, + MessageId m) throws DbException { PreparedStatement ps = null; ResultSet rs = null; try { - String sql = "SELECT NULL FROM messages" - + " WHERE messageId = ? AND groupId = ?"; + String sql = "SELECT d.messageId, m.state" + + " FROM messageDependencies AS d" + + " JOIN messages AS m" + + " ON d.messageId = m.messageId" + + " WHERE dependencyId = ?"; ps = txn.prepareStatement(sql); ps.setBytes(1, m.getBytes()); - ps.setBytes(2, g); rs = ps.executeQuery(); - boolean same = rs.next(); - if (rs.next()) throw new DbStateException(); + Map<MessageId, State> dependents = new HashMap<MessageId, State>(); + while (rs.next()) { + MessageId dependent = new MessageId(rs.getBytes(1)); + State state = State.fromValue(rs.getInt(2)); + dependents.put(dependent, state); + } rs.close(); ps.close(); - return same; + return Collections.unmodifiableMap(dependents); } catch (SQLException e) { tryToClose(rs); tryToClose(ps); @@ -1504,28 +1519,21 @@ abstract class JdbcDatabase implements Database<Connection> { } } - public Map<MessageId, State> getMessageDependents(Connection txn, - MessageId m) throws DbException { + public State getMessageState(Connection txn, MessageId m) + throws DbException { PreparedStatement ps = null; ResultSet rs = null; try { - String sql = "SELECT d.messageId, m.state" - + " FROM messageDependencies AS d" - + " LEFT OUTER JOIN messages AS m" - + " ON d.messageId = m.messageId" - + " WHERE dependencyId = ?"; + String sql = "SELECT state FROM messages WHERE messageId = ?"; ps = txn.prepareStatement(sql); ps.setBytes(1, m.getBytes()); rs = ps.executeQuery(); - Map<MessageId, State> dependents = new HashMap<MessageId, State>(); - while (rs.next()) { - MessageId messageId = new MessageId(rs.getBytes(1)); - State state = State.fromValue(rs.getInt(2)); - dependents.put(messageId, state); - } + if (!rs.next()) throw new DbStateException(); + State state = State.fromValue(rs.getInt(1)); + if (rs.next()) throw new DbStateException(); rs.close(); ps.close(); - return Collections.unmodifiableMap(dependents); + return state; } catch (SQLException e) { tryToClose(rs); tryToClose(ps); @@ -1655,11 +1663,6 @@ abstract class JdbcDatabase implements Database<Connection> { return getMessagesInState(txn, c, UNKNOWN); } - public Collection<MessageId> getMessagesToDeliver(Connection txn, - ClientId c) throws DbException { - return getMessagesInState(txn, c, VALID); - } - public Collection<MessageId> getPendingMessages(Connection txn, ClientId c) throws DbException { return getMessagesInState(txn, c, PENDING); @@ -1689,6 +1692,7 @@ abstract class JdbcDatabase implements Database<Connection> { } } + @Nullable public byte[] getRawMessage(Connection txn, MessageId m) throws DbException { PreparedStatement ps = null; diff --git a/briar-core/src/org/briarproject/forum/ForumManagerImpl.java b/briar-core/src/org/briarproject/forum/ForumManagerImpl.java index 3e8b5de64e..09c8b82bd8 100644 --- a/briar-core/src/org/briarproject/forum/ForumManagerImpl.java +++ b/briar-core/src/org/briarproject/forum/ForumManagerImpl.java @@ -33,7 +33,6 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.logging.Logger; import javax.inject.Inject; @@ -49,9 +48,6 @@ import static org.briarproject.api.identity.Author.Status.ANONYMOUS; class ForumManagerImpl extends BdfIncomingMessageHook implements ForumManager { - private static final Logger LOG = - Logger.getLogger(ForumManagerImpl.class.getName()); - static final ClientId CLIENT_ID = new ClientId(StringUtils.fromHexString( "859a7be50dca035b64bd6902fb797097" + "795af837abbf8c16d750b3c2ccc186ea")); @@ -133,7 +129,7 @@ class ForumManagerImpl extends BdfIncomingMessageHook implements ForumManager { } meta.put(KEY_LOCAL, true); meta.put(KEY_READ, true); - clientHelper.addLocalMessage(p.getMessage(), CLIENT_ID, meta, true); + clientHelper.addLocalMessage(p.getMessage(), meta, true); } catch (FormatException e) { throw new RuntimeException(e); } diff --git a/briar-core/src/org/briarproject/forum/ForumPostValidator.java b/briar-core/src/org/briarproject/forum/ForumPostValidator.java index e0a86a7b8c..258ac9bb5f 100644 --- a/briar-core/src/org/briarproject/forum/ForumPostValidator.java +++ b/briar-core/src/org/briarproject/forum/ForumPostValidator.java @@ -101,7 +101,7 @@ class ForumPostValidator extends BdfMessageValidator { } // Return the metadata and dependencies BdfDictionary meta = new BdfDictionary(); - Collection<MessageId> dependencies = null; + Collection<MessageId> dependencies = Collections.emptyList(); meta.put("timestamp", m.getTimestamp()); if (parent != null) { meta.put("parent", parent); diff --git a/briar-core/src/org/briarproject/introduction/IntroduceeManager.java b/briar-core/src/org/briarproject/introduction/IntroduceeManager.java index 0223c14b3e..c5df31c108 100644 --- a/briar-core/src/org/briarproject/introduction/IntroduceeManager.java +++ b/briar-core/src/org/briarproject/introduction/IntroduceeManager.java @@ -177,8 +177,7 @@ class IntroduceeManager { d.put(REMOTE_AUTHOR_IS_US, introducesOtherIdentity); // save local state to database - clientHelper.addLocalMessage(txn, localMsg, - IntroductionManagerImpl.CLIENT_ID, d, false); + clientHelper.addLocalMessage(txn, localMsg, d, false); return d; } diff --git a/briar-core/src/org/briarproject/introduction/IntroducerManager.java b/briar-core/src/org/briarproject/introduction/IntroducerManager.java index 891c4b7b59..b8c414a477 100644 --- a/briar-core/src/org/briarproject/introduction/IntroducerManager.java +++ b/briar-core/src/org/briarproject/introduction/IntroducerManager.java @@ -98,9 +98,7 @@ class IntroducerManager { d.put(AUTHOR_ID_2, c2.getAuthor().getId()); // save local state to database - clientHelper - .addLocalMessage(txn, m, IntroductionManagerImpl.CLIENT_ID, d, - false); + clientHelper.addLocalMessage(txn, m, d, false); return d; } diff --git a/briar-core/src/org/briarproject/messaging/MessagingManagerImpl.java b/briar-core/src/org/briarproject/messaging/MessagingManagerImpl.java index 052df83c3b..f66e9d7e7a 100644 --- a/briar-core/src/org/briarproject/messaging/MessagingManagerImpl.java +++ b/briar-core/src/org/briarproject/messaging/MessagingManagerImpl.java @@ -117,7 +117,7 @@ class MessagingManagerImpl extends BdfIncomingMessageHook meta.put("contentType", m.getContentType()); meta.put("local", true); meta.put("read", true); - clientHelper.addLocalMessage(m.getMessage(), CLIENT_ID, meta, true); + clientHelper.addLocalMessage(m.getMessage(), meta, true); } catch (FormatException e) { throw new RuntimeException(e); } diff --git a/briar-core/src/org/briarproject/properties/TransportPropertyManagerImpl.java b/briar-core/src/org/briarproject/properties/TransportPropertyManagerImpl.java index b957adab5c..8c7e60549a 100644 --- a/briar-core/src/org/briarproject/properties/TransportPropertyManagerImpl.java +++ b/briar-core/src/org/briarproject/properties/TransportPropertyManagerImpl.java @@ -246,7 +246,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, meta.put("transportId", t.getString()); meta.put("version", version); meta.put("local", local); - clientHelper.addLocalMessage(txn, m, CLIENT_ID, meta, shared); + clientHelper.addLocalMessage(txn, m, meta, shared); } catch (FormatException e) { throw new RuntimeException(e); } diff --git a/briar-core/src/org/briarproject/sharing/SharingManagerImpl.java b/briar-core/src/org/briarproject/sharing/SharingManagerImpl.java index 9528bf88ba..8f2d4925a8 100644 --- a/briar-core/src/org/briarproject/sharing/SharingManagerImpl.java +++ b/briar-core/src/org/briarproject/sharing/SharingManagerImpl.java @@ -626,7 +626,7 @@ abstract class SharingManagerImpl<S extends Shareable, I extends Invitation, IS // save local state to database BdfDictionary d = s.toBdfDictionary(); - clientHelper.addLocalMessage(txn, m, getClientId(), d, false); + clientHelper.addLocalMessage(txn, m, d, false); return s; } @@ -652,7 +652,7 @@ abstract class SharingManagerImpl<S extends Shareable, I extends Invitation, IS // save local state to database BdfDictionary d = s.toBdfDictionary(); - clientHelper.addLocalMessage(txn, m, getClientId(), d, false); + clientHelper.addLocalMessage(txn, m, d, false); return s; } diff --git a/briar-core/src/org/briarproject/sync/ValidationManagerImpl.java b/briar-core/src/org/briarproject/sync/ValidationManagerImpl.java index 86cd80d73a..1dba8b9c37 100644 --- a/briar-core/src/org/briarproject/sync/ValidationManagerImpl.java +++ b/briar-core/src/org/briarproject/sync/ValidationManagerImpl.java @@ -41,7 +41,6 @@ import static org.briarproject.api.sync.SyncConstants.MESSAGE_HEADER_LENGTH; import static org.briarproject.api.sync.ValidationManager.State.DELIVERED; import static org.briarproject.api.sync.ValidationManager.State.INVALID; import static org.briarproject.api.sync.ValidationManager.State.PENDING; -import static org.briarproject.api.sync.ValidationManager.State.VALID; class ValidationManagerImpl implements ValidationManager, Service, EventListener { @@ -71,8 +70,8 @@ class ValidationManagerImpl implements ValidationManager, Service, public void startService() { if (used.getAndSet(true)) throw new IllegalStateException(); for (ClientId c : validators.keySet()) { - validateOutstandingMessages(c); - deliverOutstandingMessages(c); + validateOutstandingMessagesAsync(c); + deliverOutstandingMessagesAsync(c); } } @@ -91,166 +90,154 @@ class ValidationManagerImpl implements ValidationManager, Service, hooks.put(c, hook); } - private void validateOutstandingMessages(final ClientId c) { + private void validateOutstandingMessagesAsync(final ClientId c) { dbExecutor.execute(new Runnable() { @Override public void run() { - try { - Queue<MessageId> unvalidated = new LinkedList<MessageId>(); - Transaction txn = db.startTransaction(true); - try { - unvalidated.addAll(db.getMessagesToValidate(txn, c)); - txn.setComplete(); - } finally { - db.endTransaction(txn); - } - validateNextMessage(unvalidated); - } catch (DbException e) { - if (LOG.isLoggable(WARNING)) - LOG.log(WARNING, e.toString(), e); - } + validateOutstandingMessages(c); } }); } - private void validateNextMessage(final Queue<MessageId> unvalidated) { + private void validateOutstandingMessages(ClientId c) { + try { + Queue<MessageId> unvalidated = new LinkedList<MessageId>(); + Transaction txn = db.startTransaction(true); + try { + unvalidated.addAll(db.getMessagesToValidate(txn, c)); + txn.setComplete(); + } finally { + db.endTransaction(txn); + } + validateNextMessageAsync(unvalidated); + } catch (DbException e) { + if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); + } + } + + private void validateNextMessageAsync(final Queue<MessageId> unvalidated) { if (unvalidated.isEmpty()) return; dbExecutor.execute(new Runnable() { @Override public void run() { - try { - Message m = null; - Group g = null; - Transaction txn = db.startTransaction(true); - try { - MessageId id = unvalidated.poll(); - byte[] raw = db.getRawMessage(txn, id); - m = parseMessage(id, raw); - g = db.getGroup(txn, m.getGroupId()); - txn.setComplete(); - } catch (NoSuchMessageException e) { - LOG.info("Message removed before validation"); - // Continue to next message - } catch (NoSuchGroupException e) { - LOG.info("Group removed before validation"); - // Continue to next message - } finally { - if (!txn.isComplete()) txn.setComplete(); - db.endTransaction(txn); - } - if (m != null && g != null) validateMessage(m, g); - validateNextMessage(unvalidated); - } catch (DbException e) { - if (LOG.isLoggable(WARNING)) - LOG.log(WARNING, e.toString(), e); - } + validateNextMessage(unvalidated); } }); } - private void deliverOutstandingMessages(final ClientId c) { - dbExecutor.execute(new Runnable() { - @Override - public void run() { - try { - Queue<MessageId> validated = new LinkedList<MessageId>(); - Queue<MessageId> pending = new LinkedList<MessageId>(); - Transaction txn = db.startTransaction(true); - try { - validated.addAll(db.getMessagesToDeliver(txn, c)); - pending.addAll(db.getPendingMessages(txn, c)); - txn.setComplete(); - } finally { - db.endTransaction(txn); - } - deliverNextMessage(validated); - deliverNextPendingMessage(pending); - } catch (DbException e) { - if (LOG.isLoggable(WARNING)) - LOG.log(WARNING, e.toString(), e); - } + private void validateNextMessage(Queue<MessageId> unvalidated) { + try { + Message m; + Group g; + Transaction txn = db.startTransaction(true); + try { + MessageId id = unvalidated.poll(); + byte[] raw = db.getRawMessage(txn, id); + m = parseMessage(id, raw); + g = db.getGroup(txn, m.getGroupId()); + txn.setComplete(); + } finally { + db.endTransaction(txn); } - }); + validateMessageAsync(m, g); + validateNextMessageAsync(unvalidated); + } catch (NoSuchMessageException e) { + LOG.info("Message removed before validation"); + validateNextMessageAsync(unvalidated); + } catch (NoSuchGroupException e) { + LOG.info("Group removed before validation"); + validateNextMessageAsync(unvalidated); + } catch (DbException e) { + if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); + } } - private void deliverNextMessage(final Queue<MessageId> validated) { - if (validated.isEmpty()) return; + private void deliverOutstandingMessagesAsync(final ClientId c) { dbExecutor.execute(new Runnable() { @Override public void run() { - try { - Message m = null; - Group g = null; - Metadata meta = null; - Transaction txn = db.startTransaction(true); - try { - MessageId id = validated.poll(); - byte[] raw = db.getRawMessage(txn, id); - m = parseMessage(id, raw); - g = db.getGroup(txn, m.getGroupId()); - meta = db.getMessageMetadataForValidator(txn, id); - txn.setComplete(); - } finally { - db.endTransaction(txn); - } - if (g != null) deliverMessage(m, g.getClientId(), meta); - deliverNextMessage(validated); - } catch (DbException e) { - if (LOG.isLoggable(WARNING)) - LOG.log(WARNING, e.toString(), e); - } + deliverOutstandingMessages(c); } }); } - private void deliverNextPendingMessage(final Queue<MessageId> pending) { + private void deliverOutstandingMessages(ClientId c) { + try { + Queue<MessageId> pending = new LinkedList<MessageId>(); + Transaction txn = db.startTransaction(true); + try { + pending.addAll(db.getPendingMessages(txn, c)); + txn.setComplete(); + } finally { + db.endTransaction(txn); + } + deliverNextPendingMessageAsync(pending); + } catch (DbException e) { + if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); + } + } + + private void deliverNextPendingMessageAsync( + final Queue<MessageId> pending) { if (pending.isEmpty()) return; dbExecutor.execute(new Runnable() { @Override public void run() { - Message m = null; - ClientId c = null; - try { - boolean allDelivered = true; - Metadata meta = null; - Transaction txn = db.startTransaction(true); - try { - MessageId id = pending.poll(); - byte[] raw = db.getRawMessage(txn, id); - m = parseMessage(id, raw); + deliverNextPendingMessage(pending); + } + }); + } + + private void deliverNextPendingMessage(Queue<MessageId> pending) { + try { + boolean anyInvalid = false, allDelivered = true; + Queue<MessageId> invalidate = null; + Transaction txn = db.startTransaction(false); + try { + MessageId id = pending.poll(); + // Check if message is still pending + if (db.getMessageState(txn, id) == PENDING) { + // Check if dependencies are valid and delivered + Map<MessageId, State> states = + db.getMessageDependencies(txn, id); + for (Entry<MessageId, State> e : states.entrySet()) { + if (e.getValue() == INVALID) anyInvalid = true; + if (e.getValue() != DELIVERED) allDelivered = false; + } + if (anyInvalid) { + if (db.getMessageState(txn, id) != INVALID) { + invalidateMessage(txn, id); + invalidate = getDependentsToInvalidate(txn, id); + } + } else if (allDelivered) { + Message m = parseMessage(id, db.getRawMessage(txn, id)); Group g = db.getGroup(txn, m.getGroupId()); - c = g.getClientId(); - - // check if a dependency is invalid - Map<MessageId, State> states = - db.getMessageDependencies(txn, id); - for (Entry<MessageId, State> d : states.entrySet()) { - if (d.getValue() == INVALID) { - throw new InvalidMessageException( - "Invalid Dependency"); - } - if (d.getValue() != DELIVERED) allDelivered = false; + ClientId c = g.getClientId(); + Metadata meta = db.getMessageMetadataForValidator(txn, + id); + if (deliverMessage(txn, m, c, meta)) { + pending.addAll(getPendingDependents(txn, id)); + } else { + invalidate = getDependentsToInvalidate(txn, id); } - if (allDelivered) - meta = db.getMessageMetadataForValidator(txn, id); - txn.setComplete(); - } finally { - if (!txn.isComplete()) txn.setComplete(); - db.endTransaction(txn); } - if (c != null && allDelivered) deliverMessage(m, c, meta); - deliverNextPendingMessage(pending); - } catch(InvalidMessageException e) { - if (LOG.isLoggable(INFO)) - LOG.log(INFO, e.toString(), e); - markMessageInvalid(m, c); - deliverNextPendingMessage(pending); - } catch (DbException e) { - if (LOG.isLoggable(WARNING)) - LOG.log(WARNING, e.toString(), e); } + txn.setComplete(); + } finally { + db.endTransaction(txn); } - }); + if (invalidate != null) invalidateNextMessageAsync(invalidate); + deliverNextPendingMessageAsync(pending); + } catch (NoSuchMessageException e) { + LOG.info("Message removed before delivery"); + deliverNextPendingMessageAsync(pending); + } catch (NoSuchGroupException e) { + LOG.info("Group removed before delivery"); + deliverNextPendingMessageAsync(pending); + } catch (DbException e) { + if (LOG.isLoggable(WARNING)) + LOG.log(WARNING, e.toString(), e); + } } private Message parseMessage(MessageId id, byte[] raw) { @@ -262,199 +249,168 @@ class ValidationManagerImpl implements ValidationManager, Service, return new Message(id, new GroupId(groupId), timestamp, raw); } - private void validateMessage(final Message m, final Group g) { + private void validateMessageAsync(final Message m, final Group g) { cryptoExecutor.execute(new Runnable() { @Override public void run() { - MessageValidator v = validators.get(g.getClientId()); - if (v == null) { - LOG.warning("No validator"); - } else { - try { - MessageContext context = v.validateMessage(m, g); - storeMessageContext(m, g.getClientId(), context); - } catch (InvalidMessageException e) { - if (LOG.isLoggable(INFO)) - LOG.log(INFO, e.toString(), e); - markMessageInvalid(m, g.getClientId()); - } - } + validateMessage(m, g); } }); } - private void storeMessageContext(final Message m, final ClientId c, + private void validateMessage(Message m, Group g) { + MessageValidator v = validators.get(g.getClientId()); + if (v == null) { + LOG.warning("No validator"); + } else { + try { + MessageContext context = v.validateMessage(m, g); + storeMessageContextAsync(m, g.getClientId(), context); + } catch (InvalidMessageException e) { + if (LOG.isLoggable(INFO)) LOG.info(e.toString()); + Queue<MessageId> invalidate = new LinkedList<MessageId>(); + invalidate.add(m.getId()); + invalidateNextMessageAsync(invalidate); + } + } + } + + private void storeMessageContextAsync(final Message m, final ClientId c, final MessageContext result) { dbExecutor.execute(new Runnable() { @Override public void run() { - try { - State newState = null; - Metadata meta = null; - Transaction txn = db.startTransaction(false); - try { - // store dependencies - Collection<MessageId> dependencies = - result.getDependencies(); - if (dependencies != null && dependencies.size() > 0) { - db.addMessageDependencies(txn, m, dependencies); - } - // check if a dependency is invalid - // and if all dependencies have been delivered - Map<MessageId, State> states = - db.getMessageDependencies(txn, m.getId()); - newState = VALID; - for (Entry<MessageId, State> d : states.entrySet()) { - if (d.getValue() == INVALID) { - throw new InvalidMessageException( - "Dependency Invalid"); - } - if (d.getValue() != DELIVERED) { - newState = PENDING; - LOG.info("depend. undelivered, set to PENDING"); - break; - } - } - // save metadata and new message state - meta = result.getMetadata(); - db.mergeMessageMetadata(txn, m.getId(), meta); - db.setMessageState(txn, m, c, newState); - txn.setComplete(); - } finally { - if (!txn.isComplete()) txn.setComplete(); - db.endTransaction(txn); - } - // deliver message if valid - if (newState == VALID) { - deliverMessage(m, c, meta); - } - } catch (InvalidMessageException e) { - if (LOG.isLoggable(INFO)) - LOG.log(INFO, e.toString(), e); - markMessageInvalid(m, c); - } catch (DbException e) { - if (LOG.isLoggable(WARNING)) - LOG.log(WARNING, e.toString(), e); - } + storeMessageContext(m, c, result); } }); } - private void deliverMessage(final Message m, final ClientId c, - final Metadata meta) { - dbExecutor.execute(new Runnable() { - @Override - public void run() { - try { - Queue<MessageId> pending = new LinkedList<MessageId>(); - Transaction txn = db.startTransaction(false); - try { - IncomingMessageHook hook = hooks.get(c); - if (hook != null) - hook.incomingMessage(txn, m, meta); - - // check if message was deleted by client - if (db.getRawMessage(txn, m.getId()) == null) { - throw new InvalidMessageException( - "Deleted by Client"); - } - db.setMessageState(txn, m, c, DELIVERED); - - // deliver pending dependents - Map<MessageId, State> dependents = - db.getMessageDependents(txn, m.getId()); - for (Entry<MessageId, State> i : dependents - .entrySet()) { - if (i.getValue() != PENDING) continue; - - // check that all dependencies are delivered - Map<MessageId, State> dependencies = - db.getMessageDependencies(txn, i.getKey()); - for (Entry<MessageId, State> j : dependencies - .entrySet()) { - if (j.getValue() != DELIVERED) return; - } - pending.add(i.getKey()); + private void storeMessageContext(Message m, ClientId c, + MessageContext result) { + try { + MessageId id = m.getId(); + boolean anyInvalid = false, allDelivered = true; + Queue<MessageId> invalidate = null; + Queue<MessageId> pending = null; + Transaction txn = db.startTransaction(false); + try { + // Check if message has any dependencies + Collection<MessageId> dependencies = result.getDependencies(); + if (!dependencies.isEmpty()) { + db.addMessageDependencies(txn, m, dependencies); + // Check if dependencies are valid and delivered + Map<MessageId, State> states = + db.getMessageDependencies(txn, id); + for (Entry<MessageId, State> e : states.entrySet()) { + if (e.getValue() == INVALID) anyInvalid = true; + if (e.getValue() != DELIVERED) allDelivered = false; + } + } + if (anyInvalid) { + if (db.getMessageState(txn, id) != INVALID) { + invalidateMessage(txn, id); + invalidate = getDependentsToInvalidate(txn, id); + } + } else { + Metadata meta = result.getMetadata(); + db.mergeMessageMetadata(txn, id, meta); + if (allDelivered) { + if (deliverMessage(txn, m, c, meta)) { + pending = getPendingDependents(txn, id); + } else { + invalidate = getDependentsToInvalidate(txn, id); } - txn.setComplete(); - } finally { - if (!txn.isComplete()) txn.setComplete(); - db.endTransaction(txn); + } else { + db.setMessageState(txn, id, PENDING); } - deliverNextMessage(pending); - } catch (InvalidMessageException e) { - if (LOG.isLoggable(INFO)) - LOG.log(INFO, e.toString(), e); - markMessageInvalid(m, c); - } catch (DbException e) { - if (LOG.isLoggable(WARNING)) - LOG.log(WARNING, e.toString(), e); } + txn.setComplete(); + } finally { + db.endTransaction(txn); } - }); + if (invalidate != null) invalidateNextMessageAsync(invalidate); + if (pending != null) deliverNextPendingMessageAsync(pending); + } catch (NoSuchMessageException e) { + LOG.info("Message removed during validation"); + } catch (NoSuchGroupException e) { + LOG.info("Group removed during validation"); + } catch (DbException e) { + if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); + } + } + + private boolean deliverMessage(Transaction txn, Message m, ClientId c, + Metadata meta) throws DbException { + IncomingMessageHook hook = hooks.get(c); + if (hook == null) throw new DbException(); + hook.incomingMessage(txn, m, meta); + // TODO: Find a better way for clients to signal validity, #643 + if (db.getRawMessage(txn, m.getId()) == null) { + db.setMessageState(txn, m.getId(), INVALID); + return false; + } else { + db.setMessageState(txn, m.getId(), DELIVERED); + return true; + } + } + + private Queue<MessageId> getPendingDependents(Transaction txn, MessageId m) + throws DbException { + Queue<MessageId> pending = new LinkedList<MessageId>(); + Map<MessageId, State> states = db.getMessageDependents(txn, m); + for (Entry<MessageId, State> e : states.entrySet()) { + if (e.getValue() == PENDING) pending.add(e.getKey()); + } + return pending; } - private void markMessageInvalid(final Message m, final ClientId c) { + private void invalidateNextMessageAsync(final Queue<MessageId> invalidate) { + if (invalidate.isEmpty()) return; dbExecutor.execute(new Runnable() { @Override public void run() { - try { - Queue<MessageId> invalid = new LinkedList<MessageId>(); - Transaction txn = db.startTransaction(false); - try { - Map<MessageId, State> dependents = - db.getMessageDependents(txn, m.getId()); - db.setMessageState(txn, m, c, INVALID); - db.deleteMessage(txn, m.getId()); - db.deleteMessageMetadata(txn, m.getId()); - - // recursively invalidate all messages that depend on m - // TODO check that cycles are properly taken care of - for (Entry<MessageId, State> i : dependents - .entrySet()) { - if (i.getValue() != INVALID) { - invalid.add(i.getKey()); - } - } - txn.setComplete(); - } finally { - db.endTransaction(txn); - } - markNextMessageInvalid(invalid); - } catch (DbException e) { - if (LOG.isLoggable(WARNING)) - LOG.log(WARNING, e.toString(), e); - } + invalidateNextMessage(invalidate); } }); } - private void markNextMessageInvalid(final Queue<MessageId> invalid) { - if (invalid.isEmpty()) return; - dbExecutor.execute(new Runnable() { - @Override - public void run() { - try { - Message m = null; - Group g = null; - Transaction txn = db.startTransaction(true); - try { - MessageId id = invalid.poll(); - byte[] raw = db.getRawMessage(txn, id); - m = parseMessage(id, raw); - g = db.getGroup(txn, m.getGroupId()); - txn.setComplete(); - } finally { - db.endTransaction(txn); - } - if (g != null) markMessageInvalid(m, g.getClientId()); - markNextMessageInvalid(invalid); - } catch (DbException e) { - if (LOG.isLoggable(WARNING)) - LOG.log(WARNING, e.toString(), e); + private void invalidateNextMessage(Queue<MessageId> invalidate) { + try { + Transaction txn = db.startTransaction(false); + try { + MessageId id = invalidate.poll(); + if (db.getMessageState(txn, id) != INVALID) { + invalidateMessage(txn, id); + invalidate.addAll(getDependentsToInvalidate(txn, id)); } + txn.setComplete(); + } finally { + db.endTransaction(txn); } - }); + invalidateNextMessageAsync(invalidate); + } catch (NoSuchMessageException e) { + LOG.info("Message removed before invalidation"); + invalidateNextMessageAsync(invalidate); + } catch (DbException e) { + if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); + } + } + + private void invalidateMessage(Transaction txn, MessageId m) + throws DbException { + db.setMessageState(txn, m, INVALID); + db.deleteMessage(txn, m); + db.deleteMessageMetadata(txn, m); + } + + private Queue<MessageId> getDependentsToInvalidate(Transaction txn, + MessageId m) throws DbException { + Queue<MessageId> invalidate = new LinkedList<MessageId>(); + Map<MessageId, State> states = db.getMessageDependents(txn, m); + for (Entry<MessageId, State> e : states.entrySet()) { + if (e.getValue() != INVALID) invalidate.add(e.getKey()); + } + return invalidate; } @Override @@ -462,31 +418,35 @@ class ValidationManagerImpl implements ValidationManager, Service, if (e instanceof MessageAddedEvent) { // Validate the message if it wasn't created locally MessageAddedEvent m = (MessageAddedEvent) e; - if (m.getContactId() != null) loadGroupAndValidate(m.getMessage()); + if (m.getContactId() != null) + loadGroupAndValidateAsync(m.getMessage()); } } - private void loadGroupAndValidate(final Message m) { + private void loadGroupAndValidateAsync(final Message m) { dbExecutor.execute(new Runnable() { @Override public void run() { - try { - Group g; - Transaction txn = db.startTransaction(true); - try { - g = db.getGroup(txn, m.getGroupId()); - txn.setComplete(); - } finally { - db.endTransaction(txn); - } - validateMessage(m, g); - } catch (NoSuchGroupException e) { - LOG.info("Group removed before validation"); - } catch (DbException e) { - if (LOG.isLoggable(WARNING)) - LOG.log(WARNING, e.toString(), e); - } + loadGroupAndValidate(m); } }); } + + private void loadGroupAndValidate(final Message m) { + try { + Group g; + Transaction txn = db.startTransaction(true); + try { + g = db.getGroup(txn, m.getGroupId()); + txn.setComplete(); + } finally { + db.endTransaction(txn); + } + validateMessageAsync(m, g); + } catch (NoSuchGroupException e) { + LOG.info("Group removed before validation"); + } catch (DbException e) { + if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); + } + } } diff --git a/briar-tests/src/org/briarproject/blogs/BlogManagerImplTest.java b/briar-tests/src/org/briarproject/blogs/BlogManagerImplTest.java index f2531a6c6b..c7819edddd 100644 --- a/briar-tests/src/org/briarproject/blogs/BlogManagerImplTest.java +++ b/briar-tests/src/org/briarproject/blogs/BlogManagerImplTest.java @@ -292,7 +292,7 @@ public class BlogManagerImplTest extends BriarTestCase { oneOf(db).startTransaction(false); will(returnValue(txn)); oneOf(clientHelper) - .addLocalMessage(txn, message, CLIENT_ID, meta, true); + .addLocalMessage(txn, message, meta, true); oneOf(identityManager) .getAuthorStatus(txn, blog1.getAuthor().getId()); will(returnValue(VERIFIED)); diff --git a/briar-tests/src/org/briarproject/clients/MessageQueueManagerImplTest.java b/briar-tests/src/org/briarproject/clients/MessageQueueManagerImplTest.java index 75dcd60f6f..a46b0227a6 100644 --- a/briar-tests/src/org/briarproject/clients/MessageQueueManagerImplTest.java +++ b/briar-tests/src/org/briarproject/clients/MessageQueueManagerImplTest.java @@ -16,11 +16,11 @@ import org.briarproject.api.sync.ClientId; import org.briarproject.api.sync.Group; import org.briarproject.api.sync.GroupId; import org.briarproject.api.sync.Message; +import org.briarproject.api.sync.MessageContext; import org.briarproject.api.sync.MessageId; import org.briarproject.api.sync.ValidationManager; import org.briarproject.api.sync.ValidationManager.IncomingMessageHook; import org.briarproject.api.sync.ValidationManager.MessageValidator; -import org.briarproject.api.sync.MessageContext; import org.briarproject.util.ByteUtils; import org.hamcrest.Description; import org.jmock.Expectations; @@ -77,7 +77,7 @@ public class MessageQueueManagerImplTest extends BriarTestCase { body); will(new CreateMessageAction()); oneOf(db).addLocalMessage(with(txn), with(any(QueueMessage.class)), - with(clientId), with(messageMetadata), with(true)); + with(messageMetadata), with(true)); // Second message: queue state exists oneOf(db).getGroupMetadata(txn, groupId); will(returnValue(groupMetadata1)); @@ -91,7 +91,7 @@ public class MessageQueueManagerImplTest extends BriarTestCase { body); will(new CreateMessageAction()); oneOf(db).addLocalMessage(with(txn), with(any(QueueMessage.class)), - with(clientId), with(messageMetadata), with(true)); + with(messageMetadata), with(true)); }}); MessageQueueManagerImpl mqm = new MessageQueueManagerImpl(db, diff --git a/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java b/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java index 123ca114c0..7227fcea63 100644 --- a/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java +++ b/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java @@ -27,9 +27,9 @@ import org.briarproject.api.event.LocalAuthorRemovedEvent; import org.briarproject.api.event.MessageAddedEvent; import org.briarproject.api.event.MessageRequestedEvent; import org.briarproject.api.event.MessageSharedEvent; +import org.briarproject.api.event.MessageStateChangedEvent; import org.briarproject.api.event.MessageToAckEvent; import org.briarproject.api.event.MessageToRequestEvent; -import org.briarproject.api.event.MessageStateChangedEvent; import org.briarproject.api.event.MessagesAckedEvent; import org.briarproject.api.event.MessagesSentEvent; import org.briarproject.api.event.SettingsUpdatedEvent; @@ -62,7 +62,6 @@ import java.util.Map; import static org.briarproject.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH; import static org.briarproject.api.sync.ValidationManager.State.DELIVERED; import static org.briarproject.api.sync.ValidationManager.State.UNKNOWN; -import static org.briarproject.api.sync.ValidationManager.State.VALID; import static org.briarproject.api.transport.TransportConstants.REORDERING_WINDOW_SIZE; import static org.briarproject.db.DatabaseConstants.MAX_OFFERED_MESSAGES; import static org.junit.Assert.assertEquals; @@ -241,7 +240,7 @@ public class DatabaseComponentImplTest extends BriarTestCase { Transaction transaction = db.startTransaction(false); try { - db.addLocalMessage(transaction, message, clientId, metadata, true); + db.addLocalMessage(transaction, message, metadata, true); fail(); } catch (NoSuchGroupException expected) { // Expected @@ -284,7 +283,7 @@ public class DatabaseComponentImplTest extends BriarTestCase { Transaction transaction = db.startTransaction(false); try { - db.addLocalMessage(transaction, message, clientId, metadata, true); + db.addLocalMessage(transaction, message, metadata, true); transaction.setComplete(); } finally { db.endTransaction(transaction); @@ -677,11 +676,11 @@ public class DatabaseComponentImplTest extends BriarTestCase { final EventBus eventBus = context.mock(EventBus.class); context.checking(new Expectations() {{ // Check whether the message is in the DB (which it's not) - exactly(10).of(database).startTransaction(); + exactly(11).of(database).startTransaction(); will(returnValue(txn)); - exactly(10).of(database).containsMessage(txn, messageId); + exactly(11).of(database).containsMessage(txn, messageId); will(returnValue(false)); - exactly(10).of(database).abortTransaction(txn); + exactly(11).of(database).abortTransaction(txn); // This is needed for getMessageStatus() to proceed exactly(1).of(database).containsContact(txn, contactId); will(returnValue(true)); @@ -729,6 +728,16 @@ public class DatabaseComponentImplTest extends BriarTestCase { db.endTransaction(transaction); } + transaction = db.startTransaction(false); + try { + db.getMessageState(transaction, messageId); + fail(); + } catch (NoSuchMessageException expected) { + // Expected + } finally { + db.endTransaction(transaction); + } + transaction = db.startTransaction(false); try { db.getMessageStatus(transaction, contactId, messageId); @@ -761,7 +770,7 @@ public class DatabaseComponentImplTest extends BriarTestCase { transaction = db.startTransaction(false); try { - db.setMessageState(transaction, message, clientId, VALID); + db.setMessageState(transaction, messageId, DELIVERED); fail(); } catch (NoSuchMessageException expected) { // Expected @@ -1652,8 +1661,10 @@ public class DatabaseComponentImplTest extends BriarTestCase { // addMessageDependencies() oneOf(database).containsMessage(txn, messageId); will(returnValue(true)); - oneOf(database).addMessageDependency(txn, messageId, messageId1); - oneOf(database).addMessageDependency(txn, messageId, messageId2); + oneOf(database).addMessageDependency(txn, groupId, messageId, + messageId1); + oneOf(database).addMessageDependency(txn, groupId, messageId, + messageId2); // getMessageDependencies() oneOf(database).containsMessage(txn, messageId); will(returnValue(true)); @@ -1664,7 +1675,8 @@ public class DatabaseComponentImplTest extends BriarTestCase { oneOf(database).getMessageDependents(txn, messageId); // broadcast for message added event oneOf(eventBus).broadcast(with(any(MessageAddedEvent.class))); - oneOf(eventBus).broadcast(with(any(MessageStateChangedEvent.class))); + oneOf(eventBus).broadcast(with(any( + MessageStateChangedEvent.class))); oneOf(eventBus).broadcast(with(any(MessageSharedEvent.class))); // endTransaction() oneOf(database).commitTransaction(txn); @@ -1678,7 +1690,7 @@ public class DatabaseComponentImplTest extends BriarTestCase { assertFalse(db.open()); Transaction transaction = db.startTransaction(false); try { - db.addLocalMessage(transaction, message, clientId, metadata, true); + db.addLocalMessage(transaction, message, metadata, true); Collection<MessageId> dependencies = new ArrayList<>(2); dependencies.add(messageId1); dependencies.add(messageId2); diff --git a/briar-tests/src/org/briarproject/db/H2DatabaseTest.java b/briar-tests/src/org/briarproject/db/H2DatabaseTest.java index ffe2917779..87c404006b 100644 --- a/briar-tests/src/org/briarproject/db/H2DatabaseTest.java +++ b/briar-tests/src/org/briarproject/db/H2DatabaseTest.java @@ -50,7 +50,6 @@ import static org.briarproject.api.sync.ValidationManager.State.DELIVERED; import static org.briarproject.api.sync.ValidationManager.State.INVALID; import static org.briarproject.api.sync.ValidationManager.State.PENDING; import static org.briarproject.api.sync.ValidationManager.State.UNKNOWN; -import static org.briarproject.api.sync.ValidationManager.State.VALID; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -67,6 +66,7 @@ public class H2DatabaseTest extends BriarTestCase { private final File testDir = TestUtils.getTestDirectory(); private final GroupId groupId; + private final ClientId clientId; private final Group group; private final Author author; private final AuthorId localAuthorId; @@ -81,7 +81,7 @@ public class H2DatabaseTest extends BriarTestCase { public H2DatabaseTest() throws Exception { groupId = new GroupId(TestUtils.getRandomId()); - ClientId clientId = new ClientId(TestUtils.getRandomId()); + clientId = new ClientId(TestUtils.getRandomId()); byte[] descriptor = new byte[0]; group = new Group(groupId, clientId, descriptor); AuthorId authorId = new AuthorId(TestUtils.getRandomId()); @@ -239,16 +239,7 @@ public class H2DatabaseTest extends BriarTestCase { ids = db.getMessagesToOffer(txn, contactId, 100); assertTrue(ids.isEmpty()); - // Marking the message valid should make it unsendable - // TODO do we maybe want to already send valid messages? If we do, we need also to call db.setMessageShared() earlier. - db.setMessageState(txn, messageId, VALID); - ids = db.getMessagesToSend(txn, contactId, ONE_MEGABYTE); - assertTrue(ids.isEmpty()); - ids = db.getMessagesToOffer(txn, contactId, 100); - assertTrue(ids.isEmpty()); - // Marking the message pending should make it unsendable - // TODO do we maybe want to already send pending messages? If we do, we need also to call db.setMessageShared() earlier. db.setMessageState(txn, messageId, PENDING); ids = db.getMessagesToSend(txn, contactId, ONE_MEGABYTE); assertTrue(ids.isEmpty()); @@ -1019,13 +1010,6 @@ public class H2DatabaseTest extends BriarTestCase { map = db.getMessageMetadata(txn, groupId); assertTrue(map.isEmpty()); - // No metadata for valid messages - db.setMessageState(txn, messageId, VALID); - retrieved = db.getMessageMetadata(txn, messageId); - assertTrue(retrieved.isEmpty()); - map = db.getMessageMetadata(txn, groupId); - assertTrue(map.isEmpty()); - // No metadata for pending messages db.setMessageState(txn, messageId, PENDING); retrieved = db.getMessageMetadata(txn, messageId); @@ -1033,7 +1017,7 @@ public class H2DatabaseTest extends BriarTestCase { map = db.getMessageMetadata(txn, groupId); assertTrue(map.isEmpty()); - // validator gets also metadata for pending messages + // Validator can get metadata for pending messages retrieved = db.getMessageMetadataForValidator(txn, messageId); assertFalse(retrieved.isEmpty()); @@ -1198,12 +1182,6 @@ public class H2DatabaseTest extends BriarTestCase { all = db.getMessageMetadata(txn, groupId, query); assertTrue(all.isEmpty()); - // No metadata for valid messages - db.setMessageState(txn, messageId, VALID); - db.setMessageState(txn, messageId1, VALID); - all = db.getMessageMetadata(txn, groupId, query); - assertTrue(all.isEmpty()); - // No metadata for pending messages db.setMessageState(txn, messageId, PENDING); db.setMessageState(txn, messageId1, PENDING); @@ -1217,149 +1195,139 @@ public class H2DatabaseTest extends BriarTestCase { @Test public void testMessageDependencies() throws Exception { + MessageId messageId1 = new MessageId(TestUtils.getRandomId()); + MessageId messageId2 = new MessageId(TestUtils.getRandomId()); + MessageId messageId3 = new MessageId(TestUtils.getRandomId()); + MessageId messageId4 = new MessageId(TestUtils.getRandomId()); + Message message1 = new Message(messageId1, groupId, timestamp, raw); + Message message2 = new Message(messageId2, groupId, timestamp, raw); + Database<Connection> db = open(false); Connection txn = db.startTransaction(); - // Add a group and a message + // Add a group and some messages db.addGroup(txn, group); - db.addMessage(txn, message, VALID, true); - - // Create more messages - MessageId mId1 = new MessageId(TestUtils.getRandomId()); - MessageId mId2 = new MessageId(TestUtils.getRandomId()); - MessageId dId1 = new MessageId(TestUtils.getRandomId()); - MessageId dId2 = new MessageId(TestUtils.getRandomId()); - Message m1 = new Message(mId1, groupId, timestamp, raw); - Message m2 = new Message(mId2, groupId, timestamp, raw); - - // Add new messages - db.addMessage(txn, m1, VALID, true); - db.addMessage(txn, m2, INVALID, true); + db.addMessage(txn, message, PENDING, true); + db.addMessage(txn, message1, DELIVERED, true); + db.addMessage(txn, message2, INVALID, true); // Add dependencies - db.addMessageDependency(txn, messageId, mId1); - db.addMessageDependency(txn, messageId, mId2); - db.addMessageDependency(txn, mId1, dId1); - db.addMessageDependency(txn, mId2, dId2); + db.addMessageDependency(txn, groupId, messageId, messageId1); + db.addMessageDependency(txn, groupId, messageId, messageId2); + db.addMessageDependency(txn, groupId, messageId1, messageId3); + db.addMessageDependency(txn, groupId, messageId2, messageId4); Map<MessageId, State> dependencies; // Retrieve dependencies for root dependencies = db.getMessageDependencies(txn, messageId); assertEquals(2, dependencies.size()); - assertEquals(VALID, dependencies.get(mId1)); - assertEquals(INVALID, dependencies.get(mId2)); + assertEquals(DELIVERED, dependencies.get(messageId1)); + assertEquals(INVALID, dependencies.get(messageId2)); - // Retrieve dependencies for m1 - dependencies = db.getMessageDependencies(txn, mId1); + // Retrieve dependencies for message 1 + dependencies = db.getMessageDependencies(txn, messageId1); assertEquals(1, dependencies.size()); - assertEquals(UNKNOWN, dependencies.get(dId1)); + assertEquals(UNKNOWN, dependencies.get(messageId3)); // Missing - // Retrieve dependencies for m2 - dependencies = db.getMessageDependencies(txn, mId2); + // Retrieve dependencies for message 2 + dependencies = db.getMessageDependencies(txn, messageId2); assertEquals(1, dependencies.size()); - assertEquals(UNKNOWN, dependencies.get(dId2)); + assertEquals(UNKNOWN, dependencies.get(messageId4)); // Missing - // Make sure d's have no dependencies - dependencies = db.getMessageDependencies(txn, dId1); - assertTrue(dependencies.isEmpty()); - dependencies = db.getMessageDependencies(txn, dId2); - assertTrue(dependencies.isEmpty()); + // Make sure leaves have no dependencies + dependencies = db.getMessageDependencies(txn, messageId3); + assertEquals(0, dependencies.size()); + dependencies = db.getMessageDependencies(txn, messageId4); + assertEquals(0, dependencies.size()); Map<MessageId, State> dependents; // Root message does not have dependents dependents = db.getMessageDependents(txn, messageId); - assertTrue(dependents.isEmpty()); + assertEquals(0, dependents.size()); - // The root message depends on both m's - dependents = db.getMessageDependents(txn, mId1); + // Messages 1 and 2 have the root as a dependent + dependents = db.getMessageDependents(txn, messageId1); assertEquals(1, dependents.size()); - assertEquals(VALID, dependents.get(messageId)); - dependents = db.getMessageDependents(txn, mId2); + assertEquals(PENDING, dependents.get(messageId)); + dependents = db.getMessageDependents(txn, messageId2); assertEquals(1, dependents.size()); - assertEquals(VALID, dependents.get(messageId)); + assertEquals(PENDING, dependents.get(messageId)); - // Both m's depend on the d's - dependents = db.getMessageDependents(txn, dId1); + // Message 3 has message 1 as a dependent + dependents = db.getMessageDependents(txn, messageId3); assertEquals(1, dependents.size()); - assertEquals(VALID, dependents.get(mId1)); - dependents = db.getMessageDependents(txn, dId2); + assertEquals(DELIVERED, dependents.get(messageId1)); + + // Message 4 has message 2 as a dependent + dependents = db.getMessageDependents(txn, messageId4); assertEquals(1, dependents.size()); - assertEquals(INVALID, dependents.get(mId2)); + assertEquals(INVALID, dependents.get(messageId2)); db.commitTransaction(txn); db.close(); } @Test - public void testMessageDependenciesInSameGroup() throws Exception { + public void testMessageDependenciesAcrossGroups() throws Exception { Database<Connection> db = open(false); Connection txn = db.startTransaction(); // Add a group and a message db.addGroup(txn, group); - db.addMessage(txn, message, DELIVERED, true); + db.addMessage(txn, message, PENDING, true); // Add a second group GroupId groupId1 = new GroupId(TestUtils.getRandomId()); - Group group1 = new Group(groupId1, group.getClientId(), + Group group1 = new Group(groupId1, clientId, TestUtils.getRandomBytes(42)); db.addGroup(txn, group1); // Add a message to the second group - MessageId mId1 = new MessageId(TestUtils.getRandomId()); - Message m1 = new Message(mId1, groupId1, timestamp, raw); - db.addMessage(txn, m1, DELIVERED, true); + MessageId messageId1 = new MessageId(TestUtils.getRandomId()); + Message message1 = new Message(messageId1, groupId1, timestamp, raw); + db.addMessage(txn, message1, DELIVERED, true); - // Create a fake dependency as well - MessageId mId2 = new MessageId(TestUtils.getRandomId()); + // Create an ID for a missing message + MessageId messageId2 = new MessageId(TestUtils.getRandomId()); - // Create and add a real and proper dependency - MessageId mId3 = new MessageId(TestUtils.getRandomId()); - Message m3 = new Message(mId3, groupId, timestamp, raw); - db.addMessage(txn, m3, PENDING, true); + // Add another message to the first group + MessageId messageId3 = new MessageId(TestUtils.getRandomId()); + Message message3 = new Message(messageId3, groupId, timestamp, raw); + db.addMessage(txn, message3, DELIVERED, true); - // Add dependencies - db.addMessageDependency(txn, messageId, mId1); - db.addMessageDependency(txn, messageId, mId2); - db.addMessageDependency(txn, messageId, mId3); + // Add dependencies between the messages + db.addMessageDependency(txn, groupId, messageId, messageId1); + db.addMessageDependency(txn, groupId, messageId, messageId2); + db.addMessageDependency(txn, groupId, messageId, messageId3); - // Return invalid dependencies for delivered message m1 + // Retrieve the dependencies for the root Map<MessageId, State> dependencies; dependencies = db.getMessageDependencies(txn, messageId); - assertEquals(INVALID, dependencies.get(mId1)); - assertEquals(UNKNOWN, dependencies.get(mId2)); - assertEquals(PENDING, dependencies.get(mId3)); - // Return invalid dependencies for valid message m1 - db.setMessageState(txn, mId1, VALID); - dependencies = db.getMessageDependencies(txn, messageId); - assertEquals(INVALID, dependencies.get(mId1)); - assertEquals(UNKNOWN, dependencies.get(mId2)); - assertEquals(PENDING, dependencies.get(mId3)); + // The cross-group dependency should have state INVALID + assertEquals(INVALID, dependencies.get(messageId1)); - // Return invalid dependencies for pending message m1 - db.setMessageState(txn, mId1, PENDING); - dependencies = db.getMessageDependencies(txn, messageId); - assertEquals(INVALID, dependencies.get(mId1)); - assertEquals(UNKNOWN, dependencies.get(mId2)); - assertEquals(PENDING, dependencies.get(mId3)); + // The missing dependency should have state UNKNOWN + assertEquals(UNKNOWN, dependencies.get(messageId2)); + + // The valid dependency should have its real state + assertEquals(DELIVERED, dependencies.get(messageId3)); + + // Retrieve the dependents for the message in the second group + Map<MessageId, State> dependents; + dependents = db.getMessageDependents(txn, messageId1); + + // The cross-group dependent should have its real state + assertEquals(PENDING, dependents.get(messageId)); db.commitTransaction(txn); db.close(); } @Test - public void testGetMessagesForValidationAndDelivery() throws Exception { - Database<Connection> db = open(false); - Connection txn = db.startTransaction(); - - // Add a group and a message - db.addGroup(txn, group); - db.addMessage(txn, message, VALID, true); - - // Create more messages + public void testGetPendingMessagesForDelivery() throws Exception { MessageId mId1 = new MessageId(TestUtils.getRandomId()); MessageId mId2 = new MessageId(TestUtils.getRandomId()); MessageId mId3 = new MessageId(TestUtils.getRandomId()); @@ -1369,7 +1337,11 @@ public class H2DatabaseTest extends BriarTestCase { Message m3 = new Message(mId3, groupId, timestamp, raw); Message m4 = new Message(mId4, groupId, timestamp, raw); - // Add new messages with different states + Database<Connection> db = open(false); + Connection txn = db.startTransaction(); + + // Add a group and some messages with different states + db.addGroup(txn, group); db.addMessage(txn, m1, UNKNOWN, true); db.addMessage(txn, m2, INVALID, true); db.addMessage(txn, m3, PENDING, true); @@ -1378,17 +1350,12 @@ public class H2DatabaseTest extends BriarTestCase { Collection<MessageId> result; // Retrieve messages to be validated - result = db.getMessagesToValidate(txn, group.getClientId()); + result = db.getMessagesToValidate(txn, clientId); assertEquals(1, result.size()); assertTrue(result.contains(mId1)); - // Retrieve messages to be delivered - result = db.getMessagesToDeliver(txn, group.getClientId()); - assertEquals(1, result.size()); - assertTrue(result.contains(messageId)); - // Retrieve pending messages - result = db.getPendingMessages(txn, group.getClientId()); + result = db.getPendingMessages(txn, clientId); assertEquals(1, result.size()); assertTrue(result.contains(mId3)); @@ -1607,6 +1574,29 @@ public class H2DatabaseTest extends BriarTestCase { db.close(); } + @Test + public void testSetMessageState() throws Exception { + + Database<Connection> db = open(false); + Connection txn = db.startTransaction(); + + // Add a group and a message + db.addGroup(txn, group); + db.addMessage(txn, message, UNKNOWN, false); + + // Walk the message through the validation and delivery states + assertEquals(UNKNOWN, db.getMessageState(txn, messageId)); + db.setMessageState(txn, messageId, INVALID); + assertEquals(INVALID, db.getMessageState(txn, messageId)); + db.setMessageState(txn, messageId, PENDING); + assertEquals(PENDING, db.getMessageState(txn, messageId)); + db.setMessageState(txn, messageId, DELIVERED); + assertEquals(DELIVERED, db.getMessageState(txn, messageId)); + + db.commitTransaction(txn); + db.close(); + } + @Test public void testExceptionHandling() throws Exception { Database<Connection> db = open(false); diff --git a/briar-tests/src/org/briarproject/introduction/IntroduceeManagerTest.java b/briar-tests/src/org/briarproject/introduction/IntroduceeManagerTest.java index 0a91bd6864..a7ee386a5d 100644 --- a/briar-tests/src/org/briarproject/introduction/IntroduceeManagerTest.java +++ b/briar-tests/src/org/briarproject/introduction/IntroduceeManagerTest.java @@ -279,8 +279,7 @@ public class IntroduceeManagerTest extends BriarTestCase { // store session state oneOf(clientHelper) - .addLocalMessage(txn, localStateMessage, clientId, state, - false); + .addLocalMessage(txn, localStateMessage, state, false); }}); BdfDictionary result = introduceeManager.initialize(txn, groupId, msg); diff --git a/briar-tests/src/org/briarproject/introduction/IntroducerManagerTest.java b/briar-tests/src/org/briarproject/introduction/IntroducerManagerTest.java index 9dd4f913cc..caa0aeb84f 100644 --- a/briar-tests/src/org/briarproject/introduction/IntroducerManagerTest.java +++ b/briar-tests/src/org/briarproject/introduction/IntroducerManagerTest.java @@ -167,8 +167,7 @@ public class IntroducerManagerTest extends BriarTestCase { oneOf(introductionGroupFactory) .createIntroductionGroup(introducee2); will(returnValue(introductionGroup2)); - oneOf(clientHelper).addLocalMessage(txn, msg, getClientId(), state, - false); + oneOf(clientHelper).addLocalMessage(txn, msg, state, false); // send message oneOf(clientHelper).mergeMessageMetadata(txn, msg.getId(), state2); diff --git a/briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java b/briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java index a46a027c77..e7c2e85006 100644 --- a/briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java +++ b/briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java @@ -26,11 +26,9 @@ import org.jmock.Expectations; import org.jmock.Mockery; import org.junit.Test; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.concurrent.Executor; @@ -38,7 +36,7 @@ import static org.briarproject.api.sync.ValidationManager.State.DELIVERED; import static org.briarproject.api.sync.ValidationManager.State.INVALID; import static org.briarproject.api.sync.ValidationManager.State.PENDING; import static org.briarproject.api.sync.ValidationManager.State.UNKNOWN; -import static org.briarproject.api.sync.ValidationManager.State.VALID; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class ValidationManagerImplTest extends BriarTestCase { @@ -58,21 +56,17 @@ public class ValidationManagerImplTest extends BriarTestCase { raw); private final Message message2 = new Message(messageId2, groupId, timestamp, raw); + private final Metadata metadata = new Metadata(); private final MessageContext validResult = new MessageContext(metadata); private final ContactId contactId = new ContactId(234); - private final Collection<MessageId> dependencies = new ArrayList<>(); private final MessageContext validResultWithDependencies = - new MessageContext(metadata, dependencies); - private final Map<MessageId, State> states = new HashMap<>(); + new MessageContext(metadata, Collections.singletonList(messageId1)); public ValidationManagerImplTest() { // Encode the messages System.arraycopy(groupId.getBytes(), 0, raw, 0, UniqueId.LENGTH); ByteUtils.writeUint64(timestamp, raw, UniqueId.LENGTH); - - dependencies.add(messageId1); - states.put(messageId1, INVALID); } @Test @@ -84,11 +78,10 @@ public class ValidationManagerImplTest extends BriarTestCase { final MessageValidator validator = context.mock(MessageValidator.class); final IncomingMessageHook hook = context.mock(IncomingMessageHook.class); - final Transaction txn = new Transaction(null, false); - final Transaction txn1 = new Transaction(null, false); + final Transaction txn = new Transaction(null, true); + final Transaction txn1 = new Transaction(null, true); final Transaction txn2 = new Transaction(null, false); - final Transaction txn2b = new Transaction(null, false); - final Transaction txn3 = new Transaction(null, false); + final Transaction txn3 = new Transaction(null, true); final Transaction txn4 = new Transaction(null, false); final Transaction txn5 = new Transaction(null, true); context.checking(new Expectations() {{ @@ -112,21 +105,16 @@ public class ValidationManagerImplTest extends BriarTestCase { // Store the validation result for the first message oneOf(db).startTransaction(false); will(returnValue(txn2)); - oneOf(db).getMessageDependencies(txn2, messageId); oneOf(db).mergeMessageMetadata(txn2, messageId, metadata); - oneOf(db).setMessageState(txn2, message, clientId, VALID); - oneOf(db).endTransaction(txn2); - // Async delivery - oneOf(db).startTransaction(false); - will(returnValue(txn2b)); - // Call the hook for the first message - oneOf(hook).incomingMessage(txn2b, message, metadata); - oneOf(db).getRawMessage(txn2b, messageId); + // Deliver the first message + oneOf(hook).incomingMessage(txn2, message, metadata); + oneOf(db).getRawMessage(txn2, messageId); will(returnValue(raw)); - oneOf(db).setMessageState(txn2b, message, clientId, DELIVERED); - oneOf(db).getMessageDependents(txn2b, messageId); + oneOf(db).setMessageState(txn2, messageId, DELIVERED); + // Get any pending dependents + oneOf(db).getMessageDependents(txn2, messageId); will(returnValue(Collections.emptyMap())); - oneOf(db).endTransaction(txn2b); + oneOf(db).endTransaction(txn2); // Load the second raw message and group oneOf(db).startTransaction(true); will(returnValue(txn3)); @@ -141,101 +129,20 @@ public class ValidationManagerImplTest extends BriarTestCase { // Store the validation result for the second message oneOf(db).startTransaction(false); will(returnValue(txn4)); - oneOf(db).setMessageState(txn4, message1, clientId, INVALID); - // Recursively invalidate dependents - oneOf(db).getMessageDependents(txn4, messageId1); + oneOf(db).getMessageState(txn4, messageId1); + will(returnValue(UNKNOWN)); + oneOf(db).setMessageState(txn4, messageId1, INVALID); oneOf(db).deleteMessage(txn4, messageId1); oneOf(db).deleteMessageMetadata(txn4, messageId1); + // Recursively invalidate any dependents + oneOf(db).getMessageDependents(txn4, messageId1); + will(returnValue(Collections.emptyMap())); oneOf(db).endTransaction(txn4); - // Get other messages to deliver + // Get pending messages to deliver oneOf(db).startTransaction(true); will(returnValue(txn5)); - oneOf(db).getMessagesToDeliver(txn5, clientId); oneOf(db).getPendingMessages(txn5, clientId); - oneOf(db).endTransaction(txn5); - }}); - - ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor, - cryptoExecutor); - vm.registerMessageValidator(clientId, validator); - vm.registerIncomingMessageHook(clientId, hook); - vm.startService(); - - context.assertIsSatisfied(); - } - - @Test - public void testMessagesAreDeliveredAtStartup() throws Exception { - Mockery context = new Mockery(); - final DatabaseComponent db = context.mock(DatabaseComponent.class); - final Executor dbExecutor = new ImmediateExecutor(); - final Executor cryptoExecutor = new ImmediateExecutor(); - final MessageValidator validator = context.mock(MessageValidator.class); - final IncomingMessageHook hook = - context.mock(IncomingMessageHook.class); - final Transaction txn = new Transaction(null, true); - final Transaction txn1 = new Transaction(null, true); - final Transaction txn2 = new Transaction(null, true); - final Transaction txn3 = new Transaction(null, false); - final Transaction txn4 = new Transaction(null, true); - final Transaction txn5 = new Transaction(null, false); - - states.put(messageId1, PENDING); - - context.checking(new Expectations() {{ - // Get messages to validate - oneOf(db).startTransaction(true); - will(returnValue(txn)); - oneOf(db).getMessagesToValidate(txn, clientId); - oneOf(db).endTransaction(txn); - // Get IDs of messages to deliver - oneOf(db).startTransaction(true); - will(returnValue(txn1)); - oneOf(db).getMessagesToDeliver(txn1, clientId); - will(returnValue(Collections.singletonList(messageId))); - oneOf(db).getPendingMessages(txn1, clientId); - oneOf(db).endTransaction(txn1); - // Get message and its metadata to deliver - oneOf(db).startTransaction(true); - will(returnValue(txn2)); - oneOf(db).getRawMessage(txn2, messageId); - will(returnValue(message.getRaw())); - oneOf(db).getGroup(txn2, message.getGroupId()); - will(returnValue(group)); - oneOf(db).getMessageMetadataForValidator(txn2, messageId); - will(returnValue(metadata)); - oneOf(db).endTransaction(txn2); - // Deliver message in a new transaction - oneOf(db).startTransaction(false); - will(returnValue(txn3)); - oneOf(db).setMessageState(txn3, message, clientId, DELIVERED); - oneOf(hook).incomingMessage(txn3, message, metadata); - oneOf(db).getRawMessage(txn3, messageId); - will(returnValue(message.getRaw())); - // Try to also deliver pending dependents - oneOf(db).getMessageDependents(txn3, messageId); - will(returnValue(states)); - oneOf(db).getMessageDependencies(txn3, messageId1); - will(returnValue(Collections.singletonMap(messageId2, DELIVERED))); - oneOf(db).endTransaction(txn3); - // Get the dependent to deliver - oneOf(db).startTransaction(true); - will(returnValue(txn4)); - oneOf(db).getRawMessage(txn4, messageId1); - will(returnValue(message1.getRaw())); - oneOf(db).getGroup(txn4, message.getGroupId()); - will(returnValue(group)); - oneOf(db).getMessageMetadataForValidator(txn4, messageId1); - will(returnValue(metadata)); - oneOf(db).endTransaction(txn4); - // Deliver the dependent in a new transaction - oneOf(db).startTransaction(false); - will(returnValue(txn5)); - oneOf(db).setMessageState(txn5, message1, clientId, DELIVERED); - oneOf(hook).incomingMessage(txn5, message1, metadata); - oneOf(db).getRawMessage(txn5, messageId1); - will(returnValue(message1.getRaw())); - oneOf(db).getMessageDependents(txn5, messageId1); + will(returnValue(Collections.emptyList())); oneOf(db).endTransaction(txn5); }}); @@ -266,7 +173,7 @@ public class ValidationManagerImplTest extends BriarTestCase { context.mock(IncomingMessageHook.class); final Transaction txn = new Transaction(null, true); final Transaction txn1 = new Transaction(null, true); - final Transaction txn2 = new Transaction(null, true); + final Transaction txn2 = new Transaction(null, false); final Transaction txn3 = new Transaction(null, false); context.checking(new Expectations() {{ @@ -274,33 +181,59 @@ public class ValidationManagerImplTest extends BriarTestCase { oneOf(db).startTransaction(true); will(returnValue(txn)); oneOf(db).getMessagesToValidate(txn, clientId); + will(returnValue(Collections.emptyList())); oneOf(db).endTransaction(txn); - // Get IDs of messages to deliver + // Get pending messages to deliver oneOf(db).startTransaction(true); will(returnValue(txn1)); - oneOf(db).getMessagesToDeliver(txn1, clientId); oneOf(db).getPendingMessages(txn1, clientId); will(returnValue(Collections.singletonList(messageId))); oneOf(db).endTransaction(txn1); - // Get message and its metadata to deliver - oneOf(db).startTransaction(true); + // Check whether the message is ready to deliver + oneOf(db).startTransaction(false); will(returnValue(txn2)); - oneOf(db).getRawMessage(txn2, messageId); - will(returnValue(message.getRaw())); - oneOf(db).getGroup(txn2, message.getGroupId()); - will(returnValue(group)); + oneOf(db).getMessageState(txn2, messageId); + will(returnValue(PENDING)); oneOf(db).getMessageDependencies(txn2, messageId); will(returnValue(Collections.singletonMap(messageId1, DELIVERED))); + // Get the message and its metadata to deliver + oneOf(db).getRawMessage(txn2, messageId); + will(returnValue(raw)); + oneOf(db).getGroup(txn2, groupId); + will(returnValue(group)); oneOf(db).getMessageMetadataForValidator(txn2, messageId); + will(returnValue(new Metadata())); + // Deliver the message + oneOf(hook).incomingMessage(txn2, message, metadata); + oneOf(db).getRawMessage(txn2, messageId); + will(returnValue(raw)); + oneOf(db).setMessageState(txn2, messageId, DELIVERED); + // Get any pending dependents + oneOf(db).getMessageDependents(txn2, messageId); + will(returnValue(Collections.singletonMap(messageId2, PENDING))); oneOf(db).endTransaction(txn2); - // Deliver the pending message + // Check whether the dependent is ready to deliver oneOf(db).startTransaction(false); will(returnValue(txn3)); - oneOf(db).setMessageState(txn3, message, clientId, DELIVERED); - oneOf(hook).incomingMessage(txn3, message, metadata); - oneOf(db).getRawMessage(txn3, messageId); - will(returnValue(message.getRaw())); - oneOf(db).getMessageDependents(txn3, messageId); + oneOf(db).getMessageState(txn3, messageId2); + will(returnValue(PENDING)); + oneOf(db).getMessageDependencies(txn3, messageId2); + will(returnValue(Collections.singletonMap(messageId1, DELIVERED))); + // Get the dependent and its metadata to deliver + oneOf(db).getRawMessage(txn3, messageId2); + will(returnValue(raw)); + oneOf(db).getGroup(txn3, groupId); + will(returnValue(group)); + oneOf(db).getMessageMetadataForValidator(txn3, messageId2); + will(returnValue(metadata)); + // Deliver the dependent + oneOf(hook).incomingMessage(txn3, message2, metadata); + oneOf(db).getRawMessage(txn3, messageId2); + will(returnValue(raw)); + oneOf(db).setMessageState(txn3, messageId2, DELIVERED); + // Get any pending dependents + oneOf(db).getMessageDependents(txn3, messageId2); + will(returnValue(Collections.emptyMap())); oneOf(db).endTransaction(txn3); }}); @@ -357,20 +290,23 @@ public class ValidationManagerImplTest extends BriarTestCase { // Validate the second message: invalid oneOf(validator).validateMessage(message1, group); will(throwException(new InvalidMessageException())); - // Store the validation result for the second message + // Invalidate the second message oneOf(db).startTransaction(false); will(returnValue(txn3)); - oneOf(db).setMessageState(txn3, message1, clientId, INVALID); - // recursively invalidate dependents - oneOf(db).getMessageDependents(txn3, messageId1); + oneOf(db).getMessageState(txn3, messageId1); + will(returnValue(UNKNOWN)); + oneOf(db).setMessageState(txn3, messageId1, INVALID); oneOf(db).deleteMessage(txn3, messageId1); oneOf(db).deleteMessageMetadata(txn3, messageId1); + // Recursively invalidate dependents + oneOf(db).getMessageDependents(txn3, messageId1); + will(returnValue(Collections.emptyMap())); oneOf(db).endTransaction(txn3); - // Get other messages to deliver + // Get pending messages to deliver oneOf(db).startTransaction(true); will(returnValue(txn4)); - oneOf(db).getMessagesToDeliver(txn4, clientId); oneOf(db).getPendingMessages(txn4, clientId); + will(returnValue(Collections.emptyList())); oneOf(db).endTransaction(txn4); }}); @@ -383,7 +319,7 @@ public class ValidationManagerImplTest extends BriarTestCase { context.assertIsSatisfied(); assertTrue(txn.isComplete()); - assertTrue(txn1.isComplete()); + assertFalse(txn1.isComplete()); // Aborted due to NoSuchMessageException assertTrue(txn2.isComplete()); assertTrue(txn3.isComplete()); assertTrue(txn4.isComplete()); @@ -434,17 +370,20 @@ public class ValidationManagerImplTest extends BriarTestCase { // Store the validation result for the second message oneOf(db).startTransaction(false); will(returnValue(txn3)); - oneOf(db).setMessageState(txn3, message1, clientId, INVALID); - // recursively invalidate dependents - oneOf(db).getMessageDependents(txn3, messageId1); + oneOf(db).getMessageState(txn3, messageId1); + will(returnValue(UNKNOWN)); + oneOf(db).setMessageState(txn3, messageId1, INVALID); oneOf(db).deleteMessage(txn3, messageId1); oneOf(db).deleteMessageMetadata(txn3, messageId1); + // Recursively invalidate dependents + oneOf(db).getMessageDependents(txn3, messageId1); + will(returnValue(Collections.emptyMap())); oneOf(db).endTransaction(txn3); - // Get other messages to deliver + // Get pending messages to deliver oneOf(db).startTransaction(true); will(returnValue(txn4)); - oneOf(db).getMessagesToDeliver(txn4, clientId); oneOf(db).getPendingMessages(txn4, clientId); + will(returnValue(Collections.emptyList())); oneOf(db).endTransaction(txn4); }}); @@ -457,7 +396,7 @@ public class ValidationManagerImplTest extends BriarTestCase { context.assertIsSatisfied(); assertTrue(txn.isComplete()); - assertTrue(txn1.isComplete()); + assertFalse(txn1.isComplete()); // Aborted due to NoSuchGroupException assertTrue(txn2.isComplete()); assertTrue(txn3.isComplete()); assertTrue(txn4.isComplete()); @@ -474,7 +413,6 @@ public class ValidationManagerImplTest extends BriarTestCase { context.mock(IncomingMessageHook.class); final Transaction txn = new Transaction(null, true); final Transaction txn1 = new Transaction(null, false); - final Transaction txn2 = new Transaction(null, false); context.checking(new Expectations() {{ // Load the group oneOf(db).startTransaction(true); @@ -488,20 +426,16 @@ public class ValidationManagerImplTest extends BriarTestCase { // Store the validation result oneOf(db).startTransaction(false); will(returnValue(txn1)); - oneOf(db).getMessageDependencies(txn1, messageId); oneOf(db).mergeMessageMetadata(txn1, messageId, metadata); - oneOf(db).setMessageState(txn1, message, clientId, VALID); - oneOf(db).endTransaction(txn1); - // async delivery - oneOf(db).startTransaction(false); - will(returnValue(txn2)); - // Call the hook - oneOf(hook).incomingMessage(txn2, message, metadata); - oneOf(db).getRawMessage(txn2, messageId); + // Deliver the message + oneOf(hook).incomingMessage(txn1, message, metadata); + oneOf(db).getRawMessage(txn1, messageId); will(returnValue(raw)); - oneOf(db).setMessageState(txn2, message, clientId, DELIVERED); - oneOf(db).getMessageDependents(txn2, messageId); - oneOf(db).endTransaction(txn2); + oneOf(db).setMessageState(txn1, messageId, DELIVERED); + // Get any pending dependents + oneOf(db).getMessageDependents(txn1, messageId); + will(returnValue(Collections.emptyMap())); + oneOf(db).endTransaction(txn1); }}); ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor, @@ -514,7 +448,6 @@ public class ValidationManagerImplTest extends BriarTestCase { assertTrue(txn.isComplete()); assertTrue(txn1.isComplete()); - assertTrue(txn2.isComplete()); } @Test @@ -537,10 +470,9 @@ public class ValidationManagerImplTest extends BriarTestCase { } @Test - public void testMessagesWithNonDeliveredDependenciesArePending() + public void testMessagesWithUndeliveredDependenciesArePending() throws Exception { - states.put(messageId1, UNKNOWN); Mockery context = new Mockery(); final DatabaseComponent db = context.mock(DatabaseComponent.class); final Executor dbExecutor = new ImmediateExecutor(); @@ -566,9 +498,9 @@ public class ValidationManagerImplTest extends BriarTestCase { oneOf(db).addMessageDependencies(txn1, message, validResultWithDependencies.getDependencies()); oneOf(db).getMessageDependencies(txn1, messageId); - will(returnValue(states)); + will(returnValue(Collections.singletonMap(messageId1, UNKNOWN))); oneOf(db).mergeMessageMetadata(txn1, messageId, metadata); - oneOf(db).setMessageState(txn1, message, clientId, PENDING); + oneOf(db).setMessageState(txn1, messageId, PENDING); oneOf(db).endTransaction(txn1); }}); @@ -587,8 +519,6 @@ public class ValidationManagerImplTest extends BriarTestCase { @Test public void testMessagesWithDeliveredDependenciesGetDelivered() throws Exception { - - states.put(messageId1, DELIVERED); Mockery context = new Mockery(); final DatabaseComponent db = context.mock(DatabaseComponent.class); final Executor dbExecutor = new ImmediateExecutor(); @@ -598,7 +528,6 @@ public class ValidationManagerImplTest extends BriarTestCase { context.mock(IncomingMessageHook.class); final Transaction txn = new Transaction(null, true); final Transaction txn1 = new Transaction(null, false); - final Transaction txn2 = new Transaction(null, false); context.checking(new Expectations() {{ // Load the group oneOf(db).startTransaction(true); @@ -615,20 +544,17 @@ public class ValidationManagerImplTest extends BriarTestCase { oneOf(db).addMessageDependencies(txn1, message, validResultWithDependencies.getDependencies()); oneOf(db).getMessageDependencies(txn1, messageId); - will(returnValue(states)); + will(returnValue(Collections.singletonMap(messageId1, DELIVERED))); oneOf(db).mergeMessageMetadata(txn1, messageId, metadata); - oneOf(db).setMessageState(txn1, message, clientId, VALID); - oneOf(db).endTransaction(txn1); - // async delivery - oneOf(db).startTransaction(false); - will(returnValue(txn2)); - // Call the hook - oneOf(hook).incomingMessage(txn2, message, metadata); - oneOf(db).getRawMessage(txn2, messageId); + // Deliver the message + oneOf(hook).incomingMessage(txn1, message, metadata); + oneOf(db).getRawMessage(txn1, messageId); will(returnValue(raw)); - oneOf(db).setMessageState(txn2, message, clientId, DELIVERED); - oneOf(db).getMessageDependents(txn2, messageId); - oneOf(db).endTransaction(txn2); + oneOf(db).setMessageState(txn1, messageId, DELIVERED); + // Get any pending dependents + oneOf(db).getMessageDependents(txn1, messageId); + will(returnValue(Collections.emptyMap())); + oneOf(db).endTransaction(txn1); }}); ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor, @@ -641,7 +567,6 @@ public class ValidationManagerImplTest extends BriarTestCase { assertTrue(txn.isComplete()); assertTrue(txn1.isComplete()); - assertTrue(txn2.isComplete()); } @Test @@ -657,8 +582,6 @@ public class ValidationManagerImplTest extends BriarTestCase { final Transaction txn = new Transaction(null, true); final Transaction txn1 = new Transaction(null, false); final Transaction txn2 = new Transaction(null, false); - final Transaction txn3 = new Transaction(null, true); - final Transaction txn4 = new Transaction(null, false); context.checking(new Expectations() {{ // Load the group oneOf(db).startTransaction(true); @@ -674,35 +597,142 @@ public class ValidationManagerImplTest extends BriarTestCase { will(returnValue(txn1)); oneOf(db).addMessageDependencies(txn1, message, validResultWithDependencies.getDependencies()); + // Check for invalid dependencies oneOf(db).getMessageDependencies(txn1, messageId); - will(returnValue(states)); + will(returnValue(Collections.singletonMap(messageId1, INVALID))); + // Invalidate message + oneOf(db).getMessageState(txn1, messageId); + will(returnValue(UNKNOWN)); + oneOf(db).setMessageState(txn1, messageId, INVALID); + oneOf(db).deleteMessage(txn1, messageId); + oneOf(db).deleteMessageMetadata(txn1, messageId); + // Recursively invalidate dependents + oneOf(db).getMessageDependents(txn1, messageId); + will(returnValue(Collections.singletonMap(messageId2, UNKNOWN))); oneOf(db).endTransaction(txn1); - // Invalidate message in a new transaction + // Invalidate dependent in a new transaction oneOf(db).startTransaction(false); will(returnValue(txn2)); - oneOf(db).getMessageDependents(txn2, messageId); - will(returnValue(Collections.singletonMap(messageId2, UNKNOWN))); - oneOf(db).setMessageState(txn2, message, clientId, INVALID); - oneOf(db).deleteMessage(txn2, messageId); - oneOf(db).deleteMessageMetadata(txn2, messageId); + oneOf(db).getMessageState(txn2, messageId2); + will(returnValue(UNKNOWN)); + oneOf(db).setMessageState(txn2, messageId2, INVALID); + oneOf(db).deleteMessage(txn2, messageId2); + oneOf(db).deleteMessageMetadata(txn2, messageId2); + oneOf(db).getMessageDependents(txn2, messageId2); + will(returnValue(Collections.emptyMap())); oneOf(db).endTransaction(txn2); - // Get message to invalidate in a new transaction + }}); + + ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor, + cryptoExecutor); + vm.registerMessageValidator(clientId, validator); + vm.registerIncomingMessageHook(clientId, hook); + vm.eventOccurred(new MessageAddedEvent(message, contactId)); + + context.assertIsSatisfied(); + + assertTrue(txn.isComplete()); + assertTrue(txn1.isComplete()); + assertTrue(txn2.isComplete()); + } + + @Test + public void testRecursiveInvalidation() throws Exception { + Mockery context = new Mockery(); + final DatabaseComponent db = context.mock(DatabaseComponent.class); + final Executor dbExecutor = new ImmediateExecutor(); + final Executor cryptoExecutor = new ImmediateExecutor(); + final MessageValidator validator = context.mock(MessageValidator.class); + final IncomingMessageHook hook = + context.mock(IncomingMessageHook.class); + final MessageId messageId3 = new MessageId(TestUtils.getRandomId()); + final MessageId messageId4 = new MessageId(TestUtils.getRandomId()); + final Map<MessageId, State> twoDependents = new LinkedHashMap<>(); + twoDependents.put(messageId1, PENDING); + twoDependents.put(messageId2, PENDING); + final Transaction txn = new Transaction(null, true); + final Transaction txn1 = new Transaction(null, false); + final Transaction txn2 = new Transaction(null, false); + final Transaction txn3 = new Transaction(null, false); + final Transaction txn4 = new Transaction(null, false); + final Transaction txn5 = new Transaction(null, false); + final Transaction txn6 = new Transaction(null, false); + context.checking(new Expectations() {{ + // Load the group oneOf(db).startTransaction(true); - will(returnValue(txn3)); - oneOf(db).getRawMessage(txn3, messageId2); - will(returnValue(message2.getRaw())); - oneOf(db).getGroup(txn3, message2.getGroupId()); + will(returnValue(txn)); + oneOf(db).getGroup(txn, groupId); will(returnValue(group)); + oneOf(db).endTransaction(txn); + // Validate the message: invalid + oneOf(validator).validateMessage(message, group); + will(throwException(new InvalidMessageException())); + // Invalidate the message + oneOf(db).startTransaction(false); + will(returnValue(txn1)); + oneOf(db).getMessageState(txn1, messageId); + will(returnValue(UNKNOWN)); + oneOf(db).setMessageState(txn1, messageId, INVALID); + oneOf(db).deleteMessage(txn1, messageId); + oneOf(db).deleteMessageMetadata(txn1, messageId); + // The message has two dependents: 1 and 2 + oneOf(db).getMessageDependents(txn1, messageId); + will(returnValue(twoDependents)); + oneOf(db).endTransaction(txn1); + // Invalidate message 1 + oneOf(db).startTransaction(false); + will(returnValue(txn2)); + oneOf(db).getMessageState(txn2, messageId1); + will(returnValue(PENDING)); + oneOf(db).setMessageState(txn2, messageId1, INVALID); + oneOf(db).deleteMessage(txn2, messageId1); + oneOf(db).deleteMessageMetadata(txn2, messageId1); + // Message 1 has one dependent: 3 + oneOf(db).getMessageDependents(txn2, messageId1); + will(returnValue(Collections.singletonMap(messageId3, PENDING))); + oneOf(db).endTransaction(txn2); + // Invalidate message 2 + oneOf(db).startTransaction(false); + will(returnValue(txn3)); + oneOf(db).getMessageState(txn3, messageId2); + will(returnValue(PENDING)); + oneOf(db).setMessageState(txn3, messageId2, INVALID); + oneOf(db).deleteMessage(txn3, messageId2); + oneOf(db).deleteMessageMetadata(txn3, messageId2); + // Message 2 has one dependent: 3 (same dependent as 1) + oneOf(db).getMessageDependents(txn3, messageId2); + will(returnValue(Collections.singletonMap(messageId3, PENDING))); oneOf(db).endTransaction(txn3); - // Invalidate dependent message in a new transaction + // Invalidate message 3 (via 1) oneOf(db).startTransaction(false); will(returnValue(txn4)); - oneOf(db).getMessageDependents(txn4, messageId2); - will(returnValue(Collections.emptyMap())); - oneOf(db).setMessageState(txn4, message2, clientId, INVALID); - oneOf(db).deleteMessage(txn4, messageId2); - oneOf(db).deleteMessageMetadata(txn4, messageId2); + oneOf(db).getMessageState(txn4, messageId3); + will(returnValue(PENDING)); + oneOf(db).setMessageState(txn4, messageId3, INVALID); + oneOf(db).deleteMessage(txn4, messageId3); + oneOf(db).deleteMessageMetadata(txn4, messageId3); + // Message 3 has one dependent: 4 + oneOf(db).getMessageDependents(txn4, messageId3); + will(returnValue(Collections.singletonMap(messageId4, PENDING))); oneOf(db).endTransaction(txn4); + // Invalidate message 3 (again, via 2) + oneOf(db).startTransaction(false); + will(returnValue(txn5)); + oneOf(db).getMessageState(txn5, messageId3); + will(returnValue(INVALID)); // Already invalidated + oneOf(db).endTransaction(txn5); + // Invalidate message 4 (via 1 and 3) + oneOf(db).startTransaction(false); + will(returnValue(txn6)); + oneOf(db).getMessageState(txn6, messageId4); + will(returnValue(PENDING)); + oneOf(db).setMessageState(txn6, messageId4, INVALID); + oneOf(db).deleteMessage(txn6, messageId4); + oneOf(db).deleteMessageMetadata(txn6, messageId4); + // Message 4 has no dependents + oneOf(db).getMessageDependents(txn6, messageId4); + will(returnValue(Collections.emptyMap())); + oneOf(db).endTransaction(txn6); }}); ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor, @@ -716,8 +746,6 @@ public class ValidationManagerImplTest extends BriarTestCase { assertTrue(txn.isComplete()); assertTrue(txn1.isComplete()); assertTrue(txn2.isComplete()); - assertTrue(txn3.isComplete()); - assertTrue(txn4.isComplete()); } @Test @@ -729,11 +757,25 @@ public class ValidationManagerImplTest extends BriarTestCase { final MessageValidator validator = context.mock(MessageValidator.class); final IncomingMessageHook hook = context.mock(IncomingMessageHook.class); + final MessageId messageId3 = new MessageId(TestUtils.getRandomId()); + final MessageId messageId4 = new MessageId(TestUtils.getRandomId()); + final Message message3 = new Message(messageId3, groupId, timestamp, + raw); + final Message message4 = new Message(messageId4, groupId, timestamp, + raw); + final Map<MessageId, State> twoDependents = new LinkedHashMap<>(); + twoDependents.put(messageId1, PENDING); + twoDependents.put(messageId2, PENDING); + final Map<MessageId, State> twoDependencies = new LinkedHashMap<>(); + twoDependencies.put(messageId1, DELIVERED); + twoDependencies.put(messageId2, DELIVERED); final Transaction txn = new Transaction(null, true); final Transaction txn1 = new Transaction(null, false); final Transaction txn2 = new Transaction(null, false); - final Transaction txn3 = new Transaction(null, true); + final Transaction txn3 = new Transaction(null, false); final Transaction txn4 = new Transaction(null, false); + final Transaction txn5 = new Transaction(null, false); + final Transaction txn6 = new Transaction(null, false); context.checking(new Expectations() {{ // Load the group oneOf(db).startTransaction(true); @@ -747,43 +789,114 @@ public class ValidationManagerImplTest extends BriarTestCase { // Store the validation result oneOf(db).startTransaction(false); will(returnValue(txn1)); - oneOf(db).getMessageDependencies(txn1, messageId); - will(returnValue(Collections.emptyMap())); oneOf(db).mergeMessageMetadata(txn1, messageId, metadata); - oneOf(db).setMessageState(txn1, message, clientId, VALID); + // Deliver the message + oneOf(hook).incomingMessage(txn1, message, metadata); + oneOf(db).getRawMessage(txn1, messageId); + will(returnValue(raw)); + oneOf(db).setMessageState(txn1, messageId, DELIVERED); + // The message has two pending dependents: 1 and 2 + oneOf(db).getMessageDependents(txn1, messageId); + will(returnValue(twoDependents)); oneOf(db).endTransaction(txn1); - // Deliver first message + // Check whether message 1 is ready to be delivered oneOf(db).startTransaction(false); will(returnValue(txn2)); - oneOf(hook).incomingMessage(txn2, message, metadata); - oneOf(db).getRawMessage(txn2, messageId); - will(returnValue(raw)); - oneOf(db).setMessageState(txn2, message, clientId, DELIVERED); - oneOf(db).getMessageDependents(txn2, messageId); - will(returnValue(Collections.singletonMap(messageId1, PENDING))); + oneOf(db).getMessageState(txn2, messageId1); + will(returnValue(PENDING)); oneOf(db).getMessageDependencies(txn2, messageId1); - will(returnValue(Collections.singletonMap(messageId2, DELIVERED))); + will(returnValue(Collections.singletonMap(messageId, DELIVERED))); + // Get message 1 and its metadata + oneOf(db).getRawMessage(txn2, messageId1); + will(returnValue(raw)); + oneOf(db).getGroup(txn2, groupId); + will(returnValue(group)); + oneOf(db).getMessageMetadataForValidator(txn2, messageId1); + will(returnValue(metadata)); + // Deliver message 1 + oneOf(hook).incomingMessage(txn2, message1, metadata); + oneOf(db).getRawMessage(txn2, messageId1); + will(returnValue(raw)); + oneOf(db).setMessageState(txn2, messageId1, DELIVERED); + // Message 1 has one pending dependent: 3 + oneOf(db).getMessageDependents(txn2, messageId1); + will(returnValue(Collections.singletonMap(messageId3, PENDING))); oneOf(db).endTransaction(txn2); - // Also get the pending message for delivery - oneOf(db).startTransaction(true); + // Check whether message 2 is ready to be delivered + oneOf(db).startTransaction(false); will(returnValue(txn3)); - oneOf(db).getRawMessage(txn3, messageId1); - will(returnValue(message1.getRaw())); - oneOf(db).getGroup(txn3, message1.getGroupId()); + oneOf(db).getMessageState(txn3, messageId2); + will(returnValue(PENDING)); + oneOf(db).getMessageDependencies(txn3, messageId2); + will(returnValue(Collections.singletonMap(messageId, DELIVERED))); + // Get message 2 and its metadata + oneOf(db).getRawMessage(txn3, messageId2); + will(returnValue(raw)); + oneOf(db).getGroup(txn3, groupId); will(returnValue(group)); - oneOf(db).getMessageMetadataForValidator(txn3, messageId1); + oneOf(db).getMessageMetadataForValidator(txn3, messageId2); will(returnValue(metadata)); + // Deliver message 2 + oneOf(hook).incomingMessage(txn3, message2, metadata); + oneOf(db).getRawMessage(txn3, messageId2); + will(returnValue(raw)); + oneOf(db).setMessageState(txn3, messageId2, DELIVERED); + // Message 2 has one pending dependent: 3 (same dependent as 1) + oneOf(db).getMessageDependents(txn3, messageId2); + will(returnValue(Collections.singletonMap(messageId3, PENDING))); oneOf(db).endTransaction(txn3); - // Deliver the pending message + // Check whether message 3 is ready to be delivered (via 1) oneOf(db).startTransaction(false); will(returnValue(txn4)); - oneOf(hook).incomingMessage(txn4, message1, metadata); - oneOf(db).getRawMessage(txn4, messageId1); + oneOf(db).getMessageState(txn4, messageId3); + will(returnValue(PENDING)); + oneOf(db).getMessageDependencies(txn4, messageId3); + will(returnValue(twoDependencies)); + // Get message 3 and its metadata + oneOf(db).getRawMessage(txn4, messageId3); will(returnValue(raw)); - oneOf(db).setMessageState(txn4, message1, clientId, DELIVERED); - oneOf(db).getMessageDependents(txn4, messageId1); - will(returnValue(Collections.emptyMap())); + oneOf(db).getGroup(txn4, groupId); + will(returnValue(group)); + oneOf(db).getMessageMetadataForValidator(txn4, messageId3); + will(returnValue(metadata)); + // Deliver message 3 + oneOf(hook).incomingMessage(txn4, message3, metadata); + oneOf(db).getRawMessage(txn4, messageId3); + will(returnValue(raw)); + oneOf(db).setMessageState(txn4, messageId3, DELIVERED); + // Message 3 has one pending dependent: 4 + oneOf(db).getMessageDependents(txn4, messageId3); + will(returnValue(Collections.singletonMap(messageId4, PENDING))); oneOf(db).endTransaction(txn4); + // Check whether message 3 is ready to be delivered (again, via 2) + oneOf(db).startTransaction(false); + will(returnValue(txn5)); + oneOf(db).getMessageState(txn5, messageId3); + will(returnValue(DELIVERED)); // Already delivered + oneOf(db).endTransaction(txn5); + // Check whether message 4 is ready to be delivered (via 1 and 3) + oneOf(db).startTransaction(false); + will(returnValue(txn6)); + oneOf(db).getMessageState(txn6, messageId4); + will(returnValue(PENDING)); + oneOf(db).getMessageDependencies(txn6, messageId4); + will(returnValue(Collections.singletonMap(messageId3, DELIVERED))); + // Get message 4 and its metadata + oneOf(db).getRawMessage(txn6, messageId4); + will(returnValue(raw)); + oneOf(db).getGroup(txn6, groupId); + will(returnValue(group)); + oneOf(db).getMessageMetadataForValidator(txn6, messageId4); + will(returnValue(metadata)); + // Deliver message 4 + oneOf(hook).incomingMessage(txn6, message4, metadata); + oneOf(db).getRawMessage(txn6, messageId4); + will(returnValue(raw)); + oneOf(db).setMessageState(txn6, messageId4, DELIVERED); + // Message 4 has no pending dependents + oneOf(db).getMessageDependents(txn6, messageId4); + will(returnValue(Collections.emptyMap())); + oneOf(db).endTransaction(txn6); }}); ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor, @@ -799,6 +912,8 @@ public class ValidationManagerImplTest extends BriarTestCase { assertTrue(txn2.isComplete()); assertTrue(txn3.isComplete()); assertTrue(txn4.isComplete()); + assertTrue(txn5.isComplete()); + assertTrue(txn6.isComplete()); } @Test @@ -810,6 +925,9 @@ public class ValidationManagerImplTest extends BriarTestCase { final MessageValidator validator = context.mock(MessageValidator.class); final IncomingMessageHook hook = context.mock(IncomingMessageHook.class); + final Map<MessageId, State> twoDependencies = new LinkedHashMap<>(); + twoDependencies.put(messageId, DELIVERED); + twoDependencies.put(messageId2, UNKNOWN); final Transaction txn = new Transaction(null, true); final Transaction txn1 = new Transaction(null, false); final Transaction txn2 = new Transaction(null, false); @@ -826,22 +944,23 @@ public class ValidationManagerImplTest extends BriarTestCase { // Store the validation result oneOf(db).startTransaction(false); will(returnValue(txn1)); - oneOf(db).getMessageDependencies(txn1, messageId); - will(returnValue(Collections.emptyMap())); oneOf(db).mergeMessageMetadata(txn1, messageId, metadata); - oneOf(db).setMessageState(txn1, message, clientId, VALID); + // Deliver the message + oneOf(hook).incomingMessage(txn1, message, metadata); + oneOf(db).getRawMessage(txn1, messageId); + will(returnValue(raw)); + oneOf(db).setMessageState(txn1, messageId, DELIVERED); + // Get any pending dependents + oneOf(db).getMessageDependents(txn1, messageId); + will(returnValue(Collections.singletonMap(messageId1, PENDING))); oneOf(db).endTransaction(txn1); - // Deliver first message + // Check whether the pending dependent is ready to be delivered oneOf(db).startTransaction(false); will(returnValue(txn2)); - oneOf(hook).incomingMessage(txn2, message, metadata); - oneOf(db).getRawMessage(txn2, messageId); - will(returnValue(raw)); - oneOf(db).setMessageState(txn2, message, clientId, DELIVERED); - oneOf(db).getMessageDependents(txn2, messageId); - will(returnValue(Collections.singletonMap(messageId1, PENDING))); + oneOf(db).getMessageState(txn2, messageId1); + will(returnValue(PENDING)); oneOf(db).getMessageDependencies(txn2, messageId1); - will(returnValue(Collections.singletonMap(messageId2, VALID))); + will(returnValue(twoDependencies)); oneOf(db).endTransaction(txn2); }}); @@ -860,7 +979,6 @@ public class ValidationManagerImplTest extends BriarTestCase { @Test public void testMessageDependencyCycle() throws Exception { - states.put(messageId1, UNKNOWN); final MessageContext cycleContext = new MessageContext(metadata, Collections.singletonList(messageId)); @@ -891,9 +1009,9 @@ public class ValidationManagerImplTest extends BriarTestCase { oneOf(db).addMessageDependencies(txn1, message, validResultWithDependencies.getDependencies()); oneOf(db).getMessageDependencies(txn1, messageId); - will(returnValue(states)); + will(returnValue(Collections.singletonMap(messageId1, UNKNOWN))); oneOf(db).mergeMessageMetadata(txn1, messageId, metadata); - oneOf(db).setMessageState(txn1, message, clientId, PENDING); + oneOf(db).setMessageState(txn1, messageId, PENDING); oneOf(db).endTransaction(txn1); // Second message is coming in oneOf(db).startTransaction(true); @@ -912,7 +1030,7 @@ public class ValidationManagerImplTest extends BriarTestCase { oneOf(db).getMessageDependencies(txn3, messageId1); will(returnValue(Collections.singletonMap(messageId, PENDING))); oneOf(db).mergeMessageMetadata(txn3, messageId1, metadata); - oneOf(db).setMessageState(txn3, message1, clientId, PENDING); + oneOf(db).setMessageState(txn3, messageId1, PENDING); oneOf(db).endTransaction(txn3); }}); -- GitLab