From 1b9f97519951b83d72d99337a7514e26b1fc37bf Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Mon, 30 Apr 2018 13:55:40 +0100
Subject: [PATCH] Return default message status if group is invisible.
---
.../bramble/api/db/DatabaseComponent.java | 24 ++--
.../org/briarproject/bramble/db/Database.java | 27 +++--
.../bramble/db/DatabaseComponentImpl.java | 11 +-
.../briarproject/bramble/db/JdbcDatabase.java | 74 ++++--------
.../bramble/db/DatabaseComponentImplTest.java | 111 ++++++++++++++++++
.../bramble/db/JdbcDatabaseTest.java | 33 ++++++
6 files changed, 205 insertions(+), 75 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 3715674413..680ddcad96 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
@@ -136,8 +136,8 @@ public interface DatabaseComponent {
/**
* Deletes the message with the given ID. Unlike
- * {@link #removeMessage(Transaction, MessageId)}, the message ID and any
- * other associated data are not deleted.
+ * {@link #removeMessage(Transaction, MessageId)}, the message ID,
+ * 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
- * that matches all entries in the given query. If the query is empty, the
- * metadata for all messages is returned.
+ * Returns the metadata for any delivered messages in the given group with
+ * metadata that matches all entries in the given query. If the query is
+ * 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.
- * This is meant to be only used by the ValidationManager
+ * Returns the metadata for the given delivered or pending message.
+ * 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
- * the given contact.
+ * Returns the status of all delivered messages in the given group with
+ * 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
- * contact.
+ * Returns the status of the given delivered message with respect to the
+ * given contact.
* <p/>
* Read-only.
*/
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 f20c0da5ed..bfcb3a2f1a 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
@@ -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
- * matching all entries in the given query. If the query is empty, the IDs
- * of all messages are returned.
+ * Returns the IDs of any delivered messages in the given group with
+ * metadata that matches all entries in the given query. If the query is
+ * 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
- * matching all entries in the given query. If the query is empty, the
- * metadata for all messages is returned.
+ * Returns the metadata for any delivered messages in the given group with
+ * metadata that matches all entries in the given query. If the query is
+ * 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
- * to the given contact.
+ * Returns the status of all delivered messages in the given group with
+ * 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;
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 1534a9d81b..ad83111180 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
@@ -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
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 7115808e28..21103c2d43 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
@@ -1515,33 +1515,12 @@ abstract class JdbcDatabase implements Database<Connection> {
throws DbException {
PreparedStatement ps = null;
ResultSet rs = null;
- try {
- String sql = "SELECT messageId FROM messages WHERE groupId = ?";
- ps = txn.prepareStatement(sql);
- ps.setBytes(1, g.getBytes());
- rs = ps.executeQuery();
- 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 = ?";
+ + " WHERE groupId = ? AND state = ?";
ps = txn.prepareStatement(sql);
- ps.setInt(1, state.getValue());
- ps.setBytes(2, g.getBytes());
+ ps.setBytes(1, g.getBytes());
+ ps.setInt(2, DELIVERED.getValue());
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();
- boolean sent = rs.getBoolean(1);
- boolean seen = rs.getBoolean(2);
+ MessageStatus status = null;
+ if (rs.next()) {
+ 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 {
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java
index c228bf67eb..1fec34ce9d 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java
+++ b/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();
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 d5b29dc657..c8dc8dccc6 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
@@ -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();
}
--
GitLab