Skip to content

Commit a366fff

Browse files
author
Javen
committed
Fix bug - null extras will cause IllgalArgumentException, android notification.anroid() will enter into this situation.
1 parent e7f6e0e commit a366fff

6 files changed

Lines changed: 110 additions & 15 deletions

File tree

‎src/cn/jpush/api/push/model/notification/AndroidNotification.java‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,11 @@ public Builder setAlert(String alert) {
7676
}
7777

7878
public Builder addExtra(String key, String value) {
79-
Preconditions.checkArgument(! (null == key || null == value), "Key/Value should not be null.");
79+
Preconditions.checkArgument(! (null == key), "Key should not be null.");
80+
if (null == value) {
81+
LOG.debug("Extra value is null, throw away it.");
82+
return this;
83+
}
8084
if (null == extrasBuilder) {
8185
extrasBuilder = ImmutableMap.builder();
8286
}
@@ -85,7 +89,11 @@ public Builder addExtra(String key, String value) {
8589
}
8690

8791
public Builder addExtras(Map<String, String> extras) {
88-
Preconditions.checkArgument(! (null == extras), "extras should not be null.");
92+
if (null == extras) {
93+
LOG.warn("Null extras param. Throw away it.");
94+
return this;
95+
}
96+
8997
if (null == extrasBuilder) {
9098
extrasBuilder = ImmutableMap.builder();
9199
}
@@ -96,7 +104,11 @@ public Builder addExtras(Map<String, String> extras) {
96104
}
97105

98106
public Builder addExtra(String key, Number value) {
99-
Preconditions.checkArgument(! (null == key || null == value), "Key/Value should not be null.");
107+
Preconditions.checkArgument(! (null == key), "Key should not be null.");
108+
if (null == value) {
109+
LOG.debug("Extra value is null, throw away it.");
110+
return this;
111+
}
100112
if (null == numberExtrasBuilder) {
101113
numberExtrasBuilder = ImmutableMap.builder();
102114
}
@@ -105,7 +117,11 @@ public Builder addExtra(String key, Number value) {
105117
}
106118

107119
public Builder addExtra(String key, Boolean value) {
108-
Preconditions.checkArgument(! (null == key || null == value), "Key/Value should not be null.");
120+
Preconditions.checkArgument(! (null == key), "Key should not be null.");
121+
if (null == value) {
122+
LOG.debug("Extra value is null, throw away it.");
123+
return this;
124+
}
109125
if (null == booleanExtrasBuilder) {
110126
booleanExtrasBuilder = ImmutableMap.builder();
111127
}

‎src/cn/jpush/api/push/model/notification/IosNotification.java‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,11 @@ public Builder setAlert(String alert) {
117117
}
118118

119119
public Builder addExtra(String key, String value) {
120-
Preconditions.checkArgument(! (null == key || null == value), "Key/Value should not be null.");
120+
Preconditions.checkArgument(! (null == key), "Key should not be null.");
121+
if (null == value) {
122+
LOG.debug("Extra value is null, throw away it.");
123+
return this;
124+
}
121125
if (null == extrasBuilder) {
122126
extrasBuilder = ImmutableMap.builder();
123127
}
@@ -126,7 +130,11 @@ public Builder addExtra(String key, String value) {
126130
}
127131

128132
public Builder addExtras(Map<String, String> extras) {
129-
Preconditions.checkArgument(! (null == extras), "extras should not be null.");
133+
if (null == extras) {
134+
LOG.warn("Null extras param. Throw away it.");
135+
return this;
136+
}
137+
130138
if (null == extrasBuilder) {
131139
extrasBuilder = ImmutableMap.builder();
132140
}
@@ -137,7 +145,11 @@ public Builder addExtras(Map<String, String> extras) {
137145
}
138146

139147
public Builder addExtra(String key, Number value) {
140-
Preconditions.checkArgument(! (null == key || null == value), "Key/Value should not be null.");
148+
Preconditions.checkArgument(! (null == key), "Key should not be null.");
149+
if (null == value) {
150+
LOG.debug("Extra value is null, throw away it.");
151+
return this;
152+
}
141153
if (null == numberExtrasBuilder) {
142154
numberExtrasBuilder = ImmutableMap.builder();
143155
}
@@ -146,7 +158,11 @@ public Builder addExtra(String key, Number value) {
146158
}
147159

148160
public Builder addExtra(String key, Boolean value) {
149-
Preconditions.checkArgument(! (null == key || null == value), "Key/Value should not be null.");
161+
Preconditions.checkArgument(! (null == key), "Key should not be null.");
162+
if (null == value) {
163+
LOG.debug("Extra value is null, throw away it.");
164+
return this;
165+
}
150166
if (null == booleanExtrasBuilder) {
151167
booleanExtrasBuilder = ImmutableMap.builder();
152168
}

‎src/cn/jpush/api/push/model/notification/PlatformNotification.java‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package cn.jpush.api.push.model.notification;
22

3+
import org.slf4j.Logger;
4+
import org.slf4j.LoggerFactory;
5+
36
import cn.jpush.api.push.model.PushModel;
47

58
import com.google.common.collect.ImmutableMap;
@@ -10,6 +13,8 @@
1013
public abstract class PlatformNotification implements PushModel {
1114
public static final String ALERT = "alert";
1215
private static final String EXTRAS = "extras";
16+
17+
protected static final Logger LOG = LoggerFactory.getLogger(PlatformNotification.class);
1318

1419
private String alert;
1520
private final ImmutableMap<String, String> extras;
@@ -38,18 +43,30 @@ public JsonElement toJSON() {
3843
}
3944

4045
if (null != extras) {
46+
String value = null;
4147
for (String key : extras.keySet()) {
42-
extrasObject.add(key, new JsonPrimitive(extras.get(key)));
48+
value = extras.get(key);
49+
if (null != value) {
50+
extrasObject.add(key, new JsonPrimitive(value));
51+
}
4352
}
4453
}
4554
if (null != numberExtras) {
55+
Number value = null;
4656
for (String key : numberExtras.keySet()) {
47-
extrasObject.add(key, new JsonPrimitive(numberExtras.get(key)));
57+
value = numberExtras.get(key);
58+
if (null != value) {
59+
extrasObject.add(key, new JsonPrimitive(value));
60+
}
4861
}
4962
}
5063
if (null != booleanExtras) {
64+
Boolean value = null;
5165
for (String key : booleanExtras.keySet()) {
52-
extrasObject.add(key, new JsonPrimitive(booleanExtras.get(key)));
66+
value = booleanExtras.get(key);
67+
if (null != value) {
68+
extrasObject.add(key, new JsonPrimitive(value));
69+
}
5370
}
5471
}
5572

‎src/cn/jpush/api/push/model/notification/WinphoneNotification.java‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
import java.util.Map;
44

5+
import cn.jpush.api.push.model.notification.AndroidNotification.Builder;
6+
57
import com.google.common.base.Preconditions;
68
import com.google.common.collect.ImmutableMap;
79
import com.google.gson.JsonElement;
@@ -76,7 +78,11 @@ public Builder setAlert(String alert) {
7678
}
7779

7880
public Builder addExtra(String key, String value) {
79-
Preconditions.checkArgument(! (null == key || null == value), "Key/Value should not be null.");
81+
Preconditions.checkArgument(! (null == key), "Key should not be null.");
82+
if (null == value) {
83+
LOG.debug("Extra value is null, throw away it.");
84+
return this;
85+
}
8086
if (null == extrasBuilder) {
8187
extrasBuilder = ImmutableMap.builder();
8288
}
@@ -85,7 +91,11 @@ public Builder addExtra(String key, String value) {
8591
}
8692

8793
public Builder addExtras(Map<String, String> extras) {
88-
Preconditions.checkArgument(! (null == extras), "extras should not be null.");
94+
if (null == extras) {
95+
LOG.warn("Null extras param. Throw away it.");
96+
return this;
97+
}
98+
8999
if (null == extrasBuilder) {
90100
extrasBuilder = ImmutableMap.builder();
91101
}
@@ -96,7 +106,11 @@ public Builder addExtras(Map<String, String> extras) {
96106
}
97107

98108
public Builder addExtra(String key, Number value) {
99-
Preconditions.checkArgument(! (null == key || null == value), "Key/Value should not be null.");
109+
Preconditions.checkArgument(! (null == key), "Key should not be null.");
110+
if (null == value) {
111+
LOG.debug("Extra value is null, throw away it.");
112+
return this;
113+
}
100114
if (null == numberExtrasBuilder) {
101115
numberExtrasBuilder = ImmutableMap.builder();
102116
}
@@ -105,7 +119,11 @@ public Builder addExtra(String key, Number value) {
105119
}
106120

107121
public Builder addExtra(String key, Boolean value) {
108-
Preconditions.checkArgument(! (null == key || null == value), "Key/Value should not be null.");
122+
Preconditions.checkArgument(! (null == key), "Key should not be null.");
123+
if (null == value) {
124+
LOG.debug("Extra value is null, throw away it.");
125+
return this;
126+
}
109127
if (null == booleanExtrasBuilder) {
110128
booleanExtrasBuilder = ImmutableMap.builder();
111129
}

‎test/cn/jpush/api/push/model/notification/AndroidNotificationTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,20 @@ public void testExtra() {
4343
Assert.assertEquals("", json, an.toJSON());
4444
}
4545

46+
@Test
47+
public void testExtra_nullvalue() {
48+
String value2 = "value2";
49+
value2 = null;
50+
AndroidNotification an = AndroidNotification.newBuilder()
51+
.addExtra("key2", value2)
52+
.addExtra("key1", "value1").build();
53+
JsonObject json = new JsonObject();
54+
JsonObject extra = new JsonObject();
55+
extra.add("key1", new JsonPrimitive("value1"));
56+
json.add("extras", extra);
57+
Assert.assertEquals("", json, an.toJSON());
58+
}
59+
4660

4761
}
4862

‎test/cn/jpush/api/push/model/notification/NotificationTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,18 @@ public void testAlertAll() {
7878
Assert.assertEquals("", json, notification.toJSON());
7979
}
8080

81+
@Test
82+
public void testShortcut_android() {
83+
Notification notification = Notification.android("alert", "title", null);
84+
JsonObject json = new JsonObject();
85+
JsonObject android = new JsonObject();
86+
android.add("alert", new JsonPrimitive("alert"));
87+
android.add("title", new JsonPrimitive("title"));
88+
json.add("android", android);
89+
90+
Assert.assertEquals("", json, notification.toJSON());
91+
}
92+
93+
94+
8195
}

0 commit comments

Comments
 (0)