Check for partial setup of clients due to feature flags reverting.

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";

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

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";

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<S extends Shareable>
 
 	@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

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<MessageId, BdfDictionary> oneResult =
-			Collections.singletonMap(storageMessage.getId(), bdfSession);
-	private final Map<MessageId, BdfDictionary> noResults =
-			Collections.emptyMap();
+			singletonMap(storageMessage.getId(), bdfSession);
+	private final Map<MessageId, BdfDictionary> 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() {{

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<Blog> 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<Contact> contacts =
-			Collections.singletonList(contact);
+	private final Collection<Contact> 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<Blog> 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<MessageId, BdfDictionary> sessions = Collections.emptyMap();
+		Map<MessageId, BdfDictionary> 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);