Speculative fix: ensure avatars are not created with empty email addresses

b/17802490

I was unable to recreate this bug using 5 different renamed gmail accounts.
But, the code lends itself to locating the bug with a reasonable amount of
certainty.

ContactRequest throws IllegalArgumentException if it is created with a null
or "" email address.

SendersView.handlePriority(...) houses the algorithm that chooses the email
address used as the avatar for the TL entry. The algorithm already avoided
choosing null email addresses, but there was at least one code path through
the algorithm that would accept an empty email address.

Specifically, if all messages were read, and the last message was sent by
someone with an empty email address (obviously this represents bad data),
the algorithm would still happily choose that sender.

If this case happens, we do something the algorithm has done since time
immemorial: set the email address to the *name* of the particpant (which
is not empty). This will produce a failed avatar lookup and result in a
letter tile or generic avatar if the letter is not in A-Z.

Change-Id: I0a0ad0e5e66cc2316ef188b833485daa3c90c4b6

1 files changed, 44 insertions, 0 deletions

diff --git a/tests/src/com/android/mail/browse/SendersFormattingTests.java b/tests/src/com/android/mail/browse/SendersFormattingTests.java
index cbfb7acfc..14b31ec7c 100644
--- a/tests/src/com/android/mail/browse/SendersFormattingTests.java
+++ b/tests/src/com/android/mail/browse/SendersFormattingTests.java
@@ -139,10 +139,31 @@ public class SendersFormattingTests extends AndroidTestCase {
         SendersView.format(getContext(), conv, "", 100, styledSenders, displayableSenderNames,
                 senderAvatarModel, account, false, false);
 
+        // b is the first unread message with a valid email address
         assertEquals("b@b.com", senderAvatarModel.getEmailAddress());
         assertEquals("b", senderAvatarModel.getName());
     }
 
+    public void testSenderAvatarDoesNotChooseEmptyEmailAddress() {
+        final ConversationInfo conv = createConversationInfo();
+        conv.addParticipant(new ParticipantInfo("a", "a@a.com", 0, true));
+        conv.addParticipant(new ParticipantInfo("b", "", 0, false));
+        conv.addParticipant(new ParticipantInfo("c", "c@c.com", 0, false));
+
+        final ArrayList<SpannableString> styledSenders = Lists.newArrayList();
+        final ArrayList<String> displayableSenderNames = Lists.newArrayList();
+        final ConversationItemViewModel.SenderAvatarModel senderAvatarModel =
+                new ConversationItemViewModel.SenderAvatarModel();
+
+        final Account account = createAccount();
+        SendersView.format(getContext(), conv, "", 100, styledSenders, displayableSenderNames,
+                senderAvatarModel, account, false, false);
+
+        // b is unread but has an invalid email address so email address is set to the name
+        assertEquals("b", senderAvatarModel.getEmailAddress());
+        assertEquals("b", senderAvatarModel.getName());
+    }
+
     public void testSenderAvatarIsLastSenderIfAllMessagesAreRead() {
         final ConversationInfo conv = createConversationInfo();
         conv.addParticipant(new ParticipantInfo("a", "a@a.com", 0, true));
@@ -158,10 +179,31 @@ public class SendersFormattingTests extends AndroidTestCase {
         SendersView.format(getContext(), conv, "", 100, styledSenders, displayableSenderNames,
                 senderAvatarModel, account, false, false);
 
+        // all are read, so c is chosen because it is the last sender
         assertEquals("c@c.com", senderAvatarModel.getEmailAddress());
         assertEquals("c", senderAvatarModel.getName());
     }
 
+    public void testSenderAvatarIsLastSenderWithValidEmailAddressIfAllMessagesAreRead() {
+        final ConversationInfo conv = createConversationInfo();
+        conv.addParticipant(new ParticipantInfo("a", "a@a.com", 0, true));
+        conv.addParticipant(new ParticipantInfo("b", "b@b.com", 0, true));
+        conv.addParticipant(new ParticipantInfo("c", "", 0, true));
+
+        final ArrayList<SpannableString> styledSenders = Lists.newArrayList();
+        final ArrayList<String> displayableSenderNames = Lists.newArrayList();
+        final ConversationItemViewModel.SenderAvatarModel senderAvatarModel =
+                new ConversationItemViewModel.SenderAvatarModel();
+
+        final Account account = createAccount();
+        SendersView.format(getContext(), conv, "", 100, styledSenders, displayableSenderNames,
+                senderAvatarModel, account, false, false);
+
+        // all are read, c has an invalid email address, so email address is set to the name
+        assertEquals("c", senderAvatarModel.getEmailAddress());
+        assertEquals("c", senderAvatarModel.getName());
+    }
+
     public void testSenderAvatarIsLastSenderThatIsNotTheCurrentAccountIfAllMessagesAreRead() {
         final ConversationInfo conv = createConversationInfo();
         conv.addParticipant(new ParticipantInfo("a", "a@a.com", 0, true));
@@ -178,6 +220,7 @@ public class SendersFormattingTests extends AndroidTestCase {
         SendersView.format(getContext(), conv, "", 100, styledSenders, displayableSenderNames,
                 senderAvatarModel, account, false, false);
 
+        // c is the last sender, but it is the current account, so b is chosen instead
         assertEquals("b@b.com", senderAvatarModel.getEmailAddress());
         assertEquals("b", senderAvatarModel.getName());
     }
@@ -196,6 +239,7 @@ public class SendersFormattingTests extends AndroidTestCase {
         SendersView.format(getContext(), conv, "", 100, styledSenders, displayableSenderNames,
                 senderAvatarModel, account, false, false);
 
+        // only one sender exists and it is the current account, so the current account is chosen
         assertEquals("fflintstone@example.com", senderAvatarModel.getEmailAddress());
         assertEquals("Fred Flintstone", senderAvatarModel.getName());
     }