android notif: Group "recipient" fields in an ADT, to crunchify more.

This makes our "crunchy shell" crunchy in an additional area where up
to this point we'd been soft (and prone to crashing, or other buggy
behavior, on surprises): now the "stream" and "topic" fields are
required for a stream message.

Perhaps of more importance in this case: it makes the interrelationships
of all these fields explicit, making the application code that refers
to them easier to reason about.

Author: gnprice

android/app/src/main/java/com/zulipmobile/notifications/FCMPushNotifications.java

@@ -143,13 +143,11 @@ private static Notification.Builder getNotificationBuilder(
 
         builder.setAutoCancel(true);
 
-        String type = fcmMessage.getRecipientType();
+        Recipient recipient = fcmMessage.getRecipient();
         String content = fcmMessage.getContent();
         String senderFullName = fcmMessage.getSenderFullName();
         String avatarURL = fcmMessage.getAvatarURL();
         String time = fcmMessage.getTime();
-        String stream = fcmMessage.getStream();
-        String topic = fcmMessage.getTopic();
         int totalMessagesCount = extractTotalMessagesCount(conversations);
 
         if (BuildConfig.DEBUG) {
@@ -166,8 +164,9 @@ private static Notification.Builder getNotificationBuilder(
                 builder.setContentTitle(senderFullName);
             }
             builder.setContentText(content);
-            if (type.equals("stream")) {
-                String displayTopic = stream + " > " + topic;
+            if (recipient instanceof Recipient.Stream) {
+                Recipient.Stream r = (Recipient.Stream) recipient;
+                String displayTopic = r.getStream() + " > " + r.getTopic();
                 builder.setSubText("Message on " + displayTopic);
             }
             if (avatarURL.startsWith("http")) {

android/app/src/main/java/com/zulipmobile/notifications/MessageFcmMessage.kt

@@ -4,6 +4,24 @@ import android.os.Bundle
 import java.net.MalformedURLException
 import java.net.URL
 
+/**
+ * Data identifying where a Zulip message was sent.
+ */
+internal sealed class Recipient {
+    /** A 1:1 private message.  Must have been sent to this user, so nothing more to say. */
+    object Pm : Recipient()
+
+    /**
+     * A group PM.
+     *
+     * pmUsers: the user IDs of all users in the conversation, sorted,
+     *     in ASCII decimal, comma-separated.
+     */
+    data class GroupPm(val pmUsers: String) : Recipient()
+
+    /** A stream message. */
+    data class Stream(val stream: String, val topic: String) : Recipient()
+}
 
 /**
  * Parsed version of an FCM message of type `message`.
@@ -28,13 +46,8 @@ internal data class MessageFcmMessage(
         val senderFullName: String,
         val avatarURL: String,
 
-        val recipientType: String,
-        val isGroupMessage: Boolean,
-        val stream: String?,
-        val topic: String?,
-        val pmUsers: String?,
-
         val zulipMessageId: Int,
+        val recipient: Recipient,
         val content: String,
         val time: String,
 
@@ -43,14 +56,15 @@ internal data class MessageFcmMessage(
     companion object {
         fun fromBundle(bundle: Bundle): MessageFcmMessage {
             val recipientType = bundle.requireString("recipient_type")
-            when (recipientType) {
-                "stream" -> {
-                    bundle.requireString("stream")
-                    bundle.requireString("topic")
-                }
-                "private" -> {
-                    // "pm_users" optional -- present just for group PMs
-                }
+            val recipient = when (recipientType) {
+                "stream" ->
+                    Recipient.Stream(
+                            bundle.requireString("stream"),
+                            bundle.requireString("topic"))
+                "private" ->
+                    bundle.getString("pm_users")?.let {
+                        Recipient.GroupPm(it)
+                    } ?: Recipient.Pm
                 else -> throw FcmMessageParseException("unexpected recipient_type: $recipientType")
             }
 
@@ -66,13 +80,8 @@ internal data class MessageFcmMessage(
                 senderFullName = bundle.requireString("sender_full_name"),
                 avatarURL = avatarURL,
 
-                recipientType = recipientType,
-                isGroupMessage = recipientType == "private" && bundle.getString("pm_users") != null,
-                stream = bundle.getString("stream"),
-                topic = bundle.getString("topic"),
-                pmUsers = bundle.getString("pm_users"),
-
                 zulipMessageId = bundle.requireIntString("zulip_message_id"),
+                recipient = recipient,
                 content = bundle.requireString("content"),
                 time = bundle.requireString("time"),
 

android/app/src/main/java/com/zulipmobile/notifications/NotificationHelper.java

@@ -90,10 +90,13 @@ static int extractTotalMessagesCount(ConversationMap conversations) {
      * private message - fullName:Email:'private'
      */
     private static String buildKeyString(MessageFcmMessage fcmMessage) {
-        if (fcmMessage.getRecipientType().equals("stream"))
-            return String.format("%s:%s:stream", fcmMessage.getSenderFullName(), fcmMessage.getStream());
-        else if (fcmMessage.isGroupMessage()) {
-            return String.format("%s:%s:group", fcmMessage.getSenderFullName(), fcmMessage.getPmUsers());
+        final Recipient recipient = fcmMessage.getRecipient();
+        if (recipient instanceof Recipient.Stream)
+            return String.format("%s:%s:stream", fcmMessage.getSenderFullName(),
+                    ((Recipient.Stream) recipient).getStream());
+        else if (recipient instanceof Recipient.GroupPm) {
+            return String.format("%s:%s:group", fcmMessage.getSenderFullName(),
+                    ((Recipient.GroupPm) recipient).getPmUsers());
         } else {
             return String.format("%s:%s:private", fcmMessage.getSenderFullName(), fcmMessage.getEmail());
         }