Replaced Database.getParent() with getGroupMessageParent().

The new method checks whether the parent is present in the database
and belongs to the same group, so separate methods for those checks
have been removed.

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

@@ -191,11 +191,13 @@ interface Database<T> {
 	long getFreeSpace() throws DbException;
 
 	/**
-	 * Returns the group that contains the given message.
+	 * Returns the parent of the given group message, or null if either the
+	 * message has no parent, or the parent is absent from the database, or the
+	 * parent belongs to a different group.
 	 * <p>
 	 * Locking: messages read.
 	 */
-	GroupId getGroup(T txn, MessageId m) throws DbException;
+	MessageId getGroupMessageParent(T txn, MessageId m) throws DbException;
 
 	/**
 	 * Returns the IDs of any batches sent to the given contact that should now
@@ -248,14 +250,6 @@ interface Database<T> {
 	 */
 	Collection<MessageId> getOldMessages(T txn, int size) throws DbException;
 
-	/**
-	 * Returns the parent of the given message, or null if the message has
-	 * no parent.
-	 * <p>
-	 * Locking: messages read.
-	 */
-	MessageId getParent(T txn, MessageId m) throws DbException;
-
 	/**
 	 * Returns the user's rating for the given author.
 	 * <p>

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

@@ -278,19 +278,13 @@ DatabaseCleaner.Callback {
 	 */
 	private int updateAncestorSendability(T txn, MessageId m, boolean increment)
 	throws DbException {
-		GroupId group = db.getGroup(txn, m);
 		int affected = 0;
 		boolean changed = true;
 		while(changed) {
-			// Stop if the message has no parent
-			MessageId parent = db.getParent(txn, m);
+			// Stop if the message has no parent, or the parent isn't in the
+			// database, or the parent belongs to a different group
+			MessageId parent = db.getGroupMessageParent(txn, m);
 			if(parent == null) break;
-			// Stop if the parent isn't in the database
-			if(!db.containsMessage(txn, parent)) break;
-			// Stop if the message and the parent aren't in the same group
-			assert group != null;
-			GroupId parentGroup = db.getGroup(txn, parent);
-			if(!group.equals(parentGroup)) break;
 			// Increment or decrement the parent's sendability
 			int parentSendability = db.getSendability(txn, parent);
 			if(increment) {
@@ -306,7 +300,6 @@ DatabaseCleaner.Callback {
 			db.setSendability(txn, parent, parentSendability);
 			// Move on to the parent's parent
 			m = parent;
-			group = parentGroup;
 		}
 		return affected;
 	}

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

@@ -1098,20 +1098,27 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
-	public MessageId getParent(Connection txn, MessageId m) throws DbException {
+	public MessageId getGroupMessageParent(Connection txn, MessageId m)
+	throws DbException {
 		PreparedStatement ps = null;
 		ResultSet rs = null;
 		try {
-			String sql = "SELECT parentId FROM messages WHERE messageId = ?";
+			String sql = "SELECT m1.parentId FROM messages AS m1"
+				+ " JOIN messages AS m2"
+				+ " ON m1.parentId = m2.messageId"
+				+ " AND m1.groupId = m2.groupId"
+				+ " WHERE m1.messageId = ?";
 			ps = txn.prepareStatement(sql);
 			ps.setBytes(1, m.getBytes());
 			rs = ps.executeQuery();
-			if(!rs.next()) throw new DbStateException();
-			byte[] parent = rs.getBytes(1);
-			if(rs.next()) throw new DbStateException();
+			MessageId parent = null;
+			if(rs.next()) {
+				parent = new MessageId(rs.getBytes(1));
+				if(rs.next()) throw new DbStateException();
+			}
 			rs.close();
 			ps.close();
-			return parent == null ? null : new MessageId(parent);
+			return parent;
 		} catch(SQLException e) {
 			tryToClose(rs);
 			tryToClose(ps);

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

@@ -105,9 +105,7 @@ public class DatabaseComponentImplTest extends DatabaseComponentTest {
 			will(returnValue(Collections.singleton(messageId)));
 			oneOf(database).getSendability(txn, messageId);
 			will(returnValue(1));
-			oneOf(database).getGroup(txn, messageId);
-			will(returnValue(groupId));
-			oneOf(database).getParent(txn, messageId);
+			oneOf(database).getGroupMessageParent(txn, messageId);
 			will(returnValue(null));
 			oneOf(database).removeMessage(txn, messageId);
 			oneOf(database).commitTransaction(txn);

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

@@ -200,117 +200,7 @@ public abstract class DatabaseComponentTest extends TestCase {
 			will(returnValue(0));
 			oneOf(database).setSendability(txn, messageId, 1);
 			// Backward inclusion stops when the message has no parent
-			oneOf(database).getGroup(txn, messageId);
-			will(returnValue(groupId));
-			oneOf(database).getParent(txn, messageId);
-			will(returnValue(null));
-			oneOf(database).commitTransaction(txn);
-		}});
-		DatabaseComponent db = createDatabaseComponent(database, cleaner);
-
-		db.setRating(authorId, Rating.GOOD);
-
-		context.assertIsSatisfied();
-	}
-
-	@Test
-	public void testMissingParentStopsBackwardInclusion() throws DbException {
-		Mockery context = new Mockery();
-		@SuppressWarnings("unchecked")
-		final Database<Object> database = context.mock(Database.class);
-		final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
-		context.checking(new Expectations() {{
-			// setRating(Rating.GOOD)
-			oneOf(database).startTransaction();
-			will(returnValue(txn));
-			oneOf(database).setRating(txn, authorId, Rating.GOOD);
-			// The sendability of the author's messages should be incremented
-			oneOf(database).getMessagesByAuthor(txn, authorId);
-			will(returnValue(Collections.singletonList(messageId)));
-			oneOf(database).getSendability(txn, messageId);
-			will(returnValue(0));
-			oneOf(database).setSendability(txn, messageId, 1);
-			// The parent exists
-			oneOf(database).getGroup(txn, messageId);
-			will(returnValue(groupId));
-			oneOf(database).getParent(txn, messageId);
-			will(returnValue(parentId));
-			// The parent isn't in the DB
-			oneOf(database).containsMessage(txn, parentId);
-			will(returnValue(false));
-			oneOf(database).commitTransaction(txn);
-		}});
-		DatabaseComponent db = createDatabaseComponent(database, cleaner);
-
-		db.setRating(authorId, Rating.GOOD);
-
-		context.assertIsSatisfied();
-	}
-
-	@Test
-	public void testParentInAnotherGroupStopsBackwardInclusion()
-	throws DbException {
-		final GroupId groupId1 = new GroupId(TestUtils.getRandomId());
-		Mockery context = new Mockery();
-		@SuppressWarnings("unchecked")
-		final Database<Object> database = context.mock(Database.class);
-		final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
-		context.checking(new Expectations() {{
-			// setRating(Rating.GOOD)
-			oneOf(database).startTransaction();
-			will(returnValue(txn));
-			oneOf(database).setRating(txn, authorId, Rating.GOOD);
-			// The sendability of the author's messages should be incremented
-			oneOf(database).getMessagesByAuthor(txn, authorId);
-			will(returnValue(Collections.singletonList(messageId)));
-			oneOf(database).getSendability(txn, messageId);
-			will(returnValue(0));
-			oneOf(database).setSendability(txn, messageId, 1);
-			// The parent exists and is in the database
-			oneOf(database).getGroup(txn, messageId);
-			will(returnValue(groupId));
-			oneOf(database).getParent(txn, messageId);
-			will(returnValue(parentId));
-			oneOf(database).containsMessage(txn, parentId);
-			will(returnValue(true));
-			// The parent is in a different group
-			oneOf(database).getGroup(txn, parentId);
-			will(returnValue(groupId1));
-			oneOf(database).commitTransaction(txn);
-		}});
-		DatabaseComponent db = createDatabaseComponent(database, cleaner);
-
-		db.setRating(authorId, Rating.GOOD);
-
-		context.assertIsSatisfied();
-	}
-
-	@Test
-	public void testPrivateParentStopsBackwardInclusion() throws DbException {
-		Mockery context = new Mockery();
-		@SuppressWarnings("unchecked")
-		final Database<Object> database = context.mock(Database.class);
-		final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
-		context.checking(new Expectations() {{
-			// setRating(Rating.GOOD)
-			oneOf(database).startTransaction();
-			will(returnValue(txn));
-			oneOf(database).setRating(txn, authorId, Rating.GOOD);
-			// The sendability of the author's messages should be incremented
-			oneOf(database).getMessagesByAuthor(txn, authorId);
-			will(returnValue(Collections.singletonList(messageId)));
-			oneOf(database).getSendability(txn, messageId);
-			will(returnValue(0));
-			oneOf(database).setSendability(txn, messageId, 1);
-			// The parent exists and is in the database
-			oneOf(database).getGroup(txn, messageId);
-			will(returnValue(groupId));
-			oneOf(database).getParent(txn, messageId);
-			will(returnValue(parentId));
-			oneOf(database).containsMessage(txn, parentId);
-			will(returnValue(true));
-			// The parent is a private message
-			oneOf(database).getGroup(txn, parentId);
+			oneOf(database).getGroupMessageParent(txn, messageId);
 			will(returnValue(null));
 			oneOf(database).commitTransaction(txn);
 		}});
@@ -340,14 +230,8 @@ public abstract class DatabaseComponentTest extends TestCase {
 			will(returnValue(0));
 			oneOf(database).setSendability(txn, messageId, 1);
 			// The parent exists, is in the DB, and is in the same group
-			oneOf(database).getGroup(txn, messageId);
-			will(returnValue(groupId));
-			oneOf(database).getParent(txn, messageId);
+			oneOf(database).getGroupMessageParent(txn, messageId);
 			will(returnValue(parentId));
-			oneOf(database).containsMessage(txn, parentId);
-			will(returnValue(true));
-			oneOf(database).getGroup(txn, parentId);
-			will(returnValue(groupId));
 			// The parent is already sendable
 			oneOf(database).getSendability(txn, parentId);
 			will(returnValue(1));
@@ -380,19 +264,14 @@ public abstract class DatabaseComponentTest extends TestCase {
 			will(returnValue(0));
 			oneOf(database).setSendability(txn, messageId, 1);
 			// The parent exists, is in the DB, and is in the same group
-			oneOf(database).getGroup(txn, messageId);
-			will(returnValue(groupId));
-			oneOf(database).getParent(txn, messageId);
+			oneOf(database).getGroupMessageParent(txn, messageId);
 			will(returnValue(parentId));
-			oneOf(database).containsMessage(txn, parentId);
-			will(returnValue(true));
-			oneOf(database).getGroup(txn, parentId);
-			will(returnValue(groupId));
 			// The parent is not already sendable
 			oneOf(database).getSendability(txn, parentId);
 			will(returnValue(0));
 			oneOf(database).setSendability(txn, parentId, 1);
-			oneOf(database).getParent(txn, parentId);
+			// The parent has no parent
+			oneOf(database).getGroupMessageParent(txn, parentId);
 			will(returnValue(null));
 			oneOf(database).commitTransaction(txn);
 		}});
@@ -505,9 +384,7 @@ public abstract class DatabaseComponentTest extends TestCase {
 			will(returnValue(2));
 			oneOf(database).setSendability(txn, messageId, 3);
 			// The sendability of the message's ancestors should be updated
-			oneOf(database).getGroup(txn, messageId);
-			will(returnValue(groupId));
-			oneOf(database).getParent(txn, messageId);
+			oneOf(database).getGroupMessageParent(txn, messageId);
 			will(returnValue(null));
 			oneOf(database).commitTransaction(txn);
 		}});
@@ -1155,9 +1032,7 @@ public abstract class DatabaseComponentTest extends TestCase {
 			oneOf(database).getNumberOfSendableChildren(txn, messageId);
 			will(returnValue(1));
 			oneOf(database).setSendability(txn, messageId, 2);
-			oneOf(database).getGroup(txn, messageId);
-			will(returnValue(groupId));
-			oneOf(database).getParent(txn, messageId);
+			oneOf(database).getGroupMessageParent(txn, messageId);
 			will(returnValue(null));
 			// The batch needs to be acknowledged
 			oneOf(batch).getId();

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

@@ -1430,6 +1430,127 @@ public class H2DatabaseTest extends TestCase {
 		db.close();
 	}
 
+	@Test
+	public void testGetGroupMessageParentWithNoParent() throws DbException {
+		Database<Connection> db = open(false);
+		Connection txn = db.startTransaction();
+
+		// Subscribe to a group
+		db.addSubscription(txn, group);
+
+		// A message with no parent should return null
+		MessageId childId = new MessageId(TestUtils.getRandomId());
+		Message child = new TestMessage(childId, null, groupId, null,
+				timestamp, raw);
+		db.addGroupMessage(txn, child);
+		assertTrue(db.containsMessage(txn, childId));
+		assertNull(db.getGroupMessageParent(txn, childId));
+
+		db.commitTransaction(txn);
+		db.close();
+	}
+
+	@Test
+	public void testGetGroupMessageParentWithAbsentParent() throws DbException {
+		Database<Connection> db = open(false);
+		Connection txn = db.startTransaction();
+
+		// Subscribe to a group
+		db.addSubscription(txn, group);
+
+		// A message with an absent parent should return null
+		MessageId childId = new MessageId(TestUtils.getRandomId());
+		MessageId parentId = new MessageId(TestUtils.getRandomId());
+		Message child = new TestMessage(childId, parentId, groupId, null,
+				timestamp, raw);
+		db.addGroupMessage(txn, child);
+		assertTrue(db.containsMessage(txn, childId));
+		assertFalse(db.containsMessage(txn, parentId));
+		assertNull(db.getGroupMessageParent(txn, childId));
+
+		db.commitTransaction(txn);
+		db.close();
+	}
+
+	@Test
+	public void testGetGroupMessageParentWithParentInAnotherGroup()
+	throws DbException {
+		GroupId groupId1 = new GroupId(TestUtils.getRandomId());
+		Group group1 = groupFactory.createGroup(groupId1, "Group name", null);
+		Database<Connection> db = open(false);
+		Connection txn = db.startTransaction();
+
+		// Subscribe to two groups
+		db.addSubscription(txn, group);
+		db.addSubscription(txn, group1);
+
+		// A message with a parent in another group should return null
+		MessageId childId = new MessageId(TestUtils.getRandomId());
+		MessageId parentId = new MessageId(TestUtils.getRandomId());
+		Message child = new TestMessage(childId, parentId, groupId, null,
+				timestamp, raw);
+		Message parent = new TestMessage(parentId, null, groupId1, null,
+				timestamp, raw);
+		db.addGroupMessage(txn, child);
+		db.addGroupMessage(txn, parent);
+		assertTrue(db.containsMessage(txn, childId));
+		assertTrue(db.containsMessage(txn, parentId));
+		assertNull(db.getGroupMessageParent(txn, childId));
+
+		db.commitTransaction(txn);
+		db.close();
+	}
+
+	@Test
+	public void testGetGroupMessageParentWithPrivateParent()
+	throws DbException {
+		Database<Connection> db = open(false);
+		Connection txn = db.startTransaction();
+
+		// Add a contact and subscribe to a group
+		assertEquals(contactId, db.addContact(txn, transports, secret));
+		db.addSubscription(txn, group);
+
+		// A message with a private parent should return null
+		MessageId childId = new MessageId(TestUtils.getRandomId());
+		Message child = new TestMessage(childId, privateMessageId, groupId, null,
+				timestamp, raw);
+		db.addGroupMessage(txn, child);
+		db.addPrivateMessage(txn, privateMessage, contactId);
+		assertTrue(db.containsMessage(txn, childId));
+		assertTrue(db.containsMessage(txn, privateMessageId));
+		assertNull(db.getGroupMessageParent(txn, childId));
+
+		db.commitTransaction(txn);
+		db.close();
+	}
+
+	@Test
+	public void testGetGroupMessageParentWithParentInSameGroup()
+	throws DbException {
+		Database<Connection> db = open(false);
+		Connection txn = db.startTransaction();
+
+		// Subscribe to a group
+		db.addSubscription(txn, group);
+
+		// A message with a parent in the same group should return the parent
+		MessageId childId = new MessageId(TestUtils.getRandomId());
+		MessageId parentId = new MessageId(TestUtils.getRandomId());
+		Message child = new TestMessage(childId, parentId, groupId, null,
+				timestamp, raw);
+		Message parent = new TestMessage(parentId, null, groupId, null,
+				timestamp, raw);
+		db.addGroupMessage(txn, child);
+		db.addGroupMessage(txn, parent);
+		assertTrue(db.containsMessage(txn, childId));
+		assertTrue(db.containsMessage(txn, parentId));
+		assertEquals(parentId, db.getGroupMessageParent(txn, childId));
+
+		db.commitTransaction(txn);
+		db.close();
+	}
+
 	@Test
 	public void testExceptionHandling() throws DbException {
 		Database<Connection> db = open(false);