Small improvements to DB interface.

briar-api/src/org/briarproject/api/db/DatabaseComponent.java

@@ -46,7 +46,7 @@ public interface DatabaseComponent {
 	ContactId addContact(Author remote, AuthorId local) throws DbException;
 
 	/** Adds a group to the given contact's subscriptions. */
-	void addGroup(ContactId c, Group g) throws DbException;
+	void addContactGroup(ContactId c, Group g) throws DbException;
 
 	/**
 	 * Subscribes to a group, or returns false if the user already has the
@@ -140,8 +140,11 @@ public interface DatabaseComponent {
 	Collection<TransportUpdate> generateTransportUpdates(ContactId c,
 			int maxLatency) throws DbException;
 
-	/** Returns all groups to which the user could subscribe. */
-	Collection<Group> getAvailableGroups() throws DbException;
+	/**
+	 * Returns all groups belonging to the given client to which the user could
+	 * subscribe.
+	 */
+	Collection<Group> getAvailableGroups(ClientId c) throws DbException;
 
 	/** Returns the contact with the given ID. */
 	Contact getContact(ContactId c) throws DbException;
@@ -152,8 +155,11 @@ public interface DatabaseComponent {
 	/** Returns the group with the given ID, if the user subscribes to it. */
 	Group getGroup(GroupId g) throws DbException;
 
-	/** Returns all groups to which the user subscribes. */
-	Collection<Group> getGroups() throws DbException;
+	/**
+	 * Returns all groups belonging to the given client to which the user
+	 * subscribes.
+	 */
+	Collection<Group> getGroups(ClientId c) throws DbException;
 
 	/** Returns the local pseudonym with the given ID. */
 	LocalAuthor getLocalAuthor(AuthorId a) throws DbException;

briar-api/src/org/briarproject/api/db/MessageExistsException.java

@@ -0,0 +1,8 @@
+package org.briarproject.api.db;
+
+/**
+ * Thrown when a duplicate message is added to the database. This exception may
+ * occur due to concurrent updates and does not indicate a database error.
+ */
+public class MessageExistsException extends DbException {
+}

briar-core/src/org/briarproject/db/Database.java

@@ -92,7 +92,7 @@ interface Database<T> {
 	 * <p>
 	 * Locking: write.
 	 */
-	void addGroup(T txn, ContactId c, Group g) throws DbException;
+	void addContactGroup(T txn, ContactId c, Group g) throws DbException;
 
 	/**
 	 * Subscribes to a group, or returns false if the user already has the
@@ -225,11 +225,12 @@ interface Database<T> {
 	int countOfferedMessages(T txn, ContactId c) throws DbException;
 
 	/**
-	 * Returns all groups to which the user could subscribe.
+	 * Returns all groups belonging to the given client to which the user could
+	 * subscribe.
 	 * <p>
 	 * Locking: read.
 	 */
-	Collection<Group> getAvailableGroups(T txn) throws DbException;
+	Collection<Group> getAvailableGroups(T txn, ClientId c) throws DbException;
 
 	/**
 	 * Returns the contact with the given ID.
@@ -274,11 +275,12 @@ interface Database<T> {
 	Group getGroup(T txn, GroupId g) throws DbException;
 
 	/**
-	 * Returns all groups to which the user subscribes.
+	 * Returns all groups belonging to the given client to which the user
+	 * subscribes.
 	 * <p>
 	 * Locking: read.
 	 */
-	Collection<Group> getGroups(T txn) throws DbException;
+	Collection<Group> getGroups(T txn, ClientId c) throws DbException;
 
 	/**
 	 * Returns the local pseudonym with the given ID.

briar-core/src/org/briarproject/db/DatabaseComponentImpl.java

@@ -9,6 +9,7 @@ import org.briarproject.api.db.ContactExistsException;
 import org.briarproject.api.db.DatabaseComponent;
 import org.briarproject.api.db.DbException;
 import org.briarproject.api.db.LocalAuthorExistsException;
+import org.briarproject.api.db.MessageExistsException;
 import org.briarproject.api.db.Metadata;
 import org.briarproject.api.db.NoSuchContactException;
 import org.briarproject.api.db.NoSuchLocalAuthorException;
@@ -168,12 +169,12 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 		return c;
 	}
 
-	public void addGroup(ContactId c, Group g) throws DbException {
+	public void addContactGroup(ContactId c, Group g) throws DbException {
 		lock.writeLock().lock();
 		try {
 			T txn = db.startTransaction();
 			try {
-				db.addGroup(txn, c, g);
+				db.addContactGroup(txn, c, g);
 				db.commitTransaction(txn);
 			} catch (DbException e) {
 				db.abortTransaction(txn);
@@ -225,17 +226,16 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 
 	public void addLocalMessage(Message m, ClientId c, Metadata meta)
 			throws DbException {
-		boolean duplicate, subscribed;
 		lock.writeLock().lock();
 		try {
 			T txn = db.startTransaction();
 			try {
-				duplicate = db.containsMessage(txn, m.getId());
-				subscribed = db.containsGroup(txn, m.getGroupId());
-				if (!duplicate && subscribed) {
-					addMessage(txn, m, null);
-					db.mergeMessageMetadata(txn, m.getId(), meta);
-				}
+				if (db.containsMessage(txn, m.getId()))
+					throw new MessageExistsException();
+				if (!db.containsGroup(txn, m.getGroupId()))
+					throw new NoSuchSubscriptionException();
+				addMessage(txn, m, null);
+				db.mergeMessageMetadata(txn, m.getId(), meta);
 				db.commitTransaction(txn);
 			} catch (DbException e) {
 				db.abortTransaction(txn);
@@ -244,10 +244,8 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 		} finally {
 			lock.writeLock().unlock();
 		}
-		if (!duplicate && subscribed) {
-			eventBus.broadcast(new MessageAddedEvent(m, null));
-			eventBus.broadcast(new MessageValidatedEvent(m, c, true, true));
-		}
+		eventBus.broadcast(new MessageAddedEvent(m, null));
+		eventBus.broadcast(new MessageValidatedEvent(m, c, true, true));
 	}
 
 	/**
@@ -524,12 +522,12 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 		}
 	}
 
-	public Collection<Group> getAvailableGroups() throws DbException {
+	public Collection<Group> getAvailableGroups(ClientId c) throws DbException {
 		lock.readLock().lock();
 		try {
 			T txn = db.startTransaction();
 			try {
-				Collection<Group> groups = db.getAvailableGroups(txn);
+				Collection<Group> groups = db.getAvailableGroups(txn, c);
 				db.commitTransaction(txn);
 				return groups;
 			} catch (DbException e) {
@@ -596,12 +594,12 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 		}
 	}
 
-	public Collection<Group> getGroups() throws DbException {
+	public Collection<Group> getGroups(ClientId c) throws DbException {
 		lock.readLock().lock();
 		try {
 			T txn = db.startTransaction();
 			try {
-				Collection<Group> groups = db.getGroups(txn);
+				Collection<Group> groups = db.getGroups(txn, c);
 				db.commitTransaction(txn);
 				return groups;
 			} catch (DbException e) {

briar-core/src/org/briarproject/db/JdbcDatabase.java

@@ -647,7 +647,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
-	public void addGroup(Connection txn, ContactId c, Group g)
+	public void addContactGroup(Connection txn, ContactId c, Group g)
 			throws DbException {
 		PreparedStatement ps = null;
 		ResultSet rs = null;
@@ -1155,28 +1155,28 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
-	public Collection<Group> getAvailableGroups(Connection txn)
+	public Collection<Group> getAvailableGroups(Connection txn, ClientId c)
 			throws DbException {
 		PreparedStatement ps = null;
 		ResultSet rs = null;
 		try {
-			String sql = "SELECT DISTINCT"
-					+ " cg.groupId, cg.clientId, cg.descriptor"
+			String sql = "SELECT DISTINCT cg.groupId, cg.descriptor"
 					+ " FROM contactGroups AS cg"
 					+ " LEFT OUTER JOIN groups AS g"
 					+ " ON cg.groupId = g.groupId"
-					+ " WHERE g.groupId IS NULL"
+					+ " WHERE cg.clientId = ?"
+					+ " AND g.groupId IS NULL"
 					+ " GROUP BY cg.groupId";
 			ps = txn.prepareStatement(sql);
+			ps.setBytes(1, c.getBytes());
 			rs = ps.executeQuery();
 			List<Group> groups = new ArrayList<Group>();
 			Set<GroupId> ids = new HashSet<GroupId>();
 			while (rs.next()) {
 				GroupId id = new GroupId(rs.getBytes(1));
 				if (!ids.add(id)) throw new DbStateException();
-				ClientId clientId = new ClientId(rs.getBytes(2));
-				byte[] descriptor = rs.getBytes(3);
-				groups.add(new Group(id, clientId, descriptor));
+				byte[] descriptor = rs.getBytes(2);
+				groups.add(new Group(id, c, descriptor));
 			}
 			rs.close();
 			ps.close();
@@ -1308,19 +1308,21 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
-	public Collection<Group> getGroups(Connection txn) throws DbException {
+	public Collection<Group> getGroups(Connection txn, ClientId c)
+			throws DbException {
 		PreparedStatement ps = null;
 		ResultSet rs = null;
 		try {
-			String sql = "SELECT groupId, clientId, descriptor FROM groups";
+			String sql = "SELECT groupId, descriptor FROM groups"
+					+ " WHERE clientId = ?";
 			ps = txn.prepareStatement(sql);
+			ps.setBytes(1, c.getBytes());
 			rs = ps.executeQuery();
 			List<Group> groups = new ArrayList<Group>();
 			while (rs.next()) {
 				GroupId id = new GroupId(rs.getBytes(1));
-				ClientId clientId = new ClientId(rs.getBytes(2));
-				byte[] descriptor = rs.getBytes(3);
-				groups.add(new Group(id, clientId, descriptor));
+				byte[] descriptor = rs.getBytes(2);
+				groups.add(new Group(id, c, descriptor));
 			}
 			rs.close();
 			ps.close();

briar-core/src/org/briarproject/forum/ForumManagerImpl.java

@@ -143,17 +143,13 @@ class ForumManagerImpl implements ForumManager {
 
 	@Override
 	public Collection<Forum> getAvailableForums() throws DbException {
-		// TODO: Get groups by client ID
-		Collection<Group> groups = db.getAvailableGroups();
+		Collection<Group> groups = db.getAvailableGroups(CLIENT_ID);
 		List<Forum> forums = new ArrayList<Forum>(groups.size());
 		for (Group g : groups) {
-			if (g.getClientId().equals(CLIENT_ID)) {
-				try {
-					forums.add(parseForum(g));
-				} catch (FormatException e) {
-					if (LOG.isLoggable(WARNING))
-						LOG.log(WARNING, e.toString(), e);
-				}
+			try {
+				forums.add(parseForum(g));
+			} catch (FormatException e) {
+				if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 			}
 		}
 		return Collections.unmodifiableList(forums);
@@ -193,17 +189,13 @@ class ForumManagerImpl implements ForumManager {
 
 	@Override
 	public Collection<Forum> getForums() throws DbException {
-		// TODO: Get groups by client ID
-		Collection<Group> groups = db.getGroups();
+		Collection<Group> groups = db.getGroups(CLIENT_ID);
 		List<Forum> forums = new ArrayList<Forum>(groups.size());
 		for (Group g : groups) {
-			if (g.getClientId().equals(CLIENT_ID)) {
-				try {
-					forums.add(parseForum(g));
-				} catch (FormatException e) {
-					if (LOG.isLoggable(WARNING))
-						LOG.log(WARNING, e.toString(), e);
-				}
+			try {
+				forums.add(parseForum(g));
+			} catch (FormatException e) {
+				if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 			}
 		}
 		return Collections.unmodifiableList(forums);

briar-core/src/org/briarproject/messaging/MessagingManagerImpl.java

@@ -82,7 +82,7 @@ class MessagingManagerImpl implements MessagingManager {
 		Group conversation = createConversationGroup(db.getContact(c));
 		// Subscribe to the group and share it with the contact
 		db.addGroup(conversation);
-		db.addGroup(c, conversation);
+		db.addContactGroup(c, conversation);
 		db.setVisibility(conversation.getId(), Collections.singletonList(c));
 	}
 
@@ -141,7 +141,6 @@ class MessagingManagerImpl implements MessagingManager {
 
 	@Override
 	public GroupId getConversationId(ContactId c) throws DbException {
-		// TODO: Make this more efficient
 		return createConversationGroup(db.getContact(c)).getId();
 	}
 

briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java

@@ -9,6 +9,7 @@ import org.briarproject.api.contact.Contact;
 import org.briarproject.api.contact.ContactId;
 import org.briarproject.api.crypto.SecretKey;
 import org.briarproject.api.db.DatabaseComponent;
+import org.briarproject.api.db.MessageExistsException;
 import org.briarproject.api.db.Metadata;
 import org.briarproject.api.db.NoSuchContactException;
 import org.briarproject.api.db.NoSuchLocalAuthorException;
@@ -166,7 +167,7 @@ public class DatabaseComponentImplTest extends BriarTestCase {
 			oneOf(database).containsGroup(txn, groupId);
 			will(returnValue(true));
 			// getGroups()
-			oneOf(database).getGroups(txn);
+			oneOf(database).getGroups(txn, clientId);
 			will(returnValue(Collections.singletonList(group)));
 			// removeGroup()
 			oneOf(database).containsGroup(txn, groupId);
@@ -205,7 +206,7 @@ public class DatabaseComponentImplTest extends BriarTestCase {
 				db.getRemoteProperties(transportId));
 		db.addGroup(group); // First time - listeners called
 		db.addGroup(group); // Second time - not called
-		assertEquals(Collections.singletonList(group), db.getGroups());
+		assertEquals(Collections.singletonList(group), db.getGroups(clientId));
 		db.removeGroup(group);
 		db.removeContact(contactId);
 		db.removeLocalAuthor(localAuthorId);
@@ -226,14 +227,17 @@ public class DatabaseComponentImplTest extends BriarTestCase {
 			will(returnValue(txn));
 			oneOf(database).containsMessage(txn, messageId);
 			will(returnValue(true));
-			oneOf(database).containsGroup(txn, groupId);
-			will(returnValue(true));
-			oneOf(database).commitTransaction(txn);
+			oneOf(database).abortTransaction(txn);
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, eventBus,
 				shutdown);
 
-		db.addLocalMessage(message, clientId, metadata);
+		try {
+			db.addLocalMessage(message, clientId, metadata);
+			fail();
+		} catch (MessageExistsException expected) {
+			// Expected
+		}
 
 		context.assertIsSatisfied();
 	}
@@ -253,12 +257,17 @@ public class DatabaseComponentImplTest extends BriarTestCase {
 			will(returnValue(false));
 			oneOf(database).containsGroup(txn, groupId);
 			will(returnValue(false));
-			oneOf(database).commitTransaction(txn);
+			oneOf(database).abortTransaction(txn);
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, eventBus,
 				shutdown);
 
-		db.addLocalMessage(message, clientId, metadata);
+		try {
+			db.addLocalMessage(message, clientId, metadata);
+			fail();
+		} catch (NoSuchSubscriptionException expected) {
+			// Expected
+		}
 
 		context.assertIsSatisfied();
 	}

briar-tests/src/org/briarproject/db/H2DatabaseTest.java

@@ -58,6 +58,7 @@ public class H2DatabaseTest extends BriarTestCase {
 
 	private final File testDir = TestUtils.getTestDirectory();
 	private final Random random = new Random();
+	private final ClientId clientId;
 	private final GroupId groupId;
 	private final Group group;
 	private final Author author;
@@ -72,8 +73,8 @@ public class H2DatabaseTest extends BriarTestCase {
 	private final ContactId contactId;
 
 	public H2DatabaseTest() throws Exception {
+		clientId = new ClientId(TestUtils.getRandomId());
 		groupId = new GroupId(TestUtils.getRandomId());
-		ClientId clientId = new ClientId(TestUtils.getRandomId());
 		byte[] descriptor = new byte[MAX_GROUP_DESCRIPTOR_LENGTH];
 		group = new Group(groupId, clientId, descriptor);
 		AuthorId authorId = new AuthorId(TestUtils.getRandomId());
@@ -901,31 +902,34 @@ public class H2DatabaseTest extends BriarTestCase {
 		db.setGroups(txn, contactId1, Collections.singletonList(group), 1);
 
 		// The group should be available
-		assertEquals(Collections.emptyList(), db.getGroups(txn));
+		assertEquals(Collections.emptyList(), db.getGroups(txn, clientId));
 		assertEquals(Collections.singletonList(group),
-				db.getAvailableGroups(txn));
+				db.getAvailableGroups(txn, clientId));
 
 		// Subscribe to the group - it should no longer be available
 		db.addGroup(txn, group);
-		assertEquals(Collections.singletonList(group), db.getGroups(txn));
-		assertEquals(Collections.emptyList(), db.getAvailableGroups(txn));
+		assertEquals(Collections.singletonList(group),
+				db.getGroups(txn, clientId));
+		assertEquals(Collections.emptyList(),
+				db.getAvailableGroups(txn, clientId));
 
 		// Unsubscribe from the group - it should be available again
 		db.removeGroup(txn, groupId);
-		assertEquals(Collections.emptyList(), db.getGroups(txn));
+		assertEquals(Collections.emptyList(), db.getGroups(txn, clientId));
 		assertEquals(Collections.singletonList(group),
-				db.getAvailableGroups(txn));
+				db.getAvailableGroups(txn, clientId));
 
 		// The first contact unsubscribes - it should still be available
 		db.setGroups(txn, contactId, Collections.<Group>emptyList(), 2);
-		assertEquals(Collections.emptyList(), db.getGroups(txn));
+		assertEquals(Collections.emptyList(), db.getGroups(txn, clientId));
 		assertEquals(Collections.singletonList(group),
-				db.getAvailableGroups(txn));
+				db.getAvailableGroups(txn, clientId));
 
 		// The second contact unsubscribes - it should no longer be available
 		db.setGroups(txn, contactId1, Collections.<Group>emptyList(), 2);
-		assertEquals(Collections.emptyList(), db.getGroups(txn));
-		assertEquals(Collections.emptyList(), db.getAvailableGroups(txn));
+		assertEquals(Collections.emptyList(), db.getGroups(txn, clientId));
+		assertEquals(Collections.emptyList(),
+				db.getAvailableGroups(txn, clientId));
 
 		db.commitTransaction(txn);
 		db.close();