Merge branch '758-db-exception-for-deleted-messages' into 'master'

Throw an exception if a deleted message is requested from the DB

Closes #758

See merge request briar/briar!904

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;
 

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;
 
 	/**

bramble-api/src/main/java/org/briarproject/bramble/api/db/MessageDeletedException.java

+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 {
+}

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);
 	}

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;
 
 	/**

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 {

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);

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);

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();

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);
 		}

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();

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);
 		}

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);
 		}

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();

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);
 		}

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);
 		}

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);
 	}

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);
 	}