From 85c11f8e1f74d7b00e940d1ca23a976e3c6820d9 Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Fri, 13 Apr 2018 15:40:39 +0100
Subject: [PATCH] Remove redundant checks when adding contacts.
Hooks are now called exactly once per contact.
---
.../TransportPropertyManagerImpl.java | 4 +-
.../TransportPropertyManagerImplTest.java | 39 ++++++------------
.../briar/messaging/MessagingManagerImpl.java | 4 +-
.../GroupInvitationManagerImpl.java | 4 +-
.../briar/sharing/BlogSharingManagerImpl.java | 12 ++----
.../briar/sharing/SharingManagerImpl.java | 10 ++---
.../GroupInvitationManagerImplTest.java | 22 ++++------
.../sharing/BlogSharingManagerImplTest.java | 40 +------------------
8 files changed, 33 insertions(+), 102 deletions(-)
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java
index 1296f03bcb..f3b62bb220 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java
@@ -65,7 +65,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
public void createLocalState(Transaction txn) throws DbException {
if (db.containsGroup(txn, localGroup.getId())) return;
db.addGroup(txn, localGroup);
- // Ensure we've set things up for any pre-existing contacts
+ // Set things up for any pre-existing contacts
for (Contact c : db.getContacts(txn)) addingContact(txn, c);
}
@@ -73,8 +73,6 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
public void addingContact(Transaction txn, Contact c) throws DbException {
// Create a group to share with the contact
Group g = getContactGroup(c);
- // Return if we've already set things up for this contact
- if (db.containsGroup(txn, g.getId())) return;
// Store the group and share it with the contact
db.addGroup(txn, g);
db.setGroupVisibility(txn, c.getId(), g.getId(), SHARED);
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java
index 1f690b14e5..6f12898b9d 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java
+++ b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java
@@ -29,6 +29,7 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import static java.util.Collections.singletonList;
import static org.briarproject.bramble.api.properties.TransportPropertyManager.CLIENT_ID;
import static org.briarproject.bramble.api.properties.TransportPropertyManager.CLIENT_VERSION;
import static org.briarproject.bramble.api.sync.Group.Visibility.SHARED;
@@ -87,39 +88,27 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
@Test
public void testCreatesGroupsAtStartup() throws Exception {
Transaction txn = new Transaction(null, false);
- Contact contact1 = getContact(true);
- Contact contact2 = getContact(true);
- List<Contact> contacts = Arrays.asList(contact1, contact2);
- Group contactGroup1 = getGroup(CLIENT_ID, CLIENT_VERSION);
- Group contactGroup2 = getGroup(CLIENT_ID, CLIENT_VERSION);
+ Contact contact = getContact(true);
+ Group contactGroup = getGroup(CLIENT_ID, CLIENT_VERSION);
context.checking(new Expectations() {{
oneOf(db).containsGroup(txn, localGroup.getId());
will(returnValue(false));
oneOf(db).addGroup(txn, localGroup);
oneOf(db).getContacts(txn);
- will(returnValue(contacts));
- // The first contact's group has already been set up
+ will(returnValue(singletonList(contact)));
oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
- CLIENT_VERSION, contact1);
- will(returnValue(contactGroup1));
- oneOf(db).containsGroup(txn, contactGroup1.getId());
- will(returnValue(true));
- // The second contact's group hasn't been set up
- oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
- CLIENT_VERSION, contact2);
- will(returnValue(contactGroup2));
- oneOf(db).containsGroup(txn, contactGroup2.getId());
- will(returnValue(false));
- oneOf(db).addGroup(txn, contactGroup2);
- oneOf(db).setGroupVisibility(txn, contact2.getId(),
- contactGroup2.getId(), SHARED);
+ CLIENT_VERSION, contact);
+ will(returnValue(contactGroup));
+ oneOf(db).addGroup(txn, contactGroup);
+ oneOf(db).setGroupVisibility(txn, contact.getId(),
+ contactGroup.getId(), SHARED);
}});
// Copy the latest local properties into the group
expectGetLocalProperties(txn);
- expectStoreMessage(txn, contactGroup2.getId(), "foo", fooPropertiesDict,
+ expectStoreMessage(txn, contactGroup.getId(), "foo", fooPropertiesDict,
1, true, true);
- expectStoreMessage(txn, contactGroup2.getId(), "bar", barPropertiesDict,
+ expectStoreMessage(txn, contactGroup.getId(), "bar", barPropertiesDict,
1, true, true);
TransportPropertyManagerImpl t = createInstance();
@@ -151,8 +140,6 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
CLIENT_VERSION, contact);
will(returnValue(contactGroup));
- oneOf(db).containsGroup(txn, contactGroup.getId());
- will(returnValue(false));
oneOf(db).addGroup(txn, contactGroup);
oneOf(db).setGroupVisibility(txn, contact.getId(),
contactGroup.getId(), SHARED);
@@ -538,7 +525,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
fooPropertiesDict, 1, true, false);
// Store the new properties in each contact's group, version 1
oneOf(db).getContacts(txn);
- will(returnValue(Collections.singletonList(contact)));
+ will(returnValue(singletonList(contact)));
oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
CLIENT_VERSION, contact);
will(returnValue(contactGroup));
@@ -597,7 +584,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
oneOf(db).removeMessage(txn, localGroupUpdateId);
// Store the merged properties in each contact's group, version 2
oneOf(db).getContacts(txn);
- will(returnValue(Collections.singletonList(contact)));
+ will(returnValue(singletonList(contact)));
oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
CLIENT_VERSION, contact);
will(returnValue(contactGroup));
diff --git a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java
index c81766492f..804dbeccfe 100644
--- a/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java
@@ -58,7 +58,7 @@ class MessagingManagerImpl extends ConversationClientImpl
CLIENT_VERSION);
if (db.containsGroup(txn, localGroup.getId())) return;
db.addGroup(txn, localGroup);
- // Ensure we've set things up for any pre-existing contacts
+ // Set things up for any pre-existing contacts
for (Contact c : db.getContacts(txn)) addingContact(txn, c);
}
@@ -67,8 +67,6 @@ class MessagingManagerImpl extends ConversationClientImpl
try {
// Create a group to share with the contact
Group g = getContactGroup(c);
- // Return if we've already set things up for this contact
- if (db.containsGroup(txn, g.getId())) return;
// Store the group and share it with the contact
db.addGroup(txn, g);
db.setGroupVisibility(txn, c.getId(), g.getId(), SHARED);
diff --git a/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java
index a03c384094..5ae9a3f81b 100644
--- a/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java
+++ b/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java
@@ -100,7 +100,7 @@ class GroupInvitationManagerImpl extends ConversationClientImpl
CLIENT_VERSION);
if (db.containsGroup(txn, localGroup.getId())) return;
db.addGroup(txn, localGroup);
- // Ensure we've set things up for any pre-existing contacts
+ // Set things up for any pre-existing contacts
for (Contact c : db.getContacts(txn)) addingContact(txn, c);
}
@@ -108,8 +108,6 @@ class GroupInvitationManagerImpl extends ConversationClientImpl
public void addingContact(Transaction txn, Contact c) throws DbException {
// Create a group to share with the contact
Group g = getContactGroup(c);
- // Return if we've already set things up for this contact
- if (db.containsGroup(txn, g.getId())) return;
// Store the group and share it with the contact
db.addGroup(txn, g);
db.setGroupVisibility(txn, c.getId(), g.getId(), SHARED);
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 f0086e856c..7a92f1c5f1 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
@@ -56,23 +56,17 @@ class BlogSharingManagerImpl extends SharingManagerImpl<Blog>
return CLIENT_VERSION;
}
- /**
- * This is called during each startup for each existing Contact.
- */
@Override
public void addingContact(Transaction txn, Contact c) throws DbException {
- // Return if we've already set things up for this contact
- if (db.containsGroup(txn, getContactGroup(c).getId())) return;
-
- // creates a group to share with the contact
+ // Create a group to share with the contact
super.addingContact(txn, c);
- // get our blog and that of Contact c
+ // Get our blog and that of the contact
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
+ // Pre-share both blogs, if they have not been shared already
try {
preShareShareable(txn, c, ourBlog);
preShareShareable(txn, c, theirBlog);
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 f26996438d..711e3875e5 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
@@ -87,7 +87,7 @@ abstract class SharingManagerImpl<S extends Shareable>
getClientVersion());
if (db.containsGroup(txn, localGroup.getId())) return;
db.addGroup(txn, localGroup);
- // Ensure we've set things up for any pre-existing contacts
+ // Set things up for any pre-existing contacts
for (Contact c : db.getContacts(txn)) addingContact(txn, c);
}
@@ -96,8 +96,6 @@ abstract class SharingManagerImpl<S extends Shareable>
try {
// Create a group to share with the contact
Group g = getContactGroup(c);
- // Return if we've already set things up for this contact
- if (db.containsGroup(txn, g.getId())) return;
// Store the group and share it with the contact
db.addGroup(txn, g);
db.setGroupVisibility(txn, c.getId(), g.getId(), SHARED);
@@ -152,17 +150,17 @@ abstract class SharingManagerImpl<S extends Shareable>
*/
void preShareShareable(Transaction txn, Contact c, S shareable)
throws DbException, FormatException {
- // return if a session already exists with that Contact
+ // Return if a session already exists with the 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
+ // Add and share the shareable with the contact
db.addGroup(txn, shareable.getGroup());
db.setGroupVisibility(txn, c.getId(), shareable.getId(), SHARED);
- // initialize session in sharing state
+ // Initialize session in sharing state
Session session = new Session(SHARING, contactGroupId,
shareable.getId(), null, null, 0, 0);
MessageId storageId = createStorageId(txn, contactGroupId);
diff --git a/briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImplTest.java
index 543af95b11..1a7ae0857d 100644
--- a/briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImplTest.java
+++ b/briar-core/src/test/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImplTest.java
@@ -159,7 +159,7 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
oneOf(db).getContacts(txn);
will(returnValue(Collections.singletonList(contact)));
}});
- expectAddingContact(contact, true);
+ expectAddingContact(contact);
groupInvitationManager.createLocalState(txn);
}
@@ -175,20 +175,14 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
groupInvitationManager.createLocalState(txn);
}
- private void expectAddingContact(Contact c, boolean contactExists)
- throws Exception {
+ private void expectAddingContact(Contact c) throws Exception {
+ BdfDictionary meta = BdfDictionary
+ .of(new BdfEntry(GROUP_KEY_CONTACT_ID, c.getId().getInt()));
+
context.checking(new Expectations() {{
oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
CLIENT_VERSION, c);
will(returnValue(contactGroup));
- oneOf(db).containsGroup(txn, contactGroup.getId());
- will(returnValue(contactExists));
- }});
- if (contactExists) return;
-
- BdfDictionary meta = BdfDictionary
- .of(new BdfEntry(GROUP_KEY_CONTACT_ID, c.getId().getInt()));
- context.checking(new Expectations() {{
oneOf(db).addGroup(txn, contactGroup);
oneOf(db).setGroupVisibility(txn, c.getId(), contactGroup.getId(),
SHARED);
@@ -197,8 +191,8 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
oneOf(db).getGroups(txn, PrivateGroupManager.CLIENT_ID,
PrivateGroupManager.CLIENT_VERSION);
will(returnValue(Collections.singletonList(privateGroup)));
- oneOf(privateGroupManager)
- .isMember(txn, privateGroup.getId(), c.getAuthor());
+ oneOf(privateGroupManager).isMember(txn, privateGroup.getId(),
+ c.getAuthor());
will(returnValue(true));
}});
expectAddingMember(privateGroup.getId(), c);
@@ -255,7 +249,7 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase {
@Test
public void testAddingContact() throws Exception {
- expectAddingContact(contact, false);
+ expectAddingContact(contact);
groupInvitationManager.addingContact(txn, contact);
}
diff --git a/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingManagerImplTest.java
index db3b6af2cc..8a7a3775f0 100644
--- a/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingManagerImplTest.java
+++ b/briar-core/src/test/java/org/briarproject/briar/sharing/BlogSharingManagerImplTest.java
@@ -91,7 +91,7 @@ public class BlogSharingManagerImplTest extends BrambleMockTestCase {
}
@Test
- public void testCreateLocalStateFirstTimeWithExistingContactNotSetUp()
+ public void testCreateLocalStateFirstTimeWithExistingContact()
throws Exception {
Transaction txn = new Transaction(null, false);
@@ -119,19 +119,10 @@ public class BlogSharingManagerImplTest extends BrambleMockTestCase {
Map<MessageId, BdfDictionary> sessions = Collections.emptyMap();
context.checking(new Expectations() {{
- // Check for contact group in BlogSharingManagerImpl
- oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
- CLIENT_VERSION, contact);
- will(returnValue(contactGroup));
- oneOf(db).containsGroup(txn, contactGroup.getId());
- will(returnValue(false));
- // Check for contact group again in SharingManagerImpl
+ // Create the contact group and share it with the contact
oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
CLIENT_VERSION, contact);
will(returnValue(contactGroup));
- oneOf(db).containsGroup(txn, contactGroup.getId());
- will(returnValue(false));
- // Create the contact group and share it with the contact
oneOf(db).addGroup(txn, contactGroup);
oneOf(db).setGroupVisibility(txn, contactId, contactGroup.getId(),
SHARED);
@@ -151,33 +142,6 @@ public class BlogSharingManagerImplTest extends BrambleMockTestCase {
expectPreShareShareable(txn, contact, blog, sessions);
}
- @Test
- public void testCreateLocalStateFirstTimeWithExistingContactAlreadySetUp()
- throws Exception {
- Transaction txn = new Transaction(null, false);
-
- context.checking(new Expectations() {{
- // The local group doesn't exist - we need to set things up
- oneOf(contactGroupFactory).createLocalGroup(CLIENT_ID,
- CLIENT_VERSION);
- will(returnValue(localGroup));
- oneOf(db).containsGroup(txn, localGroup.getId());
- will(returnValue(false));
- oneOf(db).addGroup(txn, localGroup);
- // Get contacts
- oneOf(db).getContacts(txn);
- will(returnValue(contacts));
- // The contact has already been set up
- oneOf(contactGroupFactory).createContactGroup(CLIENT_ID,
- CLIENT_VERSION, contact);
- will(returnValue(contactGroup));
- oneOf(db).containsGroup(txn, contactGroup.getId());
- will(returnValue(true));
- }});
-
- blogSharingManager.createLocalState(txn);
- }
-
@Test
public void testCreateLocalStateSubsequentTime() throws Exception {
Transaction txn = new Transaction(null, false);
--
GitLab