fixup! Clean up any invalid group V1 ids in database.

signalapp · Nov 30, 2020 · cce8cdc · cce8cdc
1 parent 834c2c2
commit cce8cdc
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 12 deletions.
diff --git a/...org/thoughtcrime/securesms/jobmanager/migrations/RecipientIdFollowUpJobMigrationTest.java b/...org/thoughtcrime/securesms/jobmanager/migrations/RecipientIdFollowUpJobMigrationTest.java
@@ -31,15 +31,15 @@ public void init() {
   @Test
   public void migrate_requestGroupInfoJob_good() throws Exception {
     JobData testData = new JobData("RequestGroupInfoJob", null, new Data.Builder().putString("source", "1")
-                                                                                  .putString("group_id", "__textsecure_group__!abcd")
+                                                                                  .putString("group_id", "__textsecure_group__!abcdef0123456789abcdef0123456789")
                                                                                   .build());
     RecipientIdFollowUpJobMigration subject   = new RecipientIdFollowUpJobMigration();
     JobData                         converted = subject.migrate(testData);
 
     assertEquals("RequestGroupInfoJob", converted.getFactoryKey());
     assertNull(converted.getQueueKey());
     assertEquals("1", converted.getData().getString("source"));
-    assertEquals("__textsecure_group__!abcd", converted.getData().getString("group_id"));
+    assertEquals("__textsecure_group__!abcdef0123456789abcdef0123456789", converted.getData().getString("group_id"));
 
     new RequestGroupInfoJob.Factory().create(mock(Job.Parameters.class), converted.getData());
   }

diff --git a/...st/java/org/thoughtcrime/securesms/jobmanager/migrations/RecipientIdJobMigrationTest.java b/...st/java/org/thoughtcrime/securesms/jobmanager/migrations/RecipientIdJobMigrationTest.java
@@ -24,7 +24,6 @@
 import org.thoughtcrime.securesms.jobs.PushTextSendJob;
 import org.thoughtcrime.securesms.jobs.RequestGroupInfoJob;
 import org.thoughtcrime.securesms.jobs.RetrieveProfileAvatarJob;
-import org.thoughtcrime.securesms.jobs.RetrieveProfileJob;
 import org.thoughtcrime.securesms.jobs.SendDeliveryReceiptJob;
 import org.thoughtcrime.securesms.jobs.SmsSendJob;
 import org.thoughtcrime.securesms.recipients.Recipient;
@@ -88,7 +87,7 @@ public void migrate_multiDeviceViewOnceOpenJob() throws Exception {
   @Test
   public void migrate_requestGroupInfoJob() throws Exception {
     JobData testData = new JobData("RequestGroupInfoJob", null, new Data.Builder().putString("source", "+16101234567")
-                                                                                  .putString("group_id", "__textsecure_group__!abcd")
+                                                                                  .putString("group_id", "__textsecure_group__!abcdef0123456789abcdef0123456789")
                                                                                   .build());
     mockRecipientResolve("+16101234567", 1);
 
@@ -98,7 +97,7 @@ public void migrate_requestGroupInfoJob() throws Exception {
     assertEquals("RequestGroupInfoJob", converted.getFactoryKey());
     assertNull(converted.getQueueKey());
     assertEquals("1", converted.getData().getString("source"));
-    assertEquals("__textsecure_group__!abcd", converted.getData().getString("group_id"));
+    assertEquals("__textsecure_group__!abcdef0123456789abcdef0123456789", converted.getData().getString("group_id"));
 
     new RequestGroupInfoJob.Factory().create(mock(Job.Parameters.class), converted.getData());
   }
@@ -185,7 +184,7 @@ public void migrate_pushGroupSendJob_nonNull() throws Exception {
   @Test
   public void migrate_pushGroupUpdateJob() throws Exception {
     JobData testData = new JobData("PushGroupUpdateJob", null, new Data.Builder().putString("source", "+16101234567")
-                                                                                 .putString("group_id", "__textsecure_group__!abcd")
+                                                                                 .putString("group_id", "__textsecure_group__!abcdef0123456789abcdef0123456789")
                                                                                  .build());
     mockRecipientResolve("+16101234567", 1);
 
@@ -195,7 +194,7 @@ public void migrate_pushGroupUpdateJob() throws Exception {
     assertEquals("PushGroupUpdateJob", converted.getFactoryKey());
     assertNull(converted.getQueueKey());
     assertEquals("1", converted.getData().getString("source"));
-    assertEquals("__textsecure_group__!abcd", converted.getData().getString("group_id"));
+    assertEquals("__textsecure_group__!abcdef0123456789abcdef0123456789", converted.getData().getString("group_id"));
 
     new PushGroupUpdateJob.Factory().create(mock(Job.Parameters.class), converted.getData());
   }

diff --git a/app/src/test/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMergerTest.java b/app/src/test/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMergerTest.java
@@ -15,6 +15,7 @@
 import static org.powermock.api.mockito.PowerMockito.mock;
 import static org.powermock.api.mockito.PowerMockito.when;
 import static org.thoughtcrime.securesms.testutil.TestHelpers.byteArray;
+import static org.thoughtcrime.securesms.testutil.ZkGroupLibraryUtil.assumeZkGroupSupportedOnOS;
 
 public final class GroupV1ConflictMergerTest {
 
@@ -27,13 +28,13 @@ public final class GroupV1ConflictMergerTest {
 
   @Test
   public void merge_alwaysPreferRemote() {
-    SignalGroupV1Record remote = new SignalGroupV1Record.Builder(byteArray(1), byteArray(100))
+    SignalGroupV1Record remote = new SignalGroupV1Record.Builder(byteArray(1), byteArray(100, 16))
                                                         .setBlocked(false)
                                                         .setProfileSharingEnabled(false)
                                                         .setArchived(false)
                                                         .setForcedUnread(false)
                                                         .build();
-    SignalGroupV1Record local  = new SignalGroupV1Record.Builder(byteArray(2), byteArray(100))
+    SignalGroupV1Record local  = new SignalGroupV1Record.Builder(byteArray(2), byteArray(100, 16))
                                                         .setBlocked(true)
                                                         .setProfileSharingEnabled(true)
                                                         .setArchived(true)
@@ -43,7 +44,7 @@ public void merge_alwaysPreferRemote() {
     SignalGroupV1Record merged = new GroupV1ConflictMerger(Collections.singletonList(local), id -> false).merge(remote, local, KEY_GENERATOR);
 
     assertArrayEquals(remote.getId().getRaw(), merged.getId().getRaw());
-    assertArrayEquals(byteArray(100), merged.getGroupId());
+    assertArrayEquals(byteArray(100, 16), merged.getGroupId());
     assertFalse(merged.isProfileSharingEnabled());
     assertFalse(merged.isBlocked());
     assertFalse(merged.isArchived());
@@ -52,12 +53,12 @@ public void merge_alwaysPreferRemote() {
 
   @Test
   public void merge_returnRemoteIfEndResultMatchesRemote() {
-    SignalGroupV1Record remote = new SignalGroupV1Record.Builder(byteArray(1), byteArray(100))
+    SignalGroupV1Record remote = new SignalGroupV1Record.Builder(byteArray(1), byteArray(100, 16))
                                                         .setBlocked(false)
                                                         .setProfileSharingEnabled(true)
                                                         .setArchived(true)
                                                         .build();
-    SignalGroupV1Record local  = new SignalGroupV1Record.Builder(byteArray(2), byteArray(100))
+    SignalGroupV1Record local  = new SignalGroupV1Record.Builder(byteArray(2), byteArray(100, 16))
                                                         .setBlocked(true)
                                                         .setProfileSharingEnabled(false)
                                                         .setArchived(false)
@@ -70,6 +71,8 @@ public void merge_returnRemoteIfEndResultMatchesRemote() {
 
   @Test
   public void merge_excludeBadGroupId() {
+    assumeZkGroupSupportedOnOS();
+
     SignalGroupV1Record badRemote  = new SignalGroupV1Record.Builder(byteArray(1), badGroupKey(99))
                                                             .setBlocked(false)
                                                             .setProfileSharingEnabled(true)
@@ -89,6 +92,8 @@ public void merge_excludeBadGroupId() {
 
   @Test
   public void merge_excludeMigratedGroupId() {
+    assumeZkGroupSupportedOnOS();
+
     GroupId.V1 v1Id = GroupId.v1orThrow(groupKey(1));
     GroupId.V2 v2Id = v1Id.deriveV2MigrationGroupId();
 

diff --git a/app/src/test/java/org/thoughtcrime/securesms/testutil/ZkGroupLibraryUtil.java b/app/src/test/java/org/thoughtcrime/securesms/testutil/ZkGroupLibraryUtil.java
@@ -0,0 +1,42 @@
+package org.thoughtcrime.securesms.testutil;
+
+import org.signal.zkgroup.internal.Native;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeNoException;
+
+public final class ZkGroupLibraryUtil {
+
+  private ZkGroupLibraryUtil() {
+  }
+
+  /**
+   * Attempts to initialize the ZkGroup Native class, which will load the native binaries.
+   * <p>
+   * If that fails to link, then on Unix, it will fail as we rely on that for CI.
+   * <p>
+   * If that fails to link, and it's not Unix, it will skip the test via assumption violation.
+   */
+  public static void assumeZkGroupSupportedOnOS() {
+    try {
+      Class.forName(Native.class.getName());
+    } catch (ClassNotFoundException e) {
+      fail();
+    } catch (UnsatisfiedLinkError | NoClassDefFoundError e) {
+      String osName = System.getProperty("os.name");
+
+      if (isUnix(osName)) {
+        fail("Not able to link native ZkGroup on a key OS: " + osName);
+      } else {
+        assumeNoException("Not able to link native ZkGroup on this operating system: " + osName, e);
+      }
+    }
+  }
+
+  private static boolean isUnix(String osName) {
+    assertNotNull(osName);
+    osName = osName.toLowerCase();
+    return osName.contains("nix") || osName.contains("nux") || osName.contains("aix");
+  }
+}