DatabaseComponent throws an exception instead of returning silently if a contact is removed during an operation involving that contact. More unit tests.

api/net/sf/briar/api/db/DatabaseComponent.java

@@ -21,7 +21,6 @@ public interface DatabaseComponent {
 	static final long MAX_BYTES_BETWEEN_SPACE_CHECKS = 5L * MEGABYTES;
 	static final long MAX_MS_BETWEEN_SPACE_CHECKS = 60L * 1000L; // 1 min
 	static final long BYTES_PER_SWEEP = 5L * MEGABYTES;
-	static final int RETRANSMIT_THRESHOLD = 3;
 
 	/**
 	 * Opens the database.

api/net/sf/briar/api/db/DbException.java

@@ -4,6 +4,10 @@ public class DbException extends Exception {
 
 	private static final long serialVersionUID = 3706581789209939441L;
 
+	public DbException() {
+		super();
+	}
+
 	public DbException(Throwable t) {
 		super(t);
 	}

api/net/sf/briar/api/db/NoSuchContactException.java

@@ -0,0 +1,14 @@
+package net.sf.briar.api.db;
+
+/**
+ * Thrown when a database operation is attempted for a contact that is not in
+ * the database.
+ */
+public class NoSuchContactException extends DbException {
+
+	private static final long serialVersionUID = -7048538231308207386L;
+
+	public NoSuchContactException() {
+		super();
+	}
+}

components/net/sf/briar/db/Database.java

@@ -32,6 +32,16 @@ import net.sf.briar.api.protocol.MessageId;
  */
 interface Database<T> {
 
+	/**
+	 * A batch sent to a contact is considered lost when this many bundles have
+	 * been received from the contact since the batch was sent.
+	 * <p>
+	 * FIXME: Come up with a better retransmission scheme. This scheme doesn't
+	 * cope well with transports that have high latency but send bundles
+	 * frequently.
+	 */
+	static final int RETRANSMIT_THRESHOLD = 3;
+
 	/**
 	 * Opens the database.
 	 * @param resume True to reopen an existing database, false to create a

components/net/sf/briar/db/JdbcDatabase.java

@@ -18,7 +18,6 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import net.sf.briar.api.db.ContactId;
-import net.sf.briar.api.db.DatabaseComponent;
 import net.sf.briar.api.db.DbException;
 import net.sf.briar.api.db.Rating;
 import net.sf.briar.api.db.Status;
@@ -527,7 +526,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 			rs.close();
 			ps.close();
 			Set<BatchId> lost;
-			if(received == DatabaseComponent.RETRANSMIT_THRESHOLD) {
+			if(received == RETRANSMIT_THRESHOLD) {
 				// Expire batches related to the oldest received bundle
 				assert oldestBundle != null;
 				lost = findLostBatches(txn, c, oldestBundle);

components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java

@@ -7,8 +7,9 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import net.sf.briar.api.db.DbException;
 import net.sf.briar.api.db.ContactId;
+import net.sf.briar.api.db.DbException;
+import net.sf.briar.api.db.NoSuchContactException;
 import net.sf.briar.api.db.Rating;
 import net.sf.briar.api.protocol.AuthorId;
 import net.sf.briar.api.protocol.Batch;
@@ -120,7 +121,10 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 							// unsubscribed from the group
 							if(db.containsSubscription(txn, m.getGroup())) {
 								boolean added = storeMessage(txn, m, null);
-								assert added;
+								if(!added) {
+									if(LOG.isLoggable(Level.FINE))
+										LOG.fine("Duplicate local message");
+								}
 							} else {
 								if(LOG.isLoggable(Level.FINE))
 									LOG.fine("Not subscribed");
@@ -177,7 +181,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 		// Ack all batches received from c
 		contactLock.readLock().lock();
 		try {
-			if(!containsContact(c)) return;
+			if(!containsContact(c)) throw new NoSuchContactException();
 			messageStatusLock.writeLock().lock();
 			try {
 				Txn txn = db.startTransaction();
@@ -203,7 +207,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 		// Add a list of subscriptions
 		contactLock.readLock().lock();
 		try {
-			if(!containsContact(c)) return;
+			if(!containsContact(c)) throw new NoSuchContactException();
 			subscriptionLock.readLock().lock();
 			try {
 				Txn txn = db.startTransaction();
@@ -246,7 +250,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 	private Batch fillBatch(ContactId c, long capacity) throws DbException {
 		contactLock.readLock().lock();
 		try {
-			if(!containsContact(c)) return null;
+			if(!containsContact(c)) throw new NoSuchContactException();
 			messageLock.readLock().lock();
 			try {
 				Set<MessageId> sent;
@@ -344,7 +348,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 		// Mark all messages in acked batches as seen
 		contactLock.readLock().lock();
 		try {
-			if(!containsContact(c)) return;
+			if(!containsContact(c)) throw new NoSuchContactException();
 			messageLock.readLock().lock();
 			try {
 				messageStatusLock.writeLock().lock();
@@ -375,7 +379,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 		// Update the contact's subscriptions
 		contactLock.readLock().lock();
 		try {
-			if(!containsContact(c)) return;
+			if(!containsContact(c)) throw new NoSuchContactException();
 			messageStatusLock.writeLock().lock();
 			try {
 				Txn txn = db.startTransaction();
@@ -406,7 +410,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 			waitForPermissionToWrite();
 			contactLock.readLock().lock();
 			try {
-				if(!containsContact(c)) return;
+				if(!containsContact(c)) throw new NoSuchContactException();
 				messageLock.writeLock().lock();
 				try {
 					messageStatusLock.writeLock().lock();
@@ -451,7 +455,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 		Set<BatchId> lost;
 		contactLock.readLock().lock();
 		try {
-			if(!containsContact(c)) return;
+			if(!containsContact(c)) throw new NoSuchContactException();
 			messageLock.readLock().lock();
 			try {
 				messageStatusLock.writeLock().lock();
@@ -476,7 +480,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 		for(BatchId batch : lost) {
 			contactLock.readLock().lock();
 			try {
-				if(!containsContact(c)) return;
+				if(!containsContact(c)) throw new NoSuchContactException();
 				messageLock.readLock().lock();
 				try {
 					messageStatusLock.writeLock().lock();

components/net/sf/briar/db/SynchronizedDatabaseComponent.java

@@ -8,6 +8,7 @@ import java.util.logging.Logger;
 
 import net.sf.briar.api.db.DbException;
 import net.sf.briar.api.db.ContactId;
+import net.sf.briar.api.db.NoSuchContactException;
 import net.sf.briar.api.db.Rating;
 import net.sf.briar.api.protocol.AuthorId;
 import net.sf.briar.api.protocol.Batch;
@@ -89,7 +90,10 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 							// unsubscribed from the group
 							if(db.containsSubscription(txn, m.getGroup())) {
 								boolean added = storeMessage(txn, m, null);
-								assert added;
+								if(!added) {
+									if(LOG.isLoggable(Level.FINE))
+										LOG.fine("Duplicate local message");
+								}
 							} else {
 								if(LOG.isLoggable(Level.FINE))
 									LOG.fine("Not subscribed");
@@ -128,7 +132,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 		if(LOG.isLoggable(Level.FINE)) LOG.fine("Generating bundle for " + c);
 		// Ack all batches received from c
 		synchronized(contactLock) {
-			if(!containsContact(c)) return;
+			if(!containsContact(c)) throw new NoSuchContactException();
 			synchronized(messageStatusLock) {
 				Txn txn = db.startTransaction();
 				try {
@@ -148,7 +152,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 		}
 		// Add a list of subscriptions
 		synchronized(contactLock) {
-			if(!containsContact(c)) return;
+			if(!containsContact(c)) throw new NoSuchContactException();
 			synchronized(subscriptionLock) {
 				Txn txn = db.startTransaction();
 				try {
@@ -185,7 +189,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 
 	private Batch fillBatch(ContactId c, long capacity) throws DbException {
 		synchronized(contactLock) {
-			if(!containsContact(c)) return null;
+			if(!containsContact(c)) throw new NoSuchContactException();
 			synchronized(messageLock) {
 				synchronized(messageStatusLock) {
 					Txn txn = db.startTransaction();
@@ -254,7 +258,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 					+ b.getSize() + " bytes");
 		// Mark all messages in acked batches as seen
 		synchronized(contactLock) {
-			if(!containsContact(c)) return;
+			if(!containsContact(c)) throw new NoSuchContactException();
 			synchronized(messageLock) {
 				synchronized(messageStatusLock) {
 					int acks = 0;
@@ -276,7 +280,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 		}
 		// Update the contact's subscriptions
 		synchronized(contactLock) {
-			if(!containsContact(c)) return;
+			if(!containsContact(c)) throw new NoSuchContactException();
 			synchronized(messageStatusLock) {
 				Txn txn = db.startTransaction();
 				try {
@@ -301,7 +305,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 			batches++;
 			waitForPermissionToWrite();
 			synchronized(contactLock) {
-				if(!containsContact(c)) return;
+				if(!containsContact(c)) throw new NoSuchContactException();
 				synchronized(messageLock) {
 					synchronized(messageStatusLock) {
 						synchronized(subscriptionLock) {
@@ -334,7 +338,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 		// Find any lost batches that need to be retransmitted
 		Set<BatchId> lost;
 		synchronized(contactLock) {
-			if(!containsContact(c)) return;
+			if(!containsContact(c)) throw new NoSuchContactException();
 			synchronized(messageLock) {
 				synchronized(messageStatusLock) {
 					Txn txn = db.startTransaction();
@@ -350,7 +354,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 		}
 		for(BatchId batch : lost) {
 			synchronized(contactLock) {
-				if(!containsContact(c)) return;
+				if(!containsContact(c)) throw new NoSuchContactException();
 				synchronized(messageLock) {
 					synchronized(messageStatusLock) {
 						Txn txn = db.startTransaction();

test/build.xml

@@ -15,6 +15,8 @@
 			</classpath>
 			<test name='net.sf.briar.db.DatabaseCleanerImplTest'/>
 			<test name='net.sf.briar.db.H2DatabaseTest'/>
+			<test name='net.sf.briar.db.ReadWriteLockDatabaseComponentTest'/>
+			<test name='net.sf.briar.db.SynchronizedDatabaseComponentTest'/>
 			<test name='net.sf.briar.i18n.FontManagerTest'/>
 			<test name='net.sf.briar.i18n.I18nTest'/>
 			<test name='net.sf.briar.invitation.InvitationWorkerTest'/>

test/net/sf/briar/TestUtils.java

@@ -4,12 +4,16 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.PrintStream;
+import java.util.Random;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import net.sf.briar.api.protocol.UniqueId;
+
 public class TestUtils {
 
 	private static final AtomicInteger nextTestDir =
 		new AtomicInteger((int) (Math.random() * 1000 * 1000));
+	private static final Random random = new Random();
 
 	public static void delete(File f) {
 		if(f.isDirectory()) for(File child : f.listFiles()) delete(child);
@@ -42,4 +46,10 @@ public class TestUtils {
 		if(bin.exists() && bin.isDirectory()) return bin;
 		throw new RuntimeException("Could not find build directory");
 	}
+
+	public static byte[] getRandomId() {
+		byte[] b = new byte[UniqueId.LENGTH];
+		random.nextBytes(b);
+		return b;
+	}
 }

test/net/sf/briar/db/DatabaseComponentImplTest.java

@@ -0,0 +1,140 @@
+package net.sf.briar.db;
+
+import static net.sf.briar.api.db.DatabaseComponent.BYTES_PER_SWEEP;
+import static net.sf.briar.api.db.DatabaseComponent.MIN_FREE_SPACE;
+
+import java.util.Collections;
+
+import net.sf.briar.api.db.DbException;
+import net.sf.briar.api.protocol.Batch;
+import net.sf.briar.api.protocol.MessageId;
+import net.sf.briar.db.DatabaseCleaner.Callback;
+
+import org.jmock.Expectations;
+import org.jmock.Mockery;
+import org.junit.Test;
+
+import com.google.inject.Provider;
+
+/**
+ * Tests that use the DatabaseCleaner.Callback interface of
+ * DatabaseComponentImpl.
+ */
+public abstract class DatabaseComponentImplTest extends DatabaseComponentTest {
+
+	protected abstract <T> DatabaseComponentImpl<T> createDatabaseComponentImpl(
+			Database<T> database, DatabaseCleaner cleaner,
+			Provider<Batch> batchProvider);
+
+	@Test
+	public void testNotCleanedIfEnoughFreeSpace() throws DbException {
+		Mockery context = new Mockery();
+		@SuppressWarnings("unchecked")
+		final Database<Object> database = context.mock(Database.class);
+		final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
+		@SuppressWarnings("unchecked")
+		final Provider<Batch> batchProvider = context.mock(Provider.class);
+		context.checking(new Expectations() {{
+			oneOf(database).getFreeSpace();
+			will(returnValue(MIN_FREE_SPACE));
+		}});
+		Callback db = createDatabaseComponentImpl(database, cleaner,
+				batchProvider);
+
+		db.checkFreeSpaceAndClean();
+
+		context.assertIsSatisfied();
+	}
+
+	@Test
+	public void testCleanedIfNotEnoughFreeSpace() throws DbException {
+		Mockery context = new Mockery();
+		@SuppressWarnings("unchecked")
+		final Database<Object> database = context.mock(Database.class);
+		final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
+		@SuppressWarnings("unchecked")
+		final Provider<Batch> batchProvider = context.mock(Provider.class);
+		context.checking(new Expectations() {{
+			oneOf(database).getFreeSpace();
+			will(returnValue(MIN_FREE_SPACE - 1));
+			oneOf(database).startTransaction();
+			will(returnValue(txn));
+			oneOf(database).getOldMessages(txn, BYTES_PER_SWEEP);
+			will(returnValue(Collections.emptySet()));
+			oneOf(database).commitTransaction(txn);
+			// As if by magic, some free space has appeared
+			oneOf(database).getFreeSpace();
+			will(returnValue(MIN_FREE_SPACE));
+		}});
+		Callback db = createDatabaseComponentImpl(database, cleaner,
+				batchProvider);
+
+		db.checkFreeSpaceAndClean();
+
+		context.assertIsSatisfied();
+	}
+
+	@Test
+	public void testExpiringUnsendableMessageDoesNotTriggerBackwardInclusion()
+	throws DbException {
+		Mockery context = new Mockery();
+		@SuppressWarnings("unchecked")
+		final Database<Object> database = context.mock(Database.class);
+		final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
+		@SuppressWarnings("unchecked")
+		final Provider<Batch> batchProvider = context.mock(Provider.class);
+		context.checking(new Expectations() {{
+			oneOf(database).getFreeSpace();
+			will(returnValue(MIN_FREE_SPACE - 1));
+			oneOf(database).startTransaction();
+			will(returnValue(txn));
+			oneOf(database).getOldMessages(txn, BYTES_PER_SWEEP);
+			will(returnValue(Collections.singleton(messageId)));
+			oneOf(database).getSendability(txn, messageId);
+			will(returnValue(0));
+			oneOf(database).removeMessage(txn, messageId);
+			oneOf(database).commitTransaction(txn);
+			oneOf(database).getFreeSpace();
+			will(returnValue(MIN_FREE_SPACE));
+		}});
+		Callback db = createDatabaseComponentImpl(database, cleaner,
+				batchProvider);
+
+		db.checkFreeSpaceAndClean();
+
+		context.assertIsSatisfied();
+	}
+
+	@Test
+	public void testExpiringSendableMessageTriggersBackwardInclusion()
+	throws DbException {
+		Mockery context = new Mockery();
+		@SuppressWarnings("unchecked")
+		final Database<Object> database = context.mock(Database.class);
+		final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
+		@SuppressWarnings("unchecked")
+		final Provider<Batch> batchProvider = context.mock(Provider.class);
+		context.checking(new Expectations() {{
+			oneOf(database).getFreeSpace();
+			will(returnValue(MIN_FREE_SPACE - 1));
+			oneOf(database).startTransaction();
+			will(returnValue(txn));
+			oneOf(database).getOldMessages(txn, BYTES_PER_SWEEP);
+			will(returnValue(Collections.singleton(messageId)));
+			oneOf(database).getSendability(txn, messageId);
+			will(returnValue(1));
+			oneOf(database).getParent(txn, messageId);
+			will(returnValue(MessageId.NONE));
+			oneOf(database).removeMessage(txn, messageId);
+			oneOf(database).commitTransaction(txn);
+			oneOf(database).getFreeSpace();
+			will(returnValue(MIN_FREE_SPACE));
+		}});
+		Callback db = createDatabaseComponentImpl(database, cleaner,
+				batchProvider);
+
+		db.checkFreeSpaceAndClean();
+
+		context.assertIsSatisfied();
+	}
+}

test/net/sf/briar/db/DatabaseComponentTest.java

@@ -1,8 +1,6 @@
 package net.sf.briar.db;
 
-import java.io.File;
 import java.util.Collections;
-import java.util.Random;
 import java.util.Set;
 
 import junit.framework.TestCase;
@@ -10,10 +8,12 @@ import net.sf.briar.TestUtils;
 import net.sf.briar.api.db.ContactId;
 import net.sf.briar.api.db.DatabaseComponent;
 import net.sf.briar.api.db.DbException;
+import net.sf.briar.api.db.NoSuchContactException;
 import net.sf.briar.api.db.Rating;
 import net.sf.briar.api.db.Status;
 import net.sf.briar.api.protocol.AuthorId;
 import net.sf.briar.api.protocol.Batch;
+import net.sf.briar.api.protocol.Bundle;
 import net.sf.briar.api.protocol.GroupId;
 import net.sf.briar.api.protocol.Message;
 import net.sf.briar.api.protocol.MessageId;
@@ -21,37 +21,32 @@ import net.sf.briar.protocol.MessageImpl;
 
 import org.jmock.Expectations;
 import org.jmock.Mockery;
-import org.junit.After;
-import org.junit.Before;
 import org.junit.Test;
 
 import com.google.inject.Provider;
 
 public abstract class DatabaseComponentTest extends TestCase {
 
-	private final File testDir = TestUtils.getTestDirectory();
-	private final Random random = new Random();
-	private final AuthorId authorId;
-	private final ContactId contactId;
-	private final GroupId groupId;
-	private final MessageId messageId, parentId;
+	protected final Object txn = new Object();
+	protected final AuthorId authorId;
+	protected final ContactId contactId;
+	protected final GroupId groupId;
+	protected final MessageId messageId, parentId;
 	private final long timestamp;
 	private final int size;
 	private final byte[] body;
 	private final Message message;
-	private final Object txn = new Object();
 
 	public DatabaseComponentTest() {
 		super();
-		authorId = new AuthorId(getRandomId());
+		authorId = new AuthorId(TestUtils.getRandomId());
 		contactId = new ContactId(123);
-		groupId = new GroupId(getRandomId());
-		messageId = new MessageId(getRandomId());
-		parentId = new MessageId(getRandomId());
+		groupId = new GroupId(TestUtils.getRandomId());
+		messageId = new MessageId(TestUtils.getRandomId());
+		parentId = new MessageId(TestUtils.getRandomId());
 		timestamp = System.currentTimeMillis();
 		size = 1234;
 		body = new byte[size];
-		random.nextBytes(body);
 		message = new MessageImpl(messageId, MessageId.NONE, groupId, authorId,
 				timestamp, body);
 	}
@@ -60,11 +55,6 @@ public abstract class DatabaseComponentTest extends TestCase {
 			Database<T> database, DatabaseCleaner cleaner,
 			Provider<Batch> batchProvider);
 
-	@Before
-	public void setUp() {
-		testDir.mkdirs();
-	}
-
 	@Test
 	public void testSimpleCalls() throws DbException {
 		final Set<GroupId> subs = Collections.singleton(groupId);
@@ -137,8 +127,6 @@ public abstract class DatabaseComponentTest extends TestCase {
 		@SuppressWarnings("unchecked")
 		final Provider<Batch> batchProvider = context.mock(Provider.class);
 		context.checking(new Expectations() {{
-			oneOf(database).open(false);
-			oneOf(cleaner).startCleaning();
 			// setRating(Rating.GOOD)
 			oneOf(database).startTransaction();
 			will(returnValue(txn));
@@ -153,15 +141,11 @@ public abstract class DatabaseComponentTest extends TestCase {
 			oneOf(database).getParent(txn, messageId);
 			will(returnValue(MessageId.NONE));
 			oneOf(database).commitTransaction(txn);
-			oneOf(cleaner).stopCleaning();
-			oneOf(database).close();
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner,
 				batchProvider);
 
-		db.open(false);
 		db.setRating(authorId, Rating.GOOD);
-		db.close();
 
 		context.assertIsSatisfied();
 	}
@@ -176,8 +160,6 @@ public abstract class DatabaseComponentTest extends TestCase {
 		@SuppressWarnings("unchecked")
 		final Provider<Batch> batchProvider = context.mock(Provider.class);
 		context.checking(new Expectations() {{
-			oneOf(database).open(false);
-			oneOf(cleaner).startCleaning();
 			// setRating(Rating.GOOD)
 			oneOf(database).startTransaction();
 			will(returnValue(txn));
@@ -195,22 +177,18 @@ public abstract class DatabaseComponentTest extends TestCase {
 			oneOf(database).containsMessage(txn, parentId);
 			will(returnValue(false));
 			oneOf(database).commitTransaction(txn);
-			oneOf(cleaner).stopCleaning();
-			oneOf(database).close();
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner,
 				batchProvider);
 
-		db.open(false);
 		db.setRating(authorId, Rating.GOOD);
-		db.close();
 
 		context.assertIsSatisfied();
 	}
 
 	@Test
 	public void testChangingGroupsStopsBackwardInclusion() throws DbException {
-		final GroupId groupId1 = new GroupId(getRandomId());
+		final GroupId groupId1 = new GroupId(TestUtils.getRandomId());
 		final Set<MessageId> messages = Collections.singleton(messageId);
 		Mockery context = new Mockery();
 		@SuppressWarnings("unchecked")
@@ -219,8 +197,6 @@ public abstract class DatabaseComponentTest extends TestCase {
 		@SuppressWarnings("unchecked")
 		final Provider<Batch> batchProvider = context.mock(Provider.class);
 		context.checking(new Expectations() {{
-			oneOf(database).open(false);
-			oneOf(cleaner).startCleaning();
 			// setRating(Rating.GOOD)
 			oneOf(database).startTransaction();
 			will(returnValue(txn));
@@ -242,15 +218,11 @@ public abstract class DatabaseComponentTest extends TestCase {
 			oneOf(database).getGroup(txn, parentId);
 			will(returnValue(groupId1));
 			oneOf(database).commitTransaction(txn);
-			oneOf(cleaner).stopCleaning();
-			oneOf(database).close();
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner,
 				batchProvider);
 
-		db.open(false);
 		db.setRating(authorId, Rating.GOOD);
-		db.close();
 
 		context.assertIsSatisfied();
 	}
@@ -266,8 +238,6 @@ public abstract class DatabaseComponentTest extends TestCase {
 		@SuppressWarnings("unchecked")
 		final Provider<Batch> batchProvider = context.mock(Provider.class);
 		context.checking(new Expectations() {{
-			oneOf(database).open(false);
-			oneOf(cleaner).startCleaning();
 			// setRating(Rating.GOOD)
 			oneOf(database).startTransaction();
 			will(returnValue(txn));
@@ -292,15 +262,11 @@ public abstract class DatabaseComponentTest extends TestCase {
 			will(returnValue(1));
 			oneOf(database).setSendability(txn, parentId, 2);
 			oneOf(database).commitTransaction(txn);
-			oneOf(cleaner).stopCleaning();
-			oneOf(database).close();
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner,
 				batchProvider);
 
-		db.open(false);
 		db.setRating(authorId, Rating.GOOD);
-		db.close();
 
 		context.assertIsSatisfied();
 	}
@@ -316,8 +282,6 @@ public abstract class DatabaseComponentTest extends TestCase {
 		@SuppressWarnings("unchecked")
 		final Provider<Batch> batchProvider = context.mock(Provider.class);
 		context.checking(new Expectations() {{
-			oneOf(database).open(false);
-			oneOf(cleaner).startCleaning();
 			// setRating(Rating.GOOD)
 			oneOf(database).startTransaction();
 			will(returnValue(txn));
@@ -344,15 +308,11 @@ public abstract class DatabaseComponentTest extends TestCase {
 			oneOf(database).getParent(txn, parentId);
 			will(returnValue(MessageId.NONE));
 			oneOf(database).commitTransaction(txn);
-			oneOf(cleaner).stopCleaning();
-			oneOf(database).close();
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner,
 				batchProvider);
 
-		db.open(false);
 		db.setRating(authorId, Rating.GOOD);
-		db.close();
 
 		context.assertIsSatisfied();
 	}
@@ -367,23 +327,17 @@ public abstract class DatabaseComponentTest extends TestCase {
 		@SuppressWarnings("unchecked")
 		final Provider<Batch> batchProvider = context.mock(Provider.class);
 		context.checking(new Expectations() {{
-			oneOf(database).open(false);
-			oneOf(cleaner).startCleaning();
 			// addLocallyGeneratedMessage(message)
 			oneOf(database).startTransaction();
 			will(returnValue(txn));
 			oneOf(database).containsSubscription(txn, groupId);
 			will(returnValue(false));
 			oneOf(database).commitTransaction(txn);
-			oneOf(cleaner).stopCleaning();
-			oneOf(database).close();
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner,
 				batchProvider);
 
-		db.open(false);
 		db.addLocallyGeneratedMessage(message);
-		db.close();
 
 		context.assertIsSatisfied();
 	}
@@ -397,8 +351,6 @@ public abstract class DatabaseComponentTest extends TestCase {
 		@SuppressWarnings("unchecked")
 		final Provider<Batch> batchProvider = context.mock(Provider.class);
 		context.checking(new Expectations() {{
-			oneOf(database).open(false);
-			oneOf(cleaner).startCleaning();
 			// addLocallyGeneratedMessage(message)
 			oneOf(database).startTransaction();
 			will(returnValue(txn));
@@ -407,15 +359,11 @@ public abstract class DatabaseComponentTest extends TestCase {
 			oneOf(database).addMessage(txn, message);
 			will(returnValue(false));
 			oneOf(database).commitTransaction(txn);
-			oneOf(cleaner).stopCleaning();
-			oneOf(database).close();
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner,
 				batchProvider);
 
-		db.open(false);
 		db.addLocallyGeneratedMessage(message);
-		db.close();
 
 		context.assertIsSatisfied();
 	}
@@ -429,8 +377,6 @@ public abstract class DatabaseComponentTest extends TestCase {
 		@SuppressWarnings("unchecked")
 		final Provider<Batch> batchProvider = context.mock(Provider.class);
 		context.checking(new Expectations() {{
-			oneOf(database).open(false);
-			oneOf(cleaner).startCleaning();
 			// addLocallyGeneratedMessage(message)
 			oneOf(database).startTransaction();
 			will(returnValue(txn));
@@ -448,15 +394,11 @@ public abstract class DatabaseComponentTest extends TestCase {
 			will(returnValue(0));
 			oneOf(database).setSendability(txn, messageId, 0);
 			oneOf(database).commitTransaction(txn);
-			oneOf(cleaner).stopCleaning();
-			oneOf(database).close();
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner,
 				batchProvider);
 
-		db.open(false);
 		db.addLocallyGeneratedMessage(message);
-		db.close();
 
 		context.assertIsSatisfied();
 	}
@@ -471,8 +413,6 @@ public abstract class DatabaseComponentTest extends TestCase {
 		@SuppressWarnings("unchecked")
 		final Provider<Batch> batchProvider = context.mock(Provider.class);
 		context.checking(new Expectations() {{
-			oneOf(database).open(false);
-			oneOf(cleaner).startCleaning();
 			// addLocallyGeneratedMessage(message)
 			oneOf(database).startTransaction();
 			will(returnValue(txn));
@@ -493,27 +433,40 @@ public abstract class DatabaseComponentTest extends TestCase {
 			oneOf(database).getParent(txn, messageId);
 			will(returnValue(MessageId.NONE));
 			oneOf(database).commitTransaction(txn);
-			oneOf(cleaner).stopCleaning();
-			oneOf(database).close();
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner,
 				batchProvider);
 
-		db.open(false);
 		db.addLocallyGeneratedMessage(message);
-		db.close();
 
 		context.assertIsSatisfied();
 	}
 
-	private byte[] getRandomId() {
-		byte[] b = new byte[32];
-		random.nextBytes(b);
-		return b;
-	}
+	@Test
+	public void testGenerateBundleThrowsExceptionIfContactIsMissing()
+	throws DbException {
+		Mockery context = new Mockery();
+		@SuppressWarnings("unchecked")
+		final Database<Object> database = context.mock(Database.class);
+		final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
+		@SuppressWarnings("unchecked")
+		final Provider<Batch> batchProvider = context.mock(Provider.class);
+		final Bundle bundle = context.mock(Bundle.class);
+		context.checking(new Expectations() {{
+			oneOf(database).startTransaction();
+			will(returnValue(txn));
+			oneOf(database).containsContact(txn, contactId);
+			will(returnValue(false));
+			oneOf(database).commitTransaction(txn);
+		}});
+		DatabaseComponent db = createDatabaseComponent(database, cleaner,
+				batchProvider);
 
-	@After
-	public void tearDown() {
-		TestUtils.deleteTestDirectory(testDir);
+		try {
+			db.generateBundle(contactId, bundle);
+			assertTrue(false);
+		} catch(NoSuchContactException expected) {}
+
+		context.assertIsSatisfied();
 	}
 }

test/net/sf/briar/db/H2DatabaseTest.java

@@ -53,11 +53,11 @@ public class H2DatabaseTest extends TestCase {
 
 	public H2DatabaseTest() {
 		super();
-		authorId = new AuthorId(getRandomId());
-		batchId = new BatchId(getRandomId());
+		authorId = new AuthorId(TestUtils.getRandomId());
+		batchId = new BatchId(TestUtils.getRandomId());
 		contactId = new ContactId(123);
-		groupId = new GroupId(getRandomId());
-		messageId = new MessageId(getRandomId());
+		groupId = new GroupId(TestUtils.getRandomId());
+		messageId = new MessageId(TestUtils.getRandomId());
 		timestamp = System.currentTimeMillis();
 		size = 1234;
 		body = new byte[size];
@@ -338,7 +338,7 @@ public class H2DatabaseTest extends TestCase {
 
 	@Test
 	public void testBatchesToAck() throws DbException {
-		BatchId batchId1 = new BatchId(getRandomId());
+		BatchId batchId1 = new BatchId(TestUtils.getRandomId());
 		Mockery context = new Mockery();
 		MessageFactory messageFactory = context.mock(MessageFactory.class);
 
@@ -461,13 +461,13 @@ public class H2DatabaseTest extends TestCase {
 
 	@Test
 	public void testRetransmission() throws DbException {
-		BundleId bundleId = new BundleId(getRandomId());
-		BundleId bundleId1 = new BundleId(getRandomId());
-		BundleId bundleId2 = new BundleId(getRandomId());
-		BundleId bundleId3 = new BundleId(getRandomId());
-		BundleId bundleId4 = new BundleId(getRandomId());
-		BatchId batchId1 = new BatchId(getRandomId());
-		BatchId batchId2 = new BatchId(getRandomId());
+		BundleId bundleId = new BundleId(TestUtils.getRandomId());
+		BundleId bundleId1 = new BundleId(TestUtils.getRandomId());
+		BundleId bundleId2 = new BundleId(TestUtils.getRandomId());
+		BundleId bundleId3 = new BundleId(TestUtils.getRandomId());
+		BundleId bundleId4 = new BundleId(TestUtils.getRandomId());
+		BatchId batchId1 = new BatchId(TestUtils.getRandomId());
+		BatchId batchId2 = new BatchId(TestUtils.getRandomId());
 		Set<MessageId> empty = Collections.emptySet();
 		Mockery context = new Mockery();
 		MessageFactory messageFactory = context.mock(MessageFactory.class);
@@ -508,8 +508,8 @@ public class H2DatabaseTest extends TestCase {
 
 	@Test
 	public void testGetMessagesByAuthor() throws DbException {
-		AuthorId authorId1 = new AuthorId(getRandomId());
-		MessageId messageId1 = new MessageId(getRandomId());
+		AuthorId authorId1 = new AuthorId(TestUtils.getRandomId());
+		MessageId messageId1 = new MessageId(TestUtils.getRandomId());
 		Message message1 = new MessageImpl(messageId1, MessageId.NONE, groupId,
 				authorId1, timestamp, body);
 		Mockery context = new Mockery();
@@ -543,10 +543,10 @@ public class H2DatabaseTest extends TestCase {
 
 	@Test
 	public void testGetNumberOfSendableChildren() throws DbException {
-		MessageId childId1 = new MessageId(getRandomId());
-		MessageId childId2 = new MessageId(getRandomId());
-		MessageId childId3 = new MessageId(getRandomId());
-		GroupId groupId1 = new GroupId(getRandomId());
+		MessageId childId1 = new MessageId(TestUtils.getRandomId());
+		MessageId childId2 = new MessageId(TestUtils.getRandomId());
+		MessageId childId3 = new MessageId(TestUtils.getRandomId());
+		GroupId groupId1 = new GroupId(TestUtils.getRandomId());
 		Message child1 = new MessageImpl(childId1, messageId, groupId,
 				authorId, timestamp, body);
 		Message child2 = new MessageImpl(childId2, messageId, groupId,
@@ -588,7 +588,7 @@ public class H2DatabaseTest extends TestCase {
 
 	@Test
 	public void testGetOldMessages() throws DbException {
-		MessageId messageId1 = new MessageId(getRandomId());
+		MessageId messageId1 = new MessageId(TestUtils.getRandomId());
 		Message message1 = new MessageImpl(messageId1, MessageId.NONE, groupId,
 				authorId, timestamp + 1000, body);
 		Mockery context = new Mockery();
@@ -758,12 +758,6 @@ public class H2DatabaseTest extends TestCase {
 		TestUtils.deleteTestDirectory(testDir);
 	}
 
-	private byte[] getRandomId() {
-		byte[] b = new byte[32];
-		random.nextBytes(b);
-		return b;
-	}
-
 	private static class TestMessageFactory implements MessageFactory {
 
 		public Message createMessage(MessageId id, MessageId parent,

test/net/sf/briar/db/ReadWriteLockDatabaseComponentTest.java

@@ -5,12 +5,20 @@ import net.sf.briar.api.protocol.Batch;
 
 import com.google.inject.Provider;
 
-public class ReadWriteLockDatabaseComponentTest extends DatabaseComponentTest {
+public class ReadWriteLockDatabaseComponentTest
+extends DatabaseComponentImplTest {
 
 	@Override
 	protected <T> DatabaseComponent createDatabaseComponent(
 			Database<T> database, DatabaseCleaner cleaner,
 			Provider<Batch> batchProvider) {
+		return createDatabaseComponentImpl(database, cleaner, batchProvider);
+	}
+
+	@Override
+	protected <T> DatabaseComponentImpl<T> createDatabaseComponentImpl(
+			Database<T> database, DatabaseCleaner cleaner,
+			Provider<Batch> batchProvider) {
 		return new ReadWriteLockDatabaseComponent<T>(database, cleaner,
 				batchProvider);
 	}

test/net/sf/briar/db/SynchronizedDatabaseComponentTest.java

@@ -5,14 +5,21 @@ import net.sf.briar.api.protocol.Batch;
 
 import com.google.inject.Provider;
 
-public class SynchronizedDatabaseComponentTest extends DatabaseComponentTest {
+public class SynchronizedDatabaseComponentTest
+extends DatabaseComponentImplTest {
 
 	@Override
 	protected <T> DatabaseComponent createDatabaseComponent(
 			Database<T> database, DatabaseCleaner cleaner,
 			Provider<Batch> batchProvider) {
+		return createDatabaseComponentImpl(database, cleaner, batchProvider);
+	}
+
+	@Override
+	protected <T> DatabaseComponentImpl<T> createDatabaseComponentImpl(
+			Database<T> database, DatabaseCleaner cleaner,
+			Provider<Batch> batchProvider) {
 		return new SynchronizedDatabaseComponent<T>(database, cleaner,
 				batchProvider);
 	}
-
 }