From 934997ed91fc9cfb3d5b56d5613a0cc8b313442d Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 31 Jan 2023 11:23:19 +0000 Subject: [PATCH] Check for partial setup of clients due to feature flags reverting. --- .../invitation/GroupInvitationConstants.java | 6 ++ .../GroupInvitationManagerImpl.java | 34 ++++++++-- .../briar/sharing/SharingConstants.java | 6 ++ .../briar/sharing/SharingManagerImpl.java | 34 ++++++++-- .../GroupInvitationManagerImplTest.java | 62 ++++++++++++++--- .../sharing/BlogSharingManagerImplTest.java | 66 ++++++++++++++++--- 6 files changed, 181 insertions(+), 27 deletions(-) diff --git a/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationConstants.java b/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationConstants.java index ede12734a..7232f6915 100644 --- a/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationConstants.java +++ b/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationConstants.java @@ -2,6 +2,12 @@ package org.briarproject.briar.privategroup.invitation; interface GroupInvitationConstants { + /** + * Metadata key for the client's local group, indicating that contact + * groups have been created for all contacts. + */ + String GROUP_KEY_SETUP = "setup"; + // Message metadata keys String MSG_KEY_MESSAGE_TYPE = "messageType"; String MSG_KEY_PRIVATE_GROUP_ID = "privateGroupId"; 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 cd78802a5..0e39bf7e9 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 @@ -8,6 +8,7 @@ import org.briarproject.bramble.api.contact.Contact; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.ContactManager.ContactHook; import org.briarproject.bramble.api.data.BdfDictionary; +import org.briarproject.bramble.api.data.BdfEntry; import org.briarproject.bramble.api.data.BdfList; import org.briarproject.bramble.api.data.MetadataParser; import org.briarproject.bramble.api.db.DatabaseComponent; @@ -63,6 +64,7 @@ import static org.briarproject.briar.privategroup.invitation.CreatorState.ERROR; import static org.briarproject.briar.privategroup.invitation.CreatorState.INVITED; import static org.briarproject.briar.privategroup.invitation.CreatorState.JOINED; import static org.briarproject.briar.privategroup.invitation.CreatorState.START; +import static org.briarproject.briar.privategroup.invitation.GroupInvitationConstants.GROUP_KEY_SETUP; import static org.briarproject.briar.privategroup.invitation.MessageType.ABORT; import static org.briarproject.briar.privategroup.invitation.MessageType.INVITE; import static org.briarproject.briar.privategroup.invitation.MessageType.JOIN; @@ -114,13 +116,35 @@ class GroupInvitationManagerImpl extends ConversationClientImpl @Override public void onDatabaseOpened(Transaction txn) throws DbException { - // Create a local group to indicate that we've set this client up + // If the feature flag for this client was set and then cleared, we may + // have set things up for some pre-existing contacts and not others. + // If the local group exists and has GROUP_KEY_SETUP == true in its + // metadata then we've set things up for all contacts. Otherwise we + // need to check whether a contact group exists for each contact. Group localGroup = contactGroupFactory.createLocalGroup(CLIENT_ID, MAJOR_VERSION); - if (db.containsGroup(txn, localGroup.getId())) return; - db.addGroup(txn, localGroup); - // Set things up for any pre-existing contacts - for (Contact c : db.getContacts(txn)) addingContact(txn, c); + try { + if (db.containsGroup(txn, localGroup.getId())) { + // If GROUP_KEY_SETUP == true then we're done + BdfDictionary meta = clientHelper + .getGroupMetadataAsDictionary(txn, localGroup.getId()); + if (meta.getBoolean(GROUP_KEY_SETUP, false)) return; + } else { + db.addGroup(txn, localGroup); + } + BdfDictionary meta = + BdfDictionary.of(new BdfEntry(GROUP_KEY_SETUP, true)); + clientHelper.mergeGroupMetadata(txn, localGroup.getId(), meta); + } catch (FormatException e) { + throw new DbException(e); + } + // Set things up for any contacts that haven't already been set up + for (Contact c : db.getContacts(txn)) { + Group contactGroup = getContactGroup(c); + if (!db.containsGroup(txn, contactGroup.getId())) { + addingContact(txn, c); + } + } } @Override diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/SharingConstants.java b/briar-core/src/main/java/org/briarproject/briar/sharing/SharingConstants.java index 87ce53589..1b7ea6ce6 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/SharingConstants.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/SharingConstants.java @@ -4,6 +4,12 @@ import org.briarproject.briar.client.MessageTrackerConstants; interface SharingConstants { + /** + * Metadata key for the client's local group, indicating that contact + * groups have been created for all contacts. + */ + String GROUP_KEY_SETUP = "setup"; + // Message metadata keys String MSG_KEY_MESSAGE_TYPE = "messageType"; String MSG_KEY_SHAREABLE_ID = "shareableId"; 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 b34b9965e..03144cc81 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 @@ -8,6 +8,7 @@ import org.briarproject.bramble.api.contact.Contact; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.ContactManager.ContactHook; import org.briarproject.bramble.api.data.BdfDictionary; +import org.briarproject.bramble.api.data.BdfEntry; import org.briarproject.bramble.api.data.BdfList; import org.briarproject.bramble.api.data.MetadataParser; import org.briarproject.bramble.api.db.DatabaseComponent; @@ -58,6 +59,7 @@ import static org.briarproject.briar.sharing.MessageType.ACCEPT; 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_SETUP; import static org.briarproject.briar.sharing.State.LOCAL_INVITED; import static org.briarproject.briar.sharing.State.LOCAL_LEFT; import static org.briarproject.briar.sharing.State.REMOTE_HANGING; @@ -106,13 +108,35 @@ abstract class SharingManagerImpl @Override public void onDatabaseOpened(Transaction txn) throws DbException { - // Create a local group to indicate that we've set this client up + // If the feature flag for this client was set and then cleared, we may + // have set things up for some pre-existing contacts and not others. + // If the local group exists and has GROUP_KEY_SETUP == true in its + // metadata then we've set things up for all contacts. Otherwise we + // need to check whether a contact group exists for each contact. Group localGroup = contactGroupFactory.createLocalGroup(getClientId(), getMajorVersion()); - if (db.containsGroup(txn, localGroup.getId())) return; - db.addGroup(txn, localGroup); - // Set things up for any pre-existing contacts - for (Contact c : db.getContacts(txn)) addingContact(txn, c); + try { + if (db.containsGroup(txn, localGroup.getId())) { + // If GROUP_KEY_SETUP == true then we're done + BdfDictionary meta = clientHelper + .getGroupMetadataAsDictionary(txn, localGroup.getId()); + if (meta.getBoolean(GROUP_KEY_SETUP, false)) return; + } else { + db.addGroup(txn, localGroup); + } + BdfDictionary meta = + BdfDictionary.of(new BdfEntry(GROUP_KEY_SETUP, true)); + clientHelper.mergeGroupMetadata(txn, localGroup.getId(), meta); + } catch (FormatException e) { + throw new DbException(e); + } + // Set things up for any contacts that haven't already been set up + for (Contact c : db.getContacts(txn)) { + Group contactGroup = getContactGroup(c); + if (!db.containsGroup(txn, contactGroup.getId())) { + addingContact(txn, c); + } + } } @Override 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 8f3c2398c..9fb3774bd 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 @@ -24,8 +24,8 @@ import org.briarproject.bramble.test.BrambleMockTestCase; import org.briarproject.bramble.test.DbExpectations; import org.briarproject.bramble.test.TestUtils; import org.briarproject.briar.api.client.MessageTracker; -import org.briarproject.briar.api.client.SessionId; import org.briarproject.briar.api.client.ProtocolStateException; +import org.briarproject.briar.api.client.SessionId; import org.briarproject.briar.api.conversation.ConversationMessageHeader; import org.briarproject.briar.api.privategroup.PrivateGroup; import org.briarproject.briar.api.privategroup.PrivateGroupFactory; @@ -39,7 +39,6 @@ import org.jmock.imposters.ByteBuddyClassImposteriser; import org.junit.Test; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -47,7 +46,9 @@ import javax.annotation.Nullable; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; import static junit.framework.TestCase.fail; import static org.briarproject.bramble.api.sync.Group.Visibility.SHARED; import static org.briarproject.bramble.api.sync.validation.IncomingMessageHook.DeliveryAction.ACCEPT_DO_NOT_SHARE; @@ -67,6 +68,7 @@ import static org.briarproject.briar.api.sharing.SharingManager.SharingStatus.ER import static org.briarproject.briar.api.sharing.SharingManager.SharingStatus.INVITE_RECEIVED; import static org.briarproject.briar.api.sharing.SharingManager.SharingStatus.SHAREABLE; import static org.briarproject.briar.api.sharing.SharingManager.SharingStatus.SHARING; +import static org.briarproject.briar.privategroup.invitation.GroupInvitationConstants.GROUP_KEY_SETUP; import static org.briarproject.briar.privategroup.invitation.MessageType.ABORT; import static org.briarproject.briar.privategroup.invitation.MessageType.INVITE; import static org.briarproject.briar.privategroup.invitation.MessageType.JOIN; @@ -124,9 +126,10 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase { private final BdfDictionary bdfSession = BdfDictionary.of(new BdfEntry("f", "o")); private final Map oneResult = - Collections.singletonMap(storageMessage.getId(), bdfSession); - private final Map noResults = - Collections.emptyMap(); + singletonMap(storageMessage.getId(), bdfSession); + private final Map noResults = emptyMap(); + private final BdfDictionary localGroupMeta = + BdfDictionary.of(new BdfEntry(GROUP_KEY_SETUP, true)); public GroupInvitationManagerImplTest() { @@ -167,21 +170,64 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase { oneOf(db).containsGroup(txn, localGroup.getId()); will(returnValue(false)); oneOf(db).addGroup(txn, localGroup); + oneOf(clientHelper).mergeGroupMetadata(txn, localGroup.getId(), + localGroupMeta); oneOf(db).getContacts(txn); will(returnValue(singletonList(contact))); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(false)); }}); expectAddingContact(contact, emptyList()); groupInvitationManager.onDatabaseOpened(txn); } @Test - public void testOpenDatabaseHookSubsequentTime() throws Exception { + public void testOpenDatabaseHookSubsequentTimeWithoutSetupFlag() + throws Exception { context.checking(new Expectations() {{ + // The local group exists but the metadata flag has not been set oneOf(contactGroupFactory).createLocalGroup(CLIENT_ID, MAJOR_VERSION); will(returnValue(localGroup)); oneOf(db).containsGroup(txn, localGroup.getId()); will(returnValue(true)); + oneOf(clientHelper).getGroupMetadataAsDictionary(txn, + localGroup.getId()); + will(returnValue(new BdfDictionary())); + // Store the metadata flag + oneOf(clientHelper).mergeGroupMetadata(txn, localGroup.getId(), + localGroupMeta); + // Check whether the contact group exists - it doesn't + oneOf(db).getContacts(txn); + will(returnValue(singletonList(contact))); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(false)); + }}); + // Set things up for the contact + expectAddingContact(contact, emptyList()); + + groupInvitationManager.onDatabaseOpened(txn); + } + + @Test + public void testOpenDatabaseHookSubsequentTimeWithSetupFlag() + throws Exception { + context.checking(new Expectations() {{ + // The local group exists and the metadata flag has been set + oneOf(contactGroupFactory).createLocalGroup(CLIENT_ID, + MAJOR_VERSION); + will(returnValue(localGroup)); + oneOf(db).containsGroup(txn, localGroup.getId()); + will(returnValue(true)); + oneOf(clientHelper).getGroupMetadataAsDictionary(txn, + localGroup.getId()); + will(returnValue(localGroupMeta)); }}); groupInvitationManager.onDatabaseOpened(txn); } @@ -957,9 +1003,9 @@ public class GroupInvitationManagerImplTest extends BrambleMockTestCase { BdfDictionary.of(new BdfEntry("f3", "o")); expectGetSession(oneResult, sessionId, contactGroup.getId()); - expectGetSession(Collections.singletonMap(storageId2, bdfSession2), + expectGetSession(singletonMap(storageId2, bdfSession2), sessionId, contactGroup2.getId()); - expectGetSession(Collections.singletonMap(storageId3, bdfSession3), + expectGetSession(singletonMap(storageId3, bdfSession3), sessionId, contactGroup3.getId()); context.checking(new Expectations() {{ 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 78ec37f1f..1b64524ab 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 @@ -5,6 +5,7 @@ import org.briarproject.bramble.api.client.ContactGroupFactory; import org.briarproject.bramble.api.contact.Contact; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.data.BdfDictionary; +import org.briarproject.bramble.api.data.BdfEntry; import org.briarproject.bramble.api.data.MetadataParser; import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.DbException; @@ -27,10 +28,11 @@ import org.jmock.Expectations; import org.junit.Test; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.Map; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; import static org.briarproject.bramble.api.sync.Group.Visibility.SHARED; import static org.briarproject.bramble.test.TestUtils.getAuthor; import static org.briarproject.bramble.test.TestUtils.getContact; @@ -40,6 +42,7 @@ import static org.briarproject.bramble.test.TestUtils.getMessage; import static org.briarproject.bramble.test.TestUtils.getRandomId; import static org.briarproject.briar.api.blog.BlogSharingManager.CLIENT_ID; import static org.briarproject.briar.api.blog.BlogSharingManager.MAJOR_VERSION; +import static org.briarproject.briar.sharing.SharingConstants.GROUP_KEY_SETUP; public class BlogSharingManagerImplTest extends BrambleMockTestCase { @@ -57,14 +60,16 @@ public class BlogSharingManagerImplTest extends BrambleMockTestCase { private final ContactGroupFactory contactGroupFactory = context.mock(ContactGroupFactory.class); private final BlogManager blogManager = context.mock(BlogManager.class); + @SuppressWarnings("unchecked") + private final ProtocolEngine engine = + context.mock(ProtocolEngine.class); private final LocalAuthor localAuthor = getLocalAuthor(); private final Author author = getAuthor(); private final Contact contact = getContact(author, localAuthor.getId(), true); private final ContactId contactId = contact.getId(); - private final Collection contacts = - Collections.singletonList(contact); + private final Collection contacts = singletonList(contact); private final Group localGroup = getGroup(CLIENT_ID, MAJOR_VERSION); private final Group contactGroup = getGroup(CLIENT_ID, MAJOR_VERSION); private final Group blogGroup = @@ -73,9 +78,8 @@ public class BlogSharingManagerImplTest extends BrambleMockTestCase { private final Group localBlogGroup = getGroup(BlogManager.CLIENT_ID, BlogManager.MAJOR_VERSION); private final Blog localBlog = new Blog(localBlogGroup, localAuthor, false); - @SuppressWarnings("unchecked") - private final ProtocolEngine engine = - context.mock(ProtocolEngine.class); + private final BdfDictionary localGroupMeta = + BdfDictionary.of(new BdfEntry(GROUP_KEY_SETUP, true)); @SuppressWarnings("unchecked") public BlogSharingManagerImplTest() { @@ -104,9 +108,16 @@ public class BlogSharingManagerImplTest extends BrambleMockTestCase { oneOf(db).containsGroup(txn, localGroup.getId()); will(returnValue(false)); oneOf(db).addGroup(txn, localGroup); + oneOf(clientHelper).mergeGroupMetadata(txn, localGroup.getId(), + localGroupMeta); // Get contacts oneOf(db).getContacts(txn); will(returnValue(contacts)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(false)); }}); // Set things up for the contact expectAddingContact(txn); @@ -115,7 +126,7 @@ public class BlogSharingManagerImplTest extends BrambleMockTestCase { } private void expectAddingContact(Transaction txn) throws Exception { - Map sessions = Collections.emptyMap(); + Map sessions = emptyMap(); context.checking(new Expectations() {{ // Create the contact group and share it with the contact @@ -145,16 +156,53 @@ public class BlogSharingManagerImplTest extends BrambleMockTestCase { } @Test - public void testOpenDatabaseHookSubsequentTime() throws Exception { + public void testOpenDatabaseHookSubsequentTimeWithoutSetupFlag() + throws Exception { Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ - // The local group exists - everything has been set up + // The local group exists but the metadata flag has not been set oneOf(contactGroupFactory).createLocalGroup(CLIENT_ID, MAJOR_VERSION); will(returnValue(localGroup)); oneOf(db).containsGroup(txn, localGroup.getId()); will(returnValue(true)); + oneOf(clientHelper).getGroupMetadataAsDictionary(txn, + localGroup.getId()); + will(returnValue(new BdfDictionary())); + // Store the metadata flag + oneOf(clientHelper).mergeGroupMetadata(txn, localGroup.getId(), + localGroupMeta); + // Check whether the contact group exists - it doesn't + oneOf(db).getContacts(txn); + will(returnValue(contacts)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(false)); + }}); + // Set things up for the contact + expectAddingContact(txn); + + blogSharingManager.onDatabaseOpened(txn); + } + + @Test + public void testOpenDatabaseHookSubsequentTimeWithSetupFlag() + throws Exception { + Transaction txn = new Transaction(null, false); + + context.checking(new Expectations() {{ + // The local group exists and the metadata flag has been set + oneOf(contactGroupFactory).createLocalGroup(CLIENT_ID, + MAJOR_VERSION); + will(returnValue(localGroup)); + oneOf(db).containsGroup(txn, localGroup.getId()); + will(returnValue(true)); + oneOf(clientHelper).getGroupMetadataAsDictionary(txn, + localGroup.getId()); + will(returnValue(localGroupMeta)); }}); blogSharingManager.onDatabaseOpened(txn);