android notif: Keep parsed URL of avatarURL, rather than String.

We already ensure in the "crunchy shell" that the value does in fact parse successfully as a URL; so this just means keeping the result, rather than discarding it. This also allows a couple of bits of logic in the interior to behave more precisely: * Instead of just appending a string like `&s=160` to the URL -- implicitly assuming that the URL already contains a query (`?...`) and doesn't contain a fragment (`#...`) -- use the URL's structure to add to the query what we want. * Drop a `startsWith("http")` condition. This was a crude heuristic check that the URL was a real absolute URL; since cee71e0, no such check has been necessary.
zulip · Mar 15, 2019 · 1620867 · 1620867
1 parent 5027e23
commit 1620867
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 13 deletions.
diff --git a/android/app/src/main/java/com/zulipmobile/notifications/FCMPushNotifications.java b/android/app/src/main/java/com/zulipmobile/notifications/FCMPushNotifications.java
@@ -114,7 +114,7 @@ private static Notification.Builder getNotificationBuilder(
         Recipient recipient = fcmMessage.getRecipient();
         String content = fcmMessage.getContent();
         String senderFullName = fcmMessage.getSender().getFullName();
-        String avatarURL = fcmMessage.getSender().getAvatarURL();
+        URL avatarURL = fcmMessage.getSender().getAvatarURL();
         String time = fcmMessage.getTime();
         int totalMessagesCount = extractTotalMessagesCount(conversations);
 
@@ -137,12 +137,10 @@ private static Notification.Builder getNotificationBuilder(
                 String displayTopic = r.getStream() + " > " + r.getTopic();
                 builder.setSubText("Message on " + displayTopic);
             }
-            if (avatarURL.startsWith("http")) {
-                Bitmap avatar = fetchAvatar(NotificationHelper.sizedURL(context,
-                        avatarURL, 64));
-                if (avatar != null) {
-                    builder.setLargeIcon(avatar);
-                }
+            Bitmap avatar = fetchAvatar(NotificationHelper.sizedURL(context,
+                    avatarURL, 64));
+            if (avatar != null) {
+                builder.setLargeIcon(avatar);
             }
             builder.setStyle(new Notification.BigTextStyle().bigText(content));
         } else {

diff --git a/android/app/src/main/java/com/zulipmobile/notifications/FcmMessage.kt b/android/app/src/main/java/com/zulipmobile/notifications/FcmMessage.kt
@@ -35,7 +35,7 @@ internal data class Identity(
 internal data class Sender(
     val id: Int?,
     val email: String,
-    val avatarURL: String,
+    val avatarURL: URL,
     val fullName: String
 )
 
@@ -163,8 +163,7 @@ internal data class MessageFcmMessage(
                 else -> throw FcmMessageParseException("unexpected recipient_type: $recipientType")
             }
 
-            val avatarURL = data.require("sender_avatar_url")
-            parseUrl(avatarURL, "sender_avatar_url")
+            val avatarURL = data.requireUrl("sender_avatar_url")
 
             return MessageFcmMessage(
                 identity = identity,
@@ -216,6 +215,8 @@ private fun parseInt(s: String, msg: String): Int = try {
     throw FcmMessageParseException("$msg: $s")
 }
 
+private fun Map<String, String>.requireUrl(key: String): URL = parseUrl(require(key), key)
+
 private fun parseUrl(s: String, loc: String): URL = try {
     URL(s)
 } catch (e: MalformedURLException) {

diff --git a/android/app/src/main/java/com/zulipmobile/notifications/NotificationHelper.java b/android/app/src/main/java/com/zulipmobile/notifications/NotificationHelper.java
@@ -44,13 +44,14 @@ static Bitmap fetch(URL url) throws IOException {
         return null;
     }
 
-    static URL sizedURL(Context context, String url, float dpSize) {
+    static URL sizedURL(Context context, URL url, float dpSize) {
         // From http://stackoverflow.com/questions/4605527/
         Resources r = context.getResources();
         float px = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP,
                 dpSize, r.getDisplayMetrics());
+        final String query = url.getQuery() != null ? url.getQuery() + "&s=" + px : "s=" + px;
         try {
-            return new URL(url + "&s=" + px);
+            return new URL(url, "?" + query);
         } catch (MalformedURLException e) {
             Log.e(TAG, "ERROR: " + e.toString());
             return null;

diff --git a/android/app/src/test/java/com/zulipmobile/notifications/FcmMessageTest.kt b/android/app/src/test/java/com/zulipmobile/notifications/FcmMessageTest.kt
@@ -108,7 +108,7 @@ class MessageFcmMessageTest : FcmMessageTestBase() {
                 sender = Sender(
                     id = 123,
                     email = Example.stream["sender_email"]!!,
-                    avatarURL = Example.stream["sender_avatar_url"]!!,
+                    avatarURL = URL(Example.stream["sender_avatar_url"]!!),
                     fullName = Example.stream["sender_full_name"]!!
                 ),
                 zulipMessageId = 12345,