diff --git a/components/net/sf/briar/db/Database.java b/components/net/sf/briar/db/Database.java
index 7065e02332376062b6edba2dbc4854f540fb8d59..f43a98cde8019f4edafc83bcb659d99604dd6732 100644
--- a/components/net/sf/briar/db/Database.java
+++ b/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>
diff --git a/components/net/sf/briar/db/DatabaseComponentImpl.java b/components/net/sf/briar/db/DatabaseComponentImpl.java
index 11ad89ca39fcb034f340d8243eff2c09920bcd1e..042b714f21392c90bb7af9db73c990af95e1f955 100644
--- a/components/net/sf/briar/db/DatabaseComponentImpl.java
+++ b/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;
}
diff --git a/components/net/sf/briar/db/JdbcDatabase.java b/components/net/sf/briar/db/JdbcDatabase.java
index 3b0b235ccf6e9a869e042ca86ceed7bb13bf1c19..c33d6fcc874d0f60c05ec7f2c6b353857679ea1c 100644
--- a/components/net/sf/briar/db/JdbcDatabase.java
+++ b/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);
diff --git a/test/net/sf/briar/db/DatabaseComponentImplTest.java b/test/net/sf/briar/db/DatabaseComponentImplTest.java
index 6578ed1c48a7128dad18ead81cc6dc6b86317ad6..a7771e174eb9272ef8bdda1193a7dbc9d2b8138b 100644
--- a/test/net/sf/briar/db/DatabaseComponentImplTest.java
+++ b/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);
diff --git a/test/net/sf/briar/db/DatabaseComponentTest.java b/test/net/sf/briar/db/DatabaseComponentTest.java
index 13f49901e1d3904980b487c4836fe0f316f4389f..1e8d39f07eda0b8491a63cd6f50be6885a6a7346 100644
--- a/test/net/sf/briar/db/DatabaseComponentTest.java
+++ b/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();
diff --git a/test/net/sf/briar/db/H2DatabaseTest.java b/test/net/sf/briar/db/H2DatabaseTest.java
index bef871e2d3bba4d542bc814cf4946a4cdc9a37bb..131fd465cfba34455d62b7689d9e002571f18d6a 100644
--- a/test/net/sf/briar/db/H2DatabaseTest.java
+++ b/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);