Removing the ability to delete user properties by passing None (#320)

Author: hiranya911

firebase_admin/_user_mgt.py

@@ -34,10 +34,6 @@ def __init__(self, description):
         self.description = description
 
 
-# Use this internally, until sentinels are available in the public API.
-_UNSPECIFIED = Sentinel('No value specified')
-
-
 DELETE_ATTRIBUTE = Sentinel('Value used to delete an attribute from a user profile')
 
 
@@ -524,9 +520,9 @@ def create_user(self, uid=None, display_name=None, email=None, phone_number=None
                     'Failed to create new user.', http_response=http_resp)
             return body.get('localId')
 
-    def update_user(self, uid, display_name=_UNSPECIFIED, email=None, phone_number=_UNSPECIFIED,
-                    photo_url=_UNSPECIFIED, password=None, disabled=None, email_verified=None,
-                    valid_since=None, custom_claims=_UNSPECIFIED):
+    def update_user(self, uid, display_name=None, email=None, phone_number=None,
+                    photo_url=None, password=None, disabled=None, email_verified=None,
+                    valid_since=None, custom_claims=None):
         """Updates an existing user account with the specified properties"""
         payload = {
             'localId': _auth_utils.validate_uid(uid, required=True),
@@ -538,27 +534,27 @@ def update_user(self, uid, display_name=_UNSPECIFIED, email=None, phone_number=_
         }
 
         remove = []
-        if display_name is not _UNSPECIFIED:
-            if display_name is None or display_name is DELETE_ATTRIBUTE:
+        if display_name is not None:
+            if display_name is DELETE_ATTRIBUTE:
                 remove.append('DISPLAY_NAME')
             else:
                 payload['displayName'] = _auth_utils.validate_display_name(display_name)
-        if photo_url is not _UNSPECIFIED:
-            if photo_url is None or photo_url is DELETE_ATTRIBUTE:
+        if photo_url is not None:
+            if photo_url is DELETE_ATTRIBUTE:
                 remove.append('PHOTO_URL')
             else:
                 payload['photoUrl'] = _auth_utils.validate_photo_url(photo_url)
         if remove:
             payload['deleteAttribute'] = remove
 
-        if phone_number is not _UNSPECIFIED:
-            if phone_number is None or phone_number is DELETE_ATTRIBUTE:
+        if phone_number is not None:
+            if phone_number is DELETE_ATTRIBUTE:
                 payload['deleteProvider'] = ['phone']
             else:
                 payload['phoneNumber'] = _auth_utils.validate_phone(phone_number)
 
-        if custom_claims is not _UNSPECIFIED:
-            if custom_claims is None or custom_claims is DELETE_ATTRIBUTE:
+        if custom_claims is not None:
+            if custom_claims is DELETE_ATTRIBUTE:
                 custom_claims = {}
             json_claims = json.dumps(custom_claims) if isinstance(
                 custom_claims, dict) else custom_claims

tests/test_user_mgt.py

@@ -465,16 +465,6 @@ def test_delete_user_custom_claims(self, user_mgt_app):
         request = json.loads(recorder[0].body.decode())
         assert request == {'localId' : 'testuser', 'customAttributes' : json.dumps({})}
 
-    def test_update_user_delete_fields_with_none(self, user_mgt_app):
-        user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}')
-        user_mgt.update_user('testuser', display_name=None, photo_url=None, phone_number=None)
-        request = json.loads(recorder[0].body.decode())
-        assert request == {
-            'localId' : 'testuser',
-            'deleteAttribute' : ['DISPLAY_NAME', 'PHOTO_URL'],
-            'deleteProvider' : ['phone'],
-        }
-
     def test_update_user_delete_fields(self, user_mgt_app):
         user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}')
         user_mgt.update_user(
@@ -561,9 +551,9 @@ def test_set_custom_user_claims_str(self, user_mgt_app):
         request = json.loads(recorder[0].body.decode())
         assert request == {'localId' : 'testuser', 'customAttributes' : claims}
 
-    def test_set_custom_user_claims_none(self, user_mgt_app):
+    def test_set_custom_user_claims_remove(self, user_mgt_app):
         _, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}')
-        auth.set_custom_user_claims('testuser', None, app=user_mgt_app)
+        auth.set_custom_user_claims('testuser', auth.DELETE_ATTRIBUTE, app=user_mgt_app)
         request = json.loads(recorder[0].body.decode())
         assert request == {'localId' : 'testuser', 'customAttributes' : json.dumps({})}