Repository: firebase/firebase-admin-java
Pull request #309 (2019-09-09 11:33:34)

feat(fcm): Added Notification Count parameter to `AndroidNotification` class

### Discussion

As discussed in issue #308 

### Testing

  * Added Test for setNotificationCount. I noticed that your JsonParser implementation prefers BigDecimal and it would appear that Integer, Long, etc. are deserialized into BigDecimal.

### API Changes

  * None


knocknarea: Not really, the spec allows for null/empty  in which case the badge count is unaffected.
chong-shao: LGTM! 
I did some comments on this website trying to fix the merge issues.

Patch:
diff --git a/pom.xml b/pom.xml
index cffff27..c2b896f 100644
--- a/pom.xml
+++ b/pom.xml
@@ -19,7 +19,7 @@
 
     <groupId>com.google.firebase</groupId>
     <artifactId>firebase-admin</artifactId>
-    <version>6.10.1-SNAPSHOT</version>
+    <version>6.11.1-SNAPSHOT</version>
     <packaging>jar</packaging>
 
     <name>firebase-admin</name>
diff --git a/src/main/java/com/google/firebase/auth/hash/Md5.java b/src/main/java/com/google/firebase/auth/hash/Md5.java
index f45362a..2abbe55 100644
--- a/src/main/java/com/google/firebase/auth/hash/Md5.java
+++ b/src/main/java/com/google/firebase/auth/hash/Md5.java
@@ -23,7 +23,7 @@ package com.google.firebase.auth.hash;
 public class Md5 extends RepeatableHash {
 
   private Md5(Builder builder) {
-    super("MD5", 0, 120000, builder);
+    super("MD5", 0, 8192, builder);
   }
 
   public static Builder builder() {
diff --git a/src/main/java/com/google/firebase/auth/hash/Sha1.java b/src/main/java/com/google/firebase/auth/hash/Sha1.java
index d14975f..385f431 100644
--- a/src/main/java/com/google/firebase/auth/hash/Sha1.java
+++ b/src/main/java/com/google/firebase/auth/hash/Sha1.java
@@ -23,7 +23,7 @@ package com.google.firebase.auth.hash;
 public class Sha1 extends RepeatableHash {
 
   private Sha1(Builder builder) {
-    super("SHA1", 0, 120000, builder);
+    super("SHA1", 1, 8192, builder);
   }
 
   public static Builder builder() {
diff --git a/src/main/java/com/google/firebase/auth/hash/Sha256.java b/src/main/java/com/google/firebase/auth/hash/Sha256.java
index ecc0e72..f65aee1 100644
--- a/src/main/java/com/google/firebase/auth/hash/Sha256.java
+++ b/src/main/java/com/google/firebase/auth/hash/Sha256.java
@@ -23,7 +23,7 @@ package com.google.firebase.auth.hash;
 public class Sha256 extends RepeatableHash {
 
   private Sha256(Builder builder) {
-    super("SHA256", 0, 120000, builder);
+    super("SHA256", 1, 8192, builder);
   }
 
   public static Builder builder() {
diff --git a/src/main/java/com/google/firebase/auth/hash/Sha512.java b/src/main/java/com/google/firebase/auth/hash/Sha512.java
index 858d16e..e582520 100644
--- a/src/main/java/com/google/firebase/auth/hash/Sha512.java
+++ b/src/main/java/com/google/firebase/auth/hash/Sha512.java
@@ -23,7 +23,7 @@ package com.google.firebase.auth.hash;
 public class Sha512 extends RepeatableHash {
 
   private Sha512(Builder builder) {
-    super("SHA512", 0, 120000, builder);
+    super("SHA512", 1, 8192, builder);
   }
 
   public static Builder builder() {
diff --git a/src/main/java/com/google/firebase/internal/DateUtils.java b/src/main/java/com/google/firebase/internal/DateUtils.java
index 6c4eeb7..a55fac8 100644
--- a/src/main/java/com/google/firebase/internal/DateUtils.java
+++ b/src/main/java/com/google/firebase/internal/DateUtils.java
@@ -20,6 +20,7 @@ import java.text.ParsePosition;
 import java.text.SimpleDateFormat;
 import java.util.Calendar;
 import java.util.Date;
+import java.util.Locale;
 import java.util.TimeZone;
 
 /**
@@ -57,9 +58,9 @@ final class DateUtils {
   static final String PATTERN_ASCTIME = "EEE MMM d HH:mm:ss yyyy";
 
   private static final SimpleDateFormat[] DEFAULT_PATTERNS = new SimpleDateFormat[] {
-      new SimpleDateFormat(PATTERN_RFC1123),
-      new SimpleDateFormat(PATTERN_RFC1036),
-      new SimpleDateFormat(PATTERN_ASCTIME)
+      new SimpleDateFormat(PATTERN_RFC1123, Locale.US),
+      new SimpleDateFormat(PATTERN_RFC1036, Locale.US),
+      new SimpleDateFormat(PATTERN_ASCTIME, Locale.US)
   };
 
   static final TimeZone GMT = TimeZone.getTimeZone("GMT");
diff --git a/src/main/java/com/google/firebase/messaging/AndroidNotification.java b/src/main/java/com/google/firebase/messaging/AndroidNotification.java
index adb84c6..08a872c 100644
--- a/src/main/java/com/google/firebase/messaging/AndroidNotification.java
+++ b/src/main/java/com/google/firebase/messaging/AndroidNotification.java
@@ -119,7 +119,7 @@ public class AndroidNotification {
           .put(Priority.HIGH, "PRIORITY_HIGH")
           .put(Priority.MAX, "PRIORITY_MAX")
           .build();
-
+  
   private AndroidNotification(Builder builder) {
     this.title = builder.title;
     this.body = builder.body;
@@ -150,7 +150,7 @@ public class AndroidNotification {
       this.titleLocArgs = null;
     }
     this.channelId = builder.channelId;
-    this.image = builder.image;
+    this.image = builder.image;    
     this.ticker = builder.ticker;
     this.sticky = builder.sticky;
     this.eventTime = builder.eventTime;
@@ -174,6 +174,10 @@ public class AndroidNotification {
     } else {
       this.visibility = null;
     }
+    if (builder.notificationCount != null) {
+      checkArgument(builder.notificationCount >= 0, 
+          "notificationCount if specified must be zero or positive valued");
+    }
     this.notificationCount = builder.notificationCount;
   }
 
@@ -220,6 +224,7 @@ public class AndroidNotification {
     private List<String> titleLocArgs = new ArrayList<>();
     private String channelId;
     private String image;
+    private Integer notificationCount;
     private String ticker;
     private Boolean sticky;
     private String eventTime;
@@ -231,7 +236,6 @@ public class AndroidNotification {
     private LightSettings lightSettings;
     private Boolean defaultLightSettings;
     private Visibility visibility;
-    private Integer notificationCount;
 
     private Builder() {}
 
@@ -443,7 +447,7 @@ public class AndroidNotification {
     /**
      * For notifications that inform users about events with an absolute time reference, sets
      * the time that the event in the notification occurred in milliseconds. Notifications
-     * in the panel are sorted by this time. The time is be formated in RFC3339 UTC "Zulu" 
+     * in the panel are sorted by this time. The time is formatted in RFC3339 UTC "Zulu" 
      * format, accurate to nanoseconds. Example: "2014-10-02T15:01:23.045123456Z". Note that 
      * since the time is in milliseconds, the last section of the time representation always
      * has 6 leading zeros.
@@ -580,13 +584,15 @@ public class AndroidNotification {
 
     /**
      * Sets the number of items this notification represents. May be displayed as a badge 
-     * count for launchers that support badging. For example, this might be useful if you're 
-     * using just one notification to represent multiple new messages but you want the count 
-     * here to represent the number of total new messages. If zero or unspecified, systems 
-     * that support badging use the default, which is to increment a number displayed on 
+     * count for launchers that support badging. 
+     * If not invoked then notification count is left unchanged.
+     * For example, this might be useful if you're using just one notification to represent
+     * multiple new messages but you want the count here to represent the number of total
+     * new messages. If zero or unspecified, systems that support badging use the default, 
+     * which is to increment a number displayed on 
      * the long-press menu each time a new notification arrives.
      *
-     * @param notificationCount The notification count
+     * @param notificationCount Zero or positive value. Zero indicates leave unchanged.
      * @return This builder.
      */
     public Builder setNotificationCount(int notificationCount) {


Test patch:
diff --git a/src/test/java/com/google/firebase/auth/hash/InvalidHashTest.java b/src/test/java/com/google/firebase/auth/hash/InvalidHashTest.java
index aabe444..5186a44 100644
--- a/src/test/java/com/google/firebase/auth/hash/InvalidHashTest.java
+++ b/src/test/java/com/google/firebase/auth/hash/InvalidHashTest.java
@@ -48,17 +48,21 @@ public class InvalidHashTest {
 
   @Test
   public void testInvalidRepeatableHash() {
+    // TODO(rsgowman): Once we can update to Java8, we could just do something like this instead of
+    // having all of the helpers:
+    //     assertThrows(IllegalArgumentException.class, ()-> Md5.builder().setRounds(-1).build());
+
     List<RepeatableHash.Builder> builders = ImmutableList.<RepeatableHash.Builder>builder()
-        .add(Sha512.builder().setRounds(-1))
-        .add(Sha256.builder().setRounds(-1))
-        .add(Sha1.builder().setRounds(-1))
+        .add(Sha512.builder().setRounds(0))
+        .add(Sha256.builder().setRounds(0))
+        .add(Sha1.builder().setRounds(0))
         .add(Md5.builder().setRounds(-1))
         .add(Pbkdf2Sha256.builder().setRounds(-1))
         .add(PbkdfSha1.builder().setRounds(-1))
-        .add(Sha512.builder().setRounds(120001))
-        .add(Sha256.builder().setRounds(120001))
-        .add(Sha1.builder().setRounds(120001))
-        .add(Md5.builder().setRounds(120001))
+        .add(Sha512.builder().setRounds(8193))
+        .add(Sha256.builder().setRounds(8193))
+        .add(Sha1.builder().setRounds(8193))
+        .add(Md5.builder().setRounds(8193))
         .add(Pbkdf2Sha256.builder().setRounds(120001))
         .add(PbkdfSha1.builder().setRounds(120001))
         .build();
@@ -72,6 +76,22 @@ public class InvalidHashTest {
     }
   }
 
+  @Test
+  public void testValidRepeatableHash() {
+    Md5.builder().setRounds(0).build();
+    Md5.builder().setRounds(8192).build();
+    Sha1.builder().setRounds(1).build();
+    Sha1.builder().setRounds(8192).build();
+    Sha256.builder().setRounds(1).build();
+    Sha256.builder().setRounds(8192).build();
+    Sha512.builder().setRounds(1).build();
+    Sha512.builder().setRounds(8192).build();
+    PbkdfSha1.builder().setRounds(0).build();
+    PbkdfSha1.builder().setRounds(120000).build();
+    Pbkdf2Sha256.builder().setRounds(0).build();
+    Pbkdf2Sha256.builder().setRounds(120000).build();
+  }
+
   @Test
   public void testInvalidScrypt() {
     List<Scrypt.Builder> builders = ImmutableList.of(
diff --git a/src/test/java/com/google/firebase/internal/RetryUnsuccessfulResponseHandlerTest.java b/src/test/java/com/google/firebase/internal/RetryUnsuccessfulResponseHandlerTest.java
index 077e9e5..f0bc2d0 100644
--- a/src/test/java/com/google/firebase/internal/RetryUnsuccessfulResponseHandlerTest.java
+++ b/src/test/java/com/google/firebase/internal/RetryUnsuccessfulResponseHandlerTest.java
@@ -35,7 +35,9 @@ import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
+import java.util.Locale;
 import java.util.TimeZone;
+
 import org.junit.Test;
 
 public class RetryUnsuccessfulResponseHandlerTest {
@@ -137,7 +139,8 @@ public class RetryUnsuccessfulResponseHandlerTest {
 
   @Test
   public void testRetryAfterGivenAsDate() throws IOException {
-    SimpleDateFormat dateFormat = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz");
+    SimpleDateFormat dateFormat =
+            new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US);
     dateFormat.setTimeZone(TimeZone.getTimeZone("GMT"));
     Date date = new Date(1000);
     Clock clock = new FixedClock(date.getTime());
diff --git a/src/test/java/com/google/firebase/messaging/MessageTest.java b/src/test/java/com/google/firebase/messaging/MessageTest.java
index 7c8f528..5c8c3cb 100644
--- a/src/test/java/com/google/firebase/messaging/MessageTest.java
+++ b/src/test/java/com/google/firebase/messaging/MessageTest.java
@@ -33,6 +33,7 @@ import java.text.SimpleDateFormat;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import org.junit.Test;
@@ -166,6 +167,7 @@ public class MessageTest {
                 .addBodyLocalizationArg("body-arg1")
                 .addAllBodyLocalizationArgs(ImmutableList.of("body-arg2", "body-arg3"))
                 .setChannelId("channel-id")
+                .setNotificationCount(4)
                 .build())
             .build())
         .setTopic("test-topic")
@@ -183,6 +185,10 @@ public class MessageTest {
         .put("body_loc_key", "body-loc")
         .put("body_loc_args", ImmutableList.of("body-arg1", "body-arg2", "body-arg3"))
         .put("channel_id", "channel-id")
+        // There is a problem with the JsonParser assignment to BigDecimal takes priority over
+        // all other number types and so this integer value is interpreted as a BigDecimal 
+        // rather than an Integer.
+        .put("notification_count", BigDecimal.valueOf(4L))
         .build();
     Map<String, Object> data = ImmutableMap.of(
         "collapse_key", "test-key",
@@ -194,6 +200,11 @@ public class MessageTest {
     assertJsonEquals(ImmutableMap.of("topic", "test-topic", "android", data), message);
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testAndroidNotificationWithNegativeCount() throws IllegalArgumentException {
+    AndroidNotification.builder().setNotificationCount(-1).build();
+  }
+  
   @Test
   public void testAndroidMessageWithoutLocalization() throws IOException {
     Message message = Message.builder()
@@ -769,7 +780,7 @@ public class MessageTest {
     assertJsonEquals(ImmutableMap.of(
         "topic", "test-topic", "notification", notification, "android", androidConfig), message);
   }
-  
+
   @Test
   public void testImageInApnsNotification() throws IOException {
     Message message = Message.builder()
@@ -799,7 +810,7 @@ public class MessageTest {
             .build();
     assertJsonEquals(expected, message);
   }
-  
+
   @Test
   public void testInvalidColorInAndroidNotificationLightSettings() throws IOException {
     try {
@@ -814,7 +825,7 @@ public class MessageTest {
       // expected
     }
   }
-  
+
   @Test
   public void testExtendedAndroidNotificationParameters() throws IOException {
     long[] vibrateTimings = {1000L, 1001L};
@@ -848,7 +859,7 @@ public class MessageTest {
         .put("title", "title")
         .put("body", "body")
         .build();
-    String eventTime = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSSSSSSS'Z'")
+    String eventTime = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSSSSSSS'Z'", Locale.US)
         .format(new Date(1546304523123L));
     Map<String, Object> androidConfig = ImmutableMap.<String, Object>builder()
         .put("notification", ImmutableMap.<String, Object>builder()