Use a DB index to avoid linear insertion time.

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
+			// Find the new element's predecessor
-			byte[] id = g.getBytes();
+			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();
+			if(!rs.next()) {
-			// Scan through the list to find the insertion point
+				// The predecessor has a null nextId so it's at the tail
-			byte[] groupId = rs.getBytes(1);
+				rs.close();
-			if(groupId != null) throw new DbStateException();
+				ps.close();
-			byte[] nextId = rs.getBytes(2);
+				sql = "SELECT groupId, nextId, deleted FROM visibilities"
-			long deleted = rs.getLong(3);
+					+ " WHERE contactId = ? AND nextId IS NULL";
-			while(nextId != null && ByteUtils.compare(id, nextId) > 0) {
+				ps = txn.prepareStatement(sql);
+				ps.setInt(1, c.getInt());
+				rs = ps.executeQuery();
 				if(!rs.next()) throw new DbStateException();				
-				groupId = rs.getBytes(1);
-				if(groupId == null) throw new DbStateException();
-				nextId = rs.getBytes(2);
-				deleted = rs.getLong(3);
 			}
+			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);

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"
+			"CREATE TABLE foo"
-		+ " (uniqueId BINARY(32) NOT NULL,"
+					+ " (uniqueId BINARY(32),"
-		+ " name VARCHAR NOT NULL,"
+					+ " name VARCHAR NOT NULL)";
-		+ " PRIMARY KEY (uniqueId))";
 
 	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
+		// Generate an ID
-		byte[] uniqueId = new byte[32];
+		byte[] id = new byte[32];
-		new Random().nextBytes(uniqueId);
+		new Random().nextBytes(id);
-		// Insert the unique ID and name into the table
+		// Insert the ID and name into the table
-		addRow(uniqueId, "foo");
+		addRow(id, "foo");
 	}
 
 	@Test
 	public void testCreateTableAddAndRetrieveRow() throws Exception {
 		// Create the table
 		createTable(connection);
-		// Generate a unique ID
+		// Generate an ID
-		byte[] uniqueId = new byte[32];
+		byte[] id = new byte[32];
-		new Random().nextBytes(uniqueId);
+		new Random().nextBytes(id);
-		// Insert the unique ID and name into the table
+		// Insert the ID and name into the table
-		addRow(uniqueId, "foo");
+		addRow(id, "foo");
-		// Check that the name can be retrieved using the unique ID
+		// Check that the name can be retrieved using the ID
-		assertEquals("foo", getName(uniqueId));
+		assertEquals("foo", getName(id));
 	}
 
-	private void addRow(byte[] uniqueId, String name) throws SQLException {
+	@Test
-		String sql = "INSERT INTO foo (uniqueId, name) VALUES (?, ?)";
+	public void testSortOrder() throws Exception {
-		PreparedStatement ps = null;
+		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 {
-			ps = connection.prepareStatement(sql);
+			Statement s = connection.createStatement();
-			ps.setBytes(1, uniqueId);
+			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 (?, ?)";
+		try {
+			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);
+			PreparedStatement ps = connection.prepareStatement(sql);
-			ps.setBytes(1, uniqueId);
+			if(id != null) ps.setBytes(1, id);
-			rs = ps.executeQuery();
+			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 {
+	private String getPredecessor(byte[] id) throws SQLException {
-		Statement s;
+		String sql = "SELECT name FROM foo WHERE uniqueId < ?"
+				+ " ORDER BY uniqueId DESC LIMIT ?";
 		try {
-			s = connection.createStatement();
+			PreparedStatement ps = connection.prepareStatement(sql);
-			s.executeUpdate(CREATE_TABLE);
+			ps.setBytes(1, id);
-			s.close();
+			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;

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());
 		}