Repository: relayrides/pushy (java)

Issue:
Auto-collapsing alert dictionary into string
Long ago there was an advice in Apple's documentation:
> Don't specify a dictionary as the value of alert if the dictionary only has the body property

And Pushy's ApnsPayloadBuilder does exactly that -- it collapses `alert:{body:"text"}` into `alert:"text"`.
But now [the page containing the advice](https://developer.apple.com/library/content/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/Chapters/TheNotificationPayload.html#//apple_ref/doc/uid/TP40008194-CH107-SW10) is no longer available and [new version of documentation](https://developer.apple.com/library/content/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/PayloadKeyReference.html#//apple_ref/doc/uid/TP40008194-CH17-SW2) says the opposite:
> The **preferred value** for this key is a **dictionary**, the keys for which are listed in Table 9-2.

So I would like to have some kind of "switch" to enable/disable auto-collapsing of the `alert` dictionary into a string.

Pull request description:
Prefer dictionary representation for alerts (closes #390)
As discussed in #390, APNs now prefers that we send simple alerts as dictionaries rather than strings. There was some discussion of adding an optional "represent as string" flag, which would allow us to save 8 bytes out of a 4096-byte payload in some cases. My wager is that the complexity just isn't worth it, though.

Patch:
diff --git a/pushy/src/main/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilder.java b/pushy/src/main/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilder.java
index 34af285e..5c84d867 100644
--- a/pushy/src/main/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilder.java
+++ b/pushy/src/main/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilder.java
@@ -59,6 +59,8 @@ public class ApnsPayloadBuilder {
     private boolean contentAvailable = false;
     private boolean mutableContent = false;
 
+    private boolean preferStringRepresentationForAlerts = false;
+
     private final CharArrayWriter buffer = new CharArrayWriter(DEFAULT_PAYLOAD_SIZE / 4);
 
     private static final String APS_KEY = "aps";
@@ -97,6 +99,21 @@ public class ApnsPayloadBuilder {
      */
     public static final String DEFAULT_SOUND_FILENAME = "default";
 
+    /**
+     * Sets whether this payload builder will attempt to represent alerts as strings when possible. Older versions of
+     * the APNs specification recommended representing alerts as strings when only a literal alert body was provided,
+     * but recent versions recommend representing alerts as dictionaries regardless. This method is provided primarily
+     * for backward-compatibility. By default, payload builders will always represent alerts as dictionaries.
+     *
+     * @return a reference to this payload builder
+     *
+     * @since 0.8.2
+     */
+    public ApnsPayloadBuilder setPreferStringRepresentationForAlerts(final boolean preferStringRepresentationForAlerts) {
+        this.preferStringRepresentationForAlerts = preferStringRepresentationForAlerts;
+        return this;
+    }
+
     /**
      * <p>Sets the literal text of the alert message to be shown for the push notification. Clears any previously-set
      * localized alert message key and arguments.</p>
@@ -442,10 +459,62 @@ public class ApnsPayloadBuilder {
                 aps.put(MUTABLE_CONTENT_KEY, 1);
             }
 
-            final Object alertObject = this.createAlertObject();
+            final Map<String, Object> alert = new HashMap<>();
+            {
+                if (this.alertBody != null) {
+                    alert.put(ALERT_BODY_KEY, this.alertBody);
+                }
+
+                if (this.alertTitle != null) {
+                    alert.put(ALERT_TITLE_KEY, this.alertTitle);
+                }
+
+                if (this.alertSubtitle != null) {
+                    alert.put(ALERT_SUBTITLE_KEY, this.alertSubtitle);
+                }
+
+                if (this.showActionButton) {
+                    if (this.localizedActionButtonKey != null) {
+                        alert.put(ACTION_LOC_KEY, this.localizedActionButtonKey);
+                    }
+                } else {
+                    // To hide the action button, the key needs to be present, but the value needs to be null
+                    alert.put(ACTION_LOC_KEY, null);
+                }
+
+                if (this.localizedAlertKey != null) {
+                    alert.put(ALERT_LOC_KEY, this.localizedAlertKey);
 
-            if (alertObject != null) {
-                aps.put(ALERT_KEY, alertObject);
+                    if (this.localizedAlertArguments != null) {
+                        alert.put(ALERT_ARGS_KEY, Arrays.asList(this.localizedAlertArguments));
+                    }
+                }
+
+                if (this.localizedAlertTitleKey != null) {
+                    alert.put(ALERT_TITLE_LOC_KEY, this.localizedAlertTitleKey);
+
+                    if (this.localizedAlertTitleArguments != null) {
+                        alert.put(ALERT_TITLE_ARGS_KEY, Arrays.asList(this.localizedAlertTitleArguments));
+                    }
+                }
+
+                if (this.localizedAlertSubtitleKey != null) {
+                    alert.put(ALERT_SUBTITLE_LOC_KEY, this.localizedAlertSubtitleKey);
+
+                    if (this.localizedAlertSubtitleArguments != null) {
+                        alert.put(ALERT_SUBTITLE_ARGS_KEY, Arrays.asList(this.localizedAlertSubtitleArguments));
+                    }
+                }
+
+                if (this.launchImageFileName != null) {
+                    alert.put(LAUNCH_IMAGE_KEY, this.launchImageFileName);
+                }
+            }
+
+            if (alert.size() == 1 && alert.containsKey(ALERT_BODY_KEY) && this.preferStringRepresentationForAlerts) {
+                aps.put(ALERT_KEY, alert.get(ALERT_BODY_KEY));
+            } else if (!alert.isEmpty()) {
+                aps.put(ALERT_KEY, alert);
             }
 
             payload.put(APS_KEY, aps);
@@ -523,99 +592,6 @@ public class ApnsPayloadBuilder {
         }
     }
 
-    private Object createAlertObject() {
-        if (this.hasAlertContent()) {
-            if (this.shouldRepresentAlertAsString()) {
-                return this.alertBody;
-            } else {
-                final HashMap<String, Object> alert = new HashMap<String, Object>();
-
-                if (this.alertBody != null) {
-                    alert.put(ALERT_BODY_KEY, this.alertBody);
-                }
-
-                if (this.alertTitle != null) {
-                    alert.put(ALERT_TITLE_KEY, this.alertTitle);
-                }
-
-                if (this.alertSubtitle != null) {
-                    alert.put(ALERT_SUBTITLE_KEY, this.alertSubtitle);
-                }
-
-                if (this.showActionButton) {
-                    if (this.localizedActionButtonKey != null) {
-                        alert.put(ACTION_LOC_KEY, this.localizedActionButtonKey);
-                    }
-                } else {
-                    // To hide the action button, the key needs to be present, but the value needs to be null
-                    alert.put(ACTION_LOC_KEY, null);
-                }
-
-                if (this.localizedAlertKey != null) {
-                    alert.put(ALERT_LOC_KEY, this.localizedAlertKey);
-
-                    if (this.localizedAlertArguments != null) {
-                        alert.put(ALERT_ARGS_KEY, Arrays.asList(this.localizedAlertArguments));
-                    }
-                }
-
-                if (this.localizedAlertTitleKey != null) {
-                    alert.put(ALERT_TITLE_LOC_KEY, this.localizedAlertTitleKey);
-
-                    if (this.localizedAlertTitleArguments != null) {
-                        alert.put(ALERT_TITLE_ARGS_KEY, Arrays.asList(this.localizedAlertTitleArguments));
-                    }
-                }
-
-                if (this.localizedAlertSubtitleKey != null) {
-                    alert.put(ALERT_SUBTITLE_LOC_KEY, this.localizedAlertSubtitleKey);
-
-                    if (this.localizedAlertSubtitleArguments != null) {
-                        alert.put(ALERT_SUBTITLE_ARGS_KEY, Arrays.asList(this.localizedAlertSubtitleArguments));
-                    }
-                }
-
-                if (this.launchImageFileName != null) {
-                    alert.put(LAUNCH_IMAGE_KEY, this.launchImageFileName);
-                }
-
-                return alert;
-            }
-        } else {
-            return null;
-        }
-    }
-
-    /**
-     * Checks whether the notification under construction has content that warrants an {@code alert} section.
-     *
-     * @return {@code true} if this notification should have an {@code alert} section or {@code false} otherwise
-     */
-    private boolean hasAlertContent() {
-        return this.alertBody != null || this.alertTitle != null || this.localizedAlertTitleKey != null
-                || this.localizedAlertKey != null || this.localizedActionButtonKey != null
-                || this.launchImageFileName != null || this.showActionButton == false || this.alertSubtitle != null
-                || this.localizedAlertSubtitleKey != null;
-    }
-
-    /**
-     * <p>Checks whether the alert message for the push notification should be represented as a string or a
-     * dictionary. According to Apple's documentation:</p>
-     *
-     * <blockquote>If you want the device to display the message text as-is in an alert that has both the Close and
-     * View buttons, then specify a string as the direct value of {@code alert}. Don't specify a dictionary as the
-     * value of {@code alert} if the dictionary only has the {@code body} property.</blockquote>
-     *
-     * @return {@code true} if the alert message (if present) should be represented as a string or {@code false} if it
-     *         should be represented as a dictionary
-     */
-    private boolean shouldRepresentAlertAsString() {
-        return this.alertBody != null && this.launchImageFileName == null && this.showActionButton
-                && this.localizedActionButtonKey == null && this.alertTitle == null && this.alertSubtitle == null
-                && this.localizedAlertTitleKey == null && this.localizedAlertKey == null
-                && this.localizedAlertArguments == null && this.localizedAlertTitleArguments == null;
-    }
-
     static int getLengthOfJsonEscapedUtf8StringFittingSize(final String string, final int maximumSize) {
         int i = 0;
         int cumulativeSize = 0;


Test patch:
diff --git a/pushy/src/test/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilderTest.java b/pushy/src/test/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilderTest.java
index bd4aa472..920ac6f5 100644
--- a/pushy/src/test/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilderTest.java
+++ b/pushy/src/test/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilderTest.java
@@ -61,7 +61,16 @@ public class ApnsPayloadBuilderTest {
         {
             final Map<String, Object> aps = this.extractApsObjectFromPayloadString(this.builder.buildWithDefaultMaximumLength());
 
-            // The alert property should be a string if all we're specifying is a literal alert message
+            @SuppressWarnings("unchecked")
+            final Map<String, Object> alert = (Map<String, Object>) aps.get("alert");
+
+            assertEquals(alertBody, alert.get("body"));
+        }
+
+        this.builder.setPreferStringRepresentationForAlerts(true);
+
+        {
+            final Map<String, Object> aps = this.extractApsObjectFromPayloadString(this.builder.buildWithDefaultMaximumLength());
             assertEquals(alertBody, aps.get("alert"));
         }
 
@@ -410,7 +419,10 @@ public class ApnsPayloadBuilderTest {
         this.builder.setAlertBody(shortAlertMessage);
         final Map<String, Object> aps = this.extractApsObjectFromPayloadString(this.builder.buildWithMaximumLength(Integer.MAX_VALUE));
 
-        assertEquals(shortAlertMessage, aps.get("alert"));
+        @SuppressWarnings("unchecked")
+        final Map<String, Object> alert = (Map<String, Object>) aps.get("alert");
+
+        assertEquals(shortAlertMessage, alert.get("body"));
     }
 
     @Test(expected = IllegalArgumentException.class)