Refactored the _user_mgt module (#154)

* Refactored the validators and paylaod constructors

* Fixed a lint error and other regressions

* Facilitating more duck typing in the API

Author: hiranya911

firebase_admin/_user_mgt.py

@@ -37,66 +37,79 @@
 ])
 
 
-class _Validator(object):
-    """A collection of data validation utilities.
+class _Unspecified(object):
+    pass
 
-    Methods provided in this class raise ValueErrors if any validations fail. Normal returns
-    signal success.
-    """
+# Use this internally, until sentinels are available in the public API.
+_UNSPECIFIED = _Unspecified()
+
+
+class _Validator(object):
+    """A collection of data validation utilities."""
 
     @classmethod
-    def validate_uid(cls, uid):
+    def validate_uid(cls, uid, required=False):
+        if uid is None and not required:
+            return None
         if not isinstance(uid, six.string_types) or not uid or len(uid) > 128:
             raise ValueError(
                 'Invalid uid: "{0}". The uid must be a non-empty string with no more than 128 '
                 'characters.'.format(uid))
+        return uid
 
     @classmethod
-    def validate_email(cls, email):
-        if not isinstance(email, six.string_types) or not email:
+    def validate_email(cls, email, required=False):
+        if email is None and not required:
+            return None
+        if not isinstance(email, six.string_types):
             raise ValueError(
                 'Invalid email: "{0}". Email must be a non-empty string.'.format(email))
         parts = email.split('@')
         if len(parts) != 2 or not parts[0] or not parts[1]:
             raise ValueError('Malformed email address string: "{0}".'.format(email))
+        return email
 
     @classmethod
-    def validate_phone(cls, phone):
+    def validate_phone(cls, phone, required=False):
         """Validates the specified phone number.
 
         Phone number vlidation is very lax here. Backend will enforce E.164 spec compliance, and
         normalize accordingly. Here we check if the number starts with + sign, and contains at
         least one alphanumeric character.
         """
-        if not isinstance(phone, six.string_types) or not phone:
+        if phone is None and not required:
+            return None
+        if not isinstance(phone, six.string_types):
             raise ValueError('Invalid phone number: "{0}". Phone number must be a non-empty '
                              'string.'.format(phone))
         if not phone.startswith('+') or not re.search('[a-zA-Z0-9]', phone):
             raise ValueError('Invalid phone number: "{0}". Phone number must be a valid, E.164 '
                              'compliant identifier.'.format(phone))
+        return phone
 
     @classmethod
-    def validate_password(cls, password):
+    def validate_password(cls, password, required=False):
+        if password is None and not required:
+            return None
         if not isinstance(password, six.string_types) or len(password) < 6:
             raise ValueError(
                 'Invalid password string. Password must be a string at least 6 characters long.')
+        return password
 
     @classmethod
-    def validate_email_verified(cls, email_verified):
-        if not isinstance(email_verified, bool):
-            raise ValueError(
-                'Invalid email verified status: "{0}". Email verified status must be '
-                'boolean.'.format(email_verified))
-
-    @classmethod
-    def validate_display_name(cls, display_name):
+    def validate_display_name(cls, display_name, required=False):
+        if display_name is None and not required:
+            return None
         if not isinstance(display_name, six.string_types) or not display_name:
             raise ValueError(
                 'Invalid display name: "{0}". Display name must be a non-empty '
                 'string.'.format(display_name))
+        return display_name
 
     @classmethod
