diff --git a/components/net/sf/briar/db/JdbcDatabase.java b/components/net/sf/briar/db/JdbcDatabase.java index 0a4a074762c622201a1d84d45729b17f4023e2ee..faecea8611fb799f44d4bba9df2d18d51b8d66c4 100644 --- a/components/net/sf/briar/db/JdbcDatabase.java +++ b/components/net/sf/briar/db/JdbcDatabase.java @@ -42,7 +42,6 @@ import net.sf.briar.api.transport.ConnectionContext; import net.sf.briar.api.transport.ConnectionContextFactory; import net.sf.briar.api.transport.ConnectionWindow; import net.sf.briar.api.transport.ConnectionWindowFactory; -import net.sf.briar.util.ByteUtils; import net.sf.briar.util.FileUtils; /** @@ -110,6 +109,9 @@ abstract class JdbcDatabase implements Database<Connection> { private static final String INDEX_VISIBILITIES_BY_GROUP = "CREATE INDEX visibilitiesByGroup ON visibilities (groupId)"; + private static final String INDEX_VISIBILITIES_BY_NEXT = + "CREATE INDEX visibilitiesByNext on visibilities (nextId)"; + private static final String CREATE_BATCHES_TO_ACK = "CREATE TABLE batchesToAck" + " (batchId HASH NOT NULL," @@ -336,6 +338,7 @@ abstract class JdbcDatabase implements Database<Connection> { s.executeUpdate(INDEX_MESSAGES_BY_SENDABILITY); s.executeUpdate(insertTypeNames(CREATE_VISIBILITIES)); s.executeUpdate(INDEX_VISIBILITIES_BY_GROUP); + s.executeUpdate(INDEX_VISIBILITIES_BY_NEXT); s.executeUpdate(insertTypeNames(CREATE_BATCHES_TO_ACK)); s.executeUpdate(insertTypeNames(CREATE_CONTACT_SUBSCRIPTIONS)); s.executeUpdate(insertTypeNames(CREATE_OUTSTANDING_BATCHES)); @@ -794,42 +797,47 @@ abstract class JdbcDatabase implements Database<Connection> { PreparedStatement ps = null; ResultSet rs = null; try { - // Insert the group ID into the linked list - byte[] id = g.getBytes(); + // Find the new element's predecessor + byte[] groupId = null, nextId = null; + long deleted = 0L; String sql = "SELECT groupId, nextId, deleted FROM visibilities" - + " WHERE contactId = ? ORDER BY groupId"; + + " WHERE contactId = ? AND nextId > ? ORDER BY nextId LIMIT ?"; ps = txn.prepareStatement(sql); ps.setInt(1, c.getInt()); + ps.setBytes(2, g.getBytes()); + ps.setInt(3, 1); rs = ps.executeQuery(); - if(!rs.next()) throw new DbStateException(); - // Scan through the list to find the insertion point - byte[] groupId = rs.getBytes(1); - if(groupId != null) throw new DbStateException(); - byte[] nextId = rs.getBytes(2); - long deleted = rs.getLong(3); - while(nextId != null && ByteUtils.compare(id, nextId) > 0) { - if(!rs.next()) throw new DbStateException(); - groupId = rs.getBytes(1); - if(groupId == null) throw new DbStateException(); - nextId = rs.getBytes(2); - deleted = rs.getLong(3); + if(!rs.next()) { + // The predecessor has a null nextId so it's at the tail + rs.close(); + ps.close(); + sql = "SELECT groupId, nextId, deleted FROM visibilities" + + " WHERE contactId = ? AND nextId IS NULL"; + ps = txn.prepareStatement(sql); + ps.setInt(1, c.getInt()); + rs = ps.executeQuery(); + if(!rs.next()) throw new DbStateException(); } + groupId = rs.getBytes(1); + nextId = rs.getBytes(2); + deleted = rs.getLong(3); + if(rs.next()) throw new DbStateException(); rs.close(); ps.close(); - // Update the previous element + // Update the predecessor's nextId if(groupId == null) { // Inserting at the head of the list sql = "UPDATE visibilities SET nextId = ?" + " WHERE contactId = ? AND groupId IS NULL"; ps = txn.prepareStatement(sql); - ps.setBytes(1, id); + ps.setBytes(1, g.getBytes()); ps.setInt(2, c.getInt()); } else { // Inserting in the middle or at the tail of the list sql = "UPDATE visibilities SET nextId = ?" + " WHERE contactId = ? AND groupId = ?"; ps = txn.prepareStatement(sql); - ps.setBytes(1, id); + ps.setBytes(1, g.getBytes()); ps.setInt(2, c.getInt()); ps.setBytes(3, groupId); } @@ -841,7 +849,7 @@ abstract class JdbcDatabase implements Database<Connection> { + " (contactId, groupId, nextId, deleted) VALUES (?, ?, ?, ?)"; ps = txn.prepareStatement(sql); ps.setInt(1, c.getInt()); - ps.setBytes(2, id); + ps.setBytes(2, g.getBytes()); if(nextId == null) ps.setNull(3, Types.BINARY); // At the tail else ps.setBytes(3, nextId); // In the middle ps.setLong(4, deleted); diff --git a/test/net/sf/briar/db/BasicH2Test.java b/test/net/sf/briar/db/BasicH2Test.java index c21488ed4fa10ce9b87ffa13cd68669dc0da7afe..76e2384f663dd8e2d51f3f6512fb5e1c1f1956b9 100644 --- a/test/net/sf/briar/db/BasicH2Test.java +++ b/test/net/sf/briar/db/BasicH2Test.java @@ -7,6 +7,9 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import java.sql.Types; +import java.util.ArrayList; +import java.util.List; import java.util.Random; import net.sf.briar.BriarTestCase; @@ -19,10 +22,9 @@ import org.junit.Test; public class BasicH2Test extends BriarTestCase { private static final String CREATE_TABLE = - "CREATE TABLE foo" - + " (uniqueId BINARY(32) NOT NULL," - + " name VARCHAR NOT NULL," - + " PRIMARY KEY (uniqueId))"; + "CREATE TABLE foo" + + " (uniqueId BINARY(32)," + + " name VARCHAR NOT NULL)"; private final File testDir = TestUtils.getTestDirectory(); private final File db = new File(testDir, "db"); @@ -41,32 +43,84 @@ public class BasicH2Test extends BriarTestCase { public void testCreateTableAndAddRow() throws Exception { // Create the table createTable(connection); - // Generate a unique ID - byte[] uniqueId = new byte[32]; - new Random().nextBytes(uniqueId); - // Insert the unique ID and name into the table - addRow(uniqueId, "foo"); + // Generate an ID + byte[] id = new byte[32]; + new Random().nextBytes(id); + // Insert the ID and name into the table + addRow(id, "foo"); } @Test public void testCreateTableAddAndRetrieveRow() throws Exception { // Create the table createTable(connection); - // Generate a unique ID - byte[] uniqueId = new byte[32]; - new Random().nextBytes(uniqueId); - // Insert the unique ID and name into the table - addRow(uniqueId, "foo"); - // Check that the name can be retrieved using the unique ID - assertEquals("foo", getName(uniqueId)); + // Generate an ID + byte[] id = new byte[32]; + new Random().nextBytes(id); + // Insert the ID and name into the table + addRow(id, "foo"); + // Check that the name can be retrieved using the ID + assertEquals("foo", getName(id)); } - private void addRow(byte[] uniqueId, String name) throws SQLException { + @Test + public void testSortOrder() throws Exception { + byte[] first = new byte[] { + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, -128 + }; + byte[] second = new byte[] { + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0 + }; + byte[] third = new byte[] { + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 127 + }; + // Create the table + createTable(connection); + // Insert the rows + addRow(first, "first"); + addRow(second, "second"); + addRow(third, "third"); + addRow(null, "null"); + // Check the ordering of the < operator: the null ID is not comparable + assertNull(getPredecessor(first)); + assertEquals("first", getPredecessor(second)); + assertEquals("second", getPredecessor(third)); + assertNull(getPredecessor(null)); + // Check the ordering of ORDER BY: nulls come first + List<String> names = getNames(); + assertEquals(4, names.size()); + assertEquals("null", names.get(0)); + assertEquals("first", names.get(1)); + assertEquals("second", names.get(2)); + assertEquals("third", names.get(3)); + } + + private void createTable(Connection connection) throws SQLException { + try { + Statement s = connection.createStatement(); + s.executeUpdate(CREATE_TABLE); + s.close(); + } catch(SQLException e) { + connection.close(); + throw e; + } + } + + private void addRow(byte[] id, String name) throws SQLException { String sql = "INSERT INTO foo (uniqueId, name) VALUES (?, ?)"; - PreparedStatement ps = null; try { - ps = connection.prepareStatement(sql); - ps.setBytes(1, uniqueId); + PreparedStatement ps = connection.prepareStatement(sql); + if(id == null) ps.setNull(1, Types.BINARY); + else ps.setBytes(1, id); ps.setString(2, name); int rowsAffected = ps.executeUpdate(); ps.close(); @@ -77,14 +131,12 @@ public class BasicH2Test extends BriarTestCase { } } - private String getName(byte[] uniqueId) throws SQLException { + private String getName(byte[] id) throws SQLException { String sql = "SELECT name FROM foo WHERE uniqueID = ?"; - PreparedStatement ps = null; - ResultSet rs = null; try { - ps = connection.prepareStatement(sql); - ps.setBytes(1, uniqueId); - rs = ps.executeQuery(); + PreparedStatement ps = connection.prepareStatement(sql); + if(id != null) ps.setBytes(1, id); + ResultSet rs = ps.executeQuery(); assertTrue(rs.next()); String name = rs.getString(1); assertFalse(rs.next()); @@ -97,12 +149,35 @@ public class BasicH2Test extends BriarTestCase { } } - private void createTable(Connection connection) throws SQLException { - Statement s; + private String getPredecessor(byte[] id) throws SQLException { + String sql = "SELECT name FROM foo WHERE uniqueId < ?" + + " ORDER BY uniqueId DESC LIMIT ?"; try { - s = connection.createStatement(); - s.executeUpdate(CREATE_TABLE); - s.close(); + PreparedStatement ps = connection.prepareStatement(sql); + ps.setBytes(1, id); + ps.setInt(2, 1); + ResultSet rs = ps.executeQuery(); + String name = rs.next() ? rs.getString(1) : null; + assertFalse(rs.next()); + rs.close(); + ps.close(); + return name; + } catch(SQLException e) { + connection.close(); + throw e; + } + } + + private List<String> getNames() throws SQLException { + String sql = "SELECT name FROM foo ORDER BY uniqueId"; + List<String> names = new ArrayList<String>(); + try { + PreparedStatement ps = connection.prepareStatement(sql); + ResultSet rs = ps.executeQuery(); + while(rs.next()) names.add(rs.getString(1)); + rs.close(); + ps.close(); + return names; } catch(SQLException e) { connection.close(); throw e; diff --git a/test/net/sf/briar/db/H2DatabaseTest.java b/test/net/sf/briar/db/H2DatabaseTest.java index 0e6aa4ddb9a9bf8615a3d923cd24acb41fedeb4e..4a00ad10a9c5244f9a537e734fcdb27ff78a6210 100644 --- a/test/net/sf/briar/db/H2DatabaseTest.java +++ b/test/net/sf/briar/db/H2DatabaseTest.java @@ -1872,10 +1872,11 @@ public class H2DatabaseTest extends BriarTestCase { Collections.shuffle(groups); for(Group g : groups) db.addVisibility(txn, contactId, g.getId()); - // Make the groups invisible to the contact and remove them + // Make some of the groups invisible to the contact and remove them all Collections.shuffle(groups); for(Group g : groups) { - db.removeVisibility(txn, contactId, g.getId()); + if(Math.random() < 0.5) + db.removeVisibility(txn, contactId, g.getId()); db.removeSubscription(txn, g.getId()); }