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 be4aba0cfcdf12eb37b5fd074e7f8c7e2766f5c9..08fbe45405b0b5c3a1011e4780156a7d157c6219 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 @@ -18,7 +18,6 @@ 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.ValidationManager; import org.briarproject.bramble.api.transport.TransportKeys; import java.util.Collection; @@ -339,12 +338,8 @@ public interface DatabaseComponent { /** * Returns the IDs and states of all dependencies of the given message. - * Missing dependencies have the state - * {@link ValidationManager.State UNKNOWN}. - * Dependencies in other groups have the state - * {@link ValidationManager.State INVALID}. - * Note that these states are not set on the dependencies themselves; the - * returned states should only be taken in the context of the given message. + * For missing dependencies and dependencies in other groups, the state + * {@link State UNKNOWN} is returned. * <p/> * Read-only. */ @@ -352,9 +347,9 @@ public interface DatabaseComponent { throws DbException; /** - * Returns all IDs of messages that depend on the given message. - * Messages in other groups that declare a dependency on the given message - * will be returned even though such dependencies are invalid. + * Returns the IDs and states of all dependents of the given message. + * Dependents in other groups are not returned. If the given message is + * missing, no dependents are returned. * <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 28666f9a3231ef7c2551fa292e5e4f285a7d209c..bc3166d5622e605a1fdddcbf0858bc438b2216ce 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 @@ -105,10 +105,11 @@ interface Database<T> { @Nullable ContactId sender) throws DbException; /** - * Adds a dependency between two messages in the given group. + * Adds a dependency between two messages, where the dependent message is + * in the given state. */ - void addMessageDependency(T txn, GroupId g, MessageId dependent, - MessageId dependency) throws DbException; + void addMessageDependency(T txn, Message dependent, MessageId dependency, + State dependentState) throws DbException; /** * Records that a message has been offered by the given contact. @@ -292,10 +293,8 @@ interface Database<T> { /** * Returns the IDs and states of all dependencies of the given message. - * Missing dependencies have the state {@link State UNKNOWN}. - * Dependencies in other groups have the state {@link State INVALID}. - * Note that these states are not set on the dependencies themselves; the - * returned states should only be taken in the context of the given message. + * For missing dependencies and dependencies in other groups, the state + * {@link State UNKNOWN} is returned. * <p/> * Read-only. */ @@ -303,9 +302,9 @@ interface Database<T> { throws DbException; /** - * Returns all IDs and states of all dependents of the given message. - * Messages in other groups that declare a dependency on the given message - * will be returned even though such dependencies are invalid. + * Returns the IDs and states of all dependents of the given message. + * Dependents in other groups are not returned. If the given message is + * missing, no dependents are returned. * <p/> * Read-only. */ 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 c97a63a611c3f868627cb92fbc3a924d94371bf1..013233f395ceac5ff0f65cf5a0044473238858c4 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 @@ -765,6 +765,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { T txn = unbox(transaction); if (!db.containsMessage(txn, m)) throw new NoSuchMessageException(); + // TODO: Don't allow messages with dependents to be removed db.removeMessage(txn, m); } @@ -850,9 +851,9 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { T txn = unbox(transaction); if (!db.containsMessage(txn, dependent.getId())) throw new NoSuchMessageException(); + State dependentState = db.getMessageState(txn, dependent.getId()); for (MessageId dependency : dependencies) { - db.addMessageDependency(txn, dependent.getGroupId(), - dependent.getId(), dependency); + db.addMessageDependency(txn, dependent, dependency, dependentState); } } 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 02f2876b0edd4f73fb27e8fa3f7f4ff96f267ea5..51fa9ae309c39b93421e37673745ce143e8e59f5 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 @@ -50,6 +50,7 @@ import java.util.logging.Logger; import javax.annotation.Nullable; +import static java.sql.Types.INTEGER; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; import static org.briarproject.bramble.api.db.Metadata.REMOVE; @@ -57,7 +58,6 @@ import static org.briarproject.bramble.api.sync.Group.Visibility.INVISIBLE; import static org.briarproject.bramble.api.sync.Group.Visibility.SHARED; import static org.briarproject.bramble.api.sync.Group.Visibility.VISIBLE; import static org.briarproject.bramble.api.sync.ValidationManager.State.DELIVERED; -import static org.briarproject.bramble.api.sync.ValidationManager.State.INVALID; import static org.briarproject.bramble.api.sync.ValidationManager.State.PENDING; import static org.briarproject.bramble.api.sync.ValidationManager.State.UNKNOWN; import static org.briarproject.bramble.db.DatabaseConstants.DB_SETTINGS_NAMESPACE; @@ -72,7 +72,7 @@ import static org.briarproject.bramble.db.ExponentialBackoff.calculateExpiry; abstract class JdbcDatabase implements Database<Connection> { // Package access for testing - static final int CODE_SCHEMA_VERSION = 35; + static final int CODE_SCHEMA_VERSION = 36; private static final String CREATE_SETTINGS = "CREATE TABLE settings" @@ -170,6 +170,10 @@ abstract class JdbcDatabase implements Database<Connection> { + " (groupId _HASH NOT NULL," + " messageId _HASH NOT NULL," + " dependencyId _HASH NOT NULL," // Not a foreign key + + " messageState INT NOT NULL," // Denormalised + // Denormalised, null if dependency is missing or in a + // different group + + " dependencyState INT," + " FOREIGN KEY (groupId)" + " REFERENCES groups (groupId)" + " ON DELETE CASCADE," @@ -264,6 +268,10 @@ abstract class JdbcDatabase implements Database<Connection> { "CREATE INDEX IF NOT EXISTS messageMetadataByGroupIdState" + " ON messageMetadata (groupId, state)"; + private static final String INDEX_MESSAGE_DEPENDENCIES_BY_DEPENDENCY_ID = + "CREATE INDEX IF NOT EXISTS messageDependenciesByDependencyId" + + " ON messageDependencies (dependencyId)"; + private static final String INDEX_STATUSES_BY_CONTACT_ID_GROUP_ID = "CREATE INDEX IF NOT EXISTS statusesByContactIdGroupId" + " ON statuses (contactId, groupId)"; @@ -423,6 +431,7 @@ abstract class JdbcDatabase implements Database<Connection> { s.executeUpdate(INDEX_CONTACTS_BY_AUTHOR_ID); s.executeUpdate(INDEX_GROUPS_BY_CLIENT_ID); s.executeUpdate(INDEX_MESSAGE_METADATA_BY_GROUP_ID_STATE); + s.executeUpdate(INDEX_MESSAGE_DEPENDENCIES_BY_DEPENDENCY_ID); s.executeUpdate(INDEX_STATUSES_BY_CONTACT_ID_GROUP_ID); s.executeUpdate(INDEX_STATUSES_BY_CONTACT_ID_TIMESTAMP); s.close(); @@ -715,6 +724,17 @@ abstract class JdbcDatabase implements Database<Connection> { m.getLength(), state, e.getValue(), messageShared, false, seen); } + // Update denormalised column in messageDependencies if dependency + // is in same group as dependent + sql = "UPDATE messageDependencies SET dependencyState = ?" + + " WHERE groupId = ? AND dependencyId = ?"; + ps = txn.prepareStatement(sql); + ps.setInt(1, state.getValue()); + ps.setBytes(2, m.getGroupId().getBytes()); + ps.setBytes(3, m.getId().getBytes()); + affected = ps.executeUpdate(); + if (affected < 0) throw new DbStateException(); + ps.close(); } catch (SQLException e) { tryToClose(ps); throw new DbException(e); @@ -784,21 +804,42 @@ abstract class JdbcDatabase implements Database<Connection> { } @Override - public void addMessageDependency(Connection txn, GroupId g, - MessageId dependent, MessageId dependency) throws DbException { + public void addMessageDependency(Connection txn, Message dependent, + MessageId dependency, State dependentState) throws DbException { PreparedStatement ps = null; + ResultSet rs = null; try { - String sql = "INSERT INTO messageDependencies" - + " (groupId, messageId, dependencyId)" - + " VALUES (?, ?, ?)"; + // Get state of dependency if present and in same group as dependent + String sql = "SELECT state FROM messages" + + " WHERE messageId = ? AND groupId = ?"; ps = txn.prepareStatement(sql); - ps.setBytes(1, g.getBytes()); - ps.setBytes(2, dependent.getBytes()); + ps.setBytes(1, dependency.getBytes()); + ps.setBytes(2, dependent.getGroupId().getBytes()); + rs = ps.executeQuery(); + State dependencyState = null; + if (rs.next()) { + dependencyState = State.fromValue(rs.getInt(1)); + if (rs.next()) throw new DbStateException(); + } + rs.close(); + ps.close(); + // Create messageDependencies row + sql = "INSERT INTO messageDependencies" + + " (groupId, messageId, dependencyId, messageState," + + " dependencyState)" + + " VALUES (?, ?, ?, ? ,?)"; + ps = txn.prepareStatement(sql); + ps.setBytes(1, dependent.getGroupId().getBytes()); + ps.setBytes(2, dependent.getId().getBytes()); ps.setBytes(3, dependency.getBytes()); + ps.setInt(4, dependentState.getValue()); + if (dependencyState == null) ps.setNull(5, INTEGER); + else ps.setInt(5, dependencyState.getValue()); int affected = ps.executeUpdate(); if (affected != 1) throw new DbStateException(); ps.close(); } catch (SQLException e) { + tryToClose(rs); tryToClose(ps); throw new DbException(e); } @@ -1663,11 +1704,9 @@ abstract class JdbcDatabase implements Database<Connection> { PreparedStatement ps = null; ResultSet rs = null; try { - String sql = "SELECT d.dependencyId, m.state, d.groupId, m.groupId" - + " FROM messageDependencies AS d" - + " LEFT OUTER JOIN messages AS m" - + " ON d.dependencyId = m.messageId" - + " WHERE d.messageId = ?"; + String sql = "SELECT dependencyId, dependencyState" + + " FROM messageDependencies" + + " WHERE messageId = ?"; ps = txn.prepareStatement(sql); ps.setBytes(1, m.getBytes()); rs = ps.executeQuery(); @@ -1675,14 +1714,8 @@ abstract class JdbcDatabase implements Database<Connection> { while (rs.next()) { MessageId dependency = new MessageId(rs.getBytes(1)); State state = State.fromValue(rs.getInt(2)); - if (rs.wasNull()) { - state = UNKNOWN; // Missing dependency - } else { - GroupId dependentGroupId = new GroupId(rs.getBytes(3)); - GroupId dependencyGroupId = new GroupId(rs.getBytes(4)); - if (!dependentGroupId.equals(dependencyGroupId)) - state = INVALID; // Dependency in another group - } + if (rs.wasNull()) + state = UNKNOWN; // Missing or in a different group dependencies.put(dependency, state); } rs.close(); @@ -1701,11 +1734,12 @@ abstract class JdbcDatabase implements Database<Connection> { PreparedStatement ps = null; ResultSet rs = null; try { - String sql = "SELECT d.messageId, m.state" - + " FROM messageDependencies AS d" - + " JOIN messages AS m" - + " ON d.messageId = m.messageId" - + " WHERE dependencyId = ?"; + // Exclude dependencies that are missing or in a different group + // from the dependent + String sql = "SELECT messageId, messageState" + + " FROM messageDependencies" + + " WHERE dependencyId = ?" + + " AND dependencyState IS NOT NULL"; ps = txn.prepareStatement(sql); ps.setBytes(1, m.getBytes()); rs = ps.executeQuery(); @@ -2731,6 +2765,25 @@ abstract class JdbcDatabase implements Database<Connection> { affected = ps.executeUpdate(); if (affected < 0) throw new DbStateException(); ps.close(); + // Update denormalised column in messageDependencies + sql = "UPDATE messageDependencies SET messageState = ?" + + " WHERE messageId = ?"; + ps = txn.prepareStatement(sql); + ps.setInt(1, state.getValue()); + ps.setBytes(2, m.getBytes()); + affected = ps.executeUpdate(); + if (affected < 0) throw new DbStateException(); + ps.close(); + // Update denormalised column in messageDependencies if dependency + // is present and in same group as dependent + sql = "UPDATE messageDependencies SET dependencyState = ?" + + " WHERE dependencyId = ? AND dependencyState IS NOT NULL"; + ps = txn.prepareStatement(sql); + ps.setInt(1, state.getValue()); + ps.setBytes(2, m.getBytes()); + affected = ps.executeUpdate(); + if (affected < 0) throw new DbStateException(); + ps.close(); } catch (SQLException e) { tryToClose(ps); throw new DbException(e); 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 18de0d7e847f57156dd7ebbcab6474bd63e21404..0ed4574618fc17e8eac6f0dcb59cb395f206e78d 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 @@ -1518,10 +1518,12 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { // addMessageDependencies() oneOf(database).containsMessage(txn, messageId); will(returnValue(true)); - oneOf(database).addMessageDependency(txn, groupId, messageId, - messageId1); - oneOf(database).addMessageDependency(txn, groupId, messageId, - messageId2); + oneOf(database).getMessageState(txn, messageId); + will(returnValue(DELIVERED)); + oneOf(database).addMessageDependency(txn, message, messageId1, + DELIVERED); + oneOf(database).addMessageDependency(txn, message, messageId2, + DELIVERED); // getMessageDependencies() oneOf(database).containsMessage(txn, messageId); will(returnValue(true)); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java index df308cd7fdec81530272925ba624d51db5d9fec7..389f7c1c3a5e50b3152f046578b0a038022f3d60 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java @@ -572,8 +572,9 @@ public abstract class DatabasePerformanceTest extends BrambleTestCase { messageMeta.get(g.getId()).add(mm); db.mergeMessageMetadata(txn, m.getId(), mm); if (k > 0) { - db.addMessageDependency(txn, g.getId(), m.getId(), - pickRandom(groupMessages.get(g.getId()))); + MessageId dependency = + pickRandom(groupMessages.get(g.getId())); + db.addMessageDependency(txn, m, dependency, state); } groupMessages.get(g.getId()).add(m.getId()); } @@ -598,8 +599,9 @@ public abstract class DatabasePerformanceTest extends BrambleTestCase { messageMeta.get(g.getId()).add(mm); db.mergeMessageMetadata(txn, m.getId(), mm); if (j > 0) { - db.addMessageDependency(txn, g.getId(), m.getId(), - pickRandom(groupMessages.get(g.getId()))); + MessageId dependency = + pickRandom(groupMessages.get(g.getId())); + db.addMessageDependency(txn, m, dependency, DELIVERED); } groupMessages.get(g.getId()).add(m.getId()); } 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 c21d5b26a4bd050b387a8a0f07cee7b9dfadc0d9..d81cfd85149cdc1ec6e2a53d0b1a4de2b4afeb7a 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 @@ -1227,6 +1227,8 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { MessageId messageId4 = new MessageId(getRandomId()); Message message1 = new Message(messageId1, groupId, timestamp, raw); Message message2 = new Message(messageId2, groupId, timestamp, raw); + Message message3 = new Message(messageId3, groupId, timestamp, raw); + Message message4 = new Message(messageId4, groupId, timestamp, raw); Database<Connection> db = open(false); Connection txn = db.startTransaction(); @@ -1234,21 +1236,21 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a group and some messages db.addGroup(txn, group); db.addMessage(txn, message, PENDING, true, contactId); - db.addMessage(txn, message1, DELIVERED, true, contactId); + db.addMessage(txn, message1, PENDING, true, contactId); db.addMessage(txn, message2, INVALID, true, contactId); // Add dependencies - db.addMessageDependency(txn, groupId, messageId, messageId1); - db.addMessageDependency(txn, groupId, messageId, messageId2); - db.addMessageDependency(txn, groupId, messageId1, messageId3); - db.addMessageDependency(txn, groupId, messageId2, messageId4); + db.addMessageDependency(txn, message, messageId1, PENDING); + db.addMessageDependency(txn, message, messageId2, PENDING); + db.addMessageDependency(txn, message1, messageId3, PENDING); + db.addMessageDependency(txn, message2, messageId4, INVALID); Map<MessageId, State> dependencies; // Retrieve dependencies for root dependencies = db.getMessageDependencies(txn, messageId); assertEquals(2, dependencies.size()); - assertEquals(DELIVERED, dependencies.get(messageId1)); + assertEquals(PENDING, dependencies.get(messageId1)); assertEquals(INVALID, dependencies.get(messageId2)); // Retrieve dependencies for message 1 @@ -1281,10 +1283,24 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertEquals(1, dependents.size()); assertEquals(PENDING, dependents.get(messageId)); + // Message 3 is missing, so it has no dependents + dependents = db.getMessageDependents(txn, messageId3); + assertEquals(0, dependents.size()); + + // Add message 3 + db.addMessage(txn, message3, UNKNOWN, false, contactId); + // Message 3 has message 1 as a dependent dependents = db.getMessageDependents(txn, messageId3); assertEquals(1, dependents.size()); - assertEquals(DELIVERED, dependents.get(messageId1)); + assertEquals(PENDING, dependents.get(messageId1)); + + // Message 4 is missing, so it has no dependents + dependents = db.getMessageDependents(txn, messageId4); + assertEquals(0, dependents.size()); + + // Add message 4 + db.addMessage(txn, message4, UNKNOWN, false, contactId); // Message 4 has message 2 as a dependent dependents = db.getMessageDependents(txn, messageId4); @@ -1324,16 +1340,16 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addMessage(txn, message3, DELIVERED, true, contactId); // Add dependencies between the messages - db.addMessageDependency(txn, groupId, messageId, messageId1); - db.addMessageDependency(txn, groupId, messageId, messageId2); - db.addMessageDependency(txn, groupId, messageId, messageId3); + db.addMessageDependency(txn, message, messageId1, PENDING); + db.addMessageDependency(txn, message, messageId2, PENDING); + db.addMessageDependency(txn, message, messageId3, PENDING); // Retrieve the dependencies for the root Map<MessageId, State> dependencies; dependencies = db.getMessageDependencies(txn, messageId); - // The cross-group dependency should have state INVALID - assertEquals(INVALID, dependencies.get(messageId1)); + // The cross-group dependency should have state UNKNOWN + assertEquals(UNKNOWN, dependencies.get(messageId1)); // The missing dependency should have state UNKNOWN assertEquals(UNKNOWN, dependencies.get(messageId2)); @@ -1345,8 +1361,8 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { Map<MessageId, State> dependents; dependents = db.getMessageDependents(txn, messageId1); - // The cross-group dependent should have its real state - assertEquals(PENDING, dependents.get(messageId)); + // The cross-group dependent should be excluded + assertFalse(dependents.containsKey(messageId)); db.commitTransaction(txn); db.close(); @@ -1411,9 +1427,9 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addMessage(txn, m4, DELIVERED, true, contactId); // Introduce dependencies between the messages - db.addMessageDependency(txn, groupId, mId1, mId2); - db.addMessageDependency(txn, groupId, mId3, mId1); - db.addMessageDependency(txn, groupId, mId4, mId3); + db.addMessageDependency(txn, m1, mId2, DELIVERED); + db.addMessageDependency(txn, m3, mId1, DELIVERED); + db.addMessageDependency(txn, m4, mId3, DELIVERED); // Retrieve messages to be shared Collection<MessageId> result = db.getMessagesToShare(txn);