Files
notifications-api/tests/conftest.py

228 lines
7.0 KiB
Python
Raw Normal View History

import os
2021-03-10 13:55:06 +00:00
from contextlib import contextmanager
import pytest
import sqlalchemy
2021-03-10 13:55:06 +00:00
from alembic.command import upgrade
from alembic.config import Config
2022-10-25 11:53:24 -04:00
from flask import Flask
from app import create_app
from app.dao.provider_details_dao import get_provider_details_by_identifier
@pytest.fixture(scope='session')
def notify_app(worker_id):
2017-11-22 17:54:59 +00:00
app = Flask('test')
# Override the SQLALCHEMY_DATABASE_URI config before the app is
# initialized to account for Flask-SQLAlchemy 3.0.x changes.
# What is ultimately happening is the create_engine call made with
# SQLAlchemy itself is now only happening at the time of calling
# init_app with Flask instead of at the time it is first accessed,
# which the _notify_db fixture method was relying on.
# See the following for more information:
# https://github.com/pallets-eco/flask-sqlalchemy/pull/1087
# https://flask-sqlalchemy.palletsprojects.com/en/3.0.x/api/#module-flask_sqlalchemy
app.config['SQLALCHEMY_DATABASE_URI'] = '{}_{}'.format(
os.getenv('SQLALCHEMY_DATABASE_TEST_URI', '').replace('postgres://', 'postgresql://'),
worker_id
)
2017-11-22 17:54:59 +00:00
create_app(app)
return app
@pytest.fixture(scope='session')
def notify_api(notify_app):
# deattach server-error error handlers - error_handler_spec looks like:
# {'blueprint_name': {
# status_code: [error_handlers],
2017-08-18 15:39:39 +01:00
# None: { ExceptionClass: error_handler }
# }}
for error_handlers in notify_app.error_handler_spec.values():
error_handlers.pop(500, None)
if None in error_handlers:
2017-08-18 15:39:39 +01:00
error_handlers[None] = {
exc_class: error_handler
for exc_class, error_handler in error_handlers[None].items()
if exc_class != Exception
}
if error_handlers[None] == []:
error_handlers.pop(None)
ctx = notify_app.app_context()
ctx.push()
yield notify_app
ctx.pop()
@pytest.fixture(scope='function')
def client(notify_api):
with notify_api.test_request_context(), notify_api.test_client() as client:
yield client
def create_test_db(database_uri):
# get the
db_uri_parts = database_uri.split('/')
postgres_db_uri = '/'.join(db_uri_parts[:-1] + ['postgres'])
postgres_db = sqlalchemy.create_engine(
postgres_db_uri,
echo=False,
isolation_level='AUTOCOMMIT',
client_encoding='utf8'
)
try:
result = postgres_db.execute(sqlalchemy.sql.text('CREATE DATABASE {}'.format(db_uri_parts[-1])))
result.close()
except sqlalchemy.exc.ProgrammingError:
# database "test_notification_api_master" already exists
pass
finally:
postgres_db.dispose()
@pytest.fixture(scope='session')
def _notify_db(notify_api, worker_id):
"""
Manages the connection to the database. Generally this shouldn't be used, instead you should use the
`notify_db_session` fixture which also cleans up any data you've got left over after your test run.
"""
# Create a database for this worker thread; note that we still have
# to reset it here to point to the correct database.
from flask import current_app
current_app.config['SQLALCHEMY_DATABASE_URI'] += '_{}'.format(
worker_id
)
create_test_db(current_app.config['SQLALCHEMY_DATABASE_URI'])
BASE_DIR = os.path.dirname(os.path.dirname(__file__))
ALEMBIC_CONFIG = os.path.join(BASE_DIR, 'migrations')
config = Config(ALEMBIC_CONFIG + '/alembic.ini')
config.set_main_option("script_location", ALEMBIC_CONFIG)
with notify_api.app_context():
upgrade(config, 'head')
# Retrieve the DB object from the initialized app.
db = notify_api.extensions['sqlalchemy']
# Check the DB name.
assert 'test_notification_api' in db.engine.url.database, 'dont run tests against main db'
# Modify the URL to point to the correct test database.
db.engine.url = current_app.config['SQLALCHEMY_DATABASE_URI']
yield db
db.session.remove()
db.engine.dispose()
@pytest.fixture(scope='function')
2022-05-03 17:13:03 +01:00
def sms_providers(_notify_db):
"""
In production we randomly choose which provider to use based on their priority. To guarantee tests run the same each
time, make sure we always choose sns. You'll need to override them in your tests if you wish to do something
different.
"""
get_provider_details_by_identifier('sns').priority = 100
@pytest.fixture(scope='function')
2022-05-03 17:13:03 +01:00
def notify_db_session(_notify_db, sms_providers):
"""
This fixture clears down all non static data after your test run. It yields the sqlalchemy session variable
so you can manually add, commit, etc if needed.
`notify_db_session.commit()`
"""
2022-05-03 17:13:03 +01:00
yield _notify_db.session
2022-05-03 17:13:03 +01:00
_notify_db.session.remove()
for tbl in reversed(_notify_db.metadata.sorted_tables):
if tbl.name not in ["provider_details",
"key_types",
"branding_type",
"job_status",
"provider_details_history",
"template_process_type",
Use notification view for status / billing tasks This fixes a bug where (letter) notifications left in sending would temporarily get excluded from billing and status calculations once the service retention period had elapsed, and then get included once again when they finally get marked as delivered.* Status and billing tasks shouldn't need to have knowledge about which table their data is in and getting this wrong is the fundamental cause of the bug here. Adding a view across both tables abstracts this away while keeping the query complexity the same. Using a view also has the added benefit that we no longer need to care when the status / billing tasks run in comparison to the deletion task, since we will retrieve the same data irrespective (see below for a more detailed discussion on data integrity). *Such a scenario is rare but has happened. A New View ========== I've included all the columns that are shared between the two tables, even though only a subset are actually needed. Having extra columns has no impact and may be useful in future. Although the view isn't actually a table, SQLAlchemy appears to wrap it without any issues, noting that the package doesn't have any direct support for "view models". Because we're never inserting data, we don't need most of the kwargs when defining columns.* *Note that the "default" kwarg doesn't affect data that's retrieved, only data that's written (if no value is set). Data Integrity ============== The (new) tests cover the main scenarios. We need to be careful with how the view interacts with the deletion / archiving task. There are two concerns here: - Duplicates. The deletion task inserts before it deletes [^1], so we could end up double counting. It turns out this isn't a problem because a Postgres UNION is an implicit "DISTINCT" [^2]. I've also verified this manually, just to be on the safe side. - No data. It's conceivable that the query will check the history table just before the insertion, then check the notifications table just after the deletion. It turns out this isn't a problem either because the whole query sees the same DB snapshot [^3][^4].* *I can't think of a way to test this as it's a race condition, but I'm confident the Postgres docs are accurate. Performance =========== I copied the relevant (non-PII) columns from Production for data going back to 2022-04-01. I then ran several tests. Queries using the new view still make use of indices on a per-table basis, as the following query plan illustrates: QUERY PLAN ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ GroupAggregate (cost=1130820.02..1135353.89 rows=46502 width=97) (actual time=629.863..756.703 rows=72 loops=1) Group Key: notifications_all_time_view.template_id, notifications_all_time_view.sent_by, notifications_all_time_view.rate_multiplier, notifications_all_time_view.international -> Sort (cost=1130820.02..1131401.28 rows=232506 width=85) (actual time=629.756..708.914 rows=217563 loops=1) Sort Key: notifications_all_time_view.template_id, notifications_all_time_view.sent_by, notifications_all_time_view.rate_multiplier, notifications_all_time_view.international Sort Method: external merge Disk: 9320kB -> Subquery Scan on notifications_all_time_view (cost=1088506.43..1098969.20 rows=232506 width=85) (actual time=416.118..541.669 rows=217563 loops=1) -> Unique (cost=1088506.43..1096644.14 rows=232506 width=725) (actual time=416.115..513.065 rows=217563 loops=1) -> Sort (cost=1088506.43..1089087.70 rows=232506 width=725) (actual time=416.115..451.190 rows=217563 loops=1) Sort Key: notifications_no_pii.id, notifications_no_pii.job_id, notifications_no_pii.service_id, notifications_no_pii.template_id, notifications_no_pii.key_type, notifications_no_pii.billable_units, notifications_no_pii.notification_type, notifications_no_pii.created_at, notifications_no_pii.sent_by, notifications_no_pii.notification_status, notifications_no_pii.international, notifications_no_pii.rate_multiplier, notifications_no_pii.postage Sort Method: external merge Disk: 23936kB -> Append (cost=114.42..918374.12 rows=232506 width=725) (actual time=2.051..298.229 rows=217563 loops=1) -> Bitmap Heap Scan on notifications_no_pii (cost=114.42..8557.55 rows=2042 width=113) (actual time=1.405..1.442 rows=0 loops=1) Recheck Cond: ((service_id = 'c5956607-20b1-48b4-8983-85d11404e61f'::uuid) AND (notification_type = 'sms'::notification_type) AND (notification_status = ANY ('{sending,sent,delivered,pending,temporary-failure,permanent-failure}'::text[])) AND (created_at >= '2022-05-01 23:00:00'::timestamp without time zone) AND (created_at < '2022-05-02 23:00:00'::timestamp without time zone)) Filter: ((key_type)::text = ANY ('{normal,team}'::text[])) -> Bitmap Index Scan on ix_notifications_no_piiservice_id_composite (cost=0.00..113.91 rows=2202 width=0) (actual time=1.402..1.439 rows=0 loops=1) Index Cond: ((service_id = 'c5956607-20b1-48b4-8983-85d11404e61f'::uuid) AND (notification_type = 'sms'::notification_type) AND (notification_status = ANY ('{sending,sent,delivered,pending,temporary-failure,permanent-failure}'::text[])) AND (created_at >= '2022-05-01 23:00:00'::timestamp without time zone) AND (created_at < '2022-05-02 23:00:00'::timestamp without time zone)) -> Index Scan using ix_notifications_history_no_pii_service_id_composite on notifications_history_no_pii (cost=0.70..906328.97 rows=230464 width=113) (actual time=0.645..281.612 rows=217563 loops=1) Index Cond: ((service_id = 'c5956607-20b1-48b4-8983-85d11404e61f'::uuid) AND ((key_type)::text = ANY ('{normal,team}'::text[])) AND (notification_type = 'sms'::notification_type) AND (created_at >= '2022-05-01 23:00:00'::timestamp without time zone) AND (created_at < '2022-05-02 23:00:00'::timestamp without time zone)) Filter: (notification_status = ANY ('{sending,sent,delivered,pending,temporary-failure,permanent-failure}'::text[])) Planning Time: 18.032 ms Execution Time: 759.001 ms (21 rows) Queries using the new view appear to be slower than without, but the differences I've seen are minimal: the original queries execute in seconds locally and in Production, so it's not a big issue. Notes: Performance ================== I downloaded a minimal set of columns for testing: \copy ( select id, notification_type, key_type, created_at, service_id, template_id, sent_by, rate_multiplier, international, billable_units, postage, job_id, notification_status from notifications ) to 'notifications.csv' delimiter ',' csv header; CREATE TABLE notifications_no_pii ( id uuid NOT NULL, notification_type public.notification_type NOT NULL, key_type character varying(255) NOT NULL, created_at timestamp without time zone NOT NULL, service_id uuid, template_id uuid, sent_by character varying, rate_multiplier numeric, international boolean, billable_units integer NOT NULL, postage character varying, job_id uuid, notification_status text ); copy notifications_no_pii from '/Users/ben.thorner/Desktop/notifications.csv' delimiter ',' csv header; CREATE INDEX ix_notifications_no_piicreated_at ON notifications_no_pii USING btree (created_at); CREATE INDEX ix_notifications_no_piijob_id ON notifications_no_pii USING btree (job_id); CREATE INDEX ix_notifications_no_piinotification_type_composite ON notifications_no_pii USING btree (notification_type, notification_status, created_at); CREATE INDEX ix_notifications_no_piiservice_created_at ON notifications_no_pii USING btree (service_id, created_at); CREATE INDEX ix_notifications_no_piiservice_id_composite ON notifications_no_pii USING btree (service_id, notification_type, notification_status, created_at); CREATE INDEX ix_notifications_no_piitemplate_id ON notifications_no_pii USING btree (template_id); And similarly for the history table. I then created a sepatate view across both of these temporary tables using just these columns. To test performance I created some queries that reflect what is run by the billing [^5] and status [^6] tasks e.g. explain analyze select template_id, sent_by, rate_multiplier, international, sum(billable_units), count(*) from notifications_all_time_view where notification_status in ('sending', 'sent', 'delivered', 'pending', 'temporary-failure', 'permanent-failure') and key_type in ('normal', 'team') and created_at >= '2022-05-01 23:00' and created_at < '2022-05-02 23:00' and notification_type = 'sms' and service_id = 'c5956607-20b1-48b4-8983-85d11404e61f' group by 1,2,3,4; explain analyze select template_id, job_id, key_type, notification_status, count(*) from notifications_all_time_view where created_at >= '2022-05-01 23:00' and created_at < '2022-05-02 23:00' and notification_type = 'sms' and service_id = 'c5956607-20b1-48b4-8983-85d11404e61f' and key_type in ('normal', 'team') group by 1,2,3,4; Between running queries I restarted my local database and also ran a command to purge disk caches [^7]. I tested on a few services: - c5956607-20b1-48b4-8983-85d11404e61f on 2022-05-02 (high volume) - 0cc696c6-b792-409d-99e9-64232f461b0f on 2022-04-06 (highest volume) - 01135db6-7819-4121-8b97-4aa2d741e372 on 2022-04-14 (very low volume) All execution results are of the same magnitude using the view compared to the worst case of either table on its own. [^1]: https://github.com/alphagov/notifications-api/blob/00a04ebf54c97fc695f013de0a497e5490ddb558/app/dao/notifications_dao.py#L389 [^2]: https://stackoverflow.com/questions/49925/what-is-the-difference-between-union-and-union-all [^3]: https://www.postgresql.org/docs/current/transaction-iso.html [^4]: https://dba.stackexchange.com/questions/210485/can-sub-selects-change-in-one-single-query-in-a-read-committed-transaction [^5]: https://github.com/alphagov/notifications-api/blob/00a04ebf54c97fc695f013de0a497e5490ddb558/app/dao/fact_billing_dao.py#L471 [^6]: https://github.com/alphagov/notifications-api/blob/00a04ebf54c97fc695f013de0a497e5490ddb558/app/dao/fact_notification_status_dao.py#L58 [^7]: https://stackoverflow.com/questions/28845524/echo-3-proc-sys-vm-drop-caches-on-mac-osx
2022-05-19 11:28:44 +01:00
"notifications_all_time_view",
"notification_status_types",
2023-07-10 11:06:29 -07:00
"organization_types",
"service_permission_types",
"auth_type",
"invite_status_type",
"service_callback_type"]:
2022-05-03 17:13:03 +01:00
_notify_db.engine.execute(tbl.delete())
_notify_db.session.commit()
2016-12-08 12:12:45 +00:00
@pytest.fixture
def os_environ():
"""
clear os.environ, and restore it after the test runs
"""
# for use whenever you expect code to edit environment variables
old_env = os.environ.copy()
os.environ.clear()
yield
# clear afterwards in case anything extra was added to the environment during the test
os.environ.clear()
for k, v in old_env.items():
os.environ[k] = v
def pytest_generate_tests(metafunc):
# Copied from https://gist.github.com/pfctdayelise/5719730
2018-10-26 17:54:53 +01:00
idparametrize = metafunc.definition.get_closest_marker('idparametrize')
if idparametrize:
argnames, testdata = idparametrize.args
ids, argvalues = zip(*sorted(testdata.items()))
metafunc.parametrize(argnames, argvalues, ids=ids)
@contextmanager
def set_config(app, name, value):
old_val = app.config.get(name)
app.config[name] = value
try:
yield
finally:
app.config[name] = old_val
@contextmanager
def set_config_values(app, dict):
old_values = {}
for key in dict:
old_values[key] = app.config.get(key)
app.config[key] = dict[key]
try:
yield
finally:
for key in dict:
app.config[key] = old_values[key]
class Matcher:
def __init__(self, description, key):
self.description = description
self.key = key
def __eq__(self, other):
return self.key(other)
def __repr__(self):
return '<Matcher: {}>'.format(self.description)