From eccb83ed44c4bcd3c8399467a7cd7004a2949e93 Mon Sep 17 00:00:00 2001 From: Matheus Lima Date: Mon, 25 Feb 2019 19:03:08 -0300 Subject: [PATCH] - Refactoring to use a AccountRepository to get an account using the device id - Created tests for the metrics service --- api/public/api.py | 30 -------- api/public/public_api/api.py | 17 ++--- .../public_api/endpoints/device_email.py | 8 +-- .../public_api/endpoints/device_metrics.py | 34 +++++---- .../endpoints/device_subscription.py | 10 ++- .../tests/features/device_metrics.feature | 12 ++++ .../tests/features/steps/device_metrics.py | 56 +++++++++++++++ .../features/steps/get_device_subscription.py | 2 +- .../selene/data/account/repository/account.py | 11 ++- .../sql/get_account_by_device_id.sql | 69 +++++++++++++++++++ .../selene/data/device/repository/device.py | 36 +--------- .../sql/get_account_email_by_device_id.sql | 8 --- .../sql/get_account_id_by_device_id.sql | 8 --- .../get_subscription_type_by_device_id.sql | 12 ---- 14 files changed, 192 insertions(+), 121 deletions(-) delete mode 100644 api/public/api.py create mode 100644 api/public/tests/features/device_metrics.feature create mode 100644 api/public/tests/features/steps/device_metrics.py create mode 100644 shared/selene/data/account/repository/sql/get_account_by_device_id.sql delete mode 100644 shared/selene/data/device/repository/sql/get_account_email_by_device_id.sql delete mode 100644 shared/selene/data/device/repository/sql/get_account_id_by_device_id.sql delete mode 100644 shared/selene/data/device/repository/sql/get_subscription_type_by_device_id.sql diff --git a/api/public/api.py b/api/public/api.py deleted file mode 100644 index 081a4d65..00000000 --- a/api/public/api.py +++ /dev/null @@ -1,30 +0,0 @@ -from flask import Flask -from flask_restful import Api -from selene.api import JSON_MIMETYPE, output_json - -from public_api.endpoints.device_metrics import DeviceMetricsEndpoint -from .endpoints.device import DeviceEndpoint -from .endpoints.device_setting import DeviceSettingEndpoint -from .endpoints.device_skill import DeviceSkillEndpoint -from .endpoints.device_skills import DeviceSkillsEndpoint -from selene.api.base_config import get_base_config - -from .endpoints.device_subscription import DeviceSubscriptionEndpoint -from .endpoints.open_weather_map import OpenWeatherMapEndpoint -from .endpoints.wolfram_alpha import WolframAlphaEndpoint - -public = Flask(__name__) -public.config.from_object(get_base_config()) -public_api = Api(public) -public_api.representations[JSON_MIMETYPE] = output_json - -public_api.representations.update() - -public_api.add_resource(DeviceSkillsEndpoint, '/device//skill') -public_api.add_resource(DeviceSkillEndpoint, '/device//userSkill') -public_api.add_resource(DeviceEndpoint, '/device/') -public_api.add_resource(DeviceSettingEndpoint, '/device//setting') -public_api.add_resource(DeviceSubscriptionEndpoint, '/device//subscription') -public_api.add_resource(WolframAlphaEndpoint, '/wa') # TODO: change this path in the API v2 -public_api.add_resource(OpenWeatherMapEndpoint, '/owm/') # TODO: change this path in the API v2 -public_api.add_resource(DeviceMetricsEndpoint, '/device//metric/') \ No newline at end of file diff --git a/api/public/public_api/api.py b/api/public/public_api/api.py index ea387567..f07dda9f 100644 --- a/api/public/public_api/api.py +++ b/api/public/public_api/api.py @@ -3,23 +3,22 @@ import smtplib from flask import Flask +from selene.api import SeleneResponse, selene_api from selene.api.base_config import get_base_config from selene.util.cache import SeleneCache -from selene.api import SeleneResponse, selene_api - +from .endpoints.account_device import AccountDeviceEndpoint from .endpoints.device import DeviceEndpoint +from .endpoints.device_activate import DeviceActivateEndpoint +from .endpoints.device_code import DeviceCodeEndpoint +from .endpoints.device_email import DeviceEmailEndpoint +from .endpoints.device_metrics import DeviceMetricsEndpoint, MetricsService from .endpoints.device_setting import DeviceSettingEndpoint from .endpoints.device_skill import DeviceSkillEndpoint from .endpoints.device_skills import DeviceSkillsEndpoint from .endpoints.device_subscription import DeviceSubscriptionEndpoint +from .endpoints.google_stt import GoogleSTTEndpoint from .endpoints.open_weather_map import OpenWeatherMapEndpoint from .endpoints.wolfram_alpha import WolframAlphaEndpoint -from .endpoints.google_stt import GoogleSTTEndpoint -from .endpoints.device_code import DeviceCodeEndpoint -from .endpoints.device_activate import DeviceActivateEndpoint -from .endpoints.account_device import AccountDeviceEndpoint -from .endpoints.device_email import DeviceEmailEndpoint -from .endpoints.device_metrics import DeviceMetricsEndpoint public = Flask(__name__) public.config.from_object(get_base_config()) @@ -35,6 +34,8 @@ email_client = smtplib.SMTP(host, port) email_client.login(user, password) public.config['EMAIL_CLIENT'] = email_client +public.config['METRICS_SERVICE'] = MetricsService() + public.response_class = SeleneResponse public.register_blueprint(selene_api) diff --git a/api/public/public_api/endpoints/device_email.py b/api/public/public_api/endpoints/device_email.py index 037f7e5c..801601c3 100644 --- a/api/public/public_api/endpoints/device_email.py +++ b/api/public/public_api/endpoints/device_email.py @@ -7,7 +7,7 @@ from schematics import Model from schematics.types import StringType from selene.api import SeleneEndpoint -from selene.data.device import DeviceRepository +from selene.data.account import AccountRepository from selene.util.db import get_db_connection @@ -30,14 +30,14 @@ class DeviceEmailEndpoint(SeleneEndpoint): send_email.validate() with get_db_connection(self.config['DB_CONNECTION_POOL']) as db: - email_address = DeviceRepository(db).get_account_email_by_device_id(device_id) + account = AccountRepository(db).get_account_by_device_id(device_id) - if email_address: + if account: message = EmailMessage() message['Subject'] = str(send_email.title) message['From'] = str(send_email.sender) message.set_content(str(send_email.body)) - message['To'] = email_address + message['To'] = account.email_address self.email_client.send_message(message) self.email_client.quit() response = '', HTTPStatus.OK diff --git a/api/public/public_api/endpoints/device_metrics.py b/api/public/public_api/endpoints/device_metrics.py index 081560dd..e099ef28 100644 --- a/api/public/public_api/endpoints/device_metrics.py +++ b/api/public/public_api/endpoints/device_metrics.py @@ -1,31 +1,41 @@ +import json import os from http import HTTPStatus import requests + from selene.api import SeleneEndpoint -from selene.data.device import DeviceRepository +from selene.data.account import AccountRepository from selene.util.db import get_db_connection +class MetricsService(object): + def __init__(self): + self.metrics_service_host = os.environ['METRICS_SERVICE_HOST'] + + def send_metric(self, metric: str, user_id: str, device_id: str, data: dict): + body = dict( + userUuid=user_id, + deviceUuid=device_id, + data=data + ) + url = '{host}/{metric}'.format(host=self.metrics_service_host, metric=metric) + requests.post(url, body) + + class DeviceMetricsEndpoint(SeleneEndpoint): """Endpoint to communicate with the metrics service""" def __init__(self): super(DeviceMetricsEndpoint, self).__init__() - self.metrics_service_host = os.environ['METRICS_SERVICE_HOST'] + self.metrics_service: MetricsService = self.config['METRICS_SERVICE'] def post(self, device_id, metric): - payload = self.request.get_json() + payload = json.loads(self.request.data) with get_db_connection(self.config['DB_CONNECTION_POOL']) as db: - account_id = DeviceRepository(db).get_account_id_by_device_id(device_id) - if account_id: - body = dict( - userUuid=account_id, - deviceUuid=device_id, - data=payload - ) - url = '{host}/{metric}'.format(host=self.metrics_service_host, metric=metric) - requests.post(url, body) + account = AccountRepository(db).get_account_by_device_id(device_id) + if account: + self.metrics_service.send_metric(metric, account.id, device_id, payload) response = '', HTTPStatus.OK else: response = '', HTTPStatus.NO_CONTENT diff --git a/api/public/public_api/endpoints/device_subscription.py b/api/public/public_api/endpoints/device_subscription.py index c91d08b5..286bcb99 100644 --- a/api/public/public_api/endpoints/device_subscription.py +++ b/api/public/public_api/endpoints/device_subscription.py @@ -1,7 +1,7 @@ from http import HTTPStatus from selene.api import SeleneEndpoint -from selene.data.device import DeviceRepository +from selene.data.account import AccountRepository from selene.util.db import get_db_connection @@ -11,6 +11,10 @@ class DeviceSubscriptionEndpoint(SeleneEndpoint): def get(self, device_id): with get_db_connection(self.config['DB_CONNECTION_POOL']) as db: - subscription = DeviceRepository(db).get_subscription_type_by_device_id(device_id) - response = (subscription, HTTPStatus.OK) if subscription is not None else ('', HTTPStatus.NO_CONTENT) + account = AccountRepository(db).get_account_by_device_id(device_id) + if account: + subscription = account.subscription + response = {'@type': subscription.type if subscription is not None else 'free'}, HTTPStatus.OK + else: + response = '', HTTPStatus.NO_CONTENT return response diff --git a/api/public/tests/features/device_metrics.feature b/api/public/tests/features/device_metrics.feature new file mode 100644 index 00000000..d76436b6 --- /dev/null +++ b/api/public/tests/features/device_metrics.feature @@ -0,0 +1,12 @@ +Feature: Send a metric to the metric service + + Scenario: a metric is sent to the metric endpoint by a valid device + Given a device pairing code + When a device is added to an account using the pairing code + And device is activated + And the metric is sent + Then 200 status code should be returned + + Scenario: a metric is sent by an invalid device + When the metric is sent by an invalid device + Then metrics endpoint should return 204 \ No newline at end of file diff --git a/api/public/tests/features/steps/device_metrics.py b/api/public/tests/features/steps/device_metrics.py new file mode 100644 index 00000000..fe82a087 --- /dev/null +++ b/api/public/tests/features/steps/device_metrics.py @@ -0,0 +1,56 @@ +import json +import uuid +from http import HTTPStatus +from unittest import mock +from unittest.mock import patch, MagicMock, call + +from behave import when, then +from hamcrest import assert_that, equal_to + +payload = dict( + type='timing', + start='123' +) + + +@when('the metric is sent') +@patch('public_api.endpoints.device_metrics.MetricsService') +def send_metric(context, metrics_service): + context.client_config['METRICS_SERVICE'] = metrics_service + context.response = context.client.post( + '/device/{uuid}/metric/{metric}'.format(uuid=context.device_id, metric='timing'), + data=json.dumps(payload), + content_type='application_json' + ) + + +@then('200 status code should be returned') +def validate_response(context): + response = context.response + assert_that(response.status_code, equal_to(HTTPStatus.OK)) + metrics_service: MagicMock = context.client_config['METRICS_SERVICE'] + metrics_service.send_metric.assert_has_calls([ + call('timing', + context.account.id, + context.device_id, + mock.ANY) + ]) + + +@when('the metric is sent by an invalid device') +@patch('public_api.endpoints.device_metrics.MetricsService') +def send_metrics_invalid(context, metrics_service): + context.client_config['METRICS_SERVICE'] = metrics_service + context.response = context.client.post( + '/device/{uuid}/metric/{metric}'.format(uuid=str(uuid.uuid4()), metric='timing'), + data=json.dumps(payload), + content_type='application_json' + ) + + +@then('metrics endpoint should return 204') +def validate_invalid_device(context): + response = context.response + assert_that(response.status_code, equal_to(HTTPStatus.NO_CONTENT)) + metrics_service: MagicMock = context.client_config['METRICS_SERVICE'] + metrics_service.send_metric.assert_not_called() diff --git a/api/public/tests/features/steps/get_device_subscription.py b/api/public/tests/features/steps/get_device_subscription.py index 97aaa07f..059c820f 100644 --- a/api/public/tests/features/steps/get_device_subscription.py +++ b/api/public/tests/features/steps/get_device_subscription.py @@ -36,7 +36,7 @@ def validate_response_monthly(context): response = context.subscription_response assert_that(response.status_code, HTTPStatus.OK) subscription = json.loads(response.data) - assert_that(subscription, has_entry('@type', 'month')) + assert_that(subscription, has_entry('@type', 'Monthly Supporter')) @when('try to get the subscription for a nonexistent device') diff --git a/shared/selene/data/account/repository/account.py b/shared/selene/data/account/repository/account.py index 375e55a7..7092ef7a 100644 --- a/shared/selene/data/account/repository/account.py +++ b/shared/selene/data/account/repository/account.py @@ -1,8 +1,9 @@ from logging import getLogger -from passlib.hash import sha512_crypt from os import environ, path from typing import List +from passlib.hash import sha512_crypt + from selene.util.db import ( DatabaseRequest, Cursor, @@ -164,3 +165,11 @@ class AccountRepository(object): account = Account(**result['account']) return account + + def get_account_by_device_id(self, device_id) -> Account: + """Return an account using the id of the device associated to the account""" + request = DatabaseRequest( + sql=get_sql_from_file(path.join(SQL_DIR, 'get_account_by_device_id.sql')), + args=dict(device_id=device_id) + ) + return self._get_account(request) diff --git a/shared/selene/data/account/repository/sql/get_account_by_device_id.sql b/shared/selene/data/account/repository/sql/get_account_by_device_id.sql new file mode 100644 index 00000000..0a725dd1 --- /dev/null +++ b/shared/selene/data/account/repository/sql/get_account_by_device_id.sql @@ -0,0 +1,69 @@ +WITH + refresh_tokens AS ( + SELECT + array_agg(refresh_token) + FROM + account.refresh_token acc_ref + INNER JOIN + account.account acc ON acc_ref.account_id = acc.id + INNER JOIN + device.device dev ON acc.id = dev.account_id + WHERE + dev.id = %(device_id)s + ), + agreements AS ( + SELECT + array_agg( + json_build_object( + 'id', aa.id, + 'type', ag.agreement, + 'accept_date', aa.accept_date + ) + ) + FROM + account.account_agreement aa + INNER JOIN + account.agreement ag ON ag.id = aa.agreement_id + INNER JOIN + account.account acc ON aa.account_id = acc.id + INNER JOIN + device.device dev ON acc.id = dev.account_id + WHERE + dev.id = %(device_id)s + ), + subscription AS ( + SELECT + json_build_object( + 'id', asub.id, + 'type', s.subscription, + 'start_date', lower(asub.subscription_ts_range)::DATE, + 'stripe_customer_id', asub.stripe_customer_id + ) + FROM + account.account_subscription asub + INNER JOIN + account.subscription s ON asub.subscription_id = s.id + INNER JOIN + account.account acc ON asub.account_id = acc.id + INNER JOIN + device.device dev ON acc.id = dev.account_id + WHERE + dev.id = %(device_id)s + AND upper(asub.subscription_ts_range) IS NULL + ) +SELECT + json_build_object( + 'id', acc.id, + 'email_address', acc.email_address, + 'username', acc.username, + 'subscription', (SELECT * FROM subscription), + 'refresh_tokens', (SELECT * FROM refresh_tokens), + 'agreements', (SELECT * FROM agreements) + ) as account +FROM + account.account acc +INNER JOIN + device.device dev ON acc.id = dev.account_id +WHERE + dev.id = %(device_id)s + diff --git a/shared/selene/data/device/repository/device.py b/shared/selene/data/device/repository/device.py index 569f4bdf..e14d56f6 100644 --- a/shared/selene/data/device/repository/device.py +++ b/shared/selene/data/device/repository/device.py @@ -1,10 +1,10 @@ from os import path from typing import List +from selene.util.db import DatabaseRequest, get_sql_from_file, Cursor +from ..entity.device import Device from ..entity.text_to_speech import TextToSpeech from ..entity.wake_word import WakeWord -from ..entity.device import Device -from selene.util.db import DatabaseRequest, get_sql_from_file, Cursor SQL_DIR = path.join(path.dirname(__file__), 'sql') @@ -42,20 +42,6 @@ class DeviceRepository(object): sql_results = self.cursor.select_all(query) return [Device(**result) for result in sql_results] - def get_subscription_type_by_device_id(self, device_id): - """Return the type of subscription of device's owner - :param device_id: device uuid - """ - query = DatabaseRequest( - sql=get_sql_from_file(path.join(SQL_DIR, 'get_subscription_type_by_device_id.sql')), - args=dict(device_id=device_id) - ) - sql_result = self.cursor.select_one(query) - if sql_result: - rate_period = sql_result['rate_period'] - # TODO: Remove the @ in the API v2 - return {'@type': rate_period} if rate_period is not None else {'@type': 'free'} - def add_device(self, account_id: str, name: str, wake_word_id: str, text_to_speech_id: str): """ Creates a new device with a given name and associate it to an account""" # TODO: validate foreign keys @@ -130,21 +116,3 @@ class DeviceRepository(object): args=dict(text_to_speech_id=text_to_speech_id) ) self.cursor.delete(query) - - def get_account_email_by_device_id(self, device_id): - query = DatabaseRequest( - sql=get_sql_from_file(path.join(SQL_DIR, 'get_account_email_by_device_id.sql')), - args=dict(device_id=device_id) - ) - sql_result = self.cursor.select_one(query) - if sql_result: - return sql_result['email_address'] - - def get_account_id_by_device_id(self, device_id): - query = DatabaseRequest( - sql=get_sql_from_file(path.join(SQL_DIR, 'get_account_id_by_device_id.sql')), - args=dict(device_id=device_id) - ) - sql_result = self.cursor.select_one(query) - if sql_result: - return sql_result['id'] diff --git a/shared/selene/data/device/repository/sql/get_account_email_by_device_id.sql b/shared/selene/data/device/repository/sql/get_account_email_by_device_id.sql deleted file mode 100644 index 73043630..00000000 --- a/shared/selene/data/device/repository/sql/get_account_email_by_device_id.sql +++ /dev/null @@ -1,8 +0,0 @@ -SELECT - acc.email_address -FROM - account.account acc -INNER JOIN - device.device dev ON acc.id = dev.account_id -WHERE - dev.id = %(device_id)s \ No newline at end of file diff --git a/shared/selene/data/device/repository/sql/get_account_id_by_device_id.sql b/shared/selene/data/device/repository/sql/get_account_id_by_device_id.sql deleted file mode 100644 index 839eeda3..00000000 --- a/shared/selene/data/device/repository/sql/get_account_id_by_device_id.sql +++ /dev/null @@ -1,8 +0,0 @@ -SELECT - acc.id -FROM - account.account acc -INNER JOIN - device.device dev ON acc.id = dev.account_id -WHERE - dev.id = %(device_id)s \ No newline at end of file diff --git a/shared/selene/data/device/repository/sql/get_subscription_type_by_device_id.sql b/shared/selene/data/device/repository/sql/get_subscription_type_by_device_id.sql deleted file mode 100644 index d9a0cb47..00000000 --- a/shared/selene/data/device/repository/sql/get_subscription_type_by_device_id.sql +++ /dev/null @@ -1,12 +0,0 @@ -SELECT - sub.rate_period -FROM - device.device dev -INNER JOIN - account.account acc ON dev.account_id = acc.id -LEFT JOIN - account.account_subscription acc_sub ON acc.id = acc_sub.account_id -LEFT JOIN - account.subscription sub ON acc_sub.subscription_id = sub.id -WHERE - dev.id = %(device_id)s \ No newline at end of file