Attach the affected contact IDs to subscription update events.

api/net/sf/briar/api/db/event/BatchReceivedEvent.java

@@ -1,5 +1,6 @@
 package net.sf.briar.api.db.event;
 
+/** An event that is broadcast when a batch of messages is received. */
 public class BatchReceivedEvent extends DatabaseEvent {
 
 }

api/net/sf/briar/api/db/event/ContactAddedEvent.java

@@ -2,6 +2,7 @@ package net.sf.briar.api.db.event;
 
 import net.sf.briar.api.ContactId;
 
+/** An event that is broadcast when a contact is added. */
 public class ContactAddedEvent extends DatabaseEvent {
 
 	private final ContactId contactId;

api/net/sf/briar/api/db/event/ContactRemovedEvent.java

@@ -2,6 +2,7 @@ package net.sf.briar.api.db.event;
 
 import net.sf.briar.api.ContactId;
 
+/** An event that is broadcast when a contact is removed. */
 public class ContactRemovedEvent extends ContactAddedEvent {
 
 	public ContactRemovedEvent(ContactId contactId) {

api/net/sf/briar/api/db/event/DatabaseEvent.java

@@ -1,5 +1,6 @@
 package net.sf.briar.api.db.event;
 
+/** An abstract superclass for database events. */
 public abstract class DatabaseEvent {
 
 }

api/net/sf/briar/api/db/event/DatabaseListener.java

@@ -1,6 +1,5 @@
 package net.sf.briar.api.db.event;
 
-
 /** An interface for receiving notifications when database events occur. */
 public interface DatabaseListener {
 

api/net/sf/briar/api/db/event/MessagesAddedEvent.java

@@ -1,5 +1,9 @@
 package net.sf.briar.api.db.event;
 
+/**
+ * An event that is broadcast when one or more messages are added to the
+ * database.
+ */
 public class MessagesAddedEvent extends DatabaseEvent {
 
 }

api/net/sf/briar/api/db/event/SubscriptionsUpdatedEvent.java

@@ -1,18 +1,27 @@
 package net.sf.briar.api.db.event;
 
 import java.util.Collection;
+import java.util.Collections;
 
 import net.sf.briar.api.ContactId;
 
+/**
+ * An event that is broadcast when the set of subscriptions visible to one or
+ * more contacts is updated.
+ */
 public class SubscriptionsUpdatedEvent extends DatabaseEvent {
 
 	private final Collection<ContactId> affectedContacts;
 
-	// FIXME: Replace this constructor
 	public SubscriptionsUpdatedEvent() {
-		affectedContacts = null;
+		affectedContacts = Collections.emptyList();
 	}
 
+	public SubscriptionsUpdatedEvent(Collection<ContactId> affectedContacts) {
+		this.affectedContacts = affectedContacts;
+	}
+
+	/** Returns the contacts affected by the update. */
 	public Collection<ContactId> getAffectedContacts() {
 		return affectedContacts;
 	}

api/net/sf/briar/api/db/event/TransportsUpdatedEvent.java

@@ -1,5 +1,6 @@
 package net.sf.briar.api.db.event;
 
+/** An event that is broadcast when the local transports are updated. */
 public class TransportsUpdatedEvent extends DatabaseEvent {
 
 }

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

@@ -116,11 +116,12 @@ interface Database<T> {
 	boolean addPrivateMessage(T txn, Message m, ContactId c) throws DbException;
 
 	/**
-	 * Subscribes to the given group.
+	 * Subscribes to the given group and returns true if the subscription did
+	 * not previously exist.
 	 * <p>
 	 * Locking: subscriptions write.
 	 */
-	void addSubscription(T txn, Group g) throws DbException;
+	boolean addSubscription(T txn, Group g) throws DbException;
 
 	/**
 	 * Returns true if the database contains the given contact.

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;
@@ -1277,19 +1278,19 @@ DatabaseCleaner.Callback {
 
 	public void setVisibility(GroupId g, Collection<ContactId> visible)
 	throws DbException {
+		Collection<ContactId> then, now;
 		contactLock.readLock().lock();
 		try {
 			subscriptionLock.writeLock().lock();
 			try {
 				T txn = db.startTransaction();
 				try {
-					// Remove any ex-contacts from the set
+					// Get the contacts to which the group used to be visible
-					Collection<ContactId> present =
+					then = new HashSet<ContactId>(db.getVisibility(txn, g));
-						new ArrayList<ContactId>(visible.size());
+					// Don't try to make the group visible to ex-contacts
-					for(ContactId c : visible) {
+					now = new HashSet<ContactId>(visible);
-						if(db.containsContact(txn, c)) present.add(c);
+					now.retainAll(new HashSet<ContactId>(db.getContacts(txn)));
-					}
+					db.setVisibility(txn, g, now);
-					db.setVisibility(txn, g, present);
 					db.commitTransaction(txn);
 				} catch(DbException e) {
 					db.abortTransaction(txn);
@@ -1301,19 +1302,22 @@ 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
+		callListeners(new SubscriptionsUpdatedEvent(affected));
 	}
 
 	public void subscribe(Group g) throws DbException {
 		if(LOG.isLoggable(Level.FINE)) LOG.fine("Subscribing to " + g);
-		boolean added = false;
+		boolean added;
 		subscriptionLock.writeLock().lock();
 		try {
 			T txn = db.startTransaction();
 			try {
-				if(db.containsSubscription(txn, g.getId())) {
+				added = db.addSubscription(txn, g);
-					db.addSubscription(txn, g);
-					added = true;
-				}
 				db.commitTransaction(txn);
 			} catch(DbException e) {
 				db.abortTransaction(txn);
@@ -1328,7 +1332,8 @@ DatabaseCleaner.Callback {
 
 	public void unsubscribe(GroupId g) throws DbException {
 		if(LOG.isLoggable(Level.FINE)) LOG.fine("Unsubscribing from " + g);
-		boolean removed = false;
+		boolean removed;
+		Collection<ContactId> affected;
 		contactLock.readLock().lock();
 		try {
 			messageLock.writeLock().lock();
@@ -1339,6 +1344,7 @@ DatabaseCleaner.Callback {
 					try {
 						T txn = db.startTransaction();
 						try {
+							affected = db.getVisibility(txn, g);
 							removed = db.removeSubscription(txn, g);
 							db.commitTransaction(txn);
 						} catch(DbException e) {
@@ -1358,7 +1364,7 @@ DatabaseCleaner.Callback {
 			contactLock.readLock().unlock();
 		}
 		// Call the listeners outside the lock
-		if(removed) callListeners(new SubscriptionsUpdatedEvent());
+		if(removed) callListeners(new SubscriptionsUpdatedEvent(affected));
 	}
 
 	public void checkFreeSpaceAndClean() throws DbException {

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

@@ -652,10 +652,20 @@ abstract class JdbcDatabase implements Database<Connection> {
 		}
 	}
 
-	public void addSubscription(Connection txn, Group g) throws DbException {
+	public boolean addSubscription(Connection txn, Group g) throws DbException {
 		PreparedStatement ps = null;
+		ResultSet rs = null;
 		try {
-			String sql = "INSERT INTO subscriptions"
+			String sql = "SELECT NULL FROM subscriptions WHERE groupId = ?";
+			ps = txn.prepareStatement(sql);
+			ps.setBytes(1, g.getId().getBytes());
+			rs = ps.executeQuery();
+			boolean found = rs.next();
+			if(rs.next()) throw new DbStateException();
+			rs.close();
+			ps.close();
+			if(found) return false;
+			sql = "INSERT INTO subscriptions"
 				+ " (groupId, groupName, groupKey, start)"
 				+ " VALUES (?, ?, ?, ?)";
 			ps = txn.prepareStatement(sql);
@@ -666,6 +676,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 			int affected = ps.executeUpdate();
 			if(affected != 1) throw new DbStateException();
 			ps.close();
+			return true;
 		} catch(SQLException e) {
 			tryToClose(ps);
 			throw new DbException(e);

components/net/sf/briar/transport/stream/StreamConnection.java

@@ -102,9 +102,12 @@ abstract class StreamConnection implements DatabaseListener {
 				writerFlags |= Flags.MESSAGES_ADDED;
 				notifyAll();
 			} else if(e instanceof SubscriptionsUpdatedEvent) {
-				// FIXME: Check whether the change affected this contact
+				Collection<ContactId> affected =
-				writerFlags |= Flags.SUBSCRIPTIONS_UPDATED;
+					((SubscriptionsUpdatedEvent) e).getAffectedContacts();
-				notifyAll();
+				if(affected.contains(contactId)) {
+					writerFlags |= Flags.SUBSCRIPTIONS_UPDATED;
+					notifyAll();
+				}
 			} else if(e instanceof TransportsUpdatedEvent) {
 				writerFlags |= Flags.TRANSPORTS_UPDATED;
 				notifyAll();

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

@@ -134,27 +134,26 @@ public abstract class DatabaseComponentTest extends TestCase {
 			oneOf(database).getRemoteProperties(txn, transportId);
 			will(returnValue(remoteProperties));
 			// subscribe(group)
-			oneOf(group).getId();
-			will(returnValue(groupId));
-			oneOf(database).containsSubscription(txn, groupId);
-			will(returnValue(false));
 			oneOf(database).addSubscription(txn, group);
+			will(returnValue(true));
 			oneOf(listener).eventOccurred(with(any(
 					SubscriptionsUpdatedEvent.class)));
 			// subscribe(group) again
-			oneOf(group).getId();
+			oneOf(database).addSubscription(txn, group);
-			will(returnValue(groupId));
+			will(returnValue(false));
-			oneOf(database).containsSubscription(txn, groupId);
-			will(returnValue(true));
 			// getSubscriptions()
 			oneOf(database).getSubscriptions(txn);
 			will(returnValue(Collections.singletonList(groupId)));
 			// unsubscribe(groupId)
+			oneOf(database).getVisibility(txn, groupId);
+			will(returnValue(Collections.<ContactId>emptySet()));
 			oneOf(database).removeSubscription(txn, groupId);
 			will(returnValue(true));
 			oneOf(listener).eventOccurred(with(any(
 					SubscriptionsUpdatedEvent.class)));
 			// unsubscribe(groupId) again
+			oneOf(database).getVisibility(txn, groupId);
+			will(returnValue(Collections.<ContactId>emptySet()));
 			oneOf(database).removeSubscription(txn, groupId);
 			will(returnValue(false));
 			// setConnectionWindow(contactId, 123, connectionWindow)