From ffc9e8caff3d02a6330a2d451524c7c57506ff78 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Mon, 11 Oct 2021 14:20:32 -0400 Subject: [PATCH] Add additional unit tests for phone number fuzzy matching. --- .../contacts/sync/FuzzyPhoneNumberHelper.java | 98 +++++++++++-------- .../sync/ArgentinaFuzzyMatcherTest.java | 45 +++++++++ .../sync/FuzzyPhoneNumberHelperTest.java | 16 +-- .../contacts/sync/MexicoFuzzyMatcherTest.java | 45 +++++++++ 4 files changed, 156 insertions(+), 48 deletions(-) create mode 100644 app/src/test/java/org/thoughtcrime/securesms/contacts/sync/ArgentinaFuzzyMatcherTest.java create mode 100644 app/src/test/java/org/thoughtcrime/securesms/contacts/sync/MexicoFuzzyMatcherTest.java diff --git a/app/src/main/java/org/thoughtcrime/securesms/contacts/sync/FuzzyPhoneNumberHelper.java b/app/src/main/java/org/thoughtcrime/securesms/contacts/sync/FuzzyPhoneNumberHelper.java index 0c80dc2085..d1432202f4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/contacts/sync/FuzzyPhoneNumberHelper.java +++ b/app/src/main/java/org/thoughtcrime/securesms/contacts/sync/FuzzyPhoneNumberHelper.java @@ -1,6 +1,7 @@ package org.thoughtcrime.securesms.contacts.sync; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import java.util.Arrays; @@ -20,28 +21,28 @@ import java.util.UUID; */ class FuzzyPhoneNumberHelper { - private final static List FUZZY_VARIANTS = Arrays.asList(new MxFuzzyVariant(), new ArFuzzyVariant()); + private final static List FUZZY_MATCHERS = Arrays.asList(new MexicoFuzzyMatcher(), new ArgentinaFuzzyMatcher()); /** * This should be run on the list of eligible numbers for contact intersection so that we can * create an updated list that has potentially more "fuzzy" number matches in it. */ static @NonNull InputResult generateInput(@NonNull Collection testNumbers, @NonNull Collection storedNumbers) { - Set allNumbers = new HashSet<>(testNumbers); - Map fuzzies = new HashMap<>(); + Set allNumbers = new HashSet<>(testNumbers); + Map originalToVariant = new HashMap<>(); for (String number : testNumbers) { - for (FuzzyVariant fuzzyVariant: FUZZY_VARIANTS) { - if(fuzzyVariant.hasVariants(number)) { - String variant = fuzzyVariant.getVariant(number); + for (FuzzyMatcher matcher : FUZZY_MATCHERS) { + if(matcher.matches(number)) { + String variant = matcher.getVariant(number); if(variant != null && !storedNumbers.contains(variant) && allNumbers.add(variant)) { - fuzzies.put(number, variant); + originalToVariant.put(number, variant); } } } } - return new InputResult(allNumbers, fuzzies); + return new InputResult(allNumbers, originalToVariant); } /** @@ -53,45 +54,57 @@ class FuzzyPhoneNumberHelper { Map allNumbers = new HashMap<>(registeredNumbers); Map rewrites = new HashMap<>(); - for (Map.Entry entry : inputResult.getFuzzies().entrySet()) { - if (registeredNumbers.containsKey(entry.getKey()) && registeredNumbers.containsKey(entry.getValue())) { - for (FuzzyVariant fuzzyVariant: FUZZY_VARIANTS) { - if(fuzzyVariant.hasVariants(entry.getKey())) { - if (fuzzyVariant.isDefaultVariant(entry.getKey())) { - allNumbers.remove(entry.getValue()); + for (Map.Entry entry : inputResult.getMapOfOriginalToVariant().entrySet()) { + String original = entry.getKey(); + String variant = entry.getValue(); + + if (registeredNumbers.containsKey(original) && registeredNumbers.containsKey(variant)) { + for (FuzzyMatcher matcher: FUZZY_MATCHERS) { + if(matcher.matches(original)) { + if (matcher.isPreferredVariant(original)) { + allNumbers.remove(variant); } else { - rewrites.put(entry.getKey(), entry.getValue()); - allNumbers.remove(entry.getKey()); + rewrites.put(original, variant); + allNumbers.remove(original); } } } - } else if (registeredNumbers.containsKey(entry.getValue())) { - rewrites.put(entry.getKey(), entry.getValue()); - allNumbers.remove(entry.getKey()); + } else if (registeredNumbers.containsKey(variant)) { + rewrites.put(original, variant); + allNumbers.remove(original); } } return new OutputResult(allNumbers, rewrites); } - private interface FuzzyVariant { - boolean hasVariants(@NonNull String number); - String getVariant(@NonNull String number); - boolean isDefaultVariant(@NonNull String number); + private interface FuzzyMatcher { + boolean matches(@NonNull String number); + @Nullable String getVariant(@NonNull String number); + boolean isPreferredVariant(@NonNull String number); } - private static class MxFuzzyVariant implements FuzzyVariant { + /** + * Mexico has an optional 1 after their +52 country code, e.g. both of these numbers are valid and map to the same logical number: + * +525512345678 + * +5215512345678 + * + * Mexico used to require the 1, but has since removed the requirement. + */ + @VisibleForTesting + static class MexicoFuzzyMatcher implements FuzzyMatcher { @Override - public boolean hasVariants(@NonNull String number) { + public boolean matches(@NonNull String number) { return number.startsWith("+52") && (number.length() == 13 || number.length() == 14); } @Override - public String getVariant(String number) { + public @Nullable String getVariant(String number) { if(number.startsWith("+521") && number.length() == 14) { return "+52" + number.substring("+521".length()); } + if(number.startsWith("+52") && !number.startsWith("+521") && number.length() == 13) { return "+521" + number.substring("+52".length()); } @@ -100,24 +113,30 @@ class FuzzyPhoneNumberHelper { } @Override - public boolean isDefaultVariant(@NonNull String number) { + public boolean isPreferredVariant(@NonNull String number) { return number.startsWith("+52") && !number.startsWith("+521") && number.length() == 13; } - } - private static class ArFuzzyVariant implements FuzzyVariant { + /** + * Argentina has an optional 9 after their +54 country code, e.g. both of these numbers are valid and map to the same logical number: + * +545512345678 + * +5495512345678 + */ + @VisibleForTesting + static class ArgentinaFuzzyMatcher implements FuzzyMatcher { @Override - public boolean hasVariants(@NonNull String number) { + public boolean matches(@NonNull String number) { return number.startsWith("+54") && (number.length() == 13 || number.length() == 14); } @Override - public String getVariant(String number) { + public @Nullable String getVariant(String number) { if(number.startsWith("+549") && number.length() == 14) { return "+54" + number.substring("+549".length()); } + if(number.startsWith("+54") && !number.startsWith("+549") && number.length() == 13) { return "+549" + number.substring("+54".length()); } @@ -126,28 +145,27 @@ class FuzzyPhoneNumberHelper { } @Override - public boolean isDefaultVariant(@NonNull String number) { - return number.startsWith("+54") && !number.startsWith("+549") && number.length() == 13; + public boolean isPreferredVariant(@NonNull String number) { + return number.startsWith("+549") && number.length() == 14; } - } public static class InputResult { private final Set numbers; - private final Map fuzzies; + private final Map originalToVariant; @VisibleForTesting - InputResult(@NonNull Set numbers, @NonNull Map fuzzies) { - this.numbers = numbers; - this.fuzzies = fuzzies; + InputResult(@NonNull Set numbers, @NonNull Map originalToVariant) { + this.numbers = numbers; + this.originalToVariant = originalToVariant; } public @NonNull Set getNumbers() { return numbers; } - public @NonNull Map getFuzzies() { - return fuzzies; + public @NonNull Map getMapOfOriginalToVariant() { + return originalToVariant; } } diff --git a/app/src/test/java/org/thoughtcrime/securesms/contacts/sync/ArgentinaFuzzyMatcherTest.java b/app/src/test/java/org/thoughtcrime/securesms/contacts/sync/ArgentinaFuzzyMatcherTest.java new file mode 100644 index 0000000000..a722462da9 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/contacts/sync/ArgentinaFuzzyMatcherTest.java @@ -0,0 +1,45 @@ +package org.thoughtcrime.securesms.contacts.sync; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +public class ArgentinaFuzzyMatcherTest { + + @Test + public void matches() { + FuzzyPhoneNumberHelper.ArgentinaFuzzyMatcher subject = new FuzzyPhoneNumberHelper.ArgentinaFuzzyMatcher(); + + assertTrue(subject.matches("+545512345678")); + assertTrue(subject.matches("+5495512345678")); + + assertFalse(subject.matches("+54155123456")); + assertFalse(subject.matches("+54551234567890")); + assertFalse(subject.matches("+535512345678")); + assertFalse(subject.matches("+5315512345678")); + } + + @Test + public void getVariant() { + FuzzyPhoneNumberHelper.ArgentinaFuzzyMatcher subject = new FuzzyPhoneNumberHelper.ArgentinaFuzzyMatcher(); + + assertEquals("+545512345678", subject.getVariant("+5495512345678")); + assertEquals("+5495512345678", subject.getVariant("+545512345678")); + + assertNull(subject.getVariant("")); + assertNull(subject.getVariant("+535512345678")); + } + + @Test + public void isPreferredVariant() { + FuzzyPhoneNumberHelper.ArgentinaFuzzyMatcher subject = new FuzzyPhoneNumberHelper.ArgentinaFuzzyMatcher(); + + assertTrue(subject.isPreferredVariant("+5495512345678")); + + assertFalse(subject.isPreferredVariant("+545512345678")); + assertFalse(subject.isPreferredVariant("+54551234567")); + } +} diff --git a/app/src/test/java/org/thoughtcrime/securesms/contacts/sync/FuzzyPhoneNumberHelperTest.java b/app/src/test/java/org/thoughtcrime/securesms/contacts/sync/FuzzyPhoneNumberHelperTest.java index 54429554c2..d898b6c81e 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/contacts/sync/FuzzyPhoneNumberHelperTest.java +++ b/app/src/test/java/org/thoughtcrime/securesms/contacts/sync/FuzzyPhoneNumberHelperTest.java @@ -32,7 +32,7 @@ public class FuzzyPhoneNumberHelperTest { assertEquals(2, result.getNumbers().size()); assertTrue(result.getNumbers().containsAll(setOf(US_A, US_B))); - assertTrue(result.getFuzzies().isEmpty()); + assertTrue(result.getMapOfOriginalToVariant().isEmpty()); } @Test @@ -41,7 +41,7 @@ public class FuzzyPhoneNumberHelperTest { assertEquals(3, result.getNumbers().size()); assertTrue(result.getNumbers().containsAll(setOf(US_A, MX_A_1, MX_A))); - assertEquals(MX_A, result.getFuzzies().get(MX_A_1)); + assertEquals(MX_A, result.getMapOfOriginalToVariant().get(MX_A_1)); } @Test @@ -50,7 +50,7 @@ public class FuzzyPhoneNumberHelperTest { assertEquals(2, result.getNumbers().size()); assertTrue(result.getNumbers().containsAll(setOf(US_A, MX_A_1))); - assertTrue(result.getFuzzies().isEmpty()); + assertTrue(result.getMapOfOriginalToVariant().isEmpty()); } @Test @@ -59,7 +59,7 @@ public class FuzzyPhoneNumberHelperTest { assertEquals(3, result.getNumbers().size()); assertTrue(result.getNumbers().containsAll(setOf(US_A, MX_A_1, MX_A))); - assertEquals(MX_A_1, result.getFuzzies().get(MX_A)); + assertEquals(MX_A_1, result.getMapOfOriginalToVariant().get(MX_A)); } @Test @@ -68,7 +68,7 @@ public class FuzzyPhoneNumberHelperTest { assertEquals(2, result.getNumbers().size()); assertTrue(result.getNumbers().containsAll(setOf(US_A, MX_A))); - assertTrue(result.getFuzzies().isEmpty()); + assertTrue(result.getMapOfOriginalToVariant().isEmpty()); } @Test @@ -77,7 +77,7 @@ public class FuzzyPhoneNumberHelperTest { assertEquals(3, result.getNumbers().size()); assertTrue(result.getNumbers().containsAll(setOf(US_A, MX_A_1, MX_A))); - assertTrue(result.getFuzzies().isEmpty()); + assertTrue(result.getMapOfOriginalToVariant().isEmpty()); } @Test @@ -86,7 +86,7 @@ public class FuzzyPhoneNumberHelperTest { assertEquals(3, result.getNumbers().size()); assertTrue(result.getNumbers().containsAll(setOf(US_A, MX_A_1, MX_A))); - assertTrue(result.getFuzzies().isEmpty()); + assertTrue(result.getMapOfOriginalToVariant().isEmpty()); } @Test @@ -95,7 +95,7 @@ public class FuzzyPhoneNumberHelperTest { assertEquals(3, result.getNumbers().size()); assertTrue(result.getNumbers().containsAll(setOf(US_A, MX_A_1, MX_A))); - assertTrue(result.getFuzzies().isEmpty()); + assertTrue(result.getMapOfOriginalToVariant().isEmpty()); } @Test diff --git a/app/src/test/java/org/thoughtcrime/securesms/contacts/sync/MexicoFuzzyMatcherTest.java b/app/src/test/java/org/thoughtcrime/securesms/contacts/sync/MexicoFuzzyMatcherTest.java new file mode 100644 index 0000000000..358b526b67 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/contacts/sync/MexicoFuzzyMatcherTest.java @@ -0,0 +1,45 @@ +package org.thoughtcrime.securesms.contacts.sync; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +public class MexicoFuzzyMatcherTest { + + @Test + public void matches() { + FuzzyPhoneNumberHelper.MexicoFuzzyMatcher subject = new FuzzyPhoneNumberHelper.MexicoFuzzyMatcher(); + + assertTrue(subject.matches("+525512345678")); + assertTrue(subject.matches("+5215512345678")); + + assertFalse(subject.matches("+52155123456")); + assertFalse(subject.matches("+52551234567890")); + assertFalse(subject.matches("+535512345678")); + assertFalse(subject.matches("+5315512345678")); + } + + @Test + public void getVariant() { + FuzzyPhoneNumberHelper.MexicoFuzzyMatcher subject = new FuzzyPhoneNumberHelper.MexicoFuzzyMatcher(); + + assertEquals("+525512345678", subject.getVariant("+5215512345678")); + assertEquals("+5215512345678", subject.getVariant("+525512345678")); + + assertNull(subject.getVariant("")); + assertNull(subject.getVariant("+535512345678")); + } + + @Test + public void isPreferredVariant() { + FuzzyPhoneNumberHelper.MexicoFuzzyMatcher subject = new FuzzyPhoneNumberHelper.MexicoFuzzyMatcher(); + + assertTrue(subject.isPreferredVariant("+525512345678")); + + assertFalse(subject.isPreferredVariant("+5215512345678")); + assertFalse(subject.isPreferredVariant("+52551234567")); + } +}