diff --git a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java index c7c427db877f883f30fb94928444751fa76d420d..84cca32ea7d2333d80d60b4b7a4b3654ce5face5 100644 --- a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java +++ b/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; diff --git a/briar-api/src/org/briarproject/api/db/MessageExistsException.java b/briar-api/src/org/briarproject/api/db/MessageExistsException.java new file mode 100644 index 0000000000000000000000000000000000000000..7a3a6d2e00f5f42737bf9b78a3e72d553056feeb --- /dev/null +++ b/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 { +} diff --git a/briar-core/src/org/briarproject/db/Database.java b/briar-core/src/org/briarproject/db/Database.java index 21ac0c61e00cf7d8cdde57826a2a895dbf670a78..f512b855303e249fc994924cd5baa80737694241 100644 --- a/briar-core/src/org/briarproject/db/Database.java +++ b/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. diff --git a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java index 250fa4dbfae596240964e4d604e3dcafea1f84e2..3378b8ff537e468bc58a476ddf6bf3cf922daf48 100644 --- a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java +++ b/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) { diff --git a/briar-core/src/org/briarproject/db/JdbcDatabase.java b/briar-core/src/org/briarproject/db/JdbcDatabase.java index ce44fb1411cc98be030e0d83c39f255d9113b088..80c9a1dd90d26ca7b86bfcf720c792bce3851431 100644 --- a/briar-core/src/org/briarproject/db/JdbcDatabase.java +++ b/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(); diff --git a/briar-core/src/org/briarproject/forum/ForumManagerImpl.java b/briar-core/src/org/briarproject/forum/ForumManagerImpl.java index c73bbde1864ac1ad1f8f89595db7581821aaf18f..ba604e93809ff82de3d26a2503268bcbe3834c62 100644 --- a/briar-core/src/org/briarproject/forum/ForumManagerImpl.java +++ b/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); diff --git a/briar-core/src/org/briarproject/messaging/MessagingManagerImpl.java b/briar-core/src/org/briarproject/messaging/MessagingManagerImpl.java index 1ef1b7b61a6e5400e99372c94faa6fdf97cfe583..39ba71647e09b77b985d94a39b9b0b777f1e6a7d 100644 --- a/briar-core/src/org/briarproject/messaging/MessagingManagerImpl.java +++ b/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(); } diff --git a/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java b/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java index c55c32343a9e8e196bbe988f59a489f27b798581..a6bd31ab33e861eda3e7a2803a8fa01b9126a80c 100644 --- a/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java +++ b/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(); } diff --git a/briar-tests/src/org/briarproject/db/H2DatabaseTest.java b/briar-tests/src/org/briarproject/db/H2DatabaseTest.java index 1302e15965e00cafabde69d9da06ef51b255b37e..f034e3f4be0165ea7d47cce36fd08af602427076 100644 --- a/briar-tests/src/org/briarproject/db/H2DatabaseTest.java +++ b/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();