From fbd38dbb947fdb3472aa4595c02097be02cb450c Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Thu, 23 Aug 2018 14:51:56 +0100
Subject: [PATCH] Throw an exception if a raw message has been deleted.

---
 .../bramble/api/client/ClientHelper.java           |  6 ------
 .../bramble/api/db/DatabaseComponent.java          |  6 +++---
 .../bramble/api/db/MessageDeletedException.java    |  9 +++++++++
 .../bramble/client/ClientHelperImpl.java           |  5 +----
 .../java/org/briarproject/bramble/db/Database.java |  7 ++++---
 .../bramble/db/DatabaseComponentImpl.java          |  1 -
 .../org/briarproject/bramble/db/JdbcDatabase.java  |  3 ++-
 .../properties/TransportPropertyManagerImpl.java   |  4 ----
 .../bramble/sync/ValidationManagerImpl.java        |  2 --
 .../versioning/ClientVersioningManagerImpl.java    | 10 +++-------
 .../briarproject/bramble/db/JdbcDatabaseTest.java  | 14 ++++++++++----
 .../briarproject/briar/blog/BlogManagerImpl.java   |  7 ++-----
 .../briarproject/briar/forum/ForumManagerImpl.java |  5 +----
 .../introduction/IntroductionManagerImpl.java      |  1 -
 .../briar/messaging/MessagingManagerImpl.java      |  4 +---
 .../privategroup/PrivateGroupManagerImpl.java      |  4 +---
 .../privategroup/invitation/MessageParserImpl.java |  1 -
 .../briar/sharing/MessageParserImpl.java           |  1 -
 18 files changed, 37 insertions(+), 53 deletions(-)
 create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/db/MessageDeletedException.java

diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java b/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java
index b0de14cdac..591ef458ea 100644
--- a/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java
+++ b/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java
@@ -16,8 +16,6 @@ import org.briarproject.bramble.api.sync.MessageId;
 import java.security.GeneralSecurityException;
 import java.util.Map;
 
