From d381e25e86b457eb9b185ab2e1714a1f2da4ce09 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Wed, 5 Apr 2017 14:03:40 +0100
Subject: [PATCH] Limit the number of validation tasks on the crypto executor.

---
 .../briarproject/bramble/sync/SyncModule.java |  23 +-
 .../bramble/sync/ValidationExecutor.java      |  25 ++
 .../bramble/sync/ValidationManagerImpl.java   |  42 ++-
 .../sync/ValidationManagerImplTest.java       | 357 ++++--------------
 4 files changed, 140 insertions(+), 307 deletions(-)
 create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/sync/ValidationExecutor.java

diff --git a/bramble-core/src/main/java/org/briarproject/bramble/sync/SyncModule.java b/bramble-core/src/main/java/org/briarproject/bramble/sync/SyncModule.java
index 09aaa199b1..d4cc88881f 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/sync/SyncModule.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/sync/SyncModule.java
@@ -1,6 +1,8 @@
 package org.briarproject.bramble.sync;
 
+import org.briarproject.bramble.PoliteExecutor;
 import org.briarproject.bramble.api.crypto.CryptoComponent;
+import org.briarproject.bramble.api.crypto.CryptoExecutor;
 import org.briarproject.bramble.api.db.DatabaseComponent;
 import org.briarproject.bramble.api.db.DatabaseExecutor;
 import org.briarproject.bramble.api.event.EventBus;
@@ -29,6 +31,13 @@ public class SyncModule {
 		ValidationManager validationManager;
 	}
 
+	/**
+	 * The maximum number of validation tasks to delegate to the crypto
+	 * executor concurrently.
+	 */
+	private static final int MAX_CONCURRENT_VALIDATION_TASKS =
+			Runtime.getRuntime().availableProcessors();
+
 	@Provides
 	GroupFactory provideGroupFactory(CryptoComponent crypto) {
 		return new GroupFactoryImpl(crypto);
@@ -62,10 +71,20 @@ public class SyncModule {
 
 	@Provides
 	@Singleton
-	ValidationManager getValidationManager(LifecycleManager lifecycleManager,
-			EventBus eventBus, ValidationManagerImpl validationManager) {
+	ValidationManager provideValidationManager(
+			LifecycleManager lifecycleManager, EventBus eventBus,
+			ValidationManagerImpl validationManager) {
 		lifecycleManager.registerService(validationManager);
 		eventBus.addListener(validationManager);
 		return validationManager;
 	}
+
+	@Provides
+	@Singleton
+	@ValidationExecutor
+	Executor provideValidationExecutor(
+			@CryptoExecutor Executor cryptoExecutor) {
+		return new PoliteExecutor("ValidationExecutor", cryptoExecutor,
+				MAX_CONCURRENT_VALIDATION_TASKS);
+	}
 }
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/sync/ValidationExecutor.java b/bramble-core/src/main/java/org/briarproject/bramble/sync/ValidationExecutor.java
new file mode 100644
index 0000000000..41cda1d35a
--- /dev/null
+++ b/bramble-core/src/main/java/org/briarproject/bramble/sync/ValidationExecutor.java
@@ -0,0 +1,25 @@
+package org.briarproject.bramble.sync;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import javax.inject.Qualifier;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.PARAMETER;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+/**
+ * Annotation for injecting the executor for validation tasks. Also used for
+ * annotating methods that should run on the validation executor.
+ * <p>
+ * The contract of this executor is that tasks may be run concurrently, and
+ * submitting a task will never block. Tasks must not run indefinitely. Tasks
+ * submitted during shutdown are discarded.
+ */
+@Qualifier
+@Target({FIELD, METHOD, PARAMETER})
+@Retention(RUNTIME)
+@interface ValidationExecutor {
+}
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/sync/ValidationManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/sync/ValidationManagerImpl.java
index 7e7aa865fb..8888908640 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/sync/ValidationManagerImpl.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/sync/ValidationManagerImpl.java
@@ -1,6 +1,5 @@
 package org.briarproject.bramble.sync;
 
-import org.briarproject.bramble.api.crypto.CryptoExecutor;
 import org.briarproject.bramble.api.db.DatabaseComponent;
 import org.briarproject.bramble.api.db.DatabaseExecutor;
 import org.briarproject.bramble.api.db.DbException;
@@ -50,8 +49,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 			Logger.getLogger(ValidationManagerImpl.class.getName());
 
 	private final DatabaseComponent db;
-	private final Executor dbExecutor;
-	private final Executor cryptoExecutor;
+	private final Executor dbExecutor, validationExecutor;
 	private final MessageFactory messageFactory;
 	private final Map<ClientId, MessageValidator> validators;
 	private final Map<ClientId, IncomingMessageHook> hooks;
@@ -60,11 +58,11 @@ class ValidationManagerImpl implements ValidationManager, Service,
 	@Inject
 	ValidationManagerImpl(DatabaseComponent db,
 			@DatabaseExecutor Executor dbExecutor,
-			@CryptoExecutor Executor cryptoExecutor,
+			@ValidationExecutor Executor validationExecutor,
 			MessageFactory messageFactory) {
 		this.db = db;
 		this.dbExecutor = dbExecutor;
-		this.cryptoExecutor = cryptoExecutor;
+		this.validationExecutor = validationExecutor;
 		this.messageFactory = messageFactory;
 		validators = new ConcurrentHashMap<ClientId, MessageValidator>();
 		hooks = new ConcurrentHashMap<ClientId, IncomingMessageHook>();
@@ -104,6 +102,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		});
 	}
 
