Fix error messages and MulticastMessage constructor and add mix types of fids and tokens

Author: yvonnep165

firebase_admin/_messaging_encoder.py

@@ -50,7 +50,7 @@ def __init__(self, data=None, notification=None, android=None, webpush=None, apn
                  fcm_options=None, token=None, topic=None, condition=None, fid=None):
         if token is not None:
             warnings.warn(
-                "Message.token is deprecated. Use fid instead.",
+                "Message.token is deprecated. Use Message.fid instead.",
                 DeprecationWarning,
                 stacklevel=2
             )
@@ -73,7 +73,6 @@ class MulticastMessage:
     """A message that can be sent to multiple tokens or fids via Firebase Cloud Messaging.
 
     Args:
-        fids: A list of Firebase Installation IDs of targeted app instances (optional).
         tokens: Deprecated. Use ``fids`` instead (optional).
         data: A dictionary of data fields (optional). All keys and values in the dictionary must be
             strings.
@@ -82,33 +81,35 @@ class MulticastMessage:
         webpush: An instance of ``messaging.WebpushConfig`` (optional).
         apns: An instance of ``messaging.ApnsConfig`` (optional).
         fcm_options: An instance of ``messaging.FCMOptions`` (optional).
+        fids: A list of Firebase Installation IDs of targeted app instances (optional).
     """
     def __init__(
-            self, tokens=None, fids=None, data=None, notification=None, android=None,
-            webpush=None, apns=None, fcm_options=None):
+            self, tokens=None, data=None, notification=None, android=None,
+            webpush=None, apns=None, fcm_options=None, fids=None):
         if tokens is not None:
             warnings.warn(
-                "MulticastMessage.tokens is deprecated. Use fids instead.",
+                "MulticastMessage.tokens is deprecated. Use MulticastMessage.fids instead.",
                 DeprecationWarning,
                 stacklevel=2
             )
 
-        if (tokens is None and fids is None) or (tokens is not None and fids is not None):
-            raise ValueError("Must specify either 'tokens' or 'fids'.")
+        if tokens is None and fids is None:
+            raise ValueError(
+                "Must specify at least one of MulticastMessage.tokens or MulticastMessage.fids.")
 
         if tokens is not None:
             _Validators.check_string_list('MulticastMessage.tokens', tokens)
-            if len(tokens) > 500:
-                raise ValueError('MulticastMessage.tokens must not contain more than 500 tokens.')
-            self.tokens = tokens
-            self.fids = None
-        else:
+        if fids is not None:
             _Validators.check_string_list('MulticastMessage.fids', fids)
-            if len(fids) > 500:
-                raise ValueError('MulticastMessage.fids must not contain more than 500 fids.')
-            self.fids = fids
-            self.tokens = None
 
+        tokens_len = len(tokens) if tokens is not None else 0
+        fids_len = len(fids) if fids is not None else 0
+        if tokens_len + fids_len > 500:
+            raise ValueError(
+                'Total number of tokens and fids must not exceed 500.')
+
+        self.tokens = tokens
+        self.fids = fids
         self.data = data
         self.notification = notification
         self.android = android

firebase_admin/messaging.py

