From 5d768a5718ab893b17e77f36b2eaafa59eeb590c Mon Sep 17 00:00:00 2001 From: akwizgran <akwizgran@users.sourceforge.net> Date: Tue, 5 Jul 2011 18:15:44 +0100 Subject: [PATCH] DatabaseComponent throws an exception instead of returning silently if a contact is removed during an operation involving that contact. More unit tests. --- .../sf/briar/api/db/DatabaseComponent.java | 1 - api/net/sf/briar/api/db/DbException.java | 4 + .../briar/api/db/NoSuchContactException.java | 14 ++ components/net/sf/briar/db/Database.java | 10 ++ components/net/sf/briar/db/JdbcDatabase.java | 3 +- .../db/ReadWriteLockDatabaseComponent.java | 24 +-- .../db/SynchronizedDatabaseComponent.java | 22 +-- test/build.xml | 2 + test/net/sf/briar/TestUtils.java | 10 ++ .../briar/db/DatabaseComponentImplTest.java | 140 ++++++++++++++++++ .../sf/briar/db/DatabaseComponentTest.java | 121 +++++---------- test/net/sf/briar/db/H2DatabaseTest.java | 44 +++--- .../ReadWriteLockDatabaseComponentTest.java | 10 +- .../db/SynchronizedDatabaseComponentTest.java | 11 +- 14 files changed, 282 insertions(+), 134 deletions(-) create mode 100644 api/net/sf/briar/api/db/NoSuchContactException.java create mode 100644 test/net/sf/briar/db/DatabaseComponentImplTest.java diff --git a/api/net/sf/briar/api/db/DatabaseComponent.java b/api/net/sf/briar/api/db/DatabaseComponent.java index b72849d6f9..f646a9e412 100644 --- a/api/net/sf/briar/api/db/DatabaseComponent.java +++ b/api/net/sf/briar/api/db/DatabaseComponent.java @@ -21,7 +21,6 @@ public interface DatabaseComponent { static final long MAX_BYTES_BETWEEN_SPACE_CHECKS = 5L * MEGABYTES; static final long MAX_MS_BETWEEN_SPACE_CHECKS = 60L * 1000L; // 1 min static final long BYTES_PER_SWEEP = 5L * MEGABYTES; - static final int RETRANSMIT_THRESHOLD = 3; /** * Opens the database. diff --git a/api/net/sf/briar/api/db/DbException.java b/api/net/sf/briar/api/db/DbException.java index 1061f76e4e..3ad5030a9e 100644 --- a/api/net/sf/briar/api/db/DbException.java +++ b/api/net/sf/briar/api/db/DbException.java @@ -4,6 +4,10 @@ public class DbException extends Exception { private static final long serialVersionUID = 3706581789209939441L; + public DbException() { + super(); + } + public DbException(Throwable t) { super(t); } diff --git a/api/net/sf/briar/api/db/NoSuchContactException.java b/api/net/sf/briar/api/db/NoSuchContactException.java new file mode 100644 index 0000000000..72b735493f --- /dev/null +++ b/api/net/sf/briar/api/db/NoSuchContactException.java @@ -0,0 +1,14 @@ +package net.sf.briar.api.db; + +/** + * Thrown when a database operation is attempted for a contact that is not in + * the database. + */ +public class NoSuchContactException extends DbException { + + private static final long serialVersionUID = -7048538231308207386L; + + public NoSuchContactException() { + super(); + } +} diff --git a/components/net/sf/briar/db/Database.java b/components/net/sf/briar/db/Database.java index 9965e950d0..9246874b05 100644 --- a/components/net/sf/briar/db/Database.java +++ b/components/net/sf/briar/db/Database.java @@ -32,6 +32,16 @@ import net.sf.briar.api.protocol.MessageId; */ interface Database<T> { + /** + * A batch sent to a contact is considered lost when this many bundles have + * been received from the contact since the batch was sent. + * <p> + * FIXME: Come up with a better retransmission scheme. This scheme doesn't + * cope well with transports that have high latency but send bundles + * frequently. + */ + static final int RETRANSMIT_THRESHOLD = 3; + /** * Opens the database. * @param resume True to reopen an existing database, false to create a diff --git a/components/net/sf/briar/db/JdbcDatabase.java b/components/net/sf/briar/db/JdbcDatabase.java index efb6c67ee7..1270687995 100644 --- a/components/net/sf/briar/db/JdbcDatabase.java +++ b/components/net/sf/briar/db/JdbcDatabase.java @@ -18,7 +18,6 @@ import java.util.logging.Level; import java.util.logging.Logger; import net.sf.briar.api.db.ContactId; -import net.sf.briar.api.db.DatabaseComponent; import net.sf.briar.api.db.DbException; import net.sf.briar.api.db.Rating; import net.sf.briar.api.db.Status; @@ -527,7 +526,7 @@ abstract class JdbcDatabase implements Database<Connection> { rs.close(); ps.close(); Set<BatchId> lost; - if(received == DatabaseComponent.RETRANSMIT_THRESHOLD) { + if(received == RETRANSMIT_THRESHOLD) { // Expire batches related to the oldest received bundle assert oldestBundle != null; lost = findLostBatches(txn, c, oldestBundle); diff --git a/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java b/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java index c5d207b153..921e7fb026 100644 --- a/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java +++ b/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java @@ -7,8 +7,9 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.logging.Level; import java.util.logging.Logger; -import net.sf.briar.api.db.DbException; import net.sf.briar.api.db.ContactId; +import net.sf.briar.api.db.DbException; +import net.sf.briar.api.db.NoSuchContactException; import net.sf.briar.api.db.Rating; import net.sf.briar.api.protocol.AuthorId; import net.sf.briar.api.protocol.Batch; @@ -120,7 +121,10 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { // unsubscribed from the group if(db.containsSubscription(txn, m.getGroup())) { boolean added = storeMessage(txn, m, null); - assert added; + if(!added) { + if(LOG.isLoggable(Level.FINE)) + LOG.fine("Duplicate local message"); + } } else { if(LOG.isLoggable(Level.FINE)) LOG.fine("Not subscribed"); @@ -177,7 +181,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { // Ack all batches received from c contactLock.readLock().lock(); try { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); messageStatusLock.writeLock().lock(); try { Txn txn = db.startTransaction(); @@ -203,7 +207,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { // Add a list of subscriptions contactLock.readLock().lock(); try { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); subscriptionLock.readLock().lock(); try { Txn txn = db.startTransaction(); @@ -246,7 +250,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { private Batch fillBatch(ContactId c, long capacity) throws DbException { contactLock.readLock().lock(); try { - if(!containsContact(c)) return null; + if(!containsContact(c)) throw new NoSuchContactException(); messageLock.readLock().lock(); try { Set<MessageId> sent; @@ -344,7 +348,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { // Mark all messages in acked batches as seen contactLock.readLock().lock(); try { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); messageLock.readLock().lock(); try { messageStatusLock.writeLock().lock(); @@ -375,7 +379,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { // Update the contact's subscriptions contactLock.readLock().lock(); try { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); messageStatusLock.writeLock().lock(); try { Txn txn = db.startTransaction(); @@ -406,7 +410,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { waitForPermissionToWrite(); contactLock.readLock().lock(); try { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); messageLock.writeLock().lock(); try { messageStatusLock.writeLock().lock(); @@ -451,7 +455,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { Set<BatchId> lost; contactLock.readLock().lock(); try { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); messageLock.readLock().lock(); try { messageStatusLock.writeLock().lock(); @@ -476,7 +480,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { for(BatchId batch : lost) { contactLock.readLock().lock(); try { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); messageLock.readLock().lock(); try { messageStatusLock.writeLock().lock(); diff --git a/components/net/sf/briar/db/SynchronizedDatabaseComponent.java b/components/net/sf/briar/db/SynchronizedDatabaseComponent.java index 4ed4f309cb..959b3be61d 100644 --- a/components/net/sf/briar/db/SynchronizedDatabaseComponent.java +++ b/components/net/sf/briar/db/SynchronizedDatabaseComponent.java @@ -8,6 +8,7 @@ import java.util.logging.Logger; import net.sf.briar.api.db.DbException; import net.sf.briar.api.db.ContactId; +import net.sf.briar.api.db.NoSuchContactException; import net.sf.briar.api.db.Rating; import net.sf.briar.api.protocol.AuthorId; import net.sf.briar.api.protocol.Batch; @@ -89,7 +90,10 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { // unsubscribed from the group if(db.containsSubscription(txn, m.getGroup())) { boolean added = storeMessage(txn, m, null); - assert added; + if(!added) { + if(LOG.isLoggable(Level.FINE)) + LOG.fine("Duplicate local message"); + } } else { if(LOG.isLoggable(Level.FINE)) LOG.fine("Not subscribed"); @@ -128,7 +132,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { if(LOG.isLoggable(Level.FINE)) LOG.fine("Generating bundle for " + c); // Ack all batches received from c synchronized(contactLock) { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); synchronized(messageStatusLock) { Txn txn = db.startTransaction(); try { @@ -148,7 +152,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { } // Add a list of subscriptions synchronized(contactLock) { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); synchronized(subscriptionLock) { Txn txn = db.startTransaction(); try { @@ -185,7 +189,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { private Batch fillBatch(ContactId c, long capacity) throws DbException { synchronized(contactLock) { - if(!containsContact(c)) return null; + if(!containsContact(c)) throw new NoSuchContactException(); synchronized(messageLock) { synchronized(messageStatusLock) { Txn txn = db.startTransaction(); @@ -254,7 +258,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { + b.getSize() + " bytes"); // Mark all messages in acked batches as seen synchronized(contactLock) { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); synchronized(messageLock) { synchronized(messageStatusLock) { int acks = 0; @@ -276,7 +280,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { } // Update the contact's subscriptions synchronized(contactLock) { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); synchronized(messageStatusLock) { Txn txn = db.startTransaction(); try { @@ -301,7 +305,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { batches++; waitForPermissionToWrite(); synchronized(contactLock) { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); synchronized(messageLock) { synchronized(messageStatusLock) { synchronized(subscriptionLock) { @@ -334,7 +338,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { // Find any lost batches that need to be retransmitted Set<BatchId> lost; synchronized(contactLock) { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); synchronized(messageLock) { synchronized(messageStatusLock) { Txn txn = db.startTransaction(); @@ -350,7 +354,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> { } for(BatchId batch : lost) { synchronized(contactLock) { - if(!containsContact(c)) return; + if(!containsContact(c)) throw new NoSuchContactException(); synchronized(messageLock) { synchronized(messageStatusLock) { Txn txn = db.startTransaction(); diff --git a/test/build.xml b/test/build.xml index 0db40f821b..4b864204d2 100644 --- a/test/build.xml +++ b/test/build.xml @@ -15,6 +15,8 @@ </classpath> <test name='net.sf.briar.db.DatabaseCleanerImplTest'/> <test name='net.sf.briar.db.H2DatabaseTest'/> + <test name='net.sf.briar.db.ReadWriteLockDatabaseComponentTest'/> + <test name='net.sf.briar.db.SynchronizedDatabaseComponentTest'/> <test name='net.sf.briar.i18n.FontManagerTest'/> <test name='net.sf.briar.i18n.I18nTest'/> <test name='net.sf.briar.invitation.InvitationWorkerTest'/> diff --git a/test/net/sf/briar/TestUtils.java b/test/net/sf/briar/TestUtils.java index 552fbd427e..6046f65443 100644 --- a/test/net/sf/briar/TestUtils.java +++ b/test/net/sf/briar/TestUtils.java @@ -4,12 +4,16 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.PrintStream; +import java.util.Random; import java.util.concurrent.atomic.AtomicInteger; +import net.sf.briar.api.protocol.UniqueId; + public class TestUtils { private static final AtomicInteger nextTestDir = new AtomicInteger((int) (Math.random() * 1000 * 1000)); + private static final Random random = new Random(); public static void delete(File f) { if(f.isDirectory()) for(File child : f.listFiles()) delete(child); @@ -42,4 +46,10 @@ public class TestUtils { if(bin.exists() && bin.isDirectory()) return bin; throw new RuntimeException("Could not find build directory"); } + + public static byte[] getRandomId() { + byte[] b = new byte[UniqueId.LENGTH]; + random.nextBytes(b); + return b; + } } diff --git a/test/net/sf/briar/db/DatabaseComponentImplTest.java b/test/net/sf/briar/db/DatabaseComponentImplTest.java new file mode 100644 index 0000000000..e713c5ebad --- /dev/null +++ b/test/net/sf/briar/db/DatabaseComponentImplTest.java @@ -0,0 +1,140 @@ +package net.sf.briar.db; + +import static net.sf.briar.api.db.DatabaseComponent.BYTES_PER_SWEEP; +import static net.sf.briar.api.db.DatabaseComponent.MIN_FREE_SPACE; + +import java.util.Collections; + +import net.sf.briar.api.db.DbException; +import net.sf.briar.api.protocol.Batch; +import net.sf.briar.api.protocol.MessageId; +import net.sf.briar.db.DatabaseCleaner.Callback; + +import org.jmock.Expectations; +import org.jmock.Mockery; +import org.junit.Test; + +import com.google.inject.Provider; + +/** + * Tests that use the DatabaseCleaner.Callback interface of + * DatabaseComponentImpl. + */ +public abstract class DatabaseComponentImplTest extends DatabaseComponentTest { + + protected abstract <T> DatabaseComponentImpl<T> createDatabaseComponentImpl( + Database<T> database, DatabaseCleaner cleaner, + Provider<Batch> batchProvider); + + @Test + public void testNotCleanedIfEnoughFreeSpace() throws DbException { + Mockery context = new Mockery(); + @SuppressWarnings("unchecked") + final Database<Object> database = context.mock(Database.class); + final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class); + @SuppressWarnings("unchecked") + final Provider<Batch> batchProvider = context.mock(Provider.class); + context.checking(new Expectations() {{ + oneOf(database).getFreeSpace(); + will(returnValue(MIN_FREE_SPACE)); + }}); + Callback db = createDatabaseComponentImpl(database, cleaner, + batchProvider); + + db.checkFreeSpaceAndClean(); + + context.assertIsSatisfied(); + } + + @Test + public void testCleanedIfNotEnoughFreeSpace() throws DbException { + Mockery context = new Mockery(); + @SuppressWarnings("unchecked") + final Database<Object> database = context.mock(Database.class); + final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class); + @SuppressWarnings("unchecked") + final Provider<Batch> batchProvider = context.mock(Provider.class); + context.checking(new Expectations() {{ + oneOf(database).getFreeSpace(); + will(returnValue(MIN_FREE_SPACE - 1)); + oneOf(database).startTransaction(); + will(returnValue(txn)); + oneOf(database).getOldMessages(txn, BYTES_PER_SWEEP); + will(returnValue(Collections.emptySet())); + oneOf(database).commitTransaction(txn); + // As if by magic, some free space has appeared + oneOf(database).getFreeSpace(); + will(returnValue(MIN_FREE_SPACE)); + }}); + Callback db = createDatabaseComponentImpl(database, cleaner, + batchProvider); + + db.checkFreeSpaceAndClean(); + + context.assertIsSatisfied(); + } + + @Test + public void testExpiringUnsendableMessageDoesNotTriggerBackwardInclusion() + throws DbException { + Mockery context = new Mockery(); + @SuppressWarnings("unchecked") + final Database<Object> database = context.mock(Database.class); + final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class); + @SuppressWarnings("unchecked") + final Provider<Batch> batchProvider = context.mock(Provider.class); + context.checking(new Expectations() {{ + oneOf(database).getFreeSpace(); + will(returnValue(MIN_FREE_SPACE - 1)); + oneOf(database).startTransaction(); + will(returnValue(txn)); + oneOf(database).getOldMessages(txn, BYTES_PER_SWEEP); + will(returnValue(Collections.singleton(messageId))); + oneOf(database).getSendability(txn, messageId); + will(returnValue(0)); + oneOf(database).removeMessage(txn, messageId); + oneOf(database).commitTransaction(txn); + oneOf(database).getFreeSpace(); + will(returnValue(MIN_FREE_SPACE)); + }}); + Callback db = createDatabaseComponentImpl(database, cleaner, + batchProvider); + + db.checkFreeSpaceAndClean(); + + context.assertIsSatisfied(); + } + + @Test + public void testExpiringSendableMessageTriggersBackwardInclusion() + throws DbException { + Mockery context = new Mockery(); + @SuppressWarnings("unchecked") + final Database<Object> database = context.mock(Database.class); + final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class); + @SuppressWarnings("unchecked") + final Provider<Batch> batchProvider = context.mock(Provider.class); + context.checking(new Expectations() {{ + oneOf(database).getFreeSpace(); + will(returnValue(MIN_FREE_SPACE - 1)); + oneOf(database).startTransaction(); + will(returnValue(txn)); + oneOf(database).getOldMessages(txn, BYTES_PER_SWEEP); + will(returnValue(Collections.singleton(messageId))); + oneOf(database).getSendability(txn, messageId); + will(returnValue(1)); + oneOf(database).getParent(txn, messageId); + will(returnValue(MessageId.NONE)); + oneOf(database).removeMessage(txn, messageId); + oneOf(database).commitTransaction(txn); + oneOf(database).getFreeSpace(); + will(returnValue(MIN_FREE_SPACE)); + }}); + Callback db = createDatabaseComponentImpl(database, cleaner, + batchProvider); + + db.checkFreeSpaceAndClean(); + + context.assertIsSatisfied(); + } +} diff --git a/test/net/sf/briar/db/DatabaseComponentTest.java b/test/net/sf/briar/db/DatabaseComponentTest.java index 30054c9113..ea3b011bf5 100644 --- a/test/net/sf/briar/db/DatabaseComponentTest.java +++ b/test/net/sf/briar/db/DatabaseComponentTest.java @@ -1,8 +1,6 @@ package net.sf.briar.db; -import java.io.File; import java.util.Collections; -import java.util.Random; import java.util.Set; import junit.framework.TestCase; @@ -10,10 +8,12 @@ import net.sf.briar.TestUtils; import net.sf.briar.api.db.ContactId; import net.sf.briar.api.db.DatabaseComponent; import net.sf.briar.api.db.DbException; +import net.sf.briar.api.db.NoSuchContactException; import net.sf.briar.api.db.Rating; import net.sf.briar.api.db.Status; import net.sf.briar.api.protocol.AuthorId; import net.sf.briar.api.protocol.Batch; +import net.sf.briar.api.protocol.Bundle; import net.sf.briar.api.protocol.GroupId; import net.sf.briar.api.protocol.Message; import net.sf.briar.api.protocol.MessageId; @@ -21,37 +21,32 @@ import net.sf.briar.protocol.MessageImpl; import org.jmock.Expectations; import org.jmock.Mockery; -import org.junit.After; -import org.junit.Before; import org.junit.Test; import com.google.inject.Provider; public abstract class DatabaseComponentTest extends TestCase { - private final File testDir = TestUtils.getTestDirectory(); - private final Random random = new Random(); - private final AuthorId authorId; - private final ContactId contactId; - private final GroupId groupId; - private final MessageId messageId, parentId; + protected final Object txn = new Object(); + protected final AuthorId authorId; + protected final ContactId contactId; + protected final GroupId groupId; + protected final MessageId messageId, parentId; private final long timestamp; private final int size; private final byte[] body; private final Message message; - private final Object txn = new Object(); public DatabaseComponentTest() { super(); - authorId = new AuthorId(getRandomId()); + authorId = new AuthorId(TestUtils.getRandomId()); contactId = new ContactId(123); - groupId = new GroupId(getRandomId()); - messageId = new MessageId(getRandomId()); - parentId = new MessageId(getRandomId()); + groupId = new GroupId(TestUtils.getRandomId()); + messageId = new MessageId(TestUtils.getRandomId()); + parentId = new MessageId(TestUtils.getRandomId()); timestamp = System.currentTimeMillis(); size = 1234; body = new byte[size]; - random.nextBytes(body); message = new MessageImpl(messageId, MessageId.NONE, groupId, authorId, timestamp, body); } @@ -60,11 +55,6 @@ public abstract class DatabaseComponentTest extends TestCase { Database<T> database, DatabaseCleaner cleaner, Provider<Batch> batchProvider); - @Before - public void setUp() { - testDir.mkdirs(); - } - @Test public void testSimpleCalls() throws DbException { final Set<GroupId> subs = Collections.singleton(groupId); @@ -137,8 +127,6 @@ public abstract class DatabaseComponentTest extends TestCase { @SuppressWarnings("unchecked") final Provider<Batch> batchProvider = context.mock(Provider.class); context.checking(new Expectations() {{ - oneOf(database).open(false); - oneOf(cleaner).startCleaning(); // setRating(Rating.GOOD) oneOf(database).startTransaction(); will(returnValue(txn)); @@ -153,15 +141,11 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(database).getParent(txn, messageId); will(returnValue(MessageId.NONE)); oneOf(database).commitTransaction(txn); - oneOf(cleaner).stopCleaning(); - oneOf(database).close(); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner, batchProvider); - db.open(false); db.setRating(authorId, Rating.GOOD); - db.close(); context.assertIsSatisfied(); } @@ -176,8 +160,6 @@ public abstract class DatabaseComponentTest extends TestCase { @SuppressWarnings("unchecked") final Provider<Batch> batchProvider = context.mock(Provider.class); context.checking(new Expectations() {{ - oneOf(database).open(false); - oneOf(cleaner).startCleaning(); // setRating(Rating.GOOD) oneOf(database).startTransaction(); will(returnValue(txn)); @@ -195,22 +177,18 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(database).containsMessage(txn, parentId); will(returnValue(false)); oneOf(database).commitTransaction(txn); - oneOf(cleaner).stopCleaning(); - oneOf(database).close(); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner, batchProvider); - db.open(false); db.setRating(authorId, Rating.GOOD); - db.close(); context.assertIsSatisfied(); } @Test public void testChangingGroupsStopsBackwardInclusion() throws DbException { - final GroupId groupId1 = new GroupId(getRandomId()); + final GroupId groupId1 = new GroupId(TestUtils.getRandomId()); final Set<MessageId> messages = Collections.singleton(messageId); Mockery context = new Mockery(); @SuppressWarnings("unchecked") @@ -219,8 +197,6 @@ public abstract class DatabaseComponentTest extends TestCase { @SuppressWarnings("unchecked") final Provider<Batch> batchProvider = context.mock(Provider.class); context.checking(new Expectations() {{ - oneOf(database).open(false); - oneOf(cleaner).startCleaning(); // setRating(Rating.GOOD) oneOf(database).startTransaction(); will(returnValue(txn)); @@ -242,15 +218,11 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(database).getGroup(txn, parentId); will(returnValue(groupId1)); oneOf(database).commitTransaction(txn); - oneOf(cleaner).stopCleaning(); - oneOf(database).close(); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner, batchProvider); - db.open(false); db.setRating(authorId, Rating.GOOD); - db.close(); context.assertIsSatisfied(); } @@ -266,8 +238,6 @@ public abstract class DatabaseComponentTest extends TestCase { @SuppressWarnings("unchecked") final Provider<Batch> batchProvider = context.mock(Provider.class); context.checking(new Expectations() {{ - oneOf(database).open(false); - oneOf(cleaner).startCleaning(); // setRating(Rating.GOOD) oneOf(database).startTransaction(); will(returnValue(txn)); @@ -292,15 +262,11 @@ public abstract class DatabaseComponentTest extends TestCase { will(returnValue(1)); oneOf(database).setSendability(txn, parentId, 2); oneOf(database).commitTransaction(txn); - oneOf(cleaner).stopCleaning(); - oneOf(database).close(); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner, batchProvider); - db.open(false); db.setRating(authorId, Rating.GOOD); - db.close(); context.assertIsSatisfied(); } @@ -316,8 +282,6 @@ public abstract class DatabaseComponentTest extends TestCase { @SuppressWarnings("unchecked") final Provider<Batch> batchProvider = context.mock(Provider.class); context.checking(new Expectations() {{ - oneOf(database).open(false); - oneOf(cleaner).startCleaning(); // setRating(Rating.GOOD) oneOf(database).startTransaction(); will(returnValue(txn)); @@ -344,15 +308,11 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(database).getParent(txn, parentId); will(returnValue(MessageId.NONE)); oneOf(database).commitTransaction(txn); - oneOf(cleaner).stopCleaning(); - oneOf(database).close(); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner, batchProvider); - db.open(false); db.setRating(authorId, Rating.GOOD); - db.close(); context.assertIsSatisfied(); } @@ -367,23 +327,17 @@ public abstract class DatabaseComponentTest extends TestCase { @SuppressWarnings("unchecked") final Provider<Batch> batchProvider = context.mock(Provider.class); context.checking(new Expectations() {{ - oneOf(database).open(false); - oneOf(cleaner).startCleaning(); // addLocallyGeneratedMessage(message) oneOf(database).startTransaction(); will(returnValue(txn)); oneOf(database).containsSubscription(txn, groupId); will(returnValue(false)); oneOf(database).commitTransaction(txn); - oneOf(cleaner).stopCleaning(); - oneOf(database).close(); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner, batchProvider); - db.open(false); db.addLocallyGeneratedMessage(message); - db.close(); context.assertIsSatisfied(); } @@ -397,8 +351,6 @@ public abstract class DatabaseComponentTest extends TestCase { @SuppressWarnings("unchecked") final Provider<Batch> batchProvider = context.mock(Provider.class); context.checking(new Expectations() {{ - oneOf(database).open(false); - oneOf(cleaner).startCleaning(); // addLocallyGeneratedMessage(message) oneOf(database).startTransaction(); will(returnValue(txn)); @@ -407,15 +359,11 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(database).addMessage(txn, message); will(returnValue(false)); oneOf(database).commitTransaction(txn); - oneOf(cleaner).stopCleaning(); - oneOf(database).close(); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner, batchProvider); - db.open(false); db.addLocallyGeneratedMessage(message); - db.close(); context.assertIsSatisfied(); } @@ -429,8 +377,6 @@ public abstract class DatabaseComponentTest extends TestCase { @SuppressWarnings("unchecked") final Provider<Batch> batchProvider = context.mock(Provider.class); context.checking(new Expectations() {{ - oneOf(database).open(false); - oneOf(cleaner).startCleaning(); // addLocallyGeneratedMessage(message) oneOf(database).startTransaction(); will(returnValue(txn)); @@ -448,15 +394,11 @@ public abstract class DatabaseComponentTest extends TestCase { will(returnValue(0)); oneOf(database).setSendability(txn, messageId, 0); oneOf(database).commitTransaction(txn); - oneOf(cleaner).stopCleaning(); - oneOf(database).close(); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner, batchProvider); - db.open(false); db.addLocallyGeneratedMessage(message); - db.close(); context.assertIsSatisfied(); } @@ -471,8 +413,6 @@ public abstract class DatabaseComponentTest extends TestCase { @SuppressWarnings("unchecked") final Provider<Batch> batchProvider = context.mock(Provider.class); context.checking(new Expectations() {{ - oneOf(database).open(false); - oneOf(cleaner).startCleaning(); // addLocallyGeneratedMessage(message) oneOf(database).startTransaction(); will(returnValue(txn)); @@ -493,27 +433,40 @@ public abstract class DatabaseComponentTest extends TestCase { oneOf(database).getParent(txn, messageId); will(returnValue(MessageId.NONE)); oneOf(database).commitTransaction(txn); - oneOf(cleaner).stopCleaning(); - oneOf(database).close(); }}); DatabaseComponent db = createDatabaseComponent(database, cleaner, batchProvider); - db.open(false); db.addLocallyGeneratedMessage(message); - db.close(); context.assertIsSatisfied(); } - private byte[] getRandomId() { - byte[] b = new byte[32]; - random.nextBytes(b); - return b; - } + @Test + public void testGenerateBundleThrowsExceptionIfContactIsMissing() + throws DbException { + Mockery context = new Mockery(); + @SuppressWarnings("unchecked") + final Database<Object> database = context.mock(Database.class); + final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class); + @SuppressWarnings("unchecked") + final Provider<Batch> batchProvider = context.mock(Provider.class); + final Bundle bundle = context.mock(Bundle.class); + context.checking(new Expectations() {{ + oneOf(database).startTransaction(); + will(returnValue(txn)); + oneOf(database).containsContact(txn, contactId); + will(returnValue(false)); + oneOf(database).commitTransaction(txn); + }}); + DatabaseComponent db = createDatabaseComponent(database, cleaner, + batchProvider); - @After - public void tearDown() { - TestUtils.deleteTestDirectory(testDir); + try { + db.generateBundle(contactId, bundle); + assertTrue(false); + } catch(NoSuchContactException expected) {} + + context.assertIsSatisfied(); } } diff --git a/test/net/sf/briar/db/H2DatabaseTest.java b/test/net/sf/briar/db/H2DatabaseTest.java index 483b507af2..3c18e2314b 100644 --- a/test/net/sf/briar/db/H2DatabaseTest.java +++ b/test/net/sf/briar/db/H2DatabaseTest.java @@ -53,11 +53,11 @@ public class H2DatabaseTest extends TestCase { public H2DatabaseTest() { super(); - authorId = new AuthorId(getRandomId()); - batchId = new BatchId(getRandomId()); + authorId = new AuthorId(TestUtils.getRandomId()); + batchId = new BatchId(TestUtils.getRandomId()); contactId = new ContactId(123); - groupId = new GroupId(getRandomId()); - messageId = new MessageId(getRandomId()); + groupId = new GroupId(TestUtils.getRandomId()); + messageId = new MessageId(TestUtils.getRandomId()); timestamp = System.currentTimeMillis(); size = 1234; body = new byte[size]; @@ -338,7 +338,7 @@ public class H2DatabaseTest extends TestCase { @Test public void testBatchesToAck() throws DbException { - BatchId batchId1 = new BatchId(getRandomId()); + BatchId batchId1 = new BatchId(TestUtils.getRandomId()); Mockery context = new Mockery(); MessageFactory messageFactory = context.mock(MessageFactory.class); @@ -461,13 +461,13 @@ public class H2DatabaseTest extends TestCase { @Test public void testRetransmission() throws DbException { - BundleId bundleId = new BundleId(getRandomId()); - BundleId bundleId1 = new BundleId(getRandomId()); - BundleId bundleId2 = new BundleId(getRandomId()); - BundleId bundleId3 = new BundleId(getRandomId()); - BundleId bundleId4 = new BundleId(getRandomId()); - BatchId batchId1 = new BatchId(getRandomId()); - BatchId batchId2 = new BatchId(getRandomId()); + BundleId bundleId = new BundleId(TestUtils.getRandomId()); + BundleId bundleId1 = new BundleId(TestUtils.getRandomId()); + BundleId bundleId2 = new BundleId(TestUtils.getRandomId()); + BundleId bundleId3 = new BundleId(TestUtils.getRandomId()); + BundleId bundleId4 = new BundleId(TestUtils.getRandomId()); + BatchId batchId1 = new BatchId(TestUtils.getRandomId()); + BatchId batchId2 = new BatchId(TestUtils.getRandomId()); Set<MessageId> empty = Collections.emptySet(); Mockery context = new Mockery(); MessageFactory messageFactory = context.mock(MessageFactory.class); @@ -508,8 +508,8 @@ public class H2DatabaseTest extends TestCase { @Test public void testGetMessagesByAuthor() throws DbException { - AuthorId authorId1 = new AuthorId(getRandomId()); - MessageId messageId1 = new MessageId(getRandomId()); + AuthorId authorId1 = new AuthorId(TestUtils.getRandomId()); + MessageId messageId1 = new MessageId(TestUtils.getRandomId()); Message message1 = new MessageImpl(messageId1, MessageId.NONE, groupId, authorId1, timestamp, body); Mockery context = new Mockery(); @@ -543,10 +543,10 @@ public class H2DatabaseTest extends TestCase { @Test public void testGetNumberOfSendableChildren() throws DbException { - MessageId childId1 = new MessageId(getRandomId()); - MessageId childId2 = new MessageId(getRandomId()); - MessageId childId3 = new MessageId(getRandomId()); - GroupId groupId1 = new GroupId(getRandomId()); + MessageId childId1 = new MessageId(TestUtils.getRandomId()); + MessageId childId2 = new MessageId(TestUtils.getRandomId()); + MessageId childId3 = new MessageId(TestUtils.getRandomId()); + GroupId groupId1 = new GroupId(TestUtils.getRandomId()); Message child1 = new MessageImpl(childId1, messageId, groupId, authorId, timestamp, body); Message child2 = new MessageImpl(childId2, messageId, groupId, @@ -588,7 +588,7 @@ public class H2DatabaseTest extends TestCase { @Test public void testGetOldMessages() throws DbException { - MessageId messageId1 = new MessageId(getRandomId()); + MessageId messageId1 = new MessageId(TestUtils.getRandomId()); Message message1 = new MessageImpl(messageId1, MessageId.NONE, groupId, authorId, timestamp + 1000, body); Mockery context = new Mockery(); @@ -758,12 +758,6 @@ public class H2DatabaseTest extends TestCase { TestUtils.deleteTestDirectory(testDir); } - private byte[] getRandomId() { - byte[] b = new byte[32]; - random.nextBytes(b); - return b; - } - private static class TestMessageFactory implements MessageFactory { public Message createMessage(MessageId id, MessageId parent, diff --git a/test/net/sf/briar/db/ReadWriteLockDatabaseComponentTest.java b/test/net/sf/briar/db/ReadWriteLockDatabaseComponentTest.java index 8de0dc1802..16ff7eae5e 100644 --- a/test/net/sf/briar/db/ReadWriteLockDatabaseComponentTest.java +++ b/test/net/sf/briar/db/ReadWriteLockDatabaseComponentTest.java @@ -5,12 +5,20 @@ import net.sf.briar.api.protocol.Batch; import com.google.inject.Provider; -public class ReadWriteLockDatabaseComponentTest extends DatabaseComponentTest { +public class ReadWriteLockDatabaseComponentTest +extends DatabaseComponentImplTest { @Override protected <T> DatabaseComponent createDatabaseComponent( Database<T> database, DatabaseCleaner cleaner, Provider<Batch> batchProvider) { + return createDatabaseComponentImpl(database, cleaner, batchProvider); + } + + @Override + protected <T> DatabaseComponentImpl<T> createDatabaseComponentImpl( + Database<T> database, DatabaseCleaner cleaner, + Provider<Batch> batchProvider) { return new ReadWriteLockDatabaseComponent<T>(database, cleaner, batchProvider); } diff --git a/test/net/sf/briar/db/SynchronizedDatabaseComponentTest.java b/test/net/sf/briar/db/SynchronizedDatabaseComponentTest.java index 758ce1ce48..647f50490d 100644 --- a/test/net/sf/briar/db/SynchronizedDatabaseComponentTest.java +++ b/test/net/sf/briar/db/SynchronizedDatabaseComponentTest.java @@ -5,14 +5,21 @@ import net.sf.briar.api.protocol.Batch; import com.google.inject.Provider; -public class SynchronizedDatabaseComponentTest extends DatabaseComponentTest { +public class SynchronizedDatabaseComponentTest +extends DatabaseComponentImplTest { @Override protected <T> DatabaseComponent createDatabaseComponent( Database<T> database, DatabaseCleaner cleaner, Provider<Batch> batchProvider) { + return createDatabaseComponentImpl(database, cleaner, batchProvider); + } + + @Override + protected <T> DatabaseComponentImpl<T> createDatabaseComponentImpl( + Database<T> database, DatabaseCleaner cleaner, + Provider<Batch> batchProvider) { return new SynchronizedDatabaseComponent<T>(database, cleaner, batchProvider); } - } -- GitLab