-    def validate_photo_url(cls, photo_url):
+    def validate_photo_url(cls, photo_url, required=False):
+        if photo_url is None and not required:
+            return None
         if not isinstance(photo_url, six.string_types) or not photo_url:
             raise ValueError(
                 'Invalid photo URL: "{0}". Photo URL must be a non-empty '
@@ -105,63 +118,53 @@ def validate_photo_url(cls, photo_url):
             parsed = urllib.parse.urlparse(photo_url)
             if not parsed.netloc:
                 raise ValueError('Malformed photo URL: "{0}".'.format(photo_url))
+            return photo_url
         except Exception:
             raise ValueError('Malformed photo URL: "{0}".'.format(photo_url))
 
     @classmethod
-    def validate_valid_since(cls, valid_since):
-        # isinstance(True, int) is True hence the extra check
-        if valid_since is None or isinstance(valid_since, bool) or not isinstance(valid_since, int):
-            raise ValueError(
-                'Invalid time string for: "{0}". Valid Since must be an int'.format(valid_since))
-        if int(valid_since) <= 0:
-            raise ValueError(
-                'Invalid valid_since: must be a positive interger. {0}'.format(valid_since))
-
-    @classmethod
-    def validate_disabled(cls, disabled):
-        if not isinstance(disabled, bool):
-            raise ValueError(
-                'Invalid disabled status: "{0}". Disabled status must be '
-                'boolean.'.format(disabled))
-
-    @classmethod
-    def validate_delete_list(cls, delete_attr):
-        if not isinstance(delete_attr, list) or not delete_attr:
-            raise ValueError(
-                'Invalid delete list: "{0}". Delete list must be a '
-                'non-empty list.'.format(delete_attr))
+    def validate_timestamp(cls, timestamp, label, required=False):
+        if timestamp is None and not required:
+            return None
+        if isinstance(timestamp, bool):
+            raise ValueError('Boolean value specified as timestamp.')
+        try:
+            timestamp_int = int(timestamp)
+            if timestamp_int <= 0:
+                raise ValueError('{0} timestamp must be a positive interger.'.format(label))
+            return timestamp_int
+        except TypeError:
+            raise ValueError('Invalid type for timestamp value: {0}.'.format(timestamp))
 
     @classmethod
-    def validate_custom_claims(cls, custom_claims):
+    def validate_custom_claims(cls, custom_claims, required=False):
         """Validates the specified custom claims.
 
-        Custom claims must be specified as a JSON string.The string must not exceed 1000
+        Custom claims must be specified as a JSON string. The string must not exceed 1000
         characters, and the parsed JSON payload must not contain reserved JWT claims.
         """
-        if not isinstance(custom_claims, six.string_types) or not custom_claims:
-            raise ValueError(
-                'Invalid custom claims: "{0}". Custom claims must be a non-empty JSON '
-                'string.'.format(custom_claims))
-
-        if len(custom_claims) > MAX_CLAIMS_PAYLOAD_SIZE:
+        if custom_claims is None and not required:
+            return None
+        claims_str = str(custom_claims)
+        if len(claims_str) > MAX_CLAIMS_PAYLOAD_SIZE:
             raise ValueError(
                 'Custom claims payload must not exceed {0} '
                 'characters.'.format(MAX_CLAIMS_PAYLOAD_SIZE))
         try:
-            parsed = json.loads(custom_claims)
+            parsed = json.loads(claims_str)
         except Exception:
             raise ValueError('Failed to parse custom claims string as JSON.')
-        else:
-            if not isinstance(parsed, dict):
-                raise ValueError('Custom claims must be parseable as a JSON object.')
-            invalid_claims = RESERVED_CLAIMS.intersection(set(parsed.keys()))
-            if len(invalid_claims) > 1:
-                joined = ', '.join(sorted(invalid_claims))
-                raise ValueError('Claims "{0}" are reserved, and must not be set.'.format(joined))
-            elif len(invalid_claims) == 1:
-                raise ValueError(
-                    'Claim "{0}" is reserved, and must not be set.'.format(invalid_claims.pop()))
+
+        if not isinstance(parsed, dict):
+            raise ValueError('Custom claims must be parseable as a JSON object.')
+        invalid_claims = RESERVED_CLAIMS.intersection(set(parsed.keys()))
+        if len(invalid_claims) > 1:
+            joined = ', '.join(sorted(invalid_claims))
+            raise ValueError('Claims "{0}" are reserved, and must not be set.'.format(joined))
+        elif len(invalid_claims) == 1:
+            raise ValueError(
+                'Claim "{0}" is reserved, and must not be set.'.format(invalid_claims.pop()))
+        return claims_str
 
 
 class ApiCallError(Exception):
@@ -176,67 +179,20 @@ def __init__(self, code, message, error=None):
 class UserManager(object):
     """Provides methods for interacting with the Google Identity Toolkit."""
 
-    _VALIDATORS = {
-        'customAttributes' : _Validator.validate_custom_claims,
-        'deleteAttribute' : _Validator.validate_delete_list,
-        'deleteProvider' : _Validator.validate_delete_list,
-        'disabled' : _Validator.validate_disabled,
-        'disableUser' : _Validator.validate_disabled,
-        'displayName' : _Validator.validate_display_name,
-        'email' : _Validator.validate_email,
-        'emailVerified' : _Validator.validate_email_verified,
-        'localId' : _Validator.validate_uid,
-        'password' : _Validator.validate_password,
-        'phoneNumber' : _Validator.validate_phone,
-        'photoUrl' : _Validator.validate_photo_url,
-        'validSince' : _Validator.validate_valid_since,
-    }
-
-    _CREATE_USER_FIELDS = {
-        'uid' : 'localId',
-        'display_name' : 'displayName',
-        'email' : 'email',
-        'email_verified' : 'emailVerified',
-        'phone_number' : 'phoneNumber',
-        'photo_url' : 'photoUrl',
-        'password' : 'password',
-        'disabled' : 'disabled',
-    }
-
-    _UPDATE_USER_FIELDS = {
-        'display_name' : 'displayName',
-        'email' : 'email',
-        'email_verified' : 'emailVerified',
-        'phone_number' : 'phoneNumber',
-        'photo_url' : 'photoUrl',
-        'password' : 'password',
-        'disabled' : 'disableUser',
-        'custom_claims' : 'customAttributes',
-        'valid_since' : 'validSince',
-    }
-
-    _REMOVABLE_FIELDS = {
-        'displayName' : 'DISPLAY_NAME',
-        'photoUrl' : 'PHOTO_URL'
-    }
-
     def __init__(self, client):
         self._client = client
 
     def get_user(self, **kwargs):
         """Gets the user data corresponding to the provided key."""
         if 'uid' in kwargs:
             key, key_type = kwargs.pop('uid'), 'user ID'
-            _Validator.validate_uid(key)
-            payload = {'localId' : [key]}
+            payload = {'localId' : [_Validator.validate_uid(key, required=True)]}
         elif 'email' in kwargs:
             key, key_type = kwargs.pop('email'), 'email'
-            _Validator.validate_email(key)
-            payload = {'email' : [key]}
+            payload = {'email' : [_Validator.validate_email(key, required=True)]}
         elif 'phone_number' in kwargs:
             key, key_type = kwargs.pop('phone_number'), 'phone number'
-            _Validator.validate_phone(key)
-            payload = {'phoneNumber' : [key]}
+            payload = {'phoneNumber' : [_Validator.validate_phone(key, required=True)]}
         else:
             raise ValueError('Unsupported keyword arguments: {0}.'.format(kwargs))
 
@@ -272,10 +228,20 @@ def list_users(self, page_token=None, max_results=MAX_LIST_USERS_RESULTS):
         except requests.exceptions.RequestException as error:
             self._handle_http_error(USER_DOWNLOAD_ERROR, 'Failed to download user accounts.', error)
 
-    def create_user(self, **kwargs):
+    def create_user(self, uid=None, display_name=None, email=None, phone_number=None,
+                    photo_url=None, password=None, disabled=None, email_verified=None):
         """Creates a new user account with the specified properties."""
-        payload = self._init_payload('create_user', UserManager._CREATE_USER_FIELDS, **kwargs)
-        self._validate(payload, self._VALIDATORS, 'create user')
+        payload = {
+            'localId': _Validator.validate_uid(uid),
+            'displayName': _Validator.validate_display_name(display_name),
+            'email': _Validator.validate_email(email),
+            'phoneNumber': _Validator.validate_phone(phone_number),
+            'photoUrl': _Validator.validate_photo_url(photo_url),
+            'password': _Validator.validate_password(password),
+            'emailVerified': bool(email_verified) if email_verified is not None else None,
+            'disabled': bool(disabled) if disabled is not None else None,
+        }
+        payload = {k: v for k, v in payload.items() if v is not None}
         try:
             response = self._client.request('post', 'signupNewUser', json=payload)
         except requests.exceptions.RequestException as error:
@@ -285,30 +251,47 @@ def create_user(self, **kwargs):
                 raise ApiCallError(USER_CREATE_ERROR, 'Failed to create new user.')
             return response.get('localId')
 
-    def update_user(self, uid, **kwargs):
+    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):
         """Updates an existing user account with the specified properties"""
