DB interface cleanup, removed unnecessary exceptions.

This commit is contained in:
akwizgran
2016-01-28 15:45:49 +00:00
parent 3837efca6b
commit 225d0ebeef
8 changed files with 50 additions and 101 deletions

View File

@@ -55,14 +55,11 @@ public interface DatabaseComponent {
void addLocalMessage(Message m, ClientId c, Metadata meta, boolean shared) void addLocalMessage(Message m, ClientId c, Metadata meta, boolean shared)
throws DbException; throws DbException;
/** /** Stores a transport. */
* Stores a transport and returns true if the transport was not previously void addTransport(TransportId t, int maxLatency) throws DbException;
* in the database.
*/
boolean 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; void addTransportKeys(ContactId c, TransportKeys k) throws DbException;

View File

@@ -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;
}

View File

@@ -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 {
}

View File

@@ -124,20 +124,20 @@ interface Database<T> {
throws DbException; throws DbException;
/** /**
* Stores a transport and returns true if the transport was not previously * Stores a transport.
* in the database.
* <p> * <p>
* Locking: write. * Locking: write.
*/ */
boolean addTransport(T txn, TransportId t, int maxLatency) void addTransport(T txn, TransportId t, int maxLatency)
throws DbException; throws DbException;
/** /**
* Stores the given transport keys for a newly added contact. * Stores transport keys for a newly added contact.
* <p> * <p>
* Locking: write. * 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. * Makes a group visible to the given contact.

View File

@@ -7,8 +7,6 @@ import org.briarproject.api.contact.ContactId;
import org.briarproject.api.db.ContactExistsException; import org.briarproject.api.db.ContactExistsException;
import org.briarproject.api.db.DatabaseComponent; import org.briarproject.api.db.DatabaseComponent;
import org.briarproject.api.db.DbException; 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.Metadata;
import org.briarproject.api.db.NoSuchContactException; import org.briarproject.api.db.NoSuchContactException;
import org.briarproject.api.db.NoSuchGroupException; import org.briarproject.api.db.NoSuchGroupException;
@@ -168,8 +166,8 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
T txn = db.startTransaction(); T txn = db.startTransaction();
try { try {
if (!db.containsGroup(txn, g.getId())) { if (!db.containsGroup(txn, g.getId())) {
added = true;
db.addGroup(txn, g); db.addGroup(txn, g);
added = true;
} }
db.commitTransaction(txn); db.commitTransaction(txn);
} catch (DbException e) { } catch (DbException e) {
@@ -187,9 +185,9 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
try { try {
T txn = db.startTransaction(); T txn = db.startTransaction();
try { try {
if (db.containsLocalAuthor(txn, a.getId())) if (!db.containsLocalAuthor(txn, a.getId())) {
throw new LocalAuthorExistsException(); db.addLocalAuthor(txn, a);
db.addLocalAuthor(txn, a); }
db.commitTransaction(txn); db.commitTransaction(txn);
} catch (DbException e) { } catch (DbException e) {
db.abortTransaction(txn); db.abortTransaction(txn);
@@ -202,15 +200,17 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
public void addLocalMessage(Message m, ClientId c, Metadata meta, public void addLocalMessage(Message m, ClientId c, Metadata meta,
boolean shared) throws DbException { boolean shared) throws DbException {
boolean added = false;
lock.writeLock().lock(); lock.writeLock().lock();
try { try {
T txn = db.startTransaction(); T txn = db.startTransaction();
try { try {
if (db.containsMessage(txn, m.getId()))
throw new MessageExistsException();
if (!db.containsGroup(txn, m.getGroupId())) if (!db.containsGroup(txn, m.getGroupId()))
throw new NoSuchGroupException(); 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.mergeMessageMetadata(txn, m.getId(), meta);
db.commitTransaction(txn); db.commitTransaction(txn);
} catch (DbException e) { } catch (DbException e) {
@@ -220,8 +220,10 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
} finally { } finally {
lock.writeLock().unlock(); lock.writeLock().unlock();
} }
eventBus.broadcast(new MessageAddedEvent(m, null)); if (added) {
eventBus.broadcast(new MessageValidatedEvent(m, c, true, true)); 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) public void addTransport(TransportId t, int maxLatency) throws DbException {
throws DbException { boolean added = false;
boolean added;
lock.writeLock().lock(); lock.writeLock().lock();
try { try {
T txn = db.startTransaction(); T txn = db.startTransaction();
try { try {
added = db.addTransport(txn, t, maxLatency); if (!db.containsTransport(txn, t)) {
db.addTransport(txn, t, maxLatency);
added = true;
}
db.commitTransaction(txn); db.commitTransaction(txn);
} catch (DbException e) { } catch (DbException e) {
db.abortTransaction(txn); db.abortTransaction(txn);
@@ -265,7 +269,6 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
lock.writeLock().unlock(); lock.writeLock().unlock();
} }
if (added) eventBus.broadcast(new TransportAddedEvent(t, maxLatency)); if (added) eventBus.broadcast(new TransportAddedEvent(t, maxLatency));
return added;
} }
public void addTransportKeys(ContactId c, TransportKeys k) public void addTransportKeys(ContactId c, TransportKeys k)
@@ -909,8 +912,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
lock.writeLock().unlock(); lock.writeLock().unlock();
} }
if (visible) { if (visible) {
if (!duplicate) if (!duplicate) eventBus.broadcast(new MessageAddedEvent(m, c));
eventBus.broadcast(new MessageAddedEvent(m, c));
eventBus.broadcast(new MessageToAckEvent(c)); eventBus.broadcast(new MessageToAckEvent(c));
} }
} }

View File

@@ -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 { throws DbException {
PreparedStatement ps = null; PreparedStatement ps = null;
ResultSet rs = null;
try { try {
// Return false if the transport is already in the database String sql = "INSERT INTO transports (transportId, maxLatency)"
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)"
+ " VALUES (?, ?)"; + " VALUES (?, ?)";
ps = txn.prepareStatement(sql); ps = txn.prepareStatement(sql);
ps.setString(1, t.getString()); ps.setString(1, t.getString());
@@ -685,10 +673,8 @@ abstract class JdbcDatabase implements Database<Connection> {
int affected = ps.executeUpdate(); int affected = ps.executeUpdate();
if (affected != 1) throw new DbStateException(); if (affected != 1) throw new DbStateException();
ps.close(); ps.close();
return true;
} catch (SQLException e) { } catch (SQLException e) {
tryToClose(ps); tryToClose(ps);
tryToClose(rs);
throw new DbException(e); throw new DbException(e);
} }
} }

View File

@@ -7,7 +7,6 @@ import org.briarproject.api.contact.Contact;
import org.briarproject.api.contact.ContactId; import org.briarproject.api.contact.ContactId;
import org.briarproject.api.crypto.SecretKey; import org.briarproject.api.crypto.SecretKey;
import org.briarproject.api.db.DatabaseComponent; import org.briarproject.api.db.DatabaseComponent;
import org.briarproject.api.db.MessageExistsException;
import org.briarproject.api.db.Metadata; import org.briarproject.api.db.Metadata;
import org.briarproject.api.db.NoSuchContactException; import org.briarproject.api.db.NoSuchContactException;
import org.briarproject.api.db.NoSuchGroupException; import org.briarproject.api.db.NoSuchGroupException;
@@ -192,33 +191,6 @@ public class DatabaseComponentImplTest extends BriarTestCase {
context.assertIsSatisfied(); 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 @Test
public void testLocalMessagesAreNotStoredUnlessGroupExists() public void testLocalMessagesAreNotStoredUnlessGroupExists()
throws Exception { throws Exception {
@@ -230,8 +202,6 @@ public class DatabaseComponentImplTest extends BriarTestCase {
context.checking(new Expectations() {{ context.checking(new Expectations() {{
oneOf(database).startTransaction(); oneOf(database).startTransaction();
will(returnValue(txn)); will(returnValue(txn));
oneOf(database).containsMessage(txn, messageId);
will(returnValue(false));
oneOf(database).containsGroup(txn, groupId); oneOf(database).containsGroup(txn, groupId);
will(returnValue(false)); will(returnValue(false));
oneOf(database).abortTransaction(txn); oneOf(database).abortTransaction(txn);
@@ -259,10 +229,10 @@ public class DatabaseComponentImplTest extends BriarTestCase {
context.checking(new Expectations() {{ context.checking(new Expectations() {{
oneOf(database).startTransaction(); oneOf(database).startTransaction();
will(returnValue(txn)); will(returnValue(txn));
oneOf(database).containsMessage(txn, messageId);
will(returnValue(false));
oneOf(database).containsGroup(txn, groupId); oneOf(database).containsGroup(txn, groupId);
will(returnValue(true)); will(returnValue(true));
oneOf(database).containsMessage(txn, messageId);
will(returnValue(false));
oneOf(database).addMessage(txn, message, VALID, true); oneOf(database).addMessage(txn, message, VALID, true);
oneOf(database).mergeMessageMetadata(txn, messageId, metadata); oneOf(database).mergeMessageMetadata(txn, messageId, metadata);
oneOf(database).getVisibility(txn, groupId); oneOf(database).getVisibility(txn, groupId);
@@ -295,11 +265,11 @@ public class DatabaseComponentImplTest extends BriarTestCase {
final EventBus eventBus = context.mock(EventBus.class); final EventBus eventBus = context.mock(EventBus.class);
context.checking(new Expectations() {{ context.checking(new Expectations() {{
// Check whether the contact is in the DB (which it's not) // 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)); will(returnValue(txn));
exactly(13).of(database).containsContact(txn, contactId); exactly(15).of(database).containsContact(txn, contactId);
will(returnValue(false)); will(returnValue(false));
exactly(13).of(database).abortTransaction(txn); exactly(15).of(database).abortTransaction(txn);
}}); }});
DatabaseComponent db = createDatabaseComponent(database, eventBus, DatabaseComponent db = createDatabaseComponent(database, eventBus,
shutdown); shutdown);
@@ -332,6 +302,13 @@ public class DatabaseComponentImplTest extends BriarTestCase {
// Expected // Expected
} }
try {
db.generateRequest(contactId, 123);
fail();
} catch (NoSuchContactException expected) {
// Expected
}
try { try {
db.getContact(contactId); db.getContact(contactId);
fail(); fail();
@@ -383,6 +360,14 @@ public class DatabaseComponentImplTest extends BriarTestCase {
// Expected // Expected
} }
try {
Request r = new Request(Collections.singletonList(messageId));
db.receiveRequest(contactId, r);
fail();
} catch (NoSuchContactException expected) {
// Expected
}
try { try {
db.removeContact(contactId); db.removeContact(contactId);
fail(); fail();

View File

@@ -83,7 +83,6 @@ public class PluginManagerImplTest extends BriarTestCase {
oneOf(simplexPlugin).getMaxLatency(); oneOf(simplexPlugin).getMaxLatency();
will(returnValue(simplexLatency)); will(returnValue(simplexLatency));
oneOf(db).addTransport(simplexId, simplexLatency); oneOf(db).addTransport(simplexId, simplexLatency);
will(returnValue(true));
oneOf(simplexPlugin).start(); oneOf(simplexPlugin).start();
will(returnValue(true)); // Started will(returnValue(true)); // Started
oneOf(simplexPlugin).shouldPoll(); oneOf(simplexPlugin).shouldPoll();
@@ -98,7 +97,6 @@ public class PluginManagerImplTest extends BriarTestCase {
oneOf(simplexFailPlugin).getMaxLatency(); oneOf(simplexFailPlugin).getMaxLatency();
will(returnValue(simplexFailLatency)); will(returnValue(simplexFailLatency));
oneOf(db).addTransport(simplexFailId, simplexFailLatency); oneOf(db).addTransport(simplexFailId, simplexFailLatency);
will(returnValue(true));
oneOf(simplexFailPlugin).start(); oneOf(simplexFailPlugin).start();
will(returnValue(false)); // Failed to start will(returnValue(false)); // Failed to start
// First duplex plugin // First duplex plugin
@@ -112,7 +110,6 @@ public class PluginManagerImplTest extends BriarTestCase {
oneOf(duplexPlugin).getMaxLatency(); oneOf(duplexPlugin).getMaxLatency();
will(returnValue(duplexLatency)); will(returnValue(duplexLatency));
oneOf(db).addTransport(duplexId, duplexLatency); oneOf(db).addTransport(duplexId, duplexLatency);
will(returnValue(true));
oneOf(duplexPlugin).start(); oneOf(duplexPlugin).start();
will(returnValue(true)); // Started will(returnValue(true)); // Started
oneOf(duplexPlugin).shouldPoll(); oneOf(duplexPlugin).shouldPoll();