Unit tests, refactoring and bugfixes for the database. Replies to messages in

other groups no longer affect sendability, which makes it safe to delete all
messages from a group when unsubscribing.

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

@@ -15,13 +15,12 @@ public interface DatabaseComponent {
 
 	static final long MEGABYTES = 1024L * 1024L;
 
-	// FIXME: Some of these should be configurable
+	// FIXME: These should be configurable
 	static final long MIN_FREE_SPACE = 300L * MEGABYTES;
 	static final long CRITICAL_FREE_SPACE = 100L * MEGABYTES;
 	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 MS_BETWEEN_SWEEPS = 1000; // 1 sec
 	static final int RETRANSMIT_THRESHOLD = 3;
 
 	/**

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

@@ -153,6 +153,13 @@ interface Database<T> {
 	 */
 	long getFreeSpace() throws DbException;
 
+	/**
+	 * Returns the group that contains the given message.
+	 * <p>
+	 * Locking: messages read.
+	 */
+	GroupId getGroup(T txn, MessageId m) throws DbException;
+
 	/**
 	 * Returns the message identified by the given ID.
 	 * <p>
@@ -168,12 +175,12 @@ interface Database<T> {
 	Iterable<MessageId> getMessagesByAuthor(T txn, AuthorId a) throws DbException;
 
 	/**
-	 * Returns the IDs of all children of the message identified by the given
+	 * Returns the number of children of the message identified by the given
-	 * ID that are present in the database.
+	 * ID that are present in the database and sendable.
 	 * <p>
 	 * Locking: messages read.
 	 */
-	Iterable<MessageId> getMessagesByParent(T txn, MessageId m) throws DbException;
+	int getNumberOfSendableChildren(T txn, MessageId m) throws DbException;
 
 	/**
 	 * Returns the IDs of the oldest messages in the database, with a total

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

@@ -16,16 +16,17 @@ interface DatabaseCleaner {
 	interface Callback {
 
 		/**
-		 * Checks how much free storage space is available to the database, and if
+		 * Checks how much free storage space is available to the database, and
-		 * necessary expires old messages until the free space is at least
+		 * if necessary expires old messages until the free space is at least
-		 * MIN_FREE_SPACE. While the free space is less than CRITICAL_FREE_SPACE,
+		 * MIN_FREE_SPACE. While the free space is less than
-		 * operations that attempt to store messages in the database will block.
+		 * CRITICAL_FREE_SPACE, operations that attempt to store messages in
+		 * the database will block.
 		 */
 		void checkFreeSpaceAndClean() throws DbException;
 
 		/**
-		 * Called by the cleaner; returns true iff the amount of free storage space
+		 * Returns true iff the amount of free storage space available to the
-		 * available to the database should be checked.
+		 * database should be checked.
 		 */
 		boolean shouldCheckFreeSpace();
 	}

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

@@ -3,9 +3,9 @@ package net.sf.briar.db;
 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.ContactId;
 import net.sf.briar.api.db.Rating;
 import net.sf.briar.api.db.Status;
 import net.sf.briar.api.protocol.AuthorId;
@@ -63,11 +63,7 @@ DatabaseCleaner.Callback {
 		// One point for a good rating
 		if(getRating(m.getAuthor()) == Rating.GOOD) sendability++;
 		// One point per sendable child (backward inclusion)
-		for(MessageId kid : db.getMessagesByParent(txn, m.getId())) {
+		sendability += db.getNumberOfSendableChildren(txn, m.getId());
-			Integer kidSendability = db.getSendability(txn, kid);
-			assert kidSendability != null;
-			if(kidSendability > 0) sendability++;
-		}
 		return sendability;
 	}
 
