Skip to content

Commit 3cbfacb

Browse files
committed
fix: remove artificial 500 message limit from sendEach
1 parent 5ac228c commit 3cbfacb

3 files changed

Lines changed: 52 additions & 27 deletions

File tree

src/main/java/com/google/firebase/messaging/FirebaseMessaging.java

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,8 @@ protected String execute() throws FirebaseMessagingException {
152152
* <p>The list of responses obtained by calling {@link BatchResponse#getResponses()} on the return
153153
* value is in the same order as the input list.
154154
*
155-
* @param messages A non-null, non-empty list containing up to 500 messages.
155+
* @param messages A non-null, non-empty list of messages. For very large lists, consider
156+
* chunking into smaller batches to avoid FCM server-side rate limiting.
156157
* @return A {@link BatchResponse} indicating the result of the operation.
157158
* @throws FirebaseMessagingException If an error occurs while handing the messages off to FCM for
158159
* delivery. An exception here or a {@link BatchResponse} with all failures indicates a total
@@ -163,7 +164,6 @@ public BatchResponse sendEach(@NonNull List<Message> messages) throws FirebaseMe
163164
return sendEach(messages, false);
164165
}
165166

166-
167167
/**
168168
* Sends each message in the given list via Firebase Cloud Messaging.
169169
* Unlike {@link #sendAll(List)}, this method makes an HTTP call for each message in the
@@ -177,7 +177,8 @@ public BatchResponse sendEach(@NonNull List<Message> messages) throws FirebaseMe
177177
* <p>The list of responses obtained by calling {@link BatchResponse#getResponses()} on the return
178178
* value is in the same order as the input list.
179179
*
180-
* @param messages A non-null, non-empty list containing up to 500 messages.
180+
* @param messages A non-null, non-empty list of messages. For very large lists, consider
181+
* chunking into smaller batches to avoid FCM server-side rate limiting.
181182
* @param dryRun A boolean indicating whether to perform a dry run (validation only) of the send.
182183
* @return A {@link BatchResponse} indicating the result of the operation.
183184
* @throws FirebaseMessagingException If an error occurs while handing the messages off to FCM for
@@ -197,7 +198,8 @@ public BatchResponse sendEach(
197198
/**
198199
* Similar to {@link #sendEach(List)} but performs the operation asynchronously.
199200
*
200-
* @param messages A non-null, non-empty list containing up to 500 messages.
201+
* @param messages A non-null, non-empty list of messages. For very large lists, consider
202+
* chunking into smaller batches to avoid FCM server-side rate limiting.
201203
* @return An {@code ApiFuture} that will complete with a {@link BatchResponse} when
202204
* the messages have been sent.
203205
*/
@@ -208,7 +210,8 @@ public ApiFuture<BatchResponse> sendEachAsync(@NonNull List<Message> messages) {
208210
/**
209211
* Similar to {@link #sendEach(List, boolean)} but performs the operation asynchronously.
210212
*
211-
* @param messages A non-null, non-empty list containing up to 500 messages.
213+
* @param messages A non-null, non-empty list of messages. For very large lists, consider
214+
* chunking into smaller batches to avoid FCM server-side rate limiting.
212215
* @param dryRun A boolean indicating whether to perform a dry run (validation only) of the send.
213216
* @return An {@code ApiFuture} that will complete with a {@link BatchResponse} when
214217
* the messages have been sent.
@@ -217,30 +220,32 @@ public ApiFuture<BatchResponse> sendEachAsync(@NonNull List<Message> messages, b
217220
return sendEachOpAsync(messages, dryRun);
218221
}
219222

220-
// Returns an ApiFuture directly since this function is non-blocking. Individual child send
223+
// Returns an ApiFuture directly since this function is non-blocking. Individual child send
221224
// requests are still called async and run in background threads.
222225
private ApiFuture<BatchResponse> sendEachOpAsync(
223226
final List<Message> messages, final boolean dryRun) {
224227
final List<Message> immutableMessages = ImmutableList.copyOf(messages);
225228
checkArgument(!immutableMessages.isEmpty(), "messages list must not be empty");
226-
checkArgument(immutableMessages.size() <= 500,
227-
"messages list must not contain more than 500 elements");
229+
// NOTE: The 500-message limit that existed here was inherited from the deprecated sendAll()
230+
// method, which used a single HTTP batch request limited by Google's batch API. Since
231+
// sendEach() makes individual HTTP calls per message, no such limit applies. Callers should
232+
// chunk very large lists themselves to avoid FCM server-side rate limiting.
228233

229234
List<ApiFuture<SendResponse>> list = new ArrayList<>();
230235
for (Message message : immutableMessages) {
231236
// Make async send calls per message
232237
ApiFuture<SendResponse> messageId = sendOpForSendResponse(message, dryRun).callAsync(app);
233238
list.add(messageId);
234239
}
235-
240+
236241
// Gather all futures and combine into a list
237242
ApiFuture<List<SendResponse>> responsesFuture = ApiFutures.allAsList(list);
238243

239244
// Chain this future to wrap the eventual responses in a BatchResponse without blocking
240245
// the main thread. This uses the current thread to execute, but since the transformation
241246
// function is non-blocking the transformation itself is also non-blocking.
242247
return ApiFutures.transform(
243-
responsesFuture,
248+
responsesFuture,
244249
(responses) -> {
245250
return new BatchResponseImpl(responses);
246251
},
@@ -272,7 +277,8 @@ protected SendResponse execute() {
272277
* {@link BatchResponse#getResponses()} on the return value is in the same order as the
273278
* tokens in the {@link MulticastMessage}.
274279
*
275-
* @param message A non-null {@link MulticastMessage}
280+
* @param message A non-null {@link MulticastMessage}. For very large token lists, consider
281+
* chunking into smaller batches to avoid FCM server-side rate limiting.
276282
* @return A {@link BatchResponse} indicating the result of the operation.
277283
* @throws FirebaseMessagingException If an error occurs while handing the messages off to FCM for
278284
* delivery. An exception here or a {@link BatchResponse} with all failures indicates a total
@@ -297,7 +303,8 @@ public BatchResponse sendEachForMulticast(
297303
* {@link BatchResponse#getResponses()} on the return value is in the same order as the
298304
* tokens in the {@link MulticastMessage}.
299305
*
300-
* @param message A non-null {@link MulticastMessage}.
306+
* @param message A non-null {@link MulticastMessage}. For very large token lists, consider
307+
* chunking into smaller batches to avoid FCM server-side rate limiting.
301308
* @param dryRun A boolean indicating whether to perform a dry run (validation only) of the send.
302309
* @return A {@link BatchResponse} indicating the result of the operation.
303310
* @throws FirebaseMessagingException If an error occurs while handing the messages off to FCM for
@@ -315,7 +322,8 @@ public BatchResponse sendEachForMulticast(@NonNull MulticastMessage message, boo
315322
* Similar to {@link #sendEachForMulticast(MulticastMessage)} but performs the operation
316323
* asynchronously.
317324
*
318-
* @param message A non-null {@link MulticastMessage}.
325+
* @param message A non-null {@link MulticastMessage}. For very large token lists, consider
326+
* chunking into smaller batches to avoid FCM server-side rate limiting.
319327
* @return An {@code ApiFuture} that will complete with a {@link BatchResponse} when
320328
* the messages have been sent.
321329
*/
@@ -327,7 +335,8 @@ public ApiFuture<BatchResponse> sendEachForMulticastAsync(@NonNull MulticastMess
327335
* Similar to {@link #sendEachForMulticast(MulticastMessage, boolean)} but performs the operation
328336
* asynchronously.
329337
*
330-
* @param message A non-null {@link MulticastMessage}.
338+
* @param message A non-null {@link MulticastMessage}. For very large token lists, consider
339+
* chunking into smaller batches to avoid FCM server-side rate limiting.
331340
* @param dryRun A boolean indicating whether to perform a dry run (validation only) of the send.
332341
* @return An {@code ApiFuture} that will complete with a {@link BatchResponse} when
333342
* the messages have been sent.
@@ -677,4 +686,4 @@ FirebaseMessaging build() {
677686
return new FirebaseMessaging(this);
678687
}
679688
}
680-
}
689+
}

src/test/java/com/google/firebase/messaging/FirebaseMessagingIT.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,22 @@ public void testSendFiveHundredWithSendEach() throws Exception {
169169
assertNull(sendResponse.getException());
170170
}
171171
}
172+
173+
@Test
174+
public void testSendMoreThanFiveHundredWithSendEach() throws Exception {
175+
List<Message> messages = new ArrayList<>();
176+
for (int i = 0; i < 501; i++) {
177+
messages.add(Message.builder().setTopic("foo-bar-" + (i % 10)).build());
178+
}
179+
180+
// Previously this would throw IllegalArgumentException due to the 500 limit.
181+
// After removing the limit, this should succeed.
182+
BatchResponse response = FirebaseMessaging.getInstance().sendEach(messages, true);
183+
184+
assertEquals(501, response.getResponses().size());
185+
assertEquals(501, response.getSuccessCount());
186+
assertEquals(0, response.getFailureCount());
187+
}
172188

