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 be4aba0cf..08fbe4540 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. *

* 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. *

* 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 28666f9a3..bc3166d56 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 { @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 { /** * 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. *

* Read-only. */ @@ -303,9 +302,9 @@ interface Database { 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. *

* 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 c97a63a61..013233f39 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 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 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 ed9bc88f2..5246021b7 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; @@ -71,7 +72,7 @@ import static org.briarproject.bramble.db.ExponentialBackoff.calculateExpiry; abstract class JdbcDatabase implements Database { // 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" @@ -169,6 +170,10 @@ abstract class JdbcDatabase implements Database { + " (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," @@ -714,6 +719,17 @@ abstract class JdbcDatabase implements Database { 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); @@ -783,21 +799,42 @@ abstract class JdbcDatabase implements Database { } @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); } @@ -1662,11 +1699,9 @@ abstract class JdbcDatabase implements Database { 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(); @@ -1674,14 +1709,8 @@ abstract class JdbcDatabase implements Database { 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 = UNKNOWN; // Dependency in another group - } + if (rs.wasNull()) + state = UNKNOWN; // Missing or in a different group dependencies.put(dependency, state); } rs.close(); @@ -1700,11 +1729,12 @@ abstract class JdbcDatabase implements Database { 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(); @@ -2730,6 +2760,25 @@ abstract class JdbcDatabase implements Database { 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 18de0d7e8..0ed457461 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 df308cd7f..389f7c1c3 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 04244a310..d81cfd851 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 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 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,9 +1340,9 @@ 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 dependencies; @@ -1345,8 +1361,8 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { Map 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 result = db.getMessagesToShare(txn);