PR Fixes

Author: lahirumaramba

firebase_admin/_messaging_encoder.py

@@ -337,27 +337,27 @@ def encode_android_notification(cls, notification):
             result['event_time'] = str(event_time.isoformat()) + 'Z'
 
         priority = result.get('notification_priority')
-        if priority and priority not in ('min', 'low', 'default', 'high', 'max'):
-            raise ValueError('AndroidNotification.priority must be "default", "min", "low", "high" '
-                             'or "max".')
         if priority:
+            if priority not in ('min', 'low', 'default', 'high', 'max'):
+                raise ValueError('AndroidNotification.priority must be "default", "min", "low", '
+                                 '"high" or "max".')
             result['notification_priority'] = 'PRIORITY_' + priority.upper()
 
         visibility = result.get('visibility')
-        if visibility and visibility not in ('private', 'public', 'secret'):
-            raise ValueError(
-                'AndroidNotification.visibility must be "private", "public" or "secret".')
         if visibility:
+            if visibility not in ('private', 'public', 'secret'):
+                raise ValueError(
+                    'AndroidNotification.visibility must be "private", "public" or "secret".')
             result['visibility'] = visibility.upper()
 
         vibrate_timings_millis = result.get('vibrate_timings')
         if vibrate_timings_millis:
-            vibrate_timings_secs = []
+            vibrate_timing_strings = []
             for msec in vibrate_timings_millis:
                 formated_string = cls.encode_milliseconds(
                     'AndroidNotification.vibrate_timings_millis', msec)
-                vibrate_timings_secs.append(formated_string)
-            result['vibrate_timings'] = vibrate_timings_secs
+                vibrate_timing_strings.append(formated_string)
+            result['vibrate_timings'] = vibrate_timing_strings
         return result
 
     @classmethod
@@ -378,24 +378,25 @@ def encode_light_settings(cls, light_settings):
                 light_settings.light_off_duration_millis),
         }
         result = cls.remove_null_values(result)
-        color = result.get('color')
-        if not color:
-            raise ValueError('LightSettings.color is required.')
         light_on_duration = result.get('light_on_duration')
         if not light_on_duration:
             raise ValueError(
                 'LightSettings.light_on_duration_millis is required.')
+
         light_off_duration = result.get('light_off_duration')
         if not light_off_duration:
             raise ValueError(
                 'LightSettings.light_off_duration_millis is required.')
 
+        color = result.get('color')
+        if not color:
+            raise ValueError('LightSettings.color is required.')
         if not re.match(r'^#[0-9a-fA-F]{6}$', color) and not re.match(r'^#[0-9a-fA-F]{8}$', color):
             raise ValueError(
                 'LightSettings.color must be in the form #RRGGBB or #RRGGBBAA.')
         if len(color) == 7:
             color = (color+'FF')
-        rgba = [int(color[i:i + 2], 16) / 255. for i in (1, 3, 5, 7)]
+        rgba = [int(color[i:i + 2], 16) / 255.0 for i in (1, 3, 5, 7)]
         result['color'] = {'red': rgba[0], 'green': rgba[1],
                            'blue': rgba[2], 'alpha': rgba[3]}
         return result

firebase_admin/_messaging_utils.py

@@ -97,8 +97,9 @@ class AndroidNotification(object):
             user clicks it in the panel. When set to ``True``, the notification persists even when
             the user clicks it (optional).
         event_timestamp: For notifications that inform users about events with an absolute time
-            reference, sets the time that the event in the notification occurred. Notifications
-            in the panel are sorted by this time (optional).
+            reference, sets the time that the event in the notification occurred as a
+            ``datetime.datetime`` instance. Notifications in the panel are sorted by this time
+            (optional).
         local_only: Set whether or not this notification is relevant only to the current device.
             Some notifications can be bridged to other devices for remote display, such as a Wear OS
             watch. This hint can be set to recommend this notification not be bridged (optional).
@@ -110,13 +111,12 @@ class AndroidNotification(object):
             has been delivered. Whereas ``AndroidMessagePriority`` is an FCM concept that controls
             when the message is delivered (optional). Must be one of ``default``, ``min``, ``low``,
             ``high``, ``max`` or ``normal``.
-        vibrate_timings_millis: Set the vibration pattern to use. Pass in an array of seconds
-            to turn the vibrator on or off (optional). The first value indicates the duration to
-            wait before turning the vibrator on. The next value indicates the duration to keep the
-            vibrator on. Subsequent values alternate between duration to turn the vibrator off and
-            to turn the vibrator on. If ``vibrate_timings_millis`` is set and
-            ``default_vibrate_timings`` is set to ``True``, the default value is used instead of the
-            user-specified ``vibrate_timings_millis``.
+        vibrate_timings_millis: Sets the vibration pattern to use. Pass in an array of milliseconds
+            to turn the vibrator on or off. The first value indicates the duration to wait before
+            turning the vibrator on. The next value indicates the duration to keep the vibrator on.
+            Subsequent values alternate between duration to turn the vibrator off and to turn the
+            vibrator on. If ``vibrate_timings`` is set and ``default_vibrate_timings`` is set to
+            ``True``, the default value is used instead of the user-specified ``vibrate_timings``.
         default_vibrate_timings: If set to ``True``, use the Android framework's default vibrate
             pattern for the notification (optional). Default values are specified in ``config.xml``
             https://android.googlesource.com/platform/frameworks/base/+/master/core/res/res/values/config.xml.
