Merge branch '375-extract-forumfactory-from-forummanager' into 'master'

Extract ForumFactory from ForumManager

The code for creating forums in ForumManager was used by
ForumSharingManager and also needed by InviteeEngine.
This extracts it into its own class.

Closes #375

See merge request !195

briar-android-tests/src/test/java/org/briarproject/ForumSharingIntegrationTest.java

@@ -647,8 +647,6 @@ public class ForumSharingIntegrationTest extends BriarTestCase {
 				eventWaiter.assertEquals(contactId0, event.getContactId());
 				requestReceived = true;
 				Forum f = event.getForum();
-				// work-around because the forum does not contain the group
-				f = forumManager1.createForum(f.getName(), f.getSalt());
 				try {
 					forumSharingManager1.respondToInvitation(f, accept);
 				} catch (DbException ex) {
@@ -719,8 +717,7 @@ public class ForumSharingIntegrationTest extends BriarTestCase {
 
 	private void addForumForSharer() throws DbException {
 		// sharer creates forum
-		forum0 = forumManager0.createForum("Test Forum");
+		forum0 = forumManager0.addForum("Test Forum");
-		forumManager0.addForum(forum0);
 	}
 
 	private void listenToEvents(boolean accept) {

briar-android/src/org/briarproject/android/forum/CreateForumActivity.java

@@ -129,8 +129,7 @@ public class CreateForumActivity extends BriarActivity
 			public void run() {
 				try {
 					long now = System.currentTimeMillis();
-					Forum f = forumManager.createForum(name);
+					Forum f = forumManager.addForum(name);
-					forumManager.addForum(f);
 					long duration = System.currentTimeMillis() - now;
 					if (LOG.isLoggable(INFO))
 						LOG.info("Storing forum took " + duration + " ms");

briar-api/src/org/briarproject/api/forum/ForumFactory.java

@@ -0,0 +1,11 @@
+package org.briarproject.api.forum;
+
+public interface ForumFactory {
+
+	/** Creates a forum with the given name. */
+	Forum createForum(String name);
+
+	/** Creates a forum with the given name and salt. */
+	Forum createForum(String name, byte[] salt);
+
+}

briar-api/src/org/briarproject/api/forum/ForumManager.java

@@ -13,14 +13,8 @@ public interface ForumManager {
 	/** Returns the unique ID of the forum client. */
 	ClientId getClientId();
 
-	/** Creates a forum with the given name. */
-	Forum createForum(String name);
-
-	/** Creates a forum with the given name and salt. */
-	Forum createForum(String name, byte[] salt);
-
 	/** Subscribes to a forum. */
-	void addForum(Forum f) throws DbException;
+	Forum addForum(String name) throws DbException;
 
 	/** Unsubscribes from a forum. */
 	void removeForum(Forum f) throws DbException;

briar-core/src/org/briarproject/forum/ForumFactoryImpl.java

@@ -0,0 +1,58 @@
+package org.briarproject.forum;
+
+import org.briarproject.api.FormatException;
+import org.briarproject.api.clients.ClientHelper;
+import org.briarproject.api.data.BdfList;
+import org.briarproject.api.forum.Forum;
+import org.briarproject.api.forum.ForumFactory;
+import org.briarproject.api.sync.Group;
+import org.briarproject.api.sync.GroupFactory;
+import org.briarproject.util.StringUtils;
+
+import java.security.SecureRandom;
+
+import javax.inject.Inject;
+
+import static org.briarproject.api.forum.ForumConstants.FORUM_SALT_LENGTH;
+import static org.briarproject.api.forum.ForumConstants.MAX_FORUM_NAME_LENGTH;
+
+class ForumFactoryImpl implements ForumFactory {
+
+	private final GroupFactory groupFactory;
+	private final ClientHelper clientHelper;
+	private final SecureRandom random;
+
+	@Inject
+	ForumFactoryImpl(GroupFactory groupFactory, ClientHelper clientHelper,
+			SecureRandom random) {
+
+		this.groupFactory = groupFactory;
+		this.clientHelper = clientHelper;
+		this.random = random;
+	}
+
+	@Override
+	public Forum createForum(String name) {
+		int length = StringUtils.toUtf8(name).length;
+		if (length == 0) throw new IllegalArgumentException();
+		if (length > MAX_FORUM_NAME_LENGTH)
+			throw new IllegalArgumentException();
+		byte[] salt = new byte[FORUM_SALT_LENGTH];
+		random.nextBytes(salt);
+		return createForum(name, salt);
+	}
+
+	@Override
+	public Forum createForum(String name, byte[] salt) {
+		try {
+			BdfList forum = BdfList.of(name, salt);
+			byte[] descriptor = clientHelper.toByteArray(forum);
+			Group g = groupFactory
+					.createGroup(ForumManagerImpl.CLIENT_ID, descriptor);
+			return new Forum(g, name, salt);
+		} catch (FormatException e) {
+			throw new RuntimeException(e);
+		}
+	}
+
+}

briar-core/src/org/briarproject/forum/ForumManagerImpl.java

@@ -9,6 +9,7 @@ import org.briarproject.api.db.DatabaseComponent;
 import org.briarproject.api.db.DbException;
 import org.briarproject.api.db.Transaction;
 import org.briarproject.api.forum.Forum;
+import org.briarproject.api.forum.ForumFactory;
 import org.briarproject.api.forum.ForumManager;
 import org.briarproject.api.forum.ForumPost;
 import org.briarproject.api.forum.ForumPostHeader;
@@ -17,12 +18,10 @@ import org.briarproject.api.identity.AuthorId;
 import org.briarproject.api.identity.LocalAuthor;
 import org.briarproject.api.sync.ClientId;
 import org.briarproject.api.sync.Group;
-import org.briarproject.api.sync.GroupFactory;
 import org.briarproject.api.sync.GroupId;
 import org.briarproject.api.sync.MessageId;
 import org.briarproject.util.StringUtils;
 
-import java.security.SecureRandom;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -35,8 +34,6 @@ import java.util.concurrent.CopyOnWriteArrayList;
 
 import javax.inject.Inject;
 
-import static org.briarproject.api.forum.ForumConstants.FORUM_SALT_LENGTH;
-import static org.briarproject.api.forum.ForumConstants.MAX_FORUM_NAME_LENGTH;
 import static org.briarproject.api.identity.Author.Status.ANONYMOUS;
 import static org.briarproject.api.identity.Author.Status.UNKNOWN;
 import static org.briarproject.api.identity.Author.Status.VERIFIED;
@@ -49,18 +46,16 @@ class ForumManagerImpl implements ForumManager {
 
 	private final DatabaseComponent db;
 	private final ClientHelper clientHelper;
-	private final GroupFactory groupFactory;
+	private final ForumFactory forumFactory;
-	private final SecureRandom random;
 	private final List<RemoveForumHook> removeHooks;
 
 	@Inject
 	ForumManagerImpl(DatabaseComponent db, ClientHelper clientHelper,
-			GroupFactory groupFactory, SecureRandom random) {
+			ForumFactory forumFactory) {
 
 		this.db = db;
 		this.clientHelper = clientHelper;
-		this.groupFactory = groupFactory;
+		this.forumFactory = forumFactory;
-		this.random = random;
 		removeHooks = new CopyOnWriteArrayList<RemoveForumHook>();
 	}
 
@@ -70,30 +65,9 @@ class ForumManagerImpl implements ForumManager {
 	}
 
 	@Override
-	public Forum createForum(String name) {
+	public Forum addForum(String name) throws DbException {
-		int length = StringUtils.toUtf8(name).length;
+		Forum f = forumFactory.createForum(name);
-		if (length == 0) throw new IllegalArgumentException();
-		if (length > MAX_FORUM_NAME_LENGTH)
-			throw new IllegalArgumentException();
-		byte[] salt = new byte[FORUM_SALT_LENGTH];
-		random.nextBytes(salt);
-		return createForum(name, salt);
-	}
 
-	@Override
-	public Forum createForum(String name, byte[] salt) {
-		try {
-			BdfList forum = BdfList.of(name, salt);
-			byte[] descriptor = clientHelper.toByteArray(forum);
-			Group g = groupFactory.createGroup(getClientId(), descriptor);
-			return new Forum(g, name, salt);
-		} catch (FormatException e) {
-			throw new RuntimeException(e);
-		}
-	}
-
-	@Override
-	public void addForum(Forum f) throws DbException {
 		Transaction txn = db.startTransaction(false);
 		try {
 			db.addGroup(txn, f.getGroup());
@@ -101,6 +75,7 @@ class ForumManagerImpl implements ForumManager {
 		} finally {
 			db.endTransaction(txn);
 		}
+		return f;
 	}
 
 	@Override

briar-core/src/org/briarproject/forum/ForumModule.java

@@ -6,6 +6,7 @@ import org.briarproject.api.contact.ContactManager;
 import org.briarproject.api.crypto.CryptoComponent;
 import org.briarproject.api.data.MetadataEncoder;
 import org.briarproject.api.db.DatabaseComponent;
+import org.briarproject.api.forum.ForumFactory;
 import org.briarproject.api.forum.ForumManager;
 import org.briarproject.api.forum.ForumPostFactory;
 import org.briarproject.api.forum.ForumSharingManager;
@@ -38,9 +39,8 @@ public class ForumModule {
 	@Provides
 	@Singleton
 	ForumManager provideForumManager(DatabaseComponent db,
-			ClientHelper clientHelper,
+			ClientHelper clientHelper, ForumFactory forumFactory) {
-			GroupFactory groupFactory, SecureRandom random) {
+		return new ForumManagerImpl(db, clientHelper, forumFactory);
-		return new ForumManagerImpl(db, clientHelper, groupFactory, random);
 	}
 
 	@Provides
@@ -49,6 +49,12 @@ public class ForumModule {
 		return new ForumPostFactoryImpl(crypto, clientHelper);
 	}
 
+	@Provides
+	ForumFactory provideForumFactory(GroupFactory groupFactory,
+			ClientHelper clientHelper, SecureRandom random) {
+		return new ForumFactoryImpl(groupFactory, clientHelper, random);
+	}
+
 	@Provides
 	@Singleton
 	ForumPostValidator provideForumPostValidator(
@@ -82,7 +88,7 @@ public class ForumModule {
 			LifecycleManager lifecycleManager,
 			ContactManager contactManager,
 			MessageQueueManager messageQueueManager,
-			ForumManager forumManager,
+			ForumManager forumManager, ForumFactory forumFactory,
 			ForumSharingManagerImpl forumSharingManager) {
 
 		lifecycleManager.registerClient(forumSharingManager);

briar-core/src/org/briarproject/forum/ForumSharingManagerImpl.java

@@ -23,6 +23,7 @@ import org.briarproject.api.db.NoSuchMessageException;
 import org.briarproject.api.db.Transaction;
 import org.briarproject.api.event.Event;
 import org.briarproject.api.forum.Forum;
+import org.briarproject.api.forum.ForumFactory;
 import org.briarproject.api.forum.ForumInvitationMessage;
 import org.briarproject.api.forum.ForumManager;
 import org.briarproject.api.forum.ForumSharingManager;
@@ -111,6 +112,7 @@ class ForumSharingManagerImpl extends BdfIncomingMessageHook
 	private final MetadataEncoder metadataEncoder;
 	private final SecureRandom random;
 	private final PrivateGroupFactory privateGroupFactory;
+	private final ForumFactory forumFactory;
 	private final Clock clock;
 	private final Group localGroup;
 
@@ -119,7 +121,7 @@ class ForumSharingManagerImpl extends BdfIncomingMessageHook
 			MessageQueueManager messageQueueManager, ClientHelper clientHelper,
 			MetadataParser metadataParser, MetadataEncoder metadataEncoder,
 			SecureRandom random, PrivateGroupFactory privateGroupFactory,
-			Clock clock) {
+			ForumFactory forumFactory, Clock clock) {
 
 		super(clientHelper, metadataParser);
 		this.db = db;
@@ -128,6 +130,7 @@ class ForumSharingManagerImpl extends BdfIncomingMessageHook
 		this.metadataEncoder = metadataEncoder;
 		this.random = random;
 		this.privateGroupFactory = privateGroupFactory;
+		this.forumFactory = forumFactory;
 		this.clock = clock;
 		localGroup = privateGroupFactory.createLocalGroup(getClientId());
 	}
@@ -203,7 +206,7 @@ class ForumSharingManagerImpl extends BdfIncomingMessageHook
 				if (stateExists) throw new FormatException();
 
 				// check if forum can be shared
-				Forum f = forumManager.createForum(msg.getString(FORUM_NAME),
+				Forum f = forumFactory.createForum(msg.getString(FORUM_NAME),
 						msg.getRaw(FORUM_SALT));
 				ContactId contactId = getContactId(txn, m.getGroupId());
 				Contact contact = db.getContact(txn, contactId);
@@ -213,7 +216,7 @@ class ForumSharingManagerImpl extends BdfIncomingMessageHook
 				// initialize state and process invitation
 				BdfDictionary state =
 						initializeInviteeState(txn, contactId, msg);
-				InviteeEngine engine = new InviteeEngine();
+				InviteeEngine engine = new InviteeEngine(forumFactory);
 				processStateUpdate(txn, m.getId(),
 						engine.onMessageReceived(state, msg));
 			} catch (FormatException e) {
@@ -238,7 +241,7 @@ class ForumSharingManagerImpl extends BdfIncomingMessageHook
 						engine.onMessageReceived(state, msg));
 			} else {
 				// we are an invitee and the sharer wants to leave or abort
-				InviteeEngine engine = new InviteeEngine();
+				InviteeEngine engine = new InviteeEngine(forumFactory);
 				processStateUpdate(txn, m.getId(),
 						engine.onMessageReceived(state, msg));
 			}
@@ -301,7 +304,7 @@ class ForumSharingManagerImpl extends BdfIncomingMessageHook
 			}
 
 			// start engine and process its state update
-			InviteeEngine engine = new InviteeEngine();
+			InviteeEngine engine = new InviteeEngine(forumFactory);
 			processStateUpdate(txn, null,
 					engine.onLocalAction(localState, localAction));
 
@@ -526,7 +529,7 @@ class ForumSharingManagerImpl extends BdfIncomingMessageHook
 		Group group = getContactGroup(c);
 		String name = msg.getString(FORUM_NAME);
 		byte[] salt = msg.getRaw(FORUM_SALT);
-		Forum f = forumManager.createForum(name, salt);
+		Forum f = forumFactory.createForum(name, salt);
 
 		// create local message to keep engine state
 		long now = clock.currentTimeMillis();
@@ -692,7 +695,7 @@ class ForumSharingManagerImpl extends BdfIncomingMessageHook
 		// get forum for later
 		String name = localState.getString(FORUM_NAME);
 		byte[] salt = localState.getRaw(FORUM_SALT);
-		Forum f = forumManager.createForum(name, salt);
+		Forum f = forumFactory.createForum(name, salt);
 
 		// perform tasks
 		if (task == TASK_ADD_FORUM_TO_LIST_SHARED_WITH_US) {
@@ -791,7 +794,7 @@ class ForumSharingManagerImpl extends BdfIncomingMessageHook
 			processStateUpdate(txn, null,
 					engine.onLocalAction(state, action));
 		} else {
-			InviteeEngine engine = new InviteeEngine();
+			InviteeEngine engine = new InviteeEngine(forumFactory);
 			processStateUpdate(txn, null,
 					engine.onLocalAction(state, action));
 		}
@@ -859,7 +862,7 @@ class ForumSharingManagerImpl extends BdfIncomingMessageHook
 		List<Forum> forums = new ArrayList<Forum>(list.size());
 		for (int i = 0; i < list.size(); i++) {
 			BdfList forum = list.getList(i);
-			forums.add(forumManager
+			forums.add(forumFactory
 					.createForum(forum.getString(0), forum.getRaw(1)));
 		}
 		return forums;

briar-core/src/org/briarproject/forum/InviteeEngine.java

@@ -8,6 +8,7 @@ import org.briarproject.api.data.BdfEntry;
 import org.briarproject.api.event.Event;
 import org.briarproject.api.event.ForumInvitationReceivedEvent;
 import org.briarproject.api.forum.Forum;
+import org.briarproject.api.forum.ForumFactory;
 import org.briarproject.api.forum.InviteeAction;
 import org.briarproject.api.forum.InviteeProtocolState;
 
@@ -48,9 +49,14 @@ import static org.briarproject.api.forum.InviteeProtocolState.LEFT;
 public class InviteeEngine
 		implements ProtocolEngine<BdfDictionary, BdfDictionary, BdfDictionary> {
 
+	private final ForumFactory forumFactory;
 	private static final Logger LOG =
 			Logger.getLogger(SharerEngine.class.getName());
 
+	InviteeEngine(ForumFactory forumFactory) {
+		this.forumFactory = forumFactory;
+	}
+
 	@Override
 	public StateUpdate<BdfDictionary, BdfDictionary> onLocalAction(
 			BdfDictionary localState, BdfDictionary localAction) {
@@ -157,9 +163,9 @@ public class InviteeEngine
 			// we have just received our invitation
 			else if (action == REMOTE_INVITATION) {
 				localState.put(TASK, TASK_ADD_FORUM_TO_LIST_SHARED_WITH_US);
-				// TODO how to get the proper group here?
+				Forum forum = forumFactory
-				Forum forum = new Forum(null, localState.getString(FORUM_NAME),
+						.createForum(localState.getString(FORUM_NAME),
-						localState.getRaw(FORUM_SALT));
+								localState.getRaw(FORUM_SALT));
 				ContactId contactId = new ContactId(
 						localState.getLong(CONTACT_ID).intValue());
 				Event event = new ForumInvitationReceivedEvent(forum, contactId);