From 3eb3dbde09e228fac5c8cf022f03918181adc202 Mon Sep 17 00:00:00 2001 From: Daniel Lublin Date: Wed, 8 Dec 2021 09:15:03 +0100 Subject: [PATCH 1/2] Add database method to reset retransmission times Will be used to ensure messages are not stranded on a Mailbox, when such is added, removed, or otherwise changed. Closes #2190. --- .../bramble/api/db/DatabaseComponent.java | 8 +++ .../org/briarproject/bramble/db/Database.java | 11 ++++- .../bramble/db/DatabaseComponentImpl.java | 12 +++++ .../briarproject/bramble/db/JdbcDatabase.java | 28 +++++++++++ .../bramble/db/JdbcDatabaseTest.java | 49 +++++++++++++++++++ 5 files changed, 106 insertions(+), 2 deletions(-) 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 3413dc080..3c736aca3 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 @@ -471,6 +471,14 @@ public interface DatabaseComponent extends TransactionManager { Map getUnackedMessagesToSend(Transaction txn, ContactId c) throws DbException; + /** + * Reset the transmission count, expiry time and ETA of all messages that + * are eligible to be sent to the given contact. Messages are selected in + * the same way as {@link #getUnackedMessagesToSend(Transaction, ContactId)} + */ + void resetUnackedMessagesToSend(Transaction txn, ContactId c) + throws DbException; + /** * Returns the total length, including headers, of all messages that are * eligible to be sent to the given contact. This may include messages 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 ab2376a4b..78a91f592 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 @@ -757,6 +757,13 @@ interface Database { */ void resetExpiryTime(T txn, ContactId c, MessageId m) throws DbException; + /** + * Resets the transmission count, expiry time and ETA of the given messages + * with respect to the given contact. + */ + void resetExpiryTimeAndEta(T txn, ContactId c, Collection ids) + throws DbException; + /** * Sets the cleanup timer duration for the given message. This does not * start the message's cleanup timer. @@ -845,8 +852,8 @@ interface Database { * of the given message with respect to the given contact, using the latency * of the transport over which it was sent. */ - void updateExpiryTimeAndEta(T txn, ContactId c, MessageId m, long maxLatency) - throws DbException; + void updateExpiryTimeAndEta(T txn, ContactId c, MessageId m, + long maxLatency) throws DbException; /** * Stores the given transport keys, deleting any keys they have replaced. 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 4cbf4b865..4cbad6ee0 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 @@ -750,6 +750,18 @@ class DatabaseComponentImpl implements DatabaseComponent { return db.getUnackedMessagesToSend(txn, c); } + @Override + public void resetUnackedMessagesToSend(Transaction transaction, ContactId c) + throws DbException { + T txn = unbox(transaction); + if (!db.containsContact(txn, c)) + throw new NoSuchContactException(); + Collection unackedToSend = + new ArrayList<>(db.getUnackedMessagesToSend(txn, c).keySet()); + if (unackedToSend.isEmpty()) return; + db.resetExpiryTimeAndEta(txn, c, unackedToSend); + } + @Override public long getUnackedMessageBytesToSend(Transaction transaction, ContactId c) throws DbException { 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 7f236eda2..49f748c45 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 @@ -3290,6 +3290,34 @@ abstract class JdbcDatabase implements Database { } } + @Override + public void resetExpiryTimeAndEta(Connection txn, ContactId c, + Collection ids) + throws DbException { + PreparedStatement ps = null; + try { + String sql = "UPDATE statuses SET expiry = 0, txCount = 0, eta = 0" + + " WHERE contactId = ? AND messageId = ?"; + ps = txn.prepareStatement(sql); + ps.setInt(1, c.getInt()); + for (MessageId m : ids) { + ps.setBytes(2, m.getBytes()); + ps.addBatch(); + } + int[] batchAffected = ps.executeBatch(); + if (batchAffected.length != ids.size()) { + throw new DbStateException(); + } + for (int rows : batchAffected) { + if (rows < 0 || rows > 1) throw new DbStateException(); + } + ps.close(); + } catch (SQLException e) { + tryToClose(ps, LOG, WARNING); + throw new DbException(e); + } + } + @Override public void setCleanupTimerDuration(Connection txn, MessageId m, long duration) throws DbException { 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 649b09369..cddcca6d6 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 @@ -2197,6 +2197,55 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.close(); } + @Test + public void testResetRetransmissionTimes() throws Exception { + long now = System.currentTimeMillis(); + AtomicLong time = new AtomicLong(now); + Database db = + open(false, new TestMessageFactory(), new SettableClock(time)); + Connection txn = db.startTransaction(); + + // Add a contact, a shared group and a shared message + db.addIdentity(txn, identity); + assertEquals(contactId, + db.addContact(txn, author, localAuthor.getId(), null, true)); + db.addGroup(txn, group); + db.addGroupVisibility(txn, contactId, groupId, true); + db.addMessage(txn, message, DELIVERED, true, false, null); + + // Time: now + // Retrieve the message from the database + Collection ids = db.getMessagesToSend(txn, contactId, + ONE_MEGABYTE, MAX_LATENCY); + assertEquals(singletonList(messageId), ids); + + // Time: now + // Mark the message as sent + db.updateExpiryTimeAndEta(txn, contactId, messageId, MAX_LATENCY); + + // The message should expire after 2 * MAX_LATENCY + assertEquals(now + MAX_LATENCY * 2, db.getNextSendTime(txn, contactId)); + + // Time: now + MAX_LATENCY * 2 - 1 + // The message should not yet be sendable + time.set(now + MAX_LATENCY * 2 - 1); + ids = db.getMessagesToSend(txn, contactId, ONE_MEGABYTE, MAX_LATENCY); + assertTrue(ids.isEmpty()); + + // Reset the retransmission times + db.resetExpiryTimeAndEta(txn, contactId, singletonList(messageId)); + + // The message should have infinitely short expiry + assertEquals(0, db.getNextSendTime(txn, contactId)); + + // The message should be sendable + ids = db.getMessagesToSend(txn, contactId, ONE_MEGABYTE, MAX_LATENCY); + assertFalse(ids.isEmpty()); + + db.commitTransaction(txn); + db.close(); + } + @Test public void testCompactionTime() throws Exception { MessageFactory messageFactory = new TestMessageFactory(); From 93a03d7e15ad3133a31c39bd57122714ca245621 Mon Sep 17 00:00:00 2001 From: Daniel Lublin Date: Thu, 9 Dec 2021 12:52:43 +0100 Subject: [PATCH 2/2] Reset using a single db query --- .../bramble/api/db/DatabaseComponent.java | 4 ++-- .../org/briarproject/bramble/db/Database.java | 8 ++++---- .../bramble/db/DatabaseComponentImpl.java | 5 +---- .../briarproject/bramble/db/JdbcDatabase.java | 19 +++++++------------ .../bramble/db/JdbcDatabaseTest.java | 2 +- 5 files changed, 15 insertions(+), 23 deletions(-) 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 3c736aca3..ac1566c01 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 @@ -473,8 +473,8 @@ public interface DatabaseComponent extends TransactionManager { /** * Reset the transmission count, expiry time and ETA of all messages that - * are eligible to be sent to the given contact. Messages are selected in - * the same way as {@link #getUnackedMessagesToSend(Transaction, ContactId)} + * are eligible to be sent to the given contact. This includes messages that + * have already been sent and are not yet due for retransmission. */ void resetUnackedMessagesToSend(Transaction txn, ContactId c) throws DbException; 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 78a91f592..a90019bc9 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 @@ -758,11 +758,11 @@ interface Database { void resetExpiryTime(T txn, ContactId c, MessageId m) throws DbException; /** - * Resets the transmission count, expiry time and ETA of the given messages - * with respect to the given contact. + * Resets the transmission count, expiry time and ETA of all messages that + * are eligible to be sent to the given contact. This includes messages that + * have already been sent and are not yet due for retransmission. */ - void resetExpiryTimeAndEta(T txn, ContactId c, Collection ids) - throws DbException; + void resetUnackedMessagesToSend(T txn, ContactId c) throws DbException; /** * Sets the cleanup timer duration for the given message. This does not 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 4cbad6ee0..b137fcfc3 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 @@ -756,10 +756,7 @@ class DatabaseComponentImpl implements DatabaseComponent { T txn = unbox(transaction); if (!db.containsContact(txn, c)) throw new NoSuchContactException(); - Collection unackedToSend = - new ArrayList<>(db.getUnackedMessagesToSend(txn, c).keySet()); - if (unackedToSend.isEmpty()) return; - db.resetExpiryTimeAndEta(txn, c, unackedToSend); + db.resetUnackedMessagesToSend(txn, c); } @Override 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 49f748c45..f2b71dda9 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 @@ -3291,26 +3291,21 @@ abstract class JdbcDatabase implements Database { } @Override - public void resetExpiryTimeAndEta(Connection txn, ContactId c, - Collection ids) + public void resetUnackedMessagesToSend(Connection txn, ContactId c) throws DbException { PreparedStatement ps = null; try { String sql = "UPDATE statuses SET expiry = 0, txCount = 0, eta = 0" - + " WHERE contactId = ? AND messageId = ?"; + + " WHERE contactId = ? AND state = ?" + + " AND groupShared = TRUE AND messageShared = TRUE" + + " AND deleted = FALSE AND seen = FALSE"; ps = txn.prepareStatement(sql); ps.setInt(1, c.getInt()); - for (MessageId m : ids) { - ps.setBytes(2, m.getBytes()); - ps.addBatch(); - } - int[] batchAffected = ps.executeBatch(); - if (batchAffected.length != ids.size()) { + ps.setInt(2, DELIVERED.getValue()); + int affected = ps.executeUpdate(); + if (affected < 0) { throw new DbStateException(); } - for (int rows : batchAffected) { - if (rows < 0 || rows > 1) throw new DbStateException(); - } ps.close(); } catch (SQLException e) { tryToClose(ps, LOG, WARNING); 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 cddcca6d6..ae344fa05 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 @@ -2233,7 +2233,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertTrue(ids.isEmpty()); // Reset the retransmission times - db.resetExpiryTimeAndEta(txn, contactId, singletonList(messageId)); + db.resetUnackedMessagesToSend(txn, contactId); // The message should have infinitely short expiry assertEquals(0, db.getNextSendTime(txn, contactId));