From 04f2ad1a0055d0f5b3cf10fcc5ab0bc854c97c00 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 24 Oct 2018 15:11:57 -0700 Subject: [PATCH 01/13] more tests and test configging --- Makefile | 2 +- dev-requirements.txt | 3 ++- feeds/auth.py | 2 +- feeds/config.py | 2 +- test/conftest.py | 19 +++++++++++++++++++ test/test.cfg | 11 +++++++++++ test/test_actor.py | 13 +++++++++++++ test/test_config.py | 28 +++++++++++++++++++++++++--- 8 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 test/conftest.py create mode 100644 test/test.cfg diff --git a/Makefile b/Makefile index bf61455..314f153 100644 --- a/Makefile +++ b/Makefile @@ -9,7 +9,7 @@ docs: test: flake8 feeds - pytest --verbose test --cov feeds + pytest --verbose test --cov --cov-report html feeds -s start: gunicorn --worker-class gevent --timeout 300 --workers 10 --bind :5000 feeds.server:app diff --git a/dev-requirements.txt b/dev-requirements.txt index 62364b7..a45c6b0 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -2,4 +2,5 @@ coverage==4.5.1 pytest-cov==2.6.0 flake8==3.5.0 pytest==3.8.2 -coveralls==1.5.1 \ No newline at end of file +coveralls==1.5.1 +requests-mock==1.5.2 \ No newline at end of file diff --git a/feeds/auth.py b/feeds/auth.py index f5bdb87..253f5ec 100644 --- a/feeds/auth.py +++ b/feeds/auth.py @@ -95,7 +95,7 @@ def validate_user_ids(user_ids): filtered_users = set(user_ids).difference(set(users)) if not filtered_users: return users - r = __auth_request('users?list={}'.format(','.join(filtered_users))) + r = __auth_request('users?list={}'.format(','.join(filtered_users)), config.auth_token) found_users = json.loads(r.content) __user_cache.update(found_users) users.update(found_users) diff --git a/feeds/config.py b/feeds/config.py index 88e6062..9ad0855 100644 --- a/feeds/config.py +++ b/feeds/config.py @@ -108,7 +108,7 @@ def _get_line(self, config, key, required=True): __config = None -def get_config(): +def get_config(from_disk=False): global __config if not __config: __config = FeedsConfig() diff --git a/test/conftest.py b/test/conftest.py new file mode 100644 index 0000000..b93db29 --- /dev/null +++ b/test/conftest.py @@ -0,0 +1,19 @@ +import os +import configparser + +def pytest_sessionstart(session): + os.environ['AUTH_TOKEN'] = 'foo' + os.environ['FEEDS_CONFIG'] = os.path.join(os.path.dirname(__file__), 'test.cfg') + +def pytest_sessionfinish(session, exitstatus): + pass + +def test_config(): + """ + Returns a ConfigParser. + Because I'm lazy. + """ + cfg = configparser.ConfigParser() + with open(os.environ['FEEDS_CONFIG'], 'r') as f: + cfg.read_file(f) + return cfg \ No newline at end of file diff --git a/test/test.cfg b/test/test.cfg new file mode 100644 index 0000000..a61f519 --- /dev/null +++ b/test/test.cfg @@ -0,0 +1,11 @@ +[feeds] +db-engine=mongodb +db-host=localhost +db-port=27017 +db-user= +db-pw= +db-name=feeds +auth-url=http://localhost/auth +admins=feeds_admin +global-feed=_kbase_ +debug=True \ No newline at end of file diff --git a/test/test_actor.py b/test/test_actor.py index e69de29..3533819 100644 --- a/test/test_actor.py +++ b/test/test_actor.py @@ -0,0 +1,13 @@ +import pytest +import requests +import json +import os +from feeds.actor import validate_actor +from .conftest import test_config + +def test_validate_actor(requests_mock): + cfg = test_config() + user_id = "foo" + user_display = "Foo Bar" + requests_mock.get('{}/api/V2/users?list={}'.format(cfg.get('feeds', 'auth-url'), user_id), text=json.dumps({user_id: user_display})) + assert user_id in validate_actor(user_id) \ No newline at end of file diff --git a/test/test_config.py b/test/test_config.py index e5b38c8..b836f2c 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -22,9 +22,12 @@ @pytest.fixture(scope="function") def dummy_auth_token(): + backup_token = os.environ.get('AUTH_TOKEN') os.environ['AUTH_TOKEN'] = FAKE_AUTH_TOKEN yield del os.environ['AUTH_TOKEN'] + if backup_token is not None: + os.environ['AUTH_TOKEN'] = backup_token @pytest.fixture(scope="function") def dummy_config(): @@ -42,6 +45,7 @@ def _write_test_cfg(cfg_lines): def test_config_from_env_ok(dummy_config, dummy_auth_token): cfg_path = dummy_config(GOOD_CONFIG) + feeds_config_backup = os.environ.get('FEEDS_CONFIG') os.environ['FEEDS_CONFIG'] = cfg_path cfg = config.FeedsConfig() assert cfg.auth_url == 'baz' @@ -49,13 +53,18 @@ def test_config_from_env_ok(dummy_config, dummy_auth_token): assert cfg.db_port == 5 del os.environ['FEEDS_CONFIG'] + kb_dep_config = os.environ.get('KB_DEPLOYMENT_CONFIG') os.environ['KB_DEPLOYMENT_CONFIG'] = cfg_path cfg = config.FeedsConfig() assert cfg.auth_url == 'baz' assert cfg.db_host == 'foo' assert cfg.db_port == 5 - del os.environ['KB_DEPLOYMENT_CONFIG'] + if kb_dep_config is not None: + os.environ['KB_DEPLOYMENT_CONFIG'] = path_backup + + if feeds_config_backup is not None: + os.environ['FEEDS_CONFIG'] = feeds_config_backup def test_config_from_env_errors(dummy_config, dummy_auth_token): @@ -65,25 +74,38 @@ def test_config_from_env_errors(dummy_config, dummy_auth_token): ] cfg_path = dummy_config(cfg_lines) + path_backup = os.environ.get('FEEDS_CONFIG') os.environ['FEEDS_CONFIG'] = cfg_path with pytest.raises(ConfigError) as e: config.FeedsConfig() assert "Error parsing config file: section feeds not found!" in str(e.value) del os.environ['FEEDS_CONFIG'] - + if path_backup is not None: + os.environ['FEEDS_CONFIG'] = path_backup def test_config_from_env_no_auth(): + backup_token = os.environ.get('AUTH_TOKEN') + if 'AUTH_TOKEN' in os.environ: + del os.environ['AUTH_TOKEN'] with pytest.raises(RuntimeError) as e: config.FeedsConfig() assert "The AUTH_TOKEN environment variable must be set!" in str(e.value) + if backup_token is not None: + os.environ['AUTH_TOKEN'] = backup_token def test_get_config(dummy_config, dummy_auth_token): cfg_path = dummy_config(GOOD_CONFIG) + + path_backup = os.environ.get('FEEDS_CONFIG') os.environ['FEEDS_CONFIG'] = cfg_path + config.__config = None cfg = config.get_config() assert cfg.db_host == 'foo' assert cfg.db_port == 5 assert cfg.auth_url == 'baz' assert cfg.auth_token == FAKE_AUTH_TOKEN - del os.environ['FEEDS_CONFIG'] \ No newline at end of file + del os.environ['FEEDS_CONFIG'] + if path_backup is not None: + os.environ['FEEDS_CONFIG'] = path_backup + config.__config = None \ No newline at end of file From f10927593cf0922de182e17ef4dc25e9ff6719f5 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 24 Oct 2018 16:12:37 -0700 Subject: [PATCH 02/13] add things to notification --- feeds/activity/notification.py | 59 +++++++++++++++++++++++++++------- feeds/server.py | 2 +- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/feeds/activity/notification.py b/feeds/activity/notification.py index 7b6a545..c69816a 100644 --- a/feeds/activity/notification.py +++ b/feeds/activity/notification.py @@ -3,14 +3,13 @@ import json from ..util import epoch_ms from .. import verbs -# from ..actor import validate_actor +from ..actor import validate_actor from .. import notification_level -SERIAL_TOKEN = "|" - class Notification(BaseActivity): - def __init__(self, actor, verb, note_object, source, level='alert', target=None, context={}): + def __init__(self, actor, verb, note_object, source, level='alert', target=None, + context={}, expires=None, external_key=None): """ A notification is roughly of this form: actor, verb, object, (target) @@ -36,6 +35,9 @@ def __init__(self, actor, verb, note_object, source, level='alert', target=None, :param target: target of the note. Optional. Should be a user id or group id if present. :param context: freeform context of the note. key-value pairs. :param validate: if True, runs _validate immediately + :param expires: if not None, set a new expiration date - should be an int, ms since epoch + :param external_key: an optional special key given by the service that created the + notification TODO: * decide on global ids for admin use @@ -53,19 +55,42 @@ def __init__(self, actor, verb, note_object, source, level='alert', target=None, self.target = target self.context = context self.level = notification_level.translate_level(level) - self.time = epoch_ms() # int timestamp down to millisecond + self.created = epoch_ms() # int timestamp down to millisecond + if expires is not None: + self.expires = self._default_expiration() + else: + self.validate_expiration(expires) + self.expires = expires + self.external_key = external_key def validate(self): """ Validates whether the notification fields are accurate. Should be called before sending a new notification to storage. """ + self.validate_expiration(self.expires, self.created) + validate_actor(self.actor) + + def validate_expiration(self, expires, created): + """ + Validates whether the expiration time is valid and after the created time. + If yes, returns True. If not, raises an InvalidExpirationTimeError. + """ + pass + + def _default_expiration(self): + """ + Returns the default expiration time of this notification. + """ pass - def to_json(self): - # returns a jsonifyable structure - # leave out target. don't need to see who else saw this. - return { + def to_dict(self): + """ + Returns a dict form of the Notification. + Useful for storing in a document store. + Less useful, but not terrible, for returning to a user. + """ + dict_form = { "id": self.id, "actor": self.actor, "verb": self.verb.infinitive, @@ -73,8 +98,18 @@ def to_json(self): "source": self.source, "context": self.context, "level": self.level.name, - "time": self.time + "created": self.created, + "expires": self.expires, + "external_key": self.external_key } + return dict_form + + def user_view(self): + """ + Returns a view of the Notification that's intended for the user. + That means we leave out the target and external keys. + """ + pass def serialize(self): """ @@ -90,7 +125,7 @@ def serialize(self): "s": self.source, "t": self.target, "l": self.level.id, - "m": self.time + "c": self.created } return json.dumps(serial, separators=(',', ':')) @@ -111,7 +146,7 @@ def deserialize(cls, serial): target=struct.get('t'), context=struct.get('c') ) - deserial.time = struct['m'] + deserial.time = struct['c'] deserial.id = struct['i'] return deserial diff --git a/feeds/server.py b/feeds/server.py index 04f20d2..6e26477 100644 --- a/feeds/server.py +++ b/feeds/server.py @@ -145,7 +145,7 @@ def get_notifications(): notes = feed.get_notifications(count=max_notes) return_list = list() for note in notes: - return_list.append(note.to_json()) + return_list.append(note.to_dict()) return (flask.jsonify(return_list), 200) @app.route('/api/V1/notification/', methods=['GET']) From 55634dd66480ef0664e9331ad8301ee68435af8e Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 24 Oct 2018 17:31:21 -0700 Subject: [PATCH 03/13] add optional external key and expiration time --- feeds/activity/notification.py | 26 +++++++++++++++++++++----- feeds/exceptions.py | 7 +++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/feeds/activity/notification.py b/feeds/activity/notification.py index c69816a..2daaa8e 100644 --- a/feeds/activity/notification.py +++ b/feeds/activity/notification.py @@ -5,6 +5,8 @@ from .. import verbs from ..actor import validate_actor from .. import notification_level +from feeds.exceptions import InvalidExpirationError +import datetime class Notification(BaseActivity): @@ -74,8 +76,16 @@ def validate(self): def validate_expiration(self, expires, created): """ Validates whether the expiration time is valid and after the created time. - If yes, returns True. If not, raises an InvalidExpirationTimeError. - """ + If yes, returns True. If not, raises an InvalidExpirationError. + """ + # Just validate that the time looks like a real time in epoch millis. + try: + datetime.datetime.fromtimestamp(expires/1000) + except (TypeError, ValueError): + raise InvalidExpirationError( + "Expiration time should be the number " + "of milliseconds since the epoch." + ) pass def _default_expiration(self): @@ -125,7 +135,9 @@ def serialize(self): "s": self.source, "t": self.target, "l": self.level.id, - "c": self.created + "c": self.created, + "e": self.expires, + "x": self.external_key } return json.dumps(serial, separators=(',', ':')) @@ -144,10 +156,12 @@ def deserialize(cls, serial): struct['s'], level=str(struct['l']), target=struct.get('t'), - context=struct.get('c') + context=struct.get('c'), + external_key=struct.get('x') ) deserial.time = struct['c'] deserial.id = struct['i'] + deserial.time = struct['e'] return deserial @classmethod @@ -163,8 +177,10 @@ def from_dict(cls, serial): serial['source'], level=str(serial['level']), target=serial.get('target'), - context=serial.get('context') + context=serial.get('context'), + external_key=serial.get('external_key') ) deserial.time = serial['created'] + deserial.expires = serial['expires'] deserial.id = serial['act_id'] return deserial diff --git a/feeds/exceptions.py b/feeds/exceptions.py index 284dfea..fdf8b96 100644 --- a/feeds/exceptions.py +++ b/feeds/exceptions.py @@ -83,3 +83,10 @@ class ActivityRetrievalError(Exception): Raised if the service fails to retrieve an activity from a database. """ pass + + +class InvalidExpirationError(Exception): + """ + Raised when trying to give a Notification an invalid expiration time. + """ + pass From 48cadaac6ecfd0c71f3907f137f3eeaaf7a76169 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 24 Oct 2018 18:03:13 -0700 Subject: [PATCH 04/13] add default lifespan --- deploy.cfg.example | 10 ++++++++++ feeds/activity/notification.py | 25 +++++++++++++++++++------ feeds/config.py | 6 ++++++ feeds/server.py | 2 +- test/test.cfg | 3 ++- test/test_config.py | 3 ++- 6 files changed, 40 insertions(+), 9 deletions(-) diff --git a/deploy.cfg.example b/deploy.cfg.example index a7b7b8c..4d933c4 100644 --- a/deploy.cfg.example +++ b/deploy.cfg.example @@ -21,3 +21,13 @@ admins=wjriehl,scanon,kkeller,mmdrake # fake user name for the global feed. Should be something that's not a valid # user name. global-feed=_global_ + +# Default lifetime for each notification in days. Notes older than this won't be +# returned without explicitly looking them up by either their id or external key +# (when given). +lifespan=30 + +# In debug mode, auth is effectively ignored. +# Useful for testing, etc. +# SET TO FALSE IN PRODUCTION! +debug=False \ No newline at end of file diff --git a/feeds/activity/notification.py b/feeds/activity/notification.py index 2daaa8e..4df1d23 100644 --- a/feeds/activity/notification.py +++ b/feeds/activity/notification.py @@ -7,6 +7,7 @@ from .. import notification_level from feeds.exceptions import InvalidExpirationError import datetime +from feeds.config import get_config class Notification(BaseActivity): @@ -59,9 +60,9 @@ def __init__(self, actor, verb, note_object, source, level='alert', target=None, self.level = notification_level.translate_level(level) self.created = epoch_ms() # int timestamp down to millisecond if expires is not None: - self.expires = self._default_expiration() + self.expires = self._default_lifespan() + self.created else: - self.validate_expiration(expires) + self.validate_expiration(expires, self.created) self.expires = expires self.external_key = external_key @@ -88,11 +89,11 @@ def validate_expiration(self, expires, created): ) pass - def _default_expiration(self): + def _default_lifespan(self): """ - Returns the default expiration time of this notification. + Returns the default lifespan of this notification in ms. """ - pass + return get_config().lifespan * 24 * 60 * 60 * 1000 def to_dict(self): """ @@ -107,6 +108,7 @@ def to_dict(self): "object": self.object, "source": self.source, "context": self.context, + "target": self.target, "level": self.level.name, "created": self.created, "expires": self.expires, @@ -119,7 +121,18 @@ def user_view(self): Returns a view of the Notification that's intended for the user. That means we leave out the target and external keys. """ - pass + view = { + "id": self.id, + "actor": self.actor, + "verb": self.verb.past_tense, + "object": self.object, + "source": self.source, + "context": self.context, + "level": self.level.name, + "created": self.created, + "expires": self.expires + } + return view def serialize(self): """ diff --git a/feeds/config.py b/feeds/config.py index 9ad0855..a56f2ed 100644 --- a/feeds/config.py +++ b/feeds/config.py @@ -19,6 +19,7 @@ KEY_ADMIN_LIST = "admins" KEY_GLOBAL_FEED = "global-feed" KEY_DEBUG = "debug" +KEY_LIFESPAN = "lifespan" class FeedsConfig(object): @@ -52,6 +53,11 @@ def __init__(self): self.global_feed = self._get_line(cfg, KEY_GLOBAL_FEED) self.auth_url = self._get_line(cfg, KEY_AUTH_URL) self.admins = self._get_line(cfg, KEY_ADMIN_LIST).split(",") + self.lifespan = self._get_line(cfg, KEY_LIFESPAN) + try: + self.lifespan = int(self._get_line(cfg, KEY_LIFESPAN)) + except ValueError: + raise ConfigError("{} must be an int! Got {}".format(KEY_LIFESPAN, self.lifespan)) self.debug = self._get_line(cfg, KEY_DEBUG, required=False) if not self.debug or self.debug.lower() != "true": self.debug = False diff --git a/feeds/server.py b/feeds/server.py index 6e26477..393d0c4 100644 --- a/feeds/server.py +++ b/feeds/server.py @@ -145,7 +145,7 @@ def get_notifications(): notes = feed.get_notifications(count=max_notes) return_list = list() for note in notes: - return_list.append(note.to_dict()) + return_list.append(note.user_view()) return (flask.jsonify(return_list), 200) @app.route('/api/V1/notification/', methods=['GET']) diff --git a/test/test.cfg b/test/test.cfg index a61f519..c6ad7e3 100644 --- a/test/test.cfg +++ b/test/test.cfg @@ -8,4 +8,5 @@ db-name=feeds auth-url=http://localhost/auth admins=feeds_admin global-feed=_kbase_ -debug=True \ No newline at end of file +debug=True +lifespan=30 \ No newline at end of file diff --git a/test/test_config.py b/test/test_config.py index b836f2c..c11e089 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -17,7 +17,8 @@ 'db-port=5', 'auth-url=baz', 'global-feed=global', - 'admins=admin1,admin2,admin3' + 'admins=admin1,admin2,admin3', + 'lifespan=30' ] @pytest.fixture(scope="function") From 8ccce0fd3d413dddb7665402901f56c114541a88 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 24 Oct 2018 18:06:35 -0700 Subject: [PATCH 05/13] add expires and creation to stored notifications. --- feeds/activity/notification.py | 6 +++--- feeds/storage/mongodb/activity_storage.py | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/feeds/activity/notification.py b/feeds/activity/notification.py index 4df1d23..1142d74 100644 --- a/feeds/activity/notification.py +++ b/feeds/activity/notification.py @@ -98,18 +98,18 @@ def _default_lifespan(self): def to_dict(self): """ Returns a dict form of the Notification. - Useful for storing in a document store. + Useful for storing in a document store, returns the id of each verb and level. Less useful, but not terrible, for returning to a user. """ dict_form = { "id": self.id, "actor": self.actor, - "verb": self.verb.infinitive, + "verb": self.verb.id, "object": self.object, "source": self.source, "context": self.context, "target": self.target, - "level": self.level.name, + "level": self.level.id, "created": self.created, "expires": self.expires, "external_key": self.external_key diff --git a/feeds/storage/mongodb/activity_storage.py b/feeds/storage/mongodb/activity_storage.py index 38c9a17..7b07c2f 100644 --- a/feeds/storage/mongodb/activity_storage.py +++ b/feeds/storage/mongodb/activity_storage.py @@ -4,7 +4,7 @@ from feeds.exceptions import ( ActivityStorageError ) -from pymongo import PyMongoError +from pymongo.errors import PyMongoError class MongoActivityStorage(ActivityStorage): @@ -23,6 +23,8 @@ def add_to_storage(self, activity, target_users: List[str]): "target": activity.target, "source": activity.source, "level": activity.level.id, + "created": activity.created, + "expires": activity.expires, "users": target_users, "unseen": target_users, "created": activity.time, From 9f7099d88ebf6e23275fa356af684794beeeeac9 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Wed, 24 Oct 2018 18:12:40 -0700 Subject: [PATCH 06/13] officially make BaseActivity a base class --- feeds/activity/base.py | 7 ++++++- feeds/storage/mongodb/activity_storage.py | 18 +++--------------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/feeds/activity/base.py b/feeds/activity/base.py index d32f229..c2888bd 100644 --- a/feeds/activity/base.py +++ b/feeds/activity/base.py @@ -1,6 +1,11 @@ +from abc import abstractmethod + + class BaseActivity(object): """ Common parent class for Activity and Notification. Activity will be done later. But a Notification is an Activity. """ - pass + @abstractmethod + def to_dict(self): + pass diff --git a/feeds/storage/mongodb/activity_storage.py b/feeds/storage/mongodb/activity_storage.py index 7b07c2f..ca1afa6 100644 --- a/feeds/storage/mongodb/activity_storage.py +++ b/feeds/storage/mongodb/activity_storage.py @@ -15,21 +15,9 @@ def add_to_storage(self, activity, target_users: List[str]): Raises an ActivityStorageError if it fails. """ coll = get_feeds_collection() - act_doc = { - "act_id": activity.id, - "actor": activity.actor, - "verb": activity.verb.id, - "object": activity.object, - "target": activity.target, - "source": activity.source, - "level": activity.level.id, - "created": activity.created, - "expires": activity.expires, - "users": target_users, - "unseen": target_users, - "created": activity.time, - "context": activity.context - } + act_doc = activity.to_dict() + act_doc["users"] = target_users + act_doc["unseen"] = target_users try: coll.insert_one(act_doc) except PyMongoError as e: From 7713732706f3faa02367bf0730e5b566d20cf77d Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Thu, 25 Oct 2018 13:57:15 -0700 Subject: [PATCH 07/13] base tests for notification --- feeds/activity/notification.py | 9 +- feeds/auth.py | 2 +- feeds/server.py | 9 +- feeds/storage/mongodb/connection.py | 32 +++++- test/activity/__init__.py | 0 test/activity/test_notification.py | 161 ++++++++++++++++++++++++++-- test/feeds/__init__.py | 0 test/managers/__init__.py | 0 test/storage/__init__.py | 0 9 files changed, 194 insertions(+), 19 deletions(-) create mode 100644 test/activity/__init__.py create mode 100644 test/feeds/__init__.py create mode 100644 test/managers/__init__.py create mode 100644 test/storage/__init__.py diff --git a/feeds/activity/notification.py b/feeds/activity/notification.py index 1142d74..824bda2 100644 --- a/feeds/activity/notification.py +++ b/feeds/activity/notification.py @@ -59,11 +59,10 @@ def __init__(self, actor, verb, note_object, source, level='alert', target=None, self.context = context self.level = notification_level.translate_level(level) self.created = epoch_ms() # int timestamp down to millisecond - if expires is not None: - self.expires = self._default_lifespan() + self.created - else: - self.validate_expiration(expires, self.created) - self.expires = expires + if expires is None: + expires = self._default_lifespan() + self.created + self.validate_expiration(expires, self.created) + self.expires = expires self.external_key = external_key def validate(self): diff --git a/feeds/auth.py b/feeds/auth.py index 253f5ec..01063c4 100644 --- a/feeds/auth.py +++ b/feeds/auth.py @@ -52,7 +52,6 @@ def validate_service_token(token): TODO: I know this is going to be rife with issues. The name of the token doesn't have to be the service. But as long as it's a Service token, then it came from in KBase, so everything should be ok. - TODO: Add 'source' to PUT notification endpoint. """ token = __fetch_token(token) if token.get('type') == 'Service': @@ -67,6 +66,7 @@ def validate_user_token(token): """ Validates a user auth token. If valid, returns the user id. If invalid, raises an InvalidTokenError. + If debug is True, always validates and returns a nonsense user name """ return __fetch_token(token)['user'] diff --git a/feeds/server.py b/feeds/server.py index 393d0c4..0f78eae 100644 --- a/feeds/server.py +++ b/feeds/server.py @@ -4,6 +4,7 @@ Flask, request ) +import traceback import logging from http.client import responses from flask.logging import default_handler @@ -38,7 +39,13 @@ def _log(msg, *args, level=logging.INFO): def _log_error(error): - _log("Exception: " + str(error) + error, level=logging.ERROR) + formatted_error = ''.join( + traceback.format_exception( + etype=type(error), + value=error, + tb=error.__traceback__) + ) + _log("Exception: " + formatted_error, level=logging.ERROR) def _get_auth_token(request, required=True): diff --git a/feeds/storage/mongodb/connection.py b/feeds/storage/mongodb/connection.py index b03bdde..4e9c549 100644 --- a/feeds/storage/mongodb/connection.py +++ b/feeds/storage/mongodb/connection.py @@ -1,6 +1,7 @@ -import pymongo from pymongo import ( - MongoClient + MongoClient, + ASCENDING, + DESCENDING ) from feeds.config import get_config import feeds.logger as log @@ -8,8 +9,33 @@ _connection = None _COL_NOTIFICATIONS = "notifications" + +# Searches to support: +# 1. Lookup by activity id. Easy. +# 2. Lookup all by user, include docs where user is not in unseen, sort by time. +# 3. Lookup all by user, ignore docs where user is not in unseen, sort by time. +# 4. Lookup all by user, sort by source, then sort by time. +# 5. Lookup all by user, sort by type, then sort by time. +# 6. Lookup all by user, sort by source, then sort by time, then sort by time. +# 7. Aggregations... later. Maybe part of the Timeline class. + _INDEXES = [ - [("created", pymongo.DESCENDING)] + [("act_id", ASCENDING)], + + # sort by creation date + [("created", DESCENDING)], + + # sort by target users + [("users", ASCENDING)], + + # sort by unseen users + [("unseen", ASCENDING)], + + # sort by source, then creation date + [("users", ASCENDING), ("source", ASCENDING), ("created", DESCENDING)], + + # sort by level, then creation date + [("users", ASCENDING), ("level", ASCENDING), ("created", DESCENDING)] ] diff --git a/test/activity/__init__.py b/test/activity/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/test/activity/test_notification.py b/test/activity/test_notification.py index 415cff3..ece314a 100644 --- a/test/activity/test_notification.py +++ b/test/activity/test_notification.py @@ -1,13 +1,156 @@ import pytest from feeds.activity.notification import Notification import uuid +from feeds.util import epoch_ms +from ..conftest import test_config -def test_basic_notification(): - assert True - # n = Notification('foo', 'bar', 'baz') - # assert n.actor == 'foo' - # assert n.note_type == 'bar' - # assert n.object == 'baz' - # assert n.content == {} - # assert n.target == None - # assert validate_uuid(n.id) \ No newline at end of file + # def __init__(self, actor, verb, note_object, source, level='alert', target=None, + # context={}, expires=None, external_key=None): + +cfg = test_config() + +# some dummy "good" inputs for testing +actor = "test_actor" +verb_inf = "invite" +verb_id = 1 +note_object = "foo" +source = "groups" +level_name = "warning" +level_id = 2 +target = ["target_actor"] +context = {"some": "context"} +expires = epoch_ms() + (10 * 24 * 60 * 60 * 1000) # 10 days +external_key = "an_external_key" + +def assert_note_ok(note, **kwargs): + keys = [ + 'actor', 'object', 'source', 'target', 'context', 'expires', 'external_key' + ] + for k in keys: + if k in kwargs: + assert getattr(note, k) == kwargs[k] + if 'verb_id' in kwargs: + assert note.verb.id == int(kwargs['verb_id']) + if 'verb_inf' in kwargs: + assert note.verb.infinitive == kwargs['verb_inf'] + if 'level_id' in kwargs: + assert note.level.id == int(kwargs['level_id']) + if 'level_name' in kwargs: + assert note.level.name == kwargs['level_name'] + if 'expires' not in kwargs: + assert note.expires == note.created + (int(cfg.get('feeds', 'lifespan')) * 24 * 60 * 60 * 1000) + +def test_note_new_ok_no_kwargs(): + note = Notification(actor, verb_inf, note_object, source) + assert_note_ok(note, actor=actor, verb_inf=verb_inf, object=note_object, source=source) + +def test_note_new_diff_levels(): + assert_args = { + "actor": actor, + "verb_inf": verb_inf, + "object": note_object, + "source": source + } + for name in ['alert', 'warning', 'request', 'error']: + note = Notification(actor, verb_inf, note_object, source, level=name) + test_args = assert_args.copy() + test_args['level_name'] = name + assert_note_ok(note, **test_args) + for id_ in ['1', '2', '3', '4']: + note = Notification(actor, verb_inf, note_object, source, level=id_) + test_args = assert_args.copy() + test_args['level_id'] = id_ + assert_note_ok(note, **test_args) + + +def test_note_new_target(): + note = Notification(actor, verb_inf, note_object, source, target=target) + assert_note_ok(note, actor=actor, verb_inf=verb_inf, + object=note_object, source=source, target=target) + + +def test_note_new_context(): + note = Notification(actor, verb_inf, note_object, source, context=context) + assert_note_ok(note, actor=actor, verb_inf=verb_inf, + object=note_object, source=source, context=context) + + +def test_note_new_expiration(): + note = Notification(actor, verb_inf, note_object, source, expires=expires) + assert_note_ok(note, actor=actor, verb_inf=verb_inf, + object=note_object, source=source, expires=expires) + + +def test_note_new_external_key(): + note = Notification(actor, verb_inf, note_object, source, external_key=external_key) + assert_note_ok(note, actor=actor, verb_inf=verb_inf, + object=note_object, source=source, external_key=external_key) + + + +def test_note_new_bad_actor(): + pass + + +def test_note_new_bad_verb(): + pass + + +def test_note_new_bad_object(): + pass + + +def test_note_new_bad_source(): + pass + + +def test_note_new_bad_level(): + pass + + +def test_note_new_bad_target(): + pass + + +def test_note_new_bad_context(): + pass + + +def test_note_new_bad_expires(): + pass + + +def test_validate_ok(): + pass + + +def test_validate_bad(): + pass + + +def test_validate_expiration(): + pass + + +def test_default_lifespan(): + pass + + +def test_to_dict(): + pass + + +def test_user_view(): + pass + + +def test_from_dict(): + pass + + +def test_serialize(): + pass + + +def test_deserialize(): + pass diff --git a/test/feeds/__init__.py b/test/feeds/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/test/managers/__init__.py b/test/managers/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/test/storage/__init__.py b/test/storage/__init__.py new file mode 100644 index 0000000..e69de29 From 11cd7dd4ed5853262ced1ccee45866f4127c0001 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Thu, 25 Oct 2018 14:52:18 -0700 Subject: [PATCH 08/13] more tests --- feeds/activity/notification.py | 16 ++++++-- test/activity/test_notification.py | 64 +++++++++++++++++++++++++----- test/util.py | 5 +++ 3 files changed, 72 insertions(+), 13 deletions(-) diff --git a/feeds/activity/notification.py b/feeds/activity/notification.py index 824bda2..9618545 100644 --- a/feeds/activity/notification.py +++ b/feeds/activity/notification.py @@ -12,7 +12,7 @@ class Notification(BaseActivity): def __init__(self, actor, verb, note_object, source, level='alert', target=None, - context={}, expires=None, external_key=None): + context=None, expires=None, external_key=None): """ A notification is roughly of this form: actor, verb, object, (target) @@ -50,6 +50,13 @@ def __init__(self, actor, verb, note_object, source, level='alert', target=None, * validate target is valid * validate context fits """ + assert actor is not None, "actor must not be None" + assert verb is not None, "verb must not be None" + assert note_object is not None, "note_object must not be None" + assert source is not None, "source must not be None" + assert level is not None, "level must not be None" + assert target is None or isinstance(target, list), "target must be either a list or None" + assert context is None or isinstance(context, dict), "context must be either a dict or None" self.id = str(uuid.uuid4()) self.actor = actor self.verb = verbs.translate_verb(verb) @@ -84,9 +91,12 @@ def validate_expiration(self, expires, created): except (TypeError, ValueError): raise InvalidExpirationError( "Expiration time should be the number " - "of milliseconds since the epoch." + "of milliseconds since the epoch" + ) + if expires <= created: + raise InvalidExpirationError( + "Notifications should expire sometime after they are created" ) - pass def _default_lifespan(self): """ diff --git a/test/activity/test_notification.py b/test/activity/test_notification.py index ece314a..2d10685 100644 --- a/test/activity/test_notification.py +++ b/test/activity/test_notification.py @@ -3,6 +3,12 @@ import uuid from feeds.util import epoch_ms from ..conftest import test_config +from ..util import assert_is_uuid +from feeds.exceptions import ( + MissingVerbError, + MissingLevelError, + InvalidExpirationError +) # def __init__(self, actor, verb, note_object, source, level='alert', target=None, # context={}, expires=None, external_key=None): @@ -39,6 +45,7 @@ def assert_note_ok(note, **kwargs): assert note.level.name == kwargs['level_name'] if 'expires' not in kwargs: assert note.expires == note.created + (int(cfg.get('feeds', 'lifespan')) * 24 * 60 * 60 * 1000) + assert_is_uuid(note.id) def test_note_new_ok_no_kwargs(): note = Notification(actor, verb_inf, note_object, source) @@ -87,37 +94,74 @@ def test_note_new_external_key(): object=note_object, source=source, external_key=external_key) - def test_note_new_bad_actor(): - pass + # TODO: Should only fail on validate - shouldn't do a lookup whenever a new note is made. + # also, shouldn't be None. + with pytest.raises(AssertionError) as e: + Notification(None, verb_inf, note_object, source) + assert "actor must not be None" in str(e.value) def test_note_new_bad_verb(): - pass + with pytest.raises(AssertionError) as e: + Notification(actor, None, note_object, source) + assert "verb must not be None" in str(e.value) + with pytest.raises(MissingVerbError) as e: + Notification(actor, "foobar", note_object, source) + assert 'Verb "foobar" not found' in str(e.value) -def test_note_new_bad_object(): - pass +def test_note_new_bad_object(): + # TODO: Also test object validation itself later. + with pytest.raises(AssertionError) as e: + Notification(actor, verb_inf, None, source) + assert 'note_object must not be None' in str(e.value) def test_note_new_bad_source(): - pass + # TODO: Validate sources as being real. + with pytest.raises(AssertionError) as e: + Notification(actor, verb_inf, note_object, None) + assert 'source must not be None' in str(e.value) def test_note_new_bad_level(): - pass + with pytest.raises(AssertionError) as e: + Notification(actor, verb_inf, note_object, source, level=None) + assert "level must not be None" in str(e.value) + + with pytest.raises(MissingLevelError) as e: + Notification(actor, verb_inf, note_object, source, level="foobar") + assert 'Level "foobar" not found' in str(e.value) def test_note_new_bad_target(): - pass + bad_targets = [{}, "foo", 123, False] + for bad in bad_targets: + with pytest.raises(AssertionError) as e: + Notification(actor, verb_inf, note_object, source, target=bad) + assert "target must be either a list or None" in str(e.value) def test_note_new_bad_context(): - pass + bad_context = [[], "foo", 123, False] + for bad in bad_context: + with pytest.raises(AssertionError) as e: + Notification(actor, verb_inf, note_object, source, context=bad) + assert "context must be either a dict or None" in str(e.value) def test_note_new_bad_expires(): - pass + bad_expires = ["foo", {}, []] + for bad in bad_expires: + with pytest.raises(InvalidExpirationError) as e: + Notification(actor, verb_inf, note_object, source, expires=bad) + assert "Expiration time should be the number of milliseconds" in str(e.value) + bad_expires = [123, True, False] + for bad in bad_expires: + with pytest.raises(InvalidExpirationError) as e: + Notification(actor, verb_inf, note_object, source, expires=bad) + assert "Notifications should expire sometime after they are created" in str(e.value) def test_validate_ok(): diff --git a/test/util.py b/test/util.py index e69de29..a517730 100644 --- a/test/util.py +++ b/test/util.py @@ -0,0 +1,5 @@ +import uuid + +def assert_is_uuid(s): + # raises a ValueError if not. Good enough for testing. + uuid.UUID(s) \ No newline at end of file From 5fb89f276e086ebb70c9e7e7e93ef6e917167f40 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Thu, 25 Oct 2018 15:18:20 -0700 Subject: [PATCH 09/13] more tests, API tweaks --- feeds/actor.py | 6 +++++- feeds/auth.py | 6 +++++- test/activity/test_notification.py | 26 +++++++++++++++----------- test/test_actor.py | 13 +++++++++++-- 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/feeds/actor.py b/feeds/actor.py index 7a4abcf..b3e9d12 100644 --- a/feeds/actor.py +++ b/feeds/actor.py @@ -3,10 +3,14 @@ TODO: decide whether to use a class, or just a validated string. I'm leaning toward string. """ from .auth import validate_user_id +from .exceptions import InvalidActorError def validate_actor(actor): """ TODO: groups can be actors, too, when that's ready. """ - return validate_user_id(actor) + if validate_user_id(actor): + return True + else: + raise InvalidActorError("Actor '{}' is not a real user.".format(actor)) diff --git a/feeds/auth.py b/feeds/auth.py index 01063c4..8b8176f 100644 --- a/feeds/auth.py +++ b/feeds/auth.py @@ -72,7 +72,11 @@ def validate_user_token(token): def validate_user_id(user_id): - return validate_user_ids([user_id]) + """ + Validates whether a SINGLE user is real or not. + Returns a boolean. + """ + return user_id in validate_user_ids([user_id]) def validate_user_ids(user_ids): diff --git a/test/activity/test_notification.py b/test/activity/test_notification.py index 2d10685..e2bf2f1 100644 --- a/test/activity/test_notification.py +++ b/test/activity/test_notification.py @@ -1,4 +1,5 @@ import pytest +import json from feeds.activity.notification import Notification import uuid from feeds.util import epoch_ms @@ -10,9 +11,6 @@ InvalidExpirationError ) - # def __init__(self, actor, verb, note_object, source, level='alert', target=None, - # context={}, expires=None, external_key=None): - cfg = test_config() # some dummy "good" inputs for testing @@ -45,6 +43,7 @@ def assert_note_ok(note, **kwargs): assert note.level.name == kwargs['level_name'] if 'expires' not in kwargs: assert note.expires == note.created + (int(cfg.get('feeds', 'lifespan')) * 24 * 60 * 60 * 1000) + assert note.created < note.expires assert_is_uuid(note.id) def test_note_new_ok_no_kwargs(): @@ -164,16 +163,21 @@ def test_note_new_bad_expires(): assert "Notifications should expire sometime after they are created" in str(e.value) -def test_validate_ok(): - pass - - -def test_validate_bad(): - pass +def test_validate_ok(requests_mock): + user_id = "foo" + user_display = "Foo Bar" + requests_mock.get('{}/api/V2/users?list={}'.format(cfg.get('feeds', 'auth-url'), user_id), text=json.dumps({user_id: user_display})) + note = Notification(user_id, verb_inf, note_object, source) + # If this doesn't throw any errors, then it passes! + note.validate() -def test_validate_expiration(): - pass +def test_validate_bad(requests_mock): + user_id = "foo" + requests_mock.get('{}/api/V2/users?list={}'.format(cfg.get('feeds', 'auth-url'), user_id), text=json.dumps({})) + note = Notification(user_id, verb_inf, note_object, source) + # If this doesn't throw any errors, then it passes! + note.validate() def test_default_lifespan(): diff --git a/test/test_actor.py b/test/test_actor.py index 3533819..8c11b39 100644 --- a/test/test_actor.py +++ b/test/test_actor.py @@ -4,10 +4,19 @@ import os from feeds.actor import validate_actor from .conftest import test_config +from feeds.exceptions import InvalidActorError +cfg = test_config() def test_validate_actor(requests_mock): - cfg = test_config() user_id = "foo" user_display = "Foo Bar" requests_mock.get('{}/api/V2/users?list={}'.format(cfg.get('feeds', 'auth-url'), user_id), text=json.dumps({user_id: user_display})) - assert user_id in validate_actor(user_id) \ No newline at end of file + assert validate_actor(user_id) + + +def test_validate_actor_fail(requests_mock): + user_id = "foo2" + requests_mock.get('{}/api/V2/users?list={}'.format(cfg.get('feeds', 'auth-url'), user_id), text=json.dumps({})) + with pytest.raises(InvalidActorError) as e: + validate_actor(user_id) + assert "Actor '{}' is not a real user".format(user_id) From 0456618144a8cba0dc7e82b635b0fb7a3dbdeb86 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Thu, 25 Oct 2018 16:47:21 -0700 Subject: [PATCH 10/13] more tests --- feeds/notification_level.py | 4 +++- feeds/verbs.py | 4 +++- test/activity/test_notification.py | 31 +++++++++++++++++++++++++++--- test/test_actor.py | 2 +- 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/feeds/notification_level.py b/feeds/notification_level.py index efd424d..63da3e0 100644 --- a/feeds/notification_level.py +++ b/feeds/notification_level.py @@ -40,7 +40,9 @@ def translate_level(level): :param level: Either a string or a Level. (stringify numerical ids before looking them up) """ - if isinstance(level, str): + if isinstance(level, int): + return get_level(str(level)) + elif isinstance(level, str): return get_level(level) elif isinstance(level, Level): return get_level(level.name) diff --git a/feeds/verbs.py b/feeds/verbs.py index cc8c0a3..731b175 100644 --- a/feeds/verbs.py +++ b/feeds/verbs.py @@ -39,7 +39,9 @@ def translate_verb(verb): - if it's a verb that's registered, return it - if it's not a Verb or a str, raise a TypeError """ - if isinstance(verb, str): + if isinstance(verb, int): + return get_verb(str(verb)) + elif isinstance(verb, str): return get_verb(verb) elif isinstance(verb, Verb): return get_verb(verb.infinitive) diff --git a/test/activity/test_notification.py b/test/activity/test_notification.py index e2bf2f1..84bdfbe 100644 --- a/test/activity/test_notification.py +++ b/test/activity/test_notification.py @@ -16,6 +16,7 @@ # some dummy "good" inputs for testing actor = "test_actor" verb_inf = "invite" +verb_past = "invited" verb_id = 1 note_object = "foo" source = "groups" @@ -181,15 +182,39 @@ def test_validate_bad(requests_mock): def test_default_lifespan(): - pass + note = Notification(actor, verb_inf, note_object, source) + lifespan = int(cfg.get('feeds', 'lifespan')) + assert note.expires - note.created == lifespan * 24 * 60 * 60 * 1000 def test_to_dict(): - pass + note = Notification(actor, verb_inf, note_object, source, level=level_name) + d = note.to_dict() + assert d["actor"] == actor + assert d["verb"] == verb_id + assert d["object"] == note_object + assert d["source"] == source + assert isinstance(d["expires"], int) and d["expires"] == note.expires + assert isinstance(d["created"], int) and d["created"] == note.created + assert d["target"] is None + assert d["context"] is None + assert d["level"] == level_id + assert d["external_key"] is None def test_user_view(): - pass + note = Notification(actor, verb_inf, note_object, source, level=level_id) + v = note.user_view() + assert v["actor"] == actor + assert v["verb"] == verb_past + assert v["object"] == note_object + assert v["source"] == source + assert isinstance(v["expires"], int) and v["expires"] == note.expires + assert isinstance(v["created"], int) and v["created"] == note.created + assert "target" not in v + assert v["context"] is None + assert v["level"] == level_name + assert "external_key" not in v def test_from_dict(): diff --git a/test/test_actor.py b/test/test_actor.py index 8c11b39..48faae5 100644 --- a/test/test_actor.py +++ b/test/test_actor.py @@ -19,4 +19,4 @@ def test_validate_actor_fail(requests_mock): requests_mock.get('{}/api/V2/users?list={}'.format(cfg.get('feeds', 'auth-url'), user_id), text=json.dumps({})) with pytest.raises(InvalidActorError) as e: validate_actor(user_id) - assert "Actor '{}' is not a real user".format(user_id) + assert "Actor '{}' is not a real user".format(user_id) in str(e.value) From 290c6b7acb85b7f065eba2e3e2517ab27a1a31e4 Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Thu, 25 Oct 2018 16:56:35 -0700 Subject: [PATCH 11/13] more tests, fix from_dict --- feeds/activity/notification.py | 8 ++++---- test/activity/test_notification.py | 22 ++++++++++++++++++++-- test/test_notification_level.py | 4 ++++ test/test_verbs.py | 4 ++++ 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/feeds/activity/notification.py b/feeds/activity/notification.py index 9618545..9b76e86 100644 --- a/feeds/activity/notification.py +++ b/feeds/activity/notification.py @@ -181,9 +181,9 @@ def deserialize(cls, serial): context=struct.get('c'), external_key=struct.get('x') ) - deserial.time = struct['c'] + deserial.created = struct['c'] deserial.id = struct['i'] - deserial.time = struct['e'] + deserial.expires = struct['e'] return deserial @classmethod @@ -202,7 +202,7 @@ def from_dict(cls, serial): context=serial.get('context'), external_key=serial.get('external_key') ) - deserial.time = serial['created'] + deserial.created = serial['created'] deserial.expires = serial['expires'] - deserial.id = serial['act_id'] + deserial.id = serial['id'] return deserial diff --git a/test/activity/test_notification.py b/test/activity/test_notification.py index 84bdfbe..bc91298 100644 --- a/test/activity/test_notification.py +++ b/test/activity/test_notification.py @@ -218,8 +218,26 @@ def test_user_view(): def test_from_dict(): - pass - + act_id = str(uuid.uuid4()) + verb = [verb_id, str(verb_id), verb_inf, verb_past] + level = [level_id, level_name, str(level_id)] + d = { + "actor": actor, + "object": note_object, + "source": source, + "expires": 1234567890111, + "created": 1234567890000, + "target": target, + "context": context, + "external_key": external_key, + "id": act_id + } + for v in verb: + for l in level: + note_d = d.copy() + note_d.update({'level': l, 'verb': v}) + note = Notification.from_dict(note_d) + assert_note_ok(note, **note_d) def test_serialize(): pass diff --git a/test/test_notification_level.py b/test/test_notification_level.py index 00a5165..7eabdb9 100644 --- a/test/test_notification_level.py +++ b/test/test_notification_level.py @@ -72,6 +72,10 @@ def test_translate_level(): l_trans = level.translate_level(l) assert isinstance(l_trans, level.Alert) + l = level.translate_level(1) + assert isinstance(l, level.Alert) + assert l.name == 'alert' + l = level.translate_level('1') assert isinstance(l, level.Alert) assert l.name == 'alert' diff --git a/test/test_verbs.py b/test/test_verbs.py index d38bd81..dba41cb 100644 --- a/test/test_verbs.py +++ b/test/test_verbs.py @@ -92,6 +92,10 @@ def test_translate_verb(): v_trans = verbs.translate_verb(v) assert isinstance(v_trans, verbs.Request) + v = verbs.translate_verb(1) + assert isinstance(v, verbs.Invite) + assert v.infinitive == 'invite' + v = verbs.translate_verb('1') assert isinstance(v, verbs.Invite) assert v.infinitive == 'invite' From 6f4a127d77b4b0b4b6e15118ce928c07cdcabc9f Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Thu, 25 Oct 2018 17:31:34 -0700 Subject: [PATCH 12/13] finished testing notification --- feeds/activity/notification.py | 38 +++++++++--- feeds/exceptions.py | 7 +++ test/activity/test_notification.py | 96 ++++++++++++++++++++++++++++-- 3 files changed, 128 insertions(+), 13 deletions(-) diff --git a/feeds/activity/notification.py b/feeds/activity/notification.py index 9b76e86..48ff61e 100644 --- a/feeds/activity/notification.py +++ b/feeds/activity/notification.py @@ -5,7 +5,10 @@ from .. import verbs from ..actor import validate_actor from .. import notification_level -from feeds.exceptions import InvalidExpirationError +from feeds.exceptions import ( + InvalidExpirationError, + InvalidNotificationError +) import datetime from feeds.config import get_config @@ -145,7 +148,7 @@ def user_view(self): def serialize(self): """ - Serializes this notification for caching / simple storage. + Serializes this notification to a string for caching / simple storage. Assumes it's been validated. Just dumps it all to a json string. """ @@ -159,7 +162,8 @@ def serialize(self): "l": self.level.id, "c": self.created, "e": self.expires, - "x": self.external_key + "x": self.external_key, + "n": self.context } return json.dumps(serial, separators=(',', ':')) @@ -168,9 +172,18 @@ def deserialize(cls, serial): """ Deserializes and returns a new Notification instance. """ - if serial is None: - return None - struct = json.loads(serial) + try: + assert serial + except AssertionError: + raise InvalidNotificationError("Can't deserialize an input of 'None'") + try: + struct = json.loads(serial) + except json.JSONDecodeError: + raise InvalidNotificationError("Can only deserialize a JSON string") + required_keys = set(['a', 'v', 'o', 's', 'l', 't', 'c', 'i', 'e']) + missing_keys = required_keys.difference(struct.keys()) + if missing_keys: + raise InvalidNotificationError('Missing keys: {}'.format(missing_keys)) deserial = cls( struct['a'], str(struct['v']), @@ -178,7 +191,7 @@ def deserialize(cls, serial): struct['s'], level=str(struct['l']), target=struct.get('t'), - context=struct.get('c'), + context=struct.get('n'), external_key=struct.get('x') ) deserial.created = struct['c'] @@ -191,7 +204,16 @@ def from_dict(cls, serial): """ Returns a new Notification from a serialized dictionary (e.g. used in Mongo) """ - assert serial + try: + assert serial is not None and isinstance(serial, dict) + except AssertionError: + raise InvalidNotificationError("Can only run 'from_dict' on a dict.") + required_keys = set([ + 'actor', 'verb', 'object', 'source', 'level', 'created', 'expires', 'id' + ]) + missing_keys = required_keys.difference(set(serial.keys())) + if missing_keys: + raise InvalidNotificationError('Missing keys: {}'.format(missing_keys)) deserial = cls( serial['actor'], str(serial['verb']), diff --git a/feeds/exceptions.py b/feeds/exceptions.py index fdf8b96..0cce2a5 100644 --- a/feeds/exceptions.py +++ b/feeds/exceptions.py @@ -90,3 +90,10 @@ class InvalidExpirationError(Exception): Raised when trying to give a Notification an invalid expiration time. """ pass + + +class InvalidNotificationError(Exception): + """ + Raised when trying to deserialize a Notification that has been stored badly. + """ + pass diff --git a/test/activity/test_notification.py b/test/activity/test_notification.py index bc91298..c6d3ee2 100644 --- a/test/activity/test_notification.py +++ b/test/activity/test_notification.py @@ -8,7 +8,8 @@ from feeds.exceptions import ( MissingVerbError, MissingLevelError, - InvalidExpirationError + InvalidExpirationError, + InvalidNotificationError ) cfg = test_config() @@ -239,9 +240,94 @@ def test_from_dict(): note = Notification.from_dict(note_d) assert_note_ok(note, **note_d) -def test_serialize(): - pass +def test_from_dict_missing_keys(): + d = { + "actor": actor + } + with pytest.raises(InvalidNotificationError) as e: + Notification.from_dict(d) + assert "Missing keys" in str(e.value) + + with pytest.raises(InvalidNotificationError) as e: + Notification.from_dict(None) + assert "Can only run 'from_dict' on a dict" in str(e.value) -def test_deserialize(): - pass + +def test_serialization(): + note = Notification(actor, verb_inf, note_object, source, level=level_id) + serial = note.serialize() + json_serial = json.loads(serial) + # serial = { + # "i": self.id, + # "a": self.actor, + # "v": self.verb.id, + # "o": self.object, + # "s": self.source, + # "t": self.target, + # "l": self.level.id, + # "c": self.created, + # "e": self.expires, + # "x": self.external_key + # } + assert "i" in json_serial + assert_is_uuid(json_serial['i']) + assert "a" in json_serial and json_serial['a'] == actor + assert "v" in json_serial and json_serial['v'] == verb_id + assert "o" in json_serial and json_serial['o'] == note_object + assert "s" in json_serial and json_serial['s'] == source + assert "l" in json_serial and json_serial['l'] == level_id + assert "c" in json_serial and json_serial['c'] == note.created + assert "e" in json_serial and json_serial['e'] == note.expires + assert "n" in json_serial and json_serial['n'] == None + assert "x" in json_serial and json_serial['x'] == None + assert "t" in json_serial and json_serial['t'] == None + + +def test_serialization_all_kwargs(): + note = Notification(actor, verb_inf, note_object, source, level=level_id, + target=target, external_key=external_key, context=context) + serial = note.serialize() + json_serial = json.loads(serial) + assert "i" in json_serial + assert_is_uuid(json_serial['i']) + assert "a" in json_serial and json_serial['a'] == actor + assert "v" in json_serial and json_serial['v'] == verb_id + assert "o" in json_serial and json_serial['o'] == note_object + assert "s" in json_serial and json_serial['s'] == source + assert "l" in json_serial and json_serial['l'] == level_id + assert "c" in json_serial and json_serial['c'] == note.created + assert "e" in json_serial and json_serial['e'] == note.expires + assert "n" in json_serial and json_serial['n'] == context + assert "x" in json_serial and json_serial['x'] == external_key + assert "t" in json_serial and json_serial['t'] == target + + +def test_deserialization(): + note = Notification(actor, verb_inf, note_object, source, level=level_id, + target=target, external_key=external_key, context=context) + serial = note.serialize() + note2 = Notification.deserialize(serial) + assert note2.id == note.id + assert note2.actor == note.actor + assert note2.verb.id == note.verb.id + assert note2.object == note.object + assert note2.source == note.source + assert note2.level.id == note.level.id + assert note2.target == note.target + assert note2.external_key == note.external_key + assert note2.context == note.context + + +def test_deserialize_bad(): + with pytest.raises(InvalidNotificationError) as e: + Notification.deserialize(None) + assert "Can't deserialize an input of 'None'" in str(e.value) + + with pytest.raises(InvalidNotificationError) as e: + Notification.deserialize(json.dumps({'a': actor})) + assert "Missing keys" in str(e.value) + + with pytest.raises(InvalidNotificationError) as e: + Notification.deserialize("foo") + assert "Can only deserialize a JSON string" in str(e.value) \ No newline at end of file From 69907dde0579990c8e94b2458d69e6b58aff911e Mon Sep 17 00:00:00 2001 From: Bill Riehl Date: Thu, 25 Oct 2018 17:55:46 -0700 Subject: [PATCH 13/13] some type hints --- feeds/activity/notification.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/feeds/activity/notification.py b/feeds/activity/notification.py index 48ff61e..ccf41f7 100644 --- a/feeds/activity/notification.py +++ b/feeds/activity/notification.py @@ -14,8 +14,8 @@ class Notification(BaseActivity): - def __init__(self, actor, verb, note_object, source, level='alert', target=None, - context=None, expires=None, external_key=None): + def __init__(self, actor: str, verb, note_object: str, source: str, level='alert', + target: list=None, context: dict=None, expires: int=None, external_key: str=None): """ A notification is roughly of this form: actor, verb, object, (target) @@ -83,7 +83,7 @@ def validate(self): self.validate_expiration(self.expires, self.created) validate_actor(self.actor) - def validate_expiration(self, expires, created): + def validate_expiration(self, expires: int, created: int): """ Validates whether the expiration time is valid and after the created time. If yes, returns True. If not, raises an InvalidExpirationError. @@ -101,13 +101,13 @@ def validate_expiration(self, expires, created): "Notifications should expire sometime after they are created" ) - def _default_lifespan(self): + def _default_lifespan(self) -> int: """ Returns the default lifespan of this notification in ms. """ return get_config().lifespan * 24 * 60 * 60 * 1000 - def to_dict(self): + def to_dict(self) -> dict: """ Returns a dict form of the Notification. Useful for storing in a document store, returns the id of each verb and level. @@ -128,7 +128,7 @@ def to_dict(self): } return dict_form - def user_view(self): + def user_view(self) -> dict: """ Returns a view of the Notification that's intended for the user. That means we leave out the target and external keys. @@ -146,7 +146,7 @@ def user_view(self): } return view - def serialize(self): + def serialize(self) -> str: """ Serializes this notification to a string for caching / simple storage. Assumes it's been validated. @@ -168,7 +168,7 @@ def serialize(self): return json.dumps(serial, separators=(',', ':')) @classmethod - def deserialize(cls, serial): + def deserialize(cls, serial: str): """ Deserializes and returns a new Notification instance. """ @@ -200,7 +200,7 @@ def deserialize(cls, serial): return deserial @classmethod - def from_dict(cls, serial): + def from_dict(cls, serial: dict): """ Returns a new Notification from a serialized dictionary (e.g. used in Mongo) """