+	@DatabaseExecutor
 	private void validateOutstandingMessages(ClientId c) {
 		try {
 			Queue<MessageId> unvalidated = new LinkedList<MessageId>();
@@ -130,6 +129,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		});
 	}
 
+	@DatabaseExecutor
 	private void validateNextMessage(Queue<MessageId> unvalidated) {
 		try {
 			Message m;
@@ -167,6 +167,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		});
 	}
 
+	@DatabaseExecutor
 	private void deliverOutstandingMessages(ClientId c) {
 		try {
 			Queue<MessageId> pending = new LinkedList<MessageId>();
@@ -194,6 +195,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		});
 	}
 
+	@DatabaseExecutor
 	private void deliverNextPendingMessage(Queue<MessageId> pending) {
 		try {
 			boolean anyInvalid = false, allDelivered = true;
@@ -220,8 +222,8 @@ class ValidationManagerImpl implements ValidationManager, Service,
 						Message m = messageFactory.createMessage(id, raw);
 						Group g = db.getGroup(txn, m.getGroupId());
 						ClientId c = g.getClientId();
-						Metadata meta = db.getMessageMetadataForValidator(txn,
-								id);
+						Metadata meta =
+								db.getMessageMetadataForValidator(txn, id);
 						DeliveryResult result = deliverMessage(txn, m, c, meta);
 						if (result.valid) {
 							pending.addAll(getPendingDependents(txn, id));
@@ -240,8 +242,8 @@ class ValidationManagerImpl implements ValidationManager, Service,
 				db.endTransaction(txn);
 			}
 			if (invalidate != null) invalidateNextMessageAsync(invalidate);
-			deliverNextPendingMessageAsync(pending);
 			if (toShare != null) shareNextMessageAsync(toShare);
+			deliverNextPendingMessageAsync(pending);
 		} catch (NoSuchMessageException e) {
 			LOG.info("Message removed before delivery");
 			deliverNextPendingMessageAsync(pending);
@@ -249,13 +251,12 @@ class ValidationManagerImpl implements ValidationManager, Service,
 			LOG.info("Group removed before delivery");
 			deliverNextPendingMessageAsync(pending);
 		} catch (DbException e) {
-			if (LOG.isLoggable(WARNING))
-				LOG.log(WARNING, e.toString(), e);
+			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 		}
 	}
 
 	private void validateMessageAsync(final Message m, final Group g) {
-		cryptoExecutor.execute(new Runnable() {
+		validationExecutor.execute(new Runnable() {
 			@Override
 			public void run() {
 				validateMessage(m, g);
@@ -263,10 +264,12 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		});
 	}
 
+	@ValidationExecutor
 	private void validateMessage(Message m, Group g) {
 		MessageValidator v = validators.get(g.getClientId());
 		if (v == null) {
-			LOG.warning("No validator");
+			if (LOG.isLoggable(WARNING))
+				LOG.warning("No validator for " + g.getClientId().getString());
 		} else {
 			try {
 				MessageContext context = v.validateMessage(m, g);
@@ -291,6 +294,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		});
 	}
 
+	@DatabaseExecutor
 	private void storeMessageContext(Message m, ClientId c,
 			MessageContext context) {
 		try {
@@ -353,6 +357,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		}
 	}
 
+	@DatabaseExecutor
 	private DeliveryResult deliverMessage(Transaction txn, Message m,
 			ClientId c, Metadata meta) throws DbException {
 		// Deliver the message to the client if it's registered a hook
@@ -362,10 +367,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 			try {
 				shareMsg = hook.incomingMessage(txn, m, meta);
 			} catch (InvalidMessageException e) {
-				// message is invalid, mark it as such and delete it
-				db.setMessageState(txn, m.getId(), INVALID);
-				db.deleteMessageMetadata(txn, m.getId());
-				db.deleteMessage(txn, m.getId());
+				invalidateMessage(txn, m.getId());
 				return new DeliveryResult(false, false);
 			}
 		}
@@ -373,6 +375,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		return new DeliveryResult(true, shareMsg);
 	}
 
+	@DatabaseExecutor
 	private Queue<MessageId> getPendingDependents(Transaction txn, MessageId m)
 			throws DbException {
 		Queue<MessageId> pending = new LinkedList<MessageId>();
@@ -392,6 +395,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		});
 	}
 