173189
@Test
174190
public void testSendEachForMulticast() throws Exception {

src/test/java/com/google/firebase/messaging/FirebaseMessagingTest.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import com.google.firebase.auth.MockGoogleCredentials;
3939
import com.google.firebase.internal.Nullable;
4040

41+
import java.util.ArrayList;
4142
import java.util.HashMap;
4243
import java.util.List;
4344
import java.util.Map;
@@ -305,22 +306,21 @@ public void testSendEachWithEmptyList() throws FirebaseMessagingException {
305306
}
306307

307308
@Test
308-
public void testSendEachWithTooManyMessages() throws FirebaseMessagingException {
309-
MockFirebaseMessagingClient client = MockFirebaseMessagingClient.fromMessageId(null);
310-
FirebaseMessaging messaging = getMessagingForSend(Suppliers.ofInstance(client));
311-
ImmutableList.Builder<Message> listBuilder = ImmutableList.builder();
309+
public void testSendEachWithMoreThanFiveHundredMessages() {
310+
List<Message> messages = new ArrayList<>();
312311
for (int i = 0; i < 501; i++) {
313-
listBuilder.add(Message.builder().setTopic("topic").build());
312+
messages.add(Message.builder().setTopic("foo-bar").build());
314313
}
315-
314+
// Previously threw IllegalArgumentException due to artificial 500 limit.
315+
// sendEach() makes individual HTTP calls per message, so no such limit applies.
316+
// Verify that 501 messages no longer throws an IllegalArgumentException.
316317
try {
317-
messaging.sendEach(listBuilder.build(), false);
318-
fail("No error thrown for too many messages in the list");
319-
} catch (IllegalArgumentException expected) {
320-
// expected
318+
FirebaseMessaging.getInstance().sendEach(messages);
319+
} catch (IllegalArgumentException e) {
320+
fail("sendEach() should not throw IllegalArgumentException for more than 500 messages");
321+
} catch (Exception e) {
322+
// Other exceptions (e.g. network) are acceptable in unit test context
321323
}
322-
323-
assertNull(client.lastMessage);
324324
}
325325

326326
@Test

0 commit comments

Comments
 (0)