From 5e98126e774ef458726f9fe968cf6e2557069067 Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Thu, 9 Nov 2017 10:58:51 +0000
Subject: [PATCH] Completely remove old local updates from the database.

---
 .../bramble/api/db/DatabaseComponent.java     | 10 ++++--
 .../bramble/db/DatabaseComponentImpl.java     | 10 ++++++
 .../TransportPropertyManagerImpl.java         | 34 ++++++++++---------
 .../TransportPropertyManagerImplTest.java     | 12 +++----
 4 files changed, 40 insertions(+), 26 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 1de66e9342..f12a0680bb 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
@@ -122,8 +122,9 @@ public interface DatabaseComponent {
 			throws DbException;
 
 	/**
-	 * Deletes the message with the given ID. The message ID and any other
-	 * associated data are not deleted.
+	 * Deletes the message with the given ID. Unlike
+	 * {@link #removeMessage(Transaction, MessageId)}, the message ID and any
+	 * other associated data are not deleted.
 	 */
 	void deleteMessage(Transaction txn, MessageId m) throws DbException;
 
@@ -452,6 +453,11 @@ public interface DatabaseComponent {
 	 */
 	void removeLocalAuthor(Transaction txn, AuthorId a) throws DbException;
 
+	/**
+	 * Removes a message (and all associated state) from the database.
+	 */
+	void removeMessage(Transaction txn, MessageId m) throws DbException;
+
 	/**
 	 * Removes a transport (and all associated state) from the database.
 	 */
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 9e8d31a614..8665c8675a 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
@@ -770,6 +770,16 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 		transaction.attach(new LocalAuthorRemovedEvent(a));
 	}
 
+	@Override
+	public void removeMessage(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.removeMessage(txn, m);
+	}
+
 	@Override
 	public void removeTransport(Transaction transaction, TransportId t)
 			throws DbException {
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java
index 0aa1c7e9f8..d002a6b02b 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java
@@ -269,10 +269,8 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 					storeMessage(txn, localGroup.getId(), t, merged, version,
 							true, false);
 					// Delete the previous update, if any
-					if (latest != null) {
-						db.deleteMessage(txn, latest.messageId);
-						db.deleteMessageMetadata(txn, latest.messageId);
-					}
+					if (latest != null)
+						db.removeMessage(txn, latest.messageId);
 					// Store the merged properties in each contact's group
 					for (Contact c : db.getContacts(txn)) {
 						Group g = getContactGroup(c);
@@ -281,10 +279,8 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 						storeMessage(txn, g.getId(), t, merged, version,
 								true, true);
 						// Delete the previous update, if any
-						if (latest != null) {
-							db.deleteMessage(txn, latest.messageId);
-							db.deleteMessageMetadata(txn, latest.messageId);
-						}
+						if (latest != null)
+							db.removeMessage(txn, latest.messageId);
 					}
 				}
 				db.commitTransaction(txn);
@@ -338,13 +334,11 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 				latestUpdates.put(t, new LatestUpdate(e.getKey(), version));
 			} else if (version > latest.version) {
 				// This update is newer - delete the previous one
-				db.deleteMessage(txn, latest.messageId);
-				db.deleteMessageMetadata(txn, latest.messageId);
+				db.removeMessage(txn, latest.messageId);
 				latestUpdates.put(t, new LatestUpdate(e.getKey(), version));
 			} else {
 				// We've already found a newer update - delete this one
-				db.deleteMessage(txn, e.getKey());
-				db.deleteMessageMetadata(txn, e.getKey());
+				db.removeMessage(txn, e.getKey());
 			}
 		}
 		return latestUpdates;
@@ -366,13 +360,21 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 					latest = new LatestUpdate(e.getKey(), version);
 				} else if (version > latest.version) {
 					// This update is newer - delete the previous one
-					db.deleteMessage(txn, latest.messageId);
-					db.deleteMessageMetadata(txn, latest.messageId);
+					if (local) {
+						db.removeMessage(txn, latest.messageId);
+					} else {
+						db.deleteMessage(txn, latest.messageId);
+						db.deleteMessageMetadata(txn, latest.messageId);
+					}
 					latest = new LatestUpdate(e.getKey(), version);
 				} else {
 					// We've already found a newer update - delete this one
-					db.deleteMessage(txn, e.getKey());
-					db.deleteMessageMetadata(txn, e.getKey());
+					if (local) {
+						db.removeMessage(txn, e.getKey());
+					} else {
+						db.deleteMessage(txn, e.getKey());
+						db.deleteMessageMetadata(txn, e.getKey());
+					}
 				}
 			}
 		}
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java
index d200fe3571..245a4d01df 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java
+++ b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java
@@ -587,8 +587,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			expectStoreMessage(txn, localGroup.getId(), "foo",
 					fooPropertiesDict, 2, true, false);
 			// Delete the previous update
-			oneOf(db).deleteMessage(txn, localGroupUpdateId);
-			oneOf(db).deleteMessageMetadata(txn, localGroupUpdateId);
+			oneOf(db).removeMessage(txn, localGroupUpdateId);
 			// Store the merged properties in each contact's group, version 2
 			oneOf(db).getContacts(txn);
 			will(returnValue(Collections.singletonList(contact)));
@@ -600,8 +599,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			expectStoreMessage(txn, contactGroup.getId(), "foo",
 					fooPropertiesDict, 2, true, true);
 			// Delete the previous update
-			oneOf(db).deleteMessage(txn, contactGroupUpdateId);
-			oneOf(db).deleteMessageMetadata(txn, contactGroupUpdateId);
+			oneOf(db).removeMessage(txn, contactGroupUpdateId);
 			oneOf(db).commitTransaction(txn);
 			oneOf(db).endTransaction(txn);
 		}});
@@ -676,10 +674,8 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
 					localGroup.getId());
 			will(returnValue(messageMetadata));
-			oneOf(db).deleteMessage(txn, barVersion1);
-			oneOf(db).deleteMessageMetadata(txn, barVersion1);
-			oneOf(db).deleteMessage(txn, barVersion2);
-			oneOf(db).deleteMessageMetadata(txn, barVersion2);
+			oneOf(db).removeMessage(txn, barVersion1);
+			oneOf(db).removeMessage(txn, barVersion2);
 			// Retrieve and parse the latest local properties
 			oneOf(clientHelper).getMessageAsList(txn, fooVersion999);
 			will(returnValue(fooUpdate));
-- 
GitLab