From cd40e771d262b3eaad384ba0ebe09c449929b976 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 12 Jun 2019 15:11:10 +0100 Subject: [PATCH 1/4] Allow messages to be marked as temporary. --- .../bramble/api/db/DatabaseComponent.java | 13 ++- .../bramble/client/ClientHelperImpl.java | 3 +- .../org/briarproject/bramble/db/Database.java | 13 ++- .../bramble/db/DatabaseComponentImpl.java | 28 ++++- .../briarproject/bramble/db/JdbcDatabase.java | 71 +++++++++--- .../bramble/db/Migration45_46.java | 41 +++++++ .../ClientVersioningManagerImpl.java | 2 +- .../bramble/client/ClientHelperImplTest.java | 2 +- .../bramble/db/DatabaseComponentImplTest.java | 53 ++++++--- .../bramble/db/DatabasePerformanceTest.java | 6 +- .../bramble/db/JdbcDatabaseTest.java | 107 +++++++++++------- .../ClientVersioningManagerImplTest.java | 6 +- .../introduction/IntroductionManagerImpl.java | 2 +- .../GroupInvitationManagerImpl.java | 2 +- .../briar/sharing/SharingManagerImpl.java | 2 +- .../GroupInvitationManagerImplTest.java | 2 +- .../sharing/BlogSharingManagerImplTest.java | 3 +- 17 files changed, 267 insertions(+), 89 deletions(-) create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/db/Migration45_46.java diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java index 775a0d869..a5d8dc653 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java @@ -77,7 +77,7 @@ public interface DatabaseComponent extends TransactionManager { * Stores a local message. */ void addLocalMessage(Transaction txn, Message m, Metadata meta, - boolean shared) throws DbException; + boolean shared, boolean temporary) throws DbException; /** * Stores a pending contact. @@ -510,6 +510,12 @@ public interface DatabaseComponent extends TransactionManager { void removePendingContact(Transaction txn, PendingContactId p) throws DbException; + /** + * Removes all temporary messages (and all associated state) from the + * database. + */ + void removeTemporaryMessages(Transaction txn) throws DbException; + /** * Removes a transport (and all associated state) from the database. */ @@ -538,6 +544,11 @@ public interface DatabaseComponent extends TransactionManager { void setGroupVisibility(Transaction txn, ContactId c, GroupId g, Visibility v) throws DbException; + /** + * Marks the given message as permanent, i.e. not temporary. + */ + void setMessagePermanent(Transaction txn, MessageId m) throws DbException; + /** * Marks the given message as shared. */ diff --git a/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java index 4bb5c68cd..7a0ca9e6e 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java @@ -92,7 +92,8 @@ class ClientHelperImpl implements ClientHelper { public void addLocalMessage(Transaction txn, Message m, BdfDictionary metadata, boolean shared) throws DbException, FormatException { - db.addLocalMessage(txn, m, metadataEncoder.encode(metadata), shared); + db.addLocalMessage(txn, m, metadataEncoder.encode(metadata), shared, + false); } @Override diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java b/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java index 65203229a..395800eac 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java @@ -115,7 +115,7 @@ interface Database { * if the message was created locally. */ void addMessage(T txn, Message m, MessageState state, boolean shared, - @Nullable ContactId sender) throws DbException; + boolean temporary, @Nullable ContactId sender) throws DbException; /** * Adds a dependency between two messages, where the dependent message is @@ -630,6 +630,12 @@ interface Database { */ void removePendingContact(T txn, PendingContactId p) throws DbException; + /** + * Removes all temporary messages (and all associated state) from the + * database. + */ + void removeTemporaryMessages(T txn) throws DbException; + /** * Removes a transport (and all associated state) from the database. */ @@ -671,6 +677,11 @@ interface Database { void setHandshakeKeyPair(T txn, AuthorId local, PublicKey publicKey, PrivateKey privateKey) throws DbException; + /** + * Marks the given message as permanent, i.e. not temporary. + */ + void setMessagePermanent(T txn, MessageId m) throws DbException; + /** * Marks the given message as shared. */ diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java index 666c54a23..8023763b4 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java @@ -273,13 +273,14 @@ class DatabaseComponentImpl implements DatabaseComponent { @Override public void addLocalMessage(Transaction transaction, Message m, - Metadata meta, boolean shared) throws DbException { + Metadata meta, boolean shared, boolean temporary) + throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); T txn = unbox(transaction); if (!db.containsGroup(txn, m.getGroupId())) throw new NoSuchGroupException(); if (!db.containsMessage(txn, m.getId())) { - db.addMessage(txn, m, DELIVERED, shared, null); + db.addMessage(txn, m, DELIVERED, shared, temporary, null); transaction.attach(new MessageAddedEvent(m, null)); transaction.attach(new MessageStateChangedEvent(m.getId(), true, DELIVERED)); @@ -800,7 +801,7 @@ class DatabaseComponentImpl implements DatabaseComponent { db.raiseSeenFlag(txn, c, m.getId()); db.raiseAckFlag(txn, c, m.getId()); } else { - db.addMessage(txn, m, UNKNOWN, false, c); + db.addMessage(txn, m, UNKNOWN, false, false, c); transaction.attach(new MessageAddedEvent(m, c)); } transaction.attach(new MessageToAckEvent(c)); @@ -908,6 +909,14 @@ class DatabaseComponentImpl implements DatabaseComponent { transaction.attach(new PendingContactRemovedEvent(p)); } + @Override + public void removeTemporaryMessages(Transaction transaction) + throws DbException { + if (transaction.isReadOnly()) throw new IllegalArgumentException(); + T txn = unbox(transaction); + db.removeTemporaryMessages(txn); + } + @Override public void removeTransport(Transaction transaction, TransportId t) throws DbException { @@ -967,6 +976,16 @@ class DatabaseComponentImpl implements DatabaseComponent { transaction.attach(new GroupVisibilityUpdatedEvent(affected)); } + @Override + public void setMessagePermanent(Transaction transaction, MessageId m) + throws DbException { + if (transaction.isReadOnly()) throw new IllegalArgumentException(); + T txn = unbox(transaction); + if (!db.containsMessage(txn, m)) + throw new NoSuchMessageException(); + db.setMessagePermanent(txn, m); + } + @Override public void setMessageShared(Transaction transaction, MessageId m) throws DbException { @@ -975,8 +994,7 @@ class DatabaseComponentImpl implements DatabaseComponent { if (!db.containsMessage(txn, m)) throw new NoSuchMessageException(); if (db.getMessageState(txn, m) != DELIVERED) - throw new IllegalArgumentException( - "Shared undelivered message"); + throw new IllegalArgumentException("Shared undelivered message"); db.setMessageShared(txn, m); transaction.attach(new MessageSharedEvent(m)); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java index b6e65894e..058c33726 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java @@ -62,6 +62,7 @@ import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; import javax.annotation.Nullable; +import javax.annotation.concurrent.GuardedBy; import static java.sql.Types.BINARY; import static java.sql.Types.BOOLEAN; @@ -97,7 +98,7 @@ import static org.briarproject.bramble.util.LogUtils.now; abstract class JdbcDatabase implements Database { // Package access for testing - static final int CODE_SCHEMA_VERSION = 45; + static final int CODE_SCHEMA_VERSION = 46; // Time period offsets for incoming transport keys private static final int OFFSET_PREV = -1; @@ -177,6 +178,7 @@ abstract class JdbcDatabase implements Database { + " timestamp BIGINT NOT NULL," + " state INT NOT NULL," + " shared BOOLEAN NOT NULL," + + " temporary BOOLEAN NOT NULL," + " length INT NOT NULL," + " raw BLOB," // Null if message has been deleted + " PRIMARY KEY (messageId)," @@ -336,25 +338,26 @@ abstract class JdbcDatabase implements Database { private static final Logger LOG = getLogger(JdbcDatabase.class.getName()); - // Different database libraries use different names for certain types private final MessageFactory messageFactory; private final Clock clock; private final DatabaseTypes dbTypes; - // Locking: connectionsLock + private final Lock connectionsLock = new ReentrantLock(); + private final Condition connectionsChanged = connectionsLock.newCondition(); + + @GuardedBy("connectionsLock") private final LinkedList connections = new LinkedList<>(); - private int openConnections = 0; // Locking: connectionsLock - private boolean closed = false; // Locking: connectionsLock + @GuardedBy("connectionsLock") + private int openConnections = 0; + @GuardedBy("connectionsLock") + private boolean closed = false; protected abstract Connection createConnection() throws DbException, SQLException; protected abstract void compactAndClose() throws DbException; - private final Lock connectionsLock = new ReentrantLock(); - private final Condition connectionsChanged = connectionsLock.newCondition(); - JdbcDatabase(DatabaseTypes databaseTypes, MessageFactory messageFactory, Clock clock) { this.dbTypes = databaseTypes; @@ -457,7 +460,8 @@ abstract class JdbcDatabase implements Database { new Migration41_42(dbTypes), new Migration42_43(dbTypes), new Migration43_44(dbTypes), - new Migration44_45() + new Migration44_45(), + new Migration45_46() ); } @@ -777,22 +781,23 @@ abstract class JdbcDatabase implements Database { @Override public void addMessage(Connection txn, Message m, MessageState state, - boolean messageShared, @Nullable ContactId sender) + boolean shared, boolean temporary, @Nullable ContactId sender) throws DbException { PreparedStatement ps = null; try { String sql = "INSERT INTO messages (messageId, groupId, timestamp," - + " state, shared, length, raw)" - + " VALUES (?, ?, ?, ?, ?, ?, ?)"; + + " state, shared, temporary, length, raw)" + + " VALUES (?, ?, ?, ?, ?, ?, ?, ?)"; ps = txn.prepareStatement(sql); ps.setBytes(1, m.getId().getBytes()); ps.setBytes(2, m.getGroupId().getBytes()); ps.setLong(3, m.getTimestamp()); ps.setInt(4, state.getValue()); - ps.setBoolean(5, messageShared); + ps.setBoolean(5, shared); + ps.setBoolean(6, temporary); byte[] raw = messageFactory.getRawMessage(m); - ps.setInt(6, raw.length); - ps.setBytes(7, raw); + ps.setInt(7, raw.length); + ps.setBytes(8, raw); int affected = ps.executeUpdate(); if (affected != 1) throw new DbStateException(); ps.close(); @@ -804,8 +809,7 @@ abstract class JdbcDatabase implements Database { boolean offered = removeOfferedMessage(txn, c, m.getId()); boolean seen = offered || c.equals(sender); addStatus(txn, m.getId(), c, m.getGroupId(), m.getTimestamp(), - raw.length, state, e.getValue(), messageShared, - false, seen); + raw.length, state, e.getValue(), shared, false, seen); } // Update denormalised column in messageDependencies if dependency // is in same group as dependent @@ -2876,6 +2880,21 @@ abstract class JdbcDatabase implements Database { } } + @Override + public void removeTemporaryMessages(Connection txn) throws DbException { + Statement s = null; + try { + String sql = "DELETE FROM messages WHERE temporary = TRUE"; + s = txn.createStatement(); + int affected = s.executeUpdate(sql); + if (affected < 0) throw new DbStateException(); + s.close(); + } catch (SQLException e) { + tryToClose(s, LOG, WARNING); + throw new DbException(e); + } + } + @Override public void removeTransport(Connection txn, TransportId t) throws DbException { @@ -3021,6 +3040,24 @@ abstract class JdbcDatabase implements Database { } } + @Override + public void setMessagePermanent(Connection txn, MessageId m) + throws DbException { + PreparedStatement ps = null; + try { + String sql = "UPDATE messages SET temporary = FALSE" + + " WHERE messageId = ?"; + ps = txn.prepareStatement(sql); + ps.setBytes(1, m.getBytes()); + int affected = ps.executeUpdate(); + if (affected < 0 || affected > 1) throw new DbStateException(); + ps.close(); + } catch (SQLException e) { + tryToClose(ps, LOG, WARNING); + throw new DbException(e); + } + } + @Override public void setMessageShared(Connection txn, MessageId m) throws DbException { diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/Migration45_46.java b/bramble-core/src/main/java/org/briarproject/bramble/db/Migration45_46.java new file mode 100644 index 000000000..a6c31dba5 --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/Migration45_46.java @@ -0,0 +1,41 @@ +package org.briarproject.bramble.db; + +import org.briarproject.bramble.api.db.DbException; + +import java.sql.Connection; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.logging.Logger; + +import static java.util.logging.Level.WARNING; +import static java.util.logging.Logger.getLogger; +import static org.briarproject.bramble.db.JdbcUtils.tryToClose; + +class Migration45_46 implements Migration { + + private static final Logger LOG = getLogger(Migration45_46.class.getName()); + + @Override + public int getStartVersion() { + return 45; + } + + @Override + public int getEndVersion() { + return 46; + } + + @Override + public void migrate(Connection txn) throws DbException { + Statement s = null; + try { + s = txn.createStatement(); + s.execute("ALTER TABLE messages" + + " ADD COLUMN temporary BOOLEAN NOT NULL" + + " DEFAULT (FALSE)"); + } catch (SQLException e) { + tryToClose(s, LOG, WARNING); + throw new DbException(e); + } + } +} \ No newline at end of file diff --git a/bramble-core/src/main/java/org/briarproject/bramble/versioning/ClientVersioningManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/versioning/ClientVersioningManagerImpl.java index 7e6dd41d5..0f1573d82 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/versioning/ClientVersioningManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/versioning/ClientVersioningManagerImpl.java @@ -243,7 +243,7 @@ class ClientVersioningManagerImpl implements ClientVersioningManager, try { Message m = clientHelper.createMessage(localGroup.getId(), now, body); - db.addLocalMessage(txn, m, new Metadata(), false); + db.addLocalMessage(txn, m, new Metadata(), false, false); } catch (FormatException e) { throw new AssertionError(e); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/client/ClientHelperImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/client/ClientHelperImplTest.java index 47a10e9a6..a79c18752 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/client/ClientHelperImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/client/ClientHelperImplTest.java @@ -96,7 +96,7 @@ public class ClientHelperImplTest extends BrambleTestCase { oneOf(db).transaction(with(false), withDbRunnable(txn)); oneOf(metadataEncoder).encode(dictionary); will(returnValue(metadata)); - oneOf(db).addLocalMessage(txn, message, metadata, shared); + oneOf(db).addLocalMessage(txn, message, metadata, shared, false); }}); clientHelper.addLocalMessage(message, dictionary, shared); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java index 1570d1b08..645afcdd9 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java @@ -61,6 +61,7 @@ import org.junit.Test; import java.util.ArrayList; import java.util.Collection; +import java.util.Random; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicReference; @@ -120,6 +121,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { private final Contact contact; private final KeySetId keySetId; private final PendingContactId pendingContactId; + private final Random random = new Random(); public DatabaseComponentImplTest() { clientId = getClientId(); @@ -242,6 +244,9 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { @Test(expected = NoSuchGroupException.class) public void testLocalMessagesAreNotStoredUnlessGroupExists() throws Exception { + boolean shared = random.nextBoolean(); + boolean temporary = random.nextBoolean(); + context.checking(new Expectations() {{ oneOf(database).startTransaction(); will(returnValue(txn)); @@ -253,11 +258,15 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { eventExecutor, shutdownManager); db.transaction(false, transaction -> - db.addLocalMessage(transaction, message, metadata, true)); + db.addLocalMessage(transaction, message, metadata, shared, + temporary)); } @Test public void testAddLocalMessage() throws Exception { + boolean shared = random.nextBoolean(); + boolean temporary = random.nextBoolean(); + context.checking(new Expectations() {{ oneOf(database).startTransaction(); will(returnValue(txn)); @@ -265,20 +274,23 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { will(returnValue(true)); oneOf(database).containsMessage(txn, messageId); will(returnValue(false)); - oneOf(database).addMessage(txn, message, DELIVERED, true, null); + oneOf(database).addMessage(txn, message, DELIVERED, shared, + temporary, null); oneOf(database).mergeMessageMetadata(txn, messageId, metadata); oneOf(database).commitTransaction(txn); // The message was added, so the listeners should be called oneOf(eventBus).broadcast(with(any(MessageAddedEvent.class))); - oneOf(eventBus) - .broadcast(with(any(MessageStateChangedEvent.class))); - oneOf(eventBus).broadcast(with(any(MessageSharedEvent.class))); + oneOf(eventBus).broadcast(with(any( + MessageStateChangedEvent.class))); + if (shared) + oneOf(eventBus).broadcast(with(any(MessageSharedEvent.class))); }}); DatabaseComponent db = createDatabaseComponent(database, eventBus, eventExecutor, shutdownManager); db.transaction(false, transaction -> - db.addLocalMessage(transaction, message, metadata, true)); + db.addLocalMessage(transaction, message, metadata, shared, + temporary)); } @Test @@ -569,11 +581,11 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { throws Exception { context.checking(new Expectations() {{ // Check whether the message is in the DB (which it's not) - exactly(11).of(database).startTransaction(); + exactly(12).of(database).startTransaction(); will(returnValue(txn)); - exactly(11).of(database).containsMessage(txn, messageId); + exactly(12).of(database).containsMessage(txn, messageId); will(returnValue(false)); - exactly(11).of(database).abortTransaction(txn); + exactly(12).of(database).abortTransaction(txn); // Allow other checks to pass allowing(database).containsContact(txn, contactId); will(returnValue(true)); @@ -637,6 +649,14 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { // Expected } + try { + db.transaction(false, transaction -> + db.setMessagePermanent(transaction, message.getId())); + fail(); + } catch (NoSuchMessageException expected) { + // Expected + } + try { db.transaction(false, transaction -> db.setMessageShared(transaction, message.getId())); @@ -972,7 +992,8 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { will(returnValue(VISIBLE)); oneOf(database).containsMessage(txn, messageId); will(returnValue(false)); - oneOf(database).addMessage(txn, message, UNKNOWN, false, contactId); + oneOf(database).addMessage(txn, message, UNKNOWN, false, false, + contactId); // Second time oneOf(database).containsContact(txn, contactId); will(returnValue(true)); @@ -1507,6 +1528,9 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { public void testMessageDependencies() throws Exception { int shutdownHandle = 12345; MessageId messageId2 = new MessageId(getRandomId()); + boolean shared = random.nextBoolean(); + boolean temporary = random.nextBoolean(); + context.checking(new Expectations() {{ // open() oneOf(database).open(key, null); @@ -1521,7 +1545,8 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { will(returnValue(true)); oneOf(database).containsMessage(txn, messageId); will(returnValue(false)); - oneOf(database).addMessage(txn, message, DELIVERED, true, null); + oneOf(database).addMessage(txn, message, DELIVERED, shared, + temporary, null); oneOf(database).mergeMessageMetadata(txn, messageId, metadata); // addMessageDependencies() oneOf(database).containsMessage(txn, messageId); @@ -1544,7 +1569,8 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { oneOf(eventBus).broadcast(with(any(MessageAddedEvent.class))); oneOf(eventBus).broadcast(with(any( MessageStateChangedEvent.class))); - oneOf(eventBus).broadcast(with(any(MessageSharedEvent.class))); + if (shared) + oneOf(eventBus).broadcast(with(any(MessageSharedEvent.class))); // endTransaction() oneOf(database).commitTransaction(txn); // close() @@ -1555,7 +1581,8 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { assertFalse(db.open(key, null)); db.transaction(false, transaction -> { - db.addLocalMessage(transaction, message, metadata, true); + db.addLocalMessage(transaction, message, metadata, shared, + temporary); Collection dependencies = new ArrayList<>(2); dependencies.add(messageId1); dependencies.add(messageId2); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java index 556634431..3e76c794f 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java @@ -567,8 +567,9 @@ public abstract class DatabasePerformanceTest extends BrambleTestCase { MessageState state = MessageState.fromValue(random.nextInt(4)); boolean shared = random.nextBoolean(); + boolean temporary = random.nextBoolean(); ContactId sender = random.nextBoolean() ? c : null; - db.addMessage(txn, m, state, shared, sender); + db.addMessage(txn, m, state, shared, temporary, sender); if (random.nextBoolean()) db.raiseRequestedFlag(txn, c, m.getId()); Metadata mm = getMetadata(METADATA_KEYS_PER_MESSAGE); @@ -597,7 +598,8 @@ public abstract class DatabasePerformanceTest extends BrambleTestCase { for (int j = 0; j < MESSAGES_PER_GROUP; j++) { Message m = getMessage(g.getId()); messages.add(m); - db.addMessage(txn, m, DELIVERED, false, null); + boolean temporary = random.nextBoolean(); + db.addMessage(txn, m, DELIVERED, false, temporary, null); Metadata mm = getMetadata(METADATA_KEYS_PER_MESSAGE); messageMeta.get(g.getId()).add(mm); db.mergeMessageMetadata(txn, m.getId(), mm); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java index 1bf92d83a..006c1e67c 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java @@ -153,7 +153,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addGroup(txn, group); assertTrue(db.containsGroup(txn, groupId)); assertFalse(db.containsMessage(txn, messageId)); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); assertTrue(db.containsMessage(txn, messageId)); db.commitTransaction(txn); db.close(); @@ -191,7 +191,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a group and a message db.addGroup(txn, group); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); // Removing the group should remove the message assertTrue(db.containsMessage(txn, messageId)); @@ -213,7 +213,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); // The contact has not seen the message, so it should be sendable Collection ids = @@ -244,7 +244,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); - db.addMessage(txn, message, UNKNOWN, true, null); + db.addMessage(txn, message, UNKNOWN, true, false, null); // The message has not been validated, so it should not be sendable Collection ids = db.getMessagesToSend(txn, contactId, @@ -288,7 +288,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertEquals(contactId, db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); // The group is invisible, so the message should not be sendable Collection ids = db.getMessagesToSend(txn, contactId, @@ -340,7 +340,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); - db.addMessage(txn, message, DELIVERED, false, null); + db.addMessage(txn, message, DELIVERED, false, false, null); // The message is not shared, so it should not be sendable Collection ids = db.getMessagesToSend(txn, contactId, @@ -371,7 +371,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); // The message is sendable, but too large to send Collection ids = @@ -402,8 +402,8 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add some messages to ack Message message1 = getMessage(groupId); MessageId messageId1 = message1.getId(); - db.addMessage(txn, message, DELIVERED, true, contactId); - db.addMessage(txn, message1, DELIVERED, true, contactId); + db.addMessage(txn, message, DELIVERED, true, false, contactId); + db.addMessage(txn, message1, DELIVERED, true, false, contactId); // Both message IDs should be returned Collection ids = db.getMessagesToAck(txn, contactId, 1234); @@ -439,7 +439,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); // Retrieve the message from the database and mark it as sent Collection ids = db.getMessagesToSend(txn, contactId, @@ -608,7 +608,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertEquals(contactId, db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); // The group is not visible so the message should not be visible assertFalse(db.containsVisibleMessage(txn, contactId, messageId)); @@ -1223,7 +1223,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a group and a message db.addGroup(txn, group); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); // Attach some metadata to the message Metadata metadata = new Metadata(); @@ -1294,7 +1294,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a group and a message db.addGroup(txn, group); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); // Attach some metadata to the message Metadata metadata = new Metadata(); @@ -1355,8 +1355,8 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a group and two messages db.addGroup(txn, group); - db.addMessage(txn, message, DELIVERED, true, null); - db.addMessage(txn, message1, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); + db.addMessage(txn, message1, DELIVERED, true, false, null); // Attach some metadata to the messages Metadata metadata = new Metadata(); @@ -1459,8 +1459,8 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a group and two messages db.addGroup(txn, group); - db.addMessage(txn, message, DELIVERED, true, null); - db.addMessage(txn, message1, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); + db.addMessage(txn, message1, DELIVERED, true, false, null); // Attach some metadata to the messages Metadata metadata = new Metadata(); @@ -1536,9 +1536,9 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a group and some messages db.addGroup(txn, group); - db.addMessage(txn, message, PENDING, true, contactId); - db.addMessage(txn, message1, PENDING, true, contactId); - db.addMessage(txn, message2, INVALID, true, contactId); + db.addMessage(txn, message, PENDING, true, false, contactId); + db.addMessage(txn, message1, PENDING, true, false, contactId); + db.addMessage(txn, message2, INVALID, true, false, contactId); // Add dependencies db.addMessageDependency(txn, message, messageId1, PENDING); @@ -1589,7 +1589,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertEquals(0, dependents.size()); // Add message 3 - db.addMessage(txn, message3, UNKNOWN, false, contactId); + db.addMessage(txn, message3, UNKNOWN, false, false, contactId); // Message 3 has message 1 as a dependent dependents = db.getMessageDependents(txn, messageId3); @@ -1601,7 +1601,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertEquals(0, dependents.size()); // Add message 4 - db.addMessage(txn, message4, UNKNOWN, false, contactId); + db.addMessage(txn, message4, UNKNOWN, false, false, contactId); // Message 4 has message 2 as a dependent dependents = db.getMessageDependents(txn, messageId4); @@ -1619,7 +1619,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a group and a message db.addGroup(txn, group); - db.addMessage(txn, message, PENDING, true, contactId); + db.addMessage(txn, message, PENDING, true, false, contactId); // Add a second group Group group1 = getGroup(clientId, 123); @@ -1629,7 +1629,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a message to the second group Message message1 = getMessage(groupId1); MessageId messageId1 = message1.getId(); - db.addMessage(txn, message1, DELIVERED, true, contactId); + db.addMessage(txn, message1, DELIVERED, true, false, contactId); // Create an ID for a missing message MessageId messageId2 = new MessageId(getRandomId()); @@ -1637,7 +1637,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add another message to the first group Message message3 = getMessage(groupId); MessageId messageId3 = message3.getId(); - db.addMessage(txn, message3, DELIVERED, true, contactId); + db.addMessage(txn, message3, DELIVERED, true, false, contactId); // Add dependencies between the messages db.addMessageDependency(txn, message, messageId1, PENDING); @@ -1680,10 +1680,10 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a group and some messages with different states db.addGroup(txn, group); - db.addMessage(txn, message1, UNKNOWN, true, contactId); - db.addMessage(txn, message2, INVALID, true, contactId); - db.addMessage(txn, message3, PENDING, true, contactId); - db.addMessage(txn, message4, DELIVERED, true, contactId); + db.addMessage(txn, message1, UNKNOWN, true, false, contactId); + db.addMessage(txn, message2, INVALID, true, false, contactId); + db.addMessage(txn, message3, PENDING, true, false, contactId); + db.addMessage(txn, message4, DELIVERED, true, false, contactId); Collection result; @@ -1713,10 +1713,10 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a group and some messages db.addGroup(txn, group); - db.addMessage(txn, message1, DELIVERED, true, contactId); - db.addMessage(txn, message2, DELIVERED, false, contactId); - db.addMessage(txn, message3, DELIVERED, false, contactId); - db.addMessage(txn, message4, DELIVERED, true, contactId); + db.addMessage(txn, message1, DELIVERED, true, false, contactId); + db.addMessage(txn, message2, DELIVERED, false, false, contactId); + db.addMessage(txn, message3, DELIVERED, false, false, contactId); + db.addMessage(txn, message4, DELIVERED, true, false, contactId); // Introduce dependencies between the messages db.addMessageDependency(txn, message1, message2.getId(), DELIVERED); @@ -1744,7 +1744,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); // The message should not be sent or seen MessageStatus status = db.getMessageStatus(txn, contactId, messageId); @@ -1878,7 +1878,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); // The message should be visible to the contact assertTrue(db.containsVisibleMessage(txn, contactId, messageId)); @@ -1961,7 +1961,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a group and a message db.addGroup(txn, group); - db.addMessage(txn, message, UNKNOWN, false, contactId); + db.addMessage(txn, message, UNKNOWN, false, false, contactId); // Walk the message through the validation and delivery states assertEquals(UNKNOWN, db.getMessageState(txn, messageId)); @@ -1988,7 +1988,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertEquals(contactId, db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); - db.addMessage(txn, message, UNKNOWN, false, null); + db.addMessage(txn, message, UNKNOWN, false, false, null); // There should be no messages to send assertEquals(Long.MAX_VALUE, db.getNextSendTime(txn, contactId)); @@ -2073,7 +2073,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); // Time: now // Retrieve the message from the database @@ -2118,7 +2118,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); - db.addMessage(txn, message, DELIVERED, true, null); + db.addMessage(txn, message, DELIVERED, true, false, null); // Time: now // Retrieve the message from the database @@ -2257,6 +2257,35 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.close(); } + @Test + public void testTemporaryMessages() throws Exception { + Message message1 = getMessage(groupId); + MessageId messageId1 = message1.getId(); + + Database db = open(false); + Connection txn = db.startTransaction(); + + // Add a group and two temporary messages + db.addGroup(txn, group); + db.addMessage(txn, message, DELIVERED, false, true, null); + db.addMessage(txn, message1, DELIVERED, false, true, null); + + // Mark one of the messages as permanent + db.setMessagePermanent(txn, messageId); + + // Remove all temporary messages + db.removeTemporaryMessages(txn); + + // The permanent message should not have been removed + assertTrue(db.containsMessage(txn, messageId)); + + // The temporary message should have been removed + assertFalse(db.containsMessage(txn, messageId1)); + + db.commitTransaction(txn); + db.close(); + } + private Database open(boolean resume) throws Exception { return open(resume, new TestMessageFactory(), new SystemClock()); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java index dd85557ea..8007c7c4a 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java @@ -172,7 +172,7 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { localVersionsBody); will(returnValue(localVersions)); oneOf(db).addLocalMessage(txn, localVersions, new Metadata(), - false); + false, false); // Inform contacts that client versions have changed oneOf(db).getContacts(txn); will(returnValue(singletonList(contact))); @@ -259,7 +259,7 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { newLocalVersionsBody); will(returnValue(newLocalVersions)); oneOf(db).addLocalMessage(txn, newLocalVersions, new Metadata(), - false); + false, false); // Inform contacts that client versions have changed oneOf(db).getContacts(txn); will(returnValue(singletonList(contact))); @@ -355,7 +355,7 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { newLocalVersionsBody); will(returnValue(newLocalVersions)); oneOf(db).addLocalMessage(txn, newLocalVersions, new Metadata(), - false); + false, false); // Inform contacts that client versions have changed oneOf(db).getContacts(txn); will(returnValue(singletonList(contact))); diff --git a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroductionManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroductionManagerImpl.java index abe54aa7a..0b5afe9df 100644 --- a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroductionManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroductionManagerImpl.java @@ -270,7 +270,7 @@ class IntroductionManagerImpl extends ConversationClientImpl private MessageId createStorageId(Transaction txn) throws DbException { Message m = clientHelper .createMessageForStoringMetadata(localGroup.getId()); - db.addLocalMessage(txn, m, new Metadata(), false); + db.addLocalMessage(txn, m, new Metadata(), false, false); return m.getId(); } diff --git a/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java index 8cd75d090..e72bc3af9 100644 --- a/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java @@ -248,7 +248,7 @@ class GroupInvitationManagerImpl extends ConversationClientImpl private MessageId createStorageId(Transaction txn, GroupId g) throws DbException { Message m = clientHelper.createMessageForStoringMetadata(g); - db.addLocalMessage(txn, m, new Metadata(), false); + db.addLocalMessage(txn, m, new Metadata(), false, false); return m.getId(); } diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java index 481d65f09..2ac7dd8d1 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java @@ -240,7 +240,7 @@ abstract class SharingManagerImpl private MessageId createStorageId(Transaction txn, GroupId g) throws DbException { Message m = clientHelper.createMessageForStoringMetadata(g); - db.addLocalMessage(txn, m, new Metadata(), false); + db.addLocalMessage(txn, m, new Metadata(), false, false); return m.getId(); } diff --git a/briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImplTest.java index bba390590..58df14e7c 100644 --- a/briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImplTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImplTest.java @@ -226,7 +226,7 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase { .createMessageForStoringMetadata(contactGroup.getId()); will(returnValue(storageMessage)); oneOf(db).addLocalMessage(txn, storageMessage, new Metadata(), - false); + false, false); }}); } diff --git a/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingManagerImplTest.java index e89e37c0d..41cf72502 100644 --- a/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingManagerImplTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingManagerImplTest.java @@ -221,7 +221,8 @@ public class BlogSharingManagerImplTest extends BrambleMockTestCase { oneOf(clientHelper) .createMessageForStoringMetadata(contactGroup.getId()); will(returnValue(message)); - oneOf(db).addLocalMessage(txn, message, new Metadata(), false); + oneOf(db).addLocalMessage(txn, message, new Metadata(), false, + false); oneOf(sessionEncoder).encodeSession(with(any(Session.class))); will(returnValue(sessionDict)); oneOf(clientHelper).mergeMessageMetadata(txn, message.getId(), From da4b63f20fbe5426026959767399263dd3cd9072 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 12 Jun 2019 15:11:34 +0100 Subject: [PATCH 2/4] Clean up ValidationManagerImplTest. --- .../validation/ValidationManagerImplTest.java | 300 ++++++++---------- 1 file changed, 131 insertions(+), 169 deletions(-) diff --git a/bramble-core/src/test/java/org/briarproject/bramble/sync/validation/ValidationManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/sync/validation/ValidationManagerImplTest.java index c586b12b5..09018c36e 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/sync/validation/ValidationManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/sync/validation/ValidationManagerImplTest.java @@ -28,7 +28,6 @@ import java.util.Map; import java.util.concurrent.Executor; import static java.util.Arrays.asList; -import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; @@ -80,24 +79,9 @@ public class ValidationManagerImplTest extends BrambleMockTestCase { @Test public void testStartAndStop() throws Exception { - Transaction txn = new Transaction(null, true); - Transaction txn1 = new Transaction(null, true); - Transaction txn2 = new Transaction(null, true); - - context.checking(new DbExpectations() {{ - // validateOutstandingMessages() - oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); - oneOf(db).getMessagesToValidate(txn); - will(returnValue(emptyList())); - // deliverOutstandingMessages() - oneOf(db).transactionWithResult(with(true), withDbCallable(txn1)); - oneOf(db).getPendingMessages(txn1); - will(returnValue(emptyList())); - // shareOutstandingMessages() - oneOf(db).transactionWithResult(with(true), withDbCallable(txn2)); - oneOf(db).getMessagesToShare(txn2); - will(returnValue(emptyList())); - }}); + expectGetMessagesToValidate(); + expectGetPendingMessages(); + expectGetMessagesToShare(); vm.startService(); vm.stopService(); @@ -106,167 +90,134 @@ public class ValidationManagerImplTest extends BrambleMockTestCase { @Test public void testMessagesAreValidatedAtStartup() throws Exception { Transaction txn = new Transaction(null, true); - Transaction txn1 = new Transaction(null, true); - Transaction txn2 = new Transaction(null, false); - Transaction txn3 = new Transaction(null, true); - Transaction txn4 = new Transaction(null, false); - Transaction txn5 = new Transaction(null, true); - Transaction txn6 = new Transaction(null, true); + Transaction txn1 = new Transaction(null, false); + Transaction txn2 = new Transaction(null, true); + Transaction txn3 = new Transaction(null, false); + + expectGetMessagesToValidate(messageId, messageId1); context.checking(new DbExpectations() {{ - // Get messages to validate - oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); - oneOf(db).getMessagesToValidate(txn); - will(returnValue(asList(messageId, messageId1))); // Load the first raw message and group - oneOf(db).transactionWithResult(with(true), withDbCallable(txn1)); - oneOf(db).getMessage(txn1, messageId); + oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + oneOf(db).getMessage(txn, messageId); will(returnValue(message)); - oneOf(db).getGroup(txn1, groupId); + oneOf(db).getGroup(txn, groupId); will(returnValue(group)); // Validate the first message: valid oneOf(validator).validateMessage(message, group); will(returnValue(validResult)); // Store the validation result for the first message - oneOf(db).transaction(with(false), withDbRunnable(txn2)); - oneOf(db).mergeMessageMetadata(txn2, messageId, metadata); + oneOf(db).transaction(with(false), withDbRunnable(txn1)); + oneOf(db).mergeMessageMetadata(txn1, messageId, metadata); // Deliver the first message - oneOf(hook).incomingMessage(txn2, message, metadata); + oneOf(hook).incomingMessage(txn1, message, metadata); will(returnValue(false)); - oneOf(db).setMessageState(txn2, messageId, DELIVERED); + oneOf(db).setMessageState(txn1, messageId, DELIVERED); // Get any pending dependents - oneOf(db).getMessageDependents(txn2, messageId); + oneOf(db).getMessageDependents(txn1, messageId); will(returnValue(emptyMap())); // Load the second raw message and group - oneOf(db).transactionWithResult(with(true), withDbCallable(txn3)); - oneOf(db).getMessage(txn3, messageId1); + oneOf(db).transactionWithResult(with(true), withDbCallable(txn2)); + oneOf(db).getMessage(txn2, messageId1); will(returnValue(message1)); - oneOf(db).getGroup(txn3, groupId); + oneOf(db).getGroup(txn2, groupId); will(returnValue(group)); // Validate the second message: invalid oneOf(validator).validateMessage(message1, group); will(throwException(new InvalidMessageException())); // Store the validation result for the second message - oneOf(db).transaction(with(false), withDbRunnable(txn4)); - oneOf(db).getMessageState(txn4, messageId1); + oneOf(db).transaction(with(false), withDbRunnable(txn3)); + oneOf(db).getMessageState(txn3, messageId1); will(returnValue(UNKNOWN)); - oneOf(db).setMessageState(txn4, messageId1, INVALID); - oneOf(db).deleteMessage(txn4, messageId1); - oneOf(db).deleteMessageMetadata(txn4, messageId1); + oneOf(db).setMessageState(txn3, messageId1, INVALID); + oneOf(db).deleteMessage(txn3, messageId1); + oneOf(db).deleteMessageMetadata(txn3, messageId1); // Recursively invalidate any dependents - oneOf(db).getMessageDependents(txn4, messageId1); + oneOf(db).getMessageDependents(txn3, messageId1); will(returnValue(emptyMap())); - // Get pending messages to deliver - oneOf(db).transactionWithResult(with(true), withDbCallable(txn5)); - oneOf(db).getPendingMessages(txn5); - will(returnValue(emptyList())); - // Get messages to share - oneOf(db).transactionWithResult(with(true), withDbCallable(txn6)); - oneOf(db).getMessagesToShare(txn6); - will(returnValue(emptyList())); }}); + expectGetPendingMessages(); + expectGetMessagesToShare(); + vm.startService(); } @Test public void testPendingMessagesAreDeliveredAtStartup() throws Exception { - Transaction txn = new Transaction(null, true); - Transaction txn1 = new Transaction(null, true); - Transaction txn2 = new Transaction(null, false); - Transaction txn3 = new Transaction(null, false); - Transaction txn4 = new Transaction(null, true); + Transaction txn = new Transaction(null, false); + Transaction txn1 = new Transaction(null, false); + + expectGetMessagesToValidate(); + expectGetPendingMessages(messageId); context.checking(new DbExpectations() {{ - // Get messages to validate - oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); - oneOf(db).getMessagesToValidate(txn); - will(returnValue(emptyList())); - // Get pending messages to deliver - oneOf(db).transactionWithResult(with(true), withDbCallable(txn1)); - oneOf(db).getPendingMessages(txn1); - will(returnValue(singletonList(messageId))); // Check whether the message is ready to deliver - oneOf(db).transaction(with(false), withDbRunnable(txn2)); - oneOf(db).getMessageState(txn2, messageId); + oneOf(db).transaction(with(false), withDbRunnable(txn)); + oneOf(db).getMessageState(txn, messageId); will(returnValue(PENDING)); - oneOf(db).getMessageDependencies(txn2, messageId); + oneOf(db).getMessageDependencies(txn, messageId); will(returnValue(singletonMap(messageId1, DELIVERED))); // Get the message and its metadata to deliver - oneOf(db).getMessage(txn2, messageId); + oneOf(db).getMessage(txn, messageId); will(returnValue(message)); - oneOf(db).getGroup(txn2, groupId); + oneOf(db).getGroup(txn, groupId); will(returnValue(group)); - oneOf(db).getMessageMetadataForValidator(txn2, messageId); + oneOf(db).getMessageMetadataForValidator(txn, messageId); will(returnValue(new Metadata())); // Deliver the message - oneOf(hook).incomingMessage(txn2, message, metadata); + oneOf(hook).incomingMessage(txn, message, metadata); will(returnValue(false)); - oneOf(db).setMessageState(txn2, messageId, DELIVERED); + oneOf(db).setMessageState(txn, messageId, DELIVERED); // Get any pending dependents - oneOf(db).getMessageDependents(txn2, messageId); + oneOf(db).getMessageDependents(txn, messageId); will(returnValue(singletonMap(messageId2, PENDING))); // Check whether the dependent is ready to deliver - oneOf(db).transaction(with(false), withDbRunnable(txn3)); - oneOf(db).getMessageState(txn3, messageId2); + oneOf(db).transaction(with(false), withDbRunnable(txn1)); + oneOf(db).getMessageState(txn1, messageId2); will(returnValue(PENDING)); - oneOf(db).getMessageDependencies(txn3, messageId2); + oneOf(db).getMessageDependencies(txn1, messageId2); will(returnValue(singletonMap(messageId1, DELIVERED))); // Get the dependent and its metadata to deliver - oneOf(db).getMessage(txn3, messageId2); + oneOf(db).getMessage(txn1, messageId2); will(returnValue(message2)); - oneOf(db).getGroup(txn3, groupId); + oneOf(db).getGroup(txn1, groupId); will(returnValue(group)); - oneOf(db).getMessageMetadataForValidator(txn3, messageId2); + oneOf(db).getMessageMetadataForValidator(txn1, messageId2); will(returnValue(metadata)); // Deliver the dependent - oneOf(hook).incomingMessage(txn3, message2, metadata); + oneOf(hook).incomingMessage(txn1, message2, metadata); will(returnValue(false)); - oneOf(db).setMessageState(txn3, messageId2, DELIVERED); + oneOf(db).setMessageState(txn1, messageId2, DELIVERED); // Get any pending dependents - oneOf(db).getMessageDependents(txn3, messageId2); + oneOf(db).getMessageDependents(txn1, messageId2); will(returnValue(emptyMap())); - - // Get messages to share - oneOf(db).transactionWithResult(with(true), withDbCallable(txn4)); - oneOf(db).getMessagesToShare(txn4); - will(returnValue(emptyList())); }}); + expectGetMessagesToShare(); + vm.startService(); } @Test public void testMessagesAreSharedAtStartup() throws Exception { - Transaction txn = new Transaction(null, true); - Transaction txn1 = new Transaction(null, true); - Transaction txn2 = new Transaction(null, true); - Transaction txn3 = new Transaction(null, false); - Transaction txn4 = new Transaction(null, false); + Transaction txn = new Transaction(null, false); + Transaction txn1 = new Transaction(null, false); + + expectGetMessagesToValidate(); + expectGetPendingMessages(); + expectGetMessagesToShare(messageId); context.checking(new DbExpectations() {{ - // No messages to validate - oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); - oneOf(db).getMessagesToValidate(txn); - will(returnValue(emptyList())); - // No pending messages to deliver - oneOf(db).transactionWithResult(with(true), withDbCallable(txn1)); - oneOf(db).getPendingMessages(txn1); - will(returnValue(emptyList())); - - // Get messages to share - oneOf(db).transactionWithResult(with(true), withDbCallable(txn2)); - oneOf(db).getMessagesToShare(txn2); - will(returnValue(singletonList(messageId))); // Share message and get dependencies - oneOf(db).transaction(with(false), withDbRunnable(txn3)); - oneOf(db).setMessageShared(txn3, messageId); - oneOf(db).getMessageDependencies(txn3, messageId); + oneOf(db).transaction(with(false), withDbRunnable(txn)); + oneOf(db).setMessageShared(txn, messageId); + oneOf(db).getMessageDependencies(txn, messageId); will(returnValue(singletonMap(messageId2, DELIVERED))); // Share dependency - oneOf(db).transaction(with(false), withDbRunnable(txn4)); - oneOf(db).setMessageShared(txn4, messageId2); - oneOf(db).getMessageDependencies(txn4, messageId2); + oneOf(db).transaction(with(false), withDbRunnable(txn1)); + oneOf(db).setMessageShared(txn1, messageId2); + oneOf(db).getMessageDependencies(txn1, messageId2); will(returnValue(emptyMap())); }}); @@ -318,49 +269,39 @@ public class ValidationManagerImplTest extends BrambleMockTestCase { throws Exception { Transaction txn = new Transaction(null, true); Transaction txn1 = new Transaction(null, true); - Transaction txn2 = new Transaction(null, true); - Transaction txn3 = new Transaction(null, false); - Transaction txn4 = new Transaction(null, true); - Transaction txn5 = new Transaction(null, true); + Transaction txn2 = new Transaction(null, false); + + expectGetMessagesToValidate(messageId, messageId1); context.checking(new DbExpectations() {{ - // Get messages to validate - oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); - oneOf(db).getMessagesToValidate(txn); - will(returnValue(asList(messageId, messageId1))); // Load the first raw message - *gasp* it's gone! - oneOf(db).transactionWithResult(with(true), withDbCallable(txn1)); - oneOf(db).getMessage(txn1, messageId); + oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + oneOf(db).getMessage(txn, messageId); will(throwException(new NoSuchMessageException())); // Load the second raw message and group - oneOf(db).transactionWithResult(with(true), withDbCallable(txn2)); - oneOf(db).getMessage(txn2, messageId1); + oneOf(db).transactionWithResult(with(true), withDbCallable(txn1)); + oneOf(db).getMessage(txn1, messageId1); will(returnValue(message1)); - oneOf(db).getGroup(txn2, groupId); + oneOf(db).getGroup(txn1, groupId); will(returnValue(group)); // Validate the second message: invalid oneOf(validator).validateMessage(message1, group); will(throwException(new InvalidMessageException())); // Invalidate the second message - oneOf(db).transaction(with(false), withDbRunnable(txn3)); - oneOf(db).getMessageState(txn3, messageId1); + oneOf(db).transaction(with(false), withDbRunnable(txn2)); + oneOf(db).getMessageState(txn2, messageId1); will(returnValue(UNKNOWN)); - oneOf(db).setMessageState(txn3, messageId1, INVALID); - oneOf(db).deleteMessage(txn3, messageId1); - oneOf(db).deleteMessageMetadata(txn3, messageId1); + oneOf(db).setMessageState(txn2, messageId1, INVALID); + oneOf(db).deleteMessage(txn2, messageId1); + oneOf(db).deleteMessageMetadata(txn2, messageId1); // Recursively invalidate dependents - oneOf(db).getMessageDependents(txn3, messageId1); + oneOf(db).getMessageDependents(txn2, messageId1); will(returnValue(emptyMap())); - // Get pending messages to deliver - oneOf(db).transactionWithResult(with(true), withDbCallable(txn4)); - oneOf(db).getPendingMessages(txn4); - will(returnValue(emptyList())); - // Get messages to share - oneOf(db).transactionWithResult(with(true), withDbCallable(txn5)); - oneOf(db).getMessagesToShare(txn5); - will(returnValue(emptyList())); }}); + expectGetPendingMessages(); + expectGetMessagesToShare(); + vm.startService(); } @@ -369,52 +310,42 @@ public class ValidationManagerImplTest extends BrambleMockTestCase { throws Exception { Transaction txn = new Transaction(null, true); Transaction txn1 = new Transaction(null, true); - Transaction txn2 = new Transaction(null, true); - Transaction txn3 = new Transaction(null, false); - Transaction txn4 = new Transaction(null, true); - Transaction txn5 = new Transaction(null, true); + Transaction txn2 = new Transaction(null, false); + + expectGetMessagesToValidate(messageId, messageId1); context.checking(new DbExpectations() {{ - // Get messages to validate - oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); - oneOf(db).getMessagesToValidate(txn); - will(returnValue(asList(messageId, messageId1))); // Load the first raw message - oneOf(db).transactionWithResult(with(true), withDbCallable(txn1)); - oneOf(db).getMessage(txn1, messageId); + oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + oneOf(db).getMessage(txn, messageId); will(returnValue(message)); // Load the group - *gasp* it's gone! - oneOf(db).getGroup(txn1, groupId); + oneOf(db).getGroup(txn, groupId); will(throwException(new NoSuchGroupException())); // Load the second raw message and group - oneOf(db).transactionWithResult(with(true), withDbCallable(txn2)); - oneOf(db).getMessage(txn2, messageId1); + oneOf(db).transactionWithResult(with(true), withDbCallable(txn1)); + oneOf(db).getMessage(txn1, messageId1); will(returnValue(message1)); - oneOf(db).getGroup(txn2, groupId); + oneOf(db).getGroup(txn1, groupId); will(returnValue(group)); // Validate the second message: invalid oneOf(validator).validateMessage(message1, group); will(throwException(new InvalidMessageException())); // Store the validation result for the second message - oneOf(db).transaction(with(false), withDbRunnable(txn3)); - oneOf(db).getMessageState(txn3, messageId1); + oneOf(db).transaction(with(false), withDbRunnable(txn2)); + oneOf(db).getMessageState(txn2, messageId1); will(returnValue(UNKNOWN)); - oneOf(db).setMessageState(txn3, messageId1, INVALID); - oneOf(db).deleteMessage(txn3, messageId1); - oneOf(db).deleteMessageMetadata(txn3, messageId1); + oneOf(db).setMessageState(txn2, messageId1, INVALID); + oneOf(db).deleteMessage(txn2, messageId1); + oneOf(db).deleteMessageMetadata(txn2, messageId1); // Recursively invalidate dependents - oneOf(db).getMessageDependents(txn3, messageId1); + oneOf(db).getMessageDependents(txn2, messageId1); will(returnValue(emptyMap())); - // Get pending messages to deliver - oneOf(db).transactionWithResult(with(true), withDbCallable(txn4)); - oneOf(db).getPendingMessages(txn4); - will(returnValue(emptyList())); - // Get messages to share - oneOf(db).transactionWithResult(with(true), withDbCallable(txn5)); - oneOf(db).getMessagesToShare(txn5); - will(returnValue(emptyList())); }}); + expectGetPendingMessages(); + expectGetMessagesToShare(); + vm.startService(); } @@ -801,4 +732,35 @@ public class ValidationManagerImplTest extends BrambleMockTestCase { vm.eventOccurred(new MessageAddedEvent(message, contactId)); } + + private void expectGetMessagesToValidate(MessageId... ids) + throws Exception { + Transaction txn = new Transaction(null, true); + + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + oneOf(db).getMessagesToValidate(txn); + will(returnValue(asList(ids))); + }}); + } + + private void expectGetPendingMessages(MessageId... ids) throws Exception { + Transaction txn = new Transaction(null, true); + + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + oneOf(db).getPendingMessages(txn); + will(returnValue(asList(ids))); + }}); + } + + private void expectGetMessagesToShare(MessageId... ids) throws Exception { + Transaction txn = new Transaction(null, true); + + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + oneOf(db).getMessagesToShare(txn); + will(returnValue(asList(ids))); + }}); + } } From 8f839e2c308831246b82003af96756cb015b955b Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 12 Jun 2019 15:21:48 +0100 Subject: [PATCH 3/4] Remove temporary messages at startup. --- .../briarproject/bramble/lifecycle/LifecycleManagerImpl.java | 5 ++++- .../bramble/lifecycle/LifecycleManagerImplTest.java | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/lifecycle/LifecycleManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/lifecycle/LifecycleManagerImpl.java index d749f616a..fc1259d2a 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/lifecycle/LifecycleManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/lifecycle/LifecycleManagerImpl.java @@ -107,8 +107,11 @@ class LifecycleManagerImpl implements LifecycleManager, MigrationListener { else logDuration(LOG, "Creating database", start); db.transaction(false, txn -> { + long start1 = now(); + db.removeTemporaryMessages(txn); + logDuration(LOG, "Removing temporary messages", start1); for (OpenDatabaseHook hook : openDatabaseHooks) { - long start1 = now(); + start1 = now(); hook.onDatabaseOpened(txn); if (LOG.isLoggable(FINE)) { logDuration(LOG, "Calling open database hook " diff --git a/bramble-core/src/test/java/org/briarproject/bramble/lifecycle/LifecycleManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/lifecycle/LifecycleManagerImplTest.java index ee6e4ae55..09ce697bf 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/lifecycle/LifecycleManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/lifecycle/LifecycleManagerImplTest.java @@ -42,6 +42,7 @@ public class LifecycleManagerImplTest extends BrambleMockTestCase { oneOf(db).open(dbKey, lifecycleManager); will(returnValue(false)); oneOf(db).transaction(with(false), withDbRunnable(txn)); + oneOf(db).removeTemporaryMessages(txn); allowing(eventBus).broadcast(with(any(LifecycleEvent.class))); }}); From 3c8b8c39e17e911bad19bc7e900a90afa1d5f727 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 12 Jun 2019 16:29:24 +0100 Subject: [PATCH 4/4] Turn commonly used variables into fields. --- .../bramble/db/DatabaseComponentImplTest.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java index 645afcdd9..8ef5f4d67 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java @@ -122,6 +122,8 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { private final KeySetId keySetId; private final PendingContactId pendingContactId; private final Random random = new Random(); + private final boolean shared = random.nextBoolean(); + private final boolean temporary = random.nextBoolean(); public DatabaseComponentImplTest() { clientId = getClientId(); @@ -244,9 +246,6 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { @Test(expected = NoSuchGroupException.class) public void testLocalMessagesAreNotStoredUnlessGroupExists() throws Exception { - boolean shared = random.nextBoolean(); - boolean temporary = random.nextBoolean(); - context.checking(new Expectations() {{ oneOf(database).startTransaction(); will(returnValue(txn)); @@ -264,9 +263,6 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { @Test public void testAddLocalMessage() throws Exception { - boolean shared = random.nextBoolean(); - boolean temporary = random.nextBoolean(); - context.checking(new Expectations() {{ oneOf(database).startTransaction(); will(returnValue(txn)); @@ -1528,8 +1524,6 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { public void testMessageDependencies() throws Exception { int shutdownHandle = 12345; MessageId messageId2 = new MessageId(getRandomId()); - boolean shared = random.nextBoolean(); - boolean temporary = random.nextBoolean(); context.checking(new Expectations() {{ // open()