@@ -195,6 +191,7 @@ DatabaseCleaner.Callback {
 			MessageId parent = db.getParent(txn, m);
 			if(parent.equals(MessageId.NONE)) break;
 			if(!db.containsMessage(txn, parent)) break;
+			if(!db.getGroup(txn, m).equals(db.getGroup(txn, parent))) break;
 			Integer parentSendability = db.getSendability(txn, parent);
 			assert parentSendability != null;
 			if(increment) {

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

@@ -17,9 +17,9 @@ import java.util.Set;
 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.ContactId;
 import net.sf.briar.api.db.Rating;
 import net.sf.briar.api.db.Status;
 import net.sf.briar.api.protocol.AuthorId;
@@ -376,6 +376,16 @@ abstract class JdbcDatabase implements Database<Connection> {
 			int rowsAffected = ps.executeUpdate();
 			assert rowsAffected == 1;
 			ps.close();
+			sql = "INSERT INTO receivedBundles"
+				+ " (bundleId, contactId, timestamp)"
+				+ " VALUES (?, ?, ?)";
+			ps = txn.prepareStatement(sql);
+			ps.setBytes(1, BundleId.NONE.getBytes());
+			ps.setInt(2, c.getInt());
+			ps.setLong(3, System.currentTimeMillis());
+			rowsAffected = ps.executeUpdate();
+			assert rowsAffected == 1;
+			ps.close();
 		} catch(SQLException e) {
 			tryToClose(ps);
 			tryToClose(txn);
@@ -736,6 +746,30 @@ abstract class JdbcDatabase implements Database<Connection> {
 		} else return f.length();
 	}
 
+	public GroupId getGroup(Connection txn, MessageId m) throws DbException {
+		PreparedStatement ps = null;
+		ResultSet rs = null;
+		try {
+			String sql = "SELECT groupId FROM messages WHERE messageId = ?";
+			ps = txn.prepareStatement(sql);
+			ps.setBytes(1, m.getBytes());
+			rs = ps.executeQuery();
+			boolean found = rs.next();
+			assert found;
+			byte[] group = rs.getBytes(1);
+			boolean more = rs.next();
+			assert !more;
+			rs.close();
+			ps.close();
+			return new GroupId(group);
+		} catch(SQLException e) {
+			tryToClose(rs);
+			tryToClose(ps);
+			tryToClose(txn);
+			throw new DbException(e);
+		}
+	}
+
 	public Message getMessage(Connection txn, MessageId m) throws DbException {
 		PreparedStatement ps = null;
 		ResultSet rs = null;
@@ -792,29 +826,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
-	public Iterable<MessageId> getMessagesByParent(Connection txn, MessageId m)
+	private int getNumberOfMessages(Connection txn) throws DbException {
-	throws DbException {
-		PreparedStatement ps = null;
-		ResultSet rs = null;
-		try {
-			String sql = "SELECT messageId FROM messages WHERE parentId = ?";
-			ps = txn.prepareStatement(sql);
-			ps.setBytes(1, m.getBytes());
-			rs = ps.executeQuery();
-			List<MessageId> ids = new ArrayList<MessageId>();
-			while(rs.next()) ids.add(new MessageId(rs.getBytes(1)));
-			rs.close();
-			ps.close();
-			return ids;
-		} catch(SQLException e) {
-			tryToClose(rs);
-			tryToClose(ps);
-			tryToClose(txn);
-			throw new DbException(e);
-		}
-	}
-
-	public int getNumberOfMessages(Connection txn) throws DbException {
 		PreparedStatement ps = null;
 		ResultSet rs = null;
 		try {
@@ -837,6 +849,46 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
+	public int getNumberOfSendableChildren(Connection txn, MessageId m)
+	throws DbException {
+		PreparedStatement ps = null;
+		ResultSet rs = null;
+		try {
+			// Children in other groups should not be counted
+			String sql = "SELECT groupId FROM messages WHERE messageId = ?";
+			ps = txn.prepareStatement(sql);
+			ps.setBytes(1, m.getBytes());
+			rs = ps.executeQuery();
+			boolean found = rs.next();
+			assert found;
+			byte[] group = rs.getBytes(1);
+			boolean more = rs.next();
+			assert !more;
+			rs.close();
+			ps.close();
+			sql = "SELECT COUNT(messageId) FROM messages"
+				+ " WHERE parentId = ? AND groupId = ?"
+				+ " AND sendability > ZERO()";
+			ps = txn.prepareStatement(sql);
+			ps.setBytes(1, m.getBytes());
+			ps.setBytes(2, group);
+			rs = ps.executeQuery();
+			found = rs.next();
+			assert found;
+			int count = rs.getInt(1);
+			more = rs.next();
+			assert !more;
+			rs.close();
+			ps.close();
+			return count;
+		} catch(SQLException e) {
+			tryToClose(rs);
+			tryToClose(ps);
+			tryToClose(txn);
+			throw new DbException(e);
+		}
+	}
+
 	public Iterable<MessageId> getOldMessages(Connection txn, long capacity)
 	throws DbException {
 		PreparedStatement ps = null;

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

@@ -6,6 +6,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.Random;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -18,6 +19,7 @@ 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.BatchId;
+import net.sf.briar.api.protocol.BundleId;
 import net.sf.briar.api.protocol.GroupId;
 import net.sf.briar.api.protocol.Message;
 import net.sf.briar.api.protocol.MessageFactory;
@@ -38,28 +40,29 @@ public class H2DatabaseTest extends TestCase {
 	private final File testDir = TestUtils.getTestDirectory();
 	// The password has the format <file password> <space> <user password>
 	private final String passwordString = "foo bar";
-	// Some bytes for test IDs
+	private final Random random;
-	private final byte[] idBytes = new byte[32], idBytes1 = new byte[32];
+	private final AuthorId authorId;
 	private final BatchId batchId;
 	private final ContactId contactId;
-	private final MessageId messageId;
 	private final GroupId groupId;
-	private final AuthorId authorId;
+	private final MessageId messageId;
-	private final long timestamp = System.currentTimeMillis();
+	private final long timestamp;
-	private final int size = 1234;
+	private final int size;
-	private final byte[] body = new byte[size];
+	private final byte[] body;
 	private final Message message;
 
 	public H2DatabaseTest() {
 		super();
-		for(int i = 0; i < idBytes.length; i++) idBytes[i] = (byte) i;
+		random = new Random();
-		for(int i = 0; i < idBytes1.length; i++) idBytes1[i] = (byte) (i + 1);
+		authorId = new AuthorId(getRandomId());
-		for(int i = 0; i < body.length; i++) body[i] = (byte) i;
+		batchId = new BatchId(getRandomId());
-		batchId = new BatchId(idBytes);
 		contactId = new ContactId(123);
-		messageId = new MessageId(idBytes);
+		groupId = new GroupId(getRandomId());
-		groupId = new GroupId(idBytes);
+		messageId = new MessageId(getRandomId());
-		authorId = new AuthorId(idBytes);
+		timestamp = System.currentTimeMillis();
+		size = 1234;
+		body = new byte[size];
+		random.nextBytes(body);
 		message = new MessageImpl(messageId, MessageId.NONE, groupId, authorId,
 				timestamp, body);
 	}
@@ -336,7 +339,7 @@ public class H2DatabaseTest extends TestCase {
 
 	@Test
 	public void testBatchesToAck() throws DbException {
-		BatchId batchId1 = new BatchId(idBytes1);
+		BatchId batchId1 = new BatchId(getRandomId());
 		Mockery context = new Mockery();
 		MessageFactory messageFactory = context.mock(MessageFactory.class);
 
@@ -457,10 +460,57 @@ public class H2DatabaseTest extends TestCase {
 		context.assertIsSatisfied();
 	}
 
+	@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());
+		Set<MessageId> empty = Collections.emptySet();
+		Mockery context = new Mockery();
+		MessageFactory messageFactory = context.mock(MessageFactory.class);
+
+		// Create a new database
+		Database<Connection> db = open(false, messageFactory);
+		// Add a contact
+		Connection txn = db.startTransaction();
+		db.addContact(txn, contactId);
+		// Add an oustanding batch (associated with BundleId.NONE)
+		db.addOutstandingBatch(txn, contactId, batchId, empty);
+		// Receive a bundle
+		Set<BatchId> lost = db.addReceivedBundle(txn, contactId, bundleId);
+		assertTrue(lost.isEmpty());
+		// Add a couple more outstanding batches (associated with bundleId)
+		db.addOutstandingBatch(txn, contactId, batchId1, empty);
+		db.addOutstandingBatch(txn, contactId, batchId2, empty);
+		// Receive another bundle
+		lost = db.addReceivedBundle(txn, contactId, bundleId1);
+		assertTrue(lost.isEmpty());
+		// The contact acks one of the batches - it should not be retransmitted
+		db.removeAckedBatch(txn, contactId, batchId1);
+		// Receive another bundle - batchId should now be considered lost
+		lost = db.addReceivedBundle(txn, contactId, bundleId2);
+		assertEquals(1, lost.size());
+		assertTrue(lost.contains(batchId));
+		// Receive another bundle - batchId2 should now be considered lost
+		lost = db.addReceivedBundle(txn, contactId, bundleId3);
+		assertEquals(1, lost.size());
+		assertTrue(lost.contains(batchId2));
+		// Receive another bundle - no further losses
+		lost = db.addReceivedBundle(txn, contactId, bundleId4);
+		assertTrue(lost.isEmpty());
+		db.commitTransaction(txn);
+		db.close();
+		context.assertIsSatisfied();
+	}
+
 	@Test
 	public void testGetMessagesByAuthor() throws DbException {
-		AuthorId authorId1 = new AuthorId(idBytes1);
+		AuthorId authorId1 = new AuthorId(getRandomId());
-		MessageId messageId1 = new MessageId(idBytes1);
+		MessageId messageId1 = new MessageId(getRandomId());
 		Message message1 = new MessageImpl(messageId1, MessageId.NONE, groupId,
 				authorId1, timestamp, body);
 		Mockery context = new Mockery();
@@ -493,28 +543,44 @@ public class H2DatabaseTest extends TestCase {
 	}
 
 	@Test
-	public void testGetMessagesByParent() throws DbException {
+	public void testGetNumberOfSendableChildren() throws DbException {
-		MessageId parentId = new MessageId(idBytes1);
+		MessageId childId1 = new MessageId(getRandomId());
-		Message message1 = new MessageImpl(messageId, parentId, groupId,
+		MessageId childId2 = new MessageId(getRandomId());
+		MessageId childId3 = new MessageId(getRandomId());
+		GroupId groupId1 = new GroupId(getRandomId());
+		Message child1 = new MessageImpl(childId1, messageId, groupId,
+				authorId, timestamp, body);
+		Message child2 = new MessageImpl(childId2, messageId, groupId,
+				authorId, timestamp, body);
+		// The third child is in a different group
+		Message child3 = new MessageImpl(childId3, messageId, groupId1,
 				authorId, timestamp, body);
 		Mockery context = new Mockery();
 		MessageFactory messageFactory = context.mock(MessageFactory.class);
 
 		// Create a new database
 		Database<Connection> db = open(false, messageFactory);
-		// Subscribe to a group and store a message
+		// Subscribe to the groups and store the messages
 		Connection txn = db.startTransaction();
 		db.addSubscription(txn, groupId);
-		db.addMessage(txn, message1);
+		db.addSubscription(txn, groupId1);
+		db.addMessage(txn, message);
+		db.addMessage(txn, child1);
+		db.addMessage(txn, child2);
+		db.addMessage(txn, child3);
+		// Make all the children sendable
+		db.setSendability(txn, childId1, 1);
+		db.setSendability(txn, childId2, 5);
+		db.setSendability(txn, childId3, 3);
 		db.commitTransaction(txn);
 
-		// Check that the message is retrievable via its parent
+		// There should be two sendable children
 		txn = db.startTransaction();
-		Iterator<MessageId> it =
+		assertEquals(2, db.getNumberOfSendableChildren(txn, messageId));
-			db.getMessagesByParent(txn, parentId).iterator();
+		// Make one of the children unsendable
-		assertTrue(it.hasNext());
+		db.setSendability(txn, childId1, 0);
-		assertEquals(messageId, it.next());
+		// Now there should be one sendable child
-		assertFalse(it.hasNext());
+		assertEquals(1, db.getNumberOfSendableChildren(txn, messageId));
 		db.commitTransaction(txn);
 
 		db.close();
@@ -523,7 +589,7 @@ public class H2DatabaseTest extends TestCase {
 
 	@Test
 	public void testGetOldMessages() throws DbException {
-		MessageId messageId1 = new MessageId(idBytes1);
+		MessageId messageId1 = new MessageId(getRandomId());
 		Message message1 = new MessageImpl(messageId1, MessageId.NONE, groupId,
 				authorId, timestamp + 1000, body);
 		Mockery context = new Mockery();
@@ -621,7 +687,7 @@ public class H2DatabaseTest extends TestCase {
 		db.commitTransaction(txn);
 		// The other thread should now terminate
 		try {
-			t.join(10000);
+			t.join(10 * 1000);
 		} catch(InterruptedException ignored) {}
 		assertTrue(closed.get());
 		// Check that the other thread didn't encounter an error
@@ -693,6 +759,12 @@ 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/util/StringUtilsTest.java

@@ -1,5 +1,7 @@
 package net.sf.briar.util;
 
+import java.util.Arrays;
+
 import junit.framework.TestCase;
 
 import org.junit.Test;
@@ -17,4 +19,27 @@ public class StringUtilsTest extends TestCase {
 		String tail = StringUtils.tail("987654321", 5);
 		assertEquals("...54321", tail);
 	}
+
+	@Test
+	public void testToHexString() {
+		byte[] b = new byte[] {1, 2, 3, 127, -128};
+		String s = StringUtils.toHexString(b);
+		assertEquals("0102037F80", s);
+	}
+
+	@Test
+	public void testFromHexString() {
+		try {
+			StringUtils.fromHexString("12345");
+			assertTrue(false);
+		} catch(IllegalArgumentException expected) {}
+		try {
+			StringUtils.fromHexString("ABCDEFGH");
+			assertTrue(false);
+		} catch(IllegalArgumentException expected) {}
+		byte[] b = StringUtils.fromHexString("0102037F80");
+		assertTrue(Arrays.equals(new byte[] {1, 2, 3, 127, -128}, b));
+		b = StringUtils.fromHexString("0a0b0c0d0e0f");
+		assertTrue(Arrays.equals(new byte[] {10, 11, 12, 13, 14, 15}, b));
+	}
 }

util/net/sf/briar/util/StringUtils.java

@@ -2,6 +2,11 @@ package net.sf.briar.util;
 
 public class StringUtils {
 
+	private static final char[] HEX = new char[] {
+		'0', '1', '2', '3', '4', '5', '6', '7',
+		'8', '9', 'A', 'B', 'C', 'D', 'E', 'F'
+	};
+
 	/**
 	 * Trims the given string to the given length, returning the head and
 	 * appending "..." if the string was trimmed.
@@ -19,4 +24,38 @@ public class StringUtils {
 		if(s.length() > length) return "..." + s.substring(s.length() - length);
 		else return s;
 	}
+
+	/** Converts the given raw byte array to a hex string. */
+	public static String toHexString(byte[] bytes) {
+		StringBuilder s = new StringBuilder(bytes.length * 2);
+		for(byte b : bytes) {
+			int high = (b >> 4) & 0xF;
+			s.append(HEX[high]);
+			int low = b & 0xF;
+			s.append(HEX[low]);
+		}
+		return s.toString();
+	}
+
+	/** Converts the given hex string to a raw byte array. */
+	public static byte[] fromHexString(String hex) {
+		int len = hex.length();
+		if(len % 2 != 0) throw new IllegalArgumentException("Not a hex string");
+		byte[] bytes = new byte[len / 2];
+		for(int i = 0, j = 0; i < len; i += 2, j++) {
+			int high = hexDigitToInt(hex.charAt(i));
+			int low = hexDigitToInt(hex.charAt(i + 1));
+			int b = (high << 4) + low;
+			if(b > 127) b -= 256;
+			bytes[j] = (byte) b;
+		}
+		return bytes;
+	}
+
+	private static int hexDigitToInt(char c) {
+		if(c >= '0' && c <= '9') return c - '0';
+		if(c >= 'A' && c <= 'F') return c - 'A' + 10;
+		if(c >= 'a' && c <= 'f') return c - 'a' + 10;
+		throw new IllegalArgumentException("Not a hex digit: " + c);
+	}
 }