-        _Validator.validate_uid(uid)
-        payload = self._init_payload('update_user', UserManager._UPDATE_USER_FIELDS, **kwargs)
-        payload['localId'] = uid
+        payload = {
+            'localId': _Validator.validate_uid(uid, required=True),
+            'email': _Validator.validate_email(email),
+            'password': _Validator.validate_password(password),
+            'validSince': _Validator.validate_timestamp(valid_since, 'valid_since'),
+            'emailVerified': bool(email_verified) if email_verified is not None else None,
+            'disableUser': bool(disabled) if disabled is not None else None,
+        }
 
         remove = []
-        for key, value in UserManager._REMOVABLE_FIELDS.items():
-            if key in payload and payload[key] is None:
-                remove.append(value)
-                del payload[key]
+        if display_name is not _UNSPECIFIED:
+            if display_name is None:
+                remove.append('DISPLAY_NAME')
+            else:
+                payload['displayName'] = _Validator.validate_display_name(display_name)
+        if photo_url is not _UNSPECIFIED:
+            if photo_url is None:
+                remove.append('PHOTO_URL')
+            else:
+                payload['photoUrl'] = _Validator.validate_photo_url(photo_url)
         if remove:
-            payload['deleteAttribute'] = sorted(remove)
-        if 'phoneNumber' in payload and payload['phoneNumber'] is None:
-            payload['deleteProvider'] = ['phone']
-            del payload['phoneNumber']
-        if 'customAttributes' in payload:
-            custom_claims = payload['customAttributes']
+            payload['deleteAttribute'] = remove
+
+        if phone_number is not _UNSPECIFIED:
+            if phone_number is None:
+                payload['deleteProvider'] = ['phone']
+            else:
+                payload['phoneNumber'] = _Validator.validate_phone(phone_number)
+
+        if custom_claims is not _UNSPECIFIED:
             if custom_claims is None:
                 custom_claims = {}
