From 723ef2d8f20cf7e64013fd65c81b87a40a6a00f7 Mon Sep 17 00:00:00 2001 From: Ingrid Fielker Date: Thu, 15 Aug 2019 15:39:23 -0400 Subject: [PATCH 1/7] added GetModel --- firebase_admin/mlkit.py | 76 +++++++++++++++++++++++++++++++++++++++-- tests/test_mlkit.py | 73 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 tests/test_mlkit.py diff --git a/firebase_admin/mlkit.py b/firebase_admin/mlkit.py index 9271fd668..a380b358a 100644 --- a/firebase_admin/mlkit.py +++ b/firebase_admin/mlkit.py @@ -18,8 +18,80 @@ deleting, publishing and unpublishing Firebase ML Kit models. """ +import re +import six + +import firebase_admin +from firebase_admin import _http_client +from firebase_admin import _utils + +_MLKIT_ATTRIBUTE = '_mlkit' + +def _get_mlkit_service(app): + """ Returns an _MLKitService instance for an App. + + Args: + app: A Firebase App instance (or None to use the default App). + + Returns: + _MLKitService: An _MLKitService for the specified App instance. + + Raises: + ValueError: If the app argument is invalid. + """ + return _utils.get_app_service(app, _MLKIT_ATTRIBUTE, _MLKitService) + +def get_model(model_id, app=None): + mlkit_service = _get_mlkit_service(app) + return Model(mlkit_service.get_model(model_id)) + +class Model(object): + """A Firebase ML Kit Model object.""" + def __init__(self, data): + """Created from a data dictionary.""" + self._data = data + + #TODO(ifielker): define the Model properties etc + class _MLKitService(object): """Firebase MLKit service.""" - BASE_URL = 'https://mlkit.googleapis.com' - PROJECT_URL = 'https://mlkit.googleapis.com/projects/{0}/' + BASE_URL = 'https://mlkit.googleapis.com/v1beta1/' + PROJECT_URL = 'https://mlkit.googleapis.com/v1beta1/projects/{0}/' + + def __init__(self, app): + project_id = app.project_id + if not project_id: + raise ValueError( + 'Project ID is required to access MLKit service. Either set the ' + 'projectId option, or use service account credentials.') + self._project_url = _MLKitService.PROJECT_URL.format(project_id) + self._client = _http_client.JsonHttpClient(credential=app.credential.get_credential()) + + def _request(self, method, urlpath, **kwargs): + """Makes an HTTP call using the Python requests library. + + Args: + method: HTTP method name as a string (e.g. get, post, patch, delete). + urlpath: URL path to the endpoint. This will be appended to the + server's base project URL. + kwargs: An additional set of keyword arguments to be passed into requests + API (e.g. json, params) + + Returns: + dict: The parsed JSON response. + """ + return self._client.body(method, url=self._project_url + urlpath, **kwargs) + + def get_model(self, model_id): + if not model_id: + raise ValueError('Model Id is required for GetModel.') + if not isinstance(model_id, six.string_types): + raise TypeError('Model Id must be a string') + if not re.match(r'^[A-Za-z0-9_-]{1,60}$', model_id): + raise ValueError('Model Id format is invalid.') + try: + return self._request('get', 'models/{0}'.format(model_id)) + except requests.exceptions.RequestException as error: + raise _utils.handle_requests_error(error) + diff --git a/tests/test_mlkit.py b/tests/test_mlkit.py new file mode 100644 index 000000000..199331056 --- /dev/null +++ b/tests/test_mlkit.py @@ -0,0 +1,73 @@ +# Copyright 2019 Google Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Test cases for the firebase_admin.mlkit module.""" + +import json +import pytest +import six + +import firebase_admin +from firebase_admin import exceptions +from firebase_admin import mlkit +from tests import testutils + +PROJECT_ID = 'myProject1' +MODEL_ID_1 = 'modelId1' +DISPLAY_NAME_1 = 'displayName1' +MODEL_JSON_1 = { + 'name:': 'projects/{0}/models/{1}'.format(PROJECT_ID, MODEL_ID_1), + 'displayName': DISPLAY_NAME_1 +} +MODEL_1 = mlkit.Model(MODEL_JSON_1) +_DEFAULT_RESPONSE = json.dumps(MODEL_JSON_1) + + +class TestGetModel(object): + """Tests mlkit.get_model.""" + @classmethod + def setup_class(cls): + cred = testutils.MockCredential() + firebase_admin.initialize_app(cred, {'projectId': PROJECT_ID}) + + @classmethod + def teardown_class(cls): + testutils.cleanup_apps() + + def _get_url(self, project_id, model_id): + return mlkit._MLKitService.BASE_URL + 'projects/{0}/models/{1}'.format(project_id, model_id) + + def _instrument_mlkit_service(self, app=None, status=200, payload=_DEFAULT_RESPONSE): + if not app: + app = firebase_admin.get_app() + mlkit_service = mlkit._get_mlkit_service(app) + recorder = [] + mlkit_service._client.session.mount( + 'https://mlkit.googleapis.com', + testutils.MockAdapter(payload, status, recorder) + ) + return mlkit_service, recorder + + def test_get_model(self): + _, recorder = self._instrument_mlkit_service() + model = mlkit.get_model(MODEL_ID_1) + assert len(recorder) == 1 + assert recorder[0].method == 'GET' + assert recorder[0].url == self._get_url(PROJECT_ID, MODEL_ID_1) + #assert json.loads(recorder[0].body.decode()) == MODEL_JSON_1 + + #TODO(ifielker): test_get_model_error, test_get_model_no_project_id etc + + + From 976bf1ea3e3aa558cb71d48f762d2a2be73a8a8c Mon Sep 17 00:00:00 2001 From: ifielker Date: Mon, 19 Aug 2019 14:08:57 -0400 Subject: [PATCH 2/7] Added tests for get_model --- firebase_admin/mlkit.py | 13 ++++++++- tests/test_mlkit.py | 65 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/firebase_admin/mlkit.py b/firebase_admin/mlkit.py index a380b358a..05f11f184 100644 --- a/firebase_admin/mlkit.py +++ b/firebase_admin/mlkit.py @@ -19,11 +19,13 @@ """ import re +import requests import six import firebase_admin from firebase_admin import _http_client from firebase_admin import _utils +from firebase_admin import exceptions _MLKIT_ATTRIBUTE = '_mlkit' @@ -51,6 +53,15 @@ def __init__(self, data): """Created from a data dictionary.""" self._data = data + def __eq__(self, other): + if isinstance(other, self.__class__): + return self._data == other._data + else: + return False + + def __ne__(self, other): + return not self.__eq__(other) + #TODO(ifielker): define the Model properties etc class _MLKitService(object): @@ -87,7 +98,7 @@ def get_model(self, model_id): if not model_id: raise ValueError('Model Id is required for GetModel.') if not isinstance(model_id, six.string_types): - raise TypeError('Model Id must be a string') + raise TypeError('Model Id must be a string.') if not re.match(r'^[A-Za-z0-9_-]{1,60}$', model_id): raise ValueError('Model Id format is invalid.') try: diff --git a/tests/test_mlkit.py b/tests/test_mlkit.py index 199331056..6fd89d8fd 100644 --- a/tests/test_mlkit.py +++ b/tests/test_mlkit.py @@ -25,14 +25,27 @@ PROJECT_ID = 'myProject1' MODEL_ID_1 = 'modelId1' +MODEL_NAME_1 = 'projects/{0}/models/{1}'.format(PROJECT_ID, MODEL_ID_1) DISPLAY_NAME_1 = 'displayName1' MODEL_JSON_1 = { - 'name:': 'projects/{0}/models/{1}'.format(PROJECT_ID, MODEL_ID_1), + 'name': MODEL_NAME_1, 'displayName': DISPLAY_NAME_1 } MODEL_1 = mlkit.Model(MODEL_JSON_1) _DEFAULT_RESPONSE = json.dumps(MODEL_JSON_1) +ERROR_CODE = 404 +ERROR_MSG = 'The resource was not found' +ERROR_STATUS = 'NOT_FOUND' +ERROR_JSON = { + 'error': { + 'code': ERROR_CODE, + 'message': ERROR_MSG, + 'status': ERROR_STATUS + } +} +_ERROR_RESPONSE = json.dumps(ERROR_JSON) + class TestGetModel(object): """Tests mlkit.get_model.""" @@ -45,6 +58,18 @@ def setup_class(cls): def teardown_class(cls): testutils.cleanup_apps() + @staticmethod + def check_error(err, errType, msg): + assert isinstance(err, errType) + assert str(err) == msg + + @staticmethod + def check_firebase_error(err, code, status): + assert isinstance(err, exceptions.FirebaseError) + assert err.code == code + assert err.http_response is not None + assert err.http_response.status_code == status + def _get_url(self, project_id, model_id): return mlkit._MLKitService.BASE_URL + 'projects/{0}/models/{1}'.format(project_id, model_id) @@ -65,9 +90,43 @@ def test_get_model(self): assert len(recorder) == 1 assert recorder[0].method == 'GET' assert recorder[0].url == self._get_url(PROJECT_ID, MODEL_ID_1) - #assert json.loads(recorder[0].body.decode()) == MODEL_JSON_1 + assert model == MODEL_1 + assert model._data['name'] == MODEL_NAME_1 + assert model._data['displayName'] == DISPLAY_NAME_1 + + def test_get_model_validation_errors(self): + _, recorder = self._instrument_mlkit_service() + #Empty model-id + with pytest.raises(ValueError) as err: + mlkit.get_model('') + self.check_error(err.value, ValueError, 'Model Id is required for GetModel.') + + #Wrong type + with pytest.raises(TypeError) as err: + mlkit.get_model(12345) + self.check_error(err.value, TypeError, 'Model Id must be a string.') + + #Invalid characters + with pytest.raises(ValueError) as err: + mlkit.get_model('&_*#@:/?') + self.check_error(err.value, ValueError, 'Model Id format is invalid.') + + def test_get_model_error(self): + _, recorder = self._instrument_mlkit_service(status=404, payload=_ERROR_RESPONSE) + with pytest.raises(exceptions.NotFoundError) as err: + mlkit.get_model(MODEL_ID_1) + self.check_firebase_error(err.value, ERROR_STATUS, ERROR_CODE) + assert len(recorder) == 1 + assert recorder[0].method == 'GET' + assert recorder[0].url == self._get_url(PROJECT_ID, MODEL_ID_1) + + def test_no_project_id(self): + def evaluate(): + app = firebase_admin.initialize_app(testutils.MockCredential(), name='no_project_id') + with pytest.raises(ValueError): + mlkit.get_model(MODEL_ID_1, app) + testutils.run_without_project_id(evaluate) - #TODO(ifielker): test_get_model_error, test_get_model_no_project_id etc From b47e70b405c7e7162f059a0a4c2059676caa2bf2 Mon Sep 17 00:00:00 2001 From: ifielker Date: Mon, 19 Aug 2019 15:05:23 -0400 Subject: [PATCH 3/7] fixed lint error --- .travis.yml | 1 - firebase_admin/mlkit.py | 3 +-- tests/test_mlkit.py | 8 ++------ 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4db3c3708..c89a1db76 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,7 +16,6 @@ before_install: - nvm install 8 && npm install -g firebase-tools script: - pytest - - firebase emulators:exec --only database --project fake-project-id 'pytest integration/test_db.py' cache: pip: true npm: true diff --git a/firebase_admin/mlkit.py b/firebase_admin/mlkit.py index 05f11f184..a11813b78 100644 --- a/firebase_admin/mlkit.py +++ b/firebase_admin/mlkit.py @@ -55,7 +55,7 @@ def __init__(self, data): def __eq__(self, other): if isinstance(other, self.__class__): - return self._data == other._data + return self._data == other._data # pylint: disable=protected-access else: return False @@ -105,4 +105,3 @@ def get_model(self, model_id): return self._request('get', 'models/{0}'.format(model_id)) except requests.exceptions.RequestException as error: raise _utils.handle_requests_error(error) - diff --git a/tests/test_mlkit.py b/tests/test_mlkit.py index 6fd89d8fd..49b69d6fc 100644 --- a/tests/test_mlkit.py +++ b/tests/test_mlkit.py @@ -59,8 +59,8 @@ def teardown_class(cls): testutils.cleanup_apps() @staticmethod - def check_error(err, errType, msg): - assert isinstance(err, errType) + def check_error(err, err_type, msg): + assert isinstance(err, err_type) assert str(err) == msg @staticmethod @@ -126,7 +126,3 @@ def evaluate(): with pytest.raises(ValueError): mlkit.get_model(MODEL_ID_1, app) testutils.run_without_project_id(evaluate) - - - - From 95e7a7ae002e4e77db6703d17dda463f9dea77cd Mon Sep 17 00:00:00 2001 From: ifielker Date: Mon, 19 Aug 2019 15:42:56 -0400 Subject: [PATCH 4/7] fixed issues from reviewer comments --- firebase_admin/mlkit.py | 33 +++++++++++---------------------- tests/test_mlkit.py | 20 ++++++++++++++------ 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/firebase_admin/mlkit.py b/firebase_admin/mlkit.py index a11813b78..cbb180e6b 100644 --- a/firebase_admin/mlkit.py +++ b/firebase_admin/mlkit.py @@ -27,8 +27,10 @@ from firebase_admin import _utils from firebase_admin import exceptions + _MLKIT_ATTRIBUTE = '_mlkit' + def _get_mlkit_service(app): """ Returns an _MLKitService instance for an App. @@ -43,10 +45,12 @@ def _get_mlkit_service(app): """ return _utils.get_app_service(app, _MLKIT_ATTRIBUTE, _MLKitService) + def get_model(model_id, app=None): mlkit_service = _get_mlkit_service(app) return Model(mlkit_service.get_model(model_id)) + class Model(object): """A Firebase ML Kit Model object.""" def __init__(self, data): @@ -64,10 +68,10 @@ def __ne__(self, other): #TODO(ifielker): define the Model properties etc + class _MLKitService(object): """Firebase MLKit service.""" - BASE_URL = 'https://mlkit.googleapis.com/v1beta1/' PROJECT_URL = 'https://mlkit.googleapis.com/v1beta1/projects/{0}/' def __init__(self, app): @@ -79,29 +83,14 @@ def __init__(self, app): self._project_url = _MLKitService.PROJECT_URL.format(project_id) self._client = _http_client.JsonHttpClient(credential=app.credential.get_credential()) - def _request(self, method, urlpath, **kwargs): - """Makes an HTTP call using the Python requests library. - - Args: - method: HTTP method name as a string (e.g. get, post, patch, delete). - urlpath: URL path to the endpoint. This will be appended to the - server's base project URL. - kwargs: An additional set of keyword arguments to be passed into requests - API (e.g. json, params) - - Returns: - dict: The parsed JSON response. - """ - return self._client.body(method, url=self._project_url + urlpath, **kwargs) - def get_model(self, model_id): - if not model_id: - raise ValueError('Model Id is required for GetModel.') if not isinstance(model_id, six.string_types): - raise TypeError('Model Id must be a string.') + raise TypeError('Model ID must be a string.') if not re.match(r'^[A-Za-z0-9_-]{1,60}$', model_id): - raise ValueError('Model Id format is invalid.') + raise ValueError('Model ID format is invalid.') try: - return self._request('get', 'models/{0}'.format(model_id)) + return self._client.body( + 'get', + url=self._project_url + 'models/{0}'.format(model_id)) except requests.exceptions.RequestException as error: - raise _utils.handle_requests_error(error) + raise _utils.handle_platform_error_from_requests(error) diff --git a/tests/test_mlkit.py b/tests/test_mlkit.py index 49b69d6fc..856c3cf39 100644 --- a/tests/test_mlkit.py +++ b/tests/test_mlkit.py @@ -23,6 +23,8 @@ from firebase_admin import mlkit from tests import testutils +BASE_URL = 'https://mlkit.googleapis.com/v1beta1/' + PROJECT_ID = 'myProject1' MODEL_ID_1 = 'modelId1' MODEL_NAME_1 = 'projects/{0}/models/{1}'.format(PROJECT_ID, MODEL_ID_1) @@ -64,14 +66,15 @@ def check_error(err, err_type, msg): assert str(err) == msg @staticmethod - def check_firebase_error(err, code, status): + def check_firebase_error(err, code, status, msg): assert isinstance(err, exceptions.FirebaseError) assert err.code == code assert err.http_response is not None assert err.http_response.status_code == status + assert str(err) == msg def _get_url(self, project_id, model_id): - return mlkit._MLKitService.BASE_URL + 'projects/{0}/models/{1}'.format(project_id, model_id) + return BASE_URL + 'projects/{0}/models/{1}'.format(project_id, model_id) def _instrument_mlkit_service(self, app=None, status=200, payload=_DEFAULT_RESPONSE): if not app: @@ -99,23 +102,28 @@ def test_get_model_validation_errors(self): #Empty model-id with pytest.raises(ValueError) as err: mlkit.get_model('') - self.check_error(err.value, ValueError, 'Model Id is required for GetModel.') + self.check_error(err.value, ValueError, 'Model ID format is invalid.') + + #None model-id + with pytest.raises(TypeError) as err: + mlkit.get_model(None) + self.check_error(err.value, TypeError, 'Model ID must be a string.') #Wrong type with pytest.raises(TypeError) as err: mlkit.get_model(12345) - self.check_error(err.value, TypeError, 'Model Id must be a string.') + self.check_error(err.value, TypeError, 'Model ID must be a string.') #Invalid characters with pytest.raises(ValueError) as err: mlkit.get_model('&_*#@:/?') - self.check_error(err.value, ValueError, 'Model Id format is invalid.') + self.check_error(err.value, ValueError, 'Model ID format is invalid.') def test_get_model_error(self): _, recorder = self._instrument_mlkit_service(status=404, payload=_ERROR_RESPONSE) with pytest.raises(exceptions.NotFoundError) as err: mlkit.get_model(MODEL_ID_1) - self.check_firebase_error(err.value, ERROR_STATUS, ERROR_CODE) + self.check_firebase_error(err.value, ERROR_STATUS, ERROR_CODE, ERROR_MSG) assert len(recorder) == 1 assert recorder[0].method == 'GET' assert recorder[0].url == self._get_url(PROJECT_ID, MODEL_ID_1) From aa263aca3d8e94837fc31e7ca45fec195bcb8493 Mon Sep 17 00:00:00 2001 From: ifielker Date: Mon, 19 Aug 2019 15:54:19 -0400 Subject: [PATCH 5/7] fixed lint --- firebase_admin/mlkit.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/firebase_admin/mlkit.py b/firebase_admin/mlkit.py index cbb180e6b..0198b539c 100644 --- a/firebase_admin/mlkit.py +++ b/firebase_admin/mlkit.py @@ -22,10 +22,8 @@ import requests import six -import firebase_admin from firebase_admin import _http_client from firebase_admin import _utils -from firebase_admin import exceptions _MLKIT_ATTRIBUTE = '_mlkit' From 757cbc83fb80f6d0ec1474007f32666c019342f9 Mon Sep 17 00:00:00 2001 From: ifielker Date: Mon, 19 Aug 2019 16:02:59 -0400 Subject: [PATCH 6/7] more lint fixes --- tests/test_mlkit.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_mlkit.py b/tests/test_mlkit.py index 856c3cf39..85edaa4a1 100644 --- a/tests/test_mlkit.py +++ b/tests/test_mlkit.py @@ -16,7 +16,6 @@ import json import pytest -import six import firebase_admin from firebase_admin import exceptions @@ -98,7 +97,6 @@ def test_get_model(self): assert model._data['displayName'] == DISPLAY_NAME_1 def test_get_model_validation_errors(self): - _, recorder = self._instrument_mlkit_service() #Empty model-id with pytest.raises(ValueError) as err: mlkit.get_model('') From b688250bcb13395dffaa938e1d2b423cd2a2123d Mon Sep 17 00:00:00 2001 From: ifielker Date: Mon, 19 Aug 2019 16:21:33 -0400 Subject: [PATCH 7/7] fixed url handling --- firebase_admin/mlkit.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/firebase_admin/mlkit.py b/firebase_admin/mlkit.py index 0198b539c..e86a827e0 100644 --- a/firebase_admin/mlkit.py +++ b/firebase_admin/mlkit.py @@ -79,7 +79,9 @@ def __init__(self, app): 'Project ID is required to access MLKit service. Either set the ' 'projectId option, or use service account credentials.') self._project_url = _MLKitService.PROJECT_URL.format(project_id) - self._client = _http_client.JsonHttpClient(credential=app.credential.get_credential()) + self._client = _http_client.JsonHttpClient( + credential=app.credential.get_credential(), + base_url=self._project_url) def get_model(self, model_id): if not isinstance(model_id, six.string_types): @@ -87,8 +89,6 @@ def get_model(self, model_id): if not re.match(r'^[A-Za-z0-9_-]{1,60}$', model_id): raise ValueError('Model ID format is invalid.') try: - return self._client.body( - 'get', - url=self._project_url + 'models/{0}'.format(model_id)) + return self._client.body('get', url='models/{0}'.format(model_id)) except requests.exceptions.RequestException as error: raise _utils.handle_platform_error_from_requests(error)