@@ -177,28 +177,33 @@ def _get_messages_from_multicast(multicast_message: MulticastMessage) -> List[Me
     """Extracts individual Message objects from a MulticastMessage."""
     if not isinstance(multicast_message, MulticastMessage):
         raise ValueError('Message must be an instance of messaging.MulticastMessage class.')
+
+    messages = []
     if multicast_message.tokens is not None:
         with warnings.catch_warnings():
             warnings.simplefilter("ignore", DeprecationWarning)
-            return [Message(
+            messages.extend([Message(
                 data=multicast_message.data,
                 notification=multicast_message.notification,
                 android=multicast_message.android,
                 webpush=multicast_message.webpush,
                 apns=multicast_message.apns,
                 fcm_options=multicast_message.fcm_options,
                 token=token
-            ) for token in multicast_message.tokens]
-
-    return [Message(
-        data=multicast_message.data,
-        notification=multicast_message.notification,
-        android=multicast_message.android,
-        webpush=multicast_message.webpush,
-        apns=multicast_message.apns,
-        fcm_options=multicast_message.fcm_options,
-        fid=fid
-    ) for fid in multicast_message.fids]
+            ) for token in multicast_message.tokens])
+
+    if multicast_message.fids is not None:
+        messages.extend([Message(
+            data=multicast_message.data,
+            notification=multicast_message.notification,
+            android=multicast_message.android,
+            webpush=multicast_message.webpush,
+            apns=multicast_message.apns,
+            fcm_options=multicast_message.fcm_options,
+            fid=fid
+        ) for fid in multicast_message.fids])
+
+    return messages
 
 async def send_each_for_multicast_async(
         multicast_message: MulticastMessage,

tests/test_messaging.py

@@ -102,11 +102,8 @@ class TestMulticastMessage:
     def test_invalid_targets(self):
         with pytest.raises(ValueError) as excinfo:
             messaging.MulticastMessage()
-        assert str(excinfo.value) == "Must specify either 'tokens' or 'fids'."
-
-        with pytest.raises(ValueError) as excinfo:
-            messaging.MulticastMessage(tokens=['token'], fids=['fid'])
-        assert str(excinfo.value) == "Must specify either 'tokens' or 'fids'."
+        expected = "Must specify at least one of MulticastMessage.tokens or MulticastMessage.fids."
+        assert str(excinfo.value) == expected
 
     @pytest.mark.parametrize('tokens', NON_LIST_ARGS)
     def test_invalid_tokens_type(self, tokens):
@@ -122,7 +119,7 @@ def test_invalid_tokens_type(self, tokens):
     def test_tokens_over_500(self):
         with pytest.raises(ValueError) as excinfo:
             messaging.MulticastMessage(tokens=['token' for _ in range(0, 501)])
-        expected = 'MulticastMessage.tokens must not contain more than 500 tokens.'
+        expected = 'Total number of tokens and fids must not exceed 500.'
         assert str(excinfo.value) == expected
 
     def test_tokens_type(self):
@@ -146,7 +143,7 @@ def test_invalid_fids_type(self, fids):
     def test_fids_over_500(self):
         with pytest.raises(ValueError) as excinfo:
             messaging.MulticastMessage(fids=['fid' for _ in range(0, 501)])
-        expected = 'MulticastMessage.fids must not contain more than 500 fids.'
+        expected = 'Total number of tokens and fids must not exceed 500.'
         assert str(excinfo.value) == expected
 
     def test_fids_type(self):
@@ -156,13 +153,27 @@ def test_fids_type(self):
         message = messaging.MulticastMessage(fids=['fid' for _ in range(0, 500)])
         assert len(message.fids) == 500
 
+    def test_combined_over_500(self):
+        with pytest.raises(ValueError) as excinfo:
+            messaging.MulticastMessage(
+                tokens=['token' for _ in range(0, 250)],
+                fids=['fid' for _ in range(0, 251)]
+            )
+        expected = 'Total number of tokens and fids must not exceed 500.'
+        assert str(excinfo.value) == expected
+
+    def test_mixed_targets(self):
+        message = messaging.MulticastMessage(tokens=['token'], fids=['fid'])
+        assert len(message.tokens) == 1
+        assert len(message.fids) == 1
+
     def test_tokens_deprecation_warning(self):
-        msg = 'MulticastMessage.tokens is deprecated. Use fids instead.'
+        msg = 'MulticastMessage.tokens is deprecated. Use MulticastMessage.fids instead.'
         with pytest.warns(DeprecationWarning, match=msg):
             messaging.MulticastMessage(tokens=['token'])
 
     def test_tokens_deprecation_warning_positional(self):
-        msg = 'MulticastMessage.tokens is deprecated. Use fids instead.'
+        msg = 'MulticastMessage.tokens is deprecated. Use MulticastMessage.fids instead.'
         with pytest.warns(DeprecationWarning, match=msg):
             messaging.MulticastMessage(['token'])
 
@@ -222,7 +233,7 @@ def test_empty_message(self):
         check_encoding(messaging.Message(condition='value'), {'condition': 'value'})
 
     def test_token_deprecation_warning(self):
-        msg = 'Message.token is deprecated. Use fid instead.'
+        msg = 'Message.token is deprecated. Use Message.fid instead.'
         with pytest.warns(DeprecationWarning, match=msg):
             messaging.Message(token='value')
 
@@ -2290,6 +2301,20 @@ def test_send_each_for_multicast_fids(self):
         assert all(r.success for r in batch_response.responses)
         assert not any(r.exception for r in batch_response.responses)
 
+    def test_send_each_for_multicast_mixed(self):
+        payload1 = json.dumps({'name': 'message-id1'})
+        payload2 = json.dumps({'name': 'message-id2'})
+        _ = self._instrument_messaging_service(
+            response_dict={'foo1': [200, payload1], 'foo2': [200, payload2]})
+        msg = messaging.MulticastMessage(tokens=['foo1'], fids=['foo2'])
+        batch_response = messaging.send_each_for_multicast(msg, dry_run=True)
+        assert batch_response.success_count == 2
+        assert batch_response.failure_count == 0
+        assert len(batch_response.responses) == 2
+        assert [r.message_id for r in batch_response.responses] == ['message-id1', 'message-id2']
+        assert all(r.success for r in batch_response.responses)
+        assert not any(r.exception for r in batch_response.responses)
+
     @respx.mock
     @pytest.mark.asyncio
     async def test_send_each_for_multicast_async(self):