Merge branch '1217-get-message-status' into 'master'

Return default message status if group is invisible

Closes #1217

See merge request akwizgran/briar!783

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

@@ -136,8 +136,8 @@ public interface DatabaseComponent {
 
 	/**
 	 * Deletes the message with the given ID. Unlike
-	 * {@link #removeMessage(Transaction, MessageId)}, the message ID and any
+	 * {@link #removeMessage(Transaction, MessageId)}, the message ID,
-	 * other associated data are not deleted.
+	 * dependencies, metadata, and any other associated state are not deleted.
 	 */
 	void deleteMessage(Transaction txn, MessageId m) throws DbException;
 
@@ -268,7 +268,7 @@ public interface DatabaseComponent {
 	Collection<LocalAuthor> getLocalAuthors(Transaction txn) throws DbException;
 
 	/**
-	 * Returns the IDs of all messages in the given group.
+	 * Returns the IDs of all delivered messages in the given group.
 	 * <p/>
 	 * Read-only.
 	 */
@@ -319,9 +319,9 @@ public interface DatabaseComponent {
 			throws DbException;
 
 	/**
-	 * Returns the metadata for any messages in the given group with metadata
+	 * Returns the metadata for any delivered messages in the given group with
-	 * that matches all entries in the given query. If the query is empty, the
+	 * metadata that matches all entries in the given query. If the query is
-	 * metadata for all messages is returned.
+	 * empty, the metadata for all delivered messages is returned.
 	 * <p/>
 	 * Read-only.
 	 */
@@ -337,8 +337,8 @@ public interface DatabaseComponent {
 			throws DbException;
 
 	/**
-	 * Returns the metadata for the given delivered and pending message.
+	 * Returns the metadata for the given delivered or pending message.
-	 * This is meant to be only used by the ValidationManager
+	 * This is only meant to be used by the ValidationManager.
 	 * <p/>
 	 * Read-only.
 	 */
@@ -346,8 +346,8 @@ public interface DatabaseComponent {
 			throws DbException;
 
 	/**
-	 * Returns the status of all messages in the given group with respect to
+	 * Returns the status of all delivered messages in the given group with
-	 * the given contact.
+	 * respect to the given contact.
 	 * <p/>
 	 * Read-only.
 	 */
@@ -382,8 +382,8 @@ public interface DatabaseComponent {
 	State getMessageState(Transaction txn, MessageId m) throws DbException;
 
 	/**
-	 * Returns the status of the given message with respect to the given
+	 * Returns the status of the given delivered message with respect to the
-	 * contact.
+	 * given contact.
 	 * <p/>
 	 * Read-only.
 	 */

bramble-core/src/main/java/org/briarproject/bramble/db/Database.java

@@ -322,16 +322,16 @@ interface Database<T> {
 			throws DbException;
 
 	/**
-	 * Returns the IDs of all messages in the given group.
+	 * Returns the IDs of all delivered messages in the given group.
 	 * <p/>
 	 * Read-only.
 	 */
 	Collection<MessageId> getMessageIds(T txn, GroupId g) throws DbException;
 
 	/**
-	 * Returns the IDs of any messages in the given group with metadata
+	 * Returns the IDs of any delivered messages in the given group with
-	 * matching all entries in the given query. If the query is empty, the IDs
+	 * metadata that matches all entries in the given query. If the query is
-	 * of all messages are returned.
+	 * empty, the IDs of all delivered messages are returned.
 	 * <p/>
 	 * Read-only.
 	 */
@@ -347,9 +347,9 @@ interface Database<T> {
 			throws DbException;
 
 	/**
-	 * Returns the metadata for any messages in the given group with metadata
+	 * Returns the metadata for any delivered messages in the given group with
-	 * matching all entries in the given query. If the query is empty, the
+	 * metadata that matches all entries in the given query. If the query is
-	 * metadata for all messages is returned.
+	 * empty, the metadata for all delivered messages is returned.
 	 * <p/>
 	 * Read-only.
 	 */
@@ -357,7 +357,8 @@ interface Database<T> {
 			Metadata query) throws DbException;
 
 	/**
-	 * Returns the metadata for the given delivered message.
+	 * Returns the metadata for the given delivered or pending message.
+	 * This is only meant to be used by the ValidationManager.
 	 * <p/>
 	 * Read-only.
 	 */
@@ -365,7 +366,7 @@ interface Database<T> {
 			throws DbException;
 
 	/**
-	 * Returns the metadata for the given message.
+	 * Returns the metadata for the given delivered message.
 	 * <p/>
 	 * Read-only.
 	 */
@@ -379,8 +380,8 @@ interface Database<T> {
 	State getMessageState(T txn, MessageId m) throws DbException;
 
 	/**
-	 * Returns the status of all messages in the given group with respect
+	 * Returns the status of all delivered messages in the given group with
-	 * to the given contact.
+	 * respect to the given contact.
 	 * <p/>
 	 * Read-only.
 	 */
@@ -388,11 +389,13 @@ interface Database<T> {
 			throws DbException;
 
 	/**
-	 * Returns the status of the given message with respect to the given
+	 * Returns the status of the given delivered message with respect to the
+	 * given contact, or null if the message's group is invisible to the
 	 * contact.
 	 * <p/>
 	 * Read-only.
 	 */
+	@Nullable
 	MessageStatus getMessageStatus(T txn, ContactId c, MessageId m)
 			throws DbException;
 

bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java

@@ -560,6 +560,13 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 			throw new NoSuchContactException();
 		if (!db.containsGroup(txn, g))
 			throw new NoSuchGroupException();
+		if (db.getGroupVisibility(txn, c, g) == INVISIBLE) {
+			// No status rows exist - return default statuses
+			Collection<MessageStatus> statuses = new ArrayList<>();
+			for (MessageId m : db.getMessageIds(txn, g))
+				statuses.add(new MessageStatus(m, c, false, false));
+			return statuses;
+		}
 		return db.getMessageStatus(txn, c, g);
 	}
 
@@ -571,7 +578,9 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 			throw new NoSuchContactException();
 		if (!db.containsMessage(txn, m))
 			throw new NoSuchMessageException();
-		return db.getMessageStatus(txn, c, m);
+		MessageStatus status = db.getMessageStatus(txn, c, m);
+		if (status == null) return new MessageStatus(m, c, false, false);
+		return status;
 	}
 
 	@Override

bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java

@@ -1516,32 +1516,11 @@ abstract class JdbcDatabase implements Database<Connection> {
 		PreparedStatement ps = null;
 		ResultSet rs = null;
 		try {
-			String sql = "SELECT messageId FROM messages WHERE groupId = ?";
+			String sql = "SELECT messageId FROM messages"
+					+ " WHERE groupId = ? AND state = ?";
 			ps = txn.prepareStatement(sql);
 			ps.setBytes(1, g.getBytes());
-			rs = ps.executeQuery();
+			ps.setInt(2, DELIVERED.getValue());
-			List<MessageId> ids = new ArrayList<>();
-			while (rs.next()) ids.add(new MessageId(rs.getBytes(1)));
-			rs.close();
-			ps.close();
-			return ids;
-		} catch (SQLException e) {
-			tryToClose(rs);
-			tryToClose(ps);
-			throw new DbException(e);
-		}
-	}
-
-	private Collection<MessageId> getMessageIds(Connection txn, GroupId g,
-			State state) throws DbException {
-		PreparedStatement ps = null;
-		ResultSet rs = null;
-		try {
-			String sql = "SELECT messageId FROM messages"
-					+ " WHERE state = ? AND groupId = ?";
-			ps = txn.prepareStatement(sql);
-			ps.setInt(1, state.getValue());
-			ps.setBytes(2, g.getBytes());
 			rs = ps.executeQuery();
 			List<MessageId> ids = new ArrayList<>();
 			while (rs.next()) ids.add(new MessageId(rs.getBytes(1)));
@@ -1559,7 +1538,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 	public Collection<MessageId> getMessageIds(Connection txn, GroupId g,
 			Metadata query) throws DbException {
 		// If there are no query terms, return all delivered messages
-		if (query.isEmpty()) return getMessageIds(txn, g, DELIVERED);
+		if (query.isEmpty()) return getMessageIds(txn, g);
 		PreparedStatement ps = null;
 		ResultSet rs = null;
 		try {
@@ -1718,10 +1697,11 @@ abstract class JdbcDatabase implements Database<Connection> {
 		ResultSet rs = null;
 		try {
 			String sql = "SELECT messageId, txCount > 0, seen FROM statuses"
-					+ " WHERE groupId = ? AND contactId = ?";
+					+ " WHERE groupId = ? AND contactId = ? AND state = ?";
 			ps = txn.prepareStatement(sql);
 			ps.setBytes(1, g.getBytes());
 			ps.setInt(2, c.getInt());
+			ps.setInt(3, DELIVERED.getValue());
 			rs = ps.executeQuery();
 			List<MessageStatus> statuses = new ArrayList<>();
 			while (rs.next()) {
@@ -1741,24 +1721,29 @@ abstract class JdbcDatabase implements Database<Connection> {
 	}
 
 	@Override
+	@Nullable
 	public MessageStatus getMessageStatus(Connection txn, ContactId c,
 			MessageId m) throws DbException {
 		PreparedStatement ps = null;
 		ResultSet rs = null;
 		try {
 			String sql = "SELECT txCount > 0, seen FROM statuses"
-					+ " WHERE messageId = ? AND contactId = ?";
+					+ " WHERE messageId = ? AND contactId = ? AND state = ?";
 			ps = txn.prepareStatement(sql);
 			ps.setBytes(1, m.getBytes());
 			ps.setInt(2, c.getInt());
+			ps.setInt(3, DELIVERED.getValue());
 			rs = ps.executeQuery();
-			if (!rs.next()) throw new DbStateException();
+			MessageStatus status = null;
-			boolean sent = rs.getBoolean(1);
+			if (rs.next()) {
-			boolean seen = rs.getBoolean(2);
+				boolean sent = rs.getBoolean(1);
+				boolean seen = rs.getBoolean(2);
+				status = new MessageStatus(m, c, sent, seen);
+			}
 			if (rs.next()) throw new DbStateException();
 			rs.close();
 			ps.close();
-			return new MessageStatus(m, c, sent, seen);
+			return status;
 		} catch (SQLException e) {
 			tryToClose(rs);
 			tryToClose(ps);
@@ -2578,7 +2563,14 @@ abstract class JdbcDatabase implements Database<Connection> {
 			if (affected != 1) throw new DbStateException();
 			ps.close();
 			// Remove status rows for the messages in the group
-			for (MessageId m : getMessageIds(txn, g)) removeStatus(txn, c, m);
+			sql = "DELETE FROM statuses"
+					+ " WHERE contactId = ? AND groupId = ?";
+			ps = txn.prepareStatement(sql);
+			ps.setInt(1, c.getInt());
+			ps.setBytes(2, g.getBytes());
+			affected = ps.executeUpdate();
+			if (affected < 0) throw new DbStateException();
+			ps.close();
 		} catch (SQLException e) {
 			tryToClose(ps);
 			throw new DbException(e);
@@ -2662,24 +2654,6 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
-	private void removeStatus(Connection txn, ContactId c, MessageId m)
-			throws DbException {
-		PreparedStatement ps = null;
-		try {
-			String sql = "DELETE FROM statuses"
-					+ " WHERE messageId = ? AND contactId = ?";
-			ps = txn.prepareStatement(sql);
-			ps.setBytes(1, m.getBytes());
-			ps.setInt(2, c.getInt());
-			int affected = ps.executeUpdate();
-			if (affected != 1) throw new DbStateException();
-			ps.close();
-		} catch (SQLException e) {
-			tryToClose(ps);
-			throw new DbException(e);
-		}
-	}
-
 	@Override
 	public void removeTransport(Connection txn, TransportId t)
 			throws DbException {

bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java

@@ -30,6 +30,7 @@ import org.briarproject.bramble.api.sync.Group;
 import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.Message;
 import org.briarproject.bramble.api.sync.MessageId;
+import org.briarproject.bramble.api.sync.MessageStatus;
 import org.briarproject.bramble.api.sync.Offer;
 import org.briarproject.bramble.api.sync.Request;
 import org.briarproject.bramble.api.sync.event.GroupAddedEvent;
@@ -77,6 +78,7 @@ import static org.briarproject.bramble.test.TestUtils.getTransportId;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 public class DatabaseComponentImplTest extends BrambleMockTestCase {
@@ -1319,6 +1321,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase {
 		TransportKeys transportKeys = createTransportKeys();
 		KeySet ks = new KeySet(keySetId, contactId, transportKeys);
 		Collection<KeySet> keys = singletonList(ks);
+
 		context.checking(new Expectations() {{
 			// startTransaction()
 			oneOf(database).startTransaction();
@@ -1348,6 +1351,114 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase {
 		}
 	}
 
+	@Test
+	public void testGetMessageStatusByGroupId() throws Exception {
+		MessageStatus status =
+				new MessageStatus(messageId, contactId, true, true);
+
+		context.checking(new Expectations() {{
+			// startTransaction()
+			oneOf(database).startTransaction();
+			will(returnValue(txn));
+			// getMessageStatus()
+			oneOf(database).containsContact(txn, contactId);
+			will(returnValue(true));
+			oneOf(database).containsGroup(txn, groupId);
+			will(returnValue(true));
+			oneOf(database).getGroupVisibility(txn, contactId, groupId);
+			will(returnValue(VISIBLE));
+			oneOf(database).getMessageStatus(txn, contactId, groupId);
+			will(returnValue(singletonList(status)));
+			// getMessageStatus() again
+			oneOf(database).containsContact(txn, contactId);
+			will(returnValue(true));
+			oneOf(database).containsGroup(txn, groupId);
+			will(returnValue(true));
+			oneOf(database).getGroupVisibility(txn, contactId, groupId);
+			will(returnValue(INVISIBLE));
+			oneOf(database).getMessageIds(txn, groupId);
+			will(returnValue(singletonList(messageId)));
+			// endTransaction()
+			oneOf(database).commitTransaction(txn);
+		}});
+		DatabaseComponent db = createDatabaseComponent(database, eventBus,
+				shutdown);
+
+		Transaction transaction = db.startTransaction(true);
+		try {
+			// With visible group - return stored status
+			Collection<MessageStatus> statuses =
+					db.getMessageStatus(transaction, contactId, groupId);
+			assertEquals(1, statuses.size());
+			MessageStatus s = statuses.iterator().next();
+			assertEquals(messageId, s.getMessageId());
+			assertEquals(contactId, s.getContactId());
+			assertTrue(s.isSent());
+			assertTrue(s.isSeen());
+			// With invisible group - return default status
+			statuses = db.getMessageStatus(transaction, contactId, groupId);
+			assertEquals(1, statuses.size());
+			s = statuses.iterator().next();
+			assertEquals(messageId, s.getMessageId());
+			assertEquals(contactId, s.getContactId());
+			assertFalse(s.isSent());
+			assertFalse(s.isSeen());
+			db.commitTransaction(transaction);
+		} finally {
+			db.endTransaction(transaction);
+		}
+	}
+
+	@Test
+	public void testGetMessageStatusByMessageId() throws Exception {
+		MessageStatus status =
+				new MessageStatus(messageId, contactId, true, true);
+
+		context.checking(new Expectations() {{
+			// startTransaction()
+			oneOf(database).startTransaction();
+			will(returnValue(txn));
+			// getMessageStatus()
+			oneOf(database).containsContact(txn, contactId);
+			will(returnValue(true));
+			oneOf(database).containsMessage(txn, messageId);
+			will(returnValue(true));
+			oneOf(database).getMessageStatus(txn, contactId, messageId);
+			will(returnValue(status));
+			// getMessageStatus() again
+			oneOf(database).containsContact(txn, contactId);
+			will(returnValue(true));
+			oneOf(database).containsMessage(txn, messageId);
+			will(returnValue(true));
+			oneOf(database).getMessageStatus(txn, contactId, messageId);
+			will(returnValue(null));
+			// endTransaction()
+			oneOf(database).commitTransaction(txn);
+		}});
+		DatabaseComponent db = createDatabaseComponent(database, eventBus,
+				shutdown);
+
+		Transaction transaction = db.startTransaction(true);
+		try {
+			// With visible group - return stored status
+			MessageStatus s =
+					db.getMessageStatus(transaction, contactId, messageId);
+			assertEquals(messageId, s.getMessageId());
+			assertEquals(contactId, s.getContactId());
+			assertTrue(s.isSent());
+			assertTrue(s.isSeen());
+			// With invisible group - return default status
+			s = db.getMessageStatus(transaction, contactId, messageId);
+			assertEquals(messageId, s.getMessageId());
+			assertEquals(contactId, s.getContactId());
+			assertFalse(s.isSent());
+			assertFalse(s.isSeen());
+			db.commitTransaction(transaction);
+		} finally {
+			db.endTransaction(transaction);
+		}
+	}
+
 	private TransportKeys createTransportKeys() {
 		SecretKey inPrevTagKey = getSecretKey();
 		SecretKey inPrevHeaderKey = getSecretKey();

bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java

@@ -1596,6 +1596,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase {
 
 		// The message should not be sent or seen
 		MessageStatus status = db.getMessageStatus(txn, contactId, messageId);
+		assertNotNull(status);
 		assertEquals(messageId, status.getMessageId());
 		assertEquals(contactId, status.getContactId());
 		assertFalse(status.isSent());
@@ -1616,6 +1617,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase {
 
 		// The message should be sent but not seen
 		status = db.getMessageStatus(txn, contactId, messageId);
+		assertNotNull(status);
 		assertEquals(messageId, status.getMessageId());
 		assertEquals(contactId, status.getContactId());
 		assertTrue(status.isSent());
@@ -1635,6 +1637,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase {
 
 		// The message should be sent and seen
 		status = db.getMessageStatus(txn, contactId, messageId);
+		assertNotNull(status);
 		assertEquals(messageId, status.getMessageId());
 		assertEquals(contactId, status.getContactId());
 		assertTrue(status.isSent());
@@ -1649,6 +1652,36 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase {
 		assertTrue(status.isSent());
 		assertTrue(status.isSeen());
 
+		// Make the group invisible to the contact
+		db.removeGroupVisibility(txn, contactId, groupId);
+
+		// Null should be returned when querying by message
+		assertNull(db.getMessageStatus(txn, contactId, messageId));
+
+		// No statuses should be returned when querying by group
+		statuses = db.getMessageStatus(txn, contactId, groupId);
+		assertEquals(0, statuses.size());
+
+		// Make the group visible to the contact again
+		db.addGroupVisibility(txn, contactId, groupId, false);
+
+		// The default status should be returned when querying by message
+		status = db.getMessageStatus(txn, contactId, messageId);
+		assertNotNull(status);
+		assertEquals(messageId, status.getMessageId());
+		assertEquals(contactId, status.getContactId());
+		assertFalse(status.isSent());
+		assertFalse(status.isSeen());
+
+		// The default status should be returned when querying by group
+		statuses = db.getMessageStatus(txn, contactId, groupId);
+		assertEquals(1, statuses.size());
+		status = statuses.iterator().next();
+		assertEquals(messageId, status.getMessageId());
+		assertEquals(contactId, status.getContactId());
+		assertFalse(status.isSent());
+		assertFalse(status.isSeen());
+
 		db.commitTransaction(txn);
 		db.close();
 	}