Allow to remove pre-shared blogs of our contacts

briar-android/src/main/java/org/briarproject/briar/android/blog/BlogControllerImpl.java

@@ -169,7 +169,7 @@ class BlogControllerImpl extends BaseControllerImpl
 					LocalAuthor a = identityManager.getLocalAuthor();
 					Blog b = blogManager.getBlog(groupId);
 					boolean ours = a.getId().equals(b.getAuthor().getId());
-					boolean removable = blogManager.canBeRemoved(groupId);
+					boolean removable = blogManager.canBeRemoved(b);
 					BlogItem blog = new BlogItem(b, ours, removable);
 					long duration = System.currentTimeMillis() - now;
 					if (LOG.isLoggable(INFO))

briar-api/src/main/java/org/briarproject/briar/api/blog/BlogManager.java

@@ -34,7 +34,7 @@ public interface BlogManager {
 	/**
 	 * Returns true if a blog can be removed.
 	 */
-	boolean canBeRemoved(GroupId g) throws DbException;
+	boolean canBeRemoved(Blog b) throws DbException;
 
 	/**
 	 * Removes and deletes a blog.

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

@@ -3,7 +3,6 @@ package org.briarproject.briar.blog;
 import org.briarproject.bramble.api.FormatException;
 import org.briarproject.bramble.api.client.ClientHelper;
 import org.briarproject.bramble.api.contact.Contact;
-import org.briarproject.bramble.api.contact.ContactManager;
 import org.briarproject.bramble.api.data.BdfDictionary;
 import org.briarproject.bramble.api.data.BdfEntry;
 import org.briarproject.bramble.api.data.BdfList;
@@ -76,7 +75,6 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
 		AddContactHook, RemoveContactHook, Client {
 
 	private final IdentityManager identityManager;
-	private final ContactManager contactManager;
 	private final BlogFactory blogFactory;
 	private final BlogPostFactory blogPostFactory;
 	private final List<RemoveBlogHook> removeHooks;
@@ -84,12 +82,10 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
 	@Inject
 	BlogManagerImpl(DatabaseComponent db, IdentityManager identityManager,
 			ClientHelper clientHelper, MetadataParser metadataParser,
-			ContactManager contactManager, BlogFactory blogFactory,
+			BlogFactory blogFactory, BlogPostFactory blogPostFactory) {
-			BlogPostFactory blogPostFactory) {
 		super(db, clientHelper, metadataParser);
 
 		this.identityManager = identityManager;
-		this.contactManager = contactManager;
 		this.blogFactory = blogFactory;
 		this.blogPostFactory = blogPostFactory;
 		removeHooks = new CopyOnWriteArrayList<RemoveBlogHook>();
@@ -120,7 +116,7 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
 	@Override
 	public void removingContact(Transaction txn, Contact c) throws DbException {
 		Blog b = blogFactory.createBlog(c.getAuthor());
-		removeBlog(txn, b, true);
+		removeBlog(txn, b);
 	}
 
 	@Override
@@ -191,10 +187,10 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
 	}
 
 	@Override
-	public boolean canBeRemoved(GroupId g) throws DbException {
+	public boolean canBeRemoved(Blog b) throws DbException {
 		Transaction txn = db.startTransaction(true);
 		try {
-			boolean canBeRemoved = canBeRemoved(txn, g);
+			boolean canBeRemoved = canBeRemoved(txn, b);
 			db.commitTransaction(txn);
 			return canBeRemoved;
 		} finally {
@@ -202,23 +198,18 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
 		}
 	}
 
-	private boolean canBeRemoved(Transaction txn, GroupId g)
+	private boolean canBeRemoved(Transaction txn, Blog b)
 			throws DbException {
-		boolean canBeRemoved;
-		Blog b = getBlog(txn, g);
 		AuthorId authorId = b.getAuthor().getId();
 		LocalAuthor localAuthor = identityManager.getLocalAuthor(txn);
-		if (localAuthor.getId().equals(authorId)) return false;
+		return !localAuthor.getId().equals(authorId);
-		canBeRemoved = !contactManager
-				.contactExists(txn, authorId, localAuthor.getId());
-		return canBeRemoved;
 	}
 
 	@Override
 	public void removeBlog(Blog b) throws DbException {
 		Transaction txn = db.startTransaction(false);
 		try {
-			removeBlog(txn, b, false);
+			removeBlog(txn, b);
 			db.commitTransaction(txn);
 		} finally {
 			db.endTransaction(txn);
@@ -227,12 +218,7 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
 
 	@Override
 	public void removeBlog(Transaction txn, Blog b) throws DbException {
-		removeBlog(txn, b, false);
+		if (!canBeRemoved(txn, b))
-	}
-
-	private void removeBlog(Transaction txn, Blog b, boolean forced)
-			throws DbException {
-		if (!forced && !canBeRemoved(txn, b.getId()))
 			throw new IllegalArgumentException();
 		for (RemoveBlogHook hook : removeHooks)
 			hook.removingBlog(txn, b);

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

@@ -1,5 +1,6 @@
 package org.briarproject.briar.sharing;
 
+import org.briarproject.bramble.api.FormatException;
 import org.briarproject.bramble.api.client.ClientHelper;
 import org.briarproject.bramble.api.client.ContactGroupFactory;
 import org.briarproject.bramble.api.contact.Contact;
@@ -11,7 +12,6 @@ import org.briarproject.bramble.api.identity.IdentityManager;
 import org.briarproject.bramble.api.identity.LocalAuthor;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.sync.ClientId;
-import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.briar.api.blog.Blog;
 import org.briarproject.briar.api.blog.BlogInvitationResponse;
 import org.briarproject.briar.api.blog.BlogManager;
@@ -52,18 +52,17 @@ class BlogSharingManagerImpl extends SharingManagerImpl<Blog>
 	}
 
 	@Override
-	protected boolean canBeShared(Transaction txn, GroupId shareableId,
+	public void addingContact(Transaction txn, Contact c) throws DbException {
-			Contact c) throws DbException {
+		super.addingContact(txn, c);
-		// check if shareableId belongs to our personal blog
+		LocalAuthor localAuthor = identityManager.getLocalAuthor(txn);
-		LocalAuthor author = identityManager.getLocalAuthor(txn);
+		Blog ourBlog = blogManager.getPersonalBlog(localAuthor);
-		Blog b = blogManager.getPersonalBlog(author);
+		Blog theirBlog = blogManager.getPersonalBlog(c.getAuthor());
-		if (b.getId().equals(shareableId)) return false;
+		try {
-
+			initializeSharedSession(txn, c, ourBlog);
-		// check if shareableId belongs to c's personal blog
+			initializeSharedSession(txn, c, theirBlog);
-		b = blogManager.getPersonalBlog(c.getAuthor());
+		} catch (FormatException e) {
-		if (b.getId().equals(shareableId)) return false;
+			throw new DbException(e);
-
+		}
-		return super.canBeShared(txn, shareableId, c);
 	}
 
 	@Override

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

@@ -48,6 +48,7 @@ import static org.briarproject.briar.sharing.MessageType.DECLINE;
 import static org.briarproject.briar.sharing.MessageType.INVITE;
 import static org.briarproject.briar.sharing.MessageType.LEAVE;
 import static org.briarproject.briar.sharing.SharingConstants.GROUP_KEY_CONTACT_ID;
+import static org.briarproject.briar.sharing.State.SHARING;
 
 @NotNullByDefault
 abstract class SharingManagerImpl<S extends Shareable>
@@ -138,6 +139,16 @@ abstract class SharingManagerImpl<S extends Shareable>
 		return false;
 	}
 
+	protected void initializeSharedSession(Transaction txn, Contact c,
+			S shareable) throws DbException, FormatException {
+		GroupId contactGroupId = getContactGroup(c).getId();
+		Session session =
+				new Session(SHARING, contactGroupId, shareable.getId(), null,
+						null, 0, 0);
+		MessageId storageId = createStorageId(txn, contactGroupId);
+		storeSession(txn, storageId, session);
+	}
+
 	private SessionId getSessionId(GroupId shareableId) {
 		return new SessionId(shareableId.getBytes());
 	}
@@ -414,7 +425,7 @@ abstract class SharingManagerImpl<S extends Shareable>
 		}
 	}
 
-	protected boolean canBeShared(Transaction txn, GroupId g, Contact c)
+	private boolean canBeShared(Transaction txn, GroupId g, Contact c)
 			throws DbException {
 		GroupId contactGroupId = getContactGroup(c).getId();
 		SessionId sessionId = getSessionId(g);

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

@@ -4,7 +4,6 @@ import org.briarproject.bramble.api.FormatException;
 import org.briarproject.bramble.api.client.ClientHelper;
 import org.briarproject.bramble.api.contact.Contact;
 import org.briarproject.bramble.api.contact.ContactId;
-import org.briarproject.bramble.api.contact.ContactManager;
 import org.briarproject.bramble.api.data.BdfDictionary;
 import org.briarproject.bramble.api.data.BdfEntry;
 import org.briarproject.bramble.api.data.BdfList;
@@ -62,8 +61,6 @@ public class BlogManagerImplTest extends BriarTestCase {
 	private final IdentityManager identityManager =
 			context.mock(IdentityManager.class);
 	private final ClientHelper clientHelper = context.mock(ClientHelper.class);
-	private final ContactManager contactManager =
-			context.mock(ContactManager.class);
 	private final BlogFactory blogFactory = context.mock(BlogFactory.class);
 
 	private final Blog blog1, blog2;
@@ -74,7 +71,7 @@ public class BlogManagerImplTest extends BriarTestCase {
 		MetadataParser metadataParser = context.mock(MetadataParser.class);
 		BlogPostFactory blogPostFactory = context.mock(BlogPostFactory.class);
 		blogManager = new BlogManagerImpl(db, identityManager, clientHelper,
-				metadataParser, contactManager, blogFactory, blogPostFactory);
+				metadataParser, blogFactory, blogPostFactory);
 
 		blog1 = createBlog();
 		blog2 = createBlog();
@@ -126,6 +123,8 @@ public class BlogManagerImplTest extends BriarTestCase {
 		context.checking(new Expectations() {{
 			oneOf(blogFactory).createBlog(blog2.getAuthor());
 			will(returnValue(blog2));
+			oneOf(identityManager).getLocalAuthor(txn);
+			will(returnValue(blog1.getAuthor()));
 			oneOf(db).removeGroup(txn, blog2.getGroup());
 		}});
 
@@ -173,13 +172,11 @@ public class BlogManagerImplTest extends BriarTestCase {
 	public void testRemoveBlog() throws Exception {
 		final Transaction txn = new Transaction(null, false);
 
-		checkGetBlogExpectations(txn, false, blog1);
 		context.checking(new Expectations() {{
+			oneOf(db).startTransaction(false);
+			will(returnValue(txn));
 			oneOf(identityManager).getLocalAuthor(txn);
 			will(returnValue(blog2.getAuthor()));
-			oneOf(contactManager).contactExists(txn, blog1.getAuthor().getId(),
-					blog2.getAuthor().getId());
-			will(returnValue(false));
 			oneOf(db).removeGroup(txn, blog1.getGroup());
 			oneOf(db).commitTransaction(txn);
 			oneOf(db).endTransaction(txn);
@@ -240,57 +237,29 @@ public class BlogManagerImplTest extends BriarTestCase {
 	public void testBlogCanBeRemoved() throws Exception {
 		// check that own personal blogs can not be removed
 		final Transaction txn = new Transaction(null, true);
-		checkGetBlogExpectations(txn, true, blog1);
 		context.checking(new Expectations() {{
+			oneOf(db).startTransaction(true);
+			will(returnValue(txn));
 			oneOf(identityManager).getLocalAuthor(txn);
 			will(returnValue(blog1.getAuthor()));
 			oneOf(db).commitTransaction(txn);
 			oneOf(db).endTransaction(txn);
 		}});
-		assertFalse(blogManager.canBeRemoved(blog1.getId()));
+		assertFalse(blogManager.canBeRemoved(blog1));
 		context.assertIsSatisfied();
 
-		// check that blogs of contacts can not be removed
+		// check that blogs of contacts can be removed
 		final Transaction txn2 = new Transaction(null, true);
-		checkGetBlogExpectations(txn2, true, blog1);
 		context.checking(new Expectations() {{
+			oneOf(db).startTransaction(true);
+			will(returnValue(txn2));
 			oneOf(identityManager).getLocalAuthor(txn2);
 			will(returnValue(blog2.getAuthor()));
-			oneOf(contactManager).contactExists(txn2, blog1.getAuthor().getId(),
-					blog2.getAuthor().getId());
-			will(returnValue(true));
 			oneOf(db).commitTransaction(txn2);
 			oneOf(db).endTransaction(txn2);
 		}});
-		assertFalse(blogManager.canBeRemoved(blog1.getId()));
+		assertTrue(blogManager.canBeRemoved(blog1));
 		context.assertIsSatisfied();
-
-		// check that blogs can be removed if they don't belong to a contact
-		final Transaction txn3 = new Transaction(null, true);
-		checkGetBlogExpectations(txn3, true, blog1);
-		context.checking(new Expectations() {{
-			oneOf(identityManager).getLocalAuthor(txn3);
-			will(returnValue(blog2.getAuthor()));
-			oneOf(contactManager).contactExists(txn3, blog1.getAuthor().getId(),
-					blog2.getAuthor().getId());
-			will(returnValue(false));
-			oneOf(db).commitTransaction(txn3);
-			oneOf(db).endTransaction(txn3);
-		}});
-		assertTrue(blogManager.canBeRemoved(blog1.getId()));
-		context.assertIsSatisfied();
-	}
-
-	private void checkGetBlogExpectations(final Transaction txn,
-			final boolean readOnly, final Blog blog) throws Exception {
-		context.checking(new Expectations() {{
-			oneOf(db).startTransaction(readOnly);
-			will(returnValue(txn));
-			oneOf(db).getGroup(txn, blog.getId());
-			will(returnValue(blog.getGroup()));
-			oneOf(blogFactory).parseBlog(blog.getGroup());
-			will(returnValue(blog));
-		}});
 	}
 
 	private Blog createBlog() {

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

@@ -21,7 +21,6 @@ import org.junit.rules.ExpectedException;
 import java.util.Collection;
 import java.util.Iterator;
 
-import static junit.framework.Assert.assertFalse;
 import static junit.framework.Assert.assertNotNull;
 import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH;
 import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH;
@@ -185,19 +184,19 @@ public class BlogManagerIntegrationTest
 	}
 
 	@Test
-	public void testCanNotRemoveContactsPersonalBlog() throws Exception {
+	public void testCanRemoveContactsPersonalBlog() throws Exception {
-		assertFalse(blogManager0.canBeRemoved(blog1.getId()));
+		assertTrue(blogManager0.canBeRemoved(blog1));
-		assertFalse(blogManager1.canBeRemoved(blog0.getId()));
+		assertTrue(blogManager1.canBeRemoved(blog0));
 
-		// the following two calls should throw a DbException now
+		assertEquals(4, blogManager0.getBlogs().size());
-		thrown.expect(IllegalArgumentException.class);
+		assertEquals(2, blogManager1.getBlogs().size());
 
 		blogManager0.removeBlog(blog1);
 		blogManager1.removeBlog(blog0);
 
-		// blogs have not been removed
+		// blogs have been removed
-		assertEquals(2, blogManager0.getBlogs().size());
+		assertEquals(3, blogManager0.getBlogs().size());
-		assertEquals(2, blogManager1.getBlogs().size());
+		assertEquals(1, blogManager1.getBlogs().size());
 	}
 
 	@Test

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

@@ -41,7 +41,7 @@ import static org.junit.Assert.fail;
 public class BlogSharingIntegrationTest
 		extends BriarIntegrationTest<BriarIntegrationTestComponent> {
 
-	private BlogManager blogManager1;
+	private BlogManager blogManager0, blogManager1;
 	private Blog blog0, blog1, blog2;
 	private SharerListener listener0;
 	private InviteeListener listener1;
@@ -60,7 +60,7 @@ public class BlogSharingIntegrationTest
 	public void setUp() throws Exception {
 		super.setUp();
 
-		BlogManager blogManager0 = c0.getBlogManager();
+		blogManager0 = c0.getBlogManager();
 		blogManager1 = c1.getBlogManager();
 		blogSharingManager0 = c0.getBlogSharingManager();
 		blogSharingManager1 = c1.getBlogSharingManager();
@@ -370,7 +370,7 @@ public class BlogSharingIntegrationTest
 		assertEquals(contact0From1, sharedBy.iterator().next());
 
 		// shared blog can be removed
-		assertTrue(blogManager1.canBeRemoved(blog2.getId()));
+		assertTrue(blogManager1.canBeRemoved(blog2));
 
 		// invitee removes blog again
 		blogManager1.removeBlog(blog2);
@@ -386,44 +386,33 @@ public class BlogSharingIntegrationTest
 	}
 
 	@Test
-	public void testSharedBlogBecomesPermanent() throws Exception {
+	public void testRemovePreSharedBlog() throws Exception {
 		// let invitee accept all requests
 		listenToEvents(true);
 
-		// invitee only sees two blogs
+		// 0 and 1 are sharing blog 1 with each other
-		assertEquals(2, blogManager1.getBlogs().size());
+		assertTrue(blogSharingManager0.getSharedWith(blog1.getId())
+				.contains(contact1From0));
+		assertTrue(blogSharingManager1.getSharedWith(blog1.getId())
+				.contains(contact0From1));
 
-		// sharer sends invitation for 2's blog to 1
+		// 0 removes blog 1
-		blogSharingManager0
+		assertTrue(blogManager0.getBlogs().contains(blog1));
-				.sendInvitation(blog2.getId(), contactId1From0, "Hi!",
+		blogManager0.removeBlog(blog1);
-						clock.currentTimeMillis());
+		assertFalse(blogManager0.getBlogs().contains(blog1));
 
-		// sync first request message
+		// sync leave message to 0
 		sync0To1(1, true);
-		eventWaiter.await(TIMEOUT, 1);
-		assertTrue(listener1.requestReceived);
 
-		// make sure blog2 is shared by 0
+		// 0 and 1 are no longer sharing blog 1 with each other
-		Collection<Contact> contacts =
+		assertFalse(blogSharingManager0.getSharedWith(blog1.getId())
-				blogSharingManager1.getSharedWith(blog2.getId());
+				.contains(contact1From0));
-		assertEquals(1, contacts.size());
+		assertFalse(blogSharingManager1.getSharedWith(blog1.getId())
-		assertTrue(contacts.contains(contact0From1));
+				.contains(contact0From1));
 
-		// sync response back
+		// 1 can again share blog 1 with 0
-		sync1To0(1, true);
+		assertTrue(
-		eventWaiter.await(TIMEOUT, 1);
+				blogSharingManager1.canBeShared(blog1.getId(), contact0From1));
-		assertTrue(listener0.responseReceived);
-
-		// blog was added and can be removed
-		assertEquals(3, blogManager1.getBlogs().size());
-		assertTrue(blogManager1.canBeRemoved(blog2.getId()));
-
-		// 1 and 2 are adding each other
-		addContacts1And2();
-		assertEquals(3, blogManager1.getBlogs().size());
-
-		// now blog can not be removed anymore
-		assertFalse(blogManager1.canBeRemoved(blog2.getId()));
 	}
 
 	@Test