Simplify Database methods, move logic to DatabaseComponent.

api/net/sf/briar/api/db/DatabaseComponent.java

@@ -121,6 +121,9 @@ public interface DatabaseComponent {
 	/** Returns the IDs of all contacts. */
 	Collection<ContactId> getContacts() throws DbException;
 
+	/** Returns the local transport properties for the given transport. */
+	TransportProperties getLocalProperties(TransportId t) throws DbException;
+
 	/** Returns all local transport properties. */
 	Map<TransportId, TransportProperties> getLocalTransports()
 	throws DbException;

components/net/sf/briar/db/Database.java

@@ -40,12 +40,6 @@ import net.sf.briar.api.transport.ConnectionWindow;
  */
 interface Database<T> {
 
-	/**
-	 * A batch sent to a contact is considered lost when this many more
-	 * recently sent batches have been acknowledged.
-	 */
-	static final int RETRANSMIT_THRESHOLD = 5;
-
 	/**
 	 * Opens the database.
 	 * @param resume True to reopen an existing database, false to create a
@@ -220,6 +214,14 @@ interface Database<T> {
 	 */
 	MessageId getGroupMessageParent(T txn, MessageId m) throws DbException;
 
+	/**
+	 * Returns the local transport properties for the given transport.
+	 * <p>
+	 * Locking: transports read.
+	 */
+	TransportProperties getLocalProperties(T txn, TransportId t)
+	throws DbException;
+
 	/**
 	 * Returns all local transport properties.
 	 * <p>
@@ -406,15 +408,13 @@ interface Database<T> {
 	void removeMessage(T txn, MessageId m) throws DbException;
 
 	/**
-	 * Unsubscribes from the given group and returns the IDs of any contacts
-	 * affected by the change. Any messages belonging to the group are deleted
-	 * from the database.
+	 * Unsubscribes from the given group. Any messages belonging to the group
+	 * are deleted from the database.
 	 * <p>
 	 * Locking: contacts read, messages write, messageStatuses write,
 	 * subscriptions write.
 	 */
-	Collection<ContactId> removeSubscription(T txn, GroupId g)
-	throws DbException;
+	void removeSubscription(T txn, GroupId g) throws DbException;
 
 	/**
 	 * Sets the configuration for the given transport, replacing any existing
@@ -436,12 +436,11 @@ interface Database<T> {
 
 	/**
 	 * Sets the local transport properties for the given transport, replacing
-	 * any existing properties for that transport. Returns true if the
-	 * properties have changed.
+	 * any existing properties for that transport.
 	 * <p>
 	 * Locking: transports write.
 	 */
-	boolean setLocalProperties(T txn, TransportId t, TransportProperties p)
+	void setLocalProperties(T txn, TransportId t, TransportProperties p)
 	throws DbException;
 
 	/**
@@ -487,13 +486,22 @@ interface Database<T> {
 	void setSubscriptions(T txn, ContactId c, Map<Group, Long> subs,
 			long timestamp) throws DbException;
 
+	/**
+	 * Records the time at which the subscriptions visible to the given contacts
+	 * were last modified.
+	 * <p>
+	 * Locking: contacts read, subscriptions write.
+	 */
+	void setSubscriptionsModifiedTimestamp(T txn,
+			Collection<ContactId> contacts, long timestamp) throws DbException;
+
 	/**
 	 * Records the time at which a subscription update was last sent to the
 	 * given contact.
 	 * <p>
 	 * Locking: contacts read, subscriptions write.
 	 */
-	void setSubscriptionTimestamp(T txn, ContactId c, long timestamp)
+	void setSubscriptionsSentTimestamp(T txn, ContactId c, long timestamp)
 	throws DbException;
 
 	/**
@@ -507,22 +515,29 @@ interface Database<T> {
 			Map<TransportId, TransportProperties> transports, long timestamp)
 	throws DbException;
 
+	/**
+	 * Records the time at which the local transports were last modified.
+	 * <p>
+	 * Locking: contacts read, transports write.
+	 */
+	void setTransportsModifiedTimestamp(T txn, long timestamp)
+	throws DbException;
+
 	/**
 	 * Records the time at which a transport update was last sent to the given
 	 * contact.
 	 * <p>
 	 * Locking: contacts read, transports write.
 	 */
-	void setTransportTimestamp(T txn, ContactId c, long timestamp)
+	void setTransportsSentTimestamp(T txn, ContactId c, long timestamp)
 	throws DbException;
 
 	/**
 	 * Makes the given group visible to the given set of contacts and invisible
-	 * to any other contacts. Returns the IDs of any contacts affected by the
-	 * change.
+	 * to any other contacts.
 	 * <p>
 	 * Locking: contacts read, subscriptions write.
 	 */
-	Collection<ContactId> setVisibility(T txn, GroupId g,
-			Collection<ContactId> visible) throws DbException;
+	void setVisibility(T txn, GroupId g, Collection<ContactId> visible)
+	throws DbException;
 }

components/net/sf/briar/db/DatabaseComponentImpl.java

@@ -11,6 +11,7 @@ import java.util.ArrayList;
 import java.util.BitSet;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -595,7 +596,7 @@ DatabaseCleaner.Callback {
 				try {
 					subs = db.getVisibleSubscriptions(txn, c);
 					timestamp = System.currentTimeMillis();
-					db.setSubscriptionTimestamp(txn, c, timestamp);
+					db.setSubscriptionsSentTimestamp(txn, c, timestamp);
 					db.commitTransaction(txn);
 				} catch(DbException e) {
 					db.abortTransaction(txn);
@@ -625,7 +626,7 @@ DatabaseCleaner.Callback {
 				try {
 					transports = db.getLocalTransports(txn);
 					timestamp = System.currentTimeMillis();
-					db.setTransportTimestamp(txn, c, timestamp);
+					db.setTransportsSentTimestamp(txn, c, timestamp);
 					db.commitTransaction(txn);
 				} catch(DbException e) {
 					db.abortTransaction(txn);
@@ -724,6 +725,24 @@ DatabaseCleaner.Callback {
 		}
 	}
 
+	public TransportProperties getLocalProperties(TransportId t)
+	throws DbException {
+		transportLock.readLock().lock();
+		try {
+			T txn = db.startTransaction();
+			try {
+				TransportProperties p = db.getLocalProperties(txn, t);
+				db.commitTransaction(txn);
+				return p;
+			} catch(DbException e) {
+				db.abortTransaction(txn);
+				throw e;
+			}
+		} finally {
+			transportLock.readLock().unlock();
+		}
+	}
+
 	public Map<TransportId, TransportProperties> getLocalTransports()
 	throws DbException {
 		transportLock.readLock().lock();
@@ -1111,13 +1130,13 @@ DatabaseCleaner.Callback {
 		callListeners(new ContactRemovedEvent(c));
 	}
 
-	public void setConfig(TransportId t, TransportConfig config)
+	public void setConfig(TransportId t, TransportConfig c)
 	throws DbException {
 		transportLock.writeLock().lock();
 		try {
 			T txn = db.startTransaction();
 			try {
-				db.setConfig(txn, t, config);
+				db.setConfig(txn, t, c);
 				db.commitTransaction(txn);
 			} catch(DbException e) {
 				db.abortTransaction(txn);
@@ -1150,14 +1169,19 @@ DatabaseCleaner.Callback {
 		}
 	}
 
-	public void setLocalProperties(TransportId t,
-			TransportProperties properties) throws DbException {
-		boolean changed;
+	public void setLocalProperties(TransportId t, TransportProperties p)
+	throws DbException {
+		boolean changed = false;
 		transportLock.writeLock().lock();
 		try {
 			T txn = db.startTransaction();
 			try {
-				changed = db.setLocalProperties(txn, t, properties);
+				if(!p.equals(db.getLocalProperties(txn, t))) {
+					db.setLocalProperties(txn, t, p);
+					db.setTransportsModifiedTimestamp(txn,
+							System.currentTimeMillis());
+					changed = true;
+				}
 				db.commitTransaction(txn);
 			} catch(DbException e) {
 				db.abortTransaction(txn);
@@ -1266,16 +1290,20 @@ DatabaseCleaner.Callback {
 
 	public void setVisibility(GroupId g, Collection<ContactId> visible)
 	throws DbException {
-		Collection<ContactId> affected;
+		// Use HashSets for O(1) lookups, giving O(n) overall running time
+		HashSet<ContactId> then, now;
 		contactLock.readLock().lock();
 		try {
 			subscriptionLock.writeLock().lock();
 			try {
 				T txn = db.startTransaction();
 				try {
+					// Retrieve the group's current visibility
+					then = new HashSet<ContactId>(db.getVisibility(txn, g));
 					// Don't try to make the group visible to ex-contacts
-					visible.retainAll((db.getContacts(txn)));
-					affected = db.setVisibility(txn, g, visible);
+					now = new HashSet<ContactId>(visible);
+					now.retainAll(new HashSet<ContactId>(db.getContacts(txn)));
+					db.setVisibility(txn, g, now);
 					db.commitTransaction(txn);
 				} catch(DbException e) {
 					db.abortTransaction(txn);
@@ -1287,6 +1315,10 @@ DatabaseCleaner.Callback {
 		} finally {
 			contactLock.readLock().unlock();
 		}
+		// Work out which contacts were affected by the change
+		Collection<ContactId> affected = new ArrayList<ContactId>();
+		for(ContactId c : then) if(!now.contains(c)) affected.add(c);
+		for(ContactId c : now) if(!then.contains(c)) affected.add(c);
 		// Call the listeners outside the lock
 		if(!affected.isEmpty())
 			callListeners(new SubscriptionsUpdatedEvent(affected));
@@ -1325,7 +1357,8 @@ DatabaseCleaner.Callback {
 						T txn = db.startTransaction();
 						try {
 							if(db.containsSubscription(txn, g)) {
-								affected = db.removeSubscription(txn, g);
+								affected = db.getVisibility(txn, g);
+								db.removeSubscription(txn, g);
 								removed = true;
 							}
 							db.commitTransaction(txn);

components/net/sf/briar/db/DatabaseConstants.java

@@ -3,14 +3,48 @@ package net.sf.briar.db;
 interface DatabaseConstants {
 
 	// FIXME: These should be configurable
+
+	/**
+	 * The minimum amount of space in bytes that should be kept free on the
+	 * device where the database is stored. Whenever less than this much space
+	 * is free, old messages will be expired from the database.
+	 */
 	static final long MIN_FREE_SPACE = 300 * 1024 * 1024; // 300 MiB
+
+	/**
+	 * The minimum amount of space in bytes that must be kept free on the device
+	 * where the database is stored. If less than this much space is free and
+	 * there are no more messages to expire, the program will shut down.
+	 */
 	static final long CRITICAL_FREE_SPACE = 100 * 1024 * 1024; // 100 MiB
 
+	/**
+	 * The amount of free space will be checked whenever this many bytes of
+	 * messages have been added to the database since the last check.
+	 */
 	static final int MAX_BYTES_BETWEEN_SPACE_CHECKS = 5 * 1024 * 1024; // 5 MiB
+
+	/**
+	 * The amount of free space will be checked whenever this many milliseconds
+	 * have passed since the last check.
+	 */
 	static final long MAX_MS_BETWEEN_SPACE_CHECKS = 60L * 1000L; // 1 min
 
-	static final long MS_PER_SWEEP = 10L * 1000L; // 10 sec
+	/**
+	 * Up to this many bytes of messages will be expired from the database each
+	 * time it is necessary to expire messages.
+	 */
 	static final int BYTES_PER_SWEEP = 5 * 1024 * 1024; // 5 MiB
 
+	/**
+	 * The timestamp of the oldest message in the database is rounded using
+	 * this modulus to avoid revealing the presence of any particular message.
+	 */
 	static final long EXPIRY_MODULUS = 60L * 60L * 1000L; // 1 hour
+
+	/**
+	 * A batch sent to a contact is considered lost when this many more
+	 * recently sent batches have been acknowledged.
+	 */
+	static final int RETRANSMIT_THRESHOLD = 5;
 }

components/net/sf/briar/db/JdbcDatabase.java

@@ -3,7 +3,6 @@ package net.sf.briar.db;
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
-import java.sql.Blob;
 import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
@@ -12,9 +11,7 @@ import java.sql.Statement;
 import java.sql.Types;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -272,8 +269,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 			if(resume) {
 				if(LOG.isLoggable(Level.FINE))
 					LOG.fine(getNumberOfMessages(txn) + " messages");
-			}
-			else {
+			} else {
 				if(LOG.isLoggable(Level.FINE))
 					LOG.fine("Creating database tables");
 				createTables(txn);
@@ -384,13 +380,11 @@ abstract class JdbcDatabase implements Database<Connection> {
 			}
 		} catch(SQLException e) {
 			// Try to close the connection
-			if(LOG.isLoggable(Level.WARNING))
-				LOG.warning(e.getMessage());
+			if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.getMessage());
 			try {
 				txn.close();
 			} catch(SQLException e1) {
-				if(LOG.isLoggable(Level.WARNING))
-					LOG.warning(e1.getMessage());
+				if(LOG.isLoggable(Level.WARNING)) LOG.warning(e1.getMessage());
 			}
 			// Whatever happens, allow the database to close
 			synchronized(connections) {
@@ -823,11 +817,11 @@ abstract class JdbcDatabase implements Database<Connection> {
 			ps = txn.prepareStatement(sql);
 			ps.setInt(1, t.getInt());
 			rs = ps.executeQuery();
-			TransportConfig config = new TransportConfig();
-			while(rs.next()) config.put(rs.getString(1), rs.getString(2));
+			TransportConfig c = new TransportConfig();
+			while(rs.next()) c.put(rs.getString(1), rs.getString(2));
 			rs.close();
 			ps.close();
-			return config;
+			return c;
 		} catch(SQLException e) {
 			tryToClose(rs);
 			tryToClose(ps);
@@ -847,6 +841,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 			ps.setInt(2, t.getInt());
 			rs = ps.executeQuery();
 			if(rs.next()) {
+				// A connection window row exists - update it
 				long outgoing = rs.getLong(1);
 				if(rs.next()) throw new DbStateException();
 				rs.close();
@@ -862,6 +857,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 				ps.close();
 				return outgoing;
 			} else {
+				// No connection window row exists - create one
 				rs.close();
 				ps.close();
 				sql = "INSERT INTO connectionWindows"
@@ -967,6 +963,28 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
+	public TransportProperties getLocalProperties(Connection txn, TransportId t)
+	throws DbException {
+		PreparedStatement ps = null;
+		ResultSet rs = null;
+		try {
+			String sql = "SELECT key, value FROM transports"
+				+ " WHERE transportId = ?";
+			ps = txn.prepareStatement(sql);
+			ps.setInt(1, t.getInt());
+			rs = ps.executeQuery();
+			TransportProperties p = new TransportProperties();
+			while(rs.next()) p.put(rs.getString(1), rs.getString(2));
+			rs.close();
+			ps.close();
+			return p;
+		} catch(SQLException e) {
+			tryToClose(rs);
+			tryToClose(ps);
+			throw new DbException(e);
+		}
+	}
+
 	public Map<TransportId, TransportProperties> getLocalTransports(
 			Connection txn) throws DbException {
 		PreparedStatement ps = null;
@@ -978,15 +996,15 @@ abstract class JdbcDatabase implements Database<Connection> {
 			rs = ps.executeQuery();
 			Map<TransportId, TransportProperties> transports =
 				new HashMap<TransportId, TransportProperties>();
-			TransportProperties properties = null;
+			TransportProperties p = null;
 			TransportId lastId = null;
 			while(rs.next()) {
 				TransportId id = new TransportId(rs.getInt(1));
 				if(!id.equals(lastId)) {
-					properties = new TransportProperties();
-					transports.put(id, properties);
+					p = new TransportProperties();
+					transports.put(id, p);
 				}
-				properties.put(rs.getString(2), rs.getString(3));
+				p.put(rs.getString(2), rs.getString(3));
 			}
 			rs.close();
 			ps.close();
@@ -1007,7 +1025,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 				+ " WHERE contactId = ? AND passover >= ?";
 			ps = txn.prepareStatement(sql);
 			ps.setInt(1, c.getInt());
-			ps.setInt(2, RETRANSMIT_THRESHOLD);
+			ps.setInt(2, DatabaseConstants.RETRANSMIT_THRESHOLD);
 			rs = ps.executeQuery();
 			Collection<BatchId> ids = new ArrayList<BatchId>();
 			while(rs.next()) ids.add(new BatchId(rs.getBytes(1)));
@@ -1031,8 +1049,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 			rs = ps.executeQuery();
 			if(!rs.next()) throw new DbStateException();
 			int size = rs.getInt(1);
-			Blob b = rs.getBlob(2);
-			byte[] raw = b.getBytes(1, size);
+			byte[] raw = rs.getBlob(2).getBytes(1, size);
 			if(raw.length != size) throw new DbStateException();
 			if(rs.next()) throw new DbStateException();
 			rs.close();
@@ -1063,8 +1080,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 			byte[] raw = null;
 			if(rs.next()) {
 				int size = rs.getInt(1);
-				Blob b = rs.getBlob(2);
-				raw = b.getBytes(1, size);
+				raw = rs.getBlob(2).getBytes(1, size);
 				if(raw.length != size) throw new DbStateException();
 			}
 			if(rs.next()) throw new DbStateException();
@@ -1093,8 +1109,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 			rs = ps.executeQuery();
 			if(rs.next()) {
 				int size = rs.getInt(1);
-				Blob b = rs.getBlob(2);
-				raw = b.getBytes(1, size);
+				raw = rs.getBlob(2).getBytes(1, size);
 				if(raw.length != size) throw new DbStateException();
 			}
 			if(rs.next()) throw new DbStateException();
@@ -1160,7 +1175,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 			ps.setBytes(1, m.getBytes());
 			rs = ps.executeQuery();
 			if(!rs.next()) throw new DbStateException();
-			byte[] group = rs.getBytes(1);
+			byte[] groupId = rs.getBytes(1);
 			if(rs.next()) throw new DbStateException();
 			rs.close();
 			ps.close();
@@ -1169,7 +1184,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 				+ " AND sendability > ZERO()";
 			ps = txn.prepareStatement(sql);
 			ps.setBytes(1, m.getBytes());
-			ps.setBytes(2, group);
+			ps.setBytes(2, groupId);
 			rs = ps.executeQuery();
 			if(!rs.next()) throw new DbStateException();
 			int count = rs.getInt(1);
@@ -1249,12 +1264,12 @@ abstract class JdbcDatabase implements Database<Connection> {
 			Map<ContactId, TransportProperties> properties =
 				new HashMap<ContactId, TransportProperties>();
 			TransportProperties p = null;
-			ContactId lastContactId = null;
+			ContactId lastId = null;
 			while(rs.next()) {
-				ContactId contactId = new ContactId(rs.getInt(1));
-				if(!contactId.equals(lastContactId)) {
+				ContactId id = new ContactId(rs.getInt(1));
+				if(!id.equals(lastId)) {
 					p = new TransportProperties();
-					properties.put(contactId, p);
+					properties.put(id, p);
 				}
 				p.put(rs.getString(2), rs.getString(3));
 			}
@@ -1504,8 +1519,8 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
-	public Map<Group, Long> getVisibleSubscriptions(Connection txn,
-			ContactId c) throws DbException {
+	public Map<Group, Long> getVisibleSubscriptions(Connection txn, ContactId c)
+	throws DbException {
 		long expiry = getApproximateExpiryTime(txn);
 		PreparedStatement ps = null;
 		ResultSet rs = null;
@@ -1754,36 +1769,23 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
-	public Collection<ContactId> removeSubscription(Connection txn, GroupId g)
+	public void removeSubscription(Connection txn, GroupId g)
 	throws DbException {
 		PreparedStatement ps = null;
-		ResultSet rs = null;
 		try {
-			// Retrieve the contacts to which the subscription is visible
-			String sql = "SELECT contactId FROM visibilities WHERE groupId = ?";
-			ps = txn.prepareStatement(sql);
-			ps.setBytes(1, g.getBytes());
-			rs = ps.executeQuery();
-			Collection<ContactId> visible = new ArrayList<ContactId>();
-			while(rs.next()) visible.add(new ContactId(rs.getInt(1)));
-			rs.close();
-			ps.close();
-			// Delete the subscription
-			sql = "DELETE FROM subscriptions WHERE groupId = ?";
+			String sql = "DELETE FROM subscriptions WHERE groupId = ?";
 			ps = txn.prepareStatement(sql);
 			ps.setBytes(1, g.getBytes());
 			int affected = ps.executeUpdate();
 			if(affected != 1) throw new DbStateException();
 			ps.close();
-			return visible;
 		} catch(SQLException e) {
-			tryToClose(rs);
 			tryToClose(ps);
 			throw new DbException(e);
 		}
 	}
 
-	public void setConfig(Connection txn, TransportId t, TransportConfig config)
+	public void setConfig(Connection txn, TransportId t, TransportConfig c)
 	throws DbException {
 		PreparedStatement ps = null;
 		try {
@@ -1798,13 +1800,13 @@ abstract class JdbcDatabase implements Database<Connection> {
 				+ " VALUES (?, ?, ?)";
 			ps = txn.prepareStatement(sql);
 			ps.setInt(1, t.getInt());
-			for(Entry<String, String> e : config.entrySet()) {
+			for(Entry<String, String> e : c.entrySet()) {
 				ps.setString(2, e.getKey());
 				ps.setString(3, e.getValue());
 				ps.addBatch();
 			}
 			int[] batchAffected = ps.executeBatch();
-			if(batchAffected.length != config.size())
+			if(batchAffected.length != c.size())
 				throw new DbStateException();
 			for(int i = 0; i < batchAffected.length; i++) {
 				if(batchAffected[i] != 1) throw new DbStateException();
@@ -1827,10 +1829,12 @@ abstract class JdbcDatabase implements Database<Connection> {
 			ps.setInt(1, c.getInt());
 			ps.setInt(2, t.getInt());
 			rs = ps.executeQuery();
-			if(rs.next()) {
-				if(rs.next()) throw new DbStateException();
-				rs.close();
-				ps.close();
+			boolean found = rs.next();
+			if(rs.next()) throw new DbStateException();
+			rs.close();
+			ps.close();
+			if(found) {
+				// A connection window row exists - update it
 				sql = "UPDATE connectionWindows SET centre = ?, bitmap = ?"
 					+ " WHERE contactId = ? AND transportId = ?";
 				ps = txn.prepareStatement(sql);
@@ -1842,8 +1846,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 				if(affected != 1) throw new DbStateException();
 				ps.close();
 			} else {
-				rs.close();
-				ps.close();
+				// No connection window row exists - create one
 				sql = "INSERT INTO connectionWindows"
 					+ " (contactId, transportId, centre, bitmap, outgoing)"
 					+ " VALUES(?, ?, ?, ?, ZERO())";
@@ -1863,25 +1866,12 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
-	public boolean setLocalProperties(Connection txn, TransportId t,
-			TransportProperties properties) throws DbException {
+	public void setLocalProperties(Connection txn, TransportId t,
+			TransportProperties p) throws DbException {
 		PreparedStatement ps = null;
-		ResultSet rs = null;
 		try {
-			// Retrieve any existing properties for the given transport
-			String sql = "SELECT key, value FROM transports"
-				+ " WHERE transportId = ?";
-			ps = txn.prepareStatement(sql);
-			ps.setInt(1, t.getInt());
-			rs = ps.executeQuery();
-			TransportProperties old = new TransportProperties();
-			while(rs.next()) old.put(rs.getString(1), rs.getString(2));
-			rs.close();
-			ps.close();
-			// If the properties haven't changed, return
-			if(old.equals(properties)) return false;
 			// Delete any existing properties for the given transport
-			sql = "DELETE FROM transports WHERE transportId = ?";
+			String sql = "DELETE FROM transports WHERE transportId = ?";
 			ps = txn.prepareStatement(sql);
 			ps.setInt(1, t.getInt());
 			ps.executeUpdate();
@@ -1891,27 +1881,19 @@ abstract class JdbcDatabase implements Database<Connection> {
 				+ " VALUES (?, ?, ?)";
 			ps = txn.prepareStatement(sql);
 			ps.setInt(1, t.getInt());
-			for(Entry<String, String> e : properties.entrySet()) {
+			for(Entry<String, String> e : p.entrySet()) {
 				ps.setString(2, e.getKey());
 				ps.setString(3, e.getValue());
 				ps.addBatch();
 			}
 			int[] batchAffected = ps.executeBatch();
-			if(batchAffected.length != properties.size())
+			if(batchAffected.length != p.size())
 				throw new DbStateException();
 			for(int i = 0; i < batchAffected.length; i++) {
 				if(batchAffected[i] != 1) throw new DbStateException();
 			}
 			ps.close();
-			// Update the transport timestamps of all contacts
-			sql = "UPDATE transportTimestamps set modified = ?";
-			ps = txn.prepareStatement(sql);
-			ps.setLong(1, System.currentTimeMillis());
-			ps.executeUpdate();
-			ps.close();
-			return true;
 		} catch(SQLException e) {
-			tryToClose(rs);
 			tryToClose(ps);
 			throw new DbException(e);
 		}
@@ -1928,18 +1910,22 @@ abstract class JdbcDatabase implements Database<Connection> {
 			rs = ps.executeQuery();
 			Rating old;
 			if(rs.next()) {
+				// A rating row exists - update it
 				old = Rating.values()[rs.getByte(1)];
 				if(rs.next()) throw new DbStateException();
 				rs.close();
 				ps.close();
-				sql = "UPDATE ratings SET rating = ? WHERE authorId = ?";
-				ps = txn.prepareStatement(sql);
-				ps.setShort(1, (short) r.ordinal());
-				ps.setBytes(2, a.getBytes());
-				int affected = ps.executeUpdate();
-				if(affected != 1) throw new DbStateException();
-				ps.close();
+				if(!old.equals(r)) {
+					sql = "UPDATE ratings SET rating = ? WHERE authorId = ?";
+					ps = txn.prepareStatement(sql);
+					ps.setShort(1, (short) r.ordinal());
+					ps.setBytes(2, a.getBytes());
+					int affected = ps.executeUpdate();
+					if(affected != 1) throw new DbStateException();
+					ps.close();
+				}
 			} else {
+				// No rating row exists - create one
 				rs.close();
 				ps.close();
 				old = Rating.UNRATED;
@@ -1989,6 +1975,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 			ps.setInt(2, c.getInt());
 			rs = ps.executeQuery();
 			if(rs.next()) {
+				// A status row exists - update it
 				Status old = Status.values()[rs.getByte(1)];
 				if(rs.next()) throw new DbStateException();
 				rs.close();
@@ -2005,6 +1992,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 					ps.close();
 				}
 			} else {
+				// No status row exists - create one
 				rs.close();
 				ps.close();
 				sql = "INSERT INTO statuses (messageId, contactId, status)"
@@ -2118,12 +2106,38 @@ abstract class JdbcDatabase implements Database<Connection> {
 			if(affected != 1) throw new DbStateException();
 			ps.close();
 		} catch(SQLException e) {
+			tryToClose(rs);
 			tryToClose(ps);
 			throw new DbException(e);
 		}
 	}
 
-	public void setSubscriptionTimestamp(Connection txn, ContactId c,
+	public void setSubscriptionsModifiedTimestamp(Connection txn,
+			Collection<ContactId> contacts, long timestamp) throws DbException {
+		PreparedStatement ps = null;
+		try {
+			String sql = "UPDATE subscriptionTimestamps SET modified = ?"
+				+ " WHERE contactId = ?";
+			ps = txn.prepareStatement(sql);
+			ps.setLong(1, timestamp);
+			for(ContactId c : contacts) {
+				ps.setInt(2, c.getInt());
+				ps.addBatch();
+			}
+			int[] batchAffected = ps.executeBatch();
+			if(batchAffected.length != contacts.size())
+				throw new DbStateException();
+			for(int i = 0; i < batchAffected.length; i++) {
+				if(batchAffected[i] > 1) throw new DbStateException();
+			}
+			ps.close();
+		} catch(SQLException e) {
+			tryToClose(ps);
+			throw new DbException(e);
+		}
+	}
+
+	public void setSubscriptionsSentTimestamp(Connection txn, ContactId c,
 			long timestamp) throws DbException {
 		PreparedStatement ps = null;
 		try {
@@ -2199,12 +2213,28 @@ abstract class JdbcDatabase implements Database<Connection> {
 			if(affected != 1) throw new DbStateException();
 			ps.close();
 		} catch(SQLException e) {
+			tryToClose(rs);
 			tryToClose(ps);
 			throw new DbException(e);
 		}
 	}
 
-	public void setTransportTimestamp(Connection txn, ContactId c,
+	public void setTransportsModifiedTimestamp(Connection txn, long timestamp)
+	throws DbException {
+		PreparedStatement ps = null;
+		try {
+			String sql = "UPDATE transportTimestamps set modified = ?";
+			ps = txn.prepareStatement(sql);
+			ps.setLong(1, timestamp);
+			ps.executeUpdate();
+			ps.close();
+		} catch(SQLException e) {
+			tryToClose(ps);
+			throw new DbException(e);
+		}
+	}
+
+	public void setTransportsSentTimestamp(Connection txn, ContactId c,
 			long timestamp) throws DbException {
 		PreparedStatement ps = null;
 		try {
@@ -2223,25 +2253,12 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
-	public Collection<ContactId> setVisibility(Connection txn, GroupId g,
+	public void setVisibility(Connection txn, GroupId g,
 			Collection<ContactId> visible) throws DbException {
 		PreparedStatement ps = null;
-		ResultSet rs = null;
 		try {
-			// Retrieve any existing visibilities
-			String sql = "SELECT contactId FROM visibilities WHERE groupId = ?";
-			ps = txn.prepareStatement(sql);
-			ps.setBytes(1, g.getBytes());
-			rs = ps.executeQuery();
-			Collection<ContactId> then = new HashSet<ContactId>();
-			while(rs.next()) then.add(new ContactId(rs.getInt(1)));
-			rs.close();
-			ps.close();
-			// If the visibilities haven't changed, return
-			Collection<ContactId> now = new HashSet<ContactId>(visible);
-			if(then.equals(now)) return Collections.emptyList();
 			// Delete any existing visibilities
-			sql = "DELETE FROM visibilities where groupId = ?";
+			String sql = "DELETE FROM visibilities where groupId = ?";
 			ps = txn.prepareStatement(sql);
 			ps.setBytes(1, g.getBytes());
 			ps.executeUpdate();
@@ -2262,28 +2279,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 				if(batchAffected[i] != 1) throw new DbStateException();
 			}
 			ps.close();
-			// Update the subscription timestamps of any affected contacts
-			Collection<ContactId> affected = new ArrayList<ContactId>();
-			for(ContactId c : then) if(!now.contains(c)) affected.add(c);
-			for(ContactId c : now) if(!then.contains(c)) affected.add(c);
-			sql = "UPDATE subscriptionTimestamps SET modified = ?"
-				+ " WHERE contactId = ?";
-			ps = txn.prepareStatement(sql);
-			ps.setLong(1, System.currentTimeMillis());
-			for(ContactId c : affected) {
-				ps.setInt(2, c.getInt());
-				ps.addBatch();
-			}
-			batchAffected = ps.executeBatch();
-			if(batchAffected.length != affected.size())
-				throw new DbStateException();
-			for(int i = 0; i < batchAffected.length; i++) {
-				if(batchAffected[i] > 1) throw new DbStateException();
-			}
-			ps.close();
-			return affected;
 		} catch(SQLException e) {
-			tryToClose(rs);
 			tryToClose(ps);
 			throw new DbException(e);
 		}

components/net/sf/briar/plugins/PluginManagerImpl.java

@@ -190,7 +190,7 @@ class PluginManagerImpl implements PluginManager {
 		public TransportProperties getLocalProperties() {
 			assert id != null;
 			try {
-				TransportProperties p = db.getLocalTransports().get(id);
+				TransportProperties p = db.getLocalProperties(id);
 				return p == null ? new TransportProperties() : p;
 			} catch(DbException e) {
 				if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.getMessage());

test/net/sf/briar/db/DatabaseComponentTest.java

@@ -148,8 +148,9 @@ public abstract class DatabaseComponentTest extends TestCase {
 			// unsubscribe(groupId)
 			oneOf(database).containsSubscription(txn, groupId);
 			will(returnValue(true));
-			oneOf(database).removeSubscription(txn, groupId);
+			oneOf(database).getVisibility(txn, groupId);
 			will(returnValue(Collections.<ContactId>emptyList()));
+			oneOf(database).removeSubscription(txn, groupId);
 			// unsubscribe(groupId) again
 			oneOf(database).containsSubscription(txn, groupId);
 			will(returnValue(false));
@@ -755,7 +756,7 @@ public abstract class DatabaseComponentTest extends TestCase {
 			// Get the visible subscriptions
 			oneOf(database).getVisibleSubscriptions(txn, contactId);
 			will(returnValue(Collections.singletonMap(group, 0L)));
-			oneOf(database).setSubscriptionTimestamp(with(txn), with(contactId),
+			oneOf(database).setSubscriptionsSentTimestamp(with(txn), with(contactId),
 					with(any(long.class)));
 			// Add the subscriptions to the writer
 			oneOf(subscriptionWriter).writeSubscriptions(
@@ -790,7 +791,7 @@ public abstract class DatabaseComponentTest extends TestCase {
 			// Get the local transport properties
 			oneOf(database).getLocalTransports(txn);
 			will(returnValue(transports));
-			oneOf(database).setTransportTimestamp(with(txn), with(contactId),
+			oneOf(database).setTransportsSentTimestamp(with(txn), with(contactId),
 					with(any(long.class)));
 			// Add the properties to the writer
 			oneOf(transportWriter).writeTransports(with(transports),
@@ -1283,8 +1284,11 @@ public abstract class DatabaseComponentTest extends TestCase {
 		context.checking(new Expectations() {{
 			oneOf(database).startTransaction();
 			will(returnValue(txn));
+			oneOf(database).getLocalProperties(txn, transportId);
+			will(returnValue(new TransportProperties()));
 			oneOf(database).setLocalProperties(txn, transportId, properties);
-			will(returnValue(true));
+			oneOf(database).setTransportsModifiedTimestamp(with(txn),
+					with(any(long.class)));
 			oneOf(database).commitTransaction(txn);
 			oneOf(listener).eventOccurred(with(any(
 					TransportsUpdatedEvent.class)));
@@ -1310,8 +1314,8 @@ public abstract class DatabaseComponentTest extends TestCase {
 		context.checking(new Expectations() {{
 			oneOf(database).startTransaction();
 			will(returnValue(txn));
-			oneOf(database).setLocalProperties(txn, transportId, properties);
-			will(returnValue(false));
+			oneOf(database).getLocalProperties(txn, transportId);
+			will(returnValue(properties));
 			oneOf(database).commitTransaction(txn);
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner);

test/net/sf/briar/db/H2DatabaseTest.java

@@ -719,7 +719,7 @@ public class H2DatabaseTest extends TestCase {
 
 	@Test
 	public void testRetransmission() throws Exception {
-		BatchId[] ids = new BatchId[Database.RETRANSMIT_THRESHOLD + 5];
+		BatchId[] ids = new BatchId[DatabaseConstants.RETRANSMIT_THRESHOLD + 5];
 		for(int i = 0; i < ids.length; i++) {
 			ids[i] = new BatchId(TestUtils.getRandomId());
 		}
@@ -740,7 +740,7 @@ public class H2DatabaseTest extends TestCase {
 
 		// The contact acks the batches in reverse order. The first
 		// RETRANSMIT_THRESHOLD - 1 acks should not trigger any retransmissions
-		for(int i = 0; i < Database.RETRANSMIT_THRESHOLD - 1; i++) {
+		for(int i = 0; i < DatabaseConstants.RETRANSMIT_THRESHOLD - 1; i++) {
 			db.removeAckedBatch(txn, contactId, ids[ids.length - i - 1]);
 			Collection<BatchId> lost = db.getLostBatches(txn, contactId);
 			assertEquals(Collections.emptyList(), lost);
@@ -748,7 +748,7 @@ public class H2DatabaseTest extends TestCase {
 
 		// The next ack should trigger the retransmission of the remaining
 		// five outstanding batches
-		int index = ids.length - Database.RETRANSMIT_THRESHOLD;
+		int index = ids.length - DatabaseConstants.RETRANSMIT_THRESHOLD;
 		db.removeAckedBatch(txn, contactId, ids[index]);
 		Collection<BatchId> lost = db.getLostBatches(txn, contactId);
 		for(int i = 0; i < index; i++) {
@@ -761,7 +761,7 @@ public class H2DatabaseTest extends TestCase {
 
 	@Test
 	public void testNoRetransmission() throws Exception {
-		BatchId[] ids = new BatchId[Database.RETRANSMIT_THRESHOLD * 2];
+		BatchId[] ids = new BatchId[DatabaseConstants.RETRANSMIT_THRESHOLD * 2];
 		for(int i = 0; i < ids.length; i++) {
 			ids[i] = new BatchId(TestUtils.getRandomId());
 		}

test/net/sf/briar/plugins/PluginManagerImplTest.java

@@ -1,6 +1,5 @@
 package net.sf.briar.plugins;
 
-import java.util.Map;
 import java.util.concurrent.Executor;
 
 import junit.framework.TestCase;
@@ -20,14 +19,11 @@ public class PluginManagerImplTest extends TestCase {
 	public void testStartAndStop() throws Exception {
 		Mockery context = new Mockery();
 		final DatabaseComponent db = context.mock(DatabaseComponent.class);
-		final Map<?, ?> localTransports = context.mock(Map.class);
 		final ConnectionDispatcher dispatcher =
 			context.mock(ConnectionDispatcher.class);
 		final UiCallback uiCallback = context.mock(UiCallback.class);
 		context.checking(new Expectations() {{
-			allowing(db).getLocalTransports();
-			will(returnValue(localTransports));
-			allowing(localTransports).get(with(any(TransportId.class)));
+			allowing(db).getLocalProperties(with(any(TransportId.class)));
 			will(returnValue(new TransportProperties()));
 			allowing(db).getRemoteProperties(with(any(TransportId.class)));
 			will(returnValue(new TransportProperties()));