-import javax.annotation.Nullable;
-
 @NotNullByDefault
 public interface ClientHelper {
 
@@ -32,16 +30,12 @@ public interface ClientHelper {
 
 	Message createMessageForStoringMetadata(GroupId g);
 
-	@Nullable
 	Message getMessage(MessageId m) throws DbException;
 
-	@Nullable
 	Message getMessage(Transaction txn, MessageId m) throws DbException;
 
-	@Nullable
 	BdfList getMessageAsList(MessageId m) throws DbException, FormatException;
 
-	@Nullable
 	BdfList getMessageAsList(Transaction txn, MessageId m) throws DbException,
 			FormatException;
 
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 f4161cee44..6de023cba1 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
@@ -298,12 +298,12 @@ public interface DatabaseComponent {
 			throws DbException;
 
 	/**
-	 * Returns the message with the given ID, in serialised form, or null if
-	 * the message has been deleted.
+	 * Returns the message with the given ID, in serialised form.
 	 * <p/>
 	 * Read-only.
+	 *
+	 * @throws MessageDeletedException if the message has been deleted
 	 */
-	@Nullable
 	byte[] getRawMessage(Transaction txn, MessageId m) throws DbException;
 
 	/**
diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/MessageDeletedException.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/MessageDeletedException.java
new file mode 100644
index 0000000000..334000255b
--- /dev/null
+++ b/bramble-api/src/main/java/org/briarproject/bramble/api/db/MessageDeletedException.java
@@ -0,0 +1,9 @@
+package org.briarproject.bramble.api.db;
+
+/**
+ * Thrown when a message that has been deleted is requested from the database.
+ * This exception may occur due to concurrent updates and does not indicate a
+ * database error.
+ */
+public class MessageDeletedException extends DbException {
+}
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 afc40d91b8..e457865243 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
@@ -127,9 +127,7 @@ class ClientHelperImpl implements ClientHelper {
 
 	@Override
 	public Message getMessage(Transaction txn, MessageId m) throws DbException {
-		byte[] raw = db.getRawMessage(txn, m);
-		if (raw == null) return null;
-		return messageFactory.createMessage(m, raw);
+		return messageFactory.createMessage(m, db.getRawMessage(txn, m));
 	}
 
 	@Override
@@ -150,7 +148,6 @@ class ClientHelperImpl implements ClientHelper {
 	public BdfList getMessageAsList(Transaction txn, MessageId m)
 			throws DbException, FormatException {
 		byte[] raw = db.getRawMessage(txn, m);
-		if (raw == null) return null;
 		return toList(raw, MESSAGE_HEADER_LENGTH,
 				raw.length - MESSAGE_HEADER_LENGTH);
 	}
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 796e4bbe40..93a573f85e 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
@@ -6,6 +6,7 @@ import org.briarproject.bramble.api.crypto.SecretKey;
 import org.briarproject.bramble.api.db.DataTooNewException;
 import org.briarproject.bramble.api.db.DataTooOldException;
 import org.briarproject.bramble.api.db.DbException;
+import org.briarproject.bramble.api.db.MessageDeletedException;
 import org.briarproject.bramble.api.db.Metadata;
 import org.briarproject.bramble.api.db.MigrationListener;
 import org.briarproject.bramble.api.identity.Author;
@@ -465,12 +466,12 @@ interface Database<T> {
 	long getNextSendTime(T txn, ContactId c) throws DbException;
 
 	/**
-	 * Returns the message with the given ID, in serialised form, or null if
-	 * the message has been deleted.
+	 * Returns the message with the given ID, in serialised form.
 	 * <p/>
 	 * Read-only.
+	 *
+	 * @throws MessageDeletedException if the message has been deleted
 	 */
-	@Nullable
 	byte[] getRawMessage(T txn, MessageId m) throws DbException;
 
 	/**
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 2913c783f4..38e31f58cd 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
@@ -487,7 +487,6 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 		return db.getMessagesToShare(txn);
 	}
 
-	@Nullable
 	@Override
 	public byte[] getRawMessage(Transaction transaction, MessageId m)
 			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 fd248f811e..b88b24e412 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
@@ -7,6 +7,7 @@ import org.briarproject.bramble.api.db.DataTooNewException;
 import org.briarproject.bramble.api.db.DataTooOldException;
 import org.briarproject.bramble.api.db.DbClosedException;
 import org.briarproject.bramble.api.db.DbException;
+import org.briarproject.bramble.api.db.MessageDeletedException;
 import org.briarproject.bramble.api.db.Metadata;
 import org.briarproject.bramble.api.db.MigrationListener;
 import org.briarproject.bramble.api.identity.Author;
@@ -2019,7 +2020,6 @@ abstract class JdbcDatabase implements Database<Connection> {
 	}
 
 	@Override
-	@Nullable
 	public byte[] getRawMessage(Connection txn, MessageId m)
 			throws DbException {
 		PreparedStatement ps = null;
@@ -2034,6 +2034,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 			if (rs.next()) throw new DbStateException();
 			rs.close();
 			ps.close();
+			if (raw == null) throw new MessageDeletedException();
 			return raw;
 		} catch (SQLException e) {
 			tryToClose(rs);
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 4b4c360dbd..3c933669ed 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
@@ -164,7 +164,6 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 			for (Entry<TransportId, LatestUpdate> e : latest.entrySet()) {
 				BdfList message = clientHelper.getMessageAsList(txn,
 						e.getValue().messageId);
-				if (message == null) throw new DbException();
 				local.put(e.getKey(), parseProperties(message));
 			}
 			return local;
@@ -187,7 +186,6 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 					// Retrieve and parse the latest local properties
 					BdfList message = clientHelper.getMessageAsList(txn,
 							latest.messageId);
-					if (message == null) throw new DbException();
 					p = parseProperties(message);
 				}
 				db.commitTransaction(txn);
@@ -227,7 +225,6 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 			// Retrieve and parse the latest remote properties
 			BdfList message =
 					clientHelper.getMessageAsList(txn, latest.messageId);
-			if (message == null) throw new DbException();
 			return parseProperties(message);
 		} catch (FormatException e) {
 			throw new DbException(e);
@@ -265,7 +262,6 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 				} else {
 					BdfList message = clientHelper.getMessageAsList(txn,
 							latest.messageId);
-					if (message == null) throw new DbException();
 					TransportProperties old = parseProperties(message);
 					merged = new TransportProperties(old);
 					merged.putAll(p);
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/sync/ValidationManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/sync/ValidationManagerImpl.java
index f8bb8a616b..64edbbb973 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/sync/ValidationManagerImpl.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/sync/ValidationManagerImpl.java
@@ -129,7 +129,6 @@ class ValidationManagerImpl implements ValidationManager, Service,
 			try {
 				MessageId id = unvalidated.poll();
 				byte[] raw = db.getRawMessage(txn, id);
-				if (raw == null) throw new DbException();
 				m = messageFactory.createMessage(id, raw);
 				g = db.getGroup(txn, m.getGroupId());
 				db.commitTransaction(txn);
@@ -198,7 +197,6 @@ class ValidationManagerImpl implements ValidationManager, Service,
 						invalidate = getDependentsToInvalidate(txn, id);
 					} else if (allDelivered) {
 						byte[] raw = db.getRawMessage(txn, id);
-						if (raw == null) throw new DbException();
 						Message m = messageFactory.createMessage(id, raw);
 						Group g = db.getGroup(txn, m.getGroupId());
 						ClientId c = g.getClientId();
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 4c73abfe37..c246c11b6d 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
@@ -139,7 +139,7 @@ class ClientVersioningManagerImpl implements ClientVersioningManager, Client,
 	}
 
 	@Override
-	public void stopService() throws ServiceException {
+	public void stopService() {
 	}
 
 	@Override
@@ -274,9 +274,7 @@ class ClientVersioningManagerImpl implements ClientVersioningManager, Client,
 	private List<ClientVersion> loadClientVersions(Transaction txn,
 			MessageId m) throws DbException {
 		try {
-			BdfList body = clientHelper.getMessageAsList(txn, m);
-			if (body == null) throw new DbException();
-			return parseClientVersions(body);
+			return parseClientVersions(clientHelper.getMessageAsList(txn, m));
 		} catch (FormatException e) {
 			throw new DbException(e);
 		}
@@ -359,9 +357,7 @@ class ClientVersioningManagerImpl implements ClientVersioningManager, Client,
 
 	private Update loadUpdate(Transaction txn, MessageId m) throws DbException {
 		try {
-			BdfList body = clientHelper.getMessageAsList(txn, m);
-			if (body == null) throw new DbException();
-			return parseUpdate(body);
+			return parseUpdate(clientHelper.getMessageAsList(txn, m));
 		} catch (FormatException e) {
 			throw new DbException(e);
 		}
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 e88038258a..486ac0e824 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
@@ -5,6 +5,7 @@ import org.briarproject.bramble.api.contact.ContactId;
 import org.briarproject.bramble.api.crypto.SecretKey;
 import org.briarproject.bramble.api.db.DatabaseConfig;
 import org.briarproject.bramble.api.db.DbException;
+import org.briarproject.bramble.api.db.MessageDeletedException;
 import org.briarproject.bramble.api.db.Metadata;
 import org.briarproject.bramble.api.identity.Author;
 import org.briarproject.bramble.api.identity.LocalAuthor;
@@ -1654,8 +1655,8 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase {
 		ids = db.getMessagesToOffer(txn, contactId, 100);
 		assertEquals(singletonList(messageId), ids);
 
-		// The raw message should not be null
-		assertNotNull(db.getRawMessage(txn, messageId));
+		// The raw message should be available
+		assertArrayEquals(raw, db.getRawMessage(txn, messageId));
 
 		// Delete the message
 		db.deleteMessage(txn, messageId);
@@ -1669,8 +1670,13 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase {
 		ids = db.getMessagesToOffer(txn, contactId, 100);
 		assertTrue(ids.isEmpty());
 
-		// The raw message should be null
-		assertNull(db.getRawMessage(txn, messageId));
+		// Requesting the raw message should throw an exception
+		try {
+			db.getRawMessage(txn, messageId);
+			fail();
+		} catch (MessageDeletedException expected) {
+			// Expected
+		}
 
 		db.commitTransaction(txn);
 		db.close();
diff --git a/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java
index 6a05b9a154..7718b0d68a 100644
--- a/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java
@@ -94,7 +94,7 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
 	}
 
 	@Override
-	public void addingContact(Transaction txn, Contact c) throws DbException {
+	public void addingContact(Transaction txn, Contact c) {
 	}
 
 	@Override
@@ -311,7 +311,6 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
 
 		// Get body of message to be wrapped
 		BdfList body = clientHelper.getMessageAsList(txn, header.getId());
-		if (body == null) throw new DbException();
 		long timestamp = header.getTimestamp();
 		Message wrappedMessage;
 
@@ -459,9 +458,7 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
 	@Override
 	public String getPostBody(MessageId m) throws DbException {
 		try {
-			BdfList message = clientHelper.getMessageAsList(m);
-			if (message == null) throw new DbException();
-			return getPostBody(message);
+			return getPostBody(clientHelper.getMessageAsList(m));
 		} catch (FormatException e) {
 			throw new DbException(e);
 		}
diff --git a/briar-core/src/main/java/org/briarproject/briar/forum/ForumManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/forum/ForumManagerImpl.java
index 3a38642080..58b35ea656 100644
--- a/briar-core/src/main/java/org/briarproject/briar/forum/ForumManagerImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/forum/ForumManagerImpl.java
@@ -204,10 +204,7 @@ class ForumManagerImpl extends BdfIncomingMessageHook implements ForumManager {
 	@Override
 	public String getPostBody(MessageId m) throws DbException {
 		try {
-			// Parent ID, author, forum post body, signature
-			BdfList body = clientHelper.getMessageAsList(m);
-			if (body == null) throw new DbException();
-			return getPostBody(body);
+			return getPostBody(clientHelper.getMessageAsList(m));
 		} catch (FormatException e) {
 			throw new DbException(e);
 		}
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 64fc1b40d4..b6a56f7091 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
@@ -463,7 +463,6 @@ class IntroductionManagerImpl extends ConversationClientImpl
 			author = session.getRemote().author;
 		} else throw new AssertionError();
 		Message msg = clientHelper.getMessage(txn, m);
-		if (msg == null) throw new AssertionError();
 		BdfList body = clientHelper.toList(msg);
 		RequestMessage rm = messageParser.parseRequestMessage(msg, body);
 		String message = rm.getMessage();
diff --git a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java
index 555a209905..4ee791aea7 100644
--- a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java
@@ -217,9 +217,7 @@ class MessagingManagerImpl extends ConversationClientImpl
 	public String getMessageBody(MessageId m) throws DbException {
 		try {
 			// 0: private message body
-			BdfList message = clientHelper.getMessageAsList(m);
-			if (message == null) throw new DbException();
-			return message.getString(0);
+			return clientHelper.getMessageAsList(m).getString(0);
 		} catch (FormatException e) {
 			throw new DbException(e);
 		}
diff --git a/briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java
index 531282fdf0..4a615fe343 100644
--- a/briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/privategroup/PrivateGroupManagerImpl.java
@@ -301,9 +301,7 @@ class PrivateGroupManagerImpl extends BdfIncomingMessageHook
 	@Override
 	public String getMessageBody(MessageId m) throws DbException {
 		try {
-			BdfList body = clientHelper.getMessageAsList(m);
-			if (body == null) throw new DbException();
-			return getMessageBody(body);
+			return getMessageBody(clientHelper.getMessageAsList(m));
 		} catch (FormatException e) {
 			throw new DbException(e);
 		}
diff --git a/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/MessageParserImpl.java b/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/MessageParserImpl.java
index 5c1ad6b7ad..aa682fa52f 100644
--- a/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/MessageParserImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/MessageParserImpl.java
@@ -91,7 +91,6 @@ class MessageParserImpl implements MessageParser {
 	public InviteMessage getInviteMessage(Transaction txn, MessageId m)
 			throws DbException, FormatException {
 		Message message = clientHelper.getMessage(txn, m);
-		if (message == null) throw new DbException();
 		BdfList body = clientHelper.toList(message);
 		return parseInviteMessage(message, body);
 	}
diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/MessageParserImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/MessageParserImpl.java
index eb294975ac..32659fe4fd 100644
--- a/briar-core/src/main/java/org/briarproject/briar/sharing/MessageParserImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/sharing/MessageParserImpl.java
@@ -78,7 +78,6 @@ abstract class MessageParserImpl<S extends Shareable>
 	public InviteMessage<S> getInviteMessage(Transaction txn, MessageId m)
 			throws DbException, FormatException {
 		Message message = clientHelper.getMessage(txn, m);
-		if (message == null) throw new DbException();
 		BdfList body = clientHelper.toList(message);
 		return parseInviteMessage(message, body);
 	}
-- 
GitLab