From 2f457162a538e5885e3d21c533f7b5d747c98599 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Mon, 17 Oct 2011 23:24:23 +0100
Subject: [PATCH] Attach the affected contact IDs to subscription update
 events.

---
 .../api/db/event/BatchReceivedEvent.java      |  1 +
 .../briar/api/db/event/ContactAddedEvent.java |  1 +
 .../api/db/event/ContactRemovedEvent.java     |  1 +
 .../sf/briar/api/db/event/DatabaseEvent.java  |  1 +
 .../briar/api/db/event/DatabaseListener.java  |  1 -
 .../api/db/event/MessagesAddedEvent.java      |  4 +++
 .../db/event/SubscriptionsUpdatedEvent.java   | 13 +++++--
 .../api/db/event/TransportsUpdatedEvent.java  |  1 +
 components/net/sf/briar/db/Database.java      |  5 +--
 .../sf/briar/db/DatabaseComponentImpl.java    | 34 +++++++++++--------
 components/net/sf/briar/db/JdbcDatabase.java  | 15 ++++++--
 .../transport/stream/StreamConnection.java    |  9 +++--
 .../sf/briar/db/DatabaseComponentTest.java    | 15 ++++----
 13 files changed, 69 insertions(+), 32 deletions(-)

diff --git a/api/net/sf/briar/api/db/event/BatchReceivedEvent.java b/api/net/sf/briar/api/db/event/BatchReceivedEvent.java
index 6791a53108..a7ebcdef98 100644
--- a/api/net/sf/briar/api/db/event/BatchReceivedEvent.java
+++ b/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 {
 
 }
diff --git a/api/net/sf/briar/api/db/event/ContactAddedEvent.java b/api/net/sf/briar/api/db/event/ContactAddedEvent.java
index 2d02c03de9..84cc5f2988 100644
--- a/api/net/sf/briar/api/db/event/ContactAddedEvent.java
+++ b/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;
diff --git a/api/net/sf/briar/api/db/event/ContactRemovedEvent.java b/api/net/sf/briar/api/db/event/ContactRemovedEvent.java
index e9322fcaa5..face9f4e6d 100644
--- a/api/net/sf/briar/api/db/event/ContactRemovedEvent.java
+++ b/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) {
diff --git a/api/net/sf/briar/api/db/event/DatabaseEvent.java b/api/net/sf/briar/api/db/event/DatabaseEvent.java
index ff3b363dc8..2316bdef35 100644
--- a/api/net/sf/briar/api/db/event/DatabaseEvent.java
+++ b/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 {
 
 }
diff --git a/api/net/sf/briar/api/db/event/DatabaseListener.java b/api/net/sf/briar/api/db/event/DatabaseListener.java
index d55be9b767..58605bee12 100644
--- a/api/net/sf/briar/api/db/event/DatabaseListener.java
+++ b/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 {
 
diff --git a/api/net/sf/briar/api/db/event/MessagesAddedEvent.java b/api/net/sf/briar/api/db/event/MessagesAddedEvent.java
index d8efb677be..c51515d528 100644
--- a/api/net/sf/briar/api/db/event/MessagesAddedEvent.java
+++ b/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 {
 
 }
diff --git a/api/net/sf/briar/api/db/event/SubscriptionsUpdatedEvent.java b/api/net/sf/briar/api/db/event/SubscriptionsUpdatedEvent.java
index 416a49752a..2f878438b6 100644
--- a/api/net/sf/briar/api/db/event/SubscriptionsUpdatedEvent.java
+++ b/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;
 	}
diff --git a/api/net/sf/briar/api/db/event/TransportsUpdatedEvent.java b/api/net/sf/briar/api/db/event/TransportsUpdatedEvent.java
index 0778eacf83..1360fe9ace 100644
--- a/api/net/sf/briar/api/db/event/TransportsUpdatedEvent.java
+++ b/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 {
 
 }
diff --git a/components/net/sf/briar/db/Database.java b/components/net/sf/briar/db/Database.java
index 7a423ae9a3..61f34953a7 100644
--- a/components/net/sf/briar/db/Database.java
+++ b/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.
diff --git a/components/net/sf/briar/db/DatabaseComponentImpl.java b/components/net/sf/briar/db/DatabaseComponentImpl.java
index 927ca49ce8..7fdebaa51e 100644
--- a/components/net/sf/briar/db/DatabaseComponentImpl.java
+++ b/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
-					Collection<ContactId> present =
-						new ArrayList<ContactId>(visible.size());
-					for(ContactId c : visible) {
-						if(db.containsContact(txn, c)) present.add(c);
-					}
-					db.setVisibility(txn, g, present);
+					// Get the contacts to which the group used to be visible
+					then = new HashSet<ContactId>(db.getVisibility(txn, g));
+					// Don't try to make the group visible to ex-contacts
+					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);
@@ -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())) {
-					db.addSubscription(txn, g);
-					added = true;
-				}
+				added = db.addSubscription(txn, g);
 				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 {
diff --git a/components/net/sf/briar/db/JdbcDatabase.java b/components/net/sf/briar/db/JdbcDatabase.java
index 59c5fd7e05..15a28e3cf9 100644
--- a/components/net/sf/briar/db/JdbcDatabase.java
+++ b/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);
diff --git a/components/net/sf/briar/transport/stream/StreamConnection.java b/components/net/sf/briar/transport/stream/StreamConnection.java
index 8683d25fd1..4199094376 100644
--- a/components/net/sf/briar/transport/stream/StreamConnection.java
+++ b/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
-				writerFlags |= Flags.SUBSCRIPTIONS_UPDATED;
-				notifyAll();
+				Collection<ContactId> affected =
+					((SubscriptionsUpdatedEvent) e).getAffectedContacts();
+				if(affected.contains(contactId)) {
+					writerFlags |= Flags.SUBSCRIPTIONS_UPDATED;
+					notifyAll();
+				}
 			} else if(e instanceof TransportsUpdatedEvent) {
 				writerFlags |= Flags.TRANSPORTS_UPDATED;
 				notifyAll();
diff --git a/test/net/sf/briar/db/DatabaseComponentTest.java b/test/net/sf/briar/db/DatabaseComponentTest.java
index f629c64512..15b1b74014 100644
--- a/test/net/sf/briar/db/DatabaseComponentTest.java
+++ b/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();
-			will(returnValue(groupId));
-			oneOf(database).containsSubscription(txn, groupId);
-			will(returnValue(true));
+			oneOf(database).addSubscription(txn, group);
+			will(returnValue(false));
 			// 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)
-- 
GitLab