+	@DatabaseExecutor
 	private void shareOutstandingMessages(ClientId c) {
 		try {
 			Queue<MessageId> toShare = new LinkedList<MessageId>();
@@ -424,6 +428,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		});
 	}
 
+	@DatabaseExecutor
 	private void shareNextMessage(Queue<MessageId> toShare) {
 		try {
 			Transaction txn = db.startTransaction(false);
@@ -457,6 +462,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		});
 	}
 
+	@DatabaseExecutor
 	private void invalidateNextMessage(Queue<MessageId> invalidate) {
 		try {
 			Transaction txn = db.startTransaction(false);
@@ -479,6 +485,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		}
 	}
 
+	@DatabaseExecutor
 	private void invalidateMessage(Transaction txn, MessageId m)
 			throws DbException {
 		db.setMessageState(txn, m, INVALID);
@@ -486,6 +493,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		db.deleteMessageMetadata(txn, m);
 	}
 
+	@DatabaseExecutor
 	private Queue<MessageId> getDependentsToInvalidate(Transaction txn,
 			MessageId m) throws DbException {
 		Queue<MessageId> invalidate = new LinkedList<MessageId>();
@@ -515,6 +523,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 		});
 	}
 
+	@DatabaseExecutor
 	private void loadGroupAndValidate(final Message m) {
 		try {
 			Group g;
@@ -534,6 +543,7 @@ class ValidationManagerImpl implements ValidationManager, Service,
 	}
 
 	private static class DeliveryResult {
+
 		private final boolean valid, share;
 
 		private DeliveryResult(boolean valid, boolean share) {
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/sync/ValidationManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/sync/ValidationManagerImplTest.java
index b99c2f0627..ed6c361de4 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/sync/ValidationManagerImplTest.java
+++ b/bramble-core/src/test/java/org/briarproject/bramble/sync/ValidationManagerImplTest.java
@@ -19,12 +19,12 @@ import org.briarproject.bramble.api.sync.ValidationManager.IncomingMessageHook;
 import org.briarproject.bramble.api.sync.ValidationManager.MessageValidator;
 import org.briarproject.bramble.api.sync.ValidationManager.State;
 import org.briarproject.bramble.api.sync.event.MessageAddedEvent;
-import org.briarproject.bramble.test.BrambleTestCase;
+import org.briarproject.bramble.test.BrambleMockTestCase;
 import org.briarproject.bramble.test.ImmediateExecutor;
 import org.briarproject.bramble.test.TestUtils;
 import org.briarproject.bramble.util.ByteUtils;
 import org.jmock.Expectations;
-import org.jmock.Mockery;
+import org.junit.Before;
 import org.junit.Test;
 
 import java.util.Arrays;
@@ -38,8 +38,18 @@ import static org.briarproject.bramble.api.sync.ValidationManager.State.INVALID;
 import static org.briarproject.bramble.api.sync.ValidationManager.State.PENDING;
 import static org.briarproject.bramble.api.sync.ValidationManager.State.UNKNOWN;
 
-public class ValidationManagerImplTest extends BrambleTestCase {
+public class ValidationManagerImplTest extends BrambleMockTestCase {
 
+	private final DatabaseComponent db = context.mock(DatabaseComponent.class);
+	private final MessageFactory messageFactory =
+			context.mock(MessageFactory.class);
+	private final MessageValidator validator =
+			context.mock(MessageValidator.class);
+	private final IncomingMessageHook hook =
+			context.mock(IncomingMessageHook.class);
+
+	private final Executor dbExecutor = new ImmediateExecutor();
+	private final Executor validationExecutor = new ImmediateExecutor();
 	private final ClientId clientId =
 			new ClientId(TestUtils.getRandomString(5));
 	private final MessageId messageId = new MessageId(TestUtils.getRandomId());
@@ -63,23 +73,58 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 	private final MessageContext validResultWithDependencies =
 			new MessageContext(metadata, Collections.singletonList(messageId1));
 
+	private ValidationManagerImpl vm;
+
 	public ValidationManagerImplTest() {
 		// Encode the messages
 		System.arraycopy(groupId.getBytes(), 0, raw, 0, UniqueId.LENGTH);
 		ByteUtils.writeUint64(timestamp, raw, UniqueId.LENGTH);
 	}
 
+	@Before
+	public void setUp() {
+		vm = new ValidationManagerImpl(db, dbExecutor, validationExecutor,
+				messageFactory);
+		vm.registerMessageValidator(clientId, validator);
+		vm.registerIncomingMessageHook(clientId, hook);
+	}
+
+	@Test
+	public void testStartAndStop() throws Exception {
+		final Transaction txn = new Transaction(null, true);
+		final Transaction txn1 = new Transaction(null, true);
+		final Transaction txn2 = new Transaction(null, true);
+
+		context.checking(new Expectations() {{
+			// validateOutstandingMessages()
+			oneOf(db).startTransaction(true);
+			will(returnValue(txn));
+			oneOf(db).getMessagesToValidate(txn, clientId);
+			will(returnValue(Collections.emptyList()));
+			oneOf(db).commitTransaction(txn);
+			oneOf(db).endTransaction(txn);
+			// deliverOutstandingMessages()
+			oneOf(db).startTransaction(true);
+			will(returnValue(txn1));
+			oneOf(db).getPendingMessages(txn1, clientId);
+			will(returnValue(Collections.emptyList()));
+			oneOf(db).commitTransaction(txn1);
+			oneOf(db).endTransaction(txn1);
+			// shareOutstandingMessages()
+			oneOf(db).startTransaction(true);
+			will(returnValue(txn2));
+			oneOf(db).getMessagesToShare(txn2, clientId);
+			will(returnValue(Collections.emptyList()));
+			oneOf(db).commitTransaction(txn2);
+			oneOf(db).endTransaction(txn2);
+		}});
+
+		vm.startService();
+		vm.stopService();
+	}
+
 	@Test
 	public void testMessagesAreValidatedAtStartup() throws Exception {
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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, false);
@@ -87,6 +132,7 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 		final Transaction txn4 = new Transaction(null, false);
 		final Transaction txn5 = new Transaction(null, true);
 		final Transaction txn6 = new Transaction(null, true);
+
 		context.checking(new Expectations() {{
 			// Get messages to validate
 			oneOf(db).startTransaction(true);
@@ -165,26 +211,11 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn6);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.startService();
-
-		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testPendingMessagesAreDeliveredAtStartup() throws Exception {
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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, false);
@@ -266,26 +297,11 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn4);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.startService();
-
-		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testMessagesAreSharedAtStartup() throws Exception {
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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);
@@ -333,29 +349,15 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn4);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.startService();
-
-		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testIncomingMessagesAreShared() throws Exception {
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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, false);
 		final Transaction txn2 = new Transaction(null, false);
+
 		context.checking(new Expectations() {{
 			// Load the group
 			oneOf(db).startTransaction(true);
@@ -396,33 +398,19 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn2);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.eventOccurred(new MessageAddedEvent(message, contactId));
-
-		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testValidationContinuesAfterNoSuchMessageException()
 			throws Exception {
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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, true);
+
 		context.checking(new Expectations() {{
 			// Get messages to validate
 			oneOf(db).startTransaction(true);
@@ -481,33 +469,19 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn5);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.startService();
-
-		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testValidationContinuesAfterNoSuchGroupException()
 			throws Exception {
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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, true);
+
 		context.checking(new Expectations() {{
 			// Get messages to validate
 			oneOf(db).startTransaction(true);
@@ -571,28 +545,14 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn5);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.startService();
-
-		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testNonLocalMessagesAreValidatedWhenAdded() throws Exception {
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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, false);
+
 		context.checking(new Expectations() {{
 			// Load the group
 			oneOf(db).startTransaction(true);
@@ -619,51 +579,20 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn1);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.eventOccurred(new MessageAddedEvent(message, contactId));
-
-		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testLocalMessagesAreNotValidatedWhenAdded() throws Exception {
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		final MessageValidator validator = context.mock(MessageValidator.class);
-		final IncomingMessageHook hook =
-				context.mock(IncomingMessageHook.class);
-
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.eventOccurred(new MessageAddedEvent(message, null));
-
-		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testMessagesWithUndeliveredDependenciesArePending()
 			throws Exception {
-
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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, false);
+
 		context.checking(new Expectations() {{
 			// Load the group
 			oneOf(db).startTransaction(true);
@@ -688,29 +617,15 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn1);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.eventOccurred(new MessageAddedEvent(message, contactId));
-
-		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testMessagesWithDeliveredDependenciesGetDelivered()
 			throws Exception {
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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, false);
+
 		context.checking(new Expectations() {{
 			// Load the group
 			oneOf(db).startTransaction(true);
@@ -741,30 +656,16 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn1);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.eventOccurred(new MessageAddedEvent(message, contactId));
-
-		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testMessagesWithInvalidDependenciesAreInvalid()
 			throws Exception {
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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, false);
 		final Transaction txn2 = new Transaction(null, false);
+
 		context.checking(new Expectations() {{
 			// Load the group
 			oneOf(db).startTransaction(true);
@@ -809,26 +710,11 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn2);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.eventOccurred(new MessageAddedEvent(message, contactId));
-
-		context.assertIsSatisfied();
 	}
 
 	@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 MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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 =
@@ -842,6 +728,7 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 		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);
@@ -927,26 +814,11 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn6);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.eventOccurred(new MessageAddedEvent(message, contactId));
-
-		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testPendingDependentsGetDelivered() throws Exception {
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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,
@@ -968,6 +840,7 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 		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);
@@ -1100,26 +973,11 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn6);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.eventOccurred(new MessageAddedEvent(message, contactId));
-
-		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testOnlyReadyPendingDependentsGetDelivered() throws Exception {
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		final MessageValidator validator = context.mock(MessageValidator.class);
-		final IncomingMessageHook hook =
-				context.mock(IncomingMessageHook.class);
 		final Map<MessageId, State> twoDependencies =
 				new LinkedHashMap<MessageId, State>();
 		twoDependencies.put(messageId, DELIVERED);
@@ -1127,6 +985,7 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 		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);
@@ -1162,86 +1021,6 @@ public class ValidationManagerImplTest extends BrambleTestCase {
 			oneOf(db).endTransaction(txn2);
 		}});
 
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
-		vm.eventOccurred(new MessageAddedEvent(message, contactId));
-
-		context.assertIsSatisfied();
-	}
-
-	@Test
-	public void testMessageDependencyCycle() throws Exception {
-		final MessageContext cycleContext = new MessageContext(metadata,
-				Collections.singletonList(messageId));
-
-		Mockery context = new Mockery();
-		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Executor dbExecutor = new ImmediateExecutor();
-		final Executor cryptoExecutor = new ImmediateExecutor();
-		final MessageFactory messageFactory =
-				context.mock(MessageFactory.class);
-		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, false);
-		final Transaction txn2 = new Transaction(null, true);
-		final Transaction txn3 = new Transaction(null, false);
-		context.checking(new Expectations() {{
-			// Load the group
-			oneOf(db).startTransaction(true);
-			will(returnValue(txn));
-			oneOf(db).getGroup(txn, groupId);
-			will(returnValue(group));
-			oneOf(db).commitTransaction(txn);
-			oneOf(db).endTransaction(txn);
-			// Validate the message: valid
-			oneOf(validator).validateMessage(message, group);
-			will(returnValue(validResultWithDependencies));
-			// Store the validation result
-			oneOf(db).startTransaction(false);
-			will(returnValue(txn1));
-			oneOf(db).addMessageDependencies(txn1, message,
-					validResultWithDependencies.getDependencies());
-			oneOf(db).getMessageDependencies(txn1, messageId);
-			will(returnValue(Collections.singletonMap(messageId1, UNKNOWN)));
-			oneOf(db).mergeMessageMetadata(txn1, messageId, metadata);
-			oneOf(db).setMessageState(txn1, messageId, PENDING);
-			oneOf(db).commitTransaction(txn1);
-			oneOf(db).endTransaction(txn1);
-			// Second message is coming in
-			oneOf(db).startTransaction(true);
-			will(returnValue(txn2));
-			oneOf(db).getGroup(txn2, groupId);
-			will(returnValue(group));
-			oneOf(db).commitTransaction(txn2);
-			oneOf(db).endTransaction(txn2);
-			// Validate the message: valid
-			oneOf(validator).validateMessage(message1, group);
-			will(returnValue(cycleContext));
-			// Store the validation result
-			oneOf(db).startTransaction(false);
-			will(returnValue(txn3));
-			oneOf(db).addMessageDependencies(txn3, message1,
-					cycleContext.getDependencies());
-			oneOf(db).getMessageDependencies(txn3, messageId1);
-			will(returnValue(Collections.singletonMap(messageId, PENDING)));
-			oneOf(db).mergeMessageMetadata(txn3, messageId1, metadata);
-			oneOf(db).setMessageState(txn3, messageId1, PENDING);
-			oneOf(db).commitTransaction(txn3);
-			oneOf(db).endTransaction(txn3);
-		}});
-
-		ValidationManagerImpl vm = new ValidationManagerImpl(db, dbExecutor,
-				cryptoExecutor, messageFactory);
-		vm.registerMessageValidator(clientId, validator);
-		vm.registerIncomingMessageHook(clientId, hook);
 		vm.eventOccurred(new MessageAddedEvent(message, contactId));
-		vm.eventOccurred(new MessageAddedEvent(message1, contactId));
-
-		context.assertIsSatisfied();
 	}
-
 }
-- 
GitLab