From c0ee6224d9aa744d0e9ea384078cffd4a711727d Mon Sep 17 00:00:00 2001 From: Matheus Lima Date: Tue, 19 Mar 2019 12:42:16 -0300 Subject: [PATCH] Created functions to epire etags from the device entity and the device's settings entity, at device and account level. Fixed the query used to fetch the devices from a user --- api/public/public_api/endpoints/device.py | 6 +- .../public_api/endpoints/device_setting.py | 7 +-- api/public/tests/features/environment.py | 3 + api/public/tests/features/get_device.feature | 2 +- .../features/get_device_settings.feature | 7 ++- api/public/tests/features/steps/get_device.py | 20 +++---- .../features/steps/get_device_settings.py | 31 +++++++--- shared/selene/api/__init__.py | 1 + shared/selene/api/etag.py | 58 +++++++++++++++++++ shared/selene/api/public_endpoint.py | 11 +--- shared/selene/data/device/entity/geography.py | 1 - .../sql/get_devices_by_account_id.sql | 9 ++- 12 files changed, 111 insertions(+), 45 deletions(-) create mode 100644 shared/selene/api/etag.py diff --git a/api/public/public_api/endpoints/device.py b/api/public/public_api/endpoints/device.py index 1f72e44a..313cf023 100644 --- a/api/public/public_api/endpoints/device.py +++ b/api/public/public_api/endpoints/device.py @@ -5,6 +5,7 @@ from schematics import Model from schematics.types import StringType from selene.api import PublicEndpoint +from selene.api import device_etag_key from selene.data.device import DeviceRepository from selene.util.db import get_db_connection @@ -23,8 +24,7 @@ class DeviceEndpoint(PublicEndpoint): def get(self, device_id): self._authenticate(device_id) - etag_key = 'device.etag:{uuid}'.format(uuid=device_id) - self._validate_etag(etag_key) + self._validate_etag(device_etag_key(device_id)) with get_db_connection(self.config['DB_CONNECTION_POOL']) as db: device = DeviceRepository(db).get_device_by_id(device_id) if device: @@ -38,7 +38,7 @@ class DeviceEndpoint(PublicEndpoint): del device['account_id'] response = device, HTTPStatus.OK - self._add_etag(etag_key) + self._add_etag(device_etag_key(device_id)) else: response = '', HTTPStatus.NO_CONTENT return response diff --git a/api/public/public_api/endpoints/device_setting.py b/api/public/public_api/endpoints/device_setting.py index ac41c5ac..d8d78677 100644 --- a/api/public/public_api/endpoints/device_setting.py +++ b/api/public/public_api/endpoints/device_setting.py @@ -1,6 +1,6 @@ from http import HTTPStatus -from selene.api import PublicEndpoint +from selene.api import PublicEndpoint, device_setting_etag_key from selene.data.device import SettingRepository from selene.util.db import get_db_connection @@ -12,13 +12,12 @@ class DeviceSettingEndpoint(PublicEndpoint): def get(self, device_id): self._authenticate(device_id) - etag_key = 'device.setting.etag:{uuid}'.format(uuid=device_id) - self._validate_etag(etag_key) + self._validate_etag(device_setting_etag_key(device_id)) with get_db_connection(self.config['DB_CONNECTION_POOL']) as db: setting = SettingRepository(db).get_device_settings(device_id) if setting is not None: response = (setting, HTTPStatus.OK) - self._add_etag(etag_key) + self._add_etag(device_setting_etag_key(device_id)) else: response = ('', HTTPStatus.NO_CONTENT) return response diff --git a/api/public/tests/features/environment.py b/api/public/tests/features/environment.py index 15f84ac1..b74b80de 100644 --- a/api/public/tests/features/environment.py +++ b/api/public/tests/features/environment.py @@ -5,6 +5,7 @@ from behave import fixture, use_fixture from public_api.api import public from selene.api import generate_device_login +from selene.api.etag import ETagManager from selene.data.account import ( Account, AccountRepository, @@ -55,6 +56,8 @@ def before_feature(context, _): def before_scenario(context, _): + cache = context.client_config['SELENE_CACHE'] + context.etag_manager = ETagManager(cache, context.client_config) with get_db_connection(context.client_config['DB_CONNECTION_POOL']) as db: _add_agreements(context, db) _add_account(context, db) diff --git a/api/public/tests/features/get_device.feature b/api/public/tests/features/get_device.feature index ea7a187f..40d8d44c 100644 --- a/api/public/tests/features/get_device.feature +++ b/api/public/tests/features/get_device.feature @@ -24,7 +24,7 @@ Feature: Get device's information Then 304 status code should be returned by the device endpoint Scenario: Get a device using an expired etag - Given an etag expired by selene ui + Given a device's etag expired by the web ui When try to fetch a device using an expired etag Then should return status 200 And a new etag \ No newline at end of file diff --git a/api/public/tests/features/get_device_settings.feature b/api/public/tests/features/get_device_settings.feature index b5c7b442..fea6a005 100644 --- a/api/public/tests/features/get_device_settings.feature +++ b/api/public/tests/features/get_device_settings.feature @@ -15,6 +15,11 @@ Feature: Retrieve device's settings Then 304 status code should be returned by the device's settings endpoint Scenario: Try to get a device's settings using a expired etag - Given a device's setting with a valid etag + Given a device's setting etag expired by the web ui at device level When try to fetch the device's settings using an expired etag Then 200 status code should be returned by the device's setting endpoint and a new etag + + Scenario: Try to get a device's settings using a expired etag + Given a device's setting etag expired by the web ui at account level + When try to fetch the device's settings using an expired etag + Then 200 status code should be returned by the device's setting endpoint and a new etag \ No newline at end of file diff --git a/api/public/tests/features/steps/get_device.py b/api/public/tests/features/steps/get_device.py index b81197c9..e44f1750 100644 --- a/api/public/tests/features/steps/get_device.py +++ b/api/public/tests/features/steps/get_device.py @@ -5,7 +5,7 @@ from http import HTTPStatus from behave import when, then, given from hamcrest import assert_that, equal_to, has_key, not_none, is_not -from selene.util.cache import SeleneCache +from selene.api.etag import ETagManager, device_etag_key new_fields = dict( platform='mycroft_mark_1', @@ -93,14 +93,9 @@ def validate_update(context): @given('a device with a valid etag') def get_device_etag(context): - access_token = context.device_login['accessToken'] - headers = dict(Authorization='Bearer {token}'.format(token=access_token)) + etag_manager: ETagManager = context.etag_manager device_id = context.device_login['uuid'] - context.get_device_response = context.client.get( - '/v1/device/{uuid}'.format(uuid=device_id), - headers=headers - ) - context.device_etag = context.get_device_response.headers.get('ETag') + context.device_etag = etag_manager.get(device_etag_key(device_id)) @when('try to fetch a device using a valid etag') @@ -125,13 +120,12 @@ def validate_etag(context): assert_that(response.status_code, equal_to(HTTPStatus.NOT_MODIFIED)) -@given('an etag expired by selene ui') +@given('a device\'s etag expired by the web ui') def expire_etag(context): - context.device_etag = '123' - new_etag = '456' + etag_manager: ETagManager = context.etag_manager device_id = context.device_login['uuid'] - cache: SeleneCache = context.client_config['SELENE_CACHE'] - cache.set('device.etag:{uuid}'.format(uuid=device_id), new_etag) + context.device_etag = etag_manager.get(device_etag_key(device_id)) + etag_manager.expire_device_etag_by_device_id(device_id) @when('try to fetch a device using an expired etag') diff --git a/api/public/tests/features/steps/get_device_settings.py b/api/public/tests/features/steps/get_device_settings.py index 4ac80f17..9d40c087 100644 --- a/api/public/tests/features/steps/get_device_settings.py +++ b/api/public/tests/features/steps/get_device_settings.py @@ -5,6 +5,8 @@ from http import HTTPStatus from behave import when, then, given from hamcrest import assert_that, equal_to, has_key, is_not +from selene.api.etag import ETagManager, device_setting_etag_key + @when('try to fetch device\'s setting') def get_device_settings(context): @@ -49,14 +51,9 @@ def validate_response(context): @given('a device\'s setting with a valid etag') def get_device_setting_etag(context): - access_token = context.device_login['accessToken'] - headers = dict(Authorization='Bearer {token}'.format(token=access_token)) device_id = context.device_login['uuid'] - context.get_device_response = context.client.get( - '/v1/device/{uuid}/setting'.format(uuid=device_id), - headers=headers - ) - context.device_etag = context.get_device_response.headers.get('ETag') + etag_manager: ETagManager = context.etag_manager + context.device_etag = etag_manager.get(device_setting_etag_key(device_id)) @when('try to fetch the device\'s settings using a valid etag') @@ -80,10 +77,26 @@ def validate_etag_response(context): assert_that(response.status_code, equal_to(HTTPStatus.NOT_MODIFIED)) +@given('a device\'s setting etag expired by the web ui at device level') +def expire_etag_device_level(context): + device_id = context.device_login['uuid'] + etag_manager: ETagManager = context.etag_manager + context.device_etag = etag_manager.get(device_setting_etag_key(device_id)) + etag_manager.expire_device_setting_etag_by_device_id(device_id) + + +@given('a device\'s setting etag expired by the web ui at account level') +def expire_etag_account_level(context): + account_id = context.account.id + device_id = context.device_login['uuid'] + etag_manager: ETagManager = context.etag_manager + context.device_etag = etag_manager.get(device_setting_etag_key(device_id)) + etag_manager.expire_device_setting_etag_by_account_id(account_id) + + @when('try to fetch the device\'s settings using an expired etag') def get_device_settings_using_etag(context): - etag = '123' - context.device_etag = etag + etag = context.device_etag access_token = context.device_login['accessToken'] device_id = context.device_login['uuid'] headers = { diff --git a/shared/selene/api/__init__.py b/shared/selene/api/__init__.py index 9b49f653..13216302 100644 --- a/shared/selene/api/__init__.py +++ b/shared/selene/api/__init__.py @@ -1,6 +1,7 @@ from .base_config import get_base_config from .base_endpoint import APIError, SeleneEndpoint from .blueprint import selene_api +from .etag import device_etag_key, device_setting_etag_key from .public_endpoint import PublicEndpoint from .public_endpoint import generate_device_login from .response import SeleneResponse, snake_to_camel diff --git a/shared/selene/api/etag.py b/shared/selene/api/etag.py new file mode 100644 index 00000000..81dec581 --- /dev/null +++ b/shared/selene/api/etag.py @@ -0,0 +1,58 @@ +import random +import string + +from selene.data.device import DeviceRepository +from selene.util.cache import SeleneCache +from selene.util.db import get_db_connection + + +def device_etag_key(device_id: str): + return 'device.etag:{uuid}'.format(uuid=device_id) + + +def device_setting_etag_key(device_id: str): + return 'device.setting.etag:{uuid}'.format(uuid=device_id) + + +class ETagManager(object): + """Class responsible for generate and expire etags""" + + etag_chars = string.ascii_letters + string.digits + + def __init__(self, cache: SeleneCache, config: dict): + self.cache: SeleneCache = cache + self.db_connection_pool = config['DB_CONNECTION_POOL'] + + def get(self, key: str) -> str: + """Generate a etag with 32 random chars and store it into a given key + :param key: key where the etag will be stored + :return etag""" + etag = self.cache.get(key) + if etag is None: + etag = ''.join(random.choice(self.etag_chars) for _ in range(32)) + self.cache.set(key, etag) + return etag + + def _expire(self, key): + """Expires an existent etag + :param key: key where the etag is stored""" + etag = ''.join(random.choice(self.etag_chars) for _ in range(32)) + self.cache.set(key, etag) + + def expire_device_etag_by_device_id(self, device_id: str): + """Expire the etag associated with a device entity + :param device_id: device uuid""" + self._expire(device_etag_key(device_id)) + + def expire_device_setting_etag_by_device_id(self, device_id: str): + """Expire the etag associated with a device's settings entity + :param device_id: device uuid""" + self._expire(device_setting_etag_key(device_id)) + + def expire_device_setting_etag_by_account_id(self, account_id): + """Expire the settings' etags for all devices from a given account. Used when the settings are updated + at account level""" + with get_db_connection(self.db_connection_pool) as db: + devices = DeviceRepository(db).get_devices_by_account_id(account_id) + for device in devices: + self.expire_device_setting_etag_by_device_id(device.id) diff --git a/shared/selene/api/public_endpoint.py b/shared/selene/api/public_endpoint.py index a08db2ad..ed0483f2 100644 --- a/shared/selene/api/public_endpoint.py +++ b/shared/selene/api/public_endpoint.py @@ -1,12 +1,11 @@ import hashlib import json -import random -import string import uuid from flask import current_app, request, Response, after_this_request from flask.views import MethodView +from selene.api.etag import ETagManager from selene.util.auth import AuthenticationError from selene.util.not_modified import NotModifiedError from ..util.cache import SeleneCache @@ -61,12 +60,11 @@ def generate_device_login(device_id: str, cache: SeleneCache) -> dict: class PublicEndpoint(MethodView): """Abstract class for all endpoints used by Mycroft devices""" - etag_chars = string.ascii_letters + string.digits - def __init__(self): self.config: dict = current_app.config self.request = request self.cache: SeleneCache = self.config['SELENE_CACHE'] + self.etag_manager: ETagManager = ETagManager(self.cache, self.config) def _authenticate(self, device_id: str = None): headers = self.request.headers @@ -90,10 +88,7 @@ class PublicEndpoint(MethodView): def _add_etag(self, key): """Add a etag header to the response. We try to get the etag from the cache using the given key. If the cache has the etag, we use it, otherwise we generate a etag, store it and add it to the response""" - etag = self.cache.get(key) - if etag is None: - etag = ''.join(random.choice(self.etag_chars) for _ in range(32)) - self.cache.set(key, etag) + etag = self.etag_manager.get(key) @after_this_request def set_etag_header(response: Response): diff --git a/shared/selene/data/device/entity/geography.py b/shared/selene/data/device/entity/geography.py index 7afbcc63..41e52cf2 100644 --- a/shared/selene/data/device/entity/geography.py +++ b/shared/selene/data/device/entity/geography.py @@ -4,6 +4,5 @@ from dataclasses import dataclass @dataclass class Geography(object): country: str - postal_code: str time_zone: str id: str = None diff --git a/shared/selene/data/device/repository/sql/get_devices_by_account_id.sql b/shared/selene/data/device/repository/sql/get_devices_by_account_id.sql index 5554f99b..85f3b9f0 100644 --- a/shared/selene/data/device/repository/sql/get_devices_by_account_id.sql +++ b/shared/selene/data/device/repository/sql/get_devices_by_account_id.sql @@ -18,15 +18,14 @@ SELECT 'id', tts.id ) AS text_to_speech, json_build_object( - 'id', l.id, - 'country', l.country, - 'postal_code', l.postal_code, - 'time_zone', l.time_zone + 'id', g.id, + 'country', g.country, + 'time_zone', g.time_zone ) AS geography FROM device.device d INNER JOIN device.wake_word ww ON d.wake_word_id = ww.id INNER JOIN device.text_to_speech tts ON d.text_to_speech_id = tts.id - LEFT JOIN device.location l ON d.location_id = l.id + LEFT JOIN device.geography g ON d.geography_id = g.id WHERE d.account_id = %(account_id)s