Repository: relayrides/pushy
Pull request #369 (2016-09-08 01:56:18)

Make ApnsPayloadBuilder a little gentler
These changes make `ApnsPayloadBuilder` a little friendlier in two main ways:

1. Methods that accept a localization key and localized arguments are now variadic.
2. We no longer throw exceptions for conflicting content; instead, we just silently clear out the old content.

TODO:

- [x] Add tests to make sure we're clearing the right content


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 9e34b94c..34af285e 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
@@ -98,14 +98,8 @@ public class ApnsPayloadBuilder {
     public static final String DEFAULT_SOUND_FILENAME = "default";
 
     /**
-     * Constructs a new payload builder.
-     */
-    public ApnsPayloadBuilder() {
-    }
-
-    /**
-     * <p>Sets the literal text of the alert message to be shown for the push notification. A literal alert message may
-     * not be set if a localized alert message key is already specified.</p>
+     * <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>
      *
      * <p>By default, no message is shown.</p>
      *
@@ -113,23 +107,21 @@ public class ApnsPayloadBuilder {
      *
      * @return a reference to this payload builder
      *
-     * @see ApnsPayloadBuilder#setLocalizedAlertMessage(String, String[])
+     * @see ApnsPayloadBuilder#setLocalizedAlertMessage(String, String...)
      */
     public ApnsPayloadBuilder setAlertBody(final String alertBody) {
-        if (alertBody != null && this.localizedAlertKey != null) {
-            throw new IllegalStateException(
-                    "Cannot set a literal alert body when a localized alert key has already been set.");
-        }
-
         this.alertBody = alertBody;
 
+        this.localizedAlertKey = null;
+        this.localizedAlertArguments = null;
+
         return this;
     }
 
     /**
      * <p>Sets the key of a message in the receiving app's localized string list to be shown for the push notification.
      * The message in the app's string list may optionally have placeholders, which will be populated by values from the
-     * given {@code alertArguments}.</p>
+     * given {@code alertArguments}. Clears any previously-set literal alert body.</p>
      *
      * <p>By default, no message is shown.</p>
      *
@@ -137,79 +129,67 @@ public class ApnsPayloadBuilder {
      * @param alertArguments arguments to populate placeholders in the localized alert string; may be {@code null}
      *
      * @return a reference to this payload builder
+     *
+     * @see ApnsPayloadBuilder#setAlertBody(String)
      */
-    public ApnsPayloadBuilder setLocalizedAlertMessage(final String localizedAlertKey, final String[] alertArguments) {
-        if (localizedAlertKey != null && this.alertBody != null) {
-            throw new IllegalStateException(
-                    "Cannot set a localized alert key when a literal alert body has already been set.");
-        }
-
-        if (localizedAlertKey == null && alertArguments != null) {
-            throw new IllegalArgumentException(
-                    "Cannot set localized alert arguments without a localized alert message key.");
-        }
-
+    public ApnsPayloadBuilder setLocalizedAlertMessage(final String localizedAlertKey, final String... alertArguments) {
         this.localizedAlertKey = localizedAlertKey;
-        this.localizedAlertArguments = alertArguments;
+        this.localizedAlertArguments = alertArguments.length > 0 ? alertArguments : null;
+
+        this.alertBody = null;
 
         return this;
     }
 
     /**
-     * <p>Sets a short description of the notification purpose. The Apple Watch will display this as part of the
-     * notification. According to Apple's documentation, this should be:</p>
+     * <p>Sets a short description of the notification purpose. Clears any previously-set localized title key and
+     * arguments. The Apple Watch will display the title as part of the notification. According to Apple's
+     * documentation, this should be:</p>
      *
      * <blockquote>A short string describing the purpose of the notification. Apple Watch displays this string as part
      * of the notification interface. This string is displayed only briefly and should be crafted so that it can be
      * understood quickly.</blockquote>
      *
+     * <p>By default, no title is included.</p>
+     *
      * @param alertTitle the description to be shown for this push notification
      *
      * @return a reference to this payload builder
      *
-     * @see ApnsPayloadBuilder#setLocalizedAlertTitle(String, String[])
+     * @see ApnsPayloadBuilder#setLocalizedAlertTitle(String, String...)
      */
     public ApnsPayloadBuilder setAlertTitle(final String alertTitle) {
-        if (alertTitle != null && this.localizedAlertTitleKey != null) {
-            throw new IllegalStateException(
-                    "Cannot set a literal alert title when a localized alert title key has already been set.");
-        }
-
         this.alertTitle = alertTitle;
 
+        this.localizedAlertTitleKey = null;
+        this.localizedAlertTitleArguments = null;
+
         return this;
     }
 
     /**
      * <p>Sets the key of the title string in the receiving app's localized string list to be shown for the push
-     * notification. The message in the app's string list may optionally have placeholders, which will be populated by
-     * values from the given {@code alertArguments}.</p>
+     * notification. Clears any previously-set literal alert title. The message in the app's string list may optionally
+     * have placeholders, which will be populated by values from the given {@code alertArguments}.</p>
      *
      * @param localizedAlertTitleKey a key to a string in the receiving app's localized string list
      * @param alertTitleArguments arguments to populate placeholders in the localized alert string; may be {@code null}
      *
      * @return a reference to this payload builder
      */
-    public ApnsPayloadBuilder setLocalizedAlertTitle(final String localizedAlertTitleKey,
-            final String[] alertTitleArguments) {
-        if (localizedAlertTitleKey != null && this.alertTitle != null) {
-            throw new IllegalStateException(
-                    "Cannot set a localized alert key when a literal alert body has already been set.");
-        }
-
-        if (localizedAlertTitleKey == null && alertTitleArguments != null) {
-            throw new IllegalArgumentException(
-                    "Cannot set localized alert arguments without a localized alert message key.");
-        }
-
+    public ApnsPayloadBuilder setLocalizedAlertTitle(final String localizedAlertTitleKey, final String... alertTitleArguments) {
         this.localizedAlertTitleKey = localizedAlertTitleKey;
-        this.localizedAlertTitleArguments = alertTitleArguments;
+        this.localizedAlertTitleArguments = alertTitleArguments.length > 0 ? alertTitleArguments : null;
+
+        this.alertTitle = null;
 
         return this;
     }
 
     /**
-     * Sets a subtitle for the notification. Requires iOS 10 or newer.
+     * <p>Sets a subtitle for the notification. Clears any previously-set localized subtitle key and arguments.</p>
+     *
+     * <p>By default, no subtitle is included. Requires iOS 10 or newer.</p>
      *
      * @param alertSubtitle the subtitle for this push notification
      *
@@ -218,19 +198,20 @@ public class ApnsPayloadBuilder {
      * @since 0.8.1
      */
     public ApnsPayloadBuilder setAlertSubtitle(final String alertSubtitle) {
-        if (alertSubtitle != null && this.localizedAlertSubtitleKey != null) {
-            throw new IllegalStateException("Cannot set a literal subtitle when a localized subtitle key has already been set");
-        }
-
         this.alertSubtitle = alertSubtitle;
 
+        this.localizedAlertSubtitleKey = null;
+        this.localizedAlertSubtitleArguments = null;
+
         return this;
     }
 
     /**
      * <p>Sets the key of the subtitle string in the receiving app's localized string list to be shown for the push
-     * notification. The message in the app's string list may optionally have placeholders, which will be populated by
-     * values from the given {@code alertSubtitleArguments}.</p>
+     * notification. Clears any previously-set literal subtitle. The message in the app's string list may optionally
+     * have placeholders, which will be populated by values from the given {@code alertSubtitleArguments}.</p>
+     *
+     * <p>By default, no subtitle is included. Requires iOS 10 or newer.</p>
      *
      * @param localizedAlertSubtitleKey a key to a string in the receiving app's localized string list
      * @param alertSubtitleArguments arguments to populate placeholders in the localized subtitle string; may be
@@ -241,16 +222,10 @@ public class ApnsPayloadBuilder {
      * @since 0.8.1
      */
     public ApnsPayloadBuilder setLocalizedAlertSubtitle(final String localizedAlertSubtitleKey, final String... alertSubtitleArguments) {
-        if (localizedAlertSubtitleKey != null && this.alertSubtitle != null) {
-            throw new IllegalStateException("Cannot set a localized subtitle key when a literal subtitle has already been set");
-        }
-
-        if (localizedAlertSubtitleKey == null && alertSubtitleArguments != null) {
-            throw new IllegalArgumentException("Cannot set localized subtitle arguments without a localized subtitle key.");
-        }
-
         this.localizedAlertSubtitleKey = localizedAlertSubtitleKey;
-        this.localizedAlertSubtitleArguments = alertSubtitleArguments;
+        this.localizedAlertSubtitleArguments = alertSubtitleArguments.length > 0 ? alertSubtitleArguments : null;
+
+        this.alertSubtitle = null;
 
         return this;
     }


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 db5fc4e8..bd4aa472 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
@@ -55,6 +55,7 @@ public class ApnsPayloadBuilderTest {
     public void testSetAlertBody() {
         final String alertBody = "This is a test alert message.";
 
+        this.builder.setLocalizedAlertMessage("test.alert");
         this.builder.setAlertBody(alertBody);
 
         {
@@ -64,6 +65,8 @@ public class ApnsPayloadBuilderTest {
             assertEquals(alertBody, aps.get("alert"));
         }
 
+        this.builder.setLocalizedAlertMessage("test.alert");
+        this.builder.setAlertBody(alertBody);
         this.builder.setShowActionButton(false);
 
         {
@@ -73,6 +76,8 @@ public class ApnsPayloadBuilderTest {
             final Map<String, Object> alert = (Map<String, Object>) aps.get("alert");
 
             assertEquals(alertBody, alert.get("body"));
+            assertNull(alert.get("loc-key"));
+            assertNull(alert.get("loc-args"));
         }
     }
 
@@ -80,7 +85,9 @@ public class ApnsPayloadBuilderTest {
     @Test
     public void testSetLocalizedAlertBody() {
         final String alertKey = "test.alert";
-        this.builder.setLocalizedAlertMessage(alertKey, null);
+
+        this.builder.setAlertBody("Alert body!");
+        this.builder.setLocalizedAlertMessage(alertKey);
 
         {
             final Map<String, Object> aps = this.extractApsObjectFromPayloadString(this.builder.buildWithDefaultMaximumLength());
@@ -88,6 +95,7 @@ public class ApnsPayloadBuilderTest {
 
             assertEquals(alertKey, alert.get("loc-key"));
             assertNull(alert.get("loc-args"));
+            assertNull(alert.get("body"));
         }
 
         final String[] alertArgs = new String[] { "Moose", "helicopter" };
@@ -103,24 +111,14 @@ public class ApnsPayloadBuilderTest {
             assertEquals(alertArgs.length, argsArray.size());
             assertTrue(argsArray.containsAll(java.util.Arrays.asList(alertArgs)));
         }
-    }
 
-    @Test(expected = IllegalStateException.class)
-    public void testSetAlertBodyWithExistingLocalizedAlert() {
-        this.builder.setLocalizedAlertMessage("Test", null);
-        this.builder.setAlertBody("Test");
-    }
-
-    @Test(expected = IllegalStateException.class)
-    public void testSetLocalizedAlertWithExistingAlertBody() {
-        this.builder.setAlertBody("Test");
-        this.builder.setLocalizedAlertMessage("Test", null);
     }
 
     @Test
     public void testSetAlertTitle() {
         final String alertTitle = "This is a test alert message.";
 
+        this.builder.setLocalizedAlertTitle("alert.title");
         this.builder.setAlertTitle(alertTitle);
 
         {
@@ -130,6 +128,8 @@ public class ApnsPayloadBuilderTest {
             final Map<String, Object> alert = (Map<String, Object>) aps.get("alert");
 
             assertEquals(alertTitle, alert.get("title"));
+            assertNull(alert.get("title-loc-key"));
+            assertNull(alert.get("title-loc-args"));
         }
     }
 
@@ -137,7 +137,8 @@ public class ApnsPayloadBuilderTest {
     public void testSetLocalizedAlertTitle() {
         final String localizedAlertTitleKey = "alert.title";
 
-        this.builder.setLocalizedAlertTitle(localizedAlertTitleKey, null);
+        this.builder.setAlertTitle("Alert title!");
+        this.builder.setLocalizedAlertTitle(localizedAlertTitleKey);
 
         {
             final Map<String, Object> aps = this.extractApsObjectFromPayloadString(this.builder.buildWithDefaultMaximumLength());
@@ -146,25 +147,33 @@ public class ApnsPayloadBuilderTest {
             final Map<String, Object> alert = (Map<String, Object>) aps.get("alert");
 
             assertEquals(localizedAlertTitleKey, alert.get("title-loc-key"));
+            assertNull(alert.get("title-loc-args"));
+            assertNull(alert.get("title"));
         }
-    }
 
-    @Test(expected = IllegalStateException.class)
-    public void testSetAlertTitleWithExistingLocalizedAlertTitle() {
-        this.builder.setLocalizedAlertTitle("Test", null);
-        this.builder.setAlertTitle("Test");
-    }
+        final String[] alertArgs = new String[] { "Moose", "helicopter" };
+        this.builder.setLocalizedAlertTitle(localizedAlertTitleKey, alertArgs);
+
+        {
+            final Map<String, Object> aps = this.extractApsObjectFromPayloadString(this.builder.buildWithDefaultMaximumLength());
+
+            @SuppressWarnings("unchecked")
+            final Map<String, Object> alert = (Map<String, Object>) aps.get("alert");
+
+            assertEquals(localizedAlertTitleKey, alert.get("title-loc-key"));
 
-    @Test(expected = IllegalStateException.class)
-    public void testSetLocalizedAlertTitleWithExistingAlertTitle() {
-        this.builder.setAlertTitle("Test");
-        this.builder.setLocalizedAlertTitle("Test", null);
+            @SuppressWarnings("unchecked")
+            final List<Object> argsArray = (List<Object>) alert.get("title-loc-args");
+            assertEquals(alertArgs.length, argsArray.size());
+            assertTrue(argsArray.containsAll(java.util.Arrays.asList(alertArgs)));
+        }
     }
 
     @Test
     public void testSetAlertSubtitle() {
         final String alertSubtitle = "This is a test alert message.";
 
+        this.builder.setLocalizedAlertSubtitle("alert.subtitle");
         this.builder.setAlertSubtitle(alertSubtitle);
 
         {
@@ -174,35 +183,41 @@ public class ApnsPayloadBuilderTest {
             final Map<String, Object> alert = (Map<String, Object>) aps.get("alert");
 
             assertEquals(alertSubtitle, alert.get("subtitle"));
+            assertNull(alert.get("subtitle-loc-key"));
+            assertNull(alert.get("subtitle-loc-args"));
         }
     }
 
+    @SuppressWarnings("unchecked")
     @Test
     public void testSetLocalizedAlertSubitle() {
-        final String localizedAlertSubtitleKey = "alert.subtitle";
+        final String subtitleKey = "test.subtitle";
 
-        this.builder.setLocalizedAlertSubtitle(localizedAlertSubtitleKey);
+        this.builder.setAlertSubtitle("Subtitle!");
+        this.builder.setLocalizedAlertSubtitle(subtitleKey);
 
         {
             final Map<String, Object> aps = this.extractApsObjectFromPayloadString(this.builder.buildWithDefaultMaximumLength());
-
-            @SuppressWarnings("unchecked")
             final Map<String, Object> alert = (Map<String, Object>) aps.get("alert");
 
-            assertEquals(localizedAlertSubtitleKey, alert.get("subtitle-loc-key"));
+            assertEquals(subtitleKey, alert.get("subtitle-loc-key"));
+            assertNull(alert.get("subtitle-loc-args"));
+            assertNull(alert.get("subtitle"));
         }
-    }
 
-    @Test(expected = IllegalStateException.class)
-    public void testSetAlertSubtitleWithExistingLocalizedAlertSubtitle() {
-        this.builder.setLocalizedAlertSubtitle("Test");
-        this.builder.setAlertSubtitle("Test");
-    }
+        final String[] subtitleArgs = new String[] { "Moose", "helicopter" };
+        this.builder.setLocalizedAlertSubtitle(subtitleKey, subtitleArgs);
+
+        {
+            final Map<String, Object> aps = this.extractApsObjectFromPayloadString(this.builder.buildWithDefaultMaximumLength());
+            final Map<String, Object> alert = (Map<String, Object>) aps.get("alert");
 
-    @Test(expected = IllegalStateException.class)
-    public void testSetLocalizedAlertSubtitleWithExistingAlertSubtitle() {
-        this.builder.setAlertSubtitle("Test");
-        this.builder.setLocalizedAlertSubtitle("Test");
+            assertEquals(subtitleKey, alert.get("subtitle-loc-key"));
+
+            final List<Object> argsArray = (List<Object>) alert.get("subtitle-loc-args");
+            assertEquals(subtitleArgs.length, argsArray.size());
+            assertTrue(argsArray.containsAll(java.util.Arrays.asList(subtitleArgs)));
+        }
     }
 
     @Test
@@ -405,7 +420,7 @@ public class ApnsPayloadBuilderTest {
 
         final int maxLength = 128;
 
-        this.builder.setLocalizedAlertMessage(reallyLongAlertKey, null);
+        this.builder.setLocalizedAlertMessage(reallyLongAlertKey);
 
         final String payloadString = this.builder.buildWithMaximumLength(maxLength);