From 225d0ebeef7a396900721ede3b88017c18f516c9 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Thu, 28 Jan 2016 15:45:49 +0000
Subject: [PATCH] DB interface cleanup, removed unnecessary exceptions.
---
.../api/db/DatabaseComponent.java | 9 +--
.../api/db/LocalAuthorExistsException.java | 10 ----
.../api/db/MessageExistsException.java | 8 ---
.../src/org/briarproject/db/Database.java | 10 ++--
.../db/DatabaseComponentImpl.java | 38 +++++++------
.../src/org/briarproject/db/JdbcDatabase.java | 18 +-----
.../db/DatabaseComponentImplTest.java | 55 +++++++------------
.../plugins/PluginManagerImplTest.java | 3 -
8 files changed, 50 insertions(+), 101 deletions(-)
delete mode 100644 briar-api/src/org/briarproject/api/db/LocalAuthorExistsException.java
delete mode 100644 briar-api/src/org/briarproject/api/db/MessageExistsException.java
diff --git a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java
index 7d50be9282..3f01a4d40b 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 549140364a..0000000000
--- 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 7a3a6d2e00..0000000000
--- 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 4d147ccf46..e5b65296d3 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 649e47b2ad..44c177dc82 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 30a4f32e44..f922c548c7 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 a4128df560..238b62970b 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 4ec79282e5..23fbddea8d 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();
--
GitLab