From 53b5a61ab32c7061811a66424b54d3d28e374d74 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Mon, 26 Sep 2011 18:00:56 +0100
Subject: [PATCH] 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 | 14 +-
.../sf/briar/db/DatabaseComponentImpl.java | 13 +-
components/net/sf/briar/db/JdbcDatabase.java | 19 ++-
.../briar/db/DatabaseComponentImplTest.java | 4 +-
.../sf/briar/db/DatabaseComponentTest.java | 139 +-----------------
test/net/sf/briar/db/H2DatabaseTest.java | 121 +++++++++++++++
6 files changed, 149 insertions(+), 161 deletions(-)
diff --git a/components/net/sf/briar/db/Database.java b/components/net/sf/briar/db/Database.java
index 7065e02332..f43a98cde8 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 11ad89ca39..042b714f21 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 3b0b235ccf..c33d6fcc87 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 6578ed1c48..a7771e174e 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 13f49901e1..1e8d39f07e 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 bef871e2d3..131fd465cf 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);
--
GitLab