From 7a4d681a14b4a2dfcc143ceff7ed8e2afcf4ee75 Mon Sep 17 00:00:00 2001 From: Olivier Yiptong Date: Thu, 23 Apr 2015 10:01:09 -0400 Subject: [PATCH] Bug 1134550 - Serve suggested tiles with v3 endpoint while keeping v1/v2 compatibility Squashed commit of the following: commit f707c4cd38ba9700b5bf2ec7134a4fc6c084c6e6 Author: Olivier Yiptong Date: Wed Apr 22 16:26:43 2015 -0400 remove compatibility for old index file in environment.py (keep testing coverage though) commit 377bd03e53f55dc1b43c6dd9af9ef5be9c1d16a8 Author: Olivier Yiptong Date: Tue Apr 21 14:34:55 2015 -0400 add backward compatibility for legacy tile indexes commit e1b77c9b99c75d65606c0dc9d575f3debe5b8c6f Author: Olivier Yiptong Date: Mon Apr 20 11:57:34 2015 -0400 ignore wsgi.py module in coverage commit 7db0d4a865b13629ad61199116d398f3993cf51f Author: Olivier Yiptong Date: Mon Apr 20 11:57:10 2015 -0400 remove unsuded encryption module commit 2ea9ae1db8c91583018766993dd3433894014c20 Author: Olivier Yiptong Date: Mon Apr 20 11:50:29 2015 -0400 PEP8 fix commit 60906e4ba8b7e1e528669bd84c2851d7c3f9b07f Author: Olivier Yiptong Date: Mon Apr 20 11:48:31 2015 -0400 remove error checking for payload errors in v3. there are no payloads anymore commit 66244f01397f3cd942daa61628ba462100fa16a3 Author: Olivier Yiptong Date: Mon Apr 20 11:40:38 2015 -0400 initial v3 api commit 710e718a17644618385c47e1349d351ce8d003c6 Author: Olivier Yiptong Date: Mon Apr 6 17:06:43 2015 -0400 PEP8 adjustments commit 05ed0b4a4a361f9816e2331cb32aed72e21ebd63 Author: Olivier Yiptong Date: Mon Apr 6 16:59:31 2015 -0400 initial v3 api implementation and tests closes bug 1134550 closes #15 --- .coveragerc | 2 +- onyx/api/v1.py | 4 +- onyx/api/v2.py | 8 +-- onyx/api/v3.py | 88 +++++++++++++++++++++++++++++++ onyx/encryption.py | 32 ------------ onyx/environment.py | 6 +-- onyx/webapp.py | 3 ++ tests/api/test_v3.py | 106 ++++++++++++++++++++++++++++++++++++++ tests/base.py | 5 +- tests/test_environment.py | 59 +++++++++++++++++++++ 10 files changed, 270 insertions(+), 43 deletions(-) create mode 100644 onyx/api/v3.py delete mode 100644 onyx/encryption.py create mode 100644 tests/api/test_v3.py create mode 100644 tests/test_environment.py diff --git a/.coveragerc b/.coveragerc index aef3370..a957daa 100644 --- a/.coveragerc +++ b/.coveragerc @@ -2,7 +2,7 @@ branch = True source = onyx include = onyx/* -omit = onyx/tests/* +omit = onyx/tests/*,onyx/wsgi.py [report] show_missing = True diff --git a/onyx/api/v1.py b/onyx/api/v1.py index cae0ce2..897d33b 100644 --- a/onyx/api/v1.py +++ b/onyx/api/v1.py @@ -72,9 +72,9 @@ def fetch(): except: country = "ERROR" - localized = current_app.config['LINKS_LOCALIZATIONS'].get("%s/%s" % (country, locale)) + localized = current_app.config['LINKS_LOCALIZATIONS'].get("%s/%s" % (country, locale), {}).get('legacy') if localized is None: - localized = env.config.LINKS_LOCALIZATIONS.get("STAR/%s" % locale) + localized = env.config.LINKS_LOCALIZATIONS.get("STAR/%s" % locale, {}).get('legacy') if localized: # 303 hints to the client to always use GET for the redirect diff --git a/onyx/api/v2.py b/onyx/api/v2.py index 7e37d97..2d7fc39 100644 --- a/onyx/api/v2.py +++ b/onyx/api/v2.py @@ -57,9 +57,9 @@ def fetch(locale=None): except: country = "STAR" - localized = env.config.LINKS_LOCALIZATIONS.get("%s/%s" % (country, locale)) + localized = env.config.LINKS_LOCALIZATIONS.get("%s/%s" % (country, locale), {}).get('legacy') if localized is None: - localized = env.config.LINKS_LOCALIZATIONS.get("STAR/%s" % locale) + localized = env.config.LINKS_LOCALIZATIONS.get("STAR/%s" % locale, {}).get('legacy') if localized is not None: # 303 hints to the client to always use GET for the redirect @@ -85,7 +85,7 @@ def fetch(locale=None): return response -def handle_ping(ping_type): +def handle_ping(ping_type, api_version="2"): """ A ping handler that just logs the data it receives for further processing in the backend @@ -111,7 +111,7 @@ def handle_ping(ping_type): "ip": ip_addr, "ua": ua, "locale": locale, - "ver": "2", + "ver": api_version, }) env.statsd.incr("{0}_error".format(ping_type)) diff --git a/onyx/api/v3.py b/onyx/api/v3.py new file mode 100644 index 0000000..37a9fe2 --- /dev/null +++ b/onyx/api/v3.py @@ -0,0 +1,88 @@ +from flask import ( + Blueprint, + request, + make_response, + redirect +) +from onyx.environment import Environment +from onyx.api.v2 import handle_ping + + +links = Blueprint('v3_links', __name__, url_prefix='/v3/links') +env = Environment.instance() + + +@links.route('/fetch//', methods=['GET']) +@env.statsd.timer('v3_links_fetch') +def fetch(locale=None, channel=None): + """ + Given a locale, return locale-specific links if possible. + """ + ip_addrs = None + ip_addr = None + ua = None + + ip_addrs = request.headers.get('X-Forwarded-For') + if ip_addrs is None: + ip_addrs = request.remote_addr + + if ip_addrs is not None: + ip_addr = ip_addrs.split(',')[0] + + ua = request.headers.get('User-Agent') + + try: + country = env.geoip_db.country(ip_addr).country.iso_code + except: + country = "STAR" + + localized = env.config.LINKS_LOCALIZATIONS.get("%s/%s" % (country, locale), {}).get('ag') + if localized is None: + localized = env.config.LINKS_LOCALIZATIONS.get("STAR/%s" % locale, {}).get('ag') + + if localized is not None: + # 303 hints to the client to always use GET for the redirect + # ETag is handled by the directory link hosting server + response = make_response(redirect(localized, code=303)) + env.log_dict(name="application", action="fetch_served", message={ + "ip": ip_addrs, + "ua": ua, + "locale": locale, + "channel": channel, + "ver": "3", + }) + env.statsd.incr("fetch_success") + else: + response = make_response(('', 204)) + env.log_dict(name="application", action="fetch_locale_unavailable", message={ + "ip": ip_addrs, + "ua": ua, + "locale": locale, + "channel": channel, + "ver": "3", + }) + env.statsd.incr("fetch_locale_unavailable") + + return response + + +@links.route('/view', methods=['POST']) +@env.statsd.timer('v3_links_view') +def view(): + """ + Log impression ping sent from Firefox on each newtab open event + """ + return handle_ping("view", api_version="3") + + +@links.route('/click', methods=['POST']) +@env.statsd.timer('v3_links_click') +def click(): + """ + Log tile ping sent from Firefox on each tile action + """ + return handle_ping("click", api_version="3") + + +def register_routes(app): + app.register_blueprint(links) diff --git a/onyx/encryption.py b/onyx/encryption.py deleted file mode 100644 index 03f5b4c..0000000 --- a/onyx/encryption.py +++ /dev/null @@ -1,32 +0,0 @@ -import base64 -from flask import current_app -from Crypto.Cipher import AES -from Crypto import Random - - -def encrypt(message): - """ - Encrypt a message using AES-CFB - @param message the data to be encrypted - @returns (ciphertext, iv) base64-encoded byte-strings - """ - key = current_app.config['ENCRYPTION']['AES_KEY'] - iv = Random.new().read(AES.block_size) - cipher = AES.new(key, AES.MODE_CFB, iv) - ciphertext = cipher.encrypt(message) - return map(base64.b64encode, [ciphertext, iv]) - - -def decrypt(ciphertext, iv): - """ - Decrypt a ciphertext using AES-CFB - @param message the ciphertext to decrypt, a base64-encoded byte-string - @param iv the IV used for encryption - - @returns the original, encrypted message - """ - ciphertext, iv = map(base64.b64decode, [ciphertext, iv]) - key = current_app.config['ENCRYPTION']['AES_KEY'] - cipher = AES.new(key, AES.MODE_CFB, iv) - msg = cipher.decrypt(ciphertext) - return msg diff --git a/onyx/environment.py b/onyx/environment.py index 702f2dc..08da6f2 100644 --- a/onyx/environment.py +++ b/onyx/environment.py @@ -139,17 +139,17 @@ class Environment(object): raise EnvironmentUninitializedError("Cannot obtain instance if uninitialized") -def _read_tile_index_loop(env): +def _read_tile_index_loop(env, failure_sleep_duration=5, success_sleep_duration=15 * 60): """wait for 15 minutes (greenlet), then open tile index file and replace LINKS_LOCALIZATIONS""" while True: try: with open(os.path.join(env.config.TILE_INDEX_DIR, env.config.TILE_INDEX_FILE), "r") as fp: data = fp.read() env.config.LINKS_LOCALIZATIONS = ujson.decode(data) - gevent.sleep(15 * 60) + gevent.sleep(success_sleep_duration) except Exception, e: env.log_dict(name="application", action="gevent_tiles_update_error", message={ "err": e.message, "traceback": traceback.format_exc(), }) - gevent.sleep(5) + gevent.sleep(failure_sleep_duration) diff --git a/onyx/webapp.py b/onyx/webapp.py index d65b9a1..aee18ce 100644 --- a/onyx/webapp.py +++ b/onyx/webapp.py @@ -4,3 +4,6 @@ def setup_routes(app): import onyx.api.v2 onyx.api.v2.register_routes(app) + + import onyx.api.v3 + onyx.api.v3.register_routes(app) diff --git a/tests/api/test_v3.py b/tests/api/test_v3.py new file mode 100644 index 0000000..89cdda1 --- /dev/null +++ b/tests/api/test_v3.py @@ -0,0 +1,106 @@ +import json +from flask import url_for +from nose.tools import assert_equals +from tests.base import BaseTestCase + + +class TestNewtabServing(BaseTestCase): + + def test_unknown_locale(self): + """ + A call with an unknown locale yields an HTTP 204 response + """ + response = self.client.get( + url_for('v3_links.fetch', locale='zh-CN', channel='beta'), + content_type='application/json', + headers=[("User-Agent", "TestClient")]) + assert_equals(response.status_code, 204) + assert_equals(int(response.headers.get('Content-Length')), 0) + + def test_unknown_country(self): + """ + A call with an unknown country, but valid locale is success because of STAR + """ + response = self.client.get( + url_for('v3_links.fetch', locale='en-US', channel='beta'), + content_type='application/json', + headers=[("User-Agent", "TestClient")], + environ_base={"REMOTE_ADDR": "202.224.135.69"}) + assert_equals(response.status_code, 303) + + def test_success(self): + """ + A call with an known geo/locale pair redirects + """ + response = self.client.get( + url_for('v3_links.fetch', locale='en-US', channel='beta'), + content_type='application/json', + headers=[("User-Agent", "TestClient")], + environ_base={"REMOTE_ADDR": "173.194.43.105"}) + assert_equals(response.status_code, 303) + + +class TestClickPing(BaseTestCase): + + def test_missing_payload(self): + """ + A click ping call without a payload errors + """ + response = self.client.post(url_for('v3_links.click'), + content_type='application/json', + headers=[("User-Agent", "TestClient")]) + assert_equals(response.status_code, 400) + + def test_empty_payload(self): + """ + A click ping call with an empty payload should pass + """ + response = self.client.post(url_for('v3_links.click'), + content_type='application/json', + headers=[("User-Agent", "TestClient")], + data=json.dumps({})) + assert_equals(response.status_code, 200) + + def test_payload_meta(self): + """ + A click ping succeeds + """ + response = self.client.post(url_for('v3_links.click'), + content_type='application/json', + headers=[("User-Agent", "TestClient")], + data=json.dumps({"data": "test"})) + assert_equals(response.status_code, 200) + assert_equals(int(response.headers.get('Content-Length')), 0) + + +class TestViewPing(BaseTestCase): + + def test_missing_payload(self): + """ + A view ping call without a payload errors + """ + response = self.client.post(url_for('v3_links.view'), + content_type='application/json', + headers=[("User-Agent", "TestClient")]) + assert_equals(response.status_code, 400) + + def test_junk_payload(self): + """ + A view ping with valid json, but illegal payload (not a dict) errors + """ + response = self.client.post(url_for('v3_links.view'), + content_type='application/json', + headers=[("User-Agent", "TestClient")], + data='"hfdsfdsjkl"') + assert_equals(response.status_code, 400) + + def test_payload_meta(self): + """ + A view ping succeeds + """ + response = self.client.post(url_for('v3_links.view'), + content_type='application/json', + headers=[("User-Agent", "TestClient")], + data=json.dumps({"data": "test"})) + assert_equals(response.status_code, 200) + assert_equals(int(response.headers.get('Content-Length')), 0) diff --git a/tests/base.py b/tests/base.py index f4635dc..8d73459 100644 --- a/tests/base.py +++ b/tests/base.py @@ -11,6 +11,9 @@ class BaseTestCase(TestCase): self.app = environment_manager_create() env = Environment.instance() env.config.LINKS_LOCALIZATIONS = { - "STAR/en-US": "http://valid.url.com" + 'STAR/en-US': { + 'legacy': 'http://valid.url.com', + 'ag': 'http://valid.url.again.com', + } } return self.app diff --git a/tests/test_environment.py b/tests/test_environment.py new file mode 100644 index 0000000..ca65e0b --- /dev/null +++ b/tests/test_environment.py @@ -0,0 +1,59 @@ +import os +import json +import gevent +from tempfile import NamedTemporaryFile +from nose.tools import assert_equals +from mock import Mock, PropertyMock +from onyx.environment import Environment, _read_tile_index_loop +from tests.base import BaseTestCase + + +class TestReadLoop(BaseTestCase): + + def test_v3_index(self): + """ + Test reading v3 tile indexes + """ + env = Environment.instance() + test_file = NamedTemporaryFile() + + env.config = Mock() + env.config.TILE_INDEX_DIR, env.config.TILE_INDEX_FILE = os.path.split(test_file.name) + + v3_data = {'STAR/en-US': {'legacy': 'data'}, '__ver__': 3} + json.dump(v3_data, test_file) + test_file.flush() + + index_mock = PropertyMock() + type(env.config).LINKS_LOCALIZATIONS = index_mock + + gevent.spawn(_read_tile_index_loop, env) + + gevent.sleep(0) # make the event loop tick + index_mock.assert_any_call(v3_data) + + def test_index_failure(self): + """ + Test json file read failure + """ + env = Environment.instance() + test_file = NamedTemporaryFile() + + env.config = Mock() + env.config.TILE_INDEX_DIR, env.config.TILE_INDEX_FILE = os.path.split(test_file.name) + env.log_dict = Mock() + + v3_data = "{'STAR/en-US': {'legacy': 'data'}, '__ver__': 3" + test_file.write(v3_data) + test_file.flush() + + index_mock = PropertyMock() + type(env.config).LINKS_LOCALIZATIONS = index_mock + + gevent.spawn(_read_tile_index_loop, env) + + gevent.sleep(0) # make the event loop tick + assert_equals(0, index_mock.call_count) + + env.log_dict.assert_any_calls() + assert_equals('gevent_tiles_update_error', env.log_dict.call_args[1]['action'])