Unit tests and fixes for DatabaseComponent(Impl).

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

@@ -51,6 +51,9 @@ public interface DatabaseComponent {
 	 */
 	ContactId addContact() throws DbException;
 
+	/** Adds a contact transport to the database. */
+	void addContactTransport(ContactTransport ct) throws DbException;
+
 	/** Adds a locally generated group message to the database. */
 	void addLocalGroupMessage(Message m) throws DbException;
 
@@ -130,6 +133,9 @@ public interface DatabaseComponent {
 	Map<ContactId, TransportProperties> getRemoteProperties(TransportId t)
 			throws DbException;
 
+	/** Returns all temporary secrets. */
+	Collection<TemporarySecret> getSecrets() throws DbException;
+
 	/** Returns the set of groups to which the user subscribes. */
 	Collection<Group> getSubscriptions() throws DbException;
 

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

@@ -61,7 +61,6 @@ import net.sf.briar.api.protocol.SubscriptionUpdate;
 import net.sf.briar.api.protocol.Transport;
 import net.sf.briar.api.protocol.TransportId;
 import net.sf.briar.api.protocol.TransportUpdate;
-import net.sf.briar.util.ByteUtils;
 
 import com.google.inject.Inject;
 
@@ -173,7 +172,6 @@ DatabaseCleaner.Callback {
 
 	public ContactId addContact() throws DbException {
 		ContactId c;
-		Collection<byte[]> erase = new ArrayList<byte[]>();
 		contactLock.writeLock().lock();
 		try {
 			subscriptionLock.writeLock().lock();
@@ -201,8 +199,6 @@ DatabaseCleaner.Callback {
 			}
 		} finally {
 			contactLock.writeLock().unlock();
-			// Erase the secrets after committing or aborting the transaction
-			for(byte[] b : erase) ByteUtils.erase(b);
 		}
 		// Call the listeners outside the lock
 		callListeners(new ContactAddedEvent(c));
@@ -214,6 +210,29 @@ DatabaseCleaner.Callback {
 		for(DatabaseListener d : listeners) d.eventOccurred(e);
 	}
 
+	public void addContactTransport(ContactTransport ct) throws DbException {
+		contactLock.readLock().lock();
+		try {
+			windowLock.writeLock().lock();
+			try {
+				T txn = db.startTransaction();
+				try {
+					if(!db.containsContact(txn, ct.getContactId()))
+						throw new NoSuchContactException();
+					db.addContactTransport(txn, ct);
+					db.commitTransaction(txn);
+				} catch(DbException e) {
+					db.abortTransaction(txn);
+					throw e;
+				}
+			} finally {
+				windowLock.writeLock().unlock();
+			}
+		} finally {
+			contactLock.readLock().unlock();
+		}
+	}
+
 	public void addLocalGroupMessage(Message m) throws DbException {
 		boolean added = false;
 		contactLock.readLock().lock();
@@ -398,7 +417,7 @@ DatabaseCleaner.Callback {
 				windowLock.writeLock().unlock();
 			}
 		} finally {
-			contactLock.writeLock().unlock();
+			contactLock.readLock().unlock();
 		}
 	}
 
@@ -862,6 +881,28 @@ DatabaseCleaner.Callback {
 		}
 	}
 
+	public Collection<TemporarySecret> getSecrets() throws DbException {
+		contactLock.readLock().lock();
+		try {
+			windowLock.readLock().lock();
+			try {
+				T txn = db.startTransaction();
+				try {
+					Collection<TemporarySecret> secrets = db.getSecrets(txn);
+					db.commitTransaction(txn);
+					return secrets;
+				} catch(DbException e) {
+					db.abortTransaction(txn);
+					throw e;
+				}
+			} finally {
+				windowLock.readLock().unlock();
+			}
+		} finally {
+			contactLock.readLock().unlock();
+		}
+	}
+
 	public Collection<Group> getSubscriptions() throws DbException {
 		subscriptionLock.readLock().lock();
 		try {
@@ -977,6 +1018,7 @@ DatabaseCleaner.Callback {
 					db.commitTransaction(txn);
 				} catch(DbException e) {
 					db.abortTransaction(txn);
+					throw e;
 				}
 			} finally {
 				windowLock.writeLock().unlock();
@@ -1274,6 +1316,7 @@ DatabaseCleaner.Callback {
 					db.commitTransaction(txn);
 				} catch(DbException e) {
 					db.abortTransaction(txn);
+					throw e;
 				}
 			} finally {
 				windowLock.writeLock().unlock();

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

@@ -5,18 +5,18 @@ import java.util.Arrays;
 import java.util.BitSet;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Map;
-import java.util.Random;
 
 import net.sf.briar.BriarTestCase;
 import net.sf.briar.TestUtils;
 import net.sf.briar.api.ContactId;
 import net.sf.briar.api.Rating;
 import net.sf.briar.api.TransportProperties;
+import net.sf.briar.api.db.ContactTransport;
 import net.sf.briar.api.db.DatabaseComponent;
-import net.sf.briar.api.db.MessageHeader;
 import net.sf.briar.api.db.NoSuchContactException;
+import net.sf.briar.api.db.NoSuchContactTransportException;
 import net.sf.briar.api.db.Status;
+import net.sf.briar.api.db.TemporarySecret;
 import net.sf.briar.api.db.event.ContactAddedEvent;
 import net.sf.briar.api.db.event.ContactRemovedEvent;
 import net.sf.briar.api.db.event.DatabaseListener;
@@ -61,8 +61,8 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 	private final Group group;
 	private final TransportId transportId;
 	private final Collection<Transport> transports;
-	private final Map<ContactId, TransportProperties> remoteProperties;
-	private final byte[] inSecret, outSecret;
+	private final ContactTransport contactTransport;
+	private final TemporarySecret temporarySecret;
 
 	public DatabaseComponentTest() {
 		super();
@@ -84,14 +84,12 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 		transportId = new TransportId(TestUtils.getRandomId());
 		TransportProperties properties = new TransportProperties(
 				Collections.singletonMap("foo", "bar"));
-		remoteProperties = Collections.singletonMap(contactId, properties);
 		Transport transport = new Transport(transportId, properties);
 		transports = Collections.singletonList(transport);
-		Random r = new Random();
-		inSecret = new byte[32];
-		r.nextBytes(inSecret);
-		outSecret = new byte[32];
-		r.nextBytes(outSecret);
+		contactTransport = new ContactTransport(contactId, transportId, 123L,
+				234L, 345L, true);
+		temporarySecret = new TemporarySecret(contactId, transportId, 0L,
+				new byte[32], 0L, 0L, new byte[4]);
 	}
 
 	protected abstract <T> DatabaseComponent createDatabaseComponent(
@@ -101,7 +99,6 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 	@Test
 	@SuppressWarnings("unchecked")
 	public void testSimpleCalls() throws Exception {
-		// FIXME: Test new methods
 		final int shutdownHandle = 12345;
 		Mockery context = new Mockery();
 		final Database<Object> database = context.mock(Database.class);
@@ -128,7 +125,7 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 			oneOf(database).setRating(txn, authorId, Rating.GOOD);
 			will(returnValue(Rating.UNRATED));
 			oneOf(database).getMessagesByAuthor(txn, authorId);
-			will(returnValue(Collections.<MessageId>emptyList()));
+			will(returnValue(Collections.emptyList()));
 			oneOf(listener).eventOccurred(with(any(RatingChangedEvent.class)));
 			// setRating(authorId, Rating.GOOD) again
 			oneOf(database).setRating(txn, authorId, Rating.GOOD);
@@ -142,7 +139,7 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 			will(returnValue(Collections.singletonList(contactId)));
 			// getTransportProperties(transportId)
 			oneOf(database).getRemoteProperties(txn, transportId);
-			will(returnValue(remoteProperties));
+			will(returnValue(Collections.emptyMap()));
 			// subscribe(group)
 			oneOf(group).getId();
 			will(returnValue(groupId));
@@ -156,7 +153,7 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 			will(returnValue(true));
 			// getMessageHeaders(groupId)
 			oneOf(database).getMessageHeaders(txn, groupId);
-			will(returnValue(Collections.<MessageHeader>emptyList()));
+			will(returnValue(Collections.emptyList()));
 			// getSubscriptions()
 			oneOf(database).getSubscriptions(txn);
 			will(returnValue(Collections.singletonList(groupId)));
@@ -164,7 +161,7 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 			oneOf(database).containsSubscription(txn, groupId);
 			will(returnValue(true));
 			oneOf(database).getVisibility(txn, groupId);
-			will(returnValue(Collections.<ContactId>emptyList()));
+			will(returnValue(Collections.emptyList()));
 			oneOf(database).removeSubscription(txn, groupId);
 			// unsubscribe(groupId) again
 			oneOf(database).containsSubscription(txn, groupId);
@@ -189,7 +186,8 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 		db.setRating(authorId, Rating.GOOD); // Second time - not called
 		assertEquals(contactId, db.addContact());
 		assertEquals(Collections.singletonList(contactId), db.getContacts());
-		assertEquals(remoteProperties, db.getRemoteProperties(transportId));
+		assertEquals(Collections.emptyMap(),
+				db.getRemoteProperties(transportId));
 		db.subscribe(group); // First time - listeners called
 		db.subscribe(group); // Second time - not called
 		assertEquals(Collections.emptyList(), db.getMessageHeaders(groupId));
@@ -495,7 +493,6 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 	@Test
 	public void testVariousMethodsThrowExceptionIfContactIsMissing()
 			throws Exception {
-		// FIXME: Test new methods
 		Mockery context = new Mockery();
 		@SuppressWarnings("unchecked")
 		final Database<Object> database = context.mock(Database.class);
@@ -510,16 +507,21 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 		final TransportUpdate transportUpdate =
 				context.mock(TransportUpdate.class);
 		context.checking(new Expectations() {{
-			// Check whether the contact is still in the DB (which it's not)
-			exactly(15).of(database).startTransaction();
+			// Check whether the contact is in the DB (which it's not)
+			exactly(16).of(database).startTransaction();
 			will(returnValue(txn));
-			exactly(15).of(database).containsContact(txn, contactId);
+			exactly(16).of(database).containsContact(txn, contactId);
 			will(returnValue(false));
-			exactly(15).of(database).abortTransaction(txn);
+			exactly(16).of(database).abortTransaction(txn);
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner,
 				shutdown, packetFactory);
 
+		try {
+			db.addContactTransport(contactTransport);
+			fail();
+		} catch(NoSuchContactException expected) {}
+
 		try {
 			db.addLocalPrivateMessage(privateMessage, contactId);
 			fail();
@@ -599,6 +601,40 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 		context.assertIsSatisfied();
 	}
 
+	@Test
+	public void testVariousMethodsThrowExceptionIfContactTransportIsMissing()
+			throws Exception {
+		Mockery context = new Mockery();
+		@SuppressWarnings("unchecked")
+		final Database<Object> database = context.mock(Database.class);
+		final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
+		final ShutdownManager shutdown = context.mock(ShutdownManager.class);
+		final PacketFactory packetFactory = context.mock(PacketFactory.class);
+		context.checking(new Expectations() {{
+			// Check whether the contact transport is in the DB (which it's not)
+			exactly(2).of(database).startTransaction();
+			will(returnValue(txn));
+			exactly(2).of(database).containsContactTransport(txn, contactId,
+					transportId);
+			will(returnValue(false));
+			exactly(2).of(database).abortTransaction(txn);
+		}});
+		DatabaseComponent db = createDatabaseComponent(database, cleaner,
+				shutdown, packetFactory);
+
+		try {
+			db.incrementConnectionCounter(contactId, transportId, 0L);
+			fail();
+		} catch(NoSuchContactTransportException expected) {}
+
+		try {
+			db.setConnectionWindow(contactId, transportId, 0L, 0L, new byte[4]);
+			fail();
+		} catch(NoSuchContactTransportException expected) {}
+
+		context.assertIsSatisfied();
+	}
+
 	@Test
 	public void testGenerateAck() throws Exception {
 		final BatchId batchId1 = new BatchId(TestUtils.getRandomId());
@@ -779,7 +815,7 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 			// Get the visible holes and subscriptions
 			oneOf(database).getVisibleHoles(with(txn), with(contactId),
 					with(any(long.class)));
-			will(returnValue(Collections.<GroupId, GroupId>emptyMap()));
+			will(returnValue(Collections.emptyMap()));
 			oneOf(database).getVisibleSubscriptions(with(txn), with(contactId),
 					with(any(long.class)));
 			will(returnValue(Collections.singletonMap(group, 0L)));
@@ -1533,4 +1569,39 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 
 		context.assertIsSatisfied();
 	}
+
+	@Test
+	public void testTemporarySecrets() throws Exception {
+		Mockery context = new Mockery();
+		@SuppressWarnings("unchecked")
+		final Database<Object> database = context.mock(Database.class);
+		final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
+		final ShutdownManager shutdown = context.mock(ShutdownManager.class);
+		final PacketFactory packetFactory = context.mock(PacketFactory.class);
+		context.checking(new Expectations() {{
+			// addSecrets()
+			oneOf(database).startTransaction();
+			will(returnValue(txn));
+			oneOf(database).containsContactTransport(txn, contactId,
+					transportId);
+			will(returnValue(true));
+			oneOf(database).addSecrets(txn,
+					Collections.singletonList(temporarySecret));
+			oneOf(database).commitTransaction(txn);
+			// getSecrets()
+			oneOf(database).startTransaction();
+			will(returnValue(txn));
+			oneOf(database).getSecrets(txn);
+			will(returnValue(Collections.singletonList(temporarySecret)));
+			oneOf(database).commitTransaction(txn);
+		}});
+		DatabaseComponent db = createDatabaseComponent(database, cleaner,
+				shutdown, packetFactory);
+
+		db.addSecrets(Collections.singletonList(temporarySecret));
+		assertEquals(Collections.singletonList(temporarySecret),
+				db.getSecrets());
+
+		context.assertIsSatisfied();
+	}
 }