diff --git a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java index 7d50be928295542ae86e1d5477e5cd4c7658ac50..3f01a4d40b45ea30262388b56dc41682a3f4b092 100644 --- a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java +++ b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java @@ -55,14 +55,11 @@ public interface DatabaseComponent { void addLocalMessage(Message m, ClientId c, Metadata meta, boolean shared) throws DbException; - /** - * Stores a transport and returns true if the transport was not previously - * in the database. - */ - boolean addTransport(TransportId t, int maxLatency) throws DbException; + /** Stores a transport. */ + void addTransport(TransportId t, int maxLatency) throws DbException; /** - * Stores the given transport keys for a newly added contact. + * Stores transport keys for a newly added contact. */ void addTransportKeys(ContactId c, TransportKeys k) throws DbException; diff --git a/briar-api/src/org/briarproject/api/db/LocalAuthorExistsException.java b/briar-api/src/org/briarproject/api/db/LocalAuthorExistsException.java deleted file mode 100644 index 549140364a4b2e619096002c76fe240da224a9ee..0000000000000000000000000000000000000000 --- a/briar-api/src/org/briarproject/api/db/LocalAuthorExistsException.java +++ /dev/null @@ -1,10 +0,0 @@ -package org.briarproject.api.db; - -/** - * Thrown when a duplicate pseudonym is added to the database. This exception - * may occur due to concurrent updates and does not indicate a database error. - */ -public class LocalAuthorExistsException extends DbException { - - private static final long serialVersionUID = -1483877298070151673L; -} diff --git a/briar-api/src/org/briarproject/api/db/MessageExistsException.java b/briar-api/src/org/briarproject/api/db/MessageExistsException.java deleted file mode 100644 index 7a3a6d2e00f5f42737bf9b78a3e72d553056feeb..0000000000000000000000000000000000000000 --- a/briar-api/src/org/briarproject/api/db/MessageExistsException.java +++ /dev/null @@ -1,8 +0,0 @@ -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 4d147ccf46103707dd5acf0c97d9f21aac9f0279..e5b65296d33857f32fe3eb02c6e25249e2a7a04e 100644 --- a/briar-core/src/org/briarproject/db/Database.java +++ b/briar-core/src/org/briarproject/db/Database.java @@ -124,20 +124,20 @@ interface Database<T> { throws DbException; /** - * Stores a transport and returns true if the transport was not previously - * in the database. + * Stores a transport. * <p> * Locking: write. */ - boolean addTransport(T txn, TransportId t, int maxLatency) + void addTransport(T txn, TransportId t, int maxLatency) throws DbException; /** - * Stores the given transport keys for a newly added contact. + * Stores transport keys for a newly added contact. * <p> * Locking: write. */ - void addTransportKeys(T txn, ContactId c, TransportKeys k) throws DbException; + void addTransportKeys(T txn, ContactId c, TransportKeys k) + throws DbException; /** * Makes a group visible to the given contact. diff --git a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java index 649e47b2adcf7f41477a4f59795b4220a71e0a5c..44c177dc8226ea6ff90eb2ee9c9c10e6f41abfb9 100644 --- a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java +++ b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java @@ -7,8 +7,6 @@ import org.briarproject.api.contact.ContactId; 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.NoSuchGroupException; @@ -168,8 +166,8 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { T txn = db.startTransaction(); try { if (!db.containsGroup(txn, g.getId())) { - added = true; db.addGroup(txn, g); + added = true; } db.commitTransaction(txn); } catch (DbException e) { @@ -187,9 +185,9 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { try { T txn = db.startTransaction(); try { - if (db.containsLocalAuthor(txn, a.getId())) - throw new LocalAuthorExistsException(); - db.addLocalAuthor(txn, a); + if (!db.containsLocalAuthor(txn, a.getId())) { + db.addLocalAuthor(txn, a); + } db.commitTransaction(txn); } catch (DbException e) { db.abortTransaction(txn); @@ -202,15 +200,17 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { public void addLocalMessage(Message m, ClientId c, Metadata meta, boolean shared) throws DbException { + boolean added = false; lock.writeLock().lock(); try { T txn = db.startTransaction(); try { - if (db.containsMessage(txn, m.getId())) - throw new MessageExistsException(); if (!db.containsGroup(txn, m.getGroupId())) throw new NoSuchGroupException(); - addMessage(txn, m, VALID, shared, null); + if (!db.containsMessage(txn, m.getId())) { + addMessage(txn, m, VALID, shared, null); + added = true; + } db.mergeMessageMetadata(txn, m.getId(), meta); db.commitTransaction(txn); } catch (DbException e) { @@ -220,8 +220,10 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { } finally { lock.writeLock().unlock(); } - eventBus.broadcast(new MessageAddedEvent(m, null)); - eventBus.broadcast(new MessageValidatedEvent(m, c, true, true)); + if (added) { + eventBus.broadcast(new MessageAddedEvent(m, null)); + eventBus.broadcast(new MessageValidatedEvent(m, c, true, true)); + } } /** @@ -248,14 +250,16 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { } } - public boolean addTransport(TransportId t, int maxLatency) - throws DbException { - boolean added; + public void addTransport(TransportId t, int maxLatency) throws DbException { + boolean added = false; lock.writeLock().lock(); try { T txn = db.startTransaction(); try { - added = db.addTransport(txn, t, maxLatency); + if (!db.containsTransport(txn, t)) { + db.addTransport(txn, t, maxLatency); + added = true; + } db.commitTransaction(txn); } catch (DbException e) { db.abortTransaction(txn); @@ -265,7 +269,6 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { lock.writeLock().unlock(); } if (added) eventBus.broadcast(new TransportAddedEvent(t, maxLatency)); - return added; } public void addTransportKeys(ContactId c, TransportKeys k) @@ -909,8 +912,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { lock.writeLock().unlock(); } if (visible) { - if (!duplicate) - eventBus.broadcast(new MessageAddedEvent(m, c)); + if (!duplicate) eventBus.broadcast(new MessageAddedEvent(m, c)); eventBus.broadcast(new MessageToAckEvent(c)); } } diff --git a/briar-core/src/org/briarproject/db/JdbcDatabase.java b/briar-core/src/org/briarproject/db/JdbcDatabase.java index 30a4f32e4440630bde158c1a2aae2a7d4f6cea25..f922c548c724e2c3b5b783b3b0a9baef4336fbae 100644 --- a/briar-core/src/org/briarproject/db/JdbcDatabase.java +++ b/briar-core/src/org/briarproject/db/JdbcDatabase.java @@ -661,23 +661,11 @@ abstract class JdbcDatabase implements Database<Connection> { } } - public boolean addTransport(Connection txn, TransportId t, int maxLatency) + public void addTransport(Connection txn, TransportId t, int maxLatency) throws DbException { PreparedStatement ps = null; - ResultSet rs = null; try { - // Return false if the transport is already in the database - String sql = "SELECT NULL FROM transports WHERE transportId = ?"; - ps = txn.prepareStatement(sql); - ps.setString(1, t.getString()); - rs = ps.executeQuery(); - boolean found = rs.next(); - if (rs.next()) throw new DbStateException(); - rs.close(); - ps.close(); - if (found) return false; - // Create a transport row - sql = "INSERT INTO transports (transportId, maxLatency)" + String sql = "INSERT INTO transports (transportId, maxLatency)" + " VALUES (?, ?)"; ps = txn.prepareStatement(sql); ps.setString(1, t.getString()); @@ -685,10 +673,8 @@ abstract class JdbcDatabase implements Database<Connection> { int affected = ps.executeUpdate(); if (affected != 1) throw new DbStateException(); ps.close(); - return true; } catch (SQLException e) { tryToClose(ps); - tryToClose(rs); throw new DbException(e); } } diff --git a/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java b/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java index a4128df5600883fa2b60e346ff5c781bfcb11552..238b62970b05dd3fcda690283a7aa81545117332 100644 --- a/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java +++ b/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java @@ -7,7 +7,6 @@ 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.NoSuchGroupException; @@ -192,33 +191,6 @@ public class DatabaseComponentImplTest extends BriarTestCase { context.assertIsSatisfied(); } - @Test - public void testDuplicateLocalMessagesAreNotStored() throws Exception { - Mockery context = new Mockery(); - @SuppressWarnings("unchecked") - final Database<Object> database = context.mock(Database.class); - final ShutdownManager shutdown = context.mock(ShutdownManager.class); - final EventBus eventBus = context.mock(EventBus.class); - context.checking(new Expectations() {{ - oneOf(database).startTransaction(); - will(returnValue(txn)); - oneOf(database).containsMessage(txn, messageId); - will(returnValue(true)); - oneOf(database).abortTransaction(txn); - }}); - DatabaseComponent db = createDatabaseComponent(database, eventBus, - shutdown); - - try { - db.addLocalMessage(message, clientId, metadata, true); - fail(); - } catch (MessageExistsException expected) { - // Expected - } - - context.assertIsSatisfied(); - } - @Test public void testLocalMessagesAreNotStoredUnlessGroupExists() throws Exception { @@ -230,8 +202,6 @@ public class DatabaseComponentImplTest extends BriarTestCase { context.checking(new Expectations() {{ oneOf(database).startTransaction(); will(returnValue(txn)); - oneOf(database).containsMessage(txn, messageId); - will(returnValue(false)); oneOf(database).containsGroup(txn, groupId); will(returnValue(false)); oneOf(database).abortTransaction(txn); @@ -259,10 +229,10 @@ public class DatabaseComponentImplTest extends BriarTestCase { context.checking(new Expectations() {{ oneOf(database).startTransaction(); will(returnValue(txn)); - oneOf(database).containsMessage(txn, messageId); - will(returnValue(false)); oneOf(database).containsGroup(txn, groupId); will(returnValue(true)); + oneOf(database).containsMessage(txn, messageId); + will(returnValue(false)); oneOf(database).addMessage(txn, message, VALID, true); oneOf(database).mergeMessageMetadata(txn, messageId, metadata); oneOf(database).getVisibility(txn, groupId); @@ -295,11 +265,11 @@ public class DatabaseComponentImplTest extends BriarTestCase { final EventBus eventBus = context.mock(EventBus.class); context.checking(new Expectations() {{ // Check whether the contact is in the DB (which it's not) - exactly(13).of(database).startTransaction(); + exactly(15).of(database).startTransaction(); will(returnValue(txn)); - exactly(13).of(database).containsContact(txn, contactId); + exactly(15).of(database).containsContact(txn, contactId); will(returnValue(false)); - exactly(13).of(database).abortTransaction(txn); + exactly(15).of(database).abortTransaction(txn); }}); DatabaseComponent db = createDatabaseComponent(database, eventBus, shutdown); @@ -332,6 +302,13 @@ public class DatabaseComponentImplTest extends BriarTestCase { // Expected } + try { + db.generateRequest(contactId, 123); + fail(); + } catch (NoSuchContactException expected) { + // Expected + } + try { db.getContact(contactId); fail(); @@ -383,6 +360,14 @@ public class DatabaseComponentImplTest extends BriarTestCase { // Expected } + try { + Request r = new Request(Collections.singletonList(messageId)); + db.receiveRequest(contactId, r); + fail(); + } catch (NoSuchContactException expected) { + // Expected + } + try { db.removeContact(contactId); fail(); diff --git a/briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java b/briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java index 4ec79282e5fe0d955b8e7ee32829ce1abdff1e6d..23fbddea8dc420ac9afa29677a0454b6c8ec8edf 100644 --- a/briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java +++ b/briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java @@ -83,7 +83,6 @@ public class PluginManagerImplTest extends BriarTestCase { oneOf(simplexPlugin).getMaxLatency(); will(returnValue(simplexLatency)); oneOf(db).addTransport(simplexId, simplexLatency); - will(returnValue(true)); oneOf(simplexPlugin).start(); will(returnValue(true)); // Started oneOf(simplexPlugin).shouldPoll(); @@ -98,7 +97,6 @@ public class PluginManagerImplTest extends BriarTestCase { oneOf(simplexFailPlugin).getMaxLatency(); will(returnValue(simplexFailLatency)); oneOf(db).addTransport(simplexFailId, simplexFailLatency); - will(returnValue(true)); oneOf(simplexFailPlugin).start(); will(returnValue(false)); // Failed to start // First duplex plugin @@ -112,7 +110,6 @@ public class PluginManagerImplTest extends BriarTestCase { oneOf(duplexPlugin).getMaxLatency(); will(returnValue(duplexLatency)); oneOf(db).addTransport(duplexId, duplexLatency); - will(returnValue(true)); oneOf(duplexPlugin).start(); will(returnValue(true)); // Started oneOf(duplexPlugin).shouldPoll();