-            if isinstance(custom_claims, dict):
-                payload['customAttributes'] = json.dumps(custom_claims)
+            json_claims = json.dumps(custom_claims) if isinstance(
+                custom_claims, dict) else custom_claims
+            payload['customAttributes'] = _Validator.validate_custom_claims(json_claims)
 
-        self._validate(payload, self._VALIDATORS, 'update user')
+        payload = {k: v for k, v in payload.items() if v is not None}
         try:
             response = self._client.request('post', 'setAccountInfo', json=payload)
         except requests.exceptions.RequestException as error:
@@ -321,7 +304,7 @@ def update_user(self, uid, **kwargs):
 
     def delete_user(self, uid):
         """Deletes the user identified by the specified user ID."""
-        _Validator.validate_uid(uid)
+        _Validator.validate_uid(uid, required=True)
         try:
             response = self._client.request('post', 'deleteAccount', json={'localId' : uid})
         except requests.exceptions.RequestException as error:
@@ -338,24 +321,6 @@ def _handle_http_error(self, code, msg, error):
             msg += '\nReason: {0}'.format(error)
         raise ApiCallError(code, msg, error)
 
-    def _init_payload(self, operation, fields, **kwargs):
-        payload = {}
-        for key, value in fields.items():
-            if key in kwargs:
-                payload[value] = kwargs.pop(key)
-        if kwargs:
-            unexpected_keys = ', '.join(kwargs.keys())
-            raise ValueError(
-                'Unsupported arguments: "{0}" in call to {1}()'.format(unexpected_keys, operation))
-        return payload
-
-    def _validate(self, properties, validators, operation):
-        for key, value in properties.items():
-            validator = validators.get(key)
-            if not validator:
-                raise ValueError('Unsupported property: "{0}" in {1} call.'.format(key, operation))
-            validator(value)
-
 
 class UserIterator(object):
     """An iterator that allows iterating over user accounts, one at a time.