From d580639c7da4a01596bc93a0727c02c611166f9e Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Wed, 29 Jan 2020 11:55:50 -0800 Subject: [PATCH 1/4] Setting a default timeout on all HTTP connections --- firebase_admin/__init__.py | 4 ++-- firebase_admin/_http_client.py | 18 +++++++++++++--- firebase_admin/db.py | 9 +++----- firebase_admin/messaging.py | 11 +++++----- firebase_admin/project_management.py | 8 +++---- tests/test_db.py | 24 +++++++++++++-------- tests/test_http_client.py | 31 ++++++++++++++++++++++++++++ tests/test_instance_id.py | 7 +++++++ tests/test_messaging.py | 8 ++++++- tests/test_project_management.py | 3 +++ tests/test_user_mgt.py | 7 +++++++ 11 files changed, 99 insertions(+), 31 deletions(-) diff --git a/firebase_admin/__init__.py b/firebase_admin/__init__.py index 40039626..7e3b2eab 100644 --- a/firebase_admin/__init__.py +++ b/firebase_admin/__init__.py @@ -49,8 +49,8 @@ def initialize_app(credential=None, options=None, name=_DEFAULT_APP_NAME): Google Application Default Credentials are used. options: A dictionary of configuration options (optional). Supported options include ``databaseURL``, ``storageBucket``, ``projectId``, ``databaseAuthVariableOverride``, - ``serviceAccountId`` and ``httpTimeout``. If ``httpTimeout`` is not set, HTTP - connections initiated by client modules such as ``db`` will not time out. + ``serviceAccountId`` and ``httpTimeout``. If ``httpTimeout`` is not set, the SDK + uses a default timeout of 120 seconds. name: Name of the app (optional). Returns: App: A newly initialized instance of App. diff --git a/firebase_admin/_http_client.py b/firebase_admin/_http_client.py index 1daaf371..f6f0d89f 100644 --- a/firebase_admin/_http_client.py +++ b/firebase_admin/_http_client.py @@ -32,6 +32,9 @@ raise_on_status=False, backoff_factor=0.5) +DEFAULT_TIMEOUT_SECONDS = 120 + + class HttpClient: """Base HTTP client used to make HTTP calls. @@ -41,7 +44,7 @@ class HttpClient: def __init__( self, credential=None, session=None, base_url='', headers=None, - retries=DEFAULT_RETRY_CONFIG): + retries=DEFAULT_RETRY_CONFIG, timeout=DEFAULT_TIMEOUT_SECONDS): """Creates a new HttpClient instance from the provided arguments. If a credential is provided, initializes a new HTTP session authorized with it. If neither @@ -55,6 +58,8 @@ def __init__( retries: A urllib retry configuration. Default settings would retry once for low-level connection and socket read errors, and up to 4 times for HTTP 500 and 503 errors. Pass a False value to disable retries (optional). + timeout: HTTP timeout in seconds. Defaults to 120 seconds when not specified. Set to + None to disable timeouts (optional). """ if credential: self._session = transport.requests.AuthorizedSession(credential) @@ -69,6 +74,7 @@ def __init__( self._session.mount('http://', requests.adapters.HTTPAdapter(max_retries=retries)) self._session.mount('https://', requests.adapters.HTTPAdapter(max_retries=retries)) self._base_url = base_url + self._timeout = timeout @property def session(self): @@ -78,6 +84,10 @@ def session(self): def base_url(self): return self._base_url + @property + def timeout(self): + return self._timeout + def parse_body(self, resp): raise NotImplementedError @@ -93,7 +103,7 @@ class call this method to send HTTP requests out. Refer to method: HTTP method name as a string (e.g. get, post). url: URL of the remote endpoint. kwargs: An additional set of keyword arguments to be passed into the requests API - (e.g. json, params). + (e.g. json, params, timeout). Returns: Response: An HTTP response object. @@ -101,7 +111,9 @@ class call this method to send HTTP requests out. Refer to Raises: RequestException: Any requests exceptions encountered while making the HTTP call. """ - resp = self._session.request(method, self._base_url + url, **kwargs) + if 'timeout' not in kwargs: + kwargs['timeout'] = self.timeout + resp = self._session.request(method, self.base_url + url, **kwargs) resp.raise_for_status() return resp diff --git a/firebase_admin/db.py b/firebase_admin/db.py index 9092a955..1c1bb29b 100644 --- a/firebase_admin/db.py +++ b/firebase_admin/db.py @@ -775,7 +775,7 @@ def __init__(self, app): self._auth_override = json.dumps(auth_override, separators=(',', ':')) else: self._auth_override = None - self._timeout = app.options.get('httpTimeout') + self._timeout = app.options.get('httpTimeout', _http_client.DEFAULT_TIMEOUT_SECONDS) self._clients = {} emulator_host = os.environ.get(_EMULATOR_HOST_ENV_VAR) @@ -905,9 +905,8 @@ def __init__(self, credential, base_url, timeout, params=None): params: Dict of query parameters to add to all outgoing requests. """ _http_client.JsonHttpClient.__init__( - self, credential=credential, base_url=base_url, headers={'User-Agent': _USER_AGENT}) - self.credential = credential - self.timeout = timeout + self, credential=credential, base_url=base_url, timeout=timeout, + headers={'User-Agent': _USER_AGENT}) self.params = params if params else {} def request(self, method, url, **kwargs): @@ -937,8 +936,6 @@ def request(self, method, url, **kwargs): query = extra_params kwargs['params'] = query - if self.timeout: - kwargs['timeout'] = self.timeout try: return super(_Client, self).request(method, url, **kwargs) except requests.exceptions.RequestException as error: diff --git a/firebase_admin/messaging.py b/firebase_admin/messaging.py index 9262751a..78887504 100644 --- a/firebase_admin/messaging.py +++ b/firebase_admin/messaging.py @@ -330,8 +330,9 @@ def __init__(self, app): 'X-GOOG-API-FORMAT-VERSION': '2', 'X-FIREBASE-CLIENT': 'fire-admin-python/{0}'.format(firebase_admin.__version__), } - self._client = _http_client.JsonHttpClient(credential=app.credential.get_credential()) - self._timeout = app.options.get('httpTimeout') + timeout = app.options.get('httpTimeout', _http_client.DEFAULT_TIMEOUT_SECONDS) + self._client = _http_client.JsonHttpClient( + credential=app.credential.get_credential(), timeout=timeout) self._transport = _auth.authorized_http(app.credential.get_credential()) @classmethod @@ -348,8 +349,7 @@ def send(self, message, dry_run=False): 'post', url=self._fcm_url, headers=self._fcm_headers, - json=data, - timeout=self._timeout + json=data ) except requests.exceptions.RequestException as error: raise self._handle_fcm_error(error) @@ -416,8 +416,7 @@ def make_topic_management_request(self, tokens, topic, operation): 'post', url=url, json=data, - headers=_MessagingService.IID_HEADERS, - timeout=self._timeout + headers=_MessagingService.IID_HEADERS ) except requests.exceptions.RequestException as error: raise self._handle_iid_error(error) diff --git a/firebase_admin/project_management.py b/firebase_admin/project_management.py index 91aa1eeb..ed292b80 100644 --- a/firebase_admin/project_management.py +++ b/firebase_admin/project_management.py @@ -478,11 +478,12 @@ def __init__(self, app): 'the GOOGLE_CLOUD_PROJECT environment variable.') self._project_id = project_id version_header = 'Python/Admin/{0}'.format(firebase_admin.__version__) + timeout = app.options.get('httpTimeout', _http_client.DEFAULT_TIMEOUT_SECONDS) self._client = _http_client.JsonHttpClient( credential=app.credential.get_credential(), base_url=_ProjectManagementService.BASE_URL, - headers={'X-Client-Version': version_header}) - self._timeout = app.options.get('httpTimeout') + headers={'X-Client-Version': version_header}, + timeout=timeout) def get_android_app_metadata(self, app_id): return self._get_app_metadata( @@ -658,7 +659,6 @@ def _make_request(self, method, url, json=None): def _body_and_response(self, method, url, json=None): try: - return self._client.body_and_response( - method=method, url=url, json=json, timeout=self._timeout) + return self._client.body_and_response(method=method, url=url, json=json) except requests.exceptions.RequestException as error: raise _utils.handle_platform_error_from_requests(error) diff --git a/tests/test_db.py b/tests/test_db.py index b20f99cb..6bb8174c 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -23,6 +23,7 @@ import firebase_admin from firebase_admin import db from firebase_admin import exceptions +from firebase_admin import _http_client from firebase_admin import _sseclient from tests import testutils @@ -736,10 +737,11 @@ def test_valid_db_url(self, url): ref._client.session.mount(url, adapter) assert ref._client.base_url == 'https://test.firebaseio.com' assert 'auth_variable_override' not in ref._client.params - assert ref._client.timeout is None + assert ref._client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS assert ref.get() == {} assert len(recorder) == 1 - assert recorder[0]._extra_kwargs.get('timeout') is None + assert recorder[0]._extra_kwargs.get('timeout') == pytest.approx( + _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) @pytest.mark.parametrize('url', [ None, '', 'foo', 'http://test.firebaseio.com', 'https://google.com', @@ -761,7 +763,7 @@ def test_multi_db_support(self): ref = db.reference() assert ref._client.base_url == default_url assert 'auth_variable_override' not in ref._client.params - assert ref._client.timeout is None + assert ref._client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS assert ref._client is db.reference()._client assert ref._client is db.reference(url=default_url)._client @@ -769,7 +771,7 @@ def test_multi_db_support(self): other_ref = db.reference(url=other_url) assert other_ref._client.base_url == other_url assert 'auth_variable_override' not in ref._client.params - assert other_ref._client.timeout is None + assert other_ref._client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS assert other_ref._client is db.reference(url=other_url)._client assert other_ref._client is db.reference(url=other_url + '/')._client @@ -782,7 +784,7 @@ def test_valid_auth_override(self, override): default_ref = db.reference() other_ref = db.reference(url='https://other.firebaseio.com') for ref in [default_ref, other_ref]: - assert ref._client.timeout is None + assert ref._client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS if override == {}: assert 'auth_variable_override' not in ref._client.params else: @@ -804,11 +806,12 @@ def test_invalid_auth_override(self, override): with pytest.raises(ValueError): db.reference(app=other_app, url='https://other.firebaseio.com') - def test_http_timeout(self): + @pytest.mark.parametrize('timeout', [60, None]) + def test_http_timeout(self, timeout): test_url = 'https://test.firebaseio.com' firebase_admin.initialize_app(testutils.MockCredential(), { 'databaseURL' : test_url, - 'httpTimeout': 60 + 'httpTimeout': timeout }) default_ref = db.reference() other_ref = db.reference(url='https://other.firebaseio.com') @@ -816,10 +819,13 @@ def test_http_timeout(self): recorder = [] adapter = MockAdapter('{}', 200, recorder) ref._client.session.mount(ref._client.base_url, adapter) - assert ref._client.timeout == 60 + assert ref._client.timeout == timeout assert ref.get() == {} assert len(recorder) == 1 - assert recorder[0]._extra_kwargs['timeout'] == pytest.approx(60, 0.001) + if timeout is None: + assert recorder[0]._extra_kwargs['timeout'] is None + else: + assert recorder[0]._extra_kwargs['timeout'] == pytest.approx(timeout, 0.001) def test_app_delete(self): app = firebase_admin.initialize_app( diff --git a/tests/test_http_client.py b/tests/test_http_client.py index d4d2885f..291bffe8 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -28,6 +28,7 @@ def test_http_client_default_session(): client = _http_client.HttpClient() assert client.session is not None assert client.base_url == '' + assert client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS recorder = _instrument(client, 'body') resp = client.request('get', _TEST_URL) assert resp.status_code == 200 @@ -35,12 +36,15 @@ def test_http_client_default_session(): assert len(recorder) == 1 assert recorder[0].method == 'GET' assert recorder[0].url == _TEST_URL + assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( + _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) def test_http_client_custom_session(): session = requests.Session() client = _http_client.HttpClient(session=session) assert client.session is session assert client.base_url == '' + assert client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS recorder = _instrument(client, 'body') resp = client.request('get', _TEST_URL) assert resp.status_code == 200 @@ -48,11 +52,14 @@ def test_http_client_custom_session(): assert len(recorder) == 1 assert recorder[0].method == 'GET' assert recorder[0].url == _TEST_URL + assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( + _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) def test_base_url(): client = _http_client.HttpClient(base_url=_TEST_URL) assert client.session is not None assert client.base_url == _TEST_URL + assert client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS recorder = _instrument(client, 'body') resp = client.request('get', 'foo') assert resp.status_code == 200 @@ -60,11 +67,14 @@ def test_base_url(): assert len(recorder) == 1 assert recorder[0].method == 'GET' assert recorder[0].url == _TEST_URL + 'foo' + assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( + _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) def test_credential(): client = _http_client.HttpClient( credential=testutils.MockGoogleCredential()) assert client.session is not None + assert client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS recorder = _instrument(client, 'body') resp = client.request('get', _TEST_URL) assert resp.status_code == 200 @@ -73,6 +83,27 @@ def test_credential(): assert recorder[0].method == 'GET' assert recorder[0].url == _TEST_URL assert recorder[0].headers['Authorization'] == 'Bearer mock-token' + assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( + _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) + +@pytest.mark.parametrize('timeout', [7, None]) +def test_timeout(timeout): + client = _http_client.HttpClient(timeout=timeout) + assert client.session is not None + assert client.base_url == '' + assert client.timeout == timeout + recorder = _instrument(client, 'body') + resp = client.request('get', _TEST_URL) + assert resp.status_code == 200 + assert resp.text == 'body' + assert len(recorder) == 1 + assert recorder[0].method == 'GET' + assert recorder[0].url == _TEST_URL + if timeout: + assert recorder[0]._extra_kwargs['timeout'] == pytest.approx(timeout, 0.001) + else: + assert recorder[0]._extra_kwargs['timeout'] is None + def _instrument(client, payload, status=200): recorder = [] diff --git a/tests/test_instance_id.py b/tests/test_instance_id.py index a13506a0..08b0fe6d 100644 --- a/tests/test_instance_id.py +++ b/tests/test_instance_id.py @@ -19,6 +19,7 @@ import firebase_admin from firebase_admin import exceptions from firebase_admin import instance_id +from firebase_admin import _http_client from tests import testutils @@ -73,6 +74,12 @@ def evaluate(): instance_id.delete_instance_id('test') testutils.run_without_project_id(evaluate) + def test_default_timeout(self): + cred = testutils.MockCredential() + app = firebase_admin.initialize_app(cred, {'projectId': 'explicit-project-id'}) + iid_service = instance_id._get_iid_service(app) + assert iid_service._client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS + def test_delete_instance_id(self): cred = testutils.MockCredential() app = firebase_admin.initialize_app(cred, {'projectId': 'explicit-project-id'}) diff --git a/tests/test_messaging.py b/tests/test_messaging.py index 33c99445..eff4d060 100644 --- a/tests/test_messaging.py +++ b/tests/test_messaging.py @@ -23,6 +23,7 @@ import firebase_admin from firebase_admin import exceptions from firebase_admin import messaging +from firebase_admin import _http_client from tests import testutils @@ -1641,7 +1642,8 @@ def test_send(self): assert recorder[0].url == self._get_url('explicit-project-id') assert recorder[0].headers['X-GOOG-API-FORMAT-VERSION'] == '2' assert recorder[0].headers['X-FIREBASE-CLIENT'] == self._CLIENT_VERSION - assert recorder[0]._extra_kwargs['timeout'] is None + assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( + _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) body = {'message': messaging._MessagingService.encode_message(msg)} assert json.loads(recorder[0].body.decode()) == body @@ -2224,6 +2226,8 @@ def test_subscribe_to_topic(self, args): assert recorder[0].method == 'POST' assert recorder[0].url == self._get_url('iid/v1:batchAdd') assert json.loads(recorder[0].body.decode()) == args[2] + assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( + _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) @pytest.mark.parametrize('status, exc_type', HTTP_ERROR_CODES.items()) def test_subscribe_to_topic_error(self, status, exc_type): @@ -2256,6 +2260,8 @@ def test_unsubscribe_from_topic(self, args): assert recorder[0].method == 'POST' assert recorder[0].url == self._get_url('iid/v1:batchRemove') assert json.loads(recorder[0].body.decode()) == args[2] + assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( + _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) @pytest.mark.parametrize('status, exc_type', HTTP_ERROR_CODES.items()) def test_unsubscribe_from_topic_error(self, status, exc_type): diff --git a/tests/test_project_management.py b/tests/test_project_management.py index aa717bbf..16446e2d 100644 --- a/tests/test_project_management.py +++ b/tests/test_project_management.py @@ -22,6 +22,7 @@ import firebase_admin from firebase_admin import exceptions from firebase_admin import project_management +from firebase_admin import _http_client from tests import testutils OPERATION_IN_PROGRESS_RESPONSE = json.dumps({ @@ -522,6 +523,8 @@ def _assert_request_is_correct( assert request.url == expected_url client_version = 'Python/Admin/{0}'.format(firebase_admin.__version__) assert request.headers['X-Client-Version'] == client_version + assert request._extra_kwargs['timeout'] == pytest.approx( + _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) if expected_body is None: assert request.body is None else: diff --git a/tests/test_user_mgt.py b/tests/test_user_mgt.py index 9b0b4ce1..958bbf9c 100644 --- a/tests/test_user_mgt.py +++ b/tests/test_user_mgt.py @@ -25,6 +25,7 @@ from firebase_admin import auth from firebase_admin import exceptions from firebase_admin import _auth_utils +from firebase_admin import _http_client from firebase_admin import _user_import from firebase_admin import _user_mgt from tests import testutils @@ -102,12 +103,18 @@ def _check_user_record(user, expected_uid='testuser'): class TestAuthServiceInitialization: + def test_default_timeout(self, user_mgt_app): + auth_service = auth._get_auth_service(user_mgt_app) + user_manager = auth_service.user_manager + assert user_manager._client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS + def test_fail_on_no_project_id(self): app = firebase_admin.initialize_app(testutils.MockCredential(), name='userMgt2') with pytest.raises(ValueError): auth._get_auth_service(app) firebase_admin.delete_app(app) + class TestUserRecord: # Input dict must be non-empty, and must not contain unsupported keys. From 995f7a0142901de74388b64593b552e255208f46 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Wed, 29 Jan 2020 14:48:39 -0800 Subject: [PATCH 2/4] Refactored and cleaned up tests --- firebase_admin/db.py | 8 ++-- tests/test_db.py | 47 +++++++++++--------- tests/test_http_client.py | 35 ++++++--------- tests/test_messaging.py | 73 ++++++++++++++++++-------------- tests/test_project_management.py | 26 +++++++++++- 5 files changed, 108 insertions(+), 81 deletions(-) diff --git a/firebase_admin/db.py b/firebase_admin/db.py index 1c1bb29b..b82a327e 100644 --- a/firebase_admin/db.py +++ b/firebase_admin/db.py @@ -900,13 +900,13 @@ def __init__(self, credential, base_url, timeout, params=None): credential: A Google credential that can be used to authenticate requests. base_url: A URL prefix to be added to all outgoing requests. This is typically the Firebase Realtime Database URL. - timeout: HTTP request timeout in seconds. If not set connections will never + timeout: HTTP request timeout in seconds. If set to None connections will never timeout, which is the default behavior of the underlying requests library. params: Dict of query parameters to add to all outgoing requests. """ - _http_client.JsonHttpClient.__init__( - self, credential=credential, base_url=base_url, timeout=timeout, - headers={'User-Agent': _USER_AGENT}) + super().__init__( + credential=credential, base_url=base_url, + timeout=timeout, headers={'User-Agent': _USER_AGENT}) self.params = params if params else {} def request(self, method, url, **kwargs): diff --git a/tests/test_db.py b/tests/test_db.py index 6bb8174c..70da07da 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -732,16 +732,8 @@ def test_parse_db_url_errors(self, url, emulator_host): def test_valid_db_url(self, url): firebase_admin.initialize_app(testutils.MockCredential(), {'databaseURL' : url}) ref = db.reference() - recorder = [] - adapter = MockAdapter('{}', 200, recorder) - ref._client.session.mount(url, adapter) assert ref._client.base_url == 'https://test.firebaseio.com' assert 'auth_variable_override' not in ref._client.params - assert ref._client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS - assert ref.get() == {} - assert len(recorder) == 1 - assert recorder[0]._extra_kwargs.get('timeout') == pytest.approx( - _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) @pytest.mark.parametrize('url', [ None, '', 'foo', 'http://test.firebaseio.com', 'https://google.com', @@ -763,7 +755,6 @@ def test_multi_db_support(self): ref = db.reference() assert ref._client.base_url == default_url assert 'auth_variable_override' not in ref._client.params - assert ref._client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS assert ref._client is db.reference()._client assert ref._client is db.reference(url=default_url)._client @@ -771,7 +762,6 @@ def test_multi_db_support(self): other_ref = db.reference(url=other_url) assert other_ref._client.base_url == other_url assert 'auth_variable_override' not in ref._client.params - assert other_ref._client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS assert other_ref._client is db.reference(url=other_url)._client assert other_ref._client is db.reference(url=other_url + '/')._client @@ -806,8 +796,20 @@ def test_invalid_auth_override(self, override): with pytest.raises(ValueError): db.reference(app=other_app, url='https://other.firebaseio.com') + def test_default_http_timeout(self): + default_url = 'https://test.firebaseio.com' + firebase_admin.initialize_app(testutils.MockCredential(), { + 'databaseURL' : default_url, + }) + ref = db.reference() + self._check_timeout(ref, _http_client.DEFAULT_TIMEOUT_SECONDS) + + other_url = 'https://other.firebaseio.com' + other_ref = db.reference(url=other_url) + self._check_timeout(other_ref, _http_client.DEFAULT_TIMEOUT_SECONDS) + @pytest.mark.parametrize('timeout', [60, None]) - def test_http_timeout(self, timeout): + def test_custom_http_timeout(self, timeout): test_url = 'https://test.firebaseio.com' firebase_admin.initialize_app(testutils.MockCredential(), { 'databaseURL' : test_url, @@ -816,16 +818,7 @@ def test_http_timeout(self, timeout): default_ref = db.reference() other_ref = db.reference(url='https://other.firebaseio.com') for ref in [default_ref, other_ref]: - recorder = [] - adapter = MockAdapter('{}', 200, recorder) - ref._client.session.mount(ref._client.base_url, adapter) - assert ref._client.timeout == timeout - assert ref.get() == {} - assert len(recorder) == 1 - if timeout is None: - assert recorder[0]._extra_kwargs['timeout'] is None - else: - assert recorder[0]._extra_kwargs['timeout'] == pytest.approx(timeout, 0.001) + self._check_timeout(ref, timeout) def test_app_delete(self): app = firebase_admin.initialize_app( @@ -847,6 +840,18 @@ def test_user_agent_format(self): firebase_admin.__version__, sys.version_info.major, sys.version_info.minor) assert db._USER_AGENT == expected + def _check_timeout(self, ref, timeout): + assert ref._client.timeout == timeout + recorder = [] + adapter = MockAdapter('{}', 200, recorder) + ref._client.session.mount(ref._client.base_url, adapter) + assert ref.get() == {} + assert len(recorder) == 1 + if timeout is None: + assert recorder[0]._extra_kwargs['timeout'] is None + else: + assert recorder[0]._extra_kwargs['timeout'] == pytest.approx(timeout, 0.001) + @pytest.fixture(params=['foo', '$key', '$value']) def initquery(request): diff --git a/tests/test_http_client.py b/tests/test_http_client.py index 291bffe8..88735080 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -28,7 +28,6 @@ def test_http_client_default_session(): client = _http_client.HttpClient() assert client.session is not None assert client.base_url == '' - assert client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS recorder = _instrument(client, 'body') resp = client.request('get', _TEST_URL) assert resp.status_code == 200 @@ -36,15 +35,12 @@ def test_http_client_default_session(): assert len(recorder) == 1 assert recorder[0].method == 'GET' assert recorder[0].url == _TEST_URL - assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( - _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) def test_http_client_custom_session(): session = requests.Session() client = _http_client.HttpClient(session=session) assert client.session is session assert client.base_url == '' - assert client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS recorder = _instrument(client, 'body') resp = client.request('get', _TEST_URL) assert resp.status_code == 200 @@ -52,14 +48,11 @@ def test_http_client_custom_session(): assert len(recorder) == 1 assert recorder[0].method == 'GET' assert recorder[0].url == _TEST_URL - assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( - _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) def test_base_url(): client = _http_client.HttpClient(base_url=_TEST_URL) assert client.session is not None assert client.base_url == _TEST_URL - assert client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS recorder = _instrument(client, 'body') resp = client.request('get', 'foo') assert resp.status_code == 200 @@ -67,14 +60,11 @@ def test_base_url(): assert len(recorder) == 1 assert recorder[0].method == 'GET' assert recorder[0].url == _TEST_URL + 'foo' - assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( - _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) def test_credential(): client = _http_client.HttpClient( credential=testutils.MockGoogleCredential()) assert client.session is not None - assert client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS recorder = _instrument(client, 'body') resp = client.request('get', _TEST_URL) assert resp.status_code == 200 @@ -83,26 +73,27 @@ def test_credential(): assert recorder[0].method == 'GET' assert recorder[0].url == _TEST_URL assert recorder[0].headers['Authorization'] == 'Bearer mock-token' + +def test_default_timeout(): + client = _http_client.HttpClient() + assert client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS + recorder = _instrument(client, 'body') + client.request('get', _TEST_URL) + assert len(recorder) == 1 assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) -@pytest.mark.parametrize('timeout', [7, None]) -def test_timeout(timeout): +@pytest.mark.parametrize('timeout', [7, 0, None]) +def test_custom_timeout(timeout): client = _http_client.HttpClient(timeout=timeout) - assert client.session is not None - assert client.base_url == '' assert client.timeout == timeout recorder = _instrument(client, 'body') - resp = client.request('get', _TEST_URL) - assert resp.status_code == 200 - assert resp.text == 'body' + client.request('get', _TEST_URL) assert len(recorder) == 1 - assert recorder[0].method == 'GET' - assert recorder[0].url == _TEST_URL - if timeout: - assert recorder[0]._extra_kwargs['timeout'] == pytest.approx(timeout, 0.001) - else: + if timeout is None: assert recorder[0]._extra_kwargs['timeout'] is None + else: + assert recorder[0]._extra_kwargs['timeout'] == pytest.approx(timeout, 0.001) def _instrument(client, payload, status=200): diff --git a/tests/test_messaging.py b/tests/test_messaging.py index eff4d060..f8be4cd6 100644 --- a/tests/test_messaging.py +++ b/tests/test_messaging.py @@ -1538,42 +1538,57 @@ def test_aps_alert_custom_data_override(self): } check_encoding(msg, expected) -class TestTimeout: - @classmethod - def setup_class(cls): - cred = testutils.MockCredential() - firebase_admin.initialize_app(cred, {'httpTimeout': 4, 'projectId': 'explicit-project-id'}) +class TestTimeout: - @classmethod - def teardown_class(cls): + def teardown(self): testutils.cleanup_apps() - def setup(self): + def _instrument_service(self, url, response): app = firebase_admin.get_app() - self.fcm_service = messaging._get_messaging_service(app) - self.recorder = [] + fcm_service = messaging._get_messaging_service(app) + recorder = [] + fcm_service._client.session.mount( + url, testutils.MockAdapter(json.dumps(response), 200, recorder)) + return recorder - def test_send(self): - self.fcm_service._client.session.mount( - 'https://fcm.googleapis.com', - testutils.MockAdapter(json.dumps({'name': 'message-id'}), 200, self.recorder)) + def _check_timeout(self, recorder, timeout): + assert len(recorder) == 1 + if timeout is None: + assert recorder[0]._extra_kwargs['timeout'] is None + else: + assert recorder[0]._extra_kwargs['timeout'] == pytest.approx(timeout, 0.001) + + @pytest.mark.parametrize('options, timeout', [ + ({'httpTimeout': 4}, 4), + ({'httpTimeout': None}, None), + ({}, _http_client.DEFAULT_TIMEOUT_SECONDS), + ]) + def test_send(self, options, timeout): + cred = testutils.MockCredential() + all_options = {'projectId': 'explicit-project-id'} + all_options.update(options) + firebase_admin.initialize_app(cred, all_options) + recorder = self._instrument_service( + 'https://fcm.googleapis.com', {'name': 'message-id'}) msg = messaging.Message(topic='foo') messaging.send(msg) - assert len(self.recorder) == 1 - assert self.recorder[0]._extra_kwargs['timeout'] == pytest.approx(4, 0.001) + self._check_timeout(recorder, timeout) - def test_topic_management_timeout(self): - self.fcm_service._client.session.mount( - 'https://iid.googleapis.com', - testutils.MockAdapter( - json.dumps({'results': [{}, {'error': 'error_reason'}]}), - 200, - self.recorder) - ) + @pytest.mark.parametrize('options, timeout', [ + ({'httpTimeout': 4}, 4), + ({'httpTimeout': None}, None), + ({}, _http_client.DEFAULT_TIMEOUT_SECONDS), + ]) + def test_topic_management_custom_timeout(self, options, timeout): + cred = testutils.MockCredential() + all_options = {'projectId': 'explicit-project-id'} + all_options.update(options) + firebase_admin.initialize_app(cred, all_options) + recorder = self._instrument_service( + 'https://iid.googleapis.com', {'results': [{}, {'error': 'error_reason'}]}) messaging.subscribe_to_topic(['1'], 'a') - assert len(self.recorder) == 1 - assert self.recorder[0]._extra_kwargs['timeout'] == pytest.approx(4, 0.001) + self._check_timeout(recorder, timeout) class TestSend: @@ -1642,8 +1657,6 @@ def test_send(self): assert recorder[0].url == self._get_url('explicit-project-id') assert recorder[0].headers['X-GOOG-API-FORMAT-VERSION'] == '2' assert recorder[0].headers['X-FIREBASE-CLIENT'] == self._CLIENT_VERSION - assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( - _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) body = {'message': messaging._MessagingService.encode_message(msg)} assert json.loads(recorder[0].body.decode()) == body @@ -2226,8 +2239,6 @@ def test_subscribe_to_topic(self, args): assert recorder[0].method == 'POST' assert recorder[0].url == self._get_url('iid/v1:batchAdd') assert json.loads(recorder[0].body.decode()) == args[2] - assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( - _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) @pytest.mark.parametrize('status, exc_type', HTTP_ERROR_CODES.items()) def test_subscribe_to_topic_error(self, status, exc_type): @@ -2260,8 +2271,6 @@ def test_unsubscribe_from_topic(self, args): assert recorder[0].method == 'POST' assert recorder[0].url == self._get_url('iid/v1:batchRemove') assert json.loads(recorder[0].body.decode()) == args[2] - assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( - _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) @pytest.mark.parametrize('status, exc_type', HTTP_ERROR_CODES.items()) def test_unsubscribe_from_topic_error(self, status, exc_type): diff --git a/tests/test_project_management.py b/tests/test_project_management.py index 16446e2d..31a7d8a2 100644 --- a/tests/test_project_management.py +++ b/tests/test_project_management.py @@ -523,14 +523,36 @@ def _assert_request_is_correct( assert request.url == expected_url client_version = 'Python/Admin/{0}'.format(firebase_admin.__version__) assert request.headers['X-Client-Version'] == client_version - assert request._extra_kwargs['timeout'] == pytest.approx( - _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) if expected_body is None: assert request.body is None else: assert json.loads(request.body.decode()) == expected_body +class TestTimeout(BaseProjectManagementTest): + + def test_default_timeout(self): + app = firebase_admin.get_app() + project_management_service = project_management._get_project_management_service(app) + assert project_management_service._client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS + + @pytest.mark.parametrize('timeout', [4, None]) + def test_custom_timeout(self, timeout): + options = { + 'httpTimeout': timeout, + 'projectId': 'test-project-id' + } + app = firebase_admin.initialize_app(testutils.MockCredential(), options, 'timeout-app') + try: + project_management_service = project_management._get_project_management_service(app) + if timeout is None: + assert project_management_service._client.timeout is None + else: + assert project_management_service._client.timeout == pytest.approx(timeout, 0.001) + finally: + firebase_admin.delete_app(app) + + class TestCreateAndroidApp(BaseProjectManagementTest): _CREATION_URL = 'https://firebase.googleapis.com/v1beta1/projects/test-project-id/androidApps' From 19f2ed4081e71a3a62b7df5129ffe5ba5441c336 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Wed, 29 Jan 2020 14:57:36 -0800 Subject: [PATCH 3/4] Further cleaning up the tests --- tests/test_db.py | 28 ++++++++++------------------ tests/test_project_management.py | 5 +---- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/tests/test_db.py b/tests/test_db.py index 70da07da..1743347c 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -774,7 +774,6 @@ def test_valid_auth_override(self, override): default_ref = db.reference() other_ref = db.reference(url='https://other.firebaseio.com') for ref in [default_ref, other_ref]: - assert ref._client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS if override == {}: assert 'auth_variable_override' not in ref._client.params else: @@ -796,25 +795,18 @@ def test_invalid_auth_override(self, override): with pytest.raises(ValueError): db.reference(app=other_app, url='https://other.firebaseio.com') - def test_default_http_timeout(self): - default_url = 'https://test.firebaseio.com' - firebase_admin.initialize_app(testutils.MockCredential(), { - 'databaseURL' : default_url, - }) - ref = db.reference() - self._check_timeout(ref, _http_client.DEFAULT_TIMEOUT_SECONDS) - - other_url = 'https://other.firebaseio.com' - other_ref = db.reference(url=other_url) - self._check_timeout(other_ref, _http_client.DEFAULT_TIMEOUT_SECONDS) - - @pytest.mark.parametrize('timeout', [60, None]) - def test_custom_http_timeout(self, timeout): + @pytest.mark.parametrize('options, timeout', [ + ({'httpTimeout': 4}, 4), + ({'httpTimeout': None}, None), + ({}, _http_client.DEFAULT_TIMEOUT_SECONDS), + ]) + def test_http_timeout(self, options, timeout): test_url = 'https://test.firebaseio.com' - firebase_admin.initialize_app(testutils.MockCredential(), { + all_options = { 'databaseURL' : test_url, - 'httpTimeout': timeout - }) + } + all_options.update(options) + firebase_admin.initialize_app(testutils.MockCredential(), all_options) default_ref = db.reference() other_ref = db.reference(url='https://other.firebaseio.com') for ref in [default_ref, other_ref]: diff --git a/tests/test_project_management.py b/tests/test_project_management.py index 31a7d8a2..44ade753 100644 --- a/tests/test_project_management.py +++ b/tests/test_project_management.py @@ -545,10 +545,7 @@ def test_custom_timeout(self, timeout): app = firebase_admin.initialize_app(testutils.MockCredential(), options, 'timeout-app') try: project_management_service = project_management._get_project_management_service(app) - if timeout is None: - assert project_management_service._client.timeout is None - else: - assert project_management_service._client.timeout == pytest.approx(timeout, 0.001) + assert project_management_service._client.timeout == timeout finally: firebase_admin.delete_app(app) From aff21c84fd8315caf78c9483dbc84b361d53bff9 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 30 Jan 2020 11:08:29 -0800 Subject: [PATCH 4/4] Cleaning up tests based on feedback --- tests/test_http_client.py | 20 ++++++++------------ tests/test_project_management.py | 10 ++++------ 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/tests/test_http_client.py b/tests/test_http_client.py index 88735080..12ba03b4 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -74,18 +74,14 @@ def test_credential(): assert recorder[0].url == _TEST_URL assert recorder[0].headers['Authorization'] == 'Bearer mock-token' -def test_default_timeout(): - client = _http_client.HttpClient() - assert client.timeout == _http_client.DEFAULT_TIMEOUT_SECONDS - recorder = _instrument(client, 'body') - client.request('get', _TEST_URL) - assert len(recorder) == 1 - assert recorder[0]._extra_kwargs['timeout'] == pytest.approx( - _http_client.DEFAULT_TIMEOUT_SECONDS, 0.001) - -@pytest.mark.parametrize('timeout', [7, 0, None]) -def test_custom_timeout(timeout): - client = _http_client.HttpClient(timeout=timeout) +@pytest.mark.parametrize('options, timeout', [ + ({}, _http_client.DEFAULT_TIMEOUT_SECONDS), + ({'timeout': 7}, 7), + ({'timeout': 0}, 0), + ({'timeout': None}, None), +]) +def test_timeout(options, timeout): + client = _http_client.HttpClient(**options) assert client.timeout == timeout recorder = _instrument(client, 'body') client.request('get', _TEST_URL) diff --git a/tests/test_project_management.py b/tests/test_project_management.py index 44ade753..18319551 100644 --- a/tests/test_project_management.py +++ b/tests/test_project_management.py @@ -542,12 +542,10 @@ def test_custom_timeout(self, timeout): 'httpTimeout': timeout, 'projectId': 'test-project-id' } - app = firebase_admin.initialize_app(testutils.MockCredential(), options, 'timeout-app') - try: - project_management_service = project_management._get_project_management_service(app) - assert project_management_service._client.timeout == timeout - finally: - firebase_admin.delete_app(app) + app = firebase_admin.initialize_app( + testutils.MockCredential(), options, 'timeout-{0}'.format(timeout)) + project_management_service = project_management._get_project_management_service(app) + assert project_management_service._client.timeout == timeout class TestCreateAndroidApp(BaseProjectManagementTest):