@@ -183,14 +183,14 @@ class LightSettings(object):
     ``messaging.AndroidNotification``.
 
     Args:
-        color: Set color of the LED in ``#rrggbb`` or ``#rrggbbaa`` format (required).
+        color: Set color of the LED in ``#rrggbb`` or ``#rrggbbaa`` format.
         light_on_duration_millis: Along with ``light_off_duration``, define the blink rate of LED
-            flashes (required).
+            flashes.
         light_off_duration_millis: Along with ``light_on_duration``, define the blink rate of LED
-            flashes (required).
+            flashes.
     """
-    def __init__(self, color=None, light_on_duration_millis=None,
-                 light_off_duration_millis=None):
+    def __init__(self, color, light_on_duration_millis,
+                 light_off_duration_millis):
         self.color = color
         self.light_on_duration_millis = light_on_duration_millis
         self.light_off_duration_millis = light_off_duration_millis

integration/test_messaging.py

@@ -42,8 +42,10 @@ def test_send():
                 image='https://images.unsplash.com/'
                       'photo-1494438639946-1ebd1d20bf85?fit=crop&w=900&q=60',
                 event_timestamp=datetime.now(),
-                priority='high', vibrate_timings_millis=[100, 200, 300, 400],
-                visibility='public', light_settings=messaging.LightSettings(
+                priority='high',
+                vibrate_timings_millis=[100, 200, 300, 400],
+                visibility='public',
+                light_settings=messaging.LightSettings(
                     color='#aabbcc',
                     light_off_duration_millis=200,
                     light_on_duration_millis=300

tests/test_messaging.py

@@ -32,6 +32,7 @@
 NON_DICT_ARGS = ['', list(), tuple(), True, False, 1, 0, {1: 'foo'}, {'foo': 1}]
 NON_OBJECT_ARGS = [list(), tuple(), dict(), 'foo', 0, 1, True, False]
 NON_LIST_ARGS = ['', tuple(), dict(), True, False, 1, 0, [1], ['foo', 1]]
+NON_UNUM_ARGS = ['1.23s', list(), tuple(), dict(), -1.23]
 HTTP_ERROR_CODES = {
     400: exceptions.InvalidArgumentError,
     403: exceptions.PermissionDeniedError,
@@ -292,7 +293,7 @@ def test_invalid_priority(self, data):
         else:
             assert str(excinfo.value) == 'AndroidConfig.priority must be a non-empty string.'
 
-    @pytest.mark.parametrize('data', ['1.23s', list(), tuple(), dict(), -1.23])
+    @pytest.mark.parametrize('data', NON_UNUM_ARGS)
     def test_invalid_ttl(self, data):
         with pytest.raises(ValueError) as excinfo:
             check_encoding(messaging.Message(
@@ -479,60 +480,51 @@ def test_invalid_channel_id(self, data):
         excinfo = self._check_notification(notification)
         assert str(excinfo.value) == 'AndroidNotification.channel_id must be a string.'
 
-    @pytest.mark.parametrize('data', NON_STRING_ARGS)
-    def test_invalid_event_timestamp(self, data):
-        notification = messaging.AndroidNotification(event_timestamp=data)
+    @pytest.mark.parametrize('timestamp', [100, '', 'foo', {}, [], list(), dict()])
+    def test_invalid_event_timestamp(self, timestamp):
+        notification = messaging.AndroidNotification(event_timestamp=timestamp)
         excinfo = self._check_notification(notification)
         expected = 'AndroidNotification.event_timestamp must be a datetime.'
         assert str(excinfo.value) == expected
 
-    @pytest.mark.parametrize('data', NON_STRING_ARGS + ['', 'topic', 'priority', 'foo'])
-    def test_invalid_priority(self, data):
-        notification = messaging.AndroidNotification(priority=data)
+    @pytest.mark.parametrize('priority', NON_STRING_ARGS + ['foo'])
+    def test_invalid_priority(self, priority):
+        notification = messaging.AndroidNotification(priority=priority)
         excinfo = self._check_notification(notification)
-        if isinstance(data, six.string_types):
-            if not data:
+        if isinstance(priority, six.string_types):
+            if not priority:
                 expected = 'AndroidNotification.priority must be a non-empty string.'
             else:
                 expected = ('AndroidNotification.priority must be "default", "min", "low", "high" '
                             'or "max".')
-            assert str(excinfo.value) == expected
         else:
             expected = 'AndroidNotification.priority must be a non-empty string.'
-            assert str(excinfo.value) == expected
+        assert str(excinfo.value) == expected
 
-    @pytest.mark.parametrize('data', NON_STRING_ARGS + ['', 'topic', 'priority', 'foo'])
-    def test_invalid_visibility(self, data):
-        notification = messaging.AndroidNotification(visibility=data)
+    @pytest.mark.parametrize('visibility', NON_STRING_ARGS + ['', 'topic', 'priority', 'foo'])
+    def test_invalid_visibility(self, visibility):
+        notification = messaging.AndroidNotification(visibility=visibility)
         excinfo = self._check_notification(notification)
-        if isinstance(data, six.string_types):
-            if not data:
+        if isinstance(visibility, six.string_types):
+            if not visibility:
                 expected = 'AndroidNotification.visibility must be a non-empty string.'
             else:
                 expected = ('AndroidNotification.visibility must be "private", "public" or'
                             ' "secret".')
-            assert str(excinfo.value) == expected
         else:
             expected = 'AndroidNotification.visibility must be a non-empty string.'
-            assert str(excinfo.value) == expected
+        assert str(excinfo.value) == expected
 
-    @pytest.mark.parametrize('data', ['', 1, True, 'msec', ['500', 500], [0, 'abc']])
-    def test_invalid_vibrate_timings_millis(self, data):
-        notification = messaging.AndroidNotification(vibrate_timings_millis=data)
+    @pytest.mark.parametrize('vibrate_timings', ['', 1, True, 'msec', ['500', 500], [0, 'abc']])
+    def test_invalid_vibrate_timings_millis(self, vibrate_timings):
+        notification = messaging.AndroidNotification(vibrate_timings_millis=vibrate_timings)
         excinfo = self._check_notification(notification)
-        if isinstance(data, list):
+        if isinstance(vibrate_timings, list):
             expected = ('AndroidNotification.vibrate_timings_millis must not contain non-number '
                         'values.')
-            assert str(excinfo.value) == expected
         else:
             expected = 'AndroidNotification.vibrate_timings_millis must be a list of numbers.'
-            assert str(excinfo.value) == expected
-
-    @pytest.mark.parametrize('data', ['', 'foo', list(), tuple(), dict()])
-    def test_invalid_notification_count(self, data):
-        notification = messaging.AndroidNotification(notification_count=data)
-        excinfo = self._check_notification(notification)
-        assert str(excinfo.value) == 'AndroidNotification.notification_count must be a number.'
+        assert str(excinfo.value) == expected
 
     def test_negative_vibrate_timings_millis(self):
         notification = messaging.AndroidNotification(
@@ -541,6 +533,12 @@ def test_negative_vibrate_timings_millis(self):
         expected = 'AndroidNotification.vibrate_timings_millis must not be negative.'
         assert str(excinfo.value) == expected
 
+    @pytest.mark.parametrize('notification_count', ['', 'foo', list(), tuple(), dict()])
+    def test_invalid_notification_count(self, notification_count):
+        notification = messaging.AndroidNotification(notification_count=notification_count)
+        excinfo = self._check_notification(notification)
+        assert str(excinfo.value) == 'AndroidNotification.notification_count must be a number.'
+
     def test_android_notification(self):
         msg = messaging.Message(
             topic='topic',
@@ -628,27 +626,27 @@ def test_invalid_light_settings(self, data):
         assert str(excinfo.value) == expected
 
     def test_no_color(self):
-        light_settings = messaging.LightSettings(light_on_duration_millis=200,
+        light_settings = messaging.LightSettings(color=None, light_on_duration_millis=200,
                                                  light_off_duration_millis=200)
         excinfo = self._check_light_settings(light_settings)
         expected = 'LightSettings.color is required.'
         assert str(excinfo.value) == expected
 
     def test_no_light_on_duration_millis(self):
-        light_settings = messaging.LightSettings(color='#aabbcc',
+        light_settings = messaging.LightSettings(color='#aabbcc', light_on_duration_millis=None,
                                                  light_off_duration_millis=200)
         excinfo = self._check_light_settings(light_settings)
         expected = 'LightSettings.light_on_duration_millis is required.'
         assert str(excinfo.value) == expected
 
     def test_no_light_off_duration_millis(self):
-        light_settings = messaging.LightSettings(color='#aabbcc',
-                                                 light_on_duration_millis=200)
+        light_settings = messaging.LightSettings(color='#aabbcc', light_on_duration_millis=200,
+                                                 light_off_duration_millis=None)
         excinfo = self._check_light_settings(light_settings)
         expected = 'LightSettings.light_off_duration_millis is required.'
         assert str(excinfo.value) == expected
 
-    @pytest.mark.parametrize('data', ['1.23s', list(), tuple(), dict(), -1.23])
+    @pytest.mark.parametrize('data', NON_UNUM_ARGS)
     def test_invalid_light_off_duration_millis(self, data):
         light_settings = messaging.LightSettings(color='#aabbcc',
                                                  light_on_duration_millis=200,
@@ -662,7 +660,7 @@ def test_invalid_light_off_duration_millis(self, data):
                                           'duration in milliseconds or '
                                           'an instance of datetime.timedelta.')
 
-    @pytest.mark.parametrize('data', ['1.23s', list(), tuple(), dict(), -1.23])
+    @pytest.mark.parametrize('data', NON_UNUM_ARGS)
     def test_invalid_light_on_duration_millis(self, data):
         light_settings = messaging.LightSettings(color='#aabbcc',
                                                  light_on_duration_millis=data,