Merge branch 'add-contact-transaction' into 'master'

Add contact and transport keys in the same transaction

This avoids a potential problem where the app crashes after adding the contact but before adding the transport keys, leaving the contact unusable.

See merge request !112

briar-core/src/org/briarproject/contact/ContactManagerImpl.java

@@ -5,6 +5,7 @@ import com.google.inject.Inject;
 import org.briarproject.api.contact.Contact;
 import org.briarproject.api.contact.ContactId;
 import org.briarproject.api.contact.ContactManager;
+import org.briarproject.api.crypto.SecretKey;
 import org.briarproject.api.db.DatabaseComponent;
 import org.briarproject.api.db.DbException;
 import org.briarproject.api.db.Transaction;
@@ -12,6 +13,7 @@ import org.briarproject.api.identity.Author;
 import org.briarproject.api.identity.AuthorId;
 import org.briarproject.api.identity.IdentityManager.RemoveIdentityHook;
 import org.briarproject.api.identity.LocalAuthor;
+import org.briarproject.api.transport.KeyManager;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -22,12 +24,14 @@ import java.util.concurrent.CopyOnWriteArrayList;
 class ContactManagerImpl implements ContactManager, RemoveIdentityHook {
 
 	private final DatabaseComponent db;
+	private final KeyManager keyManager;
 	private final List<AddContactHook> addHooks;
 	private final List<RemoveContactHook> removeHooks;
 
 	@Inject
-	ContactManagerImpl(DatabaseComponent db) {
+	ContactManagerImpl(DatabaseComponent db, KeyManager keyManager) {
 		this.db = db;
+		this.keyManager = keyManager;
 		addHooks = new CopyOnWriteArrayList<AddContactHook>();
 		removeHooks = new CopyOnWriteArrayList<RemoveContactHook>();
 	}
@@ -43,12 +47,14 @@ class ContactManagerImpl implements ContactManager, RemoveIdentityHook {
 	}
 
 	@Override
-	public ContactId addContact(Author remote, AuthorId local, boolean active)
+	public ContactId addContact(Author remote, AuthorId local, SecretKey master,
+			long timestamp, boolean alice, boolean active)
 			throws DbException {
 		ContactId c;
 		Transaction txn = db.startTransaction();
 		try {
 			c = db.addContact(txn, remote, local, active);
+			keyManager.addContact(txn, c, master, timestamp, alice);
 			Contact contact = db.getContact(txn, c);
 			for (AddContactHook hook : addHooks)
 				hook.addingContact(txn, contact);

briar-core/src/org/briarproject/invitation/AliceConnector.java

@@ -15,9 +15,7 @@ import org.briarproject.api.identity.LocalAuthor;
 import org.briarproject.api.plugins.ConnectionManager;
 import org.briarproject.api.plugins.duplex.DuplexPlugin;
 import org.briarproject.api.plugins.duplex.DuplexTransportConnection;
-import org.briarproject.api.sync.GroupFactory;
 import org.briarproject.api.system.Clock;
-import org.briarproject.api.transport.KeyManager;
 import org.briarproject.api.transport.StreamReaderFactory;
 import org.briarproject.api.transport.StreamWriterFactory;
 
@@ -41,12 +39,11 @@ class AliceConnector extends Connector {
 			BdfWriterFactory bdfWriterFactory,
 			StreamReaderFactory streamReaderFactory,
 			StreamWriterFactory streamWriterFactory,
-			AuthorFactory authorFactory, GroupFactory groupFactory,
-			KeyManager keyManager, ConnectionManager connectionManager,
+			AuthorFactory authorFactory, ConnectionManager connectionManager,
 			ContactManager contactManager, Clock clock, ConnectorGroup group,
 			DuplexPlugin plugin, LocalAuthor localAuthor, PseudoRandom random) {
 		super(crypto, bdfReaderFactory, bdfWriterFactory, streamReaderFactory,
-				streamWriterFactory, authorFactory, groupFactory, keyManager,
+				streamWriterFactory, authorFactory,
 				connectionManager, contactManager, clock, group, plugin,
 				localAuthor, random);
 	}

briar-core/src/org/briarproject/invitation/BobConnector.java

@@ -15,9 +15,7 @@ import org.briarproject.api.identity.LocalAuthor;
 import org.briarproject.api.plugins.ConnectionManager;
 import org.briarproject.api.plugins.duplex.DuplexPlugin;
 import org.briarproject.api.plugins.duplex.DuplexTransportConnection;
-import org.briarproject.api.sync.GroupFactory;
 import org.briarproject.api.system.Clock;
-import org.briarproject.api.transport.KeyManager;
 import org.briarproject.api.transport.StreamReaderFactory;
 import org.briarproject.api.transport.StreamWriterFactory;
 
@@ -41,12 +39,11 @@ class BobConnector extends Connector {
 			BdfWriterFactory bdfWriterFactory,
 			StreamReaderFactory streamReaderFactory,
 			StreamWriterFactory streamWriterFactory,
-			AuthorFactory authorFactory, GroupFactory groupFactory,
-			KeyManager keyManager, ConnectionManager connectionManager,
+			AuthorFactory authorFactory, ConnectionManager connectionManager,
 			ContactManager contactManager, Clock clock, ConnectorGroup group,
 			DuplexPlugin plugin, LocalAuthor localAuthor, PseudoRandom random) {
 		super(crypto, bdfReaderFactory, bdfWriterFactory, streamReaderFactory,
-				streamWriterFactory, authorFactory, groupFactory, keyManager,
+				streamWriterFactory, authorFactory,
 				connectionManager, contactManager, clock, group, plugin,
 				localAuthor, random);
 	}

briar-core/src/org/briarproject/invitation/Connector.java

@@ -22,9 +22,7 @@ import org.briarproject.api.identity.LocalAuthor;
 import org.briarproject.api.plugins.ConnectionManager;
 import org.briarproject.api.plugins.duplex.DuplexPlugin;
 import org.briarproject.api.plugins.duplex.DuplexTransportConnection;
-import org.briarproject.api.sync.GroupFactory;
 import org.briarproject.api.system.Clock;
-import org.briarproject.api.transport.KeyManager;
 import org.briarproject.api.transport.StreamReaderFactory;
 import org.briarproject.api.transport.StreamWriterFactory;
 
@@ -52,8 +50,6 @@ abstract class Connector extends Thread {
 	protected final StreamReaderFactory streamReaderFactory;
 	protected final StreamWriterFactory streamWriterFactory;
 	protected final AuthorFactory authorFactory;
-	protected final GroupFactory groupFactory;
-	protected final KeyManager keyManager;
 	protected final ConnectionManager connectionManager;
 	protected final ContactManager contactManager;
 	protected final Clock clock;
@@ -74,8 +70,7 @@ abstract class Connector extends Thread {
 			BdfWriterFactory bdfWriterFactory,
 			StreamReaderFactory streamReaderFactory,
 			StreamWriterFactory streamWriterFactory,
-			AuthorFactory authorFactory, GroupFactory groupFactory,
-			KeyManager keyManager, ConnectionManager connectionManager,
+			AuthorFactory authorFactory, ConnectionManager connectionManager,
 			ContactManager contactManager, Clock clock, ConnectorGroup group,
 			DuplexPlugin plugin, LocalAuthor localAuthor, PseudoRandom random) {
 		super("Connector");
@@ -85,8 +80,6 @@ abstract class Connector extends Thread {
 		this.streamReaderFactory = streamReaderFactory;
 		this.streamWriterFactory = streamWriterFactory;
 		this.authorFactory = authorFactory;
-		this.groupFactory = groupFactory;
-		this.keyManager = keyManager;
 		this.connectionManager = connectionManager;
 		this.contactManager = contactManager;
 		this.clock = clock;
@@ -219,9 +212,7 @@ abstract class Connector extends Thread {
 			long timestamp, boolean alice) throws DbException {
 		// Add the contact to the database
 		contactId = contactManager.addContact(remoteAuthor,
-				localAuthor.getId(), true);
-		// Derive transport keys
-		keyManager.addContact(contactId, master, timestamp, alice);
+				localAuthor.getId(), master, timestamp, alice, true);
 		return contactId;
 	}
 

briar-core/src/org/briarproject/invitation/ConnectorGroup.java

@@ -17,9 +17,7 @@ import org.briarproject.api.invitation.InvitationTask;
 import org.briarproject.api.plugins.ConnectionManager;
 import org.briarproject.api.plugins.PluginManager;
 import org.briarproject.api.plugins.duplex.DuplexPlugin;
-import org.briarproject.api.sync.GroupFactory;
 import org.briarproject.api.system.Clock;
-import org.briarproject.api.transport.KeyManager;
 import org.briarproject.api.transport.StreamReaderFactory;
 import org.briarproject.api.transport.StreamWriterFactory;
 
@@ -48,8 +46,6 @@ class ConnectorGroup extends Thread implements InvitationTask {
 	private final StreamReaderFactory streamReaderFactory;
 	private final StreamWriterFactory streamWriterFactory;
 	private final AuthorFactory authorFactory;
-	private final GroupFactory groupFactory;
-	private final KeyManager keyManager;
 	private final ConnectionManager connectionManager;
 	private final IdentityManager identityManager;
 	private final ContactManager contactManager;
@@ -74,8 +70,7 @@ class ConnectorGroup extends Thread implements InvitationTask {
 			BdfWriterFactory bdfWriterFactory,
 			StreamReaderFactory streamReaderFactory,
 			StreamWriterFactory streamWriterFactory,
-			AuthorFactory authorFactory, GroupFactory groupFactory,
-			KeyManager keyManager, ConnectionManager connectionManager,
+			AuthorFactory authorFactory, ConnectionManager connectionManager,
 			IdentityManager identityManager, ContactManager contactManager,
 			Clock clock, PluginManager pluginManager, AuthorId localAuthorId,
 			int localInvitationCode, int remoteInvitationCode) {
@@ -86,8 +81,6 @@ class ConnectorGroup extends Thread implements InvitationTask {
 		this.streamReaderFactory = streamReaderFactory;
 		this.streamWriterFactory = streamWriterFactory;
 		this.authorFactory = authorFactory;
-		this.groupFactory = groupFactory;
-		this.keyManager = keyManager;
 		this.connectionManager = connectionManager;
 		this.identityManager = identityManager;
 		this.contactManager = contactManager;
@@ -181,8 +174,8 @@ class ConnectorGroup extends Thread implements InvitationTask {
 				remoteInvitationCode);
 		return new AliceConnector(crypto, bdfReaderFactory, bdfWriterFactory,
 				streamReaderFactory, streamWriterFactory, authorFactory,
-				groupFactory, keyManager, connectionManager, contactManager,
-				clock, this, plugin, localAuthor, random);
+				connectionManager, contactManager, clock, this, plugin,
+				localAuthor, random);
 	}
 
 	private Connector createBobConnector(DuplexPlugin plugin,
@@ -191,8 +184,8 @@ class ConnectorGroup extends Thread implements InvitationTask {
 				localInvitationCode);
 		return new BobConnector(crypto, bdfReaderFactory, bdfWriterFactory,
 				streamReaderFactory, streamWriterFactory, authorFactory,
-				groupFactory, keyManager, connectionManager, contactManager,
-				clock, this, plugin, localAuthor, random);
+				connectionManager, contactManager, clock, this, plugin,
+				localAuthor, random);
 	}
 
 	public void localConfirmationSucceeded() {

briar-core/src/org/briarproject/invitation/InvitationTaskFactoryImpl.java

@@ -11,9 +11,7 @@ import org.briarproject.api.invitation.InvitationTask;
 import org.briarproject.api.invitation.InvitationTaskFactory;
 import org.briarproject.api.plugins.ConnectionManager;
 import org.briarproject.api.plugins.PluginManager;
-import org.briarproject.api.sync.GroupFactory;
 import org.briarproject.api.system.Clock;
-import org.briarproject.api.transport.KeyManager;
 import org.briarproject.api.transport.StreamReaderFactory;
 import org.briarproject.api.transport.StreamWriterFactory;
 
@@ -27,8 +25,6 @@ class InvitationTaskFactoryImpl implements InvitationTaskFactory {
 	private final StreamReaderFactory streamReaderFactory;
 	private final StreamWriterFactory streamWriterFactory;
 	private final AuthorFactory authorFactory;
-	private final GroupFactory groupFactory;
-	private final KeyManager keyManager;
 	private final ConnectionManager connectionManager;
 	private final IdentityManager identityManager;
 	private final ContactManager contactManager;
@@ -40,8 +36,7 @@ class InvitationTaskFactoryImpl implements InvitationTaskFactory {
 			BdfReaderFactory bdfReaderFactory, BdfWriterFactory bdfWriterFactory,
 			StreamReaderFactory streamReaderFactory,
 			StreamWriterFactory streamWriterFactory,
-			AuthorFactory authorFactory, GroupFactory groupFactory,
-			KeyManager keyManager, ConnectionManager connectionManager,
+			AuthorFactory authorFactory, ConnectionManager connectionManager,
 			IdentityManager identityManager, ContactManager contactManager,
 			Clock clock, PluginManager pluginManager) {
 		this.crypto = crypto;
@@ -50,8 +45,6 @@ class InvitationTaskFactoryImpl implements InvitationTaskFactory {
 		this.streamReaderFactory = streamReaderFactory;
 		this.streamWriterFactory = streamWriterFactory;
 		this.authorFactory = authorFactory;
-		this.groupFactory = groupFactory;
-		this.keyManager = keyManager;
 		this.connectionManager = connectionManager;
 		this.identityManager = identityManager;
 		this.contactManager = contactManager;
@@ -63,8 +56,7 @@ class InvitationTaskFactoryImpl implements InvitationTaskFactory {
 			int remoteCode) {
 		return new ConnectorGroup(crypto, bdfReaderFactory, bdfWriterFactory,
 				streamReaderFactory, streamWriterFactory, authorFactory,
-				groupFactory, keyManager, connectionManager, identityManager,
-				contactManager, clock, pluginManager, localAuthorId, localCode,
-				remoteCode);
+				connectionManager, identityManager, contactManager, clock,
+				pluginManager, localAuthorId, localCode, remoteCode);
 	}
 }

briar-core/src/org/briarproject/transport/KeyManagerImpl.java

@@ -88,10 +88,10 @@ class KeyManagerImpl implements KeyManager, Service, EventListener {
 		return true;
 	}
 
-	public void addContact(ContactId c, SecretKey master, long timestamp,
-			boolean alice) {
+	public void addContact(Transaction txn, ContactId c, SecretKey master,
+			long timestamp, boolean alice) throws DbException {
 		for (TransportKeyManager m : managers.values())
-			m.addContact(c, master, timestamp, alice);
+			m.addContact(txn, c, master, timestamp, alice);
 	}
 
 	public StreamContext getStreamContext(ContactId c, TransportId t) {

briar-core/src/org/briarproject/transport/TransportKeyManager.java

@@ -149,8 +149,8 @@ class TransportKeyManager {
 		timer.schedule(task, delay);
 	}
 
-	void addContact(ContactId c, SecretKey master, long timestamp,
-			boolean alice) {
+	void addContact(Transaction txn, ContactId c, SecretKey master,
+			long timestamp, boolean alice) throws DbException {
 		lock.lock();
 		try {
 			// Work out what rotation period the timestamp belongs to
@@ -164,15 +164,7 @@ class TransportKeyManager {
 			// Initialise mutable state for the contact
 			addKeys(c, new MutableTransportKeys(k));
 			// Write the keys back to the DB
-			Transaction txn = db.startTransaction();
-			try {
-				db.addTransportKeys(txn, c, k);
-				txn.setComplete();
-			} finally {
-				db.endTransaction(txn);
-			}
-		} catch (DbException e) {
-			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+			db.addTransportKeys(txn, c, k);
 		} finally {
 			lock.unlock();
 		}