From 6a07d8f2c9c6df030f1cde2c1cc4cd4efd28a99e Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Wed, 26 Apr 2017 18:51:08 -0300
Subject: [PATCH] Allow to remove pre-shared blogs of our contacts
---
.../android/blog/BlogControllerImpl.java | 2 +-
.../briar/api/blog/BlogManager.java | 2 +-
.../briar/blog/BlogManagerImpl.java | 30 +++-------
.../briar/sharing/BlogSharingManagerImpl.java | 25 ++++-----
.../briar/sharing/SharingManagerImpl.java | 13 ++++-
.../briar/blog/BlogManagerImplTest.java | 55 ++++---------------
.../blog/BlogManagerIntegrationTest.java | 17 +++---
.../sharing/BlogSharingIntegrationTest.java | 55 ++++++++-----------
8 files changed, 76 insertions(+), 123 deletions(-)
diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogControllerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogControllerImpl.java
index ae666f691a..414420dbd2 100644
--- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogControllerImpl.java
+++ b/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))
diff --git a/briar-api/src/main/java/org/briarproject/briar/api/blog/BlogManager.java b/briar-api/src/main/java/org/briarproject/briar/api/blog/BlogManager.java
index 041d474a4e..cc7dc13fc3 100644
--- a/briar-api/src/main/java/org/briarproject/briar/api/blog/BlogManager.java
+++ b/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.
diff --git a/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java
index b27a3bf116..02e4f1eae7 100644
--- a/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java
+++ b/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,
- BlogPostFactory blogPostFactory) {
+ BlogFactory blogFactory, 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;
- canBeRemoved = !contactManager
- .contactExists(txn, authorId, localAuthor.getId());
- return canBeRemoved;
+ return !localAuthor.getId().equals(authorId);
}
@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);
- }
-
- private void removeBlog(Transaction txn, Blog b, boolean forced)
- throws DbException {
- if (!forced && !canBeRemoved(txn, b.getId()))
+ if (!canBeRemoved(txn, b))
throw new IllegalArgumentException();
for (RemoveBlogHook hook : removeHooks)
hook.removingBlog(txn, b);
diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/BlogSharingManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/BlogSharingManagerImpl.java
index 26da642ba2..6664c67dcd 100644
--- a/briar-core/src/main/java/org/briarproject/briar/sharing/BlogSharingManagerImpl.java
+++ 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,
- Contact c) throws DbException {
- // check if shareableId belongs to our personal blog
- LocalAuthor author = identityManager.getLocalAuthor(txn);
- Blog b = blogManager.getPersonalBlog(author);
- if (b.getId().equals(shareableId)) return false;
-
- // check if shareableId belongs to c's personal blog
- b = blogManager.getPersonalBlog(c.getAuthor());
- if (b.getId().equals(shareableId)) return false;
-
- return super.canBeShared(txn, shareableId, c);
+ public void addingContact(Transaction txn, Contact c) throws DbException {
+ super.addingContact(txn, c);
+ LocalAuthor localAuthor = identityManager.getLocalAuthor(txn);
+ Blog ourBlog = blogManager.getPersonalBlog(localAuthor);
+ Blog theirBlog = blogManager.getPersonalBlog(c.getAuthor());
+ try {
+ initializeSharedSession(txn, c, ourBlog);
+ initializeSharedSession(txn, c, theirBlog);
+ } catch (FormatException e) {
+ throw new DbException(e);
+ }
}
@Override
diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java
index fc61bf49df..0b99d75bf6 100644
--- a/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java
+++ b/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);
diff --git a/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerImplTest.java
index 1ff6404811..5b9cee48cc 100644
--- a/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerImplTest.java
+++ b/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() {
diff --git a/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerIntegrationTest.java
index 289f3c0991..6c88846cf3 100644
--- a/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerIntegrationTest.java
+++ b/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 {
- assertFalse(blogManager0.canBeRemoved(blog1.getId()));
- assertFalse(blogManager1.canBeRemoved(blog0.getId()));
+ public void testCanRemoveContactsPersonalBlog() throws Exception {
+ assertTrue(blogManager0.canBeRemoved(blog1));
+ assertTrue(blogManager1.canBeRemoved(blog0));
- // the following two calls should throw a DbException now
- thrown.expect(IllegalArgumentException.class);
+ assertEquals(4, blogManager0.getBlogs().size());
+ assertEquals(2, blogManager1.getBlogs().size());
blogManager0.removeBlog(blog1);
blogManager1.removeBlog(blog0);
- // blogs have not been removed
- assertEquals(2, blogManager0.getBlogs().size());
- assertEquals(2, blogManager1.getBlogs().size());
+ // blogs have been removed
+ assertEquals(3, blogManager0.getBlogs().size());
+ assertEquals(1, blogManager1.getBlogs().size());
}
@Test
diff --git a/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingIntegrationTest.java
index 31608a0261..e13718352e 100644
--- a/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingIntegrationTest.java
+++ b/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
- assertEquals(2, blogManager1.getBlogs().size());
+ // 0 and 1 are sharing blog 1 with each other
+ assertTrue(blogSharingManager0.getSharedWith(blog1.getId())
+ .contains(contact1From0));
+ assertTrue(blogSharingManager1.getSharedWith(blog1.getId())
+ .contains(contact0From1));
- // sharer sends invitation for 2's blog to 1
- blogSharingManager0
- .sendInvitation(blog2.getId(), contactId1From0, "Hi!",
- clock.currentTimeMillis());
+ // 0 removes blog 1
+ assertTrue(blogManager0.getBlogs().contains(blog1));
+ blogManager0.removeBlog(blog1);
+ 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
- Collection<Contact> contacts =
- blogSharingManager1.getSharedWith(blog2.getId());
- assertEquals(1, contacts.size());
- assertTrue(contacts.contains(contact0From1));
-
- // sync response back
- sync1To0(1, true);
- eventWaiter.await(TIMEOUT, 1);
- 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());
+ // 0 and 1 are no longer sharing blog 1 with each other
+ assertFalse(blogSharingManager0.getSharedWith(blog1.getId())
+ .contains(contact1From0));
+ assertFalse(blogSharingManager1.getSharedWith(blog1.getId())
+ .contains(contact0From1));
- // now blog can not be removed anymore
- assertFalse(blogManager1.canBeRemoved(blog2.getId()));
+ // 1 can again share blog 1 with 0
+ assertTrue(
+ blogSharingManager1.canBeShared(blog1.getId(), contact0From1));
}
@Test
--
GitLab