Move responsibility for pre-sharing blogs to sharing manager

to have all the code related to that in one place,
so it is easier to maintain and to spot bugs.

This also checks that only blogs without an existing sharing session
are shared and initialized again.
It extends an existing test to catch the missing check.

This removes some debugging information from the previous commit
to not leak private information via the sharing sessions.

Fixes #979

briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java

@@ -48,9 +48,7 @@ import java.util.concurrent.CopyOnWriteArrayList;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
 
-import static org.briarproject.bramble.api.contact.ContactManager.AddContactHook;
 import static org.briarproject.bramble.api.contact.ContactManager.RemoveContactHook;
-import static org.briarproject.bramble.api.sync.Group.Visibility.SHARED;
 import static org.briarproject.briar.api.blog.BlogConstants.KEY_AUTHOR;
 import static org.briarproject.briar.api.blog.BlogConstants.KEY_AUTHOR_ID;
 import static org.briarproject.briar.api.blog.BlogConstants.KEY_AUTHOR_NAME;
@@ -72,7 +70,7 @@ import static org.briarproject.briar.blog.BlogPostValidator.authorToBdfDictionar
 
 @NotNullByDefault
 class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
-		AddContactHook, RemoveContactHook, Client {
+		RemoveContactHook, Client {
 
 	private final IdentityManager identityManager;
 	private final BlogFactory blogFactory;
@@ -96,26 +94,13 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
 		// Create our personal blog if necessary
 		LocalAuthor a = identityManager.getLocalAuthor(txn);
 		Blog b = blogFactory.createBlog(a);
-		db.addGroup(txn, b.getGroup());
+		db.addGroup(txn, b.getGroup());  // does nothing, if group exists
-		// Ensure that we have the personal blogs of all contacts
-		for (Contact c : db.getContacts(txn)) addingContact(txn, c);
-	}
-
-	@Override
-	public void addingContact(Transaction txn, Contact c) throws DbException {
-		// Add the personal blog of the contact and share it with the contact
-		Blog b = blogFactory.createBlog(c.getAuthor());
-		addBlog(txn, b);
-		db.setGroupVisibility(txn, c.getId(), b.getId(), SHARED);
-		// Share our personal blog with the contact
-		LocalAuthor a = identityManager.getLocalAuthor(txn);
-		Blog b2 = blogFactory.createBlog(a);
-		db.setGroupVisibility(txn, c.getId(), b2.getId(), SHARED);
 	}
 
 	@Override
 	public void removingContact(Transaction txn, Contact c) throws DbException {
 		Blog b = blogFactory.createBlog(c.getAuthor());
+		// TODO we might want to reconsider removing b, if otherwise shared
 		if (db.containsGroup(txn, b.getId())) removeBlog(txn, b);
 	}
 

briar-core/src/main/java/org/briarproject/briar/blog/BlogModule.java

@@ -38,7 +38,6 @@ public class BlogModule {
 			ValidationManager validationManager) {
 
 		lifecycleManager.registerClient(blogManager);
-		contactManager.registerAddContactHook(blogManager);
 		contactManager.registerRemoveContactHook(blogManager);
 		validationManager.registerIncomingMessageHook(CLIENT_ID, blogManager);
 		return blogManager;

briar-core/src/main/java/org/briarproject/briar/sharing/BlogSharingManagerImpl.java

@@ -51,15 +51,23 @@ class BlogSharingManagerImpl extends SharingManagerImpl<Blog>
 		return CLIENT_ID;
 	}
 
+	/**
+	 * This is called during each startup for each existing Contact.
+	 */
 	@Override
 	public void addingContact(Transaction txn, Contact c) throws DbException {
+		// creates a group to share with the contact
 		super.addingContact(txn, c);
+
+		// get our blog and that of Contact c
 		LocalAuthor localAuthor = identityManager.getLocalAuthor(txn);
 		Blog ourBlog = blogManager.getPersonalBlog(localAuthor);
 		Blog theirBlog = blogManager.getPersonalBlog(c.getAuthor());
+
+		// pre-share both blogs, if they have not been shared already
 		try {
-			initializeSharedSession(txn, c, ourBlog);
+			preShareShareable(txn, c, ourBlog);
-			initializeSharedSession(txn, c, theirBlog);
+			preShareShareable(txn, c, theirBlog);
 		} catch (FormatException e) {
 			throw new DbException(e);
 		}

briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java

@@ -139,9 +139,24 @@ abstract class SharingManagerImpl<S extends Shareable>
 		return false;
 	}
 
-	void initializeSharedSession(Transaction txn, Contact c, S shareable)
+	/**
+	 * Adds the given Shareable and initializes a session between us
+	 * and the Contact c in state SHARING.
+	 * If a session already exists, this does nothing.
+	 */
+	void preShareShareable(Transaction txn, Contact c, S shareable)
 			throws DbException, FormatException {
+		// return if a session already exists with that Contact
 		GroupId contactGroupId = getContactGroup(c).getId();
+		StoredSession existingSession = getSession(txn, contactGroupId,
+				getSessionId(shareable.getId()));
+		if (existingSession != null) return;
+
+		// add and shares the shareable with the Contact
+		db.addGroup(txn, shareable.getGroup());
+		db.setGroupVisibility(txn, c.getId(), shareable.getId(), SHARED);
+
+		// initialize session in sharing state
 		Session session = new Session(SHARING, contactGroupId,
 				shareable.getId(), null, null, 0, 0);
 		MessageId storageId = createStorageId(txn, contactGroupId);

briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerImplTest.java

@@ -31,15 +31,11 @@ import org.jmock.Expectations;
 import org.jmock.Mockery;
 import org.junit.Test;
 
-import java.util.Collection;
-import java.util.Collections;
-
 import static org.briarproject.bramble.api.identity.Author.Status.NONE;
 import static org.briarproject.bramble.api.identity.Author.Status.OURSELVES;
 import static org.briarproject.bramble.api.identity.Author.Status.VERIFIED;
 import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH;
 import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH;
-import static org.briarproject.bramble.api.sync.Group.Visibility.SHARED;
 import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_LENGTH;
 import static org.briarproject.bramble.test.TestUtils.getRandomBytes;
 import static org.briarproject.bramble.test.TestUtils.getRandomId;
@@ -117,29 +113,12 @@ public class BlogManagerImplTest extends BriarTestCase {
 	public void testCreateLocalState() throws DbException {
 		final Transaction txn = new Transaction(null, false);
 
-		final ContactId contactId = new ContactId(0);
-
-		Contact contact = new Contact(contactId, blog2.getAuthor(),
-				blog1.getAuthor().getId(), true, true);
-		final Collection<Contact> contacts = Collections.singletonList(contact);
-
 		context.checking(new Expectations() {{
 			oneOf(identityManager).getLocalAuthor(txn);
 			will(returnValue(blog1.getAuthor()));
 			oneOf(blogFactory).createBlog(blog1.getAuthor());
 			will(returnValue(blog1));
 			oneOf(db).addGroup(txn, blog1.getGroup());
-			oneOf(db).getContacts(txn);
-			will(returnValue(contacts));
-			oneOf(blogFactory).createBlog(blog2.getAuthor());
-			will(returnValue(blog2));
-			oneOf(db).addGroup(txn, blog2.getGroup());
-			oneOf(db).setGroupVisibility(txn, contactId, blog2.getId(), SHARED);
-			oneOf(identityManager).getLocalAuthor(txn);
-			will(returnValue(blog1.getAuthor()));
-			oneOf(blogFactory).createBlog(blog1.getAuthor());
-			will(returnValue(blog1));
-			oneOf(db).setGroupVisibility(txn, contactId, blog1.getId(), SHARED);
 		}});
 
 		blogManager.createLocalState(txn);

briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingIntegrationTest.java

@@ -5,6 +5,7 @@ import net.jodah.concurrentunit.Waiter;
 import org.briarproject.bramble.api.contact.Contact;
 import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.db.NoSuchGroupException;
+import org.briarproject.bramble.api.db.Transaction;
 import org.briarproject.bramble.api.event.Event;
 import org.briarproject.bramble.api.event.EventListener;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
@@ -492,6 +493,26 @@ public class BlogSharingIntegrationTest
 		// 1 can again share blog 1 with 0
 		assertTrue(
 				blogSharingManager1.canBeShared(blog1.getId(), contact0From1));
+
+		// re-create local state (simulates sign-out and sign-in)
+		Transaction txn0 = db0.startTransaction(false);
+		((BlogSharingManagerImpl) blogSharingManager0).createLocalState(txn0);
+		db0.commitTransaction(txn0);
+		db0.endTransaction(txn0);
+		Transaction txn1 = db1.startTransaction(false);
+		((BlogSharingManagerImpl) blogSharingManager1).createLocalState(txn1);
+		db1.commitTransaction(txn1);
+		db1.endTransaction(txn1);
+
+		// ensure there's no error with the sessions by checking shareability
+		assertFalse(
+				blogSharingManager0.canBeShared(blog1.getId(), contact1From0));
+		assertTrue(
+				blogSharingManager1.canBeShared(blog1.getId(), contact0From1));
+
+		// contacts should still be able to remove each other without errors
+		contactManager0.removeContact(contactId1From0);
+		contactManager1.removeContact(contactId0From1);
 	}
 
 	@Test