From 008147248937cfc2cfaa2de23aa3adbbc06ce883 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 11 Dec 2018 17:25:29 +0000 Subject: [PATCH 1/9] Add method for querying contact's client minor version. --- .../versioning/ClientVersioningManager.java | 7 ++++ .../ClientVersioningManagerImpl.java | 38 +++++++++++++++---- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/versioning/ClientVersioningManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/versioning/ClientVersioningManager.java index 00b3d672c..76b261d1b 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/versioning/ClientVersioningManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/versioning/ClientVersioningManager.java @@ -38,6 +38,13 @@ public interface ClientVersioningManager { Visibility getClientVisibility(Transaction txn, ContactId contactId, ClientId clientId, int majorVersion) throws DbException; + /** + * Returns the minor version of the given client that is supported by the + * given contact, or -1 if the contact does not support the client. + */ + int getClientMinorVersion(Transaction txn, ContactId contactId, + ClientId clientId, int majorVersion) throws DbException; + interface ClientVersioningHook { void onClientVisibilityChanging(Transaction txn, Contact c, diff --git a/bramble-core/src/main/java/org/briarproject/bramble/versioning/ClientVersioningManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/versioning/ClientVersioningManagerImpl.java index a7a6ac312..ccc7b41bb 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/versioning/ClientVersioningManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/versioning/ClientVersioningManagerImpl.java @@ -89,14 +89,9 @@ class ClientVersioningManagerImpl implements ClientVersioningManager, Client, public Visibility getClientVisibility(Transaction txn, ContactId contactId, ClientId clientId, int majorVersion) throws DbException { try { - Contact contact = db.getContact(txn, contactId); - Group g = getContactGroup(contact); - // Contact may be in the process of being added or removed, so - // contact group may not exist - if (!db.containsGroup(txn, g.getId())) return INVISIBLE; - LatestUpdates latest = findLatestUpdates(txn, g.getId()); + LatestUpdates latest = findLatestUpdates(txn, contactId); + if (latest == null || latest.remote == null) return INVISIBLE; if (latest.local == null) throw new DbException(); - if (latest.remote == null) return INVISIBLE; Update localUpdate = loadUpdate(txn, latest.local.messageId); Update remoteUpdate = loadUpdate(txn, latest.remote.messageId); Map visibilities = @@ -110,6 +105,24 @@ class ClientVersioningManagerImpl implements ClientVersioningManager, Client, } } + @Override + public int getClientMinorVersion(Transaction txn, ContactId contactId, + ClientId clientId, int majorVersion) throws DbException { + try { + LatestUpdates latest = findLatestUpdates(txn, contactId); + if (latest == null || latest.remote == null) return -1; + Update remoteUpdate = loadUpdate(txn, latest.remote.messageId); + ClientMajorVersion cv = + new ClientMajorVersion(clientId, majorVersion); + for (ClientState remote : remoteUpdate.states) { + if (remote.majorVersion.equals(cv)) return remote.minorVersion; + } + return -1; + } catch (FormatException e) { + throw new DbException(e); + } + } + @Override public void createLocalState(Transaction txn) throws DbException { if (db.containsGroup(txn, localGroup.getId())) return; @@ -336,6 +349,17 @@ class ClientVersioningManagerImpl implements ClientVersioningManager, Client, MAJOR_VERSION, c); } + @Nullable + private LatestUpdates findLatestUpdates(Transaction txn, ContactId c) + throws DbException, FormatException { + Contact contact = db.getContact(txn, c); + Group g = getContactGroup(contact); + // Contact may be in the process of being added or removed, so + // contact group may not exist + if (!db.containsGroup(txn, g.getId())) return null; + return findLatestUpdates(txn, g.getId()); + } + private LatestUpdates findLatestUpdates(Transaction txn, GroupId g) throws DbException, FormatException { Map metadata = From b8f248ca9c1c84b3488a4aa0d62bf0669ed62926 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 11 Dec 2018 17:51:42 +0000 Subject: [PATCH 2/9] Add tests for getClientVisibility(). --- .../ClientVersioningManagerImplTest.java | 233 ++++++++++++++++++ 1 file changed, 233 insertions(+) diff --git a/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java index a4287fa78..b613c2b4e 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java @@ -8,6 +8,7 @@ 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.db.DatabaseComponent; +import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Metadata; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.sync.ClientId; @@ -43,6 +44,7 @@ import static org.briarproject.bramble.util.StringUtils.getRandomString; import static org.briarproject.bramble.versioning.ClientVersioningConstants.GROUP_KEY_CONTACT_ID; import static org.briarproject.bramble.versioning.ClientVersioningConstants.MSG_KEY_LOCAL; import static org.briarproject.bramble.versioning.ClientVersioningConstants.MSG_KEY_UPDATE_VERSION; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; public class ClientVersioningManagerImplTest extends BrambleMockTestCase { @@ -657,4 +659,235 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { c.registerClient(clientId, 123, 234, hook); assertFalse(c.incomingMessage(txn, newRemoteUpdate, new Metadata())); } + + @Test + public void testReturnsInvisibleIfContactGroupDoesNotExist() + throws Exception { + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(false)); + }}); + + ClientVersioningManagerImpl c = createInstance(); + assertEquals(INVISIBLE, c.getClientVisibility(txn, contact.getId(), + clientId, 123)); + } + + @Test + public void testReturnsInvisibleIfNoRemoteUpdateExists() throws Exception { + MessageId localUpdateId = new MessageId(getRandomId()); + BdfDictionary localUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, true)); + + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(true)); + oneOf(clientHelper).getMessageMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(singletonMap(localUpdateId, localUpdateMeta))); + }}); + + ClientVersioningManagerImpl c = createInstance(); + assertEquals(INVISIBLE, c.getClientVisibility(txn, contact.getId(), + clientId, 123)); + } + + @Test(expected = DbException.class) + public void testThrowsExceptionIfNoLocalUpdateExists() throws Exception { + MessageId remoteUpdateId = new MessageId(getRandomId()); + BdfDictionary remoteUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, false)); + + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(true)); + oneOf(clientHelper).getMessageMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(singletonMap(remoteUpdateId, remoteUpdateMeta))); + }}); + + ClientVersioningManagerImpl c = createInstance(); + c.getClientVisibility(txn, contact.getId(), clientId, 123); + } + + @Test + public void testReturnsInvisibleIfClientNotSupportedLocally() + throws Exception { + MessageId localUpdateId = new MessageId(getRandomId()); + BdfDictionary localUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, true)); + MessageId remoteUpdateId = new MessageId(getRandomId()); + BdfDictionary remoteUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, false)); + Map messageMetadata = new HashMap<>(); + messageMetadata.put(localUpdateId, localUpdateMeta); + messageMetadata.put(remoteUpdateId, remoteUpdateMeta); + // The client is supported remotely but not locally + BdfList localUpdateBody = BdfList.of(new BdfList(), 1L); + BdfList remoteUpdateBody = BdfList.of(BdfList.of( + BdfList.of(clientId.getString(), 123, 234, false)), 1L); + + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(true)); + oneOf(clientHelper).getMessageMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(messageMetadata)); + oneOf(clientHelper).getMessageAsList(txn, localUpdateId); + will(returnValue(localUpdateBody)); + oneOf(clientHelper).getMessageAsList(txn, remoteUpdateId); + will(returnValue(remoteUpdateBody)); + }}); + + ClientVersioningManagerImpl c = createInstance(); + assertEquals(INVISIBLE, c.getClientVisibility(txn, contact.getId(), + clientId, 123)); + } + + @Test + public void testReturnsInvisibleIfClientNotSupportedRemotely() + throws Exception { + MessageId localUpdateId = new MessageId(getRandomId()); + BdfDictionary localUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, true)); + MessageId remoteUpdateId = new MessageId(getRandomId()); + BdfDictionary remoteUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, false)); + Map messageMetadata = new HashMap<>(); + messageMetadata.put(localUpdateId, localUpdateMeta); + messageMetadata.put(remoteUpdateId, remoteUpdateMeta); + // The client is supported locally but not remotely + BdfList localUpdateBody = BdfList.of(BdfList.of( + BdfList.of(clientId.getString(), 123, 234, false)), 1L); + BdfList remoteUpdateBody = BdfList.of(new BdfList(), 1L); + + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(true)); + oneOf(clientHelper).getMessageMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(messageMetadata)); + oneOf(clientHelper).getMessageAsList(txn, localUpdateId); + will(returnValue(localUpdateBody)); + oneOf(clientHelper).getMessageAsList(txn, remoteUpdateId); + will(returnValue(remoteUpdateBody)); + }}); + + ClientVersioningManagerImpl c = createInstance(); + assertEquals(INVISIBLE, c.getClientVisibility(txn, contact.getId(), + clientId, 123)); + } + + @Test + public void testReturnsVisibleIfClientNotActiveRemotely() throws Exception { + MessageId localUpdateId = new MessageId(getRandomId()); + BdfDictionary localUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, true)); + MessageId remoteUpdateId = new MessageId(getRandomId()); + BdfDictionary remoteUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, false)); + Map messageMetadata = new HashMap<>(); + messageMetadata.put(localUpdateId, localUpdateMeta); + messageMetadata.put(remoteUpdateId, remoteUpdateMeta); + // The client is supported locally and remotely but not active + BdfList localUpdateBody = BdfList.of(BdfList.of( + BdfList.of(clientId.getString(), 123, 234, false)), 1L); + BdfList remoteUpdateBody = BdfList.of(BdfList.of( + BdfList.of(clientId.getString(), 123, 234, false)), 1L); + + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(true)); + oneOf(clientHelper).getMessageMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(messageMetadata)); + oneOf(clientHelper).getMessageAsList(txn, localUpdateId); + will(returnValue(localUpdateBody)); + oneOf(clientHelper).getMessageAsList(txn, remoteUpdateId); + will(returnValue(remoteUpdateBody)); + }}); + + ClientVersioningManagerImpl c = createInstance(); + assertEquals(VISIBLE, c.getClientVisibility(txn, contact.getId(), + clientId, 123)); + } + + @Test + public void testReturnsSharedIfClientActiveRemotely() throws Exception { + MessageId localUpdateId = new MessageId(getRandomId()); + BdfDictionary localUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, true)); + MessageId remoteUpdateId = new MessageId(getRandomId()); + BdfDictionary remoteUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, false)); + Map messageMetadata = new HashMap<>(); + messageMetadata.put(localUpdateId, localUpdateMeta); + messageMetadata.put(remoteUpdateId, remoteUpdateMeta); + // The client is supported locally and remotely and active + BdfList localUpdateBody = BdfList.of(BdfList.of( + BdfList.of(clientId.getString(), 123, 234, true)), 1L); + BdfList remoteUpdateBody = BdfList.of(BdfList.of( + BdfList.of(clientId.getString(), 123, 234, true)), 1L); + + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(true)); + oneOf(clientHelper).getMessageMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(messageMetadata)); + oneOf(clientHelper).getMessageAsList(txn, localUpdateId); + will(returnValue(localUpdateBody)); + oneOf(clientHelper).getMessageAsList(txn, remoteUpdateId); + will(returnValue(remoteUpdateBody)); + }}); + + ClientVersioningManagerImpl c = createInstance(); + assertEquals(SHARED, c.getClientVisibility(txn, contact.getId(), + clientId, 123)); + } } From 1d5214117f4403f7dae4e9b33528cc757d945f05 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 11 Dec 2018 17:55:39 +0000 Subject: [PATCH 3/9] Add tests for getClientMinorVersion(). --- .../ClientVersioningManagerImplTest.java | 156 ++++++++++++++++++ 1 file changed, 156 insertions(+) diff --git a/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java index b613c2b4e..790d0a1ac 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java @@ -890,4 +890,160 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { assertEquals(SHARED, c.getClientVisibility(txn, contact.getId(), clientId, 123)); } + + @Test + public void testReturnsNegativeIfContactGroupDoesNotExist() + throws Exception { + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(false)); + }}); + + ClientVersioningManagerImpl c = createInstance(); + assertEquals(-1, c.getClientMinorVersion(txn, contact.getId(), + clientId, 123)); + } + + @Test + public void testReturnsNegativeIfNoRemoteUpdateExists() throws Exception { + MessageId localUpdateId = new MessageId(getRandomId()); + BdfDictionary localUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, true)); + + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(true)); + oneOf(clientHelper).getMessageMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(singletonMap(localUpdateId, localUpdateMeta))); + }}); + + ClientVersioningManagerImpl c = createInstance(); + assertEquals(-1, c.getClientMinorVersion(txn, contact.getId(), + clientId, 123)); + } + + @Test + public void testReturnsNegativeIfClientNotSupportedRemotely() + throws Exception { + MessageId localUpdateId = new MessageId(getRandomId()); + BdfDictionary localUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, true)); + MessageId remoteUpdateId = new MessageId(getRandomId()); + BdfDictionary remoteUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, false)); + Map messageMetadata = new HashMap<>(); + messageMetadata.put(localUpdateId, localUpdateMeta); + messageMetadata.put(remoteUpdateId, remoteUpdateMeta); + // The client is not supported remotely + BdfList remoteUpdateBody = BdfList.of(new BdfList(), 1L); + + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(true)); + oneOf(clientHelper).getMessageMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(messageMetadata)); + oneOf(clientHelper).getMessageAsList(txn, remoteUpdateId); + will(returnValue(remoteUpdateBody)); + }}); + + ClientVersioningManagerImpl c = createInstance(); + assertEquals(-1, c.getClientMinorVersion(txn, contact.getId(), + clientId, 123)); + } + + @Test + public void testReturnsMinorVersionIfClientNotActiveRemotely() + throws Exception { + MessageId localUpdateId = new MessageId(getRandomId()); + BdfDictionary localUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, true)); + MessageId remoteUpdateId = new MessageId(getRandomId()); + BdfDictionary remoteUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, false)); + Map messageMetadata = new HashMap<>(); + messageMetadata.put(localUpdateId, localUpdateMeta); + messageMetadata.put(remoteUpdateId, remoteUpdateMeta); + // The client is supported remotely but not active + BdfList remoteUpdateBody = BdfList.of(BdfList.of( + BdfList.of(clientId.getString(), 123, 234, false)), 1L); + + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(true)); + oneOf(clientHelper).getMessageMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(messageMetadata)); + oneOf(clientHelper).getMessageAsList(txn, remoteUpdateId); + will(returnValue(remoteUpdateBody)); + }}); + + ClientVersioningManagerImpl c = createInstance(); + assertEquals(234, c.getClientMinorVersion(txn, contact.getId(), + clientId, 123)); + } + + @Test + public void testReturnsMinorVersionIfClientActiveRemotely() + throws Exception { + MessageId localUpdateId = new MessageId(getRandomId()); + BdfDictionary localUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, true)); + MessageId remoteUpdateId = new MessageId(getRandomId()); + BdfDictionary remoteUpdateMeta = BdfDictionary.of( + new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), + new BdfEntry(MSG_KEY_LOCAL, false)); + Map messageMetadata = new HashMap<>(); + messageMetadata.put(localUpdateId, localUpdateMeta); + messageMetadata.put(remoteUpdateId, remoteUpdateMeta); + // The client is supported remotely and active + BdfList remoteUpdateBody = BdfList.of(BdfList.of( + BdfList.of(clientId.getString(), 123, 234, true)), 1L); + + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(true)); + oneOf(clientHelper).getMessageMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(messageMetadata)); + oneOf(clientHelper).getMessageAsList(txn, remoteUpdateId); + will(returnValue(remoteUpdateBody)); + }}); + + ClientVersioningManagerImpl c = createInstance(); + assertEquals(234, c.getClientMinorVersion(txn, contact.getId(), + clientId, 123)); + } } From 149e67c0f79a232078d4fbfd24ba48ad62f2b697 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 12 Dec 2018 11:57:35 +0000 Subject: [PATCH 4/9] Reduce code duplication in tests. --- .../ClientVersioningManagerImplTest.java | 112 ++++-------------- 1 file changed, 24 insertions(+), 88 deletions(-) diff --git a/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java index 790d0a1ac..1c6253c8b 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/versioning/ClientVersioningManagerImplTest.java @@ -663,15 +663,7 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { @Test public void testReturnsInvisibleIfContactGroupDoesNotExist() throws Exception { - context.checking(new Expectations() {{ - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, - MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(db).containsGroup(txn, contactGroup.getId()); - will(returnValue(false)); - }}); + expectGetContactGroup(false); ClientVersioningManagerImpl c = createInstance(); assertEquals(INVISIBLE, c.getClientVisibility(txn, contact.getId(), @@ -685,14 +677,8 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), new BdfEntry(MSG_KEY_LOCAL, true)); + expectGetContactGroup(true); context.checking(new Expectations() {{ - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, - MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(db).containsGroup(txn, contactGroup.getId()); - will(returnValue(true)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup.getId()); will(returnValue(singletonMap(localUpdateId, localUpdateMeta))); @@ -710,14 +696,8 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), new BdfEntry(MSG_KEY_LOCAL, false)); + expectGetContactGroup(true); context.checking(new Expectations() {{ - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, - MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(db).containsGroup(txn, contactGroup.getId()); - will(returnValue(true)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup.getId()); will(returnValue(singletonMap(remoteUpdateId, remoteUpdateMeta))); @@ -746,14 +726,8 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { BdfList remoteUpdateBody = BdfList.of(BdfList.of( BdfList.of(clientId.getString(), 123, 234, false)), 1L); + expectGetContactGroup(true); context.checking(new Expectations() {{ - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, - MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(db).containsGroup(txn, contactGroup.getId()); - will(returnValue(true)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup.getId()); will(returnValue(messageMetadata)); @@ -787,14 +761,8 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { BdfList.of(clientId.getString(), 123, 234, false)), 1L); BdfList remoteUpdateBody = BdfList.of(new BdfList(), 1L); + expectGetContactGroup(true); context.checking(new Expectations() {{ - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, - MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(db).containsGroup(txn, contactGroup.getId()); - will(returnValue(true)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup.getId()); will(returnValue(messageMetadata)); @@ -828,14 +796,8 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { BdfList remoteUpdateBody = BdfList.of(BdfList.of( BdfList.of(clientId.getString(), 123, 234, false)), 1L); + expectGetContactGroup(true); context.checking(new Expectations() {{ - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, - MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(db).containsGroup(txn, contactGroup.getId()); - will(returnValue(true)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup.getId()); will(returnValue(messageMetadata)); @@ -869,14 +831,8 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { BdfList remoteUpdateBody = BdfList.of(BdfList.of( BdfList.of(clientId.getString(), 123, 234, true)), 1L); + expectGetContactGroup(true); context.checking(new Expectations() {{ - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, - MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(db).containsGroup(txn, contactGroup.getId()); - will(returnValue(true)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup.getId()); will(returnValue(messageMetadata)); @@ -894,15 +850,7 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { @Test public void testReturnsNegativeIfContactGroupDoesNotExist() throws Exception { - context.checking(new Expectations() {{ - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, - MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(db).containsGroup(txn, contactGroup.getId()); - will(returnValue(false)); - }}); + expectGetContactGroup(false); ClientVersioningManagerImpl c = createInstance(); assertEquals(-1, c.getClientMinorVersion(txn, contact.getId(), @@ -916,14 +864,8 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { new BdfEntry(MSG_KEY_UPDATE_VERSION, 1L), new BdfEntry(MSG_KEY_LOCAL, true)); + expectGetContactGroup(true); context.checking(new Expectations() {{ - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, - MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(db).containsGroup(txn, contactGroup.getId()); - will(returnValue(true)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup.getId()); will(returnValue(singletonMap(localUpdateId, localUpdateMeta))); @@ -951,14 +893,8 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { // The client is not supported remotely BdfList remoteUpdateBody = BdfList.of(new BdfList(), 1L); + expectGetContactGroup(true); context.checking(new Expectations() {{ - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, - MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(db).containsGroup(txn, contactGroup.getId()); - will(returnValue(true)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup.getId()); will(returnValue(messageMetadata)); @@ -989,14 +925,8 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { BdfList remoteUpdateBody = BdfList.of(BdfList.of( BdfList.of(clientId.getString(), 123, 234, false)), 1L); + expectGetContactGroup(true); context.checking(new Expectations() {{ - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, - MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(db).containsGroup(txn, contactGroup.getId()); - will(returnValue(true)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup.getId()); will(returnValue(messageMetadata)); @@ -1027,14 +957,8 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { BdfList remoteUpdateBody = BdfList.of(BdfList.of( BdfList.of(clientId.getString(), 123, 234, true)), 1L); + expectGetContactGroup(true); context.checking(new Expectations() {{ - oneOf(db).getContact(txn, contact.getId()); - will(returnValue(contact)); - oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, - MAJOR_VERSION, contact); - will(returnValue(contactGroup)); - oneOf(db).containsGroup(txn, contactGroup.getId()); - will(returnValue(true)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup.getId()); will(returnValue(messageMetadata)); @@ -1046,4 +970,16 @@ public class ClientVersioningManagerImplTest extends BrambleMockTestCase { assertEquals(234, c.getClientMinorVersion(txn, contact.getId(), clientId, 123)); } + + private void expectGetContactGroup(boolean exists) throws Exception { + context.checking(new Expectations() {{ + oneOf(db).getContact(txn, contact.getId()); + will(returnValue(contact)); + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(db).containsGroup(txn, contactGroup.getId()); + will(returnValue(exists)); + }}); + } } From 4796902b9cbc109fbe50e3aaa44dd38a54b49db8 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 23 Nov 2018 18:58:32 -0200 Subject: [PATCH 5/9] [android] store attachments and actually attach them to sent messages --- .../conversation/AttachmentController.java | 23 ++- .../conversation/ConversationActivity.java | 64 +------ .../conversation/ConversationViewModel.java | 156 +++++++++++++++++- .../briar/android/util/UiUtils.java | 18 ++ .../briar/api/messaging/MessagingManager.java | 4 +- .../briar/messaging/MessagingManagerImpl.java | 4 +- 6 files changed, 207 insertions(+), 62 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java index 2d85e2668..b1a37456f 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java @@ -90,10 +90,16 @@ class AttachmentController { return attachments; } + /** + * Creates {@link AttachmentItem}s from the passed headers and Attachments. + * Note: This marks the {@link Attachment}'s {@link InputStream} + * and closes the streams. + */ List getAttachmentItems( List> attachments) { List items = new ArrayList<>(attachments.size()); for (Pair a : attachments) { + a.getSecond().getStream().mark(Integer.MAX_VALUE); AttachmentItem item = getAttachmentItem(a.getFirst(), a.getSecond()); items.add(item); @@ -101,12 +107,18 @@ class AttachmentController { return items; } - private AttachmentItem getAttachmentItem(AttachmentHeader h, Attachment a) { + /** + * Creates an {@link AttachmentItem} from the {@link Attachment}'s + * {@link InputStream}. + * Note: Requires a resettable InputStream + * with the beginning already marked. + * The stream will be closed when this method returns. + */ + AttachmentItem getAttachmentItem(AttachmentHeader h, Attachment a) { MessageId messageId = h.getMessageId(); Size size = new Size(); InputStream is = a.getStream(); - is.mark(Integer.MAX_VALUE); try { // use exif to get size if (h.getContentType().equals("image/jpeg")) { @@ -178,6 +190,13 @@ class AttachmentController { options.outMimeType); } + static String getContentTypeFromBitmap(InputStream is) { + BitmapFactory.Options options = new BitmapFactory.Options(); + options.inJustDecodeBounds = true; + BitmapFactory.decodeStream(is, null, options); + return options.outMimeType; + } + private Size getThumbnailSize(int width, int height, String mimeType) { float widthPercentage = maxWidth / (float) width; float heightPercentage = maxHeight / (float) height; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java index 3d3611f90..9f48b5628 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java @@ -29,7 +29,6 @@ import android.widget.ImageView; import android.widget.TextView; import android.widget.Toast; -import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.Pair; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.ContactManager; @@ -49,7 +48,6 @@ import org.briarproject.bramble.api.plugin.event.ContactDisconnectedEvent; import org.briarproject.bramble.api.settings.Settings; import org.briarproject.bramble.api.settings.SettingsManager; import org.briarproject.bramble.api.sync.GroupId; -import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.api.sync.event.MessagesAckedEvent; import org.briarproject.bramble.api.sync.event.MessagesSentEvent; @@ -83,7 +81,6 @@ import org.briarproject.briar.api.introduction.IntroductionManager; import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.AttachmentHeader; import org.briarproject.briar.api.messaging.MessagingManager; -import org.briarproject.briar.api.messaging.PrivateMessage; import org.briarproject.briar.api.messaging.PrivateMessageFactory; import org.briarproject.briar.api.messaging.PrivateMessageHeader; import org.briarproject.briar.api.privategroup.invitation.GroupInvitationManager; @@ -196,8 +193,6 @@ public class ConversationActivity extends BriarActivity ViewModelProvider.Factory viewModelFactory; private volatile ContactId contactId; - @Nullable - private volatile GroupId messagingGroupId; private final Observer contactNameObserver = name -> { requireNonNull(name); @@ -245,6 +240,8 @@ public class ConversationActivity extends BriarActivity requireNonNull(deleted); if (deleted) finish(); }); + viewModel.getAddedPrivateMessage() + .observe(this, this::onAddedPrivateMessage); setTransitionName(toolbarAvatar, getAvatarTransitionName(contactId)); setTransitionName(toolbarStatus, getBulbTransitionName(contactId)); @@ -600,16 +597,11 @@ public class ConversationActivity extends BriarActivity @Override public void onSendClick(@Nullable String text, List imageUris) { - if (!imageUris.isEmpty()) { - Toast.makeText(this, "Not yet implemented.", LENGTH_LONG).show(); - textInputView.clearText(); - return; - } - if (isNullOrEmpty(text)) throw new AssertionError(); + if (isNullOrEmpty(text) && imageUris.isEmpty()) + throw new AssertionError(); long timestamp = System.currentTimeMillis(); timestamp = Math.max(timestamp, getMinTimestampForNewMessage()); - if (messagingGroupId == null) loadGroupId(text, timestamp); - else createMessage(text, timestamp); + viewModel.sendMessage(text, imageUris, timestamp); textInputView.clearText(); } @@ -619,48 +611,10 @@ public class ConversationActivity extends BriarActivity return item == null ? 0 : item.getTime() + 1; } - private void loadGroupId(String text, long timestamp) { - runOnDbThread(() -> { - try { - messagingGroupId = - messagingManager.getConversationId(contactId); - createMessage(text, timestamp); - } catch (DbException e) { - logException(LOG, WARNING, e); - } - - }); - } - - private void createMessage(String text, long timestamp) { - cryptoExecutor.execute(() -> { - try { - //noinspection ConstantConditions init in loadGroupId() - storeMessage(privateMessageFactory.createPrivateMessage( - messagingGroupId, timestamp, text, emptyList()), text); - } catch (FormatException e) { - throw new RuntimeException(e); - } - }); - } - - private void storeMessage(PrivateMessage m, String text) { - runOnDbThread(() -> { - try { - long start = now(); - messagingManager.addLocalMessage(m); - logDuration(LOG, "Storing message", start); - Message message = m.getMessage(); - PrivateMessageHeader h = new PrivateMessageHeader( - message.getId(), message.getGroupId(), - message.getTimestamp(), true, false, false, false, - true, emptyList()); - textCache.put(message.getId(), text); - addConversationItem(h.accept(visitor)); - } catch (DbException e) { - logException(LOG, WARNING, e); - } - }); + private void onAddedPrivateMessage(@Nullable PrivateMessageHeader h) { + if (h == null) return; + addConversationItem(h.accept(visitor)); + viewModel.onAddedPrivateMessageSeen(); } private void askToRemoveContact() { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java index 3a8e6e364..b4e158253 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java @@ -5,19 +5,37 @@ import android.arch.lifecycle.AndroidViewModel; import android.arch.lifecycle.LiveData; import android.arch.lifecycle.MutableLiveData; import android.arch.lifecycle.Transformations; +import android.content.ContentResolver; +import android.net.Uri; import android.support.annotation.Nullable; +import android.support.annotation.UiThread; +import org.briarproject.bramble.api.FormatException; +import org.briarproject.bramble.api.Pair; 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.crypto.CryptoExecutor; import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.NoSuchContactException; import org.briarproject.bramble.api.identity.AuthorId; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.sync.GroupId; +import org.briarproject.bramble.api.sync.Message; import org.briarproject.briar.android.util.UiUtils; +import org.briarproject.briar.api.messaging.Attachment; +import org.briarproject.briar.api.messaging.AttachmentHeader; import org.briarproject.briar.api.messaging.MessagingManager; +import org.briarproject.briar.api.messaging.PrivateMessage; +import org.briarproject.briar.api.messaging.PrivateMessageFactory; +import org.briarproject.briar.api.messaging.PrivateMessageHeader; +import java.io.BufferedInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.Executor; import java.util.logging.Logger; @@ -29,6 +47,8 @@ import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.now; +import static org.briarproject.briar.android.conversation.AttachmentController.getContentTypeFromBitmap; +import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce; @NotNullByDefault public class ConversationViewModel extends AndroidViewModel { @@ -38,7 +58,11 @@ public class ConversationViewModel extends AndroidViewModel { @DatabaseExecutor private final Executor dbExecutor; + @CryptoExecutor + private final Executor cryptoExecutor; + private final MessagingManager messagingManager; private final ContactManager contactManager; + private final PrivateMessageFactory privateMessageFactory; private final AttachmentController attachmentController; @Nullable @@ -50,14 +74,24 @@ public class ConversationViewModel extends AndroidViewModel { Transformations.map(contact, UiUtils::getContactDisplayName); private final MutableLiveData contactDeleted = new MutableLiveData<>(); + private final MutableLiveData messagingGroupId = + new MutableLiveData<>(); + private final MutableLiveData addedHeader = + new MutableLiveData<>(); @Inject ConversationViewModel(Application application, @DatabaseExecutor Executor dbExecutor, - ContactManager contactManager, MessagingManager messagingManager) { + @CryptoExecutor Executor cryptoExecutor, + MessagingManager messagingManager, + ContactManager contactManager, + PrivateMessageFactory privateMessageFactory) { super(application); this.dbExecutor = dbExecutor; + this.cryptoExecutor = cryptoExecutor; + this.messagingManager = messagingManager; this.contactManager = contactManager; + this.privateMessageFactory = privateMessageFactory; this.attachmentController = new AttachmentController(messagingManager, application.getResources()); contactDeleted.setValue(false); @@ -100,6 +134,122 @@ public class ConversationViewModel extends AndroidViewModel { }); } + void sendMessage(@Nullable String text, List uris, long timestamp) { + if (messagingGroupId.getValue() == null) loadGroupId(); + observeForeverOnce(messagingGroupId, groupId -> { + if (groupId == null) return; + // calls through to creating and storing the message + // (wouldn't Kotlin's co-routines be nice here?) + storeAttachments(groupId, text, uris, timestamp); + }); + } + + private void loadGroupId() { + if (contactId == null) throw new IllegalStateException(); + dbExecutor.execute(() -> { + try { + messagingGroupId.postValue( + messagingManager.getConversationId(contactId)); + } catch (DbException e) { + logException(LOG, WARNING, e); + } + }); + } + + private void storeAttachments(GroupId groupId, @Nullable String text, + List uris, long timestamp) { + dbExecutor.execute(() -> { + long start = now(); + List attachments = new ArrayList<>(); + List items = new ArrayList<>(); + for (Uri uri : uris) { + Pair pair = + createAttachmentHeader(groupId, uri, timestamp); + if (pair == null) continue; + attachments.add(pair.getFirst()); + items.add(pair.getSecond()); + } + logDuration(LOG, "Storing attachments", start); + createMessage(groupId, text, attachments, items, timestamp); + }); + } + + @Nullable + @DatabaseExecutor + private Pair createAttachmentHeader( + GroupId groupId, Uri uri, long timestamp) { + InputStream is = null; + try { + ContentResolver contentResolver = + getApplication().getContentResolver(); + is = contentResolver.openInputStream(uri); + if (is == null) throw new IOException(); + is = new BufferedInputStream(is); // adds support for reset() + is.mark(Integer.MAX_VALUE); + String contentType = contentResolver.getType(uri); + if (contentType == null) contentType = getContentTypeFromBitmap(is); + AttachmentHeader h = messagingManager + .addLocalAttachment(groupId, timestamp, contentType, is); + AttachmentItem item = attachmentController + .getAttachmentItem(h, new Attachment(is)); + return new Pair<>(h, item); + } catch (DbException | IOException e) { + logException(LOG, WARNING, e); + return null; + } finally { + if (is != null) { + try { + is.close(); + } catch (IOException e) { + logException(LOG, WARNING, e); + } + } + } + } + + private void createMessage(GroupId groupId, @Nullable String text, + List attachments, List aItems, + long timestamp) { + cryptoExecutor.execute(() -> { + try { + // TODO remove when text can be null in the backend + String msgText = text == null ? "null" : text; + PrivateMessage pm = privateMessageFactory + .createPrivateMessage(groupId, timestamp, msgText, + attachments); + attachmentController.put(pm.getMessage().getId(), aItems); + storeMessage(pm, msgText, attachments); + } catch (FormatException e) { + throw new RuntimeException(e); + } + }); + } + + private void storeMessage(PrivateMessage m, @Nullable String text, + List attachments) { + dbExecutor.execute(() -> { + try { + long start = now(); + messagingManager.addLocalMessage(m); + logDuration(LOG, "Storing message", start); + Message message = m.getMessage(); + PrivateMessageHeader h = new PrivateMessageHeader( + message.getId(), message.getGroupId(), + message.getTimestamp(), true, true, false, false, + text != null, attachments); + // TODO add text to cache when available here + addedHeader.postValue(h); + } catch (DbException e) { + logException(LOG, WARNING, e); + } + }); + } + + @UiThread + void onAddedPrivateMessageSeen() { + addedHeader.setValue(null); + } + AttachmentController getAttachmentController() { return attachmentController; } @@ -120,4 +270,8 @@ public class ConversationViewModel extends AndroidViewModel { return contactDeleted; } + LiveData getAddedPrivateMessage() { + return addedHeader; + } + } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java b/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java index fcd7fcb14..b74e8a216 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java @@ -354,4 +354,22 @@ public class UiUtils { }); } + /** + * Same as {@link #observeOnce(LiveData, LifecycleOwner, Observer)}, + * but without a {@link LifecycleOwner}. + * + * Warning: Do NOT call from objects that have a lifecycle. + */ + @MainThread + public static void observeForeverOnce(LiveData liveData, + Observer observer) { + liveData.observeForever(new Observer() { + @Override + public void onChanged(@Nullable T t) { + observer.onChanged(t); + liveData.removeObserver(this); + } + }); + } + } diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java index d7f4997a7..91b8d866c 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java @@ -8,7 +8,7 @@ import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.api.conversation.ConversationManager.ConversationClient; -import java.nio.ByteBuffer; +import java.io.InputStream; @NotNullByDefault public interface MessagingManager extends ConversationClient { @@ -37,7 +37,7 @@ public interface MessagingManager extends ConversationClient { * Stores a local attachment message. */ AttachmentHeader addLocalAttachment(GroupId groupId, long timestamp, - String contentType, ByteBuffer data) throws DbException; + String contentType, InputStream is) throws DbException; /** * Returns the ID of the contact with the given private conversation. 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 bc92e6952..d8c06c4aa 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 @@ -32,7 +32,7 @@ import org.briarproject.briar.api.messaging.PrivateMessageHeader; import org.briarproject.briar.api.messaging.event.PrivateMessageReceivedEvent; import org.briarproject.briar.client.ConversationClientImpl; -import java.nio.ByteBuffer; +import java.io.InputStream; import java.util.ArrayList; import java.util.Collection; import java.util.Map; @@ -152,7 +152,7 @@ class MessagingManagerImpl extends ConversationClientImpl @Override public AttachmentHeader addLocalAttachment(GroupId groupId, long timestamp, - String contentType, ByteBuffer data) { + String contentType, InputStream is) { // TODO add real implementation byte[] b = new byte[MessageId.LENGTH]; new Random().nextBytes(b); From 80ee35d926ef110096eac531081238889a19983b Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 27 Nov 2018 18:41:36 -0200 Subject: [PATCH 6/9] [core] Return fake mini PNG as Attachment instead of throwing exception --- .../briar/messaging/MessagingManagerImpl.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 d8c06c4aa..5011fe46e 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 @@ -32,6 +32,7 @@ import org.briarproject.briar.api.messaging.PrivateMessageHeader; import org.briarproject.briar.api.messaging.event.PrivateMessageReceivedEvent; import org.briarproject.briar.client.ConversationClientImpl; +import java.io.ByteArrayInputStream; import java.io.InputStream; import java.util.ArrayList; import java.util.Collection; @@ -42,6 +43,7 @@ import javax.annotation.concurrent.Immutable; import javax.inject.Inject; import static java.util.Collections.emptyList; +import static org.briarproject.bramble.util.StringUtils.fromHexString; import static org.briarproject.briar.client.MessageTrackerConstants.MSG_KEY_READ; @Immutable @@ -237,7 +239,11 @@ class MessagingManagerImpl extends ConversationClientImpl @Override public Attachment getAttachment(MessageId m) { // TODO add real implementation - throw new IllegalStateException("Not yet implemented"); + byte[] bytes = fromHexString("89504E470D0A1A0A0000000D49484452" + + "000000010000000108060000001F15C4" + + "890000000A49444154789C6300010000" + + "0500010D0A2DB40000000049454E44AE426082"); + return new Attachment(new ByteArrayInputStream(bytes)); } } From e85fbfb9528056491df1b6de14e441db5ec4b58a Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 27 Nov 2018 19:05:05 -0200 Subject: [PATCH 7/9] [android] close InputStream with new IoUtils method --- .../android/conversation/ConversationViewModel.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java index b4e158253..600b58bfe 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java @@ -44,6 +44,7 @@ import javax.inject.Inject; import static java.util.Objects.requireNonNull; import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; +import static org.briarproject.bramble.util.IoUtils.tryToClose; import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.now; @@ -197,13 +198,7 @@ public class ConversationViewModel extends AndroidViewModel { logException(LOG, WARNING, e); return null; } finally { - if (is != null) { - try { - is.close(); - } catch (IOException e) { - logException(LOG, WARNING, e); - } - } + if (is != null) tryToClose(is, LOG, WARNING); } } From 3cfb04b60d10f48382f53f4e157aa38737a6a388 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 7 Dec 2018 16:13:07 -0200 Subject: [PATCH 8/9] Establish some rules for handling InputStreams * Methods shouldn't place any special requirements on the streams passed into them * This implies that if a stream's going to be marked and reset, that should all happen within one method * This also implies that if a method needs to mark and reset a stream, it should wrap the stream in a BufferedInputStream before doing so, rather than requiring a markable stream to be passed in --- .../conversation/AttachmentController.java | 21 ++++++------------- .../conversation/ConversationViewModel.java | 11 +++++----- .../briar/api/messaging/MessagingManager.java | 3 ++- .../briar/messaging/MessagingManagerImpl.java | 4 +++- 4 files changed, 16 insertions(+), 23 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java index b1a37456f..bf28e2458 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java @@ -16,6 +16,7 @@ import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.AttachmentHeader; import org.briarproject.briar.api.messaging.MessagingManager; +import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; @@ -92,14 +93,13 @@ class AttachmentController { /** * Creates {@link AttachmentItem}s from the passed headers and Attachments. - * Note: This marks the {@link Attachment}'s {@link InputStream} - * and closes the streams. + * + * Note: This closes the {@link Attachment}'s {@link InputStream}. */ List getAttachmentItems( List> attachments) { List items = new ArrayList<>(attachments.size()); for (Pair a : attachments) { - a.getSecond().getStream().mark(Integer.MAX_VALUE); AttachmentItem item = getAttachmentItem(a.getFirst(), a.getSecond()); items.add(item); @@ -109,16 +109,14 @@ class AttachmentController { /** * Creates an {@link AttachmentItem} from the {@link Attachment}'s - * {@link InputStream}. - * Note: Requires a resettable InputStream - * with the beginning already marked. - * The stream will be closed when this method returns. + * {@link InputStream} which will be closed when this method returns. */ AttachmentItem getAttachmentItem(AttachmentHeader h, Attachment a) { MessageId messageId = h.getMessageId(); Size size = new Size(); - InputStream is = a.getStream(); + InputStream is = new BufferedInputStream(a.getStream()); + is.mark(Integer.MAX_VALUE); try { // use exif to get size if (h.getContentType().equals("image/jpeg")) { @@ -190,13 +188,6 @@ class AttachmentController { options.outMimeType); } - static String getContentTypeFromBitmap(InputStream is) { - BitmapFactory.Options options = new BitmapFactory.Options(); - options.inJustDecodeBounds = true; - BitmapFactory.decodeStream(is, null, options); - return options.outMimeType; - } - private Size getThumbnailSize(int width, int height, String mimeType) { float widthPercentage = maxWidth / (float) width; float heightPercentage = maxHeight / (float) height; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java index 600b58bfe..707012157 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java @@ -31,7 +31,6 @@ import org.briarproject.briar.api.messaging.PrivateMessage; import org.briarproject.briar.api.messaging.PrivateMessageFactory; import org.briarproject.briar.api.messaging.PrivateMessageHeader; -import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; @@ -48,7 +47,6 @@ import static org.briarproject.bramble.util.IoUtils.tryToClose; import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.now; -import static org.briarproject.briar.android.conversation.AttachmentController.getContentTypeFromBitmap; import static org.briarproject.briar.android.util.UiUtils.observeForeverOnce; @NotNullByDefault @@ -140,7 +138,6 @@ public class ConversationViewModel extends AndroidViewModel { observeForeverOnce(messagingGroupId, groupId -> { if (groupId == null) return; // calls through to creating and storing the message - // (wouldn't Kotlin's co-routines be nice here?) storeAttachments(groupId, text, uris, timestamp); }); } @@ -185,12 +182,14 @@ public class ConversationViewModel extends AndroidViewModel { getApplication().getContentResolver(); is = contentResolver.openInputStream(uri); if (is == null) throw new IOException(); - is = new BufferedInputStream(is); // adds support for reset() - is.mark(Integer.MAX_VALUE); String contentType = contentResolver.getType(uri); - if (contentType == null) contentType = getContentTypeFromBitmap(is); + if (contentType == null) throw new IOException("null content type"); AttachmentHeader h = messagingManager .addLocalAttachment(groupId, timestamp, contentType, is); + is.close(); + // re-open stream to get AttachmentItem + is = contentResolver.openInputStream(uri); + if (is == null) throw new IOException(); AttachmentItem item = attachmentController .getAttachmentItem(h, new Attachment(is)); return new Pair<>(h, item); diff --git a/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java b/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java index 91b8d866c..03bc693eb 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/messaging/MessagingManager.java @@ -8,6 +8,7 @@ import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.api.conversation.ConversationManager.ConversationClient; +import java.io.IOException; import java.io.InputStream; @NotNullByDefault @@ -37,7 +38,7 @@ public interface MessagingManager extends ConversationClient { * Stores a local attachment message. */ AttachmentHeader addLocalAttachment(GroupId groupId, long timestamp, - String contentType, InputStream is) throws DbException; + String contentType, InputStream is) throws DbException, IOException; /** * Returns the ID of the contact with the given private conversation. 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 5011fe46e..223c980a2 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 @@ -33,6 +33,7 @@ import org.briarproject.briar.api.messaging.event.PrivateMessageReceivedEvent; import org.briarproject.briar.client.ConversationClientImpl; import java.io.ByteArrayInputStream; +import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.Collection; @@ -154,8 +155,9 @@ class MessagingManagerImpl extends ConversationClientImpl @Override public AttachmentHeader addLocalAttachment(GroupId groupId, long timestamp, - String contentType, InputStream is) { + String contentType, InputStream is) throws IOException { // TODO add real implementation + if (is.available() == 0) throw new IOException(); byte[] b = new byte[MessageId.LENGTH]; new Random().nextBytes(b); return new AttachmentHeader(new MessageId(b), "image/png"); From 8c6dfaa196205e273843c26f28101073cb029564 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 12 Dec 2018 16:18:43 -0200 Subject: [PATCH 9/9] [android] Use @UiThread instead of @MainThread --- .../java/org/briarproject/briar/android/util/UiUtils.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java b/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java index b74e8a216..b23f24dcd 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java @@ -14,9 +14,9 @@ import android.os.PowerManager; import android.support.annotation.AttrRes; import android.support.annotation.ColorInt; import android.support.annotation.ColorRes; -import android.support.annotation.MainThread; import android.support.annotation.Nullable; import android.support.annotation.RequiresApi; +import android.support.annotation.UiThread; import android.support.design.widget.TextInputLayout; import android.support.v4.app.FragmentManager; import android.support.v4.content.ContextCompat; @@ -342,7 +342,7 @@ public class UiUtils { * If the LiveData's value is available, the {@link Observer} will be * called right away. */ - @MainThread + @UiThread public static void observeOnce(LiveData liveData, LifecycleOwner owner, Observer observer) { liveData.observe(owner, new Observer() { @@ -360,7 +360,7 @@ public class UiUtils { * * Warning: Do NOT call from objects that have a lifecycle. */ - @MainThread + @UiThread public static void observeForeverOnce(LiveData liveData, Observer observer) { liveData.observeForever(new Observer() {